Re: [PATCH 2/4] target/riscv: Add a general status enum for extensions

[LIU Zhiwei <zhiwei_liu@linux.alibaba.com>]

On 2023/3/24 20:53, liweiwei wrote:
>
> On 2023/3/24 13:59, LIU Zhiwei wrote:
>> The pointer masking is the only extension that directly use status.
>> The vector or float extension uses the status in an indirect way.
>>
>> Replace the pointer masking extension special status fields with
>> the general status.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   target/riscv/cpu.c      |  2 +-
>>   target/riscv/cpu.h      |  9 +++++++++
>>   target/riscv/cpu_bits.h |  6 ------
>>   target/riscv/csr.c      | 14 +++++++-------
>>   4 files changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 1e97473af2..1135106b3e 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj)
>>           i++;
>>       }
>>       /* mmte is supposed to have pm.current hardwired to 1 */
>> -    env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
>> +    env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
>>   #endif
>>       env->xl = riscv_cpu_mxl(env);
>>       riscv_cpu_update_mask(env);
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 12fe8d8546..5049e21518 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -99,6 +99,15 @@ enum {
>>       TRANSLATE_G_STAGE_FAIL
>>   };
>>   +/* Extension Context Status */
>> +enum {
>> +    EXT_STATUS_DISABLED = 0,
>> +    EXT_STATUS_INITIAL,
>> +    EXT_STATUS_CLEAN,
>> +    EXT_STATUS_DIRTY,
>> +    EXT_STATUS_MASK,
>
> I think the right value for EXT_STATUS_MASK  should be 3 here.
Yes, it is.
>
> And it can replace the following  PM_XS_MASK.

Once I wanted to replace PM_XS_MASK with EXT_STATUS_MASK here.
But PM_XS_MASK has a ULL type which is needed for a 64-bit register.


So I want to drop the definition of EXT_STATUS_MASK from here. And define a
EXT_STATUS_MASK macro in cpu_bits.h. It will  replace the PM_XS_MASK.

Zhiwei

>
> Regards,
>
> Weiwei Li
>
>> +};
>> +
>>   #define MMU_USER_IDX 3
>>     #define MAX_RISCV_PMPS (16)
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index fca7ef0cef..5280bd41c2 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -736,12 +736,6 @@ typedef enum RISCVException {
>>   #define PM_INSN         0x00000004ULL
>>   #define PM_XS_MASK      0x00000003ULL
>>   -/* PointerMasking XS bits values */
>> -#define PM_EXT_DISABLE  0x00000000ULL
>> -#define PM_EXT_INITIAL  0x00000001ULL
>> -#define PM_EXT_CLEAN    0x00000002ULL
>> -#define PM_EXT_DIRTY    0x00000003ULL
>> -
>>   /* Execution enviornment configuration bits */
>>   #define MENVCFG_FIOM                       BIT(0)
>>   #define MENVCFG_CBIE                       (3UL << 4)
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index d522efc0b6..abea7b749e 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState 
>> *env, int csrno,
>>         /* hardwiring pm.instruction bit to 0, since it's not 
>> supported yet */
>>       wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
>> -    env->mmte = wpri_val | PM_EXT_DIRTY;
>> +    env->mmte = wpri_val | EXT_STATUS_DIRTY;
>>       riscv_cpu_update_mask(env);
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>> @@ -3593,7 +3593,7 @@ static RISCVException 
>> write_mpmmask(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
>>           env->cur_pmmask = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3621,7 +3621,7 @@ static RISCVException 
>> write_spmmask(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
>>           env->cur_pmmask = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3649,7 +3649,7 @@ static RISCVException 
>> write_upmmask(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
>>           env->cur_pmmask = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3673,7 +3673,7 @@ static RISCVException 
>> write_mpmbase(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
>>           env->cur_pmbase = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3701,7 +3701,7 @@ static RISCVException 
>> write_spmbase(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
>>           env->cur_pmbase = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;
>> @@ -3729,7 +3729,7 @@ static RISCVException 
>> write_upmbase(CPURISCVState *env, int csrno,
>>       if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
>>           env->cur_pmbase = val;
>>       }
>> -    env->mmte |= PM_EXT_DIRTY;
>> +    env->mmte |= EXT_STATUS_DIRTY;
>>         /* Set XS and SD bits, since PM CSRs are dirty */
>>       mstatus = env->mstatus | MSTATUS_XS;