Re: [PATCH 4/8] hw/ppc/ppc440_bamboo: Drop use of ppcuic_init()

[Mark Cave-Ayland]

On 11/01/2021 17:14, Peter Maydell wrote:

> > On Sat, Dec 12, 2020 at 12:15:33AM +0000, Peter Maydell wrote:
> > > Switch the bamboo board to directly creating and configuring the UIC,
> > > rather than doing it via the old ppcuic_init() helper function.
>
> > Hopefully reporting this here is okay, I find Launchpad hard to use but
> > I can file it there if need be.
>
> Email reports are fine, thanks.
>
> > This patch causes a panic while trying to boot a ppc44x_defconfig Linux
> > kernel:
>
> Sorry, yes, this is a bug in this commit of mine. I didn't notice
> in the conversion that Bamboo has a non-standard DCR base register
> value. I'll just send a patch for this in a second.
>
> Thanks for the repro case.
>
> NB: with the fix for the the kernel proceeds on to the PCI
> related assert that it hit in 5.2.

FWIW the assert() seems to be the same problem that Guenter reported at 
https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg05847.html.

According to Linux's bamboo.dts the PCI slots look this:

    /* Bamboo has all 4 IRQ pins tied together per slot */
    interrupt-map-mask = <0xf800 0x0 0x0 0x0>;
    interrupt-map = <
        /* IDSEL 1 */
        0x800 0x0 0x0 0x0 &UIC0 0x1c 0x8

        /* IDSEL 2 */
        0x1000 0x0 0x0 0x0 &UIC0 0x1b 0x8

        /* IDSEL 3 */
        0x1800 0x0 0x0 0x0 &UIC0 0x1a 0x8

        /* IDSEL 4 */
        0x2000 0x0 0x0 0x0 &UIC0 0x19 0x8
    >;

But the assert() is suggesting that we are getting an PCI IRQ of -1 which can only 
come from slot 0 which isn't physically possible:

/* On Bamboo, all pins from each slot are tied to a single board IRQ. This
 * may need further refactoring for other boards. */
static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
{
    int slot = PCI_SLOT(pci_dev->devfn);

    trace_ppc4xx_pci_map_irq(pci_dev->devfn, irq_num, slot);

    return slot - 1;
}

And indeed the backtrace shows that the PCI host bridge itself is generating 
this IRQ:

(gdb) bt
#0  ppc4xx_pci_map_irq (pci_dev=0x555556c0efe0, irq_num=0) at 
../hw/ppc/ppc4xx_pci.c:250
#1  0x00005555559420d9 in pci_change_irq_level (pci_dev=0x555556c0efe0, irq_num=0, 
change=0) at ../hw/pci/pci.c:266
#2  0x0000555555944d56 in pci_update_irq_disabled (d=0x555556c0efe0, 
was_irq_disabled=0) at ../hw/pci/pci.c:1386
#3  0x0000555555945086 in pci_default_write_config (d=0x555556c0efe0, addr=4, 
val_in=1030, l=2) at ../hw/pci/pci.c:1426
#4  0x0000555555aa5b7e in pci_host_config_write_common (pci_dev=0x555556c0efe0, 
addr=4, limit=256, val=1030, len=2) at ../hw/pci/pci_host.c:83
#5  0x0000555555aa5ce2 in pci_data_write (s=0x555556c0e400, addr=2147483652, 
val=1030, len=2) at ../hw/pci/pci_host.c:120
#6  0x0000555555aa5e08 in pci_host_data_write (opaque=0x555556c0ce70, addr=0, 
val=1030, len=2) at ../hw/pci/pci_host.c:167
#7  0x0000555555cb4797 in memory_region_write_accessor (mr=0x555556c0d280, addr=0, 
value=0x7fffe60234a8, size=2, shift=0, mask=65535, attrs=...)
    at ../softmmu/memory.c:491

The assert() is correct since it correctly identifies an underflow of the PCI IRQ 
array but what is the best solution? I guess the possibilities are to either i) map 
the PCI host bridge IRQ to PCI IRQ 0 so at least it doesn't fail or ii) add an extra 
PCI IRQ 0 for the PCI host bridge and leave it unconnected.


ATB,

Mark.