On Mon, Apr 28, 2025 at 03:37:26PM +0200, Radim Krčmář wrote:
> 2025-04-28T14:08:59+02:00, Andrew Jones <ajo...@ventanamicro.com>:
> > On Mon, Apr 28, 2025 at 11:30:36AM +0200, Radim Krčmář wrote:
> >> 2025-04-28T09:00:55+02:00, Andrew Jones <ajo...@ventanamicro.com>:
> >> > On Sun, Apr 27, 2025 at 09:25:57PM +0800, Meng Zhuo wrote:
> >> >> This patch adds host satp mode while kvm/host cpu satp mode is not
> >> >> set.
> >> >
> >> > Huh, the KVM side[1] was written for this purpose, but it appears we
> >> > never
> >> > got a QEMU side merged.
> >> >
> >> > [1] commit 2776421e6839 ("RISC-V: KVM: provide UAPI for host SATP mode")
> >>
> >> KVM satp_mode is the current SATP.mode and I don't think the other
> >> SATP.modes can generally be guessed from the host SATP mode.
> >>
> >> Can't QEMU use the host capabilities from cpuinfo or something?
> >>
> >> Do we need to return a bitmask from KVM?
> >> (e.g. WARL all modes in vsatp and return what sticks.)
> >>
> >
> > The widest supported is sufficient because all narrower must also be
> > supported. Linux should be figuring out the widest and capturing that
> > at boot time and we should be returing that info for the KVM satp_mode
> > get-one-reg call.
>
> Linux has command line overrides for the mode (no4lvl and no5vlv), so
> the active mode in Linux might not be the widest supported by the cpu.
>
> Let's say Linux decides to use 9 on a host cpu that has 0,8,9,10.
> set_satp_mode_max_supported() will set supported vcpu modes to 0,8,9.
>
> Should "-cpu host" contain the 10?
If the architecture allows it and the host kernel/KVM is aware of it. If,
OTOH, the boot options / hardware description has hidden the platform's
capabilities from KVM, then it won't know 10 is supported, so it won't
tell the vmm it is and the vmm shouldn't second guess that.
>
> > If the satp_mode we're currently returning isn't the widest possible,
> > then we should fix that in KVM.
>
> The numbers are even more complicated... Pasting the values from manual:
>
> 0 Bare
> 1-7 Reserved for standard use
> 8 Sv39
> 9 Sv48
> 10 Sv57
> 11 Reserved for Sv64
> 12-13 Reserved for standard use
> 14-15 Designated for custom use
>
> The reserved values make this extra juicy, let's say Linux uses 14 on
> machine that has 0,8,9,14. QEMU sees 14 and sets the vcpu modes to
> 0,8,9,10 -- it's not even a subset of the host CPU.
> (There might be similar problems even with future standard extensions.)
Currently QEMU won't recognize anything higher than 10 and it knows how
to skip 1-7. If we need to add more standard support (or custom support)
we'll have to modify set_satp_mode_max_supported(). But let's burn that
bridge later. We should probably sanity check the input to that function
with something like the (untested) diff below, though.
Thanks,
drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 09ded6829a2c..a9088c7b1770 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -435,10 +435,18 @@ const char *satp_mode_str(uint8_t satp_mode, bool
is_32_bit)
}
static void set_satp_mode_max_supported(RISCVCPU *cpu,
- uint8_t satp_mode)
+ uint8_t satp_mode,
+ Error **errp)
{
bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
+ int satp_max = rv32 ? VM_1_10_SV32 : VM_1_10_MAX;
+
+ if (satp_mode > satp_max) {
+ error_setg(errp, "satp_mode %s is higher than the supported max %s",
+ satp_mode_str(satp_max, rv32),
+ satp_mode_str(satp_max, rv32));
+ }
for (int i = 0; i <= satp_mode; ++i) {
if (valid_vm[i]) {
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index a30317c61781..46d206abba97 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -662,6 +662,7 @@ typedef enum {
#define VM_1_10_SV48 9
#define VM_1_10_SV57 10
#define VM_1_10_SV64 11
+#define VM_1_10_MAX VM_1_10_SV64
/* Page table entry (PTE) fields */
#define PTE_V 0x001 /* Valid */
