Re: [PATCH v6 1/5] hw/riscv: Use misa_mxl instead of misa_mxl_max

[Alistair Francis]

On Fri, Dec 15, 2023 at 4:34 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2023/12/15 14:34, Alistair Francis wrote:
> > On Thu, Nov 23, 2023 at 5:24 PM Akihiko Odaki <akihiko.odaki@daynix.com> 
> > wrote:
> >>
> >> On 2023/11/23 12:04, Alistair Francis wrote:
> >>> On Mon, Oct 30, 2023 at 3:50 PM Akihiko Odaki <akihiko.odaki@daynix.com> 
> >>> wrote:
> >>>>
> >>>> The effective MXL value matters when booting.
> >>>
> >>> This doesn't sound right. Surely the max is what matters here
> >>>
> >>> Also, this was specifically changed to misa_mxl_max in db23e5d981a
> >>> "target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl".
> >>>
> >>> This needs a much better description of why this change should be made
> >>   >
> >>   > Alistair
> >>
> >> The kernel will be executed with the current MXL rather than the initial
> >> MXL value so the current MXL should be used here.
> >>
> >> For example, if you are going to emulate a system that has a RV64 CPU
> >> and a firmware that sets the MXL to RV32, then mxl_max should be
> >> MXL_RV64 and mxl should be MXL_RV32, and the kernel should be assumed as
> >> a RV32 binary. Loading a 64-bit kernel will not work in such a case.
> >
> > But this is called before the firmware runs, so it won't be changed by 
> > firmware.
>
> It's more like QEMU emulates the firmware. It's the responsibility of
> the firmware to load kernels for the real hardware, but QEMU does it
> instead.
>
> The firmware can change the MXL to load a 32-bit kernel on a 64-bit
> system so if QEMU happens to emulate such a behavior, mxl should be used
> when loading the kernel instead of mxl_max. QEMU currently does not
> implement such a feature, but in such a case mxl == mxl_max so it does
> not hurt to use mxl.
>
> >
> > Maybe it's worth putting what this fixes in the commit message?
>
> What about:
>
> A later commit requires one extra step to retrieve mxl_max. As mxl is
> semantically more correct and does not need such a extra step, refer to
> mxl instead.
>
> Currently mxl always equals to mxl_max so it does not matter which of
> mxl or mxl_max to refer to. However, it is possible to have different
> values for mxl and mxl_max if QEMU gains a new feature to load a RV32
> kernel on a RV64 system, for example. For such a behavior, the real
> system will need the firmware to switch MXL to RV32, and if QEMU
> implements the same behavior, mxl will represent the MXL that
> corresponds to the kernel being loaded. Therefore, it is more
> appropriate to refer to mxl instead of mxl_max when mxl != mxl_max.

Great! That explains it really well. Can you include that in the
commit message in the next revision

Alistair

>
> Regards,
> Akihiko Odaki