* Re: [Qemu-devel] [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Pankaj Gupta @ 2019-04-11 15:57 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, KVM list, Michael S. Tsirkin, david, Qemu Developers,
virtualization, Andreas Dilger, Ross Zwisler, Andrea Arcangeli,
Dave Jiang, linux-nvdimm, Vishal L Verma, Matthew Wilcox,
Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4, Len Brown,
kilobyte, Rik van Riel, yuval shaia, Stefan Hajnoczi,
Igor Mammedov, lcapitulino, Nitesh Narayan Lal <nila>
In-Reply-To: <CAPcyv4i1FjBzfcWtzeCvj-giHa9xaX0dMQ==q4LZC_L8VEqkHg@mail.gmail.com>
> >
> > This patch adds functionality to perform flush from guest
> > to host over VIRTIO. We are registering a callback based
> > on 'nd_region' type. virtio_pmem driver requires this special
> > flush function. For rest of the region types we are registering
> > existing flush function. Report error returned by host fsync
> > failure to userspace.
> >
> > This also handles asynchronous flush requests from the block layer
> > by creating a child bio and chaining it with parent bio.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---bio_chain Dan williams
> [..]
> > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > index b4ef7d9ff22e..fb1041ab32a6 100644
> > --- a/drivers/nvdimm/region_devs.c
> > +++ b/drivers/nvdimm/region_devs.c
> > @@ -295,7 +295,9 @@ static ssize_t deep_flush_store(struct device *dev,
> > struct device_attribute *att
> > return rc;
> > if (!flush)
> > return -EINVAL;
> > - nvdimm_flush(nd_region);
> > + rc = nvdimm_flush(nd_region, NULL, false);
> > + if (rc)
> > + return rc;
> >
> > return len;
> > }
> > @@ -1085,6 +1087,11 @@ static struct nd_region *nd_region_create(struct
> > nvdimm_bus *nvdimm_bus,
> > dev->of_node = ndr_desc->of_node;
> > nd_region->ndr_size = resource_size(ndr_desc->res);
> > nd_region->ndr_start = ndr_desc->res->start;
> > + if (ndr_desc->flush)
> > + nd_region->flush = ndr_desc->flush;
> > + else
> > + nd_region->flush = generic_nvdimm_flush;
> > +
> > nd_device_register(dev);
> >
> > return nd_region;
> > @@ -1125,11 +1132,36 @@ struct nd_region
> > *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> > }
> > EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
> >
> > +int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async)
> > +{
>
> I don't quite see the point of the 'async' argument. All the usages of
> this routine are either
>
> nvdimm_flush(nd_region, bio, true)
> ...or:
> nvdimm_flush(nd_region, NULL, false)
Agree.
>
> ...so why not gate async behavior on the presence of the 'bio' argument?
Sure.
>
>
> > + int rc = 0;
> > +
> > + /* Create child bio for asynchronous flush and chain with
> > + * parent bio. Otherwise directly call nd_region flush.
> > + */
> > + if (async && bio->bi_iter.bi_sector != -1) {
> > +
> > + struct bio *child = bio_alloc(GFP_ATOMIC, 0);
> > +
> > + if (!child)
> > + return -ENOMEM;
> > + bio_copy_dev(child, bio);
> > + child->bi_opf = REQ_PREFLUSH;
> > + child->bi_iter.bi_sector = -1;
> > + bio_chain(child, bio);
> > + submit_bio(child);
>
> I understand how this works, but it's a bit too "magical" for my
> taste. I would prefer that all flush implementations take an optional
> 'bio' argument rather than rely on the make_request implementation to
> stash the bio away on a driver specific list.
I did this to make use of "bio_chain" for chaining child bio for async flush
suggested [1]. Are you saying to remove this and just call "flush" based on
bio argument? Or I implemented the 'bio_chain' request entirely wrong?
[1] https://lkml.org/lkml/2018/9/27/1028
>
> > + } else {
> > + if (nd_region->flush(nd_region))
> > + rc = -EIO;
>
> Given the common case wants to be fast and synchronous I think we
> should try to avoid retpoline overhead by default. So something like
> this:
>
> if (nd_region->flush == generic_nvdimm_flush)
> rc = generic_nvdimm_flush(...);
Sure.
Thanks,
Pankaj
>
>
^ permalink raw reply
* Re: [PATCH v5 3/6] libnvdimm: add dax_dev sync flag
From: Pankaj Gupta @ 2019-04-11 15:39 UTC (permalink / raw)
To: Dan Williams
Cc: cohuck, Jan Kara, KVM list, Michael S. Tsirkin, david,
Qemu Developers, virtualization, Andreas Dilger, Ross Zwisler,
Andrea Arcangeli, Dave Jiang, linux-nvdimm, Vishal L Verma,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4,
Len Brown, kilobyte, Rik van Riel, yuval shaia, Stefan Hajnoczi,
Paolo Bonzini, lcapitulino, Nites
In-Reply-To: <CAPcyv4gXwE2k3JU94GPav0xn2LEgR6-7ZEVYyTHFRjCfTYjerw@mail.gmail.com>
Hi Dan,
Thank you for the review.
> >
> > This patch adds 'DAXDEV_SYNC' flag which is set
> > for nd_region doing synchronous flush. This later
> > is used to disable MAP_SYNC functionality for
> > ext4 & xfs filesystem for devices don't support
> > synchronous flush.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> > drivers/dax/bus.c | 2 +-
> > drivers/dax/super.c | 13 ++++++++++++-
> > drivers/md/dm.c | 2 +-
> > drivers/nvdimm/pmem.c | 3 ++-
> > drivers/nvdimm/region_devs.c | 7 +++++++
> > include/linux/dax.h | 9 +++++++--
> > include/linux/libnvdimm.h | 1 +
> > 7 files changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index 2109cfe80219..431bf7d2a7f9 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -388,7 +388,7 @@ struct dev_dax *__devm_create_dev_dax(struct dax_region
> > *dax_region, int id,
> > * No 'host' or dax_operations since there is no access to this
> > * device outside of mmap of the resulting character device.
> > */
> > - dax_dev = alloc_dax(dev_dax, NULL, NULL);
> > + dax_dev = alloc_dax(dev_dax, NULL, NULL, true);
>
> I find apis that take a boolean as unreadable. What does 'true' mean?
> It wastes time to go look at the function definition vs something
> like:
>
> alloc_dax(dev_dax, NULL, NULL, DAXDEV_F_SYNC);
Agree. Will change as suggested.
Best regards,
Pankaj
>
^ permalink raw reply
* Re: [PATCH v5 3/6] libnvdimm: add dax_dev sync flag
From: Dan Williams @ 2019-04-11 14:56 UTC (permalink / raw)
To: Pankaj Gupta
Cc: cohuck, Jan Kara, KVM list, Michael S. Tsirkin, david,
Qemu Developers, virtualization, Andreas Dilger, Ross Zwisler,
Andrea Arcangeli, Dave Jiang, linux-nvdimm, Vishal L Verma,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4,
Len Brown, kilobyte, Rik van Riel, yuval.shaia, Stefan Hajnoczi,
Paolo Bonzini, lcapitulino, Nitesh
In-Reply-To: <20190410040826.24371-4-pagupta@redhat.com>
On Tue, Apr 9, 2019 at 9:10 PM Pankaj Gupta <pagupta@redhat.com> wrote:
>
> This patch adds 'DAXDEV_SYNC' flag which is set
> for nd_region doing synchronous flush. This later
> is used to disable MAP_SYNC functionality for
> ext4 & xfs filesystem for devices don't support
> synchronous flush.
>
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
> drivers/dax/bus.c | 2 +-
> drivers/dax/super.c | 13 ++++++++++++-
> drivers/md/dm.c | 2 +-
> drivers/nvdimm/pmem.c | 3 ++-
> drivers/nvdimm/region_devs.c | 7 +++++++
> include/linux/dax.h | 9 +++++++--
> include/linux/libnvdimm.h | 1 +
> 7 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 2109cfe80219..431bf7d2a7f9 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -388,7 +388,7 @@ struct dev_dax *__devm_create_dev_dax(struct dax_region *dax_region, int id,
> * No 'host' or dax_operations since there is no access to this
> * device outside of mmap of the resulting character device.
> */
> - dax_dev = alloc_dax(dev_dax, NULL, NULL);
> + dax_dev = alloc_dax(dev_dax, NULL, NULL, true);
I find apis that take a boolean as unreadable. What does 'true' mean?
It wastes time to go look at the function definition vs something
like:
alloc_dax(dev_dax, NULL, NULL, DAXDEV_F_SYNC);
^ permalink raw reply
* Re: [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Dan Williams @ 2019-04-11 14:51 UTC (permalink / raw)
To: Pankaj Gupta
Cc: cohuck, Jan Kara, KVM list, Michael S. Tsirkin, david,
Qemu Developers, virtualization, Andreas Dilger, Ross Zwisler,
Andrea Arcangeli, Dave Jiang, linux-nvdimm, Vishal L Verma,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4,
Len Brown, kilobyte, Rik van Riel, yuval.shaia, Stefan Hajnoczi,
Paolo Bonzini, lcapitulino, Nitesh
In-Reply-To: <20190410040826.24371-2-pagupta@redhat.com>
On Tue, Apr 9, 2019 at 9:09 PM Pankaj Gupta <pagupta@redhat.com> wrote:
>
> This patch adds functionality to perform flush from guest
> to host over VIRTIO. We are registering a callback based
> on 'nd_region' type. virtio_pmem driver requires this special
> flush function. For rest of the region types we are registering
> existing flush function. Report error returned by host fsync
> failure to userspace.
>
> This also handles asynchronous flush requests from the block layer
> by creating a child bio and chaining it with parent bio.
>
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
[..]
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index b4ef7d9ff22e..fb1041ab32a6 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -295,7 +295,9 @@ static ssize_t deep_flush_store(struct device *dev, struct device_attribute *att
> return rc;
> if (!flush)
> return -EINVAL;
> - nvdimm_flush(nd_region);
> + rc = nvdimm_flush(nd_region, NULL, false);
> + if (rc)
> + return rc;
>
> return len;
> }
> @@ -1085,6 +1087,11 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
> dev->of_node = ndr_desc->of_node;
> nd_region->ndr_size = resource_size(ndr_desc->res);
> nd_region->ndr_start = ndr_desc->res->start;
> + if (ndr_desc->flush)
> + nd_region->flush = ndr_desc->flush;
> + else
> + nd_region->flush = generic_nvdimm_flush;
> +
> nd_device_register(dev);
>
> return nd_region;
> @@ -1125,11 +1132,36 @@ struct nd_region *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> }
> EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
>
> +int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async)
> +{
I don't quite see the point of the 'async' argument. All the usages of
this routine are either
nvdimm_flush(nd_region, bio, true)
...or:
nvdimm_flush(nd_region, NULL, false)
...so why not gate async behavior on the presence of the 'bio' argument?
> + int rc = 0;
> +
> + /* Create child bio for asynchronous flush and chain with
> + * parent bio. Otherwise directly call nd_region flush.
> + */
> + if (async && bio->bi_iter.bi_sector != -1) {
> +
> + struct bio *child = bio_alloc(GFP_ATOMIC, 0);
> +
> + if (!child)
> + return -ENOMEM;
> + bio_copy_dev(child, bio);
> + child->bi_opf = REQ_PREFLUSH;
> + child->bi_iter.bi_sector = -1;
> + bio_chain(child, bio);
> + submit_bio(child);
I understand how this works, but it's a bit too "magical" for my
taste. I would prefer that all flush implementations take an optional
'bio' argument rather than rely on the make_request implementation to
stash the bio away on a driver specific list.
> + } else {
> + if (nd_region->flush(nd_region))
> + rc = -EIO;
Given the common case wants to be fast and synchronous I think we
should try to avoid retpoline overhead by default. So something like
this:
if (nd_region->flush == generic_nvdimm_flush)
rc = generic_nvdimm_flush(...);
^ permalink raw reply
* Re: [RFC PATCH 05/12] s390/cio: add protected virtualization support to cio
From: Sebastian Ott @ 2019-04-11 14:15 UTC (permalink / raw)
To: Halil Pasic
Cc: Farhan Ali, linux-s390, Eric Farman, Claudio Imbrenda,
Vasily Gorbik, kvm, Cornelia Huck, virtualization,
Martin Schwidefsky, Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190404231622.52531-6-pasic@linux.ibm.com>
On Fri, 5 Apr 2019, Halil Pasic wrote:
> @@ -1593,20 +1609,29 @@ struct ccw_device * __init ccw_device_create_console(struct ccw_driver *drv)
> return ERR_CAST(sch);
>
> io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
> - if (!io_priv) {
> - put_device(&sch->dev);
> - return ERR_PTR(-ENOMEM);
> - }
> + if (!io_priv)
> + goto err_priv;
> + io_priv->dma_area = cio_dma_zalloc(sizeof(*io_priv->dma_area));
This is called very early - the dma pool is not yet initialized.
^ permalink raw reply
* [RFC 3/3] RDMA/virtio-rdma: VirtIO rdma driver
From: Yuval Shaia @ 2019-04-11 11:01 UTC (permalink / raw)
To: virtualization, qemu-devel, mst, cohuck, marcel.apfelbaum,
linux-rdma, jgg
Cc: Yuval Shaia
In-Reply-To: <20190411110157.14252-1-yuval.shaia@oracle.com>
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
drivers/infiniband/Kconfig | 1 +
drivers/infiniband/hw/Makefile | 1 +
drivers/infiniband/hw/virtio/Kconfig | 6 +
drivers/infiniband/hw/virtio/Makefile | 4 +
drivers/infiniband/hw/virtio/virtio_rdma.h | 40 +
.../infiniband/hw/virtio/virtio_rdma_device.c | 59 ++
.../infiniband/hw/virtio/virtio_rdma_device.h | 32 +
drivers/infiniband/hw/virtio/virtio_rdma_ib.c | 711 ++++++++++++++++++
drivers/infiniband/hw/virtio/virtio_rdma_ib.h | 48 ++
.../infiniband/hw/virtio/virtio_rdma_main.c | 149 ++++
.../infiniband/hw/virtio/virtio_rdma_netdev.c | 44 ++
.../infiniband/hw/virtio/virtio_rdma_netdev.h | 33 +
include/uapi/linux/virtio_ids.h | 1 +
13 files changed, 1129 insertions(+)
create mode 100644 drivers/infiniband/hw/virtio/Kconfig
create mode 100644 drivers/infiniband/hw/virtio/Makefile
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma.h
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_device.c
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_device.h
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_ib.c
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_ib.h
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_main.c
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index a1fb840de45d..218a47d4cecf 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -107,6 +107,7 @@ source "drivers/infiniband/hw/hfi1/Kconfig"
source "drivers/infiniband/hw/qedr/Kconfig"
source "drivers/infiniband/sw/rdmavt/Kconfig"
source "drivers/infiniband/sw/rxe/Kconfig"
+source "drivers/infiniband/hw/virtio/Kconfig"
endif
source "drivers/infiniband/ulp/ipoib/Kconfig"
diff --git a/drivers/infiniband/hw/Makefile b/drivers/infiniband/hw/Makefile
index e4f31c1be8f7..10ffb2c421e4 100644
--- a/drivers/infiniband/hw/Makefile
+++ b/drivers/infiniband/hw/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_INFINIBAND_HFI1) += hfi1/
obj-$(CONFIG_INFINIBAND_HNS) += hns/
obj-$(CONFIG_INFINIBAND_QEDR) += qedr/
obj-$(CONFIG_INFINIBAND_BNXT_RE) += bnxt_re/
+obj-$(CONFIG_INFINIBAND_VIRTIO_RDMA) += virtio/
diff --git a/drivers/infiniband/hw/virtio/Kconfig b/drivers/infiniband/hw/virtio/Kconfig
new file mode 100644
index 000000000000..92e41691cf5d
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/Kconfig
@@ -0,0 +1,6 @@
+config INFINIBAND_VIRTIO_RDMA
+ tristate "VirtIO Paravirtualized RDMA Driver"
+ depends on NETDEVICES && ETHERNET && PCI && INET
+ ---help---
+ This driver provides low-level support for VirtIO Paravirtual
+ RDMA adapter.
diff --git a/drivers/infiniband/hw/virtio/Makefile b/drivers/infiniband/hw/virtio/Makefile
new file mode 100644
index 000000000000..fb637e467167
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/Makefile
@@ -0,0 +1,4 @@
+obj-$(CONFIG_INFINIBAND_VIRTIO_RDMA) += virtio_rdma.o
+
+virtio_rdma-y := virtio_rdma_main.o virtio_rdma_device.o virtio_rdma_ib.o \
+ virtio_rdma_netdev.o
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma.h b/drivers/infiniband/hw/virtio/virtio_rdma.h
new file mode 100644
index 000000000000..7896a2dfb812
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma.h
@@ -0,0 +1,40 @@
+/*
+ * Virtio RDMA device: Driver main data types
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef __VIRTIO_RDMA__
+#define __VIRTIO_RDMA__
+
+#include <linux/virtio.h>
+#include <rdma/ib_verbs.h>
+
+struct virtio_rdma_info {
+ struct ib_device ib_dev;
+ struct virtio_device *vdev;
+ struct virtqueue *ctrl_vq;
+ wait_queue_head_t acked; /* arm on send to host, release on recv */
+ struct net_device *netdev;
+};
+
+static inline struct virtio_rdma_info *to_vdev(struct ib_device *ibdev)
+{
+ return container_of(ibdev, struct virtio_rdma_info, ib_dev);
+}
+
+#endif
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_device.c b/drivers/infiniband/hw/virtio/virtio_rdma_device.c
new file mode 100644
index 000000000000..ae41e530644f
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_device.c
@@ -0,0 +1,59 @@
+/*
+ * Virtio RDMA device: Device related functions and data
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <linux/virtio_config.h>
+
+#include "virtio_rdma.h"
+
+static void rdma_ctrl_ack(struct virtqueue *vq)
+{
+ struct virtio_rdma_info *dev = vq->vdev->priv;
+
+ wake_up(&dev->acked);
+
+ printk("%s\n", __func__);
+}
+
+int init_device(struct virtio_rdma_info *dev)
+{
+#define TMP_MAX_VQ 1
+ int rc;
+ struct virtqueue *vqs[TMP_MAX_VQ];
+ vq_callback_t *cbs[TMP_MAX_VQ];
+ const char *names[TMP_MAX_VQ];
+
+ names[0] = "ctrl";
+ cbs[0] = rdma_ctrl_ack;
+ cbs[0] = NULL;
+
+ rc = virtio_find_vqs(dev->vdev, TMP_MAX_VQ, vqs, cbs, names, NULL);
+ if (rc)
+ return rc;
+
+ dev->ctrl_vq = vqs[0];
+
+ return 0;
+}
+
+void fini_device(struct virtio_rdma_info *dev)
+{
+ dev->vdev->config->reset(dev->vdev);
+ dev->vdev->config->del_vqs(dev->vdev);
+}
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_device.h b/drivers/infiniband/hw/virtio/virtio_rdma_device.h
new file mode 100644
index 000000000000..d9b1240daf92
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_device.h
@@ -0,0 +1,32 @@
+/*
+ * Virtio RDMA device: Device related functions and data
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef __VIRTIO_RDMA_DEVICE__
+#define __VIRTIO_RDMA_DEVICE__
+
+#define VIRTIO_RDMA_BOARD_ID 1
+#define VIRTIO_RDMA_HW_NAME "virtio-rdma"
+#define VIRTIO_RDMA_HW_REV 1
+#define VIRTIO_RDMA_DRIVER_VER "1.0"
+
+int init_device(struct virtio_rdma_info *dev);
+void fini_device(struct virtio_rdma_info *dev);
+
+#endif
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_ib.c b/drivers/infiniband/hw/virtio/virtio_rdma_ib.c
new file mode 100644
index 000000000000..02bf4a332611
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_ib.c
@@ -0,0 +1,711 @@
+/*
+ * Virtio RDMA device: IB related functions and data
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <linux/scatterlist.h>
+#include <linux/virtio.h>
+#include <rdma/ib_mad.h>
+
+#include "virtio_rdma.h"
+#include "virtio_rdma_device.h"
+#include "virtio_rdma_ib.h"
+
+/* TODO: Move to uapi header file */
+
+/*
+ * Control virtqueue data structures
+ *
+ * The control virtqueue expects a header in the first sg entry
+ * and an ack/status response in the last entry. Data for the
+ * command goes in between.
+ */
+
+#define VIRTIO_RDMA_CTRL_OK 0
+#define VIRTIO_RDMA_CTRL_ERR 1
+
+struct control_buf {
+ __u8 cmd;
+ __u8 status;
+};
+
+enum {
+ VIRTIO_CMD_QUERY_DEVICE = 10,
+ VIRTIO_CMD_QUERY_PORT,
+ VIRTIO_CMD_CREATE_CQ,
+ VIRTIO_CMD_DESTROY_CQ,
+ VIRTIO_CMD_CREATE_PD,
+ VIRTIO_CMD_DESTROY_PD,
+ VIRTIO_CMD_GET_DMA_MR,
+};
+
+struct cmd_query_port {
+ __u8 port;
+};
+
+struct cmd_create_cq {
+ __u32 cqe;
+};
+
+struct rsp_create_cq {
+ __u32 cqn;
+};
+
+struct cmd_destroy_cq {
+ __u32 cqn;
+};
+
+struct rsp_create_pd {
+ __u32 pdn;
+};
+
+struct cmd_destroy_pd {
+ __u32 pdn;
+};
+
+struct cmd_get_dma_mr {
+ __u32 pdn;
+ __u32 access_flags;
+};
+
+struct rsp_get_dma_mr {
+ __u32 mrn;
+ __u32 lkey;
+ __u32 rkey;
+};
+
+/* TODO: Move to uapi header file */
+
+struct virtio_rdma_ib_cq {
+ struct ib_cq ibcq;
+ u32 cq_handle;
+};
+
+/* TODO: For the scope fof the RFC i'm utilizing ib*_*_attr structures */
+
+static int virtio_rdma_exec_cmd(struct virtio_rdma_info *di, int cmd,
+ struct scatterlist *in, struct scatterlist *out)
+{
+ struct scatterlist *sgs[4], hdr, status;
+ struct control_buf *ctrl;
+ unsigned tmp;
+ int rc;
+
+ ctrl = kmalloc(sizeof(*ctrl), GFP_ATOMIC);
+ ctrl->cmd = cmd;
+ ctrl->status = ~0;
+
+ sg_init_one(&hdr, &ctrl->cmd, sizeof(ctrl->cmd));
+ sgs[0] = &hdr;
+ sgs[1] = in;
+ sgs[2] = out;
+ sg_init_one(&status, &ctrl->status, sizeof(ctrl->status));
+ sgs[3] = &status;
+
+ rc = virtqueue_add_sgs(di->ctrl_vq, sgs, 2, 2, di, GFP_ATOMIC);
+ if (rc)
+ goto out;
+
+ if (unlikely(!virtqueue_kick(di->ctrl_vq))) {
+ goto out_with_status;
+ }
+
+ /* Spin for a response, the kick causes an ioport write, trapping
+ * into the hypervisor, so the request should be handled
+ * immediately */
+ while (!virtqueue_get_buf(di->ctrl_vq, &tmp) &&
+ !virtqueue_is_broken(di->ctrl_vq))
+ cpu_relax();
+
+out_with_status:
+ printk("%s: cmd %d, status %d\n", __func__, ctrl->cmd, ctrl->status);
+ rc = ctrl->status == VIRTIO_RDMA_CTRL_OK ? 0 : 1;
+
+out:
+ kfree(ctrl);
+
+ return rc;
+}
+
+static int virtio_rdma_port_immutable(struct ib_device *ibdev, u8 port_num,
+ struct ib_port_immutable *immutable)
+{
+ struct ib_port_attr attr;
+ int rc;
+
+ rc = ib_query_port(ibdev, port_num, &attr);
+ if (rc)
+ return rc;
+
+ immutable->core_cap_flags |= RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
+ immutable->pkey_tbl_len = attr.pkey_tbl_len;
+ immutable->gid_tbl_len = attr.gid_tbl_len;
+ immutable->max_mad_size = IB_MGMT_MAD_SIZE;
+
+ return 0;
+}
+
+static int virtio_rdma_query_device(struct ib_device *ibdev,
+ struct ib_device_attr *props,
+ struct ib_udata *uhw)
+{
+ struct scatterlist data;
+ int offs;
+ int rc;
+
+ if (uhw->inlen || uhw->outlen)
+ return -EINVAL;
+
+ /* We start with sys_image_guid because of inconsistency beween ib_
+ * and ibv_ */
+ offs = offsetof(struct ib_device_attr, sys_image_guid);
+ sg_init_one(&data, (void *)props + offs, sizeof(*props) - offs);
+
+ rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_QUERY_DEVICE, NULL,
+ &data);
+
+ printk("%s: sys_image_guid 0x%llx\n", __func__,
+ be64_to_cpu(props->sys_image_guid));
+
+ return rc;
+}
+
+static int virtio_rdma_query_port(struct ib_device *ibdev, u8 port,
+ struct ib_port_attr *props)
+{
+ struct scatterlist in, out;
+ struct cmd_query_port *cmd;
+ int offs;
+ int rc;
+
+ cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+ if (!cmd)
+ return -ENOMEM;
+
+ /* We start with state because of inconsistency beween ib and ibv */
+ offs = offsetof(struct ib_port_attr, state);
+ sg_init_one(&out, (void *)props + offs, sizeof(*props) - offs);
+
+ cmd->port = port;
+ sg_init_one(&in, cmd, sizeof(*cmd));
+ printk("%s: port %d\n", __func__, cmd->port);
+
+ rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_QUERY_PORT, &in,
+ &out);
+
+ printk("%s: gid_tbl_len %d\n", __func__, props->gid_tbl_len);
+
+ kfree(cmd);
+
+ return rc;
+}
+
+static struct net_device *virtio_rdma_get_netdev(struct ib_device *ibdev,
+ u8 port_num)
+{
+ struct virtio_rdma_info *ri = to_vdev(ibdev);
+
+ printk("%s:\n", __func__);
+
+ return ri->netdev;
+}
+
+struct ib_cq *virtio_rdma_create_cq(struct ib_device *ibdev,
+ const struct ib_cq_init_attr *attr,
+ struct ib_ucontext *context,
+ struct ib_udata *udata)
+{
+ struct scatterlist in, out;
+ struct virtio_rdma_ib_cq *vcq;
+ struct cmd_create_cq *cmd;
+ struct rsp_create_cq *rsp;
+ struct ib_cq *cq = NULL;
+ int rc;
+
+ /* TODO: Check MAX_CQ */
+
+ cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+ if (!cmd)
+ return ERR_PTR(-ENOMEM);
+
+ rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
+ if (!rsp) {
+ kfree(cmd);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ vcq = kzalloc(sizeof(*vcq), GFP_KERNEL);
+ if (!vcq)
+ goto out;
+
+ cmd->cqe = attr->cqe;
+ sg_init_one(&in, cmd, sizeof(*cmd));
+ printk("%s: cqe %d\n", __func__, cmd->cqe);
+
+ sg_init_one(&out, rsp, sizeof(*rsp));
+
+ rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_CREATE_CQ, &in,
+ &out);
+ if (rc)
+ goto out_err;
+
+ printk("%s: cqn 0x%x\n", __func__, rsp->cqn);
+ vcq->cq_handle = rsp->cqn;
+ vcq->ibcq.cqe = attr->cqe;
+ cq = &vcq->ibcq;
+
+ goto out;
+
+out_err:
+ kfree(vcq);
+ return ERR_PTR(rc);
+
+out:
+ kfree(rsp);
+ kfree(cmd);
+ return cq;
+}
+
+int virtio_rdma_destroy_cq(struct ib_cq *cq)
+{
+ struct virtio_rdma_ib_cq *vcq;
+ struct scatterlist in;
+ struct cmd_destroy_cq *cmd;
+ int rc;
+
+ cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+ if (!cmd)
+ return -ENOMEM;
+
+ vcq = container_of(cq, struct virtio_rdma_ib_cq, ibcq);
+
+ cmd->cqn = vcq->cq_handle;
+ sg_init_one(&in, cmd, sizeof(*cmd));
+
+ rc = virtio_rdma_exec_cmd(to_vdev(cq->device), VIRTIO_CMD_DESTROY_CQ,
+ &in, NULL);
+
+ kfree(cmd);
+
+ kfree(vcq);
+
+ return rc;
+}
+
+int virtio_rdma_alloc_pd(struct ib_pd *ibpd, struct ib_ucontext *context,
+ struct ib_udata *udata)
+{
+ struct virtio_rdma_pd *pd = to_vpd(ibpd);
+ struct ib_device *ibdev = ibpd->device;
+ struct rsp_create_pd *rsp;
+ struct scatterlist out;
+ int rc;
+
+ /* TODO: Check MAX_PD */
+
+ rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
+ if (!rsp)
+ return -ENOMEM;
+
+ sg_init_one(&out, rsp, sizeof(*rsp));
+
+ rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_CREATE_PD, NULL,
+ &out);
+ if (rc)
+ goto out;
+
+ pd->pd_handle = rsp->pdn;
+
+ printk("%s: pd_handle=%d\n", __func__, pd->pd_handle);
+
+out:
+ kfree(rsp);
+
+ printk("%s: rc=%d\n", __func__, rc);
+ return rc;
+}
+
+void virtio_rdma_dealloc_pd(struct ib_pd *pd)
+{
+ struct virtio_rdma_pd *vpd = to_vpd(pd);
+ struct ib_device *ibdev = pd->device;
+ struct cmd_destroy_pd *cmd;
+ struct scatterlist in;
+
+ printk("%s:\n", __func__);
+
+ cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+ if (!cmd)
+ return;
+
+ cmd->pdn = vpd->pd_handle;
+ sg_init_one(&in, cmd, sizeof(*cmd));
+
+ virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_DESTROY_PD, &in, NULL);
+
+ kfree(cmd);
+}
+
+struct ib_mr *virtio_rdma_get_dma_mr(struct ib_pd *pd, int acc)
+
+{
+ struct virtio_rdma_user_mr *mr;
+ struct scatterlist in, out;
+ struct cmd_get_dma_mr *cmd = NULL;
+ struct rsp_get_dma_mr *rsp = NULL;
+ int rc;
+
+ mr = kzalloc(sizeof(*mr), GFP_KERNEL);
+ if (!mr)
+ return ERR_PTR(-ENOMEM);
+
+ cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+ if (!cmd) {
+ kfree(mr);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
+ if (!cmd) {
+ kfree(mr);
+ kfree(cmd);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ cmd->pdn = to_vpd(pd)->pd_handle;
+ cmd->access_flags = acc;
+ sg_init_one(&in, cmd, sizeof(*cmd));
+
+ sg_init_one(&out, rsp, sizeof(*rsp));
+
+ rc = virtio_rdma_exec_cmd(to_vdev(pd->device), VIRTIO_CMD_GET_DMA_MR,
+ &in, &out);
+ if (rc) {
+ kfree(mr);
+ kfree(cmd);
+ return ERR_PTR(rc);
+ }
+
+ mr->mr_handle = rsp->mrn;
+ mr->ibmr.lkey = rsp->lkey;
+ mr->ibmr.rkey = rsp->rkey;
+
+ printk("%s: mr_handle=0x%x\n", __func__, mr->mr_handle);
+
+ kfree(cmd);
+ kfree(rsp);
+
+ return &mr->ibmr;
+}
+
+struct ib_qp *virtio_rdma_create_qp(struct ib_pd *pd,
+ struct ib_qp_init_attr *init_attr,
+ struct ib_udata *udata)
+{
+ /* struct pvrdma_dev *dev = to_vdev(pd->device); */
+ struct virtio_rdma_qp *qp;
+
+ printk("%s:\n", __func__);
+
+ qp = kzalloc(sizeof(*qp), GFP_KERNEL);
+ if (!qp)
+ return ERR_PTR(-ENOMEM);
+
+ return &qp->ibqp;
+}
+
+int virtio_rdma_query_gid(struct ib_device *ibdev, u8 port, int index,
+ union ib_gid *gid)
+{
+ memset(gid, 0, sizeof(union ib_gid));
+
+ printk("%s: port %d, index %d\n", __func__, port, index);
+
+ return 0;
+}
+
+static int virtio_rdma_add_gid(const struct ib_gid_attr *attr, void **context)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+struct ib_mr *virtio_rdma_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
+ u32 max_num_sg)
+{
+ printk("%s: mr_type %d, max_num_sg %d\n", __func__, mr_type,
+ max_num_sg);
+
+ return NULL;
+}
+
+int virtio_rdma_alloc_ucontext(struct ib_ucontext *uctx, struct ib_udata *udata)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+struct ib_ah *virtio_rdma_create_ah(struct ib_pd *pd,
+ struct rdma_ah_attr *ah_attr, u32 flags,
+ struct ib_udata *udata)
+{
+ printk("%s:\n", __func__);
+
+ return NULL;
+}
+
+void virtio_rdma_dealloc_ucontext(struct ib_ucontext *ibcontext)
+
+{
+}
+
+static int virtio_rdma_del_gid(const struct ib_gid_attr *attr, void **context)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_dereg_mr(struct ib_mr *ibmr)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_destroy_ah(struct ib_ah *ah, u32 flags)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+struct virtio_rdma_cq {
+ struct ib_cq ibcq;
+};
+
+int virtio_rdma_destroy_qp(struct ib_qp *qp)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+static void virtio_rdma_get_fw_ver_str(struct ib_device *device, char *str)
+{
+ printk("%s:\n", __func__);
+}
+
+enum rdma_link_layer virtio_rdma_port_link_layer(struct ib_device *ibdev,
+ u8 port)
+{
+ return IB_LINK_LAYER_ETHERNET;
+}
+
+int virtio_rdma_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg,
+ int sg_nents, unsigned int *sg_offset)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_mmap(struct ib_ucontext *ibcontext, struct vm_area_struct *vma)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_modify_port(struct ib_device *ibdev, u8 port, int mask,
+ struct ib_port_modify *props)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
+ int attr_mask, struct ib_udata *udata)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *wr,
+ const struct ib_recv_wr **bad_wr)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
+ const struct ib_send_wr **bad_wr)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
+ u16 *pkey)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+int virtio_rdma_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
+ int attr_mask, struct ib_qp_init_attr *init_attr)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+struct ib_mr *virtio_rdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
+ u64 virt_addr, int access_flags,
+ struct ib_udata *udata)
+{
+ printk("%s:\n", __func__);
+
+ return NULL;
+}
+
+int virtio_rdma_req_notify_cq(struct ib_cq *ibcq,
+ enum ib_cq_notify_flags notify_flags)
+{
+ printk("%s:\n", __func__);
+
+ return 0;
+}
+
+static const struct ib_device_ops virtio_rdma_dev_ops = {
+ .get_port_immutable = virtio_rdma_port_immutable,
+ .query_device = virtio_rdma_query_device,
+ .query_port = virtio_rdma_query_port,
+ .get_netdev = virtio_rdma_get_netdev,
+ .create_cq = virtio_rdma_create_cq,
+ .destroy_cq = virtio_rdma_destroy_cq,
+ .alloc_pd = virtio_rdma_alloc_pd,
+ .dealloc_pd = virtio_rdma_dealloc_pd,
+ .get_dma_mr = virtio_rdma_get_dma_mr,
+ .create_qp = virtio_rdma_create_qp,
+ .query_gid = virtio_rdma_query_gid,
+ .add_gid = virtio_rdma_add_gid,
+ .alloc_mr = virtio_rdma_alloc_mr,
+ .alloc_ucontext = virtio_rdma_alloc_ucontext,
+ .create_ah = virtio_rdma_create_ah,
+ .dealloc_ucontext = virtio_rdma_dealloc_ucontext,
+ .del_gid = virtio_rdma_del_gid,
+ .dereg_mr = virtio_rdma_dereg_mr,
+ .destroy_ah = virtio_rdma_destroy_ah,
+ .destroy_qp = virtio_rdma_destroy_qp,
+ .get_dev_fw_str = virtio_rdma_get_fw_ver_str,
+ .get_link_layer = virtio_rdma_port_link_layer,
+ .get_port_immutable = virtio_rdma_port_immutable,
+ .map_mr_sg = virtio_rdma_map_mr_sg,
+ .mmap = virtio_rdma_mmap,
+ .modify_port = virtio_rdma_modify_port,
+ .modify_qp = virtio_rdma_modify_qp,
+ .poll_cq = virtio_rdma_poll_cq,
+ .post_recv = virtio_rdma_post_recv,
+ .post_send = virtio_rdma_post_send,
+ .query_device = virtio_rdma_query_device,
+ .query_pkey = virtio_rdma_query_pkey,
+ .query_port = virtio_rdma_query_port,
+ .query_qp = virtio_rdma_query_qp,
+ .reg_user_mr = virtio_rdma_reg_user_mr,
+ .req_notify_cq = virtio_rdma_req_notify_cq,
+ INIT_RDMA_OBJ_SIZE(ib_pd, virtio_rdma_pd, ibpd),
+};
+
+static ssize_t hca_type_show(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%s-%s\n", VIRTIO_RDMA_HW_NAME,
+ VIRTIO_RDMA_DRIVER_VER);
+}
+static DEVICE_ATTR_RO(hca_type);
+
+static ssize_t hw_rev_show(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", VIRTIO_RDMA_HW_REV);
+}
+static DEVICE_ATTR_RO(hw_rev);
+
+static ssize_t board_id_show(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", VIRTIO_RDMA_BOARD_ID);
+}
+static DEVICE_ATTR_RO(board_id);
+
+static struct attribute *virtio_rdmaa_class_attributes[] = {
+ &dev_attr_hw_rev.attr,
+ &dev_attr_hca_type.attr,
+ &dev_attr_board_id.attr,
+ NULL,
+};
+
+static const struct attribute_group virtio_rdmaa_attr_group = {
+ .attrs = virtio_rdmaa_class_attributes,
+};
+
+int init_ib(struct virtio_rdma_info *ri)
+{
+ int rc;
+
+ ri->ib_dev.owner = THIS_MODULE;
+ ri->ib_dev.num_comp_vectors = 1;
+ ri->ib_dev.dev.parent = &ri->vdev->dev;
+ ri->ib_dev.node_type = RDMA_NODE_IB_CA;
+ ri->ib_dev.phys_port_cnt = 1;
+ ri->ib_dev.uverbs_cmd_mask =
+ (1ull << IB_USER_VERBS_CMD_QUERY_DEVICE) |
+ (1ull << IB_USER_VERBS_CMD_QUERY_PORT) |
+ (1ull << IB_USER_VERBS_CMD_CREATE_CQ) |
+ (1ull << IB_USER_VERBS_CMD_DESTROY_CQ) |
+ (1ull << IB_USER_VERBS_CMD_ALLOC_PD) |
+ (1ull << IB_USER_VERBS_CMD_DEALLOC_PD);
+
+ rdma_set_device_sysfs_group(&ri->ib_dev, &virtio_rdmaa_attr_group);
+
+ ib_set_device_ops(&ri->ib_dev, &virtio_rdma_dev_ops);
+
+ rc = ib_register_device(&ri->ib_dev, "virtio_rdma%d");
+
+ return rc;
+}
+
+void fini_ib(struct virtio_rdma_info *ri)
+{
+ ib_unregister_device(&ri->ib_dev);
+}
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_ib.h b/drivers/infiniband/hw/virtio/virtio_rdma_ib.h
new file mode 100644
index 000000000000..7b82a60581ff
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_ib.h
@@ -0,0 +1,48 @@
+/*
+ * Virtio RDMA device: IB related functions and data
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef __VIRTIO_RDMA_IB__
+#define __VIRTIO_RDMA_IB__
+
+#include <rdma/ib_verbs.h>
+
+struct virtio_rdma_pd {
+ struct ib_pd ibpd;
+ u32 pd_handle;
+};
+
+struct virtio_rdma_user_mr {
+ struct ib_mr ibmr;
+ u32 mr_handle;
+};
+
+struct virtio_rdma_qp {
+ struct ib_qp ibqp;
+};
+
+static inline struct virtio_rdma_pd *to_vpd(struct ib_pd *ibpd)
+{
+ return container_of(ibpd, struct virtio_rdma_pd, ibpd);
+}
+
+int init_ib(struct virtio_rdma_info *ri);
+void fini_ib(struct virtio_rdma_info *ri);
+
+#endif
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_main.c b/drivers/infiniband/hw/virtio/virtio_rdma_main.c
new file mode 100644
index 000000000000..811533d63160
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_main.c
@@ -0,0 +1,149 @@
+/*
+ * Virtio RDMA device
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <linux/err.h>
+#include <linux/scatterlist.h>
+#include <linux/spinlock.h>
+#include <linux/virtio.h>
+#include <linux/module.h>
+#include <uapi/linux/virtio_ids.h>
+
+#include "virtio_rdma.h"
+#include "virtio_rdma_device.h"
+#include "virtio_rdma_ib.h"
+#include "virtio_rdma_netdev.h"
+
+/* TODO:
+ * - How to hook to unload driver, we need to undo all the stuff with did
+ * for all the devices that probed
+ * -
+ */
+
+static int virtio_rdma_probe(struct virtio_device *vdev)
+{
+ struct virtio_rdma_info *ri;
+ int rc = -EIO;
+
+ ri = ib_alloc_device(virtio_rdma_info, ib_dev);
+ if (!ri) {
+ pr_err("Fail to allocate IB device\n");
+ rc = -ENOMEM;
+ goto out;
+ }
+ vdev->priv = ri;
+
+ ri->vdev = vdev;
+
+ rc = init_device(ri);
+ if (rc) {
+ pr_err("Fail to connect to device\n");
+ goto out_dealloc_ib_device;
+ }
+
+ rc = init_netdev(ri);
+ if (rc) {
+ pr_err("Fail to connect to NetDev layer\n");
+ goto out_fini_device;
+ }
+
+ rc = init_ib(ri);
+ if (rc) {
+ pr_err("Fail to connect to IB layer\n");
+ goto out_fini_netdev;
+ }
+
+ pr_info("VirtIO RDMA device %d probed\n", vdev->index);
+
+ goto out;
+
+out_fini_netdev:
+ fini_netdev(ri);
+
+out_fini_device:
+ fini_device(ri);
+
+out_dealloc_ib_device:
+ ib_dealloc_device(&ri->ib_dev);
+
+ vdev->priv = NULL;
+
+out:
+ return rc;
+}
+
+static void virtio_rdma_remove(struct virtio_device *vdev)
+{
+ struct virtio_rdma_info *ri = vdev->priv;
+
+ if (!ri)
+ return;
+
+ vdev->priv = NULL;
+
+ fini_ib(ri);
+
+ fini_netdev(ri);
+
+ fini_device(ri);
+
+ ib_dealloc_device(&ri->ib_dev);
+
+ pr_info("VirtIO RDMA device %d removed\n", vdev->index);
+}
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_RDMA, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_rdma_driver = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .id_table = id_table,
+ .probe = virtio_rdma_probe,
+ .remove = virtio_rdma_remove,
+};
+
+static int __init virtio_rdma_init(void)
+{
+ int rc;
+
+ rc = register_virtio_driver(&virtio_rdma_driver);
+ if (rc) {
+ pr_err("%s: Fail to register virtio driver (%d)\n", __func__,
+ rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+static void __exit virtio_rdma_fini(void)
+{
+ unregister_virtio_driver(&virtio_rdma_driver);
+}
+
+module_init(virtio_rdma_init);
+module_exit(virtio_rdma_fini);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_AUTHOR("Yuval Shaia");
+MODULE_DESCRIPTION("Virtio RDMA driver");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
new file mode 100644
index 000000000000..001f30b3e0b9
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
@@ -0,0 +1,44 @@
+/*
+ * Virtio RDMA device
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "virtio_rdma_netdev.h"
+
+int init_netdev(struct virtio_rdma_info *ri)
+{
+ struct net_device *dev;
+ struct virtio_rdma_netdev_info *vrndi;
+
+ dev = alloc_etherdev(sizeof(struct virtio_rdma_netdev_info));
+ if (!dev) {
+ return -ENOMEM;
+ }
+
+ SET_NETDEV_DEV(dev, &ri->vdev->dev);
+ vrndi = netdev_priv(dev);
+ vrndi->ri = ri;
+ ri->netdev = dev;
+
+ return 0;
+}
+
+void fini_netdev(struct virtio_rdma_info *ri)
+{
+ unregister_netdev(ri->netdev);
+}
diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
new file mode 100644
index 000000000000..e7e5d276d8ec
--- /dev/null
+++ b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
@@ -0,0 +1,33 @@
+/*
+ * Virtio RDMA device: Netdev related functions and data
+ *
+ * Copyright (C) 2019 Yuval Shaia Oracle Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef __VIRTIO_RDMA_NETDEV__
+#define __VIRTIO_RDMA_NETDEV__
+
+#include "virtio_rdma.h"
+
+struct virtio_rdma_netdev_info {
+ struct virtio_rdma_info *ri;
+};
+
+int init_netdev(struct virtio_rdma_info *ri);
+void fini_netdev(struct virtio_rdma_info *ri);
+
+#endif
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 6d5c3b2d4f4d..288ee6fec8d3 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -43,5 +43,6 @@
#define VIRTIO_ID_INPUT 18 /* virtio input */
#define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
#define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
+#define VIRTIO_ID_RDMA 26 /* RDMA */
#endif /* _LINUX_VIRTIO_IDS_H */
--
2.17.0
^ permalink raw reply related
* [RFC 2/3] hw/virtio-rdma: VirtIO rdma device
From: Yuval Shaia @ 2019-04-11 11:01 UTC (permalink / raw)
To: virtualization, qemu-devel, mst, cohuck, marcel.apfelbaum,
linux-rdma, jgg
Cc: Yuval Shaia
In-Reply-To: <20190411110157.14252-1-yuval.shaia@oracle.com>
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
hw/Kconfig | 1 +
hw/rdma/Kconfig | 4 +
hw/rdma/Makefile.objs | 2 +
hw/rdma/virtio/virtio-rdma-ib.c | 287 ++++++++++++++++++++
hw/rdma/virtio/virtio-rdma-ib.h | 93 +++++++
hw/rdma/virtio/virtio-rdma-main.c | 185 +++++++++++++
hw/virtio/Makefile.objs | 1 +
hw/virtio/virtio-rdma-pci.c | 108 ++++++++
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-rdma.h | 44 +++
include/standard-headers/linux/virtio_ids.h | 1 +
11 files changed, 727 insertions(+)
create mode 100644 hw/rdma/Kconfig
create mode 100644 hw/rdma/virtio/virtio-rdma-ib.c
create mode 100644 hw/rdma/virtio/virtio-rdma-ib.h
create mode 100644 hw/rdma/virtio/virtio-rdma-main.c
create mode 100644 hw/virtio/virtio-rdma-pci.c
create mode 100644 include/hw/virtio/virtio-rdma.h
diff --git a/hw/Kconfig b/hw/Kconfig
index d5ecd02070..88b9f15007 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -26,6 +26,7 @@ source pci-bridge/Kconfig
source pci-host/Kconfig
source pcmcia/Kconfig
source pci/Kconfig
+source rdma/Kconfig
source scsi/Kconfig
source sd/Kconfig
source smbios/Kconfig
diff --git a/hw/rdma/Kconfig b/hw/rdma/Kconfig
new file mode 100644
index 0000000000..b10bd7182b
--- /dev/null
+++ b/hw/rdma/Kconfig
@@ -0,0 +1,4 @@
+config VIRTIO_RDMA
+ bool
+ default y
+ depends on VIRTIO
diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
index c354e60e5b..ed640882be 100644
--- a/hw/rdma/Makefile.objs
+++ b/hw/rdma/Makefile.objs
@@ -3,3 +3,5 @@ obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma.o
obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
endif
+obj-$(CONFIG_VIRTIO_RDMA) += virtio/virtio-rdma-main.o \
+ virtio/virtio-rdma-ib.o
diff --git a/hw/rdma/virtio/virtio-rdma-ib.c b/hw/rdma/virtio/virtio-rdma-ib.c
new file mode 100644
index 0000000000..2590a831a2
--- /dev/null
+++ b/hw/rdma/virtio/virtio-rdma-ib.c
@@ -0,0 +1,287 @@
+/*
+ * Virtio RDMA Device - IB verbs
+ *
+ * Copyright (C) 2019 Oracle
+ *
+ * Authors:
+ * Yuval Shaia <yuval.shaia@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <infiniband/verbs.h>
+
+#include "qemu/osdep.h"
+
+#include "virtio-rdma-ib.h"
+#include "../rdma_utils.h"
+#include "../rdma_rm.h"
+#include "../rdma_backend.h"
+
+int virtio_rdma_query_device(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct ibv_device_attr attr = {};
+ int offs;
+ size_t s;
+
+ addrconf_addr_eui48((unsigned char *)&attr.sys_image_guid,
+ (const char *)&rdev->netdev->mac);
+
+ attr.max_mr_size = 4096;
+ attr.page_size_cap = 4096;
+ attr.vendor_id = 1;
+ attr.vendor_part_id = 1;
+ attr.hw_ver = VIRTIO_RDMA_HW_VER;
+ attr.max_qp = 1024;
+ attr.max_qp_wr = 1024;
+ attr.device_cap_flags = 0;
+ attr.max_sge = 64;
+ attr.max_sge_rd = 64;
+ attr.max_cq = 1024;
+ attr.max_cqe = 64;
+ attr.max_mr = 1024;
+ attr.max_pd = 1024;
+ attr.max_qp_rd_atom = 0;
+ attr.max_ee_rd_atom = 0;
+ attr.max_res_rd_atom = 0;
+ attr.max_qp_init_rd_atom = 0;
+ attr.max_ee_init_rd_atom = 0;
+ attr.atomic_cap = IBV_ATOMIC_NONE;
+ attr.max_ee = 0;
+ attr.max_rdd = 0;
+ attr.max_mw = 0;
+ attr.max_raw_ipv6_qp = 0;
+ attr.max_raw_ethy_qp = 0;
+ attr.max_mcast_grp = 0;
+ attr.max_mcast_qp_attach = 0;
+ attr.max_total_mcast_qp_attach = 0;
+ attr.max_ah = 1024;
+ attr.max_fmr = 0;
+ attr.max_map_per_fmr = 0;
+ attr.max_srq = 0;
+ attr.max_srq_wr = 0;
+ attr.max_srq_sge = 0;
+ attr.max_pkeys = 1;
+ attr.local_ca_ack_delay = 0;
+ attr.phys_port_cnt = VIRTIO_RDMA_PORT_CNT;
+
+ offs = offsetof(struct ibv_device_attr, sys_image_guid);
+ s = iov_from_buf(out, 1, 0, (void *)&attr + offs, sizeof(attr) - offs);
+
+ return s == sizeof(attr) - offs ? VIRTIO_RDMA_CTRL_OK :
+ VIRTIO_RDMA_CTRL_ERR;
+}
+
+int virtio_rdma_query_port(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct ibv_port_attr attr = {};
+ struct cmd_query_port cmd = {};
+ int offs;
+ size_t s;
+
+ s = iov_to_buf(in, 1, 0, &cmd, sizeof(cmd));
+ if (s != sizeof(cmd)) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ if (cmd.port != 1) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ attr.state = IBV_PORT_ACTIVE;
+ attr.max_mtu = attr.active_mtu = IBV_MTU_1024;
+ attr.gid_tbl_len = 256;
+ attr.port_cap_flags = 0;
+ attr.max_msg_sz = 1024;
+ attr.bad_pkey_cntr = 0;
+ attr.qkey_viol_cntr = 0;
+ attr.pkey_tbl_len = 1;
+ attr.lid = 0;
+ attr.sm_lid = 0;
+ attr.lmc = 0;
+ attr.max_vl_num = 1;
+ attr.sm_sl = 0;
+ attr.subnet_timeout = 0;
+ attr.init_type_reply = 0;
+ attr.active_width = 0;
+ attr.active_speed = 0;
+ attr.phys_state = 0;
+
+ offs = offsetof(struct ibv_port_attr, state);
+ s = iov_from_buf(out, 1, 0, (void *)&attr + offs, sizeof(attr) - offs);
+
+ return s == sizeof(attr) - offs ? VIRTIO_RDMA_CTRL_OK :
+ VIRTIO_RDMA_CTRL_ERR;
+}
+
+int virtio_rdma_create_cq(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct cmd_create_cq cmd = {};
+ struct rsp_create_cq rsp = {};
+ size_t s;
+ int rc;
+
+ s = iov_to_buf(in, 1, 0, &cmd, sizeof(cmd));
+ if (s != sizeof(cmd)) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ /* TODO: Define MAX_CQE */
+#define MAX_CQE 1024
+ /* TODO: Check MAX_CQ */
+ if (cmd.cqe > MAX_CQE) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ printf("%s: %d\n", __func__, cmd.cqe);
+
+ /* TODO: Create VirtQ */
+
+ rc = rdma_rm_alloc_cq(rdev->rdma_dev_res, rdev->backend_dev, cmd.cqe,
+ &rsp.cqn, NULL);
+ if (rc) {
+ /* TODO: Destroy VirtQ */
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ printf("%s: %d\n", __func__, rsp.cqn);
+
+ s = iov_from_buf(out, 1, 0, &rsp, sizeof(rsp));
+
+ return s == sizeof(rsp) ? VIRTIO_RDMA_CTRL_OK :
+ VIRTIO_RDMA_CTRL_ERR;
+}
+
+int virtio_rdma_destroy_cq(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct cmd_destroy_cq cmd = {};
+ size_t s;
+
+ s = iov_to_buf(in, 1, 0, &cmd, sizeof(cmd));
+ if (s != sizeof(cmd)) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ printf("%s: %d\n", __func__, cmd.cqn);
+
+ /* TODO: Destroy VirtQ */
+
+ rdma_rm_dealloc_cq(rdev->rdma_dev_res, cmd.cqn);
+
+ return VIRTIO_RDMA_CTRL_OK;
+}
+
+int virtio_rdma_create_pd(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct rsp_create_pd rsp = {};
+ size_t s;
+ int rc;
+
+ /* TODO: Check MAX_PD */
+
+ /* TODO: ctx */
+ rc = rdma_rm_alloc_pd(rdev->rdma_dev_res, rdev->backend_dev, &rsp.pdn,
+ 0);
+ if (rc)
+ return VIRTIO_RDMA_CTRL_ERR;
+
+ printf("%s: %d\n", __func__, rsp.pdn);
+
+ s = iov_from_buf(out, 1, 0, &rsp, sizeof(rsp));
+
+ return s == sizeof(rsp) ? VIRTIO_RDMA_CTRL_OK :
+ VIRTIO_RDMA_CTRL_ERR;
+}
+
+int virtio_rdma_destroy_pd(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct cmd_destroy_pd cmd = {};
+ size_t s;
+
+ s = iov_to_buf(in, 1, 0, &cmd, sizeof(cmd));
+ if (s != sizeof(cmd)) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ printf("%s: %d\n", __func__, cmd.pdn);
+
+ rdma_rm_dealloc_cq(rdev->rdma_dev_res, cmd.pdn);
+
+ return VIRTIO_RDMA_CTRL_OK;
+}
+
+int virtio_rdma_get_dma_mr(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out)
+{
+ struct cmd_get_dma_mr cmd = {};
+ struct rsp_get_dma_mr rsp = {};
+ size_t s;
+
+ s = iov_to_buf(in, 1, 0, &cmd, sizeof(cmd));
+ if (s != sizeof(cmd)) {
+ return VIRTIO_RDMA_CTRL_ERR;
+ }
+
+ /* TODO: Call rdma_rm_alloc_mr */
+ rsp.mrn = 0x10;
+ rsp.lkey = 0x11;
+ rsp.rkey = 0x12;
+ printf("%s: 0x%x\n", __func__, rsp.mrn);
+
+ s = iov_from_buf(out, 1, 0, &rsp, sizeof(rsp));
+
+ return s == sizeof(rsp) ? VIRTIO_RDMA_CTRL_OK :
+ VIRTIO_RDMA_CTRL_ERR;
+}
+
+static void virtio_rdma_init_dev_caps(VirtIORdma *rdev)
+{
+ rdev->dev_attr.max_qp_wr = 1024;
+}
+
+int virtio_rdma_init_ib(VirtIORdma *rdev)
+{
+ int rc;
+
+ virtio_rdma_init_dev_caps(rdev);
+
+ rdev->rdma_dev_res = g_malloc0(sizeof(RdmaDeviceResources));
+ rdev->backend_dev = g_malloc0(sizeof(RdmaBackendDev));
+
+ rc = rdma_backend_init(rdev->backend_dev, NULL, rdev->rdma_dev_res,
+ rdev->backend_device_name,
+ rdev->backend_port_num, &rdev->dev_attr,
+ &rdev->mad_chr);
+ if (rc) {
+ rdma_error_report("Fail to initialize backend device");
+ return rc;
+ }
+
+ rc = rdma_rm_init(rdev->rdma_dev_res, &rdev->dev_attr);
+ if (rc) {
+ rdma_error_report("Fail to initialize resource manager");
+ return rc;
+ }
+
+ /* rdma_backend_start(rdev->backend_dev); */
+
+ return 0;
+}
+
+void virtio_rdma_fini_ib(VirtIORdma *rdev)
+{
+ /* rdma_backend_stop(rdev->backend_dev); */
+ rdma_rm_fini(rdev->rdma_dev_res, rdev->backend_dev,
+ rdev->backend_eth_device_name);
+ rdma_backend_fini(rdev->backend_dev);
+ g_free(rdev->rdma_dev_res);
+ g_free(rdev->backend_dev);
+}
diff --git a/hw/rdma/virtio/virtio-rdma-ib.h b/hw/rdma/virtio/virtio-rdma-ib.h
new file mode 100644
index 0000000000..c4bdc063ac
--- /dev/null
+++ b/hw/rdma/virtio/virtio-rdma-ib.h
@@ -0,0 +1,93 @@
+/*
+ * Virtio RDMA Device - IB verbs
+ *
+ * Copyright (C) 2019 Oracle
+ *
+ * Authors:
+ * Yuval Shaia <yuval.shaia@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "hw/virtio/virtio-rdma.h"
+
+/* TODO: Move to uapi header file */
+#define VIRTIO_RDMA_CTRL_OK 0
+#define VIRTIO_RDMA_CTRL_ERR 1
+
+enum {
+ VIRTIO_CMD_QUERY_DEVICE = 10,
+ VIRTIO_CMD_QUERY_PORT,
+ VIRTIO_CMD_CREATE_CQ,
+ VIRTIO_CMD_DESTROY_CQ,
+ VIRTIO_CMD_CREATE_PD,
+ VIRTIO_CMD_DESTROY_PD,
+ VIRTIO_CMD_GET_DMA_MR,
+};
+
+struct control_buf {
+ uint8_t cmd;
+ uint8_t status;
+};
+
+struct cmd_query_port {
+ uint8_t port;
+};
+
+struct cmd_create_cq {
+ uint32_t cqe;
+};
+
+struct rsp_create_cq {
+ uint32_t cqn;
+};
+
+struct cmd_destroy_cq {
+ uint32_t cqn;
+};
+
+struct rsp_create_pd {
+ uint32_t pdn;
+};
+
+struct cmd_destroy_pd {
+ uint32_t pdn;
+};
+
+struct cmd_get_dma_mr {
+ uint32_t pdn;
+ uint32_t access_flags;
+};
+
+struct rsp_get_dma_mr {
+ uint32_t mrn;
+ uint32_t lkey;
+ uint32_t rkey;
+};
+
+/* TODO: Move to uapi header file */
+
+#define VIRTIO_RDMA_PORT_CNT 1
+#define VIRTIO_RDMA_HW_VER 1
+
+int virtio_rdma_init_ib(VirtIORdma *rdev);
+void virtio_rdma_fini_ib(VirtIORdma *rdev);
+
+int virtio_rdma_query_device(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_query_port(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_create_cq(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_destroy_cq(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_create_pd(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_destroy_pd(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
+int virtio_rdma_get_dma_mr(VirtIORdma *rdev, struct iovec *in,
+ struct iovec *out);
diff --git a/hw/rdma/virtio/virtio-rdma-main.c b/hw/rdma/virtio/virtio-rdma-main.c
new file mode 100644
index 0000000000..54f75b14c0
--- /dev/null
+++ b/hw/rdma/virtio/virtio-rdma-main.c
@@ -0,0 +1,185 @@
+/*
+ * Virtio RDMA Device
+ *
+ * Copyright (C) 2019 Oracle
+ *
+ * Authors:
+ * Yuval Shaia <yuval.shaia@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <infiniband/verbs.h>
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/error-report.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-rdma.h"
+#include "include/standard-headers/linux/virtio_ids.h"
+
+#include "virtio-rdma-ib.h"
+#include "../rdma_rm_defs.h"
+#include "../rdma_utils.h"
+
+static void virtio_rdma_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VirtIORdma *r = VIRTIO_RDMA(vdev);
+ struct control_buf cb;
+ VirtQueueElement *e;
+ size_t s;
+
+ virtio_queue_set_notification(vq, 0);
+
+ for (;;) {
+ e = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ if (!e) {
+ break;
+ }
+
+ if (iov_size(e->in_sg, e->in_num) < sizeof(cb.status) ||
+ iov_size(e->out_sg, e->out_num) < sizeof(cb.cmd)) {
+ virtio_error(vdev, "Got invalid message size");
+ virtqueue_detach_element(vq, e, 0);
+ g_free(e);
+ break;
+ }
+
+ s = iov_to_buf(&e->out_sg[0], 1, 0, &cb.cmd, sizeof(cb.cmd));
+ if (s != sizeof(cb.cmd)) {
+ cb.status = VIRTIO_RDMA_CTRL_ERR;
+ } else {
+ printf("cmd=%d\n", cb.cmd);
+ switch (cb.cmd) {
+ case VIRTIO_CMD_QUERY_DEVICE:
+ cb.status = virtio_rdma_query_device(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_QUERY_PORT:
+ cb.status = virtio_rdma_query_port(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_CREATE_CQ:
+ cb.status = virtio_rdma_create_cq(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_DESTROY_CQ:
+ cb.status = virtio_rdma_destroy_cq(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_CREATE_PD:
+ cb.status = virtio_rdma_create_pd(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_DESTROY_PD:
+ cb.status = virtio_rdma_destroy_pd(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ case VIRTIO_CMD_GET_DMA_MR:
+ cb.status = virtio_rdma_get_dma_mr(r, &e->out_sg[1],
+ &e->in_sg[0]);
+ break;
+ default:
+ cb.status = VIRTIO_RDMA_CTRL_ERR;
+ }
+ }
+ printf("status=%d\n", cb.status);
+ s = iov_from_buf(&e->in_sg[1], 1, 0, &cb.status, sizeof(cb.status));
+ assert(s == sizeof(cb.status));
+
+ virtqueue_push(vq, e, sizeof(cb.status));
+ virtio_notify(vdev, vq);
+ }
+
+ virtio_queue_set_notification(vq, 1);
+}
+
+static void virtio_rdma_device_realize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIORdma *r = VIRTIO_RDMA(dev);
+ int rc;
+
+ rc = virtio_rdma_init_ib(r);
+ if (rc) {
+ rdma_error_report("Fail to initialize IB layer");
+ return;
+ }
+
+ virtio_init(vdev, "virtio-rdma", VIRTIO_ID_RDMA, 1024);
+
+ r->ctrl_vq = virtio_add_queue(vdev, 64, virtio_rdma_handle_ctrl);
+}
+
+static void virtio_rdma_device_unrealize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIORdma *r = VIRTIO_RDMA(dev);
+
+ virtio_del_queue(vdev, 0);
+
+ virtio_cleanup(vdev);
+
+ virtio_rdma_fini_ib(r);
+}
+
+static uint64_t virtio_rdma_get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp)
+{
+ /* virtio_add_feature(&features, VIRTIO_NET_F_MAC); */
+
+ vdev->backend_features = features;
+
+ return features;
+}
+
+
+static Property virtio_rdma_dev_properties[] = {
+ DEFINE_PROP_STRING("netdev", VirtIORdma, backend_eth_device_name),
+ DEFINE_PROP_STRING("ibdev",VirtIORdma, backend_device_name),
+ DEFINE_PROP_UINT8("ibport", VirtIORdma, backend_port_num, 1),
+ DEFINE_PROP_UINT64("dev-caps-max-mr-size", VirtIORdma, dev_attr.max_mr_size,
+ MAX_MR_SIZE),
+ DEFINE_PROP_INT32("dev-caps-max-qp", VirtIORdma, dev_attr.max_qp, MAX_QP),
+ DEFINE_PROP_INT32("dev-caps-max-cq", VirtIORdma, dev_attr.max_cq, MAX_CQ),
+ DEFINE_PROP_INT32("dev-caps-max-mr", VirtIORdma, dev_attr.max_mr, MAX_MR),
+ DEFINE_PROP_INT32("dev-caps-max-pd", VirtIORdma, dev_attr.max_pd, MAX_PD),
+ DEFINE_PROP_INT32("dev-caps-qp-rd-atom", VirtIORdma,
+ dev_attr.max_qp_rd_atom, MAX_QP_RD_ATOM),
+ DEFINE_PROP_INT32("dev-caps-max-qp-init-rd-atom", VirtIORdma,
+ dev_attr.max_qp_init_rd_atom, MAX_QP_INIT_RD_ATOM),
+ DEFINE_PROP_INT32("dev-caps-max-ah", VirtIORdma, dev_attr.max_ah, MAX_AH),
+ DEFINE_PROP_CHR("mad-chardev", VirtIORdma, mad_chr),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_rdma_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+ set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+ vdc->realize = virtio_rdma_device_realize;
+ vdc->unrealize = virtio_rdma_device_unrealize;
+ vdc->get_features = virtio_rdma_get_features;
+
+ dc->desc = "Virtio RDMA Device";
+ dc->props = virtio_rdma_dev_properties;
+ set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+}
+
+static const TypeInfo virtio_rdma_info = {
+ .name = TYPE_VIRTIO_RDMA,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VirtIORdma),
+ .class_init = virtio_rdma_class_init,
+};
+
+static void virtio_register_types(void)
+{
+ type_register_static(&virtio_rdma_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index f2ab667a21..fd701feb9f 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -27,6 +27,7 @@ obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
obj-$(CONFIG_VIRTIO_NET) += virtio-net-pci.o
+obj-$(CONFIG_VIRTIO_RDMA) += virtio-rdma-pci.o
obj-$(CONFIG_VIRTIO_SERIAL) += virtio-serial-pci.o
endif
else
diff --git a/hw/virtio/virtio-rdma-pci.c b/hw/virtio/virtio-rdma-pci.c
new file mode 100644
index 0000000000..36efce285b
--- /dev/null
+++ b/hw/virtio/virtio-rdma-pci.c
@@ -0,0 +1,108 @@
+/*
+ * Virtio rdma PCI Bindings
+ *
+ * Copyright (C) 2019 Oracle
+ *
+ * Authors:
+ * Yuval Shaia <yuval.shaia@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/virtio/virtio-net-pci.h"
+#include "hw/virtio/virtio-rdma.h"
+#include "virtio-pci.h"
+#include "qapi/error.h"
+
+typedef struct VirtIORdmaPCI VirtIORdmaPCI;
+
+/*
+ * virtio-rdma-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_RDMA_PCI "virtio-rdma-pci-base"
+#define VIRTIO_RDMA_PCI(obj) \
+ OBJECT_CHECK(VirtIORdmaPCI, (obj), TYPE_VIRTIO_RDMA_PCI)
+
+struct VirtIORdmaPCI {
+ VirtIOPCIProxy parent_obj;
+ VirtIORdma vdev;
+};
+
+static Property virtio_rdma_properties[] = {
+ DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+ VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_rdma_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VirtIORdmaPCI *dev = VIRTIO_RDMA_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&dev->vdev);
+ VirtIONetPCI *vnet_pci;
+ PCIDevice *func0;
+
+ qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+ object_property_set_bool(OBJECT(vdev), true, "realized", errp);
+
+ func0 = pci_get_function_0(&vpci_dev->pci_dev);
+ /* Break if not virtio device in slot 0 */
+ if (strcmp(object_get_typename(OBJECT(func0)),
+ TYPE_VIRTIO_NET_PCI_GENERIC)) {
+ error_setg(errp, "Device on %x.0 is type %s but must be %s",
+ PCI_SLOT(vpci_dev->pci_dev.devfn),
+ object_get_typename(OBJECT(func0)),
+ TYPE_VIRTIO_NET_PCI_GENERIC);
+ return;
+ }
+ vnet_pci = VIRTIO_NET_PCI(func0);
+ dev->vdev.netdev = &vnet_pci->vdev;
+}
+
+static void virtio_rdma_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+
+ k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ k->device_id = PCI_DEVICE_ID_VIRTIO_RDMA;
+ k->revision = VIRTIO_PCI_ABI_VERSION;
+ k->class_id = PCI_CLASS_NETWORK_OTHER;
+ set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+ dc->props = virtio_rdma_properties;
+ vpciklass->realize = virtio_rdma_pci_realize;
+}
+
+static void virtio_rdma_pci_instance_init(Object *obj)
+{
+ VirtIORdmaPCI *dev = VIRTIO_RDMA_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VIRTIO_RDMA);
+ /*
+ object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+ "bootindex", &error_abort);
+ */
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_rdma_pci_info = {
+ .base_name = TYPE_VIRTIO_RDMA_PCI,
+ .generic_name = "virtio-rdma-pci",
+ .transitional_name = "virtio-rdma-pci-transitional",
+ .non_transitional_name = "virtio-rdma-pci-non-transitional",
+ .instance_size = sizeof(VirtIORdmaPCI),
+ .instance_init = virtio_rdma_pci_instance_init,
+ .class_init = virtio_rdma_pci_class_init,
+};
+
+static void virtio_rdma_pci_register(void)
+{
+ virtio_pci_types_register(&virtio_rdma_pci_info);
+}
+
+type_init(virtio_rdma_pci_register)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d87f5f93e9..c2d34c382f 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -85,6 +85,7 @@ extern bool pci_available;
#define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
#define PCI_DEVICE_ID_VIRTIO_9P 0x1009
#define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
+#define PCI_DEVICE_ID_VIRTIO_RDMA 0x1013
#define PCI_VENDOR_ID_REDHAT 0x1b36
#define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
diff --git a/include/hw/virtio/virtio-rdma.h b/include/hw/virtio/virtio-rdma.h
new file mode 100644
index 0000000000..3c7534cd8a
--- /dev/null
+++ b/include/hw/virtio/virtio-rdma.h
@@ -0,0 +1,44 @@
+/*
+ * Virtio RDMA Device
+ *
+ * Copyright (C) 2019 Oracle
+ *
+ * Authors:
+ * Yuval Shaia <yuval.shaia@oracle.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_VIRTIO_RDMA_H
+#define QEMU_VIRTIO_RDMA_H
+
+#include <infiniband/verbs.h>
+
+#include "chardev/char-fe.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-net.h"
+
+#define TYPE_VIRTIO_RDMA "virtio-rdma-device"
+#define VIRTIO_RDMA(obj) \
+ OBJECT_CHECK(VirtIORdma, (obj), TYPE_VIRTIO_RDMA)
+
+typedef struct RdmaBackendDev RdmaBackendDev;
+typedef struct RdmaDeviceResources RdmaDeviceResources;
+struct ibv_device_attr;
+
+typedef struct VirtIORdma {
+ VirtIODevice parent_obj;
+ VirtQueue *ctrl_vq;
+ VirtIONet *netdev;
+ RdmaBackendDev *backend_dev;
+ RdmaDeviceResources *rdma_dev_res;
+ CharBackend mad_chr;
+ char *backend_eth_device_name;
+ char *backend_device_name;
+ uint8_t backend_port_num;
+ struct ibv_device_attr dev_attr;
+} VirtIORdma;
+
+#endif
diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index 6d5c3b2d4f..bd2c699450 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -43,5 +43,6 @@
#define VIRTIO_ID_INPUT 18 /* virtio input */
#define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
#define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
+#define VIRTIO_ID_RDMA 26 /* virtio crypto */
#endif /* _LINUX_VIRTIO_IDS_H */
--
2.20.1
^ permalink raw reply related
* [RFC 1/3] virtio-net: Move some virtio-net-pci decl to include/hw/virtio
From: Yuval Shaia @ 2019-04-11 11:01 UTC (permalink / raw)
To: virtualization, qemu-devel, mst, cohuck, marcel.apfelbaum,
linux-rdma, jgg
Cc: Yuval Shaia
In-Reply-To: <20190411110157.14252-1-yuval.shaia@oracle.com>
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
hw/virtio/virtio-net-pci.c | 18 ++-------------
include/hw/virtio/virtio-net-pci.h | 35 ++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 16 deletions(-)
create mode 100644 include/hw/virtio/virtio-net-pci.h
diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index db07ab9e21..63617d5550 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -17,24 +17,10 @@
#include "qemu/osdep.h"
-#include "hw/virtio/virtio-net.h"
+#include "hw/virtio/virtio-net-pci.h"
#include "virtio-pci.h"
#include "qapi/error.h"
-typedef struct VirtIONetPCI VirtIONetPCI;
-
-/*
- * virtio-net-pci: This extends VirtioPCIProxy.
- */
-#define TYPE_VIRTIO_NET_PCI "virtio-net-pci-base"
-#define VIRTIO_NET_PCI(obj) \
- OBJECT_CHECK(VirtIONetPCI, (obj), TYPE_VIRTIO_NET_PCI)
-
-struct VirtIONetPCI {
- VirtIOPCIProxy parent_obj;
- VirtIONet vdev;
-};
-
static Property virtio_net_properties[] = {
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
@@ -82,7 +68,7 @@ static void virtio_net_pci_instance_init(Object *obj)
static const VirtioPCIDeviceTypeInfo virtio_net_pci_info = {
.base_name = TYPE_VIRTIO_NET_PCI,
- .generic_name = "virtio-net-pci",
+ .generic_name = TYPE_VIRTIO_NET_PCI_GENERIC,
.transitional_name = "virtio-net-pci-transitional",
.non_transitional_name = "virtio-net-pci-non-transitional",
.instance_size = sizeof(VirtIONetPCI),
diff --git a/include/hw/virtio/virtio-net-pci.h b/include/hw/virtio/virtio-net-pci.h
new file mode 100644
index 0000000000..f14e6ed992
--- /dev/null
+++ b/include/hw/virtio/virtio-net-pci.h
@@ -0,0 +1,35 @@
+/*
+ * PCI Virtio Network Device
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_VIRTIO_NET_PCI_H
+#define QEMU_VIRTIO_NET_PCI_H
+
+#include "hw/virtio/virtio-net.h"
+#include "virtio-pci.h"
+
+typedef struct VirtIONetPCI VirtIONetPCI;
+
+/*
+ * virtio-net-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_NET_PCI_GENERIC "virtio-net-pci"
+#define TYPE_VIRTIO_NET_PCI "virtio-net-pci-base"
+#define VIRTIO_NET_PCI(obj) \
+ OBJECT_CHECK(VirtIONetPCI, (obj), TYPE_VIRTIO_NET_PCI)
+
+struct VirtIONetPCI {
+ VirtIOPCIProxy parent_obj;
+ VirtIONet vdev;
+};
+
+#endif
--
2.20.1
^ permalink raw reply related
* [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-11 11:01 UTC (permalink / raw)
To: virtualization, qemu-devel, mst, cohuck, marcel.apfelbaum,
linux-rdma, jgg
Cc: Yuval Shaia
Data center backends use more and more RDMA or RoCE devices and more and
more software runs in virtualized environment.
There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
Virtio is the optimal solution since is the de-facto para-virtualizaton
technology and also because the Virtio specification
allows Hardware Vendors to support Virtio protocol natively in order to
achieve bare metal performance.
This RFC is an effort to addresses challenges in defining the RDMA/RoCE
Virtio Specification and a look forward on possible implementation
techniques.
Open issues/Todo list:
List is huge, this is only start point of the project.
Anyway, here is one example of item in the list:
- Multi VirtQ: Every QP has two rings and every CQ has one. This means that
in order to support for example 32K QPs we will need 64K VirtQ. Not sure
that this is reasonable so one option is to have one for all and
multiplex the traffic on it. This is not good approach as by design it
introducing an optional starvation. Another approach would be multi
queues and round-robin (for example) between them.
Expectations from this posting:
In general, any comment is welcome, starting from hey, drop this as it is a
very bad idea, to yeah, go ahead, we really want it.
Idea here is that since it is not a minor effort i first want to know if
there is some sort interest in the community for such device.
The scope of the implementation is limited to probing the device and doing
some basic ibverbs commands. Data-path is not yet implemented. So with this
one can expect only that driver is (partialy) loaded and basic queries and
resource allocation is done.
One note regarding the patchset.
I know it is not standard to collaps patches from several repos as i did
here (qemu and linux) but decided to do it anyway so the whole picture can
be seen.
patch 1: virtio-net: Move some virtio-net-pci decl to include/hw/virtio
This is a prelimenary patch just as a hack so i will not need to
impelement new netdev
patch 2: hw/virtio-rdma: VirtIO rdma device
The implementation of the device
patch 3: RDMA/virtio-rdma: VirtIO rdma driver
The device driver
--
2.20.1
^ permalink raw reply
* Re: [RFC PATCH 10/12] virtio/s390: consolidate DMA allocations
From: Cornelia Huck @ 2019-04-11 9:24 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410194849.511ecc46@oc2783563651>
On Wed, 10 Apr 2019 19:48:49 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Wed, 10 Apr 2019 18:36:43 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Wed, 10 Apr 2019 17:12:54 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> > > On Wed, 10 Apr 2019 10:46:49 +0200
> > > Cornelia Huck <cohuck@redhat.com> wrote:
> > >
> > > > On Fri, 5 Apr 2019 01:16:20 +0200
> > > > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> > > > > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> > > > > index aa45a6a027ae..7268149f2ee8 100644
> > > > > --- a/drivers/s390/virtio/virtio_ccw.c
> > > > > +++ b/drivers/s390/virtio/virtio_ccw.c
> > > > > @@ -49,12 +49,12 @@ struct vq_config_block {
> > > > > struct vcdev_dma_area {
> > > > > unsigned long indicators;
> > > > > unsigned long indicators2;
> > > > > + struct vq_config_block config_block;
> > > > > + __u8 status; /* TODO check __aligned(8); */
> > > >
> > > > ...I think that needs attention.
> > >
> > > Yes I wanted to discuss this with you. I could not find anything
> > > in the virtio spec that would put requirements on how this
> > > status field needs to be aligned. But I did not look to hard.
> > >
> > > The ccw.cda can hold an arbitrary data address AFAIR (for indirect,
> > > of course we do have alignment requirements).
> >
> > I think it needs to be doubleword aligned.
> >
>
> I've re-read the part of the PoP that describes the ccw formats. And
> it reinforced my position: for IDA and MIDA we need proper alignment,
> but if the CCW ain't an indirect one there is no alignment requirement.
>
> QEMU also does not seem to check either.
>
> Can you double-check and provide me with a reference that proves me
> wrong if I'm wrong.
Ah, it was the ccw itself, not the cda. Indeed, there do not seem to be
any requirements for direct addressing.
>
> > >
> > > Apparently status used to be a normal field, and became a pointer with
> > > 73fa21ea4fc6 "KVM: s390: Dynamic allocation of virtio-ccw I/O
> > > data." (Cornelia Huck, 2013-01-07). I could not quite figure out why.
> >
> > In the beginning, the code used a below-2G-area for all commands.
> > Rather than adding locking to avoid races there, that commit switches
> > to allocating the needed structures individually. The status field
> > needed to be below 2G, so it needed to be allocated separately.
> >
>
> I get it now. The confusing part was that the field 'area' was about
> holding the address of the also previously dynamically allocated
> below 2G area that was used for talking to the hypervisor via CCW I/O.
>
> > >
> > > So maybe dropping the TODO comment will do just fine. What do you think?
> >
>
> I still think we just need to drop the comment, as we don't have to
> align it.
Agreed.
^ permalink raw reply
* Re: [PATCH net] vhost: reject zero size iova range
From: David Miller @ 2019-04-11 5:46 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <20190409041025.20922-1-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 9 Apr 2019 12:10:25 +0800
> We used to accept zero size iova range which will lead a infinite loop
> in translate_desc(). Fixing this by failing the request in this case.
>
> Reported-by: syzbot+d21e6e297322a900c128@syzkaller.appspotmail.com
> Fixes: 6b1e6cc7 ("vhost: new device IOTLB API")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net v8] failover: allow name change on IFF_UP slave interfaces
From: David Miller @ 2019-04-11 5:14 UTC (permalink / raw)
To: si-wei.liu
Cc: jiri, mst, kubakici, sridhar.samudrala, alexander.duyck,
virtualization, liran.alon, netdev, boris.ostrovsky
In-Reply-To: <1554767127-32576-1-git-send-email-si-wei.liu@oracle.com>
From: Si-Wei Liu <si-wei.liu@oracle.com>
Date: Mon, 8 Apr 2019 19:45:27 -0400
> When a netdev appears through hot plug then gets enslaved by a failover
> master that is already up and running, the slave will be opened
> right away after getting enslaved. Today there's a race that userspace
> (udev) may fail to rename the slave if the kernel (net_failover)
> opens the slave earlier than when the userspace rename happens.
> Unlike bond or team, the primary slave of failover can't be renamed by
> userspace ahead of time, since the kernel initiated auto-enslavement is
> unable to, or rather, is never meant to be synchronized with the rename
> request from userspace.
>
> As the failover slave interfaces are not designed to be operated
> directly by userspace apps: IP configuration, filter rules with
> regard to network traffic passing and etc., should all be done on master
> interface. In general, userspace apps only care about the
> name of master interface, while slave names are less important as long
> as admin users can see reliable names that may carry
> other information describing the netdev. For e.g., they can infer that
> "ens3nsby" is a standby slave of "ens3", while for a
> name like "eth0" they can't tell which master it belongs to.
>
> Historically the name of IFF_UP interface can't be changed because
> there might be admin script or management software that is already
> relying on such behavior and assumes that the slave name can't be
> changed once UP. But failover is special: with the in-kernel
> auto-enslavement mechanism, the userspace expectation for device
> enumeration and bring-up order is already broken. Previously initramfs
> and various userspace config tools were modified to bypass failover
> slaves because of auto-enslavement and duplicate MAC address. Similarly,
> in case that users care about seeing reliable slave name, the new type
> of failover slaves needs to be taken care of specifically in userspace
> anyway.
>
> It's less risky to lift up the rename restriction on failover slave
> which is already UP. Although it's possible this change may potentially
> break userspace component (most likely configuration scripts or
> management software) that assumes slave name can't be changed while
> UP, it's relatively a limited and controllable set among all userspace
> components, which can be fixed specifically to listen for the rename
> events on failover slaves. Userspace component interacting with slaves
> is expected to be changed to operate on failover master interface
> instead, as the failover slave is dynamic in nature which may come and
> go at any point. The goal is to make the role of failover slaves less
> relevant, and userspace components should only deal with failover master
> in the long run.
>
> Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Reviewed-by: Liran Alon <liran.alon@oracle.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH 3/3] virtio-gpu api: VIRTIO_GPU_F_RESSOURCE_V2
From: Gerd Hoffmann @ 2019-04-11 5:03 UTC (permalink / raw)
To: Gurchetan Singh
Cc: Tomeu Vizoso, Michael S. Tsirkin, David Airlie, open list,
ML dri-devel, open list:VIRTIO CORE, NET AND BLOCK DRIVERS,
Marc-André Lureau, David Airlie, virtio
In-Reply-To: <CAAfnVBm5XZx8P7k5np823-uRr-kqf0ey9Y3GtoE2Geo4dCdQ=g@mail.gmail.com>
> > +/* VIRTIO_GPU_CMD_RESOURCE_CREATE_V2 */
> > +struct virtio_gpu_cmd_resource_create_v2 {
> > + struct virtio_gpu_ctrl_hdr hdr;
> > + __le32 resource_id;
> > + __le32 format;
> > + __le32 width;
> > + __le32 height;
> > + /* 3d only */
> > + __le32 target;
> > + __le32 bind;
> > + __le32 depth;
> > + __le32 array_size;
> > + __le32 last_level;
> > + __le32 nr_samples;
> > + __le32 flags;
> > +};
>
>
> I assume this is always backed by some host side allocation, without any
> guest side pages associated with it?
No. It is not backed at all yet. Workflow would be like this:
(1) VIRTIO_GPU_CMD_RESOURCE_CREATE_V2
(2) VIRTIO_GPU_CMD_MEMORY_CREATE (see patch 2)
(3) VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH (see patch 2)
You could also create a larger pool with VIRTIO_GPU_CMD_MEMORY_CREATE,
then go attach multiple resources to it.
> If so, do we want the option for the guest allocate?
Allocation options are handled by VIRTIO_GPU_CMD_MEMORY_CREATE
(initially guest allocated only, i.e. what virtio-gpu supports today,
the plan is to add other allocation types later on).
> > +/* VIRTIO_GPU_RESP_OK_RESOURCE_INFO */
> > +struct virtio_gpu_resp_resource_info {
> > + struct virtio_gpu_ctrl_hdr hdr;
> > + __le32 stride[4];
> > + __le32 size[4];
> > +};
>
> offsets[4] needed too
That is in VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH ...
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH] drm/bochs: use simple display pipe
From: Sam Ravnborg @ 2019-04-10 21:22 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU
In-Reply-To: <20190410074828.10296-1-kraxel@redhat.com>
Hi Gerd.
>
> #include <drm/drm_gem.h>
>
> @@ -69,9 +70,8 @@ struct bochs_device {
> struct edid *edid;
>
> /* drm */
> - struct drm_device *dev;
> - struct drm_crtc crtc;
> - struct drm_encoder encoder;
> + struct drm_device *dev;
> + struct drm_simple_display_pipe pipe;
> struct drm_connector connector;
Have you considered to embed drm_device in the bochs_device struct?
This is the new way to do it.
This should likely be a follow-up patch...
Rest of the patch looked good.
Did not do a proper review but you can add my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Sam
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Cornelia Huck @ 2019-04-10 16:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Pankaj Gupta, jack, kvm, david, qemu-devel, virtualization,
adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
kilobyte, riel, yuval shaia, stefanha, imammedo, dan j williams,
lcapitulino, nilal, tytso, xiaoguangrong eric, darrick wong, rjw,
linux-kernel, linux-xfs, linux-fsdevel
In-Reply-To: <20190410124114-mutt-send-email-mst@kernel.org>
On Wed, 10 Apr 2019 12:46:12 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Apr 10, 2019 at 04:31:39PM +0200, Cornelia Huck wrote:
> > On Wed, 10 Apr 2019 10:03:01 -0400 (EDT)
> > Pankaj Gupta <pagupta@redhat.com> wrote:
> >
> > > >
> > > > On Wed, Apr 10, 2019 at 09:38:22AM +0530, Pankaj Gupta wrote:
> > > > > This patch adds virtio-pmem driver for KVM guest.
> > > > >
> > > > > Guest reads the persistent memory range information from
> > > > > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > > > > creates a nd_region object with the persistent memory
> > > > > range information so that existing 'nvdimm/pmem' driver
> > > > > can reserve this into system memory map. This way
> > > > > 'virtio-pmem' driver uses existing functionality of pmem
> > > > > driver to register persistent memory compatible for DAX
> > > > > capable filesystems.
> > > > >
> > > > > This also provides function to perform guest flush over
> > > > > VIRTIO from 'pmem' driver when userspace performs flush
> > > > > on DAX memory range.
> > > > >
> > > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >
> > > > > diff --git a/include/uapi/linux/virtio_ids.h
> > > > > b/include/uapi/linux/virtio_ids.h
> > > > > index 6d5c3b2d4f4d..346389565ac1 100644
> > > > > --- a/include/uapi/linux/virtio_ids.h
> > > > > +++ b/include/uapi/linux/virtio_ids.h
> > > > > @@ -43,5 +43,6 @@
> > > > > #define VIRTIO_ID_INPUT 18 /* virtio input */
> > > > > #define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
> > > > > #define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
> > > > > +#define VIRTIO_ID_PMEM 25 /* virtio pmem */
> > > > >
> > > > > #endif /* _LINUX_VIRTIO_IDS_H */
> > > >
> > > > Didn't Paolo point out someone is using 25 for audio?
> > >
> > >
> > > Sorry! I did not notice this.
> > >
> > > >
> > > > Please, reserve an ID with the Virtio TC before using it.
> > >
> > > I thought I requested[1] ID 25.
> > >
> > > [1] https://github.com/oasis-tcs/virtio-spec/issues/38
> > > [2] https://lists.oasis-open.org/archives/virtio-dev/201805/threads.html
> > >
> > > In that case I will send request for next available ID i.e 26?
> >
> > Have the folks using this for audio sent a reservation request as well?
> > If not, I'd say the first requester should get the id...
>
> No but I think they ship their's already. No one ships pmem
> so it's less pain for everyone if we just skip 25.
Ugh. Ok, then we should change pmem...
>
> > (And yes, we need to be quicker with voting on/applying id
> > reservations :/)
>
> We can't vote on what was not proposed for a vote.
> At the moment that responsibility is with the commented
> once discussion on the comment has taken place.
>
> I think what's missing is a description of the process
> in the README. Want to write it up and post it?
I can add that to my TODO list, can't promise speedy resolution, though.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Michael S. Tsirkin @ 2019-04-10 16:46 UTC (permalink / raw)
To: Cornelia Huck
Cc: Pankaj Gupta, jack, kvm, david, qemu-devel, virtualization,
adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
kilobyte, riel, yuval shaia, stefanha, imammedo, dan j williams,
lcapitulino, nilal, tytso, xiaoguangrong eric, darrick wong, rjw,
linux-kernel, linux-xfs, linux-fsdevel
In-Reply-To: <20190410163139.4a04175e.cohuck@redhat.com>
On Wed, Apr 10, 2019 at 04:31:39PM +0200, Cornelia Huck wrote:
> On Wed, 10 Apr 2019 10:03:01 -0400 (EDT)
> Pankaj Gupta <pagupta@redhat.com> wrote:
>
> > >
> > > On Wed, Apr 10, 2019 at 09:38:22AM +0530, Pankaj Gupta wrote:
> > > > This patch adds virtio-pmem driver for KVM guest.
> > > >
> > > > Guest reads the persistent memory range information from
> > > > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > > > creates a nd_region object with the persistent memory
> > > > range information so that existing 'nvdimm/pmem' driver
> > > > can reserve this into system memory map. This way
> > > > 'virtio-pmem' driver uses existing functionality of pmem
> > > > driver to register persistent memory compatible for DAX
> > > > capable filesystems.
> > > >
> > > > This also provides function to perform guest flush over
> > > > VIRTIO from 'pmem' driver when userspace performs flush
> > > > on DAX memory range.
> > > >
> > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
>
> > > > diff --git a/include/uapi/linux/virtio_ids.h
> > > > b/include/uapi/linux/virtio_ids.h
> > > > index 6d5c3b2d4f4d..346389565ac1 100644
> > > > --- a/include/uapi/linux/virtio_ids.h
> > > > +++ b/include/uapi/linux/virtio_ids.h
> > > > @@ -43,5 +43,6 @@
> > > > #define VIRTIO_ID_INPUT 18 /* virtio input */
> > > > #define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
> > > > #define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
> > > > +#define VIRTIO_ID_PMEM 25 /* virtio pmem */
> > > >
> > > > #endif /* _LINUX_VIRTIO_IDS_H */
> > >
> > > Didn't Paolo point out someone is using 25 for audio?
> >
> >
> > Sorry! I did not notice this.
> >
> > >
> > > Please, reserve an ID with the Virtio TC before using it.
> >
> > I thought I requested[1] ID 25.
> >
> > [1] https://github.com/oasis-tcs/virtio-spec/issues/38
> > [2] https://lists.oasis-open.org/archives/virtio-dev/201805/threads.html
> >
> > In that case I will send request for next available ID i.e 26?
>
> Have the folks using this for audio sent a reservation request as well?
> If not, I'd say the first requester should get the id...
No but I think they ship their's already. No one ships pmem
so it's less pain for everyone if we just skip 25.
> (And yes, we need to be quicker with voting on/applying id
> reservations :/)
We can't vote on what was not proposed for a vote.
At the moment that responsibility is with the commented
once discussion on the comment has taken place.
I think what's missing is a description of the process
in the README. Want to write it up and post it?
--
MST
^ permalink raw reply
* Re: [RFC PATCH 10/12] virtio/s390: consolidate DMA allocations
From: Cornelia Huck @ 2019-04-10 16:36 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410171254.71206015@oc2783563651>
On Wed, 10 Apr 2019 17:12:54 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Wed, 10 Apr 2019 10:46:49 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Fri, 5 Apr 2019 01:16:20 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> > > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> > > index aa45a6a027ae..7268149f2ee8 100644
> > > --- a/drivers/s390/virtio/virtio_ccw.c
> > > +++ b/drivers/s390/virtio/virtio_ccw.c
> > > @@ -49,12 +49,12 @@ struct vq_config_block {
> > > struct vcdev_dma_area {
> > > unsigned long indicators;
> > > unsigned long indicators2;
> > > + struct vq_config_block config_block;
> > > + __u8 status; /* TODO check __aligned(8); */
> >
> > ...I think that needs attention.
>
> Yes I wanted to discuss this with you. I could not find anything
> in the virtio spec that would put requirements on how this
> status field needs to be aligned. But I did not look to hard.
>
> The ccw.cda can hold an arbitrary data address AFAIR (for indirect,
> of course we do have alignment requirements).
I think it needs to be doubleword aligned.
>
> Apparently status used to be a normal field, and became a pointer with
> 73fa21ea4fc6 "KVM: s390: Dynamic allocation of virtio-ccw I/O
> data." (Cornelia Huck, 2013-01-07). I could not quite figure out why.
In the beginning, the code used a below-2G-area for all commands.
Rather than adding locking to avoid races there, that commit switches
to allocating the needed structures individually. The status field
needed to be below 2G, so it needed to be allocated separately.
>
> So maybe dropping the TODO comment will do just fine. What do you think?
^ permalink raw reply
* Re: [RFC PATCH 00/12] s390: virtio: support protected virtualization
From: Cornelia Huck @ 2019-04-10 16:24 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410175750.0ed0a454@oc2783563651>
On Wed, 10 Apr 2019 17:57:50 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Wed, 10 Apr 2019 11:20:48 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Fri, 5 Apr 2019 01:16:10 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> > > Enhanced virtualization protection technology may require the use of
> > > bounce buffers for I/O. While support for this was built into the
> > > virtio core, virtio-ccw wasn't changed accordingly.
> > >
> > > Thus what needs to be done to bring virtio-ccw up to speed with
> > > respect to this is:
> > > * use some 'new' common virtio stuff
> > > * make sure that virtio-ccw specific stuff uses shared memory when
> > > talking to the hypervisor (except communication blocks like ORB,
> > > these are handled by the hypervisor)
> > > * make sure the DMA API does what is necessary to talk through shared
> > > memory if we are a protected virtualization guest.
> > > * make sure the common IO layer plays along as well (airqs, sense).
> >
> > It would be good to have a summary somewhere in the code (or
> > Documentation/) as to what needs the dma treatment and what doesn't,
> > for later reference. We don't want people to accidentally break things
> > (especially if they cannot refer to architecture documentation - or
> > will at least some of that be published?)
> >
>
> I can put documentation on my TODO list. This cover letter was also
> supposed to provide a bird's-eye view on what needs to be done.
Some comments in the code to prevent further headscratching are also a
good idea.
>
> > >
> > > The series is structured in incremental fashion: some of the changes
> > > are overridden by following patches. The main reason why is that
> > > this is how I developed. But I think it ain't bad for the didactic
> > > and we are a bit more flexible with regards to throwing out some of
> > > the stuff in the end.
> >
> > FWIW, I think reshuffling the patches in the next iteration would ease
> > review.
> >
>
> Can you please tell me more about what is desired here? I mean, I made
> some tentative proposals on squashing some patches together. I don't
> remember any requests to reorder patches or split.
Just try to avoid to rewrite things multiple times -- if we agree on
the end result, we should be able to go there directly :)
^ permalink raw reply
* Re: [RFC PATCH 07/12] virtio/s390: use DMA memory for ccw I/O
From: Cornelia Huck @ 2019-04-10 16:21 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410164245.53f8b26d@oc2783563651>
On Wed, 10 Apr 2019 16:42:45 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Wed, 10 Apr 2019 10:42:51 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Fri, 5 Apr 2019 01:16:17 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> > > @@ -167,6 +170,28 @@ static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev)
> > > return container_of(vdev, struct virtio_ccw_device, vdev);
> > > }
> > >
> > > +#define vc_dma_decl_struct(type, field) \
> > > + dma_addr_t field ## _dma_addr; \
> > > + struct type *field
> > > +
> > > +static inline void *__vc_dma_alloc(struct virtio_device *vdev, size_t size,
> > > + dma_addr_t *dma_handle)
> > > +{
> > > + return dma_alloc_coherent(vdev->dev.parent, size, dma_handle,
> > > + GFP_DMA | GFP_KERNEL | __GFP_ZERO);
> > > +}
> > > +
> > > +static inline void __vc_dma_free(struct virtio_device *vdev, size_t size,
> > > + void *cpu_addr, dma_addr_t dma_handle)
> > > +{
> > > + dma_free_coherent(vdev->dev.parent, size, cpu_addr, dma_handle);
> > > +}
> > > +
> > > +#define vc_dma_alloc_struct(vdev, ptr) \
> > > + ({ ptr = __vc_dma_alloc(vdev, (sizeof(*(ptr))), &(ptr ## _dma_addr)); })
> > > +#define vc_dma_free_struct(vdev, ptr) \
> > > + __vc_dma_free(vdev, sizeof(*(ptr)), (ptr), (ptr ## _dma_addr))
> >
> > Not sure I'm a fan of those wrappers... I think they actually hurt
> > readability of the code.
> >
>
> By wrappers you mean just the macros or also the inline functions?
In particular, I dislike the macros.
>
> If we agree to go with the cio DMA pool instead of using DMA API
> facilities for allocation (dma_alloc_coherent or maybe a per ccw-device
> dma_pool) I think I could just use cio_dma_zalloc() directly if you like.
If we go with the pool (I'm not familiar enough with the dma stuff to
be able to make a good judgment there), nice and obvious calls sound
good to me :)
>
> I was quite insecure about how this gen_pool idea is going to be received
> here. That's why I decided to keep the dma_alloc_coherent() version in
> for the RFC.
>
> If you prefer I can squash patches #7 #9 #10 and #11 together and
> pull #8 forward. Would you prefer that?
If that avoids multiple switches of the approach used, that sounds like
a good idea.
(Still would like to see some feedback from others.)
^ permalink raw reply
* Re: [RFC PATCH 05/12] s390/cio: add protected virtualization support to cio
From: Cornelia Huck @ 2019-04-10 16:16 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410150225.61b86cd9@oc2783563651>
On Wed, 10 Apr 2019 15:02:25 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Wed, 10 Apr 2019 10:25:57 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> > So, what is a high-level summary of areas that need the treatment?
> > What I get from looking at the patches so far, it's:
> > - stuff that is written by the hypervisor's interrupt injection code:
> > IRB, indicators, etc.
>
> In interrupt context IRB behaves like a control block. We don't have
> to make it shared.
>
> I moved IRB because of snse:
>
>
> @@ -329,9 +329,9 @@ ccw_device_do_sense(struct ccw_device *cdev, struct
> irb *irb) /*
> * We have ending status but no sense information. Do a basic
> sense. */
> - sense_ccw = &to_io_private(sch)->sense_ccw;
> + sense_ccw = &to_io_private(sch)->dma_area->sense_ccw;
> sense_ccw->cmd_code = CCW_CMD_BASIC_SENSE;
> - sense_ccw->cda = (__u32) __pa(cdev->private->irb.ecw);
> + sense_ccw->cda = (__u32) __pa(cdev->private->dma_area->irb.ecw);
>
> as the irb.ecw is used as channel program data. And that needs to be
> shared.
Ah, I see. So it's more "guest points to some memory where the
hypervisor needs to write data"?
>
> > - buffers that are filled by a channel program: sense, sense id, etc.
> > - ccws themselves (because of translation?)
> >
>
> Right. The idea is: smallish, basically fixed size a readily available
> control blocks (specified as a ) are copied back and forth by the
As a what? :)
> ultravisor (via SIE SD).
Ok. It's really hard to be sure from the outside what falls into which
category :(
^ permalink raw reply
* Re: [RFC PATCH 04/12] s390/cio: introduce cio DMA pool
From: Cornelia Huck @ 2019-04-10 16:07 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190410173148.067555dc@oc2783563651>
On Wed, 10 Apr 2019 17:31:48 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Tue, 9 Apr 2019 19:14:53 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> [..]
>
> > > > At this point, you're always going via the css0 device. I'm
> > > > wondering whether you should pass in the cssid here and use
> > > > css_by_id(cssid) to make this future proof. But then, I'm not
> > > > quite clear from which context this will be called.
> > >
> > > As said before I don't see MCSS-E coming. Regarding the client code,
> > > please check out the rest of the series. Especially patch 6.
> > >
> > > From my perspective it would be at this stage saner to make it more
> > > obvious that only one css is supported (at the moment), than to
> > > implement MCSS-E, or to make this (kind of) MCSS-E ready.
> >
> > I disagree. I think there's value in keeping the interfaces clean
> > (within reasonable bounds, of course.) Even if there is no
> > implementation of MCSS-E other than QEMU... it is probably a good idea
> > to spend some brain cycles to make this conceptually clean.
>
> AFAIU Linux currently does not support MCSS-E. I don't have the
> bandwidth to implement MCSS-E support in the kernel.
>
> I fully agree for external interfaces should be MCSS-E conform, so
> should we ever decide to implement we don't have to overcome self-made
> obstacles.
>
> Kernel internal stuff however, IMHO, should avoid carrying a ballast of
> an 20%-30% implemented MCSS-E support. I see no benefit.
>
> But I don't insist. If you have a good idea how to make this more MCSS-E
> conform, you are welcome. In think something like this is best done on
> top of a series that provides a basic working solution. Especially if the
> conceptually clean thing is conceptually or code-wise more complex than
> the basic solution. If you have something in mind that is simpler and
> more lightweight than what I did here, I would be more than happy to make
> that happen.
I haven't asked for adding complete support for MCSS-E, just to keep it
in the back of our minds...
Back to the issue at hand. You're currently hard-wiring this to the
css0 device. My question is: Does it make sense to have a per-css area?
From my limited perspective, I think it does (more css images would
mean more devices, and just adding smaller areas per-css is probably
easier than adding one big area scaling with the number of css images).
In that case, what about tacking the area to the actual css object,
instead of having the global variable? Should be an easy change, and
feels cleaner.
^ permalink raw reply
* Re: [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Pankaj Gupta @ 2019-04-10 15:44 UTC (permalink / raw)
To: Yuval Shaia
Cc: jack, kvm, mst, david, qemu-devel, virtualization, adilger kernel,
zwisler, aarcange, dave jiang, linux-nvdimm, vishal l verma,
willy, hch, linux-acpi, jmoyer, linux-ext4, lenb, kilobyte, riel,
stefanha, pbonzini, dan j williams, lcapitulino, nilal, tytso,
xiaoguangrong eric, Cornelia Huck, rjw, linux-kernel, linux-xfs,
linux-fsdevel, imammedo, darrick wong <darrick.wong@
In-Reply-To: <20190410143826.GA7856@lap1>
> >
> > > This patch adds virtio-pmem driver for KVM guest.
> > >
> > > Guest reads the persistent memory range information from
> > > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > > creates a nd_region object with the persistent memory
> > > range information so that existing 'nvdimm/pmem' driver
> > > can reserve this into system memory map. This way
> > > 'virtio-pmem' driver uses existing functionality of pmem
> > > driver to register persistent memory compatible for DAX
> > > capable filesystems.
> > >
> > > This also provides function to perform guest flush over
> > > VIRTIO from 'pmem' driver when userspace performs flush
> > > on DAX memory range.
> > >
> > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > ---
> > > drivers/nvdimm/virtio_pmem.c | 88 ++++++++++++++++++++++
> > > drivers/virtio/Kconfig | 10 +++
> > > drivers/virtio/Makefile | 1 +
> > > drivers/virtio/pmem.c | 124 +++++++++++++++++++++++++++++++
> > > include/linux/virtio_pmem.h | 60 +++++++++++++++
> > > include/uapi/linux/virtio_ids.h | 1 +
> > > include/uapi/linux/virtio_pmem.h | 10 +++
> > > 7 files changed, 294 insertions(+)
> > > create mode 100644 drivers/nvdimm/virtio_pmem.c
> > > create mode 100644 drivers/virtio/pmem.c
> > > create mode 100644 include/linux/virtio_pmem.h
> > > create mode 100644 include/uapi/linux/virtio_pmem.h
> > >
> > (...)
> > > diff --git a/drivers/virtio/pmem.c b/drivers/virtio/pmem.c
> > > new file mode 100644
> > > index 000000000000..cc9de9589d56
> > > --- /dev/null
> > > +++ b/drivers/virtio/pmem.c
> > > @@ -0,0 +1,124 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * virtio_pmem.c: Virtio pmem Driver
> > > + *
> > > + * Discovers persistent memory range information
> > > + * from host and registers the virtual pmem device
> > > + * with libnvdimm core.
> > > + */
> > > +#include <linux/virtio_pmem.h>
> > > +#include <../../drivers/nvdimm/nd.h>
> > > +
> > > +static struct virtio_device_id id_table[] = {
> > > + { VIRTIO_ID_PMEM, VIRTIO_DEV_ANY_ID },
> > > + { 0 },
> > > +};
> > > +
> > > + /* Initialize virt queue */
> > > +static int init_vq(struct virtio_pmem *vpmem)
> >
> > IMHO, you don't gain much by splitting off this function...
>
> It make sense to have all the vq-init-related stuff in one function, so
> here pmem_lock and req_list are used only for the vq.
Yes.
> Saying that - maybe it would be better to have the 3 in one struct.
>
> >
> > > +{
> > > + struct virtqueue *vq;
> > > +
> > > + /* single vq */
> > > + vpmem->req_vq = vq = virtio_find_single_vq(vpmem->vdev,
> > > + host_ack, "flush_queue");
> > > + if (IS_ERR(vq))
> > > + return PTR_ERR(vq);
> >
> > I'm personally not a fan of chained assignments... I think I'd just
> > drop the 'vq' variable and operate on vpmem->req_vq directly.
>
> +1
Will drop extra vq.
>
> >
> > > +
> > > + spin_lock_init(&vpmem->pmem_lock);
> > > + INIT_LIST_HEAD(&vpmem->req_list);
> > > +
> > > + return 0;
> > > +};
> > > +
> > > +static int virtio_pmem_probe(struct virtio_device *vdev)
> > > +{
> > > + int err = 0;
> > > + struct resource res;
> > > + struct virtio_pmem *vpmem;
> > > + struct nvdimm_bus *nvdimm_bus;
> > > + struct nd_region_desc ndr_desc = {};
> > > + int nid = dev_to_node(&vdev->dev);
> > > + struct nd_region *nd_region;
> > > +
> > > + if (!vdev->config->get) {
> > > + dev_err(&vdev->dev, "%s failure: config disabled\n",
> >
> > Maybe s/config disabled/config access disabled/ ? That seems to be the
> > more common message.
> >
> > > + __func__);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + vdev->priv = vpmem = devm_kzalloc(&vdev->dev, sizeof(*vpmem),
> > > + GFP_KERNEL);
> >
> > Here, the vpmem variable makes sense for convenience, but I'm again not
> > a fan of the chaining :)
>
> +1
here as well.
>
> >
> > > + if (!vpmem) {
> > > + err = -ENOMEM;
> > > + goto out_err;
> > > + }
> > > +
> > > + vpmem->vdev = vdev;
> > > + err = init_vq(vpmem);
> > > + if (err)
> > > + goto out_err;
> > > +
> > > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > > + start, &vpmem->start);
> > > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > > + size, &vpmem->size);
> > > +
> > > + res.start = vpmem->start;
> > > + res.end = vpmem->start + vpmem->size-1;
> > > + vpmem->nd_desc.provider_name = "virtio-pmem";
> > > + vpmem->nd_desc.module = THIS_MODULE;
> > > +
> > > + vpmem->nvdimm_bus = nvdimm_bus = nvdimm_bus_register(&vdev->dev,
> > > + &vpmem->nd_desc);
> >
> > And here :)
> >
> > > + if (!nvdimm_bus)
> > > + goto out_vq;
> > > +
> > > + dev_set_drvdata(&vdev->dev, nvdimm_bus);
> > > +
> > > + ndr_desc.res = &res;
> > > + ndr_desc.numa_node = nid;
> > > + ndr_desc.flush = virtio_pmem_flush;
> > > + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
> > > + set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> > > + nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
> > > + nd_region->provider_data = dev_to_virtio
> > > + (nd_region->dev.parent->parent);
> >
> > Isn't it clear that this parent chain will always end up at &vdev->dev?
> > Maybe simply set ->provider_data to vdev directly? (Does it need to
> > grab a reference count of the device, BTW?)
> >
> > > +
> > > + if (!nd_region)
> > > + goto out_nd;
> >
> > Probably better to do this check before you access nd_region's
> > members :)
> >
> > > +
> > > + return 0;
> > > +out_nd:
> > > + err = -ENXIO;
> > > + nvdimm_bus_unregister(nvdimm_bus);
> > > +out_vq:
> > > + vdev->config->del_vqs(vdev);
> > > +out_err:
> > > + dev_err(&vdev->dev, "failed to register virtio pmem memory\n");
> > > + return err;
> > > +}
> > > +
> > > +static void virtio_pmem_remove(struct virtio_device *vdev)
> > > +{
> > > + struct virtio_pmem *vpmem = vdev->priv;
> > > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
> > > +
> > > + nvdimm_bus_unregister(nvdimm_bus);
> >
> > I haven't followed this around the nvdimm code, but is the nd_region
> > you created during probe cleaned up automatically, or would you need to
> > do something here?
> >
> > > + vdev->config->del_vqs(vdev);
> > > + vdev->config->reset(vdev);
> > > + kfree(vpmem);
> >
> > You allocated vpmem via devm_kzalloc; isn't it freed automatically on
> > remove?
> >
> > > +}
> > > +
> > > +static struct virtio_driver virtio_pmem_driver = {
> > > + .driver.name = KBUILD_MODNAME,
> > > + .driver.owner = THIS_MODULE,
> > > + .id_table = id_table,
> > > + .probe = virtio_pmem_probe,
> > > + .remove = virtio_pmem_remove,
> > > +};
> > > +
> > > +module_virtio_driver(virtio_pmem_driver);
> > > +MODULE_DEVICE_TABLE(virtio, id_table);
> > > +MODULE_DESCRIPTION("Virtio pmem driver");
> > > +MODULE_LICENSE("GPL");
> >
> > Only looked at this from the general virtio driver angle; seems fine
> > apart from some easy-to-fix issues and some personal style preference
> > things.
>
Best regards,
Pankaj
^ permalink raw reply
* Re: [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Pankaj Gupta @ 2019-04-10 15:38 UTC (permalink / raw)
To: Cornelia Huck
Cc: jack, kvm, mst, david, qemu-devel, virtualization, adilger kernel,
zwisler, aarcange, dave jiang, linux-nvdimm, vishal l verma,
willy, hch, linux-acpi, jmoyer, linux-ext4, lenb, kilobyte, riel,
yuval shaia, stefanha, pbonzini, dan j williams, lcapitulino,
nilal, tytso, xiaoguangrong eric, darrick wong, rjw, linux-kernel,
linux-xfs, linux-fsdevel, imamm
In-Reply-To: <20190410142426.5bf0d9a4.cohuck@redhat.com>
>
> > This patch adds virtio-pmem driver for KVM guest.
> >
> > Guest reads the persistent memory range information from
> > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > creates a nd_region object with the persistent memory
> > range information so that existing 'nvdimm/pmem' driver
> > can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem
> > driver to register persistent memory compatible for DAX
> > capable filesystems.
> >
> > This also provides function to perform guest flush over
> > VIRTIO from 'pmem' driver when userspace performs flush
> > on DAX memory range.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> > drivers/nvdimm/virtio_pmem.c | 88 ++++++++++++++++++++++
> > drivers/virtio/Kconfig | 10 +++
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/pmem.c | 124 +++++++++++++++++++++++++++++++
> > include/linux/virtio_pmem.h | 60 +++++++++++++++
> > include/uapi/linux/virtio_ids.h | 1 +
> > include/uapi/linux/virtio_pmem.h | 10 +++
> > 7 files changed, 294 insertions(+)
> > create mode 100644 drivers/nvdimm/virtio_pmem.c
> > create mode 100644 drivers/virtio/pmem.c
> > create mode 100644 include/linux/virtio_pmem.h
> > create mode 100644 include/uapi/linux/virtio_pmem.h
> >
> (...)
> > diff --git a/drivers/virtio/pmem.c b/drivers/virtio/pmem.c
> > new file mode 100644
> > index 000000000000..cc9de9589d56
> > --- /dev/null
> > +++ b/drivers/virtio/pmem.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and registers the virtual pmem device
> > + * with libnvdimm core.
> > + */
> > +#include <linux/virtio_pmem.h>
> > +#include <../../drivers/nvdimm/nd.h>
> > +
> > +static struct virtio_device_id id_table[] = {
> > + { VIRTIO_ID_PMEM, VIRTIO_DEV_ANY_ID },
> > + { 0 },
> > +};
> > +
> > + /* Initialize virt queue */
> > +static int init_vq(struct virtio_pmem *vpmem)
>
> IMHO, you don't gain much by splitting off this function...
o.k. I kept this for better code structure.
>
> > +{
> > + struct virtqueue *vq;
> > +
> > + /* single vq */
> > + vpmem->req_vq = vq = virtio_find_single_vq(vpmem->vdev,
> > + host_ack, "flush_queue");
> > + if (IS_ERR(vq))
> > + return PTR_ERR(vq);
>
> I'm personally not a fan of chained assignments... I think I'd just
> drop the 'vq' variable and operate on vpmem->req_vq directly.
Sure.
>
> > +
> > + spin_lock_init(&vpmem->pmem_lock);
> > + INIT_LIST_HEAD(&vpmem->req_list);
> > +
> > + return 0;
> > +};
> > +
> > +static int virtio_pmem_probe(struct virtio_device *vdev)
> > +{
> > + int err = 0;
> > + struct resource res;
> > + struct virtio_pmem *vpmem;
> > + struct nvdimm_bus *nvdimm_bus;
> > + struct nd_region_desc ndr_desc = {};
> > + int nid = dev_to_node(&vdev->dev);
> > + struct nd_region *nd_region;
> > +
> > + if (!vdev->config->get) {
> > + dev_err(&vdev->dev, "%s failure: config disabled\n",
>
> Maybe s/config disabled/config access disabled/ ? That seems to be the
> more common message.
This is better.
>
> > + __func__);
> > + return -EINVAL;
> > + }
> > +
> > + vdev->priv = vpmem = devm_kzalloc(&vdev->dev, sizeof(*vpmem),
> > + GFP_KERNEL);
>
> Here, the vpmem variable makes sense for convenience, but I'm again not
> a fan of the chaining :)
Sure will change :)
>
> > + if (!vpmem) {
> > + err = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + vpmem->vdev = vdev;
> > + err = init_vq(vpmem);
> > + if (err)
> > + goto out_err;
> > +
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + start, &vpmem->start);
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + size, &vpmem->size);
> > +
> > + res.start = vpmem->start;
> > + res.end = vpmem->start + vpmem->size-1;
> > + vpmem->nd_desc.provider_name = "virtio-pmem";
> > + vpmem->nd_desc.module = THIS_MODULE;
> > +
> > + vpmem->nvdimm_bus = nvdimm_bus = nvdimm_bus_register(&vdev->dev,
> > + &vpmem->nd_desc);
>
> And here :)
Sure.
>
> > + if (!nvdimm_bus)
> > + goto out_vq;
> > +
> > + dev_set_drvdata(&vdev->dev, nvdimm_bus);
> > +
> > + ndr_desc.res = &res;
> > + ndr_desc.numa_node = nid;
> > + ndr_desc.flush = virtio_pmem_flush;
> > + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
> > + set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> > + nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
> > + nd_region->provider_data = dev_to_virtio
> > + (nd_region->dev.parent->parent);
>
> Isn't it clear that this parent chain will always end up at &vdev->dev?
Yes, It will resolve to &vdev->dev.
> Maybe simply set ->provider_data to vdev directly? (Does it need to
> grab a reference count of the device, BTW?)
reference is already taken when registering the device with nvdimm_bus.
>
> > +
> > + if (!nd_region)
> > + goto out_nd;
>
> Probably better to do this check before you access nd_region's
> members :)
ah Sorry! Will correct this.
>
> > +
> > + return 0;
> > +out_nd:
> > + err = -ENXIO;
> > + nvdimm_bus_unregister(nvdimm_bus);
> > +out_vq:
> > + vdev->config->del_vqs(vdev);
> > +out_err:
> > + dev_err(&vdev->dev, "failed to register virtio pmem memory\n");
> > + return err;
> > +}
> > +
> > +static void virtio_pmem_remove(struct virtio_device *vdev)
> > +{
> > + struct virtio_pmem *vpmem = vdev->priv;
> > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
> > +
> > + nvdimm_bus_unregister(nvdimm_bus);
>
> I haven't followed this around the nvdimm code, but is the nd_region
> you created during probe cleaned up automatically, or would you need to
> do something here?
'nvdimm_bus_unregister' does that.
>
> > + vdev->config->del_vqs(vdev);
> > + vdev->config->reset(vdev);
> > + kfree(vpmem);
>
> You allocated vpmem via devm_kzalloc; isn't it freed automatically on
> remove?
yes. Will remove free(vpmem).
>
> > +}
> > +
> > +static struct virtio_driver virtio_pmem_driver = {
> > + .driver.name = KBUILD_MODNAME,
> > + .driver.owner = THIS_MODULE,
> > + .id_table = id_table,
> > + .probe = virtio_pmem_probe,
> > + .remove = virtio_pmem_remove,
> > +};
> > +
> > +module_virtio_driver(virtio_pmem_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("Virtio pmem driver");
> > +MODULE_LICENSE("GPL");
>
> Only looked at this from the general virtio driver angle; seems fine
> apart from some easy-to-fix issues and some personal style preference
> things.
Thank you for the review.
Best regards,
Pankaj
>
^ permalink raw reply
* Re: [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Yuval Shaia @ 2019-04-10 14:41 UTC (permalink / raw)
To: Pankaj Gupta
Cc: cohuck, jack, kvm, mst, david, qemu-devel, virtualization,
adilger.kernel, zwisler, aarcange, dave.jiang, linux-nvdimm,
vishal.l.verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
kilobyte, riel, stefanha, pbonzini, dan.j.williams, lcapitulino,
nilal, tytso, xiaoguangrong.eric, darrick.wong, rjw, linux-kernel,
linux-xfs, linux-fsdevel, imammedo
In-Reply-To: <20190410040826.24371-3-pagupta@redhat.com>
> +
> +static int virtio_pmem_probe(struct virtio_device *vdev)
> +{
> + int err = 0;
> + struct resource res;
> + struct virtio_pmem *vpmem;
> + struct nvdimm_bus *nvdimm_bus;
> + struct nd_region_desc ndr_desc = {};
> + int nid = dev_to_node(&vdev->dev);
> + struct nd_region *nd_region;
> +
> + if (!vdev->config->get) {
> + dev_err(&vdev->dev, "%s failure: config disabled\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + vdev->priv = vpmem = devm_kzalloc(&vdev->dev, sizeof(*vpmem),
> + GFP_KERNEL);
> + if (!vpmem) {
> + err = -ENOMEM;
> + goto out_err;
> + }
> +
> + vpmem->vdev = vdev;
> + err = init_vq(vpmem);
> + if (err)
> + goto out_err;
> +
> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + start, &vpmem->start);
> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + size, &vpmem->size);
> +
> + res.start = vpmem->start;
> + res.end = vpmem->start + vpmem->size-1;
> + vpmem->nd_desc.provider_name = "virtio-pmem";
> + vpmem->nd_desc.module = THIS_MODULE;
> +
> + vpmem->nvdimm_bus = nvdimm_bus = nvdimm_bus_register(&vdev->dev,
> + &vpmem->nd_desc);
> + if (!nvdimm_bus)
> + goto out_vq;
> +
> + dev_set_drvdata(&vdev->dev, nvdimm_bus);
> +
> + ndr_desc.res = &res;
> + ndr_desc.numa_node = nid;
> + ndr_desc.flush = virtio_pmem_flush;
> + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
> + set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> + nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
> + nd_region->provider_data = dev_to_virtio
Pleas delete the extra space.
> + (nd_region->dev.parent->parent);
> +
> + if (!nd_region)
> + goto out_nd;
> +
> + return 0;
> +out_nd:
> + err = -ENXIO;
> + nvdimm_bus_unregister(nvdimm_bus);
> +out_vq:
> + vdev->config->del_vqs(vdev);
> +out_err:
> + dev_err(&vdev->dev, "failed to register virtio pmem memory\n");
> + return err;
> +}
^ permalink raw reply
* Re: [PATCH v5 2/5] virtio-pmem: Add virtio pmem driver
From: Yuval Shaia @ 2019-04-10 14:38 UTC (permalink / raw)
To: Cornelia Huck
Cc: Pankaj Gupta, jack, kvm, mst, david, qemu-devel, virtualization,
adilger.kernel, zwisler, aarcange, dave.jiang, linux-nvdimm,
vishal.l.verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
kilobyte, riel, stefanha, pbonzini, dan.j.williams, lcapitulino,
nilal, tytso, xiaoguangrong.eric, darrick.wong, rjw, linux-kernel,
linux-xfs, linux-fsdevel, imammedo
In-Reply-To: <20190410142426.5bf0d9a4.cohuck@redhat.com>
On Wed, Apr 10, 2019 at 02:24:26PM +0200, Cornelia Huck wrote:
> On Wed, 10 Apr 2019 09:38:22 +0530
> Pankaj Gupta <pagupta@redhat.com> wrote:
>
> > This patch adds virtio-pmem driver for KVM guest.
> >
> > Guest reads the persistent memory range information from
> > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > creates a nd_region object with the persistent memory
> > range information so that existing 'nvdimm/pmem' driver
> > can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem
> > driver to register persistent memory compatible for DAX
> > capable filesystems.
> >
> > This also provides function to perform guest flush over
> > VIRTIO from 'pmem' driver when userspace performs flush
> > on DAX memory range.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> > drivers/nvdimm/virtio_pmem.c | 88 ++++++++++++++++++++++
> > drivers/virtio/Kconfig | 10 +++
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/pmem.c | 124 +++++++++++++++++++++++++++++++
> > include/linux/virtio_pmem.h | 60 +++++++++++++++
> > include/uapi/linux/virtio_ids.h | 1 +
> > include/uapi/linux/virtio_pmem.h | 10 +++
> > 7 files changed, 294 insertions(+)
> > create mode 100644 drivers/nvdimm/virtio_pmem.c
> > create mode 100644 drivers/virtio/pmem.c
> > create mode 100644 include/linux/virtio_pmem.h
> > create mode 100644 include/uapi/linux/virtio_pmem.h
> >
> (...)
> > diff --git a/drivers/virtio/pmem.c b/drivers/virtio/pmem.c
> > new file mode 100644
> > index 000000000000..cc9de9589d56
> > --- /dev/null
> > +++ b/drivers/virtio/pmem.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and registers the virtual pmem device
> > + * with libnvdimm core.
> > + */
> > +#include <linux/virtio_pmem.h>
> > +#include <../../drivers/nvdimm/nd.h>
> > +
> > +static struct virtio_device_id id_table[] = {
> > + { VIRTIO_ID_PMEM, VIRTIO_DEV_ANY_ID },
> > + { 0 },
> > +};
> > +
> > + /* Initialize virt queue */
> > +static int init_vq(struct virtio_pmem *vpmem)
>
> IMHO, you don't gain much by splitting off this function...
It make sense to have all the vq-init-related stuff in one function, so
here pmem_lock and req_list are used only for the vq.
Saying that - maybe it would be better to have the 3 in one struct.
>
> > +{
> > + struct virtqueue *vq;
> > +
> > + /* single vq */
> > + vpmem->req_vq = vq = virtio_find_single_vq(vpmem->vdev,
> > + host_ack, "flush_queue");
> > + if (IS_ERR(vq))
> > + return PTR_ERR(vq);
>
> I'm personally not a fan of chained assignments... I think I'd just
> drop the 'vq' variable and operate on vpmem->req_vq directly.
+1
>
> > +
> > + spin_lock_init(&vpmem->pmem_lock);
> > + INIT_LIST_HEAD(&vpmem->req_list);
> > +
> > + return 0;
> > +};
> > +
> > +static int virtio_pmem_probe(struct virtio_device *vdev)
> > +{
> > + int err = 0;
> > + struct resource res;
> > + struct virtio_pmem *vpmem;
> > + struct nvdimm_bus *nvdimm_bus;
> > + struct nd_region_desc ndr_desc = {};
> > + int nid = dev_to_node(&vdev->dev);
> > + struct nd_region *nd_region;
> > +
> > + if (!vdev->config->get) {
> > + dev_err(&vdev->dev, "%s failure: config disabled\n",
>
> Maybe s/config disabled/config access disabled/ ? That seems to be the
> more common message.
>
> > + __func__);
> > + return -EINVAL;
> > + }
> > +
> > + vdev->priv = vpmem = devm_kzalloc(&vdev->dev, sizeof(*vpmem),
> > + GFP_KERNEL);
>
> Here, the vpmem variable makes sense for convenience, but I'm again not
> a fan of the chaining :)
+1
>
> > + if (!vpmem) {
> > + err = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + vpmem->vdev = vdev;
> > + err = init_vq(vpmem);
> > + if (err)
> > + goto out_err;
> > +
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + start, &vpmem->start);
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + size, &vpmem->size);
> > +
> > + res.start = vpmem->start;
> > + res.end = vpmem->start + vpmem->size-1;
> > + vpmem->nd_desc.provider_name = "virtio-pmem";
> > + vpmem->nd_desc.module = THIS_MODULE;
> > +
> > + vpmem->nvdimm_bus = nvdimm_bus = nvdimm_bus_register(&vdev->dev,
> > + &vpmem->nd_desc);
>
> And here :)
>
> > + if (!nvdimm_bus)
> > + goto out_vq;
> > +
> > + dev_set_drvdata(&vdev->dev, nvdimm_bus);
> > +
> > + ndr_desc.res = &res;
> > + ndr_desc.numa_node = nid;
> > + ndr_desc.flush = virtio_pmem_flush;
> > + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
> > + set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> > + nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
> > + nd_region->provider_data = dev_to_virtio
> > + (nd_region->dev.parent->parent);
>
> Isn't it clear that this parent chain will always end up at &vdev->dev?
> Maybe simply set ->provider_data to vdev directly? (Does it need to
> grab a reference count of the device, BTW?)
>
> > +
> > + if (!nd_region)
> > + goto out_nd;
>
> Probably better to do this check before you access nd_region's
> members :)
>
> > +
> > + return 0;
> > +out_nd:
> > + err = -ENXIO;
> > + nvdimm_bus_unregister(nvdimm_bus);
> > +out_vq:
> > + vdev->config->del_vqs(vdev);
> > +out_err:
> > + dev_err(&vdev->dev, "failed to register virtio pmem memory\n");
> > + return err;
> > +}
> > +
> > +static void virtio_pmem_remove(struct virtio_device *vdev)
> > +{
> > + struct virtio_pmem *vpmem = vdev->priv;
> > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
> > +
> > + nvdimm_bus_unregister(nvdimm_bus);
>
> I haven't followed this around the nvdimm code, but is the nd_region
> you created during probe cleaned up automatically, or would you need to
> do something here?
>
> > + vdev->config->del_vqs(vdev);
> > + vdev->config->reset(vdev);
> > + kfree(vpmem);
>
> You allocated vpmem via devm_kzalloc; isn't it freed automatically on
> remove?
>
> > +}
> > +
> > +static struct virtio_driver virtio_pmem_driver = {
> > + .driver.name = KBUILD_MODNAME,
> > + .driver.owner = THIS_MODULE,
> > + .id_table = id_table,
> > + .probe = virtio_pmem_probe,
> > + .remove = virtio_pmem_remove,
> > +};
> > +
> > +module_virtio_driver(virtio_pmem_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("Virtio pmem driver");
> > +MODULE_LICENSE("GPL");
>
> Only looked at this from the general virtio driver angle; seems fine
> apart from some easy-to-fix issues and some personal style preference
> things.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox