From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmGx-0007DC-8g for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:07:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFmGr-000271-Po for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:07:47 -0500 Received: from cantor2.suse.de ([195.135.220.15]:56357 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFmGr-00026q-IO for qemu-devel@nongnu.org; Tue, 18 Feb 2014 10:07:41 -0500 Message-ID: <5303773A.80702@suse.de> Date: Tue, 18 Feb 2014 16:07:38 +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> <530375FA.4070109@suse.de> <20140218151119.GB9504@redhat.com> In-Reply-To: <20140218151119.GB9504@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:11 PM, Michael S. Tsirkin wrote: > On Tue, Feb 18, 2014 at 04:02:18PM +0100, Alexander Graf wrote: >> 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 > 1.0 won't use the flag, it's all LE. At which point we need another flag indicating that the fields are always LE which means we can reuse this flag, no? Alex