Re: [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
On Tue, Sep 10, 2019 at 01:06:50PM -0700, David Riley wrote: > Factor function in preparation to generating scatterlist prior to locking. Patches are looking good now, but they don't apply. What tree was used to create them? Latest virtio-gpu driver bits are in drm-misc-next (see https://cgit.freedesktop.org/drm/drm-misc). cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On Wed, Sep 11, 2019 at 10:52:03AM +0800, Jason Wang wrote:
> On 2019/9/11 上午9:47, Tiwei Bie wrote:
> > On Tue, Sep 10, 2019 at 04:19:34PM +0800, Jason Wang wrote:
> > > This path 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, unlike the exist hardware
> > > transport, this is a software transport between mdev driver and mdev
> > > device. The transport was implemented through:
> > >
> > > - configuration access was implemented through parent_ops->read()/write()
> > > - vq/config callback was implemented through parent_ops->ioctl()
> > >
> > > This transport is derived from virtio MMIO protocol and was wrote for
> > > kernel driver. But for the transport itself, but the design goal is to
> > > be generic enough to support userspace driver (this part will be added
> > > in the future).
> > >
> > > Note:
> > > - current mdev assume all the parameter of parent_ops was from
> > >userspace. This prevents us from implementing the kernel mdev
> > >driver. For a quick POC, this patch just abuse those parameter and
> > >assume the mdev device implementation will treat them as kernel
> > >pointer. This should be addressed in the formal series by extending
> > >mdev_parent_ops.
> > > - for a quick POC, I just drive the transport from MMIO, I'm pretty
> > >there's lot of optimization space for this.
> > >
> > > Signed-off-by: Jason Wang
> > > ---
> > > drivers/vfio/mdev/Kconfig| 7 +
> > > drivers/vfio/mdev/Makefile | 1 +
> > > drivers/vfio/mdev/virtio_mdev.c | 500 +++
> > > include/uapi/linux/virtio_mdev.h | 131
> > > 4 files changed, 639 insertions(+)
> > > create mode 100644 drivers/vfio/mdev/virtio_mdev.c
> > > create mode 100644 include/uapi/linux/virtio_mdev.h
> > >
> > [...]
> > > diff --git a/include/uapi/linux/virtio_mdev.h
> > > b/include/uapi/linux/virtio_mdev.h
> > > new file mode 100644
> > > index ..8040de6b960a
> > > --- /dev/null
> > > +++ b/include/uapi/linux/virtio_mdev.h
> > > @@ -0,0 +1,131 @@
> > > +/*
> > > + * Virtio mediated device driver
> > > + *
> > > + * Copyright 2019, Red Hat Corp.
> > > + *
> > > + * Based on Virtio MMIO driver by ARM Ltd, copyright ARM Ltd. 2011
> > > + *
> > > + * This header is BSD licensed so anyone can use the definitions to
> > > implement
> > > + * compatible drivers/servers.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or without
> > > + * modification, are permitted provided that the following conditions
> > > + * are met:
> > > + * 1. Redistributions of source code must retain the above copyright
> > > + *notice, this list of conditions and the following disclaimer.
> > > + * 2. Redistributions in binary form must reproduce the above copyright
> > > + *notice, this list of conditions and the following disclaimer in the
> > > + *documentation and/or other materials provided with the
> > > distribution.
> > > + * 3. Neither the name of IBM nor the names of its contributors
> > > + *may be used to endorse or promote products derived from this
> > > software
> > > + *without specific prior written permission.
> > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > > ``AS IS'' AND
> > > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> > > PURPOSE
> > > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> > > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > > CONSEQUENTIAL
> > > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
> > > GOODS
> > > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> > > STRICT
> > > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> > > WAY
> > > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > > + * SUCH DAMAGE.
> > > + */
> > > +#ifndef _LINUX_VIRTIO_MDEV_H
> > > +#define _LINUX_VIRTIO_MDEV_H
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +/*
> > > + * Ioctls
> > > + */
> > > +
> > > +struct virtio_mdev_callback {
> > > + irqreturn_t (*callback)(void *);
> > > + void *private;
> > > +};
> > > +
> > > +#define VIRTIO_MDEV 0xAF
> > > +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> > > + struct virtio_mdev_callback)
> > > +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> > > + struct virt
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On 2019/9/11 上午9:47, Tiwei Bie wrote:
On Tue, Sep 10, 2019 at 04:19:34PM +0800, Jason Wang wrote:
This path 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, unlike the exist hardware
transport, this is a software transport between mdev driver and mdev
device. The transport was implemented through:
- configuration access was implemented through parent_ops->read()/write()
- vq/config callback was implemented through parent_ops->ioctl()
This transport is derived from virtio MMIO protocol and was wrote for
kernel driver. But for the transport itself, but the design goal is to
be generic enough to support userspace driver (this part will be added
in the future).
Note:
- current mdev assume all the parameter of parent_ops was from
userspace. This prevents us from implementing the kernel mdev
driver. For a quick POC, this patch just abuse those parameter and
assume the mdev device implementation will treat them as kernel
pointer. This should be addressed in the formal series by extending
mdev_parent_ops.
- for a quick POC, I just drive the transport from MMIO, I'm pretty
there's lot of optimization space for this.
Signed-off-by: Jason Wang
---
drivers/vfio/mdev/Kconfig| 7 +
drivers/vfio/mdev/Makefile | 1 +
drivers/vfio/mdev/virtio_mdev.c | 500 +++
include/uapi/linux/virtio_mdev.h | 131
4 files changed, 639 insertions(+)
create mode 100644 drivers/vfio/mdev/virtio_mdev.c
create mode 100644 include/uapi/linux/virtio_mdev.h
[...]
diff --git a/include/uapi/linux/virtio_mdev.h b/include/uapi/linux/virtio_mdev.h
new file mode 100644
index ..8040de6b960a
--- /dev/null
+++ b/include/uapi/linux/virtio_mdev.h
@@ -0,0 +1,131 @@
+/*
+ * Virtio mediated device driver
+ *
+ * Copyright 2019, Red Hat Corp.
+ *
+ * Based on Virtio MMIO driver by ARM Ltd, copyright ARM Ltd. 2011
+ *
+ * This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include
+#include
+#include
+
+/*
+ * Ioctls
+ */
+
+struct virtio_mdev_callback {
+ irqreturn_t (*callback)(void *);
+ void *private;
+};
+
+#define VIRTIO_MDEV 0xAF
+#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
+struct virtio_mdev_callback)
+#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
+ struct virtio_mdev_callback)
+
+#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
+
+/*
+ * Control registers
+ */
+
+/* Magic value ("virt" string) - Read Only */
+#define VIRTIO_MDEV_MAGIC_VALUE0x000
+
+/* Virtio device version - Read Only */
+#define VIRTIO_MDEV_VERSION0x004
+
+/* Virtio device ID - Read Only */
+#define VIRTIO_MDEV_DEVICE_ID 0x008
+
+/* Virtio vendor ID - Read Only */
+#define VIRTIO_MDEV_VENDOR_ID 0x00c
+
+/* Bitmask of the features supported by the device (host)
+ * (32 bits per set) - Read Only */
+#define VIRTIO_MDEV_DEVICE_FEATURES0x010
+
+/* Device (host) features set selector - Write Only */
+#define VIRTIO_MDEV
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On 2019/9/10 下午9:52, Michael S. Tsirkin wrote:
On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include
+#include
+#include
+
+/*
+ * Ioctls
+ */
Pls add a bit more content here. It's redundant to state these
are ioctls. Much better to document what does each one do.
Ok.
+
+struct virtio_mdev_callback {
+ irqreturn_t (*callback)(void *);
+ void *private;
+};
+
+#define VIRTIO_MDEV 0xAF
+#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
+struct virtio_mdev_callback)
+#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
+ struct virtio_mdev_callback)
Function pointer in an ioctl parameter? How does this ever make sense?
I admit this is hacky (casting).
And can't we use a couple of registers for this, and avoid ioctls?
Yes, how about something like interrupt numbers for each virtqueue and
config?
Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI
transport in fact. And using either MSIX or irq number is actually
another layer of indirection. So I think we can just write callback
function and parameter through registers.
+
+#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
+
+/*
+ * Control registers
+ */
+
+/* Magic value ("virt" string) - Read Only */
+#define VIRTIO_MDEV_MAGIC_VALUE0x000
+
+/* Virtio device version - Read Only */
+#define VIRTIO_MDEV_VERSION0x004
+
+/* Virtio device ID - Read Only */
+#define VIRTIO_MDEV_DEVICE_ID 0x008
+
+/* Virtio vendor ID - Read Only */
+#define VIRTIO_MDEV_VENDOR_ID 0x00c
+
+/* Bitmask of the features supported by the device (host)
+ * (32 bits per set) - Read Only */
+#define VIRTIO_MDEV_DEVICE_FEATURES0x010
+
+/* Device (host) features set selector - Write Only */
+#define VIRTIO_MDEV_DEVICE_FEATURES_SEL0x014
+
+/* Bitmask of features activated by the driver (guest)
+ * (32 bits per set) - Write Only */
+#define VIRTIO_MDEV_DRIVER_FEATURES0x020
+
+/* Activated features set selector - Write Only */
+#define VIRTIO_MDEV_DRIVER_FEATURES_SEL0x024
+
+/* Queue selector - Write Only */
+#define VIRTIO_MDEV_QUEUE_SEL 0x030
+
+/* Maximum size of the currently selected queue - Read Only */
+#define VIRTIO_MDEV_QUEUE_NUM_MAX 0x034
+
+/* Queue size for the currently selected queue - Write Only */
+#define VIRTIO_MDEV_QUEUE_NUM 0x038
+
+/* Ready bit for the currently selected queue - Read Write */
+#define VIRTIO_MDEV_QUEUE_READY0x044
Is this same as started?
Do you mean "status"?
I really meant "enabled", didn't remember the correct name.
As in: VIRTIO_PCI_COMMON_Q_ENABLE
Yes, it's the same.
Thanks
+
+/* Alignment of virtqueue - Read Only */
+#define VIRTIO_MDEV_QUEUE_ALIGN0x048
+
+/* Queue notifier - Write Only */
+#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
+
+/* Device status register - Read Write */
+#define VIRTIO_MDEV_STATUS 0x060
+
+/* Selected queue's Descriptor Table address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
+#define VIRTIO_MDEV_QUEUE_DESC_HIGH0x084
+
+/* Selected queue's Available Ring address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_AVAIL_LOW0x090
+#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
+
+/* Selected queue's Used Ring address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
+#define VIRTIO_MDEV_QUEUE_USED_HIGH0x0a4
+
+/* Configuration atomicity value */
+#define VIRTIO_MDEV_CONFIG_GENERATION 0x0fc
+
+/* The config space is defined by each driver as
+ * the per-driver configuration space - Read Write */
+#define VIRTIO_MDEV_CONFIG 0x100
Mixing device and generic config space is what virtio pci did,
caused lots of problems with extensions.
It would be better to reserve much more space.
I see, will do this.
Thanks
+
+#endif
+
+
+/* Ready bit for the currently selected queue - Read Write */
--
2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] scsi: virtio_scsi: unplug LUNs when events missed
Matt, > The event handler calls scsi_scan_host() when events are missed, which > will hotplug new LUNs. However, this function won't remove any > unplugged LUNs. The result is that hotunplug doesn't work properly > when the number of unplugged LUNs exceeds the event queue size > (currently 8). > > Scan existing LUNs when events are missed to check if they are still > present. If not, remove them. Applied to 5.4/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On Tue, Sep 10, 2019 at 04:19:34PM +0800, Jason Wang wrote:
> This path 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, unlike the exist hardware
> transport, this is a software transport between mdev driver and mdev
> device. The transport was implemented through:
>
> - configuration access was implemented through parent_ops->read()/write()
> - vq/config callback was implemented through parent_ops->ioctl()
>
> This transport is derived from virtio MMIO protocol and was wrote for
> kernel driver. But for the transport itself, but the design goal is to
> be generic enough to support userspace driver (this part will be added
> in the future).
>
> Note:
> - current mdev assume all the parameter of parent_ops was from
> userspace. This prevents us from implementing the kernel mdev
> driver. For a quick POC, this patch just abuse those parameter and
> assume the mdev device implementation will treat them as kernel
> pointer. This should be addressed in the formal series by extending
> mdev_parent_ops.
> - for a quick POC, I just drive the transport from MMIO, I'm pretty
> there's lot of optimization space for this.
>
> Signed-off-by: Jason Wang
> ---
> drivers/vfio/mdev/Kconfig| 7 +
> drivers/vfio/mdev/Makefile | 1 +
> drivers/vfio/mdev/virtio_mdev.c | 500 +++
> include/uapi/linux/virtio_mdev.h | 131
> 4 files changed, 639 insertions(+)
> create mode 100644 drivers/vfio/mdev/virtio_mdev.c
> create mode 100644 include/uapi/linux/virtio_mdev.h
>
[...]
> diff --git a/include/uapi/linux/virtio_mdev.h
> b/include/uapi/linux/virtio_mdev.h
> new file mode 100644
> index ..8040de6b960a
> --- /dev/null
> +++ b/include/uapi/linux/virtio_mdev.h
> @@ -0,0 +1,131 @@
> +/*
> + * Virtio mediated device driver
> + *
> + * Copyright 2019, Red Hat Corp.
> + *
> + * Based on Virtio MMIO driver by ARM Ltd, copyright ARM Ltd. 2011
> + *
> + * This header is BSD licensed so anyone can use the definitions to implement
> + * compatible drivers/servers.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of IBM nor the names of its contributors
> + *may be used to endorse or promote products derived from this software
> + *without specific prior written permission.
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS
> IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#ifndef _LINUX_VIRTIO_MDEV_H
> +#define _LINUX_VIRTIO_MDEV_H
> +
> +#include
> +#include
> +#include
> +
> +/*
> + * Ioctls
> + */
> +
> +struct virtio_mdev_callback {
> + irqreturn_t (*callback)(void *);
> + void *private;
> +};
> +
> +#define VIRTIO_MDEV 0xAF
> +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> + struct virtio_mdev_callback)
> +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> + struct virtio_mdev_callback)
> +
> +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev"
> +
> +/*
> + * Control registers
> + */
> +
> +/* Magic value ("virt" string) - Read Only */
> +#define VIRTIO_MDEV_MAGIC_VALUE 0x000
> +
> +/* Virtio device version - Read Only */
> +#define VIRTIO_MDEV_VERSION 0x004
> +
> +/* Virtio device ID - Read Only */
> +#define VIRTIO_MDEV_DEVICE_ID0x008
> +
> +/* Virtio vendor ID - Read Only */
> +#define VIRTIO_MDEV_VENDOR_ID0x00c
> +
> +/* Bitmask of the features su
[PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations.
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 79 +++---
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index ac60be9b5c19..a8732a8af766 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
if (ret)
goto out_free;
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
ttm_eu_backoff_reservation(&ticket, &validate_list);
out_free:
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index bf5a4a50b002..76cf2b9d5d1d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -251,13 +251,54 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct
*work)
wake_up(&vgdev->cursorq.ack_queue);
}
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ if (WARN_ON(!PAGE_ALIGNED(data)))
+ return NULL;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ *sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
+ ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = min_t(int, PAGE_SIZE, size);
+ sg_set_page(sg, pg, s, 0);
+
+ size -= s;
+ data += s;
+ }
+
+ return sgt;
+}
+
static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(&vgdev->ctrlq.qlock)
__acquires(&vgdev->ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
int ret;
@@ -268,9 +309,8 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
sgs[outcnt + incnt] = &vcmd;
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] = &vout;
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -305,7 +345,24 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
int rc;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+&outcnt);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
+ vout = &sg;
+ outcnt = 1;
+ }
+ }
again:
spin_lock(&vgdev->ctrlq.qlock);
@@ -318,7 +375,7 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
* to
[PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
Factor function in preparation to generating scatterlist prior to locking.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 981ee16e3ee9..bf5a4a50b002 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -299,17 +299,6 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
return ret;
}
-static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
-{
- int rc;
-
- spin_lock(&vgdev->ctrlq.qlock);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(&vgdev->ctrlq.qlock);
- return rc;
-}
-
static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
@@ -335,13 +324,19 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
goto again;
}
- if (fence)
+ if (hdr && fence)
virtio_gpu_fence_emit(vgdev, hdr, fence);
rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
spin_unlock(&vgdev->ctrlq.qlock);
return rc;
}
+static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.162.g0b9fbb3734-goog
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 69/92] kvm: x86: keep the page protected if tracked by the introspection tool
On Tue, 10 Sep 2019 10:26:42 -0400, Konrad Rzeszutek Wilk wrote: > On Fri, Aug 09, 2019 at 07:00:24PM +0300, Adalbert Lazăr wrote: > > This patch might be obsolete thanks to single-stepping. > > sooo should it be skipped from this large patchset to easy > review? I'll add a couple of warning messages to check if this patch is still needed, in order to skip it from the next submission (which will be smaller:) However, on AMD, single-stepping is not an option. Thanks, Adalbert > > > > > Signed-off-by: Adalbert Lazăr > > --- > > arch/x86/kvm/x86.c | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 2c06de73a784..06f44ce8ed07 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -6311,7 +6311,8 @@ static bool reexecute_instruction(struct kvm_vcpu > > *vcpu, gva_t cr2, > > indirect_shadow_pages = vcpu->kvm->arch.indirect_shadow_pages; > > spin_unlock(&vcpu->kvm->mmu_lock); > > > > - if (indirect_shadow_pages) > > + if (indirect_shadow_pages > > + && !kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > > kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > > > return true; > > @@ -6322,7 +6323,8 @@ static bool reexecute_instruction(struct kvm_vcpu > > *vcpu, gva_t cr2, > > * and it failed try to unshadow page and re-enter the > > * guest to let CPU execute the instruction. > > */ > > - kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > + if (!kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > > + kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > > > /* > > * If the access faults on its page table, it can not > > @@ -6374,6 +6376,9 @@ static bool retry_instruction(struct x86_emulate_ctxt > > *ctxt, > > if (!vcpu->arch.mmu->direct_map) > > gpa = kvm_mmu_gva_to_gpa_write(vcpu, cr2, NULL); > > > > + if (kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > > + return false; > > + > > kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > > > return true; ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] x86/paravirt: Use a single ops structure
On 10.09.19 16:53, Dan Carpenter wrote:
Hello Juergen Gross,
The patch 5c83511bdb98: "x86/paravirt: Use a single ops structure"
from Aug 28, 2018, leads to the following static checker warning:
arch/x86/kernel/paravirt.c:123 paravirt_patch_default()
warn: uncapped user index '*(&pv_ops + type)'
arch/x86/kernel/paravirt.c:124 paravirt_patch_default()
error: buffer overflow '&pv_ops' 90 <= 255 user_rl='0-255'
arch/x86/kernel/paravirt.c
116 unsigned paravirt_patch_default(u8 type, void *insn_buff,
117 unsigned long addr, unsigned len)
118 {
119 /*
120 * Neat trick to map patch type back to the call within the
121 * corresponding structure.
122 */
123 void *opfunc = *((void **)&pv_ops + type);
^^
This code is actually pretty old...
This isn't a security issue, but the size of &pv_ops is variable but
type isn't checked so we could be reading beyond the end. We could do
something like:
if (type >= sizeof(pv_ops) / sizeof(void *))
return -EINVAL;
No, not really. Please check how th return value is being used: it
specifies the length of the instruction to patch. Just returning
-EINVAL would probably clobber most of the kernel.
Please note that type is derived from the pv_ops field names, so
in reality the risk to read beyond the end of pv_ops is zero.
Juergen
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] x86/paravirt: Use a single ops structure
Hello Juergen Gross,
The patch 5c83511bdb98: "x86/paravirt: Use a single ops structure"
from Aug 28, 2018, leads to the following static checker warning:
arch/x86/kernel/paravirt.c:123 paravirt_patch_default()
warn: uncapped user index '*(&pv_ops + type)'
arch/x86/kernel/paravirt.c:124 paravirt_patch_default()
error: buffer overflow '&pv_ops' 90 <= 255 user_rl='0-255'
arch/x86/kernel/paravirt.c
116 unsigned paravirt_patch_default(u8 type, void *insn_buff,
117 unsigned long addr, unsigned len)
118 {
119 /*
120 * Neat trick to map patch type back to the call within the
121 * corresponding structure.
122 */
123 void *opfunc = *((void **)&pv_ops + type);
^^
This code is actually pretty old...
This isn't a security issue, but the size of &pv_ops is variable but
type isn't checked so we could be reading beyond the end. We could do
something like:
if (type >= sizeof(pv_ops) / sizeof(void *))
return -EINVAL;
opfunc = *((void **)&pv_ops + type);
124 unsigned ret;
125
126 if (opfunc == NULL)
127 /* If there's no function, patch it with a ud2a (BUG) */
128 ret = paravirt_patch_insns(insn_buff, len, ud2a,
ud2a+sizeof(ud2a));
129 else if (opfunc == _paravirt_nop)
130 ret = 0;
131
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 69/92] kvm: x86: keep the page protected if tracked by the introspection tool
On Fri, Aug 09, 2019 at 07:00:24PM +0300, Adalbert Lazăr wrote: > This patch might be obsolete thanks to single-stepping. sooo should it be skipped from this large patchset to easy review? > > Signed-off-by: Adalbert Lazăr > --- > arch/x86/kvm/x86.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2c06de73a784..06f44ce8ed07 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6311,7 +6311,8 @@ static bool reexecute_instruction(struct kvm_vcpu > *vcpu, gva_t cr2, > indirect_shadow_pages = vcpu->kvm->arch.indirect_shadow_pages; > spin_unlock(&vcpu->kvm->mmu_lock); > > - if (indirect_shadow_pages) > + if (indirect_shadow_pages > + && !kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > return true; > @@ -6322,7 +6323,8 @@ static bool reexecute_instruction(struct kvm_vcpu > *vcpu, gva_t cr2, >* and it failed try to unshadow page and re-enter the >* guest to let CPU execute the instruction. >*/ > - kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > + if (!kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > + kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > /* >* If the access faults on its page table, it can not > @@ -6374,6 +6376,9 @@ static bool retry_instruction(struct x86_emulate_ctxt > *ctxt, > if (!vcpu->arch.mmu->direct_map) > gpa = kvm_mmu_gva_to_gpa_write(vcpu, cr2, NULL); > > + if (kvmi_tracked_gfn(vcpu, gpa_to_gfn(gpa))) > + return false; > + > kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)); > > return true; ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
>
> On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
> > > +#ifndef _LINUX_VIRTIO_MDEV_H
> > > +#define _LINUX_VIRTIO_MDEV_H
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +/*
> > > + * Ioctls
> > > + */
> > Pls add a bit more content here. It's redundant to state these
> > are ioctls. Much better to document what does each one do.
>
>
> Ok.
>
>
> >
> > > +
> > > +struct virtio_mdev_callback {
> > > + irqreturn_t (*callback)(void *);
> > > + void *private;
> > > +};
> > > +
> > > +#define VIRTIO_MDEV 0xAF
> > > +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> > > + struct virtio_mdev_callback)
> > > +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> > > + struct virtio_mdev_callback)
> > Function pointer in an ioctl parameter? How does this ever make sense?
>
>
> I admit this is hacky (casting).
>
>
> > And can't we use a couple of registers for this, and avoid ioctls?
>
>
> Yes, how about something like interrupt numbers for each virtqueue and
> config?
Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
>
> >
> > > +
> > > +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev"
> > > +
> > > +/*
> > > + * Control registers
> > > + */
> > > +
> > > +/* Magic value ("virt" string) - Read Only */
> > > +#define VIRTIO_MDEV_MAGIC_VALUE 0x000
> > > +
> > > +/* Virtio device version - Read Only */
> > > +#define VIRTIO_MDEV_VERSION 0x004
> > > +
> > > +/* Virtio device ID - Read Only */
> > > +#define VIRTIO_MDEV_DEVICE_ID0x008
> > > +
> > > +/* Virtio vendor ID - Read Only */
> > > +#define VIRTIO_MDEV_VENDOR_ID0x00c
> > > +
> > > +/* Bitmask of the features supported by the device (host)
> > > + * (32 bits per set) - Read Only */
> > > +#define VIRTIO_MDEV_DEVICE_FEATURES 0x010
> > > +
> > > +/* Device (host) features set selector - Write Only */
> > > +#define VIRTIO_MDEV_DEVICE_FEATURES_SEL 0x014
> > > +
> > > +/* Bitmask of features activated by the driver (guest)
> > > + * (32 bits per set) - Write Only */
> > > +#define VIRTIO_MDEV_DRIVER_FEATURES 0x020
> > > +
> > > +/* Activated features set selector - Write Only */
> > > +#define VIRTIO_MDEV_DRIVER_FEATURES_SEL 0x024
> > > +
> > > +/* Queue selector - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_SEL0x030
> > > +
> > > +/* Maximum size of the currently selected queue - Read Only */
> > > +#define VIRTIO_MDEV_QUEUE_NUM_MAX0x034
> > > +
> > > +/* Queue size for the currently selected queue - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_NUM0x038
> > > +
> > > +/* Ready bit for the currently selected queue - Read Write */
> > > +#define VIRTIO_MDEV_QUEUE_READY 0x044
> > Is this same as started?
>
>
> Do you mean "status"?
I really meant "enabled", didn't remember the correct name.
As in: VIRTIO_PCI_COMMON_Q_ENABLE
>
> >
> > > +
> > > +/* Alignment of virtqueue - Read Only */
> > > +#define VIRTIO_MDEV_QUEUE_ALIGN 0x048
> > > +
> > > +/* Queue notifier - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
> > > +
> > > +/* Device status register - Read Write */
> > > +#define VIRTIO_MDEV_STATUS 0x060
> > > +
> > > +/* Selected queue's Descriptor Table address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
> > > +#define VIRTIO_MDEV_QUEUE_DESC_HIGH 0x084
> > > +
> > > +/* Selected queue's Available Ring address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_AVAIL_LOW 0x090
> > > +#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
> > > +
> > > +/* Selected queue's Used Ring address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
> > > +#define VIRTIO_MDEV_QUEUE_USED_HIGH 0x0a4
> > > +
> > > +/* Configuration atomicity value */
> > > +#define VIRTIO_MDEV_CONFIG_GENERATION0x0fc
> > > +
> > > +/* The config space is defined by each driver as
> > > + * the per-driver configuration space - Read Write */
> > > +#define VIRTIO_MDEV_CONFIG 0x100
> > Mixing device and generic config space is what virtio pci did,
> > caused lots of problems with extensions.
> > It would be better to reserve much more space.
>
>
> I see, will do this.
>
> Thanks
>
>
> >
> >
> > > +
> > > +#endif
> > > +
> > > +
> > > +/* Ready bit for the currently selected queue - Read Write */
> > > --
> > > 2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 4/4] docs: Sample driver to demonstrate how to implement virtio-mdev framework
On 2019/9/10 下午6:15, Michael S. Tsirkin wrote:
On Tue, Sep 10, 2019 at 04:19:35PM +0800, Jason Wang wrote:
This sample driver creates mdev device that simulate virtio net device
over virtio mdev transport. The device is implemented through vringh
and workqueue.
Signed-off-by: Jason Wang
---
samples/Kconfig| 7 +
samples/vfio-mdev/Makefile | 1 +
samples/vfio-mdev/mvnet.c | 766 +
3 files changed, 774 insertions(+)
create mode 100644 samples/vfio-mdev/mvnet.c
So for a POC, this is a bit too rough to be able to judge
whether the approach is workable.
Can you get it a bit more into shape esp wrt UAPI?
Will do, actually I'm not 100% sure it need to go through UAPI.
Technically we can expose them for userspace but do we want something
simpler e.g vhost? Then we can make it as an internal API without caring
about UAPI stuffs like compatibility etc.
Except for UAPI, another other thing that can help you to judge this
approach? E.g we can make the device more reasonable rather than a
simple loopback device, e.g implementing a pair network device. But it
might be too complex for just a sample.
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 ..da295b41955e
--- /dev/null
+++ b/samples/vfio-mdev/mvnet.c
@@ -0,0 +1,766 @@
+// 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
Documentation on how to use this?
Will add.
that creates mdev device that simulates ethernet
+ * device virtio mdev transport.
Well not exactly. What it seems to do is short-circuit
RX and TX rings.
Yes, a loopback device.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define VERSION_STRING "0.1"
+#define DRIVER_AUTHOR "NVIDIA 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;
+ u32 desc_addr_lo;
+ u32 desc_addr_hi;
+ u32 device_addr_lo;
+ u32 device_addr_hi;
+ u32 driver_addr_lo;
+ u32 driver_addr_hi;
+ u64 desc_addr;
+ u64 device_addr;
+ u64 driver_addr;
+ void *private;
+ irqreturn_t (*cb)(void *);
+};
+
+#define MVNET_QUEUE_ALIGN PAGE_SIZE
+#define MVNET_QUEUE_MAX 256
+#define MVNET_MAGIC_VALUE ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)
+#define MVNET_VERSION 0x1 /* Implies virtio 1.0 */
+#define MVNET_DEVICE_ID 0x1 /* network card */
+#define MVNET_VENDOR_ID 0 /* is this correct ? */
+#define MVNET_DEVICE_FEATURES VIRTIO_F_VERSION_1
+
+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;
+ struct virtio_mdev_callback *cbs;
+ void *buffer;
+ u32 queue_sel;
+ u32 driver_features_sel;
+ u32 driver_features[2];
+ u32 device_features_sel;
+ u32 status;
+ u32 generation;
+ u32 num;
+ struct list_head next;
+};
+
+static struct
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include
+#include
+#include
+
+/*
+ * Ioctls
+ */
Pls add a bit more content here. It's redundant to state these
are ioctls. Much better to document what does each one do.
Ok.
+
+struct virtio_mdev_callback {
+ irqreturn_t (*callback)(void *);
+ void *private;
+};
+
+#define VIRTIO_MDEV 0xAF
+#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
+struct virtio_mdev_callback)
+#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
+ struct virtio_mdev_callback)
Function pointer in an ioctl parameter? How does this ever make sense?
I admit this is hacky (casting).
And can't we use a couple of registers for this, and avoid ioctls?
Yes, how about something like interrupt numbers for each virtqueue and
config?
+
+#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
+
+/*
+ * Control registers
+ */
+
+/* Magic value ("virt" string) - Read Only */
+#define VIRTIO_MDEV_MAGIC_VALUE0x000
+
+/* Virtio device version - Read Only */
+#define VIRTIO_MDEV_VERSION0x004
+
+/* Virtio device ID - Read Only */
+#define VIRTIO_MDEV_DEVICE_ID 0x008
+
+/* Virtio vendor ID - Read Only */
+#define VIRTIO_MDEV_VENDOR_ID 0x00c
+
+/* Bitmask of the features supported by the device (host)
+ * (32 bits per set) - Read Only */
+#define VIRTIO_MDEV_DEVICE_FEATURES0x010
+
+/* Device (host) features set selector - Write Only */
+#define VIRTIO_MDEV_DEVICE_FEATURES_SEL0x014
+
+/* Bitmask of features activated by the driver (guest)
+ * (32 bits per set) - Write Only */
+#define VIRTIO_MDEV_DRIVER_FEATURES0x020
+
+/* Activated features set selector - Write Only */
+#define VIRTIO_MDEV_DRIVER_FEATURES_SEL0x024
+
+/* Queue selector - Write Only */
+#define VIRTIO_MDEV_QUEUE_SEL 0x030
+
+/* Maximum size of the currently selected queue - Read Only */
+#define VIRTIO_MDEV_QUEUE_NUM_MAX 0x034
+
+/* Queue size for the currently selected queue - Write Only */
+#define VIRTIO_MDEV_QUEUE_NUM 0x038
+
+/* Ready bit for the currently selected queue - Read Write */
+#define VIRTIO_MDEV_QUEUE_READY0x044
Is this same as started?
Do you mean "status"?
+
+/* Alignment of virtqueue - Read Only */
+#define VIRTIO_MDEV_QUEUE_ALIGN0x048
+
+/* Queue notifier - Write Only */
+#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
+
+/* Device status register - Read Write */
+#define VIRTIO_MDEV_STATUS 0x060
+
+/* Selected queue's Descriptor Table address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
+#define VIRTIO_MDEV_QUEUE_DESC_HIGH0x084
+
+/* Selected queue's Available Ring address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_AVAIL_LOW0x090
+#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
+
+/* Selected queue's Used Ring address, 64 bits in two halves */
+#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
+#define VIRTIO_MDEV_QUEUE_USED_HIGH0x0a4
+
+/* Configuration atomicity value */
+#define VIRTIO_MDEV_CONFIG_GENERATION 0x0fc
+
+/* The config space is defined by each driver as
+ * the per-driver configuration space - Read Write */
+#define VIRTIO_MDEV_CONFIG 0x100
Mixing device and generic config space is what virtio pci did,
caused lots of problems with extensions.
It would be better to reserve much more space.
I see, will do this.
Thanks
+
+#endif
+
+
+/* Ready bit for the currently selected queue - Read Write */
--
2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/4] Merge VRAM MM and GEM VRAM source files
On Mon, Sep 09, 2019 at 03:04:49PM +0200, Thomas Zimmermann wrote: > VRAM MM and GEM VRAM are only used with each other. This patch set > moves VRAM MM into GEM VRAM source files and cleans up the helper's > public interface. > > Thomas Zimmermann (4): > drm/vram: Move VRAM memory manager to GEM VRAM implementation > drm/vram: Have VRAM MM call GEM VRAM functions directly > drm/vram: Unexport internal functions of VRAM MM > drm/vram: Unconditonally set BO call-back functions Looks all sane. Acked-by: Gerd Hoffmann cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 4/4] docs: Sample driver to demonstrate how to implement virtio-mdev framework
On Tue, Sep 10, 2019 at 04:19:35PM +0800, Jason Wang wrote:
> This sample driver creates mdev device that simulate virtio net device
> over virtio mdev transport. The device is implemented through vringh
> and workqueue.
>
> Signed-off-by: Jason Wang
> ---
> samples/Kconfig| 7 +
> samples/vfio-mdev/Makefile | 1 +
> samples/vfio-mdev/mvnet.c | 766 +
> 3 files changed, 774 insertions(+)
> create mode 100644 samples/vfio-mdev/mvnet.c
So for a POC, this is a bit too rough to be able to judge
whether the approach is workable.
Can you get it a bit more into shape esp wrt UAPI?
> 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 ..da295b41955e
> --- /dev/null
> +++ b/samples/vfio-mdev/mvnet.c
> @@ -0,0 +1,766 @@
> +// 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
Documentation on how to use this?
> that creates mdev device that simulates ethernet
> + * device virtio mdev transport.
Well not exactly. What it seems to do is short-circuit
RX and TX rings.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define VERSION_STRING "0.1"
> +#define DRIVER_AUTHOR "NVIDIA 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;
> + u32 desc_addr_lo;
> + u32 desc_addr_hi;
> + u32 device_addr_lo;
> + u32 device_addr_hi;
> + u32 driver_addr_lo;
> + u32 driver_addr_hi;
> + u64 desc_addr;
> + u64 device_addr;
> + u64 driver_addr;
> + void *private;
> + irqreturn_t (*cb)(void *);
> +};
> +
> +#define MVNET_QUEUE_ALIGN PAGE_SIZE
> +#define MVNET_QUEUE_MAX 256
> +#define MVNET_MAGIC_VALUE ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)
> +#define MVNET_VERSION 0x1 /* Implies virtio 1.0 */
> +#define MVNET_DEVICE_ID 0x1 /* network card */
> +#define MVNET_VENDOR_ID 0 /* is this correct ? */
> +#define MVNET_DEVICE_FEATURES VIRTIO_F_VERSION_1
> +
> +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;
> + struct virtio_mdev_callback *cbs;
> + void *buffer;
> + u32 queue_sel;
> + u32 driver_features_sel;
> + u32 driver_features[2];
> + u32 device_features_sel;
> + u32 status;
> + u32 generation;
> + u32 num;
> + 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 idx)
> +{
> + struct mvnet_virtqueue *vq = &mvnet->vqs[idx];
> + int ret;
> +
> + vq->desc_addr = (u64)vq->desc_addr_hi << 32 | vq->desc_addr_lo;
> + vq->device_addr = (u64)vq->device_addr_hi << 32 | vq->device_addr_lo;
> + vq->driver_addr = (u64)vq->driver_addr_
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On Tue, Sep 10, 2019 at 04:19:34PM +0800, Jason Wang wrote:
> This path 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, unlike the exist hardware
> transport, this is a software transport between mdev driver and mdev
> device. The transport was implemented through:
>
> - configuration access was implemented through parent_ops->read()/write()
> - vq/config callback was implemented through parent_ops->ioctl()
>
> This transport is derived from virtio MMIO protocol and was wrote for
> kernel driver. But for the transport itself, but the design goal is to
> be generic enough to support userspace driver (this part will be added
> in the future).
>
> Note:
> - current mdev assume all the parameter of parent_ops was from
> userspace. This prevents us from implementing the kernel mdev
> driver. For a quick POC, this patch just abuse those parameter and
> assume the mdev device implementation will treat them as kernel
> pointer. This should be addressed in the formal series by extending
> mdev_parent_ops.
> - for a quick POC, I just drive the transport from MMIO, I'm pretty
> there's lot of optimization space for this.
>
> Signed-off-by: Jason Wang
> ---
> drivers/vfio/mdev/Kconfig| 7 +
> drivers/vfio/mdev/Makefile | 1 +
> drivers/vfio/mdev/virtio_mdev.c | 500 +++
> include/uapi/linux/virtio_mdev.h | 131
> 4 files changed, 639 insertions(+)
> create mode 100644 drivers/vfio/mdev/virtio_mdev.c
> create mode 100644 include/uapi/linux/virtio_mdev.h
>
> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
> index 5da27f2100f9..c488c31fc137 100644
> --- a/drivers/vfio/mdev/Kconfig
> +++ b/drivers/vfio/mdev/Kconfig
> @@ -16,3 +16,10 @@ config VFIO_MDEV_DEVICE
> default n
> help
> VFIO based driver for Mediated devices.
> +
> +config VIRTIO_MDEV_DEVICE
> + tristate "VIRTIO driver for Mediated devices"
> + depends on VFIO_MDEV && VIRTIO
> + default n
> + help
> + VIRTIO based driver for Mediated devices.
> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
> index 101516fdf375..99d31e29c23e 100644
> --- a/drivers/vfio/mdev/Makefile
> +++ b/drivers/vfio/mdev/Makefile
> @@ -4,3 +4,4 @@ mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>
> obj-$(CONFIG_VFIO_MDEV) += mdev.o
> obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
> +obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o
> diff --git a/drivers/vfio/mdev/virtio_mdev.c b/drivers/vfio/mdev/virtio_mdev.c
> new file mode 100644
> index ..5ff09089297e
> --- /dev/null
> +++ b/drivers/vfio/mdev/virtio_mdev.c
> @@ -0,0 +1,500 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * VIRTIO based driver for Mediated device
> + *
> + * Copyright (c) 2019, Red Hat. All rights reserved.
> + * Author: Jason Wang
> + *
> + * Based on Virtio MMIO driver.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "mdev_private.h"
> +
> +#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;
> +
> + struct virtqueue **vqs;
> + spinlock_t lock;
> +};
> +
> +struct virtio_mdev_vq_info {
> + /* the actual virtqueue */
> + struct virtqueue *vq;
> +
> + /* the list node for the virtqueues list */
> + struct list_head node;
> +};
> +
> +static u32 virtio_mdev_readl(struct mdev_device *mdev,
> + loff_t off)
> +{
> + struct mdev_parent *parent = mdev->parent;
> + ssize_t len;
> + u32 val;
> +
> + if (unlikely(!parent->ops->read))
> + return 0x;
> +
> + len = parent->ops->read(mdev, (char *)&val, 4, &off);
> + if (len != 4)
> + return 0x;
> +
> + return val;
> +}
> +
> +static void virtio_mdev_writel(struct mdev_device *mdev,
> +loff_t off, u32 val)
> +{
> + struct mdev_parent *parent = mdev->parent;
> +
> + if (unlikely(!parent->ops->write))
> + return;
> +
> + parent->ops->write(mdev, (char *)&val, 4, &off);
> +
> + return;
> +}
> +
> +static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset,
> + void *buf, unsigned len)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev)
[RFC PATCH 3/4] virtio: introudce a mdev based transport
This path 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, unlike the exist hardware
transport, this is a software transport between mdev driver and mdev
device. The transport was implemented through:
- configuration access was implemented through parent_ops->read()/write()
- vq/config callback was implemented through parent_ops->ioctl()
This transport is derived from virtio MMIO protocol and was wrote for
kernel driver. But for the transport itself, but the design goal is to
be generic enough to support userspace driver (this part will be added
in the future).
Note:
- current mdev assume all the parameter of parent_ops was from
userspace. This prevents us from implementing the kernel mdev
driver. For a quick POC, this patch just abuse those parameter and
assume the mdev device implementation will treat them as kernel
pointer. This should be addressed in the formal series by extending
mdev_parent_ops.
- for a quick POC, I just drive the transport from MMIO, I'm pretty
there's lot of optimization space for this.
Signed-off-by: Jason Wang
---
drivers/vfio/mdev/Kconfig| 7 +
drivers/vfio/mdev/Makefile | 1 +
drivers/vfio/mdev/virtio_mdev.c | 500 +++
include/uapi/linux/virtio_mdev.h | 131
4 files changed, 639 insertions(+)
create mode 100644 drivers/vfio/mdev/virtio_mdev.c
create mode 100644 include/uapi/linux/virtio_mdev.h
diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
index 5da27f2100f9..c488c31fc137 100644
--- a/drivers/vfio/mdev/Kconfig
+++ b/drivers/vfio/mdev/Kconfig
@@ -16,3 +16,10 @@ config VFIO_MDEV_DEVICE
default n
help
VFIO based driver for Mediated devices.
+
+config VIRTIO_MDEV_DEVICE
+ tristate "VIRTIO driver for Mediated devices"
+ depends on VFIO_MDEV && VIRTIO
+ default n
+ help
+ VIRTIO based driver for Mediated devices.
diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
index 101516fdf375..99d31e29c23e 100644
--- a/drivers/vfio/mdev/Makefile
+++ b/drivers/vfio/mdev/Makefile
@@ -4,3 +4,4 @@ mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
obj-$(CONFIG_VFIO_MDEV) += mdev.o
obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
+obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o
diff --git a/drivers/vfio/mdev/virtio_mdev.c b/drivers/vfio/mdev/virtio_mdev.c
new file mode 100644
index ..5ff09089297e
--- /dev/null
+++ b/drivers/vfio/mdev/virtio_mdev.c
@@ -0,0 +1,500 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VIRTIO based driver for Mediated device
+ *
+ * Copyright (c) 2019, Red Hat. All rights reserved.
+ * Author: Jason Wang
+ *
+ * Based on Virtio MMIO driver.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "mdev_private.h"
+
+#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;
+
+ struct virtqueue **vqs;
+ spinlock_t lock;
+};
+
+struct virtio_mdev_vq_info {
+ /* the actual virtqueue */
+ struct virtqueue *vq;
+
+ /* the list node for the virtqueues list */
+ struct list_head node;
+};
+
+static u32 virtio_mdev_readl(struct mdev_device *mdev,
+loff_t off)
+{
+ struct mdev_parent *parent = mdev->parent;
+ ssize_t len;
+ u32 val;
+
+ if (unlikely(!parent->ops->read))
+ return 0x;
+
+ len = parent->ops->read(mdev, (char *)&val, 4, &off);
+ if (len != 4)
+ return 0x;
+
+ return val;
+}
+
+static void virtio_mdev_writel(struct mdev_device *mdev,
+ loff_t off, u32 val)
+{
+ struct mdev_parent *parent = mdev->parent;
+
+ if (unlikely(!parent->ops->write))
+ return;
+
+ parent->ops->write(mdev, (char *)&val, 4, &off);
+
+ return;
+}
+
+static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset,
+ void *buf, unsigned len)
+{
+ struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
+ struct mdev_device *mdev = vm_dev->mdev;
+ struct mdev_parent *parent = mdev->parent;
+
+ loff_t off = offset + VIRTIO_MDEV_CONFIG;
+
+ switch (len) {
+ case 1:
+ *(u8 *)buf = parent->ops->read(mdev, buf, 1, &off);
+ break;
[RFC PATCH 4/4] 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.
Signed-off-by: Jason Wang
---
samples/Kconfig| 7 +
samples/vfio-mdev/Makefile | 1 +
samples/vfio-mdev/mvnet.c | 766 +
3 files changed, 774 insertions(+)
create mode 100644 samples/vfio-mdev/mvnet.c
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 ..da295b41955e
--- /dev/null
+++ b/samples/vfio-mdev/mvnet.c
@@ -0,0 +1,766 @@
+// 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
+ * device virtio mdev transport.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define VERSION_STRING "0.1"
+#define DRIVER_AUTHOR "NVIDIA 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;
+ u32 desc_addr_lo;
+ u32 desc_addr_hi;
+ u32 device_addr_lo;
+ u32 device_addr_hi;
+ u32 driver_addr_lo;
+ u32 driver_addr_hi;
+ u64 desc_addr;
+ u64 device_addr;
+ u64 driver_addr;
+ void *private;
+ irqreturn_t (*cb)(void *);
+};
+
+#define MVNET_QUEUE_ALIGN PAGE_SIZE
+#define MVNET_QUEUE_MAX 256
+#define MVNET_MAGIC_VALUE ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)
+#define MVNET_VERSION 0x1 /* Implies virtio 1.0 */
+#define MVNET_DEVICE_ID 0x1 /* network card */
+#define MVNET_VENDOR_ID 0 /* is this correct ? */
+#define MVNET_DEVICE_FEATURES VIRTIO_F_VERSION_1
+
+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;
+ struct virtio_mdev_callback *cbs;
+ void *buffer;
+ u32 queue_sel;
+ u32 driver_features_sel;
+ u32 driver_features[2];
+ u32 device_features_sel;
+ u32 status;
+ u32 generation;
+ u32 num;
+ 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 idx)
+{
+ struct mvnet_virtqueue *vq = &mvnet->vqs[idx];
+ int ret;
+
+ vq->desc_addr = (u64)vq->desc_addr_hi << 32 | vq->desc_addr_lo;
+ vq->device_addr = (u64)vq->device_addr_hi << 32 | vq->device_addr_lo;
+ vq->driver_addr = (u64)vq->driver_addr_hi << 32 | vq->driver_addr_lo;
+
+ ret = vringh_init_kern(&vq->vring, mvnet_features, MVNET_QUEUE_MAX,
+ false, (struct vring_desc *)vq->desc_addr,
+ (struct vring_avail *)vq->driver_addr,
+ (struct vring_used *)vq->device_addr);
+}
+
+static ssize_t mvnet_read_config(struct mdev_device *mdev,
+u32 *val, loff_t pos)
+{
+ struct mvnet_state *mvnet;
+ struct mvnet_virtqueue *vq;
+
[RFC PATCH 2/4] mdev: introduce helper to set per device dma ops
This patch introduces mdev_set_dma_ops() which allows parent to set
per device DMA ops. This help for the kernel driver to setup a correct
DMA mappings.
Signed-off-by: Jason Wang
---
drivers/vfio/mdev/mdev_core.c | 7 +++
include/linux/mdev.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..eb28552082d7 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "mdev_private.h"
@@ -27,6 +28,12 @@ static struct class_compat *mdev_bus_compat_class;
static LIST_HEAD(mdev_list);
static DEFINE_MUTEX(mdev_list_lock);
+void mdev_set_dma_ops(struct mdev_device *mdev, struct dma_map_ops *ops)
+{
+ set_dma_ops(&mdev->dev, ops);
+}
+EXPORT_SYMBOL(mdev_set_dma_ops);
+
struct device *mdev_parent_dev(struct mdev_device *mdev)
{
return mdev->parent->dev;
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0ce30ca78db0..7195f40bf8bf 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -145,4 +145,6 @@ struct device *mdev_parent_dev(struct mdev_device *mdev);
struct device *mdev_dev(struct mdev_device *mdev);
struct mdev_device *mdev_from_dev(struct device *dev);
+void mdev_set_dma_ops(struct mdev_device *mdev, struct dma_map_ops *ops);
+
#endif /* MDEV_H */
--
2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[RFC PATCH 1/4] vringh: fix copy direction of vringh_iov_push_kern()
We want to copy from iov to buf, so the direction was wrong.
Signed-off-by: Jason Wang
---
drivers/vhost/vringh.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 08ad0d1f0476..a0a2d74967ef 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -852,6 +852,12 @@ static inline int xfer_kern(void *src, void *dst, size_t
len)
return 0;
}
+static inline int kern_xfer(void *dst, void *src, size_t len)
+{
+ memcpy(dst, src, len);
+ return 0;
+}
+
/**
* vringh_init_kern - initialize a vringh for a kernelspace vring.
* @vrh: the vringh to initialize.
@@ -958,7 +964,7 @@ EXPORT_SYMBOL(vringh_iov_pull_kern);
ssize_t vringh_iov_push_kern(struct vringh_kiov *wiov,
const void *src, size_t len)
{
- return vringh_iov_xfer(wiov, (void *)src, len, xfer_kern);
+ return vringh_iov_xfer(wiov, (void *)src, len, kern_xfer);
}
EXPORT_SYMBOL(vringh_iov_push_kern);
--
2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[RFC PATCH 0/4] mdev based hardware virtio offloading support
Hi all: 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 contain 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. Notes: - Some of the key transport command for vhost-mdev(userspace driver) is not introduced. This includes: 1) set/get virtqueue state (idx etc), this could be simply done by introducing new transport command 2) dirty pages tracking, could be simply done by introducing new transport command 3) set/get device internal state, this requires more thought, of course we can introduce device specific transport command, but it would be better to have a unified API - Current mdev_parent_ops assumes all pointers are userspace pointer, this block the kernel driver, this series just abuse those as kernel pointer and this could be addressed by inventing new parent_ops. - For quick POC, mdev transport was just derived from virtio-MMIO, I'm pretty sure it has lots of space to be optimized, please share your thought. Please review. [1] https://lkml.org/lkml/2019/8/28/35 Jason Wang (4): vringh: fix copy direction of vringh_iov_push_kern() mdev: introduce helper to set per device dma ops virtio: introudce a mdev based transport docs: Sample driver to demonstrate how to implement virtio-mdev framework drivers/vfio/mdev/Kconfig| 7 + drivers/vfio/mdev/Makefile | 1 + drivers/vfio/mdev/mdev_core.c| 7 + drivers/vfio/mdev/virtio_mdev.c | 500 drivers/vhost/vringh.c | 8 +- include/linux/mdev.h | 2 + include/uapi/linux/virtio_mdev.h | 131 ++ samples/Kconfig | 7 + samples/vfio-mdev/Makefile | 1 + samples/vfio-mdev/mvnet.c| 766 +++ 10 files changed, 1429 insertions(+), 1 deletion(-) create mode 100644 drivers/vfio/mdev/virtio_mdev.c create mode 100644 include/uapi/linux/virtio_mdev.h create mode 100644 samples/vfio-mdev/mvnet.c -- 2.19.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH untested] vhost: block speculation of translated descriptors
On 2019/9/10 下午2:48, Michael S. Tsirkin wrote: On Tue, Sep 10, 2019 at 09:52:10AM +0800, Jason Wang wrote: On 2019/9/9 下午10:45, Michael S. Tsirkin wrote: On Mon, Sep 09, 2019 at 03:19:55PM +0800, Jason Wang wrote: On 2019/9/8 下午7:05, Michael S. Tsirkin wrote: iovec addresses coming from vhost are assumed to be pre-validated, but in fact can be speculated to a value out of range. Userspace address are later validated with array_index_nospec so we can be sure kernel info does not leak through these addresses, but vhost must also not leak userspace info outside the allowed memory table to guests. Following the defence in depth principle, make sure the address is not validated out of node range. Signed-off-by: Michael S. Tsirkin --- drivers/vhost/vhost.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 5dc174ac8cac..0ee375fb7145 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2072,7 +2072,9 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, size = node->size - addr + node->start; _iov->iov_len = min((u64)len - s, size); _iov->iov_base = (void __user *)(unsigned long) - (node->userspace_addr + addr - node->start); + (node->userspace_addr + +array_index_nospec(addr - node->start, + node->size)); s += size; addr += size; ++ret; I've tried this on Kaby Lake smap off metadata acceleration off using testpmd (virtio-user) + vhost_net. I don't see obvious performance difference with TX PPS. Thanks Should I push this to Linus right now then? It's a security thing so maybe we better do it ASAP ... what's your opinion? Yes, you can. Acked-by: Jason Wang And should I include Tested-by: Jason Wang ? Yes. Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
