Re: [kvm-unit-tests PATCH] lib/devicetree: Support 64 bit addresses for the initrd

[Alexandru Elisei <alexandru.elisei@arm.com>]

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@arm.com>
> > ---
> >  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?

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

Thanks,
Alex

>         *size = end - start;
>  
>         return 0;
> 
> 
> To fix compilation on 32-bit arm.
> 
> 
> And now merged through misc/queue.
> 
> Thanks,
> drew
>