kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v5 0/3] plumb kvm/vfio to notify kvm:group attach/detach
@ 2016-11-17 12:57 Jike Song
  2016-11-17 12:57 ` [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier Jike Song
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jike Song @ 2016-11-17 12:57 UTC (permalink / raw)
  To: alex.williamson, pbonzini, guangrong.xiao
  Cc: kwankhede, cjia, kevin.tian, kvm, Jike Song

So far KVM and VFIO are mostly transparent to each other.
However, there are users who would rely on them both. For
example, KVMGT relies on VFIO to mediate device operations,
and it also relies on KVM for features such as guest page
tracking. To do that, it needs to know which KVM instance
a vfio_group is attached to, when it will be detached.

There is already a kvm_vfio device serving for similar
purpose, this series extends it to allow users register
notifiers on a vfio_group, and notify kvm:group attaching
and detaching events to that.


Based on Kirti's v14 series, without [12/22]:

	http://www.spinics.net/lists/kvm/msg140951.html

Changes v5:
	- separate vfio_group and vfio_iommu events from each other
	- force user to specifiy which type it is interested in, and which events.
	  Register it IFF all events are supported
Changes v4:
	- use a notifier-based implementation in vfio_group
	- combine the notifier along with iommu notifier by Kirti
Changes v3:
	- don't touch kvm_put_kvm, vfio won't need it
	- fix a race condition
Changes v2:
	- As Paolo suggested, drop the type and get/put ops from
	  vfio, assuming that the data will always be kvm.



Jike Song (3):
  vfio: vfio_register_notifier: classify iommu notfier
  vfio: support notifier chain in vfio_group
  kvm: set/clear kvm to/from vfio_group when group add/delete

 drivers/vfio/vfio.c  | 145 +++++++++++++++++++++++++++++++++++++++++++--------
 include/linux/vfio.h |  22 ++++++--
 virt/kvm/vfio.c      |  31 +++++++++++
 3 files changed, 173 insertions(+), 25 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier
  2016-11-17 12:57 [v5 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
@ 2016-11-17 12:57 ` Jike Song
  2016-11-17 22:10   ` Alex Williamson
  2016-11-17 12:57 ` [v5 2/3] vfio: support notifier chain in vfio_group Jike Song
  2016-11-17 12:57 ` [v5 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song
  2 siblings, 1 reply; 6+ messages in thread
From: Jike Song @ 2016-11-17 12:57 UTC (permalink / raw)
  To: alex.williamson, pbonzini, guangrong.xiao
  Cc: kwankhede, cjia, kevin.tian, kvm, Jike Song

Currently vfio_register_notifier assumes that there is only one
notifier chain, which is in vfio_iommu. However, the user might
also be interested in events other than vfio_iommu, for example,
vfio_group. Refactor vfio_{un}register_notifier implementation
to make it feasible.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
 drivers/vfio/vfio.c  | 88 +++++++++++++++++++++++++++++++++++++++-------------
 include/linux/vfio.h | 16 ++++++++--
 2 files changed, 79 insertions(+), 25 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index c815067..819c7f9 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2007,23 +2007,25 @@ int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage)
 }
 EXPORT_SYMBOL(vfio_unpin_pages);
 
-int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
+static int vfio_register_iommu_notifier(struct vfio_group *group,
+					unsigned long *events,
+					struct notifier_block *nb)
 {
 	struct vfio_container *container;
-	struct vfio_group *group;
 	struct vfio_iommu_driver *driver;
 	int ret;
 
-	if (!dev || !nb)
-		return -EINVAL;
+	/* clear known events */
+	if (*events & VFIO_IOMMU_NOTIFY_DMA_UNMAP)
+		*events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
 
-	group = vfio_group_get_from_dev(dev);
-	if (IS_ERR(group))
-		return PTR_ERR(group);
+	/* refuse to register if still events remaining */
+	if (*events)
+		return -EINVAL;
 
 	ret = vfio_group_add_container_user(group);
 	if (ret)
-		goto err_register_nb;
+		return -EINVAL;
 
 	container = group->container;
 	down_read(&container->group_lock);
@@ -2037,29 +2039,19 @@ int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
 	up_read(&container->group_lock);
 	vfio_group_try_dissolve_container(group);
 
-err_register_nb:
-	vfio_group_put(group);
 	return ret;
 }
-EXPORT_SYMBOL(vfio_register_notifier);
 
-int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb)
+static int vfio_unregister_iommu_notifier(struct vfio_group *group,
+					  struct notifier_block *nb)
 {
 	struct vfio_container *container;
-	struct vfio_group *group;
 	struct vfio_iommu_driver *driver;
 	int ret;
 
-	if (!dev || !nb)
-		return -EINVAL;
-
-	group = vfio_group_get_from_dev(dev);
-	if (IS_ERR(group))
-		return PTR_ERR(group);
-
 	ret = vfio_group_add_container_user(group);
 	if (ret)
-		goto err_unregister_nb;
+		return -EINVAL;
 
 	container = group->container;
 	down_read(&container->group_lock);
@@ -2074,7 +2066,59 @@ int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb)
 	up_read(&container->group_lock);
 	vfio_group_try_dissolve_container(group);
 
-err_unregister_nb:
+	return ret;
+}
+
+int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
+			   unsigned long *events,
+			   struct notifier_block *nb)
+{
+	struct vfio_group *group;
+	int ret;
+
+	if (!dev || !nb)
+		return -EINVAL;
+	if (!events || *events == 0)
+		return -EINVAL;
+
+	group = vfio_group_get_from_dev(dev);
+	if (IS_ERR(group))
+		return PTR_ERR(group);
+
+	switch (type) {
+	case VFIO_IOMMU_NOTIFY:
+		ret = vfio_register_iommu_notifier(group, events, nb);
+		break;
+	default:
+		ret = EINVAL;
+	}
+
+	vfio_group_put(group);
+	return ret;
+}
+EXPORT_SYMBOL(vfio_register_notifier);
+
+int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type,
+			     struct notifier_block *nb)
+{
+	struct vfio_group *group;
+	int ret;
+
+	if (!dev || !nb)
+		return -EINVAL;
+
+	group = vfio_group_get_from_dev(dev);
+	if (IS_ERR(group))
+		return PTR_ERR(group);
+
+	switch (type) {
+	case VFIO_IOMMU_NOTIFY:
+		ret = vfio_unregister_iommu_notifier(group, nb);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
 	vfio_group_put(group);
 	return ret;
 }
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 15ff042..6f3ff31 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -107,14 +107,24 @@ extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
 extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
 			    int npage);
 
-#define VFIO_IOMMU_NOTIFY_DMA_UNMAP	(1)
-
+typedef unsigned short vfio_notify_type_t;
 extern int vfio_register_notifier(struct device *dev,
+				  vfio_notify_type_t type,
+				  unsigned long *required_events,
 				  struct notifier_block *nb);
-
 extern int vfio_unregister_notifier(struct device *dev,
+				    vfio_notify_type_t type,
 				    struct notifier_block *nb);
 
+/* each type has independent events */
+enum vfio_notify_type {
+	VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0,
+};
+
+/* events for VFIO_IOMMU_NOTIFY */
+#define VFIO_IOMMU_NOTIFY_DMA_UNMAP	BIT(0)
+
+
 /*
  * Sub-module helpers
  */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [v5 2/3] vfio: support notifier chain in vfio_group
  2016-11-17 12:57 [v5 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
  2016-11-17 12:57 ` [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier Jike Song
@ 2016-11-17 12:57 ` Jike Song
  2016-11-17 22:10   ` Alex Williamson
  2016-11-17 12:57 ` [v5 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song
  2 siblings, 1 reply; 6+ messages in thread
From: Jike Song @ 2016-11-17 12:57 UTC (permalink / raw)
  To: alex.williamson, pbonzini, guangrong.xiao
  Cc: kwankhede, cjia, kevin.tian, kvm, Jike Song

Beyond vfio_iommu events, users might also be interested in
vfio_group events. For example, if a vfio_group is used along
with Qemu/KVM, whenever kvm pointer is set to/cleared from the
vfio_group, users could be notified.

Currently only VFIO_GROUP_NOTIFY_SET_KVM supported.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
 drivers/vfio/vfio.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/vfio.h |  6 ++++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 819c7f9..85a14ee 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -86,6 +86,8 @@ struct vfio_group {
 	struct mutex			unbound_lock;
 	atomic_t			opened;
 	bool				noiommu;
+	struct kvm			*kvm;
+	struct blocking_notifier_head	notifier;
 };
 
 struct vfio_device {
@@ -312,6 +314,10 @@ static void vfio_group_unlock_and_free(struct vfio_group *group)
 	 * that the group is no longer in vfio.group_list.
 	 */
 	iommu_group_unregister_notifier(group->iommu_group, &group->nb);
+
+	/* Any user didn't unregister? */
+	WARN_ON(group->notifier.head);
+
 	kfree(group);
 }
 
@@ -339,6 +345,7 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
 #ifdef CONFIG_VFIO_NOIOMMU
 	group->noiommu = (iommu_group_get_iommudata(iommu_group) == &noiommu);
 #endif
+	BLOCKING_INIT_NOTIFIER_HEAD(&group->notifier);
 
 	group->nb.notifier_call = vfio_iommu_group_notifier;
 
@@ -1015,6 +1022,50 @@ static long vfio_ioctl_check_extension(struct vfio_container *container,
 	return ret;
 }
 
+void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+{
+	group->kvm = kvm;
+	blocking_notifier_call_chain(&group->notifier,
+				VFIO_GROUP_NOTIFY_SET_KVM, kvm);
+}
+EXPORT_SYMBOL_GPL(vfio_group_set_kvm);
+
+static int vfio_register_group_notifier(struct vfio_group *group,
+					unsigned long *events,
+					struct notifier_block *nb)
+{
+	int ret;
+	bool replay = false;
+
+	/* clear known events */
+	if (*events & VFIO_GROUP_NOTIFY_SET_KVM) {
+		replay = true;
+		*events &= ~VFIO_GROUP_NOTIFY_SET_KVM;
+	}
+
+	/* refuse to continue if still events remaining */
+	if (*events)
+		return -EINVAL;
+
+	ret = blocking_notifier_chain_register(&group->notifier, nb);
+
+	/*
+	 * The attaching of kvm and vfio_group might already happen, so
+	 * here we replay once upon registration.
+	 */
+	if (!ret && replay && group->kvm)
+		blocking_notifier_call_chain(&group->notifier,
+					VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
+
+	return ret;
+}
+
+static int vfio_unregister_group_notifier(struct vfio_group *group,
+					 struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&group->notifier, nb);
+}
+
 /* hold write lock on container->group_lock */
 static int __vfio_container_attach_groups(struct vfio_container *container,
 					  struct vfio_iommu_driver *driver,
@@ -2089,6 +2140,9 @@ int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
 	case VFIO_IOMMU_NOTIFY:
 		ret = vfio_register_iommu_notifier(group, events, nb);
 		break;
+	case VFIO_GROUP_NOTIFY:
+		ret = vfio_register_group_notifier(group, events, nb);
+		break;
 	default:
 		ret = EINVAL;
 	}
@@ -2115,6 +2169,9 @@ int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type,
 	case VFIO_IOMMU_NOTIFY:
 		ret = vfio_unregister_iommu_notifier(group, nb);
 		break;
+	case VFIO_GROUP_NOTIFY:
+		ret = vfio_unregister_group_notifier(group, nb);
+		break;
 	default:
 		ret = -EINVAL;
 	}
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 6f3ff31..5d46e3c 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -119,11 +119,17 @@ extern int vfio_unregister_notifier(struct device *dev,
 /* each type has independent events */
 enum vfio_notify_type {
 	VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0,
+	VFIO_GROUP_NOTIFY = (__force vfio_notify_type_t)1,
 };
 
 /* events for VFIO_IOMMU_NOTIFY */
 #define VFIO_IOMMU_NOTIFY_DMA_UNMAP	BIT(0)
 
+/* events for VFIO_GROUP_NOTIFY */
+#define VFIO_GROUP_NOTIFY_SET_KVM	BIT(0)
+
+struct kvm;
+extern void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
 
 /*
  * Sub-module helpers
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [v5 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete
  2016-11-17 12:57 [v5 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
  2016-11-17 12:57 ` [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier Jike Song
  2016-11-17 12:57 ` [v5 2/3] vfio: support notifier chain in vfio_group Jike Song
@ 2016-11-17 12:57 ` Jike Song
  2 siblings, 0 replies; 6+ messages in thread
From: Jike Song @ 2016-11-17 12:57 UTC (permalink / raw)
  To: alex.williamson, pbonzini, guangrong.xiao
  Cc: kwankhede, cjia, kevin.tian, kvm, Jike Song

Sometimes users need to be aware when a vfio_group attaches to a
KVM or detaches from it. KVM already calls get/put method from vfio to
manipulate the vfio_group reference, it can notify vfio_group in
a similar way.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
 virt/kvm/vfio.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 1dd087d..ea5b4bb 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -60,6 +60,32 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
 	symbol_put(vfio_group_put_external_user);
 }
 
+static void kvm_vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+{
+	void (*fn)(struct vfio_group *, struct kvm *);
+
+	fn = symbol_get(vfio_group_set_kvm);
+	if (!fn)
+		return;
+
+	fn(group, kvm);
+
+	symbol_put(vfio_group_set_kvm);
+}
+
+static void kvm_vfio_group_clear_kvm(struct vfio_group *group, struct kvm *kvm)
+{
+	void (*fn)(struct vfio_group *, struct kvm *);
+
+	fn = symbol_get(vfio_group_set_kvm);
+	if (!fn)
+		return;
+
+	fn(group, NULL);
+
+	symbol_put(vfio_group_set_kvm);
+}
+
 static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group)
 {
 	long (*fn)(struct vfio_group *, unsigned long);
@@ -155,6 +181,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
 		list_add_tail(&kvg->node, &kv->group_list);
 		kvg->vfio_group = vfio_group;
 
+		kvm_vfio_group_set_kvm(vfio_group, dev->kvm);
+
 		kvm_arch_start_assignment(dev->kvm);
 
 		mutex_unlock(&kv->lock);
@@ -196,6 +224,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
 
 		mutex_unlock(&kv->lock);
 
+		kvm_vfio_group_clear_kvm(vfio_group, dev->kvm);
+
 		kvm_vfio_group_put_external_user(vfio_group);
 
 		kvm_vfio_update_coherency(dev);
@@ -240,6 +270,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
 	struct kvm_vfio_group *kvg, *tmp;
 
 	list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) {
+		kvm_vfio_group_clear_kvm(kvg->vfio_group, dev->kvm);
 		kvm_vfio_group_put_external_user(kvg->vfio_group);
 		list_del(&kvg->node);
 		kfree(kvg);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier
  2016-11-17 12:57 ` [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier Jike Song
@ 2016-11-17 22:10   ` Alex Williamson
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2016-11-17 22:10 UTC (permalink / raw)
  To: Jike Song; +Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm

On Thu, 17 Nov 2016 20:57:09 +0800
Jike Song <jike.song@intel.com> wrote:

> Currently vfio_register_notifier assumes that there is only one
> notifier chain, which is in vfio_iommu. However, the user might
> also be interested in events other than vfio_iommu, for example,
> vfio_group. Refactor vfio_{un}register_notifier implementation
> to make it feasible.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
>  drivers/vfio/vfio.c  | 88 +++++++++++++++++++++++++++++++++++++++-------------
>  include/linux/vfio.h | 16 ++++++++--
>  2 files changed, 79 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index c815067..819c7f9 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2007,23 +2007,25 @@ int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage)
>  }
>  EXPORT_SYMBOL(vfio_unpin_pages);
>  
> -int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
> +static int vfio_register_iommu_notifier(struct vfio_group *group,
> +					unsigned long *events,
> +					struct notifier_block *nb)
>  {
>  	struct vfio_container *container;
> -	struct vfio_group *group;
>  	struct vfio_iommu_driver *driver;
>  	int ret;
>  
> -	if (!dev || !nb)
> -		return -EINVAL;
> +	/* clear known events */
> +	if (*events & VFIO_IOMMU_NOTIFY_DMA_UNMAP)

This test is unnecessary.

> +		*events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
>  
> -	group = vfio_group_get_from_dev(dev);
> -	if (IS_ERR(group))
> -		return PTR_ERR(group);
> +	/* refuse to register if still events remaining */
> +	if (*events)
> +		return -EINVAL;
>  
>  	ret = vfio_group_add_container_user(group);
>  	if (ret)
> -		goto err_register_nb;
> +		return -EINVAL;
>  
>  	container = group->container;
>  	down_read(&container->group_lock);
> @@ -2037,29 +2039,19 @@ int vfio_register_notifier(struct device *dev, struct notifier_block *nb)
>  	up_read(&container->group_lock);
>  	vfio_group_try_dissolve_container(group);
>  
> -err_register_nb:
> -	vfio_group_put(group);
>  	return ret;
>  }
> -EXPORT_SYMBOL(vfio_register_notifier);
>  
> -int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb)
> +static int vfio_unregister_iommu_notifier(struct vfio_group *group,
> +					  struct notifier_block *nb)
>  {
>  	struct vfio_container *container;
> -	struct vfio_group *group;
>  	struct vfio_iommu_driver *driver;
>  	int ret;
>  
> -	if (!dev || !nb)
> -		return -EINVAL;
> -
> -	group = vfio_group_get_from_dev(dev);
> -	if (IS_ERR(group))
> -		return PTR_ERR(group);
> -
>  	ret = vfio_group_add_container_user(group);
>  	if (ret)
> -		goto err_unregister_nb;
> +		return -EINVAL;
>  
>  	container = group->container;
>  	down_read(&container->group_lock);
> @@ -2074,7 +2066,59 @@ int vfio_unregister_notifier(struct device *dev, struct notifier_block *nb)
>  	up_read(&container->group_lock);
>  	vfio_group_try_dissolve_container(group);
>  
> -err_unregister_nb:
> +	return ret;
> +}
> +
> +int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
> +			   unsigned long *events,
> +			   struct notifier_block *nb)
> +{
> +	struct vfio_group *group;
> +	int ret;
> +
> +	if (!dev || !nb)
> +		return -EINVAL;
> +	if (!events || *events == 0)
> +		return -EINVAL;
> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	switch (type) {
> +	case VFIO_IOMMU_NOTIFY:
> +		ret = vfio_register_iommu_notifier(group, events, nb);
> +		break;
> +	default:
> +		ret = EINVAL;
> +	}
> +
> +	vfio_group_put(group);
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_register_notifier);
> +
> +int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type,
> +			     struct notifier_block *nb)
> +{
> +	struct vfio_group *group;
> +	int ret;
> +
> +	if (!dev || !nb)
> +		return -EINVAL;
> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	switch (type) {
> +	case VFIO_IOMMU_NOTIFY:
> +		ret = vfio_unregister_iommu_notifier(group, nb);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
>  	vfio_group_put(group);
>  	return ret;
>  }
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 15ff042..6f3ff31 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -107,14 +107,24 @@ extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
>  extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
>  			    int npage);
>  
> -#define VFIO_IOMMU_NOTIFY_DMA_UNMAP	(1)
> -
> +typedef unsigned short vfio_notify_type_t;
>  extern int vfio_register_notifier(struct device *dev,
> +				  vfio_notify_type_t type,
> +				  unsigned long *required_events,
>  				  struct notifier_block *nb);
> -
>  extern int vfio_unregister_notifier(struct device *dev,
> +				    vfio_notify_type_t type,
>  				    struct notifier_block *nb);
>  
> +/* each type has independent events */
> +enum vfio_notify_type {
> +	VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0,
> +};
> +
> +/* events for VFIO_IOMMU_NOTIFY */
> +#define VFIO_IOMMU_NOTIFY_DMA_UNMAP	BIT(0)
> +
> +
>  /*
>   * Sub-module helpers
>   */


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [v5 2/3] vfio: support notifier chain in vfio_group
  2016-11-17 12:57 ` [v5 2/3] vfio: support notifier chain in vfio_group Jike Song
@ 2016-11-17 22:10   ` Alex Williamson
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2016-11-17 22:10 UTC (permalink / raw)
  To: Jike Song; +Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm

On Thu, 17 Nov 2016 20:57:10 +0800
Jike Song <jike.song@intel.com> wrote:

> Beyond vfio_iommu events, users might also be interested in
> vfio_group events. For example, if a vfio_group is used along
> with Qemu/KVM, whenever kvm pointer is set to/cleared from the
> vfio_group, users could be notified.
> 
> Currently only VFIO_GROUP_NOTIFY_SET_KVM supported.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
>  drivers/vfio/vfio.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/vfio.h |  6 ++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 819c7f9..85a14ee 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -86,6 +86,8 @@ struct vfio_group {
>  	struct mutex			unbound_lock;
>  	atomic_t			opened;
>  	bool				noiommu;
> +	struct kvm			*kvm;
> +	struct blocking_notifier_head	notifier;
>  };
>  
>  struct vfio_device {
> @@ -312,6 +314,10 @@ static void vfio_group_unlock_and_free(struct vfio_group *group)
>  	 * that the group is no longer in vfio.group_list.
>  	 */
>  	iommu_group_unregister_notifier(group->iommu_group, &group->nb);
> +
> +	/* Any user didn't unregister? */
> +	WARN_ON(group->notifier.head);
> +

Should be in vfio_group_fops_release() per the other thread.

>  	kfree(group);
>  }
>  
> @@ -339,6 +345,7 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
>  #ifdef CONFIG_VFIO_NOIOMMU
>  	group->noiommu = (iommu_group_get_iommudata(iommu_group) == &noiommu);
>  #endif
> +	BLOCKING_INIT_NOTIFIER_HEAD(&group->notifier);
>  
>  	group->nb.notifier_call = vfio_iommu_group_notifier;
>  
> @@ -1015,6 +1022,50 @@ static long vfio_ioctl_check_extension(struct vfio_container *container,
>  	return ret;
>  }
>  
> +void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> +{
> +	group->kvm = kvm;
> +	blocking_notifier_call_chain(&group->notifier,
> +				VFIO_GROUP_NOTIFY_SET_KVM, kvm);
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_set_kvm);
> +
> +static int vfio_register_group_notifier(struct vfio_group *group,
> +					unsigned long *events,
> +					struct notifier_block *nb)
> +{
> +	int ret;
> +	bool replay = false;

'bool set_kvm' maybe

> +
> +	/* clear known events */
> +	if (*events & VFIO_GROUP_NOTIFY_SET_KVM) {
> +		replay = true;
> +		*events &= ~VFIO_GROUP_NOTIFY_SET_KVM;
> +	}
> +
> +	/* refuse to continue if still events remaining */
> +	if (*events)
> +		return -EINVAL;
> +
> +	ret = blocking_notifier_chain_register(&group->notifier, nb);
> +
> +	/*
> +	 * The attaching of kvm and vfio_group might already happen, so
> +	 * here we replay once upon registration.
> +	 */
> +	if (!ret && replay && group->kvm)
> +		blocking_notifier_call_chain(&group->notifier,
> +					VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
> +
> +	return ret;
> +}
> +
> +static int vfio_unregister_group_notifier(struct vfio_group *group,
> +					 struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&group->notifier, nb);
> +}
> +
>  /* hold write lock on container->group_lock */
>  static int __vfio_container_attach_groups(struct vfio_container *container,
>  					  struct vfio_iommu_driver *driver,
> @@ -2089,6 +2140,9 @@ int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
>  	case VFIO_IOMMU_NOTIFY:
>  		ret = vfio_register_iommu_notifier(group, events, nb);
>  		break;
> +	case VFIO_GROUP_NOTIFY:
> +		ret = vfio_register_group_notifier(group, events, nb);

When we set a notifier on an iommu_group we add a container user which
goes through validating whether the container is in the correct state.
We're doing none of that for groups.  Do we really want the vendor
driver to be able to set a notifier for an unused group?
vfio_register_group_notifier() should probably be checking
group->opened.

> +		break;
>  	default:
>  		ret = EINVAL;
>  	}
> @@ -2115,6 +2169,9 @@ int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type,
>  	case VFIO_IOMMU_NOTIFY:
>  		ret = vfio_unregister_iommu_notifier(group, nb);
>  		break;
> +	case VFIO_GROUP_NOTIFY:
> +		ret = vfio_unregister_group_notifier(group, nb);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 6f3ff31..5d46e3c 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -119,11 +119,17 @@ extern int vfio_unregister_notifier(struct device *dev,
>  /* each type has independent events */
>  enum vfio_notify_type {
>  	VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0,
> +	VFIO_GROUP_NOTIFY = (__force vfio_notify_type_t)1,
>  };
>  
>  /* events for VFIO_IOMMU_NOTIFY */
>  #define VFIO_IOMMU_NOTIFY_DMA_UNMAP	BIT(0)
>  
> +/* events for VFIO_GROUP_NOTIFY */
> +#define VFIO_GROUP_NOTIFY_SET_KVM	BIT(0)
> +
> +struct kvm;
> +extern void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
>  
>  /*
>   * Sub-module helpers


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-17 22:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-17 12:57 [v5 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
2016-11-17 12:57 ` [v5 1/3] vfio: vfio_register_notifier: classify iommu notfier Jike Song
2016-11-17 22:10   ` Alex Williamson
2016-11-17 12:57 ` [v5 2/3] vfio: support notifier chain in vfio_group Jike Song
2016-11-17 22:10   ` Alex Williamson
2016-11-17 12:57 ` [v5 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).