| Message ID | 1326240442-22915-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On 01/10/2012 06:07 PM, Alexander Graf wrote: > From: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > 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<benh@kernel.crashing.org> > Signed-off-by: Alexander Graf<agraf@suse.de> > [agraf: keep virtio in libhw and determine endianness through a > helper function in exec.c] Both: Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> 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); > } >
On 11.01.2012, at 01:07, Anthony Liguori wrote: > On 01/10/2012 06:07 PM, Alexander Graf wrote: >> From: Benjamin Herrenschmidt<benh@kernel.crashing.org> >> >> 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<benh@kernel.crashing.org> >> Signed-off-by: Alexander Graf<agraf@suse.de> >> [agraf: keep virtio in libhw and determine endianness through a >> helper function in exec.c] > > Both: > > Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> Cool :). Queued up in ppc-next. Alex
On Wed, 2012-01-11 at 01:08 +0100, Alexander Graf wrote: > On 11.01.2012, at 01:07, Anthony Liguori wrote: > > > On 01/10/2012 06:07 PM, Alexander Graf wrote: > >> From: Benjamin Herrenschmidt<benh@kernel.crashing.org> > >> > >> 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<benh@kernel.crashing.org> > >> Signed-off-by: Alexander Graf<agraf@suse.de> > >> [agraf: keep virtio in libhw and determine endianness through a > >> helper function in exec.c] > > > > Both: > > > > Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> > > Cool :). Queued up in ppc-next. Fix the commit log, you aren't moving it to Makefile.target anymore :-) Cheers, Ben.
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); }