[v3] hw/loongarch: add the advanced extended interrupt controllers (AVECINTC) support

[PATCH v3 2/9] hw/loongarch: add virt feature avecintc support

Posted by Song Gao 4 months, 3 weeks ago

LoongArchVirtMachinState adds avecintc features, and
it use to check whether virt machine support advance interrupt controller
and default set avecintc = ON_OFF_AUTO_ON.
LoongArchVirtMachineState adds misc_feature and misc_status for
misc fetures and status. and set default avec feture bit.

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 hw/loongarch/virt.c         | 43 +++++++++++++++++++++++++++++++++----
 include/hw/loongarch/virt.h | 15 +++++++++++++
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 6a169d4824..426eaaef84 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -47,6 +47,28 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "qemu/error-report.h"
 
+static void virt_get_avecintc(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
+    OnOffAuto avecintc = lvms->avecintc;
+
+    visit_type_OnOffAuto(v, name, &avecintc, errp);
+
+}
+static void virt_set_avecintc(Object *obj, Visitor *v, const char *name,
+                              void *opaque, Error **errp)
+{
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
+
+    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
+        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
+        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
+    }
+
+    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
+}
+
 static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
@@ -539,6 +561,10 @@ static MemTxResult virt_iocsr_misc_write(void *opaque, hwaddr addr,
             return MEMTX_OK;
         }
 
+        if (val & BIT(IOCSRM_AVEC_EN)) {
+            lvms->misc_status |= BIT(IOCSRM_AVEC_EN);
+        }
+
         features = address_space_ldl(&lvms->as_iocsr,
                                      EXTIOI_VIRT_BASE + EXTIOI_VIRT_CONFIG,
                                      attrs, NULL);
@@ -574,8 +600,9 @@ static MemTxResult virt_iocsr_misc_read(void *opaque, hwaddr addr,
         break;
     case FEATURE_REG:
         ret = BIT(IOCSRF_MSI) | BIT(IOCSRF_EXTIOI) | BIT(IOCSRF_CSRIPI);
-        /*TODO: check bit IOCSRF_AVEC with virt_is_avec_enabled */
-        ret |= BIT(IOCSRF_AVEC);
+        if (virt_is_avecintc_enabled(lvms)) {
+            ret |= BIT(IOCSRF_AVEC);
+        }
         if (kvm_enabled()) {
             ret |= BIT(IOCSRF_VM);
         }
@@ -605,8 +632,10 @@ static MemTxResult virt_iocsr_misc_read(void *opaque, hwaddr addr,
         if (features & BIT(EXTIOI_ENABLE_INT_ENCODE)) {
             ret |= BIT_ULL(IOCSRM_EXTIOI_INT_ENCODE);
         }
-        /* enable avec default */
-        ret |= BIT_ULL(IOCSRM_AVEC_EN);
+        if (virt_is_avecintc_enabled(lvms) &&
+            (lvms->misc_status & BIT(IOCSRM_AVEC_EN))) {
+            ret |= BIT_ULL(IOCSRM_AVEC_EN);
+        }
         break;
     default:
         g_assert_not_reached();
@@ -850,6 +879,8 @@ static void virt_initfn(Object *obj)
     if (tcg_enabled()) {
         lvms->veiointc = ON_OFF_AUTO_OFF;
     }
+    lvms->misc_feature = BIT(IOCSRF_AVEC);
+    lvms->avecintc = ON_OFF_AUTO_ON;
     lvms->acpi = ON_OFF_AUTO_AUTO;
     lvms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
     lvms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
@@ -1242,6 +1273,10 @@ static void virt_class_init(ObjectClass *oc, const void *data)
         NULL, NULL);
     object_class_property_set_description(oc, "v-eiointc",
                             "Enable Virt Extend I/O Interrupt Controller.");
+    object_class_property_add(oc, "avecintc", "OnOffAuto",
+        virt_get_avecintc, virt_set_avecintc, NULL, NULL);
+    object_class_property_set_description(oc, "avecintc",
+                            "Enable Advance Interrupt Controller.");
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
 #ifdef CONFIG_TPM
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index cc6656619d..44504e5501 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -70,6 +70,7 @@ struct LoongArchVirtMachineState {
     Notifier     powerdown_notifier;
     OnOffAuto    acpi;
     OnOffAuto    veiointc;
+    OnOffAuto    avecintc;
     char         *oem_id;
     char         *oem_table_id;
     DeviceState  *acpi_ged;
@@ -85,6 +86,8 @@ struct LoongArchVirtMachineState {
     DeviceState *extioi;
     struct memmap_entry *memmap_table;
     unsigned int memmap_entries;
+    uint64_t misc_feature;
+    uint64_t misc_status;
 };
 
 #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
@@ -92,6 +95,18 @@ OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, LOONGARCH_VIRT_MACHINE)
 void virt_acpi_setup(LoongArchVirtMachineState *lvms);
 void virt_fdt_setup(LoongArchVirtMachineState *lvms);
 
+static inline bool virt_is_avecintc_enabled(LoongArchVirtMachineState *lvms)
+{
+    if (!(lvms->misc_feature & BIT(IOCSRF_AVEC))) {
+        return false;
+    }
+
+    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
+        return false;
+    }
+    return true;
+}
+
 static inline bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
 {
     if (lvms->veiointc == ON_OFF_AUTO_OFF) {
-- 
2.34.1

Re: [PATCH v3 2/9] hw/loongarch: add virt feature avecintc support

Posted by Bibo Mao 4 months, 2 weeks ago


On 2025/6/27 上午11:01, Song Gao wrote:
> LoongArchVirtMachinState adds avecintc features, and
> it use to check whether virt machine support advance interrupt controller
> and default set avecintc = ON_OFF_AUTO_ON.
> LoongArchVirtMachineState adds misc_feature and misc_status for
> misc fetures and status. and set default avec feture bit.
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   hw/loongarch/virt.c         | 43 +++++++++++++++++++++++++++++++++----
>   include/hw/loongarch/virt.h | 15 +++++++++++++
>   2 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 6a169d4824..426eaaef84 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -47,6 +47,28 @@
>   #include "hw/virtio/virtio-iommu.h"
>   #include "qemu/error-report.h"
>   
> +static void virt_get_avecintc(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)
> +{
> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
> +    OnOffAuto avecintc = lvms->avecintc;
> +
> +    visit_type_OnOffAuto(v, name, &avecintc, errp);
> +
> +}
> +static void virt_set_avecintc(Object *obj, Visitor *v, const char *name,
> +                              void *opaque, Error **errp)
> +{
> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
> +
> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
> +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
> +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
> +    }
> +
> +    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
It is a little strange that firstly check variable and then set it, I 
think that visit_type_OnOffAuto() should be before
  +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
  +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
  +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
  +    }

> +}
> +
>   static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
>                                 void *opaque, Error **errp)
>   {
> @@ -539,6 +561,10 @@ static MemTxResult virt_iocsr_misc_write(void *opaque, hwaddr addr,
>               return MEMTX_OK;
>           }
>   
> +        if (val & BIT(IOCSRM_AVEC_EN)) {
> +            lvms->misc_status |= BIT(IOCSRM_AVEC_EN);
> +        }
> +
>           features = address_space_ldl(&lvms->as_iocsr,
>                                        EXTIOI_VIRT_BASE + EXTIOI_VIRT_CONFIG,
>                                        attrs, NULL);
> @@ -574,8 +600,9 @@ static MemTxResult virt_iocsr_misc_read(void *opaque, hwaddr addr,
>           break;
>       case FEATURE_REG:
>           ret = BIT(IOCSRF_MSI) | BIT(IOCSRF_EXTIOI) | BIT(IOCSRF_CSRIPI);
> -        /*TODO: check bit IOCSRF_AVEC with virt_is_avec_enabled */
> -        ret |= BIT(IOCSRF_AVEC);
> +        if (virt_is_avecintc_enabled(lvms)) {
> +            ret |= BIT(IOCSRF_AVEC);
> +        }
>           if (kvm_enabled()) {
>               ret |= BIT(IOCSRF_VM);
>           }
> @@ -605,8 +632,10 @@ static MemTxResult virt_iocsr_misc_read(void *opaque, hwaddr addr,
>           if (features & BIT(EXTIOI_ENABLE_INT_ENCODE)) {
>               ret |= BIT_ULL(IOCSRM_EXTIOI_INT_ENCODE);
>           }
> -        /* enable avec default */
> -        ret |= BIT_ULL(IOCSRM_AVEC_EN);
> +        if (virt_is_avecintc_enabled(lvms) &&
> +            (lvms->misc_status & BIT(IOCSRM_AVEC_EN))) {
> +            ret |= BIT_ULL(IOCSRM_AVEC_EN);
> +        }
>           break;
>       default:
>           g_assert_not_reached();
> @@ -850,6 +879,8 @@ static void virt_initfn(Object *obj)
>       if (tcg_enabled()) {
>           lvms->veiointc = ON_OFF_AUTO_OFF;
>       }
> +    lvms->misc_feature = BIT(IOCSRF_AVEC);
> +    lvms->avecintc = ON_OFF_AUTO_ON;
>       lvms->acpi = ON_OFF_AUTO_AUTO;
>       lvms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>       lvms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
> @@ -1242,6 +1273,10 @@ static void virt_class_init(ObjectClass *oc, const void *data)
>           NULL, NULL);
>       object_class_property_set_description(oc, "v-eiointc",
>                               "Enable Virt Extend I/O Interrupt Controller.");
> +    object_class_property_add(oc, "avecintc", "OnOffAuto",
> +        virt_get_avecintc, virt_set_avecintc, NULL, NULL);
> +    object_class_property_set_description(oc, "avecintc",
> +                            "Enable Advance Interrupt Controller.");
It is only can be added in TCG mode, in kvm mode avec should not be 
supported now.
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
>   #ifdef CONFIG_TPM
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index cc6656619d..44504e5501 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -70,6 +70,7 @@ struct LoongArchVirtMachineState {
>       Notifier     powerdown_notifier;
>       OnOffAuto    acpi;
>       OnOffAuto    veiointc;
> +    OnOffAuto    avecintc;
>       char         *oem_id;
>       char         *oem_table_id;
>       DeviceState  *acpi_ged;
> @@ -85,6 +86,8 @@ struct LoongArchVirtMachineState {
>       DeviceState *extioi;
>       struct memmap_entry *memmap_table;
>       unsigned int memmap_entries;
> +    uint64_t misc_feature;
> +    uint64_t misc_status;
>   };
>   
>   #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
> @@ -92,6 +95,18 @@ OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, LOONGARCH_VIRT_MACHINE)
>   void virt_acpi_setup(LoongArchVirtMachineState *lvms);
>   void virt_fdt_setup(LoongArchVirtMachineState *lvms);
>   
> +static inline bool virt_is_avecintc_enabled(LoongArchVirtMachineState *lvms)
> +{
> +    if (!(lvms->misc_feature & BIT(IOCSRF_AVEC))) {
> +        return false;
> +    }
> +
> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
> +        return false;
> +    }
Is it enough to only check variable misc_feature? checking avecintc 
seems unnecessary duplicated.

Regards
Bibo Mao
> +    return true;
> +}
> +
>   static inline bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
>   {
>       if (lvms->veiointc == ON_OFF_AUTO_OFF) {
>

Re: [PATCH v3 2/9] hw/loongarch: add virt feature avecintc support

Posted by gaosong 4 months, 2 weeks ago

在 2025/7/2 上午10:03, Bibo Mao 写道:
>
>
> On 2025/6/27 上午11:01, Song Gao wrote:
>> LoongArchVirtMachinState adds avecintc features, and
>> it use to check whether virt machine support advance interrupt 
>> controller
>> and default set avecintc = ON_OFF_AUTO_ON.
>> LoongArchVirtMachineState adds misc_feature and misc_status for
>> misc fetures and status. and set default avec feture bit.
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   hw/loongarch/virt.c         | 43 +++++++++++++++++++++++++++++++++----
>>   include/hw/loongarch/virt.h | 15 +++++++++++++
>>   2 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index 6a169d4824..426eaaef84 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -47,6 +47,28 @@
>>   #include "hw/virtio/virtio-iommu.h"
>>   #include "qemu/error-report.h"
>>   +static void virt_get_avecintc(Object *obj, Visitor *v, const char 
>> *name,
>> +                             void *opaque, Error **errp)
>> +{
>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>> +    OnOffAuto avecintc = lvms->avecintc;
>> +
>> +    visit_type_OnOffAuto(v, name, &avecintc, errp);
>> +
>> +}
>> +static void virt_set_avecintc(Object *obj, Visitor *v, const char 
>> *name,
>> +                              void *opaque, Error **errp)
>> +{
>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>> +
>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>> +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>> +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>> +    }
>> +
>> +    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
> It is a little strange that firstly check variable and then set it, I 
> think that visit_type_OnOffAuto() should be before
>  +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>  +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>  +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>  +    }
>
>> +}
>> +
>>   static void virt_get_veiointc(Object *obj, Visitor *v, const char 
>> *name,
>>                                 void *opaque, Error **errp)
>>   {
>> @@ -539,6 +561,10 @@ static MemTxResult virt_iocsr_misc_write(void 
>> *opaque, hwaddr addr,
>>               return MEMTX_OK;
>>           }
>>   +        if (val & BIT(IOCSRM_AVEC_EN)) {
>> +            lvms->misc_status |= BIT(IOCSRM_AVEC_EN);
>> +        }
>> +
>>           features = address_space_ldl(&lvms->as_iocsr,
>>                                        EXTIOI_VIRT_BASE + 
>> EXTIOI_VIRT_CONFIG,
>>                                        attrs, NULL);
>> @@ -574,8 +600,9 @@ static MemTxResult virt_iocsr_misc_read(void 
>> *opaque, hwaddr addr,
>>           break;
>>       case FEATURE_REG:
>>           ret = BIT(IOCSRF_MSI) | BIT(IOCSRF_EXTIOI) | 
>> BIT(IOCSRF_CSRIPI);
>> -        /*TODO: check bit IOCSRF_AVEC with virt_is_avec_enabled */
>> -        ret |= BIT(IOCSRF_AVEC);
>> +        if (virt_is_avecintc_enabled(lvms)) {
>> +            ret |= BIT(IOCSRF_AVEC);
>> +        }
>>           if (kvm_enabled()) {
>>               ret |= BIT(IOCSRF_VM);
>>           }
>> @@ -605,8 +632,10 @@ static MemTxResult virt_iocsr_misc_read(void 
>> *opaque, hwaddr addr,
>>           if (features & BIT(EXTIOI_ENABLE_INT_ENCODE)) {
>>               ret |= BIT_ULL(IOCSRM_EXTIOI_INT_ENCODE);
>>           }
>> -        /* enable avec default */
>> -        ret |= BIT_ULL(IOCSRM_AVEC_EN);
>> +        if (virt_is_avecintc_enabled(lvms) &&
>> +            (lvms->misc_status & BIT(IOCSRM_AVEC_EN))) {
>> +            ret |= BIT_ULL(IOCSRM_AVEC_EN);
>> +        }
>>           break;
>>       default:
>>           g_assert_not_reached();
>> @@ -850,6 +879,8 @@ static void virt_initfn(Object *obj)
>>       if (tcg_enabled()) {
>>           lvms->veiointc = ON_OFF_AUTO_OFF;
>>       }
>> +    lvms->misc_feature = BIT(IOCSRF_AVEC);
>> +    lvms->avecintc = ON_OFF_AUTO_ON;
>>       lvms->acpi = ON_OFF_AUTO_AUTO;
>>       lvms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>>       lvms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
>> @@ -1242,6 +1273,10 @@ static void virt_class_init(ObjectClass *oc, 
>> const void *data)
>>           NULL, NULL);
>>       object_class_property_set_description(oc, "v-eiointc",
>>                               "Enable Virt Extend I/O Interrupt 
>> Controller.");
>> +    object_class_property_add(oc, "avecintc", "OnOffAuto",
>> +        virt_get_avecintc, virt_set_avecintc, NULL, NULL);
>> +    object_class_property_set_description(oc, "avecintc",
>> +                            "Enable Advance Interrupt Controller.");
> It is only can be added in TCG mode, in kvm mode avec should not be 
> supported now.
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
>>   #ifdef CONFIG_TPM
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index cc6656619d..44504e5501 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -70,6 +70,7 @@ struct LoongArchVirtMachineState {
>>       Notifier     powerdown_notifier;
>>       OnOffAuto    acpi;
>>       OnOffAuto    veiointc;
>> +    OnOffAuto    avecintc;
>>       char         *oem_id;
>>       char         *oem_table_id;
>>       DeviceState  *acpi_ged;
>> @@ -85,6 +86,8 @@ struct LoongArchVirtMachineState {
>>       DeviceState *extioi;
>>       struct memmap_entry *memmap_table;
>>       unsigned int memmap_entries;
>> +    uint64_t misc_feature;
>> +    uint64_t misc_status;
>>   };
>>     #define TYPE_LOONGARCH_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
>> @@ -92,6 +95,18 @@ 
>> OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, 
>> LOONGARCH_VIRT_MACHINE)
>>   void virt_acpi_setup(LoongArchVirtMachineState *lvms);
>>   void virt_fdt_setup(LoongArchVirtMachineState *lvms);
>>   +static inline bool 
>> virt_is_avecintc_enabled(LoongArchVirtMachineState *lvms)
>> +{
>> +    if (!(lvms->misc_feature & BIT(IOCSRF_AVEC))) {
>> +        return false;
>> +    }
>> +
>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>> +        return false;
>> +    }
> Is it enough to only check variable misc_feature? checking avecintc 
> seems unnecessary duplicated.
>
  Someone may  disable avecintc with command  'avecintc = off'.

Thanks.
Song Gao.
> Regards
> Bibo Mao
>> +    return true;
>> +}
>> +
>>   static inline bool 
>> virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
>>   {
>>       if (lvms->veiointc == ON_OFF_AUTO_OFF) {
>>

Re: [PATCH v3 2/9] hw/loongarch: add virt feature avecintc support

Posted by Bibo Mao 4 months, 2 weeks ago


On 2025/7/2 下午3:10, gaosong wrote:
> 在 2025/7/2 上午10:03, Bibo Mao 写道:
>>
>>
>> On 2025/6/27 上午11:01, Song Gao wrote:
>>> LoongArchVirtMachinState adds avecintc features, and
>>> it use to check whether virt machine support advance interrupt 
>>> controller
>>> and default set avecintc = ON_OFF_AUTO_ON.
>>> LoongArchVirtMachineState adds misc_feature and misc_status for
>>> misc fetures and status. and set default avec feture bit.
>>>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>   hw/loongarch/virt.c         | 43 +++++++++++++++++++++++++++++++++----
>>>   include/hw/loongarch/virt.h | 15 +++++++++++++
>>>   2 files changed, 54 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>>> index 6a169d4824..426eaaef84 100644
>>> --- a/hw/loongarch/virt.c
>>> +++ b/hw/loongarch/virt.c
>>> @@ -47,6 +47,28 @@
>>>   #include "hw/virtio/virtio-iommu.h"
>>>   #include "qemu/error-report.h"
>>>   +static void virt_get_avecintc(Object *obj, Visitor *v, const char 
>>> *name,
>>> +                             void *opaque, Error **errp)
>>> +{
>>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>>> +    OnOffAuto avecintc = lvms->avecintc;
>>> +
>>> +    visit_type_OnOffAuto(v, name, &avecintc, errp);
>>> +
>>> +}
>>> +static void virt_set_avecintc(Object *obj, Visitor *v, const char 
>>> *name,
>>> +                              void *opaque, Error **errp)
>>> +{
>>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>>> +
>>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>> +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>>> +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>>> +    }
>>> +
>>> +    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
>> It is a little strange that firstly check variable and then set it, I 
>> think that visit_type_OnOffAuto() should be before
>>  +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>  +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>>  +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>>  +    }
>>
>>> +}
>>> +
>>>   static void virt_get_veiointc(Object *obj, Visitor *v, const char 
>>> *name,
>>>                                 void *opaque, Error **errp)
>>>   {
>>> @@ -539,6 +561,10 @@ static MemTxResult virt_iocsr_misc_write(void 
>>> *opaque, hwaddr addr,
>>>               return MEMTX_OK;
>>>           }
>>>   +        if (val & BIT(IOCSRM_AVEC_EN)) {
>>> +            lvms->misc_status |= BIT(IOCSRM_AVEC_EN);
>>> +        }
>>> +
>>>           features = address_space_ldl(&lvms->as_iocsr,
>>>                                        EXTIOI_VIRT_BASE + 
>>> EXTIOI_VIRT_CONFIG,
>>>                                        attrs, NULL);
>>> @@ -574,8 +600,9 @@ static MemTxResult virt_iocsr_misc_read(void 
>>> *opaque, hwaddr addr,
>>>           break;
>>>       case FEATURE_REG:
>>>           ret = BIT(IOCSRF_MSI) | BIT(IOCSRF_EXTIOI) | 
>>> BIT(IOCSRF_CSRIPI);
>>> -        /*TODO: check bit IOCSRF_AVEC with virt_is_avec_enabled */
>>> -        ret |= BIT(IOCSRF_AVEC);
>>> +        if (virt_is_avecintc_enabled(lvms)) {
>>> +            ret |= BIT(IOCSRF_AVEC);
>>> +        }
>>>           if (kvm_enabled()) {
>>>               ret |= BIT(IOCSRF_VM);
>>>           }
>>> @@ -605,8 +632,10 @@ static MemTxResult virt_iocsr_misc_read(void 
>>> *opaque, hwaddr addr,
>>>           if (features & BIT(EXTIOI_ENABLE_INT_ENCODE)) {
>>>               ret |= BIT_ULL(IOCSRM_EXTIOI_INT_ENCODE);
>>>           }
>>> -        /* enable avec default */
>>> -        ret |= BIT_ULL(IOCSRM_AVEC_EN);
>>> +        if (virt_is_avecintc_enabled(lvms) &&
>>> +            (lvms->misc_status & BIT(IOCSRM_AVEC_EN))) {
>>> +            ret |= BIT_ULL(IOCSRM_AVEC_EN);
>>> +        }
>>>           break;
>>>       default:
>>>           g_assert_not_reached();
>>> @@ -850,6 +879,8 @@ static void virt_initfn(Object *obj)
>>>       if (tcg_enabled()) {
>>>           lvms->veiointc = ON_OFF_AUTO_OFF;
>>>       }
>>> +    lvms->misc_feature = BIT(IOCSRF_AVEC);
>>> +    lvms->avecintc = ON_OFF_AUTO_ON;
>>>       lvms->acpi = ON_OFF_AUTO_AUTO;
>>>       lvms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>>>       lvms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
>>> @@ -1242,6 +1273,10 @@ static void virt_class_init(ObjectClass *oc, 
>>> const void *data)
>>>           NULL, NULL);
>>>       object_class_property_set_description(oc, "v-eiointc",
>>>                               "Enable Virt Extend I/O Interrupt 
>>> Controller.");
>>> +    object_class_property_add(oc, "avecintc", "OnOffAuto",
>>> +        virt_get_avecintc, virt_set_avecintc, NULL, NULL);
>>> +    object_class_property_set_description(oc, "avecintc",
>>> +                            "Enable Advance Interrupt Controller.");
>> It is only can be added in TCG mode, in kvm mode avec should not be 
>> supported now.
>>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>>>       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
>>>   #ifdef CONFIG_TPM
>>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>>> index cc6656619d..44504e5501 100644
>>> --- a/include/hw/loongarch/virt.h
>>> +++ b/include/hw/loongarch/virt.h
>>> @@ -70,6 +70,7 @@ struct LoongArchVirtMachineState {
>>>       Notifier     powerdown_notifier;
>>>       OnOffAuto    acpi;
>>>       OnOffAuto    veiointc;
>>> +    OnOffAuto    avecintc;
>>>       char         *oem_id;
>>>       char         *oem_table_id;
>>>       DeviceState  *acpi_ged;
>>> @@ -85,6 +86,8 @@ struct LoongArchVirtMachineState {
>>>       DeviceState *extioi;
>>>       struct memmap_entry *memmap_table;
>>>       unsigned int memmap_entries;
>>> +    uint64_t misc_feature;
>>> +    uint64_t misc_status;
>>>   };
>>>     #define TYPE_LOONGARCH_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
>>> @@ -92,6 +95,18 @@ 
>>> OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, 
>>> LOONGARCH_VIRT_MACHINE)
>>>   void virt_acpi_setup(LoongArchVirtMachineState *lvms);
>>>   void virt_fdt_setup(LoongArchVirtMachineState *lvms);
>>>   +static inline bool 
>>> virt_is_avecintc_enabled(LoongArchVirtMachineState *lvms)
>>> +{
>>> +    if (!(lvms->misc_feature & BIT(IOCSRF_AVEC))) {
>>> +        return false;
>>> +    }
>>> +
>>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>> +        return false;
>>> +    }
>> Is it enough to only check variable misc_feature? checking avecintc 
>> seems unnecessary duplicated.
>>
>   Someone may  disable avecintc with command  'avecintc = off'.
+static void virt_set_avecintc(Object *obj, Visitor *v, const char *name,
+                              void *opaque, Error **errp)
+{
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
+
+    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
+        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
+        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
+    }
With avecintc = off option, misc_feature will be cleared with 
IOCSRF_AVEC also.

Regards
Bibo Mao
> 
> Thanks.
> Song Gao.
>> Regards
>> Bibo Mao
>>> +    return true;
>>> +}
>>> +
>>>   static inline bool 
>>> virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
>>>   {
>>>       if (lvms->veiointc == ON_OFF_AUTO_OFF) {
>>>

Re: [PATCH v3 2/9] hw/loongarch: add virt feature avecintc support

Posted by gaosong 4 months, 2 weeks ago

在 2025/7/2 下午5:46, Bibo Mao 写道:
>
>
> On 2025/7/2 下午3:10, gaosong wrote:
>> 在 2025/7/2 上午10:03, Bibo Mao 写道:
>>>
>>>
>>> On 2025/6/27 上午11:01, Song Gao wrote:
>>>> LoongArchVirtMachinState adds avecintc features, and
>>>> it use to check whether virt machine support advance interrupt 
>>>> controller
>>>> and default set avecintc = ON_OFF_AUTO_ON.
>>>> LoongArchVirtMachineState adds misc_feature and misc_status for
>>>> misc fetures and status. and set default avec feture bit.
>>>>
>>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>>> ---
>>>>   hw/loongarch/virt.c         | 43 
>>>> +++++++++++++++++++++++++++++++++----
>>>>   include/hw/loongarch/virt.h | 15 +++++++++++++
>>>>   2 files changed, 54 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>>>> index 6a169d4824..426eaaef84 100644
>>>> --- a/hw/loongarch/virt.c
>>>> +++ b/hw/loongarch/virt.c
>>>> @@ -47,6 +47,28 @@
>>>>   #include "hw/virtio/virtio-iommu.h"
>>>>   #include "qemu/error-report.h"
>>>>   +static void virt_get_avecintc(Object *obj, Visitor *v, const 
>>>> char *name,
>>>> +                             void *opaque, Error **errp)
>>>> +{
>>>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>>>> +    OnOffAuto avecintc = lvms->avecintc;
>>>> +
>>>> +    visit_type_OnOffAuto(v, name, &avecintc, errp);
>>>> +
>>>> +}
>>>> +static void virt_set_avecintc(Object *obj, Visitor *v, const char 
>>>> *name,
>>>> +                              void *opaque, Error **errp)
>>>> +{
>>>> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
>>>> +
>>>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>>> +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>>>> +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>>>> +    }
>>>> +
>>>> +    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
>>> It is a little strange that firstly check variable and then set it, 
>>> I think that visit_type_OnOffAuto() should be before
>>>  +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>>  +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
>>>  +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
>>>  +    }
>>>
>>>> +}
>>>> +
>>>>   static void virt_get_veiointc(Object *obj, Visitor *v, const char 
>>>> *name,
>>>>                                 void *opaque, Error **errp)
>>>>   {
>>>> @@ -539,6 +561,10 @@ static MemTxResult virt_iocsr_misc_write(void 
>>>> *opaque, hwaddr addr,
>>>>               return MEMTX_OK;
>>>>           }
>>>>   +        if (val & BIT(IOCSRM_AVEC_EN)) {
>>>> +            lvms->misc_status |= BIT(IOCSRM_AVEC_EN);
>>>> +        }
>>>> +
>>>>           features = address_space_ldl(&lvms->as_iocsr,
>>>>                                        EXTIOI_VIRT_BASE + 
>>>> EXTIOI_VIRT_CONFIG,
>>>>                                        attrs, NULL);
>>>> @@ -574,8 +600,9 @@ static MemTxResult virt_iocsr_misc_read(void 
>>>> *opaque, hwaddr addr,
>>>>           break;
>>>>       case FEATURE_REG:
>>>>           ret = BIT(IOCSRF_MSI) | BIT(IOCSRF_EXTIOI) | 
>>>> BIT(IOCSRF_CSRIPI);
>>>> -        /*TODO: check bit IOCSRF_AVEC with virt_is_avec_enabled */
>>>> -        ret |= BIT(IOCSRF_AVEC);
>>>> +        if (virt_is_avecintc_enabled(lvms)) {
>>>> +            ret |= BIT(IOCSRF_AVEC);
>>>> +        }
>>>>           if (kvm_enabled()) {
>>>>               ret |= BIT(IOCSRF_VM);
>>>>           }
>>>> @@ -605,8 +632,10 @@ static MemTxResult virt_iocsr_misc_read(void 
>>>> *opaque, hwaddr addr,
>>>>           if (features & BIT(EXTIOI_ENABLE_INT_ENCODE)) {
>>>>               ret |= BIT_ULL(IOCSRM_EXTIOI_INT_ENCODE);
>>>>           }
>>>> -        /* enable avec default */
>>>> -        ret |= BIT_ULL(IOCSRM_AVEC_EN);
>>>> +        if (virt_is_avecintc_enabled(lvms) &&
>>>> +            (lvms->misc_status & BIT(IOCSRM_AVEC_EN))) {
>>>> +            ret |= BIT_ULL(IOCSRM_AVEC_EN);
>>>> +        }
>>>>           break;
>>>>       default:
>>>>           g_assert_not_reached();
>>>> @@ -850,6 +879,8 @@ static void virt_initfn(Object *obj)
>>>>       if (tcg_enabled()) {
>>>>           lvms->veiointc = ON_OFF_AUTO_OFF;
>>>>       }
>>>> +    lvms->misc_feature = BIT(IOCSRF_AVEC);
>>>> +    lvms->avecintc = ON_OFF_AUTO_ON;
>>>>       lvms->acpi = ON_OFF_AUTO_AUTO;
>>>>       lvms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>>>>       lvms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
>>>> @@ -1242,6 +1273,10 @@ static void virt_class_init(ObjectClass *oc, 
>>>> const void *data)
>>>>           NULL, NULL);
>>>>       object_class_property_set_description(oc, "v-eiointc",
>>>>                               "Enable Virt Extend I/O Interrupt 
>>>> Controller.");
>>>> +    object_class_property_add(oc, "avecintc", "OnOffAuto",
>>>> +        virt_get_avecintc, virt_set_avecintc, NULL, NULL);
>>>> +    object_class_property_set_description(oc, "avecintc",
>>>> +                            "Enable Advance Interrupt Controller.");
>>> It is only can be added in TCG mode, in kvm mode avec should not be 
>>> supported now.
>>>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>>>>       machine_class_allow_dynamic_sysbus_dev(mc, 
>>>> TYPE_UEFI_VARS_SYSBUS);
>>>>   #ifdef CONFIG_TPM
>>>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>>>> index cc6656619d..44504e5501 100644
>>>> --- a/include/hw/loongarch/virt.h
>>>> +++ b/include/hw/loongarch/virt.h
>>>> @@ -70,6 +70,7 @@ struct LoongArchVirtMachineState {
>>>>       Notifier     powerdown_notifier;
>>>>       OnOffAuto    acpi;
>>>>       OnOffAuto    veiointc;
>>>> +    OnOffAuto    avecintc;
>>>>       char         *oem_id;
>>>>       char         *oem_table_id;
>>>>       DeviceState  *acpi_ged;
>>>> @@ -85,6 +86,8 @@ struct LoongArchVirtMachineState {
>>>>       DeviceState *extioi;
>>>>       struct memmap_entry *memmap_table;
>>>>       unsigned int memmap_entries;
>>>> +    uint64_t misc_feature;
>>>> +    uint64_t misc_status;
>>>>   };
>>>>     #define TYPE_LOONGARCH_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
>>>> @@ -92,6 +95,18 @@ 
>>>> OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, 
>>>> LOONGARCH_VIRT_MACHINE)
>>>>   void virt_acpi_setup(LoongArchVirtMachineState *lvms);
>>>>   void virt_fdt_setup(LoongArchVirtMachineState *lvms);
>>>>   +static inline bool 
>>>> virt_is_avecintc_enabled(LoongArchVirtMachineState *lvms)
>>>> +{
>>>> +    if (!(lvms->misc_feature & BIT(IOCSRF_AVEC))) {
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
>>>> +        return false;
>>>> +    }
>>> Is it enough to only check variable misc_feature? checking avecintc 
>>> seems unnecessary duplicated.
>>>
>>   Someone may  disable avecintc with command  'avecintc = off'.
> +static void virt_set_avecintc(Object *obj, Visitor *v, const char *name,
> +                              void *opaque, Error **errp)
> +{
> +    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(obj);
> +
> +    if (lvms->avecintc == ON_OFF_AUTO_OFF) {
> +        lvms->misc_feature &= ~BIT(IOCSRF_AVEC);
> +        lvms->misc_status &= ~BIT(IOCSRM_AVEC_EN);
> +    }
> With avecintc = off option, misc_feature will be cleared with 
> IOCSRF_AVEC also.
>
you 're right , check  misc_feature is enough.

thanks.
Song Gao
> Regards
> Bibo Mao
>>
>> Thanks.
>> Song Gao.
>>> Regards
>>> Bibo Mao
>>>> +    return true;
>>>> +}
>>>> +
>>>>   static inline bool 
>>>> virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
>>>>   {
>>>>       if (lvms->veiointc == ON_OFF_AUTO_OFF) {
>>>>