[PATCH v7 00/17] efi_loader: add capsule update support

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Hi Tom,

On Thu, Oct 29, 2020 at 10:43:01AM -0400, Tom Rini wrote:
> On Thu, Oct 29, 2020 at 01:47:40PM +0900, AKASHI Takahiro wrote:
> 
> > Summary
> > =======
> > 'UpdateCapsule' is one of runtime services defined in UEFI specification
> > and its aim is to allow a caller (OS) to pass information to the firmware,
> > i.e. U-Boot. This is mostly used to update firmware binary on devices by
> > instructions from OS.
> > 
> > While 'UpdateCapsule' is a runtime services function, it is, at least
> > initially, supported only before exiting boot services alike other runtime
> > functions, [Get/]SetVariable. This is because modifying storage which may
> > be shared with OS must be carefully designed and there is no general
> > assumption that we can do it.
> > 
> > Therefore, we practically support only "capsule on disk"; any capsule can
> > be handed over to UEFI subsystem as a file on a specific file system.
> > 
> > In this patch series, all the related definitions and structures are given
> > as UEFI specification describes, and basic framework for capsule support
> > is provided. Currently supported is
> >  * firmware update (Firmware Management Protocol or simply FMP)
> > 
> > Most of functionality of firmware update is provided by FMP driver and
> > it can be, by nature, system/platform-specific. So you can and should
> > implement your own FMP driver(s) based on your system requirements.
> > Under the current implementation, we provide two basic but generic
> > drivers with two formats:
> >   * FIT image format (as used in TFTP update and dfu)
> >   * raw image format
> > 
> > It's totally up to users which one, or both, should be used on users'
> > system depending on user requirements.
> > 
> > Quick usage
> > ===========
> > 1. You can create a capsule file with the following host command:
> > 
> >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>
> > 
> > 2. Put the file under:
> > 
> >   /EFI/UpdateCapsule of UEFI system partition
> > 
> > 3. Specify firmware storage to be updated in "dfu_alt_info" variable
> >    (Please follow README.dfu for details.)
> > 
> >   ==> env set dfu_alt_info '...'
> > 
> > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:
> > 
> >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> > 
> > Patch structure
> > ===============
> > Patch#1-#4,#12: preparatory patches
> > Patch#5-#11,#13: main part of implementation
> > Patch#14-#15: utilities
> > Patch#16-#17: pytests
> > 
> > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule
> > 
> > Prerequisite patches
> > ====================
> > None
> > 
> > Test
> > ====
> > * passed all the pytests which are included in this patch series
> >   on sandbox build locally.
> > 
> > Please note that the capsule pytest itself won't be run in the CI
> > partly because some specific configuration for sandbox build is
> > required and partly because there is a problem with virt-make-fs.
> > See test_efi_capsule_firmware.py.
> 
> Maybe we need to talk about this differently then.  Today, the tests
> start on CI and then fail to run.  That's a problem that needs to be
> fixed, they must "skip" properly.  I would really like to see these
> tests run with every CI loop.  That means doing a "try virt-make-fs, if
> that fails, try sudo, if that fails, skip".  That also means updating
> some other tests that have logic for virt-make-fs, but it then causes CI
> to fail rather than try sudo.

I think that you still misunderstand the point.
Whatever you expect by saying "try virt-make-fs, if that fails, try sudo,
if that fails, skip," Heinrich will *not* accept any test script that
contains "sudo" command.
I can do nothing here unless he changes his mind.

Or do you want to see the solution like "try virt-make-fs, if
that fails, skip"?

-Takahiro Akashi

> What I see as the worst case is that we have tests for this feature that
> are only ever run when someone runs them intentionally, and CI skips
> them all the time.
> 
> -- 
> Tom