Re: [PATCH 1/4] tests/util/grub-shell-luks-tester: Add missing line to create RET variable in cleanup
Hi, please ignore my proposal about "set -e". My assessment of "set -e" in tests/util/grub-shell-luks-tester.in is obviously wrong, probably because "set -e" is not inherited by sub-shells. The line #! @BUILD_SHEBANG@ -e enables script abort on command error, but causes reply errexit on only with echo set -o | grep errexit but not with echo "grub-shell-luks-tester.in: $(set -o | grep errexit)" >>/tmp/grub-shell-luks-tester.errexit.log Sorry for the noise. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] loader/efi/chainloader: Add efidriver command (Nikita Travkin)
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
To: grub-devel@gnu.org
Cc: Nikita Travkin
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
---
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 '('
+
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[])
[PATCH v2] loader/efi/chainloader: Add efidriver command
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
---
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
#include
#include
#include
@@ -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 @
Re: [PATCH] loader/efi/chainloader: Add efidriver command (Nikita Travkin)
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
>> To: grub-devel@gnu.org
>> Cc: Nikita Travkin
>> 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
>> ---
>> 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, p
