kvm: Request VM specific limits instead of system-wide ones

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

Reported-by Ard Biesheuvel <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kvm.c +1 -1

index e327541..3d5173d 100644
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
 	if (ret <= 0)
 		/*
 		 * api.txt states that if KVM_CAP_NR_VCPUS does not exist,

Hi,

Quoted 29 lines · Marc Zyngier

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

Reported-by Ard Biesheuvel <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kvm.c +1 -1

index e327541..3d5173d 100644
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
 	if (ret <= 0)
 		/*
 		 * api.txt states that if KVM_CAP_NR_VCPUS does not exist,

kvm.c int kvm__max_cpus(struct kvm *kvm) · line 427

Quoted 8 lines

kvm.c +1 -1

@@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
 	if (ret <= 0)
 		ret = kvm__recommended_cpus(kvm);
 

I've checked that gic__create comes before the call kvm__recommended_capus:
gic__create is in core_init (called via kvm__init->kvm_arch_init), and kvm__recommended_cpus is in base_init (called via kvm__cpu_init -> kvm__{recommended,max}_cpus).

The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool already has a function for checking extensions on the vm fd, it's called kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on the vm fd?

Thanks,
Alex

Hi,

Quoted 45 lines · Alexandru Elisei

Hi,

Quoted 29 lines · Marc Zyngier

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

Reported-by Ard Biesheuvel <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kvm.c +1 -1

index e327541..3d5173d 100644
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
 	if (ret <= 0)
 		/*
 		 * api.txt states that if KVM_CAP_NR_VCPUS does not exist,

kvm.c int kvm__max_cpus(struct kvm *kvm) · line 427

Quoted 8 lines

kvm.c +1 -1

@@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
 	if (ret <= 0)
 		ret = kvm__recommended_cpus(kvm);
 

I've checked that gic__create comes before the call kvm__recommended_capus:
gic__create is in core_init (called via kvm__init->kvm_arch_init), and kvm__recommended_cpus is in base_init (called via kvm__cpu_init -> kvm__{recommended,max}_cpus).

Quoted 5 lines

The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool already has a function for checking extensions on the vm fd, it's called kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on the vm fd?

Scratch that, kvm__supports_vm_extension returns a bool, not an int. How about we write kvm__check_vm_extension that returns an int, and kvm__supports_vm_extension calls it?

Quoted 53 lines · On Mon, 27 Apr 2020 16:00:58 +0100 Alexandru Elisei <[email protected]>…

Hi,

Quoted 50 lines · Alexandru Elisei

Hi,

Quoted 29 lines · Marc Zyngier

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

Reported-by Ard Biesheuvel <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kvm.c +1 -1

index e327541..3d5173d 100644
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
 	if (ret <= 0)
 		/*
 		 * api.txt states that if KVM_CAP_NR_VCPUS does not exist,

kvm.c int kvm__max_cpus(struct kvm *kvm) · line 427

Quoted 8 lines

kvm.c +1 -1

@@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
 	if (ret <= 0)
 		ret = kvm__recommended_cpus(kvm);
 

I've checked that gic__create comes before the call kvm__recommended_capus:
gic__create is in core_init (called via kvm__init->kvm_arch_init), and kvm__recommended_cpus is in base_init (called via kvm__cpu_init -> kvm__{recommended,max}_cpus).

The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool already has a function for checking extensions on the vm fd, it's called kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on the vm fd?

Quoted 3 lines

Scratch that, kvm__supports_vm_extension returns a bool, not an int. How about we write kvm__check_vm_extension that returns an int, and kvm__supports_vm_extension calls it?

That, or we just change the return type for kvm__supports_vm_extension, and hack the only places that uses it so far (the GIC code) to detect the error.

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

Reported-by Ard Biesheuvel <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kvm.c +2 -2

index e327541..3d5173d 100644
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
 	if (ret <= 0)
 		/*
 		 * api.txt states that if KVM_CAP_NR_VCPUS does not exist,

@@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
 {
 	int ret;
 
-	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
 	if (ret <= 0)
 		ret = kvm__recommended_cpus(kvm);
 

On 27/04/2020 15:17, Marc Zyngier wrote:
Hi,

Quoted 8 lines

On arm64, the maximum number of vcpus is constrained by the type of interrupt controller that has been selected (GICv2 imposes a limit of 8 vcpus, while GICv3 currently has a limit of 512).

It is thus important to request this limit on the VM file descriptor rather than on the one that corresponds to /dev/kvm, as the latter is likely to return something that doesn't take the constraints into account.

That sounds reasonable, but I fail to find any distinction in the kernel code. We don't make any difference between the VM or the system FD in the ioctl handler for those two extensions. For arm64 we always return max. 512 (max VCPUs on GICv3), and number of online host cores for the recommended value. For arm there was a distinction between GICv3 support compiled in or not, but otherwise the same constant values returned. Quickly tested on Juno and N1SDP, the ioctls return the same expected values, regardless of sys_fd vs vm_fd.

So what am I missing here? Is this for some older or even newer kernels?