Re: [PATCH V3 6/7] virtio: introduce a mdev based transport

2019-10-14 Thread Jason Wang



On 2019/10/15 上午1:39, Stefan Hajnoczi wrote:

On Fri, Oct 11, 2019 at 04:15:56PM +0800, Jason Wang wrote:

+struct virtio_mdev_device {
+   struct virtio_device vdev;
+   struct mdev_device *mdev;
+   unsigned long version;
+
+   struct virtqueue **vqs;
+   /* The lock to protect virtqueue list */
+   spinlock_t lock;
+   struct list_head virtqueues;

Is this a list of struct virtio_mdev_vq_info?  Please document the
actual type in a comment.



Ok.



+static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+   struct virtqueue *vqs[],
+   vq_callback_t *callbacks[],
+   const char * const names[],
+   const bool *ctx,
+   struct irq_affinity *desc)
+{
+   struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+   struct virtio_mdev_callback cb;
+   int i, err, queue_idx = 0;
+
+   vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
+   GFP_KERNEL);

kmalloc_array(0, ...)?  I would have expected nvqs instead of queue_idx
(0).

What is this the purpose of vm_dev->vqs and does anything ever access it?



It's useless, will remove it.

Thanks



Re: [PATCH V3 6/7] virtio: introduce a mdev based transport

2019-10-14 Thread Stefan Hajnoczi
On Fri, Oct 11, 2019 at 04:15:56PM +0800, Jason Wang wrote:
> +struct virtio_mdev_device {
> + struct virtio_device vdev;
> + struct mdev_device *mdev;
> + unsigned long version;
> +
> + struct virtqueue **vqs;
> + /* The lock to protect virtqueue list */
> + spinlock_t lock;
> + struct list_head virtqueues;

Is this a list of struct virtio_mdev_vq_info?  Please document the
actual type in a comment.

> +static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> + struct virtqueue *vqs[],
> + vq_callback_t *callbacks[],
> + const char * const names[],
> + const bool *ctx,
> + struct irq_affinity *desc)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_get_mdev(vdev);
> + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
> + struct virtio_mdev_callback cb;
> + int i, err, queue_idx = 0;
> +
> + vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
> + GFP_KERNEL);

kmalloc_array(0, ...)?  I would have expected nvqs instead of queue_idx
(0).

What is this the purpose of vm_dev->vqs and does anything ever access it?


signature.asc
Description: PGP signature


[PATCH V3 6/7] virtio: introduce a mdev based transport

2019-10-11 Thread Jason Wang
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 opswhich 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 | 416 +++
 3 files changed, 424 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 ..8516f3f0f658
--- /dev/null
+++ b/drivers/virtio/virtio_mdev.c
@@ -0,0 +1,416 @@
+// 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;
+
+   struct virtqueue **vqs;
+   /* The lock to protect virtqueue list */
+   spinlock_t lock;
+   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 = mdev_get_dev_ops(mdev);