Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
On Thu, 17 Nov 2016 21:30:26 +0530 Kirti Wankhedewrote: > On 11/17/2016 8:57 PM, Alex Williamson wrote: > > On Thu, 17 Nov 2016 20:35:38 +0800 > > Jike Song wrote: > > > >> On 11/17/2016 04:46 AM, Kirti Wankhede wrote: > >>> 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 | 34 +- > >>> include/linux/mdev.h | 9 + > >>> 2 files changed, 42 insertions(+), 1 deletion(-) > >>> > >> > >> Hi Alex, Kirti, > >> > >> Since everyone agreed we should let the vendor driver call > >> vfio_register_notifier > >> directly, can you drop this patch from merging? So that I don't need to > >> send a > >> reverse patch. > > > > This seems like a reasonable request to me, this patch drops cleanly > > from the series. Any objection Kirti? It seems like it removes a > > little bit of pre-release churn from the API. Thanks, > > > > This is independent patch, I don't have any concern to drop this patch. > > Small nit: Since Documentation patch is later in series, its mentioned > in the doc. One line need to be removed from vfio-mediated-device.txt. > > --- a/Documentation/vfio-mediated-device.txt > +++ b/Documentation/vfio-mediated-device.txt > @@ -150,7 +150,6 @@ The callbacks in the parent_ops structure are as > follows: > * read : read emulation callback > * write: write emulation callback > * mmap: mmap emulation callback > -* notifier: notifier callback Good catch, I'll remove this line. Thanks, Alex
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
On 11/17/2016 8:57 PM, Alex Williamson wrote: > On Thu, 17 Nov 2016 20:35:38 +0800 > Jike Songwrote: > >> On 11/17/2016 04:46 AM, Kirti Wankhede wrote: >>> 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 | 34 +- >>> include/linux/mdev.h | 9 + >>> 2 files changed, 42 insertions(+), 1 deletion(-) >>> >> >> Hi Alex, Kirti, >> >> Since everyone agreed we should let the vendor driver call >> vfio_register_notifier >> directly, can you drop this patch from merging? So that I don't need to send >> a >> reverse patch. > > This seems like a reasonable request to me, this patch drops cleanly > from the series. Any objection Kirti? It seems like it removes a > little bit of pre-release churn from the API. Thanks, > This is independent patch, I don't have any concern to drop this patch. Small nit: Since Documentation patch is later in series, its mentioned in the doc. One line need to be removed from vfio-mediated-device.txt. --- a/Documentation/vfio-mediated-device.txt +++ b/Documentation/vfio-mediated-device.txt @@ -150,7 +150,6 @@ The callbacks in the parent_ops structure are as follows: * read : read emulation callback * write: write emulation callback * mmap: mmap emulation callback -* notifier: notifier callback Thanks, Kirti
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
On Thu, 17 Nov 2016 20:35:38 +0800 Jike Songwrote: > On 11/17/2016 04:46 AM, Kirti Wankhede wrote: > > 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 | 34 +- > > include/linux/mdev.h | 9 + > > 2 files changed, 42 insertions(+), 1 deletion(-) > > > > Hi Alex, Kirti, > > Since everyone agreed we should let the vendor driver call > vfio_register_notifier > directly, can you drop this patch from merging? So that I don't need to send a > reverse patch. This seems like a reasonable request to me, this patch drops cleanly from the series. Any objection Kirti? It seems like it removes a little bit of pre-release churn from the API. Thanks, Alex
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
On 11/17/2016 04:46 AM, Kirti Wankhede wrote: > 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 | 34 +- > include/linux/mdev.h | 9 + > 2 files changed, 42 insertions(+), 1 deletion(-) > > diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c > index ffc36758cb84..2f8e06e5f95a 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,27 @@ 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; > + ret = vfio_register_notifier(>dev, >nb); > + > + /* > + * This should not fail if backend iommu module doesn't support > + * register_notifier. > + */ > + if (ret && (ret != -ENOTTY)) { > + pr_err("Failed to register notifier for mdev\n"); > + module_put(THIS_MODULE); > + return ret; > + } > + } > + > 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 +78,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 */ > Hi Alex, Kirti, Since everyone agreed we should let the vendor driver call vfio_register_notifier directly, can you drop this patch from merging? So that I don't need to send a reverse patch. Thanks :) -- Thanks, Jike
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
On 11/17/2016 7:45 AM, Dong Jia Shi wrote: > * Kirti Wankhede[2016-11-17 02:16:24 +0530]: > > Hi Kirti, > >> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c > [...] > >> @@ -51,6 +78,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"); > For the -ENOTTY case, we should not fail here either. > Removing the error print and ignoring return from this unregister call. Updating this patch on this thread. >> +} >> + >> module_put(THIS_MODULE); >> } >> > [...] >
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
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 WankhedeSigned-off-by: Neo Jia Change-Id: Iafa6f1721aecdd6e50eb93b153b5621e6d29b637 --- drivers/vfio/mdev/vfio_mdev.c | 32 +++- include/linux/mdev.h | 9 + 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index ffc36758cb84..7ffdc317319d 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,27 @@ 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; + ret = vfio_register_notifier(>dev, >nb); + + /* +* This should not fail if backend iommu module doesn't support +* register_notifier. +*/ + if (ret && (ret != -ENOTTY)) { + pr_err("Failed to register notifier for mdev\n"); + module_put(THIS_MODULE); + return ret; + } + } + 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 +78,9 @@ static void vfio_mdev_release(void *device_data) if (likely(parent->ops->release)) parent->ops->release(mdev); + if (likely(parent->ops->notifier)) + vfio_unregister_notifier(>dev, >nb); + 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
Re: [Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
* Kirti Wankhede[2016-11-17 02:16:24 +0530]: Hi Kirti, > diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c [...] > @@ -51,6 +78,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"); For the -ENOTTY case, we should not fail here either. > + } > + > module_put(THIS_MODULE); > } > [...] -- Dong Jia
[Qemu-devel] [PATCH v14 12/22] vfio: Add notifier callback to parent's ops structure of mdev
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 WankhedeSigned-off-by: Neo Jia Change-Id: Iafa6f1721aecdd6e50eb93b153b5621e6d29b637 --- drivers/vfio/mdev/vfio_mdev.c | 34 +- include/linux/mdev.h | 9 + 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index ffc36758cb84..2f8e06e5f95a 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,27 @@ 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; + ret = vfio_register_notifier(>dev, >nb); + + /* +* This should not fail if backend iommu module doesn't support +* register_notifier. +*/ + if (ret && (ret != -ENOTTY)) { + pr_err("Failed to register notifier for mdev\n"); + module_put(THIS_MODULE); + return ret; + } + } + 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 +78,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