Re: [PATCH 2/7] target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part of Zjpm v1.0

[Richard Henderson]

On 11/7/24 22:01, baturo.alexey@gmail.com wrote:
> @@ -2424,6 +2433,12 @@ static RISCVException write_senvcfg(CPURISCVState *env, 
> int csrno,
>                                       target_ulong val)
>   {
>       uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | 
> SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v1.0 */
> +    if (env_archcpu(env)->cfg.ext_ssnpm &&
> +        riscv_cpu_mxl(env) == MXL_RV64 &&
> +        (get_field(val, SENVCFG_PMM) != PMM_FIELD_RESERVED)) {
> +        mask |= SENVCFG_PMM;
> +    }
>       RISCVException ret;
>   
>       ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);

Do not insert statements into the declarations at the start of the block.
Drop the unnecessary () around != (multiple instances).

> @@ -3525,6 +3546,15 @@ static RISCVException read_hstatus(CPURISCVState *env, 
> int csrno,
>   static RISCVException write_hstatus(CPURISCVState *env, int csrno,
>                                       target_ulong val)
>   {
> +    uint64_t mask = (target_ulong)-1;
> +    /* Update PMM field only if the value is valid according to Zjpm v1.0 */
> +    if (env_archcpu(env)->cfg.ext_ssnpm &&
> +        (riscv_cpu_mxl(env) == MXL_RV64) &&
> +        (get_field(val, HSTATUS_HUPMM) == PMM_FIELD_RESERVED)) {
> +        mask &= ~HSTATUS_HUPMM;
> +    }

Surely you'd not install HUPMM if ext_ssnpm is false?
Is this missing other extension checks?


> +    env->hstatus = (env->hstatus & ~mask) | (val & mask);
> +
>       env->hstatus = val;

Failure to remove previous assignment.


r~