Re: [PATCH v4 16/17] KVM: introduce a KVM_DESTROY_DEVICE ioctl

["Cédric Le Goater" <clg@kaod.org>]

On 3/20/19 9:37 AM, Cédric Le Goater wrote:
> The 'destroy' method is currently used to destroy all devices when the
> VM is destroyed after the vCPUs have been freed.
> 
> This new KVM ioctl exposes the same KVM device method. It acts as a
> software reset of the VM to 'destroy' selected devices when necessary
> and perform the required cleanups on the vCPUs. Called with the
> kvm->lock.
> 
> The 'destroy' method could be improved by returning an error code.

I am working on a new approach for the interrupt device switching 
that I will send as a replacement of patch 16 and 17.  This is for 
discussion and a v5 will be needed at the end.

C.  

 


> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> 
>  Changes since v3 :
> 
>  - Removed temporary TODO comment in kvm_ioctl_destroy_device()
>    regarding kvm_put_kvm()
>  
>  Changes since v2 :
> 
>  - checked that device is owned by VM
>  
>  include/uapi/linux/kvm.h          |  7 ++++++
>  virt/kvm/kvm_main.c               | 41 +++++++++++++++++++++++++++++++
>  Documentation/virtual/kvm/api.txt | 20 +++++++++++++++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 52bf74a1616e..d78fafa54274 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1183,6 +1183,11 @@ struct kvm_create_device {
>  	__u32	flags;	/* in: KVM_CREATE_DEVICE_xxx */
>  };
>  
> +struct kvm_destroy_device {
> +	__u32	fd;	/* in: device handle */
> +	__u32	flags;	/* in: unused */
> +};
> +
>  struct kvm_device_attr {
>  	__u32	flags;		/* no flags currently defined */
>  	__u32	group;		/* device-defined */
> @@ -1331,6 +1336,8 @@ struct kvm_s390_ucas_mapping {
>  #define KVM_GET_DEVICE_ATTR	  _IOW(KVMIO,  0xe2, struct kvm_device_attr)
>  #define KVM_HAS_DEVICE_ATTR	  _IOW(KVMIO,  0xe3, struct kvm_device_attr)
>  
> +#define KVM_DESTROY_DEVICE	  _IOWR(KVMIO,  0xf0, struct kvm_destroy_device)
> +
>  /*
>   * ioctls for vcpu fds
>   */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5e2fa5c7dd1a..9601c2ddecc5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3032,6 +3032,33 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>  	return 0;
>  }
>  
> +static int kvm_ioctl_destroy_device(struct kvm *kvm,
> +				    struct kvm_destroy_device *dd)
> +{
> +	struct fd f;
> +	struct kvm_device *dev;
> +
> +	f = fdget(dd->fd);
> +	if (!f.file)
> +		return -EBADF;
> +
> +	dev = kvm_device_from_filp(f.file);
> +	fdput(f);
> +
> +	if (!dev)
> +		return -ENODEV;
> +
> +	if (dev->kvm != kvm)
> +		return -EPERM;
> +
> +	mutex_lock(&kvm->lock);
> +	list_del(&dev->vm_node);
> +	dev->ops->destroy(dev);
> +	mutex_unlock(&kvm->lock);
> +
> +	return 0;
> +}
> +
>  static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>  {
>  	switch (arg) {
> @@ -3276,6 +3303,20 @@ static long kvm_vm_ioctl(struct file *filp,
>  		r = 0;
>  		break;
>  	}
> +	case KVM_DESTROY_DEVICE: {
> +		struct kvm_destroy_device dd;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&dd, argp, sizeof(dd)))
> +			goto out;
> +
> +		r = kvm_ioctl_destroy_device(kvm, &dd);
> +		if (r)
> +			goto out;
> +
> +		r = 0;
> +		break;
> +	}
>  	case KVM_CHECK_EXTENSION:
>  		r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
>  		break;
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 8022ecce2c47..abe8433adf4f 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3874,6 +3874,26 @@ number of valid entries in the 'entries' array, which is then filled.
>  'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently reserved,
>  userspace should not expect to get any particular value there.
>  
> +4.119 KVM_DESTROY_DEVICE
> +
> +Capability: KVM_CAP_DEVICE_CTRL
> +Type: vm ioctl
> +Parameters: struct kvm_destroy_device (in)
> +Returns: 0 on success, -1 on error
> +Errors:
> +  ENODEV: The device type is unknown or unsupported
> +  EPERM: The device does not belong to the VM
> +
> +  Other error conditions may be defined by individual device types or
> +  have their standard meanings.
> +
> +Destroys an emulated device in the kernel.
> +
> +struct kvm_destroy_device {
> +	__u32	fd;	/* in: device handle */
> +	__u32	flags;	/* unused */
> +};
> +
>  5. The kvm_run structure
>  ------------------------
>  
>