On 5/20/25 4:50 AM, Andrew Jones wrote:
On Mon, May 19, 2025 at 02:15:05PM -0300, Daniel Henrique Barboza wrote:
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?
Is sv64 defined yet? I thought it's just a placeholder. Anyway, I'd expect
it to be like sv57 and sv48 in that each narrower width must be supported,
which means sv39 would also still be supported, and that means the cpu
could be rva23. If, OTOH, an sv64 cpu cannot support sv39, then the cpu
cannot have both, so it cannot be rva23. IOW, as long as sv39 is _also_
supported, then sv64 is OK to select and still be rva23.
We have sv64 defined in QEMU. Not sure if it's already a thing or not ....
it seems to me that ppl cares to sv57 mostly, so perhaps my sv64 worry
unjustified.
Just took a look in how we're validating satp for profiles and we're
allowing a higher satp mode than the profile mandates, issuing warnings
if the satp mode is set to a lower mode than the profile value.
So I guess the way we would like people to use rva23s64 with sv57 would be:
-cpu rva23s64,sv57=on
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.
A and B are incompatible already if A is '-cpu rva23,sv64=on' and B is
'-cpu rva23,sv57=on', so the migration manager should already disallow
that.
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.
Great, thanks!
Another comment I thought of later that I should have put in my original
reply is that we of course want 'max' to default to the widest QEMU
supports. Then, users that want to ensure they get sv57 or sv64 without
having to explicitly add it to their command lines can use 'max'.
Specifying '-cpu rva23' means you just want the mandatory base of the
given profile and if you want more than that then you need to append each
optional extension explicitly. If we don't have that documented somewhere,
then maybe we should, in order to help clarify the intent.
max CPU is using satp_mode = sv57. Since sv64 is mostly a placeholder then I
believe it's all good. Perhaps we could add a satp_mode_latest value for these
situations.
Thanks,
Daniel
Thanks,
drew
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
