Re: [PATCH v2 09/10] Documentation: efi: describe load options handling

[Marco Felsch <m.felsch@pengutronix.de>]

On 26-10-15 11:18, Ahmad Fatoum wrote:
> Document that arguments following an EFI application's path on the shell
> command line become its load options, that bootm populates the load options
> from global.efi.bootargs.* and that the Linux bootargs follow for
> EFI-stubbed kernels, both for barebox as EFI payload and as EFI loader.
> 
> Assisted-by: Claude:fable-5
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  Documentation/boards/efi.rst         | 64 +++++++++++++++++++++++++++-
>  Documentation/user/booting-linux.rst |  6 ++-
>  2 files changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/boards/efi.rst b/Documentation/boards/efi.rst
> index 5f7dd60998dc..2dde52bc6665 100644
> --- a/Documentation/boards/efi.rst
> +++ b/Documentation/boards/efi.rst
> @@ -112,8 +112,70 @@ means passing control to the calling instance. Note that currently the :ref:`com
>  command will pass the control to the calling instance rather than resetting
>  the CPU. This may change in the future.
>  
> +Arguments following the path on the command line are passed to the
> +application as its load options, so an EFI shell can for example be
> +started with:
> +
> +.. code-block:: sh
> +
> +  barebox:/ /boot/shell.efi -nostartup
> +
>  Although the Linux Kernel can be directly executed one should use the :ref:`command_bootm`
> -command. Only the bootm command passes the Kernel commandline to the Kernel.
> +command. Only the bootm command passes the Kernel commandline to the Kernel
> +and provides it with an initrd and a device tree.
> +
> +.. _efi_load_options:
> +
> +Load options
> +^^^^^^^^^^^^
> +
> +When booting an EFI application via :ref:`command_bootm`, barebox populates
> +its load options from the :ref:`global device<global_device>` variables
> +beginning with ``global.efi.bootargs.``. Like the Linux bootargs, these are
> +concatenated in lexicographical order of their names. For EFI-stubbed Linux
> +kernels, the ``global.linux.bootargs.``-prefixed
> +:ref:`kernel arguments <booting_linux>` are appended to the load options,
> +so that the EFI stub sees both as the kernel command line:
> +
> +.. code-block:: sh
> +
> +  global efi.bootargs.base="efi=debug"
> +  global linux.bootargs.base="console=ttyS0,115200"
> +
> +  bootm /boot/vmlinuz.efi
> +
> +  ...
> +
> +  Kernel command line: efi=debug console=ttyS0,115200
> +
> +This works both when barebox runs as EFI payload and when it acts as
> +EFI loader for the application. The load options are not used when booting
> +x86 kernels via the legacy handover protocol (``global.linux.efi.handover``),
> +as the EFI stub, which would interpret them, is bypassed in that case.
> +
> +Like :ref:`boot entries <boot_entries>` do for the Linux
> +bootargs, boot entries should add load options to variables with ``.dyn``
> +in their name, e.g. ``global.efi.bootargs.dyn.initrd``. These are cleared
> +after each boot entry, so that load options do not leak into subsequently
> +booted entries.
> +
> +Unified kernel images (UKIs) are not detected as Linux kernels, but as
> +regular EFI applications. Only ``global.efi.bootargs.*`` is passed to them
> +as load options; the Linux bootargs, including the ``root=`` option added
> +by ``global.bootm.appendroot``, are not appended. systemd-stub uses
> +non-empty load options in place of the command line embedded in the UKI,
> +unless Secure Boot is enabled and the UKI has an embedded command line, in
> +which case the load options are ignored.
> +
> +To pass a ``root=`` option to a UKI, have :ref:`command_devlookup`
> +generate it for the root partition, the same way ``global.bootm.appendroot``
> +would, including the option name from ``global.bootm.root_param``:
> +
> +.. code-block:: sh
> +
> +  devlookup -v rootarg -k /dev/disk0.1
> +  global efi.bootargs.root="${rootarg}"
> +  bootm /boot/EFI/Linux/uki.efi

Just a comment on this: devlookup can't be compared to the appendroot
mechansim, due to the partition enumeration mismatch between barebox and
linux, e.g. mmc block device partitions start at 0, whereas linux starts
counting at 1.

On embedded devices booting from an MMC device the root=/dev/mmcblk*
mechanism should be used if the device supports dual boot e.g. from SD
and from eMMC.

Therefore I would have liked to see that the appendroot mechanism would
be supported for UKIs as well which would stop the requirement of having
different UKI profiles just to encode the different boot mediums.

However, as said this is just a comment and doesn't block this patch:

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

>  Drivers
>  ^^^^^^^
> diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
> index 8e6f3bbbe9ce..dc7ac6dc8716 100644
> --- a/Documentation/user/booting-linux.rst
> +++ b/Documentation/user/booting-linux.rst
> @@ -103,6 +103,9 @@ to the bootargs. Variables without a value are skipped:
>  
>    Kernel command line: ignore_loglevel console=ttyO0,115200 earlyprintk
>  
> +When booting an EFI-stubbed kernel as EFI application, the
> +:ref:`EFI load options <efi_load_options>` precede the Linux bootargs.
> +
>  .. _bootargs_concat_order:
>  
>  Concatenation order
> @@ -170,7 +173,8 @@ This takes the kernel from ``/mnt/mmc1/zImage`` (which could be an
>  boot entries should always add Kernel command line parameters to variables with
>  ``.dyn`` in it. These will be cleared before booting different boot entries.
>  This is done so that following boot entries do not leak command line
> -parameters from the previous boot entries.
> +parameters from the previous boot entries. The same applies to the
> +:ref:`EFI load options <efi_load_options>` in ``global.efi.bootargs.dyn.*``.
>  
>  This entry can be booted with ``boot mmc``. It can also be made the default by
>  setting the :ref:`global.boot.default <magicvar_global_boot_default>` variable
> -- 
> 2.47.3
> 
>