Re: [RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Peter Maydell]

On Thu, 28 Mar 2024 at 08:57, Jinjie Ruan via <qemu-arm@nongnu.org> wrote:
>
>
>
> On 2024/3/20 0:47, Peter Maydell wrote:
> > On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> >>
> >> Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
> >> SCTLR_ELx.SPINTMASK bit.
> >>
> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> v3:
> >> - Add Reviewed-by.
> >> ---
> >>  target/arm/helper.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/target/arm/helper.c b/target/arm/helper.c
> >> index 4bc63bf7ca..81f4a8f194 100644
> >> --- a/target/arm/helper.c
> >> +++ b/target/arm/helper.c
> >> @@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState 
> >> *cs)
> >>          }
> >>      }
> >>
> >> +    if (cpu_isar_feature(aa64_nmi, cpu) &&
> >> +        (env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {
> >
> > This shouldn't be checking the value of SCTLR_NMI here:
> > the new PSTATE.ALLINT is set to !SPINTMASK even if NMI == 0.
> > (The SPINTMASK bit description is a bit confusing, but
> > the correct behaviour is clear in the AArch64.TakeException()
> > pseudocode.)
>
> It seems unreasonable to remove the SCTLR_NMI check, because if the
> hardware supports FEAT_NMI but the kernel do not enable it, the ALLINT
> bit in pstate will also set or clear when an exception is caught, which
> seems unreasonable.

Whether we personally think it is "unreasonable" or not does not
matter here. The architecture says that we must not check SCTLR_NMI,
and therefore we must not check SCTLR_NMI, or we will be implementing
the wrong behaviour.

thanks
-- PMM