* [v8 0/3] plumb kvm/vfio to notify kvm:group attach/detach
@ 2016-12-01 5:20 Jike Song
2016-12-01 5:20 ` [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier Jike Song
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jike Song @ 2016-12-01 5:20 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.
Changes v8:
- pass events to separate iommu backends (currently only type1)
- remove _clear_kvm() and use _set_kvm(NULL) instead, move it out of lock
Changes v7:
- replace vfio->opened with container user like iommu notifier registration
- fix a typo
Changes v6:
- only allow registration on already opened vfio_group
- moved WARN_ON from vfio_group_release to vfio_group_fops_release
- remove unnecessary check
Changes v5:
- separate vfio_group and vfio_iommu events from each other
- force user to specify which type it is interested in, and which events.
Register it IFF all required 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 notifier
vfio: support notifier chain in vfio_group
kvm: set/clear kvm to/from vfio_group when group add/delete
drivers/vfio/vfio.c | 167 ++++++++++++++++++++++++++++++++++------
drivers/vfio/vfio_iommu_type1.c | 8 ++
include/linux/vfio.h | 23 +++++-
virt/kvm/vfio.c | 18 +++++
4 files changed, 188 insertions(+), 28 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier
2016-12-01 5:20 [v8 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
@ 2016-12-01 5:20 ` Jike Song
2016-12-01 16:21 ` Alex Williamson
2016-12-01 5:20 ` [v8 2/3] vfio: support notifier chain in vfio_group Jike Song
2016-12-01 5:20 ` [v8 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song
2 siblings, 1 reply; 8+ messages in thread
From: Jike Song @ 2016-12-01 5:20 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: Paolo Bonzini <pbonzini@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
drivers/vfio/vfio.c | 85 +++++++++++++++++++++++++++++------------
drivers/vfio/vfio_iommu_type1.c | 8 ++++
include/linux/vfio.h | 17 +++++++--
3 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 0aac3ca..cb2ce2d 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2008,59 +2008,44 @@ 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;
-
- 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_register_nb;
+ return -EINVAL;
container = group->container;
down_read(&container->group_lock);
driver = container->iommu_driver;
if (likely(driver && driver->ops->register_notifier))
- ret = driver->ops->register_notifier(container->iommu_data, nb);
+ ret = driver->ops->register_notifier(container->iommu_data,
+ events, nb);
else
ret = -ENOTTY;
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);
@@ -2075,7 +2060,57 @@ 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 || !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/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 51810a9..b88ad1e 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1584,10 +1584,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
}
static int vfio_iommu_type1_register_notifier(void *iommu_data,
+ unsigned long *events,
struct notifier_block *nb)
{
struct vfio_iommu *iommu = iommu_data;
+ /* clear known events */
+ *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
+
+ /* refuse to register if still events remaining */
+ if (*events)
+ return -EINVAL;
+
return blocking_notifier_chain_register(&iommu->notifier, nb);
}
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 15ff042..fdb8460 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -81,6 +81,7 @@ struct vfio_iommu_driver_ops {
int (*unpin_pages)(void *iommu_data,
unsigned long *user_pfn, int npage);
int (*register_notifier)(void *iommu_data,
+ unsigned long *events,
struct notifier_block *nb);
int (*unregister_notifier)(void *iommu_data,
struct notifier_block *nb);
@@ -107,14 +108,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] 8+ messages in thread
* [v8 2/3] vfio: support notifier chain in vfio_group
2016-12-01 5:20 [v8 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
2016-12-01 5:20 ` [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier Jike Song
@ 2016-12-01 5:20 ` Jike Song
2016-12-01 5:20 ` [v8 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song
2 siblings, 0 replies; 8+ messages in thread
From: Jike Song @ 2016-12-01 5:20 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 | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/vfio.h | 6 ++++
2 files changed, 88 insertions(+)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index cb2ce2d..6cf21a80 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 {
@@ -339,6 +341,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;
@@ -1581,6 +1584,9 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
filep->private_data = NULL;
+ /* Any user didn't unregister? */
+ WARN_ON(group->notifier.head);
+
vfio_group_try_dissolve_container(group);
atomic_dec(&group->opened);
@@ -2063,6 +2069,76 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
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)
+{
+ struct vfio_container *container;
+ int ret;
+ bool set_kvm = false;
+
+ if (*events & VFIO_GROUP_NOTIFY_SET_KVM)
+ set_kvm = true;
+
+ /* clear known events */
+ *events &= ~VFIO_GROUP_NOTIFY_SET_KVM;
+
+ /* refuse to continue if still events remaining */
+ if (*events)
+ return -EINVAL;
+
+ ret = vfio_group_add_container_user(group);
+ if (ret)
+ return -EINVAL;
+
+ container = group->container;
+ down_read(&container->group_lock);
+
+ 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 && set_kvm && group->kvm)
+ blocking_notifier_call_chain(&group->notifier,
+ VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
+
+ up_read(&container->group_lock);
+ vfio_group_try_dissolve_container(group);
+
+ return ret;
+}
+
+static int vfio_unregister_group_notifier(struct vfio_group *group,
+ struct notifier_block *nb)
+{
+ struct vfio_container *container;
+ int ret;
+
+ ret = vfio_group_add_container_user(group);
+ if (ret)
+ return -EINVAL;
+
+ container = group->container;
+ down_read(&container->group_lock);
+
+ ret = blocking_notifier_chain_unregister(&group->notifier, nb);
+
+ up_read(&container->group_lock);
+ vfio_group_try_dissolve_container(group);
+
+ return ret;
+}
+
int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
unsigned long *events,
struct notifier_block *nb)
@@ -2081,6 +2157,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;
}
@@ -2107,6 +2186,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 fdb8460..3b843f3 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -120,11 +120,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] 8+ messages in thread
* [v8 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete
2016-12-01 5:20 [v8 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
2016-12-01 5:20 ` [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier Jike Song
2016-12-01 5:20 ` [v8 2/3] vfio: support notifier chain in vfio_group Jike Song
@ 2016-12-01 5:20 ` Jike Song
2 siblings, 0 replies; 8+ messages in thread
From: Jike Song @ 2016-12-01 5:20 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: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
virt/kvm/vfio.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 1dd087d..d32f239 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -60,6 +60,19 @@ 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 bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group)
{
long (*fn)(struct vfio_group *, unsigned long);
@@ -159,6 +172,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
mutex_unlock(&kv->lock);
+ kvm_vfio_group_set_kvm(vfio_group, dev->kvm);
+
kvm_vfio_update_coherency(dev);
return 0;
@@ -196,6 +211,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
mutex_unlock(&kv->lock);
+ kvm_vfio_group_set_kvm(vfio_group, NULL);
+
kvm_vfio_group_put_external_user(vfio_group);
kvm_vfio_update_coherency(dev);
@@ -240,6 +257,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_set_kvm(kvg->vfio_group, NULL);
kvm_vfio_group_put_external_user(kvg->vfio_group);
list_del(&kvg->node);
kfree(kvg);
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier
2016-12-01 5:20 ` [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier Jike Song
@ 2016-12-01 16:21 ` Alex Williamson
2016-12-01 16:22 ` Alex Williamson
0 siblings, 1 reply; 8+ messages in thread
From: Alex Williamson @ 2016-12-01 16:21 UTC (permalink / raw)
To: Jike Song; +Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm
On Thu, 1 Dec 2016 13:20:05 +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: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
> drivers/vfio/vfio.c | 85 +++++++++++++++++++++++++++++------------
> drivers/vfio/vfio_iommu_type1.c | 8 ++++
> include/linux/vfio.h | 17 +++++++--
> 3 files changed, 82 insertions(+), 28 deletions(-)
I'm making the following fixup to merge this with Christophe's patch
found here https://lkml.org/lkml/2016/11/30/57
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 0aac3ca..cb2ce2d 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2008,59 +2008,44 @@ 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;
> -
> - 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_register_nb;
> + return -EINVAL;
>
> container = group->container;
> down_read(&container->group_lock);
>
> driver = container->iommu_driver;
> if (likely(driver && driver->ops->register_notifier))
> - ret = driver->ops->register_notifier(container->iommu_data, nb);
> + ret = driver->ops->register_notifier(container->iommu_data,
> + events, nb);
> else
> ret = -ENOTTY;
>
> 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);
> @@ -2075,7 +2060,57 @@ 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 || !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/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 51810a9..b88ad1e 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1584,10 +1584,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> }
>
> static int vfio_iommu_type1_register_notifier(void *iommu_data,
> + unsigned long *events,
> struct notifier_block *nb)
> {
> struct vfio_iommu *iommu = iommu_data;
>
> + /* clear known events */
> + *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
> +
> + /* refuse to register if still events remaining */
> + if (*events)
> + return -EINVAL;
> +
> return blocking_notifier_chain_register(&iommu->notifier, nb);
> }
>
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 15ff042..fdb8460 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -81,6 +81,7 @@ struct vfio_iommu_driver_ops {
> int (*unpin_pages)(void *iommu_data,
> unsigned long *user_pfn, int npage);
> int (*register_notifier)(void *iommu_data,
> + unsigned long *events,
> struct notifier_block *nb);
> int (*unregister_notifier)(void *iommu_data,
> struct notifier_block *nb);
> @@ -107,14 +108,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] 8+ messages in thread
* Re: [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier
2016-12-01 16:21 ` Alex Williamson
@ 2016-12-01 16:22 ` Alex Williamson
2016-12-01 17:45 ` Alex Williamson
0 siblings, 1 reply; 8+ messages in thread
From: Alex Williamson @ 2016-12-01 16:22 UTC (permalink / raw)
To: Jike Song; +Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm
On Thu, 1 Dec 2016 09:21:22 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Thu, 1 Dec 2016 13:20:05 +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: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> > Signed-off-by: Jike Song <jike.song@intel.com>
> > ---
> > drivers/vfio/vfio.c | 85 +++++++++++++++++++++++++++++------------
> > drivers/vfio/vfio_iommu_type1.c | 8 ++++
> > include/linux/vfio.h | 17 +++++++--
> > 3 files changed, 82 insertions(+), 28 deletions(-)
>
> I'm making the following fixup to merge this with Christophe's patch
> found here https://lkml.org/lkml/2016/11/30/57
Whoops, somehow that got sent before I added the changes:
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2008,59 +2008,44 @@ int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage)
@@ -79,8 +39,8 @@
- return -EINVAL;
-
- group = vfio_group_get_from_dev(dev);
-- if (IS_ERR(group))
-- return PTR_ERR(group);
+- if (!group)
+- return -ENODEV;
-
ret = vfio_group_add_container_user(group);
if (ret)
@@ -120,8 +80,8 @@
- return -EINVAL;
-
- group = vfio_group_get_from_dev(dev);
-- if (IS_ERR(group))
-- return PTR_ERR(group);
+- if (!group)
+- return -ENODEV;
-
ret = vfio_group_add_container_user(group);
if (ret)
@@ -149,8 +109,8 @@
+ return -EINVAL;
+
+ group = vfio_group_get_from_dev(dev);
-+ if (IS_ERR(group))
-+ return PTR_ERR(group);
++ if (!group)
++ return -ENODEV;
+
+ switch (type) {
+ case VFIO_IOMMU_NOTIFY:
@@ -175,8 +135,8 @@
+ return -EINVAL;
+
+ group = vfio_group_get_from_dev(dev);
-+ if (IS_ERR(group))
-+ return PTR_ERR(group);
++ if (!group)
++ return -ENODEV;
+
+ switch (type) {
+ case VFIO_IOMMU_NOTIFY:
@@ -252,7 +212,3 @@
/*
* Sub-module helpers
*/
>
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index 0aac3ca..cb2ce2d 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -2008,59 +2008,44 @@ 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;
> > -
> > - 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_register_nb;
> > + return -EINVAL;
> >
> > container = group->container;
> > down_read(&container->group_lock);
> >
> > driver = container->iommu_driver;
> > if (likely(driver && driver->ops->register_notifier))
> > - ret = driver->ops->register_notifier(container->iommu_data, nb);
> > + ret = driver->ops->register_notifier(container->iommu_data,
> > + events, nb);
> > else
> > ret = -ENOTTY;
> >
> > 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);
> > @@ -2075,7 +2060,57 @@ 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 || !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/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 51810a9..b88ad1e 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -1584,10 +1584,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> > }
> >
> > static int vfio_iommu_type1_register_notifier(void *iommu_data,
> > + unsigned long *events,
> > struct notifier_block *nb)
> > {
> > struct vfio_iommu *iommu = iommu_data;
> >
> > + /* clear known events */
> > + *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
> > +
> > + /* refuse to register if still events remaining */
> > + if (*events)
> > + return -EINVAL;
> > +
> > return blocking_notifier_chain_register(&iommu->notifier, nb);
> > }
> >
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index 15ff042..fdb8460 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -81,6 +81,7 @@ struct vfio_iommu_driver_ops {
> > int (*unpin_pages)(void *iommu_data,
> > unsigned long *user_pfn, int npage);
> > int (*register_notifier)(void *iommu_data,
> > + unsigned long *events,
> > struct notifier_block *nb);
> > int (*unregister_notifier)(void *iommu_data,
> > struct notifier_block *nb);
> > @@ -107,14 +108,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] 8+ messages in thread
* Re: [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier
2016-12-01 16:22 ` Alex Williamson
@ 2016-12-01 17:45 ` Alex Williamson
2016-12-02 2:07 ` Jike Song
0 siblings, 1 reply; 8+ messages in thread
From: Alex Williamson @ 2016-12-01 17:45 UTC (permalink / raw)
To: Jike Song; +Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm
On Thu, 1 Dec 2016 09:22:33 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Thu, 1 Dec 2016 09:21:22 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
> > On Thu, 1 Dec 2016 13:20:05 +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: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > > Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> > > Signed-off-by: Jike Song <jike.song@intel.com>
> > > ---
> > > drivers/vfio/vfio.c | 85 +++++++++++++++++++++++++++++------------
> > > drivers/vfio/vfio_iommu_type1.c | 8 ++++
> > > include/linux/vfio.h | 17 +++++++--
> > > 3 files changed, 82 insertions(+), 28 deletions(-)
> >
> > I'm making the following fixup to merge this with Christophe's patch
> > found here https://lkml.org/lkml/2016/11/30/57
>
> Whoops, somehow that got sent before I added the changes:
checkpatch.pl also complains about adding new typedefs, so perhaps
vfio_notify_type_t is not way to go. This is the patch as I have it
now, I've also updated 2/3 to remove the typedef there. If there are
no objections, this is what I'll go with. Thanks,
Alex
commit 22195cbd3451a75abaf30651a61cf85c89061327
Author: Jike Song <jike.song@intel.com>
Date: Thu Dec 1 13:20:05 2016 +0800
vfio: vfio_register_notifier: classify iommu notifier
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Jike Song <jike.song@intel.com>
[aw: merge with commit 816ca69ea9c7 ("vfio: Fix handling of error returned by 'vfio_group_get_from_dev()'"), remove typedef]
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 7b39313..6cc2a9f 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2008,59 +2008,44 @@ 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;
-
- group = vfio_group_get_from_dev(dev);
- if (!group)
- return -ENODEV;
-
ret = vfio_group_add_container_user(group);
if (ret)
- goto err_register_nb;
+ return -EINVAL;
container = group->container;
down_read(&container->group_lock);
driver = container->iommu_driver;
if (likely(driver && driver->ops->register_notifier))
- ret = driver->ops->register_notifier(container->iommu_data, nb);
+ ret = driver->ops->register_notifier(container->iommu_data,
+ events, nb);
else
ret = -ENOTTY;
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 (!group)
- return -ENODEV;
-
ret = vfio_group_add_container_user(group);
if (ret)
- goto err_unregister_nb;
+ return -EINVAL;
container = group->container;
down_read(&container->group_lock);
@@ -2075,7 +2060,56 @@ 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, enum vfio_notify_type type,
+ unsigned long *events, struct notifier_block *nb)
+{
+ struct vfio_group *group;
+ int ret;
+
+ if (!dev || !nb || !events || (*events == 0))
+ return -EINVAL;
+
+ group = vfio_group_get_from_dev(dev);
+ if (!group)
+ return -ENODEV;
+
+ 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, enum vfio_notify_type type,
+ struct notifier_block *nb)
+{
+ struct vfio_group *group;
+ int ret;
+
+ if (!dev || !nb)
+ return -EINVAL;
+
+ group = vfio_group_get_from_dev(dev);
+ if (!group)
+ return -ENODEV;
+
+ 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/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 51810a9..b88ad1e 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1584,10 +1584,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
}
static int vfio_iommu_type1_register_notifier(void *iommu_data,
+ unsigned long *events,
struct notifier_block *nb)
{
struct vfio_iommu *iommu = iommu_data;
+ /* clear known events */
+ *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
+
+ /* refuse to register if still events remaining */
+ if (*events)
+ return -EINVAL;
+
return blocking_notifier_chain_register(&iommu->notifier, nb);
}
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 15ff042..0e5201f 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -81,6 +81,7 @@ struct vfio_iommu_driver_ops {
int (*unpin_pages)(void *iommu_data,
unsigned long *user_pfn, int npage);
int (*register_notifier)(void *iommu_data,
+ unsigned long *events,
struct notifier_block *nb);
int (*unregister_notifier)(void *iommu_data,
struct notifier_block *nb);
@@ -107,12 +108,20 @@ 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)
+/* each type has independent events */
+enum vfio_notify_type {
+ VFIO_IOMMU_NOTIFY = 0,
+};
+
+/* events for VFIO_IOMMU_NOTIFY */
+#define VFIO_IOMMU_NOTIFY_DMA_UNMAP BIT(0)
extern int vfio_register_notifier(struct device *dev,
+ enum vfio_notify_type type,
+ unsigned long *required_events,
struct notifier_block *nb);
-
extern int vfio_unregister_notifier(struct device *dev,
+ enum vfio_notify_type type,
struct notifier_block *nb);
/*
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier
2016-12-01 17:45 ` Alex Williamson
@ 2016-12-02 2:07 ` Jike Song
0 siblings, 0 replies; 8+ messages in thread
From: Jike Song @ 2016-12-02 2:07 UTC (permalink / raw)
To: Alex Williamson
Cc: pbonzini, guangrong.xiao, kwankhede, cjia, kevin.tian, kvm
On 12/02/2016 01:45 AM, Alex Williamson wrote:
> On Thu, 1 Dec 2016 09:22:33 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
>> On Thu, 1 Dec 2016 09:21:22 -0700
>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>
>>> On Thu, 1 Dec 2016 13:20:05 +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: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>>> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
>>>> Signed-off-by: Jike Song <jike.song@intel.com>
>>>> ---
>>>> drivers/vfio/vfio.c | 85 +++++++++++++++++++++++++++++------------
>>>> drivers/vfio/vfio_iommu_type1.c | 8 ++++
>>>> include/linux/vfio.h | 17 +++++++--
>>>> 3 files changed, 82 insertions(+), 28 deletions(-)
>>>
>>> I'm making the following fixup to merge this with Christophe's patch
>>> found here https://lkml.org/lkml/2016/11/30/57
>>
>> Whoops, somehow that got sent before I added the changes:
>
> checkpatch.pl also complains about adding new typedefs, so perhaps
> vfio_notify_type_t is not way to go. This is the patch as I have it
> now, I've also updated 2/3 to remove the typedef there. If there are
> no objections, this is what I'll go with. Thanks,
Thanks for the fixup! I actually saw the checkpatch.pl warning about
new typedef but scrupled to remove it :-)
--
Thanks,
Jike
>
> commit 22195cbd3451a75abaf30651a61cf85c89061327
> Author: Jike Song <jike.song@intel.com>
> Date: Thu Dec 1 13:20:05 2016 +0800
>
> vfio: vfio_register_notifier: classify iommu notifier
>
> 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: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> [aw: merge with commit 816ca69ea9c7 ("vfio: Fix handling of error returned by 'vfio_group_get_from_dev()'"), remove typedef]
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 7b39313..6cc2a9f 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2008,59 +2008,44 @@ 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;
> -
> - group = vfio_group_get_from_dev(dev);
> - if (!group)
> - return -ENODEV;
> -
> ret = vfio_group_add_container_user(group);
> if (ret)
> - goto err_register_nb;
> + return -EINVAL;
>
> container = group->container;
> down_read(&container->group_lock);
>
> driver = container->iommu_driver;
> if (likely(driver && driver->ops->register_notifier))
> - ret = driver->ops->register_notifier(container->iommu_data, nb);
> + ret = driver->ops->register_notifier(container->iommu_data,
> + events, nb);
> else
> ret = -ENOTTY;
>
> 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 (!group)
> - return -ENODEV;
> -
> ret = vfio_group_add_container_user(group);
> if (ret)
> - goto err_unregister_nb;
> + return -EINVAL;
>
> container = group->container;
> down_read(&container->group_lock);
> @@ -2075,7 +2060,56 @@ 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, enum vfio_notify_type type,
> + unsigned long *events, struct notifier_block *nb)
> +{
> + struct vfio_group *group;
> + int ret;
> +
> + if (!dev || !nb || !events || (*events == 0))
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + 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, enum vfio_notify_type type,
> + struct notifier_block *nb)
> +{
> + struct vfio_group *group;
> + int ret;
> +
> + if (!dev || !nb)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + 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/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 51810a9..b88ad1e 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1584,10 +1584,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> }
>
> static int vfio_iommu_type1_register_notifier(void *iommu_data,
> + unsigned long *events,
> struct notifier_block *nb)
> {
> struct vfio_iommu *iommu = iommu_data;
>
> + /* clear known events */
> + *events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
> +
> + /* refuse to register if still events remaining */
> + if (*events)
> + return -EINVAL;
> +
> return blocking_notifier_chain_register(&iommu->notifier, nb);
> }
>
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 15ff042..0e5201f 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -81,6 +81,7 @@ struct vfio_iommu_driver_ops {
> int (*unpin_pages)(void *iommu_data,
> unsigned long *user_pfn, int npage);
> int (*register_notifier)(void *iommu_data,
> + unsigned long *events,
> struct notifier_block *nb);
> int (*unregister_notifier)(void *iommu_data,
> struct notifier_block *nb);
> @@ -107,12 +108,20 @@ 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)
> +/* each type has independent events */
> +enum vfio_notify_type {
> + VFIO_IOMMU_NOTIFY = 0,
> +};
> +
> +/* events for VFIO_IOMMU_NOTIFY */
> +#define VFIO_IOMMU_NOTIFY_DMA_UNMAP BIT(0)
>
> extern int vfio_register_notifier(struct device *dev,
> + enum vfio_notify_type type,
> + unsigned long *required_events,
> struct notifier_block *nb);
> -
> extern int vfio_unregister_notifier(struct device *dev,
> + enum vfio_notify_type type,
> struct notifier_block *nb);
>
> /*
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-02 2:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-01 5:20 [v8 0/3] plumb kvm/vfio to notify kvm:group attach/detach Jike Song
2016-12-01 5:20 ` [v8 1/3] vfio: vfio_register_notifier: classify iommu notifier Jike Song
2016-12-01 16:21 ` Alex Williamson
2016-12-01 16:22 ` Alex Williamson
2016-12-01 17:45 ` Alex Williamson
2016-12-02 2:07 ` Jike Song
2016-12-01 5:20 ` [v8 2/3] vfio: support notifier chain in vfio_group Jike Song
2016-12-01 5:20 ` [v8 3/3] kvm: set/clear kvm to/from vfio_group when group add/delete Jike Song
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.