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

2023-07-26 Thread AKASHI Takahiro
Hi Michal, Sughosh,

On Wed, Jul 26, 2023 at 06:36:56PM +0200, Michal Simek wrote:
> 
> 
> On 7/26/23 15:07, Ilias Apalodimas wrote:
> > Hi all
> > 
> > [...]
> > 
> > > > > 
> > > > 
> > > > 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?
> > > 
> > > Here is a story behind efi_update_capsule():
> > > ===
> > > commit a6aafce494ab
> > > Author: Masami Hiramatsu 
> > > Date:   Wed Feb 16 15:15:42 2022 +0900
> > > 
> > >  efi_loader: use efi_update_capsule_firmware() for capsule on disk
> > > ===
> > > 
> > > I still believe that this is a valid change, but we should have
> > > moved 'capsule->capsule_guid' check into efi_update_capsule_firmware()
> > > at the same time.
> > 
> > I agree with Akashi-san here.  I am also fine with this patchset since
> > running the A/B update from an EFI app should work. But can we do a v2
> > with 2 patches?
> > #1 move the capsule check along with the empty capsule checks in
> > efi_update_capsule_firmware()
> > #2 fix the a/b updates via the runtime calls and adjust the commit
> > message accordingly, explaining why this change is needed?
> 
> Can someone from Linaro create v2 on this?
> I just wanted to pointed to it.

Yes, I posted "efi_loader: capsule: enforce guid check in api and
capsule_on_disk".
Since I didn't run any test against A/B update, can you please
confirm that this patch works in your environment?

Unlike Ilias suggested, I made a single patch because an empty
capsule will be handled any way at the beginning of
efi_capsule_update_firmware() and we process only normal capsules
after that.

-Takahiro Akashi

> Thanks,
> Michal


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

2023-07-26 Thread Ilias Apalodimas
Hi all

[...]

> > >
> >
> > 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?
>
> Here is a story behind efi_update_capsule():
> ===
> commit a6aafce494ab
> Author: Masami Hiramatsu 
> Date:   Wed Feb 16 15:15:42 2022 +0900
>
> efi_loader: use efi_update_capsule_firmware() for capsule on disk
> ===
>
> I still believe that this is a valid change, but we should have
> moved 'capsule->capsule_guid' check into efi_update_capsule_firmware()
> at the same time.

I agree with Akashi-san here.  I am also fine with this patchset since
running the A/B update from an EFI app should work. But can we do a v2
with 2 patches?
#1 move the capsule check along with the empty capsule checks in
efi_update_capsule_firmware()
#2 fix the a/b updates via the runtime calls and adjust the commit
message accordingly, explaining why this change is needed?

Thanks
/Ilias
>
> -Takahiro Akashi
>
>
>
> > Best regards
> >
> > Heinrich
> >
> > >
> > > [1] - https://lists.denx.de/pipermail/u-boot/2022-February/473891.html
> >


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

2023-07-26 Thread Ilias Apalodimas
Hi Heinirch,


On Tue, 18 Jul 2023 at 18: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?

Yes, there is. It's described here along with the relevant uuids

[0] 
https://gitlab.com/Linaro/trustedsubstrate/mbfw/uploads/3d0d7d11ca9874dc9115616b418aa330/mbfw.pdf
"2.3.3 OS directed FW image acceptance"

Regards
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Michal Simek 
> > ---
> >
> >   lib/efi_loader/efi_capsule.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 7a6f195cbc02..93e83e5f04c3 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -752,7 +752,8 @@ efi_status_t EFIAPI efi_update_capsule(
> >   log_debug("Capsule[%d] (guid:%pUs)\n",
> > i, >capsule_guid);
> >   if (!guidcmp(>capsule_guid,
> > -  _guid_firmware_management_capsule_id)) {
> > +  _guid_firmware_management_capsule_id) ||
> > + fwu_empty_capsule(capsule)) {
> >   ret  = efi_capsule_update_firmware(capsule);
> >   } else {
> >   log_err("Unsupported capsule type: %pUs\n",
>


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

2023-07-26 Thread Michal Simek




On 7/26/23 15:07, Ilias Apalodimas wrote:

Hi all

[...]





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?


Here is a story behind efi_update_capsule():
===
commit a6aafce494ab
Author: Masami Hiramatsu 
Date:   Wed Feb 16 15:15:42 2022 +0900

 efi_loader: use efi_update_capsule_firmware() for capsule on disk
===

I still believe that this is a valid change, but we should have
moved 'capsule->capsule_guid' check into efi_update_capsule_firmware()
at the same time.


I agree with Akashi-san here.  I am also fine with this patchset since
running the A/B update from an EFI app should work. But can we do a v2
with 2 patches?
#1 move the capsule check along with the empty capsule checks in
efi_update_capsule_firmware()
#2 fix the a/b updates via the runtime calls and adjust the commit
message accordingly, explaining why this change is needed?


Can someone from Linaro create v2 on this?
I just wanted to pointed to it.

Thanks,
Michal


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

2023-07-20 Thread AKASHI Takahiro
On Thu, Jul 20, 2023 at 10:42:10PM +0200, Heinrich Schuchardt wrote:
> On 7/20/23 11:48, Sughosh Ganu wrote:
> > On Thu, 20 Jul 2023 at 14:56, Michal Simek  wrote:
> > > 
> > > 
> > > 
> > > On 7/20/23 10:45, Sughosh Ganu wrote:
> > > > On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 7/20/23 08:36, Sughosh Ganu wrote:
> > > > > > On Thu, 20 Jul 2023 at 11:37, Michal Simek  
> > > > > > 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 

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

2023-07-20 Thread Heinrich Schuchardt

On 7/20/23 11:48, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 14:56, Michal Simek  wrote:




On 7/20/23 10:45, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:




On 7/20/23 08:36, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 11:37, Michal Simek  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




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

2023-07-20 Thread Sughosh Ganu
On Thu, 20 Jul 2023 at 14:56, Michal Simek  wrote:
>
>
>
> On 7/20/23 10:45, Sughosh Ganu wrote:
> > On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:
> >>
> >>
> >>
> >> On 7/20/23 08:36, Sughosh Ganu wrote:
> >>> On Thu, 20 Jul 2023 at 11:37, Michal Simek  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


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

2023-07-20 Thread Michal Simek




On 7/20/23 10:45, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:




On 7/20/23 08:36, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 11:37, Michal Simek  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?

Thanks,
Michal


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

2023-07-20 Thread Sughosh Ganu
On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:
>
>
>
> On 7/20/23 08:36, Sughosh Ganu wrote:
> > On Thu, 20 Jul 2023 at 11:37, Michal Simek  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.

-sughosh

>
> Again I have no issue with code which is using capsule-on-disk feature but I
> think that pretty much all these steps which are done automatically should be
> possible to do through steps to see them. That's what you can do with bootm
> start and simply stepping through it.
>
> I am testing 2 images per bank and I can simply load other partition by simple
> commands
> tftpboot 0x10 192.168.0.105:capsule1.bin && efidebug capsule update -v 
> 0x10
> tftpboot 0x10 192.168.0.105:capsule2.bin && efidebug capsule update -v 
> 0x10
>
> And then perform reset to them. I would expect that the same should work also
> for empty capsules. I am not able to get in this to trial state but I expect
> this is simply because I am not creating TrialStateCtr variable by hand.
>
> Take a look at log below.
>
> Thanks,
> Michal
>
>
> U-Boot 2023.07-00693-g41137e2e3970 (Jul 20 2023 - 08:46:56 +0200)
>
> CPU:   ZynqMP
> Silicon: v3
> Chip:  xck26
> Detected name: zynqmp-sm-k26-xcl2gc-ed-revB-sck-kv-g-revB
> Model: ZynqMP KV260 revB
> Board: Xilinx ZynqMP
> DRAM:  2 GiB (effective 4 GiB)
> PMUFW:  v1.1
> Xilinx I2C FRU format at nvmem0:
>   Manufacturer Name: XILINX
>   Product 

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

2023-07-20 Thread Michal Simek




On 7/20/23 08:36, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 11:37, Michal Simek  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.


Again I have no issue with code which is using capsule-on-disk feature but I 
think that pretty much all these steps which are done automatically should be 
possible to do through steps to see them. That's what you can do with bootm 
start and simply stepping through it.


I am testing 2 images per bank and I can simply load other partition by simple 
commands

tftpboot 0x10 192.168.0.105:capsule1.bin && efidebug capsule update -v 
0x10
tftpboot 0x10 192.168.0.105:capsule2.bin && efidebug capsule update -v 
0x10

And then perform reset to them. I would expect that the same should work also 
for empty capsules. I am not able to get in this to trial state but I expect 
this is simply because I am not creating TrialStateCtr variable by hand.


Take a look at log below.

Thanks,
Michal


U-Boot 2023.07-00693-g41137e2e3970 (Jul 20 2023 - 08:46:56 +0200)

CPU:   ZynqMP
Silicon: v3
Chip:  xck26
Detected name: zynqmp-sm-k26-xcl2gc-ed-revB-sck-kv-g-revB
Model: ZynqMP KV260 revB
Board: Xilinx ZynqMP
DRAM:  2 GiB (effective 4 GiB)
PMUFW:  v1.1
Xilinx I2C FRU format at nvmem0:
 Manufacturer Name: XILINX
 Product Name: SM-K26-XCL2GC-ED
 Serial No: 50572B111F2H
 Part Number: 5057-02ED
 File ID: 0x0
 Revision Number: B
Xilinx I2C FRU format at nvmem1:
 Manufacturer Name: XILINX
 Product Name: SCK-KV-G
 Serial No: 50582B112M07
 Part Number: 5058-02
 File ID: 0x0
 Revision Number: B
EL Level:   EL2
Secure Boot:not authenticated, not encrypted
Core:  90 devices, 33 uclasses, devicetree: fit
NAND:  0 MiB
MMC:   mmc@ff16: 0, mmc@ff17: 1
Loading Environment from SPIFlash... SF: Detected mt25qu512a with page size 256 
Bytes, erase size 64 KiB, total 64 MiB

OK
In:serial
Out:   serial
Err:   serial
Net:   PHY reset timed out

ZYNQ GEM: ff0e, mdio bus ff0e, phyaddr 1, interface rgmii-id
eth0: ethernet@ff0e
fwu_plat_get_bootidx: boot_idx: 1, active_idx: 1
tpm_tis_spi_probe: missing reset GPIO
System booting in Trial State
Trial State count: attempt 1 out of 3
gpio: 

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

2023-07-20 Thread Sughosh Ganu
On Thu, 20 Jul 2023 at 11:37, Michal Simek  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. 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.

-sughosh

>
> Thanks,
> Michal


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

2023-07-20 Thread Michal Simek

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.


Thanks,
Michal


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

2023-07-19 Thread AKASHI Takahiro
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.

-Takahiro Akashi


> But definitely someone should check that logic that the patch is
> right based on intention how these empty capsules should be used.
> I am actually not quite sure how revert capsules should be used and how to
> revert only certain image if you use multiple images in the same bank.
> 
> Thanks,
> Michal


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

2023-07-19 Thread Michal Simek




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.


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. But definitely someone should check that logic that the patch is right 
based on intention how these empty capsules should be used.
I am actually not quite sure how revert capsules should be used and how to 
revert only certain image if you use multiple images in the same bank.


Thanks,
Michal


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

2023-07-18 Thread Heinrich Schuchardt

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?

Best regards

Heinrich



Signed-off-by: Michal Simek 
---

  lib/efi_loader/efi_capsule.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 7a6f195cbc02..93e83e5f04c3 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -752,7 +752,8 @@ efi_status_t EFIAPI efi_update_capsule(
log_debug("Capsule[%d] (guid:%pUs)\n",
  i, >capsule_guid);
if (!guidcmp(>capsule_guid,
-_guid_firmware_management_capsule_id)) {
+_guid_firmware_management_capsule_id) ||
+   fwu_empty_capsule(capsule)) {
ret  = efi_capsule_update_firmware(capsule);
} else {
log_err("Unsupported capsule type: %pUs\n",




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

2023-07-13 Thread Michal Simek
Empty capsule are also allowed to be process. Without it updated images
can't change their Image Acceptance state from no to yes.

Signed-off-by: Michal Simek 
---

 lib/efi_loader/efi_capsule.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 7a6f195cbc02..93e83e5f04c3 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -752,7 +752,8 @@ efi_status_t EFIAPI efi_update_capsule(
log_debug("Capsule[%d] (guid:%pUs)\n",
  i, >capsule_guid);
if (!guidcmp(>capsule_guid,
-_guid_firmware_management_capsule_id)) {
+_guid_firmware_management_capsule_id) ||
+   fwu_empty_capsule(capsule)) {
ret  = efi_capsule_update_firmware(capsule);
} else {
log_err("Unsupported capsule type: %pUs\n",
-- 
2.36.1