From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7DABFC433F5 for ; Fri, 25 Mar 2022 06:57:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A880884155; Fri, 25 Mar 2022 07:57:54 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="t1NrBwAw"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 88E5A8415F; Fri, 25 Mar 2022 07:57:53 +0100 (CET) Received: from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com [IPv6:2607:f8b0:4864:20::1133]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3BB5284153 for ; Fri, 25 Mar 2022 07:57:50 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=masahisa.kojima@linaro.org Received: by mail-yw1-x1133.google.com with SMTP id 00721157ae682-2d07ae0b1c0so73575937b3.2 for ; Thu, 24 Mar 2022 23:57:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=HrBY4PpGDEBsf5ta0G04VoYfbCrVdfL6qD0pLaNPijc=; b=t1NrBwAwW/R6MZWeYld4vcMX6yIsFKd0N8frIFs0FRRb/3AzKzJ3q8/lHM604rP6Dy sqQ2ZtWPRBEEH8iHLtot4i7C+9EtYe3vyxDlHQlvVbICbnv+1oUEUFu5Zwla2PDZ/qqa 7VEuGi4LwEyoLm6h+PTNL7c25q9iuck59Ra7aicleBmXZckIFcIq26DDsrNIEo16sJaT +oh93E0z9S1GsWXHaxhsZBuf5I1cw3J76Y0SYhx9W1iJBO5FkVXjXZqVRCCWEod7NKj9 FMkjsiw3VeBOvEIizxmWNINel8VYyfDkkMPRe/Y9KjUX30uZBljknB+kfgE/7dFItLuj QqNQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=HrBY4PpGDEBsf5ta0G04VoYfbCrVdfL6qD0pLaNPijc=; b=3OMThsi+l3+vNbhO//D7N84Vm/jZaTy6pqnyH04DdqYWVXhgKHNYQiS0pVNZT6G/ug A7jO12uANAwFZnn4vX6dN90yGHkYzZGGcOYcVnfAe6XFHmkwh1ihMJVQAFNDoo0i2tZl x4dhMkRXptrkDI1zgkHvcVe+rRuSmEgDtr7uhHsT5ig7rt0+z4zvuNOfjJ10QaFH1mvr rtdPmV//iC4PMoRuedUK+PxYFXqrrAXBIWAl4uqgOnl6nvZzb7KLCyUBskswvGM2tz7z J0cHubBIA+XiPwvOWh2RlYiSDVEwHDY2HZ1a4bt9hyvKZiItIC+mWsLWK+dGCNXO/zTC nQYg== X-Gm-Message-State: AOAM531ZvEdD+qFkXwyFwbhXeaXcZU4W+A/15gk3sKTmDavbGGiS86Uo xSPizC8EW/XEhDyIdsOftcywiI+W55C7/GRrUb0qZg== X-Google-Smtp-Source: ABdhPJy15byX1aWWVX00tyn9bEIm7HGHGapXKfnt4VMbKaXCo/l87AbyBt59WzwTOO3MF4VIWcfe2IoT3/SjA9MNraI= X-Received: by 2002:a81:364e:0:b0:2e5:475d:c2f8 with SMTP id d75-20020a81364e000000b002e5475dc2f8mr9052757ywa.97.1648191468840; Thu, 24 Mar 2022 23:57:48 -0700 (PDT) MIME-Version: 1.0 References: <20220324135443.1571-1-masahisa.kojima@linaro.org> <20220325012020.GA42849@laputa> In-Reply-To: <20220325012020.GA42849@laputa> From: Masahisa Kojima Date: Fri, 25 Mar 2022 15:57:37 +0900 Message-ID: Subject: Re: [PATCH v4 00/11] enable menu-driven boot device selection To: Takahiro Akashi , Masahisa Kojima , u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Simon Glass , Francois Ozog , Mark Kettenis Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Hi Akashi-san, On Fri, 25 Mar 2022 at 10:20, Takahiro Akashi 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. > > 1) 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 : debian > is a user-defined boot option, while > > UEFI BOOT0002 : mmc0:1 > is 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 : ubuntu > in an order of "BootOrder", > > and > /* 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. > > 2) 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 : dhcp > > > > Press 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)] > > 1) clone source code > > $ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ > > -b kojima/bootmenu_v4_upstream_0324 --depth 1 > > > > 2) prepare U-Boot .config > > $ make qemu_arm64_menuconfig > > then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW > > > > 3) 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 > >