[Patch 0/12] Pciinit redesign and 64bit PCI support

[Alexey Korolev]

This patch series redesigns and adds new features to pciinit.c code.

[Patches 1-4]
There are no more arrays of bases and counts in new implementation. The
new implementation is based on dynamic allocation of pci_region_entry
structures which are grouped into lists. Each pci_region_entry structure
could be a PCI bar or a downstream PCI region (bridge). Each entry
has a set of attributes: type (IO, MEM,PREFMEM), is64bit, size.

[Patch 5]
Bridge regions are no longer rounded up to the next highest size - 
instead track alignment explicitly.  This should improve the memory layout 
for bridge regions.
Also, unused bridge regions will no longer be allocated any space.

[Patch 6]
Patch takes into account PCI bar and ROM regions of PCI bridges

[Patches 7-12]
Make pciinit.c 64bit aware. Support discovery and configuration of 64bit
bars, with non-zero upper32 bits. Code allocates the 64bit PCI bars in
high address range if they don't fir in 32bit range.

Kevin O'Konor (6):
0001-pciinit-Introduction-of-pci_region_entry-structure.patch
0002-pciinit-Move-bus-bar-asignment.patch
0004-pciinit-Use-sorted-order-allocation.patch
0005-pciinit-Track-region-alignment-explicitly.patch
0006-pciinit-bridges-can-have-two-regions-too.patch
0007-pciinit-Switch-to-64bit-variable-types.patch

Alexey Korolev (6):
0003-pciinit-Remove-size-element-from-pci_bus-r-structure.patch
0008-pciinit-Add-pci_region-structure.patch
0009-pciinit-64bit-capability-discovery-for-pci-bridges.patch
0010-Do-not-store-pci-region-stats-instead-calulate-the-s.patch
0011-Migrate-64bit-entries-to-64bit-pci-regions.patch
0012-Fix-64bit-PCI-issues-on-Windows.patch

src/acpi-dsdt.dsl |    7 +
 src/acpi-dsdt.hex |   64 +++++++-
 src/config.h      |    2 +
 src/pci.h         |    6 +-
 src/pciinit.c     |  464 ++++++++++++++++++++++++++++++-----------------------
 5 files changed, 325 insertions(+), 218 deletions(-)

[PATCH 01/12] pciinit: Introduction of pci_region_entry structure

[Alexey Korolev]

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 
Signed-off-by: Kevin O'Connor 
---
 src/pci.h     |    5 --
 src/pciinit.c |  116 +++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 81 insertions(+), 40 deletions(-)

diff --git a/src/pci.h b/src/pci.h
index a2a5a4c..5598100 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -51,11 +51,6 @@ struct pci_device {
     u8 prog_if, revision;
     u8 header_type;
     u8 secondary_bus;
-    struct {
-        u32 addr;
-        u32 size;
-        int is64;
-    } bars[PCI_NUM_REGIONS];

// Local information on device.
     int have_driver;
diff --git a/src/pciinit.c b/src/pciinit.c
index 9f3fdd4..953f3bd 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -31,6 +31,15 @@ static const char *region_type_name[] = {
     [ PCI_REGION_TYPE_PREFMEM ] = "prefmem",
 };

+struct pci_region_entry {
+    struct pci_device *dev;
+    int bar;
+    u32 size;
+    int is64;
+    enum pci_region_type type;
+    struct pci_region_entry *next;
+};
+
 struct pci_bus {
     struct {
         /* pci region stats */
@@ -41,6 +50,7 @@ struct pci_bus {
         /* pci region assignments */
         u32 bases[32 - PCI_MEM_INDEX_SHIFT];
         u32 base;
+        struct pci_region_entry *list;
     } r[PCI_REGION_TYPE_COUNT];
     struct pci_device *bus_dev;
 };
@@ -352,19 +362,41 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
     *size = (~(*val & mask)) + 1;
 }

-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size)
+static struct pci_region_entry *
+pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
+                        int bar, u32 size, int type, int is64)
 {
-    u32 index;
+    struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
+    if (!entry) {
+        warn_noalloc();
+        return NULL;
+    }
+    memset(entry, 0, sizeof(*entry));
+    entry->dev = dev;
+    entry->bar = bar;
+    entry->size = size;
+    entry->is64 = is64;
+    entry->type = type;
+    // Insert into list in sorted order.
+    struct pci_region_entry **pprev;
+    for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) {
+        struct pci_region_entry *pos = *pprev;
+        if (pos->size < size)
+            break;
+    }
+    entry->next = *pprev;
+    *pprev = entry;

-    index = pci_size_to_index(size, type);
+    int index = pci_size_to_index(size, type);
     size = pci_index_to_size(index, type);
     bus->r[type].count[index]++;
     bus->r[type].sum += size;
     if (bus->r[type].max < size)
         bus->r[type].max = size;
+    return entry;
 }

-static void pci_bios_check_devices(struct pci_bus *busses)
+static int pci_bios_check_devices(struct pci_bus *busses)
 {
     dprintf(1, "PCI: check devices\n");

@@ -383,14 +415,15 @@ static void pci_bios_check_devices(struct pci_bus *busses)
             if (val == 0)
                 continue;

-            pci_bios_bus_reserve(bus, pci_addr_to_type(val), size);
-            pci->bars[i].addr = val;
-            pci->bars[i].size = size;
-            pci->bars[i].is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
-                                 (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
-                                 == PCI_BASE_ADDRESS_MEM_TYPE_64);
+            int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
+                        (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+                        == PCI_BASE_ADDRESS_MEM_TYPE_64);
+            struct pci_region_entry *entry = pci_region_create_entry(
+                bus, pci, i, size, pci_addr_to_type(val), is64);
+            if (!entry)
+                return -1;

-            if (pci->bars[i].is64)
+            if (is64)
                 i++;
         }
     }
@@ -410,7 +443,11 @@ static void pci_bios_check_devices(struct pci_bus *busses)
             if (s->r[type].size < limit)
                 s->r[type].size = limit;
             s->r[type].size = pci_size_roundup(s->r[type].size);
-            pci_bios_bus_reserve(parent, type, s->r[type].size);
+            // entry->bar is -1 if the entry represents a bridge region
+            struct pci_region_entry *entry = pci_region_create_entry(
+                parent, s->bus_dev, -1, s->r[type].size, type, 0);
+            if (!entry)
+                return -1;
         }
         dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n",
                 secondary_bus,
@@ -418,6 +455,7 @@ static void pci_bios_check_devices(struct pci_bus *busses)
                 s->r[PCI_REGION_TYPE_MEM].size,
                 s->r[PCI_REGION_TYPE_PREFMEM].size);
     }
+    return 0;
 }

#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
@@ -483,6 +521,24 @@ static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
 #define PCI_MEMORY_SHIFT        16
 #define PCI_PREF_MEMORY_SHIFT   16

+static void
+pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
+{
+    u16 bdf = entry->dev->bdf;
+    struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
+    if (entry->bar >= 0) {
+        u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size);
+        dprintf(1, "PCI: map device bdf=%02x:%02x.%x"
+                "  bar %d, addr %08x, size %08x [%s]\n",
+                pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf),
+                entry->bar, addr, entry->size, region_type_name[entry->type]);
+
+        pci_set_io_region_addr(entry->dev, entry->bar, addr);
+        if (entry->is64)
+            pci_set_io_region_addr(entry->dev, entry->bar + 1, 0);
+    }
+}
+
 static void pci_bios_map_devices(struct pci_bus *busses)
 {
     // Setup bases for root bus.
@@ -526,28 +582,16 @@ static void pci_bios_map_devices(struct pci_bus *busses)
     }

// Map regions on each device.
-    struct pci_device *pci;
-    foreachpci(pci) {
-        if (pci->class == PCI_CLASS_BRIDGE_PCI)
-            continue;
-        u16 bdf = pci->bdf;
-        dprintf(1, "PCI: map device bdf=%02x:%02x.%x\n"
-                , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf));
-        struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
-        int i;
-        for (i = 0; i < PCI_NUM_REGIONS; i++) {
-            if (pci->bars[i].addr == 0)
-                continue;
-
-            int type = pci_addr_to_type(pci->bars[i].addr);
-            u32 addr = pci_bios_bus_get_addr(bus, type, pci->bars[i].size);
-            dprintf(1, "  bar %d, addr %x, size %x [%s]\n",
-                    i, addr, pci->bars[i].size, region_type_name[type]);
-            pci_set_io_region_addr(pci, i, addr);
-
-            if (pci->bars[i].is64) {
-                i++;
-                pci_set_io_region_addr(pci, i, 0);
+    int bus;
+    for (bus = 0; bus<=MaxPCIBus; bus++) {
+        int type;
+        for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
+            struct pci_region_entry *entry = busses[bus].r[type].list;
+            while (entry) {
+                pci_region_map_one_entry(busses, entry);
+                struct pci_region_entry *next = entry->next;
+                free(entry);
+                entry = next;
             }
         }
     }
@@ -588,7 +632,9 @@ pci_setup(void)
         return;
     }
     memset(busses, 0, sizeof(*busses) * (MaxPCIBus + 1));
-    pci_bios_check_devices(busses);
+    if (pci_bios_check_devices(busses))
+        return;
+
     if (pci_bios_init_root_regions(&busses[0], start, end) != 0) {
         panic("PCI: out of address space\n");
     }

[PATCH 02/12] pciinit: Move bus bar asignment

[Alexey Korolev]

Perform bus bar assignment at same time as normal bar assignment

Signed-off-by: Kevin O'Connor 
Signed-off-by: Alexey Korolev

---
 src/pciinit.c |   53 ++++++++++++++++++-----------------------------------
 1 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 953f3bd..74ade52 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -526,8 +526,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
 {
     u16 bdf = entry->dev->bdf;
     struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
+    u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size);
     if (entry->bar >= 0) {
-        u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size);
         dprintf(1, "PCI: map device bdf=%02x:%02x.%x"
                 "  bar %d, addr %08x, size %08x [%s]\n",
                 pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf),
@@ -536,54 +536,37 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
         pci_set_io_region_addr(entry->dev, entry->bar, addr);
         if (entry->is64)
             pci_set_io_region_addr(entry->dev, entry->bar + 1, 0);
+        return;
     }
-}
-
-static void pci_bios_map_devices(struct pci_bus *busses)
-{
-    // Setup bases for root bus.
-    dprintf(1, "PCI: init bases bus 0 (primary)\n");
-    pci_bios_init_bus_bases(&busses[0]);
-
-    // Map regions on each secondary bus.
-    int secondary_bus;
-    for (secondary_bus=1; secondary_bus<=MaxPCIBus; secondary_bus++) {
-        struct pci_bus *s = &busses[secondary_bus];
-        if (!s->bus_dev)
-            continue;
-        u16 bdf = s->bus_dev->bdf;
-        struct pci_bus *parent = &busses[pci_bdf_to_bus(bdf)];
-        int type;
-        for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
-            s->r[type].base = pci_bios_bus_get_addr(
-                parent, type, s->r[type].size);
-        }
-        dprintf(1, "PCI: init bases bus %d (secondary)\n", secondary_bus);
-        pci_bios_init_bus_bases(s);

-        u32 base = s->r[PCI_REGION_TYPE_IO].base;
-        u32 limit = base + s->r[PCI_REGION_TYPE_IO].size - 1;
-        pci_config_writeb(bdf, PCI_IO_BASE, base >> PCI_IO_SHIFT);
+    struct pci_bus *child_bus = &busses[entry->dev->secondary_bus];
+    child_bus->r[entry->type].base = addr;
+    u32 limit = addr + entry->size - 1;
+    if (entry->type == PCI_REGION_TYPE_IO) {
+        pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
         pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
         pci_config_writeb(bdf, PCI_IO_LIMIT, limit >> PCI_IO_SHIFT);
         pci_config_writew(bdf, PCI_IO_LIMIT_UPPER16, 0);
-
-        base = s->r[PCI_REGION_TYPE_MEM].base;
-        limit = base + s->r[PCI_REGION_TYPE_MEM].size - 1;
-        pci_config_writew(bdf, PCI_MEMORY_BASE, base >> PCI_MEMORY_SHIFT);
+    }
+    if (entry->type == PCI_REGION_TYPE_MEM) {
+        pci_config_writew(bdf, PCI_MEMORY_BASE, addr >> PCI_MEMORY_SHIFT);
         pci_config_writew(bdf, PCI_MEMORY_LIMIT, limit >> PCI_MEMORY_SHIFT);
-
-        base = s->r[PCI_REGION_TYPE_PREFMEM].base;
-        limit = base + s->r[PCI_REGION_TYPE_PREFMEM].size - 1;
-        pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, base >> PCI_PREF_MEMORY_SHIFT);
+    }
+    if (entry->type == PCI_REGION_TYPE_PREFMEM) {
+        pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, addr >> PCI_PREF_MEMORY_SHIFT);
         pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> PCI_PREF_MEMORY_SHIFT);
         pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0);
         pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0);
     }
+}

+static void pci_bios_map_devices(struct pci_bus *busses)
+{
     // Map regions on each device.
     int bus;
     for (bus = 0; bus<=MaxPCIBus; bus++) {
+        dprintf(1, "PCI: init bases bus %d\n", bus);
+        pci_bios_init_bus_bases(&busses[bus]);
         int type;
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             struct pci_region_entry *entry = busses[bus].r[type].list;

[PATCH 03/12] pciinit: Remove size element from pci_bus->r structure

[Alexey Korolev]

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 
Signed-off-by: Kevin O'Connor 
---
 src/pciinit.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 74ade52..4617793 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -45,8 +45,6 @@ struct pci_bus {
         /* pci region stats */
         u32 count[32 - PCI_MEM_INDEX_SHIFT];
         u32 sum, max;
-        /* seconday bus region sizes */
-        u32 size;
         /* pci region assignments */
         u32 bases[32 - PCI_MEM_INDEX_SHIFT];
         u32 base;
@@ -439,21 +437,19 @@ static int pci_bios_check_devices(struct pci_bus *busses)
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u32 limit = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-            s->r[type].size = s->r[type].sum;
-            if (s->r[type].size < limit)
-                s->r[type].size = limit;
-            s->r[type].size = pci_size_roundup(s->r[type].size);
+            u32 size = s->r[type].sum;
+            if (size < limit)
+                size = limit;
+            size = pci_size_roundup(size);
             // entry->bar is -1 if the entry represents a bridge region
             struct pci_region_entry *entry = pci_region_create_entry(
-                parent, s->bus_dev, -1, s->r[type].size, type, 0);
+                parent, s->bus_dev, -1, size, type, 0);
             if (!entry)
                 return -1;
+            dprintf(1, "PCI: secondary bus %d size %x type %s\n",
+                      entry->dev->secondary_bus, size,
+                      region_type_name[entry->type]);
         }
-        dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n",
-                secondary_bus,
-                s->r[PCI_REGION_TYPE_IO].size,
-                s->r[PCI_REGION_TYPE_MEM].size,
-                s->r[PCI_REGION_TYPE_PREFMEM].size);
     }
     return 0;
 }

[PATCH 04/12] pciinit: Use sorted order allocation

[Alexey Korolev]

Use sorted order allocation scheme instead of
array based count scheme.

Signed-off-by: Alexey Korolev 
Signed-off-by: Kevin O'Connor 
---
 src/pciinit.c |   71 +++++---------------------------------------------------
 1 files changed, 7 insertions(+), 64 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 4617793..1b31177 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -12,9 +12,7 @@
 #include "pci_regs.h" // PCI_COMMAND
 #include "xen.h" // usingXen

-#define PCI_IO_INDEX_SHIFT 2
-#define PCI_MEM_INDEX_SHIFT 12
-
+#define PCI_DEVICE_MEM_MIN     0x1000
 #define PCI_BRIDGE_IO_MIN      0x1000
 #define PCI_BRIDGE_MEM_MIN   0x100000

@@ -43,36 +41,14 @@ struct pci_region_entry {
 struct pci_bus {
     struct {
         /* pci region stats */
-        u32 count[32 - PCI_MEM_INDEX_SHIFT];
         u32 sum, max;
         /* pci region assignments */
-        u32 bases[32 - PCI_MEM_INDEX_SHIFT];
         u32 base;
         struct pci_region_entry *list;
     } r[PCI_REGION_TYPE_COUNT];
     struct pci_device *bus_dev;
 };

-static int pci_size_to_index(u32 size, enum pci_region_type type)
-{
-    int index = __fls(size);
-    int shift = (type == PCI_REGION_TYPE_IO) ?
-        PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-    if (index < shift)
-        index = shift;
-    index -= shift;
-    return index;
-}
-
-static u32 pci_index_to_size(int index, enum pci_region_type type)
-{
-    int shift = (type == PCI_REGION_TYPE_IO) ?
-        PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-    return 0x1 << (index + shift);
-}
-
 static enum pci_region_type pci_addr_to_type(u32 addr)
 {
     if (addr & PCI_BASE_ADDRESS_SPACE_IO)
@@ -385,9 +361,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
     entry->next = *pprev;
     *pprev = entry;

-    int index = pci_size_to_index(size, type);
-    size = pci_index_to_size(index, type);
-    bus->r[type].count[index]++;
     bus->r[type].sum += size;
     if (bus->r[type].max < size)
         bus->r[type].max = size;
@@ -413,11 +386,14 @@ static int pci_bios_check_devices(struct pci_bus *busses)
             if (val == 0)
                 continue;

+            int type = pci_addr_to_type(val);
+            if (type != PCI_REGION_TYPE_IO && size < PCI_DEVICE_MEM_MIN)
+                size = PCI_DEVICE_MEM_MIN;
             int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
                         (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
                         == PCI_BASE_ADDRESS_MEM_TYPE_64);
             struct pci_region_entry *entry = pci_region_create_entry(
-                bus, pci, i, size, pci_addr_to_type(val), is64);
+                bus, pci, i, size, type, is64);
             if (!entry)
                 return -1;

@@ -481,38 +457,6 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end)
  * BAR assignment
  ****************************************************************/

-static void pci_bios_init_bus_bases(struct pci_bus *bus)
-{
-    u32 base, newbase, size;
-    int type, i;
-
-    for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
-        dprintf(1, "  type %s max %x sum %x base %x\n", region_type_name[type],
-                bus->r[type].max, bus->r[type].sum, bus->r[type].base);
-        base = bus->r[type].base;
-        for (i = ARRAY_SIZE(bus->r[type].count)-1; i >= 0; i--) {
-            size = pci_index_to_size(i, type);
-            if (!bus->r[type].count[i])
-                continue;
-            newbase = base + size * bus->r[type].count[i];
-            dprintf(1, "    size %8x: %d bar(s), %8x -> %8x\n",
-                    size, bus->r[type].count[i], base, newbase - 1);
-            bus->r[type].bases[i] = base;
-            base = newbase;
-        }
-    }
-}
-
-static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
-{
-    u32 index, addr;
-
-    index = pci_size_to_index(size, type);
-    addr = bus->r[type].bases[index];
-    bus->r[type].bases[index] += pci_index_to_size(index, type);
-    return addr;
-}
-
 #define PCI_IO_SHIFT            8
 #define PCI_MEMORY_SHIFT        16
 #define PCI_PREF_MEMORY_SHIFT   16
@@ -522,7 +466,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
 {
     u16 bdf = entry->dev->bdf;
     struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
-    u32 addr = pci_bios_bus_get_addr(bus, entry->type, entry->size);
+    u32 addr = bus->r[entry->type].base;
+    bus->r[entry->type].base += entry->size;
     if (entry->bar >= 0) {
         dprintf(1, "PCI: map device bdf=%02x:%02x.%x"
                 "  bar %d, addr %08x, size %08x [%s]\n",
@@ -561,8 +506,6 @@ static void pci_bios_map_devices(struct pci_bus *busses)
     // Map regions on each device.
     int bus;
     for (bus = 0; bus<=MaxPCIBus; bus++) {
-        dprintf(1, "PCI: init bases bus %d\n", bus);
-        pci_bios_init_bus_bases(&busses[bus]);
         int type;
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             struct pci_region_entry *entry = busses[bus].r[type].list;

[PATCH 05/12] pciinit: Track region alignment explicitly.

[Alexey Korolev]

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 
---
 src/pciinit.c |   41 ++++++++++++++++++-----------------------
 1 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 1b31177..2bd4426 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -33,6 +33,7 @@ struct pci_region_entry {
     struct pci_device *dev;
     int bar;
     u32 size;
+    u32 align;
     int is64;
     enum pci_region_type type;
     struct pci_region_entry *next;
@@ -41,7 +42,7 @@ struct pci_region_entry {
 struct pci_bus {
     struct {
         /* pci region stats */
-        u32 sum, max;
+        u32 sum, align;
         /* pci region assignments */
         u32 base;
         struct pci_region_entry *list;
@@ -307,12 +308,6 @@ pci_bios_init_bus(void)
  * Bus sizing
  ****************************************************************/

-static u32 pci_size_roundup(u32 size)
-{
-    int index = __fls(size-1)+1;
-    return 0x1 << index;
-}
-
 static void
 pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
 {
@@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
u32 *val, u32 *size)

static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
-                        int bar, u32 size, int type, int is64)
+                        int bar, u32 size, u32 align, int type, int
is64)
 {
     struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
     if (!entry) {
@@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus,
struct pci_device *dev,
     entry->dev = dev;
     entry->bar = bar;
     entry->size = size;
+    entry->align = align;
     entry->is64 = is64;
     entry->type = type;
     // Insert into list in sorted order.
     struct pci_region_entry **pprev;
     for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) {
         struct pci_region_entry *pos = *pprev;
-        if (pos->size < size)
+        if (pos->align < align || (pos->align == align && pos->size <
size))
             break;
     }
     entry->next = *pprev;
     *pprev = entry;

bus->r[type].sum += size;
-    if (bus->r[type].max < size)
-        bus->r[type].max = size;
+    if (bus->r[type].align < align)
+        bus->r[type].align = align;
     return entry;
 }

@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
                         (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
                         == PCI_BASE_ADDRESS_MEM_TYPE_64);
             struct pci_region_entry *entry = pci_region_create_entry(
-                bus, pci, i, size, type, is64);
+                bus, pci, i, size, size, type, is64);
             if (!entry)
                 return -1;

@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
         struct pci_bus *parent =
&busses[pci_bdf_to_bus(s->bus_dev->bdf)];
         int type;
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
-            u32 limit = (type == PCI_REGION_TYPE_IO) ?
+            u32 align = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-            u32 size = s->r[type].sum;
-            if (size < limit)
-                size = limit;
-            size = pci_size_roundup(size);
+            if (s->r[type].align > align)
+                align = s->r[type].align;
+            u32 size = ALIGN(s->r[type].sum, align);
             // entry->bar is -1 if the entry represents a bridge region
             struct pci_region_entry *entry = pci_region_create_entry(
-                parent, s->bus_dev, -1, size, type, 0);
+                parent, s->bus_dev, -1, size, align, type, 0);
             if (!entry)
                 return -1;
             dprintf(1, "PCI: secondary bus %d size %x type %s\n",
@@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
     return 0;
 }

-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
+#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)

// Setup region bases (given the regions' size and alignment)
 static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start,
u32 end)
@@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct
pci_bus *bus, u32 start, u32 end)
     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-    if (bus->r[reg1].sum < bus->r[reg2].sum) {
-        // Swap regions so larger area is more likely to align well.
+    if (bus->r[reg1].align < bus->r[reg2].align) {
+        // Swap regions to improve alignment.
         reg1 = PCI_REGION_TYPE_MEM;
         reg2 = PCI_REGION_TYPE_PREFMEM;
     }
-    bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].max);
+    bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].align);
     bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
-                                  , bus->r[reg1].max);
+                                  , bus->r[reg1].align);
     if (bus->r[reg1].base < start)
         // Memory range requested is larger than available.
         return -1;

[PATCH 05/12] pciinit: Track region alignment explicitly.

[Gerd Hoffmann]

On 04/24/12 08:17, Alexey Korolev wrote:
> Don't round up bridge regions to the next highest size - instead track
> alignment explicitly.  This should improve the memory layout for
> bridge regions.

This one got mangled too:

Applying: pciinit: Track region alignment explicitly.
fatal: corrupt patch at line 40
Patch failed at 0005 pciinit: Track region alignment explicitly.

Do you have a git tree I can pull from for testing?

thanks,
  Gerd
> Also, unused bridge regions will no longer be allocated any space.
> Signed-off-by: Kevin O'Connor
>
> src/pciinit.c |   41 ++++++++++++++++++-----------------------
>  1 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/src/pciinit.c b/src/pciinit.c
> index 1b31177..2bd4426 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -33,6 +33,7 @@ struct pci_region_entry {
>      struct pci_device *dev;
>      int bar;
>      u32 size;
> u32 align;
> int is64;
> enum pci_region_type type;
> struct pci_region_entry *next;
>
> @@ -41,7 +42,7 @@ struct pci_region_entry {
>  struct pci_bus {
>      struct {
>          /* pci region stats */
>    u32 sum, max;
>    u32 sum, align;
>    /* pci region assignments */
>    u32 base;
>    struct pci_region_entry *list;
>
> @@ -307,12 +308,6 @@ pci_bios_init_bus(void)
> Bus sizing
>
> ****************************************************************/
>
> -static u32 pci_size_roundup(u32 size)
> -{
> int index = __fls(size-1)+1;
> return 0x1 << index;
>
> -}
>
> static void
>  pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
>  {
> @@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
> u32 *val, u32 *size)
>
> static struct pci_region_entry *
>  pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
>                    int bar, u32 size, int type, int is64)
>                    int bar, u32 size, u32 align, int type, int
>
> is64)
>  {
>      struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
>      if (!entry) {
> @@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus,
> struct pci_device *dev,
>      entry->dev = dev;
>      entry->bar = bar;
>      entry->size = size;
> entry->align = align;
> entry->is64 = is64;
> entry->type = type;
> // Insert into list in sorted order.
> struct pci_region_entry **pprev;
> for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) {
>     struct pci_region_entry *pos = *pprev;
>    if (pos->size < size)
>    if (pos->align < align || (pos->align == align && pos->size <
>
> size))
>              break;
>      }
>      entry->next = *pprev;
>      *pprev = entry;
>  bus->r[type].sum += size;
> if (bus->r[type].max < size)
>    bus->r[type].max = size;
> if (bus->r[type].align < align)
>    bus->r[type].align = align;
> return entry;
>
> }
>
> @@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus
> *busses)
>                          (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
>                          == PCI_BASE_ADDRESS_MEM_TYPE_64);
>              struct pci_region_entry *entry = pci_region_create_entry(
>            bus, pci, i, size, type, is64);
>            bus, pci, i, size, size, type, is64);
>        if (!entry)
>            return -1;
>
> @@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus
> *busses)
>          struct pci_bus *parent =
> &busses[pci_bdf_to_bus(s->bus_dev->bdf)];
>          int type;
>          for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>        u32 limit = (type == PCI_REGION_TYPE_IO) ?
>        u32 align = (type == PCI_REGION_TYPE_IO) ?
>            PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>        u32 size = s->r[type].sum;
>        if (size < limit)
>            size = limit;
>        size = pci_size_roundup(size);
>        if (s->r[type].align > align)
>            align = s->r[type].align;
>        u32 size = ALIGN(s->r[type].sum, align);
>        // entry->bar is -1 if the entry represents a bridge region
>        struct pci_region_entry *entry = pci_region_create_entry(
>            parent, s->bus_dev, -1, size, type, 0);
>            parent, s->bus_dev, -1, size, align, type, 0);
>        if (!entry)
>            return -1;
>        dprintf(1, "PCI: secondary bus %d size %x type %s\n",
>
> @@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus
> *busses)
>      return 0;
>  }
>
> -#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
> +#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
>
> // Setup region bases (given the regions' size and alignment)
>  static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start,
> u32 end)
> @@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct
> pci_bus *bus, u32 start, u32 end)
>      bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
>  int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
> if (bus->r[reg1].sum < bus->r[reg2].sum) {
>    // Swap regions so larger area is more likely to align well.
> if (bus->r[reg1].align < bus->r[reg2].align) {
>    // Swap regions to improve alignment.
>    reg1 = PCI_REGION_TYPE_MEM;
>    reg2 = PCI_REGION_TYPE_PREFMEM;
> }
> bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
>
> bus->r[reg2].max);
> bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
>
> bus->r[reg2].align);
>      bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
>                              , bus->r[reg1].max);
>                              , bus->r[reg1].align);
> if (bus->r[reg1].base < start)
>        // Memory range requested is larger than available.
>        return -1;

[PATCH 05/12] pciinit: Track region alignment explicitly.

[Alexey Korolev]

On 24/04/12 18:56, Gerd Hoffmann wrote:
> On 04/24/12 08:17, Alexey Korolev wrote:
> > Don't round up bridge regions to the next highest size - instead track
> > alignment explicitly.  This should improve the memory layout for
> > bridge regions.
>
> This one got mangled too:
>
> Applying: pciinit: Track region alignment explicitly.
> fatal: corrupt patch at line 40
> Patch failed at 0005 pciinit: Track region alignment explicitly.
>
> Do you have a git tree I can pull from for testing?

Hi,
Thank you.
I don't have an public available git tree.
Not sure if I can create any from workplace.

So I just reposted the patch.
> thanks,
>   Gerd
> > Also, unused bridge regions will no longer be allocated any space.
> > Signed-off-by: Kevin O'Connor
> >
> > src/pciinit.c |   41 ++++++++++++++++++-----------------------
> >  1 files changed, 18 insertions(+), 23 deletions(-)
> >
> > diff --git a/src/pciinit.c b/src/pciinit.c
> > index 1b31177..2bd4426 100644
> > --- a/src/pciinit.c
> > +++ b/src/pciinit.c
> > @@ -33,6 +33,7 @@ struct pci_region_entry {
> >      struct pci_device *dev;
> >      int bar;
> >      u32 size;
> > u32 align;
> > int is64;
> > enum pci_region_type type;
> > struct pci_region_entry *next;
> >
> > @@ -41,7 +42,7 @@ struct pci_region_entry {
> >  struct pci_bus {
> >      struct {
> >          /* pci region stats */
> >    u32 sum, max;
> >    u32 sum, align;
> >    /* pci region assignments */
> >    u32 base;
> >    struct pci_region_entry *list;
> >
> > @@ -307,12 +308,6 @@ pci_bios_init_bus(void)
> > Bus sizing
> >
> > ****************************************************************/
> >
> > -static u32 pci_size_roundup(u32 size)
> > -{
> > int index = __fls(size-1)+1;
> > return 0x1 << index;
> >
> > -}
> >
> > static void
> >  pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
> >  {
> > @@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
> > u32 *val, u32 *size)
> >
> > static struct pci_region_entry *
> >  pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
> >                    int bar, u32 size, int type, int is64)
> >                    int bar, u32 size, u32 align, int type, int
> >
> > is64)
> >  {
> >      struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
> >      if (!entry) {
> > @@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus,
> > struct pci_device *dev,
> >      entry->dev = dev;
> >      entry->bar = bar;
> >      entry->size = size;
> > entry->align = align;
> > entry->is64 = is64;
> > entry->type = type;
> > // Insert into list in sorted order.
> > struct pci_region_entry **pprev;
> > for (pprev = &bus->r[type].list; *pprev; pprev = &(*pprev)->next) {
> >     struct pci_region_entry *pos = *pprev;
> >    if (pos->size < size)
> >    if (pos->align < align || (pos->align == align && pos->size <
> >
> > size))
> >              break;
> >      }
> >      entry->next = *pprev;
> >      *pprev = entry;
> >  bus->r[type].sum += size;
> > if (bus->r[type].max < size)
> >    bus->r[type].max = size;
> > if (bus->r[type].align < align)
> >    bus->r[type].align = align;
> > return entry;
> >
> > }
> >
> > @@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus
> > *busses)
> >                          (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> >                          == PCI_BASE_ADDRESS_MEM_TYPE_64);
> >              struct pci_region_entry *entry = pci_region_create_entry(
> >            bus, pci, i, size, type, is64);
> >            bus, pci, i, size, size, type, is64);
> >        if (!entry)
> >            return -1;
> >
> > @@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus
> > *busses)
> >          struct pci_bus *parent =
> > &busses[pci_bdf_to_bus(s->bus_dev->bdf)];
> >          int type;
> >          for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
> >        u32 limit = (type == PCI_REGION_TYPE_IO) ?
> >        u32 align = (type == PCI_REGION_TYPE_IO) ?
> >            PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> >        u32 size = s->r[type].sum;
> >        if (size < limit)
> >            size = limit;
> >        size = pci_size_roundup(size);
> >        if (s->r[type].align > align)
> >            align = s->r[type].align;
> >        u32 size = ALIGN(s->r[type].sum, align);
> >        // entry->bar is -1 if the entry represents a bridge region
> >        struct pci_region_entry *entry = pci_region_create_entry(
> >            parent, s->bus_dev, -1, size, type, 0);
> >            parent, s->bus_dev, -1, size, align, type, 0);
> >        if (!entry)
> >            return -1;
> >        dprintf(1, "PCI: secondary bus %d size %x type %s\n",
> >
> > @@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus
> > *busses)
> >      return 0;
> >  }
> >
> > -#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
> > +#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
> >
> > // Setup region bases (given the regions' size and alignment)
> >  static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start,
> > u32 end)
> > @@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct
> > pci_bus *bus, u32 start, u32 end)
> >      bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
> >  int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
> > if (bus->r[reg1].sum < bus->r[reg2].sum) {
> >    // Swap regions so larger area is more likely to align well.
> > if (bus->r[reg1].align < bus->r[reg2].align) {
> >    // Swap regions to improve alignment.
> >    reg1 = PCI_REGION_TYPE_MEM;
> >    reg2 = PCI_REGION_TYPE_PREFMEM;
> > }
> > bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
> >
> > bus->r[reg2].max);
> > bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
> >
> > bus->r[reg2].align);
> >      bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
> >                              , bus->r[reg1].max);
> >                              , bus->r[reg1].align);
> > if (bus->r[reg1].base < start)
> >        // Memory range requested is larger than available.
> >        return -1;

[PATCH 06/12] pciinit: bridges can have two regions too

[Alexey Korolev]

Patch takes into account PCI bar and ROM regions of PCI bridges

Original patch by: Gerd Hoffmann 
Signed-off-by: Kevin O'Connor 
Signed-off-by: Alexey Korolev 
---
 src/pci.h     |    1 +
 src/pciinit.c |    8 +++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/pci.h b/src/pci.h
index 5598100..6be838c 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -5,6 +5,7 @@

#define PCI_ROM_SLOT 6
 #define PCI_NUM_REGIONS 7
+#define PCI_BRIDGE_NUM_REGIONS 2

static inline u8 pci_bdf_to_bus(u16 bdf) {
     return bdf >> 8;
diff --git a/src/pciinit.c b/src/pciinit.c
index 2bd4426..9b521e3 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -370,13 +370,15 @@ static int pci_bios_check_devices(struct pci_bus *busses)
     // Calculate resources needed for regular (non-bus) devices.
     struct pci_device *pci;
     foreachpci(pci) {
-        if (pci->class == PCI_CLASS_BRIDGE_PCI) {
+        if (pci->class == PCI_CLASS_BRIDGE_PCI)
             busses[pci->secondary_bus].bus_dev = pci;
-            continue;
-        }
+
         struct pci_bus *bus = &busses[pci_bdf_to_bus(pci->bdf)];
         int i;
         for (i = 0; i < PCI_NUM_REGIONS; i++) {
+            if ((pci->class == PCI_CLASS_BRIDGE_PCI) &&
+                (i >= PCI_BRIDGE_NUM_REGIONS && i < PCI_ROM_SLOT))
+                continue;
             u32 val, size;
             pci_bios_get_bar(pci, i, &val, &size);
             if (val == 0)

[PATCH 07/12] pciinit: Switch to 64bit variable types.

[Alexey Korolev]

Switch to 64bit variable types.
Add parsing  64bit bars.

Original patch by: Gerd Hoffmann 
Signed-off-by: Kevin O'Connor 
---
 src/pciinit.c |  116 ++++++++++++++++++++++++++++++---------------------------
 1 files changed, 61 insertions(+), 55 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 9b521e3..bc603c7 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -32,8 +32,8 @@ static const char *region_type_name[] = {
 struct pci_region_entry {
     struct pci_device *dev;
     int bar;
-    u32 size;
-    u32 align;
+    u64 size;
+    u64 align;
     int is64;
     enum pci_region_type type;
     struct pci_region_entry *next;
@@ -42,23 +42,14 @@ struct pci_region_entry {
 struct pci_bus {
     struct {
         /* pci region stats */
-        u32 sum, align;
+        u64 sum, align;
         /* pci region assignments */
-        u32 base;
+        u64 base;
         struct pci_region_entry *list;
     } r[PCI_REGION_TYPE_COUNT];
     struct pci_device *bus_dev;
 };

-static enum pci_region_type pci_addr_to_type(u32 addr)
-{
-    if (addr & PCI_BASE_ADDRESS_SPACE_IO)
-        return PCI_REGION_TYPE_IO;
-    if (addr & PCI_BASE_ADDRESS_MEM_PREFETCH)
-        return PCI_REGION_TYPE_PREFMEM;
-    return PCI_REGION_TYPE_MEM;
-}
-
 static u32 pci_bar(struct pci_device *pci, int region_num)
 {
     if (region_num != PCI_ROM_SLOT) {
@@ -71,9 +62,12 @@ static u32 pci_bar(struct pci_device *pci, int region_num)
 }

static void
-pci_set_io_region_addr(struct pci_device *pci, int region_num, u32 addr)
+pci_set_io_region_addr(struct pci_device *pci, int bar, u64 addr, int is64)
 {
-    pci_config_writel(pci->bdf, pci_bar(pci, region_num), addr);
+    u32 ofs = pci_bar(pci, bar);
+    pci_config_writel(pci->bdf, ofs, addr);
+    if (is64)
+        pci_config_writel(pci->bdf, ofs + 4, addr >> 32);
 }

@@ -126,10 +120,10 @@ static const struct pci_device_id pci_isa_bridge_tbl[] = {
 static void storage_ide_init(struct pci_device *pci, void *arg)
 {
     /* IDE: we map it as in ISA mode */
-    pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE);
-    pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE);
-    pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE);
-    pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE);
+    pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE, 0);
+    pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE, 0);
+    pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE, 0);
+    pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE, 0);
 }

/* PIIX3/PIIX4 IDE */
@@ -143,13 +137,13 @@ static void piix_ide_init(struct pci_device *pci, void *arg)
 static void pic_ibm_init(struct pci_device *pci, void *arg)
 {
     /* PIC, IBM, MPIC & MPIC2 */
-    pci_set_io_region_addr(pci, 0, 0x80800000 + 0x00040000);
+    pci_set_io_region_addr(pci, 0, 0x80800000 + 0x00040000, 0);
 }

static void apple_macio_init(struct pci_device *pci, void *arg)
 {
     /* macio bridge */
-    pci_set_io_region_addr(pci, 0, 0x80800000);
+    pci_set_io_region_addr(pci, 0, 0x80800000, 0);
 }

static const struct pci_device_id pci_class_tbl[] = {
@@ -309,31 +303,51 @@ pci_bios_init_bus(void)
  ****************************************************************/

static void
-pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
+pci_bios_get_bar(struct pci_device *pci, int bar,
+                 int *ptype, u64 *psize, int *pis64)
 {
     u32 ofs = pci_bar(pci, bar);
     u16 bdf = pci->bdf;
     u32 old = pci_config_readl(bdf, ofs);
-    u32 mask;
+    int is64 = 0, type = PCI_REGION_TYPE_MEM;
+    u64 mask;

if (bar == PCI_ROM_SLOT) {
         mask = PCI_ROM_ADDRESS_MASK;
         pci_config_writel(bdf, ofs, mask);
     } else {
-        if (old & PCI_BASE_ADDRESS_SPACE_IO)
+        if (old & PCI_BASE_ADDRESS_SPACE_IO) {
             mask = PCI_BASE_ADDRESS_IO_MASK;
-        else
+            type = PCI_REGION_TYPE_IO;
+        } else {
             mask = PCI_BASE_ADDRESS_MEM_MASK;
+            if (old & PCI_BASE_ADDRESS_MEM_PREFETCH)
+                type = PCI_REGION_TYPE_PREFMEM;
+            is64 = ((old & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+                    == PCI_BASE_ADDRESS_MEM_TYPE_64);
+        }
         pci_config_writel(bdf, ofs, ~0);
     }
-    *val = pci_config_readl(bdf, ofs);
+    u64 val = pci_config_readl(bdf, ofs);
     pci_config_writel(bdf, ofs, old);
-    *size = (~(*val & mask)) + 1;
+    if (is64) {
+        u32 hold = pci_config_readl(bdf, ofs + 4);
+        pci_config_writel(bdf, ofs + 4, ~0);
+        u32 high = pci_config_readl(bdf, ofs + 4);
+        pci_config_writel(bdf, ofs + 4, hold);
+        val |= ((u64)high << 32);
+        mask |= ((u64)0xffffffff << 32);
+        *psize = (~(val & mask)) + 1;
+    } else {
+        *psize = ((~(val & mask)) + 1) & 0xffffffff;
+    }
+    *ptype = type;
+    *pis64 = is64;
 }

static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
-                        int bar, u32 size, u32 align, int type, int is64)
+                        int bar, u64 size, u64 align, int type, int is64)
 {
     struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
     if (!entry) {
@@ -379,17 +393,14 @@ static int pci_bios_check_devices(struct pci_bus *busses)
             if ((pci->class == PCI_CLASS_BRIDGE_PCI) &&
                 (i >= PCI_BRIDGE_NUM_REGIONS && i < PCI_ROM_SLOT))
                 continue;
-            u32 val, size;
-            pci_bios_get_bar(pci, i, &val, &size);
-            if (val == 0)
+            int type, is64;
+            u64 size;
+            pci_bios_get_bar(pci, i, &type, &size, &is64);
+            if (size == 0)
                 continue;

-            int type = pci_addr_to_type(val);
             if (type != PCI_REGION_TYPE_IO && size < PCI_DEVICE_MEM_MIN)
                 size = PCI_DEVICE_MEM_MIN;
-            int is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
-                        (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
-                        == PCI_BASE_ADDRESS_MEM_TYPE_64);
             struct pci_region_entry *entry = pci_region_create_entry(
                 bus, pci, i, size, size, type, is64);
             if (!entry)
@@ -409,17 +420,17 @@ static int pci_bios_check_devices(struct pci_bus *busses)
         struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)];
         int type;
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
-            u32 align = (type == PCI_REGION_TYPE_IO) ?
+            u64 align = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
             if (s->r[type].align > align)
                 align = s->r[type].align;
-            u32 size = ALIGN(s->r[type].sum, align);
+            u64 size = ALIGN(s->r[type].sum, align);
             // entry->bar is -1 if the entry represents a bridge region
             struct pci_region_entry *entry = pci_region_create_entry(
                 parent, s->bus_dev, -1, size, align, type, 0);
             if (!entry)
                 return -1;
-            dprintf(1, "PCI: secondary bus %d size %x type %s\n",
+            dprintf(1, "PCI: secondary bus %d size %08llx type %s\n",
                       entry->dev->secondary_bus, size,
                       region_type_name[entry->type]);
         }
@@ -430,8 +441,11 @@ static int pci_bios_check_devices(struct pci_bus *busses)
 #define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)

// Setup region bases (given the regions' size and alignment)
-static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end)
+static void pci_bios_init_root_regions(struct pci_bus *bus)
 {
+    u64 start = BUILD_PCIMEM_START;
+    u64 end   = BUILD_PCIMEM_END;
+
     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
@@ -445,8 +459,7 @@ static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start, u32 end)
                                   , bus->r[reg1].align);
     if (bus->r[reg1].base < start)
         // Memory range requested is larger than available.
-        return -1;
-    return 0;
+        panic("PCI: out of address space\n");
 }

@@ -463,23 +476,21 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
 {
     u16 bdf = entry->dev->bdf;
     struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
-    u32 addr = bus->r[entry->type].base;
+    u64 addr = bus->r[entry->type].base;
     bus->r[entry->type].base += entry->size;
     if (entry->bar >= 0) {
         dprintf(1, "PCI: map device bdf=%02x:%02x.%x"
-                "  bar %d, addr %08x, size %08x [%s]\n",
+                "  bar %d, addr %08llx, size %08llx [%s]\n",
                 pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf),
                 entry->bar, addr, entry->size, region_type_name[entry->type]);

-        pci_set_io_region_addr(entry->dev, entry->bar, addr);
-        if (entry->is64)
-            pci_set_io_region_addr(entry->dev, entry->bar + 1, 0);
+        pci_set_io_region_addr(entry->dev, entry->bar, addr, entry->is64);
         return;
     }

struct pci_bus *child_bus = &busses[entry->dev->secondary_bus];
     child_bus->r[entry->type].base = addr;
-    u32 limit = addr + entry->size - 1;
+    u64 limit = addr + entry->size - 1;
     if (entry->type == PCI_REGION_TYPE_IO) {
         pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
         pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
@@ -493,8 +504,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
     if (entry->type == PCI_REGION_TYPE_PREFMEM) {
         pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, addr >> PCI_PREF_MEMORY_SHIFT);
         pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit >> PCI_PREF_MEMORY_SHIFT);
-        pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0);
-        pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0);
+        pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, addr >> 32);
+        pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, limit >> 32);
     }
 }

@@ -532,9 +543,6 @@ pci_setup(void)

dprintf(3, "pci setup\n");

-    u32 start = BUILD_PCIMEM_START;
-    u32 end   = BUILD_PCIMEM_END;
-
     dprintf(1, "=== PCI bus & bridge init ===\n");
     if (pci_probe_host() != 0) {
         return;
@@ -554,9 +562,7 @@ pci_setup(void)
     if (pci_bios_check_devices(busses))
         return;

-    if (pci_bios_init_root_regions(&busses[0], start, end) != 0) {
-        panic("PCI: out of address space\n");
-    }
+    pci_bios_init_root_regions(&busses[0]);

dprintf(1, "=== PCI new allocation pass #2 ===\n");
     pci_bios_map_devices(busses);

[PATCH 08/12] pciinit: Add pci_region structure.

[Alexey Korolev]

The pci_region structure is added.
Move setting of bus base address to pci_region_map_entries.

Signed-off-by: Alexey Korolev 
---
 src/pciinit.c |   50 ++++++++++++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index bc603c7..f185cbd 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -39,14 +39,16 @@ struct pci_region_entry {
     struct pci_region_entry *next;
 };

+struct pci_region {
+    /* pci region stats */
+    u64 sum, align;
+    /* pci region assignments */
+    u64 base;
+    struct pci_region_entry *list;
+};
+
 struct pci_bus {
-    struct {
-        /* pci region stats */
-        u64 sum, align;
-        /* pci region assignments */
-        u64 base;
-        struct pci_region_entry *list;
-    } r[PCI_REGION_TYPE_COUNT];
+    struct pci_region r[PCI_REGION_TYPE_COUNT];
     struct pci_device *bus_dev;
 };

@@ -472,12 +474,9 @@ static void pci_bios_init_root_regions(struct pci_bus *bus)
 #define PCI_PREF_MEMORY_SHIFT   16

static void
-pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
+pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr)
 {
     u16 bdf = entry->dev->bdf;
-    struct pci_bus *bus = &busses[pci_bdf_to_bus(bdf)];
-    u64 addr = bus->r[entry->type].base;
-    bus->r[entry->type].base += entry->size;
     if (entry->bar >= 0) {
         dprintf(1, "PCI: map device bdf=%02x:%02x.%x"
                 "  bar %d, addr %08llx, size %08llx [%s]\n",
@@ -488,8 +487,6 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
         return;
     }

-    struct pci_bus *child_bus = &busses[entry->dev->secondary_bus];
-    child_bus->r[entry->type].base = addr;
     u64 limit = addr + entry->size - 1;
     if (entry->type == PCI_REGION_TYPE_IO) {
         pci_config_writeb(bdf, PCI_IO_BASE, addr >> PCI_IO_SHIFT);
@@ -509,21 +506,30 @@ pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry *entry)
     }
 }

+static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)
+{
+    struct pci_region_entry *entry = r->list;
+    while(entry) {
+        u64 addr = r->base;
+        r->base += entry->size;
+        if (entry->bar == -1)
+            // Update bus base address if entry is a bridge region
+            busses[entry->dev->secondary_bus].r[entry->type].base = addr;
+        pci_region_map_one_entry(entry, addr);
+        struct pci_region_entry *next = entry->next;
+        free(entry);
+        entry = next;
+    }
+}
+
 static void pci_bios_map_devices(struct pci_bus *busses)
 {
     // Map regions on each device.
     int bus;
     for (bus = 0; bus<=MaxPCIBus; bus++) {
         int type;
-        for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
-            struct pci_region_entry *entry = busses[bus].r[type].list;
-            while (entry) {
-                pci_region_map_one_entry(busses, entry);
-                struct pci_region_entry *next = entry->next;
-                free(entry);
-                entry = next;
-            }
-        }
+        for (type = 0; type < PCI_REGION_TYPE_COUNT; type++)
+            pci_region_map_entries(busses, &busses[bus].r[type]);
     }
 }

[PATCH 09/12] pciinit: 64bit capability discovery for pci bridges

[Alexey Korolev]

Add discovery if bridge region is 64bit is capable.

Signed-off-by: Alexey Korolev 
---
 src/pciinit.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index f185cbd..0d66dbe 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -347,6 +347,28 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
     *pis64 = is64;
 }

+static int pci_bios_bridge_region_is64(struct pci_region *r,
+                                 struct pci_device *pci, int type)
+{
+    if (type != PCI_REGION_TYPE_PREFMEM)
+        return 0;
+    u32 pmem = pci_config_readl(pci->bdf, PCI_PREF_MEMORY_BASE);
+    if (!pmem) {
+        pci_config_writel(pci->bdf, PCI_PREF_MEMORY_BASE, 0xfff0fff0);
+        pmem = pci_config_readl(pci->bdf, PCI_PREF_MEMORY_BASE);
+        pci_config_writel(pci->bdf, PCI_PREF_MEMORY_BASE, 0x0);
+    }
+    if ((pmem & PCI_PREF_RANGE_TYPE_MASK) != PCI_PREF_RANGE_TYPE_64)
+       return 0;
+    struct pci_region_entry *entry = r->list;
+    while (entry) {
+        if (!entry->is64)
+            return 0;
+        entry = entry->next;
+    }
+    return 1;
+}
+
 static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
                         int bar, u64 size, u64 align, int type, int is64)
@@ -427,9 +449,11 @@ static int pci_bios_check_devices(struct pci_bus *busses)
             if (s->r[type].align > align)
                 align = s->r[type].align;
             u64 size = ALIGN(s->r[type].sum, align);
+            int is64 = pci_bios_bridge_region_is64(&s->r[type],
+                                            s->bus_dev, type);
             // entry->bar is -1 if the entry represents a bridge region
             struct pci_region_entry *entry = pci_region_create_entry(
-                parent, s->bus_dev, -1, size, align, type, 0);
+                parent, s->bus_dev, -1, size, align, type, is64);
             if (!entry)
                 return -1;
             dprintf(1, "PCI: secondary bus %d size %08llx type %s\n",

[PATCH 10/12] Calculate pci region stats on demand

[Alexey Korolev]

Do not store pci region stats - instead calulate the
sum and alignment on demand.

Signed-off-by: Alexey Korolev 
---
 src/pciinit.c |   57
+++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 0d66dbe..a6cf98b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -40,8 +40,6 @@ struct pci_region_entry {
 };

struct pci_region {
-    /* pci region stats */
-    u64 sum, align;
     /* pci region assignments */
     u64 base;
     struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct
pci_region *r,
     return 1;
 }

+static u64 pci_region_align(struct pci_region *r)
+{
+    if (!r->list)
+        return 1;
+    // The first entry in the sorted list has the largest alignment
+    return r->list->align;
+}
+
+static u64 pci_region_sum(struct pci_region *r)
+{
+    struct pci_region_entry *entry = r->list;
+    u64 sum = 0;
+    while (entry) {
+        sum += entry->size;
+        entry = entry->next;
+   }
+   return sum;
+}
+
 static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
                         int bar, u64 size, u64 align, int type, int
is64)
@@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
     }
     entry->next = *pprev;
     *pprev = entry;
-
-    bus->r[type].sum += size;
-    if (bus->r[type].align < align)
-        bus->r[type].align = align;
     return entry;
 }

@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u64 align = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-            if (s->r[type].align > align)
-                align = s->r[type].align;
-            u64 size = ALIGN(s->r[type].sum, align);
+            if (pci_region_align(&s->r[type]) > align)
+                 align = pci_region_align(&s->r[type]);
+            u64 size = ALIGN(pci_region_sum(&s->r[type]), align);
             int is64 = pci_bios_bridge_region_is64(&s->r[type],
                                             s->bus_dev, type);
             // entry->bar is -1 if the entry represents a bridge region
@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
     return 0;
 }

-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
-
 // Setup region bases (given the regions' size and alignment)
 static void pci_bios_init_root_regions(struct pci_bus *bus)
 {
-    u64 start = BUILD_PCIMEM_START;
-    u64 end   = BUILD_PCIMEM_END;
-
     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

-    int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-    if (bus->r[reg1].align < bus->r[reg2].align) {
+    struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM];
+    struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM];
+
+    if (pci_region_align(r_start) < pci_region_align(r_end)) {
         // Swap regions to improve alignment.
-        reg1 = PCI_REGION_TYPE_MEM;
-        reg2 = PCI_REGION_TYPE_PREFMEM;
+        r_end = r_start;
+        r_start = &bus->r[PCI_REGION_TYPE_PREFMEM];
     }
-    bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
bus->r[reg2].align);
-    bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
-                                  , bus->r[reg1].align);
-    if (bus->r[reg1].base < start)
+    r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END -
pci_region_sum(r_end)),
+                                pci_region_align(r_end));
+    r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
+                                pci_region_align(r_start));
+
+    if ((r_start->base < BUILD_PCIMEM_START) ||
+         (r_start->base > BUILD_PCIMEM_END))
         // Memory range requested is larger than available.
         panic("PCI: out of address space\n");
 }

[PATCH 10/12] Calculate pci region stats on demand

[Alexey Korolev]

A flowed text. Please apply another [10/12] in this series.

On Tue, 2012-04-24 at 18:23 +1200, Alexey Korolev wrote:
> Do not store pci region stats - instead calulate the
> sum and alignment on demand.
> Signed-off-by: Alexey Korolev
>
> src/pciinit.c |   57
> +++++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/src/pciinit.c b/src/pciinit.c
> index 0d66dbe..a6cf98b 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -40,8 +40,6 @@ struct pci_region_entry {
>  };
>
> struct pci_region {
> /* pci region stats */
> u64 sum, align;
> /* pci region assignments */
> u64 base;
> struct pci_region_entry *list;
>
> @@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct
> pci_region *r,
>      return 1;
>  }
>
> +static u64 pci_region_align(struct pci_region *r)
> +{
> if (!r->list)
>    return 1;
> // The first entry in the sorted list has the largest alignment
> return r->list->align;
>
> +}
>
> +static u64 pci_region_sum(struct pci_region *r)
> +{
> struct pci_region_entry *entry = r->list;
> u64 sum = 0;
> while (entry) {
>    sum += entry->size;
>    entry = entry->next;
> }
> return sum;
>
> +}
>
> static struct pci_region_entry *
>  pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
>                          int bar, u64 size, u64 align, int type, int
> is64)
> @@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
> pci_device *dev,
>      }
>      entry->next = *pprev;
>      *pprev = entry;
> bus->r[type].sum += size;
> if (bus->r[type].align < align)
>    bus->r[type].align = align;
> return entry;
>
> }
>
> @@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus
> *busses)
>          for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>              u64 align = (type == PCI_REGION_TYPE_IO) ?
>                  PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>        if (s->r[type].align > align)
>            align = s->r[type].align;
>        u64 size = ALIGN(s->r[type].sum, align);
>        if (pci_region_align(&s->r[type]) > align)
>             align = pci_region_align(&s->r[type]);
>        u64 size = ALIGN(pci_region_sum(&s->r[type]), align);
>        int is64 = pci_bios_bridge_region_is64(&s->r[type],
>                                        s->bus_dev, type);
>        // entry->bar is -1 if the entry represents a bridge region
>
> @@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus
> *busses)
>      return 0;
>  }
>
> -#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
>
> // Setup region bases (given the regions' size and alignment)
>  static void pci_bios_init_root_regions(struct pci_bus *bus)
>  {
>
> u64 start = BUILD_PCIMEM_START;
>
> u64 end   = BUILD_PCIMEM_END;
>
> bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
>
> int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
>
> if (bus->r[reg1].align < bus->r[reg2].align) {
> struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM];
> struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM];
> if (pci_region_align(r_start) < pci_region_align(r_end)) {
>     // Swap regions to improve alignment.
>    reg1 = PCI_REGION_TYPE_MEM;
>    reg2 = PCI_REGION_TYPE_PREFMEM;
>    r_end = r_start;
>    r_start = &bus->r[PCI_REGION_TYPE_PREFMEM];
> }
> bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum,
>
> bus->r[reg2].align);
> bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
>                              , bus->r[reg1].align);
> if (bus->r[reg1].base < start)
> r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END -
>
> pci_region_sum(r_end)),
>                            pci_region_align(r_end));
> r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
>                            pci_region_align(r_start));
> if ((r_start->base < BUILD_PCIMEM_START) ||
>     (r_start->base > BUILD_PCIMEM_END))
>    // Memory range requested is larger than available.
>    panic("PCI: out of address space\n");
>
> }

[PATCH 10/12] Calculate pci region stats on demand

[Alexey Korolev]

Do not store pci region stats - instead calulate the
sum and alignment on demand.

Signed-off-by: Alexey Korolev

---
 src/pciinit.c |   57 +++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/src/pciinit.c b/src/pciinit.c
index 0d66dbe..a6cf98b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -40,8 +40,6 @@ struct pci_region_entry {
 };

struct pci_region {
-    /* pci region stats */
-    u64 sum, align;
     /* pci region assignments */
     u64 base;
     struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct pci_region *r,
     return 1;
 }

+static u64 pci_region_align(struct pci_region *r)
+{
+    if (!r->list)
+        return 1;
+    // The first entry in the sorted list has the largest alignment
+    return r->list->align;
+}
+
+static u64 pci_region_sum(struct pci_region *r)
+{
+    struct pci_region_entry *entry = r->list;
+    u64 sum = 0;
+    while (entry) {
+        sum += entry->size;
+        entry = entry->next;
+   }
+   return sum;
+}
+
 static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
                         int bar, u64 size, u64 align, int type, int is64)
@@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
     }
     entry->next = *pprev;
     *pprev = entry;
-
-    bus->r[type].sum += size;
-    if (bus->r[type].align < align)
-        bus->r[type].align = align;
     return entry;
 }

@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u64 align = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-            if (s->r[type].align > align)
-                align = s->r[type].align;
-            u64 size = ALIGN(s->r[type].sum, align);
+            if (pci_region_align(&s->r[type]) > align)
+                 align = pci_region_align(&s->r[type]);
+            u64 size = ALIGN(pci_region_sum(&s->r[type]), align);
             int is64 = pci_bios_bridge_region_is64(&s->r[type],
                                             s->bus_dev, type);
             // entry->bar is -1 if the entry represents a bridge region
@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus *busses)
     return 0;
 }

-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
-
 // Setup region bases (given the regions' size and alignment)
 static void pci_bios_init_root_regions(struct pci_bus *bus)
 {
-    u64 start = BUILD_PCIMEM_START;
-    u64 end   = BUILD_PCIMEM_END;
-
     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

-    int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-    if (bus->r[reg1].align < bus->r[reg2].align) {
+    struct pci_region *r_end = &bus->r[PCI_REGION_TYPE_PREFMEM];
+    struct pci_region *r_start = &bus->r[PCI_REGION_TYPE_MEM];
+
+    if (pci_region_align(r_start) < pci_region_align(r_end)) {
         // Swap regions to improve alignment.
-        reg1 = PCI_REGION_TYPE_MEM;
-        reg2 = PCI_REGION_TYPE_PREFMEM;
+        r_end = r_start;
+        r_start = &bus->r[PCI_REGION_TYPE_PREFMEM];
     }
-    bus->r[reg2].base = ROOT_BASE(end, bus->r[reg2].sum, bus->r[reg2].align);
-    bus->r[reg1].base = ROOT_BASE(bus->r[reg2].base, bus->r[reg1].sum
-                                  , bus->r[reg1].align);
-    if (bus->r[reg1].base < start)
+    r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)),
+                                pci_region_align(r_end));
+    r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
+                                pci_region_align(r_start));
+
+    if ((r_start->base < BUILD_PCIMEM_START) ||
+         (r_start->base > BUILD_PCIMEM_END))
         // Memory range requested is larger than available.
         panic("PCI: out of address space\n");
 }

[PATCH 10/12] Calculate pci region stats on demand

[Kevin O'Connor]

On Tue, Apr 24, 2012 at 06:24:27PM +1200, Alexey Korolev wrote:
> Do not store pci region stats - instead calulate the
> sum and alignment on demand.

[...]
> @@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>          for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>              u64 align = (type == PCI_REGION_TYPE_IO) ?
>                  PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
>        if (s->r[type].align > align)
>            align = s->r[type].align;
>        u64 size = ALIGN(s->r[type].sum, align);
>        if (pci_region_align(&s->r[type]) > align)
>             align = pci_region_align(&s->r[type]);
>        u64 size = ALIGN(pci_region_sum(&s->r[type]), align);

[...]
> r_end->base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)),
>                            pci_region_align(r_end));
> r_start->base = ALIGN_DOWN((r_end->base - pci_region_sum(r_start)),
>                            pci_region_align(r_start));

I'd avoid making function calls in the parameter of a macro (it can be
executed multiple times and it's non-obvious).

-Kevin

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Alexey Korolev]

Migrate 64bit entries to 64bit pci regions if they do
not fit in 32bit range.

Signed-off-by: Alexey Korolev 
---
 src/config.h  |    2 ++
 src/pciinit.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/src/config.h b/src/config.h
index b0187a4..bbacae7 100644
--- a/src/config.h
+++ b/src/config.h
@@ -47,6 +47,8 @@

#define BUILD_PCIMEM_START        0xe0000000
 #define BUILD_PCIMEM_END          0xfec00000    /* IOAPIC is mapped at */
+#define BUILD_PCIMEM64_START      0x8000000000ULL
+#define BUILD_PCIMEM64_END        0x10000000000ULL

#define BUILD_IOAPIC_ADDR         0xfec00000
 #define BUILD_HPET_ADDRESS        0xfed00000
diff --git a/src/pciinit.c b/src/pciinit.c
index a6cf98b..3d7640b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -386,6 +386,31 @@ static u64 pci_region_sum(struct pci_region *r)
    return sum;
 }

+static void pci_region_migrate_64bit_entries(struct pci_region *from,
+                                             struct pci_region *to)
+{
+    struct pci_region_entry **pprev = &from->list;
+    struct pci_region_entry **last = &to->list;
+    while(*pprev) {
+        if ((*pprev)->is64) {
+            struct pci_region_entry *entry;
+            entry = *pprev;
+            /* Delete the entry and move next */
+            *pprev = (*pprev)->next;
+            /* Add entry at tail to keep a sorted order */
+            entry->next = NULL;
+            if (*last) {
+               (*last)->next = entry;
+                last  = &(*last)->next;
+            }
+            else
+               (*last) = entry;
+        }
+        else
+            pprev = &(*pprev)->next;
+    }
+}
+
 static struct pci_region_entry *
 pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
                         int bar, u64 size, u64 align, int type, int is64)
@@ -478,7 +503,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
 }

// Setup region bases (given the regions' size and alignment)
-static void pci_bios_init_root_regions(struct pci_bus *bus)
+static int pci_bios_init_root_regions(struct pci_bus *bus)
 {
     bus->r[PCI_REGION_TYPE_IO].base = 0xc000;

@@ -498,7 +523,8 @@ static void pci_bios_init_root_regions(struct pci_bus *bus)
     if ((r_start->base < BUILD_PCIMEM_START) ||
          (r_start->base > BUILD_PCIMEM_END))
         // Memory range requested is larger than available.
-        panic("PCI: out of address space\n");
+        return -1;
+    return 0;
 }

@@ -561,6 +587,24 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)

static void pci_bios_map_devices(struct pci_bus *busses)
 {
+    if (pci_bios_init_root_regions(busses)) {
+        struct pci_region r64_mem, r64_pref;
+        r64_mem.list = NULL;
+        r64_pref.list = NULL;
+        pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
+                                         &r64_mem);
+        pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
+                                         &r64_pref);
+
+        if (pci_bios_init_root_regions(busses))
+            panic("PCI: out of address space\n");
+
+        r64_mem.base = BUILD_PCIMEM64_START;
+        r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),
+                              pci_region_align(&r64_pref));
+        pci_region_map_entries(busses, &r64_mem);
+        pci_region_map_entries(busses, &r64_pref);
+    }
     // Map regions on each device.
     int bus;
     for (bus = 0; bus<=MaxPCIBus; bus++) {
@@ -605,8 +649,6 @@ pci_setup(void)
     if (pci_bios_check_devices(busses))
         return;

-    pci_bios_init_root_regions(&busses[0]);
-
     dprintf(1, "=== PCI new allocation pass #2 ===\n");
     pci_bios_map_devices(busses);

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
> Migrate 64bit entries to 64bit pci regions if they do
> not fit in 32bit range.

[...]
> +static void pci_region_migrate_64bit_entries(struct pci_region *from,
>                                         struct pci_region *to)
>
> +{
> struct pci_region_entry **pprev = &from->list;
> struct pci_region_entry **last = &to->list;
> while(*pprev) {
>    if ((*pprev)->is64) {
>        struct pci_region_entry *entry;
>        entry = *pprev;
>        /* Delete the entry and move next */
>        *pprev = (*pprev)->next;
>        /* Add entry at tail to keep a sorted order */
>        entry->next = NULL;
>        if (*last) {
>           (*last)->next = entry;
>            last  = &(*last)->next;
>        }
>        else
>           (*last) = entry;
>    }
>    else
>        pprev = &(*pprev)->next;
> }
>
> +}

It should be possible to simplify this - something like (untested):

static void pci_region_migrate_64bit_entries(struct pci_region *from,
                                             struct pci_region *to)
{
    struct pci_region_entry **pprev = &from->list, **last = &to->list;
    for (; *pprev; pprev = &(*pprev)->next) {
        struct pci_region_entry *entry = *pprev;
        if (!entry->is64)
            continue;
        // Move from source list to dest list.
        *pprev = entry->next;
        entry->next = NULL;
        *last = entry;
        last = &entry->next;
    }
}

[...]
> static void pci_bios_map_devices(struct pci_bus *busses)
>  {
> if (pci_bios_init_root_regions(busses)) {
>    struct pci_region r64_mem, r64_pref;
>    r64_mem.list = NULL;
>    r64_pref.list = NULL;
>    pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
>                                     &r64_mem);
>    pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
>                                     &r64_pref);
>    if (pci_bios_init_root_regions(busses))
>        panic("PCI: out of address space\n");
>    r64_mem.base = BUILD_PCIMEM64_START;
>    r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),
>                          pci_region_align(&r64_pref));

There should be a check to see if the regions fit.  Maybe pass
start/end into pci_bios_init_root_regions() and call it again for the
> 4g region?
>    pci_region_map_entries(busses, &r64_mem);
>    pci_region_map_entries(busses, &r64_pref);
> }
> // Map regions on each device.

This doesn't look right to me.  This will map the devices on bus 0 to
the proper >4g address, but devices on any subsequent bus will use
busses[0].r[].base which will be reset to the <4gig address.  Perhaps
pull base out of pci_region and make pci_region_map_entries()
recursive?

-Kevin

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Alexey Korolev]

On 25/04/12 13:48, Kevin O'Connor wrote:
> On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
> > Migrate 64bit entries to 64bit pci regions if they do
> > not fit in 32bit range.
>
> [...]
> > +static void pci_region_migrate_64bit_entries(struct pci_region *from,
> >                                         struct pci_region *to)
> >
> > +{
> > struct pci_region_entry **pprev = &from->list;
> > struct pci_region_entry **last = &to->list;
> > while(*pprev) {
> >    if ((*pprev)->is64) {
> >        struct pci_region_entry *entry;
> >        entry = *pprev;
> >        /* Delete the entry and move next */
> >        *pprev = (*pprev)->next;
> >        /* Add entry at tail to keep a sorted order */
> >        entry->next = NULL;
> >        if (*last) {
> >           (*last)->next = entry;
> >            last  = &(*last)->next;
> >        }
> >        else
> >           (*last) = entry;
> >    }
> >    else
> >        pprev = &(*pprev)->next;
> > }
> >
> > +}
>
> It should be possible to simplify this - something like (untested):
>
> static void pci_region_migrate_64bit_entries(struct pci_region *from,
>                                              struct pci_region *to)
> {
>     struct pci_region_entry **pprev = &from->list, **last = &to->list;
>     for (; *pprev; pprev = &(*pprev)->next) {
>         struct pci_region_entry *entry = *pprev;
>         if (!entry->is64)
>             continue;
>         // Move from source list to dest list.
>         *pprev = entry->next;
>         entry->next = NULL;
>         *last = entry;
>         last = &entry->next;
>     }
> }

Sorry it's not working.
I agree it's possible to simplify code a bit.

static void pci_region_migrate_64bit_entries(struct pci_region *from,
                                             struct pci_region *to)
{
    struct pci_region_entry **pprev = &from->list;
    struct pci_region_entry **last = &to->list;
    while(*pprev) {
        if ((*pprev)->is64) {
            struct pci_region_entry *entry;
            entry = *pprev;
            /* Delete the entry and move next */
            *pprev = (*pprev)->next;
            /* Add entry at tail to keep the order */
            entry->next = NULL;
            *last = entry;
            last = &entry->next;
        }
        else
            pprev = &(*pprev)->next;
    }
}

That should work.
> [...]
> > static void pci_bios_map_devices(struct pci_bus *busses)
> >  {
> > if (pci_bios_init_root_regions(busses)) {
> >    struct pci_region r64_mem, r64_pref;
> >    r64_mem.list = NULL;
> >    r64_pref.list = NULL;
> >    pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
> >                                     &r64_mem);
> >    pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
> >                                     &r64_pref);
> >    if (pci_bios_init_root_regions(busses))
> >        panic("PCI: out of address space\n");
> >    r64_mem.base = BUILD_PCIMEM64_START;
> >    r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),
> >                          pci_region_align(&r64_pref));
>
> There should be a check to see if the regions fit.  Maybe pass
> start/end into pci_bios_init_root_regions() and call it again for the
> > 4g region?

Agree, I just ignored the check as 64bit range size is 2^39.
I will think how to make it better.
> >    pci_region_map_entries(busses, &r64_mem);
> >    pci_region_map_entries(busses, &r64_pref);
> > }
> > // Map regions on each device.
>
> This doesn't look right to me.  This will map the devices on bus 0 to
> the proper >4g address, but devices on any subsequent bus will use
> busses[0].r[].base which will be reset to the <4gig address.  Perhaps
> pull base out of pci_region and make pci_region_map_entries()
> recursive?

No recursion is need here!
We map all entries which are 64bit on root bus.
If entry is a bridge region - a corresponding bus address will be updated.
Region won't be reseted to <4gig address as address is derived from parent region only.

Thanks,
Alexey

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Wed, Apr 25, 2012 at 06:25:07PM +1200, Alexey Korolev wrote:
> On 25/04/12 13:48, Kevin O'Connor wrote:
> > On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
> > >    pci_region_map_entries(busses, &r64_mem);
> > >    pci_region_map_entries(busses, &r64_pref);
> > > }
> > > // Map regions on each device.
> >
> > This doesn't look right to me.  This will map the devices on bus 0 to
> > the proper >4g address, but devices on any subsequent bus will use
> > busses[0].r[].base which will be reset to the <4gig address.  Perhaps
> > pull base out of pci_region and make pci_region_map_entries()
> > recursive?
>
> No recursion is need here!
> We map all entries which are 64bit on root bus.
> If entry is a bridge region - a corresponding bus address will be updated.
> Region won't be reseted to <4gig address as address is derived from parent region only.

Okay - I missed that.  I think the patches look okay to be committed -
any additional changes can be made on top.  Gerd - do you have any
comments?

-Kevin

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Gerd Hoffmann]

On 04/25/12 14:51, Kevin O'Connor wrote:
> On Wed, Apr 25, 2012 at 06:25:07PM +1200, Alexey Korolev wrote:
> > On 25/04/12 13:48, Kevin O'Connor wrote:
> > > On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
> > > >    pci_region_map_entries(busses, &r64_mem);
> > > >    pci_region_map_entries(busses, &r64_pref);
> > > > }
> > > > // Map regions on each device.
> > >
> > > This doesn't look right to me.  This will map the devices on bus 0 to
> > > the proper >4g address, but devices on any subsequent bus will use
> > > busses[0].r[].base which will be reset to the <4gig address.  Perhaps
> > > pull base out of pci_region and make pci_region_map_entries()
> > > recursive?
> >
> > No recursion is need here!
> > We map all entries which are 64bit on root bus.
> > If entry is a bridge region - a corresponding bus address will be updated.
> > Region won't be reseted to <4gig address as address is derived from parent region only.
>
> Okay - I missed that.  I think the patches look okay to be committed -
> any additional changes can be made on top.  Gerd - do you have any
> comments?

Great job overall.  Can't spot issues in the code.  And the patches do
fine in testing too.  Some minor issues poped up:

Issue #1:  seabios can't boot from a virtio-scsi disk if the controller
is behind a pci bridge.  I think the reason simply is that (according to
the seabios log) only root bus pci devices are initialized.  Probably
even isn't related to this patch set, just trapped into this while
testing the bridge mapping code of the patch series.

Issue #2: root bus (non-pref) memory regions are mapped above 4G if they
are 64bit capable.  That happened to include the xhci usb controller.  I
don't think we want that.  Some day seabios will get xhci support, and
having the bar above 4G makes it unreachable in 32bit mode.  So this
needs some refinement.  Options I can think of:

(1) Don't bother mapping non-prefmem bars above 4G.
  (2) Only map them above 4G if they are larger than a certain limit.
  (3) Allow devices to be excluded on certain conditions, for example
      when seabios has a driver, when they have an option rom, when
      they have a specific pci id, ...

These are certainly no blockers and can be discussed and solved after
committing this series.

cheers,
  Gerd

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Alexey Korolev]

Hi
Thank you for review!
On 26/04/12 03:29, Gerd Hoffmann wrote:
> > Okay - I missed that.  I think the patches look okay to be committed -
> > any additional changes can be made on top.  Gerd - do you have any
> > comments?
>
> Great job overall.  Can't spot issues in the code.  And the patches do
> fine in testing too.  Some minor issues poped up:
>
> Issue #1:  seabios can't boot from a virtio-scsi disk if the controller
> is behind a pci bridge.  I think the reason simply is that (according to
> the seabios log) only root bus pci devices are initialized.  Probably
> even isn't related to this patch set, just trapped into this while
> testing the bridge mapping code of the patch series.

I observed similar but ignored since "bridge test" requires special qemu build.
It's a general bridge support issue. This series don't address this issue.
This issue occurs regardless if the patches applied or not.
> Issue #2: root bus (non-pref) memory regions are mapped above 4G if they
> are 64bit capable.  That happened to include the xhci usb controller.  I
> don't think we want that.  Some day seabios will get xhci support, and
> having the bar above 4G makes it unreachable in 32bit mode.  So this
> needs some refinement.  Options I can think of:
>
> (1) Don't bother mapping non-prefmem bars above 4G.
>   (2) Only map them above 4G if they are larger than a certain limit.
>   (3) Allow devices to be excluded on certain conditions, for example
>       when seabios has a driver, when they have an option rom, when
>       they have a specific pci id, ...

I like (3) more than the (1) and (2).
> These are certainly no blockers and can be discussed and solved after
> committing this series.

I've just fixed what you have suggested to fix.
Just very minor changes in patches 10/12 and 11/12:
- Simplify a bit list operation.
- Remove function calls away of macros
- Add condition to check crossing the end of 64bit PCI range

Thanks,
Alexey

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Gerd Hoffmann]

Hi,
> > Issue #1:  seabios can't boot from a virtio-scsi disk if the controller
> > is behind a pci bridge.  I think the reason simply is that (according to
> > the seabios log) only root bus pci devices are initialized.  Probably
> > even isn't related to this patch set, just trapped into this while
> > testing the bridge mapping code of the patch series.
>
> I observed similar but ignored since "bridge test" requires special qemu build.
> It's a general bridge support issue. This series don't address this issue.
> This issue occurs regardless if the patches applied or not.

FYI: Meanwhile the bridge patches have been merged, latest qemu master
branch will do.

cheers,
  Gerd

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Thu, Apr 26, 2012 at 07:12:22PM +1200, Alexey Korolev wrote:
> On 26/04/12 03:29, Gerd Hoffmann wrote:
> > > Okay - I missed that.  I think the patches look okay to be committed -
> > > any additional changes can be made on top.  Gerd - do you have any
> > > comments?
> >
> > These are certainly no blockers and can be discussed and solved after
> > committing this series.
>
> I've just fixed what you have suggested to fix.
> Just very minor changes in patches 10/12 and 11/12:
> Simplify a bit list operation.
> Remove function calls away of macros
> Add condition to check crossing the end of 64bit PCI range

Thanks.  I committed this series.

-Kevin

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
> Issue #1:  seabios can't boot from a virtio-scsi disk if the controller
> is behind a pci bridge.  I think the reason simply is that (according to
> the seabios log) only root bus pci devices are initialized.  Probably
> even isn't related to this patch set, just trapped into this while
> testing the bridge mapping code of the patch series.

Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
0) break" from pci_bios_init_devices()?

The code should probably handle the irq swizzling that pci bridges do
though.
> Issue #2: root bus (non-pref) memory regions are mapped above 4G if they
> are 64bit capable.  That happened to include the xhci usb controller.  I
> don't think we want that.  Some day seabios will get xhci support, and
> having the bar above 4G makes it unreachable in 32bit mode.  So this
> needs some refinement.  Options I can think of:
>
> (1) Don't bother mapping non-prefmem bars above 4G.
>   (2) Only map them above 4G if they are larger than a certain limit.
>   (3) Allow devices to be excluded on certain conditions, for example
>       when seabios has a driver, when they have an option rom, when
>       they have a specific pci id, ...

I'd vote for 2.  There's nearly 500MB of space for PCI devices under
4G - small regions (say under 1MB) are unlikely to be the cause of an
allocation failure (1MB would be .2% of total space).

-Kevin

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Alexey Korolev]

On 27/04/12 00:45, Kevin O'Connor wrote:
> On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
> > Issue #1:  seabios can't boot from a virtio-scsi disk if the controller
> > is behind a pci bridge.  I think the reason simply is that (according to
> > the seabios log) only root bus pci devices are initialized.  Probably
> > even isn't related to this patch set, just trapped into this while
> > testing the bridge mapping code of the patch series.
>
> Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> 0) break" from pci_bios_init_devices()?

This helps in my tests. The devices are no longer disabled. Not sure
about virtio-scsi test though.
> The code should probably handle the irq swizzling that pci bridges do
> though.

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Gerd Hoffmann]

Hi,
> Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> 0) break" from pci_bios_init_devices()?

Seems to do the trick, at least the disks connected appear in the boot
menu now and the seabios log file looks sane.

The guest kernel has no virtio-scsi drivers though, need to update it
for more testing.
> The code should probably handle the irq swizzling that pci bridges do
> though.

i.e. add bridge handling to pci_slot_get_irq() ?

cheers,
  Gerd

[PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Wed, May 02, 2012 at 03:42:51PM +0200, Gerd Hoffmann wrote:
> Hi,
> > Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> > 0) break" from pci_bios_init_devices()?
>
> Seems to do the trick, at least the disks connected appear in the boot
> menu now and the seabios log file looks sane.
>
> The guest kernel has no virtio-scsi drivers though, need to update it
> for more testing.
> > The code should probably handle the irq swizzling that pci bridges do
> > though.
>
> i.e. add bridge handling to pci_slot_get_irq() ?

Yes.

-Kevin

[PATCH 12/12] Fix 64bit PCI issues on Windows

[Alexey Korolev]

This patch solves issues on Windows guests, when 64bit
BAR's are present. It is also helpful on Linux guests
when use_crs kernel boot option is set.

Signed-off-by: Alexey Korolev 
Signed-off-by: Michael S. Tsirkin 
---
 src/acpi-dsdt.dsl |    7 +++++
 src/acpi-dsdt.hex |   64 +++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 4bdc268..4a18617 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -175,6 +175,13 @@ DefinitionBlock (
                     0x00000000,         // Address Translation Offset
                     0x1EC00000,         // Address Length
                     ,, , AddressRangeMemory, TypeStatic)
+                QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
+                    0x00000000,          // Address Space Granularity
+                    0x8000000000,        // Address Range Minimum
+                    0xFFFFFFFFFF,        // Address Range Maximum
+                    0x00000000,          // Address Translation Offset
+                    0x8000000000,        // Address Length
+                    ,, , AddressRangeMemory, TypeStatic)
             })
         }
     }
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index a4af597..07f0e18 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
 0x53,
 0x44,
 0x54,
-0x21,
+0x4f,
 0x11,
 0x0,
 0x0,
 0x1,
-0xe8,
+0xca,
 0x42,
 0x58,
 0x50,
@@ -110,16 +110,16 @@ static unsigned char AmlCode[] = {
 0x47,
 0x42,
 0x10,
-0x44,
-0x81,
+0x42,
+0x84,
 0x5f,
 0x53,
 0x42,
 0x5f,
 0x5b,
 0x82,
-0x4c,
-0x80,
+0x4a,
+0x83,
 0x50,
 0x43,
 0x49,
@@ -2064,10 +2064,10 @@ static unsigned char AmlCode[] = {
 0x52,
 0x53,
 0x11,
-0x42,
-0x7,
+0x40,
 0xa,
-0x6e,
+0xa,
+0x9c,
 0x88,
 0xd,
 0x0,
@@ -2176,6 +2176,52 @@ static unsigned char AmlCode[] = {
 0x0,
 0xc0,
 0x1e,
+0x8a,
+0x2b,
+0x0,
+0x0,
+0xc,
+0x3,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
+0xff,
+0xff,
+0xff,
+0xff,
+0xff,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
 0x79,
 0x0,
 0x10,

[PATCH 12/12] Fix 64bit PCI issues on Windows

[Kevin O'Connor]

On Tue, Apr 24, 2012 at 06:26:30PM +1200, Alexey Korolev wrote:
> This patch solves issues on Windows guests, when 64bit
> BAR's are present. It is also helpful on Linux guests
> when use_crs kernel boot option is set.

Unfortunately, this patch causes problems for WinXP, so I reverted it.

-Kevin