[SeaBIOS] [PATCH 3/4] Switch from array based resource allocation to list

Kevin O'Connor kevin at koconnor.net
Sun Apr 1 09:28:34 CEST 2012


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
-------------- next part --------------
>From 2f6e81d884dfbb01a12ddfb10a64bf87e864a19c Mon Sep 17 00:00:00 2001
From: Alexey Korolev <alexey.korolev at endace.com>
Date: Wed, 28 Mar 2012 17:41:41 +1300
Subject: [PATCH 1/3] Added a pci_region_entry structure
To: seabios at 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 at endace.com>
Signed-off-by: Kevin O'Connor <kevin at 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

-------------- next part --------------
>From 6b6f20425b14f7f99aefb28a8553363573544409 Mon Sep 17 00:00:00 2001
From: Kevin O'Connor <kevin at 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 at seabios.org

Signed-off-by: Kevin O'Connor <kevin at 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

-------------- next part --------------
>From 91b8ba37b87187643b96dca9eb135998eeead44e Mon Sep 17 00:00:00 2001
From: Alexey Korolev <alexey.korolev at 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 at 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 at 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



More information about the SeaBIOS mailing list