| Message ID | 20191209160638.141431-1-andre.przywara@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | pcie: Add quirk for the Arm Neoverse N1SDP platform | expand |
On Mon, Dec 09, 2019 at 04:06:38PM +0000, Andre Przywara wrote: > From: Deepak Pandey <Deepak.Pandey@arm.com> > > The Arm N1SDP SoC suffers from some PCIe integration issues, most > prominently config space accesses to not existing BDFs being answered > with a bus abort, resulting in an SError. Can we tease this apart a little more? Linux doesn't program all the bits that control error signaling, so even on hardware that works perfectly, much of this behavior is determined by what firmware did. I wonder if Linux could be more careful about this. "Bus abort" is not a term used in PCIe. IIUC, a config read to a device that doesn't exist should terminate with an Unsupported Request completion, e.g., see the implementation note in PCIe r5.0 sec 2.3.1. The UR should be an uncorrectable non-fatal error (Table 6-5), and Figures 6-2 and 6-3 show how it should be handled and when it should be signaled as a system error. In case you don't have a copy of the spec, I extracted those two figures and put them at [1]. Can you collect "lspci -vvxxx" output to see if we can correlate it with those figures and the behavior you see? [1] https://drive.google.com/file/d/1ihhdQvr0a7ZEJG-3gPddw1Tq7cTFAsah/view?usp=sharing > To mitigate this, the firmware scans the bus before boot (catching the > SErrors) and creates a table with valid BDFs, which acts as a filter for > Linux' config space accesses. > > Add code consulting the table as an ACPI PCIe quirk, also register the > corresponding device tree based description of the host controller. > Also fix the other two minor issues on the way, namely not being fully > ECAM compliant and config space accesses being restricted to 32-bit > accesses only. As I'm sure you've noticed, controllers that support only 32-bit config writes are not spec compliant and devices may not work correctly. The comment in pci_generic_config_write32() explains why. You may not trip over this problem frequently, but I wouldn't call it a "minor" issue because when you *do* trip over it, you have no indication that a register was corrupted. Even ECAM compliance is not really minor -- if this controller were fully compliant with the spec, you would need ZERO Linux changes to support it. Every quirk like this means additional maintenance burden, and it's not just a one-time thing. It means old kernels that *should* "just work" on your system will not work unless somebody backports the quirk. > This allows the Arm Neoverse N1SDP board to boot Linux without crashing > and to access *any* devices (there are no platform devices except UART).
On Mon, Dec 09, 2019 at 04:06:38PM +0000, Andre Przywara wrote: > From: Deepak Pandey <Deepak.Pandey@arm.com> > > The Arm N1SDP SoC suffers from some PCIe integration issues, most > prominently config space accesses to not existing BDFs being answered > with a bus abort, resulting in an SError. It wouldn't be a surprise if the host controller handled UR completions differently depending on if they were in response to a Type 0 configuration request or a Type 1 configuration request. (I think I've seen this before). Have you verified that you still get a bus abort when you attempt to perform a config read of a non-existent device downstream of the PCIe switch? (and thus as a response to a Type 1 request). I ask because if this is the case, and knowing that the PCIe switch is fixed, then it would be possible to simplify this quirk (by just making assumptions of the presence of devices in bus 0 rather than all the busses). > To mitigate this, the firmware scans the bus before boot (catching the > SErrors) and creates a table with valid BDFs, which acts as a filter for > Linux' config space accesses. > > Add code consulting the table as an ACPI PCIe quirk, also register the > corresponding device tree based description of the host controller. > Also fix the other two minor issues on the way, namely not being fully > ECAM compliant and config space accesses being restricted to 32-bit > accesses only. > > This allows the Arm Neoverse N1SDP board to boot Linux without crashing > and to access *any* devices (there are no platform devices except UART). This implies that this quirk has no side-effects and everything will work as expected - but this is only true for the simple case. For example hot plug won't work, SR-IOV, and others won't work. Also what happens for devices that return CRS? - does this also result in an abort? Does that mean that the firmware will consider these devices as not present instead of not ready yet? If this is an issue, then FLR of devices will also create issues (resulting in SErrors for users). I think it would be helpful to update this commit message to indicate that this makes it work better, but it may be broken in certain ways. > > Signed-off-by: Deepak Pandey <Deepak.Pandey@arm.com> > [Sudipto: extend to cover the CCIX root port as well] > Signed-off-by: Sudipto Paul <sudipto.paul@arm.com> > [Andre: fix coding style issues, rewrite some parts, add DT support] > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm64/configs/defconfig | 1 + > drivers/acpi/pci_mcfg.c | 7 + > drivers/pci/controller/Kconfig | 11 ++ > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pcie-n1sdp.c | 196 ++++++++++++++++++++++++++++ > include/linux/pci-ecam.h | 2 + > 6 files changed, 218 insertions(+) > create mode 100644 drivers/pci/controller/pcie-n1sdp.c > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > index 6a83ba2aea3e..58124ef5070b 100644 > --- a/arch/arm64/configs/defconfig > +++ b/arch/arm64/configs/defconfig > @@ -177,6 +177,7 @@ CONFIG_NET_9P=y > CONFIG_NET_9P_VIRTIO=y > CONFIG_PCI=y > CONFIG_PCIEPORTBUS=y > +CONFIG_PCI_QUIRKS=y > CONFIG_PCI_IOV=y > CONFIG_HOTPLUG_PCI=y > CONFIG_HOTPLUG_PCI_ACPI=y > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c > index 6b347d9920cc..7a2b41b9ab57 100644 > --- a/drivers/acpi/pci_mcfg.c > +++ b/drivers/acpi/pci_mcfg.c > @@ -142,6 +142,13 @@ static struct mcfg_fixup mcfg_quirks[] = { > XGENE_V2_ECAM_MCFG(4, 0), > XGENE_V2_ECAM_MCFG(4, 1), > XGENE_V2_ECAM_MCFG(4, 2), > + > +#define N1SDP_ECAM_MCFG(rev, seg, ops) \ > + {"ARMLTD", "ARMN1SDP", rev, seg, MCFG_BUS_ANY, ops } > + > + /* N1SDP SoC with v1 PCIe controller */ > + N1SDP_ECAM_MCFG(0x20181101, 0, &pci_n1sdp_pcie_ecam_ops), > + N1SDP_ECAM_MCFG(0x20181101, 1, &pci_n1sdp_ccix_ecam_ops), > }; > > static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index c77069c8ee5d..45700d32f02e 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -37,6 +37,17 @@ config PCI_FTPCI100 > depends on OF > default ARCH_GEMINI > > +config PCIE_HOST_N1SDP_ECAM > + bool "ARM N1SDP PCIe Controller" > + depends on ARM64 > + depends on OF || (ACPI && PCI_QUIRKS) > + select PCI_HOST_COMMON > + default y if ARCH_VEXPRESS > + help > + Say Y here if you want PCIe support for the Arm N1SDP platform. > + The controller is ECAM compliant, but needs a quirk to workaround > + an integration issue. Again - please indicate the scope of support provided. > + > config PCI_TEGRA > bool "NVIDIA Tegra PCIe controller" > depends on ARCH_TEGRA || COMPILE_TEST > diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile > index 3d4f597f15ce..5f47fefbd67d 100644 > --- a/drivers/pci/controller/Makefile > +++ b/drivers/pci/controller/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o > obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o > obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o > obj-$(CONFIG_VMD) += vmd.o > +obj-$(CONFIG_PCIE_HOST_N1SDP_ECAM) += pcie-n1sdp.o > # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW > obj-y += dwc/ > > diff --git a/drivers/pci/controller/pcie-n1sdp.c b/drivers/pci/controller/pcie-n1sdp.c > new file mode 100644 > index 000000000000..620ab221466c > --- /dev/null > +++ b/drivers/pci/controller/pcie-n1sdp.c > @@ -0,0 +1,196 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018/2019 ARM Ltd. > + * > + * This quirk is to mask the following issues: > + * - PCIE SLVERR: config space accesses to invalid PCIe BDFs cause a bus > + * error (signalled as an asynchronous SError) > + * - MCFG BDF mapping: the root complex is mapped separately from the device > + * config space > + * - Non 32-bit accesses to config space are not supported. > + * > + * At boot time the SCP board firmware creates a discovery table with > + * the root complex' base address and the valid BDF values, discovered while > + * scanning the config space and catching the SErrors. > + * Linux responds only to the EPs listed in this table, returning NULL NIT: it will respond to the switch devices which aren't EPs. > + * for the rest. > + */ > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/ioport.h> > +#include <linux/sizes.h> > +#include <linux/of_pci.h> > +#include <linux/of.h> > +#include <linux/pci-ecam.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > + > +/* Platform specific values as hardcoded in the firmware. */ > +#define AP_NS_SHARED_MEM_BASE 0x06000000 > +#define MAX_SEGMENTS 2 /* Two PCIe root complexes. */ > +#define BDF_TABLE_SIZE SZ_16K > + > +/* > + * Shared memory layout as written by the SCP upon boot time: > + * ---- > + * Discover data header --> RC base address > + * \-> BDF Count > + * Discover data --> BDF 0...n > + * ---- > + */ > +struct pcie_discovery_data { > + u32 rc_base_addr; > + u32 nr_bdfs; > + u32 valid_bdfs[0]; > +} *pcie_discovery_data[MAX_SEGMENTS]; > + > +void __iomem *rc_remapped_addr[MAX_SEGMENTS]; > + > +/* > + * map_bus() is called before we do a config space access for a certain > + * device. We use this to check whether this device is valid, avoiding > + * config space accesses which would result in an SError otherwise. > + */ > +static void __iomem *pci_n1sdp_map_bus(struct pci_bus *bus, unsigned int devfn, > + int where) > +{ > + struct pci_config_window *cfg = bus->sysdata; > + unsigned int devfn_shift = cfg->ops->bus_shift - 8; > + unsigned int busn = bus->number; > + unsigned int segment = bus->domain_nr; > + unsigned int bdf_addr; > + unsigned int table_count, i; > + > + if (segment >= MAX_SEGMENTS || > + busn < cfg->busr.start || busn > cfg->busr.end) > + return NULL; > + > + /* The PCIe root complex has a separate config space mapping. */ > + if (busn == 0 && devfn == 0) > + return rc_remapped_addr[segment] + where; > + > + busn -= cfg->busr.start; > + bdf_addr = (busn << cfg->ops->bus_shift) + (devfn << devfn_shift); > + table_count = pcie_discovery_data[segment]->nr_bdfs; > + for (i = 0; i < table_count; i++) { > + if (bdf_addr == pcie_discovery_data[segment]->valid_bdfs[i]) > + return pci_ecam_map_bus(bus, devfn, where); > + } > + > + return NULL; > +} > + > +static int pci_n1sdp_init(struct pci_config_window *cfg, unsigned int segment) > +{ > + phys_addr_t table_base; > + struct device *dev = cfg->parent; > + struct pcie_discovery_data *shared_data; > + size_t bdfs_size; > + > + if (segment >= MAX_SEGMENTS) > + return -ENODEV; > + > + table_base = AP_NS_SHARED_MEM_BASE + segment * BDF_TABLE_SIZE; How can you be sure that this table is populated and isn't junk? I.e. using an older SCP version? > + > + if (!request_mem_region(table_base, BDF_TABLE_SIZE, > + "PCIe valid BDFs")) { > + dev_err(dev, "PCIe BDF shared region request failed\n"); > + return -ENOMEM; > + } > + > + shared_data = devm_ioremap(dev, > + table_base, BDF_TABLE_SIZE); > + if (!shared_data) > + return -ENOMEM; > + > + /* Copy the valid BDFs structure to allocated normal memory. */ > + bdfs_size = sizeof(struct pcie_discovery_data) + > + sizeof(u32) * shared_data->nr_bdfs; > + pcie_discovery_data[segment] = devm_kmalloc(dev, bdfs_size, GFP_KERNEL); > + if (!pcie_discovery_data[segment]) > + return -ENOMEM; > + > + memcpy_fromio(pcie_discovery_data[segment], shared_data, bdfs_size); > + > + rc_remapped_addr[segment] = devm_ioremap_nocache(dev, > + shared_data->rc_base_addr, > + PCI_CFG_SPACE_EXP_SIZE); > + if (!rc_remapped_addr[segment]) { > + dev_err(dev, "Cannot remap root port base\n"); > + return -ENOMEM; > + } > + > + devm_iounmap(dev, shared_data); > + > + return 0; > +} > + > +static int pci_n1sdp_pcie_init(struct pci_config_window *cfg) > +{ > + return pci_n1sdp_init(cfg, 0); > +} > + > +static int pci_n1sdp_ccix_init(struct pci_config_window *cfg) > +{ > + return pci_n1sdp_init(cfg, 1); > +} > + > +struct pci_ecam_ops pci_n1sdp_pcie_ecam_ops = { > + .bus_shift = 20, > + .init = pci_n1sdp_pcie_init, > + .pci_ops = { > + .map_bus = pci_n1sdp_map_bus, > + .read = pci_generic_config_read32, > + .write = pci_generic_config_write32, > + } > +}; > + > +struct pci_ecam_ops pci_n1sdp_ccix_ecam_ops = { > + .bus_shift = 20, > + .init = pci_n1sdp_ccix_init, > + .pci_ops = { > + .map_bus = pci_n1sdp_map_bus, > + .read = pci_generic_config_read32, > + .write = pci_generic_config_write32, > + } > +}; > + > +static const struct of_device_id n1sdp_pcie_of_match[] = { > + { .compatible = "arm,n1sdp-pcie" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, n1sdp_pcie_of_match); > + > +static int n1sdp_pcie_probe(struct platform_device *pdev) > +{ > + const struct device_node *of_node = pdev->dev.of_node; > + u32 segment; > + > + if (of_property_read_u32(of_node, "linux,pci-domain", &segment)) { > + dev_err(&pdev->dev, "N1SDP PCI controllers require linux,pci-domain property\n"); > + return -EINVAL; > + } Can you use of_get_pci_domain_nr here? Thanks, Andrew Murray > + > + switch (segment) { > + case 0: > + return pci_host_common_probe(pdev, &pci_n1sdp_pcie_ecam_ops); > + case 1: > + return pci_host_common_probe(pdev, &pci_n1sdp_ccix_ecam_ops); > + } > + > + dev_err(&pdev->dev, "Invalid segment number, must be smaller than %d\n", > + MAX_SEGMENTS); > + > + return -EINVAL; > +} > + > +static struct platform_driver n1sdp_pcie_driver = { > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = n1sdp_pcie_of_match, > + .suppress_bind_attrs = true, > + }, > + .probe = n1sdp_pcie_probe, > +}; > +builtin_platform_driver(n1sdp_pcie_driver); > diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h > index a73164c85e78..03cdea69f4e8 100644 > --- a/include/linux/pci-ecam.h > +++ b/include/linux/pci-ecam.h > @@ -57,6 +57,8 @@ extern struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX 1.x */ > extern struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 */ > extern struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */ > extern struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */ > +extern struct pci_ecam_ops pci_n1sdp_pcie_ecam_ops; /* Arm N1SDP PCIe */ > +extern struct pci_ecam_ops pci_n1sdp_ccix_ecam_ops; /* Arm N1SDP PCIe */ > #endif > > #ifdef CONFIG_PCI_HOST_COMMON > -- > 2.17.1 >
On 12/9/19 11:26 AM, Will Deacon wrote: > On Mon, Dec 09, 2019 at 04:06:38PM +0000, Andre Przywara wrote: >> From: Deepak Pandey <Deepak.Pandey@arm.com> >> >> The Arm N1SDP SoC suffers from some PCIe integration issues, most >> prominently config space accesses to not existing BDFs being answered >> with a bus abort, resulting in an SError. > > "Do as I say, not as I do"? In my former role I asked nicely that these patches not be posted upstream, but I see that they ended up being posted anyway. Hacking up upstream Linux to cover for the fact that a (reference) platform is non-standard is not only not good form but it actively harms the community. You'll have people consume this platform and not realize that it's broken, IP won't get fixed, and generally it'll be a mess. Yes, it's unfortunate, but so was taping out that platform without working PCI. We all know what should have happened, and what the right move ahead is. Jon. -- Computer Architect
On Tue, 17 Dec 2019 21:21:17 -0500 Jon Masters <jcm@jonmasters.org> wrote: Hi Jon, > On 12/9/19 11:26 AM, Will Deacon wrote: > > On Mon, Dec 09, 2019 at 04:06:38PM +0000, Andre Przywara wrote: > >> From: Deepak Pandey <Deepak.Pandey@arm.com> > >> > >> The Arm N1SDP SoC suffers from some PCIe integration issues, most > >> prominently config space accesses to not existing BDFs being answered > >> with a bus abort, resulting in an SError. > > > > "Do as I say, not as I do"? > > In my former role I asked nicely that these patches not be posted > upstream, but I see that they ended up being posted anyway. Hacking up > upstream Linux to cover for the fact that a (reference) platform is > non-standard is not only not good form but it actively harms the community. Please keep in mind that this platform was designed to be standards compliant, it is just due to an integration problem that this is not the case with this silicon. So we end up with the usual hardware errata, which the kernel can fix up. I agree it's not nice, and I also want it fixed in hardware, but I guess that's the usual software guy's pipe dream. > You'll have people consume this platform and not realize that it's > broken, IP won't get fixed, and generally it'll be a mess. I don't see how yet another ACPI quirk in the ACPI quirk framework(!) will make a mess. Actually the rest of the system is standards compliant (it even uses ACPI from the very beginning ;-), so it's just this problem that prevents us from using the system in the proper, standards compliant way. Effectively we are back to the embedded times with compiling your own kernel and somehow getting a root filesystem on the hard drive. If there would be mainline kernel support, all of this would go away and would could use standard distro installers (given they backport the patch). > Yes, it's > unfortunate, but so was taping out that platform without working PCI. We > all know what should have happened, and what the right move ahead is. That may come as a surprise to some, but Arm Ltd. is actually not really in the business of *producing silicon*, so a respin of the chip was and is not an option. I too wish it would be different, but that's how it is. Cheers, Andre.
On 2019-12-18 10:22, Andre Przywara wrote: > On Tue, 17 Dec 2019 21:21:17 -0500 > Jon Masters <jcm@jonmasters.org> wrote: > > Hi Jon, > >> On 12/9/19 11:26 AM, Will Deacon wrote: >> > On Mon, Dec 09, 2019 at 04:06:38PM +0000, Andre Przywara wrote: >> >> From: Deepak Pandey <Deepak.Pandey@arm.com> >> >> >> >> The Arm N1SDP SoC suffers from some PCIe integration issues, most >> >> prominently config space accesses to not existing BDFs being >> answered >> >> with a bus abort, resulting in an SError. >> > >> > "Do as I say, not as I do"? >> >> In my former role I asked nicely that these patches not be posted >> upstream, but I see that they ended up being posted anyway. Hacking >> up >> upstream Linux to cover for the fact that a (reference) platform is >> non-standard is not only not good form but it actively harms the >> community. > > Please keep in mind that this platform was designed to be standards > compliant, it is just due to an integration problem that this is not > the case with this silicon. So we end up with the usual hardware > errata, which the kernel can fix up. I agree it's not nice, and I > also > want it fixed in hardware, but I guess that's the usual software > guy's > pipe dream. > >> You'll have people consume this platform and not realize that it's >> broken, IP won't get fixed, and generally it'll be a mess. > > I don't see how yet another ACPI quirk in the ACPI quirk framework(!) > will make a mess. > Actually the rest of the system is standards compliant (it even uses > ACPI from the very beginning ;-), so it's just this problem that > prevents us from using the system in the proper, standards compliant > way. Effectively we are back to the embedded times with compiling > your > own kernel and somehow getting a root filesystem on the hard drive. > If there would be mainline kernel support, all of this would go away > and would could use standard distro installers (given they backport > the patch). > >> Yes, it's >> unfortunate, but so was taping out that platform without working >> PCI. We >> all know what should have happened, and what the right move ahead >> is. > > That may come as a surprise to some, but Arm Ltd. is actually not > really in the business of *producing silicon*, so a respin of the > chip > was and is not an option. I too wish it would be different, but > that's > how it is. In all honesty, I really wonder why we are even having this discussion: - The HW is unavailable to the mere mortal. And even most superheroes cannot get their hands on it - Even with this terrible son of a hack, essential PCIe features cannot work (and yes, I do consider SR-IOV as an essential feature) - If we take this hack and somehow get this thing to run with mainline, we will never be able to say "no" to this kind of unfinished, *prototype* implementations ever again To sum it up: a hack that doesn't really work for a prototype that you can't really buy? Why should we even care? M. -- Jazz is not dead. It just smells funny...
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 6a83ba2aea3e..58124ef5070b 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -177,6 +177,7 @@ CONFIG_NET_9P=y CONFIG_NET_9P_VIRTIO=y CONFIG_PCI=y CONFIG_PCIEPORTBUS=y +CONFIG_PCI_QUIRKS=y CONFIG_PCI_IOV=y CONFIG_HOTPLUG_PCI=y CONFIG_HOTPLUG_PCI_ACPI=y diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c index 6b347d9920cc..7a2b41b9ab57 100644 --- a/drivers/acpi/pci_mcfg.c +++ b/drivers/acpi/pci_mcfg.c @@ -142,6 +142,13 @@ static struct mcfg_fixup mcfg_quirks[] = { XGENE_V2_ECAM_MCFG(4, 0), XGENE_V2_ECAM_MCFG(4, 1), XGENE_V2_ECAM_MCFG(4, 2), + +#define N1SDP_ECAM_MCFG(rev, seg, ops) \ + {"ARMLTD", "ARMN1SDP", rev, seg, MCFG_BUS_ANY, ops } + + /* N1SDP SoC with v1 PCIe controller */ + N1SDP_ECAM_MCFG(0x20181101, 0, &pci_n1sdp_pcie_ecam_ops), + N1SDP_ECAM_MCFG(0x20181101, 1, &pci_n1sdp_ccix_ecam_ops), }; static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index c77069c8ee5d..45700d32f02e 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -37,6 +37,17 @@ config PCI_FTPCI100 depends on OF default ARCH_GEMINI +config PCIE_HOST_N1SDP_ECAM + bool "ARM N1SDP PCIe Controller" + depends on ARM64 + depends on OF || (ACPI && PCI_QUIRKS) + select PCI_HOST_COMMON + default y if ARCH_VEXPRESS + help + Say Y here if you want PCIe support for the Arm N1SDP platform. + The controller is ECAM compliant, but needs a quirk to workaround + an integration issue. + config PCI_TEGRA bool "NVIDIA Tegra PCIe controller" depends on ARCH_TEGRA || COMPILE_TEST diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile index 3d4f597f15ce..5f47fefbd67d 100644 --- a/drivers/pci/controller/Makefile +++ b/drivers/pci/controller/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o obj-$(CONFIG_VMD) += vmd.o +obj-$(CONFIG_PCIE_HOST_N1SDP_ECAM) += pcie-n1sdp.o # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW obj-y += dwc/ diff --git a/drivers/pci/controller/pcie-n1sdp.c b/drivers/pci/controller/pcie-n1sdp.c new file mode 100644 index 000000000000..620ab221466c --- /dev/null +++ b/drivers/pci/controller/pcie-n1sdp.c @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018/2019 ARM Ltd. + * + * This quirk is to mask the following issues: + * - PCIE SLVERR: config space accesses to invalid PCIe BDFs cause a bus + * error (signalled as an asynchronous SError) + * - MCFG BDF mapping: the root complex is mapped separately from the device + * config space + * - Non 32-bit accesses to config space are not supported. + * + * At boot time the SCP board firmware creates a discovery table with + * the root complex' base address and the valid BDF values, discovered while + * scanning the config space and catching the SErrors. + * Linux responds only to the EPs listed in this table, returning NULL + * for the rest. + */ + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/ioport.h> +#include <linux/sizes.h> +#include <linux/of_pci.h> +#include <linux/of.h> +#include <linux/pci-ecam.h> +#include <linux/platform_device.h> +#include <linux/module.h> + +/* Platform specific values as hardcoded in the firmware. */ +#define AP_NS_SHARED_MEM_BASE 0x06000000 +#define MAX_SEGMENTS 2 /* Two PCIe root complexes. */ +#define BDF_TABLE_SIZE SZ_16K + +/* + * Shared memory layout as written by the SCP upon boot time: + * ---- + * Discover data header --> RC base address + * \-> BDF Count + * Discover data --> BDF 0...n + * ---- + */ +struct pcie_discovery_data { + u32 rc_base_addr; + u32 nr_bdfs; + u32 valid_bdfs[0]; +} *pcie_discovery_data[MAX_SEGMENTS]; + +void __iomem *rc_remapped_addr[MAX_SEGMENTS]; + +/* + * map_bus() is called before we do a config space access for a certain + * device. We use this to check whether this device is valid, avoiding + * config space accesses which would result in an SError otherwise. + */ +static void __iomem *pci_n1sdp_map_bus(struct pci_bus *bus, unsigned int devfn, + int where) +{ + struct pci_config_window *cfg = bus->sysdata; + unsigned int devfn_shift = cfg->ops->bus_shift - 8; + unsigned int busn = bus->number; + unsigned int segment = bus->domain_nr; + unsigned int bdf_addr; + unsigned int table_count, i; + + if (segment >= MAX_SEGMENTS || + busn < cfg->busr.start || busn > cfg->busr.end) + return NULL; + + /* The PCIe root complex has a separate config space mapping. */ + if (busn == 0 && devfn == 0) + return rc_remapped_addr[segment] + where; + + busn -= cfg->busr.start; + bdf_addr = (busn << cfg->ops->bus_shift) + (devfn << devfn_shift); + table_count = pcie_discovery_data[segment]->nr_bdfs; + for (i = 0; i < table_count; i++) { + if (bdf_addr == pcie_discovery_data[segment]->valid_bdfs[i]) + return pci_ecam_map_bus(bus, devfn, where); + } + + return NULL; +} + +static int pci_n1sdp_init(struct pci_config_window *cfg, unsigned int segment) +{ + phys_addr_t table_base; + struct device *dev = cfg->parent; + struct pcie_discovery_data *shared_data; + size_t bdfs_size; + + if (segment >= MAX_SEGMENTS) + return -ENODEV; + + table_base = AP_NS_SHARED_MEM_BASE + segment * BDF_TABLE_SIZE; + + if (!request_mem_region(table_base, BDF_TABLE_SIZE, + "PCIe valid BDFs")) { + dev_err(dev, "PCIe BDF shared region request failed\n"); + return -ENOMEM; + } + + shared_data = devm_ioremap(dev, + table_base, BDF_TABLE_SIZE); + if (!shared_data) + return -ENOMEM; + + /* Copy the valid BDFs structure to allocated normal memory. */ + bdfs_size = sizeof(struct pcie_discovery_data) + + sizeof(u32) * shared_data->nr_bdfs; + pcie_discovery_data[segment] = devm_kmalloc(dev, bdfs_size, GFP_KERNEL); + if (!pcie_discovery_data[segment]) + return -ENOMEM; + + memcpy_fromio(pcie_discovery_data[segment], shared_data, bdfs_size); + + rc_remapped_addr[segment] = devm_ioremap_nocache(dev, + shared_data->rc_base_addr, + PCI_CFG_SPACE_EXP_SIZE); + if (!rc_remapped_addr[segment]) { + dev_err(dev, "Cannot remap root port base\n"); + return -ENOMEM; + } + + devm_iounmap(dev, shared_data); + + return 0; +} + +static int pci_n1sdp_pcie_init(struct pci_config_window *cfg) +{ + return pci_n1sdp_init(cfg, 0); +} + +static int pci_n1sdp_ccix_init(struct pci_config_window *cfg) +{ + return pci_n1sdp_init(cfg, 1); +} + +struct pci_ecam_ops pci_n1sdp_pcie_ecam_ops = { + .bus_shift = 20, + .init = pci_n1sdp_pcie_init, + .pci_ops = { + .map_bus = pci_n1sdp_map_bus, + .read = pci_generic_config_read32, + .write = pci_generic_config_write32, + } +}; + +struct pci_ecam_ops pci_n1sdp_ccix_ecam_ops = { + .bus_shift = 20, + .init = pci_n1sdp_ccix_init, + .pci_ops = { + .map_bus = pci_n1sdp_map_bus, + .read = pci_generic_config_read32, + .write = pci_generic_config_write32, + } +}; + +static const struct of_device_id n1sdp_pcie_of_match[] = { + { .compatible = "arm,n1sdp-pcie" }, + { }, +}; +MODULE_DEVICE_TABLE(of, n1sdp_pcie_of_match); + +static int n1sdp_pcie_probe(struct platform_device *pdev) +{ + const struct device_node *of_node = pdev->dev.of_node; + u32 segment; + + if (of_property_read_u32(of_node, "linux,pci-domain", &segment)) { + dev_err(&pdev->dev, "N1SDP PCI controllers require linux,pci-domain property\n"); + return -EINVAL; + } + + switch (segment) { + case 0: + return pci_host_common_probe(pdev, &pci_n1sdp_pcie_ecam_ops); + case 1: + return pci_host_common_probe(pdev, &pci_n1sdp_ccix_ecam_ops); + } + + dev_err(&pdev->dev, "Invalid segment number, must be smaller than %d\n", + MAX_SEGMENTS); + + return -EINVAL; +} + +static struct platform_driver n1sdp_pcie_driver = { + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = n1sdp_pcie_of_match, + .suppress_bind_attrs = true, + }, + .probe = n1sdp_pcie_probe, +}; +builtin_platform_driver(n1sdp_pcie_driver); diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h index a73164c85e78..03cdea69f4e8 100644 --- a/include/linux/pci-ecam.h +++ b/include/linux/pci-ecam.h @@ -57,6 +57,8 @@ extern struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX 1.x */ extern struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 */ extern struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */ extern struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */ +extern struct pci_ecam_ops pci_n1sdp_pcie_ecam_ops; /* Arm N1SDP PCIe */ +extern struct pci_ecam_ops pci_n1sdp_ccix_ecam_ops; /* Arm N1SDP PCIe */ #endif #ifdef CONFIG_PCI_HOST_COMMON