[SeaBIOS] [RfC PATCH 1/2] pci: move to two-pass pci initialization

[Gerd Hoffmann]

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