[PATCH v4 0/3] EFI Capsule update explicitly sets dfu_alt_info

[PATCH v4 0/3] 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

Changes from v2:
- add patch for xilinx boards to set the dfu_string member with the created
  dfu_alt_info string for capsule updates
Link to v2: https://lore.kernel.org/r/20250206154719.3032322-1-j-humphreys@ti.com

Changes from v3:
- in case that the dfu_alt_info env variable is set and we save a copy
  using strdup(), check that strdup() doesn't fail
- separate the reporting of an error due to the DFU operation from failure
  to restore the dfu_alt_info environment variable. In the latter case,
  just emit a warning and return success for the DFU operation.
Link to v3: https://lore.kernel.org/r/20250213195351.3518305-1-j-humphreys@ti.com

Tested-by: Michal Simek <michal.simek@amd.com>

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

Michal Simek (1):
  xilinx: dfu: Fill directly update_info.dfu_string

 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 ---
 board/xilinx/common/board.h                   |  3 +
 board/xilinx/versal/board.c                   | 16 +++---
 board/xilinx/zynq/board.c                     | 16 +++---
 board/xilinx/zynqmp/zynqmp.c                  | 16 +++---
 lib/efi_loader/Kconfig                        |  2 -
 lib/efi_loader/efi_firmware.c                 | 56 ++++++++++++++++---
 17 files changed, 72 insertions(+), 107 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Jonathan Humphreys]

From: Michal Simek <michal.simek@amd.com>

Directly fill update_info.dfu_string to prepare platforms to switch
from using dfu_alt_info variable to dfu_string which contains description
for capsule update when switch is done.

Signed-off-by: Michal Simek <michal.simek@amd.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 board/xilinx/versal/board.c  | 3 +++
 board/xilinx/zynq/board.c    | 3 +++
 board/xilinx/zynqmp/zynqmp.c | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c
index b4483d00ad1..2c387630a61 100644
--- a/board/xilinx/versal/board.c
+++ b/board/xilinx/versal/board.c
@@ -8,6 +8,7 @@
 #include <cpu_func.h>
 #include <dfu.h>
 #include <env.h>
+#include <efi_loader.h>
 #include <fdtdec.h>
 #include <init.h>
 #include <env_internal.h>
@@ -438,5 +439,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
 
 	env_set("dfu_alt_info", buf);
 	puts("DFU alt info setting: done\n");
+	update_info.dfu_string = strdup(buf);
+	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
 #endif
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index a852d5b8ed5..bd004b86a88 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -7,6 +7,7 @@
 #include <config.h>
 #include <debug_uart.h>
 #include <dfu.h>
+#include <efi_loader.h>
 #include <init.h>
 #include <log.h>
 #include <dm/uclass.h>
@@ -199,5 +200,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
 
 	env_set("dfu_alt_info", buf);
 	puts("DFU alt info setting: done\n");
+	update_info.dfu_string = strdup(buf);
+	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
 #endif
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 820fb252a3f..8060d54428d 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -11,6 +11,7 @@
 #include <dfu.h>
 #include <env.h>
 #include <env_internal.h>
+#include <efi_loader.h>
 #include <init.h>
 #include <log.h>
 #include <net.h>
@@ -734,6 +735,8 @@ void set_dfu_alt_info(char *interface, char *devstr)
 
 	env_set("dfu_alt_info", buf);
 	puts("DFU alt info setting: done\n");
+	update_info.dfu_string = strdup(buf);
+	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
 #endif
 
-- 
2.34.1

[PATCH v4 2/3] 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>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 lib/efi_loader/efi_firmware.c | 51 ++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 5a754c9cd03..0ffaf5c8f72 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,28 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
 	if (status != EFI_SUCCESS)
 		return EFI_EXIT(status);
 
-	if (fit_update(image))
+	orig_dfu_env = env_get("dfu_alt_info");
+	if (orig_dfu_env) {
+		orig_dfu_env = strdup(orig_dfu_env);
+		if (!orig_dfu_env) {
+			log_err("strdup() failed!\n");
+			return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+		}
+	}
+	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_warning("Unable to restore env variable \"dfu_alt_info\".  Further DFU operations may fail!\n");
+
+	free(orig_dfu_env);
+
+	if (ret)
 		return EFI_EXIT(EFI_DEVICE_ERROR);
 
 	efi_firmware_set_fmp_state_var(&state, image_index);
@@ -717,6 +740,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 +771,29 @@ 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 = env_get("dfu_alt_info");
+	if (orig_dfu_env) {
+		orig_dfu_env = strdup(orig_dfu_env);
+		if (!orig_dfu_env) {
+			log_err("strdup() failed!\n");
+			return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+		}
+	}
+	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_warning("Unable to restore env variable \"dfu_alt_info\".  Further DFU operations may fail!\n");
+
+	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 v4 3/3] 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>
Signed-off-by: Michal Simek <michal.simek@amd.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> # for board/libre-computer/*
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 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 --------
 board/xilinx/common/board.h                        |  3 +++
 board/xilinx/versal/board.c                        | 13 ++++---------
 board/xilinx/zynq/board.c                          | 13 ++++---------
 board/xilinx/zynqmp/zynqmp.c                       | 13 ++++---------
 lib/efi_loader/Kconfig                             |  2 --
 lib/efi_loader/efi_firmware.c                      |  5 -----
 17 files changed, 15 insertions(+), 104 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/board/xilinx/common/board.h b/board/xilinx/common/board.h
index 64d657673e9..cb86c4c5b91 100644
--- a/board/xilinx/common/board.h
+++ b/board/xilinx/common/board.h
@@ -18,4 +18,7 @@ bool board_detection(void);
 char *soc_name_decode(void);
 
 bool soc_detection(void);
+
+void configure_capsule_updates(void);
+
 #endif /* BOARD_XILINX_COMMON_BOARD_H */
diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c
index 2c387630a61..05530736751 100644
--- a/board/xilinx/versal/board.c
+++ b/board/xilinx/versal/board.c
@@ -281,6 +281,9 @@ int board_late_init(void)
 {
 	int ret;
 
+	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
+		configure_capsule_updates();
+
 	if (!(gd->flags & GD_FLG_ENV_DEFAULT)) {
 		debug("Saved variables - Skipping\n");
 		return 0;
@@ -357,8 +360,6 @@ enum env_location env_get_location(enum env_operation op, int prio)
 }
 #endif
 
-#if defined(CONFIG_SET_DFU_ALT_INFO)
-
 #define DFU_ALT_BUF_LEN		SZ_1K
 
 static void mtd_found_part(u32 *base, u32 *size)
@@ -386,7 +387,7 @@ static void mtd_found_part(u32 *base, u32 *size)
 	}
 }
 
-void set_dfu_alt_info(char *interface, char *devstr)
+void configure_capsule_updates(void)
 {
 	int bootseq = 0, len = 0;
 	u32 multiboot = versal_multi_boot();
@@ -394,9 +395,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
 
-	if (env_get("dfu_alt_info"))
-		return;
-
 	memset(buf, 0, sizeof(buf));
 
 	multiboot = env_get_hex("multiboot", multiboot);
@@ -437,9 +435,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 		return;
 	}
 
-	env_set("dfu_alt_info", buf);
-	puts("DFU alt info setting: done\n");
 	update_info.dfu_string = strdup(buf);
 	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
-#endif
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index bd004b86a88..b1fd768a26c 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -53,6 +53,9 @@ int board_late_init(void)
 	char *new_targets;
 	char *env_targets;
 
+	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
+		configure_capsule_updates();
+
 	if (!(gd->flags & GD_FLG_ENV_DEFAULT)) {
 		debug("Saved variables - Skipping\n");
 		return 0;
@@ -166,17 +169,12 @@ enum env_location env_get_location(enum env_operation op, int prio)
 	}
 }
 
-#if defined(CONFIG_SET_DFU_ALT_INFO)
-
 #define DFU_ALT_BUF_LEN                SZ_1K
 
-void set_dfu_alt_info(char *interface, char *devstr)
+void configure_capsule_updates(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
 
-	if (env_get("dfu_alt_info"))
-		return;
-
 	memset(buf, 0, sizeof(buf));
 
 	switch ((zynq_slcr_get_boot_mode()) & ZYNQ_BM_MASK) {
@@ -198,9 +196,6 @@ void set_dfu_alt_info(char *interface, char *devstr)
 		return;
 	}
 
-	env_set("dfu_alt_info", buf);
-	puts("DFU alt info setting: done\n");
 	update_info.dfu_string = strdup(buf);
 	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
-#endif
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 8060d54428d..33205d4cf1d 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -527,6 +527,9 @@ int board_late_init(void)
 	usb_ether_init();
 #endif
 
+	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
+		configure_capsule_updates();
+
 	multiboot = multi_boot();
 	if (multiboot >= 0)
 		env_set_hex("multiboot", multiboot);
@@ -632,8 +635,6 @@ enum env_location env_get_location(enum env_operation op, int prio)
 }
 #endif
 
-#if defined(CONFIG_SET_DFU_ALT_INFO)
-
 #define DFU_ALT_BUF_LEN		SZ_1K
 
 static void mtd_found_part(u32 *base, u32 *size)
@@ -661,15 +662,12 @@ static void mtd_found_part(u32 *base, u32 *size)
 	}
 }
 
-void set_dfu_alt_info(char *interface, char *devstr)
+void configure_capsule_updates(void)
 {
 	int multiboot, bootseq = 0, len = 0;
 
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
 
-	if (env_get("dfu_alt_info"))
-		return;
-
 	memset(buf, 0, sizeof(buf));
 
 	multiboot = multi_boot();
@@ -733,12 +731,9 @@ void set_dfu_alt_info(char *interface, char *devstr)
 		return;
 	}
 
-	env_set("dfu_alt_info", buf);
-	puts("DFU alt info setting: done\n");
 	update_info.dfu_string = strdup(buf);
 	debug("Capsule DFU: %s\n", update_info.dfu_string);
 }
-#endif
 
 #if defined(CONFIG_SPL_SPI_LOAD)
 unsigned int spl_spi_get_uboot_offs(struct spi_flash *flash)
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 0ffaf5c8f72..d44dc09813e 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 v4 3/3] board: remove capsule update support in set_dfu_alt_info()

[Wadim Egorov]

Am 27.02.25 um 05:35 schrieb 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>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> # for board/libre-computer/*
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   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 --------
>   board/xilinx/common/board.h                        |  3 +++
>   board/xilinx/versal/board.c                        | 13 ++++---------
>   board/xilinx/zynq/board.c                          | 13 ++++---------
>   board/xilinx/zynqmp/zynqmp.c                       | 13 ++++---------
>   lib/efi_loader/Kconfig                             |  2 --
>   lib/efi_loader/efi_firmware.c                      |  5 -----
>   17 files changed, 15 insertions(+), 104 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)
>   {

Reviewed-by: Wadim Egorov <w.egorov@phytec.de> # for 
board/phytec/common/k3/board.c

Re: [PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Heinrich Schuchardt]

On 26.02.25 23:35, Jonathan Humphreys wrote:
> From: Michal Simek <michal.simek@amd.com>
>
> Directly fill update_info.dfu_string to prepare platforms to switch
> from using dfu_alt_info variable to dfu_string which contains description
> for capsule update when switch is done.

Other platforms only set the environment variable.

Do we have a problem that needs to be solved for all problems?
Or is anything special about the Xilinx platform?

If we have a general problem, solving it for Xilinx only would not be
the right approach.

Best regards

Heinrich

>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   board/xilinx/versal/board.c  | 3 +++
>   board/xilinx/zynq/board.c    | 3 +++
>   board/xilinx/zynqmp/zynqmp.c | 3 +++
>   3 files changed, 9 insertions(+)
>
> diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c
> index b4483d00ad1..2c387630a61 100644
> --- a/board/xilinx/versal/board.c
> +++ b/board/xilinx/versal/board.c
> @@ -8,6 +8,7 @@
>   #include <cpu_func.h>
>   #include <dfu.h>
>   #include <env.h>
> +#include <efi_loader.h>
>   #include <fdtdec.h>
>   #include <init.h>
>   #include <env_internal.h>
> @@ -438,5 +439,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
>
>   	env_set("dfu_alt_info", buf);
>   	puts("DFU alt info setting: done\n");
> +	update_info.dfu_string = strdup(buf);
> +	debug("Capsule DFU: %s\n", update_info.dfu_string);
>   }
>   #endif
> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> index a852d5b8ed5..bd004b86a88 100644
> --- a/board/xilinx/zynq/board.c
> +++ b/board/xilinx/zynq/board.c
> @@ -7,6 +7,7 @@
>   #include <config.h>
>   #include <debug_uart.h>
>   #include <dfu.h>
> +#include <efi_loader.h>
>   #include <init.h>
>   #include <log.h>
>   #include <dm/uclass.h>
> @@ -199,5 +200,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
>
>   	env_set("dfu_alt_info", buf);
>   	puts("DFU alt info setting: done\n");
> +	update_info.dfu_string = strdup(buf);
> +	debug("Capsule DFU: %s\n", update_info.dfu_string);
>   }
>   #endif
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 820fb252a3f..8060d54428d 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -11,6 +11,7 @@
>   #include <dfu.h>
>   #include <env.h>
>   #include <env_internal.h>
> +#include <efi_loader.h>
>   #include <init.h>
>   #include <log.h>
>   #include <net.h>
> @@ -734,6 +735,8 @@ void set_dfu_alt_info(char *interface, char *devstr)
>
>   	env_set("dfu_alt_info", buf);
>   	puts("DFU alt info setting: done\n");
> +	update_info.dfu_string = strdup(buf);
> +	debug("Capsule DFU: %s\n", update_info.dfu_string);
>   }
>   #endif
>

Re: [PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Michal Simek]

On 2/28/25 15:33, Heinrich Schuchardt wrote:
> On 26.02.25 23:35, Jonathan Humphreys wrote:
>> From: Michal Simek <michal.simek@amd.com>
>>
>> Directly fill update_info.dfu_string to prepare platforms to switch
>> from using dfu_alt_info variable to dfu_string which contains description
>> for capsule update when switch is done.
> 
> Other platforms only set the environment variable.

Not really.

git grep \.dfu_string board/

beagle, imx, rpi, ti and others are also doing it.

> 
> Do we have a problem that needs to be solved for all problems?
> Or is anything special about the Xilinx platform?
> 
> If we have a general problem, solving it for Xilinx only would not be
> the right approach.

This is just preparation patch for switch in 3/3 because then Jon would have to 
do this change in his patch.
But the biggest reason is when 2/3 is applied, capsule update will stop to work 
on our platform and I wanted to make sure that we can avoid it.

Thanks,
Michal

Re: [PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Heinrich Schuchardt]

On 28.02.25 16:41, Michal Simek wrote:
>
>
> On 2/28/25 15:33, Heinrich Schuchardt wrote:
>> On 26.02.25 23:35, Jonathan Humphreys wrote:
>>> From: Michal Simek <michal.simek@amd.com>
>>>
>>> Directly fill update_info.dfu_string to prepare platforms to switch
>>> from using dfu_alt_info variable to dfu_string which contains
>>> description
>>> for capsule update when switch is done.
>>
>> Other platforms only set the environment variable.
>
> Not really.
>
> git grep \.dfu_string board/
>
> beagle, imx, rpi, ti and others are also doing it.

But why do we have platforms that don't?

>
>>
>> Do we have a problem that needs to be solved for all problems?
>> Or is anything special about the Xilinx platform?
>>
>> If we have a general problem, solving it for Xilinx only would not be
>> the right approach.
>
> This is just preparation patch for switch in 3/3 because then Jon would
> have to do this change in his patch.
> But the biggest reason is when 2/3 is applied, capsule update will stop
> to work on our platform and I wanted to make sure that we can avoid it.
>
> Thanks,
> Michal
>
>

Re: [PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Jon Humphreys]

Heinrich Schuchardt <xypron.glpk@gmx.de> writes:

> On 28.02.25 16:41, Michal Simek wrote:
>>
>>
>> On 2/28/25 15:33, Heinrich Schuchardt wrote:
>>> On 26.02.25 23:35, Jonathan Humphreys wrote:
>>>> From: Michal Simek <michal.simek@amd.com>
>>>>
>>>> Directly fill update_info.dfu_string to prepare platforms to switch
>>>> from using dfu_alt_info variable to dfu_string which contains
>>>> description
>>>> for capsule update when switch is done.
>>>
>>> Other platforms only set the environment variable.
>>
>> Not really.
>>
>> git grep \.dfu_string board/
>>
>> beagle, imx, rpi, ti and others are also doing it.
>
> But why do we have platforms that don't?
>

I'm not aware of the history of the Xilinx platforms and why they didn't,
but Michal has fixed this.  I'm not aware of any other platforms not
setting .dfu_string.

Jon

>>
>>>
>>> Do we have a problem that needs to be solved for all problems?
>>> Or is anything special about the Xilinx platform?
>>>
>>> If we have a general problem, solving it for Xilinx only would not be
>>> the right approach.
>>
>> This is just preparation patch for switch in 3/3 because then Jon would
>> have to do this change in his patch.
>> But the biggest reason is when 2/3 is applied, capsule update will stop
>> to work on our platform and I wanted to make sure that we can avoid it.
>>
>> Thanks,
>> Michal
>>
>>

Re: [PATCH v4 1/3] xilinx: dfu: Fill directly update_info.dfu_string

[Michal Simek]

On 2/28/25 19:15, Jon Humphreys wrote:
> Heinrich Schuchardt <xypron.glpk@gmx.de> writes:
> 
>> On 28.02.25 16:41, Michal Simek wrote:
>>>
>>>
>>> On 2/28/25 15:33, Heinrich Schuchardt wrote:
>>>> On 26.02.25 23:35, Jonathan Humphreys wrote:
>>>>> From: Michal Simek <michal.simek@amd.com>
>>>>>
>>>>> Directly fill update_info.dfu_string to prepare platforms to switch
>>>>> from using dfu_alt_info variable to dfu_string which contains
>>>>> description
>>>>> for capsule update when switch is done.
>>>>
>>>> Other platforms only set the environment variable.
>>>
>>> Not really.
>>>
>>> git grep \.dfu_string board/
>>>
>>> beagle, imx, rpi, ti and others are also doing it.
>>
>> But why do we have platforms that don't?
>>
> 
> I'm not aware of the history of the Xilinx platforms and why they didn't,
> but Michal has fixed this.  I'm not aware of any other platforms not
> setting .dfu_string.

It has never been wired like this from the beginning.
In April 2022 the patch
741ef867288b ("capsule: board: Add information needed for capsule updates")
added description for capsules without defining dfu_string because dfu_alt_info 
has been properly generated.

M

Re: [PATCH v4 2/3] efi_firmware: set EFI capsule dfu_alt_info env explicitly

[Ilias Apalodimas]

On Thu, 27 Feb 2025 at 00:36, Jonathan Humphreys <j-humphreys@ti.com> 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>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>  lib/efi_loader/efi_firmware.c | 51 ++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 5a754c9cd03..0ffaf5c8f72 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,28 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>         if (status != EFI_SUCCESS)
>                 return EFI_EXIT(status);
>
> -       if (fit_update(image))
> +       orig_dfu_env = env_get("dfu_alt_info");
> +       if (orig_dfu_env) {
> +               orig_dfu_env = strdup(orig_dfu_env);
> +               if (!orig_dfu_env) {
> +                       log_err("strdup() failed!\n");
> +                       return EFI_EXIT(EFI_OUT_OF_RESOURCES);
> +               }
> +       }
> +       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_warning("Unable to restore env variable \"dfu_alt_info\".  Further DFU operations may fail!\n");
> +
> +       free(orig_dfu_env);
> +
> +       if (ret)
>                 return EFI_EXIT(EFI_DEVICE_ERROR);
>
>         efi_firmware_set_fmp_state_var(&state, image_index);
> @@ -717,6 +740,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 +771,29 @@ 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 = env_get("dfu_alt_info");
> +       if (orig_dfu_env) {
> +               orig_dfu_env = strdup(orig_dfu_env);
> +               if (!orig_dfu_env) {
> +                       log_err("strdup() failed!\n");
> +                       return EFI_EXIT(EFI_OUT_OF_RESOURCES);
> +               }
> +       }
> +       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_warning("Unable to restore env variable \"dfu_alt_info\".  Further DFU operations may fail!\n");
> +
> +       free(orig_dfu_env);
> +
> +       if (ret)
>                 return EFI_EXIT(EFI_DEVICE_ERROR);
>
>         efi_firmware_set_fmp_state_var(&state, image_index);
> --
> 2.34.1
>

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

Re: [PATCH v4 0/3] EFI Capsule update explicitly sets dfu_alt_info

[Michal Simek]

Hi,

st 26. 2. 2025 v 23:36 odesílatel Jonathan Humphreys
<j-humphreys@ti.com> napsal:
>
> 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
>
> Changes from v2:
> - add patch for xilinx boards to set the dfu_string member with the created
>   dfu_alt_info string for capsule updates
> Link to v2: https://lore.kernel.org/r/20250206154719.3032322-1-j-humphreys@ti.com
>
> Changes from v3:
> - in case that the dfu_alt_info env variable is set and we save a copy
>   using strdup(), check that strdup() doesn't fail
> - separate the reporting of an error due to the DFU operation from failure
>   to restore the dfu_alt_info environment variable. In the latter case,
>   just emit a warning and return success for the DFU operation.
> Link to v3: https://lore.kernel.org/r/20250213195351.3518305-1-j-humphreys@ti.com
>
> Tested-by: Michal Simek <michal.simek@amd.com>
>
> Jonathan Humphreys (2):
>   efi_firmware: set EFI capsule dfu_alt_info env explicitly
>   board: remove capsule update support in set_dfu_alt_info()
>
> Michal Simek (1):
>   xilinx: dfu: Fill directly update_info.dfu_string
>
>  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 ---
>  board/xilinx/common/board.h                   |  3 +
>  board/xilinx/versal/board.c                   | 16 +++---
>  board/xilinx/zynq/board.c                     | 16 +++---
>  board/xilinx/zynqmp/zynqmp.c                  | 16 +++---
>  lib/efi_loader/Kconfig                        |  2 -
>  lib/efi_loader/efi_firmware.c                 | 56 ++++++++++++++++---
>  17 files changed, 72 insertions(+), 107 deletions(-)
>
> --
> 2.34.1
>

Is anybody going to take this series?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

Re: [PATCH v4 0/3] EFI Capsule update explicitly sets dfu_alt_info

[Jon Humphreys]

Michal Simek <monstr@monstr.eu> writes:

> Hi,
>
> st 26. 2. 2025 v 23:36 odesílatel Jonathan Humphreys
> <j-humphreys@ti.com> napsal:
>>
>> 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
>>
>> Changes from v2:
>> - add patch for xilinx boards to set the dfu_string member with the created
>>   dfu_alt_info string for capsule updates
>> Link to v2: https://lore.kernel.org/r/20250206154719.3032322-1-j-humphreys@ti.com
>>
>> Changes from v3:
>> - in case that the dfu_alt_info env variable is set and we save a copy
>>   using strdup(), check that strdup() doesn't fail
>> - separate the reporting of an error due to the DFU operation from failure
>>   to restore the dfu_alt_info environment variable. In the latter case,
>>   just emit a warning and return success for the DFU operation.
>> Link to v3: https://lore.kernel.org/r/20250213195351.3518305-1-j-humphreys@ti.com
>>
>> Tested-by: Michal Simek <michal.simek@amd.com>
>>
>> Jonathan Humphreys (2):
>>   efi_firmware: set EFI capsule dfu_alt_info env explicitly
>>   board: remove capsule update support in set_dfu_alt_info()
>>
>> Michal Simek (1):
>>   xilinx: dfu: Fill directly update_info.dfu_string
>>
>>  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 ---
>>  board/xilinx/common/board.h                   |  3 +
>>  board/xilinx/versal/board.c                   | 16 +++---
>>  board/xilinx/zynq/board.c                     | 16 +++---
>>  board/xilinx/zynqmp/zynqmp.c                  | 16 +++---
>>  lib/efi_loader/Kconfig                        |  2 -
>>  lib/efi_loader/efi_firmware.c                 | 56 ++++++++++++++++---
>>  17 files changed, 72 insertions(+), 107 deletions(-)
>>
>> --
>> 2.34.1
>>
>
> Is anybody going to take this series?
>
> Thanks,
> Michal
>
> -- 
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

I believe it is Heinrich Schuchardt's queue.

Heinrich, is there anything else to be closed?

Thanks
Jon