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

Wed Nov 18 15:49:22 CET 2020

On Wed, Nov 18, 2020 at 12:11:29PM +0900, AKASHI Takahiro wrote:
> On Tue, Nov 17, 2020 at 10:02:41PM -0500, Tom Rini wrote:
> > On Wed, Nov 18, 2020 at 09:26:38AM +0900, AKASHI Takahiro wrote:
> > > 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?
> > 
> > Given that it takes Travis-CI several hours to complete a run, and Azure
> > around 2 hours, most of the time it's fine the CI runs to get put off
> > until the PR is being made.  In _this_ case you're adding tests, and
> > they had previously caused CI to fail, so you needed to get CI to work.
> 
> Even if we don't know how we can kick off CI jobs and see the results?
> That's why I request you to make information/documents publicly available.

Yes, it would be good to add a section to the rST about CI.  I've noted
how to trigger Azure in a few threads a few times, outside of this
thread.

> > > Heinrich requires CI results, but many custodians not.
> > > Why this inconsistency?
> 
> You didn't give me the answer for this.

Yes, I did.  You, right here, are adding tests.  That's why you need to
run CI.  Custodians who don't run CI before sending me a PR, and who
then have a problem CI catches, get the PR nak'd.  Most changes are
localized and don't break builds outside of the SoC being changed.

> Moreover, it's not very clear in which case you demand test cases being added.
> Again, that's why I request you to add information/documents about
> the criteria for "testing before submission".

Well, how important is it to YOU that the EFI capsule code doesn't get
broken?  Code that we can run tests on in CI should get tests run in CI.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201118/10ddbbdb/attachment.sig>