Re: [PATCH V3 1/7] mdev: class id support
On 2019/10/16 下午12:57, Parav Pandit wrote:
+static struct mdev_class_id id_table[] = {
static const
+ { MDEV_ID_VFIO },
I guess you don't need extra braces for each entry.
Since this enum represents MDEV class id, it better to name it as
MDEV_CLASS_ID_VFIO.
(Similar to PCI_VENDOR_ID, PCI_DEVICE_ID)..
Gcc start to complain like:
warning: missing braces around initializer [-Wmissing-braces]
static const struct mdev_class_id id_table[] = {
^
MDEV_ID_VFIO, 0,
{ } {
};
}
So I will keep this part untouched.
Thanks
+ { 0 },
+};
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 1/7] mdev: class id support
On 2019/10/16 下午12:57, Parav Pandit wrote:
-Original Message-
From: Jason Wang
Sent: Friday, October 11, 2019 3:16 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: virtualization@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; Jason Wang
Subject: [PATCH V3 1/7] mdev: class id support
Mdev bus only supports vfio driver right now, so it doesn't implement match
method. But in the future, we may add drivers other than vfio, the first
driver could be virtio-mdev. This means we need to add device class id
support in bus match method to pair the mdev device and mdev driver
correctly.
So this patch adds id_table to mdev_driver and class_id for mdev device
with the match method for mdev bus.
Signed-off-by: Jason Wang
---
Documentation/driver-api/vfio-mediated-device.rst | 7 ++-
drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
drivers/s390/cio/vfio_ccw_ops.c | 1 +
drivers/s390/crypto/vfio_ap_ops.c | 1 +
drivers/vfio/mdev/mdev_core.c | 11 +++
drivers/vfio/mdev/mdev_driver.c | 14 ++
drivers/vfio/mdev/mdev_private.h | 1 +
drivers/vfio/mdev/vfio_mdev.c | 6 ++
include/linux/mdev.h | 8
include/linux/mod_devicetable.h | 8
samples/vfio-mdev/mbochs.c| 1 +
samples/vfio-mdev/mdpy.c | 1 +
samples/vfio-mdev/mtty.c | 1 +
13 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..2035e48da7b2 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
* @probe: called when new device created
* @remove: called when device removed
* @driver: device driver structure
+ * @id_table: the ids serviced by this driver
*/
struct mdev_driver {
const char *name;
int (*probe) (struct device *dev);
void (*remove) (struct device *dev);
struct device_driverdriver;
+const struct mdev_class_id *id_table;
};
A mediated bus driver for mdev should use this structure in the function
calls @@ -165,12 +167,15 @@ 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 through::
+
+ extern int mdev_set_class(struct device *dev, u16 id);
Drop extern.
In actual API you have correct signature, i.e. struct mdev_device.
s/struct device/struct mdev_device.
Yes, will fix.
+
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::
extern void mdev_unregister_device(struct device *dev);
-
Mediated Device Management Interface Through sysfs
==
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 343d79c1cb7e..17e9d4634c84 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -678,6 +678,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_ID_VFIO);
out:
return ret;
}
diff --git a/drivers/s390/cio/vfio_ccw_ops.c
b/drivers/s390/cio/vfio_ccw_ops.c index f0d71ab77c50..b5d223882c6c
100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj,
struct mdev_device *mdev)
private->
Re: [PATCH V3 1/7] mdev: class id support
On 2019/10/16 上午12:38, Alex Williamson wrote:
On Fri, 11 Oct 2019 16:15:51 +0800
Jason Wang wrote:
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..724e9b9841d8 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -45,6 +45,12 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
}
EXPORT_SYMBOL(mdev_set_drvdata);
+void mdev_set_class(struct mdev_device *mdev, u16 id)
+{
+ mdev->class_id = id;
+}
+EXPORT_SYMBOL(mdev_set_class);
+
struct device *mdev_dev(struct mdev_device *mdev)
{
return >dev;
@@ -135,6 +141,7 @@ static int mdev_device_remove_cb(struct device *dev, void
*data)
* mdev_register_device : Register a device
* @dev: device structure representing parent device.
* @ops: Parent device operation structure to be registered.
+ * @id: class id.
*
* Add device to list of registered parent devices.
* Returns a negative value on error, otherwise 0.
@@ -324,6 +331,9 @@ int mdev_device_create(struct kobject *kobj,
if (ret)
goto ops_create_fail;
+ if (!mdev->class_id)
This is a sanity test failure of the parent driver on a privileged
path, I think it's fair to print a warning when this occurs rather than
only return an errno to the user. In fact, ret is not set to an error
value here, so it looks like this fails to create the device but
returns success. Thanks,
Alex
Will fix.
Thanks
+ goto class_id_fail;
+
ret = device_add(>dev);
if (ret)
goto add_fail;
@@ -340,6 +350,7 @@ int mdev_device_create(struct kobject *kobj,
sysfs_fail:
device_del(>dev);
+class_id_fail:
add_fail:
parent->ops->remove(mdev);
ops_create_fail:
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 1/7] mdev: class id support
On Fri, 11 Oct 2019 16:15:51 +0800
Jason Wang wrote:
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b558d4cfd082..724e9b9841d8 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -45,6 +45,12 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
> }
> EXPORT_SYMBOL(mdev_set_drvdata);
>
> +void mdev_set_class(struct mdev_device *mdev, u16 id)
> +{
> + mdev->class_id = id;
> +}
> +EXPORT_SYMBOL(mdev_set_class);
> +
> struct device *mdev_dev(struct mdev_device *mdev)
> {
> return >dev;
> @@ -135,6 +141,7 @@ static int mdev_device_remove_cb(struct device *dev, void
> *data)
> * mdev_register_device : Register a device
> * @dev: device structure representing parent device.
> * @ops: Parent device operation structure to be registered.
> + * @id: class id.
> *
> * Add device to list of registered parent devices.
> * Returns a negative value on error, otherwise 0.
> @@ -324,6 +331,9 @@ int mdev_device_create(struct kobject *kobj,
> if (ret)
> goto ops_create_fail;
>
> + if (!mdev->class_id)
This is a sanity test failure of the parent driver on a privileged
path, I think it's fair to print a warning when this occurs rather than
only return an errno to the user. In fact, ret is not set to an error
value here, so it looks like this fails to create the device but
returns success. Thanks,
Alex
> + goto class_id_fail;
> +
> ret = device_add(>dev);
> if (ret)
> goto add_fail;
> @@ -340,6 +350,7 @@ int mdev_device_create(struct kobject *kobj,
>
> sysfs_fail:
> device_del(>dev);
> +class_id_fail:
> add_fail:
> parent->ops->remove(mdev);
> ops_create_fail:
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 1/7] mdev: class id support
On 2019/10/15 下午6:26, Cornelia Huck wrote:
On Fri, 11 Oct 2019 16:15:51 +0800
Jason Wang wrote:
Mdev bus only supports vfio driver right now, so it doesn't implement
match method. But in the future, we may add drivers other than vfio,
the first driver could be virtio-mdev. This means we need to add
device class id support in bus match method to pair the mdev device
and mdev driver correctly.
So this patch adds id_table to mdev_driver and class_id for mdev
device with the match method for mdev bus.
Signed-off-by: Jason Wang
---
Documentation/driver-api/vfio-mediated-device.rst | 7 ++-
drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
drivers/s390/cio/vfio_ccw_ops.c | 1 +
drivers/s390/crypto/vfio_ap_ops.c | 1 +
drivers/vfio/mdev/mdev_core.c | 11 +++
drivers/vfio/mdev/mdev_driver.c | 14 ++
drivers/vfio/mdev/mdev_private.h | 1 +
drivers/vfio/mdev/vfio_mdev.c | 6 ++
include/linux/mdev.h | 8
include/linux/mod_devicetable.h | 8
samples/vfio-mdev/mbochs.c| 1 +
samples/vfio-mdev/mdpy.c | 1 +
samples/vfio-mdev/mtty.c | 1 +
13 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..2035e48da7b2 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
* @probe: called when new device created
* @remove: called when device removed
* @driver: device driver structure
+ * @id_table: the ids serviced by this driver
*/
struct mdev_driver {
const char *name;
int (*probe) (struct device *dev);
void (*remove) (struct device *dev);
struct device_driverdriver;
+const struct mdev_class_id *id_table;
};
A mediated bus driver for mdev should use this structure in the function calls
@@ -165,12 +167,15 @@ 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 through::
+
+ extern int mdev_set_class(struct device *dev, u16 id);
Should the document state explicitly that this should be done in the
->create() callback?
Yes, it's better.
Also, I think that the class_id might be different
for different mdevs (even if the parent is the same) -- should that be
mentioned explicitly?
Yes, depends on the mdev_supported_type.
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::
extern void mdev_unregister_device(struct device *dev);
-
Mediated Device Management Interface Through sysfs
==
(...)
Looks reasonable to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 1/7] mdev: class id support
On Fri, 11 Oct 2019 16:15:51 +0800
Jason Wang wrote:
> Mdev bus only supports vfio driver right now, so it doesn't implement
> match method. But in the future, we may add drivers other than vfio,
> the first driver could be virtio-mdev. This means we need to add
> device class id support in bus match method to pair the mdev device
> and mdev driver correctly.
>
> So this patch adds id_table to mdev_driver and class_id for mdev
> device with the match method for mdev bus.
>
> Signed-off-by: Jason Wang
> ---
> Documentation/driver-api/vfio-mediated-device.rst | 7 ++-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
> drivers/s390/cio/vfio_ccw_ops.c | 1 +
> drivers/s390/crypto/vfio_ap_ops.c | 1 +
> drivers/vfio/mdev/mdev_core.c | 11 +++
> drivers/vfio/mdev/mdev_driver.c | 14 ++
> drivers/vfio/mdev/mdev_private.h | 1 +
> drivers/vfio/mdev/vfio_mdev.c | 6 ++
> include/linux/mdev.h | 8
> include/linux/mod_devicetable.h | 8
> samples/vfio-mdev/mbochs.c| 1 +
> samples/vfio-mdev/mdpy.c | 1 +
> samples/vfio-mdev/mtty.c | 1 +
> 13 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..2035e48da7b2 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
>* @probe: called when new device created
>* @remove: called when device removed
>* @driver: device driver structure
> + * @id_table: the ids serviced by this driver
>*/
> struct mdev_driver {
>const char *name;
>int (*probe) (struct device *dev);
>void (*remove) (struct device *dev);
>struct device_driverdriver;
> + const struct mdev_class_id *id_table;
> };
>
> A mediated bus driver for mdev should use this structure in the function
> calls
> @@ -165,12 +167,15 @@ 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 through::
> +
> + extern int mdev_set_class(struct device *dev, u16 id);
Should the document state explicitly that this should be done in the
->create() callback? Also, I think that the class_id might be different
for different mdevs (even if the parent is the same) -- should that be
mentioned explicitly?
> +
> 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::
>
> extern void mdev_unregister_device(struct device *dev);
>
> -
> Mediated Device Management Interface Through sysfs
> ==
>
(...)
Looks reasonable to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V3 1/7] mdev: class id support
Mdev bus only supports vfio driver right now, so it doesn't implement
match method. But in the future, we may add drivers other than vfio,
the first driver could be virtio-mdev. This means we need to add
device class id support in bus match method to pair the mdev device
and mdev driver correctly.
So this patch adds id_table to mdev_driver and class_id for mdev
device with the match method for mdev bus.
Signed-off-by: Jason Wang
---
Documentation/driver-api/vfio-mediated-device.rst | 7 ++-
drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
drivers/s390/cio/vfio_ccw_ops.c | 1 +
drivers/s390/crypto/vfio_ap_ops.c | 1 +
drivers/vfio/mdev/mdev_core.c | 11 +++
drivers/vfio/mdev/mdev_driver.c | 14 ++
drivers/vfio/mdev/mdev_private.h | 1 +
drivers/vfio/mdev/vfio_mdev.c | 6 ++
include/linux/mdev.h | 8
include/linux/mod_devicetable.h | 8
samples/vfio-mdev/mbochs.c| 1 +
samples/vfio-mdev/mdpy.c | 1 +
samples/vfio-mdev/mtty.c | 1 +
13 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..2035e48da7b2 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
* @probe: called when new device created
* @remove: called when device removed
* @driver: device driver structure
+ * @id_table: the ids serviced by this driver
*/
struct mdev_driver {
const char *name;
int (*probe) (struct device *dev);
void (*remove) (struct device *dev);
struct device_driverdriver;
+const struct mdev_class_id *id_table;
};
A mediated bus driver for mdev should use this structure in the function calls
@@ -165,12 +167,15 @@ 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 through::
+
+ extern int mdev_set_class(struct device *dev, 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::
extern void mdev_unregister_device(struct device *dev);
-
Mediated Device Management Interface Through sysfs
==
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 343d79c1cb7e..17e9d4634c84 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -678,6 +678,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_ID_VFIO);
out:
return ret;
}
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index f0d71ab77c50..b5d223882c6c 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj,
struct mdev_device *mdev)
private->sch->schid.ssid,
private->sch->schid.sch_no);
+ mdev_set_class(mdev, MDEV_ID_VFIO);
return 0;
}
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 5c0f53c6dde7..47df1c593c35 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -343,6 +343,7 @@ static int vfio_ap_mdev_create(struct kobject *kobj, struct
mdev_device *mdev)
list_add(_mdev->node, _dev->mdev_list);
mutex_unlock(_dev->lock);
+ mdev_set_class(mdev, MDEV_ID_VFIO);
return 0;
}
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..724e9b9841d8 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -45,6 +45,12 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
}
EXPORT_SYMBOL(mdev_set_drvdata);
+void mdev_set_class(struct mdev_device *mdev, u16 id)
+{
+ mdev->class_id = id;
+}
+EXPORT_SYMBOL(mdev_set_class);
+
struct device *mdev_dev(struct mdev_device *mdev)
{
return >dev;
@@ -135,6 +141,7 @@ static int mdev_device_remove_cb(struct device *dev, void
*data)
* mdev_register_device : Register a device
* @dev: device structure representing parent device.
* @ops: Parent device operation structure to be registered.
+ * @id: class id.
*
* Add device to list
