[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation
From: |
Kevin O'Connor |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list |
Date: |
Wed, 11 Apr 2012 23:15:51 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
> "busses".
> - 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
> "entry->bar".
> 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?
-Kevin
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Kevin O'Connor, 2012/04/01
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Kevin O'Connor, 2012/04/01
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Alexey Korolev, 2012/04/03
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Kevin O'Connor, 2012/04/03
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Alexey Korolev, 2012/04/11
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list,
Kevin O'Connor <=
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Alexey Korolev, 2012/04/11
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Kevin O'Connor, 2012/04/12
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Alexey Korolev, 2012/04/19
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Kevin O'Connor, 2012/04/19
- Re: [Qemu-devel] [PATCH 3/4] Switch from array based resource allocation to list, Alexey Korolev, 2012/04/20