Skip to contentSkip to conversation

PCI updates - prefetchable memory regions

PATCH In review Russell King 3 participants 5 messages Dec 19, 2001 – Dec 20, 2001 PCI

lore
Share & tools
lore

Russell King

author
#0 lore raw

PATCH 1/1 PCI updates - prefetchable memory regions

Traditionally, Linux has treated prefetchable memory regions as if they were non-prefetchable memory regions.

The following patch allows architectures to split the prefetchable memory regions from the non-prefetchable regions if they wish by supplying a third bus resource. This third resource must have the IORESOURCE_PREFETCH flag set. (Other parts of the PCI layer already assume this is true). Currently, some ARM based machines make use of this facility.

If this resource is not present, we fall back to the original scheme, where we allocate prefetchable memory regions as if they were non-prefetchable. This magic is handled by pdev_adjust_mem_ranges().

We introduce 3 new elements into pbus_set_ranges_data:
- prefetch_valid - indicates if we have resource[2] in the parent bus (which is used for the prefetchable memory region).
- prefetch_start - the start of the prefetchable memory region.
- prefetch_end - the end of the prefetchable memory region.

The last two are treated in the same way as mem_start and mem_end for determining the memory region which the bridge should forward.

Please review this patch, which is targetted solely for 2.5.

drivers/pci/setup-bus.c +59 -9
diff -ur orig/drivers/pci/setup-bus.c linux/drivers/pci/setup-bus.c
@@ -12,6 +12,8 @@
 /*
  * Nov 2000, Ivan Kokshaysky <[email protected]>
  *	     PCI-PCI bridges cleanup, sorted resource allocation
+ * May 2001, Russell King <[email protected]>
+ *           Allocate prefetchable memory regions where available.
  */
 
 #include <linux/init.h>

@@ -32,6 +34,26 @@
 
 #define ROUND_UP(x, a)		(((x) + (a) - 1) & ~((a) - 1))
 
+static inline void
+pdev_adjust_mem_ranges(struct resource *res, struct pbus_set_ranges_data *ranges)
+{
+	unsigned long *end = &ranges->mem_end;
+
+	/*
+	 * We can't use this resources prefetch flag to determine which
+	 * region it belongs to - we may have allocated it in the non-
+	 * prefetchable region.  Use the parent resource instead.
+	 */
+	if (res->parent->flags & IORESOURCE_PREFETCH) {
+		end = &ranges->prefetch_end;
+		if (ranges->prefetch_valid == 0)
+			BUG();
+	}
+
+	if (*end < res->end)
+		*end = res->end;
+}
+
 static int __init
 pbus_assign_resources_sorted(struct pci_bus *bus,
 			     struct pbus_set_ranges_data *ranges)

@@ -89,9 +111,8 @@
 	for (list = head_mem.next; list;) {
 		res = list->res;
 		idx = res - &list->dev->resource[0];
-		if (pci_assign_resource(list->dev, idx) == 0
-		    && ranges->mem_end < res->end)
-			ranges->mem_end = res->end;
+		if (pci_assign_resource(list->dev, idx) == 0)
+			pdev_adjust_mem_ranges(res, ranges);
 		tmp = list;
 		list = list->next;
 		kfree(tmp);

@@ -113,9 +134,12 @@
 		ranges->io_end += 1;
 	if (ranges->mem_end == ranges->mem_start)
 		ranges->mem_end += 1;
+	if (ranges->prefetch_end == ranges->prefetch_start)
+		ranges->prefetch_end += 1;
 #endif
 	ranges->io_end = ROUND_UP(ranges->io_end, 4*1024);
 	ranges->mem_end = ROUND_UP(ranges->mem_end, 1024*1024);
+	ranges->prefetch_end = ROUND_UP(ranges->prefetch_end, 1024*1024);
 
 	return found_vga;
 }

@@ -130,15 +154,24 @@
 
 	if (!bridge || (bridge->class >> 8) != PCI_CLASS_BRIDGE_PCI)
 		return;
+
 	ranges.io_start = bus->resource[0]->start;
 	ranges.io_end = bus->resource[0]->end;
 	ranges.mem_start = bus->resource[1]->start;
 	ranges.mem_end = bus->resource[1]->end;
+	ranges.prefetch_start = bus->resource[2]->start;
+	ranges.prefetch_end = bus->resource[2]->end;
+
 	pcibios_fixup_pbus_ranges(bus, &ranges);
 
-	DBGC((KERN_ERR "PCI: Bus %d, bridge: %s\n", bus->number, bridge->name));
-	DBGC((KERN_ERR "  IO window: %04lx-%04lx\n", ranges.io_start, ranges.io_end));
-	DBGC((KERN_ERR "  MEM window: %08lx-%08lx\n", ranges.mem_start, ranges.mem_end));
+	DBGC((KERN_ERR "PCI: Bus %d, bridge: %s\n",
+		bus->number, bridge->name));
+	DBGC((KERN_ERR "  IO window: %08lx-%08lx\n",
+		ranges.io_start, ranges.io_end));
+	DBGC((KERN_ERR "  MEM window: %08lx-%08lx\n",
+		ranges.mem_start, ranges.mem_end));
+	DBGC((KERN_ERR "  PREFETCH window: %08lx-%08lx\n",
+		ranges.prefetch_start, ranges.prefetch_end));
 
 	/* Set up the top and bottom of the PCI I/O segment for this bus. */
 	pci_read_config_dword(bridge, PCI_IO_BASE, &l);

@@ -161,13 +197,15 @@
 	pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);
 
 	/* Set up PREF base/limit. */
-	l = (bus->resource[2]->start >> 16) & 0xfff0;
-	l |= bus->resource[2]->end & 0xfff00000;
+	l = (ranges.prefetch_start >> 16) & 0xfff0;
+	l |= ranges.prefetch_end & 0xfff00000;
 	pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
 
 	/* Check if we have VGA behind the bridge.
 	   Enable ISA in either case. */
-	l = (bus->resource[0]->flags & IORESOURCE_BUS_HAS_VGA) ? 0x0c : 0x04;
+	l = (bus->resource[0]->flags & IORESOURCE_BUS_HAS_VGA) ?
+		PCI_BRIDGE_CTL_VGA | PCI_BRIDGE_CTL_NO_ISA :
+		PCI_BRIDGE_CTL_NO_ISA;
 	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, l);
 }
 

@@ -195,11 +233,21 @@
 
 		b->resource[0]->start = ranges->io_start = ranges->io_end;
 		b->resource[1]->start = ranges->mem_start = ranges->mem_end;
+		if (ranges->prefetch_valid)
+			b->resource[2]->start = ranges->prefetch_start =
+					 ranges->prefetch_end;
 
 		pbus_assign_resources(b, ranges);
 
 		b->resource[0]->end = ranges->io_end - 1;
 		b->resource[1]->end = ranges->mem_end - 1;
+		if (ranges->prefetch_valid)
+			b->resource[2]->end = ranges->prefetch_end - 1;
+
+		request_resource(bus->resource[0], b->resource[0]);
+		request_resource(bus->resource[1], b->resource[1]);
+		if (ranges->prefetch_valid)
+			request_resource(bus->resource[2], b->resource[2]);
 
 		pci_setup_bridge(b);
 	}

@@ -219,6 +267,11 @@
 		ranges.mem_start = b->resource[1]->start + PCIBIOS_MIN_MEM;
 		ranges.io_end = ranges.io_start;
 		ranges.mem_end = ranges.mem_start;
+		ranges.prefetch_valid = b->resource[2] != NULL;
+		if (ranges.prefetch_valid) {
+			ranges.prefetch_start = b->resource[2]->start + PCIBIOS_MIN_MEM;
+			ranges.prefetch_end = ranges.prefetch_start;
+		}
 		ranges.found_vga = 0;
 		pbus_assign_resources(b, &ranges);
 	}
include/linux/pci.h +3 -1
@@ -460,9 +460,11 @@
 
 struct pbus_set_ranges_data
 {
-	int found_vga;
+	int found_vga:1;
+	int prefetch_valid:1;
 	unsigned long io_start, io_end;
 	unsigned long mem_start, mem_end;
+	unsigned long prefetch_start, prefetch_end;
 };
 
 struct pci_device_id {

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

#1 lore raw
Quoted 5 lines · Russell King

The following patch allows architectures to split the prefetchable memory regions from the non-prefetchable regions if they wish by supplying a third bus resource. This third resource must have the IORESOURCE_PREFETCH flag set. (Other parts of the PCI layer already assume this is true). Currently, some ARM based machines make use of this facility.

This looks fine, but I don't like the idea of artificial splitting the PCI memory region if we want prefetch support. I can imagine a situation where we need to set PCI memory window as small as possible, and proper splitting won't be easy in this case.
So I'm using different approach, a bit more complex, but [hopefully] more generic:
- pass 1. Allocate only prefetchable resources. At this point, we have IORESOURCE_MEM flag cleared for bridged buses.
- pass 2. Allocate IO and memory resources. If we have separate prefetchable region on the root bus, start at PCIBIOS_MIN_MEM as usual, if not - at the end of prefetchable area from pass 1.

The patch is available at ftp://ftp.park.msu.ru/ink/patches-2.5/prefetch.diff

Unfortunately, it depends on ftp://ftp.park.msu.ru/ink/patches-2.5/pci-2.5.diff

The latter is mostly setup-[bus,res].c reorganization, plus generic fast-back-to-back etc. support, and corresponding alpha specific stuff.
I'm planning to split it up and post for a discussion in next few days.

Ivan.

#2 lore raw
In reply to Russell King
Quoted 2 lines · Ivan Kokshaysky

This looks fine, but I don't like the idea of artificial splitting the PCI memory region if we want prefetch support.

Could you explain this a bit better. The reason we need to split the prefetchable regions from the non-prefetchable regions is that most bridges can only cope with one region which is prefetchable.

Also, some machines have a limited (sometimes fixed address and size) region that can only be used for prefetchable memory. How do you cater for this?

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

#3 lore raw
In reply to Russell King
Quoted 3 lines · Russell King

Could you explain this a bit better. The reason we need to split the prefetchable regions from the non-prefetchable regions is that most bridges can only cope with one region which is prefetchable.

No. Most host-to-pci bridges actually have only non-prefetchable memory region. That is, they don't generate Memory Read Line or Memory Read Multiple transactions at all.
For pci-to-pci bridges, prefetchable region is optional (as well as I/O), but most (if not all) bridges have it.
If we place memory-like resource behind the pci-pci bridge in the prefetchable region, the bridge can convert Memory Read command from its primary bus to Read Line/Multiply on the secondary bus, improving performance thus.

Quoted 3 lines

Also, some machines have a limited (sometimes fixed address and size) region that can only be used for prefetchable memory. How do you cater for this?

Just fine, if your root_bus->resource[2] is not NULL and initialized properly. If it's too small to hold all prefetchable resources, the rest will be allocated in the non-prefetch memory region.

Ivan.

#4 lore raw
In reply to Russell King
Quoted 3 lines · Ivan Kokshaysky

Just fine, if your root_bus->resource[2] is not NULL and initialized properly. If it's too small to hold all prefetchable resources, the rest will be allocated in the non-prefetch memory region.

Which is also what my patch does.

Ok, I'll wait until you put out your patch before further comment.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html