[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function
From: |
Michael Clark |
Subject: |
Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function |
Date: |
Sun, 6 May 2018 09:59:50 +1200 |
On Sat, May 5, 2018 at 11:48 PM, David Gibson <address@hidden>
wrote:
> On Sat, May 05, 2018 at 11:44:25AM +0100, Peter Maydell wrote:
> > cc'ing David -- is this the best way to do this?
> >
> > (It would be nicer if libfdt would just dynamically
> > reallocate the buffer as needed,
>
> That's a deliberate design tradeoff. Because it's designed to be
> embeddable in really limited environments, libfdt tries to keep to
> really minimal dependencies - in particular it doesn't depend on an
> allocator.
>
> It'd be nice to have another (optional) layer on top that does
> reallocate automatically.allocator. In principle it's pretty simple -
> you just call down to the existing functions and if they return
> FDT_ERR_NOSPACE, rellocate and retry. I've never had the time to
> write it though - especially since in most simple cases just
> allocating a far-to-big buffer initially actually works pretty well,
> crude though it may be.
>
> For complex fdt manipulations.. well.. there comes a point where fdt
> is the wrong tool for the job and you'd be better off using a "live"
> representation of the tree and just flattening it when you're
> finished. IMO qemu is already past that point. I send some early
> draft patches adding live tree infrastructure a while back but got too
> sidetracked by higher priorities to follow it through.
>
> > and then tell you
> > the final size, rather than having to specify a
> > maximum buffer size up front, but given the API we
> > have a "tell me how big this thing actually is"
> > function seems reasonable. I'm just surprised we
> > need to know so much about the DT internals to do it.)
>
> I'm not familiar with the rest of this thread, so I'm not sure at what
> point you're wanting to know this. If it's only right at the end
> before loading the final blob you can fdt_pack() and then check
> fdt_totalsize().
Okay, so an alternative is to call fdt_pack() and then fdt_totalsize().
Thanks!
QEMU has its own wrapper around libfdt and neither the fdt_pack() nor
fdt_totalsize() functions are exposed.
Some architecture use only the wrapper functions provided by
qemu/include/include/sysemu/device_tree.h
It seems that target/ppc has #include <libfdt.h> and calls fdt_pack() and
fdt_totalsize() directly.
Some architectures appear to copy the unpacked fdt into their ROMS where
the allocated size in QEMU defaults to 1MiB.
QEMU has a wrapper function called create_device_tree that
calls fdt_create(), fdt_finish_reservemap(), fdt_begin_node(),
fdt_end_node(), fdt_finish(), fdt_open_into()
with a 1MiB initial buffer size. The resulting device tree is modified in
memory using various QEMU wrapper APIs that call fdt_setprop_string(),
fdt_setprop_cell(), etc
For RISC-V we were able to get an accurate size using this function:
size_t qemu_fdt_totalsize(void *fdt)
{
return fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt) +
fdt_size_dt_strings(fdt) + sizeof(uint32_t) /* terminator */;
}
Now that we're aware of fdt_pack() and then fdt_totalsize() then we can
avoid changing target neutral code in
qemu/include/include/sysemu/device_tree.h by following ppc's lead and add
#include <libfdt.h> and call fdt_pack() and fdt_totalsize() directly.
The thing I spotted in fdt_resize() was the calculation to check that the
new buffer was large enough however it excludes dt_off_dt_struct(fdt) and
space for the uin32_t terminator. See here:
int fdt_resize(void *fdt, void *buf, int bufsize)
{
...
headsize = fdt_off_dt_struct(fdt);
tailsize = fdt_size_dt_strings(fdt);
if ((headsize + tailsize) > bufsize)
return -FDT_ERR_NOSPACE;
...
return 0;
}
>
> > thanks
> > -- PMM
> >
> > On 4 May 2018 at 02:19, Michael Clark <address@hidden> wrote:
> > > Currently the device-tree create_device_tree function
> > > returns the size of the allocated device tree buffer
> > > however there is no way to get the actual amount of
> > > buffer space used by the device-tree.
> > >
> > > 14ec3cbd7c1e31dca4d23f028100c8f43e156573 increases
> > > the FDT_MAX_SIZE to 1 MiB. This creates an issue
> > > for boards that have less than 1 MiB in their ROM
> > > for device tree. While cpu_physical_memory_write
> > > will not write past the end of a buffer there is
> > > and a board is aware of its ROM buffer size, so
> > > can use min(fdt_size,rom_size); this provides no
> > > indication as to whether the device-tree may be
> > > truncated. qemu_fdt_totalsize allows a board to
> > > check that a dynamically created device tree will
> > > fit within its alloted ROM space.
> > >
> > > Add qemu_fdt_totalsize which uses the logic and
> > > public APIs from libfdt to calculate the device
> > > size: struct_offset + struct_size + strings_size
> > > + terminator.
> > >
> > > Cc: Peter Crosthwaite <address@hidden>
> > > Cc: Alexander Graf <address@hidden>
> > > Cc: Alistair Francis <address@hidden>
> > > Cc: Peter Maydell <address@hidden>
> > > ---
> > > device_tree.c | 6 ++++++
> > > include/sysemu/device_tree.h | 6 ++++++
> > > 2 files changed, 12 insertions(+)
> > >
> > > diff --git a/device_tree.c b/device_tree.c
> > > index 52c3358a5583..3a2166d61f37 100644
> > > --- a/device_tree.c
> > > +++ b/device_tree.c
> > > @@ -215,6 +215,12 @@ void *load_device_tree_from_sysfs(void)
> > >
> > > #endif /* CONFIG_LINUX */
> > >
> > > +size_t qemu_fdt_totalsize(void *fdt)
> > > +{
> > > + return fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt) +
> > > + fdt_size_dt_strings(fdt) + sizeof(uint32_t) /* terminator
> */;
> > > +}
> > > +
> > > static int findnode_nofail(void *fdt, const char *node_path)
> > > {
> > > int offset;
> > > diff --git a/include/sysemu/device_tree.h
> b/include/sysemu/device_tree.h
> > > index e22e5bec9c3f..4af232dfdc65 100644
> > > --- a/include/sysemu/device_tree.h
> > > +++ b/include/sysemu/device_tree.h
> > > @@ -26,6 +26,12 @@ void *load_device_tree_from_sysfs(void);
> > > #endif
> > >
> > > /**
> > > + * qemu_fdt_total_size: returns the size required to store the current
> > > + * device tree versus the buffer size returned by create_device_tree
> > > + */
> > > +size_t qemu_fdt_totalsize(void *fdt);
> > > +
> > > +/**
> > > * qemu_fdt_node_path: return the paths of nodes matching a given
> > > * name and compat string
> > > * @fdt: pointer to the dt blob
> >
>
> --
> 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
>
- [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Michael Clark, 2018/05/03
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Peter Maydell, 2018/05/05
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, David Gibson, 2018/05/05
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function,
Michael Clark <=
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Michael Clark, 2018/05/05
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Peter Maydell, 2018/05/06
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, David Gibson, 2018/05/06
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Peter Maydell, 2018/05/06
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, David Gibson, 2018/05/09
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, Peter Maydell, 2018/05/09
- Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function, David Gibson, 2018/05/06