[patch 0/2] Collecting host bridge resources - take 2

[patch 0/2] Collecting host bridge resources - take 2

[rajesh.shah]

ACPI hotplug code now uses the PCI core to allocate and manage
resources for hot-plug devices. To work correctly, this requires
all bridges to report resources they are decoding in their
pci_bus structure. We already do this for PCI-PCI bridges, but
not for host bridges. This patchset reads and stores host bridge
resources reported by ACPI BIOS for i386 and x86_64 systems.

This is the 2nd version of this patchset. The major change is
that it increases the number of resource pointers in the pci_bus
structure and eliminates the need to have a boot time parameter
to enable the code.

Andrew, if you add this to the next -mm, please also drop the
previous version this patchset (i386-collect-host-bridge-resources.patch
and x86_64-collect-host-bridge-resources.patch).

Rajesh
--

[patch 1/2] Increase the number of PCI bus resources

[rajesh.shah]

[-- Attachment #1: inc-pci-bus-resources --]
[-- Type: text/plain, Size: 893 bytes --]

This patch increases the number of resource pointers in the
pci_bus structure. This is needed to store >4 resource ranges
for host bridges and transparent PCI bridges. With this change,
all PCI buses will have more resource pointers, but most PCI
buses will only use the first 3 or 4, the remaining being NULL.
The PCI core already deals with this correctly.

Signed-off-by: Rajesh Shah <rajesh.shah@intel.com>

Index: linux-2.6.12-rc5/include/linux/pci.h
===================================================================
--- linux-2.6.12-rc5.orig/include/linux/pci.h
+++ linux-2.6.12-rc5/include/linux/pci.h
@@ -586,7 +586,7 @@ struct pci_dev {
 #define PCI_NUM_RESOURCES 11
 
 #ifndef PCI_BUS_NUM_RESOURCES
-#define PCI_BUS_NUM_RESOURCES 4
+#define PCI_BUS_NUM_RESOURCES 8
 #endif
   
 #define PCI_REGION_FLAG_MASK 0x0fU	/* These bits of resource flags tell us the PCI region flags */

--

[patch 2/2] i386/x86_64: collect host bridge resources v2

[rajesh.shah]

[-- Attachment #1: i386-host-bridge-resources-v2 --]
[-- Type: text/plain, Size: 4889 bytes --]

This patch reads and stores host bridge resources reported by
ACPI BIOS for i386 and x86_64 systems.  This is needed since
ACPI hotplug code now uses the PCI core for resource management. 

Signed-off-by: Rajesh Shah <rajesh.shah@intel.com>

Index: linux-2.6.12-rc5/arch/i386/pci/acpi.c
===================================================================
--- linux-2.6.12-rc5.orig/arch/i386/pci/acpi.c
+++ linux-2.6.12-rc5/arch/i386/pci/acpi.c
@@ -5,14 +5,165 @@
 #include <asm/hw_irq.h>
 #include "pci.h"
 
+static void inline
+add_resource_range(int idx, struct pci_bus *bus,
+		struct acpi_resource_address64 *addr, unsigned long flags)
+{
+	memset(bus->resource[idx], 0, sizeof(*(bus->resource[idx])));
+	bus->resource[idx]->name = bus->name;
+	bus->resource[idx]->start = addr->min_address_range;
+	bus->resource[idx]->end = addr->max_address_range;
+	bus->resource[idx]->flags = flags;
+}
+
+static acpi_status
+add_resources(struct acpi_resource *acpi_res, void *context)
+{
+	struct acpi_resource_address64 address;
+	struct pci_bus *bus = context;
+	int idx;
+	unsigned long flags = 0;
+	struct resource *busr;
+
+	if (acpi_res->id != ACPI_RSTYPE_ADDRESS16 &&
+	    acpi_res->id != ACPI_RSTYPE_ADDRESS32 &&
+	    acpi_res->id != ACPI_RSTYPE_ADDRESS64)
+		return AE_OK;
+
+	if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &address)))
+		return AE_OK;
+
+	/*
+	 * Per the ACPI spec, we should pick up only ACPI_PRODUCER type
+	 * resources. However, many BIOSs get this wrong and report
+	 * resources they pass down as ACPI_CONSUMER type resources. For now,
+	 * pick up all resources here.
+	 */
+	if (address.address_length <= 0)
+		return AE_OK;
+
+	switch (address.resource_type) {
+		case ACPI_IO_RANGE:
+			flags = IORESOURCE_IO;
+			break;
+		case ACPI_MEMORY_RANGE:
+			flags = IORESOURCE_MEM;
+			if (address.attribute.memory.cache_attribute ==
+					ACPI_PREFETCHABLE_MEMORY)
+				flags |= IORESOURCE_PREFETCH;
+			break;
+		default:
+			return AE_OK;
+			break;
+	}
+	for (idx = 0; idx < PCI_BUS_NUM_RESOURCES; idx++) {
+		if (!bus->resource[idx]) {
+			bus->resource[idx] = kmalloc(
+					sizeof(*(bus->resource[idx])),
+					GFP_KERNEL);
+			if (!bus->resource[idx])
+				break;
+			add_resource_range(idx, bus, &address, flags);
+			return AE_OK;
+		}
+		busr = bus->resource[idx];
+		if (busr->flags != flags)
+			continue;
+		/* Consolidate adjacent resource ranges */
+		if (busr->end + 1 == address.min_address_range) {
+			busr->end = address.max_address_range;
+			return AE_OK;
+		}
+		if (address.max_address_range + 1 == busr->start) {
+			busr->start = address.min_address_range;
+			return AE_OK;
+		}
+		/* Consolidate overlapping resource ranges */
+		if (address.max_address_range < busr->start)
+			continue;
+		if (address.min_address_range > busr->end)
+			continue;
+		if (address.min_address_range < busr->start)
+			busr->start = address.min_address_range;
+		if (address.max_address_range > busr->end)
+			busr->end = address.max_address_range;
+		return AE_OK;
+	}
+	printk(KERN_WARNING
+		"PCI: Cannot add host bridge range %Lx-%Lx, type %x\n",
+		address.min_address_range, address.max_address_range,
+		address.resource_type);
+	return AE_ERROR;
+}
+
+static int __devinit
+verify_root_windows(struct pci_bus *bus)
+{
+	int i, num_io = 0, num_mem = 0;
+	int type_mask = IORESOURCE_IO | IORESOURCE_MEM;
+
+	for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
+		if (!bus->resource[i])
+			continue;
+		switch (bus->resource[i]->flags & type_mask) {
+			case IORESOURCE_IO:
+				num_io++;
+				break;
+			case IORESOURCE_MEM:
+				num_mem++;
+				break;
+			default:
+				break;
+		}
+	}
+
+	if (num_io || num_mem)
+		return 1;
+	else
+		printk(KERN_WARNING
+			"PCI: BIOS reported bogus host bridge resources\n");
+	return 0;
+}
+
+static void __devinit
+pcibios_setup_root_windows(struct pci_bus *bus, acpi_handle handle)
+{
+	int i;
+	acpi_status status;
+	struct resource *bres[PCI_BUS_NUM_RESOURCES];
+
+	for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
+		bres[i] = bus->resource[i];
+		bus->resource[i] = NULL;
+	}
+
+	sprintf(bus->name, "PCI Bus #%02x", bus->number);
+	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
+			add_resources, bus);
+	if (ACPI_FAILURE(status) || !verify_root_windows(bus)) {
+		printk(KERN_WARNING
+			"PCI: Falling back to default host bridge resources\n");
+		for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
+			kfree(bus->resource[i]);
+			bus->resource[i] = bres[i];
+		}
+	}
+}
+
+
 struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int domain, int busnum)
 {
+	struct pci_bus *bus;
+
 	if (domain != 0) {
 		printk(KERN_WARNING "PCI: Multiple domains not supported\n");
 		return NULL;
 	}
 
-	return pcibios_scan_root(busnum);
+	bus =  pcibios_scan_root(busnum);
+	if (bus) 
+		pcibios_setup_root_windows(bus, device->handle);
+	return bus;
 }
 
 extern int pci_routeirq;

--

Re: [patch 2/2] i386/x86_64: collect host bridge resources v2

[Ivan Kokshaysky]

[This refers to "gregkh-pci-pci-collect-host-bridge-resources-02.patch"
which went into 2.6.12-mm1 and -mm2]

On Thu, Jun 02, 2005 at 03:41:49PM -0700, rajesh.shah@intel.com wrote:
> This patch reads and stores host bridge resources reported by
> ACPI BIOS for i386 and x86_64 systems.  This is needed since
> ACPI hotplug code now uses the PCI core for resource management. 

That patch introduces two major problems.

1. The new root bus resources aren't properly inserted into global
   resource tree (missing request_resource calls). This leads to
   completely messed up PCI setup, especially with
   pci_assign_unassigned_resources() call, as seen in -mm1 (mm2 is
   also unsafe, though).

2. Transparent bridge handling is broken, no matter is it old code
   in Linus' tree or the new one in -mm. At the point then
   pcibios_setup_root_windows() is called, the "transparent"
   resource pointers have been already set up in pci_read_bridge_bases()
   (typically to ioport_resource and iomem_resource), so after changing
   the root bus windows these pointers are wrong.

The appended patch fixes that. Just compare /proc/ioports and /proc/iomem
with and without it. ;-)

Ivan.

--- 2.6.12-mm2/arch/i386/pci/acpi.c	2005-06-28 13:30:24.000000000 +0400
+++ linux/arch/i386/pci/acpi.c	2005-06-28 14:13:51.000000000 +0400
@@ -107,10 +107,22 @@ verify_root_windows(struct pci_bus *bus)
 			continue;
 		switch (bus->resource[i]->flags & type_mask) {
 			case IORESOURCE_IO:
-				num_io++;
+				if (!request_resource(&ioport_resource,
+						      bus->resource[i]))
+					num_io++;
+				else {
+					kfree(bus->resource[i]);
+					bus->resource[i] = NULL;
+				}
 				break;
 			case IORESOURCE_MEM:
-				num_mem++;
+				if (!request_resource(&iomem_resource,
+						      bus->resource[i]))
+					num_mem++;
+				else {
+					kfree(bus->resource[i]);
+					bus->resource[i] = NULL;
+				}
 				break;
 			default:
 				break;
@@ -126,6 +138,21 @@ verify_root_windows(struct pci_bus *bus)
 }
 
 static void __devinit
+fixup_transparent_bridges(struct list_head *bus_list)
+{
+	int i;
+	struct pci_bus *b;
+
+	list_for_each_entry(b, bus_list, node) {
+		if (b->self && b->self->transparent) {
+			for (i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
+				b->resource[i] = b->parent->resource[i - 3];
+		}
+		fixup_transparent_bridges(&b->children);
+	}
+}
+
+static void __devinit
 pcibios_setup_root_windows(struct pci_bus *bus, acpi_handle handle)
 {
 	int i;
@@ -147,6 +174,21 @@ pcibios_setup_root_windows(struct pci_bu
 			kfree(bus->resource[i]);
 			bus->resource[i] = bres[i];
 		}
+	} else {
+		/* Squeeze out unused resource pointers. */
+		int idx = 0;
+
+		for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
+			if (bus->resource[i])
+				bus->resource[idx++] = bus->resource[i];
+		}
+		for (; idx < PCI_BUS_NUM_RESOURCES; idx++)
+			bus->resource[idx] = NULL;
+
+		/* The root bus resources have been changed. Fix up the
+		   resource pointers of buses behind "transparent" bridges
+		   as well. */
+		fixup_transparent_bridges(&bus->children);
 	}
 }
 

Re: [patch 2/2] i386/x86_64: collect host bridge resources v2

[Kristen Accardi]

On Tue, 2005-06-28 at 15:51 +0400, Ivan Kokshaysky wrote:
> [This refers to "gregkh-pci-pci-collect-host-bridge-resources-02.patch"
> which went into 2.6.12-mm1 and -mm2]
> 
> On Thu, Jun 02, 2005 at 03:41:49PM -0700, rajesh.shah@intel.com wrote:
> > This patch reads and stores host bridge resources reported by
> > ACPI BIOS for i386 and x86_64 systems.  This is needed since
> > ACPI hotplug code now uses the PCI core for resource management. 
> 
> That patch introduces two major problems.
> 
> 1. The new root bus resources aren't properly inserted into global
>    resource tree (missing request_resource calls). This leads to
>    completely messed up PCI setup, especially with
>    pci_assign_unassigned_resources() call, as seen in -mm1 (mm2 is
>    also unsafe, though).
> 
> 2. Transparent bridge handling is broken, no matter is it old code
>    in Linus' tree or the new one in -mm. At the point then
>    pcibios_setup_root_windows() is called, the "transparent"
>    resource pointers have been already set up in pci_read_bridge_bases()
>    (typically to ioport_resource and iomem_resource), so after changing
>    the root bus windows these pointers are wrong.
> 
> The appended patch fixes that. Just compare /proc/ioports and /proc/iomem
> with and without it. ;-)
> 


I gave this patch a try (against mm2), and found that I now get many
errors on boot up complaining about not being able to allocate PCI
resources due to resource collisions, and then the system begins to
complain about lost interrupts on hda, and is never able to mount the
root filesystem.
Kristen


> Ivan.
> 
> --- 2.6.12-mm2/arch/i386/pci/acpi.c	2005-06-28 13:30:24.000000000 +0400
> +++ linux/arch/i386/pci/acpi.c	2005-06-28 14:13:51.000000000 +0400
> @@ -107,10 +107,22 @@ verify_root_windows(struct pci_bus *bus)
>  			continue;
>  		switch (bus->resource[i]->flags & type_mask) {
>  			case IORESOURCE_IO:
> -				num_io++;
> +				if (!request_resource(&ioport_resource,
> +						      bus->resource[i]))
> +					num_io++;
> +				else {
> +					kfree(bus->resource[i]);
> +					bus->resource[i] = NULL;
> +				}
>  				break;
>  			case IORESOURCE_MEM:
> -				num_mem++;
> +				if (!request_resource(&iomem_resource,
> +						      bus->resource[i]))
> +					num_mem++;
> +				else {
> +					kfree(bus->resource[i]);
> +					bus->resource[i] = NULL;
> +				}
>  				break;
>  			default:
>  				break;
> @@ -126,6 +138,21 @@ verify_root_windows(struct pci_bus *bus)
>  }
>  
>  static void __devinit
> +fixup_transparent_bridges(struct list_head *bus_list)
> +{
> +	int i;
> +	struct pci_bus *b;
> +
> +	list_for_each_entry(b, bus_list, node) {
> +		if (b->self && b->self->transparent) {
> +			for (i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
> +				b->resource[i] = b->parent->resource[i - 3];
> +		}
> +		fixup_transparent_bridges(&b->children);
> +	}
> +}
> +
> +static void __devinit
>  pcibios_setup_root_windows(struct pci_bus *bus, acpi_handle handle)
>  {
>  	int i;
> @@ -147,6 +174,21 @@ pcibios_setup_root_windows(struct pci_bu
>  			kfree(bus->resource[i]);
>  			bus->resource[i] = bres[i];
>  		}
> +	} else {
> +		/* Squeeze out unused resource pointers. */
> +		int idx = 0;
> +
> +		for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
> +			if (bus->resource[i])
> +				bus->resource[idx++] = bus->resource[i];
> +		}
> +		for (; idx < PCI_BUS_NUM_RESOURCES; idx++)
> +			bus->resource[idx] = NULL;
> +
> +		/* The root bus resources have been changed. Fix up the
> +		   resource pointers of buses behind "transparent" bridges
> +		   as well. */
> +		fixup_transparent_bridges(&bus->children);
>  	}
>  }
>  

Re: [patch 2/2] i386/x86_64: collect host bridge resources v2

[Ivan Kokshaysky]

On Tue, Jun 28, 2005 at 11:21:54AM -0700, Kristen Accardi wrote:
> I gave this patch a try (against mm2), and found that I now get many
> errors on boot up complaining about not being able to allocate PCI
> resources due to resource collisions, and then the system begins to
> complain about lost interrupts on hda, and is never able to mount the
> root filesystem.

Well, I'm not surprised. :-(
Probably there is a conflict between e820 map and root bus ranges
reported by ACPI. I think that it would be better to just drop
gregkh-pci-pci-collect-host-bridge-resources-02.patch rather than
try to fix it, at least until such conflicts can be resolved in
a sane way.

Ivan.

Re: [patch 2/2] i386/x86_64: collect host bridge resources v2

[Greg KH]

On Wed, Jun 29, 2005 at 12:03:00AM +0400, Ivan Kokshaysky wrote:
> On Tue, Jun 28, 2005 at 11:21:54AM -0700, Kristen Accardi wrote:
> > I gave this patch a try (against mm2), and found that I now get many
> > errors on boot up complaining about not being able to allocate PCI
> > resources due to resource collisions, and then the system begins to
> > complain about lost interrupts on hda, and is never able to mount the
> > root filesystem.
> 
> Well, I'm not surprised. :-(
> Probably there is a conflict between e820 map and root bus ranges
> reported by ACPI. I think that it would be better to just drop
> gregkh-pci-pci-collect-host-bridge-resources-02.patch rather than
> try to fix it, at least until such conflicts can be resolved in
> a sane way.

Ok, I'll drop it.  Any objections to me doing this?

So, with the remaining pci patches, (as seen in
kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/pci/) are
there any objections to me pushing these (with the exception of the
above one) to Linus?  I think there was one report of the
pci-assign-unassigned-resources.patch patch causing boot problems on one
box, but that might have also been due to the above patch, am not sure.

Ah, the joys of acpi and pci resources... bleah.

thanks,

greg k-h

Re: [patch 2/2] i386/x86_64: collect host bridge resources v2

[Rajesh Shah]

On Thu, Jun 30, 2005 at 09:05:06AM -0700, Greg KH wrote:
> > 
> > Probably there is a conflict between e820 map and root bus ranges
> > reported by ACPI. I think that it would be better to just drop
> > gregkh-pci-pci-collect-host-bridge-resources-02.patch rather than
> > try to fix it, at least until such conflicts can be resolved in
> > a sane way.
> 
> Ok, I'll drop it.  Any objections to me doing this?
> 
OK with me. I actually think there are interactions between other
patches that may also be the cause for some of the problems
reported recently. However, I'm mostly out for the next 2
weeks, and will look at this more closely when I return.

Rajesh