[Qemu-devel] [RFC 0/4] virtio-mmio transport

[Qemu-devel] [RFC 0/4] virtio-mmio transport

[Peter Maydell]

This set of patches implements the QEMU end of the MMIO virtio transport
(as specified by Appendix X of the latest virtio spec from here
http://ozlabs.org/~rusty/virtio-spec/virtio.pdf
and implemented by patches which I think are going into Linux 3.2).

I submit this only as an RFC because although it works (tested
virtio-blk anyway) it has some issues:
 * extra vring alignment field not saved/restored (because virtio
   layer isn't using VMState and doesn't allow the virtio base layer
   to specify a version for its data so back-compat would be tricky)
 * you have to specify which kind of virtio device you want in the
   board model. In particular this means that for virtio-blk the user
   has to say "-drive if=none,file=whatever.img,id=myimg 
   -global virtio-blk-mmio.drive=myimg" or the virtio-blk layer will
   refuse to start, which is not at all userfriendly

Anthony's suggestion was that we should have a qdev bus between
the transport layer and the -blk/-net/-etc parts. Then the board could
instantiate N virtio-mmio transport instances, and the user uses
-device to create a virtio-blk device and assign it to the relevant
slot. (PCI virtio would also split similarly so you'd end up with
pci-controller -> n lots of [ virtio-pci-transport + virtio-blk])
plus some syntactic sugar for back-compatibility I guess.)

However I'm not currently sure when I'll have time to do that
refactoring, so I'm just sending this patchset out as an RFC
for now.

Peter Maydell (4):
  virtio: Add support for guest setting of queue size
  virtio: Support transports which can specify the vring alignment
  Add MMIO based virtio transport
  hw/vexpress.c: Add MMIO virtio transport

 Makefile.objs    |    1 +
 hw/vexpress.c    |    3 +
 hw/virtio-mmio.c |  426 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio.c      |   20 +++-
 hw/virtio.h      |    2 +
 5 files changed, 450 insertions(+), 2 deletions(-)
 create mode 100644 hw/virtio-mmio.c

[Qemu-devel] [RFC 1/4] 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.c |    6 ++++++
 hw/virtio.h |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 7011b5b..5bd43a1 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -619,6 +619,12 @@ target_phys_addr_t 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/hw/virtio.h b/hw/virtio.h
index 2d18209..352aba6 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -178,6 +178,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, target_phys_addr_t addr);
 target_phys_addr_t 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.1

[Qemu-devel] [RFC 2/4] 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.)

FIXME save/load support for this new field!

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

diff --git a/hw/virtio.c b/hw/virtio.c
index 5bd43a1..a08bacb 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -19,7 +19,9 @@
 #include "qemu-barrier.h"
 
 /* The alignment to use between consumer and producer parts of vring.
- * x86 pagesize again. */
+ * 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
@@ -53,6 +55,7 @@ typedef struct VRingUsed
 typedef struct VRing
 {
     unsigned int num;
+    unsigned int align;
     target_phys_addr_t desc;
     target_phys_addr_t avail;
     target_phys_addr_t used;
@@ -90,7 +93,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(target_phys_addr_t desc_pa, int i)
@@ -630,6 +633,12 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
     return vdev->vq[n].vring.num;
 }
 
+void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
+{
+    vdev->vq[n].vring.align = align;
+    virtqueue_init(&vdev->vq[n]);
+}
+
 void virtio_queue_notify_vq(VirtQueue *vq)
 {
     if (vq->vring.desc) {
@@ -670,6 +679,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];
diff --git a/hw/virtio.h b/hw/virtio.h
index 352aba6..9b068e3 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -180,6 +180,7 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, target_phys_addr_t addr);
 target_phys_addr_t 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.1

[Qemu-devel] [RFC 3/4] Add MMIO based virtio transport

[Peter Maydell]

Add support for the generic MMIO based virtio transport (including
blk, net, serial and balloon devices).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 Makefile.objs    |    1 +
 hw/virtio-mmio.c |  426 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 427 insertions(+), 0 deletions(-)
 create mode 100644 hw/virtio-mmio.c

diff --git a/Makefile.objs b/Makefile.objs
index d7a6539..f7b6dbc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -188,6 +188,7 @@ hw-obj-y += vl.o loader.o
 hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
 hw-obj-y += usb-libhw.o
 hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
+hw-obj-$(CONFIG_VIRTIO) += virtio-mmio.o
 hw-obj-y += fw_cfg.o
 hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
 hw-obj-$(CONFIG_PCI) += msix.o msi.o
diff --git a/hw/virtio-mmio.c b/hw/virtio-mmio.c
new file mode 100644
index 0000000..9c81440
--- /dev/null
+++ b/hw/virtio-mmio.c
@@ -0,0 +1,426 @@
+/*
+ * 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/>.
+ */
+
+/* TODO:
+ *  * save/load support
+ *  * test net, serial, balloon
+ */
+
+#include "sysbus.h"
+#include "virtio.h"
+#include "virtio-blk.h"
+#include "virtio-net.h"
+#include "virtio-serial.h"
+#include "host-utils.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
+
+/* 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 busdev;
+    VirtIODevice *vdev;
+    MemoryRegion iomem;
+    qemu_irq irq;
+    uint32_t int_enable;
+    uint32_t host_features;
+    uint32_t host_features_sel;
+    uint32_t guest_features_sel;
+    uint32_t guest_page_shift;
+    /* virtio-blk */
+    BlockConf block;
+    char *block_serial;
+    /* virtio-net */
+    NICConf nic;
+    virtio_net_conf net;
+    /* virtio-serial */
+    virtio_serial_conf serial;
+} VirtIOMMIOProxy;
+
+static uint64_t virtio_mmio_read(void *opaque, target_phys_addr_t offset,
+                                 unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->vdev;
+    DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
+    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, target_phys_addr_t offset,
+                              uint64_t value, unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->vdev;
+    DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
+            (int)offset, value);
+    if (offset >= VIRTIO_MMIO_CONFIG) {
+        offset -= VIRTIO_MMIO_CONFIG;
+        switch (size) {
+        case 1:
+            virtio_config_writeb(vdev, offset, value);
+        case 2:
+            virtio_config_writew(vdev, offset, value);
+        case 4:
+            virtio_config_writel(vdev, offset, value);
+        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) {
+            vdev->guest_features = 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(void *opaque, uint16_t vector)
+{
+    VirtIOMMIOProxy *proxy = opaque;
+    int level = (proxy->vdev->isr != 0);
+    DPRINTF("virtio_mmio setting IRQ %d\n", level);
+    qemu_set_irq(proxy->irq, level);
+}
+
+static unsigned int virtio_mmio_get_features(void *opaque)
+{
+    VirtIOMMIOProxy *proxy = opaque;
+    return proxy->host_features;
+}
+
+static int virtio_mmio_load_config(void *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = opaque;
+    proxy->int_enable = qemu_get_be32(f);
+    proxy->host_features = qemu_get_be32(f);
+    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(void *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = opaque;
+    qemu_put_be32(f, proxy->int_enable);
+    qemu_put_be32(f, proxy->host_features);
+    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 VirtIOBindings virtio_mmio_bindings = {
+    .notify = virtio_mmio_update_irq,
+    .get_features = virtio_mmio_get_features,
+    .save_config = virtio_mmio_save_config,
+    .load_config = virtio_mmio_load_config,
+};
+
+static int virtio_init_mmio(VirtIOMMIOProxy *proxy, VirtIODevice *vdev)
+{
+    proxy->vdev = vdev;
+    proxy->vdev->nvectors = 0;
+    sysbus_init_irq(&proxy->busdev, &proxy->irq);
+    memory_region_init_io(&proxy->iomem, &virtio_mem_ops, proxy,
+                          "virtio-mmio", 0x1000);
+    sysbus_init_mmio_region(&proxy->busdev, &proxy->iomem);
+    virtio_bind_device(vdev, &virtio_mmio_bindings, proxy);
+    proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+    proxy->host_features = vdev->get_features(vdev, proxy->host_features);
+    return 0;
+}
+
+
+static int virtio_blk_init_mmio(SysBusDevice *dev)
+{
+    VirtIODevice *vdev;
+    VirtIOMMIOProxy *proxy = FROM_SYSBUS(VirtIOMMIOProxy, dev);
+    vdev = virtio_blk_init(&dev->qdev, &proxy->block, &proxy->block_serial);
+    if (!vdev) {
+        return -1;
+    }
+    return virtio_init_mmio(proxy, vdev);
+}
+
+static int virtio_net_init_mmio(SysBusDevice *dev)
+{
+    VirtIODevice *vdev;
+    VirtIOMMIOProxy *proxy = FROM_SYSBUS(VirtIOMMIOProxy, dev);
+    vdev = virtio_net_init(&dev->qdev, &proxy->nic, &proxy->net);
+    if (!vdev) {
+        return -1;
+    }
+    return virtio_init_mmio(proxy, vdev);
+}
+
+static int virtio_serial_init_mmio(SysBusDevice *dev)
+{
+    VirtIODevice *vdev;
+    VirtIOMMIOProxy *proxy = FROM_SYSBUS(VirtIOMMIOProxy, dev);
+    vdev = virtio_serial_init(&dev->qdev, &proxy->serial);
+    if (!vdev) {
+        return -1;
+    }
+    return virtio_init_mmio(proxy, vdev);
+}
+
+static int virtio_balloon_init_mmio(SysBusDevice *dev)
+{
+    VirtIODevice *vdev;
+    VirtIOMMIOProxy *proxy = FROM_SYSBUS(VirtIOMMIOProxy, dev);
+    vdev = virtio_balloon_init(&dev->qdev);
+    if (!vdev) {
+        return -1;
+    }
+    return virtio_init_mmio(proxy, vdev);
+}
+
+static void virtio_mmio_reset(DeviceState *d)
+{
+    VirtIOMMIOProxy *proxy = FROM_SYSBUS(VirtIOMMIOProxy, sysbus_from_qdev(d));
+    virtio_reset(proxy->vdev);
+}
+
+static SysBusDeviceInfo virtio_mmio_info[] = {
+    {
+        .qdev.name = "virtio-blk-mmio",
+        .qdev.size = sizeof(VirtIOMMIOProxy),
+        .init = virtio_blk_init_mmio,
+        .qdev.reset = virtio_mmio_reset,
+        .qdev.props = (Property[]) {
+            DEFINE_BLOCK_PROPERTIES(VirtIOMMIOProxy, block),
+            DEFINE_PROP_STRING("serial", VirtIOMMIOProxy, block_serial),
+            DEFINE_VIRTIO_BLK_FEATURES(VirtIOMMIOProxy, host_features),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        .qdev.name = "virtio-net-mmio",
+        .qdev.size = sizeof(VirtIOMMIOProxy),
+        .init = virtio_net_init_mmio,
+        .qdev.reset = virtio_mmio_reset,
+        .qdev.props = (Property[]) {
+            DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features),
+            DEFINE_NIC_PROPERTIES(VirtIOMMIOProxy, nic),
+            DEFINE_PROP_UINT32("x-txtimer", VirtIOMMIOProxy,
+                               net.txtimer, TX_TIMER_INTERVAL),
+            DEFINE_PROP_INT32("x-txburst", VirtIOMMIOProxy,
+                              net.txburst, TX_BURST),
+            DEFINE_PROP_STRING("tx", VirtIOMMIOProxy, net.tx),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        .qdev.name = "virtio-serial-mmio",
+        .qdev.size = sizeof(VirtIOMMIOProxy),
+        .init = virtio_serial_init_mmio,
+        .qdev.reset = virtio_mmio_reset,
+        .qdev.props = (Property[]) {
+            DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features),
+            DEFINE_PROP_UINT32("max_ports", VirtIOMMIOProxy,
+                               serial.max_virtserial_ports, 31),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        .qdev.name = "virtio-balloon-mmio",
+        .qdev.size = sizeof(VirtIOMMIOProxy),
+        .init = virtio_balloon_init_mmio,
+        .qdev.reset = virtio_mmio_reset,
+        .qdev.props = (Property[]) {
+            DEFINE_VIRTIO_COMMON_FEATURES(VirtIOMMIOProxy, host_features),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        /* end of list */
+    }
+};
+
+static void virtio_mmio_register_devices(void)
+{
+    SysBusDeviceInfo *info;
+    for (info = virtio_mmio_info; info->qdev.name; info++) {
+        sysbus_register_withprop(info);
+    }
+}
+
+device_init(virtio_mmio_register_devices)
-- 
1.7.1

[Qemu-devel] [RFC 4/4] hw/vexpress.c: Add MMIO virtio transport

[Peter Maydell]

Add the MMIO virtio transport.
---
 hw/vexpress.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/vexpress.c b/hw/vexpress.c
index 0940a26..fe66185 100644
--- a/hw/vexpress.c
+++ b/hw/vexpress.c
@@ -150,6 +150,9 @@ static void vexpress_a9_init(ram_addr_t ram_size,
 
     /* 0x1001a000 Compact Flash */
 
+    /* 0x1001e000 MMIO virtio transport */
+    sysbus_create_simple("virtio-blk-mmio", 0x1001e000, pic[42]);
+
     /* 0x1001f000 PL111 CLCD (motherboard) */
 
     /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
-- 
1.7.1

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Stefan Hajnoczi]

On Mon, Nov 14, 2011 at 2:55 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> This set of patches implements the QEMU end of the MMIO virtio transport
> (as specified by Appendix X of the latest virtio spec from here
> http://ozlabs.org/~rusty/virtio-spec/virtio.pdf
> and implemented by patches which I think are going into Linux 3.2).
>
> I submit this only as an RFC because although it works (tested
> virtio-blk anyway) it has some issues:

virtio-net is more complicated and definitely worth testing.

Stefan

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paolo Bonzini]

On 11/14/2011 03:55 PM, Peter Maydell wrote:
> This set of patches implements the QEMU end of the MMIO virtio transport
> (as specified by Appendix X of the latest virtio spec from here
> http://ozlabs.org/~rusty/virtio-spec/virtio.pdf
> and implemented by patches which I think are going into Linux 3.2).

How does this compare against hw/syborg_virtio.c?

Paolo

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Peter Maydell]

On 16 November 2011 14:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 11/14/2011 03:55 PM, Peter Maydell wrote:
>>
>> This set of patches implements the QEMU end of the MMIO virtio transport
>> (as specified by Appendix X of the latest virtio spec from here
>> http://ozlabs.org/~rusty/virtio-spec/virtio.pdf
>> and implemented by patches which I think are going into Linux 3.2).
>
> How does this compare against hw/syborg_virtio.c?

Pawel may have more detail, but to me the significant difference
is that virtio-mmio is an implementation of a specification extension
agreed with the virtio spec maintainers, whereas syborg doesn't seem
to be mentioned in the virtio spec anywhere, so I am unsure what it
is intended to be implementing.

(There are some technical differences too, like virtio-mmio allowing
the guest to specify queue sizes and alignments; these mostly came
out of the process of agreeing the spec extension.)

-- PMM

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Anthony Liguori]

On 11/16/2011 12:41 PM, Peter Maydell wrote:
> On 16 November 2011 14:33, Paolo Bonzini<pbonzini@redhat.com>  wrote:
>> On 11/14/2011 03:55 PM, Peter Maydell wrote:
>>>
>>> This set of patches implements the QEMU end of the MMIO virtio transport
>>> (as specified by Appendix X of the latest virtio spec from here
>>> http://ozlabs.org/~rusty/virtio-spec/virtio.pdf
>>> and implemented by patches which I think are going into Linux 3.2).
>>
>> How does this compare against hw/syborg_virtio.c?
>
> Pawel may have more detail, but to me the significant difference
> is that virtio-mmio is an implementation of a specification extension
> agreed with the virtio spec maintainers, whereas syborg doesn't seem
> to be mentioned in the virtio spec anywhere, so I am unsure what it
> is intended to be implementing.
>
> (There are some technical differences too, like virtio-mmio allowing
> the guest to specify queue sizes and alignments; these mostly came
> out of the process of agreeing the spec extension.)

Correct.  Syborg virtio was something Paul Brook did bit is not an "official" 
virtio transport as far as Linux or the spec is concerned.

I'm not sure what guest software uses the syborg virtio transport.

Regards,

Anthony Liguori

>
> -- PMM
>

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Wed, 2011-11-16 at 19:56 +0000, Anthony Liguori wrote:
> On 11/16/2011 12:41 PM, Peter Maydell wrote:
> > Pawel may have more detail, but to me the significant difference
> > is that virtio-mmio is an implementation of a specification extension
> > agreed with the virtio spec maintainers, whereas syborg doesn't seem
> > to be mentioned in the virtio spec anywhere, so I am unsure what it
> > is intended to be implementing.
> >
> > (There are some technical differences too, like virtio-mmio allowing
> > the guest to specify queue sizes and alignments; these mostly came
> > out of the process of agreeing the spec extension.)
> 
> Correct.  Syborg virtio was something Paul Brook did bit is not an "official" 
> virtio transport as far as Linux or the spec is concerned.

Honestly, that's the first time I hear about it, and as I'm not allowed
to look at qemu code (legal reasons, just don't ask ;-) it's hard for me
to comment. But during the discussions about virtio-mmio, no one
mentioned it at all! Is it similar in ideas?

I do apologise if I jeopardised somebody's work, but it wasn't on
purpose...

Cheers!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paolo Bonzini]

On 11/17/2011 12:20 PM, Pawel Moll wrote:
>> >  Correct.  Syborg virtio was something Paul Brook did bit is not an "official"
>> >  virtio transport as far as Linux or the spec is concerned.
> Honestly, that's the first time I hear about it, and as I'm not allowed
> to look at qemu code (legal reasons, just don't ask;-)  it's hard for me
> to comment. But during the discussions about virtio-mmio, no one
> mentioned it at all! Is it similar in ideas?

Yes. :)

> I do apologise if I jeopardised somebody's work, but it wasn't on
> purpose...

No worries at all, I was more interested about code duplication in QEMU. 
  But perhaps syborg_virtio could simply go away.

Paolo

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paul Brook]

[Replying to various bits of this thread all at once]

>  * you have to specify which kind of virtio device you want in the
>    board model. In particular this means that for virtio-blk the user
>    has to say "-drive if=none,file=whatever.img,id=myimg
>    -global virtio-blk-mmio.drive=myimg" or the virtio-blk layer will
>    refuse to start, which is not at all userfriendly

This is pretty lame.  I see two or three bugs here:

1) We're greating virtio devices unconditionally.  Much better would be to 
have devices be allocated as necessary.  e.g. unless the user explicitly 
requests otherwise, put the first device at a particular address, and 
subsequent devices at sequential addresses. The guest OS can then easily probe 
them to see which are present.  I guess a null virtio device might make sense 
to avoid accessing undefined memory regions.
For the vexpress board the logic tile site (0xc0000000) seems like a sensible 
location.  Your current choice of an area reserved by the southbridge seems 
somewhat questionable.

2) virtio-blk fails if no drive is connected.  I guess you could call this a 
feature, which means you must fix (1).  If not then I think it's reasonable 
for hardcoded board devices should also use hardcoded drive IDs.  i.e. we act 
as if the user specified "-global virtio-blk-mmio.drive=virtio" or similar

3) "qemu -drive if=virtio" assmes you want PCI virtio.  Either we consider 
this an obsolete legacy option and document it as such, or fix it as part of 
(1).  In the latter case it's probably going to do the wrong thing on boards 
that have both virtio-mmio and PCI (there are at least two correct answers).

>[...]
> Correct.  Syborg virtio was something Paul Brook did bit is not an
> "official" virtio transport as far as Linux or the spec is concerned.
>
> I'm not sure what guest software uses the syborg virtio transport.

It is/was a virtual reference platform for SymbianOS.  However since then the 
Symbian Foundation got shot in the back of the head and the rest of Nokia 
jumped ship to Windows, so I'd be surprised if anyone really cares about it.

Paul

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Anthony Liguori]

On 12/09/2011 09:16 AM, Paul Brook wrote:
> [Replying to various bits of this thread all at once]
>
>>   * you have to specify which kind of virtio device you want in the
>>     board model. In particular this means that for virtio-blk the user
>>     has to say "-drive if=none,file=whatever.img,id=myimg
>>     -global virtio-blk-mmio.drive=myimg" or the virtio-blk layer will
>>     refuse to start, which is not at all userfriendly
>
> This is pretty lame.  I see two or three bugs here:
>
> 1) We're greating virtio devices unconditionally.  Much better would be to
> have devices be allocated as necessary.  e.g. unless the user explicitly
> requests otherwise, put the first device at a particular address, and
> subsequent devices at sequential addresses. The guest OS can then easily probe
> them to see which are present.  I guess a null virtio device might make sense
> to avoid accessing undefined memory regions.
> For the vexpress board the logic tile site (0xc0000000) seems like a sensible
> location.  Your current choice of an area reserved by the southbridge seems
> somewhat questionable.
>
> 2) virtio-blk fails if no drive is connected.  I guess you could call this a
> feature, which means you must fix (1).  If not then I think it's reasonable
> for hardcoded board devices should also use hardcoded drive IDs.  i.e. we act
> as if the user specified "-global virtio-blk-mmio.drive=virtio" or similar
>
> 3) "qemu -drive if=virtio" assmes you want PCI virtio.  Either we consider
> this an obsolete legacy option and document it as such, or fix it as part of
> (1).  In the latter case it's probably going to do the wrong thing on boards
> that have both virtio-mmio and PCI (there are at least two correct answers).
>
>> [...]
>> Correct.  Syborg virtio was something Paul Brook did bit is not an
>> "official" virtio transport as far as Linux or the spec is concerned.
>>
>> I'm not sure what guest software uses the syborg virtio transport.
>
> It is/was a virtual reference platform for SymbianOS.  However since then the
> Symbian Foundation got shot in the back of the head and the rest of Nokia
> jumped ship to Windows, so I'd be surprised if anyone really cares about it.

Can we remove syborg virtio then?

Regards,

Anthony Liguori

>
> Paul
>

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paul Brook]

> >> I'm not sure what guest software uses the syborg virtio transport.
> > 
> > It is/was a virtual reference platform for SymbianOS.  However since then
> > the Symbian Foundation got shot in the back of the head and the rest of
> > Nokia jumped ship to Windows, so I'd be surprised if anyone really cares
> > about it.
> 
> Can we remove syborg virtio then?

If we do, then I recommend removing the whole of the rest of the syborg 
machine at the same time.

AFAIK the only use of this machine was the Symbian Virtual Platform reference 
environment provided by the Symbian Foundation.  This organisation no longer 
exists, and the code (both qemu and guest OS) they hosted is no longer 
publicly available.  The qemu.org code is also a stripped-down version of the 
full SVP.

In short I do not have any good reasons to keep this code.


I've taken a look at the virtion-mmio spec, and it looks fairly reasonable.

The only thing I'd change is the GuestPageSize/QueuePFN mess.  Seems like just 
using straight 64-bit addresses would be a better solution.  Maybe split into 
a high/low pair to keep all registers as 32-bit registers.

Assuming a 4k page size, the pfn approach makes it painful to exceed a 44-bit 
physical address space.  Modern x86 cores have already supprt larger physical 
address spaces, and architectural limits are even higher.  Remember that 
physical address space need not be densely populated.

Paul

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 02:52 +0000, Paul Brook wrote:
> I've taken a look at the virtion-mmio spec, and it looks fairly
> reasonable.
> 
> The only thing I'd change is the GuestPageSize/QueuePFN mess.  Seems like just 
> using straight 64-bit addresses would be a better solution.  Maybe split into 
> a high/low pair to keep all registers as 32-bit registers.

This can be done fairly simple by:
1. Bumping Version register content.
2. Adding two registers: QueueAddrLow (0x44) and QueueAddrHigh (0x48).
3. Describing the QueuePFN as obsolete.

Than the driver would just use Addr or PFN depending on the device's
version.

I can do that, but not this year (on holiday from Friday 16th, without
any access to Internet whatsoever :-) One think to be decided is in what
order the halfs should be filled? Low first, then high? High then low?
Does it matter at all? :-)

Cheers!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 02:52 +0000, Paul Brook wrote:
> I've taken a look at the virtion-mmio spec, and it looks fairly
> reasonable.
> 
> The only thing I'd change is the GuestPageSize/QueuePFN mess.  Seems like just 
> using straight 64-bit addresses would be a better solution.  Maybe split into 
> a high/low pair to keep all registers as 32-bit registers.

This can be done fairly simple by:
1. Bumping Version register content.
2. Adding two registers: QueueAddrLow (0x44) and QueueAddrHigh (0x48).
3. Describing the QueuePFN as obsolete.

Than the driver would just use Addr or PFN depending on the device's
version.

I can do that, but not this year (on holiday from Friday 16th, without
any access to Internet whatsoever :-) One think to be decided is in what
order the halfs should be filled? Low first, then high? High then low?
Does it matter at all? :-)

Cheers!

Paweł


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Stefan Hajnoczi]

I noticed the virtio-mmio spec has an interrupt status register.  On
x86 and virtio-pci things are moving towards Message Signalled
Interrupts and virtqueues having their own interrupts for better
performance and flexibility.  Any thoughts on how 1 interrupt per
virtqueue works for virtio-mmio?

My feeling is that the interrupt details are board-specific and can't
be described in virtio-mmio, but it still jumped out at me that it
looks like you're only thinking of 1 interrupt for the device.

Stefan

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 12:14 +0000, Stefan Hajnoczi wrote:
> I noticed the virtio-mmio spec has an interrupt status register.  On
> x86 and virtio-pci things are moving towards Message Signalled
> Interrupts and virtqueues having their own interrupts for better
> performance and flexibility.  Any thoughts on how 1 interrupt per
> virtqueue works for virtio-mmio?

This could be done by either creating devices with more then one
interrupt (platform device can take any number of resources) and
declaring that first queue uses the first one etc.

> My feeling is that the interrupt details are board-specific and can't
> be described in virtio-mmio, 

It's just the the "design pattern" in the "embedded world" that devices
usually have one interrupt output, shared between its internal
functions. And - of course - there is no in-band signalling (like MSI)
possible - interrupt lines are just "wires" :-) In a boundary case
scenario we may face a situation when total amount of interrupts for all
queues may actually exceed amount of interrupt inputs available in the
interrupt controller...

There may be a half-way solution - one interrupt per device but the
"active" queue number notified via the interrupt status register (as a
FIFO) so the driver wouldn't have to enumerate all the queues.

> but it still jumped out at me that it
> looks like you're only thinking of 1 interrupt for the device.

Yes, current version assumes tgat. I'll keep this in mind when planning
changes for v2 (next year ;-), thanks for letting me know!

Cheers!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Michael S. Tsirkin]

On Mon, Dec 12, 2011 at 12:28:27PM +0000, Pawel Moll wrote:
> On Mon, 2011-12-12 at 12:14 +0000, Stefan Hajnoczi wrote:
> > I noticed the virtio-mmio spec has an interrupt status register.  On
> > x86 and virtio-pci things are moving towards Message Signalled
> > Interrupts and virtqueues having their own interrupts for better
> > performance and flexibility.  Any thoughts on how 1 interrupt per
> > virtqueue works for virtio-mmio?
> 
> This could be done by either creating devices with more then one
> interrupt (platform device can take any number of resources) and
> declaring that first queue uses the first one etc.

We currently support mapping from virtqueues to interrupt
vectors in virtio core. Only virtio pci uses that
but mmio can too. It's better than fixed mapping
IMO as driver can control resources per queue.

> > My feeling is that the interrupt details are board-specific and can't
> > be described in virtio-mmio, 
> 
> It's just the the "design pattern" in the "embedded world" that devices
> usually have one interrupt output, shared between its internal
> functions. And - of course - there is no in-band signalling (like MSI)
> possible - interrupt lines are just "wires" :-) In a boundary case
> scenario we may face a situation when total amount of interrupts for all
> queues may actually exceed amount of interrupt inputs available in the
> interrupt controller...
> 
> There may be a half-way solution - one interrupt per device but the
> "active" queue number notified via the interrupt status register (as a
> FIFO) so the driver wouldn't have to enumerate all the queues.

We could use a queue for this certainly.
Why do you have so many queues?

> > but it still jumped out at me that it
> > looks like you're only thinking of 1 interrupt for the device.
> 
> Yes, current version assumes tgat. I'll keep this in mind when planning
> changes for v2 (next year ;-), thanks for letting me know!
> 
> Cheers!
> 
> Paweł
> 

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 13:12 +0000, Michael S. Tsirkin wrote:
> On Mon, Dec 12, 2011 at 12:28:27PM +0000, Pawel Moll wrote:
> > On Mon, 2011-12-12 at 12:14 +0000, Stefan Hajnoczi wrote:
> > > I noticed the virtio-mmio spec has an interrupt status register.  On
> > > x86 and virtio-pci things are moving towards Message Signalled
> > > Interrupts and virtqueues having their own interrupts for better
> > > performance and flexibility.  Any thoughts on how 1 interrupt per
> > > virtqueue works for virtio-mmio?
> > 
> > This could be done by either creating devices with more then one
> > interrupt (platform device can take any number of resources) and
> > declaring that first queue uses the first one etc.
> 
> We currently support mapping from virtqueues to interrupt
> vectors in virtio core. Only virtio pci uses that
> but mmio can too. It's better than fixed mapping
> IMO as driver can control resources per queue.

I'll keep that in mind.

> > > My feeling is that the interrupt details are board-specific and can't
> > > be described in virtio-mmio, 
> > 
> > It's just the the "design pattern" in the "embedded world" that devices
> > usually have one interrupt output, shared between its internal
> > functions. And - of course - there is no in-band signalling (like MSI)
> > possible - interrupt lines are just "wires" :-) In a boundary case
> > scenario we may face a situation when total amount of interrupts for all
> > queues may actually exceed amount of interrupt inputs available in the
> > interrupt controller...
> > 
> > There may be a half-way solution - one interrupt per device but the
> > "active" queue number notified via the interrupt status register (as a
> > FIFO) so the driver wouldn't have to enumerate all the queues.
> 
> We could use a queue for this certainly.

Hm, yes, I suppose so :-) This would be a "system-level" queue rather
than the normal one, but I guess we could do that.

> Why do you have so many queues?

I assume this a question about the example I gave above? The answer is:
I obviously don't :-) This was just to point out that there _may_ be a
problem if we wanted to allocate an interrupt per queue.

Cheers!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paul Brook]

> I can do that, but not this year (on holiday from Friday 16th, without
> any access to Internet whatsoever :-) One think to be decided is in what
> order the halfs should be filled? Low first, then high? High then low?
> Does it matter at all? :-)

My inital though was that you shouldn't be changing this value when the ring 
is enabled.  Unfortunately you disable the ring by setting the address to zero 
so that argument doesn't work :-/

I suggest that the device to buffer writes to the high part, and construct the 
actual 64-bit value when the low part is written.  That allows 32-bit guests 
can ignore the high part entirely.  Requiring the guest always write high then 
low also works, though I don't see any benefit to either guest or host.

Acting on writes as soon as they occur is not a viable option as it causes the 
device to be enabled before the full address has bee written.  You could say 
the address takes effect after both halves have been written, writes must come 
in pairs, but may occur in either order.  However there is a risk that host 
and guest will somehow get out of sync on which values pair together, so IMO 
this is a bad idea.


If you can't stomach the above then I guess changing the condition for ring 
enablement to QueueNum != 0 and rearanging the configuration sequence 
appropriately would also do the trick.  Having this be different between PCI 
and mmio is probably not worth the confusion though.

Paul

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Paul Brook]

> I can do that, but not this year (on holiday from Friday 16th, without
> any access to Internet whatsoever :-) One think to be decided is in what
> order the halfs should be filled? Low first, then high? High then low?
> Does it matter at all? :-)

My inital though was that you shouldn't be changing this value when the ring 
is enabled.  Unfortunately you disable the ring by setting the address to zero 
so that argument doesn't work :-/

I suggest that the device to buffer writes to the high part, and construct the 
actual 64-bit value when the low part is written.  That allows 32-bit guests 
can ignore the high part entirely.  Requiring the guest always write high then 
low also works, though I don't see any benefit to either guest or host.

Acting on writes as soon as they occur is not a viable option as it causes the 
device to be enabled before the full address has bee written.  You could say 
the address takes effect after both halves have been written, writes must come 
in pairs, but may occur in either order.  However there is a risk that host 
and guest will somehow get out of sync on which values pair together, so IMO 
this is a bad idea.


If you can't stomach the above then I guess changing the condition for ring 
enablement to QueueNum != 0 and rearanging the configuration sequence 
appropriately would also do the trick.  Having this be different between PCI 
and mmio is probably not worth the confusion though.

Paul

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 14:45 +0000, Paul Brook wrote:
> I suggest that the device to buffer writes to the high part, and construct the 
> actual 64-bit value when the low part is written.  That allows 32-bit guests 
> can ignore the high part entirely.

This sounds good to me. If we define the reset value of both registers
as 0 (which is consistent with "0 = stop" condition) a 32-bit guest will
be able to behave as you are suggesting.

Cool, I will keep this in mind.

Thanks!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 14:45 +0000, Paul Brook wrote:
> I suggest that the device to buffer writes to the high part, and construct the 
> actual 64-bit value when the low part is written.  That allows 32-bit guests 
> can ignore the high part entirely.

This sounds good to me. If we define the reset value of both registers
as 0 (which is consistent with "0 = stop" condition) a 32-bit guest will
be able to behave as you are suggesting.

Cool, I will keep this in mind.

Thanks!

Paweł


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Stefan Hajnoczi]

On Mon, Dec 12, 2011 at 12:28 PM, Pawel Moll <pawel.moll@arm.com> wrote:
> On Mon, 2011-12-12 at 12:14 +0000, Stefan Hajnoczi wrote:
>> I noticed the virtio-mmio spec has an interrupt status register.  On
>> x86 and virtio-pci things are moving towards Message Signalled
>> Interrupts and virtqueues having their own interrupts for better
>> performance and flexibility.  Any thoughts on how 1 interrupt per
>> virtqueue works for virtio-mmio?
>
> This could be done by either creating devices with more then one
> interrupt (platform device can take any number of resources) and
> declaring that first queue uses the first one etc.
>
>> My feeling is that the interrupt details are board-specific and can't
>> be described in virtio-mmio,
>
> It's just the the "design pattern" in the "embedded world" that devices
> usually have one interrupt output, shared between its internal
> functions. And - of course - there is no in-band signalling (like MSI)
> possible - interrupt lines are just "wires" :-) In a boundary case
> scenario we may face a situation when total amount of interrupts for all
> queues may actually exceed amount of interrupt inputs available in the
> interrupt controller...
>
> There may be a half-way solution - one interrupt per device but the
> "active" queue number notified via the interrupt status register (as a
> FIFO) so the driver wouldn't have to enumerate all the queues.

If there aren't already then pretty soon ARM-based systems will deal
with PCIe and Message Signalled Interrupts.  Are you sure new ARM
boards are constraints to a small number of physical interrupts?

Stefan

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Peter Maydell]

On 12 December 2011 15:11, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> If there aren't already then pretty soon ARM-based systems will deal
> with PCIe and Message Signalled Interrupts.  Are you sure new ARM
> boards are constraints to a small number of physical interrupts?

Depends what you mean by "small number". The GIC has up to 224
interrupt lines...

-- PMM

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Pawel Moll]

On Mon, 2011-12-12 at 15:11 +0000, Stefan Hajnoczi wrote:
> If there aren't already then pretty soon ARM-based systems will deal
> with PCIe and Message Signalled Interrupts. 

Actually PCI is not an alien in ARM world - we had platforms with PCI
long time ago. And new SOCs aimed at servers come with PCIe indeed. It's
just that bulk of available systems were designed for mobile phones
etc., and I think you agree that in such case not "wasting" silicon on
huge PCI root complex makes at least some sense...

And generally, when you have PCI-ed platform, you just use
virtio-pci :-)

> Are you sure new ARM
> boards are constraints to a small number of physical interrupts?

It's about SOC rather then a board. If I remember correctly Cortex-A9's
GIC (interrupt controller) can be synthesized with up to 224 interrupt
inputs. And I've already seen SOCs with more interrupt sources in the
past...

Having said all this, I don't think it is really a problem now. At least
today virtio-mmio will be used in simple models rather that in huge
KVM/Xen/whatever virtualised systems with hundreds of PV devices.

Cheers!

Paweł

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Rusty Russell]

On Mon, 12 Dec 2011 14:45:26 +0000, Paul Brook <paul@codesourcery.com> wrote:
> > I can do that, but not this year (on holiday from Friday 16th, without
> > any access to Internet whatsoever :-) One think to be decided is in what
> > order the halfs should be filled? Low first, then high? High then low?
> > Does it matter at all? :-)
> 
> My inital though was that you shouldn't be changing this value when the ring 
> is enabled.  Unfortunately you disable the ring by setting the address to zero 
> so that argument doesn't work :-/

It only does that after a reset, and since reset should set the values
to 0 (I don't think the spec says that, but it will for the new config)
we needn't do it at all.

It's just a convenient value for the driver to read and know the ring
has been cleaned up.

I think initialize by QueueNum makes sense.

Thanks,
Rusty.

Re: [Qemu-devel] [RFC 0/4] virtio-mmio transport

[Rusty Russell]

On Mon, 12 Dec 2011 14:45:26 +0000, Paul Brook <paul@codesourcery.com> wrote:
> > I can do that, but not this year (on holiday from Friday 16th, without
> > any access to Internet whatsoever :-) One think to be decided is in what
> > order the halfs should be filled? Low first, then high? High then low?
> > Does it matter at all? :-)
> 
> My inital though was that you shouldn't be changing this value when the ring 
> is enabled.  Unfortunately you disable the ring by setting the address to zero 
> so that argument doesn't work :-/

It only does that after a reset, and since reset should set the values
to 0 (I don't think the spec says that, but it will for the new config)
we needn't do it at all.

It's just a convenient value for the driver to read and know the ring
has been cleaned up.

I think initialize by QueueNum makes sense.

Thanks,
Rusty.