Hi Akashi-san, On Thu, 2 Mar 2023 at 14:09, Takahiro Akashi <takahiro.aka...@linaro.org> wrote: > > On Wed, Mar 01, 2023 at 06:15:19PM +0900, Masahisa Kojima wrote: > > Firmware version management is not implemented in the current > > FMP protocol. > > EDK2 reference implementation capsule generation script inserts > > the FMP Payload Header right before the payload, it contains the > > firmware version and lowest supported version. > > > > This commit utilizes the FMP Payload Header, read the header and > > stores the firmware version, lowest supported version, > > last attempt version and last attempt status into "FmpStateXXXX" > > EFI non-volatile variable. XXXX indicates the image index, > > since FMP protocol handles multiple image indexes. > > > > This change is compatible with the existing FMP implementation. > > This change does not mandate the FMP Payload Header. > > If no FMP Payload Header is found in the capsule file, fw_version, > > lowest supported version, last attempt version and last attempt > > status is 0 and this is the same behavior as existing FMP > > implementation. > > > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > > --- > > Changes in v2: > > - modify indent > > > > lib/efi_loader/efi_firmware.c | 198 ++++++++++++++++++++++++++++++---- > > 1 file changed, 175 insertions(+), 23 deletions(-) > > > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > > index 93e2b01c07..d1afafb052 100644 > > --- a/lib/efi_loader/efi_firmware.c > > +++ b/lib/efi_loader/efi_firmware.c > > @@ -10,6 +10,7 @@ > > #include <charset.h> > > #include <dfu.h> > > #include <efi_loader.h> > > +#include <efi_variable.h> > > #include <fwu.h> > > #include <image.h> > > #include <signatures.h> > > @@ -18,6 +19,12 @@ > > > > #define FMP_PAYLOAD_HDR_SIGNATURE SIGNATURE_32('M', 'S', 'S', '1') > > > > +#define EFI_FMP_STATE_GUID \ > > + EFI_GUID(0x84bed885, 0x193a, 0x403f, 0xa2, 0x78, \ > > + 0xe8, 0x9e, 0x23, 0x8a, 0xd6, 0xe1) > > + > > +static const efi_guid_t efi_guid_fmp_state = EFI_FMP_STATE_GUID; > > + > > /** > > * struct fmp_payload_header - EDK2 header for the FMP payload > > * > > @@ -36,6 +43,24 @@ struct fmp_payload_header { > > u32 lowest_supported_version; > > }; > > > > +/** > > + * struct fmp_state - fmp firmware update state > > + * > > + * This structure describes the state of the firmware update > > + * through FMP protocol. > > + * > > + * @fw_version: Firmware versions used > > + * @lowest_supported_version: Lowest supported version > > + * @last_attempt_version: Last attempt version > > + * @last_attempt_status: Last attempt status > > + */ > > +struct fmp_state { > > + u32 fw_version; > > + u32 lowest_supported_version; > > + u32 last_attempt_version; > > + u32 last_attempt_status; > > +}; > > + > > __weak void set_dfu_alt_info(char *interface, char *devstr) > > { > > env_set("dfu_alt_info", update_info.dfu_string); > > @@ -182,6 +207,7 @@ static efi_status_t efi_fill_image_desc_array( > > * efi_firmware_capsule_authenticate - authenticate the capsule if enabled > > * @p_image: Pointer to new image > > * @p_image_size: Pointer to size of new image > > + * @state Pointer to fmp state > > * > > * Authenticate the capsule if authentication is enabled. > > * The image pointer and the image size are updated in case of success. > > @@ -190,12 +216,11 @@ static efi_status_t efi_fill_image_desc_array( > > */ > > static > > efi_status_t efi_firmware_capsule_authenticate(const void **p_image, > > - efi_uintn_t *p_image_size) > > + efi_uintn_t *p_image_size, > > + struct fmp_state *state) > > { > > const void *image = *p_image; > > efi_uintn_t image_size = *p_image_size; > > - u32 fmp_hdr_signature; > > - struct fmp_payload_header *header; > > void *capsule_payload; > > efi_status_t status; > > efi_uintn_t capsule_payload_size; > > @@ -209,8 +234,12 @@ efi_status_t efi_firmware_capsule_authenticate(const > > void **p_image, > > > > if (status == EFI_SECURITY_VIOLATION) { > > printf("Capsule authentication check failed. Aborting > > update\n"); > > + state->last_attempt_status = > > + LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR; > > return status; > > } else if (status != EFI_SUCCESS) { > > + state->last_attempt_status = > > + LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > > return status; > > } > > > > @@ -222,24 +251,124 @@ efi_status_t efi_firmware_capsule_authenticate(const > > void **p_image, > > debug("Updating capsule without authenticating.\n"); > > } > > > > - fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE; > > - header = (void *)image; > > + *p_image = image; > > + *p_image_size = image_size; > > + > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + * efi_firmware_set_fmp_state_var - set FmpStateXXXX variable > > + * @state: Pointer to fmp state > > + * @image_index: image index > > + * @updated: flag to indicate firmware update is successful > > + * > > + * Update the FmpStateXXXX variable with the firmware update state. > > + * > > + * Return: status code > > + */ > > +static > > +efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 > > image_index, > > + bool updated) > > +{ > > + 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", > > + image_index); > > + size = sizeof(var_state); > > + ret = efi_get_variable_int(varname, &efi_guid_fmp_state, NULL, &size, > > + &var_state, NULL); > > + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > > + return ret; > > + > > + /* > > + * When the capsule update is successful, FmpStateXXXX variable is set > > + * according to the fmp payload header information. If there is no > > fmp payload > > + * header in the capsule file, all values are set to 0. > > + * When the capsule update fails, only last attempt information of > > FmpStateXXXX > > + * variable is updated, fw_version and lowest_supported_version keep > > original > > + * value or 0(in case no FmpStateXXXX variable found). > > + */ > > + if (updated) { > > + var_state.fw_version = state->fw_version; > > + var_state.lowest_supported_version = > > state->lowest_supported_version; > > + var_state.last_attempt_version = state->last_attempt_version; > > + var_state.last_attempt_status = state->last_attempt_status; > > + } else { > > + var_state.last_attempt_version = state->last_attempt_version; > > + var_state.last_attempt_status = state->last_attempt_status; > > + } > > + > > + ret = efi_set_variable_int(varname, &efi_guid_fmp_state, > > + EFI_VARIABLE_NON_VOLATILE | > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS, > > + sizeof(var_state), &var_state, false); > > + > > + return ret; > > +} > > > > +/** > > + * efi_firmware_parse_payload_header - parse FMP payload header > > + * @p_image: Pointer to new image > > + * @p_image_size: Pointer to size of new image > > + * @state Pointer to fmp state > > + * > > + * Parse the FMP payload header and fill the fmp_state structure. > > + * If no FMP payload header is found, fmp_state structure is not updated. > > + * > > + */ > > +static > > +void efi_firmware_parse_payload_header(const void **p_image, > > + efi_uintn_t *p_image_size, > > + struct fmp_state *state) > > +{ > > + const void *image = *p_image; > > + efi_uintn_t image_size = *p_image_size; > > + const struct fmp_payload_header *header; > > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE; > > + > > + header = image; > > if (!memcmp(&header->signature, &fmp_hdr_signature, > > sizeof(fmp_hdr_signature))) { > > - /* > > - * When building the capsule with the scripts in > > - * edk2, a FMP header is inserted above the capsule > > - * payload. Compensate for this header to get the > > - * actual payload that is to be updated. > > - */ > > + /* FMP header is inserted above the capsule payload */ > > + state->fw_version = header->fw_version; > > + state->lowest_supported_version = > > header->lowest_supported_version; > > + state->last_attempt_version = header->fw_version; > > image += header->header_size; > > image_size -= header->header_size; > > } > > > > *p_image = image; > > *p_image_size = image_size; > > - return EFI_SUCCESS; > > +} > > + > > +/** > > + * efi_firmware_verify_image - verify image > > + * @p_image: Pointer to new image > > + * @p_image_size: Pointer to size of new image > > + * @image_index Image index > > + * @state Pointer to fmp state > > + * > > + * Verify the capsule file > > + * > > + * Return: status code > > + */ > > +static > > +efi_status_t efi_firmware_verify_image(const void **p_image, > > + efi_uintn_t *p_image_size, > > + u8 image_index, > > + struct fmp_state *state) > > +{ > > + efi_status_t ret; > > + > > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state); > > + efi_firmware_parse_payload_header(p_image, p_image_size, state); > > + > > + return ret; > > } > > > > /** > > @@ -330,7 +459,9 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > > efi_status_t (*progress)(efi_uintn_t completion), > > u16 **abort_reason) > > { > > + bool updated; > > efi_status_t status; > > + struct fmp_state state = { 0 }; > > > > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image, > > image_size, vendor_code, progress, abort_reason); > > @@ -338,14 +469,22 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > > if (!image || image_index != 1) > > return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > - status = efi_firmware_capsule_authenticate(&image, &image_size); > > + status = efi_firmware_verify_image(&image, &image_size, image_index, > > + &state); > > if (status != EFI_SUCCESS) > > - return EFI_EXIT(status); > > + goto err; > > > > - if (fit_update(image)) > > - return EFI_EXIT(EFI_DEVICE_ERROR); > > + if (fit_update(image)) { > > + status = EFI_DEVICE_ERROR; > > I think we need to add here: > state.last_attempt_status = > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
Yes, I will fix it. Thanks, Masahisa Kojima > > -Takahiro Akashi > > > + goto err; > > + } > > > > - return EFI_EXIT(EFI_SUCCESS); > > + state.last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > > +err: > > + updated = (status == EFI_SUCCESS) ? true : false; > > + efi_firmware_set_fmp_state_var(&state, image_index, updated); > > + > > + return EFI_EXIT(status); > > } > > > > const struct efi_firmware_management_protocol efi_fmp_fit = { > > @@ -391,7 +530,9 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > > u16 **abort_reason) > > { > > int ret; > > + bool updated; > > efi_status_t status; > > + struct fmp_state state = { 0 }; > > > > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image, > > image_size, vendor_code, progress, abort_reason); > > @@ -399,9 +540,10 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > > if (!image) > > return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > - status = efi_firmware_capsule_authenticate(&image, &image_size); > > + status = efi_firmware_verify_image(&image, &image_size, image_index, > > + &state); > > if (status != EFI_SUCCESS) > > - return EFI_EXIT(status); > > + goto err; > > > > if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) { > > /* > > @@ -410,16 +552,26 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > > */ > > ret = fwu_get_image_index(&image_index); > > if (ret) { > > + state.last_attempt_status = > > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > > log_debug("Unable to get FWU image_index\n"); > > - return EFI_EXIT(EFI_DEVICE_ERROR); > > + status = EFI_DEVICE_ERROR; > > + goto err; > > } > > } > > > > if (dfu_write_by_alt(image_index - 1, (void *)image, image_size, > > - NULL, NULL)) > > - return EFI_EXIT(EFI_DEVICE_ERROR); > > + NULL, NULL)) { > > + status = EFI_DEVICE_ERROR; > > + state.last_attempt_status = > > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > > + goto err; > > + } > > + > > + state.last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > > +err: > > + updated = (status == EFI_SUCCESS) ? true : false; > > + efi_firmware_set_fmp_state_var(&state, image_index, updated); > > > > - return EFI_EXIT(EFI_SUCCESS); > > + return EFI_EXIT(status); > > } > > > > const struct efi_firmware_management_protocol efi_fmp_raw = { > > -- > > 2.17.1 > >