Re: [PATCH 02/10] loongarch: add virt feature avecintc support

[Bibo Mao <maobibo@loongson.cn>]

On 2025/6/13 下午3:54, gaosong wrote:
> 在 2025/6/11 下午2:46, Bibo Mao 写道:
>>
>>
>> On 2025/6/9 下午6:48, Song Gao wrote:
>>> LoongArchVirtMachinState add  avecintc features, and
>>> it use to check whether virt machine support advance interrupt 
>>> controller
>>> and default is on.
>>>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>   hw/loongarch/virt.c         | 31 +++++++++++++++++++++++++++----
>>>   include/hw/loongarch/virt.h |  9 +++++++++
>>>   2 files changed, 36 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>>> index 90d4643721..35643a4e0b 100644
>>> --- a/hw/loongarch/virt.c
>>> +++ b/hw/loongarch/virt.c
>>> @@ -47,6 +47,23 @@
>>>   #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);
>>> +
>>> +    visit_type_OnOffAuto(v, name, &lvms->avecintc, errp);
>>> +}
>>> +
>>>   static void virt_get_veiointc(Object *obj, Visitor *v, const char 
>>> *name,
>>>                                 void *opaque, Error **errp)
>>>   {
>>> @@ -548,8 +565,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);
>>>           }
>>> @@ -575,8 +593,9 @@ 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)) {
>>> +            ret |= BIT_ULL(IOCSRM_AVEC_EN);
>>> +        }
>> Is it really that reading of MISC_FUNC_REG will return 
>> BIT_ULL(IOCSRM_AVEC_EN) if there is avec?
>>
> no, it is IOCSRM_AVEC_EN after drivier  enable avecintc.
> static inline void avecintc_enable(void)
> {
>          u64 value;
> 
>          value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
>          value |= IOCSR_MISC_FUNC_AVEC_EN;
>          iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
> }
> 
> my thought  is that qemu has enabled avec by default,
> so I just add a  variable  OnOffAuto avecintc   on 
> LoongArchVirtMachineState,
>> Where is the write operation with register MISC_FUNC_REG?
>>
> no,  I will add it on v2?
> 
> how about add features and status on  LoongArchVirtMachineState for the 
> macine misc fetureas and status?
> like LoongArchExtIOICommonState
That sounds good to me.

Regards
Bibo Mao
> 
> Thanks.
> Song Gao
>> Regard
>> Bibo Mao
>>>           break;
>>>       default:
>>>           g_assert_not_reached();
>>> @@ -1212,6 +1231,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 2b7d19953f..3a81f048e8 100644
>>> --- a/include/hw/loongarch/virt.h
>>> +++ b/include/hw/loongarch/virt.h
>>> @@ -50,6 +50,7 @@ struct LoongArchVirtMachineState {
>>>       Notifier     powerdown_notifier;
>>>       OnOffAuto    acpi;
>>>       OnOffAuto    veiointc;
>>> +    OnOffAuto    avecintc;
>>>       char         *oem_id;
>>>       char         *oem_table_id;
>>>       DeviceState  *acpi_ged;
>>> @@ -70,6 +71,14 @@ 
>>> 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->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) {
>>>