[PATCH 04/10] efi_loader: capsule: add capsule_on_disk support

[Sughosh Ganu]

On Fri, 1 May 2020 at 01:22, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:

> On 4/30/20 2:52 PM, Sughosh Ganu wrote:
> >
> > On Tue, 28 Apr 2020 at 05:58, AKASHI Takahiro
> > <takahiro.akashi at linaro.org <mailto:takahiro.akashi at linaro.org>> wrote:
> >
> >     Heinrich,
> >
> >     On Mon, Apr 27, 2020 at 10:28:35PM +0200, Heinrich Schuchardt wrote:
> >     > On 4/27/20 11:48 AM, AKASHI Takahiro wrote:
> >     > > Capsule data can be loaded into the system either via
> UpdateCapsule
> >     > > runtime service or files on a file system (of boot device).
> >     > > The latter case is called "capsules on disk", and actual updates
> >     will
> >     > > take place at the next boot time.
> >     > >
> >     > > In this commit, we will support capsule on disk mechanism.
> >     > >
> >     > > Please note that U-Boot itself has no notion of "boot device" and
> >     > > all the capsule files to be executed will be detected only if
> they
> >     > > are located in a specific directory, \EFI\UpdateCapsule, on a
> device
> >     > > that is identified as a boot device by "BootXXXX" variables.
> >     > >
> >     > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org
> >     <mailto:takahiro.akashi at linaro.org>>
> >     > > ---
> >     > >  common/main.c                |   4 +
> >     > >  include/efi_loader.h         |  16 ++
> >     > >  lib/efi_loader/Kconfig       |  22 ++
> >     > >  lib/efi_loader/efi_capsule.c | 449
> >     +++++++++++++++++++++++++++++++++++
> >     > >  lib/efi_loader/efi_setup.c   |   9 +
> >     > >  5 files changed, 500 insertions(+)
> >     > >
> >     > > diff --git a/common/main.c b/common/main.c
> >     > > index 06d7ff56d60c..877ae63b708d 100644
> >     > > --- a/common/main.c
> >     > > +++ b/common/main.c
> >     > > @@ -14,6 +14,7 @@
> >     > >  #include <env.h>
> >     > >  #include <init.h>
> >     > >  #include <version.h>
> >     > > +#include <efi_loader.h>
> >     > >
> >     > >  static void run_preboot_environment_command(void)
> >     > >  {
> >     > > @@ -51,6 +52,9 @@ void main_loop(void)
> >     > >     if (IS_ENABLED(CONFIG_UPDATE_TFTP))
> >     > >             update_tftp(0UL, NULL, NULL);
> >     > >
> >     > > +   if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> >     > > +           efi_launch_capsules();
> >     > > +
> >     >
> >     > Can't we move this to efi_init_obj_list() and do away with
> >     > CONFIG_EFI_CAPSULE_ON_DISK_EARLY?
> >
> >     With CONFIG_EFI_CAPSULE_ON_DISK_EARLY disabled,
> >     efi_launch_capsules() will be called in efi_init_obj_list()
> >     as you expect. See the code below in efi_setup.c.
> >
> >
> > Instead of calling efi_launch_capsules in efi_init_obj_list, can we
> > invoke the function explicitly through a dedicated command line, under
> > the 'efidebug capsule' class of commands. I think that would be a
> > cleaner approach, since efi_init_obj_list gets called for a lot of efi
> > functions, which are unrelated to capsule update.
>
> Who would invoke that command line on an IoT device?
>

Understand your point, but the 'efidebug capsule' class of commands are
anyways going to be used for debug purpose -- there is also an 'efidebug
capsule update' command that is being added as part of this patch series.
My point was that it is better to call the capsule update explicitly rather
than through efi_init_obj_list, which also gets called for all types of
unrelated commands like printenv -e.


> My understanding of the UEFI spec is that capsule updates should be
> invoked automatically.
>

Right, and in that case the capsule update should be invoked automatically
at some point during the boot. Takahiro has added a patch which invokes the
capsule update as part of the main_loop. In that case, why should it be
invoked from efi_init_obj_list. What is the scenario where the capsule
update is required to be invoked as part of the efi_init_obj_list function.

-sughosh