On 2024/4/5 21:48, Peter Maydell wrote:
> On Wed, 3 Apr 2024 at 11:18, Jinjie Ruan <ruanjin...@huawei.com> wrote:
>>
>> A PE that implements FEAT_NMI and FEAT_GICv3 also implements
>> FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
>> FEAT_GICv3_NMI
>
> This is true but not really relevant here -- FEAT_GICv3_NMI
> is not "NMI support in the GIC", it's "does the CPU interface
> support NMIs". (And so I'm wondering if the code in arm_gicv3_cpuif.c
> should be checking cpu_isar_feature(aa64_nmi, cpu) rather than
> cs->gic->nmi_support; but I need to think through the consequences
> of that first.)
In "1.1.2 GIC architecture extensions", it said:
FEAT_GICv3_NMI introduces GIC support for non-maskable interrupts (NMIs).
So in my opinion, it is relevant here.
>
> The justification for "enable NMIs in the GIC device if the
> CPU has FEAT_NMI" is that (a) it's only OK to have a GIC with
> NMI support if the CPU also has NMI support and (b) if we
> can turn on NMI support in the GIC we should, so that we can
> provide the feature to the guest.
>
>> So included support FEAT_GICv3_NMI feature as part of virt platform
>> GIC initialization if FEAT_NMI and FEAT_GICv3 supported.
>>
>> Signed-off-by: Jinjie Ruan <ruanjin...@huawei.com>
>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> v4:
>> - Add Reviewed-by.
>> v3:
>> - Adjust to be the last after add FEAT_NMI to max.
>> - Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
>> ---
>> hw/arm/virt.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index ef2e6c2c4d..63d9f5b553 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -729,6 +729,19 @@ static void create_v2m(VirtMachineState *vms)
>> vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
>> }
>>
>> +/*
>> + * A PE that implements FEAT_NMI and FEAT_GICv3 also implements
>> + * FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
>> + * FEAT_GICv3_NMI.
>> + */
>> +static bool gicv3_nmi_present(VirtMachineState *vms)
>> +{
>> + ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
>> +
>> + return cpu_isar_feature(aa64_nmi, cpu) &&
>> + (vms->gic_version != VIRT_GIC_VERSION_2);
>
> I think we should add tcg_enabled() to this condition:
> neither KVM nor hvf support FEAT_NMI yet. Defaulting QEMU to
> not trying to enable NMI in the GIC device is the safe
> option. As and when those accelerators get NMI support, we
> can add the handling to QEMU and update this code in the virt board.
>
> thanks
> -- PMM
