mbox series

[0/8,v2] Change logic of EFI LoadFile2 protocol for initrd loading

Message ID 20201230150722.154663-1-ilias.apalodimas@linaro.org
Headers show
Series Change logic of EFI LoadFile2 protocol for initrd loading | expand

Message

Ilias Apalodimas Dec. 30, 2020, 3:07 p.m. UTC
Hi!

This is v2 of [1].

There's been a couple of changes regarding where we install the protocol. 
The initial patchset was completely disregarding BootNext, so that's taken 
into account now and we can use the new feature. 
This brought a few changes on the selftests as well, since we now use the 
loaded image handle to install the protocol, as a consequence the custom 
handle in the tests is now uninstalled during the test .teardown(), but 
the overall approach remains identical.

Changes since v1:
- Use efi_create_indexed_name() instead of open coding it. An extra
  patch has been applied to efi_create_indexed_name() to ensure the
  destination buffer is big enough for our variable name.
- Install the protocol during do_efibootmgr() on the loaded image
  handle and adjust selftests accordingly. The protocol will be 
  uninstalled if efi_exit() is called from and EFI app now.
- Make sure strings are null terminated in efi_helper.c
- Added documentation in uefi.rst describing the new feature.

[1] https://lists.denx.de/pipermail/u-boot/2020-December/436080.html

Ilias Apalodimas (8):
  efi_loader: Remove unused headers from efi_load_initrd.c
  efi_loader: Remove unconditional installation of file2 protocol for
    initrd
  efi_loader: Add size checks to efi_create_indexed_name()
  efi_loader: Introduce helper functions for EFI
  efi_loader: bootmgr: Use get_var from efi_helper file
  efi_loader: Replace config option with EFI variable for initrd loading
  efi_selftest: Modify self-tests for initrd loading via Loadfile2
  doc: uefi: Add instruction for initrd loading

 cmd/bootefi.c                               |  13 ++
 doc/uefi/uefi.rst                           |  15 ++
 include/efi_helper.h                        |  28 +++
 include/efi_loader.h                        |   5 +-
 lib/efi_loader/Kconfig                      |  13 +-
 lib/efi_loader/Makefile                     |   2 +-
 lib/efi_loader/efi_bootmgr.c                |  37 +---
 lib/efi_loader/efi_helper.c                 | 180 ++++++++++++++++++++
 lib/efi_loader/efi_load_initrd.c            | 101 ++++-------
 lib/efi_loader/efi_setup.c                  |   5 -
 lib/efi_loader/efi_string.c                 |  10 +-
 lib/efi_selftest/efi_selftest_load_initrd.c | 100 ++++++++++-
 test/unicode_ut.c                           |   2 +-
 13 files changed, 384 insertions(+), 127 deletions(-)
 create mode 100644 include/efi_helper.h
 create mode 100644 lib/efi_loader/efi_helper.c

-- 
2.30.0

Comments

Heinrich Schuchardt Dec. 30, 2020, 8:44 p.m. UTC | #1
On 12/30/20 4:07 PM, Ilias Apalodimas wrote:
> Hi!

>

> This is v2 of [1].

>

> There's been a couple of changes regarding where we install the protocol.

> The initial patchset was completely disregarding BootNext, so that's taken

> into account now and we can use the new feature.

> This brought a few changes on the selftests as well, since we now use the

> loaded image handle to install the protocol, as a consequence the custom

> handle in the tests is now uninstalled during the test .teardown(), but

> the overall approach remains identical.


Boot#### contains a device path.

Why should Initrd#### contain something U-Boot specific and not a device
path?

Think of a Linux distribution. When updating the kernel it will have to
write Boot#### and Initrd####. It can determine the current boot device
via BootCurrent and Boot####. Next it can add the new file paths for
Boot#### and Initrd#### relative to the same device.

Best regards

Heinrich

>

> Changes since v1:

> - Use efi_create_indexed_name() instead of open coding it. An extra

>    patch has been applied to efi_create_indexed_name() to ensure the

>    destination buffer is big enough for our variable name.

> - Install the protocol during do_efibootmgr() on the loaded image

>    handle and adjust selftests accordingly. The protocol will be

>    uninstalled if efi_exit() is called from and EFI app now.

> - Make sure strings are null terminated in efi_helper.c

> - Added documentation in uefi.rst describing the new feature.

>

> [1] https://lists.denx.de/pipermail/u-boot/2020-December/436080.html

>

> Ilias Apalodimas (8):

>    efi_loader: Remove unused headers from efi_load_initrd.c

>    efi_loader: Remove unconditional installation of file2 protocol for

>      initrd

>    efi_loader: Add size checks to efi_create_indexed_name()

>    efi_loader: Introduce helper functions for EFI

>    efi_loader: bootmgr: Use get_var from efi_helper file

>    efi_loader: Replace config option with EFI variable for initrd loading

>    efi_selftest: Modify self-tests for initrd loading via Loadfile2

>    doc: uefi: Add instruction for initrd loading

>

>   cmd/bootefi.c                               |  13 ++

>   doc/uefi/uefi.rst                           |  15 ++

>   include/efi_helper.h                        |  28 +++

>   include/efi_loader.h                        |   5 +-

>   lib/efi_loader/Kconfig                      |  13 +-

>   lib/efi_loader/Makefile                     |   2 +-

>   lib/efi_loader/efi_bootmgr.c                |  37 +---

>   lib/efi_loader/efi_helper.c                 | 180 ++++++++++++++++++++

>   lib/efi_loader/efi_load_initrd.c            | 101 ++++-------

>   lib/efi_loader/efi_setup.c                  |   5 -

>   lib/efi_loader/efi_string.c                 |  10 +-

>   lib/efi_selftest/efi_selftest_load_initrd.c | 100 ++++++++++-

>   test/unicode_ut.c                           |   2 +-

>   13 files changed, 384 insertions(+), 127 deletions(-)

>   create mode 100644 include/efi_helper.h

>   create mode 100644 lib/efi_loader/efi_helper.c

>
Ilias Apalodimas Dec. 30, 2020, 9:17 p.m. UTC | #2
Hi Heinrich, 

On Wed, Dec 30, 2020 at 09:44:39PM +0100, Heinrich Schuchardt wrote:
> On 12/30/20 4:07 PM, Ilias Apalodimas wrote:

> > Hi!

> > 

> > This is v2 of [1].

> > 

> > There's been a couple of changes regarding where we install the protocol.

> > The initial patchset was completely disregarding BootNext, so that's taken

> > into account now and we can use the new feature.

> > This brought a few changes on the selftests as well, since we now use the

> > loaded image handle to install the protocol, as a consequence the custom

> > handle in the tests is now uninstalled during the test .teardown(), but

> > the overall approach remains identical.

> 

> Boot#### contains a device path.

> 

> Why should Initrd#### contain something U-Boot specific and not a device

> path?


Because there was no standard on it, since we used the path as a config option
up to now I just kept it as is.

> 

> Think of a Linux distribution. When updating the kernel it will have to

> write Boot#### and Initrd####. It can determine the current boot device

> via BootCurrent and Boot####. Next it can add the new file paths for

> Boot#### and Initrd#### relative to the same device.


This isn't a bad idea. Let me update the patch and send a v3.

Cheers
/Ilias