Re: [PATCH v12 18/23] hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()

[Peter Maydell]

On Wed, 3 Apr 2024 at 11:18, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> Implement icv_nmiar1_read() for icc_nmiar1_read(), so add definition for
> ICH_LR_EL2.NMI and ICH_AP1R_EL2.NMI bit.
>
> If FEAT_GICv3_NMI is supported, ich_ap_write() should consider 
> ICV_AP1R_EL1.NMI
> bit. In icv_activate_irq() and icv_eoir_write(), the ICV_AP1R_EL1.NMI bit
> should be set or clear according to the Non-maskable property. And the RPR
> priority should also update the NMI bit according to the APR priority NMI bit.
>
> By the way, add gicv3_icv_nmiar1_read trace event.
>
> If the hpp irq is a NMI, the icv iar read should return 1022 and trap for
> NMI again
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


> @@ -301,10 +310,11 @@ static bool icv_hppi_can_preempt(GICv3CPUState *cs, 
> uint64_t lr)
>       */
>
>      prio = ich_lr_prio(lr);
> +    is_nmi = lr & ICH_LR_EL2_NMI;

If you want to be able to skip the cs->gic->nmi_support check here
then you need to enforce in ich_lr_write() that the guest cannot
write a 1 to the ICH_LR_EL2_NMI bit when the GIC doesn't implement NMIs.

@@ -2833,6 +2833,10 @@ static void ich_lr_write(CPUARMState *env,
const ARMCPRegInfo *ri,
                           8 - cs->vpribits, 0);
     }

+    if (!cs->gic->nmi_support) {
+        value &= ~ICH_LR_EL2_NMI;
+    }
+
     cs->ich_lr_el2[regno] = value;
     gicv3_cpuif_virt_update(cs);
 }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM