[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] target/riscv: Don't expose the CPU properties on names CP
|
From: |
Bin Meng |
|
Subject: |
Re: [PATCH v2] target/riscv: Don't expose the CPU properties on names CPUs |
|
Date: |
Wed, 8 Jun 2022 07:27:05 +0800 |
On Wed, Jun 1, 2022 at 9:37 AM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> There are currently two types of RISC-V CPUs:
> - Generic CPUs (base or any) that allow complete custimisation
> - "Named" CPUs that match existing hardware
>
> Users can use the base CPUs to custimise the extensions that they want, for
> example -cpu rv64,v=true.
>
> We originally exposed these as part of the named CPUs as well, but that was
> by accident.
>
> Exposing the CPU properties to named CPUs means that we accidently
> enable extensinos that don't exist on the CPUs by default. For example
typo: extensions
> the SiFive E CPU currently support the zba extension, which is a bug.
>
> This patch instead only expose the CPU extensions to the generic CPUs.
exposes
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> target/riscv/cpu.c | 57 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index a91253d4bd..c0c0f7e71f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -118,6 +118,8 @@ static const char * const riscv_intr_names[] = {
> "reserved"
> };
>
> +static void register_cpu_props(DeviceState *dev);
nits: please move the whole static function implementation here to
avoid the forward declaration
> +
> const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
> {
> if (async) {
> @@ -161,6 +163,7 @@ static void riscv_any_cpu_init(Object *obj)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> #endif
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + register_cpu_props(DEVICE(obj));
> }
>
> #if defined(TARGET_RISCV64)
> @@ -169,6 +172,7 @@ static void rv64_base_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> /* We set this in the realise function */
> set_misa(env, MXL_RV64, 0);
> + register_cpu_props(DEVICE(obj));
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -181,9 +185,11 @@ static void rv64_sifive_u_cpu_init(Object *obj)
> static void rv64_sifive_e_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> - qdev_prop_set_bit(DEVICE(obj), "mmu", false);
> + cpu->cfg.mmu = false;
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -197,6 +203,7 @@ static void rv128_base_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> /* We set this in the realise function */
> set_misa(env, MXL_RV128, 0);
> + register_cpu_props(DEVICE(obj));
> }
> #else
> static void rv32_base_cpu_init(Object *obj)
> @@ -204,6 +211,7 @@ static void rv32_base_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> /* We set this in the realise function */
> set_misa(env, MXL_RV32, 0);
> + register_cpu_props(DEVICE(obj));
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -216,27 +224,33 @@ static void rv32_sifive_u_cpu_init(Object *obj)
> static void rv32_sifive_e_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> - qdev_prop_set_bit(DEVICE(obj), "mmu", false);
> + cpu->cfg.mmu = false;
> }
>
> static void rv32_ibex_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> - qdev_prop_set_bit(DEVICE(obj), "mmu", false);
> - qdev_prop_set_bit(DEVICE(obj), "x-epmp", true);
> + cpu->cfg.mmu = false;
> + cpu->cfg.epmp = true;
> }
>
> static void rv32_imafcu_nommu_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(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);
> - qdev_prop_set_bit(DEVICE(obj), "mmu", false);
> + cpu->cfg.mmu = false;
> }
> #endif
>
> @@ -249,6 +263,7 @@ static void riscv_host_cpu_init(Object *obj)
> #elif defined(TARGET_RISCV64)
> set_misa(env, MXL_RV64, 0);
> #endif
> + register_cpu_props(DEVICE(obj));
> }
> #endif
>
> @@ -831,6 +846,12 @@ static void riscv_cpu_init(Object *obj)
> {
> RISCVCPU *cpu = RISCV_CPU(obj);
>
> + cpu->cfg.ext_counters = true;
> + 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
> @@ -839,7 +860,7 @@ static void riscv_cpu_init(Object *obj)
> #endif /* CONFIG_USER_ONLY */
> }
>
> -static Property riscv_cpu_properties[] = {
> +static Property riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
> DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
> @@ -862,17 +883,12 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false),
> DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> - DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
>
> DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
> DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> - DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> - DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
> - DEFINE_PROP_UINT64("mimpid", RISCVCPU, cfg.mimpid, RISCV_CPU_MIMPID),
> -
> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> @@ -909,6 +925,25 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
> DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
>
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void register_cpu_props(DeviceState *dev)
> +{
> + Property *prop;
> +
> + for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
> + qdev_property_add_static(dev, prop);
> + }
> +}
> +
> +static Property riscv_cpu_properties[] = {
> + DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
> +
> + DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> + 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),
>
> DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string,
> false),
> --
Otherwise LGTM.
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Regards,
Bin