Re: [PATCH v4 06/10] RISC-V: Add Linux load logic

[Daniel Kiper]

On Thu, Jan 17, 2019 at 02:50:19PM +0000, Leif Lindholm wrote:
> On Thu, Jan 17, 2019 at 01:24:29PM +0100, Daniel Kiper wrote:
> > > +static grub_err_t
> > > +finalize_params_linux (void)
> > > +{
> > > +  int node, retval;
> > > +
> >
> > Please drop this empty line.
> >
> > > +  void *fdt;
> > > +
> > > +  fdt = grub_fdt_load (0x400);
> >
> > Why this number? Please define constant or add a comment here.
> > Whichever is better. And I can see the same value in ARM64. So,
> > maybe it is worth using the same constant here and there. Anyway,
> > please fix it somehow.
>
> So, this one is my fault.
> It effectively comes down to "the space made available for the chosen
> node" - meaning space for the initrd start/end address nodes (and
> their associated strings), and now the address-cells node as well.
> So we need "some extra space".
> (The parameter is the "extra space" that will be dynamically added to
> the DT - the static struct is separately accounted for if we're
> creating an empty one.)
>
> Since at the point this memory gets allocated we're already very close
> to jumping into the kernel, it didn't feel worth trying to calculate
> the exact number of bytes needed.
>
> I agree it's ugly. Would you be OK with a centralised
> GRUB_EFI_LINUX_FDT_EXTRA_SPACE #define?

Yes, go ahead with that.

Daniel