[PATCH v8 00/18] efi_loader: add capsule update support

Wed Nov 18 01:26:38 CET 2020

On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:
> On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:
> > Tom, Heinrich,
> > 
> > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:
> > > Hi Tom,
> > > 
> > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:
> > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:
> > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:
> > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:
> > > > > > > Heinrich,
> > > > > > > 
> > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:
> > > > > > > > On 11/13/20 5:14 AM, 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
> > > > > > > > > Patch#18: for sandbox test
> > > > > > > > >
> > > > > > > > > [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.
> > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because
> > > > > > > > >   "virt-make-fs" cannot be executed.
> > > > > > > > 
> > > > > > > > Dear Takahiro,
> > > > > > > > 
> > > > > > > > please, rebase your series on origin/master. A prior version of the
> > > > > > > > first patches is already applied.
> > > > > > > 
> > > > > > > You can simply pick up and apply non-merged patches without problems
> > > > > > > except a function prototype change of efi_create_indexed_name() you made,
> > > > > > > which is not trivial to me.
> > > > > > > 
> > > > > > > But anyway, I will post a clean patch set soon.
> > > > > > > 
> > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging
> > > > > > > > the remaining patches.
> > 
> > @Heinrich,
> > 
> > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907
> > https://github.com/u-boot/u-boot/pull/39
> > 
> > Do those results satisfy your request?
> > I don't know how I can deal with gitlab and amazon CI.
> 
> I believe that was a typo of "Azure", which you have above.  And since
> https://github.com/u-boot/u-boot/pull/39/checks shows green, I would
> expect that so long as you linted your .gitlab-ci.yml changes, if any,
> then it too should pass.
> 
> > > > > > > Where can we find the results?
> > > > > > > I don't think there is no official information on those CI's.
> > > > > > 
> > > > > > If you submit a pull request against https://github.com/u-boot/u-boot
> > > > > > Azure will run automatically and show the results.  We don't have
> > > > > 
> > > > > We can get a free account for Azure, but yet it requires us to give
> > > > > our credit card information to Azure. I don't want to accept it.
> > > > > So I believe requiring Azure test results is just inappropriate.
> > > > 
> > > > That's not correct, as far as I can tell.  You just need to submit a
> > > > pull request against the repository I mentioned above and that triggers
> > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is
> > > > the result of one such PR.
> > > 
> > > Okay, I will check and try.
> > > But I hope that you should have, on the home page?, more documents
> > > on those CI loops and requirements of test results as part of
> > > submission process.
> > 
> > @Tom,
> > 
> > Again, I'd like you to add a rule and a related document with regard to
> > criteria for "testing before submission". Otherwise, it's confusing.
> 
> I'm sorry you found it confusing.  It has been stated many times in
> these threads that since you're adding tests, they're expected to not
> break CI, and should even run when possible.  Updating the rST with a CI
> section would be good, yes.

For clarity, my claim above is not only for me, but also for "all"
contributors.
When I looked at
https://github.com/u-boot/u-boot/pulls
I was a bit disappointed as there are so few people who have actually
tried to invoke Azure CI (prior to their submissions?).
Heinrich requires CI results, but many custodians not.
Why this inconsistency?

-Takahiro Akashi

> > @Heinrich,
> > 
> > As I requested several times, please clarify your view on virt-make-fs/sudo
> > issue and have consensus on it with Tom.
> 
> Yes, please.
> 
> -- 
> Tom