[Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer

[Anthony Liguori]

PCI devices have a fixed byte order that is independent of the host byte order.
For the most part, PCI devices are little endian.  Because we have allowed PCI
devices in the past to directly register memory functions with the CPU, these
devices have been subject to automagic swapping which means that they end up
returning target endianness instead of the fixed endianness that the device
supports.

This patch fixes this by allow devices to return native words in their interface
and allowing the PCI bus to do appropriate swapping before the data makes it
to the CPU.

Right now, most devices do this wrong so this should, in theory, fix a lot of
PCI devices on big endian targets.

Signed-off-by: Anthony Liguori <address@hidden>
---
 hw/e1000.c   |    9 +--------
 hw/pci.c     |   18 ++++++++++++++++--
 hw/rtl8139.c |   44 +-------------------------------------------
 3 files changed, 18 insertions(+), 53 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 823088c..0738e38 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -847,9 +847,6 @@ static void e1000_mmio_write(PCIDevice *dev, pcibus_t addr, 
int size,
     E1000State *d = DO_UPCAST(E1000State, dev, dev);
     unsigned int index = (addr & 0x1ffff) >> 2;
 
-#ifdef TARGET_WORDS_BIGENDIAN
-    value = bswap32(value);
-#endif
     value = (value & size_mask(size)) << addr_shift(addr);
 
     if (index < NWRITEOPS && macreg_writeops[index])
@@ -873,11 +870,7 @@ static uint32_t e1000_mmio_read(PCIDevice *dev, pcibus_t 
addr, int size)
         DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
     }
 
-    value = (value >> addr_shift(addr)) & size_mask(size);
-#ifdef TARGET_WORDS_BIGENDIAN
-    value = bswap32(value);
-#endif
-    return value;
+    return (value >> addr_shift(addr)) & size_mask(size);
 }
 
 static bool is_version_1(void *opaque, int version_id)
diff --git a/hw/pci.c b/hw/pci.c
index 50ae917..f1816f9 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -765,6 +765,9 @@ static void pci_io_region_writew(void *opaque, 
target_phys_addr_t addr,
                                  uint32_t value)
 {
     PCIIORegion *r = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap16(value);
+#endif
     r->write(r->dev, addr, 2, value);
 }
 
@@ -772,6 +775,9 @@ static void pci_io_region_writel(void *opaque, 
target_phys_addr_t addr,
                                  uint32_t value)
 {
     PCIIORegion *r = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap32(value);
+#endif
     r->write(r->dev, addr, 4, value);
 }
 
@@ -784,13 +790,21 @@ static uint32_t pci_io_region_readb(void *opaque, 
target_phys_addr_t addr)
 static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
 {
     PCIIORegion *r = opaque;
-    return r->read(r->dev, addr, 2);
+    uint32_t value = r->read(r->dev, addr, 2);
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap16(value);
+#endif
+    return value;
 }
 
 static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
 {
     PCIIORegion *r = opaque;
-    return r->read(r->dev, addr, 4);
+    uint32_t value = r->read(r->dev, addr, 4);
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap32(value);
+#endif
+    return value;
 }
 
 static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 95e7aeb..a812c5f 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3107,48 +3107,6 @@ static const VMStateDescription vmstate_rtl8139 = {
 /***********************************************************/
 /* PCI RTL8139 definitions */
 
-static uint32_t rtl8139_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
-{
-    RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
-    uint32_t value;
-
-    if (size == 1) {
-        value = rtl8139_io_readb(s, addr);
-    } else if (size == 2) {
-        value = rtl8139_io_readw(s, addr);
-#ifdef TARGET_WORDS_BIGENDIAN
-        value = bswap16(value);
-#endif
-    } else {
-        value = rtl8139_io_readl(s, addr & 0xFF);
-#ifdef TARGET_WORDS_BIGENDIAN
-        value = bswap32(value);
-#endif
-    }
-
-    return value;
-}
-
-static void rtl8139_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
-                               uint32_t value)
-{
-    RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
-
-    if (size == 1) {
-        rtl8139_io_writeb(s, addr, value);
-    } else if (size == 2) {
-#ifdef TARGET_WORDS_BIGENDIAN
-        value = bswap16(value);
-#endif
-        rtl8139_io_writew(s, addr, value);
-    } else if (size == 4) {
-#ifdef TARGET_WORDS_BIGENDIAN
-        value = bswap32(value);
-#endif
-        rtl8139_io_writel(s, addr, value);
-    }
-}
-
 static uint32_t rtl8139_io_read(PCIDevice *dev, pcibus_t addr, int size)
 {
     RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
@@ -3286,7 +3244,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
     pci_register_io_region(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO,
                            rtl8139_io_read, rtl8139_io_write);
     pci_register_io_region(&s->dev, 1, 0x100, PCI_BASE_ADDRESS_SPACE_MEMORY,
-                           rtl8139_mmio_read, rtl8139_mmio_write);
+                           rtl8139_io_read, rtl8139_io_write);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
-- 
1.6.5.2