On Tue, May 20, 2025 at 07:50:15AM -0300, Daniel Henrique Barboza wrote: > > > 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
yup > > > > > > > > > > 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. Sounds good. Thanks, drew > > > 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 > > > > > > > > > > > > > >
