[SeaBIOS] [PATCH 1/4] pci: add two-pass pci initialization code

[Gerd Hoffmann]

   Hi,

>> +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;
>
> Hrmm - is there an opportunity to define an array of three "region
> types" (io, mem, prefmem) instead?  The coreboot PCI assignment code
> does this.

I'll try and see how the code will look like.
Was thinking about that too.

> [...]
>> +static void pci_bios_check_device_in_bus(int bus)
>> +{
>> +    struct pci_device *pci;
>> +
>> +    dprintf(1, "PCI: check devices bus %d\n", bus);
>> +    foreachpci(pci) {
>> +        if (pci->rootbus != bus)
>
> rootbus doesn't look correct here - it's an identifier for independent
> root PCI buses - pci_bdf_to_bus(pci->bdf) is likely what you want.

Ok.

>> +static int pci_bios_init_root_regions(void)
>> +{
>> +    struct pci_bus *bus =&busses[0];
>> +    int h1, h2;
>> +
>> +    /* io ports */
>> +    bus->io_base = 0xc000;
>> +
>> +    /* try to fit all into one memory hole */
>> +    for (h1 = mem_holes_count-1; h1>= 0; h1--) {
>> +        if (bus->mem_sum<  bus->prefmem_sum) {
>
> I'm a bit confused by what this code does.

A little more background:

I had (and still have in another branch) code which creates the 
mem_holes array from the e820 table to figure where seabios can allocate 
address space for the PCI memory regions.  With the piix emulation this 
gave us one memory hold, from RamSize to BUILD_IOAPIC_ADDR.  With the 
q35 emulation this gave us two memory holes, one from RamSize to 
0xe0000000 and one from 0xf0000000 to BUILD_IOAPIC_ADDR due to mmconfig 
@ 0xe0000000 -> 0xefffffff.

The pci_bios_init_root_regions() function can place one pci memory 
region in one memory hole and the other one in the second in case they 
are to big to fit into one hole.

So the question is whenever we'll wanna go down the route to have 
dynamic i/o regions, with the dsdt changes discussed here recently.  Or 
whenever we'll just stick with a fixed region and just make it larger 
for the q35 emulation.  Say allow guest ram up to 0xc0000000, then have 
the mmconfig region @ 0xc0000000 -> 0xcfffffff, and have the single hole 
0xd0000000 -> BUILD_IOAPIC_ADDR available for pci memory bar 
allocations.  If we plan for a single memory hole the function can be 
simplified alot, basically everything below "if (mem_holes_count <= 1) { 
return -1 }" can be zapped then.

cheers,
   Gerd