On 2023/3/23 06:20, Daniel Henrique Barboza wrote:
> write_misa() is able to use the same validation workflow
> riscv_cpu_realize() uses. But it's still not capable of updating
> cpu->cfg misa props yet.
>
> We have no way of blocking future (and current) code from checking
> env->misa_ext (via riscv_has_ext()) or reading cpu->cfg directly, so our
> best alternative is to keep everything in sync.
>
> riscv_cpu_commit_cpu_cfg() now receives an extra 'misa_ext' parameter.
> If this val is different from the existing env->misa_ext, update
> env->misa and cpu->cfg with the new value. riscv_cpu_realize() will
> ignore this code since env->misa_ext isn't touched during validation,
> but write_misa() will use it to keep cpu->cfg in sync with the new
> env->misa_ext value.
>
> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.c | 16 ++++++++++++++--
>   target/riscv/cpu.h |  2 +-
>   target/riscv/csr.c |  3 +--
>   3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 41b17ba0c3..88806d1050 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1204,8 +1204,20 @@ void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
>       }
>   }
>   
> -void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
> +void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext)
>   {
> +    CPURISCVState *env = &cpu->env;
> +
> +    /*
> +     * write_misa() needs to update cpu->cfg with the new
> +     * MISA bits. This is a no-op for the riscv_cpu_realize()
> +     * path.
> +     */
> +    if (env->misa_ext != misa_ext) {
> +        env->misa_ext = misa_ext;
> +        riscv_set_cpucfg_with_misa(&cpu->cfg, misa_ext);
> +    }
> +
>       if (cpu->cfg.ext_zk) {
>           cpu->cfg.ext_zkn = true;
>           cpu->cfg.ext_zkr = true;

These zk* related assignment and riscv_cpu_disable_priv_spec_isa_exts() 
can be moved to other places.

They needn't be done for write_misa.

Regards,

Weiwei Li

> @@ -1374,7 +1386,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> -    riscv_cpu_commit_cpu_cfg(cpu);
> +    riscv_cpu_commit_cpu_cfg(cpu, env->misa_ext);
>   
>   #ifndef CONFIG_USER_ONLY
>       if (cpu->cfg.ext_sstc) {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index ca2ba6a647..befc3b8fff 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -597,7 +597,7 @@ void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
>                                    Error **errp);
>   void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
>                                      Error **errp);
> -void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu);
> +void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext);
>   
>   #define cpu_list riscv_cpu_list
>   #define cpu_mmu_index riscv_cpu_mmu_index
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 8d5e8f9ad1..839862f1a8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1396,7 +1396,7 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>           return RISCV_EXCP_NONE;
>       }
>   
> -    riscv_cpu_commit_cpu_cfg(cpu);
> +    riscv_cpu_commit_cpu_cfg(cpu, val);
>   
>       if (!(val & RVF)) {
>           env->mstatus &= ~MSTATUS_FS;
> @@ -1404,7 +1404,6 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>   
>       /* flush translation cache */
>       tb_flush(env_cpu(env));
> -    env->misa_ext = val;
>       env->xl = riscv_cpu_mxl(env);
>       return RISCV_EXCP_NONE;
>   }