Re: [PATCH v5 2/2] riscv: Allow user to set the satp mode

[Alistair Francis]

On Thu, Jan 19, 2023 at 11:00 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> Hi Alistair, Andrew,
>
> On Thu, Jan 19, 2023 at 1:25 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Jan 18, 2023 at 10:19 PM Andrew Jones <ajones@ventanamicro.com> 
> > wrote:
> > >
> > > On Wed, Jan 18, 2023 at 10:28:46AM +1000, Alistair Francis wrote:
> > > > On Wed, Jan 18, 2023 at 2:32 AM Andrew Jones <ajones@ventanamicro.com> 
> > > > wrote:
> > > > >
> > > > > On Fri, Jan 13, 2023 at 11:34:53AM +0100, Alexandre Ghiti wrote:
> > > ...
> > > > > > +
> > > > > > +    /* Get rid of 32-bit/64-bit incompatibility */
> > > > > > +    for (int i = 0; i < 16; ++i) {
> > > > > > +        if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> > > > >
> > > > > If we ever define mode=1 for rv64, then 'sv32=on' will be incorrectly
> > > > > accepted as an alias. I think we should simply not define the sv32
> > > > > property for rv64 nor the rv64-only modes for rv32. So, down in
> > > > > riscv_add_satp_mode_properties() we can add some
> > > > >
> > > > >   #if defined(TARGET_RISCV32)
> > > > >   ...
> > > > >   #elif defined(TARGET_RISCV64)
> > > > >   ...
> > > > >   #endif
> > > >
> > > > Do not add any #if defined(TARGET_RISCV32) to QEMU.
> > > >
> > > > We are aiming for the riscv64-softmmu to be able to emulate 32-bit
> > > > CPUs and compile time macros are the wrong solution here. Instead you
> > > > can get the xlen of the hart and use that.
> > > >
> > >
> > > Does this mean we want to be able to do the following?
> > >
> > >   qemu-system-riscv64 -cpu rv32,sv32=on ...
> >
> > That's the plan
> >
> > >
> > > If so, then can we move the object_property_add() for sv32 to
> > > rv32_base_cpu_init() and the rest to rv64_base_cpu_init()?

Wait! Sorry I didn't read this carefully enough. No, that is not what
we want to do. That then won't support the vendor CPUs.

We just want to add the properties to all CPUs. Then if an invalid
option is set we should return an error.

Note that the 64-bit only configs can be hidden behind a #if
defined(TARGET_RISCV64).

Alistair

> > > Currently, that would be doing the same thing as proposed above,
> > > since those functions are under TARGET_RISCV* defines, but I guess
> > > the object_property_add()'s would then be in more or less the right
> > > places for when the 32-bit emulation support work is started.
> >
> > Sounds like a good idea :)
>
> What about riscv_any_cpu_init and riscv_host_cpu_init?
>
> >
> > Alistair
> >
> > >
> > > Thanks,
> > > drew