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
> >
