From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWNpL-00041J-0j for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:32:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YWNpC-0008Ee-Km for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:32:26 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:52306) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWNpC-0008ER-At for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:32:18 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 13 Mar 2015 11:32:16 -0000 Date: Fri, 13 Mar 2015 12:32:12 +0100 From: Greg Kurz Message-ID: <20150313123212.481ce3f6@bahia.local> In-Reply-To: <5502C49E.2020908@redhat.com> References: <20150313090312.742d135c@bahia.local> <20150313081125.4669.92074.stgit@bahia.local> <5502C49E.2020908@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] memory: fix the eventfd data endianness according to the host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Michael Roth , Cedric Le Goater , "Michael S. Tsirkin" , qemu-devel@nongnu.org, qemu-stable@nongnu.org On Fri, 13 Mar 2015 12:06:06 +0100 Paolo Bonzini wrote: >=20 >=20 > On 13/03/2015 09:11, Greg Kurz wrote: > > The data argument is a host entity. It is not related to the target > > endianness. Let's introduce a HOST_WORDS_BIGENDIAN based helper for > > that. > >=20 > > This patch fixes ioeventfd and vhost for a ppc64le host running a ppc64= le > > guest (only virtqueue 0 was handled, all others being byteswapped becau= se > > of TARGET_WORDS_BIGENDIAN). It doesn't change functionnality for fixed > > endian architectures (i.e. doesn't break x86). > >=20 > > Reported-by: C=C3=A9dric Le Goater > > Signed-off-by: Greg Kurz > > --- > > memory.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > >=20 > > diff --git a/memory.c b/memory.c > > index 6291cc0..1e29d40 100644 > > --- a/memory.c > > +++ b/memory.c > > @@ -1549,6 +1549,15 @@ void memory_region_clear_flush_coalesced(MemoryR= egion *mr) > > } > > } > > =20 > > +static bool eventfd_wrong_endianness(MemoryRegion *mr) > > +{ > > +#ifdef HOST_WORDS_BIGENDIAN > > + return mr->ops->endianness =3D=3D DEVICE_LITTLE_ENDIAN; > > +#else > > + return mr->ops->endianness =3D=3D DEVICE_BIG_ENDIAN; > > +#endif > > +} > > + > > void memory_region_add_eventfd(MemoryRegion *mr, > > hwaddr addr, > > unsigned size, > > @@ -1565,7 +1574,7 @@ void memory_region_add_eventfd(MemoryRegion *mr, > > }; > > unsigned i; > > =20 > > - adjust_endianness(&mrfd.data, size, memory_region_wrong_endianness= (mr)); > > + adjust_endianness(&mrfd.data, size, eventfd_wrong_endianness(mr)); >=20 > Strictly speaking, the place to do this would be kvm_set_ioeventfd_mmio. FWIW the swap is being done in memory.c since commit: commit 28f362be6e7f45ea9b7a57a08555c4c784f36198 Author: Alexander Graf Date: Mon Oct 15 20:30:28 2012 +0200 memory: Make eventfd adhere to device endianness Are you asking to revert this commit and to pass the device endianness to kvm_set_ioeventfd_mmio() so it can fix the ordering ? > A hypothetical userspace ioeventfd emulation would not need the swap. >=20 I don't understand why "would not need the swap"... > I can accept the patch, but it's better to add a comment. >=20 ... and so I don't know what to write. :) Please enlight ! -- Greg > Paolo >=20 > > memory_region_transaction_begin(); > > for (i =3D 0; i < mr->ioeventfd_nb; ++i) { > > if (memory_region_ioeventfd_before(mrfd, mr->ioeventfds[i])) { > > @@ -1598,7 +1607,7 @@ void memory_region_del_eventfd(MemoryRegion *mr, > > }; > > unsigned i; > > =20 > > - adjust_endianness(&mrfd.data, size, memory_region_wrong_endianness= (mr)); > > + adjust_endianness(&mrfd.data, size, eventfd_wrong_endianness(mr)); > > memory_region_transaction_begin(); > > for (i =3D 0; i < mr->ioeventfd_nb; ++i) { > > if (memory_region_ioeventfd_equal(mrfd, mr->ioeventfds[i])) { > >=20 >=20