| Message ID | 20250206182711.2420505-7-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: declarative CPU definitions | expand |
On Fri, Feb 7, 2025 at 4:29 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > Allow using RISCVCPUDef to replicate all the logic of custom .instance_init > functions. To simulate inheritance, merge the child's RISCVCPUDef with > the parent and then finally move it to the CPUState at the end of > TYPE_RISCV_CPU's own instance_init function. > > STRUCT_FIELD is introduced here because I am not sure it is needed; > it is a bit ugly and I wanted not to have it in the patch that > introduces cpu_cfg_fields.h.inc. I don't really understand why satp_mode > is included in RISCVCPUConfig; therefore, the end of the series includes I don't follow. `satp_mode` is a configurable option, hence the inclusion in `RISCVCPUConfig` Alistair > a patch to move satp_mode directly in RISCVCPU, thus removing the need > for STRUCT_FIELD; it can be moved before this one in a non-RFC posting. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/riscv/cpu.h | 6 ++++ > target/riscv/cpu_cfg.h | 1 + > target/riscv/cpu_cfg_fields.h.inc | 6 +++- > target/riscv/cpu.c | 48 ++++++++++++++++++++++++++++++- > 4 files changed, 59 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index f757f0b6210..9b25c0c889b 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -519,6 +519,12 @@ struct ArchCPU { > > typedef struct RISCVCPUDef { > RISCVMXL misa_mxl_max; /* max mxl for this cpu */ > + uint32_t misa_ext; > + int priv_spec; > + int32_t vext_spec; > + int satp_mode32; > + int satp_mode64; > + RISCVCPUConfig cfg; > } RISCVCPUDef; > > /** > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index ad02693fa66..07789a9de88 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -39,6 +39,7 @@ typedef struct { > struct RISCVCPUConfig { > #define BOOL_FIELD(x) bool x; > #define TYPED_FIELD(type, x) type x; > +#define STRUCT_FIELD(type, x) type x; > #include "cpu_cfg_fields.h.inc" > }; > > diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc > index 56fffb5f177..cbedf0a703b 100644 > --- a/target/riscv/cpu_cfg_fields.h.inc > +++ b/target/riscv/cpu_cfg_fields.h.inc > @@ -4,6 +4,9 @@ > #ifndef TYPED_FIELD > #define TYPED_FIELD(type, x) > #endif > +#ifndef STRUCT_FIELD > +#define STRUCT_FIELD(type, x) > +#endif > > BOOL_FIELD(ext_zba) > BOOL_FIELD(ext_zbb) > @@ -160,8 +163,9 @@ TYPED_FIELD(uint16_t, cbop_blocksize) > TYPED_FIELD(uint16_t, cboz_blocksize) > > #ifndef CONFIG_USER_ONLY > -TYPED_FIELD(RISCVSATPMap, satp_mode); > +STRUCT_FIELD(RISCVSATPMap, satp_mode) > #endif > > #undef BOOL_FIELD > #undef TYPED_FIELD > +#undef STRUCT_FIELD > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index baf4dd017b2..1d999488465 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -74,6 +74,15 @@ bool riscv_cpu_option_set(const char *optname) > return g_hash_table_contains(general_user_opts, optname); > } > > +static void riscv_cpu_cfg_merge(RISCVCPUConfig *dest, RISCVCPUConfig *src) > +{ > +#define BOOL_FIELD(x) dest->x |= src->x; > +#define TYPED_FIELD(type, x) if (src->x) dest->x = src->x; > + /* only satp_mode, which is initialized by instance_init */ > +#define STRUCT_FIELD(type, x) > +#include "cpu_cfg_fields.h.inc" > +} > + > #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ > {#_name, _min_ver, CPU_CFG_OFFSET(_prop)} > > @@ -432,7 +441,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit) > } > > static void set_satp_mode_max_supported(RISCVCPU *cpu, > - uint8_t satp_mode) > + int satp_mode) > { > bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32; > const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64; > @@ -1476,6 +1485,24 @@ static void riscv_cpu_init(Object *obj) > cpu->cfg.cbop_blocksize = 64; > cpu->cfg.cboz_blocksize = 64; > cpu->env.vext_ver = VEXT_VERSION_1_00_0; > + > + env->misa_ext_mask = env->misa_ext = mcc->def->misa_ext; > + riscv_cpu_cfg_merge(&cpu->cfg, &mcc->def->cfg); > + > + if (mcc->def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { > + cpu->env.priv_ver = mcc->def->priv_spec; > + } > + if (mcc->def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { > + cpu->env.vext_ver = mcc->def->vext_spec; > + } > +#ifndef CONFIG_USER_ONLY > + if (riscv_cpu_mxl(env) == MXL_RV32 && mcc->def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { > + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode32); > + } > + if (riscv_cpu_mxl(env) >= MXL_RV64 && mcc->def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { > + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode64); > + } > +#endif > } > > static void riscv_bare_cpu_init(Object *obj) > @@ -2968,6 +2995,25 @@ static void riscv_cpu_class_base_init(ObjectClass *c, void *data) > assert(def->misa_mxl_max <= MXL_RV128); > mcc->def->misa_mxl_max = def->misa_mxl_max; > } > + if (def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { > + assert(def->priv_spec <= PRIV_VERSION_LATEST); > + mcc->def->priv_spec = def->priv_spec; > + } > + if (def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { > + assert(def->vext_spec != 0); > + mcc->def->vext_spec = def->vext_spec; > + } > + if (def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { > + assert(def->satp_mode32 <= VM_1_10_SV32); > + mcc->def->satp_mode32 = def->satp_mode32; > + } > + if (def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { > + assert(def->satp_mode64 <= VM_1_10_SV64); > + mcc->def->satp_mode64 = def->satp_mode64; > + } > + mcc->def->misa_ext |= def->misa_ext; > + > + riscv_cpu_cfg_merge(&mcc->def->cfg, &def->cfg); > } > > if (!object_class_is_abstract(c)) { > -- > 2.48.1 > >
On 2/18/25 01:23, Alistair Francis wrote: > On Fri, Feb 7, 2025 at 4:29 AM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> Allow using RISCVCPUDef to replicate all the logic of custom .instance_init >> functions. To simulate inheritance, merge the child's RISCVCPUDef with >> the parent and then finally move it to the CPUState at the end of >> TYPE_RISCV_CPU's own instance_init function. >> >> STRUCT_FIELD is introduced here because I am not sure it is needed; >> it is a bit ugly and I wanted not to have it in the patch that >> introduces cpu_cfg_fields.h.inc. I don't really understand why satp_mode >> is included in RISCVCPUConfig; therefore, the end of the series includes > > I don't follow. `satp_mode` is a configurable option, hence the > inclusion in `RISCVCPUConfig` My understanding is that satp_mode is a combination of: - the maximum supported satp mode; this is stored in cpu->satp_modes.supported and actually comes from the instance_init function. - the value of the properties, which is stored in satp_map->map and satp_map->init For non-bare CPU models, including all vendor CPU models, cpu->satp_modes.supported also acts as the default value for the mode properties. Furthermore, for all vendor CPU models all modes are supported up to the max mode, for example you never have a CPU that supports sv48 but not sv39. So it seems that the CPU config is really the supported satp modes, it's just bare CPU models that have the further restriction that you have to specify at least one mode by hand. Under this interpretation: - all that needs to be in RISCVCPUConfig is what I called satp_mode32 and satp_mode64 - satp_mode32 or satp_mode64 therefore can be removed from RISCVCPUDef - init and map would be moved to RISCVCPU because they are purely a QOM concept Paolo > Alistair > >> a patch to move satp_mode directly in RISCVCPU, thus removing the need >> for STRUCT_FIELD; it can be moved before this one in a non-RFC posting. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> target/riscv/cpu.h | 6 ++++ >> target/riscv/cpu_cfg.h | 1 + >> target/riscv/cpu_cfg_fields.h.inc | 6 +++- >> target/riscv/cpu.c | 48 ++++++++++++++++++++++++++++++- >> 4 files changed, 59 insertions(+), 2 deletions(-) >> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >> index f757f0b6210..9b25c0c889b 100644 >> --- a/target/riscv/cpu.h >> +++ b/target/riscv/cpu.h >> @@ -519,6 +519,12 @@ struct ArchCPU { >> >> typedef struct RISCVCPUDef { >> RISCVMXL misa_mxl_max; /* max mxl for this cpu */ >> + uint32_t misa_ext; >> + int priv_spec; >> + int32_t vext_spec; >> + int satp_mode32; >> + int satp_mode64; >> + RISCVCPUConfig cfg; >> } RISCVCPUDef; >> >> /** >> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h >> index ad02693fa66..07789a9de88 100644 >> --- a/target/riscv/cpu_cfg.h >> +++ b/target/riscv/cpu_cfg.h >> @@ -39,6 +39,7 @@ typedef struct { >> struct RISCVCPUConfig { >> #define BOOL_FIELD(x) bool x; >> #define TYPED_FIELD(type, x) type x; >> +#define STRUCT_FIELD(type, x) type x; >> #include "cpu_cfg_fields.h.inc" >> }; >> >> diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc >> index 56fffb5f177..cbedf0a703b 100644 >> --- a/target/riscv/cpu_cfg_fields.h.inc >> +++ b/target/riscv/cpu_cfg_fields.h.inc >> @@ -4,6 +4,9 @@ >> #ifndef TYPED_FIELD >> #define TYPED_FIELD(type, x) >> #endif >> +#ifndef STRUCT_FIELD >> +#define STRUCT_FIELD(type, x) >> +#endif >> >> BOOL_FIELD(ext_zba) >> BOOL_FIELD(ext_zbb) >> @@ -160,8 +163,9 @@ TYPED_FIELD(uint16_t, cbop_blocksize) >> TYPED_FIELD(uint16_t, cboz_blocksize) >> >> #ifndef CONFIG_USER_ONLY >> -TYPED_FIELD(RISCVSATPMap, satp_mode); >> +STRUCT_FIELD(RISCVSATPMap, satp_mode) >> #endif >> >> #undef BOOL_FIELD >> #undef TYPED_FIELD >> +#undef STRUCT_FIELD >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index baf4dd017b2..1d999488465 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -74,6 +74,15 @@ bool riscv_cpu_option_set(const char *optname) >> return g_hash_table_contains(general_user_opts, optname); >> } >> >> +static void riscv_cpu_cfg_merge(RISCVCPUConfig *dest, RISCVCPUConfig *src) >> +{ >> +#define BOOL_FIELD(x) dest->x |= src->x; >> +#define TYPED_FIELD(type, x) if (src->x) dest->x = src->x; >> + /* only satp_mode, which is initialized by instance_init */ >> +#define STRUCT_FIELD(type, x) >> +#include "cpu_cfg_fields.h.inc" >> +} >> + >> #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ >> {#_name, _min_ver, CPU_CFG_OFFSET(_prop)} >> >> @@ -432,7 +441,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit) >> } >> >> static void set_satp_mode_max_supported(RISCVCPU *cpu, >> - uint8_t satp_mode) >> + int satp_mode) >> { >> bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32; >> const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64; >> @@ -1476,6 +1485,24 @@ static void riscv_cpu_init(Object *obj) >> cpu->cfg.cbop_blocksize = 64; >> cpu->cfg.cboz_blocksize = 64; >> cpu->env.vext_ver = VEXT_VERSION_1_00_0; >> + >> + env->misa_ext_mask = env->misa_ext = mcc->def->misa_ext; >> + riscv_cpu_cfg_merge(&cpu->cfg, &mcc->def->cfg); >> + >> + if (mcc->def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { >> + cpu->env.priv_ver = mcc->def->priv_spec; >> + } >> + if (mcc->def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { >> + cpu->env.vext_ver = mcc->def->vext_spec; >> + } >> +#ifndef CONFIG_USER_ONLY >> + if (riscv_cpu_mxl(env) == MXL_RV32 && mcc->def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { >> + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode32); >> + } >> + if (riscv_cpu_mxl(env) >= MXL_RV64 && mcc->def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { >> + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode64); >> + } >> +#endif >> } >> >> static void riscv_bare_cpu_init(Object *obj) >> @@ -2968,6 +2995,25 @@ static void riscv_cpu_class_base_init(ObjectClass *c, void *data) >> assert(def->misa_mxl_max <= MXL_RV128); >> mcc->def->misa_mxl_max = def->misa_mxl_max; >> } >> + if (def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { >> + assert(def->priv_spec <= PRIV_VERSION_LATEST); >> + mcc->def->priv_spec = def->priv_spec; >> + } >> + if (def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { >> + assert(def->vext_spec != 0); >> + mcc->def->vext_spec = def->vext_spec; >> + } >> + if (def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { >> + assert(def->satp_mode32 <= VM_1_10_SV32); >> + mcc->def->satp_mode32 = def->satp_mode32; >> + } >> + if (def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { >> + assert(def->satp_mode64 <= VM_1_10_SV64); >> + mcc->def->satp_mode64 = def->satp_mode64; >> + } >> + mcc->def->misa_ext |= def->misa_ext; >> + >> + riscv_cpu_cfg_merge(&mcc->def->cfg, &def->cfg); >> } >> >> if (!object_class_is_abstract(c)) { >> -- >> 2.48.1 >> >> > >
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index f757f0b6210..9b25c0c889b 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -519,6 +519,12 @@ struct ArchCPU { typedef struct RISCVCPUDef { RISCVMXL misa_mxl_max; /* max mxl for this cpu */ + uint32_t misa_ext; + int priv_spec; + int32_t vext_spec; + int satp_mode32; + int satp_mode64; + RISCVCPUConfig cfg; } RISCVCPUDef; /** diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h index ad02693fa66..07789a9de88 100644 --- a/target/riscv/cpu_cfg.h +++ b/target/riscv/cpu_cfg.h @@ -39,6 +39,7 @@ typedef struct { struct RISCVCPUConfig { #define BOOL_FIELD(x) bool x; #define TYPED_FIELD(type, x) type x; +#define STRUCT_FIELD(type, x) type x; #include "cpu_cfg_fields.h.inc" }; diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc index 56fffb5f177..cbedf0a703b 100644 --- a/target/riscv/cpu_cfg_fields.h.inc +++ b/target/riscv/cpu_cfg_fields.h.inc @@ -4,6 +4,9 @@ #ifndef TYPED_FIELD #define TYPED_FIELD(type, x) #endif +#ifndef STRUCT_FIELD +#define STRUCT_FIELD(type, x) +#endif BOOL_FIELD(ext_zba) BOOL_FIELD(ext_zbb) @@ -160,8 +163,9 @@ TYPED_FIELD(uint16_t, cbop_blocksize) TYPED_FIELD(uint16_t, cboz_blocksize) #ifndef CONFIG_USER_ONLY -TYPED_FIELD(RISCVSATPMap, satp_mode); +STRUCT_FIELD(RISCVSATPMap, satp_mode) #endif #undef BOOL_FIELD #undef TYPED_FIELD +#undef STRUCT_FIELD diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index baf4dd017b2..1d999488465 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -74,6 +74,15 @@ bool riscv_cpu_option_set(const char *optname) return g_hash_table_contains(general_user_opts, optname); } +static void riscv_cpu_cfg_merge(RISCVCPUConfig *dest, RISCVCPUConfig *src) +{ +#define BOOL_FIELD(x) dest->x |= src->x; +#define TYPED_FIELD(type, x) if (src->x) dest->x = src->x; + /* only satp_mode, which is initialized by instance_init */ +#define STRUCT_FIELD(type, x) +#include "cpu_cfg_fields.h.inc" +} + #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ {#_name, _min_ver, CPU_CFG_OFFSET(_prop)} @@ -432,7 +441,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit) } static void set_satp_mode_max_supported(RISCVCPU *cpu, - uint8_t satp_mode) + int satp_mode) { bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32; const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64; @@ -1476,6 +1485,24 @@ static void riscv_cpu_init(Object *obj) cpu->cfg.cbop_blocksize = 64; cpu->cfg.cboz_blocksize = 64; cpu->env.vext_ver = VEXT_VERSION_1_00_0; + + env->misa_ext_mask = env->misa_ext = mcc->def->misa_ext; + riscv_cpu_cfg_merge(&cpu->cfg, &mcc->def->cfg); + + if (mcc->def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { + cpu->env.priv_ver = mcc->def->priv_spec; + } + if (mcc->def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { + cpu->env.vext_ver = mcc->def->vext_spec; + } +#ifndef CONFIG_USER_ONLY + if (riscv_cpu_mxl(env) == MXL_RV32 && mcc->def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode32); + } + if (riscv_cpu_mxl(env) >= MXL_RV64 && mcc->def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { + set_satp_mode_max_supported(RISCV_CPU(obj), mcc->def->satp_mode64); + } +#endif } static void riscv_bare_cpu_init(Object *obj) @@ -2968,6 +2995,25 @@ static void riscv_cpu_class_base_init(ObjectClass *c, void *data) assert(def->misa_mxl_max <= MXL_RV128); mcc->def->misa_mxl_max = def->misa_mxl_max; } + if (def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) { + assert(def->priv_spec <= PRIV_VERSION_LATEST); + mcc->def->priv_spec = def->priv_spec; + } + if (def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) { + assert(def->vext_spec != 0); + mcc->def->vext_spec = def->vext_spec; + } + if (def->satp_mode32 != RISCV_PROFILE_ATTR_UNUSED) { + assert(def->satp_mode32 <= VM_1_10_SV32); + mcc->def->satp_mode32 = def->satp_mode32; + } + if (def->satp_mode64 != RISCV_PROFILE_ATTR_UNUSED) { + assert(def->satp_mode64 <= VM_1_10_SV64); + mcc->def->satp_mode64 = def->satp_mode64; + } + mcc->def->misa_ext |= def->misa_ext; + + riscv_cpu_cfg_merge(&mcc->def->cfg, &def->cfg); } if (!object_class_is_abstract(c)) {
Allow using RISCVCPUDef to replicate all the logic of custom .instance_init functions. To simulate inheritance, merge the child's RISCVCPUDef with the parent and then finally move it to the CPUState at the end of TYPE_RISCV_CPU's own instance_init function. STRUCT_FIELD is introduced here because I am not sure it is needed; it is a bit ugly and I wanted not to have it in the patch that introduces cpu_cfg_fields.h.inc. I don't really understand why satp_mode is included in RISCVCPUConfig; therefore, the end of the series includes a patch to move satp_mode directly in RISCVCPU, thus removing the need for STRUCT_FIELD; it can be moved before this one in a non-RFC posting. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/riscv/cpu.h | 6 ++++ target/riscv/cpu_cfg.h | 1 + target/riscv/cpu_cfg_fields.h.inc | 6 +++- target/riscv/cpu.c | 48 ++++++++++++++++++++++++++++++- 4 files changed, 59 insertions(+), 2 deletions(-)