Hi,
On 4/27/20 3:15 PM, Marc Zyngier wrote:
> KVM_CAP_MAX_VCPUS always return the maximum possible number of
s/return/returns?
> VCPUs, irrespective of the selected interrupt controller. This
> is pretty misleading for userspace that selects a GICv2 on a GICv3
> system that supports v2 compat: It always gets a maximum of 512
> VCPUs, even if the effective limit is 8. The 9th VCPU will fail
> to be created, which is unexpected as far as userspace is concerned.
>
> Fortunately, we already have the right information stashed in the
> kvm structure, and we can return it as requested.
>
> Reported-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> virt/kvm/arm/arm.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 48d0ec44ad77..f9b0528f7305 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -95,6 +95,11 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> return r;
> }
>
> +static int kvm_arm_default_max_vcpus(void)
> +{
> + return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
> +}
> +
> /**
> * kvm_arch_init_vm - initializes a VM data structure
> * @kvm: pointer to the KVM struct
> @@ -128,8 +133,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> kvm->arch.vmid.vmid_gen = 0;
>
> /* The maximum number of VCPUs is limited by the host's GIC model */
> - kvm->arch.max_vcpus = vgic_present ?
> - kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
> + kvm->arch.max_vcpus = kvm_arm_default_max_vcpus();
Nitpicking, but the comment is not 100% true because the maximum number of vcpus
is limited based on the requested vgic type, even before this patch.
>
> return ret;
> out_free_stage2_pgd:
> @@ -204,10 +208,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> r = num_online_cpus();
Not relevant to this patch. If the host has a GICv3, and userspace requests a
GICv2, it is possible that KVM_CAP_NR_VCPUS > KVM_CAP_MAX_VCPUS. I am curious, I
don't see anything in the KVM API documentation about this case, so I suppose it's
perfectly legal, right?
> break;
> case KVM_CAP_MAX_VCPUS:
> - r = KVM_MAX_VCPUS;
> - break;
> case KVM_CAP_MAX_VCPU_ID:
> - r = KVM_MAX_VCPU_ID;
> + if (kvm)
> + r = kvm->arch.max_vcpus;
> + else
> + r = kvm_arm_default_max_vcpus();
This works as expected - when KVM_CHECK_EXTENSION is called on the kvm fd, struct
kvm is NULL.
> break;
> case KVM_CAP_MSI_DEVID:
> if (!kvm)
The patch looks fine to me:
Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Tested it on a rockpro64, which has a GICv3 that can also emulate a GICv2. When
the vgic is a GICv3, before and after instantiating the device, the ioctl returns
512 on both /dev/kvm and the vm fd, as you would expect. When the vgic is a GICv2,
the ioctl return 512 on /dev/kvm and the vm fd before instantiating the vgic;
afterward it returns 512 on /dev/kvm and 8 on the vm fd:
Tested-by: Alexandru Elisei
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
