Re: [RFC PATCH v5 12/22] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()

[Richard Henderson]

On 2/29/24 17:42, Jinjie Ruan wrote:
> On 2024/3/1 7:09, Richard Henderson wrote:
> > On 2/29/24 03:10, Jinjie Ruan via wrote:
> > > According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
> > > with superpriority is always IRQ, never FIQ, so the NMI exception trap
> > > entry
> > > behave like IRQ. However, VNMI can be IRQ or FIQ, FIQ can only come from
> > > hcrx_el2.HCRX_VFNMI bit, IRQ can be raised from the GIC or come from the
> > > hcrx_el2.HCRX_VINMI bit.
> > >
> > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > > ---
> > > v4:
> > > - Also handle VNMI in arm_cpu_do_interrupt_aarch64().
> > > v3:
> > > - Remove the FIQ NMI handle.
> > > ---
> > >    target/arm/helper.c | 9 +++++++++
> > >    1 file changed, 9 insertions(+)
> > >
> > > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > > index b796dbdf21..bd34b3506a 100644
> > > --- a/target/arm/helper.c
> > > +++ b/target/arm/helper.c
> > > @@ -11459,12 +11459,21 @@ static void
> > > arm_cpu_do_interrupt_aarch64(CPUState *cs)
> > >            break;
> > >        case EXCP_IRQ:
> > >        case EXCP_VIRQ:
> > > +    case EXCP_NMI:
> > >            addr += 0x80;
> > >            break;
> > >        case EXCP_FIQ:
> > >        case EXCP_VFIQ:
> > >            addr += 0x100;
> > >            break;
> > > +    case EXCP_VNMI:
> > > +        if (env->irq_line_state & CPU_INTERRUPT_VNMI ||
> > > +            env->cp15.hcrx_el2 & HCRX_VINMI) {
> > > +            addr += 0x80;
> > > +        } else if (env->cp15.hcrx_el2 & HCRX_VFNMI) {
> > > +            addr += 0x100;
> > > +        }
> > > +        break;
> >
> > By not combining VFNMI with CPU_INTERRUPT_VNMI, you don't need this
> > complication.
> > Just
> >
> >       case EXCP_IRQ:
> >       case EXCP_VIRQ:
> > +    case EXCP_NMI:
>
> Not understand it. both VIRQ and VFIQ will set CPU_INTERRUPT_VNMI and
> cause EXCP_VNMI if they have Superpriority, the distinction jump here is
> necessary.

In my comment against patch 5, that's exactly what I said *not* to do.


r~