On 05/30/11 17:29, Kevin O'Connor wrote:
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.
That has been fixed already. This and other changes can be found in http://www.kraxel.org/cgit/seabios/log/?h=kraxel.q35
The patches need some more work before they can be posted for re-review+merging though.
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().
I'll have a look.
- 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.
Given the small size roms usually have I dont think this optimization is worth special-casing the rom bar.
cheers, Gerd