Re: [PATCH v2 2/3] target/riscv/tcg: decouple profile enablement from user prop
On Thu, Jun 5, 2025 at 3:40 AM Daniel Henrique Barboza
wrote:
>
> We have code in riscv_cpu_add_profiles() to enable a profile right away
> in case a CPU chose the profile during its cpu_init(). But we're using
> the user callback option to do so, setting profile->user_set.
>
> Create a new helper that does all the grunt work to enable/disable a
> given profile. Use this new helper in the cases where we want a CPU to
> be compatible to a certain profile, leaving the user callback to be used
> exclusively by users.
>
> Fixes: fba92a92e3 ("target/riscv: add 'rva22u64' CPU")
> Signed-off-by: Daniel Henrique Barboza
> Reviewed-by: Andrew Jones
> Reviewed-by: Björn Töpel
> Tested-by: Björn Töpel
Acked-by: Alistair Francis
Alistair
> ---
> target/riscv/tcg/tcg-cpu.c | 127 +++--
> 1 file changed, 67 insertions(+), 60 deletions(-)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 857c625580..f8489d79d7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -1140,6 +1140,70 @@ static bool riscv_cpu_is_generic(Object *cpu_obj)
> return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
> }
>
> +static void riscv_cpu_set_profile(RISCVCPU *cpu,
> + RISCVCPUProfile *profile,
> + bool enabled)
> +{
> +int i, ext_offset;
> +
> +if (profile->u_parent != NULL) {
> +riscv_cpu_set_profile(cpu, profile->u_parent, enabled);
> +}
> +
> +if (profile->s_parent != NULL) {
> +riscv_cpu_set_profile(cpu, profile->s_parent, enabled);
> +}
> +
> +profile->enabled = enabled;
> +
> +if (profile->enabled) {
> +cpu->env.priv_ver = profile->priv_spec;
> +
> +#ifndef CONFIG_USER_ONLY
> +if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
> +object_property_set_bool(OBJECT(cpu), "mmu", true, NULL);
> +const char *satp_prop = satp_mode_str(profile->satp_mode,
> + riscv_cpu_is_32bit(cpu));
> +object_property_set_bool(OBJECT(cpu), satp_prop, true, NULL);
> +}
> +#endif
> +}
> +
> +for (i = 0; misa_bits[i] != 0; i++) {
> +uint32_t bit = misa_bits[i];
> +
> +if (!(profile->misa_ext & bit)) {
> +continue;
> +}
> +
> +if (bit == RVI && !profile->enabled) {
> +/*
> + * Disabling profiles will not disable the base
> + * ISA RV64I.
> + */
> +continue;
> +}
> +
> +cpu_misa_ext_add_user_opt(bit, profile->enabled);
> +riscv_cpu_write_misa_bit(cpu, bit, profile->enabled);
> +}
> +
> +for (i = 0; profile->ext_offsets[i] != RISCV_PROFILE_EXT_LIST_END; i++) {
> +ext_offset = profile->ext_offsets[i];
> +
> +if (profile->enabled) {
> +if (cpu_cfg_offset_is_named_feat(ext_offset)) {
> +riscv_cpu_enable_named_feat(cpu, ext_offset);
> +}
> +
> +cpu_bump_multi_ext_priv_ver(&cpu->env, ext_offset);
> +}
> +
> +cpu_cfg_ext_add_user_opt(ext_offset, profile->enabled);
> +isa_ext_update_enabled(cpu, ext_offset, profile->enabled);
> +}
> +}
> +
> /*
> * We'll get here via the following path:
> *
> @@ -1306,7 +1370,6 @@ static void cpu_set_profile(Object *obj, Visitor *v,
> const char *name,
> RISCVCPUProfile *profile = opaque;
> RISCVCPU *cpu = RISCV_CPU(obj);
> bool value;
> -int i, ext_offset;
>
> if (riscv_cpu_is_vendor(obj)) {
> error_setg(errp, "Profile %s is not available for vendor CPUs",
> @@ -1325,64 +1388,8 @@ static void cpu_set_profile(Object *obj, Visitor *v,
> const char *name,
> }
>
> profile->user_set = true;
> -profile->enabled = value;
> -
> -if (profile->u_parent != NULL) {
> -object_property_set_bool(obj, profile->u_parent->name,
> - profile->enabled, NULL);
> -}
> -
> -if (profile->s_parent != NULL) {
> -object_property_set_bool(obj, profile->s_parent->name,
> - profile->enabled, NULL);
> -}
> -
> -if (profile->enabled) {
> -cpu->env.priv_ver = profile->priv_spec;
> -
> -#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, true, NULL);
> -}
> -#endif
> -}
> -
> -for (i = 0; misa_bits[i] != 0; i++) {
> -uint32_t bit = misa_bits[i];
> -
> -if (!(profile->misa_ext & bit)) {
> -continue;
> -}
>
> -if (bit == RVI && !profile->enabled) {
> -/*
> -
[PATCH v2 2/3] target/riscv/tcg: decouple profile enablement from user prop
We have code in riscv_cpu_add_profiles() to enable a profile right away
in case a CPU chose the profile during its cpu_init(). But we're using
the user callback option to do so, setting profile->user_set.
Create a new helper that does all the grunt work to enable/disable a
given profile. Use this new helper in the cases where we want a CPU to
be compatible to a certain profile, leaving the user callback to be used
exclusively by users.
Fixes: fba92a92e3 ("target/riscv: add 'rva22u64' CPU")
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Andrew Jones
Reviewed-by: Björn Töpel
Tested-by: Björn Töpel
---
target/riscv/tcg/tcg-cpu.c | 127 +++--
1 file changed, 67 insertions(+), 60 deletions(-)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 857c625580..f8489d79d7 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1140,6 +1140,70 @@ static bool riscv_cpu_is_generic(Object *cpu_obj)
return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
}
+static void riscv_cpu_set_profile(RISCVCPU *cpu,
+ RISCVCPUProfile *profile,
+ bool enabled)
+{
+int i, ext_offset;
+
+if (profile->u_parent != NULL) {
+riscv_cpu_set_profile(cpu, profile->u_parent, enabled);
+}
+
+if (profile->s_parent != NULL) {
+riscv_cpu_set_profile(cpu, profile->s_parent, enabled);
+}
+
+profile->enabled = enabled;
+
+if (profile->enabled) {
+cpu->env.priv_ver = profile->priv_spec;
+
+#ifndef CONFIG_USER_ONLY
+if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
+object_property_set_bool(OBJECT(cpu), "mmu", true, NULL);
+const char *satp_prop = satp_mode_str(profile->satp_mode,
+ riscv_cpu_is_32bit(cpu));
+object_property_set_bool(OBJECT(cpu), satp_prop, true, NULL);
+}
+#endif
+}
+
+for (i = 0; misa_bits[i] != 0; i++) {
+uint32_t bit = misa_bits[i];
+
+if (!(profile->misa_ext & bit)) {
+continue;
+}
+
+if (bit == RVI && !profile->enabled) {
+/*
+ * Disabling profiles will not disable the base
+ * ISA RV64I.
+ */
+continue;
+}
+
+cpu_misa_ext_add_user_opt(bit, profile->enabled);
+riscv_cpu_write_misa_bit(cpu, bit, profile->enabled);
+}
+
+for (i = 0; profile->ext_offsets[i] != RISCV_PROFILE_EXT_LIST_END; i++) {
+ext_offset = profile->ext_offsets[i];
+
+if (profile->enabled) {
+if (cpu_cfg_offset_is_named_feat(ext_offset)) {
+riscv_cpu_enable_named_feat(cpu, ext_offset);
+}
+
+cpu_bump_multi_ext_priv_ver(&cpu->env, ext_offset);
+}
+
+cpu_cfg_ext_add_user_opt(ext_offset, profile->enabled);
+isa_ext_update_enabled(cpu, ext_offset, profile->enabled);
+}
+}
+
/*
* We'll get here via the following path:
*
@@ -1306,7 +1370,6 @@ static void cpu_set_profile(Object *obj, Visitor *v,
const char *name,
RISCVCPUProfile *profile = opaque;
RISCVCPU *cpu = RISCV_CPU(obj);
bool value;
-int i, ext_offset;
if (riscv_cpu_is_vendor(obj)) {
error_setg(errp, "Profile %s is not available for vendor CPUs",
@@ -1325,64 +1388,8 @@ static void cpu_set_profile(Object *obj, Visitor *v,
const char *name,
}
profile->user_set = true;
-profile->enabled = value;
-
-if (profile->u_parent != NULL) {
-object_property_set_bool(obj, profile->u_parent->name,
- profile->enabled, NULL);
-}
-
-if (profile->s_parent != NULL) {
-object_property_set_bool(obj, profile->s_parent->name,
- profile->enabled, NULL);
-}
-
-if (profile->enabled) {
-cpu->env.priv_ver = profile->priv_spec;
-
-#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, true, NULL);
-}
-#endif
-}
-
-for (i = 0; misa_bits[i] != 0; i++) {
-uint32_t bit = misa_bits[i];
-
-if (!(profile->misa_ext & bit)) {
-continue;
-}
-if (bit == RVI && !profile->enabled) {
-/*
- * Disabling profiles will not disable the base
- * ISA RV64I.
- */
-continue;
-}
-
-cpu_misa_ext_add_user_opt(bit, profile->enabled);
-riscv_cpu_write_misa_bit(cpu, bit, profile->enabled);
-}
-
-for (i = 0; profile->ext_offsets[i] != RISCV_PROFILE_EXT_LIST_END; i++) {
-ext_offset = profile->ex
[PATCH v2 2/3] target/riscv/tcg: decouple profile enablement from user prop
We have code in riscv_cpu_add_profiles() to enable a profile right away
in case a CPU chose the profile during its cpu_init(). But we're using
the user callback option to do so, setting profile->user_set.
Create a new helper that does all the grunt work to enable/disable a
given profile. Use this new helper in the cases where we want a CPU to
be compatible to a certain profile, leaving the user callback to be used
exclusively by users.
Fixes: fba92a92e3 ("target/riscv: add 'rva22u64' CPU")
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Andrew Jones
Reviewed-by: Björn Töpel
Tested-by: Björn Töpel
---
target/riscv/tcg/tcg-cpu.c | 127 +++--
1 file changed, 67 insertions(+), 60 deletions(-)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 857c625580..f8489d79d7 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1140,6 +1140,70 @@ static bool riscv_cpu_is_generic(Object *cpu_obj)
return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
}
+static void riscv_cpu_set_profile(RISCVCPU *cpu,
+ RISCVCPUProfile *profile,
+ bool enabled)
+{
+int i, ext_offset;
+
+if (profile->u_parent != NULL) {
+riscv_cpu_set_profile(cpu, profile->u_parent, enabled);
+}
+
+if (profile->s_parent != NULL) {
+riscv_cpu_set_profile(cpu, profile->s_parent, enabled);
+}
+
+profile->enabled = enabled;
+
+if (profile->enabled) {
+cpu->env.priv_ver = profile->priv_spec;
+
+#ifndef CONFIG_USER_ONLY
+if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
+object_property_set_bool(OBJECT(cpu), "mmu", true, NULL);
+const char *satp_prop = satp_mode_str(profile->satp_mode,
+ riscv_cpu_is_32bit(cpu));
+object_property_set_bool(OBJECT(cpu), satp_prop, true, NULL);
+}
+#endif
+}
+
+for (i = 0; misa_bits[i] != 0; i++) {
+uint32_t bit = misa_bits[i];
+
+if (!(profile->misa_ext & bit)) {
+continue;
+}
+
+if (bit == RVI && !profile->enabled) {
+/*
+ * Disabling profiles will not disable the base
+ * ISA RV64I.
+ */
+continue;
+}
+
+cpu_misa_ext_add_user_opt(bit, profile->enabled);
+riscv_cpu_write_misa_bit(cpu, bit, profile->enabled);
+}
+
+for (i = 0; profile->ext_offsets[i] != RISCV_PROFILE_EXT_LIST_END; i++) {
+ext_offset = profile->ext_offsets[i];
+
+if (profile->enabled) {
+if (cpu_cfg_offset_is_named_feat(ext_offset)) {
+riscv_cpu_enable_named_feat(cpu, ext_offset);
+}
+
+cpu_bump_multi_ext_priv_ver(&cpu->env, ext_offset);
+}
+
+cpu_cfg_ext_add_user_opt(ext_offset, profile->enabled);
+isa_ext_update_enabled(cpu, ext_offset, profile->enabled);
+}
+}
+
/*
* We'll get here via the following path:
*
@@ -1306,7 +1370,6 @@ static void cpu_set_profile(Object *obj, Visitor *v,
const char *name,
RISCVCPUProfile *profile = opaque;
RISCVCPU *cpu = RISCV_CPU(obj);
bool value;
-int i, ext_offset;
if (riscv_cpu_is_vendor(obj)) {
error_setg(errp, "Profile %s is not available for vendor CPUs",
@@ -1325,64 +1388,8 @@ static void cpu_set_profile(Object *obj, Visitor *v,
const char *name,
}
profile->user_set = true;
-profile->enabled = value;
-
-if (profile->u_parent != NULL) {
-object_property_set_bool(obj, profile->u_parent->name,
- profile->enabled, NULL);
-}
-
-if (profile->s_parent != NULL) {
-object_property_set_bool(obj, profile->s_parent->name,
- profile->enabled, NULL);
-}
-
-if (profile->enabled) {
-cpu->env.priv_ver = profile->priv_spec;
-
-#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, true, NULL);
-}
-#endif
-}
-
-for (i = 0; misa_bits[i] != 0; i++) {
-uint32_t bit = misa_bits[i];
-
-if (!(profile->misa_ext & bit)) {
-continue;
-}
-if (bit == RVI && !profile->enabled) {
-/*
- * Disabling profiles will not disable the base
- * ISA RV64I.
- */
-continue;
-}
-
-cpu_misa_ext_add_user_opt(bit, profile->enabled);
-riscv_cpu_write_misa_bit(cpu, bit, profile->enabled);
-}
-
-for (i = 0; profile->ext_offsets[i] != RISCV_PROFILE_EXT_LIST_END; i++) {
-ext_offset = profile->ex
