From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RkjmP-0006nD-NK for qemu-devel@nongnu.org; Tue, 10 Jan 2012 17:02:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RkjmN-0002I5-QX for qemu-devel@nongnu.org; Tue, 10 Jan 2012 17:02:53 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:40393) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RkjmN-0002HW-M5 for qemu-devel@nongnu.org; Tue, 10 Jan 2012 17:02:51 -0500 Received: by iagw33 with SMTP id w33so90882iag.4 for ; Tue, 10 Jan 2012 14:02:50 -0800 (PST) Message-ID: <4F0CB585.2000206@codemonkey.ws> Date: Tue, 10 Jan 2012 16:02:45 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1326195311.23910.59.camel@pasglop> In-Reply-To: <1326195311.23910.59.camel@pasglop> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-pci: Fix endianness of virtio config List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: kvm-ppc@vger.kernel.org, David Gibson , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , Alexander Graf On 01/10/2012 05:35 AM, Benjamin Herrenschmidt wrote: > The virtio config area in PIO space is a bit special. The initial > header is little endian but the rest (device specific) is guest > native endian. > > The PIO accessors for PCI on machines that don't have native IO ports > assume that all PIO is little endian, which works fine for everything > except the above. > > A complicated way to fix it would be to split the BAR into two memory > regions with different endianess settings, but this isn't practical > to do, besides, the PIO code doesn't honor region endianness anyway > (I have a patch for that too but it isn't necessary at this stage). > > So I decided to go for the quick fix instead which consists of > reverting the swap in virtio-pci in selected places, hoping that when > we eventually do a "v2" of the virtio protocols, we sort that out once > and for all using a fixed endian setting for everything. > > Unfortunately, that mean moving virtio-pci from Makefile.objs to > Makefile.target so we can use TARGET_WORDS_BIGENDIAN which would > otherwise be poisoned. > > Signed-off-by: Benjamin Herrenschmidt > --- > Makefile.objs | 1 - > Makefile.target | 1 + > hw/virtio-pci.c | 24 ++++++++++++++++++++++-- > 3 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index 4f6d26c..b721fca 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -186,7 +186,6 @@ hw-obj-y = > hw-obj-y += vl.o loader.o > hw-obj-$(CONFIG_VIRTIO) += virtio-console.o > hw-obj-y += usb-libhw.o > -hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o > hw-obj-y += fw_cfg.o > hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o > hw-obj-$(CONFIG_PCI) += msix.o msi.o > diff --git a/Makefile.target b/Makefile.target > index ef6834b..03d44c3 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -191,6 +191,7 @@ obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o > # need to fix this properly > obj-$(CONFIG_NO_PCI) += pci-stub.o > obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o > +obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o > obj-y += vhost_net.o > obj-$(CONFIG_VHOST_NET) += vhost.o > obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 77b75bc..ca70e42 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -412,20 +412,34 @@ static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr) > { > VirtIOPCIProxy *proxy = opaque; > uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); > + uint16_t val; > if (addr< config) > return virtio_ioport_read(proxy, addr); > addr -= config; > - return virtio_config_readw(proxy->vdev, addr); > + val = virtio_config_readw(proxy->vdev, addr); > +#ifdef TARGET_WORDS_BIGENDIAN > + /* virtio is odd, ioports are LE but config space is target native > + * endian. However, in qemu, all PIO is LE, so we need to re-swap > + * on BE targets > + */ > + val = bswap16(val); > +#endif I think this is the only reasonable way to do it, but I'd suggest adding target_is_bigendian() to arch_init.c that way we wouldn't have to move the object from libhw. Regards, Anthony Liguori > + return val; > } > > static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr) > { > VirtIOPCIProxy *proxy = opaque; > uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev); > + uint32_t val; > if (addr< config) > return virtio_ioport_read(proxy, addr); > addr -= config; > - return virtio_config_readl(proxy->vdev, addr); > + val = virtio_config_readl(proxy->vdev, addr); > +#ifdef TARGET_WORDS_BIGENDIAN > + val = bswap32(val); > +#endif > + return val; > } > > static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val) > @@ -449,6 +463,9 @@ static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val) > return; > } > addr -= config; > +#ifdef TARGET_WORDS_BIGENDIAN > + val = bswap16(val); > +#endif > virtio_config_writew(proxy->vdev, addr, val); > } > > @@ -461,6 +478,9 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val) > return; > } > addr -= config; > +#ifdef TARGET_WORDS_BIGENDIAN > + val = bswap32(val); > +#endif > virtio_config_writel(proxy->vdev, addr, val); > } > > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html