Skip to contentSkip to conversation

Collecting host bridge resources - take 2

PATCH In review [email protected] 4 participants 8 messages Jun 2, 2005 – Jul 1, 2005

Subsystems (2)
ACPI PCI
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover Collecting host bridge resources - take 2

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

author
#1 lore raw

PATCH 1/2 Increase the number of PCI bus resources

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 <[email protected]>
include/linux/pci.h +1 -1
Index: 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 */

--

author
#2 lore raw

PATCH 2/2 i386/x86_64: collect host bridge resources v2

In reply to [email protected]

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 <[email protected]>
arch/i386/pci/acpi.c +152 -1
Index: 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;

--

#3 lore raw

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

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

Quoted 3 lines · On Thu, Jun 02, 2005 at 03:41:49PM -0700, [email protected] 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.

arch/i386/pci/acpi.c +44 -2
@@ -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);
 	}
 }
#4 lore raw

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

Quoted 23 lines · Ivan Kokshaysky

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

Quoted 3 lines · On Thu, Jun 02, 2005 at 03:41:49PM -0700, [email protected] 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.

Quoted 3 lines

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

Quoted 73 lines

Ivan.

arch/i386/pci/acpi.c +44 -2
@@ -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);
 	}
 }
#5 lore raw

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

Quoted 5 lines · Kristen Accardi

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.

-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

#6 lore raw

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

Quoted 7 lines · Ivan Kokshaysky
Quoted 5 lines · Kristen Accardi

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.

Quoted 6 lines

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

-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

author
#7 lore raw

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

Quoted 9 lines · Greg KH
Quoted 6 lines

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