On Mon, Feb 14, 2022 at 02:06:04PM +0000, Alexandru Elisei wrote:
> Hi Drew,
>
> On Mon, Feb 14, 2022 at 02:52:26PM +0100, Andrew Jones wrote:
> > On Mon, Feb 14, 2022 at 12:05:06PM +0000, Alexandru Elisei wrote:
> > > The "linux,initrd-start" and "linux,initrd-end" properties encode the start
> > > and end address of the initrd. The size of the address is encoded in the
> > > root node #address-cells property and can be 1 cell (32 bits) or 2 cells
> > > (64 bits). Add support for parsing a 64 bit address.
> > >
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> > > ---
> > > lib/devicetree.c | 18 +++++++++++++-----
> > > 1 file changed, 13 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/devicetree.c b/lib/devicetree.c
> > > index 409d18bedbba..7cf64309a912 100644
> > > --- a/lib/devicetree.c
> > > +++ b/lib/devicetree.c
> > > @@ -288,7 +288,7 @@ int dt_get_default_console_node(void)
> > > int dt_get_initrd(const char **initrd, u32 *size)
> > > {
> > > const struct fdt_property *prop;
> > > - const char *start, *end;
> > > + u64 start, end;
> > > int node, len;
> > > u32 *data;
> > >
> > > @@ -303,7 +303,11 @@ int dt_get_initrd(const char **initrd, u32 *size)
> > > if (!prop)
> > > return len;
> > > data = (u32 *)prop->data;
> > > - start = (const char *)(unsigned long)fdt32_to_cpu(*data);
> > > + start = fdt32_to_cpu(*data);
> > > + if (len == 8) {
> > > + data++;
> > > + start = (start << 32) | fdt32_to_cpu(*data);
> > > + }
> > >
> > > prop = fdt_get_property(fdt, node, "linux,initrd-end", &len);
> > > if (!prop) {
> > > @@ -311,10 +315,14 @@ int dt_get_initrd(const char **initrd, u32 *size)
> > > return len;
> > > }
> > > data = (u32 *)prop->data;
> > > - end = (const char *)(unsigned long)fdt32_to_cpu(*data);
> > > + end = fdt32_to_cpu(*data);
> > > + if (len == 8) {
> > > + data++;
> > > + end = (end << 32) | fdt32_to_cpu(*data);
> > > + }
> > >
> > > - *initrd = start;
> > > - *size = (unsigned long)end - (unsigned long)start;
> > > + *initrd = (char *)start;
> > > + *size = end - start;
> > >
> > > return 0;
> > > }
> > > --
> > > 2.35.1
> > >
> >
> > I added this patch on
>
> Thanks for the quick reply!
>
> >
> > diff --git a/lib/devicetree.c b/lib/devicetree.c
> > index 7cf64309a912..fa8399a7513d 100644
> > --- a/lib/devicetree.c
> > +++ b/lib/devicetree.c
> > @@ -305,6 +305,7 @@ int dt_get_initrd(const char **initrd, u32 *size)
> > data = (u32 *)prop->data;
> > start = fdt32_to_cpu(*data);
> > if (len == 8) {
> > + assert(sizeof(long) == 8);
>
> I'm sketchy about arm with LPAE, but wouldn't it be legal to have here a 64
> bit address, even if the architecture is 32 bits? Or was the assert added
> more because kvm-unit-tests doesn't support LPAE on arm?
It's possible, but only if we choose to manage it. We're (I'm) lazy and
require physical addresses to fit in the pointers, at least for the test
framework. Of course a unit test can feel free to play around with larger
physical addresses if it wants to.
>
> > data++;
> > start = (start << 32) | fdt32_to_cpu(*data);
> > }
> > @@ -321,7 +322,7 @@ int dt_get_initrd(const char **initrd, u32 *size)
> > end = (end << 32) | fdt32_to_cpu(*data);
> > }
> >
> > - *initrd = (char *)start;
> > + *initrd = (char *)(unsigned long)start;
>
> My bad here, I forgot to test on arm. Tested your fix and the compilation
> error goes away.
I'm actually kicking myself a bit for the hasty fix, because the assert
would be better done at the end and written something like this
assert(sizeof(long) == 8 || !(end >> 32));
I'm not sure it's worth adding another patch on top for that now, though.
By the lack of new 32-bit arm unit tests getting submitted, I'm not even
sure it's worth maintaining 32-bit arm at all...
Thanks,
drew
>
> Thanks,
> Alex
>
> > *size = end - start;
> >
> > return 0;
> >
> >
> > To fix compilation on 32-bit arm.
> >
> >
> > And now merged through misc/queue.
> >
> > Thanks,
> > drew
> >
>
