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, <javi...@redhat.com>
> > wrote:
> >
> > > From: Mark Salter <msal...@redhat.com>
> > >
> > > 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 <msal...@redhat.com>
> > > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com>
> > > ---
> > >
> > > 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,
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
