Re: [PATCH v8 4/6] hw/ppc/pegasos2: Fix PCI interrupt routing

[BALATON Zoltan]

On Mon, 6 Mar 2023, Mark Cave-Ayland wrote:
> On 06/03/2023 12:33, BALATON Zoltan wrote:
> > According to the PegasosII schematics the PCI interrupt lines are
> > connected to both the gpp pins of the Mv64361 north bridge and the
> > PINT pins of the VT8231 south bridge so guests can get interrupts from
> > either of these. So far we only had the MV64361 connections which
> > worked for on board devices but for additional PCI devices (such as
> > network or sound card added with -device) guest OSes expect interrupt
> > from the ISA IRQ 9 where the firmware routes these PCI interrupts in
> > VT8231 ISA bridge. After the previous patches we can now model this
> > and also remove the board specific connection from mv64361. Also
> > configure routing of these lines when using Virtual Open Firmware to
> > match board firmware for guests that expect this.
> >
> > This fixes PCI interrupts on pegasos2 under Linux, MorphOS and AmigaOS.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > Tested-by: Rene Engel <ReneEngel80@emailn.de>
> > ---
> >   hw/pci-host/mv64361.c |  4 ----
> >   hw/ppc/pegasos2.c     | 26 +++++++++++++++++++++++++-
> >   2 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
> > index 298564f1f5..19e8031a3f 100644
> > --- a/hw/pci-host/mv64361.c
> > +++ b/hw/pci-host/mv64361.c
> > @@ -873,10 +873,6 @@ static void mv64361_realize(DeviceState *dev, Error 
> > **errp)
> >       }
> >       sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cpu_irq);
> >       qdev_init_gpio_in_named(dev, mv64361_gpp_irq, "gpp", 32);
> > -    /* FIXME: PCI IRQ connections may be board specific */
> > -    for (i = 0; i < PCI_NUM_PINS; i++) {
> > -        s->pci[1].irq[i] = qdev_get_gpio_in_named(dev, "gpp", 12 + i);
> > -    }
> >   }
> >     static void mv64361_reset(DeviceState *dev)
> > diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> > index b0ada9c963..ded5dc2dc9 100644
> > --- a/hw/ppc/pegasos2.c
> > +++ b/hw/ppc/pegasos2.c
> > @@ -73,6 +73,8 @@ struct Pegasos2MachineState {
> >       MachineState parent_obj;
> >       PowerPCCPU *cpu;
> >       DeviceState *mv;
> > +    qemu_irq mv_pirq[PCI_NUM_PINS];
> > +    qemu_irq via_pirq[PCI_NUM_PINS];
> >       Vof *vof;
> >       void *fdt_blob;
> >       uint64_t kernel_addr;
> > @@ -95,6 +97,15 @@ static void pegasos2_cpu_reset(void *opaque)
> >       }
> >   }
> >   +static void pegasos2_pci_irq(void *opaque, int n, int level)
> > +{
> > +    Pegasos2MachineState *pm = opaque;
> > +
> > +    /* PCI interrupt lines are connected to both MV64361 and VT8231 */
> > +    qemu_set_irq(pm->mv_pirq[n], level);
> > +    qemu_set_irq(pm->via_pirq[n], level);
> > +}
> > +
>
> Can you explain a bit more about how the PCI interrupt lines are connected to 
> both the MV64361 and VT8231? The reason for asking is that I see a similar 
> pattern in the bonito board, but there I can't see how those lines would be 
> used because they can also raise a CPU interrupt, yet it is a different one 
> compared to the 8259.
>
> Given that we know from Bernhard's tests that the fuloong2e board works with 
> pci_bus_irqs() included in via_isa_realize() which overwrites the bonito 
> equivalent, I'm wondering if the mv_pirq array is actually needed at all and

Also I'd be cautious of tests on fuloong2e unless it was done with binary 
known to work on real machine as we have found before that those binaries 
for the real machine expect IDE to use IRQ 14/15 like pegasos2 guests but 
the e.g. the default config in Linux would also work with native IRQs as 
documented in the datasheet but that's apparently not how it really works 
on real hardware (proven by binaries written for and tested on real 
hardware did not work with the model which followed the datasheet) so we 
had to change that later to match hardware. You may remember this, it was 
found around the time when we tested via-ide with different guests and 
some worked some didn't with the native mode IRQs. This suggests that 
datasheets and tests with code not verified to work on real hardware is 
unreliable (so are PCI standards that the VIA chip apparently does not 
follow). So I'd only trust guests that were running on the real machine.

Regards,
BALATON Zoltan