* [PATCH 0/2] target/riscv: Cleanup exposed CPU properties @ 2022-05-17 4:10 Alistair Francis 2022-05-17 4:10 ` [PATCH 1/2] target/riscv: Don't expose the CPU properties on names CPUs Alistair Francis 2022-05-17 4:11 ` [PATCH 2/2] target/riscv: Run extension checks for all CPUs Alistair Francis 0 siblings, 2 replies; 5+ messages in thread From: Alistair Francis @ 2022-05-17 4:10 UTC (permalink / raw) To: qemu-riscv, qemu-devel; +Cc: bmeng.cn, palmer, alistair23 From: Alistair Francis <alistair.francis@wdc.com> The RISC-V CPUs have been incorrectly enabling features in the named vendor CPUs that aren't enabled in hardware. This patchset changes this so that named vendor CPUs are not runtime configurable. I was torn for the best approach here. The other idea I had was to disable features by default and instead enable them in CPUs. I ended up going this approach as I felt it made more sense to not expose configuration options for vendor CPUs, it just seems difficult to support now that we have a large list of CPUs Alistair Francis (2): target/riscv: Don't expose the CPU properties on names CPUs target/riscv: Run extension checks for all CPUs target/riscv/cpu.c | 217 ++++++++++++++++++++++++++------------------- 1 file changed, 125 insertions(+), 92 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] target/riscv: Don't expose the CPU properties on names CPUs 2022-05-17 4:10 [PATCH 0/2] target/riscv: Cleanup exposed CPU properties Alistair Francis @ 2022-05-17 4:10 ` Alistair Francis 2022-05-17 4:11 ` [PATCH 2/2] target/riscv: Run extension checks for all CPUs Alistair Francis 1 sibling, 0 replies; 5+ messages in thread From: Alistair Francis @ 2022-05-17 4:10 UTC (permalink / raw) To: qemu-riscv, qemu-devel; +Cc: bmeng.cn, palmer, alistair23 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 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. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- target/riscv/cpu.c | 56 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 6d01569cad..49b844535a 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); + 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 @@ -829,6 +843,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 @@ -837,7 +857,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), @@ -860,17 +880,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("mipid", RISCVCPU, cfg.mipid, RISCV_CPU_MIPID), - 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), @@ -907,6 +922,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("mipid", RISCVCPU, cfg.mipid, RISCV_CPU_MIPID), + DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC), DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false), -- 2.35.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] target/riscv: Run extension checks for all CPUs 2022-05-17 4:10 [PATCH 0/2] target/riscv: Cleanup exposed CPU properties Alistair Francis 2022-05-17 4:10 ` [PATCH 1/2] target/riscv: Don't expose the CPU properties on names CPUs Alistair Francis @ 2022-05-17 4:11 ` Alistair Francis 2022-05-17 5:02 ` Weiwei Li 1 sibling, 1 reply; 5+ messages in thread From: Alistair Francis @ 2022-05-17 4:11 UTC (permalink / raw) To: qemu-riscv, qemu-devel; +Cc: bmeng.cn, palmer, alistair23 From: Alistair Francis <alistair.francis@wdc.com> Instead of just running the extension checks for the base CPUs, instead run them for all CPUs. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- target/riscv/cpu.c | 161 ++++++++++++++++++++++----------------------- 1 file changed, 80 insertions(+), 81 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 49b844535a..ee48a18ae4 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -593,102 +593,101 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) } assert(env->misa_mxl_max == env->misa_mxl); - /* If only MISA_EXT is unset for misa, then set it from properties */ - if (env->misa_ext == 0) { - uint32_t ext = 0; + /* Do some ISA extension error checking */ + if (cpu->cfg.ext_i && cpu->cfg.ext_e) { + error_setg(errp, + "I and E extensions are incompatible"); + return; + } - /* Do some ISA extension error checking */ - if (cpu->cfg.ext_i && cpu->cfg.ext_e) { - error_setg(errp, - "I and E extensions are incompatible"); - return; - } + if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { + error_setg(errp, + "Either I or E extension must be set"); + return; + } - if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { - error_setg(errp, - "Either I or E extension must be set"); - return; - } + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && + cpu->cfg.ext_a && cpu->cfg.ext_f && + cpu->cfg.ext_d && + cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { + warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); + cpu->cfg.ext_i = true; + cpu->cfg.ext_m = true; + cpu->cfg.ext_a = true; + cpu->cfg.ext_f = true; + cpu->cfg.ext_d = true; + cpu->cfg.ext_icsr = true; + cpu->cfg.ext_ifencei = true; + } - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && - cpu->cfg.ext_a && cpu->cfg.ext_f && - cpu->cfg.ext_d && - cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); - cpu->cfg.ext_i = true; - cpu->cfg.ext_m = true; - cpu->cfg.ext_a = true; - cpu->cfg.ext_f = true; - cpu->cfg.ext_d = true; - cpu->cfg.ext_icsr = true; - cpu->cfg.ext_ifencei = true; - } + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { + error_setg(errp, "F extension requires Zicsr"); + return; + } - if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { - error_setg(errp, "F extension requires Zicsr"); - return; - } + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); + return; + } - if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { - error_setg(errp, "Zfh/Zfhmin extensions require F extension"); - return; - } + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { + error_setg(errp, "D extension requires F extension"); + return; + } - if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { - error_setg(errp, "D extension requires F extension"); - return; - } + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { + error_setg(errp, "V extension requires D extension"); + return; + } - if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { - error_setg(errp, "V extension requires D extension"); - return; - } + if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { + error_setg(errp, "Zve32f/Zve64f extensions require F extension"); + return; + } + + if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || + cpu->cfg.ext_zhinxmin) { + cpu->cfg.ext_zfinx = true; + } - if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { - error_setg(errp, "Zve32f/Zve64f extensions require F extension"); + if (cpu->cfg.ext_zfinx) { + if (!cpu->cfg.ext_icsr) { + error_setg(errp, "Zfinx extension requires Zicsr"); return; } - - /* Set the ISA extensions, checks should have happened above */ - if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || - cpu->cfg.ext_zhinxmin) { - cpu->cfg.ext_zfinx = true; + if (cpu->cfg.ext_f) { + error_setg(errp, + "Zfinx cannot be supported together with F extension"); + return; } + } - if (cpu->cfg.ext_zfinx) { - if (!cpu->cfg.ext_icsr) { - error_setg(errp, "Zfinx extension requires Zicsr"); - return; - } - if (cpu->cfg.ext_f) { - error_setg(errp, - "Zfinx cannot be supported together with F extension"); - return; - } - } + if (cpu->cfg.ext_zk) { + cpu->cfg.ext_zkn = true; + cpu->cfg.ext_zkr = true; + cpu->cfg.ext_zkt = true; + } - if (cpu->cfg.ext_zk) { - cpu->cfg.ext_zkn = true; - cpu->cfg.ext_zkr = true; - cpu->cfg.ext_zkt = true; - } + if (cpu->cfg.ext_zkn) { + cpu->cfg.ext_zbkb = true; + cpu->cfg.ext_zbkc = true; + cpu->cfg.ext_zbkx = true; + cpu->cfg.ext_zkne = true; + cpu->cfg.ext_zknd = true; + cpu->cfg.ext_zknh = true; + } - if (cpu->cfg.ext_zkn) { - cpu->cfg.ext_zbkb = true; - cpu->cfg.ext_zbkc = true; - cpu->cfg.ext_zbkx = true; - cpu->cfg.ext_zkne = true; - cpu->cfg.ext_zknd = true; - cpu->cfg.ext_zknh = true; - } + if (cpu->cfg.ext_zks) { + cpu->cfg.ext_zbkb = true; + cpu->cfg.ext_zbkc = true; + cpu->cfg.ext_zbkx = true; + cpu->cfg.ext_zksed = true; + cpu->cfg.ext_zksh = true; + } - if (cpu->cfg.ext_zks) { - cpu->cfg.ext_zbkb = true; - cpu->cfg.ext_zbkc = true; - cpu->cfg.ext_zbkx = true; - cpu->cfg.ext_zksed = true; - cpu->cfg.ext_zksh = true; - } + /* If only MISA_EXT is unset for misa, then set it from properties */ + if (env->misa_ext == 0) { + uint32_t ext = 0; if (cpu->cfg.ext_i) { ext |= RVI; -- 2.35.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] target/riscv: Run extension checks for all CPUs 2022-05-17 4:11 ` [PATCH 2/2] target/riscv: Run extension checks for all CPUs Alistair Francis @ 2022-05-17 5:02 ` Weiwei Li 2022-05-17 5:07 ` Alistair Francis 0 siblings, 1 reply; 5+ messages in thread From: Weiwei Li @ 2022-05-17 5:02 UTC (permalink / raw) To: Alistair Francis, qemu-riscv, qemu-devel; +Cc: bmeng.cn, palmer, alistair23 在 2022/5/17 下午12:11, Alistair Francis 写道: > From: Alistair Francis <alistair.francis@wdc.com> > > Instead of just running the extension checks for the base CPUs, instead > run them for all CPUs. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/cpu.c | 161 ++++++++++++++++++++++----------------------- > 1 file changed, 80 insertions(+), 81 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 49b844535a..ee48a18ae4 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -593,102 +593,101 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > } > assert(env->misa_mxl_max == env->misa_mxl); > > - /* If only MISA_EXT is unset for misa, then set it from properties */ > - if (env->misa_ext == 0) { > - uint32_t ext = 0; > + /* Do some ISA extension error checking */ > + if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > + error_setg(errp, > + "I and E extensions are incompatible"); > + return; > + } > > - /* Do some ISA extension error checking */ > - if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > - error_setg(errp, > - "I and E extensions are incompatible"); > - return; > - } > + if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { > + error_setg(errp, > + "Either I or E extension must be set"); > + return; > + } > > - if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { > - error_setg(errp, > - "Either I or E extension must be set"); > - return; > - } > + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && > + cpu->cfg.ext_a && cpu->cfg.ext_f && > + cpu->cfg.ext_d && > + cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { > + warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > + cpu->cfg.ext_i = true; > + cpu->cfg.ext_m = true; > + cpu->cfg.ext_a = true; > + cpu->cfg.ext_f = true; > + cpu->cfg.ext_d = true; > + cpu->cfg.ext_icsr = true; > + cpu->cfg.ext_ifencei = true; > + } Maybe you can merge the changes from my first patch to here and put this before the check on 'I' and 'E'. Regards, Weiwei Li > > - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && > - cpu->cfg.ext_a && cpu->cfg.ext_f && > - cpu->cfg.ext_d && > - cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { > - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > - cpu->cfg.ext_i = true; > - cpu->cfg.ext_m = true; > - cpu->cfg.ext_a = true; > - cpu->cfg.ext_f = true; > - cpu->cfg.ext_d = true; > - cpu->cfg.ext_icsr = true; > - cpu->cfg.ext_ifencei = true; > - } > + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > + error_setg(errp, "F extension requires Zicsr"); > + return; > + } > > - if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > - error_setg(errp, "F extension requires Zicsr"); > - return; > - } > + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > + return; > + } > > - if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > - error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > - return; > - } > + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > + error_setg(errp, "D extension requires F extension"); > + return; > + } > > - if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > - error_setg(errp, "D extension requires F extension"); > - return; > - } > + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > + error_setg(errp, "V extension requires D extension"); > + return; > + } > > - if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > - error_setg(errp, "V extension requires D extension"); > - return; > - } > + if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { > + error_setg(errp, "Zve32f/Zve64f extensions require F extension"); > + return; > + } > + > + if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > + cpu->cfg.ext_zhinxmin) { > + cpu->cfg.ext_zfinx = true; > + } > > - if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { > - error_setg(errp, "Zve32f/Zve64f extensions require F extension"); > + if (cpu->cfg.ext_zfinx) { > + if (!cpu->cfg.ext_icsr) { > + error_setg(errp, "Zfinx extension requires Zicsr"); > return; > } > - > - /* Set the ISA extensions, checks should have happened above */ > - if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > - cpu->cfg.ext_zhinxmin) { > - cpu->cfg.ext_zfinx = true; > + if (cpu->cfg.ext_f) { > + error_setg(errp, > + "Zfinx cannot be supported together with F extension"); > + return; > } > + } > > - if (cpu->cfg.ext_zfinx) { > - if (!cpu->cfg.ext_icsr) { > - error_setg(errp, "Zfinx extension requires Zicsr"); > - return; > - } > - if (cpu->cfg.ext_f) { > - error_setg(errp, > - "Zfinx cannot be supported together with F extension"); > - return; > - } > - } > + if (cpu->cfg.ext_zk) { > + cpu->cfg.ext_zkn = true; > + cpu->cfg.ext_zkr = true; > + cpu->cfg.ext_zkt = true; > + } > > - if (cpu->cfg.ext_zk) { > - cpu->cfg.ext_zkn = true; > - cpu->cfg.ext_zkr = true; > - cpu->cfg.ext_zkt = true; > - } > + if (cpu->cfg.ext_zkn) { > + cpu->cfg.ext_zbkb = true; > + cpu->cfg.ext_zbkc = true; > + cpu->cfg.ext_zbkx = true; > + cpu->cfg.ext_zkne = true; > + cpu->cfg.ext_zknd = true; > + cpu->cfg.ext_zknh = true; > + } > > - if (cpu->cfg.ext_zkn) { > - cpu->cfg.ext_zbkb = true; > - cpu->cfg.ext_zbkc = true; > - cpu->cfg.ext_zbkx = true; > - cpu->cfg.ext_zkne = true; > - cpu->cfg.ext_zknd = true; > - cpu->cfg.ext_zknh = true; > - } > + if (cpu->cfg.ext_zks) { > + cpu->cfg.ext_zbkb = true; > + cpu->cfg.ext_zbkc = true; > + cpu->cfg.ext_zbkx = true; > + cpu->cfg.ext_zksed = true; > + cpu->cfg.ext_zksh = true; > + } > > - if (cpu->cfg.ext_zks) { > - cpu->cfg.ext_zbkb = true; > - cpu->cfg.ext_zbkc = true; > - cpu->cfg.ext_zbkx = true; > - cpu->cfg.ext_zksed = true; > - cpu->cfg.ext_zksh = true; > - } > + /* If only MISA_EXT is unset for misa, then set it from properties */ > + if (env->misa_ext == 0) { > + uint32_t ext = 0; > > if (cpu->cfg.ext_i) { > ext |= RVI; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] target/riscv: Run extension checks for all CPUs 2022-05-17 5:02 ` Weiwei Li @ 2022-05-17 5:07 ` Alistair Francis 0 siblings, 0 replies; 5+ messages in thread From: Alistair Francis @ 2022-05-17 5:07 UTC (permalink / raw) To: Weiwei Li Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers, Bin Meng, Palmer Dabbelt On Tue, May 17, 2022 at 3:02 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > 在 2022/5/17 下午12:11, Alistair Francis 写道: > > From: Alistair Francis <alistair.francis@wdc.com> > > > > Instead of just running the extension checks for the base CPUs, instead > > run them for all CPUs. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > target/riscv/cpu.c | 161 ++++++++++++++++++++++----------------------- > > 1 file changed, 80 insertions(+), 81 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index 49b844535a..ee48a18ae4 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -593,102 +593,101 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > > } > > assert(env->misa_mxl_max == env->misa_mxl); > > > > - /* If only MISA_EXT is unset for misa, then set it from properties */ > > - if (env->misa_ext == 0) { > > - uint32_t ext = 0; > > + /* Do some ISA extension error checking */ > > + if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > > + error_setg(errp, > > + "I and E extensions are incompatible"); > > + return; > > + } > > > > - /* Do some ISA extension error checking */ > > - if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > > - error_setg(errp, > > - "I and E extensions are incompatible"); > > - return; > > - } > > + if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { > > + error_setg(errp, > > + "Either I or E extension must be set"); > > + return; > > + } > > > > - if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) { > > - error_setg(errp, > > - "Either I or E extension must be set"); > > - return; > > - } > > + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && > > + cpu->cfg.ext_a && cpu->cfg.ext_f && > > + cpu->cfg.ext_d && > > + cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { > > + warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > > + cpu->cfg.ext_i = true; > > + cpu->cfg.ext_m = true; > > + cpu->cfg.ext_a = true; > > + cpu->cfg.ext_f = true; > > + cpu->cfg.ext_d = true; > > + cpu->cfg.ext_icsr = true; > > + cpu->cfg.ext_ifencei = true; > > + } > > Maybe you can merge the changes from my first patch to here and put this > before the check on 'I' and 'E'. This patch causes some failures as the extensions aren't set on all machines now, so I'm actually going to drop it Alistair > > Regards, > > Weiwei Li > > > > > - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m && > > - cpu->cfg.ext_a && cpu->cfg.ext_f && > > - cpu->cfg.ext_d && > > - cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) { > > - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > > - cpu->cfg.ext_i = true; > > - cpu->cfg.ext_m = true; > > - cpu->cfg.ext_a = true; > > - cpu->cfg.ext_f = true; > > - cpu->cfg.ext_d = true; > > - cpu->cfg.ext_icsr = true; > > - cpu->cfg.ext_ifencei = true; > > - } > > + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > > + error_setg(errp, "F extension requires Zicsr"); > > + return; > > + } > > > > - if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > > - error_setg(errp, "F extension requires Zicsr"); > > - return; > > - } > > + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > > + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > > + return; > > + } > > > > - if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > > - error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > > - return; > > - } > > + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > > + error_setg(errp, "D extension requires F extension"); > > + return; > > + } > > > > - if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > > - error_setg(errp, "D extension requires F extension"); > > - return; > > - } > > + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > > + error_setg(errp, "V extension requires D extension"); > > + return; > > + } > > > > - if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > > - error_setg(errp, "V extension requires D extension"); > > - return; > > - } > > + if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { > > + error_setg(errp, "Zve32f/Zve64f extensions require F extension"); > > + return; > > + } > > + > > + if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > > + cpu->cfg.ext_zhinxmin) { > > + cpu->cfg.ext_zfinx = true; > > + } > > > > - if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) { > > - error_setg(errp, "Zve32f/Zve64f extensions require F extension"); > > + if (cpu->cfg.ext_zfinx) { > > + if (!cpu->cfg.ext_icsr) { > > + error_setg(errp, "Zfinx extension requires Zicsr"); > > return; > > } > > - > > - /* Set the ISA extensions, checks should have happened above */ > > - if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > > - cpu->cfg.ext_zhinxmin) { > > - cpu->cfg.ext_zfinx = true; > > + if (cpu->cfg.ext_f) { > > + error_setg(errp, > > + "Zfinx cannot be supported together with F extension"); > > + return; > > } > > + } > > > > - if (cpu->cfg.ext_zfinx) { > > - if (!cpu->cfg.ext_icsr) { > > - error_setg(errp, "Zfinx extension requires Zicsr"); > > - return; > > - } > > - if (cpu->cfg.ext_f) { > > - error_setg(errp, > > - "Zfinx cannot be supported together with F extension"); > > - return; > > - } > > - } > > + if (cpu->cfg.ext_zk) { > > + cpu->cfg.ext_zkn = true; > > + cpu->cfg.ext_zkr = true; > > + cpu->cfg.ext_zkt = true; > > + } > > > > - if (cpu->cfg.ext_zk) { > > - cpu->cfg.ext_zkn = true; > > - cpu->cfg.ext_zkr = true; > > - cpu->cfg.ext_zkt = true; > > - } > > + if (cpu->cfg.ext_zkn) { > > + cpu->cfg.ext_zbkb = true; > > + cpu->cfg.ext_zbkc = true; > > + cpu->cfg.ext_zbkx = true; > > + cpu->cfg.ext_zkne = true; > > + cpu->cfg.ext_zknd = true; > > + cpu->cfg.ext_zknh = true; > > + } > > > > - if (cpu->cfg.ext_zkn) { > > - cpu->cfg.ext_zbkb = true; > > - cpu->cfg.ext_zbkc = true; > > - cpu->cfg.ext_zbkx = true; > > - cpu->cfg.ext_zkne = true; > > - cpu->cfg.ext_zknd = true; > > - cpu->cfg.ext_zknh = true; > > - } > > + if (cpu->cfg.ext_zks) { > > + cpu->cfg.ext_zbkb = true; > > + cpu->cfg.ext_zbkc = true; > > + cpu->cfg.ext_zbkx = true; > > + cpu->cfg.ext_zksed = true; > > + cpu->cfg.ext_zksh = true; > > + } > > > > - if (cpu->cfg.ext_zks) { > > - cpu->cfg.ext_zbkb = true; > > - cpu->cfg.ext_zbkc = true; > > - cpu->cfg.ext_zbkx = true; > > - cpu->cfg.ext_zksed = true; > > - cpu->cfg.ext_zksh = true; > > - } > > + /* If only MISA_EXT is unset for misa, then set it from properties */ > > + if (env->misa_ext == 0) { > > + uint32_t ext = 0; > > > > if (cpu->cfg.ext_i) { > > ext |= RVI; > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-17 5:09 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-05-17 4:10 [PATCH 0/2] target/riscv: Cleanup exposed CPU properties Alistair Francis 2022-05-17 4:10 ` [PATCH 1/2] target/riscv: Don't expose the CPU properties on names CPUs Alistair Francis 2022-05-17 4:11 ` [PATCH 2/2] target/riscv: Run extension checks for all CPUs Alistair Francis 2022-05-17 5:02 ` Weiwei Li 2022-05-17 5:07 ` Alistair Francis
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).