On 2019/9/25 上午7:06, Alex Williamson wrote:
On Tue, 24 Sep 2019 21:53:29 +0800 Jason Wang jasowang@redhat.com 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 jasowang@redhat.com
.../driver-api/vfio-mediated-device.rst | 4 +- MAINTAINERS | 1 + drivers/gpu/drm/i915/gvt/kvmgt.c | 17 +++--- drivers/s390/cio/vfio_ccw_ops.c | 17 ++++-- drivers/s390/crypto/vfio_ap_ops.c | 13 +++-- drivers/vfio/mdev/mdev_core.c | 12 +++++ drivers/vfio/mdev/mdev_private.h | 1 + drivers/vfio/mdev/vfio_mdev.c | 37 ++++++------- include/linux/mdev.h | 42 ++++----------- include/linux/vfio_mdev.h | 52 +++++++++++++++++++ samples/vfio-mdev/mbochs.c | 19 ++++--- samples/vfio-mdev/mdpy.c | 19 ++++--- samples/vfio-mdev/mtty.c | 17 ++++-- 13 files changed, 168 insertions(+), 83 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 a5bdc60d62a1..d50425b368bb 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -152,7 +152,9 @@ 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: +The device specific callbacks are referred through device_ops pointer +in mdev_parent_ops. For vfio-mdev device, its callbacks in device_ops +are as follows:
This is not accurate. device_ops is now on the mdev_device and is an mdev bus driver specific structure of callbacks that must be registered for each mdev device by the parent driver during the create callback. There's a one to one mapping of class_id to mdev_device_ops callbacks.
Yes.
That also suggests to me that we could be more clever in registering both of these with mdev-core. Can we embed the class_id in the ops structure in a common way so that the core can extract it and the bus drivers can access their specific callbacks?
That seems much cleaner, let me try to do that in V3.
- open: open callback of mediated device
- close: close callback of mediated device
diff --git a/MAINTAINERS b/MAINTAINERS index b2326dece28e..89832b316500 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17075,6 +17075,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 f793252a3d2a..b274f5ee481f 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -42,6 +42,7 @@ #include <linux/kvm_host.h> #include <linux/vfio.h> #include <linux/mdev.h> +#include <linux/vfio_mdev.h> #include <linux/debugfs.h>
#include <linux/nospec.h> @@ -643,6 +644,8 @@ static void kvmgt_put_vfio_device(void *vgpu) vfio_device_put(((struct intel_vgpu *)vgpu)->vdev.vfio_device); }
+static 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; @@ -679,6 +682,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) ret = 0;
mdev_set_class_id(mdev, MDEV_ID_VFIO);
- mdev_set_dev_ops(mdev, &intel_vfio_vgpu_dev_ops);
This seems rather unrefined. We're registering interdependent data in separate calls. All drivers need to make both of these calls. I'm not sure if this is a good idea, but what if we had:
static const struct vfio_mdev_device_ops intel_vfio_vgpu_dev_ops = { .id = MDEV_ID_VFIO, .open = intel_vgpu_open, .release = intel_vgpu_release, ...
And the set function passed &intel_vfio_vgpu_dev_ops.id and the mdev bus drivers used container_of to get to their callbacks?
Yes, with the check of mdev_device_create() if nothing is set by the device.
out: return ret; } @@ -1601,20 +1605,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 struct vfio_mdev_device_ops intel_vfio_vgpu_dev_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 = {
These could maybe be made const at the same time. Thanks,
Alex
Ok, let me fix.
Thanks