From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rklj9-0006BM-Me for qemu-devel@nongnu.org; Tue, 10 Jan 2012 19:07:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rklj6-0006iu-Gl for qemu-devel@nongnu.org; Tue, 10 Jan 2012 19:07:39 -0500 Message-ID: <4F0CD2C2.6040909@codemonkey.ws> Date: Tue, 10 Jan 2012 18:07:30 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1326240442-22915-1-git-send-email-agraf@suse.de> In-Reply-To: <1326240442-22915-1-git-send-email-agraf@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] virtio-pci: Fix endianness of virtio config List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-ppc@nongnu.org, "qemu-devel@nongnu.org Developers" On 01/10/2012 06:07 PM, Alexander Graf wrote: > From: Benjamin Herrenschmidt > > 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 > Signed-off-by: Alexander Graf > [agraf: keep virtio in libhw and determine endianness through a > helper function in exec.c] Both: Reviewed-by: Anthony Liguori Regards, Anthony Liguori > --- > exec.c | 14 ++++++++++++++ > hw/virtio-pci.c | 28 ++++++++++++++++++++++++++-- > 2 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index b1d6602..319e867 100644 > --- a/exec.c > +++ b/exec.c > @@ -4390,6 +4390,20 @@ tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong addr) > return qemu_ram_addr_from_host_nofail(p); > } > > +/* > + * A helper function for the _utterly broken_ virtio device model to find out if > + * it's running on a big endian machine. Don't do this at home kids! > + */ > +bool virtio_is_big_endian(void); > +bool virtio_is_big_endian(void) > +{ > +#if defined(TARGET_WORDS_BIGENDIAN) > + return true; > +#else > + return false; > +#endif > +} > + > #define MMUSUFFIX _cmmu > #undef GETPC > #define GETPC() NULL > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 77b75bc..c8d5b5f 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -89,6 +89,9 @@ > */ > #define wmb() do { } while (0) > > +/* HACK for virtio to determine if it's running a big endian guest */ > +bool virtio_is_big_endian(void); > + > /* virtio device */ > > static void virtio_pci_notify(void *opaque, uint16_t vector) > @@ -412,20 +415,35 @@ 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); > + if (virtio_is_big_endian()) { > + /* > + * 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); > + } > + 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); > + if (virtio_is_big_endian()) { > + val = bswap32(val); > + } > + return val; > } > > static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val) > @@ -449,6 +467,9 @@ static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val) > return; > } > addr -= config; > + if (virtio_is_big_endian()) { > + val = bswap16(val); > + } > virtio_config_writew(proxy->vdev, addr, val); > } > > @@ -461,6 +482,9 @@ static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val) > return; > } > addr -= config; > + if (virtio_is_big_endian()) { > + val = bswap32(val); > + } > virtio_config_writel(proxy->vdev, addr, val); > } >