[PATCH 0/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[PATCH 0/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

Fix a bug in the Raven PCI host, plus few cleanups while here.

Philippe Mathieu-Daudé (4):
  hw/pci-host/prep: Update coding style to make checkpatch.pl happy
  hw/pci-host/prep: Remove legacy PReP machine temporary workaround
  hw/pci-host/prep: Fix PCI swizzling in map_irq()
  docs/system/target-ppc.rst: Update PReP historical information

 docs/system/target-ppc.rst |  8 ++++----
 hw/pci-host/prep.c         | 32 ++++++++++++--------------------
 2 files changed, 16 insertions(+), 24 deletions(-)

-- 
2.26.2

[PATCH 1/4] hw/pci-host/prep: Update coding style to make checkpatch.pl happy

[Philippe Mathieu-Daudé]

To make the next commit easier to review, clean this code first.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/prep.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index d0323fefb10..80dfb67da43 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -234,8 +234,10 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
             sysbus_init_irq(dev, &s->pci_irqs[i]);
         }
     } else {
-        /* According to PReP specification section 6.1.6 "System Interrupt
-         * Assignments", all PCI interrupts are routed via IRQ 15 */
+        /*
+         * According to PReP specification section 6.1.6 "System Interrupt
+         * Assignments", all PCI interrupts are routed via IRQ 15
+         */
         s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
         object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
                                 &error_fatal);
-- 
2.26.2

[PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Philippe Mathieu-Daudé]

The legacy PReP machine has been removed in commit b2ce76a0730
("hw/ppc/prep: Remove the deprecated "prep" machine and the
OpenHackware BIOS"). This temporary workaround is no more
required, remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/prep.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 80dfb67da43..064593d1e52 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -75,7 +75,6 @@ struct PRePPCIState {
     RavenPCIState pci_dev;
 
     int contiguous_map;
-    bool is_legacy_prep;
 };
 
 #define BIOS_SIZE (1 * MiB)
@@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
     MemoryRegion *address_space_mem = get_system_memory();
     int i;
 
-    if (s->is_legacy_prep) {
-        for (i = 0; i < PCI_NUM_PINS; i++) {
-            sysbus_init_irq(dev, &s->pci_irqs[i]);
-        }
-    } else {
-        /*
-         * According to PReP specification section 6.1.6 "System Interrupt
-         * Assignments", all PCI interrupts are routed via IRQ 15
-         */
-        s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
-        object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
-                                &error_fatal);
-        qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
-        sysbus_init_irq(dev, &s->or_irq->out_irq);
+    /*
+     * According to PReP specification section 6.1.6 "System Interrupt
+     * Assignments", all PCI interrupts are routed via IRQ 15.
+     */
+    s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
+    object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
+                            &error_fatal);
+    qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
+    sysbus_init_irq(dev, &s->or_irq->out_irq);
 
-        for (i = 0; i < PCI_NUM_PINS; i++) {
-            s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
-        }
+    for (i = 0; i < PCI_NUM_PINS; i++) {
+        s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
     }
 
     qdev_init_gpio_in(d, raven_change_gpio, 1);
@@ -403,9 +396,6 @@ 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),
-    /* Temporary workaround until legacy prep machine is removed */
-    DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
-                     false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.26.2

[PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

In commit a01d8cadadf we changed the number of IRQs to 4 but
forgot to update the map_irq() function. Do it now.

Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Jocelyn Mayer <l_indien@magic.fr>
Cc: Julian Seward <julian@valgrind.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/prep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 064593d1e52..2224135fedb 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
 
 static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-    return (irq_num + (pci_dev->devfn >> 3)) & 1;
+    return (irq_num + (pci_dev->devfn >> 3)) & 3;
 }
 
 static void raven_set_irq(void *opaque, int irq_num, int level)
-- 
2.26.2

[PATCH 4/4] docs/system/target-ppc.rst: Update PReP historical information

[Philippe Mathieu-Daudé]

Link Jocelyn Mayer's web page from the Wayback Machine,
and correct the PReP acronym.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Jocelyn Mayer <l_indien@magic.fr>
---
 docs/system/target-ppc.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/system/target-ppc.rst b/docs/system/target-ppc.rst
index a2f04c533c2..1f57966771d 100644
--- a/docs/system/target-ppc.rst
+++ b/docs/system/target-ppc.rst
@@ -3,7 +3,7 @@
 PowerPC System emulator
 -----------------------
 
-Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PREP)
+Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PReP)
 or PowerMac PowerPC system.
 
 QEMU emulates the following PowerMac peripherals:
@@ -20,7 +20,7 @@ QEMU emulates the following PowerMac peripherals:
 
 -  VIA-CUDA with ADB keyboard and mouse.
 
-QEMU emulates the following 40P (PREP) peripherals:
+QEMU emulates the following 40P (PReP) peripherals:
 
 -  PCI Bridge
 
@@ -43,5 +43,5 @@ the g3beige and mac99 PowerMac and the 40p machines. OpenBIOS is a free
 (GPL v2) portable firmware implementation. The goal is to implement a
 100% IEEE 1275-1994 (referred to as Open Firmware) compliant firmware.
 
-More information is available at
-http://perso.magic.fr/l_indien/qemu-ppc/.
+Archived historical information is available at
+https://web.archive.org/web/20051212135300/http://perso.magic.fr/l_indien/qemu-ppc/.
-- 
2.26.2

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Thomas Huth]

On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:
> The legacy PReP machine has been removed in commit b2ce76a0730
> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
> OpenHackware BIOS"). This temporary workaround is no more
> required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/prep.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 80dfb67da43..064593d1e52 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -75,7 +75,6 @@ struct PRePPCIState {
>      RavenPCIState pci_dev;
>  
>      int contiguous_map;
> -    bool is_legacy_prep;
>  };
>  
>  #define BIOS_SIZE (1 * MiB)
> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>      MemoryRegion *address_space_mem = get_system_memory();
>      int i;
>  
> -    if (s->is_legacy_prep) {
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            sysbus_init_irq(dev, &s->pci_irqs[i]);
> -        }
> -    } else {
> -        /*
> -         * According to PReP specification section 6.1.6 "System Interrupt
> -         * Assignments", all PCI interrupts are routed via IRQ 15
> -         */
> -        s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> -        object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> -                                &error_fatal);
> -        qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> -        sysbus_init_irq(dev, &s->or_irq->out_irq);
> +    /*
> +     * According to PReP specification section 6.1.6 "System Interrupt
> +     * Assignments", all PCI interrupts are routed via IRQ 15.
> +     */

Since you're changing the indentation of these lines anyway, I think you
could also simply squash patch 1 into this one here (just a matter of taste,
though).

> +    s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> +    object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> +                            &error_fatal);
> +    qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> +    sysbus_init_irq(dev, &s->or_irq->out_irq);
>  
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
> -        }
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>      }
>  
>      qdev_init_gpio_in(d, raven_change_gpio, 1);
> @@ -403,9 +396,6 @@ 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),
> -    /* Temporary workaround until legacy prep machine is removed */
> -    DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
> -                     false),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 4/4] docs/system/target-ppc.rst: Update PReP historical information

[Thomas Huth]

On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:
> Link Jocelyn Mayer's web page from the Wayback Machine,
> and correct the PReP acronym.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Jocelyn Mayer <l_indien@magic.fr>
> ---
>  docs/system/target-ppc.rst | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/system/target-ppc.rst b/docs/system/target-ppc.rst
> index a2f04c533c2..1f57966771d 100644
> --- a/docs/system/target-ppc.rst
> +++ b/docs/system/target-ppc.rst
> @@ -3,7 +3,7 @@
>  PowerPC System emulator
>  -----------------------
>  
> -Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PREP)
> +Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PReP)
>  or PowerMac PowerPC system.
>  
>  QEMU emulates the following PowerMac peripherals:
> @@ -20,7 +20,7 @@ QEMU emulates the following PowerMac peripherals:
>  
>  -  VIA-CUDA with ADB keyboard and mouse.
>  
> -QEMU emulates the following 40P (PREP) peripherals:
> +QEMU emulates the following 40P (PReP) peripherals:
>  
>  -  PCI Bridge
>  
> @@ -43,5 +43,5 @@ the g3beige and mac99 PowerMac and the 40p machines. OpenBIOS is a free
>  (GPL v2) portable firmware implementation. The goal is to implement a
>  100% IEEE 1275-1994 (referred to as Open Firmware) compliant firmware.
>  
> -More information is available at
> -http://perso.magic.fr/l_indien/qemu-ppc/.
> +Archived historical information is available at
> +https://web.archive.org/web/20051212135300/http://perso.magic.fr/l_indien/qemu-ppc/.

There seems to be a later version available in the archive, too:

https://web.archive.org/web/20080212093125/http://perso.magic.fr/l_indien/qemu-ppc/

... but I think the contents did not change anymore. So:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Philippe Mathieu-Daudé]

On 10/12/20 9:28 AM, Thomas Huth wrote:
> On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:
>> The legacy PReP machine has been removed in commit b2ce76a0730
>> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
>> OpenHackware BIOS"). This temporary workaround is no more
>> required, remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/pci-host/prep.c | 32 +++++++++++---------------------
>>   1 file changed, 11 insertions(+), 21 deletions(-)
>>
>> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
>> index 80dfb67da43..064593d1e52 100644
>> --- a/hw/pci-host/prep.c
>> +++ b/hw/pci-host/prep.c
>> @@ -75,7 +75,6 @@ struct PRePPCIState {
>>       RavenPCIState pci_dev;
>>   
>>       int contiguous_map;
>> -    bool is_legacy_prep;
>>   };
>>   
>>   #define BIOS_SIZE (1 * MiB)
>> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>>       MemoryRegion *address_space_mem = get_system_memory();
>>       int i;
>>   
>> -    if (s->is_legacy_prep) {
>> -        for (i = 0; i < PCI_NUM_PINS; i++) {
>> -            sysbus_init_irq(dev, &s->pci_irqs[i]);
>> -        }
>> -    } else {
>> -        /*
>> -         * According to PReP specification section 6.1.6 "System Interrupt
>> -         * Assignments", all PCI interrupts are routed via IRQ 15
>> -         */
>> -        s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
>> -        object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
>> -                                &error_fatal);
>> -        qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
>> -        sysbus_init_irq(dev, &s->or_irq->out_irq);
>> +    /*
>> +     * According to PReP specification section 6.1.6 "System Interrupt
>> +     * Assignments", all PCI interrupts are routed via IRQ 15.
>> +     */
> 
> Since you're changing the indentation of these lines anyway, I think you
> could also simply squash patch 1 into this one here (just a matter of taste,
> though).

Then the diff is not trivial to review :/

> 
>> +    s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
>> +    object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
>> +                            &error_fatal);
>> +    qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
>> +    sysbus_init_irq(dev, &s->or_irq->out_irq);
>>   
>> -        for (i = 0; i < PCI_NUM_PINS; i++) {
>> -            s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>> -        }
>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>> +        s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>>       }
>>   
>>       qdev_init_gpio_in(d, raven_change_gpio, 1);
>> @@ -403,9 +396,6 @@ 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),
>> -    /* Temporary workaround until legacy prep machine is removed */
>> -    DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
>> -                     false),
>>       DEFINE_PROP_END_OF_LIST()
>>   };
>>   
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> The legacy PReP machine has been removed in commit b2ce76a0730
> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
> OpenHackware BIOS"). This temporary workaround is no more
> required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/prep.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 80dfb67da43..064593d1e52 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -75,7 +75,6 @@ struct PRePPCIState {
>      RavenPCIState pci_dev;
>  
>      int contiguous_map;
> -    bool is_legacy_prep;
>  };
>  
>  #define BIOS_SIZE (1 * MiB)
> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>      MemoryRegion *address_space_mem = get_system_memory();
>      int i;
>  
> -    if (s->is_legacy_prep) {
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            sysbus_init_irq(dev, &s->pci_irqs[i]);
> -        }
> -    } else {
> -        /*
> -         * According to PReP specification section 6.1.6 "System Interrupt
> -         * Assignments", all PCI interrupts are routed via IRQ 15
> -         */
> -        s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> -        object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> -                                &error_fatal);
> -        qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> -        sysbus_init_irq(dev, &s->or_irq->out_irq);
> +    /*
> +     * According to PReP specification section 6.1.6 "System Interrupt
> +     * Assignments", all PCI interrupts are routed via IRQ 15.
> +     */
> +    s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> +    object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> +                            &error_fatal);
> +    qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> +    sysbus_init_irq(dev, &s->or_irq->out_irq);
>  
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
> -        }
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>      }
>  
>      qdev_init_gpio_in(d, raven_change_gpio, 1);
> @@ -403,9 +396,6 @@ 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),
> -    /* Temporary workaround until legacy prep machine is removed */
> -    DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
> -                     false),
>      DEFINE_PROP_END_OF_LIST()
>  };

Yes indeed, this workaround is certainly no longer needed.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> In commit a01d8cadadf we changed the number of IRQs to 4 but
> forgot to update the map_irq() function. Do it now.
> 
> Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Jocelyn Mayer <l_indien@magic.fr>
> Cc: Julian Seward <julian@valgrind.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/prep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 064593d1e52..2224135fedb 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
>  
>  static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
> -    return (irq_num + (pci_dev->devfn >> 3)) & 1;
> +    return (irq_num + (pci_dev->devfn >> 3)) & 3;
>  }
>  
>  static void raven_set_irq(void *opaque, int irq_num, int level)

It feels like this should also have a corresponding change in OpenBIOS for
consistency, even though technically because of the OR on IRQ 15 it doesn't really
matter. The relevant part in OpenBIOS can be found here:
https://git.qemu.org/?p=openbios.git;a=blob;f=drivers/pci.c;h=34ae69a907b6312a3a7ab218afe8ba9efded1df7;hb=7f28286f5cb1ca682e3ba0a8706d8884f12bc49e#l2001
and in particular this section:

    /* Use the same "physical" routing as QEMU's raven_map_irq() although
       ultimately all 4 PCI interrupts are ORd to IRQ 15 as indicated
       by the PReP specification */
    props[(*ncells)++] = arch->irqs[((intno - 1) + (addr >> 11)) & 1];


ATB,

Mark.

Re: [PATCH 1/4] hw/pci-host/prep: Update coding style to make checkpatch.pl happy

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> To make the next commit easier to review, clean this code first.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/prep.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index d0323fefb10..80dfb67da43 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -234,8 +234,10 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>              sysbus_init_irq(dev, &s->pci_irqs[i]);
>          }
>      } else {
> -        /* According to PReP specification section 6.1.6 "System Interrupt
> -         * Assignments", all PCI interrupts are routed via IRQ 15 */
> +        /*
> +         * According to PReP specification section 6.1.6 "System Interrupt
> +         * Assignments", all PCI interrupts are routed via IRQ 15
> +         */
>          s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
>          object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
>                                  &error_fatal);

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

On 10/12/20 11:37 AM, Mark Cave-Ayland wrote:
> On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:
> 
>> In commit a01d8cadadf we changed the number of IRQs to 4 but
>> forgot to update the map_irq() function. Do it now.
>>
>> Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Cc: Jocelyn Mayer <l_indien@magic.fr>
>> Cc: Julian Seward <julian@valgrind.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/pci-host/prep.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
>> index 064593d1e52..2224135fedb 100644
>> --- a/hw/pci-host/prep.c
>> +++ b/hw/pci-host/prep.c
>> @@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
>>   
>>   static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
>>   {
>> -    return (irq_num + (pci_dev->devfn >> 3)) & 1;
>> +    return (irq_num + (pci_dev->devfn >> 3)) & 3;
>>   }
>>   
>>   static void raven_set_irq(void *opaque, int irq_num, int level)
> 
> It feels like this should also have a corresponding change in OpenBIOS for
> consistency, even though technically because of the OR on IRQ 15 it doesn't really
> matter. The relevant part in OpenBIOS can be found here:
> https://git.qemu.org/?p=openbios.git;a=blob;f=drivers/pci.c;h=34ae69a907b6312a3a7ab218afe8ba9efded1df7;hb=7f28286f5cb1ca682e3ba0a8706d8884f12bc49e#l2001
> and in particular this section:
> 
>      /* Use the same "physical" routing as QEMU's raven_map_irq() although
>         ultimately all 4 PCI interrupts are ORd to IRQ 15 as indicated
>         by the PReP specification */
>      props[(*ncells)++] = arch->irqs[((intno - 1) + (addr >> 11)) & 1];

Done:
https://github.com/openbios/openbios/pull/7

> 
> 
> ATB,
> 
> Mark.
> 

Re: [PATCH 1/4] hw/pci-host/prep: Update coding style to make checkpatch.pl happy

[David Gibson]

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

On Mon, Oct 12, 2020 at 09:19:03AM +0200, Philippe Mathieu-Daudé wrote:
> To make the next commit easier to review, clean this code first.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci-host/prep.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index d0323fefb10..80dfb67da43 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -234,8 +234,10 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>              sysbus_init_irq(dev, &s->pci_irqs[i]);
>          }
>      } else {
> -        /* According to PReP specification section 6.1.6 "System Interrupt
> -         * Assignments", all PCI interrupts are routed via IRQ 15 */
> +        /*
> +         * According to PReP specification section 6.1.6 "System Interrupt
> +         * Assignments", all PCI interrupts are routed via IRQ 15
> +         */
>          s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
>          object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
>                                  &error_fatal);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[David Gibson]

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

On Mon, Oct 12, 2020 at 09:19:04AM +0200, Philippe Mathieu-Daudé wrote:
> The legacy PReP machine has been removed in commit b2ce76a0730
> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
> OpenHackware BIOS"). This temporary workaround is no more
> required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci-host/prep.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 80dfb67da43..064593d1e52 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -75,7 +75,6 @@ struct PRePPCIState {
>      RavenPCIState pci_dev;
>  
>      int contiguous_map;
> -    bool is_legacy_prep;
>  };
>  
>  #define BIOS_SIZE (1 * MiB)
> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>      MemoryRegion *address_space_mem = get_system_memory();
>      int i;
>  
> -    if (s->is_legacy_prep) {
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            sysbus_init_irq(dev, &s->pci_irqs[i]);
> -        }
> -    } else {
> -        /*
> -         * According to PReP specification section 6.1.6 "System Interrupt
> -         * Assignments", all PCI interrupts are routed via IRQ 15
> -         */
> -        s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> -        object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> -                                &error_fatal);
> -        qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> -        sysbus_init_irq(dev, &s->or_irq->out_irq);
> +    /*
> +     * According to PReP specification section 6.1.6 "System Interrupt
> +     * Assignments", all PCI interrupts are routed via IRQ 15.
> +     */
> +    s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> +    object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> +                            &error_fatal);
> +    qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> +    sysbus_init_irq(dev, &s->or_irq->out_irq);
>  
> -        for (i = 0; i < PCI_NUM_PINS; i++) {
> -            s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
> -        }
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>      }
>  
>      qdev_init_gpio_in(d, raven_change_gpio, 1);
> @@ -403,9 +396,6 @@ 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),
> -    /* Temporary workaround until legacy prep machine is removed */
> -    DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
> -                     false),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]