[PATCH v2 0/2] EFI Capsule update explicitly sets dfu_alt_info

[PATCH v2 0/2] EFI Capsule update explicitly sets dfu_alt_info

[Jonathan Humphreys]

For capsule update, explicitly set the dfu_alt_info environment variable
before the DFU operation, and then restore it to the original value.
Previously, the dfu_alt_info environment variable was set with the
set_dfu_alt_info() function.

The problem with setting the capsule update's dfu_alt_info setting in
set_dfu_alt_info() is that set_dfu_alt_info() lacks the context of what DFU
operation is being performed (eg, capsule update, DFU boot, listing the
alt_info, etc) so the capsule update setting was overwriting the setting
for other DFU operations.

Changes from v1:
- use log_err() instead of pr_err()
- create a local copy of the original dfu_alt_info environment variable to
  be used to later restore it, rather than just a pointer to the stored
  value, because changing its value to the EFI capsule update setting will
  cause the original string location to be freed.
- even in the case of a DFU operation error, restore the dfu_alt_info
  environment variable to its original value.
- return EFI_EXIT based error codes if setting environment variables fails
Link to v1: https://lore.kernel.org/r/20250203215351.2840144-1-j-humphreys@ti.com

Jonathan Humphreys (2):
  efi_firmware: set EFI capsule dfu_alt_info env explicitly
  board: remove capsule update support in set_dfu_alt_info()

 board/beagle/beagleboneai64/beagleboneai64.c  |  8 ----
 board/beagle/beagleplay/beagleplay.c          |  8 ----
 .../aml-a311d-cc/aml-a311d-cc.c               |  2 -
 .../aml-s805x-ac/aml-s805x-ac.c               |  2 -
 .../aml-s905d3-cc/aml-s905d3-cc.c             |  2 -
 board/phytec/common/k3/board.c                |  8 ----
 board/ti/am62px/evm.c                         |  8 ----
 board/ti/am62x/evm.c                          |  8 ----
 board/ti/am64x/evm.c                          |  8 ----
 board/ti/j721e/evm.c                          |  8 ----
 board/ti/j784s4/evm.c                         |  8 ----
 lib/efi_loader/Kconfig                        |  2 -
 lib/efi_loader/efi_firmware.c                 | 44 +++++++++++++++----
 13 files changed, 36 insertions(+), 80 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] efi_firmware: set EFI capsule dfu_alt_info env explicitly

[Jonathan Humphreys]

The current implementation of EFI capsule update uses set_dfu_alt_info() to
set the dfu_alt_info environment variable with the settings it requires.
However, set_dfu_alt_info() is doing this for all DFU operations, even
those unrelated to capsule update.

Thus other uses of DFU, such as DFU boot which sets its own value for the
dfu_alt_info environment variable, will have that setting overwritten with
the capsule update setting. Similarly, any user defined value for the
dfu_alt_info environment variable would get overwritten when any DFU
operation was performed, including simply performing a "dfu 0 list"
command.

The solution is stop using the set_dfu_alt_info() mechanism to set the
dfu_alt_info environment variable and instead explicitly set it to the
capsule update's setting just before performing the capsule update's DFU
operation, and then restore the environment variable back to its original
value.

This patch implements the explicit setting and restoring of the
dfu_alt_info environment variable as part of the EFI capsule update
operation.

The fix is fully implemented in a subsequent patch that removes the capsule
update dfu_alt_info support in set_dfu_alt_info().

Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
---
 lib/efi_loader/efi_firmware.c | 39 ++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 5a754c9cd03..1a1cf3b55e1 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -649,8 +649,10 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
 	efi_status_t (*progress)(efi_uintn_t completion),
 	u16 **abort_reason)
 {
+	int ret;
 	efi_status_t status;
 	struct fmp_state state = { 0 };
+	char *orig_dfu_env;
 
 	EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
 		  image_size, vendor_code, progress, abort_reason);
@@ -663,7 +665,22 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
 	if (status != EFI_SUCCESS)
 		return EFI_EXIT(status);
 
-	if (fit_update(image))
+	orig_dfu_env = strdup(env_get("dfu_alt_info"));
+	if (env_set("dfu_alt_info", update_info.dfu_string)) {
+		log_err("unable to set env variable \"dfu_alt_info\"!\n");
+		free(orig_dfu_env);
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+	}
+
+	ret = fit_update(image);
+
+	if (env_set("dfu_alt_info", orig_dfu_env)) {
+		log_err("unable to set env variable \"dfu_alt_info\"!\n");
+		ret = 1;
+	}
+	free(orig_dfu_env);
+
+	if (ret)
 		return EFI_EXIT(EFI_DEVICE_ERROR);
 
 	efi_firmware_set_fmp_state_var(&state, image_index);
@@ -717,6 +734,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
 	u8 dfu_alt_num;
 	efi_status_t status;
 	struct fmp_state state = { 0 };
+	char *orig_dfu_env;
 
 	EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
 		  image_size, vendor_code, progress, abort_reason);
@@ -747,8 +765,23 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
 		}
 	}
 
-	if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size,
-			     NULL, NULL))
+	orig_dfu_env = strdup(env_get("dfu_alt_info"));
+	if (env_set("dfu_alt_info", update_info.dfu_string)) {
+		log_err("unable to set env variable \"dfu_alt_info\"!\n");
+		free(orig_dfu_env);
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+	}
+
+	ret = dfu_write_by_alt(dfu_alt_num, (void *)image, image_size,
+			       NULL, NULL);
+
+	if (env_set("dfu_alt_info", orig_dfu_env)) {
+		log_err("unable to set env variable \"dfu_alt_info\"!\n");
+		ret = 1;
+	}
+	free(orig_dfu_env);
+
+	if (ret)
 		return EFI_EXIT(EFI_DEVICE_ERROR);
 
 	efi_firmware_set_fmp_state_var(&state, image_index);
-- 
2.34.1

[PATCH v2 2/2] board: remove capsule update support in set_dfu_alt_info()

[Jonathan Humphreys]

Now that capsule update sets the dfu_alt_info environment variable
explicitly, there is no need to support it in the set_dfu_alt_info()
function. Decouple SET_DFU_ALT_INFO from EFI_CAPSULE_FIRMWARE_FIT and
EFI_CAPSULE_FIRMWARE_RAW. For many boards, this was the only use of
set_dfu_alt_info() so remove the function entirely.

Fixes: a9e6f01a941f ("efi: Define set_dfu_alt_info() for boards with UEFI capsule update enabled")

Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 board/beagle/beagleboneai64/beagleboneai64.c       | 8 --------
 board/beagle/beagleplay/beagleplay.c               | 8 --------
 board/libre-computer/aml-a311d-cc/aml-a311d-cc.c   | 2 --
 board/libre-computer/aml-s805x-ac/aml-s805x-ac.c   | 2 --
 board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c | 2 --
 board/phytec/common/k3/board.c                     | 8 --------
 board/ti/am62px/evm.c                              | 8 --------
 board/ti/am62x/evm.c                               | 8 --------
 board/ti/am64x/evm.c                               | 8 --------
 board/ti/j721e/evm.c                               | 8 --------
 board/ti/j784s4/evm.c                              | 8 --------
 lib/efi_loader/Kconfig                             | 2 --
 lib/efi_loader/efi_firmware.c                      | 5 -----
 13 files changed, 77 deletions(-)

diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c
index e8d07f1f95f..99eb8972cf3 100644
--- a/board/beagle/beagleboneai64/beagleboneai64.c
+++ b/board/beagle/beagleboneai64/beagleboneai64.c
@@ -45,14 +45,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/beagle/beagleplay/beagleplay.c b/board/beagle/beagleplay/beagleplay.c
index fae69b37585..78635810585 100644
--- a/board/beagle/beagleplay/beagleplay.c
+++ b/board/beagle/beagleplay/beagleplay.c
@@ -41,14 +41,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c b/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
index e45cfd5d8a3..24363d21ab0 100644
--- a/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
+++ b/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
@@ -31,8 +31,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 {
 	if (strcmp(interface, "ram") == 0)
 		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
-	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
 }
 #endif
 
diff --git a/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c b/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
index 94cf5b4361f..42442f26acc 100644
--- a/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
+++ b/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
@@ -38,8 +38,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 {
 	if (strcmp(interface, "ram") == 0)
 		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
-	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
 }
 #endif
 
diff --git a/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c b/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
index f641db5a494..5223f1c8ab8 100644
--- a/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
+++ b/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
@@ -31,8 +31,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 {
 	if (strcmp(interface, "ram") == 0)
 		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
-	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
 }
 #endif
 
diff --git a/board/phytec/common/k3/board.c b/board/phytec/common/k3/board.c
index 9d833456810..58859f279f1 100644
--- a/board/phytec/common/k3/board.c
+++ b/board/phytec/common/k3/board.c
@@ -82,14 +82,6 @@ void configure_capsule_updates(void)
 }
 #endif
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 #if IS_ENABLED(CONFIG_ENV_IS_IN_FAT) || IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
 int mmc_get_env_dev(void)
 {
diff --git a/board/ti/am62px/evm.c b/board/ti/am62px/evm.c
index 75359fa1614..379d1a5b316 100644
--- a/board/ti/am62px/evm.c
+++ b/board/ti/am62px/evm.c
@@ -41,14 +41,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c
index 279ceba9554..3051a0a27a1 100644
--- a/board/ti/am62x/evm.c
+++ b/board/ti/am62x/evm.c
@@ -74,14 +74,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c
index 6a17737d266..35fd30dbceb 100644
--- a/board/ti/am64x/evm.c
+++ b/board/ti/am64x/evm.c
@@ -54,14 +54,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
index 1fa78ff7b30..0525f6e6f97 100644
--- a/board/ti/j721e/evm.c
+++ b/board/ti/j721e/evm.c
@@ -65,14 +65,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/board/ti/j784s4/evm.c b/board/ti/j784s4/evm.c
index d317f3eccbb..c6e46b7ee0e 100644
--- a/board/ti/j784s4/evm.c
+++ b/board/ti/j784s4/evm.c
@@ -40,14 +40,6 @@ struct efi_capsule_update_info update_info = {
 	.images = fw_images,
 };
 
-#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
-void set_dfu_alt_info(char *interface, char *devstr)
-{
-	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
-		env_set("dfu_alt_info", update_info.dfu_string);
-}
-#endif
-
 int board_init(void)
 {
 	return 0;
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index d4f6b56afaa..1be11aed901 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -304,7 +304,6 @@ config EFI_CAPSULE_FIRMWARE_FIT
 	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
 	select UPDATE_FIT
 	select DFU
-	select SET_DFU_ALT_INFO
 	select EFI_CAPSULE_FIRMWARE
 	help
 	  Select this option if you want to enable firmware management protocol
@@ -316,7 +315,6 @@ config EFI_CAPSULE_FIRMWARE_RAW
 	depends on SANDBOX || (!SANDBOX && !EFI_CAPSULE_FIRMWARE_FIT)
 	select DFU_WRITE_ALT
 	select DFU
-	select SET_DFU_ALT_INFO
 	select EFI_CAPSULE_FIRMWARE
 	help
 	  Select this option if you want to enable firmware management protocol
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 1a1cf3b55e1..3286c21a2b7 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -56,11 +56,6 @@ struct fmp_state {
 	u32 last_attempt_status; /* not used */
 };
 
-__weak void set_dfu_alt_info(char *interface, char *devstr)
-{
-	env_set("dfu_alt_info", update_info.dfu_string);
-}
-
 /**
  * efi_firmware_get_image_type_id - get image_type_id
  * @image_index:	image index
-- 
2.34.1

Re: [PATCH v2 2/2] board: remove capsule update support in set_dfu_alt_info()

[Neil Armstrong]

On 06/02/2025 16:47, Jonathan Humphreys wrote:
> Now that capsule update sets the dfu_alt_info environment variable
> explicitly, there is no need to support it in the set_dfu_alt_info()
> function. Decouple SET_DFU_ALT_INFO from EFI_CAPSULE_FIRMWARE_FIT and
> EFI_CAPSULE_FIRMWARE_RAW. For many boards, this was the only use of
> set_dfu_alt_info() so remove the function entirely.
> 
> Fixes: a9e6f01a941f ("efi: Define set_dfu_alt_info() for boards with UEFI capsule update enabled")
> 
> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>   board/beagle/beagleboneai64/beagleboneai64.c       | 8 --------
>   board/beagle/beagleplay/beagleplay.c               | 8 --------
>   board/libre-computer/aml-a311d-cc/aml-a311d-cc.c   | 2 --
>   board/libre-computer/aml-s805x-ac/aml-s805x-ac.c   | 2 --
>   board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c | 2 --

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> # for board/libre-computer/*

>   board/phytec/common/k3/board.c                     | 8 --------
>   board/ti/am62px/evm.c                              | 8 --------
>   board/ti/am62x/evm.c                               | 8 --------
>   board/ti/am64x/evm.c                               | 8 --------
>   board/ti/j721e/evm.c                               | 8 --------
>   board/ti/j784s4/evm.c                              | 8 --------
>   lib/efi_loader/Kconfig                             | 2 --
>   lib/efi_loader/efi_firmware.c                      | 5 -----
>   13 files changed, 77 deletions(-)
> 
> diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c
> index e8d07f1f95f..99eb8972cf3 100644
> --- a/board/beagle/beagleboneai64/beagleboneai64.c
> +++ b/board/beagle/beagleboneai64/beagleboneai64.c
> @@ -45,14 +45,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/beagle/beagleplay/beagleplay.c b/board/beagle/beagleplay/beagleplay.c
> index fae69b37585..78635810585 100644
> --- a/board/beagle/beagleplay/beagleplay.c
> +++ b/board/beagle/beagleplay/beagleplay.c
> @@ -41,14 +41,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c b/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
> index e45cfd5d8a3..24363d21ab0 100644
> --- a/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
> +++ b/board/libre-computer/aml-a311d-cc/aml-a311d-cc.c
> @@ -31,8 +31,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
>   {
>   	if (strcmp(interface, "ram") == 0)
>   		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
> -	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
>   }
>   #endif
>   
> diff --git a/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c b/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
> index 94cf5b4361f..42442f26acc 100644
> --- a/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
> +++ b/board/libre-computer/aml-s805x-ac/aml-s805x-ac.c
> @@ -38,8 +38,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
>   {
>   	if (strcmp(interface, "ram") == 0)
>   		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
> -	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
>   }
>   #endif
>   
> diff --git a/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c b/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
> index f641db5a494..5223f1c8ab8 100644
> --- a/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
> +++ b/board/libre-computer/aml-s905d3-cc/aml-s905d3-cc.c
> @@ -31,8 +31,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
>   {
>   	if (strcmp(interface, "ram") == 0)
>   		env_set("dfu_alt_info", "fitimage ram 0x08080000 0x4000000");
> -	else if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
>   }
>   #endif
>   
> diff --git a/board/phytec/common/k3/board.c b/board/phytec/common/k3/board.c
> index 9d833456810..58859f279f1 100644
> --- a/board/phytec/common/k3/board.c
> +++ b/board/phytec/common/k3/board.c
> @@ -82,14 +82,6 @@ void configure_capsule_updates(void)
>   }
>   #endif
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   #if IS_ENABLED(CONFIG_ENV_IS_IN_FAT) || IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
>   int mmc_get_env_dev(void)
>   {
> diff --git a/board/ti/am62px/evm.c b/board/ti/am62px/evm.c
> index 75359fa1614..379d1a5b316 100644
> --- a/board/ti/am62px/evm.c
> +++ b/board/ti/am62px/evm.c
> @@ -41,14 +41,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c
> index 279ceba9554..3051a0a27a1 100644
> --- a/board/ti/am62x/evm.c
> +++ b/board/ti/am62x/evm.c
> @@ -74,14 +74,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c
> index 6a17737d266..35fd30dbceb 100644
> --- a/board/ti/am64x/evm.c
> +++ b/board/ti/am64x/evm.c
> @@ -54,14 +54,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
> index 1fa78ff7b30..0525f6e6f97 100644
> --- a/board/ti/j721e/evm.c
> +++ b/board/ti/j721e/evm.c
> @@ -65,14 +65,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/board/ti/j784s4/evm.c b/board/ti/j784s4/evm.c
> index d317f3eccbb..c6e46b7ee0e 100644
> --- a/board/ti/j784s4/evm.c
> +++ b/board/ti/j784s4/evm.c
> @@ -40,14 +40,6 @@ struct efi_capsule_update_info update_info = {
>   	.images = fw_images,
>   };
>   
> -#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
> -void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> -		env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -#endif
> -
>   int board_init(void)
>   {
>   	return 0;
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index d4f6b56afaa..1be11aed901 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -304,7 +304,6 @@ config EFI_CAPSULE_FIRMWARE_FIT
>   	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
>   	select UPDATE_FIT
>   	select DFU
> -	select SET_DFU_ALT_INFO
>   	select EFI_CAPSULE_FIRMWARE
>   	help
>   	  Select this option if you want to enable firmware management protocol
> @@ -316,7 +315,6 @@ config EFI_CAPSULE_FIRMWARE_RAW
>   	depends on SANDBOX || (!SANDBOX && !EFI_CAPSULE_FIRMWARE_FIT)
>   	select DFU_WRITE_ALT
>   	select DFU
> -	select SET_DFU_ALT_INFO
>   	select EFI_CAPSULE_FIRMWARE
>   	help
>   	  Select this option if you want to enable firmware management protocol
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 1a1cf3b55e1..3286c21a2b7 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -56,11 +56,6 @@ struct fmp_state {
>   	u32 last_attempt_status; /* not used */
>   };
>   
> -__weak void set_dfu_alt_info(char *interface, char *devstr)
> -{
> -	env_set("dfu_alt_info", update_info.dfu_string);
> -}
> -
>   /**
>    * efi_firmware_get_image_type_id - get image_type_id
>    * @image_index:	image index

Re: [PATCH v2 1/2] efi_firmware: set EFI capsule dfu_alt_info env explicitly

[Michal Simek]

On 2/6/25 16:47, Jonathan Humphreys wrote:
> The current implementation of EFI capsule update uses set_dfu_alt_info() to
> set the dfu_alt_info environment variable with the settings it requires.
> However, set_dfu_alt_info() is doing this for all DFU operations, even
> those unrelated to capsule update.
> 
> Thus other uses of DFU, such as DFU boot which sets its own value for the
> dfu_alt_info environment variable, will have that setting overwritten with
> the capsule update setting. Similarly, any user defined value for the
> dfu_alt_info environment variable would get overwritten when any DFU
> operation was performed, including simply performing a "dfu 0 list"
> command.
> 
> The solution is stop using the set_dfu_alt_info() mechanism to set the
> dfu_alt_info environment variable and instead explicitly set it to the
> capsule update's setting just before performing the capsule update's DFU
> operation, and then restore the environment variable back to its original
> value.
> 
> This patch implements the explicit setting and restoring of the
> dfu_alt_info environment variable as part of the EFI capsule update
> operation.
> 
> The fix is fully implemented in a subsequent patch that removes the capsule
> update dfu_alt_info support in set_dfu_alt_info().
> 
> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
> ---
>   lib/efi_loader/efi_firmware.c | 39 ++++++++++++++++++++++++++++++++---
>   1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 5a754c9cd03..1a1cf3b55e1 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -649,8 +649,10 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>   	efi_status_t (*progress)(efi_uintn_t completion),
>   	u16 **abort_reason)
>   {
> +	int ret;
>   	efi_status_t status;
>   	struct fmp_state state = { 0 };
> +	char *orig_dfu_env;
>   
>   	EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
>   		  image_size, vendor_code, progress, abort_reason);
> @@ -663,7 +665,22 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>   	if (status != EFI_SUCCESS)
>   		return EFI_EXIT(status);
>   
> -	if (fit_update(image))
> +	orig_dfu_env = strdup(env_get("dfu_alt_info"));
> +	if (env_set("dfu_alt_info", update_info.dfu_string)) {

This pretty much breaks all xilinx platforms because we actually are not 
configuring dfu_string.

I have sent RFC. If you can squash it to your patch that would be the best.
Pretty much the part of it should be in 1/2 and the part in 2/2.

M

Re: [PATCH v2 1/2] efi_firmware: set EFI capsule dfu_alt_info env explicitly

[Mattijs Korpershoek]

Hi Michal,

Thank you for testing this.

On lun., févr. 10, 2025 at 13:40, Michal Simek <michal.simek@amd.com> wrote:

> On 2/6/25 16:47, Jonathan Humphreys wrote:
>> The current implementation of EFI capsule update uses set_dfu_alt_info() to
>> set the dfu_alt_info environment variable with the settings it requires.
>> However, set_dfu_alt_info() is doing this for all DFU operations, even
>> those unrelated to capsule update.
>> 
>> Thus other uses of DFU, such as DFU boot which sets its own value for the
>> dfu_alt_info environment variable, will have that setting overwritten with
>> the capsule update setting. Similarly, any user defined value for the
>> dfu_alt_info environment variable would get overwritten when any DFU
>> operation was performed, including simply performing a "dfu 0 list"
>> command.
>> 
>> The solution is stop using the set_dfu_alt_info() mechanism to set the
>> dfu_alt_info environment variable and instead explicitly set it to the
>> capsule update's setting just before performing the capsule update's DFU
>> operation, and then restore the environment variable back to its original
>> value.
>> 
>> This patch implements the explicit setting and restoring of the
>> dfu_alt_info environment variable as part of the EFI capsule update
>> operation.
>> 
>> The fix is fully implemented in a subsequent patch that removes the capsule
>> update dfu_alt_info support in set_dfu_alt_info().
>> 
>> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com>
>> ---
>>   lib/efi_loader/efi_firmware.c | 39 ++++++++++++++++++++++++++++++++---
>>   1 file changed, 36 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
>> index 5a754c9cd03..1a1cf3b55e1 100644
>> --- a/lib/efi_loader/efi_firmware.c
>> +++ b/lib/efi_loader/efi_firmware.c
>> @@ -649,8 +649,10 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>>   	efi_status_t (*progress)(efi_uintn_t completion),
>>   	u16 **abort_reason)
>>   {
>> +	int ret;
>>   	efi_status_t status;
>>   	struct fmp_state state = { 0 };
>> +	char *orig_dfu_env;
>>   
>>   	EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
>>   		  image_size, vendor_code, progress, abort_reason);
>> @@ -663,7 +665,22 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>>   	if (status != EFI_SUCCESS)
>>   		return EFI_EXIT(status);
>>   
>> -	if (fit_update(image))
>> +	orig_dfu_env = strdup(env_get("dfu_alt_info"));
>> +	if (env_set("dfu_alt_info", update_info.dfu_string)) {
>
> This pretty much breaks all xilinx platforms because we actually are not 
> configuring dfu_string.
>
> I have sent RFC. If you can squash it to your patch that would be the best.
> Pretty much the part of it should be in 1/2 and the part in 2/2.

For reference, the patch that has been send as RFC is:
http://lore.kernel.org/r/c8378bd1bbc7a96ecd802897ca72e26a02bf5a2b.1739190503.git.michal.simek@amd.com

>
> M