Hi Sughosh, On Mon, Feb 07, 2022 at 11:49:55PM +0530, Sughosh Ganu wrote: > The FWU Multi Banks Update feature allows updating different types of > updatable firmware images on the platform. These image types are > identified using the ImageTypeId GUID value. Add support in the > GetImageInfo function of the FMP protocol to get the GUID values for > the individual images and populate these in the image descriptor for > the corresponding images.
After re-thinking of your approach here, I would have to say NAK. You use ImageTypeId to identify a particular firmware object. (By "object," I mean one of firmware instances represented by "dfu_alto_info". Please don't confuse it with the binary blob embedded in a capsule file.) But ImageTypeId is not for that purpose, at least, as my intention in initially implementing capsule framework and FMP drivers. 1) ImageTypeId is used to uniquely identify a corresponding FMP driver, either FIT FMP driver or Raw FMP driver. 2) Each firmware object handled by a given FMP driver can further be identified by ImageIndex. My implementation of efi_fmp_find() does (1) and Raw FMP driver does (2) in efi_firmware_raw_set_image() which takes "image_index" as a parameter. Using ImageTypeId as an identifier is simply wrong in my opinion and doesn't meet the UEFI specification. -Takahiro Akashi > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > --- > > Changes since V3: > * Define a weak function fill_image_type_guid_array for populating the > image descriptor array with u-boot's raw and fit image GUIDs > > include/efi_loader.h | 2 + > lib/efi_loader/efi_firmware.c | 71 +++++++++++++++++++++++++++++++---- > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index f4860e87fc..ae60de0be5 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -992,4 +992,6 @@ efi_status_t efi_esrt_populate(void); > efi_status_t efi_load_capsule_drivers(void); > > efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz); > +efi_status_t fill_image_type_guid_array(const efi_guid_t *default_guid, > + efi_guid_t **part_guid_arr); > #endif /* _EFI_LOADER_H */ > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index a1b88dbfc2..5642be9f9a 100644 > --- a/lib/efi_loader/efi_firmware.c > +++ b/lib/efi_loader/efi_firmware.c > @@ -96,6 +96,46 @@ efi_status_t EFIAPI > efi_firmware_set_package_info_unsupported( > return EFI_EXIT(EFI_UNSUPPORTED); > } > > +efi_status_t __weak fill_image_type_guid_array(const efi_guid_t *guid, > + efi_guid_t **part_guid_arr) > +{ > + int i; > + int dfu_num = 0; > + efi_guid_t *guid_arr; > + struct dfu_entity *dfu; > + efi_status_t ret = EFI_SUCCESS; > + > + dfu_init_env_entities(NULL, NULL); > + > + dfu_num = 0; > + list_for_each_entry(dfu, &dfu_list, list) { > + dfu_num++; > + } > + > + if (!dfu_num) { > + log_warning("Probably dfu_alt_info not defined\n"); > + ret = EFI_NOT_READY; > + goto out; > + } > + > + *part_guid_arr = malloc(sizeof(efi_guid_t) * dfu_num); > + if (!*part_guid_arr) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > + } > + > + guid_arr = *part_guid_arr; > + for (i = 0; i < dfu_num; i++) { > + guidcpy(guid_arr, guid); > + ++guid_arr; > + } > + > +out: > + dfu_free_entities(); > + > + return ret; > +} > + > /** > * efi_get_dfu_info - return information about the current firmware image > * @this: Protocol instance > @@ -104,9 +144,9 @@ efi_status_t EFIAPI > efi_firmware_set_package_info_unsupported( > * @descriptor_version: Pointer to version number > * @descriptor_count: Pointer to number of descriptors > * @descriptor_size: Pointer to descriptor size > - * package_version: Package version > - * package_version_name: Package version's name > - * image_type: Image type GUID > + * @package_version: Package version > + * @package_version_name: Package version's name > + * @guid_array: Image type GUID array > * > * Return information bout the current firmware image in @image_info. > * @image_info will consist of a number of descriptors. > @@ -122,7 +162,7 @@ static efi_status_t efi_get_dfu_info( > efi_uintn_t *descriptor_size, > u32 *package_version, > u16 **package_version_name, > - const efi_guid_t *image_type) > + const efi_guid_t *guid_array) > { > struct dfu_entity *dfu; > size_t names_len, total_size; > @@ -172,7 +212,7 @@ static efi_status_t efi_get_dfu_info( > next = name; > list_for_each_entry(dfu, &dfu_list, list) { > image_info[i].image_index = dfu->alt + 1; > - image_info[i].image_type_id = *image_type; > + image_info[i].image_type_id = guid_array[i]; > image_info[i].image_id = dfu->alt; > > /* copy the DFU entity name */ > @@ -250,6 +290,7 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( > u16 **package_version_name) > { > efi_status_t ret; > + efi_guid_t *part_guid_arr = NULL; > > EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, > image_info_size, image_info, > @@ -264,12 +305,19 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( > !descriptor_size || !package_version || !package_version_name)) > return EFI_EXIT(EFI_INVALID_PARAMETER); > > + ret = fill_image_type_guid_array(&efi_firmware_image_type_uboot_fit, > + &part_guid_arr); > + if (ret != EFI_SUCCESS) > + goto out; > + > ret = efi_get_dfu_info(image_info_size, image_info, > descriptor_version, descriptor_count, > descriptor_size, > package_version, package_version_name, > - &efi_firmware_image_type_uboot_fit); > + part_guid_arr); > > +out: > + free(part_guid_arr); > return EFI_EXIT(ret); > } > > @@ -359,6 +407,7 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info( > u16 **package_version_name) > { > efi_status_t ret = EFI_SUCCESS; > + efi_guid_t *part_guid_arr = NULL; > > EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, > image_info_size, image_info, > @@ -373,12 +422,20 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info( > !descriptor_size || !package_version || !package_version_name)) > return EFI_EXIT(EFI_INVALID_PARAMETER); > > + ret = fill_image_type_guid_array( > + &efi_firmware_image_type_uboot_raw, > + &part_guid_arr); > + if (ret != EFI_SUCCESS) > + goto out; > + > ret = efi_get_dfu_info(image_info_size, image_info, > descriptor_version, descriptor_count, > descriptor_size, > package_version, package_version_name, > - &efi_firmware_image_type_uboot_raw); > + part_guid_arr); > > +out: > + free(part_guid_arr); > return EFI_EXIT(ret); > } > > -- > 2.17.1 >