[Qemu-devel] [PATCH v5 0/8] virtio endian-ambivalent target fixes

[Qemu-devel] [PATCH v5 0/8] virtio endian-ambivalent target fixes

[Greg Kurz]

Hi,

This serie introduces helpers to enable virtio devices in a cross-endian
environment. As of today, we only have legacy virtio but hopefully this
helpers will be reused when we implement virtio 1.0.

Some assumptions are made for the legacy implementation:
- all guest cpus have the same endianness
- all virtio devices have the same endianness
- endianness does not change while the device is in use

The decision to byteswap or not is hence controlled by a global variable that
gets initialized on the virtio reset path for each device. This is slightly
suboptimal, but since reset is not a critical path, it is a viable solution.

Of course, this patchset needs some arch specific enablement to be fully
functionnal (PPC patches have already been posted to support KVM and TCG).

The changes since the last post are:
- fixed SoB lines and subjects, as suggested by Andreas
- fixed missing virtio_ in patch 2/8, spotted by Cornelia
- relicensed virtio-access.h to GPLv2+ in patch 1/8, as requested by Rusty

Thanks for your comments.

Best Regards.

--
Greg

---

Greg Kurz (1):
      virtio-9p: use virtio wrappers to access headers

Rusty Russell (7):
      virtio_get_byteswap: function for endian-ambivalent targets using virtio
      virtio: allow byte swapping for vring and config access
      virtio-net: use virtio wrappers to access headers
      virtio-balloon: use virtio wrappers to access page frame numbers
      virtio-blk: use virtio wrappers to access headers
      virtio-scsi: use virtio wrappers to access headers
      virtio-serial-bus: use virtio wrappers to access headers


 hw/9pfs/virtio-9p-device.c        |    3 +
 hw/block/virtio-blk.c             |   35 +++++-----
 hw/char/virtio-serial-bus.c       |   34 +++++----
 hw/net/virtio-net.c               |   15 ++--
 hw/scsi/virtio-scsi.c             |   33 +++++----
 hw/virtio/virtio-balloon.c        |    3 +
 hw/virtio/virtio.c                |   38 ++++++----
 include/hw/virtio/virtio-access.h |  134 +++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h        |    2 +
 stubs/Makefile.objs               |    1 
 stubs/virtio_get_byteswap.c       |    6 ++
 11 files changed, 230 insertions(+), 74 deletions(-)
 create mode 100644 include/hw/virtio/virtio-access.h
 create mode 100644 stubs/virtio_get_byteswap.c

[Qemu-devel] [PATCH v5 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

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.  Meanwhile, create a hook for
little endian ppc (and potentially ARM).  This is called at device
reset time (which is done before any driver is loaded) since it
may involve a system call to get the status when running under kvm.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
[ fixed checkpatch.pl error with the virtio_byteswap initialisation,
  ldq_phys() API change,
  relicensed virtio-access.h to GPLv2+ on Rusty's request,
  Greg Kurz <gkurz@linux.vnet.ibm.com> ]
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/virtio/virtio.c                |    6 ++
 include/hw/virtio/virtio-access.h |  134 +++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h        |    2 +
 stubs/Makefile.objs               |    1 
 stubs/virtio_get_byteswap.c       |    6 ++
 5 files changed, 149 insertions(+)
 create mode 100644 include/hw/virtio/virtio-access.h
 create mode 100644 stubs/virtio_get_byteswap.c

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aeabf3a..4fd6ac2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -19,6 +19,9 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/atomic.h"
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+
+bool virtio_byteswap;
 
 /*
  * The alignment to use between consumer and producer parts of vring.
@@ -546,6 +549,9 @@ void virtio_reset(void *opaque)
 
     virtio_set_status(vdev, 0);
 
+    /* We assume all devices are the same endian. */
+    virtio_byteswap = virtio_get_byteswap();
+
     if (k->reset) {
         k->reset(vdev);
     }
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
new file mode 100644
index 0000000..34e2fe7
--- /dev/null
+++ b/include/hw/virtio/virtio-access.h
@@ -0,0 +1,134 @@
+/*
+ * Virtio Accessor Support: In case your target can change endian.
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Rusty Russell   <rusty@au.ibm.com>
+ *
+ * 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"
+
+/* Initialized by virtio_get_byteswap() at any virtio device reset. */
+extern bool virtio_byteswap;
+
+static inline uint16_t virtio_lduw_phys(AddressSpace *as, hwaddr pa)
+{
+    if (virtio_byteswap) {
+        return bswap16(lduw_phys(as, pa));
+    }
+    return lduw_phys(as, pa);
+}
+
+static inline uint32_t virtio_ldl_phys(AddressSpace *as, hwaddr pa)
+{
+    if (virtio_byteswap) {
+        return bswap32(ldl_phys(as, pa));
+    }
+    return ldl_phys(as, pa);
+}
+
+static inline uint64_t virtio_ldq_phys(AddressSpace *as, hwaddr pa)
+{
+    if (virtio_byteswap) {
+        return bswap64(ldq_phys(as, pa));
+    }
+    return ldq_phys(as, pa);
+}
+
+static inline void virtio_stw_phys(AddressSpace *as, hwaddr pa, uint16_t value)
+{
+    if (virtio_byteswap) {
+        stw_phys(as, pa, bswap16(value));
+    } else {
+        stw_phys(as, pa, value);
+    }
+}
+
+static inline void virtio_stl_phys(AddressSpace *as, hwaddr pa, uint32_t value)
+{
+    if (virtio_byteswap) {
+        stl_phys(as, pa, bswap32(value));
+    } else {
+        stl_phys(as, pa, value);
+    }
+}
+
+static inline void virtio_stw_p(void *ptr, uint16_t v)
+{
+    if (virtio_byteswap) {
+        stw_p(ptr, bswap16(v));
+    } else {
+        stw_p(ptr, v);
+    }
+}
+
+static inline void virtio_stl_p(void *ptr, uint32_t v)
+{
+    if (virtio_byteswap) {
+        stl_p(ptr, bswap32(v));
+    } else {
+        stl_p(ptr, v);
+    }
+}
+
+static inline void virtio_stq_p(void *ptr, uint64_t v)
+{
+    if (virtio_byteswap) {
+        stq_p(ptr, bswap64(v));
+    } else {
+        stq_p(ptr, v);
+    }
+}
+
+static inline int virtio_lduw_p(const void *ptr)
+{
+    if (virtio_byteswap) {
+        return bswap16(lduw_p(ptr));
+    } else {
+        return lduw_p(ptr);
+    }
+}
+
+static inline int virtio_ldl_p(const void *ptr)
+{
+    if (virtio_byteswap) {
+        return bswap32(ldl_p(ptr));
+    } else {
+        return ldl_p(ptr);
+    }
+}
+
+static inline uint64_t virtio_ldq_p(const void *ptr)
+{
+    if (virtio_byteswap) {
+        return bswap64(ldq_p(ptr));
+    } else {
+        return ldq_p(ptr);
+    }
+}
+
+static inline uint32_t virtio_tswap32(uint32_t s)
+{
+    if (virtio_byteswap) {
+        return bswap32(tswap32(s));
+    } else {
+        return tswap32(s);
+    }
+}
+
+static inline void virtio_tswap32s(uint32_t *s)
+{
+    tswap32s(s);
+    if (virtio_byteswap) {
+        *s = bswap32(*s);
+    }
+}
+#endif /* _QEMU_VIRTIO_ACCESS_H */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 3e54e90..5009945 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -253,4 +253,6 @@ 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);
+
+extern bool virtio_get_byteswap(void);
 #endif
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index df92fe5..7e7a9c8 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -27,3 +27,4 @@ stub-obj-y += vm-stop.o
 stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
+stub-obj-y += virtio_get_byteswap.o
diff --git a/stubs/virtio_get_byteswap.c b/stubs/virtio_get_byteswap.c
new file mode 100644
index 0000000..7cf764d
--- /dev/null
+++ b/stubs/virtio_get_byteswap.c
@@ -0,0 +1,6 @@
+#include "hw/virtio/virtio.h"
+
+bool virtio_get_byteswap(void)
+{
+    return false;
+}

[Qemu-devel] [PATCH v5 2/8] virtio: allow byte swapping for vring and config access

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

This is based on a simpler patch by Anthony Liguouri, which only handled
the vring accesses.  We also need some drivers to access these helpers,
eg. for data which contains headers.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
[ ldq_phys() API change,
  fixed missing virtio_ in vring_used_flags_unset_bit(),
  Greg Kurz <gkurz@linux.vnet.ibm.com> ]
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/virtio/virtio.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4fd6ac2..cecf9fc 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -108,49 +108,49 @@ static inline uint64_t vring_desc_addr(hwaddr desc_pa, int i)
 {
     hwaddr pa;
     pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, addr);
-    return ldq_phys(&address_space_memory, pa);
+    return virtio_ldq_phys(&address_space_memory, pa);
 }
 
 static inline uint32_t vring_desc_len(hwaddr desc_pa, int i)
 {
     hwaddr pa;
     pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, len);
-    return ldl_phys(&address_space_memory, pa);
+    return virtio_ldl_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_desc_flags(hwaddr desc_pa, int i)
 {
     hwaddr pa;
     pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, flags);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_desc_next(hwaddr desc_pa, int i)
 {
     hwaddr pa;
     pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, next);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_avail_flags(VirtQueue *vq)
 {
     hwaddr pa;
     pa = vq->vring.avail + offsetof(VRingAvail, flags);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_avail_idx(VirtQueue *vq)
 {
     hwaddr pa;
     pa = vq->vring.avail + offsetof(VRingAvail, idx);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
 {
     hwaddr pa;
     pa = vq->vring.avail + offsetof(VRingAvail, ring[i]);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline uint16_t vring_used_event(VirtQueue *vq)
@@ -162,44 +162,44 @@ static inline void vring_used_ring_id(VirtQueue *vq, int i, uint32_t val)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, ring[i].id);
-    stl_phys(&address_space_memory, pa, val);
+    virtio_stl_phys(&address_space_memory, pa, val);
 }
 
 static inline void vring_used_ring_len(VirtQueue *vq, int i, uint32_t val)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, ring[i].len);
-    stl_phys(&address_space_memory, pa, val);
+    virtio_stl_phys(&address_space_memory, pa, val);
 }
 
 static uint16_t vring_used_idx(VirtQueue *vq)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, idx);
-    return lduw_phys(&address_space_memory, pa);
+    return virtio_lduw_phys(&address_space_memory, pa);
 }
 
 static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, idx);
-    stw_phys(&address_space_memory, pa, val);
+    virtio_stw_phys(&address_space_memory, pa, val);
 }
 
 static inline void vring_used_flags_set_bit(VirtQueue *vq, int mask)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, flags);
-    stw_phys(&address_space_memory,
-             pa, lduw_phys(&address_space_memory, pa) | mask);
+    virtio_stw_phys(&address_space_memory,
+             pa, virtio_lduw_phys(&address_space_memory, pa) | mask);
 }
 
 static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask)
 {
     hwaddr pa;
     pa = vq->vring.used + offsetof(VRingUsed, flags);
-    stw_phys(&address_space_memory,
-             pa, lduw_phys(&address_space_memory, pa) & ~mask);
+    virtio_stw_phys(&address_space_memory,
+                    pa, virtio_lduw_phys(&address_space_memory, pa) & ~mask);
 }
 
 static inline void vring_avail_event(VirtQueue *vq, uint16_t val)
@@ -209,7 +209,7 @@ static inline void vring_avail_event(VirtQueue *vq, uint16_t val)
         return;
     }
     pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
-    stw_phys(&address_space_memory, pa, val);
+    virtio_stw_phys(&address_space_memory, pa, val);
 }
 
 void virtio_queue_set_notification(VirtQueue *vq, int enable)

[Qemu-devel] [PATCH v5 3/8] virtio-net: use virtio wrappers to access headers

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/net/virtio-net.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3626608..34d6b48 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/qmp/qjson.h"
 #include "monitor/monitor.h"
+#include "hw/virtio/virtio-access.h"
 
 #define VIRTIO_NET_VM_VERSION    11
 
@@ -72,8 +73,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg;
 
-    stw_p(&netcfg.status, n->status);
-    stw_p(&netcfg.max_virtqueue_pairs, n->max_queues);
+    virtio_stw_p(&netcfg.status, n->status);
+    virtio_stw_p(&netcfg.max_virtqueue_pairs, n->max_queues);
     memcpy(netcfg.mac, n->mac, ETH_ALEN);
     memcpy(config, &netcfg, n->config_size);
 }
@@ -618,7 +619,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 
     s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
                    sizeof(mac_data.entries));
-    mac_data.entries = ldl_p(&mac_data.entries);
+    mac_data.entries = virtio_ldl_p(&mac_data.entries);
     if (s != sizeof(mac_data.entries)) {
         goto error;
     }
@@ -645,7 +646,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 
     s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
                    sizeof(mac_data.entries));
-    mac_data.entries = ldl_p(&mac_data.entries);
+    mac_data.entries = virtio_ldl_p(&mac_data.entries);
     if (s != sizeof(mac_data.entries)) {
         goto error;
     }
@@ -690,7 +691,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
     NetClientState *nc = qemu_get_queue(n->nic);
 
     s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
-    vid = lduw_p(&vid);
+    vid = virtio_lduw_p(&vid);
     if (s != sizeof(vid)) {
         return VIRTIO_NET_ERR;
     }
@@ -727,7 +728,7 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
         return VIRTIO_NET_ERR;
     }
 
-    queues = lduw_p(&mq.virtqueue_pairs);
+    queues = virtio_lduw_p(&mq.virtqueue_pairs);
 
     if (queues < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
         queues > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
@@ -1026,7 +1027,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
     }
 
     if (mhdr_cnt) {
-        stw_p(&mhdr.num_buffers, i);
+        virtio_stw_p(&mhdr.num_buffers, i);
         iov_from_buf(mhdr_sg, mhdr_cnt,
                      0,
                      &mhdr.num_buffers, sizeof mhdr.num_buffers);

[Qemu-devel] [PATCH v5 4/8] virtio-balloon: use virtio wrappers to access page frame numbers

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/virtio/virtio-balloon.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a470a0b..5e7f26f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -30,6 +30,7 @@
 #endif
 
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
 
 static void balloon_page(void *addr, int deflate)
 {
@@ -192,7 +193,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
             ram_addr_t pa;
             ram_addr_t addr;
 
-            pa = (ram_addr_t)ldl_p(&pfn) << VIRTIO_BALLOON_PFN_SHIFT;
+            pa = (ram_addr_t)virtio_ldl_p(&pfn) << VIRTIO_BALLOON_PFN_SHIFT;
             offset += 4;
 
             /* FIXME: remove get_system_memory(), but how? */

[Qemu-devel] [PATCH v5 5/8] virtio-blk: use virtio wrappers to access headers

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/block/virtio-blk.c |   35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8a568e5..f55f89c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -26,6 +26,7 @@
 # include <scsi/sg.h>
 #endif
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
 
 typedef struct VirtIOBlockReq
 {
@@ -77,7 +78,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
     trace_virtio_blk_rw_complete(req, ret);
 
     if (ret) {
-        bool is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT);
+        bool is_read = !(virtio_ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT);
         if (virtio_blk_handle_rw_error(req, -ret, is_read))
             return;
     }
@@ -224,12 +225,12 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
         hdr.status = CHECK_CONDITION;
     }
 
-    stl_p(&req->scsi->errors,
-          hdr.status | (hdr.msg_status << 8) |
-          (hdr.host_status << 16) | (hdr.driver_status << 24));
-    stl_p(&req->scsi->residual, hdr.resid);
-    stl_p(&req->scsi->sense_len, hdr.sb_len_wr);
-    stl_p(&req->scsi->data_len, hdr.dxfer_len);
+    virtio_stl_p(&req->scsi->errors,
+                 hdr.status | (hdr.msg_status << 8) |
+                 (hdr.host_status << 16) | (hdr.driver_status << 24));
+    virtio_stl_p(&req->scsi->residual, hdr.resid);
+    virtio_stl_p(&req->scsi->sense_len, hdr.sb_len_wr);
+    virtio_stl_p(&req->scsi->data_len, hdr.dxfer_len);
 
     virtio_blk_req_complete(req, status);
     g_free(req);
@@ -240,7 +241,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 
 fail:
     /* Just put anything nonzero so that the ioctl fails in the guest.  */
-    stl_p(&req->scsi->errors, 255);
+    virtio_stl_p(&req->scsi->errors, 255);
     virtio_blk_req_complete(req, status);
     g_free(req);
 }
@@ -286,7 +287,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
     BlockRequest *blkreq;
     uint64_t sector;
 
-    sector = ldq_p(&req->out->sector);
+    sector = virtio_ldq_p(&req->out->sector);
 
     bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE);
 
@@ -320,7 +321,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
 {
     uint64_t sector;
 
-    sector = ldq_p(&req->out->sector);
+    sector = virtio_ldq_p(&req->out->sector);
 
     bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ);
 
@@ -358,7 +359,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
     req->out = (void *)req->elem.out_sg[0].iov_base;
     req->in = (void *)req->elem.in_sg[req->elem.in_num - 1].iov_base;
 
-    type = ldl_p(&req->out->type);
+    type = virtio_ldl_p(&req->out->type);
 
     if (type & VIRTIO_BLK_T_FLUSH) {
         virtio_blk_handle_flush(req, mrb);
@@ -487,12 +488,12 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 
     bdrv_get_geometry(s->bs, &capacity);
     memset(&blkcfg, 0, sizeof(blkcfg));
-    stq_raw(&blkcfg.capacity, capacity);
-    stl_raw(&blkcfg.seg_max, 128 - 2);
-    stw_raw(&blkcfg.cylinders, s->conf->cyls);
-    stl_raw(&blkcfg.blk_size, blk_size);
-    stw_raw(&blkcfg.min_io_size, s->conf->min_io_size / blk_size);
-    stw_raw(&blkcfg.opt_io_size, s->conf->opt_io_size / blk_size);
+    virtio_stq_p(&blkcfg.capacity, capacity);
+    virtio_stl_p(&blkcfg.seg_max, 128 - 2);
+    virtio_stw_p(&blkcfg.cylinders, s->conf->cyls);
+    virtio_stl_p(&blkcfg.blk_size, blk_size);
+    virtio_stw_p(&blkcfg.min_io_size, s->conf->min_io_size / blk_size);
+    virtio_stw_p(&blkcfg.opt_io_size, s->conf->opt_io_size / blk_size);
     blkcfg.heads = s->conf->heads;
     /*
      * We must ensure that the block device capacity is a multiple of

[Qemu-devel] [PATCH v5 6/8] virtio-scsi: use virtio wrappers to access headers

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/scsi/virtio-scsi.c |   33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 6610b3a..df6c0e2 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -18,6 +18,7 @@
 #include <hw/scsi/scsi.h>
 #include <block/scsi.h>
 #include <hw/virtio/virtio-bus.h>
+#include "hw/virtio/virtio-access.h"
 
 typedef struct VirtIOSCSIReq {
     VirtIOSCSI *dev;
@@ -313,12 +314,12 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
     req->resp.cmd->response = VIRTIO_SCSI_S_OK;
     req->resp.cmd->status = status;
     if (req->resp.cmd->status == GOOD) {
-        req->resp.cmd->resid = tswap32(resid);
+        req->resp.cmd->resid = virtio_tswap32(resid);
     } else {
         req->resp.cmd->resid = 0;
         sense_len = scsi_req_get_sense(r, req->resp.cmd->sense,
                                        VIRTIO_SCSI_SENSE_SIZE);
-        req->resp.cmd->sense_len = tswap32(sense_len);
+        req->resp.cmd->sense_len = virtio_tswap32(sense_len);
     }
     virtio_scsi_complete_req(req);
 }
@@ -414,16 +415,16 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
     VirtIOSCSIConfig *scsiconf = (VirtIOSCSIConfig *)config;
     VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev);
 
-    stl_raw(&scsiconf->num_queues, s->conf.num_queues);
-    stl_raw(&scsiconf->seg_max, 128 - 2);
-    stl_raw(&scsiconf->max_sectors, s->conf.max_sectors);
-    stl_raw(&scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
-    stl_raw(&scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
-    stl_raw(&scsiconf->sense_size, s->sense_size);
-    stl_raw(&scsiconf->cdb_size, s->cdb_size);
-    stw_raw(&scsiconf->max_channel, VIRTIO_SCSI_MAX_CHANNEL);
-    stw_raw(&scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
-    stl_raw(&scsiconf->max_lun, VIRTIO_SCSI_MAX_LUN);
+    virtio_stl_p(&scsiconf->num_queues, s->conf.num_queues);
+    virtio_stl_p(&scsiconf->seg_max, 128 - 2);
+    virtio_stl_p(&scsiconf->max_sectors, s->conf.max_sectors);
+    virtio_stl_p(&scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
+    virtio_stl_p(&scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
+    virtio_stl_p(&scsiconf->sense_size, s->sense_size);
+    virtio_stl_p(&scsiconf->cdb_size, s->cdb_size);
+    virtio_stw_p(&scsiconf->max_channel, VIRTIO_SCSI_MAX_CHANNEL);
+    virtio_stw_p(&scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
+    virtio_stl_p(&scsiconf->max_lun, VIRTIO_SCSI_MAX_LUN);
 }
 
 static void virtio_scsi_set_config(VirtIODevice *vdev,
@@ -432,14 +433,14 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
     VirtIOSCSIConfig *scsiconf = (VirtIOSCSIConfig *)config;
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
-    if ((uint32_t) ldl_raw(&scsiconf->sense_size) >= 65536 ||
-        (uint32_t) ldl_raw(&scsiconf->cdb_size) >= 256) {
+    if ((uint32_t) virtio_ldl_p(&scsiconf->sense_size) >= 65536 ||
+        (uint32_t) virtio_ldl_p(&scsiconf->cdb_size) >= 256) {
         error_report("bad data written to virtio-scsi configuration space");
         exit(1);
     }
 
-    vs->sense_size = ldl_raw(&scsiconf->sense_size);
-    vs->cdb_size = ldl_raw(&scsiconf->cdb_size);
+    vs->sense_size = virtio_ldl_p(&scsiconf->sense_size);
+    vs->cdb_size = virtio_ldl_p(&scsiconf->cdb_size);
 }
 
 static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,

[Qemu-devel] [PATCH v5 7/8] virtio-serial-bus: use virtio wrappers to access headers

[Greg Kurz]

From: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/char/virtio-serial-bus.c |   34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 226e9f9..243bf31 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -24,6 +24,7 @@
 #include "hw/sysbus.h"
 #include "trace.h"
 #include "hw/virtio/virtio-serial.h"
+#include "hw/virtio/virtio-access.h"
 
 static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
 {
@@ -185,9 +186,9 @@ static size_t send_control_event(VirtIOSerial *vser, uint32_t port_id,
 {
     struct virtio_console_control cpkt;
 
-    stl_p(&cpkt.id, port_id);
-    stw_p(&cpkt.event, event);
-    stw_p(&cpkt.value, value);
+    virtio_stl_p(&cpkt.id, port_id);
+    virtio_stw_p(&cpkt.event, event);
+    virtio_stw_p(&cpkt.value, value);
 
     trace_virtio_serial_send_control_event(port_id, event, value);
     return send_control_msg(vser, &cpkt, sizeof(cpkt));
@@ -291,8 +292,8 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
         return;
     }
 
-    cpkt.event = lduw_p(&gcpkt->event);
-    cpkt.value = lduw_p(&gcpkt->value);
+    cpkt.event = virtio_lduw_p(&gcpkt->event);
+    cpkt.value = virtio_lduw_p(&gcpkt->value);
 
     trace_virtio_serial_handle_control_message(cpkt.event, cpkt.value);
 
@@ -312,10 +313,10 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
         return;
     }
 
-    port = find_port_by_id(vser, ldl_p(&gcpkt->id));
+    port = find_port_by_id(vser, virtio_ldl_p(&gcpkt->id));
     if (!port) {
         error_report("virtio-serial-bus: Unexpected port id %u for device %s",
-                     ldl_p(&gcpkt->id), vser->bus.qbus.name);
+                     virtio_ldl_p(&gcpkt->id), vser->bus.qbus.name);
         return;
     }
 
@@ -342,9 +343,9 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
         }
 
         if (port->name) {
-            stl_p(&cpkt.id, port->id);
-            stw_p(&cpkt.event, VIRTIO_CONSOLE_PORT_NAME);
-            stw_p(&cpkt.value, 1);
+            virtio_stl_p(&cpkt.id, port->id);
+            virtio_stw_p(&cpkt.event, VIRTIO_CONSOLE_PORT_NAME);
+            virtio_stw_p(&cpkt.value, 1);
 
             buffer_len = sizeof(cpkt) + strlen(port->name) + 1;
             buffer = g_malloc(buffer_len);
@@ -536,7 +537,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
     qemu_put_be32s(f, &s->config.max_nr_ports);
 
     /* The ports map */
-    max_nr_ports = tswap32(s->config.max_nr_ports);
+    max_nr_ports = virtio_tswap32(s->config.max_nr_ports);
     for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
         qemu_put_be32s(f, &s->ports_map[i]);
     }
@@ -690,8 +691,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     qemu_get_be16s(f, &s->config.rows);
 
     qemu_get_be32s(f, &max_nr_ports);
-    tswap32s(&max_nr_ports);
-    if (max_nr_ports > tswap32(s->config.max_nr_ports)) {
+    virtio_tswap32s(&max_nr_ports);
+    if (max_nr_ports > virtio_tswap32(s->config.max_nr_ports)) {
         /* Source could have had more ports than us. Fail migration. */
         return -EINVAL;
     }
@@ -760,7 +761,7 @@ static uint32_t find_free_port_id(VirtIOSerial *vser)
 {
     unsigned int i, max_nr_ports;
 
-    max_nr_ports = tswap32(vser->config.max_nr_ports);
+    max_nr_ports = virtio_tswap32(vser->config.max_nr_ports);
     for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
         uint32_t map, bit;
 
@@ -846,7 +847,7 @@ static int virtser_port_qdev_init(DeviceState *qdev)
         }
     }
 
-    max_nr_ports = tswap32(port->vser->config.max_nr_ports);
+    max_nr_ports = virtio_tswap32(port->vser->config.max_nr_ports);
     if (port->id >= max_nr_ports) {
         error_report("virtio-serial-bus: Out-of-range port id specified, max. allowed: %u",
                      max_nr_ports - 1);
@@ -949,7 +950,8 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
         vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
     }
 
-    vser->config.max_nr_ports = tswap32(vser->serial.max_virtserial_ports);
+    vser->config.max_nr_ports =
+        virtio_tswap32(vser->serial.max_virtserial_ports);
     vser->ports_map = g_malloc0(((vser->serial.max_virtserial_ports + 31) / 32)
         * sizeof(vser->ports_map[0]));
     /*

[Qemu-devel] [PATCH v5 8/8] virtio-9p: use virtio wrappers to access headers

[Greg Kurz]

Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-device.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 15a4983..b3161ec 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -19,6 +19,7 @@
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-xattr.h"
 #include "virtio-9p-coth.h"
+#include "hw/virtio/virtio-access.h"
 
 static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
 {
@@ -34,7 +35,7 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config)
 
     len = strlen(s->tag);
     cfg = g_malloc0(sizeof(struct virtio_9p_config) + len);
-    stw_raw(&cfg->tag_len, len);
+    virtio_stw_p(&cfg->tag_len, len);
     /* We don't copy the terminating null to config space */
     memcpy(cfg->tag, s->tag, len);
     memcpy(config, cfg, s->config_size);

Re: [Qemu-devel] [PATCH v5 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio

[Stefan Hajnoczi]

On Fri, Feb 21, 2014 at 12:28:11PM +0100, Greg Kurz wrote:
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index aeabf3a..4fd6ac2 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -19,6 +19,9 @@
>  #include "hw/virtio/virtio.h"
>  #include "qemu/atomic.h"
>  #include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-access.h"
> +
> +bool virtio_byteswap;
>  
>  /*
>   * The alignment to use between consumer and producer parts of vring.
> @@ -546,6 +549,9 @@ void virtio_reset(void *opaque)
>  
>      virtio_set_status(vdev, 0);
>  
> +    /* We assume all devices are the same endian. */
> +    virtio_byteswap = virtio_get_byteswap();
> +
>      if (k->reset) {
>          k->reset(vdev);
>      }

In the previous version it was pointed out that the global is nasty.

A cleaner solution is a per-VirtIODevice enum device_endian value which
gets passed to lduw_phys_internal()/stl_phys_internal()/etc.

Now the same code can be used for:
1. Host == guest-endian
2. Host != guest-endian
3. "Everything is little-endian" (VIRTIO 1.0)

Keeping it per-device means we can have both legacy and VIRTIO 1.0
devices in a guest.

That's better than hacking in a global now and having to undo it when
VIRTIO 1.0 support gets merged.

Stefan

Re: [Qemu-devel] [PATCH v5 0/8] virtio endian-ambivalent target fixes

[Stefan Hajnoczi]

On Fri, Feb 21, 2014 at 12:28:03PM +0100, Greg Kurz wrote:
> This serie introduces helpers to enable virtio devices in a cross-endian
> environment. As of today, we only have legacy virtio but hopefully this
> helpers will be reused when we implement virtio 1.0.
> 
> Some assumptions are made for the legacy implementation:
> - all guest cpus have the same endianness
> - all virtio devices have the same endianness
> - endianness does not change while the device is in use
> 
> The decision to byteswap or not is hence controlled by a global variable that
> gets initialized on the virtio reset path for each device. This is slightly
> suboptimal, but since reset is not a critical path, it is a viable solution.
> 
> Of course, this patchset needs some arch specific enablement to be fully
> functionnal (PPC patches have already been posted to support KVM and TCG).
> 
> The changes since the last post are:
> - fixed SoB lines and subjects, as suggested by Andreas
> - fixed missing virtio_ in patch 2/8, spotted by Cornelia
> - relicensed virtio-access.h to GPLv2+ in patch 1/8, as requested by Rusty

The device conversions look fine but I think we should avoid using a
global bool.  I replied in more detail to Patch 1.

Re: [Qemu-devel] [PATCH v5 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio

[Greg Kurz]

On Tue, 25 Feb 2014 16:21:21 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Fri, Feb 21, 2014 at 12:28:11PM +0100, Greg Kurz wrote:
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index aeabf3a..4fd6ac2 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -19,6 +19,9 @@
> >  #include "hw/virtio/virtio.h"
> >  #include "qemu/atomic.h"
> >  #include "hw/virtio/virtio-bus.h"
> > +#include "hw/virtio/virtio-access.h"
> > +
> > +bool virtio_byteswap;
> >  
> >  /*
> >   * The alignment to use between consumer and producer parts of vring.
> > @@ -546,6 +549,9 @@ void virtio_reset(void *opaque)
> >  
> >      virtio_set_status(vdev, 0);
> >  
> > +    /* We assume all devices are the same endian. */
> > +    virtio_byteswap = virtio_get_byteswap();
> > +
> >      if (k->reset) {
> >          k->reset(vdev);
> >      }
> 
> In the previous version it was pointed out that the global is nasty.
> 
> A cleaner solution is a per-VirtIODevice enum device_endian value which
> gets passed to lduw_phys_internal()/stl_phys_internal()/etc.
> 
> Now the same code can be used for:
> 1. Host == guest-endian
> 2. Host != guest-endian
> 3. "Everything is little-endian" (VIRTIO 1.0)
> 
> Keeping it per-device means we can have both legacy and VIRTIO 1.0
> devices in a guest.
> 
> That's better than hacking in a global now and having to undo it when
> VIRTIO 1.0 support gets merged.
> 
> Stefan
> 

Stefan,

I was unsure whether people wanted that global (nasty indeed) to be kicked
out of the picture right now or later. Things are clearer now. :)
I will come back with something more appropriate.

Thanks for your time.

Regards.

-- 
Gregory Kurz                                     kurzgreg@fr.ibm.com
                                                 gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.