Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it

[Leif Lindholm <leif@nuviainc.com>]

On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote:
> Now that we implemented supported for the LoadFile2 protocol for initrd
> loading, there is no longer a need to pass the initrd parameters via
> the device tree. This means there is no longer a reason to update the
> device tree in the first place, and so we can ignore it entirely.

There is a change in behaviour here which I don't think matters, but
I'll call it out anyway:
If there was ever a kernel out there with an EFI stub that depended on
a chosen node existing in the DT, and the one provide by firmware did
not contain one, that setup would break from this *if* it didn't use
an initrd.

/
    Leif

> The only remaining reason to deal with the devicetree is if we are
> using the 'devicetree' command to load one from disk, so tweak the
> logic in grub_fdt_install() to take that into account.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  grub-core/loader/arm64/linux.c | 22 ++++++++++----------
>  grub-core/loader/efi/fdt.c     |  7 +++++--
>  2 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 285422c7bd43..9e282b6660fe 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -93,21 +93,21 @@ finalize_params_linux (void)
>  
>    void *fdt;
>  
> -  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
> +  /* Set initrd info */
> +  if (initrd_start && initrd_end > initrd_start)
> +    {
> +      fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
>  
> -  if (!fdt)
> -    goto failure;
> +      if (!fdt)
> +	goto failure;
>  
> -  node = grub_fdt_find_subnode (fdt, 0, "chosen");
> -  if (node < 0)
> -    node = grub_fdt_add_subnode (fdt, 0, "chosen");
> +      node = grub_fdt_find_subnode (fdt, 0, "chosen");
> +      if (node < 0)
> +	node = grub_fdt_add_subnode (fdt, 0, "chosen");
>  
> -  if (node < 1)
> -    goto failure;
> +      if (node < 1)
> +	goto failure;
>  
> -  /* Set initrd info */
> -  if (initrd_start && initrd_end > initrd_start)
> -    {
>        grub_dprintf ("linux", "Initrd @ %p-%p\n",
>  		    (void *) initrd_start, (void *) initrd_end);
>  
> diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
> index ee9c5592c700..ab900b27d927 100644
> --- a/grub-core/loader/efi/fdt.c
> +++ b/grub-core/loader/efi/fdt.c
> @@ -85,13 +85,16 @@ grub_fdt_install (void)
>    grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
>    grub_efi_status_t status;
>  
> +  if (!fdt && !loaded_fdt)
> +    return GRUB_ERR_NONE;
> +
>    b = grub_efi_system_table->boot_services;
> -  status = b->install_configuration_table (&fdt_guid, fdt);
> +  status = b->install_configuration_table (&fdt_guid, fdt ?: loaded_fdt);
>    if (status != GRUB_EFI_SUCCESS)
>      return grub_error (GRUB_ERR_IO, "failed to install FDT");
>  
>    grub_dprintf ("fdt", "Installed/updated FDT configuration table @ %p\n",
> -		fdt);
> +		fdt ?: loaded_fdt);
>    return GRUB_ERR_NONE;
>  }
>  
> -- 
> 2.17.1
>