[Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Jason Wang
Currently, except for the create and remove, the rest of
mdev_parent_ops is designed for vfio-mdev driver only and may not help
for kernel mdev driver. With the help of class id, this patch
introduces device specific callbacks inside mdev_device
structure. This allows different set of callback to be used by
vfio-mdev and virtio-mdev.

Signed-off-by: Jason Wang 
---
 .../driver-api/vfio-mediated-device.rst   | 25 +
 MAINTAINERS   |  1 +
 drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
 drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
 drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
 drivers/vfio/mdev/mdev_core.c | 18 +--
 drivers/vfio/mdev/mdev_private.h  |  1 +
 drivers/vfio/mdev/vfio_mdev.c | 37 ++---
 include/linux/mdev.h  | 45 
 include/linux/vfio_mdev.h | 52 +++
 samples/vfio-mdev/mbochs.c| 20 ---
 samples/vfio-mdev/mdpy.c  | 20 ---
 samples/vfio-mdev/mtty.c  | 18 ---
 13 files changed, 184 insertions(+), 103 deletions(-)
 create mode 100644 include/linux/vfio_mdev.h

diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
b/Documentation/driver-api/vfio-mediated-device.rst
index f9a78d75a67a..0cca84d19603 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or any 
other categorization.
 Vendor drivers are expected to be fully asynchronous in this respect or
 provide their own internal resource protection.)
 
-The callbacks in the mdev_parent_ops structure are as follows:
-
-* open: open callback of mediated device
-* close: close callback of mediated device
-* ioctl: ioctl callback of mediated device
+As multiple types of mediated devices may be supported, the device
+must set up the class id and the device specific callbacks in create()
+callback. E.g for vfio-mdev device it needs to be done through:
+
+int mdev_set_vfio_ops(struct mdev_device *mdev,
+  const struct vfio_mdev_ops *vfio_ops);
+
+The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device
+with an mdev driver via its id table. The device specific callbacks
+(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by
+the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
+uses the following device-specific ops:
+
+* open: open callback of vfio mediated device
+* close: close callback of vfio mediated device
+* ioctl: ioctl callback of vfio mediated device
 * read : read emulation callback
 * write: write emulation callback
 * mmap: mmap emulation callback
@@ -167,10 +178,6 @@ register itself with the mdev core driver::
extern int  mdev_register_device(struct device *dev,
 const struct mdev_parent_ops *ops);
 
-It is also required to specify the class_id in create() callback through::
-
-   int mdev_set_class(struct mdev_device *mdev, u16 id);
-
 However, the mdev_parent_ops structure is not required in the function call
 that a driver should use to unregister itself with the mdev core driver::
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 8824f61cd2c0..3d196a023b5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17127,6 +17127,7 @@ S:  Maintained
 F: Documentation/driver-api/vfio-mediated-device.rst
 F: drivers/vfio/mdev/
 F: include/linux/mdev.h
+F: include/linux/vfio_mdev.h
 F: samples/vfio-mdev/
 
 VFIO PLATFORM DRIVER
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 6420f0dbd31b..306f934c7857 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -643,6 +644,8 @@ static void kvmgt_put_vfio_device(void *vgpu)
vfio_device_put(((struct intel_vgpu *)vgpu)->vdev.vfio_device);
 }
 
+static const struct vfio_mdev_device_ops intel_vfio_vgpu_dev_ops;
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
struct intel_vgpu *vgpu = NULL;
@@ -678,7 +681,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct 
mdev_device *mdev)
 dev_name(mdev_dev(mdev)));
ret = 0;
 
-   mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
+   mdev_set_vfio_ops(mdev, &intel_vfio_vgpu_dev_ops);
 out:
return ret;
 }
@@ -1599,20 +1602,21 @@ static const struct attribute_group 
*intel_vgpu_groups[] = {
NULL,
 };
 
-static struct mdev_parent_ops intel_vgpu_ops = {
-   .mdev_attr_groups   = intel_vgpu_groups,
-   .create = intel_vgpu_create,
-   .remove = intel_vgpu_remove,
-
+static const struct vfio_mdev_device_ops intel_vf

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Cornelia Huck
On Thu, 17 Oct 2019 18:48:33 +0800
Jason Wang  wrote:

> Currently, except for the create and remove, the rest of
> mdev_parent_ops is designed for vfio-mdev driver only and may not help
> for kernel mdev driver. With the help of class id, this patch
> introduces device specific callbacks inside mdev_device
> structure. This allows different set of callback to be used by
> vfio-mdev and virtio-mdev.
> 
> Signed-off-by: Jason Wang 
> ---
>  .../driver-api/vfio-mediated-device.rst   | 25 +
>  MAINTAINERS   |  1 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
>  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
>  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
>  drivers/vfio/mdev/mdev_core.c | 18 +--
>  drivers/vfio/mdev/mdev_private.h  |  1 +
>  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
>  include/linux/mdev.h  | 45 
>  include/linux/vfio_mdev.h | 52 +++
>  samples/vfio-mdev/mbochs.c| 20 ---
>  samples/vfio-mdev/mdpy.c  | 20 ---
>  samples/vfio-mdev/mtty.c  | 18 ---
>  13 files changed, 184 insertions(+), 103 deletions(-)
>  create mode 100644 include/linux/vfio_mdev.h
> 
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
> b/Documentation/driver-api/vfio-mediated-device.rst
> index f9a78d75a67a..0cca84d19603 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or any 
> other categorization.
>  Vendor drivers are expected to be fully asynchronous in this respect or
>  provide their own internal resource protection.)
>  
> -The callbacks in the mdev_parent_ops structure are as follows:
> -
> -* open: open callback of mediated device
> -* close: close callback of mediated device
> -* ioctl: ioctl callback of mediated device
> +As multiple types of mediated devices may be supported, the device
> +must set up the class id and the device specific callbacks in create()

s/in create()/in the create()/

> +callback. E.g for vfio-mdev device it needs to be done through:

"Each class provides a helper function to do so; e.g. for vfio-mdev
devices, the function to be called is:"

?

> +
> +int mdev_set_vfio_ops(struct mdev_device *mdev,
> +  const struct vfio_mdev_ops *vfio_ops);
> +
> +The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device

"(set by this helper function to MDEV_CLASS_ID_VFIO)" ?

> +with an mdev driver via its id table. The device specific callbacks
> +(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by

"(specified in *vfio_ops by the caller)" ?

> +the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
> +uses the following device-specific ops:
> +
> +* open: open callback of vfio mediated device
> +* close: close callback of vfio mediated device
> +* ioctl: ioctl callback of vfio mediated device
>  * read : read emulation callback
>  * write: write emulation callback
>  * mmap: mmap emulation callback
> @@ -167,10 +178,6 @@ register itself with the mdev core driver::
>   extern int  mdev_register_device(struct device *dev,
>const struct mdev_parent_ops *ops);
>  
> -It is also required to specify the class_id in create() callback through::
> -
> - int mdev_set_class(struct mdev_device *mdev, u16 id);
> -

I'm wondering if this patch set should start out with introducing
helper functions already (i.e. don't introduce mdev_set_class(), but
start out with mdev_set_class_vfio() which will gain the *vfio_ops
argument in this patch.)

>  However, the mdev_parent_ops structure is not required in the function call
>  that a driver should use to unregister itself with the mdev core driver::
>  

(...)

> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 3a9c52d71b4e..d0f3113c8071 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -45,15 +45,23 @@ void mdev_set_drvdata(struct mdev_device *mdev, void 
> *data)
>  }
>  EXPORT_SYMBOL(mdev_set_drvdata);
>  
> -/* Specify the class for the mdev device, this must be called during
> - * create() callback.
> +/* Specify the VFIO device ops for the mdev device, this
> + * must be called during create() callback for VFIO mdev device.
>   */

/*
 * Specify the mdev device to be a VFIO mdev device, and set the
 * VFIO devices ops for it. This must be called from the create()
 * callback for VFIO mdev devices.
 */

?

> -void mdev_set_class(struct mdev_device *mdev, u16 id)
> +void mdev_set_vfio_ops(struct mdev_device *mdev,
> +const struct vfio_mdev_device_ops *vfio_ops)
>  {
>   WARN_ON(mdev->class_id);
> - mdev->class_

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-17 Thread Alex Williamson
On Thu, 17 Oct 2019 17:07:55 +0200
Cornelia Huck  wrote:

> On Thu, 17 Oct 2019 18:48:33 +0800
> Jason Wang  wrote:
> 
> > Currently, except for the create and remove, the rest of
> > mdev_parent_ops is designed for vfio-mdev driver only and may not help
> > for kernel mdev driver. With the help of class id, this patch
> > introduces device specific callbacks inside mdev_device
> > structure. This allows different set of callback to be used by
> > vfio-mdev and virtio-mdev.
> > 
> > Signed-off-by: Jason Wang 
> > ---
> >  .../driver-api/vfio-mediated-device.rst   | 25 +
> >  MAINTAINERS   |  1 +
> >  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
> >  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
> >  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
> >  drivers/vfio/mdev/mdev_core.c | 18 +--
> >  drivers/vfio/mdev/mdev_private.h  |  1 +
> >  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
> >  include/linux/mdev.h  | 45 
> >  include/linux/vfio_mdev.h | 52 +++
> >  samples/vfio-mdev/mbochs.c| 20 ---
> >  samples/vfio-mdev/mdpy.c  | 20 ---
> >  samples/vfio-mdev/mtty.c  | 18 ---
> >  13 files changed, 184 insertions(+), 103 deletions(-)
> >  create mode 100644 include/linux/vfio_mdev.h
> > 
> > diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
> > b/Documentation/driver-api/vfio-mediated-device.rst
> > index f9a78d75a67a..0cca84d19603 100644
> > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > @@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or 
> > any other categorization.
> >  Vendor drivers are expected to be fully asynchronous in this respect or
> >  provide their own internal resource protection.)
> >  
> > -The callbacks in the mdev_parent_ops structure are as follows:
> > -
> > -* open: open callback of mediated device
> > -* close: close callback of mediated device
> > -* ioctl: ioctl callback of mediated device
> > +As multiple types of mediated devices may be supported, the device
> > +must set up the class id and the device specific callbacks in create()  
> 
> s/in create()/in the create()/
> 
> > +callback. E.g for vfio-mdev device it needs to be done through:  
> 
> "Each class provides a helper function to do so; e.g. for vfio-mdev
> devices, the function to be called is:"
> 
> ?
> 
> > +
> > +int mdev_set_vfio_ops(struct mdev_device *mdev,
> > +  const struct vfio_mdev_ops *vfio_ops);
> > +
> > +The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device  
> 
> "(set by this helper function to MDEV_CLASS_ID_VFIO)" ?
> 
> > +with an mdev driver via its id table. The device specific callbacks
> > +(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by  
> 
> "(specified in *vfio_ops by the caller)" ?
> 
> > +the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
> > +uses the following device-specific ops:
> > +
> > +* open: open callback of vfio mediated device
> > +* close: close callback of vfio mediated device
> > +* ioctl: ioctl callback of vfio mediated device
> >  * read : read emulation callback
> >  * write: write emulation callback
> >  * mmap: mmap emulation callback
> > @@ -167,10 +178,6 @@ register itself with the mdev core driver::
> > extern int  mdev_register_device(struct device *dev,
> >  const struct mdev_parent_ops *ops);
> >  
> > -It is also required to specify the class_id in create() callback through::
> > -
> > -   int mdev_set_class(struct mdev_device *mdev, u16 id);
> > -  
> 
> I'm wondering if this patch set should start out with introducing
> helper functions already (i.e. don't introduce mdev_set_class(), but
> start out with mdev_set_class_vfio() which will gain the *vfio_ops
> argument in this patch.)

Yes, it would be cleaner, but is it really worth the churn?  Correct me
if I'm wrong, but I think we get to the same point after this patch and
aside from the function name itself, the difference is really just that
the class_id is briefly exposed to the parent driver, right?  Thanks,

Alex
 
> >  However, the mdev_parent_ops structure is not required in the function call
> >  that a driver should use to unregister itself with the mdev core driver::
> >
> 
> (...)
> 
> > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> > index 3a9c52d71b4e..d0f3113c8071 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -45,15 +45,23 @@ void mdev_set_drvdata(struct mdev_device *mdev, void 
> > *data)
> >  }
> >  EXPORT_SYMBOL(mdev_set_drvdata);
> >  
> > -/* Specify the class for the mdev device, this must be called during
> >

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-18 Thread Jason Wang


On 2019/10/17 下午11:07, Cornelia Huck wrote:

On Thu, 17 Oct 2019 18:48:33 +0800
Jason Wang  wrote:


Currently, except for the create and remove, the rest of
mdev_parent_ops is designed for vfio-mdev driver only and may not help
for kernel mdev driver. With the help of class id, this patch
introduces device specific callbacks inside mdev_device
structure. This allows different set of callback to be used by
vfio-mdev and virtio-mdev.

Signed-off-by: Jason Wang 
---
  .../driver-api/vfio-mediated-device.rst   | 25 +
  MAINTAINERS   |  1 +
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
  drivers/vfio/mdev/mdev_core.c | 18 +--
  drivers/vfio/mdev/mdev_private.h  |  1 +
  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
  include/linux/mdev.h  | 45 
  include/linux/vfio_mdev.h | 52 +++
  samples/vfio-mdev/mbochs.c| 20 ---
  samples/vfio-mdev/mdpy.c  | 20 ---
  samples/vfio-mdev/mtty.c  | 18 ---
  13 files changed, 184 insertions(+), 103 deletions(-)
  create mode 100644 include/linux/vfio_mdev.h

diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
b/Documentation/driver-api/vfio-mediated-device.rst
index f9a78d75a67a..0cca84d19603 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or any 
other categorization.
  Vendor drivers are expected to be fully asynchronous in this respect or
  provide their own internal resource protection.)
  
-The callbacks in the mdev_parent_ops structure are as follows:

-
-* open: open callback of mediated device
-* close: close callback of mediated device
-* ioctl: ioctl callback of mediated device
+As multiple types of mediated devices may be supported, the device
+must set up the class id and the device specific callbacks in create()

s/in create()/in the create()/



Will fix.





+callback. E.g for vfio-mdev device it needs to be done through:

"Each class provides a helper function to do so; e.g. for vfio-mdev
devices, the function to be called is:"

?



This looks better.





+
+int mdev_set_vfio_ops(struct mdev_device *mdev,
+  const struct vfio_mdev_ops *vfio_ops);
+
+The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device

"(set by this helper function to MDEV_CLASS_ID_VFIO)" ?



Yes.



+with an mdev driver via its id table. The device specific callbacks
+(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by

"(specified in *vfio_ops by the caller)" ?



Yes.



+the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
+uses the following device-specific ops:
+
+* open: open callback of vfio mediated device
+* close: close callback of vfio mediated device
+* ioctl: ioctl callback of vfio mediated device
  * read : read emulation callback
  * write: write emulation callback
  * mmap: mmap emulation callback
@@ -167,10 +178,6 @@ register itself with the mdev core driver::
extern int  mdev_register_device(struct device *dev,
 const struct mdev_parent_ops *ops);
  
-It is also required to specify the class_id in create() callback through::

-
-   int mdev_set_class(struct mdev_device *mdev, u16 id);
-

I'm wondering if this patch set should start out with introducing
helper functions already (i.e. don't introduce mdev_set_class(), but
start out with mdev_set_class_vfio() which will gain the *vfio_ops
argument in this patch.)



I think it doesn't harm to keep it as is since in patch 1 we introduce 
class_id and bus match method based on that without device ops there.  
But if you stick I can change.


Thanks





  However, the mdev_parent_ops structure is not required in the function call
  that a driver should use to unregister itself with the mdev core driver::
  

(...)


diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 3a9c52d71b4e..d0f3113c8071 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -45,15 +45,23 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
  }
  EXPORT_SYMBOL(mdev_set_drvdata);
  
-/* Specify the class for the mdev device, this must be called during

- * create() callback.
+/* Specify the VFIO device ops for the mdev device, this
+ * must be called during create() callback for VFIO mdev device.
   */

/*
  * Specify the mdev device to be a VFIO mdev device, and set the
  * VFIO devices ops for it. This must be called from the create()
  * callback for VFIO mdev devices.
  */

?


-void mdev_set_class(struct mdev_device *mdev, u16 id)
+vo

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-18 Thread Cornelia Huck
On Thu, 17 Oct 2019 11:53:10 -0600
Alex Williamson  wrote:

> On Thu, 17 Oct 2019 17:07:55 +0200
> Cornelia Huck  wrote:
> 
> > On Thu, 17 Oct 2019 18:48:33 +0800
> > Jason Wang  wrote:
> >   
> > > Currently, except for the create and remove, the rest of
> > > mdev_parent_ops is designed for vfio-mdev driver only and may not help
> > > for kernel mdev driver. With the help of class id, this patch
> > > introduces device specific callbacks inside mdev_device
> > > structure. This allows different set of callback to be used by
> > > vfio-mdev and virtio-mdev.
> > > 
> > > Signed-off-by: Jason Wang 
> > > ---
> > >  .../driver-api/vfio-mediated-device.rst   | 25 +
> > >  MAINTAINERS   |  1 +
> > >  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
> > >  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
> > >  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
> > >  drivers/vfio/mdev/mdev_core.c | 18 +--
> > >  drivers/vfio/mdev/mdev_private.h  |  1 +
> > >  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
> > >  include/linux/mdev.h  | 45 
> > >  include/linux/vfio_mdev.h | 52 +++
> > >  samples/vfio-mdev/mbochs.c| 20 ---
> > >  samples/vfio-mdev/mdpy.c  | 20 ---
> > >  samples/vfio-mdev/mtty.c  | 18 ---
> > >  13 files changed, 184 insertions(+), 103 deletions(-)
> > >  create mode 100644 include/linux/vfio_mdev.h
> > > 
> > > diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
> > > b/Documentation/driver-api/vfio-mediated-device.rst
> > > index f9a78d75a67a..0cca84d19603 100644
> > > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > > @@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type, or 
> > > any other categorization.
> > >  Vendor drivers are expected to be fully asynchronous in this respect or
> > >  provide their own internal resource protection.)
> > >  
> > > -The callbacks in the mdev_parent_ops structure are as follows:
> > > -
> > > -* open: open callback of mediated device
> > > -* close: close callback of mediated device
> > > -* ioctl: ioctl callback of mediated device
> > > +As multiple types of mediated devices may be supported, the device
> > > +must set up the class id and the device specific callbacks in create()   
> > >  
> > 
> > s/in create()/in the create()/
> >   
> > > +callback. E.g for vfio-mdev device it needs to be done through:
> > 
> > "Each class provides a helper function to do so; e.g. for vfio-mdev
> > devices, the function to be called is:"
> > 
> > ?
> >   
> > > +
> > > +int mdev_set_vfio_ops(struct mdev_device *mdev,
> > > +  const struct vfio_mdev_ops *vfio_ops);
> > > +
> > > +The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device
> > 
> > "(set by this helper function to MDEV_CLASS_ID_VFIO)" ?
> >   
> > > +with an mdev driver via its id table. The device specific callbacks
> > > +(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by
> > 
> > "(specified in *vfio_ops by the caller)" ?
> >   
> > > +the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
> > > +uses the following device-specific ops:
> > > +
> > > +* open: open callback of vfio mediated device
> > > +* close: close callback of vfio mediated device
> > > +* ioctl: ioctl callback of vfio mediated device
> > >  * read : read emulation callback
> > >  * write: write emulation callback
> > >  * mmap: mmap emulation callback
> > > @@ -167,10 +178,6 @@ register itself with the mdev core driver::
> > >   extern int  mdev_register_device(struct device *dev,
> > >const struct mdev_parent_ops *ops);
> > >  
> > > -It is also required to specify the class_id in create() callback 
> > > through::
> > > -
> > > - int mdev_set_class(struct mdev_device *mdev, u16 id);
> > > -
> > 
> > I'm wondering if this patch set should start out with introducing
> > helper functions already (i.e. don't introduce mdev_set_class(), but
> > start out with mdev_set_class_vfio() which will gain the *vfio_ops
> > argument in this patch.)  
> 
> Yes, it would be cleaner, but is it really worth the churn?  Correct me
> if I'm wrong, but I think we get to the same point after this patch and
> aside from the function name itself, the difference is really just that
> the class_id is briefly exposed to the parent driver, right?  Thanks,

Yes, it does not really matter much. If there's no other reason to
rework things, I'd just keep this as it is now.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-20 Thread Jason Wang


On 2019/10/21 上午7:41, Parav Pandit wrote:



-Original Message-
From: Jason Wang 
Sent: Thursday, October 17, 2019 5:49 AM
To: k...@vger.kernel.org; linux-s...@vger.kernel.org; linux-
ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; intel-
g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org;
kwankh...@nvidia.com; alex.william...@redhat.com; m...@redhat.com;
tiwei@intel.com
Cc: virtualizat...@lists.linux-foundation.org; net...@vger.kernel.org;
coh...@redhat.com; maxime.coque...@redhat.com;
cunming.li...@intel.com; zhihong.w...@intel.com;
rob.mil...@broadcom.com; xiao.w.w...@intel.com;
haotian.w...@sifive.com; zhen...@linux.intel.com; zhi.a.w...@intel.com;
jani.nik...@linux.intel.com; joonas.lahti...@linux.intel.com;
rodrigo.v...@intel.com; airl...@linux.ie; dan...@ffwll.ch;
far...@linux.ibm.com; pa...@linux.ibm.com; seb...@linux.ibm.com;
ober...@linux.ibm.com; heiko.carst...@de.ibm.com; g...@linux.ibm.com;
borntrae...@de.ibm.com; akrow...@linux.ibm.com; fre...@linux.ibm.com;
lingshan@intel.com; Ido Shamay ;
epere...@redhat.com; l...@redhat.com; Parav Pandit
; christophe.de.dinec...@gmail.com;
kevin.t...@intel.com; stefa...@redhat.com; Jason Wang

Subject: [PATCH V4 3/6] mdev: introduce device specific ops

Currently, except for the create and remove, the rest of mdev_parent_ops is
designed for vfio-mdev driver only and may not help for kernel mdev driver.
With the help of class id, this patch introduces device specific callbacks
inside mdev_device structure. This allows different set of callback to be used
by vfio-mdev and virtio-mdev.

Signed-off-by: Jason Wang 
---
  .../driver-api/vfio-mediated-device.rst   | 25 +
  MAINTAINERS   |  1 +
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
  drivers/vfio/mdev/mdev_core.c | 18 +--
  drivers/vfio/mdev/mdev_private.h  |  1 +
  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
  include/linux/mdev.h  | 45 
  include/linux/vfio_mdev.h | 52 +++
  samples/vfio-mdev/mbochs.c| 20 ---
  samples/vfio-mdev/mdpy.c  | 20 ---
  samples/vfio-mdev/mtty.c  | 18 ---
  13 files changed, 184 insertions(+), 103 deletions(-)  create mode 100644
include/linux/vfio_mdev.h

diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index f9a78d75a67a..0cca84d19603 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type,
or any other categorization.
  Vendor drivers are expected to be fully asynchronous in this respect or
provide their own internal resource protection.)

-The callbacks in the mdev_parent_ops structure are as follows:
-
-* open: open callback of mediated device
-* close: close callback of mediated device
-* ioctl: ioctl callback of mediated device
+As multiple types of mediated devices may be supported, the device must
+set up the class id and the device specific callbacks in create()
+callback. E.g for vfio-mdev device it needs to be done through:
+
+int mdev_set_vfio_ops(struct mdev_device *mdev,
+  const struct vfio_mdev_ops *vfio_ops);
+
+The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device with
+an mdev driver via its id table. The device specific callbacks
+(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by
+the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
+uses the following device-specific ops:
+
+* open: open callback of vfio mediated device
+* close: close callback of vfio mediated device
+* ioctl: ioctl callback of vfio mediated device
  * read : read emulation callback
  * write: write emulation callback
  * mmap: mmap emulation callback
@@ -167,10 +178,6 @@ register itself with the mdev core driver::
extern int  mdev_register_device(struct device *dev,
 const struct mdev_parent_ops *ops);

-It is also required to specify the class_id in create() callback through::
-
-   int mdev_set_class(struct mdev_device *mdev, u16 id);
-
  However, the mdev_parent_ops structure is not required in the function call
that a driver should use to unregister itself with the mdev core driver::

diff --git a/MAINTAINERS b/MAINTAINERS
index 8824f61cd2c0..3d196a023b5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17127,6 +17127,7 @@ S:  Maintained
  F:Documentation/driver-api/vfio-mediated-device.rst
  F:drivers/vfio/mdev/
  F:include/linux/mdev.h
+F: include/linux/vfio_mdev.h
  F:samples/vfio-mdev/

  VFIO PLATFORM DRIVER
diff --git a/

Re: [Intel-gfx] [PATCH V4 3/6] mdev: introduce device specific ops

2019-10-21 Thread Parav Pandit


> -Original Message-
> From: Jason Wang 
> Sent: Thursday, October 17, 2019 5:49 AM
> To: k...@vger.kernel.org; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org;
> kwankh...@nvidia.com; alex.william...@redhat.com; m...@redhat.com;
> tiwei@intel.com
> Cc: virtualizat...@lists.linux-foundation.org; net...@vger.kernel.org;
> coh...@redhat.com; maxime.coque...@redhat.com;
> cunming.li...@intel.com; zhihong.w...@intel.com;
> rob.mil...@broadcom.com; xiao.w.w...@intel.com;
> haotian.w...@sifive.com; zhen...@linux.intel.com; zhi.a.w...@intel.com;
> jani.nik...@linux.intel.com; joonas.lahti...@linux.intel.com;
> rodrigo.v...@intel.com; airl...@linux.ie; dan...@ffwll.ch;
> far...@linux.ibm.com; pa...@linux.ibm.com; seb...@linux.ibm.com;
> ober...@linux.ibm.com; heiko.carst...@de.ibm.com; g...@linux.ibm.com;
> borntrae...@de.ibm.com; akrow...@linux.ibm.com; fre...@linux.ibm.com;
> lingshan@intel.com; Ido Shamay ;
> epere...@redhat.com; l...@redhat.com; Parav Pandit
> ; christophe.de.dinec...@gmail.com;
> kevin.t...@intel.com; stefa...@redhat.com; Jason Wang
> 
> Subject: [PATCH V4 3/6] mdev: introduce device specific ops
> 
> Currently, except for the create and remove, the rest of mdev_parent_ops is
> designed for vfio-mdev driver only and may not help for kernel mdev driver.
> With the help of class id, this patch introduces device specific callbacks
> inside mdev_device structure. This allows different set of callback to be used
> by vfio-mdev and virtio-mdev.
> 
> Signed-off-by: Jason Wang 
> ---
>  .../driver-api/vfio-mediated-device.rst   | 25 +
>  MAINTAINERS   |  1 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 18 ---
>  drivers/s390/cio/vfio_ccw_ops.c   | 18 ---
>  drivers/s390/crypto/vfio_ap_ops.c | 14 +++--
>  drivers/vfio/mdev/mdev_core.c | 18 +--
>  drivers/vfio/mdev/mdev_private.h  |  1 +
>  drivers/vfio/mdev/vfio_mdev.c | 37 ++---
>  include/linux/mdev.h  | 45 
>  include/linux/vfio_mdev.h | 52 +++
>  samples/vfio-mdev/mbochs.c| 20 ---
>  samples/vfio-mdev/mdpy.c  | 20 ---
>  samples/vfio-mdev/mtty.c  | 18 ---
>  13 files changed, 184 insertions(+), 103 deletions(-)  create mode 100644
> include/linux/vfio_mdev.h
> 
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> b/Documentation/driver-api/vfio-mediated-device.rst
> index f9a78d75a67a..0cca84d19603 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -152,11 +152,22 @@ callbacks per mdev parent device, per mdev type,
> or any other categorization.
>  Vendor drivers are expected to be fully asynchronous in this respect or
> provide their own internal resource protection.)
> 
> -The callbacks in the mdev_parent_ops structure are as follows:
> -
> -* open: open callback of mediated device
> -* close: close callback of mediated device
> -* ioctl: ioctl callback of mediated device
> +As multiple types of mediated devices may be supported, the device must
> +set up the class id and the device specific callbacks in create()
> +callback. E.g for vfio-mdev device it needs to be done through:
> +
> +int mdev_set_vfio_ops(struct mdev_device *mdev,
> +  const struct vfio_mdev_ops *vfio_ops);
> +
> +The class id (set to MDEV_CLASS_ID_VFIO) is used to match a device with
> +an mdev driver via its id table. The device specific callbacks
> +(specified in *ops) are obtainable via mdev_get_dev_ops() (for use by
> +the mdev bus driver). A vfio-mdev device (class id MDEV_CLASS_ID_VFIO)
> +uses the following device-specific ops:
> +
> +* open: open callback of vfio mediated device
> +* close: close callback of vfio mediated device
> +* ioctl: ioctl callback of vfio mediated device
>  * read : read emulation callback
>  * write: write emulation callback
>  * mmap: mmap emulation callback
> @@ -167,10 +178,6 @@ register itself with the mdev core driver::
>   extern int  mdev_register_device(struct device *dev,
>const struct mdev_parent_ops *ops);
> 
> -It is also required to specify the class_id in create() callback through::
> -
> - int mdev_set_class(struct mdev_device *mdev, u16 id);
> -
>  However, the mdev_parent_ops structure is not required in the function call
> that a driver should use to unregister itself with the mdev core driver::
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8824f61cd2c0..3d196a023b5e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17127,6 +17127,7 @@ S:Maintained
>  F:   Documentation/driver-api/vfio-mediated-device.rst
>  F:   drivers/vfio/mdev/
>