* [PATCH v9 0/5] riscv: Allow user to set the satp mode
@ 2023-01-31 13:39 Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti
This introduces new properties to allow the user to set the satp mode,
see patch 3 for full syntax. In addition, it prevents cpus to boot in a
satp mode they do not support (see patch 4).
v9:
- Move valid_vm[i] up, Andrew
- Fixed expansion of the bitmap map, Bin
- Rename set_satp_mode_default into set_satp_mode_default_map, Bin
- Remove outer parenthesis and alignment, Bin
- Fix qemu32 build failure, Bin
- Fixed a few typos, Bin
- Add RB from Andrew and Bin
v8:
- Remove useless !map check, Andrew
- Add RB from Andrew
v7:
- Expand map to contain all valid modes, Andrew
- Fix commit log for patch 3, Andrew
- Remove is_32_bit argument from set_satp_mode_default, Andrew
- Move and fixed comment, Andrew
- Fix satp_mode_map_max in riscv_cpu_satp_mode_finalize which was set
too early, Alex
- Remove is_32_bit argument from set_satp_mode_max_supported, Andrew
- Use satp_mode directly instead of a string in
set_satp_mode_max_supported, Andrew
- Swap the patch introducing supported bitmap and the patch that sets
sv57 in the dt, Andrew
- Add various RB from Andrew and Alistair, thanks
v6:
- Remove the valid_vm check in validate_vm and add it to the finalize function
so that map already contains the constraint, Alex
- Add forgotten mbare to satp_mode_from_str, Alex
- Move satp mode properties handling to riscv_cpu_satp_mode_finalize, Andrew
- Only add satp mode properties corresponding to the cpu, and then remove the
check against valid_vm_1_10_32/64 in riscv_cpu_satp_mode_finalize,
Andrew/Alistair/Alex
- Move mmu-type setting to its own patch, Andrew
- patch 5 is new and is a fix, Alex
v5:
- Simplify v4 implementation by leveraging valid_vm_1_10_32/64, as
suggested by Andrew
- Split the v4 patch into 2 patches as suggested by Andrew
- Lot of other minor corrections, from Andrew
- Set the satp mode N by disabling the satp mode N + 1
- Add a helper to set satp mode from a string, as suggested by Frank
v4:
- Use custom boolean properties instead of OnOffAuto properties, based
on ARMVQMap, as suggested by Andrew
v3:
- Free sv_name as pointed by Bin
- Replace satp-mode with boolean properties as suggested by Andrew
- Removed RB from Atish as the patch considerably changed
v2:
- Use error_setg + return as suggested by Alistair
- Add RB from Atish
- Fixed checkpatch issues missed in v1
- Replaced Ludovic email address with the rivos one
Alexandre Ghiti (5):
riscv: Pass Object to register_cpu_props instead of DeviceState
riscv: Change type of valid_vm_1_10_[32|64] to bool
riscv: Allow user to set the satp mode
riscv: Introduce satp mode hw capabilities
riscv: Correctly set the device-tree entry 'mmu-type'
hw/riscv/virt.c | 19 ++--
target/riscv/cpu.c | 251 +++++++++++++++++++++++++++++++++++++++++++--
target/riscv/cpu.h | 23 +++++
target/riscv/csr.c | 29 +++---
4 files changed, 291 insertions(+), 31 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v9 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-01-31 13:39 ` Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti, Bin Meng
One can extract the DeviceState pointer from the Object pointer, so pass
the Object for future commits to access other fields of Object.
No functional changes intended.
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/cpu.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..7181b34f86 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -200,7 +200,7 @@ static const char * const riscv_intr_names[] = {
"reserved"
};
-static void register_cpu_props(DeviceState *dev);
+static void register_cpu_props(Object *obj);
const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
{
@@ -238,7 +238,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));
+ register_cpu_props(obj);
}
#if defined(TARGET_RISCV64)
@@ -247,7 +247,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));
+ register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -280,7 +280,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));
+ register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -290,7 +290,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));
+ register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -343,7 +343,7 @@ static void riscv_host_cpu_init(Object *obj)
#elif defined(TARGET_RISCV64)
set_misa(env, MXL_RV64, 0);
#endif
- register_cpu_props(DEVICE(obj));
+ register_cpu_props(obj);
}
#endif
@@ -1083,9 +1083,10 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void register_cpu_props(DeviceState *dev)
+static void register_cpu_props(Object *obj)
{
Property *prop;
+ DeviceState *dev = DEVICE(obj);
for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
qdev_property_add_static(dev, prop);
--
2.37.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
@ 2023-01-31 13:39 ` Alexandre Ghiti
2023-02-01 13:21 ` Frank Chang
2023-01-31 13:39 ` [PATCH v9 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti, Bin Meng
This array is actually used as a boolean so swap its current char type
to a boolean and at the same time, change the type of validate_vm to
bool since it returns valid_vm_1_10_[32|64].
Suggested-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0db2c233e5..6b157806a5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1117,16 +1117,16 @@ static const target_ulong hip_writable_mask = MIP_VSSIP;
static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
static const target_ulong vsip_writable_mask = MIP_VSSIP;
-static const char valid_vm_1_10_32[16] = {
- [VM_1_10_MBARE] = 1,
- [VM_1_10_SV32] = 1
+static const bool valid_vm_1_10_32[16] = {
+ [VM_1_10_MBARE] = true,
+ [VM_1_10_SV32] = true
};
-static const char valid_vm_1_10_64[16] = {
- [VM_1_10_MBARE] = 1,
- [VM_1_10_SV39] = 1,
- [VM_1_10_SV48] = 1,
- [VM_1_10_SV57] = 1
+static const bool valid_vm_1_10_64[16] = {
+ [VM_1_10_MBARE] = true,
+ [VM_1_10_SV39] = true,
+ [VM_1_10_SV48] = true,
+ [VM_1_10_SV57] = true
};
/* Machine Information Registers */
@@ -1209,7 +1209,7 @@ static RISCVException read_mstatus(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
-static int validate_vm(CPURISCVState *env, target_ulong vm)
+static bool validate_vm(CPURISCVState *env, target_ulong vm)
{
if (riscv_cpu_mxl(env) == MXL_RV32) {
return valid_vm_1_10_32[vm & 0xf];
@@ -2648,7 +2648,8 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
static RISCVException write_satp(CPURISCVState *env, int csrno,
target_ulong val)
{
- target_ulong vm, mask;
+ target_ulong mask;
+ bool vm;
if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
return RISCV_EXCP_NONE;
--
2.37.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v9 3/5] riscv: Allow user to set the satp mode
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
@ 2023-01-31 13:39 ` Alexandre Ghiti
2023-02-01 4:17 ` Bin Meng
2023-02-02 0:48 ` Alistair Francis
2023-01-31 13:39 ` [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
4 siblings, 2 replies; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti, Ludovic Henry
RISC-V specifies multiple sizes for addressable memory and Linux probes for
the machine's support at startup via the satp CSR register (done in
csr.c:validate_vm).
As per the specification, sv64 must support sv57, which in turn must
support sv48...etc. So we can restrict machine support by simply setting the
"highest" supported mode and the bare mode is always supported.
You can set the satp mode using the new properties "sv32", "sv39", "sv48",
"sv57" and "sv64" as follows:
-cpu rv64,sv57=on # Linux will boot using sv57 scheme
-cpu rv64,sv39=on # Linux will boot using sv39 scheme
-cpu rv64,sv57=off # Linux will boot using sv48 scheme
-cpu rv64 # Linux will boot using sv57 scheme by default
We take the highest level set by the user:
-cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme
We make sure that invalid configurations are rejected:
-cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are
# enabled
We accept "redundant" configurations:
-cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme
And contradictory configurations:
-cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme
Co-Developed-by: Ludovic Henry <ludovic@rivosinc.com>
Signed-off-by: Ludovic Henry <ludovic@rivosinc.com>
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
target/riscv/cpu.c | 207 +++++++++++++++++++++++++++++++++++++++++++++
target/riscv/cpu.h | 19 +++++
target/riscv/csr.c | 12 ++-
3 files changed, 231 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7181b34f86..3a7a1746aa 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -27,6 +27,7 @@
#include "time_helper.h"
#include "exec/exec-all.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "qemu/error-report.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
@@ -229,6 +230,81 @@ static void set_vext_version(CPURISCVState *env, int vext_ver)
env->vext_ver = vext_ver;
}
+static uint8_t satp_mode_from_str(const char *satp_mode_str)
+{
+ if (!strncmp(satp_mode_str, "mbare", 5)) {
+ return VM_1_10_MBARE;
+ }
+
+ if (!strncmp(satp_mode_str, "sv32", 4)) {
+ return VM_1_10_SV32;
+ }
+
+ if (!strncmp(satp_mode_str, "sv39", 4)) {
+ return VM_1_10_SV39;
+ }
+
+ if (!strncmp(satp_mode_str, "sv48", 4)) {
+ return VM_1_10_SV48;
+ }
+
+ if (!strncmp(satp_mode_str, "sv57", 4)) {
+ return VM_1_10_SV57;
+ }
+
+ if (!strncmp(satp_mode_str, "sv64", 4)) {
+ return VM_1_10_SV64;
+ }
+
+ g_assert_not_reached();
+}
+
+uint8_t satp_mode_max_from_map(uint32_t map)
+{
+ /* map here has at least one bit set, so no problem with clz */
+ return 31 - __builtin_clz(map);
+}
+
+const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
+{
+ if (is_32_bit) {
+ switch (satp_mode) {
+ case VM_1_10_SV32:
+ return "sv32";
+ case VM_1_10_MBARE:
+ return "none";
+ }
+ } else {
+ switch (satp_mode) {
+ case VM_1_10_SV64:
+ return "sv64";
+ case VM_1_10_SV57:
+ return "sv57";
+ case VM_1_10_SV48:
+ return "sv48";
+ case VM_1_10_SV39:
+ return "sv39";
+ case VM_1_10_MBARE:
+ return "none";
+ }
+ }
+
+ g_assert_not_reached();
+}
+
+/* Sets the satp mode to the max supported */
+static void set_satp_mode_default_map(RISCVCPU *cpu)
+{
+ bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
+
+ if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
+ cpu->cfg.satp_mode.map |=
+ (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
+ } else {
+ cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
+ }
+}
+
static void riscv_any_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -619,6 +695,83 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
}
}
+static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
+{
+ bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
+ const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
+ uint8_t satp_mode_max;
+
+ if (cpu->cfg.satp_mode.map == 0) {
+ if (cpu->cfg.satp_mode.init == 0) {
+ /* If unset by the user, we fallback to the default satp mode. */
+ set_satp_mode_default_map(cpu);
+ } else {
+ /*
+ * Find the lowest level that was disabled and then enable the
+ * first valid level below which can be found in
+ * valid_vm_1_10_32/64.
+ */
+ for (int i = 1; i < 16; ++i) {
+ if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
+ for (int j = i - 1; j >= 0; --j) {
+ if (valid_vm[j]) {
+ cpu->cfg.satp_mode.map |= (1 << j);
+ break;
+ }
+ }
+ break;
+ }
+ }
+ }
+ }
+
+ /* Make sure the configuration asked is supported by qemu */
+ for (int i = 0; i < 16; ++i) {
+ if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
+ error_setg(errp, "satp_mode %s is not valid",
+ satp_mode_str(i, rv32));
+ return;
+ }
+ }
+
+ /*
+ * Make sure the user did not ask for an invalid configuration as per
+ * the specification.
+ */
+ satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
+
+ if (!rv32) {
+ for (int i = satp_mode_max - 1; i >= 0; --i) {
+ if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
+ (cpu->cfg.satp_mode.init & (1 << i)) &&
+ valid_vm[i]) {
+ error_setg(errp, "cannot disable %s satp mode if %s "
+ "is enabled", satp_mode_str(i, false),
+ satp_mode_str(satp_mode_max, false));
+ return;
+ }
+ }
+ }
+
+ /* Finally expand the map so that all valid modes are set */
+ for (int i = satp_mode_max - 1; i >= 0; --i) {
+ if (valid_vm[i]) {
+ cpu->cfg.satp_mode.map |= (1 << i);
+ }
+ }
+}
+
+static void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
+{
+ Error *local_err = NULL;
+
+ riscv_cpu_satp_mode_finalize(cpu, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
+}
+
static void riscv_cpu_realize(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
@@ -919,6 +1072,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
}
#endif
+ riscv_cpu_finalize_features(cpu, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
riscv_cpu_register_gdb_regs_for_features(cs);
qemu_init_vcpu(cs);
@@ -927,6 +1086,52 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
mcc->parent_realize(dev, errp);
}
+static void cpu_riscv_get_satp(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVSATPMap *satp_map = opaque;
+ uint8_t satp = satp_mode_from_str(name);
+ bool value;
+
+ value = satp_map->map & (1 << satp);
+
+ visit_type_bool(v, name, &value, errp);
+}
+
+static void cpu_riscv_set_satp(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVSATPMap *satp_map = opaque;
+ uint8_t satp = satp_mode_from_str(name);
+ bool value;
+
+ if (!visit_type_bool(v, name, &value, errp)) {
+ return;
+ }
+
+ satp_map->map = deposit32(satp_map->map, satp, 1, value);
+ satp_map->init |= 1 << satp;
+}
+
+static void riscv_add_satp_mode_properties(Object *obj)
+{
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
+ if (cpu->env.misa_mxl == MXL_RV32) {
+ object_property_add(obj, "sv32", "bool", cpu_riscv_get_satp,
+ cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
+ } else {
+ object_property_add(obj, "sv39", "bool", cpu_riscv_get_satp,
+ cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
+ object_property_add(obj, "sv48", "bool", cpu_riscv_get_satp,
+ cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
+ object_property_add(obj, "sv57", "bool", cpu_riscv_get_satp,
+ cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
+ object_property_add(obj, "sv64", "bool", cpu_riscv_get_satp,
+ cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
+ }
+}
+
#ifndef CONFIG_USER_ONLY
static void riscv_cpu_set_irq(void *opaque, int irq, int level)
{
@@ -1091,6 +1296,8 @@ static void register_cpu_props(Object *obj)
for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
qdev_property_add_static(dev, prop);
}
+
+ riscv_add_satp_mode_properties(obj);
}
static Property riscv_cpu_properties[] = {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..e37177db5c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -27,6 +27,7 @@
#include "qom/object.h"
#include "qemu/int128.h"
#include "cpu_bits.h"
+#include "qapi/qapi-types-common.h"
#define TCG_GUEST_DEFAULT_MO 0
@@ -413,6 +414,17 @@ struct RISCVCPUClass {
ResettablePhases parent_phases;
};
+/*
+ * map is a 16-bit bitmap: the most significant set bit in map is the maximum
+ * satp mode that is supported.
+ *
+ * init is a 16-bit bitmap used to make sure the user selected a correct
+ * configuration as per the specification.
+ */
+typedef struct {
+ uint16_t map, init;
+} RISCVSATPMap;
+
struct RISCVCPUConfig {
bool ext_i;
bool ext_e;
@@ -488,6 +500,8 @@ struct RISCVCPUConfig {
bool debug;
bool short_isa_string;
+
+ RISCVSATPMap satp_mode;
};
typedef struct RISCVCPUConfig RISCVCPUConfig;
@@ -794,9 +808,14 @@ enum riscv_pmu_event_idx {
/* CSR function table */
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
+extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
+
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
+uint8_t satp_mode_max_from_map(uint32_t map);
+const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6b157806a5..f9eff3f1e3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1117,12 +1117,12 @@ static const target_ulong hip_writable_mask = MIP_VSSIP;
static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
static const target_ulong vsip_writable_mask = MIP_VSSIP;
-static const bool valid_vm_1_10_32[16] = {
+const bool valid_vm_1_10_32[16] = {
[VM_1_10_MBARE] = true,
[VM_1_10_SV32] = true
};
-static const bool valid_vm_1_10_64[16] = {
+const bool valid_vm_1_10_64[16] = {
[VM_1_10_MBARE] = true,
[VM_1_10_SV39] = true,
[VM_1_10_SV48] = true,
@@ -1211,11 +1211,9 @@ static RISCVException read_mstatus(CPURISCVState *env, int csrno,
static bool validate_vm(CPURISCVState *env, target_ulong vm)
{
- if (riscv_cpu_mxl(env) == MXL_RV32) {
- return valid_vm_1_10_32[vm & 0xf];
- } else {
- return valid_vm_1_10_64[vm & 0xf];
- }
+ RISCVCPU *cpu = RISCV_CPU(env_cpu(env));
+
+ return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
}
static RISCVException write_mstatus(CPURISCVState *env, int csrno,
--
2.37.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
` (2 preceding siblings ...)
2023-01-31 13:39 ` [PATCH v9 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-01-31 13:39 ` Alexandre Ghiti
2023-02-01 4:21 ` Bin Meng
2023-02-01 15:49 ` Frank Chang
2023-01-31 13:39 ` [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
4 siblings, 2 replies; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti
Currently, the max satp mode is set with the only constraint that it must be
implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
But we actually need to add another level of constraint: what the hw is
actually capable of, because currently, a linux booting on a sifive-u54
boots in sv57 mode which is incompatible with the cpu's sv39 max
capability.
So add a new bitmap to RISCVSATPMap which contains this capability and
initialize it in every XXX_cpu_init.
Finally:
- valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
- the CPU hw capabilities constrains what the user may select
- the user's selection then constrains what's available to the guest
OS.
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
target/riscv/cpu.c | 79 +++++++++++++++++++++++++++++++---------------
target/riscv/cpu.h | 8 +++--
2 files changed, 60 insertions(+), 27 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3a7a1746aa..6dd76355ec 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -292,26 +292,36 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
g_assert_not_reached();
}
-/* Sets the satp mode to the max supported */
-static void set_satp_mode_default_map(RISCVCPU *cpu)
+static void set_satp_mode_max_supported(RISCVCPU *cpu,
+ uint8_t satp_mode)
{
bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
+ const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
- if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
- cpu->cfg.satp_mode.map |=
- (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
- } else {
- cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
+ for (int i = 0; i <= satp_mode; ++i) {
+ if (valid_vm[i]) {
+ cpu->cfg.satp_mode.supported |= (1 << i);
+ }
}
}
+/* Set the satp mode to the max supported */
+static void set_satp_mode_default_map(RISCVCPU *cpu)
+{
+ cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
+}
+
static void riscv_any_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
#if defined(TARGET_RISCV32)
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV32);
#elif defined(TARGET_RISCV64)
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV57);
#endif
set_priv_version(env, PRIV_VERSION_1_12_0);
register_cpu_props(obj);
@@ -321,18 +331,24 @@ static void riscv_any_cpu_init(Object *obj)
static void rv64_base_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
/* We set this in the realise function */
set_misa(env, MXL_RV64, 0);
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV57);
}
static void rv64_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV39);
}
static void rv64_sifive_e_cpu_init(Object *obj)
@@ -343,6 +359,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
cpu->cfg.mmu = false;
+ set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
}
static void rv128_base_cpu_init(Object *obj)
@@ -354,28 +371,36 @@ static void rv128_base_cpu_init(Object *obj)
exit(EXIT_FAILURE);
}
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
/* We set this in the realise function */
set_misa(env, MXL_RV128, 0);
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV57);
}
#else
static void rv32_base_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
/* We set this in the realise function */
set_misa(env, MXL_RV32, 0);
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV32);
}
static void rv32_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+ RISCVCPU *cpu = RISCV_CPU(obj);
+
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
+ set_satp_mode_max_supported(cpu, VM_1_10_SV32);
}
static void rv32_sifive_e_cpu_init(Object *obj)
@@ -386,6 +411,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
cpu->cfg.mmu = false;
+ set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
}
static void rv32_ibex_cpu_init(Object *obj)
@@ -396,6 +422,7 @@ static void rv32_ibex_cpu_init(Object *obj)
set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
set_priv_version(env, PRIV_VERSION_1_11_0);
cpu->cfg.mmu = false;
+ set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
cpu->cfg.epmp = true;
}
@@ -407,6 +434,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
cpu->cfg.mmu = false;
+ set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
}
#endif
@@ -698,8 +726,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
{
bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
- const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
- uint8_t satp_mode_max;
+ uint8_t satp_mode_map_max;
+ uint8_t satp_mode_supported_max =
+ satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
if (cpu->cfg.satp_mode.map == 0) {
if (cpu->cfg.satp_mode.init == 0) {
@@ -712,9 +741,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
* valid_vm_1_10_32/64.
*/
for (int i = 1; i < 16; ++i) {
- if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
+ if ((cpu->cfg.satp_mode.init & (1 << i)) &&
+ (cpu->cfg.satp_mode.supported & (1 << i))) {
for (int j = i - 1; j >= 0; --j) {
- if (valid_vm[j]) {
+ if (cpu->cfg.satp_mode.supported & (1 << j)) {
cpu->cfg.satp_mode.map |= (1 << j);
break;
}
@@ -725,37 +755,36 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
}
}
- /* Make sure the configuration asked is supported by qemu */
- for (int i = 0; i < 16; ++i) {
- if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
- error_setg(errp, "satp_mode %s is not valid",
- satp_mode_str(i, rv32));
- return;
- }
+ satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
+
+ /* Make sure the user asked for a supported configuration (HW and qemu) */
+ if (satp_mode_map_max > satp_mode_supported_max) {
+ error_setg(errp, "satp_mode %s is higher than hw max capability %s",
+ satp_mode_str(satp_mode_map_max, rv32),
+ satp_mode_str(satp_mode_supported_max, rv32));
+ return;
}
/*
* Make sure the user did not ask for an invalid configuration as per
* the specification.
*/
- satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
-
if (!rv32) {
- for (int i = satp_mode_max - 1; i >= 0; --i) {
+ for (int i = satp_mode_map_max - 1; i >= 0; --i) {
if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
(cpu->cfg.satp_mode.init & (1 << i)) &&
- valid_vm[i]) {
+ (cpu->cfg.satp_mode.supported & (1 << i))) {
error_setg(errp, "cannot disable %s satp mode if %s "
"is enabled", satp_mode_str(i, false),
- satp_mode_str(satp_mode_max, false));
+ satp_mode_str(satp_mode_map_max, false));
return;
}
}
}
/* Finally expand the map so that all valid modes are set */
- for (int i = satp_mode_max - 1; i >= 0; --i) {
- if (valid_vm[i]) {
+ for (int i = satp_mode_map_max - 1; i >= 0; --i) {
+ if (cpu->cfg.satp_mode.supported & (1 << i)) {
cpu->cfg.satp_mode.map |= (1 << i);
}
}
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e37177db5c..b591122099 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -416,13 +416,17 @@ struct RISCVCPUClass {
/*
* map is a 16-bit bitmap: the most significant set bit in map is the maximum
- * satp mode that is supported.
+ * satp mode that is supported. It may be chosen by the user and must respect
+ * what qemu implements (valid_1_10_32/64) and what the hw is capable of
+ * (supported bitmap below).
*
* init is a 16-bit bitmap used to make sure the user selected a correct
* configuration as per the specification.
+ *
+ * supported is a 16-bit bitmap used to reflect the hw capabilities.
*/
typedef struct {
- uint16_t map, init;
+ uint16_t map, init, supported;
} RISCVSATPMap;
struct RISCVCPUConfig {
--
2.37.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type'
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
` (3 preceding siblings ...)
2023-01-31 13:39 ` [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
@ 2023-01-31 13:39 ` Alexandre Ghiti
2023-02-01 13:23 ` Frank Chang
4 siblings, 1 reply; 14+ messages in thread
From: Alexandre Ghiti @ 2023-01-31 13:39 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
Cc: Alexandre Ghiti, Bin Meng
The 'mmu-type' should reflect what the hardware is capable of so use the
new satp_mode field in RISCVCPUConfig to do that.
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
---
hw/riscv/virt.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 94ff2a1584..48d034a5f7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -228,7 +228,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
int cpu;
uint32_t cpu_phandle;
MachineState *mc = MACHINE(s);
- char *name, *cpu_name, *core_name, *intc_name;
+ uint8_t satp_mode_max;
+ char *name, *cpu_name, *core_name, *intc_name, *sv_name;
for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
cpu_phandle = (*phandle)++;
@@ -236,14 +237,14 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
cpu_name = g_strdup_printf("/cpus/cpu@%d",
s->soc[socket].hartid_base + cpu);
qemu_fdt_add_subnode(mc->fdt, cpu_name);
- if (riscv_feature(&s->soc[socket].harts[cpu].env,
- RISCV_FEATURE_MMU)) {
- qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
- (is_32_bit) ? "riscv,sv32" : "riscv,sv48");
- } else {
- qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
- "riscv,none");
- }
+
+ satp_mode_max = satp_mode_max_from_map(
+ s->soc[socket].harts[cpu].cfg.satp_mode.map);
+ sv_name = g_strdup_printf("riscv,%s",
+ satp_mode_str(satp_mode_max, is_32_bit));
+ qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", sv_name);
+ g_free(sv_name);
+
name = riscv_isa_string(&s->soc[socket].harts[cpu]);
qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
g_free(name);
--
2.37.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v9 3/5] riscv: Allow user to set the satp mode
2023-01-31 13:39 ` [PATCH v9 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-02-01 4:17 ` Bin Meng
2023-02-02 0:48 ` Alistair Francis
1 sibling, 0 replies; 14+ messages in thread
From: Bin Meng @ 2023-02-01 4:17 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel, Ludovic Henry
On Tue, Jan 31, 2023 at 11:13 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> RISC-V specifies multiple sizes for addressable memory and Linux probes for
> the machine's support at startup via the satp CSR register (done in
> csr.c:validate_vm).
>
> As per the specification, sv64 must support sv57, which in turn must
> support sv48...etc. So we can restrict machine support by simply setting the
> "highest" supported mode and the bare mode is always supported.
>
> You can set the satp mode using the new properties "sv32", "sv39", "sv48",
> "sv57" and "sv64" as follows:
> -cpu rv64,sv57=on # Linux will boot using sv57 scheme
> -cpu rv64,sv39=on # Linux will boot using sv39 scheme
> -cpu rv64,sv57=off # Linux will boot using sv48 scheme
> -cpu rv64 # Linux will boot using sv57 scheme by default
>
> We take the highest level set by the user:
> -cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme
>
> We make sure that invalid configurations are rejected:
> -cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are
> # enabled
>
> We accept "redundant" configurations:
> -cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme
>
> And contradictory configurations:
> -cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme
>
> Co-Developed-by: Ludovic Henry <ludovic@rivosinc.com>
> Signed-off-by: Ludovic Henry <ludovic@rivosinc.com>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> target/riscv/cpu.c | 207 +++++++++++++++++++++++++++++++++++++++++++++
> target/riscv/cpu.h | 19 +++++
> target/riscv/csr.c | 12 ++-
> 3 files changed, 231 insertions(+), 7 deletions(-)
>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities
2023-01-31 13:39 ` [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
@ 2023-02-01 4:21 ` Bin Meng
2023-02-01 15:49 ` Frank Chang
1 sibling, 0 replies; 14+ messages in thread
From: Bin Meng @ 2023-02-01 4:21 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
On Tue, Jan 31, 2023 at 10:41 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> Currently, the max satp mode is set with the only constraint that it must be
> implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
>
> But we actually need to add another level of constraint: what the hw is
> actually capable of, because currently, a linux booting on a sifive-u54
> boots in sv57 mode which is incompatible with the cpu's sv39 max
> capability.
>
> So add a new bitmap to RISCVSATPMap which contains this capability and
> initialize it in every XXX_cpu_init.
>
> Finally:
> - valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
> - the CPU hw capabilities constrains what the user may select
> - the user's selection then constrains what's available to the guest
> OS.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> target/riscv/cpu.c | 79 +++++++++++++++++++++++++++++++---------------
> target/riscv/cpu.h | 8 +++--
> 2 files changed, 60 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3a7a1746aa..6dd76355ec 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -292,26 +292,36 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
> g_assert_not_reached();
> }
>
> -/* Sets the satp mode to the max supported */
> -static void set_satp_mode_default_map(RISCVCPU *cpu)
> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> + uint8_t satp_mode)
> {
> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> + const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
>
> - if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> - cpu->cfg.satp_mode.map |=
> - (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
> - } else {
> - cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> + for (int i = 0; i <= satp_mode; ++i) {
> + if (valid_vm[i]) {
> + cpu->cfg.satp_mode.supported |= (1 << i);
> + }
> }
> }
>
> +/* Set the satp mode to the max supported */
> +static void set_satp_mode_default_map(RISCVCPU *cpu)
> +{
> + cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
> +}
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> #if defined(TARGET_RISCV32)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> #elif defined(TARGET_RISCV64)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> #endif
> set_priv_version(env, PRIV_VERSION_1_12_0);
> register_cpu_props(obj);
> @@ -321,18 +331,24 @@ static void riscv_any_cpu_init(Object *obj)
> static void rv64_base_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> /* We set this in the realise function */
> set_misa(env, MXL_RV64, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> }
>
> static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -343,6 +359,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -354,28 +371,36 @@ static void rv128_base_cpu_init(Object *obj)
> exit(EXIT_FAILURE);
> }
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
nits: for consistency with other cpu_init, needs a blank line after this
> /* We set this in the realise function */
> set_misa(env, MXL_RV128, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> }
> #else
> static void rv32_base_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> /* We set this in the realise function */
> set_misa(env, MXL_RV32, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> }
>
> static void rv32_sifive_e_cpu_init(Object *obj)
> @@ -386,6 +411,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
>
> static void rv32_ibex_cpu_init(Object *obj)
> @@ -396,6 +422,7 @@ static void rv32_ibex_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_11_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> cpu->cfg.epmp = true;
> }
>
> @@ -407,6 +434,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
> #endif
>
> @@ -698,8 +726,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> {
> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> - const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> - uint8_t satp_mode_max;
> + uint8_t satp_mode_map_max;
> + uint8_t satp_mode_supported_max =
> + satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
>
> if (cpu->cfg.satp_mode.map == 0) {
> if (cpu->cfg.satp_mode.init == 0) {
> @@ -712,9 +741,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> * valid_vm_1_10_32/64.
> */
> for (int i = 1; i < 16; ++i) {
> - if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
> + if ((cpu->cfg.satp_mode.init & (1 << i)) &&
> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> for (int j = i - 1; j >= 0; --j) {
> - if (valid_vm[j]) {
> + if (cpu->cfg.satp_mode.supported & (1 << j)) {
> cpu->cfg.satp_mode.map |= (1 << j);
> break;
> }
> @@ -725,37 +755,36 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> }
> }
>
> - /* Make sure the configuration asked is supported by qemu */
> - for (int i = 0; i < 16; ++i) {
> - if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> - error_setg(errp, "satp_mode %s is not valid",
> - satp_mode_str(i, rv32));
> - return;
> - }
> + satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> +
> + /* Make sure the user asked for a supported configuration (HW and qemu) */
> + if (satp_mode_map_max > satp_mode_supported_max) {
> + error_setg(errp, "satp_mode %s is higher than hw max capability %s",
> + satp_mode_str(satp_mode_map_max, rv32),
> + satp_mode_str(satp_mode_supported_max, rv32));
> + return;
> }
>
> /*
> * Make sure the user did not ask for an invalid configuration as per
> * the specification.
> */
> - satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> -
> if (!rv32) {
> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
> (cpu->cfg.satp_mode.init & (1 << i)) &&
> - valid_vm[i]) {
> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> error_setg(errp, "cannot disable %s satp mode if %s "
> "is enabled", satp_mode_str(i, false),
> - satp_mode_str(satp_mode_max, false));
> + satp_mode_str(satp_mode_map_max, false));
> return;
> }
> }
> }
>
> /* Finally expand the map so that all valid modes are set */
> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> - if (valid_vm[i]) {
> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> + if (cpu->cfg.satp_mode.supported & (1 << i)) {
> cpu->cfg.satp_mode.map |= (1 << i);
> }
> }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e37177db5c..b591122099 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -416,13 +416,17 @@ struct RISCVCPUClass {
>
> /*
> * map is a 16-bit bitmap: the most significant set bit in map is the maximum
> - * satp mode that is supported.
> + * satp mode that is supported. It may be chosen by the user and must respect
> + * what qemu implements (valid_1_10_32/64) and what the hw is capable of
> + * (supported bitmap below).
> *
> * init is a 16-bit bitmap used to make sure the user selected a correct
> * configuration as per the specification.
> + *
> + * supported is a 16-bit bitmap used to reflect the hw capabilities.
> */
> typedef struct {
> - uint16_t map, init;
> + uint16_t map, init, supported;
> } RISCVSATPMap;
>
> struct RISCVCPUConfig {
> --
Otherwise,
Reviewed-by: Bin Meng <bmeng@tinylab.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool
2023-01-31 13:39 ` [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
@ 2023-02-01 13:21 ` Frank Chang
0 siblings, 0 replies; 14+ messages in thread
From: Frank Chang @ 2023-02-01 13:21 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel, Bin Meng
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Tue, Jan 31, 2023 at 10:29 PM Alexandre Ghiti <alexghiti@rivosinc.com>
wrote:
> This array is actually used as a boolean so swap its current char type
> to a boolean and at the same time, change the type of validate_vm to
> bool since it returns valid_vm_1_10_[32|64].
>
> Suggested-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>
> ---
> target/riscv/csr.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0db2c233e5..6b157806a5 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1117,16 +1117,16 @@ static const target_ulong hip_writable_mask =
> MIP_VSSIP;
> static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
> MIP_VSEIP;
> static const target_ulong vsip_writable_mask = MIP_VSSIP;
>
> -static const char valid_vm_1_10_32[16] = {
> - [VM_1_10_MBARE] = 1,
> - [VM_1_10_SV32] = 1
> +static const bool valid_vm_1_10_32[16] = {
> + [VM_1_10_MBARE] = true,
> + [VM_1_10_SV32] = true
> };
>
> -static const char valid_vm_1_10_64[16] = {
> - [VM_1_10_MBARE] = 1,
> - [VM_1_10_SV39] = 1,
> - [VM_1_10_SV48] = 1,
> - [VM_1_10_SV57] = 1
> +static const bool valid_vm_1_10_64[16] = {
> + [VM_1_10_MBARE] = true,
> + [VM_1_10_SV39] = true,
> + [VM_1_10_SV48] = true,
> + [VM_1_10_SV57] = true
> };
>
> /* Machine Information Registers */
> @@ -1209,7 +1209,7 @@ static RISCVException read_mstatus(CPURISCVState
> *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> -static int validate_vm(CPURISCVState *env, target_ulong vm)
> +static bool validate_vm(CPURISCVState *env, target_ulong vm)
> {
> if (riscv_cpu_mxl(env) == MXL_RV32) {
> return valid_vm_1_10_32[vm & 0xf];
> @@ -2648,7 +2648,8 @@ static RISCVException read_satp(CPURISCVState *env,
> int csrno,
> static RISCVException write_satp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - target_ulong vm, mask;
> + target_ulong mask;
> + bool vm;
>
> if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
> return RISCV_EXCP_NONE;
> --
> 2.37.2
>
>
>
[-- Attachment #2: Type: text/html, Size: 3356 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type'
2023-01-31 13:39 ` [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
@ 2023-02-01 13:23 ` Frank Chang
0 siblings, 0 replies; 14+ messages in thread
From: Frank Chang @ 2023-02-01 13:23 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel, Bin Meng
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
Reviewed-by: Frank Chang <frank.chang@sifive.com>
On Tue, Jan 31, 2023 at 10:36 PM Alexandre Ghiti <alexghiti@rivosinc.com>
wrote:
> The 'mmu-type' should reflect what the hardware is capable of so use the
> new satp_mode field in RISCVCPUConfig to do that.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>
> ---
> hw/riscv/virt.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 94ff2a1584..48d034a5f7 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -228,7 +228,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s,
> int socket,
> int cpu;
> uint32_t cpu_phandle;
> MachineState *mc = MACHINE(s);
> - char *name, *cpu_name, *core_name, *intc_name;
> + uint8_t satp_mode_max;
> + char *name, *cpu_name, *core_name, *intc_name, *sv_name;
>
> for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
> cpu_phandle = (*phandle)++;
> @@ -236,14 +237,14 @@ static void create_fdt_socket_cpus(RISCVVirtState
> *s, int socket,
> cpu_name = g_strdup_printf("/cpus/cpu@%d",
> s->soc[socket].hartid_base + cpu);
> qemu_fdt_add_subnode(mc->fdt, cpu_name);
> - if (riscv_feature(&s->soc[socket].harts[cpu].env,
> - RISCV_FEATURE_MMU)) {
> - qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
> - (is_32_bit) ? "riscv,sv32" :
> "riscv,sv48");
> - } else {
> - qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
> - "riscv,none");
> - }
> +
> + satp_mode_max = satp_mode_max_from_map(
> + s->soc[socket].harts[cpu].cfg.satp_mode.map);
> + sv_name = g_strdup_printf("riscv,%s",
> + satp_mode_str(satp_mode_max,
> is_32_bit));
> + qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", sv_name);
> + g_free(sv_name);
> +
> name = riscv_isa_string(&s->soc[socket].harts[cpu]);
> qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
> g_free(name);
> --
> 2.37.2
>
>
>
[-- Attachment #2: Type: text/html, Size: 3432 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities
2023-01-31 13:39 ` [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
2023-02-01 4:21 ` Bin Meng
@ 2023-02-01 15:49 ` Frank Chang
2023-02-02 13:00 ` Alexandre Ghiti
1 sibling, 1 reply; 14+ messages in thread
From: Frank Chang @ 2023-02-01 15:49 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 10892 bytes --]
On Tue, Jan 31, 2023 at 10:36 PM Alexandre Ghiti <alexghiti@rivosinc.com>
wrote:
> Currently, the max satp mode is set with the only constraint that it must
> be
> implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
>
> But we actually need to add another level of constraint: what the hw is
> actually capable of, because currently, a linux booting on a sifive-u54
> boots in sv57 mode which is incompatible with the cpu's sv39 max
> capability.
>
> So add a new bitmap to RISCVSATPMap which contains this capability and
> initialize it in every XXX_cpu_init.
>
> Finally:
> - valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
> - the CPU hw capabilities constrains what the user may select
> - the user's selection then constrains what's available to the guest
> OS.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> target/riscv/cpu.c | 79 +++++++++++++++++++++++++++++++---------------
> target/riscv/cpu.h | 8 +++--
> 2 files changed, 60 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3a7a1746aa..6dd76355ec 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -292,26 +292,36 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> is_32_bit)
> g_assert_not_reached();
> }
>
> -/* Sets the satp mode to the max supported */
> -static void set_satp_mode_default_map(RISCVCPU *cpu)
> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> + uint8_t satp_mode)
> {
> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> + const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
>
This will break user-mode QEMU.
valid_vm_1_10_32 and valid_vm_1_10_64 are defined in !CONFIG_USER_ONLY
section.
This issue also exists in patch 3.
You have to move valid_vm_1_10_32 and valid_vm_1_10_64 out from
!CONFIG_USER_ONLY.
Regards,
Frank Chang
> - if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> - cpu->cfg.satp_mode.map |=
> - (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
> - } else {
> - cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> + for (int i = 0; i <= satp_mode; ++i) {
> + if (valid_vm[i]) {
> + cpu->cfg.satp_mode.supported |= (1 << i);
> + }
> }
> }
>
> +/* Set the satp mode to the max supported */
> +static void set_satp_mode_default_map(RISCVCPU *cpu)
> +{
> + cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
> +}
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> #if defined(TARGET_RISCV32)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> #elif defined(TARGET_RISCV64)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> #endif
> set_priv_version(env, PRIV_VERSION_1_12_0);
> register_cpu_props(obj);
> @@ -321,18 +331,24 @@ static void riscv_any_cpu_init(Object *obj)
> static void rv64_base_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> /* We set this in the realise function */
> set_misa(env, MXL_RV64, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> }
>
> static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -343,6 +359,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -354,28 +371,36 @@ static void rv128_base_cpu_init(Object *obj)
> exit(EXIT_FAILURE);
> }
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> /* We set this in the realise function */
> set_misa(env, MXL_RV128, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> }
> #else
> static void rv32_base_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> /* We set this in the realise function */
> set_misa(env, MXL_RV32, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> }
>
> static void rv32_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> }
>
> static void rv32_sifive_e_cpu_init(Object *obj)
> @@ -386,6 +411,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
>
> static void rv32_ibex_cpu_init(Object *obj)
> @@ -396,6 +422,7 @@ static void rv32_ibex_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_11_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> cpu->cfg.epmp = true;
> }
>
> @@ -407,6 +434,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> }
> #endif
>
> @@ -698,8 +726,9 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> disassemble_info *info)
> static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> {
> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> - const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> - uint8_t satp_mode_max;
> + uint8_t satp_mode_map_max;
> + uint8_t satp_mode_supported_max =
> +
> satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
>
> if (cpu->cfg.satp_mode.map == 0) {
> if (cpu->cfg.satp_mode.init == 0) {
> @@ -712,9 +741,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU
> *cpu, Error **errp)
> * valid_vm_1_10_32/64.
> */
> for (int i = 1; i < 16; ++i) {
> - if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
> + if ((cpu->cfg.satp_mode.init & (1 << i)) &&
> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> for (int j = i - 1; j >= 0; --j) {
> - if (valid_vm[j]) {
> + if (cpu->cfg.satp_mode.supported & (1 << j)) {
> cpu->cfg.satp_mode.map |= (1 << j);
> break;
> }
> @@ -725,37 +755,36 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU
> *cpu, Error **errp)
> }
> }
>
> - /* Make sure the configuration asked is supported by qemu */
> - for (int i = 0; i < 16; ++i) {
> - if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> - error_setg(errp, "satp_mode %s is not valid",
> - satp_mode_str(i, rv32));
> - return;
> - }
> + satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> +
> + /* Make sure the user asked for a supported configuration (HW and
> qemu) */
> + if (satp_mode_map_max > satp_mode_supported_max) {
> + error_setg(errp, "satp_mode %s is higher than hw max capability
> %s",
> + satp_mode_str(satp_mode_map_max, rv32),
> + satp_mode_str(satp_mode_supported_max, rv32));
> + return;
> }
>
> /*
> * Make sure the user did not ask for an invalid configuration as per
> * the specification.
> */
> - satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> -
> if (!rv32) {
> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
> (cpu->cfg.satp_mode.init & (1 << i)) &&
> - valid_vm[i]) {
> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> error_setg(errp, "cannot disable %s satp mode if %s "
> "is enabled", satp_mode_str(i, false),
> - satp_mode_str(satp_mode_max, false));
> + satp_mode_str(satp_mode_map_max, false));
> return;
> }
> }
> }
>
> /* Finally expand the map so that all valid modes are set */
> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> - if (valid_vm[i]) {
> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> + if (cpu->cfg.satp_mode.supported & (1 << i)) {
> cpu->cfg.satp_mode.map |= (1 << i);
> }
> }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e37177db5c..b591122099 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -416,13 +416,17 @@ struct RISCVCPUClass {
>
> /*
> * map is a 16-bit bitmap: the most significant set bit in map is the
> maximum
> - * satp mode that is supported.
> + * satp mode that is supported. It may be chosen by the user and must
> respect
> + * what qemu implements (valid_1_10_32/64) and what the hw is capable of
> + * (supported bitmap below).
> *
> * init is a 16-bit bitmap used to make sure the user selected a correct
> * configuration as per the specification.
> + *
> + * supported is a 16-bit bitmap used to reflect the hw capabilities.
> */
> typedef struct {
> - uint16_t map, init;
> + uint16_t map, init, supported;
> } RISCVSATPMap;
>
> struct RISCVCPUConfig {
> --
> 2.37.2
>
>
>
[-- Attachment #2: Type: text/html, Size: 13463 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 3/5] riscv: Allow user to set the satp mode
2023-01-31 13:39 ` [PATCH v9 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-02-01 4:17 ` Bin Meng
@ 2023-02-02 0:48 ` Alistair Francis
1 sibling, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2023-02-02 0:48 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
Frank Chang, qemu-riscv, qemu-devel, Ludovic Henry
On Wed, Feb 1, 2023 at 1:13 AM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> RISC-V specifies multiple sizes for addressable memory and Linux probes for
> the machine's support at startup via the satp CSR register (done in
> csr.c:validate_vm).
>
> As per the specification, sv64 must support sv57, which in turn must
> support sv48...etc. So we can restrict machine support by simply setting the
> "highest" supported mode and the bare mode is always supported.
>
> You can set the satp mode using the new properties "sv32", "sv39", "sv48",
> "sv57" and "sv64" as follows:
> -cpu rv64,sv57=on # Linux will boot using sv57 scheme
> -cpu rv64,sv39=on # Linux will boot using sv39 scheme
> -cpu rv64,sv57=off # Linux will boot using sv48 scheme
> -cpu rv64 # Linux will boot using sv57 scheme by default
>
> We take the highest level set by the user:
> -cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme
>
> We make sure that invalid configurations are rejected:
> -cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are
> # enabled
>
> We accept "redundant" configurations:
> -cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme
>
> And contradictory configurations:
> -cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme
>
> Co-Developed-by: Ludovic Henry <ludovic@rivosinc.com>
> Signed-off-by: Ludovic Henry <ludovic@rivosinc.com>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 207 +++++++++++++++++++++++++++++++++++++++++++++
> target/riscv/cpu.h | 19 +++++
> target/riscv/csr.c | 12 ++-
> 3 files changed, 231 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7181b34f86..3a7a1746aa 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -27,6 +27,7 @@
> #include "time_helper.h"
> #include "exec/exec-all.h"
> #include "qapi/error.h"
> +#include "qapi/visitor.h"
> #include "qemu/error-report.h"
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> @@ -229,6 +230,81 @@ static void set_vext_version(CPURISCVState *env, int vext_ver)
> env->vext_ver = vext_ver;
> }
>
> +static uint8_t satp_mode_from_str(const char *satp_mode_str)
> +{
> + if (!strncmp(satp_mode_str, "mbare", 5)) {
> + return VM_1_10_MBARE;
> + }
> +
> + if (!strncmp(satp_mode_str, "sv32", 4)) {
> + return VM_1_10_SV32;
> + }
> +
> + if (!strncmp(satp_mode_str, "sv39", 4)) {
> + return VM_1_10_SV39;
> + }
> +
> + if (!strncmp(satp_mode_str, "sv48", 4)) {
> + return VM_1_10_SV48;
> + }
> +
> + if (!strncmp(satp_mode_str, "sv57", 4)) {
> + return VM_1_10_SV57;
> + }
> +
> + if (!strncmp(satp_mode_str, "sv64", 4)) {
> + return VM_1_10_SV64;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +uint8_t satp_mode_max_from_map(uint32_t map)
> +{
> + /* map here has at least one bit set, so no problem with clz */
> + return 31 - __builtin_clz(map);
> +}
> +
> +const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
> +{
> + if (is_32_bit) {
> + switch (satp_mode) {
> + case VM_1_10_SV32:
> + return "sv32";
> + case VM_1_10_MBARE:
> + return "none";
> + }
> + } else {
> + switch (satp_mode) {
> + case VM_1_10_SV64:
> + return "sv64";
> + case VM_1_10_SV57:
> + return "sv57";
> + case VM_1_10_SV48:
> + return "sv48";
> + case VM_1_10_SV39:
> + return "sv39";
> + case VM_1_10_MBARE:
> + return "none";
> + }
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +/* Sets the satp mode to the max supported */
> +static void set_satp_mode_default_map(RISCVCPU *cpu)
> +{
> + bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> +
> + if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> + cpu->cfg.satp_mode.map |=
> + (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
> + } else {
> + cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> + }
> +}
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -619,6 +695,83 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> }
> }
>
> +static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> +{
> + bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> + const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> + uint8_t satp_mode_max;
> +
> + if (cpu->cfg.satp_mode.map == 0) {
> + if (cpu->cfg.satp_mode.init == 0) {
> + /* If unset by the user, we fallback to the default satp mode. */
> + set_satp_mode_default_map(cpu);
> + } else {
> + /*
> + * Find the lowest level that was disabled and then enable the
> + * first valid level below which can be found in
> + * valid_vm_1_10_32/64.
> + */
> + for (int i = 1; i < 16; ++i) {
> + if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
> + for (int j = i - 1; j >= 0; --j) {
> + if (valid_vm[j]) {
> + cpu->cfg.satp_mode.map |= (1 << j);
> + break;
> + }
> + }
> + break;
> + }
> + }
> + }
> + }
> +
> + /* Make sure the configuration asked is supported by qemu */
> + for (int i = 0; i < 16; ++i) {
> + if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> + error_setg(errp, "satp_mode %s is not valid",
> + satp_mode_str(i, rv32));
> + return;
> + }
> + }
> +
> + /*
> + * Make sure the user did not ask for an invalid configuration as per
> + * the specification.
> + */
> + satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> +
> + if (!rv32) {
> + for (int i = satp_mode_max - 1; i >= 0; --i) {
> + if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
> + (cpu->cfg.satp_mode.init & (1 << i)) &&
> + valid_vm[i]) {
> + error_setg(errp, "cannot disable %s satp mode if %s "
> + "is enabled", satp_mode_str(i, false),
> + satp_mode_str(satp_mode_max, false));
> + return;
> + }
> + }
> + }
> +
> + /* Finally expand the map so that all valid modes are set */
> + for (int i = satp_mode_max - 1; i >= 0; --i) {
> + if (valid_vm[i]) {
> + cpu->cfg.satp_mode.map |= (1 << i);
> + }
> + }
> +}
> +
> +static void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> +{
> + Error *local_err = NULL;
> +
> + riscv_cpu_satp_mode_finalize(cpu, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +}
> +
> static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> {
> CPUState *cs = CPU(dev);
> @@ -919,6 +1072,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> }
> #endif
>
> + riscv_cpu_finalize_features(cpu, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> riscv_cpu_register_gdb_regs_for_features(cs);
>
> qemu_init_vcpu(cs);
> @@ -927,6 +1086,52 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> mcc->parent_realize(dev, errp);
> }
>
> +static void cpu_riscv_get_satp(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + RISCVSATPMap *satp_map = opaque;
> + uint8_t satp = satp_mode_from_str(name);
> + bool value;
> +
> + value = satp_map->map & (1 << satp);
> +
> + visit_type_bool(v, name, &value, errp);
> +}
> +
> +static void cpu_riscv_set_satp(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + RISCVSATPMap *satp_map = opaque;
> + uint8_t satp = satp_mode_from_str(name);
> + bool value;
> +
> + if (!visit_type_bool(v, name, &value, errp)) {
> + return;
> + }
> +
> + satp_map->map = deposit32(satp_map->map, satp, 1, value);
> + satp_map->init |= 1 << satp;
> +}
> +
> +static void riscv_add_satp_mode_properties(Object *obj)
> +{
> + RISCVCPU *cpu = RISCV_CPU(obj);
> +
> + if (cpu->env.misa_mxl == MXL_RV32) {
> + object_property_add(obj, "sv32", "bool", cpu_riscv_get_satp,
> + cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
> + } else {
> + object_property_add(obj, "sv39", "bool", cpu_riscv_get_satp,
> + cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
> + object_property_add(obj, "sv48", "bool", cpu_riscv_get_satp,
> + cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
> + object_property_add(obj, "sv57", "bool", cpu_riscv_get_satp,
> + cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
> + object_property_add(obj, "sv64", "bool", cpu_riscv_get_satp,
> + cpu_riscv_set_satp, NULL, &cpu->cfg.satp_mode);
> + }
> +}
> +
> #ifndef CONFIG_USER_ONLY
> static void riscv_cpu_set_irq(void *opaque, int irq, int level)
> {
> @@ -1091,6 +1296,8 @@ static void register_cpu_props(Object *obj)
> for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
> qdev_property_add_static(dev, prop);
> }
> +
> + riscv_add_satp_mode_properties(obj);
> }
>
> static Property riscv_cpu_properties[] = {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f5609b62a2..e37177db5c 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -27,6 +27,7 @@
> #include "qom/object.h"
> #include "qemu/int128.h"
> #include "cpu_bits.h"
> +#include "qapi/qapi-types-common.h"
>
> #define TCG_GUEST_DEFAULT_MO 0
>
> @@ -413,6 +414,17 @@ struct RISCVCPUClass {
> ResettablePhases parent_phases;
> };
>
> +/*
> + * map is a 16-bit bitmap: the most significant set bit in map is the maximum
> + * satp mode that is supported.
> + *
> + * init is a 16-bit bitmap used to make sure the user selected a correct
> + * configuration as per the specification.
> + */
> +typedef struct {
> + uint16_t map, init;
> +} RISCVSATPMap;
> +
> struct RISCVCPUConfig {
> bool ext_i;
> bool ext_e;
> @@ -488,6 +500,8 @@ struct RISCVCPUConfig {
> bool debug;
>
> bool short_isa_string;
> +
> + RISCVSATPMap satp_mode;
> };
>
> typedef struct RISCVCPUConfig RISCVCPUConfig;
> @@ -794,9 +808,14 @@ enum riscv_pmu_event_idx {
> /* CSR function table */
> extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
>
> +extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
> +
> void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
>
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
>
> +uint8_t satp_mode_max_from_map(uint32_t map);
> +const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> +
> #endif /* RISCV_CPU_H */
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6b157806a5..f9eff3f1e3 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1117,12 +1117,12 @@ static const target_ulong hip_writable_mask = MIP_VSSIP;
> static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
> static const target_ulong vsip_writable_mask = MIP_VSSIP;
>
> -static const bool valid_vm_1_10_32[16] = {
> +const bool valid_vm_1_10_32[16] = {
> [VM_1_10_MBARE] = true,
> [VM_1_10_SV32] = true
> };
>
> -static const bool valid_vm_1_10_64[16] = {
> +const bool valid_vm_1_10_64[16] = {
> [VM_1_10_MBARE] = true,
> [VM_1_10_SV39] = true,
> [VM_1_10_SV48] = true,
> @@ -1211,11 +1211,9 @@ static RISCVException read_mstatus(CPURISCVState *env, int csrno,
>
> static bool validate_vm(CPURISCVState *env, target_ulong vm)
> {
> - if (riscv_cpu_mxl(env) == MXL_RV32) {
> - return valid_vm_1_10_32[vm & 0xf];
> - } else {
> - return valid_vm_1_10_64[vm & 0xf];
> - }
> + RISCVCPU *cpu = RISCV_CPU(env_cpu(env));
> +
> + return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> }
>
> static RISCVException write_mstatus(CPURISCVState *env, int csrno,
> --
> 2.37.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities
2023-02-01 15:49 ` Frank Chang
@ 2023-02-02 13:00 ` Alexandre Ghiti
2023-02-02 13:04 ` Frank Chang
0 siblings, 1 reply; 14+ messages in thread
From: Alexandre Ghiti @ 2023-02-02 13:00 UTC (permalink / raw)
To: Frank Chang
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
qemu-riscv, qemu-devel
Hi Frank,
On Wed, Feb 1, 2023 at 4:49 PM Frank Chang <frank.chang@sifive.com> wrote:
>
> On Tue, Jan 31, 2023 at 10:36 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>>
>> Currently, the max satp mode is set with the only constraint that it must be
>> implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
>>
>> But we actually need to add another level of constraint: what the hw is
>> actually capable of, because currently, a linux booting on a sifive-u54
>> boots in sv57 mode which is incompatible with the cpu's sv39 max
>> capability.
>>
>> So add a new bitmap to RISCVSATPMap which contains this capability and
>> initialize it in every XXX_cpu_init.
>>
>> Finally:
>> - valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
>> - the CPU hw capabilities constrains what the user may select
>> - the user's selection then constrains what's available to the guest
>> OS.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>> ---
>> target/riscv/cpu.c | 79 +++++++++++++++++++++++++++++++---------------
>> target/riscv/cpu.h | 8 +++--
>> 2 files changed, 60 insertions(+), 27 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 3a7a1746aa..6dd76355ec 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -292,26 +292,36 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
>> g_assert_not_reached();
>> }
>>
>> -/* Sets the satp mode to the max supported */
>> -static void set_satp_mode_default_map(RISCVCPU *cpu)
>> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
>> + uint8_t satp_mode)
>> {
>> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
>> + const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
>
>
> This will break user-mode QEMU.
> valid_vm_1_10_32 and valid_vm_1_10_64 are defined in !CONFIG_USER_ONLY section.
> This issue also exists in patch 3.
> You have to move valid_vm_1_10_32 and valid_vm_1_10_64 out from !CONFIG_USER_ONLY.
Indeed, good catch, thanks! Rather than moving valid_vm_1_10_[32|64],
I'm going to try to surround all the satp handling inside #ifdef
CONFIG_USER_ONLY. But if it's too cumbersome, I'll do as you suggest.
Thanks again,
Alex
>
> Regards,
> Frank Chang
>
>>
>> - if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
>> - cpu->cfg.satp_mode.map |=
>> - (1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
>> - } else {
>> - cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
>> + for (int i = 0; i <= satp_mode; ++i) {
>> + if (valid_vm[i]) {
>> + cpu->cfg.satp_mode.supported |= (1 << i);
>> + }
>> }
>> }
>>
>> +/* Set the satp mode to the max supported */
>> +static void set_satp_mode_default_map(RISCVCPU *cpu)
>> +{
>> + cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
>> +}
>> +
>> static void riscv_any_cpu_init(Object *obj)
>> {
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> +
>> #if defined(TARGET_RISCV32)
>> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
>> #elif defined(TARGET_RISCV64)
>> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
>> #endif
>> set_priv_version(env, PRIV_VERSION_1_12_0);
>> register_cpu_props(obj);
>> @@ -321,18 +331,24 @@ static void riscv_any_cpu_init(Object *obj)
>> static void rv64_base_cpu_init(Object *obj)
>> {
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> +
>> /* We set this in the realise function */
>> set_misa(env, MXL_RV64, 0);
>> register_cpu_props(obj);
>> /* Set latest version of privileged specification */
>> set_priv_version(env, PRIV_VERSION_1_12_0);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
>> }
>>
>> static void rv64_sifive_u_cpu_init(Object *obj)
>> {
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> +
>> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>> set_priv_version(env, PRIV_VERSION_1_10_0);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV39);
>> }
>>
>> static void rv64_sifive_e_cpu_init(Object *obj)
>> @@ -343,6 +359,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
>> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
>> set_priv_version(env, PRIV_VERSION_1_10_0);
>> cpu->cfg.mmu = false;
>> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
>> }
>>
>> static void rv128_base_cpu_init(Object *obj)
>> @@ -354,28 +371,36 @@ static void rv128_base_cpu_init(Object *obj)
>> exit(EXIT_FAILURE);
>> }
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> /* We set this in the realise function */
>> set_misa(env, MXL_RV128, 0);
>> register_cpu_props(obj);
>> /* Set latest version of privileged specification */
>> set_priv_version(env, PRIV_VERSION_1_12_0);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
>> }
>> #else
>> static void rv32_base_cpu_init(Object *obj)
>> {
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> +
>> /* We set this in the realise function */
>> set_misa(env, MXL_RV32, 0);
>> register_cpu_props(obj);
>> /* Set latest version of privileged specification */
>> set_priv_version(env, PRIV_VERSION_1_12_0);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
>> }
>>
>> static void rv32_sifive_u_cpu_init(Object *obj)
>> {
>> CPURISCVState *env = &RISCV_CPU(obj)->env;
>> + RISCVCPU *cpu = RISCV_CPU(obj);
>> +
>> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>> set_priv_version(env, PRIV_VERSION_1_10_0);
>> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
>> }
>>
>> static void rv32_sifive_e_cpu_init(Object *obj)
>> @@ -386,6 +411,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
>> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
>> set_priv_version(env, PRIV_VERSION_1_10_0);
>> cpu->cfg.mmu = false;
>> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
>> }
>>
>> static void rv32_ibex_cpu_init(Object *obj)
>> @@ -396,6 +422,7 @@ static void rv32_ibex_cpu_init(Object *obj)
>> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
>> set_priv_version(env, PRIV_VERSION_1_11_0);
>> cpu->cfg.mmu = false;
>> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
>> cpu->cfg.epmp = true;
>> }
>>
>> @@ -407,6 +434,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
>> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
>> set_priv_version(env, PRIV_VERSION_1_10_0);
>> cpu->cfg.mmu = false;
>> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
>> }
>> #endif
>>
>> @@ -698,8 +726,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>> static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>> {
>> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
>> - const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
>> - uint8_t satp_mode_max;
>> + uint8_t satp_mode_map_max;
>> + uint8_t satp_mode_supported_max =
>> + satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
>>
>> if (cpu->cfg.satp_mode.map == 0) {
>> if (cpu->cfg.satp_mode.init == 0) {
>> @@ -712,9 +741,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>> * valid_vm_1_10_32/64.
>> */
>> for (int i = 1; i < 16; ++i) {
>> - if ((cpu->cfg.satp_mode.init & (1 << i)) && valid_vm[i]) {
>> + if ((cpu->cfg.satp_mode.init & (1 << i)) &&
>> + (cpu->cfg.satp_mode.supported & (1 << i))) {
>> for (int j = i - 1; j >= 0; --j) {
>> - if (valid_vm[j]) {
>> + if (cpu->cfg.satp_mode.supported & (1 << j)) {
>> cpu->cfg.satp_mode.map |= (1 << j);
>> break;
>> }
>> @@ -725,37 +755,36 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>> }
>> }
>>
>> - /* Make sure the configuration asked is supported by qemu */
>> - for (int i = 0; i < 16; ++i) {
>> - if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
>> - error_setg(errp, "satp_mode %s is not valid",
>> - satp_mode_str(i, rv32));
>> - return;
>> - }
>> + satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>> +
>> + /* Make sure the user asked for a supported configuration (HW and qemu) */
>> + if (satp_mode_map_max > satp_mode_supported_max) {
>> + error_setg(errp, "satp_mode %s is higher than hw max capability %s",
>> + satp_mode_str(satp_mode_map_max, rv32),
>> + satp_mode_str(satp_mode_supported_max, rv32));
>> + return;
>> }
>>
>> /*
>> * Make sure the user did not ask for an invalid configuration as per
>> * the specification.
>> */
>> - satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>> -
>> if (!rv32) {
>> - for (int i = satp_mode_max - 1; i >= 0; --i) {
>> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
>> if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
>> (cpu->cfg.satp_mode.init & (1 << i)) &&
>> - valid_vm[i]) {
>> + (cpu->cfg.satp_mode.supported & (1 << i))) {
>> error_setg(errp, "cannot disable %s satp mode if %s "
>> "is enabled", satp_mode_str(i, false),
>> - satp_mode_str(satp_mode_max, false));
>> + satp_mode_str(satp_mode_map_max, false));
>> return;
>> }
>> }
>> }
>>
>> /* Finally expand the map so that all valid modes are set */
>> - for (int i = satp_mode_max - 1; i >= 0; --i) {
>> - if (valid_vm[i]) {
>> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
>> + if (cpu->cfg.satp_mode.supported & (1 << i)) {
>> cpu->cfg.satp_mode.map |= (1 << i);
>> }
>> }
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index e37177db5c..b591122099 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -416,13 +416,17 @@ struct RISCVCPUClass {
>>
>> /*
>> * map is a 16-bit bitmap: the most significant set bit in map is the maximum
>> - * satp mode that is supported.
>> + * satp mode that is supported. It may be chosen by the user and must respect
>> + * what qemu implements (valid_1_10_32/64) and what the hw is capable of
>> + * (supported bitmap below).
>> *
>> * init is a 16-bit bitmap used to make sure the user selected a correct
>> * configuration as per the specification.
>> + *
>> + * supported is a 16-bit bitmap used to reflect the hw capabilities.
>> */
>> typedef struct {
>> - uint16_t map, init;
>> + uint16_t map, init, supported;
>> } RISCVSATPMap;
>>
>> struct RISCVCPUConfig {
>> --
>> 2.37.2
>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities
2023-02-02 13:00 ` Alexandre Ghiti
@ 2023-02-02 13:04 ` Frank Chang
0 siblings, 0 replies; 14+ messages in thread
From: Frank Chang @ 2023-02-02 13:04 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
qemu-riscv, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 12328 bytes --]
On Thu, Feb 2, 2023 at 9:01 PM Alexandre Ghiti <alexghiti@rivosinc.com>
wrote:
> Hi Frank,
>
> On Wed, Feb 1, 2023 at 4:49 PM Frank Chang <frank.chang@sifive.com> wrote:
> >
> > On Tue, Jan 31, 2023 at 10:36 PM Alexandre Ghiti <alexghiti@rivosinc.com>
> wrote:
> >>
> >> Currently, the max satp mode is set with the only constraint that it
> must be
> >> implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
> >>
> >> But we actually need to add another level of constraint: what the hw is
> >> actually capable of, because currently, a linux booting on a sifive-u54
> >> boots in sv57 mode which is incompatible with the cpu's sv39 max
> >> capability.
> >>
> >> So add a new bitmap to RISCVSATPMap which contains this capability and
> >> initialize it in every XXX_cpu_init.
> >>
> >> Finally:
> >> - valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
> >> - the CPU hw capabilities constrains what the user may select
> >> - the user's selection then constrains what's available to the guest
> >> OS.
> >>
> >> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> >> ---
> >> target/riscv/cpu.c | 79 +++++++++++++++++++++++++++++++---------------
> >> target/riscv/cpu.h | 8 +++--
> >> 2 files changed, 60 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 3a7a1746aa..6dd76355ec 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -292,26 +292,36 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> is_32_bit)
> >> g_assert_not_reached();
> >> }
> >>
> >> -/* Sets the satp mode to the max supported */
> >> -static void set_satp_mode_default_map(RISCVCPU *cpu)
> >> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> >> + uint8_t satp_mode)
> >> {
> >> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> >> + const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> >
> >
> > This will break user-mode QEMU.
> > valid_vm_1_10_32 and valid_vm_1_10_64 are defined in !CONFIG_USER_ONLY
> section.
> > This issue also exists in patch 3.
> > You have to move valid_vm_1_10_32 and valid_vm_1_10_64 out from
> !CONFIG_USER_ONLY.
>
> Indeed, good catch, thanks! Rather than moving valid_vm_1_10_[32|64],
> I'm going to try to surround all the satp handling inside #ifdef
> CONFIG_USER_ONLY. But if it's too cumbersome, I'll do as you suggest.
>
Hi Alex,
I think surrounding all the satp handling inside #ifdef is okay,
since satp is not been used in user-mode QEMU.
Regards,
Frank Chang
>
> Thanks again,
>
> Alex
>
> >
> > Regards,
> > Frank Chang
> >
> >>
> >> - if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> >> - cpu->cfg.satp_mode.map |=
> >> - (1 << satp_mode_from_str(rv32 ? "sv32" :
> "sv57"));
> >> - } else {
> >> - cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> >> + for (int i = 0; i <= satp_mode; ++i) {
> >> + if (valid_vm[i]) {
> >> + cpu->cfg.satp_mode.supported |= (1 << i);
> >> + }
> >> }
> >> }
> >>
> >> +/* Set the satp mode to the max supported */
> >> +static void set_satp_mode_default_map(RISCVCPU *cpu)
> >> +{
> >> + cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
> >> +}
> >> +
> >> static void riscv_any_cpu_init(Object *obj)
> >> {
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> +
> >> #if defined(TARGET_RISCV32)
> >> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> >> #elif defined(TARGET_RISCV64)
> >> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> >> #endif
> >> set_priv_version(env, PRIV_VERSION_1_12_0);
> >> register_cpu_props(obj);
> >> @@ -321,18 +331,24 @@ static void riscv_any_cpu_init(Object *obj)
> >> static void rv64_base_cpu_init(Object *obj)
> >> {
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> +
> >> /* We set this in the realise function */
> >> set_misa(env, MXL_RV64, 0);
> >> register_cpu_props(obj);
> >> /* Set latest version of privileged specification */
> >> set_priv_version(env, PRIV_VERSION_1_12_0);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> >> }
> >>
> >> static void rv64_sifive_u_cpu_init(Object *obj)
> >> {
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> +
> >> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
> >> set_priv_version(env, PRIV_VERSION_1_10_0);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> >> }
> >>
> >> static void rv64_sifive_e_cpu_init(Object *obj)
> >> @@ -343,6 +359,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
> >> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> >> set_priv_version(env, PRIV_VERSION_1_10_0);
> >> cpu->cfg.mmu = false;
> >> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> >> }
> >>
> >> static void rv128_base_cpu_init(Object *obj)
> >> @@ -354,28 +371,36 @@ static void rv128_base_cpu_init(Object *obj)
> >> exit(EXIT_FAILURE);
> >> }
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> /* We set this in the realise function */
> >> set_misa(env, MXL_RV128, 0);
> >> register_cpu_props(obj);
> >> /* Set latest version of privileged specification */
> >> set_priv_version(env, PRIV_VERSION_1_12_0);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV57);
> >> }
> >> #else
> >> static void rv32_base_cpu_init(Object *obj)
> >> {
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> +
> >> /* We set this in the realise function */
> >> set_misa(env, MXL_RV32, 0);
> >> register_cpu_props(obj);
> >> /* Set latest version of privileged specification */
> >> set_priv_version(env, PRIV_VERSION_1_12_0);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> >> }
> >>
> >> static void rv32_sifive_u_cpu_init(Object *obj)
> >> {
> >> CPURISCVState *env = &RISCV_CPU(obj)->env;
> >> + RISCVCPU *cpu = RISCV_CPU(obj);
> >> +
> >> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
> >> set_priv_version(env, PRIV_VERSION_1_10_0);
> >> + set_satp_mode_max_supported(cpu, VM_1_10_SV32);
> >> }
> >>
> >> static void rv32_sifive_e_cpu_init(Object *obj)
> >> @@ -386,6 +411,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
> >> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
> >> set_priv_version(env, PRIV_VERSION_1_10_0);
> >> cpu->cfg.mmu = false;
> >> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> >> }
> >>
> >> static void rv32_ibex_cpu_init(Object *obj)
> >> @@ -396,6 +422,7 @@ static void rv32_ibex_cpu_init(Object *obj)
> >> set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> >> set_priv_version(env, PRIV_VERSION_1_11_0);
> >> cpu->cfg.mmu = false;
> >> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> >> cpu->cfg.epmp = true;
> >> }
> >>
> >> @@ -407,6 +434,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> >> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> >> set_priv_version(env, PRIV_VERSION_1_10_0);
> >> cpu->cfg.mmu = false;
> >> + set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> >> }
> >> #endif
> >>
> >> @@ -698,8 +726,9 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> disassemble_info *info)
> >> static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> >> {
> >> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> >> - const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> >> - uint8_t satp_mode_max;
> >> + uint8_t satp_mode_map_max;
> >> + uint8_t satp_mode_supported_max =
> >> +
> satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> >>
> >> if (cpu->cfg.satp_mode.map == 0) {
> >> if (cpu->cfg.satp_mode.init == 0) {
> >> @@ -712,9 +741,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU
> *cpu, Error **errp)
> >> * valid_vm_1_10_32/64.
> >> */
> >> for (int i = 1; i < 16; ++i) {
> >> - if ((cpu->cfg.satp_mode.init & (1 << i)) &&
> valid_vm[i]) {
> >> + if ((cpu->cfg.satp_mode.init & (1 << i)) &&
> >> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> >> for (int j = i - 1; j >= 0; --j) {
> >> - if (valid_vm[j]) {
> >> + if (cpu->cfg.satp_mode.supported & (1 << j)) {
> >> cpu->cfg.satp_mode.map |= (1 << j);
> >> break;
> >> }
> >> @@ -725,37 +755,36 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU
> *cpu, Error **errp)
> >> }
> >> }
> >>
> >> - /* Make sure the configuration asked is supported by qemu */
> >> - for (int i = 0; i < 16; ++i) {
> >> - if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> >> - error_setg(errp, "satp_mode %s is not valid",
> >> - satp_mode_str(i, rv32));
> >> - return;
> >> - }
> >> + satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> >> +
> >> + /* Make sure the user asked for a supported configuration (HW and
> qemu) */
> >> + if (satp_mode_map_max > satp_mode_supported_max) {
> >> + error_setg(errp, "satp_mode %s is higher than hw max
> capability %s",
> >> + satp_mode_str(satp_mode_map_max, rv32),
> >> + satp_mode_str(satp_mode_supported_max, rv32));
> >> + return;
> >> }
> >>
> >> /*
> >> * Make sure the user did not ask for an invalid configuration as
> per
> >> * the specification.
> >> */
> >> - satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> >> -
> >> if (!rv32) {
> >> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> >> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> >> if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
> >> (cpu->cfg.satp_mode.init & (1 << i)) &&
> >> - valid_vm[i]) {
> >> + (cpu->cfg.satp_mode.supported & (1 << i))) {
> >> error_setg(errp, "cannot disable %s satp mode if %s "
> >> "is enabled", satp_mode_str(i, false),
> >> - satp_mode_str(satp_mode_max, false));
> >> + satp_mode_str(satp_mode_map_max, false));
> >> return;
> >> }
> >> }
> >> }
> >>
> >> /* Finally expand the map so that all valid modes are set */
> >> - for (int i = satp_mode_max - 1; i >= 0; --i) {
> >> - if (valid_vm[i]) {
> >> + for (int i = satp_mode_map_max - 1; i >= 0; --i) {
> >> + if (cpu->cfg.satp_mode.supported & (1 << i)) {
> >> cpu->cfg.satp_mode.map |= (1 << i);
> >> }
> >> }
> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >> index e37177db5c..b591122099 100644
> >> --- a/target/riscv/cpu.h
> >> +++ b/target/riscv/cpu.h
> >> @@ -416,13 +416,17 @@ struct RISCVCPUClass {
> >>
> >> /*
> >> * map is a 16-bit bitmap: the most significant set bit in map is the
> maximum
> >> - * satp mode that is supported.
> >> + * satp mode that is supported. It may be chosen by the user and must
> respect
> >> + * what qemu implements (valid_1_10_32/64) and what the hw is capable
> of
> >> + * (supported bitmap below).
> >> *
> >> * init is a 16-bit bitmap used to make sure the user selected a
> correct
> >> * configuration as per the specification.
> >> + *
> >> + * supported is a 16-bit bitmap used to reflect the hw capabilities.
> >> */
> >> typedef struct {
> >> - uint16_t map, init;
> >> + uint16_t map, init, supported;
> >> } RISCVSATPMap;
> >>
> >> struct RISCVCPUConfig {
> >> --
> >> 2.37.2
> >>
> >>
>
[-- Attachment #2: Type: text/html, Size: 16747 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-02-02 13:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-31 13:39 [PATCH v9 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
2023-01-31 13:39 ` [PATCH v9 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
2023-02-01 13:21 ` Frank Chang
2023-01-31 13:39 ` [PATCH v9 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-02-01 4:17 ` Bin Meng
2023-02-02 0:48 ` Alistair Francis
2023-01-31 13:39 ` [PATCH v9 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
2023-02-01 4:21 ` Bin Meng
2023-02-01 15:49 ` Frank Chang
2023-02-02 13:00 ` Alexandre Ghiti
2023-02-02 13:04 ` Frank Chang
2023-01-31 13:39 ` [PATCH v9 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
2023-02-01 13:23 ` Frank Chang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).