Re: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-18 Thread Jason Wang


On 2019/9/18 上午10:57, Tian, Kevin wrote:

From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Tuesday, September 17, 2019 6:17 PM

On 2019/9/17 下午4:09, Tian, Kevin wrote:

From: Jason Wang
Sent: Thursday, September 12, 2019 5:40 PM

Currently, except for the crate and remove. The rest fields of
mdev_parent_ops is just designed for vfio-mdev driver and may not

help

for kernel mdev driver. So follow the device id support by previous
patch, this patch introduces device specific ops which points to
device specific ops (e.g vfio ops). This allows the future drivers
like virtio-mdev to implement its own device specific ops.

Can you give an example about what ops might be required to support
kernel mdev driver? I know you posted a link earlier, but putting a small
example here can save time and avoid inconsistent understanding. Then
it will help whether the proposed split makes sense or there is a
possibility of redefining the callbacks to meet the both requirements.
imo those callbacks fulfill some basic requirements when mediating
a device...

I put it in the cover letter.

The link ishttps://lkml.org/lkml/2019/9/10/135  which abuses the current
VFIO based mdev parent ops.

Thanks

So the main problem is the handling of userspace pointers vs.
kernel space pointers. You still implement read/write/ioctl
callbacks which is a subset of current parent_ops definition.
In that regard is it better to introduce some helper to handle
the pointer difference in mdev core, while still keeping the
same set of parent ops (in whatever form suitable for both)?



Pointers is one of the issues. And read/write/ioctl is designed for 
userspace API not kernel. Technically, we can use them for kernel but it 
would not be as simple and straightforward a set of device specific 
callbacks functions. The link above is just an example, e.g we can 
simply pass the vring address through a dedicated API instead of 
mandatory an offset of a file.


Thanks




___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-17 Thread Jason Wang


On 2019/9/17 下午8:42, Cornelia Huck wrote:

On Thu, 12 Sep 2019 17:40:12 +0800
Jason Wang  wrote:


Currently, except for the crate and remove. The rest fields of
mdev_parent_ops is just designed for vfio-mdev driver and may not help
for kernel mdev driver. So follow the device id support by previous
patch, this patch introduces device specific ops which points to
device specific ops (e.g vfio ops). This allows the future drivers
like virtio-mdev to implement its own device specific ops.

Signed-off-by: Jason Wang 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
  drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
  drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
  drivers/vfio/mdev/vfio_mdev.c | 30 +++--
  include/linux/mdev.h  | 72 ++-
  samples/vfio-mdev/mbochs.c| 16 ---
  samples/vfio-mdev/mdpy.c  | 16 ---
  samples/vfio-mdev/mtty.c  | 14 +++---
  8 files changed, 113 insertions(+), 73 deletions(-)
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index f85045392120..3b8a76bc69cf 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -27,27 +27,9 @@ int mdev_set_iommu_device(struct device *dev, struct device 
*iommu_device);
  struct device *mdev_get_iommu_device(struct device *dev);
  
  /**

- * struct mdev_parent_ops - Structure to be registered for each parent device 
to
- * register the device to mdev module.
+ * struct vfio_mdev_parent_ops - Structure to be registered for each
+ * parent device to register the device to vfio-mdev module.
   *
- * @owner: The module owner.
- * @dev_attr_groups:   Attributes of the parent device.
- * @mdev_attr_groups:  Attributes of the mediated device.
- * @supported_type_groups: Attributes to define supported types. It is 
mandatory
- * to provide supported types.
- * @create:Called to allocate basic resources in parent device's
- * driver for a particular mediated device. It is
- * mandatory to provide create ops.
- * @kobj: kobject of type for which 'create' is called.
- * @mdev: mdev_device structure on of mediated device
- *   that is being created
- * Returns integer: success (0) or error (< 0)
- * @remove:Called to free resources in parent device's driver for a
- * a mediated device. It is mandatory to provide 'remove'
- * ops.
- * @mdev: mdev_device device structure which is being
- *destroyed
- * Returns integer: success (0) or error (< 0)
   * @open: Open mediated device.
   *@mdev: mediated device.
   *Returns integer: success (0) or error (< 0)
@@ -72,6 +54,43 @@ struct device *mdev_get_iommu_device(struct device *dev);
   * @mmap: mmap callback
   *@mdev: mediated device structure
   *@vma: vma structure
+ */
+struct vfio_mdev_parent_ops {
+   int (*open)(struct mdev_device *mdev);
+   void(*release)(struct mdev_device *mdev);
+   ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
+   size_t count, loff_t *ppos);
+   ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
+size_t count, loff_t *ppos);
+   long(*ioctl)(struct mdev_device *mdev, unsigned int cmd,
+unsigned long arg);
+   int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
+};
+
+/**
+ * struct mdev_parent_ops - Structure to be registered for each parent device 
to
+ * register the device to mdev module.
+ *
+ * @owner: The module owner.
+ * @dev_attr_groups:   Attributes of the parent device.
+ * @mdev_attr_groups:  Attributes of the mediated device.
+ * @supported_type_groups: Attributes to define supported types. It is 
mandatory
+ * to provide supported types.
+ * @create:Called to allocate basic resources in parent device's
+ * driver for a particular mediated device. It is
+ * mandatory to provide create ops.
+ * @kobj: kobject of type for which 'create' is called.
+ * @mdev: mdev_device structure on of mediated device
+ *   that is being created
+ * Returns integer: success (0) or error (< 0)
+ * @remove:Called to free resources in parent device's driver for a
+ * a mediated device. It is mandatory to provide 'remove'
+ * ops.
+ * @mdev: mdev_device device structure which is being
+ *destroyed
+ * Returns integer: success (0) or error (< 0)
+ * @device_ops: Device 

RE: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-17 Thread Tian, Kevin
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Tuesday, September 17, 2019 6:17 PM
> 
> On 2019/9/17 下午4:09, Tian, Kevin wrote:
> >> From: Jason Wang
> >> Sent: Thursday, September 12, 2019 5:40 PM
> >>
> >> Currently, except for the crate and remove. The rest fields of
> >> mdev_parent_ops is just designed for vfio-mdev driver and may not
> help
> >> for kernel mdev driver. So follow the device id support by previous
> >> patch, this patch introduces device specific ops which points to
> >> device specific ops (e.g vfio ops). This allows the future drivers
> >> like virtio-mdev to implement its own device specific ops.
> > Can you give an example about what ops might be required to support
> > kernel mdev driver? I know you posted a link earlier, but putting a small
> > example here can save time and avoid inconsistent understanding. Then
> > it will help whether the proposed split makes sense or there is a
> > possibility of redefining the callbacks to meet the both requirements.
> > imo those callbacks fulfill some basic requirements when mediating
> > a device...
> 
> 
> I put it in the cover letter.
> 
> The link is https://lkml.org/lkml/2019/9/10/135 which abuses the current
> VFIO based mdev parent ops.
> 
> Thanks

So the main problem is the handling of userspace pointers vs.
kernel space pointers. You still implement read/write/ioctl
callbacks which is a subset of current parent_ops definition.
In that regard is it better to introduce some helper to handle
the pointer difference in mdev core, while still keeping the 
same set of parent ops (in whatever form suitable for both)? 

> 
> 
> >> Signed-off-by: Jason Wang 
> >> ---
> >>   drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
> >>   drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
> >>   drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
> >>   drivers/vfio/mdev/vfio_mdev.c | 30 +++--
> >>   include/linux/mdev.h  | 72 ++-
> >>   samples/vfio-mdev/mbochs.c| 16 ---
> >>   samples/vfio-mdev/mdpy.c  | 16 ---
> >>   samples/vfio-mdev/mtty.c  | 14 +++---
> >>   8 files changed, 113 insertions(+), 73 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> index 19d51a35f019..64823998fd58 100644
> >> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> @@ -1600,20 +1600,22 @@ 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 struct vfio_mdev_parent_ops intel_vfio_vgpu_ops = {
> >>.open   = intel_vgpu_open,
> >>.release= intel_vgpu_release,
> >> -
> >>.read   = intel_vgpu_read,
> >>.write  = intel_vgpu_write,
> >>.mmap   = intel_vgpu_mmap,
> >>.ioctl  = intel_vgpu_ioctl,
> >>   };
> >>
> >> +static struct mdev_parent_ops intel_vgpu_ops = {
> >> +  .mdev_attr_groups   = intel_vgpu_groups,
> >> +  .create = intel_vgpu_create,
> >> +  .remove = intel_vgpu_remove,
> >> +  .device_ops = _vfio_vgpu_ops,
> >> +};
> >> +
> >>   static int kvmgt_host_init(struct device *dev, void *gvt, const void 
> >> *ops)
> >>   {
> >>struct attribute **kvm_type_attrs;
> >> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> >> b/drivers/s390/cio/vfio_ccw_ops.c
> >> index f87d9409e290..96e9f18100ae 100644
> >> --- a/drivers/s390/cio/vfio_ccw_ops.c
> >> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> >> @@ -564,11 +564,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct
> >> mdev_device *mdev,
> >>}
> >>   }
> >>
> >> -static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> >> -  .owner  = THIS_MODULE,
> >> -  .supported_type_groups  = mdev_type_groups,
> >> -  .create = vfio_ccw_mdev_create,
> >> -  .remove = vfio_ccw_mdev_remove,
> >> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
> >>.open   = vfio_ccw_mdev_open,
> >>.release= vfio_ccw_mdev_release,
> >>.read   = vfio_ccw_mdev_read,
> >> @@ -576,6 +572,14 @@ static const struct mdev_parent_ops
> >> vfio_ccw_mdev_ops = {
> >>.ioctl  = vfio_ccw_mdev_ioctl,
> >>   };
> >>
> >> +static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> >> +  .owner  = THIS_MODULE,
> >> +  .supported_type_groups  = mdev_type_groups,
> >> +  .create = vfio_ccw_mdev_create,
> >> +  .remove = vfio_ccw_mdev_remove,
> >> +  .device_ops = _mdev_ops,
> >> +};
> >> +
> >>   int vfio_ccw_mdev_reg(struct subchannel *sch)
> >>   {
> >>return 

Re: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-17 Thread Cornelia Huck
On Thu, 12 Sep 2019 17:40:12 +0800
Jason Wang  wrote:

> Currently, except for the crate and remove. The rest fields of
> mdev_parent_ops is just designed for vfio-mdev driver and may not help
> for kernel mdev driver. So follow the device id support by previous
> patch, this patch introduces device specific ops which points to
> device specific ops (e.g vfio ops). This allows the future drivers
> like virtio-mdev to implement its own device specific ops.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
>  drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
>  drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
>  drivers/vfio/mdev/vfio_mdev.c | 30 +++--
>  include/linux/mdev.h  | 72 ++-
>  samples/vfio-mdev/mbochs.c| 16 ---
>  samples/vfio-mdev/mdpy.c  | 16 ---
>  samples/vfio-mdev/mtty.c  | 14 +++---
>  8 files changed, 113 insertions(+), 73 deletions(-)

> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index f85045392120..3b8a76bc69cf 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -27,27 +27,9 @@ int mdev_set_iommu_device(struct device *dev, struct 
> device *iommu_device);
>  struct device *mdev_get_iommu_device(struct device *dev);
>  
>  /**
> - * struct mdev_parent_ops - Structure to be registered for each parent 
> device to
> - * register the device to mdev module.
> + * struct vfio_mdev_parent_ops - Structure to be registered for each
> + * parent device to register the device to vfio-mdev module.
>   *
> - * @owner:   The module owner.
> - * @dev_attr_groups: Attributes of the parent device.
> - * @mdev_attr_groups:Attributes of the mediated device.
> - * @supported_type_groups: Attributes to define supported types. It is 
> mandatory
> - *   to provide supported types.
> - * @create:  Called to allocate basic resources in parent device's
> - *   driver for a particular mediated device. It is
> - *   mandatory to provide create ops.
> - *   @kobj: kobject of type for which 'create' is called.
> - *   @mdev: mdev_device structure on of mediated device
> - * that is being created
> - *   Returns integer: success (0) or error (< 0)
> - * @remove:  Called to free resources in parent device's driver for a
> - *   a mediated device. It is mandatory to provide 'remove'
> - *   ops.
> - *   @mdev: mdev_device device structure which is being
> - *  destroyed
> - *   Returns integer: success (0) or error (< 0)
>   * @open:Open mediated device.
>   *   @mdev: mediated device.
>   *   Returns integer: success (0) or error (< 0)
> @@ -72,6 +54,43 @@ struct device *mdev_get_iommu_device(struct device *dev);
>   * @mmap:mmap callback
>   *   @mdev: mediated device structure
>   *   @vma: vma structure
> + */
> +struct vfio_mdev_parent_ops {
> + int (*open)(struct mdev_device *mdev);
> + void(*release)(struct mdev_device *mdev);
> + ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
> + size_t count, loff_t *ppos);
> + ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
> +  size_t count, loff_t *ppos);
> + long(*ioctl)(struct mdev_device *mdev, unsigned int cmd,
> +  unsigned long arg);
> + int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
> +};
> +
> +/**
> + * struct mdev_parent_ops - Structure to be registered for each parent 
> device to
> + * register the device to mdev module.
> + *
> + * @owner:   The module owner.
> + * @dev_attr_groups: Attributes of the parent device.
> + * @mdev_attr_groups:Attributes of the mediated device.
> + * @supported_type_groups: Attributes to define supported types. It is 
> mandatory
> + *   to provide supported types.
> + * @create:  Called to allocate basic resources in parent device's
> + *   driver for a particular mediated device. It is
> + *   mandatory to provide create ops.
> + *   @kobj: kobject of type for which 'create' is called.
> + *   @mdev: mdev_device structure on of mediated device
> + * that is being created
> + *   Returns integer: success (0) or error (< 0)
> + * @remove:  Called to free resources in parent device's driver for a
> + *   a mediated device. It is mandatory to provide 'remove'
> + *   ops.
> + *   @mdev: mdev_device device structure which is being
> + *  destroyed
> + *   Returns integer: success (0) 

Re: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-17 Thread Jason Wang


On 2019/9/17 下午4:09, Tian, Kevin wrote:

From: Jason Wang
Sent: Thursday, September 12, 2019 5:40 PM

Currently, except for the crate and remove. The rest fields of
mdev_parent_ops is just designed for vfio-mdev driver and may not help
for kernel mdev driver. So follow the device id support by previous
patch, this patch introduces device specific ops which points to
device specific ops (e.g vfio ops). This allows the future drivers
like virtio-mdev to implement its own device specific ops.

Can you give an example about what ops might be required to support
kernel mdev driver? I know you posted a link earlier, but putting a small
example here can save time and avoid inconsistent understanding. Then
it will help whether the proposed split makes sense or there is a
possibility of redefining the callbacks to meet the both requirements.
imo those callbacks fulfill some basic requirements when mediating
a device...



I put it in the cover letter.

The link is https://lkml.org/lkml/2019/9/10/135 which abuses the current 
VFIO based mdev parent ops.


Thanks



Signed-off-by: Jason Wang 
---
  drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
  drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
  drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
  drivers/vfio/mdev/vfio_mdev.c | 30 +++--
  include/linux/mdev.h  | 72 ++-
  samples/vfio-mdev/mbochs.c| 16 ---
  samples/vfio-mdev/mdpy.c  | 16 ---
  samples/vfio-mdev/mtty.c  | 14 +++---
  8 files changed, 113 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 19d51a35f019..64823998fd58 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1600,20 +1600,22 @@ 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 struct vfio_mdev_parent_ops intel_vfio_vgpu_ops = {
.open   = intel_vgpu_open,
.release= intel_vgpu_release,
-
.read   = intel_vgpu_read,
.write  = intel_vgpu_write,
.mmap   = intel_vgpu_mmap,
.ioctl  = intel_vgpu_ioctl,
  };

+static struct mdev_parent_ops intel_vgpu_ops = {
+   .mdev_attr_groups   = intel_vgpu_groups,
+   .create = intel_vgpu_create,
+   .remove = intel_vgpu_remove,
+   .device_ops = _vfio_vgpu_ops,
+};
+
  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
  {
struct attribute **kvm_type_attrs;
diff --git a/drivers/s390/cio/vfio_ccw_ops.c
b/drivers/s390/cio/vfio_ccw_ops.c
index f87d9409e290..96e9f18100ae 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -564,11 +564,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct
mdev_device *mdev,
}
  }

-static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
-   .owner  = THIS_MODULE,
-   .supported_type_groups  = mdev_type_groups,
-   .create = vfio_ccw_mdev_create,
-   .remove = vfio_ccw_mdev_remove,
+static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
.open   = vfio_ccw_mdev_open,
.release= vfio_ccw_mdev_release,
.read   = vfio_ccw_mdev_read,
@@ -576,6 +572,14 @@ static const struct mdev_parent_ops
vfio_ccw_mdev_ops = {
.ioctl  = vfio_ccw_mdev_ioctl,
  };

+static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
+   .owner  = THIS_MODULE,
+   .supported_type_groups  = mdev_type_groups,
+   .create = vfio_ccw_mdev_create,
+   .remove = vfio_ccw_mdev_remove,
+   .device_ops = _mdev_ops,
+};
+
  int vfio_ccw_mdev_reg(struct subchannel *sch)
  {
return mdev_register_vfio_device(>dev,
_ccw_mdev_ops);
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index eacbde3c7a97..a48282bff066 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1280,15 +1280,19 @@ static ssize_t vfio_ap_mdev_ioctl(struct
mdev_device *mdev,
return ret;
  }

+static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
+   .open   = vfio_ap_mdev_open,
+   .release= vfio_ap_mdev_release,
+   .ioctl  = vfio_ap_mdev_ioctl,
+};
+
  static const struct mdev_parent_ops vfio_ap_matrix_ops = {
.owner  = THIS_MODULE,
.supported_type_groups  = vfio_ap_mdev_type_groups,
.mdev_attr_groups   = vfio_ap_mdev_attr_groups,
   

RE: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-17 Thread Tian, Kevin
> From: Jason Wang
> Sent: Thursday, September 12, 2019 5:40 PM
> 
> Currently, except for the crate and remove. The rest fields of
> mdev_parent_ops is just designed for vfio-mdev driver and may not help
> for kernel mdev driver. So follow the device id support by previous
> patch, this patch introduces device specific ops which points to
> device specific ops (e.g vfio ops). This allows the future drivers
> like virtio-mdev to implement its own device specific ops.

Can you give an example about what ops might be required to support
kernel mdev driver? I know you posted a link earlier, but putting a small
example here can save time and avoid inconsistent understanding. Then
it will help whether the proposed split makes sense or there is a 
possibility of redefining the callbacks to meet the both requirements.
imo those callbacks fulfill some basic requirements when mediating
a device...

> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
>  drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
>  drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
>  drivers/vfio/mdev/vfio_mdev.c | 30 +++--
>  include/linux/mdev.h  | 72 ++-
>  samples/vfio-mdev/mbochs.c| 16 ---
>  samples/vfio-mdev/mdpy.c  | 16 ---
>  samples/vfio-mdev/mtty.c  | 14 +++---
>  8 files changed, 113 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 19d51a35f019..64823998fd58 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1600,20 +1600,22 @@ 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 struct vfio_mdev_parent_ops intel_vfio_vgpu_ops = {
>   .open   = intel_vgpu_open,
>   .release= intel_vgpu_release,
> -
>   .read   = intel_vgpu_read,
>   .write  = intel_vgpu_write,
>   .mmap   = intel_vgpu_mmap,
>   .ioctl  = intel_vgpu_ioctl,
>  };
> 
> +static struct mdev_parent_ops intel_vgpu_ops = {
> + .mdev_attr_groups   = intel_vgpu_groups,
> + .create = intel_vgpu_create,
> + .remove = intel_vgpu_remove,
> + .device_ops = _vfio_vgpu_ops,
> +};
> +
>  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
>  {
>   struct attribute **kvm_type_attrs;
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c
> b/drivers/s390/cio/vfio_ccw_ops.c
> index f87d9409e290..96e9f18100ae 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -564,11 +564,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct
> mdev_device *mdev,
>   }
>  }
> 
> -static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> - .owner  = THIS_MODULE,
> - .supported_type_groups  = mdev_type_groups,
> - .create = vfio_ccw_mdev_create,
> - .remove = vfio_ccw_mdev_remove,
> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>   .open   = vfio_ccw_mdev_open,
>   .release= vfio_ccw_mdev_release,
>   .read   = vfio_ccw_mdev_read,
> @@ -576,6 +572,14 @@ static const struct mdev_parent_ops
> vfio_ccw_mdev_ops = {
>   .ioctl  = vfio_ccw_mdev_ioctl,
>  };
> 
> +static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> + .owner  = THIS_MODULE,
> + .supported_type_groups  = mdev_type_groups,
> + .create = vfio_ccw_mdev_create,
> + .remove = vfio_ccw_mdev_remove,
> + .device_ops = _mdev_ops,
> +};
> +
>  int vfio_ccw_mdev_reg(struct subchannel *sch)
>  {
>   return mdev_register_vfio_device(>dev,
> _ccw_mdev_ops);
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index eacbde3c7a97..a48282bff066 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1280,15 +1280,19 @@ static ssize_t vfio_ap_mdev_ioctl(struct
> mdev_device *mdev,
>   return ret;
>  }
> 
> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
> + .open   = vfio_ap_mdev_open,
> + .release= vfio_ap_mdev_release,
> + .ioctl  = vfio_ap_mdev_ioctl,
> +};
> +
>  static const struct mdev_parent_ops vfio_ap_matrix_ops = {
>   .owner  = THIS_MODULE,
>   .supported_type_groups  = vfio_ap_mdev_type_groups,
>   .mdev_attr_groups   = vfio_ap_mdev_attr_groups,
>   .create = vfio_ap_mdev_create,
>  

Re: [RFC PATCH 2/2] mdev: introduce device specific ops

2019-09-12 Thread Michael S. Tsirkin
On Thu, Sep 12, 2019 at 05:40:12PM +0800, Jason Wang wrote:
> Currently, except for the crate and remove. The rest fields of


better:

Currently, except for create and remove, the rest of the field in ...


> mdev_parent_ops is just designed for vfio-mdev driver and may not help
> for kernel mdev driver. So follow the device id support by previous
> patch, this patch introduces device specific ops which points to
> device specific ops (e.g vfio ops). This allows the future drivers
> like virtio-mdev to implement its own device specific ops.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c  | 14 +++---
>  drivers/s390/cio/vfio_ccw_ops.c   | 14 +++---
>  drivers/s390/crypto/vfio_ap_ops.c | 10 +++--
>  drivers/vfio/mdev/vfio_mdev.c | 30 +++--
>  include/linux/mdev.h  | 72 ++-
>  samples/vfio-mdev/mbochs.c| 16 ---
>  samples/vfio-mdev/mdpy.c  | 16 ---
>  samples/vfio-mdev/mtty.c  | 14 +++---
>  8 files changed, 113 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 19d51a35f019..64823998fd58 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1600,20 +1600,22 @@ 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 struct vfio_mdev_parent_ops intel_vfio_vgpu_ops = {
>   .open   = intel_vgpu_open,
>   .release= intel_vgpu_release,
> -
>   .read   = intel_vgpu_read,
>   .write  = intel_vgpu_write,
>   .mmap   = intel_vgpu_mmap,
>   .ioctl  = intel_vgpu_ioctl,
>  };
>  
> +static struct mdev_parent_ops intel_vgpu_ops = {
> + .mdev_attr_groups   = intel_vgpu_groups,
> + .create = intel_vgpu_create,
> + .remove = intel_vgpu_remove,
> + .device_ops = _vfio_vgpu_ops,
> +};
> +
>  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
>  {
>   struct attribute **kvm_type_attrs;
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index f87d9409e290..96e9f18100ae 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -564,11 +564,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device 
> *mdev,
>   }
>  }
>  
> -static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> - .owner  = THIS_MODULE,
> - .supported_type_groups  = mdev_type_groups,
> - .create = vfio_ccw_mdev_create,
> - .remove = vfio_ccw_mdev_remove,
> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>   .open   = vfio_ccw_mdev_open,
>   .release= vfio_ccw_mdev_release,
>   .read   = vfio_ccw_mdev_read,
> @@ -576,6 +572,14 @@ static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
>   .ioctl  = vfio_ccw_mdev_ioctl,
>  };
>  
> +static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
> + .owner  = THIS_MODULE,
> + .supported_type_groups  = mdev_type_groups,
> + .create = vfio_ccw_mdev_create,
> + .remove = vfio_ccw_mdev_remove,
> + .device_ops = _mdev_ops,
> +};
> +
>  int vfio_ccw_mdev_reg(struct subchannel *sch)
>  {
>   return mdev_register_vfio_device(>dev, _ccw_mdev_ops);
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
> b/drivers/s390/crypto/vfio_ap_ops.c
> index eacbde3c7a97..a48282bff066 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1280,15 +1280,19 @@ static ssize_t vfio_ap_mdev_ioctl(struct mdev_device 
> *mdev,
>   return ret;
>  }
>  
> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
> + .open   = vfio_ap_mdev_open,
> + .release= vfio_ap_mdev_release,
> + .ioctl  = vfio_ap_mdev_ioctl,
> +};
> +
>  static const struct mdev_parent_ops vfio_ap_matrix_ops = {
>   .owner  = THIS_MODULE,
>   .supported_type_groups  = vfio_ap_mdev_type_groups,
>   .mdev_attr_groups   = vfio_ap_mdev_attr_groups,
>   .create = vfio_ap_mdev_create,
>   .remove = vfio_ap_mdev_remove,
> - .open   = vfio_ap_mdev_open,
> - .release= vfio_ap_mdev_release,
> - .ioctl  = vfio_ap_mdev_ioctl,
> + .device_ops = _mdev_ops,
>  };
>  
>  int vfio_ap_mdev_register(void)
> diff --git a/drivers/vfio/mdev/vfio_mdev.c