Re: [PATCH v4 07/11] bootmenu: add UEFI and disto_boot entries

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Kojima-san,

[...]

> +			entry->title = u16_strdup(lo.label);
> +			if (!entry->title) {
> +				free(load_option);
> +				free(entry);

We need to free bootorder as well

> +				return -ENOMEM;
> +			}
> +			entry->command = strdup("bootefi bootmgr");
> +			sprintf(entry->key, "%d", i);
> +			entry->num = i;
> +			entry->menu = menu;
> +			entry->type = BOOTMENU_TYPE_UEFI_BOOT_OPTION;
> +			entry->bootorder = bootorder[j];
> +			entry->next = NULL;
> +
> +			if (!iter)
> +				menu->first = entry;
> +			else
> +				iter->next = entry;
> +
> +			iter = entry;
> +			i++;
> +		}
> +
> +		free(load_option);
> +
> +		if (i == MAX_COUNT - 1)
> +			break;
> +	}
> +
> +	free(bootorder);
> +	*index = i;
> +	*current = iter;
> +
> +	return 1;
> +}
> +
> +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);
> +	p = calloc(1, len + 1);
> +	strlcpy(p, boot_targets, len);
> +
> +	token = strtok(p, " ");
> +
> +	do {
> +		u16 *buf;
> +		char *command;
> +		int command_size;
> +
> +		entry = malloc(sizeof(struct bootmenu_entry));
> +		if (!entry)
> +			return -ENOMEM;
> +
> +		len = strlen(token);
> +		buf = calloc(1, (len + 1) * sizeof(u16));
> +		entry->title = buf;
> +		if (!entry->title) {
> +			free(entry);
> +			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);
> +			return -ENOMEM;

We need to free p as well

> +		}
> +		snprintf(command, command_size, "run bootcmd_%s", token);
> +		entry->command = command;
> +		entry->type = BOOTMENU_TYPE_DISTRO_BOOT;
>  		entry->next = NULL;
>  
>  		if (!iter)
> @@ -341,10 +512,59 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  			iter->next = entry;
>  
>  		iter = entry;
> -		++i;
> +		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;
> +	unsigned short int i = 0;
> +	struct bootmenu_data *menu;
> +	struct bootmenu_entry *iter = NULL;
> +	struct bootmenu_entry *entry;
> +
> +	char *default_str;
> +
> +	menu = malloc(sizeof(struct bootmenu_data));
> +	if (!menu)
> +		return NULL;
> +
> +	menu->delay = delay;
> +	menu->active = 0;
> +	menu->first = NULL;
> +
> +	default_str = env_get("bootmenu_default");
> +	if (default_str)
> +		menu->active = (int)simple_strtol(default_str, NULL, 10);
> +
> +	ret = prepare_bootmenu_entry(menu, &iter, &i);
> +	if (ret < 0)
> +		goto cleanup;
> +
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> +		if (i < MAX_COUNT - 1) {
> +			ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> +			if (ret < 0 && ret != -ENOENT)
> +				goto cleanup;
> +		}
> +	}
> +
> +	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 */
> @@ -353,7 +573,12 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  		if (!entry)
>  			goto cleanup;
>  
> -		entry->title = strdup("U-Boot console");
> +		/* Add dummy entry if entering U-Boot console is disabled */
> +		if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE))
> +			entry->title = u16_strdup(u"U-Boot console");
> +		else
> +			entry->title = u16_strdup(u"");
> +
>  		if (!entry->title) {
>  			free(entry);
>  			goto cleanup;
> @@ -370,6 +595,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  
>  		entry->num = i;
>  		entry->menu = menu;
> +		entry->type = BOOTMENU_TYPE_NONE;
>  		entry->next = NULL;
>  
>  		if (!iter)
> @@ -378,7 +604,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  			iter->next = entry;
>  
>  		iter = entry;
> -		++i;
> +		i++;
>  	}
>  
>  	menu->count = i;
> @@ -423,43 +649,73 @@ static void menu_display_statusline(struct menu *m)
>  	puts(ANSI_CLEAR_LINE);
>  }
>  
> -static void bootmenu_show(int delay)
> +static void handle_uefi_bootnext(void)
>  {
> +	u16 bootnext;
> +	efi_status_t ret;
> +	efi_uintn_t size;
> +
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +
> +		return;
> +	}
> +
> +	/* If UEFI BootNext variable is set, boot the BootNext load option */
> +	size = sizeof(u16);
> +	ret = efi_get_variable_int(u"BootNext",
> +				   &efi_global_variable_guid,
> +				   NULL, &size, &bootnext, NULL);
> +	if (ret == EFI_SUCCESS)
> +		/* BootNext does exist here, try to boot */
> +		run_command("bootefi bootmgr", 0);
> +}
> +
> +static enum bootmenu_ret bootmenu_show(int delay)
> +{
> +	int cmd_ret;
>  	int init = 0;
>  	void *choice = NULL;
> -	char *title = NULL;
> +	u16 *title = NULL;
>  	char *command = NULL;
>  	struct menu *menu;
>  	struct bootmenu_data *bootmenu;
>  	struct bootmenu_entry *iter;
> +	efi_status_t efi_ret = EFI_SUCCESS;
>  	char *option, *sep;
>  
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
> +		handle_uefi_bootnext();
> +
>  	/* If delay is 0 do not create menu, just run first entry */
>  	if (delay == 0) {
>  		option = bootmenu_getoption(0);
>  		if (!option) {
>  			puts("bootmenu option 0 was not found\n");
> -			return;
> +			return BOOTMENU_RET_FAIL;
>  		}
>  		sep = strchr(option, '=');
>  		if (!sep) {
>  			puts("bootmenu option 0 is invalid\n");
> -			return;
> +			return BOOTMENU_RET_FAIL;
>  		}
> -		run_command(sep+1, 0);
> -		return;
> +		cmd_ret = run_command(sep + 1, 0);
> +		return (cmd_ret == CMD_RET_SUCCESS ? BOOTMENU_RET_SUCCESS : BOOTMENU_RET_FAIL);
>  	}
>  
>  	bootmenu = bootmenu_create(delay);
>  	if (!bootmenu)
> -		return;
> +		return BOOTMENU_RET_FAIL;
>  
>  	menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
>  			   bootmenu_print_entry, bootmenu_choice_entry,
>  			   bootmenu);
>  	if (!menu) {
>  		bootmenu_destroy(bootmenu);
> -		return;
> +		return BOOTMENU_RET_FAIL;
>  	}
>  
>  	for (iter = bootmenu->first; iter; iter = iter->next) {
> @@ -478,8 +734,33 @@ static void bootmenu_show(int delay)
>  
>  	if (menu_get_choice(menu, &choice) == 1) {
>  		iter = choice;
> -		title = strdup(iter->title);
> +		/* last entry is U-Boot console or Quit */
> +		if (iter->num == iter->menu->count - 1) {
> +			menu_destroy(menu);
> +			bootmenu_destroy(bootmenu);
> +			return BOOTMENU_RET_QUIT;
> +		}
> +
> +		title = u16_strdup(iter->title);
>  		command = strdup(iter->command);
> +	} else {
> +		goto cleanup;
> +	}
> +
> +	/*
> +	 * If the selected entry is UEFI BOOT####, set the BootNext variable.
> +	 * Then uefi bootmgr is invoked by the preset command in iter->command.
> +	 */
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> +		if (iter->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION) {
> +			efi_ret = efi_set_variable_int(u"BootNext", &efi_global_variable_guid,
> +						       EFI_VARIABLE_NON_VOLATILE |
> +						       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +						       EFI_VARIABLE_RUNTIME_ACCESS,
> +						       sizeof(u16), &iter->bootorder, false);
> +			if (efi_ret != EFI_SUCCESS)
> +				goto cleanup;
> +		}
>  	}
>  
>  cleanup:
> @@ -493,21 +774,47 @@ cleanup:
>  	}
>  
>  	if (title && command) {
> -		debug("Starting entry '%s'\n", title);
> +		debug("Starting entry '%ls'\n", title);
>  		free(title);
> -		run_command(command, 0);
> +		if (efi_ret == EFI_SUCCESS)
> +			cmd_ret = run_command(command, 0);
>  		free(command);
>  	}
>  
>  #ifdef CONFIG_POSTBOOTMENU
>  	run_command(CONFIG_POSTBOOTMENU, 0);
>  #endif
> +
> +	if (efi_ret == EFI_SUCCESS && cmd_ret == CMD_RET_SUCCESS)
> +		return BOOTMENU_RET_SUCCESS;
> +
> +	return BOOTMENU_RET_FAIL;
>  }
>  
>  #ifdef CONFIG_AUTOBOOT_MENU_SHOW
>  int menu_show(int bootdelay)
>  {
> -	bootmenu_show(bootdelay);
> +	int ret;
> +
> +	while (1) {
> +		ret = bootmenu_show(bootdelay);
> +		bootdelay = -1;
> +		if (ret == BOOTMENU_RET_UPDATED)
> +			continue;
> +
> +		if (!IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) {
> +			if (ret == BOOTMENU_RET_QUIT) {
> +				/* default boot process */
> +				if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
> +					run_command("bootefi bootmgr", 0);
> +
> +				run_command("run bootcmd", 0);
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +
>  	return -1; /* -1 - abort boot and run monitor code */
>  }
>  #endif
> -- 
> 2.17.1
> 

Thanks
/Ilias