Re: [PATCH v9 10/15] FWU: Add support for the FWU Multi Bank Update feature

Hi Sughosh,

Overall, I think the code is covering all the cases for A/B updates. Irrelevant to this patchset, but in the future I think we shopuld look into having efi_launch_capsules() call efi_update_capsule() would be cleaner.

[...]

I think you can get rid all of these

+__maybe_unused static u32 update_index;

This can be calculated at runtime

+__maybe_unused static bool capsule_update;

Dittio, you just need a call to figure out if it's an empty capsule or not

+__maybe_unused static bool fw_accept_os;

Same for this one

+static bool image_index_check = true;

Can't this just be an ifdef in the if statement that uses it?

#ifdef CONFIG_EFI_CAPSULE_ON_DISK /* for file system access */ @@ -205,7 +217,8 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance, log_debug("+++ desc[%d] index: %d, name: %ls\n", j, desc->image_index, desc->image_id_name); if (!guidcmp(&desc->image_type_id, image_type) &&

• 	    (desc->image_index == image_index) &&
  

• 	    (!image_index_check ||
  

• 	     desc->image_index == image_index) &&
      (!instance ||
       !desc->hardware_instance ||
        desc->hardware_instance == instance))
  

@@ -388,6 +401,83 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */

+static bool fwu_empty_capsule(struct efi_capsule_header *capsule) +{

• return !guidcmp(&capsule->capsule_guid,

• 	&fwu_guid_os_request_fw_revert) ||
  

• !guidcmp(&capsule->capsule_guid,
  

• 	 &fwu_guid_os_request_fw_accept);
  

+}

+static efi_status_t fwu_to_efi_error(int err) +{

• efi_status_t ret;
• switch(err) {
• case 0:

• ret = EFI_SUCCESS;
  

• break;
  

• case -ENODEV:
• case -ERANGE:
• case -EIO:

• ret = EFI_DEVICE_ERROR;
  

• break;
  

• case -EINVAL:

• ret = EFI_INVALID_PARAMETER;
  

• break;
  

• default:

• ret = EFI_OUT_OF_RESOURCES;
  

• }
• return ret;

+}

+static efi_status_t fwu_empty_capsule_process(

• struct efi_capsule_header *capsule)

+{

• int status;
• u32 active_idx;
• efi_status_t ret;
• efi_guid_t *image_guid;
• 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();
  

• ret = fwu_to_efi_error(status);
  

• if (ret == EFI_SUCCESS)
  

• 	log_info("Reverted the FWU active_index. Recommend rebooting the system\n");
  

• else
  

• 	log_err("Failed to revert the FWU boot index\n");
  

• } else {

• /*
  

•  * Image accepted by the OS. Set the acceptance
  

•  * status for the image.
  

•  */
  

• image_guid = (void *)(char *)capsule +
  

• 	capsule->header_size;
  

• status = fwu_get_active_index(&active_idx);
  

• ret = fwu_to_efi_error(status);
  

• if (ret != EFI_SUCCESS) {
  

• 	log_err("Unable to get the active_index from the FWU metadata\n");
  

• 	return ret;
  

• }
  

• status = fwu_accept_image(image_guid, active_idx);
  

• ret = fwu_to_efi_error(status);
  

• if (ret != EFI_SUCCESS)
  

• 	log_err("Unable to set the Accept bit for the image %pUs\n",
  

• 		image_guid);
  

• }
• return ret;

+}

/**

• efi_capsule_update_firmware - update firmware from capsule

@@ -407,10 +497,42 @@ 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;

• if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {

• if (!fwu_empty_capsule(capsule_data) &&
  

•     !fwu_update_checks_pass()) {
  

• 	log_err("FWU checks failed. Cannot start update\n");
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  

• if (fwu_empty_capsule(capsule_data)) {
  

• 	capsule_update = false;
  

• 	return fwu_empty_capsule_process(capsule_data);
  

• } else {
  

• 	capsule_update = true;
  

• }
  

• /* 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");
  

• 	return EFI_DEVICE_ERROR;
  

• }
  

• fw_accept_os = capsule_data->flags & FW_ACCEPT_OS ? 0x1 : 0x0;
  

• /*
  

•  * For Multi Bank updates, the image index is determined at
  

•  * runtime based on the value of the update bank.
  

•  */
  

• image_index_check = false;
  

• }

  /* sanity check */ if (capsule_data->header_size < sizeof(*capsule) ||

@@ -485,8 +607,31 @@ 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);
  

• 	ret = fwu_to_efi_error(status);
  

• 	if (ret != EFI_SUCCESS) {
  

• 		log_err("Unable to get the alt no for the image type %pUs\n",
  

• 			&image_type_id);
  

• 		goto out;
  

• 	}
  

• 	log_debug("alt_no %u for Image Type Id %pUs\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,
  

@@ -497,6 +642,33 @@ static efi_status_t efi_capsule_update_firmware( efi_free_pool(abort_reason); goto out; }

• if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
  

• 	if (!fw_accept_os) {
  

• 		/*
  

• 		 * The OS will not be accepting the firmware
  

• 		 * images. Set the accept bit of all the
  

• 		 * images contained in this capsule.
  

• 		 */
  

• 		status = fwu_accept_image(&image_type_id,
  

• 					  update_index);
  

• 	} else {
  

• 		status = fwu_clear_accept_image(&image_type_id,
  

• 						update_index);
  

• 	}
  

• 	ret = fwu_to_efi_error(status);
  

• 	if (ret != EFI_SUCCESS) {
  

• 		log_err("Unable to %s the accept bit for the image %pUs\n",
  

• 			fw_accept_os ? "clear" : "set",
  

• 			&image_type_id);
  

• 		goto out;
  

• 	}
  

• 	log_debug("%s the accepted bit for Image %pUs\n",
  

• 		  fw_accept_os ? "Cleared" : "Set",
  

• 		  &image_type_id);
  

• }
  

• }

out: @@ -1102,8 +1274,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;

@@ -1131,12 +1305,14 @@ efi_status_t efi_launch_capsules(void) ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { ret = efi_capsule_update_firmware(capsule);

• 	if (ret != EFI_SUCCESS)
  

• 	if (ret != EFI_SUCCESS) {
  	log_err("Applying capsule %ls failed.\n",
  		files[i]);
  

• 	else
  

• 		update_status = false;
  

• 	} else {
  	log_info("Applying capsule %ls succeeded.\n",
  		 files[i]);
  

• 	}
  
  /* create CapsuleXXXX */
  set_capsule_result(index, capsule, ret);
  

@@ -1144,6 +1320,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]);

@@ -1151,7 +1328,33 @@ 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);
  

• 	ret = fwu_to_efi_error(status);
  

• 	if (ret != EFI_SUCCESS) {
  

• 		log_err("Failed to update FWU metadata index values\n");
  

• 	} else {
  

• 		log_debug("Successfully updated the active_index\n");
  

• 		ret = EFI_SUCCESS;
  

• 		if (fw_accept_os) {
  

• 			status = fwu_trial_state_ctr_start();
  

• 			if (status < 0)
  

• 				ret = EFI_DEVICE_ERROR;
  

• 		}
  

• 	}
  

• } 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 23dc7a0710..60012e873e 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -351,7 +351,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/Kconfig b/lib/fwu_updates/Kconfig new file mode 100644 index 0000000000..e33085f1d3 --- /dev/null +++ b/lib/fwu_updates/Kconfig @@ -0,0 +1,32 @@ +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
• select EVENT
• 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/fwu_updates/Makefile b/lib/fwu_updates/Makefile new file mode 100644 index 0000000000..1993088e5b --- /dev/null +++ b/lib/fwu_updates/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Copyright (c) 2022, Linaro Limited +#

+obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o +obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c index 27f48855af..de2412785d 100644 --- a/lib/fwu_updates/fwu.c +++ b/lib/fwu_updates/fwu.c @@ -513,7 +513,30 @@ u8 fwu_update_checks_pass(void) return !trial_state && boottime_check; }

+/**

• • fwu_trial_state_ctr_start() - Start the Trial State counter
• • Start the counter to identify the platform booting in the
• • Trial State. The counter is implemented as an EFI variable.
• • Return: 0 if OK, -ve on error
• */

+int fwu_trial_state_ctr_start(void) +{

• int ret;
• u16 trial_state_ctr;
• trial_state_ctr = 0;
• ret = trial_counter_update(&trial_state_ctr);
• if (ret)

• log_err("Unable to initialise TrialStateCtr\n");
  

• return ret;

+}

static int fwu_boottime_checks(void *ctx, struct event *event)

{ int ret; struct udevice *dev; -- 2.34.1

Thanks /Ilias