diff mbox

[v2,1/2] PCI: generic: remove dependency on hw_pci

Message ID 554ADCE0.8020603@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Suravee Suthikulpanit May 7, 2015, 3:32 a.m. UTC
On 5/6/2015 10:18 AM, Bjorn Helgaas wrote:
> On Wed, May 6, 2015 at 9:18 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
>> On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
>>> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
>>>> The current code in pci-host-generic.c uses pci_common_init_dev()
>>>> from the arch/arm/ to do a part of the PCI initialization, and this
>>>> prevents it from being used on arm64.
>>>>
>>>> The initialization done by pci_common_init_dev() that is really
>>>> needed by pci-host-generic.c can be done in the same file without
>>>> using the hw_pci API of ARM.
>>>>
>>>> The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
>>>> this is be handled by setting up 'struct gen_pci' to embed a
>>>> pci_sys_data variable as the first element on the ARM platform.
>>>>
>>>> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
>>>> ---
>>>> Here's v2 of the patches, this enables use of pci-host-generic on
>>>> arm64.
>>>>
>>>> This has been tested on both qemu and fast model for arm64, and on
>>>> qemu for arm32.
>>>>
>>>> v1->v2
>>>>   - Address comments from Arnd Bergmann and Lorenzo Pieralisi
>>>>      - move contents of gen_pci_init to gen_pci_probe
>>>>      - assign resources only when !probe_only
>>>>   - tested on ARM32 with qemu option -M virt
>>>
>>> I tried this with an arm64 kernel running under kvmtool, but I get the
>>> following errors (a 32-bit ARM kernel does seem to work):
>>>
>>>    PCI host bridge /pci ranges:
>>>       IO 0x00000000..0x0000ffff -> 0x00000000
>>>      MEM 0x41000000..0x7fffffff -> 0x41000000
>>>    pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>>>    pci_bus 0000:00: root bus resource [bus 00-01]
>>>    pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>>    pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>>>    pci_bus 0000:00: scanning bus
>>>    pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>>>    pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>>>    pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>>>    pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>>>    pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>>>    pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>>>    pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>>>    pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>>>    pci_bus 0000:00: fixups for bus
>>>    pci_bus 0000:00: bus scan returning with max=00
>>>    pci 0000:00:00.0: fixup irq: got 10
>>>    pci 0000:00:00.0: assigning IRQ 10
>>>    pci 0000:00:01.0: fixup irq: got 11
>>>    pci 0000:00:01.0: assigning IRQ 11
>>>    virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>>>    virtio-pci: probe of 0000:00:00.0 failed with error -22
>>>    virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>>>    virtio-pci: probe of 0000:00:01.0 failed with error -22
>>
>> Ok, had a further look.
>>
>> Referring to this thread:
>>
>> https://lkml.org/lkml/2014/9/29/557
>>
>> By looking at other architectures code, resources should be claimed
>> (ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC
>> seem to do that, in slightly different fashions.
>>
>> I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used
>> to prevent enabling resources through a PCI command, which is what
>> pci_enable_resources does.
>>
>> What we can do, is providing a generic PCI layer API that allows claiming
>> resources for a specific PCI bus, something similar if not identical
>> to what is done on alpha:
>>
>> arch/alpha/kernel/pci.c pcibios_claim_one_bus()
>>
>> that is not alpha specific at all. That way, we can use the API to claim
>> bus resources instead of assigning them on PCI_PROBE_ONLY (I *think*
>> that alpha calls pci_assign_unassigned_resources() even if
>> PCI_PROBE_ONLY is set, it should be safe since resources are claimed
>> first so IIUC the PCI layer would revert to FW BAR configuration on
>> assignment failure).
>>
>> Bjorn, any opinion on this ? Putting together a patch is easy when
>> we agree on the solution.
>
> I would like claiming resources, i.e., pci_claim_resource(), to happen
> in the core instead of in arch code because it's not inherently
> arch-specific.  I don't think it should depend on PCI_PROBE_ONLY.
>
> Bjorn
>

Hi All,

I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode 
and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the 
resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from 
arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in 
pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please 
see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I 
need to reference back to the resource in the root bus, which are 
defined from the DT node. Does this sounds like a reasonable approach?

  		    int (*)(const struct pci_dev *, u8, u8));
-------- END PATCH ----

Thanks,

Suravee

Comments

Bjorn Helgaas May 12, 2015, 1:34 p.m. UTC | #1
On Wed, May 06, 2015 at 10:32:48PM -0500, Suravee Suthikulanit wrote:
> ...
> I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY
> mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run
> into the resource not claimed issue (no surprise here).
> 
> So, I tried porting the pcibios_claim_one_bus() from
> arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small
> change in pci_claim_resource(), and it seems to work w/
> PCI_PROBE_ONLY. (Please see example patch below.)
> 
> The additional while loop is needed in the pci_claim_resource()
> since I need to reference back to the resource in the root bus,
> which are defined from the DT node. Does this sounds like a
> reasonable approach?
> 
> diff --git a/drivers/pci/host/pci-host-generic.c
> b/drivers/pci/host/pci-host-generic.c
> index e9cc559..0dfa23d 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)
>  	if (!pci_has_flag(PCI_PROBE_ONLY)) {
>  		pci_bus_size_bridges(bus);
>  		pci_bus_assign_resources(bus);
> +	} else {
> +		pci_claim_one_bus(bus);
>  	}
> +
>  	pci_bus_add_devices(bus);
> 
>  	/* Configure PCI Express settings */
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 232f925..d4b43b3 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
> resource)
>  {
>  	struct resource *res = &dev->resource[resource];
>  	struct resource *root, *conflict;
> +	struct pci_dev *pdev = dev;
> 
>  	if (res->flags & IORESOURCE_UNSET) {
>  		dev_info(&dev->dev, "can't claim BAR %d %pR: no address assigned\n",
> @@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
> resource)
>  		return -EINVAL;
>  	}
> 
> -	root = pci_find_parent_resource(dev, res);
> +	while (pdev) {
> +		root = pci_find_parent_resource(pdev, res);
> +		if (root)
> +			break;
> +
> +		if (pci_has_flag(PCI_PROBE_ONLY) &&
> +		    !pci_is_root_bus(pdev->bus))
> +			pdev = pdev->bus->self;
> +		else
> +			break;
> +	}

I don't understand this new loop.  Apparently you have a device BAR, and
the upstream bridge doesn't have a window that contains the BAR?  That
sounds like a problem with the upstream bridge resources.

Do you have an example that would make this more concrete, e.g., a host
bridge, P2P bridge(s), and endpoint with their resources?

> +
>  	if (!root) {
>  		dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge
> window\n",
>  			 resource, res);
> @@ -136,6 +148,36 @@ int pci_claim_resource(struct pci_dev *dev, int
> resource)
>  }
>  EXPORT_SYMBOL(pci_claim_resource);
> 
> +void pci_claim_one_bus(struct pci_bus *b)
> +{
> +	struct pci_dev *pdev;
> +	struct pci_bus *child_bus;
> +
> +	list_for_each_entry(pdev, &b->devices, bus_list) {
> +		int i;
> +
> +		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> +			struct resource *r = &pdev->resource[i];
> +
> +			if (r->parent || !r->start || !r->flags)
> +				continue;
> +
> +			if (pci_has_flag(PCI_PROBE_ONLY) ||
> +			    (r->flags & IORESOURCE_PCI_FIXED)) {
> +				if (pci_claim_resource(pdev, i) == 0)
> +					continue;
> +
> +				pci_claim_bridge_resource(pdev, i);
> +			}
> +		}
> +	}
> +
> +	list_for_each_entry(child_bus, &b->children, node) {
> +		pci_claim_one_bus(child_bus);
> +	}
> +}
> +EXPORT_SYMBOL(pci_claim_one_bus);

I'm not a fan of pci_claim_one_bus(), on the philosophical grounds that
claiming resources is a per-device thing, and I don't want to encourage
people to do it on a per-bus level.

I'd rather claim them somewhere in the pci_device_add() path, as s390 does
in pcibios_add_device().  In fact, I'd *like* to do it even earlier, when
we read each BAR, so we could identify invalid or unassigned BARs
immediately.

> +
>  void pci_disable_bridge_window(struct pci_dev *dev)
>  {
>  	dev_info(&dev->dev, "disabling bridge mem windows\n");
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 353db8d..b59ad4b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1085,6 +1085,7 @@ void
> pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
>  void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
>  void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
>  void pdev_enable_device(struct pci_dev *);
> +void pci_claim_one_bus(struct pci_bus *b);
>  int pci_enable_resources(struct pci_dev *, int mask);
>  void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
>  		    int (*)(const struct pci_dev *, u8, u8));
> -------- END PATCH ----
> 
> Thanks,
> 
> Suravee
>
Lorenzo Pieralisi May 12, 2015, 4:34 p.m. UTC | #2
On Tue, May 12, 2015 at 02:34:31PM +0100, Bjorn Helgaas wrote:

[...]

> > +void pci_claim_one_bus(struct pci_bus *b)
> > +{
> > +	struct pci_dev *pdev;
> > +	struct pci_bus *child_bus;
> > +
> > +	list_for_each_entry(pdev, &b->devices, bus_list) {
> > +		int i;
> > +
> > +		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> > +			struct resource *r = &pdev->resource[i];
> > +
> > +			if (r->parent || !r->start || !r->flags)
> > +				continue;
> > +
> > +			if (pci_has_flag(PCI_PROBE_ONLY) ||
> > +			    (r->flags & IORESOURCE_PCI_FIXED)) {
> > +				if (pci_claim_resource(pdev, i) == 0)
> > +					continue;
> > +
> > +				pci_claim_bridge_resource(pdev, i);
> > +			}
> > +		}
> > +	}
> > +
> > +	list_for_each_entry(child_bus, &b->children, node) {
> > +		pci_claim_one_bus(child_bus);
> > +	}
> > +}
> > +EXPORT_SYMBOL(pci_claim_one_bus);
> 
> I'm not a fan of pci_claim_one_bus(), on the philosophical grounds that
> claiming resources is a per-device thing, and I don't want to encourage
> people to do it on a per-bus level.
> 
> I'd rather claim them somewhere in the pci_device_add() path, as s390 does
> in pcibios_add_device().  In fact, I'd *like* to do it even earlier, when
> we read each BAR, so we could identify invalid or unassigned BARs
> immediately.

You mean claiming the resources in __pci_read_base (and unset the resource
if claiming it fails ?) regardless of PCI_PROBE_ONLY ?

I will give it a go, I fear it might trigger regressions on other archs
though.

We could claim the resources in pcibios_add_device on arm64, but this
means arm code should be patched too since I am not happy at all to let
arm and arm64 diverge even more.

Lorenzo
Bjorn Helgaas May 12, 2015, 7:20 p.m. UTC | #3
On Tue, May 12, 2015 at 11:34 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, May 12, 2015 at 02:34:31PM +0100, Bjorn Helgaas wrote:
>
> [...]
>
>> > +void pci_claim_one_bus(struct pci_bus *b)
>> > +{
>> > +   struct pci_dev *pdev;
>> > +   struct pci_bus *child_bus;
>> > +
>> > +   list_for_each_entry(pdev, &b->devices, bus_list) {
>> > +           int i;
>> > +
>> > +           for (i = 0; i < PCI_NUM_RESOURCES; i++) {
>> > +                   struct resource *r = &pdev->resource[i];
>> > +
>> > +                   if (r->parent || !r->start || !r->flags)
>> > +                           continue;
>> > +
>> > +                   if (pci_has_flag(PCI_PROBE_ONLY) ||
>> > +                       (r->flags & IORESOURCE_PCI_FIXED)) {
>> > +                           if (pci_claim_resource(pdev, i) == 0)
>> > +                                   continue;
>> > +
>> > +                           pci_claim_bridge_resource(pdev, i);
>> > +                   }
>> > +           }
>> > +   }
>> > +
>> > +   list_for_each_entry(child_bus, &b->children, node) {
>> > +           pci_claim_one_bus(child_bus);
>> > +   }
>> > +}
>> > +EXPORT_SYMBOL(pci_claim_one_bus);
>>
>> I'm not a fan of pci_claim_one_bus(), on the philosophical grounds that
>> claiming resources is a per-device thing, and I don't want to encourage
>> people to do it on a per-bus level.
>>
>> I'd rather claim them somewhere in the pci_device_add() path, as s390 does
>> in pcibios_add_device().  In fact, I'd *like* to do it even earlier, when
>> we read each BAR, so we could identify invalid or unassigned BARs
>> immediately.
>
> You mean claiming the resources in __pci_read_base (and unset the resource
> if claiming it fails ?) regardless of PCI_PROBE_ONLY ?

That's what I'm thinking, but only in the long term.  I doubt we're
ready to go that far yet.  I was thinking more along the lines of just
getting it uniformly into the core first, maybe even incrementally,
and only after that moving it around inside the core.

I don't think it would work right now because I think we read device
BARs before we read the apertures of the upstream bridge.

> We could claim the resources in pcibios_add_device on arm64, but this
> means arm code should be patched too since I am not happy at all to let
> arm and arm64 diverge even more.

I agree, it'd be nice to have arm and arm64 be similar, but from my
point of view, they wouldn't have to be changed at the same time.  To
me, they just look like two different arches.

Bjorn
Suravee Suthikulpanit May 13, 2015, 12:47 p.m. UTC | #4
Hi Bjorn,

Somehow, I didn't get your reply email from the ML. So, I've captured 
your questions here, please see my reply below.

On 5/6/2015 10:32 PM, Suravee Suthikulanit wrote:
> Hi All,
>
> I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode
> and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the
> resource not claimed issue (no surprise here).
>
> So, I tried porting the pcibios_claim_one_bus() from
> arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in
> pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please
> see example patch below.)
>
> The additional while loop is needed in the pci_claim_resource() since I
> need to reference back to the resource in the root bus, which are
> defined from the DT node. Does this sounds like a reasonable approach?
>
> diff --git a/drivers/pci/host/pci-host-generic.c
> b/drivers/pci/host/pci-host-generic.c
> index e9cc559..0dfa23d 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)
>       if (!pci_has_flag(PCI_PROBE_ONLY)) {
>           pci_bus_size_bridges(bus);
>           pci_bus_assign_resources(bus);
> +    } else {
> +        pci_claim_one_bus(bus);
>       }
> +
>       pci_bus_add_devices(bus);
>
>       /* Configure PCI Express settings */
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 232f925..d4b43b3 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
> resource)
>   {
>       struct resource *res = &dev->resource[resource];
>       struct resource *root, *conflict;
> +    struct pci_dev *pdev = dev;
>
>       if (res->flags & IORESOURCE_UNSET) {
>           dev_info(&dev->dev, "can't claim BAR %d %pR: no address
> assigned\n",
> @@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
> resource)
>           return -EINVAL;
>       }
>
> -    root = pci_find_parent_resource(dev, res);
> +    while (pdev) {
> +        root = pci_find_parent_resource(pdev, res);
> +        if (root)
> +            break;
> +
> +        if (pci_has_flag(PCI_PROBE_ONLY) &&
> +            !pci_is_root_bus(pdev->bus))
> +            pdev = pdev->bus->self;
> +        else
> +            break;
> +    }
> +
>       if (!root) {
>           dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible
> bridge window\n",
>                resource, res);


[From Bjorn]
I don't understand this new loop.  Apparently you have a device BAR, and
the upstream bridge doesn't have a window that contains the BAR?  That
sounds like a problem with the upstream bridge resources.

Do you have an example that would make this more concrete, e.g., a host
bridge, P2P bridge(s), and endpoint with their resources?

[Suravee]
Here is the information you were asking for. This information is setup 
from the FW. In the PCI bridge (00:02.1), I see the prefetchable memory 
behind bridge is already setup.

OUTPUT from lspci -tv:

-[0000:00]-+-00.0  Advanced Micro Devices, Inc. [AMD] Device 1a00
            +-02.0  Advanced Micro Devices, Inc. [AMD] Device 1a01
            \-02.1  PCI Bridge -[01]--+-00.0  Intel Corporation 82599ES 
10-Gigabit SFI/SFP+ Network Connection
                         \-00.1  Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection

OUTPUT from lspci -vvv:

00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a00
	Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1a00
	Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0

00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a01
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-

00:02.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 1a02 
(prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
	I/O behind bridge: 00fff000-00000fff
	Memory behind bridge: fff00000-000fffff
	Prefetchable memory behind bridge: 0000007fffe00000-0000007fffffffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort+ <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
.......

01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
	Subsystem: Intel Corporation Ethernet Server Adapter X520-2
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 46
	Region 0: Memory at 7fffe80000 (64-bit, prefetchable) [size=512K]
	Region 2: I/O ports at 0020 [size=32]
	Region 4: Memory at 7ffff04000 (64-bit, prefetchable) [size=16K]
.......

01:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
	Subsystem: Intel Corporation Ethernet Server Adapter X520-2
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin B routed to IRQ 47
	Region 0: Memory at 7fffe00000 (64-bit, prefetchable) [size=512K]
	Region 2: I/O ports at 0000 [size=32]
	Region 4: Memory at 7ffff00000 (64-bit, prefetchable) [size=16K]
........

Thanks,
Suravee
Bjorn Helgaas May 13, 2015, 1:54 p.m. UTC | #5
Hi Suravee,

On Wed, May 13, 2015 at 07:47:42AM -0500, Suravee Suthikulanit wrote:
> On 5/6/2015 10:32 PM, Suravee Suthikulanit wrote:
> >I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode
> >and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the
> >resource not claimed issue (no surprise here).
> >
> >So, I tried porting the pcibios_claim_one_bus() from
> >arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in
> >pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please
> >see example patch below.)
> >
> >The additional while loop is needed in the pci_claim_resource() since I
> >need to reference back to the resource in the root bus, which are
> >defined from the DT node. Does this sounds like a reasonable approach?
> >
> >diff --git a/drivers/pci/host/pci-host-generic.c
> >b/drivers/pci/host/pci-host-generic.c
> >index e9cc559..0dfa23d 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)
> >      if (!pci_has_flag(PCI_PROBE_ONLY)) {
> >          pci_bus_size_bridges(bus);
> >          pci_bus_assign_resources(bus);
> >+    } else {
> >+        pci_claim_one_bus(bus);
> >      }
> >+
> >      pci_bus_add_devices(bus);
> >
> >      /* Configure PCI Express settings */
> >diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> >index 232f925..d4b43b3 100644
> >--- a/drivers/pci/setup-res.c
> >+++ b/drivers/pci/setup-res.c
> >@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
> >resource)
> >  {
> >      struct resource *res = &dev->resource[resource];
> >      struct resource *root, *conflict;
> >+    struct pci_dev *pdev = dev;
> >
> >      if (res->flags & IORESOURCE_UNSET) {
> >          dev_info(&dev->dev, "can't claim BAR %d %pR: no address
> >assigned\n",
> >@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
> >resource)
> >          return -EINVAL;
> >      }
> >
> >-    root = pci_find_parent_resource(dev, res);
> >+    while (pdev) {
> >+        root = pci_find_parent_resource(pdev, res);
> >+        if (root)
> >+            break;
> >+
> >+        if (pci_has_flag(PCI_PROBE_ONLY) &&
> >+            !pci_is_root_bus(pdev->bus))
> >+            pdev = pdev->bus->self;
> >+        else
> >+            break;
> >+    }
> >+
> >      if (!root) {
> >          dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible
> >bridge window\n",
> >               resource, res);
> 
> 
> [From Bjorn]
> I don't understand this new loop.  Apparently you have a device BAR, and
> the upstream bridge doesn't have a window that contains the BAR?  That
> sounds like a problem with the upstream bridge resources.
> 
> Do you have an example that would make this more concrete, e.g., a host
> bridge, P2P bridge(s), and endpoint with their resources?
> 
> [Suravee]
> Here is the information you were asking for. This information is
> setup from the FW. In the PCI bridge (00:02.1), I see the
> prefetchable memory behind bridge is already setup.

Here's a summary:

  00:02.1: PCI bridge to [bus 01]
  00:02.1:   [io window disabled]
  00:02.1:   [mem window disabled]
  00:02.1:   bridge window [mem 0x7f_ffe0_0000-0x7f_ffff_ffff 64bit pref]
  01:00.0: BAR 0: [mem 0x7f_ffe8_0000-0x... 64bit pref]
  01:00.0: BAR 2: [io  0x0020-0x3f]
  01:00.0: BAR 4: [mem 0x7f_fff0_4000-0x... 64bit pref]

So I guess the new loop would be for the I/O resource because the
00:02.1 I/O window is disabled?  This definitely seems like a problem --
we should enable that I/O window so we can claim the 01:00.0 I/O
resource with the 00:02.1 I/O window as the parent.  We don't want the
parent to be the host bridge window.  In fact, unless we enable that
I/O window, the 01:00.0 I/O BAR won't even work!

It may be that the driver for 01:00.0 doesn't need the I/O BAR.  We
can leave the bridge I/O window disabled and let pci_claim_resource()
fail, which means we'll treat the 01:00.0 I/O BAR as unassigned.
That's all fine; we'll never turn on PCI_COMMAND_IO, and as long as
the driver doesn't request I/O space, everything should just work.
Note that the driver would have to use pci_enable_device_mem() to
tell us that it doesn't need I/O space.

> OUTPUT from lspci -tv:
> 
> -[0000:00]-+-00.0  Advanced Micro Devices, Inc. [AMD] Device 1a00
>            +-02.0  Advanced Micro Devices, Inc. [AMD] Device 1a01
>            \-02.1  PCI Bridge -[01]--+-00.0  Intel Corporation
> 82599ES 10-Gigabit SFI/SFP+ Network Connection
>                         \-00.1  Intel Corporation 82599ES 10-Gigabit
> SFI/SFP+ Network Connection
> 
> OUTPUT from lspci -vvv:
> 
> 00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a00
> 	Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1a00
> 	Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx-
> 	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0
> 
> 00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a01
> 	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx-
> 	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 
> 00:02.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 1a02
> (prog-if 00 [Normal decode])
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx-
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0
> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> 	I/O behind bridge: 00fff000-00000fff
> 	Memory behind bridge: fff00000-000fffff
> 	Prefetchable memory behind bridge: 0000007fffe00000-0000007fffffffff
> 	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort+ <SERR- <PERR-
> 	BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
> 		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
> .......
> 
> 01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit
> SFI/SFP+ Network Connection (rev 01)
> 	Subsystem: Intel Corporation Ethernet Server Adapter X520-2
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx+
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0
> 	Interrupt: pin A routed to IRQ 46
> 	Region 0: Memory at 7fffe80000 (64-bit, prefetchable) [size=512K]
> 	Region 2: I/O ports at 0020 [size=32]
> 	Region 4: Memory at 7ffff04000 (64-bit, prefetchable) [size=16K]
> .......
> 
> 01:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit
> SFI/SFP+ Network Connection (rev 01)
> 	Subsystem: Intel Corporation Ethernet Server Adapter X520-2
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx+
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0
> 	Interrupt: pin B routed to IRQ 47
> 	Region 0: Memory at 7fffe00000 (64-bit, prefetchable) [size=512K]
> 	Region 2: I/O ports at 0000 [size=32]
> 	Region 4: Memory at 7ffff00000 (64-bit, prefetchable) [size=16K]
> ........
> 
> Thanks,
> Suravee
>
Suravee Suthikulpanit May 13, 2015, 3:05 p.m. UTC | #6
On 5/13/2015 8:54 AM, Bjorn Helgaas wrote:
> Hi Suravee,
>
> On Wed, May 13, 2015 at 07:47:42AM -0500, Suravee Suthikulanit wrote:
>> On 5/6/2015 10:32 PM, Suravee Suthikulanit wrote:
>>> I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode
>>> and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the
>>> resource not claimed issue (no surprise here).
>>>
>>> So, I tried porting the pcibios_claim_one_bus() from
>>> arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in
>>> pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please
>>> see example patch below.)
>>>
>>> The additional while loop is needed in the pci_claim_resource() since I
>>> need to reference back to the resource in the root bus, which are
>>> defined from the DT node. Does this sounds like a reasonable approach?
>>>
>>> diff --git a/drivers/pci/host/pci-host-generic.c
>>> b/drivers/pci/host/pci-host-generic.c
>>> index e9cc559..0dfa23d 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)
>>>       if (!pci_has_flag(PCI_PROBE_ONLY)) {
>>>           pci_bus_size_bridges(bus);
>>>           pci_bus_assign_resources(bus);
>>> +    } else {
>>> +        pci_claim_one_bus(bus);
>>>       }
>>> +
>>>       pci_bus_add_devices(bus);
>>>
>>>       /* Configure PCI Express settings */
>>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>>> index 232f925..d4b43b3 100644
>>> --- a/drivers/pci/setup-res.c
>>> +++ b/drivers/pci/setup-res.c
>>> @@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
>>> resource)
>>>   {
>>>       struct resource *res = &dev->resource[resource];
>>>       struct resource *root, *conflict;
>>> +    struct pci_dev *pdev = dev;
>>>
>>>       if (res->flags & IORESOURCE_UNSET) {
>>>           dev_info(&dev->dev, "can't claim BAR %d %pR: no address
>>> assigned\n",
>>> @@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
>>> resource)
>>>           return -EINVAL;
>>>       }
>>>
>>> -    root = pci_find_parent_resource(dev, res);
>>> +    while (pdev) {
>>> +        root = pci_find_parent_resource(pdev, res);
>>> +        if (root)
>>> +            break;
>>> +
>>> +        if (pci_has_flag(PCI_PROBE_ONLY) &&
>>> +            !pci_is_root_bus(pdev->bus))
>>> +            pdev = pdev->bus->self;
>>> +        else
>>> +            break;
>>> +    }
>>> +
>>>       if (!root) {
>>>           dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible
>>> bridge window\n",
>>>                resource, res);
>>
>>
>> [From Bjorn]
>> I don't understand this new loop.  Apparently you have a device BAR, and
>> the upstream bridge doesn't have a window that contains the BAR?  That
>> sounds like a problem with the upstream bridge resources.
>>
>> Do you have an example that would make this more concrete, e.g., a host
>> bridge, P2P bridge(s), and endpoint with their resources?
>>
>> [Suravee]
>> Here is the information you were asking for. This information is
>> setup from the FW. In the PCI bridge (00:02.1), I see the
>> prefetchable memory behind bridge is already setup.
>
> Here's a summary:
>
>    00:02.1: PCI bridge to [bus 01]
>    00:02.1:   [io window disabled]
>    00:02.1:   [mem window disabled]
>    00:02.1:   bridge window [mem 0x7f_ffe0_0000-0x7f_ffff_ffff 64bit pref]
>    01:00.0: BAR 0: [mem 0x7f_ffe8_0000-0x... 64bit pref]
>    01:00.0: BAR 2: [io  0x0020-0x3f]
>    01:00.0: BAR 4: [mem 0x7f_fff0_4000-0x... 64bit pref]
>
> So I guess the new loop would be for the I/O resource because the
> 00:02.1 I/O window is disabled?  This definitely seems like a problem --
> we should enable that I/O window so we can claim the 01:00.0 I/O
> resource with the 00:02.1 I/O window as the parent.  We don't want the
> parent to be the host bridge window.  In fact, unless we enable that
> I/O window, the 01:00.0 I/O BAR won't even work!
>
> It may be that the driver for 01:00.0 doesn't need the I/O BAR.  We
> can leave the bridge I/O window disabled and let pci_claim_resource()
> fail, which means we'll treat the 01:00.0 I/O BAR as unassigned.
> That's all fine; we'll never turn on PCI_COMMAND_IO, and as long as
> the driver doesn't request I/O space, everything should just work.
> Note that the driver would have to use pci_enable_device_mem() to
> tell us that it doesn't need I/O space.
>

I see your point on the I/O window. Let me double check the FW why this 
is getting setup this way. My guess is that the I/O windows are not used 
by ARM64 systems, therefore the FW disabled it at the bridge.

However, the loop is mainly to recursively search for parent resource 
for the 64bit pref resource of device 1:00.0 when calling 
pci_claim_one_bus() on PROBE_ONLY. It doesn't seem to find compatible 
bridge windows in the parent bridge (0:2.1), and I was not sure if that 
is where the device is supposed to be claiming from.

Since you mentioned in a separate reply in this thread that we might not 
want to be using pci_claim_one_bus(), I guess we probably should drop 
this approach for now.

Thanks,
Suravee
Bjorn Helgaas May 13, 2015, 3:11 p.m. UTC | #7
On Wed, May 13, 2015 at 10:05:23AM -0500, Suravee Suthikulanit wrote:
> On 5/13/2015 8:54 AM, Bjorn Helgaas wrote:
> >Hi Suravee,
> >
> >On Wed, May 13, 2015 at 07:47:42AM -0500, Suravee Suthikulanit wrote:
> >>On 5/6/2015 10:32 PM, Suravee Suthikulanit wrote:
> >>>I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode
> >>>and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the
> >>>resource not claimed issue (no surprise here).
> >>>
> >>>So, I tried porting the pcibios_claim_one_bus() from
> >>>arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in
> >>>pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please
> >>>see example patch below.)
> >>>
> >>>The additional while loop is needed in the pci_claim_resource() since I
> >>>need to reference back to the resource in the root bus, which are
> >>>defined from the DT node. Does this sounds like a reasonable approach?
> >>>
> >>>diff --git a/drivers/pci/host/pci-host-generic.c
> >>>b/drivers/pci/host/pci-host-generic.c
> >>>index e9cc559..0dfa23d 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)
> >>>      if (!pci_has_flag(PCI_PROBE_ONLY)) {
> >>>          pci_bus_size_bridges(bus);
> >>>          pci_bus_assign_resources(bus);
> >>>+    } else {
> >>>+        pci_claim_one_bus(bus);
> >>>      }
> >>>+
> >>>      pci_bus_add_devices(bus);
> >>>
> >>>      /* Configure PCI Express settings */
> >>>diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> >>>index 232f925..d4b43b3 100644
> >>>--- a/drivers/pci/setup-res.c
> >>>+++ b/drivers/pci/setup-res.c
> >>>@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
> >>>resource)
> >>>  {
> >>>      struct resource *res = &dev->resource[resource];
> >>>      struct resource *root, *conflict;
> >>>+    struct pci_dev *pdev = dev;
> >>>
> >>>      if (res->flags & IORESOURCE_UNSET) {
> >>>          dev_info(&dev->dev, "can't claim BAR %d %pR: no address
> >>>assigned\n",
> >>>@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
> >>>resource)
> >>>          return -EINVAL;
> >>>      }
> >>>
> >>>-    root = pci_find_parent_resource(dev, res);
> >>>+    while (pdev) {
> >>>+        root = pci_find_parent_resource(pdev, res);
> >>>+        if (root)
> >>>+            break;
> >>>+
> >>>+        if (pci_has_flag(PCI_PROBE_ONLY) &&
> >>>+            !pci_is_root_bus(pdev->bus))
> >>>+            pdev = pdev->bus->self;
> >>>+        else
> >>>+            break;
> >>>+    }
> >>>+
> >>>      if (!root) {
> >>>          dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible
> >>>bridge window\n",
> >>>               resource, res);
> >>
> >>
> >>[From Bjorn]
> >>I don't understand this new loop.  Apparently you have a device BAR, and
> >>the upstream bridge doesn't have a window that contains the BAR?  That
> >>sounds like a problem with the upstream bridge resources.
> >>
> >>Do you have an example that would make this more concrete, e.g., a host
> >>bridge, P2P bridge(s), and endpoint with their resources?
> >>
> >>[Suravee]
> >>Here is the information you were asking for. This information is
> >>setup from the FW. In the PCI bridge (00:02.1), I see the
> >>prefetchable memory behind bridge is already setup.
> >
> >Here's a summary:
> >
> >   00:02.1: PCI bridge to [bus 01]
> >   00:02.1:   [io window disabled]
> >   00:02.1:   [mem window disabled]
> >   00:02.1:   bridge window [mem 0x7f_ffe0_0000-0x7f_ffff_ffff 64bit pref]
> >   01:00.0: BAR 0: [mem 0x7f_ffe8_0000-0x... 64bit pref]
> >   01:00.0: BAR 2: [io  0x0020-0x3f]
> >   01:00.0: BAR 4: [mem 0x7f_fff0_4000-0x... 64bit pref]
> >
> >So I guess the new loop would be for the I/O resource because the
> >00:02.1 I/O window is disabled?  This definitely seems like a problem --
> >we should enable that I/O window so we can claim the 01:00.0 I/O
> >resource with the 00:02.1 I/O window as the parent.  We don't want the
> >parent to be the host bridge window.  In fact, unless we enable that
> >I/O window, the 01:00.0 I/O BAR won't even work!
> >
> >It may be that the driver for 01:00.0 doesn't need the I/O BAR.  We
> >can leave the bridge I/O window disabled and let pci_claim_resource()
> >fail, which means we'll treat the 01:00.0 I/O BAR as unassigned.
> >That's all fine; we'll never turn on PCI_COMMAND_IO, and as long as
> >the driver doesn't request I/O space, everything should just work.
> >Note that the driver would have to use pci_enable_device_mem() to
> >tell us that it doesn't need I/O space.
> >
> 
> I see your point on the I/O window. Let me double check the FW why
> this is getting setup this way. My guess is that the I/O windows are
> not used by ARM64 systems, therefore the FW disabled it at the
> bridge.
> 
> However, the loop is mainly to recursively search for parent
> resource for the 64bit pref resource of device 1:00.0 when calling
> pci_claim_one_bus() on PROBE_ONLY. It doesn't seem to find
> compatible bridge windows in the parent bridge (0:2.1), and I was
> not sure if that is where the device is supposed to be claiming
> from.

An endpoint should claim resources from its immediate upstream bridge.
There's no point in claiming a resource from the top-level bridge if a
bridge in the middle doesn't forward that resource down the the endpoint.

Bjorn
diff mbox

Patch

diff --git a/drivers/pci/host/pci-host-generic.c 
b/drivers/pci/host/pci-host-generic.c
index e9cc559..0dfa23d 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)
  	if (!pci_has_flag(PCI_PROBE_ONLY)) {
  		pci_bus_size_bridges(bus);
  		pci_bus_assign_resources(bus);
+	} else {
+		pci_claim_one_bus(bus);
  	}
+
  	pci_bus_add_devices(bus);

  	/* Configure PCI Express settings */
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 232f925..d4b43b3 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -109,6 +109,7 @@  int pci_claim_resource(struct pci_dev *dev, int 
resource)
  {
  	struct resource *res = &dev->resource[resource];
  	struct resource *root, *conflict;
+	struct pci_dev *pdev = dev;

  	if (res->flags & IORESOURCE_UNSET) {
  		dev_info(&dev->dev, "can't claim BAR %d %pR: no address assigned\n",
@@ -116,7 +117,18 @@  int pci_claim_resource(struct pci_dev *dev, int 
resource)
  		return -EINVAL;
  	}

-	root = pci_find_parent_resource(dev, res);
+	while (pdev) {
+		root = pci_find_parent_resource(pdev, res);
+		if (root)
+			break;
+
+		if (pci_has_flag(PCI_PROBE_ONLY) &&
+		    !pci_is_root_bus(pdev->bus))
+			pdev = pdev->bus->self;
+		else
+			break;
+	}
+
  	if (!root) {
  		dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge 
window\n",
  			 resource, res);
@@ -136,6 +148,36 @@  int pci_claim_resource(struct pci_dev *dev, int 
resource)
  }
  EXPORT_SYMBOL(pci_claim_resource);

+void pci_claim_one_bus(struct pci_bus *b)
+{
+	struct pci_dev *pdev;
+	struct pci_bus *child_bus;
+
+	list_for_each_entry(pdev, &b->devices, bus_list) {
+		int i;
+
+		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+			struct resource *r = &pdev->resource[i];
+
+			if (r->parent || !r->start || !r->flags)
+				continue;
+
+			if (pci_has_flag(PCI_PROBE_ONLY) ||
+			    (r->flags & IORESOURCE_PCI_FIXED)) {
+				if (pci_claim_resource(pdev, i) == 0)
+					continue;
+
+				pci_claim_bridge_resource(pdev, i);
+			}
+		}
+	}
+
+	list_for_each_entry(child_bus, &b->children, node) {
+		pci_claim_one_bus(child_bus);
+	}
+}
+EXPORT_SYMBOL(pci_claim_one_bus);
+
  void pci_disable_bridge_window(struct pci_dev *dev)
  {
  	dev_info(&dev->dev, "disabling bridge mem windows\n");
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 353db8d..b59ad4b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1085,6 +1085,7 @@  void pci_assign_unassigned_bridge_resources(struct 
pci_dev *bridge);
  void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
  void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
  void pdev_enable_device(struct pci_dev *);
+void pci_claim_one_bus(struct pci_bus *b);
  int pci_enable_resources(struct pci_dev *, int mask);
  void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),