From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57557) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmrI-0003IR-6S for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:45:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFmrA-0005sV-B3 for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:45:20 -0500 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:52222) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmrA-0005sN-2Q for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:45:12 -0500 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 18 Feb 2014 15:45:10 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id A024017D805A for ; Tue, 18 Feb 2014 15:45:37 +0000 (GMT) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s1IFiutw65863826 for ; Tue, 18 Feb 2014 15:44:56 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s1IGj77R018884 for ; Tue, 18 Feb 2014 09:45:08 -0700 Date: Tue, 18 Feb 2014 16:45:01 +0100 From: Cornelia Huck Message-ID: <20140218164501.7acd2f84.cornelia.huck@de.ibm.com> In-Reply-To: <20140218161208.2b174e13.cornelia.huck@de.ibm.com> References: <20140218123844.9849.58557.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <20140218123849.9849.77875.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <530372C6.70503@suse.de> <20140218150327.GA9457@redhat.com> <20140218161208.2b174e13.cornelia.huck@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, peter.maydell@linaro.org, thuth@linux.vnet.ibm.com, "Michael S. Tsirkin" , marc.zyngier@arm.com, rusty@rustcorp.com.au, Alexander Graf , stefanha@redhat.com, anthony@codemonkey.ws, pbonzini@redhat.com, afaerber@suse.de, Greg Kurz On Tue, 18 Feb 2014 16:12:08 +0100 Cornelia Huck wrote: > On Tue, 18 Feb 2014 17:03:27 +0200 > "Michael S. Tsirkin" wrote: > > > On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > > > On 02/18/2014 01:38 PM, Greg Kurz wrote: > > > >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. 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. > > > > > > > >[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > > > ldq_phys() API change, Greg Kurz ] > > > >Signed-off-by: Rusty Russell > > > >Signed-off-by: Greg Kurz > > > >--- > > > > hw/virtio/virtio.c | 6 ++ > > > > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > > > > include/hw/virtio/virtio.h | 2 + > > > > stubs/Makefile.objs | 1 > > > > stubs/virtio_get_byteswap.c | 6 ++ > > > > 5 files changed, 147 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; > > > > > > Could this be a virtio object property rather than a global? Imagine > > > an AMP guest system with a BE and an LE system running in parallel > > > accessing two separate virtio devices. With a single global that > > > would break. > > > > > > > > > Alex > > > > Well, how does a device know which CPU uses it? > > I suspect we are better off waiting for 1.0 with this one. > > > > 1.0 makes this a bit more complex, no? > > virtio-endian accessors are defined by the endianness of host and guest > (doing a bswap depends on the host/guest combination). This needs to be > per qemu instance. (ioctl under kvm? machine option?) > > For 1.0, we'll have everything le, so a be host will always do a bswap > (as will a be guest). But whether a device is 1.0 or legacy is not > something that can be decided globally, or we can't have transitional > devices with qemu. > So here are two stupid tables on who needs to do byteswaps, one for legacy devices, one for 1.0 devices: legacy devices: host be le g be host no host yes u guest no guest no e s le host yes host no t guest no guest no virtio 1.0 devices: host be le g be host yes host no u guest yes guest yes e s le host yes host no t guest no guest no This means byteswaps in qemu always depend on guest-endianness for legacy and on host-endianness for 1.0. If we want to support transitional devices with a mixture of legacy/1.0, we'll need both a per-machine and per-device swap flag: virtio_whatever(device, parameters...) { if (device->legacy) { if (guest_needs_byteswap) { whatever_byteswap(parameters...); } else { whatever(parameters...); } } else { /* 1.0 */ whatever_le(parameters...); } } Comments?