Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
On Thu, Oct 29, 2020 at 01:40:31PM +0900, AKASHI Takahiro wrote: > On Wed, Oct 28, 2020 at 11:22:29PM -0400, Tom Rini wrote: > > On Thu, Oct 29, 2020 at 09:59:12AM +0900, AKASHI Takahiro wrote: > > > Tom, > > > > > > On Wed, Oct 28, 2020 at 08:30:49PM -0400, Tom Rini wrote: > > > > On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > > > > > > > > > # > > > > > # This is a reminder. Nothing changed, but rebasing. > > > > > # > > > > > > > > Wasn't one of the outstanding requests to make the DFU related changes > > > > less intrusive and thus easier to review > > > > > > I don't know what you're talking about. > > > I believe that I have addressed all the comments made on DFU stuff. > > > > Please go back and re-read the v5 thread then. What you're doing today > > causes odroid, trats, trats2 and s5p_goni to fail to build now and I > > believe there was a suggestion what to do to fix that, previously. > > Okay, I have missed it. > Is this the reason that I have seen your review comments in more than > one month? To be clear, I reported a fail to build at the time and yes, I was waiting for that to be addressed before reviewing everything else. -- Tom signature.asc Description: PGP signature
Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
On Wed, Oct 28, 2020 at 11:22:29PM -0400, Tom Rini wrote: > On Thu, Oct 29, 2020 at 09:59:12AM +0900, AKASHI Takahiro wrote: > > Tom, > > > > On Wed, Oct 28, 2020 at 08:30:49PM -0400, Tom Rini wrote: > > > On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > > > > > > > # > > > > # This is a reminder. Nothing changed, but rebasing. > > > > # > > > > > > Wasn't one of the outstanding requests to make the DFU related changes > > > less intrusive and thus easier to review > > > > I don't know what you're talking about. > > I believe that I have addressed all the comments made on DFU stuff. > > Please go back and re-read the v5 thread then. What you're doing today > causes odroid, trats, trats2 and s5p_goni to fail to build now and I > believe there was a suggestion what to do to fix that, previously. Okay, I have missed it. Is this the reason that I have seen your review comments in more than one month? Anyhow, I will post v7 right after this. -Takahiro Akashi > > > > There used to be a prerequisite patch from Heinrich, but he decided > > to drop it. Necessary modification was done in v5. > > > > > by someone (me) who is not the > > > primary author of the DFU code but does want to unblock this overall > > > series right here? > > > > So you can review DFU part now (or even against the original v6). > > # Another minor issue: the name of file, dfu_alt.c > > # It will never be a blocking factor of your review, though. > > > > The only outstanding issue is on make-virt-fs[1]. > > Here, my standpoint is clear: Heinrich's patch. > > It is the only solution as far as Heinrich does reject any other solution > > using 'sudo' in python scripts. > > Which is I guess why the tests fail to run here: > https://gitlab.denx.de/u-boot/u-boot/-/jobs/171480 > and why I think I had suggested that you need to do some try/catch logic > so that if one set of tools isn't available, the other set of tools will > be used. > > -- > Tom
Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
Tom, On Wed, Oct 28, 2020 at 11:22:29PM -0400, Tom Rini wrote: > On Thu, Oct 29, 2020 at 09:59:12AM +0900, AKASHI Takahiro wrote: > > Tom, > > > > On Wed, Oct 28, 2020 at 08:30:49PM -0400, Tom Rini wrote: > > > On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > > > > > > > # > > > > # This is a reminder. Nothing changed, but rebasing. > > > > # > > > > > > Wasn't one of the outstanding requests to make the DFU related changes > > > less intrusive and thus easier to review > > > > I don't know what you're talking about. > > I believe that I have addressed all the comments made on DFU stuff. > > Please go back and re-read the v5 thread then. What you're doing today > causes odroid, trats, trats2 and s5p_goni to fail to build now and I > believe there was a suggestion what to do to fix that, previously. > > > > > There used to be a prerequisite patch from Heinrich, but he decided > > to drop it. Necessary modification was done in v5. > > > > > by someone (me) who is not the > > > primary author of the DFU code but does want to unblock this overall > > > series right here? > > > > So you can review DFU part now (or even against the original v6). > > # Another minor issue: the name of file, dfu_alt.c > > # It will never be a blocking factor of your review, though. > > > > The only outstanding issue is on make-virt-fs[1]. > > Here, my standpoint is clear: Heinrich's patch. > > It is the only solution as far as Heinrich does reject any other solution > > using 'sudo' in python scripts. > > Which is I guess why the tests fail to run here: > https://gitlab.denx.de/u-boot/u-boot/-/jobs/171480 > and why I think I had suggested that you need to do some try/catch logic > so that if one set of tools isn't available, the other set of tools will > be used. I think you misunderstand my point. Whatever logic I add, Heinrich will reject that solution if it includes "sudo". As far as I can imagine, "sudo" is necessary to workaround the issue because we need some way to create a filesystem with partitions. Whether a tool, make-virt-fs in this case, is available or not doesn't matter. -Takahiro Akashi > -- > Tom
Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
On Thu, Oct 29, 2020 at 09:59:12AM +0900, AKASHI Takahiro wrote: > Tom, > > On Wed, Oct 28, 2020 at 08:30:49PM -0400, Tom Rini wrote: > > On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > > > > > # > > > # This is a reminder. Nothing changed, but rebasing. > > > # > > > > Wasn't one of the outstanding requests to make the DFU related changes > > less intrusive and thus easier to review > > I don't know what you're talking about. > I believe that I have addressed all the comments made on DFU stuff. Please go back and re-read the v5 thread then. What you're doing today causes odroid, trats, trats2 and s5p_goni to fail to build now and I believe there was a suggestion what to do to fix that, previously. > > There used to be a prerequisite patch from Heinrich, but he decided > to drop it. Necessary modification was done in v5. > > > by someone (me) who is not the > > primary author of the DFU code but does want to unblock this overall > > series right here? > > So you can review DFU part now (or even against the original v6). > # Another minor issue: the name of file, dfu_alt.c > # It will never be a blocking factor of your review, though. > > The only outstanding issue is on make-virt-fs[1]. > Here, my standpoint is clear: Heinrich's patch. > It is the only solution as far as Heinrich does reject any other solution > using 'sudo' in python scripts. Which is I guess why the tests fail to run here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/171480 and why I think I had suggested that you need to do some try/catch logic so that if one set of tools isn't available, the other set of tools will be used. -- Tom signature.asc Description: PGP signature
Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
Tom, On Wed, Oct 28, 2020 at 08:30:49PM -0400, Tom Rini wrote: > On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > > > # > > # This is a reminder. Nothing changed, but rebasing. > > # > > Wasn't one of the outstanding requests to make the DFU related changes > less intrusive and thus easier to review I don't know what you're talking about. I believe that I have addressed all the comments made on DFU stuff. There used to be a prerequisite patch from Heinrich, but he decided to drop it. Necessary modification was done in v5. > by someone (me) who is not the > primary author of the DFU code but does want to unblock this overall > series right here? So you can review DFU part now (or even against the original v6). # Another minor issue: the name of file, dfu_alt.c # It will never be a blocking factor of your review, though. The only outstanding issue is on make-virt-fs[1]. Here, my standpoint is clear: Heinrich's patch. It is the only solution as far as Heinrich does reject any other solution using 'sudo' in python scripts. -Takahiro Akashi [1] https://lists.denx.de/pipermail/u-boot/2020-September/426250.html > -- > Tom
Re: [RESEND PATCH v6 00/17] efi_loader: add capsule update support
On Thu, Oct 29, 2020 at 09:25:11AM +0900, AKASHI Takahiro wrote: > # > # This is a reminder. Nothing changed, but rebasing. > # Wasn't one of the outstanding requests to make the DFU related changes less intrusive and thus easier to review by someone (me) who is not the primary author of the DFU code but does want to unblock this overall series right here? -- Tom signature.asc Description: PGP signature
[RESEND PATCH v6 00/17] efi_loader: add capsule update support
# # This is a reminder. Nothing changed, but rebasing. # 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 | --raw ] 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 In this version, the DFU change by Heinrich[2] is, at least temporarily, not a prerequisite due to the discussions[3]. [2] https://lists.denx.de/pipermail/u-boot/2020-July/420950.html [3] https://lists.denx.de/pipermail/u-boot/2020-August/424716.html 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. Issues == * Timing of executing capsules-on-disk Currently, processing a capsule is triggered only as part of UEFI subsystem initialization. This means that, for example, firmware update, may not take place at system booting time and will potentially be delayed until a first call of any UEFI functions. => See patch#5 for my proposal * A bunch of warnings like WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible I don't think that fixing those improves anything. * Add a document in uefi.rst TODO's == (Won't be addressed in this series.) * capsule authentication * capsule dependency (dependency expression instruction set, or depex) * loading drivers in a capsule * handling RESET flag in a capsule and QeuryCapsuleCaps * full semantics of ESRT (EFI System Resource Table) * enabling capsule API at runtime * json capsule * recovery from update failure Changes === v6 RESEND (October 29, 2020) * rebased on v2021.01-rc1 v6 (September 7, 2020) * temporarily drop the prerequisite patch[2] * add a missing field (dependencies) in efi_api.h (but never used) (Patch#10) * add a missing field (image_capsule_support) and related definitions in efi_api.h (Patch#10, #15) * cosmetic changes on constant definitions in efi_api.h (Patch#10) * strict check for INVALID_PARAMETER at GET_IMAGE_INFO api (Patch#11,#13) * fix warnings in pytest (Patch#16,#17) v5 (August 3, 2020) * removed superfluous type conversion at dfu_write_from_mem_addr() (Patch#2) * introduced a common helper function, efi_create_indexed_name() (Patch#6,#7,#8) * use efi_[get|set]_variable_int(), if necessary, with READ_ONLY (Patch#7,#8) * return EFI_UNSUPPORTED at Patch#7 * changed the word, number, to 'index' (Patch#7,#8) * removed 'ifdef CONFIG_EFI_CAPSULE_ON_DISK' from a header (Patch#8) * initialize 'CapsuleLast' in efi_init_obj_list() (Patch#7,#8) * added 'const' qualifier for filename argument at efi_capsule_[read|delete]_file() (Patch#8) v4 (July 22, 2020) * rebased to Heinrich's current efi-2020-10 * rework dfu-related code to align with Heinrich's change
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Fri, Sep 11, 2020 at 11:31:11AM +0900, AKASHI Takahiro wrote: > Tom, > > On Thu, Sep 10, 2020 at 09:02:59PM -0400, Tom Rini wrote: > > On Fri, Sep 11, 2020 at 09:52:10AM +0900, AKASHI Takahiro wrote: > > > On Wed, Sep 09, 2020 at 10:58:53PM -0400, Tom Rini wrote: > > > > On Thu, Sep 10, 2020 at 11:52:37AM +0900, AKASHI Takahiro wrote: > > > > > On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > > > > > > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > > > > > > On 07.09.20 07:34, AKASHI Takahiro wrote: > > > > > > [snip] > > > > > > > > required and partly because there is a problem with > > > > > > > > virt-make-fs. > > > > > > > > > > > > > > What problem with virt-make-fs exists? How will this be solved? > > > > > > > > > > > > This I suspect is related to the difficulty of getting tests to run > > > > > > in > > > > > > all of our CI environments, where at least in one case they run and > > > > > > pass, rather than skip. This will require that the series get run > > > > > > through at least Travis and Azure before re-posting, at this point. > > > > > > Thanks. > > > > > > > > > > I think that we discussed this issue several times before. > > > > > > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419430.html > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/420712.html > > > > > > > > > > I have not implemented a fallback handling in case of virt-make-fs > > > > > failure > > > > > as Heinrich doesn't like it (even dropped the code from my patches.) > > > > > > > > > > The only solution is Heinrich's patch: > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419976.html > > > > > > > > The problem is that Heinrich's patch breaks things. A complete solution > > > > is required. > > > > > > What kind of issue did you see? > > > > I believe it's in the thread. But in short, we then stop running the > > other filesystem tests as they get skipped. > > I believe that the reason of failures in filesystem tests is the same; > /boot/vmlinuz* have not 'read' permission to users and any commands > in libguestfs-tools, including guestmount and virt-make-fs, cannot > set up a virtual machine internally used in those commands. > > So again, the only solution is to apply Heinrich's patch. No, I believe I fixed that as part of digging in to this at the time. > > > > That patch might be part of the solution, along with > > > > perhaps a rework of the logic to try and gracefully catch and re-try > > > > when we have a problem with one of the possible tools. > > > > > > Anyway, I can't do anything here as Heinrich rejects any sudo-based > > > solution. See: > > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419951.html > > > > > > So he is responsible for fixing it. > > > > No, it's on you to provide a solution that runs in both CI and developer > > machines. It's not sudo or make-virt-fs, it's get the try/catch logic > > correct in the test framework such that if we can't run sudo > > The point is that Heinrich doesn't accept any sudo solution and that > he modified my tests to solely use virt-make-fs by the patch above. > So what can I do here? > > > we do run > > make-virt-fs, and if we're able to run the tests somehow, we run the > > tests somehow. > > Again, Heinrich's patch will fix it. Please put together a series that passes and runs on CI and doesn't make other tests stop running. That's the requirement. If it doesn't run in CI, it doesn't get run frequently enough. If it makes some CI tests stop running, that's a problem too. -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 00/17] efi_loader: add capsule update support
Tom, On Thu, Sep 10, 2020 at 09:02:59PM -0400, Tom Rini wrote: > On Fri, Sep 11, 2020 at 09:52:10AM +0900, AKASHI Takahiro wrote: > > On Wed, Sep 09, 2020 at 10:58:53PM -0400, Tom Rini wrote: > > > On Thu, Sep 10, 2020 at 11:52:37AM +0900, AKASHI Takahiro wrote: > > > > On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > > > > > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > > > > > On 07.09.20 07:34, AKASHI Takahiro wrote: > > > > > [snip] > > > > > > > required and partly because there is a problem with virt-make-fs. > > > > > > > > > > > > What problem with virt-make-fs exists? How will this be solved? > > > > > > > > > > This I suspect is related to the difficulty of getting tests to run in > > > > > all of our CI environments, where at least in one case they run and > > > > > pass, rather than skip. This will require that the series get run > > > > > through at least Travis and Azure before re-posting, at this point. > > > > > Thanks. > > > > > > > > I think that we discussed this issue several times before. > > > > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419430.html > > > > https://lists.denx.de/pipermail/u-boot/2020-July/420712.html > > > > > > > > I have not implemented a fallback handling in case of virt-make-fs > > > > failure > > > > as Heinrich doesn't like it (even dropped the code from my patches.) > > > > > > > > The only solution is Heinrich's patch: > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419976.html > > > > > > The problem is that Heinrich's patch breaks things. A complete solution > > > is required. > > > > What kind of issue did you see? > > I believe it's in the thread. But in short, we then stop running the > other filesystem tests as they get skipped. I believe that the reason of failures in filesystem tests is the same; /boot/vmlinuz* have not 'read' permission to users and any commands in libguestfs-tools, including guestmount and virt-make-fs, cannot set up a virtual machine internally used in those commands. So again, the only solution is to apply Heinrich's patch. > > > That patch might be part of the solution, along with > > > perhaps a rework of the logic to try and gracefully catch and re-try > > > when we have a problem with one of the possible tools. > > > > Anyway, I can't do anything here as Heinrich rejects any sudo-based > > solution. See: > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419951.html > > > > So he is responsible for fixing it. > > No, it's on you to provide a solution that runs in both CI and developer > machines. It's not sudo or make-virt-fs, it's get the try/catch logic > correct in the test framework such that if we can't run sudo The point is that Heinrich doesn't accept any sudo solution and that he modified my tests to solely use virt-make-fs by the patch above. So what can I do here? > we do run > make-virt-fs, and if we're able to run the tests somehow, we run the > tests somehow. Again, Heinrich's patch will fix it. -Takahiro Akashi > > -- > Tom
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Fri, Sep 11, 2020 at 09:52:10AM +0900, AKASHI Takahiro wrote: > On Wed, Sep 09, 2020 at 10:58:53PM -0400, Tom Rini wrote: > > On Thu, Sep 10, 2020 at 11:52:37AM +0900, AKASHI Takahiro wrote: > > > On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > > > > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > > > > On 07.09.20 07:34, AKASHI Takahiro wrote: > > > > [snip] > > > > > > required and partly because there is a problem with virt-make-fs. > > > > > > > > > > What problem with virt-make-fs exists? How will this be solved? > > > > > > > > This I suspect is related to the difficulty of getting tests to run in > > > > all of our CI environments, where at least in one case they run and > > > > pass, rather than skip. This will require that the series get run > > > > through at least Travis and Azure before re-posting, at this point. > > > > Thanks. > > > > > > I think that we discussed this issue several times before. > > > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419430.html > > > https://lists.denx.de/pipermail/u-boot/2020-July/420712.html > > > > > > I have not implemented a fallback handling in case of virt-make-fs failure > > > as Heinrich doesn't like it (even dropped the code from my patches.) > > > > > > The only solution is Heinrich's patch: > > > https://lists.denx.de/pipermail/u-boot/2020-July/419976.html > > > > The problem is that Heinrich's patch breaks things. A complete solution > > is required. > > What kind of issue did you see? I believe it's in the thread. But in short, we then stop running the other filesystem tests as they get skipped. > > That patch might be part of the solution, along with > > perhaps a rework of the logic to try and gracefully catch and re-try > > when we have a problem with one of the possible tools. > > Anyway, I can't do anything here as Heinrich rejects any sudo-based > solution. See: > > https://lists.denx.de/pipermail/u-boot/2020-July/419951.html > > So he is responsible for fixing it. No, it's on you to provide a solution that runs in both CI and developer machines. It's not sudo or make-virt-fs, it's get the try/catch logic correct in the test framework such that if we can't run sudo we do run make-virt-fs, and if we're able to run the tests somehow, we run the tests somehow. -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Wed, Sep 09, 2020 at 10:58:53PM -0400, Tom Rini wrote: > On Thu, Sep 10, 2020 at 11:52:37AM +0900, AKASHI Takahiro wrote: > > On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > > > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > > > On 07.09.20 07:34, AKASHI Takahiro wrote: > > > [snip] > > > > > required and partly because there is a problem with virt-make-fs. > > > > > > > > What problem with virt-make-fs exists? How will this be solved? > > > > > > This I suspect is related to the difficulty of getting tests to run in > > > all of our CI environments, where at least in one case they run and > > > pass, rather than skip. This will require that the series get run > > > through at least Travis and Azure before re-posting, at this point. > > > Thanks. > > > > I think that we discussed this issue several times before. > > > > https://lists.denx.de/pipermail/u-boot/2020-July/419430.html > > https://lists.denx.de/pipermail/u-boot/2020-July/420712.html > > > > I have not implemented a fallback handling in case of virt-make-fs failure > > as Heinrich doesn't like it (even dropped the code from my patches.) > > > > The only solution is Heinrich's patch: > > https://lists.denx.de/pipermail/u-boot/2020-July/419976.html > > The problem is that Heinrich's patch breaks things. A complete solution > is required. What kind of issue did you see? > That patch might be part of the solution, along with > perhaps a rework of the logic to try and gracefully catch and re-try > when we have a problem with one of the possible tools. Anyway, I can't do anything here as Heinrich rejects any sudo-based solution. See: https://lists.denx.de/pipermail/u-boot/2020-July/419951.html So he is responsible for fixing it. -Takahiro Akashi > -- > Tom
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Thu, Sep 10, 2020 at 11:52:37AM +0900, AKASHI Takahiro wrote: > On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > > On 07.09.20 07:34, AKASHI Takahiro wrote: > > [snip] > > > > required and partly because there is a problem with virt-make-fs. > > > > > > What problem with virt-make-fs exists? How will this be solved? > > > > This I suspect is related to the difficulty of getting tests to run in > > all of our CI environments, where at least in one case they run and > > pass, rather than skip. This will require that the series get run > > through at least Travis and Azure before re-posting, at this point. > > Thanks. > > I think that we discussed this issue several times before. > > https://lists.denx.de/pipermail/u-boot/2020-July/419430.html > https://lists.denx.de/pipermail/u-boot/2020-July/420712.html > > I have not implemented a fallback handling in case of virt-make-fs failure > as Heinrich doesn't like it (even dropped the code from my patches.) > > The only solution is Heinrich's patch: > https://lists.denx.de/pipermail/u-boot/2020-July/419976.html The problem is that Heinrich's patch breaks things. A complete solution is required. That patch might be part of the solution, along with perhaps a rework of the logic to try and gracefully catch and re-try when we have a problem with one of the possible tools. -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Wed, Sep 09, 2020 at 12:56:28PM -0400, Tom Rini wrote: > On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > > On 07.09.20 07:34, AKASHI Takahiro wrote: > [snip] > > > required and partly because there is a problem with virt-make-fs. > > > > What problem with virt-make-fs exists? How will this be solved? > > This I suspect is related to the difficulty of getting tests to run in > all of our CI environments, where at least in one case they run and > pass, rather than skip. This will require that the series get run > through at least Travis and Azure before re-posting, at this point. > Thanks. I think that we discussed this issue several times before. https://lists.denx.de/pipermail/u-boot/2020-July/419430.html https://lists.denx.de/pipermail/u-boot/2020-July/420712.html I have not implemented a fallback handling in case of virt-make-fs failure as Heinrich doesn't like it (even dropped the code from my patches.) The only solution is Heinrich's patch: https://lists.denx.de/pipermail/u-boot/2020-July/419976.html -Takahiro Akashi > -- > Tom
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > On 07.09.20 07:34, 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 | --raw ] > > > > 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 > > > > In this version, the DFU change by Heinrich[2] is, at least temporarily, > > not a prerequisite due to the discussions[3]. > > > > [2] https://lists.denx.de/pipermail/u-boot/2020-July/420950.html > > [3] https://lists.denx.de/pipermail/u-boot/2020-August/424716.html > > The end of the discussion was that the patch is postponed to v2021.01 > like your series. My understanding was that the patch will be deferred until v2021.01 and we need more discussions. Right? > > > > 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 > > Shouldn't the sandbox_defconfig changes be added to your series? I won't hesitate to add them if Simon agrees. Since the current test removed the dependency on ENV_IS_IN_SPI_FLASH which was required in v3 or early, it would be more acceptable. > Is this the correct change: It would be fine. > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig > index 6e9f029cc9..da82afaa01 100644 > --- a/configs/sandbox_defconfig > +++ b/configs/sandbox_defconfig > @@ -129,6 +129,7 @@ CONFIG_DM_DEMO_SIMPLE=y > CONFIG_DM_DEMO_SHAPE=y > CONFIG_BOARD=y > CONFIG_BOARD_SANDBOX=y > +CONFIG_DFU_SF=y > CONFIG_DMA=y > CONFIG_DMA_CHANNELS=y > CONFIG_SANDBOX_DMA=y > @@ -261,6 +262,10 @@ CONFIG_CMD_DHRYSTONE=y > CONFIG_TPM=y > CONFIG_LZ4=y > CONFIG_ERRNO_STR=y > +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y > +CONFIG_EFI_CAPSULE_ON_DISK=y > +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y > +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y > CONFIG_EFI_SECURE_BOOT=y > CONFIG_TEST_FDTDEC=y > CONFIG_UNIT_TEST=y > > > required and partly because there is a problem with virt-make-fs. > > What problem with virt-make-fs exists? How will this be solved? See my reply to Tom. -Takahiro Akashi > > See test_efi_capsule_firmware.py. > > > > Issues > > == > > * Timing of executing capsules-on-disk > > Currently, processing a capsule is triggered only as part of > > UEFI subsystem initialization. This means that, for example, > > firmware update, may not take place at system booting time and > > will potentially be delayed until a first call of any UEFI functions. > > => See patch#5 for my proposal > > * A bunch of warnings like > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On Wed, Sep 09, 2020 at 04:48:30PM +0200, Heinrich Schuchardt wrote: > On 07.09.20 07:34, AKASHI Takahiro wrote: [snip] > > required and partly because there is a problem with virt-make-fs. > > What problem with virt-make-fs exists? How will this be solved? This I suspect is related to the difficulty of getting tests to run in all of our CI environments, where at least in one case they run and pass, rather than skip. This will require that the series get run through at least Travis and Azure before re-posting, at this point. Thanks. -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 00/17] efi_loader: add capsule update support
On 07.09.20 07:34, 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 | --raw ] > > 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 > > In this version, the DFU change by Heinrich[2] is, at least temporarily, > not a prerequisite due to the discussions[3]. > > [2] https://lists.denx.de/pipermail/u-boot/2020-July/420950.html > [3] https://lists.denx.de/pipermail/u-boot/2020-August/424716.html The end of the discussion was that the patch is postponed to v2021.01 like your series. > > 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 Shouldn't the sandbox_defconfig changes be added to your series? Is this the correct change: diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 6e9f029cc9..da82afaa01 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -129,6 +129,7 @@ CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y CONFIG_BOARD=y CONFIG_BOARD_SANDBOX=y +CONFIG_DFU_SF=y CONFIG_DMA=y CONFIG_DMA_CHANNELS=y CONFIG_SANDBOX_DMA=y @@ -261,6 +262,10 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y CONFIG_ERRNO_STR=y +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y +CONFIG_EFI_CAPSULE_ON_DISK=y +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y > required and partly because there is a problem with virt-make-fs. What problem with virt-make-fs exists? How will this be solved? > See test_efi_capsule_firmware.py. > > Issues > == > * Timing of executing capsules-on-disk > Currently, processing a capsule is triggered only as part of > UEFI subsystem initialization. This means that, for example, > firmware update, may not take place at system booting time and > will potentially be delayed until a first call of any UEFI functions. > => See patch#5 for my proposal > * A bunch of warnings like > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' > where possible > I don't think that fixing those improves anything. Please, use 'if' instead of '#if' where possible so that the compiler checks the code syntax even if the configuration is not set. Like with '#if' the compiler will eliminate unused code. Best regards Heinrich > * Add a document in uefi.rst > > TODO's > == > (Won't be addressed in this series.) > * capsule authentication > * capsule dependency (dependency expression instruction set, or depex) > * loading drivers in a capsule > * handling RESET flag in a capsule and QeuryCapsuleCaps > * full semantics of ESRT (EFI System Resource Table) > * enabling
[PATCH v6 00/17] efi_loader: add capsule update support
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 | --raw ] 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 In this version, the DFU change by Heinrich[2] is, at least temporarily, not a prerequisite due to the discussions[3]. [2] https://lists.denx.de/pipermail/u-boot/2020-July/420950.html [3] https://lists.denx.de/pipermail/u-boot/2020-August/424716.html 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. Issues == * Timing of executing capsules-on-disk Currently, processing a capsule is triggered only as part of UEFI subsystem initialization. This means that, for example, firmware update, may not take place at system booting time and will potentially be delayed until a first call of any UEFI functions. => See patch#5 for my proposal * A bunch of warnings like WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible I don't think that fixing those improves anything. * Add a document in uefi.rst TODO's == (Won't be addressed in this series.) * capsule authentication * capsule dependency (dependency expression instruction set, or depex) * loading drivers in a capsule * handling RESET flag in a capsule and QeuryCapsuleCaps * full semantics of ESRT (EFI System Resource Table) * enabling capsule API at runtime * json capsule * recovery from update failure Changes === v6 (September 7, 2020) * temporarily drop the prerequisite patch[2] * add a missing field (dependencies) in efi_api.h (but never used) (Patch#10) * add a missing field (image_capsule_support) and related definitions in efi_api.h (Patch#10, #15) * cosmetic changes on constant definitions in efi_api.h (Patch#10) * strict check for INVALID_PARAMETER at GET_IMAGE_INFO api (Patch#11,#13) * fix warnings in pytest (Patch#16,#17) v5 (August 3, 2020) * removed superfluous type conversion at dfu_write_from_mem_addr() (Patch#2) * introduced a common helper function, efi_create_indexed_name() (Patch#6,#7,#8) * use efi_[get|set]_variable_int(), if necessary, with READ_ONLY (Patch#7,#8) * return EFI_UNSUPPORTED at Patch#7 * changed the word, number, to 'index' (Patch#7,#8) * removed 'ifdef CONFIG_EFI_CAPSULE_ON_DISK' from a header (Patch#8) * initialize 'CapsuleLast' in efi_init_obj_list() (Patch#7,#8) * added 'const' qualifier for filename argument at efi_capsule_[read|delete]_file() (Patch#8) v4 (July 22, 2020) * rebased to Heinrich's current efi-2020-10 * rework dfu-related code to align with Heinrich's change (Patch#1,#3) * change a type of 'addr' argument from int to 'void *' per Sughosh's comment (Patch#2-#3,#11-#12) *
