On Thu, 28 Nov 2024 at 13:45, Akihiko Odaki <[email protected]>
wrote:

> On 2024/11/28 0:02, Phil Dennis-Jordan wrote:
> > From: Alexander Graf <[email protected]>
> >
> > Some boards such as vmapple don't do real legacy PCI IRQ swizzling.
> > Instead, they just keep allocating more board IRQ lines for each new
> > legacy IRQ. Let's support that mode by giving instantiators a new
> > "nr_irqs" property they can use to support more than 4 legacy IRQ lines.
> > In this mode, GPEX will export more IRQ lines, one for each device.
> >
> > Signed-off-by: Alexander Graf <[email protected]>
> > Signed-off-by: Phil Dennis-Jordan <[email protected]>
> > Reviewed-by: Akihiko Odaki <[email protected]>> --->
> > v4:
> >
> >   * Turned pair of IRQ arrays into array of structs.
> >   * Simplified swizzling logic selection.
> >
> >   hw/arm/sbsa-ref.c          |  2 +-
> >   hw/arm/virt.c              |  2 +-
> >   hw/i386/microvm.c          |  2 +-
> >   hw/loongarch/virt.c        |  2 +-
> >   hw/mips/loongson3_virt.c   |  2 +-
> >   hw/openrisc/virt.c         | 12 +++++------
> >   hw/pci-host/gpex.c         | 43 ++++++++++++++++++++++++++++++--------
> >   hw/riscv/virt.c            | 12 +++++------
> >   hw/xtensa/virt.c           |  2 +-
> >   include/hw/pci-host/gpex.h |  7 +++----
> >   10 files changed, 55 insertions(+), 31 deletions(-)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index e3195d54497..7e7322486c2 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -673,7 +673,7 @@ static void create_pcie(SBSAMachineState *sms)
> >       /* Map IO port space */
> >       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >                              qdev_get_gpio_in(sms->gic, irq + i));
> >           gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 1a381e9a2bd..8aa22ea3155 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1547,7 +1547,7 @@ static void create_pcie(VirtMachineState *vms)
> >       /* Map IO port space */
> >       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >                              qdev_get_gpio_in(vms->gic, irq + i));
> >           gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
> > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> > index 86637afa0f3..ce80596c239 100644
> > --- a/hw/i386/microvm.c
> > +++ b/hw/i386/microvm.c
> > @@ -139,7 +139,7 @@ static void create_gpex(MicrovmMachineState *mms)
> >                                       mms->gpex.mmio64.base,
> mmio64_alias);
> >       }
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >                              x86ms->gsi[mms->gpex.irq + i]);
> >       }
> > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> > index 9a635d1d3d3..50056384994 100644
> > --- a/hw/loongarch/virt.c
> > +++ b/hw/loongarch/virt.c
> > @@ -741,7 +741,7 @@ static void virt_devices_init(DeviceState *pch_pic,
> >       memory_region_add_subregion(get_system_memory(), VIRT_PCI_IO_BASE,
> >                                   pio_alias);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           sysbus_connect_irq(d, i,
> >                              qdev_get_gpio_in(pch_pic, 16 + i));
> >           gpex_set_irq_num(GPEX_HOST(gpex_dev), i, 16 + i);
> > diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> > index f3b6326cc59..884b5f23a99 100644
> > --- a/hw/mips/loongson3_virt.c
> > +++ b/hw/mips/loongson3_virt.c
> > @@ -458,7 +458,7 @@ static inline void
> loongson3_virt_devices_init(MachineState *machine,
> >                                   virt_memmap[VIRT_PCIE_PIO].base,
> s->pio_alias);
> >       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2,
> virt_memmap[VIRT_PCIE_PIO].base);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           irq = qdev_get_gpio_in(pic, PCIE_IRQ_BASE + i);
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
> >           gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ_BASE + i);
> > diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> > index 47d2c9bd3c7..6f053bf48e0 100644
> > --- a/hw/openrisc/virt.c
> > +++ b/hw/openrisc/virt.c
> > @@ -318,7 +318,7 @@ static void create_pcie_irq_map(void *fdt, char
> *nodename, int irq_base,
> >   {
> >       int pin, dev;
> >       uint32_t irq_map_stride = 0;
> > -    uint32_t full_irq_map[GPEX_NUM_IRQS * GPEX_NUM_IRQS * 6] = {};
> > +    uint32_t full_irq_map[PCI_NUM_PINS * PCI_NUM_PINS * 6] = {};
> >       uint32_t *irq_map = full_irq_map;
> >
> >       /*
> > @@ -330,11 +330,11 @@ static void create_pcie_irq_map(void *fdt, char
> *nodename, int irq_base,
> >        * possible slot) seeing the interrupt-map-mask will allow the
> table
> >        * to wrap to any number of devices.
> >        */
> > -    for (dev = 0; dev < GPEX_NUM_IRQS; dev++) {
> > +    for (dev = 0; dev < PCI_NUM_PINS; dev++) {
> >           int devfn = dev << 3;
> >
> > -        for (pin = 0; pin < GPEX_NUM_IRQS; pin++) {
> > -            int irq_nr = irq_base + ((pin + PCI_SLOT(devfn)) %
> GPEX_NUM_IRQS);
> > +        for (pin = 0; pin < PCI_NUM_PINS; pin++) {
> > +            int irq_nr = irq_base + ((pin + PCI_SLOT(devfn)) %
> PCI_NUM_PINS);
> >               int i = 0;
> >
> >               /* Fill PCI address cells */
> > @@ -357,7 +357,7 @@ static void create_pcie_irq_map(void *fdt, char
> *nodename, int irq_base,
> >       }
> >
> >       qemu_fdt_setprop(fdt, nodename, "interrupt-map", full_irq_map,
> > -                     GPEX_NUM_IRQS * GPEX_NUM_IRQS *
> > +                     PCI_NUM_PINS * PCI_NUM_PINS *
> >                        irq_map_stride * sizeof(uint32_t));
> >
> >       qemu_fdt_setprop_cells(fdt, nodename, "interrupt-map-mask",
> > @@ -409,7 +409,7 @@ static void openrisc_virt_pcie_init(OR1KVirtState
> *state,
> >       memory_region_add_subregion(get_system_memory(), pio_base, alias);
> >
> >       /* Connect IRQ lines. */
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           pcie_irq = get_per_cpu_irq(cpus, num_cpus, irq_base + i);
> >
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pcie_irq);
> > diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> > index e9cf455bf52..cd63aa2d3cf 100644
> > --- a/hw/pci-host/gpex.c
> > +++ b/hw/pci-host/gpex.c
> > @@ -32,6 +32,7 @@
> >   #include "qemu/osdep.h"
> >   #include "qapi/error.h"
> >   #include "hw/irq.h"
> > +#include "hw/pci/pci_bus.h"
> >   #include "hw/pci-host/gpex.h"
> >   #include "hw/qdev-properties.h"
> >   #include "migration/vmstate.h"
> > @@ -41,20 +42,25 @@
> >    * GPEX host
> >    */
> >
> > +struct GPEXIrq {
> > +    qemu_irq irq;
> > +    int irq_num;
> > +};
> > +
> >   static void gpex_set_irq(void *opaque, int irq_num, int level)
> >   {
> >       GPEXHost *s = opaque;
> >
> > -    qemu_set_irq(s->irq[irq_num], level);
> > +    qemu_set_irq(s->irq[irq_num].irq, level);
> >   }
> >
> >   int gpex_set_irq_num(GPEXHost *s, int index, int gsi)
> >   {
> > -    if (index >= GPEX_NUM_IRQS) {
> > +    if (index >= s->num_irqs) {
> >           return -EINVAL;
> >       }
> >
> > -    s->irq_num[index] = gsi;
> > +    s->irq[index].irq_num = gsi;
> >       return 0;
> >   }
> >
> > @@ -62,7 +68,7 @@ static PCIINTxRoute gpex_route_intx_pin_to_irq(void
> *opaque, int pin)
> >   {
> >       PCIINTxRoute route;
> >       GPEXHost *s = opaque;
> > -    int gsi = s->irq_num[pin];
> > +    int gsi = s->irq[pin].irq_num;
> >
> >       route.irq = gsi;
> >       if (gsi < 0) {
> > @@ -74,6 +80,13 @@ static PCIINTxRoute gpex_route_intx_pin_to_irq(void
> *opaque, int pin)
> >       return route;
> >   }
> >
> > +static int gpex_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
> > +{
> > +    PCIBus *bus = pci_device_root_bus(pci_dev);
> > +
> > +    return (PCI_SLOT(pci_dev->devfn) + pin) % bus->nirq;
> > +}
> > +
> >   static void gpex_host_realize(DeviceState *dev, Error **errp)
> >   {
> >       PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> > @@ -82,6 +95,8 @@ static void gpex_host_realize(DeviceState *dev, Error
> **errp)
> >       PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
> >       int i;
> >
> > +    s->irq = g_malloc0_n(s->num_irqs, sizeof(*s->irq));
> > +
> >       pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MAX);
> >       sysbus_init_mmio(sbd, &pex->mmio);
> >
> > @@ -128,19 +143,27 @@ static void gpex_host_realize(DeviceState *dev,
> Error **errp)
> >           sysbus_init_mmio(sbd, &s->io_ioport);
> >       }
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > -        sysbus_init_irq(sbd, &s->irq[i]);
> > -        s->irq_num[i] = -1;
> > +    for (i = 0; i < s->num_irqs; i++) {
> > +        sysbus_init_irq(sbd, &s->irq[i].irq);
> > +        s->irq[i].irq_num = -1;
> >       }
> >
> >       pci->bus = pci_register_root_bus(dev, "pcie.0", gpex_set_irq,
> > -                                     pci_swizzle_map_irq_fn, s,
> &s->io_mmio,
> > -                                     &s->io_ioport, 0, 4,
> TYPE_PCIE_BUS);
> > +                                     gpex_swizzle_map_irq_fn,
> > +                                     s, &s->io_mmio, &s->io_ioport, 0,
> > +                                     s->num_irqs, TYPE_PCIE_BUS);
> >
> >       pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
> >       qdev_realize(DEVICE(&s->gpex_root), BUS(pci->bus), &error_fatal);
> >   }
> >
> > +static void gpex_host_unrealize(DeviceState *dev)
> > +{
> > +    GPEXHost *s = GPEX_HOST(dev);
> > +
> > +    g_free(s->irq);
> > +}
> > +
> >   static const char *gpex_host_root_bus_path(PCIHostState *host_bridge,
> >                                             PCIBus *rootbus)
> >   {
> > @@ -166,6 +189,7 @@ static Property gpex_host_properties[] = {
> >                          gpex_cfg.mmio64.base, 0),
> >       DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MMIO_SIZE, GPEXHost,
> >                        gpex_cfg.mmio64.size, 0),
> > +    DEFINE_PROP_UINT8("num-irqs", GPEXHost, num_irqs, PCI_NUM_PINS),
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >
> > @@ -176,6 +200,7 @@ static void gpex_host_class_init(ObjectClass *klass,
> void *data)
> >
> >       hc->root_bus_path = gpex_host_root_bus_path;
> >       dc->realize = gpex_host_realize;
> > +    dc->unrealize = gpex_host_unrealize;
> >       set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >       dc->fw_name = "pci";
> >       device_class_set_props(dc, gpex_host_properties);
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index 45a8c4f8190..567fe92a136 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -168,7 +168,7 @@ static void create_pcie_irq_map(RISCVVirtState *s,
> void *fdt, char *nodename,
> >   {
> >       int pin, dev;
> >       uint32_t irq_map_stride = 0;
> > -    uint32_t full_irq_map[GPEX_NUM_IRQS * GPEX_NUM_IRQS *
> > +    uint32_t full_irq_map[PCI_NUM_PINS * PCI_NUM_PINS *
> >                             FDT_MAX_INT_MAP_WIDTH] = {};
> >       uint32_t *irq_map = full_irq_map;
> >
> > @@ -180,11 +180,11 @@ static void create_pcie_irq_map(RISCVVirtState *s,
> void *fdt, char *nodename,
> >        * possible slot) seeing the interrupt-map-mask will allow the
> table
> >        * to wrap to any number of devices.
> >        */
> > -    for (dev = 0; dev < GPEX_NUM_IRQS; dev++) {
> > +    for (dev = 0; dev < PCI_NUM_PINS; dev++) {
> >           int devfn = dev * 0x8;
> >
> > -        for (pin = 0; pin < GPEX_NUM_IRQS; pin++) {
> > -            int irq_nr = PCIE_IRQ + ((pin + PCI_SLOT(devfn)) %
> GPEX_NUM_IRQS);
> > +        for (pin = 0; pin < PCI_NUM_PINS; pin++) {
> > +            int irq_nr = PCIE_IRQ + ((pin + PCI_SLOT(devfn)) %
> PCI_NUM_PINS);
> >               int i = 0;
> >
> >               /* Fill PCI address cells */
> > @@ -210,7 +210,7 @@ static void create_pcie_irq_map(RISCVVirtState *s,
> void *fdt, char *nodename,
> >       }
> >
> >       qemu_fdt_setprop(fdt, nodename, "interrupt-map", full_irq_map,
> > -                     GPEX_NUM_IRQS * GPEX_NUM_IRQS *
> > +                     PCI_NUM_PINS * PCI_NUM_PINS *
> >                        irq_map_stride * sizeof(uint32_t));
> >
> >       qemu_fdt_setprop_cells(fdt, nodename, "interrupt-map-mask",
> > @@ -1182,7 +1182,7 @@ static inline DeviceState
> *gpex_pcie_init(MemoryRegion *sys_mem,
> >
> >       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, pio_base);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           irq = qdev_get_gpio_in(irqchip, PCIE_IRQ + i);
> >
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
> > diff --git a/hw/xtensa/virt.c b/hw/xtensa/virt.c
> > index 5310a888613..8f5c2009d29 100644
> > --- a/hw/xtensa/virt.c
> > +++ b/hw/xtensa/virt.c
> > @@ -93,7 +93,7 @@ static void create_pcie(MachineState *ms,
> CPUXtensaState *env, int irq_base,
> >       /* Connect IRQ lines. */
> >       extints = xtensa_get_extints(env);
> >
> > -    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >           void *q = extints[irq_base + i];
> >
> >           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, q);
> > diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> > index dce883573ba..84471533af0 100644
> > --- a/include/hw/pci-host/gpex.h
> > +++ b/include/hw/pci-host/gpex.h
> > @@ -32,8 +32,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(GPEXHost, GPEX_HOST)
> >   #define TYPE_GPEX_ROOT_DEVICE "gpex-root"
> >   OBJECT_DECLARE_SIMPLE_TYPE(GPEXRootState, GPEX_ROOT_DEVICE)
> >
> > -#define GPEX_NUM_IRQS 4
>
> I found some references to GPEX_NUM_IRQS still remain:
>
> $ git grep GPEX_NUM_IRQS
> hw/loongarch/virt.c:    uint32_t full_irq_map[GPEX_NUM_IRQS
> *GPEX_NUM_IRQS * 10] = {};
> hw/loongarch/virt.c:    for (dev = 0; dev < GPEX_NUM_IRQS; dev++) {
> hw/loongarch/virt.c:        for (pin = 0; pin  < GPEX_NUM_IRQS; pin++) {
> hw/loongarch/virt.c:            int irq_nr = 16 + ((pin +
> PCI_SLOT(devfn)) % GPEX_NUM_IRQS);
> hw/loongarch/virt.c:                     GPEX_NUM_IRQS * GPEX_NUM_IRQS *
> hw/vmapple/vmapple.c:#define GPEX_NUM_IRQS 16
> hw/vmapple/vmapple.c:    qdev_prop_set_uint32(dev, "num-irqs",
> GPEX_NUM_IRQS);
> hw/vmapple/vmapple.c:    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> hw/xen/xen-pvh-common.c:    for (i = 0; i < GPEX_NUM_IRQS; i++) {
>
>
Good catch! These were added to the code base long after v1 of the patch
was posted…
It looks like all of these are safe to replace with PCI_NUM_PINS. (Except
the ones in vmapple.c, which #defines its own local value and is
intentional.)


> > -
> >   struct GPEXRootState {
> >       /*< private >*/
> >       PCIDevice parent_obj;
> > @@ -49,6 +47,7 @@ struct GPEXConfig {
> >       PCIBus      *bus;
> >   };
> >
> > +typedef struct GPEXIrq GPEXIrq;
> >   struct GPEXHost {
> >       /*< private >*/
> >       PCIExpressHost parent_obj;
> > @@ -60,8 +59,8 @@ struct GPEXHost {
> >       MemoryRegion io_mmio;
> >       MemoryRegion io_ioport_window;
> >       MemoryRegion io_mmio_window;
> > -    qemu_irq irq[GPEX_NUM_IRQS];
> > -    int irq_num[GPEX_NUM_IRQS];
> > +    GPEXIrq *irq;
> > +    uint8_t num_irqs;
> >
> >       bool allow_unmapped_accesses;
> >
>
>

Reply via email to