From: Alistair Francis <alistair23@gmail.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
alistair.francis@wdc.com, bmeng@tinylab.org,
liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com,
palmer@rivosinc.com
Subject: Re: [PATCH v6 8/9] target/riscv/cpu.c: remove cfg setup from riscv_cpu_init()
Date: Thu, 6 Apr 2023 12:12:11 +1000 [thread overview]
Message-ID: <CAKmqyKMGoeqHhpf0ScT=Enq+c7rm=Suw3qVKdvu6cro_kkS9rw@mail.gmail.com> (raw)
In-Reply-To: <20230329200856.658733-9-dbarboza@ventanamicro.com>
On Thu, Mar 30, 2023 at 6:11 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> 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>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 59 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 331272c8a0..4aa119b9bc 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)
> @@ -339,6 +340,12 @@ static void riscv_any_cpu_init(Object *obj)
> #endif
>
> env->priv_ver = PRIV_VERSION_LATEST;
> +
> + /* 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)
> @@ -357,12 +364,19 @@ 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);
> 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)
> @@ -372,10 +386,14 @@ static void rv64_sifive_e_cpu_init(Object *obj)
>
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> 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)
> @@ -403,6 +421,9 @@ 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.pmp = true;
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -439,12 +460,19 @@ 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);
> env->priv_ver = PRIV_VERSION_1_10_0;
> #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)
> @@ -454,10 +482,14 @@ static void rv32_sifive_e_cpu_init(Object *obj)
>
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> 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)
> @@ -467,11 +499,15 @@ static void rv32_ibex_cpu_init(Object *obj)
>
> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> 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)
> @@ -481,10 +517,14 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
>
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> 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
>
> @@ -1343,11 +1383,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;
> -
> cpu_set_cpustate_pointers(cpu);
>
> #ifndef CONFIG_USER_ONLY
> --
> 2.39.2
>
>
next prev parent reply other threads:[~2023-04-06 2:13 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-29 20:08 [PATCH v6 0/9] target/riscv: rework CPU extensions validation Daniel Henrique Barboza
2023-03-29 20:08 ` [PATCH v6 1/9] target/riscv/cpu.c: add riscv_cpu_validate_v() Daniel Henrique Barboza
2023-04-06 1:59 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 2/9] target/riscv/cpu.c: remove set_vext_version() Daniel Henrique Barboza
2023-04-06 2:00 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 3/9] target/riscv/cpu.c: remove set_priv_version() Daniel Henrique Barboza
2023-04-06 2:05 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 4/9] target/riscv: add PRIV_VERSION_LATEST Daniel Henrique Barboza
2023-04-06 2:06 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers Daniel Henrique Barboza
2023-04-06 2:08 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 6/9] target/riscv/cpu.c: add riscv_cpu_validate_misa_mxl() Daniel Henrique Barboza
2023-04-06 2:09 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 7/9] target/riscv/cpu.c: validate extensions before riscv_timer_init() Daniel Henrique Barboza
2023-04-06 2:10 ` Alistair Francis
2023-03-29 20:08 ` [PATCH v6 8/9] target/riscv/cpu.c: remove cfg setup from riscv_cpu_init() Daniel Henrique Barboza
2023-04-06 2:12 ` Alistair Francis [this message]
2023-03-29 20:08 ` [PATCH v6 9/9] target/riscv: rework write_misa() Daniel Henrique Barboza
2023-03-29 20:12 ` [PATCH v6 0/9] target/riscv: rework CPU extensions validation Daniel Henrique Barboza
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKmqyKMGoeqHhpf0ScT=Enq+c7rm=Suw3qVKdvu6cro_kkS9rw@mail.gmail.com' \
--to=alistair23@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=dbarboza@ventanamicro.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).