Re: [PATCH RFC 2/3] usb, kcov: collect coverage from hub_event
On Thu, Oct 17, 2019 at 09:06:56PM +0200, Andrey Konovalov wrote: > On Thu, Oct 17, 2019 at 8:19 PM Greg Kroah-Hartman > wrote: > > > > On Thu, Oct 17, 2019 at 07:44:14PM +0200, Andrey Konovalov wrote: > > > This patch adds kcov_remote_start/kcov_remote_stop annotations to the > > > hub_event function, which is responsible for processing events on USB > > > buses, in particular events that happen during USB device enumeration. > > > Each USB bus gets a unique id, which can be used to attach a kcov device > > > to a particular USB bus for coverage collection. > > > > > > Signed-off-by: Andrey Konovalov > > > --- > > > drivers/usb/core/hub.c| 4 > > > include/linux/kcov.h | 1 + > > > include/uapi/linux/kcov.h | 7 +++ > > > 3 files changed, 12 insertions(+) > > > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > > index 236313f41f4a..03a40e41b099 100644 > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -5374,6 +5374,8 @@ static void hub_event(struct work_struct *work) > > > hub_dev = hub->intfdev; > > > intf = to_usb_interface(hub_dev); > > > > > > + kcov_remote_start(kcov_remote_handle_usb(hdev->bus->busnum)); > > > + > > > dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n", > > > hdev->state, hdev->maxchild, > > > /* NOTE: expects max 15 ports... */ > > > @@ -5480,6 +5482,8 @@ static void hub_event(struct work_struct *work) > > > /* Balance the stuff in kick_hub_wq() and allow autosuspend */ > > > usb_autopm_put_interface(intf); > > > kref_put(>kref, hub_release); > > > + > > > + kcov_remote_stop(); > > > } > > > > > > static const struct usb_device_id hub_id_table[] = { > > > diff --git a/include/linux/kcov.h b/include/linux/kcov.h > > > index 702672d98d35..38a47e0b67c2 100644 > > > --- a/include/linux/kcov.h > > > +++ b/include/linux/kcov.h > > > @@ -30,6 +30,7 @@ void kcov_task_exit(struct task_struct *t); > > > /* > > > * Reserved handle ranges: > > > * - : common handles > > > + * 0001 - 0001 : USB subsystem handles > > > > So how many bits are you going to have for any in-kernel tasks? Aren't > > you going to run out quickly? > > With these patches we only collect coverage from hub_event threads, > and we need one ID per USB bus, the number of which is quite limited. > But then we might want to collect coverage from other parts of the USB > subsystem, so we might need more IDs. I don't expect the number of > different subsystem from which we want to collect coverage to be > large, so the idea here is to use 2 bytes of an ID to denote the > subsystem, and the other 6 to denote different coverage collection > sections within it. > > But overall, which encoding scheme to use here is a good question. > Ideas are welcome. > > > > */ > > > void kcov_remote_start(u64 handle); > > > void kcov_remote_stop(void); > > > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h > > > index 46f78f716ca9..45c9ae59cebc 100644 > > > --- a/include/uapi/linux/kcov.h > > > +++ b/include/uapi/linux/kcov.h > > > @@ -43,4 +43,11 @@ enum { > > > #define KCOV_CMP_SIZE(n)((n) << 1) > > > #define KCOV_CMP_MASK KCOV_CMP_SIZE(3) > > > > > > +#define KCOV_REMOTE_HANDLE_USB 0x0001ull > > > + > > > +static inline __u64 kcov_remote_handle_usb(unsigned int bus) > > > +{ > > > + return KCOV_REMOTE_HANDLE_USB + (__u64)bus; > > > +} > > > > Why is this function in a uapi .h file? What userspace code would call > > this? > > A userspace process that wants to collect coverage from USB bus # N > needs to pass kcov_remote_handle_usb(N) into KCOV_REMOTE_ENABLE ioctl. Ugh, ok. Then you should make "unsigned int bus" a __u64 so that this actually will work on all kernels properly. thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC 3/3] vhost, kcov: collect coverage from vhost_worker
On Thu, Oct 17, 2019 at 09:00:18PM +0200, Andrey Konovalov wrote: > On Thu, Oct 17, 2019 at 8:18 PM Greg Kroah-Hartman > wrote: > > > > On Thu, Oct 17, 2019 at 07:44:15PM +0200, Andrey Konovalov wrote: > > > This patch adds kcov_remote_start/kcov_remote_stop annotations to the > > > vhost_worker function, which is responsible for processing vhost works. > > > Since vhost_worker is spawned when a vhost device instance is created, > > > the common kcov handle is used for kcov_remote_start/stop annotations. > > > > > > Signed-off-by: Andrey Konovalov > > > --- > > > drivers/vhost/vhost.c | 15 +++ > > > drivers/vhost/vhost.h | 3 +++ > > > 2 files changed, 18 insertions(+) > > > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > > index 36ca2cf419bf..71a349f6b352 100644 > > > --- a/drivers/vhost/vhost.c > > > +++ b/drivers/vhost/vhost.c > > > @@ -357,7 +357,13 @@ static int vhost_worker(void *data) > > > llist_for_each_entry_safe(work, work_next, node, node) { > > > clear_bit(VHOST_WORK_QUEUED, >flags); > > > __set_current_state(TASK_RUNNING); > > > +#ifdef CONFIG_KCOV > > > + kcov_remote_start(dev->kcov_handle); > > > +#endif > > > > Shouldn't you hide these #ifdefs in a .h file? This is not a "normal" > > kernel coding style at all. > > Well, if it's acceptable to add a kcov_handle field into vhost_dev > even when CONFIG_KCOV is not enabled, then we can get rid of those > #ifdefs. It should be, it's not a big deal and there's not a ton of those structures around that one more field is going to hurt anything... thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC 2/3] usb, kcov: collect coverage from hub_event
On Thu, Oct 17, 2019 at 07:44:14PM +0200, Andrey Konovalov wrote: > This patch adds kcov_remote_start/kcov_remote_stop annotations to the > hub_event function, which is responsible for processing events on USB > buses, in particular events that happen during USB device enumeration. > Each USB bus gets a unique id, which can be used to attach a kcov device > to a particular USB bus for coverage collection. > > Signed-off-by: Andrey Konovalov > --- > drivers/usb/core/hub.c| 4 > include/linux/kcov.h | 1 + > include/uapi/linux/kcov.h | 7 +++ > 3 files changed, 12 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 236313f41f4a..03a40e41b099 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -5374,6 +5374,8 @@ static void hub_event(struct work_struct *work) > hub_dev = hub->intfdev; > intf = to_usb_interface(hub_dev); > > + kcov_remote_start(kcov_remote_handle_usb(hdev->bus->busnum)); > + > dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n", > hdev->state, hdev->maxchild, > /* NOTE: expects max 15 ports... */ > @@ -5480,6 +5482,8 @@ static void hub_event(struct work_struct *work) > /* Balance the stuff in kick_hub_wq() and allow autosuspend */ > usb_autopm_put_interface(intf); > kref_put(>kref, hub_release); > + > + kcov_remote_stop(); > } > > static const struct usb_device_id hub_id_table[] = { > diff --git a/include/linux/kcov.h b/include/linux/kcov.h > index 702672d98d35..38a47e0b67c2 100644 > --- a/include/linux/kcov.h > +++ b/include/linux/kcov.h > @@ -30,6 +30,7 @@ void kcov_task_exit(struct task_struct *t); > /* > * Reserved handle ranges: > * - : common handles > + * 0001 - 0001 : USB subsystem handles So how many bits are you going to have for any in-kernel tasks? Aren't you going to run out quickly? > */ > void kcov_remote_start(u64 handle); > void kcov_remote_stop(void); > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h > index 46f78f716ca9..45c9ae59cebc 100644 > --- a/include/uapi/linux/kcov.h > +++ b/include/uapi/linux/kcov.h > @@ -43,4 +43,11 @@ enum { > #define KCOV_CMP_SIZE(n)((n) << 1) > #define KCOV_CMP_MASK KCOV_CMP_SIZE(3) > > +#define KCOV_REMOTE_HANDLE_USB 0x0001ull > + > +static inline __u64 kcov_remote_handle_usb(unsigned int bus) > +{ > + return KCOV_REMOTE_HANDLE_USB + (__u64)bus; > +} Why is this function in a uapi .h file? What userspace code would call this? thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC 3/3] vhost, kcov: collect coverage from vhost_worker
On Thu, Oct 17, 2019 at 07:44:15PM +0200, Andrey Konovalov wrote: > This patch adds kcov_remote_start/kcov_remote_stop annotations to the > vhost_worker function, which is responsible for processing vhost works. > Since vhost_worker is spawned when a vhost device instance is created, > the common kcov handle is used for kcov_remote_start/stop annotations. > > Signed-off-by: Andrey Konovalov > --- > drivers/vhost/vhost.c | 15 +++ > drivers/vhost/vhost.h | 3 +++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 36ca2cf419bf..71a349f6b352 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -357,7 +357,13 @@ static int vhost_worker(void *data) > llist_for_each_entry_safe(work, work_next, node, node) { > clear_bit(VHOST_WORK_QUEUED, >flags); > __set_current_state(TASK_RUNNING); > +#ifdef CONFIG_KCOV > + kcov_remote_start(dev->kcov_handle); > +#endif Shouldn't you hide these #ifdefs in a .h file? This is not a "normal" kernel coding style at all. > work->fn(work); > +#ifdef CONFIG_KCOV > + kcov_remote_stop(); > +#endif > if (need_resched()) > schedule(); > } > @@ -546,6 +552,9 @@ long vhost_dev_set_owner(struct vhost_dev *dev) > > /* No owner, become one */ > dev->mm = get_task_mm(current); > +#ifdef CONFIG_KCOV > + dev->kcov_handle = current->kcov_handle; > +#endif > worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid); > if (IS_ERR(worker)) { > err = PTR_ERR(worker); > @@ -571,6 +580,9 @@ long vhost_dev_set_owner(struct vhost_dev *dev) > if (dev->mm) > mmput(dev->mm); > dev->mm = NULL; > +#ifdef CONFIG_KCOV > + dev->kcov_handle = 0; > +#endif > err_mm: > return err; > } > @@ -682,6 +694,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev) > if (dev->worker) { > kthread_stop(dev->worker); > dev->worker = NULL; > +#ifdef CONFIG_KCOV > + dev->kcov_handle = 0; > +#endif > } > if (dev->mm) > mmput(dev->mm); > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index e9ed2722b633..010ca1ebcbd5 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -173,6 +173,9 @@ struct vhost_dev { > int iov_limit; > int weight; > int byte_weight; > +#ifdef CONFIG_KCOV > + u64 kcov_handle; > +#endif Why is this a #ifdef at all here? thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops
On Thu, 17 Oct 2019 18:48:34 +0800 Jason Wang wrote: > This patch implements basic support for mdev driver that supports > virtio transport for kernel virtio driver. > > Signed-off-by: Jason Wang > --- > drivers/vfio/mdev/mdev_core.c | 12 +++ > include/linux/mdev.h | 4 + > include/linux/virtio_mdev.h | 151 ++ > 3 files changed, 167 insertions(+) > create mode 100644 include/linux/virtio_mdev.h > > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > index d0f3113c8071..5834f6b7c7a5 100644 > --- a/drivers/vfio/mdev/mdev_core.c > +++ b/drivers/vfio/mdev/mdev_core.c > @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, > } > EXPORT_SYMBOL(mdev_set_vfio_ops); > > +/* Specify the virtio device ops for the mdev device, this > + * must be called during create() callback for virtio mdev device. > + */ > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops) > +{ > + BUG_ON(mdev->class_id); Nit, this one is a BUG_ON, but the vfio one is a WARN_ON. Thanks, Alex > + mdev->class_id = MDEV_CLASS_ID_VIRTIO; > + mdev->device_ops = virtio_ops; > +} > +EXPORT_SYMBOL(mdev_set_virtio_ops); > + > const void *mdev_get_dev_ops(struct mdev_device *mdev) > { > return mdev->device_ops; > diff --git a/include/linux/mdev.h b/include/linux/mdev.h > index 3d29e09e20c9..13e045e09d3b 100644 > --- a/include/linux/mdev.h > +++ b/include/linux/mdev.h > @@ -17,6 +17,7 @@ > > struct mdev_device; > struct vfio_mdev_device_ops; > +struct virtio_mdev_device_ops; > > /* > * Called by the parent device driver to set the device which represents > @@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void > *data); > const guid_t *mdev_uuid(struct mdev_device *mdev); > void mdev_set_vfio_ops(struct mdev_device *mdev, > const struct vfio_mdev_device_ops *vfio_ops); > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops); > const void *mdev_get_dev_ops(struct mdev_device *mdev); > > extern struct bus_type mdev_bus_type; > @@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev); > > enum { > MDEV_CLASS_ID_VFIO = 1, > + MDEV_CLASS_ID_VIRTIO = 2, > /* New entries must be added here */ > }; > > diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h > new file mode 100644 > index ..b965b50f9b24 > --- /dev/null > +++ b/include/linux/virtio_mdev.h > @@ -0,0 +1,151 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Virtio mediated device driver > + * > + * Copyright 2019, Red Hat Corp. > + * Author: Jason Wang > + */ > +#ifndef _LINUX_VIRTIO_MDEV_H > +#define _LINUX_VIRTIO_MDEV_H > + > +#include > +#include > +#include > + > +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev" > +#define VIRTIO_MDEV_F_VERSION_1 0x1 > + > +struct virtio_mdev_callback { > + irqreturn_t (*callback)(void *data); > + void *private; > +}; > + > +/** > + * struct vfio_mdev_device_ops - Structure to be registered for each > + * mdev device to register the device to virtio-mdev module. > + * > + * @set_vq_address: Set the address of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @desc_area: address of desc area > + * @driver_area: address of driver area > + * @device_area: address of device area > + * Returns integer: success (0) or error (< 0) > + * @set_vq_num: Set the size of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @num: the size of virtqueue > + * @kick_vq: Kick the virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @set_vq_cb: Set the interrupt callback function for > + * a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @cb: virtio-mdev interrupt callback structure > + * @set_vq_ready:Set ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @ready: ready (true) not ready(false) > + * @get_vq_ready:Get ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns boolean: ready (true) or not (false) > + * @set_vq_state:Set the state for a
Re: [PATCH V4 3/6] mdev: introduce device specific ops
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: [PATCH V4 3/6] mdev: introduce device specific ops
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); > -
[PATCH 4/5] drm/qxl: use DEFINE_DRM_GEM_FOPS()
We have no qxl-specific fops any more. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_drv.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 65464630ac98..1d601f57a6ba 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -150,15 +150,7 @@ qxl_pci_remove(struct pci_dev *pdev) drm_dev_put(dev); } -static const struct file_operations qxl_fops = { - .owner = THIS_MODULE, - .open = drm_open, - .release = drm_release, - .unlocked_ioctl = drm_ioctl, - .poll = drm_poll, - .read = drm_read, - .mmap = drm_gem_mmap, -}; +DEFINE_DRM_GEM_FOPS(qxl_fops); static int qxl_drm_freeze(struct drm_device *dev) { -- 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 5/5] drm/qxl: allocate small objects top-down
qxl uses small buffer objects for qxl commands. Allocate them top-down to reduce fragmentation. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_object.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index 927ab917b834..ad336c98a0cf 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -54,9 +54,14 @@ bool qxl_ttm_bo_is_qxl_bo(struct ttm_buffer_object *bo) void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned) { u32 c = 0; - u32 pflag = pinned ? TTM_PL_FLAG_NO_EVICT : 0; + u32 pflag = 0; unsigned int i; + if (pinned) + pflag |= TTM_PL_FLAG_NO_EVICT; + if (qbo->tbo.base.size <= PAGE_SIZE) + pflag |= TTM_PL_FLAG_TOPDOWN; + qbo->placement.placement = qbo->placements; qbo->placement.busy_placement = qbo->placements; if (domain == QXL_GEM_DOMAIN_VRAM) -- 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/5] drm/qxl: drop qxl_ttm_fault
Not sure what this hook is supposed to do. vmf->vma->vm_private_data should never be NULL, so the extra check in qxl_ttm_fault should have no effect. Drop it. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_ttm.c | 27 +-- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index cbc6c2ba8630..dba925589e17 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -48,24 +48,8 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device *bdev) return qdev; } -static struct vm_operations_struct qxl_ttm_vm_ops; -static const struct vm_operations_struct *ttm_vm_ops; - -static vm_fault_t qxl_ttm_fault(struct vm_fault *vmf) -{ - struct ttm_buffer_object *bo; - vm_fault_t ret; - - bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data; - if (bo == NULL) - return VM_FAULT_NOPAGE; - ret = ttm_vm_ops->fault(vmf); - return ret; -} - int qxl_mmap(struct file *filp, struct vm_area_struct *vma) { - int r; struct drm_file *file_priv = filp->private_data; struct qxl_device *qdev = file_priv->minor->dev->dev_private; @@ -77,16 +61,7 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma) DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n", filp->private_data, vma->vm_pgoff); - r = ttm_bo_mmap(filp, vma, >mman.bdev); - if (unlikely(r != 0)) - return r; - if (unlikely(ttm_vm_ops == NULL)) { - ttm_vm_ops = vma->vm_ops; - qxl_ttm_vm_ops = *ttm_vm_ops; - qxl_ttm_vm_ops.fault = _ttm_fault; - } - vma->vm_ops = _ttm_vm_ops; - return 0; + return ttm_bo_mmap(filp, vma, >mman.bdev); } static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags) -- 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/5] drm/qxl: switch qxl to _gem_object_funcs.mmap
Wire up the new drm_gem_ttm_mmap() helper function. Use generic drm_gem_mmap() and remove qxl_mmap(). Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_drv.h| 1 - drivers/gpu/drm/qxl/qxl_drv.c| 2 +- drivers/gpu/drm/qxl/qxl_object.c | 1 + drivers/gpu/drm/qxl/qxl_ttm.c| 16 4 files changed, 2 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index d4051409ce64..a5cb3864d686 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -355,7 +355,6 @@ int qxl_mode_dumb_mmap(struct drm_file *filp, /* qxl ttm */ int qxl_ttm_init(struct qxl_device *qdev); void qxl_ttm_fini(struct qxl_device *qdev); -int qxl_mmap(struct file *filp, struct vm_area_struct *vma); /* qxl image */ diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 483b4c57554a..65464630ac98 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -157,7 +157,7 @@ static const struct file_operations qxl_fops = { .unlocked_ioctl = drm_ioctl, .poll = drm_poll, .read = drm_read, - .mmap = qxl_mmap, + .mmap = drm_gem_mmap, }; static int qxl_drm_freeze(struct drm_device *dev) diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index c013c516f561..927ab917b834 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = { .get_sg_table = qxl_gem_prime_get_sg_table, .vmap = qxl_gem_prime_vmap, .vunmap = qxl_gem_prime_vunmap, + .mmap = drm_gem_ttm_mmap, .print_info = drm_gem_ttm_print_info, }; diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index dba925589e17..629ac8e77a21 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -48,22 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device *bdev) return qdev; } -int qxl_mmap(struct file *filp, struct vm_area_struct *vma) -{ - struct drm_file *file_priv = filp->private_data; - struct qxl_device *qdev = file_priv->minor->dev->dev_private; - - if (qdev == NULL) { - DRM_ERROR( -"filp->private_data->minor->dev->dev_private == NULL\n"); - return -EINVAL; - } - DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n", - filp->private_data, vma->vm_pgoff); - - return ttm_bo_mmap(filp, vma, >mman.bdev); -} - static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags) { return 0; -- 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 3/5] drm/qxl: drop verify_access
Not needed any more. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_ttm.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 629ac8e77a21..54cc5a5b607e 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -110,14 +110,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo, *placement = qbo->placement; } -static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp) -{ - struct qxl_bo *qbo = to_qxl_bo(bo); - - return drm_vma_node_verify_access(>tbo.base.vma_node, - filp->private_data); -} - static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) { @@ -269,7 +261,6 @@ static struct ttm_bo_driver qxl_bo_driver = { .eviction_valuable = ttm_bo_eviction_valuable, .evict_flags = _evict_flags, .move = _bo_move, - .verify_access = _verify_access, .io_mem_reserve = _ttm_io_mem_reserve, .io_mem_free = _ttm_io_mem_free, .move_notify = _bo_move_notify, -- 2.18.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH net 1/2] vsock/virtio: send a credit update when buffer size is changed
When the user application set a new buffer size value, we should update the remote peer about this change, since it uses this information to calculate the credit available. Signed-off-by: Stefano Garzarella --- net/vmw_vsock/virtio_transport_common.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index a666ef8fc54e..db127a69f5c3 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -458,6 +458,9 @@ void virtio_transport_set_buffer_size(struct vsock_sock *vsk, u64 val) vvs->buf_size_max = val; vvs->buf_size = val; vvs->buf_alloc = val; + + virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM, + NULL); } EXPORT_SYMBOL_GPL(virtio_transport_set_buffer_size); -- 2.21.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH net 2/2] vsock/virtio: discard packets if credit is not respected
If the remote peer doesn't respect the credit information (buf_alloc, fwd_cnt), sending more data than it can send, we should drop the packets to prevent a malicious peer from using all of our memory. This is patch follows the VIRTIO spec: "VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has sufficient free buffer space for the payload" Signed-off-by: Stefano Garzarella --- net/vmw_vsock/virtio_transport_common.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index db127a69f5c3..481f7f8a1655 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -204,10 +204,14 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, return virtio_transport_get_ops()->send_pkt(pkt); } -static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, +static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) { + if (vvs->rx_bytes + pkt->len > vvs->buf_alloc) + return false; + vvs->rx_bytes += pkt->len; + return true; } static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, @@ -879,14 +883,18 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, struct virtio_vsock_pkt *pkt) { struct virtio_vsock_sock *vvs = vsk->trans; - bool free_pkt = false; + bool can_enqueue, free_pkt = false; pkt->len = le32_to_cpu(pkt->hdr.len); pkt->off = 0; spin_lock_bh(>rx_lock); - virtio_transport_inc_rx_pkt(vvs, pkt); + can_enqueue = virtio_transport_inc_rx_pkt(vvs, pkt); + if (!can_enqueue) { + free_pkt = true; + goto out; + } /* Try to copy small packets into the buffer of last packet queued, * to avoid wasting memory queueing the entire buffer with a small -- 2.21.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH net 0/2] vsock/virtio: make the credit mechanism more robust
This series makes the credit mechanism implemented in the virtio-vsock devices more robust. Patch 1 sends an update to the remote peer when the buf_alloc change. Patch 2 prevents a malicious peer (especially the guest) can consume all the memory of the other peer, discarding packets when the credit available is not respected. Stefano Garzarella (2): vsock/virtio: send a credit update when buffer size is changed vsock/virtio: discard packets if credit is not respected net/vmw_vsock/virtio_transport_common.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) -- 2.21.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH -next] drm/qxl: Fix randbuild error
On Tue, Oct 08, 2019 at 10:40:54AM +0800, YueHaibing wrote: > If DEM_QXL is y and DRM_TTM_HELPER is m, building fails: > > drivers/gpu/drm/qxl/qxl_object.o: undefined reference to > `drm_gem_ttm_print_info' > > Select DRM_TTM_HELPER to fix this. Queued up for drm-misc-next. thanks, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V4 6/6] docs: sample driver to demonstrate how to implement virtio-mdev framework
This sample driver creates mdev device that simulate virtio net device over virtio mdev transport. The device is implemented through vringh and workqueue. A device specific dma ops is to make sure HVA is used directly as the IOVA. This should be sufficient for kernel virtio driver to work. Only 'virtio' type is supported right now. I plan to add 'vhost' type on top which requires some virtual IOMMU implemented in this sample driver. Signed-off-by: Jason Wang --- MAINTAINERS| 1 + samples/Kconfig| 7 + samples/vfio-mdev/Makefile | 1 + samples/vfio-mdev/mvnet.c | 691 + 4 files changed, 700 insertions(+) create mode 100644 samples/vfio-mdev/mvnet.c diff --git a/MAINTAINERS b/MAINTAINERS index 3d196a023b5e..cb51351cd5c9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17254,6 +17254,7 @@ F: include/linux/virtio*.h F: include/uapi/linux/virtio_*.h F: drivers/crypto/virtio/ F: mm/balloon_compaction.c +F: samples/vfio-mdev/mvnet.c VIRTIO BLOCK AND SCSI DRIVERS M: "Michael S. Tsirkin" diff --git a/samples/Kconfig b/samples/Kconfig index c8dacb4dda80..a1a1ca2c00b7 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -131,6 +131,13 @@ config SAMPLE_VFIO_MDEV_MDPY mediated device. It is a simple framebuffer and supports the region display interface (VFIO_GFX_PLANE_TYPE_REGION). +config SAMPLE_VIRTIO_MDEV_NET +tristate "Build virtio mdev net example mediated device sample code -- loadable modules only" + depends on VIRTIO_MDEV_DEVICE && VHOST_RING && m + help + Build a networking sample device for use as a virtio + mediated device. + config SAMPLE_VFIO_MDEV_MDPY_FB tristate "Build VFIO mdpy example guest fbdev driver -- loadable module only" depends on FB && m diff --git a/samples/vfio-mdev/Makefile b/samples/vfio-mdev/Makefile index 10d179c4fdeb..f34af90ed0a0 100644 --- a/samples/vfio-mdev/Makefile +++ b/samples/vfio-mdev/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_SAMPLE_VFIO_MDEV_MTTY) += mtty.o obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY) += mdpy.o obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY_FB) += mdpy-fb.o obj-$(CONFIG_SAMPLE_VFIO_MDEV_MBOCHS) += mbochs.o +obj-$(CONFIG_SAMPLE_VIRTIO_MDEV_NET) += mvnet.o diff --git a/samples/vfio-mdev/mvnet.c b/samples/vfio-mdev/mvnet.c new file mode 100644 index ..19e823a449cd --- /dev/null +++ b/samples/vfio-mdev/mvnet.c @@ -0,0 +1,691 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Mediated virtual virtio-net device driver. + * + * Copyright (c) 2019, Red Hat Inc. All rights reserved. + * Author: Jason Wang + * + * Sample driver that creates mdev device that simulates ethernet loopback + * device. + * + * Usage: + * + * # modprobe virtio_mdev + * # modprobe mvnet + * # cd /sys/devices/virtual/mvnet/mvnet/mdev_supported_types/mvnet-virtio + * # echo "83b8f4f2-509f-382f-3c1e-e6bfe0fa1001" > ./create + * # cd devices/83b8f4f2-509f-382f-3c1e-e6bfe0fa1001 + * # ls -d virtio0 + * virtio0 + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Red Hat Corporation" + +#define MVNET_CLASS_NAME "mvnet" +#define MVNET_NAME "mvnet" + +/* + * Global Structures + */ + +static struct mvnet_dev { + struct class*vd_class; + struct idr vd_idr; + struct device dev; +} mvnet_dev; + +struct mvnet_virtqueue { + struct vringh vring; + struct vringh_kiov iov; + unsigned short head; + bool ready; + u64 desc_addr; + u64 device_addr; + u64 driver_addr; + u32 num; + void *private; + irqreturn_t (*cb)(void *data); +}; + +#define MVNET_QUEUE_ALIGN PAGE_SIZE +#define MVNET_QUEUE_MAX 256 +#define MVNET_DEVICE_ID 0x1 +#define MVNET_VENDOR_ID 0 + +u64 mvnet_features = (1ULL << VIRTIO_F_ANY_LAYOUT) | +(1ULL << VIRTIO_F_VERSION_1) | +(1ULL << VIRTIO_F_IOMMU_PLATFORM); + +/* State of each mdev device */ +struct mvnet_state { + struct mvnet_virtqueue vqs[2]; + struct work_struct work; + spinlock_t lock; + struct mdev_device *mdev; + struct virtio_net_config config; + void *buffer; + u32 status; + u32 generation; + u64 features; + struct list_head next; +}; + +static struct mutex mdev_list_lock; +static struct list_head mdev_devices_list; + +static void mvnet_queue_ready(struct mvnet_state *mvnet, unsigned int idx) +{ + struct mvnet_virtqueue *vq = >vqs[idx]; + int ret; + + ret = vringh_init_kern(>vring, mvnet_features, MVNET_QUEUE_MAX, + false, (struct vring_desc *)vq->desc_addr, + (struct vring_avail *)vq->driver_addr, +
[PATCH V4 5/6] virtio: introduce a mdev based transport
This patch introduces a new mdev transport for virtio. This is used to use kernel virtio driver to drive the mediated device that is capable of populating virtqueue directly. A new virtio-mdev driver will be registered to the mdev bus, when a new virtio-mdev device is probed, it will register the device with mdev based config ops. This means it is a software transport between mdev driver and mdev device. The transport was implemented through device specific ops which is a part of mdev_parent_ops now. Signed-off-by: Jason Wang --- drivers/virtio/Kconfig | 7 + drivers/virtio/Makefile | 1 + drivers/virtio/virtio_mdev.c | 409 +++ 3 files changed, 417 insertions(+) create mode 100644 drivers/virtio/virtio_mdev.c diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 078615cf2afc..8d18722ab572 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -43,6 +43,13 @@ config VIRTIO_PCI_LEGACY If unsure, say Y. +config VIRTIO_MDEV_DEVICE + tristate "VIRTIO driver for Mediated devices" + depends on VFIO_MDEV && VIRTIO + default n + help + VIRTIO based driver for Mediated devices. + config VIRTIO_PMEM tristate "Support for virtio pmem driver" depends on VIRTIO diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index 3a2b5c5dcf46..ebc7fa15ae82 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -6,3 +6,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o +obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o diff --git a/drivers/virtio/virtio_mdev.c b/drivers/virtio/virtio_mdev.c new file mode 100644 index ..24c0df5ffe77 --- /dev/null +++ b/drivers/virtio/virtio_mdev.c @@ -0,0 +1,409 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * VIRTIO based driver for Mediated device + * + * Copyright (c) 2019, Red Hat. All rights reserved. + * Author: Jason Wang + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_VERSION "0.1" +#define DRIVER_AUTHOR "Red Hat Corporation" +#define DRIVER_DESC "VIRTIO based driver for Mediated device" + +#define to_virtio_mdev_device(dev) \ + container_of(dev, struct virtio_mdev_device, vdev) + +struct virtio_mdev_device { + struct virtio_device vdev; + struct mdev_device *mdev; + unsigned long version; + + /* The lock to protect virtqueue list */ + spinlock_t lock; + /* List of virtio_mdev_vq_info */ + struct list_head virtqueues; +}; + +struct virtio_mdev_vq_info { + /* the actual virtqueue */ + struct virtqueue *vq; + + /* the list node for the virtqueues list */ + struct list_head node; +}; + +static struct mdev_device *vm_get_mdev(struct virtio_device *vdev) +{ + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev); + struct mdev_device *mdev = vm_dev->mdev; + + return mdev; +} + +static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset, + void *buf, unsigned len) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + ops->get_config(mdev, offset, buf, len); +} + +static void virtio_mdev_set(struct virtio_device *vdev, unsigned offset, + const void *buf, unsigned len) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + ops->set_config(mdev, offset, buf, len); +} + +static u32 virtio_mdev_generation(struct virtio_device *vdev) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + return ops->get_generation(mdev); +} + +static u8 virtio_mdev_get_status(struct virtio_device *vdev) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + return ops->get_status(mdev); +} + +static void virtio_mdev_set_status(struct virtio_device *vdev, u8 status) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + return ops->set_status(mdev, status); +} + +static void virtio_mdev_reset(struct virtio_device *vdev) +{ + struct mdev_device *mdev = vm_get_mdev(vdev); + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev); + + return ops->set_status(mdev, 0); +} + +static bool virtio_mdev_notify(struct virtqueue *vq) +{ + struct mdev_device *mdev = vm_get_mdev(vq->vdev); + const struct virtio_mdev_device_ops *ops =
[PATCH V4 4/6] mdev: introduce virtio device and its device ops
This patch implements basic support for mdev driver that supports virtio transport for kernel virtio driver. Signed-off-by: Jason Wang --- drivers/vfio/mdev/mdev_core.c | 12 +++ include/linux/mdev.h | 4 + include/linux/virtio_mdev.h | 151 ++ 3 files changed, 167 insertions(+) create mode 100644 include/linux/virtio_mdev.h diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index d0f3113c8071..5834f6b7c7a5 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, } EXPORT_SYMBOL(mdev_set_vfio_ops); +/* Specify the virtio device ops for the mdev device, this + * must be called during create() callback for virtio mdev device. + */ +void mdev_set_virtio_ops(struct mdev_device *mdev, +const struct virtio_mdev_device_ops *virtio_ops) +{ + BUG_ON(mdev->class_id); + mdev->class_id = MDEV_CLASS_ID_VIRTIO; + mdev->device_ops = virtio_ops; +} +EXPORT_SYMBOL(mdev_set_virtio_ops); + const void *mdev_get_dev_ops(struct mdev_device *mdev) { return mdev->device_ops; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 3d29e09e20c9..13e045e09d3b 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -17,6 +17,7 @@ struct mdev_device; struct vfio_mdev_device_ops; +struct virtio_mdev_device_ops; /* * Called by the parent device driver to set the device which represents @@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data); const guid_t *mdev_uuid(struct mdev_device *mdev); void mdev_set_vfio_ops(struct mdev_device *mdev, const struct vfio_mdev_device_ops *vfio_ops); +void mdev_set_virtio_ops(struct mdev_device *mdev, + const struct virtio_mdev_device_ops *virtio_ops); const void *mdev_get_dev_ops(struct mdev_device *mdev); extern struct bus_type mdev_bus_type; @@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev); enum { MDEV_CLASS_ID_VFIO = 1, + MDEV_CLASS_ID_VIRTIO = 2, /* New entries must be added here */ }; diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h new file mode 100644 index ..b965b50f9b24 --- /dev/null +++ b/include/linux/virtio_mdev.h @@ -0,0 +1,151 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Virtio mediated device driver + * + * Copyright 2019, Red Hat Corp. + * Author: Jason Wang + */ +#ifndef _LINUX_VIRTIO_MDEV_H +#define _LINUX_VIRTIO_MDEV_H + +#include +#include +#include + +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev" +#define VIRTIO_MDEV_F_VERSION_1 0x1 + +struct virtio_mdev_callback { + irqreturn_t (*callback)(void *data); + void *private; +}; + +/** + * struct vfio_mdev_device_ops - Structure to be registered for each + * mdev device to register the device to virtio-mdev module. + * + * @set_vq_address:Set the address of virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @desc_area: address of desc area + * @driver_area: address of driver area + * @device_area: address of device area + * Returns integer: success (0) or error (< 0) + * @set_vq_num:Set the size of virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @num: the size of virtqueue + * @kick_vq: Kick the virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @set_vq_cb: Set the interrupt callback function for + * a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @cb: virtio-mdev interrupt callback structure + * @set_vq_ready: Set ready status for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @ready: ready (true) not ready(false) + * @get_vq_ready: Get ready status for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * Returns boolean: ready (true) or not (false) + * @set_vq_state: Set the state for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @state: virtqueue state (last_avail_idx) + * Returns integer: success (0) or error (< 0) + * @get_vq_state: Get the state for a
[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/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, _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
[PATCH V4 2/6] modpost: add support for mdev class id
Add support to parse mdev class id table. Signed-off-by: Jason Wang --- drivers/vfio/mdev/vfio_mdev.c | 2 ++ scripts/mod/devicetable-offsets.c | 3 +++ scripts/mod/file2alias.c | 10 ++ 3 files changed, 15 insertions(+) diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c index 7b24ee9cb8dd..cb701cd646f0 100644 --- a/drivers/vfio/mdev/vfio_mdev.c +++ b/drivers/vfio/mdev/vfio_mdev.c @@ -125,6 +125,8 @@ static const struct mdev_class_id id_table[] = { { 0 }, }; +MODULE_DEVICE_TABLE(mdev, id_table); + static struct mdev_driver vfio_mdev_driver = { .name = "vfio_mdev", .probe = vfio_mdev_probe, diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index 054405b90ba4..6cbb1062488a 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -231,5 +231,8 @@ int main(void) DEVID(wmi_device_id); DEVID_FIELD(wmi_device_id, guid_string); + DEVID(mdev_class_id); + DEVID_FIELD(mdev_class_id, id); + return 0; } diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index c91eba751804..d365dfe7c718 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1335,6 +1335,15 @@ static int do_wmi_entry(const char *filename, void *symval, char *alias) return 1; } +/* looks like: "mdev:cN" */ +static int do_mdev_entry(const char *filename, void *symval, char *alias) +{ + DEF_FIELD(symval, mdev_class_id, id); + + sprintf(alias, "mdev:c%02X", id); + return 1; +} + /* Does namelen bytes of name exactly match the symbol? */ static bool sym_is(const char *name, unsigned namelen, const char *symbol) { @@ -1407,6 +1416,7 @@ static const struct devtable devtable[] = { {"typec", SIZE_typec_device_id, do_typec_entry}, {"tee", SIZE_tee_client_device_id, do_tee_entry}, {"wmi", SIZE_wmi_device_id, do_wmi_entry}, + {"mdev", SIZE_mdev_class_id, do_mdev_entry}, }; /* Create MODULE_ALIAS() statements. -- 2.19.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V4 1/6] 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 --- .../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 | 18 +++ drivers/vfio/mdev/mdev_driver.c | 22 +++ 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, 75 insertions(+), 1 deletion(-) diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 25eb7d5b834b..f9a78d75a67a 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 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:: 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..6420f0dbd31b 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_CLASS_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..cf2c013ae32f 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_CLASS_ID_VFIO); return 0; } diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 5c0f53c6dde7..07c31070afeb 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_CLASS_ID_VFIO); return 0; } diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index b558d4cfd082..3a9c52d71b4e 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -45,6 +45,16 @@ 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. + */ +void mdev_set_class(struct mdev_device *mdev, u16 id) +{ + WARN_ON(mdev->class_id); + mdev->class_id = id; +} +EXPORT_SYMBOL(mdev_set_class); + struct device *mdev_dev(struct mdev_device *mdev) { return >dev; @@ -135,6 +145,7 @@ static int mdev_device_remove_cb(struct device *dev, void *data) * mdev_register_device : Register a device * @dev: device structure representing
[PATCH V4 0/6] mdev based hardware virtio offloading support
Hi all: There are hardwares that can do virtio datapath offloading while having its own control path. This path tries to implement a mdev based unified API to support using kernel virtio driver to drive those devices. This is done by introducing a new mdev transport for virtio (virtio_mdev) and register itself as a new kind of mdev driver. Then it provides a unified way for kernel virtio driver to talk with mdev device implementation. Though the series only contains kernel driver support, the goal is to make the transport generic enough to support userspace drivers. This means vhost-mdev[1] could be built on top as well by resuing the transport. A sample driver is also implemented which simulate a virito-net loopback ethernet device on top of vringh + workqueue. This could be used as a reference implementation for real hardware driver. Also a real ICF VF driver was also posted here[2] which is a good reference for vendors who is interested in their own virtio datapath offloading product. Consider mdev framework only support VFIO device and driver right now, this series also extend it to support other types. This is done through introducing class id to the device and pairing it with id_talbe claimed by the driver. On top, this seris also decouple device specific parents ops out of the common ones. Pktgen test was done with virito-net + mvnet loop back device. Please review. [1] https://lkml.org/lkml/2019/9/26/15 [2] https://lkml.org/lkml/2019/10/15/1226 Changes from V3: - document that class id (device ops) must be specified in create() - add WARN() when trying to set class_id when it has already set - add WARN() when class_id is not specified in create() and correctly return an error in this case - correct the prototype of mdev_set_class() in the doc - add documention of mdev_set_class() - remove the unnecessary "class_id_fail" label when class id is not specified in create() - convert id_table in vfio_mdev to const - move mdev_set_class and its friends after mdev_uuid() - suqash the patch of bus uevent into patch of introducing class id - tweak the words in the docs per Cornelia suggestion - tie class_id and device ops through class specific initialization routine like mdev_set_vfio_ops() - typos fixes in the docs of virtio-mdev callbacks - document the usage of virtqueues in struct virtio_mdev_device - remove the useless vqs array in struct virtio_mdev_device - rename MDEV_ID_XXX to MDEV_CLASS_ID_XXX Changes from V2: - fail when class_id is not specified - drop the vringh patch - match the doc to the code - tweak the commit log - move device_ops from parent to mdev device - remove the unused MDEV_ID_VHOST Changes from V1: - move virtio_mdev.c to drivers/virtio - store class_id in mdev_device instead of mdev_parent - store device_ops in mdev_device instead of mdev_parent - reorder the patch, vringh fix comes first - really silent compiling warnings - really switch to use u16 for class_id - uevent and modpost support for mdev class_id - vraious tweaks per comments from Parav Changes from RFC-V2: - silent compile warnings on some specific configuration - use u16 instead u8 for class id - reseve MDEV_ID_VHOST for future vhost-mdev work - introduce "virtio" type for mvnet and make "vhost" type for future work - add entries in MAINTAINER - tweak and typos fixes in commit log Changes from RFC-V1: - rename device id to class id - add docs for class id and device specific ops (device_ops) - split device_ops into seperate headers - drop the mdev_set_dma_ops() - use device_ops to implement the transport API, then it's not a part of UAPI any more - use GFP_ATOMIC in mvnet sample device and other tweaks - set_vring_base/get_vring_base support for mvnet device Jason Wang (6): mdev: class id support modpost: add support for mdev class id mdev: introduce device specific ops mdev: introduce virtio device and its device ops virtio: introduce a mdev based transport docs: sample driver to demonstrate how to implement virtio-mdev framework .../driver-api/vfio-mediated-device.rst | 22 +- MAINTAINERS | 2 + 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 | 38 + drivers/vfio/mdev/mdev_driver.c | 22 + drivers/vfio/mdev/mdev_private.h | 2 + drivers/vfio/mdev/vfio_mdev.c | 45 +- drivers/virtio/Kconfig| 7 + drivers/virtio/Makefile | 1 + drivers/virtio/virtio_mdev.c | 409 +++ include/linux/mdev.h | 55 +- include/linux/mod_devicetable.h | 8 + include/linux/vfio_mdev.h | 52 ++ include/linux/virtio_mdev.h | 151 samples/Kconfig | 7 +
Re: [PATCH V3 0/7] mdev based hardware virtio offloading support
On Thu, Oct 17, 2019 at 09:42:53AM +0800, Jason Wang wrote: > > On 2019/10/15 下午10:37, Stefan Hajnoczi wrote: > > On Tue, Oct 15, 2019 at 11:37:17AM +0800, Jason Wang wrote: > > > On 2019/10/15 上午1:49, Stefan Hajnoczi wrote: > > > > On Fri, Oct 11, 2019 at 04:15:50PM +0800, Jason Wang wrote: > > > > > There are hardware that can do virtio datapath offloading while having > > > > > its own control path. This path tries to implement a mdev based > > > > > unified API to support using kernel virtio driver to drive those > > > > > devices. This is done by introducing a new mdev transport for virtio > > > > > (virtio_mdev) and register itself as a new kind of mdev driver. Then > > > > > it provides a unified way for kernel virtio driver to talk with mdev > > > > > device implementation. > > > > > > > > > > Though the series only contains kernel driver support, the goal is to > > > > > make the transport generic enough to support userspace drivers. This > > > > > means vhost-mdev[1] could be built on top as well by resuing the > > > > > transport. > > > > > > > > > > A sample driver is also implemented which simulate a virito-net > > > > > loopback ethernet device on top of vringh + workqueue. This could be > > > > > used as a reference implementation for real hardware driver. > > > > > > > > > > Consider mdev framework only support VFIO device and driver right now, > > > > > this series also extend it to support other types. This is done > > > > > through introducing class id to the device and pairing it with > > > > > id_talbe claimed by the driver. On top, this seris also decouple > > > > > device specific parents ops out of the common ones. > > > > I was curious so I took a quick look and posted comments. > > > > > > > > I guess this driver runs inside the guest since it registers virtio > > > > devices? > > > > > > It could run in either guest or host. But the main focus is to run in the > > > host then we can use virtio drivers in containers. > > > > > > > > > > If this is used with physical PCI devices that support datapath > > > > offloading then how are physical devices presented to the guest without > > > > SR-IOV? > > > > > > We will do control path meditation through vhost-mdev[1] and > > > vhost-vfio[2]. > > > Then we will present a full virtio compatible ethernet device for guest. > > > > > > SR-IOV is not a must, any mdev device that implements the API defined in > > > patch 5 can be used by this framework. > > What I'm trying to understand is: if you want to present a virtio-pci > > device to the guest (e.g. using vhost-mdev or vhost-vfio), then how is > > that related to this patch series? > > > This series introduce some infrastructure that would be used by vhost-mdev: > > 1) allow new type of mdev devices/drivers other than vfio (through class_id > and device ops) > > 2) a set of virtio specific callbacks that will be used by both vhost-mdev > and virtio-mdev defined in patch 5 > > Then vhost-mdev can be implemented on top: a new mdev class id but reuse the > callback defined in 2. Through this way the parent can provides a single set > of callbacks (device ops) for both kernel virtio driver (through > virtio-mdev) or userspace virtio driver (through vhost-mdev). Okay, thanks for explaining! Stefan signature.asc Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 4/7] mdev: introduce device specific ops
On 2019/10/17 下午4:45, Cornelia Huck wrote: On Thu, 17 Oct 2019 16:30:43 +0800 Jason Wang wrote: On 2019/10/17 上午12:53, Alex Williamson wrote: Yet another suggestion: have the class id derive from the function you use to set up the ops. void mdev_set_vfio_ops(struct mdev_device *mdev, const struct vfio_mdev_ops *vfio_ops) { mdev->device_ops = vfio_ops; mdev->class_id = MDEV_ID_VFIO; } void mdev_set_virtio_ops(struct mdev_device *mdev, const struct virtio_mdev_ops *virtio_ops) { mdev->device_ops = virtio_ops; mdev->class_id = MDEV_ID_VIRTIO; } void mdev_set_vhost_ops(struct mdev_device *mdev, const struct virtio_mdev_ops *virtio_ops) { mdev->device_ops = virtio_ops; mdev->class_id = MDEV_ID_VHOST; } void mdev_set_vendor_ops(struct mdev_device *mdev) /* no ops */ { mdev->class_id = MDEV_ID_VENDOR; } One further step towards making this hard to use incorrectly might be to return an error if class_id is already set. Thanks, Alex I will add a BUG_ON() when class_id has already set. Probably better a WARN_ON()? Right. Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 4/7] mdev: introduce device specific ops
On Thu, 17 Oct 2019 16:30:43 +0800 Jason Wang wrote: > On 2019/10/17 上午12:53, Alex Williamson wrote: > >>> Yet another suggestion: have the class id derive from the function > >>> you use to set up the ops. > >>> > >>> void mdev_set_vfio_ops(struct mdev_device *mdev, const struct > >>> vfio_mdev_ops *vfio_ops) { > >>> mdev->device_ops = vfio_ops; > >>> mdev->class_id = MDEV_ID_VFIO; > >>> } > >>> > >>> void mdev_set_virtio_ops(struct mdev_device *mdev, const struct > >>> virtio_mdev_ops *virtio_ops) { > >>> mdev->device_ops = virtio_ops; > >>> mdev->class_id = MDEV_ID_VIRTIO; > >>> } > >>> > >>> void mdev_set_vhost_ops(struct mdev_device *mdev, const struct > >>> virtio_mdev_ops *virtio_ops) { > >>> mdev->device_ops = virtio_ops; > >>> mdev->class_id = MDEV_ID_VHOST; > >>> } > >>> > >>> void mdev_set_vendor_ops(struct mdev_device *mdev) /* no ops */ { > >>> mdev->class_id = MDEV_ID_VENDOR; > >>> } > > One further step towards making this hard to use incorrectly might be > > to return an error if class_id is already set. Thanks, > > > > Alex > > > I will add a BUG_ON() when class_id has already set. Probably better a WARN_ON()? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 4/7] mdev: introduce device specific ops
On 2019/10/17 上午12:53, Alex Williamson wrote: On Wed, 16 Oct 2019 15:31:25 + Parav Pandit wrote: -Original Message- From: Cornelia Huck Sent: Wednesday, October 16, 2019 3:53 AM To: Parav Pandit Cc: Alex Williamson ; Jason Wang ; 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; m...@redhat.com; tiwei@intel.com; virtualization@lists.linux-foundation.org; net...@vger.kernel.org; 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; christophe.de.dinec...@gmail.com; kevin.t...@intel.com Subject: Re: [PATCH V3 4/7] mdev: introduce device specific ops On Wed, 16 Oct 2019 05:50:08 + Parav Pandit wrote: Hi Alex, -Original Message- From: Alex Williamson Sent: Tuesday, October 15, 2019 12:27 PM To: Jason Wang Cc: Cornelia Huck ; k...@vger.kernel.org; linux- s...@vger.kernel.org; linux-ker...@vger.kernel.org; dri- de...@lists.freedesktop.org; intel-...@lists.freedesktop.org; intel-gvt- d...@lists.freedesktop.org; kwankh...@nvidia.com; m...@redhat.com; tiwei@intel.com; virtualization@lists.linux-foundation.org; net...@vger.kernel.org; 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 Subject: Re: [PATCH V3 4/7] mdev: introduce device specific ops On Tue, 15 Oct 2019 20:17:01 +0800 Jason Wang wrote: On 2019/10/15 下午6:41, Cornelia Huck wrote: On Fri, 11 Oct 2019 16:15:54 +0800 Jason Wang wrote: @@ -167,9 +176,10 @@ 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:: +It is also required to specify the class_id and device +specific ops through:: - extern int mdev_set_class(struct device *dev, u16 id); + extern int mdev_set_class(struct device *dev, u16 id, + const void *ops); Apologies if that has already been discussed, but do we want a 1:1 relationship between id and ops, or can different devices with the same id register different ops? I think we have a N:1 mapping between id and ops, e.g we want both virtio-mdev and vhost-mdev use a single set of device ops. The contents of the ops structure is essentially defined by the id, which is why I was leaning towards them being defined together. They are effectively interlocked, the id defines which mdev "endpoint" driver is loaded and that driver requires mdev_get_dev_ops() to return the structure required by the driver. I wish there was a way we could incorporate type checking here. We toyed with the idea of having the class in the same structure as the ops, but I think this approach was chosen for simplicity. We could still do something like: int mdev_set_class_struct(struct device *dev, const struct mdev_class_struct *class); struct mdev_class_struct { u16 id; union { struct vfio_mdev_ops vfio_ops; struct virtio_mdev_ops virtio_ops; }; }; Maybe even: struct vfio_mdev_ops *mdev_get_vfio_ops(struct mdev_device *mdev) { BUG_ON(mdev->class.id != MDEV_ID_VFIO); return >class.vfio_ops; } The match callback would of course just use the mdev->class.id value. Functionally equivalent, but maybe better type characteristics. Thanks, Alex We have 3 use cases of mdev. 1. current mdev binding to vfio_mdev 2. mdev binding to virtio 3. mdev binding to mlx5_core without dev_ops Also (a) a given parent may serve multiple types of classes in future. (b) number of classes may not likely explode, they will be handful of them. (vfio_mdev, virtio) So, instead of making copies of this dev_ops pointer in each mdev, it
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