Re: [Qemu-devel] [Qemu-ppc] [PATCH 28/58] device tree: give dt more size

[David Gibson]

On Thu, Sep 15, 2011 at 05:00:41PM +0200, Alexander Graf wrote:
> 
> Am 15.09.2011 um 13:03 schrieb David Gibson <address@hidden>:
> 
> > On Thu, Sep 15, 2011 at 09:37:48AM +0200, Alexander Graf wrote:
> >> 
> >> On 15.09.2011, at 05:19, David Gibson wrote:
> >> 
> >>> On Wed, Sep 14, 2011 at 10:42:52AM +0200, Alexander Graf wrote:
> >>>> We currently load a device tree blob and then just take its size x2 to
> >>>> account for modifications we do inside. While this is nice and great,
> >>>> it fails when we have a small device tree as blob and lots of nodes added
> >>>> in machine init code.
> >>>> 
> >>>> So for now, just make it 20k bigger than it was before. We maybe want to
> >>>> be more clever about this later.
> >>> 
> >>> In fact, one of the few things I can think of that might justify
> >>> qemu's "abstraction" of the libfdt interface, is that the wrappers
> >>> could be modified to detect -FDT_ERR_NOSPACE and realloc()
> >>> appropriately.
> >> 
> >> Oh, yeah, that sounds like a very good idea!
> >> 
> >>> Otherwise the wrappers, which are limited and not notably simpler to
> >>> use than the raw libfdt functions seem pretty pointless to me.
> >>> 
> >>> Not that I'm biased as the author of libfdt or anything :).
> >> 
> >> I agree that the wrappers are not all that overly useful atm. I was
> >> actually very close to just ripping them out completely instead of
> >> extending them for new functionality. I did have the feeling that
> >> wrapping libfdt would give us a few benefits, maybe even the chance
> >> of getting rid of #ifdefs in target code.
> > 
> > Hrm, maybe.  Can't really see it.  Of course, my preference would be
> > to get rid of those #ifdefs by embedding libfdt in qemu so it's always
> > there.
> 
> It's a library and should be treated that way.

What the hell does that mean.  It's a library, and things are easier
when you can count on it being there.  Embedding means we can do that
without adding an external dependency.  And we already use git
submodules, so that provides an easy way of embedding.

> But yeah, I'm
> inclined to fail configure for ppc when libfdt can't be found too.
> 
> >> Could you please put this on your todo list? We should probably
> >> force every target code in QEMU to only use the wrappers and
> >> dynamically realloc() in them.
> > 
> > Uh, sure, but it's a long list and it won't be near the top.
> > 
> > The wrappers would need to be a lot more extensive to do this.  I use
> > libfdt directly in the spapr code for a reason.
> 
> Expensiveness is not too bad here, no? It should still be fast.

Uh, how did expensiveness get into this?  I'm talking about the sheer
number of potentially useful interfaces that would have to be wrapped
with boilerplate code.

> Also, I'm perfectly fine with this being low on the list.

Also note that handling -FDT_ERR_NOSPACE only applies to write
functions.  I still see little point in wrapping the read only
functions.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson