[PATCH v2 0/6] riscv: named features riscv,isa, 'svade' rework

[PATCH v2 0/6] riscv: named features riscv,isa, 'svade' rework

[Andrew Jones]

Hi,

This is a bundle of fixes based on discoveries that were made in the
last week or so:

- what we call "named features" are actually real extensions, which are
  considered to be ratified by the profile spec that defines them. This
  means that we need to add riscv,isa strings for them. More info can be
  found on the commit msg of patch 2;

- the design behind 'svade' and 'svadu' is wrong. 'svade' does not mean
  'we do not have svadu'. In fact they can coexist. Patch 5 gives more
  details about it.


After this series, 'svade' is promoted to a regular extension and all
the named features QEMU implements are now being displayed in riscv,isa.

v2:
 - Ensure svade is off by default even for the max cpu type


Andrew Jones (3):
  target/riscv: Reset henvcfg to zero
  target/riscv: Gate hardware A/D PTE bit updating
  target/riscv: Promote svade to a normal extension

Daniel Henrique Barboza (3):
  target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile()
  target/riscv: add riscv,isa to named features
  target/riscv: add remaining named features

 target/riscv/cpu.c         | 63 ++++++++++++++++++++++++++++----------
 target/riscv/cpu_cfg.h     | 15 +++++++--
 target/riscv/cpu_helper.c  | 18 ++++++++---
 target/riscv/csr.c         |  2 +-
 target/riscv/tcg/tcg-cpu.c | 48 +++++++++++++++++++----------
 5 files changed, 105 insertions(+), 41 deletions(-)

-- 
2.43.0

[PATCH v2 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile()

[Andrew Jones]

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Recent changes in options handling removed the 'mmu' default the bare
CPUs had, meaning that we must enable 'mmu' by hand when using the
rva22s64 profile CPU.

Given that this profile is setting a satp mode, it already implies that
we need a 'mmu'. Enable the 'mmu' in this case.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/tcg/tcg-cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index da437975b429..88f92d1c7d2c 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1107,6 +1107,7 @@ static void cpu_set_profile(Object *obj, Visitor *v, const char *name,
 
 #ifndef CONFIG_USER_ONLY
     if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
+        object_property_set_bool(obj, "mmu", true, NULL);
         const char *satp_prop = satp_mode_str(profile->satp_mode,
                                               riscv_cpu_is_32bit(cpu));
         object_property_set_bool(obj, satp_prop, profile->enabled, NULL);
-- 
2.43.0

[PATCH v2 2/6] target/riscv: add riscv,isa to named features

[Andrew Jones]

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Further discussions after the introduction of rva22 support in QEMU
revealed that what we've been calling 'named features' are actually
regular extensions, with their respective riscv,isa DTs. This is
clarified in [1]. [2] is a bug tracker asking for the profile spec to be
less cryptic about it.

As far as QEMU goes we understand extensions as something that the user
can enable/disable in the command line. This isn't the case for named
features, so we'll have to reach a middle ground.

We'll keep our existing nomenclature 'named features' to refer to any
extension that the user can't control in the command line. We'll also do
the following:

- 'svade' and 'zic64b' flags are renamed to 'ext_svade' and
  'ext_zic64b'. 'ext_svade' and 'ext_zic64b' now have riscv,isa strings and
  priv_spec versions;

- skip name feature check in cpu_bump_multi_ext_priv_ver(). Now that
  named features have a riscv,isa and an entry in isa_edata_arr[] we
  don't need to gate the call to cpu_cfg_ext_get_min_version() anymore.

[1] https://github.com/riscv/riscv-profiles/issues/121
[2] https://github.com/riscv/riscv-profiles/issues/142

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c         | 17 +++++++++++++----
 target/riscv/cpu_cfg.h     |  6 ++++--
 target/riscv/tcg/tcg-cpu.c | 16 ++++++----------
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 88e8cc868144..28d3cfa8ce59 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -97,6 +97,7 @@ bool riscv_cpu_option_set(const char *optname)
  * instead.
  */
 const RISCVIsaExtData isa_edata_arr[] = {
+    ISA_EXT_DATA_ENTRY(zic64b, PRIV_VERSION_1_12_0, ext_zic64b),
     ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
     ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
     ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
@@ -171,6 +172,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
     ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
     ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
+    ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
     ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
     ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
     ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
@@ -1510,9 +1512,16 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+/*
+ * 'Named features' is the name we give to extensions that we
+ * don't want to expose to users. They are either immutable
+ * (always enabled/disable) or they'll vary depending on
+ * the resulting CPU state. They have riscv,isa strings
+ * and priv_ver like regular extensions.
+ */
 const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
-    MULTI_EXT_CFG_BOOL("svade", svade, true),
-    MULTI_EXT_CFG_BOOL("zic64b", zic64b, true),
+    MULTI_EXT_CFG_BOOL("svade", ext_svade, true),
+    MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
 
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -2130,7 +2139,7 @@ static RISCVCPUProfile RVA22U64 = {
         CPU_CFG_OFFSET(ext_zicbop), CPU_CFG_OFFSET(ext_zicboz),
 
         /* mandatory named features for this profile */
-        CPU_CFG_OFFSET(zic64b),
+        CPU_CFG_OFFSET(ext_zic64b),
 
         RISCV_PROFILE_EXT_LIST_END
     }
@@ -2161,7 +2170,7 @@ static RISCVCPUProfile RVA22S64 = {
         CPU_CFG_OFFSET(ext_svinval),
 
         /* rva22s64 named features */
-        CPU_CFG_OFFSET(svade),
+        CPU_CFG_OFFSET(ext_svade),
 
         RISCV_PROFILE_EXT_LIST_END
     }
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index e241922f89c4..698f926ab1be 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -117,13 +117,15 @@ struct RISCVCPUConfig {
     bool ext_smepmp;
     bool rvv_ta_all_1s;
     bool rvv_ma_all_1s;
-    bool svade;
-    bool zic64b;
 
     uint32_t mvendorid;
     uint64_t marchid;
     uint64_t mimpid;
 
+    /* Named features  */
+    bool ext_svade;
+    bool ext_zic64b;
+
     /* Vendor-specific custom extensions */
     bool ext_xtheadba;
     bool ext_xtheadbb;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 88f92d1c7d2c..90861cc065e5 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -197,12 +197,12 @@ static bool cpu_cfg_offset_is_named_feat(uint32_t ext_offset)
 static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
 {
     switch (feat_offset) {
-    case CPU_CFG_OFFSET(zic64b):
+    case CPU_CFG_OFFSET(ext_zic64b):
         cpu->cfg.cbom_blocksize = 64;
         cpu->cfg.cbop_blocksize = 64;
         cpu->cfg.cboz_blocksize = 64;
         break;
-    case CPU_CFG_OFFSET(svade):
+    case CPU_CFG_OFFSET(ext_svade):
         cpu->cfg.ext_svadu = false;
         break;
     default:
@@ -219,10 +219,6 @@ static void cpu_bump_multi_ext_priv_ver(CPURISCVState *env,
         return;
     }
 
-    if (cpu_cfg_offset_is_named_feat(ext_offset)) {
-        return;
-    }
-
     ext_priv_ver = cpu_cfg_ext_get_min_version(ext_offset);
 
     if (env->priv_ver < ext_priv_ver) {
@@ -349,11 +345,11 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
 
 static void riscv_cpu_update_named_features(RISCVCPU *cpu)
 {
-    cpu->cfg.zic64b = cpu->cfg.cbom_blocksize == 64 &&
-                      cpu->cfg.cbop_blocksize == 64 &&
-                      cpu->cfg.cboz_blocksize == 64;
+    cpu->cfg.ext_zic64b = cpu->cfg.cbom_blocksize == 64 &&
+                          cpu->cfg.cbop_blocksize == 64 &&
+                          cpu->cfg.cboz_blocksize == 64;
 
-    cpu->cfg.svade = !cpu->cfg.ext_svadu;
+    cpu->cfg.ext_svade = !cpu->cfg.ext_svadu;
 }
 
 static void riscv_cpu_validate_g(RISCVCPU *cpu)
-- 
2.43.0

[PATCH v2 3/6] target/riscv: add remaining named features

[Andrew Jones]

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

The RVA22U64 and RVA22S64 profiles mandates certain extensions that,
until now, we were implying that they were available.

We can't do this anymore since named features also has a riscv,isa
entry.  Let's add them to riscv_cpu_named_features[].

They will also need to be explicitly enabled in both profile
descriptions. TCG will enable the named features it already implements,
other accelerators are free to handle it as they like.

After this patch, here's the riscv,isa from a buildroot using the
'rva22s64' CPU:

 # cat /proc/device-tree/cpus/cpu@0/riscv,isa
rv64imafdc_zic64b_zicbom_zicbop_zicboz_ziccamoa_ziccif_zicclsm_ziccrse_
zicntr_zicsr_zifencei_zihintpause_zihpm_za64rs_zfhmin_zca_zcd_zba_zbb_
zbs_zkt_sscounterenw_sstvala_sstvecd_svade_svinval_svpbmt#

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c         | 41 +++++++++++++++++++++++++++++---------
 target/riscv/cpu_cfg.h     |  9 +++++++++
 target/riscv/tcg/tcg-cpu.c | 19 +++++++++++++++++-
 3 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d3cfa8ce59..1ecd8a57ed02 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -101,6 +101,10 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
     ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
     ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
+    ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, ext_ziccamoa),
+    ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, ext_ziccif),
+    ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, ext_zicclsm),
+    ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, ext_ziccrse),
     ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
     ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
     ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
@@ -109,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
     ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
     ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
+    ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, ext_za64rs),
     ISA_EXT_DATA_ENTRY(zacas, PRIV_VERSION_1_12_0, ext_zacas),
     ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
     ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa),
@@ -170,8 +175,12 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
     ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
     ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
+    ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, ext_ssccptr),
     ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
+    ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, ext_sscounterenw),
     ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
+    ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, ext_sstvala),
+    ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, ext_sstvecd),
     ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
     ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
     ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
@@ -1523,6 +1532,22 @@ const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
     MULTI_EXT_CFG_BOOL("svade", ext_svade, true),
     MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
 
+    /*
+     * cache-related extensions that are always enabled
+     * since QEMU RISC-V does not have a cache model.
+     */
+    MULTI_EXT_CFG_BOOL("za64rs", ext_za64rs, true),
+    MULTI_EXT_CFG_BOOL("ziccif", ext_ziccif, true),
+    MULTI_EXT_CFG_BOOL("ziccrse", ext_ziccrse, true),
+    MULTI_EXT_CFG_BOOL("ziccamoa", ext_ziccamoa, true),
+    MULTI_EXT_CFG_BOOL("zicclsm", ext_zicclsm, true),
+    MULTI_EXT_CFG_BOOL("ssccptr", ext_ssccptr, true),
+
+    /* Other named features that QEMU TCG always implements */
+    MULTI_EXT_CFG_BOOL("sstvecd", ext_sstvecd, true),
+    MULTI_EXT_CFG_BOOL("sstvala", ext_sstvala, true),
+    MULTI_EXT_CFG_BOOL("sscounterenw", ext_sscounterenw, true),
+
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2116,13 +2141,8 @@ static const PropertyInfo prop_marchid = {
 };
 
 /*
- * RVA22U64 defines some 'named features' or 'synthetic extensions'
- * that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
- * and Zicclsm. We do not implement caching in QEMU so we'll consider
- * all these named features as always enabled.
- *
- * There's no riscv,isa update for them (nor for zic64b, despite it
- * having a cfg offset) at this moment.
+ * RVA22U64 defines some cache related extensions: Za64rs,
+ * Ziccif, Ziccrse, Ziccamoa and Zicclsm.
  */
 static RISCVCPUProfile RVA22U64 = {
     .parent = NULL,
@@ -2139,7 +2159,9 @@ static RISCVCPUProfile RVA22U64 = {
         CPU_CFG_OFFSET(ext_zicbop), CPU_CFG_OFFSET(ext_zicboz),
 
         /* mandatory named features for this profile */
-        CPU_CFG_OFFSET(ext_zic64b),
+        CPU_CFG_OFFSET(ext_za64rs), CPU_CFG_OFFSET(ext_zic64b),
+        CPU_CFG_OFFSET(ext_ziccif), CPU_CFG_OFFSET(ext_ziccrse),
+        CPU_CFG_OFFSET(ext_ziccamoa), CPU_CFG_OFFSET(ext_zicclsm),
 
         RISCV_PROFILE_EXT_LIST_END
     }
@@ -2170,7 +2192,8 @@ static RISCVCPUProfile RVA22S64 = {
         CPU_CFG_OFFSET(ext_svinval),
 
         /* rva22s64 named features */
-        CPU_CFG_OFFSET(ext_svade),
+        CPU_CFG_OFFSET(ext_sstvecd), CPU_CFG_OFFSET(ext_sstvala),
+        CPU_CFG_OFFSET(ext_sscounterenw), CPU_CFG_OFFSET(ext_svade),
 
         RISCV_PROFILE_EXT_LIST_END
     }
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 698f926ab1be..f79fc3dfd199 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -125,6 +125,15 @@ struct RISCVCPUConfig {
     /* Named features  */
     bool ext_svade;
     bool ext_zic64b;
+    bool ext_za64rs;
+    bool ext_ziccif;
+    bool ext_ziccrse;
+    bool ext_ziccamoa;
+    bool ext_zicclsm;
+    bool ext_ssccptr;
+    bool ext_sstvecd;
+    bool ext_sstvala;
+    bool ext_sscounterenw;
 
     /* Vendor-specific custom extensions */
     bool ext_xtheadba;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 90861cc065e5..6d5028cf84d0 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -206,7 +206,8 @@ static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
         cpu->cfg.ext_svadu = false;
         break;
     default:
-        g_assert_not_reached();
+        /* Named feature already enabled in riscv_tcg_cpu_instance_init */
+        return;
     }
 }
 
@@ -1342,6 +1343,20 @@ static bool riscv_cpu_has_max_extensions(Object *cpu_obj)
     return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
 }
 
+/* Named features that TCG always implements */
+static void riscv_tcg_cpu_enable_named_feats(RISCVCPU *cpu)
+{
+    cpu->cfg.ext_za64rs = true;
+    cpu->cfg.ext_ziccif = true;
+    cpu->cfg.ext_ziccrse = true;
+    cpu->cfg.ext_ziccamoa = true;
+    cpu->cfg.ext_zicclsm = true;
+    cpu->cfg.ext_ssccptr = true;
+    cpu->cfg.ext_sstvecd = true;
+    cpu->cfg.ext_sstvala = true;
+    cpu->cfg.ext_sscounterenw = true;
+}
+
 static void riscv_tcg_cpu_instance_init(CPUState *cs)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
@@ -1354,6 +1369,8 @@ static void riscv_tcg_cpu_instance_init(CPUState *cs)
     if (riscv_cpu_has_max_extensions(obj)) {
         riscv_init_max_cpu_extensions(obj);
     }
+
+    riscv_tcg_cpu_enable_named_feats(cpu);
 }
 
 static void riscv_tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
-- 
2.43.0

[PATCH v2 4/6] target/riscv: Reset henvcfg to zero

[Andrew Jones]

The hypervisor should decide what it wants to enable. Zero all
configuration enable bits on reset.

Also, commit ed67d63798f2 ("target/riscv: Update CSR bits name for
svadu extension") missed one reference to 'hade'. Change it now.

Fixes: 0af3f115e68e ("target/riscv: Add *envcfg.HADE related check in address translation")
Fixes: ed67d63798f2 ("target/riscv: Update CSR bits name for svadu extension")
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c | 3 +--
 target/riscv/csr.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1ecd8a57ed02..7fd433daee74 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -961,8 +961,7 @@ static void riscv_cpu_reset_hold(Object *obj)
 
     env->menvcfg = (cpu->cfg.ext_svpbmt ? MENVCFG_PBMTE : 0) |
                    (cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0);
-    env->henvcfg = (cpu->cfg.ext_svpbmt ? HENVCFG_PBMTE : 0) |
-                   (cpu->cfg.ext_svadu ? HENVCFG_ADUE : 0);
+    env->henvcfg = 0;
 
     /* Initialized default priorities of local interrupts. */
     for (i = 0; i < ARRAY_SIZE(env->miprio); i++) {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d9a010387f72..93f7bc2cb48f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2115,7 +2115,7 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
     /*
      * henvcfg.pbmte is read_only 0 when menvcfg.pbmte = 0
      * henvcfg.stce is read_only 0 when menvcfg.stce = 0
-     * henvcfg.hade is read_only 0 when menvcfg.hade = 0
+     * henvcfg.adue is read_only 0 when menvcfg.adue = 0
      */
     *val = env->henvcfg & (~(HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE) |
                            env->menvcfg);
-- 
2.43.0

[PATCH v2 5/6] target/riscv: Gate hardware A/D PTE bit updating

[Andrew Jones]

Gate hardware A/D PTE bit updating on {m,h}envcfg.ADUE and only
enable menvcfg.ADUE on reset if svade has not been selected. Now
that we also consider svade, we have four possible configurations:

 1) !svade && !svadu
    use hardware updating and there's no way to disable it
    (the default, which maintains past behavior. Maintaining
     the default, even with !svadu is a change that fixes [1])

 2) !svade && svadu
    use hardware updating, but also provide {m,h}envcfg.ADUE,
    allowing software to switch to exception mode
    (being able to switch is a change which fixes [1])

 3) svade && !svadu
    use exception mode and there's no way to switch to hardware
    updating
    (this behavior change fixes [2])

 4) svade && svadu
    use exception mode, but also provide {m,h}envcfg.ADUE,
    allowing software to switch to hardware updating
    (this behavior change fixes [2])

Fixes: 0af3f115e68e ("target/riscv: Add *envcfg.HADE related check in address translation") [1]
Fixes: 48531f5adb2a ("target/riscv: implement svade") [2]
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c         |  2 +-
 target/riscv/cpu_helper.c  | 18 ++++++++++++++----
 target/riscv/tcg/tcg-cpu.c | 16 +++++-----------
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7fd433daee74..a56c2ff91d6d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -960,7 +960,7 @@ static void riscv_cpu_reset_hold(Object *obj)
     env->two_stage_lookup = false;
 
     env->menvcfg = (cpu->cfg.ext_svpbmt ? MENVCFG_PBMTE : 0) |
-                   (cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0);
+                   (!cpu->cfg.ext_svade && cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0);
     env->henvcfg = 0;
 
     /* Initialized default priorities of local interrupts. */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 8da9104da450..9da9758cb4d4 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -907,7 +907,9 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
     }
 
     bool pbmte = env->menvcfg & MENVCFG_PBMTE;
-    bool adue = env->menvcfg & MENVCFG_ADUE;
+    bool svade = riscv_cpu_cfg(env)->ext_svade;
+    bool svadu = riscv_cpu_cfg(env)->ext_svadu;
+    bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
 
     if (first_stage && two_stage && env->virt_enabled) {
         pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
@@ -1082,9 +1084,17 @@ restart:
         return TRANSLATE_FAIL;
     }
 
-    /* If necessary, set accessed and dirty bits. */
-    target_ulong updated_pte = pte | PTE_A |
-                (access_type == MMU_DATA_STORE ? PTE_D : 0);
+    target_ulong updated_pte = pte;
+
+    /*
+     * If ADUE is enabled, set accessed and dirty bits.
+     * Otherwise raise an exception if necessary.
+     */
+    if (adue) {
+        updated_pte |= PTE_A | (access_type == MMU_DATA_STORE ? PTE_D : 0);
+    } else if (!(pte & PTE_A) || (access_type == MMU_DATA_STORE && !(pte & PTE_D))) {
+        return TRANSLATE_FAIL;
+    }
 
     /* Page table updates need to be atomic with MTTCG enabled */
     if (updated_pte != pte && !is_debug) {
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 6d5028cf84d0..bc3c45b11704 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -196,18 +196,14 @@ static bool cpu_cfg_offset_is_named_feat(uint32_t ext_offset)
 
 static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
 {
-    switch (feat_offset) {
-    case CPU_CFG_OFFSET(ext_zic64b):
+     /*
+      * All other named features are already enabled
+      * in riscv_tcg_cpu_instance_init().
+      */
+    if (feat_offset == CPU_CFG_OFFSET(ext_zic64b)) {
         cpu->cfg.cbom_blocksize = 64;
         cpu->cfg.cbop_blocksize = 64;
         cpu->cfg.cboz_blocksize = 64;
-        break;
-    case CPU_CFG_OFFSET(ext_svade):
-        cpu->cfg.ext_svadu = false;
-        break;
-    default:
-        /* Named feature already enabled in riscv_tcg_cpu_instance_init */
-        return;
     }
 }
 
@@ -349,8 +345,6 @@ static void riscv_cpu_update_named_features(RISCVCPU *cpu)
     cpu->cfg.ext_zic64b = cpu->cfg.cbom_blocksize == 64 &&
                           cpu->cfg.cbop_blocksize == 64 &&
                           cpu->cfg.cboz_blocksize == 64;
-
-    cpu->cfg.ext_svade = !cpu->cfg.ext_svadu;
 }
 
 static void riscv_cpu_validate_g(RISCVCPU *cpu)
-- 
2.43.0

[PATCH v2 6/6] target/riscv: Promote svade to a normal extension

[Andrew Jones]

Named features are extensions which don't make sense for users to
control and are therefore not exposed on the command line. However,
svade is an extension which makes sense for users to control, so treat
it like a "normal" extension. The default is false, even for the max
cpu type, since QEMU has always implemented hardware A/D PTE bit
updating, so users must opt into svade (or get it from a CPU type
which enables it by default).

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c         | 8 +++-----
 target/riscv/tcg/tcg-cpu.c | 6 ++++++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a56c2ff91d6d..4ddde2541233 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1421,6 +1421,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
 
     MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
     MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
+    MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
     MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
     MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
     MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
@@ -1528,7 +1529,6 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
  * and priv_ver like regular extensions.
  */
 const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
-    MULTI_EXT_CFG_BOOL("svade", ext_svade, true),
     MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
 
     /*
@@ -2175,8 +2175,6 @@ static RISCVCPUProfile RVA22U64 = {
  * Other named features that we already implement: Sstvecd, Sstvala,
  * Sscounterenw
  *
- * Named features that we need to enable: svade
- *
  * The remaining features/extensions comes from RVA22U64.
  */
 static RISCVCPUProfile RVA22S64 = {
@@ -2188,11 +2186,11 @@ static RISCVCPUProfile RVA22S64 = {
     .ext_offsets = {
         /* rva22s64 exts */
         CPU_CFG_OFFSET(ext_zifencei), CPU_CFG_OFFSET(ext_svpbmt),
-        CPU_CFG_OFFSET(ext_svinval),
+        CPU_CFG_OFFSET(ext_svinval), CPU_CFG_OFFSET(ext_svade),
 
         /* rva22s64 named features */
         CPU_CFG_OFFSET(ext_sstvecd), CPU_CFG_OFFSET(ext_sstvala),
-        CPU_CFG_OFFSET(ext_sscounterenw), CPU_CFG_OFFSET(ext_svade),
+        CPU_CFG_OFFSET(ext_sscounterenw),
 
         RISCV_PROFILE_EXT_LIST_END
     }
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index bc3c45b11704..b93df1725a79 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1314,6 +1314,12 @@ static void riscv_init_max_cpu_extensions(Object *obj)
         isa_ext_update_enabled(cpu, prop->offset, true);
     }
 
+    /*
+     * Some extensions can't be added without backward compatibilty concerns.
+     * Disable those, the user can still opt in to them on the command line.
+     */
+    cpu->cfg.ext_svade = false;
+
     /* set vector version */
     env->vext_ver = VEXT_VERSION_1_00_0;
 
-- 
2.43.0

Re: [PATCH v2 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile()

[Alistair Francis]

On Sat, Jan 27, 2024 at 12:18 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> Recent changes in options handling removed the 'mmu' default the bare
> CPUs had, meaning that we must enable 'mmu' by hand when using the
> rva22s64 profile CPU.
>
> Given that this profile is setting a satp mode, it already implies that
> we need a 'mmu'. Enable the 'mmu' in this case.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/tcg/tcg-cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index da437975b429..88f92d1c7d2c 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -1107,6 +1107,7 @@ static void cpu_set_profile(Object *obj, Visitor *v, const char *name,
>
>  #ifndef CONFIG_USER_ONLY
>      if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
> +        object_property_set_bool(obj, "mmu", true, NULL);
>          const char *satp_prop = satp_mode_str(profile->satp_mode,
>                                                riscv_cpu_is_32bit(cpu));
>          object_property_set_bool(obj, satp_prop, profile->enabled, NULL);
> --
> 2.43.0
>
>

Re: [PATCH v2 2/6] target/riscv: add riscv,isa to named features

[Alistair Francis]

Alistair

On Fri, Jan 26, 2024 at 11:32 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> Further discussions after the introduction of rva22 support in QEMU
> revealed that what we've been calling 'named features' are actually
> regular extensions, with their respective riscv,isa DTs. This is
> clarified in [1]. [2] is a bug tracker asking for the profile spec to be
> less cryptic about it.
>
> As far as QEMU goes we understand extensions as something that the user
> can enable/disable in the command line. This isn't the case for named
> features, so we'll have to reach a middle ground.
>
> We'll keep our existing nomenclature 'named features' to refer to any
> extension that the user can't control in the command line. We'll also do
> the following:
>
> - 'svade' and 'zic64b' flags are renamed to 'ext_svade' and
>   'ext_zic64b'. 'ext_svade' and 'ext_zic64b' now have riscv,isa strings and
>   priv_spec versions;
>
> - skip name feature check in cpu_bump_multi_ext_priv_ver(). Now that
>   named features have a riscv,isa and an entry in isa_edata_arr[] we
>   don't need to gate the call to cpu_cfg_ext_get_min_version() anymore.
>
> [1] https://github.com/riscv/riscv-profiles/issues/121
> [2] https://github.com/riscv/riscv-profiles/issues/142
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/cpu.c         | 17 +++++++++++++----
>  target/riscv/cpu_cfg.h     |  6 ++++--
>  target/riscv/tcg/tcg-cpu.c | 16 ++++++----------
>  3 files changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 88e8cc868144..28d3cfa8ce59 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -97,6 +97,7 @@ bool riscv_cpu_option_set(const char *optname)
>   * instead.
>   */
>  const RISCVIsaExtData isa_edata_arr[] = {
> +    ISA_EXT_DATA_ENTRY(zic64b, PRIV_VERSION_1_12_0, ext_zic64b),
>      ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
>      ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
>      ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
> @@ -171,6 +172,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
>      ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
>      ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
> +    ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
>      ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
>      ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
>      ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
> @@ -1510,9 +1512,16 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> +/*
> + * 'Named features' is the name we give to extensions that we
> + * don't want to expose to users. They are either immutable
> + * (always enabled/disable) or they'll vary depending on
> + * the resulting CPU state. They have riscv,isa strings
> + * and priv_ver like regular extensions.
> + */
>  const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
> -    MULTI_EXT_CFG_BOOL("svade", svade, true),
> -    MULTI_EXT_CFG_BOOL("zic64b", zic64b, true),
> +    MULTI_EXT_CFG_BOOL("svade", ext_svade, true),
> +    MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
>
>      DEFINE_PROP_END_OF_LIST(),
>  };
> @@ -2130,7 +2139,7 @@ static RISCVCPUProfile RVA22U64 = {
>          CPU_CFG_OFFSET(ext_zicbop), CPU_CFG_OFFSET(ext_zicboz),
>
>          /* mandatory named features for this profile */
> -        CPU_CFG_OFFSET(zic64b),
> +        CPU_CFG_OFFSET(ext_zic64b),
>
>          RISCV_PROFILE_EXT_LIST_END
>      }
> @@ -2161,7 +2170,7 @@ static RISCVCPUProfile RVA22S64 = {
>          CPU_CFG_OFFSET(ext_svinval),
>
>          /* rva22s64 named features */
> -        CPU_CFG_OFFSET(svade),
> +        CPU_CFG_OFFSET(ext_svade),
>
>          RISCV_PROFILE_EXT_LIST_END
>      }
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index e241922f89c4..698f926ab1be 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -117,13 +117,15 @@ struct RISCVCPUConfig {
>      bool ext_smepmp;
>      bool rvv_ta_all_1s;
>      bool rvv_ma_all_1s;
> -    bool svade;
> -    bool zic64b;
>
>      uint32_t mvendorid;
>      uint64_t marchid;
>      uint64_t mimpid;
>
> +    /* Named features  */
> +    bool ext_svade;
> +    bool ext_zic64b;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_xtheadba;
>      bool ext_xtheadbb;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 88f92d1c7d2c..90861cc065e5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -197,12 +197,12 @@ static bool cpu_cfg_offset_is_named_feat(uint32_t ext_offset)
>  static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
>  {
>      switch (feat_offset) {
> -    case CPU_CFG_OFFSET(zic64b):
> +    case CPU_CFG_OFFSET(ext_zic64b):
>          cpu->cfg.cbom_blocksize = 64;
>          cpu->cfg.cbop_blocksize = 64;
>          cpu->cfg.cboz_blocksize = 64;
>          break;
> -    case CPU_CFG_OFFSET(svade):
> +    case CPU_CFG_OFFSET(ext_svade):
>          cpu->cfg.ext_svadu = false;
>          break;
>      default:
> @@ -219,10 +219,6 @@ static void cpu_bump_multi_ext_priv_ver(CPURISCVState *env,
>          return;
>      }
>
> -    if (cpu_cfg_offset_is_named_feat(ext_offset)) {
> -        return;
> -    }
> -
>      ext_priv_ver = cpu_cfg_ext_get_min_version(ext_offset);
>
>      if (env->priv_ver < ext_priv_ver) {
> @@ -349,11 +345,11 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>
>  static void riscv_cpu_update_named_features(RISCVCPU *cpu)
>  {
> -    cpu->cfg.zic64b = cpu->cfg.cbom_blocksize == 64 &&
> -                      cpu->cfg.cbop_blocksize == 64 &&
> -                      cpu->cfg.cboz_blocksize == 64;
> +    cpu->cfg.ext_zic64b = cpu->cfg.cbom_blocksize == 64 &&
> +                          cpu->cfg.cbop_blocksize == 64 &&
> +                          cpu->cfg.cboz_blocksize == 64;
>
> -    cpu->cfg.svade = !cpu->cfg.ext_svadu;
> +    cpu->cfg.ext_svade = !cpu->cfg.ext_svadu;
>  }
>
>  static void riscv_cpu_validate_g(RISCVCPU *cpu)
> --
> 2.43.0
>
>

Re: [PATCH v2 3/6] target/riscv: add remaining named features

[Alistair Francis]

On Fri, Jan 26, 2024 at 11:33 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> The RVA22U64 and RVA22S64 profiles mandates certain extensions that,
> until now, we were implying that they were available.
>
> We can't do this anymore since named features also has a riscv,isa
> entry.  Let's add them to riscv_cpu_named_features[].
>
> They will also need to be explicitly enabled in both profile
> descriptions. TCG will enable the named features it already implements,
> other accelerators are free to handle it as they like.
>
> After this patch, here's the riscv,isa from a buildroot using the
> 'rva22s64' CPU:
>
>  # cat /proc/device-tree/cpus/cpu@0/riscv,isa
> rv64imafdc_zic64b_zicbom_zicbop_zicboz_ziccamoa_ziccif_zicclsm_ziccrse_
> zicntr_zicsr_zifencei_zihintpause_zihpm_za64rs_zfhmin_zca_zcd_zba_zbb_
> zbs_zkt_sscounterenw_sstvala_sstvecd_svade_svinval_svpbmt#
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  target/riscv/cpu.c         | 41 +++++++++++++++++++++++++++++---------
>  target/riscv/cpu_cfg.h     |  9 +++++++++
>  target/riscv/tcg/tcg-cpu.c | 19 +++++++++++++++++-
>  3 files changed, 59 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 28d3cfa8ce59..1ecd8a57ed02 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -101,6 +101,10 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
>      ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
>      ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
> +    ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, ext_ziccamoa),
> +    ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, ext_ziccif),
> +    ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, ext_zicclsm),
> +    ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, ext_ziccrse),
>      ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
>      ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
>      ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
> @@ -109,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
>      ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
>      ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
> +    ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, ext_za64rs),
>      ISA_EXT_DATA_ENTRY(zacas, PRIV_VERSION_1_12_0, ext_zacas),
>      ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
>      ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa),
> @@ -170,8 +175,12 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
>      ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
>      ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
> +    ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, ext_ssccptr),
>      ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
> +    ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, ext_sscounterenw),
>      ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
> +    ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, ext_sstvala),
> +    ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, ext_sstvecd),
>      ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
>      ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
>      ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
> @@ -1523,6 +1532,22 @@ const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
>      MULTI_EXT_CFG_BOOL("svade", ext_svade, true),
>      MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
>
> +    /*
> +     * cache-related extensions that are always enabled
> +     * since QEMU RISC-V does not have a cache model.
> +     */
> +    MULTI_EXT_CFG_BOOL("za64rs", ext_za64rs, true),
> +    MULTI_EXT_CFG_BOOL("ziccif", ext_ziccif, true),
> +    MULTI_EXT_CFG_BOOL("ziccrse", ext_ziccrse, true),
> +    MULTI_EXT_CFG_BOOL("ziccamoa", ext_ziccamoa, true),
> +    MULTI_EXT_CFG_BOOL("zicclsm", ext_zicclsm, true),
> +    MULTI_EXT_CFG_BOOL("ssccptr", ext_ssccptr, true),
> +
> +    /* Other named features that QEMU TCG always implements */
> +    MULTI_EXT_CFG_BOOL("sstvecd", ext_sstvecd, true),
> +    MULTI_EXT_CFG_BOOL("sstvala", ext_sstvala, true),
> +    MULTI_EXT_CFG_BOOL("sscounterenw", ext_sscounterenw, true),
> +
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -2116,13 +2141,8 @@ static const PropertyInfo prop_marchid = {
>  };
>
>  /*
> - * RVA22U64 defines some 'named features' or 'synthetic extensions'
> - * that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
> - * and Zicclsm. We do not implement caching in QEMU so we'll consider
> - * all these named features as always enabled.
> - *
> - * There's no riscv,isa update for them (nor for zic64b, despite it
> - * having a cfg offset) at this moment.
> + * RVA22U64 defines some cache related extensions: Za64rs,
> + * Ziccif, Ziccrse, Ziccamoa and Zicclsm.
>   */
>  static RISCVCPUProfile RVA22U64 = {
>      .parent = NULL,
> @@ -2139,7 +2159,9 @@ static RISCVCPUProfile RVA22U64 = {
>          CPU_CFG_OFFSET(ext_zicbop), CPU_CFG_OFFSET(ext_zicboz),
>
>          /* mandatory named features for this profile */
> -        CPU_CFG_OFFSET(ext_zic64b),
> +        CPU_CFG_OFFSET(ext_za64rs), CPU_CFG_OFFSET(ext_zic64b),
> +        CPU_CFG_OFFSET(ext_ziccif), CPU_CFG_OFFSET(ext_ziccrse),
> +        CPU_CFG_OFFSET(ext_ziccamoa), CPU_CFG_OFFSET(ext_zicclsm),
>
>          RISCV_PROFILE_EXT_LIST_END
>      }
> @@ -2170,7 +2192,8 @@ static RISCVCPUProfile RVA22S64 = {
>          CPU_CFG_OFFSET(ext_svinval),
>
>          /* rva22s64 named features */
> -        CPU_CFG_OFFSET(ext_svade),
> +        CPU_CFG_OFFSET(ext_sstvecd), CPU_CFG_OFFSET(ext_sstvala),
> +        CPU_CFG_OFFSET(ext_sscounterenw), CPU_CFG_OFFSET(ext_svade),
>
>          RISCV_PROFILE_EXT_LIST_END
>      }
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 698f926ab1be..f79fc3dfd199 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -125,6 +125,15 @@ struct RISCVCPUConfig {
>      /* Named features  */
>      bool ext_svade;
>      bool ext_zic64b;
> +    bool ext_za64rs;
> +    bool ext_ziccif;
> +    bool ext_ziccrse;
> +    bool ext_ziccamoa;
> +    bool ext_zicclsm;
> +    bool ext_ssccptr;
> +    bool ext_sstvecd;
> +    bool ext_sstvala;
> +    bool ext_sscounterenw;

Daniel and I have discussed this a bit before. This doesn't feel
right. We are adding variables that must always be true. To me this
seems confusing to anyone reading the code in the future. I understand
it isn't exposed to users, but it still feels odd.

I guess it's nice that we reuse the same logic and this is in some
ways cleaner than an "always_true", but it just seems clunky.
Especially as this list seems to be growing very quickly.

>
>      /* Vendor-specific custom extensions */
>      bool ext_xtheadba;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 90861cc065e5..6d5028cf84d0 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -206,7 +206,8 @@ static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
>          cpu->cfg.ext_svadu = false;
>          break;
>      default:
> -        g_assert_not_reached();
> +        /* Named feature already enabled in riscv_tcg_cpu_instance_init */
> +        return;
>      }
>  }
>
> @@ -1342,6 +1343,20 @@ static bool riscv_cpu_has_max_extensions(Object *cpu_obj)
>      return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
>  }
>
> +/* Named features that TCG always implements */
> +static void riscv_tcg_cpu_enable_named_feats(RISCVCPU *cpu)
> +{
> +    cpu->cfg.ext_za64rs = true;
> +    cpu->cfg.ext_ziccif = true;
> +    cpu->cfg.ext_ziccrse = true;
> +    cpu->cfg.ext_ziccamoa = true;
> +    cpu->cfg.ext_zicclsm = true;
> +    cpu->cfg.ext_ssccptr = true;
> +    cpu->cfg.ext_sstvecd = true;
> +    cpu->cfg.ext_sstvala = true;
> +    cpu->cfg.ext_sscounterenw = true;

Maybe we should assert that these are all true somewhere in the CPU
init/realise/reset. That at least indicates to someone using the code
that these can't actually be disabled.

@Daniel Henrique Barboza that might be a reasonable compromise as I
know I complained to you about the same thing. Unless you have
implemented something really clean and nice :)

Alistair

> +}
> +
>  static void riscv_tcg_cpu_instance_init(CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -1354,6 +1369,8 @@ static void riscv_tcg_cpu_instance_init(CPUState *cs)
>      if (riscv_cpu_has_max_extensions(obj)) {
>          riscv_init_max_cpu_extensions(obj);
>      }
> +
> +    riscv_tcg_cpu_enable_named_feats(cpu);
>  }
>
>  static void riscv_tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
> --
> 2.43.0
>
>