Re: [PATCH v6 00/13] FWU: Add FWU Multi Bank Update feature support

[Takahiro Akashi <takahiro.akashi@linaro.org>]

On Mon, Jul 04, 2022 at 10:46:45AM +0530, Sughosh Ganu wrote:
> 
> The patchset adds support for the FWU Multi Bank Update[1]
> feature. Certain aspects of the Dependable Boot[2] specification have
> also been implemented.
> 
> The FWU multi bank update feature is used for supporting multiple
> sets(also called banks) of firmware image(s), allowing the platform to
> boot from a different bank, in case it fails to boot from the active
> bank. This functionality is supported by keeping the relevant
> information in a structure called metadata, which provides information
> on the images. Among other parameters, the metadata structure contains
> information on the currect active bank that is being used to boot
> image(s).
> 
> Functionality is being added to work with the UEFI capsule driver in
> u-boot. The metadata is read to gather information on the update bank,
> which is the bank to which the firmware images would be flashed to. On
> a successful completion of the update of all components, the active
> bank field in the metadata is updated, to reflect the bank from which
> the platform will boot on the subsequent boots.
> 
> Currently, the feature is being enabled on the STM32MP157C-DK2 and
> Synquacer boards. The DK2 board boots a FIP image from a uSD card
> partitioned with the GPT partioning scheme, while the Synquacer board
> boots a FIP image from a MTD partitioned SPI NOR flash device.
> 
> This feature also requires changes in a previous stage of
> bootloader, which parses the metadata and selects the bank to boot the
> image(s) from. Support has being added in tf-a(BL2 stage) for the
> STM32MP157C-DK2 board to boot the active bank images. These changes 
> have been merged to the upstream tf-a repository.
> 
> The earlier patchset contained patches for both the DK2 and the
> Synquacer platforms. The handling of review comments for the Synquacer
> platform is to be taken up by a different engineer, and has not been
> done yet. After discussion with Tom Rini and Heinrich, it was decided
> to send the patches for the DK2 platform separately for review. The
> patch for adding a python test for the feature has been developed, and
> was sent in the version 5 of the patches[3]. However, the test script
> depends on adding support for the feature on MTD SPI NOR devices, and
> that is being done as part of the Synquacer patches. Hence these set
> of patches do not have the test script for the feature. That will be
> added through the patches for adding support for the feauture on
> Synquacer platform.
> 
> [1] - https://developer.arm.com/documentation/den0118/a
> [2] - https://git.codelinaro.org/linaro/dependable-boot/mbfw/uploads/6f7ddfe3be24e18d4319e108a758d02e/mbfw.pdf
> [3] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html
> 
> Changes since V5:
> * Changed to yaml file from txt as per review comment
> * Change the parameter to the function fwu_plat_get_alt_num to pass
>   the FWU udevice pointer instead of passing the metadata device
>   directly.
> * Changed the logic to store the GPT partitioned block device through
>   a priv structure as suggested by Patrick
> * Used dev_read_prop() to get the phandle_p instead of
>   ofnode_get_property() used earlier as suggested by Patrick
> * Made relevant functions static as suggested by Etienne
> * Change the mechanism to get the block device descriptor in
>   fwu_plat_get_alt_num() due to introduction of struct
>   fwu_mdata_gpt_blk_priv in the earlier patch.
> * Shuffled the location of the TAMP_FWU_* macros as suggested by
>   Patrick
> * Use u"TrialStateCtr" for the EFI variable name as suggested by
>   Patrick
> * Dropped the call to uclass_get_device() in fwu_boottime_checks() as
>   suggested by Patrick
> * Pass NULL instead of a pointer to trial_state_ctr variable when
>   deleting the variable as suggested by Etienne
> * Use u"TrialStateCtr" as suggested by Patrick
> * Do a metadata validity check by calling fwu_mdata_check() before
>   printing the FWU metadata as suggested by Michal
> * Use ret and res variables in do_fwu_mdata_read() as suggested by
>   Patrick
> * Change 'default y if FWU_MULTI_BANK_UPDATE' to default y as
>   suggested by Patrick
> * Use capsule_type instead of capsule variable that was created
>   earlier to check for the type of capsule
> * Remove use of payload variable in create_empty_capsule() as
>   suggested by Etienne
> * Initialise the struct efi_capsule_header as suggested by Etienne
> * Add some description about the reasoning for accept capsule needing
>   image GUID as suggested by Takahiro

Thanks, but I don't still understand why we need GUID here
as I said in
https://lists.denx.de/pipermail/u-boot/2022-June/486733.html

We need discussions.

-Takahiro Akashi


> 
> Sughosh Ganu (13):
>   dt/bindings: Add bindings for FWU Metadata storage device
>   FWU: Add FWU metadata structure and driver for accessing metadata
>   FWU: Add FWU metadata access driver for GPT partitioned block devices
>   stm32mp1: dk2: Add a node for the FWU metadata device
>   stm32mp1: dk2: Add image information for capsule updates
>   FWU: stm32mp1: Add helper functions for accessing FWU metadata
>   FWU: STM32MP1: Add support to read boot index from backup register
>   FWU: Add boot time checks as highlighted by the FWU specification
>   FWU: Add support for the FWU Multi Bank Update feature
>   FWU: cmd: Add a command to read FWU metadata
>   mkeficapsule: Add support for generating empty capsules
>   mkeficapsule: Add support for setting OEM flags in capsule header
>   FWU: doc: Add documentation for the FWU feature
> 
>  arch/arm/dts/stm32mp157c-dk2-u-boot.dtsi      |   7 +
>  arch/arm/mach-stm32mp/include/mach/stm32.h    |   5 +
>  board/st/stm32mp1/stm32mp1.c                  |  67 +++
>  cmd/Kconfig                                   |   7 +
>  cmd/Makefile                                  |   1 +
>  cmd/fwu_mdata.c                               |  80 +++
>  common/board_r.c                              |   5 +
>  doc/develop/uefi/fwu_updates.rst              | 156 ++++++
>  doc/develop/uefi/index.rst                    |   1 +
>  doc/develop/uefi/uefi.rst                     |   2 +
>  .../firmware/fwu-mdata.yaml                   |  32 ++
>  doc/mkeficapsule.1                            |  33 +-
>  drivers/Kconfig                               |   2 +
>  drivers/Makefile                              |   1 +
>  drivers/fwu-mdata/Kconfig                     |  16 +
>  drivers/fwu-mdata/Makefile                    |   7 +
>  drivers/fwu-mdata/fwu-mdata-uclass.c          | 458 ++++++++++++++++++
>  drivers/fwu-mdata/fwu_mdata_gpt_blk.c         | 408 ++++++++++++++++
>  include/configs/stm32mp15_common.h            |   4 +
>  include/dm/uclass-id.h                        |   1 +
>  include/fwu.h                                 |  71 +++
>  include/fwu_mdata.h                           |  67 +++
>  lib/Kconfig                                   |   6 +
>  lib/Makefile                                  |   1 +
>  lib/efi_loader/efi_capsule.c                  | 231 ++++++++-
>  lib/efi_loader/efi_setup.c                    |   3 +-
>  lib/fwu_updates/Kconfig                       |  31 ++
>  lib/fwu_updates/Makefile                      |   7 +
>  lib/fwu_updates/fwu.c                         | 191 ++++++++
>  lib/fwu_updates/fwu_gpt.c                     |  88 ++++
>  tools/eficapsule.h                            |   8 +
>  tools/mkeficapsule.c                          | 105 +++-
>  32 files changed, 2081 insertions(+), 21 deletions(-)
>  create mode 100644 cmd/fwu_mdata.c
>  create mode 100644 doc/develop/uefi/fwu_updates.rst
>  create mode 100644 doc/device-tree-bindings/firmware/fwu-mdata.yaml
>  create mode 100644 drivers/fwu-mdata/Kconfig
>  create mode 100644 drivers/fwu-mdata/Makefile
>  create mode 100644 drivers/fwu-mdata/fwu-mdata-uclass.c
>  create mode 100644 drivers/fwu-mdata/fwu_mdata_gpt_blk.c
>  create mode 100644 include/fwu.h
>  create mode 100644 include/fwu_mdata.h
>  create mode 100644 lib/fwu_updates/Kconfig
>  create mode 100644 lib/fwu_updates/Makefile
>  create mode 100644 lib/fwu_updates/fwu.c
>  create mode 100644 lib/fwu_updates/fwu_gpt.c
> 
> -- 
> 2.25.1
> 
>