* [PATCH 0/3] QEMU RISC-V priv spec version fixes
@ 2022-04-29 15:34 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
This series covers few fixes discovered while trying to detect priv spec
version on QEMU virt machine and QEMU sifive_u machine.
These patches can also be found in riscv_priv_version_fixes_v1 branch at:
https://github.com/avpatel/qemu.git
Anup Patel (3):
target/riscv: Don't force update priv spec version to latest
target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or
higher
target/riscv: Consider priv spec version when generating ISA string
target/riscv/cpu.c | 46 ++++++++++++++++++++++-------------------
target/riscv/cpu_bits.h | 3 +++
target/riscv/csr.c | 2 ++
3 files changed, 30 insertions(+), 21 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/3] QEMU RISC-V priv spec version fixes
@ 2022-04-29 15:34 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
This series covers few fixes discovered while trying to detect priv spec
version on QEMU virt machine and QEMU sifive_u machine.
These patches can also be found in riscv_priv_version_fixes_v1 branch at:
https://github.com/avpatel/qemu.git
Anup Patel (3):
target/riscv: Don't force update priv spec version to latest
target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or
higher
target/riscv: Consider priv spec version when generating ISA string
target/riscv/cpu.c | 46 ++++++++++++++++++++++-------------------
target/riscv/cpu_bits.h | 3 +++
target/riscv/csr.c | 2 ++
3 files changed, 30 insertions(+), 21 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
2022-04-29 15:34 ` Anup Patel
@ 2022-04-29 15:34 ` Anup Patel
-1 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
when "env->priv_ver == 0" (i.e. default v1.10) because the enum
value of priv spec v1.10 is zero.
Due to above issue, the sifive_u machine will see priv spec v1.12
instead of priv spec v1.10.
To fix this issue, we set latest priv spec version (i.e. v1.12)
for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
spec version only when "cpu->cfg.priv_spec != NULL".
Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f0a702fee6..02ee7d45d8 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -169,6 +169,8 @@ 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);
+ /* Set latest version of privileged specification */
+ set_priv_version(env, PRIV_VERSION_1_12_0);
}
static void rv64_sifive_u_cpu_init(Object *obj)
@@ -204,6 +206,8 @@ 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);
+ /* Set latest version of privileged specification */
+ set_priv_version(env, PRIV_VERSION_1_12_0);
}
static void rv32_sifive_u_cpu_init(Object *obj)
@@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
CPURISCVState *env = &cpu->env;
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
CPUClass *cc = CPU_CLASS(mcc);
- int priv_version = 0;
+ int priv_version = -1;
Error *local_err = NULL;
cpu_exec_realizefn(cs, &local_err);
@@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
}
}
- if (priv_version) {
+ if (priv_version >= PRIV_VERSION_1_10_0) {
set_priv_version(env, priv_version);
- } else if (!env->priv_ver) {
- set_priv_version(env, PRIV_VERSION_1_12_0);
}
if (cpu->cfg.mmu) {
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
@ 2022-04-29 15:34 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
when "env->priv_ver == 0" (i.e. default v1.10) because the enum
value of priv spec v1.10 is zero.
Due to above issue, the sifive_u machine will see priv spec v1.12
instead of priv spec v1.10.
To fix this issue, we set latest priv spec version (i.e. v1.12)
for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
spec version only when "cpu->cfg.priv_spec != NULL".
Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f0a702fee6..02ee7d45d8 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -169,6 +169,8 @@ 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);
+ /* Set latest version of privileged specification */
+ set_priv_version(env, PRIV_VERSION_1_12_0);
}
static void rv64_sifive_u_cpu_init(Object *obj)
@@ -204,6 +206,8 @@ 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);
+ /* Set latest version of privileged specification */
+ set_priv_version(env, PRIV_VERSION_1_12_0);
}
static void rv32_sifive_u_cpu_init(Object *obj)
@@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
CPURISCVState *env = &cpu->env;
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
CPUClass *cc = CPU_CLASS(mcc);
- int priv_version = 0;
+ int priv_version = -1;
Error *local_err = NULL;
cpu_exec_realizefn(cs, &local_err);
@@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
}
}
- if (priv_version) {
+ if (priv_version >= PRIV_VERSION_1_10_0) {
set_priv_version(env, priv_version);
- } else if (!env->priv_ver) {
- set_priv_version(env, PRIV_VERSION_1_12_0);
}
if (cpu->cfg.mmu) {
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
2022-04-29 15:34 ` Anup Patel
@ 2022-04-29 15:34 ` Anup Patel
-1 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
implementation that don't want to implement can simply have a dummy
mcountinhibit which always zero.
Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
the CSR ops.")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_bits.h | 3 +++
target/riscv/csr.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 4d04b20d06..4a55c6a709 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -159,6 +159,9 @@
#define CSR_MTVEC 0x305
#define CSR_MCOUNTEREN 0x306
+/* Machine Counter Setup */
+#define CSR_MCOUNTINHIBIT 0x320
+
/* 32-bit only */
#define CSR_MSTATUSH 0x310
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2bf0a97196..e144ce7135 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie },
[CSR_MTVEC] = { "mtvec", any, read_mtvec, write_mtvec },
[CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren, write_mcounteren },
+ [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
+ .min_priv_ver = PRIV_VERSION_1_11_0 },
[CSR_MSTATUSH] = { "mstatush", any32, read_mstatush, write_mstatush },
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
@ 2022-04-29 15:34 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
implementation that don't want to implement can simply have a dummy
mcountinhibit which always zero.
Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
the CSR ops.")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu_bits.h | 3 +++
target/riscv/csr.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 4d04b20d06..4a55c6a709 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -159,6 +159,9 @@
#define CSR_MTVEC 0x305
#define CSR_MCOUNTEREN 0x306
+/* Machine Counter Setup */
+#define CSR_MCOUNTINHIBIT 0x320
+
/* 32-bit only */
#define CSR_MSTATUSH 0x310
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2bf0a97196..e144ce7135 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie },
[CSR_MTVEC] = { "mtvec", any, read_mtvec, write_mtvec },
[CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren, write_mcounteren },
+ [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
+ .min_priv_ver = PRIV_VERSION_1_11_0 },
[CSR_MSTATUSH] = { "mstatush", any32, read_mstatush, write_mstatush },
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
2022-04-29 15:34 ` Anup Patel
@ 2022-04-29 15:34 ` Anup Patel
-1 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Anup Patel, Anup Patel, qemu-riscv, qemu-devel, Atish Patra
Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
etc) are only available after Priv spec v1.12 so ISA string generation
should check the minimum required priv spec version for all extensions.
Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
device tree")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 02ee7d45d8..d8c88b96bc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
struct isa_ext_data {
const char *name;
bool enabled;
+ uint32_t min_priv_ver;
};
const char * const riscv_int_regnames[] = {
@@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
device_class_set_props(dc, riscv_cpu_properties);
}
-#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
+#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
{
@@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
* extensions by an underscore.
*/
struct isa_ext_data isa_edata_arr[] = {
- ISA_EDATA_ENTRY(zfh, ext_zfh),
- ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
- ISA_EDATA_ENTRY(zfinx, ext_zfinx),
- ISA_EDATA_ENTRY(zhinx, ext_zhinx),
- ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
- ISA_EDATA_ENTRY(zdinx, ext_zdinx),
- ISA_EDATA_ENTRY(zba, ext_zba),
- ISA_EDATA_ENTRY(zbb, ext_zbb),
- ISA_EDATA_ENTRY(zbc, ext_zbc),
- ISA_EDATA_ENTRY(zbs, ext_zbs),
- ISA_EDATA_ENTRY(zve32f, ext_zve32f),
- ISA_EDATA_ENTRY(zve64f, ext_zve64f),
- ISA_EDATA_ENTRY(svinval, ext_svinval),
- ISA_EDATA_ENTRY(svnapot, ext_svnapot),
- ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
+ ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
};
for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
- if (isa_edata_arr[i].enabled) {
+ if (isa_edata_arr[i].enabled &&
+ cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
g_free(old);
old = new;
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
@ 2022-04-29 15:34 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-29 15:34 UTC (permalink / raw)
To: Peter Maydell, Palmer Dabbelt, Alistair Francis, Sagar Karandikar
Cc: Atish Patra, Anup Patel, qemu-riscv, qemu-devel, Anup Patel
Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
etc) are only available after Priv spec v1.12 so ISA string generation
should check the minimum required priv spec version for all extensions.
Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
device tree")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 02ee7d45d8..d8c88b96bc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
struct isa_ext_data {
const char *name;
bool enabled;
+ uint32_t min_priv_ver;
};
const char * const riscv_int_regnames[] = {
@@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
device_class_set_props(dc, riscv_cpu_properties);
}
-#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
+#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
{
@@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
* extensions by an underscore.
*/
struct isa_ext_data isa_edata_arr[] = {
- ISA_EDATA_ENTRY(zfh, ext_zfh),
- ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
- ISA_EDATA_ENTRY(zfinx, ext_zfinx),
- ISA_EDATA_ENTRY(zhinx, ext_zhinx),
- ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
- ISA_EDATA_ENTRY(zdinx, ext_zdinx),
- ISA_EDATA_ENTRY(zba, ext_zba),
- ISA_EDATA_ENTRY(zbb, ext_zbb),
- ISA_EDATA_ENTRY(zbc, ext_zbc),
- ISA_EDATA_ENTRY(zbs, ext_zbs),
- ISA_EDATA_ENTRY(zve32f, ext_zve32f),
- ISA_EDATA_ENTRY(zve64f, ext_zve64f),
- ISA_EDATA_ENTRY(svinval, ext_svinval),
- ISA_EDATA_ENTRY(svnapot, ext_svnapot),
- ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
+ ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
+ ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
};
for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
- if (isa_edata_arr[i].enabled) {
+ if (isa_edata_arr[i].enabled &&
+ cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
g_free(old);
old = new;
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
2022-04-29 15:34 ` Anup Patel
@ 2022-04-30 2:57 ` Frank Chang
-1 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 2:57 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 2442 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:41 PM Anup Patel <apatel@ventanamicro.com> wrote:
> The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
> when "env->priv_ver == 0" (i.e. default v1.10) because the enum
> value of priv spec v1.10 is zero.
>
> Due to above issue, the sifive_u machine will see priv spec v1.12
> instead of priv spec v1.10.
>
> To fix this issue, we set latest priv spec version (i.e. v1.12)
> for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
> spec version only when "cpu->cfg.priv_spec != NULL".
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f0a702fee6..02ee7d45d8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -169,6 +169,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -204,6 +206,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> CPURISCVState *env = &cpu->env;
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> CPUClass *cc = CPU_CLASS(mcc);
> - int priv_version = 0;
> + int priv_version = -1;
> Error *local_err = NULL;
>
> cpu_exec_realizefn(cs, &local_err);
> @@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> }
> }
>
> - if (priv_version) {
> + if (priv_version >= PRIV_VERSION_1_10_0) {
> set_priv_version(env, priv_version);
> - } else if (!env->priv_ver) {
> - set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> if (cpu->cfg.mmu) {
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 3211 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
@ 2022-04-30 2:57 ` Frank Chang
0 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 2:57 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 2442 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:41 PM Anup Patel <apatel@ventanamicro.com> wrote:
> The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
> when "env->priv_ver == 0" (i.e. default v1.10) because the enum
> value of priv spec v1.10 is zero.
>
> Due to above issue, the sifive_u machine will see priv spec v1.12
> instead of priv spec v1.10.
>
> To fix this issue, we set latest priv spec version (i.e. v1.12)
> for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
> spec version only when "cpu->cfg.priv_spec != NULL".
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f0a702fee6..02ee7d45d8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -169,6 +169,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -204,6 +206,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> CPURISCVState *env = &cpu->env;
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> CPUClass *cc = CPU_CLASS(mcc);
> - int priv_version = 0;
> + int priv_version = -1;
> Error *local_err = NULL;
>
> cpu_exec_realizefn(cs, &local_err);
> @@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> }
> }
>
> - if (priv_version) {
> + if (priv_version >= PRIV_VERSION_1_10_0) {
> set_priv_version(env, priv_version);
> - } else if (!env->priv_ver) {
> - set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> if (cpu->cfg.mmu) {
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 3211 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
2022-04-29 15:34 ` Anup Patel
@ 2022-04-30 3:09 ` Frank Chang
-1 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 3:09 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 4120 bytes --]
Hi Anup,
If we want to limit the generated ISA string to/after a specific privilege
spec version.
Shouldn't we also check the privilege spec version when these extensions
are enabled?
Otherwise, it's possible that one extension is enabled,
but the privilege spec version is smaller than the one in which the
extension is supported.
(This is possible if user specifies the privileged spec version through the
command line.)
The ISA string therefore won't include the enabled extension.
Regards,
Frank Chang
On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com> wrote:
> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
> etc) are only available after Priv spec v1.12 so ISA string generation
> should check the minimum required priv spec version for all extensions.
>
> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> device tree")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 02ee7d45d8..d8c88b96bc 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] =
> "IEMAFDQCPVH";
> struct isa_ext_data {
> const char *name;
> bool enabled;
> + uint32_t min_priv_ver;
> };
>
> const char * const riscv_int_regnames[] = {
> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
> *data)
> device_class_set_props(dc, riscv_cpu_properties);
> }
>
> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
>
> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int
> max_str_len)
> {
> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> * extensions by an underscore.
> */
> struct isa_ext_data isa_edata_arr[] = {
> - ISA_EDATA_ENTRY(zfh, ext_zfh),
> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
> - ISA_EDATA_ENTRY(zba, ext_zba),
> - ISA_EDATA_ENTRY(zbb, ext_zbb),
> - ISA_EDATA_ENTRY(zbc, ext_zbc),
> - ISA_EDATA_ENTRY(zbs, ext_zbs),
> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> - ISA_EDATA_ENTRY(svinval, ext_svinval),
> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
> };
>
> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> - if (isa_edata_arr[i].enabled) {
> + if (isa_edata_arr[i].enabled &&
> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
> g_free(old);
> old = new;
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 5115 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
@ 2022-04-30 3:09 ` Frank Chang
0 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 3:09 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 4120 bytes --]
Hi Anup,
If we want to limit the generated ISA string to/after a specific privilege
spec version.
Shouldn't we also check the privilege spec version when these extensions
are enabled?
Otherwise, it's possible that one extension is enabled,
but the privilege spec version is smaller than the one in which the
extension is supported.
(This is possible if user specifies the privileged spec version through the
command line.)
The ISA string therefore won't include the enabled extension.
Regards,
Frank Chang
On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com> wrote:
> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
> etc) are only available after Priv spec v1.12 so ISA string generation
> should check the minimum required priv spec version for all extensions.
>
> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> device tree")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 02ee7d45d8..d8c88b96bc 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] =
> "IEMAFDQCPVH";
> struct isa_ext_data {
> const char *name;
> bool enabled;
> + uint32_t min_priv_ver;
> };
>
> const char * const riscv_int_regnames[] = {
> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
> *data)
> device_class_set_props(dc, riscv_cpu_properties);
> }
>
> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
>
> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int
> max_str_len)
> {
> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> * extensions by an underscore.
> */
> struct isa_ext_data isa_edata_arr[] = {
> - ISA_EDATA_ENTRY(zfh, ext_zfh),
> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
> - ISA_EDATA_ENTRY(zba, ext_zba),
> - ISA_EDATA_ENTRY(zbb, ext_zbb),
> - ISA_EDATA_ENTRY(zbc, ext_zbc),
> - ISA_EDATA_ENTRY(zbs, ext_zbs),
> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> - ISA_EDATA_ENTRY(svinval, ext_svinval),
> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
> };
>
> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> - if (isa_edata_arr[i].enabled) {
> + if (isa_edata_arr[i].enabled &&
> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
> g_free(old);
> old = new;
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 5115 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
2022-04-29 15:34 ` Anup Patel
@ 2022-04-30 3:13 ` Frank Chang
-1 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 3:13 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, open list:RISC-V, Sagar Karandikar, Anup Patel,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:44 PM Anup Patel <apatel@ventanamicro.com> wrote:
> The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
> implementation that don't want to implement can simply have a dummy
> mcountinhibit which always zero.
>
> Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
> the CSR ops.")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/csr.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 4d04b20d06..4a55c6a709 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -159,6 +159,9 @@
> #define CSR_MTVEC 0x305
> #define CSR_MCOUNTEREN 0x306
>
> +/* Machine Counter Setup */
> +#define CSR_MCOUNTINHIBIT 0x320
> +
> /* 32-bit only */
> #define CSR_MSTATUSH 0x310
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2bf0a97196..e144ce7135 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie
> },
> [CSR_MTVEC] = { "mtvec", any, read_mtvec,
> write_mtvec },
> [CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren,
> write_mcounteren },
> + [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
> + .min_priv_ver =
> PRIV_VERSION_1_11_0 },
>
> [CSR_MSTATUSH] = { "mstatush", any32, read_mstatush,
> write_mstatush },
>
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2463 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
@ 2022-04-30 3:13 ` Frank Chang
0 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 3:13 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Fri, Apr 29, 2022 at 11:44 PM Anup Patel <apatel@ventanamicro.com> wrote:
> The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
> implementation that don't want to implement can simply have a dummy
> mcountinhibit which always zero.
>
> Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
> the CSR ops.")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/csr.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 4d04b20d06..4a55c6a709 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -159,6 +159,9 @@
> #define CSR_MTVEC 0x305
> #define CSR_MCOUNTEREN 0x306
>
> +/* Machine Counter Setup */
> +#define CSR_MCOUNTINHIBIT 0x320
> +
> /* 32-bit only */
> #define CSR_MSTATUSH 0x310
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2bf0a97196..e144ce7135 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie
> },
> [CSR_MTVEC] = { "mtvec", any, read_mtvec,
> write_mtvec },
> [CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren,
> write_mcounteren },
> + [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
> + .min_priv_ver =
> PRIV_VERSION_1_11_0 },
>
> [CSR_MSTATUSH] = { "mstatush", any32, read_mstatush,
> write_mstatush },
>
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2463 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
2022-04-30 3:09 ` Frank Chang
@ 2022-04-30 4:29 ` Anup Patel
-1 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-30 4:29 UTC (permalink / raw)
To: Frank Chang
Cc: Anup Patel, Peter Maydell, open list:RISC-V, Sagar Karandikar,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
On Sat, Apr 30, 2022 at 8:39 AM Frank Chang <frank.chang@sifive.com> wrote:
>
> Hi Anup,
>
> If we want to limit the generated ISA string to/after a specific privilege spec version.
> Shouldn't we also check the privilege spec version when these extensions are enabled?
> Otherwise, it's possible that one extension is enabled,
> but the privilege spec version is smaller than the one in which the extension is supported.
> (This is possible if user specifies the privileged spec version through the command line.)
> The ISA string therefore won't include the enabled extension.
This patch is only a temporary fix for the sifive_u machine where I am
seeing some
of these new extensions available in ISA string.
We need a separate series to have the priv spec version influence
individual extension
enabling/disabling.
Regards,
Anup
>
> Regards,
> Frank Chang
>
>
> On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
>> etc) are only available after Priv spec v1.12 so ISA string generation
>> should check the minimum required priv spec version for all extensions.
>>
>> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
>> device tree")
>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> ---
>> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 02ee7d45d8..d8c88b96bc 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>> struct isa_ext_data {
>> const char *name;
>> bool enabled;
>> + uint32_t min_priv_ver;
>> };
>>
>> const char * const riscv_int_regnames[] = {
>> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>> device_class_set_props(dc, riscv_cpu_properties);
>> }
>>
>> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
>> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
>>
>> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>> {
>> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>> * extensions by an underscore.
>> */
>> struct isa_ext_data isa_edata_arr[] = {
>> - ISA_EDATA_ENTRY(zfh, ext_zfh),
>> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
>> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
>> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
>> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
>> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
>> - ISA_EDATA_ENTRY(zba, ext_zba),
>> - ISA_EDATA_ENTRY(zbb, ext_zbb),
>> - ISA_EDATA_ENTRY(zbc, ext_zbc),
>> - ISA_EDATA_ENTRY(zbs, ext_zbs),
>> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
>> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
>> - ISA_EDATA_ENTRY(svinval, ext_svinval),
>> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
>> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
>> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
>> };
>>
>> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>> - if (isa_edata_arr[i].enabled) {
>> + if (isa_edata_arr[i].enabled &&
>> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
>> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
>> g_free(old);
>> old = new;
>> --
>> 2.34.1
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
@ 2022-04-30 4:29 ` Anup Patel
0 siblings, 0 replies; 24+ messages in thread
From: Anup Patel @ 2022-04-30 4:29 UTC (permalink / raw)
To: Frank Chang
Cc: Anup Patel, Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Sat, Apr 30, 2022 at 8:39 AM Frank Chang <frank.chang@sifive.com> wrote:
>
> Hi Anup,
>
> If we want to limit the generated ISA string to/after a specific privilege spec version.
> Shouldn't we also check the privilege spec version when these extensions are enabled?
> Otherwise, it's possible that one extension is enabled,
> but the privilege spec version is smaller than the one in which the extension is supported.
> (This is possible if user specifies the privileged spec version through the command line.)
> The ISA string therefore won't include the enabled extension.
This patch is only a temporary fix for the sifive_u machine where I am
seeing some
of these new extensions available in ISA string.
We need a separate series to have the priv spec version influence
individual extension
enabling/disabling.
Regards,
Anup
>
> Regards,
> Frank Chang
>
>
> On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
>> etc) are only available after Priv spec v1.12 so ISA string generation
>> should check the minimum required priv spec version for all extensions.
>>
>> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
>> device tree")
>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> ---
>> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 02ee7d45d8..d8c88b96bc 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>> struct isa_ext_data {
>> const char *name;
>> bool enabled;
>> + uint32_t min_priv_ver;
>> };
>>
>> const char * const riscv_int_regnames[] = {
>> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>> device_class_set_props(dc, riscv_cpu_properties);
>> }
>>
>> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
>> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
>>
>> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>> {
>> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>> * extensions by an underscore.
>> */
>> struct isa_ext_data isa_edata_arr[] = {
>> - ISA_EDATA_ENTRY(zfh, ext_zfh),
>> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
>> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
>> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
>> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
>> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
>> - ISA_EDATA_ENTRY(zba, ext_zba),
>> - ISA_EDATA_ENTRY(zbb, ext_zbb),
>> - ISA_EDATA_ENTRY(zbc, ext_zbc),
>> - ISA_EDATA_ENTRY(zbs, ext_zbs),
>> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
>> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
>> - ISA_EDATA_ENTRY(svinval, ext_svinval),
>> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
>> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
>> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
>> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
>> };
>>
>> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>> - if (isa_edata_arr[i].enabled) {
>> + if (isa_edata_arr[i].enabled &&
>> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
>> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
>> g_free(old);
>> old = new;
>> --
>> 2.34.1
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
2022-04-30 4:29 ` Anup Patel
@ 2022-04-30 5:33 ` Frank Chang
-1 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 5:33 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Peter Maydell, open list:RISC-V, Sagar Karandikar,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 4933 bytes --]
On Sat, Apr 30, 2022 at 12:30 PM Anup Patel <anup@brainfault.org> wrote:
> On Sat, Apr 30, 2022 at 8:39 AM Frank Chang <frank.chang@sifive.com>
> wrote:
> >
> > Hi Anup,
> >
> > If we want to limit the generated ISA string to/after a specific
> privilege spec version.
> > Shouldn't we also check the privilege spec version when these extensions
> are enabled?
> > Otherwise, it's possible that one extension is enabled,
> > but the privilege spec version is smaller than the one in which the
> extension is supported.
> > (This is possible if user specifies the privileged spec version through
> the command line.)
> > The ISA string therefore won't include the enabled extension.
>
> This patch is only a temporary fix for the sifive_u machine where I am
> seeing some
> of these new extensions available in ISA string.
>
> We need a separate series to have the priv spec version influence
> individual extension
> enabling/disabling.
>
If that's the case,
Reviewed-by: Frank Chang <frank.chang@sifive.com>
>
> Regards,
> Anup
>
> >
> > Regards,
> > Frank Chang
> >
> >
> > On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
> >>
> >> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
> >> etc) are only available after Priv spec v1.12 so ISA string generation
> >> should check the minimum required priv spec version for all extensions.
> >>
> >> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> >> device tree")
> >> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >> ---
> >> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
> >> 1 file changed, 19 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 02ee7d45d8..d8c88b96bc 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] =
> "IEMAFDQCPVH";
> >> struct isa_ext_data {
> >> const char *name;
> >> bool enabled;
> >> + uint32_t min_priv_ver;
> >> };
> >>
> >> const char * const riscv_int_regnames[] = {
> >> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c,
> void *data)
> >> device_class_set_props(dc, riscv_cpu_properties);
> >> }
> >>
> >> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
> >> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
> >>
> >> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int
> max_str_len)
> >> {
> >> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> >> * extensions by an underscore.
> >> */
> >> struct isa_ext_data isa_edata_arr[] = {
> >> - ISA_EDATA_ENTRY(zfh, ext_zfh),
> >> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> >> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> >> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> >> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
> >> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
> >> - ISA_EDATA_ENTRY(zba, ext_zba),
> >> - ISA_EDATA_ENTRY(zbb, ext_zbb),
> >> - ISA_EDATA_ENTRY(zbc, ext_zbc),
> >> - ISA_EDATA_ENTRY(zbs, ext_zbs),
> >> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
> >> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> >> - ISA_EDATA_ENTRY(svinval, ext_svinval),
> >> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
> >> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> >> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
> >> };
> >>
> >> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> >> - if (isa_edata_arr[i].enabled) {
> >> + if (isa_edata_arr[i].enabled &&
> >> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
> >> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
> >> g_free(old);
> >> old = new;
> >> --
> >> 2.34.1
> >>
> >>
>
[-- Attachment #2: Type: text/html, Size: 6711 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
@ 2022-04-30 5:33 ` Frank Chang
0 siblings, 0 replies; 24+ messages in thread
From: Frank Chang @ 2022-04-30 5:33 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 4933 bytes --]
On Sat, Apr 30, 2022 at 12:30 PM Anup Patel <anup@brainfault.org> wrote:
> On Sat, Apr 30, 2022 at 8:39 AM Frank Chang <frank.chang@sifive.com>
> wrote:
> >
> > Hi Anup,
> >
> > If we want to limit the generated ISA string to/after a specific
> privilege spec version.
> > Shouldn't we also check the privilege spec version when these extensions
> are enabled?
> > Otherwise, it's possible that one extension is enabled,
> > but the privilege spec version is smaller than the one in which the
> extension is supported.
> > (This is possible if user specifies the privileged spec version through
> the command line.)
> > The ISA string therefore won't include the enabled extension.
>
> This patch is only a temporary fix for the sifive_u machine where I am
> seeing some
> of these new extensions available in ISA string.
>
> We need a separate series to have the priv spec version influence
> individual extension
> enabling/disabling.
>
If that's the case,
Reviewed-by: Frank Chang <frank.chang@sifive.com>
>
> Regards,
> Anup
>
> >
> > Regards,
> > Frank Chang
> >
> >
> > On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
> >>
> >> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
> >> etc) are only available after Priv spec v1.12 so ISA string generation
> >> should check the minimum required priv spec version for all extensions.
> >>
> >> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> >> device tree")
> >> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >> ---
> >> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
> >> 1 file changed, 19 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 02ee7d45d8..d8c88b96bc 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] =
> "IEMAFDQCPVH";
> >> struct isa_ext_data {
> >> const char *name;
> >> bool enabled;
> >> + uint32_t min_priv_ver;
> >> };
> >>
> >> const char * const riscv_int_regnames[] = {
> >> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c,
> void *data)
> >> device_class_set_props(dc, riscv_cpu_properties);
> >> }
> >>
> >> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
> >> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
> >>
> >> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int
> max_str_len)
> >> {
> >> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> >> * extensions by an underscore.
> >> */
> >> struct isa_ext_data isa_edata_arr[] = {
> >> - ISA_EDATA_ENTRY(zfh, ext_zfh),
> >> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> >> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> >> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> >> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
> >> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
> >> - ISA_EDATA_ENTRY(zba, ext_zba),
> >> - ISA_EDATA_ENTRY(zbb, ext_zbb),
> >> - ISA_EDATA_ENTRY(zbc, ext_zbc),
> >> - ISA_EDATA_ENTRY(zbs, ext_zbs),
> >> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
> >> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> >> - ISA_EDATA_ENTRY(svinval, ext_svinval),
> >> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
> >> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> >> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
> >> };
> >>
> >> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> >> - if (isa_edata_arr[i].enabled) {
> >> + if (isa_edata_arr[i].enabled &&
> >> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
> >> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
> >> g_free(old);
> >> old = new;
> >> --
> >> 2.34.1
> >>
> >>
>
[-- Attachment #2: Type: text/html, Size: 6711 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
2022-04-29 15:34 ` Anup Patel
(?)
(?)
@ 2022-05-04 9:13 ` Alistair Francis
-1 siblings, 0 replies; 24+ messages in thread
From: Alistair Francis @ 2022-05-04 9:13 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Sat, Apr 30, 2022 at 1:43 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
> when "env->priv_ver == 0" (i.e. default v1.10) because the enum
> value of priv spec v1.10 is zero.
>
> Due to above issue, the sifive_u machine will see priv spec v1.12
> instead of priv spec v1.10.
>
> To fix this issue, we set latest priv spec version (i.e. v1.12)
> for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
> spec version only when "cpu->cfg.priv_spec != NULL".
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f0a702fee6..02ee7d45d8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -169,6 +169,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -204,6 +206,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> CPURISCVState *env = &cpu->env;
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> CPUClass *cc = CPU_CLASS(mcc);
> - int priv_version = 0;
> + int priv_version = -1;
> Error *local_err = NULL;
>
> cpu_exec_realizefn(cs, &local_err);
> @@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> }
> }
>
> - if (priv_version) {
> + if (priv_version >= PRIV_VERSION_1_10_0) {
> set_priv_version(env, priv_version);
> - } else if (!env->priv_ver) {
> - set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> if (cpu->cfg.mmu) {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
2022-04-29 15:34 ` Anup Patel
(?)
(?)
@ 2022-05-04 9:14 ` Alistair Francis
-1 siblings, 0 replies; 24+ messages in thread
From: Alistair Francis @ 2022-05-04 9:14 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
On Sat, Apr 30, 2022 at 2:13 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
> implementation that don't want to implement can simply have a dummy
> mcountinhibit which always zero.
>
> Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
> the CSR ops.")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/csr.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 4d04b20d06..4a55c6a709 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -159,6 +159,9 @@
> #define CSR_MTVEC 0x305
> #define CSR_MCOUNTEREN 0x306
>
> +/* Machine Counter Setup */
> +#define CSR_MCOUNTINHIBIT 0x320
> +
> /* 32-bit only */
> #define CSR_MSTATUSH 0x310
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2bf0a97196..e144ce7135 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie },
> [CSR_MTVEC] = { "mtvec", any, read_mtvec, write_mtvec },
> [CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren, write_mcounteren },
> + [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
> + .min_priv_ver = PRIV_VERSION_1_11_0 },
>
> [CSR_MSTATUSH] = { "mstatush", any32, read_mstatush, write_mstatush },
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
2022-04-29 15:34 ` Anup Patel
` (2 preceding siblings ...)
(?)
@ 2022-05-04 9:53 ` Frank Chang
2022-05-09 19:54 ` Atish Patra
-1 siblings, 1 reply; 24+ messages in thread
From: Frank Chang @ 2022-05-04 9:53 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers, Atish Patra
[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]
Hi Anup,
I found that Atish has already submitted a patch to implement the
mcountinhibit CSR:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg879349.html
Regards,
Frank Chang
On Fri, Apr 29, 2022 at 11:44 PM Anup Patel <apatel@ventanamicro.com> wrote:
> The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
> implementation that don't want to implement can simply have a dummy
> mcountinhibit which always zero.
>
> Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
> the CSR ops.")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/csr.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 4d04b20d06..4a55c6a709 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -159,6 +159,9 @@
> #define CSR_MTVEC 0x305
> #define CSR_MCOUNTEREN 0x306
>
> +/* Machine Counter Setup */
> +#define CSR_MCOUNTINHIBIT 0x320
> +
> /* 32-bit only */
> #define CSR_MSTATUSH 0x310
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2bf0a97196..e144ce7135 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie
> },
> [CSR_MTVEC] = { "mtvec", any, read_mtvec,
> write_mtvec },
> [CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren,
> write_mcounteren },
> + [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
> + .min_priv_ver =
> PRIV_VERSION_1_11_0 },
>
> [CSR_MSTATUSH] = { "mstatush", any32, read_mstatush,
> write_mstatush },
>
> --
> 2.34.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2677 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string
2022-04-30 4:29 ` Anup Patel
(?)
(?)
@ 2022-05-04 9:59 ` Alistair Francis
-1 siblings, 0 replies; 24+ messages in thread
From: Alistair Francis @ 2022-05-04 9:59 UTC (permalink / raw)
To: Anup Patel
Cc: Frank Chang, Anup Patel, Peter Maydell, open list:RISC-V,
Sagar Karandikar, qemu-devel@nongnu.org Developers,
Alistair Francis, Atish Patra, Palmer Dabbelt
On Sat, Apr 30, 2022 at 2:31 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sat, Apr 30, 2022 at 8:39 AM Frank Chang <frank.chang@sifive.com> wrote:
> >
> > Hi Anup,
> >
> > If we want to limit the generated ISA string to/after a specific privilege spec version.
> > Shouldn't we also check the privilege spec version when these extensions are enabled?
> > Otherwise, it's possible that one extension is enabled,
> > but the privilege spec version is smaller than the one in which the extension is supported.
> > (This is possible if user specifies the privileged spec version through the command line.)
> > The ISA string therefore won't include the enabled extension.
>
> This patch is only a temporary fix for the sifive_u machine where I am
> seeing some
> of these new extensions available in ISA string.
>
> We need a separate series to have the priv spec version influence
> individual extension
> enabling/disabling.
I agree with Frank, I think it makes more sense to just never enable
the extension instead of not telling the guest it's enabled.
Alistair
>
> Regards,
> Anup
>
> >
> > Regards,
> > Frank Chang
> >
> >
> > On Fri, Apr 29, 2022 at 11:49 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >>
> >> Most of the multi-letter extensions (such as Svpbmt, Svnapot, Svinval,
> >> etc) are only available after Priv spec v1.12 so ISA string generation
> >> should check the minimum required priv spec version for all extensions.
> >>
> >> Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> >> device tree")
> >> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >> ---
> >> target/riscv/cpu.c | 36 +++++++++++++++++++-----------------
> >> 1 file changed, 19 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 02ee7d45d8..d8c88b96bc 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -44,6 +44,7 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> >> struct isa_ext_data {
> >> const char *name;
> >> bool enabled;
> >> + uint32_t min_priv_ver;
> >> };
> >>
> >> const char * const riscv_int_regnames[] = {
> >> @@ -974,7 +975,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
> >> device_class_set_props(dc, riscv_cpu_properties);
> >> }
> >>
> >> -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop}
> >> +#define ISA_EDATA_ENTRY(name, prop, priv) {#name, cpu->cfg.prop, priv}
> >>
> >> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
> >> {
> >> @@ -1000,25 +1001,26 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
> >> * extensions by an underscore.
> >> */
> >> struct isa_ext_data isa_edata_arr[] = {
> >> - ISA_EDATA_ENTRY(zfh, ext_zfh),
> >> - ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> >> - ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> >> - ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> >> - ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
> >> - ISA_EDATA_ENTRY(zdinx, ext_zdinx),
> >> - ISA_EDATA_ENTRY(zba, ext_zba),
> >> - ISA_EDATA_ENTRY(zbb, ext_zbb),
> >> - ISA_EDATA_ENTRY(zbc, ext_zbc),
> >> - ISA_EDATA_ENTRY(zbs, ext_zbs),
> >> - ISA_EDATA_ENTRY(zve32f, ext_zve32f),
> >> - ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> >> - ISA_EDATA_ENTRY(svinval, ext_svinval),
> >> - ISA_EDATA_ENTRY(svnapot, ext_svnapot),
> >> - ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> >> + ISA_EDATA_ENTRY(zfh, ext_zfh, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfhmin, ext_zfhmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zfinx, ext_zfinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinx, ext_zhinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zdinx, ext_zdinx, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zba, ext_zba, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbb, ext_zbb, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbc, ext_zbc, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zbs, ext_zbs, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve32f, ext_zve32f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(zve64f, ext_zve64f, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svinval, ext_svinval, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svnapot, ext_svnapot, PRIV_VERSION_1_12_0),
> >> + ISA_EDATA_ENTRY(svpbmt, ext_svpbmt, PRIV_VERSION_1_12_0),
> >> };
> >>
> >> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> >> - if (isa_edata_arr[i].enabled) {
> >> + if (isa_edata_arr[i].enabled &&
> >> + cpu->env.priv_ver >= isa_edata_arr[i].min_priv_ver) {
> >> new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
> >> g_free(old);
> >> old = new;
> >> --
> >> 2.34.1
> >>
> >>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher
2022-05-04 9:53 ` Frank Chang
@ 2022-05-09 19:54 ` Atish Patra
0 siblings, 0 replies; 24+ messages in thread
From: Atish Patra @ 2022-05-09 19:54 UTC (permalink / raw)
To: Frank Chang
Cc: Anup Patel, Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Wed, May 4, 2022 at 2:53 AM Frank Chang <frank.chang@sifive.com> wrote:
>
> Hi Anup,
>
> I found that Atish has already submitted a patch to implement the mcountinhibit CSR:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg879349.html
>
Yeah. I think it depends on which series is merged first. The PMU
series actually uses mcountinhibit.
However, latest OpenSBI patches detect priv version v1.11 based on
mcountinhibit presence.
We need mcountinhibit support to test any v1.12 patches anyways.
If PMU patches are merged first, we don't need this patch.
> Regards,
> Frank Chang
>
> On Fri, Apr 29, 2022 at 11:44 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> The mcountinhibit CSR is mandatory for priv spec v1.11 or higher. For
>> implementation that don't want to implement can simply have a dummy
>> mcountinhibit which always zero.
>>
>> Fixes: a4b2fa433125 ("target/riscv: Introduce privilege version field in
>> the CSR ops.")
>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> ---
>> target/riscv/cpu_bits.h | 3 +++
>> target/riscv/csr.c | 2 ++
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 4d04b20d06..4a55c6a709 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -159,6 +159,9 @@
>> #define CSR_MTVEC 0x305
>> #define CSR_MCOUNTEREN 0x306
>>
>> +/* Machine Counter Setup */
>> +#define CSR_MCOUNTINHIBIT 0x320
>> +
>> /* 32-bit only */
>> #define CSR_MSTATUSH 0x310
>>
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index 2bf0a97196..e144ce7135 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -3391,6 +3391,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>> [CSR_MIE] = { "mie", any, NULL, NULL, rmw_mie },
>> [CSR_MTVEC] = { "mtvec", any, read_mtvec, write_mtvec },
>> [CSR_MCOUNTEREN] = { "mcounteren", any, read_mcounteren, write_mcounteren },
>> + [CSR_MCOUNTINHIBIT] = { "mcountinhibit", any, read_zero, write_ignore,
>> + .min_priv_ver = PRIV_VERSION_1_11_0 },
>>
>> [CSR_MSTATUSH] = { "mstatush", any32, read_mstatush, write_mstatush },
>>
>> --
>> 2.34.1
>>
>>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] target/riscv: Don't force update priv spec version to latest
2022-04-29 15:34 ` Anup Patel
` (2 preceding siblings ...)
(?)
@ 2022-05-09 20:02 ` Atish Patra
-1 siblings, 0 replies; 24+ messages in thread
From: Atish Patra @ 2022-05-09 20:02 UTC (permalink / raw)
To: Anup Patel
Cc: Peter Maydell, Palmer Dabbelt, Alistair Francis,
Sagar Karandikar, Anup Patel, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Fri, Apr 29, 2022 at 8:35 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The riscv_cpu_realize() sets priv spec verion to v1.12 when it is
> when "env->priv_ver == 0" (i.e. default v1.10) because the enum
> value of priv spec v1.10 is zero.
>
> Due to above issue, the sifive_u machine will see priv spec v1.12
> instead of priv spec v1.10.
>
> To fix this issue, we set latest priv spec version (i.e. v1.12)
> for base rv64/rv32 cpu and riscv_cpu_realize() will override priv
> spec version only when "cpu->cfg.priv_spec != NULL".
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> target/riscv/cpu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f0a702fee6..02ee7d45d8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -169,6 +169,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -204,6 +206,8 @@ 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);
> + /* Set latest version of privileged specification */
> + set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -509,7 +513,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> CPURISCVState *env = &cpu->env;
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> CPUClass *cc = CPU_CLASS(mcc);
> - int priv_version = 0;
> + int priv_version = -1;
> Error *local_err = NULL;
>
> cpu_exec_realizefn(cs, &local_err);
> @@ -533,10 +537,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> }
> }
>
> - if (priv_version) {
> + if (priv_version >= PRIV_VERSION_1_10_0) {
> set_priv_version(env, priv_version);
> - } else if (!env->priv_ver) {
> - set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> if (cpu->cfg.mmu) {
> --
> 2.34.1
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2022-05-09 20:04 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 15:34 [PATCH 0/3] QEMU RISC-V priv spec version fixes Anup Patel
2022-04-29 15:34 ` Anup Patel
2022-04-29 15:34 ` [PATCH 1/3] target/riscv: Don't force update priv spec version to latest Anup Patel
2022-04-29 15:34 ` Anup Patel
2022-04-30 2:57 ` Frank Chang
2022-04-30 2:57 ` Frank Chang
2022-05-04 9:13 ` Alistair Francis
2022-05-09 20:02 ` Atish Patra
2022-04-29 15:34 ` [PATCH 2/3] target/riscv: Add dummy mcountinhibit CSR for priv spec v1.11 or higher Anup Patel
2022-04-29 15:34 ` Anup Patel
2022-04-30 3:13 ` Frank Chang
2022-04-30 3:13 ` Frank Chang
2022-05-04 9:14 ` Alistair Francis
2022-05-04 9:53 ` Frank Chang
2022-05-09 19:54 ` Atish Patra
2022-04-29 15:34 ` [PATCH 3/3] target/riscv: Consider priv spec version when generating ISA string Anup Patel
2022-04-29 15:34 ` Anup Patel
2022-04-30 3:09 ` Frank Chang
2022-04-30 3:09 ` Frank Chang
2022-04-30 4:29 ` Anup Patel
2022-04-30 4:29 ` Anup Patel
2022-04-30 5:33 ` Frank Chang
2022-04-30 5:33 ` Frank Chang
2022-05-04 9:59 ` 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.