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

[Heinrich Schuchardt]

On 7/20/23 11:48, Sughosh Ganu wrote:
> 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

Hello Sugosh,

fwu_empty_capsule() detects an empty capsule as one with a GUID
fwu_guid_os_request_fw_revert or fwu_guid_os_request_fw_accept.

I am not aware of a requirement in the UEFI specification to treat
capsules read from file in a different way than capsules passed via
UpdateCapsule(). Is there any reason why UpdateCapsule() should not
process an empty capsule when called from a boot-time EFI application?

Best regards

Heinrich

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