[Qemu-devel] [PULL for-2.0-rc1 0/7] PReP patch queue 2014-03-20

[Qemu-devel] [PULL for-2.0-rc1 0/7] PReP patch queue 2014-03-20

[Andreas Färber]

Hello Peter,

Please pull the PowerPC Reference Platform (PReP) queue into qemu.git master.

Regards,
Andreas

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-ppc@nongnu.org
Cc: Alexander Graf <agraf@suse.de>
Cc: Hervé Poussineau <hpoussin@reactos.org>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>

The following changes since commit d1a1451cd3a60df7dbfd4fb7f616443d22be1beb:

  Merge remote-tracking branch 'remotes/afaerber/tags/qom-cpu-for-2.0' into staging (2014-03-19 22:36:44 +0000)

are available in the git repository at:


  git://github.com/afaerber/qemu-cpu.git tags/prep-for-2.0

for you to fetch changes up to f205da688b3dc8f6f54220b6997ed5b910afcc54:

  raven: Use raven_ for all function prefixes (2014-03-20 00:34:21 +0100)

----------------------------------------------------------------
PReP machine and devices

* Raven PCI host bridge memory fixes (remainder)

----------------------------------------------------------------
Hervé Poussineau (7):
      raven: Rename intack region to pci_intack
      raven: Implement non-contiguous I/O region
      raven: Set a correct PCI I/O memory region
      raven: Set a correct PCI memory region
      raven: Add PCI bus mastering address space
      raven: Fix PCI bus accesses with size > 1
      raven: Use raven_ for all function prefixes

 hw/pci-host/prep.c | 182 +++++++++++++++++++++++++++++++++++++++++++----------
 hw/ppc/prep.c      | 103 ++++--------------------------
 2 files changed, 161 insertions(+), 124 deletions(-)

[Qemu-devel] [PULL for-2.0 1/7] raven: Rename intack region to pci_intack

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Regions added subsequently will also have the pci_ prefix.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 94fdffa..84d50ca 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -52,9 +52,9 @@ typedef struct RavenPCIState {
 typedef struct PRePPCIState {
     PCIHostState parent_obj;
 
-    MemoryRegion intack;
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
+    MemoryRegion pci_intack;
     RavenPCIState pci_dev;
 } PREPPCIState;
 
@@ -148,8 +148,9 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
     memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s, "pciio", 0x00400000);
     memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);
 
-    memory_region_init_io(&s->intack, OBJECT(s), &PPC_intack_ops, s, "pci-intack", 1);
-    memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->intack);
+    memory_region_init_io(&s->pci_intack, OBJECT(s), &PPC_intack_ops, s,
+                          "pci-intack", 1);
+    memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack);
 
     /* TODO Remove once realize propagates to child devices. */
     object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp);
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Remove now duplicated code from prep board.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/prep.c      | 94 ++----------------------------------------------------
 2 files changed, 88 insertions(+), 91 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 84d50ca..629735e 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -54,8 +54,12 @@ typedef struct PRePPCIState {
 
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
+    AddressSpace pci_io_as;
+    MemoryRegion pci_io_non_contiguous;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
+
+    int contiguous_map;
 } PREPPCIState;
 
 #define BIOS_SIZE (1024 * 1024)
@@ -107,6 +111,71 @@ static const MemoryRegionOps PPC_intack_ops = {
     },
 };
 
+static inline hwaddr raven_io_address(PREPPCIState *s,
+                                      hwaddr addr)
+{
+    if (s->contiguous_map == 0) {
+        /* 64 KB contiguous space for IOs */
+        addr &= 0xFFFF;
+    } else {
+        /* 8 MB non-contiguous space for IOs */
+        addr = (addr & 0x1F) | ((addr & 0x007FFF000) >> 7);
+    }
+
+    /* FIXME: handle endianness switch */
+
+    return addr;
+}
+
+static uint64_t raven_io_read(void *opaque, hwaddr addr,
+                              unsigned int size)
+{
+    PREPPCIState *s = opaque;
+    uint8_t buf[4];
+
+    addr = raven_io_address(s, addr);
+    address_space_read(&s->pci_io_as, addr, buf, size);
+
+    if (size == 1) {
+        return buf[0];
+    } else if (size == 2) {
+        return lduw_p(buf);
+    } else if (size == 4) {
+        return ldl_p(buf);
+    } else {
+        g_assert_not_reached();
+    }
+}
+
+static void raven_io_write(void *opaque, hwaddr addr,
+                           uint64_t val, unsigned int size)
+{
+    PREPPCIState *s = opaque;
+    uint8_t buf[4];
+
+    addr = raven_io_address(s, addr);
+
+    if (size == 1) {
+        buf[0] = val;
+    } else if (size == 2) {
+        stw_p(buf, val);
+    } else if (size == 4) {
+        stl_p(buf, val);
+    } else {
+        g_assert_not_reached();
+    }
+
+    address_space_write(&s->pci_io_as, addr, buf, size);
+}
+
+static const MemoryRegionOps raven_io_ops = {
+    .read = raven_io_read,
+    .write = raven_io_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl.max_access_size = 4,
+    .valid.unaligned = true,
+};
+
 static int prep_map_irq(PCIDevice *pci_dev, int irq_num)
 {
     return (irq_num + (pci_dev->devfn >> 3)) & 1;
@@ -119,6 +188,13 @@ static void prep_set_irq(void *opaque, int irq_num, int level)
     qemu_set_irq(pic[irq_num] , level);
 }
 
+static void raven_change_gpio(void *opaque, int n, int level)
+{
+    PREPPCIState *s = opaque;
+
+    s->contiguous_map = level;
+}
+
 static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 {
     SysBusDevice *dev = SYS_BUS_DEVICE(d);
@@ -133,6 +209,8 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
+    qdev_init_gpio_in(d, raven_change_gpio, 1);
+
     pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, PCI_NUM_PINS);
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, s,
@@ -164,6 +242,13 @@ static void raven_pcihost_initfn(Object *obj)
     MemoryRegion *address_space_io = get_system_io();
     DeviceState *pci_dev;
 
+    memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
+                          "pci-io-non-contiguous", 0x00800000);
+    address_space_init(&s->pci_io_as, get_system_io(), "raven-io");
+
+    /* CPU address space */
+    memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
+                                        &s->pci_io_non_contiguous, 1);
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
                         address_space_mem, address_space_io, 0, TYPE_PCI_BUS);
     h->bus = &s->pci_bus;
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 81e13cb..035b5b2 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -185,6 +185,7 @@ typedef struct sysctrl_t {
     uint8_t state;
     uint8_t syscontrol;
     int contiguous_map;
+    qemu_irq contiguous_map_irq;
     int endian;
 } sysctrl_t;
 
@@ -253,6 +254,7 @@ static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
     case 0x0850:
         /* I/O map type register */
         sysctrl->contiguous_map = val & 0x01;
+        qemu_set_irq(sysctrl->contiguous_map_irq, sysctrl->contiguous_map);
         break;
     default:
         printf("ERROR: unaffected IO port write: %04" PRIx32
@@ -327,91 +329,6 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
     return retval;
 }
 
-static inline hwaddr prep_IO_address(sysctrl_t *sysctrl,
-                                                 hwaddr addr)
-{
-    if (sysctrl->contiguous_map == 0) {
-        /* 64 KB contiguous space for IOs */
-        addr &= 0xFFFF;
-    } else {
-        /* 8 MB non-contiguous space for IOs */
-        addr = (addr & 0x1F) | ((addr & 0x007FFF000) >> 7);
-    }
-
-    return addr;
-}
-
-static void PPC_prep_io_writeb (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    cpu_outb(addr, value);
-}
-
-static uint32_t PPC_prep_io_readb (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inb(addr);
-
-    return ret;
-}
-
-static void PPC_prep_io_writew (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outw(addr, value);
-}
-
-static uint32_t PPC_prep_io_readw (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inw(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
-}
-
-static void PPC_prep_io_writel (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outl(addr, value);
-}
-
-static uint32_t PPC_prep_io_readl (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inl(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
-}
-
-static const MemoryRegionOps PPC_prep_io_ops = {
-    .old_mmio = {
-        .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_NATIVE_ENDIAN,
-};
 
 #define NVRAM_SIZE        0x2000
 
@@ -458,7 +375,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     CPUPPCState *env = NULL;
     nvram_t nvram;
     M48t59State *m48t59;
-    MemoryRegion *PPC_io_memory = g_new(MemoryRegion, 1);
     PortioList *port_list = g_new(PortioList, 1);
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
@@ -567,6 +483,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
         fprintf(stderr, "Couldn't create PCI host controller.\n");
         exit(1);
     }
+    sysctrl->contiguous_map_irq = qdev_get_gpio_in(dev, 0);
 
     /* PCI -> ISA bridge */
     pci = pci_create_simple(pci_bus, PCI_DEVFN(1, 0), "i82378");
@@ -587,11 +504,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
     qdev_init_nofail(dev);
 
-    /* Register 8 MB of ISA IO space (needed for non-contiguous map) */
-    memory_region_init_io(PPC_io_memory, NULL, &PPC_prep_io_ops, sysctrl,
-                          "ppc-io", 0x00800000);
-    memory_region_add_subregion(sysmem, 0x80000000, PPC_io_memory);
-
     /* init basic PC hardware */
     pci_vga_init(pci_bus);
 
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 3/7] raven: Set a correct PCI I/O memory region

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

PCI I/O region is 0x3f800000 bytes starting at 0x80000000.
Do not use global QEMU I/O region, which is only 64KB.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 629735e..6463b6d 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -55,6 +55,7 @@ typedef struct PRePPCIState {
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
     AddressSpace pci_io_as;
+    MemoryRegion pci_io;
     MemoryRegion pci_io_non_contiguous;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
@@ -134,7 +135,7 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr,
     uint8_t buf[4];
 
     addr = raven_io_address(s, addr);
-    address_space_read(&s->pci_io_as, addr, buf, size);
+    address_space_read(&s->pci_io_as, addr + 0x80000000, buf, size);
 
     if (size == 1) {
         return buf[0];
@@ -165,7 +166,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
         g_assert_not_reached();
     }
 
-    address_space_write(&s->pci_io_as, addr, buf, size);
+    address_space_write(&s->pci_io_as, addr + 0x80000000, buf, size);
 }
 
 static const MemoryRegionOps raven_io_ops = {
@@ -215,13 +216,11 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, s,
                           "pci-conf-idx", 1);
-    sysbus_add_io(dev, 0xcf8, &h->conf_mem);
-    sysbus_init_ioports(&h->busdev, 0xcf8, 1);
+    memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
 
     memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_be_ops, s,
                           "pci-conf-data", 1);
-    sysbus_add_io(dev, 0xcfc, &h->data_mem);
-    sysbus_init_ioports(&h->busdev, 0xcfc, 1);
+    memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
 
     memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s, "pciio", 0x00400000);
     memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);
@@ -239,18 +238,20 @@ static void raven_pcihost_initfn(Object *obj)
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
     PREPPCIState *s = RAVEN_PCI_HOST_BRIDGE(obj);
     MemoryRegion *address_space_mem = get_system_memory();
-    MemoryRegion *address_space_io = get_system_io();
     DeviceState *pci_dev;
 
+    memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
     memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
                           "pci-io-non-contiguous", 0x00800000);
-    address_space_init(&s->pci_io_as, get_system_io(), "raven-io");
+    address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
 
     /* CPU address space */
+    memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
     memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
                                         &s->pci_io_non_contiguous, 1);
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
-                        address_space_mem, address_space_io, 0, TYPE_PCI_BUS);
+                        address_space_mem, &s->pci_io, 0, TYPE_PCI_BUS);
+
     h->bus = &s->pci_bus;
 
     object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 4/7] raven: Set a correct PCI memory region

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

PCI memory region is 0x3f000000 bytes starting at 0xc0000000.

However, keep compatibility with Open Hack'Ware expectations
by adding a hack for Open Hack'Ware display.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 9 ++++++---
 hw/ppc/prep.c      | 9 +++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 6463b6d..7f23a11 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -57,6 +57,7 @@ typedef struct PRePPCIState {
     AddressSpace pci_io_as;
     MemoryRegion pci_io;
     MemoryRegion pci_io_non_contiguous;
+    MemoryRegion pci_memory;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
 
@@ -204,8 +205,6 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
     MemoryRegion *address_space_mem = get_system_memory();
     int i;
 
-    isa_mem_base = 0xc0000000;
-
     for (i = 0; i < PCI_NUM_PINS; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
@@ -243,14 +242,18 @@ static void raven_pcihost_initfn(Object *obj)
     memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
     memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
                           "pci-io-non-contiguous", 0x00800000);
+    /* Open Hack'Ware hack: real size should be only 0x3f000000 bytes */
+    memory_region_init(&s->pci_memory, obj, "pci-memory",
+                       0x3f000000 + 0xc0000000ULL);
     address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
 
     /* CPU address space */
     memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
     memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
                                         &s->pci_io_non_contiguous, 1);
+    memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
-                        address_space_mem, &s->pci_io, 0, TYPE_PCI_BUS);
+                        &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
 
     h->bus = &s->pci_bus;
 
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 035b5b2..e243651 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -381,6 +381,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
 #endif
     int linux_boot, i, nb_nics1;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
+    MemoryRegion *vga = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base;
     long kernel_size, initrd_size;
     DeviceState *dev;
@@ -506,6 +507,14 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
 
     /* init basic PC hardware */
     pci_vga_init(pci_bus);
+    /* Open Hack'Ware hack: PCI BAR#0 is programmed to 0xf0000000.
+     * While bios will access framebuffer at 0xf0000000, real physical
+     * address is 0xf0000000 + 0xc0000000 (PCI memory base).
+     * Alias the wrong memory accesses to the right place.
+     */
+    memory_region_init_alias(vga, NULL, "vga-alias", pci_address_space(pci),
+                             0xf0000000, 0x1000000);
+    memory_region_add_subregion_overlap(sysmem, 0xf0000000, vga, 10);
 
     nb_nics1 = nb_nics;
     if (nb_nics1 > NE2000_NB_MAX)
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 5/7] raven: Add PCI bus mastering address space

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

This has been tested on Linux 2.4/PPC with the lsi53c895a SCSI adapter.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 7f23a11..8109625 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -59,6 +59,10 @@ typedef struct PRePPCIState {
     MemoryRegion pci_io_non_contiguous;
     MemoryRegion pci_memory;
     MemoryRegion pci_intack;
+    MemoryRegion bm;
+    MemoryRegion bm_ram_alias;
+    MemoryRegion bm_pci_memory_alias;
+    AddressSpace bm_as;
     RavenPCIState pci_dev;
 
     int contiguous_map;
@@ -190,6 +194,14 @@ static void prep_set_irq(void *opaque, int irq_num, int level)
     qemu_set_irq(pic[irq_num] , level);
 }
 
+static AddressSpace *raven_pcihost_set_iommu(PCIBus *bus, void *opaque,
+                                             int devfn)
+{
+    PREPPCIState *s = opaque;
+
+    return &s->bm_as;
+}
+
 static void raven_change_gpio(void *opaque, int n, int level)
 {
     PREPPCIState *s = opaque;
@@ -255,6 +267,18 @@ static void raven_pcihost_initfn(Object *obj)
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
                         &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
 
+    /* Bus master address space */
+    memory_region_init(&s->bm, obj, "bm-raven", UINT32_MAX);
+    memory_region_init_alias(&s->bm_pci_memory_alias, obj, "bm-pci-memory",
+                             &s->pci_memory, 0,
+                             memory_region_size(&s->pci_memory));
+    memory_region_init_alias(&s->bm_ram_alias, obj, "bm-system",
+                             get_system_memory(), 0, 0x80000000);
+    memory_region_add_subregion(&s->bm, 0         , &s->bm_pci_memory_alias);
+    memory_region_add_subregion(&s->bm, 0x80000000, &s->bm_ram_alias);
+    address_space_init(&s->bm_as, &s->bm, "raven-bm");
+    pci_setup_iommu(&s->pci_bus, raven_pcihost_set_iommu, s);
+
     h->bus = &s->pci_bus;
 
     object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 6/7] raven: Fix PCI bus accesses with size > 1

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 8109625..e452a91 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -225,12 +225,12 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, PCI_NUM_PINS);
 
-    memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, s,
-                          "pci-conf-idx", 1);
+    memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
+                          "pci-conf-idx", 4);
     memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
 
-    memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_be_ops, s,
-                          "pci-conf-data", 1);
+    memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, s,
+                          "pci-conf-data", 4);
     memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
 
     memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s, "pciio", 0x00400000);
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 7/7] raven: Use raven_ for all function prefixes

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index e452a91..d3e746c 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -70,7 +70,7 @@ typedef struct PRePPCIState {
 
 #define BIOS_SIZE (1024 * 1024)
 
-static inline uint32_t PPC_PCIIO_config(hwaddr addr)
+static inline uint32_t raven_pci_io_config(hwaddr addr)
 {
     int i;
 
@@ -82,36 +82,36 @@ static inline uint32_t PPC_PCIIO_config(hwaddr addr)
     return (addr & 0x7ff) |  (i << 11);
 }
 
-static void ppc_pci_io_write(void *opaque, hwaddr addr,
-                             uint64_t val, unsigned int size)
+static void raven_pci_io_write(void *opaque, hwaddr addr,
+                               uint64_t val, unsigned int size)
 {
     PREPPCIState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    pci_data_write(phb->bus, PPC_PCIIO_config(addr), val, size);
+    pci_data_write(phb->bus, raven_pci_io_config(addr), val, size);
 }
 
-static uint64_t ppc_pci_io_read(void *opaque, hwaddr addr,
-                                unsigned int size)
+static uint64_t raven_pci_io_read(void *opaque, hwaddr addr,
+                                  unsigned int size)
 {
     PREPPCIState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    return pci_data_read(phb->bus, PPC_PCIIO_config(addr), size);
+    return pci_data_read(phb->bus, raven_pci_io_config(addr), size);
 }
 
-static const MemoryRegionOps PPC_PCIIO_ops = {
-    .read = ppc_pci_io_read,
-    .write = ppc_pci_io_write,
+static const MemoryRegionOps raven_pci_io_ops = {
+    .read = raven_pci_io_read,
+    .write = raven_pci_io_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static uint64_t ppc_intack_read(void *opaque, hwaddr addr,
-                                unsigned int size)
+static uint64_t raven_intack_read(void *opaque, hwaddr addr,
+                                  unsigned int size)
 {
     return pic_read_irq(isa_pic);
 }
 
-static const MemoryRegionOps PPC_intack_ops = {
-    .read = ppc_intack_read,
+static const MemoryRegionOps raven_intack_ops = {
+    .read = raven_intack_read,
     .valid = {
         .max_access_size = 1,
     },
@@ -182,12 +182,12 @@ static const MemoryRegionOps raven_io_ops = {
     .valid.unaligned = true,
 };
 
-static int prep_map_irq(PCIDevice *pci_dev, int irq_num)
+static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
 {
     return (irq_num + (pci_dev->devfn >> 3)) & 1;
 }
 
-static void prep_set_irq(void *opaque, int irq_num, int level)
+static void raven_set_irq(void *opaque, int irq_num, int level)
 {
     qemu_irq *pic = opaque;
 
@@ -223,7 +223,8 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     qdev_init_gpio_in(d, raven_change_gpio, 1);
 
-    pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, PCI_NUM_PINS);
+    pci_bus_irqs(&s->pci_bus, raven_set_irq, raven_map_irq, s->irq,
+                 PCI_NUM_PINS);
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
@@ -233,10 +234,11 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
                           "pci-conf-data", 4);
     memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
 
-    memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s, "pciio", 0x00400000);
+    memory_region_init_io(&h->mmcfg, OBJECT(s), &raven_pci_io_ops, s,
+                          "pciio", 0x00400000);
     memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);
 
-    memory_region_init_io(&s->pci_intack, OBJECT(s), &PPC_intack_ops, s,
+    memory_region_init_io(&s->pci_intack, OBJECT(s), &raven_intack_ops, s,
                           "pci-intack", 1);
     memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack);
 
-- 
1.8.4.5

Re: [Qemu-devel] [PULL for-2.0-rc1 0/7] PReP patch queue 2014-03-20

[Peter Maydell]

On 19 March 2014 23:35, Andreas Färber <andreas.faerber@web.de> wrote:
> Hello Peter,
>
> Please pull the PowerPC Reference Platform (PReP) queue into qemu.git master.
>
> Regards,
> Andreas
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Hervé Poussineau <hpoussin@reactos.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
>
> The following changes since commit d1a1451cd3a60df7dbfd4fb7f616443d22be1beb:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/qom-cpu-for-2.0' into staging (2014-03-19 22:36:44 +0000)
>
> are available in the git repository at:
>
>
>   git://github.com/afaerber/qemu-cpu.git tags/prep-for-2.0
>
> for you to fetch changes up to f205da688b3dc8f6f54220b6997ed5b910afcc54:
>
>   raven: Use raven_ for all function prefixes (2014-03-20 00:34:21 +0100)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Andreas Färber]

Hi Hervé,

Am 20.03.2014 00:36, schrieb Andreas Färber:
> From: Hervé Poussineau <hpoussin@reactos.org>
> 
> Remove now duplicated code from prep board.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/prep.c      | 94 ++----------------------------------------------------
>  2 files changed, 88 insertions(+), 91 deletions(-)

I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.

$ make check-qtest-ppc V=1
[...]
  /ppc/endianness/prep:                                              **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
  /ppc/endianness/split/prep:                                        **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
  /ppc/endianness/combine/prep:                                      **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
FAIL: tests/endianness-test

On x86 everything is fine. git-bisect points to this commit.

There is one "FIXME: handle endianness switch" in here, but I don't spot
such code where it's being moved from either.

My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
the code or the test is wrong...

Any thoughts?

Thanks,
Andreas

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Hervé Poussineau]

Hi Andreas,

Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
> Hi Hervé,
>
> Am 20.03.2014 00:36, schrieb Andreas Färber:
>> From: Hervé Poussineau <hpoussin@reactos.org>
>>
>> Remove now duplicated code from prep board.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>>   hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>   2 files changed, 88 insertions(+), 91 deletions(-)
>
> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>
> $ make check-qtest-ppc V=1
> [...]
>    /ppc/endianness/prep:                                              **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
>    /ppc/endianness/split/prep:                                        **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
>    /ppc/endianness/combine/prep:                                      **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
> FAIL: tests/endianness-test
>
> On x86 everything is fine. git-bisect points to this commit.
>
> There is one "FIXME: handle endianness switch" in here, but I don't spot
> such code where it's being moved from either.
>
> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
> the code or the test is wrong...

Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then 
using cpu_inl, which does a ldl_p.
Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is 
using ldl_p.
So, yes, it seems that endianness of memory region does change things. 
Native endian means native endian of the guest of of the host?

I also checked tests/endianness-test.c.
The failing test is:
    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);
which seems perfectly valid as ISA is little-endian.

However, PReP is marked as bswap= true in this file, which means that 
values are inverted before writing and after reading them.
Paolo, what does it mean? It is supposed to be true for big endian 
machines, and false for little endian machines ?

Hervé

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Alexander Graf]

On 05.04.2014, at 22:26, Hervé Poussineau <hpoussin@reactos.org> wrote:

> Hi Andreas,
> 
> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>> Hi Hervé,
>> 
>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>> 
>>> Remove now duplicated code from prep board.
>>> 
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>>  hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>>  2 files changed, 88 insertions(+), 91 deletions(-)
>> 
>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>> 
>> $ make check-qtest-ppc V=1
>> [...]
>>   /ppc/endianness/prep:                                              **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>>   /ppc/endianness/split/prep:                                        **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>>   /ppc/endianness/combine/prep:                                      **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>> FAIL: tests/endianness-test
>> 
>> On x86 everything is fine. git-bisect points to this commit.
>> 
>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>> such code where it's being moved from either.
>> 
>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>> the code or the test is wrong...
> 
> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then using cpu_inl, which does a ldl_p.
> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is using ldl_p.
> So, yes, it seems that endianness of memory region does change things. Native endian means native endian of the guest of of the host?

It means "default endianness of the guest cpu" so that accesses coming from the CPU get passed in the value they had in the CPU registers :).


Alex

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Hervé Poussineau]

Le sam. 05 avril 2014 22:34:38 CEST, Alexander Graf a écrit :
>
> On 05.04.2014, at 22:26, Hervé Poussineau <hpoussin@reactos.org> wrote:
>
>> Hi Andreas,
>>
>> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>>> Hi Hervé,
>>>
>>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>>
>>>> Remove now duplicated code from prep board.
>>>>
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>>   hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>>
>>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>>
>>> $ make check-qtest-ppc V=1
>>> [...]
>>>    /ppc/endianness/prep:                                              **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/split/prep:                                        **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/combine/prep:                                      **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>> FAIL: tests/endianness-test
>>>
>>> On x86 everything is fine. git-bisect points to this commit.
>>>
>>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>>> such code where it's being moved from either.

The FIXME here is only when we'll handle dynamic endianess switching of 
the system (not of the CPU). It was not present in code moved in this 
commit, and I only added the comment to tell where to place it.

>>>
>>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>>> the code or the test is wrong...
>>
>> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then using cpu_inl, which does a ldl_p.
>> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is using ldl_p.
>> So, yes, it seems that endianness of memory region does change things. Native endian means native endian of the guest of of the host?
>
> It means "default endianness of the guest cpu" so that accesses coming from the CPU get passed in the value they had in the CPU registers :).

In that case, behaviour should be the same between x86 host and 
openSUSE ppc/ppc64 and OSX ppc64 hosts.
The culprit is elsewhere...

Hervé

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Peter Maydell]

On 5 April 2014 21:50, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Le sam. 05 avril 2014 22:34:38 CEST, Alexander Graf a écrit :
>> It means "default endianness of the guest cpu" so that accesses coming
>> from the CPU get passed in the value they had in the CPU registers :).
>
>
> In that case, behaviour should be the same between x86 host and openSUSE
> ppc/ppc64 and OSX ppc64 hosts.
> The culprit is elsewhere...

Yes, if we're behaving differently between different host
endiannesses then something is definitely wrong and my
suspicion would be that it's the test harness code.

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Paolo Bonzini]

Il 05/04/2014 19:20, Peter Maydell ha scritto:
>>> >> It means "default endianness of the guest cpu" so that accesses coming
>>> >> from the CPU get passed in the value they had in the CPU registers :).
>> >
>> >
>> > In that case, behaviour should be the same between x86 host and openSUSE
>> > ppc/ppc64 and OSX ppc64 hosts.
>> > The culprit is elsewhere...
> Yes, if we're behaving differently between different host
> endiannesses then something is definitely wrong and my
> suspicion would be that it's the test harness code.

No, that's very unlikely.  I think if you are using 
address_space_read/write then you must use DEVICE_NATIVE_ENDIAN, but I 
cannot check as I'm on vacation now, about 6 time zones away from 
anything big-endian.

Paolo

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Andreas Färber]

Am 05.04.2014 22:26, schrieb Hervé Poussineau:
> Hi Andreas,
> 
> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>> Hi Hervé,
>>
>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>
>>> Remove now duplicated code from prep board.
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>>   hw/pci-host/prep.c | 85
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>   hw/ppc/prep.c      | 94
>>> ++----------------------------------------------------
>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>
>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>
>> $ make check-qtest-ppc V=1
>> [...]
>>    /ppc/endianness/prep:                                              **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>>    /ppc/endianness/split/prep:                                        **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>>    /ppc/endianness/combine/prep:                                      **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>
>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>> FAIL: tests/endianness-test
>>
>> On x86 everything is fine. git-bisect points to this commit.
>>
>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>> such code where it's being moved from either.
>>
>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>> the code or the test is wrong...
> 
> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then
> using cpu_inl, which does a ldl_p.
> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is
> using ldl_p.
> So, yes, it seems that endianness of memory region does change things.
> Native endian means native endian of the guest of of the host?
> 
> I also checked tests/endianness-test.c.
> The failing test is:
>    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);
> which seems perfectly valid as ISA is little-endian.
> 
> However, PReP is marked as bswap= true in this file, which means that
> values are inverted before writing and after reading them.

I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.

Any further suggestions anyone?

Regards,
Andreas


> Paolo, what does it mean? It is supposed to be true for big endian
> machines, and false for little endian machines ?
> 
> Hervé

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Andreas Färber]

Am 07.04.2014 21:32, schrieb Andreas Färber:
> Am 05.04.2014 22:26, schrieb Hervé Poussineau:
>> Hi Andreas,
>>
>> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>>> Hi Hervé,
>>>
>>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>>
>>>> Remove now duplicated code from prep board.
>>>>
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>>   hw/pci-host/prep.c | 85
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   hw/ppc/prep.c      | 94
>>>> ++----------------------------------------------------
>>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>>
>>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>>
>>> $ make check-qtest-ppc V=1
>>> [...]
>>>    /ppc/endianness/prep:                                              **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/split/prep:                                        **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>>
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/combine/prep:                                      **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>>
>>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>> FAIL: tests/endianness-test
>>>
>>> On x86 everything is fine. git-bisect points to this commit.
>>>
>>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>>> such code where it's being moved from either.
>>>
>>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>>> the code or the test is wrong...
>>
>> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then
>> using cpu_inl, which does a ldl_p.
>> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is
>> using ldl_p.
>> So, yes, it seems that endianness of memory region does change things.
>> Native endian means native endian of the guest of of the host?
>>
>> I also checked tests/endianness-test.c.
>> The failing test is:
>>    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);
>> which seems perfectly valid as ISA is little-endian.
>>
>> However, PReP is marked as bswap= true in this file, which means that
>> values are inverted before writing and after reading them.
> 
> I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.

Same results for the following patch (x86_64 broken, ppc64 fixed):

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index d3e746c..fd3956f 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -177,7 +177,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps raven_io_ops = {
     .read = raven_io_read,
     .write = raven_io_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
+    .endianness = DEVICE_NATIVE_ENDIAN,
     .impl.max_access_size = 4,
     .valid.unaligned = true,
 };

Andreas

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Peter Maydell]

On 7 April 2014 21:40, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 07.04.2014 21:32, schrieb Andreas Färber:
>> I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.
>
> Same results for the following patch (x86_64 broken, ppc64 fixed):
>
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index d3e746c..fd3956f 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -177,7 +177,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps raven_io_ops = {
>      .read = raven_io_read,
>      .write = raven_io_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
>      .impl.max_access_size = 4,
>      .valid.unaligned = true,
>  };

Unsurprisingly, since both of those changes add/remove an
extra endianness swap for all host systems.

What you're looking for is the point in the chain where
we do something which is different depending on the
endianness of the host. You could stick in debug printfs
or just look around in gdb, to find out whether, for instance,
the values being passed into the raven_io_read/write
functions are different on the two hosts: if so then the
problem is somewhere further up the callstack...

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Peter Maydell]

On 7 April 2014 20:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I think if you are using address_space_read/write
> then you must use DEVICE_NATIVE_ENDIAN

I think it's actually worse than that. address_space_read/write
have an API which requires you to pass them a buffer which is
in guest CPU endianness. This means they cannot be used from
target-independent source files (like hw/pci-host/prep.c)
because there's no way to say "write this 32 bit value to
the buffer in target endianness". ioport.c which has pretty
much identical code works OK because it is built per target.

Worse, we have two versions of the ldl_p()/stl_p() &c
functions with conflicting semantics!
cpu-all.h defines these to be "target CPU endianness".
bswap.h defines these to be "host CPU endianness".
So which version any particular source file gets depends
on which of these two headers it ends up with. prep.c gets
the bswap.h versions, and exec.c gets the cpu-all.h versions,
which means on x86 we get the bizarre effect of doing an
stl_p() into a buffer in raven_io_write() and then having
address_space_rw() do a ldl_p() from the buffer and getting
a different value...

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Paolo Bonzini]

Il 08/04/2014 10:37, Peter Maydell ha scritto:
> I think it's actually worse than that. address_space_read/write
> have an API which requires you to pass them a buffer which is
> in guest CPU endianness. This means they cannot be used from
> target-independent source files (like hw/pci-host/prep.c)
> because there's no way to say "write this 32 bit value to
> the buffer in target endianness". ioport.c which has pretty
> much identical code works OK because it is built per target.

So the fix could be to compile prep.c per-target (and change to 
DEVICE_NATIVE_ENDIAN too).

> Worse, we have two versions of the ldl_p()/stl_p() &c
> functions with conflicting semantics!
> cpu-all.h defines these to be "target CPU endianness".
> bswap.h defines these to be "host CPU endianness".

Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a 
patch to rename bswap.h's definition to ldl_host_p/stl_host_p.

Paolo

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Peter Maydell]

On 8 April 2014 19:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
> So the fix could be to compile prep.c per-target (and change to
> DEVICE_NATIVE_ENDIAN too).

That seems like the wrong direction -- we should be
making fewer files per-target, not more.

>> Worse, we have two versions of the ldl_p()/stl_p() &c
>> functions with conflicting semantics!
>> cpu-all.h defines these to be "target CPU endianness".
>> bswap.h defines these to be "host CPU endianness".

> Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a patch to
> rename bswap.h's definition to ldl_host_p/stl_host_p.

Richard's suggestion was to make the cpu-all.h ones
be ldl_te_p/stl_te_p, which I think I like better.
We could do both in order to enforce that we audited
everything to see which it thought it was :-)

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Paolo Bonzini]

Il 08/04/2014 14:55, Peter Maydell ha scritto:
> On 8 April 2014 19:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> So the fix could be to compile prep.c per-target (and change to
>> DEVICE_NATIVE_ENDIAN too).
>
> That seems like the wrong direction -- we should be
> making fewer files per-target, not more.

I agree, and in fact we should also use DEVICE_NATIVE_ENDIAN less, not 
more.  Unfortunately, forwarding accesses from one address space to 
another via MMIO accessors requires DEVICE_NATIVE_ENDIAN, and that in 
turn requires target-endianness ldl_p/stl_p.

We could to try and make the non-contiguous I/O region an IOMMU region, 
if that can work.  But that would be post-2.0, I think.

I'm of course assuming that we cannot just revert patch 2 in this series...

>>> Worse, we have two versions of the ldl_p()/stl_p() &c
>>> functions with conflicting semantics!
>>> cpu-all.h defines these to be "target CPU endianness".
>>> bswap.h defines these to be "host CPU endianness".
>
>> Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a patch to
>> rename bswap.h's definition to ldl_host_p/stl_host_p.
>
> Richard's suggestion was to make the cpu-all.h ones
> be ldl_te_p/stl_te_p, which I think I like better.
> We could do both in order to enforce that we audited
> everything to see which it thought it was :-)

Yes, that's the next obvious step.

Paolo

Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region

[Peter Maydell]

On 8 April 2014 21:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I agree, and in fact we should also use DEVICE_NATIVE_ENDIAN less, not more.
> Unfortunately, forwarding accesses from one address space to another via
> MMIO accessors requires DEVICE_NATIVE_ENDIAN, and that in turn requires
> target-endianness ldl_p/stl_p.

I don't think this is correct. If we're purely forwarding
then you can just use DEVICE_LITTLE_ENDIAN and ldl_le_p.
There are three places in the sequence of calls from CPU to
outer MMIO accessor to stl_le_p to memory_space_write
that might insert bswaps:
 1 before the call to the outer MMIO accessor we will bswap
   if the guest endianness is not LE (because we're calling an
   MMIO accessor that was marked DEVICE_LITTLE_ENDIAN)
 2 in the stl_le_p we will bswap if the host endianness is not LE
   (because that's what stl_le_p means)
 3 in memory_space_write when we read out of the buffer we
   do a ldl_q, which will bswap if the host endianness is not
   the target endianness (because we get the cpu-all.h version
   of ldl_q, which does a target-endian load)

So:
host = guest = LE: no swapping
host = LE, guest = BE: swap 1 and 3 active
host = BE, guest = LE: swap 2 and 3 active
host = guest = BE: swap 1 and 2 active

In all cases we do an even number of swaps and the whole thing
cancels out :-)

You can use DEVICE_BIG_ENDIAN and stl_be_p if you like,
for equivalent effect.

I'm pretty sure this wasn't an intentional design property, though...

thanks
-- PMM