Re: [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Fri, May 19, 2023 at 07:32:09PM +0900, Masahisa Kojima wrote:
> Current FMP->GetImageInfo() always return 0 for the firmware
> version, user can not identify which firmware version is currently
> running through the EFI interface.
>
> This commit reads the "FmpStateXXXX" EFI variable, then fills the
> firmware version in FMP->GetImageInfo().
>
> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v6:
> - create function to fill the version information
>
>  lib/efi_loader/efi_firmware.c | 41 ++++++++++++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index fc085e3c08..64ceefa212 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -144,6 +144,39 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
>  	return EFI_EXIT(EFI_UNSUPPORTED);
>  }
>
> +/**
> + * efi_firmware_fill_version_info - fill the version information
> + * @image_info:		Image information
> + * @fw_array:		Pointer to size of new image
> + *
> + * Fill the version information into image_info strucrure.
> + *
> + */
> +static
> +void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_info,
> +				    struct efi_fw_image *fw_array)
> +{
> +	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;
> +
> +	image_info->version_name = NULL; /* not supported */
> +	image_info->lowest_supported_image_version = 0;
> +	image_info->last_attempt_version = 0;
> +	image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> +}
> +
>  /**
>   * efi_fill_image_desc_array - populate image descriptor array
>   * @image_info_size:		Size of @image_info
> @@ -193,11 +226,10 @@ static efi_status_t efi_fill_image_desc_array(
>  		image_info[i].image_index = fw_array[i].image_index;
>  		image_info[i].image_type_id = fw_array[i].image_type_id;
>  		image_info[i].image_id = fw_array[i].image_index;
> -
>  		image_info[i].image_id_name = fw_array[i].fw_name;
>
> -		image_info[i].version = 0; /* not supported */
> -		image_info[i].version_name = NULL; /* not supported */
> +		efi_firmware_fill_version_info(&image_info[i], &fw_array[i]);
> +
>  		image_info[i].size = 0;
>  		image_info[i].attributes_supported =
>  			IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
> @@ -210,9 +242,6 @@ static efi_status_t efi_fill_image_desc_array(
>  			image_info[0].attributes_setting |=
>  				IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
>
> -		image_info[i].lowest_supported_image_version = 0;
> -		image_info[i].last_attempt_version = 0;
> -		image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
>  		image_info[i].hardware_instance = 1;
>  		image_info[i].dependencies = NULL;
>  	}
> --
> 2.17.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>