On 02/10/26, Yonggang Luo wrote: > target/riscv: resolve target-dependent CPU definition fields > > The class-base initializer later reads def->misa_mxl_max and passes > def->cfg to riscv_cpu_cfg_merge(), which merges max_satp_mode. Resolve > both chooser results in a mutable copy so those steps use the selected > values without modifying the const TypeInfo class data. > > > defer riscv_cpu_validate_misa_mxl is not enough, mainly > def->misa_mxl_max is accessed in > riscv_cpu_class_base_init, for exactly match previous behavior, add > two callback for each value
Right, we would have to move more stuff to instance init which isn't the best idea. Personally I'm still not keen on adding more callbacks to initialize two fields for two cpus, surely this is solvable with the current machinery. I'll submit a v2 and try defining two tables for MAX/BARE instead, similar to ARM. > > On Fri, Oct 2, 2026 at 11:36 PM Anton Johansson <[email protected]> wrote: > > > > On 02/10/26, Pierrick Bouvier wrote: > > > On 10/1/2026 9:28 AM, Yonggang Luo wrote: > > > > On Thu, Oct 1, 2026 at 11:55 PM Anton Johansson via qemu development > > > > <[email protected]> wrote: > > > >> > > > >> Moves initialization of satp and mxl fields to a class initialization > > > >> function that checks target_riscv64(). > > > >> > > > >> Note: I'm aware the current iteration of this patch is rather hacky, I > > > >> don't like the introduction of runtime_misa_mxl_max, but I'm keeping > > > >> this in to get the series out there and collect feedback. > > > >> > > > >> Signed-off-by: Anton Johansson <[email protected]> > > > >> --- > > > >> target/riscv/cpu.h | 1 + > > > >> target/riscv/cpu.c | 90 > > > >> +++++++++++++++++++++++++++++++++++------------------- > > > >> 2 files changed, 59 insertions(+), 32 deletions(-) > > > >> > > > > > > > > This patch is how I handle these, just for reference, not a review. > > > > > > Yonggang approach looks cleaner, I would favor this one. > > > > I'm personally not a huge fan, it still feels rather hacky. Why add a > > completely new RISCVCPUDef::target_init just to make sure fields are > > initialized before riscv_cpu_validate_misa_mxl()? Wouldn't moving the > > check to a later point, maybe .instance_init, be better? Then for > > riscv_cpu_init() we get > > > > riscv_cpu_validate_misa_mxl(mcc); > > env->misa_mxl = mcc->def->misa_mxl_max; > > > > misa_mxl_max would be verified only during instance rather than > > class initialization, which seems like a decent tradeoff for a simpler > > solution. > > > > -- > > Anton Johansson > > rev.ng Labs Srl. > > > > -- > 此致 > 礼 > 罗勇刚 > Yours > sincerely, > Yonggang Luo > From 31138892fe5cf878da9818cf93535ce10b125cb0 Mon Sep 17 00:00:00 2001 > From: Yonggang Luo <[email protected]> > Date: Sat, 3 Oct 2026 01:54:52 +0800 > Subject: [PATCH] target/riscv: resolve target-dependent CPU definition fields > > The class-base initializer later reads def->misa_mxl_max and passes > def->cfg to riscv_cpu_cfg_merge(), which merges max_satp_mode. Resolve > both chooser results in a mutable copy so those steps use the selected > values without modifying the const TypeInfo class data. > > Signed-off-by: Yonggang Luo <[email protected]> > --- > target/riscv/cpu.c | 40 +++++++++++++++++++++++++++------------- > target/riscv/cpu.h | 2 ++ > 2 files changed, 29 insertions(+), 13 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index c6f1739075..754f158250 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -3092,6 +3092,8 @@ static void riscv_cpu_class_base_init(ObjectClass *c, > const void *data) > > if (pcc->def) { > mcc->def = g_memdup2(pcc->def, sizeof(*pcc->def)); > + mcc->def->max_satp_mode_choose = NULL; > + mcc->def->misa_mxl_max_choose = NULL; > } else { > mcc->def = g_new0(RISCVCPUDef, 1); > } > @@ -3108,7 +3110,16 @@ static void riscv_cpu_class_base_init(ObjectClass *c, > const void *data) > #endif > > if (data) { > - const RISCVCPUDef *def = data; > + RISCVCPUDef def_data = *(const RISCVCPUDef *)data; > + RISCVCPUDef *def = &def_data; > + > + if (def->max_satp_mode_choose) { > + def->cfg.max_satp_mode = def->max_satp_mode_choose(def); > + } > + if (def->misa_mxl_max_choose) { > + def->misa_mxl_max = def->misa_mxl_max_choose(def); > + } > + > mcc->def->bare |= def->bare; > if (def->profile) { > assert(profile_extends(def->profile, mcc->def->profile)); > @@ -3299,6 +3310,18 @@ static void riscv_cpu_instance_finalize(Object *obj) > g_clear_pointer(&cpu->multi_ext_user_opts, g_hash_table_destroy); > } > > +static int8_t riscv_cpu_max_satp_mode_choose(const RISCVCPUDef *def) > +{ > + (void)def; > + return target_riscv32() ? VM_1_10_SV32 : VM_1_10_SV57; > +} > + > +static RISCVMXL riscv_cpu_misa_mxl_max_choose(const RISCVCPUDef *def) > +{ > + (void)def; > + return target_riscv32() ? MXL_RV32 : MXL_RV64; > +} > + > static const TypeInfo riscv_cpu_type_infos[] = { > { > .name = TYPE_RISCV_CPU, > @@ -3339,21 +3362,12 @@ static const TypeInfo riscv_cpu_type_infos[] = { > * only MBARE will be available if the user doesn't enable > * a mode manually (see riscv_cpu_satp_mode_finalize()). > */ > -#ifdef TARGET_RISCV32 > - .cfg.max_satp_mode = VM_1_10_SV32, > -#else > - .cfg.max_satp_mode = VM_1_10_SV57, > -#endif > + .max_satp_mode_choose = riscv_cpu_max_satp_mode_choose, > ), > > DEFINE_RISCV_CPU(TYPE_RISCV_CPU_MAX, TYPE_RISCV_DYNAMIC_CPU, > -#if defined(TARGET_RISCV32) > - .misa_mxl_max = MXL_RV32, > - .cfg.max_satp_mode = VM_1_10_SV32, > -#elif defined(TARGET_RISCV64) > - .misa_mxl_max = MXL_RV64, > - .cfg.max_satp_mode = VM_1_10_SV57, > -#endif > + .max_satp_mode_choose = riscv_cpu_max_satp_mode_choose, > + .misa_mxl_max_choose = riscv_cpu_misa_mxl_max_choose, > ), > > DEFINE_ABSTRACT_RISCV_CPU(TYPE_RISCV_CPU_SIFIVE_E, TYPE_RISCV_VENDOR_CPU, > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 034d7e96c4..b6b5f679c9 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -597,6 +597,8 @@ typedef struct RISCVCPUDef { > #endif > /* This is just a setter for env->num_triggers. */ > uint32_t num_triggers; > + int8_t (*max_satp_mode_choose)(const struct RISCVCPUDef *def); > + RISCVMXL (*misa_mxl_max_choose)(const struct RISCVCPUDef *def); > } RISCVCPUDef; > > /** > -- > 2.55.0.windows.3 >
