* [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3)
@ 2012-03-28 4:25 Alexey Korolev
2012-03-28 4:28 ` [Qemu-devel] [PATCH 1/4] Add basic linked list operations Alexey Korolev
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Alexey Korolev @ 2012-03-28 4:25 UTC (permalink / raw)
To: kevin, seabios; +Cc: sfd, qemu-devel
Hi,
This patch series redesigns the existing pciinit.c code and introduces
linked list operations.
Changes are more about structures definitions rather than functionality.
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.
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, base address, PCI device owner, and a
pointer to the pci_bus it belongs to.
1. Introduce List operations
2. Add pci_region_entry and linked lists operations in place while still
using the array system to do the allocations.
3. Switch to lists operations
4. Get rid of size element from bus->r structure, now we use entry->size
for the same purpose
src/pci.h | 5 -
src/pciinit.c | 258 ++++++++++++++++++++++++++-------------------------------
src/util.h | 21 +++++
3 files changed, 137 insertions(+), 147 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 1/4] Add basic linked list operations 2012-03-28 4:25 [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3) Alexey Korolev @ 2012-03-28 4:28 ` Alexey Korolev 2012-03-28 14:39 ` [Qemu-devel] [SeaBIOS] " Gerd Hoffmann 2012-03-28 4:41 ` [Qemu-devel] [PATCH 2/4] Added a pci_region_entry structure Alexey Korolev ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Alexey Korolev @ 2012-03-28 4:28 UTC (permalink / raw) To: kevin; +Cc: sfd, seabios, qemu-devel This linked list implementation is partially based on kernel code. So it should be quite stable Signed-off-by: Alexey Korolev <alexey.korolev@endace.com> --- src/util.h | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/src/util.h b/src/util.h index 70d3c4c..17df3cf 100644 --- a/src/util.h +++ b/src/util.h @@ -195,6 +195,27 @@ struct descloc_s { u32 addr; } PACKED; +// Double linked lists with a single pointer list head +#define list_foreach_entry_safe(head, next, entry) \ + for (entry = head; entry && ({next=entry->next; 1;}); \ + entry = next) + +#define list_del(entry) \ + do { \ + *(entry)->pprev = (entry)->next; \ + if ((entry)->next) \ + (entry)->next->pprev = (entry)->pprev; \ + } while (0) + +#define list_add_head(phead, entry) \ + do { \ + (entry)->next = *(phead); \ + if (*(phead)) \ + (*(phead))->pprev = &(entry)->next; \ + *(phead) = entry; \ + (entry)->pprev = phead; \ + } while (0) + // util.c void cpuid(u32 index, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); struct bregs; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [SeaBIOS] [PATCH 1/4] Add basic linked list operations 2012-03-28 4:28 ` [Qemu-devel] [PATCH 1/4] Add basic linked list operations Alexey Korolev @ 2012-03-28 14:39 ` Gerd Hoffmann 2012-03-29 2:03 ` Alexey Korolev 2012-03-29 2:27 ` Kevin O'Connor 0 siblings, 2 replies; 13+ messages in thread From: Gerd Hoffmann @ 2012-03-28 14:39 UTC (permalink / raw) To: alexey.korolev; +Cc: sfd, kevin, seabios, qemu-devel On 03/28/12 06:28, Alexey Korolev wrote: > This linked list implementation is partially based on kernel code. So it > should be quite stable How about just copying the file? I've used the linux kernel list implementation elsewhere too and it worked just fine with only minor tweaks (remove some likely()/unlikely() macros IIRC). cheers, Gerd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [SeaBIOS] [PATCH 1/4] Add basic linked list operations 2012-03-28 14:39 ` [Qemu-devel] [SeaBIOS] " Gerd Hoffmann @ 2012-03-29 2:03 ` Alexey Korolev 2012-03-29 2:27 ` Kevin O'Connor 1 sibling, 0 replies; 13+ messages in thread From: Alexey Korolev @ 2012-03-29 2:03 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: sfd, kevin, seabios, qemu-devel >> This linked list implementation is partially based on kernel code. So it >> should be quite stable > How about just copying the file? > > I've used the linux kernel list implementation elsewhere too and it > worked just fine with only minor tweaks (remove some likely()/unlikely() > macros IIRC). Right, in "take-2" Kevin has suggested very similar. It will be done as a separate patch, because the changes are not just about pciinit.c, they also simplify pmm.c and stack.c a bit. In this patch I'm submitting only few necessary functions for list operations. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [SeaBIOS] [PATCH 1/4] Add basic linked list operations 2012-03-28 14:39 ` [Qemu-devel] [SeaBIOS] " Gerd Hoffmann 2012-03-29 2:03 ` Alexey Korolev @ 2012-03-29 2:27 ` Kevin O'Connor 2012-03-29 10:41 ` Ian Campbell 1 sibling, 1 reply; 13+ messages in thread From: Kevin O'Connor @ 2012-03-29 2:27 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: alexey.korolev, sfd, seabios, qemu-devel On Wed, Mar 28, 2012 at 04:39:07PM +0200, Gerd Hoffmann wrote: > On 03/28/12 06:28, Alexey Korolev wrote: > > This linked list implementation is partially based on kernel code. So it > > should be quite stable > > How about just copying the file? > > I've used the linux kernel list implementation elsewhere too and it > worked just fine with only minor tweaks (remove some likely()/unlikely() > macros IIRC). Linux is GPLv2 while SeaBIOS is LGPLv3, so some care should be taken. -Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [SeaBIOS] [PATCH 1/4] Add basic linked list operations 2012-03-29 2:27 ` Kevin O'Connor @ 2012-03-29 10:41 ` Ian Campbell 0 siblings, 0 replies; 13+ messages in thread From: Ian Campbell @ 2012-03-29 10:41 UTC (permalink / raw) To: Kevin O'Connor; +Cc: sfd, seabios, Gerd Hoffmann, qemu-devel On Wed, 2012-03-28 at 22:27 -0400, Kevin O'Connor wrote: > On Wed, Mar 28, 2012 at 04:39:07PM +0200, Gerd Hoffmann wrote: > > On 03/28/12 06:28, Alexey Korolev wrote: > > > This linked list implementation is partially based on kernel code. So it > > > should be quite stable > > > > How about just copying the file? > > > > I've used the linux kernel list implementation elsewhere too and it > > worked just fine with only minor tweaks (remove some likely()/unlikely() > > macros IIRC). > > Linux is GPLv2 while SeaBIOS is LGPLv3, so some care should be taken. FWIW In Xen we decided to go with the BSD list macros in our LGPL library interfaces to avoid a similar sort issue. http://xenbits.xen.org/hg/xen-unstable.hg/file/74d2af0b56ea/tools/include/xen-external has that plus a handy sed script to namespace the interface, although I don't suppose that will matter inside seabios. Ian. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/4] Added a pci_region_entry structure 2012-03-28 4:25 [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3) Alexey Korolev 2012-03-28 4:28 ` [Qemu-devel] [PATCH 1/4] Add basic linked list operations Alexey Korolev @ 2012-03-28 4:41 ` Alexey Korolev 2012-03-28 4:54 ` [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list Alexey Korolev 2012-03-28 4:56 ` [Qemu-devel] [PATCH 4/4] Get rid of size element of pci_bus->r structure Alexey Korolev 3 siblings, 0 replies; 13+ messages in thread From: Alexey Korolev @ 2012-03-28 4:41 UTC (permalink / raw) To: kevin; +Cc: sfd, seabios, qemu-devel 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.korolev@endace.com> --- src/pci.h | 5 -- src/pciinit.c | 116 ++++++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 86 insertions(+), 35 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..6a285c9 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -31,6 +31,20 @@ 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 base; + u32 size; + int is64bit; + enum pci_region_type type; + struct pci_bus *this_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 +55,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,6 +367,29 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size) *size = (~(*val & mask)) + 1; } +/**************************************************************** + * Build topology and calculate size of entries + ****************************************************************/ + +struct pci_region_entry * +pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, + u32 size, int type, int is64bit) +{ + struct pci_region_entry *entry= malloc_tmp(sizeof(*entry)); + if (!entry) { + warn_noalloc(); + return NULL; + } + memset(entry, 0, sizeof(*entry)); + entry->dev = dev; + entry->type = type; + entry->is64bit = is64bit; + entry->size = size; + list_add_head(&bus->r[type].list, entry); + entry->parent_bus = bus; + return entry; +} + static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size) { u32 index; @@ -364,9 +402,10 @@ static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size) bus->r[type].max = size; } -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"); + struct pci_region_entry *entry; // Calculate resources needed for regular (non-bus) devices. struct pci_device *pci; @@ -382,15 +421,22 @@ static void pci_bios_check_devices(struct pci_bus *busses) pci_bios_get_bar(pci, i, &val, &size); 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) && + int type = pci_addr_to_type(val); + int min_size = (type == PCI_REGION_TYPE_IO) ? + (1<<PCI_IO_INDEX_SHIFT) : (1<<PCI_MEM_INDEX_SHIFT); + size = (size > min_size) ? size : min_size; + int is64bit = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); - if (pci->bars[i].is64) + pci_bios_bus_reserve(bus, pci_addr_to_type(val), size); + + entry = pci_region_create_entry(bus, pci, size, type, is64bit); + if (!entry) + return -1; + entry->bar = i; + + if (is64bit) i++; } } @@ -411,6 +457,11 @@ static void pci_bios_check_devices(struct pci_bus *busses) 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 = pci_region_create_entry(parent, s->bus_dev, + s->r[type].size, type, 0); + if (!entry) + return -1; + entry->this_bus = s; } dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n", secondary_bus, @@ -418,6 +469,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) @@ -526,28 +578,30 @@ 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; + struct pci_region_entry *entry, *next; + for (bus = 0; bus<=MaxPCIBus; bus++) { + int type; + for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { + list_foreach_entry_safe(busses[bus].r[type].list, + next, entry) { + if (!entry->this_bus) { + entry->base = pci_bios_bus_get_addr(&busses[bus], + entry->type, entry->size); + pci_set_io_region_addr(entry->dev, entry->bar, entry->base); + if (entry->is64bit) + pci_set_io_region_addr(entry->dev, entry->bar, 0); + + dprintf(1, "PCI: map device bdf=%02x:%02x.%x \tbar %d" + "\tsize\t0x%08x\tbase 0x%x type %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, entry->size, entry->base, + region_type_name[entry->type]); + } + list_del(entry); + free(entry); } } } @@ -588,7 +642,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.5.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list 2012-03-28 4:25 [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3) Alexey Korolev 2012-03-28 4:28 ` [Qemu-devel] [PATCH 1/4] Add basic linked list operations Alexey Korolev 2012-03-28 4:41 ` [Qemu-devel] [PATCH 2/4] Added a pci_region_entry structure Alexey Korolev @ 2012-03-28 4:54 ` Alexey Korolev 2012-04-01 7:28 ` Kevin O'Connor 2012-03-28 4:56 ` [Qemu-devel] [PATCH 4/4] Get rid of size element of pci_bus->r structure Alexey Korolev 3 siblings, 1 reply; 13+ messages in thread From: Alexey Korolev @ 2012-03-28 4:54 UTC (permalink / raw) To: kevin; +Cc: sfd, seabios, qemu-devel In this patch instead of array based resource allocation approach we calculate resource addresses linked lists of pci_region_entry structures. Signed-off-by: Alexey Korolev <alexey.korolev@endace.com> --- src/pciinit.c | 179 ++++++++++++++++----------------------------------------- 1 files changed, 50 insertions(+), 129 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 6a285c9..85fe823 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -12,9 +12,8 @@ #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_IO_MIN 0x4 +#define PCI_DEVICE_MEM_MIN 0x1000 #define PCI_BRIDGE_IO_MIN 0x1000 #define PCI_BRIDGE_MEM_MIN 0x100000 @@ -48,38 +47,14 @@ struct pci_region_entry { struct pci_bus { struct { /* 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; 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) @@ -387,19 +362,11 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->size = size; list_add_head(&bus->r[type].list, entry); entry->parent_bus = bus; - return entry; -} - -static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size) -{ - u32 index; - 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 int pci_bios_check_devices(struct pci_bus *busses) @@ -423,14 +390,12 @@ static int pci_bios_check_devices(struct pci_bus *busses) continue; int type = pci_addr_to_type(val); int min_size = (type == PCI_REGION_TYPE_IO) ? - (1<<PCI_IO_INDEX_SHIFT) : (1<<PCI_MEM_INDEX_SHIFT); + PCI_DEVICE_IO_MIN : PCI_DEVICE_MEM_MIN; size = (size > min_size) ? size : min_size; int is64bit = (!(val & PCI_BASE_ADDRESS_SPACE_IO) && (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64); - pci_bios_bus_reserve(bus, pci_addr_to_type(val), size); - entry = pci_region_create_entry(bus, pci, size, type, is64bit); if (!entry) return -1; @@ -456,7 +421,6 @@ static int 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 = pci_region_create_entry(parent, s->bus_dev, s->r[type].size, type, 0); if (!entry) @@ -496,112 +460,69 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end) /**************************************************************** - * BAR assignment + * Map pci region entries ****************************************************************/ -static void pci_bios_init_bus_bases(struct pci_bus *bus) +static void pci_region_map_one_entry(struct pci_region_entry *entry) { - 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; - } + if (!entry->this_bus ) { + dprintf(1, "PCI: bdf %d bar %d\tsize\t0x%08x\tbase 0x%x type %s\n", + entry->dev->bdf, entry->bar, entry->size, entry->base, + region_type_name[entry->type]); + + pci_set_io_region_addr(entry->dev, entry->bar, entry->base); + if (entry->is64bit) + pci_set_io_region_addr(entry->dev, entry->bar + 1, 0); + return; } -} - -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 + entry->this_bus->r[entry->type].base = entry->base; + dprintf(1, "PCI-2-PCI bus %d\t\tsize\t0x%08x\tbase 0x%x type %s\n", + entry->dev->secondary_bus, entry->size, entry->base, + region_type_name[entry->type]); -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); + u16 bdf = entry->dev->bdf; + u32 base = entry->base; + u32 limit = entry->base + entry->size - 1; + if (entry->type == PCI_REGION_TYPE_IO) { + pci_config_writeb(bdf, PCI_IO_BASE, base >> 8); pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0); - pci_config_writeb(bdf, PCI_IO_LIMIT, limit >> PCI_IO_SHIFT); + pci_config_writeb(bdf, PCI_IO_LIMIT, limit >> 8); 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); - 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); - pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> PCI_PREF_MEMORY_SHIFT); + } + if (entry->type == PCI_REGION_TYPE_MEM) { + pci_config_writew(bdf, PCI_MEMORY_BASE, base >> 16); + pci_config_writew(bdf, PCI_MEMORY_LIMIT, limit >> 16); + } + if (entry->type == PCI_REGION_TYPE_PREFMEM) { + pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, base >> 16); + pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> 16); pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0); pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0); } + return; +} - // Map regions on each device. - int bus; +static void pci_bios_map_devices(struct pci_bus *busses) +{ struct pci_region_entry *entry, *next; + + int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - list_foreach_entry_safe(busses[bus].r[type].list, - next, entry) { - if (!entry->this_bus) { - entry->base = pci_bios_bus_get_addr(&busses[bus], - entry->type, entry->size); - pci_set_io_region_addr(entry->dev, entry->bar, entry->base); - if (entry->is64bit) - pci_set_io_region_addr(entry->dev, entry->bar, 0); - - dprintf(1, "PCI: map device bdf=%02x:%02x.%x \tbar %d" - "\tsize\t0x%08x\tbase 0x%x type %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, entry->size, entry->base, - region_type_name[entry->type]); + u32 size; + for (size = busses[bus].r[type].max; size > 0; size >>= 1) { + list_foreach_entry_safe(busses[bus].r[type].list, + next, entry) { + if (size == entry->size) { + entry->base = busses[bus].r[type].base; + busses[bus].r[type].base += size; + pci_region_map_one_entry(entry); + list_del(entry); + free(entry); + } } - list_del(entry); - free(entry); } } } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list 2012-03-28 4:54 ` [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list Alexey Korolev @ 2012-04-01 7:28 ` Kevin O'Connor 2012-04-01 8:44 ` Kevin O'Connor 0 siblings, 1 reply; 13+ messages in thread From: Kevin O'Connor @ 2012-04-01 7:28 UTC (permalink / raw) To: Alexey Korolev; +Cc: sfd, seabios, qemu-devel [-- Attachment #1: Type: text/plain, Size: 502 bytes --] 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 [-- Attachment #2: 0001-Added-a-pci_region_entry-structure.patch --] [-- Type: text/plain, Size: 7812 bytes --] >From 2f6e81d884dfbb01a12ddfb10a64bf87e864a19c Mon Sep 17 00:00:00 2001 From: Alexey Korolev <alexey.korolev@endace.com> Date: Wed, 28 Mar 2012 17:41:41 +1300 Subject: [PATCH 1/3] Added a pci_region_entry structure To: seabios@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.korolev@endace.com> Signed-off-by: Kevin O'Connor <kevin@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 [-- Attachment #3: 0002-Perform-pci-to-pci-bus-bar-assignment-at-same-time-a.patch --] [-- Type: text/plain, Size: 4226 bytes --] >From 6b6f20425b14f7f99aefb28a8553363573544409 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@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: seabios@seabios.org Signed-off-by: Kevin O'Connor <kevin@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 [-- Attachment #4: 0003-Get-rid-of-size-element-of-pci_bus-r-structure.patch --] [-- Type: text/plain, Size: 2343 bytes --] >From 91b8ba37b87187643b96dca9eb135998eeead44e Mon Sep 17 00:00:00 2001 From: Alexey Korolev <alexey.korolev@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: seabios@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.korolev@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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list 2012-04-01 7:28 ` Kevin O'Connor @ 2012-04-01 8:44 ` Kevin O'Connor 2012-04-03 6:39 ` Alexey Korolev 0 siblings, 1 reply; 13+ messages in thread From: Kevin O'Connor @ 2012-04-01 8:44 UTC (permalink / raw) To: Alexey Korolev; +Cc: sfd, seabios, qemu-devel On Sun, Apr 01, 2012 at 03:28:34AM -0400, Kevin O'Connor wrote: > 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. Looking closer at your new allocation algorithm, I see that it effectively allocates largest sizes first. It occurred to me that an easy way to do this is to keep the pci_region_entry list sorted by size. See the patch below as an example (based on top of the previous email I sent). -Kevin diff --git a/src/pciinit.c b/src/pciinit.c index f2c839a..bfee178 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -41,16 +41,13 @@ struct pci_region_entry { 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 */ - 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]; @@ -379,11 +376,18 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, entry->is64 = is64; entry->type = type; entry->parent_bus = bus; - list_add_head(&bus->r[type].list, entry); + // 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; 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; @@ -478,46 +482,14 @@ 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 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); + u32 addr = entry->parent_bus->r[entry->type].base; + entry->parent_bus->r[entry->type].base += entry->size; if (!entry->child_bus) { dprintf(1, "PCI: map device bdf=%02x:%02x.%x" " bar %d, addr %08x, size %08x [%s]\n", @@ -559,15 +531,14 @@ 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; - list_foreach_entry_safe(busses[bus].r[type].list, next, entry) { + struct pci_region_entry *entry = busses[bus].r[type].list; + while (entry) { pci_region_map_one_entry(entry); - list_del(entry); + struct pci_region_entry *next = entry->next; free(entry); + entry = next; } } } ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list 2012-04-01 8:44 ` Kevin O'Connor @ 2012-04-03 6:39 ` Alexey Korolev 2012-04-04 3:31 ` Kevin O'Connor 0 siblings, 1 reply; 13+ messages in thread From: Alexey Korolev @ 2012-04-03 6:39 UTC (permalink / raw) To: Kevin O'Connor; +Cc: sfd, seabios, qemu-devel Hi Kevin, Thank you for the patches! I've created a diff of final version of your changes over mine, to make it clear what has changed. Rather than including the complete diff, I've just left relevant parts and added comments. --- a/src/pciinit.c +++ b/src/pciinit.c@@ -12,8 +12,9 @@ ........ @@ -34,25 +35,44 @@ struct pci_region_entry { struct pci_device *dev; int bar; - u32 base; u32 size; - int is64bit; + int is64; enum pci_region_type type; - struct pci_bus *this_bus; + struct pci_bus *child_bus; Structure members naming was one of difficult things when I was writing the code. The child_bus might be a bit confusing as people may thing that it describes a child bus in the bus topology,in fact this element describes the bus this pci_region_entry is representing. ....... + +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); +} The only purpose to have these functions is to define the minimum size of pci BAR. They are used only once. What if we add size adjustment to pci_region_create_entry, or just create a function like pci_adjust_size(u32 size, enum pci_region_type type, int bridge)? ......... - list_add_head(&bus->r[type].list, entry); entry->parent_bus = bus; - + // 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; + .......... static void pci_bios_map_devices(struct pci_bus *busses) { - struct pci_region_entry *entry, *next; - + // Map regions on each device. int bus; for (bus = 0; bus<=MaxPCIBus; bus++) { int type; for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { - u32 size; - for (size = busses[bus].r[type].max; size > 0; size >>= 1) { - list_foreach_entry_safe(busses[bus].r[type].list, - next, entry) { - if (size == entry->size) { - entry->base = busses[bus].r[type].base; - busses[bus].r[type].base += size; - pci_region_map_one_entry(entry); - list_del(entry); - free(entry); - } - } + struct pci_region_entry *entry = busses[bus].r[type].list; + while (entry) { + pci_region_map_one_entry(entry); + struct pci_region_entry *next = entry->next; + free(entry); + entry = next; } } } Right, instead of sorting entries by size in pci_bios_map_devices, the entries are sorted when they are created. This makes the implementation simpler. Note: In case of 64bit BARs we have to migrate entries, so just sorting on create won't be enough, we should also add sorting when entries are migrated. This will add some more complexity, while in case of old implementation complexity will remain the same. I expect it shouldn't be that complicated, so any of these approaches are fine for me. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list 2012-04-03 6:39 ` Alexey Korolev @ 2012-04-04 3:31 ` Kevin O'Connor 0 siblings, 0 replies; 13+ messages in thread From: Kevin O'Connor @ 2012-04-04 3:31 UTC (permalink / raw) To: Alexey Korolev; +Cc: sfd, seabios, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1522 bytes --] On Tue, Apr 03, 2012 at 06:39:22PM +1200, Alexey Korolev wrote: > Hi Kevin, > > Thank you for the patches! > I've created a diff of final version of your changes over mine, to make it clear what has changed. > > Rather than including the complete diff, I've just left relevant parts and added comments. > Yes - there isn't much difference between your patches and my patches. I was really just playing with your patch. > Structure members naming was one of difficult things when I was writing the code. > The child_bus might be a bit confusing as people may thing that it describes a > child bus in the bus topology,in fact this element describes the bus this pci_region_entry > is representing. On Sunday, it occurred to me that we really don't need either parent_bus or this_bus. > +static int pci_size_to_index(u32 size, enum pci_region_type type) [...] > The only purpose to have these functions is to define the minimum size of pci BAR. > They are used only once. > What if we add size adjustment to pci_region_create_entry, or just create a function like > pci_adjust_size(u32 size, enum pci_region_type type, int bridge)? Agreed - the only thing it does is force a minimum size for memory bars as you pointed out in a previous email. As above, I did play with this a little more on Sunday. I also added in two patches from Gerd's series and made alignment handling more explicit. I'm including the series here if you're interested. Again, I think this should wait until after the 1.7.0 release. -Kevin [-- Attachment #2: 0000-cover-letter.patch --] [-- Type: text/plain, Size: 894 bytes --] >From 1ce91ff107ae43cc7fee5f87809f92078b0394ff Mon Sep 17 00:00:00 2001 Message-Id: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Tue, 3 Apr 2012 23:30:39 -0400 Subject: [PATCH 0/7] *** SUBJECT HERE *** To: seabios@seabios.org *** BLURB HERE *** Alexey Korolev (1): pciinit: Get rid of size element of pci_bus->r structure Kevin O'Connor (6): pciinit: Add a pci_region_entry structure. pciinit: Perform bus bar assignment at same time as normal bar assignment. pciinit: Use sorted order allocation scheme instead of array based count scheme. pciinit: Track region alignment explicitly. pciinit: bridges can have two regions too pciinit: 64bit support. src/pci.h | 5 - src/pciinit.c | 351 ++++++++++++++++++++++++++------------------------------- 2 files changed, 160 insertions(+), 196 deletions(-) -- 1.7.6.5 [-- Attachment #3: 0001-pciinit-Add-a-pci_region_entry-structure.patch --] [-- Type: text/plain, Size: 8131 bytes --] >From 5c88fe3a0a6bfb35abccee88904de11e2ce1896b Mon Sep 17 00:00:00 2001 Message-Id: <5c88fe3a0a6bfb35abccee88904de11e2ce1896b.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 11:23:06 -0400 Subject: [PATCH 1/7] pciinit: Add a pci_region_entry structure. To: seabios@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. Original patch by: Alexey Korolev <alexey.korolev@endace.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/pci.h | 5 -- src/pciinit.c | 115 +++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 80 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..4af4105 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,10 @@ 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, -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 +454,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 +520,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 +581,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 +631,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 [-- Attachment #4: 0002-pciinit-Perform-bus-bar-assignment-at-same-time-as-n.patch --] [-- Type: text/plain, Size: 4481 bytes --] >From 60e411a40bff65d338d2d05c4a431a4b52d1b99e Mon Sep 17 00:00:00 2001 Message-Id: <60e411a40bff65d338d2d05c4a431a4b52d1b99e.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 00:39:20 -0400 Subject: [PATCH 2/7] pciinit: Perform bus bar assignment at same time as normal bar assignment. To: seabios@seabios.org Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/pciinit.c | 53 ++++++++++++++++++----------------------------------- 1 files changed, 18 insertions(+), 35 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 4af4105..79301fd 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -525,8 +525,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), @@ -535,54 +535,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; -- 1.7.6.5 [-- Attachment #5: 0003-pciinit-Get-rid-of-size-element-of-pci_bus-r-structu.patch --] [-- Type: text/plain, Size: 2574 bytes --] >From c48faf3b52f0c0fad9da2c6edf4215ae9301b5a9 Mon Sep 17 00:00:00 2001 Message-Id: <c48faf3b52f0c0fad9da2c6edf4215ae9301b5a9.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Alexey Korolev <alexey.korolev@endace.com> Date: Wed, 28 Mar 2012 17:56:44 +1300 Subject: [PATCH 3/7] pciinit: Get rid of size element of pci_bus->r structure To: seabios@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.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 79301fd..eb8c2d7 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,20 +437,18 @@ 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, -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; } -- 1.7.6.5 [-- Attachment #6: 0004-pciinit-Use-sorted-order-allocation-scheme-instead-o.patch --] [-- Type: text/plain, Size: 5478 bytes --] >From 8c2cc80a563c5652b695c62c2002764c81359082 Mon Sep 17 00:00:00 2001 Message-Id: <8c2cc80a563c5652b695c62c2002764c81359082.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 10:58:43 -0400 Subject: [PATCH 4/7] pciinit: Use sorted order allocation scheme instead of array based count scheme. To: seabios@seabios.org 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 eb8c2d7..c9b8fc8 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; @@ -480,38 +456,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 @@ -521,7 +465,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", @@ -560,8 +505,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; -- 1.7.6.5 [-- Attachment #7: 0005-pciinit-Track-region-alignment-explicitly.patch --] [-- Type: text/plain, Size: 5785 bytes --] >From ee86fc205d405c0e3d5caae37a5e9bc057485d56 Mon Sep 17 00:00:00 2001 Message-Id: <ee86fc205d405c0e3d5caae37a5e9bc057485d56.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 12:30:32 -0400 Subject: [PATCH 5/7] pciinit: Track region alignment explicitly. To: seabios@seabios.org 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 c9b8fc8..6ef1be8 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,14 +407,13 @@ 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); 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", @@ -429,7 +424,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) @@ -437,14 +432,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; -- 1.7.6.5 [-- Attachment #8: 0006-pciinit-bridges-can-have-two-regions-too.patch --] [-- Type: text/plain, Size: 1445 bytes --] >From 5c4d5afafd6b3b032fa81250e9a32ccb89ed5739 Mon Sep 17 00:00:00 2001 Message-Id: <5c4d5afafd6b3b032fa81250e9a32ccb89ed5739.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 15:21:24 -0400 Subject: [PATCH 6/7] pciinit: bridges can have two regions too To: seabios@seabios.org Original patch by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/pciinit.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 6ef1be8..7d3f076 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -370,13 +370,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) // Calculate resources needed for regular (non-bus) devices. struct pci_device *pci; foreachpci(pci) { + int num_regions = PCI_NUM_REGIONS; if (pci->class == PCI_CLASS_BRIDGE_PCI) { busses[pci->secondary_bus].bus_dev = pci; - continue; + num_regions = 2; } struct pci_bus *bus = &busses[pci_bdf_to_bus(pci->bdf)]; int i; - for (i = 0; i < PCI_NUM_REGIONS; i++) { + for (i = 0; i < num_regions; i++) { u32 val, size; pci_bios_get_bar(pci, i, &val, &size); if (val == 0) -- 1.7.6.5 [-- Attachment #9: 0007-pciinit-64bit-support.patch --] [-- Type: text/plain, Size: 11191 bytes --] >From 1ce91ff107ae43cc7fee5f87809f92078b0394ff Mon Sep 17 00:00:00 2001 Message-Id: <1ce91ff107ae43cc7fee5f87809f92078b0394ff.1333510239.git.kevin@koconnor.net> In-Reply-To: <cover.1333510239.git.kevin@koconnor.net> References: <cover.1333510239.git.kevin@koconnor.net> From: Kevin O'Connor <kevin@koconnor.net> Date: Sun, 1 Apr 2012 16:09:59 -0400 Subject: [PATCH 7/7] pciinit: 64bit support. To: seabios@seabios.org Makes pciinit.c 64bit aware. Use 64bit everywhere. Support discovery and configuration of 64bit bars, with non-zero upper32 bits. 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 7d3f076..69d2d62 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) { @@ -378,17 +392,14 @@ static int pci_bios_check_devices(struct pci_bus *busses) struct pci_bus *bus = &busses[pci_bdf_to_bus(pci->bdf)]; int i; for (i = 0; i < num_regions; i++) { - 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) @@ -408,16 +419,16 @@ 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); 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]); } @@ -428,8 +439,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; @@ -443,8 +457,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"); } @@ -461,23 +474,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); @@ -491,8 +502,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); } } @@ -530,9 +541,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; @@ -552,9 +560,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); -- 1.7.6.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 4/4] Get rid of size element of pci_bus->r structure 2012-03-28 4:25 [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3) Alexey Korolev ` (2 preceding siblings ...) 2012-03-28 4:54 ` [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list Alexey Korolev @ 2012-03-28 4:56 ` Alexey Korolev 3 siblings, 0 replies; 13+ messages in thread From: Alexey Korolev @ 2012-03-28 4:56 UTC (permalink / raw) To: kevin; +Cc: sfd, seabios, qemu-devel 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> --- src/pciinit.c | 21 +++++++++------------ 1 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/pciinit.c b/src/pciinit.c index 85fe823..f07153a 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -48,7 +48,6 @@ struct pci_bus { struct { /* pci region stats */ u32 sum, max; - u32 size; u32 base; struct pci_region_entry *list; } r[PCI_REGION_TYPE_COUNT]; @@ -414,24 +413,22 @@ static int pci_bios_check_devices(struct pci_bus *busses) continue; struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; int type; + u32 size; 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); - entry = pci_region_create_entry(parent, s->bus_dev, - s->r[type].size, type, 0); + size = s->r[type].sum; + if (size < limit) + size = limit; + size = pci_size_roundup(size); + entry = pci_region_create_entry(parent, s->bus_dev, size, type, 0); if (!entry) return -1; entry->this_bus = s; + dprintf(1, "PCI: secondary bus %d size 0x%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.5.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-04-04 3:31 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-28 4:25 [Qemu-devel] [PATCH 0/4] Redesign of pciinit.c (take 3) Alexey Korolev 2012-03-28 4:28 ` [Qemu-devel] [PATCH 1/4] Add basic linked list operations Alexey Korolev 2012-03-28 14:39 ` [Qemu-devel] [SeaBIOS] " Gerd Hoffmann 2012-03-29 2:03 ` Alexey Korolev 2012-03-29 2:27 ` Kevin O'Connor 2012-03-29 10:41 ` Ian Campbell 2012-03-28 4:41 ` [Qemu-devel] [PATCH 2/4] Added a pci_region_entry structure Alexey Korolev 2012-03-28 4:54 ` [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list Alexey Korolev 2012-04-01 7:28 ` Kevin O'Connor 2012-04-01 8:44 ` Kevin O'Connor 2012-04-03 6:39 ` Alexey Korolev 2012-04-04 3:31 ` Kevin O'Connor 2012-03-28 4:56 ` [Qemu-devel] [PATCH 4/4] Get rid of size element of pci_bus->r structure Alexey Korolev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).