Re: [Qemu-devel] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

[Kevin O'Connor]

On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
> Migrate 64bit entries to 64bit pci regions if they do
> not fit in 32bit range.
[...]
> +static void pci_region_migrate_64bit_entries(struct pci_region *from,
> +                                             struct pci_region *to)
> +{
> +    struct pci_region_entry **pprev = &from->list;
> +    struct pci_region_entry **last = &to->list;
> +    while(*pprev) {
> +        if ((*pprev)->is64) {
> +            struct pci_region_entry *entry;
> +            entry = *pprev;
> +            /* Delete the entry and move next */
> +            *pprev = (*pprev)->next;
> +            /* Add entry at tail to keep a sorted order */
> +            entry->next = NULL;
> +            if (*last) {
> +               (*last)->next = entry;
> +                last  = &(*last)->next;
> +            }
> +            else
> +               (*last) = entry;
> +        }
> +        else
> +            pprev = &(*pprev)->next;
> +    }
> +}

It should be possible to simplify this - something like (untested):

static void pci_region_migrate_64bit_entries(struct pci_region *from,
                                             struct pci_region *to)
{
    struct pci_region_entry **pprev = &from->list, **last = &to->list;
    for (; *pprev; pprev = &(*pprev)->next) {
        struct pci_region_entry *entry = *pprev;
        if (!entry->is64)
            continue;
        // Move from source list to dest list.
        *pprev = entry->next;
        entry->next = NULL;
        *last = entry;
        last = &entry->next;
    }
}

[...]
>  static void pci_bios_map_devices(struct pci_bus *busses)
>  {
> +    if (pci_bios_init_root_regions(busses)) {
> +        struct pci_region r64_mem, r64_pref;
> +        r64_mem.list = NULL;
> +        r64_pref.list = NULL;
> +        pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
> +                                         &r64_mem);
> +        
> pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
> +                                         &r64_pref);
> +
> +        if (pci_bios_init_root_regions(busses))
> +            panic("PCI: out of address space\n");
> +
> +        r64_mem.base = BUILD_PCIMEM64_START;
> +        r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(&r64_mem),
> +                              pci_region_align(&r64_pref));

There should be a check to see if the regions fit.  Maybe pass
start/end into pci_bios_init_root_regions() and call it again for the
>4g region?

> +        pci_region_map_entries(busses, &r64_mem);
> +        pci_region_map_entries(busses, &r64_pref);
> +    }
>      // Map regions on each device.

This doesn't look right to me.  This will map the devices on bus 0 to
the proper >4g address, but devices on any subsequent bus will use
busses[0].r[].base which will be reset to the <4gig address.  Perhaps
pull base out of pci_region and make pci_region_map_entries()
recursive?

-Kevin