Re: [PATCH 1/2] efi_loader: firmware: decouple dfu_alt_num from image_index

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

Hi Balaji,


On Thu Aug 13, 2026 at 8:53 AM EEST, Balaji Selvanathan wrote:
> RAW capsule updates assume dfu_alt_num is always image_index - 1, i.e.
> that fw_images[] is a positionally-ordered mirror of the DFU alt
> settings. That holds for every board that builds its fw_images[] table
> by hand, but a platform whose image list is discovered at runtime
> (varying per board, with gaps for missing components) can't guarantee
> image_index and dfu_alt_num stay in lockstep.
>
> Move the (image_index - 1) calculation into a __weak function that
> platforms can override, following the pattern already used for
> efi_firmware_get_image_type_id(). The default keeps the
> existing behaviour, so no other board needs any change.

*efi_firmware_get_image_type_id() was always static, which funtion did you mean?
fwu_plat_get_alt_num()?

>
> Signed-off-by: Balaji Selvanathan <balaji.selvanathan@oss.qualcomm.com>
> ---
>  include/efi_loader.h          | 17 +++++++++++++++++
>  lib/efi_loader/efi_firmware.c | 21 +++++++++++++++++++--
>  2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 3a4d502631c..6626674f738 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1187,11 +1187,15 @@ efi_status_t efi_capsule_authenticate(const void *capsule,
>   * @fw_name:		Name of the firmware image
>   * @image_index:	Image Index, same as value passed to SetImage FMP
>   *                      function
> + * @dfu_alt_num:	DFU alt setting number for this image. Only consulted
> + *                      by a platform's efi_firmware_get_dfu_alt_num()
> + *                      override
>   */
>  struct efi_fw_image {
>  	efi_guid_t image_type_id;
>  	u16 *fw_name;
>  	u8 image_index;
> +	u8 dfu_alt_num;

Why do we need the extra struct member? The code doesn't update it to store any updates values.
Can't we just use the runtime result every time?

>  };
>
>  /**
> @@ -1240,6 +1244,19 @@ efi_status_t efi_ecpt_register(void);
>  efi_status_t efi_esrt_populate(void);
>  efi_status_t efi_load_capsule_drivers(void);
>
> +/**
> + * efi_firmware_get_dfu_alt_num() - get the DFU alt setting number for an image
> + * @image_index:	image index
> + *
> + * Return the DFU alt setting number to use when writing the image
> + * identified by @image_index. Weak default derives it positionally as
> + * (image_index - 1); a platform whose fw_images[] is not laid out 1:1 with
> + * DFU alt numbers should override this function.
> + *
> + * Return:		DFU alt setting number
> + */
> +u8 efi_firmware_get_dfu_alt_num(u8 image_index);
> +
>  efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz);
>
>  efi_status_t efi_locate_handle_buffer_int(enum efi_locate_search_type search_type,
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index b41969c70fd..c7339412055 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -80,6 +80,22 @@ efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
>  	return NULL;
>  }
>
> +/**
> + * efi_firmware_get_dfu_alt_num - get the DFU alt setting number for an image
> + * @image_index:	image index
> + *
> + * Return the DFU alt setting number to use when writing the image
> + * identified by @image_index. The generic default derives it positionally
> + * from @image_index; a platform whose fw_images[] is not laid out 1:1 with
> + * DFU alt numbers should override this function.
> + *
> + * Return:		DFU alt setting number
> + */
> +u8 __weak efi_firmware_get_dfu_alt_num(u8 image_index)
> +{
> +	return image_index - 1;
> +}

This is one of the things you need to support swapping image indexes on the fly, but there's way
more. One of the compromises we had to make to plug in capsuile updates via DFU is that the image
index *must* match the dfu command array member. IOW if you define the array with this

{guid_a, "u-boot", 1}, {guid_b, "u-boot-env", 2}
the dfu_string *must* list u-boot first and u-boot-env second.

IIRC we already check for a mismatch of GUID/Index in FWU code, but in the normal code we only have
a check in the efi_fmp_find(). The way you are switching happens after the check so you might end up
updating the partition with the wrond kind of firmware.

[...]

Cheers
/Ilias