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

[Jinjie Ruan]

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.

> 
>> +        if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
>> +            new_mode |= PSTATE_ALLINT;
>> +        } else {
>> +            new_mode &= ~PSTATE_ALLINT;
>> +        }
>> +    }
>> +
>>      pstate_write(env, PSTATE_DAIF | new_mode);
>>      env->aarch64 = true;
>>      aarch64_restore_sp(env, new_el);
>> --
>> 2.34.1
>>
> 
> thanks
> -- PMM