[Qemu-devel] [PATCH] 40p: fix PCI interrupt routing

[Qemu-devel] [PATCH] 40p: fix PCI interrupt routing

[Mark Cave-Ayland]

According to the PReP specification section 6.1.6 "System Interrupt
Assignments", all PCI interrupts are routed via IRQ 15.

With this patch applied it is now possible to boot the sandalfoot
zImage all the way through to a working userspace when using
OpenBIOS.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/prep.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 162b27a3b8..e82c1355d9 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
     dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
     qdev_connect_gpio_out(dev, 0,
                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
-    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
-    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
-    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
-    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
+    /* According to PReP specification section 6.1.6 "System Interrupt
+     * Assignments", all PCI interrupts are routed via IRQ 15 */
+    for (i = 0; i < PCI_NUM_PINS; i++) {
+        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
+    }
     isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
 
     /* Memory controller */
-- 
2.11.0

Re: [Qemu-devel] [PATCH] 40p: fix PCI interrupt routing

[Mark Cave-Ayland]

On 27/08/18 12:05, Mark Cave-Ayland wrote:

> According to the PReP specification section 6.1.6 "System Interrupt
> Assignments", all PCI interrupts are routed via IRQ 15.
> 
> With this patch applied it is now possible to boot the sandalfoot
> zImage all the way through to a working userspace when using
> OpenBIOS.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/ppc/prep.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 162b27a3b8..e82c1355d9 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
>      dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
>      qdev_connect_gpio_out(dev, 0,
>                            cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
> -    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
> -    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
> -    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
> -    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
> +    /* According to PReP specification section 6.1.6 "System Interrupt
> +     * Assignments", all PCI interrupts are routed via IRQ 15 */
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
> +    }
>      isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
>  
>      /* Memory controller */

Actually it seems that this is just working around the fact that a real
40p machine has certain hard-coded interrupts which only get used when
the residual data sets the machine name to "IBM PPS Model 6015".

In fact if you apply the following diff to OpenBIOS:

diff --git a/arch/ppc/qemu/context.c b/arch/ppc/qemu/context.c
index 06e0122..5815895 100644
--- a/arch/ppc/qemu/context.c
+++ b/arch/ppc/qemu/context.c
@@ -111,7 +111,7 @@ static void *
 residual_build(uint32_t memsize, uint32_t load_base, uint32_t load_size)
 {
     residual_t *res;
-    const unsigned char model[] = "Qemu\0PPC\0";
+    const unsigned char model[] = "IBM PPS Model 6015\0";
     int i;

     res = malloc(sizeof(residual_t));

then QEMU/OpenBIOS can boot a working sandalfoot zImage but only by
coincidence: it just so happens that the PCI IRQ for device 12 maps to
PCI IRQ 2 using the standard algorithm, which in the original code above
maps to IRQ 13 which is reserved on some PReP machines for the SCSI
controller.

So I think there are 2 issues here:

1) The PReP interrupt controller needs to be remodelled to allow PCI
IRQs to be mapped in this way

This is of course possible, but would require implementing logic that
wouldn't be compatible with the existing -M prep machine. But should
this wait until the old -M prep machine has been removed?

2) Setting the machine name as above breaks NetBSD boot

As per this bug report from Artyom:
http://mail-index.netbsd.org/port-prep/2017/04/02/msg000109.html. I'm
inclined to think that the solution here is to get NetBSD to fix their code.

Hervé, what do you think is the best solution for now?


ATB,

Mark.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] 40p: fix PCI interrupt routing

[BALATON Zoltan]

On Mon, 27 Aug 2018, Mark Cave-Ayland wrote:
> According to the PReP specification section 6.1.6 "System Interrupt
> Assignments", all PCI interrupts are routed via IRQ 15.
>
> With this patch applied it is now possible to boot the sandalfoot
> zImage all the way through to a working userspace when using
> OpenBIOS.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/ppc/prep.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 162b27a3b8..e82c1355d9 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
>     dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
>     qdev_connect_gpio_out(dev, 0,
>                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
> -    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
> -    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
> -    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
> -    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
> +    /* According to PReP specification section 6.1.6 "System Interrupt
> +     * Assignments", all PCI interrupts are routed via IRQ 15 */
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
> +    }

I'm not sure but this looks similar to what we had with sam460ex:

http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00359.html

I think you may not connect multiple interrupts to the same host irq line 
this way but you either need an OR gate or handle it within the mapping in 
the PCI host model (which is what we ended up with for the sam460ex). 
Peter's suggestion was to do whichever matches real hardware the most if 
you can find out that (as noted here also with more explanation that could 
be useful):

http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00360.html

But I could be mistaken in this case, haven't checked it in detail.

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] [PATCH] 40p: fix PCI interrupt routing

[David Gibson]

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

On Mon, Aug 27, 2018 at 07:12:29PM +0200, BALATON Zoltan wrote:
> On Mon, 27 Aug 2018, Mark Cave-Ayland wrote:
> > According to the PReP specification section 6.1.6 "System Interrupt
> > Assignments", all PCI interrupts are routed via IRQ 15.
> > 
> > With this patch applied it is now possible to boot the sandalfoot
> > zImage all the way through to a working userspace when using
> > OpenBIOS.
> > 
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> > hw/ppc/prep.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> > index 162b27a3b8..e82c1355d9 100644
> > --- a/hw/ppc/prep.c
> > +++ b/hw/ppc/prep.c
> > @@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
> >     dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
> >     qdev_connect_gpio_out(dev, 0,
> >                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
> > -    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
> > -    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
> > -    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
> > -    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
> > +    /* According to PReP specification section 6.1.6 "System Interrupt
> > +     * Assignments", all PCI interrupts are routed via IRQ 15 */
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> > +        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
> > +    }
> 
> I'm not sure but this looks similar to what we had with sam460ex:
> 
> http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00359.html
> 
> I think you may not connect multiple interrupts to the same host irq line
> this way but you either need an OR gate or handle it within the mapping in
> the PCI host model (which is what we ended up with for the sam460ex).
> Peter's suggestion was to do whichever matches real hardware the most if you
> can find out that (as noted here also with more explanation that could be
> useful):
> 
> http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00360.html
> 
> But I could be mistaken in this case, haven't checked it in detail.

Yeah..  I was thinking this looked very familiar.  I believe you do
need to construct the OR explicitly, not just wire all the irqs to the
same line.

-- 
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: [Qemu-devel] [Qemu-ppc] [PATCH] 40p: fix PCI interrupt routing

[Mark Cave-Ayland]

On 27/08/18 18:12, BALATON Zoltan wrote:

> On Mon, 27 Aug 2018, Mark Cave-Ayland wrote:
>> According to the PReP specification section 6.1.6 "System Interrupt
>> Assignments", all PCI interrupts are routed via IRQ 15.
>>
>> With this patch applied it is now possible to boot the sandalfoot
>> zImage all the way through to a working userspace when using
>> OpenBIOS.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/ppc/prep.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 162b27a3b8..e82c1355d9 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
>>     dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
>>     qdev_connect_gpio_out(dev, 0,
>>                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
>> -    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
>> -    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
>> -    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
>> -    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
>> +    /* According to PReP specification section 6.1.6 "System Interrupt
>> +     * Assignments", all PCI interrupts are routed via IRQ 15 */
>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>> +        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
>> +    }
> 
> I'm not sure but this looks similar to what we had with sam460ex:
> 
> http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00359.html
> 
> I think you may not connect multiple interrupts to the same host irq
> line this way but you either need an OR gate or handle it within the
> mapping in the PCI host model (which is what we ended up with for the
> sam460ex). Peter's suggestion was to do whichever matches real hardware
> the most if you can find out that (as noted here also with more
> explanation that could be useful):
> 
> http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00360.html
> 
> But I could be mistaken in this case, haven't checked it in detail.

Thanks for the pointer. I now have a follow-up patchset that implements
this, however it seems the real 40p machine has a routing quirk: the LSI
SCSI device is separately routed to IRQ 13. So while it's not quite a
pure OR of the 4 PCI IRQs, it's fairly close...


ATB,

Mark.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] 40p: fix PCI interrupt routing

[David Gibson]

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

On Sat, Sep 08, 2018 at 09:47:55AM +0100, Mark Cave-Ayland wrote:
> On 27/08/18 18:12, BALATON Zoltan wrote:
> 
> > On Mon, 27 Aug 2018, Mark Cave-Ayland wrote:
> >> According to the PReP specification section 6.1.6 "System Interrupt
> >> Assignments", all PCI interrupts are routed via IRQ 15.
> >>
> >> With this patch applied it is now possible to boot the sandalfoot
> >> zImage all the way through to a working userspace when using
> >> OpenBIOS.
> >>
> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> ---
> >> hw/ppc/prep.c | 9 +++++----
> >> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> >> index 162b27a3b8..e82c1355d9 100644
> >> --- a/hw/ppc/prep.c
> >> +++ b/hw/ppc/prep.c
> >> @@ -668,10 +668,11 @@ static void ibm_40p_init(MachineState *machine)
> >>     dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(11, 0), "i82378"));
> >>     qdev_connect_gpio_out(dev, 0,
> >>                           cpu->env.irq_inputs[PPC6xx_INPUT_INT]);
> >> -    sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(dev, 15));
> >> -    sysbus_connect_irq(pcihost, 1, qdev_get_gpio_in(dev, 13));
> >> -    sysbus_connect_irq(pcihost, 2, qdev_get_gpio_in(dev, 15));
> >> -    sysbus_connect_irq(pcihost, 3, qdev_get_gpio_in(dev, 13));
> >> +    /* According to PReP specification section 6.1.6 "System Interrupt
> >> +     * Assignments", all PCI interrupts are routed via IRQ 15 */
> >> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >> +        sysbus_connect_irq(pcihost, i, qdev_get_gpio_in(dev, 15));
> >> +    }
> > 
> > I'm not sure but this looks similar to what we had with sam460ex:
> > 
> > http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00359.html
> > 
> > I think you may not connect multiple interrupts to the same host irq
> > line this way but you either need an OR gate or handle it within the
> > mapping in the PCI host model (which is what we ended up with for the
> > sam460ex). Peter's suggestion was to do whichever matches real hardware
> > the most if you can find out that (as noted here also with more
> > explanation that could be useful):
> > 
> > http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00360.html
> > 
> > But I could be mistaken in this case, haven't checked it in detail.
> 
> Thanks for the pointer. I now have a follow-up patchset that implements
> this, however it seems the real 40p machine has a routing quirk: the LSI
> SCSI device is separately routed to IRQ 13. So while it's not quite a
> pure OR of the 4 PCI IRQs, it's fairly close...

That's very common on pre-Express PCI machines: on-board PCI devices
have their LSIs wired directly to the system interrupt controller
rather than through the PCI interrupt pins.  I think that's a separate
issue from the routing of the PCI irq pins.

-- 
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 --]