On Wed, Mar 28, 2012 at 05:54:10PM +1300, Alexey Korolev wrote: > In this patch instead of array based resource allocation approach > we calculate resource addresses linked lists of pci_region_entry structures.
Thanks. I still think this migration can be done more seamlessly. I played with your patches a bit and came up with the attached patches that do just code movement - no alogorithm changes. See the attached. Also, I think we should look to commit after the next SeaBIOS release. -Kevin
>From 2f6e81d884dfbb01a12ddfb10a64bf87e864a19c Mon Sep 17 00:00:00 2001 From: Alexey Korolev <alexey.koro...@endace.com> Date: Wed, 28 Mar 2012 17:41:41 +1300 Subject: [PATCH 1/3] Added a pci_region_entry structure To: seab...@seabios.org In this patch 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.koro...@endace.com> Signed-off-by: Kevin O'Connor <ke...@koconnor.net> --- src/pci.h | 5 -- src/pciinit.c | 115 +++++++++++++++++++++++++++++++++++++++------------------ 2 files changed, 79 insertions(+), 41 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..2831895 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -31,6 +31,19 @@ static const char *region_type_name[] = { [ PCI_REGION_TYPE_PREFMEM ] = "prefmem", }; +struct pci_bus; +struct pci_region_entry { + struct pci_device *dev; + int bar; + u32 size; + int is64; + enum pci_region_type type; + struct pci_bus *child_bus; + struct pci_bus *parent_bus; + struct pci_region_entry *next; + struct pci_region_entry **pprev; +}; + struct pci_bus { struct { /* pci region stats */ @@ -41,6 +54,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 +366,33 @@ 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, + u32 size, int type, int is64) { - u32 index; - - index = pci_size_to_index(size, type); + struct pci_region_entry *entry = malloc_tmp(sizeof(*entry)); + if (!entry) { + warn_noalloc(); + return NULL; + } + memset(entry, 0, sizeof(*entry)); + entry->dev = dev; + entry->size = size; + entry->is64 = is64; + entry->type = type; + entry->parent_bus = bus; + list_add_head(&bus->r[type].list, entry); + + u32 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 +411,16 @@ 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, size, pci_addr_to_type(val), is64); + if (!entry) + return -1; + entry->bar = i; - if (pci->bars[i].is64) + if (is64) i++; } } @@ -410,7 +440,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); + struct pci_region_entry *entry = pci_region_create_entry( + parent, s->bus_dev, s->r[type].size, type, 0); + if (!entry) + return -1; + entry->child_bus = s; } dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n", secondary_bus, @@ -418,6 +452,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 +518,25 @@ 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_region_entry *entry) +{ + if (!entry->child_bus) { + u32 addr = pci_bios_bus_get_addr( + entry->parent_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(entry->dev->bdf), + pci_bdf_to_dev(entry->dev->bdf), + pci_bdf_to_fn(entry->dev->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 +580,15 @@ 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, *next; + list_foreach_entry_safe(busses[bus].r[type].list, next, entry) { + pci_region_map_one_entry(entry); + list_del(entry); + free(entry); } } } @@ -588,7 +629,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"); } -- 1.7.6.5
>From 6b6f20425b14f7f99aefb28a8553363573544409 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <ke...@koconnor.net> Date: Sun, 1 Apr 2012 00:39:20 -0400 Subject: [PATCH 2/3] Perform pci-to-pci bus bar assignment at same time as normal bar assignment. To: seab...@seabios.org Signed-off-by: Kevin O'Connor <ke...@koconnor.net> --- src/pciinit.c | 55 +++++++++++++++++++------------------------------------ 1 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 2831895..5c37e08 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -520,9 +520,9 @@ static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size) static void pci_region_map_one_entry(struct pci_region_entry *entry) { + u32 addr = pci_bios_bus_get_addr( + entry->parent_bus, entry->type, entry->size); if (!entry->child_bus) { - u32 addr = pci_bios_bus_get_addr( - entry->parent_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(entry->dev->bdf), @@ -534,54 +534,37 @@ static void pci_region_map_one_entry(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); + entry->child_bus->r[entry->type].base = addr; + u16 bdf = entry->dev->bdf; + 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, *next; -- 1.7.6.5
>From 91b8ba37b87187643b96dca9eb135998eeead44e Mon Sep 17 00:00:00 2001 From: Alexey Korolev <alexey.koro...@endace.com> Date: Wed, 28 Mar 2012 17:56:44 +1300 Subject: [PATCH 3/3] Get rid of size element of pci_bus->r structure To: seab...@seabios.org 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.koro...@endace.com> --- src/pciinit.c | 20 ++++++++------------ 1 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 5c37e08..f2c839a 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -49,8 +49,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; @@ -436,21 +434,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); struct pci_region_entry *entry = pci_region_create_entry( - parent, s->bus_dev, s->r[type].size, type, 0); + parent, s->bus_dev, size, type, 0); if (!entry) return -1; entry->child_bus = s; + 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; } -- 1.7.6.5