> Date: Thu, 12 May 2022 12:39:39 +0200
> From: Heinrich Schuchardt <xypron.g...@gmx.de>
>
> Am 12. Mai 2022 10:44:28 MESZ schrieb Takahiro Akashi
> <takahiro.aka...@linaro.org>:
> >On Sun, May 01, 2022 at 11:48:40PM +0200, Heinrich Schuchardt wrote:
> >> On 4/28/22 10:09, Masahisa Kojima wrote:
> >> > This commit adds the distro_boot entries into the bootmenu.
> >> > The bootmenu read the "boot_targets" U-Boot environment variable
> >> > and enumerate it.
> >> > User can select the distro boot entry, then bootmenu executes
> >> > "run bootcmd_xxx" command.
> >> >
> >> > The bootmenu also checks the existing block devices and network
> >> > option("dhcp" and "pxe") availability, then filter out
> >> > the "boot_targets" appeared in bootmenu.
> >> >
> >> > The bootmenu example is as follows, distro boot entry has the
> >> > "distro_boot" prefix.
> >> >
> >> > *** U-Boot Boot Menu ***
> >> >
> >> > distro_boot : usb0
> >> > distro_boot : scsi0
> >> > distro_boot : virtio0
> >> > distro_boot : dhcp
> >>
> >> You will be creating UEFI boot options for all block devices anyway.
> >> We should avoid duplicate entries. Please, drop this patch.
> >
> >Nak
> >A distro entry should work, searching not only for a default UEFI file
> >but also sysboot (extlinux.conf) and boot scripts.
>
> Nobody expects this in an UEFI boot process.
Not sure about that. Some x86 firmware implementations present a boot
menu that displays both UEFI and "legacy" boot entries.
> Simon's work on bootflows tries to separate different bootflows.
And I'm still not convinced it does so in a sensible way. I can't
really test the bootflows since the implementation is incomplete.
The bootmenu I saw when I tested things a few weeks ago made sense to
me. It presented all the relevant options (UEFI and distroboot for
the relevant boot devices) on a system without any of the UEFI
BootXXXX options set. Some Linux distros still don't support the UEFI
bootflow (or at least don't support it very well), so distro_boot is
still important.
> So adding entries should depend on the configured bootflow.
But in my opinion, U-Boot should not depend on explicitly configured
bootflows.
Cheers,
Mark
> >> >
> >> > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> >> > ---
> >> > Changes in v5:
> >> > - split into the separate patch
> >> > - add function description comment
> >> > - handle the case boot_targets variable is empty
> >> > - filter out the non-exist device entry
> >> >
> >> > cmd/bootmenu.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
> >> > 1 file changed, 177 insertions(+)
> >> >
> >> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> >> > index da688e6213..afe42b8041 100644
> >> > --- a/cmd/bootmenu.c
> >> > +++ b/cmd/bootmenu.c
> >> > @@ -7,6 +7,7 @@
> >> > #include <common.h>
> >> > #include <command.h>
> >> > #include <ansi.h>
> >> > +#include <dm.h>
> >> > #include <efi_loader.h>
> >> > #include <efi_variable.h>
> >> > #include <env.h>
> >> > @@ -14,6 +15,7 @@
> >> > #include <menu.h>
> >> > #include <watchdog.h>
> >> > #include <malloc.h>
> >> > +#include <linux/ctype.h>
> >> > #include <linux/delay.h>
> >> > #include <linux/string.h>
> >> >
> >> > @@ -31,6 +33,7 @@ enum boot_type {
> >> > BOOTMENU_TYPE_NONE = 0,
> >> > BOOTMENU_TYPE_BOOTMENU,
> >> > BOOTMENU_TYPE_UEFI_BOOT_OPTION,
> >> > + BOOTMENU_TYPE_DISTRO_BOOT,
> >> > };
> >> >
> >> > struct bootmenu_entry {
> >> > @@ -90,6 +93,8 @@ static void bootmenu_print_entry(void *data)
> >> > printf("bootmenu_%02d : %ls", entry->bootorder,
> >> > entry->title);
> >> > else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
> >> > printf("UEFI BOOT%04X : %ls", entry->bootorder,
> >> > entry->title);
> >> > + else if (entry->type == BOOTMENU_TYPE_DISTRO_BOOT)
> >> > + printf("distro_boot : %ls", entry->title);
> >> > else
> >> > printf("%ls", entry->title);
> >> >
> >> > @@ -465,6 +470,172 @@ static int prepare_uefi_bootorder_entry(struct
> >> > bootmenu_data *menu,
> >> > return 1;
> >> > }
> >> >
> >> > +static int is_blk_device_available(char *token)
> >> > +{
> >> > + struct driver *d = ll_entry_start(struct driver, driver);
> >> > + const int n_ents = ll_entry_count(struct driver, driver);
> >> > + struct driver *entry;
> >> > + struct udevice *udev;
> >> > + struct uclass *uc;
> >> > + struct blk_desc *desc;
> >> > + int ret, i;
> >> > + const char *if_type_name;
> >> > +
> >> > + ret = uclass_get(UCLASS_BLK, &uc);
> >> > + if (ret)
> >> > + return -ENODEV;
> >> > +
> >> > + for (entry = d; entry < d + n_ents; entry++) {
> >> > + if (entry->id != UCLASS_BLK)
> >> > + continue;
> >> > + i = 0;
> >> > + uclass_foreach_dev(udev, uc) {
> >> > + if (udev->driver != entry)
> >> > + continue;
> >> > + desc = dev_get_uclass_plat(udev);
> >> > + if_type_name =
> >> > blk_get_if_type_name(desc->if_type);
> >> > + if (strncmp(token, if_type_name,
> >> > strlen(if_type_name)) == 0) {
> >> > + char *p;
> >> > + int j, len;
> >> > + int devnum = 0;
> >> > +
> >> > + p = token + strlen(if_type_name);
> >> > + len = strlen(p);
> >> > + if (!len)
> >> > + continue; /* no device number */
> >> > +
> >> > + for (j = 0; j < len; j++) {
> >> > + if (!isdigit(*p)) {
> >> > + /* invalid device
> >> > number */
> >> > + devnum = INT_MAX;
> >> > + break;
> >> > + }
> >> > + devnum = (devnum * 10) + (*p++
> >> > - '0');
> >> > + }
> >> > +
> >> > + if (devnum == INT_MAX)
> >> > + continue;
> >> > +
> >> > + if (devnum == desc->devnum)
> >> > + return 1;
> >> > + }
> >> > + }
> >> > + }
> >> > +
> >> > + if (strncmp(token, "dhcp", strlen("dhcp")) == 0) {
> >> > + if (IS_ENABLED(CONFIG_CMD_DHCP))
> >> > + return 1;
> >> > + }
> >> > +
> >> > + if (strncmp(token, "pxe", strlen("pxe")) == 0) {
> >> > + if (IS_ENABLED(CONFIG_CMD_PXE))
> >> > + return 1;
> >> > + }
> >> > +
> >> > + return -ENODEV;
> >> > +}
> >> > +
> >> > +/**
> >> > + * prepare_distro_boot_entry() - generate the distro boot entries
> >> > + *
> >> > + * This function read the "boot_targets" U-Boot environment variable
> >> > + * and generate the bootmenu entries.
> >> > + *
> >> > + * @menu: pointer to the bootmenu structure
> >> > + * @current: pointer to the last bootmenu entry list
> >> > + * @index: pointer to the index of the last bootmenu entry,
> >> > + * the number of uefi entry is added by this function
> >> > + * Return: 1 on success, negative value on error
> >> > + */
> >> > +static int prepare_distro_boot_entry(struct bootmenu_data *menu,
> >> > + struct bootmenu_entry **current,
> >> > + unsigned short int *index)
> >> > +{
> >> > + char *p;
> >> > + int len;
> >> > + char *token;
> >> > + char *boot_targets;
> >> > + unsigned short int i = *index;
> >> > + struct bootmenu_entry *entry = NULL;
> >> > + struct bootmenu_entry *iter = *current;
> >> > +
> >> > + /* list the distro boot "boot_targets" */
> >> > + boot_targets = env_get("boot_targets");
> >> > + if (!boot_targets)
> >> > + return -ENOENT;
> >> > +
> >> > + len = strlen(boot_targets);
> >> > + if (!len)
> >> > + return -ENOENT;
> >> > +
> >> > + p = calloc(1, len + 1);
> >> > + strlcpy(p, boot_targets, len);
> >> > +
> >> > + token = strtok(p, " ");
> >> > +
> >> > + do {
> >> > + u16 *buf;
> >> > + char *command;
> >> > + int command_size;
> >> > +
> >> > + if (is_blk_device_available(token) != 1) {
> >> > + token = strtok(NULL, " ");
> >> > + continue;
> >> > + }
> >> > +
> >> > + entry = malloc(sizeof(struct bootmenu_entry));
> >> > + if (!entry) {
> >> > + free(p);
> >> > + return -ENOMEM;
> >> > + }
> >> > +
> >> > + len = strlen(token);
> >> > + buf = calloc(1, (len + 1) * sizeof(u16));
> >> > + entry->title = buf;
> >> > + if (!entry->title) {
> >> > + free(entry);
> >> > + free(p);
> >> > + return -ENOMEM;
> >> > + }
> >> > + utf8_utf16_strncpy(&buf, token, len);
> >> > + sprintf(entry->key, "%d", i);
> >> > + entry->num = i;
> >> > + entry->menu = menu;
> >> > +
> >> > + command_size = sizeof("run bootcmd_") + len;
> >> > + command = calloc(1, command_size);
> >> > + if (!command) {
> >> > + free(entry->title);
> >> > + free(entry);
> >> > + free(p);
> >> > + return -ENOMEM;
> >> > + }
> >> > + snprintf(command, command_size, "run bootcmd_%s",
> >> > token);
> >> > + entry->command = command;
> >> > + entry->type = BOOTMENU_TYPE_DISTRO_BOOT;
> >> > + entry->next = NULL;
> >> > +
> >> > + if (!iter)
> >> > + menu->first = entry;
> >> > + else
> >> > + iter->next = entry;
> >> > +
> >> > + iter = entry;
> >> > + i++;
> >> > +
> >> > + if (i == MAX_COUNT - 1)
> >> > + break;
> >> > +
> >> > + token = strtok(NULL, " ");
> >> > + } while (token);
> >> > +
> >> > + free(p);
> >> > + *index = i;
> >> > + *current = iter;
> >> > +
> >> > + return 1;
> >> > +}
> >> > +
> >> > static struct bootmenu_data *bootmenu_create(int delay)
> >> > {
> >> > int ret;
> >> > @@ -498,6 +669,12 @@ static struct bootmenu_data *bootmenu_create(int
> >> > delay)
> >> > }
> >> > }
> >> >
> >> > + if (i < MAX_COUNT - 1) {
> >> > + ret = prepare_distro_boot_entry(menu, &iter, &i);
> >> > + if (ret < 0 && ret != -ENOENT)
> >> > + goto cleanup;
> >> > + }
> >> > +
> >> > /* Add U-Boot console entry at the end */
> >> > if (i <= MAX_COUNT - 1) {
> >> > entry = malloc(sizeof(struct bootmenu_entry));
> >>
>
>
