Re: [PATCH v5 10/17] bootmenu: add distro boot entry

[Takahiro Akashi <takahiro.akashi@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.

> Best regards
> 
> heinrich
> 
> > 
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@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));
>