[Qemu-devel] [PATCH] spapr-pci: remove io ports workaround

[Alexey Kardashevskiy]

In the past, IO space could not be mapped into the memory address space
so we introduced a workaround for that. Nowadays it does not look
necessary so we can remove the workaround and make sPAPR PCI
configuration simplier.

This also removes all byte swappings as it is not PHB's to take care
of endiannes - devices should do convertion if they want to. And almost
every PCI device which uses IO ports does that by registering IO ports
region with memory_region_init_io() (Intel e1000, RTL8139, virtio).

However VGA uses MemoryRegionPortio which does not support endiannes
but it still expects the convertion to be done. For this case only,
this patch adds LITTLE_ENDIAN flag to portio_ops. Tests on PPC64 show
that other devices are not affected by this change. x86 systems should
not suffer either as having LITTLE_ENDIAN there has no effect.

Signed-off-by: Alexey Kardashevskiy <address@hidden>
---

This removes bugs at least from SPAPR so any further fixes should be equal
for all platforms and hopefully will break all platform altogether but not
just PPC64-pSeries :)

Did I miss anything here?


---
 hw/ppc/spapr_pci.c | 52 +++-------------------------------------------------
 ioport.c           |  1 +
 2 files changed, 4 insertions(+), 49 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index ca588aa..8d76290 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -440,43 +440,6 @@ static void pci_spapr_set_irq(void *opaque, int irq_num, 
int level)
     qemu_set_irq(spapr_phb_lsi_qirq(phb, irq_num), level);
 }
 
-static uint64_t spapr_io_read(void *opaque, hwaddr addr,
-                              unsigned size)
-{
-    switch (size) {
-    case 1:
-        return cpu_inb(addr);
-    case 2:
-        return cpu_inw(addr);
-    case 4:
-        return cpu_inl(addr);
-    }
-    g_assert_not_reached();
-}
-
-static void spapr_io_write(void *opaque, hwaddr addr,
-                           uint64_t data, unsigned size)
-{
-    switch (size) {
-    case 1:
-        cpu_outb(addr, data);
-        return;
-    case 2:
-        cpu_outw(addr, data);
-        return;
-    case 4:
-        cpu_outl(addr, data);
-        return;
-    }
-    g_assert_not_reached();
-}
-
-static const MemoryRegionOps spapr_io_ops = {
-    .endianness = DEVICE_LITTLE_ENDIAN,
-    .read = spapr_io_read,
-    .write = spapr_io_write
-};
-
 /*
  * MSI/MSIX memory region implementation.
  * The handler handles both MSI and MSIX.
@@ -590,23 +553,14 @@ static int spapr_phb_init(SysBusDevice *s)
     memory_region_add_subregion(get_system_memory(), sphb->mem_win_addr,
                                 &sphb->memwindow);
 
-    /* On ppc, we only have MMIO no specific IO space from the CPU
-     * perspective.  In theory we ought to be able to embed the PCI IO
-     * memory region direction in the system memory space.  However,
-     * if any of the IO BAR subregions use the old_portio mechanism,
-     * that won't be processed properly unless accessed from the
-     * system io address space.  This hack to bounce things via
-     * system_io works around the problem until all the users of
-     * old_portion are updated */
+    /* Initialize IO regions */
     sprintf(namebuf, "%s.io", sphb->dtbusname);
     memory_region_init(&sphb->iospace, OBJECT(sphb),
                        namebuf, SPAPR_PCI_IO_WIN_SIZE);
-    /* FIXME: fix to support multiple PHBs */
-    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
 
     sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
-    memory_region_init_io(&sphb->iowindow, OBJECT(sphb), &spapr_io_ops, sphb,
-                          namebuf, SPAPR_PCI_IO_WIN_SIZE);
+    memory_region_init_alias(&sphb->iowindow, OBJECT(sphb),
+                             namebuf, &sphb->iospace, 0, 
SPAPR_PCI_IO_WIN_SIZE);
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
 
diff --git a/ioport.c b/ioport.c
index 89b17d6..79b7f1a 100644
--- a/ioport.c
+++ b/ioport.c
@@ -183,6 +183,7 @@ static void portio_write(void *opaque, hwaddr addr, 
uint64_t data,
 static const MemoryRegionOps portio_ops = {
     .read = portio_read,
     .write = portio_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid.unaligned = true,
     .impl.unaligned = true,
 };
-- 
1.8.3.2