From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CB28BC433F5 for ; Thu, 5 May 2022 13:25:42 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3DD4A83BF4; Thu, 5 May 2022 15:25:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id EF57183B25; Thu, 5 May 2022 15:25:36 +0200 (CEST) Received: from sibelius.xs4all.nl (80-61-163-207.fixed.kpn.net [80.61.163.207]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id C288C83B25 for ; Thu, 5 May 2022 15:25:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=mark.kettenis@xs4all.nl Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id 1a2299a3; Thu, 5 May 2022 15:25:32 +0200 (CEST) Date: Thu, 5 May 2022 15:25:32 +0200 (CEST) From: Mark Kettenis To: Heinrich Schuchardt Cc: masahisa.kojima@linaro.org, ilias.apalodimas@linaro.org, sjg@chromium.org, takahiro.akashi@linaro.org, francois.ozog@linaro.org, kettenis@openbsd.org, Peter.Hoyes@arm.com, narmstrong@baylibre.com, andre.przywara@arm.com, u-boot@lists.denx.de In-Reply-To: <13765470-fedf-4f3e-f6b6-42a0f2347b18@gmx.de> (message from Heinrich Schuchardt on Thu, 5 May 2022 14:35:36 +0200) Subject: Re: [PATCH v5 06/17] efi_loader: bootmgr: add booting from removable media References: <20220428080950.23509-1-masahisa.kojima@linaro.org> <20220428080950.23509-7-masahisa.kojima@linaro.org> <2175cecb-a392-5107-b932-b099ec21d62c@gmx.de> <7e376971-89ba-5f4e-04ac-b45433f682fe@gmx.de> <13765470-fedf-4f3e-f6b6-42a0f2347b18@gmx.de> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean > Date: Thu, 5 May 2022 14:35:36 +0200 > From: Heinrich Schuchardt > > On 5/5/22 14:20, Heinrich Schuchardt wrote: > > On 5/5/22 14:05, Mark Kettenis wrote: > >>> Date: Fri, 29 Apr 2022 19:03:22 +0200 > >>> From: Heinrich Schuchardt > >>> > >>> On 4/28/22 10:09, Masahisa Kojima wrote: > >>>> From: AKASHI Takahiro > >>>> > >>>> 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 > >>>> Signed-off-by: Masahisa Kojima > >>>> --- > >>>> 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 > >>>>    #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 > >>> > >>> #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. > > On Ubuntu 22.04: > > git reset --hard 1739a6db5403d187902dcebca548de0644c8078f > make apple_m1_defconfig > CROSS_COMPILE=aarch64-linux-gnu- make -j16 > > $ strings ./lib/efi_loader/efi_bootmgr.o | grep BOOT > /EFI/BOOT/BOOTAA64.EFI > UCLASS_REBOOT_MODE > UCLASS_BOOTSTD > EFI_OBJECT_TYPE_U_BOOT_FIRMWARE > UCLASS_BOOTDEV > UCLASS_BOOTMETH > UCLASS_BOOTCOUN > > $ strings u-boot | grep 'BOOT\S*\.EFI' > /EFI/BOOT/BOOTAA64.EFI > > How can I reproduce your problem? > Is this BSD specific? Which distro are you running? Yes, I'm always building on OpenBSD. And the probablem is the use of non-portable GNU sed extensions \s and \S. Replacing those with [[:space:]] and [^[:space:]] as in the diff below seems to work. Decoding chicken scratches is alway fun... This has been broken for a while, but didn't cause any build issues until now. I'll send a proper patch to the mailing list. diff --git a/Makefile b/Makefile index ea80f00716..6eceeb35b4 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ include include/host_arch.h ifeq ("", "$(CROSS_COMPILE)") MK_ARCH="${shell uname -m}" else - MK_ARCH="${shell echo $(CROSS_COMPILE) | sed -n 's/^\s*\([^\/]*\/\)*\([^-]*\)-\S*/\2/p'}" + MK_ARCH="${shell echo $(CROSS_COMPILE) | sed -n 's/^[[:space:]]*\([^\/]*\/\)*\([^-]*\)-[^[:space:]]*/\2/p'}" endif unexport HOST_ARCH ifeq ("x86_64", $(MK_ARCH))