Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
[...] [Patch 5] Track-alignment-explicitly Almost the same as the previous, just changed priority from r-align to r-sum when setting start address of root regions. I guess there are more chances to fit memory regions if we try place regions with higher r-sum like it was before. Consider default config #define BUILD_PCIMEM_START 0xe000 #define BUILD_PCIMEM_END 0xfec0 Image we have 1 pref. mem. region of 128MB. And many small memory regions which take rest of available 492MB - 128MB If we have alignment priority. PCI pref memory region will start from F000 and PCI memoryregion will start from 0xe000 and do not fit. The section with the lowest alignment would get allocated first and be at a higher address. So, in your example, the regular memory region would be assigned 0xe800 and then pref mem would be assigned 0xe000. Your example is why alignment should be used instead of size. If size is used, then what you cite above occurs (pref mem has the lower size and is thus allocated first at the higher address). Ahh the lowest alignment! In other words if we go from PCIMEM_START to PCIMEM_END we first assign address range for the region with the largest PCI resource size (highest alignment). I see. I will put it back then. On patch 10, it would be preferable to separate the dynamic calculation of sum/alignment changes from the 64bit support. Otherwise, the core algorithm of patch 10 looks okay. Though it seems like the code is recalculating sum/alignment more than needed, and I think the list manipulation in pci_region_migrate_64bit_entries could be streamlined. Right, sum/alignment recalculation is important for migration. Will split it and submit new series. Thanks, Alexey
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Thu, Apr 19, 2012 at 07:00:41PM +1200, Alexey Korolev wrote: Here is the whole series of patches including 64bit support. Thanks. [...] [Patch 5] Track-alignment-explicitly Almost the same as the previous, just changed priority from r-align to r-sum when setting start address of root regions. I guess there are more chances to fit memory regions if we try place regions with higher r-sum like it was before. Consider default config #define BUILD_PCIMEM_START 0xe000 #define BUILD_PCIMEM_END 0xfec0 Image we have 1 pref. mem. region of 128MB. And many small memory regions which take rest of available 492MB - 128MB If we have alignment priority. PCI pref memory region will start from F000 and PCI memoryregion will start from 0xe000 and do not fit. The section with the lowest alignment would get allocated first and be at a higher address. So, in your example, the regular memory region would be assigned 0xe800 and then pref mem would be assigned 0xe000. Your example is why alignment should be used instead of size. If size is used, then what you cite above occurs (pref mem has the lower size and is thus allocated first at the higher address). [Patch 10] New: Migrate 64bit entries to 64bit pci regions if they do not fit in 32bit range. Pci region stats are now calculated. Added protection when total size of PCI resources is over 4GB. Patches 1-9 and 11 look okay to me. On patch 10, it would be preferable to separate the dynamic calculation of sum/alignment changes from the 64bit support. Otherwise, the core algorithm of patch 10 looks okay. Though it seems like the code is recalculating sum/alignment more than needed, and I think the list manipulation in pci_region_migrate_64bit_entries could be streamlined. Thanks again, -Kevin
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Thu, Apr 12, 2012 at 03:28:02PM +1200, Alexey Korolev wrote: On 12/04/12 15:15, Kevin O'Connor wrote: This was also me playing with one of Gerd's patches. It just makes the bar read/write code 64bit aware. It doesn't actually program them. The logic to do real 64bit allocations would require list merging. Is this something you have looked at? Right. I see what you mean here. Shall I play around with 64bit support on top of these patches? If that makes sense, then sure. -Kevin
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Thu, Apr 12, 2012 at 02:44:54PM +1200, Alexey Korolev wrote: On 04/04/12 15:31, Kevin O'Connor wrote: Agreed - the only thing it does is force a minimum size for memory bars as you pointed out in a previous email. As above, I did play with this a little more on Sunday. I also added in two patches from Gerd's series and made alignment handling more explicit. I'm including the series here if you're interested. Again, I think this should wait until after the 1.7.0 release. -Kevin Hi Kevin, Sorry for late response, was quite busy. I've reviewed and tried your patches. [Patches 1-4] are almost the same as I proposed in this series. Yes. It's your patches tweaked slightly. The noticeable differences are: 1) instead of sorting entries at mapping stage your patches sort entries at allocation stage. (no difference in behavior or readability so any approach is fine for me) 2) instead of storing pointer to bus resource which the bridge device represents (this_bus), it is obtained from pci structure and array of busses. - entry-this_bus-r[entry-type].base = entry-base; +struct pci_bus *child_bus = busses[entry-dev-secondary_bus]; +child_bus-r[entry-type].base = addr; Since entry-this_bus member is removed the information about whether the resource is bridge region or PCI BAR is encoded inside entry-bar. Value -1 - means this is a bridge region, the positive values mean it is a BAR. IMHO 2) makes code less readable, at least a comment explaining the meaning of negative value of PCI BAR is required. I've found just cosmetic difference to my patches so please don't forget to add my sign-off-by to them. Okay - will do. [Patch 5] Track region alignment explicitly. Looks very good and safe. Should reduce address range wastes. [Patch 6] Small patch to account resources of PCI bridges. Looks fine. May be instead of +num_regions = 2; I would add #define PCI_BRIDGE_NUM_REGIONS 2 . +num_regions = PCI_BRIDGE_NUM_REGIONS; This was me playing with some of Gerd's patches. I think it would require additional changes before committing. In particular, the patch misses the ROM bar on bridges - I think maybe it should be changed to keep the same loop constraints but add a if (bar PCI_BRIDGE_NUM_REGIONS bar PCI_ROM_SLOT) continue;. [Patch 7] 64bit aware code. Patch is incomplete. It is not working. This was also me playing with one of Gerd's patches. It just makes the bar read/write code 64bit aware. It doesn't actually program them. The logic to do real 64bit allocations would require list merging. Is this something you have looked at? -Kevin
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On 04/04/12 15:31, Kevin O'Connor wrote: Agreed - the only thing it does is force a minimum size for memory bars as you pointed out in a previous email. As above, I did play with this a little more on Sunday. I also added in two patches from Gerd's series and made alignment handling more explicit. I'm including the series here if you're interested. Again, I think this should wait until after the 1.7.0 release. -Kevin Hi Kevin, Sorry for late response, was quite busy. I've reviewed and tried your patches. [Patches 1-4] are almost the same as I proposed in this series. The noticeable differences are: 1) instead of sorting entries at mapping stage your patches sort entries at allocation stage. (no difference in behavior or readability so any approach is fine for me) 2) instead of storing pointer to bus resource which the bridge device represents (this_bus), it is obtained from pci structure and array of busses. - entry-this_bus-r[entry-type].base = entry-base; +struct pci_bus *child_bus = busses[entry-dev-secondary_bus]; +child_bus-r[entry-type].base = addr; Since entry-this_bus member is removed the information about whether the resource is bridge region or PCI BAR is encoded inside entry-bar. Value -1 - means this is a bridge region, the positive values mean it is a BAR. IMHO 2) makes code less readable, at least a comment explaining the meaning of negative value of PCI BAR is required. I've found just cosmetic difference to my patches so please don't forget to add my sign-off-by to them. [Patch 5] Track region alignment explicitly. Looks very good and safe. Should reduce address range wastes. [Patch 6] Small patch to account resources of PCI bridges. Looks fine. May be instead of +num_regions = 2; I would add #define PCI_BRIDGE_NUM_REGIONS 2 . +num_regions = PCI_BRIDGE_NUM_REGIONS; [Patch 7] 64bit aware code. Patch is incomplete. It is not working. After 1.7.0 is fine.
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On 12/04/12 15:15, Kevin O'Connor wrote: This was also me playing with one of Gerd's patches. It just makes the bar read/write code 64bit aware. It doesn't actually program them. The logic to do real 64bit allocations would require list merging. Is this something you have looked at? Right. I see what you mean here. Shall I play around with 64bit support on top of these patches?
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
Hi Kevin,
Thank you for the patches!
I've created a diff of final version of your changes over mine, to make it
clear what has changed.
Rather than including the complete diff, I've just left relevant parts and
added comments.
--- a/src/pciinit.c +++ b/src/pciinit.c@@ -12,8 +12,9 @@
@@ -34,25 +35,44 @@
struct pci_region_entry {
struct pci_device *dev;
int bar;
-u32 base;
u32 size;
-int is64bit;
+int is64;
enum pci_region_type type;
-struct pci_bus *this_bus;
+struct pci_bus *child_bus;
Structure members naming was one of difficult things when I was writing the
code.
The child_bus might be a bit confusing as people may thing that it describes a
child bus in the bus topology,in fact this element describes the bus this
pci_region_entry
is representing.
...
+
+static int pci_size_to_index(u32 size, enum pci_region_type type)
+{
+int index = __fls(size);
+int shift = (type == PCI_REGION_TYPE_IO) ?
+PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
+
+if (index shift)
+index = shift;
+index -= shift;
+return index;
+}
+
+static u32 pci_index_to_size(int index, enum pci_region_type type)
+{
+int shift = (type == PCI_REGION_TYPE_IO) ?
+PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
+
+return 0x1 (index + shift);
+}
The only purpose to have these functions is to define the minimum size of pci
BAR.
They are used only once.
What if we add size adjustment to pci_region_create_entry, or just create a
function like
pci_adjust_size(u32 size, enum pci_region_type type, int bridge)?
.
-list_add_head(bus-r[type].list, entry);
entry-parent_bus = bus;
-
+// Insert into list in sorted order.
+struct pci_region_entry **pprev;
+for (pprev = bus-r[type].list; *pprev; pprev = (*pprev)-next) {
+struct pci_region_entry *pos = *pprev;
+if (pos-size size)
+break;
+}
+entry-next = *pprev;
+*pprev = entry;
+
..
static void pci_bios_map_devices(struct pci_bus *busses)
{
-struct pci_region_entry *entry, *next;
-
+// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-u32 size;
-for (size = busses[bus].r[type].max; size 0; size = 1) {
-list_foreach_entry_safe(busses[bus].r[type].list,
- next, entry) {
-if (size == entry-size) {
-entry-base = busses[bus].r[type].base;
-busses[bus].r[type].base += size;
-pci_region_map_one_entry(entry);
-list_del(entry);
-free(entry);
-}
-}
+struct pci_region_entry *entry = busses[bus].r[type].list;
+while (entry) {
+pci_region_map_one_entry(entry);
+struct pci_region_entry *next = entry-next;
+free(entry);
+entry = next;
}
}
}
Right, instead of sorting entries by size in pci_bios_map_devices, the entries
are sorted when they are created.
This makes the implementation simpler.
Note: In case of 64bit BARs we have to migrate entries, so just sorting on
create won't be enough,
we should also add sorting when entries are migrated. This will add some more
complexity, while in case of old
implementation complexity will remain the same. I expect it shouldn't be that
complicated, so any of these approaches
are fine for me.
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Tue, Apr 03, 2012 at 06:39:22PM +1200, Alexey Korolev wrote:
Hi Kevin,
Thank you for the patches!
I've created a diff of final version of your changes over mine, to make it
clear what has changed.
Rather than including the complete diff, I've just left relevant parts and
added comments.
Yes - there isn't much difference between your patches and my patches.
I was really just playing with your patch.
Structure members naming was one of difficult things when I was writing the
code.
The child_bus might be a bit confusing as people may thing that it describes a
child bus in the bus topology,in fact this element describes the bus this
pci_region_entry
is representing.
On Sunday, it occurred to me that we really don't need either
parent_bus or this_bus.
+static int pci_size_to_index(u32 size, enum pci_region_type type)
[...]
The only purpose to have these functions is to define the minimum size of pci
BAR.
They are used only once.
What if we add size adjustment to pci_region_create_entry, or just create a
function like
pci_adjust_size(u32 size, enum pci_region_type type, int bridge)?
Agreed - the only thing it does is force a minimum size for memory
bars as you pointed out in a previous email.
As above, I did play with this a little more on Sunday. I also added
in two patches from Gerd's series and made alignment handling more
explicit. I'm including the series here if you're interested. Again,
I think this should wait until after the 1.7.0 release.
-Kevin
From 1ce91ff107ae43cc7fee5f87809f92078b0394ff Mon Sep 17 00:00:00 2001
Message-Id: cover.1333510239.git.ke...@koconnor.net
From: Kevin O'Connor ke...@koconnor.net
Date: Tue, 3 Apr 2012 23:30:39 -0400
Subject: [PATCH 0/7] *** SUBJECT HERE ***
To: seab...@seabios.org
*** BLURB HERE ***
Alexey Korolev (1):
pciinit: Get rid of size element of pci_bus-r structure
Kevin O'Connor (6):
pciinit: Add a pci_region_entry structure.
pciinit: Perform bus bar assignment at same time as normal bar
assignment.
pciinit: Use sorted order allocation scheme instead of array based
count scheme.
pciinit: Track region alignment explicitly.
pciinit: bridges can have two regions too
pciinit: 64bit support.
src/pci.h |5 -
src/pciinit.c | 351 ++---
2 files changed, 160 insertions(+), 196 deletions(-)
--
1.7.6.5
From 5c88fe3a0a6bfb35abccee88904de11e2ce1896b Mon Sep 17 00:00:00 2001
Message-Id:
5c88fe3a0a6bfb35abccee88904de11e2ce1896b.1333510239.git.ke...@koconnor.net
In-Reply-To: cover.1333510239.git.ke...@koconnor.net
References: cover.1333510239.git.ke...@koconnor.net
From: Kevin O'Connor ke...@koconnor.net
Date: Sun, 1 Apr 2012 11:23:06 -0400
Subject: [PATCH 1/7] pciinit: Add a pci_region_entry structure.
To: seab...@seabios.org
In this patch the pci_region_entry structure is introduced.
The pci_device-bars are removed. The information from
pci_region_entry is used to program pci bars.
Original patch by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pci.h |5 --
src/pciinit.c | 115 +++-
2 files changed, 80 insertions(+), 40 deletions(-)
diff --git a/src/pci.h b/src/pci.h
index a2a5a4c..5598100 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -51,11 +51,6 @@ struct pci_device {
u8 prog_if, revision;
u8 header_type;
u8 secondary_bus;
-struct {
-u32 addr;
-u32 size;
-int is64;
-} bars[PCI_NUM_REGIONS];
// Local information on device.
int have_driver;
diff --git a/src/pciinit.c b/src/pciinit.c
index 9f3fdd4..4af4105 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -31,6 +31,15 @@ static const char *region_type_name[] = {
[ PCI_REGION_TYPE_PREFMEM ] = prefmem,
};
+struct pci_region_entry {
+struct pci_device *dev;
+int bar;
+u32 size;
+int is64;
+enum pci_region_type type;
+struct pci_region_entry *next;
+};
+
struct pci_bus {
struct {
/* pci region stats */
@@ -41,6 +50,7 @@ struct pci_bus {
/* pci region assignments */
u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
+struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
@@ -352,19 +362,41 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32
*val, u32 *size)
*size = (~(*val mask)) + 1;
}
-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size)
+static struct pci_region_entry *
+pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
+int bar, u32 size, int type, int is64)
{
-u32 index;
+struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
+if (!entry) {
+warn_noalloc();
+return NULL;
+}
+memset(entry, 0, sizeof(*entry));
+entry-dev = dev;
+entry-bar = bar;
+entry-size = size;
+
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Wed, Mar 28, 2012 at 05:54:10PM +1300, Alexey Korolev wrote:
In this patch instead of array based resource allocation approach
we calculate resource addresses linked lists of pci_region_entry structures.
Thanks. I still think this migration can be done more seamlessly. I
played with your patches a bit and came up with the attached patches
that do just code movement - no alogorithm changes. See the attached.
Also, I think we should look to commit after the next SeaBIOS release.
-Kevin
From 2f6e81d884dfbb01a12ddfb10a64bf87e864a19c Mon Sep 17 00:00:00 2001
From: Alexey Korolev alexey.koro...@endace.com
Date: Wed, 28 Mar 2012 17:41:41 +1300
Subject: [PATCH 1/3] Added a pci_region_entry structure
To: seab...@seabios.org
In this patch the pci_region_entry structure is introduced.
The pci_device-bars are removed. The information from
pci_region_entry is used to program pci bars.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pci.h |5 --
src/pciinit.c | 115 +++--
2 files changed, 79 insertions(+), 41 deletions(-)
diff --git a/src/pci.h b/src/pci.h
index a2a5a4c..5598100 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -51,11 +51,6 @@ struct pci_device {
u8 prog_if, revision;
u8 header_type;
u8 secondary_bus;
-struct {
-u32 addr;
-u32 size;
-int is64;
-} bars[PCI_NUM_REGIONS];
// Local information on device.
int have_driver;
diff --git a/src/pciinit.c b/src/pciinit.c
index 9f3fdd4..2831895 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -31,6 +31,19 @@ static const char *region_type_name[] = {
[ PCI_REGION_TYPE_PREFMEM ] = prefmem,
};
+struct pci_bus;
+struct pci_region_entry {
+struct pci_device *dev;
+int bar;
+u32 size;
+int is64;
+enum pci_region_type type;
+struct pci_bus *child_bus;
+struct pci_bus *parent_bus;
+struct pci_region_entry *next;
+struct pci_region_entry **pprev;
+};
+
struct pci_bus {
struct {
/* pci region stats */
@@ -41,6 +54,7 @@ struct pci_bus {
/* pci region assignments */
u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
+struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
@@ -352,19 +366,33 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32
*val, u32 *size)
*size = (~(*val mask)) + 1;
}
-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size)
+static struct pci_region_entry *
+pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
+u32 size, int type, int is64)
{
-u32 index;
-
-index = pci_size_to_index(size, type);
+struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
+if (!entry) {
+warn_noalloc();
+return NULL;
+}
+memset(entry, 0, sizeof(*entry));
+entry-dev = dev;
+entry-size = size;
+entry-is64 = is64;
+entry-type = type;
+entry-parent_bus = bus;
+list_add_head(bus-r[type].list, entry);
+
+u32 index = pci_size_to_index(size, type);
size = pci_index_to_size(index, type);
bus-r[type].count[index]++;
bus-r[type].sum += size;
if (bus-r[type].max size)
bus-r[type].max = size;
+return entry;
}
-static void pci_bios_check_devices(struct pci_bus *busses)
+static int pci_bios_check_devices(struct pci_bus *busses)
{
dprintf(1, PCI: check devices\n);
@@ -383,14 +411,16 @@ static void pci_bios_check_devices(struct pci_bus *busses)
if (val == 0)
continue;
-pci_bios_bus_reserve(bus, pci_addr_to_type(val), size);
-pci-bars[i].addr = val;
-pci-bars[i].size = size;
-pci-bars[i].is64 = (!(val PCI_BASE_ADDRESS_SPACE_IO)
- (val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
- == PCI_BASE_ADDRESS_MEM_TYPE_64);
+int is64 = (!(val PCI_BASE_ADDRESS_SPACE_IO)
+(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+== PCI_BASE_ADDRESS_MEM_TYPE_64);
+struct pci_region_entry *entry = pci_region_create_entry(
+bus, pci, size, pci_addr_to_type(val), is64);
+if (!entry)
+return -1;
+entry-bar = i;
-if (pci-bars[i].is64)
+if (is64)
i++;
}
}
@@ -410,7 +440,11 @@ static void pci_bios_check_devices(struct pci_bus *busses)
if (s-r[type].size limit)
s-r[type].size = limit;
s-r[type].size = pci_size_roundup(s-r[type].size);
-pci_bios_bus_reserve(parent, type, s-r[type].size);
+struct pci_region_entry *entry = pci_region_create_entry(
+parent, s-bus_dev, s-r[type].size, type, 0);
+if (!entry)
+
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
On Sun, Apr 01, 2012 at 03:28:34AM -0400, Kevin O'Connor wrote:
On Wed, Mar 28, 2012 at 05:54:10PM +1300, Alexey Korolev wrote:
In this patch instead of array based resource allocation approach
we calculate resource addresses linked lists of pci_region_entry structures.
Thanks. I still think this migration can be done more seamlessly. I
played with your patches a bit and came up with the attached patches
that do just code movement - no alogorithm changes. See the attached.
Also, I think we should look to commit after the next SeaBIOS release.
Looking closer at your new allocation algorithm, I see that it
effectively allocates largest sizes first. It occurred to me that an
easy way to do this is to keep the pci_region_entry list sorted by
size. See the patch below as an example (based on top of the previous
email I sent).
-Kevin
diff --git a/src/pciinit.c b/src/pciinit.c
index f2c839a..bfee178 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -41,16 +41,13 @@ struct pci_region_entry {
struct pci_bus *child_bus;
struct pci_bus *parent_bus;
struct pci_region_entry *next;
-struct pci_region_entry **pprev;
};
struct pci_bus {
struct {
/* pci region stats */
-u32 count[32 - PCI_MEM_INDEX_SHIFT];
u32 sum, max;
/* pci region assignments */
-u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
@@ -379,11 +376,18 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
entry-is64 = is64;
entry-type = type;
entry-parent_bus = bus;
-list_add_head(bus-r[type].list, entry);
+// Insert into list in sorted order.
+struct pci_region_entry **pprev;
+for (pprev = bus-r[type].list; *pprev; pprev = (*pprev)-next) {
+struct pci_region_entry *pos = *pprev;
+if (pos-size size)
+break;
+}
+entry-next = *pprev;
+*pprev = entry;
u32 index = pci_size_to_index(size, type);
size = pci_index_to_size(index, type);
-bus-r[type].count[index]++;
bus-r[type].sum += size;
if (bus-r[type].max size)
bus-r[type].max = size;
@@ -478,46 +482,14 @@ static int pci_bios_init_root_regions(struct pci_bus
*bus, u32 start, u32 end)
* BAR assignment
/
-static void pci_bios_init_bus_bases(struct pci_bus *bus)
-{
-u32 base, newbase, size;
-int type, i;
-
-for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-dprintf(1, type %s max %x sum %x base %x\n, region_type_name[type],
-bus-r[type].max, bus-r[type].sum, bus-r[type].base);
-base = bus-r[type].base;
-for (i = ARRAY_SIZE(bus-r[type].count)-1; i = 0; i--) {
-size = pci_index_to_size(i, type);
-if (!bus-r[type].count[i])
-continue;
-newbase = base + size * bus-r[type].count[i];
-dprintf(1, size %8x: %d bar(s), %8x - %8x\n,
-size, bus-r[type].count[i], base, newbase - 1);
-bus-r[type].bases[i] = base;
-base = newbase;
-}
-}
-}
-
-static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
-{
-u32 index, addr;
-
-index = pci_size_to_index(size, type);
-addr = bus-r[type].bases[index];
-bus-r[type].bases[index] += pci_index_to_size(index, type);
-return addr;
-}
-
#define PCI_IO_SHIFT8
#define PCI_MEMORY_SHIFT16
#define PCI_PREF_MEMORY_SHIFT 16
static void pci_region_map_one_entry(struct pci_region_entry *entry)
{
-u32 addr = pci_bios_bus_get_addr(
-entry-parent_bus, entry-type, entry-size);
+u32 addr = entry-parent_bus-r[entry-type].base;
+entry-parent_bus-r[entry-type].base += entry-size;
if (!entry-child_bus) {
dprintf(1, PCI: map device bdf=%02x:%02x.%x
bar %d, addr %08x, size %08x [%s]\n,
@@ -559,15 +531,14 @@ static void pci_bios_map_devices(struct pci_bus *busses)
// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
-dprintf(1, PCI: init bases bus %d\n, bus);
-pci_bios_init_bus_bases(busses[bus]);
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-struct pci_region_entry *entry, *next;
-list_foreach_entry_safe(busses[bus].r[type].list, next, entry) {
+struct pci_region_entry *entry = busses[bus].r[type].list;
+while (entry) {
pci_region_map_one_entry(entry);
-list_del(entry);
+struct pci_region_entry *next = entry-next;
free(entry);
+entry = next;
}
}
}
[Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list
In this patch instead of array based resource allocation approach
we calculate resource addresses linked lists of pci_region_entry structures.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 179 -
1 files changed, 50 insertions(+), 129 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 6a285c9..85fe823 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -12,9 +12,8 @@
#include pci_regs.h // PCI_COMMAND
#include xen.h // usingXen
-#define PCI_IO_INDEX_SHIFT 2
-#define PCI_MEM_INDEX_SHIFT 12
-
+#define PCI_DEVICE_IO_MIN 0x4
+#define PCI_DEVICE_MEM_MIN 0x1000
#define PCI_BRIDGE_IO_MIN 0x1000
#define PCI_BRIDGE_MEM_MIN 0x10
@@ -48,38 +47,14 @@ struct pci_region_entry {
struct pci_bus {
struct {
/* pci region stats */
-u32 count[32 - PCI_MEM_INDEX_SHIFT];
u32 sum, max;
-/* seconday bus region sizes */
u32 size;
-/* pci region assignments */
-u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
-static int pci_size_to_index(u32 size, enum pci_region_type type)
-{
-int index = __fls(size);
-int shift = (type == PCI_REGION_TYPE_IO) ?
-PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-if (index shift)
-index = shift;
-index -= shift;
-return index;
-}
-
-static u32 pci_index_to_size(int index, enum pci_region_type type)
-{
-int shift = (type == PCI_REGION_TYPE_IO) ?
-PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-return 0x1 (index + shift);
-}
-
static enum pci_region_type pci_addr_to_type(u32 addr)
{
if (addr PCI_BASE_ADDRESS_SPACE_IO)
@@ -387,19 +362,11 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
entry-size = size;
list_add_head(bus-r[type].list, entry);
entry-parent_bus = bus;
-return entry;
-}
-
-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size)
-{
-u32 index;
-index = pci_size_to_index(size, type);
-size = pci_index_to_size(index, type);
-bus-r[type].count[index]++;
bus-r[type].sum += size;
if (bus-r[type].max size)
bus-r[type].max = size;
+return entry;
}
static int pci_bios_check_devices(struct pci_bus *busses)
@@ -423,14 +390,12 @@ static int pci_bios_check_devices(struct pci_bus *busses)
continue;
int type = pci_addr_to_type(val);
int min_size = (type == PCI_REGION_TYPE_IO) ?
- (1PCI_IO_INDEX_SHIFT) : (1PCI_MEM_INDEX_SHIFT);
+ PCI_DEVICE_IO_MIN : PCI_DEVICE_MEM_MIN;
size = (size min_size) ? size : min_size;
int is64bit = (!(val PCI_BASE_ADDRESS_SPACE_IO)
(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
== PCI_BASE_ADDRESS_MEM_TYPE_64);
-pci_bios_bus_reserve(bus, pci_addr_to_type(val), size);
-
entry = pci_region_create_entry(bus, pci, size, type, is64bit);
if (!entry)
return -1;
@@ -456,7 +421,6 @@ static int pci_bios_check_devices(struct pci_bus *busses)
if (s-r[type].size limit)
s-r[type].size = limit;
s-r[type].size = pci_size_roundup(s-r[type].size);
-pci_bios_bus_reserve(parent, type, s-r[type].size);
entry = pci_region_create_entry(parent, s-bus_dev,
s-r[type].size, type, 0);
if (!entry)
@@ -496,112 +460,69 @@ static int pci_bios_init_root_regions(struct pci_bus
*bus, u32 start, u32 end)
/
- * BAR assignment
+ * Map pci region entries
/
-static void pci_bios_init_bus_bases(struct pci_bus *bus)
+static void pci_region_map_one_entry(struct pci_region_entry *entry)
{
-u32 base, newbase, size;
-int type, i;
-
-for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-dprintf(1, type %s max %x sum %x base %x\n, region_type_name[type],
-bus-r[type].max, bus-r[type].sum, bus-r[type].base);
-base = bus-r[type].base;
-for (i = ARRAY_SIZE(bus-r[type].count)-1; i = 0; i--) {
-size = pci_index_to_size(i, type);
-if (!bus-r[type].count[i])
-continue;
-newbase = base + size * bus-r[type].count[i];
-dprintf(1, size %8x: %d bar(s), %8x - %8x\n,
-size, bus-r[type].count[i], base, newbase - 1);
-bus-r[type].bases[i] = base;
-base = newbase;
-}
+if (!entry-this_bus ) {
+dprintf(1, PCI: bdf %d bar %d\tsize\t0x%08x\tbase 0x%x type %s\n,
+ entry-dev-bdf,
