[PATCH v6 5/6] doc:bootmenu: add description for UEFI boot support

[Masahisa Kojima]

Hi Akashi-san,

On Wed, 18 May 2022 at 10:31, Takahiro Akashi
<takahiro.akashi at linaro.org> wrote:
>
> Kojima-san,
>
> On Mon, May 16, 2022 at 08:00:41PM +0900, Masahisa Kojima wrote:
> > The bootmenu enumerates the UEFI boot options
> > for boot device selection.
> >
> > This commit adds the description how the UEFI boot work
> > in bootmenu. This commit also adds "Synopsis", "Description"
> > and "Configuration" sections to follow the U-Boot command
> > documentation format.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Changes in v6:
> > - remove distro boot related contents because the distro boot
> > support in bootmenu is dropped
>
> ? I rejected it in my comment.
> I don't think we have enough consensus yet.

I am aware of your comment and discussion in U-Boot ML.

In my opinion, I doubt that the distro boot users use "bootmenu"
to select the device to invoke "run bootcmd_xxx". I guess no.
And my primary goal is disabling the U-Boot console for UEFI based
system. Distro boot users can still live without the U-Boot console
without bootmenu support.

Distro boot support in bootmenu is not planned initially at my side,
but it was added during code review process, then it is
(temporary) dropped in the end. I don't have a strong opinion about it.

Anyway, distro boot support in bootmenu is an individual patch,
I am happy to re-send if needed.

>
> > - update uefi entry example
> > - add [delay] argument of bootmenu command
> > - add description to enable uefi boot entry
> >
> > Changes in v5:
> > - follow the cmd documentation format same as other command, add "Synopsis",
> >   "Description" add "Configuration" sections
> >
> > Newly created in v4
> >
> >  doc/usage/cmd/bootmenu.rst | 55 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >
> > diff --git a/doc/usage/cmd/bootmenu.rst b/doc/usage/cmd/bootmenu.rst
> > index 9430f8c9aa..6b154d9655 100644
> > --- a/doc/usage/cmd/bootmenu.rst
> > +++ b/doc/usage/cmd/bootmenu.rst
> > @@ -4,6 +4,16 @@
> >  bootmenu command
> >  ================
> >
> > +Synopsis
> > +--------
> > +
> > +::
> > +
> > +    bootmenu [delay]
> > +
> > +Description
> > +-----------
> > +
> >  The "bootmenu" command uses U-Boot menu interfaces and provides
> >  a simple mechanism for creating menus with different boot items.
> >  The cursor keys "Up" and "Down" are used for navigation through
> > @@ -79,6 +89,35 @@ The above example will be rendered as below::
> >  The selected menu entry will be highlighted - it will have inverted
> >  background and text colors.
> >
> > +UEFI boot variable enumeration
> > +''''''''''''''''''''''''''''''
> > +
> > +The bootmenu automatically generates the UEFI boot variable("BOOT####")
> > +in order of "BootOrder".
>
> To be strict, the bootmenu command *does not* create UEFI boot variables
> (except ones for removable media).
> So, I would suggest you to modify the text like:
>
> If enabled, the bootmenu command will automatically generate and add
> UEFI-related boot menu entries for
> * possible bootable media with default file names
> * user-defined UEFI boot options
>
> Then,
>
> > +The bootmenu automatically enumerates the possible bootable
> > +media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
> > +This auto generated entry is named as "<interface> <devnum>:<part>" format.
> > +(e.g. "usb 0:1")
> > +

Thank you, I agree.

>
> > When the user selects the UEFI boot
> > +variable entry, bootmenu sets the selected boot variable index
> > +to "BootNext", then call the uefi boot manager with the command
> > +"bootefi bootmgr".
> > +
> > +Example bootmenu is as below::
> > +
> > +    *** U-Boot Boot Menu ***
> > +
> > +       mmc 0:1
> > +       mmc 0:2
> > +       debian
> > +       nvme 0:1
> > +       ubuntu
> > +       nvme 0:2
> > +       usb 0:2
>
> If I understand your patch correctly, menus are initially sorted out
> in the order of
> * user-defined bootmenu_xxx variables
> * removable bootable media
> * user-defined UEFI boot options
>
> The example above looks odd.

The example is intended and same as actual behavior.

Sorry for confusing you, the function name and description of
prepare_media_device_entry() is wrong. This function just checks
the removable bootable media, then updates UEFI boot
variables(BOOTXXXX and BootOrder).

prepare_uefi_bootorder_entry() enumerates the UEFI related
entries in the order of BootOrder. So removable bootable media
entries and user-defined UEFI boot option entries are mixed in the
bootmenu.

>
> That said, it would be worth mentioning that users can changes the order
> among UEFI-related menus. (Right?)

OK.

>
> In addition, please describe the default action if users do not select
> anything.

Yes, It should be noted.

Thanks,
Masahisa Kojima

>
> -Takahiro Akashi
>
>
> > +
> > +Configuration
> > +-------------
> > +
> >  The "bootmenu" command is enabled by::
> >
> >      CONFIG_CMD_BOOTMENU=y
> > @@ -88,3 +127,19 @@ To run the bootmenu at startup add these additional settings::
> >      CONFIG_AUTOBOOT_KEYED=y
> >      CONFIG_BOOTDELAY=30
> >      CONFIG_AUTOBOOT_MENU_SHOW=y
> > +
> > +UEFI boot variable enumeration is enabled by::
> > +
> > +    CONFIG_AUTOBOOT_MENU_SHOW=y
> > +
> > +To improve the product security, entering U-Boot console from bootmenu
> > +can be disabled by::
> > +
> > +    CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE=n
> > +
> > +To scan the discoverable devices connected to the buses such as
> > +USB and PCIe prior to bootmenu showing up, CONFIG_PREBOOT can be
> > +used to run the command before showing the bootmenu, i.e.::
> > +
> > +    CONFIG_USE_PREBOOT=y
> > +    CONFIG_PREBOOT="pci enum; usb start; scsi scan; nvme scan; virtio scan"
> > --
> > 2.17.1
> >