Re: [PATCH v10 17/23] hw/intc/arm_gicv3: Add NMI handling CPU interface registers

[Peter Maydell]

On Sat, 30 Mar 2024 at 02:44, Jinjie Ruan via <qemu-arm@nongnu.org> wrote:
>
>
>
> On 2024/3/28 22:50, Peter Maydell wrote:
> > The NMI bit also exists only in the AP1R0 bit, not in every AP
> > register. So you can check it before the for() loop, something like this:
> >
> >     if (cs->gic->nmi_support) {
> >         /*
> >          * If an NMI is active this takes precedence over anything else
> >          * for priority purposes; the NMI bit is only in the AP1R0 bit.
> >          * We return here the effective priority of the NMI, which is
> >          * either 0x0 or 0x80. Callers will need to check NMI again for
> >          * purposes of either setting the RPR register bits or for
> >          * prioritization of NMI vs non-NMI.
> >          */
> >         prio = 0;
> >         if (cs->icc_apr[GICV3_G1][0] & ICC_AP1R_EL1_NMI) {
> >             return 0;
> >         }
> >         if (cs->icc_apr[GICV3_G1NS][0] & ICC_AP1R_EL1_NMI) {
> >             return (cs->gic->gicd_ctlr & GICD_CTLR_DS) ? 0 : 0x80;
> >         }
> >     }
> >
> > Then in icc_rpr_read() we can pretty much directly write the same
> > logic that the pseudocode uses to determine whether to set the RPR
> > NMI bits, after the point where we do the shifting of the prio for
> > the NS view:
> >
> >     if (cs->gic->nmi_support) {
> >         /* NMI info is reported in the high bits of RPR */
> >         if (arm_feature(env, ARM_FEATURE_EL3) && !arm_is_secure(env)) {
> >             if (cs->icc_apr[GICV3_G1NS][0] & ICC_AP1R_EL1_NMI) {
> >                 prio |= ICC_RPR_EL1_NMI;
>
> It seems ICC_RPR_EL1_NSNMI in pseudocode:
>
> // GICv3.3
> if HaveNMIExt() then
>     if HaveEL(EL3) && (IsNonSecure() || IsRealm()) then
>         pPriority<63> = ICC_AP1R_EL1NS<63>;
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> else
>     pPriority<63> = ICC_AP1R_EL1S<63>;
>     pPriority<62> = ICC_AP1R_EL1NS<63>;

I'm not sure what you have in mind here? For QEMU,
ICC_AP1R_EL1NS<63> is the ICC_AP1R_EL1_NMI bit in the
icc_apr[GICV3_G1NS][0] value, and ICC_RPR_EL1_NMI is bit 63,
so the C code seems to me to match up with the pseudocode line
that you highlight.

thanks
-- PMM