diff mbox series

[v3,2/2] efi_loader: support fmp versioning for multi bank update

Message ID 20240111053540.899997-3-masahisa.kojima@linaro.org
State Accepted
Commit b20c9c3d8681547f7c839bb4f6fcd2f31e2b8585
Headers show
Series fix FMP versioning for FWU multi bank update | expand

Commit Message

Masahisa Kojima Jan. 11, 2024, 5:35 a.m. UTC
This commit stores the firmware version into the array
of fmp_state structure to support the fmp versioning
for multi bank update. The index of the array is identified
by the bank index.

This modification keeps the backward compatibility with
the existing versioning feature.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
 lib/efi_loader/efi_firmware.c | 75 ++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 15 deletions(-)

Comments

Ilias Apalodimas Jan. 12, 2024, 9:17 a.m. UTC | #1
On Thu, Jan 11, 2024 at 02:35:40PM +0900, Masahisa Kojima wrote:
> This commit stores the firmware version into the array
> of fmp_state structure to support the fmp versioning
> for multi bank update. The index of the array is identified
> by the bank index.
>
> This modification keeps the backward compatibility with
> the existing versioning feature.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>  lib/efi_loader/efi_firmware.c | 75 ++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index e558336bc1..9fd13297a6 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -206,18 +206,10 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
>  {
>  	u16 varname[13]; /* u"FmpStateXXXX" */
>  	efi_status_t ret;
> -	efi_uintn_t size;
> -	struct fmp_state var_state = { 0 };
> -
> -	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> -				fw_array->image_index);
> -	size = sizeof(var_state);
> -	ret = efi_get_variable_int(varname, &fw_array->image_type_id,
> -				   NULL, &size, &var_state, NULL);
> -	if (ret == EFI_SUCCESS)
> -		image_info->version = var_state.fw_version;
> -	else
> -		image_info->version = 0;
> +	efi_uintn_t size, expected_size;
> +	uint num_banks = 1;
> +	uint active_index = 0;
> +	struct fmp_state *var_state;
>
>  	efi_firmware_get_lsv_from_dtb(fw_array->image_index,
>  				      &fw_array->image_type_id,
> @@ -226,6 +218,31 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
>  	image_info->version_name = NULL; /* not supported */
>  	image_info->last_attempt_version = 0;
>  	image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> +	image_info->version = 0;
> +
> +	/* get the fw_version */
> +	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> +				fw_array->image_index);
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +		ret = fwu_get_active_index(&active_index);
> +		if (ret)
> +			return;
> +
> +		num_banks = CONFIG_FWU_NUM_BANKS;
> +	}
> +
> +	size = num_banks * sizeof(*var_state);
> +	expected_size = size;
> +	var_state = calloc(1, size);
> +	if (!var_state)
> +		return;
> +
> +	ret = efi_get_variable_int(varname, &fw_array->image_type_id,
> +				   NULL, &size, var_state, NULL);
> +	if (ret == EFI_SUCCESS && expected_size == size)
> +		image_info->version = var_state[active_index].fw_version;
> +
> +	free(var_state);
>  }
>
>  /**
> @@ -361,8 +378,11 @@ efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_in
>  {
>  	u16 varname[13]; /* u"FmpStateXXXX" */
>  	efi_status_t ret;
> +	uint num_banks = 1;
> +	uint update_bank = 0;
> +	efi_uintn_t size;
>  	efi_guid_t *image_type_id;
> -	struct fmp_state var_state = { 0 };
> +	struct fmp_state *var_state;
>
>  	image_type_id = efi_firmware_get_image_type_id(image_index);
>  	if (!image_type_id)
> @@ -371,19 +391,44 @@ efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_in
>  	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
>  				image_index);
>
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> +		ret = fwu_plat_get_update_index(&update_bank);
> +		if (ret)
> +			return EFI_INVALID_PARAMETER;
> +
> +		num_banks = CONFIG_FWU_NUM_BANKS;
> +	}
> +
> +	size = num_banks * sizeof(*var_state);
> +	var_state = calloc(1, size);
> +	if (!var_state)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	/*
> +	 * GetVariable may fail, EFI_NOT_FOUND is returned if FmpState
> +	 * variable has not been set yet.
> +	 * Ignore the error here since the correct FmpState variable
> +	 * is set later.
> +	 */
> +	efi_get_variable_int(varname, image_type_id, NULL, &size, var_state,
> +			     NULL);
> +
>  	/*
>  	 * Only the fw_version is set here.
>  	 * lowest_supported_version in FmpState variable is ignored since
>  	 * it can be tampered if the file based EFI variable storage is used.
>  	 */
> -	var_state.fw_version = state->fw_version;
> +	var_state[update_bank].fw_version = state->fw_version;
>
> +	size = num_banks * sizeof(*var_state);
>  	ret = efi_set_variable_int(varname, image_type_id,
>  				   EFI_VARIABLE_READ_ONLY |
>  				   EFI_VARIABLE_NON_VOLATILE |
>  				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  				   EFI_VARIABLE_RUNTIME_ACCESS,
> -				   sizeof(var_state), &var_state, false);
> +				   size, var_state, false);
> +
> +	free(var_state);
>
>  	return ret;
>  }
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index e558336bc1..9fd13297a6 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -206,18 +206,10 @@  void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
 {
 	u16 varname[13]; /* u"FmpStateXXXX" */
 	efi_status_t ret;
-	efi_uintn_t size;
-	struct fmp_state var_state = { 0 };
-
-	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
-				fw_array->image_index);
-	size = sizeof(var_state);
-	ret = efi_get_variable_int(varname, &fw_array->image_type_id,
-				   NULL, &size, &var_state, NULL);
-	if (ret == EFI_SUCCESS)
-		image_info->version = var_state.fw_version;
-	else
-		image_info->version = 0;
+	efi_uintn_t size, expected_size;
+	uint num_banks = 1;
+	uint active_index = 0;
+	struct fmp_state *var_state;
 
 	efi_firmware_get_lsv_from_dtb(fw_array->image_index,
 				      &fw_array->image_type_id,
@@ -226,6 +218,31 @@  void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
 	image_info->version_name = NULL; /* not supported */
 	image_info->last_attempt_version = 0;
 	image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
+	image_info->version = 0;
+
+	/* get the fw_version */
+	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+				fw_array->image_index);
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
+		ret = fwu_get_active_index(&active_index);
+		if (ret)
+			return;
+
+		num_banks = CONFIG_FWU_NUM_BANKS;
+	}
+
+	size = num_banks * sizeof(*var_state);
+	expected_size = size;
+	var_state = calloc(1, size);
+	if (!var_state)
+		return;
+
+	ret = efi_get_variable_int(varname, &fw_array->image_type_id,
+				   NULL, &size, var_state, NULL);
+	if (ret == EFI_SUCCESS && expected_size == size)
+		image_info->version = var_state[active_index].fw_version;
+
+	free(var_state);
 }
 
 /**
@@ -361,8 +378,11 @@  efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_in
 {
 	u16 varname[13]; /* u"FmpStateXXXX" */
 	efi_status_t ret;
+	uint num_banks = 1;
+	uint update_bank = 0;
+	efi_uintn_t size;
 	efi_guid_t *image_type_id;
-	struct fmp_state var_state = { 0 };
+	struct fmp_state *var_state;
 
 	image_type_id = efi_firmware_get_image_type_id(image_index);
 	if (!image_type_id)
@@ -371,19 +391,44 @@  efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_in
 	efi_create_indexed_name(varname, sizeof(varname), "FmpState",
 				image_index);
 
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
+		ret = fwu_plat_get_update_index(&update_bank);
+		if (ret)
+			return EFI_INVALID_PARAMETER;
+
+		num_banks = CONFIG_FWU_NUM_BANKS;
+	}
+
+	size = num_banks * sizeof(*var_state);
+	var_state = calloc(1, size);
+	if (!var_state)
+		return EFI_OUT_OF_RESOURCES;
+
+	/*
+	 * GetVariable may fail, EFI_NOT_FOUND is returned if FmpState
+	 * variable has not been set yet.
+	 * Ignore the error here since the correct FmpState variable
+	 * is set later.
+	 */
+	efi_get_variable_int(varname, image_type_id, NULL, &size, var_state,
+			     NULL);
+
 	/*
 	 * Only the fw_version is set here.
 	 * lowest_supported_version in FmpState variable is ignored since
 	 * it can be tampered if the file based EFI variable storage is used.
 	 */
-	var_state.fw_version = state->fw_version;
+	var_state[update_bank].fw_version = state->fw_version;
 
+	size = num_banks * sizeof(*var_state);
 	ret = efi_set_variable_int(varname, image_type_id,
 				   EFI_VARIABLE_READ_ONLY |
 				   EFI_VARIABLE_NON_VOLATILE |
 				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
 				   EFI_VARIABLE_RUNTIME_ACCESS,
-				   sizeof(var_state), &var_state, false);
+				   size, var_state, false);
+
+	free(var_state);
 
 	return ret;
 }