* [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework
@ 2024-01-25 19:53 Daniel Henrique Barboza
2024-01-25 19:53 ` [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() Daniel Henrique Barboza
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
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.
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 | 42 +++++++++++++++----------
5 files changed, 99 insertions(+), 41 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-30 0:57 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 2/6] target/riscv: add riscv,isa to named features Daniel Henrique Barboza ` (4 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza 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 da437975b4..88f92d1c7d 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() 2024-01-25 19:53 ` [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() Daniel Henrique Barboza @ 2024-01-30 0:57 ` Alistair Francis 0 siblings, 0 replies; 15+ messages in thread From: Alistair Francis @ 2024-01-30 0:57 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > 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 da437975b4..88f92d1c7d 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] target/riscv: add riscv,isa to named features 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-30 1:07 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 3/6] target/riscv: add remaining " Daniel Henrique Barboza ` (3 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza 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> --- 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 88e8cc8681..28d3cfa8ce 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 e241922f89..698f926ab1 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 88f92d1c7d..90861cc065 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] target/riscv: add riscv,isa to named features 2024-01-25 19:53 ` [PATCH 2/6] target/riscv: add riscv,isa to named features Daniel Henrique Barboza @ 2024-01-30 1:07 ` Alistair Francis 0 siblings, 0 replies; 15+ messages in thread From: Alistair Francis @ 2024-01-30 1:07 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On Fri, Jan 26, 2024 at 6:55 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > 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: 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 88e8cc8681..28d3cfa8ce 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 e241922f89..698f926ab1 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 88f92d1c7d..90861cc065 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] target/riscv: add remaining named features 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 2/6] target/riscv: add riscv,isa to named features Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-30 1:10 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 4/6] target/riscv: Reset henvcfg to zero Daniel Henrique Barboza ` (2 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza 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> --- 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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 90861cc065..6d5028cf84 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/riscv: add remaining named features 2024-01-25 19:53 ` [PATCH 3/6] target/riscv: add remaining " Daniel Henrique Barboza @ 2024-01-30 1:10 ` Alistair Francis 2024-01-31 19:15 ` Daniel Henrique Barboza 0 siblings, 1 reply; 15+ messages in thread From: Alistair Francis @ 2024-01-30 1:10 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > 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> > --- > 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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; Overall this and the previous patch look fine. One thing though, why store this information? I feel it could be confusing having these variables. If a developer sets them to false nothing actually happens, which is a little misleading Alistair > > /* Vendor-specific custom extensions */ > bool ext_xtheadba; > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 90861cc065..6d5028cf84 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/riscv: add remaining named features 2024-01-30 1:10 ` Alistair Francis @ 2024-01-31 19:15 ` Daniel Henrique Barboza 2024-02-02 2:14 ` Alistair Francis 0 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-31 19:15 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On 1/29/24 22:10, Alistair Francis wrote: > On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> >> 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> >> --- >> 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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; > > Overall this and the previous patch look fine. > > One thing though, why store this information? I feel it could be > confusing having these variables. If a developer sets them to false > nothing actually happens, which is a little misleading These extensions aren't being exposed to users. riscv_cpu_named_features[] isn't being used to create any CPU user properties. I should've mentioned that in patch 2 ... As for the extra booleans that we'll be setting to 'true', as it is now isa_edata_arr[] stores a string name, priv_ver and a cpu->cfg offset, so everyone that adds a riscv,isa str must also have a valid bool offset in RISCVCPUConfig. Having a bool also allow us to treat them as regular extensions because we can re-use existing code to blindly enable them in profiles like any other profile extension. And, in case we need to promote them as regular user extensions, having the booleans in place make it easier to do so. Patch 6 is doing that with 'svade'. Thanks, Daniel We could create a single boolean that is always true in cpu->cfg and use it for these entries. Another idea would be to change the riscv,isa functions to handle these extensions separately, then we can add them in the array without a valid cpu->cfg offfset. > > Alistair > >> >> /* Vendor-specific custom extensions */ >> bool ext_xtheadba; >> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c >> index 90861cc065..6d5028cf84 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 >> >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/riscv: add remaining named features 2024-01-31 19:15 ` Daniel Henrique Barboza @ 2024-02-02 2:14 ` Alistair Francis 2024-02-02 9:44 ` Daniel Henrique Barboza 0 siblings, 1 reply; 15+ messages in thread From: Alistair Francis @ 2024-02-02 2:14 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On Thu, Feb 1, 2024 at 5:15 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 1/29/24 22:10, Alistair Francis wrote: > > On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza > > <dbarboza@ventanamicro.com> wrote: > >> > >> 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> > >> --- > >> 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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; > > > > Overall this and the previous patch look fine. > > > > One thing though, why store this information? I feel it could be > > confusing having these variables. If a developer sets them to false > > nothing actually happens, which is a little misleading > > These extensions aren't being exposed to users. riscv_cpu_named_features[] isn't Yep, we should not expose them to users. I meant developers, as in people reading the C code and compiling QEMU. > being used to create any CPU user properties. I should've mentioned that in > patch 2 ... > > As for the extra booleans that we'll be setting to 'true', as it is now > isa_edata_arr[] stores a string name, priv_ver and a cpu->cfg offset, so > everyone that adds a riscv,isa str must also have a valid bool offset in > RISCVCPUConfig. Having a bool also allow us to treat them as regular extensions > because we can re-use existing code to blindly enable them in profiles like > any other profile extension. Yep, and I think that makes sense. The odd part is that we don't actually use these bools. Which I feel is confusing when looking at the QEMU code base. Why have a variable that we don't use? > > And, in case we need to promote them as regular user extensions, having the > booleans in place make it easier to do so. Patch 6 is doing that with 'svade'. If we plan to do that, I think that also makes sense. But I suspect no one has any plant to convert some of these to real configuration options. > > > Thanks, > > Daniel > > > We could create a single boolean that is always true in cpu->cfg and use it > for these entries. Another idea would be to change the riscv,isa functions to > handle these extensions separately, then we can add them in the array without > a valid cpu->cfg offfset. I think these are better ideas. Alistair > > > > > Alistair > > > >> > >> /* Vendor-specific custom extensions */ > >> bool ext_xtheadba; > >> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > >> index 90861cc065..6d5028cf84 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 > >> > >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/riscv: add remaining named features 2024-02-02 2:14 ` Alistair Francis @ 2024-02-02 9:44 ` Daniel Henrique Barboza 2024-02-05 6:16 ` Alistair Francis 0 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-02-02 9:44 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On 2/1/24 23:14, Alistair Francis wrote: > On Thu, Feb 1, 2024 at 5:15 AM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> >> >> >> On 1/29/24 22:10, Alistair Francis wrote: >>> On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza >>> <dbarboza@ventanamicro.com> wrote: >>>> >>>> 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> >>>> --- >>>> 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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; >>> >>> Overall this and the previous patch look fine. >>> >>> One thing though, why store this information? I feel it could be >>> confusing having these variables. If a developer sets them to false >>> nothing actually happens, which is a little misleading >> >> These extensions aren't being exposed to users. riscv_cpu_named_features[] isn't > > Yep, we should not expose them to users. > > I meant developers, as in people reading the C code and compiling QEMU. > >> being used to create any CPU user properties. I should've mentioned that in >> patch 2 ... >> >> As for the extra booleans that we'll be setting to 'true', as it is now >> isa_edata_arr[] stores a string name, priv_ver and a cpu->cfg offset, so >> everyone that adds a riscv,isa str must also have a valid bool offset in >> RISCVCPUConfig. Having a bool also allow us to treat them as regular extensions >> because we can re-use existing code to blindly enable them in profiles like >> any other profile extension. > > Yep, and I think that makes sense. The odd part is that we don't > actually use these bools. Which I feel is confusing when looking at > the QEMU code base. Why have a variable that we don't use? Let's say that we create an "always_enabled" boolean in cpu->cfg, always set to 'true' during riscv_cpu_init(). Then we change riscv_cpu_named_features[] to work like this: MULTI_EXT_CFG_BOOL("svade", always_enabled, true), MULTI_EXT_CFG_BOOL("zic64b", always_enabled, true), MULTI_EXT_CFG_BOOL("za64rs", always_enabled, true), MULTI_EXT_CFG_BOOL("ziccif", always_enabled, true), (...) (I would create another macro for these named features but that's beside the point) If we go this route then we'll need changes in how profiles declare these internal extensions, because now we're using CPU_CFG_OFFSET() with a specific boolean for each. I would remove them from the profile description, otherwise the code would attempt to do stuff with an 'always_enabled' bool, and instead just cite in a comment that these other profile mandatory extensions are always enabled. Another change on top of my head would be in isa_edata_arr[]. In this case we would do: >>>> + ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, always_enabled), >>>> + ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, always_enabled), >>>> + ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, always_enabled), >>>> + ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, always_enabled), There might be other changes needed but on top of my head I think this would work. Is that what you're referring to? Thanks, Daniel > >> >> And, in case we need to promote them as regular user extensions, having the >> booleans in place make it easier to do so. Patch 6 is doing that with 'svade'. > > If we plan to do that, I think that also makes sense. But I suspect no > one has any plant to convert some of these to real configuration > options. > >> >> >> Thanks, >> >> Daniel >> >> >> We could create a single boolean that is always true in cpu->cfg and use it >> for these entries. Another idea would be to change the riscv,isa functions to >> handle these extensions separately, then we can add them in the array without >> a valid cpu->cfg offfset. > > I think these are better ideas. > > Alistair > >> >>> >>> Alistair >>> >>>> >>>> /* Vendor-specific custom extensions */ >>>> bool ext_xtheadba; >>>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c >>>> index 90861cc065..6d5028cf84 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 >>>> >>>> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/riscv: add remaining named features 2024-02-02 9:44 ` Daniel Henrique Barboza @ 2024-02-05 6:16 ` Alistair Francis 0 siblings, 0 replies; 15+ messages in thread From: Alistair Francis @ 2024-02-05 6:16 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones On Fri, Feb 2, 2024 at 7:44 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 2/1/24 23:14, Alistair Francis wrote: > > On Thu, Feb 1, 2024 at 5:15 AM Daniel Henrique Barboza > > <dbarboza@ventanamicro.com> wrote: > >> > >> > >> > >> On 1/29/24 22:10, Alistair Francis wrote: > >>> On Fri, Jan 26, 2024 at 5:54 AM Daniel Henrique Barboza > >>> <dbarboza@ventanamicro.com> wrote: > >>>> > >>>> 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> > >>>> --- > >>>> 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 28d3cfa8ce..1ecd8a57ed 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 698f926ab1..f79fc3dfd1 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; > >>> > >>> Overall this and the previous patch look fine. > >>> > >>> One thing though, why store this information? I feel it could be > >>> confusing having these variables. If a developer sets them to false > >>> nothing actually happens, which is a little misleading > >> > >> These extensions aren't being exposed to users. riscv_cpu_named_features[] isn't > > > > Yep, we should not expose them to users. > > > > I meant developers, as in people reading the C code and compiling QEMU. > > > >> being used to create any CPU user properties. I should've mentioned that in > >> patch 2 ... > >> > >> As for the extra booleans that we'll be setting to 'true', as it is now > >> isa_edata_arr[] stores a string name, priv_ver and a cpu->cfg offset, so > >> everyone that adds a riscv,isa str must also have a valid bool offset in > >> RISCVCPUConfig. Having a bool also allow us to treat them as regular extensions > >> because we can re-use existing code to blindly enable them in profiles like > >> any other profile extension. > > > > Yep, and I think that makes sense. The odd part is that we don't > > actually use these bools. Which I feel is confusing when looking at > > the QEMU code base. Why have a variable that we don't use? > > Let's say that we create an "always_enabled" boolean in cpu->cfg, always set to > 'true' during riscv_cpu_init(). Then we change riscv_cpu_named_features[] to > work like this: > > MULTI_EXT_CFG_BOOL("svade", always_enabled, true), > MULTI_EXT_CFG_BOOL("zic64b", always_enabled, true), > MULTI_EXT_CFG_BOOL("za64rs", always_enabled, true), > MULTI_EXT_CFG_BOOL("ziccif", always_enabled, true), > (...) > > (I would create another macro for these named features but that's beside the point) > > If we go this route then we'll need changes in how profiles declare these internal > extensions, because now we're using CPU_CFG_OFFSET() with a specific boolean for > each. I would remove them from the profile description, otherwise the code would > attempt to do stuff with an 'always_enabled' bool, and instead just cite in a > comment that these other profile mandatory extensions are always enabled. > > > Another change on top of my head would be in isa_edata_arr[]. In this case we > would do: > > > >>>> + ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, always_enabled), > >>>> + ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, always_enabled), > >>>> + ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, always_enabled), > >>>> + ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, always_enabled), > > > There might be other changes needed but on top of my head I think this would work. > Is that what you're referring to? I would prefer your other offer: > Another idea would be to change the riscv,isa functions to > handle these extensions separately, then we can add them in the array without > a valid cpu->cfg offfset. but this also works. Alistair > > > Thanks, > > Daniel > > > > >> > >> And, in case we need to promote them as regular user extensions, having the > >> booleans in place make it easier to do so. Patch 6 is doing that with 'svade'. > > > > If we plan to do that, I think that also makes sense. But I suspect no > > one has any plant to convert some of these to real configuration > > options. > > > >> > >> > >> Thanks, > >> > >> Daniel > >> > >> > >> We could create a single boolean that is always true in cpu->cfg and use it > >> for these entries. Another idea would be to change the riscv,isa functions to > >> handle these extensions separately, then we can add them in the array without > >> a valid cpu->cfg offfset. > > > > I think these are better ideas. > > > > Alistair > > > >> > >>> > >>> Alistair > >>> > >>>> > >>>> /* Vendor-specific custom extensions */ > >>>> bool ext_xtheadba; > >>>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > >>>> index 90861cc065..6d5028cf84 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 > >>>> > >>>> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] target/riscv: Reset henvcfg to zero 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza ` (2 preceding siblings ...) 2024-01-25 19:53 ` [PATCH 3/6] target/riscv: add remaining " Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 5/6] target/riscv: Gate hardware A/D PTE bit updating Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 6/6] target/riscv: Promote svade to a normal extension Daniel Henrique Barboza 5 siblings, 0 replies; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza From: Andrew Jones <ajones@ventanamicro.com> 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") Signed-off-by: Andrew Jones <ajones@ventanamicro.com> Reviewed-by: Daniel Henrique Barboza <dbarboza@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 1ecd8a57ed..7fd433daee 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 d9a010387f..93f7bc2cb4 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] target/riscv: Gate hardware A/D PTE bit updating 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza ` (3 preceding siblings ...) 2024-01-25 19:53 ` [PATCH 4/6] target/riscv: Reset henvcfg to zero Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 6/6] target/riscv: Promote svade to a normal extension Daniel Henrique Barboza 5 siblings, 0 replies; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza From: Andrew Jones <ajones@ventanamicro.com> 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] Signed-off-by: Andrew Jones <ajones@ventanamicro.com> Reviewed-by: Daniel Henrique Barboza <dbarboza@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 7fd433daee..a56c2ff91d 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 8da9104da4..9da9758cb4 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 6d5028cf84..bc3c45b117 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] target/riscv: Promote svade to a normal extension 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza ` (4 preceding siblings ...) 2024-01-25 19:53 ` [PATCH 5/6] target/riscv: Gate hardware A/D PTE bit updating Daniel Henrique Barboza @ 2024-01-25 19:53 ` Daniel Henrique Barboza 2024-01-26 13:03 ` Andrew Jones 5 siblings, 1 reply; 15+ messages in thread From: Daniel Henrique Barboza @ 2024-01-25 19:53 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer, ajones, Daniel Henrique Barboza From: Andrew Jones <ajones@ventanamicro.com> 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, 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). Signed-off-by: Andrew Jones <ajones@ventanamicro.com> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index a56c2ff91d..4ddde25412 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 } -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] target/riscv: Promote svade to a normal extension 2024-01-25 19:53 ` [PATCH 6/6] target/riscv: Promote svade to a normal extension Daniel Henrique Barboza @ 2024-01-26 13:03 ` Andrew Jones 0 siblings, 0 replies; 15+ messages in thread From: Andrew Jones @ 2024-01-26 13:03 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu, palmer On Thu, Jan 25, 2024 at 04:53:19PM -0300, Daniel Henrique Barboza wrote: > From: Andrew Jones <ajones@ventanamicro.com> > > 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, 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). > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index a56c2ff91d..4ddde25412 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), I forgot that the 'max' cpu type will ignore this off by default setting and enable svade by default. I'll send a v2 of this series where I ensure svade for 'max' also defaults false. Thanks, drew > 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 > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-02-05 6:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-25 19:53 [PATCH 0/6] riscv: named features riscv,isa, 'svade' rework Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 1/6] target/riscv/tcg: set 'mmu' with 'satp' in cpu_set_profile() Daniel Henrique Barboza 2024-01-30 0:57 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 2/6] target/riscv: add riscv,isa to named features Daniel Henrique Barboza 2024-01-30 1:07 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 3/6] target/riscv: add remaining " Daniel Henrique Barboza 2024-01-30 1:10 ` Alistair Francis 2024-01-31 19:15 ` Daniel Henrique Barboza 2024-02-02 2:14 ` Alistair Francis 2024-02-02 9:44 ` Daniel Henrique Barboza 2024-02-05 6:16 ` Alistair Francis 2024-01-25 19:53 ` [PATCH 4/6] target/riscv: Reset henvcfg to zero Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 5/6] target/riscv: Gate hardware A/D PTE bit updating Daniel Henrique Barboza 2024-01-25 19:53 ` [PATCH 6/6] target/riscv: Promote svade to a normal extension Daniel Henrique Barboza 2024-01-26 13:03 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).