Re: [RFC PATCH v9 00/23] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI

[Peter Maydell]

On Thu, 21 Mar 2024 at 13:10, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> This patch set implements FEAT_NMI and FEAT_GICv3_NMI for armv8. These
> introduce support for a new category of interrupts in the architecture
> which we can use to provide NMI like functionality.
>
> There are two modes for using this FEAT_NMI. When PSTATE.ALLINT or
> PSTATE.SP & SCTLR_ELx.SCTLR_SPINTMASK is set, any entry to ELx causes all
> interrupts including those with superpriority to be masked on entry to ELn
> until the mask is explicitly removed by software or hardware. PSTATE.ALLINT
> can be managed by software using the new register control ALLINT.ALLINT.
> Independent controls are provided for this feature at each EL, usage at EL1
> should not disrupt EL2 or EL3.
>
> I have tested it with the following linux patches which try to support
> FEAT_NMI in linux kernel:
>
>         
> https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/T/#mb4ba4a2c045bf72c10c2202c1dd1b82d3240dc88
>
> In the test, SGI, PPI and SPI interrupts can all be set to have super priority
> to be converted to a hardware NMI interrupt. The SGI is tested with kernel
> IPI as NMI framework, softlockup, hardlockup and kgdb test cases, and the PPI
> interrupt is tested with "perf top" command with hardware NMI enabled, and
> the SPI interrupt is tested with a custom test module, in which NMI interrupts
> can be received and sent normally.

It looks like your changes to the GIC have missed the handling
of NMIs in the active priority registers and the running
priority registers (both ICC and ICV versions). The way this
works is that the ICH_AP1R0 and ICC_AP1R0 registers get an
extra bit for NMI status in bit 63 (luckily we had the foresight
to make these struct fields be uint64_t). When we activate
an NMI IRQ, instead of setting the ICC_APR bit according to
its priority, we set the NMI bit instead. Similarly, on
deactivate we clear the NMI bit, not the priority-related bit.
The ICC_RPR register also has new bits in bit 63 and 62 for
whether there's an active NMI. On read of the RPR we figure
out the values for these bits based on the NMI bits in
ICC_AP1R_EL1S.NMI and ICC_AP1R_EL1NS.NMI (you might find the
pseudocode functions for ICC_RPR_EL1 in chapter 13 helpful to
look at here). The icc_highest_active_prio() likely also needs
changes to handle NMI. Similarly for all the ICV versions of
these registers and functions.

thanks
-- PMM