| Message ID | 1442529694-1792-3-git-send-email-ddaney.cavm@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 17 September 2015 15:41:33 David Daney wrote: > From: David Daney <david.daney@cavium.com> > > The on-chip devices all have fixed bars. So, fix them up. > > Signed-off-by: David Daney <david.daney@cavium.com> > You should be able to just mark the BARs as fixed in DT and not need this hack. If that doesn't work with current kernels, it would be better to fix the kernel to respect the flags. Arnd
On 09/18/2015 12:19 AM, Arnd Bergmann wrote: > On Thursday 17 September 2015 15:41:33 David Daney wrote: >> From: David Daney <david.daney@cavium.com> >> >> The on-chip devices all have fixed bars. So, fix them up. >> >> Signed-off-by: David Daney <david.daney@cavium.com> >> > > You should be able to just mark the BARs as fixed in DT In the case of ACPI, there is no DT. So we would need a different solution for ACPI. What would you recommend for ACPI? Also, can you point me to the OF device tree specification where it tells how to specify PCI BAR addresses, I would especially be interested in knowing how to specify fixed SRIOV BAR addresses in the device tree. Thanks, David Daney > and not need > this hack. Yes, it is a bit of a hack. That is why I put it in its own file, and only try to hack up PCI devices that exactly match the vendor and device ids that need fixing. IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle this would be much more intrusive. For the record: The PCI Enhanced Allocation (EA) capability (approved by PCI SIG on 23 October 2014) is the proper way to handle this going forward. However, this is not yet implemented in the SoCs that this patch addresses. Our plan is to implement the EA capability in the core PCI code, so that we do not need to keep adding devices to this fixup code. David Daney
On Friday 18 September 2015 10:00:32 David Daney wrote: > On 09/18/2015 12:19 AM, Arnd Bergmann wrote: > > On Thursday 17 September 2015 15:41:33 David Daney wrote: > >> From: David Daney <david.daney@cavium.com> > >> > >> The on-chip devices all have fixed bars. So, fix them up. > >> > >> Signed-off-by: David Daney <david.daney@cavium.com> > >> > > > > You should be able to just mark the BARs as fixed in DT > > In the case of ACPI, there is no DT. So we would need a different > solution for ACPI. What would you recommend for ACPI? I would expect that this does not matter at all on ACPI, because the devices that need it are not hot-plugged, and all boot-time devices are probed by the firmware: the ACPI PCI implementation does not reassign any BARs, except for the hotplug case. > Also, can you point me to the OF device tree specification where it > tells how to specify PCI BAR addresses, I would especially be interested > in knowing how to specify fixed SRIOV BAR addresses in the device tree. This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the PCI binding. When it is set, the OS is not supposed to try to reassign the BAR even on machines that otherwise do a complete rescan. The PCI binding traditionally requires you to list all PCI devices in DT, Linux as an extension (for the flattened DT format) allows leaving out the devices, but in this case you probably need to list every device that has a fixed BAR. > Yes, it is a bit of a hack. That is why I put it in its own file, and > only try to hack up PCI devices that exactly match the vendor and device > ids that need fixing. > > IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle > this would be much more intrusive. My guess is that it's already there, but even if it's not, this is a generic well-defined case that has a standardized binding, and we should implement that. > For the record: The PCI Enhanced Allocation (EA) capability (approved > by PCI SIG on 23 October 2014) is the proper way to handle this going > forward. However, this is not yet implemented in the SoCs that this > patch addresses. Our plan is to implement the EA capability in the core > PCI code, so that we do not need to keep adding devices to this fixup code. Good, but still this should only be required for the embedded case where you don't have a firmware to probe the bus. Arnd
On 09/18/2015 12:45 PM, Arnd Bergmann wrote: > On Friday 18 September 2015 10:00:32 David Daney wrote: >> On 09/18/2015 12:19 AM, Arnd Bergmann wrote: >>> On Thursday 17 September 2015 15:41:33 David Daney wrote: >>>> From: David Daney <david.daney@cavium.com> >>>> >>>> The on-chip devices all have fixed bars. So, fix them up. >>>> >>>> Signed-off-by: David Daney <david.daney@cavium.com> >>>> >>> >>> You should be able to just mark the BARs as fixed in DT I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR devices configured by firmware. This may significantly simplify any quirks required in the kernel. >> >> In the case of ACPI, there is no DT. So we would need a different >> solution for ACPI. What would you recommend for ACPI? > > I would expect that this does not matter at all on ACPI, because > the devices that need it are not hot-plugged, and all boot-time > devices are probed by the firmware: the ACPI PCI implementation > does not reassign any BARs, except for the hotplug case. > >> Also, can you point me to the OF device tree specification where it >> tells how to specify PCI BAR addresses, I would especially be interested >> in knowing how to specify fixed SRIOV BAR addresses in the device tree. > > This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the > PCI binding. When it is set, the OS is not supposed to try to > reassign the BAR even on machines that otherwise do a complete > rescan. > > The PCI binding traditionally requires you to list all PCI devices > in DT, Linux as an extension (for the flattened DT format) allows > leaving out the devices, but in this case you probably need to > list every device that has a fixed BAR. > >> Yes, it is a bit of a hack. That is why I put it in its own file, and >> only try to hack up PCI devices that exactly match the vendor and device >> ids that need fixing. >> >> IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle >> this would be much more intrusive. > > My guess is that it's already there, but even if it's not, this is a > generic well-defined case that has a standardized binding, and we should > implement that. > >> For the record: The PCI Enhanced Allocation (EA) capability (approved >> by PCI SIG on 23 October 2014) is the proper way to handle this going >> forward. However, this is not yet implemented in the SoCs that this >> patch addresses. Our plan is to implement the EA capability in the core >> PCI code, so that we do not need to keep adding devices to this fixup code. > > Good, but still this should only be required for the embedded case where > you don't have a firmware to probe the bus. > > Arnd >
Hi David, On Fri, Sep 18, 2015 at 06:00:28PM -0700, David Daney wrote: > On 09/18/2015 12:45 PM, Arnd Bergmann wrote: > >On Friday 18 September 2015 10:00:32 David Daney wrote: > >>On 09/18/2015 12:19 AM, Arnd Bergmann wrote: > >>>On Thursday 17 September 2015 15:41:33 David Daney wrote: > >>>>From: David Daney <david.daney@cavium.com> > >>>> > >>>>The on-chip devices all have fixed bars. So, fix them up. > >>>> > >>>>Signed-off-by: David Daney <david.daney@cavium.com> > >>>> > >>> > >>>You should be able to just mark the BARs as fixed in DT > > I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR > devices configured by firmware. This may significantly simplify any > quirks required in the kernel. I don't like PCI_PROBE_ONLY, and I'd like to avoid it when we can. Your original patch description said the on-chip devices have "fixed BARs." In what sense are they "fixed"? I assume they are writable enough so we can learn their sizes? If we can't learn their sizes, we have bigger problems because we can't tell what space is used. Are there other parts of the system, e.g., run-time firmware, that depend on the devices not being moved? > >>For the record: The PCI Enhanced Allocation (EA) capability (approved > >>by PCI SIG on 23 October 2014) is the proper way to handle this going > >>forward. However, this is not yet implemented in the SoCs that this > >>patch addresses. Our plan is to implement the EA capability in the core > >>PCI code, so that we do not need to keep adding devices to this fixup code. Sean Stalley has posted some patches to add EA support to Linux, but I haven't merged them yet. If we had that, another option would be to hook into your config accessors and fabricate an EA capability. Bjorn
On Fri, Sep 18, 2015 at 08:45:50PM +0100, Arnd Bergmann wrote: > On Friday 18 September 2015 10:00:32 David Daney wrote: > > On 09/18/2015 12:19 AM, Arnd Bergmann wrote: > > > On Thursday 17 September 2015 15:41:33 David Daney wrote: > > >> From: David Daney <david.daney@cavium.com> > > >> > > >> The on-chip devices all have fixed bars. So, fix them up. > > >> > > >> Signed-off-by: David Daney <david.daney@cavium.com> > > >> > > > > > > You should be able to just mark the BARs as fixed in DT > > > > In the case of ACPI, there is no DT. So we would need a different > > solution for ACPI. What would you recommend for ACPI? > > I would expect that this does not matter at all on ACPI, because > the devices that need it are not hot-plugged, and all boot-time > devices are probed by the firmware: the ACPI PCI implementation > does not reassign any BARs, except for the hotplug case. What do you mean by "the ACPI PCI implementation does not reassign any BARs" ? Do you mean on x86 ? The resource assignment is part of the resource survey on x86, where all resources that can be claimed are claimed, but still, some of them may be still reassigned IIUC. On arm64 we do not carry out any resource survey at present, but if we do (and we should), it will have to work for both DT and ACPI systems. > > Also, can you point me to the OF device tree specification where it > > tells how to specify PCI BAR addresses, I would especially be interested > > in knowing how to specify fixed SRIOV BAR addresses in the device tree. > > This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the > PCI binding. When it is set, the OS is not supposed to try to > reassign the BAR even on machines that otherwise do a complete > rescan. I do not see any code in the kernel taking care of that bit and if I am not mistaken the code that allows creating pci devices exists in PowerPC arch code (arch/powerpc/kernel/pci_of_scan.c) and should be moved out of there for the other arches to use it. Or maybe we can use DT just to fix-up the resource flags (ie every PCI device will have its own of_node set if it is present in the DT, we can use it to fixup its resources and related flags, pcibios_add_device() ?). > The PCI binding traditionally requires you to list all PCI devices > in DT, Linux as an extension (for the flattened DT format) allows > leaving out the devices, but in this case you probably need to > list every device that has a fixed BAR. > > > Yes, it is a bit of a hack. That is why I put it in its own file, and > > only try to hack up PCI devices that exactly match the vendor and device > > ids that need fixing. > > > > IMHO, putting infrastructure into drivers/pci/probe.c, et al. to handle > > this would be much more intrusive. > > My guess is that it's already there, but even if it's not, this is a > generic well-defined case that has a standardized binding, and we should > implement that. See above. Thanks, Lorenzo > > Forthe record: The PCI Enhanced Allocation (EA) capability (approved > > by PCI SIG on 23 October 2014) is the proper way to handle this going > > forward. However, this is not yet implemented in the SoCs that this > > patch addresses. Our plan is to implement the EA capability in the core > > PCI code, so that we do not need to keep adding devices to this fixup code. > > Good, but still this should only be required for the embedded case where > you don't have a firmware to probe the bus. > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Tuesday 22 September 2015 16:39:31 Lorenzo Pieralisi wrote: > On Fri, Sep 18, 2015 at 08:45:50PM +0100, Arnd Bergmann wrote: > > On Friday 18 September 2015 10:00:32 David Daney wrote: > > > On 09/18/2015 12:19 AM, Arnd Bergmann wrote: > > > > On Thursday 17 September 2015 15:41:33 David Daney wrote: > > > >> From: David Daney <david.daney@cavium.com> > > > >> > > > >> The on-chip devices all have fixed bars. So, fix them up. > > > >> > > > >> Signed-off-by: David Daney <david.daney@cavium.com> > > > >> > > > > > > > > You should be able to just mark the BARs as fixed in DT > > > > > > In the case of ACPI, there is no DT. So we would need a different > > > solution for ACPI. What would you recommend for ACPI? > > > > I would expect that this does not matter at all on ACPI, because > > the devices that need it are not hot-plugged, and all boot-time > > devices are probed by the firmware: the ACPI PCI implementation > > does not reassign any BARs, except for the hotplug case. > > What do you mean by "the ACPI PCI implementation does not reassign > any BARs" ? Do you mean on x86 ? The resource assignment is part > of the resource survey on x86, where all resources that can be claimed > are claimed, but still, some of them may be still reassigned IIUC. The ACPI PCI implementation should be completely architecture independent and do the same thing everywhere. The code is spread over drivers/acpi/pci*, with small parts currently residing in arch/{x86,ia64} that need to be moved to drivers/acpi/ > On arm64 we do not carry out any resource survey at present, but > if we do (and we should), it will have to work for both DT and ACPI > systems. This is not a difference between DT and ACPI, but a difference between embedded and server. Server platforms that have a proper firmware to scan the PCI bus should not set PCI_REASSIGN_ALL_RSRC and PCI_REASSIGN_ALL_BUS, while embedded systems that don't set up the PCI bus before boot have to set those. On ACPI, reassigning the resources would actually be dangerous because the firmware may rely on devices to be at a certain location (both bus number and mmio address), or may play tricks here to hide stuff from the OS. > > > Also, can you point me to the OF device tree specification where it > > > tells how to specify PCI BAR addresses, I would especially be interested > > > in knowing how to specify fixed SRIOV BAR addresses in the device tree. > > > > This is the 'n' bit mentioned sections 2.1.3 and 2.2.1.1 of the > > PCI binding. When it is set, the OS is not supposed to try to > > reassign the BAR even on machines that otherwise do a complete > > rescan. > > I do not see any code in the kernel taking care of that bit and > if I am not mistaken the code that allows creating pci devices > exists in PowerPC arch code (arch/powerpc/kernel/pci_of_scan.c) and > should be moved out of there for the other arches to use it. > > Or maybe we can use DT just to fix-up the resource flags (ie > every PCI device will have its own of_node set if it is present > in the DT, we can use it to fixup its resources and related flags, > pcibios_add_device() ?). I haven't looked at the code in detail now, but I'm pretty sure that a device node that refers to a PCI device is connected to the dev->of_node pointer already on all architectures. It's quite possible that we never implemented the fixed BAR logic though, but it can be done based on those pointers. Arnd
On 09/22/2015 06:19 AM, Bjorn Helgaas wrote: > Hi David, > > On Fri, Sep 18, 2015 at 06:00:28PM -0700, David Daney wrote: >> On 09/18/2015 12:45 PM, Arnd Bergmann wrote: >>> On Friday 18 September 2015 10:00:32 David Daney wrote: >>>> On 09/18/2015 12:19 AM, Arnd Bergmann wrote: >>>>> On Thursday 17 September 2015 15:41:33 David Daney wrote: >>>>>> From: David Daney <david.daney@cavium.com> >>>>>> >>>>>> The on-chip devices all have fixed bars. So, fix them up. >>>>>> >>>>>> Signed-off-by: David Daney <david.daney@cavium.com> >>>>>> >>>>> >>>>> You should be able to just mark the BARs as fixed in DT >> >> I think we can switch to PCI_PROBE_ONLY, and have all non-fixed BAR >> devices configured by firmware. This may significantly simplify any >> quirks required in the kernel. > > I don't like PCI_PROBE_ONLY, and I'd like to avoid it when we can. I don't like it either, but if it were the only way the PCI maintainers would allow us to support the hardware, I would rewrite the firmware to make it possible. However, as you say below ... > > Your original patch description said the on-chip devices have "fixed > BARs." In what sense are they "fixed"? I assume they are writable > enough so we can learn their sizes? Yes. The BAR registers are writable, but ignored. So, the size is correctly probed. The BAR registers are initialized by hardware/firmware to the proper value. > If we can't learn their sizes, we > have bigger problems because we can't tell what space is used. > > Are there other parts of the system, e.g., run-time firmware, that > depend on the devices not being moved? The devices cannot move, the address decoding is not programmable. The BAR registers are provided as an aid in integrating the devices with OS PCI infrastructure. Although, one might argue that they don't do a very good job of adhering to the PCI specifications... > >>>> For the record: The PCI Enhanced Allocation (EA) capability (approved >>>> by PCI SIG on 23 October 2014) is the proper way to handle this going >>>> forward. However, this is not yet implemented in the SoCs that this >>>> patch addresses. Our plan is to implement the EA capability in the core >>>> PCI code, so that we do not need to keep adding devices to this fixup code. > > Sean Stalley has posted some patches to add EA support to Linux, but I > haven't merged them yet. If we had that, another option would be to > hook into your config accessors and fabricate an EA capability. To me, this is the most interesting part of your message. If you really would accept a config read accessor that presented a synthetic EA capability, that would be ideal. We know exactly which roots contain the fixed devices, so it would be a trivial exercise to provide a custom config accessor. I am going to work on this in hope of eventual acceptance of this strategy. Thanks, David Daney
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index d5e58ba..20d700d 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -145,4 +145,10 @@ config PCIE_IPROC_BCMA Say Y here if you want to use the Broadcom iProc PCIe controller through the BCMA bus interface +config PCI_QUIRKS_THUNDER + bool "PCI quirks for Cavium ThunderX SoCs" + depends on ARM64 + help + Say Y here to support ThunderX SoCs + endmenu diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 140d66f..3ce7456 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o +obj-$(CONFIG_PCI_QUIRKS_THUNDER) += quirks-thunder.o diff --git a/drivers/pci/host/quirks-thunder.c b/drivers/pci/host/quirks-thunder.c new file mode 100644 index 0000000..d615fd8 --- /dev/null +++ b/drivers/pci/host/quirks-thunder.c @@ -0,0 +1,95 @@ +/* + * PCIe quirks for Cavium Thunder SOC + * + * Copyright (C) 2014, 2015 Cavium Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#include <linux/module.h> +#include <linux/pci.h> + +#define PCI_DEVICE_ID_THUNDER_BRIDGE 0xa002 + +/* + * All PCIe devices in Thunder have fixed resources, shouldn't be + * reassigned. + */ +static void thunder_pci_fixup_header(struct pci_dev *dev) +{ + int resno; + + /* Only fixup Thunder on-chip devices. */ + if ((dev->device & 0xff00) != 0xa000) + return; + + for (resno = 0; resno < PCI_NUM_RESOURCES; resno++) + if (dev->resource[resno].flags) + dev->resource[resno].flags |= IORESOURCE_PCI_FIXED; +#ifdef CONFIG_PCI_IOV + if (dev->device == 0xa01e) { + dev->resource[PCI_IOV_RESOURCES + 0] = dev->resource[0]; + dev->resource[PCI_IOV_RESOURCES + 0].start += 0xa0000000; + dev->resource[PCI_IOV_RESOURCES + 0].end = + dev->resource[PCI_IOV_RESOURCES + 0].start + 0x200000 - 1; + dev->resource[PCI_IOV_RESOURCES + 4] = dev->resource[0]; + dev->resource[PCI_IOV_RESOURCES + 4].start += 0xe0000000; + dev->resource[PCI_IOV_RESOURCES + 4].end = + dev->resource[PCI_IOV_RESOURCES + 4].start + 0x200000 - 1; + } +#endif +} +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, thunder_pci_fixup_header); + +static void thunder_pci_fixup_final(struct pci_dev *dev) +{ + struct resource *res; + int resno; + + /* Only fixup Thunder on-chip devices. */ + if ((dev->device & 0xff00) != 0xa000) + return; + + if (dev->device == PCI_DEVICE_ID_THUNDER_BRIDGE) { + /* + * This bridge is just for the sake of supporting ARI + * for downstream devices. No resources are attached + * to it. Copy upstream root bus resources to bridge + * which aide in resource claiming for downstream + * devices + */ + + struct pci_bus *bus; + int resno; + + bus = dev->subordinate; + for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) { + bus->resource[resno] = pci_bus_resource_n(bus->parent, + PCI_BRIDGE_RESOURCE_NUM + resno); + } + + for (resno = PCI_BRIDGE_RESOURCES; + resno <= PCI_BRIDGE_RESOURCE_END; resno++) { + dev->resource[resno].start = dev->resource[resno].end = 0; + dev->resource[resno].flags = 0; + } + } else { + /* + * Claim the device's valid resources to set + * 'res->parent' hierarchy. + */ + for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) { + res = &dev->resource[resno]; + if (res->parent || !(res->flags & IORESOURCE_MEM)) + continue; + pci_claim_resource(dev, resno); + } + } +} +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, thunder_pci_fixup_final); + +MODULE_DESCRIPTION("PCI quirks for Cavium Thunder ECAM based devices"); +MODULE_LICENSE("GPL v2");