From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTUTc-0005x3-O1 for qemu-devel@nongnu.org; Fri, 28 Mar 2014 06:57:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTUTV-0003Ms-2I for qemu-devel@nongnu.org; Fri, 28 Mar 2014 06:57:32 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:43540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTUTU-0003Mj-Me for qemu-devel@nongnu.org; Fri, 28 Mar 2014 06:57:24 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 28 Mar 2014 10:57:23 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id AF01217D8059 for ; Fri, 28 Mar 2014 10:58:08 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s2SAv9jw66781426 for ; Fri, 28 Mar 2014 10:57:09 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s2SAvKP7006838 for ; Fri, 28 Mar 2014 04:57:21 -0600 From: Greg Kurz Date: Fri, 28 Mar 2014 11:57:17 +0100 Message-ID: <20140328105717.21018.17649.stgit@bahia.local> In-Reply-To: <20140328105709.21018.88000.stgit@bahia.local> References: <20140328105709.21018.88000.stgit@bahia.local> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [PATCH v6 1/8] virtio_legacy_get_byteswap: endian-ambivalent targets using legacy virtio List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: afaerber@suse.de Cc: kwolf@redhat.com, peter.maydell@linaro.org, thuth@linux.vnet.ibm.com, mst@redhat.com, marc.zyngier@arm.com, rusty@rustcorp.com.au, agraf@suse.de, qemu-devel@nongnu.org, stefanha@redhat.com, cornelia.huck@de.ibm.com, pbonzini@redhat.com, anthony@codemonkey.ws From: Rusty Russell 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 [ 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 ] Signed-off-by: Greg Kurz --- 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 + * + * 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; +}