On Tue, May 24, 2011 at 11:05:32AM +0200, Gerd Hoffmann wrote:
This patch adds a second device scan to the pci initialization, which counts the memory bars of the various sizes and types. Then it calculates the sizes and the packing of the prefetchable and non-prefetchable pci memory windows and prints the results.
Thanks Gerd. In general, I'm okay with this approach.
Some random comments..
+static struct pci_bus {
- /* pci region stats */
- u32 io_count[16 - PCI_IO_INDEX_SHIFT];
- u32 mem_count[32 - PCI_MEM_INDEX_SHIFT];
- u32 prefmem_count[32 - PCI_MEM_INDEX_SHIFT];
- u32 io_sum, io_max;
- u32 mem_sum, mem_max;
- u32 prefmem_sum, prefmem_max;
- /* seconday bus region sizes */
- u32 io_size, mem_size, prefmem_size;
- /* pci region assignments */
- u32 io_bases[16 - PCI_IO_INDEX_SHIFT];
- u32 mem_bases[32 - PCI_MEM_INDEX_SHIFT];
- u32 prefmem_bases[32 - PCI_MEM_INDEX_SHIFT];
- u32 io_base, mem_base, prefmem_base;
+} busses[32];
The size of the seabios rom grows with every static variable, so this should be dynamically allocated.
Speaking of dynamic allocation - it would be great if seabios had a "struct pcidevice" for every found device - then most cases of foreachpci() could instead just walk through this list of pci devices. The lack of this has already lead to hacks like pci.c:pci_path_setup().
+static int pci_size_to_index(u32 size, int shift) +{
- int index = 0;
- while (size > (1 << index)) {
index++;- }
util.h:__fls()
+static void pci_bios_check_device(struct pci_bus *bus, u16 bdf) +{
[...]
- for (i = 0; i < PCI_NUM_REGIONS; i++) {
u32 val, size;pci_bios_bus_get_bar(bus, bdf, i, &val, &size);if (val == 0) {continue;}pci_bios_bus_reserve(bus, val, size);
If I read this correctly, the code reserves unique space for each ROM bar - this shouldn't be necessary as I believe it's safe to assume only one ROM will be mapped at a given time.
-Kevin