Kojima-san
On Thu, Dec 21, 2023 at 06:52:58PM +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.
Why do we all of them? Can't we just always store/use the version of the active
bank?
>
> This modification keeps the backward compatibility with
> the existing versioning feature.
>
Thanks
/Ilias
> Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> ---
> lib/efi_loader/efi_firmware.c | 69 +++++++++++++++++++++++++++--------
> 1 file changed, 54 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 9b8630625f..5459f3d38d 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -207,18 +207,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,
> @@ -227,6 +219,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);
> }
>
> /**
> @@ -362,8 +379,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)
> @@ -372,19 +392,38 @@ 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;
> +
> + ret = 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
>
