From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49238) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVxGS-00020Z-Ne for qemu-devel@nongnu.org; Thu, 12 Mar 2015 03:10:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVxGR-0003N1-O8 for qemu-devel@nongnu.org; Thu, 12 Mar 2015 03:10:40 -0400 Date: Thu, 12 Mar 2015 08:10:16 +0100 From: "Michael S. Tsirkin" Message-ID: <20150312080833-mutt-send-email-mst@redhat.com> References: <20150311180212.8119.14734.stgit@bahia.local> <20150311210322-mutt-send-email-mst@redhat.com> <20150311230314.08e51843@bahia.local> <1426112318.17565.60.camel@kernel.crashing.org> <20150311235213.6967e39c@bahia.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150311235213.6967e39c@bahia.local> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] virtio-pci: fix host notifiers on bi-endian architectures List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: Cedric Le Goater , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org On Wed, Mar 11, 2015 at 11:52:13PM +0100, Greg Kurz wrote: > On Thu, 12 Mar 2015 09:18:38 +1100 > Benjamin Herrenschmidt wrote: > > > On Wed, 2015-03-11 at 23:03 +0100, Greg Kurz wrote: > > > > > /* The host notifier will be swapped in adjust_endianness() according to the > > > * target default endianness. We need to negate this swap if the device uses > > > * an endianness that is not the default (ppc64le for example). > > > */ > > > > > > > > +static uint16_t cpu_to_host_notifier16(VirtIODevice *vdev, uint16_t val) > > > > > +{ > > > > > + return virtio_is_big_endian(vdev) ? val : bswap16(val); > > > > > +} > > > > But but but ... The above ... won't it do things like break x86 ? Ie, > > shouldn't we swap only if TARGET_BIG_ENDIAN and !virtio_is_big_endian ? > > Or better, "fixed target endian" ^ "virtio endian" to cover all cases ? > > > > Yeah you're right, it's a mess :) > > To avoid virtio-pci.o being built per target, we can use virtio_default_endian() > instead (to be exported from virtio.c): > > return vdev->device_endian() != virtio_default_endian() ? val : bswap16(val); > > I shall test on x86 and post a v2. > > Thanks. > > -- > G It's a mess because the issue is not in virtio I think. virtio common region is just a simple device, using fixed LE format. It's some bug in ioeventfd support. Need to understand whether it's in kernel (and maybe do a work-around) or in qemu memory core. > > Cheers, > > Ben. > > > > > > > static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy, > > > > > int n, bool assign, bool set_handler) > > > > > { > > > > > @@ -150,10 +155,12 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy, > > > > > } > > > > > virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler); > > > > > memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2, > > > > > - true, n, notifier); > > > > > + true, cpu_to_host_notifier16(vdev, n), > > > > > + notifier); > > > > > } else { > > > > > memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2, > > > > > - true, n, notifier); > > > > > + true, cpu_to_host_notifier16(vdev, n), > > > > > + notifier); > > > > > virtio_queue_set_host_notifier_fd_handler(vq, false, false); > > > > > event_notifier_cleanup(notifier); > > > > > } > > > > > > > > > > >