From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44351) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7ncK-0002xP-PQ for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:24:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V7ncF-00046s-5v for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:24:36 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34462 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7ncE-00046k-Si for qemu-devel@nongnu.org; Fri, 09 Aug 2013 10:24:31 -0400 Message-ID: <5204FB9B.9000809@suse.de> Date: Fri, 09 Aug 2013 16:24:27 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 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> <87li4bnyax.fsf@rustcorp.com.au> In-Reply-To: <87li4bnyax.fsf@rustcorp.com.au> 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: Rusty Russell Cc: Stefano Stabellini , qemu-devel@nongnu.org, Anthony Liguori , anton@samba.org Am 09.08.2013 09:00, schrieb Rusty Russell: > Andreas F=C3=A4rber writes: >> Am 08.08.2013 15:31, schrieb Anthony Liguori: >>> Rusty Russell writes: >>> We have a mechanism to do weak functions via stubs/. I think it woul= d >>> be better to do cpu_get_byteswap() as a stub function and then overlo= ad >>> 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. >=20 > Below is the minimal solution, which is sufficient for virtio. >=20 > If Anton wants per-cpu endianness for gdb, he'll need something more > sophisticated. >=20 > Feedback welcome! > Rusty. >=20 > Subject: cpu_get_byteswap: function for endian-ambivalent targets. >=20 > Signed-off-by: Rusty Russell >=20 > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index a5bb515..ed84267 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -357,4 +357,13 @@ void cpu_reset_interrupt(CPUState *cpu, int mask); > */ > void cpu_resume(CPUState *cpu); > =20 > +/** > + * cpu_get_byteswap: > + * > + * Is (any) CPU running in byteswapped mode: normally false. This > + * doesn't take a cpu argument, because we don't support heterogeneous > + * endianness. > + */ > +bool cpu_get_byteswap(void); > + > #endif > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > index 9b701b4..d4af94a 100644 > --- a/stubs/Makefile.objs > +++ b/stubs/Makefile.objs > @@ -25,3 +25,4 @@ stub-obj-y +=3D vm-stop.o > stub-obj-y +=3D vmstate.o > stub-obj-$(CONFIG_WIN32) +=3D fd-register.o > stub-obj-y +=3D cpus.o > +stub-obj-y +=3D cpu_byteswap.o > diff --git a/stubs/cpu_byteswap.c b/stubs/cpu_byteswap.c > new file mode 100644 > index 0000000..b3b669f > --- /dev/null > +++ b/stubs/cpu_byteswap.c > @@ -0,0 +1,6 @@ > +#include "qom/cpu.h" > + > +bool cpu_get_byteswap(void) > +{ > + return false; > +} That is exactly what I asked not to do. first_cpu_get_byteswap() or virtio_get_byteswap() etc. would be names OK for me. But see below. >=20 > Subject: target-ppc: ppc64 targets can be either endian. >=20 > In this case, we just query the first cpu. >=20 > Signed-off-by: Rusty Russell >=20 > diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c > index 616aab6..0a508eb 100644 > --- a/target-ppc/misc_helper.c > +++ b/target-ppc/misc_helper.c > @@ -116,3 +116,8 @@ void ppc_store_msr(CPUPPCState *env, target_ulong v= alue) > { > hreg_store_msr(env, value, 0); > } > + > +bool cpu_get_byteswap(void) > +{ > + return first_cpu->hflags & (1 << MSR_LE); > +} This assumes that first_cpu !=3D NULL, which I pointed out is not the cas= e for Xen. I'm not aware of a ppc Xen implementation, but it shows that you should define which endianness (probably little) you want to adopt in such a case. You should also urgently update your QEMU since that code will not build. first_cpu is CPUState since a number of weeks, you would need to cast to PowerPCCPU *cpu =3D POWERPC_CPU(first_cpu); in the non-NULL case. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg