On Tue, 21 Mar 2023 at 15:42, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 3/20/23 06:54, Masahisa Kojima wrote: > > The FMP Payload Header which EDK2 capsule generation scripts > > insert contains lowest supported version. > > This commit reads the lowest supported version stored in the > > "FmpStateXXXX" EFI non-volatile variable, then check if the > > firmware version of ongoing capsule is equal or greater than > > the lowest supported version. > > > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > > --- > > No changes since v2 > > > > Changes in v2: > > - add error message when the firmware version is lower than > > lowest supported version > > > > lib/efi_loader/efi_firmware.c | 50 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > > index 289456ecbb..8d6e32006d 100644 > > --- a/lib/efi_loader/efi_firmware.c > > +++ b/lib/efi_loader/efi_firmware.c > > @@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void > > **p_image, > > *p_image_size = image_size; > > } > > > > +/** > > + * efi_firmware_get_lowest_supported_version - get the lowest supported > > version > > + * @image_index: image_index > > + * > > + * Get the lowest supported version from FmpStateXXXX variable. > > + * > > + * Return: lowest supported version, return 0 if reading > > FmpStateXXXX > > + * variable failed > > + */ > > +static > > +u32 efi_firmware_get_lowest_supported_version(u8 image_index) > > +{ > > + u16 varname[13]; /* u"FmpStateXXXX" */ > > + efi_status_t ret; > > + efi_uintn_t size; > > + efi_guid_t *image_type_id; > > + struct fmp_state var_state = { 0 }; > > + > > + image_type_id = efi_firmware_get_image_type_id(image_index); > > + if (!image_type_id) > > + return 0; > > + > > + efi_create_indexed_name(varname, sizeof(varname), "FmpState", > > + image_index); > > + size = sizeof(var_state); > > + ret = efi_get_variable_int(varname, image_type_id, NULL, &size, > > + &var_state, NULL); > > + if (ret != EFI_SUCCESS) > > + return 0; > > + > > + return var_state.lowest_supported_version; > > +} > > + > > /** > > * efi_firmware_verify_image - verify image > > * @p_image: Pointer to new image > > @@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void > > **p_image, > > * @image_index Image index > > * @state Pointer to fmp state > > * > > - * Verify the capsule file > > + * Verify the capsule authentication and check if the fw_version > > + * is equal or greater than the lowest supported version. > > * > > * Return: status code > > */ > > @@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void > > **p_image, > > struct fmp_state *state) > > { > > efi_status_t ret; > > + u32 lowest_supported_version; > > > > ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state); > > efi_firmware_parse_payload_header(p_image, p_image_size, state); > > > > + /* check lowest_supported_version if capsule authentication passes */ > > + if (ret == EFI_SUCCESS) { > > + lowest_supported_version = > > + > > efi_firmware_get_lowest_supported_version(image_index); > > + if (lowest_supported_version > state->fw_version) { > > + printf("fw_version(%u) is too low(expected >%u). > > Aborting update\n", > > + state->fw_version, lowest_supported_version); > > Please, use log_warning() or log_err() instead of printf(). > > The addressee is a user not a developer: > > "Firmware version %u too low. Expecting >= %u"
OK. > > We should have one central place where upon exit we write a message > indicating that either the capsule update was successful or failed. > > "Firmware updated to version %u" : "Firmware update failed" Yes, I will add the log output for the user. Thanks, Masahisa Kojima > > Best regards > > Heinrich > > > + state->last_attempt_status = > > + LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION; > > + ret = EFI_INVALID_PARAMETER; > > + } > > + } > > + > > return ret; > > } > > >