| Message ID | 20190419032236.8242-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | efi_loader: rework bootefi/bootmgr | expand |
On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > There are several reasons that I want to rework/refactor bootefi command > as well as bootmgr: > * Some previous commits on bootefi.c have made the code complicated > and a bit hard to understand. > > * do_bootefi_exec() would better be implemented using load_image() along > with start_image() to be aligned with UEFI interfaces. > > * Contrary to the other part, efi_selftest part of the code is unusual > in terms of loading/execution path in do_bootefi(). > > * When we will support "secure boot" in the future, EFI Boot Manager > is expected to be invoked as a standalone command without any arguments > to mitigate security surfaces. > > In this patch set, > Patch#1 to #7 are preparatory patches for patch#8. > Patch#8 is a core part of reworking. > Patch#9 is for standalone boot manager. Hello Takahiro, your patches 1-8 are now (with some modifications) in the efi-2019-07 branch. I have added some more patches. https://github.com/xypron2/u-boot/commits/efi-2019-07 Travis CI testing was ok: https://travis-ci.org/xypron2/u-boot/builds/522947698 I have tested on real hardware without error: * TinkerBoard (32 bit) boot via GRUB * Pine64 A64 LTS (64 bit) boot via iPXE and GRUB * Odroid C2 (64 bit) boot via GRUB If you are ok with the adjustments to your patch series I will create a pull request. Best regards Heinrich
Heinrich, On Tue, 23 Apr 2019 at 03:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > > There are several reasons that I want to rework/refactor bootefi command > > as well as bootmgr: > > * Some previous commits on bootefi.c have made the code complicated > > and a bit hard to understand. > > > > * do_bootefi_exec() would better be implemented using load_image() along > > with start_image() to be aligned with UEFI interfaces. > > > > * Contrary to the other part, efi_selftest part of the code is unusual > > in terms of loading/execution path in do_bootefi(). > > > > * When we will support "secure boot" in the future, EFI Boot Manager > > is expected to be invoked as a standalone command without any arguments > > to mitigate security surfaces. > > > > In this patch set, > > Patch#1 to #7 are preparatory patches for patch#8. > > Patch#8 is a core part of reworking. > > Patch#9 is for standalone boot manager. > > Hello Takahiro, > > your patches 1-8 are now (with some modifications) in the efi-2019-07 > branch. I have added some more patches. > > https://github.com/xypron2/u-boot/commits/efi-2019-07 > > Travis CI testing was ok: > https://travis-ci.org/xypron2/u-boot/builds/522947698 > > I have tested on real hardware without error: > * TinkerBoard (32 bit) boot via GRUB > * Pine64 A64 LTS (64 bit) boot via iPXE and GRUB > * Odroid C2 (64 bit) boot via GRUB > > If you are ok with the adjustments to your patch series I will create a > pull request. Thank you for the merge. One question: how did you fix a grub error that you reported before? -Takahiro Akashi > Best regards > > Heinrich
On 4/23/19 2:24 AM, AKASHI, Takahiro wrote: > Heinrich, > > On Tue, 23 Apr 2019 at 03:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 4/19/19 5:22 AM, AKASHI Takahiro wrote: >>> There are several reasons that I want to rework/refactor bootefi command >>> as well as bootmgr: >>> * Some previous commits on bootefi.c have made the code complicated >>> and a bit hard to understand. >>> >>> * do_bootefi_exec() would better be implemented using load_image() along >>> with start_image() to be aligned with UEFI interfaces. >>> >>> * Contrary to the other part, efi_selftest part of the code is unusual >>> in terms of loading/execution path in do_bootefi(). >>> >>> * When we will support "secure boot" in the future, EFI Boot Manager >>> is expected to be invoked as a standalone command without any arguments >>> to mitigate security surfaces. >>> >>> In this patch set, >>> Patch#1 to #7 are preparatory patches for patch#8. >>> Patch#8 is a core part of reworking. >>> Patch#9 is for standalone boot manager. >> >> Hello Takahiro, >> >> your patches 1-8 are now (with some modifications) in the efi-2019-07 >> branch. I have added some more patches. >> >> https://github.com/xypron2/u-boot/commits/efi-2019-07 >> >> Travis CI testing was ok: >> https://travis-ci.org/xypron2/u-boot/builds/522947698 >> >> I have tested on real hardware without error: >> * TinkerBoard (32 bit) boot via GRUB >> * Pine64 A64 LTS (64 bit) boot via iPXE and GRUB >> * Odroid C2 (64 bit) boot via GRUB >> >> If you are ok with the adjustments to your patch series I will create a >> pull request. > > Thank you for the merge. > One question: how did you fix a grub error that you reported before? I fixed the issue that iPXE could not find a chained script via efi_loader: correctly split device path of loaded image https://lists.denx.de/pipermail/u-boot/2019-April/365907.html Furthermore I fixed a bug in Debian's QEMU by applying Alex's https://github.com/qemu/qemu/commit/2d2a4549cc29850aab891495685a7b31f5254b12 Best regards Heinrich > > -Takahiro Akashi > > >> Best regards >> >> Heinrich >
Okay. Once your pull is done, I will submit "efi variable" patch, which is more experimental than bootefi/bootmgr rework. -Takahiro Akashi On Tue, 23 Apr 2019 at 13:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 4/23/19 2:24 AM, AKASHI, Takahiro wrote: > > Heinrich, > > > > On Tue, 23 Apr 2019 at 03:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > >>> There are several reasons that I want to rework/refactor bootefi command > >>> as well as bootmgr: > >>> * Some previous commits on bootefi.c have made the code complicated > >>> and a bit hard to understand. > >>> > >>> * do_bootefi_exec() would better be implemented using load_image() along > >>> with start_image() to be aligned with UEFI interfaces. > >>> > >>> * Contrary to the other part, efi_selftest part of the code is unusual > >>> in terms of loading/execution path in do_bootefi(). > >>> > >>> * When we will support "secure boot" in the future, EFI Boot Manager > >>> is expected to be invoked as a standalone command without any arguments > >>> to mitigate security surfaces. > >>> > >>> In this patch set, > >>> Patch#1 to #7 are preparatory patches for patch#8. > >>> Patch#8 is a core part of reworking. > >>> Patch#9 is for standalone boot manager. > >> > >> Hello Takahiro, > >> > >> your patches 1-8 are now (with some modifications) in the efi-2019-07 > >> branch. I have added some more patches. > >> > >> https://github.com/xypron2/u-boot/commits/efi-2019-07 > >> > >> Travis CI testing was ok: > >> https://travis-ci.org/xypron2/u-boot/builds/522947698 > >> > >> I have tested on real hardware without error: > >> * TinkerBoard (32 bit) boot via GRUB > >> * Pine64 A64 LTS (64 bit) boot via iPXE and GRUB > >> * Odroid C2 (64 bit) boot via GRUB > >> > >> If you are ok with the adjustments to your patch series I will create a > >> pull request. > > > > Thank you for the merge. > > One question: how did you fix a grub error that you reported before? > > I fixed the issue that iPXE could not find a chained script via > efi_loader: correctly split device path of loaded image > https://lists.denx.de/pipermail/u-boot/2019-April/365907.html > > Furthermore I fixed a bug in Debian's QEMU by applying Alex's > https://github.com/qemu/qemu/commit/2d2a4549cc29850aab891495685a7b31f5254b12 > > Best regards > > Heinrich > > > > > -Takahiro Akashi > > > > > >> Best regards > >> > >> Heinrich > > >
On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > There are several reasons that I want to rework/refactor bootefi command > as well as bootmgr: > * Some previous commits on bootefi.c have made the code complicated > and a bit hard to understand. > > * do_bootefi_exec() would better be implemented using load_image() along > with start_image() to be aligned with UEFI interfaces. > > * Contrary to the other part, efi_selftest part of the code is unusual > in terms of loading/execution path in do_bootefi(). > > * When we will support "secure boot" in the future, EFI Boot Manager > is expected to be invoked as a standalone command without any arguments > to mitigate security surfaces. > > In this patch set, > Patch#1 to #7 are preparatory patches for patch#8. > Patch#8 is a core part of reworking. > Patch#9 is for standalone boot manager. > > # Please note that some patches, say patch#2 and #3, can be combined into one > # but I intentionally keep them separated to clarify my intentions of changes. > > Issues: > * It would be better off to change the semantics of efi_dp_from_name(). > no chance to free loaded_image_info->load_options. (see patch #8) > > -Takahiro Akashi Hello Takahiro, with the `efidebug boot add` command we can define load options for the BootXXXX variables. But in do_efibootmgr() we call do_bootefi_exec() which calls set_load_options() and passes the value of environment variable bootargs as load options or if the variable is not set an empty string. Here is an example console output: => setenv bootargs This is a value from bootargs => efidebug boot add 0000 hello scsi 0:1 helloworld.efi 'This is a value from efidebug' => efidebug boot order 0000 => bootefi bootmgr Booting: hello Hello, world! Running on UEFI 2.8 Have SMBIOS table Have device tree Load options: This is a value from bootargs ## Application terminated, r = 0 Now the same after deleting variable bootargs: => setenv bootargs => bootefi bootmgr Booting: hello Hello, world! Running on UEFI 2.8 Have SMBIOS table Have device tree Load options: <none> ## Application terminated, r = 0 => What behavior would you expect: a) if the boot option has a load options value, b) if the boot option has no load options value? One solution would be to define that bootargs is always ignored if the boot manager is used. Best regards Heinrich
Heinrich, On Fri, Jan 03, 2020 at 01:17:05AM +0100, Heinrich Schuchardt wrote: > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > >There are several reasons that I want to rework/refactor bootefi command > >as well as bootmgr: > >* Some previous commits on bootefi.c have made the code complicated > > and a bit hard to understand. > > > >* do_bootefi_exec() would better be implemented using load_image() along > > with start_image() to be aligned with UEFI interfaces. > > > >* Contrary to the other part, efi_selftest part of the code is unusual > > in terms of loading/execution path in do_bootefi(). > > > >* When we will support "secure boot" in the future, EFI Boot Manager > > is expected to be invoked as a standalone command without any arguments > > to mitigate security surfaces. > > > >In this patch set, > >Patch#1 to #7 are preparatory patches for patch#8. > >Patch#8 is a core part of reworking. > >Patch#9 is for standalone boot manager. > > > ># Please note that some patches, say patch#2 and #3, can be combined into one > ># but I intentionally keep them separated to clarify my intentions of changes. > > > >Issues: > >* It would be better off to change the semantics of efi_dp_from_name(). > >no chance to free loaded_image_info->load_options. (see patch #8) > > > >-Takahiro Akashi > > Hello Takahiro, > > with the `efidebug boot add` command we can define load options for the > BootXXXX variables. > > But in do_efibootmgr() we call do_bootefi_exec() which calls > set_load_options() and passes the value of environment variable bootargs > as load options or if the variable is not set an empty string. > > Here is an example console output: > > => setenv bootargs This is a value from bootargs > => efidebug boot add 0000 hello scsi 0:1 helloworld.efi 'This is a value > from efidebug' > => efidebug boot order 0000 > => bootefi bootmgr > Booting: hello > Hello, world! > Running on UEFI 2.8 > Have SMBIOS table > Have device tree > Load options: This is a value from bootargs > ## Application terminated, r = 0 > > Now the same after deleting variable bootargs: > > => setenv bootargs > => bootefi bootmgr > Booting: hello > Hello, world! > Running on UEFI 2.8 > Have SMBIOS table > Have device tree > Load options: <none> > ## Application terminated, r = 0 > => Yeah, this is not what I intended. > What behavior would you expect: > > a) if the boot option has a load options value, > b) if the boot option has no load options value? > > One solution would be to define that bootargs is always ignored if the > boot manager is used. I agree. Basically, "bootargs" is only for "bootefi <addr>" command, while "BootXXXX" should work in the exact same way as the UEFI specification defines. Thanks, -Takahiro Akashi > Best regards > > Heinrich