[PATCH v3 6/7] efi_loader: Pass in the required parameters from EFI bootmeth
Simon Glass
sjg at chromium.org
Thu Jan 9 16:02:11 CET 2025
Hi Heinrich,
On Sun, 22 Dec 2024 at 05:55, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 12/19/24 03:38, Simon Glass wrote:
> > Rather than setting up the global variables and then making the call,
> > pass them into function directly. This cleans up the code and makes it
> > all a bit easier to understand.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Use efi_loader tag instead of efi
> > - Drop unnecessary path removal
> > - Fix 'require' typo
> > - Move calculation of dev-name into a separate function
>
> Please, split the patch into logical units.
What do you mean? Are you wanting me to have this patch and then four
more after it to make the changes listed above? If so, why?
>
> >
> > boot/bootmeth_efi.c | 49 +------------------------
> > include/efi_loader.h | 10 +++++
> > lib/efi_loader/efi_bootbin.c | 71 ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 83 insertions(+), 47 deletions(-)
> >
> > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > index f836aa655f5..fa7f66c9a0b 100644
> > --- a/boot/bootmeth_efi.c
> > +++ b/boot/bootmeth_efi.c
> > @@ -52,40 +52,6 @@ static bool bootmeth_uses_network(struct bootflow *bflow)
> > device_get_uclass_id(media) == UCLASS_ETH;
> > }
> >
> > -static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow)
> > -{
> > - const struct udevice *media_dev;
> > - int size = bflow->size;
> > - const char *dev_name;
> > - char devnum_str[9];
> > - char dirname[200];
> > - char *last_slash;
> > -
> > - /*
> > - * This is a horrible hack to tell EFI about this boot device. Once we
> > - * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > - * exists in multiple places, e.g. in the fs, tftp and load commands.
> > - *
> > - * Once we can clean up the EFI code to make proper use of driver model,
> > - * this can go away.
> > - */
> > - media_dev = dev_get_parent(bflow->dev);
> > - snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
> > - desc ? desc->devnum : dev_seq(media_dev),
> > - bflow->part);
> > -
> > - strlcpy(dirname, bflow->fname, sizeof(dirname));
> > - last_slash = strrchr(dirname, '/');
> > - if (last_slash)
> > - *last_slash = '\0';
> > -
> > - dev_name = device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE ?
> > - "usb" : blk_get_uclass_name(device_get_uclass_id(media_dev));
> > - log_debug("setting bootdev %s, %s, %s, %p, %x\n",
> > - dev_name, devnum_str, bflow->fname, bflow->buf, size);
> > - efi_set_bootdev(dev_name, devnum_str, bflow->fname, bflow->buf, size);
> > -}
> > -
> > static int efiload_read_file(struct bootflow *bflow, ulong addr)
> > {
> > struct blk_desc *desc = NULL;
> > @@ -103,8 +69,6 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr)
> > return log_msg_ret("read", ret);
> > bflow->buf = map_sysmem(addr, bflow->size);
> >
> > - set_efi_bootdev(desc, bflow);
> > -
> > return 0;
> > }
> >
> > @@ -342,17 +306,8 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
> > fdt = env_get_hex("fdt_addr_r", 0);
> > }
> >
> > - if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
> > - log_debug("Booting with built-in fdt\n");
> > - if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
> > - EFI_FDT_USE_INTERNAL))
> > - return log_msg_ret("run", -EINVAL);
> > - } else {
> > - log_debug("Booting with external fdt\n");
> > - if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
> > - map_sysmem(fdt, 0)))
> > - return log_msg_ret("run", -EINVAL);
> > - }
> > + if (efi_bootflow_run(bflow))
> > + return log_msg_ret("run", -EINVAL);
> >
> > return 0;
> > }
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 9afbec35ebf..a9325b1a9d0 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -20,6 +20,7 @@
> > #include <linux/oid_registry.h>
> >
> > struct blk_desc;
> > +struct bootflow;
> > struct jmp_buf_data;
> >
> > #if CONFIG_IS_ENABLED(EFI_LOADER)
> > @@ -578,6 +579,15 @@ efi_status_t efi_install_fdt(void *fdt);
> > efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options);
> > /* Run loaded UEFI image with given fdt */
> > efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
> > +
> > +/**
> > + * efi_bootflow_run() - Run a bootflow containing an EFI application
> > + *
> > + * @bootflow: Bootflow to run
> > + * Return: Status code, something went wrong
> > + */
> > +efi_status_t efi_bootflow_run(struct bootflow *bootflow);
> > +
> > /* Initialize variable services */
> > efi_status_t efi_init_variables(void);
> > /* Notify ExitBootServices() is called */
> > diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> > index 8332993f421..b5d15a723b4 100644
> > --- a/lib/efi_loader/efi_bootbin.c
> > +++ b/lib/efi_loader/efi_bootbin.c
> > @@ -6,13 +6,16 @@
> >
> > #define LOG_CATEGORY LOGC_EFI
> >
> > +#include <bootflow.h>
> > #include <charset.h>
> > +#include <dm.h>
> > #include <efi.h>
> > #include <efi_loader.h>
> > #include <env.h>
> > #include <image.h>
> > #include <log.h>
> > #include <malloc.h>
> > +#include <mapmem.h>
> >
> > static struct efi_device_path *bootefi_image_path;
> > static struct efi_device_path *bootefi_device_path;
> > @@ -285,3 +288,71 @@ out:
> > return ret;
> > }
> >
> > +/**
> > + * calc_dev_name() - Calculate the device name to give to EFI
> > + *
> > + * If not supported, this shows an error.
> > + *
> > + * Return name, or NULL if not supported
> > + */
> > +static const char *calc_dev_name(struct bootflow *bflow)
> > +{
> > + const struct udevice *media_dev;
> > +
> > + media_dev = dev_get_parent(bflow->dev);
> > +
> > + if (!bflow->blk) {
> > + log_err("Cannot boot EFI app on media '%s'\n",
> > + dev_get_uclass_name(media_dev));
> > +
> > + return NULL;
> > + }
> > +
> > + if (device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE)
> > + return "usb";
> > +
> > + return blk_get_uclass_name(device_get_uclass_id(media_dev));
> > +}
> > +
> > +efi_status_t efi_bootflow_run(struct bootflow *bflow)
> > +{
> > + struct efi_device_path *device, *image;
> > + const struct udevice *media_dev;
> > + struct blk_desc *desc = NULL;
> > + const char *dev_name;
> > + char devnum_str[9];
> > + efi_status_t ret;
> > + void *fdt;
> > +
> > + media_dev = dev_get_parent(bflow->dev);
> > + if (bflow->blk) {
> > + desc = dev_get_uclass_plat(bflow->blk);
>
> Where do you handle network devices?
That's not supported with bootstd yet. See the last patch in this
series[1]. Also [2]
> > +
> > + snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
> > + desc ? desc->devnum : dev_seq(media_dev), bflow->part);
> > + } else {
> > + *devnum_str = '\0';
> > + }
> > +
> > + dev_name = calc_dev_name(bflow);
> > + log_debug("dev_name '%s' devnum_str '%s' fname '%s' media_dev '%s'\n",
> > + dev_name, devnum_str, bflow->fname, media_dev->name);
> > + if (!dev_name)
> > + return EFI_UNSUPPORTED;
> > + ret = calculate_paths(dev_name, devnum_str, bflow->fname, &device,
> > + &image);
> > + if (ret)
> > + return ret;
> > +
> > + if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
> > + log_debug("Booting with built-in fdt\n");
> > + fdt = EFI_FDT_USE_INTERNAL;
> > + } else {
> > + log_debug("Booting with external fdt\n");
> > + fdt = map_sysmem(bflow->fdt_addr, 0);
> > + }
> > + ret = _efi_binary_run(bflow->buf, bflow->size, fdt, device, image);
> > +
> > + return ret;
> > +}
> > +
>
- Simon
[1] https://patchwork.ozlabs.org/project/uboot/patch/20241219023832.1098689-8-sjg@chromium.org/
[2] https://patchwork.ozlabs.org/project/uboot/list/?series=435516
More information about the U-Boot
mailing list