Re: [PATCH 1/2] KVM: introduce a 'release' method for KVM devices

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2820 bytes --]

On Thu, Apr 11, 2019 at 03:53:01PM +0200, Cédric Le Goater wrote:
> When a P9 sPAPR VM boots, the CAS negotiation process determines which
> interrupt mode to use (XICS legacy or XIVE native) and invokes a
> machine reset to activate the chosen mode.
> 
> To be able to switch from one interrupt mode to another, we introduce
> the capability to release a KVM device without destroying the VM. The
> KVM device interface is extended with a new 'release' method which is
> called when the file descriptor of the device is closed.
> 
> Once 'release' is called, the 'destroy' method will not be called
> anymore as the device is removed from the device list of the VM.
> 
> 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>

Kind of a hack, but a better way to solve this that doesn't involve
inordinate amounts of work doesn't occur to me.

> ---
>  include/linux/kvm_host.h |  9 +++++++++
>  virt/kvm/kvm_main.c      | 13 +++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 831d963451d8..722692e2f745 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1240,6 +1240,15 @@ struct kvm_device_ops {
>  	 */
>  	void (*destroy)(struct kvm_device *dev);
>  
> +	/*
> +	 * Release is an alternative method to free the device. It is
> +	 * called when the device file descriptor is closed. Once
> +	 * release is called, the destroy method will not be called
> +	 * anymore as the device is removed from the device list of
> +	 * the VM. kvm->lock is held.
> +	 */
> +	void (*release)(struct kvm_device *dev);
> +
>  	int (*set_attr)(struct kvm_device *dev, struct kvm_device_attr *attr);
>  	int (*get_attr)(struct kvm_device *dev, struct kvm_device_attr *attr);
>  	int (*has_attr)(struct kvm_device *dev, struct kvm_device_attr *attr);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ea2018ae1cd7..ea2619d5ca98 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2938,6 +2938,19 @@ static int kvm_device_release(struct inode *inode, struct file *filp)
>  	struct kvm_device *dev = filp->private_data;
>  	struct kvm *kvm = dev->kvm;
>  
> +	if (!dev)
> +		return -ENODEV;
> +
> +	if (dev->kvm != kvm)
> +		return -EPERM;
> +
> +	if (dev->ops->release) {
> +		mutex_lock(&kvm->lock);
> +		list_del(&dev->vm_node);
> +		dev->ops->release(dev);
> +		mutex_unlock(&kvm->lock);
> +	}
> +
>  	kvm_put_kvm(kvm);
>  	return 0;
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]