[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28b31907-80d8-3d5f-a3a3-0131621fa4c2@redhat.com>
Date: Tue, 18 Apr 2017 16:16:44 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Radim Krčmář <rkrcmar@...hat.com>,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc: Christoffer Dall <cdall@...aro.org>,
Marc Zyngier <marc.zyngier@....com>,
Christian Borntraeger <borntraeger@...ibm.com>,
Cornelia Huck <cornelia.huck@...ibm.com>,
James Hogan <james.hogan@...tec.com>,
Paul Mackerras <paulus@...abs.org>,
Alexander Graf <agraf@...e.com>
Subject: Re: [PATCH 3/4] KVM: add KVM_CREATE_VM2 system ioctl
On 13/04/2017 22:19, Radim Krčmář wrote:
> This patch allows userspace to tell how many VCPUs it is going to use,
> which can save memory when allocating the kvm->vcpus array. This will
> be done with a new KVM_CREATE_VM2 IOCTL.
>
> An alternative would be to redo kvm->vcpus as a list or protect the
> array with RCU. RCU is slower and a list is not even practical as
> kvm->vcpus are being used for index-based accesses.
>
> We could have an IOCTL that is called in between KVM_CREATE_VM and first
> KVM_CREATE_VCPU and sets the size of the vcpus array, but we'd be making
> one useless allocation. Knowing the desired number of VCPUs from the
> beginning is seems best for now.
>
> This patch also prepares generic code for architectures that will set
> KVM_CONFIGURABLE_MAX_VCPUS to a non-zero value.
Why is KVM_MAX_VCPU_ID or KVM_MAX_VCPUS not enough?
Paolo
> A disputable decision is that KVM_CREATE_VM2 actually works even if
> KVM_CAP_CONFIGURABLE_MAX_VCPUS is 0, but uses that capability for its
> detection.
>
> Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
> ---
> Documentation/virtual/kvm/api.txt | 28 ++++++++++++++++++++++++
> include/linux/kvm_host.h | 3 +++
> include/uapi/linux/kvm.h | 8 +++++++
> virt/kvm/kvm_main.c | 45 +++++++++++++++++++++++++++++++++------
> 4 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index e60be91d8036..461130adbdc7 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3259,6 +3259,34 @@ Otherwise, if the MCE is a corrected error, KVM will just
> store it in the corresponding bank (provided this bank is
> not holding a previously reported uncorrected error).
>
> +
> +4.107 KVM_CREATE_VM2
> +
> +Capability: KVM_CAP_CONFIGURABLE_MAX_VCPUS
> +Architectures: all
> +Type: system ioctl
> +Parameters: struct kvm_vm_config
> +Returns: a VM fd that can be used to control the new virtual machine,
> + -E2BIG if the value of max_vcpus is not supported
> +
> +This is an extension of KVM_CREATE_VM that allows the user to pass more
> +information through
> +
> +struct kvm_vm_config {
> + __u64 type;
> + __u32 max_vcpus;
> + __u8 reserved[52];
> +};
> +
> +type is the argument to KVM_CREATE_VM
> +
> +max_vcpus is the desired maximal number of VCPUs, it must not exceed the value
> +returned by KVM_CAP_CONFIGURABLE_MAX_VCPUS. Value of 0 treated as if userspace
> +passed the value returned by KVM_CAP_MAX_VCPU instead.
> +
> +reserved is must be 0.
> +
> +
> 5. The kvm_run structure
> ------------------------
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 6ba7bc831094..b875c0997328 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -39,6 +39,9 @@
> #ifndef KVM_MAX_VCPU_ID
> #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS
> #endif
> +#ifndef KVM_CONFIGURABLE_MAX_VCPUS
> +#define KVM_CONFIGURABLE_MAX_VCPUS 0U
> +#endif
>
> /*
> * The bit 16 ~ bit 31 of kvm_memory_region::flags are internally used
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6180ea50e9ef..8349c73b3517 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -83,6 +83,12 @@ struct kvm_debug_guest {
>
> /* *** End of deprecated interfaces *** */
>
> +/* for KVM_CREATE_VM2 */
> +struct kvm_vm_config {
> + __u64 type;
> + __u32 max_vcpus;
> + __u8 reserved[52];
> +};
>
> /* for KVM_CREATE_MEMORY_REGION */
> struct kvm_memory_region {
> @@ -713,6 +719,7 @@ struct kvm_ppc_resize_hpt {
> */
> #define KVM_GET_API_VERSION _IO(KVMIO, 0x00)
> #define KVM_CREATE_VM _IO(KVMIO, 0x01) /* returns a VM fd */
> +#define KVM_CREATE_VM2 _IOR(KVMIO, 0x01, struct kvm_vm_config)
> #define KVM_GET_MSR_INDEX_LIST _IOWR(KVMIO, 0x02, struct kvm_msr_list)
>
> #define KVM_S390_ENABLE_SIE _IO(KVMIO, 0x06)
> @@ -892,6 +899,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_MIPS_64BIT 139
> #define KVM_CAP_S390_GS 140
> #define KVM_CAP_S390_AIS 141
> +#define KVM_CAP_CONFIGURABLE_MAX_VCPUS 142
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0f1579f118b4..9ef52fa006ec 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -629,11 +629,20 @@ static inline void kvm_free_vm(struct kvm *kvm)
> kfree(kvm);
> }
>
> -static struct kvm *kvm_create_vm(unsigned long type)
> +static struct kvm *kvm_create_vm(struct kvm_vm_config *vm_config)
> {
> int r, i;
> - struct kvm *kvm = kvm_alloc_vm(KVM_MAX_VCPUS);
> + struct kvm *kvm;
>
> + if (!KVM_CONFIGURABLE_MAX_VCPUS && vm_config->max_vcpus)
> + return ERR_PTR(-EINVAL);
> + if (vm_config->max_vcpus > KVM_CONFIGURABLE_MAX_VCPUS)
> + return ERR_PTR(-E2BIG);
> +
> + if (!vm_config->max_vcpus)
> + vm_config->max_vcpus = KVM_MAX_VCPUS;
> +
> + kvm = kvm_alloc_vm(vm_config->max_vcpus);
> if (!kvm)
> return ERR_PTR(-ENOMEM);
>
> @@ -647,7 +656,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
> refcount_set(&kvm->users_count, 1);
> INIT_LIST_HEAD(&kvm->devices);
>
> - r = kvm_arch_init_vm(kvm, type);
> + r = kvm_arch_init_vm(kvm, vm_config->type);
> if (r)
> goto out_err_no_disable;
>
> @@ -2957,6 +2966,8 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
> #endif
> case KVM_CAP_MAX_VCPU_ID:
> return KVM_MAX_VCPU_ID;
> + case KVM_CAP_CONFIGURABLE_MAX_VCPUS:
> + return KVM_CONFIGURABLE_MAX_VCPUS;
> default:
> break;
> }
> @@ -3182,13 +3193,13 @@ static struct file_operations kvm_vm_fops = {
> .llseek = noop_llseek,
> };
>
> -static int kvm_dev_ioctl_create_vm(unsigned long type)
> +static int kvm_dev_ioctl_create_vm(struct kvm_vm_config *vm_config)
> {
> int r;
> struct kvm *kvm;
> struct file *file;
>
> - kvm = kvm_create_vm(type);
> + kvm = kvm_create_vm(vm_config);
> if (IS_ERR(kvm))
> return PTR_ERR(kvm);
> #ifdef CONFIG_KVM_MMIO
> @@ -3223,6 +3234,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
> static long kvm_dev_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> + void __user *argp = (void __user *)arg;
> long r = -EINVAL;
>
> switch (ioctl) {
> @@ -3231,9 +3243,28 @@ static long kvm_dev_ioctl(struct file *filp,
> goto out;
> r = KVM_API_VERSION;
> break;
> - case KVM_CREATE_VM:
> - r = kvm_dev_ioctl_create_vm(arg);
> + case KVM_CREATE_VM: {
> + struct kvm_vm_config vm_config = {.type = arg};
> +
> + r = kvm_dev_ioctl_create_vm(&vm_config);
> break;
> + }
> + case KVM_CREATE_VM2: {
> + struct kvm_vm_config vm_config, check_reserved = {};
> +
> + r = -EFAULT;
> + if (copy_from_user(&vm_config, argp, sizeof vm_config))
> + goto out;
> +
> + r = -EINVAL;
> + check_reserved.type = vm_config.type;
> + check_reserved.max_vcpus = vm_config.max_vcpus;
> + if (memcmp(&vm_config, &check_reserved, sizeof check_reserved))
> + goto out;
> +
> + r = kvm_dev_ioctl_create_vm(&vm_config);
> + break;
> + }
> case KVM_CHECK_EXTENSION:
> r = kvm_vm_ioctl_check_extension_generic(NULL, arg);
> break;
>
Powered by blists - more mailing lists