avnish писал(а) 24.09.2024 14:46:
> On 2024-09-22 21:30, grub-devel-requ...@gnu.org wrote:
>>
>> Message: 1
>> Date: Sun, 22 Sep 2024 13:05:07 +0500
>> From: Nikita Travkin <nik...@trvn.ru>
>> To: grub-devel@gnu.org
>> Cc: Nikita Travkin <nik...@trvn.ru>
>> Subject: [PATCH] loader/efi/chainloader: Add efidriver command
>> Message-ID: <20240922-cmd-efidriver-v1-1-750b77bfc...@trvn.ru>
>> Content-Type: text/plain; charset="utf-8"
>>
>> 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
>> ---
>> grub-core/loader/efi/chainloader.c | 68
>> ++++++++++++++++++++++++++++++++------
>> 1 file changed, 57 insertions(+), 11 deletions(-)
>>
>> diff --git a/grub-core/loader/efi/chainloader.c
>> b/grub-core/loader/efi/chainloader.c
>> index 869307bf3..17b9c671e 100644
>> --- a/grub-core/loader/efi/chainloader.c
>> +++ b/grub-core/loader/efi/chainloader.c
>> @@ -64,9 +64,8 @@ grub_chainloader_unload (void *context)
>> }
>>
>> static grub_err_t
>> -grub_chainloader_boot (void *context)
>> +start_efi_image (grub_efi_handle_t image_handle)
>> {
>> - 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;
>> @@ -97,9 +96,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);
>
> Hi Nikita,
> It seems a space is missing before '('
Oh, you're right, I somehow completely missed different code style...
I will fix all of those as well as indents (probably happened due to
different tab settings in my editor) and send a v2.
Thanks for your review!
Nikita
>
>> +
>> grub_loader_unset ();
>>
>> - return grub_errno;
>> + return ret;
>> }
>>
>> static grub_err_t
>> @@ -209,8 +219,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 +409,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 +434,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[])
>
> alignment seems off.
> Something like this:
> 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);
>
> Ditto. Missing space before '('
>
>> + 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);
>
> Ditto. Missing space before '('
>
>> + if (ret != GRUB_ERR_NONE)
>> + return ret;
>> +
>> + ret = start_efi_image(image_handle);
>
> Ditto. Missing space before '('
>
>> +
>> + 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."));
>
> alignment seems off for above two lines. Same as mentioned earlier.
>
> Thank you!
>
> Regards,
> Avnish Chouhan
>
>> 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>
>>
>>
>>
>>
>> ------------------------------
>>
>> Subject: Digest Footer
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>> ------------------------------
>>
>> End of Grub-devel Digest, Vol 247, Issue 74
>> *******************************************
>
> _______________________________________________
> 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
