| Message ID | 20230705091535.237765-1-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISC-V: KVM: provide UAPI for host SATP mode | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 54cdede08f2f |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 2831 this patch: 2831 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 15866 this patch: 15866 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Wed, Jul 05, 2023 at 06:15:35AM -0300, Daniel Henrique Barboza wrote: > KVM userspaces need to be aware of the host SATP to allow them to > advertise it back to the guest OS. > > Since this information is used to build the guest FDT we can't wait for > the SATP reg to be readable. We just need to read the SATP mode, thus > we can use the existing 'satp_mode' global that represents the SATP reg > with MODE set and both ASID and PPN cleared. E.g. for a 32 bit host > running with sv32 satp_mode is 0x80000000, for a 64 bit host running > sv57 satp_mode is 0xa000000000000000, and so on. > > Add a new userspace virtual config register 'satp_mode' to allow > userspace to read the current SATP mode the host is using with > GET_ONE_REG API before spinning the vcpu. > > 'satp_mode' can't be changed via KVM, so SET_ONE_REG is allowed as long > as userspace writes the existing 'satp_mode'. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > arch/riscv/include/asm/csr.h | 2 ++ > arch/riscv/include/uapi/asm/kvm.h | 1 + > arch/riscv/kvm/vcpu.c | 7 +++++++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h > index b6acb7ed115f..be6e5c305e5b 100644 > --- a/arch/riscv/include/asm/csr.h > +++ b/arch/riscv/include/asm/csr.h > @@ -46,6 +46,7 @@ > #ifndef CONFIG_64BIT > #define SATP_PPN _AC(0x003FFFFF, UL) > #define SATP_MODE_32 _AC(0x80000000, UL) > +#define SATP_MODE_SHIFT 31 > #define SATP_ASID_BITS 9 > #define SATP_ASID_SHIFT 22 > #define SATP_ASID_MASK _AC(0x1FF, UL) > @@ -54,6 +55,7 @@ > #define SATP_MODE_39 _AC(0x8000000000000000, UL) > #define SATP_MODE_48 _AC(0x9000000000000000, UL) > #define SATP_MODE_57 _AC(0xa000000000000000, UL) > +#define SATP_MODE_SHIFT 60 > #define SATP_ASID_BITS 16 > #define SATP_ASID_SHIFT 44 > #define SATP_ASID_MASK _AC(0xFFFF, UL) > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h > index f92790c9481a..0493c078e64e 100644 > --- a/arch/riscv/include/uapi/asm/kvm.h > +++ b/arch/riscv/include/uapi/asm/kvm.h > @@ -54,6 +54,7 @@ struct kvm_riscv_config { > unsigned long marchid; > unsigned long mimpid; > unsigned long zicboz_block_size; > + unsigned long satp_mode; > }; > > /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */ > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c > index 8bd9f2a8a0b9..b31acf923802 100644 > --- a/arch/riscv/kvm/vcpu.c > +++ b/arch/riscv/kvm/vcpu.c > @@ -313,6 +313,9 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu, > case KVM_REG_RISCV_CONFIG_REG(mimpid): > reg_val = vcpu->arch.mimpid; > break; > + case KVM_REG_RISCV_CONFIG_REG(satp_mode): > + reg_val = satp_mode >> SATP_MODE_SHIFT; > + break; > default: > return -EINVAL; > } > @@ -395,6 +398,10 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu, > else > return -EBUSY; > break; > + case KVM_REG_RISCV_CONFIG_REG(satp_mode): > + if (reg_val != (satp_mode >> SATP_MODE_SHIFT)) > + return -EINVAL; > + break; > default: > return -EINVAL; > } > -- > 2.41.0 > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
On 05/07/2023 11:15, Daniel Henrique Barboza wrote: > KVM userspaces need to be aware of the host SATP to allow them to > advertise it back to the guest OS. > > Since this information is used to build the guest FDT we can't wait for The thing is the "mmu-type" property in the FDT is never used: the kernel will probe the hardware and choose the largest available mode, or use "no4lvl"/"no5lvl" from the command line to restrict this mode. And FYI the current mode is exposed through cpuinfo. @Conor Can we deprecate this node or something similar? Just a remark, not sure that helps :) > the SATP reg to be readable. We just need to read the SATP mode, thus > we can use the existing 'satp_mode' global that represents the SATP reg > with MODE set and both ASID and PPN cleared. E.g. for a 32 bit host > running with sv32 satp_mode is 0x80000000, for a 64 bit host running > sv57 satp_mode is 0xa000000000000000, and so on. > > Add a new userspace virtual config register 'satp_mode' to allow > userspace to read the current SATP mode the host is using with > GET_ONE_REG API before spinning the vcpu. > > 'satp_mode' can't be changed via KVM, so SET_ONE_REG is allowed as long > as userspace writes the existing 'satp_mode'. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > arch/riscv/include/asm/csr.h | 2 ++ > arch/riscv/include/uapi/asm/kvm.h | 1 + > arch/riscv/kvm/vcpu.c | 7 +++++++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h > index b6acb7ed115f..be6e5c305e5b 100644 > --- a/arch/riscv/include/asm/csr.h > +++ b/arch/riscv/include/asm/csr.h > @@ -46,6 +46,7 @@ > #ifndef CONFIG_64BIT > #define SATP_PPN _AC(0x003FFFFF, UL) > #define SATP_MODE_32 _AC(0x80000000, UL) > +#define SATP_MODE_SHIFT 31 > #define SATP_ASID_BITS 9 > #define SATP_ASID_SHIFT 22 > #define SATP_ASID_MASK _AC(0x1FF, UL) > @@ -54,6 +55,7 @@ > #define SATP_MODE_39 _AC(0x8000000000000000, UL) > #define SATP_MODE_48 _AC(0x9000000000000000, UL) > #define SATP_MODE_57 _AC(0xa000000000000000, UL) > +#define SATP_MODE_SHIFT 60 > #define SATP_ASID_BITS 16 > #define SATP_ASID_SHIFT 44 > #define SATP_ASID_MASK _AC(0xFFFF, UL) > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h > index f92790c9481a..0493c078e64e 100644 > --- a/arch/riscv/include/uapi/asm/kvm.h > +++ b/arch/riscv/include/uapi/asm/kvm.h > @@ -54,6 +54,7 @@ struct kvm_riscv_config { > unsigned long marchid; > unsigned long mimpid; > unsigned long zicboz_block_size; > + unsigned long satp_mode; > }; > > /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */ > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c > index 8bd9f2a8a0b9..b31acf923802 100644 > --- a/arch/riscv/kvm/vcpu.c > +++ b/arch/riscv/kvm/vcpu.c > @@ -313,6 +313,9 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu, > case KVM_REG_RISCV_CONFIG_REG(mimpid): > reg_val = vcpu->arch.mimpid; > break; > + case KVM_REG_RISCV_CONFIG_REG(satp_mode): > + reg_val = satp_mode >> SATP_MODE_SHIFT; > + break; > default: > return -EINVAL; > } > @@ -395,6 +398,10 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu, > else > return -EBUSY; > break; > + case KVM_REG_RISCV_CONFIG_REG(satp_mode): > + if (reg_val != (satp_mode >> SATP_MODE_SHIFT)) > + return -EINVAL; > + break; > default: > return -EINVAL; > }
On 7/5/23 09:18, Alexandre Ghiti wrote: > > On 05/07/2023 11:15, Daniel Henrique Barboza wrote: >> KVM userspaces need to be aware of the host SATP to allow them to >> advertise it back to the guest OS. >> >> Since this information is used to build the guest FDT we can't wait for > > > The thing is the "mmu-type" property in the FDT is never used: the kernel will probe the hardware and choose the largest available mode, or use "no4lvl"/"no5lvl" from the command line to restrict this mode. And FYI the current mode is exposed through cpuinfo. @Conor Can we deprecate this node or something similar? > > Just a remark, not sure that helps :) It does, thanks. I am aware that the current mode is exposed through cpuinfo. mvendorid/marchid/mimpid is also exposed there. As far as I understand we should rely on KVM to provide all CPU related info to configure a vcpu though. A little background of where I'm coming from. One of the QEMU KVM cpu types (host) doesn't have an assigned satp_mode. The FDT creation of the 'virt' board relies on that info being present, and the result is that the board will segfault. I sent a fix for it that I hope will be queued shortly: https://lore.kernel.org/qemu-devel/20230630100811.287315-3-dbarboza@ventanamicro.com/ Thus, if it's decided that the satp_mode FDT is deprecated, we can ignore this patch altogether. Thanks, Daniel > > >> the SATP reg to be readable. We just need to read the SATP mode, thus >> we can use the existing 'satp_mode' global that represents the SATP reg >> with MODE set and both ASID and PPN cleared. E.g. for a 32 bit host >> running with sv32 satp_mode is 0x80000000, for a 64 bit host running >> sv57 satp_mode is 0xa000000000000000, and so on. >> >> Add a new userspace virtual config register 'satp_mode' to allow >> userspace to read the current SATP mode the host is using with >> GET_ONE_REG API before spinning the vcpu. >> >> 'satp_mode' can't be changed via KVM, so SET_ONE_REG is allowed as long >> as userspace writes the existing 'satp_mode'. >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> arch/riscv/include/asm/csr.h | 2 ++ >> arch/riscv/include/uapi/asm/kvm.h | 1 + >> arch/riscv/kvm/vcpu.c | 7 +++++++ >> 3 files changed, 10 insertions(+) >> >> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h >> index b6acb7ed115f..be6e5c305e5b 100644 >> --- a/arch/riscv/include/asm/csr.h >> +++ b/arch/riscv/include/asm/csr.h >> @@ -46,6 +46,7 @@ >> #ifndef CONFIG_64BIT >> #define SATP_PPN _AC(0x003FFFFF, UL) >> #define SATP_MODE_32 _AC(0x80000000, UL) >> +#define SATP_MODE_SHIFT 31 >> #define SATP_ASID_BITS 9 >> #define SATP_ASID_SHIFT 22 >> #define SATP_ASID_MASK _AC(0x1FF, UL) >> @@ -54,6 +55,7 @@ >> #define SATP_MODE_39 _AC(0x8000000000000000, UL) >> #define SATP_MODE_48 _AC(0x9000000000000000, UL) >> #define SATP_MODE_57 _AC(0xa000000000000000, UL) >> +#define SATP_MODE_SHIFT 60 >> #define SATP_ASID_BITS 16 >> #define SATP_ASID_SHIFT 44 >> #define SATP_ASID_MASK _AC(0xFFFF, UL) >> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h >> index f92790c9481a..0493c078e64e 100644 >> --- a/arch/riscv/include/uapi/asm/kvm.h >> +++ b/arch/riscv/include/uapi/asm/kvm.h >> @@ -54,6 +54,7 @@ struct kvm_riscv_config { >> unsigned long marchid; >> unsigned long mimpid; >> unsigned long zicboz_block_size; >> + unsigned long satp_mode; >> }; >> /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */ >> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c >> index 8bd9f2a8a0b9..b31acf923802 100644 >> --- a/arch/riscv/kvm/vcpu.c >> +++ b/arch/riscv/kvm/vcpu.c >> @@ -313,6 +313,9 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu, >> case KVM_REG_RISCV_CONFIG_REG(mimpid): >> reg_val = vcpu->arch.mimpid; >> break; >> + case KVM_REG_RISCV_CONFIG_REG(satp_mode): >> + reg_val = satp_mode >> SATP_MODE_SHIFT; >> + break; >> default: >> return -EINVAL; >> } >> @@ -395,6 +398,10 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu, >> else >> return -EBUSY; >> break; >> + case KVM_REG_RISCV_CONFIG_REG(satp_mode): >> + if (reg_val != (satp_mode >> SATP_MODE_SHIFT)) >> + return -EINVAL; >> + break; >> default: >> return -EINVAL; >> }
On Wed, Jul 05, 2023 at 09:33:26AM -0300, Daniel Henrique Barboza wrote: > > > On 7/5/23 09:18, Alexandre Ghiti wrote: > > > > On 05/07/2023 11:15, Daniel Henrique Barboza wrote: > > > KVM userspaces need to be aware of the host SATP to allow them to > > > advertise it back to the guest OS. > > > > > > Since this information is used to build the guest FDT we can't wait for > > > > > > The thing is the "mmu-type" property in the FDT is never used: the kernel will probe the hardware and choose the largest available mode, or use "no4lvl"/"no5lvl" from the command line to restrict this mode. And FYI the current mode is exposed through cpuinfo. @Conor Can we deprecate this node or something similar? > > > > Just a remark, not sure that helps :) > > It does, thanks. I am aware that the current mode is exposed through cpuinfo. > mvendorid/marchid/mimpid is also exposed there. As far as I understand we should > rely on KVM to provide all CPU related info to configure a vcpu though. > > A little background of where I'm coming from. One of the QEMU KVM cpu types (host) > doesn't have an assigned satp_mode. The FDT creation of the 'virt' board relies on > that info being present, and the result is that the board will segfault. I sent a > fix for it that I hope will be queued shortly: > > https://lore.kernel.org/qemu-devel/20230630100811.287315-3-dbarboza@ventanamicro.com/ > > Thus, if it's decided that the satp_mode FDT is deprecated, we can ignore this > patch altogether. Thanks, We'll eventually want the ability to get and set vsatp.mode from the VMM, so we'll want KVM_REG_RISCV_CONFIG_REG(satp_mode) anyway. For now, since we only support CPU passthrough with KVM, it's a convenient way to read the host's mode (while PPC qemu-kvm does read cpuinfo, I'm not aware of any other qemu-kvm doing that). Thanks, drew
Hey, On Wed, Jul 05, 2023 at 02:58:24PM +0200, Andrew Jones wrote: > On Wed, Jul 05, 2023 at 09:33:26AM -0300, Daniel Henrique Barboza wrote: > > On 7/5/23 09:18, Alexandre Ghiti wrote: > > > On 05/07/2023 11:15, Daniel Henrique Barboza wrote: > > > > KVM userspaces need to be aware of the host SATP to allow them to > > > > advertise it back to the guest OS. > > > > > > > > Since this information is used to build the guest FDT we can't wait for > > > > > > > > > The thing is the "mmu-type" property in the FDT is never used: > > > the kernel will probe the hardware and choose the largest > > > available mode, or use "no4lvl"/"no5lvl" from the command line to > > > restrict this mode. And FYI the current mode is exposed through > > > cpuinfo. @Conor Can we deprecate this node or something similar? I'd be loathe to deprecate it just because Linux doesn't use it. I know nothing really of the BSDs or other operating systems, but from a quick check of FreeBSD: sys/riscv/riscv/mp_machdep.c 400-static bool 401-cpu_check_mmu(u_int id __unused, phandle_t node, u_int addr_size __unused, 402- pcell_t *reg __unused) 403-{ 404- char type[32]; 405- 406- /* Check if this hart supports MMU. */ 407: if (OF_getprop(node, "mmu-type", (void *)type, sizeof(type)) == -1 || 408- strncmp(type, "riscv,none", 10) == 0) 409- return (false); 410- 411- return (true); 412-} Similarly in U-Boot: mmu = dev_read_string(dev, "mmu-type"); if (mmu) info->features |= BIT(CPU_FEAT_MMU); (@Drew, I remember why now that the property is optional - nommu exists) Seems like you should indeed propagate this to the DT you give to guests. Cheers, Conor. > > > > > > Just a remark, not sure that helps :) > > > > It does, thanks. I am aware that the current mode is exposed through cpuinfo. > > mvendorid/marchid/mimpid is also exposed there. As far as I understand we should > > rely on KVM to provide all CPU related info to configure a vcpu though. > > > > A little background of where I'm coming from. One of the QEMU KVM cpu types (host) > > doesn't have an assigned satp_mode. The FDT creation of the 'virt' board relies on > > that info being present, and the result is that the board will segfault. I sent a > > fix for it that I hope will be queued shortly: > > > > https://lore.kernel.org/qemu-devel/20230630100811.287315-3-dbarboza@ventanamicro.com/ > > > > Thus, if it's decided that the satp_mode FDT is deprecated, we can ignore this > > patch altogether. Thanks, > > We'll eventually want the ability to get and set vsatp.mode from the VMM, > so we'll want KVM_REG_RISCV_CONFIG_REG(satp_mode) anyway. For now, since > we only support CPU passthrough with KVM, it's a convenient way to read > the host's mode (while PPC qemu-kvm does read cpuinfo, I'm not aware of > any other qemu-kvm doing that). > > Thanks, > drew > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Jul 05, 2023 at 03:44:02PM +0100, Conor Dooley wrote:
> (@Drew, I remember why now that the property is optional - nommu exists)
Wait, no that makes no sense - there's "riscv,none" for that.
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index b6acb7ed115f..be6e5c305e5b 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -46,6 +46,7 @@ #ifndef CONFIG_64BIT #define SATP_PPN _AC(0x003FFFFF, UL) #define SATP_MODE_32 _AC(0x80000000, UL) +#define SATP_MODE_SHIFT 31 #define SATP_ASID_BITS 9 #define SATP_ASID_SHIFT 22 #define SATP_ASID_MASK _AC(0x1FF, UL) @@ -54,6 +55,7 @@ #define SATP_MODE_39 _AC(0x8000000000000000, UL) #define SATP_MODE_48 _AC(0x9000000000000000, UL) #define SATP_MODE_57 _AC(0xa000000000000000, UL) +#define SATP_MODE_SHIFT 60 #define SATP_ASID_BITS 16 #define SATP_ASID_SHIFT 44 #define SATP_ASID_MASK _AC(0xFFFF, UL) diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h index f92790c9481a..0493c078e64e 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -54,6 +54,7 @@ struct kvm_riscv_config { unsigned long marchid; unsigned long mimpid; unsigned long zicboz_block_size; + unsigned long satp_mode; }; /* CORE registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */ diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c index 8bd9f2a8a0b9..b31acf923802 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -313,6 +313,9 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu, case KVM_REG_RISCV_CONFIG_REG(mimpid): reg_val = vcpu->arch.mimpid; break; + case KVM_REG_RISCV_CONFIG_REG(satp_mode): + reg_val = satp_mode >> SATP_MODE_SHIFT; + break; default: return -EINVAL; } @@ -395,6 +398,10 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu, else return -EBUSY; break; + case KVM_REG_RISCV_CONFIG_REG(satp_mode): + if (reg_val != (satp_mode >> SATP_MODE_SHIFT)) + return -EINVAL; + break; default: return -EINVAL; }
KVM userspaces need to be aware of the host SATP to allow them to advertise it back to the guest OS. Since this information is used to build the guest FDT we can't wait for the SATP reg to be readable. We just need to read the SATP mode, thus we can use the existing 'satp_mode' global that represents the SATP reg with MODE set and both ASID and PPN cleared. E.g. for a 32 bit host running with sv32 satp_mode is 0x80000000, for a 64 bit host running sv57 satp_mode is 0xa000000000000000, and so on. Add a new userspace virtual config register 'satp_mode' to allow userspace to read the current SATP mode the host is using with GET_ONE_REG API before spinning the vcpu. 'satp_mode' can't be changed via KVM, so SET_ONE_REG is allowed as long as userspace writes the existing 'satp_mode'. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- arch/riscv/include/asm/csr.h | 2 ++ arch/riscv/include/uapi/asm/kvm.h | 1 + arch/riscv/kvm/vcpu.c | 7 +++++++ 3 files changed, 10 insertions(+)