From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941236AbcKOIL6 (ORCPT ); Tue, 15 Nov 2016 03:11:58 -0500 Received: from hqemgate15.nvidia.com ([216.228.121.64]:19193 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751668AbcKOIL4 (ORCPT ); Tue, 15 Nov 2016 03:11:56 -0500 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Tue, 15 Nov 2016 00:11:54 -0800 Subject: Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev To: Jike Song References: <1479138156-28905-1-git-send-email-kwankhede@nvidia.com> <1479138156-28905-13-git-send-email-kwankhede@nvidia.com> <582AAF16.5080906@intel.com> CC: , , , , , , , , X-Nvconfidentiality: public From: Kirti Wankhede Message-ID: <9f612340-efd5-5f9c-60a2-08c176b0e509@nvidia.com> Date: Tue, 15 Nov 2016 13:41:48 +0530 MIME-Version: 1.0 In-Reply-To: <582AAF16.5080906@intel.com> X-Originating-IP: [10.24.216.210] X-ClientProxiedBy: BGMAIL103.nvidia.com (10.25.59.12) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/15/2016 12:15 PM, Jike Song wrote: > On 11/14/2016 11:42 PM, 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 | 19 +++++++++++++++++++ >> include/linux/mdev.h | 9 +++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c >> index ffc36758cb84..1694b1635607 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; >> @@ -40,6 +49,11 @@ static int vfio_mdev_open(void *device_data) >> if (ret) >> module_put(THIS_MODULE); >> >> + if (likely(parent->ops->notifier)) { >> + mdev->nb.notifier_call = vfio_mdev_notifier; >> + if (vfio_register_notifier(&mdev->dev, &mdev->nb)) >> + pr_err("Failed to register notifier for mdev\n"); >> + } > > Hi Kirti, > > Could you please move the notifier registration before parent->ops->open()? > as you might know, I'm extending your vfio_register_notifier to also include > the attaching/detaching events of vfio_group and kvm. Basically if vfio_group > not attached to any kvm instance, the parent->ops->open() should return -ENODEV > to indicate the failure, but to know whether kvm is available in open(), the > notifier registration should be earlier. > Ok. That seem fine to me. Thanks, Kirti > Of course I can call vfio_register_notifier() from an earlier place to > workaround it, but it doesn't seem a canonical way. > > -- > Thanks, > Jike > >> return ret; >> } >> >> @@ -48,6 +62,11 @@ static void vfio_mdev_release(void *device_data) >> struct mdev_device *mdev = device_data; >> struct parent_device *parent = mdev->parent; >> >> + if (likely(parent->ops->notifier)) { >> + if (vfio_unregister_notifier(&mdev->dev, &mdev->nb)) >> + pr_err("Failed to unregister notifier for mdev\n"); >> + } >> + >> if (likely(parent->ops->release)) >> parent->ops->release(mdev); >> >> diff --git a/include/linux/mdev.h b/include/linux/mdev.h >> index 4900cc472364..665afe0a4c31 100644 >> --- a/include/linux/mdev.h >> +++ b/include/linux/mdev.h >> @@ -37,6 +37,7 @@ struct mdev_device { >> struct kref ref; >> struct list_head next; >> 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 */ >> From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6YqZ-0002r1-Fs for qemu-devel@nongnu.org; Tue, 15 Nov 2016 03:12:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6YqU-0004MY-Da for qemu-devel@nongnu.org; Tue, 15 Nov 2016 03:12:03 -0500 Received: from hqemgate15.nvidia.com ([216.228.121.64]:19198) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6YqU-0004MU-4r for qemu-devel@nongnu.org; Tue, 15 Nov 2016 03:11:58 -0500 References: <1479138156-28905-1-git-send-email-kwankhede@nvidia.com> <1479138156-28905-13-git-send-email-kwankhede@nvidia.com> <582AAF16.5080906@intel.com> From: Kirti Wankhede Message-ID: <9f612340-efd5-5f9c-60a2-08c176b0e509@nvidia.com> Date: Tue, 15 Nov 2016 13:41:48 +0530 MIME-Version: 1.0 In-Reply-To: <582AAF16.5080906@intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jike Song Cc: alex.williamson@redhat.com, pbonzini@redhat.com, kraxel@redhat.com, cjia@nvidia.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, kevin.tian@intel.com, bjsdjshi@linux.vnet.ibm.com, linux-kernel@vger.kernel.org On 11/15/2016 12:15 PM, Jike Song wrote: > On 11/14/2016 11:42 PM, 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 | 19 +++++++++++++++++++ >> include/linux/mdev.h | 9 +++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c >> index ffc36758cb84..1694b1635607 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; >> @@ -40,6 +49,11 @@ static int vfio_mdev_open(void *device_data) >> if (ret) >> module_put(THIS_MODULE); >> >> + if (likely(parent->ops->notifier)) { >> + mdev->nb.notifier_call = vfio_mdev_notifier; >> + if (vfio_register_notifier(&mdev->dev, &mdev->nb)) >> + pr_err("Failed to register notifier for mdev\n"); >> + } > > Hi Kirti, > > Could you please move the notifier registration before parent->ops->open()? > as you might know, I'm extending your vfio_register_notifier to also include > the attaching/detaching events of vfio_group and kvm. Basically if vfio_group > not attached to any kvm instance, the parent->ops->open() should return -ENODEV > to indicate the failure, but to know whether kvm is available in open(), the > notifier registration should be earlier. > Ok. That seem fine to me. Thanks, Kirti > Of course I can call vfio_register_notifier() from an earlier place to > workaround it, but it doesn't seem a canonical way. > > -- > Thanks, > Jike > >> return ret; >> } >> >> @@ -48,6 +62,11 @@ static void vfio_mdev_release(void *device_data) >> struct mdev_device *mdev = device_data; >> struct parent_device *parent = mdev->parent; >> >> + if (likely(parent->ops->notifier)) { >> + if (vfio_unregister_notifier(&mdev->dev, &mdev->nb)) >> + pr_err("Failed to unregister notifier for mdev\n"); >> + } >> + >> if (likely(parent->ops->release)) >> parent->ops->release(mdev); >> >> diff --git a/include/linux/mdev.h b/include/linux/mdev.h >> index 4900cc472364..665afe0a4c31 100644 >> --- a/include/linux/mdev.h >> +++ b/include/linux/mdev.h >> @@ -37,6 +37,7 @@ struct mdev_device { >> struct kref ref; >> struct list_head next; >> 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 */ >>