[Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

This series fixes the I/O port endianness problems that came when
port I/O was made to go through the normal dispatch path.  Targets
that used isa_mmio to invoke cpu_inl and friends now tried to do
byte swapping once more than they previously did, causing garbage
to be written.

This series drops isa_mmio and replace it with an alias of the
root I/O memory region.  After applying back the LITTLE_ENDIAN
mark for PortioLists, everything works as expected: the port memory
regions appear directly in the FlatView with the right endianness,
and MMIO is dispatched straight to them.

PReP cannot use this due to the non-contiguous map, so we just
make sure the prep_io_ops are marked DEVICE_NATIVE_ENDIAN; the swap
will happen when prep_io_ops redispatch to the device.

So we had a change that broke multiple ports, most of them
under-maintained.  What better occasion to add a unit test?  This
series does that, using the pc-testdev device as the harness.
The test was run before and after the I/O changes (in particular
on top of commit a014ed0, memory: accept mismatching sizes in
memory_region_access_valid, 2013-05-24), and passes there too.

This also found some unrelated problems caused by the new QOM cast in
commit 7588e2b (pci: Fold host_buses list into PCIHostState functionality,
2013-06-06).

I also smoke-tested prep, with the kernel that Herve provided, and
pseries.  VGA works for both, which tests the PortioList path.
Alexey tested Linux (with virtio) on pseries.

Patches 1 to 15 fix all bugs and remove isa_mmio.

Patches 16 to 23 prepare the boards so that the testcase runs
successfully.  This includes adding missing devices and unbreaking
other breakage.

Patch 24 adds the test case, patches 25-27 fix big-endian split/combine,
patch 28 adds a test case for that too.

If preferred, I can prepare a pull request that merges the tests
from an earlier branchpoint.  This makes it easy to run the tests
on both the old and the new code.

Paolo

Alexey Kardashevskiy (1):
  spapr_pci: remove indirection for I/O port access

Paolo Bonzini (27):
  sh4: do not use isa_mmio
  ppc_oldworld: do not use isa_mmio
  ppc_newworld: do not use isa_mmio
  prep: fix I/O port endianness
  mips_jazz: do not use isa_mmio
  mips_r4k: do not use isa_mmio
  mips_malta: do not use isa_mmio
  ppc440_bamboo: do not use isa_mmio
  mipssim: do not use isa_mmio
  mips_fulong2e: do not use isa_mmio
  sparc64: remove indirection for I/O port access
  ebus: do not use isa_mmio
  isa_mmio: delete
  Revert "ioport: remove LITTLE_ENDIAN mark for portio"
  pc-testdev: support 8 and 16-bit accesses to 0xe0
  pc-testdev: remove useless cpu_to_le64/le64_to_cpu
  mips: degrade BIOS error to warning
  sh4: unbreak r2d
  sparc64: unbreak
  default-configs: add test device to all machines supporting ISA
  default-configs: add SuperIO to SH4
  default-configs/ppc64: add all components of i82378 SuperIO chip used
    by prep
  qtest: add test for ISA I/O space endianness
  memory: move functions around
  memory: pass MemoryRegion to access_with_adjusted_size
  memory: check memory region endianness, not target's
  pc-testdev: add I/O port to test memory.c auto split/combine

 default-configs/alpha-softmmu.mak    |   1 +
 default-configs/mips-softmmu.mak     |   2 +-
 default-configs/mips64-softmmu.mak   |   2 +-
 default-configs/mips64el-softmmu.mak |   2 +-
 default-configs/mipsel-softmmu.mak   |   2 +-
 default-configs/ppc-softmmu.mak      |   1 +
 default-configs/ppc64-softmmu.mak    |   7 +
 default-configs/ppcemb-softmmu.mak   |   1 +
 default-configs/sh4-softmmu.mak      |   9 +-
 default-configs/sh4eb-softmmu.mak    |   9 +-
 default-configs/sparc64-softmmu.mak  |   1 +
 hw/isa/Makefile.objs                 |   1 -
 hw/isa/isa_mmio.c                    |  81 ---------
 hw/mips/gt64xxx_pci.c                |   3 +-
 hw/mips/mips_fulong2e.c              |   3 +-
 hw/mips/mips_jazz.c                  |   8 +-
 hw/mips/mips_malta.c                 |   3 +-
 hw/mips/mips_mipssim.c               |   8 +-
 hw/mips/mips_r4k.c                   |   6 +-
 hw/misc/pc-testdev.c                 |  28 +++-
 hw/pci-host/apb.c                    | 101 ++++-------
 hw/pci-host/bonito.c                 |  25 ++-
 hw/ppc/mac_newworld.c                |   5 +-
 hw/ppc/mac_oldworld.c                |   5 +-
 hw/ppc/ppc440_bamboo.c               |   5 +-
 hw/ppc/prep.c                        |   2 +-
 hw/ppc/spapr_pci.c                   |  41 +----
 hw/sh4/sh_pci.c                      |  42 +++--
 hw/sparc64/sun4u.c                   |   6 +-
 include/hw/isa/isa.h                 |   3 -
 ioport.c                             |   1 +
 memory.c                             | 101 +++++------
 tests/Makefile                       |  14 +-
 tests/endianness-test.c              | 316 +++++++++++++++++++++++++++++++++++
 34 files changed, 543 insertions(+), 302 deletions(-)
 delete mode 100644 hw/isa/isa_mmio.c
 create mode 100644 tests/endianness-test.c

-- 
1.8.1.4

[Qemu-devel] [PATCH 01/28] sh4: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access (for sh4eb).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/sh4/sh_pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index c33a72f..53e9eb8 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -129,7 +129,8 @@ static int sh_pci_device_init(SysBusDevice *dev)
                           "sh_pci", 0x224);
     memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",
                              &s->memconfig_p4, 0, 0x224);
-    isa_mmio_setup(&s->isa, 0x40000);
+    memory_region_init_alias(&s->isa, OBJECT(s), "sh_pci.isa",
+                             get_system_io(), 0, 0x40000);
     sysbus_init_mmio(dev, &s->memconfig_p4);
     sysbus_init_mmio(dev, &s->memconfig_a7);
     s->iobr = 0xfe240000;
-- 
1.8.1.4

[Qemu-devel] [PATCH 02/28] ppc_oldworld: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/mac_oldworld.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 8b8c6b9..42bb9d5 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -87,6 +87,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     int linux_boot, i;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base, cmdline_base = 0;
     int32_t kernel_size, initrd_size;
     PCIBus *pci_bus;
@@ -225,7 +226,9 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     }
 
     /* Register 2 MB of ISA IO space */
-    isa_mmio_init(0xfe000000, 0x00200000);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, 0x00200000);
+    memory_region_add_subregion(sysmem, 0xfe000000, isa);
 
     /* XXX: we register only 1 output pin for heathrow PIC */
     heathrow_irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *));
-- 
1.8.1.4

[Qemu-devel] [PATCH 03/28] ppc_newworld: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/mac_newworld.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index fe80348..7ef806e 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -152,6 +152,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     CPUPPCState *env = NULL;
     char *filename;
     qemu_irq *pic, **openpic_irqs;
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *unin_memory = g_new(MemoryRegion, 1);
     MemoryRegion *unin2_memory = g_new(MemoryRegion, 1);
     int linux_boot, i, j, k;
@@ -288,7 +289,9 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     }
 
     /* Register 8 MB of ISA IO space */
-    isa_mmio_init(0xf2000000, 0x00800000);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, 0x00800000);
+    memory_region_add_subregion(get_system_memory(), 0xf2000000, isa);
 
     /* UniN init: XXX should be a real device */
     memory_region_init_io(unin_memory, NULL, &unin_ops, token, "unin", 0x1000);
-- 
1.8.1.4

[Qemu-devel] [PATCH 04/28] spapr_pci: remove indirection for I/O port access

[Paolo Bonzini]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This fixes endianness bugs in I/O port access.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/spapr_pci.c | 41 ++---------------------------------------
 1 file changed, 2 insertions(+), 39 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 318bc9d..c880a75 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.
@@ -605,8 +568,8 @@ static int spapr_phb_init(SysBusDevice *s)
     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,
+                             get_system_io(), 0, SPAPR_PCI_IO_WIN_SIZE);
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 05/28] prep: fix I/O port endianness

[Paolo Bonzini]

Do not swap endianness here, it will happen during cpu_{in,out}{b,w,l}.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/prep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 19f2442..7e04b1a 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -410,7 +410,7 @@ static const MemoryRegionOps PPC_prep_io_ops = {
         .read = { PPC_prep_io_readb, PPC_prep_io_readw, PPC_prep_io_readl },
         .write = { PPC_prep_io_writeb, PPC_prep_io_writew, PPC_prep_io_writel },
     },
-    .endianness = DEVICE_LITTLE_ENDIAN,
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 #define NVRAM_SIZE        0x2000
-- 
1.8.1.4

[Qemu-devel] [PATCH 06/28] mips_jazz: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_jazz.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 31e138b..2c4acb9 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -119,6 +119,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
     qemu_irq *rc4030, *i8259;
     rc4030_dma *dmas;
     void* rc4030_opaque;
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *rtc = g_new(MemoryRegion, 1);
     MemoryRegion *i8042 = g_new(MemoryRegion, 1);
     MemoryRegion *dma_dummy = g_new(MemoryRegion, 1);
@@ -201,7 +202,9 @@ static void mips_jazz_init(MemoryRegion *address_space,
     pcspk_init(isa_bus, pit);
 
     /* ISA IO space at 0x90000000 */
-    isa_mmio_init(0x90000000, 0x01000000);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, 0x01000000);
+    memory_region_add_subregion(address_space, 0x90000000, isa);
     isa_mem_base = 0x11000000;
 
     /* Video card */
-- 
1.8.1.4

[Qemu-devel] [PATCH 07/28] mips_r4k: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_r4k.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 22beb0a..4bc2e3f 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -164,6 +164,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args)
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios;
     MemoryRegion *iomem = g_new(MemoryRegion, 1);
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     int bios_size;
     MIPSCPU *cpu;
     CPUMIPSState *env;
@@ -273,7 +274,10 @@ void mips_r4k_init(QEMUMachineInitArgs *args)
     rtc_init(isa_bus, 2000, NULL);
 
     /* Register 64 KB of ISA IO space at 0x14000000 */
-    isa_mmio_init(0x14000000, 0x00010000);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, 0x00010000);
+    memory_region_add_subregion(get_system_memory(), 0x14000000, isa);
+
     isa_mem_base = 0x10000000;
 
     pit = pit_init(isa_bus, 0x40, 0, NULL);
-- 
1.8.1.4

[Qemu-devel] [PATCH 08/28] mips_malta: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/gt64xxx_pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 5843417..3da2e67 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -304,7 +304,8 @@ static void gt64120_pci_mapping(GT64120State *s)
       s->PCI0IO_length = ((s->regs[GT_PCI0IOHD] + 1) - (s->regs[GT_PCI0IOLD] & 0x7f)) << 21;
       isa_mem_base = s->PCI0IO_start;
       if (s->PCI0IO_length) {
-          isa_mmio_setup(&s->PCI0IO_mem, s->PCI0IO_length);
+          memory_region_init_alias(&s->PCI0IO_mem, OBJECT(s), "isa_mmio",
+                                   get_system_io(), 0, s->PCI0IO_length);
           memory_region_add_subregion(get_system_memory(), s->PCI0IO_start,
                                       &s->PCI0IO_mem);
       }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 08/28] mips_malta: do not use isa_mmio

[Aurelien Jarno]

On Mon, Jul 22, 2013 at 03:54:18PM +0200, Paolo Bonzini wrote:
> This fixes endianness bugs in I/O port access.

It looks like it actually did the reverse, ie introducing endianness
bugs. With this patch, the pcnet-pci NIC (default NIC card) doesn't work
any more on big endian Malta, while it still works on little endian
Malta.

Reverting this commit fixes the issue for the pcnet card, but it makes
the IDE controller to fail, likely due to endianness issues.

This is reproducible using the following kernel and the following line:

http://ftp.debian.org/debian/dists/wheezy/main/installer-mips/current/images/malta/netboot/vmlinux-3.2.0-4-4kc-malta
qemu-system-mips -kernel vmlinux-3.2.0-4-4kc-malta -nographic 

In that case the boot log is:

| [    0.464000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
| [    0.464000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
| [    0.468000] pcnet32: No access methods

Without this patch, the boot log is:

| [    0.524000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
| [    0.524000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
| [    0.524000] pcnet32: PCnet/PCI II 79C970A at 0x1020, 52:54:00:12:34:56 assigned IRQ 10
| [    0.528000] pcnet32: eth0: registered as PCnet/PCI II 79C970A
| [    0.532000] pcnet32: 1 cards_found

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/mips/gt64xxx_pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 5843417..3da2e67 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -304,7 +304,8 @@ static void gt64120_pci_mapping(GT64120State *s)
>        s->PCI0IO_length = ((s->regs[GT_PCI0IOHD] + 1) - (s->regs[GT_PCI0IOLD] & 0x7f)) << 21;
>        isa_mem_base = s->PCI0IO_start;
>        if (s->PCI0IO_length) {
> -          isa_mmio_setup(&s->PCI0IO_mem, s->PCI0IO_length);
> +          memory_region_init_alias(&s->PCI0IO_mem, OBJECT(s), "isa_mmio",
> +                                   get_system_io(), 0, s->PCI0IO_length);
>            memory_region_add_subregion(get_system_memory(), s->PCI0IO_start,
>                                        &s->PCI0IO_mem);
>        }
> -- 
> 1.8.1.4
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 08/28] mips_malta: do not use isa_mmio

[Aurelien Jarno]

On Wed, Aug 28, 2013 at 01:03:01PM +0200, Aurelien Jarno wrote:
> On Mon, Jul 22, 2013 at 03:54:18PM +0200, Paolo Bonzini wrote:
> > This fixes endianness bugs in I/O port access.
> 
> It looks like it actually did the reverse, ie introducing endianness
> bugs. With this patch, the pcnet-pci NIC (default NIC card) doesn't work
> any more on big endian Malta, while it still works on little endian
> Malta.
> 
> Reverting this commit fixes the issue for the pcnet card, but it makes
> the IDE controller to fail, likely due to endianness issues.
> 
> This is reproducible using the following kernel and the following line:
> 
> http://ftp.debian.org/debian/dists/wheezy/main/installer-mips/current/images/malta/netboot/vmlinux-3.2.0-4-4kc-malta
> qemu-system-mips -kernel vmlinux-3.2.0-4-4kc-malta -nographic 
> 
> In that case the boot log is:
> 
> | [    0.464000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
> | [    0.464000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
> | [    0.468000] pcnet32: No access methods
> 
> Without this patch, the boot log is:
> 
> | [    0.524000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
> | [    0.524000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
> | [    0.524000] pcnet32: PCnet/PCI II 79C970A at 0x1020, 52:54:00:12:34:56 assigned IRQ 10
> | [    0.528000] pcnet32: eth0: registered as PCnet/PCI II 79C970A
> | [    0.532000] pcnet32: 1 cards_found
> 

It seems to be due to the fact that the pcnet-pci device is declared as
NATIVE_ENDIAN. I don't really understand why. Changing it to
LITTLE_ENDIAN as in the following patch also fixes the problem, but I am
not sure it is correct.

diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index a893165..865f2f0 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -134,7 +134,7 @@ static void pcnet_ioport_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps pcnet_io_ops = {
     .read = pcnet_ioport_read,
     .write = pcnet_ioport_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void pcnet_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
@@ -256,7 +256,7 @@ static const MemoryRegionOps pcnet_mmio_ops = {
         .read = { pcnet_mmio_readb, pcnet_mmio_readw, pcnet_mmio_readl },
         .write = { pcnet_mmio_writeb, pcnet_mmio_writew, pcnet_mmio_writel },
     },
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void pci_physical_memory_write(void *dma_opaque, hwaddr addr,

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 08/28] mips_malta: do not use isa_mmio

[Paolo Bonzini]

Il 28/08/2013 13:13, Aurelien Jarno ha scritto:
> On Wed, Aug 28, 2013 at 01:03:01PM +0200, Aurelien Jarno wrote:
>> On Mon, Jul 22, 2013 at 03:54:18PM +0200, Paolo Bonzini wrote:
>>> This fixes endianness bugs in I/O port access.
>>
>> It looks like it actually did the reverse, ie introducing endianness
>> bugs. With this patch, the pcnet-pci NIC (default NIC card) doesn't work
>> any more on big endian Malta, while it still works on little endian
>> Malta.
>>
>> Reverting this commit fixes the issue for the pcnet card, but it makes
>> the IDE controller to fail, likely due to endianness issues.
>>
>> This is reproducible using the following kernel and the following line:
>>
>> http://ftp.debian.org/debian/dists/wheezy/main/installer-mips/current/images/malta/netboot/vmlinux-3.2.0-4-4kc-malta
>> qemu-system-mips -kernel vmlinux-3.2.0-4-4kc-malta -nographic 
>>
>> In that case the boot log is:
>>
>> | [    0.464000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
>> | [    0.464000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
>> | [    0.468000] pcnet32: No access methods
>>
>> Without this patch, the boot log is:
>>
>> | [    0.524000] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
>> | [    0.524000] PCI: Enabling device 0000:00:0b.0 (0000 -> 0003)
>> | [    0.524000] pcnet32: PCnet/PCI II 79C970A at 0x1020, 52:54:00:12:34:56 assigned IRQ 10
>> | [    0.528000] pcnet32: eth0: registered as PCnet/PCI II 79C970A
>> | [    0.532000] pcnet32: 1 cards_found
>>
> 
> It seems to be due to the fact that the pcnet-pci device is declared as
> NATIVE_ENDIAN. I don't really understand why. Changing it to
> LITTLE_ENDIAN as in the following patch also fixes the problem, but I am
> not sure it is correct.

Yes, it is.  See patch 15 for an example of doing the same thing.

Paolo

> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
> index a893165..865f2f0 100644
> --- a/hw/net/pcnet-pci.c
> +++ b/hw/net/pcnet-pci.c
> @@ -134,7 +134,7 @@ static void pcnet_ioport_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps pcnet_io_ops = {
>      .read = pcnet_ioport_read,
>      .write = pcnet_ioport_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
>  static void pcnet_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
> @@ -256,7 +256,7 @@ static const MemoryRegionOps pcnet_mmio_ops = {
>          .read = { pcnet_mmio_readb, pcnet_mmio_readw, pcnet_mmio_readl },
>          .write = { pcnet_mmio_writeb, pcnet_mmio_writew, pcnet_mmio_writel },
>      },
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
>  static void pci_physical_memory_write(void *dma_opaque, hwaddr addr,
> 

[Qemu-devel] [PATCH 09/28] ppc440_bamboo: do not use isa_mmio

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/ppc440_bamboo.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 5b039ab..369ab9e 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -164,6 +164,7 @@ static void bamboo_init(QEMUMachineInitArgs *args)
     const char *initrd_filename = args->initrd_filename;
     unsigned int pci_irq_nrs[4] = { 28, 27, 26, 25 };
     MemoryRegion *address_space_mem = get_system_memory();
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *ram_memories
         = g_malloc(PPC440EP_SDRAM_NR_BANKS * sizeof(*ram_memories));
     hwaddr ram_bases[PPC440EP_SDRAM_NR_BANKS];
@@ -225,7 +226,9 @@ static void bamboo_init(QEMUMachineInitArgs *args)
         exit(1);
     }
 
-    isa_mmio_init(PPC440EP_PCI_IO, PPC440EP_PCI_IOLEN);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, PPC440EP_PCI_IOLEN);
+    memory_region_add_subregion(get_system_memory(), PPC440EP_PCI_IO, isa);
 
     if (serial_hds[0] != NULL) {
         serial_mm_init(address_space_mem, 0xef600300, 0, pic[0],
-- 
1.8.1.4

[Qemu-devel] [PATCH 10/28] mipssim: do not use isa_mmio

[Paolo Bonzini]

Untested, this board does not support PCI so it cannot run endianness-test.
It should fix endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_mipssim.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index a10cf13..e4a11c8 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -140,6 +140,7 @@ mips_mipssim_init(QEMUMachineInitArgs *args)
     const char *initrd_filename = args->initrd_filename;
     char *filename;
     MemoryRegion *address_space_mem = get_system_memory();
+    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     MIPSCPU *cpu;
@@ -212,7 +213,9 @@ mips_mipssim_init(QEMUMachineInitArgs *args)
     cpu_mips_clock_init(env);
 
     /* Register 64 KB of ISA IO space at 0x1fd00000. */
-    isa_mmio_init(0x1fd00000, 0x00010000);
+    memory_region_init_alias(isa, NULL, "isa_mmio",
+                             get_system_io(), 0, 0x00010000);
+    memory_region_add_subregion(get_system_memory(), 0x1fd00000, isa);
 
     /* A single 16450 sits at offset 0x3f8. It is attached to
        MIPS CPU INT2, which is interrupt 4. */
-- 
1.8.1.4

[Qemu-devel] [PATCH 11/28] mips_fulong2e: do not use isa_mmio

[Paolo Bonzini]

This board is little-endian, but still isa_mmio should die. :)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci-host/bonito.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 592d666..5086d42 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -210,14 +210,8 @@ typedef struct PCIBonitoState
     MemoryRegion iomem;
     MemoryRegion iomem_ldma;
     MemoryRegion iomem_cop;
-
-    hwaddr bonito_pciio_start;
-    hwaddr bonito_pciio_length;
-    int bonito_pciio_handle;
-
-    hwaddr bonito_localio_start;
-    hwaddr bonito_localio_length;
-    int bonito_localio_handle;
+    MemoryRegion bonito_pciio;
+    MemoryRegion bonito_localio;
 
 } PCIBonitoState;
 
@@ -750,15 +744,16 @@ static int bonito_initfn(PCIDevice *dev)
     sysbus_mmio_map(sysbus, 4, 0xbfe00300);
 
     /* Map PCI IO Space  0x1fd0 0000 - 0x1fd1 0000 */
-    s->bonito_pciio_start = BONITO_PCIIO_BASE;
-    s->bonito_pciio_length = BONITO_PCIIO_SIZE;
-    isa_mem_base = s->bonito_pciio_start;
-    isa_mmio_init(s->bonito_pciio_start, s->bonito_pciio_length);
+    memory_region_init_alias(&s->bonito_pciio, OBJECT(s), "isa_mmio",
+                             get_system_io(), 0, BONITO_PCIIO_SIZE);
+    sysbus_init_mmio(sysbus, &s->bonito_pciio);
+    sysbus_mmio_map(sysbus, 5, BONITO_PCIIO_BASE);
 
     /* add pci local io mapping */
-    s->bonito_localio_start = BONITO_DEV_BASE;
-    s->bonito_localio_length = BONITO_DEV_SIZE;
-    isa_mmio_init(s->bonito_localio_start, s->bonito_localio_length);
+    memory_region_init_alias(&s->bonito_localio, OBJECT(s), "isa_mmio",
+                             get_system_io(), 0, BONITO_DEV_SIZE);
+    sysbus_init_mmio(sysbus, &s->bonito_localio);
+    sysbus_mmio_map(sysbus, 6, BONITO_DEV_BASE);
 
     /* set the default value of north bridge pci config */
     pci_set_word(dev->config + PCI_COMMAND, 0x0000);
-- 
1.8.1.4

[Qemu-devel] [PATCH 12/28] sparc64: remove indirection for I/O port access

[Paolo Bonzini]

This fixes endianness bugs in I/O port access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci-host/apb.c | 54 ++----------------------------------------------------
 1 file changed, 2 insertions(+), 52 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 06ace08..208ac85 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -302,56 +302,6 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
     return ret;
 }
 
-static void pci_apb_iowriteb (void *opaque, hwaddr addr,
-                                  uint32_t val)
-{
-    cpu_outb(addr & IOPORTS_MASK, val);
-}
-
-static void pci_apb_iowritew (void *opaque, hwaddr addr,
-                                  uint32_t val)
-{
-    cpu_outw(addr & IOPORTS_MASK, bswap16(val));
-}
-
-static void pci_apb_iowritel (void *opaque, hwaddr addr,
-                                uint32_t val)
-{
-    cpu_outl(addr & IOPORTS_MASK, bswap32(val));
-}
-
-static uint32_t pci_apb_ioreadb (void *opaque, hwaddr addr)
-{
-    uint32_t val;
-
-    val = cpu_inb(addr & IOPORTS_MASK);
-    return val;
-}
-
-static uint32_t pci_apb_ioreadw (void *opaque, hwaddr addr)
-{
-    uint32_t val;
-
-    val = bswap16(cpu_inw(addr & IOPORTS_MASK));
-    return val;
-}
-
-static uint32_t pci_apb_ioreadl (void *opaque, hwaddr addr)
-{
-    uint32_t val;
-
-    val = bswap32(cpu_inl(addr & IOPORTS_MASK));
-    return val;
-}
-
-static const MemoryRegionOps pci_ioport_ops = {
-    .old_mmio = {
-        .read = { pci_apb_ioreadb, pci_apb_ioreadw, pci_apb_ioreadl },
-        .write = { pci_apb_iowriteb, pci_apb_iowritew, pci_apb_iowritel, },
-    },
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 /* The APB host has an IRQ line for each IRQ line of each slot.  */
 static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
 {
@@ -536,8 +486,8 @@ static int pci_pbm_init_device(SysBusDevice *dev)
     sysbus_init_mmio(dev, &s->pci_config);
 
     /* pci_ioport */
-    memory_region_init_io(&s->pci_ioport, OBJECT(s), &pci_ioport_ops, s,
-                          "apb-pci-ioport", 0x10000);
+    memory_region_init_alias(&s->pci_ioport, OBJECT(s), "apb-pci-ioport",
+                             get_system_io(), 0, 0x10000);
     /* at region 2 */
     sysbus_init_mmio(dev, &s->pci_ioport);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 13/28] ebus: do not use isa_mmio

[Paolo Bonzini]

This is untested, because ebus does not have a libqos module.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/sparc64/sun4u.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 564c195..34a5e73 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -605,9 +605,11 @@ pci_ebus_init1(PCIDevice *pci_dev)
     pci_dev->config[0x09] = 0x00; // programming i/f
     pci_dev->config[0x0D] = 0x0a; // latency_timer
 
-    isa_mmio_setup(&s->bar0, 0x1000000);
+    memory_region_init_alias(&s->bar0, OBJECT(s), "bar0", get_system_io(),
+                             0, 0x1000000);
     pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar0);
-    isa_mmio_setup(&s->bar1, 0x800000);
+    memory_region_init_alias(&s->bar1, OBJECT(s), "bar1", get_system_io(),
+                             0, 0x800000);
     pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar1);
     return 0;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 14/28] isa_mmio: delete

[Paolo Bonzini]

It is not used anymore.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 default-configs/mips-softmmu.mak     |  1 -
 default-configs/mips64-softmmu.mak   |  1 -
 default-configs/mips64el-softmmu.mak |  1 -
 default-configs/mipsel-softmmu.mak   |  1 -
 default-configs/sh4-softmmu.mak      |  1 -
 default-configs/sh4eb-softmmu.mak    |  1 -
 hw/isa/Makefile.objs                 |  1 -
 hw/isa/isa_mmio.c                    | 81 ------------------------------------
 include/hw/isa/isa.h                 |  3 --
 9 files changed, 91 deletions(-)
 delete mode 100644 hw/isa/isa_mmio.c

diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index b443702..6cdca11 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -3,7 +3,6 @@
 include pci.mak
 include sound.mak
 include usb.mak
-CONFIG_ISA_MMIO=y
 CONFIG_ESP=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
index d638957..8997854 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -3,7 +3,6 @@
 include pci.mak
 include sound.mak
 include usb.mak
-CONFIG_ISA_MMIO=y
 CONFIG_ESP=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak
index c9be3f4..f18f3fc 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -3,7 +3,6 @@
 include pci.mak
 include sound.mak
 include usb.mak
-CONFIG_ISA_MMIO=y
 CONFIG_ESP=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak
index 4f4a449..6281f48 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -3,7 +3,6 @@
 include pci.mak
 include sound.mak
 include usb.mak
-CONFIG_ISA_MMIO=y
 CONFIG_ESP=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index f6bf62d..ef963ee 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -5,7 +5,6 @@ include usb.mak
 CONFIG_SERIAL=y
 CONFIG_PTIMER=y
 CONFIG_PFLASH_CFI02=y
-CONFIG_ISA_MMIO=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
diff --git a/default-configs/sh4eb-softmmu.mak b/default-configs/sh4eb-softmmu.mak
index c1d513d..e744486 100644
--- a/default-configs/sh4eb-softmmu.mak
+++ b/default-configs/sh4eb-softmmu.mak
@@ -5,7 +5,6 @@ include usb.mak
 CONFIG_SERIAL=y
 CONFIG_PTIMER=y
 CONFIG_PFLASH_CFI02=y
-CONFIG_ISA_MMIO=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
index 193746a..9164556 100644
--- a/hw/isa/Makefile.objs
+++ b/hw/isa/Makefile.objs
@@ -1,7 +1,6 @@
 common-obj-y += isa-bus.o
 common-obj-$(CONFIG_APM) += apm.o
 common-obj-$(CONFIG_I82378) += i82378.o
-common-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 common-obj-$(CONFIG_PC87312) += pc87312.o
 common-obj-$(CONFIG_PIIX4) += piix4.o
 common-obj-$(CONFIG_VT82C686) += vt82c686.o
diff --git a/hw/isa/isa_mmio.c b/hw/isa/isa_mmio.c
deleted file mode 100644
index 00ae8eb..0000000
--- a/hw/isa/isa_mmio.c
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * Memory mapped access to ISA IO space.
- *
- * Copyright (c) 2006 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "hw/hw.h"
-#include "hw/isa/isa.h"
-#include "exec/address-spaces.h"
-
-static void isa_mmio_writeb (void *opaque, hwaddr addr,
-                                  uint32_t val)
-{
-    cpu_outb(addr & IOPORTS_MASK, val);
-}
-
-static void isa_mmio_writew(void *opaque, hwaddr addr,
-                               uint32_t val)
-{
-    cpu_outw(addr & IOPORTS_MASK, val);
-}
-
-static void isa_mmio_writel(void *opaque, hwaddr addr,
-                               uint32_t val)
-{
-    cpu_outl(addr & IOPORTS_MASK, val);
-}
-
-static uint32_t isa_mmio_readb (void *opaque, hwaddr addr)
-{
-    return cpu_inb(addr & IOPORTS_MASK);
-}
-
-static uint32_t isa_mmio_readw(void *opaque, hwaddr addr)
-{
-    return cpu_inw(addr & IOPORTS_MASK);
-}
-
-static uint32_t isa_mmio_readl(void *opaque, hwaddr addr)
-{
-    return cpu_inl(addr & IOPORTS_MASK);
-}
-
-static const MemoryRegionOps isa_mmio_ops = {
-    .old_mmio = {
-        .write = { isa_mmio_writeb, isa_mmio_writew, isa_mmio_writel },
-        .read = { isa_mmio_readb, isa_mmio_readw, isa_mmio_readl, },
-    },
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-void isa_mmio_setup(MemoryRegion *mr, hwaddr size)
-{
-    memory_region_init_io(mr, NULL, &isa_mmio_ops, NULL, "isa-mmio", size);
-}
-
-void isa_mmio_init(hwaddr base, hwaddr size)
-{
-    MemoryRegion *mr = g_malloc(sizeof(*mr));
-
-    isa_mmio_setup(mr, size);
-    memory_region_add_subregion(get_system_memory(), base, mr);
-}
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e1bf96c..495bcf3 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -92,9 +92,6 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
 
 extern hwaddr isa_mem_base;
 
-void isa_mmio_setup(MemoryRegion *mr, hwaddr size);
-void isa_mmio_init(hwaddr base, hwaddr size);
-
 /* dma.c */
 int DMA_get_channel_mode (int nchan);
 int DMA_read_memory (int nchan, void *buf, int pos, int size);
-- 
1.8.1.4

[Qemu-devel] [PATCH 15/28] Revert "ioport: remove LITTLE_ENDIAN mark for portio"

[Paolo Bonzini]

This reverts commit c3cb8e77804313e1be99b5f28a34a346736707a5.

The scenario where I/O ports are accessed with DEVICE_LITTLE_ENDIAN
endianness now works and will soon be unit tested.  Since the PortioList
indirection assumes little endian, define portio_ops the same way.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ioport.c | 1 +
 1 file changed, 1 insertion(+)

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.1.4

[Qemu-devel] [PATCH 16/28] pc-testdev: support 8 and 16-bit accesses to 0xe0

[Paolo Bonzini]

This will let us use the testdev to test endianness.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/misc/pc-testdev.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/misc/pc-testdev.c b/hw/misc/pc-testdev.c
index 699a16f..6588ea2 100644
--- a/hw/misc/pc-testdev.c
+++ b/hw/misc/pc-testdev.c
@@ -80,13 +80,20 @@ static void test_ioport_write(void *opaque, hwaddr addr, uint64_t data,
                               unsigned len)
 {
     PCTestdev *dev = opaque;
-    dev->ioport_data = data;
+    int bits = len * 8;
+    int start_bit = (addr & 3) * 8;
+    uint32_t mask = ((uint32_t)-1 >> (32 - bits)) << start_bit;
+    dev->ioport_data &= ~mask;
+    dev->ioport_data |= data << start_bit;
 }
 
 static uint64_t test_ioport_read(void *opaque, hwaddr addr, unsigned len)
 {
     PCTestdev *dev = opaque;
-    return dev->ioport_data;
+    int bits = len * 8;
+    int start_bit = (addr & 3) * 8;
+    uint32_t mask = ((uint32_t)-1 >> (32 - bits)) << start_bit;
+    return (dev->ioport_data & mask) >> start_bit;
 }
 
 static const MemoryRegionOps test_ioport_ops = {
-- 
1.8.1.4

[Qemu-devel] [PATCH 17/28] pc-testdev: remove useless cpu_to_le64/le64_to_cpu

[Paolo Bonzini]

So far the device was only used on little-endian machines.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/misc/pc-testdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/misc/pc-testdev.c b/hw/misc/pc-testdev.c
index 6588ea2..ad1aa66 100644
--- a/hw/misc/pc-testdev.c
+++ b/hw/misc/pc-testdev.c
@@ -129,7 +129,6 @@ static uint64_t test_iomem_read(void *opaque, hwaddr addr, unsigned len)
     PCTestdev *dev = opaque;
     uint64_t ret = 0;
     memcpy(&ret, &dev->iomem_buf[addr], len);
-    ret = le64_to_cpu(ret);
 
     return ret;
 }
@@ -138,7 +137,6 @@ static void test_iomem_write(void *opaque, hwaddr addr, uint64_t val,
                              unsigned len)
 {
     PCTestdev *dev = opaque;
-    val = cpu_to_le64(val);
     memcpy(&dev->iomem_buf[addr], &val, len);
     dev->iomem_buf[addr] = val;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 18/28] mips: degrade BIOS error to warning

[Paolo Bonzini]

No free MIPS BIOS is available, so it makes little sense to quit.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_fulong2e.c | 3 +--
 hw/mips/mips_jazz.c     | 3 +--
 hw/mips/mips_malta.c    | 3 +--
 hw/mips/mips_mipssim.c  | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9e305d2..a7e9dcf 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -333,8 +333,7 @@ static void mips_fulong2e_init(QEMUMachineInitArgs *args)
         }
 
         if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) {
-            fprintf(stderr, "qemu: Could not load MIPS bios '%s'\n", bios_name);
-            exit(1);
+            fprintf(stderr, "qemu: Warning, could not load MIPS bios '%s'\n", bios_name);
         }
     }
 
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 2c4acb9..d6e0860 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -177,9 +177,8 @@ static void mips_jazz_init(MemoryRegion *address_space,
         bios_size = -1;
     }
     if (bios_size < 0 || bios_size > MAGNUM_BIOS_SIZE) {
-        fprintf(stderr, "qemu: Could not load MIPS bios '%s'\n",
+        fprintf(stderr, "qemu: Warning, could not load MIPS bios '%s'\n",
                 bios_name);
-        exit(1);
     }
 
     /* Init CPU internal devices */
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index de87241..dad58c0 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -908,9 +908,8 @@ void mips_malta_init(QEMUMachineInitArgs *args)
             }
             if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) {
                 fprintf(stderr,
-                        "qemu: Could not load MIPS bios '%s', and no -kernel argument was specified\n",
+                        "qemu: Warning, could not load MIPS bios '%s', and no -kernel argument was specified\n",
                         bios_name);
-                exit(1);
             }
         }
         /* In little endian mode the 32bit words in the bios are swapped,
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index e4a11c8..e8802c1 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -192,9 +192,8 @@ mips_mipssim_init(QEMUMachineInitArgs *args)
     if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) {
         /* Bail out if we have neither a kernel image nor boot vector code. */
         fprintf(stderr,
-                "qemu: Could not load MIPS bios '%s', and no -kernel argument was specified\n",
+                "qemu: Warning, could not load MIPS bios '%s', and no -kernel argument was specified\n",
                 filename);
-        exit(1);
     } else {
         /* We have a boot vector start address. */
         env->active_tc.PC = (target_long)(int32_t)0xbfc00000;
-- 
1.8.1.4

[Qemu-devel] [PATCH 19/28] sh4: unbreak r2d

[Paolo Bonzini]

... by making sh_pci a subclass of TYPE_PCI_HOST_BRIDGE.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/sh4/sh_pci.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index 53e9eb8..e81176a 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -28,9 +28,14 @@
 #include "qemu/bswap.h"
 #include "exec/address-spaces.h"
 
+#define TYPE_SH_PCI_HOST_BRIDGE "sh_pci"
+
+#define SH_PCI_HOST_BRIDGE(obj) \
+    OBJECT_CHECK(SHPCIState, (obj), TYPE_SH_PCI_HOST_BRIDGE)
+
 typedef struct SHPCIState {
-    SysBusDevice busdev;
-    PCIBus *bus;
+    PCIHostState parent_obj;
+
     PCIDevice *dev;
     qemu_irq irq[4];
     MemoryRegion memconfig_p4;
@@ -45,6 +50,8 @@ static void sh_pci_reg_write (void *p, hwaddr addr, uint64_t val,
                               unsigned size)
 {
     SHPCIState *pcic = p;
+    PCIHostState *phb = PCI_HOST_BRIDGE(pcic);
+
     switch(addr) {
     case 0 ... 0xfc:
         cpu_to_le32w((uint32_t*)(pcic->dev->config + addr), val);
@@ -64,7 +71,7 @@ static void sh_pci_reg_write (void *p, hwaddr addr, uint64_t val,
         }
         break;
     case 0x220:
-        pci_data_write(pcic->bus, pcic->par, val, 4);
+        pci_data_write(phb->bus, pcic->par, val, 4);
         break;
     }
 }
@@ -73,6 +80,8 @@ static uint64_t sh_pci_reg_read (void *p, hwaddr addr,
                                  unsigned size)
 {
     SHPCIState *pcic = p;
+    PCIHostState *phb = PCI_HOST_BRIDGE(pcic);
+
     switch(addr) {
     case 0 ... 0xfc:
         return le32_to_cpup((uint32_t*)(pcic->dev->config + addr));
@@ -83,7 +92,7 @@ static uint64_t sh_pci_reg_read (void *p, hwaddr addr,
     case 0x1c8:
         return pcic->iobr;
     case 0x220:
-        return pci_data_read(pcic->bus, pcic->par, 4);
+        return pci_data_read(phb->bus, pcic->par, 4);
     }
     return 0;
 }
@@ -112,19 +121,21 @@ static void sh_pci_set_irq(void *opaque, int irq_num, int level)
 
 static int sh_pci_device_init(SysBusDevice *dev)
 {
+    PCIHostState *phb;
     SHPCIState *s;
     int i;
 
-    s = FROM_SYSBUS(SHPCIState, dev);
+    s = SH_PCI_HOST_BRIDGE(dev);
+    phb = PCI_HOST_BRIDGE(s);
     for (i = 0; i < 4; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
-    s->bus = pci_register_bus(&s->busdev.qdev, "pci",
-                              sh_pci_set_irq, sh_pci_map_irq,
-                              s->irq,
-                              get_system_memory(),
-                              get_system_io(),
-                              PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+    phb->bus = pci_register_bus(DEVICE(dev), "pci",
+                                sh_pci_set_irq, sh_pci_map_irq,
+                                s->irq,
+                                get_system_memory(),
+                                get_system_io(),
+                                PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
     memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s,
                           "sh_pci", 0x224);
     memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",
@@ -136,7 +147,7 @@ static int sh_pci_device_init(SysBusDevice *dev)
     s->iobr = 0xfe240000;
     memory_region_add_subregion(get_system_memory(), s->iobr, &s->isa);
 
-    s->dev = pci_create_simple(s->bus, PCI_DEVFN(0, 0), "sh_pci_host");
+    s->dev = pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "sh_pci_host");
     return 0;
 }
 
@@ -172,8 +183,8 @@ static void sh_pci_device_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sh_pci_device_info = {
-    .name          = "sh_pci",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .name          = TYPE_SH_PCI_HOST_BRIDGE,
+    .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(SHPCIState),
     .class_init    = sh_pci_device_class_init,
 };
-- 
1.8.1.4

[Qemu-devel] [PATCH 20/28] sparc64: unbreak

[Paolo Bonzini]

... by making apb a subclass of TYPE_PCI_HOST_BRIDGE.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci-host/apb.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 208ac85..3756ce9 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -70,9 +70,14 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
 #define MAX_IVEC 0x40
 #define NO_IRQ_REQUEST (MAX_IVEC + 1)
 
+#define TYPE_APB "pbm"
+
+#define APB_DEVICE(obj) \
+    OBJECT_CHECK(APBState, (obj), TYPE_APB)
+
 typedef struct APBState {
-    SysBusDevice busdev;
-    PCIBus      *bus;
+    PCIHostState parent_obj;
+
     MemoryRegion apb_config;
     MemoryRegion pci_config;
     MemoryRegion pci_mmio;
@@ -284,10 +289,11 @@ static void apb_pci_config_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned size)
 {
     APBState *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
 
     val = qemu_bswap_len(val, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__, addr, val);
-    pci_data_write(s->bus, addr, val, size);
+    pci_data_write(phb->bus, addr, val, size);
 }
 
 static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
@@ -295,8 +301,9 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
 {
     uint32_t ret;
     APBState *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
 
-    ret = pci_data_read(s->bus, addr, size);
+    ret = pci_data_read(phb->bus, addr, size);
     ret = qemu_bswap_len(ret, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
     return ret;
@@ -381,12 +388,13 @@ PCIBus *pci_apb_init(hwaddr special_base,
 {
     DeviceState *dev;
     SysBusDevice *s;
+    PCIHostState *phb;
     APBState *d;
     PCIDevice *pci_dev;
     PCIBridge *br;
 
     /* Ultrasparc PBM main bus */
-    dev = qdev_create(NULL, "pbm");
+    dev = qdev_create(NULL, TYPE_APB);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
     /* apb_config */
@@ -395,24 +403,25 @@ PCIBus *pci_apb_init(hwaddr special_base,
     sysbus_mmio_map(s, 1, special_base + 0x1000000ULL);
     /* pci_ioport */
     sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
-    d = FROM_SYSBUS(APBState, s);
+    d = APB_DEVICE(dev);
 
     memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
     memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
 
-    d->bus = pci_register_bus(&d->busdev.qdev, "pci",
-                              pci_apb_set_irq, pci_pbm_map_irq, d,
-                              &d->pci_mmio,
-                              get_system_io(),
-                              0, 32, TYPE_PCI_BUS);
+    phb = PCI_HOST_BRIDGE(dev);
+    phb->bus = pci_register_bus(DEVICE(phb), "pci",
+                                pci_apb_set_irq, pci_pbm_map_irq, d,
+                                &d->pci_mmio,
+                                get_system_io(),
+                                0, 32, TYPE_PCI_BUS);
 
     *pbm_irqs = d->pbm_irqs;
     d->ivec_irqs = ivec_irqs;
 
-    pci_create_simple(d->bus, 0, "pbm-pci");
+    pci_create_simple(phb->bus, 0, "pbm-pci");
 
     /* APB secondary busses */
-    pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 0), true,
+    pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
                                    "pbm-bridge");
     br = DO_UPCAST(PCIBridge, dev, pci_dev);
     pci_bridge_map_irq(br, "Advanced PCI Bus secondary bridge 1",
@@ -420,7 +429,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
     qdev_init_nofail(&pci_dev->qdev);
     *bus2 = pci_bridge_get_sec_bus(br);
 
-    pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 1), true,
+    pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
                                    "pbm-bridge");
     br = DO_UPCAST(PCIBridge, dev, pci_dev);
     pci_bridge_map_irq(br, "Advanced PCI Bus secondary bridge 2",
@@ -428,13 +437,13 @@ PCIBus *pci_apb_init(hwaddr special_base,
     qdev_init_nofail(&pci_dev->qdev);
     *bus3 = pci_bridge_get_sec_bus(br);
 
-    return d->bus;
+    return phb->bus;
 }
 
 static void pci_pbm_reset(DeviceState *d)
 {
     unsigned int i;
-    APBState *s = container_of(d, APBState, busdev.qdev);
+    APBState *s = APB_DEVICE(d);
 
     for (i = 0; i < 8; i++) {
         s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
@@ -463,7 +472,7 @@ static int pci_pbm_init_device(SysBusDevice *dev)
     APBState *s;
     unsigned int i;
 
-    s = FROM_SYSBUS(APBState, dev);
+    s = APB_DEVICE(dev);
     for (i = 0; i < 8; i++) {
         s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
     }
@@ -531,8 +540,8 @@ static void pbm_host_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pbm_host_info = {
-    .name          = "pbm",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .name          = TYPE_APB,
+    .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(APBState),
     .class_init    = pbm_host_class_init,
 };
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 20/28] sparc64: unbreak

[Andreas Färber]

Am 22.07.2013 15:54, schrieb Paolo Bonzini:
> ... by making apb a subclass of TYPE_PCI_HOST_BRIDGE.

Any chance to get that into the subject somehow? :)

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/pci-host/apb.c | 47 ++++++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 208ac85..3756ce9 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -70,9 +70,14 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
>  #define MAX_IVEC 0x40
>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>  
> +#define TYPE_APB "pbm"
> +
> +#define APB_DEVICE(obj) \
> +    OBJECT_CHECK(APBState, (obj), TYPE_APB)

Was APB() already taken?

> +
>  typedef struct APBState {
> -    SysBusDevice busdev;
> -    PCIBus      *bus;
> +    PCIHostState parent_obj;
> +
>      MemoryRegion apb_config;
>      MemoryRegion pci_config;
>      MemoryRegion pci_mmio;

Thanks for fixing this.

> @@ -284,10 +289,11 @@ static void apb_pci_config_write(void *opaque, hwaddr addr,
>                                   uint64_t val, unsigned size)
>  {
>      APBState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
>  
>      val = qemu_bswap_len(val, size);
>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__, addr, val);
> -    pci_data_write(s->bus, addr, val, size);
> +    pci_data_write(phb->bus, addr, val, size);
>  }
>  
>  static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
> @@ -295,8 +301,9 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
>  {
>      uint32_t ret;
>      APBState *s = opaque;
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
>  
> -    ret = pci_data_read(s->bus, addr, size);
> +    ret = pci_data_read(phb->bus, addr, size);
>      ret = qemu_bswap_len(ret, size);
>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
>      return ret;
> @@ -381,12 +388,13 @@ PCIBus *pci_apb_init(hwaddr special_base,
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> +    PCIHostState *phb;
>      APBState *d;
>      PCIDevice *pci_dev;
>      PCIBridge *br;
>  
>      /* Ultrasparc PBM main bus */
> -    dev = qdev_create(NULL, "pbm");
> +    dev = qdev_create(NULL, TYPE_APB);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      /* apb_config */
> @@ -395,24 +403,25 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      sysbus_mmio_map(s, 1, special_base + 0x1000000ULL);
>      /* pci_ioport */
>      sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
> -    d = FROM_SYSBUS(APBState, s);
> +    d = APB_DEVICE(dev);
>  
>      memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
>      memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
>  
> -    d->bus = pci_register_bus(&d->busdev.qdev, "pci",
> -                              pci_apb_set_irq, pci_pbm_map_irq, d,
> -                              &d->pci_mmio,
> -                              get_system_io(),
> -                              0, 32, TYPE_PCI_BUS);
> +    phb = PCI_HOST_BRIDGE(dev);
> +    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +                                pci_apb_set_irq, pci_pbm_map_irq, d,
> +                                &d->pci_mmio,
> +                                get_system_io(),
> +                                0, 32, TYPE_PCI_BUS);
>  
>      *pbm_irqs = d->pbm_irqs;
>      d->ivec_irqs = ivec_irqs;
>  
> -    pci_create_simple(d->bus, 0, "pbm-pci");
> +    pci_create_simple(phb->bus, 0, "pbm-pci");
>  
>      /* APB secondary busses */
> -    pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 0), true,
> +    pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     "pbm-bridge");

Adjust indentation here ...

>      br = DO_UPCAST(PCIBridge, dev, pci_dev);
>      pci_bridge_map_irq(br, "Advanced PCI Bus secondary bridge 1",
> @@ -420,7 +429,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      qdev_init_nofail(&pci_dev->qdev);
>      *bus2 = pci_bridge_get_sec_bus(br);
>  
> -    pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 1), true,
> +    pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     "pbm-bridge");

... and here while at it?

>      br = DO_UPCAST(PCIBridge, dev, pci_dev);
>      pci_bridge_map_irq(br, "Advanced PCI Bus secondary bridge 2",
> @@ -428,13 +437,13 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      qdev_init_nofail(&pci_dev->qdev);
>      *bus3 = pci_bridge_get_sec_bus(br);
>  
> -    return d->bus;
> +    return phb->bus;
>  }
>  
>  static void pci_pbm_reset(DeviceState *d)
>  {
>      unsigned int i;
> -    APBState *s = container_of(d, APBState, busdev.qdev);
> +    APBState *s = APB_DEVICE(d);
>  
>      for (i = 0; i < 8; i++) {
>          s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
> @@ -463,7 +472,7 @@ static int pci_pbm_init_device(SysBusDevice *dev)
>      APBState *s;
>      unsigned int i;
>  
> -    s = FROM_SYSBUS(APBState, dev);
> +    s = APB_DEVICE(dev);
>      for (i = 0; i < 8; i++) {
>          s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
>      }
> @@ -531,8 +540,8 @@ static void pbm_host_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo pbm_host_info = {
> -    .name          = "pbm",
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .name          = TYPE_APB,
> +    .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(APBState),
>      .class_init    = pbm_host_class_init,
>  };

Otherwise looks fine, one FROM_SYSBUS() less. :)

Note that this will conflict trivially with my PCIBridge series.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 21/28] default-configs: add test device to all machines supporting ISA

[Paolo Bonzini]

This will let these machines run an endianness test for ISA
I/O port space.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 default-configs/alpha-softmmu.mak    | 1 +
 default-configs/mips-softmmu.mak     | 1 +
 default-configs/mips64-softmmu.mak   | 1 +
 default-configs/mips64el-softmmu.mak | 1 +
 default-configs/mipsel-softmmu.mak   | 1 +
 default-configs/ppc-softmmu.mak      | 1 +
 default-configs/ppc64-softmmu.mak    | 1 +
 default-configs/ppcemb-softmmu.mak   | 1 +
 default-configs/sh4-softmmu.mak      | 1 +
 default-configs/sh4eb-softmmu.mak    | 1 +
 default-configs/sparc64-softmmu.mak  | 1 +
 11 files changed, 11 insertions(+)

diff --git a/default-configs/alpha-softmmu.mak b/default-configs/alpha-softmmu.mak
index 18e5337..bc07600 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -14,3 +14,4 @@ CONFIG_VMWARE_VGA=y
 CONFIG_IDE_CMD646=y
 CONFIG_I8259=y
 CONFIG_MC146818RTC=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index 6cdca11..926709a 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -33,3 +33,4 @@ CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_VT82C686=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
index 8997854..0ef3f09 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -33,3 +33,4 @@ CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_VT82C686=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak
index f18f3fc..6089318 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -35,3 +35,4 @@ CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_VT82C686=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak
index 6281f48..cd59e24 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -33,3 +33,4 @@ CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_VT82C686=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 73e4cc5..eac0b28 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -46,3 +46,4 @@ CONFIG_E500=y
 CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index 6d1933b..0164443 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -49,3 +49,4 @@ CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
 CONFIG_XICS=$(CONFIG_PSERIES)
 # For PReP
 CONFIG_MC146818RTC=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
index e3b5e50..86080a7 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -41,3 +41,4 @@ CONFIG_E500=y
 CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index ef963ee..bea21ac 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -8,3 +8,4 @@ CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/sh4eb-softmmu.mak b/default-configs/sh4eb-softmmu.mak
index e744486..2fd973e 100644
--- a/default-configs/sh4eb-softmmu.mak
+++ b/default-configs/sh4eb-softmmu.mak
@@ -8,3 +8,4 @@ CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
+CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak
index 9b08ee8..299c97b 100644
--- a/default-configs/sparc64-softmmu.mak
+++ b/default-configs/sparc64-softmmu.mak
@@ -15,3 +15,4 @@ CONFIG_IDE_ISA=y
 CONFIG_IDE_CMD646=y
 CONFIG_PCI_APB=y
 CONFIG_MC146818RTC=y
+CONFIG_ISA_TESTDEV=y
-- 
1.8.1.4

[Qemu-devel] [PATCH 22/28] default-configs: add SuperIO to SH4

[Paolo Bonzini]

The device provides an ISA bus to run the endianness test on.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 default-configs/sh4-softmmu.mak   | 7 +++++++
 default-configs/sh4eb-softmmu.mak | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index bea21ac..8e00390 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -9,3 +9,10 @@ CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
 CONFIG_ISA_TESTDEV=y
+CONFIG_I82378=y
+CONFIG_I8259=y
+CONFIG_I8254=y
+CONFIG_PCSPK=y
+CONFIG_I82374=y
+CONFIG_I8257=y
+CONFIG_MC146818RTC=y
diff --git a/default-configs/sh4eb-softmmu.mak b/default-configs/sh4eb-softmmu.mak
index 2fd973e..efdd058 100644
--- a/default-configs/sh4eb-softmmu.mak
+++ b/default-configs/sh4eb-softmmu.mak
@@ -9,3 +9,10 @@ CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
 CONFIG_ISA_TESTDEV=y
+CONFIG_I82378=y
+CONFIG_I8259=y
+CONFIG_I8254=y
+CONFIG_PCSPK=y
+CONFIG_I82374=y
+CONFIG_I8257=y
+CONFIG_MC146818RTC=y
-- 
1.8.1.4

[Qemu-devel] [PATCH 23/28] default-configs/ppc64: add all components of i82378 SuperIO chip used by prep

[Paolo Bonzini]

The device provides an ISA bus so that pseries can also run the
endianness test.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 default-configs/ppc64-softmmu.mak | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index 0164443..7831c2b 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -48,5 +48,11 @@ CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM))
 # For pSeries
 CONFIG_XICS=$(CONFIG_PSERIES)
 # For PReP
+CONFIG_I82378=y
+CONFIG_I8259=y
+CONFIG_I8254=y
+CONFIG_PCSPK=y
+CONFIG_I82374=y
+CONFIG_I8257=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
-- 
1.8.1.4

[Qemu-devel] [PATCH 24/28] qtest: add test for ISA I/O space endianness

[Paolo Bonzini]

This writes a register and reads its 1/2/4 byte parts.  Masking
is done in the device model.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/Makefile          |  14 +++-
 tests/endianness-test.c | 214 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+), 2 deletions(-)
 create mode 100644 tests/endianness-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 279d5f8..ebfe9eb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -52,7 +52,8 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
 # All QTests for now are POSIX-only, but the dependencies are
 # really in libqtest, not in the testcases themselves.
-check-qtest-i386-y = tests/fdc-test$(EXESUF)
+check-qtest-i386-y = tests/endianness-test$(EXESUF)
+check-qtest-i386-y += tests/fdc-test$(EXESUF)
 gcov-files-i386-y = hw/fdc.c
 check-qtest-i386-y += tests/ide-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
@@ -63,8 +64,16 @@ check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
+check-qtest-mips-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+check-qtest-ppc-y = tests/endianness-test$(EXESUF)
+check-qtest-ppc64-y = tests/endianness-test$(EXESUF)
+check-qtest-sh4-y = tests/endianness-test$(EXESUF)
+check-qtest-sh4eb-y = tests/endianness-test$(EXESUF)
+check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 #check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
-#check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
+#check-qtest-sparc64-y += tests/m48t59-test$(EXESUF)
 gcov-files-sparc-y += hw/m48t59.c
 gcov-files-sparc64-y += hw/m48t59.c
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
@@ -131,6 +140,7 @@ libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
+tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/fdc-test$(EXESUF): tests/fdc-test.o
 tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
diff --git a/tests/endianness-test.c b/tests/endianness-test.c
new file mode 100644
index 0000000..ca66645
--- /dev/null
+++ b/tests/endianness-test.c
@@ -0,0 +1,214 @@
+/*
+ * QTest testcase for ISA endianness
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Authors:
+ *  Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "libqtest.h"
+
+#include <glib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "qemu/bswap.h"
+
+typedef struct TestCase TestCase;
+struct TestCase {
+    const char *arch;
+    const char *machine;
+    uint64_t isa_base;
+    bool bswap;
+    const char *superio;
+};
+
+static const TestCase test_cases[] = {
+    { "i386", "pc", -1 },
+    { "mips", "magnum", 0x90000000, .bswap = true },
+    { "mips", "pica61", 0x90000000, .bswap = true },
+    { "mips", "mips", 0x14000000, .bswap = true },
+    { "mips", "malta", 0x10000000, .bswap = true },
+    { "mips64", "magnum", 0x90000000, .bswap = true },
+    { "mips64", "pica61", 0x90000000, .bswap = true },
+    { "mips64", "mips", 0x14000000, .bswap = true },
+    { "mips64", "malta", 0x10000000, .bswap = true },
+    { "mips64el", "fulong2e", 0x1fd00000 },
+    { "ppc", "g3beige", 0xfe000000, .bswap = true, .superio = "i82378" },
+    { "ppc", "prep", 0x80000000, .bswap = true },
+    { "ppc", "bamboo", 0xe8000000, .bswap = true, .superio = "i82378" },
+    { "ppc64", "mac99", 0xf2000000, .bswap = true, .superio = "i82378" },
+    { "ppc64", "pseries", 0x10080000000, .bswap = true, .superio = "i82378" },
+    { "sh4", "r2d", 0xfe240000, .superio = "i82378" },
+    { "sh4eb", "r2d", 0xfe240000, .bswap = true, .superio = "i82378" },
+    { "sparc64", "sun4u", 0x1fe02000000LL, .bswap = true },
+    { "x86_64", "pc", -1 },
+    {}
+};
+
+static uint8_t isa_inb(const TestCase *test, uint16_t addr)
+{
+    uint8_t value;
+    if (test->isa_base == -1) {
+        value = inb(addr);
+    } else {
+        value = readb(test->isa_base + addr);
+    }
+    return value;
+}
+
+static uint16_t isa_inw(const TestCase *test, uint16_t addr)
+{
+    uint16_t value;
+    if (test->isa_base == -1) {
+        value = inw(addr);
+    } else {
+        value = readw(test->isa_base + addr);
+    }
+    return test->bswap ? bswap16(value) : value;
+}
+
+static uint32_t isa_inl(const TestCase *test, uint16_t addr)
+{
+    uint32_t value;
+    if (test->isa_base == -1) {
+        value = inl(addr);
+    } else {
+        value = readl(test->isa_base + addr);
+    }
+    return test->bswap ? bswap32(value) : value;
+}
+
+static void isa_outb(const TestCase *test, uint16_t addr, uint8_t value)
+{
+    if (test->isa_base == -1) {
+        outb(addr, value);
+    } else {
+        writeb(test->isa_base + addr, value);
+    }
+}
+
+static void isa_outw(const TestCase *test, uint16_t addr, uint16_t value)
+{
+    value = test->bswap ? bswap16(value) : value;
+    if (test->isa_base == -1) {
+        outw(addr, value);
+    } else {
+        writew(test->isa_base + addr, value);
+    }
+}
+
+static void isa_outl(const TestCase *test, uint16_t addr, uint32_t value)
+{
+    value = test->bswap ? bswap32(value) : value;
+    if (test->isa_base == -1) {
+        outl(addr, value);
+    } else {
+        writel(test->isa_base + addr, value);
+    }
+}
+
+
+static void test_endianness(gconstpointer data)
+{
+    const TestCase *test = data;
+    char *args;
+
+    args = g_strdup_printf("-display none -M %s%s%s -device pc-testdev",
+                           test->machine,
+                           test->superio ? " -device " : "",
+                           test->superio ?: "");
+    qtest_start(args);
+    isa_outl(test, 0xe0, 0x87654321);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x87);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x65);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x43);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x21);
+
+    isa_outw(test, 0xe2, 0x8866);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x88664321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x88);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x66);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x43);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x21);
+
+    isa_outw(test, 0xe0, 0x4422);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x88664422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4422);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x88);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x66);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x44);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x22);
+
+    isa_outb(test, 0xe3, 0x87);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87664422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8766);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x87);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x66);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x44);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x22);
+
+    isa_outb(test, 0xe2, 0x65);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4422);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x87);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x65);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x44);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x22);
+
+    isa_outb(test, 0xe1, 0x43);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654322);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4322);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x87);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x65);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x43);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x22);
+
+    isa_outb(test, 0xe0, 0x21);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+    g_assert_cmphex(isa_inb(test, 0xe3), ==, 0x87);
+    g_assert_cmphex(isa_inb(test, 0xe2), ==, 0x65);
+    g_assert_cmphex(isa_inb(test, 0xe1), ==, 0x43);
+    g_assert_cmphex(isa_inb(test, 0xe0), ==, 0x21);
+    qtest_quit(global_qtest);
+    g_free(args);
+}
+
+int main(int argc, char **argv)
+{
+    const char *arch = qtest_get_arch();
+    int ret;
+    int i;
+
+    g_test_init(&argc, &argv, NULL);
+
+    for (i = 0; test_cases[i].arch; i++) {
+        gchar *path;
+        if (strcmp(test_cases[i].arch, arch) != 0) {
+            continue;
+        }
+        path = g_strdup_printf("/%s/endianness/%s",
+                               arch, test_cases[i].machine);
+        g_test_add_data_func(path, &test_cases[i], test_endianness);
+    }
+
+    ret = g_test_run();
+
+    return ret;
+}
-- 
1.8.1.4

[Qemu-devel] [PATCH 25/28] memory: move functions around

[Paolo Bonzini]

Prepare for next patch, no semantic change.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 60 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/memory.c b/memory.c
index 34a088e..aaae10d 100644
--- a/memory.c
+++ b/memory.c
@@ -339,6 +339,36 @@ static void flatview_simplify(FlatView *view)
     }
 }
 
+static bool memory_region_wrong_endianness(MemoryRegion *mr)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+    return mr->ops->endianness == DEVICE_LITTLE_ENDIAN;
+#else
+    return mr->ops->endianness == DEVICE_BIG_ENDIAN;
+#endif
+}
+
+static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
+{
+    if (memory_region_wrong_endianness(mr)) {
+        switch (size) {
+        case 1:
+            break;
+        case 2:
+            *data = bswap16(*data);
+            break;
+        case 4:
+            *data = bswap32(*data);
+            break;
+        case 8:
+            *data = bswap64(*data);
+            break;
+        default:
+            abort();
+        }
+    }
+}
+
 static void memory_region_oldmmio_read_accessor(void *opaque,
                                                 hwaddr addr,
                                                 uint64_t *value,
@@ -786,15 +816,6 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr)
     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
 }
 
-static bool memory_region_wrong_endianness(MemoryRegion *mr)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
-    return mr->ops->endianness == DEVICE_LITTLE_ENDIAN;
-#else
-    return mr->ops->endianness == DEVICE_BIG_ENDIAN;
-#endif
-}
-
 void memory_region_init(MemoryRegion *mr,
                         Object *owner,
                         const char *name,
@@ -921,27 +942,6 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
     return data;
 }
 
-static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
-{
-    if (memory_region_wrong_endianness(mr)) {
-        switch (size) {
-        case 1:
-            break;
-        case 2:
-            *data = bswap16(*data);
-            break;
-        case 4:
-            *data = bswap32(*data);
-            break;
-        case 8:
-            *data = bswap64(*data);
-            break;
-        default:
-            abort();
-        }
-    }
-}
-
 static bool memory_region_dispatch_read(MemoryRegion *mr,
                                         hwaddr addr,
                                         uint64_t *pval,
-- 
1.8.1.4

[Qemu-devel] [PATCH 26/28] memory: pass MemoryRegion to access_with_adjusted_size

[Paolo Bonzini]

The accessors all use a MemoryRegion opaque value.  Avoid going
uselessly through void*.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	Needed in a previous (wrong) version of this series, and
	it's what Alexey tested.

 memory.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/memory.c b/memory.c
index aaae10d..77a34dd 100644
--- a/memory.c
+++ b/memory.c
@@ -369,28 +369,26 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
     }
 }
 
-static void memory_region_oldmmio_read_accessor(void *opaque,
+static void memory_region_oldmmio_read_accessor(MemoryRegion *mr,
                                                 hwaddr addr,
                                                 uint64_t *value,
                                                 unsigned size,
                                                 unsigned shift,
                                                 uint64_t mask)
 {
-    MemoryRegion *mr = opaque;
     uint64_t tmp;
 
     tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
     *value |= (tmp & mask) << shift;
 }
 
-static void memory_region_read_accessor(void *opaque,
+static void memory_region_read_accessor(MemoryRegion *mr,
                                         hwaddr addr,
                                         uint64_t *value,
                                         unsigned size,
                                         unsigned shift,
                                         uint64_t mask)
 {
-    MemoryRegion *mr = opaque;
     uint64_t tmp;
 
     if (mr->flush_coalesced_mmio) {
@@ -400,28 +398,26 @@ static void memory_region_read_accessor(void *opaque,
     *value |= (tmp & mask) << shift;
 }
 
-static void memory_region_oldmmio_write_accessor(void *opaque,
+static void memory_region_oldmmio_write_accessor(MemoryRegion *mr,
                                                  hwaddr addr,
                                                  uint64_t *value,
                                                  unsigned size,
                                                  unsigned shift,
                                                  uint64_t mask)
 {
-    MemoryRegion *mr = opaque;
     uint64_t tmp;
 
     tmp = (*value >> shift) & mask;
     mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
 }
 
-static void memory_region_write_accessor(void *opaque,
+static void memory_region_write_accessor(MemoryRegion *mr,
                                          hwaddr addr,
                                          uint64_t *value,
                                          unsigned size,
                                          unsigned shift,
                                          uint64_t mask)
 {
-    MemoryRegion *mr = opaque;
     uint64_t tmp;
 
     if (mr->flush_coalesced_mmio) {
@@ -436,13 +432,13 @@ static void access_with_adjusted_size(hwaddr addr,
                                       unsigned size,
                                       unsigned access_size_min,
                                       unsigned access_size_max,
-                                      void (*access)(void *opaque,
+                                      void (*access)(MemoryRegion *mr,
                                                      hwaddr addr,
                                                      uint64_t *value,
                                                      unsigned size,
                                                      unsigned shift,
                                                      uint64_t mask),
-                                      void *opaque)
+                                      MemoryRegion *mr)
 {
     uint64_t access_mask;
     unsigned access_size;
@@ -460,10 +456,10 @@ static void access_with_adjusted_size(hwaddr addr,
     access_mask = -1ULL >> (64 - access_size * 8);
     for (i = 0; i < size; i += access_size) {
 #ifdef TARGET_WORDS_BIGENDIAN
-        access(opaque, addr + i, value, access_size,
+        access(mr, addr + i, value, access_size,
                (size - access_size - i) * 8, access_mask);
 #else
-        access(opaque, addr + i, value, access_size, i * 8, access_mask);
+        access(mr, addr + i, value, access_size, i * 8, access_mask);
 #endif
     }
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 27/28] memory: check memory region endianness, not target's

[Paolo Bonzini]

When combining multiple accesses into a single value, we need to do so
in the device's desired endianness.  The target endianness does not have
any influence.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/memory.c b/memory.c
index 77a34dd..b9139c5 100644
--- a/memory.c
+++ b/memory.c
@@ -339,6 +339,15 @@ static void flatview_simplify(FlatView *view)
     }
 }
 
+static bool memory_region_big_endian(MemoryRegion *mr)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+    return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
+#else
+    return mr->ops->endianness == DEVICE_BIG_ENDIAN;
+#endif
+}
+
 static bool memory_region_wrong_endianness(MemoryRegion *mr)
 {
 #ifdef TARGET_WORDS_BIGENDIAN
@@ -454,13 +463,15 @@ static void access_with_adjusted_size(hwaddr addr,
     /* FIXME: support unaligned access? */
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     access_mask = -1ULL >> (64 - access_size * 8);
-    for (i = 0; i < size; i += access_size) {
-#ifdef TARGET_WORDS_BIGENDIAN
-        access(mr, addr + i, value, access_size,
-               (size - access_size - i) * 8, access_mask);
-#else
-        access(mr, addr + i, value, access_size, i * 8, access_mask);
-#endif
+    if (memory_region_big_endian(mr)) {
+        for (i = 0; i < size; i += access_size) {
+            access(mr, addr + i, value, access_size,
+                   (size - access_size - i) * 8, access_mask);
+        }
+    } else {
+        for (i = 0; i < size; i += access_size) {
+            access(mr, addr + i, value, access_size, i * 8, access_mask);
+        }
     }
 }
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 28/28] pc-testdev: add I/O port to test memory.c auto split/combine

[Paolo Bonzini]

The ports at 0xe8..0xeb have impl.min/max_access_size == 1, so
that memory accesses are split and combined by the memory core.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/misc/pc-testdev.c    |  15 +++++++
 tests/endianness-test.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/hw/misc/pc-testdev.c b/hw/misc/pc-testdev.c
index ad1aa66..5867c70 100644
--- a/hw/misc/pc-testdev.c
+++ b/hw/misc/pc-testdev.c
@@ -49,6 +49,7 @@ typedef struct PCTestdev {
     ISADevice parent_obj;
 
     MemoryRegion ioport;
+    MemoryRegion ioport_byte;
     MemoryRegion flush;
     MemoryRegion irq;
     MemoryRegion iomem;
@@ -102,6 +103,16 @@ static const MemoryRegionOps test_ioport_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static const MemoryRegionOps test_ioport_byte_ops = {
+    .read = test_ioport_read,
+    .write = test_ioport_write,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 1,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
 static void test_flush_page(void *opaque, hwaddr addr, uint64_t data,
                             unsigned len)
 {
@@ -156,6 +167,9 @@ static void testdev_realizefn(DeviceState *d, Error **errp)
 
     memory_region_init_io(&dev->ioport, OBJECT(dev), &test_ioport_ops, dev,
                           "pc-testdev-ioport", 4);
+    memory_region_init_io(&dev->ioport_byte, OBJECT(dev),
+                          &test_ioport_byte_ops, dev,
+                          "pc-testdev-ioport-byte", 4);
     memory_region_init_io(&dev->flush, OBJECT(dev), &test_flush_ops, dev,
                           "pc-testdev-flush-page", 4);
     memory_region_init_io(&dev->irq, OBJECT(dev), &test_irq_ops, dev,
@@ -165,6 +179,7 @@ static void testdev_realizefn(DeviceState *d, Error **errp)
 
     memory_region_add_subregion(io,  0xe0,       &dev->ioport);
     memory_region_add_subregion(io,  0xe4,       &dev->flush);
+    memory_region_add_subregion(io,  0xe8,       &dev->ioport_byte);
     memory_region_add_subregion(io,  0x2000,     &dev->irq);
     memory_region_add_subregion(mem, 0xff000000, &dev->iomem);
 }
diff --git a/tests/endianness-test.c b/tests/endianness-test.c
index ca66645..feb32a8 100644
--- a/tests/endianness-test.c
+++ b/tests/endianness-test.c
@@ -190,6 +190,100 @@ static void test_endianness(gconstpointer data)
     g_free(args);
 }
 
+static void test_endianness_split(gconstpointer data)
+{
+    const TestCase *test = data;
+    char *args;
+
+    args = g_strdup_printf("-display none -M %s%s%s -device pc-testdev",
+                           test->machine,
+                           test->superio ? " -device " : "",
+                           test->superio ?: "");
+    qtest_start(args);
+    isa_outl(test, 0xe8, 0x87654321);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+
+    isa_outw(test, 0xea, 0x8866);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x88664321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+
+    isa_outw(test, 0xe8, 0x4422);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x88664422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4422);
+
+    isa_outb(test, 0xeb, 0x87);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87664422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8766);
+
+    isa_outb(test, 0xea, 0x65);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654422);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4422);
+
+    isa_outb(test, 0xe9, 0x43);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654322);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4322);
+
+    isa_outb(test, 0xe8, 0x21);
+    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
+    qtest_quit(global_qtest);
+    g_free(args);
+}
+
+static void test_endianness_combine(gconstpointer data)
+{
+    const TestCase *test = data;
+    char *args;
+
+    args = g_strdup_printf("-display none -M %s%s%s -device pc-testdev",
+                           test->machine,
+                           test->superio ? " -device " : "",
+                           test->superio ?: "");
+    qtest_start(args);
+    isa_outl(test, 0xe0, 0x87654321);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4321);
+
+    isa_outw(test, 0xe2, 0x8866);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x88664321);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4321);
+
+    isa_outw(test, 0xe0, 0x4422);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x88664422);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8866);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4422);
+
+    isa_outb(test, 0xe3, 0x87);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87664422);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8766);
+
+    isa_outb(test, 0xe2, 0x65);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87654422);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4422);
+
+    isa_outb(test, 0xe1, 0x43);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87654322);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4322);
+
+    isa_outb(test, 0xe0, 0x21);
+    g_assert_cmphex(isa_inl(test, 0xe8), ==, 0x87654321);
+    g_assert_cmphex(isa_inw(test, 0xea), ==, 0x8765);
+    g_assert_cmphex(isa_inw(test, 0xe8), ==, 0x4321);
+    qtest_quit(global_qtest);
+    g_free(args);
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -206,6 +300,14 @@ int main(int argc, char **argv)
         path = g_strdup_printf("/%s/endianness/%s",
                                arch, test_cases[i].machine);
         g_test_add_data_func(path, &test_cases[i], test_endianness);
+
+        path = g_strdup_printf("/%s/endianness/split/%s",
+                               arch, test_cases[i].machine);
+        g_test_add_data_func(path, &test_cases[i], test_endianness_split);
+
+        path = g_strdup_printf("/%s/endianness/combine/%s",
+                               arch, test_cases[i].machine);
+        g_test_add_data_func(path, &test_cases[i], test_endianness_combine);
     }
 
     ret = g_test_run();
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Peter Maydell]

On 22 July 2013 14:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This series drops isa_mmio and replace it with an alias of the
> root I/O memory region.  After applying back the LITTLE_ENDIAN
> mark for PortioLists, everything works as expected: the port memory
> regions appear directly in the FlatView with the right endianness,
> and MMIO is dispatched straight to them.

In the long term it would be good to identify which boards
were using isa_mmio purely for the benefit of old_portio
(which I think is basically "boards where the CPU has no
concept of port I/O instructions"). In these cases there
should be nobody calling cpu_in/out* any more, and so the
'system io space' returned by get_system_io() has devolved
to just being used as a container corresponding to the
PCI IO address space (which is then mapped into the
system MMIO space somewhere). This can be replaced by
having the PCI bridge device create a container space
specifically to be the IO address space. (This is how
hw/pci-host/versatile.c does it, for example).

That's longer term and not 1.6 material though.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 22/07/2013 16:32, Peter Maydell ha scritto:
> On 22 July 2013 14:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> This series drops isa_mmio and replace it with an alias of the
>> root I/O memory region.  After applying back the LITTLE_ENDIAN
>> mark for PortioLists, everything works as expected: the port memory
>> regions appear directly in the FlatView with the right endianness,
>> and MMIO is dispatched straight to them.
> 
> In the long term it would be good to identify which boards
> were using isa_mmio purely for the benefit of old_portio
> (which I think is basically "boards where the CPU has no
> concept of port I/O instructions").

All of them.  Only i386/x86_64 has I/O space, as far as I know.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Peter Maydell]

On 22 July 2013 15:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 22/07/2013 16:32, Peter Maydell ha scritto:
>> In the long term it would be good to identify which boards
>> were using isa_mmio purely for the benefit of old_portio
>> (which I think is basically "boards where the CPU has no
>> concept of port I/O instructions").
>
> All of them.  Only i386/x86_64 has I/O space, as far as I know.

Sounds plausible. I had wondered if our ISA bus infrastructure
assumed that ISA device IO ports live in the system IO space,
but it doesn't.

Side question: should the ICH9 emulation ich9_lpc_initfn()
really put its ISA bus in the system IO space rather than
in the pci_address_space_io() ?

-- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 22/07/2013 17:04, Peter Maydell ha scritto:
> On 22 July 2013 15:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 22/07/2013 16:32, Peter Maydell ha scritto:
>>> In the long term it would be good to identify which boards
>>> were using isa_mmio purely for the benefit of old_portio
>>> (which I think is basically "boards where the CPU has no
>>> concept of port I/O instructions").
>>
>> All of them.  Only i386/x86_64 has I/O space, as far as I know.
> 
> Sounds plausible. I had wondered if our ISA bus infrastructure
> assumed that ISA device IO ports live in the system IO space,
> but it doesn't.

No, luckily it doesn't, and neither should the PCI-to-ISA bridges as you
found out---they do not after these patches.  PReP is an exception, but
I think it could be rewritten to use an IOMMU memory region.

> Side question: should the ICH9 emulation ich9_lpc_initfn()
> really put its ISA bus in the system IO space rather than
> in the pci_address_space_io() ?

No, I think it should use pci_address_space_io().  PCI devices (the ISA
bridge is one) should not know that a system IO space exists, only the
PCI host bridge has that knowledge.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Hervé Poussineau]

Paolo Bonzini a écrit :
> Il 22/07/2013 17:04, Peter Maydell ha scritto:
>> On 22 July 2013 15:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 22/07/2013 16:32, Peter Maydell ha scritto:
>>>> In the long term it would be good to identify which boards
>>>> were using isa_mmio purely for the benefit of old_portio
>>>> (which I think is basically "boards where the CPU has no
>>>> concept of port I/O instructions").
>>> All of them.  Only i386/x86_64 has I/O space, as far as I know.
>> Sounds plausible. I had wondered if our ISA bus infrastructure
>> assumed that ISA device IO ports live in the system IO space,
>> but it doesn't.
> 
> No, luckily it doesn't, and neither should the PCI-to-ISA bridges as you
> found out---they do not after these patches.  PReP is an exception, but
> I think it could be rewritten to use an IOMMU memory region.

PReP PCI I/O area is located at 0x80000000, up to 0xbf7fffff (in main 
memory space region), while ISA I/O area is at 0x80000000, up to 
0x8000ffff (size=64KB)

However, as they are overlapped, some strange things can happen.
For example, IBM 40p firmware configures the PCI SCSI bar at 0x20000000 
(ie 0xa0000000 in main memory), while Linux sets bar to 0x1000 (ie 
0x80001000 in main memory), ie also in ISA I/O space.

I don't know exactly what you mean by an "IOMMU memory region", but how 
would you modelize it, so that 0x80001000 and 0xa0000000 accesses are 
redirected to PCI SCSI card, while 0x800003f8 redirects (for example) to 
an ISA serial port?
If you create a new memory region for ISA I/O space, and you redirect 
all accesses from 0x80000000-0x8000ffff to this new address space, 
0x80001000 won't work to access the SCSI I/O bar (located in the PCI I/O 
address space).

That's why I think the i82378 device should not create a whole new 
address space for ISA I/O space, but use the first 64KB of the PCI I/O 
space.

BTW, I've a patch to really cleanup i82378 implementation (47 
insertions, 175 deletions). Should I send it now, during 1.6 soft freeze?

Hervé

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Alexander Graf]

On 22.07.2013, at 22:16, Hervé Poussineau wrote:

> Paolo Bonzini a écrit :
>> Il 22/07/2013 17:04, Peter Maydell ha scritto:
>>> On 22 July 2013 15:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> Il 22/07/2013 16:32, Peter Maydell ha scritto:
>>>>> In the long term it would be good to identify which boards
>>>>> were using isa_mmio purely for the benefit of old_portio
>>>>> (which I think is basically "boards where the CPU has no
>>>>> concept of port I/O instructions").
>>>> All of them.  Only i386/x86_64 has I/O space, as far as I know.
>>> Sounds plausible. I had wondered if our ISA bus infrastructure
>>> assumed that ISA device IO ports live in the system IO space,
>>> but it doesn't.
>> No, luckily it doesn't, and neither should the PCI-to-ISA bridges as you
>> found out---they do not after these patches.  PReP is an exception, but
>> I think it could be rewritten to use an IOMMU memory region.
> 
> PReP PCI I/O area is located at 0x80000000, up to 0xbf7fffff (in main memory space region), while ISA I/O area is at 0x80000000, up to 0x8000ffff (size=64KB)

Are you sure the ISA I/O space isn't just the first part of the PCI ioport range?

> 
> However, as they are overlapped, some strange things can happen.
> For example, IBM 40p firmware configures the PCI SCSI bar at 0x20000000 (ie 0xa0000000 in main memory), while Linux sets bar to 0x1000 (ie 0x80001000 in main memory), ie also in ISA I/O space.
> 
> I don't know exactly what you mean by an "IOMMU memory region", but how would you modelize it, so that 0x80001000 and 0xa0000000 accesses are redirected to PCI SCSI card, while 0x800003f8 redirects (for example) to an ISA serial port?

From what you're saying they both really live on the same address range.

> If you create a new memory region for ISA I/O space, and you redirect all accesses from 0x80000000-0x8000ffff to this new address space, 0x80001000 won't work to access the SCSI I/O bar (located in the PCI I/O address space).
> 
> That's why I think the i82378 device should not create a whole new address space for ISA I/O space, but use the first 64KB of the PCI I/O space.

Exactly :). Or the other way around really, if that's easier to model.


Alex

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Andreas Färber]

Am 22.07.2013 22:16, schrieb Hervé Poussineau:
> BTW, I've a patch to really cleanup i82378 implementation (47
> insertions, 175 deletions). Should I send it now, during 1.6 soft freeze?

Yes please, since it's a PReP-only device and relevant to the general
discussion.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 22/07/2013 22:16, Hervé Poussineau ha scritto:
>> PReP is an exception, but
>> I think it could be rewritten to use an IOMMU memory region.
> 
> PReP PCI I/O area is located at 0x80000000, up to 0xbf7fffff (in main
> memory space region), while ISA I/O area is at 0x80000000, up to
> 0x8000ffff (size=64KB)

or up to 0x807ffffff for non-contiguous mode.  The IOMMU memory region
would let you implement non-contiguous mode without calling
cpu_inb/cpu_outb.

> However, as they are overlapped, some strange things can happen.
> For example, IBM 40p firmware configures the PCI SCSI bar at 0x20000000
> (ie 0xa0000000 in main memory), while Linux sets bar to 0x1000 (ie
> 0x80001000 in main memory), ie also in ISA I/O space.

If BARs have a lower priority than assigned ISA I/O space, this should
"just work" if you use an alias memory region for ISA I/O space.  Gaps
in the ISA I/O space will let you see through the ISA I/O space and
access BARs.

If BARs have a higher priority, you need to set the priority accordingly
for the ISA I/O space alias (using memory_region_add_subregion_overlap),
but that's it.

By the way, i82378.c can also use memory_region_init_alias to initialize
the memory regions that are passed to pci_register_bar.  I didn't do
this in this series.

> I don't know exactly what you mean by an "IOMMU memory region", but how
> would you modelize it, so that 0x80001000 and 0xa0000000 accesses are
> redirected to PCI SCSI card, while 0x800003f8 redirects (for example) to
> an ISA serial port?

See above.

Paolo

> If you create a new memory region for ISA I/O space, and you redirect
> all accesses from 0x80000000-0x8000ffff to this new address space,
> 0x80001000 won't work to access the SCSI I/O bar (located in the PCI I/O
> address space).

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> This series fixes the I/O port endianness problems that came when
> port I/O was made to go through the normal dispatch path.  Targets
> that used isa_mmio to invoke cpu_inl and friends now tried to do
> byte swapping once more than they previously did, causing garbage
> to be written.
>
> This series drops isa_mmio and replace it with an alias of the
> root I/O memory region.  After applying back the LITTLE_ENDIAN
> mark for PortioLists, everything works as expected: the port memory
> regions appear directly in the FlatView with the right endianness,
> and MMIO is dispatched straight to them.
>
> PReP cannot use this due to the non-contiguous map, so we just
> make sure the prep_io_ops are marked DEVICE_NATIVE_ENDIAN; the swap
> will happen when prep_io_ops redispatch to the device.
>
> So we had a change that broke multiple ports, most of them
> under-maintained.  What better occasion to add a unit test?  This
> series does that, using the pc-testdev device as the harness.
> The test was run before and after the I/O changes (in particular
> on top of commit a014ed0, memory: accept mismatching sizes in
> memory_region_access_valid, 2013-05-24), and passes there too.
>
> This also found some unrelated problems caused by the new QOM cast in
> commit 7588e2b (pci: Fold host_buses list into PCIHostState functionality,
> 2013-06-06).
>
> I also smoke-tested prep, with the kernel that Herve provided, and
> pseries.  VGA works for both, which tests the PortioList path.
> Alexey tested Linux (with virtio) on pseries.
>
> Patches 1 to 15 fix all bugs and remove isa_mmio.
>
> Patches 16 to 23 prepare the boards so that the testcase runs
> successfully.  This includes adding missing devices and unbreaking
> other breakage.
>
> Patch 24 adds the test case, patches 25-27 fix big-endian split/combine,
> patch 28 adds a test case for that too.
>
> If preferred, I can prepare a pull request that merges the tests
> from an earlier branchpoint.  This makes it easy to run the tests
> on both the old and the new code.
>
> Paolo

Really nice series.  I'd prefer we simply got rid of the endianness flag
entirely but this is a good step.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> Alexey Kardashevskiy (1):
>   spapr_pci: remove indirection for I/O port access
>
> Paolo Bonzini (27):
>   sh4: do not use isa_mmio
>   ppc_oldworld: do not use isa_mmio
>   ppc_newworld: do not use isa_mmio
>   prep: fix I/O port endianness
>   mips_jazz: do not use isa_mmio
>   mips_r4k: do not use isa_mmio
>   mips_malta: do not use isa_mmio
>   ppc440_bamboo: do not use isa_mmio
>   mipssim: do not use isa_mmio
>   mips_fulong2e: do not use isa_mmio
>   sparc64: remove indirection for I/O port access
>   ebus: do not use isa_mmio
>   isa_mmio: delete
>   Revert "ioport: remove LITTLE_ENDIAN mark for portio"
>   pc-testdev: support 8 and 16-bit accesses to 0xe0
>   pc-testdev: remove useless cpu_to_le64/le64_to_cpu
>   mips: degrade BIOS error to warning
>   sh4: unbreak r2d
>   sparc64: unbreak
>   default-configs: add test device to all machines supporting ISA
>   default-configs: add SuperIO to SH4
>   default-configs/ppc64: add all components of i82378 SuperIO chip used
>     by prep
>   qtest: add test for ISA I/O space endianness
>   memory: move functions around
>   memory: pass MemoryRegion to access_with_adjusted_size
>   memory: check memory region endianness, not target's
>   pc-testdev: add I/O port to test memory.c auto split/combine
>
>  default-configs/alpha-softmmu.mak    |   1 +
>  default-configs/mips-softmmu.mak     |   2 +-
>  default-configs/mips64-softmmu.mak   |   2 +-
>  default-configs/mips64el-softmmu.mak |   2 +-
>  default-configs/mipsel-softmmu.mak   |   2 +-
>  default-configs/ppc-softmmu.mak      |   1 +
>  default-configs/ppc64-softmmu.mak    |   7 +
>  default-configs/ppcemb-softmmu.mak   |   1 +
>  default-configs/sh4-softmmu.mak      |   9 +-
>  default-configs/sh4eb-softmmu.mak    |   9 +-
>  default-configs/sparc64-softmmu.mak  |   1 +
>  hw/isa/Makefile.objs                 |   1 -
>  hw/isa/isa_mmio.c                    |  81 ---------
>  hw/mips/gt64xxx_pci.c                |   3 +-
>  hw/mips/mips_fulong2e.c              |   3 +-
>  hw/mips/mips_jazz.c                  |   8 +-
>  hw/mips/mips_malta.c                 |   3 +-
>  hw/mips/mips_mipssim.c               |   8 +-
>  hw/mips/mips_r4k.c                   |   6 +-
>  hw/misc/pc-testdev.c                 |  28 +++-
>  hw/pci-host/apb.c                    | 101 ++++-------
>  hw/pci-host/bonito.c                 |  25 ++-
>  hw/ppc/mac_newworld.c                |   5 +-
>  hw/ppc/mac_oldworld.c                |   5 +-
>  hw/ppc/ppc440_bamboo.c               |   5 +-
>  hw/ppc/prep.c                        |   2 +-
>  hw/ppc/spapr_pci.c                   |  41 +----
>  hw/sh4/sh_pci.c                      |  42 +++--
>  hw/sparc64/sun4u.c                   |   6 +-
>  include/hw/isa/isa.h                 |   3 -
>  ioport.c                             |   1 +
>  memory.c                             | 101 +++++------
>  tests/Makefile                       |  14 +-
>  tests/endianness-test.c              | 316 +++++++++++++++++++++++++++++++++++
>  34 files changed, 543 insertions(+), 302 deletions(-)
>  delete mode 100644 hw/isa/isa_mmio.c
>  create mode 100644 tests/endianness-test.c
>
> -- 
> 1.8.1.4

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Benjamin Herrenschmidt]

On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
> 
> Really nice series.  I'd prefer we simply got rid of the endianness
> flag
> entirely but this is a good step.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Are you going to merge this ?

Afaik (Alexey just told me), pretty much anything IO is broken for
powerpc upstream and has been for weeks now ! It looks like the only
thing that got reverted was the VGA problem but everything else is still
busted including virtio.

Why hasn't the original breakage been reverted immediately instead ?

Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Benjamin Herrenschmidt]

On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
> > 
> > Really nice series.  I'd prefer we simply got rid of the endianness
> > flag
> > entirely but this is a good step.
> > 
> > Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> Are you going to merge this ?
> 
> Afaik (Alexey just told me), pretty much anything IO is broken for
> powerpc upstream and has been for weeks now ! It looks like the only
> thing that got reverted was the VGA problem but everything else is still
> busted including virtio.
> 
> Why hasn't the original breakage been reverted immediately instead ?

It's actually worse than I thought. Alexey is showing me that in fact,
even PCI MMIO is busted, using EHCI causes qemu to segfault for example.

This is a complete trainwreck. Why was that junk merged in the first
place and why wasn't it immediately reverted ?

Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

On 2013-07-25 07:47, Benjamin Herrenschmidt wrote:
> On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
>>>
>>> Really nice series.  I'd prefer we simply got rid of the endianness
>>> flag
>>> entirely but this is a good step.
>>>
>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Are you going to merge this ?
>>
>> Afaik (Alexey just told me), pretty much anything IO is broken for
>> powerpc upstream and has been for weeks now ! It looks like the only
>> thing that got reverted was the VGA problem but everything else is still
>> busted including virtio.
>>
>> Why hasn't the original breakage been reverted immediately instead ?
> 
> It's actually worse than I thought. Alexey is showing me that in fact,
> even PCI MMIO is busted, using EHCI causes qemu to segfault for example.

Can you be more specific? I suppose this is also on Power. Is it
unrelated to the endianness topic?

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Alexey Kardashevskiy]

On 07/25/2013 04:04 PM, Jan Kiszka wrote:
> On 2013-07-25 07:47, Benjamin Herrenschmidt wrote:
>> On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
>>> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
>>>>
>>>> Really nice series.  I'd prefer we simply got rid of the endianness
>>>> flag
>>>> entirely but this is a good step.
>>>>
>>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>> Are you going to merge this ?
>>>
>>> Afaik (Alexey just told me), pretty much anything IO is broken for
>>> powerpc upstream and has been for weeks now ! It looks like the only
>>> thing that got reverted was the VGA problem but everything else is still
>>> busted including virtio.
>>>
>>> Why hasn't the original breakage been reverted immediately instead ?
>>
>> It's actually worse than I thought. Alexey is showing me that in fact,
>> even PCI MMIO is busted, using EHCI causes qemu to segfault for example.
> 
> Can you be more specific? I suppose this is also on Power. Is it
> unrelated to the endianness topic?


Not sure what it is related to. It is fixed by "fix I/O port endianness
mess" series, at least Paolo's "iommu" branch does not have this problem.


Here is my command line:

./qemu-system-ppc64 -L "qemu-ppc64-bios/" -trace "events=qemu_trace_events"
-usb -device usb-ehci -usbdevice disk:format=qcow2:virtimg/fc19beta -m
"1024" -machine "pseries" -nographic -vga "none" -enable-kvm


Or without -enable-kvm, does not make any difference.

Here is what I get with the "master" branch from qemu.org:


[    0.313165] libphy: Fixed MDIO Bus: probed
[    0.313240] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    0.313311] ehci-pci: EHCI PCI platform driver
[    0.313889] ehci-pci 0000:00:01.0: EHCI Host Controller
[    0.313992] ehci-pci 0000:00:01.0: new USB bus registered, assigned bus
number 1

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x1fffffb8eef0 (LWP 32175)]
0x0000000010382408 in memory_region_oldmmio_write_accessor
(opaque=0x10c33a18, addr=0x9, value=0x1fffffb8e220,
    size=0x1, shift=0x0, mask=0xff) at
/home/alexey/pcipassthru/qemu-impreza/memory.c:384
warning: Source file is more recent than executable.
384	    mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
(gdb) p mr->ops->old_mmio
$3 = {read = {0x0, 0x0, 0x0}, write = {0x0, 0x0, 0x0}}
(gdb) bt
#0  0x0000000010382408 in memory_region_oldmmio_write_accessor
(opaque=0x10c33a18, addr=0x9, value=0x1fffffb8e220,
    size=0x1, shift=0x0, mask=0xff) at
/home/alexey/pcipassthru/qemu-impreza/memory.c:384
#1  0x0000000010382650 in access_with_adjusted_size (addr=0x9,
value=0x1fffffb8e220, size=0x1, access_size_min=0x1,
    access_size_max=0x4, access=@0x106a2a20: 0x1038235c
<memory_region_oldmmio_write_accessor>, opaque=0x10c33a18)
    at /home/alexey/pcipassthru/qemu-impreza/memory.c:433
#2  0x0000000010384ec8 in memory_region_dispatch_write (mr=0x10c33a18,
addr=0x9, data=0x0, size=0x1)
    at /home/alexey/pcipassthru/qemu-impreza/memory.c:978
#3  0x0000000010388508 in io_mem_write (mr=0x10c33a18, addr=0x9, val=0x0,
size=0x1)
    at /home/alexey/pcipassthru/qemu-impreza/memory.c:1737
#4  0x00000000102ebb40 in address_space_rw (as=0x10af9100
<address_space_memory>, addr=0x100b0001009,
    buf=0x1ffffffd0028 "", len=0x4, is_write=0x1) at
/home/alexey/pcipassthru/qemu-impreza/exec.c:1967
#5  0x00000000102ebfec in cpu_physical_memory_rw (addr=0x100b0001009,
buf=0x1ffffffd0028 "", len=0x4, is_write=0x1)
    at /home/alexey/pcipassthru/qemu-impreza/exec.c:2036
#6  0x000000001037f9d8 in kvm_cpu_exec (cpu=0x1fffffb90010) at
/home/alexey/pcipassthru/qemu-impreza/kvm-all.c:1673
#7  0x00000000102dba58 in qemu_kvm_cpu_thread_fn (arg=0x1fffffb90010)
    at /home/alexey/pcipassthru/qemu-impreza/cpus.c:785
#8  0x00000080c70cc29c in .start_thread () from /lib64/libpthread.so.0
#9  0x00000080c6fbd110 in .__clone () from /lib64/libc.so.6
(gdb)



-- 
Alexey

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 25/07/2013 08:04, Jan Kiszka ha scritto:
> On 2013-07-25 07:47, Benjamin Herrenschmidt wrote:
>> On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
>>> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
>>>> 
>>>> Really nice series.  I'd prefer we simply got rid of the
>>>> endianness flag entirely but this is a good step.
>>>> 
>>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>> 
>>> Are you going to merge this ?
>>> 
>>> Afaik (Alexey just told me), pretty much anything IO is broken
>>> for powerpc upstream and has been for weeks now ! It looks like
>>> the only thing that got reverted was the VGA problem but
>>> everything else is still busted including virtio.
>>> 
>>> Why hasn't the original breakage been reverted immediately
>>> instead ?
>> 
>> It's actually worse than I thought. Alexey is showing me that in
>> fact, even PCI MMIO is busted, using EHCI causes qemu to segfault
>> for example.
> 
> Can you be more specific? I suppose this is also on Power. Is it 
> unrelated to the endianness topic?

It is fixed by patches 25-27 in this series.

Paolo

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR8OSyAAoJEBvWZb6bTYbyuewP/inkxtkOWlASXpCo9Ca6FB30
wJ4E/4WMNrM8pmUxU3LNEpJpmZFiuuIdNIjo5119Vn6x3lamOfCTqzej07ZYBvzV
Rp1cxvt4VoU1QieEpxvvV2aH0rb9b0dQknPIMwLWPB+m5lWtuZI4ERgE5kHXjKXk
CHvuSJmJXHUAubZFsadcOqwsfjV+dEuz/Ibb3Lk0iCnYJsDDZ3uf07b3t5pRtXaY
rQJjo0mHfYYox+Fd1llUCrUpEEwtB5ljoQXzx5HmOuHuDO3hm85VSa4CHaYgL3Q1
oYO7tBn7iXNXZ1ExWCrqijS4zm89624WabGQ4SmnU3wMrs/GBQRRnSV1wGl+3l9X
70BUFHuE4mSkQfyAdW8aHIGIOXemwt+V8jKPKXE1I0jrLgco48e6ru2Jcb2YId/E
IlizpSXIbhGg2TcW4NOotdT8NlZo/Aoge7jsaW4g7s/kkYLBeiGSig0YmKBwCCCu
bEtApgtRi6izssHKOqgsNPtKXYplA6CLRj9NMOG9bUCIFO3ZQ5exj+WI5dDdlG3g
4jJ6SuwdHI8iOqf0a2PqGX8xwGDkuMVAIB0g16b9l8UZM456OVBBGCjcFn3/yNnA
R+R6DyFh6UuzDzCSPUYNQ3iB4SH8ZleDviH8jSKlqk979qZVp6zytIJ27hQWBYVl
KsENGYl17tQJb6UVD5Gy
=saO+
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 25/07/2013 07:47, Benjamin Herrenschmidt ha scritto:
> On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
>>>
>>> Really nice series.  I'd prefer we simply got rid of the endianness
>>> flag
>>> entirely but this is a good step.
>>>
>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Are you going to merge this ?

I guess so.

>> Afaik (Alexey just told me), pretty much anything IO is broken for
>> powerpc upstream and has been for weeks now ! It looks like the only
>> thing that got reverted was the VGA problem but everything else is still
>> busted including virtio.
>>
>> Why hasn't the original breakage been reverted immediately instead ?
> 
> It's actually worse than I thought. Alexey is showing me that in fact,
> even PCI MMIO is busted, using EHCI causes qemu to segfault for example.

The MMIO problems are unrelated.  They were reported by Nikunj in June
and they only happen with an unreleased version of SLOF that he's
working on (Linux works just fine).

Fixing it was on my todo list, then I had to do other stuff (like going
on holiday for 5 days and getting a thousand emails in your inbox) and I
prioritized the more general I/O port problem.  Anyhow the fix is in
patches 25-27 of this series, Nikunj just confirmed by testing them.

> This is a complete trainwreck. Why was that junk merged in the first
> place and why wasn't it immediately reverted ?

Perhaps you haven't noticed, but this list tends to have better social
standards than this.  Anyhow, here is an explanation.

The patches were absolutely not junk.  They removed a lot of messy old
code.  Unfortunately they didn't remove _enough_ messy old code.  This
series completes the work.

They were merged because:

1) we didn't know it broke all big endian targets.  There was some
suspicion that it broke pseries, but the idea was that removing the PCI
I/O hack would have fixed it.

2) In fact it did, but the dicussion derailed on endianness topics and
Alexey's patch to revert the PCI I/O hack wasn't accepted.

3) Sometimes we have to deal with old code written by people that aren't
contributors anymore and frankly no one understands it fully.  Breaking
stuff actually forces us to understand it and fix it the right way.  It
is not the first time it happens.


They weren't immediately reverted because:

4) the actual extent of the problem was not known.  Initial reports were
just about VGA, but that was not the case.

5) because we thought it was just about VGA, everybody thought that
Anthony's VGA fix would be the end of it.  (In the end Anthony's patch
was wrong and this series reverts it!)

6) the dicussion derailed on endianness topics and nobody posted a patch
to revert.


Fixing took a while because I originally thought I would have to rack
firmwares and images for all big-endian machine types.  Once it dawned
on me that I could just use a unit test, it took 5-6 hours to write the
testcase _and_ the fixes.

In any case, let me reinforce point 6 above.  The patches were not
reverted because nobody posted the patches to do so.  Until someone does
so, it is basically Anthony's call.  If he trusts people to fix the
whole thing, he has no reason to revert anything.  My track record of
fixing mess is probably better than my track record of not causing it,
so he didn't revert it.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Benjamin Herrenschmidt]

On Thu, 2013-07-25 at 10:40 +0200, Paolo Bonzini wrote:
> In any case, let me reinforce point 6 above.  The patches were not
> reverted because nobody posted the patches to do so.  Until someone
> does
> so, it is basically Anthony's call.  If he trusts people to fix the
> whole thing, he has no reason to revert anything.  My track record of
> fixing mess is probably better than my track record of not causing it,
> so he didn't revert it.

That's fine, I know you can fix stuff :-) I'm just really annoyed that
upstream qemu remained broken for so long (and still is) while the whole
thing derailed into a mostly pointless discussion on endianness and
nobody (including Alexey) hollered loud enough that the breakage was
fairly extensive

(BTW. The EHCI problem doesn't seem limited to SLOF, from what I
remember of what Alexey showed me today, the Linux EHCI driver also
segfaults qemu).

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Peter Maydell]

On 25 July 2013 10:00, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> That's fine, I know you can fix stuff :-) I'm just really annoyed that
> upstream qemu remained broken for so long (and still is) while the whole
> thing derailed into a mostly pointless discussion on endianness and
> nobody (including Alexey) hollered loud enough that the breakage was
> fairly extensive

I think that for the minor architectures we just have to make
sure that we do yell loudly when things are broken, because
the nature of things is that people won't notice. Maybe we should
have a qemu-urgent list to parallel qemu-trivial for compile
fixes, reversions of bad breakage, etc, to try to keep them
out of the general noise ?

-- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 25/07/2013 11:38, Peter Maydell ha scritto:
> On 25 July 2013 10:00, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>> That's fine, I know you can fix stuff :-) I'm just really annoyed that
>> upstream qemu remained broken for so long (and still is) while the whole
>> thing derailed into a mostly pointless discussion on endianness and
>> nobody (including Alexey) hollered loud enough that the breakage was
>> fairly extensive
> 
> I think that for the minor architectures we just have to make
> sure that we do yell loudly when things are broken, because
> the nature of things is that people won't notice. Maybe we should
> have a qemu-urgent list to parallel qemu-trivial for compile
> fixes, reversions of bad breakage, etc, to try to keep them
> out of the general noise ?

My impression was that Alexey was fine with working temporarily on an
older version of QEMU.  So reverting the patches would have meant more
work for everyone.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Benjamin Herrenschmidt]

On Thu, 2013-07-25 at 11:40 +0200, Paolo Bonzini wrote:
> > I think that for the minor architectures we just have to make
> > sure that we do yell loudly when things are broken, because
> > the nature of things is that people won't notice. Maybe we should
> > have a qemu-urgent list to parallel qemu-trivial for compile
> > fixes, reversions of bad breakage, etc, to try to keep them
> > out of the general noise ?
> 
> My impression was that Alexey was fine with working temporarily on an
> older version of QEMU.  So reverting the patches would have meant more
> work for everyone.

Possibly but we have plenty of other people who use upstream qemu. I've
been actively encouraging people in the community and distros to use
qemu for testing their stuff on powerpc without HW access for example,
and even internally to IBM, not everybody uses Alexey internal tree.

Talking of which ... I really really really need to take a day or two
off and use them to fix the Mac G5 model once and for all :-)

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Benjamin Herrenschmidt]

On Thu, 2013-07-25 at 10:38 +0100, Peter Maydell wrote:
> On 25 July 2013 10:00, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > That's fine, I know you can fix stuff :-) I'm just really annoyed that
> > upstream qemu remained broken for so long (and still is) while the whole
> > thing derailed into a mostly pointless discussion on endianness and
> > nobody (including Alexey) hollered loud enough that the breakage was
> > fairly extensive
> 
> I think that for the minor architectures we just have to make
> sure that we do yell loudly when things are broken, because
> the nature of things is that people won't notice. Maybe we should
> have a qemu-urgent list to parallel qemu-trivial for compile
> fixes, reversions of bad breakage, etc, to try to keep them
> out of the general noise ?

Or I teach Alexey to yell louder (along with some French :-)

Not sure if a mailing list is useful. A tag might be enough
[REGRESSION] ? [URGENT] ? Those kind of subject tags tend to stand out
pretty well on mailing lists.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 25/07/2013 12:23, Benjamin Herrenschmidt ha scritto:
> On Thu, 2013-07-25 at 10:38 +0100, Peter Maydell wrote:
>> On 25 July 2013 10:00, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>>> That's fine, I know you can fix stuff :-) I'm just really annoyed that
>>> upstream qemu remained broken for so long (and still is) while the whole
>>> thing derailed into a mostly pointless discussion on endianness and
>>> nobody (including Alexey) hollered loud enough that the breakage was
>>> fairly extensive
>>
>> I think that for the minor architectures we just have to make
>> sure that we do yell loudly when things are broken, because
>> the nature of things is that people won't notice. Maybe we should
>> have a qemu-urgent list to parallel qemu-trivial for compile
>> fixes, reversions of bad breakage, etc, to try to keep them
>> out of the general noise ?
> 
> Or I teach Alexey to yell louder (along with some French :-)
> 
> Not sure if a mailing list is useful. A tag might be enough
> [REGRESSION] ? [URGENT] ? Those kind of subject tags tend to stand out
> pretty well on mailing lists.

I've definitely seen them already in the past on qemu-devel, so I concur
that a mailing list is not useful.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 25/07/2013 12:23, Benjamin Herrenschmidt ha scritto:
>> On Thu, 2013-07-25 at 10:38 +0100, Peter Maydell wrote:
>>> On 25 July 2013 10:00, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>>>> That's fine, I know you can fix stuff :-) I'm just really annoyed that
>>>> upstream qemu remained broken for so long (and still is) while the whole
>>>> thing derailed into a mostly pointless discussion on endianness and
>>>> nobody (including Alexey) hollered loud enough that the breakage was
>>>> fairly extensive
>>>
>>> I think that for the minor architectures we just have to make
>>> sure that we do yell loudly when things are broken, because
>>> the nature of things is that people won't notice. Maybe we should
>>> have a qemu-urgent list to parallel qemu-trivial for compile
>>> fixes, reversions of bad breakage, etc, to try to keep them
>>> out of the general noise ?
>> 
>> Or I teach Alexey to yell louder (along with some French :-)
>> 
>> Not sure if a mailing list is useful. A tag might be enough
>> [REGRESSION] ? [URGENT] ? Those kind of subject tags tend to stand out
>> pretty well on mailing lists.
>
> I've definitely seen them already in the past on qemu-devel, so I concur
> that a mailing list is not useful.

This is a technical problem, not a social one.  One something is broken,
it's a hell of a lot harder to fix than it is to prevent something from
breaking in the first place.

If you want to prevent minor architecture regressions, add unit tests.

Regards,

Anthony Liguori

>
> Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Peter Maydell]

On 25 July 2013 14:25, Anthony Liguori <aliguori@us.ibm.com> wrote:
> If you want to prevent minor architecture regressions, add unit tests.

Unit tests won't help with the periodic "doesn't compile on 32 bit
systems" regressions, of course...

-- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini]

Il 25/07/2013 15:28, Peter Maydell ha scritto:
>> > If you want to prevent minor architecture regressions, add unit tests.
> Unit tests won't help with the periodic "doesn't compile on 32 bit
> systems" regressions, of course...

Buildbots (and having maintainers add buildbots for their trees) would.
 This was a resolution at the summit last year, but one that wasn't
implemented very well.

Paolo

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Peter Maydell]

On 25 July 2013 14:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 25/07/2013 15:28, Peter Maydell ha scritto:
>>> > If you want to prevent minor architecture regressions, add unit tests.
>> Unit tests won't help with the periodic "doesn't compile on 32 bit
>> systems" regressions, of course...
>
> Buildbots (and having maintainers add buildbots for their trees) would.

I've found that the buildbots are pretty useless for me as
a submaintainer. If there was a way I could say "ok, tree
is ready to go, kick off a build and send me the results
in an hour" then that would be useful. A 24 hour turnaround
means I've already sent the pullrequest before the buildbot
kicks off.

Also:
 * I just tried to check http://buildbot.b1-systems.de/qemu/grid
and I get a 504 Gateway Time-out error...
 * looking at the latest arm-devs build it's failed
configure because the buildbot doesn't either (a) have
a suitable libfdt or (b) have the appropriate command
to init the dtc submodule. I'm pretty sure I never got
an email about this, so I imagine it's been busted for
months.

-- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 July 2013 14:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 25/07/2013 15:28, Peter Maydell ha scritto:
>>>> > If you want to prevent minor architecture regressions, add unit tests.
>>> Unit tests won't help with the periodic "doesn't compile on 32 bit
>>> systems" regressions, of course...
>>
>> Buildbots (and having maintainers add buildbots for their trees) would.
>
> I've found that the buildbots are pretty useless for me as
> a submaintainer. If there was a way I could say "ok, tree
> is ready to go, kick off a build and send me the results
> in an hour" then that would be useful. A 24 hour turnaround
> means I've already sent the pullrequest before the buildbot
> kicks off.
>
> Also:
>  * I just tried to check http://buildbot.b1-systems.de/qemu/grid
> and I get a 504 Gateway Time-out error...
>  * looking at the latest arm-devs build it's failed
> configure because the buildbot doesn't either (a) have
> a suitable libfdt or (b) have the appropriate command
> to init the dtc submodule. I'm pretty sure I never got
> an email about this, so I imagine it's been busted for
> months.

Yes, we need to do something about the state of buildbot.  We need a
volunteer to more actively maintain it.  If anyone is interested please
let me know.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 July 2013 14:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 25/07/2013 15:28, Peter Maydell ha scritto:
>>>> > If you want to prevent minor architecture regressions, add unit tests.
>>> Unit tests won't help with the periodic "doesn't compile on 32 bit
>>> systems" regressions, of course...
>>
>> Buildbots (and having maintainers add buildbots for their trees) would.
>
> I've found that the buildbots are pretty useless for me as
> a submaintainer. If there was a way I could say "ok, tree
> is ready to go, kick off a build and send me the results
> in an hour" then that would be useful. A 24 hour turnaround
> means I've already sent the pullrequest before the buildbot
> kicks off.

It's still useful for me though because by the time I process it,
buildbot has had a chance to run.

Regards,

Anthony Liguori

>
> Also:
>  * I just tried to check http://buildbot.b1-systems.de/qemu/grid
> and I get a 504 Gateway Time-out error...
>  * looking at the latest arm-devs build it's failed
> configure because the buildbot doesn't either (a) have
> a suitable libfdt or (b) have the appropriate command
> to init the dtc submodule. I'm pretty sure I never got
> an email about this, so I imagine it's been busted for
> months.
>
> -- PMM

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Thu, 2013-07-25 at 10:40 +0200, Paolo Bonzini wrote:
>> In any case, let me reinforce point 6 above.  The patches were not
>> reverted because nobody posted the patches to do so.  Until someone
>> does
>> so, it is basically Anthony's call.  If he trusts people to fix the
>> whole thing, he has no reason to revert anything.  My track record of
>> fixing mess is probably better than my track record of not causing it,
>> so he didn't revert it.
>
> That's fine, I know you can fix stuff :-) I'm just really annoyed that
> upstream qemu remained broken for so long (and still is) while the whole
> thing derailed into a mostly pointless discussion on endianness and
> nobody (including Alexey) hollered loud enough that the breakage was
> fairly extensive

It you want to prevent breakages, add unit tests.  No patch will ever be
merged into the tree if 'make check' fails.  It's the ultimately way to
prevent things like this from happening.

I'd rather the tree is broken for a week and we get the proper fix
merged than go through a big production of reverting things.

Regards,

Anthony Liguori

> (BTW. The EHCI problem doesn't seem limited to SLOF, from what I
> remember of what Alexey showed me today, the Linux EHCI driver also
> segfaults qemu).
>
> Cheers,
> Ben.

Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori