Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1521773 > unrolled thread

[PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-14 16:50 +0100
    Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops  structure of mdev Jike Song <jike.song@intel.com> - 2016-11-15 07:50 +0100
      Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops  structure of mdev Kirti Wankhede <kwankhede@nvidia.com> - 2016-11-15 09:20 +0100
        Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops  structure of mdev Jike Song <jike.song@intel.com> - 2016-11-15 10:40 +0100
      Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops  structure of mdev Alex Williamson <alex.williamson@redhat.com> - 2016-11-15 16:20 +0100
        Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops  structure of mdev Jike Song <jike.song@intel.com> - 2016-11-16 07:00 +0100

#1521773 — [PATCH v12 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 Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
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");
+	}
 	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 */
-- 
2.7.0

[toc] | [next] | [standalone]

#1522388 — Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

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 <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> 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.

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 */
> 

[toc] | [prev] | [next] | [standalone]

#1522429 — Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

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 <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> 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 */
>>

[toc] | [prev] | [next] | [standalone]

#1522487 — Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

On 11/15/2016 04:11 PM, Kirti Wankhede wrote:
> 
> 
> 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 <kwankhede@nvidia.com>
>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>> 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 - and I guess it's also good to move unregister after ->release(),
so that a sequence of register-open-release-unregister guaranteed :)

--
Thanks,
Jike

[toc] | [prev] | [next] | [standalone]

#1522808 — Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

On Tue, 15 Nov 2016 14:45:42 +0800
Jike Song <jike.song@intel.com> 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 <kwankhede@nvidia.com>
> > Signed-off-by: Neo Jia <cjia@nvidia.com>
> > 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.

It seems like you're giving general guidance for how a vendor driver
open() function should work, yet a hard dependency on KVM should be
discouraged.  You're making a choice for your vendor driver alone.

I would also be very cautious about the coherency of signaling the KVM
association relative to the user of the group.  Is it possible that the
association of one KVM instance by a user of the group can leak to the
next user?  Does vfio need to seen a gratuitous un-set of the KVM
association on group close()? etc.  Thanks,

Alex
 
> 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 */
> >   

[toc] | [prev] | [next] | [standalone]

#1523237 — Re: [PATCH v12 12/22] vfio: Add notifier callback to parent's ops structure of mdev

On 11/15/2016 11:19 PM, Alex Williamson wrote:
> On Tue, 15 Nov 2016 14:45:42 +0800
> Jike Song <jike.song@intel.com> 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 <kwankhede@nvidia.com>
>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>> 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.
> 
> It seems like you're giving general guidance for how a vendor driver
> open() function should work, yet a hard dependency on KVM should be
> discouraged.  You're making a choice for your vendor driver alone.

I apologize for any confusion, but all I meant here was, if the real
world requires a vendor driver to indicate errors instead of false
success, it has to know some information before making the choice.

> I would also be very cautious about the coherency of signaling the KVM
> association relative to the user of the group.  Is it possible that the
> association of one KVM instance by a user of the group can leak to the
> next user?  Does vfio need to seen a gratuitous un-set of the KVM
> association on group close()? etc.  Thanks,

I failed to see how this is possible, per my understanding the
vfio_group_set_kvm gets called twice (once with kvm, another with NULL)
during kvm's holding the group reference.

Would you elaborate a bit more?


--
Thanks,
Jike

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web