hi Ilias,
On Fri, 14 Oct 2022 at 12:12, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Thu, Oct 06, 2022 at 02:36:24PM +0530, Sughosh Ganu wrote:
The FWU Multi Bank Update feature supports updation of firmware images
s/updation/updating
Umm, is the current sentence wrong? The meaning of updation (as per google) is "the process of updating something."
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 V12: None
drivers/Kconfig | 2 + drivers/Makefile | 1 + include/fwu.h | 30 +++++ lib/Kconfig | 6 + lib/Makefile | 1 + lib/efi_loader/efi_capsule.c | 207 +++++++++++++++++++++++++++++++++- lib/efi_loader/efi_firmware.c | 14 +++ lib/fwu_updates/Kconfig | 33 ++++++ lib/fwu_updates/Makefile | 7 ++ lib/fwu_updates/fwu.c | 22 ++++ 10 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 lib/fwu_updates/Kconfig create mode 100644 lib/fwu_updates/Makefile
diff --git a/drivers/Kconfig b/drivers/Kconfig index 8b6fead351..75ac149d31 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -44,6 +44,8 @@ source "drivers/fuzz/Kconfig"
source "drivers/fpga/Kconfig"
+source "drivers/fwu-mdata/Kconfig"
source "drivers/gpio/Kconfig"
source "drivers/hwspinlock/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 9d9f69a3c9..253cbfb71d 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -85,6 +85,7 @@ obj-y += cache/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ obj-$(CONFIG_FASTBOOT) += fastboot/ +obj-$(CONFIG_FWU_MDATA) += fwu-mdata/ obj-y += misc/ obj-$(CONFIG_MMC) += mmc/ obj-$(CONFIG_NVME) += nvme/ diff --git a/include/fwu.h b/include/fwu.h index 2effa7da38..b5c59dc161 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -60,6 +60,7 @@ struct fwu_mdata_ops { };
@@ -388,6 +395,130 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+static __maybe_unused 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 __maybe_unused efi_status_t fwu_to_efi_error(int err) +{
efi_status_t ret;switch(err) {case 0:ret = EFI_SUCCESS;break;case -ENODEV:ENODEV should return EFI_INVALID_PARAMETER. The device was not found, EFI_DEVICE_ERROR usually means something bad happened to the device
Should this not be EFI_NOT_FOUND instead? I see it being used in places in the UEFI spec where a device is not found, for e.g. EFI_SD_MMC_PASS_THRU_PROTOCOL.BuildDevicePath.
-sughosh
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 __maybe_unused 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");elselog_err("Failed to revert the FWU boot index\n");} else {We should be explicit on the GUID checking here. IOW if someone hands you over and empty capsule with a guid !fwu_guid_os_request_fw_revert you'll accept the capsule
/** 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;+}
+static __maybe_unused void fwu_post_update_checks(
struct efi_capsule_header *capsule,bool *fw_accept_os, bool *capsule_update)+{
if (fwu_empty_capsule(capsule))*capsule_update = false;elseif (!*fw_accept_os)This line should fold to the upper one
*fw_accept_os =capsule->flags & FW_ACCEPT_OS ? true : false;+}
+static __maybe_unused efi_status_t fwu_post_update_process(bool fw_accept_os) +{
int status;u32 update_index;efi_status_t ret;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;}/** 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_set_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");[...]
Thanks /Ilias