Re: [PATCH RFC] target: riscv: Fix satp mode initialization based on profile

[Andrew Jones <ajones@ventanamicro.com>]

On Mon, May 19, 2025 at 09:48:14AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 5/16/25 9:23 AM, Alexandre Ghiti wrote:
> > The satp mode is set using the svXX properties, but that actually
> > restricts the satp mode to the minimum required by the profile and
> > prevents the use of higher satp modes.
> 
> For rva23s64, in "Optional Extensions" we'll find:

The keyword is "Optional". The profile should only set sv39 since that's
what rva23 (and rv22) have for the mandatory support. sv48 and sv57 are
both optional so, while the user should be allowed to turn them on, just
like other optional extensions, they shouldn't be on by default since we
don't set any optional extensions on by default.

So we don't want this change, but fixing any bugs with the first hart vs.
the other harts is of course necessary.

Thanks,
drew

> 
> https://github.com/riscv/riscv-profiles/blob/main/src/rva23-profile.adoc
> 
> - Sv48 Page-based 48-bit virtual-memory system.
> - Sv57 Page-based 57-bit virtual-memory system.
> 
> So in theory we could go up to sv57 for rva23s64 (and rva22s64, just checked).
> Changing satp_mode to the maximum allowed seems sensible.
> 
> But allowing all satp modes to go in a profile defeats the purpose, doesn't it?
> None of the existing profiles in QEMU claims supports sv64. Granted, I'm not a
> satp expert, but removing the satp restriction in profiles doesn't seem right.
> 
> 
> Thanks,
> 
> Daniel
> 
> 
> > 
> > Fix this by not setting any svXX property and allow all satp mode to be
> > supported.
> > 
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >   target/riscv/tcg/tcg-cpu.c | 3 ---
> >   1 file changed, 3 deletions(-)
> > 
> > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> > index 5aef9eef36..ca2d2950eb 100644
> > --- a/target/riscv/tcg/tcg-cpu.c
> > +++ b/target/riscv/tcg/tcg-cpu.c
> > @@ -1232,9 +1232,6 @@ static void cpu_set_profile(Object *obj, Visitor *v, const char *name,
> >   #ifndef CONFIG_USER_ONLY
> >       if (profile->satp_mode != RISCV_PROFILE_ATTR_UNUSED) {
> >           object_property_set_bool(obj, "mmu", true, NULL);
> > -        const char *satp_prop = satp_mode_str(profile->satp_mode,
> > -                                              riscv_cpu_is_32bit(cpu));
> > -        object_property_set_bool(obj, satp_prop, profile->enabled, NULL);
> >       }
> >   #endif
> 
>