On 5/19/25 1:35 PM, Andrew Jones wrote:
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.

What about satp validation for a profile? For both rva22 and rva23 the mandatory
satp is sv39, but up to sv57 is also ok. Do we care if a sv64 CPU claims rva23
support?

I am aware that sv64 also means sv57 support but I'm worried about migration
compatibility. Let's say we migrate between two hosts A and B that claim
to be rva23 compliant. A is running sv64, B is running sv57. If the software
running in A is actually using satp sv64 we can't migrate A to B.


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

I'm working on it. I'll decouple the QMP bits (all the profile validation 
business
is a QMP problem in the end) from the core CPU finalize logic. I'll send patches
soon.


Thanks,

Daniel


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 <alexgh...@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




Reply via email to