[Qemu-devel] [PATCH v7 1/8] virtio: endian-ambivalent targets using legacy virtio

[Greg Kurz]

From: Rusty Russell <address@hidden>

virtio data structures are defined as "target endian", which assumes
that's a fixed value.  In fact, that actually means it's platform-specific.
The OASIS virtio 1.0 spec will fix this, by making it all little endian.

We introduce memory accessors to be used accross the virtio code where
needed. These accessors should support both legacy and 1.0 devices.
A good way to do it is to introduce a per-device property to store the
endianness. We choose to set this flag at device reset time because it
is reasonnable to assume the endianness won't change unless we reboot or
kexec another kernel. And it is also reasonnable to assume the new kernel
will reset the devices before using them (otherwise it will break).

We reuse the virtio_is_big_endian() helper since it provides the right
value for legacy devices with most of the targets, that have fixed
endianness. It can then be overriden to support endian-ambivalent targets.

To support migration, we need to set the flag in virtio_load() as well.

(a) One solution would be to add it to the stream, but it have some
    drawbacks:
- since this only affects a few targets, the field should be put into a
  subsection
- virtio migration code should be ported to vmstate to be able to introduce
  such a subsection

(b) If we assume the following to be true:
- target endianness falls under some cpu state
- cpu state is always restored before virtio devices state because they
  get initialized in this order in main().
Then an alternative is to rely on virtio_is_big_endian() again at
load time. No need to mess around with the migration stream in this case.

This patch implements (b).

Note that the tswap helpers are implemented in virtio.c so that
virtio-access.h stays platform independant. Most of the virtio code
will be buildable under common-obj instead of obj then, and spare
some cycles when building for multiple targets.

Signed-off-by: Rusty Russell <address@hidden>
[ ldq_phys() API change,
  relicensed virtio-access.h to GPLv2+ on Rusty's request,
  introduce a per-device is_big_endian flag (supersedes needs_byteswap global)
  add VirtIODevice * arg to virtio helpers,
  use the existing virtio_is_big_endian() helper,
  virtio-pci: use the device is_big_endian flag,
  introduce virtio tswap16 and tswap64 helpers,
  move calls to tswap* out of virtio-access.h to make it platform independant,
  migration support,
  Greg Kurz <address@hidden> ]
Cc: Cédric Le Goater <address@hidden>
Signed-off-by: Greg Kurz <address@hidden>
---

Changes since v6:
- merge the virtio_needs_byteswap() helper from v6 and existing
  virtio_is_big_endian()
- virtio-pci: now supports endianness changes
- virtio-access.h fixes (target independant)

 exec.c                            |    2 -
 hw/virtio/Makefile.objs           |    2 -
 hw/virtio/virtio-pci.c            |   11 +--
 hw/virtio/virtio.c                |   35 +++++++++
 include/hw/virtio/virtio-access.h |  138 +++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h        |    2 +
 vl.c                              |    4 +
 7 files changed, 185 insertions(+), 9 deletions(-)
 create mode 100644 include/hw/virtio/virtio-access.h

diff --git a/exec.c b/exec.c
index 91513c6..e6777d0 100644
--- a/exec.c
+++ b/exec.c
@@ -42,6 +42,7 @@
 #else /* !CONFIG_USER_ONLY */
 #include "sysemu/xen-mapcache.h"
 #include "trace.h"
+#include "hw/virtio/virtio.h"
 #endif
 #include "exec/cpu-all.h"
 
@@ -2745,7 +2746,6 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
  * A helper function for the _utterly broken_ virtio device model to find out 
if
  * it's running on a big endian machine. Don't do this at home kids!
  */
-bool virtio_is_big_endian(void);
 bool virtio_is_big_endian(void)
 {
 #if defined(TARGET_WORDS_BIGENDIAN)
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 1ba53d9..68c3064 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -4,5 +4,5 @@ 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 
+obj-y += virtio.o virtio-balloon.o
 obj-$(CONFIG_LINUX) += vhost.o
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ce97514..82a1689 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -89,9 +89,6 @@
 /* Flags track per-device state like workarounds for quirks in older guests. */
 #define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1 << 0)
 
-/* HACK for virtio to determine if it's running a big endian guest */
-bool virtio_is_big_endian(void);
-
 static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtIOPCIProxy *dev);
 
@@ -409,13 +406,13 @@ static uint64_t virtio_pci_config_read(void *opaque, 
hwaddr addr,
         break;
     case 2:
         val = virtio_config_readw(vdev, addr);
-        if (virtio_is_big_endian()) {
+        if (vdev->is_big_endian) {
             val = bswap16(val);
         }
         break;
     case 4:
         val = virtio_config_readl(vdev, addr);
-        if (virtio_is_big_endian()) {
+        if (vdev->is_big_endian) {
             val = bswap32(val);
         }
         break;
@@ -443,13 +440,13 @@ static void virtio_pci_config_write(void *opaque, hwaddr 
addr,
         virtio_config_writeb(vdev, addr, val);
         break;
     case 2:
-        if (virtio_is_big_endian()) {
+        if (vdev->is_big_endian) {
             val = bswap16(val);
         }
         virtio_config_writew(vdev, addr, val);
         break;
     case 4:
-        if (virtio_is_big_endian()) {
+        if (vdev->is_big_endian) {
             val = bswap32(val);
         }
         virtio_config_writel(vdev, addr, val);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aeabf3a..bb646f0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -19,6 +19,7 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/atomic.h"
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
 
 /*
  * The alignment to use between consumer and producer parts of vring.
@@ -546,6 +547,8 @@ void virtio_reset(void *opaque)
 
     virtio_set_status(vdev, 0);
 
+    vdev->is_big_endian = virtio_is_big_endian();
+
     if (k->reset) {
         k->reset(vdev);
     }
@@ -897,6 +900,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
+    /* NOTE: we assume that endianness is a cpu state AND
+     * cpu state is restored before virtio devices.
+     */
+    vdev->is_big_endian = virtio_is_big_endian();
+
     if (k->load_config) {
         ret = k->load_config(qbus->parent, f);
         if (ret)
@@ -1153,6 +1161,33 @@ void virtio_device_set_child_bus_name(VirtIODevice 
*vdev, char *bus_name)
     }
 }
 
+uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
+{
+    if (vdev->is_big_endian) {
+        return tswap16(s);
+    } else {
+        return bswap16(tswap16(s));
+    }
+}
+
+uint32_t virtio_tswap32(VirtIODevice *vdev, uint32_t s)
+{
+    if (vdev->is_big_endian) {
+        return tswap32(s);
+    } else {
+        return bswap32(tswap32(s));
+    }
+}
+
+uint64_t virtio_tswap64(VirtIODevice *vdev, uint64_t s)
+{
+    if (vdev->is_big_endian) {
+        return tswap64(s);
+    } else {
+        return bswap64(tswap64(s));
+    }
+}
+
 static void virtio_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
new file mode 100644
index 0000000..5c9013e
--- /dev/null
+++ b/include/hw/virtio/virtio-access.h
@@ -0,0 +1,138 @@
+/*
+ * Virtio Accessor Support: In case your target can change endian.
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Rusty Russell   <address@hidden>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#ifndef _QEMU_VIRTIO_ACCESS_H
+#define _QEMU_VIRTIO_ACCESS_H
+#include "hw/virtio/virtio.h"
+#include "exec/address-spaces.h"
+
+static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
+{
+    if (vdev->is_big_endian) {
+        return lduw_be_phys(&address_space_memory, pa);
+    }
+    return lduw_le_phys(&address_space_memory, pa);
+}
+
+static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
+{
+    if (vdev->is_big_endian) {
+        return ldl_be_phys(&address_space_memory, pa);
+    }
+    return ldl_le_phys(&address_space_memory, pa);
+}
+
+static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
+{
+    if (vdev->is_big_endian) {
+        return ldq_be_phys(&address_space_memory, pa);
+    }
+    return ldq_le_phys(&address_space_memory, pa);
+}
+
+static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
+                                   uint16_t value)
+{
+    if (vdev->is_big_endian) {
+        stw_be_phys(&address_space_memory, pa, value);
+    } else {
+        stw_le_phys(&address_space_memory, pa, value);
+    }
+}
+
+static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
+                                   uint32_t value)
+{
+    if (vdev->is_big_endian) {
+        stl_be_phys(&address_space_memory, pa, value);
+    } else {
+        stl_le_phys(&address_space_memory, pa, value);
+    }
+}
+
+static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
+{
+    if (vdev->is_big_endian) {
+        stw_be_p(ptr, v);
+    } else {
+        stw_le_p(ptr, v);
+    }
+}
+
+static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
+{
+    if (vdev->is_big_endian) {
+        stl_be_p(ptr, v);
+    } else {
+        stl_le_p(ptr, v);
+    }
+}
+
+static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
+{
+    if (vdev->is_big_endian) {
+        stq_be_p(ptr, v);
+    } else {
+        stq_le_p(ptr, v);
+    }
+}
+
+static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
+{
+    if (vdev->is_big_endian) {
+        return lduw_be_p(ptr);
+    } else {
+        return lduw_le_p(ptr);
+    }
+}
+
+static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
+{
+    if (vdev->is_big_endian) {
+        return ldl_be_p(ptr);
+    } else {
+        return ldl_le_p(ptr);
+    }
+}
+
+static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
+{
+    if (vdev->is_big_endian) {
+        return ldq_be_p(ptr);
+    } else {
+        return ldq_le_p(ptr);
+    }
+}
+
+uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s);
+
+static inline void virtio_tswap16s(VirtIODevice *vdev, uint16_t *s)
+{
+    *s = virtio_tswap16(vdev, *s);
+}
+
+uint32_t virtio_tswap32(VirtIODevice *vdev, uint32_t s);
+
+static inline void virtio_tswap32s(VirtIODevice *vdev, uint32_t *s)
+{
+    *s = virtio_tswap32(vdev, *s);
+}
+
+uint64_t virtio_tswap64(VirtIODevice *vdev, uint64_t s);
+
+static inline void virtio_tswap64s(VirtIODevice *vdev, uint64_t *s)
+{
+    *s = virtio_tswap64(vdev, *s);
+}
+#endif /* _QEMU_VIRTIO_ACCESS_H */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 3e54e90..baab794 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -121,6 +121,7 @@ struct VirtIODevice
     bool vm_running;
     VMChangeStateEntry *vmstate;
     char *bus_name;
+    bool is_big_endian;
 };
 
 typedef struct VirtioDeviceClass {
@@ -253,4 +254,5 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue 
*vq, bool assign,
                                                bool set_handler);
 void virtio_queue_notify_vq(VirtQueue *vq);
 void virtio_irq(VirtQueue *vq);
+bool virtio_is_big_endian(void);
 #endif
diff --git a/vl.c b/vl.c
index 9975e5a..8918eb6 100644
--- a/vl.c
+++ b/vl.c
@@ -4379,6 +4379,10 @@ int main(int argc, char **argv, char **envp)
                                  .initrd_filename = initrd_filename,
                                  .cpu_model = cpu_model };
 
+    /* CAUTION: the virtio migration code assumes that cpus are initialized
+     * before generic devices. This means that machine->init() MUST be called
+     * before qemu_opts_foreach(...device_init_func...).
+     */
     current_machine->init_args = args;
     machine->init(&current_machine->init_args);