[RFC,v2,7/8] FWU: Add support for FWU Multi Bank Update feature

The FWU Multi Bank Update feature supports updation of firmware images
to one of multiple sets(also called banks) of images. The firmware
images are clubbed together in banks, with the system booting images
from the active bank. Information on the images such as which bank
they belong to is stored as part of the metadata structure, which is
stored on the same storage media as the firmware images on a dedicated
partition.

At the time of update, the metadata is read to identify the bank to
which the images need to be flashed(update bank). On a successful
update, the metadata is modified to set the updated bank as active
bank to subsequently boot from.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V1:
* Call function fwu_update_checks_pass to check if the
  update can be initiated
* Do not allow firmware update from efi_init_obj_list as the
  fwu boot-time checks need to be run

 include/fwu.h                |  18 +++-
 lib/Kconfig                  |  32 ++++++
 lib/Makefile                 |   1 +
 lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
 lib/efi_loader/efi_setup.c   |   3 +-
 lib/fwu_updates/Makefile     |  11 ++
 lib/fwu_updates/fwu.c        |  27 +++++
 7 files changed, 284 insertions(+), 6 deletions(-)
 create mode 100644 lib/fwu_updates/Makefile

On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
> The FWU Multi Bank Update feature supports updation of firmware images
> to one of multiple sets(also called banks) of images. The firmware
> images are clubbed together in banks, with the system booting images
> from the active bank. Information on the images such as which bank
> they belong to is stored as part of the metadata structure, which is
> stored on the same storage media as the firmware images on a dedicated
> partition.
> 
> At the time of update, the metadata is read to identify the bank to
> which the images need to be flashed(update bank). On a successful
> update, the metadata is modified to set the updated bank as active
> bank to subsequently boot from.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V1:
> * Call function fwu_update_checks_pass to check if the
>   update can be initiated
> * Do not allow firmware update from efi_init_obj_list as the
>   fwu boot-time checks need to be run
> 
>  include/fwu.h                |  18 +++-
>  lib/Kconfig                  |  32 ++++++
>  lib/Makefile                 |   1 +
>  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
>  lib/efi_loader/efi_setup.c   |   3 +-
>  lib/fwu_updates/Makefile     |  11 ++
>  lib/fwu_updates/fwu.c        |  27 +++++
>  7 files changed, 284 insertions(+), 6 deletions(-)
>  create mode 100644 lib/fwu_updates/Makefile
> 
> diff --git a/include/fwu.h b/include/fwu.h
> index 2d2e674d6a..bf50fe9277 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -10,14 +10,28 @@
>  
>  #include <linux/types.h>
>  
> -#define FWU_MDATA_VERSION	0x1
> +#define FWU_MDATA_GUID \
> +	EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> +		 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> +
> +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> +	EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> +		 0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> +
> +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> +	EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> +		 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
>  
>  #define FWU_MDATA_GUID \
>  	EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
>  		 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
>  
> -int fwu_boottime_checks(void);
> +#define FWU_MDATA_VERSION	0x1
> +#define FWU_IMAGE_ACCEPTED	0x1
> +
>  u8 fwu_update_checks_pass(void);
> +int fwu_boottime_checks(void);
> +int fwu_trial_state_ctr_start(void);
>  
>  int fwu_get_active_index(u32 *active_idx);
>  int fwu_update_active_index(u32 active_idx);
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 807a4c6ade..7cb306317c 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
>  	  When there are multiple device tree nodes with same name,
>            enable this config option to distinguish them using
>  	  phandles in fdtdec_get_alias_seq() function.
> +
> +config FWU_MULTI_BANK_UPDATE
> +	bool "Enable FWU Multi Bank Update Feature"
> +	depends on EFI_HAVE_CAPSULE_SUPPORT
> +	select PARTITION_TYPE_GUID
> +	select EFI_SETUP_EARLY
> +	help
> +	  Feature for updating firmware images on platforms having
> +	  multiple banks(copies) of the firmware images. One of the
> +	  bank is selected for updating all the firmware components
> +
> +config FWU_NUM_BANKS
> +	int "Number of Banks defined by the platform"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	help
> +	  Define the number of banks of firmware images on a platform
> +
> +config FWU_NUM_IMAGES_PER_BANK
> +	int "Number of firmware images per bank"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	help
> +	  Define the number of firmware images per bank. This value
> +	  should be the same for all the banks.
> +
> +config FWU_TRIAL_STATE_CNT
> +	int "Number of times system boots in Trial State"
> +	depends on FWU_MULTI_BANK_UPDATE
> +	default 3
> +	help
> +	  With FWU Multi Bank Update feature enabled, number of times
> +	  the platform is allowed to boot in Trial State after an
> +	  update.
> diff --git a/lib/Makefile b/lib/Makefile
> index 5ddbc77ed6..bc5c1e22fc 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
>  obj-$(CONFIG_EFI_LOADER) += efi_driver/
>  obj-$(CONFIG_EFI_LOADER) += efi_loader/
>  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
>  obj-$(CONFIG_LZMA) += lzma/
>  obj-$(CONFIG_BZIP2) += bzip2/
>  obj-$(CONFIG_TIZEN) += tizen/
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 8301eed631..6dfe56bb0f 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -14,6 +14,7 @@
>  #include <env.h>
>  #include <fdtdec.h>
>  #include <fs.h>
> +#include <fwu.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <sort.h>
> @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
>  		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>  const efi_guid_t efi_guid_firmware_management_protocol =
>  		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +const efi_guid_t fwu_guid_os_request_fw_revert =
> +		FWU_OS_REQUEST_FW_REVERT_GUID;
> +const efi_guid_t fwu_guid_os_request_fw_accept =
> +		FWU_OS_REQUEST_FW_ACCEPT_GUID;
> +
> +__maybe_unused static u32 update_index;
> +__maybe_unused static bool capsule_update;
>  
>  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
>  /* for file system access */
> @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
>  	void *image_binary, *vendor_code;
>  	efi_handle_t *handles;
>  	efi_uintn_t no_handles;
> -	int item;
> +	int item, alt_no;
>  	struct efi_firmware_management_protocol *fmp;
>  	u16 *abort_reason;
> +	efi_guid_t image_type_id;
>  	efi_status_t ret = EFI_SUCCESS;
> +	int status;
> +	u8 image_index;
>  
>  	/* sanity check */
>  	if (capsule_data->header_size < sizeof(*capsule) ||
> @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
>  				goto out;
>  		}
>  
> +		if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +			/*
> +			 * Based on the value of update_image_type_id,
> +			 * derive the alt number value. This will be
> +			 * passed as update_image_index to the
> +			 * set_image function.
> +			 */
> +			image_type_id = image->update_image_type_id;
> +			status = fwu_get_image_alt_num(image_type_id,
> +						       update_index,
> +						       &alt_no);
> +			if (status < 0) {
> +				log_err("Unable to get the alt no for the image type %pUl\n",
> +					&image_type_id);
> +				if (status == -ENODEV || status == -EIO)
> +					ret = EFI_DEVICE_ERROR;
> +				else if (status == -ENOMEM)
> +					ret = EFI_OUT_OF_RESOURCES;
> +				else if (status == -ERANGE || status == -EINVAL)
> +					ret = EFI_INVALID_PARAMETER;
> +				goto out;
> +			}
> +			log_debug("alt_no %u for Image Type Id %pUl\n",
> +				  alt_no, &image_type_id);
> +			image_index = alt_no + 1;
> +		} else {
> +			image_index = image->update_image_index;
> +		}
>  		abort_reason = NULL;
> -		ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> +		ret = EFI_CALL(fmp->set_image(fmp, image_index,
>  					      image_binary,
>  					      image_binary_size,
>  					      vendor_code, NULL,
> @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
>  			efi_free_pool(abort_reason);
>  			goto out;
>  		}
> +
> +		if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +			status = fwu_clear_accept_image(&image_type_id,
> +							update_index);
> +			if (status < 0) {
> +				log_err("Unable to clear the accept bit for the image %pUl\n",
> +					&image_type_id);
> +				if (status == -ENODEV || status == -EIO)
> +					ret = EFI_DEVICE_ERROR;
> +				else if (status == -ENOMEM)
> +					ret = EFI_OUT_OF_RESOURCES;
> +				else if (status == -ERANGE || status == -EINVAL)
> +					ret = EFI_INVALID_PARAMETER;
> +				goto out;
> +			}
> +			log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
> +		}
> +
>  	}
>  
>  out:
> @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
>  		u64 scatter_gather_list)
>  {
>  	struct efi_capsule_header *capsule;
> +	efi_guid_t *image_guid;
> +	u32 active_idx;
> +	int status;
>  	unsigned int i;
>  	efi_status_t ret;
>  
> @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
>  		goto out;
>  	}
>  
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +		/* Obtain the update_index from the platform */
> +		status = fwu_plat_get_update_index(&update_index);
> +		if (status < 0) {
> +			log_err("Failed to get the FWU update_index value\n");
> +			ret = EFI_DEVICE_ERROR;
> +			goto out;
> +		}
> +	}
> +
>  	ret = EFI_SUCCESS;
>  	for (i = 0, capsule = *capsule_header_array; i < capsule_count;
>  	     i++, capsule = *(++capsule_header_array)) {
> @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
>  			  i, &capsule->capsule_guid);
>  		if (!guidcmp(&capsule->capsule_guid,
>  			     &efi_guid_firmware_management_capsule_id)) {
> +			if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +				if (!fwu_update_checks_pass()) {
> +					log_err("FWU checks failed. Cannot start update\n");
> +					ret = EFI_INVALID_PARAMETER;
> +					goto out;
> +				}
> +			}
> +
>  			ret  = efi_capsule_update_firmware(capsule);
> +			capsule_update = true;
> +		} else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +			capsule_update = false;
> +			if (!guidcmp(&capsule->capsule_guid,
> +				     &fwu_guid_os_request_fw_revert)) {
> +				/*
> +				 * One of the previously updated image has
> +				 * failed the OS acceptance test. OS has
> +				 * requested to revert back to the earlier
> +				 * boot index
> +				 */
> +				status = fwu_revert_boot_index();
> +				if (status < 0) {
> +					log_err("Failed to revert the FWU boot index\n");
> +					if (status == -ENODEV ||
> +					    status == -ERANGE ||
> +					    status == -EIO)
> +						ret = EFI_DEVICE_ERROR;
> +					else if (status == -EINVAL)
> +						ret = EFI_INVALID_PARAMETER;
> +					else if (status == -ENOMEM)
> +						ret = EFI_OUT_OF_RESOURCES;
> +				} else {
> +					ret = EFI_SUCCESS;
> +					log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
> +						active_idx);
> +				}
> +			} else if (!guidcmp(&capsule->capsule_guid,
> +					    &fwu_guid_os_request_fw_accept)) {
> +				/*
> +				 * Image accepted by the OS. Set the acceptance
> +				 * status for the image.
> +				 */
> +				image_guid = (void *)(char *)capsule +
> +					capsule->header_size;
> +				status = fwu_accept_image(image_guid);
> +				if (status < 0) {
> +					if (status == -ENODEV ||
> +					    status == -ERANGE ||
> +					    status == -EIO)
> +						ret = EFI_DEVICE_ERROR;
> +					else if (status == -EINVAL)
> +						ret = EFI_INVALID_PARAMETER;
> +					else if (status == -ENOMEM)
> +						ret = EFI_OUT_OF_RESOURCES;
> +				} else {
> +					ret = EFI_SUCCESS;
> +				}
> +			}
>  		} else {
>  			log_err("Unsupported capsule type: %pUl\n",
>  				&capsule->capsule_guid);
> @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
>  			goto out;
>  	}
>  
> +	/*
> +	 * Update the FWU metadata once all the capsules have
> +	 * been updated. This is done only for the Runtime
> +	 * capsule update service.
> +	 * The update_index value now gets written to the
> +	 * active_index and the update_index value also
> +	 * gets updated.
> +	 * For the capsule-on-disk feature, the updation
> +	 * of the FWU metadata happens in efi_launch_capsules
> +	 */
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> +	    !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> +		status = fwu_update_active_index(update_index);
> +		if (status < 0) {
> +			log_err("Failed to update FWU metadata index values\n");
> +			if (status == -EINVAL || status == -ERANGE)
> +				ret = EFI_INVALID_PARAMETER;
> +			else if (status == -ENODEV || status == -EIO)
> +				ret = EFI_DEVICE_ERROR;
> +			else if (status == -ENOMEM)
> +				ret = EFI_OUT_OF_RESOURCES;
> +		} else {
> +			status = fwu_trial_state_ctr_start();
> +			if (status < 0)
> +				ret = EFI_DEVICE_ERROR;
> +			else
> +				ret = EFI_SUCCESS;
> +		}
> +	}
> +
>  	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
>  		/* Rebuild the ESRT to reflect any updated FW images. */
>  		ret = efi_esrt_populate();
> @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
>  {
>  	struct efi_capsule_header *capsule = NULL;
>  	u16 **files;
> +	int status;
>  	unsigned int nfiles, index, i;
>  	efi_status_t ret;
> +	bool update_status = true;
>  
>  	if (check_run_capsules() != EFI_SUCCESS)
>  		return EFI_SUCCESS;
> @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
>  		ret = efi_capsule_read_file(files[i], &capsule);
>  		if (ret == EFI_SUCCESS) {
>  			ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> -			if (ret != EFI_SUCCESS)
> +			if (ret != EFI_SUCCESS) {
>  				log_err("Applying capsule %ls failed\n",
>  					files[i]);
> +				update_status = false;
> +			}
>  
>  			/* create CapsuleXXXX */
>  			set_capsule_result(index, capsule, ret);
> @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
>  			free(capsule);
>  		} else {
>  			log_err("Reading capsule %ls failed\n", files[i]);
> +			update_status = false;
>  		}
>  		/* delete a capsule either in case of success or failure */
>  		ret = efi_capsule_delete_file(files[i]);
> @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
>  			log_err("Deleting capsule %ls failed\n",
>  				files[i]);
>  	}
> +
>  	efi_capsule_scan_done();
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +		if (update_status == true && capsule_update == true) {
> +			/*
> +			 * All the capsules have been updated successfully,
> +			 * update the FWU metadata.
> +			 */
> +			log_debug("Update Complete. Now updating active_index to %u\n",
> +				  update_index);
> +			status = fwu_update_active_index(update_index);
> +			if (status < 0) {
> +				log_err("Failed to update FWU metadata index values\n");
> +				if (status == -EINVAL || status == -ERANGE)
> +					ret = EFI_INVALID_PARAMETER;
> +				else if (status == -ENODEV || status == -EIO)
> +					ret = EFI_DEVICE_ERROR;
> +				else if (status == -ENOMEM)
> +					ret = EFI_OUT_OF_RESOURCES;
> +			} else {
> +				log_debug("Successfully updated the active_index\n");
> +				status = fwu_trial_state_ctr_start();
> +				if (status < 0)
> +					ret = EFI_DEVICE_ERROR;
> +				else
> +					ret = EFI_SUCCESS;
> +			}
> +		} else if (capsule_update == true && update_status == false) {
> +			log_err("All capsules were not updated. Not updating FWU metadata\n");
> +		}
> +	}
>  
>  	for (i = 0; i < nfiles; i++)
>  		free(files[i]);
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 1aba71cd96..6601176a2d 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
>  		goto out;
>  
>  	/* Execute capsules after reboot */
> -	if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> +	if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> +	    IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>  	    !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
>  		ret = efi_launch_capsules();
>  out:
> diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> new file mode 100644
> index 0000000000..73099a30cb
> --- /dev/null
> +++ b/lib/fwu_updates/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2021, Linaro Limited
> +#
> +
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> +
> +ifdef CONFIG_EFI_PARTITION
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> +endif

If I understand correctly, any platform may have its own
fwu_mdata_ops. So we should not unconditionally compile in
fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.

Masami must have something to say here :)

-Takahiro Akashi

> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index e964f9b0b1..bb0e1961be 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -107,6 +107,33 @@ u8 fwu_update_checks_pass(void)
>  	return !trial_state && boottime_check;
>  }
>  
> +int fwu_trial_state_ctr_start(void)
> +{
> +	int ret;
> +	u32 var_attributes;
> +	efi_status_t status;
> +	efi_uintn_t var_size;
> +	u16 trial_state_ctr;
> +
> +	var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> +	var_attributes = EFI_VARIABLE_NON_VOLATILE |
> +		EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		EFI_VARIABLE_RUNTIME_ACCESS;
> +
> +	trial_state_ctr = ret = 0;
> +	status = efi_set_variable_int(L"TrialStateCtr",
> +				      &efi_global_variable_guid,
> +				      var_attributes,
> +				      var_size,
> +				      &trial_state_ctr, false);
> +	if (status != EFI_SUCCESS) {
> +		log_err("Unable to increment TrialStateCtr variable\n");
> +		ret = -1;
> +	}
> +
> +	return ret;
> +}
> +
>  int fwu_boottime_checks(void)
>  {
>  	int ret;
> -- 
> 2.17.1
>

On Mon, 20 Dec 2021 at 11:44, AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
> > The FWU Multi Bank Update feature supports updation of firmware images
> > to one of multiple sets(also called banks) of images. The firmware
> > images are clubbed together in banks, with the system booting images
> > from the active bank. Information on the images such as which bank
> > they belong to is stored as part of the metadata structure, which is
> > stored on the same storage media as the firmware images on a dedicated
> > partition.
> >
> > At the time of update, the metadata is read to identify the bank to
> > which the images need to be flashed(update bank). On a successful
> > update, the metadata is modified to set the updated bank as active
> > bank to subsequently boot from.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V1:
> > * Call function fwu_update_checks_pass to check if the
> >   update can be initiated
> > * Do not allow firmware update from efi_init_obj_list as the
> >   fwu boot-time checks need to be run
> >
> >  include/fwu.h                |  18 +++-
> >  lib/Kconfig                  |  32 ++++++
> >  lib/Makefile                 |   1 +
> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
> >  lib/efi_loader/efi_setup.c   |   3 +-
> >  lib/fwu_updates/Makefile     |  11 ++
> >  lib/fwu_updates/fwu.c        |  27 +++++
> >  7 files changed, 284 insertions(+), 6 deletions(-)
> >  create mode 100644 lib/fwu_updates/Makefile
> >
> > diff --git a/include/fwu.h b/include/fwu.h
> > index 2d2e674d6a..bf50fe9277 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -10,14 +10,28 @@
> >
> >  #include <linux/types.h>
> >
> > -#define FWU_MDATA_VERSION    0x1
> > +#define FWU_MDATA_GUID \
> > +     EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > +              0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > +
> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> > +     EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> > +              0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> > +
> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> > +     EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> > +              0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> >
> >  #define FWU_MDATA_GUID \
> >       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >
> > -int fwu_boottime_checks(void);
> > +#define FWU_MDATA_VERSION    0x1
> > +#define FWU_IMAGE_ACCEPTED   0x1
> > +
> >  u8 fwu_update_checks_pass(void);
> > +int fwu_boottime_checks(void);
> > +int fwu_trial_state_ctr_start(void);
> >
> >  int fwu_get_active_index(u32 *active_idx);
> >  int fwu_update_active_index(u32 active_idx);
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 807a4c6ade..7cb306317c 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> >         When there are multiple device tree nodes with same name,
> >            enable this config option to distinguish them using
> >         phandles in fdtdec_get_alias_seq() function.
> > +
> > +config FWU_MULTI_BANK_UPDATE
> > +     bool "Enable FWU Multi Bank Update Feature"
> > +     depends on EFI_HAVE_CAPSULE_SUPPORT
> > +     select PARTITION_TYPE_GUID
> > +     select EFI_SETUP_EARLY
> > +     help
> > +       Feature for updating firmware images on platforms having
> > +       multiple banks(copies) of the firmware images. One of the
> > +       bank is selected for updating all the firmware components
> > +
> > +config FWU_NUM_BANKS
> > +     int "Number of Banks defined by the platform"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     help
> > +       Define the number of banks of firmware images on a platform
> > +
> > +config FWU_NUM_IMAGES_PER_BANK
> > +     int "Number of firmware images per bank"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     help
> > +       Define the number of firmware images per bank. This value
> > +       should be the same for all the banks.
> > +
> > +config FWU_TRIAL_STATE_CNT
> > +     int "Number of times system boots in Trial State"
> > +     depends on FWU_MULTI_BANK_UPDATE
> > +     default 3
> > +     help
> > +       With FWU Multi Bank Update feature enabled, number of times
> > +       the platform is allowed to boot in Trial State after an
> > +       update.
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 5ddbc77ed6..bc5c1e22fc 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
> >  obj-$(CONFIG_LZMA) += lzma/
> >  obj-$(CONFIG_BZIP2) += bzip2/
> >  obj-$(CONFIG_TIZEN) += tizen/
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 8301eed631..6dfe56bb0f 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -14,6 +14,7 @@
> >  #include <env.h>
> >  #include <fdtdec.h>
> >  #include <fs.h>
> > +#include <fwu.h>
> >  #include <malloc.h>
> >  #include <mapmem.h>
> >  #include <sort.h>
> > @@ -30,6 +31,13 @@ static const efi_guid_t
> efi_guid_firmware_management_capsule_id =
> >               EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >  const efi_guid_t efi_guid_firmware_management_protocol =
> >               EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> > +const efi_guid_t fwu_guid_os_request_fw_revert =
> > +             FWU_OS_REQUEST_FW_REVERT_GUID;
> > +const efi_guid_t fwu_guid_os_request_fw_accept =
> > +             FWU_OS_REQUEST_FW_ACCEPT_GUID;
> > +
> > +__maybe_unused static u32 update_index;
> > +__maybe_unused static bool capsule_update;
> >
> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
> >  /* for file system access */
> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
> >       void *image_binary, *vendor_code;
> >       efi_handle_t *handles;
> >       efi_uintn_t no_handles;
> > -     int item;
> > +     int item, alt_no;
> >       struct efi_firmware_management_protocol *fmp;
> >       u16 *abort_reason;
> > +     efi_guid_t image_type_id;
> >       efi_status_t ret = EFI_SUCCESS;
> > +     int status;
> > +     u8 image_index;
> >
> >       /* sanity check */
> >       if (capsule_data->header_size < sizeof(*capsule) ||
> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
> >                               goto out;
> >               }
> >
> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                     /*
> > +                      * Based on the value of update_image_type_id,
> > +                      * derive the alt number value. This will be
> > +                      * passed as update_image_index to the
> > +                      * set_image function.
> > +                      */
> > +                     image_type_id = image->update_image_type_id;
> > +                     status = fwu_get_image_alt_num(image_type_id,
> > +                                                    update_index,
> > +                                                    &alt_no);
> > +                     if (status < 0) {
> > +                             log_err("Unable to get the alt no for the
> image type %pUl\n",
> > +                                     &image_type_id);
> > +                             if (status == -ENODEV || status == -EIO)
> > +                                     ret = EFI_DEVICE_ERROR;
> > +                             else if (status == -ENOMEM)
> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > +                             else if (status == -ERANGE || status ==
> -EINVAL)
> > +                                     ret = EFI_INVALID_PARAMETER;
> > +                             goto out;
> > +                     }
> > +                     log_debug("alt_no %u for Image Type Id %pUl\n",
> > +                               alt_no, &image_type_id);
> > +                     image_index = alt_no + 1;
> > +             } else {
> > +                     image_index = image->update_image_index;
> > +             }
> >               abort_reason = NULL;
> > -             ret = EFI_CALL(fmp->set_image(fmp,
> image->update_image_index,
> > +             ret = EFI_CALL(fmp->set_image(fmp, image_index,
> >                                             image_binary,
> >                                             image_binary_size,
> >                                             vendor_code, NULL,
> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
> >                       efi_free_pool(abort_reason);
> >                       goto out;
> >               }
> > +
> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                     status = fwu_clear_accept_image(&image_type_id,
> > +                                                     update_index);
> > +                     if (status < 0) {
> > +                             log_err("Unable to clear the accept bit
> for the image %pUl\n",
> > +                                     &image_type_id);
> > +                             if (status == -ENODEV || status == -EIO)
> > +                                     ret = EFI_DEVICE_ERROR;
> > +                             else if (status == -ENOMEM)
> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > +                             else if (status == -ERANGE || status ==
> -EINVAL)
> > +                                     ret = EFI_INVALID_PARAMETER;
> > +                             goto out;
> > +                     }
> > +                     log_debug("Cleared out accepted bit for Image
> %pUl\n", &image_type_id);
> > +             }
> > +
> >       }
> >
> >  out:
> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
> >               u64 scatter_gather_list)
> >  {
> >       struct efi_capsule_header *capsule;
> > +     efi_guid_t *image_guid;
> > +     u32 active_idx;
> > +     int status;
> >       unsigned int i;
> >       efi_status_t ret;
> >
> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
> >               goto out;
> >       }
> >
> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +             /* Obtain the update_index from the platform */
> > +             status = fwu_plat_get_update_index(&update_index);
> > +             if (status < 0) {
> > +                     log_err("Failed to get the FWU update_index
> value\n");
> > +                     ret = EFI_DEVICE_ERROR;
> > +                     goto out;
> > +             }
> > +     }
> > +
> >       ret = EFI_SUCCESS;
> >       for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> >            i++, capsule = *(++capsule_header_array)) {
> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
> >                         i, &capsule->capsule_guid);
> >               if (!guidcmp(&capsule->capsule_guid,
> >                            &efi_guid_firmware_management_capsule_id)) {
> > +                     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                             if (!fwu_update_checks_pass()) {
> > +                                     log_err("FWU checks failed. Cannot
> start update\n");
> > +                                     ret = EFI_INVALID_PARAMETER;
> > +                                     goto out;
> > +                             }
> > +                     }
> > +
> >                       ret  = efi_capsule_update_firmware(capsule);
> > +                     capsule_update = true;
> > +             } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                     capsule_update = false;
> > +                     if (!guidcmp(&capsule->capsule_guid,
> > +                                  &fwu_guid_os_request_fw_revert)) {
> > +                             /*
> > +                              * One of the previously updated image has
> > +                              * failed the OS acceptance test. OS has
> > +                              * requested to revert back to the earlier
> > +                              * boot index
> > +                              */
> > +                             status = fwu_revert_boot_index();
> > +                             if (status < 0) {
> > +                                     log_err("Failed to revert the FWU
> boot index\n");
> > +                                     if (status == -ENODEV ||
> > +                                         status == -ERANGE ||
> > +                                         status == -EIO)
> > +                                             ret = EFI_DEVICE_ERROR;
> > +                                     else if (status == -EINVAL)
> > +                                             ret =
> EFI_INVALID_PARAMETER;
> > +                                     else if (status == -ENOMEM)
> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > +                             } else {
> > +                                     ret = EFI_SUCCESS;
> > +                                     log_err("Reverted the FWU
> active_index to %u. Recommend rebooting the system\n",
> > +                                             active_idx);
> > +                             }
> > +                     } else if (!guidcmp(&capsule->capsule_guid,
> > +
>  &fwu_guid_os_request_fw_accept)) {
> > +                             /*
> > +                              * Image accepted by the OS. Set the
> acceptance
> > +                              * status for the image.
> > +                              */
> > +                             image_guid = (void *)(char *)capsule +
> > +                                     capsule->header_size;
> > +                             status = fwu_accept_image(image_guid);
> > +                             if (status < 0) {
> > +                                     if (status == -ENODEV ||
> > +                                         status == -ERANGE ||
> > +                                         status == -EIO)
> > +                                             ret = EFI_DEVICE_ERROR;
> > +                                     else if (status == -EINVAL)
> > +                                             ret =
> EFI_INVALID_PARAMETER;
> > +                                     else if (status == -ENOMEM)
> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > +                             } else {
> > +                                     ret = EFI_SUCCESS;
> > +                             }
> > +                     }
> >               } else {
> >                       log_err("Unsupported capsule type: %pUl\n",
> >                               &capsule->capsule_guid);
> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
> >                       goto out;
> >       }
> >
> > +     /*
> > +      * Update the FWU metadata once all the capsules have
> > +      * been updated. This is done only for the Runtime
> > +      * capsule update service.
> > +      * The update_index value now gets written to the
> > +      * active_index and the update_index value also
> > +      * gets updated.
> > +      * For the capsule-on-disk feature, the updation
> > +      * of the FWU metadata happens in efi_launch_capsules
> > +      */
> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > +         !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> > +             status = fwu_update_active_index(update_index);
> > +             if (status < 0) {
> > +                     log_err("Failed to update FWU metadata index
> values\n");
> > +                     if (status == -EINVAL || status == -ERANGE)
> > +                             ret = EFI_INVALID_PARAMETER;
> > +                     else if (status == -ENODEV || status == -EIO)
> > +                             ret = EFI_DEVICE_ERROR;
> > +                     else if (status == -ENOMEM)
> > +                             ret = EFI_OUT_OF_RESOURCES;
> > +             } else {
> > +                     status = fwu_trial_state_ctr_start();
> > +                     if (status < 0)
> > +                             ret = EFI_DEVICE_ERROR;
> > +                     else
> > +                             ret = EFI_SUCCESS;
> > +             }
> > +     }
> > +
> >       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> >               /* Rebuild the ESRT to reflect any updated FW images. */
> >               ret = efi_esrt_populate();
> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
> >  {
> >       struct efi_capsule_header *capsule = NULL;
> >       u16 **files;
> > +     int status;
> >       unsigned int nfiles, index, i;
> >       efi_status_t ret;
> > +     bool update_status = true;
> >
> >       if (check_run_capsules() != EFI_SUCCESS)
> >               return EFI_SUCCESS;
> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
> >               ret = efi_capsule_read_file(files[i], &capsule);
> >               if (ret == EFI_SUCCESS) {
> >                       ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> > -                     if (ret != EFI_SUCCESS)
> > +                     if (ret != EFI_SUCCESS) {
> >                               log_err("Applying capsule %ls failed\n",
> >                                       files[i]);
> > +                             update_status = false;
> > +                     }
> >
> >                       /* create CapsuleXXXX */
> >                       set_capsule_result(index, capsule, ret);
> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
> >                       free(capsule);
> >               } else {
> >                       log_err("Reading capsule %ls failed\n", files[i]);
> > +                     update_status = false;
> >               }
> >               /* delete a capsule either in case of success or failure */
> >               ret = efi_capsule_delete_file(files[i]);
> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
> >                       log_err("Deleting capsule %ls failed\n",
> >                               files[i]);
> >       }
> > +
> >       efi_capsule_scan_done();
> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +             if (update_status == true && capsule_update == true) {
> > +                     /*
> > +                      * All the capsules have been updated successfully,
> > +                      * update the FWU metadata.
> > +                      */
> > +                     log_debug("Update Complete. Now updating
> active_index to %u\n",
> > +                               update_index);
> > +                     status = fwu_update_active_index(update_index);
> > +                     if (status < 0) {
> > +                             log_err("Failed to update FWU metadata
> index values\n");
> > +                             if (status == -EINVAL || status == -ERANGE)
> > +                                     ret = EFI_INVALID_PARAMETER;
> > +                             else if (status == -ENODEV || status ==
> -EIO)
> > +                                     ret = EFI_DEVICE_ERROR;
> > +                             else if (status == -ENOMEM)
> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > +                     } else {
> > +                             log_debug("Successfully updated the
> active_index\n");
> > +                             status = fwu_trial_state_ctr_start();
> > +                             if (status < 0)
> > +                                     ret = EFI_DEVICE_ERROR;
> > +                             else
> > +                                     ret = EFI_SUCCESS;
> > +                     }
> > +             } else if (capsule_update == true && update_status ==
> false) {
> > +                     log_err("All capsules were not updated. Not
> updating FWU metadata\n");
> > +             }
> > +     }
> >
> >       for (i = 0; i < nfiles; i++)
> >               free(files[i]);
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 1aba71cd96..6601176a2d 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
> >               goto out;
> >
> >       /* Execute capsules after reboot */
> > -     if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > +     if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > +         IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> >           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> >               ret = efi_launch_capsules();
> >  out:
> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> > new file mode 100644
> > index 0000000000..73099a30cb
> > --- /dev/null
> > +++ b/lib/fwu_updates/Makefile
> > @@ -0,0 +1,11 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (c) 2021, Linaro Limited
> > +#
> > +
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> > +
> > +ifdef CONFIG_EFI_PARTITION
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> > +endif
>
> If I understand correctly, any platform may have its own
> fwu_mdata_ops. So we should not unconditionally compile in
> fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.
>

I can put in an additional guard of CONFIG_BLK for building the file. Not
sure if there would be a requirement to have a platform specific set of
operations for a GPT partitioned block device. Will wait for Masami to
explain.

-sughosh



>
> Masami must have something to say here :)
>
> -Takahiro Akashi
>
> > diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> > index e964f9b0b1..bb0e1961be 100644
> > --- a/lib/fwu_updates/fwu.c
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -107,6 +107,33 @@ u8 fwu_update_checks_pass(void)
> >       return !trial_state && boottime_check;
> >  }
> >
> > +int fwu_trial_state_ctr_start(void)
> > +{
> > +     int ret;
> > +     u32 var_attributes;
> > +     efi_status_t status;
> > +     efi_uintn_t var_size;
> > +     u16 trial_state_ctr;
> > +
> > +     var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> > +     var_attributes = EFI_VARIABLE_NON_VOLATILE |
> > +             EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +             EFI_VARIABLE_RUNTIME_ACCESS;
> > +
> > +     trial_state_ctr = ret = 0;
> > +     status = efi_set_variable_int(L"TrialStateCtr",
> > +                                   &efi_global_variable_guid,
> > +                                   var_attributes,
> > +                                   var_size,
> > +                                   &trial_state_ctr, false);
> > +     if (status != EFI_SUCCESS) {
> > +             log_err("Unable to increment TrialStateCtr variable\n");
> > +             ret = -1;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >  int fwu_boottime_checks(void)
> >  {
> >       int ret;
> > --
> > 2.17.1
> >
>

Hi Sughosh,

2021年12月20日(月) 18:48 Sughosh Ganu <sughosh.ganu@linaro.org>:
>
>
> On Mon, 20 Dec 2021 at 11:44, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>>
>> On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
>> > The FWU Multi Bank Update feature supports updation of firmware images
>> > to one of multiple sets(also called banks) of images. The firmware
>> > images are clubbed together in banks, with the system booting images
>> > from the active bank. Information on the images such as which bank
>> > they belong to is stored as part of the metadata structure, which is
>> > stored on the same storage media as the firmware images on a dedicated
>> > partition.
>> >
>> > At the time of update, the metadata is read to identify the bank to
>> > which the images need to be flashed(update bank). On a successful
>> > update, the metadata is modified to set the updated bank as active
>> > bank to subsequently boot from.
>> >
>> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>> > ---
>> > Changes since V1:
>> > * Call function fwu_update_checks_pass to check if the
>> >   update can be initiated
>> > * Do not allow firmware update from efi_init_obj_list as the
>> >   fwu boot-time checks need to be run
>> >
>> >  include/fwu.h                |  18 +++-
>> >  lib/Kconfig                  |  32 ++++++
>> >  lib/Makefile                 |   1 +
>> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
>> >  lib/efi_loader/efi_setup.c   |   3 +-
>> >  lib/fwu_updates/Makefile     |  11 ++
>> >  lib/fwu_updates/fwu.c        |  27 +++++
>> >  7 files changed, 284 insertions(+), 6 deletions(-)
>> >  create mode 100644 lib/fwu_updates/Makefile
>> >
>> > diff --git a/include/fwu.h b/include/fwu.h
>> > index 2d2e674d6a..bf50fe9277 100644
>> > --- a/include/fwu.h
>> > +++ b/include/fwu.h
>> > @@ -10,14 +10,28 @@
>> >
>> >  #include <linux/types.h>
>> >
>> > -#define FWU_MDATA_VERSION    0x1
>> > +#define FWU_MDATA_GUID \
>> > +     EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
>> > +              0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
>> > +
>> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
>> > +     EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
>> > +              0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
>> > +
>> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
>> > +     EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
>> > +              0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
>> >
>> >  #define FWU_MDATA_GUID \
>> >       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
>> >                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
>> >
>> > -int fwu_boottime_checks(void);
>> > +#define FWU_MDATA_VERSION    0x1
>> > +#define FWU_IMAGE_ACCEPTED   0x1
>> > +
>> >  u8 fwu_update_checks_pass(void);
>> > +int fwu_boottime_checks(void);
>> > +int fwu_trial_state_ctr_start(void);
>> >
>> >  int fwu_get_active_index(u32 *active_idx);
>> >  int fwu_update_active_index(u32 active_idx);
>> > diff --git a/lib/Kconfig b/lib/Kconfig
>> > index 807a4c6ade..7cb306317c 100644
>> > --- a/lib/Kconfig
>> > +++ b/lib/Kconfig
>> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
>> >         When there are multiple device tree nodes with same name,
>> >            enable this config option to distinguish them using
>> >         phandles in fdtdec_get_alias_seq() function.
>> > +
>> > +config FWU_MULTI_BANK_UPDATE
>> > +     bool "Enable FWU Multi Bank Update Feature"
>> > +     depends on EFI_HAVE_CAPSULE_SUPPORT
>> > +     select PARTITION_TYPE_GUID
>> > +     select EFI_SETUP_EARLY
>> > +     help
>> > +       Feature for updating firmware images on platforms having
>> > +       multiple banks(copies) of the firmware images. One of the
>> > +       bank is selected for updating all the firmware components
>> > +
>> > +config FWU_NUM_BANKS
>> > +     int "Number of Banks defined by the platform"
>> > +     depends on FWU_MULTI_BANK_UPDATE
>> > +     help
>> > +       Define the number of banks of firmware images on a platform
>> > +
>> > +config FWU_NUM_IMAGES_PER_BANK
>> > +     int "Number of firmware images per bank"
>> > +     depends on FWU_MULTI_BANK_UPDATE
>> > +     help
>> > +       Define the number of firmware images per bank. This value
>> > +       should be the same for all the banks.
>> > +
>> > +config FWU_TRIAL_STATE_CNT
>> > +     int "Number of times system boots in Trial State"
>> > +     depends on FWU_MULTI_BANK_UPDATE
>> > +     default 3
>> > +     help
>> > +       With FWU Multi Bank Update feature enabled, number of times
>> > +       the platform is allowed to boot in Trial State after an
>> > +       update.
>> > diff --git a/lib/Makefile b/lib/Makefile
>> > index 5ddbc77ed6..bc5c1e22fc 100644
>> > --- a/lib/Makefile
>> > +++ b/lib/Makefile
>> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
>> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
>> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
>> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
>> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
>> >  obj-$(CONFIG_LZMA) += lzma/
>> >  obj-$(CONFIG_BZIP2) += bzip2/
>> >  obj-$(CONFIG_TIZEN) += tizen/
>> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>> > index 8301eed631..6dfe56bb0f 100644
>> > --- a/lib/efi_loader/efi_capsule.c
>> > +++ b/lib/efi_loader/efi_capsule.c
>> > @@ -14,6 +14,7 @@
>> >  #include <env.h>
>> >  #include <fdtdec.h>
>> >  #include <fs.h>
>> > +#include <fwu.h>
>> >  #include <malloc.h>
>> >  #include <mapmem.h>
>> >  #include <sort.h>
>> > @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
>> >               EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>> >  const efi_guid_t efi_guid_firmware_management_protocol =
>> >               EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
>> > +const efi_guid_t fwu_guid_os_request_fw_revert =
>> > +             FWU_OS_REQUEST_FW_REVERT_GUID;
>> > +const efi_guid_t fwu_guid_os_request_fw_accept =
>> > +             FWU_OS_REQUEST_FW_ACCEPT_GUID;
>> > +
>> > +__maybe_unused static u32 update_index;
>> > +__maybe_unused static bool capsule_update;
>> >
>> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
>> >  /* for file system access */
>> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
>> >       void *image_binary, *vendor_code;
>> >       efi_handle_t *handles;
>> >       efi_uintn_t no_handles;
>> > -     int item;
>> > +     int item, alt_no;
>> >       struct efi_firmware_management_protocol *fmp;
>> >       u16 *abort_reason;
>> > +     efi_guid_t image_type_id;
>> >       efi_status_t ret = EFI_SUCCESS;
>> > +     int status;
>> > +     u8 image_index;
>> >
>> >       /* sanity check */
>> >       if (capsule_data->header_size < sizeof(*capsule) ||
>> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
>> >                               goto out;
>> >               }
>> >
>> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +                     /*
>> > +                      * Based on the value of update_image_type_id,
>> > +                      * derive the alt number value. This will be
>> > +                      * passed as update_image_index to the
>> > +                      * set_image function.
>> > +                      */
>> > +                     image_type_id = image->update_image_type_id;
>> > +                     status = fwu_get_image_alt_num(image_type_id,
>> > +                                                    update_index,
>> > +                                                    &alt_no);
>> > +                     if (status < 0) {
>> > +                             log_err("Unable to get the alt no for the image type %pUl\n",
>> > +                                     &image_type_id);
>> > +                             if (status == -ENODEV || status == -EIO)
>> > +                                     ret = EFI_DEVICE_ERROR;
>> > +                             else if (status == -ENOMEM)
>> > +                                     ret = EFI_OUT_OF_RESOURCES;
>> > +                             else if (status == -ERANGE || status == -EINVAL)
>> > +                                     ret = EFI_INVALID_PARAMETER;
>> > +                             goto out;
>> > +                     }
>> > +                     log_debug("alt_no %u for Image Type Id %pUl\n",
>> > +                               alt_no, &image_type_id);
>> > +                     image_index = alt_no + 1;
>> > +             } else {
>> > +                     image_index = image->update_image_index;
>> > +             }
>> >               abort_reason = NULL;
>> > -             ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
>> > +             ret = EFI_CALL(fmp->set_image(fmp, image_index,
>> >                                             image_binary,
>> >                                             image_binary_size,
>> >                                             vendor_code, NULL,
>> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
>> >                       efi_free_pool(abort_reason);
>> >                       goto out;
>> >               }
>> > +
>> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +                     status = fwu_clear_accept_image(&image_type_id,
>> > +                                                     update_index);
>> > +                     if (status < 0) {
>> > +                             log_err("Unable to clear the accept bit for the image %pUl\n",
>> > +                                     &image_type_id);
>> > +                             if (status == -ENODEV || status == -EIO)
>> > +                                     ret = EFI_DEVICE_ERROR;
>> > +                             else if (status == -ENOMEM)
>> > +                                     ret = EFI_OUT_OF_RESOURCES;
>> > +                             else if (status == -ERANGE || status == -EINVAL)
>> > +                                     ret = EFI_INVALID_PARAMETER;
>> > +                             goto out;
>> > +                     }
>> > +                     log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
>> > +             }
>> > +
>> >       }
>> >
>> >  out:
>> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
>> >               u64 scatter_gather_list)
>> >  {
>> >       struct efi_capsule_header *capsule;
>> > +     efi_guid_t *image_guid;
>> > +     u32 active_idx;
>> > +     int status;
>> >       unsigned int i;
>> >       efi_status_t ret;
>> >
>> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
>> >               goto out;
>> >       }
>> >
>> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +             /* Obtain the update_index from the platform */
>> > +             status = fwu_plat_get_update_index(&update_index);
>> > +             if (status < 0) {
>> > +                     log_err("Failed to get the FWU update_index value\n");
>> > +                     ret = EFI_DEVICE_ERROR;
>> > +                     goto out;
>> > +             }
>> > +     }
>> > +
>> >       ret = EFI_SUCCESS;
>> >       for (i = 0, capsule = *capsule_header_array; i < capsule_count;
>> >            i++, capsule = *(++capsule_header_array)) {
>> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
>> >                         i, &capsule->capsule_guid);
>> >               if (!guidcmp(&capsule->capsule_guid,
>> >                            &efi_guid_firmware_management_capsule_id)) {
>> > +                     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +                             if (!fwu_update_checks_pass()) {
>> > +                                     log_err("FWU checks failed. Cannot start update\n");
>> > +                                     ret = EFI_INVALID_PARAMETER;
>> > +                                     goto out;
>> > +                             }
>> > +                     }
>> > +
>> >                       ret  = efi_capsule_update_firmware(capsule);
>> > +                     capsule_update = true;
>> > +             } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +                     capsule_update = false;
>> > +                     if (!guidcmp(&capsule->capsule_guid,
>> > +                                  &fwu_guid_os_request_fw_revert)) {
>> > +                             /*
>> > +                              * One of the previously updated image has
>> > +                              * failed the OS acceptance test. OS has
>> > +                              * requested to revert back to the earlier
>> > +                              * boot index
>> > +                              */
>> > +                             status = fwu_revert_boot_index();
>> > +                             if (status < 0) {
>> > +                                     log_err("Failed to revert the FWU boot index\n");
>> > +                                     if (status == -ENODEV ||
>> > +                                         status == -ERANGE ||
>> > +                                         status == -EIO)
>> > +                                             ret = EFI_DEVICE_ERROR;
>> > +                                     else if (status == -EINVAL)
>> > +                                             ret = EFI_INVALID_PARAMETER;
>> > +                                     else if (status == -ENOMEM)
>> > +                                             ret = EFI_OUT_OF_RESOURCES;
>> > +                             } else {
>> > +                                     ret = EFI_SUCCESS;
>> > +                                     log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
>> > +                                             active_idx);
>> > +                             }
>> > +                     } else if (!guidcmp(&capsule->capsule_guid,
>> > +                                         &fwu_guid_os_request_fw_accept)) {
>> > +                             /*
>> > +                              * Image accepted by the OS. Set the acceptance
>> > +                              * status for the image.
>> > +                              */
>> > +                             image_guid = (void *)(char *)capsule +
>> > +                                     capsule->header_size;
>> > +                             status = fwu_accept_image(image_guid);
>> > +                             if (status < 0) {
>> > +                                     if (status == -ENODEV ||
>> > +                                         status == -ERANGE ||
>> > +                                         status == -EIO)
>> > +                                             ret = EFI_DEVICE_ERROR;
>> > +                                     else if (status == -EINVAL)
>> > +                                             ret = EFI_INVALID_PARAMETER;
>> > +                                     else if (status == -ENOMEM)
>> > +                                             ret = EFI_OUT_OF_RESOURCES;
>> > +                             } else {
>> > +                                     ret = EFI_SUCCESS;
>> > +                             }
>> > +                     }
>> >               } else {
>> >                       log_err("Unsupported capsule type: %pUl\n",
>> >                               &capsule->capsule_guid);
>> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
>> >                       goto out;
>> >       }
>> >
>> > +     /*
>> > +      * Update the FWU metadata once all the capsules have
>> > +      * been updated. This is done only for the Runtime
>> > +      * capsule update service.
>> > +      * The update_index value now gets written to the
>> > +      * active_index and the update_index value also
>> > +      * gets updated.
>> > +      * For the capsule-on-disk feature, the updation
>> > +      * of the FWU metadata happens in efi_launch_capsules
>> > +      */
>> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
>> > +         !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
>> > +             status = fwu_update_active_index(update_index);
>> > +             if (status < 0) {
>> > +                     log_err("Failed to update FWU metadata index values\n");
>> > +                     if (status == -EINVAL || status == -ERANGE)
>> > +                             ret = EFI_INVALID_PARAMETER;
>> > +                     else if (status == -ENODEV || status == -EIO)
>> > +                             ret = EFI_DEVICE_ERROR;
>> > +                     else if (status == -ENOMEM)
>> > +                             ret = EFI_OUT_OF_RESOURCES;
>> > +             } else {
>> > +                     status = fwu_trial_state_ctr_start();
>> > +                     if (status < 0)
>> > +                             ret = EFI_DEVICE_ERROR;
>> > +                     else
>> > +                             ret = EFI_SUCCESS;
>> > +             }
>> > +     }
>> > +
>> >       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
>> >               /* Rebuild the ESRT to reflect any updated FW images. */
>> >               ret = efi_esrt_populate();
>> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
>> >  {
>> >       struct efi_capsule_header *capsule = NULL;
>> >       u16 **files;
>> > +     int status;
>> >       unsigned int nfiles, index, i;
>> >       efi_status_t ret;
>> > +     bool update_status = true;
>> >
>> >       if (check_run_capsules() != EFI_SUCCESS)
>> >               return EFI_SUCCESS;
>> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
>> >               ret = efi_capsule_read_file(files[i], &capsule);
>> >               if (ret == EFI_SUCCESS) {
>> >                       ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
>> > -                     if (ret != EFI_SUCCESS)
>> > +                     if (ret != EFI_SUCCESS) {
>> >                               log_err("Applying capsule %ls failed\n",
>> >                                       files[i]);
>> > +                             update_status = false;
>> > +                     }
>> >
>> >                       /* create CapsuleXXXX */
>> >                       set_capsule_result(index, capsule, ret);
>> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
>> >                       free(capsule);
>> >               } else {
>> >                       log_err("Reading capsule %ls failed\n", files[i]);
>> > +                     update_status = false;
>> >               }
>> >               /* delete a capsule either in case of success or failure */
>> >               ret = efi_capsule_delete_file(files[i]);
>> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
>> >                       log_err("Deleting capsule %ls failed\n",
>> >                               files[i]);
>> >       }
>> > +
>> >       efi_capsule_scan_done();
>> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>> > +             if (update_status == true && capsule_update == true) {
>> > +                     /*
>> > +                      * All the capsules have been updated successfully,
>> > +                      * update the FWU metadata.
>> > +                      */
>> > +                     log_debug("Update Complete. Now updating active_index to %u\n",
>> > +                               update_index);
>> > +                     status = fwu_update_active_index(update_index);
>> > +                     if (status < 0) {
>> > +                             log_err("Failed to update FWU metadata index values\n");
>> > +                             if (status == -EINVAL || status == -ERANGE)
>> > +                                     ret = EFI_INVALID_PARAMETER;
>> > +                             else if (status == -ENODEV || status == -EIO)
>> > +                                     ret = EFI_DEVICE_ERROR;
>> > +                             else if (status == -ENOMEM)
>> > +                                     ret = EFI_OUT_OF_RESOURCES;
>> > +                     } else {
>> > +                             log_debug("Successfully updated the active_index\n");
>> > +                             status = fwu_trial_state_ctr_start();
>> > +                             if (status < 0)
>> > +                                     ret = EFI_DEVICE_ERROR;
>> > +                             else
>> > +                                     ret = EFI_SUCCESS;
>> > +                     }
>> > +             } else if (capsule_update == true && update_status == false) {
>> > +                     log_err("All capsules were not updated. Not updating FWU metadata\n");
>> > +             }
>> > +     }
>> >
>> >       for (i = 0; i < nfiles; i++)
>> >               free(files[i]);
>> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> > index 1aba71cd96..6601176a2d 100644
>> > --- a/lib/efi_loader/efi_setup.c
>> > +++ b/lib/efi_loader/efi_setup.c
>> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
>> >               goto out;
>> >
>> >       /* Execute capsules after reboot */
>> > -     if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>> > +     if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
>> > +         IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>> >           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
>> >               ret = efi_launch_capsules();
>> >  out:
>> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
>> > new file mode 100644
>> > index 0000000000..73099a30cb
>> > --- /dev/null
>> > +++ b/lib/fwu_updates/Makefile
>> > @@ -0,0 +1,11 @@
>> > +# SPDX-License-Identifier: GPL-2.0+
>> > +#
>> > +# Copyright (c) 2021, Linaro Limited
>> > +#
>> > +
>> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
>> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
>> > +
>> > +ifdef CONFIG_EFI_PARTITION
>> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
>> > +endif
>>
>> If I understand correctly, any platform may have its own
>> fwu_mdata_ops. So we should not unconditionally compile in
>> fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.
>
>
> I can put in an additional guard of CONFIG_BLK for building the file. Not sure if there would be a requirement to have a platform specific set of operations for a GPT partitioned block device. Will wait for Masami to explain.


OK, I'm trying to implement the AB update on the developerbox
platform, which doesn't have GPT, nor BLK since the firmware will be
stored on the SPI NOR flash. So I will introduce a new fwu_mdata_sf.c.
For this purpose, I introduced CONFIG_FWU_MULTI_BANK_UPDATE_SF, which
depends on CONFIG_FWU_MULTI_BANK_UPDATE and CONFIG_DM_SPI_FLASH. Thus,
I also recommend you to introduce CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
and it should depends on CONFIG_BLK and selects CONFIG_EFI_PARTITION.

BTW, there are some code pieces which can shared with the
fwu_mdata_gpt_blk.c, for example, comparing primary and seconday
metadata and calculate the crc32.

Thank you,


>
> -sughosh
>
>
>>
>>
>> Masami must have something to say here :)
>>
>> -Takahiro Akashi
>>
>> > diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
>> > index e964f9b0b1..bb0e1961be 100644
>> > --- a/lib/fwu_updates/fwu.c
>> > +++ b/lib/fwu_updates/fwu.c
>> > @@ -107,6 +107,33 @@ u8 fwu_update_checks_pass(void)
>> >       return !trial_state && boottime_check;
>> >  }
>> >
>> > +int fwu_trial_state_ctr_start(void)
>> > +{
>> > +     int ret;
>> > +     u32 var_attributes;
>> > +     efi_status_t status;
>> > +     efi_uintn_t var_size;
>> > +     u16 trial_state_ctr;
>> > +
>> > +     var_size = (efi_uintn_t)sizeof(trial_state_ctr);
>> > +     var_attributes = EFI_VARIABLE_NON_VOLATILE |
>> > +             EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> > +             EFI_VARIABLE_RUNTIME_ACCESS;
>> > +
>> > +     trial_state_ctr = ret = 0;
>> > +     status = efi_set_variable_int(L"TrialStateCtr",
>> > +                                   &efi_global_variable_guid,
>> > +                                   var_attributes,
>> > +                                   var_size,
>> > +                                   &trial_state_ctr, false);
>> > +     if (status != EFI_SUCCESS) {
>> > +             log_err("Unable to increment TrialStateCtr variable\n");
>> > +             ret = -1;
>> > +     }
>> > +
>> > +     return ret;
>> > +}
>> > +
>> >  int fwu_boottime_checks(void)
>> >  {
>> >       int ret;
>> > --
>> > 2.17.1
>> >

On Mon, Dec 20, 2021 at 10:13:35PM +0900, Masami Hiramatsu wrote:
> Hi Sughosh,
> 
> 2021年12月20日(月) 18:48 Sughosh Ganu <sughosh.ganu@linaro.org>:
> >
> >
> > On Mon, 20 Dec 2021 at 11:44, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >>
> >> On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
> >> > The FWU Multi Bank Update feature supports updation of firmware images
> >> > to one of multiple sets(also called banks) of images. The firmware
> >> > images are clubbed together in banks, with the system booting images
> >> > from the active bank. Information on the images such as which bank
> >> > they belong to is stored as part of the metadata structure, which is
> >> > stored on the same storage media as the firmware images on a dedicated
> >> > partition.
> >> >
> >> > At the time of update, the metadata is read to identify the bank to
> >> > which the images need to be flashed(update bank). On a successful
> >> > update, the metadata is modified to set the updated bank as active
> >> > bank to subsequently boot from.
> >> >
> >> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> >> > ---
> >> > Changes since V1:
> >> > * Call function fwu_update_checks_pass to check if the
> >> >   update can be initiated
> >> > * Do not allow firmware update from efi_init_obj_list as the
> >> >   fwu boot-time checks need to be run
> >> >
> >> >  include/fwu.h                |  18 +++-
> >> >  lib/Kconfig                  |  32 ++++++
> >> >  lib/Makefile                 |   1 +
> >> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
> >> >  lib/efi_loader/efi_setup.c   |   3 +-
> >> >  lib/fwu_updates/Makefile     |  11 ++
> >> >  lib/fwu_updates/fwu.c        |  27 +++++
> >> >  7 files changed, 284 insertions(+), 6 deletions(-)
> >> >  create mode 100644 lib/fwu_updates/Makefile
> >> >
> >> > diff --git a/include/fwu.h b/include/fwu.h
> >> > index 2d2e674d6a..bf50fe9277 100644
> >> > --- a/include/fwu.h
> >> > +++ b/include/fwu.h
> >> > @@ -10,14 +10,28 @@
> >> >
> >> >  #include <linux/types.h>
> >> >
> >> > -#define FWU_MDATA_VERSION    0x1
> >> > +#define FWU_MDATA_GUID \
> >> > +     EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >> > +              0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >> > +
> >> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> >> > +     EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> >> > +              0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> >> > +
> >> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> >> > +     EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> >> > +              0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> >> >
> >> >  #define FWU_MDATA_GUID \
> >> >       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >> >                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >> >
> >> > -int fwu_boottime_checks(void);
> >> > +#define FWU_MDATA_VERSION    0x1
> >> > +#define FWU_IMAGE_ACCEPTED   0x1
> >> > +
> >> >  u8 fwu_update_checks_pass(void);
> >> > +int fwu_boottime_checks(void);
> >> > +int fwu_trial_state_ctr_start(void);
> >> >
> >> >  int fwu_get_active_index(u32 *active_idx);
> >> >  int fwu_update_active_index(u32 active_idx);
> >> > diff --git a/lib/Kconfig b/lib/Kconfig
> >> > index 807a4c6ade..7cb306317c 100644
> >> > --- a/lib/Kconfig
> >> > +++ b/lib/Kconfig
> >> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> >> >         When there are multiple device tree nodes with same name,
> >> >            enable this config option to distinguish them using
> >> >         phandles in fdtdec_get_alias_seq() function.
> >> > +
> >> > +config FWU_MULTI_BANK_UPDATE
> >> > +     bool "Enable FWU Multi Bank Update Feature"
> >> > +     depends on EFI_HAVE_CAPSULE_SUPPORT
> >> > +     select PARTITION_TYPE_GUID
> >> > +     select EFI_SETUP_EARLY
> >> > +     help
> >> > +       Feature for updating firmware images on platforms having
> >> > +       multiple banks(copies) of the firmware images. One of the
> >> > +       bank is selected for updating all the firmware components
> >> > +
> >> > +config FWU_NUM_BANKS
> >> > +     int "Number of Banks defined by the platform"
> >> > +     depends on FWU_MULTI_BANK_UPDATE
> >> > +     help
> >> > +       Define the number of banks of firmware images on a platform
> >> > +
> >> > +config FWU_NUM_IMAGES_PER_BANK
> >> > +     int "Number of firmware images per bank"
> >> > +     depends on FWU_MULTI_BANK_UPDATE
> >> > +     help
> >> > +       Define the number of firmware images per bank. This value
> >> > +       should be the same for all the banks.
> >> > +
> >> > +config FWU_TRIAL_STATE_CNT
> >> > +     int "Number of times system boots in Trial State"
> >> > +     depends on FWU_MULTI_BANK_UPDATE
> >> > +     default 3
> >> > +     help
> >> > +       With FWU Multi Bank Update feature enabled, number of times
> >> > +       the platform is allowed to boot in Trial State after an
> >> > +       update.
> >> > diff --git a/lib/Makefile b/lib/Makefile
> >> > index 5ddbc77ed6..bc5c1e22fc 100644
> >> > --- a/lib/Makefile
> >> > +++ b/lib/Makefile
> >> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
> >> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
> >> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
> >> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
> >> >  obj-$(CONFIG_LZMA) += lzma/
> >> >  obj-$(CONFIG_BZIP2) += bzip2/
> >> >  obj-$(CONFIG_TIZEN) += tizen/
> >> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> >> > index 8301eed631..6dfe56bb0f 100644
> >> > --- a/lib/efi_loader/efi_capsule.c
> >> > +++ b/lib/efi_loader/efi_capsule.c
> >> > @@ -14,6 +14,7 @@
> >> >  #include <env.h>
> >> >  #include <fdtdec.h>
> >> >  #include <fs.h>
> >> > +#include <fwu.h>
> >> >  #include <malloc.h>
> >> >  #include <mapmem.h>
> >> >  #include <sort.h>
> >> > @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
> >> >               EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >> >  const efi_guid_t efi_guid_firmware_management_protocol =
> >> >               EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> >> > +const efi_guid_t fwu_guid_os_request_fw_revert =
> >> > +             FWU_OS_REQUEST_FW_REVERT_GUID;
> >> > +const efi_guid_t fwu_guid_os_request_fw_accept =
> >> > +             FWU_OS_REQUEST_FW_ACCEPT_GUID;
> >> > +
> >> > +__maybe_unused static u32 update_index;
> >> > +__maybe_unused static bool capsule_update;
> >> >
> >> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
> >> >  /* for file system access */
> >> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
> >> >       void *image_binary, *vendor_code;
> >> >       efi_handle_t *handles;
> >> >       efi_uintn_t no_handles;
> >> > -     int item;
> >> > +     int item, alt_no;
> >> >       struct efi_firmware_management_protocol *fmp;
> >> >       u16 *abort_reason;
> >> > +     efi_guid_t image_type_id;
> >> >       efi_status_t ret = EFI_SUCCESS;
> >> > +     int status;
> >> > +     u8 image_index;
> >> >
> >> >       /* sanity check */
> >> >       if (capsule_data->header_size < sizeof(*capsule) ||
> >> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
> >> >                               goto out;
> >> >               }
> >> >
> >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +                     /*
> >> > +                      * Based on the value of update_image_type_id,
> >> > +                      * derive the alt number value. This will be
> >> > +                      * passed as update_image_index to the
> >> > +                      * set_image function.
> >> > +                      */
> >> > +                     image_type_id = image->update_image_type_id;
> >> > +                     status = fwu_get_image_alt_num(image_type_id,
> >> > +                                                    update_index,
> >> > +                                                    &alt_no);
> >> > +                     if (status < 0) {
> >> > +                             log_err("Unable to get the alt no for the image type %pUl\n",
> >> > +                                     &image_type_id);
> >> > +                             if (status == -ENODEV || status == -EIO)
> >> > +                                     ret = EFI_DEVICE_ERROR;
> >> > +                             else if (status == -ENOMEM)
> >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> >> > +                             else if (status == -ERANGE || status == -EINVAL)
> >> > +                                     ret = EFI_INVALID_PARAMETER;
> >> > +                             goto out;
> >> > +                     }
> >> > +                     log_debug("alt_no %u for Image Type Id %pUl\n",
> >> > +                               alt_no, &image_type_id);
> >> > +                     image_index = alt_no + 1;
> >> > +             } else {
> >> > +                     image_index = image->update_image_index;
> >> > +             }
> >> >               abort_reason = NULL;
> >> > -             ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> >> > +             ret = EFI_CALL(fmp->set_image(fmp, image_index,
> >> >                                             image_binary,
> >> >                                             image_binary_size,
> >> >                                             vendor_code, NULL,
> >> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
> >> >                       efi_free_pool(abort_reason);
> >> >                       goto out;
> >> >               }
> >> > +
> >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +                     status = fwu_clear_accept_image(&image_type_id,
> >> > +                                                     update_index);
> >> > +                     if (status < 0) {
> >> > +                             log_err("Unable to clear the accept bit for the image %pUl\n",
> >> > +                                     &image_type_id);
> >> > +                             if (status == -ENODEV || status == -EIO)
> >> > +                                     ret = EFI_DEVICE_ERROR;
> >> > +                             else if (status == -ENOMEM)
> >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> >> > +                             else if (status == -ERANGE || status == -EINVAL)
> >> > +                                     ret = EFI_INVALID_PARAMETER;
> >> > +                             goto out;
> >> > +                     }
> >> > +                     log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
> >> > +             }
> >> > +
> >> >       }
> >> >
> >> >  out:
> >> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
> >> >               u64 scatter_gather_list)
> >> >  {
> >> >       struct efi_capsule_header *capsule;
> >> > +     efi_guid_t *image_guid;
> >> > +     u32 active_idx;
> >> > +     int status;
> >> >       unsigned int i;
> >> >       efi_status_t ret;
> >> >
> >> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
> >> >               goto out;
> >> >       }
> >> >
> >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +             /* Obtain the update_index from the platform */
> >> > +             status = fwu_plat_get_update_index(&update_index);
> >> > +             if (status < 0) {
> >> > +                     log_err("Failed to get the FWU update_index value\n");
> >> > +                     ret = EFI_DEVICE_ERROR;
> >> > +                     goto out;
> >> > +             }
> >> > +     }
> >> > +
> >> >       ret = EFI_SUCCESS;
> >> >       for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> >> >            i++, capsule = *(++capsule_header_array)) {
> >> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
> >> >                         i, &capsule->capsule_guid);
> >> >               if (!guidcmp(&capsule->capsule_guid,
> >> >                            &efi_guid_firmware_management_capsule_id)) {
> >> > +                     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +                             if (!fwu_update_checks_pass()) {
> >> > +                                     log_err("FWU checks failed. Cannot start update\n");
> >> > +                                     ret = EFI_INVALID_PARAMETER;
> >> > +                                     goto out;
> >> > +                             }
> >> > +                     }
> >> > +
> >> >                       ret  = efi_capsule_update_firmware(capsule);
> >> > +                     capsule_update = true;
> >> > +             } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +                     capsule_update = false;
> >> > +                     if (!guidcmp(&capsule->capsule_guid,
> >> > +                                  &fwu_guid_os_request_fw_revert)) {
> >> > +                             /*
> >> > +                              * One of the previously updated image has
> >> > +                              * failed the OS acceptance test. OS has
> >> > +                              * requested to revert back to the earlier
> >> > +                              * boot index
> >> > +                              */
> >> > +                             status = fwu_revert_boot_index();
> >> > +                             if (status < 0) {
> >> > +                                     log_err("Failed to revert the FWU boot index\n");
> >> > +                                     if (status == -ENODEV ||
> >> > +                                         status == -ERANGE ||
> >> > +                                         status == -EIO)
> >> > +                                             ret = EFI_DEVICE_ERROR;
> >> > +                                     else if (status == -EINVAL)
> >> > +                                             ret = EFI_INVALID_PARAMETER;
> >> > +                                     else if (status == -ENOMEM)
> >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> >> > +                             } else {
> >> > +                                     ret = EFI_SUCCESS;
> >> > +                                     log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
> >> > +                                             active_idx);
> >> > +                             }
> >> > +                     } else if (!guidcmp(&capsule->capsule_guid,
> >> > +                                         &fwu_guid_os_request_fw_accept)) {
> >> > +                             /*
> >> > +                              * Image accepted by the OS. Set the acceptance
> >> > +                              * status for the image.
> >> > +                              */
> >> > +                             image_guid = (void *)(char *)capsule +
> >> > +                                     capsule->header_size;
> >> > +                             status = fwu_accept_image(image_guid);
> >> > +                             if (status < 0) {
> >> > +                                     if (status == -ENODEV ||
> >> > +                                         status == -ERANGE ||
> >> > +                                         status == -EIO)
> >> > +                                             ret = EFI_DEVICE_ERROR;
> >> > +                                     else if (status == -EINVAL)
> >> > +                                             ret = EFI_INVALID_PARAMETER;
> >> > +                                     else if (status == -ENOMEM)
> >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> >> > +                             } else {
> >> > +                                     ret = EFI_SUCCESS;
> >> > +                             }
> >> > +                     }
> >> >               } else {
> >> >                       log_err("Unsupported capsule type: %pUl\n",
> >> >                               &capsule->capsule_guid);
> >> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
> >> >                       goto out;
> >> >       }
> >> >
> >> > +     /*
> >> > +      * Update the FWU metadata once all the capsules have
> >> > +      * been updated. This is done only for the Runtime
> >> > +      * capsule update service.
> >> > +      * The update_index value now gets written to the
> >> > +      * active_index and the update_index value also
> >> > +      * gets updated.
> >> > +      * For the capsule-on-disk feature, the updation
> >> > +      * of the FWU metadata happens in efi_launch_capsules
> >> > +      */
> >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> >> > +         !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> >> > +             status = fwu_update_active_index(update_index);
> >> > +             if (status < 0) {
> >> > +                     log_err("Failed to update FWU metadata index values\n");
> >> > +                     if (status == -EINVAL || status == -ERANGE)
> >> > +                             ret = EFI_INVALID_PARAMETER;
> >> > +                     else if (status == -ENODEV || status == -EIO)
> >> > +                             ret = EFI_DEVICE_ERROR;
> >> > +                     else if (status == -ENOMEM)
> >> > +                             ret = EFI_OUT_OF_RESOURCES;
> >> > +             } else {
> >> > +                     status = fwu_trial_state_ctr_start();
> >> > +                     if (status < 0)
> >> > +                             ret = EFI_DEVICE_ERROR;
> >> > +                     else
> >> > +                             ret = EFI_SUCCESS;
> >> > +             }
> >> > +     }
> >> > +
> >> >       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> >> >               /* Rebuild the ESRT to reflect any updated FW images. */
> >> >               ret = efi_esrt_populate();
> >> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
> >> >  {
> >> >       struct efi_capsule_header *capsule = NULL;
> >> >       u16 **files;
> >> > +     int status;
> >> >       unsigned int nfiles, index, i;
> >> >       efi_status_t ret;
> >> > +     bool update_status = true;
> >> >
> >> >       if (check_run_capsules() != EFI_SUCCESS)
> >> >               return EFI_SUCCESS;
> >> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
> >> >               ret = efi_capsule_read_file(files[i], &capsule);
> >> >               if (ret == EFI_SUCCESS) {
> >> >                       ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> >> > -                     if (ret != EFI_SUCCESS)
> >> > +                     if (ret != EFI_SUCCESS) {
> >> >                               log_err("Applying capsule %ls failed\n",
> >> >                                       files[i]);
> >> > +                             update_status = false;
> >> > +                     }
> >> >
> >> >                       /* create CapsuleXXXX */
> >> >                       set_capsule_result(index, capsule, ret);
> >> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
> >> >                       free(capsule);
> >> >               } else {
> >> >                       log_err("Reading capsule %ls failed\n", files[i]);
> >> > +                     update_status = false;
> >> >               }
> >> >               /* delete a capsule either in case of success or failure */
> >> >               ret = efi_capsule_delete_file(files[i]);
> >> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
> >> >                       log_err("Deleting capsule %ls failed\n",
> >> >                               files[i]);
> >> >       }
> >> > +
> >> >       efi_capsule_scan_done();
> >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> >> > +             if (update_status == true && capsule_update == true) {
> >> > +                     /*
> >> > +                      * All the capsules have been updated successfully,
> >> > +                      * update the FWU metadata.
> >> > +                      */
> >> > +                     log_debug("Update Complete. Now updating active_index to %u\n",
> >> > +                               update_index);
> >> > +                     status = fwu_update_active_index(update_index);
> >> > +                     if (status < 0) {
> >> > +                             log_err("Failed to update FWU metadata index values\n");
> >> > +                             if (status == -EINVAL || status == -ERANGE)
> >> > +                                     ret = EFI_INVALID_PARAMETER;
> >> > +                             else if (status == -ENODEV || status == -EIO)
> >> > +                                     ret = EFI_DEVICE_ERROR;
> >> > +                             else if (status == -ENOMEM)
> >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> >> > +                     } else {
> >> > +                             log_debug("Successfully updated the active_index\n");
> >> > +                             status = fwu_trial_state_ctr_start();
> >> > +                             if (status < 0)
> >> > +                                     ret = EFI_DEVICE_ERROR;
> >> > +                             else
> >> > +                                     ret = EFI_SUCCESS;
> >> > +                     }
> >> > +             } else if (capsule_update == true && update_status == false) {
> >> > +                     log_err("All capsules were not updated. Not updating FWU metadata\n");
> >> > +             }
> >> > +     }
> >> >
> >> >       for (i = 0; i < nfiles; i++)
> >> >               free(files[i]);
> >> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> >> > index 1aba71cd96..6601176a2d 100644
> >> > --- a/lib/efi_loader/efi_setup.c
> >> > +++ b/lib/efi_loader/efi_setup.c
> >> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
> >> >               goto out;
> >> >
> >> >       /* Execute capsules after reboot */
> >> > -     if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> >> > +     if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> >> > +         IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> >> >           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> >> >               ret = efi_launch_capsules();
> >> >  out:
> >> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> >> > new file mode 100644
> >> > index 0000000000..73099a30cb
> >> > --- /dev/null
> >> > +++ b/lib/fwu_updates/Makefile
> >> > @@ -0,0 +1,11 @@
> >> > +# SPDX-License-Identifier: GPL-2.0+
> >> > +#
> >> > +# Copyright (c) 2021, Linaro Limited
> >> > +#
> >> > +
> >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> >> > +
> >> > +ifdef CONFIG_EFI_PARTITION
> >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> >> > +endif
> >>
> >> If I understand correctly, any platform may have its own
> >> fwu_mdata_ops. So we should not unconditionally compile in
> >> fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.
> >
> >
> > I can put in an additional guard of CONFIG_BLK for building the file. Not sure if there would be a requirement to have a platform specific set of operations for a GPT partitioned block device. Will wait for Masami to explain.
> 
> 
> OK, I'm trying to implement the AB update on the developerbox
> platform, which doesn't have GPT, nor BLK since the firmware will be
> stored on the SPI NOR flash. So I will introduce a new fwu_mdata_sf.c.
> For this purpose, I introduced CONFIG_FWU_MULTI_BANK_UPDATE_SF, which
> depends on CONFIG_FWU_MULTI_BANK_UPDATE and CONFIG_DM_SPI_FLASH. Thus,
> I also recommend you to introduce CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
> and it should depends on CONFIG_BLK and selects CONFIG_EFI_PARTITION.
> 
> BTW, there are some code pieces which can shared with the
> fwu_mdata_gpt_blk.c, for example, comparing primary and seconday
> metadata and calculate the crc32.

Here's my concern / question.  At what level is this abstraction being
used?  I want to be sure that other SoCs, such as say a Rockchip
platform (I have a SystemReady IR certified one on the way) can re-use
this code as well but I'm sure it will need things written to different
offsets within the SPI/eMMC.  Thanks!

Hi Tom,

2021年12月21日(火) 1:36 Tom Rini <trini@konsulko.com>:
>
> On Mon, Dec 20, 2021 at 10:13:35PM +0900, Masami Hiramatsu wrote:
> > Hi Sughosh,
> >
> > 2021年12月20日(月) 18:48 Sughosh Ganu <sughosh.ganu@linaro.org>:
> > >
> > >
> > > On Mon, 20 Dec 2021 at 11:44, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >>
> > >> On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
> > >> > The FWU Multi Bank Update feature supports updation of firmware images
> > >> > to one of multiple sets(also called banks) of images. The firmware
> > >> > images are clubbed together in banks, with the system booting images
> > >> > from the active bank. Information on the images such as which bank
> > >> > they belong to is stored as part of the metadata structure, which is
> > >> > stored on the same storage media as the firmware images on a dedicated
> > >> > partition.
> > >> >
> > >> > At the time of update, the metadata is read to identify the bank to
> > >> > which the images need to be flashed(update bank). On a successful
> > >> > update, the metadata is modified to set the updated bank as active
> > >> > bank to subsequently boot from.
> > >> >
> > >> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > >> > ---
> > >> > Changes since V1:
> > >> > * Call function fwu_update_checks_pass to check if the
> > >> >   update can be initiated
> > >> > * Do not allow firmware update from efi_init_obj_list as the
> > >> >   fwu boot-time checks need to be run
> > >> >
> > >> >  include/fwu.h                |  18 +++-
> > >> >  lib/Kconfig                  |  32 ++++++
> > >> >  lib/Makefile                 |   1 +
> > >> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
> > >> >  lib/efi_loader/efi_setup.c   |   3 +-
> > >> >  lib/fwu_updates/Makefile     |  11 ++
> > >> >  lib/fwu_updates/fwu.c        |  27 +++++
> > >> >  7 files changed, 284 insertions(+), 6 deletions(-)
> > >> >  create mode 100644 lib/fwu_updates/Makefile
> > >> >
> > >> > diff --git a/include/fwu.h b/include/fwu.h
> > >> > index 2d2e674d6a..bf50fe9277 100644
> > >> > --- a/include/fwu.h
> > >> > +++ b/include/fwu.h
> > >> > @@ -10,14 +10,28 @@
> > >> >
> > >> >  #include <linux/types.h>
> > >> >
> > >> > -#define FWU_MDATA_VERSION    0x1
> > >> > +#define FWU_MDATA_GUID \
> > >> > +     EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > >> > +              0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > >> > +
> > >> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> > >> > +     EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> > >> > +              0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> > >> > +
> > >> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> > >> > +     EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> > >> > +              0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> > >> >
> > >> >  #define FWU_MDATA_GUID \
> > >> >       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > >> >                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > >> >
> > >> > -int fwu_boottime_checks(void);
> > >> > +#define FWU_MDATA_VERSION    0x1
> > >> > +#define FWU_IMAGE_ACCEPTED   0x1
> > >> > +
> > >> >  u8 fwu_update_checks_pass(void);
> > >> > +int fwu_boottime_checks(void);
> > >> > +int fwu_trial_state_ctr_start(void);
> > >> >
> > >> >  int fwu_get_active_index(u32 *active_idx);
> > >> >  int fwu_update_active_index(u32 active_idx);
> > >> > diff --git a/lib/Kconfig b/lib/Kconfig
> > >> > index 807a4c6ade..7cb306317c 100644
> > >> > --- a/lib/Kconfig
> > >> > +++ b/lib/Kconfig
> > >> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> > >> >         When there are multiple device tree nodes with same name,
> > >> >            enable this config option to distinguish them using
> > >> >         phandles in fdtdec_get_alias_seq() function.
> > >> > +
> > >> > +config FWU_MULTI_BANK_UPDATE
> > >> > +     bool "Enable FWU Multi Bank Update Feature"
> > >> > +     depends on EFI_HAVE_CAPSULE_SUPPORT
> > >> > +     select PARTITION_TYPE_GUID
> > >> > +     select EFI_SETUP_EARLY
> > >> > +     help
> > >> > +       Feature for updating firmware images on platforms having
> > >> > +       multiple banks(copies) of the firmware images. One of the
> > >> > +       bank is selected for updating all the firmware components
> > >> > +
> > >> > +config FWU_NUM_BANKS
> > >> > +     int "Number of Banks defined by the platform"
> > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > >> > +     help
> > >> > +       Define the number of banks of firmware images on a platform
> > >> > +
> > >> > +config FWU_NUM_IMAGES_PER_BANK
> > >> > +     int "Number of firmware images per bank"
> > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > >> > +     help
> > >> > +       Define the number of firmware images per bank. This value
> > >> > +       should be the same for all the banks.
> > >> > +
> > >> > +config FWU_TRIAL_STATE_CNT
> > >> > +     int "Number of times system boots in Trial State"
> > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > >> > +     default 3
> > >> > +     help
> > >> > +       With FWU Multi Bank Update feature enabled, number of times
> > >> > +       the platform is allowed to boot in Trial State after an
> > >> > +       update.
> > >> > diff --git a/lib/Makefile b/lib/Makefile
> > >> > index 5ddbc77ed6..bc5c1e22fc 100644
> > >> > --- a/lib/Makefile
> > >> > +++ b/lib/Makefile
> > >> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
> > >> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
> > >> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
> > >> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
> > >> >  obj-$(CONFIG_LZMA) += lzma/
> > >> >  obj-$(CONFIG_BZIP2) += bzip2/
> > >> >  obj-$(CONFIG_TIZEN) += tizen/
> > >> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > >> > index 8301eed631..6dfe56bb0f 100644
> > >> > --- a/lib/efi_loader/efi_capsule.c
> > >> > +++ b/lib/efi_loader/efi_capsule.c
> > >> > @@ -14,6 +14,7 @@
> > >> >  #include <env.h>
> > >> >  #include <fdtdec.h>
> > >> >  #include <fs.h>
> > >> > +#include <fwu.h>
> > >> >  #include <malloc.h>
> > >> >  #include <mapmem.h>
> > >> >  #include <sort.h>
> > >> > @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
> > >> >               EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > >> >  const efi_guid_t efi_guid_firmware_management_protocol =
> > >> >               EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> > >> > +const efi_guid_t fwu_guid_os_request_fw_revert =
> > >> > +             FWU_OS_REQUEST_FW_REVERT_GUID;
> > >> > +const efi_guid_t fwu_guid_os_request_fw_accept =
> > >> > +             FWU_OS_REQUEST_FW_ACCEPT_GUID;
> > >> > +
> > >> > +__maybe_unused static u32 update_index;
> > >> > +__maybe_unused static bool capsule_update;
> > >> >
> > >> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
> > >> >  /* for file system access */
> > >> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
> > >> >       void *image_binary, *vendor_code;
> > >> >       efi_handle_t *handles;
> > >> >       efi_uintn_t no_handles;
> > >> > -     int item;
> > >> > +     int item, alt_no;
> > >> >       struct efi_firmware_management_protocol *fmp;
> > >> >       u16 *abort_reason;
> > >> > +     efi_guid_t image_type_id;
> > >> >       efi_status_t ret = EFI_SUCCESS;
> > >> > +     int status;
> > >> > +     u8 image_index;
> > >> >
> > >> >       /* sanity check */
> > >> >       if (capsule_data->header_size < sizeof(*capsule) ||
> > >> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
> > >> >                               goto out;
> > >> >               }
> > >> >
> > >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +                     /*
> > >> > +                      * Based on the value of update_image_type_id,
> > >> > +                      * derive the alt number value. This will be
> > >> > +                      * passed as update_image_index to the
> > >> > +                      * set_image function.
> > >> > +                      */
> > >> > +                     image_type_id = image->update_image_type_id;
> > >> > +                     status = fwu_get_image_alt_num(image_type_id,
> > >> > +                                                    update_index,
> > >> > +                                                    &alt_no);
> > >> > +                     if (status < 0) {
> > >> > +                             log_err("Unable to get the alt no for the image type %pUl\n",
> > >> > +                                     &image_type_id);
> > >> > +                             if (status == -ENODEV || status == -EIO)
> > >> > +                                     ret = EFI_DEVICE_ERROR;
> > >> > +                             else if (status == -ENOMEM)
> > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > >> > +                             else if (status == -ERANGE || status == -EINVAL)
> > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > >> > +                             goto out;
> > >> > +                     }
> > >> > +                     log_debug("alt_no %u for Image Type Id %pUl\n",
> > >> > +                               alt_no, &image_type_id);
> > >> > +                     image_index = alt_no + 1;
> > >> > +             } else {
> > >> > +                     image_index = image->update_image_index;
> > >> > +             }
> > >> >               abort_reason = NULL;
> > >> > -             ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> > >> > +             ret = EFI_CALL(fmp->set_image(fmp, image_index,
> > >> >                                             image_binary,
> > >> >                                             image_binary_size,
> > >> >                                             vendor_code, NULL,
> > >> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
> > >> >                       efi_free_pool(abort_reason);
> > >> >                       goto out;
> > >> >               }
> > >> > +
> > >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +                     status = fwu_clear_accept_image(&image_type_id,
> > >> > +                                                     update_index);
> > >> > +                     if (status < 0) {
> > >> > +                             log_err("Unable to clear the accept bit for the image %pUl\n",
> > >> > +                                     &image_type_id);
> > >> > +                             if (status == -ENODEV || status == -EIO)
> > >> > +                                     ret = EFI_DEVICE_ERROR;
> > >> > +                             else if (status == -ENOMEM)
> > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > >> > +                             else if (status == -ERANGE || status == -EINVAL)
> > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > >> > +                             goto out;
> > >> > +                     }
> > >> > +                     log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
> > >> > +             }
> > >> > +
> > >> >       }
> > >> >
> > >> >  out:
> > >> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
> > >> >               u64 scatter_gather_list)
> > >> >  {
> > >> >       struct efi_capsule_header *capsule;
> > >> > +     efi_guid_t *image_guid;
> > >> > +     u32 active_idx;
> > >> > +     int status;
> > >> >       unsigned int i;
> > >> >       efi_status_t ret;
> > >> >
> > >> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
> > >> >               goto out;
> > >> >       }
> > >> >
> > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +             /* Obtain the update_index from the platform */
> > >> > +             status = fwu_plat_get_update_index(&update_index);
> > >> > +             if (status < 0) {
> > >> > +                     log_err("Failed to get the FWU update_index value\n");
> > >> > +                     ret = EFI_DEVICE_ERROR;
> > >> > +                     goto out;
> > >> > +             }
> > >> > +     }
> > >> > +
> > >> >       ret = EFI_SUCCESS;
> > >> >       for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> > >> >            i++, capsule = *(++capsule_header_array)) {
> > >> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
> > >> >                         i, &capsule->capsule_guid);
> > >> >               if (!guidcmp(&capsule->capsule_guid,
> > >> >                            &efi_guid_firmware_management_capsule_id)) {
> > >> > +                     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +                             if (!fwu_update_checks_pass()) {
> > >> > +                                     log_err("FWU checks failed. Cannot start update\n");
> > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > >> > +                                     goto out;
> > >> > +                             }
> > >> > +                     }
> > >> > +
> > >> >                       ret  = efi_capsule_update_firmware(capsule);
> > >> > +                     capsule_update = true;
> > >> > +             } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +                     capsule_update = false;
> > >> > +                     if (!guidcmp(&capsule->capsule_guid,
> > >> > +                                  &fwu_guid_os_request_fw_revert)) {
> > >> > +                             /*
> > >> > +                              * One of the previously updated image has
> > >> > +                              * failed the OS acceptance test. OS has
> > >> > +                              * requested to revert back to the earlier
> > >> > +                              * boot index
> > >> > +                              */
> > >> > +                             status = fwu_revert_boot_index();
> > >> > +                             if (status < 0) {
> > >> > +                                     log_err("Failed to revert the FWU boot index\n");
> > >> > +                                     if (status == -ENODEV ||
> > >> > +                                         status == -ERANGE ||
> > >> > +                                         status == -EIO)
> > >> > +                                             ret = EFI_DEVICE_ERROR;
> > >> > +                                     else if (status == -EINVAL)
> > >> > +                                             ret = EFI_INVALID_PARAMETER;
> > >> > +                                     else if (status == -ENOMEM)
> > >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > >> > +                             } else {
> > >> > +                                     ret = EFI_SUCCESS;
> > >> > +                                     log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
> > >> > +                                             active_idx);
> > >> > +                             }
> > >> > +                     } else if (!guidcmp(&capsule->capsule_guid,
> > >> > +                                         &fwu_guid_os_request_fw_accept)) {
> > >> > +                             /*
> > >> > +                              * Image accepted by the OS. Set the acceptance
> > >> > +                              * status for the image.
> > >> > +                              */
> > >> > +                             image_guid = (void *)(char *)capsule +
> > >> > +                                     capsule->header_size;
> > >> > +                             status = fwu_accept_image(image_guid);
> > >> > +                             if (status < 0) {
> > >> > +                                     if (status == -ENODEV ||
> > >> > +                                         status == -ERANGE ||
> > >> > +                                         status == -EIO)
> > >> > +                                             ret = EFI_DEVICE_ERROR;
> > >> > +                                     else if (status == -EINVAL)
> > >> > +                                             ret = EFI_INVALID_PARAMETER;
> > >> > +                                     else if (status == -ENOMEM)
> > >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > >> > +                             } else {
> > >> > +                                     ret = EFI_SUCCESS;
> > >> > +                             }
> > >> > +                     }
> > >> >               } else {
> > >> >                       log_err("Unsupported capsule type: %pUl\n",
> > >> >                               &capsule->capsule_guid);
> > >> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
> > >> >                       goto out;
> > >> >       }
> > >> >
> > >> > +     /*
> > >> > +      * Update the FWU metadata once all the capsules have
> > >> > +      * been updated. This is done only for the Runtime
> > >> > +      * capsule update service.
> > >> > +      * The update_index value now gets written to the
> > >> > +      * active_index and the update_index value also
> > >> > +      * gets updated.
> > >> > +      * For the capsule-on-disk feature, the updation
> > >> > +      * of the FWU metadata happens in efi_launch_capsules
> > >> > +      */
> > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > >> > +         !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> > >> > +             status = fwu_update_active_index(update_index);
> > >> > +             if (status < 0) {
> > >> > +                     log_err("Failed to update FWU metadata index values\n");
> > >> > +                     if (status == -EINVAL || status == -ERANGE)
> > >> > +                             ret = EFI_INVALID_PARAMETER;
> > >> > +                     else if (status == -ENODEV || status == -EIO)
> > >> > +                             ret = EFI_DEVICE_ERROR;
> > >> > +                     else if (status == -ENOMEM)
> > >> > +                             ret = EFI_OUT_OF_RESOURCES;
> > >> > +             } else {
> > >> > +                     status = fwu_trial_state_ctr_start();
> > >> > +                     if (status < 0)
> > >> > +                             ret = EFI_DEVICE_ERROR;
> > >> > +                     else
> > >> > +                             ret = EFI_SUCCESS;
> > >> > +             }
> > >> > +     }
> > >> > +
> > >> >       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> > >> >               /* Rebuild the ESRT to reflect any updated FW images. */
> > >> >               ret = efi_esrt_populate();
> > >> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
> > >> >  {
> > >> >       struct efi_capsule_header *capsule = NULL;
> > >> >       u16 **files;
> > >> > +     int status;
> > >> >       unsigned int nfiles, index, i;
> > >> >       efi_status_t ret;
> > >> > +     bool update_status = true;
> > >> >
> > >> >       if (check_run_capsules() != EFI_SUCCESS)
> > >> >               return EFI_SUCCESS;
> > >> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
> > >> >               ret = efi_capsule_read_file(files[i], &capsule);
> > >> >               if (ret == EFI_SUCCESS) {
> > >> >                       ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> > >> > -                     if (ret != EFI_SUCCESS)
> > >> > +                     if (ret != EFI_SUCCESS) {
> > >> >                               log_err("Applying capsule %ls failed\n",
> > >> >                                       files[i]);
> > >> > +                             update_status = false;
> > >> > +                     }
> > >> >
> > >> >                       /* create CapsuleXXXX */
> > >> >                       set_capsule_result(index, capsule, ret);
> > >> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
> > >> >                       free(capsule);
> > >> >               } else {
> > >> >                       log_err("Reading capsule %ls failed\n", files[i]);
> > >> > +                     update_status = false;
> > >> >               }
> > >> >               /* delete a capsule either in case of success or failure */
> > >> >               ret = efi_capsule_delete_file(files[i]);
> > >> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
> > >> >                       log_err("Deleting capsule %ls failed\n",
> > >> >                               files[i]);
> > >> >       }
> > >> > +
> > >> >       efi_capsule_scan_done();
> > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > >> > +             if (update_status == true && capsule_update == true) {
> > >> > +                     /*
> > >> > +                      * All the capsules have been updated successfully,
> > >> > +                      * update the FWU metadata.
> > >> > +                      */
> > >> > +                     log_debug("Update Complete. Now updating active_index to %u\n",
> > >> > +                               update_index);
> > >> > +                     status = fwu_update_active_index(update_index);
> > >> > +                     if (status < 0) {
> > >> > +                             log_err("Failed to update FWU metadata index values\n");
> > >> > +                             if (status == -EINVAL || status == -ERANGE)
> > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > >> > +                             else if (status == -ENODEV || status == -EIO)
> > >> > +                                     ret = EFI_DEVICE_ERROR;
> > >> > +                             else if (status == -ENOMEM)
> > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > >> > +                     } else {
> > >> > +                             log_debug("Successfully updated the active_index\n");
> > >> > +                             status = fwu_trial_state_ctr_start();
> > >> > +                             if (status < 0)
> > >> > +                                     ret = EFI_DEVICE_ERROR;
> > >> > +                             else
> > >> > +                                     ret = EFI_SUCCESS;
> > >> > +                     }
> > >> > +             } else if (capsule_update == true && update_status == false) {
> > >> > +                     log_err("All capsules were not updated. Not updating FWU metadata\n");
> > >> > +             }
> > >> > +     }
> > >> >
> > >> >       for (i = 0; i < nfiles; i++)
> > >> >               free(files[i]);
> > >> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > >> > index 1aba71cd96..6601176a2d 100644
> > >> > --- a/lib/efi_loader/efi_setup.c
> > >> > +++ b/lib/efi_loader/efi_setup.c
> > >> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
> > >> >               goto out;
> > >> >
> > >> >       /* Execute capsules after reboot */
> > >> > -     if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > >> > +     if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > >> > +         IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > >> >           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> > >> >               ret = efi_launch_capsules();
> > >> >  out:
> > >> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> > >> > new file mode 100644
> > >> > index 0000000000..73099a30cb
> > >> > --- /dev/null
> > >> > +++ b/lib/fwu_updates/Makefile
> > >> > @@ -0,0 +1,11 @@
> > >> > +# SPDX-License-Identifier: GPL-2.0+
> > >> > +#
> > >> > +# Copyright (c) 2021, Linaro Limited
> > >> > +#
> > >> > +
> > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> > >> > +
> > >> > +ifdef CONFIG_EFI_PARTITION
> > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> > >> > +endif
> > >>
> > >> If I understand correctly, any platform may have its own
> > >> fwu_mdata_ops. So we should not unconditionally compile in
> > >> fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.
> > >
> > >
> > > I can put in an additional guard of CONFIG_BLK for building the file. Not sure if there would be a requirement to have a platform specific set of operations for a GPT partitioned block device. Will wait for Masami to explain.
> >
> >
> > OK, I'm trying to implement the AB update on the developerbox
> > platform, which doesn't have GPT, nor BLK since the firmware will be
> > stored on the SPI NOR flash. So I will introduce a new fwu_mdata_sf.c.
> > For this purpose, I introduced CONFIG_FWU_MULTI_BANK_UPDATE_SF, which
> > depends on CONFIG_FWU_MULTI_BANK_UPDATE and CONFIG_DM_SPI_FLASH. Thus,
> > I also recommend you to introduce CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
> > and it should depends on CONFIG_BLK and selects CONFIG_EFI_PARTITION.
> >
> > BTW, there are some code pieces which can shared with the
> > fwu_mdata_gpt_blk.c, for example, comparing primary and seconday
> > metadata and calculate the crc32.
>
> Here's my concern / question.  At what level is this abstraction being
> used?  I want to be sure that other SoCs, such as say a Rockchip
> platform (I have a SystemReady IR certified one on the way) can re-use
> this code as well but I'm sure it will need things written to different
> offsets within the SPI/eMMC.  Thanks!

For the metadata, I introduced CONFIG_FWU_SF_PRIMARY_METADATA_OFFSET
and CONFIG_FWU_SF_PRIMARY_METADATA_OFFSET for configuring offset of
the metadata on SPI Flash for my prototype code.
I think it might be better to introduce accessors for the metadata,
e.g. fwu_load_pri_mdata()/fwu_save_sec_mdata() etc. so that the
platform can choose the media which stores metadata independent from
the firmware storage.

For the firmware offset, it is defined in the dfu_alt_info, so you can
use (or even mix) any storage media like SPI nor or MTD or eMMC.

Thank you,


>
> --
> Tom

On Tue, Dec 21, 2021 at 08:30:02AM +0900, Masami Hiramatsu wrote:
> Hi Tom,
> 
> 2021年12月21日(火) 1:36 Tom Rini <trini@konsulko.com>:
> >
> > On Mon, Dec 20, 2021 at 10:13:35PM +0900, Masami Hiramatsu wrote:
> > > Hi Sughosh,
> > >
> > > 2021年12月20日(月) 18:48 Sughosh Ganu <sughosh.ganu@linaro.org>:
> > > >
> > > >
> > > > On Mon, 20 Dec 2021 at 11:44, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > >>
> > > >> On Sun, Dec 19, 2021 at 12:36:04PM +0530, Sughosh Ganu wrote:
> > > >> > The FWU Multi Bank Update feature supports updation of firmware images
> > > >> > to one of multiple sets(also called banks) of images. The firmware
> > > >> > images are clubbed together in banks, with the system booting images
> > > >> > from the active bank. Information on the images such as which bank
> > > >> > they belong to is stored as part of the metadata structure, which is
> > > >> > stored on the same storage media as the firmware images on a dedicated
> > > >> > partition.
> > > >> >
> > > >> > At the time of update, the metadata is read to identify the bank to
> > > >> > which the images need to be flashed(update bank). On a successful
> > > >> > update, the metadata is modified to set the updated bank as active
> > > >> > bank to subsequently boot from.
> > > >> >
> > > >> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > >> > ---
> > > >> > Changes since V1:
> > > >> > * Call function fwu_update_checks_pass to check if the
> > > >> >   update can be initiated
> > > >> > * Do not allow firmware update from efi_init_obj_list as the
> > > >> >   fwu boot-time checks need to be run
> > > >> >
> > > >> >  include/fwu.h                |  18 +++-
> > > >> >  lib/Kconfig                  |  32 ++++++
> > > >> >  lib/Makefile                 |   1 +
> > > >> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
> > > >> >  lib/efi_loader/efi_setup.c   |   3 +-
> > > >> >  lib/fwu_updates/Makefile     |  11 ++
> > > >> >  lib/fwu_updates/fwu.c        |  27 +++++
> > > >> >  7 files changed, 284 insertions(+), 6 deletions(-)
> > > >> >  create mode 100644 lib/fwu_updates/Makefile
> > > >> >
> > > >> > diff --git a/include/fwu.h b/include/fwu.h
> > > >> > index 2d2e674d6a..bf50fe9277 100644
> > > >> > --- a/include/fwu.h
> > > >> > +++ b/include/fwu.h
> > > >> > @@ -10,14 +10,28 @@
> > > >> >
> > > >> >  #include <linux/types.h>
> > > >> >
> > > >> > -#define FWU_MDATA_VERSION    0x1
> > > >> > +#define FWU_MDATA_GUID \
> > > >> > +     EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > > >> > +              0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > > >> > +
> > > >> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> > > >> > +     EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> > > >> > +              0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> > > >> > +
> > > >> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> > > >> > +     EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> > > >> > +              0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> > > >> >
> > > >> >  #define FWU_MDATA_GUID \
> > > >> >       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > > >> >                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > > >> >
> > > >> > -int fwu_boottime_checks(void);
> > > >> > +#define FWU_MDATA_VERSION    0x1
> > > >> > +#define FWU_IMAGE_ACCEPTED   0x1
> > > >> > +
> > > >> >  u8 fwu_update_checks_pass(void);
> > > >> > +int fwu_boottime_checks(void);
> > > >> > +int fwu_trial_state_ctr_start(void);
> > > >> >
> > > >> >  int fwu_get_active_index(u32 *active_idx);
> > > >> >  int fwu_update_active_index(u32 active_idx);
> > > >> > diff --git a/lib/Kconfig b/lib/Kconfig
> > > >> > index 807a4c6ade..7cb306317c 100644
> > > >> > --- a/lib/Kconfig
> > > >> > +++ b/lib/Kconfig
> > > >> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> > > >> >         When there are multiple device tree nodes with same name,
> > > >> >            enable this config option to distinguish them using
> > > >> >         phandles in fdtdec_get_alias_seq() function.
> > > >> > +
> > > >> > +config FWU_MULTI_BANK_UPDATE
> > > >> > +     bool "Enable FWU Multi Bank Update Feature"
> > > >> > +     depends on EFI_HAVE_CAPSULE_SUPPORT
> > > >> > +     select PARTITION_TYPE_GUID
> > > >> > +     select EFI_SETUP_EARLY
> > > >> > +     help
> > > >> > +       Feature for updating firmware images on platforms having
> > > >> > +       multiple banks(copies) of the firmware images. One of the
> > > >> > +       bank is selected for updating all the firmware components
> > > >> > +
> > > >> > +config FWU_NUM_BANKS
> > > >> > +     int "Number of Banks defined by the platform"
> > > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > > >> > +     help
> > > >> > +       Define the number of banks of firmware images on a platform
> > > >> > +
> > > >> > +config FWU_NUM_IMAGES_PER_BANK
> > > >> > +     int "Number of firmware images per bank"
> > > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > > >> > +     help
> > > >> > +       Define the number of firmware images per bank. This value
> > > >> > +       should be the same for all the banks.
> > > >> > +
> > > >> > +config FWU_TRIAL_STATE_CNT
> > > >> > +     int "Number of times system boots in Trial State"
> > > >> > +     depends on FWU_MULTI_BANK_UPDATE
> > > >> > +     default 3
> > > >> > +     help
> > > >> > +       With FWU Multi Bank Update feature enabled, number of times
> > > >> > +       the platform is allowed to boot in Trial State after an
> > > >> > +       update.
> > > >> > diff --git a/lib/Makefile b/lib/Makefile
> > > >> > index 5ddbc77ed6..bc5c1e22fc 100644
> > > >> > --- a/lib/Makefile
> > > >> > +++ b/lib/Makefile
> > > >> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
> > > >> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
> > > >> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
> > > >> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> > > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
> > > >> >  obj-$(CONFIG_LZMA) += lzma/
> > > >> >  obj-$(CONFIG_BZIP2) += bzip2/
> > > >> >  obj-$(CONFIG_TIZEN) += tizen/
> > > >> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > >> > index 8301eed631..6dfe56bb0f 100644
> > > >> > --- a/lib/efi_loader/efi_capsule.c
> > > >> > +++ b/lib/efi_loader/efi_capsule.c
> > > >> > @@ -14,6 +14,7 @@
> > > >> >  #include <env.h>
> > > >> >  #include <fdtdec.h>
> > > >> >  #include <fs.h>
> > > >> > +#include <fwu.h>
> > > >> >  #include <malloc.h>
> > > >> >  #include <mapmem.h>
> > > >> >  #include <sort.h>
> > > >> > @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
> > > >> >               EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> > > >> >  const efi_guid_t efi_guid_firmware_management_protocol =
> > > >> >               EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> > > >> > +const efi_guid_t fwu_guid_os_request_fw_revert =
> > > >> > +             FWU_OS_REQUEST_FW_REVERT_GUID;
> > > >> > +const efi_guid_t fwu_guid_os_request_fw_accept =
> > > >> > +             FWU_OS_REQUEST_FW_ACCEPT_GUID;
> > > >> > +
> > > >> > +__maybe_unused static u32 update_index;
> > > >> > +__maybe_unused static bool capsule_update;
> > > >> >
> > > >> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
> > > >> >  /* for file system access */
> > > >> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
> > > >> >       void *image_binary, *vendor_code;
> > > >> >       efi_handle_t *handles;
> > > >> >       efi_uintn_t no_handles;
> > > >> > -     int item;
> > > >> > +     int item, alt_no;
> > > >> >       struct efi_firmware_management_protocol *fmp;
> > > >> >       u16 *abort_reason;
> > > >> > +     efi_guid_t image_type_id;
> > > >> >       efi_status_t ret = EFI_SUCCESS;
> > > >> > +     int status;
> > > >> > +     u8 image_index;
> > > >> >
> > > >> >       /* sanity check */
> > > >> >       if (capsule_data->header_size < sizeof(*capsule) ||
> > > >> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
> > > >> >                               goto out;
> > > >> >               }
> > > >> >
> > > >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +                     /*
> > > >> > +                      * Based on the value of update_image_type_id,
> > > >> > +                      * derive the alt number value. This will be
> > > >> > +                      * passed as update_image_index to the
> > > >> > +                      * set_image function.
> > > >> > +                      */
> > > >> > +                     image_type_id = image->update_image_type_id;
> > > >> > +                     status = fwu_get_image_alt_num(image_type_id,
> > > >> > +                                                    update_index,
> > > >> > +                                                    &alt_no);
> > > >> > +                     if (status < 0) {
> > > >> > +                             log_err("Unable to get the alt no for the image type %pUl\n",
> > > >> > +                                     &image_type_id);
> > > >> > +                             if (status == -ENODEV || status == -EIO)
> > > >> > +                                     ret = EFI_DEVICE_ERROR;
> > > >> > +                             else if (status == -ENOMEM)
> > > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > > >> > +                             else if (status == -ERANGE || status == -EINVAL)
> > > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > > >> > +                             goto out;
> > > >> > +                     }
> > > >> > +                     log_debug("alt_no %u for Image Type Id %pUl\n",
> > > >> > +                               alt_no, &image_type_id);
> > > >> > +                     image_index = alt_no + 1;
> > > >> > +             } else {
> > > >> > +                     image_index = image->update_image_index;
> > > >> > +             }
> > > >> >               abort_reason = NULL;
> > > >> > -             ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> > > >> > +             ret = EFI_CALL(fmp->set_image(fmp, image_index,
> > > >> >                                             image_binary,
> > > >> >                                             image_binary_size,
> > > >> >                                             vendor_code, NULL,
> > > >> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
> > > >> >                       efi_free_pool(abort_reason);
> > > >> >                       goto out;
> > > >> >               }
> > > >> > +
> > > >> > +             if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +                     status = fwu_clear_accept_image(&image_type_id,
> > > >> > +                                                     update_index);
> > > >> > +                     if (status < 0) {
> > > >> > +                             log_err("Unable to clear the accept bit for the image %pUl\n",
> > > >> > +                                     &image_type_id);
> > > >> > +                             if (status == -ENODEV || status == -EIO)
> > > >> > +                                     ret = EFI_DEVICE_ERROR;
> > > >> > +                             else if (status == -ENOMEM)
> > > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > > >> > +                             else if (status == -ERANGE || status == -EINVAL)
> > > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > > >> > +                             goto out;
> > > >> > +                     }
> > > >> > +                     log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
> > > >> > +             }
> > > >> > +
> > > >> >       }
> > > >> >
> > > >> >  out:
> > > >> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
> > > >> >               u64 scatter_gather_list)
> > > >> >  {
> > > >> >       struct efi_capsule_header *capsule;
> > > >> > +     efi_guid_t *image_guid;
> > > >> > +     u32 active_idx;
> > > >> > +     int status;
> > > >> >       unsigned int i;
> > > >> >       efi_status_t ret;
> > > >> >
> > > >> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
> > > >> >               goto out;
> > > >> >       }
> > > >> >
> > > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +             /* Obtain the update_index from the platform */
> > > >> > +             status = fwu_plat_get_update_index(&update_index);
> > > >> > +             if (status < 0) {
> > > >> > +                     log_err("Failed to get the FWU update_index value\n");
> > > >> > +                     ret = EFI_DEVICE_ERROR;
> > > >> > +                     goto out;
> > > >> > +             }
> > > >> > +     }
> > > >> > +
> > > >> >       ret = EFI_SUCCESS;
> > > >> >       for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> > > >> >            i++, capsule = *(++capsule_header_array)) {
> > > >> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
> > > >> >                         i, &capsule->capsule_guid);
> > > >> >               if (!guidcmp(&capsule->capsule_guid,
> > > >> >                            &efi_guid_firmware_management_capsule_id)) {
> > > >> > +                     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +                             if (!fwu_update_checks_pass()) {
> > > >> > +                                     log_err("FWU checks failed. Cannot start update\n");
> > > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > > >> > +                                     goto out;
> > > >> > +                             }
> > > >> > +                     }
> > > >> > +
> > > >> >                       ret  = efi_capsule_update_firmware(capsule);
> > > >> > +                     capsule_update = true;
> > > >> > +             } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +                     capsule_update = false;
> > > >> > +                     if (!guidcmp(&capsule->capsule_guid,
> > > >> > +                                  &fwu_guid_os_request_fw_revert)) {
> > > >> > +                             /*
> > > >> > +                              * One of the previously updated image has
> > > >> > +                              * failed the OS acceptance test. OS has
> > > >> > +                              * requested to revert back to the earlier
> > > >> > +                              * boot index
> > > >> > +                              */
> > > >> > +                             status = fwu_revert_boot_index();
> > > >> > +                             if (status < 0) {
> > > >> > +                                     log_err("Failed to revert the FWU boot index\n");
> > > >> > +                                     if (status == -ENODEV ||
> > > >> > +                                         status == -ERANGE ||
> > > >> > +                                         status == -EIO)
> > > >> > +                                             ret = EFI_DEVICE_ERROR;
> > > >> > +                                     else if (status == -EINVAL)
> > > >> > +                                             ret = EFI_INVALID_PARAMETER;
> > > >> > +                                     else if (status == -ENOMEM)
> > > >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > > >> > +                             } else {
> > > >> > +                                     ret = EFI_SUCCESS;
> > > >> > +                                     log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
> > > >> > +                                             active_idx);
> > > >> > +                             }
> > > >> > +                     } else if (!guidcmp(&capsule->capsule_guid,
> > > >> > +                                         &fwu_guid_os_request_fw_accept)) {
> > > >> > +                             /*
> > > >> > +                              * Image accepted by the OS. Set the acceptance
> > > >> > +                              * status for the image.
> > > >> > +                              */
> > > >> > +                             image_guid = (void *)(char *)capsule +
> > > >> > +                                     capsule->header_size;
> > > >> > +                             status = fwu_accept_image(image_guid);
> > > >> > +                             if (status < 0) {
> > > >> > +                                     if (status == -ENODEV ||
> > > >> > +                                         status == -ERANGE ||
> > > >> > +                                         status == -EIO)
> > > >> > +                                             ret = EFI_DEVICE_ERROR;
> > > >> > +                                     else if (status == -EINVAL)
> > > >> > +                                             ret = EFI_INVALID_PARAMETER;
> > > >> > +                                     else if (status == -ENOMEM)
> > > >> > +                                             ret = EFI_OUT_OF_RESOURCES;
> > > >> > +                             } else {
> > > >> > +                                     ret = EFI_SUCCESS;
> > > >> > +                             }
> > > >> > +                     }
> > > >> >               } else {
> > > >> >                       log_err("Unsupported capsule type: %pUl\n",
> > > >> >                               &capsule->capsule_guid);
> > > >> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
> > > >> >                       goto out;
> > > >> >       }
> > > >> >
> > > >> > +     /*
> > > >> > +      * Update the FWU metadata once all the capsules have
> > > >> > +      * been updated. This is done only for the Runtime
> > > >> > +      * capsule update service.
> > > >> > +      * The update_index value now gets written to the
> > > >> > +      * active_index and the update_index value also
> > > >> > +      * gets updated.
> > > >> > +      * For the capsule-on-disk feature, the updation
> > > >> > +      * of the FWU metadata happens in efi_launch_capsules
> > > >> > +      */
> > > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > > >> > +         !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> > > >> > +             status = fwu_update_active_index(update_index);
> > > >> > +             if (status < 0) {
> > > >> > +                     log_err("Failed to update FWU metadata index values\n");
> > > >> > +                     if (status == -EINVAL || status == -ERANGE)
> > > >> > +                             ret = EFI_INVALID_PARAMETER;
> > > >> > +                     else if (status == -ENODEV || status == -EIO)
> > > >> > +                             ret = EFI_DEVICE_ERROR;
> > > >> > +                     else if (status == -ENOMEM)
> > > >> > +                             ret = EFI_OUT_OF_RESOURCES;
> > > >> > +             } else {
> > > >> > +                     status = fwu_trial_state_ctr_start();
> > > >> > +                     if (status < 0)
> > > >> > +                             ret = EFI_DEVICE_ERROR;
> > > >> > +                     else
> > > >> > +                             ret = EFI_SUCCESS;
> > > >> > +             }
> > > >> > +     }
> > > >> > +
> > > >> >       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> > > >> >               /* Rebuild the ESRT to reflect any updated FW images. */
> > > >> >               ret = efi_esrt_populate();
> > > >> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
> > > >> >  {
> > > >> >       struct efi_capsule_header *capsule = NULL;
> > > >> >       u16 **files;
> > > >> > +     int status;
> > > >> >       unsigned int nfiles, index, i;
> > > >> >       efi_status_t ret;
> > > >> > +     bool update_status = true;
> > > >> >
> > > >> >       if (check_run_capsules() != EFI_SUCCESS)
> > > >> >               return EFI_SUCCESS;
> > > >> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
> > > >> >               ret = efi_capsule_read_file(files[i], &capsule);
> > > >> >               if (ret == EFI_SUCCESS) {
> > > >> >                       ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> > > >> > -                     if (ret != EFI_SUCCESS)
> > > >> > +                     if (ret != EFI_SUCCESS) {
> > > >> >                               log_err("Applying capsule %ls failed\n",
> > > >> >                                       files[i]);
> > > >> > +                             update_status = false;
> > > >> > +                     }
> > > >> >
> > > >> >                       /* create CapsuleXXXX */
> > > >> >                       set_capsule_result(index, capsule, ret);
> > > >> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
> > > >> >                       free(capsule);
> > > >> >               } else {
> > > >> >                       log_err("Reading capsule %ls failed\n", files[i]);
> > > >> > +                     update_status = false;
> > > >> >               }
> > > >> >               /* delete a capsule either in case of success or failure */
> > > >> >               ret = efi_capsule_delete_file(files[i]);
> > > >> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
> > > >> >                       log_err("Deleting capsule %ls failed\n",
> > > >> >                               files[i]);
> > > >> >       }
> > > >> > +
> > > >> >       efi_capsule_scan_done();
> > > >> > +     if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > > >> > +             if (update_status == true && capsule_update == true) {
> > > >> > +                     /*
> > > >> > +                      * All the capsules have been updated successfully,
> > > >> > +                      * update the FWU metadata.
> > > >> > +                      */
> > > >> > +                     log_debug("Update Complete. Now updating active_index to %u\n",
> > > >> > +                               update_index);
> > > >> > +                     status = fwu_update_active_index(update_index);
> > > >> > +                     if (status < 0) {
> > > >> > +                             log_err("Failed to update FWU metadata index values\n");
> > > >> > +                             if (status == -EINVAL || status == -ERANGE)
> > > >> > +                                     ret = EFI_INVALID_PARAMETER;
> > > >> > +                             else if (status == -ENODEV || status == -EIO)
> > > >> > +                                     ret = EFI_DEVICE_ERROR;
> > > >> > +                             else if (status == -ENOMEM)
> > > >> > +                                     ret = EFI_OUT_OF_RESOURCES;
> > > >> > +                     } else {
> > > >> > +                             log_debug("Successfully updated the active_index\n");
> > > >> > +                             status = fwu_trial_state_ctr_start();
> > > >> > +                             if (status < 0)
> > > >> > +                                     ret = EFI_DEVICE_ERROR;
> > > >> > +                             else
> > > >> > +                                     ret = EFI_SUCCESS;
> > > >> > +                     }
> > > >> > +             } else if (capsule_update == true && update_status == false) {
> > > >> > +                     log_err("All capsules were not updated. Not updating FWU metadata\n");
> > > >> > +             }
> > > >> > +     }
> > > >> >
> > > >> >       for (i = 0; i < nfiles; i++)
> > > >> >               free(files[i]);
> > > >> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > > >> > index 1aba71cd96..6601176a2d 100644
> > > >> > --- a/lib/efi_loader/efi_setup.c
> > > >> > +++ b/lib/efi_loader/efi_setup.c
> > > >> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
> > > >> >               goto out;
> > > >> >
> > > >> >       /* Execute capsules after reboot */
> > > >> > -     if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > > >> > +     if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > > >> > +         IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > > >> >           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> > > >> >               ret = efi_launch_capsules();
> > > >> >  out:
> > > >> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> > > >> > new file mode 100644
> > > >> > index 0000000000..73099a30cb
> > > >> > --- /dev/null
> > > >> > +++ b/lib/fwu_updates/Makefile
> > > >> > @@ -0,0 +1,11 @@
> > > >> > +# SPDX-License-Identifier: GPL-2.0+
> > > >> > +#
> > > >> > +# Copyright (c) 2021, Linaro Limited
> > > >> > +#
> > > >> > +
> > > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> > > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> > > >> > +
> > > >> > +ifdef CONFIG_EFI_PARTITION
> > > >> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> > > >> > +endif
> > > >>
> > > >> If I understand correctly, any platform may have its own
> > > >> fwu_mdata_ops. So we should not unconditionally compile in
> > > >> fwu_mdata_gpt_blk.o even if CONFIG_EFI_PARTITION is enabled.
> > > >
> > > >
> > > > I can put in an additional guard of CONFIG_BLK for building the file. Not sure if there would be a requirement to have a platform specific set of operations for a GPT partitioned block device. Will wait for Masami to explain.
> > >
> > >
> > > OK, I'm trying to implement the AB update on the developerbox
> > > platform, which doesn't have GPT, nor BLK since the firmware will be
> > > stored on the SPI NOR flash. So I will introduce a new fwu_mdata_sf.c.
> > > For this purpose, I introduced CONFIG_FWU_MULTI_BANK_UPDATE_SF, which
> > > depends on CONFIG_FWU_MULTI_BANK_UPDATE and CONFIG_DM_SPI_FLASH. Thus,
> > > I also recommend you to introduce CONFIG_FWU_MULTI_BANK_UPDATE_GPT_BLK
> > > and it should depends on CONFIG_BLK and selects CONFIG_EFI_PARTITION.
> > >
> > > BTW, there are some code pieces which can shared with the
> > > fwu_mdata_gpt_blk.c, for example, comparing primary and seconday
> > > metadata and calculate the crc32.
> >
> > Here's my concern / question.  At what level is this abstraction being
> > used?  I want to be sure that other SoCs, such as say a Rockchip
> > platform (I have a SystemReady IR certified one on the way) can re-use
> > this code as well but I'm sure it will need things written to different
> > offsets within the SPI/eMMC.  Thanks!
> 
> For the metadata, I introduced CONFIG_FWU_SF_PRIMARY_METADATA_OFFSET
> and CONFIG_FWU_SF_PRIMARY_METADATA_OFFSET for configuring offset of
> the metadata on SPI Flash for my prototype code.
> I think it might be better to introduce accessors for the metadata,
> e.g. fwu_load_pri_mdata()/fwu_save_sec_mdata() etc. so that the
> platform can choose the media which stores metadata independent from
> the firmware storage.
> 
> For the firmware offset, it is defined in the dfu_alt_info, so you can
> use (or even mix) any storage media like SPI nor or MTD or eMMC.

OK, good so you're keeping it in mind.  Is there any chance we can get a
second platform supported as part of the series itself, to confirm it's
sufficient?  I always worry a bit about things that are supposed to be
abstract enough to support anything but only implemented once.  Thanks!

Hi Sughosh,

Can you move the FWU related configs to lib/fwu_updates/Kconfig ?
FWU multi bank update is an independent feature, thus I think it is
better to have its own Kconfig file and the lib/Kconfig only includes
it.
(I did it on my development series)

Thank you,

2021年12月19日(日) 16:07 Sughosh Ganu <sughosh.ganu@linaro.org>:
>
> The FWU Multi Bank Update feature supports updation of firmware images
> to one of multiple sets(also called banks) of images. The firmware
> images are clubbed together in banks, with the system booting images
> from the active bank. Information on the images such as which bank
> they belong to is stored as part of the metadata structure, which is
> stored on the same storage media as the firmware images on a dedicated
> partition.
>
> At the time of update, the metadata is read to identify the bank to
> which the images need to be flashed(update bank). On a successful
> update, the metadata is modified to set the updated bank as active
> bank to subsequently boot from.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V1:
> * Call function fwu_update_checks_pass to check if the
>   update can be initiated
> * Do not allow firmware update from efi_init_obj_list as the
>   fwu boot-time checks need to be run
>
>  include/fwu.h                |  18 +++-
>  lib/Kconfig                  |  32 ++++++
>  lib/Makefile                 |   1 +
>  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
>  lib/efi_loader/efi_setup.c   |   3 +-
>  lib/fwu_updates/Makefile     |  11 ++
>  lib/fwu_updates/fwu.c        |  27 +++++
>  7 files changed, 284 insertions(+), 6 deletions(-)
>  create mode 100644 lib/fwu_updates/Makefile
>
> diff --git a/include/fwu.h b/include/fwu.h
> index 2d2e674d6a..bf50fe9277 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -10,14 +10,28 @@
>
>  #include <linux/types.h>
>
> -#define FWU_MDATA_VERSION      0x1
> +#define FWU_MDATA_GUID \
> +       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> +                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> +
> +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> +       EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> +                0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> +
> +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> +       EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> +                0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
>
>  #define FWU_MDATA_GUID \
>         EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
>                  0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
>
> -int fwu_boottime_checks(void);
> +#define FWU_MDATA_VERSION      0x1
> +#define FWU_IMAGE_ACCEPTED     0x1
> +
>  u8 fwu_update_checks_pass(void);
> +int fwu_boottime_checks(void);
> +int fwu_trial_state_ctr_start(void);
>
>  int fwu_get_active_index(u32 *active_idx);
>  int fwu_update_active_index(u32 active_idx);
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 807a4c6ade..7cb306317c 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
>           When there are multiple device tree nodes with same name,
>            enable this config option to distinguish them using
>           phandles in fdtdec_get_alias_seq() function.
> +
> +config FWU_MULTI_BANK_UPDATE
> +       bool "Enable FWU Multi Bank Update Feature"
> +       depends on EFI_HAVE_CAPSULE_SUPPORT
> +       select PARTITION_TYPE_GUID
> +       select EFI_SETUP_EARLY
> +       help
> +         Feature for updating firmware images on platforms having
> +         multiple banks(copies) of the firmware images. One of the
> +         bank is selected for updating all the firmware components
> +
> +config FWU_NUM_BANKS
> +       int "Number of Banks defined by the platform"
> +       depends on FWU_MULTI_BANK_UPDATE
> +       help
> +         Define the number of banks of firmware images on a platform
> +
> +config FWU_NUM_IMAGES_PER_BANK
> +       int "Number of firmware images per bank"
> +       depends on FWU_MULTI_BANK_UPDATE
> +       help
> +         Define the number of firmware images per bank. This value
> +         should be the same for all the banks.
> +
> +config FWU_TRIAL_STATE_CNT
> +       int "Number of times system boots in Trial State"
> +       depends on FWU_MULTI_BANK_UPDATE
> +       default 3
> +       help
> +         With FWU Multi Bank Update feature enabled, number of times
> +         the platform is allowed to boot in Trial State after an
> +         update.
> diff --git a/lib/Makefile b/lib/Makefile
> index 5ddbc77ed6..bc5c1e22fc 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
>  obj-$(CONFIG_EFI_LOADER) += efi_driver/
>  obj-$(CONFIG_EFI_LOADER) += efi_loader/
>  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
>  obj-$(CONFIG_LZMA) += lzma/
>  obj-$(CONFIG_BZIP2) += bzip2/
>  obj-$(CONFIG_TIZEN) += tizen/
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 8301eed631..6dfe56bb0f 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -14,6 +14,7 @@
>  #include <env.h>
>  #include <fdtdec.h>
>  #include <fs.h>
> +#include <fwu.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <sort.h>
> @@ -30,6 +31,13 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
>                 EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>  const efi_guid_t efi_guid_firmware_management_protocol =
>                 EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +const efi_guid_t fwu_guid_os_request_fw_revert =
> +               FWU_OS_REQUEST_FW_REVERT_GUID;
> +const efi_guid_t fwu_guid_os_request_fw_accept =
> +               FWU_OS_REQUEST_FW_ACCEPT_GUID;
> +
> +__maybe_unused static u32 update_index;
> +__maybe_unused static bool capsule_update;
>
>  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
>  /* for file system access */
> @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
>         void *image_binary, *vendor_code;
>         efi_handle_t *handles;
>         efi_uintn_t no_handles;
> -       int item;
> +       int item, alt_no;
>         struct efi_firmware_management_protocol *fmp;
>         u16 *abort_reason;
> +       efi_guid_t image_type_id;
>         efi_status_t ret = EFI_SUCCESS;
> +       int status;
> +       u8 image_index;
>
>         /* sanity check */
>         if (capsule_data->header_size < sizeof(*capsule) ||
> @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
>                                 goto out;
>                 }
>
> +               if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +                       /*
> +                        * Based on the value of update_image_type_id,
> +                        * derive the alt number value. This will be
> +                        * passed as update_image_index to the
> +                        * set_image function.
> +                        */
> +                       image_type_id = image->update_image_type_id;
> +                       status = fwu_get_image_alt_num(image_type_id,
> +                                                      update_index,
> +                                                      &alt_no);
> +                       if (status < 0) {
> +                               log_err("Unable to get the alt no for the image type %pUl\n",
> +                                       &image_type_id);
> +                               if (status == -ENODEV || status == -EIO)
> +                                       ret = EFI_DEVICE_ERROR;
> +                               else if (status == -ENOMEM)
> +                                       ret = EFI_OUT_OF_RESOURCES;
> +                               else if (status == -ERANGE || status == -EINVAL)
> +                                       ret = EFI_INVALID_PARAMETER;
> +                               goto out;
> +                       }
> +                       log_debug("alt_no %u for Image Type Id %pUl\n",
> +                                 alt_no, &image_type_id);
> +                       image_index = alt_no + 1;
> +               } else {
> +                       image_index = image->update_image_index;
> +               }
>                 abort_reason = NULL;
> -               ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
> +               ret = EFI_CALL(fmp->set_image(fmp, image_index,
>                                               image_binary,
>                                               image_binary_size,
>                                               vendor_code, NULL,
> @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
>                         efi_free_pool(abort_reason);
>                         goto out;
>                 }
> +
> +               if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +                       status = fwu_clear_accept_image(&image_type_id,
> +                                                       update_index);
> +                       if (status < 0) {
> +                               log_err("Unable to clear the accept bit for the image %pUl\n",
> +                                       &image_type_id);
> +                               if (status == -ENODEV || status == -EIO)
> +                                       ret = EFI_DEVICE_ERROR;
> +                               else if (status == -ENOMEM)
> +                                       ret = EFI_OUT_OF_RESOURCES;
> +                               else if (status == -ERANGE || status == -EINVAL)
> +                                       ret = EFI_INVALID_PARAMETER;
> +                               goto out;
> +                       }
> +                       log_debug("Cleared out accepted bit for Image %pUl\n", &image_type_id);
> +               }
> +
>         }
>
>  out:
> @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
>                 u64 scatter_gather_list)
>  {
>         struct efi_capsule_header *capsule;
> +       efi_guid_t *image_guid;
> +       u32 active_idx;
> +       int status;
>         unsigned int i;
>         efi_status_t ret;
>
> @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
>                 goto out;
>         }
>
> +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +               /* Obtain the update_index from the platform */
> +               status = fwu_plat_get_update_index(&update_index);
> +               if (status < 0) {
> +                       log_err("Failed to get the FWU update_index value\n");
> +                       ret = EFI_DEVICE_ERROR;
> +                       goto out;
> +               }
> +       }
> +
>         ret = EFI_SUCCESS;
>         for (i = 0, capsule = *capsule_header_array; i < capsule_count;
>              i++, capsule = *(++capsule_header_array)) {
> @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
>                           i, &capsule->capsule_guid);
>                 if (!guidcmp(&capsule->capsule_guid,
>                              &efi_guid_firmware_management_capsule_id)) {
> +                       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +                               if (!fwu_update_checks_pass()) {
> +                                       log_err("FWU checks failed. Cannot start update\n");
> +                                       ret = EFI_INVALID_PARAMETER;
> +                                       goto out;
> +                               }
> +                       }
> +
>                         ret  = efi_capsule_update_firmware(capsule);
> +                       capsule_update = true;
> +               } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +                       capsule_update = false;
> +                       if (!guidcmp(&capsule->capsule_guid,
> +                                    &fwu_guid_os_request_fw_revert)) {
> +                               /*
> +                                * One of the previously updated image has
> +                                * failed the OS acceptance test. OS has
> +                                * requested to revert back to the earlier
> +                                * boot index
> +                                */
> +                               status = fwu_revert_boot_index();
> +                               if (status < 0) {
> +                                       log_err("Failed to revert the FWU boot index\n");
> +                                       if (status == -ENODEV ||
> +                                           status == -ERANGE ||
> +                                           status == -EIO)
> +                                               ret = EFI_DEVICE_ERROR;
> +                                       else if (status == -EINVAL)
> +                                               ret = EFI_INVALID_PARAMETER;
> +                                       else if (status == -ENOMEM)
> +                                               ret = EFI_OUT_OF_RESOURCES;
> +                               } else {
> +                                       ret = EFI_SUCCESS;
> +                                       log_err("Reverted the FWU active_index to %u. Recommend rebooting the system\n",
> +                                               active_idx);
> +                               }
> +                       } else if (!guidcmp(&capsule->capsule_guid,
> +                                           &fwu_guid_os_request_fw_accept)) {
> +                               /*
> +                                * Image accepted by the OS. Set the acceptance
> +                                * status for the image.
> +                                */
> +                               image_guid = (void *)(char *)capsule +
> +                                       capsule->header_size;
> +                               status = fwu_accept_image(image_guid);
> +                               if (status < 0) {
> +                                       if (status == -ENODEV ||
> +                                           status == -ERANGE ||
> +                                           status == -EIO)
> +                                               ret = EFI_DEVICE_ERROR;
> +                                       else if (status == -EINVAL)
> +                                               ret = EFI_INVALID_PARAMETER;
> +                                       else if (status == -ENOMEM)
> +                                               ret = EFI_OUT_OF_RESOURCES;
> +                               } else {
> +                                       ret = EFI_SUCCESS;
> +                               }
> +                       }
>                 } else {
>                         log_err("Unsupported capsule type: %pUl\n",
>                                 &capsule->capsule_guid);
> @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
>                         goto out;
>         }
>
> +       /*
> +        * Update the FWU metadata once all the capsules have
> +        * been updated. This is done only for the Runtime
> +        * capsule update service.
> +        * The update_index value now gets written to the
> +        * active_index and the update_index value also
> +        * gets updated.
> +        * For the capsule-on-disk feature, the updation
> +        * of the FWU metadata happens in efi_launch_capsules
> +        */
> +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> +           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> +               status = fwu_update_active_index(update_index);
> +               if (status < 0) {
> +                       log_err("Failed to update FWU metadata index values\n");
> +                       if (status == -EINVAL || status == -ERANGE)
> +                               ret = EFI_INVALID_PARAMETER;
> +                       else if (status == -ENODEV || status == -EIO)
> +                               ret = EFI_DEVICE_ERROR;
> +                       else if (status == -ENOMEM)
> +                               ret = EFI_OUT_OF_RESOURCES;
> +               } else {
> +                       status = fwu_trial_state_ctr_start();
> +                       if (status < 0)
> +                               ret = EFI_DEVICE_ERROR;
> +                       else
> +                               ret = EFI_SUCCESS;
> +               }
> +       }
> +
>         if (IS_ENABLED(CONFIG_EFI_ESRT)) {
>                 /* Rebuild the ESRT to reflect any updated FW images. */
>                 ret = efi_esrt_populate();
> @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
>  {
>         struct efi_capsule_header *capsule = NULL;
>         u16 **files;
> +       int status;
>         unsigned int nfiles, index, i;
>         efi_status_t ret;
> +       bool update_status = true;
>
>         if (check_run_capsules() != EFI_SUCCESS)
>                 return EFI_SUCCESS;
> @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
>                 ret = efi_capsule_read_file(files[i], &capsule);
>                 if (ret == EFI_SUCCESS) {
>                         ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> -                       if (ret != EFI_SUCCESS)
> +                       if (ret != EFI_SUCCESS) {
>                                 log_err("Applying capsule %ls failed\n",
>                                         files[i]);
> +                               update_status = false;
> +                       }
>
>                         /* create CapsuleXXXX */
>                         set_capsule_result(index, capsule, ret);
> @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
>                         free(capsule);
>                 } else {
>                         log_err("Reading capsule %ls failed\n", files[i]);
> +                       update_status = false;
>                 }
>                 /* delete a capsule either in case of success or failure */
>                 ret = efi_capsule_delete_file(files[i]);
> @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
>                         log_err("Deleting capsule %ls failed\n",
>                                 files[i]);
>         }
> +
>         efi_capsule_scan_done();
> +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +               if (update_status == true && capsule_update == true) {
> +                       /*
> +                        * All the capsules have been updated successfully,
> +                        * update the FWU metadata.
> +                        */
> +                       log_debug("Update Complete. Now updating active_index to %u\n",
> +                                 update_index);
> +                       status = fwu_update_active_index(update_index);
> +                       if (status < 0) {
> +                               log_err("Failed to update FWU metadata index values\n");
> +                               if (status == -EINVAL || status == -ERANGE)
> +                                       ret = EFI_INVALID_PARAMETER;
> +                               else if (status == -ENODEV || status == -EIO)
> +                                       ret = EFI_DEVICE_ERROR;
> +                               else if (status == -ENOMEM)
> +                                       ret = EFI_OUT_OF_RESOURCES;
> +                       } else {
> +                               log_debug("Successfully updated the active_index\n");
> +                               status = fwu_trial_state_ctr_start();
> +                               if (status < 0)
> +                                       ret = EFI_DEVICE_ERROR;
> +                               else
> +                                       ret = EFI_SUCCESS;
> +                       }
> +               } else if (capsule_update == true && update_status == false) {
> +                       log_err("All capsules were not updated. Not updating FWU metadata\n");
> +               }
> +       }
>
>         for (i = 0; i < nfiles; i++)
>                 free(files[i]);
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 1aba71cd96..6601176a2d 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
>                 goto out;
>
>         /* Execute capsules after reboot */
> -       if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> +       if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> +           IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>             !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
>                 ret = efi_launch_capsules();
>  out:
> diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> new file mode 100644
> index 0000000000..73099a30cb
> --- /dev/null
> +++ b/lib/fwu_updates/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2021, Linaro Limited
> +#
> +
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> +
> +ifdef CONFIG_EFI_PARTITION
> +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> +endif
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index e964f9b0b1..bb0e1961be 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -107,6 +107,33 @@ u8 fwu_update_checks_pass(void)
>         return !trial_state && boottime_check;
>  }
>
> +int fwu_trial_state_ctr_start(void)
> +{
> +       int ret;
> +       u32 var_attributes;
> +       efi_status_t status;
> +       efi_uintn_t var_size;
> +       u16 trial_state_ctr;
> +
> +       var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> +       var_attributes = EFI_VARIABLE_NON_VOLATILE |
> +               EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +               EFI_VARIABLE_RUNTIME_ACCESS;
> +
> +       trial_state_ctr = ret = 0;
> +       status = efi_set_variable_int(L"TrialStateCtr",
> +                                     &efi_global_variable_guid,
> +                                     var_attributes,
> +                                     var_size,
> +                                     &trial_state_ctr, false);
> +       if (status != EFI_SUCCESS) {
> +               log_err("Unable to increment TrialStateCtr variable\n");
> +               ret = -1;
> +       }
> +
> +       return ret;
> +}
> +
>  int fwu_boottime_checks(void)
>  {
>         int ret;
> --
> 2.17.1
>

hi Masami,

On Thu, 23 Dec 2021 at 06:23, Masami Hiramatsu <masami.hiramatsu@linaro.org>
wrote:

> Hi Sughosh,
>
> Can you move the FWU related configs to lib/fwu_updates/Kconfig ?
> FWU multi bank update is an independent feature, thus I think it is
> better to have its own Kconfig file and the lib/Kconfig only includes
> it.
> (I did it on my development series)
>

Okay, I have moved the FWU config options in a Kconfig file under
lib/fwu_updates/. This gets sourced from lib/Kconfig.

-sughosh


>
> Thank you,
>
> 2021年12月19日(日) 16:07 Sughosh Ganu <sughosh.ganu@linaro.org>:
> >
> > The FWU Multi Bank Update feature supports updation of firmware images
> > to one of multiple sets(also called banks) of images. The firmware
> > images are clubbed together in banks, with the system booting images
> > from the active bank. Information on the images such as which bank
> > they belong to is stored as part of the metadata structure, which is
> > stored on the same storage media as the firmware images on a dedicated
> > partition.
> >
> > At the time of update, the metadata is read to identify the bank to
> > which the images need to be flashed(update bank). On a successful
> > update, the metadata is modified to set the updated bank as active
> > bank to subsequently boot from.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V1:
> > * Call function fwu_update_checks_pass to check if the
> >   update can be initiated
> > * Do not allow firmware update from efi_init_obj_list as the
> >   fwu boot-time checks need to be run
> >
> >  include/fwu.h                |  18 +++-
> >  lib/Kconfig                  |  32 ++++++
> >  lib/Makefile                 |   1 +
> >  lib/efi_loader/efi_capsule.c | 198 ++++++++++++++++++++++++++++++++++-
> >  lib/efi_loader/efi_setup.c   |   3 +-
> >  lib/fwu_updates/Makefile     |  11 ++
> >  lib/fwu_updates/fwu.c        |  27 +++++
> >  7 files changed, 284 insertions(+), 6 deletions(-)
> >  create mode 100644 lib/fwu_updates/Makefile
> >
> > diff --git a/include/fwu.h b/include/fwu.h
> > index 2d2e674d6a..bf50fe9277 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -10,14 +10,28 @@
> >
> >  #include <linux/types.h>
> >
> > -#define FWU_MDATA_VERSION      0x1
> > +#define FWU_MDATA_GUID \
> > +       EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > +                0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> > +
> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> > +       EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> > +                0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> > +
> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> > +       EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> > +                0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> >
> >  #define FWU_MDATA_GUID \
> >         EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >                  0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >
> > -int fwu_boottime_checks(void);
> > +#define FWU_MDATA_VERSION      0x1
> > +#define FWU_IMAGE_ACCEPTED     0x1
> > +
> >  u8 fwu_update_checks_pass(void);
> > +int fwu_boottime_checks(void);
> > +int fwu_trial_state_ctr_start(void);
> >
> >  int fwu_get_active_index(u32 *active_idx);
> >  int fwu_update_active_index(u32 active_idx);
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 807a4c6ade..7cb306317c 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> >           When there are multiple device tree nodes with same name,
> >            enable this config option to distinguish them using
> >           phandles in fdtdec_get_alias_seq() function.
> > +
> > +config FWU_MULTI_BANK_UPDATE
> > +       bool "Enable FWU Multi Bank Update Feature"
> > +       depends on EFI_HAVE_CAPSULE_SUPPORT
> > +       select PARTITION_TYPE_GUID
> > +       select EFI_SETUP_EARLY
> > +       help
> > +         Feature for updating firmware images on platforms having
> > +         multiple banks(copies) of the firmware images. One of the
> > +         bank is selected for updating all the firmware components
> > +
> > +config FWU_NUM_BANKS
> > +       int "Number of Banks defined by the platform"
> > +       depends on FWU_MULTI_BANK_UPDATE
> > +       help
> > +         Define the number of banks of firmware images on a platform
> > +
> > +config FWU_NUM_IMAGES_PER_BANK
> > +       int "Number of firmware images per bank"
> > +       depends on FWU_MULTI_BANK_UPDATE
> > +       help
> > +         Define the number of firmware images per bank. This value
> > +         should be the same for all the banks.
> > +
> > +config FWU_TRIAL_STATE_CNT
> > +       int "Number of times system boots in Trial State"
> > +       depends on FWU_MULTI_BANK_UPDATE
> > +       default 3
> > +       help
> > +         With FWU Multi Bank Update feature enabled, number of times
> > +         the platform is allowed to boot in Trial State after an
> > +         update.
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 5ddbc77ed6..bc5c1e22fc 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_EFI) += efi/
> >  obj-$(CONFIG_EFI_LOADER) += efi_driver/
> >  obj-$(CONFIG_EFI_LOADER) += efi_loader/
> >  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/
> >  obj-$(CONFIG_LZMA) += lzma/
> >  obj-$(CONFIG_BZIP2) += bzip2/
> >  obj-$(CONFIG_TIZEN) += tizen/
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 8301eed631..6dfe56bb0f 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -14,6 +14,7 @@
> >  #include <env.h>
> >  #include <fdtdec.h>
> >  #include <fs.h>
> > +#include <fwu.h>
> >  #include <malloc.h>
> >  #include <mapmem.h>
> >  #include <sort.h>
> > @@ -30,6 +31,13 @@ static const efi_guid_t
> efi_guid_firmware_management_capsule_id =
> >                 EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >  const efi_guid_t efi_guid_firmware_management_protocol =
> >                 EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> > +const efi_guid_t fwu_guid_os_request_fw_revert =
> > +               FWU_OS_REQUEST_FW_REVERT_GUID;
> > +const efi_guid_t fwu_guid_os_request_fw_accept =
> > +               FWU_OS_REQUEST_FW_ACCEPT_GUID;
> > +
> > +__maybe_unused static u32 update_index;
> > +__maybe_unused static bool capsule_update;
> >
> >  #ifdef CONFIG_EFI_CAPSULE_ON_DISK
> >  /* for file system access */
> > @@ -403,10 +411,13 @@ static efi_status_t efi_capsule_update_firmware(
> >         void *image_binary, *vendor_code;
> >         efi_handle_t *handles;
> >         efi_uintn_t no_handles;
> > -       int item;
> > +       int item, alt_no;
> >         struct efi_firmware_management_protocol *fmp;
> >         u16 *abort_reason;
> > +       efi_guid_t image_type_id;
> >         efi_status_t ret = EFI_SUCCESS;
> > +       int status;
> > +       u8 image_index;
> >
> >         /* sanity check */
> >         if (capsule_data->header_size < sizeof(*capsule) ||
> > @@ -481,8 +492,36 @@ static efi_status_t efi_capsule_update_firmware(
> >                                 goto out;
> >                 }
> >
> > +               if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                       /*
> > +                        * Based on the value of update_image_type_id,
> > +                        * derive the alt number value. This will be
> > +                        * passed as update_image_index to the
> > +                        * set_image function.
> > +                        */
> > +                       image_type_id = image->update_image_type_id;
> > +                       status = fwu_get_image_alt_num(image_type_id,
> > +                                                      update_index,
> > +                                                      &alt_no);
> > +                       if (status < 0) {
> > +                               log_err("Unable to get the alt no for
> the image type %pUl\n",
> > +                                       &image_type_id);
> > +                               if (status == -ENODEV || status == -EIO)
> > +                                       ret = EFI_DEVICE_ERROR;
> > +                               else if (status == -ENOMEM)
> > +                                       ret = EFI_OUT_OF_RESOURCES;
> > +                               else if (status == -ERANGE || status ==
> -EINVAL)
> > +                                       ret = EFI_INVALID_PARAMETER;
> > +                               goto out;
> > +                       }
> > +                       log_debug("alt_no %u for Image Type Id %pUl\n",
> > +                                 alt_no, &image_type_id);
> > +                       image_index = alt_no + 1;
> > +               } else {
> > +                       image_index = image->update_image_index;
> > +               }
> >                 abort_reason = NULL;
> > -               ret = EFI_CALL(fmp->set_image(fmp,
> image->update_image_index,
> > +               ret = EFI_CALL(fmp->set_image(fmp, image_index,
> >                                               image_binary,
> >                                               image_binary_size,
> >                                               vendor_code, NULL,
> > @@ -493,6 +532,24 @@ static efi_status_t efi_capsule_update_firmware(
> >                         efi_free_pool(abort_reason);
> >                         goto out;
> >                 }
> > +
> > +               if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                       status = fwu_clear_accept_image(&image_type_id,
> > +                                                       update_index);
> > +                       if (status < 0) {
> > +                               log_err("Unable to clear the accept bit
> for the image %pUl\n",
> > +                                       &image_type_id);
> > +                               if (status == -ENODEV || status == -EIO)
> > +                                       ret = EFI_DEVICE_ERROR;
> > +                               else if (status == -ENOMEM)
> > +                                       ret = EFI_OUT_OF_RESOURCES;
> > +                               else if (status == -ERANGE || status ==
> -EINVAL)
> > +                                       ret = EFI_INVALID_PARAMETER;
> > +                               goto out;
> > +                       }
> > +                       log_debug("Cleared out accepted bit for Image
> %pUl\n", &image_type_id);
> > +               }
> > +
> >         }
> >
> >  out:
> > @@ -527,6 +584,9 @@ efi_status_t EFIAPI efi_update_capsule(
> >                 u64 scatter_gather_list)
> >  {
> >         struct efi_capsule_header *capsule;
> > +       efi_guid_t *image_guid;
> > +       u32 active_idx;
> > +       int status;
> >         unsigned int i;
> >         efi_status_t ret;
> >
> > @@ -538,6 +598,16 @@ efi_status_t EFIAPI efi_update_capsule(
> >                 goto out;
> >         }
> >
> > +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +               /* Obtain the update_index from the platform */
> > +               status = fwu_plat_get_update_index(&update_index);
> > +               if (status < 0) {
> > +                       log_err("Failed to get the FWU update_index
> value\n");
> > +                       ret = EFI_DEVICE_ERROR;
> > +                       goto out;
> > +               }
> > +       }
> > +
> >         ret = EFI_SUCCESS;
> >         for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> >              i++, capsule = *(++capsule_header_array)) {
> > @@ -552,7 +622,64 @@ efi_status_t EFIAPI efi_update_capsule(
> >                           i, &capsule->capsule_guid);
> >                 if (!guidcmp(&capsule->capsule_guid,
> >                              &efi_guid_firmware_management_capsule_id)) {
> > +                       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                               if (!fwu_update_checks_pass()) {
> > +                                       log_err("FWU checks failed.
> Cannot start update\n");
> > +                                       ret = EFI_INVALID_PARAMETER;
> > +                                       goto out;
> > +                               }
> > +                       }
> > +
> >                         ret  = efi_capsule_update_firmware(capsule);
> > +                       capsule_update = true;
> > +               } else if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +                       capsule_update = false;
> > +                       if (!guidcmp(&capsule->capsule_guid,
> > +                                    &fwu_guid_os_request_fw_revert)) {
> > +                               /*
> > +                                * One of the previously updated image
> has
> > +                                * failed the OS acceptance test. OS has
> > +                                * requested to revert back to the
> earlier
> > +                                * boot index
> > +                                */
> > +                               status = fwu_revert_boot_index();
> > +                               if (status < 0) {
> > +                                       log_err("Failed to revert the
> FWU boot index\n");
> > +                                       if (status == -ENODEV ||
> > +                                           status == -ERANGE ||
> > +                                           status == -EIO)
> > +                                               ret = EFI_DEVICE_ERROR;
> > +                                       else if (status == -EINVAL)
> > +                                               ret =
> EFI_INVALID_PARAMETER;
> > +                                       else if (status == -ENOMEM)
> > +                                               ret =
> EFI_OUT_OF_RESOURCES;
> > +                               } else {
> > +                                       ret = EFI_SUCCESS;
> > +                                       log_err("Reverted the FWU
> active_index to %u. Recommend rebooting the system\n",
> > +                                               active_idx);
> > +                               }
> > +                       } else if (!guidcmp(&capsule->capsule_guid,
> > +
>  &fwu_guid_os_request_fw_accept)) {
> > +                               /*
> > +                                * Image accepted by the OS. Set the
> acceptance
> > +                                * status for the image.
> > +                                */
> > +                               image_guid = (void *)(char *)capsule +
> > +                                       capsule->header_size;
> > +                               status = fwu_accept_image(image_guid);
> > +                               if (status < 0) {
> > +                                       if (status == -ENODEV ||
> > +                                           status == -ERANGE ||
> > +                                           status == -EIO)
> > +                                               ret = EFI_DEVICE_ERROR;
> > +                                       else if (status == -EINVAL)
> > +                                               ret =
> EFI_INVALID_PARAMETER;
> > +                                       else if (status == -ENOMEM)
> > +                                               ret =
> EFI_OUT_OF_RESOURCES;
> > +                               } else {
> > +                                       ret = EFI_SUCCESS;
> > +                               }
> > +                       }
> >                 } else {
> >                         log_err("Unsupported capsule type: %pUl\n",
> >                                 &capsule->capsule_guid);
> > @@ -563,6 +690,36 @@ efi_status_t EFIAPI efi_update_capsule(
> >                         goto out;
> >         }
> >
> > +       /*
> > +        * Update the FWU metadata once all the capsules have
> > +        * been updated. This is done only for the Runtime
> > +        * capsule update service.
> > +        * The update_index value now gets written to the
> > +        * active_index and the update_index value also
> > +        * gets updated.
> > +        * For the capsule-on-disk feature, the updation
> > +        * of the FWU metadata happens in efi_launch_capsules
> > +        */
> > +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > +           !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK)) {
> > +               status = fwu_update_active_index(update_index);
> > +               if (status < 0) {
> > +                       log_err("Failed to update FWU metadata index
> values\n");
> > +                       if (status == -EINVAL || status == -ERANGE)
> > +                               ret = EFI_INVALID_PARAMETER;
> > +                       else if (status == -ENODEV || status == -EIO)
> > +                               ret = EFI_DEVICE_ERROR;
> > +                       else if (status == -ENOMEM)
> > +                               ret = EFI_OUT_OF_RESOURCES;
> > +               } else {
> > +                       status = fwu_trial_state_ctr_start();
> > +                       if (status < 0)
> > +                               ret = EFI_DEVICE_ERROR;
> > +                       else
> > +                               ret = EFI_SUCCESS;
> > +               }
> > +       }
> > +
> >         if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> >                 /* Rebuild the ESRT to reflect any updated FW images. */
> >                 ret = efi_esrt_populate();
> > @@ -1090,8 +1247,10 @@ efi_status_t efi_launch_capsules(void)
> >  {
> >         struct efi_capsule_header *capsule = NULL;
> >         u16 **files;
> > +       int status;
> >         unsigned int nfiles, index, i;
> >         efi_status_t ret;
> > +       bool update_status = true;
> >
> >         if (check_run_capsules() != EFI_SUCCESS)
> >                 return EFI_SUCCESS;
> > @@ -1119,9 +1278,11 @@ efi_status_t efi_launch_capsules(void)
> >                 ret = efi_capsule_read_file(files[i], &capsule);
> >                 if (ret == EFI_SUCCESS) {
> >                         ret = EFI_CALL(efi_update_capsule(&capsule, 1,
> 0));
> > -                       if (ret != EFI_SUCCESS)
> > +                       if (ret != EFI_SUCCESS) {
> >                                 log_err("Applying capsule %ls failed\n",
> >                                         files[i]);
> > +                               update_status = false;
> > +                       }
> >
> >                         /* create CapsuleXXXX */
> >                         set_capsule_result(index, capsule, ret);
> > @@ -1129,6 +1290,7 @@ efi_status_t efi_launch_capsules(void)
> >                         free(capsule);
> >                 } else {
> >                         log_err("Reading capsule %ls failed\n",
> files[i]);
> > +                       update_status = false;
> >                 }
> >                 /* delete a capsule either in case of success or failure
> */
> >                 ret = efi_capsule_delete_file(files[i]);
> > @@ -1136,7 +1298,37 @@ efi_status_t efi_launch_capsules(void)
> >                         log_err("Deleting capsule %ls failed\n",
> >                                 files[i]);
> >         }
> > +
> >         efi_capsule_scan_done();
> > +       if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> > +               if (update_status == true && capsule_update == true) {
> > +                       /*
> > +                        * All the capsules have been updated
> successfully,
> > +                        * update the FWU metadata.
> > +                        */
> > +                       log_debug("Update Complete. Now updating
> active_index to %u\n",
> > +                                 update_index);
> > +                       status = fwu_update_active_index(update_index);
> > +                       if (status < 0) {
> > +                               log_err("Failed to update FWU metadata
> index values\n");
> > +                               if (status == -EINVAL || status ==
> -ERANGE)
> > +                                       ret = EFI_INVALID_PARAMETER;
> > +                               else if (status == -ENODEV || status ==
> -EIO)
> > +                                       ret = EFI_DEVICE_ERROR;
> > +                               else if (status == -ENOMEM)
> > +                                       ret = EFI_OUT_OF_RESOURCES;
> > +                       } else {
> > +                               log_debug("Successfully updated the
> active_index\n");
> > +                               status = fwu_trial_state_ctr_start();
> > +                               if (status < 0)
> > +                                       ret = EFI_DEVICE_ERROR;
> > +                               else
> > +                                       ret = EFI_SUCCESS;
> > +                       }
> > +               } else if (capsule_update == true && update_status ==
> false) {
> > +                       log_err("All capsules were not updated. Not
> updating FWU metadata\n");
> > +               }
> > +       }
> >
> >         for (i = 0; i < nfiles; i++)
> >                 free(files[i]);
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 1aba71cd96..6601176a2d 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -298,7 +298,8 @@ efi_status_t efi_init_obj_list(void)
> >                 goto out;
> >
> >         /* Execute capsules after reboot */
> > -       if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> > +       if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE) &&
> > +           IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
> >             !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> >                 ret = efi_launch_capsules();
> >  out:
> > diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> > new file mode 100644
> > index 0000000000..73099a30cb
> > --- /dev/null
> > +++ b/lib/fwu_updates/Makefile
> > @@ -0,0 +1,11 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (c) 2021, Linaro Limited
> > +#
> > +
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata.o
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
> > +
> > +ifdef CONFIG_EFI_PARTITION
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_mdata_gpt_blk.o
> > +endif
> > diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> > index e964f9b0b1..bb0e1961be 100644
> > --- a/lib/fwu_updates/fwu.c
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -107,6 +107,33 @@ u8 fwu_update_checks_pass(void)
> >         return !trial_state && boottime_check;
> >  }
> >
> > +int fwu_trial_state_ctr_start(void)
> > +{
> > +       int ret;
> > +       u32 var_attributes;
> > +       efi_status_t status;
> > +       efi_uintn_t var_size;
> > +       u16 trial_state_ctr;
> > +
> > +       var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> > +       var_attributes = EFI_VARIABLE_NON_VOLATILE |
> > +               EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +               EFI_VARIABLE_RUNTIME_ACCESS;
> > +
> > +       trial_state_ctr = ret = 0;
> > +       status = efi_set_variable_int(L"TrialStateCtr",
> > +                                     &efi_global_variable_guid,
> > +                                     var_attributes,
> > +                                     var_size,
> > +                                     &trial_state_ctr, false);
> > +       if (status != EFI_SUCCESS) {
> > +               log_err("Unable to increment TrialStateCtr variable\n");
> > +               ret = -1;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  int fwu_boottime_checks(void)
> >  {
> >         int ret;
> > --
> > 2.17.1
> >
>
>
> --
> Masami Hiramatsu
>