* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.