| Message ID | 20250115134957.2179085-4-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: RVA23 profile support | expand |
On Wed, Jan 15, 2025 at 10:49:54AM -0300, Daniel Henrique Barboza wrote: > The current 'parent' mechanic for profiles allows for one profile to be > a child of a previous/older profile, enabling all its extensions (and > the parent profile itself) and sparing us from tediously listing all > extensions for every profile. > > This works fine for u-mode profiles. For s-mode profiles this is not > enough: a s-mode profile extends not only his equivalent u-mode profile > but also the previous s-mode profile. This means, for example, that > RVA23S64 extends both RVA23U64 and RVA22S64. > > To fit this usage, rename the existing 'parent' to 'u_parent' and add a > new 's_parent' attribute for profiles. Handle both like we're doing with > the previous 'profile' attribute, i.e. if set, enable it. This change ...like we were doing with the previous 'parent'... > does nothing for the existing profiles but will make RVA23S64 simpler. > > Suggested-by: Andrew Jones <ajones@ventanamicro.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 6 ++++-- > target/riscv/cpu.h | 3 ++- > target/riscv/tcg/tcg-cpu.c | 35 ++++++++++++++++++++++++++--------- > 3 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 6fb4d5f374..e215b1004d 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -2349,7 +2349,8 @@ static const PropertyInfo prop_marchid = { > * doesn't need to be manually enabled by the profile. > */ > static RISCVCPUProfile RVA22U64 = { > - .parent = NULL, > + .u_parent = NULL, > + .s_parent = NULL, > .name = "rva22u64", > .misa_ext = RVI | RVM | RVA | RVF | RVD | RVC | RVB | RVU, > .priv_spec = RISCV_PROFILE_ATTR_UNUSED, > @@ -2381,7 +2382,8 @@ static RISCVCPUProfile RVA22U64 = { > * The remaining features/extensions comes from RVA22U64. > */ > static RISCVCPUProfile RVA22S64 = { > - .parent = &RVA22U64, > + .u_parent = &RVA22U64, > + .s_parent = NULL, > .name = "rva22s64", > .misa_ext = RVS, > .priv_spec = PRIV_VERSION_1_12_0, > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 97713681cb..986131a191 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -81,7 +81,8 @@ const char *riscv_get_misa_ext_description(uint32_t bit); > #define CPU_CFG_OFFSET(_prop) offsetof(struct RISCVCPUConfig, _prop) > > typedef struct riscv_cpu_profile { > - struct riscv_cpu_profile *parent; > + struct riscv_cpu_profile *u_parent; > + struct riscv_cpu_profile *s_parent; > const char *name; > uint32_t misa_ext; > bool enabled; > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 48be24bbbe..c9e5a3b580 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -713,13 +713,29 @@ static bool riscv_cpu_validate_profile_satp(RISCVCPU *cpu, > } > #endif > > +static void riscv_cpu_check_parent_profile(RISCVCPU *cpu, > + RISCVCPUProfile *profile, > + RISCVCPUProfile *parent) > +{ > + const char *parent_name; > + bool parent_enabled; > + > + if (!profile->enabled || !parent) { > + return; > + } > + > + parent_name = parent->name; > + parent_enabled = object_property_get_bool(OBJECT(cpu), parent_name, NULL); > + profile->enabled = profile->enabled && parent_enabled; Could drop the 'profile->enabled &&' since we already know profile->enabled is true from the test above. > +} > + > static void riscv_cpu_validate_profile(RISCVCPU *cpu, > RISCVCPUProfile *profile) > { > CPURISCVState *env = &cpu->env; > const char *warn_msg = "Profile %s mandates disabled extension %s"; > bool send_warn = profile->user_set && profile->enabled; > - bool parent_enabled, profile_impl = true; > + bool profile_impl = true; > int i; > > #ifndef CONFIG_USER_ONLY > @@ -773,12 +789,8 @@ static void riscv_cpu_validate_profile(RISCVCPU *cpu, > > profile->enabled = profile_impl; > > - if (profile->parent != NULL) { > - parent_enabled = object_property_get_bool(OBJECT(cpu), > - profile->parent->name, > - NULL); > - profile->enabled = profile->enabled && parent_enabled; > - } > + riscv_cpu_check_parent_profile(cpu, profile, profile->u_parent); > + riscv_cpu_check_parent_profile(cpu, profile, profile->s_parent); > } > > static void riscv_cpu_validate_profiles(RISCVCPU *cpu) > @@ -1181,8 +1193,13 @@ static void cpu_set_profile(Object *obj, Visitor *v, const char *name, > profile->user_set = true; > profile->enabled = value; > > - if (profile->parent != NULL) { > - object_property_set_bool(obj, profile->parent->name, > + 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); > } > > -- > 2.47.1 > Otherwise, Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 6fb4d5f374..e215b1004d 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -2349,7 +2349,8 @@ static const PropertyInfo prop_marchid = { * doesn't need to be manually enabled by the profile. */ static RISCVCPUProfile RVA22U64 = { - .parent = NULL, + .u_parent = NULL, + .s_parent = NULL, .name = "rva22u64", .misa_ext = RVI | RVM | RVA | RVF | RVD | RVC | RVB | RVU, .priv_spec = RISCV_PROFILE_ATTR_UNUSED, @@ -2381,7 +2382,8 @@ static RISCVCPUProfile RVA22U64 = { * The remaining features/extensions comes from RVA22U64. */ static RISCVCPUProfile RVA22S64 = { - .parent = &RVA22U64, + .u_parent = &RVA22U64, + .s_parent = NULL, .name = "rva22s64", .misa_ext = RVS, .priv_spec = PRIV_VERSION_1_12_0, diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 97713681cb..986131a191 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -81,7 +81,8 @@ const char *riscv_get_misa_ext_description(uint32_t bit); #define CPU_CFG_OFFSET(_prop) offsetof(struct RISCVCPUConfig, _prop) typedef struct riscv_cpu_profile { - struct riscv_cpu_profile *parent; + struct riscv_cpu_profile *u_parent; + struct riscv_cpu_profile *s_parent; const char *name; uint32_t misa_ext; bool enabled; diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 48be24bbbe..c9e5a3b580 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -713,13 +713,29 @@ static bool riscv_cpu_validate_profile_satp(RISCVCPU *cpu, } #endif +static void riscv_cpu_check_parent_profile(RISCVCPU *cpu, + RISCVCPUProfile *profile, + RISCVCPUProfile *parent) +{ + const char *parent_name; + bool parent_enabled; + + if (!profile->enabled || !parent) { + return; + } + + parent_name = parent->name; + parent_enabled = object_property_get_bool(OBJECT(cpu), parent_name, NULL); + profile->enabled = profile->enabled && parent_enabled; +} + static void riscv_cpu_validate_profile(RISCVCPU *cpu, RISCVCPUProfile *profile) { CPURISCVState *env = &cpu->env; const char *warn_msg = "Profile %s mandates disabled extension %s"; bool send_warn = profile->user_set && profile->enabled; - bool parent_enabled, profile_impl = true; + bool profile_impl = true; int i; #ifndef CONFIG_USER_ONLY @@ -773,12 +789,8 @@ static void riscv_cpu_validate_profile(RISCVCPU *cpu, profile->enabled = profile_impl; - if (profile->parent != NULL) { - parent_enabled = object_property_get_bool(OBJECT(cpu), - profile->parent->name, - NULL); - profile->enabled = profile->enabled && parent_enabled; - } + riscv_cpu_check_parent_profile(cpu, profile, profile->u_parent); + riscv_cpu_check_parent_profile(cpu, profile, profile->s_parent); } static void riscv_cpu_validate_profiles(RISCVCPU *cpu) @@ -1181,8 +1193,13 @@ static void cpu_set_profile(Object *obj, Visitor *v, const char *name, profile->user_set = true; profile->enabled = value; - if (profile->parent != NULL) { - object_property_set_bool(obj, profile->parent->name, + 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); }
The current 'parent' mechanic for profiles allows for one profile to be a child of a previous/older profile, enabling all its extensions (and the parent profile itself) and sparing us from tediously listing all extensions for every profile. This works fine for u-mode profiles. For s-mode profiles this is not enough: a s-mode profile extends not only his equivalent u-mode profile but also the previous s-mode profile. This means, for example, that RVA23S64 extends both RVA23U64 and RVA22S64. To fit this usage, rename the existing 'parent' to 'u_parent' and add a new 's_parent' attribute for profiles. Handle both like we're doing with the previous 'profile' attribute, i.e. if set, enable it. This change does nothing for the existing profiles but will make RVA23S64 simpler. Suggested-by: Andrew Jones <ajones@ventanamicro.com> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 6 ++++-- target/riscv/cpu.h | 3 ++- target/riscv/tcg/tcg-cpu.c | 35 ++++++++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 12 deletions(-)