From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47157) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmBn-00039P-2W for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:02:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFmBh-0008WT-Ls for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:02:27 -0500 Received: from cantor2.suse.de ([195.135.220.15]:56136 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmBh-0008WI-FH for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:02:21 -0500 Message-ID: <530375FA.4070109@suse.de> Date: Tue, 18 Feb 2014 16:02:18 +0100 From: Alexander Graf MIME-Version: 1.0 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> In-Reply-To: <20140218150327.GA9457@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: "Michael S. Tsirkin" Cc: kwolf@redhat.com, peter.maydell@linaro.org, thuth@linux.vnet.ibm.com, marc.zyngier@arm.com, rusty@rustcorp.com.au, qemu-devel@nongnu.org, stefanha@redhat.com, anthony@codemonkey.ws, pbonzini@redhat.com, afaerber@suse.de, Greg Kurz On 02/18/2014 04:03 PM, 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. Good point. It just feels like a heavy layering violation to have a global variable for all virtio devices. What if we start mixing legacy and 1.0 devices? We could reuse the same flag, but it would be initialized differently per device. Alex