Hi! Thanks for looking at this!

Mate Kukri писал(а) 02.11.2024 12:47:
> I think this is not really compatible with shim signing, which is how
> UEFI GRUB is usually used in the Linux world.
> Are you trying to get an MS UEFI CA signed dtbloader and use that with GRUB?
> 

FWIW at a first stage I'd want to at least have this work for SB-off
case, but more generally I'd hope such command would be able to work
with shim/distro signed drivers as well.

> The planned shim loader protocol changes will break that when used
> with shim, because all LoadImage calls would go through shim's hook at
> that point.

Hm, I had an impression that in that case the shim's LoadImage would
check the newly loaded .efi against it's cert, so when SB is enabled
and shim is used, i.e. dtbloader.efi would have to be signed with
distro cert, which is "desired", do I understand that correctly?

Or do you mean that, currently, even if the shim is installed, the
LoadImage is not overriden with the custom one and it would still try
to verify against the "system" db, which would be fixed by the planned
changes?

> Can this dtb selection functionality not be implemented in GRUB instead?

My goal here is to not just provide dtbloader.efi support but more
generally support for EFI drivers, since it seems to me that if I myself
have /two/ usecases already, we may see much more in the future. A quick
theoretical example I can think of is, if Asahi people finally push the
uefi PSCI conduit into linux, they could choose to implement their PSCI
as a efi driver, loaded by this mechanism.

(Speaking with people, I actually feel there is more interest for this
in the context of slbounce.efi and not dtbloader.efi, tho for that one
secureboot would probably not be relevant, that thing will most likely
remain a user-choice retrofit, which would just be much more convenient,
if can be hidden behind a grub boot option)

For DTB loading specifically, I believe it was decided[1] that GRUB
must not load the dtb files itself when SB is on, as it's currently not
possible to verify those. It's otherwise possible to implement the
detection in GRUB script (I believe that's what ubuntu currently uses
for the experimental arm images, disabling SB), however then the solution
is GRUB specific and won't help for sd-boot, UKI and other things.

I'd appreciate to hear your thoughts on how we can move forward and
implement driver loading for both SB/shim and non-sb cases.

Nikita

[1] https://lists.gnu.org/archive/html/grub-devel/2019-05/msg00132.html

> 
> On Tue, Sep 24, 2024 at 6:31 PM Nikita Travkin via Grub-devel
> <grub-devel@gnu.org> wrote:
>>
>> Sometimes it's useful to load EFI drivers. Since loading a driver is
>> the same as starting it and UEFI handles cleanup after on it's own, we
>> can piggy back on existing chainloader command and just start the image
>> immediately instead of defering to "boot". Conveniently this also means
>> that grub can also run plain efi applications with the new command,
>> though the command is still called "efidriver" since it's the primary
>> intended usecase.
>>
>> Refactor chainloader to split up image loading and starting from the
>> command and loader functions and add a new "efidriver" command that
>> executes those steps immediately.
>>
>> Signed-off-by: Nikita Travkin <nik...@trvn.ru>
>> ---
>> This patch adds EFI driver loading support to grub.
>>
>> Similar feature already exists in systemd-boot in a form of a dedicated
>> directory that it loads drivers from.
>>
>> A few specific examples where it can be necessary:
>>
>> Loading devicetree
>> ------------------
>>
>> Linux can't rely on ACPI on most currently existing
>> Windows-on-Snapdragon devices and instead makes use of devicetree. This
>> means that something has to load said devicetree and provide it to the
>> kerne. GRUB has a "devicetree" command for that but it has a couple of
>> shortcomings:
>>
>>  - One has to know beforehand what dtb they want to load;
>>  - This command doesn't verify the dtb and can't be used with UEFI
>>    secure-boot.
>>
>> Instead dtb could be loaded by a dedicated efi driver such as
>> dtbloader [1]. Such driver could dynamically detect the device and load
>> appropriate dtb and since it's a normal efi driver, it's verified with
>> secure-boot, and itself can implement some security model to safely load
>> dtbs.
>>
>> This would allow distro image makers to create generic os/installer
>> images instead of adding device-specific hacks or asking the users for
>> manual action to load the dtb.
>>
>> Peforming pre-boot firmware hacks
>> ---------------------------------
>>
>> Most currently shipping WoS devices boot UEFI (and thus Linux) in EL1
>> which means Linux can't use KVM and virtualization. Windows has a custom
>> api to switch to EL2. To work around that, custom efi driver,
>> slbounce[2] can be used. This driver hooks into UEFI boot services and
>> performs EL1->EL2 transition upon EBS.
>>
>> Adding "efidriver" command to GRUB would make it easy to "dual boot"
>> EL1 and EL2 modes for users since at the moment Linux can't use some
>> hardware while running in EL2.
>>
>> [1] https://github.com/TravMurav/dtbloader
>> [2] https://github.com/TravMurav/slbounce
>> ---
>> Changes in v2:
>> - Align the changes with correct code style (Avnish)
>> - Suppress EFI_ABORTED when loading efi drivers
>> - Link to v1: 
>> https://lore.kernel.org/r/20240922-cmd-efidriver-v1-1-750b77bfc...@trvn.ru
>> ---
>>  grub-core/loader/efi/chainloader.c | 73 
>> +++++++++++++++++++++++++++++++-------
>>  1 file changed, 61 insertions(+), 12 deletions(-)
>>
>> diff --git a/grub-core/loader/efi/chainloader.c 
>> b/grub-core/loader/efi/chainloader.c
>> index 869307bf3..77185e3aa 100644
>> --- a/grub-core/loader/efi/chainloader.c
>> +++ b/grub-core/loader/efi/chainloader.c
>> @@ -19,6 +19,7 @@
>>
>>  /* TODO: support load options.  */
>>
>> +#include <stdbool.h>
>>  #include <grub/loader.h>
>>  #include <grub/file.h>
>>  #include <grub/err.h>
>> @@ -64,9 +65,8 @@ grub_chainloader_unload (void *context)
>>  }
>>
>>  static grub_err_t
>> -grub_chainloader_boot (void *context)
>> +start_efi_image (grub_efi_handle_t image_handle, bool driver)
>>  {
>> -  grub_efi_handle_t image_handle = (grub_efi_handle_t) context;
>>    grub_efi_boot_services_t *b;
>>    grub_efi_status_t status;
>>    grub_efi_uintn_t exit_data_size;
>> @@ -74,7 +74,9 @@ grub_chainloader_boot (void *context)
>>
>>    b = grub_efi_system_table->boot_services;
>>    status = b->start_image (image_handle, &exit_data_size, &exit_data);
>> -  if (status != GRUB_EFI_SUCCESS)
>> +  /* EFI_ABORTED is a normal return code for "initializing" drivers
>> +     which indicates that "the driver did what it wanted and unloaded" */
>> +  if (status != GRUB_EFI_SUCCESS && !(driver && status == GRUB_EFI_ABORTED))
>>      {
>>        if (exit_data)
>>         {
>> @@ -97,9 +99,20 @@ grub_chainloader_boot (void *context)
>>    if (exit_data)
>>      b->free_pool (exit_data);
>>
>> +  return grub_errno;
>> +}
>> +
>> +static grub_err_t
>> +grub_chainloader_boot (void *context)
>> +{
>> +  grub_efi_handle_t image_handle = (grub_efi_handle_t) context;
>> +  grub_err_t ret;
>> +
>> +  ret = start_efi_image (image_handle, false);
>> +
>>    grub_loader_unset ();
>>
>> -  return grub_errno;
>> +  return ret;
>>  }
>>
>>  static grub_err_t
>> @@ -209,8 +222,7 @@ make_file_path (grub_efi_device_path_t *dp, const char 
>> *filename)
>>  }
>>
>>  static grub_err_t
>> -grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
>> -                     int argc, char *argv[])
>> +load_efi_image (int argc, char *argv[], grub_efi_handle_t *image_ret)
>>  {
>>    grub_file_t file = 0;
>>    grub_ssize_t size;
>> @@ -400,8 +412,8 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
>> ((unused)),
>>    b->free_pages (address, pages);
>>    grub_free (file_path);
>>
>> -  grub_loader_set_ex (grub_chainloader_boot, grub_chainloader_unload, 
>> image_handle, 0);
>> -  return 0;
>> +  *image_ret = image_handle;
>> +  return GRUB_ERR_NONE;
>>
>>   fail:
>>
>> @@ -425,16 +437,53 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
>> ((unused)),
>>    return grub_errno;
>>  }
>>
>> -static grub_command_t cmd;
>> +static grub_err_t
>> +grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
>> +                     int argc, char *argv[])
>> +{
>> +  grub_efi_handle_t image_handle = NULL;
>> +  grub_err_t ret;
>> +
>> +  ret = load_efi_image (argc, argv, &image_handle);
>> +  if (ret != GRUB_ERR_NONE)
>> +    return ret;
>> +
>> +  grub_loader_set_ex (grub_chainloader_boot, grub_chainloader_unload, 
>> image_handle, 0);
>> +  return GRUB_ERR_NONE;
>> +}
>> +
>> +static grub_err_t
>> +grub_cmd_efidriver (grub_command_t cmd __attribute__ ((unused)),
>> +                   int argc, char *argv[])
>> +{
>> +  grub_efi_handle_t image_handle = NULL;
>> +  grub_err_t ret;
>> +
>> +  ret = load_efi_image (argc, argv, &image_handle);
>> +  if (ret != GRUB_ERR_NONE)
>> +    return ret;
>> +
>> +  ret = start_efi_image (image_handle, true);
>> +
>> +  grub_dl_unref (my_mod);
>> +
>> +  return ret;
>> +}
>> +
>> +static grub_command_t cmd_chainloader;
>> +static grub_command_t cmd_efidriver;
>>
>>  GRUB_MOD_INIT(chainloader)
>>  {
>> -  cmd = grub_register_command ("chainloader", grub_cmd_chainloader,
>> -                              0, N_("Load another boot loader."));
>> +  cmd_chainloader = grub_register_command ("chainloader", 
>> grub_cmd_chainloader,
>> +                                          0, N_("Load another boot 
>> loader."));
>> +  cmd_efidriver = grub_register_command ("efidriver", grub_cmd_efidriver,
>> +                                        0, N_("Load a efi driver."));
>>    my_mod = mod;
>>  }
>>
>>  GRUB_MOD_FINI(chainloader)
>>  {
>> -  grub_unregister_command (cmd);
>> +  grub_unregister_command (cmd_chainloader);
>> +  grub_unregister_command (cmd_efidriver);
>>  }
>>
>> ---
>> base-commit: 9c34d56c2dafcd2737db0e3e49df63bce4d8b504
>> change-id: 20240922-cmd-efidriver-5b644a784851
>>
>> Best regards,
>> --
>> Nikita Travkin <nik...@trvn.ru>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to