Re: [PATCH 1/3] target/riscv: Set the CPU resetvec directly

[Wilfred Mallawa]

On Wed, 2022-09-14 at 12:11 +0200, Alistair Francis via wrote:
> Instead of using our properties to set a config value which then
> might
> be used to set the resetvec (depending on your timing), let's instead
> just set the resetvec directly in the env struct.
> 
> This allows us to set the reset vec from the command line with:
>     -global
> driver=riscv.hart_array,property=resetvec,value=0x20000400
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h     |  3 +--
>  target/riscv/cpu.c     | 13 +++----------
>  target/riscv/machine.c |  6 +++---
>  3 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 06751e1e3e..22344a620b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -190,7 +190,7 @@ struct CPUArchState {
>      /* This contains QEMU specific information about the virt state.
> */
>      target_ulong virt;
>      target_ulong geilen;
> -    target_ulong resetvec;
> +    uint64_t resetvec;
>  
>      target_ulong mhartid;
>      /*
> @@ -474,7 +474,6 @@ struct RISCVCPUConfig {
>      bool pmp;
>      bool epmp;
>      bool debug;
> -    uint64_t resetvec;
>  
>      bool short_isa_string;
>  };
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index aee14a239a..b29c88b9f0 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -228,13 +228,6 @@ static void set_vext_version(CPURISCVState *env,
> int vext_ver)
>      env->vext_ver = vext_ver;
>  }
>  
> -static void set_resetvec(CPURISCVState *env, target_ulong resetvec)
> -{
> -#ifndef CONFIG_USER_ONLY
> -    env->resetvec = resetvec;
> -#endif
> -}
> -
>  static void riscv_any_cpu_init(Object *obj)
>  {
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -336,7 +329,6 @@ static void rv32_imafcu_nommu_cpu_init(Object
> *obj)
>  
>      set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> -    set_resetvec(env, DEFAULT_RSTVEC);
>      cpu->cfg.mmu = false;
>  }
>  #endif
> @@ -676,7 +668,6 @@ static void riscv_cpu_realize(DeviceState *dev,
> Error **errp)
>          riscv_set_feature(env, RISCV_FEATURE_DEBUG);
>      }
>  
> -    set_resetvec(env, cpu->cfg.resetvec);
>  
>  #ifndef CONFIG_USER_ONLY
>      if (cpu->cfg.ext_sstc) {
> @@ -1079,7 +1070,9 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid,
> RISCV_CPU_MARCHID),
>      DEFINE_PROP_UINT64("mimpid", RISCVCPU, cfg.mimpid,
> RISCV_CPU_MIMPID),
>  
> -    DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec,
> DEFAULT_RSTVEC),
> +#ifndef CONFIG_USER_ONLY
> +    DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec,
> DEFAULT_RSTVEC),
> +#endif
>  
>      DEFINE_PROP_BOOL("short-isa-string", RISCVCPU,
> cfg.short_isa_string, false),
>  
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 41098f6ad0..c4e6b3bba4 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -308,8 +308,8 @@ static const VMStateDescription
> vmstate_pmu_ctr_state = {
>  
>  const VMStateDescription vmstate_riscv_cpu = {
>      .name = "cpu",
> -    .version_id = 4,
> -    .minimum_version_id = 4,
> +    .version_id = 5,
> +    .minimum_version_id = 5,
>      .post_load = riscv_cpu_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
> @@ -331,7 +331,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          VMSTATE_UINT32(env.features, RISCVCPU),
>          VMSTATE_UINTTL(env.priv, RISCVCPU),
>          VMSTATE_UINTTL(env.virt, RISCVCPU),
> -        VMSTATE_UINTTL(env.resetvec, RISCVCPU),
> +        VMSTATE_UINT64(env.resetvec, RISCVCPU),
>          VMSTATE_UINTTL(env.mhartid, RISCVCPU),
>          VMSTATE_UINT64(env.mstatus, RISCVCPU),
>          VMSTATE_UINT64(env.mip, RISCVCPU),

Reviewed by: Wilfred Mallawa <wilfred.mallawa@wdc.com>