On 2023/3/23 6:19, Daniel Henrique Barboza wrote:
> We have 4 config settings being done in riscv_cpu_init(): ext_ifencei,
> ext_icsr, mmu and pmp. This is also the constructor of the "riscv-cpu"
> device, which happens to be the parent device of every RISC-V cpu.
>
> The result is that these 4 configs are being set every time, and every
> other CPU should always account for them. CPUs such as sifive_e need to
> disable settings that aren't enabled simply because the parent class
> happens to be enabling it.
>
> Moving all configurations from the parent class to each CPU will
> centralize the config of each CPU into its own init(), which is clearer
> than having to account to whatever happens to be set in the parent
> device. These settings are also being set in register_cpu_props() when
> no 'misa_ext' is set, so for these CPUs we don't need changes. Named
> CPUs will receive all cfgs that the parent were setting into their
> init().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu.c | 60 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index fef55d7d79..c7b6e7b84b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -325,7 +325,8 @@ static void set_satp_mode_default_map(RISCVCPU *cpu)
>
> static void riscv_any_cpu_init(Object *obj)
> {
> - CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + CPURISCVState *env = &cpu->env;
> #if defined(TARGET_RISCV32)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> #elif defined(TARGET_RISCV64)
> @@ -340,6 +341,12 @@ static void riscv_any_cpu_init(Object *obj)
>
> env->priv_ver = PRIV_VERSION_LATEST;
> register_cpu_props(obj);
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.mmu = true;
> + cpu->cfg.pmp = true;
> }
>
> #if defined(TARGET_RISCV64)
> @@ -358,13 +365,20 @@ static void rv64_base_cpu_init(Object *obj)
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> - CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + CPURISCVState *env = &cpu->env;
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> register_cpu_props(obj);
> env->priv_ver = PRIV_VERSION_1_10_0;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.mmu = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -375,10 +389,14 @@ static void rv64_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> register_cpu_props(obj);
> env->priv_ver = PRIV_VERSION_1_10_0;
> - cpu->cfg.mmu = false;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv64_thead_c906_cpu_init(Object *obj)
> @@ -411,6 +429,10 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -447,7 +469,8 @@ static void rv32_base_cpu_init(Object *obj)
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> {
> - CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + CPURISCVState *env = &cpu->env;
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>
> register_cpu_props(obj);
> @@ -455,6 +478,12 @@ static void rv32_sifive_u_cpu_init(Object *obj)
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.mmu = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv32_sifive_e_cpu_init(Object *obj)
> @@ -465,10 +494,14 @@ static void rv32_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> register_cpu_props(obj);
> env->priv_ver = PRIV_VERSION_1_10_0;
> - cpu->cfg.mmu = false;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv32_ibex_cpu_init(Object *obj)
> @@ -479,11 +512,15 @@ static void rv32_ibex_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> register_cpu_props(obj);
> env->priv_ver = PRIV_VERSION_1_11_0;
> - cpu->cfg.mmu = false;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> #endif
> cpu->cfg.epmp = true;
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.pmp = true;
> }
>
> static void rv32_imafcu_nommu_cpu_init(Object *obj)
> @@ -494,10 +531,14 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> register_cpu_props(obj);
> env->priv_ver = PRIV_VERSION_1_10_0;
> - cpu->cfg.mmu = false;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> #endif
> +
> + /* inherited from parent obj via riscv_cpu_init() */
> + cpu->cfg.ext_ifencei = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.pmp = true;
> }
> #endif
>
> @@ -1384,11 +1425,6 @@ static void riscv_cpu_init(Object *obj)
> {
> RISCVCPU *cpu = RISCV_CPU(obj);
>
> - cpu->cfg.ext_ifencei = true;
> - cpu->cfg.ext_icsr = true;
> - cpu->cfg.mmu = true;
> - cpu->cfg.pmp = true;
> -
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> cpu_set_cpustate_pointers(cpu);
>
> #ifndef CONFIG_USER_ONLY