This patch series redesigns and adds new features to pciinit.c code.
[Patches 1-4] There are no more arrays of bases and counts in new implementation. The new implementation is based on dynamic allocation of pci_region_entry structures which are grouped into lists. Each pci_region_entry structure could be a PCI bar or a downstream PCI region (bridge). Each entry has a set of attributes: type (IO, MEM,PREFMEM), is64bit, size.
[Patch 5] Bridge regions are no longer rounded up to the next highest size - instead track alignment explicitly. This should improve the memory layout for bridge regions. Also, unused bridge regions will no longer be allocated any space.
[Patch 6] Patch takes into account PCI bar and ROM regions of PCI bridges
[Patches 7-12] Make pciinit.c 64bit aware. Support discovery and configuration of 64bit bars, with non-zero upper32 bits. Code allocates the 64bit PCI bars in high address range if they don't fir in 32bit range.
Kevin O'Konor (6): 0001-pciinit-Introduction-of-pci_region_entry-structure.patch 0002-pciinit-Move-bus-bar-asignment.patch 0004-pciinit-Use-sorted-order-allocation.patch 0005-pciinit-Track-region-alignment-explicitly.patch 0006-pciinit-bridges-can-have-two-regions-too.patch 0007-pciinit-Switch-to-64bit-variable-types.patch
Alexey Korolev (6): 0003-pciinit-Remove-size-element-from-pci_bus-r-structure.patch 0008-pciinit-Add-pci_region-structure.patch 0009-pciinit-64bit-capability-discovery-for-pci-bridges.patch 0010-Do-not-store-pci-region-stats-instead-calulate-the-s.patch 0011-Migrate-64bit-entries-to-64bit-pci-regions.patch 0012-Fix-64bit-PCI-issues-on-Windows.patch
src/acpi-dsdt.dsl | 7 + src/acpi-dsdt.hex | 64 +++++++- src/config.h | 2 + src/pci.h | 6 +- src/pciinit.c | 464 ++++++++++++++++++++++++++++++----------------------- 5 files changed, 325 insertions(+), 218 deletions(-)
The pci_region_entry structure is introduced. The pci_device->bars are removed. The information from pci_region_entry is used to program pci bars.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/pci.h | 5 -- src/pciinit.c | 116 +++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 81 insertions(+), 40 deletions(-)
diff --git a/src/pci.h b/src/pci.h index a2a5a4c..5598100 100644 --- a/src/pci.h +++ b/src/pci.h @@ -51,11 +51,6 @@ struct pci_device { u8 prog_if, revision; u8 header_type; u8 secondary_bus; - struct { - u32 addr; - u32 size; - int is64; - } bars[PCI_NUM_REGIONS];
// Local information on device. int have_driver; diff --git a/src/pciinit.c b/src/pciinit.c index 9f3fdd4..953f3bd 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -31,6 +31,15 @@ static const char *region_type_name[] = { [ PCI_REGION_TYPE_PREFMEM ] = "prefmem", };
+struct pci_region_entry { + struct pci_device *dev; + int bar; + u32 size; + int is64; + enum pci_region_type type; + struct pci_region_entry *next; +}; + struct pci_bus { struct { /* pci region stats */ @@ -41,6 +50,7 @@ struct pci_bus { /* pci region assignments */ u32 bases[32 - PCI_MEM_INDEX_SHIFT]; u32 base; + struct pci_region_entry *list; } r[PCI_REGION_TYPE_COUNT]; struct pci_device *bus_dev; }; @@ -352,19 +362,41 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) *size = (~(*val & mask)) + 1; }
-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size) +static struct pci_region_entry * +pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, + int bar, u32 size, int type, int is64) { - u32 index; + struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); + if (!entry) { + warn_noalloc(); + return NULL; + } + memset(entry, 0, sizeof(*entry)); + entry->dev = dev; + entry->bar = bar; + entry->size = size; + entry->is64 = is64; + entry->type = type; + // Insert into list in sorted order. + struct pci_region_entry **pprev; + for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) { + struct pci_region_entry *pos = *pprev; + if (pos->size < size) + break; + } + entry->next = *pprev; + *pprev = entry;
- index = pci_size_to_index(size, type); + int index = pci_size_to_index(size, type); size = pci_index_to_size(index, type); bus->r[type].count[index]++; bus->r[type].sum += size; if (bus->r[type].max < size) bus->r[type].max = size; + return entry; }
-static void pci_bios_check_devices(struct pci_bus *busses) +static int pci_bios_check_devices(struct pci_bus *busses) { dprintf(1, "PCI: check devices\n");
@@ -383,14 +415,15 @@ static void pci_bios_check_devices(struct pci_bus *busses) if (val == 0) continue;
- pci_bios_bus_reserve(bus, pci_addr_to_type(val), size); - pci->bars[i].addr = val; - pci->bars[i].size = size; - pci->bars[i].is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && - (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) - == PCI_BASE_ADDRESS_MEM_TYPE_64); + int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && + (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) + == PCI_BASE_ADDRESS_MEM_TYPE_64); + struct pci_region_entry *entry = pci_region_create_entry( + bus, pci, i, size, pci_addr_to_type(val), is64); + if (!entry) + return -1;
- if (pci->bars[i].is64) + if (is64) i++; } } @@ -410,7 +443,11 @@ static void pci_bios_check_devices(struct pci_bus *busses) if (s->r[type].size < limit) s->r[type].size = limit; s->r[type].size = pci_size_roundup(s->r[type].size); - pci_bios_bus_reserve(parent, type, s->r[type].size); + // entry->bar is -1 if the entry represents a bridge region + struct pci_region_entry *entry = pci_region_create_entry( + parent, s->bus_dev, -1, s->r[type].size, type, 0); + if (!entry) + return -1; } dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n", secondary_bus, @@ -418,6 +455,7 @@ static void pci_bios_check_devices(struct pci_bus *busses) s->r[PCI_REGION_TYPE_MEM].size, s->r[PCI_REGION_TYPE_PREFMEM].size); } + return 0; }
#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1) @@ -483,6 +521,24 @@ static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size) #define PCI_MEMORY_SHIFT 16 #define PCI_PREF_MEMORY_SHIFT 16
+static void +pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) +{ + u16 bdf = entry->dev->bdf; + struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; + if (entry->bar >= 0) { + u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size); + dprintf(1, "PCI: map device bdf=%02x:%02x.%x" + " bar %d, addr %08x, size %08x [%s]\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf), + entry->bar, addr, entry->size, region_type_name[entry->type]); + + pci_set_io_region_addr(entry->dev, entry->bar, addr); + if (entry->is64) + pci_set_io_region_addr(entry->dev, entry->bar + 1, 0); + } +} + static void pci_bios_map_devices(struct pci_bus *busses) { // Setup bases for root bus. @@ -526,28 +582,16 @@ static void pci_bios_map_devices(struct pci_bus *busses) }
// Map regions on each device. - struct pci_device *pci; - foreachpci(pci) { - if (pci->class == PCI_CLASS_BRIDGE_PCI) - continue; - u16 bdf = pci->bdf; - dprintf(1, "PCI: map device bdf=%02x:%02x.%x\n" - , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf)); - struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; - int i; - for (i = 0; i < PCI_NUM_REGIONS; i++) { - if (pci->bars[i].addr == 0) - continue; - - int type = pci_addr_to_type(pci->bars[i].addr); - u32 addr = pci_bios_bus_get_addr(bus, type, pci->bars[i].size); - dprintf(1, " bar %d, addr %x, size %x [%s]\n", - i, addr, pci->bars[i].size, region_type_name[type]); - pci_set_io_region_addr(pci, i, addr); - - if (pci->bars[i].is64) { - i++; - pci_set_io_region_addr(pci, i, 0); + int bus; + for (bus = 0; bus<=MaxPCIBus; bus++) { + int type; + for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { + struct pci_region_entry *entry = busses[bus].r[type].list; + while (entry) { + pci_region_map_one_entry(busses, entry); + struct pci_region_entry *next = entry->next; + free(entry); + entry = next; } } } @@ -588,7 +632,9 @@ pci_setup(void) return; } memset(busses, 0, sizeof(*busses) * (MaxPCIBus + 1)); - pci_bios_check_devices(busses); + if (pci_bios_check_devices(busses)) + return; + if (pci_bios_init_root_regions(&busses[0], start, end) != 0) { panic("PCI: out of address space\n"); }
Perform bus bar assignment at same time as normal bar assignment
Signed-off-by: Kevin O'Connor kevin@koconnor.net Signed-off-by: Alexey Korolev alexey.korolev@endace.com
--- src/pciinit.c | 53 ++++++++++++++++++----------------------------------- 1 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 953f3bd..74ade52 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -526,8 +526,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) { u16 bdf = entry->dev->bdf; struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; + u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size); if (entry->bar >= 0) { - u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size); dprintf(1, "PCI: map device bdf=%02x:%02x.%x" " bar %d, addr %08x, size %08x [%s]\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf), @@ -536,54 +536,37 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) pci_set_io_region_addr(entry->dev, entry->bar, addr); if (entry->is64) pci_set_io_region_addr(entry->dev, entry->bar + 1, 0); + return; } -} - -static void pci_bios_map_devices(struct pci_bus *busses) -{ - // Setup bases for root bus. - dprintf(1, "PCI: init bases bus 0 (primary)\n"); - pci_bios_init_bus_bases(&busses[0]); - - // Map regions on each secondary bus. - int secondary_bus; - for (secondary_bus=1; secondary_bus<=MaxPCIBus; secondary_bus++) { - struct pci_bus *s = &busses[secondary_bus]; - if (!s->bus_dev) - continue; - u16 bdf = s->bus_dev->bdf; - struct pci_bus *parent = &busses[pci_bdf_to_bus(bdf)]; - int type; - for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - s->r[type].base = pci_bios_bus_get_addr( - parent, type, s->r[type].size); - } - dprintf(1, "PCI: init bases bus %d (secondary)\n", secondary_bus); - pci_bios_init_bus_bases(s);
- u32 base = s->r[PCI_REGION_TYPE_IO].base; - u32 limit = base + s->r[PCI_REGION_TYPE_IO].size - 1; - pci_config_writeb(bdf, PCI_IO_BASE, base >> PCI_IO_SHIFT); + struct pci_bus *child_bus = &busses[entry->dev->secondary_bus]; + child_bus->r[entry->type].base = addr; + u32 limit = addr + entry->size - 1; + if (entry->type == PCI_REGION_TYPE_IO) { + pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT); pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0); pci_config_writeb(bdf, PCI_IO_LIMIT, limit >> PCI_IO_SHIFT); pci_config_writew(bdf, PCI_IO_LIMIT_UPPER16, 0); - - base = s->r[PCI_REGION_TYPE_MEM].base; - limit = base + s->r[PCI_REGION_TYPE_MEM].size - 1; - pci_config_writew(bdf, PCI_MEMORY_BASE, base >> PCI_MEMORY_SHIFT); + } + if (entry->type == PCI_REGION_TYPE_MEM) { + pci_config_writew(bdf, PCI_MEMORY_BASE, addr >> PCI_MEMORY_SHIFT); pci_config_writew(bdf, PCI_MEMORY_LIMIT, limit >> PCI_MEMORY_SHIFT); - - base = s->r[PCI_REGION_TYPE_PREFMEM].base; - limit = base + s->r[PCI_REGION_TYPE_PREFMEM].size - 1; - pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, base >> PCI_PREF_MEMORY_SHIFT); + } + if (entry->type == PCI_REGION_TYPE_PREFMEM) { + pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, addr >> PCI_PREF_MEMORY_SHIFT); pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> PCI_PREF_MEMORY_SHIFT); pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0); pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0); } +}
+static void pci_bios_map_devices(struct pci_bus *busses) +{ // Map regions on each device. int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { + dprintf(1, "PCI: init bases bus %d\n", bus); + pci_bios_init_bus_bases(&busses[bus]); int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { struct pci_region_entry *entry = busses[bus].r[type].list;
The 'size' element of pci_bus->r structure is no longer need as the information about bridge region size is already stored in pci_region_entry structure.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/pciinit.c | 20 ++++++++------------ 1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 74ade52..4617793 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -45,8 +45,6 @@ struct pci_bus { /* pci region stats */ u32 count[32 - PCI_MEM_INDEX_SHIFT]; u32 sum, max; - /* seconday bus region sizes */ - u32 size; /* pci region assignments */ u32 bases[32 - PCI_MEM_INDEX_SHIFT]; u32 base; @@ -439,21 +437,19 @@ static int pci_bios_check_devices(struct pci_bus *busses) for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u32 limit = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; - s->r[type].size = s->r[type].sum; - if (s->r[type].size < limit) - s->r[type].size = limit; - s->r[type].size = pci_size_roundup(s->r[type].size); + u32 size = s->r[type].sum; + if (size < limit) + size = limit; + size = pci_size_roundup(size); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry( - parent, s->bus_dev, -1, s->r[type].size, type, 0); + parent, s->bus_dev, -1, size, type, 0); if (!entry) return -1; + dprintf(1, "PCI: secondary bus %d size %x type %s\n", + entry->dev->secondary_bus, size, + region_type_name[entry->type]); } - dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n", - secondary_bus, - s->r[PCI_REGION_TYPE_IO].size, - s->r[PCI_REGION_TYPE_MEM].size, - s->r[PCI_REGION_TYPE_PREFMEM].size); } return 0; }
Use sorted order allocation scheme instead of array based count scheme.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/pciinit.c | 71 +++++--------------------------------------------------- 1 files changed, 7 insertions(+), 64 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 4617793..1b31177 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -12,9 +12,7 @@ #include "pci_regs.h" // PCI_COMMAND #include "xen.h" // usingXen
-#define PCI_IO_INDEX_SHIFT 2 -#define PCI_MEM_INDEX_SHIFT 12 - +#define PCI_DEVICE_MEM_MIN 0x1000 #define PCI_BRIDGE_IO_MIN 0x1000 #define PCI_BRIDGE_MEM_MIN 0x100000
@@ -43,36 +41,14 @@ struct pci_region_entry { struct pci_bus { struct { /* pci region stats */ - u32 count[32 - PCI_MEM_INDEX_SHIFT]; u32 sum, max; /* pci region assignments */ - u32 bases[32 - PCI_MEM_INDEX_SHIFT]; u32 base; struct pci_region_entry *list; } r[PCI_REGION_TYPE_COUNT]; struct pci_device *bus_dev; };
-static int pci_size_to_index(u32 size, enum pci_region_type type) -{ - int index = __fls(size); - int shift = (type == PCI_REGION_TYPE_IO) ? - PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT; - - if (index < shift) - index = shift; - index -= shift; - return index; -} - -static u32 pci_index_to_size(int index, enum pci_region_type type) -{ - int shift = (type == PCI_REGION_TYPE_IO) ? - PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT; - - return 0x1 << (index + shift); -} - static enum pci_region_type pci_addr_to_type(u32 addr) { if (addr & PCI_BASE_ADDRESS_SPACE_IO) @@ -385,9 +361,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->next = *pprev; *pprev = entry;
- int index = pci_size_to_index(size, type); - size = pci_index_to_size(index, type); - bus->r[type].count[index]++; bus->r[type].sum += size; if (bus->r[type].max < size) bus->r[type].max = size; @@ -413,11 +386,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) if (val == 0) continue;
+ int type = pci_addr_to_type(val); + if (type != PCI_REGION_TYPE_IO && size < PCI_DEVICE_MEM_MIN) + size = PCI_DEVICE_MEM_MIN; int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); struct pci_region_entry *entry = pci_region_create_entry( - bus, pci, i, size, pci_addr_to_type(val), is64); + bus, pci, i, size, type, is64); if (!entry) return -1;
@@ -481,38 +457,6 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) * BAR assignment ****************************************************************/
-static void pci_bios_init_bus_bases(struct pci_bus *bus) -{ - u32 base, newbase, size; - int type, i; - - for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - dprintf(1, " type %s max %x sum %x base %x\n", region_type_name[type], - bus->r[type].max, bus->r[type].sum, bus->r[type].base); - base = bus->r[type].base; - for (i = ARRAY_SIZE(bus->r[type].count)-1; i >= 0; i--) { - size = pci_index_to_size(i, type); - if (!bus->r[type].count[i]) - continue; - newbase = base + size * bus->r[type].count[i]; - dprintf(1, " size %8x: %d bar(s), %8x -> %8x\n", - size, bus->r[type].count[i], base, newbase - 1); - bus->r[type].bases[i] = base; - base = newbase; - } - } -} - -static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size) -{ - u32 index, addr; - - index = pci_size_to_index(size, type); - addr = bus->r[type].bases[index]; - bus->r[type].bases[index] += pci_index_to_size(index, type); - return addr; -} - #define PCI_IO_SHIFT 8 #define PCI_MEMORY_SHIFT 16 #define PCI_PREF_MEMORY_SHIFT 16 @@ -522,7 +466,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) { u16 bdf = entry->dev->bdf; struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; - u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size); + u32 addr = bus->r[entry->type].base; + bus->r[entry->type].base += entry->size; if (entry->bar >= 0) { dprintf(1, "PCI: map device bdf=%02x:%02x.%x" " bar %d, addr %08x, size %08x [%s]\n", @@ -561,8 +506,6 @@ static void pci_bios_map_devices(struct pci_bus *busses) // Map regions on each device. int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { - dprintf(1, "PCI: init bases bus %d\n", bus); - pci_bios_init_bus_bases(&busses[bus]); int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { struct pci_region_entry *entry = busses[bus].r[type].list;
Don't round up bridge regions to the next highest size - instead track alignment explicitly. This should improve the memory layout for bridge regions.
Also, unused bridge regions will no longer be allocated any space.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/pciinit.c | 41 ++++++++++++++++++----------------------- 1 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 1b31177..2bd4426 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -33,6 +33,7 @@ struct pci_region_entry { struct pci_device *dev; int bar; u32 size; + u32 align; int is64; enum pci_region_type type; struct pci_region_entry *next; @@ -41,7 +42,7 @@ struct pci_region_entry { struct pci_bus { struct { /* pci region stats */ - u32 sum, max; + u32 sum, align; /* pci region assignments */ u32 base; struct pci_region_entry *list; @@ -307,12 +308,6 @@ pci_bios_init_bus(void) * Bus sizing ****************************************************************/
-static u32 pci_size_roundup(u32 size) -{ - int index = __fls(size-1)+1; - return 0x1 << index; -} - static void pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) { @@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, - int bar, u32 size, int type, int is64) + int bar, u32 size, u32 align, int type, int is64) { struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); if (!entry) { @@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->dev = dev; entry->bar = bar; entry->size = size; + entry->align = align; entry->is64 = is64; entry->type = type; // Insert into list in sorted order. struct pci_region_entry **pprev; for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) { struct pci_region_entry *pos = *pprev; - if (pos->size < size) + if (pos->align < align || (pos->align == align && pos->size < size)) break; } entry->next = *pprev; *pprev = entry;
bus->r[type].sum += size; - if (bus->r[type].max < size) - bus->r[type].max = size; + if (bus->r[type].align < align) + bus->r[type].align = align; return entry; }
@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); struct pci_region_entry *entry = pci_region_create_entry( - bus, pci, i, size, type, is64); + bus, pci, i, size, size, type, is64); if (!entry) return -1;
@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - u32 limit = (type == PCI_REGION_TYPE_IO) ? + u32 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; - u32 size = s->r[type].sum; - if (size < limit) - size = limit; - size = pci_size_roundup(size); + if (s->r[type].align > align) + align = s->r[type].align; + u32 size = ALIGN(s->r[type].sum, align); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry( - parent, s->bus_dev, -1, size, type, 0); + parent, s->bus_dev, -1, size, align, type, 0); if (!entry) return -1; dprintf(1, "PCI: secondary bus %d size %x type %s\n", @@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1) +#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment) static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) @@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM; - if (bus->r[reg1].sum < bus->r[reg2].sum) { - // Swap regions so larger area is more likely to align well. + if (bus->r[reg1].align < bus->r[reg2].align) { + // Swap regions to improve alignment. reg1 = PCI_REGION_TYPE_MEM; reg2 = PCI_REGION_TYPE_PREFMEM; } - bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum, bus->r[reg2].max); + bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum, bus->r[reg2].align); bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum - , bus->r[reg1].max); + , bus->r[reg1].align); if (bus->r[reg1].base < start) // Memory range requested is larger than available. return -1;
Patch takes into account PCI bar and ROM regions of PCI bridges
Original patch by: Gerd Hoffmann kraxel@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net Signed-off-by: Alexey Korolev alexey.korolev@endace.com --- src/pci.h | 1 + src/pciinit.c | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/pci.h b/src/pci.h index 5598100..6be838c 100644 --- a/src/pci.h +++ b/src/pci.h @@ -5,6 +5,7 @@
#define PCI_ROM_SLOT 6 #define PCI_NUM_REGIONS 7 +#define PCI_BRIDGE_NUM_REGIONS 2
static inline u8 pci_bdf_to_bus(u16 bdf) { return bdf >> 8; diff --git a/src/pciinit.c b/src/pciinit.c index 2bd4426..9b521e3 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -370,13 +370,15 @@ static int pci_bios_check_devices(struct pci_bus *busses) // Calculate resources needed for regular (non-bus) devices. struct pci_device *pci; foreachpci(pci) { - if (pci->class == PCI_CLASS_BRIDGE_PCI) { + if (pci->class == PCI_CLASS_BRIDGE_PCI) busses[pci->secondary_bus].bus_dev = pci; - continue; - } + struct pci_bus *bus = &busses[pci_bdf_to_bus(pci->bdf)]; int i; for (i = 0; i < PCI_NUM_REGIONS; i++) { + if ((pci->class == PCI_CLASS_BRIDGE_PCI) && + (i >= PCI_BRIDGE_NUM_REGIONS && i < PCI_ROM_SLOT)) + continue; u32 val, size; pci_bios_get_bar(pci, i, &val, &size); if (val == 0)
Switch to 64bit variable types. Add parsing 64bit bars.
Original patch by: Gerd Hoffmann kraxel@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/pciinit.c | 116 ++++++++++++++++++++++++++++++--------------------------- 1 files changed, 61 insertions(+), 55 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 9b521e3..bc603c7 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -32,8 +32,8 @@ static const char *region_type_name[] = { struct pci_region_entry { struct pci_device *dev; int bar; - u32 size; - u32 align; + u64 size; + u64 align; int is64; enum pci_region_type type; struct pci_region_entry *next; @@ -42,23 +42,14 @@ struct pci_region_entry { struct pci_bus { struct { /* pci region stats */ - u32 sum, align; + u64 sum, align; /* pci region assignments */ - u32 base; + u64 base; struct pci_region_entry *list; } r[PCI_REGION_TYPE_COUNT]; struct pci_device *bus_dev; };
-static enum pci_region_type pci_addr_to_type(u32 addr) -{ - if (addr & PCI_BASE_ADDRESS_SPACE_IO) - return PCI_REGION_TYPE_IO; - if (addr & PCI_BASE_ADDRESS_MEM_PREFETCH) - return PCI_REGION_TYPE_PREFMEM; - return PCI_REGION_TYPE_MEM; -} - static u32 pci_bar(struct pci_device *pci, int region_num) { if (region_num != PCI_ROM_SLOT) { @@ -71,9 +62,12 @@ static u32 pci_bar(struct pci_device *pci, int region_num) }
static void -pci_set_io_region_addr(struct pci_device *pci, int region_num, u32 addr) +pci_set_io_region_addr(struct pci_device *pci, int bar, u64 addr, int is64) { - pci_config_writel(pci->bdf, pci_bar(pci, region_num), addr); + u32 ofs = pci_bar(pci, bar); + pci_config_writel(pci->bdf, ofs, addr); + if (is64) + pci_config_writel(pci->bdf, ofs + 4, addr >> 32); }
@@ -126,10 +120,10 @@ static const struct pci_device_id pci_isa_bridge_tbl[] = { static void storage_ide_init(struct pci_device *pci, void *arg) { /* IDE: we map it as in ISA mode */ - pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE); - pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE); - pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE); - pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE); + pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE, 0); + pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE, 0); + pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE, 0); + pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE, 0); }
/* PIIX3/PIIX4 IDE */ @@ -143,13 +137,13 @@ static void piix_ide_init(struct pci_device *pci, void *arg) static void pic_ibm_init(struct pci_device *pci, void *arg) { /* PIC, IBM, MPIC & MPIC2 */ - pci_set_io_region_addr(pci, 0, 0x80800000 + 0x00040000); + pci_set_io_region_addr(pci, 0, 0x80800000 + 0x00040000, 0); }
static void apple_macio_init(struct pci_device *pci, void *arg) { /* macio bridge */ - pci_set_io_region_addr(pci, 0, 0x80800000); + pci_set_io_region_addr(pci, 0, 0x80800000, 0); }
static const struct pci_device_id pci_class_tbl[] = { @@ -309,31 +303,51 @@ pci_bios_init_bus(void) ****************************************************************/
static void -pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) +pci_bios_get_bar(struct pci_device *pci, int bar, + int *ptype, u64 *psize, int *pis64) { u32 ofs = pci_bar(pci, bar); u16 bdf = pci->bdf; u32 old = pci_config_readl(bdf, ofs); - u32 mask; + int is64 = 0, type = PCI_REGION_TYPE_MEM; + u64 mask;
if (bar == PCI_ROM_SLOT) { mask = PCI_ROM_ADDRESS_MASK; pci_config_writel(bdf, ofs, mask); } else { - if (old & PCI_BASE_ADDRESS_SPACE_IO) + if (old & PCI_BASE_ADDRESS_SPACE_IO) { mask = PCI_BASE_ADDRESS_IO_MASK; - else + type = PCI_REGION_TYPE_IO; + } else { mask = PCI_BASE_ADDRESS_MEM_MASK; + if (old & PCI_BASE_ADDRESS_MEM_PREFETCH) + type = PCI_REGION_TYPE_PREFMEM; + is64 = ((old & PCI_BASE_ADDRESS_MEM_TYPE_MASK) + == PCI_BASE_ADDRESS_MEM_TYPE_64); + } pci_config_writel(bdf, ofs, ~0); } - *val = pci_config_readl(bdf, ofs); + u64 val = pci_config_readl(bdf, ofs); pci_config_writel(bdf, ofs, old); - *size = (~(*val & mask)) + 1; + if (is64) { + u32 hold = pci_config_readl(bdf, ofs + 4); + pci_config_writel(bdf, ofs + 4, ~0); + u32 high = pci_config_readl(bdf, ofs + 4); + pci_config_writel(bdf, ofs + 4, hold); + val |= ((u64)high << 32); + mask |= ((u64)0xffffffff << 32); + *psize = (~(val & mask)) + 1; + } else { + *psize = ((~(val & mask)) + 1) & 0xffffffff; + } + *ptype = type; + *pis64 = is64; }
static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, - int bar, u32 size, u32 align, int type, int is64) + int bar, u64 size, u64 align, int type, int is64) { struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); if (!entry) { @@ -379,17 +393,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) if ((pci->class == PCI_CLASS_BRIDGE_PCI) && (i >= PCI_BRIDGE_NUM_REGIONS && i < PCI_ROM_SLOT)) continue; - u32 val, size; - pci_bios_get_bar(pci, i, &val, &size); - if (val == 0) + int type, is64; + u64 size; + pci_bios_get_bar(pci, i, &type, &size, &is64); + if (size == 0) continue;
- int type = pci_addr_to_type(val); if (type != PCI_REGION_TYPE_IO && size < PCI_DEVICE_MEM_MIN) size = PCI_DEVICE_MEM_MIN; - int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && - (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) - == PCI_BASE_ADDRESS_MEM_TYPE_64); struct pci_region_entry *entry = pci_region_create_entry( bus, pci, i, size, size, type, is64); if (!entry) @@ -409,17 +420,17 @@ static int pci_bios_check_devices(struct pci_bus *busses) struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - u32 align = (type == PCI_REGION_TYPE_IO) ? + u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; if (s->r[type].align > align) align = s->r[type].align; - u32 size = ALIGN(s->r[type].sum, align); + u64 size = ALIGN(s->r[type].sum, align); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry( parent, s->bus_dev, -1, size, align, type, 0); if (!entry) return -1; - dprintf(1, "PCI: secondary bus %d size %x type %s\n", + dprintf(1, "PCI: secondary bus %d size %08llx type %s\n", entry->dev->secondary_bus, size, region_type_name[entry->type]); } @@ -430,8 +441,11 @@ static int pci_bios_check_devices(struct pci_bus *busses) #define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment) -static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) +static void pci_bios_init_root_regions(struct pci_bus *bus) { + u64 start = BUILD_PCIMEM_START; + u64 end = BUILD_PCIMEM_END; + bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM; @@ -445,8 +459,7 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) , bus->r[reg1].align); if (bus->r[reg1].base < start) // Memory range requested is larger than available. - return -1; - return 0; + panic("PCI: out of address space\n"); }
@@ -463,23 +476,21 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) { u16 bdf = entry->dev->bdf; struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; - u32 addr = bus->r[entry->type].base; + u64 addr = bus->r[entry->type].base; bus->r[entry->type].base += entry->size; if (entry->bar >= 0) { dprintf(1, "PCI: map device bdf=%02x:%02x.%x" - " bar %d, addr %08x, size %08x [%s]\n", + " bar %d, addr %08llx, size %08llx [%s]\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf), entry->bar, addr, entry->size, region_type_name[entry->type]);
- pci_set_io_region_addr(entry->dev, entry->bar, addr); - if (entry->is64) - pci_set_io_region_addr(entry->dev, entry->bar + 1, 0); + pci_set_io_region_addr(entry->dev, entry->bar, addr, entry->is64); return; }
struct pci_bus *child_bus = &busses[entry->dev->secondary_bus]; child_bus->r[entry->type].base = addr; - u32 limit = addr + entry->size - 1; + u64 limit = addr + entry->size - 1; if (entry->type == PCI_REGION_TYPE_IO) { pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT); pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0); @@ -493,8 +504,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) if (entry->type == PCI_REGION_TYPE_PREFMEM) { pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, addr >> PCI_PREF_MEMORY_SHIFT); pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> PCI_PREF_MEMORY_SHIFT); - pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0); - pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0); + pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, addr >> 32); + pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, limit >> 32); } }
@@ -532,9 +543,6 @@ pci_setup(void)
dprintf(3, "pci setup\n");
- u32 start = BUILD_PCIMEM_START; - u32 end = BUILD_PCIMEM_END; - dprintf(1, "=== PCI bus & bridge init ===\n"); if (pci_probe_host() != 0) { return; @@ -554,9 +562,7 @@ pci_setup(void) if (pci_bios_check_devices(busses)) return;
- if (pci_bios_init_root_regions(&busses[0], start, end) != 0) { - panic("PCI: out of address space\n"); - } + pci_bios_init_root_regions(&busses[0]);
dprintf(1, "=== PCI new allocation pass #2 ===\n"); pci_bios_map_devices(busses);
The pci_region structure is added. Move setting of bus base address to pci_region_map_entries.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com --- src/pciinit.c | 50 ++++++++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index bc603c7..f185cbd 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -39,14 +39,16 @@ struct pci_region_entry { struct pci_region_entry *next; };
+struct pci_region { + /* pci region stats */ + u64 sum, align; + /* pci region assignments */ + u64 base; + struct pci_region_entry *list; +}; + struct pci_bus { - struct { - /* pci region stats */ - u64 sum, align; - /* pci region assignments */ - u64 base; - struct pci_region_entry *list; - } r[PCI_REGION_TYPE_COUNT]; + struct pci_region r[PCI_REGION_TYPE_COUNT]; struct pci_device *bus_dev; };
@@ -472,12 +474,9 @@ static void pci_bios_init_root_regions(struct pci_bus *bus) #define PCI_PREF_MEMORY_SHIFT 16
static void -pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) +pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr) { u16 bdf = entry->dev->bdf; - struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)]; - u64 addr = bus->r[entry->type].base; - bus->r[entry->type].base += entry->size; if (entry->bar >= 0) { dprintf(1, "PCI: map device bdf=%02x:%02x.%x" " bar %d, addr %08llx, size %08llx [%s]\n", @@ -488,8 +487,6 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) return; }
- struct pci_bus *child_bus = &busses[entry->dev->secondary_bus]; - child_bus->r[entry->type].base = addr; u64 limit = addr + entry->size - 1; if (entry->type == PCI_REGION_TYPE_IO) { pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT); @@ -509,21 +506,30 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry) } }
+static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r) +{ + struct pci_region_entry *entry = r->list; + while(entry) { + u64 addr = r->base; + r->base += entry->size; + if (entry->bar == -1) + // Update bus base address if entry is a bridge region + busses[entry->dev->secondary_bus].r[entry->type].base = addr; + pci_region_map_one_entry(entry, addr); + struct pci_region_entry *next = entry->next; + free(entry); + entry = next; + } +} + static void pci_bios_map_devices(struct pci_bus *busses) { // Map regions on each device. int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { int type; - for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - struct pci_region_entry *entry = busses[bus].r[type].list; - while (entry) { - pci_region_map_one_entry(busses, entry); - struct pci_region_entry *next = entry->next; - free(entry); - entry = next; - } - } + for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) + pci_region_map_entries(busses, &busses[bus].r[type]); } }
Add discovery if bridge region is 64bit is capable.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com --- src/pciinit.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index f185cbd..0d66dbe 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -347,6 +347,28 @@ pci_bios_get_bar(struct pci_device *pci, int bar, *pis64 = is64; }
+static int pci_bios_bridge_region_is64(struct pci_region *r, + struct pci_device *pci, int type) +{ + if (type != PCI_REGION_TYPE_PREFMEM) + return 0; + u32 pmem = pci_config_readl(pci->bdf, PCI_PREF_MEMORY_BASE); + if (!pmem) { + pci_config_writel(pci->bdf, PCI_PREF_MEMORY_BASE, 0xfff0fff0); + pmem = pci_config_readl(pci->bdf, PCI_PREF_MEMORY_BASE); + pci_config_writel(pci->bdf, PCI_PREF_MEMORY_BASE, 0x0); + } + if ((pmem & PCI_PREF_RANGE_TYPE_MASK) != PCI_PREF_RANGE_TYPE_64) + return 0; + struct pci_region_entry *entry = r->list; + while (entry) { + if (!entry->is64) + return 0; + entry = entry->next; + } + return 1; +} + static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, int bar, u64 size, u64 align, int type, int is64) @@ -427,9 +449,11 @@ static int pci_bios_check_devices(struct pci_bus *busses) if (s->r[type].align > align) align = s->r[type].align; u64 size = ALIGN(s->r[type].sum, align); + int is64 = pci_bios_bridge_region_is64(&s->r[type], + s->bus_dev, type); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry( - parent, s->bus_dev, -1, size, align, type, 0); + parent, s->bus_dev, -1, size, align, type, is64); if (!entry) return -1; dprintf(1, "PCI: secondary bus %d size %08llx type %s\n",
Do not store pci region stats - instead calulate the sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com --- src/pciinit.c | 57 +++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 0d66dbe..a6cf98b 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -40,8 +40,6 @@ struct pci_region_entry { };
struct pci_region { - /* pci region stats */ - u64 sum, align; /* pci region assignments */ u64 base; struct pci_region_entry *list; @@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct pci_region *r, return 1; }
+static u64 pci_region_align(struct pci_region *r) +{ + if (!r->list) + return 1; + // The first entry in the sorted list has the largest alignment + return r->list->align; +} + +static u64 pci_region_sum(struct pci_region *r) +{ + struct pci_region_entry *entry = r->list; + u64 sum = 0; + while (entry) { + sum += entry->size; + entry = entry->next; + } + return sum; +} + static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, int bar, u64 size, u64 align, int type, int is64) @@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, } entry->next = *pprev; *pprev = entry; - - bus->r[type].sum += size; - if (bus->r[type].align < align) - bus->r[type].align = align; return entry; }
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; - if (s->r[type].align > align) - align = s->r[type].align; - u64 size = ALIGN(s->r[type].sum, align); + if (pci_region_align(&s->r[type]) > align) + align = pci_region_align(&s->r[type]); + u64 size = ALIGN(pci_region_sum(&s->r[type]), align); int is64 = pci_bios_bridge_region_is64(&s->r[type], s->bus_dev, type); // entry->bar is -1 if the entry represents a bridge region @@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1) - // Setup region bases (given the regions' size and alignment) static void pci_bios_init_root_regions(struct pci_bus *bus) { - u64 start = BUILD_PCIMEM_START; - u64 end = BUILD_PCIMEM_END; - bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
- int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM; - if (bus->r[reg1].align < bus->r[reg2].align) { + struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM]; + struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM]; + + if (pci_region_align(r_start) < pci_region_align(r_end)) { // Swap regions to improve alignment. - reg1 = PCI_REGION_TYPE_MEM; - reg2 = PCI_REGION_TYPE_PREFMEM; + r_end = r_start; + r_start = &bus->r[PCI_REGION_TYPE_PREFMEM]; } - bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum, bus->r[reg2].align); - bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum - , bus->r[reg1].align); - if (bus->r[reg1].base < start) + r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)), + pci_region_align(r_end)); + r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)), + pci_region_align(r_start)); + + if ((r_start->base < BUILD_PCIMEM_START) || + (r_start->base > BUILD_PCIMEM_END)) // Memory range requested is larger than available. panic("PCI: out of address space\n"); }
Do not store pci region stats - instead calulate the sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com
--- src/pciinit.c | 57 +++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 0d66dbe..a6cf98b 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -40,8 +40,6 @@ struct pci_region_entry { };
struct pci_region { - /* pci region stats */ - u64 sum, align; /* pci region assignments */ u64 base; struct pci_region_entry *list; @@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct pci_region *r, return 1; }
+static u64 pci_region_align(struct pci_region *r) +{ + if (!r->list) + return 1; + // The first entry in the sorted list has the largest alignment + return r->list->align; +} + +static u64 pci_region_sum(struct pci_region *r) +{ + struct pci_region_entry *entry = r->list; + u64 sum = 0; + while (entry) { + sum += entry->size; + entry = entry->next; + } + return sum; +} + static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, int bar, u64 size, u64 align, int type, int is64) @@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, } entry->next = *pprev; *pprev = entry; - - bus->r[type].sum += size; - if (bus->r[type].align < align) - bus->r[type].align = align; return entry; }
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; - if (s->r[type].align > align) - align = s->r[type].align; - u64 size = ALIGN(s->r[type].sum, align); + if (pci_region_align(&s->r[type]) > align) + align = pci_region_align(&s->r[type]); + u64 size = ALIGN(pci_region_sum(&s->r[type]), align); int is64 = pci_bios_bridge_region_is64(&s->r[type], s->bus_dev, type); // entry->bar is -1 if the entry represents a bridge region @@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1) - // Setup region bases (given the regions' size and alignment) static void pci_bios_init_root_regions(struct pci_bus *bus) { - u64 start = BUILD_PCIMEM_START; - u64 end = BUILD_PCIMEM_END; - bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
- int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM; - if (bus->r[reg1].align < bus->r[reg2].align) { + struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM]; + struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM]; + + if (pci_region_align(r_start) < pci_region_align(r_end)) { // Swap regions to improve alignment. - reg1 = PCI_REGION_TYPE_MEM; - reg2 = PCI_REGION_TYPE_PREFMEM; + r_end = r_start; + r_start = &bus->r[PCI_REGION_TYPE_PREFMEM]; } - bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum, bus->r[reg2].align); - bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum - , bus->r[reg1].align); - if (bus->r[reg1].base < start) + r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)), + pci_region_align(r_end)); + r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)), + pci_region_align(r_start)); + + if ((r_start->base < BUILD_PCIMEM_START) || + (r_start->base > BUILD_PCIMEM_END)) // Memory range requested is larger than available. panic("PCI: out of address space\n"); }
Migrate 64bit entries to 64bit pci regions if they do not fit in 32bit range.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com --- src/config.h | 2 ++ src/pciinit.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/config.h b/src/config.h index b0187a4..bbacae7 100644 --- a/src/config.h +++ b/src/config.h @@ -47,6 +47,8 @@
#define BUILD_PCIMEM_START 0xe0000000 #define BUILD_PCIMEM_END 0xfec00000 /* IOAPIC is mapped at */ +#define BUILD_PCIMEM64_START 0x8000000000ULL +#define BUILD_PCIMEM64_END 0x10000000000ULL
#define BUILD_IOAPIC_ADDR 0xfec00000 #define BUILD_HPET_ADDRESS 0xfed00000 diff --git a/src/pciinit.c b/src/pciinit.c index a6cf98b..3d7640b 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -386,6 +386,31 @@ static u64 pci_region_sum(struct pci_region *r) return sum; }
+static void pci_region_migrate_64bit_entries(struct pci_region *from, + struct pci_region *to) +{ + struct pci_region_entry **pprev = &from->list; + struct pci_region_entry **last = &to->list; + while(*pprev) { + if ((*pprev)->is64) { + struct pci_region_entry *entry; + entry = *pprev; + /* Delete the entry and move next */ + *pprev = (*pprev)->next; + /* Add entry at tail to keep a sorted order */ + entry->next = NULL; + if (*last) { + (*last)->next = entry; + last = &(*last)->next; + } + else + (*last) = entry; + } + else + pprev = &(*pprev)->next; + } +} + static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, int bar, u64 size, u64 align, int type, int is64) @@ -478,7 +503,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) }
// Setup region bases (given the regions' size and alignment) -static void pci_bios_init_root_regions(struct pci_bus *bus) +static int pci_bios_init_root_regions(struct pci_bus *bus) { bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
@@ -498,7 +523,8 @@ static void pci_bios_init_root_regions(struct pci_bus *bus) if ((r_start->base < BUILD_PCIMEM_START) || (r_start->base > BUILD_PCIMEM_END)) // Memory range requested is larger than available. - panic("PCI: out of address space\n"); + return -1; + return 0; }
@@ -561,6 +587,24 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)
static void pci_bios_map_devices(struct pci_bus *busses) { + if (pci_bios_init_root_regions(busses)) { + struct pci_region r64_mem, r64_pref; + r64_mem.list = NULL; + r64_pref.list = NULL; + pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM], + &r64_mem); + pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM], + &r64_pref); + + if (pci_bios_init_root_regions(busses)) + panic("PCI: out of address space\n"); + + r64_mem.base = BUILD_PCIMEM64_START; + r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem), + pci_region_align(&r64_pref)); + pci_region_map_entries(busses, &r64_mem); + pci_region_map_entries(busses, &r64_pref); + } // Map regions on each device. int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { @@ -605,8 +649,6 @@ pci_setup(void) if (pci_bios_check_devices(busses)) return;
- pci_bios_init_root_regions(&busses[0]); - dprintf(1, "=== PCI new allocation pass #2 ===\n"); pci_bios_map_devices(busses);
This patch solves issues on Windows guests, when 64bit BAR's are present. It is also helpful on Linux guests when use_crs kernel boot option is set.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com Signed-off-by: Michael S. Tsirkin mst@redhat.com --- src/acpi-dsdt.dsl | 7 +++++ src/acpi-dsdt.hex | 64 +++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 4bdc268..4a18617 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -175,6 +175,13 @@ DefinitionBlock ( 0x00000000, // Address Translation Offset 0x1EC00000, // Address Length ,, , AddressRangeMemory, TypeStatic) + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite, + 0x00000000, // Address Space Granularity + 0x8000000000, // Address Range Minimum + 0xFFFFFFFFFF, // Address Range Maximum + 0x00000000, // Address Translation Offset + 0x8000000000, // Address Length + ,, , AddressRangeMemory, TypeStatic) }) } } diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex index a4af597..07f0e18 100644 --- a/src/acpi-dsdt.hex +++ b/src/acpi-dsdt.hex @@ -3,12 +3,12 @@ static unsigned char AmlCode[] = { 0x53, 0x44, 0x54, -0x21, +0x4f, 0x11, 0x0, 0x0, 0x1, -0xe8, +0xca, 0x42, 0x58, 0x50, @@ -110,16 +110,16 @@ static unsigned char AmlCode[] = { 0x47, 0x42, 0x10, -0x44, -0x81, +0x42, +0x84, 0x5f, 0x53, 0x42, 0x5f, 0x5b, 0x82, -0x4c, -0x80, +0x4a, +0x83, 0x50, 0x43, 0x49, @@ -2064,10 +2064,10 @@ static unsigned char AmlCode[] = { 0x52, 0x53, 0x11, -0x42, -0x7, +0x40, 0xa, -0x6e, +0xa, +0x9c, 0x88, 0xd, 0x0, @@ -2176,6 +2176,52 @@ static unsigned char AmlCode[] = { 0x0, 0xc0, 0x1e, +0x8a, +0x2b, +0x0, +0x0, +0xc, +0x3, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x80, +0x0, +0x0, +0x0, +0xff, +0xff, +0xff, +0xff, +0xff, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x0, +0x80, +0x0, +0x0, +0x0, 0x79, 0x0, 0x10,
A flowed text. Please apply another [10/12] in this series.
On Tue, 2012-04-24 at 18:23 +1200, Alexey Korolev wrote:
Do not store pci region stats - instead calulate the sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.korolev@endace.com
src/pciinit.c | 57 +++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 0d66dbe..a6cf98b 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -40,8 +40,6 @@ struct pci_region_entry { };
struct pci_region {
- /* pci region stats */
- u64 sum, align; /* pci region assignments */ u64 base; struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct pci_region *r, return 1; }
+static u64 pci_region_align(struct pci_region *r) +{
- if (!r->list)
return 1;- // The first entry in the sorted list has the largest alignment
- return r->list->align;
+}
+static u64 pci_region_sum(struct pci_region *r) +{
- struct pci_region_entry *entry = r->list;
- u64 sum = 0;
- while (entry) {
sum += entry->size;entry = entry->next;- }
- return sum;
+}
static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, int bar, u64 size, u64 align, int type, int is64) @@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, } entry->next = *pprev; *pprev = entry;
- bus->r[type].sum += size;
- if (bus->r[type].align < align)
bus->r[type].align = align;}
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
if (s->r[type].align > align)align = s->r[type].align;u64 size = ALIGN(s->r[type].sum, align);
if (pci_region_align(&s->r[type]) > align)align = pci_region_align(&s->r[type]);u64 size = ALIGN(pci_region_sum(&s->r[type]), align); int is64 = pci_bios_bridge_region_is64(&s->r[type], s->bus_dev, type); // entry->bar is -1 if the entry represents a bridge region@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment) static void pci_bios_init_root_regions(struct pci_bus *bus) {
u64 start = BUILD_PCIMEM_START;
u64 end = BUILD_PCIMEM_END;
bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
if (bus->r[reg1].align < bus->r[reg2].align) {
- struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM];
- struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM];
- if (pci_region_align(r_start) < pci_region_align(r_end)) { // Swap regions to improve alignment.
reg1 = PCI_REGION_TYPE_MEM;reg2 = PCI_REGION_TYPE_PREFMEM;
r_end = r_start;r_start = &bus->r[PCI_REGION_TYPE_PREFMEM];
- bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].align);
- bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
, bus->r[reg1].align);- if (bus->r[reg1].base < start)
- r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END -
pci_region_sum(r_end)),
pci_region_align(r_end));- r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
pci_region_align(r_start));- if ((r_start->base < BUILD_PCIMEM_START) ||
(r_start->base > BUILD_PCIMEM_END)) // Memory range requested is larger than available. panic("PCI: out of address space\n");}
On 04/24/12 08:17, Alexey Korolev wrote:
Don't round up bridge regions to the next highest size - instead track alignment explicitly. This should improve the memory layout for bridge regions.
This one got mangled too:
Applying: pciinit: Track region alignment explicitly. fatal: corrupt patch at line 40 Patch failed at 0005 pciinit: Track region alignment explicitly.
Do you have a git tree I can pull from for testing?
thanks, Gerd
Also, unused bridge regions will no longer be allocated any space.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
src/pciinit.c | 41 ++++++++++++++++++----------------------- 1 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 1b31177..2bd4426 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -33,6 +33,7 @@ struct pci_region_entry { struct pci_device *dev; int bar; u32 size;
- u32 align; int is64; enum pci_region_type type; struct pci_region_entry *next;
@@ -41,7 +42,7 @@ struct pci_region_entry { struct pci_bus { struct { /* pci region stats */
u32 sum, max;
u32 sum, align; /* pci region assignments */ u32 base; struct pci_region_entry *list;@@ -307,12 +308,6 @@ pci_bios_init_bus(void)
- Bus sizing
****************************************************************/
-static u32 pci_size_roundup(u32 size) -{
- int index = __fls(size-1)+1;
- return 0x1 << index;
-}
static void pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) { @@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u32 size, int type, int is64)
int bar, u32 size, u32 align, int type, intis64) { struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); if (!entry) { @@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->dev = dev; entry->bar = bar; entry->size = size;
- entry->align = align; entry->is64 = is64; entry->type = type; // Insert into list in sorted order. struct pci_region_entry **pprev; for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) { struct pci_region_entry *pos = *pprev;
if (pos->size < size)
if (pos->align < align || (pos->align == align && pos->size <size)) break; } entry->next = *pprev; *pprev = entry;
bus->r[type].sum += size;
- if (bus->r[type].max < size)
bus->r[type].max = size;
- if (bus->r[type].align < align)
bus->r[type].align = align;}
@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); struct pci_region_entry *entry = pci_region_create_entry(
bus, pci, i, size, type, is64);
bus, pci, i, size, size, type, is64); if (!entry) return -1;@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
u32 limit = (type == PCI_REGION_TYPE_IO) ?
u32 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
u32 size = s->r[type].sum;if (size < limit)size = limit;size = pci_size_roundup(size);
if (s->r[type].align > align)align = s->r[type].align;u32 size = ALIGN(s->r[type].sum, align); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry(
parent, s->bus_dev, -1, size, type, 0);
parent, s->bus_dev, -1, size, align, type, 0); if (!entry) return -1; dprintf(1, "PCI: secondary bus %d size %x type %s\n",@@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1) +#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment) static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) @@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
- if (bus->r[reg1].sum < bus->r[reg2].sum) {
// Swap regions so larger area is more likely to align well.
- if (bus->r[reg1].align < bus->r[reg2].align) {
// Swap regions to improve alignment. reg1 = PCI_REGION_TYPE_MEM; reg2 = PCI_REGION_TYPE_PREFMEM;
- bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].max);
- bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].align); bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
, bus->r[reg1].max);
, bus->r[reg1].align);
On Tue, Apr 24, 2012 at 06:24:27PM +1200, Alexey Korolev wrote:
Do not store pci region stats - instead calulate the sum and alignment on demand.
[...]
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
if (s->r[type].align > align)align = s->r[type].align;u64 size = ALIGN(s->r[type].sum, align);
if (pci_region_align(&s->r[type]) > align)align = pci_region_align(&s->r[type]);u64 size = ALIGN(pci_region_sum(&s->r[type]), align);
[...]
- r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)),
pci_region_align(r_end));- r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
pci_region_align(r_start));
I'd avoid making function calls in the parameter of a macro (it can be executed multiple times and it's non-obvious).
-Kevin
On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
Migrate 64bit entries to 64bit pci regions if they do not fit in 32bit range.
[...]
+static void pci_region_migrate_64bit_entries(struct pci_region *from,
struct pci_region *to)+{
- struct pci_region_entry **pprev = &from->list;
- struct pci_region_entry **last = &to->list;
- while(*pprev) {
if ((*pprev)->is64) {struct pci_region_entry *entry;entry = *pprev;/* Delete the entry and move next */*pprev = (*pprev)->next;/* Add entry at tail to keep a sorted order */entry->next = NULL;if (*last) {(*last)->next = entry;last = &(*last)->next;}else(*last) = entry;}elsepprev = &(*pprev)->next;- }
+}
It should be possible to simplify this - something like (untested):
static void pci_region_migrate_64bit_entries(struct pci_region *from, struct pci_region *to) { struct pci_region_entry **pprev = &from->list, **last = &to->list; for (; *pprev; pprev = &(*pprev)->next) { struct pci_region_entry *entry = *pprev; if (!entry->is64) continue; // Move from source list to dest list. *pprev = entry->next; entry->next = NULL; *last = entry; last = &entry->next; } }
[...]
static void pci_bios_map_devices(struct pci_bus *busses) {
- if (pci_bios_init_root_regions(busses)) {
struct pci_region r64_mem, r64_pref;r64_mem.list = NULL;r64_pref.list = NULL;pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],&r64_mem);pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],&r64_pref);if (pci_bios_init_root_regions(busses))panic("PCI: out of address space\n");r64_mem.base = BUILD_PCIMEM64_START;r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),pci_region_align(&r64_pref));
There should be a check to see if the regions fit. Maybe pass start/end into pci_bios_init_root_regions() and call it again for the
4g region?
pci_region_map_entries(busses, &r64_mem);pci_region_map_entries(busses, &r64_pref);- } // Map regions on each device.
This doesn't look right to me. This will map the devices on bus 0 to the proper >4g address, but devices on any subsequent bus will use busses[0].r[].base which will be reset to the <4gig address. Perhaps pull base out of pci_region and make pci_region_map_entries() recursive?
-Kevin
On 24/04/12 18:56, Gerd Hoffmann wrote:
On 04/24/12 08:17, Alexey Korolev wrote:
Don't round up bridge regions to the next highest size - instead track alignment explicitly. This should improve the memory layout for bridge regions.
This one got mangled too:
Applying: pciinit: Track region alignment explicitly. fatal: corrupt patch at line 40 Patch failed at 0005 pciinit: Track region alignment explicitly.
Do you have a git tree I can pull from for testing?
Hi, Thank you. I don't have an public available git tree. Not sure if I can create any from workplace.
So I just reposted the patch.
thanks, Gerd
Also, unused bridge regions will no longer be allocated any space.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
src/pciinit.c | 41 ++++++++++++++++++----------------------- 1 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index 1b31177..2bd4426 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -33,6 +33,7 @@ struct pci_region_entry { struct pci_device *dev; int bar; u32 size;
- u32 align; int is64; enum pci_region_type type; struct pci_region_entry *next;
@@ -41,7 +42,7 @@ struct pci_region_entry { struct pci_bus { struct { /* pci region stats */
u32 sum, max;
u32 sum, align; /* pci region assignments */ u32 base; struct pci_region_entry *list;@@ -307,12 +308,6 @@ pci_bios_init_bus(void)
- Bus sizing
****************************************************************/
-static u32 pci_size_roundup(u32 size) -{
- int index = __fls(size-1)+1;
- return 0x1 << index;
-}
static void pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) { @@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
static struct pci_region_entry * pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u32 size, int type, int is64)
int bar, u32 size, u32 align, int type, intis64) { struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); if (!entry) { @@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->dev = dev; entry->bar = bar; entry->size = size;
- entry->align = align; entry->is64 = is64; entry->type = type; // Insert into list in sorted order. struct pci_region_entry **pprev; for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) { struct pci_region_entry *pos = *pprev;
if (pos->size < size)
if (pos->align < align || (pos->align == align && pos->size <size)) break; } entry->next = *pprev; *pprev = entry;
bus->r[type].sum += size;
- if (bus->r[type].max < size)
bus->r[type].max = size;
- if (bus->r[type].align < align)
bus->r[type].align = align;}
@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); struct pci_region_entry *entry = pci_region_create_entry(
bus, pci, i, size, type, is64);
bus, pci, i, size, size, type, is64); if (!entry) return -1;@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
u32 limit = (type == PCI_REGION_TYPE_IO) ?
u32 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
u32 size = s->r[type].sum;if (size < limit)size = limit;size = pci_size_roundup(size);
if (s->r[type].align > align)align = s->r[type].align;u32 size = ALIGN(s->r[type].sum, align); // entry->bar is -1 if the entry represents a bridge region struct pci_region_entry *entry = pci_region_create_entry(
parent, s->bus_dev, -1, size, type, 0);
parent, s->bus_dev, -1, size, align, type, 0); if (!entry) return -1; dprintf(1, "PCI: secondary bus %d size %x type %s\n",@@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) return 0; }
-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1) +#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment) static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) @@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
- if (bus->r[reg1].sum < bus->r[reg2].sum) {
// Swap regions so larger area is more likely to align well.
- if (bus->r[reg1].align < bus->r[reg2].align) {
// Swap regions to improve alignment. reg1 = PCI_REGION_TYPE_MEM; reg2 = PCI_REGION_TYPE_PREFMEM;
- bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].max);
- bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].align); bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
, bus->r[reg1].max);
, bus->r[reg1].align);
On 25/04/12 13:48, Kevin O'Connor wrote:
On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
Migrate 64bit entries to 64bit pci regions if they do not fit in 32bit range.
[...]
+static void pci_region_migrate_64bit_entries(struct pci_region *from,
struct pci_region *to)+{
- struct pci_region_entry **pprev = &from->list;
- struct pci_region_entry **last = &to->list;
- while(*pprev) {
if ((*pprev)->is64) {struct pci_region_entry *entry;entry = *pprev;/* Delete the entry and move next */*pprev = (*pprev)->next;/* Add entry at tail to keep a sorted order */entry->next = NULL;if (*last) {(*last)->next = entry;last = &(*last)->next;}else(*last) = entry;}elsepprev = &(*pprev)->next;- }
+}
It should be possible to simplify this - something like (untested):
static void pci_region_migrate_64bit_entries(struct pci_region *from, struct pci_region *to) { struct pci_region_entry **pprev = &from->list, **last = &to->list; for (; *pprev; pprev = &(*pprev)->next) { struct pci_region_entry *entry = *pprev; if (!entry->is64) continue; // Move from source list to dest list. *pprev = entry->next; entry->next = NULL; *last = entry; last = &entry->next; } }
Sorry it's not working. I agree it's possible to simplify code a bit.
static void pci_region_migrate_64bit_entries(struct pci_region *from, struct pci_region *to) { struct pci_region_entry **pprev = &from->list; struct pci_region_entry **last = &to->list; while(*pprev) { if ((*pprev)->is64) { struct pci_region_entry *entry; entry = *pprev; /* Delete the entry and move next */ *pprev = (*pprev)->next; /* Add entry at tail to keep the order */ entry->next = NULL; *last = entry; last = &entry->next; } else pprev = &(*pprev)->next; } }
That should work.
[...]
static void pci_bios_map_devices(struct pci_bus *busses) {
- if (pci_bios_init_root_regions(busses)) {
struct pci_region r64_mem, r64_pref;r64_mem.list = NULL;r64_pref.list = NULL;pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],&r64_mem);pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],&r64_pref);if (pci_bios_init_root_regions(busses))panic("PCI: out of address space\n");r64_mem.base = BUILD_PCIMEM64_START;r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),pci_region_align(&r64_pref));There should be a check to see if the regions fit. Maybe pass start/end into pci_bios_init_root_regions() and call it again for the
4g region?
Agree, I just ignored the check as 64bit range size is 2^39. I will think how to make it better.
pci_region_map_entries(busses, &r64_mem);pci_region_map_entries(busses, &r64_pref);- } // Map regions on each device.
This doesn't look right to me. This will map the devices on bus 0 to the proper >4g address, but devices on any subsequent bus will use busses[0].r[].base which will be reset to the <4gig address. Perhaps pull base out of pci_region and make pci_region_map_entries() recursive?
No recursion is need here! We map all entries which are 64bit on root bus. If entry is a bridge region - a corresponding bus address will be updated. Region won't be reseted to <4gig address as address is derived from parent region only.
Thanks, Alexey
On Wed, Apr 25, 2012 at 06:25:07PM +1200, Alexey Korolev wrote:
On 25/04/12 13:48, Kevin O'Connor wrote:
On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
pci_region_map_entries(busses, &r64_mem);pci_region_map_entries(busses, &r64_pref);- } // Map regions on each device.
This doesn't look right to me. This will map the devices on bus 0 to the proper >4g address, but devices on any subsequent bus will use busses[0].r[].base which will be reset to the <4gig address. Perhaps pull base out of pci_region and make pci_region_map_entries() recursive?
No recursion is need here! We map all entries which are 64bit on root bus. If entry is a bridge region - a corresponding bus address will be updated. Region won't be reseted to <4gig address as address is derived from parent region only.
Okay - I missed that. I think the patches look okay to be committed - any additional changes can be made on top. Gerd - do you have any comments?
-Kevin
On 04/25/12 14:51, Kevin O'Connor wrote:
On Wed, Apr 25, 2012 at 06:25:07PM +1200, Alexey Korolev wrote:
On 25/04/12 13:48, Kevin O'Connor wrote:
On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
pci_region_map_entries(busses, &r64_mem);pci_region_map_entries(busses, &r64_pref);- } // Map regions on each device.
This doesn't look right to me. This will map the devices on bus 0 to the proper >4g address, but devices on any subsequent bus will use busses[0].r[].base which will be reset to the <4gig address. Perhaps pull base out of pci_region and make pci_region_map_entries() recursive?
No recursion is need here! We map all entries which are 64bit on root bus. If entry is a bridge region - a corresponding bus address will be updated. Region won't be reseted to <4gig address as address is derived from parent region only.
Okay - I missed that. I think the patches look okay to be committed - any additional changes can be made on top. Gerd - do you have any comments?
Great job overall. Can't spot issues in the code. And the patches do fine in testing too. Some minor issues poped up:
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
Issue #2: root bus (non-pref) memory regions are mapped above 4G if they are 64bit capable. That happened to include the xhci usb controller. I don't think we want that. Some day seabios will get xhci support, and having the bar above 4G makes it unreachable in 32bit mode. So this needs some refinement. Options I can think of:
(1) Don't bother mapping non-prefmem bars above 4G. (2) Only map them above 4G if they are larger than a certain limit. (3) Allow devices to be excluded on certain conditions, for example when seabios has a driver, when they have an option rom, when they have a specific pci id, ...
These are certainly no blockers and can be discussed and solved after committing this series.
cheers, Gerd
Hi Thank you for review! On 26/04/12 03:29, Gerd Hoffmann wrote:
Okay - I missed that. I think the patches look okay to be committed - any additional changes can be made on top. Gerd - do you have any comments?
Great job overall. Can't spot issues in the code. And the patches do fine in testing too. Some minor issues poped up:
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
I observed similar but ignored since "bridge test" requires special qemu build. It's a general bridge support issue. This series don't address this issue. This issue occurs regardless if the patches applied or not.
Issue #2: root bus (non-pref) memory regions are mapped above 4G if they are 64bit capable. That happened to include the xhci usb controller. I don't think we want that. Some day seabios will get xhci support, and having the bar above 4G makes it unreachable in 32bit mode. So this needs some refinement. Options I can think of:
(1) Don't bother mapping non-prefmem bars above 4G. (2) Only map them above 4G if they are larger than a certain limit. (3) Allow devices to be excluded on certain conditions, for example when seabios has a driver, when they have an option rom, when they have a specific pci id, ...
I like (3) more than the (1) and (2).
These are certainly no blockers and can be discussed and solved after committing this series.
I've just fixed what you have suggested to fix. Just very minor changes in patches 10/12 and 11/12: - Simplify a bit list operation. - Remove function calls away of macros - Add condition to check crossing the end of 64bit PCI range
Thanks, Alexey
Hi,
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
I observed similar but ignored since "bridge test" requires special qemu build. It's a general bridge support issue. This series don't address this issue. This issue occurs regardless if the patches applied or not.
FYI: Meanwhile the bridge patches have been merged, latest qemu master branch will do.
cheers, Gerd
On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) != 0) break" from pci_bios_init_devices()?
The code should probably handle the irq swizzling that pci bridges do though.
Issue #2: root bus (non-pref) memory regions are mapped above 4G if they are 64bit capable. That happened to include the xhci usb controller. I don't think we want that. Some day seabios will get xhci support, and having the bar above 4G makes it unreachable in 32bit mode. So this needs some refinement. Options I can think of:
(1) Don't bother mapping non-prefmem bars above 4G. (2) Only map them above 4G if they are larger than a certain limit. (3) Allow devices to be excluded on certain conditions, for example when seabios has a driver, when they have an option rom, when they have a specific pci id, ...
I'd vote for 2. There's nearly 500MB of space for PCI devices under 4G - small regions (say under 1MB) are unlikely to be the cause of an allocation failure (1MB would be .2% of total space).
-Kevin
On Thu, Apr 26, 2012 at 07:12:22PM +1200, Alexey Korolev wrote:
On 26/04/12 03:29, Gerd Hoffmann wrote:
Okay - I missed that. I think the patches look okay to be committed - any additional changes can be made on top. Gerd - do you have any comments?
These are certainly no blockers and can be discussed and solved after committing this series.
I've just fixed what you have suggested to fix. Just very minor changes in patches 10/12 and 11/12:
- Simplify a bit list operation.
- Remove function calls away of macros
- Add condition to check crossing the end of 64bit PCI range
Thanks. I committed this series.
-Kevin
On 27/04/12 00:45, Kevin O'Connor wrote:
On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) != 0) break" from pci_bios_init_devices()?
This helps in my tests. The devices are no longer disabled. Not sure about virtio-scsi test though.
The code should probably handle the irq swizzling that pci bridges do though.
Hi,
Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) != 0) break" from pci_bios_init_devices()?
Seems to do the trick, at least the disks connected appear in the boot menu now and the seabios log file looks sane.
The guest kernel has no virtio-scsi drivers though, need to update it for more testing.
The code should probably handle the irq swizzling that pci bridges do though.
i.e. add bridge handling to pci_slot_get_irq() ?
cheers, Gerd
On Wed, May 02, 2012 at 03:42:51PM +0200, Gerd Hoffmann wrote:
Hi,
Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) != 0) break" from pci_bios_init_devices()?
Seems to do the trick, at least the disks connected appear in the boot menu now and the seabios log file looks sane.
The guest kernel has no virtio-scsi drivers though, need to update it for more testing.
The code should probably handle the irq swizzling that pci bridges do though.
i.e. add bridge handling to pci_slot_get_irq() ?
Yes.
-Kevin
On Tue, Apr 24, 2012 at 06:26:30PM +1200, Alexey Korolev wrote:
This patch solves issues on Windows guests, when 64bit BAR's are present. It is also helpful on Linux guests when use_crs kernel boot option is set.
Unfortunately, this patch causes problems for WinXP, so I reverted it.
-Kevin
-
Alexey Korolev -
Gerd Hoffmann -
Kevin O'Connor