[qcom-next] efi_loader: capsule: report spec-compliant ImageIndex for dynamic fw_images[]

arch/arm/mach-snapdragon/capsule_update.c

@@ -26,22 +26,29 @@
  * firmware components varies per board, and U-Boot itself may live on one of
  * several partitions depending on how it was booted.
  *
- * qcom_image_map[] maps each partition base name to a fixed capsule image_index
- * and a slot-independent firmware name. U-Boot's own image is image_index 1 in
- * the same table, split into boot-source-gated rows so the right partition is
- * picked for the running boot source: uefi/xbl (or the legacy "aboot" alias)
- * when flashed as XBL, boot when chainloaded from ABL.
+ * qcom_image_map[] maps each partition base name to a fixed image_index and a
+ * slot-independent firmware name. That index is used only to dedup map rows
+ * (one partition per index) and as the capsule authoring index; it is not the
+ * index reported at runtime. qcom_build_fw_images() assigns
+ * fw_images[].image_index sequentially (1..num_images) so GetImageInfo reports
+ * a valid descriptor array, and payloads are matched by image_type_id (GUID),
+ * not by index. U-Boot's own image is index 1 in the table, split into
+ * boot-source-gated rows so the right partition is picked for the running boot
+ * source: uefi/xbl (or the legacy "aboot" alias) when flashed as XBL, boot when
+ * chainloaded from ABL.
  */
 
 /**
  * struct qcom_image_map - maps a GPT partition to a capsule FMP image
  * @partition_base: base partition name, without the _a/_b slot suffix
  * @fw_name_base: slot-independent firmware name; the FMP GUID is derived
  *                from this, so it must stay constant across A/B toggles
- * @image_index: fixed image index for this component, frozen forever once
- *               a capsule ships referencing it. image_index 1 is reserved for
- *               U-Boot's own image (the three boot-source-gated rows below);
- *               exactly one of those is ever selected per boot.
+ * @image_index: fixed index for this component, used to dedup map rows in
+ *               qcom_resolve_images() and as the capsule authoring index. Not
+ *               the runtime fw_images[].image_index, which is assigned
+ *               sequentially. Index 1 is reserved for U-Boot's own image (the
+ *               three boot-source-gated rows below); exactly one of those is
+ *               ever selected per boot.
  * @boot_source: if non-zero, this row is only eligible when qcom_boot_source
  *               matches. Gates U-Boot's own image: uefi/xbl need
  *               QCOM_BOOT_SOURCE_XBL, boot needs QCOM_BOOT_SOURCE_ANDROID.
@@ -109,10 +116,10 @@ struct qcom_image_map {
 	EFI_GUID(0x17911177, 0xc9e6, 0x4372, 0x93, 0x3c, 0x80, 0x4b, 0x67, 0x8e, 0x66, 0x6f)
 #define QCOM_MULTIIMGOEM_IMAGE_TYPE_GUID \
 	EFI_GUID(0xe126a436, 0x757e, 0x42d0, 0x8d, 0x19, 0x0f, 0x36, 0x2f, 0x7a, 0x62, 0xb8)
-#define QCOM_APDP_IMAGE_TYPE_GUID \
-	EFI_GUID(0xe6e98da2, 0xe22a, 0x4d12, 0xab, 0x33, 0x16, 0x9e, 0x7d, 0xea, 0xa5, 0x07)
 #define QCOM_RPM_IMAGE_TYPE_GUID \
 	EFI_GUID(0x098df793, 0xd712, 0x413d, 0x9d, 0x4e, 0x89, 0xd7, 0x11, 0x77, 0x22, 0x28)
+#define QCOM_MULTIIMGQTI_IMAGE_TYPE_GUID \
+	EFI_GUID(0x846c6f05, 0xeb46, 0x4c0a, 0xa1, 0xa3, 0x36, 0x48, 0xef, 0x3f, 0x9d, 0x0e)
 
 static const struct qcom_image_map qcom_image_map[] = {
 	/*
@@ -163,10 +170,10 @@ static const struct qcom_image_map qcom_image_map[] = {
 	  QCOM_IMAGEFV_IMAGE_TYPE_GUID },
 	{ "multiimgoem", u"QCOM-MULTIIMGOEM", 14, 0, NULL, false,
 	  QCOM_MULTIIMGOEM_IMAGE_TYPE_GUID },
-	{ "apdp",        u"QCOM-APDP",        15, 0, NULL, false,
-	  QCOM_APDP_IMAGE_TYPE_GUID },
 	{ "rpm",         u"QCOM-RPM",         16, 0, NULL, false,
 	  QCOM_RPM_IMAGE_TYPE_GUID },
+	{ "multiimgqti", u"QCOM-MULTIIMGQTI", 17, 0, NULL, false,
+	  QCOM_MULTIIMGQTI_IMAGE_TYPE_GUID },
 };
 
 /*
@@ -235,7 +242,9 @@ static bool is_sd(struct blk_desc *desc)
  *             split xbl/xbl_config onto one LUN, tz/hyp/aop onto another) group
  *             back into the correct per-device DFU string tokens
  * @fw_name: slot-independent FMP name (from the matched map row's fw_name_base)
- * @image_index: fw_images[] image_index (from the matched map row)
+ * @image_index: fixed index from the matched map row, used to dedup rows in
+ *               qcom_resolve_images(). Not the runtime fw_images[].image_index,
+ *               which is assigned sequentially.
  * @image_type_id: fixed FMP GUID (from the matched map row's image_type_id)
  */
 struct qcom_partition_info {
@@ -493,10 +502,16 @@ static u32 qcom_resolve_images(struct qcom_row_cand *cands,
  * @num_partitions: number of valid entries in @partitions
  *
  * One fw_images[] entry per partition, in the same order, so fw_images[] and
- * @partitions stay index-aligned. .image_type_id is set from the matched map
- * row's fixed A-slot GUID. Since fw_images[0] is then non-zero,
+ * @partitions stay index-aligned. .image_index is assigned sequentially
+ * (1..num_partitions) so GetImageInfo reports a valid descriptor array (UEFI
+ * 2.9A: 1 <= ImageIndex <= DescriptorCount); the fixed per-component index from
+ * qcom_image_map[] is kept only for dedup in qcom_resolve_images(), which keys
+ * off qcom_partition_info.image_index rather than this array. Payloads are
+ * matched to this array by image_type_id (GUID), so the sequential value need
+ * not match the capsule's UpdateImageIndex. .image_type_id is set from the
+ * matched map row's fixed A-slot GUID. fw_images[0] is then non-zero, so
  * efi_gen_capsule_guids() skips derivation for the whole table (it derives only
- * when entry 0 is still zero), so the hardcoded GUIDs stand.
+ * when entry 0 is still zero) and the hardcoded GUIDs stand.
  *
  * Return: number of images written (equal to @num_partitions)
  */
@@ -507,7 +522,7 @@ static u32 qcom_build_fw_images(const struct qcom_partition_info *partitions,
 
 	for (i = 0; i < num_partitions; i++) {
 		fw_images[i].fw_name = (u16 *)partitions[i].fw_name;
-		fw_images[i].image_index = partitions[i].image_index;
+		fw_images[i].image_index = i + 1;
 		guidcpy(&fw_images[i].image_type_id, &partitions[i].image_type_id);
 	}
 
@@ -532,10 +547,12 @@ static u32 qcom_build_fw_images(const struct qcom_partition_info *partitions,
  * drop every partition on a LUN other than the first.
  *
  * dfu_alt_add() assigns each token's dfu_alt_num by its position in the parsed
- * string, so fw_images[i].dfu_alt_num is recorded here as tokens are appended,
- * not derived from image_index (which isn't contiguous or string-ordered once
- * components are missing). Relies on qcom_build_fw_images() having already
- * filled fw_images[0..num_partitions) from this same @partitions array.
+ * string, so fw_images[i].dfu_alt_num is recorded here as tokens are appended.
+ * Since qcom_build_fw_images() now assigns image_index as i + 1, dfu_alt_num
+ * equals image_index - 1; recording it positionally keeps dfu_alt_num correct
+ * regardless of how image_index is chosen. Relies on qcom_build_fw_images()
+ * having already filled fw_images[0..num_partitions) from this same
+ * @partitions array.
  *
  * Return: true on success, false if @buf was too small
  */
@@ -689,11 +706,11 @@ void qcom_configure_capsule_updates(void)
  * @image_index: fw_images[].image_index to resolve
  *
  * Strong override of the __weak default in lib/efi_loader/efi_firmware.c.
- * Qualcomm's fw_images[] is built at runtime and its image_index values aren't
- * guaranteed contiguous (a board may lack some components), so dfu_alt_num
- * can't be derived positionally -- look up the value recorded by
- * qcom_build_dfu_string() for the matching image_index instead, mirroring the
- * scan efi_firmware_get_image_type_id() already does.
+ * With image_index assigned sequentially, the weak default's image_index - 1
+ * already matches the positional dfu_alt_num. This override keeps the lookup
+ * explicit so it stays correct if fw_images[] ever becomes non-positional:
+ * return the dfu_alt_num recorded by qcom_build_dfu_string() for the matching
+ * image_index, mirroring efi_firmware_get_image_type_id().
  *
  * Falls back to the weak default's image_index - 1 if not found, which
  * shouldn't happen since every image_index passed in comes from fw_images[].
@@ -712,3 +729,32 @@ u8 efi_firmware_get_dfu_alt_num(u8 image_index)
 
 	return image_index - 1;
 }
+
+/**
+ * efi_capsule_resolve_image_index() - resolve a capsule to its image_index
+ * @image_type: image type GUID from the capsule (UpdateImageTypeId)
+ * @capsule_index: image index from the capsule (UpdateImageIndex)
+ *
+ * Strong override of the __weak default in lib/efi_loader/efi_capsule.c.
+ * fw_images[].image_index is assigned sequentially in qcom_build_fw_images(),
+ * so it depends on which components a given board carries and cannot match the
+ * capsule's frozen UpdateImageIndex. Match the payload by image_type_id (GUID)
+ * instead and return that image's own image_index for efi_fmp_find() and
+ * set_image().
+ *
+ * Falls back to @capsule_index for an absent component, letting efi_fmp_find()
+ * fail cleanly.
+ *
+ * Return: the image_index to use for efi_fmp_find() and set_image()
+ */
+u8 efi_capsule_resolve_image_index(const efi_guid_t *image_type,
+				   u8 capsule_index)
+{
+	int i;
+
+	for (i = 0; i < update_info.num_images; i++)
+		if (!guidcmp(&update_info.images[i].image_type_id, image_type))
+			return update_info.images[i].image_index;
+
+	return capsule_index;
+}

configs/qcom_defconfig

@@ -10,7 +10,6 @@ CONFIG_SYS_LOAD_ADDR=0xA0000000
 # CONFIG_EFI_HAVE_RUNTIME_RESET is not set
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_IGNORE_OSINDICATIONS=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_VARIABLE_FILE_STORE=y
 CONFIG_EFI_RT_VOLATILE_STORE=y

doc/develop/uefi/uefi.rst

@@ -615,6 +615,19 @@ on the FWU Multi Bank Update feature, please refer to
 When using the FMP for FIT images, the image index value needs to be
 set to 1.
 
+Platforms with a dynamically discovered image set
+'''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Some platforms (for example Qualcomm boards) build the fw_images array at
+runtime from the partition table, so the set of updatable images is not known
+until boot. image_index is assigned sequentially (1..num_images) at init, so
+GetImageInfo and the ESRT keep 1 <= ImageIndex <= DescriptorCount, but the
+capsule's UpdateImageIndex can then no longer be trusted to select the right
+image. Such a platform overrides efi_capsule_resolve_image_index() to map the
+incoming payload to its own image_index by image_type_id (GUID). The weak
+default returns UpdateImageIndex unchanged, so platforms with a static 1:1
+fw_images[] are unaffected.
+
 Finally, the capsule update can be initiated by rebooting the board.
 
 An example of setting the values in the struct efi_fw_image and

include/efi_loader.h

@@ -1257,6 +1257,21 @@ efi_status_t efi_load_capsule_drivers(void);
  */
 u8 efi_firmware_get_dfu_alt_num(u8 image_index);
 
+/**
+ * efi_capsule_resolve_image_index() - map a capsule to its platform image index
+ * @image_type:		image type GUID from the capsule (UpdateImageTypeId)
+ * @capsule_index:	image index from the capsule (UpdateImageIndex)
+ *
+ * Return the image_index to use for efi_fmp_find() and set_image(). The weak
+ * default returns @capsule_index unchanged; a platform whose fw_images[] is
+ * built dynamically overrides this to resolve @image_type to its own
+ * image_index.
+ *
+ * Return:		image_index to use for efi_fmp_find() and set_image()
+ */
+u8 efi_capsule_resolve_image_index(const efi_guid_t *image_type,
+				   u8 capsule_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,

lib/efi_loader/efi_capsule.c

@@ -517,6 +517,13 @@ static __maybe_unused efi_status_t fwu_post_update_process(bool fw_accept_os)
 	return ret;
 }
 
+/* Weak default; see the prototype in include/efi_loader.h. */
+u8 __weak efi_capsule_resolve_image_index(const efi_guid_t *image_type,
+					  u8 capsule_index)
+{
+	return capsule_index;
+}
+
 /**
  * efi_capsule_update_firmware - update firmware from capsule
  * @capsule_data:	Capsule
@@ -543,6 +550,7 @@ static efi_status_t efi_capsule_update_firmware(
 	int status;
 	uint update_index;
 	bool fw_accept_os;
+	u8 fmp_index;
 
 	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
 		if (fwu_empty_capsule(capsule_data)) {
@@ -615,9 +623,14 @@ static efi_status_t efi_capsule_update_firmware(
 			goto out;
 		}
 
+		/* resolve the capsule to its platform image index */
+		fmp_index = efi_capsule_resolve_image_index(
+				&image->update_image_type_id,
+				image->update_image_index);
+
 		/* find a device for update firmware */
 		fmp = efi_fmp_find(&image->update_image_type_id,
-				   image->update_image_index,
+				   fmp_index,
 				   image->update_hardware_instance,
 				   handles, no_handles);
 		if (!fmp) {
@@ -650,7 +663,7 @@ static efi_status_t efi_capsule_update_firmware(
 		}
 
 		abort_reason = NULL;
-		ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index,
+		ret = EFI_CALL(fmp->set_image(fmp, fmp_index,
 					      image_binary,
 					      image_binary_size,
 					      vendor_code, NULL,