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;
>
Yeap I also tried instance_init but it's too generic, that needs if else in
riscv_cpu_init guard which CPU name from(that's not explicitly related).
if (target_riscv32()) {
if (mcc->def->misa_mxl_max > MXL_RV32) {
mcc->def->misa_mxl_max = MXL_RV32;
mcc->def->cfg.max_satp_mode = VM_1_10_SV32;
}
if (mcc->def->bare) {
mcc->def->cfg.max_satp_mode = VM_1_10_SV32;
}
#ifndef CONFIG_USER_ONLY
if (mcc->def->misa_mxl_max == MXL_RV32 &&
!valid_vm_1_10_32[mcc->def->cfg.max_satp_mode]) {
mcc->def->cfg.max_satp_mode = VM_1_10_SV32;
}
#endif
}
That would be something like this.
Maybe another option is get max_satp_mode to be a function instead?
Indeed "max_satp_mode" depends on the chosen target.
> 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
