Hi Ilias, On Thu, 28 Dec 2023 at 00:14, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > 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?
Sorry for the late reply. When the capsule update is reverted, the active bank is back to the previous active bank. So I think versions for all banks should be stored. Thanks, Masahisa Kojima > > > > > 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 > >