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 <[email protected]> > Signed-off-by: Marc Zyngier <[email protected]> > --- > 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 <[email protected]> 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 [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
