On Wed, Oct 7, 2026 at 7:09 AM Anton Johansson <[email protected]> wrote: > > 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.
Make sense, thanks > > > > > 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 > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
