Re: [PATCH 13/15] efi: Provide wrappers for load_image, start_image, unload_image

[Vladimir 'phcoder' Serbinenko]

WDYT of moving this logic to chainloader? Yes, it will make linux
depend on chainloader but I think given its current logic it's a
honest dependency


On Fri, May 24, 2024 at 2:06 PM Mate Kukri <mate.kukri@canonical.com> wrote:
>
> From: Julian Andres Klode <julian.klode@canonical.com>
>
> These can be used to register a different implementation later,
> for example, when shim provides a protocol with those functions.
>
> Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
> ---
>  grub-core/kern/efi/efi.c           | 57 ++++++++++++++++++++++++++++++
>  grub-core/loader/efi/chainloader.c | 13 +++----
>  grub-core/loader/efi/linux.c       | 12 +++----
>  include/grub/efi/efi.h             | 37 +++++++++++++++++++
>  4 files changed, 104 insertions(+), 15 deletions(-)
>
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index f34c1ef54..3d3b884bb 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -1085,3 +1085,60 @@ grub_efi_status_to_err (grub_efi_status_t status)
>
>    return err;
>  }
> +
> +static const grub_efi_loader_t *override_loader = NULL;
> +
> +grub_err_t
> +grub_efi_register_loader (const grub_efi_loader_t *loader)
> +{
> +  if (override_loader != NULL)
> +    return grub_error (GRUB_ERR_BUG, "trying to register different loader");
> +  override_loader = loader;
> +  return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_efi_unregister_loader (const grub_efi_loader_t *loader)
> +{
> +  if (loader != override_loader)
> +    return grub_error (GRUB_ERR_BUG, "trying to unregister different 
> loader");
> +
> +  override_loader = NULL;
> +  return GRUB_ERR_NONE;
> +}
> +
> +grub_efi_status_t
> +grub_efi_load_image (grub_efi_boolean_t boot_policy,
> +                    grub_efi_handle_t parent_image_handle,
> +                    grub_efi_device_path_t *file_path, void *source_buffer,
> +                    grub_efi_uintn_t source_size,
> +                    grub_efi_handle_t *image_handle)
> +{
> +  if (override_loader != NULL)
> +    return override_loader->load_image (boot_policy, parent_image_handle,
> +                                       file_path, source_buffer, source_size,
> +                                       image_handle);
> +  return grub_efi_system_table->boot_services->load_image (
> +      boot_policy, parent_image_handle, file_path, source_buffer, 
> source_size,
> +      image_handle);
> +}
> +
> +grub_efi_status_t
> +grub_efi_start_image (grub_efi_handle_t image_handle,
> +                     grub_efi_uintn_t *exit_data_size,
> +                     grub_efi_char16_t **exit_data)
> +{
> +  if (override_loader != NULL)
> +    return override_loader->start_image (image_handle, exit_data_size,
> +                                        exit_data);
> +  return grub_efi_system_table->boot_services->start_image (
> +      image_handle, exit_data_size, exit_data);
> +}
> +
> +grub_efi_status_t
> +grub_efi_unload_image (grub_efi_handle_t image_handle)
> +{
> +  if (override_loader != NULL)
> +    return override_loader->unload_image (image_handle);
> +  return grub_efi_system_table->boot_services->unload_image (image_handle);
> +}
> diff --git a/grub-core/loader/efi/chainloader.c 
> b/grub-core/loader/efi/chainloader.c
> index 1de98f783..eb833b678 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -50,14 +50,12 @@ grub_chainloader_unload (void *context)
>  {
>    grub_efi_handle_t image_handle = (grub_efi_handle_t) context;
>    grub_efi_loaded_image_t *loaded_image;
> -  grub_efi_boot_services_t *b;
>
>    loaded_image = grub_efi_get_loaded_image (image_handle);
>    if (loaded_image != NULL)
>      grub_free (loaded_image->load_options);
>
> -  b = grub_efi_system_table->boot_services;
> -  b->unload_image (image_handle);
> +  grub_efi_unload_image (image_handle);
>
>    grub_dl_unref (my_mod);
>    return GRUB_ERR_NONE;
> @@ -73,7 +71,7 @@ grub_chainloader_boot (void *context)
>    grub_efi_char16_t *exit_data = NULL;
>
>    b = grub_efi_system_table->boot_services;
> -  status = b->start_image (image_handle, &exit_data_size, &exit_data);
> +  status = grub_efi_start_image (image_handle, &exit_data_size, &exit_data);
>    if (status != GRUB_EFI_SUCCESS)
>      {
>        if (exit_data)
> @@ -343,9 +341,8 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>      }
>  #endif
>
> -  status = b->load_image (0, grub_efi_image_handle, file_path,
> -                         boot_image, size,
> -                         &image_handle);
> +  status = grub_efi_load_image (0, grub_efi_image_handle, file_path,
> +                               boot_image, size, &image_handle);
>    if (status != GRUB_EFI_SUCCESS)
>      {
>        if (status == GRUB_EFI_OUT_OF_RESOURCES)
> @@ -422,7 +419,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>      b->free_pages (address, pages);
>
>    if (image_handle != NULL)
> -    b->unload_image (image_handle);
> +    grub_efi_unload_image (image_handle);
>
>    grub_dl_unref (my_mod);
>
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index bfbd95aee..58be3c9f8 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -187,7 +187,6 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
>  {
>    grub_efi_memory_mapped_device_path_t *mempath;
>    grub_efi_handle_t image_handle;
> -  grub_efi_boot_services_t *b;
>    grub_efi_status_t status;
>    grub_efi_loaded_image_t *loaded_image;
>    int len;
> @@ -207,10 +206,9 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
>    mempath[1].header.subtype = GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE;
>    mempath[1].header.length = sizeof (grub_efi_device_path_t);
>
> -  b = grub_efi_system_table->boot_services;
> -  status = b->load_image (0, grub_efi_image_handle,
> -                         (grub_efi_device_path_t *) mempath,
> -                         (void *) addr, size, &image_handle);
> +  status = grub_efi_load_image (0, grub_efi_image_handle,
> +                               (grub_efi_device_path_t *)mempath,
> +                               (void *)addr, size, &image_handle);
>    if (status != GRUB_EFI_SUCCESS)
>      return grub_error (GRUB_ERR_BAD_OS, "cannot load image");
>
> @@ -235,14 +233,14 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
>                             (grub_uint8_t *) args, len, NULL);
>
>    grub_dprintf ("linux", "starting image %p\n", image_handle);
> -  status = b->start_image (image_handle, 0, NULL);
> +  status = grub_efi_start_image (image_handle, 0, NULL);
>
>    /* When successful, not reached */
>    grub_error (GRUB_ERR_BAD_OS, "start_image() returned 0x%" 
> PRIxGRUB_EFI_UINTN_T, status);
>    grub_efi_free_pages ((grub_addr_t) loaded_image->load_options,
>                        GRUB_EFI_BYTES_TO_PAGES 
> (loaded_image->load_options_size));
>  unload:
> -  b->unload_image (image_handle);
> +  grub_efi_unload_image (image_handle);
>
>    return grub_errno;
>  }
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 194adc1f7..6b517a1ea 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -132,6 +132,43 @@ grub_err_t 
> grub_arch_efi_linux_load_image_header(grub_file_t file,
>  grub_err_t grub_arch_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
>                                             char *args);
>
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_load_image) (grub_efi_boolean_t boot_policy,
> +                                  grub_efi_handle_t parent_image_handle,
> +                                  grub_efi_device_path_t *file_path,
> +                                  void *source_buffer, grub_efi_uintn_t 
> source_size,
> +                                  grub_efi_handle_t *image_handle);
> +
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_start_image) (grub_efi_handle_t image_handle,
> +                                   grub_efi_uintn_t *exit_data_size,
> +                                   grub_efi_char16_t **exit_data);
> +
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_unload_image) (grub_efi_handle_t image_handle);
> +
> +typedef struct grub_efi_loader
> +{
> +  grub_efi_status_t (__grub_efi_api *load_image) (grub_efi_boolean_t 
> boot_policy,
> +                                  grub_efi_handle_t parent_image_handle,
> +                                  grub_efi_device_path_t *file_path,
> +                                  void *source_buffer,
> +                                  grub_efi_uintn_t source_size,
> +                                  grub_efi_handle_t *image_handle);
> +
> +  grub_efi_status_t (__grub_efi_api *start_image) (grub_efi_handle_t 
> image_handle,
> +                                   grub_efi_uintn_t *exit_data_size,
> +                                   grub_efi_char16_t **exit_data);
> +
> +  grub_efi_status_t (__grub_efi_api *unload_image) (grub_efi_handle_t 
> image_handle);
> +} grub_efi_loader_t;
> +
> +grub_err_t
> +EXPORT_FUNC (grub_efi_register_loader) (const grub_efi_loader_t *loader);
> +
> +grub_err_t
> +EXPORT_FUNC (grub_efi_unregister_loader) (const grub_efi_loader_t *loader);
> +
>  grub_addr_t grub_efi_section_addr (const char *section);
>
>  void grub_efi_mm_init (void);
> --
> 2.39.2
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



--
Regards
Vladimir 'phcoder' Serbinenko