Re: [PATCH 2/2] ppc/pnv: Implement POWER9 LPC PSI serirq outputs and auto-clear function

[Miles Glenn <milesg@linux.ibm.com>]

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Tue, 2024-05-28 at 16:20 +1000, 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 <npiggin@gmail.com>
> ---
> Since v1:
> - Fix and test power8
> - Rebase onto Glenn's fix
> - Move irq_to_serirq_route from global into PnvLpcController
> - Don't have SERIRQ irqs latch the OPB irq status register, docs
> don't
>   suggest they do and skiboot does not clear that bit for SERIRQ
> path.
> - Have the SERIRQ path use the LPCHC IRQ mask (missed in previous
>   patch).
> 
>  include/hw/ppc/pnv_lpc.h |  14 ++++-
>  hw/ppc/pnv.c             |  36 +++++++++--
>  hw/ppc/pnv_lpc.c         | 128 ++++++++++++++++++++++++++++++++-----
> --
>  3 files changed, 148 insertions(+), 30 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index 97c6872c3f..e0fd5e4130 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -23,6 +23,7 @@
>  #include "exec/memory.h"
>  #include "hw/ppc/pnv.h"
>  #include "hw/qdev-core.h"
> +#include "hw/isa/isa.h" /* For ISA_NUM_IRQS */
> 
>  #define TYPE_PNV_LPC "pnv-lpc"
>  typedef struct PnvLpcClass PnvLpcClass;
> @@ -87,8 +88,19 @@ 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_has_serirq;
> +
>      /* PSI to generate interrupts */
> -    qemu_irq psi_irq;
> +    qemu_irq psi_irq_lpchc;
> +
> +    /* P9 serirq lines and irq routing table */
> +    qemu_irq psi_irq_serirq[4];
> +    int irq_to_serirq_route[ISA_NUM_IRQS];
>  };
> 
>  struct PnvLpcClass {
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 6e3a5ccdec..f6c3e91b3a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -728,7 +728,8 @@ static ISABus *pnv_chip_power8_isa_create(PnvChip
> *chip, Error **errp)
>      Pnv8Chip *chip8 = PNV8_CHIP(chip);
>      qemu_irq irq = qdev_get_gpio_in(DEVICE(&chip8->psi),
> PSIHB_IRQ_EXTERNAL);
> 
> -    qdev_connect_gpio_out(DEVICE(&chip8->lpc), 0, irq);
> +    qdev_connect_gpio_out_named(DEVICE(&chip8->lpc), "LPCHC", 0,
> irq);
> +
>      return pnv_lpc_isa_create(&chip8->lpc, true, errp);
>  }
> 
> @@ -737,25 +738,48 @@ static ISABus
> *pnv_chip_power8nvl_isa_create(PnvChip *chip, Error **errp)
>      Pnv8Chip *chip8 = PNV8_CHIP(chip);
>      qemu_irq irq = qdev_get_gpio_in(DEVICE(&chip8->psi),
> PSIHB_IRQ_LPC_I2C);
> 
> -    qdev_connect_gpio_out(DEVICE(&chip8->lpc), 0, irq);
> +    qdev_connect_gpio_out_named(DEVICE(&chip8->lpc), "LPCHC", 0,
> irq);
> +
>      return pnv_lpc_isa_create(&chip8->lpc, false, 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);
> +    qemu_irq irq;
> +
> +    irq = qdev_get_gpio_in(DEVICE(&chip9->psi), PSIHB9_IRQ_LPCHC);
> +    qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "LPCHC", 0,
> irq);
> +
> +    irq = qdev_get_gpio_in(DEVICE(&chip9->psi),
> PSIHB9_IRQ_LPC_SIRQ0);
> +    qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 0,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip9->psi),
> PSIHB9_IRQ_LPC_SIRQ1);
> +    qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 1,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip9->psi),
> PSIHB9_IRQ_LPC_SIRQ2);
> +    qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 2,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip9->psi),
> PSIHB9_IRQ_LPC_SIRQ3);
> +    qdev_connect_gpio_out_named(DEVICE(&chip9->lpc), "SERIRQ", 3,
> irq);
> 
> -    qdev_connect_gpio_out(DEVICE(&chip9->lpc), 0, irq);
>      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);
> +    qemu_irq irq;
> +
> +    irq = qdev_get_gpio_in(DEVICE(&chip10->psi), PSIHB9_IRQ_LPCHC);
> +    qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "LPCHC", 0,
> irq);
> +
> +    irq = qdev_get_gpio_in(DEVICE(&chip10->psi),
> PSIHB9_IRQ_LPC_SIRQ0);
> +    qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 0,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip10->psi),
> PSIHB9_IRQ_LPC_SIRQ1);
> +    qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 1,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip10->psi),
> PSIHB9_IRQ_LPC_SIRQ2);
> +    qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 2,
> irq);
> +    irq = qdev_get_gpio_in(DEVICE(&chip10->psi),
> PSIHB9_IRQ_LPC_SIRQ3);
> +    qdev_connect_gpio_out_named(DEVICE(&chip10->lpc), "SERIRQ", 3,
> irq);
> 
> -    qdev_connect_gpio_out(DEVICE(&chip10->lpc), 0, irq);
>      return pnv_lpc_isa_create(&chip10->lpc, false, errp);
>  }
> 
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 252690dcaa..8d0895e6e8 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,32 +421,90 @@ static const MemoryRegionOps pnv_lpc_mmio_ops =
> {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
> 
> -static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
> +/* Program the POWER9 LPC irq to PSI serirq routing table */
> +static void pnv_lpc_eval_serirq_routes(PnvLpcController *lpc)
>  {
> -    bool lpc_to_opb_irq = false;
> +    int irq;
> 
> -    /* Update LPC controller to OPB line */
> -    if (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_EN) {
> -        uint32_t irqs;
> +    if (!lpc->psi_has_serirq) {
> +        if ((lpc->opb_irq_route0 & PPC_BITMASK(8, 13)) ||
> +            (lpc->opb_irq_route1 & PPC_BITMASK(4, 31))) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                "OPB: setting serirq routing on POWER8 system,
> ignoring.\n");
> +        }
> +        return;
> +    }
> 
> -        irqs = lpc->lpc_hc_irqstat & lpc->lpc_hc_irqmask;
> -        lpc_to_opb_irq = (irqs != 0);
> +    for (irq = 0; irq <= 13; irq++) {
> +        int serirq = (lpc->opb_irq_route1 >> (31 - 5 - (irq * 2))) &
> 0x3;
> +        lpc->irq_to_serirq_route[irq] = serirq;
>      }
> 
> -    /* We don't honor the polarity register, it's pointless and
> unused
> -     * anyway
> -     */
> -    if (lpc_to_opb_irq) {
> -        lpc->opb_irq_input |= OPB_MASTER_IRQ_LPC;
> -    } else {
> -        lpc->opb_irq_input &= ~OPB_MASTER_IRQ_LPC;
> +    for (irq = 14; irq < ISA_NUM_IRQS; irq++) {
> +        int serirq = (lpc->opb_irq_route0 >> (31 - 9 - (irq * 2))) &
> 0x3;
> +        lpc->irq_to_serirq_route[irq] = serirq;
>      }
> +}
> 
> -    /* Update OPB internal latch */
> -    lpc->opb_irq_stat |= lpc->opb_irq_input & lpc->opb_irq_mask;
> +static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
> +{
> +    uint32_t active_irqs = 0;
> +
> +    if (lpc->lpc_hc_irqstat & PPC_BITMASK32(16, 31)) {
> +        qemu_log_mask(LOG_UNIMP, "LPC HC Unimplemented irqs in
> IRQSTAT: "
> +                                 "0x%08"PRIx32"\n", lpc-
> >lpc_hc_irqstat);
> +    }
> +
> +    if (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_EN) {
> +        active_irqs = lpc->lpc_hc_irqstat & lpc->lpc_hc_irqmask;
> +    }
> 
>      /* Reflect the interrupt */
> -    qemu_set_irq(lpc->psi_irq, lpc->opb_irq_stat != 0);
> +    if (!lpc->psi_has_serirq) {
> +        /*
> +         * POWER8 ORs all irqs together (also with LPCHC internal
> interrupt
> +         * sources) and outputs a single line that raises the PSI
> LPCHC irq
> +         * which then latches an OPB IRQ status register that sends
> the irq
> +         * to PSI.
> +         */
> +        /* We don't honor the polarity register, it's pointless and
> unused
> +         * anyway
> +         */
> +        if (active_irqs) {
> +            lpc->opb_irq_input |= OPB_MASTER_IRQ_LPC;
> +        } else {
> +            lpc->opb_irq_input &= ~OPB_MASTER_IRQ_LPC;
> +        }
> +
> +        /* Update OPB internal latch */
> +        lpc->opb_irq_stat |= lpc->opb_irq_input & lpc->opb_irq_mask;
> +
> +        qemu_set_irq(lpc->psi_irq_lpchc, lpc->opb_irq_stat != 0);
> +    } else {
> +        /*
> +         * POWER9 and POWER10 have routing fields in OPB master
> registers that
> +         * send LPC irqs to 4 output lines that raise the PSI SERIRQ
> irqs.
> +         * These don't appear to get latched into an OPB register
> like the
> +         * LPCHC irqs.
> +         *
> +         * POWER9 LPC controller internal irqs still go via the OPB
> +         * and LPCHC PSI irqs like P8, but we have no such internal
> sources
> +         * modelled yet.
> +         */
> +        bool serirq_out[4] = { false, false, false, false };
> +        int irq;
> +
> +        for (irq = 0; irq < ISA_NUM_IRQS; irq++) {
> +            if (active_irqs & (LPC_HC_IRQ_SERIRQ0 >> irq)) {
> +                serirq_out[lpc->irq_to_serirq_route[irq]] = true;
> +            }
> +        }
> +
> +        qemu_set_irq(lpc->psi_irq_serirq[0], serirq_out[0]);
> +        qemu_set_irq(lpc->psi_irq_serirq[1], serirq_out[1]);
> +        qemu_set_irq(lpc->psi_irq_serirq[2], serirq_out[2]);
> +        qemu_set_irq(lpc->psi_irq_serirq[3], serirq_out[3]);
> +    }
>  }
> 
>  static uint64_t lpc_hc_read(void *opaque, hwaddr addr, unsigned
> size)
> @@ -543,10 +602,10 @@ static uint64_t opb_master_read(void *opaque,
> hwaddr addr, unsigned size)
>      uint64_t val = 0xfffffffffffffffful;
> 
>      switch (addr) {
> -    case OPB_MASTER_LS_ROUTE0: /* TODO */
> +    case OPB_MASTER_LS_ROUTE0:
>          val = lpc->opb_irq_route0;
>          break;
> -    case OPB_MASTER_LS_ROUTE1: /* TODO */
> +    case OPB_MASTER_LS_ROUTE1:
>          val = lpc->opb_irq_route1;
>          break;
>      case OPB_MASTER_LS_IRQ_STAT:
> @@ -575,11 +634,15 @@ static void opb_master_write(void *opaque,
> hwaddr addr,
>      PnvLpcController *lpc = opaque;
> 
>      switch (addr) {
> -    case OPB_MASTER_LS_ROUTE0: /* TODO */
> +    case OPB_MASTER_LS_ROUTE0:
>          lpc->opb_irq_route0 = val;
> +        pnv_lpc_eval_serirq_routes(lpc);
> +        pnv_lpc_eval_irqs(lpc);
>          break;
> -    case OPB_MASTER_LS_ROUTE1: /* TODO */
> +    case OPB_MASTER_LS_ROUTE1:
>          lpc->opb_irq_route1 = val;
> +        pnv_lpc_eval_serirq_routes(lpc);
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case OPB_MASTER_LS_IRQ_STAT:
>          lpc->opb_irq_stat &= ~val;
> @@ -664,6 +727,8 @@ static void pnv_lpc_power9_realize(DeviceState
> *dev, Error **errp)
>      PnvLpcClass *plc = PNV_LPC_GET_CLASS(dev);
>      Error *local_err = NULL;
> 
> +    object_property_set_bool(OBJECT(lpc), "psi-serirq", true,
> &error_abort);
> +
>      plc->parent_realize(dev, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -673,6 +738,9 @@ static void pnv_lpc_power9_realize(DeviceState
> *dev, Error **errp)
>      /* P9 uses a MMIO region */
>      memory_region_init_io(&lpc->xscom_regs, OBJECT(lpc),
> &pnv_lpc_mmio_ops,
>                            lpc, "lpcm", PNV9_LPCM_SIZE);
> +
> +    /* P9 LPC routes ISA irqs to 4 PSI SERIRQ lines */
> +    qdev_init_gpio_out_named(dev, lpc->psi_irq_serirq, "SERIRQ", 4);
>  }
> 
>  static void pnv_lpc_power9_class_init(ObjectClass *klass, void
> *data)
> @@ -751,13 +819,19 @@ static void pnv_lpc_realize(DeviceState *dev,
> Error **errp)
>      memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                  &lpc->lpc_hc_regs);
> 
> -    qdev_init_gpio_out(dev, &lpc->psi_irq, 1);
> +    qdev_init_gpio_out_named(dev, &lpc->psi_irq_lpchc, "LPCHC", 1);
>  }
> 
> +static Property pnv_lpc_properties[] = {
> +    DEFINE_PROP_BOOL("psi-serirq", PnvLpcController, psi_has_serirq,
> false),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> 
> +    device_class_set_props(dc, pnv_lpc_properties);
>      dc->realize = pnv_lpc_realize;
>      dc->desc = "PowerNV LPC Controller";
>      dc->user_creatable = false;
> @@ -803,7 +877,7 @@ static void pnv_lpc_isa_irq_handler_cpld(void
> *opaque, int n, int level)
>      }
> 
>      if (pnv->cpld_irqstate != old_state) {
> -        qemu_set_irq(lpc->psi_irq, pnv->cpld_irqstate != 0);
> +        qemu_set_irq(lpc->psi_irq_lpchc, pnv->cpld_irqstate != 0);
>      }
>  }
> 
> @@ -824,6 +898,13 @@ static void pnv_lpc_isa_irq_handler(void
> *opaque, int n, int level)
>          pnv_lpc_eval_irqs(lpc);
>      } else {
>          lpc->lpc_hc_irq_inputs &= ~irq_bit;
> +
> +        /* POWER9 adds an auto-clear mode that clears IRQSTAT bits
> on EOI */
> +        if (lpc->psi_has_serirq &&
> +            (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_AUTO_CLEAR)) {
> +            lpc->lpc_hc_irqstat &= ~irq_bit;
> +            pnv_lpc_eval_irqs(lpc);
> +        }
>      }
>  }
> 
> @@ -854,6 +935,7 @@ ISABus *pnv_lpc_isa_create(PnvLpcController *lpc,
> bool use_cpld, Error **errp)
>          handler = pnv_lpc_isa_irq_handler;
>      }
> 
> +    /* POWER has a 17th irq, QEMU only implements the 16 regular
> device irqs */
>      irqs = qemu_allocate_irqs(handler, lpc, ISA_NUM_IRQS);
> 
>      isa_bus_register_input_irqs(isa_bus, irqs);