[Qemu-devel] [PATCH v3 00/10] prep: improve Raven PCI host emulation

[Qemu-devel] [PATCH v3 00/10] prep: improve Raven PCI host emulation

[Hervé Poussineau]

This patchset improves Raven PCI host emulation, found in some PPC platforms,
like the QEMU 'prep' one, and for example the IBM RS/6000 40p.

Some features added to raven emulation were already present in prep board
(non contiguous I/O, firmware loading), while some other are new (PCI bus
mastering memory region).

This patchset has been tested against Linux 2.4 PPC and IBM RS/6000 40p
firmware.

Notable achievements are PCI bus mastering (tested with lsi53c895a SCSI
adapter), lots of cleanup and emulation correctness, and also documentation
of current hacks required by Open Hack'Ware.
This gives us a good base to replace OpenHack'Ware by a possible upcoming
OpenBIOS release.

Changes since v2:
- rebased and fixed conflicts in patches 5 and 6

Changes since v1:
- reworked a dubious memcpy to make it work on big endian hosts
- split onto multiple patches

Hervé Poussineau (10):
  prep: kill get_system_io() usage
  raven: use constant PCI_NUM_PINS instead of 4
  raven: move BIOS loading from board code to PCI host
  raven: rename intack region to pci_intack
  raven: set a correct PCI I/O memory region
  raven: set a correct PCI memory region
  raven: add PCI bus mastering address space
  raven: implement non-contiguous I/O region
  raven: fix PCI bus accesses with size > 1
  raven: use raven_ for all function prefixes

 hw/pci-host/prep.c |  235 ++++++++++++++++++++++++++++++++++++++++++++--------
 hw/ppc/prep.c      |  155 ++++++----------------------------
 2 files changed, 226 insertions(+), 164 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v3 01/10] prep: kill get_system_io() usage

[Hervé Poussineau]

While ISA address space in prep machine is currently the one returned
by get_system_io(), this depends of the implementation of i82378/raven
devices, and this may not be the case forever.

Use the right ISA address space when adding some more ports to it.
We can use whatever ISA device on the right ISA bus, as all ISA devices
on the same ISA bus share the same ISA address space.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 aad0f69..9f8538c 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -656,7 +656,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET];
 
     portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep");
-    portio_list_add(port_list, get_system_io(), 0x0);
+    portio_list_add(port_list, isa_address_space_io(isa), 0x0);
 
     /* PowerPC control and status register group */
 #if 0
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 02/10] raven: use constant PCI_NUM_PINS instead of 4

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 0e71fdb..6ca9802 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -47,7 +47,7 @@ typedef struct PRePPCIState {
     PCIHostState parent_obj;
 
     MemoryRegion intack;
-    qemu_irq irq[4];
+    qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
     RavenPCIState pci_dev;
 } PREPPCIState;
@@ -121,11 +121,11 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 
     isa_mem_base = 0xc0000000;
 
-    for (i = 0; i < 4; i++) {
+    for (i = 0; i < PCI_NUM_PINS; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
-    pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, 4);
+    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);
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Raven datasheet explains where firmware lives in system memory, so do
it there instead of in board code. Other boards using the same PCI
host will not have to copy the firmware loading code.

However, add a specific hack for Open Hack'Ware, which provides only
a 512KB blob to be loaded at 0xfff00000, but expects valid code at
0xfffffffc (specific Open Hack'Ware reset instruction pointer).

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/prep.c      |   50 +++++++++++++-------------------------------------
 2 files changed, 64 insertions(+), 37 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 6ca9802..4dc5456 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -28,7 +28,9 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_host.h"
 #include "hw/i386/pc.h"
+#include "hw/loader.h"
 #include "exec/address-spaces.h"
+#include "elf.h"
 
 #define TYPE_RAVEN_PCI_DEVICE "raven"
 #define TYPE_RAVEN_PCI_HOST_BRIDGE "raven-pcihost"
@@ -38,6 +40,9 @@
 
 typedef struct RavenPCIState {
     PCIDevice dev;
+    uint32_t elf_machine;
+    char *bios_name;
+    MemoryRegion bios;
 } RavenPCIState;
 
 #define RAVEN_PCI_HOST_BRIDGE(obj) \
@@ -52,6 +57,8 @@ typedef struct PRePPCIState {
     RavenPCIState pci_dev;
 } PREPPCIState;
 
+#define BIOS_SIZE (1024 * 1024)
+
 static inline uint32_t PPC_PCIIO_config(hwaddr addr)
 {
     int i;
@@ -169,10 +176,46 @@ static void raven_pcihost_initfn(Object *obj)
 
 static int raven_init(PCIDevice *d)
 {
+    Object *obj = OBJECT(d);
+    RavenPCIState *s = RAVEN_PCI_DEVICE(d);
+    char *filename;
+    int bios_size = -1;
+
     d->config[0x0C] = 0x08; // cache_line_size
     d->config[0x0D] = 0x10; // latency_timer
     d->config[0x34] = 0x00; // capabilities_pointer
 
+    memory_region_init_ram(&s->bios, obj, "bios", BIOS_SIZE);
+    memory_region_set_readonly(&s->bios, true);
+    memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
+                                &s->bios);
+    vmstate_register_ram_global(&s->bios);
+    if (s->bios_name) {
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, s->bios_name);
+        if (filename) {
+            if (s->elf_machine != EM_NONE) {
+                bios_size = load_elf(filename, NULL, NULL, NULL,
+                                     NULL, NULL, 1, s->elf_machine, 0);
+            }
+            if (bios_size < 0) {
+                bios_size = get_image_size(filename);
+                if (bios_size > 0 && bios_size <= BIOS_SIZE) {
+                    hwaddr bios_addr;
+                    bios_size = (bios_size + 0xfff) & ~0xfff;
+                    bios_addr = (uint32_t)(-BIOS_SIZE);
+                    bios_size = load_image_targphys(filename, bios_addr,
+                                                    bios_size);
+                }
+            }
+        }
+        if (bios_size < 0 || bios_size > BIOS_SIZE) {
+            hw_error("qemu: could not load bios image '%s'\n", s->bios_name);
+        }
+        if (filename) {
+            g_free(filename);
+        }
+    }
+
     return 0;
 }
 
@@ -208,12 +251,20 @@ static const TypeInfo raven_info = {
     .class_init = raven_class_init,
 };
 
+static Property raven_pcihost_properties[] = {
+    DEFINE_PROP_UINT32("elf-machine", PREPPCIState, pci_dev.elf_machine,
+                       EM_NONE),
+    DEFINE_PROP_STRING("bios-name", PREPPCIState, pci_dev.bios_name),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void raven_pcihost_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->realize = raven_pcihost_realizefn;
+    dc->props = raven_pcihost_properties;
     dc->fw_name = "pci";
     dc->no_user = 1;
 }
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 9f8538c..8a09e2b 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -456,7 +456,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
-    char *filename;
     nvram_t nvram;
     M48t59State *m48t59;
     MemoryRegion *PPC_io_memory = g_new(MemoryRegion, 1);
@@ -464,7 +463,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
 #endif
-    int linux_boot, i, nb_nics1, bios_size;
+    int linux_boot, i, nb_nics1;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base;
@@ -510,41 +509,13 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     memory_region_add_subregion(sysmem, 0, ram);
 
     /* allocate and load BIOS */
-    memory_region_init_ram(bios, NULL, "ppc_prep.bios", BIOS_SIZE);
-    memory_region_set_readonly(bios, true);
-    memory_region_add_subregion(sysmem, (uint32_t)(-BIOS_SIZE), bios);
-    vmstate_register_ram_global(bios);
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-    if (filename) {
-        bios_size = load_elf(filename, NULL, NULL, NULL,
-                             NULL, NULL, 1, ELF_MACHINE, 0);
-        if (bios_size < 0) {
-            bios_size = get_image_size(filename);
-            if (bios_size > 0 && bios_size <= BIOS_SIZE) {
-                hwaddr bios_addr;
-                bios_size = (bios_size + 0xfff) & ~0xfff;
-                bios_addr = (uint32_t)(-bios_size);
-                bios_size = load_image_targphys(filename, bios_addr, bios_size);
-            }
-            if (bios_size > BIOS_SIZE) {
-                fprintf(stderr, "qemu: PReP bios '%s' is too large (0x%x)\n",
-                        bios_name, bios_size);
-                exit(1);
-            }
-        }
-    } else {
-        bios_size = -1;
-    }
-    if (bios_size < 0 && !qtest_enabled()) {
-        fprintf(stderr, "qemu: could not load PPC PReP bios '%s'\n",
-                bios_name);
-        exit(1);
-    }
-    if (filename) {
-        g_free(filename);
-    }
+    /* Open Hack'Ware hack: bios size is 512K and is loaded at 0xfff00000.
+     * However, reset address is 0xfffffffc. Mirror the bios from
+     * 0xfff00000 to 0xfff80000.
+     */
+    memory_region_init_alias(bios, NULL, "bios-alias", sysmem, 0xfff00000,
+                             0x00080000);
+    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);
 
     if (linux_boot) {
         kernel_base = KERNEL_LOAD_ADDR;
@@ -593,6 +564,11 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     }
 
     dev = qdev_create(NULL, "raven-pcihost");
+    if (bios_name == NULL) {
+        bios_name = BIOS_FILENAME;
+    }
+    qdev_prop_set_string(dev, "bios-name", bios_name);
+    qdev_prop_set_uint32(dev, "elf-machine", ELF_MACHINE);
     pcihost = PCI_HOST_BRIDGE(dev);
     object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Andreas Färber]

Hi,

Am 05.11.2013 00:09, schrieb Hervé Poussineau:
> Raven datasheet explains where firmware lives in system memory, so do
> it there instead of in board code. Other boards using the same PCI
> host will not have to copy the firmware loading code.

This part we had discussed and no one objected to the approach, so OK.

> However, add a specific hack for Open Hack'Ware, which provides only
> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).

Was this part explained before? I don't spot the equivalent in the
deleted code. If this is a new workaround, I would rather like to put it
in a separate patch for bisecting (can offer to do that myself then).
What are the symptoms? I am testing all these patches with OHW.

Regards,
Andreas

> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/pci-host/prep.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/prep.c      |   50 +++++++++++++-------------------------------------
>  2 files changed, 64 insertions(+), 37 deletions(-)
[...]
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 9f8538c..8a09e2b 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
[...]
> @@ -510,41 +509,13 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>      memory_region_add_subregion(sysmem, 0, ram);
>  
>      /* allocate and load BIOS */
> -    memory_region_init_ram(bios, NULL, "ppc_prep.bios", BIOS_SIZE);
> -    memory_region_set_readonly(bios, true);
> -    memory_region_add_subregion(sysmem, (uint32_t)(-BIOS_SIZE), bios);
> -    vmstate_register_ram_global(bios);
> -    if (bios_name == NULL)
> -        bios_name = BIOS_FILENAME;
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -    if (filename) {
> -        bios_size = load_elf(filename, NULL, NULL, NULL,
> -                             NULL, NULL, 1, ELF_MACHINE, 0);
> -        if (bios_size < 0) {
> -            bios_size = get_image_size(filename);
> -            if (bios_size > 0 && bios_size <= BIOS_SIZE) {
> -                hwaddr bios_addr;
> -                bios_size = (bios_size + 0xfff) & ~0xfff;
> -                bios_addr = (uint32_t)(-bios_size);
> -                bios_size = load_image_targphys(filename, bios_addr, bios_size);
> -            }
> -            if (bios_size > BIOS_SIZE) {
> -                fprintf(stderr, "qemu: PReP bios '%s' is too large (0x%x)\n",
> -                        bios_name, bios_size);
> -                exit(1);
> -            }
> -        }
> -    } else {
> -        bios_size = -1;
> -    }
> -    if (bios_size < 0 && !qtest_enabled()) {
> -        fprintf(stderr, "qemu: could not load PPC PReP bios '%s'\n",
> -                bios_name);
> -        exit(1);
> -    }
> -    if (filename) {
> -        g_free(filename);
> -    }
> +    /* Open Hack'Ware hack: bios size is 512K and is loaded at 0xfff00000.
> +     * However, reset address is 0xfffffffc. Mirror the bios from
> +     * 0xfff00000 to 0xfff80000.
> +     */
> +    memory_region_init_alias(bios, NULL, "bios-alias", sysmem, 0xfff00000,
> +                             0x00080000);
> +    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);
>  
>      if (linux_boot) {
>          kernel_base = KERNEL_LOAD_ADDR;
[snip]

Re: [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Hi,

Andreas Färber a écrit :
> Hi,
> 
> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>> Raven datasheet explains where firmware lives in system memory, so do
>> it there instead of in board code. Other boards using the same PCI
>> host will not have to copy the firmware loading code.
> 
> This part we had discussed and no one objected to the approach, so OK.
> 
>> However, add a specific hack for Open Hack'Ware, which provides only
>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
> 
> Was this part explained before? I don't spot the equivalent in the
> deleted code. If this is a new workaround, I would rather like to put it
> in a separate patch for bisecting (can offer to do that myself then).
> What are the symptoms? I am testing all these patches with OHW.

Old code does (error checking removed):
 >> -            bios_size = get_image_size(filename);
 >> -                bios_addr = (uint32_t)(-bios_size);
 >> -                bios_size = load_image_targphys(filename, bios_addr,
Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
and firmware is loaded in the range 0xfff80000-0xffffffff
OHW expects reset instruction pointer to be 0xfffffffc (not valid for 
604, but that's not the point now), which contains a valid instruction.
Note that range 0xfff00000-0xfff7ffff is empty.

Datasheet for raven says that firmware is at 0xfff00000, so I changed 
code to:
+#define BIOS_SIZE (1024 * 1024)
+                  bios_addr = (uint32_t)(-BIOS_SIZE);
+                  bios_size = load_image_targphys(filename, bios_addr,
+                                                  bios_size);
Ie, bios_addr = -1MB = 0xfff00000
and firmware is loaded in the range 0xfff00000-0xfff7ffff.
This doesn't work due to reset instruction pointer which now is pointing 
to empty memory, and symptoms are an empty screen on OHW.

So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff 
range to 0xfff80000-0xffffffff.

So, this patch is a small functional change, as it adds a copy of the 
firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live 
with it.

We'll be able to remove it once we switch to another firmware which uses 
the right reset instruction pointer.

Regards,

Hervé

>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  hw/pci-host/prep.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/ppc/prep.c      |   50 +++++++++++++-------------------------------------
>>  2 files changed, 64 insertions(+), 37 deletions(-)
> [...]
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 9f8538c..8a09e2b 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
> [...]
>> @@ -510,41 +509,13 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>>      memory_region_add_subregion(sysmem, 0, ram);
>>  
>>      /* allocate and load BIOS */
>> -    memory_region_init_ram(bios, NULL, "ppc_prep.bios", BIOS_SIZE);
>> -    memory_region_set_readonly(bios, true);
>> -    memory_region_add_subregion(sysmem, (uint32_t)(-BIOS_SIZE), bios);
>> -    vmstate_register_ram_global(bios);
>> -    if (bios_name == NULL)
>> -        bios_name = BIOS_FILENAME;
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> -    if (filename) {
>> -        bios_size = load_elf(filename, NULL, NULL, NULL,
>> -                             NULL, NULL, 1, ELF_MACHINE, 0);
>> -        if (bios_size < 0) {
>> -            bios_size = get_image_size(filename);
>> -            if (bios_size > 0 && bios_size <= BIOS_SIZE) {
>> -                hwaddr bios_addr;
>> -                bios_size = (bios_size + 0xfff) & ~0xfff;
>> -                bios_addr = (uint32_t)(-bios_size);
>> -                bios_size = load_image_targphys(filename, bios_addr, bios_size);
>> -            }
>> -            if (bios_size > BIOS_SIZE) {
>> -                fprintf(stderr, "qemu: PReP bios '%s' is too large (0x%x)\n",
>> -                        bios_name, bios_size);
>> -                exit(1);
>> -            }
>> -        }
>> -    } else {
>> -        bios_size = -1;
>> -    }
>> -    if (bios_size < 0 && !qtest_enabled()) {
>> -        fprintf(stderr, "qemu: could not load PPC PReP bios '%s'\n",
>> -                bios_name);
>> -        exit(1);
>> -    }
>> -    if (filename) {
>> -        g_free(filename);
>> -    }
>> +    /* Open Hack'Ware hack: bios size is 512K and is loaded at 0xfff00000.
>> +     * However, reset address is 0xfffffffc. Mirror the bios from
>> +     * 0xfff00000 to 0xfff80000.
>> +     */
>> +    memory_region_init_alias(bios, NULL, "bios-alias", sysmem, 0xfff00000,
>> +                             0x00080000);
>> +    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);
>>  
>>      if (linux_boot) {
>>          kernel_base = KERNEL_LOAD_ADDR;
> [snip]
> 
> 

Re: [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Hi,

Andreas Färber a écrit :
> Hi,
> 
> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>> Raven datasheet explains where firmware lives in system memory, so do
>> it there instead of in board code. Other boards using the same PCI
>> host will not have to copy the firmware loading code.
> 
> This part we had discussed and no one objected to the approach, so OK.
> 
>> However, add a specific hack for Open Hack'Ware, which provides only
>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
> 
> Was this part explained before? I don't spot the equivalent in the
> deleted code. If this is a new workaround, I would rather like to put it
> in a separate patch for bisecting (can offer to do that myself then).
> What are the symptoms? I am testing all these patches with OHW.

Old code does (error checking removed):
 >> -            bios_size = get_image_size(filename);
 >> -                bios_addr = (uint32_t)(-bios_size);
 >> -                bios_size = load_image_targphys(filename, bios_addr,
Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
and firmware is loaded in the range 0xfff80000-0xffffffff
OHW expects reset instruction pointer to be 0xfffffffc (not valid for 
604, but that's not the point now), which contains a valid instruction.
Note that range 0xfff00000-0xfff7ffff is empty.

Datasheet for raven says that firmware is at 0xfff00000, so I changed 
code to:
+#define BIOS_SIZE (1024 * 1024)
+                  bios_addr = (uint32_t)(-BIOS_SIZE);
+                  bios_size = load_image_targphys(filename, bios_addr,
+                                                  bios_size);
Ie, bios_addr = -1MB = 0xfff00000
and firmware is loaded in the range 0xfff00000-0xfff7ffff.
This doesn't work due to reset instruction pointer which now is pointing 
to empty memory, and symptoms are an empty screen on OHW.

So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff 
range to 0xfff80000-0xffffffff.

So, this patch is a small functional change, as it adds a copy of the 
firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live 
with it.

We'll be able to remove it once we switch to another firmware which uses 
the right reset instruction pointer or whose size is 1MB.

Regards,

Hervé

>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  hw/pci-host/prep.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/ppc/prep.c      |   50 +++++++++++++-------------------------------------
>>  2 files changed, 64 insertions(+), 37 deletions(-)
> [...]
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 9f8538c..8a09e2b 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
> [...]
>> @@ -510,41 +509,13 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>>      memory_region_add_subregion(sysmem, 0, ram);
>>  
>>      /* allocate and load BIOS */
>> -    memory_region_init_ram(bios, NULL, "ppc_prep.bios", BIOS_SIZE);
>> -    memory_region_set_readonly(bios, true);
>> -    memory_region_add_subregion(sysmem, (uint32_t)(-BIOS_SIZE), bios);
>> -    vmstate_register_ram_global(bios);
>> -    if (bios_name == NULL)
>> -        bios_name = BIOS_FILENAME;
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> -    if (filename) {
>> -        bios_size = load_elf(filename, NULL, NULL, NULL,
>> -                             NULL, NULL, 1, ELF_MACHINE, 0);
>> -        if (bios_size < 0) {
>> -            bios_size = get_image_size(filename);
>> -            if (bios_size > 0 && bios_size <= BIOS_SIZE) {
>> -                hwaddr bios_addr;
>> -                bios_size = (bios_size + 0xfff) & ~0xfff;
>> -                bios_addr = (uint32_t)(-bios_size);
>> -                bios_size = load_image_targphys(filename, bios_addr, bios_size);
>> -            }
>> -            if (bios_size > BIOS_SIZE) {
>> -                fprintf(stderr, "qemu: PReP bios '%s' is too large (0x%x)\n",
>> -                        bios_name, bios_size);
>> -                exit(1);
>> -            }
>> -        }
>> -    } else {
>> -        bios_size = -1;
>> -    }
>> -    if (bios_size < 0 && !qtest_enabled()) {
>> -        fprintf(stderr, "qemu: could not load PPC PReP bios '%s'\n",
>> -                bios_name);
>> -        exit(1);
>> -    }
>> -    if (filename) {
>> -        g_free(filename);
>> -    }
>> +    /* Open Hack'Ware hack: bios size is 512K and is loaded at 0xfff00000.
>> +     * However, reset address is 0xfffffffc. Mirror the bios from
>> +     * 0xfff00000 to 0xfff80000.
>> +     */
>> +    memory_region_init_alias(bios, NULL, "bios-alias", sysmem, 0xfff00000,
>> +                             0x00080000);
>> +    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);
>>  
>>      if (linux_boot) {
>>          kernel_base = KERNEL_LOAD_ADDR;
> [snip]
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Alexander Graf]

On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:

> Hi,
> 
> Andreas Färber a écrit :
>> Hi,
>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>> Raven datasheet explains where firmware lives in system memory, so do
>>> it there instead of in board code. Other boards using the same PCI
>>> host will not have to copy the firmware loading code.
>> This part we had discussed and no one objected to the approach, so OK.
>>> However, add a specific hack for Open Hack'Ware, which provides only
>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>> Was this part explained before? I don't spot the equivalent in the
>> deleted code. If this is a new workaround, I would rather like to put it
>> in a separate patch for bisecting (can offer to do that myself then).
>> What are the symptoms? I am testing all these patches with OHW.
> 
> Old code does (error checking removed):
> >> -            bios_size = get_image_size(filename);
> >> -                bios_addr = (uint32_t)(-bios_size);
> >> -                bios_size = load_image_targphys(filename, bios_addr,
> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
> and firmware is loaded in the range 0xfff80000-0xffffffff
> OHW expects reset instruction pointer to be 0xfffffffc (not valid for 604, but that's not the point now), which contains a valid instruction.
> Note that range 0xfff00000-0xfff7ffff is empty.
> 
> Datasheet for raven says that firmware is at 0xfff00000, so I changed code to:
> +#define BIOS_SIZE (1024 * 1024)
> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
> +                  bios_size = load_image_targphys(filename, bios_addr,
> +                                                  bios_size);
> Ie, bios_addr = -1MB = 0xfff00000
> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
> This doesn't work due to reset instruction pointer which now is pointing to empty memory, and symptoms are an empty screen on OHW.
> 
> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff range to 0xfff80000-0xffffffff.
> 
> So, this patch is a small functional change, as it adds a copy of the firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live with it.
> 
> We'll be able to remove it once we switch to another firmware which uses the right reset instruction pointer or whose size is 1MB.

Couldn't we just make the ROM fill the upper part of the 1MB region when we see it's smaller than 1MB? So that we pad at the bottom, not the top?

  bios_size = get_image_size(filename);
  if (bios_size < 0) {
    // error handling
  }
  assert(bios_size <= (1*MB));
  bios_addr = (uint32_t)(-bios_size);


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Alexander Graf a écrit :
> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
> 
>> Hi,
>>
>> Andreas Färber a écrit :
>>> Hi,
>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>> it there instead of in board code. Other boards using the same PCI
>>>> host will not have to copy the firmware loading code.
>>> This part we had discussed and no one objected to the approach, so OK.
>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>> Was this part explained before? I don't spot the equivalent in the
>>> deleted code. If this is a new workaround, I would rather like to put it
>>> in a separate patch for bisecting (can offer to do that myself then).
>>> What are the symptoms? I am testing all these patches with OHW.
>> Old code does (error checking removed):
>>>> -            bios_size = get_image_size(filename);
>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>> and firmware is loaded in the range 0xfff80000-0xffffffff
>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for 604, but that's not the point now), which contains a valid instruction.
>> Note that range 0xfff00000-0xfff7ffff is empty.
>>
>> Datasheet for raven says that firmware is at 0xfff00000, so I changed code to:
>> +#define BIOS_SIZE (1024 * 1024)
>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>> +                  bios_size = load_image_targphys(filename, bios_addr,
>> +                                                  bios_size);
>> Ie, bios_addr = -1MB = 0xfff00000
>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>> This doesn't work due to reset instruction pointer which now is pointing to empty memory, and symptoms are an empty screen on OHW.
>>
>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff range to 0xfff80000-0xffffffff.
>>
>> So, this patch is a small functional change, as it adds a copy of the firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live with it.
>>
>> We'll be able to remove it once we switch to another firmware which uses the right reset instruction pointer or whose size is 1MB.
> 
> Couldn't we just make the ROM fill the upper part of the 1MB region when we see it's smaller than 1MB? So that we pad at the bottom, not the top?
> 
>   bios_size = get_image_size(filename);
>   if (bios_size < 0) {
>     // error handling
>   }
>   assert(bios_size <= (1*MB));
>   bios_addr = (uint32_t)(-bios_size);
> 

I don't think that's a good idea, because the PReP cpus (601/604) have a 
reset vector at 0xfff00100. So you have to put some firmware at this 
address, even if firmware is smaller than 1MB.

OHW is the problem here, because it is less than 1MB and expects a reset 
vector at 0xfffffffc. That's why I want to put the hack outside raven 
chipset, in prep machine code.

Regards,

Hervé

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Andreas Färber]

Am 23.12.2013 19:13, schrieb Hervé Poussineau:
> Alexander Graf a écrit :
>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>
>>> Hi,
>>>
>>> Andreas Färber a écrit :
>>>> Hi,
>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>> it there instead of in board code. Other boards using the same PCI
>>>>> host will not have to copy the firmware loading code.
>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>> Was this part explained before? I don't spot the equivalent in the
>>>> deleted code. If this is a new workaround, I would rather like to
>>>> put it
>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>> What are the symptoms? I am testing all these patches with OHW.
>>> Old code does (error checking removed):
>>>>> -            bios_size = get_image_size(filename);
>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>> 604, but that's not the point now), which contains a valid instruction.
>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>
>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>> code to:
>>> +#define BIOS_SIZE (1024 * 1024)
>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>> +                                                  bios_size);
>>> Ie, bios_addr = -1MB = 0xfff00000
>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>> This doesn't work due to reset instruction pointer which now is
>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>
>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>> range to 0xfff80000-0xffffffff.
>>>
>>> So, this patch is a small functional change, as it adds a copy of the
>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>> live with it.
>>>
>>> We'll be able to remove it once we switch to another firmware which
>>> uses the right reset instruction pointer or whose size is 1MB.
>>
>> Couldn't we just make the ROM fill the upper part of the 1MB region
>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>> the top?
>>
>>   bios_size = get_image_size(filename);
>>   if (bios_size < 0) {
>>     // error handling
>>   }
>>   assert(bios_size <= (1*MB));
>>   bios_addr = (uint32_t)(-bios_size);
>>
> 
> I don't think that's a good idea, because the PReP cpus (601/604) have a
> reset vector at 0xfff00100. So you have to put some firmware at this
> address, even if firmware is smaller than 1MB.
> 
> OHW is the problem here, because it is less than 1MB and expects a reset
> vector at 0xfffffffc. That's why I want to put the hack outside raven
> chipset, in prep machine code.

Let me clarify then that it was me who disabled some checks that used to
assure that the loaded binary is in fact 1MB:
http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb

So the issue is actually that the OHW binary is really messed up.
And me, Hervé and Mark have been working on getting OpenBIOS working for
PReP in its place.

So I'm currently considering the following options:

1)
Revert OHW alias and size/position change
Strip ELF loading and elf-machine
Add back Raven ELF support separately

2)
Apply my prep.c ELF support patch first
Apply this patch as pure loading-logic movement

3)
Leave broken OHW loading in prep.c
Only implement 1MB / ELF loading support in Raven

4)
Accept a 1MB OHW image and drop support for 512KB OHW

5)
Move only MemoryRegion into Raven PHB
Leave loading code in prep.c but move into function for reuse
-> avoids ELF machine property

Opinions?

Another issue that came to my attention is that surely the MemoryRegion
and firmware-loading code should live in the SysBusDevice and not in the
PCIDevice? Also some instance_init vs. realize separation would seem
possible.

Regards,
Andreas

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>> Alexander Graf a écrit :
>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> Andreas Färber a écrit :
>>>>> Hi,
>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>> host will not have to copy the firmware loading code.
>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>> put it
>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>> Old code does (error checking removed):
>>>>>> -            bios_size = get_image_size(filename);
>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>> 604, but that's not the point now), which contains a valid instruction.
>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>
>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>> code to:
>>>> +#define BIOS_SIZE (1024 * 1024)
>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>> +                                                  bios_size);
>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>> This doesn't work due to reset instruction pointer which now is
>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>
>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>> range to 0xfff80000-0xffffffff.
>>>>
>>>> So, this patch is a small functional change, as it adds a copy of the
>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>> live with it.
>>>>
>>>> We'll be able to remove it once we switch to another firmware which
>>>> uses the right reset instruction pointer or whose size is 1MB.
>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>> the top?
>>>
>>>   bios_size = get_image_size(filename);
>>>   if (bios_size < 0) {
>>>     // error handling
>>>   }
>>>   assert(bios_size <= (1*MB));
>>>   bios_addr = (uint32_t)(-bios_size);
>>>
>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>> reset vector at 0xfff00100. So you have to put some firmware at this
>> address, even if firmware is smaller than 1MB.
>>
>> OHW is the problem here, because it is less than 1MB and expects a reset
>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>> chipset, in prep machine code.
> 
> Let me clarify then that it was me who disabled some checks that used to
> assure that the loaded binary is in fact 1MB:
> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
> 
> So the issue is actually that the OHW binary is really messed up.
> And me, Hervé and Mark have been working on getting OpenBIOS working for
> PReP in its place.
> 
> So I'm currently considering the following options:
> 
> 1)
> Revert OHW alias and size/position change
> Strip ELF loading and elf-machine
> Add back Raven ELF support separately
> 
> 2)
> Apply my prep.c ELF support patch first
> Apply this patch as pure loading-logic movement
> 
> 3)
> Leave broken OHW loading in prep.c
> Only implement 1MB / ELF loading support in Raven
> 
> 4)
> Accept a 1MB OHW image and drop support for 512KB OHW
> 
> 5)
> Move only MemoryRegion into Raven PHB
> Leave loading code in prep.c but move into function for reuse
> -> avoids ELF machine property
> 
> Opinions?

Or maybe:
6) Add the bios loading in Raven like done on this patch (only 1M / ELF 
loading support), which won't currently be used as long as 
raven.bios-size is not set,
and keep the broken code in prep.c, which will be removed when switching 
to OpenBIOS ?

Hervé

> 
> Another issue that came to my attention is that surely the MemoryRegion
> and firmware-loading code should live in the SysBusDevice and not in the
> PCIDevice? Also some instance_init vs. realize separation would seem
> possible.
> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Alexander Graf]

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

> Andreas Färber a écrit :
>> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>>> Alexander Graf a écrit :
>>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> Andreas Färber a écrit :
>>>>>> Hi,
>>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>>> host will not have to copy the firmware loading code.
>>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>>> put it
>>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>>> Old code does (error checking removed):
>>>>>>> -            bios_size = get_image_size(filename);
>>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>>> 604, but that's not the point now), which contains a valid instruction.
>>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>> 
>>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>>> code to:
>>>>> +#define BIOS_SIZE (1024 * 1024)
>>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>>> +                                                  bios_size);
>>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>>> This doesn't work due to reset instruction pointer which now is
>>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>> 
>>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>>> range to 0xfff80000-0xffffffff.
>>>>> 
>>>>> So, this patch is a small functional change, as it adds a copy of the
>>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>>> live with it.
>>>>> 
>>>>> We'll be able to remove it once we switch to another firmware which
>>>>> uses the right reset instruction pointer or whose size is 1MB.
>>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>>> the top?
>>>> 
>>>>  bios_size = get_image_size(filename);
>>>>  if (bios_size < 0) {
>>>>    // error handling
>>>>  }
>>>>  assert(bios_size <= (1*MB));
>>>>  bios_addr = (uint32_t)(-bios_size);
>>>> 
>>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>>> reset vector at 0xfff00100. So you have to put some firmware at this
>>> address, even if firmware is smaller than 1MB.
>>> 
>>> OHW is the problem here, because it is less than 1MB and expects a reset
>>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>>> chipset, in prep machine code.
>> Let me clarify then that it was me who disabled some checks that used to
>> assure that the loaded binary is in fact 1MB:
>> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
>> So the issue is actually that the OHW binary is really messed up.
>> And me, Hervé and Mark have been working on getting OpenBIOS working for
>> PReP in its place.
>> So I'm currently considering the following options:
>> 1)
>> Revert OHW alias and size/position change
>> Strip ELF loading and elf-machine
>> Add back Raven ELF support separately
>> 2)
>> Apply my prep.c ELF support patch first
>> Apply this patch as pure loading-logic movement
>> 3)
>> Leave broken OHW loading in prep.c
>> Only implement 1MB / ELF loading support in Raven
>> 4)
>> Accept a 1MB OHW image and drop support for 512KB OHW

I like this option the best.


Alex

>> 5)
>> Move only MemoryRegion into Raven PHB
>> Leave loading code in prep.c but move into function for reuse
>> -> avoids ELF machine property
>> Opinions?
> 
> Or maybe:
> 6) Add the bios loading in Raven like done on this patch (only 1M / ELF loading support), which won't currently be used as long as raven.bios-size is not set,
> and keep the broken code in prep.c, which will be removed when switching to OpenBIOS ?
> 
> Hervé
> 
>> Another issue that came to my attention is that surely the MemoryRegion
>> and firmware-loading code should live in the SysBusDevice and not in the
>> PCIDevice? Also some instance_init vs. realize separation would seem
>> possible.
>> Regards,
>> Andreas
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Andreas Färber]

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

Am 24.12.2013 01:32, schrieb Alexander Graf:
> 
> On 23.12.2013, at 22:54, Hervé Poussineau <hpoussin@reactos.org> wrote:
> 
>> Andreas Färber a écrit :
>>> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>>>> Alexander Graf a écrit :
>>>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Andreas Färber a écrit :
>>>>>>> Hi,
>>>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>>>> host will not have to copy the firmware loading code.
>>>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>>>> put it
>>>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>>>> Old code does (error checking removed):
>>>>>>>> -            bios_size = get_image_size(filename);
>>>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>>>> 604, but that's not the point now), which contains a valid instruction.
>>>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>>>
>>>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>>>> code to:
>>>>>> +#define BIOS_SIZE (1024 * 1024)
>>>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>>>> +                                                  bios_size);
>>>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>>>> This doesn't work due to reset instruction pointer which now is
>>>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>>>
>>>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>>>> range to 0xfff80000-0xffffffff.
>>>>>>
>>>>>> So, this patch is a small functional change, as it adds a copy of the
>>>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>>>> live with it.
>>>>>>
>>>>>> We'll be able to remove it once we switch to another firmware which
>>>>>> uses the right reset instruction pointer or whose size is 1MB.
>>>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>>>> the top?
>>>>>
>>>>>  bios_size = get_image_size(filename);
>>>>>  if (bios_size < 0) {
>>>>>    // error handling
>>>>>  }
>>>>>  assert(bios_size <= (1*MB));
>>>>>  bios_addr = (uint32_t)(-bios_size);
>>>>>
>>>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>>>> reset vector at 0xfff00100. So you have to put some firmware at this
>>>> address, even if firmware is smaller than 1MB.
>>>>
>>>> OHW is the problem here, because it is less than 1MB and expects a reset
>>>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>>>> chipset, in prep machine code.
>>> Let me clarify then that it was me who disabled some checks that used to
>>> assure that the loaded binary is in fact 1MB:
>>> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
>>> So the issue is actually that the OHW binary is really messed up.
>>> And me, Hervé and Mark have been working on getting OpenBIOS working for
>>> PReP in its place.
>>> So I'm currently considering the following options:
>>> 1)
>>> Revert OHW alias and size/position change
>>> Strip ELF loading and elf-machine
>>> Add back Raven ELF support separately
>>> 2)
>>> Apply my prep.c ELF support patch first
>>> Apply this patch as pure loading-logic movement
>>> 3)
>>> Leave broken OHW loading in prep.c
>>> Only implement 1MB / ELF loading support in Raven
>>> 4)
>>> Accept a 1MB OHW image and drop support for 512KB OHW
> 
> I like this option the best.

Alex, are you volunteering to build one? You wanted to look into that
with me since a looong time ago... :)

http://repo.or.cz/w/openhackware.git

As you will remember, Jocelyn fixed an IDE issue binary-only, without
updating pc-bios/ohw.diff:
http://git.qemu.org/?p=qemu.git;a=commit;h=55aa45ddde3283cdd781326d001f7456bf02f684

I dug out the patch René Rebe proposed later for fixing that IDE issue:
http://lists.gnu.org/archive/html/qemu-devel/2008-05/msg00223.html

I've just managed to fix up that patch to finally apply (apart from line
wraps in a patch to a patch - gosh! - there was also an "address@hidden"
from the archive hidden in the patch context), attached, but haven't yet
re-tried to build with it. It includes a linker script fix that might
explain my previous build issues.

Andreas

> 
> 
> Alex
> 
>>> 5)
>>> Move only MemoryRegion into Raven PHB
>>> Leave loading code in prep.c but move into function for reuse
>>> -> avoids ELF machine property
>>> Opinions?
>>
>> Or maybe:
>> 6) Add the bios loading in Raven like done on this patch (only 1M / ELF loading support), which won't currently be used as long as raven.bios-size is not set,
>> and keep the broken code in prep.c, which will be removed when switching to OpenBIOS ?

P.S. That's my 3). :)

>>
>> Hervé
>>
>>> Another issue that came to my attention is that surely the MemoryRegion
>>> and firmware-loading code should live in the SysBusDevice and not in the
>>> PCIDevice? Also some instance_init vs. realize separation would seem
>>> possible.
>>> Regards,
>>> Andreas
>>
> 


[-- Attachment #2: 0001-fix-PPC-OpenHackWare-for-Linux-2.6.patch --]
[-- Type: text/x-patch, Size: 3912 bytes --]

>From 79265ffab56281416edc48b3daf825a1e00ee5fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Rebe?= <rene@exactcode.de>
Date: Tue, 24 Dec 2013 02:39:54 +0100
Subject: [PATCH] fix PPC OpenHackWare for Linux 2.6

---
 pc-bios/ohw.diff | 67 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/pc-bios/ohw.diff b/pc-bios/ohw.diff
index c6b6623..4f1680b 100644
--- a/pc-bios/ohw.diff
+++ b/pc-bios/ohw.diff
@@ -1,3 +1,19 @@
+Make it link at al.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/main.ld       2005-03-31 09:23:33.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/main.ld        2008-05-06 11:23:29.000000000 +0200
+@@ -49,7 +49,7 @@
+         _sdata_end = . ;
+         . = ALIGN(4) ;
+         _ro_start = . ;
+-        .rodata    : { *(.rodata) } > bios
++        .rodata    : { *(.rodata*) } > bios
+         _ro_end = . ;
+         . = ALIGN(4) ;
+         _RTAS_start = .;
+
 diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --exclude mkdiff OpenHackWare-release-0.4.org/src/bios.h OpenHackWare-release-0.4/src/bios.h
 --- OpenHackWare-release-0.4.org/src/bios.h	2005-04-06 23:20:22.000000000 +0200
 +++ OpenHackWare-release-0.4/src/bios.h	2005-07-07 01:10:20.000000000 +0200
@@ -748,24 +764,14 @@ diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --
          {
              /* Hack taken 'as-is' from PearPC */
              unsigned char info[] = {
-@@ -1596,7 +1627,9 @@
-         OF_node_put(OF_env, brom);
+@@ -1596,6 +1627,7 @@
          OF_node_put(OF_env, rom);
      }
-+#if 0
      /* From here, hardcoded hacks to get a Mac-like machine */
-+    /* XXX: Core99 does not seem to like this NVRAM tree */
++    /* XXX: Not yet perfect, but Linux 2.6 does oops on boot on Core99 without NVRAM node */
      /* "/nvram@fff04000" node */
      {
          OF_regprop_t regs;
-@@ -1617,6 +1650,7 @@
-         OF_prop_int_new(OF_env, chs, "nvram", OF_pack_handle(OF_env, nvr));
-         OF_node_put(OF_env, nvr);
-     }
-+#endif
-     /* "/pseudo-hid" : hid emulation as Apple does */
-     {
-         OF_node_t *hid;
 @@ -1663,7 +1697,27 @@
          }
          OF_node_put(OF_env, hid);
@@ -1841,3 +1847,40 @@ diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --
      case ARCH_MAC99:
          /* We are supposed to have 3 host bridges:
           * - the uninorth AGP bridge at 0xF0000000
+
+
+The 2.6 Linux kernel checks for 1, as I do not have the IEEE spec on
+my desk I can only guess it should return 1 on success, not the
+string length.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/of.c  2005-04-06 23:17:26.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/of.c   2008-05-06 14:50:48.000000000 +0200
+@@ -3841,7 +4061,7 @@
+             OF_DPRINTF("Return property name [%s]\n", next->name);
+             OF_sts(next_name, (void *)(next->name));
+             OF_DUMP_STRING(OF_env, next_name);
+-            pushd(OF_env, strlen(next->name) + 1);
++            pushd(OF_env, 1); /* ReneR: Linux 2.6 flatten_device_tree */
+         }
+     }
+ }
+
+
+In qemu r3309 j_mayer did some "Quickly hack PowerPC BIOS able to boot
+on CDROM again.", as I did not feel like disassemble to find out this
+worked for me for now.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/bloc.c        2005-04-06 23:21:00.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/bloc.c 2008-05-06 14:20:10.000000000 +0200
+@@ -1021,7 +1038,7 @@
+         status = ide_port_read(bd, 0x07);
+         if (status != 0x08) {
+             ERROR("ATAPI TEST_UNIT_READY : status %0x != 0x08\n", status);
+-            return -1;
++            /*return -1;*/ /* fails to boot from cdrom? */
+         }
+         for (i = 0; i < 3; i++) {
-- 
1.8.4

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 24.12.2013 01:32, schrieb Alexander Graf:
>> On 23.12.2013, at 22:54, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>
>>> Andreas Färber a écrit :
>>>> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>>>>> Alexander Graf a écrit :
>>>>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Andreas Färber a écrit :
>>>>>>>> Hi,
>>>>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>>>>> host will not have to copy the firmware loading code.
>>>>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>>>>> put it
>>>>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>>>>> Old code does (error checking removed):
>>>>>>>>> -            bios_size = get_image_size(filename);
>>>>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>>>>> 604, but that's not the point now), which contains a valid instruction.
>>>>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>>>>
>>>>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>>>>> code to:
>>>>>>> +#define BIOS_SIZE (1024 * 1024)
>>>>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>>>>> +                                                  bios_size);
>>>>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>>>>> This doesn't work due to reset instruction pointer which now is
>>>>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>>>>
>>>>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>>>>> range to 0xfff80000-0xffffffff.
>>>>>>>
>>>>>>> So, this patch is a small functional change, as it adds a copy of the
>>>>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>>>>> live with it.
>>>>>>>
>>>>>>> We'll be able to remove it once we switch to another firmware which
>>>>>>> uses the right reset instruction pointer or whose size is 1MB.
>>>>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>>>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>>>>> the top?
>>>>>>
>>>>>>  bios_size = get_image_size(filename);
>>>>>>  if (bios_size < 0) {
>>>>>>    // error handling
>>>>>>  }
>>>>>>  assert(bios_size <= (1*MB));
>>>>>>  bios_addr = (uint32_t)(-bios_size);
>>>>>>
>>>>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>>>>> reset vector at 0xfff00100. So you have to put some firmware at this
>>>>> address, even if firmware is smaller than 1MB.
>>>>>
>>>>> OHW is the problem here, because it is less than 1MB and expects a reset
>>>>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>>>>> chipset, in prep machine code.
>>>> Let me clarify then that it was me who disabled some checks that used to
>>>> assure that the loaded binary is in fact 1MB:
>>>> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
>>>> So the issue is actually that the OHW binary is really messed up.
>>>> And me, Hervé and Mark have been working on getting OpenBIOS working for
>>>> PReP in its place.
>>>> So I'm currently considering the following options:
>>>> 1)
>>>> Revert OHW alias and size/position change
>>>> Strip ELF loading and elf-machine
>>>> Add back Raven ELF support separately
>>>> 2)
>>>> Apply my prep.c ELF support patch first
>>>> Apply this patch as pure loading-logic movement
>>>> 3)
>>>> Leave broken OHW loading in prep.c
>>>> Only implement 1MB / ELF loading support in Raven
>>>> 4)
>>>> Accept a 1MB OHW image and drop support for 512KB OHW
>> I like this option the best.
> 
> Alex, are you volunteering to build one? You wanted to look into that
> with me since a looong time ago... :)

I can provide one 1MB OHW binary soon.
One is already available (without René Rebe patch) at
http://repo.or.cz/w/qemu/hpoussin.git/blob/openhackware:/ppc_rom.bin

> 
> http://repo.or.cz/w/openhackware.git
> 
> As you will remember, Jocelyn fixed an IDE issue binary-only, without
> updating pc-bios/ohw.diff:
> http://git.qemu.org/?p=qemu.git;a=commit;h=55aa45ddde3283cdd781326d001f7456bf02f684
> 
> I dug out the patch René Rebe proposed later for fixing that IDE issue:
> http://lists.gnu.org/archive/html/qemu-devel/2008-05/msg00223.html
> 
> I've just managed to fix up that patch to finally apply (apart from line
> wraps in a patch to a patch - gosh! - there was also an "address@hidden"
> from the archive hidden in the patch context), attached, but haven't yet
> re-tried to build with it. It includes a linker script fix that might
> explain my previous build issues.
> 

Hervé

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Alexander Graf]

On 24.12.2013, at 03:02, Andreas Färber <andreas.faerber@web.de> wrote:

> Am 24.12.2013 01:32, schrieb Alexander Graf:
>> 
>> On 23.12.2013, at 22:54, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> 
>>> Andreas Färber a écrit :
>>>> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>>>>> Alexander Graf a écrit :
>>>>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>>>> 
>>>>>>> Hi,
>>>>>>> 
>>>>>>> Andreas Färber a écrit :
>>>>>>>> Hi,
>>>>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>>>>> host will not have to copy the firmware loading code.
>>>>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>>>>> put it
>>>>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>>>>> Old code does (error checking removed):
>>>>>>>>> -            bios_size = get_image_size(filename);
>>>>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>>>>> 604, but that's not the point now), which contains a valid instruction.
>>>>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>>>> 
>>>>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>>>>> code to:
>>>>>>> +#define BIOS_SIZE (1024 * 1024)
>>>>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>>>>> +                                                  bios_size);
>>>>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>>>>> This doesn't work due to reset instruction pointer which now is
>>>>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>>>> 
>>>>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>>>>> range to 0xfff80000-0xffffffff.
>>>>>>> 
>>>>>>> So, this patch is a small functional change, as it adds a copy of the
>>>>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>>>>> live with it.
>>>>>>> 
>>>>>>> We'll be able to remove it once we switch to another firmware which
>>>>>>> uses the right reset instruction pointer or whose size is 1MB.
>>>>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>>>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>>>>> the top?
>>>>>> 
>>>>>> bios_size = get_image_size(filename);
>>>>>> if (bios_size < 0) {
>>>>>>   // error handling
>>>>>> }
>>>>>> assert(bios_size <= (1*MB));
>>>>>> bios_addr = (uint32_t)(-bios_size);
>>>>>> 
>>>>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>>>>> reset vector at 0xfff00100. So you have to put some firmware at this
>>>>> address, even if firmware is smaller than 1MB.
>>>>> 
>>>>> OHW is the problem here, because it is less than 1MB and expects a reset
>>>>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>>>>> chipset, in prep machine code.
>>>> Let me clarify then that it was me who disabled some checks that used to
>>>> assure that the loaded binary is in fact 1MB:
>>>> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
>>>> So the issue is actually that the OHW binary is really messed up.
>>>> And me, Hervé and Mark have been working on getting OpenBIOS working for
>>>> PReP in its place.
>>>> So I'm currently considering the following options:
>>>> 1)
>>>> Revert OHW alias and size/position change
>>>> Strip ELF loading and elf-machine
>>>> Add back Raven ELF support separately
>>>> 2)
>>>> Apply my prep.c ELF support patch first
>>>> Apply this patch as pure loading-logic movement
>>>> 3)
>>>> Leave broken OHW loading in prep.c
>>>> Only implement 1MB / ELF loading support in Raven
>>>> 4)
>>>> Accept a 1MB OHW image and drop support for 512KB OHW
>> 
>> I like this option the best.
> 
> Alex, are you volunteering to build one? You wanted to look into that
> with me since a looong time ago... :)

You got mail kupo :).


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Mark Cave-Ayland]

On 23/12/13 20:02, Andreas Färber wrote:

> So the issue is actually that the OHW binary is really messed up.
> And me, Hervé and Mark have been working on getting OpenBIOS working for
> PReP in its place.

It may be worth adding that when Hervé's latest patches are applied to 
OpenBIOS SVN trunk (currently frozen while I wait for git.qemu.org to 
update its mirror) then AFAIK the main issue preventing OHW from being 
switched for OpenBIOS is that OpenBIOS hangs somewhere in the MMU setup 
code unless QEMU is launched with -cpu 750. It's probably fairly easy to 
resolve for someone with knowledge of 600 and 700 series CPUs, but my 
background is mainly SPARC so it's outside my area of expertise.

FWIW what images are people currently booting under -M prep? My general 
feeling is that OpenBIOS will be able to boot a lot more images straight 
off, but it would be good to get a list of what images people are 
actually using to ensure that there are no regressions.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Peter Maydell]

On 23 December 2013 06:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
> So, this patch is a small functional change, as it adds a copy of the
> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live with
> it.
>
> We'll be able to remove it once we switch to another firmware which uses the
> right reset instruction pointer or whose size is 1MB.
>
>>> +    /* Open Hack'Ware hack: bios size is 512K and is loaded at
>>> 0xfff00000.
>>> +     * However, reset address is 0xfffffffc. Mirror the bios from
>>> +     * 0xfff00000 to 0xfff80000.
>>> +     */
>>> +    memory_region_init_alias(bios, NULL, "bios-alias", sysmem,
>>> 0xfff00000,
>>> +                             0x00080000);
>>> +    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);

This code creates the mirrored region regardless of the size of the
firmware blob, right? I think that means that if we do supply a
1MB blob it'll do the wrong thing. You probably want to have some
"mirror this object as many times as necessary to fill the space"
logic.

We could probably do with having a generic MemoryRegion
API for that, actually -- it's not uncommon behaviour for devices
to be accessible every N bytes because they simply don't
decode the full set of address lines.

memory_region_add_subregion_tiled(MemoryRegion *mr,
                                 hwaddr offset, hwaddr tilelen,
                                 MemoryRegion *subregion)

to add copies of subregion to container mr starting at offset
for tilelen bytes, maybe? (we assume subregion to be created
at the length that each 'tile' should be, so don't need to pass
that too).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Peter Maydell a écrit :
> On 23 December 2013 06:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> So, this patch is a small functional change, as it adds a copy of the
>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can live with
>> it.
>>
>> We'll be able to remove it once we switch to another firmware which uses the
>> right reset instruction pointer or whose size is 1MB.
>>
>>>> +    /* Open Hack'Ware hack: bios size is 512K and is loaded at
>>>> 0xfff00000.
>>>> +     * However, reset address is 0xfffffffc. Mirror the bios from
>>>> +     * 0xfff00000 to 0xfff80000.
>>>> +     */
>>>> +    memory_region_init_alias(bios, NULL, "bios-alias", sysmem,
>>>> 0xfff00000,
>>>> +                             0x00080000);
>>>> +    memory_region_add_subregion_overlap(sysmem, 0xfff80000, bios, 1);
> 
> This code creates the mirrored region regardless of the size of the
> firmware blob, right? I think that means that if we do supply a
> 1MB blob it'll do the wrong thing. You probably want to have some
> "mirror this object as many times as necessary to fill the space"
> logic.
> 
> We could probably do with having a generic MemoryRegion
> API for that, actually -- it's not uncommon behaviour for devices
> to be accessible every N bytes because they simply don't
> decode the full set of address lines.
> 
> memory_region_add_subregion_tiled(MemoryRegion *mr,
>                                  hwaddr offset, hwaddr tilelen,
>                                  MemoryRegion *subregion)
> 
> to add copies of subregion to container mr starting at offset
> for tilelen bytes, maybe? (we assume subregion to be created
> at the length that each 'tile' should be, so don't need to pass
> that too).

This hack is meant to exist only as long as OHW has not been replaced by 
something else. That's a hack which has to be used only for *current OHW 
firmware* (ie 512KB) and only for *a short time*. I've already patches 
to replace OHW by OpenBIOS, but some details need some more polish.

So, I don't want to invest too much time to polish this hack. Choose 
whatever you want, but I don't want to take more time to push this patchset.

If you really don't like it, I can provide a OHW image which is 1MB, so 
this hack becomes moot. It will be created by concatenating 2 512KB OHW 
images in a 1MB image.

Regards,

Hervé

[Qemu-devel] [PATCH v3 04/10] raven: rename intack region to pci_intack

[Hervé Poussineau]

Regions added in next patches will also have the pci_ prefix.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 4dc5456..aaf5818 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -51,9 +51,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;
 
@@ -147,8 +147,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.7.10.4

[Qemu-devel] [PATCH v3 05/10] raven: set a correct PCI I/O memory region

[Hervé Poussineau]

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>
---
 hw/pci-host/prep.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index aaf5818..22e0b88 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -53,6 +53,7 @@ typedef struct PRePPCIState {
 
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
+    MemoryRegion pci_io;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
 } PREPPCIState;
@@ -136,13 +137,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);
@@ -160,11 +159,15 @@ 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);
+
+    /* CPU address space */
+    memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
     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.7.10.4

Re: [Qemu-devel] [PATCH v3 05/10] raven: set a correct PCI I/O memory region

[Andreas Färber]

Am 05.11.2013 00:09, schrieb Hervé Poussineau:
> 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>

With this patch I get only a blank screen in OHW/Etch.

Regards,
Andreas

Re: [Qemu-devel] [PATCH v3 05/10] raven: set a correct PCI I/O memory region

[Hervé Poussineau]

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

Le 13/03/2014 18:09, Andreas Färber a écrit :
> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>> 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>
>
> With this patch I get only a blank screen in OHW/Etch.

There seems to be conflict with the PCI I/O region added by this patch with the memory region registered by PReP machine to handle non-contiguous I/O.
Attached patch fixes it, but it may break non-contiguous I/O up to patch 08/10 (I've no mean to test it)
Another possibility is to merge patches 05 and 08 into one, as 06/10 and 07/10 can be freely reordered.

Regards,

Hervé

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-prep-repair-PReP-boot.patch --]
[-- Type: text/x-patch; name="0001-prep-repair-PReP-boot.patch", Size: 1082 bytes --]

>From 62cdb27e3902101a468fe34462f690d50f511c6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= <hpoussin@reactos.org>
Date: Thu, 13 Mar 2014 21:53:08 +0100
Subject: [PATCH] prep: repair PReP boot
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 9f8538c..f1476e8 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -624,7 +624,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     /* 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);
+    memory_region_add_subregion_overlap(sysmem, 0x80000000, PPC_io_memory, -1);
 
     /* init basic PC hardware */
     pci_vga_init(pci_bus);
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 06/10] raven: set a correct PCI memory region

[Hervé Poussineau]

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>
---
 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 22e0b88..8ff58a9 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -54,6 +54,7 @@ typedef struct PRePPCIState {
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
     MemoryRegion pci_io;
+    MemoryRegion pci_memory;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
 } PREPPCIState;
@@ -127,8 +128,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]);
     }
@@ -162,11 +161,15 @@ static void raven_pcihost_initfn(Object *obj)
     DeviceState *pci_dev;
 
     memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
+    /* Open Hack'Ware hack: real size should be only 0x3f000000 bytes */
+    memory_region_init(&s->pci_memory, obj, "pci-memory",
+                       0x3f000000 + 0xc0000000ULL);
 
     /* CPU address space */
     memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io);
+    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 8a09e2b..89da030 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -466,6 +466,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     int linux_boot, i, nb_nics1;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
+    MemoryRegion *vga = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base;
     long kernel_size, initrd_size;
     DeviceState *dev;
@@ -604,6 +605,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.7.10.4

[Qemu-devel] [PATCH v3 07/10] raven: add PCI bus mastering address space

[Hervé Poussineau]

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

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 8ff58a9..cffb21e 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -56,6 +56,10 @@ typedef struct PRePPCIState {
     MemoryRegion pci_io;
     MemoryRegion pci_memory;
     MemoryRegion pci_intack;
+    MemoryRegion bm;
+    MemoryRegion bm_ram_alias;
+    MemoryRegion bm_pci_memory_alias;
+    AddressSpace bm_as;
     RavenPCIState pci_dev;
 } PREPPCIState;
 
@@ -120,6 +124,13 @@ 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_pcihost_realizefn(DeviceState *d, Error **errp)
 {
     SysBusDevice *dev = SYS_BUS_DEVICE(d);
@@ -171,6 +182,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.7.10.4

[Qemu-devel] [PATCH v3 08/10] raven: implement non-contiguous I/O region

[Hervé Poussineau]

Remove now duplicated code from prep board.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/pci-host/prep.c |   82 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/prep.c      |   94 ++--------------------------------------------------
 2 files changed, 85 insertions(+), 91 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index cffb21e..c11679a 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -53,7 +53,9 @@ 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_memory;
     MemoryRegion pci_intack;
     MemoryRegion bm;
@@ -61,6 +63,8 @@ typedef struct PRePPCIState {
     MemoryRegion bm_pci_memory_alias;
     AddressSpace bm_as;
     RavenPCIState pci_dev;
+
+    int contiguous_map;
 } PREPPCIState;
 
 #define BIOS_SIZE (1024 * 1024)
@@ -112,6 +116,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 + 0x80000000, 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 {
+        assert(false);
+    }
+}
+
+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 {
+        assert(false);
+    }
+
+    address_space_write(&s->pci_io_as, addr + 0x80000000, 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;
@@ -131,6 +200,12 @@ static AddressSpace *raven_pcihost_set_iommu(PCIBus *bus, void *opaque,
     return &s->bm_as;
 }
 
+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);
@@ -143,6 +218,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,
@@ -172,12 +249,17 @@ static void raven_pcihost_initfn(Object *obj)
     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);
     /* 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,
                         &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 89da030..da4c5e8 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);
@@ -578,6 +494,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");
@@ -598,11 +515,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);
     /* Open Hack'Ware hack: PCI BAR#0 is programmed to 0xf0000000.
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v3 08/10] raven: implement non-contiguous I/O region

[Andreas Färber]

Am 05.11.2013 00:09, schrieb Hervé Poussineau:
> Remove now duplicated code from prep board.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

With this patch things start working again for me. Could you help find
out what needs to be squashed or reordered?

Thanks,
Andreas

[Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 c11679a..4eabe31 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -222,12 +222,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.7.10.4

[Qemu-devel] [PATCH v3 10/10] raven: use raven_ for all function prefixes

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 4eabe31..77cdfdd 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -69,7 +69,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;
 
@@ -81,36 +81,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,
     },
@@ -181,12 +181,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;
 
@@ -220,7 +220,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);
@@ -230,10 +231,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.7.10.4

Re: [Qemu-devel] [PATCH v3 00/10] prep: improve Raven PCI host emulation

[Andreas Färber]

Am 05.11.2013 00:09, schrieb Hervé Poussineau:
> Hervé Poussineau (10):
>   prep: kill get_system_io() usage
>   raven: use constant PCI_NUM_PINS instead of 4

Thanks and sorry for the delay, I've applied these to prep-up:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/prep-up
(An intermittent memory'ish regression has been fixed now apparently.)

>   raven: move BIOS loading from board code to PCI host

Hearing no objections or comments on this approach of loading firmware
in the PCI host bridge device, we have discussed that I'll be accepting
it to allow for easy sharing between in-tree and upcoming PReP machines.

However I'm still struggling to understand another aspect of the patch,
question inline.

Regards,
Andreas

>   raven: rename intack region to pci_intack
>   raven: set a correct PCI I/O memory region
>   raven: set a correct PCI memory region
>   raven: add PCI bus mastering address space
>   raven: implement non-contiguous I/O region
>   raven: fix PCI bus accesses with size > 1
>   raven: use raven_ for all function prefixes
> 
>  hw/pci-host/prep.c |  235 ++++++++++++++++++++++++++++++++++++++++++++--------
>  hw/ppc/prep.c      |  155 ++++++----------------------------
>  2 files changed, 226 insertions(+), 164 deletions(-)
> 

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Artyom Tarasenko]

On Tue, Nov 5, 2013 at 12:09 AM, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Without this patch PReP is broken really bad. Was going to submit the
same fix, and then found that the bug was already fixed 4 months ago.

Hope it helps getting it closer to master:

Tested-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  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 c11679a..4eabe31 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -222,12 +222,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.7.10.4
>
>



-- 
Regards,
Artyom Tarasenko

linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Artyom Tarasenko]

Hi Andreas, Hervé,

this patch seems still be missing in master. Is it causing any problems?

Artyom

On Mon, Feb 10, 2014 at 11:46 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> On Tue, Nov 5, 2013 at 12:09 AM, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>
> Without this patch PReP is broken really bad. Was going to submit the
> same fix, and then found that the bug was already fixed 4 months ago.
>
> Hope it helps getting it closer to master:
>
> Tested-by: Artyom Tarasenko <atar4qemu@gmail.com>
>
>> ---
>>  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 c11679a..4eabe31 100644
>> --- a/hw/pci-host/prep.c
>> +++ b/hw/pci-host/prep.c
>> @@ -222,12 +222,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);

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Andreas Färber]

Hi Artyom,

Am 16.03.2014 23:27, schrieb Artyom Tarasenko:
> Hi Andreas, Hervé,
> 
> this patch seems still be missing in master. Is it causing any problems?

It does not apply without the preceding patches. Here's my cherry-pick
result:

diff --cc hw/pci-host/prep.c
index 94fdffa,fed6c26..0000000
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@@ -133,17 -219,17 +133,27 @@@ static void raven_pcihost_realizefn(Dev
          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
);

++<<<<<<< HEAD
 +    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_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_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_le_ops, s,
+                           "pci-conf-data", 4);
+     memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
++>>>>>>> 67472dc... raven: fix PCI bus accesses with size > 1

      memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s,
"pciio", 0x00400000);
      memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);

I.e. we might change 1 -> 4 in the SysBus API, but would that work given
that endianness is being changed alongside?

If either of you could submit a version limited to bug fixes or explain
why the whole refactoring is needed as bug fix and provide a bisectable
version, I can certainly apply it for -rc1 if my test cases continue
working.

BTW another unresolved issue that's been discussed is whether we should
change the default CPU for -M prep. I've been open to doing so for 2.0
but would like some pointer that such a machine did exist rather than
just happens to work better with OpenBIOS.

Regards,
Andreas


> On Mon, Feb 10, 2014 at 11:46 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>> On Tue, Nov 5, 2013 at 12:09 AM, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>
>> Without this patch PReP is broken really bad. Was going to submit the
>> same fix, and then found that the bug was already fixed 4 months ago.
>>
>> Hope it helps getting it closer to master:
>>
>> Tested-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>
>>> ---
>>>  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 c11679a..4eabe31 100644
>>> --- a/hw/pci-host/prep.c
>>> +++ b/hw/pci-host/prep.c
>>> @@ -222,12 +222,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);

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Artyom Tarasenko]

Hi Andreas,

On Mon, Mar 17, 2014 at 8:59 PM, Andreas Färber <andreas.faerber@web.de> wrote:
>> this patch seems still be missing in master. Is it causing any problems?
>
> It does not apply without the preceding patches. Here's my cherry-pick
> result:
>[...]
> I.e. we might change 1 -> 4 in the SysBus API, but would that work given
> that endianness is being changed alongside?

Yes, and that's the point of this patch. PCI configuration space is
little-endian. With 1 byte access size, no byte swapping happens, so
the bug is hidden.
But on 32- and 16- bit accesses the bytes are swapped.

> If either of you could submit a version limited to bug fixes or explain
> why the whole refactoring is needed as bug fix and provide a bisectable
> version, I can certainly apply it for -rc1 if my test cases continue
> working.

No refactoring is necessary: only be->le and 1->4, and this is a pure
bugfix, which has no side effects because OHW seems to use 1 byte
accesses only.

> BTW another unresolved issue that's been discussed is whether we should
> change the default CPU for -M prep. I've been open to doing so for 2.0
> but would like some pointer that such a machine did exist

That's fair. I don't have any preference here though, as long as the
necessary cpu can be selected via the command line.

> rather than just happens to work better with OpenBIOS.

Oh, there is a compatible version of OpenBIOS available?! Are the
binaries shared somewhere?
BTW is there any Linux distribution newer than Debian Woody available for PReP?

Artyom

>> On Mon, Feb 10, 2014 at 11:46 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>>> On Tue, Nov 5, 2013 at 12:09 AM, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>
>>> Without this patch PReP is broken really bad. Was going to submit the
>>> same fix, and then found that the bug was already fixed 4 months ago.
>>>
>>> Hope it helps getting it closer to master:
>>>
>>> Tested-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>>
>>>> ---
>>>>  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 c11679a..4eabe31 100644
>>>> --- a/hw/pci-host/prep.c
>>>> +++ b/hw/pci-host/prep.c
>>>> @@ -222,12 +222,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);
>



-- 
Regards,
Artyom Tarasenko

linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Hervé Poussineau]

Hi,
Le lun. 17 mars 2014 22:55:37 CET, Artyom Tarasenko a écrit :
[...]
>> BTW another unresolved issue that's been discussed is whether we should
>> change the default CPU for -M prep. I've been open to doing so for 2.0
>> but would like some pointer that such a machine did exist
>
> That's fair. I don't have any preference here though, as long as the
> necessary cpu can be selected via the command line.
>
>> rather than just happens to work better with OpenBIOS.
>
> Oh, there is a compatible version of OpenBIOS available?! Are the
> binaries shared somewhere?

You can check http://repo.or.cz/w/qemu/hpoussin.git , branch raven.
It contains the patchset I just sent, and a few more patches to switch 
to OpenBIOS on PReP.
The required binary is already committed in QEMU (pc-bios/openbios-ppc)

> BTW is there any Linux distribution newer than Debian Woody available for PReP?

My PReP machine is running Debian Woody with a custom 2.4.22 kernel. 
Enough for most things :)

Hervé

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Andreas Färber]

Am 17.03.2014 23:25, schrieb Hervé Poussineau:
> Hi,
> Le lun. 17 mars 2014 22:55:37 CET, Artyom Tarasenko a écrit :
> [...]
>>> BTW another unresolved issue that's been discussed is whether we should
>>> change the default CPU for -M prep. I've been open to doing so for 2.0
>>> but would like some pointer that such a machine did exist
>>
>> That's fair. I don't have any preference here though, as long as the
>> necessary cpu can be selected via the command line.
>>
>>> rather than just happens to work better with OpenBIOS.
>>
>> Oh, there is a compatible version of OpenBIOS available?! Are the
>> binaries shared somewhere?
> 
> You can check http://repo.or.cz/w/qemu/hpoussin.git , branch raven.
> It contains the patchset I just sent, and a few more patches to switch
> to OpenBIOS on PReP.
> The required binary is already committed in QEMU (pc-bios/openbios-ppc)

Do you or Mark want to mention that in the 2.0 release notes somehow?

http://wiki.qemu.org/ChangeLog/2.0#Power

On the other hand, do your patches differ from my two? If so, how? I
think I still have an ugly #if 0 somewhere...

Regards,
Andreas

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Mark Cave-Ayland]

On 17/03/14 21:55, Artyom Tarasenko wrote:

> Hi Andreas,
>
> On Mon, Mar 17, 2014 at 8:59 PM, Andreas Färber<andreas.faerber@web.de>  wrote:
>>> this patch seems still be missing in master. Is it causing any problems?
>>
>> It does not apply without the preceding patches. Here's my cherry-pick
>> result:
>> [...]
>> I.e. we might change 1 ->  4 in the SysBus API, but would that work given
>> that endianness is being changed alongside?
>
> Yes, and that's the point of this patch. PCI configuration space is
> little-endian. With 1 byte access size, no byte swapping happens, so
> the bug is hidden.
> But on 32- and 16- bit accesses the bytes are swapped.
>
>> If either of you could submit a version limited to bug fixes or explain
>> why the whole refactoring is needed as bug fix and provide a bisectable
>> version, I can certainly apply it for -rc1 if my test cases continue
>> working.
>
> No refactoring is necessary: only be->le and 1->4, and this is a pure
> bugfix, which has no side effects because OHW seems to use 1 byte
> accesses only.

Yes, this is my understanding of the patch. However I also see that 
Hervé has just posted a revised raven patchset. If this patchset passes 
testing, I'd be inclined to apply it for 2.0 mainly because Hervé has 
done a great deal of testing on real OSs during development over the 
last 4 months and I'd bet that this fixes many more bugs than it would 
likely introduce.

Andreas, what's your test harness for PReP look like? Can you point us 
towards specific ISOs so that we can try these patches out?

>> BTW another unresolved issue that's been discussed is whether we should
>> change the default CPU for -M prep. I've been open to doing so for 2.0
>> but would like some pointer that such a machine did exist
>
> That's fair. I don't have any preference here though, as long as the
> necessary cpu can be selected via the command line.

I think I'd prefer to stick with the 600 series as I know there has been 
some talk of people wanting to run BeOS under QEMU, and given that OHW 
runs fine under an emulated 600 series processor then we should try and 
keep compatibility with that.

Given where we are time-wise, my preference would be to do the switch to 
OpenBIOS during the 2.1 cycle.

>> rather than just happens to work better with OpenBIOS.
>
> Oh, there is a compatible version of OpenBIOS available?! Are the
> binaries shared somewhere?

Yes - it's called git master ;) Make sure you apply Hervé's patcheset 
just posted to the list, plus the last 3 [RFC] OpenBIOS patches at 
http://repo.or.cz/w/qemu/hpoussin.git/shortlog/refs/heads/raven.

With those in place you should be able to launch something like this:

./qemu-system-ppc -M prep -cpu 750 -bios openbios-ppc

As I mentioned above, the main problem with switching is that OpenBIOS 
doesn't seem to (yet) work with the PReP machine's default 604 processor.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Andreas Färber]

Am 17.03.2014 22:55, schrieb Artyom Tarasenko:
> Hi Andreas,
> 
> On Mon, Mar 17, 2014 at 8:59 PM, Andreas Färber <andreas.faerber@web.de> wrote:
>>> this patch seems still be missing in master. Is it causing any problems?
>>
>> It does not apply without the preceding patches. Here's my cherry-pick
>> result:
>> [...]
>> I.e. we might change 1 -> 4 in the SysBus API, but would that work given
>> that endianness is being changed alongside?
> 
> Yes, and that's the point of this patch. PCI configuration space is
> little-endian. With 1 byte access size, no byte swapping happens, so
> the bug is hidden.
> But on 32- and 16- bit accesses the bytes are swapped.

I think you misunderstood my question: I wanted to know whether
switching from SysBus API to pure MemoryRegion API was part of the fix
or an independent cleanup. But now that I'm going through the respun
series it doesn't really matter any more...

Andreas