Re: [Qemu-devel] [PATCH 1/2] device_tree: Add qemu_devtree_setprop_sized_cells() utility function

[David Gibson]

On Mon, Jun 24, 2013 at 12:02:39PM +0100, Peter Maydell wrote:
> On 24 June 2013 11:56, Alexander Graf <address@hidden> wrote:
> > On 24.06.2013, at 12:22, Peter Maydell wrote:
> >> We already have a qemu_devtree_setprop_cells() which sets a dtb
> >> property to an array of cells whose values are specified by varargs.
> >> However for the fairly common case of setting a property to a list
> >> of addresses or of address,size pairs the number of cells used by
> >> each element in the list depends on the parent's #address-cells
> >> and #size-cells properties. To make this easier we provide an analogous
> >> qemu_devtree_setprop_sized_cells() function which allows the number
> >> of cells used by each element to be specified.
> >>
> >> Signed-off-by: Peter Maydell <address@hidden>
> >
> > This looks pretty complicated for something actually quite
> > simple: All you want is to pass in a number of 64bit values,
> > rather than 32bit ones, right?
> 
> Nope. If the device tree blob says #address-cells is 1
> and #size-cells is 1, then I want to pass in values to
> go in 32 bit cells. If it says #address-cells is 2 but
> #size-cells is still 1, then I want to pass in a value
> for a 64 bit cell then one for a 32 bit cell. If they're
> both 2 then I want to pass in values for two 64 bit
> cells.

Hmm.. the property certainly needs to be constructed that way.  But I
think Alex's point is that you could make the arguments all 64-bit,
and then truncate them in the generated property.

There's a bigger problem, though, that exists with both versions.  In
general people expect integer arguments like this not to care too much
about the exact integer type, because it will be promoted to the right
thing.  Except with varargs it won't.  So if you ever have a
notionally 64-bit address that happens to fit in a 32-bit variable and
you pass that it, this function will be broken.  And the worst of it
is, it'll work most of the time, until you happen to hit the wrong ABI
and parameter combination :(.

> It's pretty complicated because the device tree
> spec is pretty complicated (if it had just mandated
> 64 bit addr/size everywhere then this would be easy).

Actually, no, because #address-cells and #size-cells cover things
other than "memory-like" addresses.  e.g. #address-cells is 3 for PCI,
which wouldn't be covered by 64-bit addressing.

-- 
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

pgpuHbZDmKanR.pgp
Description: PGP signature