[PATCH] efi_loader: Allow also empty capsule to be process

[Sughosh Ganu]

On Thu, 20 Jul 2023 at 14:56, Michal Simek <michal.simek at amd.com> wrote:
>
>
>
> On 7/20/23 10:45, Sughosh Ganu wrote:
> > On Thu, 20 Jul 2023 at 13:26, Michal Simek <michal.simek at amd.com> wrote:
> >>
> >>
> >>
> >> On 7/20/23 08:36, Sughosh Ganu wrote:
> >>> On Thu, 20 Jul 2023 at 11:37, Michal Simek <michal.simek at amd.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 7/20/23 07:49, AKASHI Takahiro wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Wed, Jul 19, 2023 at 08:28:41AM +0200, Michal Simek wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 7/18/23 17:41, Heinrich Schuchardt wrote:
> >>>>>>> On 13.07.23 16:35, Michal Simek wrote:
> >>>>>>>> Empty capsule are also allowed to be process. Without it updated images
> >>>>>>>> can't change their Image Acceptance state from no to yes.
> >>>>>>>
> >>>>>>> Is there any documentation describing the usage of empty capsule to set
> >>>>>>> the image acceptance state?
> >>>>>>
> >>>>>> I actually don't know about documentation. I was talking to Ilias to make
> >>>>>> sure that documentation is up2date because there are missing couple of
> >>>>>> things there.
> >>>>>
> >>>>> Sughosh should have more to say here about A/B update.
> >>>>>
> >>>>>> I am testing A/B update and if you setup oemflags to 0x8000 then capsules
> >>>>>> are not automatically accepted and waiting for acceptance capsule to be
> >>>>>> passed.
> >>>>>> When I tested it I found out that they are not process that's why I created
> >>>>>> this patch.
> >>>>>
> >>>>> The path you tried to modify is only executed by "efidebug capsule update"
> >>>>> or more specifically via the runtime service, UPDATE_CAPSULE.
> >>>>>
> >>>>> But this API is NOT officially supported in the current capsule implementation
> >>>>> (at least, in my initial intention).
> >>>>> The only way to invoke capsule updates is to reboot the system.
> >>>>> If you want to test A/B update, please do the reboot.
> >>>>
> >>>> I realized that to get full flow you need to use capsule update on disk to get
> >>>> all functionalities. But it is very impractical. Actually I would expect via
> >>>> efidebug you should be able to perform all steps as capsule update performs when
> >>>> you do reboot.
> >>>> I would also understand that via efidebug you are not able to apply any capsule
> >>>> but I don't think it is right that you can apply just update capsules but not
> >>>> empty capsules. I would understand none or all but not something in the middle.
> >>>
> >>> The A/B update functionality requires using the capsule-on-disk
> >>> functionality for performing the updates. This is also mentioned in
> >>> the fwu_updates.rst document. You should be able to apply empty
> >>> capsules even with the 'efidebug disk-update' command.
> >>
> >> Yes this is working fine.
> >>
> >> ZynqMP> efidebug capsule disk-update
> >> #####
> >> Applying capsule capsule1.bin succeeded.
> >> #########################
> >> Applying capsule capsule2.bin succeeded.
> >> Reboot after firmware update.
> >>
> >> I tested it also with empty capsules which are also process properly.
> >>
> >>> I have never
> >>> used the 'efidebug capsule update' command, so I'm not sure if that is
> >>> supported. Like Takahiro mentioned, if you place the capsules(genuine
> >>> or empty) under the /EFI/UpdateCapsule/ directory, the update should
> >>> happen automatically, since the fwu update feature also enables the
> >>> EFI_CAPSULE_ON_DISK_EARLY config.
> >>
> >> Yes that's work fine on production systems.
> >> But from my point of view there shouldn't be really a problem to also apply
> >> empty capsule via efidebug capsule update to be able to see that steps and
> >> changes in mdata structure without performing reset.
> >
> > The 'efidebug capsule update' command calls the efi_update_capsule
> > function, which implements the UpdateCapsule runtime service call. The
> > initial versions of my fwu patches were indeed adding support for this
> > path, but one of the review comments was to restrict support only for
> > the capsule-on-disk path when performing the update in u-boot, since
> > we are not using the runtime call in u-boot.
>
> I don't think this is a valid argument. As I said I would understand if there is
> no interface for any capsule. It means having support for both or none is IMHO
> the way we should support.
> Can you please point me to that discussion?

There is mention of the point in this discussion [1]. Even this thread
has Takahiro mention the point he is making above, that maybe there
shouldn't be the efi_update_capsule function.

-sughosh

[1] - https://lists.denx.de/pipermail/u-boot/2022-February/473891.html