Re: [PATCH v12 23/23] hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC

[Peter Maydell]

On Wed, 3 Apr 2024 at 11:18, Jinjie Ruan <ruanjinjie@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.)

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 <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@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