PCI: override BIOS/firmware resource allocation

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

The following patch provides the ability to override the BIOS
allocations using user-hints provided through kernel
command-line parameters.

The patch is based upon an idea by Linus, details of which can be found at
http://marc.info/?l=linux-kernel&m=127846075028242&w=2

Change log:
PCI Bridge window reset is done early just after
early_dump_pci_devices() (Suggested by Yinghai)
command line parameters processing is moved from pci_setup() to
pcibios_setup() (Suggested by Yinghai)

Documentation/kernel-parameters.txt +12 -0

index 8dd7248..99ee10e 100644
@@ -1961,6 +1961,18 @@ and is between 256 and 4096 characters. It is defined in the file
 				PAGE_SIZE is used as alignment.
 				PCI-PCI bridge can be specified, if resource
 				windows need to be expanded.
+             	override=[off, conflict, always, <device>]
+                             	off : Do not override BIOS/firmware memory resource
+					allocations. This is the default.
+                             	conflict : override BIOS/firmware memory resource
+					allocations if conflicting or not allocated.
+                             	always : override all BIOS/firmware memory resource
+					allocations.
+                             	<device>: Format [<domain>:]<bus>:<slot>.<func>[; ...]
+                                     override BIOS/firmware memory resource allocations
+				     of specified devices. If the device is a bridge
+				     override allocations of all devices under the
+				     bridge.
 		ecrc=		Enable/disable PCIe ECRC (transaction layer
 				end-to-end CRC checking).
 				bios: Use BIOS/firmware settings. This is the

arch/x86/include/asm/pci-direct.h +1 -0

index b1e7a45..3a2c439 100644
@@ -18,4 +18,5 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+extern void early_reset_pci_devices(void);
 #endif /* _ASM_X86_PCI_DIRECT_H */

arch/x86/kernel/setup.c +1 -0

index c3a4fbb..956d7ac 100644
@@ -855,6 +855,7 @@ void __init setup_arch(char **cmdline_p)
 	if (pci_early_dump_regs)
 		early_dump_pci_devices();
 #endif
+	early_reset_pci_devices();
 
 	finish_e820_parsing();
 

arch/x86/pci/common.c +63 -0

index a0772af..b26f782 100644
@@ -453,6 +453,66 @@ int __init pcibios_init(void)
 	return 0;
 }
 
+#define RESOURCE_RELEASE_PARAM_SIZE 256
+static char __initdata resource_release_param[RESOURCE_RELEASE_PARAM_SIZE] = {0};
+int pci_override=PCI_OVERRIDE_OFF;
+
+char *next_pci_device(int *bus, int *slot, int *func, char *p)
+{
+	int seg, count;
+
+	if ( !p )
+		p = resource_release_param;
+
+	if ( !*p )
+		return NULL;
+
+	count = 0;
+	if (sscanf(p, "%x:%x:%x.%x%n",
+		&seg, bus, slot, func, &count) != 4) {
+		seg = 0;
+		if (sscanf(p, "%x:%x.%x%n",
+				bus, slot, func, &count) != 3) {
+			/* Invalid format */
+			printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
+				p);
+			return NULL;
+		}
+	}
+	p += count;
+	if (!*p)
+		return p;
+	if (*p == ';')
+		return ++p;
+	return NULL;
+}
+
+
+void pci_override_setup(const char *str, int override)
+{
+	int count;
+	if (override && !strncmp(str, "off", 3)) {
+		pci_override = PCI_OVERRIDE_OFF;
+		printk(KERN_INFO "pci: do not override BIOS/uEFI memory allocations\n");
+	} else if (override && !strncmp(str, "conflict", 8)) {
+		pci_override = PCI_OVERRIDE_CONFLICT;
+		printk(KERN_INFO "pci: reallocate BIOS/uEFI allocated memory resource conflicts\n");
+	} else if (override && !strncmp(str, "always", 6)) {
+		pci_override =  PCI_OVERRIDE_ALWAYS;
+		printk(KERN_INFO "pci: override all BIOS/uEFI memory resource allocations\n");
+	} else {
+		pci_override =  PCI_OVERRIDE_DEVICE;
+		count=strlen(str);
+		if (count > RESOURCE_RELEASE_PARAM_SIZE - 1)
+			count = RESOURCE_RELEASE_PARAM_SIZE - 1;
+		strncpy(resource_release_param, str, count);
+		resource_release_param[count] = '\0';
+		printk(KERN_INFO "pci: override BIOS/uEFI memory allocations for %s\n",
+			   resource_release_param);
+	}
+	return;
+}
+
 char * __devinit  pcibios_setup(char *str)
 {
 	if (!strcmp(str, "off")) {

@@ -558,6 +618,9 @@ char * __devinit  pcibios_setup(char *str)
 		if (noioapicreroute != -1)
 			noioapicreroute = 1;
 		return NULL;
+	} else if (!strncmp(str, "override=", 9)) {
+		pci_override_setup(str + 9, 1);
+		return NULL;
 	}
 	return str;
 }

arch/x86/pci/early.c +98 -0

index d1067d5..7a30a87 100644
@@ -109,3 +109,101 @@ void early_dump_pci_devices(void)
 		}
 	}
 }
+
+static void __reset_bridge_window(u8 bus, u8 slot, u8 func)
+{
+	/* reset mem resources */
+	write_pci_config_16(bus, slot, func, PCI_MEMORY_BASE, 0x00);
+
+	/* reset pref mem resources */
+	write_pci_config(bus, slot, func, PCI_PREF_LIMIT_UPPER32, 0x00);
+	write_pci_config(bus, slot, func, PCI_PREF_MEMORY_BASE, 0x00);
+	write_pci_config(bus, slot, func, PCI_PREF_BASE_UPPER32, 0x00);
+	write_pci_config(bus, slot, func, PCI_PREF_LIMIT_UPPER32, 0x00);
+}
+
+
+int find_parent_bridge(u8 *bus, u8 *slot, u8 *func, u8 secondary_bus)
+{
+   u8 tbus, tslot, tfunc;
+	for (tbus = secondary_bus-1; tbus >= 0; tbus--) {
+ 	 for (tslot = 0; tslot < 32; tslot++) {
+	  for (tfunc = 0; tfunc < 8; tfunc++) {
+		if (0xffffffff == read_pci_config(tbus,
+			    tslot, tfunc, PCI_CLASS_REVISION))
+			continue;
+
+		if (read_pci_config_byte(tbus, tslot, tfunc,
+			PCI_HEADER_TYPE) != PCI_HEADER_TYPE_BRIDGE)
+			continue;
+
+		if (secondary_bus == read_pci_config_byte(tbus,
+			  tslot, tfunc, PCI_SECONDARY_BUS)) {
+			*bus=tbus;
+			*slot=tslot;
+			*func=tfunc;
+			return 1;
+		}
+	  }
+         }
+	}
+	return 0;
+}
+
+void reset_bridge_window(u8 bus, u8 slot, u8 func)
+{
+	u8 tbus, tslot, tfunc;
+	if (read_pci_config_byte(bus, slot, func, PCI_HEADER_TYPE) ==
+			PCI_HEADER_TYPE_BRIDGE) {
+		__reset_bridge_window(bus, slot, func);
+	} else if (find_parent_bridge(&tbus, &tslot, &tfunc, bus))
+		__reset_bridge_window(tbus, tslot, tfunc);
+	return;
+}
+
+void reset_topmost_bridges(void)
+{
+	u8 slot, func;
+
+	/* topmost bridges as assumed to be on bus 0. good assumption? */
+	for (slot = 0; slot < 32; slot++) {
+	 for (func = 0; func < 8; func++) {
+		if (0xffffffff == read_pci_config(0, slot, func, PCI_CLASS_REVISION))
+			continue;
+
+		if (read_pci_config_byte(0, slot, func, PCI_HEADER_TYPE) ==
+				PCI_HEADER_TYPE_BRIDGE) {
+			__reset_bridge_window(0, slot, func);
+		}
+	 }
+	}
+	return;
+}
+
+extern char * next_pci_device(int *bus, int *slot, int *func, char *pr);
+extern int pci_override;
+
+void early_reset_pci_devices(void)
+{
+	unsigned bus, slot, func;
+	char *ptr=NULL;
+
+	if (!early_pci_allowed())
+		return;
+
+	if ( pci_override == PCI_OVERRIDE_OFF )
+		return;
+
+	if ( pci_override == PCI_OVERRIDE_ALWAYS ) {
+		reset_topmost_bridges();
+		return;
+	}
+
+	while((ptr = next_pci_device(&bus, &slot, &func, ptr))) {
+		if (0xffffffff == read_pci_config(bus, slot,
+				func, PCI_CLASS_REVISION))
+			continue;
+		reset_bridge_window(bus, slot, func);
+	}
+	return;
+}

drivers/pci/setup-bus.c +65 -11

index 66cb8f4..981e701 100644
@@ -768,6 +768,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	}
 }
 
+
 enum release_type {
 	leaf_only,
 	whole_subtree,

@@ -808,6 +809,7 @@ static void __ref pci_bus_release_bridge_resources(struct pci_bus *bus,
 		pci_bridge_release_resources(bus, type);
 }
 
+
 static void pci_bus_dump_res(struct pci_bus *bus)
 {
 	struct resource *res;

@@ -838,26 +840,78 @@ static void pci_bus_dump_resources(struct pci_bus *bus)
 	}
 }
 
+static void __init
+pci_release_resources(struct resource_list_x *head)
+{
+	struct resource_list_x *list;
+	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+				  IORESOURCE_PREFETCH;
+	enum release_type rel_type = whole_subtree;
+	/*
+	 * Try to release leaf bridge's resources that doesn't fit resource of
+	 * child device under that bridge
+	 */
+	for (list = head->next; list;) {
+		struct pci_bus *bus =  list->dev->bus;
+		pci_bus_release_bridge_resources(bus, list->flags & type_mask,
+						  rel_type);
+		list = list->next;
+ 	}
+	/* restore size and flags */
+	for (list = head->next; list;) {
+		struct resource *res = list->res;
+
+		res->start = list->start;
+		res->end = list->end;
+		res->flags = list->flags;
+		if (list->dev->subordinate)
+			res->flags = 0;
+
+		list = list->next;
+	}
+	free_failed_list(head);
+}
+
 void __init
 pci_assign_unassigned_resources(void)
 {
 	struct pci_bus *bus;
+	int tried_times = 0;
+	struct resource_list_x head;
+
+ 	head.next = NULL;
+	do { /* loop at most 2 times */
+		/* Depth first, calculate sizes and alignments of all
+		   subordinate buses. */
+		list_for_each_entry(bus, &pci_root_buses, node) {
+			pci_bus_size_bridges(bus);
+		}
+		/* Depth last, allocate resources and update the hardware. */
+		list_for_each_entry(bus, &pci_root_buses, node) {
+			__pci_bus_assign_resources(bus, &head);
+		}
+		/* any device complain? */
+		if (!head.next)
+			goto enable_and_dump;
+
+		/* do we care if any device complained? */
+		if (pci_override != PCI_OVERRIDE_CONFLICT) {
+			free_failed_list(&head);
+			goto enable_and_dump;
+		}
+		printk(KERN_INFO "PCI: No. %d try to assign unassigned res\n",
+			tried_times+1);
 
-	/* Depth first, calculate sizes and alignments of all
-	   subordinate buses. */
-	list_for_each_entry(bus, &pci_root_buses, node) {
-		pci_bus_size_bridges(bus);
-	}
-	/* Depth last, allocate resources and update the hardware. */
-	list_for_each_entry(bus, &pci_root_buses, node) {
-		pci_bus_assign_resources(bus);
+		pci_release_resources(&head);
+	} while (!tried_times++);
+
+enable_and_dump:
+	list_for_each_entry(bus, &pci_root_buses, node)
 		pci_enable_bridges(bus);
-	}
 
 	/* dump the resource on buses */
-	list_for_each_entry(bus, &pci_root_buses, node) {
+	list_for_each_entry(bus, &pci_root_buses, node)
 		pci_bus_dump_resources(bus);
-	}
 }
 
 void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)

include/linux/pci.h +6 -0

index c8d95e3..3449297 100644
@@ -1360,6 +1360,12 @@ extern u8 pci_cache_line_size;
 extern unsigned long pci_hotplug_io_size;
 extern unsigned long pci_hotplug_mem_size;
 
+extern int pci_override;
+#define  PCI_OVERRIDE_OFF	     1
+#define  PCI_OVERRIDE_CONFLICT	     2
+#define  PCI_OVERRIDE_ALWAYS	     3
+#define  PCI_OVERRIDE_DEVICE	     4
+
 int pcibios_add_platform_entries(struct pci_dev *dev);
 void pcibios_disable_device(struct pci_dev *dev);
 int pcibios_set_pcie_reset_state(struct pci_dev *dev,

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

The choice is between
(a) an automated patch with the risk of regressing some platforms.
(b) an semi-automated patch that does not regress *any* platform, with the ability to fix platforms that are currently broken.
(c) status quo, which means broken platforms continue to be so.

I thought the initial proposal was to use (b), with the long term goal of fixing it automatically, assuming that it is even possible.

Let me know if that is *not* the goal and I will change directions.

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

I think what would be more interesting is to look at a log from *your* system, where you have SRIOV devices that don't have resources assigned, and see whether we have enough information to make the minimal changes required to assign resources to them. If we can come up with a strategy that only does something when absolutely required, and does it a little more carefully, I think we have a decent chance of success.

Quoted 10 lines

The choice is between
(a) an automated patch with the risk of regressing some platforms.
(b) an semi-automated patch that does not regress *any* platform, with the ability to fix platforms that are currently broken.
(c) status quo, which means broken platforms continue to be so.

I thought the initial proposal was to use (b), with the long term goal of fixing it automatically, assuming that it is even possible.

Quoted 1 line

Let me know if that is *not* the goal and I will change directions.

*My* goal is that a user would never need a kernel option except to help debug kernel problems. I think of an option like "pci=override" as a band-aid that covers up a kernel problem without really fixing it, so I guess my choice would be (a). Yes, there's a risk of regression, and we have to do everything we can to avoid it. But the result is a more usable system.

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Quoted 9 lines

Quoted 9 lines

The choice is between
(a) an automated patch with the risk of regressing some platforms.
(b) an semi-automated patch that does not regress *any* platform, with the ability to fix platforms that are currently broken.
(c) status quo, which means broken platforms continue to be so.

I thought the initial proposal was to use (b), with the long term goal of fixing it automatically, assuming that it is even possible.

Quoted 8 lines

Quoted 1 line

Let me know if that is *not* the goal and I will change directions.

*My* goal is that a user would never need a kernel option except to help debug kernel problems. I think of an option like "pci=override" as a band-aid that covers up a kernel problem without really fixing it, so I guess my choice would be (a). Yes, there's a risk of regression, and we have to do everything we can to avoid it. But the result is a more usable system.

Ok. I think we agree with your goal, but the disagreement is on the approach. You want to take one big leap, whereas the consensus I heard in earlier threads was that we need to take baby steps.

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Quoted 77 lines · Bjorn Helgaas

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

Quoted 8 lines

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Actually the message preceeding to them are even more surprising:

Apr 20 20:31:42 [kernel] pci 0000:04:00.0: BAR 8: can't assign mem (size
0xc00000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 8: can't assign mem (size
0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 9: can't assign mem pref
(size 0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:02.0: BAR 8: can't assign mem (size
0x400000)
Apr 20 20:31:42 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size

0x1000)

Do these bridges have IOV BARs and those BARs are demanding i/o resources? Something is really funny with this machine. Or I am reading this wrong?

Quoted 98 lines · Ram Pai

Quoted 77 lines · Bjorn Helgaas

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

Quoted 8 lines

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Actually the message preceeding to them are even more surprising:

Apr 20 20:31:42 [kernel] pci 0000:04:00.0: BAR 8: can't assign mem (size
0xc00000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 8: can't assign mem (size
0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 9: can't assign mem pref
(size 0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:02.0: BAR 8: can't assign mem (size
0x400000)
Apr 20 20:31:42 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size

Quoted 4 lines

0x1000)

Do these bridges have IOV BARs and those BARs are demanding i/o resources? Something is really funny with this machine. Or I am reading this wrong?

I don't see anything strange about the machine, but I don't understand what Yinghai's patch was doing. The lspci output is here:

    http://marc.info/?l=linux-pci&m=127184080929189&w=2

I don't see any SRIOV devices, and this kernel wasn't built with IOV support anyway. The messages above are attempts to assign resources for bridge windows.

But the only real device in the whole hierarchy behind 00:1c.0 is 09:04.0, which doesn't require memory space, so I have no idea why we're trying to open memory windows.

This is what I meant about "we haven't tried hard enough yet." I'd rather spend time fixing the problems like this, than just putting the buggy code in and adding a kernel parameter to enable it. That guarantees that we'll never get enough testing to really shake it out.

Quoted 108 lines · Bjorn Helgaas

Quoted 98 lines · Ram Pai

Quoted 77 lines · Bjorn Helgaas

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

Quoted 8 lines

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Actually the message preceeding to them are even more surprising:

Apr 20 20:31:42 [kernel] pci 0000:04:00.0: BAR 8: can't assign mem (size
0xc00000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 8: can't assign mem (size
0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 9: can't assign mem pref
(size 0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:02.0: BAR 8: can't assign mem (size
0x400000)
Apr 20 20:31:42 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size

Quoted 4 lines

0x1000)

Do these bridges have IOV BARs and those BARs are demanding i/o resources? Something is really funny with this machine. Or I am reading this wrong?

I don't see anything strange about the machine, but I don't understand what Yinghai's patch was doing. The lspci output is here:

    http://marc.info/?l=linux-pci&m=127184080929189&w=2

Quoted 7 lines

I don't see any SRIOV devices, and this kernel wasn't built with IOV support anyway. The messages above are attempts to assign resources for bridge windows.

But the only real device in the whole hierarchy behind 00:1c.0 is 09:04.0, which doesn't require memory space, so I have no idea why we're trying to open memory windows.

I dont understand the code much. But there is some alignment constraints on the memory and io space dictated by the PCI code in pbus_size_io() and pbus_size_mem(), a constraint that ends up associating a minimum of 4096 bytes io window and nearly about 2MB mem window to a bridge irrespective of the requirements of the devices behind the bridge. That sounds like a bug to me. But I don't know if those are valid constraints either. Any idea?

Quoted 5 lines

This is what I meant about "we haven't tried hard enough yet." I'd rather spend time fixing the problems like this, than just putting the buggy code in and adding a kernel parameter to enable it. That guarantees that we'll never get enough testing to really shake it out.

Ok. I am no fan of kernel parameters either. However I am afraid we will attract furious looks as and when someone regresses.

Quoted 117 lines · Ram Pai

Quoted 108 lines · Bjorn Helgaas

Quoted 98 lines · Ram Pai

Quoted 77 lines · Bjorn Helgaas

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

Quoted 8 lines

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Actually the message preceeding to them are even more surprising:

Apr 20 20:31:42 [kernel] pci 0000:04:00.0: BAR 8: can't assign mem (size
0xc00000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 8: can't assign mem (size
0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 9: can't assign mem pref
(size 0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:02.0: BAR 8: can't assign mem (size
0x400000)
Apr 20 20:31:42 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size

Quoted 4 lines

0x1000)

Do these bridges have IOV BARs and those BARs are demanding i/o resources? Something is really funny with this machine. Or I am reading this wrong?

I don't see anything strange about the machine, but I don't understand what Yinghai's patch was doing. The lspci output is here:

    http://marc.info/?l=linux-pci&m=127184080929189&w=2

Quoted 7 lines

I don't see any SRIOV devices, and this kernel wasn't built with IOV support anyway. The messages above are attempts to assign resources for bridge windows.

But the only real device in the whole hierarchy behind 00:1c.0 is 09:04.0, which doesn't require memory space, so I have no idea why we're trying to open memory windows.

Quoted 8 lines

I dont understand the code much. But there is some alignment constraints on the memory and io space dictated by the PCI code in pbus_size_io() and pbus_size_mem(), a constraint that ends up associating a minimum of 4096 bytes io window and nearly about 2MB mem window to a bridge irrespective of the requirements of the devices behind the bridge. That sounds like a bug to me. But I don't know if those are valid constraints either. Any idea?

I think memory windows comes in 1MB chunks and I/O in 4096 port chunks. There are some bridges that support smaller I/O windows (search for "en1k" in drivers/pci/quirks.c), but it looks like we don't take that into account when allocating space.

And of course, there's no requirement to enable the window at all if we don't have any downstream devices.

Quoted 6 lines

Quoted 5 lines

This is what I meant about "we haven't tried hard enough yet." I'd rather spend time fixing the problems like this, than just putting the buggy code in and adding a kernel parameter to enable it. That guarantees that we'll never get enough testing to really shake it out.

Quoted 2 lines

Ok. I am no fan of kernel parameters either. However I am afraid we will attract furious looks as and when someone regresses.

Yep, I'm used to that :-)

Quoted 141 lines · On Tue, 12 Oct 2010 13:01:54 -0600 Bjorn Helgaas <[email protected]> wrote:

Quoted 117 lines · Ram Pai

Quoted 108 lines · Bjorn Helgaas

Quoted 98 lines · Ram Pai

Quoted 77 lines · Bjorn Helgaas

Quoted 41 lines · Ram Pai

Quoted 31 lines · Bjorn Helgaas

Quoted 20 lines · Ram Pai

Quoted 10 lines · Bjorn Helgaas

Quoted 9 lines · Ram Pai

PCI: override BIOS/firmware memory resource allocation
through command line parameters

Platforms that are unaware of SRIOV BARs fail to allocate MMIO resources to SRIOV PCIe devices. Hence on such platforms the OS fails to enable SRIOV.
Some platforms where BIOS/uEFI resource allocations conflict
the conflicting devices are disabled.

Quoted 8 lines

Quoted 3 lines

Ideally we would want the OS to detect and fix automatically such problems and conflicts. However previous attempts to do so have led to regression on legacy platforms.

I'm sorry to be a nay-sayer, but I think we just haven't tried hard enough. Our ACPI/PCI/e820 resource management is not well integrated, and I suspect if we straightened that out, we could avoid some of the regressions we saw with previous attempts.

Quoted 4 lines

Can you be more specific as to what can be done to fix it automatically?

Neither accepting this approach nor telling what needs to be straightened out to automatically fix all the systems out there, is just a deadend.

Yeah, I guess that wasn't really fair, sorry. And keep in mind that I'm not the PCI maintainer, so these are just my opinions, nothing like an official "nack."

I did look at this dmesg log from the thread you referenced:

Quoted 8 lines

    http://marc.info/?l=linux-kernel&m=127178918128740&w=2

but it looks to me like we just completely botched it. I don't see an SRIOV device or anything else that didn't have resources, so as far as I can tell, we started with working resource assignments from the BIOS, threw them away, and started over from scratch. We failed because we tried to assign I/O port space to bridges with nothing behind them, and there was nothing left by the time we got to the 0000:09:04.0 device that actually *did* need the space.

Quoted 3 lines

hmm.. is that possible? Yinghai's patch sized the resource requirement of each of the bridges, before actually allocating them. Which means a bridge with no device behind it would not get any i/o space.

Here's what I see in the dmesg log referenced above:

    ACPI: PCI Root Bridge [PCI0] (0000:00)
    pci_root PNP0A08:00: host bridge window [io 0x0000-0x0cf7]
    pci_root PNP0A08:00: host bridge window [io 0x0d00-0xffff]
    pci 0000:00:1c.0: PCI bridge to [bus 04-09]
    pci 0000:00:1c.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:04:00.0: PCI bridge to [bus 05-09]
    pci 0000:04:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:05:01.0: PCI bridge to [bus 08-09]
    pci 0000:05:01.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:08:00.0: PCI bridge to [bus 09-09]
    pci 0000:08:00.0:   bridge window [io 0xd000-0xdfff]
    pci 0000:09:04.0: found [13f6:8788] class 000401 header type 00
    pci 0000:09:04.0: reg 10: [io  0xd800-0xd8ff]
    pci 0000:05:02.0: PCI bridge to [bus 07-07]
    pci 0000:05:02.0:   bridge window [io 0xf000-0x0000] (disabled)
    pci 0000:05:03.0: PCI bridge to [bus 06-06]
    pci 0000:05:03.0:   bridge window [io 0xf000-0x0000] (disabled)

The above is the state as we got it from BIOS. Despite all the bridges, 09:04.0 is the only device below the 00:1c.0 bridge, and it requires only 0x100 I/O ports.

There are no devices on buses 06 (below 05:03.0) or 07 (below 05:02.0).

I didn't look at Yinghai's patch to figure out *why*, but it sure looks like we released the 09:04.0 space, then tried to assign 0x2000 ports to 05:01.0 (which needs 0x100 and had 0x1000 originally), 0x1000 to 05:02.0 (which needs none), and 0x1000 to 05:03.0 (which also needs none):

Quoted 8 lines

    PCI: No. 3 try to assign unassigned res
    release child resource [io  0xd800-0xd8ff]
    pci 0000:08:00.0: resource 7 [io 0xd000-0xdfff] released
    pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
    pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
    pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
    pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)

Actually the message preceeding to them are even more surprising:

Apr 20 20:31:42 [kernel] pci 0000:04:00.0: BAR 8: can't assign mem (size
0xc00000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 8: can't assign mem (size
0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:01.0: BAR 9: can't assign mem pref
(size 0x200000)
Apr 20 20:31:42 [kernel] pci 0000:05:02.0: BAR 8: can't assign mem (size
0x400000)
Apr 20 20:31:42 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size

Quoted 4 lines

0x1000)

Do these bridges have IOV BARs and those BARs are demanding i/o resources? Something is really funny with this machine. Or I am reading this wrong?

I don't see anything strange about the machine, but I don't understand what Yinghai's patch was doing. The lspci output is here:

    http://marc.info/?l=linux-pci&m=127184080929189&w=2

Quoted 7 lines

I don't see any SRIOV devices, and this kernel wasn't built with IOV support anyway. The messages above are attempts to assign resources for bridge windows.

But the only real device in the whole hierarchy behind 00:1c.0 is 09:04.0, which doesn't require memory space, so I have no idea why we're trying to open memory windows.

Quoted 8 lines

I dont understand the code much. But there is some alignment constraints on the memory and io space dictated by the PCI code in pbus_size_io() and pbus_size_mem(), a constraint that ends up associating a minimum of 4096 bytes io window and nearly about 2MB mem window to a bridge irrespective of the requirements of the devices behind the bridge. That sounds like a bug to me. But I don't know if those are valid constraints either. Any idea?

I think memory windows comes in 1MB chunks and I/O in 4096 port chunks. There are some bridges that support smaller I/O windows (search for "en1k" in drivers/pci/quirks.c), but it looks like we don't take that into account when allocating space.

And of course, there's no requirement to enable the window at all if we don't have any downstream devices.

Quoted 6 lines

Quoted 5 lines

This is what I meant about "we haven't tried hard enough yet." I'd rather spend time fixing the problems like this, than just putting the buggy code in and adding a kernel parameter to enable it. That guarantees that we'll never get enough testing to really shake it out.

Quoted 4 lines

Quoted 2 lines

Ok. I am no fan of kernel parameters either. However I am afraid we will attract furious looks as and when someone regresses.

Yep, I'm used to that :-)

So where do we stand with this machine's problem?

Ram, do you have other machines that require your override patch?

Until we understand what's failing and why, I'm hesitant to apply a patch that will work around the problem but require an extra kernel parameter.

Our general approach is to avoid overwriting the BIOS provided configuration, and when we do, to use available e820 and PnP data to figure out where to put things, in addition to allocating space in a similar way to Windows to avoid hitting undiscovered platform bugs (i.e. Bjorn's recent patchset).

So while I'm not totally opposed to this patch, I'd feel much better about it if we really understood how the failing machines were failing before we try to workaround potential BIOS limitations.

Quoted 1 line

So where do we stand with this machine's problem?

I think, this machine with the latest mainline kernel, will see memory resource allocation failure messages. Since the latest kernel does not release and retry to allocate resources, the io resources allocated by the BIOS continue to stay put and hence the problem is masked.

However, any attempt to release and reallocate the resource on that machine will fail; because as pointed out by Bjorn, there is some weird allocation behavior in the current code. Unfortunately I cannot trigger that behavior on any of my machines.

I have requested data from Peter, who originally reported the problem. Hope he still has his setup with the Xonar card available for debugging.

Anyway, I do see a smoking gun in pbus_size_io() and pbus_size_mem(). They call resource_size() to find the size requirement of resources of all devices behind the bridge. However for resources whose start and size are set to zero, resource_size() returns one. Later ALIGN() rounds it up to the next higher alignment boundary.

Quoted 1 line

Ram, do you have other machines that require your override patch?

Yes, I have a couple of machines whose BIOS is unaware of SRIOV resources, These machines need the override patch. :(

Quoted 3 lines

Until we understand what's failing and why, I'm hesitant to apply a patch that will work around the problem but require an extra kernel parameter.

We have already come a full circle here. The original approach was reverted because it regressed a platform. Now we are rejecting this approach because we want the original approach.

Quoted 12 lines · On Tue, 19 Oct 2010 10:17:40 -0700 Ram Pai <[email protected]> wrote:

Quoted 1 line

So where do we stand with this machine's problem?

I think, this machine with the latest mainline kernel, will see memory resource allocation failure messages. Since the latest kernel does not release and retry to allocate resources, the io resources allocated by the BIOS continue to stay put and hence the problem is masked.

However, any attempt to release and reallocate the resource on that machine will fail; because as pointed out by Bjorn, there is some weird allocation behavior in the current code. Unfortunately I cannot trigger that behavior on any of my machines.

Quoted 8 lines

I have requested data from Peter, who originally reported the problem. Hope he still has his setup with the Xonar card available for debugging.

Anyway, I do see a smoking gun in pbus_size_io() and pbus_size_mem(). They call resource_size() to find the size requirement of resources of all devices behind the bridge. However for resources whose start and size are set to zero, resource_size() returns one. Later ALIGN() rounds it up to the next higher alignment boundary.

Right, if there are no devices with actual sizes behind a given bridge window we shouldn't bother to allocate space (that may mean re-allocation later if a device is added, but that needs extra work anyway).

And like Bjorn said, I/O sizing has special requirements for PCI-PCI bridges, but for others we may be able to make the windows smaller.

Quoted 5 lines

Quoted 2 lines

Ram, do you have other machines that require your override patch?

Yes, I have a couple of machines whose BIOS is unaware of SRIOV resources, These machines need the override patch. :(

Ok, but hopefully we can make those machines work without extra kernel options; at worst maybe we can special case SRIOV resources and cause them to trigger more aggressive reallocation.

Quoted 7 lines

Quoted 3 lines

Until we understand what's failing and why, I'm hesitant to apply a patch that will work around the problem but require an extra kernel parameter.

We have already come a full circle here. The original approach was reverted because it regressed a platform. Now we are rejecting this approach because we want the original approach.

Well the original approach had several problems:
- unclear try= parameter
- undocumented and ad-hoc reallocation behavior
- poor (well, lack of) overall design

The root of the issue is still that we have poor data on where we're allowed to put device resources. Bjorn has been improving this, along with changing the way we do allocations so as to avoid problem areas, but ultimately this is the area where we need the most work.

We've tried and failed to add chipset specific drivers to give us safe ranges, but those just can't keep up with the number of platforms and variations out there. On x86, I think the only reasonable approach is to use the platforms as designed, i.e. use the resources Windows uses and in the way Windows uses them. Anything else just means we'll be playing catch up.

As special cases arise (i.e. ways we use the platform that depart from its original design and Windows version), as I suspect this SRIOV issue is, we may need to apply additional conditions. But I'd like to avoid that if at all possible.

So on that note, does Windows on these machines support allocation of SRIOV resources? If so, how is it handled? Which resource ranges are used for the extra BARs?

On Tue, Oct 19, 2010 at 11:24:39AM -0700, Jesse Barnes wrote:

Ok. After further investigation, I find that the BIOS has not allocated any resource to hotplug bridges that have no devices behind them. However the OS tries to allocate some minimal resources, 4096 I/O ports and 2M mem window, to the these hotplug bridges but fails because there are not enough resources to satisfy all the hotplug bridges.

This issue exists even today with the latest mainline kernel but is masked because no devices are really effected. However Yinghai's reallocation patch exposed the issue, since it released the BIOS allocated window and tried to reallocate. Unfortunately it ended up allocating in the wrong order. The bridges with real devices got no resources, where as the hotplug bridges with no devices got some mimimal resources.

The details are captured at: https://bugzilla.kernel.org/show_bug.cgi?id=15960

I suppose the solution is to not pre-allocate resources to hotplug bridges that have no devices behind them, and then bring back Yinghai's reallocation patch.

Quoted 3 lines

So on that note, does Windows on these machines support allocation of SRIOV resources? If so, how is it handled? Which resource ranges are used for the extra BARs?

No. I have not tried windows on these boxes. But last I heard windows did not support SRIOV. Does it?

Quoted 17 lines · Ram Pai

On Tue, Oct 19, 2010 at 11:24:39AM -0700, Jesse Barnes wrote:

Ok. After further investigation, I find that the BIOS has not allocated any resource to hotplug bridges that have no devices behind them. However the OS tries to allocate some minimal resources, 4096 I/O ports and 2M mem window, to the these hotplug bridges but fails because there are not enough resources to satisfy all the hotplug bridges.

This issue exists even today with the latest mainline kernel but is masked because no devices are really effected. However Yinghai's reallocation patch exposed the issue, since it released the BIOS allocated window and tried to reallocate. Unfortunately it ended up allocating in the wrong order. The bridges with real devices got no resources, where as the hotplug bridges with no devices got some mimimal resources.

The details are captured at: https://bugzilla.kernel.org/show_bug.cgi?id=15960

Quoted 2 lines

I suppose the solution is to not pre-allocate resources to hotplug bridges that have no devices behind them, and then bring back Yinghai's reallocation patch.

If we assign resources to bridges with no devices behind them while starving bridges that DO have devices behind them, I think that's a bug. It'd be great if you could isolate and fix that before we complicate things by throwing Yinghai's patch into the mix.

I think it'd interesting to have a debug parameter like "pci=assign-all" that meant "ignore all BIOS PCI config and start from scratch." I'm sure we'd trip over lots of issues and it would be easier to resolve them before trying to make things fancier.

Quoted 7 lines

Quoted 4 lines

So on that note, does Windows on these machines support allocation of SRIOV resources? If so, how is it handled? Which resource ranges are used for the extra BARs?

No. I have not tried windows on these boxes. But last I heard windows did not support SRIOV. Does it?

I've heard anecdotally that Windows supports SRIOV much better than Linux does, but I have no evidence. I would guess that it depends on driver support in addition to Windows itself.

Bjorn

Quoted 4 lines · Bjorn Helgaas

If we assign resources to bridges with no devices behind them while starving bridges that DO have devices behind them, I think that's a bug. It'd be great if you could isolate and fix that before we complicate things by throwing Yinghai's patch into the mix.

Yes. I sent out a patch an hour back with a fix for that.

Quoted 4 lines

I think it'd interesting to have a debug parameter like "pci=assign-all" that meant "ignore all BIOS PCI config and start from scratch." I'm sure we'd trip over lots of issues and it would be easier to resolve them before trying to make things fancier.

The patch that I had sent 3-weeks back had that feature. It can be triggered by pci=override=always.

However, your arguement earlier was that it would make no difference because no one will enable that parameter by default. This means hardly anyone will report any bugs.

Gating "Yinghai's patch or any other approach" on fixing all the bugs in the pci subsystem, implicitly means 'go away' ;). I hope you dont mean that.

I propose to bring Yinghai's patch, for that will enable us to expose bugs and at the same time it will enable SRIOV on *all* the platforms that don't have it.

PCI: ignore failure to preallocate minimal resources to
hotplug bridges

Linux tries to pre-allocate minimal resources to hotplug
bridges. This works fine as long as there are enough
resources to satisfy all other genuine resource
requirements. However if enough resources are not
available to satisfy the pre-allocation, the
resource-allocator reports errors and returns failure.

This patch distinguishes between must-need resources and
nice-to-have resources. Any failure to allocate
nice-to-have resources are ignored.

This behavior can be particularly useful to trigger automatic reallocation, if the OS discovers genuine resource allocation conflicts or genuine unallocated BARs caused by not-so-smart allocation behavior of the native BIOS.

The motivation for this patch is due a issue reported in
https://bugzilla.kernel.org/show_bug.cgi?id=15960

drivers/pci/setup-bus.c +16 -2

 	if (!b_res)
  		return;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
-
+		dev_present=1;
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 			struct resource *r = &dev->resource[i];
 			unsigned long r_size;

@@ -460,6 +460,12 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 	b_res->start = 4096;
 	b_res->end = b_res->start + size - 1;
 	b_res->flags |= IORESOURCE_STARTALIGN;
+
+	/* if no devices are behind this bus, inform
+	 * resource-allocater to try hard but not to worry
+	 * if allocations fail 
+	 */
+	b_res->flags |= (!dev_present) ? IORESOURCE_IGNORE_FAIL : 0;
 }
 
 /* Calculate the size of the bus and minimal alignment which

@@ -470,7 +476,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	struct pci_dev *dev;
 	resource_size_t min_align, align, size, old_size;
 	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
-	int order, max_order;
+	int order, max_order, dev_present=0;
 	struct resource *b_res = find_free_bus_resource(bus, type);
 	unsigned int mem64_mask = 0;
 

@@ -487,6 +493,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 
+		dev_present=1;
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 			struct resource *r = &dev->resource[i];
 			resource_size_t r_size;

@@ -550,6 +557,13 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	b_res->end = size + min_align - 1;
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	b_res->flags |= mem64_mask;
+
+	/* if no devices are behind this bus, inform
+	 * resource-allocater to try hard but not to worry
+	 * if allocations fail 
+	 */
+	b_res->flags |= (!dev_present) ? IORESOURCE_IGNORE_FAIL : 0;
+
 	return 1;
 }
 

drivers/pci/setup-res.c +2 -1

index 2aaa131..a8ce953 100644
@@ -210,7 +210,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 	if (!align) {
 		dev_info(&dev->dev, "BAR %d: can't assign %pR "
 			 "(bogus alignment)\n", resno, res);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	bus = dev->bus;

drivers/pci/setup-res.c +5 -0

+			ret = 0;
+			goto out;
+		}
 		if (res->flags & IORESOURCE_MEM)
 			if (res->flags & IORESOURCE_PREFETCH)
 				type = "mem pref";

@@ -239,6 +244,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 			 resno, type, (unsigned long long) resource_size(res));
 	}
 
+out:
+	res->flags &= ~IORESOURCE_IGNORE_FAIL;
 	return ret;
 }
 

include/linux/ioport.h +1 -0

index b227902..941624b 100644
@@ -49,6 +49,7 @@ struct resource_list {
 
 #define IORESOURCE_SIZEALIGN	0x00040000	/* size indicates alignment */
 #define IORESOURCE_STARTALIGN	0x00080000	/* start field is alignment */
+#define IORESOURCE_IGNORE_FAIL	0x00800000 	/* IGNORE if allocation fail*/
 
 #define IORESOURCE_MEM_64	0x00100000
 #define IORESOURCE_WINDOW	0x00200000	/* forwarded by bridge */

PCI: ignore failure to preallocate minimal resources to
hotplug bridges

Linux tries to pre-allocate minimal resources to hotplug
bridges. This works fine as long as there are enough
resources to satisfy all other genuine resource
requirements. However if enough resources are not
available to satisfy the pre-allocation, the
resource-allocator reports errors and returns failure.

This patch distinguishes between must-need resources and
nice-to-have resources. Any failure to allocate
nice-to-have resources are ignored.

This behavior can be particularly useful to trigger automatic reallocation, if the OS discovers genuine resource allocation conflicts or genuine unallocated BARs caused by not-so-smart allocation behavior of the native BIOS.

The motivation for this patch is due a issue reported in
https://bugzilla.kernel.org/show_bug.cgi?id=15960

Signed-off-by Ram Pai <[email protected]>

Quoted 23 lines · On Fri, 22 Oct 2010 10:16:40 -0700 Ram Pai <[email protected]> wrote:

PCI: ignore failure to preallocate minimal resources to
hotplug bridges

Linux tries to pre-allocate minimal resources to hotplug
bridges. This works fine as long as there are enough
resources to satisfy all other genuine resource
requirements. However if enough resources are not
available to satisfy the pre-allocation, the
resource-allocator reports errors and returns failure.

This patch distinguishes between must-need resources and
nice-to-have resources. Any failure to allocate
nice-to-have resources are ignored.

This behavior can be particularly useful to trigger automatic reallocation, if the OS discovers genuine resource allocation conflicts or genuine unallocated BARs caused by not-so-smart allocation behavior of the native BIOS.

The motivation for this patch is due a issue reported in
https://bugzilla.kernel.org/show_bug.cgi?id=15960

Quoted 2 lines

Signed-off-by Ram Pai <[email protected]>

Seems like we want a fix like this; did you see Bjorn's comments? Maybe we can adjust the order of allocation instead?

What issues do you still have outstanding wrt resource allocation? Do you still think you need the big "reallocate everything" option?

Thanks,

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center

PCI: reserve additional resources for hotplug bridges only after all essential resources are allocated.

Linux tries to reserve minimal resources for hotplug
bridges. This works fine as long as there are enough
resources to satisfy all other essential resource
requirements. However if enough resources are not
available to satisfy both the essential resources
and the reservation for hotplug resources, the
resource-allocator reports error and returns failure.

This patch distinguishes between essential resources and
nice-to-have resources. Any failure to allocate
nice-to-have resources are ignored.

This behavior can be particularly useful to trigger automatic reallocation, if the OS discovers genuine resource allocation conflicts or genuine unallocated requests caused by not-so-smart allocation behavior of the native BIOS.

https://bugzilla.kernel.org/show_bug.cgi?id=15960
captures the details of the issue and the motivation behind this patch.

Signed-off-by Ram Pai <[email protected]>

drivers/pci/setup-bus.c | 142 +++++++++++++++++++++++++++++++++++-------------
include/linux/ioport.h | 1

2 files changed, 105 insertions(+), 38 deletions(-)

drivers/pci/setup-bus.c +104 -38

index 66cb8f4..76a7fb7 100644
@@ -91,7 +91,26 @@ static void __dev_sort_resources(struct pci_dev *dev,
 	pdev_sort_resources(dev, head);
 }
 
-static void __assign_resources_sorted(struct resource_list *head,
+
+static void adjust_resources_sorted(struct resource_list *head)
+{
+	struct resource *res;
+	struct resource_list *list, *tmp;
+	int idx;
+
+	for (list = head->next; list;) {
+		res = list->res;
+		idx = res - &list->dev->resource[0];
+		if (res->add_size)
+			adjust_resource(res, res->start, 
+				resource_size(res) + res->add_size);
+		tmp = list;
+		list = list->next;
+		kfree(tmp);
+	}
+}
+
+static void assign_requested_resources_sorted(struct resource_list *head,
 				 struct resource_list_x *fail_head)
 {
 	struct resource *res;

@@ -114,14 +133,25 @@ static void __assign_resources_sorted(struct resource_list *head,
 			}
 			res->start = 0;
 			res->end = 0;
+			res->add_size = 0;
 			res->flags = 0;
 		}
 		tmp = list;
 		list = list->next;
-		kfree(tmp);
 	}
 }
 
+static void __assign_resources_sorted(struct resource_list *head,
+				 struct resource_list_x *fail_head)
+{
+	/* Satisfy the must-have resource requests */
+	assign_requested_resources_sorted(head, fail_head);
+
+	/* Try to satisfy any additional nice-to-have resource 
+		requests */
+	adjust_resources_sorted(head);
+}
+
 static void pdev_assign_resources_sorted(struct pci_dev *dev,
 				 struct resource_list_x *fail_head)
 {

@@ -404,15 +434,37 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, unsigned lon
 	return NULL;
 }
 
+static resource_size_t calculate_iosize(resource_size_t size, 
+		resource_size_t min_size, 
+		resource_size_t size1, 
+		resource_size_t old_size, 
+		resource_size_t align)
+{
+	if (size < min_size)
+		size = min_size;
+	if (old_size == 1 )
+		old_size = 0;
+	/* To be fixed in 2.5: we should have sort of HAVE_ISA
+	   flag in the struct pci_bus. */
+#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
+	size = (size & 0xff) + ((size & ~0xffUL) << 2);
+#endif
+	size = ALIGN(size + size1, align);
+	if (size < old_size)
+		size = old_size;
+	return size;
+}
+
 /* Sizing the IO windows of the PCI-PCI bridge is trivial,
    since these windows have 4K granularity and the IO ranges
    of non-bridge PCI devices are limited to 256 bytes.
    We must be careful with the ISA aliasing though. */
-static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
+static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, 
+			resource_size_t add_size)
 {
 	struct pci_dev *dev;
 	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
-	unsigned long size = 0, size1 = 0, old_size;
+	unsigned long size = 0, size1 = 0;
 
 	if (!b_res)
  		return;

@@ -435,20 +487,16 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 				size1 += r_size;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-/* To be fixed in 2.5: we should have sort of HAVE_ISA
-   flag in the struct pci_bus. */
-#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
-	size = (size & 0xff) + ((size & ~0xffUL) << 2);
-#endif
-	size = ALIGN(size + size1, 4096);
-	if (size < old_size)
-		size = old_size;
-	if (!size) {
+
+	size = calculate_iosize(size, min_size, size1, 
+			resource_size(b_res), 4096);
+	if (add_size)
+		size1 = calculate_iosize(size, min_size+add_size, size1, 
+				resource_size(b_res), 4096);
+	else
+		size1  = size;
+
+	if (!size && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
 				 "%pR to [bus %02x-%02x] (unused)\n", b_res,

@@ -459,16 +507,35 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 	/* Alignment of the IO window is always 4K */
 	b_res->start = 4096;
 	b_res->end = b_res->start + size - 1;
+	b_res->add_size = size1-size;
 	b_res->flags |= IORESOURCE_STARTALIGN;
 }
 
+
+static resource_size_t calculate_memsize(resource_size_t size, 
+		resource_size_t min_size, 
+		resource_size_t size1, 
+		resource_size_t old_size, 
+		resource_size_t align)
+{
+	if (size < min_size)
+		size = min_size;
+	if (old_size == 1 )
+		old_size = 0;
+	if (size < old_size)
+		size = old_size;
+	size = ALIGN(size + size1, align);
+	return size;
+}
+
 /* Calculate the size of the bus and minimal alignment which
    guarantees that all child resources fit in this size. */
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
-			 unsigned long type, resource_size_t min_size)
+			 unsigned long type, resource_size_t min_size, 
+				resource_size_t add_size)
 {
 	struct pci_dev *dev;
-	resource_size_t min_align, align, size, old_size;
+	resource_size_t min_align, align, size, size1;
 	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);

@@ -516,13 +583,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			mem64_mask &= r->flags & IORESOURCE_MEM_64;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-	if (size < old_size)
-		size = old_size;
 
 	align = 0;
 	min_align = 0;

@@ -537,8 +597,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			min_align = align1 >> 1;
 		align += aligns[order];
 	}
-	size = ALIGN(size, min_align);
-	if (!size) {
+
+	size = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
+	if (add_size)
+		size1 = calculate_memsize(size, min_size+add_size, 0, resource_size(b_res), align);
+	else
+		size1  = size;
+
+	if (!size && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
 				 "%pR to [bus %02x-%02x] (unused)\n", b_res,

@@ -548,8 +614,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	}
 	b_res->start = min_align;
 	b_res->end = size + min_align - 1;
-	b_res->flags |= IORESOURCE_STARTALIGN;
-	b_res->flags |= mem64_mask;
+	b_res->add_size = size1 - size;
+	b_res->flags |= IORESOURCE_STARTALIGN | mem64_mask;
 	return 1;
 }
 

@@ -606,7 +672,7 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 	unsigned long mask, prefmask;
-	resource_size_t min_mem_size = 0, min_io_size = 0;
+	resource_size_t additional_mem_size = 0, additional_io_size = 0;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct pci_bus *b = dev->subordinate;

@@ -637,11 +703,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 	case PCI_CLASS_BRIDGE_PCI:
 		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
-			min_io_size  = pci_hotplug_io_size;
-			min_mem_size = pci_hotplug_mem_size;
+			additional_io_size  = pci_hotplug_io_size;
+			additional_mem_size = pci_hotplug_mem_size;
 		}
 	default:
-		pbus_size_io(bus, min_io_size);
+		pbus_size_io(bus, 0, additional_io_size);
 		/* If the bridge supports prefetchable range, size it
 		   separately. If it doesn't, or its prefetchable window
 		   has already been allocated by arch code, try

@@ -649,11 +715,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 		   resources. */
 		mask = IORESOURCE_MEM;
 		prefmask = IORESOURCE_MEM | IORESOURCE_PREFETCH;
-		if (pbus_size_mem(bus, prefmask, prefmask, min_mem_size))
+		if (pbus_size_mem(bus, prefmask, prefmask, 0, additional_mem_size))
 			mask = prefmask; /* Success, size non-prefetch only. */
 		else
-			min_mem_size += min_mem_size;
-		pbus_size_mem(bus, mask, IORESOURCE_MEM, min_mem_size);
+			additional_mem_size += additional_mem_size;
+		pbus_size_mem(bus, mask, IORESOURCE_MEM, 0, additional_mem_size);
 		break;
 	}
 }

include/linux/ioport.h +1 -0

index e9bb22c..d00c61c 100644
@@ -18,6 +18,7 @@
 struct resource {
 	resource_size_t start;
 	resource_size_t end;
+	resource_size_t add_size;
 	const char *name;
 	unsigned long flags;
 	struct resource *parent, *sibling, *child;

Quoted 21 lines · Ram Pai

PCI: reserve additional resources for hotplug bridges only after all essential resources are allocated.

Linux tries to reserve minimal resources for hotplug
bridges. This works fine as long as there are enough
resources to satisfy all other essential resource
requirements. However if enough resources are not
available to satisfy both the essential resources
and the reservation for hotplug resources, the
resource-allocator reports error and returns failure.

This patch distinguishes between essential resources and
nice-to-have resources. Any failure to allocate
nice-to-have resources are ignored.

This behavior can be particularly useful to trigger automatic reallocation, if the OS discovers genuine resource allocation conflicts or genuine unallocated requests caused by not-so-smart allocation behavior of the native BIOS.

Quoted 3 lines

https://bugzilla.kernel.org/show_bug.cgi?id=15960
captures the details of the issue and the motivation behind this patch.

Please don't indent and right-justify the changelog.

This is a PCI-specific issue, so I'm not comfortable adding add_size to struct resource. That penalizes all resource users while only helping PCI. Also, the struct resource lives forever, and the add_size information is only useful while we're configuring the bridge.

Bjorn

Quoted 261 lines

Signed-off-by Ram Pai <[email protected]>

drivers/pci/setup-bus.c | 142 +++++++++++++++++++++++++++++++++++-------------
include/linux/ioport.h | 1

2 files changed, 105 insertions(+), 38 deletions(-)

drivers/pci/setup-bus.c +104 -38

index 66cb8f4..76a7fb7 100644
@@ -91,7 +91,26 @@ static void __dev_sort_resources(struct pci_dev *dev,
 	pdev_sort_resources(dev, head);
 }
 
-static void __assign_resources_sorted(struct resource_list *head,
+
+static void adjust_resources_sorted(struct resource_list *head)
+{
+	struct resource *res;
+	struct resource_list *list, *tmp;
+	int idx;
+
+	for (list = head->next; list;) {
+		res = list->res;
+		idx = res - &list->dev->resource[0];
+		if (res->add_size)
+			adjust_resource(res, res->start, 
+				resource_size(res) + res->add_size);
+		tmp = list;
+		list = list->next;
+		kfree(tmp);
+	}
+}
+
+static void assign_requested_resources_sorted(struct resource_list *head,
 				 struct resource_list_x *fail_head)
 {
 	struct resource *res;

@@ -114,14 +133,25 @@ static void __assign_resources_sorted(struct resource_list *head,
 			}
 			res->start = 0;
 			res->end = 0;
+			res->add_size = 0;
 			res->flags = 0;
 		}
 		tmp = list;
 		list = list->next;
-		kfree(tmp);
 	}
 }
 
+static void __assign_resources_sorted(struct resource_list *head,
+				 struct resource_list_x *fail_head)
+{
+	/* Satisfy the must-have resource requests */
+	assign_requested_resources_sorted(head, fail_head);
+
+	/* Try to satisfy any additional nice-to-have resource 
+		requests */
+	adjust_resources_sorted(head);
+}
+
 static void pdev_assign_resources_sorted(struct pci_dev *dev,
 				 struct resource_list_x *fail_head)
 {

@@ -404,15 +434,37 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, unsigned lon
 	return NULL;
 }
 
+static resource_size_t calculate_iosize(resource_size_t size, 
+		resource_size_t min_size, 
+		resource_size_t size1, 
+		resource_size_t old_size, 
+		resource_size_t align)
+{
+	if (size < min_size)
+		size = min_size;
+	if (old_size == 1 )
+		old_size = 0;
+	/* To be fixed in 2.5: we should have sort of HAVE_ISA
+	   flag in the struct pci_bus. */
+#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
+	size = (size & 0xff) + ((size & ~0xffUL) << 2);
+#endif
+	size = ALIGN(size + size1, align);
+	if (size < old_size)
+		size = old_size;
+	return size;
+}
+
 /* Sizing the IO windows of the PCI-PCI bridge is trivial,
    since these windows have 4K granularity and the IO ranges
    of non-bridge PCI devices are limited to 256 bytes.
    We must be careful with the ISA aliasing though. */
-static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
+static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, 
+			resource_size_t add_size)
 {
 	struct pci_dev *dev;
 	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
-	unsigned long size = 0, size1 = 0, old_size;
+	unsigned long size = 0, size1 = 0;
 
 	if (!b_res)
  		return;

@@ -435,20 +487,16 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 				size1 += r_size;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-/* To be fixed in 2.5: we should have sort of HAVE_ISA
-   flag in the struct pci_bus. */
-#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
-	size = (size & 0xff) + ((size & ~0xffUL) << 2);
-#endif
-	size = ALIGN(size + size1, 4096);
-	if (size < old_size)
-		size = old_size;
-	if (!size) {
+
+	size = calculate_iosize(size, min_size, size1, 
+			resource_size(b_res), 4096);
+	if (add_size)
+		size1 = calculate_iosize(size, min_size+add_size, size1, 
+				resource_size(b_res), 4096);
+	else
+		size1  = size;
+
+	if (!size && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
 				 "%pR to [bus %02x-%02x] (unused)\n", b_res,

@@ -459,16 +507,35 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 	/* Alignment of the IO window is always 4K */
 	b_res->start = 4096;
 	b_res->end = b_res->start + size - 1;
+	b_res->add_size = size1-size;
 	b_res->flags |= IORESOURCE_STARTALIGN;
 }
 
+
+static resource_size_t calculate_memsize(resource_size_t size, 
+		resource_size_t min_size, 
+		resource_size_t size1, 
+		resource_size_t old_size, 
+		resource_size_t align)
+{
+	if (size < min_size)
+		size = min_size;
+	if (old_size == 1 )
+		old_size = 0;
+	if (size < old_size)
+		size = old_size;
+	size = ALIGN(size + size1, align);
+	return size;
+}
+
 /* Calculate the size of the bus and minimal alignment which
    guarantees that all child resources fit in this size. */
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
-			 unsigned long type, resource_size_t min_size)
+			 unsigned long type, resource_size_t min_size, 
+				resource_size_t add_size)
 {
 	struct pci_dev *dev;
-	resource_size_t min_align, align, size, old_size;
+	resource_size_t min_align, align, size, size1;
 	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);

@@ -516,13 +583,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			mem64_mask &= r->flags & IORESOURCE_MEM_64;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-	if (size < old_size)
-		size = old_size;
 
 	align = 0;
 	min_align = 0;

@@ -537,8 +597,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			min_align = align1 >> 1;
 		align += aligns[order];
 	}
-	size = ALIGN(size, min_align);
-	if (!size) {
+
+	size = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
+	if (add_size)
+		size1 = calculate_memsize(size, min_size+add_size, 0, resource_size(b_res), align);
+	else
+		size1  = size;
+
+	if (!size && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
 				 "%pR to [bus %02x-%02x] (unused)\n", b_res,

@@ -548,8 +614,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	}
 	b_res->start = min_align;
 	b_res->end = size + min_align - 1;
-	b_res->flags |= IORESOURCE_STARTALIGN;
-	b_res->flags |= mem64_mask;
+	b_res->add_size = size1 - size;
+	b_res->flags |= IORESOURCE_STARTALIGN | mem64_mask;
 	return 1;
 }
 

@@ -606,7 +672,7 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 	unsigned long mask, prefmask;
-	resource_size_t min_mem_size = 0, min_io_size = 0;
+	resource_size_t additional_mem_size = 0, additional_io_size = 0;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct pci_bus *b = dev->subordinate;

@@ -637,11 +703,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 	case PCI_CLASS_BRIDGE_PCI:
 		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
-			min_io_size  = pci_hotplug_io_size;
-			min_mem_size = pci_hotplug_mem_size;
+			additional_io_size  = pci_hotplug_io_size;
+			additional_mem_size = pci_hotplug_mem_size;
 		}
 	default:
-		pbus_size_io(bus, min_io_size);
+		pbus_size_io(bus, 0, additional_io_size);
 		/* If the bridge supports prefetchable range, size it
 		   separately. If it doesn't, or its prefetchable window
 		   has already been allocated by arch code, try

@@ -649,11 +715,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus)
 		   resources. */
 		mask = IORESOURCE_MEM;
 		prefmask = IORESOURCE_MEM | IORESOURCE_PREFETCH;
-		if (pbus_size_mem(bus, prefmask, prefmask, min_mem_size))
+		if (pbus_size_mem(bus, prefmask, prefmask, 0, additional_mem_size))
 			mask = prefmask; /* Success, size non-prefetch only. */
 		else
-			min_mem_size += min_mem_size;
-		pbus_size_mem(bus, mask, IORESOURCE_MEM, min_mem_size);
+			additional_mem_size += additional_mem_size;
+		pbus_size_mem(bus, mask, IORESOURCE_MEM, 0, additional_mem_size);
 		break;
 	}
 }

include/linux/ioport.h +0 -0

index e9bb22c..d00c61c 100644

include/linux/ioport.h line 24

Quoted 7 lines

include/linux/ioport.h +1 -0

@@ -18,6 +18,7 @@
 struct resource {
 	resource_size_t start;
 	resource_size_t end;
+	resource_size_t add_size;
 	const char *name;
 	unsigned long flags;
 	struct resource *parent, *sibling, *child;

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 5 lines · Bjorn Helgaas

This is a PCI-specific issue, so I'm not comfortable adding add_size to struct resource. That penalizes all resource users while only helping PCI. Also, the struct resource lives forever, and the add_size information is only useful while we're configuring the bridge.

Any suggestion on how to do this without adding a field to struct resource?

   PCI: pre-allocate additional resources to devices only after successful allocation of essential resources.

   Linux tries to pre-allocate minimal resources to hotplug bridges. This works fine as long as there are enough resources  to satisfy all other genuine resource requirements. However if enough resources are not available to satisfy any of these nice-to-have pre-allocations, the resource-allocator reports errors and returns failure.

   This patch distinguishes between must-have resource from nice-to-have resource.  Any failure to allocate nice-to-have resources are ignored.

   This behavior can be particularly useful to trigger automatic reallocation when the OS discovers genuine allocation-conflicts or genuine unallocated-requests caused by buggy allocation behavior of the native BIOS/uEFI.

   https://bugzilla.kernel.org/show_bug.cgi?id=15960 captures the movitation behind the patch.

   changelog v2:  o  fixed a bug where pci_assign_resource() was called on a resource of zero resource size.

   changelog v3:  addressed Bjorn's comment
              o  "Please don't indent and right-justify the changelog".
              o  removed add_size from struct resource.  The additional size is now tracked using a linked list.

   changelog v4:  o moved freeing up of elements of head list from assign_requested_resources_sorted() to __assign_resources_sorted(). This fixes a corruption bug.
              o removed a wrong reference to 'add_size' in pbus_size_mem(). Erroneously got introduced while generating the patch.
              o some code optimizations in adjust_resources_sorted() and assign_requested_resources_sorted()

Signed-off-by Ram Pai <[email protected]>

drivers/pci/setup-bus.c +1 -1

index 66cb8f4..efbdff2 100644
@@ -33,11 +33,23 @@ struct resource_list_x {
        struct pci_dev *dev;
        resource_size_t start;
        resource_size_t end;
+       resource_size_t add_size;
        unsigned long flags;
  };
 
-static void add_to_failed_list(struct resource_list_x *head,

Quoted 51 lines · Ram Pai

   PCI: pre-allocate additional resources to devices only after successful allocation of essential resources.

   Linux tries to pre-allocate minimal resources to hotplug bridges. This works fine as long as there are enough resources  to satisfy all other genuine resource requirements. However if enough resources are not available to satisfy any of these nice-to-have pre-allocations, the resource-allocator reports errors and returns failure.

   This patch distinguishes between must-have resource from nice-to-have resource.  Any failure to allocate nice-to-have resources are ignored.

   This behavior can be particularly useful to trigger automatic reallocation when the OS discovers genuine allocation-conflicts or genuine unallocated-requests caused by buggy allocation behavior of the native BIOS/uEFI.

   https://bugzilla.kernel.org/show_bug.cgi?id=15960 captures the movitation behind the patch.

   changelog v2:  o  fixed a bug where pci_assign_resource() was called on a resource of zero resource size.

   changelog v3:  addressed Bjorn's comment
              o  "Please don't indent and right-justify the changelog".
              o  removed add_size from struct resource.  The additional size is now tracked using a linked list.

   changelog v4:  o moved freeing up of elements of head list from assign_requested_resources_sorted() to __assign_resources_sorted(). This fixes a corruption bug.
              o removed a wrong reference to 'add_size' in pbus_size_mem(). Erroneously got introduced while generating the patch.
              o some code optimizations in adjust_resources_sorted() and assign_requested_resources_sorted()

Signed-off-by Ram Pai <[email protected]>

drivers/pci/setup-bus.c +1 -0

index 66cb8f4..efbdff2 100644
@@ -33,11 +33,23 @@ struct resource_list_x {
        struct pci_dev *dev;
        resource_size_t start;
        resource_size_t end;
+       resource_size_t add_size;
        unsigned long flags;
  };
 

drivers/pci/setup-bus.c struct resource_list_x { · line 48

Quoted 11 lines

drivers/pci/setup-bus.c +9 -2

-static void add_to_failed_list(struct resource_list_x *head,
-                                struct pci_dev *dev, struct resource *res)
+#define free_list(type, head) do {                      \
+       struct type *list, *tmp;                        \
+       for (list = (head)->next; list;) {              \
+               tmp = list;                             \
+               list = list->next;                      \
+               kfree(tmp);                             \
+       }                                               \
+       (head)->next = NULL;                            \
+} while (0)

inline function should be better?

Yinghai

PCI: pre-allocate additional resources to devices only after successful allocation of essential resources.
...

Quoted 3 lines · Ram Pai

PCI: pre-allocate additional resources to devices only after successful allocation of essential resources.
...

When you post a new version, can you please include some text about "what changed between v1 and v2"?