Re: [Qemu-devel] [PATCH v13 12/22] vfio: Add notifier callback to parent's ops structure of mdev

[Dong Jia Shi]

* Kirti Wankhede  [2016-11-16 20:47:18 +0530]:

> 
> 
> On 11/16/2016 12:07 PM, Dong Jia Shi wrote:
> > * Kirti Wankhede  [2016-11-15 20:59:55 +0530]:
> > 
> > Hi Kirti,
> > 
> > [...]
> > 
> >> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> >> index ffc36758cb84..4fc63db38829 100644
> >> --- a/drivers/vfio/mdev/vfio_mdev.c
> >> +++ b/drivers/vfio/mdev/vfio_mdev.c
> >> @@ -24,6 +24,15 @@
> >>  #define DRIVER_AUTHOR   "NVIDIA Corporation"
> >>  #define DRIVER_DESC "VFIO based driver for Mediated device"
> >>
> >> +static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long 
> >> action,
> >> +void *data)
> >> +{
> >> +  struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
> >> +  struct parent_device *parent = mdev->parent;
> >> +
> >> +  return parent->ops->notifier(mdev, action, data);
> >> +}
> >> +
> >>  static int vfio_mdev_open(void *device_data)
> >>  {
> >>struct mdev_device *mdev = device_data;
> >> @@ -36,9 +45,18 @@ static int vfio_mdev_open(void *device_data)
> >>if (!try_module_get(THIS_MODULE))
> >>return -ENODEV;
> >>
> >> +  if (likely(parent->ops->notifier)) {
> >> +  mdev->nb.notifier_call = vfio_mdev_notifier;
> >> +  if (vfio_register_notifier(>dev, >nb))
> >> +  pr_err("Failed to register notifier for mdev\n");
> > I think we should just return here if the error value is not -ENOTTY.
> > 
> 
> It might be the case where iommu backend module might not support
> .register_notifier(). In that case vfio_register_notifier() returns
> -ENOTTY and that should not fail this open() call
> Changing it to:
> 
> ret = vfio_register_notifier(>dev, >nb);
> if (ret && (ret != -ENOTTY)) {
> pr_err("Failed to register notifier for mdev\n");
> module_put(THIS_MODULE);
> return ret;
> }
Nod. And we need not call vfio_unregister_notifier once error occurs in
open() in this case.

> 
> Thanks,
> Kirti
> 

-- 
Dong Jia

Re: [Qemu-devel] [PATCH v13 12/22] vfio: Add notifier callback to parent's ops structure of mdev

[Kirti Wankhede]

On 11/16/2016 12:07 PM, Dong Jia Shi wrote:
> * Kirti Wankhede  [2016-11-15 20:59:55 +0530]:
> 
> Hi Kirti,
> 
> [...]
> 
>> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
>> index ffc36758cb84..4fc63db38829 100644
>> --- a/drivers/vfio/mdev/vfio_mdev.c
>> +++ b/drivers/vfio/mdev/vfio_mdev.c
>> @@ -24,6 +24,15 @@
>>  #define DRIVER_AUTHOR   "NVIDIA Corporation"
>>  #define DRIVER_DESC "VFIO based driver for Mediated device"
>>
>> +static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long 
>> action,
>> +  void *data)
>> +{
>> +struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
>> +struct parent_device *parent = mdev->parent;
>> +
>> +return parent->ops->notifier(mdev, action, data);
>> +}
>> +
>>  static int vfio_mdev_open(void *device_data)
>>  {
>>  struct mdev_device *mdev = device_data;
>> @@ -36,9 +45,18 @@ static int vfio_mdev_open(void *device_data)
>>  if (!try_module_get(THIS_MODULE))
>>  return -ENODEV;
>>
>> +if (likely(parent->ops->notifier)) {
>> +mdev->nb.notifier_call = vfio_mdev_notifier;
>> +if (vfio_register_notifier(>dev, >nb))
>> +pr_err("Failed to register notifier for mdev\n");
> I think we should just return here if the error value is not -ENOTTY.
> 

It might be the case where iommu backend module might not support
.register_notifier(). In that case vfio_register_notifier() returns
-ENOTTY and that should not fail this open() call
Changing it to:

ret = vfio_register_notifier(>dev, >nb);
if (ret && (ret != -ENOTTY)) {
pr_err("Failed to register notifier for mdev\n");
module_put(THIS_MODULE);
return ret;
}

Thanks,
Kirti

Re: [Qemu-devel] [PATCH v13 12/22] vfio: Add notifier callback to parent's ops structure of mdev

[Dong Jia Shi]

* Kirti Wankhede  [2016-11-15 20:59:55 +0530]:

Hi Kirti,

[...]

> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> index ffc36758cb84..4fc63db38829 100644
> --- a/drivers/vfio/mdev/vfio_mdev.c
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -24,6 +24,15 @@
>  #define DRIVER_AUTHOR   "NVIDIA Corporation"
>  #define DRIVER_DESC "VFIO based driver for Mediated device"
> 
> +static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long 
> action,
> +   void *data)
> +{
> + struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
> + struct parent_device *parent = mdev->parent;
> +
> + return parent->ops->notifier(mdev, action, data);
> +}
> +
>  static int vfio_mdev_open(void *device_data)
>  {
>   struct mdev_device *mdev = device_data;
> @@ -36,9 +45,18 @@ static int vfio_mdev_open(void *device_data)
>   if (!try_module_get(THIS_MODULE))
>   return -ENODEV;
> 
> + if (likely(parent->ops->notifier)) {
> + mdev->nb.notifier_call = vfio_mdev_notifier;
> + if (vfio_register_notifier(>dev, >nb))
> + pr_err("Failed to register notifier for mdev\n");
I think we should just return here if the error value is not -ENOTTY.

> + }
> +
>   ret = parent->ops->open(mdev);
> - if (ret)
> + if (ret) {
> + if (likely(parent->ops->notifier))
> + vfio_unregister_notifier(>dev, >nb);
>   module_put(THIS_MODULE);
> + }
> 
>   return ret;
>  }
[...]

-- 
Dong Jia

[Qemu-devel] [PATCH v13 12/22] vfio: Add notifier callback to parent's ops structure of mdev

[Kirti Wankhede]

Add a notifier calback to parent's ops structure of mdev device so that per
device notifer for vfio module is registered through vfio_mdev module.

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: Iafa6f1721aecdd6e50eb93b153b5621e6d29b637
---
 drivers/vfio/mdev/vfio_mdev.c | 25 -
 include/linux/mdev.h  |  9 +
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index ffc36758cb84..4fc63db38829 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -24,6 +24,15 @@
 #define DRIVER_AUTHOR   "NVIDIA Corporation"
 #define DRIVER_DESC "VFIO based driver for Mediated device"
 
+static int vfio_mdev_notifier(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+   struct mdev_device *mdev = container_of(nb, struct mdev_device, nb);
+   struct parent_device *parent = mdev->parent;
+
+   return parent->ops->notifier(mdev, action, data);
+}
+
 static int vfio_mdev_open(void *device_data)
 {
struct mdev_device *mdev = device_data;
@@ -36,9 +45,18 @@ static int vfio_mdev_open(void *device_data)
if (!try_module_get(THIS_MODULE))
return -ENODEV;
 
+   if (likely(parent->ops->notifier)) {
+   mdev->nb.notifier_call = vfio_mdev_notifier;
+   if (vfio_register_notifier(>dev, >nb))
+   pr_err("Failed to register notifier for mdev\n");
+   }
+
ret = parent->ops->open(mdev);
-   if (ret)
+   if (ret) {
+   if (likely(parent->ops->notifier))
+   vfio_unregister_notifier(>dev, >nb);
module_put(THIS_MODULE);
+   }
 
return ret;
 }
@@ -51,6 +69,11 @@ static void vfio_mdev_release(void *device_data)
if (likely(parent->ops->release))
parent->ops->release(mdev);
 
+   if (likely(parent->ops->notifier)) {
+   if (vfio_unregister_notifier(>dev, >nb))
+   pr_err("Failed to unregister notifier for mdev\n");
+   }
+
module_put(THIS_MODULE);
 }
 
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index ec819e9a115a..94c43034c297 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -37,6 +37,7 @@ struct mdev_device {
struct kref ref;
struct list_headnext;
struct kobject  *type_kobj;
+   struct notifier_block   nb;
 };
 
 /**
@@ -85,6 +86,12 @@ struct mdev_device {
  * @mmap:  mmap callback
  * @mdev: mediated device structure
  * @vma: vma structure
+ * @notifer:   Notifier callback, currently only for
+ * VFIO_IOMMU_NOTIFY_DMA_UNMAP action notified duing
+ * DMA_UNMAP call on mapped iova range.
+ * @mdev: mediated device structure
+ * @action: Action for which notifier is called
+ * @data: Data associated with the notifier
  * Parent device that support mediated device should be registered with mdev
  * module with parent_ops structure.
  **/
@@ -106,6 +113,8 @@ struct parent_ops {
ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
 unsigned long arg);
int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
+   int (*notifier)(struct mdev_device *mdev, unsigned long action,
+   void *data);
 };
 
 /* interface for exporting mdev supported type attributes */
-- 
2.7.0