Re: [PATCH 1/3] gpex-acpi: Support PCI link devices outside the host bridge

[Alistair Francis]

On Tue, May 28, 2024 at 5:32 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> Currently, PCI link devices (PNP0C0F) are always created within the
> scope of the PCI root complex. However, RISC-V needs PCI link devices to
> be outside the scope of the PCI host bridge to properly enable the probe
> order. This matches the example given in the ACPI specification section
> 6.2.13.1 as well.
>
> Enable creating link devices outside the scope of PCI root complex based
> on the flag which gets set currently only for RISC-V.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/pci-host/gpex-acpi.c    | 29 ++++++++++++++++++++++++-----
>  hw/riscv/virt-acpi-build.c |  8 +++++---
>  include/hw/pci-host/gpex.h |  5 ++++-
>  3 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index f69413ea2c..cea89a3ed8 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -7,7 +7,7 @@
>  #include "hw/pci/pcie_host.h"
>  #include "hw/acpi/cxl.h"
>
> -static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq)
> +static void acpi_dsdt_add_pci_route_table(Aml *scope, Aml *dev, uint32_t irq)
>  {
>      Aml *method, *crs;
>      int i, slot_no;
> @@ -45,7 +45,17 @@ static void acpi_dsdt_add_pci_route_table(Aml *dev, 
> uint32_t irq)
>          aml_append(dev_gsi, aml_name_decl("_CRS", crs));
>          method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
>          aml_append(dev_gsi, method);
> -        aml_append(dev, dev_gsi);
> +
> +        /*
> +         * Some architectures like RISC-V need PCI link devices created
> +         * outside the scope of the PCI host bridge similar to the example
> +         * given in the section 6.2.13.1 of ACPI spec 6.5.
> +         */
> +         if (scope) {
> +            aml_append(scope, dev_gsi);
> +        } else {
> +            aml_append(dev, dev_gsi);
> +        }
>      }
>  }
>
> @@ -174,7 +184,11 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>                  aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
>              }
>
> -            acpi_dsdt_add_pci_route_table(dev, cfg->irq);
> +            if (cfg->flags & GPEX_FLAGS_EXT_GSI_LINK) {
> +                acpi_dsdt_add_pci_route_table(scope, dev, cfg->irq);
> +            } else {
> +                acpi_dsdt_add_pci_route_table(NULL, dev, cfg->irq);
> +            }
>
>              /*
>               * Resources defined for PXBs are composed of the following 
> parts:
> @@ -205,7 +219,11 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>
> -    acpi_dsdt_add_pci_route_table(dev, cfg->irq);
> +    if (cfg->flags & GPEX_FLAGS_EXT_GSI_LINK) {
> +        acpi_dsdt_add_pci_route_table(scope, dev, cfg->irq);
> +    } else {
> +        acpi_dsdt_add_pci_route_table(NULL, dev, cfg->irq);
> +    }
>
>      method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
>      aml_append(method, aml_return(aml_int(cfg->ecam.base)));
> @@ -282,7 +300,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>      crs_range_set_free(&crs_range_set);
>  }
>
> -void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq)
> +void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq, uint32_t flags)
>  {
>      bool ambig;
>      Object *obj = object_resolve_path_type("", TYPE_GPEX_HOST, &ambig);
> @@ -292,5 +310,6 @@ void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq)
>      }
>
>      GPEX_HOST(obj)->gpex_cfg.irq = irq;
> +    GPEX_HOST(obj)->gpex_cfg.flags = flags;
>      acpi_dsdt_add_gpex(scope, &GPEX_HOST(obj)->gpex_cfg);
>  }
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 0925528160..832a3acb8d 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -417,19 +417,21 @@ static void build_dsdt(GArray *table_data,
>          virtio_acpi_dsdt_add(scope, memmap[VIRT_VIRTIO].base,
>                               memmap[VIRT_VIRTIO].size,
>                               VIRTIO_IRQ, 0, VIRTIO_COUNT);
> -        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ);
> +        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ, GPEX_FLAGS_EXT_GSI_LINK);
>      } else if (socket_count == 2) {
>          virtio_acpi_dsdt_add(scope, memmap[VIRT_VIRTIO].base,
>                               memmap[VIRT_VIRTIO].size,
>                               VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES, 0,
>                               VIRTIO_COUNT);
> -        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES);
> +        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
> +                                GPEX_FLAGS_EXT_GSI_LINK);
>      } else {
>          virtio_acpi_dsdt_add(scope, memmap[VIRT_VIRTIO].base,
>                               memmap[VIRT_VIRTIO].size,
>                               VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES, 0,
>                               VIRTIO_COUNT);
> -        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES * 
> 2);
> +        acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES * 
> 2,
> +                                GPEX_FLAGS_EXT_GSI_LINK);
>      }
>
>      aml_append(dsdt, scope);
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index dce883573b..bee17d62c5 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -47,8 +47,11 @@ struct GPEXConfig {
>      MemMapEntry pio;
>      int         irq;
>      PCIBus      *bus;
> +    uint32_t    flags;
>  };
>
> +#define GPEX_FLAGS_EXT_GSI_LINK BIT(0)
> +
>  struct GPEXHost {
>      /*< private >*/
>      PCIExpressHost parent_obj;
> @@ -71,7 +74,7 @@ struct GPEXHost {
>  int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>
>  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg);
> -void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq);
> +void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq, uint32_t flags);
>
>  #define PCI_HOST_PIO_BASE               "x-pio-base"
>  #define PCI_HOST_PIO_SIZE               "x-pio-size"
> --
> 2.40.1
>
>