[PATCH v5 06/17] efi_loader: bootmgr: add booting from removable media

Thu May 5 14:20:00 CEST 2022

On 5/5/22 14:05, Mark Kettenis wrote:
>> Date: Fri, 29 Apr 2022 19:03:22 +0200
>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>
>> On 4/28/22 10:09, Masahisa Kojima wrote:
>>> From: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>
>>> Under the current implementation, booting from removable media using
>>> a architecture-specific default image name, say BOOTAA64.EFI, is
>>> supported only in distro_bootcmd script. See the commit 74522c898b35
>>> ("efi_loader: Add distro boot script for removable media").
>>>
>>> This is, however, half-baked implementation because
>>> 1) UEFI specification requires this feature to be implemented as part
>>>      of Boot Manager's responsibility:
>>>
>>>     3 - Boot Manager
>>>     3.5.1 Boot via the Simple File Protocol
>>>     When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will
>>>     start with a device path that points to the device that implements the
>>>     EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next
>>>     part of the FilePath may point to the file name, including
>>>     subdirectories, which contain the bootable image. If the file name is
>>>     a null device path, the file name must be generated from the rules
>>>     defined below.
>>>     ...
>>>     3.5.1.1 Removable Media Boot Behavior
>>>     To generate a file name when none is present in the FilePath, the
>>>     firmware must append a default file name in the form
>>>     \EFI\BOOT\BOOT{machine type short-name}.EFI ...
>>>
>>> 2) So (1) entails the hehavior that the user's preference of boot media
>>>      order should be determined by Boot#### and BootOrder variables.
>>>
>>> With this patch, the semantics mentioned above is fully implemented.
>>> For example, if you want to boot the system from USB and SCSI in this
>>> order,
>>> * define Boot0001 which contains only a device path to the USB device
>>>     (without any file path/name)
>>> * define Boot0002 which contains only a device path to the SCSI device,
>>> and
>>> * set BootOrder to Boot0001:Boot0002
>>>
>>> To avoid build error for sandbox, default file name "BOOTSANDBOX.efi"
>>> is defined even if it is out of scope of UEFI specification.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>>> ---
>>> Changes in v5:
>>> - add default file name definition for SANDBOX to avoid build error
>>>
>>> Changes from original version:
>>> - create new include file "efi_default_filename.h" to
>>>     avoid conflict with config_distro_bootcmd.h
>>> - modify the target pointer of efi_free_pool(), expand_media_path() should
>>>     only free the pointer allocated by efi_dp_from_file() function.
>>>    include/config_distro_bootcmd.h | 14 +--------
>>>    include/efi_default_filename.h  | 33 ++++++++++++++++++++++
>>>    lib/efi_loader/efi_bootmgr.c    | 50 ++++++++++++++++++++++++++++++++-
>>>    3 files changed, 83 insertions(+), 14 deletions(-)
>>>    create mode 100644 include/efi_default_filename.h
>>>
>>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
>>> index c55023889c..6a3110f27b 100644
>>> --- a/include/config_distro_bootcmd.h
>>> +++ b/include/config_distro_bootcmd.h
>>> @@ -91,19 +91,7 @@
>>>    #endif
>>>
>>>    #ifdef CONFIG_EFI_LOADER
>>> -#if defined(CONFIG_ARM64)
>>> -#define BOOTEFI_NAME "bootaa64.efi"
>>> -#elif defined(CONFIG_ARM)
>>> -#define BOOTEFI_NAME "bootarm.efi"
>>> -#elif defined(CONFIG_X86_RUN_32BIT)
>>> -#define BOOTEFI_NAME "bootia32.efi"
>>> -#elif defined(CONFIG_X86_RUN_64BIT)
>>> -#define BOOTEFI_NAME "bootx64.efi"
>>> -#elif defined(CONFIG_ARCH_RV32I)
>>> -#define BOOTEFI_NAME "bootriscv32.efi"
>>> -#elif defined(CONFIG_ARCH_RV64I)
>>> -#define BOOTEFI_NAME "bootriscv64.efi"
>>> -#endif
>>> +#include <efi_default_filename.h>
>>>    #endif
>>>
>>>    #ifdef BOOTEFI_NAME
>>> diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h
>>> new file mode 100644
>>> index 0000000000..cb2ef9e131
>>> --- /dev/null
>>> +++ b/include/efi_default_filename.h
>>> @@ -0,0 +1,33 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * Default boot file name when none is present in the FilePath.
>>> + * This is defined in the UEFI specification.
>>> + *
>>> + * Copyright (c) 2022, Linaro Limited
>>> + */
>>> +#ifndef _EFI_DEFAULT_FILENAME_H
>>> +#define _EFI_DEFAULT_FILENAME_H
>>> +
>>> +#if defined(CONFIG_ARM64)
>>> +#define BOOTEFI_NAME "BOOTAA64.EFI"
>>> +#elif defined(CONFIG_ARM)
>>> +#define BOOTEFI_NAME "BOOTARM.EFI"
>>> +#elif defined(CONFIG_X86_64)
>>> +#define BOOTEFI_NAME "BOOTX64.EFI"
>>> +#elif defined(CONFIG_X86)
>>> +#define BOOTEFI_NAME "BOOTIA32.EFI"
>>> +#elif defined(CONFIG_ARCH_RV32I)
>>> +#define BOOTEFI_NAME "BOOTRISCV32.EFI"
>>> +#elif defined(CONFIG_ARCH_RV64I)
>>> +#define BOOTEFI_NAME "BOOTRISCV64.EFI"
>>> +#elif defined(CONFIG_SANDBOX)
>>> +/*
>>> + * SANDBOX is not defined in UEFI specification, but
>>> + * this definition avoids build failure for SANDBOX.
>>> + */
>>> +#define BOOTEFI_NAME "BOOTSANDBOX.EFI"
>>
>> The sandbox should boot the default binary for the host architecture:
>>
>> #ifndef _EFI_DEFAULT_FILENAME_H
>> #define _EFI_DEFAULT_FILENAME_H
>>
>> #include <host_arch.h>
>>
>> #undef BOOTEFI_NAME
>>
>> #if HOST_ARCH == HOST_ARCH_X86_64
>> #define BOOTEFI_NAME "BOOTX64.EFI"
>> #endif
>>
>> #if HOST_ARCH == HOST_ARCH_X86
>> #define BOOTEFI_NAME "BOOTIA32.EFI"
>> #endif
>>
>> #if HOST_ARCH == HOST_ARCH_AARCH64
>> #define BOOTEFI_NAME "BOOTAA64.EFI"
>> #endif
>>
>> #if HOST_ARCH == HOST_ARCH_ARM
>> #define BOOTEFI_NAME "BOOTARM.EFI"
>> #endif
>>
>> #if HOST_ARCH == HOST_ARCH_RISCV32
>> #define BOOTEFI_NAME "BOOTRISCV32.EFI"
>> #endif
>>
>> #if HOST_ARCH == HOST_ARCH_RISCV64
>> #define BOOTEFI_NAME "BOOTRISCV64.EFI"
>> #endif
>>
>> #ifndef BOOTEFI_NAME
>> #error Unsupported UEFI architecture
>> #endif
>>
>> #endif
>
> Maybe sanbox is special, but using the host architecture for actual
> boards makes no sense.  I see this has made its way into master
> already, but when I cross-build for apple_m1_defconfig on an amd64
> machine I end up with:
>
>      $ strings ./lib/efi_loader/efi_bootmgr.o | grep BOOT
>      /EFI/BOOT/BOOTX64.EFI

Thanks for reporting the issue.

On Sandbox it should be the host architecture. on other defconfigs the
target architecture.

Best regards

Heinrich

>
> The original diff that used CONFIG_ARM64, CONFIG_ARM, etc, did this
> right.