| Message ID | 20240212074712.3657076-1-sughosh.ganu@linaro.org |
|---|---|
| Headers | show |
| Series | FWU: Migrate FWU metadata to version 2 | expand |
Hello Sughosh, Sorry for this very late reply especially since I have a quite negative feedback on the proposed changes. I don't think FWU metadata version 1 should be removed from U-Boot support. There are existing immutable boot agent relying on format v1, starting from the Synquacer boards based on SCP-firmware v2.11 [1] onward (at least up to latest v2.13 tag) and the stm32mp1 platforms based on TF-A v2.7 [2] onward (at least up to latest tag v2.10). These platforms should be able to update there EFI firmware hence needing the update agent (U-Boot) to support format v1. With the proposed series, the new format v2 contains the same information the previous mdata v1 based implementation did (apart that some info where built-in whereas v2 describe them from the mdata storage area). Could it be possible the implementation support both, using for example a internal structure fed from either format v1 or v2 content read from the storage, and used to update the mdata v1 or v2 format storage layout? [1] https://gitlab.arm.com/firmware/SCP-firmware/-/blob/v2.11.0/product/synquacer/include/fwu_mdata.h [2] https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/refs/tags/v2.7/include/drivers/fwu/fwu_metadata.h Best regards, Etienne > From: Sughosh Ganu <sughosh.ganu@linaro.org> > > The following patches migrate the FWU metadata access code to version > 2 of the structure. This is based on the structure definition as > defined in the latest rev of the FWU Multi Bank Update specification > [1]. > > Since the version 1 of the structure has currently been adopted on a > couple of platforms, it was decided to have a clean migration of the > metadata to version 2 only, instead of supporting both the versions of > the structure. Also, based on consultations with the main author of > the specification, it is expected that any further changes in the > structure would be minor tweaks, and not be significant. Hence a > migration to version 2. > > Similar migration is also being done in TF-A, including migrating the > ST platform port to support version 2 of the metadata structure [2]. > > @Michal, I tested the metadata for the two image per bank case, and it > works fine on the ST board. Kindly test this on your board as well. > > @Kojima-san, Please help in testing the version 2 on your > board. Thanks. > > > [1] - https://developer.arm.com/documentation/den0118/latest/ > [2] - https://review.trustedfirmware.org/q/topic:%22topics/fwu_metadata_v2_migration%22 > > Changes since V1: > > * Do not define flexible array members inside the structures. > * Access the image information related fields in the metadata using > the helper functions defined in an earlier patch. > * Access fwu_fw_store_desc structure using pointer arithmetic. > > (snip)
hi Etienne, On Tue, 20 Feb 2024 at 15:14, Etienne CARRIERE - foss <etienne.carriere@foss.st.com> wrote: > > Hello Sughosh, > > Sorry for this very late reply especially since I have a quite negative feedback on the proposed changes. > > I don't think FWU metadata version 1 should be removed from U-Boot support. > There are existing immutable boot agent relying on format v1, starting from the Synquacer boards based on SCP-firmware v2.11 [1] onward (at least up to latest v2.13 tag) and the stm32mp1 platforms based on TF-A v2.7 [2] onward (at least up to latest tag v2.10). These platforms should be able to update there EFI firmware hence needing the update agent (U-Boot) to support format v1. Currently, we have two platforms which are using the feature with the Update Agent in U-Boot, one being the ST boards, and the Synquacer platform from Socionext. Support is going to be added to the Xilinx boards, but they are interested in v2 only. Before I started working on this migration, I had written to the stakeholders in early December about their thoughts on this change. I had also written to you about this, but since you did not respond, I took it as an implicit acceptance for the change. Socionext had responded saying that they are fine with the migration to v2 only support in U-Boot. You are also aware that I am making changes in TF-A to migrate support to v2 only, and that includes the ST boards as well. So, will it not be better to migrate both TF-A and U-Boot to v2 only support? Since you are proposing supporting both the versions in U-Boot, I want to check with you if there are ST platforms in the field which have version 1 support? If not, I think it will be cleaner to support only v2, and use that version in both TF-A and U-Boot henceforth. I had checked with Jose, who is the author of the spec and he had mentioned that he does not foresee any major change in the metadata structure henceforth. Which is another reason why it was deemed suitable to migrate to v2 instead of supporting both versions. > > With the proposed series, the new format v2 contains the same information the previous mdata v1 based implementation did (apart that some info where built-in whereas v2 describe them from the mdata storage area). > Could it be possible the implementation support both, using for example a internal structure fed from either format v1 or v2 content read from the storage, and used to update the mdata v1 or v2 format storage layout? This is software, so we can definitely support it. But the question is whether we really need to support v1 as well? If the feature has not been deployed in the field yet, I would say a clean migration is better. -sughosh > > [1] https://gitlab.arm.com/firmware/SCP-firmware/-/blob/v2.11.0/product/synquacer/include/fwu_mdata.h > [2] https://review.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/refs/tags/v2.7/include/drivers/fwu/fwu_metadata.h > > Best regards, > Etienne > > > From: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > The following patches migrate the FWU metadata access code to version > > 2 of the structure. This is based on the structure definition as > > defined in the latest rev of the FWU Multi Bank Update specification > > [1]. > > > > Since the version 1 of the structure has currently been adopted on a > > couple of platforms, it was decided to have a clean migration of the > > metadata to version 2 only, instead of supporting both the versions of > > the structure. Also, based on consultations with the main author of > > the specification, it is expected that any further changes in the > > structure would be minor tweaks, and not be significant. Hence a > > migration to version 2. > > > > Similar migration is also being done in TF-A, including migrating the > > ST platform port to support version 2 of the metadata structure [2]. > > > > @Michal, I tested the metadata for the two image per bank case, and it > > works fine on the ST board. Kindly test this on your board as well. > > > > @Kojima-san, Please help in testing the version 2 on your > > board. Thanks. > > > > > > [1] - https://developer.arm.com/documentation/den0118/latest/ > > [2] - https://review.trustedfirmware.org/q/topic:%22topics/fwu_metadata_v2_migration%22 > > > > Changes since V1: > > > > * Do not define flexible array members inside the structures. > > * Access the image information related fields in the metadata using > > the helper functions defined in an earlier patch. > > * Access fwu_fw_store_desc structure using pointer arithmetic. > > > > (snip)