[PATCH 0/3] kvmtool: Limit IPA space to what is actually required

[PATCH 0/3] kvmtool: Limit IPA space to what is actually required

[Marc Zyngier]

KVM hacking on the Apple M1 SoC has shown that kvmtool (and other
VMMs) make pretty poor use of the IPA space parameter (read: do not
use it and just pass 0). This results in a guest that cannot boot
(thanks to a kernel bug), but in general means we don't benefit from
smaller page tables at stage-2.

This series does two things:
- It switches kvmtool away from the default 40bit, allowing large VMs
  to be created (I have booted a 4TB VM)
- It reduces the requested IPA space to be as small as possible

It all looks really good, except that the last patch trips yet another
bug in KVM so better not apply it. Yet.

Marc Zyngier (3):
  kvmtool: Abstract KVM_VM_TYPE into a weak function
  kvmtool: arm64: Use the maximum supported IPA size when creating the
    VM
  kvmtool: arm64: Configure VM with the minimal required IPA space

 arm/aarch64/include/kvm/kvm-arch.h | 19 ++++++++++++++---
 arm/aarch64/kvm.c                  | 33 ++++++++++++++++++++++++++++++
 include/kvm/kvm.h                  |  1 +
 kvm.c                              |  7 ++++++-
 4 files changed, 56 insertions(+), 4 deletions(-)

-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] kvmtool: Abstract KVM_VM_TYPE into a weak function

[Marc Zyngier]

Most architectures pass a fixed value for their VM type. However,
arm64 uses it as a parameter describing the size of the guest's
physical address space.

In order to support this, introduce a kvm__get_vm_type() helper
that only returns KVM_VM_TYPE for now.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/kvm/kvm.h | 1 +
 kvm.c             | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h
index 53373b0..72a42e8 100644
--- a/include/kvm/kvm.h
+++ b/include/kvm/kvm.h
@@ -100,6 +100,7 @@ int kvm__init(struct kvm *kvm);
 struct kvm *kvm__new(void);
 int kvm__recommended_cpus(struct kvm *kvm);
 int kvm__max_cpus(struct kvm *kvm);
+int kvm__get_vm_type(struct kvm *kvm);
 void kvm__init_ram(struct kvm *kvm);
 int kvm__exit(struct kvm *kvm);
 bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename);
diff --git a/kvm.c b/kvm.c
index e327541..5bc66c8 100644
--- a/kvm.c
+++ b/kvm.c
@@ -428,6 +428,11 @@ int kvm__max_cpus(struct kvm *kvm)
 	return ret;
 }
 
+int __attribute__((weak)) kvm__get_vm_type(struct kvm *kvm)
+{
+	return KVM_VM_TYPE;
+}
+
 int kvm__init(struct kvm *kvm)
 {
 	int ret;
@@ -461,7 +466,7 @@ int kvm__init(struct kvm *kvm)
 		goto err_sys_fd;
 	}
 
-	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, KVM_VM_TYPE);
+	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, kvm__get_vm_type(kvm));
 	if (kvm->vm_fd < 0) {
 		pr_err("KVM_CREATE_VM ioctl");
 		ret = kvm->vm_fd;
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] kvmtool: Abstract KVM_VM_TYPE into a weak function

[Andre Przywara]

On Tue,  9 Mar 2021 16:39:07 +0000
Marc Zyngier <maz@kernel.org> wrote:

> Most architectures pass a fixed value for their VM type. However,
> arm64 uses it as a parameter describing the size of the guest's
> physical address space.
> 
> In order to support this, introduce a kvm__get_vm_type() helper
> that only returns KVM_VM_TYPE for now.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  include/kvm/kvm.h | 1 +
>  kvm.c             | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h
> index 53373b0..72a42e8 100644
> --- a/include/kvm/kvm.h
> +++ b/include/kvm/kvm.h
> @@ -100,6 +100,7 @@ int kvm__init(struct kvm *kvm);
>  struct kvm *kvm__new(void);
>  int kvm__recommended_cpus(struct kvm *kvm);
>  int kvm__max_cpus(struct kvm *kvm);
> +int kvm__get_vm_type(struct kvm *kvm);
>  void kvm__init_ram(struct kvm *kvm);
>  int kvm__exit(struct kvm *kvm);
>  bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename);
> diff --git a/kvm.c b/kvm.c
> index e327541..5bc66c8 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -428,6 +428,11 @@ int kvm__max_cpus(struct kvm *kvm)
>  	return ret;
>  }
>  
> +int __attribute__((weak)) kvm__get_vm_type(struct kvm *kvm)
> +{
> +	return KVM_VM_TYPE;
> +}
> +
>  int kvm__init(struct kvm *kvm)
>  {
>  	int ret;
> @@ -461,7 +466,7 @@ int kvm__init(struct kvm *kvm)
>  		goto err_sys_fd;
>  	}
>  
> -	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, KVM_VM_TYPE);
> +	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, kvm__get_vm_type(kvm));
>  	if (kvm->vm_fd < 0) {
>  		pr_err("KVM_CREATE_VM ioctl");
>  		ret = kvm->vm_fd;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] kvmtool: arm64: Use the maximum supported IPA size when creating the VM

[Marc Zyngier]

Instead of just asking the the default VM size, request the maximum
IPA size to the kernel, and use this at VM creation time.

The IPA space is parametrized accordingly.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arm/aarch64/include/kvm/kvm-arch.h | 19 ++++++++++++++++---
 arm/aarch64/kvm.c                  | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
index 55ef8ed..159567b 100644
--- a/arm/aarch64/include/kvm/kvm-arch.h
+++ b/arm/aarch64/include/kvm/kvm-arch.h
@@ -3,10 +3,23 @@
 
 struct kvm;
 unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
+int kvm__arch_get_ipa_limit(struct kvm *kvm);
 
-#define ARM_MAX_MEMORY(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
-				ARM_LOMAP_MAX_MEMORY		:	\
-				ARM_HIMAP_MAX_MEMORY)
+#define ARM_MAX_MEMORY(kvm)	({					\
+	u64 max_ram;							\
+									\
+	if ((kvm)->cfg.arch.aarch32_guest) {				\
+		max_ram = ARM_LOMAP_MAX_MEMORY;				\
+	} else {							\
+		int ipabits = kvm__arch_get_ipa_limit(kvm);		\
+		if (ipabits <= 0)					\
+			max_ram = ARM_HIMAP_MAX_MEMORY;			\
+		else							\
+			max_ram = (1ULL << ipabits) - ARM_MEMORY_AREA;	\
+	}								\
+									\
+	max_ram;							\
+})
 
 #include "arm-common/kvm-arch.h"
 
diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
index 49e1dd3..c4f81d0 100644
--- a/arm/aarch64/kvm.c
+++ b/arm/aarch64/kvm.c
@@ -46,3 +46,18 @@ fail:
 	return 0x80000;
 }
 
+int kvm__arch_get_ipa_limit(struct kvm *kvm)
+{
+	int ret;
+
+	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_VM_IPA_SIZE);
+	if (ret <= 0)
+		ret = 0;
+
+	return ret;
+}
+
+int kvm__get_vm_type(struct kvm *kvm)
+{
+	return kvm__arch_get_ipa_limit(kvm);
+}
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] kvmtool: arm64: Use the maximum supported IPA size when creating the VM

[Andre Przywara]

On Tue,  9 Mar 2021 16:39:08 +0000
Marc Zyngier <maz@kernel.org> wrote:

Hi,

> Instead of just asking the the default VM size, request the maximum
> IPA size to the kernel, and use this at VM creation time.
> 
> The IPA space is parametrized accordingly.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arm/aarch64/include/kvm/kvm-arch.h | 19 ++++++++++++++++---
>  arm/aarch64/kvm.c                  | 15 +++++++++++++++
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> index 55ef8ed..159567b 100644
> --- a/arm/aarch64/include/kvm/kvm-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-arch.h
> @@ -3,10 +3,23 @@
>  
>  struct kvm;
>  unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
> +int kvm__arch_get_ipa_limit(struct kvm *kvm);
>  
> -#define ARM_MAX_MEMORY(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
> -				ARM_LOMAP_MAX_MEMORY		:	\
> -				ARM_HIMAP_MAX_MEMORY)
> +#define ARM_MAX_MEMORY(kvm)	({					\
> +	u64 max_ram;							\
> +									\
> +	if ((kvm)->cfg.arch.aarch32_guest) {				\
> +		max_ram = ARM_LOMAP_MAX_MEMORY;				\
> +	} else {							\
> +		int ipabits = kvm__arch_get_ipa_limit(kvm);		\
> +		if (ipabits <= 0)					\
> +			max_ram = ARM_HIMAP_MAX_MEMORY;			\
> +		else							\
> +			max_ram = (1ULL << ipabits) - ARM_MEMORY_AREA;	\
> +	}								\
> +									\
> +	max_ram;							\
> +})
>  
>  #include "arm-common/kvm-arch.h"
>  
> diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> index 49e1dd3..c4f81d0 100644
> --- a/arm/aarch64/kvm.c
> +++ b/arm/aarch64/kvm.c
> @@ -46,3 +46,18 @@ fail:
>  	return 0x80000;
>  }
>  
> +int kvm__arch_get_ipa_limit(struct kvm *kvm)
> +{
> +	int ret;
> +
> +	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_VM_IPA_SIZE);
> +	if (ret <= 0)
> +		ret = 0;
> +
> +	return ret;
> +}
> +
> +int kvm__get_vm_type(struct kvm *kvm)
> +{
> +	return kvm__arch_get_ipa_limit(kvm);

Shouldn't that be funnelled through KVM_VM_TYPE_ARM_IPA_SIZE()?
Granted, that doesn't really do anything, but if I understand the
api.rst directly, it's logically needed to convert the IPA size into a
VM type.

Other than that looks good.

Cheers,
Andre


> +}


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] kvmtool: arm64: Use the maximum supported IPA size when creating the VM

[Marc Zyngier]

On Thu, 18 Mar 2021 10:21:25 +0000,
Andre Przywara <andre.przywara@arm.com> wrote:
> 
> On Tue,  9 Mar 2021 16:39:08 +0000
> Marc Zyngier <maz@kernel.org> wrote:
> 
> Hi,
> 
> > Instead of just asking the the default VM size, request the maximum
> > IPA size to the kernel, and use this at VM creation time.
> > 
> > The IPA space is parametrized accordingly.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arm/aarch64/include/kvm/kvm-arch.h | 19 ++++++++++++++++---
> >  arm/aarch64/kvm.c                  | 15 +++++++++++++++
> >  2 files changed, 31 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> > index 55ef8ed..159567b 100644
> > --- a/arm/aarch64/include/kvm/kvm-arch.h
> > +++ b/arm/aarch64/include/kvm/kvm-arch.h
> > @@ -3,10 +3,23 @@
> >  
> >  struct kvm;
> >  unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
> > +int kvm__arch_get_ipa_limit(struct kvm *kvm);
> >  
> > -#define ARM_MAX_MEMORY(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
> > -				ARM_LOMAP_MAX_MEMORY		:	\
> > -				ARM_HIMAP_MAX_MEMORY)
> > +#define ARM_MAX_MEMORY(kvm)	({					\
> > +	u64 max_ram;							\
> > +									\
> > +	if ((kvm)->cfg.arch.aarch32_guest) {				\
> > +		max_ram = ARM_LOMAP_MAX_MEMORY;				\
> > +	} else {							\
> > +		int ipabits = kvm__arch_get_ipa_limit(kvm);		\
> > +		if (ipabits <= 0)					\
> > +			max_ram = ARM_HIMAP_MAX_MEMORY;			\
> > +		else							\
> > +			max_ram = (1ULL << ipabits) - ARM_MEMORY_AREA;	\
> > +	}								\
> > +									\
> > +	max_ram;							\
> > +})
> >  
> >  #include "arm-common/kvm-arch.h"
> >  
> > diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> > index 49e1dd3..c4f81d0 100644
> > --- a/arm/aarch64/kvm.c
> > +++ b/arm/aarch64/kvm.c
> > @@ -46,3 +46,18 @@ fail:
> >  	return 0x80000;
> >  }
> >  
> > +int kvm__arch_get_ipa_limit(struct kvm *kvm)
> > +{
> > +	int ret;
> > +
> > +	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_VM_IPA_SIZE);
> > +	if (ret <= 0)
> > +		ret = 0;
> > +
> > +	return ret;
> > +}
> > +
> > +int kvm__get_vm_type(struct kvm *kvm)
> > +{
> > +	return kvm__arch_get_ipa_limit(kvm);
> 
> Shouldn't that be funnelled through KVM_VM_TYPE_ARM_IPA_SIZE()?
> Granted, that doesn't really do anything, but if I understand the
> api.rst directly, it's logically needed to convert the IPA size into a
> VM type.

It wouldn't do anything indeed, until we get the magic extension that
bumps the IPA space beyond 255 bits. Doesn't hurt, I guess...

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3][Do not merge] kvmtool: arm64: Configure VM with the minimal required IPA space

[Marc Zyngier]

There is some value in keeping the IPA space small, as it reduces
the size of the stage-2 page tables.

Let's compute the required space at VM creation time, and inform
the kernel of our requirements.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arm/aarch64/kvm.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
index c4f81d0..981cb42 100644
--- a/arm/aarch64/kvm.c
+++ b/arm/aarch64/kvm.c
@@ -3,6 +3,7 @@
 #include <asm/image.h>
 
 #include <linux/byteorder.h>
+#include <kvm/util.h>
 
 /*
  * Return the TEXT_OFFSET value that the guest kernel expects. Note
@@ -59,5 +60,22 @@ int kvm__arch_get_ipa_limit(struct kvm *kvm)
 
 int kvm__get_vm_type(struct kvm *kvm)
 {
-	return kvm__arch_get_ipa_limit(kvm);
+	unsigned int ipa_bits, max_ipa_bits;
+	unsigned long max_ipa;
+
+	/* If we're running on an old kernel, use 0 as the VM type */
+	max_ipa_bits = kvm__arch_get_ipa_limit(kvm);
+	if (!max_ipa_bits)
+		return 0;
+
+	/* Otherwise, compute the minimal required IPA size */
+	max_ipa = ARM_MEMORY_AREA + kvm->cfg.ram_size - 1;
+	ipa_bits = max(32, fls_long(max_ipa));
+	pr_debug("max_ipa %lx ipa_bits %d max_ipa_bits %d",
+		 max_ipa, ipa_bits, max_ipa_bits);
+
+	if (ipa_bits > max_ipa_bits)
+		die("Memory too large for this system (needs %d bits, %d available)", ipa_bits, max_ipa_bits);
+
+	return ipa_bits;
 }
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel