| Message ID | 1315989802-18753-29-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
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. 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 :).
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. 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. Alex
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. > 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.
Am 15.09.2011 um 13:03 schrieb David Gibson <david@gibson.dropbear.id.au>: > 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. 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. Also, I'm perfectly fine with this being low on the list. Alex >
On Thu, Sep 15, 2011 at 05:00:41PM +0200, Alexander Graf wrote: > > Am 15.09.2011 um 13:03 schrieb David Gibson <david@gibson.dropbear.id.au>: > > > 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.
diff --git a/device_tree.c b/device_tree.c index 751538e..dc69232 100644 --- a/device_tree.c +++ b/device_tree.c @@ -41,6 +41,7 @@ void *load_device_tree(const char *filename_path, int *sizep) } /* Expand to 2x size to give enough room for manipulation. */ + dt_size += 10000; dt_size *= 2; /* First allocate space in qemu for device tree */ fdt = g_malloc0(dt_size);
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. Signed-off-by: Alexander Graf <agraf@suse.de> --- device_tree.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)