Re: [PATCH] arm64: make sure fdt has #address-cells and #size-cells properties

[Daniel Kiper]

Adding Leif and Alex...

On Thu, Oct 31, 2019 at 01:24:01PM -0400, Mark Salter wrote:
> On Thu, 2019-10-31 at 16:59 +0100, Javier Martinez Canillas wrote:
> > On 10/31/19 12:56 PM, Vladimir 'phcoder' Serbinenko wrote:
> > > This patch looks incomplete. What if "chosen" is there but does not 
> > > contain
> > > the relevant fields?
> > >
> >
> > My understanding is that if there's a chosen node already defined in the
> > Device Tree but without #address-cells and #size-cells properties, then
> > is a malformed DT and is not up to GRUB to correct it.
>
> Right. IIRC, this was the case where firmware only has ACPI tables and grub
> has to create a DT with just a chosen node.
>
> >
> > But if GRUB is adding a child node as is the case of the chosen node, then
> > is up to GRUB to make sure that there are #address-cells and #size-cells
> > properties also defined so the child nodes are addressed correctly.
> >
> > But maybe I'm wrong on this, so I'll let Mark comment since he authored
> > the patch and knows better what's the problem that's fixing.
> >
> > > On Thu, 31 Oct 2019, 11:39 Javier Martinez Canillas, <address@hidden>
> > > wrote:
> > >
> > > > From: Mark Salter <address@hidden>
> > > >
> > > > Recent upstream changes to kexec-tools relies on #address-cells
> > > > and #size-cells properties in the FDT. If grub2 needs to create
> > > > a chosen node, it is likely because firmware did not provide one.
> > > > In that case, set #address-cells and #size-cells properties to
> > > > make sure they exist.
> > > >
> > > > Signed-off-by: Mark Salter <address@hidden>
> > > > Signed-off-by: Javier Martinez Canillas <address@hidden>
> > > > ---
> > > >
> > > >  grub-core/loader/arm64/linux.c | 16 +++++++++++++++-
> > > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git grub-core/loader/arm64/linux.c grub-core/loader/arm64/linux.c
> > > > index ef3e9f9444c..24d73732d89 100644
> > > > --- grub-core/loader/arm64/linux.c
> > > > +++ grub-core/loader/arm64/linux.c
> > > > @@ -78,7 +78,21 @@ finalize_params_linux (void)
> > > >
> > > >    node = grub_fdt_find_subnode (fdt, 0, "chosen");
> > > >    if (node < 0)
> > > > -    node = grub_fdt_add_subnode (fdt, 0, "chosen");
> > > > +    {
> > > > +      /*
> > > > +       * If we have to create a chosen node, Make sure we
> > > > +       * have #address-cells and #size-cells properties.
> > > > +       */
> > > > +      retval = grub_fdt_set_prop32(fdt, 0, "#address-cells", 2);
> > > > +      if (retval)
> > > > +        goto failure;
> > > > +
> > > > +      retval = grub_fdt_set_prop32(fdt, 0, "#size-cells", 2);
> > > > +      if (retval)
> > > > +        goto failure;
> > > > +
> > > > +      node = grub_fdt_add_subnode (fdt, 0, "chosen");
> > > > +    }
> > > >
> > > >    if (node < 1)
> > > >      goto failure;
> > > > --
> > > > 2.21.0
> > > >
> > > >
> >
> > Best regards,