Patchwork [34/42] kvm: destroy emulated devices on VM exit

login
register
mail settings
Submitter Alexander Graf
Date April 26, 2013, 6:30 p.m.
Message ID <1367001037-10394-56-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/239930/
State New
Headers show

Comments

Alexander Graf - April 26, 2013, 6:30 p.m.
From: Scott Wood <scottwood@freescale.com>

The hassle of getting refcounting right was greater than the hassle
of keeping a list of devices to destroy on VM exit.

Signed-off-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kvm/mpic.c  |    2 --
 include/linux/kvm_host.h |    3 ++-
 virt/kvm/kvm_main.c      |   29 ++++++++++++++++-------------
 3 files changed, 18 insertions(+), 16 deletions(-)
Gleb Natapov - April 28, 2013, 5:43 a.m.
On Fri, Apr 26, 2013 at 08:30:29PM +0200, Alexander Graf wrote:
> From: Scott Wood <scottwood@freescale.com>
> 
> The hassle of getting refcounting right was greater than the hassle
> of keeping a list of devices to destroy on VM exit.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/kvm/mpic.c  |    2 --
>  include/linux/kvm_host.h |    3 ++-
>  virt/kvm/kvm_main.c      |   29 ++++++++++++++++-------------
>  3 files changed, 18 insertions(+), 16 deletions(-)
> 
Just fold it into respective patches in the series. There is not point
to introduce ref counting and remove it in the same patch series.

> diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
> index 89fe1d6..795ca0c 100644
> --- a/arch/powerpc/kvm/mpic.c
> +++ b/arch/powerpc/kvm/mpic.c
> @@ -1781,7 +1781,6 @@ int kvmppc_mpic_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu,
>  	if (opp->mpic_mode_mask == GCR_MODE_PROXY)
>  		vcpu->arch.epr_flags |= KVMPPC_EPR_KERNEL;
>  
> -	kvm_device_get(dev);
>  out:
>  	spin_unlock_irq(&opp->lock);
>  	return ret;
> @@ -1797,7 +1796,6 @@ void kvmppc_mpic_disconnect_vcpu(struct openpic *opp, struct kvm_vcpu *vcpu)
>  	BUG_ON(!opp->dst[vcpu->arch.irq_cpu_id].vcpu);
>  
>  	opp->dst[vcpu->arch.irq_cpu_id].vcpu = NULL;
> -	kvm_device_put(opp->dev);
>  }
>  
>  /*
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index feffbda..36c9776 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -393,6 +393,7 @@ struct kvm {
>  	long mmu_notifier_count;
>  #endif
>  	long tlbs_dirty;
> +	struct list_head devices;
>  };
>  
>  #define kvm_err(fmt, ...) \
> @@ -1069,8 +1070,8 @@ struct kvm_device_ops;
>  struct kvm_device {
>  	struct kvm_device_ops *ops;
>  	struct kvm *kvm;
> -	atomic_t users;
>  	void *private;
> +	struct list_head vm_node;
>  };
>  
>  /* create, destroy, and name are mandatory */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f6cd14d..5da9f02 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -504,6 +504,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>  	mutex_init(&kvm->irq_lock);
>  	mutex_init(&kvm->slots_lock);
>  	atomic_set(&kvm->users_count, 1);
> +	INIT_LIST_HEAD(&kvm->devices);
>  
>  	r = kvm_init_mmu_notifier(kvm);
>  	if (r)
> @@ -581,6 +582,19 @@ void kvm_free_physmem(struct kvm *kvm)
>  	kfree(kvm->memslots);
>  }
>  
> +static void kvm_destroy_devices(struct kvm *kvm)
> +{
> +	struct list_head *node, *tmp;
> +
> +	list_for_each_safe(node, tmp, &kvm->devices) {
> +		struct kvm_device *dev =
> +			list_entry(node, struct kvm_device, vm_node);
> +
> +		list_del(node);
> +		dev->ops->destroy(dev);
> +	}
> +}
> +
>  static void kvm_destroy_vm(struct kvm *kvm)
>  {
>  	int i;
> @@ -600,6 +614,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  	kvm_arch_flush_shadow_all(kvm);
>  #endif
>  	kvm_arch_destroy_vm(kvm);
> +	kvm_destroy_devices(kvm);
>  	kvm_free_physmem(kvm);
>  	cleanup_srcu_struct(&kvm->srcu);
>  	kvm_arch_free_vm(kvm);
> @@ -2195,23 +2210,11 @@ static long kvm_device_ioctl(struct file *filp, unsigned int ioctl,
>  	}
>  }
>  
> -void kvm_device_get(struct kvm_device *dev)
> -{
> -	atomic_inc(&dev->users);
> -}
> -
> -void kvm_device_put(struct kvm_device *dev)
> -{
> -	if (atomic_dec_and_test(&dev->users))
> -		dev->ops->destroy(dev);
> -}
> -
>  static int kvm_device_release(struct inode *inode, struct file *filp)
>  {
>  	struct kvm_device *dev = filp->private_data;
>  	struct kvm *kvm = dev->kvm;
>  
> -	kvm_device_put(dev);
>  	kvm_put_kvm(kvm);
>  	return 0;
>  }
> @@ -2257,7 +2260,6 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>  
>  	dev->ops = ops;
>  	dev->kvm = kvm;
> -	atomic_set(&dev->users, 1);
>  
>  	ret = ops->create(dev, cd->type);
>  	if (ret < 0) {
> @@ -2271,6 +2273,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>  		return ret;
>  	}
>  
> +	list_add(&dev->vm_node, &kvm->devices);
>  	kvm_get_kvm(kvm);
>  	cd->fd = ret;
>  	return 0;
> -- 
> 1.6.0.2

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - April 28, 2013, 9:47 a.m.
On 28.04.2013, at 07:43, Gleb Natapov wrote:

> On Fri, Apr 26, 2013 at 08:30:29PM +0200, Alexander Graf wrote:
>> From: Scott Wood <scottwood@freescale.com>
>> 
>> The hassle of getting refcounting right was greater than the hassle
>> of keeping a list of devices to destroy on VM exit.
>> 
>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/kvm/mpic.c  |    2 --
>> include/linux/kvm_host.h |    3 ++-
>> virt/kvm/kvm_main.c      |   29 ++++++++++++++++-------------
>> 3 files changed, 18 insertions(+), 16 deletions(-)
>> 
> Just fold it into respective patches in the series. There is not point
> to introduce ref counting and remove it in the same patch series.

I can't anymore. This patch set is in my next tree which is in linux-next, so Stephen will yell at me if I rebase it now.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
index 89fe1d6..795ca0c 100644
--- a/arch/powerpc/kvm/mpic.c
+++ b/arch/powerpc/kvm/mpic.c
@@ -1781,7 +1781,6 @@  int kvmppc_mpic_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu,
 	if (opp->mpic_mode_mask == GCR_MODE_PROXY)
 		vcpu->arch.epr_flags |= KVMPPC_EPR_KERNEL;
 
-	kvm_device_get(dev);
 out:
 	spin_unlock_irq(&opp->lock);
 	return ret;
@@ -1797,7 +1796,6 @@  void kvmppc_mpic_disconnect_vcpu(struct openpic *opp, struct kvm_vcpu *vcpu)
 	BUG_ON(!opp->dst[vcpu->arch.irq_cpu_id].vcpu);
 
 	opp->dst[vcpu->arch.irq_cpu_id].vcpu = NULL;
-	kvm_device_put(opp->dev);
 }
 
 /*
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index feffbda..36c9776 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -393,6 +393,7 @@  struct kvm {
 	long mmu_notifier_count;
 #endif
 	long tlbs_dirty;
+	struct list_head devices;
 };
 
 #define kvm_err(fmt, ...) \
@@ -1069,8 +1070,8 @@  struct kvm_device_ops;
 struct kvm_device {
 	struct kvm_device_ops *ops;
 	struct kvm *kvm;
-	atomic_t users;
 	void *private;
+	struct list_head vm_node;
 };
 
 /* create, destroy, and name are mandatory */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f6cd14d..5da9f02 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -504,6 +504,7 @@  static struct kvm *kvm_create_vm(unsigned long type)
 	mutex_init(&kvm->irq_lock);
 	mutex_init(&kvm->slots_lock);
 	atomic_set(&kvm->users_count, 1);
+	INIT_LIST_HEAD(&kvm->devices);
 
 	r = kvm_init_mmu_notifier(kvm);
 	if (r)
@@ -581,6 +582,19 @@  void kvm_free_physmem(struct kvm *kvm)
 	kfree(kvm->memslots);
 }
 
+static void kvm_destroy_devices(struct kvm *kvm)
+{
+	struct list_head *node, *tmp;
+
+	list_for_each_safe(node, tmp, &kvm->devices) {
+		struct kvm_device *dev =
+			list_entry(node, struct kvm_device, vm_node);
+
+		list_del(node);
+		dev->ops->destroy(dev);
+	}
+}
+
 static void kvm_destroy_vm(struct kvm *kvm)
 {
 	int i;
@@ -600,6 +614,7 @@  static void kvm_destroy_vm(struct kvm *kvm)
 	kvm_arch_flush_shadow_all(kvm);
 #endif
 	kvm_arch_destroy_vm(kvm);
+	kvm_destroy_devices(kvm);
 	kvm_free_physmem(kvm);
 	cleanup_srcu_struct(&kvm->srcu);
 	kvm_arch_free_vm(kvm);
@@ -2195,23 +2210,11 @@  static long kvm_device_ioctl(struct file *filp, unsigned int ioctl,
 	}
 }
 
-void kvm_device_get(struct kvm_device *dev)
-{
-	atomic_inc(&dev->users);
-}
-
-void kvm_device_put(struct kvm_device *dev)
-{
-	if (atomic_dec_and_test(&dev->users))
-		dev->ops->destroy(dev);
-}
-
 static int kvm_device_release(struct inode *inode, struct file *filp)
 {
 	struct kvm_device *dev = filp->private_data;
 	struct kvm *kvm = dev->kvm;
 
-	kvm_device_put(dev);
 	kvm_put_kvm(kvm);
 	return 0;
 }
@@ -2257,7 +2260,6 @@  static int kvm_ioctl_create_device(struct kvm *kvm,
 
 	dev->ops = ops;
 	dev->kvm = kvm;
-	atomic_set(&dev->users, 1);
 
 	ret = ops->create(dev, cd->type);
 	if (ret < 0) {
@@ -2271,6 +2273,7 @@  static int kvm_ioctl_create_device(struct kvm *kvm,
 		return ret;
 	}
 
+	list_add(&dev->vm_node, &kvm->devices);
 	kvm_get_kvm(kvm);
 	cd->fd = ret;
 	return 0;