On 2024/11/28 0:02, Phil Dennis-Jordan wrote:
> From: Alexander Graf <graf@amazon.com>
>
> 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 <graf@amazon.com>
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>> --->
> 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;
>