Skip to contentSkip to conversation

drivers: pci: add generic code to claim bus resources

PATCH Superseded Lorenzo Pieralisi 3 participants 8 messages Nov 17, 2015 – Feb 25, 2016

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 drivers: pci: add generic code to claim bus resources

PCI core code contains a set of functions, eg:

pci_assign_unassigned_bus_resources()

that allow to assign the PCI resources for a given bus after enumeration.

On systems where the PCI BARs are immutable (ie they must not and can not be assigned), PCI resources must be claimed in order to be validated and inserted in the PCI resources tree, but there is no generic PCI kernel function for that purpose and the resource claiming is implemented in an arch specific fashion which resulted in arches implementations that contain duplicated code.

This patch, based on the ia64 resource claiming arch implementation, implements a set of functions in core PCI code that provides a PCI core interface for resources claiming for a given PCI bus hierarchy, paving the way for further resource claiming consolidation across architectures.

Cc Arnd Bergmann <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Yinghai Lu <[email protected]>
---
 drivers/pci/setup-bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h     |  1 +
 2 files changed, 43 insertions(+)
drivers/pci/setup-bus.c +42 -0
index 1723ac1..9054541 100644
@@ -1424,6 +1424,48 @@ void pci_bus_assign_resources(const struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_bus_assign_resources);
 
+static void pci_claim_device_resources(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+		struct resource *r = &dev->resource[i];
+
+		if (!r->flags || r->parent)
+			continue;
+
+		pci_claim_resource(dev, i);
+	}
+}
+
+static void pci_claim_bridge_resources(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
+		struct resource *r = &dev->resource[i];
+
+		if (!r->flags || r->parent)
+			continue;
+
+		pci_claim_bridge_resource(dev, i);
+	}
+}
+
+void pci_bus_claim_resources(struct pci_bus *b)
+{
+	struct pci_dev *dev;
+
+	if (b->self) {
+		pci_read_bridge_bases(b);
+		pci_claim_bridge_resources(b->self);
+	}
+
+	list_for_each_entry(dev, &b->devices, bus_list)
+		pci_claim_device_resources(dev);
+}
+EXPORT_SYMBOL(pci_bus_claim_resources);
+
 static void __pci_bridge_assign_resources(const struct pci_dev *bridge,
 					  struct list_head *add_head,
 					  struct list_head *fail_head)
include/linux/pci.h +1 -0
index e828e7b..28ce9c5 100644
@@ -1110,6 +1110,7 @@ ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
 void pci_bus_assign_resources(const struct pci_bus *bus);
+void pci_bus_claim_resources(struct pci_bus *bus);
 void pci_bus_size_bridges(struct pci_bus *bus);
 int pci_claim_resource(struct pci_dev *, int);
 int pci_claim_bridge_resource(struct pci_dev *bridge, int i);
Signature · 1 line
2.5.1
#1 lore raw

PATCH 2/3 drivers: pci: host-generic: claim bus resources on PROBE_ONLY set-ups

The PCI host generic driver does not reassign bus resources on systems that require the BARs set-up to be immutable (ie PROBE_ONLY) since that would trigger system failures. Nonetheless, PCI bus resources allocated to PCI bridge and devices must be claimed in order to be validated and inserted in the kernel resource tree, but the current driver omits the resources claiming and relies on arch specific kludges to prevent probing failure (ie preventing resources enablement on PROBE_ONLY systems).

This patch adds code to the PCI host generic driver that correctly claims bus resources upon probe on systems that are required to prevent reassignment after bus enumeration, so that the allocated resources can be enabled successfully upon PCI device drivers probing, without resorting to arch back-ends workarounds.

Cc Arnd Bergmann <[email protected]>
Cc David Daney <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
---
 drivers/pci/host/pci-host-generic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/pci/host/pci-host-generic.c +4 -1
index 5434c90..c62d13f 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
 
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+
+	if (pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_claim_resources(bus);
+	} else {
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
 
Signature · 1 line
2.5.1
#2 lore raw

PATCH 2/3 drivers: pci: host-generic: claim bus resources on PROBE_ONLY set-ups

Quoted 15 lines · Lorenzo Pieralisi

The PCI host generic driver does not reassign bus resources on systems that require the BARs set-up to be immutable (ie PROBE_ONLY) since that would trigger system failures. Nonetheless, PCI bus resources allocated to PCI bridge and devices must be claimed in order to be validated and inserted in the kernel resource tree, but the current driver omits the resources claiming and relies on arch specific kludges to prevent probing failure (ie preventing resources enablement on PROBE_ONLY systems).

This patch adds code to the PCI host generic driver that correctly claims bus resources upon probe on systems that are required to prevent reassignment after bus enumeration, so that the allocated resources can be enabled successfully upon PCI device drivers probing, without resorting to arch back-ends workarounds.

Quoted 8 lines
Cc Arnd Bergmann <[email protected]>
Cc David Daney <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
---
 drivers/pci/host/pci-host-generic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Assuming patch one is accepted, then this looks fine to me:

Will

Quoted 19 lines
drivers/pci/host/pci-host-generic.c +4 -1
index 5434c90..c62d13f 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
 
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+
+	if (pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_claim_resources(bus);
+	} else {
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
 
Signature · 1 line
2.5.1
#3 lore raw

PATCH 3/3 arm/arm64: pci: remove arch specific pcibios_enable_device()

In reply to Lorenzo Pieralisi

The arm/arm64 pcibios_enable_device() implementations exist solely to prevent enabling PCI resources on PROBE_ONLY systems, since on those systems the PCI resources are currently not claimed (ie validated and inserted in the PCI resource tree) therefore they can not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups that have PROBE_ONLY as a probe option, there is no need for arch specific pcibios_enable_device() implementations anymore in that the kernel can rely on the generic pcibios_enable_device() implementation without resorting to arch specific code to work around the missing resources claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from the arm/arm64 arch back-ends.

Cc Arnd Bergmann <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)
arch/arm/kernel/bios32.c +0 -12
index 6551d28..9c998d5 100644
@@ -590,18 +590,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 			enum pci_mmap_state mmap_state, int write_combine)
 {
arch/arm64/kernel/pci.c +0 -13
index b3d098b..4095379 100644
@@ -38,19 +38,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return res->start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- * @mask: bitmask of BARs to enable
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 /*
  * Try to assign the IRQ number from DT when adding a new device
  */
Signature · 1 line
2.5.1
#4 lore raw

PATCH 3/3 arm/arm64: pci: remove arch specific pcibios_enable_device()

Quoted 22 lines · Lorenzo Pieralisi

The arm/arm64 pcibios_enable_device() implementations exist solely to prevent enabling PCI resources on PROBE_ONLY systems, since on those systems the PCI resources are currently not claimed (ie validated and inserted in the PCI resource tree) therefore they can not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups that have PROBE_ONLY as a probe option, there is no need for arch specific pcibios_enable_device() implementations anymore in that the kernel can rely on the generic pcibios_enable_device() implementation without resorting to arch specific code to work around the missing resources claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from the arm/arm64 arch back-ends.

Cc Arnd Bergmann <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Quoted 4 lines
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)

Will

#5 lore raw

PATCH 3/3 arm/arm64: pci: remove arch specific pcibios_enable_device()

Quoted 28 lines · Will Deacon
Quoted 22 lines · Lorenzo Pieralisi

The arm/arm64 pcibios_enable_device() implementations exist solely to prevent enabling PCI resources on PROBE_ONLY systems, since on those systems the PCI resources are currently not claimed (ie validated and inserted in the PCI resource tree) therefore they can not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups that have PROBE_ONLY as a probe option, there is no need for arch specific pcibios_enable_device() implementations anymore in that the kernel can rely on the generic pcibios_enable_device() implementation without resorting to arch specific code to work around the missing resources claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from the arm/arm64 arch back-ends.

Cc Arnd Bergmann <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Quoted 4 lines
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)
Quoted 1 line

Thanks, unfortunately I spotted that ARM platforms can set PCI_PROBE_ONLY also via command line (pcibios_setup()), which means that I have to add resource claiming to all ARM PCI controllers that check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we can apply this patch or alternatevely we add the resource claiming to the pcibios_fixup_bus() callback (but we can claim resources only if PCI_PROBE_ONLY is set lest we trigger regressions) which would be the simpler solution.

Lorenzo

#6 lore raw

PATCH 3/3 arm/arm64: pci: remove arch specific pcibios_enable_device()

Hi Lorenzo,

Quoted 31 lines · Lorenzo Pieralisi
Quoted 28 lines · Will Deacon
Quoted 22 lines · Lorenzo Pieralisi

The arm/arm64 pcibios_enable_device() implementations exist solely to prevent enabling PCI resources on PROBE_ONLY systems, since on those systems the PCI resources are currently not claimed (ie validated and inserted in the PCI resource tree) therefore they can not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups that have PROBE_ONLY as a probe option, there is no need for arch specific pcibios_enable_device() implementations anymore in that the kernel can rely on the generic pcibios_enable_device() implementation without resorting to arch specific code to work around the missing resources claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from the arm/arm64 arch back-ends.

Cc Arnd Bergmann <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Quoted 4 lines
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)
Quoted 1 line
Quoted 8 lines

Thanks, unfortunately I spotted that ARM platforms can set PCI_PROBE_ONLY also via command line (pcibios_setup()), which means that I have to add resource claiming to all ARM PCI controllers that check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we can apply this patch or alternatevely we add the resource claiming to the pcibios_fixup_bus() callback (but we can claim resources only if PCI_PROBE_ONLY is set lest we trigger regressions) which would be the simpler solution.

So where are we? The above response sounds like this series still needs a little tweaking?

Bjorn

#7 lore raw

PATCH 3/3 arm/arm64: pci: remove arch specific pcibios_enable_device()

Hi Bjorn,

Quoted 43 lines · Bjorn Helgaas

Hi Lorenzo,

Quoted 31 lines · Lorenzo Pieralisi
Quoted 28 lines · Will Deacon
Quoted 22 lines · Lorenzo Pieralisi

The arm/arm64 pcibios_enable_device() implementations exist solely to prevent enabling PCI resources on PROBE_ONLY systems, since on those systems the PCI resources are currently not claimed (ie validated and inserted in the PCI resource tree) therefore they can not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups that have PROBE_ONLY as a probe option, there is no need for arch specific pcibios_enable_device() implementations anymore in that the kernel can rely on the generic pcibios_enable_device() implementation without resorting to arch specific code to work around the missing resources claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from the arm/arm64 arch back-ends.

Cc Arnd Bergmann <[email protected]>
Cc Will Deacon <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Quoted 4 lines
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)
Quoted 1 line
Quoted 8 lines

Thanks, unfortunately I spotted that ARM platforms can set PCI_PROBE_ONLY also via command line (pcibios_setup()), which means that I have to add resource claiming to all ARM PCI controllers that check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we can apply this patch or alternatevely we add the resource claiming to the pcibios_fixup_bus() callback (but we can claim resources only if PCI_PROBE_ONLY is set lest we trigger regressions) which would be the simpler solution.

Quoted 2 lines

So where are we? The above response sounds like this series still needs a little tweaking?

Yes, I rewrote the generic PCI layer for claiming resources (Tomasz's posted it in its ACPI PCI ARM64 series):

https://lkml.org/lkml/2016/2/16/412

My only concern is removing PCI_PROBE_ONLY handling from arm 32 bit code, since I noticed it can be set on the command line there, claiming resources in pcibios arm 32 should do the trick, I will move the ARM 32-bit handling in a separate patch (hopefully someone can help me test it on affected platforms (?), I will ask Russell the best way to do it).

Thanks,
Lorenzo