All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: Atish Patra <atishp@atishpatra.org>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>,
	linux-efi <linux-efi@vger.kernel.org>,
	The development of GNU GRUB <grub-devel@gnu.org>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Leif Lindholm <leif@nuviainc.com>
Subject: Re: [PATCH v2 7/8] efi: implement LoadFile2 initrd loading protocol for Linux
Date: Mon, 26 Oct 2020 23:00:53 +0100	[thread overview]
Message-ID: <CAMj1kXHdXQhxrLL3_N711XOv4SAbpwbBSScZYNt6Dy1L99vKOg@mail.gmail.com> (raw)
In-Reply-To: <CAOnJCUJVRBBVm6id-5U_zrqqKewuXLt-c9tTBuBYaLXEnox-jQ@mail.gmail.com>

On Mon, 26 Oct 2020 at 22:38, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Sun, Oct 25, 2020 at 6:50 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
> >
> > Recent Linux kernels will invoke the LoadFile2 protocol installed on a
> > well-known vendor media devicepath to load the initrd if it is exposed
> > by the firmware. Using this method is preferred for two reasons:
> > - the Linux kernel is in charge of allocating the memory, and so it can
> >   implement any placement policy it wants (given that these tend to
> >   change between kernel versions),
> > - it is no longer necessary to modify the device tree provided by the
> >   firmware.
> >
> > So let's install this protocol when handling the 'initrd' command if
> > such a recent kernel was detected (based on the PE/COFF image version),
> > and defer loading the initrd contents until the point where the kernel
> > invokes the LoadFile2 protocol.
> >
>
> Thanks for adding the support for LoadFile2 protocol. This was one of
> the blockers for full RISC-V support.
> How do you prefer to proceed with RISC-V support ?
>

I'll defer to Daniel and Leif for the answer to that question.

> For RISC-V, we just need to move the arm64 loader to a common file so
> that both RISC-V/ARM64 can use it apart from
> few header file fixes. During the last attempt[1], I moved it to
> loader/efi/linux.c.
>

That seems the most logical to me. Given that it is shared between ARM
and arm64 today, it doesn't belong in grub-core/loader/arm64/linux.c
to begin with.

As I understand it, Daniel is doing a release imminently. Once that is
out of the way, I'm happy to proceed whichever way the maintainers
prefer.

> I am happy to test it on Qemu/hardware, if you want to send the series
> either part of this one or a separate series.
> I can rebase my previous series on top of this series as well. Please
> let me know your preference.
>

That would be a good start in any case, to ensure that the PE/COFF
image loading and LoadFile2 handling works as expected. And perhaps
simply merging the changes in that order is the most straight-forward.



> [1] https://www.mail-archive.com/grub-devel@gnu.org/msg30107.html
>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > ---
> >  grub-core/loader/arm64/linux.c | 109 +++++++++++++++++++-
> >  1 file changed, 108 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> > index 28ff8584a3b5..c6a95e1f0c43 100644
> > --- a/grub-core/loader/arm64/linux.c
> > +++ b/grub-core/loader/arm64/linux.c
> > @@ -48,6 +48,10 @@ static grub_uint32_t cmdline_size;
> >  static grub_addr_t initrd_start;
> >  static grub_addr_t initrd_end;
> >
> > +static struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
> > +static grub_efi_handle_t initrd_lf2_handle;
> > +static int initrd_use_loadfile2;
> > +
> >  grub_err_t
> >  grub_arch_efi_linux_load_image_header (grub_file_t file,
> >                                        struct linux_arch_kernel_header * lh)
> > @@ -81,6 +85,18 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
> >         return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF image header");
> >      }
> >
> > +  /*
> > +   * Linux kernels built for any architecture are guaranteed to support the
> > +   * LoadFile2 based initrd loading protocol if the image version is >= 1.
> > +   */
> > +  if (lh->coff_image_header.optional_header.major_image_version >= 1)
> > +    initrd_use_loadfile2 = 1;
> > +  else
> > +    initrd_use_loadfile2 = 0;
> > +
> > +  grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
> > +               initrd_use_loadfile2 ? "en" : "dis");
> > +
> >    return GRUB_ERR_NONE;
> >  }
> >
> > @@ -250,13 +266,86 @@ allocate_initrd_mem (int initrd_pages)
> >                                        GRUB_EFI_LOADER_DATA);
> >  }
> >
> > +struct initrd_media_device_path {
> > +  grub_efi_vendor_media_device_path_t  vendor;
> > +  grub_efi_device_path_t               end;
> > +} GRUB_PACKED;
> > +
> > +#define LINUX_EFI_INITRD_MEDIA_GUID  \
> > +  { 0x5568e427, 0x68fc, 0x4f3d, \
> > +    { 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68 } \
> > +  }
> > +
> > +static struct initrd_media_device_path initrd_lf2_device_path = {
> > +  {
> > +    {
> > +      GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
> > +      GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
> > +      sizeof(grub_efi_vendor_media_device_path_t),
> > +    },
> > +    LINUX_EFI_INITRD_MEDIA_GUID
> > +  }, {
> > +    GRUB_EFI_END_DEVICE_PATH_TYPE,
> > +    GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
> > +    sizeof(grub_efi_device_path_t)
> > +  }
> > +};
> > +
> > +static grub_efi_status_t initrd_load_file2(grub_efi_load_file2_t *this,
> > +                                          grub_efi_device_path_t *device_path,
> > +                                          grub_efi_boolean_t boot_policy,
> > +                                          grub_efi_uintn_t *buffer_size,
> > +                                          void *buffer);
> > +
> > +static grub_efi_load_file2_t initrd_lf2 = {
> > +  initrd_load_file2
> > +};
> > +
> > +static grub_efi_status_t initrd_load_file2(grub_efi_load_file2_t *this,
> > +                                          grub_efi_device_path_t *device_path,
> > +                                          grub_efi_boolean_t boot_policy,
> > +                                          grub_efi_uintn_t *buffer_size,
> > +                                          void *buffer)
> > +{
> > +  grub_efi_status_t status = GRUB_EFI_SUCCESS;
> > +  grub_efi_uintn_t initrd_size;
> > +
> > +  if (!this || this != &initrd_lf2 || !buffer_size)
> > +    return GRUB_EFI_INVALID_PARAMETER;
> > +
> > +  if (device_path->type != GRUB_EFI_END_DEVICE_PATH_TYPE ||
> > +      device_path->subtype != GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)
> > +    return GRUB_EFI_NOT_FOUND;
> > +
> > +  if (boot_policy)
> > +    return GRUB_EFI_UNSUPPORTED;
> > +
> > +  initrd_size = grub_get_initrd_size (&initrd_ctx);
> > +  if (!buffer || *buffer_size < initrd_size)
> > +    {
> > +      *buffer_size = initrd_size;
> > +      return GRUB_EFI_BUFFER_TOO_SMALL;
> > +    }
> > +
> > +  grub_dprintf ("linux", "Loading initrd via LOAD_FILE2_PROTOCOL\n");
> > +
> > +  if (grub_initrd_load (&initrd_ctx, NULL, buffer))
> > +    status = GRUB_EFI_LOAD_ERROR;
> > +
> > +  grub_initrd_close (&initrd_ctx);
> > +  return status;
> > +}
> > +
> >  static grub_err_t
> >  grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
> >                  int argc, char *argv[])
> >  {
> > -  struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
> >    int initrd_size, initrd_pages;
> >    void *initrd_mem = NULL;
> > +  grub_efi_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
> > +  grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
> > +  grub_efi_boot_services_t *b;
> > +  grub_efi_status_t status;
> >
> >    if (argc == 0)
> >      {
> > @@ -274,6 +363,24 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
> >    if (grub_initrd_init (argc, argv, &initrd_ctx))
> >      goto fail;
> >
> > +  if (initrd_use_loadfile2)
> > +    {
> > +      b = grub_efi_system_table->boot_services;
> > +      status = b->install_multiple_protocol_interfaces (&initrd_lf2_handle,
> > +                                                       &load_file2_guid,
> > +                                                       &initrd_lf2,
> > +                                                       &device_path_guid,
> > +                                                       &initrd_lf2_device_path,
> > +                                                       NULL);
> > +      if (status == GRUB_EFI_OUT_OF_RESOURCES)
> > +       {
> > +         grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> > +         return grub_errno;
> > +       }
> > +      grub_dprintf ("linux", "LoadFile2 initrd loading protocol installed\n");
> > +      return GRUB_ERR_NONE;
> > +    }
> > +
> >    initrd_size = grub_get_initrd_size (&initrd_ctx);
> >    grub_dprintf ("linux", "Loading initrd\n");
> >
> > --
> > 2.17.1
> >
>
>
> --
> Regards,
> Atish

  reply	other threads:[~2020-10-26 22:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-25 13:49 [PATCH v2 0/8] linux: implement LoadFile2 initrd loading Ard Biesheuvel
2020-10-25 13:49 ` [PATCH v2 1/8] linux/arm: fix ARM Linux header layout Ard Biesheuvel
2020-11-04 12:11   ` Leif Lindholm
2020-11-04 12:19     ` Ard Biesheuvel
2020-11-04 12:31       ` Leif Lindholm
2021-03-11 16:18   ` Daniel Kiper
2020-10-25 13:49 ` [PATCH v2 2/8] loader/linux: permit NULL argument for argv[] in grub_initrd_load() Ard Biesheuvel
2020-10-25 13:49 ` [PATCH v2 3/8] efi: move MS-DOS stub out of generic PE header definition Ard Biesheuvel
2021-04-08 18:44   ` Heinrich Schuchardt
2021-04-09  6:10     ` Ard Biesheuvel
2021-04-09  6:29       ` Heinrich Schuchardt
2020-10-25 13:49 ` [PATCH v2 4/8] linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling Ard Biesheuvel
2020-10-25 13:49 ` [PATCH v2 5/8] linux/arm: account for COFF headers appearing at unexpected offsets Ard Biesheuvel
2021-04-08 18:56   ` Heinrich Schuchardt
2021-04-09  6:12     ` Ard Biesheuvel
2021-04-09  6:40       ` Heinrich Schuchardt
2021-04-09  6:58         ` Ard Biesheuvel
2020-10-25 13:49 ` [PATCH v2 6/8] efi: add definition of LoadFile2 protocol Ard Biesheuvel
2020-10-25 13:49 ` [PATCH v2 7/8] efi: implement LoadFile2 initrd loading protocol for Linux Ard Biesheuvel
2020-10-26 21:37   ` Atish Patra
2020-10-26 22:00     ` Ard Biesheuvel [this message]
2020-10-25 13:49 ` [PATCH v2 8/8] linux: ignore FDT unless we need to modify it Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMj1kXHdXQhxrLL3_N711XOv4SAbpwbBSScZYNt6Dy1L99vKOg@mail.gmail.com \
    --to=ardb@kernel.org \
    --cc=ard.biesheuvel@arm.com \
    --cc=atishp@atishpatra.org \
    --cc=daniel.kiper@oracle.com \
    --cc=grub-devel@gnu.org \
    --cc=leif@nuviainc.com \
    --cc=linux-efi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.