Re: [RFC PATCH v2 03/22] target/arm: Add PSTATE.ALLINT

[Jinjie Ruan]

On 2024/2/22 2:50, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> The ALLINT bit in PSTATE is used to mask all IRQ or FIQ interrupts.
>>
>> Place this in its own field within ENV, as that will
>> make it easier to reset from within TCG generated code.
>>
>> With the change to pstate_read/write, exception entry
>> and return are automatically handled.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>   target/arm/cpu.c | 3 +++
>>   target/arm/cpu.h | 9 +++++++--
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 5fa86bc8d5..5e5978c302 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1104,6 +1104,9 @@ static void aarch64_cpu_dump_state(CPUState *cs,
>> FILE *f, int flags)
>>       if (cpu_isar_feature(aa64_bti, cpu)) {
>>           qemu_fprintf(f, "  BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
>>       }
>> +    if (cpu_isar_feature(aa64_nmi, cpu)) {
>> +        qemu_fprintf(f, "  ALLINT=%d", (psr & PSTATE_ALLINT) >> 13);
>> +    }
> 
> This is one bit -- !!(psr & ALLINT) is better
> 
> We don't individually print DAIF either; why is this bit more special?

That makes sense, it should be removed.

> 
>> @@ -224,6 +224,7 @@ typedef struct CPUArchState {
>>        *    semantics as for AArch32, as described in the comments on
>> each field)
>>        *  nRW (also known as M[4]) is kept, inverted, in env->aarch64
>>        *  DAIF (exception masks) are kept in env->daif
>> +     *  ALLINT (all IRQ or FIQ interrupts masks) are kept in env->allint
>>        *  BTYPE is kept in env->btype
>>        *  SM and ZA are kept in env->svcr
>>        *  all other bits are stored in their correct places in
>> env->pstate
>> @@ -261,6 +262,7 @@ typedef struct CPUArchState {
>>       uint32_t btype;  /* BTI branch type.  spsr[11:10].  */
>>       uint64_t daif; /* exception masks, in the bits they are in
>> PSTATE */
>>       uint64_t svcr; /* PSTATE.{SM,ZA} in the bits they are in SVCR */
>> +    uint64_t allint; /* All IRQ or FIQ interrupt mask, in the bit in
>> PSTATE */
> 
> Why is this split out from env->pstate?
> 
> The allint bit matches the documentation for SPSR_EL1, which is how
> env->pstate is documented.  The other exclusions have some performance
> imperative which I don't see for allint.

It seems to me that allint is a bit like daif, so it is reasonable to
maintain it separately in ENV as what daif do it.

> 
> 
> r~