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

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

[Lorenzo Pieralisi]

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.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h     |  1 +
 2 files changed, 43 insertions(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1723ac1..9054541 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -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)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e828e7b..28ce9c5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.5.1

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

[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.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David Daney <david.daney@cavium.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-host-generic.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 5434c90..c62d13f 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -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);
 
-- 
2.5.1

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

[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.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 6551d28..9c998d5 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -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)
 {
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index b3d098b..4095379 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -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
  */
-- 
2.5.1

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

[Will Deacon]

On Tue, Nov 17, 2015 at 05:03:57PM +0000, Lorenzo Pieralisi wrote:
> 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.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  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:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 5434c90..c62d13f 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -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);
>  
> -- 
> 2.5.1
> 

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

[Will Deacon]

On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> 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.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  arch/arm64/kernel/pci.c  | 13 -------------
>  2 files changed, 25 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

[Lorenzo Pieralisi]

On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > 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.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  arch/arm/kernel/bios32.c | 12 ------------
> >  arch/arm64/kernel/pci.c  | 13 -------------
> >  2 files changed, 25 deletions(-)
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

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

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

[Bjorn Helgaas]

Hi Lorenzo,

On Wed, Jan 20, 2016 at 04:22:15PM +0000, Lorenzo Pieralisi wrote:
> On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> > On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > > 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.
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Russell King <linux@arm.linux.org.uk>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > ---
> > >  arch/arm/kernel/bios32.c | 12 ------------
> > >  arch/arm64/kernel/pci.c  | 13 -------------
> > >  2 files changed, 25 deletions(-)
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> 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

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

[Lorenzo Pieralisi]

Hi Bjorn,

On Thu, Feb 25, 2016 at 09:24:17AM -0600, Bjorn Helgaas wrote:
> Hi Lorenzo,
> 
> On Wed, Jan 20, 2016 at 04:22:15PM +0000, Lorenzo Pieralisi wrote:
> > On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> > > On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > > > 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.
> > > > 
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Russell King <linux@arm.linux.org.uk>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > ---
> > > >  arch/arm/kernel/bios32.c | 12 ------------
> > > >  arch/arm64/kernel/pci.c  | 13 -------------
> > > >  2 files changed, 25 deletions(-)
> > > 
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > 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?

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