[Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

This patch series adds an implementation of the virtio-mmio
transport, and uses it in the vexpress-a9 and vexpress-a15
board models.

The basic idea is that the board instantiates some transports,
the user can create backends which automatically plug into them
(via -device virtio-blk-backend and the like), and we tell the
guest kernel about them using a new arm/boot hook that lets
the board modify the user's device tree blob.

(The last part of this represents another reluctant step into
manipulating device trees; however interrupt-controller and
virtio nodes should be sufficiently stable to be safe as these
things go.)

Is there a convenient way to say "don't create the transport
if there's no backend to plug into it"?

Sample command line:

./build/x86/arm-softmmu/qemu-system-arm -machine type=vexpress-a15 \
  -serial stdio -display none   -kernel zImage \
  -dtb vexpress-v2p-ca15-tc1.dtb \
  -append "root=/dev/mmcblk0 rw console=ttyAMA0 rootwait earlyprintk" \
  -sd arm-wheezy.img -device virtio-serial-device \
  -drive if=none,file=debian_lenny_arm_small.qcow2,id=foo \
  -device virtio-blk-device,drive=foo 

(note that '-device virtio-blk' and '-drive if=virtio' won't
work since they will result in the creation of PCI virtio devices.)

The first two patches here are effectively a 'v2' of the versions
I sent out earlier this week; I've updated them as per review so
they no longer have the awkward "varargs don't promote" problem.

The core virtio patches are based initially on the prototype
versions I wrote back in 2011, which Fred Konrad has subsequently
updated to work on top of his virtio refactoring. I then did some
minor fixes and cleanup.

When I get mach-virt into shape that will use virtio-mmio too.

Peter Maydell (8):
  device_tree: Add qemu_devtree_setprop_sized_cells() utility functions
  arm/boot: Use qemu_devtree_setprop_sized_cells()
  virtio: Add support for guest setting of queue size
  virtio: Support transports which can specify the vring alignment
  virtio: Implement MMIO based virtio transport
  arm/boot: Allow boards to modify the FDT blob
  vexpress: Make VEDBoardInfo extend arm_boot_info
  vexpress: Add virtio-mmio transports

 device_tree.c                  |   31 +++
 hw/arm/boot.c                  |   33 ++--
 hw/arm/vexpress.c              |  128 ++++++++++--
 hw/virtio/Makefile.objs        |    1 +
 hw/virtio/virtio-mmio.c        |  424 ++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio.c             |   38 +++-
 include/hw/arm/arm.h           |    4 +
 include/hw/virtio/virtio-bus.h |    6 +
 include/hw/virtio/virtio.h     |    2 +
 include/sysemu/device_tree.h   |   62 ++++++
 10 files changed, 690 insertions(+), 39 deletions(-)
 create mode 100644 hw/virtio/virtio-mmio.c

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/8] device_tree: Add qemu_devtree_setprop_sized_cells() utility functions

[Peter Maydell]

We already have a qemu_devtree_setprop_cells() which sets a dtb
property to an array of cells whose values are specified by varargs.
However for the fairly common case of setting a property to a list
of addresses or of address,size pairs the number of cells used by
each element in the list depends on the parent's #address-cells
and #size-cells properties. To make this easier we provide an analogous
qemu_devtree_setprop_sized_cells() macro which allows the number
of cells used by each element to be specified. This is implemented
using an underlying qemu_devtree_setprop_sized_cells_from_array()
function which takes the values and sizes as an array; this may
also be directly useful for cases where the cell contents are
constructed programmatically.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 device_tree.c                |   31 +++++++++++++++++++++
 include/sysemu/device_tree.h |   62 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index 69be9da..38d6c09 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -319,3 +319,34 @@ void qemu_devtree_dumpdtb(void *fdt, int size)
     }
 
 }
+
+int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
+                                                const char *node_path,
+                                                const char *property,
+                                                int numvalues,
+                                                uint64_t *values)
+{
+    uint32_t *propcells;
+    uint64_t value;
+    int cellnum, vnum, ncells;
+    uint32_t hival;
+
+    propcells = g_new0(uint32_t, numvalues * 2);
+
+    cellnum = 0;
+    for (vnum = 0; vnum < numvalues; vnum++) {
+        ncells = values[vnum * 2];
+        assert(ncells == 1 || ncells == 2);
+        value = values[vnum * 2 + 1];
+        hival = cpu_to_be32(value >> 32);
+        if (ncells > 1) {
+            propcells[cellnum++] = hival;
+        } else if (hival != 0) {
+            return vnum + 1;
+        }
+        propcells[cellnum++] = cpu_to_be32(value);
+    }
+
+    return qemu_devtree_setprop(fdt, node_path, property, propcells,
+                                cellnum * sizeof(uint32_t));
+}
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index f0b3f35..8189ebc 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -51,4 +51,66 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
 
 void qemu_devtree_dumpdtb(void *fdt, int size);
 
+/**
+ * qemu_devtree_setprop_sized_cells_from_array:
+ * @fdt: device tree blob
+ * @node_path: node to set property on
+ * @property: property to set
+ * @numvalues: number of values
+ * @values: array of number-of-cells, value pairs
+ *
+ * Set the specified property on the specified node in the device tree
+ * to be an array of cells. The values of the cells are specified via
+ * the values list, which alternates between "number of cells used by
+ * this value" and "value".
+ * number-of-cells must be either 1 or 2 (other values will assert()).
+ *
+ * This function is useful because device tree nodes often have cell arrays
+ * which are either lists of addresses or lists of address,size tuples, but
+ * the number of cells used for each element vary depending on the
+ * #address-cells and #size-cells properties of their parent node.
+ * If you know all your cell elements are one cell wide you can use the
+ * simpler qemu_devtree_setprop_cells(). If you're not setting up the
+ * array programmatically, qemu_devtree_setprop_sized_cells may be more
+ * convenient.
+ *
+ * Return value: 0 on success, <0 if setting the property failed,
+ * n (for n>0) if value n wouldn't fit in the required number of cells.
+ * (This slightly odd convention is for the benefit of callers who might
+ * wish to print different error messages depending on which value
+ * was too large to fit, since values might be user-specified.)
+ */
+int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
+                                                const char *node_path,
+                                                const char *property,
+                                                int numvalues,
+                                                uint64_t *values);
+
+/**
+ * qemu_devtree_setprop_sized_cells:
+ * @fdt: device tree blob
+ * @node_path: node to set property on
+ * @property: property to set
+ * @...: list of number-of-cells, value pairs
+ *
+ * Set the specified property on the specified node in the device tree
+ * to be an array of cells. The values of the cells are specified via
+ * the variable arguments, which alternates between "number of cells
+ * used by this value" and "value".
+ *
+ * This is a convenience wrapper for the function
+ * qemu_devtree_setprop_sized_cells_from_array().
+ *
+ * Return value: 0 on success, <0 if setting the property failed,
+ * n (for n>0) if value n wouldn't fit in the required number of cells.
+ */
+#define qemu_devtree_setprop_sized_cells(fdt, node_path, property, ...)       \
+    ({                                                                        \
+        uint64_t qdt_tmp[] = { __VA_ARGS__ };                                 \
+        qemu_devtree_setprop_sized_cells_from_array(fdt, node_path,           \
+                                                    property,                 \
+                                                    ARRAY_SIZE(qdt_tmp) / 2,  \
+                                                    qdt_tmp);                 \
+    })
+
 #endif /* __DEVICE_TREE_H__ */
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Maydell]

Replace the opencoded assembly of the reg property array for the
/memory node with a call to qemu_devtree_setprop_sized_cells().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c |   28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c0090f..eb9a864 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
 
 static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
 {
-    uint32_t *mem_reg_property;
-    uint32_t mem_reg_propsize;
     void *fdt = NULL;
     char *filename;
     int size, rc;
-    uint32_t acells, scells, hival;
+    uint32_t acells, scells;
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
     if (!filename) {
@@ -255,30 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
         goto fail;
     }
 
-    mem_reg_propsize = acells + scells;
-    mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
-    mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
-    hival = cpu_to_be32(binfo->loader_start >> 32);
-    if (acells > 1) {
-        mem_reg_property[acells - 2] = hival;
-    } else if (hival != 0) {
+    rc = qemu_devtree_setprop_sized_cells(fdt, "/memory", "reg",
+                                          acells, binfo->loader_start,
+                                          scells, binfo->ram_size);
+    if (rc == 1) {
         fprintf(stderr, "qemu: dtb file not compatible with "
                 "RAM start address > 4GB\n");
         goto fail;
-    }
-    mem_reg_property[acells + scells - 1] = cpu_to_be32(binfo->ram_size);
-    hival = cpu_to_be32(binfo->ram_size >> 32);
-    if (scells > 1) {
-        mem_reg_property[acells + scells - 2] = hival;
-    } else if (hival != 0) {
+    } else if (rc == 2) {
         fprintf(stderr, "qemu: dtb file not compatible with "
                 "RAM size > 4GB\n");
         goto fail;
-    }
-
-    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
-                              mem_reg_propsize * sizeof(uint32_t));
-    if (rc < 0) {
+    } else if (rc != 0) {
         fprintf(stderr, "couldn't set /memory/reg\n");
         goto fail;
     }
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/8] virtio: Add support for guest setting of queue size

[Peter Maydell]

The MMIO virtio transport spec allows the guest to tell the host how
large the queue size is. Add virtio_queue_set_num() function which
implements this in the QEMU common virtio support code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio.c         |    6 ++++++
 include/hw/virtio/virtio.h |    1 +
 2 files changed, 7 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8176c14..8805b8a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -667,6 +667,12 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
     return vdev->vq[n].pa;
 }
 
+void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
+{
+    vdev->vq[n].vring.num = num;
+    virtqueue_init(&vdev->vq[n]);
+}
+
 int virtio_queue_get_num(VirtIODevice *vdev, int n)
 {
     return vdev->vq[n].vring.num;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a6c5c53..95c4772 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -198,6 +198,7 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
+void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
 void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
-- 
1.7.9.5

[Qemu-devel] [PATCH 4/8] virtio: Support transports which can specify the vring alignment

[Peter Maydell]

Support virtio transports which can specify the vring alignment
(ie where the guest communicates this to the host) by providing
a new virtio_queue_set_align() function. (The default alignment
remains as before.)

Transports which wish to make use of this must set the
has_variable_vring_alignment field in their VirtioBusClass
struct to true; they can then change the alignment via
virtio_queue_set_align().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio.c             |   32 +++++++++++++++++++++++++++++---
 include/hw/virtio/virtio-bus.h |    6 ++++++
 include/hw/virtio/virtio.h     |    1 +
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8805b8a..9e1a580 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -19,8 +19,11 @@
 #include "qemu/atomic.h"
 #include "hw/virtio/virtio-bus.h"
 
-/* The alignment to use between consumer and producer parts of vring.
- * x86 pagesize again. */
+/*
+ * The alignment to use between consumer and producer parts of vring.
+ * x86 pagesize again. This is the default, used by transports like PCI
+ * which don't provide a means for the guest to tell the host the alignment.
+ */
 #define VIRTIO_PCI_VRING_ALIGN         4096
 
 typedef struct VRingDesc
@@ -54,6 +57,7 @@ typedef struct VRingUsed
 typedef struct VRing
 {
     unsigned int num;
+    unsigned int align;
     hwaddr desc;
     hwaddr avail;
     hwaddr used;
@@ -93,7 +97,7 @@ static void virtqueue_init(VirtQueue *vq)
     vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
     vq->vring.used = vring_align(vq->vring.avail +
                                  offsetof(VRingAvail, ring[vq->vring.num]),
-                                 VIRTIO_PCI_VRING_ALIGN);
+                                 vq->vring.align);
 }
 
 static inline uint64_t vring_desc_addr(hwaddr desc_pa, int i)
@@ -685,6 +689,21 @@ int virtio_queue_get_id(VirtQueue *vq)
     return vq - &vdev->vq[0];
 }
 
+void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    /* Check that the transport told us it was going to do this
+     * (so a buggy transport will immediately assert rather than
+     * silently failing to migrate this state)
+     */
+    assert(k->has_variable_vring_alignment);
+
+    vdev->vq[n].vring.align = align;
+    virtqueue_init(&vdev->vq[n]);
+}
+
 void virtio_queue_notify_vq(VirtQueue *vq)
 {
     if (vq->vring.desc) {
@@ -725,6 +744,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
         abort();
 
     vdev->vq[i].vring.num = queue_size;
+    vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
     vdev->vq[i].handle_output = handle_output;
 
     return &vdev->vq[i];
@@ -831,6 +851,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
             break;
 
         qemu_put_be32(f, vdev->vq[i].vring.num);
+        if (k->has_variable_vring_alignment) {
+            qemu_put_be32(f, vdev->vq[i].vring.align);
+        }
         qemu_put_be64(f, vdev->vq[i].pa);
         qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
         if (k->save_queue) {
@@ -887,6 +910,9 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 
     for (i = 0; i < num; i++) {
         vdev->vq[i].vring.num = qemu_get_be32(f);
+        if (k->has_variable_vring_alignment) {
+            vdev->vq[i].vring.align = qemu_get_be32(f);
+        }
         vdev->vq[i].pa = qemu_get_be64(f);
         qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
         vdev->vq[i].signalled_used_valid = false;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 9ed60f9..9217f85 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -62,6 +62,12 @@ typedef struct VirtioBusClass {
      * This is called by virtio-bus just before the device is unplugged.
      */
     void (*device_unplug)(DeviceState *d);
+    /*
+     * Does the transport have variable vring alignment?
+     * (ie can it ever call virtio_queue_set_align()?)
+     * Note that changing this will break migration for this transport.
+     */
+    bool has_variable_vring_alignment;
 } VirtioBusClass;
 
 struct VirtioBusState {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 95c4772..6c2cee6 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -200,6 +200,7 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
+void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
 void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
 void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
-- 
1.7.9.5

[Qemu-devel] [PATCH 5/8] virtio: Implement MMIO based virtio transport

[Peter Maydell]

Add support for the generic MMIO based virtio transport.

This patch includes some fixes for bugs spotted by
Ying-Shiuan Pan <yspan@itri.org.tw>.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
[Fred changes: updated to new virtio-bus mechanisms]
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
[PMM changes:
 * fixed trivial makefile conflict
 * removed unused int_enable
 * host_features doesn't need migrating
 * reset guest accessible state in the reset function
 * minor style fixes like extra blank lines
 * RAZ/WI if there's no backend
 * made transport size 0x200, in line with kvmtool
 * set has_variable_vring_alignment
]
---
 hw/virtio/Makefile.objs |    1 +
 hw/virtio/virtio-mmio.c |  424 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 425 insertions(+)
 create mode 100644 hw/virtio/virtio-mmio.c

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index cbe6d51..1ba53d9 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -1,6 +1,7 @@
 common-obj-y += virtio-rng.o
 common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 common-obj-y += virtio-bus.o
+common-obj-y += virtio-mmio.o
 common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
 
 obj-y += virtio.o virtio-balloon.o 
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
new file mode 100644
index 0000000..4c16616
--- /dev/null
+++ b/hw/virtio/virtio-mmio.c
@@ -0,0 +1,424 @@
+/*
+ * Virtio MMIO bindings
+ *
+ * Copyright (c) 2011 Linaro Limited
+ *
+ * Author:
+ *  Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License; 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/host-utils.h"
+#include "hw/virtio/virtio-bus.h"
+
+/* #define DEBUG_VIRTIO_MMIO */
+
+#ifdef DEBUG_VIRTIO_MMIO
+
+#define DPRINTF(fmt, ...) \
+do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+/* QOM macros */
+/* virtio-mmio-bus */
+#define TYPE_VIRTIO_MMIO_BUS "virtio-mmio-bus"
+#define VIRTIO_MMIO_BUS(obj) \
+        OBJECT_CHECK(VirtioBusState, (obj), TYPE_VIRTIO_MMIO_BUS)
+#define VIRTIO_MMIO_BUS_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioBusClass, (obj), TYPE_VIRTIO_MMIO_BUS)
+#define VIRTIO_MMIO_BUS_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioBusClass, (klass), TYPE_VIRTIO_MMIO_BUS)
+
+/* virtio-mmio */
+#define TYPE_VIRTIO_MMIO "virtio-mmio"
+#define VIRTIO_MMIO(obj) \
+        OBJECT_CHECK(VirtIOMMIOProxy, (obj), TYPE_VIRTIO_MMIO)
+
+/* Memory mapped register offsets */
+#define VIRTIO_MMIO_MAGIC 0x0
+#define VIRTIO_MMIO_VERSION 0x4
+#define VIRTIO_MMIO_DEVICEID 0x8
+#define VIRTIO_MMIO_VENDORID 0xc
+#define VIRTIO_MMIO_HOSTFEATURES 0x10
+#define VIRTIO_MMIO_HOSTFEATURESSEL 0x14
+#define VIRTIO_MMIO_GUESTFEATURES 0x20
+#define VIRTIO_MMIO_GUESTFEATURESSEL 0x24
+#define VIRTIO_MMIO_GUESTPAGESIZE 0x28
+#define VIRTIO_MMIO_QUEUESEL 0x30
+#define VIRTIO_MMIO_QUEUENUMMAX 0x34
+#define VIRTIO_MMIO_QUEUENUM 0x38
+#define VIRTIO_MMIO_QUEUEALIGN 0x3c
+#define VIRTIO_MMIO_QUEUEPFN 0x40
+#define VIRTIO_MMIO_QUEUENOTIFY 0x50
+#define VIRTIO_MMIO_INTERRUPTSTATUS 0x60
+#define VIRTIO_MMIO_INTERRUPTACK 0x64
+#define VIRTIO_MMIO_STATUS 0x70
+/* Device specific config space starts here */
+#define VIRTIO_MMIO_CONFIG 0x100
+
+#define VIRT_MAGIC 0x74726976 /* 'virt' */
+#define VIRT_VERSION 1
+#define VIRT_VENDOR 0x554D4551 /* 'QEMU' */
+
+typedef struct {
+    /* Generic */
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+    qemu_irq irq;
+    uint32_t host_features;
+    /* Guest accessible state needing migration and reset */
+    uint32_t host_features_sel;
+    uint32_t guest_features_sel;
+    uint32_t guest_page_shift;
+    /* virtio-bus */
+    VirtioBusState bus;
+} VirtIOMMIOProxy;
+
+static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev);
+
+static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->bus.vdev;
+
+    DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
+
+    if (!vdev) {
+        /* If no backend is present, we treat most registers as
+         * read-as-zero, except for the magic number, version and
+         * vendor ID. This is not strictly sanctioned by the virtio
+         * spec, but it allows us to provide transports with no backend
+         * plugged in which don't confuse Linux's virtio code: the
+         * probe won't complain about the bad magic number, but the
+         * device ID of zero means no backend will claim it.
+         */
+        switch (offset) {
+        case VIRTIO_MMIO_MAGIC:
+            return VIRT_MAGIC;
+        case VIRTIO_MMIO_VERSION:
+            return VIRT_VERSION;
+        case VIRTIO_MMIO_VENDORID:
+            return VIRT_VENDOR;
+        default:
+            return 0;
+        }
+    }
+
+    if (offset >= VIRTIO_MMIO_CONFIG) {
+        offset -= VIRTIO_MMIO_CONFIG;
+        switch (size) {
+        case 1:
+            return virtio_config_readb(vdev, offset);
+        case 2:
+            return virtio_config_readw(vdev, offset);
+        case 4:
+            return virtio_config_readl(vdev, offset);
+        default:
+            abort();
+        }
+    }
+    if (size != 4) {
+        DPRINTF("wrong size access to register!\n");
+        return 0;
+    }
+    switch (offset) {
+    case VIRTIO_MMIO_MAGIC:
+        return VIRT_MAGIC;
+    case VIRTIO_MMIO_VERSION:
+        return VIRT_VERSION;
+    case VIRTIO_MMIO_DEVICEID:
+        return vdev->device_id;
+    case VIRTIO_MMIO_VENDORID:
+        return VIRT_VENDOR;
+    case VIRTIO_MMIO_HOSTFEATURES:
+        if (proxy->host_features_sel) {
+            return 0;
+        }
+        return proxy->host_features;
+    case VIRTIO_MMIO_QUEUENUMMAX:
+        return VIRTQUEUE_MAX_SIZE;
+    case VIRTIO_MMIO_QUEUEPFN:
+        return virtio_queue_get_addr(vdev, vdev->queue_sel)
+            >> proxy->guest_page_shift;
+    case VIRTIO_MMIO_INTERRUPTSTATUS:
+        return vdev->isr;
+    case VIRTIO_MMIO_STATUS:
+        return vdev->status;
+    case VIRTIO_MMIO_HOSTFEATURESSEL:
+    case VIRTIO_MMIO_GUESTFEATURES:
+    case VIRTIO_MMIO_GUESTFEATURESSEL:
+    case VIRTIO_MMIO_GUESTPAGESIZE:
+    case VIRTIO_MMIO_QUEUESEL:
+    case VIRTIO_MMIO_QUEUENUM:
+    case VIRTIO_MMIO_QUEUEALIGN:
+    case VIRTIO_MMIO_QUEUENOTIFY:
+    case VIRTIO_MMIO_INTERRUPTACK:
+        DPRINTF("read of write-only register\n");
+        return 0;
+    default:
+        DPRINTF("bad register offset\n");
+        return 0;
+    }
+    return 0;
+}
+
+static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
+                              unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->bus.vdev;
+
+    DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
+            (int)offset, value);
+
+    if (!vdev) {
+        /* If no backend is present, we just make all registers
+         * write-ignored. This allows us to provide transports with
+         * no backend plugged in.
+         */
+        return;
+    }
+
+    if (offset >= VIRTIO_MMIO_CONFIG) {
+        offset -= VIRTIO_MMIO_CONFIG;
+        switch (size) {
+        case 1:
+            virtio_config_writeb(vdev, offset, value);
+            break;
+        case 2:
+            virtio_config_writew(vdev, offset, value);
+            break;
+        case 4:
+            virtio_config_writel(vdev, offset, value);
+            break;
+        default:
+            abort();
+        }
+        return;
+    }
+    if (size != 4) {
+        DPRINTF("wrong size access to register!\n");
+        return;
+    }
+    switch (offset) {
+    case VIRTIO_MMIO_HOSTFEATURESSEL:
+        proxy->host_features_sel = value;
+        break;
+    case VIRTIO_MMIO_GUESTFEATURES:
+        if (!proxy->guest_features_sel) {
+            virtio_set_features(vdev, value);
+        }
+        break;
+    case VIRTIO_MMIO_GUESTFEATURESSEL:
+        proxy->guest_features_sel = value;
+        break;
+    case VIRTIO_MMIO_GUESTPAGESIZE:
+        proxy->guest_page_shift = ctz32(value);
+        if (proxy->guest_page_shift > 31) {
+            proxy->guest_page_shift = 0;
+        }
+        DPRINTF("guest page size %" PRIx64 " shift %d\n", value,
+                proxy->guest_page_shift);
+        break;
+    case VIRTIO_MMIO_QUEUESEL:
+        if (value < VIRTIO_PCI_QUEUE_MAX) {
+            vdev->queue_sel = value;
+        }
+        break;
+    case VIRTIO_MMIO_QUEUENUM:
+        DPRINTF("mmio_queue write %d max %d\n", (int)value, VIRTQUEUE_MAX_SIZE);
+        if (value <= VIRTQUEUE_MAX_SIZE) {
+            DPRINTF("calling virtio_queue_set_num\n");
+            virtio_queue_set_num(vdev, vdev->queue_sel, value);
+        }
+        break;
+    case VIRTIO_MMIO_QUEUEALIGN:
+        virtio_queue_set_align(vdev, vdev->queue_sel, value);
+        break;
+    case VIRTIO_MMIO_QUEUEPFN:
+        if (value == 0) {
+            virtio_reset(vdev);
+        } else {
+            virtio_queue_set_addr(vdev, vdev->queue_sel,
+                                  value << proxy->guest_page_shift);
+        }
+        break;
+    case VIRTIO_MMIO_QUEUENOTIFY:
+        if (value < VIRTIO_PCI_QUEUE_MAX) {
+            virtio_queue_notify(vdev, value);
+        }
+        break;
+    case VIRTIO_MMIO_INTERRUPTACK:
+        vdev->isr &= ~value;
+        virtio_update_irq(vdev);
+        break;
+    case VIRTIO_MMIO_STATUS:
+        virtio_set_status(vdev, value & 0xff);
+        if (vdev->status == 0) {
+            virtio_reset(vdev);
+        }
+        break;
+    case VIRTIO_MMIO_MAGIC:
+    case VIRTIO_MMIO_VERSION:
+    case VIRTIO_MMIO_DEVICEID:
+    case VIRTIO_MMIO_VENDORID:
+    case VIRTIO_MMIO_HOSTFEATURES:
+    case VIRTIO_MMIO_QUEUENUMMAX:
+    case VIRTIO_MMIO_INTERRUPTSTATUS:
+        DPRINTF("write to readonly register\n");
+        break;
+
+    default:
+        DPRINTF("bad register offset\n");
+    }
+}
+
+static const MemoryRegionOps virtio_mem_ops = {
+    .read = virtio_mmio_read,
+    .write = virtio_mmio_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+    int level;
+
+    if (!proxy->bus.vdev) {
+        return;
+    }
+    level = (proxy->bus.vdev->isr != 0);
+    DPRINTF("virtio_mmio setting IRQ %d\n", level);
+    qemu_set_irq(proxy->irq, level);
+}
+
+static unsigned int virtio_mmio_get_features(DeviceState *opaque)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    return proxy->host_features;
+}
+
+static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    proxy->host_features_sel = qemu_get_be32(f);
+    proxy->guest_features_sel = qemu_get_be32(f);
+    proxy->guest_page_shift = qemu_get_be32(f);
+    return 0;
+}
+
+static void virtio_mmio_save_config(DeviceState *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    qemu_put_be32(f, proxy->host_features_sel);
+    qemu_put_be32(f, proxy->guest_features_sel);
+    qemu_put_be32(f, proxy->guest_page_shift);
+}
+
+static void virtio_mmio_reset(DeviceState *d)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+
+    virtio_bus_reset(&proxy->bus);
+    proxy->host_features_sel = 0;
+    proxy->guest_features_sel = 0;
+    proxy->guest_page_shift = 0;
+}
+
+/* virtio-mmio device */
+
+/* This is called by virtio-bus just after the device is plugged. */
+static void virtio_mmio_device_plugged(DeviceState *opaque)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+    proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus,
+                                                        proxy->host_features);
+}
+
+static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(d);
+
+    virtio_mmio_bus_new(&proxy->bus, proxy);
+    sysbus_init_irq(sbd, &proxy->irq);
+    memory_region_init_io(&proxy->iomem, &virtio_mem_ops, proxy,
+                          TYPE_VIRTIO_MMIO, 0x200);
+    sysbus_init_mmio(sbd, &proxy->iomem);
+}
+
+static void virtio_mmio_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = virtio_mmio_realizefn;
+    dc->reset = virtio_mmio_reset;
+}
+
+static const TypeInfo virtio_mmio_info = {
+    .name          = TYPE_VIRTIO_MMIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VirtIOMMIOProxy),
+    .class_init    = virtio_mmio_class_init,
+};
+
+/* virtio-mmio-bus. */
+
+static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev)
+{
+    DeviceState *qdev = DEVICE(dev);
+    BusState *qbus;
+
+    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_MMIO_BUS, qdev, NULL);
+    qbus = BUS(bus);
+    qbus->allow_hotplug = 0;
+}
+
+static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *bus_class = BUS_CLASS(klass);
+    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
+
+    k->notify = virtio_mmio_update_irq;
+    k->save_config = virtio_mmio_save_config;
+    k->load_config = virtio_mmio_load_config;
+    k->get_features = virtio_mmio_get_features;
+    k->device_plugged = virtio_mmio_device_plugged;
+    k->has_variable_vring_alignment = true;
+    bus_class->max_dev = 1;
+}
+
+static const TypeInfo virtio_mmio_bus_info = {
+    .name          = TYPE_VIRTIO_MMIO_BUS,
+    .parent        = TYPE_VIRTIO_BUS,
+    .instance_size = sizeof(VirtioBusState),
+    .class_init    = virtio_mmio_bus_class_init,
+};
+
+static void virtio_mmio_register_types(void)
+{
+    type_register_static(&virtio_mmio_bus_info);
+    type_register_static(&virtio_mmio_info);
+}
+
+type_init(virtio_mmio_register_types)
-- 
1.7.9.5

[Qemu-devel] [PATCH 6/8] arm/boot: Allow boards to modify the FDT blob

[Peter Maydell]

Add a callback hook in arm_boot_info to allow board models to
modify the device tree blob if they need to. (The major expected
use case is to add virtio-mmio nodes for virtio-mmio transports
that exist in QEMU but not in the hardware.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c        |    5 +++++
 include/hw/arm/arm.h |    4 ++++
 2 files changed, 9 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index eb9a864..77d548c 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -293,6 +293,11 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
             goto fail;
         }
     }
+
+    if (binfo->modify_dtb) {
+        binfo->modify_dtb(binfo, fdt);
+    }
+
     qemu_devtree_dumpdtb(fdt, size);
 
     cpu_physical_memory_write(addr, fdt, size);
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 7b2b02d..bae87c6 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -55,6 +55,10 @@ struct arm_boot_info {
                                  const struct arm_boot_info *info);
     void (*secondary_cpu_reset_hook)(ARMCPU *cpu,
                                      const struct arm_boot_info *info);
+    /* if a board needs to be able to modify a device tree provided by
+     * the user it should implement this hook.
+     */
+    void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
     /* Used internally by arm_boot.c */
     int is_linux;
     hwaddr initrd_start;
-- 
1.7.9.5

[Qemu-devel] [PATCH 7/8] vexpress: Make VEDBoardInfo extend arm_boot_info

[Peter Maydell]

Make the VEDBoardInfo struct extend arm_boot_info; this will
allow us to get at the VEDBoardInfo information inside callbacks
from arm/boot code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/vexpress.c |   31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index a077c62..46d8377 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -36,8 +36,6 @@
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
 #define VEXPRESS_FLASH_SECT_SIZE (256 * 1024)
 
-static struct arm_boot_info vexpress_binfo;
-
 /* Address maps for peripherals:
  * the Versatile Express motherboard has two possible maps,
  * the "legacy" one (used for A9) and the "Cortex-A Series"
@@ -150,6 +148,7 @@ typedef void DBoardInitFn(const VEDBoardInfo *daughterboard,
                           qemu_irq *pic);
 
 struct VEDBoardInfo {
+    struct arm_boot_info bootinfo;
     const hwaddr *motherboard_map;
     hwaddr loader_start;
     const hwaddr gic_cpu_if_addr;
@@ -269,7 +268,7 @@ static const uint32_t a9_clocks[] = {
     66670000, /* Test chip reference clock: 66.67MHz */
 };
 
-static const VEDBoardInfo a9_daughterboard = {
+static VEDBoardInfo a9_daughterboard = {
     .motherboard_map = motherboard_legacy_map,
     .loader_start = 0x60000000,
     .gic_cpu_if_addr = 0x1e000100,
@@ -381,7 +380,7 @@ static const uint32_t a15_clocks[] = {
     40000000, /* OSCCLK8: 40MHz : DDR2 PLL reference */
 };
 
-static const VEDBoardInfo a15_daughterboard = {
+static VEDBoardInfo a15_daughterboard = {
     .motherboard_map = motherboard_aseries_map,
     .loader_start = 0x80000000,
     .gic_cpu_if_addr = 0x2c002000,
@@ -393,7 +392,7 @@ static const VEDBoardInfo a15_daughterboard = {
     .init = a15_daughterboard_init,
 };
 
-static void vexpress_common_init(const VEDBoardInfo *daughterboard,
+static void vexpress_common_init(VEDBoardInfo *daughterboard,
                                  QEMUMachineInitArgs *args)
 {
     DeviceState *dev, *sysctl, *pl041;
@@ -509,17 +508,17 @@ static void vexpress_common_init(const VEDBoardInfo *daughterboard,
 
     /* VE_DAPROM: not modelled */
 
-    vexpress_binfo.ram_size = args->ram_size;
-    vexpress_binfo.kernel_filename = args->kernel_filename;
-    vexpress_binfo.kernel_cmdline = args->kernel_cmdline;
-    vexpress_binfo.initrd_filename = args->initrd_filename;
-    vexpress_binfo.nb_cpus = smp_cpus;
-    vexpress_binfo.board_id = VEXPRESS_BOARD_ID;
-    vexpress_binfo.loader_start = daughterboard->loader_start;
-    vexpress_binfo.smp_loader_start = map[VE_SRAM];
-    vexpress_binfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
-    vexpress_binfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
-    arm_load_kernel(arm_env_get_cpu(first_cpu), &vexpress_binfo);
+    daughterboard->bootinfo.ram_size = args->ram_size;
+    daughterboard->bootinfo.kernel_filename = args->kernel_filename;
+    daughterboard->bootinfo.kernel_cmdline = args->kernel_cmdline;
+    daughterboard->bootinfo.initrd_filename = args->initrd_filename;
+    daughterboard->bootinfo.nb_cpus = smp_cpus;
+    daughterboard->bootinfo.board_id = VEXPRESS_BOARD_ID;
+    daughterboard->bootinfo.loader_start = daughterboard->loader_start;
+    daughterboard->bootinfo.smp_loader_start = map[VE_SRAM];
+    daughterboard->bootinfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
+    daughterboard->bootinfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
+    arm_load_kernel(arm_env_get_cpu(first_cpu), &daughterboard->bootinfo);
 }
 
 static void vexpress_a9_init(QEMUMachineInitArgs *args)
-- 
1.7.9.5

[Qemu-devel] [PATCH 8/8] vexpress: Add virtio-mmio transports

[Peter Maydell]

Add some virtio-mmio transports to the vexpress board model,
together with a modify_dtb hook which adds them to the device
tree so that the kernel will probe for them. We put them
in a reserved area of the address map.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/vexpress.c |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 46d8377..afcac5b 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -31,11 +31,18 @@
 #include "exec/address-spaces.h"
 #include "sysemu/blockdev.h"
 #include "hw/block/flash.h"
+#include "sysemu/device_tree.h"
+#include <libfdt.h>
 
 #define VEXPRESS_BOARD_ID 0x8e0
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
 #define VEXPRESS_FLASH_SECT_SIZE (256 * 1024)
 
+/* Number of virtio transports to create (0..8; limited by
+ * number of available IRQ lines).
+ */
+#define NUM_VIRTIO_TRANSPORTS 4
+
 /* Address maps for peripherals:
  * the Versatile Express motherboard has two possible maps,
  * the "legacy" one (used for A9) and the "Cortex-A Series"
@@ -70,6 +77,7 @@ enum {
     VE_ETHERNET,
     VE_USB,
     VE_DAPROM,
+    VE_VIRTIO,
 };
 
 static hwaddr motherboard_legacy_map[] = {
@@ -88,6 +96,7 @@ static hwaddr motherboard_legacy_map[] = {
     [VE_WDT] = 0x1000f000,
     [VE_TIMER01] = 0x10011000,
     [VE_TIMER23] = 0x10012000,
+    [VE_VIRTIO] = 0x10013000,
     [VE_SERIALDVI] = 0x10016000,
     [VE_RTC] = 0x10017000,
     [VE_COMPACTFLASH] = 0x1001a000,
@@ -132,6 +141,7 @@ static hwaddr motherboard_aseries_map[] = {
     [VE_WDT] = 0x1c0f0000,
     [VE_TIMER01] = 0x1c110000,
     [VE_TIMER23] = 0x1c120000,
+    [VE_VIRTIO] = 0x1c130000,
     [VE_SERIALDVI] = 0x1c160000,
     [VE_RTC] = 0x1c170000,
     [VE_COMPACTFLASH] = 0x1c1a0000,
@@ -392,6 +402,85 @@ static VEDBoardInfo a15_daughterboard = {
     .init = a15_daughterboard_init,
 };
 
+static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
+                                hwaddr addr, hwaddr size, uint32_t intc,
+                                int irq)
+{
+    /* Add a virtio_mmio node to the device tree blob:
+     *   virtio_mmio@ADDRESS {
+     *       compatible = "virtio,mmio";
+     *       reg = <ADDRESS, SIZE>;
+     *       interrupt-parent = <&intc>;
+     *       interrupts = <0, irq, 1>;
+     *   }
+     * (Note that the format of the interrupts property is dependent on the
+     * interrupt controller that interrupt-parent points to; these are for
+     * the ARM GIC and indicate an SPI interrupt, rising-edge-triggered.)
+     */
+    int rc;
+    char *nodename = g_strdup_printf("/virtio_mmio@%" PRIx64, addr);
+
+    rc = qemu_devtree_add_subnode(fdt, nodename);
+    rc |= qemu_devtree_setprop_string(fdt, nodename,
+                                      "compatible", "virtio,mmio");
+    rc |= qemu_devtree_setprop_sized_cells(fdt, nodename, "reg",
+                                           acells, addr, scells, size);
+    qemu_devtree_setprop_cells(fdt, nodename, "interrupt-parent", intc);
+    qemu_devtree_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
+    g_free(nodename);
+    if (rc) {
+        return -1;
+    }
+    return 0;
+}
+
+static uint32_t find_int_controller(void *fdt)
+{
+    /* Find the FDT node corresponding to the interrupt controller
+     * for virtio-mmio devices. We do this by scanning the fdt for
+     * a node with the right compatibility, since we know there is
+     * only one GIC on a vexpress board.
+     * We return the phandle of the node, or 0 if none was found.
+     */
+    const char *compat = "arm,cortex-a9-gic";
+    int offset;
+
+    offset = fdt_node_offset_by_compatible(fdt, -1, compat);
+    if (offset >= 0) {
+        return fdt_get_phandle(fdt, offset);
+    }
+    return 0;
+}
+
+static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
+{
+    uint32_t acells, scells, intc;
+    const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info;
+
+    acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
+    scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
+    intc = find_int_controller(fdt);
+    if (!intc) {
+        /* Not fatal, we just won't provide virtio. This will
+         * happen with older device tree blobs.
+         */
+        fprintf(stderr, "QEMU: warning: couldn't find interrupt controller in "
+                "dtb; will not include virtio-mmio devices in the dtb.\n");
+    } else {
+        int i;
+        const hwaddr *map = daughterboard->motherboard_map;
+
+        /* We iterate backwards here because adding nodes
+         * to the dtb puts them in last-first.
+         */
+        for (i = NUM_VIRTIO_TRANSPORTS - 1; i >= 0; i--) {
+            add_virtio_mmio_node(fdt, acells, scells,
+                                 map[VE_VIRTIO] + 0x200 * i,
+                                 0x200, intc, 40 + i);
+        }
+    }
+}
+
 static void vexpress_common_init(VEDBoardInfo *daughterboard,
                                  QEMUMachineInitArgs *args)
 {
@@ -508,6 +597,15 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
 
     /* VE_DAPROM: not modelled */
 
+    /* Create mmio transports, so the user can create virtio backends
+     * (which will be automatically plugged in to the transports). If
+     * no backend is created the transport will just sit harmlessly idle.
+     */
+    for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
+        sysbus_create_simple("virtio-mmio", map[VE_VIRTIO] + 0x200 * i,
+                             pic[40 + i]);
+    }
+
     daughterboard->bootinfo.ram_size = args->ram_size;
     daughterboard->bootinfo.kernel_filename = args->kernel_filename;
     daughterboard->bootinfo.kernel_cmdline = args->kernel_cmdline;
@@ -518,6 +616,7 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
     daughterboard->bootinfo.smp_loader_start = map[VE_SRAM];
     daughterboard->bootinfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
     daughterboard->bootinfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
+    daughterboard->bootinfo.modify_dtb = vexpress_modify_dtb;
     arm_load_kernel(arm_env_get_cpu(first_cpu), &daughterboard->bootinfo);
 }
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 27.06.2013, at 15:04, Peter Maydell wrote:

> This patch series adds an implementation of the virtio-mmio
> transport, and uses it in the vexpress-a9 and vexpress-a15
> board models.
> 
> The basic idea is that the board instantiates some transports,

I really dislike that idea. Couldn't you also create a new bus for your vexpress platform and add a virtio-mmio-vexpress device that automatically allocates an interrupt from the main PIC on instantiation? That way you could create transports using -device.

Combine that with automatic fdt generation and you get fully command line scaling device creation.


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 08.07.2013, at 14:57, Alexander Graf wrote:

> 
> On 27.06.2013, at 15:04, Peter Maydell wrote:
> 
>> This patch series adds an implementation of the virtio-mmio
>> transport, and uses it in the vexpress-a9 and vexpress-a15
>> board models.
>> 
>> The basic idea is that the board instantiates some transports,
> 
> I really dislike that idea. Couldn't you also create a new bus for your vexpress platform and add a virtio-mmio-vexpress device that automatically allocates an interrupt from the main PIC on instantiation? That way you could create transports using -device.
> 
> Combine that with automatic fdt generation and you get fully command line scaling device creation.

Ah, you already have the fdt generation bit. So you're really only missing the automatic irq allocation to make it actually user friendly and flexible.


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

On 8 July 2013 13:59, Alexander Graf <agraf@suse.de> wrote:
> On 08.07.2013, at 14:57, Alexander Graf wrote:
>> On 27.06.2013, at 15:04, Peter Maydell wrote:
>>> The basic idea is that the board instantiates some transports,
>>
>> I really dislike that idea. Couldn't you also create a new
>> bus for your vexpress platform and add a virtio-mmio-vexpress
>> device that automatically allocates an interrupt from the main
>> PIC on instantiation? That way you could create transports
>> using -device.

This doesn't seem to gain anything except that the user has
to use -device twice rather than once.

>> Combine that with automatic fdt generation and you get fully
>> command line scaling device creation.
>
> Ah, you already have the fdt generation bit. So you're really
> only missing the automatic irq allocation to make it actually
> user friendly and flexible.

It's not clear to me how you can automatically allocate an IRQ:
there just aren't that many available, and I'm already a bit
nervous about stealing 4, because we might run into issues if
newer versions of h/w use those IRQs for something else. One
approach would be to have all virtio transports share an IRQ
line (the spec currently forbids this; I need to test if it
would actually work in practice...)

As far as I know there's no way to find out at board
construction how many virtio devices you might want.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 08.07.2013, at 15:08, Peter Maydell wrote:

> On 8 July 2013 13:59, Alexander Graf <agraf@suse.de> wrote:
>> On 08.07.2013, at 14:57, Alexander Graf wrote:
>>> On 27.06.2013, at 15:04, Peter Maydell wrote:
>>>> The basic idea is that the board instantiates some transports,
>>> 
>>> I really dislike that idea. Couldn't you also create a new
>>> bus for your vexpress platform and add a virtio-mmio-vexpress
>>> device that automatically allocates an interrupt from the main
>>> PIC on instantiation? That way you could create transports
>>> using -device.
> 
> This doesn't seem to gain anything except that the user has
> to use -device twice rather than once.

Yes, it does. You can have other devices than just virtio ones on those IRQ lines. Assigned devices for example.

> 
>>> Combine that with automatic fdt generation and you get fully
>>> command line scaling device creation.
>> 
>> Ah, you already have the fdt generation bit. So you're really
>> only missing the automatic irq allocation to make it actually
>> user friendly and flexible.
> 
> It's not clear to me how you can automatically allocate an IRQ:
> there just aren't that many available, and I'm already a bit

For a first draft, how about your allocator returns those 4 consecutively? For the PV machine things will become more obvious. Maybe we could even share the same bus type between your PV machine, vexpress and the e500 machines?

> nervous about stealing 4, because we might run into issues if
> newer versions of h/w use those IRQs for something else. One
> approach would be to have all virtio transports share an IRQ
> line (the spec currently forbids this; I need to test if it
> would actually work in practice...)
> 
> As far as I know there's no way to find out at board
> construction how many virtio devices you might want.

Hence you shouldn't create them :).


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

On 8 July 2013 14:16, Alexander Graf <agraf@suse.de> wrote:
>
> On 08.07.2013, at 15:08, Peter Maydell wrote:
>
>> On 8 July 2013 13:59, Alexander Graf <agraf@suse.de> wrote:
>>> On 08.07.2013, at 14:57, Alexander Graf wrote:
>>>> On 27.06.2013, at 15:04, Peter Maydell wrote:
>>>>> The basic idea is that the board instantiates some transports,
>>>>
>>>> I really dislike that idea. Couldn't you also create a new
>>>> bus for your vexpress platform and add a virtio-mmio-vexpress
>>>> device that automatically allocates an interrupt from the main
>>>> PIC on instantiation? That way you could create transports
>>>> using -device.
>>
>> This doesn't seem to gain anything except that the user has
>> to use -device twice rather than once.
>
> Yes, it does. You can have other devices than just virtio ones
> on those IRQ lines. Assigned devices for example.

Now I'm completely confused. Why would assigned devices
have anything to do with this? Can you explain in more
detail, because I don't really see what you're suggesting?

>> As far as I know there's no way to find out at board
>> construction how many virtio devices you might want.
>
> Hence you shouldn't create them :).

You've got to create them sometime...

-- PMM

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 08.07.2013, at 15:23, Peter Maydell wrote:

> On 8 July 2013 14:16, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 08.07.2013, at 15:08, Peter Maydell wrote:
>> 
>>> On 8 July 2013 13:59, Alexander Graf <agraf@suse.de> wrote:
>>>> On 08.07.2013, at 14:57, Alexander Graf wrote:
>>>>> On 27.06.2013, at 15:04, Peter Maydell wrote:
>>>>>> The basic idea is that the board instantiates some transports,
>>>>> 
>>>>> I really dislike that idea. Couldn't you also create a new
>>>>> bus for your vexpress platform and add a virtio-mmio-vexpress
>>>>> device that automatically allocates an interrupt from the main
>>>>> PIC on instantiation? That way you could create transports
>>>>> using -device.
>>> 
>>> This doesn't seem to gain anything except that the user has
>>> to use -device twice rather than once.
>> 
>> Yes, it does. You can have other devices than just virtio ones
>> on those IRQ lines. Assigned devices for example.
> 
> Now I'm completely confused. Why would assigned devices
> have anything to do with this? Can you explain in more
> detail, because I don't really see what you're suggesting?

The only missing link we have to create any device using -device on the command line is the IRQ line enumeration. If we can allocate IRQ lines automatically, we can put any command line given -device onto our main system bus that is non-pci, non-isa.

So if we want to ever support VFIO for platform devices, the user will want to pass -device vfio-ahci,foo=bar on the command line to assign an AHCI device. The only infrastructure blocker we have for that today is the IRQ allocation. Maybe we could even try to be as smart as putting the MMIO regions into guest address space intelligently automatically.

So if we need all that logic anyway, I don't think it's a good idea to go into the wrong direction with virtio-mmio which has the exact same requirements.

> 
>>> As far as I know there's no way to find out at board
>>> construction how many virtio devices you might want.
>> 
>> Hence you shouldn't create them :).
> 
> You've got to create them sometime...

Yes, when the user says he wants them.


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

On 8 July 2013 14:45, Alexander Graf <agraf@suse.de> wrote:
> On 08.07.2013, at 15:23, Peter Maydell wrote:
>> Now I'm completely confused. Why would assigned devices
>> have anything to do with this? Can you explain in more
>> detail, because I don't really see what you're suggesting?
>
> The only missing link we have to create any device using -device
> on the command line is the IRQ line enumeration. If we can allocate
> IRQ lines automatically, we can put any command line given -device
> onto our main system bus that is non-pci, non-isa.

If the user is expected to be able to get the MMIO address
right (which they'd have to specify on the command line
somehow too) why not require them to specify the IRQ number
while they're doing it? I'm a bit suspicious of anything
that requires the user to specify to that level of detail
though, since it requires a lot of inside knowledge about the
board.

This is the whole reason for having the separate transport:
the board gets to take care of the board specific detail of
how to wire up the transport, and the user just asks to
create the backends that plug automatically into it.
The virtio command line options are complicated and confusing
enough as it is.

> So if we want to ever support VFIO for platform devices,
> the user will want to pass -device vfio-ahci,foo=bar on
> the command line to assign an AHCI device.

This appears to be seriously short on actually specifying
enough information to wire a device up.

> The only infrastructure blocker we have for that today
> is the IRQ allocation.

DMA lines? Specifying the right location in the address space?

> Maybe we could even try to be as smart as putting the MMIO
> regions into guest address space intelligently automatically.

This sounds likely to cause problems with migration unless
we can guarantee that we always pick the same place.

-- PMM

Re: [Qemu-devel] [PATCH 3/8] virtio: Add support for guest setting of queue size

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> The MMIO virtio transport spec allows the guest to tell the host how
> large the queue size is. Add virtio_queue_set_num() function which
> implements this in the QEMU common virtio support code.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/virtio/virtio.c         |    6 ++++++
>  include/hw/virtio/virtio.h |    1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 8176c14..8805b8a 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -667,6 +667,12 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
>      return vdev->vq[n].pa;
>  }
>  
> +void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> +{
> +    vdev->vq[n].vring.num = num;
> +    virtqueue_init(&vdev->vq[n]);

I think some level of checking is probably needed on num since we do a
tremendous amount of math on it.  I doubt it's exploitable since it's
always treated as a PA, but better to be safe than sorry.

Regards,

Anthony Liguori

> +}
> +
>  int virtio_queue_get_num(VirtIODevice *vdev, int n)
>  {
>      return vdev->vq[n].vring.num;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index a6c5c53..95c4772 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -198,6 +198,7 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data);
>  void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
>  void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
>  hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
> +void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
>  int virtio_queue_get_num(VirtIODevice *vdev, int n);
>  void virtio_queue_notify(VirtIODevice *vdev, int n);
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH 5/8] virtio: Implement MMIO based virtio transport

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> Add support for the generic MMIO based virtio transport.
>
> This patch includes some fixes for bugs spotted by
> Ying-Shiuan Pan <yspan@itri.org.tw>.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> [Fred changes: updated to new virtio-bus mechanisms]
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> [PMM changes:
>  * fixed trivial makefile conflict
>  * removed unused int_enable
>  * host_features doesn't need migrating
>  * reset guest accessible state in the reset function
>  * minor style fixes like extra blank lines
>  * RAZ/WI if there's no backend
>  * made transport size 0x200, in line with kvmtool
>  * set has_variable_vring_alignment
> ]

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>  hw/virtio/Makefile.objs |    1 +
>  hw/virtio/virtio-mmio.c |  424 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 425 insertions(+)
>  create mode 100644 hw/virtio/virtio-mmio.c
>
> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> index cbe6d51..1ba53d9 100644
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -1,6 +1,7 @@
>  common-obj-y += virtio-rng.o
>  common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>  common-obj-y += virtio-bus.o
> +common-obj-y += virtio-mmio.o
>  common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
>  
>  obj-y += virtio.o virtio-balloon.o 
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> new file mode 100644
> index 0000000..4c16616
> --- /dev/null
> +++ b/hw/virtio/virtio-mmio.c
> @@ -0,0 +1,424 @@
> +/*
> + * Virtio MMIO bindings
> + *
> + * Copyright (c) 2011 Linaro Limited
> + *
> + * Author:
> + *  Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License; 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/virtio/virtio.h"
> +#include "qemu/host-utils.h"
> +#include "hw/virtio/virtio-bus.h"
> +
> +/* #define DEBUG_VIRTIO_MMIO */
> +
> +#ifdef DEBUG_VIRTIO_MMIO
> +
> +#define DPRINTF(fmt, ...) \
> +do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
> +
> +/* QOM macros */
> +/* virtio-mmio-bus */
> +#define TYPE_VIRTIO_MMIO_BUS "virtio-mmio-bus"
> +#define VIRTIO_MMIO_BUS(obj) \
> +        OBJECT_CHECK(VirtioBusState, (obj), TYPE_VIRTIO_MMIO_BUS)
> +#define VIRTIO_MMIO_BUS_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(VirtioBusClass, (obj), TYPE_VIRTIO_MMIO_BUS)
> +#define VIRTIO_MMIO_BUS_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(VirtioBusClass, (klass), TYPE_VIRTIO_MMIO_BUS)
> +
> +/* virtio-mmio */
> +#define TYPE_VIRTIO_MMIO "virtio-mmio"
> +#define VIRTIO_MMIO(obj) \
> +        OBJECT_CHECK(VirtIOMMIOProxy, (obj), TYPE_VIRTIO_MMIO)
> +
> +/* Memory mapped register offsets */
> +#define VIRTIO_MMIO_MAGIC 0x0
> +#define VIRTIO_MMIO_VERSION 0x4
> +#define VIRTIO_MMIO_DEVICEID 0x8
> +#define VIRTIO_MMIO_VENDORID 0xc
> +#define VIRTIO_MMIO_HOSTFEATURES 0x10
> +#define VIRTIO_MMIO_HOSTFEATURESSEL 0x14
> +#define VIRTIO_MMIO_GUESTFEATURES 0x20
> +#define VIRTIO_MMIO_GUESTFEATURESSEL 0x24
> +#define VIRTIO_MMIO_GUESTPAGESIZE 0x28
> +#define VIRTIO_MMIO_QUEUESEL 0x30
> +#define VIRTIO_MMIO_QUEUENUMMAX 0x34
> +#define VIRTIO_MMIO_QUEUENUM 0x38
> +#define VIRTIO_MMIO_QUEUEALIGN 0x3c
> +#define VIRTIO_MMIO_QUEUEPFN 0x40
> +#define VIRTIO_MMIO_QUEUENOTIFY 0x50
> +#define VIRTIO_MMIO_INTERRUPTSTATUS 0x60
> +#define VIRTIO_MMIO_INTERRUPTACK 0x64
> +#define VIRTIO_MMIO_STATUS 0x70
> +/* Device specific config space starts here */
> +#define VIRTIO_MMIO_CONFIG 0x100
> +
> +#define VIRT_MAGIC 0x74726976 /* 'virt' */
> +#define VIRT_VERSION 1
> +#define VIRT_VENDOR 0x554D4551 /* 'QEMU' */
> +
> +typedef struct {
> +    /* Generic */
> +    SysBusDevice parent_obj;
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +    uint32_t host_features;
> +    /* Guest accessible state needing migration and reset */
> +    uint32_t host_features_sel;
> +    uint32_t guest_features_sel;
> +    uint32_t guest_page_shift;
> +    /* virtio-bus */
> +    VirtioBusState bus;
> +} VirtIOMMIOProxy;
> +
> +static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev);
> +
> +static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
> +    VirtIODevice *vdev = proxy->bus.vdev;
> +
> +    DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
> +
> +    if (!vdev) {
> +        /* If no backend is present, we treat most registers as
> +         * read-as-zero, except for the magic number, version and
> +         * vendor ID. This is not strictly sanctioned by the virtio
> +         * spec, but it allows us to provide transports with no backend
> +         * plugged in which don't confuse Linux's virtio code: the
> +         * probe won't complain about the bad magic number, but the
> +         * device ID of zero means no backend will claim it.
> +         */
> +        switch (offset) {
> +        case VIRTIO_MMIO_MAGIC:
> +            return VIRT_MAGIC;
> +        case VIRTIO_MMIO_VERSION:
> +            return VIRT_VERSION;
> +        case VIRTIO_MMIO_VENDORID:
> +            return VIRT_VENDOR;
> +        default:
> +            return 0;
> +        }
> +    }
> +
> +    if (offset >= VIRTIO_MMIO_CONFIG) {
> +        offset -= VIRTIO_MMIO_CONFIG;
> +        switch (size) {
> +        case 1:
> +            return virtio_config_readb(vdev, offset);
> +        case 2:
> +            return virtio_config_readw(vdev, offset);
> +        case 4:
> +            return virtio_config_readl(vdev, offset);
> +        default:
> +            abort();
> +        }
> +    }
> +    if (size != 4) {
> +        DPRINTF("wrong size access to register!\n");
> +        return 0;
> +    }
> +    switch (offset) {
> +    case VIRTIO_MMIO_MAGIC:
> +        return VIRT_MAGIC;
> +    case VIRTIO_MMIO_VERSION:
> +        return VIRT_VERSION;
> +    case VIRTIO_MMIO_DEVICEID:
> +        return vdev->device_id;
> +    case VIRTIO_MMIO_VENDORID:
> +        return VIRT_VENDOR;
> +    case VIRTIO_MMIO_HOSTFEATURES:
> +        if (proxy->host_features_sel) {
> +            return 0;
> +        }
> +        return proxy->host_features;
> +    case VIRTIO_MMIO_QUEUENUMMAX:
> +        return VIRTQUEUE_MAX_SIZE;
> +    case VIRTIO_MMIO_QUEUEPFN:
> +        return virtio_queue_get_addr(vdev, vdev->queue_sel)
> +            >> proxy->guest_page_shift;
> +    case VIRTIO_MMIO_INTERRUPTSTATUS:
> +        return vdev->isr;
> +    case VIRTIO_MMIO_STATUS:
> +        return vdev->status;
> +    case VIRTIO_MMIO_HOSTFEATURESSEL:
> +    case VIRTIO_MMIO_GUESTFEATURES:
> +    case VIRTIO_MMIO_GUESTFEATURESSEL:
> +    case VIRTIO_MMIO_GUESTPAGESIZE:
> +    case VIRTIO_MMIO_QUEUESEL:
> +    case VIRTIO_MMIO_QUEUENUM:
> +    case VIRTIO_MMIO_QUEUEALIGN:
> +    case VIRTIO_MMIO_QUEUENOTIFY:
> +    case VIRTIO_MMIO_INTERRUPTACK:
> +        DPRINTF("read of write-only register\n");
> +        return 0;
> +    default:
> +        DPRINTF("bad register offset\n");
> +        return 0;
> +    }
> +    return 0;
> +}
> +
> +static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
> +                              unsigned size)
> +{
> +    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
> +    VirtIODevice *vdev = proxy->bus.vdev;
> +
> +    DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
> +            (int)offset, value);
> +
> +    if (!vdev) {
> +        /* If no backend is present, we just make all registers
> +         * write-ignored. This allows us to provide transports with
> +         * no backend plugged in.
> +         */
> +        return;
> +    }
> +
> +    if (offset >= VIRTIO_MMIO_CONFIG) {
> +        offset -= VIRTIO_MMIO_CONFIG;
> +        switch (size) {
> +        case 1:
> +            virtio_config_writeb(vdev, offset, value);
> +            break;
> +        case 2:
> +            virtio_config_writew(vdev, offset, value);
> +            break;
> +        case 4:
> +            virtio_config_writel(vdev, offset, value);
> +            break;
> +        default:
> +            abort();
> +        }
> +        return;
> +    }
> +    if (size != 4) {
> +        DPRINTF("wrong size access to register!\n");
> +        return;
> +    }
> +    switch (offset) {
> +    case VIRTIO_MMIO_HOSTFEATURESSEL:
> +        proxy->host_features_sel = value;
> +        break;
> +    case VIRTIO_MMIO_GUESTFEATURES:
> +        if (!proxy->guest_features_sel) {
> +            virtio_set_features(vdev, value);
> +        }
> +        break;
> +    case VIRTIO_MMIO_GUESTFEATURESSEL:
> +        proxy->guest_features_sel = value;
> +        break;
> +    case VIRTIO_MMIO_GUESTPAGESIZE:
> +        proxy->guest_page_shift = ctz32(value);
> +        if (proxy->guest_page_shift > 31) {
> +            proxy->guest_page_shift = 0;
> +        }
> +        DPRINTF("guest page size %" PRIx64 " shift %d\n", value,
> +                proxy->guest_page_shift);
> +        break;
> +    case VIRTIO_MMIO_QUEUESEL:
> +        if (value < VIRTIO_PCI_QUEUE_MAX) {
> +            vdev->queue_sel = value;
> +        }
> +        break;
> +    case VIRTIO_MMIO_QUEUENUM:
> +        DPRINTF("mmio_queue write %d max %d\n", (int)value, VIRTQUEUE_MAX_SIZE);
> +        if (value <= VIRTQUEUE_MAX_SIZE) {
> +            DPRINTF("calling virtio_queue_set_num\n");
> +            virtio_queue_set_num(vdev, vdev->queue_sel, value);
> +        }
> +        break;
> +    case VIRTIO_MMIO_QUEUEALIGN:
> +        virtio_queue_set_align(vdev, vdev->queue_sel, value);
> +        break;
> +    case VIRTIO_MMIO_QUEUEPFN:
> +        if (value == 0) {
> +            virtio_reset(vdev);
> +        } else {
> +            virtio_queue_set_addr(vdev, vdev->queue_sel,
> +                                  value << proxy->guest_page_shift);
> +        }
> +        break;
> +    case VIRTIO_MMIO_QUEUENOTIFY:
> +        if (value < VIRTIO_PCI_QUEUE_MAX) {
> +            virtio_queue_notify(vdev, value);
> +        }
> +        break;
> +    case VIRTIO_MMIO_INTERRUPTACK:
> +        vdev->isr &= ~value;
> +        virtio_update_irq(vdev);
> +        break;
> +    case VIRTIO_MMIO_STATUS:
> +        virtio_set_status(vdev, value & 0xff);
> +        if (vdev->status == 0) {
> +            virtio_reset(vdev);
> +        }
> +        break;
> +    case VIRTIO_MMIO_MAGIC:
> +    case VIRTIO_MMIO_VERSION:
> +    case VIRTIO_MMIO_DEVICEID:
> +    case VIRTIO_MMIO_VENDORID:
> +    case VIRTIO_MMIO_HOSTFEATURES:
> +    case VIRTIO_MMIO_QUEUENUMMAX:
> +    case VIRTIO_MMIO_INTERRUPTSTATUS:
> +        DPRINTF("write to readonly register\n");
> +        break;
> +
> +    default:
> +        DPRINTF("bad register offset\n");
> +    }
> +}
> +
> +static const MemoryRegionOps virtio_mem_ops = {
> +    .read = virtio_mmio_read,
> +    .write = virtio_mmio_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> +    int level;
> +
> +    if (!proxy->bus.vdev) {
> +        return;
> +    }
> +    level = (proxy->bus.vdev->isr != 0);
> +    DPRINTF("virtio_mmio setting IRQ %d\n", level);
> +    qemu_set_irq(proxy->irq, level);
> +}
> +
> +static unsigned int virtio_mmio_get_features(DeviceState *opaque)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> +
> +    return proxy->host_features;
> +}
> +
> +static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> +
> +    proxy->host_features_sel = qemu_get_be32(f);
> +    proxy->guest_features_sel = qemu_get_be32(f);
> +    proxy->guest_page_shift = qemu_get_be32(f);
> +    return 0;
> +}
> +
> +static void virtio_mmio_save_config(DeviceState *opaque, QEMUFile *f)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> +
> +    qemu_put_be32(f, proxy->host_features_sel);
> +    qemu_put_be32(f, proxy->guest_features_sel);
> +    qemu_put_be32(f, proxy->guest_page_shift);
> +}
> +
> +static void virtio_mmio_reset(DeviceState *d)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> +
> +    virtio_bus_reset(&proxy->bus);
> +    proxy->host_features_sel = 0;
> +    proxy->guest_features_sel = 0;
> +    proxy->guest_page_shift = 0;
> +}
> +
> +/* virtio-mmio device */
> +
> +/* This is called by virtio-bus just after the device is plugged. */
> +static void virtio_mmio_device_plugged(DeviceState *opaque)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
> +
> +    proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
> +    proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus,
> +                                                        proxy->host_features);
> +}
> +
> +static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
> +{
> +    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(d);
> +
> +    virtio_mmio_bus_new(&proxy->bus, proxy);
> +    sysbus_init_irq(sbd, &proxy->irq);
> +    memory_region_init_io(&proxy->iomem, &virtio_mem_ops, proxy,
> +                          TYPE_VIRTIO_MMIO, 0x200);
> +    sysbus_init_mmio(sbd, &proxy->iomem);
> +}
> +
> +static void virtio_mmio_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = virtio_mmio_realizefn;
> +    dc->reset = virtio_mmio_reset;
> +}
> +
> +static const TypeInfo virtio_mmio_info = {
> +    .name          = TYPE_VIRTIO_MMIO,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VirtIOMMIOProxy),
> +    .class_init    = virtio_mmio_class_init,
> +};
> +
> +/* virtio-mmio-bus. */
> +
> +static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev)
> +{
> +    DeviceState *qdev = DEVICE(dev);
> +    BusState *qbus;
> +
> +    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_MMIO_BUS, qdev, NULL);
> +    qbus = BUS(bus);
> +    qbus->allow_hotplug = 0;
> +}
> +
> +static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
> +{
> +    BusClass *bus_class = BUS_CLASS(klass);
> +    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
> +
> +    k->notify = virtio_mmio_update_irq;
> +    k->save_config = virtio_mmio_save_config;
> +    k->load_config = virtio_mmio_load_config;
> +    k->get_features = virtio_mmio_get_features;
> +    k->device_plugged = virtio_mmio_device_plugged;
> +    k->has_variable_vring_alignment = true;
> +    bus_class->max_dev = 1;
> +}
> +
> +static const TypeInfo virtio_mmio_bus_info = {
> +    .name          = TYPE_VIRTIO_MMIO_BUS,
> +    .parent        = TYPE_VIRTIO_BUS,
> +    .instance_size = sizeof(VirtioBusState),
> +    .class_init    = virtio_mmio_bus_class_init,
> +};
> +
> +static void virtio_mmio_register_types(void)
> +{
> +    type_register_static(&virtio_mmio_bus_info);
> +    type_register_static(&virtio_mmio_info);
> +}
> +
> +type_init(virtio_mmio_register_types)
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 8 July 2013 14:45, Alexander Graf <agraf@suse.de> wrote:
>> On 08.07.2013, at 15:23, Peter Maydell wrote:
>>> Now I'm completely confused. Why would assigned devices
>>> have anything to do with this? Can you explain in more
>>> detail, because I don't really see what you're suggesting?
>>
>> The only missing link we have to create any device using -device
>> on the command line is the IRQ line enumeration. If we can allocate
>> IRQ lines automatically, we can put any command line given -device
>> onto our main system bus that is non-pci, non-isa.
>
> If the user is expected to be able to get the MMIO address
> right (which they'd have to specify on the command line
> somehow too) why not require them to specify the IRQ number
> while they're doing it? I'm a bit suspicious of anything
> that requires the user to specify to that level of detail
> though, since it requires a lot of inside knowledge about the
> board.
>
> This is the whole reason for having the separate transport:
> the board gets to take care of the board specific detail of
> how to wire up the transport, and the user just asks to
> create the backends that plug automatically into it.
> The virtio command line options are complicated and confusing
> enough as it is.
>
>> So if we want to ever support VFIO for platform devices,
>> the user will want to pass -device vfio-ahci,foo=bar on
>> the command line to assign an AHCI device.
>
> This appears to be seriously short on actually specifying
> enough information to wire a device up.
>
>> The only infrastructure blocker we have for that today
>> is the IRQ allocation.
>
> DMA lines? Specifying the right location in the address space?
>
>> Maybe we could even try to be as smart as putting the MMIO
>> regions into guest address space intelligently automatically.
>
> This sounds likely to cause problems with migration unless
> we can guarantee that we always pick the same place.

I think we're trying to fit a square peg into a round hole.

virtio-mmio is a virtio transport where each device has a dedicated set
of system resources.

Alex, it sounds like you want virtio-mmio-bus which would be a single
set of system resources that implemented a virtio bus on top of it.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 08.07.2013, at 22:08, Anthony Liguori wrote:

> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On 8 July 2013 14:45, Alexander Graf <agraf@suse.de> wrote:
>>> On 08.07.2013, at 15:23, Peter Maydell wrote:
>>>> Now I'm completely confused. Why would assigned devices
>>>> have anything to do with this? Can you explain in more
>>>> detail, because I don't really see what you're suggesting?
>>> 
>>> The only missing link we have to create any device using -device
>>> on the command line is the IRQ line enumeration. If we can allocate
>>> IRQ lines automatically, we can put any command line given -device
>>> onto our main system bus that is non-pci, non-isa.
>> 
>> If the user is expected to be able to get the MMIO address
>> right (which they'd have to specify on the command line
>> somehow too) why not require them to specify the IRQ number
>> while they're doing it? I'm a bit suspicious of anything
>> that requires the user to specify to that level of detail
>> though, since it requires a lot of inside knowledge about the
>> board.
>> 
>> This is the whole reason for having the separate transport:
>> the board gets to take care of the board specific detail of
>> how to wire up the transport, and the user just asks to
>> create the backends that plug automatically into it.
>> The virtio command line options are complicated and confusing
>> enough as it is.
>> 
>>> So if we want to ever support VFIO for platform devices,
>>> the user will want to pass -device vfio-ahci,foo=bar on
>>> the command line to assign an AHCI device.
>> 
>> This appears to be seriously short on actually specifying
>> enough information to wire a device up.
>> 
>>> The only infrastructure blocker we have for that today
>>> is the IRQ allocation.
>> 
>> DMA lines? Specifying the right location in the address space?
>> 
>>> Maybe we could even try to be as smart as putting the MMIO
>>> regions into guest address space intelligently automatically.
>> 
>> This sounds likely to cause problems with migration unless
>> we can guarantee that we always pick the same place.
> 
> I think we're trying to fit a square peg into a round hole.
> 
> virtio-mmio is a virtio transport where each device has a dedicated set
> of system resources.
> 
> Alex, it sounds like you want virtio-mmio-bus which would be a single
> set of system resources that implemented a virtio bus on top of it.

Well, what I really want is a sysbus that behaves like PCI from a usability point of view ;).


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Anthony Liguori]

Alexander Graf <agraf@suse.de> writes:

> On 08.07.2013, at 22:08, Anthony Liguori wrote:
>
>> I think we're trying to fit a square peg into a round hole.
>> 
>> virtio-mmio is a virtio transport where each device has a dedicated set
>> of system resources.
>> 
>> Alex, it sounds like you want virtio-mmio-bus which would be a single
>> set of system resources that implemented a virtio bus on top of it.
>
> Well, what I really want is a sysbus that behaves like PCI from a
> usability point of view ;).

Which means you need to have (1) a discovery mechanism with a stable
addressing mechanism (2) a way to communicate this to the guest from the
host.

That's all that PCI is.  A host controller is a "sysbus" device that
uses a standardized discovery and addressing mechanism.

Regards,

Anthony Liguori

>
>
> Alex

Re: [Qemu-devel] [PATCH 3/8] virtio: Add support for guest setting of queue size

[Peter Maydell]

On 8 July 2013 20:39, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> +void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>> +{
>> +    vdev->vq[n].vring.num = num;
>> +    virtqueue_init(&vdev->vq[n]);
>
> I think some level of checking is probably needed on num since we do a
> tremendous amount of math on it.  I doubt it's exploitable since it's
> always treated as a PA, but better to be safe than sorry.

So at the moment we do that in the transport:

+        if (value <= VIRTQUEUE_MAX_SIZE) {
+            DPRINTF("calling virtio_queue_set_num\n");
+            virtio_queue_set_num(vdev, vdev->queue_sel, value);
+        }

but I agree it would be better done here in the generic code.

-- PMM

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Andreas Färber]

Am 08.07.2013 23:06, schrieb Anthony Liguori:
> Alexander Graf <agraf@suse.de> writes:
> 
>> On 08.07.2013, at 22:08, Anthony Liguori wrote:
>>
>>> I think we're trying to fit a square peg into a round hole.
>>>
>>> virtio-mmio is a virtio transport where each device has a dedicated set
>>> of system resources.
>>>
>>> Alex, it sounds like you want virtio-mmio-bus which would be a single
>>> set of system resources that implemented a virtio bus on top of it.
>>
>> Well, what I really want is a sysbus that behaves like PCI from a
>> usability point of view ;).
> 
> Which means you need to have (1) a discovery mechanism with a stable
> addressing mechanism (2) a way to communicate this to the guest from the
> host.

FWIW I vaguely remember a Lightning Talk at FOSDEM 2012 about MMIO
device discovery through chained in-memory data structures or so:
https://archive.fosdem.org/2012/schedule/event/wishbone.html
http://www.ohwr.org/projects/fpga-config-space

Maybe such metadata could be supplied alongside a virtual device,
whether on Wishbone, AMBA or whatever? Just a thought.

Regards,
Andreas

> That's all that PCI is.  A host controller is a "sysbus" device that
> uses a standardized discovery and addressing mechanism.
> 
> Regards,
> 
> Anthony Liguori

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Alexander Graf]

On 08.07.2013, at 23:06, Anthony Liguori wrote:

> Alexander Graf <agraf@suse.de> writes:
> 
>> On 08.07.2013, at 22:08, Anthony Liguori wrote:
>> 
>>> I think we're trying to fit a square peg into a round hole.
>>> 
>>> virtio-mmio is a virtio transport where each device has a dedicated set
>>> of system resources.
>>> 
>>> Alex, it sounds like you want virtio-mmio-bus which would be a single
>>> set of system resources that implemented a virtio bus on top of it.
>> 
>> Well, what I really want is a sysbus that behaves like PCI from a
>> usability point of view ;).
> 
> Which means you need to have (1) a discovery mechanism with a stable
> addressing mechanism

That's what dtb usually gives you.

> (2) a way to communicate this to the guest from the
> host.

Not if the host dictates everything. PCI is only complicated because it allows the guest control. If we don't we can have a push-only interface.

But I'm not sure we should hold back this patch series based on this. I can try to come up with a bus that can automatically place memory regions and IRQs. Then I can add a virtio-mmio-awesomebus type and show you what I mean ;)

For the time being, we can live with a few statically allocated virtio transports I think. As long as we don't promise the user that they're still going to be there in the next version of QEMU.


Alex

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Christoffer Dall]

On Wed, Jul 10, 2013 at 12:56:31PM +0200, Alexander Graf wrote:
> 
> On 08.07.2013, at 23:06, Anthony Liguori wrote:
> 
> > Alexander Graf <agraf@suse.de> writes:
> > 
> >> On 08.07.2013, at 22:08, Anthony Liguori wrote:
> >> 
> >>> I think we're trying to fit a square peg into a round hole.
> >>> 
> >>> virtio-mmio is a virtio transport where each device has a dedicated set
> >>> of system resources.
> >>> 
> >>> Alex, it sounds like you want virtio-mmio-bus which would be a single
> >>> set of system resources that implemented a virtio bus on top of it.
> >> 
> >> Well, what I really want is a sysbus that behaves like PCI from a
> >> usability point of view ;).
> > 
> > Which means you need to have (1) a discovery mechanism with a stable
> > addressing mechanism
> 
> That's what dtb usually gives you.
> 
> > (2) a way to communicate this to the guest from the
> > host.
> 
> Not if the host dictates everything. PCI is only complicated because it allows the guest control. If we don't we can have a push-only interface.
> 
> But I'm not sure we should hold back this patch series based on this. I can try to come up with a bus that can automatically place memory regions and IRQs. Then I can add a virtio-mmio-awesomebus type and show you what I mean ;)
> 
> For the time being, we can live with a few statically allocated virtio transports I think. As long as we don't promise the user that they're still going to be there in the next version of QEMU.
> 
> 
I'm not familiar enough with QEMU internals to intelligently comment on
this discussion, but I do have two observations:

(1) It would be tremendously beneficial to have this patch series
merged, so people can actually start to have upstream code that supports
usable VMs on ARM, and it doesn't seem too invasive to me.

(2) About device assignment, what is the QEMU vision in terms of how it
plugs into a board?  Do we expect a mach-virt scenario where we
dynamically create assigned devices, or would we emulate some board that
actually has the peripherals that we are going to assign and emulate the
rest of the devices?

-Christoffer

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

On 17 July 2013 10:30, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> I'm not familiar enough with QEMU internals to intelligently comment on
> this discussion, but I do have two observations:
>
> (1) It would be tremendously beneficial to have this patch series
> merged, so people can actually start to have upstream code that supports
> usable VMs on ARM, and it doesn't seem too invasive to me.

Well, v3 of the patchset rather than this one, but yes.
Anthony -- are you going to apply it directly (at some point,
v3 has only been on list a day or so) as virtio maintainer,
or should I put it in via arm-devs?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/8] Add virtio-mmio and use it in vexpress

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 July 2013 10:30, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> I'm not familiar enough with QEMU internals to intelligently comment on
>> this discussion, but I do have two observations:
>>
>> (1) It would be tremendously beneficial to have this patch series
>> merged, so people can actually start to have upstream code that supports
>> usable VMs on ARM, and it doesn't seem too invasive to me.
>
> Well, v3 of the patchset rather than this one, but yes.
> Anthony -- are you going to apply it directly (at some point,
> v3 has only been on list a day or so) as virtio maintainer,
> or should I put it in via arm-devs?

I would be happy for it to go through arm-devs and for any future
changes to virtio-mmio to go through that tree.

Regards,

Anthony Liguori

>
> thanks
> -- PMM