Re: [Intel-gfx] [PATCH v4 15/15] vfio: Add struct device to vfio_device

2022-09-29 Thread Tian, Kevin
> From: Alex Williamson 
> Sent: Friday, September 30, 2022 2:24 AM
> 
> On Thu, 29 Sep 2022 14:49:56 -0300
> Jason Gunthorpe  wrote:
> 
> > On Thu, Sep 29, 2022 at 10:55:19AM -0600, Alex Williamson wrote:
> > > Hi Kevin,
> > >
> > > This introduced the regression discovered here:
> > >
> > >
> https://lore.kernel.org/all/20220928125650.0a2ea297.alex.williamson@redh
> at.com/
> > >
> > > Seems we're not releasing the resources when removing an mdev.  This is
> > > a regression, so it needs to be fixed or reverted before the merge
> > > window.  Thanks,
> >
> > My guess at the fix for this:
> >
> > https://lore.kernel.org/r/0-v1-013609965fe8+9d-
> vfio_gvt_unregister_...@nvidia.com
> 
> Indeed this seems to work  I'll look for acks and further reviews from
> Intel folks. Thanks!
> 

Thanks Jason for baking a quick fix! I've acked it.


Re: [Intel-gfx] [PATCH v4 15/15] vfio: Add struct device to vfio_device

2022-09-29 Thread Alex Williamson
On Thu, 29 Sep 2022 14:49:56 -0300
Jason Gunthorpe  wrote:

> On Thu, Sep 29, 2022 at 10:55:19AM -0600, Alex Williamson wrote:
> > Hi Kevin,
> > 
> > This introduced the regression discovered here:
> > 
> > https://lore.kernel.org/all/20220928125650.0a2ea297.alex.william...@redhat.com/
> > 
> > Seems we're not releasing the resources when removing an mdev.  This is
> > a regression, so it needs to be fixed or reverted before the merge
> > window.  Thanks,  
> 
> My guess at the fix for this:
> 
> https://lore.kernel.org/r/0-v1-013609965fe8+9d-vfio_gvt_unregister_...@nvidia.com

Indeed this seems to work  I'll look for acks and further reviews from
Intel folks. Thanks!

Alex



Re: [Intel-gfx] [PATCH v4 15/15] vfio: Add struct device to vfio_device

2022-09-29 Thread Alex Williamson
Hi Kevin,

This introduced the regression discovered here:

https://lore.kernel.org/all/20220928125650.0a2ea297.alex.william...@redhat.com/

Seems we're not releasing the resources when removing an mdev.  This is
a regression, so it needs to be fixed or reverted before the merge
window.  Thanks,

Alex

On Wed, 21 Sep 2022 18:44:01 +0800
Kevin Tian  wrote:

> From: Yi Liu 
> 
> and replace kref. With it a 'vfio-dev/vfioX' node is created under the
> sysfs path of the parent, indicating the device is bound to a vfio
> driver, e.g.:
> 
> /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0
> 
> It is also a preparatory step toward adding cdev for supporting future
> device-oriented uAPI.
> 
> Add Documentation/ABI/testing/sysfs-devices-vfio-dev.
> 
> Suggested-by: Jason Gunthorpe 
> Signed-off-by: Yi Liu 
> Signed-off-by: Kevin Tian 
> Reviewed-by: Jason Gunthorpe 
> ---
>  .../ABI/testing/sysfs-devices-vfio-dev|  8 +++
>  MAINTAINERS   |  1 +
>  drivers/vfio/vfio_main.c  | 64 +++
>  include/linux/vfio.h  |  6 +-
>  4 files changed, 65 insertions(+), 14 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-devices-vfio-dev
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-vfio-dev 
> b/Documentation/ABI/testing/sysfs-devices-vfio-dev
> new file mode 100644
> index ..e21424fd9666
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-vfio-dev
> @@ -0,0 +1,8 @@
> +What: /sys/...//vfio-dev/vfioX/
> +Date: September 2022
> +Contact:  Yi Liu 
> +Description:
> +  This directory is created when the device is bound to a
> +  vfio driver. The layout under this directory matches what
> +  exists for a standard 'struct device'. 'X' is a unique
> +  index marking this device in vfio.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d30f26e07cd3..02c8f11b1c17 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21312,6 +21312,7 @@ R:Cornelia Huck 
>  L:   k...@vger.kernel.org
>  S:   Maintained
>  T:   git git://github.com/awilliam/linux-vfio.git
> +F:   Documentation/ABI/testing/sysfs-devices-vfio-dev
>  F:   Documentation/driver-api/vfio.rst
>  F:   drivers/vfio/
>  F:   include/linux/vfio.h
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index c27449613a1d..f9d10dbcf3e6 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -49,6 +49,8 @@ static struct vfio {
>   struct mutexgroup_lock; /* locks group_list */
>   struct ida  group_ida;
>   dev_t   group_devt;
> + struct class*device_class;
> + struct ida  device_ida;
>  } vfio;
>  
>  struct vfio_iommu_driver {
> @@ -485,12 +487,13 @@ static struct vfio_device *vfio_group_get_device(struct 
> vfio_group *group,
>   * VFIO driver API
>   */
>  /* Release helper called by vfio_put_device() */
> -void vfio_device_release(struct kref *kref)
> +static void vfio_device_release(struct device *dev)
>  {
>   struct vfio_device *device =
> - container_of(kref, struct vfio_device, kref);
> + container_of(dev, struct vfio_device, device);
>  
>   vfio_release_device_set(device);
> + ida_free(_ida, device->index);
>  
>   /*
>* kvfree() cannot be done here due to a life cycle mess in
> @@ -500,7 +503,6 @@ void vfio_device_release(struct kref *kref)
>*/
>   device->ops->release(device);
>  }
> -EXPORT_SYMBOL_GPL(vfio_device_release);
>  
>  /*
>   * Allocate and initialize vfio_device so it can be registered to vfio
> @@ -548,6 +550,13 @@ int vfio_init_device(struct vfio_device *device, struct 
> device *dev,
>  {
>   int ret;
>  
> + ret = ida_alloc_max(_ida, MINORMASK, GFP_KERNEL);
> + if (ret < 0) {
> + dev_dbg(dev, "Error to alloc index\n");
> + return ret;
> + }
> +
> + device->index = ret;
>   init_completion(>comp);
>   device->dev = dev;
>   device->ops = ops;
> @@ -558,11 +567,15 @@ int vfio_init_device(struct vfio_device *device, struct 
> device *dev,
>   goto out_uninit;
>   }
>  
> - kref_init(>kref);
> + device_initialize(>device);
> + device->device.release = vfio_device_release;
> + device->device.class = vfio.device_class;
> + device->device.parent = device->dev;
>   return 0;
>  
>  out_uninit:
>   vfio_release_device_set(device);
> + ida_free(_ida, device->index);
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(vfio_init_device);
> @@ -659,6 +672,7 @@ static int __vfio_register_dev(struct vfio_device *device,
>   struct vfio_group *group)
>  {
>   struct vfio_device *existing_device;
> + int ret;
>  
>   if (IS_ERR(group))
>   return PTR_ERR(group);
> 

[Intel-gfx] [PATCH v4 15/15] vfio: Add struct device to vfio_device

2022-09-20 Thread Kevin Tian
From: Yi Liu 

and replace kref. With it a 'vfio-dev/vfioX' node is created under the
sysfs path of the parent, indicating the device is bound to a vfio
driver, e.g.:

/sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0

It is also a preparatory step toward adding cdev for supporting future
device-oriented uAPI.

Add Documentation/ABI/testing/sysfs-devices-vfio-dev.

Suggested-by: Jason Gunthorpe 
Signed-off-by: Yi Liu 
Signed-off-by: Kevin Tian 
Reviewed-by: Jason Gunthorpe 
---
 .../ABI/testing/sysfs-devices-vfio-dev|  8 +++
 MAINTAINERS   |  1 +
 drivers/vfio/vfio_main.c  | 64 +++
 include/linux/vfio.h  |  6 +-
 4 files changed, 65 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-vfio-dev

diff --git a/Documentation/ABI/testing/sysfs-devices-vfio-dev 
b/Documentation/ABI/testing/sysfs-devices-vfio-dev
new file mode 100644
index ..e21424fd9666
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-vfio-dev
@@ -0,0 +1,8 @@
+What:   /sys/...//vfio-dev/vfioX/
+Date:   September 2022
+Contact:Yi Liu 
+Description:
+This directory is created when the device is bound to a
+vfio driver. The layout under this directory matches what
+exists for a standard 'struct device'. 'X' is a unique
+index marking this device in vfio.
diff --git a/MAINTAINERS b/MAINTAINERS
index d30f26e07cd3..02c8f11b1c17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21312,6 +21312,7 @@ R:  Cornelia Huck 
 L: k...@vger.kernel.org
 S: Maintained
 T: git git://github.com/awilliam/linux-vfio.git
+F: Documentation/ABI/testing/sysfs-devices-vfio-dev
 F: Documentation/driver-api/vfio.rst
 F: drivers/vfio/
 F: include/linux/vfio.h
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index c27449613a1d..f9d10dbcf3e6 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -49,6 +49,8 @@ static struct vfio {
struct mutexgroup_lock; /* locks group_list */
struct ida  group_ida;
dev_t   group_devt;
+   struct class*device_class;
+   struct ida  device_ida;
 } vfio;
 
 struct vfio_iommu_driver {
@@ -485,12 +487,13 @@ static struct vfio_device *vfio_group_get_device(struct 
vfio_group *group,
  * VFIO driver API
  */
 /* Release helper called by vfio_put_device() */
-void vfio_device_release(struct kref *kref)
+static void vfio_device_release(struct device *dev)
 {
struct vfio_device *device =
-   container_of(kref, struct vfio_device, kref);
+   container_of(dev, struct vfio_device, device);
 
vfio_release_device_set(device);
+   ida_free(_ida, device->index);
 
/*
 * kvfree() cannot be done here due to a life cycle mess in
@@ -500,7 +503,6 @@ void vfio_device_release(struct kref *kref)
 */
device->ops->release(device);
 }
-EXPORT_SYMBOL_GPL(vfio_device_release);
 
 /*
  * Allocate and initialize vfio_device so it can be registered to vfio
@@ -548,6 +550,13 @@ int vfio_init_device(struct vfio_device *device, struct 
device *dev,
 {
int ret;
 
+   ret = ida_alloc_max(_ida, MINORMASK, GFP_KERNEL);
+   if (ret < 0) {
+   dev_dbg(dev, "Error to alloc index\n");
+   return ret;
+   }
+
+   device->index = ret;
init_completion(>comp);
device->dev = dev;
device->ops = ops;
@@ -558,11 +567,15 @@ int vfio_init_device(struct vfio_device *device, struct 
device *dev,
goto out_uninit;
}
 
-   kref_init(>kref);
+   device_initialize(>device);
+   device->device.release = vfio_device_release;
+   device->device.class = vfio.device_class;
+   device->device.parent = device->dev;
return 0;
 
 out_uninit:
vfio_release_device_set(device);
+   ida_free(_ida, device->index);
return ret;
 }
 EXPORT_SYMBOL_GPL(vfio_init_device);
@@ -659,6 +672,7 @@ static int __vfio_register_dev(struct vfio_device *device,
struct vfio_group *group)
 {
struct vfio_device *existing_device;
+   int ret;
 
if (IS_ERR(group))
return PTR_ERR(group);
@@ -675,16 +689,21 @@ static int __vfio_register_dev(struct vfio_device *device,
dev_WARN(device->dev, "Device already exists on group %d\n",
 iommu_group_id(group->iommu_group));
vfio_device_put_registration(existing_device);
-   if (group->type == VFIO_NO_IOMMU ||
-   group->type == VFIO_EMULATED_IOMMU)
-   iommu_group_remove_device(device->dev);
-   vfio_group_put(group);
-   return -EBUSY;
+