On Mon May 13, 2024 at 9:49 PM AEST, Cédric Le Goater wrote:
> Hello,
>
> On 5/10/24 16:30, Nicholas Piggin wrote:
> > The POWER8 LPC ISA device irqs all get combined and reported to the line
> > connected the PSI LPCHC irq. POWER9 changed this so only internal LPC
> > host controller irqs use that line, and the device irqs get routed to
> > 4 new lines connected to PSI SERIRQ0-3.
> >
> > POWER9 also introduced a new feature that automatically clears the irq
> > status in the LPC host controller when EOI'ed, so software does not have
> > to.
> >
> > The powernv OPAL (skiboot) firmware managed to work because the LPCHC
> > irq handler scanned all LPC irqs and handled those including clearing
> > status even on POWER9 systems. So LPC irqs worked despite OPAL thinking
> > it was running in POWER9 mode. After this change, UART interrupts show
> > up on serirq1 which is where OPAL routes them to:
> >
> > cat /proc/interrupts
> > ...
> > 20: 0 XIVE-IRQ 1048563 Level opal-psi#0:lpchc
> > ...
> > 25: 34 XIVE-IRQ 1048568 Level opal-psi#0:lpc_serirq_mux1
> >
> > Whereas they previously turn up on lpchc.
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > include/hw/ppc/pnv_lpc.h | 12 ++++-
> > hw/ppc/pnv.c | 38 +++++++++++++--
> > hw/ppc/pnv_lpc.c | 100 +++++++++++++++++++++++++++++++++++----
> > 3 files changed, 136 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> > index 5d22c45570..57e324b4dc 100644
> > --- a/include/hw/ppc/pnv_lpc.h
> > +++ b/include/hw/ppc/pnv_lpc.h
> > @@ -84,8 +84,18 @@ struct PnvLpcController {
> > /* XSCOM registers */
> > MemoryRegion xscom_regs;
> >
> > + /*
> > + * In P8, ISA irqs are combined with internal sources to drive the
> > + * LPCHC interrupt output. P9 ISA irqs raise one of 4 lines that
> > + * drive PSI SERIRQ irqs, routing according to OPB routing registers.
> > + */
> > + bool psi_serirq;
> > +
> > /* PSI to generate interrupts */
> > - qemu_irq psi_irq;
> > + qemu_irq psi_irq_lpchc;
> > +
> > + /* P9 introduced a serirq mode */
> > + qemu_irq psi_irq_serirq[4];
> > };
> >
> > struct PnvLpcClass {
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 6e3a5ccdec..3b1c05a1d8 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -744,18 +744,48 @@ static ISABus *pnv_chip_power8nvl_isa_create(PnvChip
> > *chip, Error **errp)
> > static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp)
> > {
> > Pnv9Chip *chip9 = PNV9_CHIP(chip);
> > - qemu_irq irq = qdev_get_gpio_in(DEVICE(&chip9->psi), PSIHB9_IRQ_LPCHC);
> >
> > - qdev_connect_gpio_out(DEVICE(&chip9->lpc), 0, irq);
>
> The pnv_chip_power8*_isa_create() routines also need an update.
Good catch, thank you.
> > + qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "LPCHC", 0,
> > + qdev_get_gpio_in(DEVICE(&chip9->psi),
> > + PSIHB9_IRQ_LPCHC));
> > +
> > + qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 0,
> > + qdev_get_gpio_in(DEVICE(&chip9->psi),
> > + PSIHB9_IRQ_LPC_SIRQ0));
> > + qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 1,
> > + qdev_get_gpio_in(DEVICE(&chip9->psi),
> > + PSIHB9_IRQ_LPC_SIRQ1));
> > + qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 2,
> > + qdev_get_gpio_in(DEVICE(&chip9->psi),
> > + PSIHB9_IRQ_LPC_SIRQ2));
> > + qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 3,
> > + qdev_get_gpio_in(DEVICE(&chip9->psi),
> > + PSIHB9_IRQ_LPC_SIRQ3));
> > +
> > return pnv_lpc_isa_create(&chip9->lpc, false, errp);
> > }
> >
> > static ISABus *pnv_chip_power10_isa_create(PnvChip *chip, Error **errp)
> > {
> > Pnv10Chip *chip10 = PNV10_CHIP(chip);
> > - qemu_irq irq = qdev_get_gpio_in(DEVICE(&chip10->psi),
> > PSIHB9_IRQ_LPCHC);
> >
> > - qdev_connect_gpio_out(DEVICE(&chip10->lpc), 0, irq);
> > + qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "LPCHC", 0,
> > + qdev_get_gpio_in(DEVICE(&chip10->psi),
> > + PSIHB9_IRQ_LPCHC));
> > +
> > + qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 0,
> > + qdev_get_gpio_in(DEVICE(&chip10->psi),
> > + PSIHB9_IRQ_LPC_SIRQ0));
> > + qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 1,
> > + qdev_get_gpio_in(DEVICE(&chip10->psi),
> > + PSIHB9_IRQ_LPC_SIRQ1));
> > + qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 2,
> > + qdev_get_gpio_in(DEVICE(&chip10->psi),
> > + PSIHB9_IRQ_LPC_SIRQ2));
> > + qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 3,
> > + qdev_get_gpio_in(DEVICE(&chip10->psi),
> > + PSIHB9_IRQ_LPC_SIRQ3));
> > +
> > return pnv_lpc_isa_create(&chip10->lpc, false, errp);
> > }
> >
> > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> > index d692858bee..e28eae672f 100644
> > --- a/hw/ppc/pnv_lpc.c
> > +++ b/hw/ppc/pnv_lpc.c
> > @@ -64,6 +64,7 @@ enum {
> > #define LPC_HC_IRQSER_START_4CLK 0x00000000
> > #define LPC_HC_IRQSER_START_6CLK 0x01000000
> > #define LPC_HC_IRQSER_START_8CLK 0x02000000
> > +#define LPC_HC_IRQSER_AUTO_CLEAR 0x00800000
> > #define LPC_HC_IRQMASK 0x34 /* same bit defs as
> > LPC_HC_IRQSTAT */
> > #define LPC_HC_IRQSTAT 0x38
> > #define LPC_HC_IRQ_SERIRQ0 0x80000000 /* all bits down to
> > ... */
> > @@ -420,6 +421,34 @@ static const MemoryRegionOps pnv_lpc_mmio_ops = {
> > .endianness = DEVICE_BIG_ENDIAN,
> > };
> >
> > +/* POWER9 serirq routing, see below */
> > +static int irq_to_serirq_route[ISA_NUM_IRQS];
>
> This static is not friendly for multichip machines. Could we avoid it and
> move the IRQ routes under PnvLpcController ?
Ah yes I don't know what I was thinking. It's trivial to move under
the controller. I will post a v2 in a bit.
Thanks,
Nick
