[RFC 11/14] efi_loader: move distro_efi_get_fdt_name()
Heinrich Schuchardt
heinrich.schuchardt at canonical.com
Fri Apr 26 17:18:17 CEST 2024
On 26.04.24 16:52, Caleb Connolly wrote:
> Hi Heinrich,
>
> On 26/04/2024 16:13, Heinrich Schuchardt wrote:
>> Move distro_efi_get_fdt_name() to a separate C module
>> and rename it to efi_get_distro_fdt_name().
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>> boot/bootmeth_efi.c | 60 ++-------------------------------
>> include/efi_loader.h | 2 ++
>> lib/efi_loader/Makefile | 1 +
>> lib/efi_loader/efi_fdt.c | 73 ++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 78 insertions(+), 58 deletions(-)
>> create mode 100644 lib/efi_loader/efi_fdt.c
>>
>> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
>> index aebc5207fc0..40da77c497b 100644
>> --- a/boot/bootmeth_efi.c
>> +++ b/boot/bootmeth_efi.c
>> @@ -144,62 +144,6 @@ static int distro_efi_check(struct udevice *dev,
>> struct bootflow_iter *iter)
>> return 0;
>> }
>> -/**
>> - * distro_efi_get_fdt_name() - Get the filename for reading the .dtb
>> file
>> - *
>> - * @fname: Place to put filename
>> - * @size: Max size of filename
>> - * @seq: Sequence number, to cycle through options (0=first)
>> - * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not
>> exist,
>> - * -EINVAL if there are no more options, -EALREADY if the control FDT
>> should be
>> - * used
>> - */
>> -static int distro_efi_get_fdt_name(char *fname, int size, int seq)
>> -{
>> - const char *fdt_fname;
>> - const char *prefix;
>> -
>> - /* select the prefix */
>> - switch (seq) {
>> - case 0:
>> - /* this is the default */
>> - prefix = "/dtb";
>> - break;
>> - case 1:
>> - prefix = "";
>> - break;
>> - case 2:
>> - prefix = "/dtb/current";
>> - break;
>
> This is a bit of a tangential point (and shouldn't block this series at
> all). But where do we find a balance with this search algorithm? The
> distro I work on (postmarketOS) actually installs DTB files to "/dtbs"
> (not "/dtb").
>
> No doubt we can come up with a bunch of clever ideas for optimising this
> with wildcards or whatever, but is that something we want to implement?
>
> What about distros that support falling back to previous kernel versions
> (and consequently have the DTB dir named after the kernel version).
>
> Presumably in these situations the distro would have systemd-boot, GRUB,
> etc. Would it make sense to just inform the bootloader what the .dtb
> file name is so that they can handle the path building?
>
> (I understand there
We have been carrying the logic for scanning for DTBs in U-Boot since
commit Fixes: 74522c898b35 ("efi_loader: Add distro boot script for
removable media") merged in 2016.
The script based version allowed to modify variable efi_dtb_prefixes to
scan other directories.
Now Simon has started with the 2024.01 release to move the logic from
scripts in header files with script fragments to C files in /boot/.
This series is cleaning up the changes for bootmethod_efi_mgr.c.
Debian's flash-kernel package installs device-trees in dtb. I do not
know if there are users for '/dtb/current/' or '/'. The more directories
we scan the slower booting.
This specific patch does not change the current logic. If we need the
flexibility back, that was provided by environment variable
efi_dtb_prefixes we should look at it in a future change.
Best regards
Heinrich
are other usecases and reasons to load the DTB ahead
> of time in U-Boot).
>> - default:
>> - return log_msg_ret("pref", -EINVAL);
>> - }
>> -
>> - fdt_fname = env_get("fdtfile");
>> - if (fdt_fname) {
>> - snprintf(fname, size, "%s/%s", prefix, fdt_fname);
>> - log_debug("Using device tree: %s\n", fname);
>> - } else if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE)) {
>> - strcpy(fname, "<prior>");
>> - return log_msg_ret("pref", -EALREADY);
>> - /* Use this fallback only for 32-bit ARM */
>> - } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
>> - const char *soc = env_get("soc");
>> - const char *board = env_get("board");
>> - const char *boardver = env_get("boardver");
>> -
>> - /* cf the code in label_boot() which seems very complex */
>> - snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
>> - soc ? soc : "", soc ? "-" : "", board ? board : "",
>> - boardver ? boardver : "");
>> - log_debug("Using default device tree: %s\n", fname);
>> - } else {
>> - return log_msg_ret("env", -ENOENT);
>> - }
>> -
>> - return 0;
>> -}
>> -
>> /*
>> * distro_efi_try_bootflow_files() - Check that files are present
>> *
>> @@ -241,7 +185,7 @@ static int distro_efi_try_bootflow_files(struct
>> udevice *dev,
>> ret = -ENOENT;
>> *fname = '\0';
>> for (seq = 0; ret == -ENOENT; seq++) {
>> - ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
>> + ret = efi_get_distro_fdt_name(fname, sizeof(fname), seq);
>> if (ret == -EALREADY)
>> bflow->flags = BOOTFLOWF_USE_PRIOR_FDT;
>> if (!ret) {
>> @@ -340,7 +284,7 @@ static int distro_efi_read_bootflow_net(struct
>> bootflow *bflow)
>> sprintf(file_addr, "%lx", fdt_addr);
>> /* We only allow the first prefix with PXE */
>> - ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
>> + ret = efi_get_distro_fdt_name(fname, sizeof(fname), 0);
>> if (ret)
>> return log_msg_ret("nam", ret);
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 0ac04990b8e..ed2b517b130 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -1187,4 +1187,6 @@ efi_status_t efi_disk_get_device_name(const
>> efi_handle_t handle, char *buf, int
>> */
>> void efi_add_known_memory(void);
>> +int efi_get_distro_fdt_name(char *fname, int size, int seq);
>> +
>> #endif /* _EFI_LOADER_H */
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index 034e366967f..2af6f2066b5 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -59,6 +59,7 @@ obj-y += efi_device_path.o
>> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
>> obj-$(CONFIG_EFI_DEVICE_PATH_UTIL) += efi_device_path_utilities.o
>> obj-y += efi_dt_fixup.o
>> +obj-y += efi_fdt.o
>> obj-y += efi_file.o
>> obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o
>> obj-y += efi_image_loader.o
>> diff --git a/lib/efi_loader/efi_fdt.c b/lib/efi_loader/efi_fdt.c
>> new file mode 100644
>> index 00000000000..0edf0c1e2fc
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_fdt.c
>> @@ -0,0 +1,73 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Bootmethod for distro boot via EFI
>> + *
>> + * Copyright 2021 Google LLC
>> + * Written by Simon Glass <sjg at chromium.org>
>> + */
>> +
>> +#include <efi_loader.h>
>> +#include <env.h>
>> +#include <errno.h>
>> +#include <log.h>
>> +#include <string.h>
>> +#include <vsprintf.h>
>> +
>> +/**
>> + * distro_efi_get_fdt_name() - get the filename for reading the .dtb
>> file
>> + *
>> + * @fname: buffer for filename
>> + * @size: buffer size
>> + * @seq: sequence number, to cycle through options (0=first)
>> + *
>> + * Returns:
>> + * 0 on success,
>> + * -ENOENT if the "fdtfile" env var does not exist,
>> + * -EINVAL if there are no more options,
>> + * -EALREADY if the control FDT should be used
>> + */
>> +int efi_get_distro_fdt_name(char *fname, int size, int seq)
>> +{
>> + const char *fdt_fname;
>> + const char *prefix;
>> +
>> + /* select the prefix */
>> + switch (seq) {
>> + case 0:
>> + /* this is the default */
>> + prefix = "/dtb";
>> + break;
>> + case 1:
>> + prefix = "";
>> + break;
>> + case 2:
>> + prefix = "/dtb/current";
>> + break;
>> + default:
>> + return log_msg_ret("pref", -EINVAL);
>> + }
>> +
>> + fdt_fname = env_get("fdtfile");
>> + if (fdt_fname) {
>> + snprintf(fname, size, "%s/%s", prefix, fdt_fname);
>> + log_debug("Using device tree: %s\n", fname);
>> + } else if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE)) {
>> + strcpy(fname, "<prior>");
>> + return log_msg_ret("pref", -EALREADY);
>> + /* Use this fallback only for 32-bit ARM */
>> + } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
>> + const char *soc = env_get("soc");
>> + const char *board = env_get("board");
>> + const char *boardver = env_get("boardver");
>> +
>> + /* cf the code in label_boot() which seems very complex */
>> + snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
>> + soc ? soc : "", soc ? "-" : "", board ? board : "",
>> + boardver ? boardver : "");
>> + log_debug("Using default device tree: %s\n", fname);
>> + } else {
>> + return log_msg_ret("env", -ENOENT);
>> + }
>> +
>> + return 0;
>> +}
>
More information about the U-Boot
mailing list