From: Rusty Russell <ru...@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, by making all little endian.
We need to support both implementations and we want to share as much code as possible. A good way to do it is to introduce a per-device boolean property to tell memory accessors whether they should swap bytes or not. This flag should be set at device reset time, because: - endianness won't change while the device is in use, and if we reboot into a different endianness, a new device reset will occur - as suggested by Alexander Graf, we can keep all the logic to set the property in a single place and share all the virtio memory accessors between the two implementations For legacy devices, we rely on a per-platform hook to set the flag. The virtio 1.0 implementation will just have to add some more logic in virtio_reset() instead of calling the hook: if (vdev->legacy) { vdev->needs_byteswap = virtio_legacy_get_byteswap(); } else { #ifdef HOST_WORDS_BIGENDIAN vdev->needs_byteswap = true; #else vdev->needs_byteswap = false; #endif } The needs_byteswap flag is preserved accross migrations. Signed-off-by: Rusty Russell <ru...@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, introduce a per-device needs_byteswap flag, add VirtIODevice * arg to virtio helpers, rename virtio_get_byteswap to virtio_legacy_get_byteswap, Greg Kurz <gk...@linux.vnet.ibm.com> ] Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- hw/virtio/virtio.c | 5 + include/hw/virtio/virtio-access.h | 139 +++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 3 + stubs/Makefile.objs | 1 stubs/virtio_get_byteswap.c | 6 ++ 5 files changed, 154 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..24b565f 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->needs_byteswap = virtio_legacy_get_byteswap(); + if (k->reset) { k->reset(vdev); } @@ -845,6 +848,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) qemu_put_8s(f, &vdev->status); qemu_put_8s(f, &vdev->isr); + qemu_put_8s(f, (uint8_t *) &vdev->needs_byteswap); qemu_put_be16s(f, &vdev->queue_sel); qemu_put_be32s(f, &vdev->guest_features); qemu_put_be32(f, vdev->config_len); @@ -905,6 +909,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) qemu_get_8s(f, &vdev->status); qemu_get_8s(f, &vdev->isr); + qemu_get_8s(f, (uint8_t *) &vdev->needs_byteswap); qemu_get_be16s(f, &vdev->queue_sel); qemu_get_be32s(f, &features); diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h new file mode 100644 index 0000000..70dd1e2 --- /dev/null +++ b/include/hw/virtio/virtio-access.h @@ -0,0 +1,139 @@ +/* + * Virtio Accessor Support: In case your target can change endian. + * + * Copyright IBM, Corp. 2013 + * + * Authors: + * Rusty Russell <ru...@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" + +static inline uint16_t virtio_lduw_phys(AddressSpace *as, hwaddr pa, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap16(lduw_phys(as, pa)); + } + return lduw_phys(as, pa); +} + +static inline uint32_t virtio_ldl_phys(AddressSpace *as, hwaddr pa, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap32(ldl_phys(as, pa)); + } + return ldl_phys(as, pa); +} + +static inline uint64_t virtio_ldq_phys(AddressSpace *as, hwaddr pa, + struct VirtIODevice *vdev) +{ + if (vdev->needs_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, + struct VirtIODevice *vdev) +{ + if (vdev->needs_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, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + stl_phys(as, pa, bswap32(value)); + } else { + stl_phys(as, pa, value); + } +} + +static inline void virtio_stw_p(void *ptr, uint16_t v, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + stw_p(ptr, bswap16(v)); + } else { + stw_p(ptr, v); + } +} + +static inline void virtio_stl_p(void *ptr, uint32_t v, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + stl_p(ptr, bswap32(v)); + } else { + stl_p(ptr, v); + } +} + +static inline void virtio_stq_p(void *ptr, uint64_t v, + struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + stq_p(ptr, bswap64(v)); + } else { + stq_p(ptr, v); + } +} + +static inline int virtio_lduw_p(const void *ptr, struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap16(lduw_p(ptr)); + } else { + return lduw_p(ptr); + } +} + +static inline int virtio_ldl_p(const void *ptr, struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap32(ldl_p(ptr)); + } else { + return ldl_p(ptr); + } +} + +static inline uint64_t virtio_ldq_p(const void *ptr, struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap64(ldq_p(ptr)); + } else { + return ldq_p(ptr); + } +} + +static inline uint32_t virtio_tswap32(uint32_t s, struct VirtIODevice *vdev) +{ + if (vdev->needs_byteswap) { + return bswap32(tswap32(s)); + } else { + return tswap32(s); + } +} + +static inline void virtio_tswap32s(uint32_t *s, struct VirtIODevice *vdev) +{ + tswap32s(s); + if (vdev->needs_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..3595da5 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 needs_byteswap; }; typedef struct VirtioDeviceClass { @@ -253,4 +254,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_legacy_get_byteswap(void); #endif diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 5ed1d38..a13381a 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -30,3 +30,4 @@ stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o stub-obj-y += cpus.o stub-obj-y += kvm.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..28af5e3 --- /dev/null +++ b/stubs/virtio_get_byteswap.c @@ -0,0 +1,6 @@ +#include "hw/virtio/virtio.h" + +bool virtio_legacy_get_byteswap(void) +{ + return false; +}