All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/5] riscv: Allow user to set the satp mode
@ 2023-02-03  5:58 Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 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).

base-commit: commit 75cc28648574 ("configure: remove
backwards-compatibility code"

v10:
- Fix user mode build by surrounding satp handling with #ifndef
  CONFIG_USER_ONLY, Frank
- Fix AB/RB from Frank and Alistair

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 | 271 +++++++++++++++++++++++++++++++++++++++++++--
 target/riscv/cpu.h |  25 +++++
 target/riscv/csr.c |  29 +++--
 4 files changed, 313 insertions(+), 31 deletions(-)

-- 
2.37.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v10 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-02-03  5:58 ` Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 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] 12+ messages in thread

* [PATCH v10 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
@ 2023-02-03  5:58 ` Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 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>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
---
 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] 12+ messages in thread

* [PATCH v10 3/5] riscv: Allow user to set the satp mode
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
  2023-02-03  5:58 ` [PATCH v10 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
@ 2023-02-03  5:58 ` Alexandre Ghiti
  2023-02-03  8:01   ` Frank Chang
  2023-02-03  5:58 ` [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	Frank Chang, qemu-riscv, qemu-devel
  Cc: Alexandre Ghiti, Ludovic Henry, Bin Meng

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>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 215 +++++++++++++++++++++++++++++++++++++++++++++
 target/riscv/cpu.h |  21 +++++
 target/riscv/csr.c |  12 ++-
 3 files changed, 241 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7181b34f86..56057cf87c 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,83 @@ static void set_vext_version(CPURISCVState *env, int vext_ver)
     env->vext_ver = vext_ver;
 }
 
+#ifndef CONFIG_USER_ONLY
+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"));
+    }
+}
+#endif
+
 static void riscv_any_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -619,6 +697,87 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
     }
 }
 
+#ifndef CONFIG_USER_ONLY
+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);
+        }
+    }
+}
+#endif
+
+static void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
+{
+#ifndef CONFIG_USER_ONLY
+    Error *local_err = NULL;
+
+    riscv_cpu_satp_mode_finalize(cpu, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+#endif
+}
+
 static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -919,6 +1078,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);
@@ -928,6 +1093,52 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 }
 
 #ifndef CONFIG_USER_ONLY
+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);
+    }
+}
+
 static void riscv_cpu_set_irq(void *opaque, int irq, int level)
 {
     RISCVCPU *cpu = RISCV_CPU(opaque);
@@ -1091,6 +1302,10 @@ static void register_cpu_props(Object *obj)
     for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
         qdev_property_add_static(dev, prop);
     }
+
+#ifndef CONFIG_USER_ONLY
+    riscv_add_satp_mode_properties(obj);
+#endif
 }
 
 static Property riscv_cpu_properties[] = {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..df6d9e06e4 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,10 @@ struct RISCVCPUConfig {
     bool debug;
 
     bool short_isa_string;
+
+#ifndef CONFIG_USER_ONLY
+    RISCVSATPMap satp_mode;
+#endif
 };
 
 typedef struct RISCVCPUConfig RISCVCPUConfig;
@@ -794,9 +810,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] 12+ messages in thread

* [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
                   ` (2 preceding siblings ...)
  2023-02-03  5:58 ` [PATCH v10 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-02-03  5:58 ` Alexandre Ghiti
  2023-02-03  7:58   ` Frank Chang
  2023-02-05 23:11   ` Alistair Francis
  2023-02-03  5:58 ` [PATCH v10 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
  2023-03-02 17:42 ` [PATCH v10 0/5] riscv: Allow user to set the satp mode Daniel Henrique Barboza
  5 siblings, 2 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	Frank Chang, qemu-riscv, qemu-devel
  Cc: Alexandre Ghiti, Bin Meng

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>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
---
 target/riscv/cpu.c | 91 +++++++++++++++++++++++++++++++++-------------
 target/riscv/cpu.h |  8 +++-
 2 files changed, 72 insertions(+), 27 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 56057cf87c..7e9924ede9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -293,18 +293,24 @@ 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;
+}
 #endif
 
 static void riscv_any_cpu_init(Object *obj)
@@ -315,6 +321,13 @@ static void riscv_any_cpu_init(Object *obj)
 #elif defined(TARGET_RISCV64)
     set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
 #endif
+
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj),
+            riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
+                                    VM_1_10_SV32 : VM_1_10_SV57);
+#endif
+
     set_priv_version(env, PRIV_VERSION_1_12_0);
     register_cpu_props(obj);
 }
@@ -328,6 +341,9 @@ static void rv64_base_cpu_init(Object *obj)
     register_cpu_props(obj);
     /* Set latest version of privileged specification */
     set_priv_version(env, PRIV_VERSION_1_12_0);
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
+#endif
 }
 
 static void rv64_sifive_u_cpu_init(Object *obj)
@@ -335,6 +351,9 @@ static void rv64_sifive_u_cpu_init(Object *obj)
     CPURISCVState *env = &RISCV_CPU(obj)->env;
     set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
     set_priv_version(env, PRIV_VERSION_1_10_0);
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
+#endif
 }
 
 static void rv64_sifive_e_cpu_init(Object *obj)
@@ -345,6 +364,9 @@ 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;
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
+#endif
 }
 
 static void rv128_base_cpu_init(Object *obj)
@@ -361,6 +383,9 @@ static void rv128_base_cpu_init(Object *obj)
     register_cpu_props(obj);
     /* Set latest version of privileged specification */
     set_priv_version(env, PRIV_VERSION_1_12_0);
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
+#endif
 }
 #else
 static void rv32_base_cpu_init(Object *obj)
@@ -371,6 +396,9 @@ static void rv32_base_cpu_init(Object *obj)
     register_cpu_props(obj);
     /* Set latest version of privileged specification */
     set_priv_version(env, PRIV_VERSION_1_12_0);
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
+#endif
 }
 
 static void rv32_sifive_u_cpu_init(Object *obj)
@@ -378,6 +406,9 @@ static void rv32_sifive_u_cpu_init(Object *obj)
     CPURISCVState *env = &RISCV_CPU(obj)->env;
     set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
     set_priv_version(env, PRIV_VERSION_1_10_0);
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
+#endif
 }
 
 static void rv32_sifive_e_cpu_init(Object *obj)
@@ -388,6 +419,9 @@ 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;
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
+#endif
 }
 
 static void rv32_ibex_cpu_init(Object *obj)
@@ -398,6 +432,9 @@ 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;
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
+#endif
     cpu->cfg.epmp = true;
 }
 
@@ -409,6 +446,9 @@ 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;
+#ifndef CONFIG_USER_ONLY
+    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
+#endif
 }
 #endif
 
@@ -701,8 +741,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) {
@@ -715,9 +756,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;
                         }
@@ -728,37 +770,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 df6d9e06e4..defc5769b5 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] 12+ messages in thread

* [PATCH v10 5/5] riscv: Correctly set the device-tree entry 'mmu-type'
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
                   ` (3 preceding siblings ...)
  2023-02-03  5:58 ` [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
@ 2023-02-03  5:58 ` Alexandre Ghiti
  2023-03-02 17:42 ` [PATCH v10 0/5] riscv: Allow user to set the satp mode Daniel Henrique Barboza
  5 siblings, 0 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-02-03  5:58 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>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
---
 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] 12+ messages in thread

* Re: [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities
  2023-02-03  5:58 ` [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
@ 2023-02-03  7:58   ` Frank Chang
  2023-02-05 23:11   ` Alistair Francis
  1 sibling, 0 replies; 12+ messages in thread
From: Frank Chang @ 2023-02-03  7:58 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	qemu-riscv, qemu-devel, Bin Meng

[-- Attachment #1: Type: text/plain, Size: 10606 bytes --]

Reviewed-by: Frank Chang <frank.chang@sifive.com>

On Fri, Feb 3, 2023 at 2:02 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>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>
> ---
>  target/riscv/cpu.c | 91 +++++++++++++++++++++++++++++++++-------------
>  target/riscv/cpu.h |  8 +++-
>  2 files changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 56057cf87c..7e9924ede9 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -293,18 +293,24 @@ 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;
> +}
>  #endif
>
>  static void riscv_any_cpu_init(Object *obj)
> @@ -315,6 +321,13 @@ static void riscv_any_cpu_init(Object *obj)
>  #elif defined(TARGET_RISCV64)
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>  #endif
> +
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj),
> +            riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
> +                                    VM_1_10_SV32 : VM_1_10_SV57);
> +#endif
> +
>      set_priv_version(env, PRIV_VERSION_1_12_0);
>      register_cpu_props(obj);
>  }
> @@ -328,6 +341,9 @@ static void rv64_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>
>  static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -335,6 +351,9 @@ static void rv64_sifive_u_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
> +#endif
>  }
>
>  static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -345,6 +364,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>
>  static void rv128_base_cpu_init(Object *obj)
> @@ -361,6 +383,9 @@ static void rv128_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>  #else
>  static void rv32_base_cpu_init(Object *obj)
> @@ -371,6 +396,9 @@ static void rv32_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> +#endif
>  }
>
>  static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -378,6 +406,9 @@ static void rv32_sifive_u_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS |
> RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> +#endif
>  }
>
>  static void rv32_sifive_e_cpu_init(Object *obj)
> @@ -388,6 +419,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>
>  static void rv32_ibex_cpu_init(Object *obj)
> @@ -398,6 +432,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>      cpu->cfg.epmp = true;
>  }
>
> @@ -409,6 +446,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>  #endif
>
> @@ -701,8 +741,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) {
> @@ -715,9 +756,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;
>                          }
> @@ -728,37 +770,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 df6d9e06e4..defc5769b5 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: 13001 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 3/5] riscv: Allow user to set the satp mode
  2023-02-03  5:58 ` [PATCH v10 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
@ 2023-02-03  8:01   ` Frank Chang
  0 siblings, 0 replies; 12+ messages in thread
From: Frank Chang @ 2023-02-03  8:01 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	qemu-riscv, qemu-devel, Ludovic Henry, Bin Meng

[-- Attachment #1: Type: text/plain, Size: 13268 bytes --]

On Fri, Feb 3, 2023 at 2:01 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>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c | 215 +++++++++++++++++++++++++++++++++++++++++++++
>  target/riscv/cpu.h |  21 +++++
>  target/riscv/csr.c |  12 ++-
>  3 files changed, 241 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7181b34f86..56057cf87c 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,83 @@ static void set_vext_version(CPURISCVState *env, int
> vext_ver)
>      env->vext_ver = vext_ver;
>  }
>
> +#ifndef CONFIG_USER_ONLY
> +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"));
>

Nit:
        cpu->cfg.satp_mode.map |=
                        (1 << (rv32 ? VM_1_10_SV32 : VM_1_10_SV57));


> +    } else {
> +        cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
>

        cpu->cfg.satp_mode.map |= (1 << VM_1_10_MBARE);

Otherwise,
Reviewed-by: Frank Chang <frank.chang@sifive.com>


> +    }
> +}
> +#endif
> +
>  static void riscv_any_cpu_init(Object *obj)
>  {
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -619,6 +697,87 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> disassemble_info *info)
>      }
>  }
>
> +#ifndef CONFIG_USER_ONLY
> +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);
> +        }
> +    }
> +}
> +#endif
> +
> +static void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    Error *local_err = NULL;
> +
> +    riscv_cpu_satp_mode_finalize(cpu, &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +#endif
> +}
> +
>  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -919,6 +1078,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);
> @@ -928,6 +1093,52 @@ static void riscv_cpu_realize(DeviceState *dev,
> Error **errp)
>  }
>
>  #ifndef CONFIG_USER_ONLY
> +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);
> +    }
> +}
> +
>  static void riscv_cpu_set_irq(void *opaque, int irq, int level)
>  {
>      RISCVCPU *cpu = RISCV_CPU(opaque);
> @@ -1091,6 +1302,10 @@ static void register_cpu_props(Object *obj)
>      for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
>          qdev_property_add_static(dev, prop);
>      }
> +
> +#ifndef CONFIG_USER_ONLY
> +    riscv_add_satp_mode_properties(obj);
> +#endif
>  }
>
>  static Property riscv_cpu_properties[] = {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f5609b62a2..df6d9e06e4 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,10 @@ struct RISCVCPUConfig {
>      bool debug;
>
>      bool short_isa_string;
> +
> +#ifndef CONFIG_USER_ONLY
> +    RISCVSATPMap satp_mode;
> +#endif
>  };
>
>  typedef struct RISCVCPUConfig RISCVCPUConfig;
> @@ -794,9 +810,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
>
>

[-- Attachment #2: Type: text/html, Size: 17183 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities
  2023-02-03  5:58 ` [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
  2023-02-03  7:58   ` Frank Chang
@ 2023-02-05 23:11   ` Alistair Francis
  1 sibling, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2023-02-05 23:11 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	Frank Chang, qemu-riscv, qemu-devel, Bin Meng

On Fri, Feb 3, 2023 at 4:04 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>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 91 +++++++++++++++++++++++++++++++++-------------
>  target/riscv/cpu.h |  8 +++-
>  2 files changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 56057cf87c..7e9924ede9 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -293,18 +293,24 @@ 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;
> +}
>  #endif
>
>  static void riscv_any_cpu_init(Object *obj)
> @@ -315,6 +321,13 @@ static void riscv_any_cpu_init(Object *obj)
>  #elif defined(TARGET_RISCV64)
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>  #endif
> +
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj),
> +            riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
> +                                    VM_1_10_SV32 : VM_1_10_SV57);
> +#endif
> +
>      set_priv_version(env, PRIV_VERSION_1_12_0);
>      register_cpu_props(obj);
>  }
> @@ -328,6 +341,9 @@ static void rv64_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>
>  static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -335,6 +351,9 @@ static void rv64_sifive_u_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
> +#endif
>  }
>
>  static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -345,6 +364,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>
>  static void rv128_base_cpu_init(Object *obj)
> @@ -361,6 +383,9 @@ static void rv128_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>  #else
>  static void rv32_base_cpu_init(Object *obj)
> @@ -371,6 +396,9 @@ static void rv32_base_cpu_init(Object *obj)
>      register_cpu_props(obj);
>      /* Set latest version of privileged specification */
>      set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> +#endif
>  }
>
>  static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -378,6 +406,9 @@ static void rv32_sifive_u_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> +#endif
>  }
>
>  static void rv32_sifive_e_cpu_init(Object *obj)
> @@ -388,6 +419,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>
>  static void rv32_ibex_cpu_init(Object *obj)
> @@ -398,6 +432,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>      cpu->cfg.epmp = true;
>  }
>
> @@ -409,6 +446,9 @@ 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;
> +#ifndef CONFIG_USER_ONLY
> +    set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>  #endif
>
> @@ -701,8 +741,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) {
> @@ -715,9 +756,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;
>                          }
> @@ -728,37 +770,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 df6d9e06e4..defc5769b5 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] 12+ messages in thread

* Re: [PATCH v10 0/5] riscv: Allow user to set the satp mode
  2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
                   ` (4 preceding siblings ...)
  2023-02-03  5:58 ` [PATCH v10 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
@ 2023-03-02 17:42 ` Daniel Henrique Barboza
  2023-03-02 21:03   ` Daniel Henrique Barboza
  5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-03-02 17:42 UTC (permalink / raw)
  To: Alexandre Ghiti, Palmer Dabbelt, Alistair Francis, Bin Meng,
	Andrew Jones, Frank Chang, qemu-riscv, qemu-devel

Hi Palmer,

I think this series can be picked. All patches are fully acked. There is a nit
in patch 3 that I believe you can choose to fix in-tree if you want to.


Thanks,


Daniel




On 2/3/23 02:58, Alexandre Ghiti wrote:
> 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).
> 
> base-commit: commit 75cc28648574 ("configure: remove
> backwards-compatibility code"
> 
> v10:
> - Fix user mode build by surrounding satp handling with #ifndef
>    CONFIG_USER_ONLY, Frank
> - Fix AB/RB from Frank and Alistair
> 
> 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 | 271 +++++++++++++++++++++++++++++++++++++++++++--
>   target/riscv/cpu.h |  25 +++++
>   target/riscv/csr.c |  29 +++--
>   4 files changed, 313 insertions(+), 31 deletions(-)
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 0/5] riscv: Allow user to set the satp mode
  2023-03-02 17:42 ` [PATCH v10 0/5] riscv: Allow user to set the satp mode Daniel Henrique Barboza
@ 2023-03-02 21:03   ` Daniel Henrique Barboza
  2023-03-03 11:52     ` Alexandre Ghiti
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-03-02 21:03 UTC (permalink / raw)
  To: Alexandre Ghiti, Palmer Dabbelt, Alistair Francis, Bin Meng,
	Andrew Jones, Frank Chang, qemu-riscv, qemu-devel



On 3/2/23 14:42, Daniel Henrique Barboza wrote:
> Hi Palmer,
> 
> I think this series can be picked. All patches are fully acked. There is a nit
> in patch 3 that I believe you can choose to fix in-tree if you want to.

Update: patch 1 is not applicable anymore due to changes in current master. All
other patches have conflicts as well.

I guess it's easier to Alexandre to rebase and re-send it when possible. Frank's
comment in patch 3 can also be handled during the process.


Thanks,


Daniel



> 
> 
> Thanks,
> 
> 
> Daniel
> 
> 
> 
> 
> On 2/3/23 02:58, Alexandre Ghiti wrote:
>> 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).
>>
>> base-commit: commit 75cc28648574 ("configure: remove
>> backwards-compatibility code"
>>
>> v10:
>> - Fix user mode build by surrounding satp handling with #ifndef
>>    CONFIG_USER_ONLY, Frank
>> - Fix AB/RB from Frank and Alistair
>>
>> 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 | 271 +++++++++++++++++++++++++++++++++++++++++++--
>>   target/riscv/cpu.h |  25 +++++
>>   target/riscv/csr.c |  29 +++--
>>   4 files changed, 313 insertions(+), 31 deletions(-)
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 0/5] riscv: Allow user to set the satp mode
  2023-03-02 21:03   ` Daniel Henrique Barboza
@ 2023-03-03 11:52     ` Alexandre Ghiti
  0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Ghiti @ 2023-03-03 11:52 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Andrew Jones,
	Frank Chang, qemu-riscv, qemu-devel

Hi Daniel,

On Thu, Mar 2, 2023 at 10:03 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 3/2/23 14:42, Daniel Henrique Barboza wrote:
> > Hi Palmer,
> >
> > I think this series can be picked. All patches are fully acked. There is a nit
> > in patch 3 that I believe you can choose to fix in-tree if you want to.
>
> Update: patch 1 is not applicable anymore due to changes in current master. All
> other patches have conflicts as well.
>
> I guess it's easier to Alexandre to rebase and re-send it when possible. Frank's
> comment in patch 3 can also be handled during the process.

Sure I'll do that today,

Thanks,

Alex

>
>
> Thanks,
>
>
> Daniel
>
>
>
> >
> >
> > Thanks,
> >
> >
> > Daniel
> >
> >
> >
> >
> > On 2/3/23 02:58, Alexandre Ghiti wrote:
> >> 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).
> >>
> >> base-commit: commit 75cc28648574 ("configure: remove
> >> backwards-compatibility code"
> >>
> >> v10:
> >> - Fix user mode build by surrounding satp handling with #ifndef
> >>    CONFIG_USER_ONLY, Frank
> >> - Fix AB/RB from Frank and Alistair
> >>
> >> 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 | 271 +++++++++++++++++++++++++++++++++++++++++++--
> >>   target/riscv/cpu.h |  25 +++++
> >>   target/riscv/csr.c |  29 +++--
> >>   4 files changed, 313 insertions(+), 31 deletions(-)
> >>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-03 11:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-03  5:58 [PATCH v10 0/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-02-03  5:58 ` [PATCH v10 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState Alexandre Ghiti
2023-02-03  5:58 ` [PATCH v10 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool Alexandre Ghiti
2023-02-03  5:58 ` [PATCH v10 3/5] riscv: Allow user to set the satp mode Alexandre Ghiti
2023-02-03  8:01   ` Frank Chang
2023-02-03  5:58 ` [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities Alexandre Ghiti
2023-02-03  7:58   ` Frank Chang
2023-02-05 23:11   ` Alistair Francis
2023-02-03  5:58 ` [PATCH v10 5/5] riscv: Correctly set the device-tree entry 'mmu-type' Alexandre Ghiti
2023-03-02 17:42 ` [PATCH v10 0/5] riscv: Allow user to set the satp mode Daniel Henrique Barboza
2023-03-02 21:03   ` Daniel Henrique Barboza
2023-03-03 11:52     ` Alexandre Ghiti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.