From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57135) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7csG-00058N-Q9 for qemu-devel@nongnu.org; Thu, 08 Aug 2013 22:56:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V7csC-00082v-9o for qemu-devel@nongnu.org; Thu, 08 Aug 2013 22:56:20 -0400 Received: from ozlabs.org ([203.10.76.45]:41187) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7csB-00081q-Vf for qemu-devel@nongnu.org; Thu, 08 Aug 2013 22:56:16 -0400 From: Rusty Russell In-Reply-To: <5203AB19.9070505@suse.de> References: <1375938949-22622-1-git-send-email-rusty@rustcorp.com.au> <1375938949-22622-2-git-send-email-rusty@rustcorp.com.au> <87li4cgvh1.fsf@codemonkey.ws> <5203AB19.9070505@suse.de> Date: Fri, 09 Aug 2013 09:38:07 +0930 Message-ID: <87zjsroheg.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/7] virtio: allow byte swapping for vring and config access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= , Anthony Liguori Cc: qemu-devel@nongnu.org Andreas F=C3=A4rber writes: > Am 08.08.2013 15:31, schrieb Anthony Liguori: >> Rusty Russell writes: >>=20 >>> Virtio is currently defined to work as "guest endian", but this is a >>> problem if the guest can change endian. As most targets can't change >>> endian, we make it a per-target option to avoid pessimising. >>> >>> This is based on a simpler patch by Anthony Liguouri, which only handled >>> the vring accesses. We also need some drivers to access these helpers, >>> eg. for data which contains headers. >>> >>> Signed-off-by: Rusty Russell >>> --- >>> hw/virtio/virtio.c | 46 +++++++++---- >>> include/hw/virtio/virtio-access.h | 138 ++++++++++++++++++++++++++++++= ++++++++ >>> 2 files changed, 170 insertions(+), 14 deletions(-) >>> create mode 100644 include/hw/virtio/virtio-access.h >>> >>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>> index 8176c14..2887f17 100644 >>> --- a/hw/virtio/virtio.c >>> +++ b/hw/virtio/virtio.c >>> @@ -18,6 +18,7 @@ >>> #include "hw/virtio/virtio.h" >>> #include "qemu/atomic.h" >>> #include "hw/virtio/virtio-bus.h" >>> +#include "hw/virtio/virtio-access.h" >>>=20=20 >>> /* The alignment to use between consumer and producer parts of vring. >>> * x86 pagesize again. */ >>> @@ -84,6 +85,20 @@ struct VirtQueue >>> EventNotifier host_notifier; >>> }; >>>=20=20 >>> +#ifdef TARGET_VIRTIO_SWAPENDIAN >>> +bool virtio_byteswap; >>> + >>> +/* Ask target code if we should swap endian for all vring and config a= ccess. */ >>> +static void mark_endian(void) >>> +{ >>> + virtio_byteswap =3D virtio_swap_endian(); >>> +} >>> +#else >>> +static void mark_endian(void) >>> +{ >>> +} >>> +#endif >>> + >>=20 >> It would be very good to avoid a target specific define here. We would >> like to move to only building a single copy of the virtio code. > > +1 > >> We have a mechanism to do weak functions via stubs/. I think it would >> be better to do cpu_get_byteswap() as a stub function and then overload >> it in the ppc64 code. > > If this as your name indicates is a per-CPU function then it should go > into CPUClass. Interesting question is, what is virtio supposed to do if > we have two ppc CPUs, one is Big Endian, the other is Little Endian. > We'd need to check current_cpu then, which for Xen is always NULL. This is why the check is performed on a random CPU when they first acknowledge the device. It's a hacky assumption, but that's why there's a proposal to nail virtio to LE for the 1.0 OASIS standard. You can't actually change endian of a virtio device in flight: it doesn't make sense since there's readable state there already. Cheers, Rusty.