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

[Kevin O'Connor]

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