Re: [PATCH] x86: fix _CRS resources return handling

Re: [PATCH] x86: fix _CRS resources return handling

[Yinghai Lu]

please check this one

[PATCH] x86/pci: fix boundary checking for root res

don't touch info->res_num if we are out of space

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c |   27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -68,6 +68,10 @@ setup_resource(struct acpi_resource *acp
 	unsigned long flags;
 	struct resource *root;
 	int max_root_bus_resources = PCI_BUS_NUM_RESOURCES;
+	u64 start, end;
+
+	if (bus_has_transparent_bridge(info->bus))
+		max_root_bus_resources -= 3;
 
 	status = resource_to_addr(acpi_res, &addr);
 	if (!ACPI_SUCCESS(status))
@@ -84,25 +88,24 @@ setup_resource(struct acpi_resource *acp
 	} else
 		return AE_OK;
 
-	res = &info->res[info->res_num];
-	res->name = info->name;
-	res->flags = flags;
-	res->start = addr.minimum + addr.translation_offset;
-	res->end = res->start + addr.address_length - 1;
-	res->child = NULL;
-
-	if (bus_has_transparent_bridge(info->bus))
-		max_root_bus_resources -= 3;
+	start = addr.minimum + addr.translation_offset;
+	end = start + addr.address_length - 1;
 	if (info->res_num >= max_root_bus_resources) {
 		printk(KERN_WARNING "PCI: Failed to allocate 0x%lx-0x%lx "
 			"from %s for %s due to _CRS returning more than "
-			"%d resource descriptors\n", (unsigned long) res->start,
-			(unsigned long) res->end, root->name, info->name,
+			"%d resource descriptors\n", (unsigned long) start,
+			(unsigned long) end, root->name, info->name,
 			max_root_bus_resources);
-		info->res_num++;
 		return AE_OK;
 	}
 
+	res = &info->res[info->res_num];
+	res->name = info->name;
+	res->flags = flags;
+	res->start = start;
+	res->end = end;
+	res->child = NULL;
+
 	if (insert_resource(root, res)) {
 		printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
 			"from %s for %s\n", (unsigned long) res->start,

[PATCH] x86/pci: get root CRS before scan child -v2

[Yinghai Lu]

so we could remove adjust_transparent_bridge_resources..

and give x86_pci_root_bus_res_quirks chance when _CRS is not used or
not there.

v2: add print out if pci conf reading for res is used for root

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c    |   32 +++++++++-----------------------
 arch/x86/pci/amd_bus.c |    8 ++++++--
 2 files changed, 15 insertions(+), 25 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -118,23 +118,6 @@ setup_resource(struct acpi_resource *acp
 }
 
 static void
-adjust_transparent_bridge_resources(struct pci_bus *bus)
-{
-	struct pci_dev *dev;
-
-	list_for_each_entry(dev, &bus->devices, bus_list) {
-		int i;
-		u16 class = dev->class >> 8;
-
-		if (class == PCI_CLASS_BRIDGE_PCI && dev->transparent) {
-			for(i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
-				dev->subordinate->resource[i] =
-						dev->bus->resource[i - 3];
-		}
-	}
-}
-
-static void
 get_current_resources(struct acpi_device *device, int busnum,
 			int domain, struct pci_bus *bus)
 {
@@ -161,8 +144,6 @@ get_current_resources(struct acpi_device
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, setup_resource,
 				&info);
-	if (info.res_num)
-		adjust_transparent_bridge_resources(bus);
 
 	return;
 
@@ -225,8 +206,15 @@ struct pci_bus * __devinit pci_acpi_scan
 		 */
 		memcpy(bus->sysdata, sd, sizeof(*sd));
 		kfree(sd);
-	} else
-		bus = pci_scan_bus_parented(NULL, busnum, &pci_root_ops, sd);
+	} else {
+		bus = pci_create_bus(NULL, busnum, &pci_root_ops, sd);
+		if (bus) {
+			if (!(pci_probe & PCI_NO_ROOT_CRS))
+				get_current_resources(device, busnum, domain,
+							bus);
+			bus->subordinate = pci_scan_child_bus(bus);
+		}
+	}
 
 	if (!bus)
 		kfree(sd);
@@ -241,8 +229,6 @@ struct pci_bus * __devinit pci_acpi_scan
 #endif
 	}
 
-	if (bus && !(pci_probe & PCI_NO_ROOT_CRS))
-		get_current_resources(device, busnum, domain, bus);
 	return bus;
 }
 
Index: linux-2.6/arch/x86/pci/amd_bus.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/amd_bus.c
+++ linux-2.6/arch/x86/pci/amd_bus.c
@@ -100,8 +100,9 @@ void x86_pci_root_bus_res_quirks(struct
 	int j;
 	struct pci_root_info *info;
 
-	/* don't go for it if _CRS is used */
-	if (!(pci_probe & PCI_NO_ROOT_CRS))
+       /* don't go for it if _CRS is used already */
+	if (b->resource[0] != &ioport_resource ||
+	    b->resource[1] != &iomem_resource)
 		return;
 
 	/* if only one root bus, don't need to anything */
@@ -116,6 +117,9 @@ void x86_pci_root_bus_res_quirks(struct
 	if (i == pci_root_num)
 		return;
 
+	printk(KERN_DEBUG "PCI: peer root bus %02x res updated from pci conf\n",
+			b->number);
+
 	info = &pci_root_info[i];
 	for (j = 0; j < info->res_num; j++) {
 		struct resource *res;

Re: [PATCH] x86/pci: get root CRS before scan child -v2

[Jesse Barnes]

On Wed, 24 Jun 2009 14:48:25 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> 
> so we could remove adjust_transparent_bridge_resources..
> 
> and give x86_pci_root_bus_res_quirks chance when _CRS is not used or
> not there.
> 
> v2: add print out if pci conf reading for res is used for root
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Do you think this is generally safe?  I was hoping we could do
something more complete, like making the code handle arbitrary numbers
of resources in _CRS.  Hacking that together now.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: get root CRS before scan child -v2

[Yinghai]

On Jun 24, 2009, at 3:37 PM, Jesse Barnes <jbarnes@virtuousgeek.org>  
wrote:

> On Wed, 24 Jun 2009 14:48:25 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>>
>> so we could remove adjust_transparent_bridge_resources..
>>
>> and give x86_pci_root_bus_res_quirks chance when _CRS is not used or
>> not there.
>>
>> v2: add print out if pci conf reading for res is used for root
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> Do you think this is generally safe?  I was hoping we could do
> something more complete, like making the code handle arbitrary numbers
> of resources in _CRS.  Hacking that together now.

Should be safe

YH

[PATCH] x86/pci: don't use crs for root if we only have one root bus

[Yinghai Lu]

for AMD system, when only one PCI root, just set PCI_NO_ROOT_CRS for it

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/amd_bus.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6/arch/x86/pci/amd_bus.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/amd_bus.c
+++ linux-2.6/arch/x86/pci/amd_bus.c
@@ -561,6 +561,10 @@ static int __init early_fill_mp_bus_info
 		}
 	}
 
+	/* don't use _CRS if we only have one root */
+	if (pci_root_num <= 1)
+		pci_probe |= PCI_NO_ROOT_CRS;
+
 	return 0;
 }
 

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Linus Torvalds]

Should we not just revert 9e9f46c44e487af0a82eb61b624553e2f7118f5b?

The thing says:

 "At this point, it seems to solve more problems than it causes, so let's 
  try using it by default.  It's an easy revert if it ends up causing 
  trouble."

and it clearly does _not_ solve more problems than it causes, and the 
whole message in that commit implies we should revert it.

I'm happy to apply various patches to fix it up, but regardless, I thinkwe 
should revert that commit as bogus. We can try making it the default again 
next round, when maybe it will be true that it doesn't cause issues.

What did it even ever help with?

		Linus

On Wed, 24 Jun 2009, Yinghai Lu wrote:
> 
> for AMD system, when only one PCI root, just set PCI_NO_ROOT_CRS for it
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/pci/amd_bus.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: linux-2.6/arch/x86/pci/amd_bus.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/pci/amd_bus.c
> +++ linux-2.6/arch/x86/pci/amd_bus.c
> @@ -561,6 +561,10 @@ static int __init early_fill_mp_bus_info
>  		}
>  	}
>  
> +	/* don't use _CRS if we only have one root */
> +	if (pci_root_num <= 1)
> +		pci_probe |= PCI_NO_ROOT_CRS;
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Linus Torvalds]

On Wed, 24 Jun 2009, Linus Torvalds wrote:
>
> I'm happy to apply various patches to fix it up, but regardless, I thinkwe 
> should revert that commit as bogus. We can try making it the default again 
> next round, when maybe it will be true that it doesn't cause issues.

Btw, I really think our _CRS handling sucks.

There's two things that you can do with _CRS:

 - use the _existence_ of it as an indicator of a root bus

 - try to use it to populate the resource tree.

And quite frankly, I think #2 is broken. There's no way in hell that ACPI 
tables are ever going to be better than just asking the hardware. We've 
gone through this before. Trusting ACPI over the hardware is just 
FUNDAMENTALLY WRONG.

So I'm just going to do that revert. I'm not sure if it ever makes sense 
to make that insane _CRS code the default. It seems like a fundamentally 
flawed idea.

			Linus

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Jesse Barnes]

On Wed, 24 Jun 2009 16:21:09 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Wed, 24 Jun 2009, Linus Torvalds wrote:
> >
> > I'm happy to apply various patches to fix it up, but regardless, I
> > thinkwe should revert that commit as bogus. We can try making it
> > the default again next round, when maybe it will be true that it
> > doesn't cause issues.
> 
> Btw, I really think our _CRS handling sucks.
> 
> There's two things that you can do with _CRS:
> 
>  - use the _existence_ of it as an indicator of a root bus
> 
>  - try to use it to populate the resource tree.
> 
> And quite frankly, I think #2 is broken. There's no way in hell that
> ACPI tables are ever going to be better than just asking the
> hardware. We've gone through this before. Trusting ACPI over the
> hardware is just FUNDAMENTALLY WRONG.
> 
> So I'm just going to do that revert. I'm not sure if it ever makes
> sense to make that insane _CRS code the default. It seems like a
> fundamentally flawed idea.

Yeah, I think it's reasonable to revert, especially given how we do
_CRS handling currently.  I'm hoping at some point we can use the _CRS
data to at least augment the configuration we get from hardware, since
on some machines it seems to be necessary.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Linus Torvalds]

On Wed, 24 Jun 2009, Jesse Barnes wrote:
> 
> Yeah, I think it's reasonable to revert, especially given how we do
> _CRS handling currently.  I'm hoping at some point we can use the _CRS
> data to at least augment the configuration we get from hardware, since
> on some machines it seems to be necessary.

Agreed. I do think we should take _CRS into account - possibly just as a 
minimal hint to determine which root buses to try to scan (maybe we do 
this already, I really didn't check). Or maybe we could use it to extend 
on our scan information.

But when it seems to have things like "this bus can forward VGA cycles" 
kind of "resources" (which seems to be the main reason Larry Finger has so 
many of them), then that's just worthless knowledge that we're much better 
off just determining on our own.

Anyway, I may feel pretty strongly about things like this, but I'm also 
open to being convinced otherwise for 2.6.32. I wanted to do -rc1 today 
(it's been more than two weeks), and while I don't expect -rc1 to be 
flawless, I also hate to release it with _known_ bugs.

So partly due to timing, I'd rather revert it, and we can revisit it for 
the next release - whatever the actual end result then will be.

[ There's a difference between "we're supposed to find and fix bugs in the 
  -rc series", and "I release known-buggy -rc1's since we're supposed to 
  fix it later". For similar reasons, I hate pulling known-buggy stuff 
  during the merge window - it's ok if it shows itself to be buggy 
  _later_, but if people send me stuff that they know is buggy as they 
  send it to me, then that's a problem. ]

		Linus

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Jesse Barnes]

On Wed, 24 Jun 2009 16:54:08 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, 24 Jun 2009, Jesse Barnes wrote:
> > 
> > Yeah, I think it's reasonable to revert, especially given how we do
> > _CRS handling currently.  I'm hoping at some point we can use the
> > _CRS data to at least augment the configuration we get from
> > hardware, since on some machines it seems to be necessary.
> 
> Agreed. I do think we should take _CRS into account - possibly just
> as a minimal hint to determine which root buses to try to scan (maybe
> we do this already, I really didn't check). Or maybe we could use it
> to extend on our scan information.
> 
> But when it seems to have things like "this bus can forward VGA
> cycles" kind of "resources" (which seems to be the main reason Larry
> Finger has so many of them), then that's just worthless knowledge
> that we're much better off just determining on our own.

Yeah, some of those bits don't seem useful; but OTOH if a given bridge
decodes a certain range in a non-configurable way how else will we get
that info w/o having huge tables of per-chip info?  Those are the sorts
of things I worry about with our current resource handling.  Maybe not
directly _CRS related, but still...

> Anyway, I may feel pretty strongly about things like this, but I'm
> also open to being convinced otherwise for 2.6.32. I wanted to do
> -rc1 today (it's been more than two weeks), and while I don't expect
> -rc1 to be flawless, I also hate to release it with _known_ bugs.

Sure, we'll keep at it and see if we can get something better in place
for .32.

> So partly due to timing, I'd rather revert it, and we can revisit it
> for the next release - whatever the actual end result then will be.
> 
> [ There's a difference between "we're supposed to find and fix bugs
> in the -rc series", and "I release known-buggy -rc1's since we're
> supposed to fix it later". For similar reasons, I hate pulling
> known-buggy stuff during the merge window - it's ok if it shows
> itself to be buggy _later_, but if people send me stuff that they
> know is buggy as they send it to me, then that's a problem. ]

Yeah, 100% agreed.  I didn't hear any reports until after people
started using your tree, so I think this case was handled correctly:
push something that *seems* ok upstream, but with eyes wide open for
the possibility we'd need to revert.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Ingo Molnar]

* Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> > [ There's a difference between "we're supposed to find and fix bugs
> > in the -rc series", and "I release known-buggy -rc1's since we're
> > supposed to fix it later". For similar reasons, I hate pulling
> > known-buggy stuff during the merge window - it's ok if it shows
> > itself to be buggy _later_, but if people send me stuff that they
> > know is buggy as they send it to me, then that's a problem. ]
> 
> Yeah, 100% agreed.  I didn't hear any reports until after people 
> started using your tree, so I think this case was handled 
> correctly: push something that *seems* ok upstream, but with eyes 
> wide open for the possibility we'd need to revert.

There's only one small gripe i have with the handling of it: the 
timing. "9e9f46c: PCI: use ACPI _CRS data by default" was written 
and committed on June 11th, two days _after_ the merge window 
opened.

That's way too late for maybe-broken changes to x86 lowlevel details 
(especially if it touches hw-environmental interaction - which is 
very hard to test with meaningful coverage), and it's also pretty 
much the worst moment to solicit testing from people who are busy 
getting their stuff to Linus and who are busy testing out any of the 
unexpected interactions and bugs.

So this was, to a certain degree, a predictable outcome. Trees in 
the Linux "critical path" of testing (core kernel, x86, core 
networking, very common drivers, PCI, driver core, VFS, etc.) should 
generally try to cool down 1-2 weeks before the merge window - 
because breakage there can do a lot of knock-on cascading damage. 
Two weeks is not a lot of time and the effects of showstopper bugs 
get magnified disproportionately.

	Ingo

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Jaswinder Singh Rajput]

On Thu, 2009-06-25 at 09:03 +0200, Ingo Molnar wrote:
> * Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > > [ There's a difference between "we're supposed to find and fix bugs
> > > in the -rc series", and "I release known-buggy -rc1's since we're
> > > supposed to fix it later". For similar reasons, I hate pulling
> > > known-buggy stuff during the merge window - it's ok if it shows
> > > itself to be buggy _later_, but if people send me stuff that they
> > > know is buggy as they send it to me, then that's a problem. ]
> > 
> > Yeah, 100% agreed.  I didn't hear any reports until after people 
> > started using your tree, so I think this case was handled 
> > correctly: push something that *seems* ok upstream, but with eyes 
> > wide open for the possibility we'd need to revert.
> 
> There's only one small gripe i have with the handling of it: the 
> timing. "9e9f46c: PCI: use ACPI _CRS data by default" was written 
> and committed on June 11th, two days _after_ the merge window 
> opened.
> 
> That's way too late for maybe-broken changes to x86 lowlevel details 
> (especially if it touches hw-environmental interaction - which is 
> very hard to test with meaningful coverage), and it's also pretty 
> much the worst moment to solicit testing from people who are busy 
> getting their stuff to Linus and who are busy testing out any of the 
> unexpected interactions and bugs.
> 
> So this was, to a certain degree, a predictable outcome. Trees in 
> the Linux "critical path" of testing (core kernel, x86, core 
> networking, very common drivers, PCI, driver core, VFS, etc.) should 
> generally try to cool down 1-2 weeks before the merge window - 
> because breakage there can do a lot of knock-on cascading damage. 
> Two weeks is not a lot of time and the effects of showstopper bugs 
> get magnified disproportionately.
> 

Yes, I was also thinking about this when I checked the commit date. And
totally agree with Ingo's suggestions.

Thanks,
--
JSR

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Jesse Barnes]

On Thu, 25 Jun 2009 09:03:47 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > > [ There's a difference between "we're supposed to find and fix
> > > bugs in the -rc series", and "I release known-buggy -rc1's since
> > > we're supposed to fix it later". For similar reasons, I hate
> > > pulling known-buggy stuff during the merge window - it's ok if it
> > > shows itself to be buggy _later_, but if people send me stuff
> > > that they know is buggy as they send it to me, then that's a
> > > problem. ]
> > 
> > Yeah, 100% agreed.  I didn't hear any reports until after people 
> > started using your tree, so I think this case was handled 
> > correctly: push something that *seems* ok upstream, but with eyes 
> > wide open for the possibility we'd need to revert.
> 
> There's only one small gripe i have with the handling of it: the 
> timing. "9e9f46c: PCI: use ACPI _CRS data by default" was written 
> and committed on June 11th, two days _after_ the merge window 
> opened.
> 
> That's way too late for maybe-broken changes to x86 lowlevel details 
> (especially if it touches hw-environmental interaction - which is 
> very hard to test with meaningful coverage), and it's also pretty 
> much the worst moment to solicit testing from people who are busy 
> getting their stuff to Linus and who are busy testing out any of the 
> unexpected interactions and bugs.

True, the patch had been around long before that and I definitely
should have committed it sooner.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] x86/pci: don't use crs for root if we only have one root bus

[Gary Hade]

On Wed, Jun 24, 2009 at 04:09:36PM -0700, Linus Torvalds wrote:
> 
> Should we not just revert 9e9f46c44e487af0a82eb61b624553e2f7118f5b?
> 
> The thing says:
> 
>  "At this point, it seems to solve more problems than it causes, so let's 
>   try using it by default.  It's an easy revert if it ends up causing 
>   trouble."
> 
> and it clearly does _not_ solve more problems than it causes, and the 
> whole message in that commit implies we should revert it.
> 
> I'm happy to apply various patches to fix it up, but regardless, I thinkwe 
> should revert that commit as bogus. We can try making it the default again 
> next round, when maybe it will be true that it doesn't cause issues.
> 
> What did it even ever help with?

In our case it is needed on some of our systems for PCI hotplug to
avoid MCKs due to a device under one root bus getting a resource
during hotplug that can only be used by devices under a different
root bus.  If the user does not intend to use PCI hotplug, the
function isn't needed (i.e. 'pci=use_crs' can be omitted) because
resources are properly directed to the installed cards via BIOS
pre-assignment.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

[PATCH 1/3] x86/pci: fix boundary checking when using root CRS

[Yinghai Lu]

don't touch info->res_num if we are out of space

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c |   27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -68,6 +68,10 @@ setup_resource(struct acpi_resource *acp
 	unsigned long flags;
 	struct resource *root;
 	int max_root_bus_resources = PCI_BUS_NUM_RESOURCES;
+	u64 start, end;
+
+	if (bus_has_transparent_bridge(info->bus))
+		max_root_bus_resources -= 3;
 
 	status = resource_to_addr(acpi_res, &addr);
 	if (!ACPI_SUCCESS(status))
@@ -84,25 +88,24 @@ setup_resource(struct acpi_resource *acp
 	} else
 		return AE_OK;
 
-	res = &info->res[info->res_num];
-	res->name = info->name;
-	res->flags = flags;
-	res->start = addr.minimum + addr.translation_offset;
-	res->end = res->start + addr.address_length - 1;
-	res->child = NULL;
-
-	if (bus_has_transparent_bridge(info->bus))
-		max_root_bus_resources -= 3;
+	start = addr.minimum + addr.translation_offset;
+	end = start + addr.address_length - 1;
 	if (info->res_num >= max_root_bus_resources) {
 		printk(KERN_WARNING "PCI: Failed to allocate 0x%lx-0x%lx "
 			"from %s for %s due to _CRS returning more than "
-			"%d resource descriptors\n", (unsigned long) res->start,
-			(unsigned long) res->end, root->name, info->name,
+			"%d resource descriptors\n", (unsigned long) start,
+			(unsigned long) end, root->name, info->name,
 			max_root_bus_resources);
-		info->res_num++;
 		return AE_OK;
 	}
 
+	res = &info->res[info->res_num];
+	res->name = info->name;
+	res->flags = flags;
+	res->start = start;
+	res->end = end;
+	res->child = NULL;
+
 	if (insert_resource(root, res)) {
 		printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
 			"from %s for %s\n", (unsigned long) res->start,

[PATCH 2/3] x86/pci: get root CRS before scan childs -v3

[Yinghai Lu]

so we could remove adjust_transparent_bridge_resources..

and give x86_pci_root_bus_res_quirks chance when _CRS is not used or
not there.

v2: add print out if pci conf reading for res is used for root
v3: update after revert PCI_NO_ROOT_CRS

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c    |   32 +++++++++-----------------------
 arch/x86/pci/amd_bus.c |    8 ++++++--
 2 files changed, 15 insertions(+), 25 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -118,23 +118,6 @@ setup_resource(struct acpi_resource *acp
 }
 
 static void
-adjust_transparent_bridge_resources(struct pci_bus *bus)
-{
-	struct pci_dev *dev;
-
-	list_for_each_entry(dev, &bus->devices, bus_list) {
-		int i;
-		u16 class = dev->class >> 8;
-
-		if (class == PCI_CLASS_BRIDGE_PCI && dev->transparent) {
-			for(i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
-				dev->subordinate->resource[i] =
-						dev->bus->resource[i - 3];
-		}
-	}
-}
-
-static void
 get_current_resources(struct acpi_device *device, int busnum,
 			int domain, struct pci_bus *bus)
 {
@@ -161,8 +144,6 @@ get_current_resources(struct acpi_device
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, setup_resource,
 				&info);
-	if (info.res_num)
-		adjust_transparent_bridge_resources(bus);
 
 	return;
 
@@ -225,8 +206,15 @@ struct pci_bus * __devinit pci_acpi_scan
 		 */
 		memcpy(bus->sysdata, sd, sizeof(*sd));
 		kfree(sd);
-	} else
-		bus = pci_scan_bus_parented(NULL, busnum, &pci_root_ops, sd);
+	} else {
+		bus = pci_create_bus(NULL, busnum, &pci_root_ops, sd);
+		if (bus) {
+			if (pci_probe & PCI_USE_CRS)
+				get_current_resources(device, busnum, domain,
+							bus);
+			bus->subordinate = pci_scan_child_bus(bus);
+		}
+	}
 
 	if (!bus)
 		kfree(sd);
@@ -241,8 +229,6 @@ struct pci_bus * __devinit pci_acpi_scan
 #endif
 	}
 
-	if (bus && (pci_probe & PCI_USE__CRS))
-		get_current_resources(device, busnum, domain, bus);
 	return bus;
 }
 
Index: linux-2.6/arch/x86/pci/amd_bus.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/amd_bus.c
+++ linux-2.6/arch/x86/pci/amd_bus.c
@@ -100,8 +100,9 @@ void x86_pci_root_bus_res_quirks(struct
 	int j;
 	struct pci_root_info *info;
 
-	/* don't go for it if _CRS is used */
-	if (pci_probe & PCI_USE__CRS)
+	/* don't go for it if _CRS is used already */
+	if (b->resource[0] != &ioport_resource ||
+	    b->resource[1] != &iomem_resource)
 		return;
 
 	/* if only one root bus, don't need to anything */
@@ -116,6 +117,9 @@ void x86_pci_root_bus_res_quirks(struct
 	if (i == pci_root_num)
 		return;
 
+	printk(KERN_DEBUG "PCI: peer root bus %02x res updated from pci conf\n",
+			b->number);
+
 	info = &pci_root_info[i];
 	for (j = 0; j < info->res_num; j++) {
 		struct resource *res;

[PATCH 2/3] x86/pci: get root CRS before scan childs -v4

[Yinghai Lu]

so we could remove adjust_transparent_bridge_resources..

and give x86_pci_root_bus_res_quirks chance when _CRS is not used or
not there.

v2: add print out if pci conf reading for res is used for root
v3: update after revert PCI_NO_ROOT_CRS
v4: fix typo PCI_USE__CRS

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c    |   32 +++++++++-----------------------
 arch/x86/pci/amd_bus.c |    8 ++++++--
 2 files changed, 15 insertions(+), 25 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -118,23 +118,6 @@ setup_resource(struct acpi_resource *acp
 }
 
 static void
-adjust_transparent_bridge_resources(struct pci_bus *bus)
-{
-	struct pci_dev *dev;
-
-	list_for_each_entry(dev, &bus->devices, bus_list) {
-		int i;
-		u16 class = dev->class >> 8;
-
-		if (class == PCI_CLASS_BRIDGE_PCI && dev->transparent) {
-			for(i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
-				dev->subordinate->resource[i] =
-						dev->bus->resource[i - 3];
-		}
-	}
-}
-
-static void
 get_current_resources(struct acpi_device *device, int busnum,
 			int domain, struct pci_bus *bus)
 {
@@ -161,8 +144,6 @@ get_current_resources(struct acpi_device
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, setup_resource,
 				&info);
-	if (info.res_num)
-		adjust_transparent_bridge_resources(bus);
 
 	return;
 
@@ -225,8 +206,15 @@ struct pci_bus * __devinit pci_acpi_scan
 		 */
 		memcpy(bus->sysdata, sd, sizeof(*sd));
 		kfree(sd);
-	} else
-		bus = pci_scan_bus_parented(NULL, busnum, &pci_root_ops, sd);
+	} else {
+		bus = pci_create_bus(NULL, busnum, &pci_root_ops, sd);
+		if (bus) {
+			if (pci_probe & PCI_USE__CRS)
+				get_current_resources(device, busnum, domain,
+							bus);
+			bus->subordinate = pci_scan_child_bus(bus);
+		}
+	}
 
 	if (!bus)
 		kfree(sd);
@@ -241,8 +229,6 @@ struct pci_bus * __devinit pci_acpi_scan
 #endif
 	}
 
-	if (bus && (pci_probe & PCI_USE__CRS))
-		get_current_resources(device, busnum, domain, bus);
 	return bus;
 }
 
Index: linux-2.6/arch/x86/pci/amd_bus.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/amd_bus.c
+++ linux-2.6/arch/x86/pci/amd_bus.c
@@ -100,8 +100,9 @@ void x86_pci_root_bus_res_quirks(struct
 	int j;
 	struct pci_root_info *info;
 
-	/* don't go for it if _CRS is used */
-	if (pci_probe & PCI_USE__CRS)
+	/* don't go for it if _CRS is used already */
+	if (b->resource[0] != &ioport_resource ||
+	    b->resource[1] != &iomem_resource)
 		return;
 
 	/* if only one root bus, don't need to anything */
@@ -116,6 +117,9 @@ void x86_pci_root_bus_res_quirks(struct
 	if (i == pci_root_num)
 		return;
 
+	printk(KERN_DEBUG "PCI: peer root bus %02x res updated from pci conf\n",
+			b->number);
+
 	info = &pci_root_info[i];
 	for (j = 0; j < info->res_num; j++) {
 		struct resource *res;

Re: [PATCH 1/3] x86/pci: fix boundary checking when using root CRS

[Jesse Barnes]

On Wed, 24 Jun 2009 19:01:19 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> 
> don't touch info->res_num if we are out of space
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Gary, any comments on these?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 1/3] x86/pci: fix boundary checking when using root CRS

[Gary Hade]

On Mon, Jun 29, 2009 at 06:16:03PM -0700, Jesse Barnes wrote:
> On Wed, 24 Jun 2009 19:01:19 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > 
> > don't touch info->res_num if we are out of space
> > 
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> Gary, any comments on these?

Jesse,
"[PATCH 1/3] x86/pci: fix boundary checking when using root CRS"
which convinced that my code was incorrect and
"[PATCH 2/3] x86/pci: get root CRS before scan childs -v3"
look good to me from both code review and functional [1] standpoints.
  Acked-by: <garyhade@us.ibm.com>
  Tested-by: <garyhade@us.ibm.com>

[1] Successful boot of patched 2.6.31-rc1 with pci=use_crs
    followed by successful hot-add and hot-remove of PCI-X
    and PCIe cards on an IBM x3850.

Thanks Yinghai.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH 1/3] x86/pci: fix boundary checking when using root CRS

[Jesse Barnes]

On Tue, 30 Jun 2009 11:04:49 -0700
Gary Hade <garyhade@us.ibm.com> wrote:

> On Mon, Jun 29, 2009 at 06:16:03PM -0700, Jesse Barnes wrote:
> > On Wed, 24 Jun 2009 19:01:19 -0700
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> > > 
> > > don't touch info->res_num if we are out of space
> > > 
> > > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > Gary, any comments on these?
> 
> Jesse,
> "[PATCH 1/3] x86/pci: fix boundary checking when using root CRS"
> which convinced that my code was incorrect and
> "[PATCH 2/3] x86/pci: get root CRS before scan childs -v3"
> look good to me from both code review and functional [1] standpoints.
>   Acked-by: <garyhade@us.ibm.com>
>   Tested-by: <garyhade@us.ibm.com>
> 
> [1] Successful boot of patched 2.6.31-rc1 with pci=use_crs
>     followed by successful hot-add and hot-remove of PCI-X
>     and PCIe cards on an IBM x3850.
> 
> Thanks Yinghai.

Thanks guys, I've applied these to my for-linus branch for a pull later
this week.

-- 
Jesse Barnes, Intel Open Source Technology Center