Hi Akashi-san,
On Fri, 25 Mar 2022 at 10:20, Takahiro Akashi takahiro.akashi@linaro.org wrote:
Kojima-san,
On Thu, Mar 24, 2022 at 10:54:32PM +0900, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
The menu example is as follows.
Good job done, Kojima-san. I like it. Before reviewing each commit, I would suggest a couple of improvements on the menu itself. They are more or less my opinion and other people may have their own preference, though.
Thank you for your comment and verifying the actual behavior.
- Top menu (U-Boot Boot Menu)
- In general, it's a bit difficult to understand from where each menu item comes and what it means. For instance,
UEFI BOOT0000 : debianis a user-defined boot option, while
UEFI BOOT0002 : mmc0:1is an option for removable media. Correct?
Yes, and I agree that the current menu title is a bit difficult to understand, especially for automatically listed removable media(e.g. mmc0:1).
I'd prefer to categorize items into sub-menus, particularly, UEFI items.
bootmenu_... distro_boot ... UEFI Boot UEFI Boot Manager Maintenance
Yes, it is a good idea. On the other hand, if user prefers simpler menu and only uses UEFI Boot for example, bootmenu_xx and distro boot entries can be removed by updating the U-Boot environment variable(remove bootmenu_xx and boot_targets).
and "UEFI Boot" sub-menu shows
UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntuin an order of "BootOrder",
and <removable media> /* not selectable */
UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2
It is a little difficult. UEFI BootOrder variable is also automatically updated by bootmenu to include the removable media, so the entries that user defined Boot#### and automatically added removable media are mixed in BootOrder. The UEFI Boot entry is displayed in the order of BootOrder.
- For UEFI items, I want to do "e" (edit/modify) directly here to change the option.
It is the same as the Grub menu. I will consider it but let me set as a low priority now.
- When "U-Boot console" is selected, the prompt ("=>") is displayed like UEFI Boot Manager Maintenance U-Boot console=> It should be output at the beginning of the next line or the screen be cleaned up before showing the prompt.
Thank you. It should be fixed.
- What not have "Quit" here?
OK, I will consider to add.
- UEFI Boot Manager Maintenance
- The title should be "UEFI Boot Manager Maintenance".
"UEFI Boot Manager Maintenance" is the same as the current one.
- I want to have "Edit(Modify) Boot Option"
Yes, I agree.
- "Add Boot Option"
- The menu titles should be "Select a device" and "Select a file".
Current "Add Boot Option" will support the "Select a file" feature. I think the user will normally use the boot option having FilePath.
- Some devices are shown, some are not. Why? Do we have to run, say, "scsi rescan" beforehand?
Current bootmenu enumerates the all devices implementing EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. If some devices are not shown, please run the discover command like "scsi rescan" then re-enter the bootmenu with "bootmenu" command. If the all devices listed by "efidebug dh" having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL are not shown in bootmenu, something wrong in my code.
- How can we specify "removable media" without a file path?
This patch series only supports the auto enumeration. I think the user will not manually add the "removable media".
- At "file selection" menu, "Esc" should let us go back to the "device selection" menu rather than the top, "Add Boot Option".
Yes, I agree.
- We should be able to specify initrd path, i.e. the second device path in a boot option.
- We should be able to specify "optional data" in a boot option.
- "Change Boot Order"
I consider supporting these two items.
- I like a more intuitive operation here. Say, select an item with "Enter" and then use "Up" and "Down" to move it around.
Yes, I agree and try to improve.
- Probably, it would be better to have the final confirmation, like "Do you want to save the change?"
Yes, same for "Delete Boot Option".
Thanks, Masahisa Kojima
Thanks, -Takahiro Akashi
*** U-Boot Boot Menu ***
bootmenu_00 : Boot 1. kernel bootmenu_01 : Boot 2. kernel bootmenu_02 : Reset board UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntu UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
-- 2.17.1