[SeaBIOS] [PATCH 3/4] Switch from array based resource allocation to list
kevin at koconnor.net
Thu Apr 12 05:15:51 CEST 2012
On Thu, Apr 12, 2012 at 02:44:54PM +1200, Alexey Korolev wrote:
> On 04/04/12 15:31, Kevin O'Connor wrote:
> > Agreed - the only thing it does is force a minimum size for memory bars as you pointed out in a previous email. As above, I did play with
> > this a little more on Sunday. I also added in two patches from Gerd's series and made alignment handling more explicit. I'm including the
> > series here if you're interested. Again, I think this should wait until after the 1.7.0 release. -Kevin
> Hi Kevin,
> Sorry for late response, was quite busy.
> I've reviewed and tried your patches.
> [Patches 1-4] are almost the same as I proposed in this series.
Yes. It's your patches tweaked slightly.
> The noticeable differences are:
> 1) instead of sorting entries at mapping stage your patches sort entries at allocation stage. (no difference in behavior or readability so
> any approach is fine for me)
> 2) instead of storing pointer to bus resource which the bridge device represents (this_bus), it is obtained from pci structure and array of
> - entry->this_bus->r[entry->type].base = entry->base;
> + struct pci_bus *child_bus = &busses[entry->dev->secondary_bus];
> + child_bus->r[entry->type].base = addr;
> Since "entry->this_bus" member is removed the information about whether the resource is bridge region or PCI BAR is encoded inside
> Value "-1" - means this is a bridge region, the positive values mean it is a BAR.
> IMHO 2) makes code less readable, at least a comment explaining the meaning of negative value of PCI BAR is required.
> I've found just cosmetic difference to my patches so please don't forget to add my sign-off-by to them.
Okay - will do.
> [Patch 5] Track region alignment explicitly. Looks very good and safe. Should reduce address range wastes.
> [Patch 6] Small patch to account resources of PCI bridges.
> Looks fine.
> May be instead of
> + num_regions = 2;
> I would add
> #define PCI_BRIDGE_NUM_REGIONS 2
> + num_regions = PCI_BRIDGE_NUM_REGIONS;
This was me playing with some of Gerd's patches. I think it would
require additional changes before committing. In particular, the
patch misses the ROM bar on bridges - I think maybe it should be
changed to keep the same loop constraints but add a "if (bar >
PCI_BRIDGE_NUM_REGIONS && bar < PCI_ROM_SLOT) continue;".
> [Patch 7] 64bit aware code. Patch is incomplete. It is not working.
This was also me playing with one of Gerd's patches. It just makes
the bar read/write code 64bit aware. It doesn't actually program
them. The logic to do real 64bit allocations would require list
merging. Is this something you have looked at?
More information about the SeaBIOS