All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements
@ 2022-03-31 13:27 Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates Sughosh Ganu
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek


This series is cleaning up the usage of the image GUIDs that are used
in capsule update and the EFI System Resource Table(ESRT). There are
some other enhancements being made to the capsule update code to make
it more robust.

Firstly, an overview of the fixes being made.

Currently, there are two instances of the Firmware Management
Protocol(FMP), one defined for updating the FIT images, and the other
for updating raw images. The FMP code defines two GUID values, one for
all FIT images, and one for raw images. Depending on the FMP instance
used on a platform, the platform needs to use the corresponding image
GUID value for all images on the platform, and also across platforms.

A few issues are being fixed through the patch series. One, that an
image for a different platform can be flashed on another platform if
both the platforms are using the same FMP instance. So, for e.g. a
capsule generated for the Socionext DeveloperBox platform can be
flashed on the ZynqMP platform, since both the platforms use the
CONFIG_EFI_CAPSULE_FIRMWARE_RAW instance of the FMP. This can be
corrected if each firmware image that can be updated through the
capsule update mechanism has it's own unique image GUID.

The second issue that this patch series fixes is the value of FwClass
in the ESRT. With the current logic, all firmware image entries in the
ESRT display the same GUID value -- either the FIT GUID or the raw
GUID. This is not in compliance with the UEFI specification, as the
specification requires all entries to have unique GUID values.

The third issue being fixed is the population of the
EFI_FIRMWARE_IMAGE_DESCRIPTOR array. The current code uses the dfu
framework for populating the image descriptor array. However, there
might be other images that are not to be updated through the capsule
update mechanism also registered with the dfu framework. As a result
of this, the ESRT will show up entries of images that are not to be
targeted by the capsule update mechanism.

These issues are being fixed by defining a structure, efi_fw_images. A
platform can then define image related information like the image GUID
and image name. Every platform that uses capsule update mechanism
needs to define fw_images array. This array will then be used to
populate the image descriptor array, and also in determining if a
particular capsule's payload can be used for updating an image on the
platform.


The other part of the patches are some enhancements being made to the
capsule update code to make it more robust.

The first enhancement being made is to have a check for the image
index being passed through the capsule header. The capsule update code
uses the image index value as the dfu alt number, which points to the
partition to which the update must be made. The platform is supposed
to define the image index value for the updatable firmare images as
part of the fw_images array. This value must correspond to the dfu alt
num for the corresponding image, and can be obtained by checking the
output of the 'dfu list' u-boot command. At the time of update, the
image index being passed through the capsule is checked against the
image index value obtained from the platform.

The second enhancement made is the retrieval of the dfu_alt_info
variable from the set_dfu_alt_info function defined in the board
file. The dfu framework checks for the existence of this function, and
if the function is not defined, gets the value from the
environment. This can cause in an incorrect update if the environment
variable value is incorrect. By mandating all the platforms which
enable the capsule update feature to define set_dfu_alt_info function,
the variable is then obtained from the board file rather than the
environment.

The first patch adds the structure efi_fw_images and defines an array
fw_images on all platforms which enable capsule update feature

The second patch populates the image descriptor array in the
GetImageInfo function with the values from the fw_images array

The third patch adds a check for the image index value from the
capsule header against the value obtained from the fw_images array for
the corresponding image

The fourth patch defines the function set_dfu_alt_info for all the
platforms which enable capsule update feature and selects
SET_DFU_ALT_INFO config symbol for all platforms that enable capsule
update feature.

The fifth patch splits the capsule update test script into two, one
for FMP for raw images, and one for FMP for FIT images. The test for
FIT images is being enabled on the sandbox_flattree variant.

The sixth patch removes the now unused FIT and raw image GUID values
from the FMP module.

The seventh patch removes the --raw and --fit command line parameters
in the mkeficapsule utility.

The eighth patch makes corresponding changes in the capsule update
related documentation.


Changes since V3:
-----------------

* Do not remove the existing dfu_alt_info definitions made by
  platforms in the config files, as discussed with Masami.
* Squash the selection of the SET_DFU_ALT_INFO config symbol for
  capsule update feature as part of this patch.
* Rephrase the commit message to indicate that the doc changes are not
  just limited to adding the GUID values, but other info as well.
* Elaborate with an example on the relation between the dfu alt number
  and the image index 

Changes since V2:
-----------------

* Add a new member image_index to the struct efi_fw_images to allow
  the platforms to define the values for images.
* Address review comments from Michal Simek for the xilinx boards.
* Fix double inclusion of efi_loader.h as was pointed out by Heiko
  Thiery.
* Use the image index values defined in the platform's fw_images array
  for the image descriptors
* Add a description for adding image index value and definition of
  set_dfu_alt_info function for the capsule updates.

Changes since V1:
-----------------

* Make changes for the xilinx boards as suggested by Michal Simek.
* Add a GUID for the sandbox FIT image.
* Split the capsule update test cases into two scripts, one for raw
  images and one for FIT images.
* Add the capsule update test case for FIT images on sandbox64 and
  sandbox_flattree variants.
* Add capsule update support on sandbox_flattree variant for enabling
  FIT capsule update testing as part of the python tests

Sughosh Ganu (8):
  capsule: Add Image GUIDs and image index for platforms using capsule
    updates
  capsule: FMP: Populate the image descriptor array from platform data
  capsule: Put a check for image index before the update
  board: Define set_dfu_alt_info() for boards with UEFI capsule update
    enabled
  test: capsule: Modify the capsule tests to use GUID values for sandbox
  FMP: Remove GUIDs for FIT and raw images
  mkeficapsule: Remove raw and FIT GUID types
  doc: uefi: Update the capsule update related documentation

 .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       |  44 +++++
 .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   |  43 ++++
 board/emulation/common/qemu_dfu.c             |   6 +-
 board/emulation/qemu-arm/qemu-arm.c           |  20 ++
 board/kontron/pitx_imx8m/pitx_imx8m.c         |  40 +++-
 board/kontron/sl-mx8mm/sl-mx8mm.c             |  39 ++++
 board/kontron/sl28/sl28.c                     |  40 ++++
 board/sandbox/sandbox.c                       |  54 +++++
 board/socionext/developerbox/developerbox.c   |  52 +++++
 board/xilinx/common/board.c                   |  24 +++
 board/xilinx/zynq/board.c                     |   5 +-
 board/xilinx/zynqmp/zynqmp.c                  |   5 +-
 configs/sandbox64_defconfig                   |   1 -
 configs/sandbox_defconfig                     |   1 -
 configs/sandbox_flattree_defconfig            |   5 +
 doc/develop/uefi/uefi.rst                     |  24 ++-
 include/configs/imx8mm-cl-iot-gate.h          |  10 +
 include/configs/imx8mp_rsb3720.h              |  10 +
 include/configs/kontron-sl-mx8mm.h            |   6 +
 include/configs/kontron_pitx_imx8m.h          |   6 +
 include/configs/kontron_sl28.h                |   6 +
 include/configs/qemu-arm.h                    |  10 +
 include/configs/sandbox.h                     |  14 ++
 include/configs/synquacer.h                   |  14 ++
 include/configs/xilinx_versal.h               |   6 +
 include/configs/xilinx_zynqmp.h               |  10 +
 include/configs/zynq-common.h                 |  10 +
 include/efi_api.h                             |   8 -
 include/efi_loader.h                          |  21 ++
 lib/efi_loader/Kconfig                        |   2 +
 lib/efi_loader/efi_capsule.c                  |   8 +-
 lib/efi_loader/efi_firmware.c                 |  95 +++------
 test/py/tests/test_efi_capsule/conftest.py    |  21 +-
 .../test_capsule_firmware_fit.py              | 186 ++++++++++++++++++
 ...rmware.py => test_capsule_firmware_raw.py} | 159 +++++----------
 tools/eficapsule.h                            |   8 -
 tools/mkeficapsule.c                          |  26 +--
 37 files changed, 802 insertions(+), 237 deletions(-)
 create mode 100644 test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
 rename test/py/tests/test_efi_capsule/{test_capsule_firmware.py => test_capsule_firmware_raw.py} (75%)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 14:03   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data Sughosh Ganu
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

Currently, all platforms that enable capsule updates do so using
either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID or
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID. This is based on the Firmware
Management Protocol(FMP) instance used on the platform. However, this
means that all platforms that enable a particular FMP instance have
the same GUID value for all the updatable images, either the FIT image
GUID or the raw image GUID, and that an image for some platform can be
updated on any other platform which uses the same FMP instance. Another
issue with this implementation is that the ESRT table shows the same
GUID value for all images on the platform and also across platforms,
which is not in compliance with the UEFI specification.

Fix this by defining image GUID values and firmware names for
individual images per platform. The GetImageInfo FMP hook would then
populate these values in the image descriptor array.

Also add the image index value associated with a particular
image. This is the value that should match with the image_index value
that is part of the capsule header. The capsule update code will check
if the two values match, and the update will only proceed on a match.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---

Changes since V3: None

 .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 20 +++++++++++++
 .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 19 +++++++++++++
 board/emulation/qemu-arm/qemu-arm.c           | 20 +++++++++++++
 board/kontron/pitx_imx8m/pitx_imx8m.c         | 16 ++++++++++-
 board/kontron/sl-mx8mm/sl-mx8mm.c             | 15 ++++++++++
 board/kontron/sl28/sl28.c                     | 15 ++++++++++
 board/sandbox/sandbox.c                       | 28 +++++++++++++++++++
 board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++
 board/xilinx/common/board.c                   | 24 ++++++++++++++++
 include/configs/imx8mm-cl-iot-gate.h          | 10 +++++++
 include/configs/imx8mp_rsb3720.h              | 10 +++++++
 include/configs/kontron-sl-mx8mm.h            |  6 ++++
 include/configs/kontron_pitx_imx8m.h          |  6 ++++
 include/configs/kontron_sl28.h                |  6 ++++
 include/configs/qemu-arm.h                    | 10 +++++++
 include/configs/sandbox.h                     | 14 ++++++++++
 include/configs/synquacer.h                   | 14 ++++++++++
 include/configs/xilinx_versal.h               |  6 ++++
 include/configs/xilinx_zynqmp.h               | 10 +++++++
 include/configs/zynq-common.h                 | 10 +++++++
 include/efi_loader.h                          | 18 ++++++++++++
 21 files changed, 302 insertions(+), 1 deletion(-)

diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
index 16566092bd..1c953ba195 100644
--- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
+++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
@@ -6,6 +6,8 @@
 
 #include <common.h>
 #include <dwc3-uboot.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <errno.h>
 #include <miiphy.h>
 #include <netdev.h>
@@ -21,6 +23,7 @@
 #include <asm/arch/clock.h>
 #include <asm/mach-imx/dma.h>
 #include <linux/delay.h>
+#include <linux/kernel.h>
 #include <power/pmic.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -44,6 +47,23 @@ static void setup_gpmi_nand(void)
 }
 #endif
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+#if defined(CONFIG_TARGET_IMX8MP_RSB3720A1_4G)
+		.image_type_id = IMX8MP_RSB3720A1_4G_FIT_IMAGE_GUID,
+#elif defined(CONFIG_TARGET_IMX8MP_RSB3720A1_6G)
+		.image_type_id = IMX8MP_RSB3720A1_6G_FIT_IMAGE_GUID,
+#endif
+		.fw_name = u"IMX8MP-RSB3720-FIT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
+
 int board_early_init_f(void)
 {
 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
index 7e2d88f449..f5b89a5ddc 100644
--- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
+++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
@@ -5,6 +5,8 @@
  */
 
 #include <common.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <env.h>
 #include <extension_board.h>
 #include <hang.h>
@@ -21,11 +23,28 @@
 #include <asm/mach-imx/gpio.h>
 #include <asm/mach-imx/mxc_i2c.h>
 #include <asm/sections.h>
+#include <linux/kernel.h>
 
 #include "ddr/ddr.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+#if defined(CONFIG_TARGET_IMX8MM_CL_IOT_GATE)
+		.image_type_id = IMX8MM_CL_IOT_GATE_FIT_IMAGE_GUID,
+#elif defined(CONFIG_TARGET_IMX8MM_CL_IOT_GATE_OPTEE)
+		.image_type_id = IMX8MM_CL_IOT_GATE_OPTEE_FIT_IMAGE_GUID,
+#endif
+		.fw_name = u"IMX8MM-CL-IOT-GATE-FIT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 int board_phys_sdram_size(phys_size_t *size)
 {
 	struct lpddr4_tcm_desc *lpddr4_tcm_desc =
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 16d5a97167..ef77d0dc14 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -6,15 +6,35 @@
 #include <common.h>
 #include <cpu_func.h>
 #include <dm.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <fdtdec.h>
 #include <init.h>
 #include <log.h>
 #include <virtio_types.h>
 #include <virtio.h>
 
+#include <linux/kernel.h>
+
 #ifdef CONFIG_ARM64
 #include <asm/armv8/mmu.h>
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+#if defined(CONFIG_TARGET_QEMU_ARM_32BIT)
+		.image_type_id = QEMU_ARM_UBOOT_IMAGE_GUID,
+#elif defined(CONFIG_TARGET_QEMU_ARM_64BIT)
+		.image_type_id = QEMU_ARM64_UBOOT_IMAGE_GUID,
+#endif
+		.fw_name = u"Qemu-Arm-UBOOT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 static struct mm_region qemu_arm64_mem_map[] = {
 	{
 		/* Flash */
diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
index d655fe099b..8dc04411cc 100644
--- a/board/kontron/pitx_imx8m/pitx_imx8m.c
+++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
@@ -2,6 +2,8 @@
 
 #include "pitx_misc.h"
 #include <common.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <init.h>
 #include <mmc.h>
 #include <miiphy.h>
@@ -12,7 +14,7 @@
 #include <asm/mach-imx/gpio.h>
 #include <asm/mach-imx/iomux-v3.h>
 #include <linux/delay.h>
-
+#include <linux/kernel.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -30,6 +32,18 @@ static iomux_v3_cfg_t const uart_pads[] = {
 	IMX8MQ_PAD_ECSPI1_MISO__UART3_CTS_B | MUX_PAD_CTRL(UART_PAD_CTRL),
 };
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+		.image_type_id = KONTRON_PITX_IMX8M_FIT_IMAGE_GUID,
+		.fw_name = u"KONTRON-PITX-IMX8M-UBOOT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 int board_early_init_f(void)
 {
 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
index 48376cb826..834588af3a 100644
--- a/board/kontron/sl-mx8mm/sl-mx8mm.c
+++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
@@ -6,12 +6,27 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <fdt_support.h>
 #include <linux/errno.h>
+#include <linux/kernel.h>
 #include <net.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+		.image_type_id = KONTRON_SL_MX8MM_FIT_IMAGE_GUID,
+		.fw_name = u"KONTROL-SL-MX8MM-UBOOT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 int board_phys_sdram_size(phys_size_t *size)
 {
 	u32 ddr_size = readl(M4_BOOTROM_BASE_ADDR);
diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
index 3c48a9141d..7d3635da45 100644
--- a/board/kontron/sl28/sl28.c
+++ b/board/kontron/sl28/sl28.c
@@ -3,11 +3,14 @@
 #include <common.h>
 #include <dm.h>
 #include <malloc.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <errno.h>
 #include <fsl_ddr.h>
 #include <fdt_support.h>
 #include <asm/global_data.h>
 #include <linux/libfdt.h>
+#include <linux/kernel.h>
 #include <env_internal.h>
 #include <asm/arch-fsl-layerscape/soc.h>
 #include <asm/arch-fsl-layerscape/fsl_icid.h>
@@ -23,6 +26,18 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+		.image_type_id = KONTRON_SL28_FIT_IMAGE_GUID,
+		.fw_name = u"KONTRON-SL28-FIT",
+		.image_index = 1
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 int board_early_init_f(void)
 {
 	fsl_lsch3_early_init_f();
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 5d9a945d64..c5e6e3d2a0 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -7,6 +7,8 @@
 #include <cpu_func.h>
 #include <cros_ec.h>
 #include <dm.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <env_internal.h>
 #include <init.h>
 #include <led.h>
@@ -14,6 +16,7 @@
 #include <asm/global_data.h>
 #include <asm/test.h>
 #include <asm/u-boot-sandbox.h>
+#include <linux/kernel.h>
 #include <malloc.h>
 
 #include <extension_board.h>
@@ -25,6 +28,31 @@
  */
 gd_t *gd;
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)
+	{
+		.image_type_id = SANDBOX_UBOOT_IMAGE_GUID,
+		.fw_name = u"SANDBOX-UBOOT",
+		.image_index = 1
+	},
+	{
+		.image_type_id = SANDBOX_UBOOT_ENV_IMAGE_GUID,
+		.fw_name = u"SANDBOX-UBOOT-ENV",
+		.image_index = 2
+	},
+#elif defined(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)
+	{
+		.image_type_id = SANDBOX_FIT_IMAGE_GUID,
+		.fw_name = u"SANDBOX-FIT",
+		.image_index = 1
+	},
+#endif
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 /*
  * Add a simple GPIO device (don't use with of-platdata as it interferes with
diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
index 9552bfcdc3..ae4b2d6ed8 100644
--- a/board/socionext/developerbox/developerbox.c
+++ b/board/socionext/developerbox/developerbox.c
@@ -10,10 +10,36 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <common.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <env_internal.h>
 #include <fdt_support.h>
 #include <log.h>
 
+#include <linux/kernel.h>
+
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+	{
+		.image_type_id = DEVELOPERBOX_UBOOT_IMAGE_GUID,
+		.fw_name = u"DEVELOPERBOX-UBOOT",
+		.image_index = 1
+	},
+	{
+		.image_type_id = DEVELOPERBOX_FIP_IMAGE_GUID,
+		.fw_name = u"DEVELOPERBOX-FIP",
+		.image_index = 2
+	},
+	{
+		.image_type_id = DEVELOPERBOX_OPTEE_IMAGE_GUID,
+		.fw_name = u"DEVELOPERBOX-OPTEE",
+		.image_index = 3
+	},
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 static struct mm_region sc2a11_mem_map[] = {
 	{
 		.virt = 0x0UL,
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 0068cb8792..2e63bb4d4f 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -5,6 +5,8 @@
  */
 
 #include <common.h>
+#include <efi.h>
+#include <efi_loader.h>
 #include <env.h>
 #include <log.h>
 #include <asm/global_data.h>
@@ -20,9 +22,31 @@
 #include <generated/dt.h>
 #include <soc.h>
 #include <linux/ctype.h>
+#include <linux/kernel.h>
 
 #include "fru.h"
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+struct efi_fw_images fw_images[] = {
+#if defined(XILINX_BOOT_IMAGE_GUID)
+	{
+		.image_type_id = XILINX_BOOT_IMAGE_GUID,
+		.fw_name = u"XILINX-BOOT",
+		.image_index = 1
+	},
+#endif
+#if defined(XILINX_UBOOT_IMAGE_GUID)
+	{
+		.image_type_id = XILINX_UBOOT_IMAGE_GUID,
+		.fw_name = u"XILINX-UBOOT",
+		.image_index = 2
+	},
+#endif
+};
+
+u8 num_image_type_guids = ARRAY_SIZE(fw_images);
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #if defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET)
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
 {
diff --git a/include/configs/imx8mm-cl-iot-gate.h b/include/configs/imx8mm-cl-iot-gate.h
index 7e6be6050c..35df2e755e 100644
--- a/include/configs/imx8mm-cl-iot-gate.h
+++ b/include/configs/imx8mm-cl-iot-gate.h
@@ -31,6 +31,16 @@
 
 #endif
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define IMX8MM_CL_IOT_GATE_FIT_IMAGE_GUID \
+	EFI_GUID(0x7a32a939, 0xab92, 0x467b, 0x91, 0x52, \
+		 0x74, 0x77, 0x1b, 0x95, 0xe6, 0x46)
+
+#define IMX8MM_CL_IOT_GATE_OPTEE_FIT_IMAGE_GUID \
+	EFI_GUID(0x0bf1165c, 0x1831, 0x4864, 0x94, 0x5e, \
+		 0xac, 0x3d, 0x38, 0x48, 0xf4, 0x99)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #if CONFIG_IS_ENABLED(CMD_MMC)
 # define BOOT_TARGET_MMC(func) \
 	func(MMC, mmc, 2)      \
diff --git a/include/configs/imx8mp_rsb3720.h b/include/configs/imx8mp_rsb3720.h
index ac4a7d0cb3..a5a845c2da 100644
--- a/include/configs/imx8mp_rsb3720.h
+++ b/include/configs/imx8mp_rsb3720.h
@@ -21,6 +21,16 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
 #define CONFIG_SYS_UBOOT_BASE	(QSPI0_AMBA_BASE + CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define IMX8MP_RSB3720A1_4G_FIT_IMAGE_GUID \
+	EFI_GUID(0xb1251e89, 0x384a, 0x4635, 0xa8, 0x06, \
+		 0x3a, 0xa0, 0xb0, 0xe9, 0xf9, 0x65)
+
+#define IMX8MP_RSB3720A1_6G_FIT_IMAGE_GUID \
+	EFI_GUID(0xb5fb6f08, 0xe142, 0x4db1, 0x97, 0xea, \
+		 0x5f, 0xd3, 0x6b, 0x9b, 0xe5, 0xb9)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT*/
+
 #ifdef CONFIG_SPL_BUILD
 #define CONFIG_SPL_LDSCRIPT		"arch/arm/cpu/armv8/u-boot-spl.lds"
 #define CONFIG_SPL_STACK		0x960000
diff --git a/include/configs/kontron-sl-mx8mm.h b/include/configs/kontron-sl-mx8mm.h
index 788ae77cd3..aff1b90010 100644
--- a/include/configs/kontron-sl-mx8mm.h
+++ b/include/configs/kontron-sl-mx8mm.h
@@ -38,6 +38,12 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT	2
 #endif
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define KONTRON_SL_MX8MM_FIT_IMAGE_GUID \
+	EFI_GUID(0xd488e45a, 0x4929, 0x4b55, 0x8c, 0x14, \
+		 0x86, 0xce, 0xa2, 0xcd, 0x66, 0x29)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #ifndef CONFIG_SPL_BUILD
 #define BOOT_TARGET_DEVICES(func) \
 	func(MMC, mmc, 1) \
diff --git a/include/configs/kontron_pitx_imx8m.h b/include/configs/kontron_pitx_imx8m.h
index 0f96b905ab..678364e367 100644
--- a/include/configs/kontron_pitx_imx8m.h
+++ b/include/configs/kontron_pitx_imx8m.h
@@ -14,6 +14,12 @@
 #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
 #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define KONTRON_PITX_IMX8M_FIT_IMAGE_GUID \
+	EFI_GUID(0xc898e959, 0x5b1f, 0x4e6d, 0x88, 0xe0, \
+		 0x40, 0xd4, 0x5c, 0xca, 0x13, 0x99)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #ifdef CONFIG_SPL_BUILD
 #define CONFIG_SPL_LDSCRIPT		"arch/arm/cpu/armv8/u-boot-spl.lds"
 #define CONFIG_SPL_STACK		0x187FF0
diff --git a/include/configs/kontron_sl28.h b/include/configs/kontron_sl28.h
index 448749a7f8..97d0d365f6 100644
--- a/include/configs/kontron_sl28.h
+++ b/include/configs/kontron_sl28.h
@@ -57,6 +57,12 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80200000
 #define CONFIG_SYS_MONITOR_LEN		(1024 * 1024)
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define KONTRON_SL28_FIT_IMAGE_GUID \
+	EFI_GUID(0x86ebd44f, 0xfeb8, 0x466f, 0x8b, 0xb8, \
+		 0x89, 0x06, 0x18, 0x45, 0x6d, 0x8b)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* environment */
 /* see include/configs/ti_armv7_common.h */
 #define ENV_MEM_LAYOUT_SETTINGS \
diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index d45f606860..2f2abc746d 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -17,6 +17,16 @@
 
 #define CONFIG_SYS_BOOTM_LEN		SZ_64M
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define QEMU_ARM_UBOOT_IMAGE_GUID \
+	EFI_GUID(0xf885b085, 0x99f8, 0x45af, 0x84, 0x7d, \
+		 0xd5, 0x14, 0x10, 0x7a, 0x4a, 0x2c)
+
+#define QEMU_ARM64_UBOOT_IMAGE_GUID \
+	EFI_GUID(0x058b7d83, 0x50d5, 0x4c47, 0xa1, 0x95, \
+		 0x60, 0xd8, 0x6a, 0xd3, 0x41, 0xc4)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* For timer, QEMU emulates an ARMv7/ARMv8 architected timer */
 
 /* Environment options */
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 75efbf3448..e951d08056 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -14,6 +14,20 @@
 
 #define CONFIG_SYS_CBSIZE		1024	/* Console I/O Buffer Size */
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define SANDBOX_UBOOT_IMAGE_GUID \
+	EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \
+		 0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
+
+#define SANDBOX_UBOOT_ENV_IMAGE_GUID \
+	EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \
+		 0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0)
+
+#define SANDBOX_FIT_IMAGE_GUID \
+	EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \
+		 0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* Size of our emulated memory */
 #define SB_CONCAT(x, y) x ## y
 #define SB_TO_UL(s) SB_CONCAT(s, UL)
diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
index 8dd092fc59..07e1f56e3d 100644
--- a/include/configs/synquacer.h
+++ b/include/configs/synquacer.h
@@ -51,6 +51,20 @@
 			"fip.bin raw 180000 78000;"			\
 			"optee.bin raw 500000 100000\0"
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define DEVELOPERBOX_UBOOT_IMAGE_GUID \
+	EFI_GUID(0x53a92e83, 0x4ef4, 0x473a, 0x8b, 0x0d, \
+		 0xb5, 0xd8, 0xc7, 0xb2, 0xd6, 0x00)
+
+#define DEVELOPERBOX_FIP_IMAGE_GUID \
+	EFI_GUID(0x880866e9, 0x84ba, 0x4793, 0xa9, 0x08, \
+		 0x33, 0xe0, 0xb9, 0x16, 0xf3, 0x98)
+
+#define DEVELOPERBOX_OPTEE_IMAGE_GUID \
+	EFI_GUID(0xc1b629f1, 0xce0e, 0x4894, 0x82, 0xbf, \
+		 0xf0, 0xa3, 0x83, 0x87, 0xe6, 0x30)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* Distro boot settings */
 #ifndef CONFIG_SPL_BUILD
 #ifdef CONFIG_CMD_USB
diff --git a/include/configs/xilinx_versal.h b/include/configs/xilinx_versal.h
index bc72f5f35f..95218962f7 100644
--- a/include/configs/xilinx_versal.h
+++ b/include/configs/xilinx_versal.h
@@ -31,6 +31,12 @@
 #define CONFIG_BOOTP_BOOTFILESIZE
 #define CONFIG_BOOTP_MAY_FAIL
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define XILINX_BOOT_IMAGE_GUID \
+	EFI_GUID(0x20c5fba5, 0x0171, 0x457f, 0xb9, 0xcd, \
+		 0xf5, 0x12, 0x9c, 0xd0, 0x72, 0x28)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* Miscellaneous configurable options */
 
 /* Monitor Command Prompt */
diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
index e51d92ffe4..942f004b15 100644
--- a/include/configs/xilinx_zynqmp.h
+++ b/include/configs/xilinx_zynqmp.h
@@ -31,6 +31,16 @@
 #define CONFIG_BOOTP_BOOTFILESIZE
 #define CONFIG_BOOTP_MAY_FAIL
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define XILINX_BOOT_IMAGE_GUID \
+	EFI_GUID(0xde6066e8, 0x0256, 0x4fad, 0x82, 0x38, \
+		 0xe4, 0x06, 0xe2, 0x74, 0xc4, 0xcf)
+
+#define XILINX_UBOOT_IMAGE_GUID \
+	EFI_GUID(0xcf9ecfd4, 0x938b, 0x41c5, 0x85, 0x51, \
+		 0x1f, 0x88, 0x3a, 0xb7, 0xdc, 0x18)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 #ifdef CONFIG_NAND_ARASAN
 # define CONFIG_SYS_MAX_NAND_DEVICE	1
 #endif
diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
index 780952cf5f..d116947d25 100644
--- a/include/configs/zynq-common.h
+++ b/include/configs/zynq-common.h
@@ -20,6 +20,16 @@
 #define CONFIG_SYS_TIMER_COUNTS_DOWN
 #define CONFIG_SYS_TIMER_COUNTER	(CONFIG_SYS_TIMERBASE + 0x4)
 
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+#define XILINX_BOOT_IMAGE_GUID \
+	EFI_GUID(0x1ba29a15, 0x9969, 0x40aa, 0xb4, 0x24, \
+		 0xe8, 0x61, 0x21, 0x61, 0x86, 0x64)
+
+#define XILINX_UBOOT_IMAGE_GUID \
+	EFI_GUID(0x1a5178f0, 0x87d3, 0x4f36, 0xac, 0x63, \
+		 0x3b, 0x31, 0xa2, 0x3b, 0xe3, 0x05)
+#endif /* EFI_HAVE_CAPSULE_SUPPORT */
+
 /* Serial drivers */
 /* The following table includes the supported baudrates */
 #define CONFIG_SYS_BAUDRATE_TABLE  \
diff --git a/include/efi_loader.h b/include/efi_loader.h
index af36639ec6..284d64547b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -979,6 +979,24 @@ efi_status_t efi_capsule_authenticate(const void *capsule,
 
 #define EFI_CAPSULE_DIR u"\\EFI\\UpdateCapsule\\"
 
+/**
+ * struct efi_fw_images - List of firmware images updatable through capsule
+ *                        update
+ *
+ * This structure gives information about the firmware images on the platform
+ * which can be updated through the capsule update mechanism
+ *
+ * @image_type_id:	Image GUID. Same value is to be used in the capsule
+ * @fw_name:		Name of the firmware image
+ * @image_index:	Image Index, same as value passed to SetImage FMP
+ *                      function
+ */
+struct efi_fw_images {
+	efi_guid_t image_type_id;
+	const u16 *fw_name;
+	u8 image_index;
+};
+
 /**
  * Install the ESRT system table.
  *
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 15:08   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 3/8] capsule: Put a check for image index before the update Sughosh Ganu
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

Currently, the image descriptor array that has been passed to the
GetImageInfo function of the Firmware Management Protocol(FMP) gets
populated through the data stored with the dfu framework. The
dfu data is not restricted to contain information only of the images
updatable through the capsule update mechanism, but it also contains
information on other images.

The image descriptor array is also parsed by the ESRT generation code,
and thus the ESRT table contains entries for other images that are not
being handled by the FMP for the capsule updates.

The other issue fixed is assignment of a separate GUID for all images
in the image descriptor array. The UEFI specification mandates that
all entries in the ESRT table should have a unique GUID value as part
of the FwClass member of the EFI_SYSTEM_RESOURCE_ENTRY. Currently, all
images are assigned a single GUID value, either an FIT GUID or a raw
image GUID. This is fixed by obtaining the GUID values from the
efi_fw_images array defined per platform.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---

Changes since V3: None

 include/efi_loader.h          |  3 ++
 lib/efi_loader/efi_firmware.c | 91 +++++++++++------------------------
 2 files changed, 30 insertions(+), 64 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 284d64547b..9704397bd7 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -997,6 +997,9 @@ struct efi_fw_images {
 	u8 image_index;
 };
 
+extern struct efi_fw_images fw_images[];
+extern u8 num_image_type_guids;
+
 /**
  * Install the ESRT system table.
  *
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index a5ff32f121..169f3a29bb 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -97,91 +97,60 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
 }
 
 /**
- * efi_get_dfu_info - return information about the current firmware image
+ * efi_fill_image_desc_array - populate image descriptor array
  * @this:			Protocol instance
  * @image_info_size:		Size of @image_info
  * @image_info:			Image information
  * @descriptor_version:		Pointer to version number
- * @descriptor_count:		Pointer to number of descriptors
+ * @descriptor_count:		Image count
  * @descriptor_size:		Pointer to descriptor size
- * package_version:		Package version
- * package_version_name:	Package version's name
- * image_type:			Image type GUID
+ * @package_version:		Package version
+ * @package_version_name:	Package version's name
  *
  * Return information bout the current firmware image in @image_info.
  * @image_info will consist of a number of descriptors.
- * Each descriptor will be created based on "dfu_alt_info" variable.
+ * Each descriptor will be created based on "efi_fw_images" variable.
  *
  * Return		status code
  */
-static efi_status_t efi_get_dfu_info(
+static efi_status_t efi_fill_image_desc_array(
 	efi_uintn_t *image_info_size,
 	struct efi_firmware_image_descriptor *image_info,
 	u32 *descriptor_version,
 	u8 *descriptor_count,
 	efi_uintn_t *descriptor_size,
 	u32 *package_version,
-	u16 **package_version_name,
-	const efi_guid_t *image_type)
+	u16 **package_version_name)
 {
-	struct dfu_entity *dfu;
 	size_t names_len, total_size;
-	int dfu_num, i;
-	u16 *name, *next;
-	int ret;
-
-	ret = dfu_init_env_entities(NULL, NULL);
-	if (ret)
-		return EFI_SUCCESS;
+	struct efi_fw_images *fw_array;
+	u8 image_count;
+	int i;
 
+	fw_array = &fw_images[0];
+	*descriptor_count = image_count = num_image_type_guids;
 	names_len = 0;
-	dfu_num = 0;
-	list_for_each_entry(dfu, &dfu_list, list) {
-		names_len += (utf8_utf16_strlen(dfu->name) + 1) * 2;
-		dfu_num++;
-	}
-	if (!dfu_num) {
-		log_warning("No entities in dfu_alt_info\n");
-		*image_info_size = 0;
-		dfu_free_entities();
 
-		return EFI_SUCCESS;
-	}
+	total_size = sizeof(*image_info) * image_count;
 
-	total_size = sizeof(*image_info) * dfu_num + names_len;
-	/*
-	 * we will assume that sizeof(*image_info) * dfu_name
-	 * is, at least, a multiple of 2. So the start address for
-	 * image_id_name would be aligned with 2 bytes.
-	 */
 	if (*image_info_size < total_size) {
 		*image_info_size = total_size;
-		dfu_free_entities();
 
 		return EFI_BUFFER_TOO_SMALL;
 	}
 	*image_info_size = total_size;
 
 	*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
-	*descriptor_count = dfu_num;
 	*descriptor_size = sizeof(*image_info);
 	*package_version = 0xffffffff; /* not supported */
 	*package_version_name = NULL; /* not supported */
 
-	/* DFU alt number should correspond to image_index */
-	i = 0;
-	/* Name area starts just after descriptors */
-	name = (u16 *)((u8 *)image_info + sizeof(*image_info) * dfu_num);
-	next = name;
-	list_for_each_entry(dfu, &dfu_list, list) {
-		image_info[i].image_index = dfu->alt + 1;
-		image_info[i].image_type_id = *image_type;
-		image_info[i].image_id = dfu->alt;
-
-		/* copy the DFU entity name */
-		utf8_utf16_strcpy(&next, dfu->name);
-		image_info[i].image_id_name = name;
-		name = ++next;
+	for (i = 0; i < image_count; i++) {
+		image_info[i].image_index = fw_array[i].image_index;
+		image_info[i].image_type_id = fw_array[i].image_type_id;
+		image_info[i].image_id = fw_array[i].image_index;
+
+		image_info[i].image_id_name = (u16 *)fw_array[i].fw_name;
 
 		image_info[i].version = 0; /* not supported */
 		image_info[i].version_name = NULL; /* not supported */
@@ -202,12 +171,8 @@ static efi_status_t efi_get_dfu_info(
 		image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
 		image_info[i].hardware_instance = 1;
 		image_info[i].dependencies = NULL;
-
-		i++;
 	}
 
-	dfu_free_entities();
-
 	return EFI_SUCCESS;
 }
 
@@ -267,11 +232,10 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
 	     !descriptor_size || !package_version || !package_version_name))
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-	ret = efi_get_dfu_info(image_info_size, image_info,
-			       descriptor_version, descriptor_count,
-			       descriptor_size,
-			       package_version, package_version_name,
-			       &efi_firmware_image_type_uboot_fit);
+	ret = efi_fill_image_desc_array(image_info_size, image_info,
+					descriptor_version, descriptor_count,
+					descriptor_size, package_version,
+					package_version_name);
 
 	return EFI_EXIT(ret);
 }
@@ -376,11 +340,10 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
 	     !descriptor_size || !package_version || !package_version_name))
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-	ret = efi_get_dfu_info(image_info_size, image_info,
-			       descriptor_version, descriptor_count,
-			       descriptor_size,
-			       package_version, package_version_name,
-			       &efi_firmware_image_type_uboot_raw);
+	ret = efi_fill_image_desc_array(image_info_size, image_info,
+					descriptor_version, descriptor_count,
+					descriptor_size, package_version,
+					package_version_name);
 
 	return EFI_EXIT(ret);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 3/8] capsule: Put a check for image index before the update
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 18:47   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled Sughosh Ganu
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

The current capsule update code compares the image GUID value in the
capsule header with the image GUID value obtained from the
GetImageInfo function of the Firmware Management Protocol(FMP). This
comparison is done to ascertain if the FMP's SetImage function can be
called for the update. Make this checking more robust by comparing the
image_index value passed through the capsule with that returned by the
FMP's GetImageInfo function. This protects against the scenario of the
firmware being updated in a wrong partition/location on the storage
device if an incorrect value has been passed through the capsule,
since the image_index is used to determine the location of the update
on the storage device.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---

Changes since V3: None

 lib/efi_loader/efi_capsule.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index f00440163d..f03f4c9044 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -128,6 +128,7 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
 /**
  * efi_fmp_find - search for Firmware Management Protocol drivers
  * @image_type:		Image type guid
+ * @image_index:	Image Index
  * @instance:		Instance number
  * @handles:		Handles of FMP drivers
  * @no_handles:		Number of handles
@@ -141,8 +142,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
  * * NULL		- on failure
  */
 static struct efi_firmware_management_protocol *
-efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
-	     efi_uintn_t no_handles)
+efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance,
+	     efi_handle_t *handles, efi_uintn_t no_handles)
 {
 	efi_handle_t *handle;
 	struct efi_firmware_management_protocol *fmp;
@@ -203,6 +204,7 @@ efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
 			log_debug("+++ desc[%d] index: %d, name: %ls\n",
 				  j, desc->image_index, desc->image_id_name);
 			if (!guidcmp(&desc->image_type_id, image_type) &&
+			    (desc->image_index == image_index) &&
 			    (!instance ||
 			     !desc->hardware_instance ||
 			      desc->hardware_instance == instance))
@@ -449,8 +451,8 @@ static efi_status_t efi_capsule_update_firmware(
 		}
 
 		/* find a device for update firmware */
-		/* TODO: should we pass index as well, or nothing but type? */
 		fmp = efi_fmp_find(&image->update_image_type_id,
+				   image->update_image_index,
 				   image->update_hardware_instance,
 				   handles, no_handles);
 		if (!fmp) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
                   ` (2 preceding siblings ...)
  2022-03-31 13:27 ` [PATCH v4 3/8] capsule: Put a check for image index before the update Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 14:08   ` Masami Hiramatsu
  2022-03-31 19:35   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 5/8] test: capsule: Modify the capsule tests to use GUID values for sandbox Sughosh Ganu
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

Currently, there are a bunch of boards which enable the UEFI capsule
update feature. The actual update of the firmware images is done
through the dfu framework which uses the dfu_alt_info environment
variable for getting information on the update, like device, partition
number/address etc. Currently, these boards define the dfu_alt_info
variable in the board config header, as an environment variable. With
this, the variable can be modified from the u-boot command line and
this can cause an incorrect update.

To prevent this from happening, define the set_dfu_alt_info function
in the board file, and select SET_DFU_ALT_INFO for all platforms which
enable the capsule update feature. With the function defined, the dfu
framework populates the dfu_alt_info variable through the board file,
instead of fetching the variable from the environment, thus making the
update more robust.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3:
* Do not remove the existing dfu_alt_info definitions made by
  platforms in the config files, as discussed with Masami.
* Squash the selection of the SET_DFU_ALT_INFO config symbol for
  capsule update feature as part of this patch.


 .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 24 +++++++++++++++++
 .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 24 +++++++++++++++++
 board/emulation/common/qemu_dfu.c             |  6 ++---
 board/kontron/pitx_imx8m/pitx_imx8m.c         | 24 +++++++++++++++++
 board/kontron/sl-mx8mm/sl-mx8mm.c             | 24 +++++++++++++++++
 board/kontron/sl28/sl28.c                     | 25 ++++++++++++++++++
 board/sandbox/sandbox.c                       | 26 +++++++++++++++++++
 board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++++
 board/xilinx/zynq/board.c                     |  5 ++--
 board/xilinx/zynqmp/zynqmp.c                  |  5 ++--
 lib/efi_loader/Kconfig                        |  2 ++
 11 files changed, 184 insertions(+), 7 deletions(-)

diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
index 1c953ba195..41154ca9f3 100644
--- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
+++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
@@ -5,10 +5,12 @@
  */
 
 #include <common.h>
+#include <dfu.h>
 #include <dwc3-uboot.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <errno.h>
+#include <memalign.h>
 #include <miiphy.h>
 #include <netdev.h>
 #include <spl.h>
@@ -24,6 +26,7 @@
 #include <asm/mach-imx/dma.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <power/pmic.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
 	}
 }
 #endif /* CONFIG_SPL_MMC_SUPPORT */
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
index f5b89a5ddc..1880dd9c55 100644
--- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
+++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <dfu.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <env.h>
@@ -12,6 +13,7 @@
 #include <hang.h>
 #include <i2c.h>
 #include <init.h>
+#include <memalign.h>
 #include <miiphy.h>
 #include <netdev.h>
 
@@ -24,6 +26,7 @@
 #include <asm/mach-imx/mxc_i2c.h>
 #include <asm/sections.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 
 #include "ddr/ddr.h"
 
@@ -446,3 +449,24 @@ int board_late_init(void)
 
 	return 0;
 }
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/emulation/common/qemu_dfu.c b/board/emulation/common/qemu_dfu.c
index 62234a7647..85dff4373b 100644
--- a/board/emulation/common/qemu_dfu.c
+++ b/board/emulation/common/qemu_dfu.c
@@ -44,10 +44,11 @@ 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"))
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
 		return;
 
-	memset(buf, 0, sizeof(buf));
+	memset(buf, 0, DFU_ALT_BUF_LEN);
 
 	/*
 	 * Currently dfu_alt_info is needed on Qemu ARM64 for
@@ -64,5 +65,4 @@ void set_dfu_alt_info(char *interface, char *devstr)
 	}
 
 	env_set("dfu_alt_info", buf);
-	printf("dfu_alt_info set\n");
 }
diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
index 8dc04411cc..f3f6fbee9f 100644
--- a/board/kontron/pitx_imx8m/pitx_imx8m.c
+++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
@@ -2,9 +2,11 @@
 
 #include "pitx_misc.h"
 #include <common.h>
+#include <dfu.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <init.h>
+#include <memalign.h>
 #include <mmc.h>
 #include <miiphy.h>
 #include <asm/arch/clock.h>
@@ -15,6 +17,7 @@
 #include <asm/mach-imx/iomux-v3.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -181,3 +184,24 @@ int board_late_init(void)
 {
 	return 0;
 }
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
index 834588af3a..a00eb19828 100644
--- a/board/kontron/sl-mx8mm/sl-mx8mm.c
+++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
@@ -6,11 +6,14 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
+#include <dfu.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <fdt_support.h>
+#include <memalign.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <net.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -112,3 +115,24 @@ int board_init(void)
 {
 	return 0;
 }
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "sf 0:0=flash-bin raw 0x400 0x1f0000");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
index 7d3635da45..db41e2885c 100644
--- a/board/kontron/sl28/sl28.c
+++ b/board/kontron/sl28/sl28.c
@@ -2,7 +2,9 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dfu.h>
 #include <malloc.h>
+#include <memalign.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <errno.h>
@@ -11,6 +13,7 @@
 #include <asm/global_data.h>
 #include <linux/libfdt.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <env_internal.h>
 #include <asm/arch-fsl-layerscape/soc.h>
 #include <asm/arch-fsl-layerscape/fsl_icid.h>
@@ -150,3 +153,25 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 	return 0;
 }
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "sf 0:0=u-boot-bin raw 0x210000 0x1d0000;"
+		 "u-boot-env raw 0x3e0000 0x20000");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index c5e6e3d2a0..6a0453281d 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -6,17 +6,20 @@
 #include <common.h>
 #include <cpu_func.h>
 #include <cros_ec.h>
+#include <dfu.h>
 #include <dm.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <env_internal.h>
 #include <init.h>
 #include <led.h>
+#include <memalign.h>
 #include <os.h>
 #include <asm/global_data.h>
 #include <asm/test.h>
 #include <asm/u-boot-sandbox.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <malloc.h>
 
 #include <extension_board.h>
@@ -152,3 +155,26 @@ int board_late_init(void)
 	return 0;
 }
 #endif
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		 "sf 0:0=u-boot-bin raw 0x100000 0x50000;"
+		 "u-boot-env raw 0x150000 0x200000"
+		);
+
+	env_set("dfu_alt_info", buf);
+}
+#endif
diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
index ae4b2d6ed8..6784a3db53 100644
--- a/board/socionext/developerbox/developerbox.c
+++ b/board/socionext/developerbox/developerbox.c
@@ -10,13 +10,16 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <common.h>
+#include <dfu.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <env_internal.h>
 #include <fdt_support.h>
 #include <log.h>
+#include <memalign.h>
 
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 
 #if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
 struct efi_fw_images fw_images[] = {
@@ -185,3 +188,26 @@ int print_cpuinfo(void)
 	printf("CPU:   SC2A11:Cortex-A53 MPCore 24cores\n");
 	return 0;
 }
+
+#if defined(CONFIG_SET_DFU_ALT_INFO)
+
+#define DFU_ALT_BUF_LEN		SZ_1K
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
+		return;
+
+	memset(buf, 0, DFU_ALT_BUF_LEN);
+
+	snprintf(buf, DFU_ALT_BUF_LEN,
+		"mtd nor1=u-boot.bin raw 200000 100000;"
+		"fip.bin raw 180000 78000;"
+		"optee.bin raw 500000 100000");
+
+	env_set("dfu_alt_info", buf);
+}
+#endif /* CONFIG_SET_DFU_ALT_INFO */
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index 26ef048835..c70ad07269 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -168,10 +168,11 @@ 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"))
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
 		return;
 
-	memset(buf, 0, sizeof(buf));
+	memset(buf, 0, DFU_ALT_BUF_LEN);
 
 	switch ((zynq_slcr_get_boot_mode()) & ZYNQ_BM_MASK) {
 	case ZYNQ_BM_SD:
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 70b3c81f12..d278ed21a1 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -890,10 +890,11 @@ 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"))
+	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
+	    env_get("dfu_alt_info"))
 		return;
 
-	memset(buf, 0, sizeof(buf));
+	memset(buf, 0, DFU_ALT_BUF_LEN);
 
 	multiboot = multi_boot();
 	if (multiboot < 0)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e5e35fe51f..09fb8cbe75 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -174,6 +174,7 @@ 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
@@ -185,6 +186,7 @@ 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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 5/8] test: capsule: Modify the capsule tests to use GUID values for sandbox
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
                   ` (3 preceding siblings ...)
  2022-03-31 13:27 ` [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images Sughosh Ganu
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

The current UEFI capsule updation code uses two GUID values, one for
FIT images, and one for raw images across platforms. This logic is
being changed to have GUID values per image, per platform. Change the
tests for the capsule update code to reflect this change. The GUID
values now used are the ones specific to the sandbox platform -- one
for the u-boot image, and another for the u-boot environment image.

The UEFI specification does not allow installation of multiple
Firmware Management Protocols(FMP) at the same time. Install the
FMP instance for raw images on the sandbox variant for testing the
capsule update code. Install the FMP instance for the FIT images on
the sandbox64 and sandbox_flattree variant for testing capsule update
for FIT images. This is being done by splitting the capsule update
script for FIT and raw images.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3: None

 configs/sandbox64_defconfig                   |   1 -
 configs/sandbox_defconfig                     |   1 -
 configs/sandbox_flattree_defconfig            |   5 +
 test/py/tests/test_efi_capsule/conftest.py    |  21 +-
 .../test_capsule_firmware_fit.py              | 186 ++++++++++++++++++
 ...rmware.py => test_capsule_firmware_raw.py} | 159 +++++----------
 6 files changed, 258 insertions(+), 115 deletions(-)
 create mode 100644 test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
 rename test/py/tests/test_efi_capsule/{test_capsule_firmware.py => test_capsule_firmware_raw.py} (75%)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 7c157a23d0..1a0142795a 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -247,7 +247,6 @@ CONFIG_ERRNO_STR=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index ab0e2defee..de2526df09 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -318,7 +318,6 @@ CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 217b0647bb..bbcf435ac6 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -29,6 +29,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ERASEENV=y
+CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_CMD_NVEDIT_LOAD=y
 CONFIG_CMD_NVEDIT_SELECT=y
@@ -210,3 +211,7 @@ CONFIG_HEXDUMP=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_DFU_SF=y
diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
index 9076087a12..d757415c88 100644
--- a/test/py/tests/test_efi_capsule/conftest.py
+++ b/test/py/tests/test_efi_capsule/conftest.py
@@ -72,7 +72,7 @@ def efi_capsule_data(request, u_boot_config):
 
         # Create capsule files
         # two regions: one for u-boot.bin and the other for u-boot.env
-        check_call('cd %s; echo -n u-boot:Old > u-boot.bin.old; echo -n u-boot:New > u-boot.bin.new; echo -n u-boot-env:Old -> u-boot.env.old; echo -n u-boot-env:New > u-boot.env.new' % data_dir,
+        check_call('cd %s; echo -n u-boot:Old > u-boot.bin.old; echo -n u-boot:New > u-boot.bin.new; echo -n u-boot-env:Old > u-boot.env.old; echo -n u-boot-env:New > u-boot.env.new' % data_dir,
                    shell=True)
         check_call('sed -e \"s?BINFILE1?u-boot.bin.new?\" -e \"s?BINFILE2?u-boot.env.new?\" %s/test/py/tests/test_efi_capsule/uboot_bin_env.its > %s/uboot_bin_env.its' %
                    (u_boot_config.source_dir, data_dir),
@@ -80,21 +80,29 @@ def efi_capsule_data(request, u_boot_config):
         check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
-        check_call('cd %s; %s/tools/mkeficapsule --index 1 --fit uboot_bin_env.itb Test01' %
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test01' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
-        check_call('cd %s; %s/tools/mkeficapsule --index 1 --raw u-boot.bin.new Test02' %
+        check_call('cd %s; %s/tools/mkeficapsule --index 2 --guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test02' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
-        check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid E2BB9C06-70E9-4B14-97A3-5A7913176E3F u-boot.bin.new Test03' %
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 058B7D83-50D5-4C47-A195-60D86AD341C4 u-boot.bin.new Test03' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test04' %
+                   (data_dir, u_boot_config.build_dir),
+                   shell=True)
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid  058B7D83-50D5-4C47-A195-60D86AD341C4 uboot_bin_env.itb Test05' %
+                   (data_dir, u_boot_config.build_dir),
+                   shell=True)
+
         if capsule_auth_enabled:
             # firmware signed with proper key
             check_call('cd %s; '
                        '%s/tools/mkeficapsule --index 1 --monotonic-count 1 '
                             '--private-key SIGNER.key --certificate SIGNER.crt '
-                            '--raw u-boot.bin.new Test11'
+                            '--guid 09D7DF52-0720-4710-91D1-08469B7FE9C8 '
+                            'u-boot.bin.new Test11'
                        % (data_dir, u_boot_config.build_dir),
                        shell=True)
             # firmware signed with *mal* key
@@ -102,7 +110,8 @@ def efi_capsule_data(request, u_boot_config):
                        '%s/tools/mkeficapsule --index 1 --monotonic-count 1 '
                             '--private-key SIGNER2.key '
                             '--certificate SIGNER2.crt '
-                            '--raw u-boot.bin.new Test12'
+                            '--guid 09D7DF52-0720-4710-91D1-08469B7FE9C8 '
+                            'u-boot.bin.new Test12'
                        % (data_dir, u_boot_config.build_dir),
                        shell=True)
 
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
new file mode 100644
index 0000000000..2c94df6c5c
--- /dev/null
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier:      GPL-2.0+
+# Copyright (c) 2020, Linaro Limited
+# Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
+#
+# U-Boot UEFI: Firmware Update Test
+
+"""
+This test verifies capsule-on-disk firmware update for FIT images
+"""
+
+from subprocess import check_call, check_output, CalledProcessError
+import pytest
+from capsule_defs import *
+
+
+@pytest.mark.boardspec('sandbox64')
+@pytest.mark.boardspec('sandbox_flattree')
+@pytest.mark.buildconfigspec('efi_capsule_firmware_fit')
+@pytest.mark.buildconfigspec('efi_capsule_on_disk')
+@pytest.mark.buildconfigspec('dfu')
+@pytest.mark.buildconfigspec('dfu_sf')
+@pytest.mark.buildconfigspec('cmd_efidebug')
+@pytest.mark.buildconfigspec('cmd_fat')
+@pytest.mark.buildconfigspec('cmd_memory')
+@pytest.mark.buildconfigspec('cmd_nvedit_efi')
+@pytest.mark.buildconfigspec('cmd_sf')
+@pytest.mark.slow
+class TestEfiCapsuleFirmwareFit(object):
+    def test_efi_capsule_fw1(
+            self, u_boot_config, u_boot_console, efi_capsule_data):
+        """
+        Test Case 1 - Update U-Boot and U-Boot environment on SPI Flash
+                      but with an incorrect GUID value in the capsule
+                      No update should happen
+                      0x100000-0x150000: U-Boot binary (but dummy)
+                      0x150000-0x200000: U-Boot environment (but dummy)
+        """
+        disk_img = efi_capsule_data
+        with u_boot_console.log.section('Test Case 1-a, before reboot'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % disk_img,
+                'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi -s ""',
+                'efidebug boot order 1',
+                'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
+                'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
+                'env save'])
+
+            # initialize contents
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'fatload host 0:1 4000000 %s/u-boot.bin.old' % CAPSULE_DATA_DIR,
+                'sf write 4000000 100000 10',
+                'sf read 5000000 100000 10',
+                'md.b 5000000 10'])
+            assert 'Old' in ''.join(output)
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'fatload host 0:1 4000000 %s/u-boot.env.old' % CAPSULE_DATA_DIR,
+                'sf write 4000000 150000 10',
+                'sf read 5000000 150000 10',
+                'md.b 5000000 10'])
+            assert 'Old' in ''.join(output)
+
+            # place a capsule file
+            output = u_boot_console.run_command_list([
+                'fatload host 0:1 4000000 %s/Test05' % CAPSULE_DATA_DIR,
+                'fatwrite host 0:1 4000000 %s/Test05 $filesize' % CAPSULE_INSTALL_DIR,
+                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test05' in ''.join(output)
+
+        capsule_early = u_boot_config.buildconfig.get(
+            'config_efi_capsule_on_disk_early')
+        capsule_auth = u_boot_config.buildconfig.get(
+            'config_efi_capsule_authenticate')
+
+        # reboot
+        u_boot_console.restart_uboot(expect_reset = capsule_early)
+
+        with u_boot_console.log.section('Test Case 1-b, after reboot'):
+            if not capsule_early:
+                # make sure that dfu_alt_info exists even persistent variables
+                # are not available.
+                output = u_boot_console.run_command_list([
+                    'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
+                    'host bind 0 %s' % disk_img,
+                    'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+                assert 'Test05' in ''.join(output)
+
+                # need to run uefi command to initiate capsule handling
+                output = u_boot_console.run_command(
+                    'env print -e Capsule0000', wait_for_reboot = True)
+
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'sf read 4000000 100000 10',
+                'md.b 4000000 10'])
+            assert 'u-boot:Old' in ''.join(output)
+
+            output = u_boot_console.run_command_list([
+                'sf read 4000000 150000 10',
+                'md.b 4000000 10'])
+            assert 'u-boot-env:Old' in ''.join(output)
+
+    def test_efi_capsule_fw2(
+            self, u_boot_config, u_boot_console, efi_capsule_data):
+        """
+        Test Case 2 - Update U-Boot and U-Boot environment on SPI Flash
+                      0x100000-0x150000: U-Boot binary (but dummy)
+                      0x150000-0x200000: U-Boot environment (but dummy)
+        """
+        disk_img = efi_capsule_data
+        with u_boot_console.log.section('Test Case 2-a, before reboot'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % disk_img,
+                'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi -s ""',
+                'efidebug boot order 1',
+                'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
+                'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
+                'env save'])
+
+            # initialize contents
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'fatload host 0:1 4000000 %s/u-boot.bin.old' % CAPSULE_DATA_DIR,
+                'sf write 4000000 100000 10',
+                'sf read 5000000 100000 10',
+                'md.b 5000000 10'])
+            assert 'Old' in ''.join(output)
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'fatload host 0:1 4000000 %s/u-boot.env.old' % CAPSULE_DATA_DIR,
+                'sf write 4000000 150000 10',
+                'sf read 5000000 150000 10',
+                'md.b 5000000 10'])
+            assert 'Old' in ''.join(output)
+
+            # place a capsule file
+            output = u_boot_console.run_command_list([
+                'fatload host 0:1 4000000 %s/Test04' % CAPSULE_DATA_DIR,
+                'fatwrite host 0:1 4000000 %s/Test04 $filesize' % CAPSULE_INSTALL_DIR,
+                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test04' in ''.join(output)
+
+        capsule_early = u_boot_config.buildconfig.get(
+            'config_efi_capsule_on_disk_early')
+        capsule_auth = u_boot_config.buildconfig.get(
+            'config_efi_capsule_authenticate')
+
+        # reboot
+        u_boot_console.restart_uboot(expect_reset = capsule_early)
+
+        with u_boot_console.log.section('Test Case 2-b, after reboot'):
+            if not capsule_early:
+                # make sure that dfu_alt_info exists even persistent variables
+                # are not available.
+                output = u_boot_console.run_command_list([
+                    'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
+                    'host bind 0 %s' % disk_img,
+                    'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+                assert 'Test04' in ''.join(output)
+
+                # need to run uefi command to initiate capsule handling
+                output = u_boot_console.run_command(
+                    'env print -e Capsule0000', wait_for_reboot = True)
+
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % disk_img,
+                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test04' not in ''.join(output)
+
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'sf read 4000000 100000 10',
+                'md.b 4000000 10'])
+            if capsule_auth:
+                assert 'u-boot:Old' in ''.join(output)
+            else:
+                assert 'u-boot:New' in ''.join(output)
+
+            output = u_boot_console.run_command_list([
+                'sf read 4000000 150000 10',
+                'md.b 4000000 10'])
+            if capsule_auth:
+                assert 'u-boot-env:Old' in ''.join(output)
+            else:
+                assert 'u-boot-env:New' in ''.join(output)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
similarity index 75%
rename from test/py/tests/test_efi_capsule/test_capsule_firmware.py
rename to test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
index 1dcf1c70f4..c9eaf5e6d8 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
@@ -5,7 +5,7 @@
 # U-Boot UEFI: Firmware Update Test
 
 """
-This test verifies capsule-on-disk firmware update
+This test verifies capsule-on-disk firmware update for raw images
 """
 
 from subprocess import check_call, check_output, CalledProcessError
@@ -14,7 +14,6 @@ from capsule_defs import *
 
 
 @pytest.mark.boardspec('sandbox')
-@pytest.mark.buildconfigspec('efi_capsule_firmware_fit')
 @pytest.mark.buildconfigspec('efi_capsule_firmware_raw')
 @pytest.mark.buildconfigspec('efi_capsule_on_disk')
 @pytest.mark.buildconfigspec('dfu')
@@ -25,12 +24,12 @@ from capsule_defs import *
 @pytest.mark.buildconfigspec('cmd_nvedit_efi')
 @pytest.mark.buildconfigspec('cmd_sf')
 @pytest.mark.slow
-class TestEfiCapsuleFirmwareFit(object):
+class TestEfiCapsuleFirmwareRaw(object):
     def test_efi_capsule_fw1(
             self, u_boot_config, u_boot_console, efi_capsule_data):
         """
         Test Case 1 - Update U-Boot and U-Boot environment on SPI Flash
-                      but with OsIndications unset
+                      but with an incorrect GUID value in the capsule
                       No update should happen
                       0x100000-0x150000: U-Boot binary (but dummy)
                       0x150000-0x200000: U-Boot environment (but dummy)
@@ -41,7 +40,7 @@ class TestEfiCapsuleFirmwareFit(object):
                 'host bind 0 %s' % disk_img,
                 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi -s ""',
                 'efidebug boot order 1',
-                'env set -e OsIndications',
+                'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
                 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                 'env save'])
 
@@ -63,16 +62,17 @@ class TestEfiCapsuleFirmwareFit(object):
 
             # place a capsule file
             output = u_boot_console.run_command_list([
-                'fatload host 0:1 4000000 %s/Test01' % CAPSULE_DATA_DIR,
-                'fatwrite host 0:1 4000000 %s/Test01 $filesize' % CAPSULE_INSTALL_DIR,
+                'fatload host 0:1 4000000 %s/Test03' % CAPSULE_DATA_DIR,
+                'fatwrite host 0:1 4000000 %s/Test03 $filesize' % CAPSULE_INSTALL_DIR,
                 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-            assert 'Test01' in ''.join(output)
+            assert 'Test03' in ''.join(output)
 
         # reboot
         u_boot_console.restart_uboot()
 
         capsule_early = u_boot_config.buildconfig.get(
             'config_efi_capsule_on_disk_early')
+
         with u_boot_console.log.section('Test Case 1-b, after reboot'):
             if not capsule_early:
                 # make sure that dfu_alt_info exists even persistent variables
@@ -81,17 +81,12 @@ class TestEfiCapsuleFirmwareFit(object):
                     'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                     'host bind 0 %s' % disk_img,
                     'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-                assert 'Test01' in ''.join(output)
+                assert 'Test03' in ''.join(output)
 
                 # need to run uefi command to initiate capsule handling
                 output = u_boot_console.run_command(
                     'env print -e Capsule0000')
 
-            output = u_boot_console.run_command_list([
-                'host bind 0 %s' % disk_img,
-                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-            assert 'Test01' in ''.join(output)
-
             output = u_boot_console.run_command_list([
                 'sf probe 0:0',
                 'sf read 4000000 100000 10',
@@ -107,6 +102,8 @@ class TestEfiCapsuleFirmwareFit(object):
             self, u_boot_config, u_boot_console, efi_capsule_data):
         """
         Test Case 2 - Update U-Boot and U-Boot environment on SPI Flash
+                      but with OsIndications unset
+                      No update should happen
                       0x100000-0x150000: U-Boot binary (but dummy)
                       0x150000-0x200000: U-Boot environment (but dummy)
         """
@@ -116,7 +113,7 @@ class TestEfiCapsuleFirmwareFit(object):
                 'host bind 0 %s' % disk_img,
                 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi -s ""',
                 'efidebug boot order 1',
-                'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
+                'env set -e OsIndications',
                 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                 'env save'])
 
@@ -136,21 +133,24 @@ class TestEfiCapsuleFirmwareFit(object):
                 'md.b 5000000 10'])
             assert 'Old' in ''.join(output)
 
-            # place a capsule file
+            # place the capsule files
             output = u_boot_console.run_command_list([
                 'fatload host 0:1 4000000 %s/Test01' % CAPSULE_DATA_DIR,
                 'fatwrite host 0:1 4000000 %s/Test01 $filesize' % CAPSULE_INSTALL_DIR,
                 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
             assert 'Test01' in ''.join(output)
 
-        capsule_early = u_boot_config.buildconfig.get(
-            'config_efi_capsule_on_disk_early')
-        capsule_auth = u_boot_config.buildconfig.get(
-            'config_efi_capsule_authenticate')
+            output = u_boot_console.run_command_list([
+                'fatload host 0:1 4000000 %s/Test02' % CAPSULE_DATA_DIR,
+                'fatwrite host 0:1 4000000 %s/Test02 $filesize' % CAPSULE_INSTALL_DIR,
+                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test02' in ''.join(output)
 
         # reboot
-        u_boot_console.restart_uboot(expect_reset = capsule_early)
+        u_boot_console.restart_uboot()
 
+        capsule_early = u_boot_config.buildconfig.get(
+            'config_efi_capsule_on_disk_early')
         with u_boot_console.log.section('Test Case 2-b, after reboot'):
             if not capsule_early:
                 # make sure that dfu_alt_info exists even persistent variables
@@ -160,32 +160,28 @@ class TestEfiCapsuleFirmwareFit(object):
                     'host bind 0 %s' % disk_img,
                     'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
                 assert 'Test01' in ''.join(output)
+                assert 'Test02' in ''.join(output)
 
                 # need to run uefi command to initiate capsule handling
                 output = u_boot_console.run_command(
-                    'env print -e Capsule0000', wait_for_reboot = True)
+                    'env print -e Capsule0000')
 
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
                 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-            assert 'Test01' not in ''.join(output)
+            assert 'Test01' in ''.join(output)
+            assert 'Test02' in ''.join(output)
 
             output = u_boot_console.run_command_list([
                 'sf probe 0:0',
                 'sf read 4000000 100000 10',
                 'md.b 4000000 10'])
-            if capsule_auth:
-                assert 'u-boot:Old' in ''.join(output)
-            else:
-                assert 'u-boot:New' in ''.join(output)
+            assert 'u-boot:Old' in ''.join(output)
 
             output = u_boot_console.run_command_list([
                 'sf read 4000000 150000 10',
                 'md.b 4000000 10'])
-            if capsule_auth:
-                assert 'u-boot-env:Old' in ''.join(output)
-            else:
-                assert 'u-boot-env:New' in ''.join(output)
+            assert 'u-boot-env:Old' in ''.join(output)
 
     def test_efi_capsule_fw3(
             self, u_boot_config, u_boot_console, efi_capsule_data):
@@ -203,7 +199,7 @@ class TestEfiCapsuleFirmwareFit(object):
                 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                 'env save'])
 
-            # initialize content
+            # initialize contents
             output = u_boot_console.run_command_list([
                 'sf probe 0:0',
                 'fatload host 0:1 4000000 %s/u-boot.bin.old' % CAPSULE_DATA_DIR,
@@ -212,7 +208,21 @@ class TestEfiCapsuleFirmwareFit(object):
                 'md.b 5000000 10'])
             assert 'Old' in ''.join(output)
 
-            # place a capsule file
+            output = u_boot_console.run_command_list([
+                'sf probe 0:0',
+                'fatload host 0:1 4000000 %s/u-boot.env.old' % CAPSULE_DATA_DIR,
+                'sf write 4000000 150000 10',
+                'sf read 5000000 100000 10',
+                'md.b 5000000 10'])
+            assert 'Old' in ''.join(output)
+
+            # place the capsule files
+            output = u_boot_console.run_command_list([
+                'fatload host 0:1 4000000 %s/Test01' % CAPSULE_DATA_DIR,
+                'fatwrite host 0:1 4000000 %s/Test01 $filesize' % CAPSULE_INSTALL_DIR,
+                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test01' in ''.join(output)
+
             output = u_boot_console.run_command_list([
                 'fatload host 0:1 4000000 %s/Test02' % CAPSULE_DATA_DIR,
                 'fatwrite host 0:1 4000000 %s/Test02 $filesize' % CAPSULE_INSTALL_DIR,
@@ -235,6 +245,7 @@ class TestEfiCapsuleFirmwareFit(object):
                     'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                     'host bind 0 %s' % disk_img,
                     'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+                assert 'Test01' in ''.join(output)
                 assert 'Test02' in ''.join(output)
 
                 # need to run uefi command to initiate capsule handling
@@ -246,15 +257,16 @@ class TestEfiCapsuleFirmwareFit(object):
                 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
                 'efidebug capsule esrt'])
 
-            # ensure that EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID is in the ESRT.
-            assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in ''.join(output)
+            # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
+            assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
 
-            # ensure that  EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID is in the ESRT.
-            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output)
+            # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
+            assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
 
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
                 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
+            assert 'Test01' not in ''.join(output)
             assert 'Test02' not in ''.join(output)
 
             output = u_boot_console.run_command_list([
@@ -266,78 +278,11 @@ class TestEfiCapsuleFirmwareFit(object):
             else:
                 assert 'u-boot:New' in ''.join(output)
 
-    def test_efi_capsule_fw4(
-            self, u_boot_config, u_boot_console, efi_capsule_data):
-        """
-        Test Case 4 - Test "--guid" option of mkeficapsule
-                      The test scenario is the same as Case 3.
-        """
-        disk_img = efi_capsule_data
-        with u_boot_console.log.section('Test Case 4-a, before reboot'):
-            output = u_boot_console.run_command_list([
-                'host bind 0 %s' % disk_img,
-                'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi -s ""',
-                'efidebug boot order 1',
-                'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
-                'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
-                'env save'])
-
-            # initialize content
-            output = u_boot_console.run_command_list([
-                'sf probe 0:0',
-                'fatload host 0:1 4000000 %s/u-boot.bin.old' % CAPSULE_DATA_DIR,
-                'sf write 4000000 100000 10',
-                'sf read 5000000 100000 10',
-                'md.b 5000000 10'])
-            assert 'Old' in ''.join(output)
-
-            # place a capsule file
-            output = u_boot_console.run_command_list([
-                'fatload host 0:1 4000000 %s/Test03' % CAPSULE_DATA_DIR,
-                'fatwrite host 0:1 4000000 %s/Test03 $filesize' % CAPSULE_INSTALL_DIR,
-                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-            assert 'Test03' in ''.join(output)
-
-        capsule_early = u_boot_config.buildconfig.get(
-            'config_efi_capsule_on_disk_early')
-        capsule_auth = u_boot_config.buildconfig.get(
-            'config_efi_capsule_authenticate')
-
-        # reboot
-        u_boot_console.restart_uboot(expect_reset = capsule_early)
-
-        with u_boot_console.log.section('Test Case 4-b, after reboot'):
-            if not capsule_early:
-                # make sure that dfu_alt_info exists even persistent variables
-                # are not available.
-                output = u_boot_console.run_command_list([
-                    'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
-                    'host bind 0 %s' % disk_img,
-                    'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-                assert 'Test03' in ''.join(output)
-
-                # need to run uefi command to initiate capsule handling
-                output = u_boot_console.run_command(
-                    'env print -e Capsule0000', wait_for_reboot = True)
-
-            # make sure the dfu_alt_info exists because it is required for making ESRT.
-            output = u_boot_console.run_command_list([
-                'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
-                'efidebug capsule esrt'])
-
-            # ensure that  EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID is in the ESRT.
-            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output)
-
-            output = u_boot_console.run_command_list([
-                'host bind 0 %s' % disk_img,
-                'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-            assert 'Test03' not in ''.join(output)
-
             output = u_boot_console.run_command_list([
                 'sf probe 0:0',
-                'sf read 4000000 100000 10',
+                'sf read 4000000 150000 10',
                 'md.b 4000000 10'])
             if capsule_auth:
-                assert 'u-boot:Old' in ''.join(output)
+                assert 'u-boot-env:Old' in ''.join(output)
             else:
-                assert 'u-boot:New' in ''.join(output)
+                assert 'u-boot-env:New' in ''.join(output)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
                   ` (4 preceding siblings ...)
  2022-03-31 13:27 ` [PATCH v4 5/8] test: capsule: Modify the capsule tests to use GUID values for sandbox Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 18:10   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types Sughosh Ganu
  2022-03-31 13:27 ` [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation Sughosh Ganu
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

The capsule update code has been modified for getting the image GUID
values from the platform code. With this, each image now has a unique
GUID value. With this change, there is no longer a need for defining
GUIDs for FIT and raw images. Remove these GUID values.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3: None

 include/efi_api.h             | 8 --------
 lib/efi_loader/efi_firmware.c | 4 ----
 2 files changed, 12 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 982c200172..c7f7873b5d 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1967,14 +1967,6 @@ struct efi_signature_list {
 	EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \
 		 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)
 
-#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
-	EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
-		 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
-
-#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
-	EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
-		 0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
-
 #define IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x0000000000000001
 #define IMAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002
 #define IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 169f3a29bb..7734d9f353 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -185,8 +185,6 @@ static efi_status_t efi_fill_image_desc_array(
  *   - versioning of firmware image
  *   - package information
  */
-const efi_guid_t efi_firmware_image_type_uboot_fit =
-	EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
 
 /**
  * efi_firmware_fit_get_image_info - return information about the current
@@ -293,8 +291,6 @@ const struct efi_firmware_management_protocol efi_fmp_fit = {
  * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update
  * method with raw data.
  */
-const efi_guid_t efi_firmware_image_type_uboot_raw =
-	EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
 
 /**
  * efi_firmware_raw_get_image_info - return information about the current
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
                   ` (5 preceding siblings ...)
  2022-03-31 13:27 ` [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 18:10   ` Ilias Apalodimas
  2022-03-31 13:27 ` [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation Sughosh Ganu
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

While building a capsule, the GUID value of that specific image is to
be passed through the --guid command option to the mkeficapsule
tool. This renders the EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID and
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID values superfluous. Remove the
--raw and --fit command line options as well.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3: None

 tools/eficapsule.h   |  8 --------
 tools/mkeficapsule.c | 26 +-------------------------
 2 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 69c9c58c2f..d63b831443 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -37,14 +37,6 @@ typedef struct {
 	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
 		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
 
-#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
-	EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
-		 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
-
-#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
-	EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
-		 0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
-
 #define EFI_CERT_TYPE_PKCS7_GUID \
 	EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \
 		 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index c118335b93..5f74d23b9e 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -27,17 +27,11 @@
 static const char *tool_name = "mkeficapsule";
 
 efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
-efi_guid_t efi_guid_image_type_uboot_fit =
-		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
-efi_guid_t efi_guid_image_type_uboot_raw =
-		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
 efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
 
-static const char *opts_short = "frg:i:I:v:p:c:m:dh";
+static const char *opts_short = "g:i:I:v:p:c:m:dh";
 
 static struct option options[] = {
-	{"fit", no_argument, NULL, 'f'},
-	{"raw", no_argument, NULL, 'r'},
 	{"guid", required_argument, NULL, 'g'},
 	{"index", required_argument, NULL, 'i'},
 	{"instance", required_argument, NULL, 'I'},
@@ -54,8 +48,6 @@ static void print_usage(void)
 	fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
 		"Options:\n"
 
-		"\t-f, --fit                   FIT image type\n"
-		"\t-r, --raw                   raw image type\n"
 		"\t-g, --guid <guid string>    guid for image blob type\n"
 		"\t-i, --index <index>         update image index\n"
 		"\t-I, --instance <instance>   update hardware instance\n"
@@ -606,22 +598,6 @@ int main(int argc, char **argv)
 			break;
 
 		switch (c) {
-		case 'f':
-			if (guid) {
-				fprintf(stderr,
-					"Image type already specified\n");
-				exit(EXIT_FAILURE);
-			}
-			guid = &efi_guid_image_type_uboot_fit;
-			break;
-		case 'r':
-			if (guid) {
-				fprintf(stderr,
-					"Image type already specified\n");
-				exit(EXIT_FAILURE);
-			}
-			guid = &efi_guid_image_type_uboot_raw;
-			break;
 		case 'g':
 			if (guid) {
 				fprintf(stderr,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation
  2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
                   ` (6 preceding siblings ...)
  2022-03-31 13:27 ` [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types Sughosh Ganu
@ 2022-03-31 13:27 ` Sughosh Ganu
  2022-03-31 18:14   ` Ilias Apalodimas
  7 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 13:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Masami Hiramatsu, Jassi Brar, Michal Simek,
	Michal Simek, Sughosh Ganu

Update the capsule update functionality related documentation to
refect the additional definitions that need to be made per platform
for supporting the capsule update feature.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3:
* Rephrase the commit message to indicate that the doc changes are not
  just limited to adding the GUID values, but other info as well.
* Elaborate with an example on the relation between the dfu alt number
  and the image index 

 doc/develop/uefi/uefi.rst | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index fe337c88bd..d886635cc3 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -312,8 +312,8 @@ Run the following command
 .. code-block:: console
 
     $ mkeficapsule \
-      --index 1 --instance 0 \
-      [--fit <FIT image> | --raw <raw image>] \
+      --index <index> --instance 0 \
+      --guid <image GUID> \
       <capsule_file_name>
 
 Performing the update
@@ -333,6 +333,26 @@ won't be taken over across the reboot. If this is the case, you can skip
 this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS)
 set.
 
+A few things need to be defined in the board file for performing the
+capsule upadte. The first is defining the function set_dfu_alt_info in
+the board file. This function sets the environment variable
+dfu_alt_info. Instead of taking the variable from the environment, the
+capsule update feature requires that the variable be set through the
+board function, since that is more robust. Secondly, define GUID
+values and image index of the images that are to be updated through
+the capsule update feature in the board file. Both the values are to
+be defined as part of the fw_images array. These GUID values would be
+used by the Firmware Management Protocol(FMP) to populate the image
+descriptor array and also displayed as part of the ESRT table. The
+image index values defined in the array should be one greater than the
+dfu alt number that corresponds to the firmware image. So, if the dfu
+alt number for an image is 2, the value of image index in the
+fw_images array for that image should be 3. The dfu alt number can be
+obtained by running the following command::
+
+    dfu list
+
+
 Finally, the capsule update can be initiated by rebooting the board.
 
 Enabling Capsule Authentication
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates
  2022-03-31 13:27 ` [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates Sughosh Ganu
@ 2022-03-31 14:03   ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 14:03 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

On Thu, Mar 31, 2022 at 06:57:43PM +0530, Sughosh Ganu wrote:
> Currently, all platforms that enable capsule updates do so using
> either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID or
> EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID. This is based on the Firmware
> Management Protocol(FMP) instance used on the platform. However, this
> means that all platforms that enable a particular FMP instance have
> the same GUID value for all the updatable images, either the FIT image
> GUID or the raw image GUID, and that an image for some platform can be
> updated on any other platform which uses the same FMP instance. Another
> issue with this implementation is that the ESRT table shows the same
> GUID value for all images on the platform and also across platforms,
> which is not in compliance with the UEFI specification.
> 
> Fix this by defining image GUID values and firmware names for
> individual images per platform. The GetImageInfo FMP hook would then
> populate these values in the image descriptor array.
> 
> Also add the image index value associated with a particular
> image. This is the value that should match with the image_index value
> that is part of the capsule header. The capsule update code will check
> if the two values match, and the update will only proceed on a match.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes since V3: None
> 
>  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 20 +++++++++++++
>  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 19 +++++++++++++
>  board/emulation/qemu-arm/qemu-arm.c           | 20 +++++++++++++
>  board/kontron/pitx_imx8m/pitx_imx8m.c         | 16 ++++++++++-
>  board/kontron/sl-mx8mm/sl-mx8mm.c             | 15 ++++++++++
>  board/kontron/sl28/sl28.c                     | 15 ++++++++++
>  board/sandbox/sandbox.c                       | 28 +++++++++++++++++++
>  board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++
>  board/xilinx/common/board.c                   | 24 ++++++++++++++++
>  include/configs/imx8mm-cl-iot-gate.h          | 10 +++++++
>  include/configs/imx8mp_rsb3720.h              | 10 +++++++
>  include/configs/kontron-sl-mx8mm.h            |  6 ++++
>  include/configs/kontron_pitx_imx8m.h          |  6 ++++
>  include/configs/kontron_sl28.h                |  6 ++++
>  include/configs/qemu-arm.h                    | 10 +++++++
>  include/configs/sandbox.h                     | 14 ++++++++++
>  include/configs/synquacer.h                   | 14 ++++++++++
>  include/configs/xilinx_versal.h               |  6 ++++
>  include/configs/xilinx_zynqmp.h               | 10 +++++++
>  include/configs/zynq-common.h                 | 10 +++++++
>  include/efi_loader.h                          | 18 ++++++++++++
>  21 files changed, 302 insertions(+), 1 deletion(-)
> 
> diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> index 16566092bd..1c953ba195 100644
> --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> @@ -6,6 +6,8 @@
>  
>  #include <common.h>
>  #include <dwc3-uboot.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <errno.h>
>  #include <miiphy.h>
>  #include <netdev.h>
> @@ -21,6 +23,7 @@
>  #include <asm/arch/clock.h>
>  #include <asm/mach-imx/dma.h>
>  #include <linux/delay.h>
> +#include <linux/kernel.h>
>  #include <power/pmic.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -44,6 +47,23 @@ static void setup_gpmi_nand(void)
>  }
>  #endif
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +#if defined(CONFIG_TARGET_IMX8MP_RSB3720A1_4G)
> +		.image_type_id = IMX8MP_RSB3720A1_4G_FIT_IMAGE_GUID,
> +#elif defined(CONFIG_TARGET_IMX8MP_RSB3720A1_6G)
> +		.image_type_id = IMX8MP_RSB3720A1_6G_FIT_IMAGE_GUID,
> +#endif
> +		.fw_name = u"IMX8MP-RSB3720-FIT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
> +
>  int board_early_init_f(void)
>  {
>  	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> index 7e2d88f449..f5b89a5ddc 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> @@ -5,6 +5,8 @@
>   */
>  
>  #include <common.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <env.h>
>  #include <extension_board.h>
>  #include <hang.h>
> @@ -21,11 +23,28 @@
>  #include <asm/mach-imx/gpio.h>
>  #include <asm/mach-imx/mxc_i2c.h>
>  #include <asm/sections.h>
> +#include <linux/kernel.h>
>  
>  #include "ddr/ddr.h"
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +#if defined(CONFIG_TARGET_IMX8MM_CL_IOT_GATE)
> +		.image_type_id = IMX8MM_CL_IOT_GATE_FIT_IMAGE_GUID,
> +#elif defined(CONFIG_TARGET_IMX8MM_CL_IOT_GATE_OPTEE)
> +		.image_type_id = IMX8MM_CL_IOT_GATE_OPTEE_FIT_IMAGE_GUID,
> +#endif
> +		.fw_name = u"IMX8MM-CL-IOT-GATE-FIT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  int board_phys_sdram_size(phys_size_t *size)
>  {
>  	struct lpddr4_tcm_desc *lpddr4_tcm_desc =
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 16d5a97167..ef77d0dc14 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -6,15 +6,35 @@
>  #include <common.h>
>  #include <cpu_func.h>
>  #include <dm.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <fdtdec.h>
>  #include <init.h>
>  #include <log.h>
>  #include <virtio_types.h>
>  #include <virtio.h>
>  
> +#include <linux/kernel.h>
> +
>  #ifdef CONFIG_ARM64
>  #include <asm/armv8/mmu.h>
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +#if defined(CONFIG_TARGET_QEMU_ARM_32BIT)
> +		.image_type_id = QEMU_ARM_UBOOT_IMAGE_GUID,
> +#elif defined(CONFIG_TARGET_QEMU_ARM_64BIT)
> +		.image_type_id = QEMU_ARM64_UBOOT_IMAGE_GUID,
> +#endif
> +		.fw_name = u"Qemu-Arm-UBOOT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  static struct mm_region qemu_arm64_mem_map[] = {
>  	{
>  		/* Flash */
> diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
> index d655fe099b..8dc04411cc 100644
> --- a/board/kontron/pitx_imx8m/pitx_imx8m.c
> +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
> @@ -2,6 +2,8 @@
>  
>  #include "pitx_misc.h"
>  #include <common.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <init.h>
>  #include <mmc.h>
>  #include <miiphy.h>
> @@ -12,7 +14,7 @@
>  #include <asm/mach-imx/gpio.h>
>  #include <asm/mach-imx/iomux-v3.h>
>  #include <linux/delay.h>
> -
> +#include <linux/kernel.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -30,6 +32,18 @@ static iomux_v3_cfg_t const uart_pads[] = {
>  	IMX8MQ_PAD_ECSPI1_MISO__UART3_CTS_B | MUX_PAD_CTRL(UART_PAD_CTRL),
>  };
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +		.image_type_id = KONTRON_PITX_IMX8M_FIT_IMAGE_GUID,
> +		.fw_name = u"KONTRON-PITX-IMX8M-UBOOT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  int board_early_init_f(void)
>  {
>  	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
> index 48376cb826..834588af3a 100644
> --- a/board/kontron/sl-mx8mm/sl-mx8mm.c
> +++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
> @@ -6,12 +6,27 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <fdt_support.h>
>  #include <linux/errno.h>
> +#include <linux/kernel.h>
>  #include <net.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +		.image_type_id = KONTRON_SL_MX8MM_FIT_IMAGE_GUID,
> +		.fw_name = u"KONTROL-SL-MX8MM-UBOOT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  int board_phys_sdram_size(phys_size_t *size)
>  {
>  	u32 ddr_size = readl(M4_BOOTROM_BASE_ADDR);
> diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
> index 3c48a9141d..7d3635da45 100644
> --- a/board/kontron/sl28/sl28.c
> +++ b/board/kontron/sl28/sl28.c
> @@ -3,11 +3,14 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <malloc.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <errno.h>
>  #include <fsl_ddr.h>
>  #include <fdt_support.h>
>  #include <asm/global_data.h>
>  #include <linux/libfdt.h>
> +#include <linux/kernel.h>
>  #include <env_internal.h>
>  #include <asm/arch-fsl-layerscape/soc.h>
>  #include <asm/arch-fsl-layerscape/fsl_icid.h>
> @@ -23,6 +26,18 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +		.image_type_id = KONTRON_SL28_FIT_IMAGE_GUID,
> +		.fw_name = u"KONTRON-SL28-FIT",
> +		.image_index = 1
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  int board_early_init_f(void)
>  {
>  	fsl_lsch3_early_init_f();
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index 5d9a945d64..c5e6e3d2a0 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -7,6 +7,8 @@
>  #include <cpu_func.h>
>  #include <cros_ec.h>
>  #include <dm.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <env_internal.h>
>  #include <init.h>
>  #include <led.h>
> @@ -14,6 +16,7 @@
>  #include <asm/global_data.h>
>  #include <asm/test.h>
>  #include <asm/u-boot-sandbox.h>
> +#include <linux/kernel.h>
>  #include <malloc.h>
>  
>  #include <extension_board.h>
> @@ -25,6 +28,31 @@
>   */
>  gd_t *gd;
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)
> +	{
> +		.image_type_id = SANDBOX_UBOOT_IMAGE_GUID,
> +		.fw_name = u"SANDBOX-UBOOT",
> +		.image_index = 1
> +	},
> +	{
> +		.image_type_id = SANDBOX_UBOOT_ENV_IMAGE_GUID,
> +		.fw_name = u"SANDBOX-UBOOT-ENV",
> +		.image_index = 2
> +	},
> +#elif defined(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)
> +	{
> +		.image_type_id = SANDBOX_FIT_IMAGE_GUID,
> +		.fw_name = u"SANDBOX-FIT",
> +		.image_index = 1
> +	},
> +#endif
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
>  /*
>   * Add a simple GPIO device (don't use with of-platdata as it interferes with
> diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> index 9552bfcdc3..ae4b2d6ed8 100644
> --- a/board/socionext/developerbox/developerbox.c
> +++ b/board/socionext/developerbox/developerbox.c
> @@ -10,10 +10,36 @@
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <common.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <env_internal.h>
>  #include <fdt_support.h>
>  #include <log.h>
>  
> +#include <linux/kernel.h>
> +
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +	{
> +		.image_type_id = DEVELOPERBOX_UBOOT_IMAGE_GUID,
> +		.fw_name = u"DEVELOPERBOX-UBOOT",
> +		.image_index = 1
> +	},
> +	{
> +		.image_type_id = DEVELOPERBOX_FIP_IMAGE_GUID,
> +		.fw_name = u"DEVELOPERBOX-FIP",
> +		.image_index = 2
> +	},
> +	{
> +		.image_type_id = DEVELOPERBOX_OPTEE_IMAGE_GUID,
> +		.fw_name = u"DEVELOPERBOX-OPTEE",
> +		.image_index = 3
> +	},
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  static struct mm_region sc2a11_mem_map[] = {
>  	{
>  		.virt = 0x0UL,
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index 0068cb8792..2e63bb4d4f 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -5,6 +5,8 @@
>   */
>  
>  #include <common.h>
> +#include <efi.h>
> +#include <efi_loader.h>
>  #include <env.h>
>  #include <log.h>
>  #include <asm/global_data.h>
> @@ -20,9 +22,31 @@
>  #include <generated/dt.h>
>  #include <soc.h>
>  #include <linux/ctype.h>
> +#include <linux/kernel.h>
>  
>  #include "fru.h"
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +struct efi_fw_images fw_images[] = {
> +#if defined(XILINX_BOOT_IMAGE_GUID)
> +	{
> +		.image_type_id = XILINX_BOOT_IMAGE_GUID,
> +		.fw_name = u"XILINX-BOOT",
> +		.image_index = 1
> +	},
> +#endif
> +#if defined(XILINX_UBOOT_IMAGE_GUID)
> +	{
> +		.image_type_id = XILINX_UBOOT_IMAGE_GUID,
> +		.fw_name = u"XILINX-UBOOT",
> +		.image_index = 2
> +	},
> +#endif
> +};
> +
> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #if defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET)
>  int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
>  {
> diff --git a/include/configs/imx8mm-cl-iot-gate.h b/include/configs/imx8mm-cl-iot-gate.h
> index 7e6be6050c..35df2e755e 100644
> --- a/include/configs/imx8mm-cl-iot-gate.h
> +++ b/include/configs/imx8mm-cl-iot-gate.h
> @@ -31,6 +31,16 @@
>  
>  #endif
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define IMX8MM_CL_IOT_GATE_FIT_IMAGE_GUID \
> +	EFI_GUID(0x7a32a939, 0xab92, 0x467b, 0x91, 0x52, \
> +		 0x74, 0x77, 0x1b, 0x95, 0xe6, 0x46)
> +
> +#define IMX8MM_CL_IOT_GATE_OPTEE_FIT_IMAGE_GUID \
> +	EFI_GUID(0x0bf1165c, 0x1831, 0x4864, 0x94, 0x5e, \
> +		 0xac, 0x3d, 0x38, 0x48, 0xf4, 0x99)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #if CONFIG_IS_ENABLED(CMD_MMC)
>  # define BOOT_TARGET_MMC(func) \
>  	func(MMC, mmc, 2)      \
> diff --git a/include/configs/imx8mp_rsb3720.h b/include/configs/imx8mp_rsb3720.h
> index ac4a7d0cb3..a5a845c2da 100644
> --- a/include/configs/imx8mp_rsb3720.h
> +++ b/include/configs/imx8mp_rsb3720.h
> @@ -21,6 +21,16 @@
>  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
>  #define CONFIG_SYS_UBOOT_BASE	(QSPI0_AMBA_BASE + CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define IMX8MP_RSB3720A1_4G_FIT_IMAGE_GUID \
> +	EFI_GUID(0xb1251e89, 0x384a, 0x4635, 0xa8, 0x06, \
> +		 0x3a, 0xa0, 0xb0, 0xe9, 0xf9, 0x65)
> +
> +#define IMX8MP_RSB3720A1_6G_FIT_IMAGE_GUID \
> +	EFI_GUID(0xb5fb6f08, 0xe142, 0x4db1, 0x97, 0xea, \
> +		 0x5f, 0xd3, 0x6b, 0x9b, 0xe5, 0xb9)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT*/
> +
>  #ifdef CONFIG_SPL_BUILD
>  #define CONFIG_SPL_LDSCRIPT		"arch/arm/cpu/armv8/u-boot-spl.lds"
>  #define CONFIG_SPL_STACK		0x960000
> diff --git a/include/configs/kontron-sl-mx8mm.h b/include/configs/kontron-sl-mx8mm.h
> index 788ae77cd3..aff1b90010 100644
> --- a/include/configs/kontron-sl-mx8mm.h
> +++ b/include/configs/kontron-sl-mx8mm.h
> @@ -38,6 +38,12 @@
>  #define CONFIG_USB_MAX_CONTROLLER_COUNT	2
>  #endif
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define KONTRON_SL_MX8MM_FIT_IMAGE_GUID \
> +	EFI_GUID(0xd488e45a, 0x4929, 0x4b55, 0x8c, 0x14, \
> +		 0x86, 0xce, 0xa2, 0xcd, 0x66, 0x29)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #ifndef CONFIG_SPL_BUILD
>  #define BOOT_TARGET_DEVICES(func) \
>  	func(MMC, mmc, 1) \
> diff --git a/include/configs/kontron_pitx_imx8m.h b/include/configs/kontron_pitx_imx8m.h
> index 0f96b905ab..678364e367 100644
> --- a/include/configs/kontron_pitx_imx8m.h
> +++ b/include/configs/kontron_pitx_imx8m.h
> @@ -14,6 +14,12 @@
>  #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
>  #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define KONTRON_PITX_IMX8M_FIT_IMAGE_GUID \
> +	EFI_GUID(0xc898e959, 0x5b1f, 0x4e6d, 0x88, 0xe0, \
> +		 0x40, 0xd4, 0x5c, 0xca, 0x13, 0x99)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #ifdef CONFIG_SPL_BUILD
>  #define CONFIG_SPL_LDSCRIPT		"arch/arm/cpu/armv8/u-boot-spl.lds"
>  #define CONFIG_SPL_STACK		0x187FF0
> diff --git a/include/configs/kontron_sl28.h b/include/configs/kontron_sl28.h
> index 448749a7f8..97d0d365f6 100644
> --- a/include/configs/kontron_sl28.h
> +++ b/include/configs/kontron_sl28.h
> @@ -57,6 +57,12 @@
>  #define CONFIG_SYS_SPL_MALLOC_START	0x80200000
>  #define CONFIG_SYS_MONITOR_LEN		(1024 * 1024)
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define KONTRON_SL28_FIT_IMAGE_GUID \
> +	EFI_GUID(0x86ebd44f, 0xfeb8, 0x466f, 0x8b, 0xb8, \
> +		 0x89, 0x06, 0x18, 0x45, 0x6d, 0x8b)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* environment */
>  /* see include/configs/ti_armv7_common.h */
>  #define ENV_MEM_LAYOUT_SETTINGS \
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index d45f606860..2f2abc746d 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -17,6 +17,16 @@
>  
>  #define CONFIG_SYS_BOOTM_LEN		SZ_64M
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define QEMU_ARM_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0xf885b085, 0x99f8, 0x45af, 0x84, 0x7d, \
> +		 0xd5, 0x14, 0x10, 0x7a, 0x4a, 0x2c)
> +
> +#define QEMU_ARM64_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0x058b7d83, 0x50d5, 0x4c47, 0xa1, 0x95, \
> +		 0x60, 0xd8, 0x6a, 0xd3, 0x41, 0xc4)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* For timer, QEMU emulates an ARMv7/ARMv8 architected timer */
>  
>  /* Environment options */
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 75efbf3448..e951d08056 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -14,6 +14,20 @@
>  
>  #define CONFIG_SYS_CBSIZE		1024	/* Console I/O Buffer Size */
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define SANDBOX_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \
> +		 0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
> +
> +#define SANDBOX_UBOOT_ENV_IMAGE_GUID \
> +	EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \
> +		 0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0)
> +
> +#define SANDBOX_FIT_IMAGE_GUID \
> +	EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \
> +		 0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* Size of our emulated memory */
>  #define SB_CONCAT(x, y) x ## y
>  #define SB_TO_UL(s) SB_CONCAT(s, UL)
> diff --git a/include/configs/synquacer.h b/include/configs/synquacer.h
> index 8dd092fc59..07e1f56e3d 100644
> --- a/include/configs/synquacer.h
> +++ b/include/configs/synquacer.h
> @@ -51,6 +51,20 @@
>  			"fip.bin raw 180000 78000;"			\
>  			"optee.bin raw 500000 100000\0"
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define DEVELOPERBOX_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0x53a92e83, 0x4ef4, 0x473a, 0x8b, 0x0d, \
> +		 0xb5, 0xd8, 0xc7, 0xb2, 0xd6, 0x00)
> +
> +#define DEVELOPERBOX_FIP_IMAGE_GUID \
> +	EFI_GUID(0x880866e9, 0x84ba, 0x4793, 0xa9, 0x08, \
> +		 0x33, 0xe0, 0xb9, 0x16, 0xf3, 0x98)
> +
> +#define DEVELOPERBOX_OPTEE_IMAGE_GUID \
> +	EFI_GUID(0xc1b629f1, 0xce0e, 0x4894, 0x82, 0xbf, \
> +		 0xf0, 0xa3, 0x83, 0x87, 0xe6, 0x30)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* Distro boot settings */
>  #ifndef CONFIG_SPL_BUILD
>  #ifdef CONFIG_CMD_USB
> diff --git a/include/configs/xilinx_versal.h b/include/configs/xilinx_versal.h
> index bc72f5f35f..95218962f7 100644
> --- a/include/configs/xilinx_versal.h
> +++ b/include/configs/xilinx_versal.h
> @@ -31,6 +31,12 @@
>  #define CONFIG_BOOTP_BOOTFILESIZE
>  #define CONFIG_BOOTP_MAY_FAIL
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define XILINX_BOOT_IMAGE_GUID \
> +	EFI_GUID(0x20c5fba5, 0x0171, 0x457f, 0xb9, 0xcd, \
> +		 0xf5, 0x12, 0x9c, 0xd0, 0x72, 0x28)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* Miscellaneous configurable options */
>  
>  /* Monitor Command Prompt */
> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
> index e51d92ffe4..942f004b15 100644
> --- a/include/configs/xilinx_zynqmp.h
> +++ b/include/configs/xilinx_zynqmp.h
> @@ -31,6 +31,16 @@
>  #define CONFIG_BOOTP_BOOTFILESIZE
>  #define CONFIG_BOOTP_MAY_FAIL
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define XILINX_BOOT_IMAGE_GUID \
> +	EFI_GUID(0xde6066e8, 0x0256, 0x4fad, 0x82, 0x38, \
> +		 0xe4, 0x06, 0xe2, 0x74, 0xc4, 0xcf)
> +
> +#define XILINX_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0xcf9ecfd4, 0x938b, 0x41c5, 0x85, 0x51, \
> +		 0x1f, 0x88, 0x3a, 0xb7, 0xdc, 0x18)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  #ifdef CONFIG_NAND_ARASAN
>  # define CONFIG_SYS_MAX_NAND_DEVICE	1
>  #endif
> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
> index 780952cf5f..d116947d25 100644
> --- a/include/configs/zynq-common.h
> +++ b/include/configs/zynq-common.h
> @@ -20,6 +20,16 @@
>  #define CONFIG_SYS_TIMER_COUNTS_DOWN
>  #define CONFIG_SYS_TIMER_COUNTER	(CONFIG_SYS_TIMERBASE + 0x4)
>  
> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> +#define XILINX_BOOT_IMAGE_GUID \
> +	EFI_GUID(0x1ba29a15, 0x9969, 0x40aa, 0xb4, 0x24, \
> +		 0xe8, 0x61, 0x21, 0x61, 0x86, 0x64)
> +
> +#define XILINX_UBOOT_IMAGE_GUID \
> +	EFI_GUID(0x1a5178f0, 0x87d3, 0x4f36, 0xac, 0x63, \
> +		 0x3b, 0x31, 0xa2, 0x3b, 0xe3, 0x05)
> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
> +
>  /* Serial drivers */
>  /* The following table includes the supported baudrates */
>  #define CONFIG_SYS_BAUDRATE_TABLE  \
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index af36639ec6..284d64547b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -979,6 +979,24 @@ efi_status_t efi_capsule_authenticate(const void *capsule,
>  
>  #define EFI_CAPSULE_DIR u"\\EFI\\UpdateCapsule\\"
>  
> +/**
> + * struct efi_fw_images - List of firmware images updatable through capsule
> + *                        update
> + *
> + * This structure gives information about the firmware images on the platform
> + * which can be updated through the capsule update mechanism
> + *
> + * @image_type_id:	Image GUID. Same value is to be used in the capsule
> + * @fw_name:		Name of the firmware image
> + * @image_index:	Image Index, same as value passed to SetImage FMP
> + *                      function
> + */
> +struct efi_fw_images {
> +	efi_guid_t image_type_id;
> +	const u16 *fw_name;
> +	u8 image_index;
> +};
> +
>  /**
>   * Install the ESRT system table.
>   *
> -- 
> 2.25.1
> 
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
  2022-03-31 13:27 ` [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled Sughosh Ganu
@ 2022-03-31 14:08   ` Masami Hiramatsu
  2022-03-31 19:35   ` Ilias Apalodimas
  1 sibling, 0 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2022-03-31 14:08 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, AKASHI Takahiro,
	Ying-Chun Liu, Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf,
	Michael Walle, Jassi Brar, Michal Simek, Michal Simek

Hi Sughosh,

OK, this looks good to me.

Acked-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

for DeveloperBox part.
Thank you,

2022年3月31日(木) 22:28 Sughosh Ganu <sughosh.ganu@linaro.org>:
>
> Currently, there are a bunch of boards which enable the UEFI capsule
> update feature. The actual update of the firmware images is done
> through the dfu framework which uses the dfu_alt_info environment
> variable for getting information on the update, like device, partition
> number/address etc. Currently, these boards define the dfu_alt_info
> variable in the board config header, as an environment variable. With
> this, the variable can be modified from the u-boot command line and
> this can cause an incorrect update.
>
> To prevent this from happening, define the set_dfu_alt_info function
> in the board file, and select SET_DFU_ALT_INFO for all platforms which
> enable the capsule update feature. With the function defined, the dfu
> framework populates the dfu_alt_info variable through the board file,
> instead of fetching the variable from the environment, thus making the
> update more robust.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>
> Changes since V3:
> * Do not remove the existing dfu_alt_info definitions made by
>   platforms in the config files, as discussed with Masami.
> * Squash the selection of the SET_DFU_ALT_INFO config symbol for
>   capsule update feature as part of this patch.
>
>
>  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 24 +++++++++++++++++
>  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 24 +++++++++++++++++
>  board/emulation/common/qemu_dfu.c             |  6 ++---
>  board/kontron/pitx_imx8m/pitx_imx8m.c         | 24 +++++++++++++++++
>  board/kontron/sl-mx8mm/sl-mx8mm.c             | 24 +++++++++++++++++
>  board/kontron/sl28/sl28.c                     | 25 ++++++++++++++++++
>  board/sandbox/sandbox.c                       | 26 +++++++++++++++++++
>  board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++++
>  board/xilinx/zynq/board.c                     |  5 ++--
>  board/xilinx/zynqmp/zynqmp.c                  |  5 ++--
>  lib/efi_loader/Kconfig                        |  2 ++
>  11 files changed, 184 insertions(+), 7 deletions(-)
>
> diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> index 1c953ba195..41154ca9f3 100644
> --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> @@ -5,10 +5,12 @@
>   */
>
>  #include <common.h>
> +#include <dfu.h>
>  #include <dwc3-uboot.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <errno.h>
> +#include <memalign.h>
>  #include <miiphy.h>
>  #include <netdev.h>
>  #include <spl.h>
> @@ -24,6 +26,7 @@
>  #include <asm/mach-imx/dma.h>
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>  #include <power/pmic.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
>         }
>  }
>  #endif /* CONFIG_SPL_MMC_SUPPORT */
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> index f5b89a5ddc..1880dd9c55 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <common.h>
> +#include <dfu.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <env.h>
> @@ -12,6 +13,7 @@
>  #include <hang.h>
>  #include <i2c.h>
>  #include <init.h>
> +#include <memalign.h>
>  #include <miiphy.h>
>  #include <netdev.h>
>
> @@ -24,6 +26,7 @@
>  #include <asm/mach-imx/mxc_i2c.h>
>  #include <asm/sections.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>
>  #include "ddr/ddr.h"
>
> @@ -446,3 +449,24 @@ int board_late_init(void)
>
>         return 0;
>  }
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/emulation/common/qemu_dfu.c b/board/emulation/common/qemu_dfu.c
> index 62234a7647..85dff4373b 100644
> --- a/board/emulation/common/qemu_dfu.c
> +++ b/board/emulation/common/qemu_dfu.c
> @@ -44,10 +44,11 @@ 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"))
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
>                 return;
>
> -       memset(buf, 0, sizeof(buf));
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
>
>         /*
>          * Currently dfu_alt_info is needed on Qemu ARM64 for
> @@ -64,5 +65,4 @@ void set_dfu_alt_info(char *interface, char *devstr)
>         }
>
>         env_set("dfu_alt_info", buf);
> -       printf("dfu_alt_info set\n");
>  }
> diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
> index 8dc04411cc..f3f6fbee9f 100644
> --- a/board/kontron/pitx_imx8m/pitx_imx8m.c
> +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c
> @@ -2,9 +2,11 @@
>
>  #include "pitx_misc.h"
>  #include <common.h>
> +#include <dfu.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <init.h>
> +#include <memalign.h>
>  #include <mmc.h>
>  #include <miiphy.h>
>  #include <asm/arch/clock.h>
> @@ -15,6 +17,7 @@
>  #include <asm/mach-imx/iomux-v3.h>
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -181,3 +184,24 @@ int board_late_init(void)
>  {
>         return 0;
>  }
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/kontron/sl-mx8mm/sl-mx8mm.c b/board/kontron/sl-mx8mm/sl-mx8mm.c
> index 834588af3a..a00eb19828 100644
> --- a/board/kontron/sl-mx8mm/sl-mx8mm.c
> +++ b/board/kontron/sl-mx8mm/sl-mx8mm.c
> @@ -6,11 +6,14 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> +#include <dfu.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <fdt_support.h>
> +#include <memalign.h>
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>  #include <net.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -112,3 +115,24 @@ int board_init(void)
>  {
>         return 0;
>  }
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "sf 0:0=flash-bin raw 0x400 0x1f0000");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c
> index 7d3635da45..db41e2885c 100644
> --- a/board/kontron/sl28/sl28.c
> +++ b/board/kontron/sl28/sl28.c
> @@ -2,7 +2,9 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <dfu.h>
>  #include <malloc.h>
> +#include <memalign.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <errno.h>
> @@ -11,6 +13,7 @@
>  #include <asm/global_data.h>
>  #include <linux/libfdt.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>  #include <env_internal.h>
>  #include <asm/arch-fsl-layerscape/soc.h>
>  #include <asm/arch-fsl-layerscape/fsl_icid.h>
> @@ -150,3 +153,25 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>
>         return 0;
>  }
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "sf 0:0=u-boot-bin raw 0x210000 0x1d0000;"
> +                "u-boot-env raw 0x3e0000 0x20000");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index c5e6e3d2a0..6a0453281d 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -6,17 +6,20 @@
>  #include <common.h>
>  #include <cpu_func.h>
>  #include <cros_ec.h>
> +#include <dfu.h>
>  #include <dm.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <env_internal.h>
>  #include <init.h>
>  #include <led.h>
> +#include <memalign.h>
>  #include <os.h>
>  #include <asm/global_data.h>
>  #include <asm/test.h>
>  #include <asm/u-boot-sandbox.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>  #include <malloc.h>
>
>  #include <extension_board.h>
> @@ -152,3 +155,26 @@ int board_late_init(void)
>         return 0;
>  }
>  #endif
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +                "sf 0:0=u-boot-bin raw 0x100000 0x50000;"
> +                "u-boot-env raw 0x150000 0x200000"
> +               );
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif
> diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> index ae4b2d6ed8..6784a3db53 100644
> --- a/board/socionext/developerbox/developerbox.c
> +++ b/board/socionext/developerbox/developerbox.c
> @@ -10,13 +10,16 @@
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <common.h>
> +#include <dfu.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <env_internal.h>
>  #include <fdt_support.h>
>  #include <log.h>
> +#include <memalign.h>
>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>
>  #if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
>  struct efi_fw_images fw_images[] = {
> @@ -185,3 +188,26 @@ int print_cpuinfo(void)
>         printf("CPU:   SC2A11:Cortex-A53 MPCore 24cores\n");
>         return 0;
>  }
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN                SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +       ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
> +               return;
> +
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
> +
> +       snprintf(buf, DFU_ALT_BUF_LEN,
> +               "mtd nor1=u-boot.bin raw 200000 100000;"
> +               "fip.bin raw 180000 78000;"
> +               "optee.bin raw 500000 100000");
> +
> +       env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */
> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> index 26ef048835..c70ad07269 100644
> --- a/board/xilinx/zynq/board.c
> +++ b/board/xilinx/zynq/board.c
> @@ -168,10 +168,11 @@ 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"))
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
>                 return;
>
> -       memset(buf, 0, sizeof(buf));
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
>
>         switch ((zynq_slcr_get_boot_mode()) & ZYNQ_BM_MASK) {
>         case ZYNQ_BM_SD:
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 70b3c81f12..d278ed21a1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -890,10 +890,11 @@ 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"))
> +       if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +           env_get("dfu_alt_info"))
>                 return;
>
> -       memset(buf, 0, sizeof(buf));
> +       memset(buf, 0, DFU_ALT_BUF_LEN);
>
>         multiboot = multi_boot();
>         if (multiboot < 0)
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e5e35fe51f..09fb8cbe75 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -174,6 +174,7 @@ 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
> @@ -185,6 +186,7 @@ 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
> --
> 2.25.1
>


-- 
Masami Hiramatsu

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data
  2022-03-31 13:27 ` [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data Sughosh Ganu
@ 2022-03-31 15:08   ` Ilias Apalodimas
  2022-03-31 16:29     ` Sughosh Ganu
  0 siblings, 1 reply; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 15:08 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

Hi Sughosh, 

On Thu, Mar 31, 2022 at 06:57:44PM +0530, Sughosh Ganu wrote:
> Currently, the image descriptor array that has been passed to the
> GetImageInfo function of the Firmware Management Protocol(FMP) gets
> populated through the data stored with the dfu framework. The
> dfu data is not restricted to contain information only of the images
> updatable through the capsule update mechanism, but it also contains
> information on other images.
> 
> The image descriptor array is also parsed by the ESRT generation code,
> and thus the ESRT table contains entries for other images that are not
> being handled by the FMP for the capsule updates.
> 
> The other issue fixed is assignment of a separate GUID for all images
> in the image descriptor array. The UEFI specification mandates that
> all entries in the ESRT table should have a unique GUID value as part
> of the FwClass member of the EFI_SYSTEM_RESOURCE_ENTRY. Currently, all
> images are assigned a single GUID value, either an FIT GUID or a raw
> image GUID. This is fixed by obtaining the GUID values from the
> efi_fw_images array defined per platform.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
> 
> Changes since V3: None
> 
>  include/efi_loader.h          |  3 ++
>  lib/efi_loader/efi_firmware.c | 91 +++++++++++------------------------
>  2 files changed, 30 insertions(+), 64 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 284d64547b..9704397bd7 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -997,6 +997,9 @@ struct efi_fw_images {
>  	u8 image_index;
>  };
>  
> +extern struct efi_fw_images fw_images[];
> +extern u8 num_image_type_guids;
> +
>  /**
>   * Install the ESRT system table.
>   *
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index a5ff32f121..169f3a29bb 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -97,91 +97,60 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
>  }
>  
>  /**
> - * efi_get_dfu_info - return information about the current firmware image
> + * efi_fill_image_desc_array - populate image descriptor array
>   * @this:			Protocol instance
>   * @image_info_size:		Size of @image_info
>   * @image_info:			Image information
>   * @descriptor_version:		Pointer to version number
> - * @descriptor_count:		Pointer to number of descriptors
> + * @descriptor_count:		Image count
>   * @descriptor_size:		Pointer to descriptor size
> - * package_version:		Package version
> - * package_version_name:	Package version's name
> - * image_type:			Image type GUID
> + * @package_version:		Package version
> + * @package_version_name:	Package version's name
>   *
>   * Return information bout the current firmware image in @image_info.
>   * @image_info will consist of a number of descriptors.
> - * Each descriptor will be created based on "dfu_alt_info" variable.
> + * Each descriptor will be created based on "efi_fw_images" variable.
>   *
>   * Return		status code
>   */
> -static efi_status_t efi_get_dfu_info(
> +static efi_status_t efi_fill_image_desc_array(
>  	efi_uintn_t *image_info_size,
>  	struct efi_firmware_image_descriptor *image_info,
>  	u32 *descriptor_version,
>  	u8 *descriptor_count,
>  	efi_uintn_t *descriptor_size,
>  	u32 *package_version,
> -	u16 **package_version_name,
> -	const efi_guid_t *image_type)
> +	u16 **package_version_name)
>  {
> -	struct dfu_entity *dfu;
>  	size_t names_len, total_size;
> -	int dfu_num, i;
> -	u16 *name, *next;
> -	int ret;
> -
> -	ret = dfu_init_env_entities(NULL, NULL);
> -	if (ret)
> -		return EFI_SUCCESS;
> +	struct efi_fw_images *fw_array;
> +	u8 image_count;
> +	int i;
>  
> +	fw_array = &fw_images[0];

nit, but fw_array = fw_images looks better

> +	*descriptor_count = image_count = num_image_type_guids;

We could get rid of image_count here

>  	names_len = 0;
> -	dfu_num = 0;
> -	list_for_each_entry(dfu, &dfu_list, list) {
> -		names_len += (utf8_utf16_strlen(dfu->name) + 1) * 2;
> -		dfu_num++;
> -	}
> -	if (!dfu_num) {
> -		log_warning("No entities in dfu_alt_info\n");
> -		*image_info_size = 0;
> -		dfu_free_entities();
>  
> -		return EFI_SUCCESS;
> -	}
> +	total_size = sizeof(*image_info) * image_count;
>  
> -	total_size = sizeof(*image_info) * dfu_num + names_len;
> -	/*
> -	 * we will assume that sizeof(*image_info) * dfu_name
> -	 * is, at least, a multiple of 2. So the start address for
> -	 * image_id_name would be aligned with 2 bytes.
> -	 */
>  	if (*image_info_size < total_size) {
>  		*image_info_size = total_size;
> -		dfu_free_entities();
>  
>  		return EFI_BUFFER_TOO_SMALL;
>  	}
>  	*image_info_size = total_size;
>  
>  	*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> -	*descriptor_count = dfu_num;
>  	*descriptor_size = sizeof(*image_info);
>  	*package_version = 0xffffffff; /* not supported */
>  	*package_version_name = NULL; /* not supported */
>  
> -	/* DFU alt number should correspond to image_index */
> -	i = 0;
> -	/* Name area starts just after descriptors */
> -	name = (u16 *)((u8 *)image_info + sizeof(*image_info) * dfu_num);
> -	next = name;
> -	list_for_each_entry(dfu, &dfu_list, list) {
> -		image_info[i].image_index = dfu->alt + 1;
> -		image_info[i].image_type_id = *image_type;
> -		image_info[i].image_id = dfu->alt;
> -
> -		/* copy the DFU entity name */
> -		utf8_utf16_strcpy(&next, dfu->name);
> -		image_info[i].image_id_name = name;
> -		name = ++next;
> +	for (i = 0; i < image_count; i++) {
> +		image_info[i].image_index = fw_array[i].image_index;
> +		image_info[i].image_type_id = fw_array[i].image_type_id;
> +		image_info[i].image_id = fw_array[i].image_index;
> +
> +		image_info[i].image_id_name = (u16 *)fw_array[i].fw_name;

We shouldn't be casting const qualifications away.  In theory no one will
even modify that in this case and nothing bad will happen.  Since the efi
spec defines the image_id_name as u16 * are we better of removing the const
from efi_fw_images?

Thanks
/Ilias
>  
>  		image_info[i].version = 0; /* not supported */
>  		image_info[i].version_name = NULL; /* not supported */
> @@ -202,12 +171,8 @@ static efi_status_t efi_get_dfu_info(
>  		image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
>  		image_info[i].hardware_instance = 1;
>  		image_info[i].dependencies = NULL;
> -
> -		i++;
>  	}
>  
> -	dfu_free_entities();
> -
>  	return EFI_SUCCESS;
>  }
>  
> @@ -267,11 +232,10 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
>  	     !descriptor_size || !package_version || !package_version_name))
>  		return EFI_EXIT(EFI_INVALID_PARAMETER);
>  
> -	ret = efi_get_dfu_info(image_info_size, image_info,
> -			       descriptor_version, descriptor_count,
> -			       descriptor_size,
> -			       package_version, package_version_name,
> -			       &efi_firmware_image_type_uboot_fit);
> +	ret = efi_fill_image_desc_array(image_info_size, image_info,
> +					descriptor_version, descriptor_count,
> +					descriptor_size, package_version,
> +					package_version_name);
>  
>  	return EFI_EXIT(ret);
>  }
> @@ -376,11 +340,10 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
>  	     !descriptor_size || !package_version || !package_version_name))
>  		return EFI_EXIT(EFI_INVALID_PARAMETER);
>  
> -	ret = efi_get_dfu_info(image_info_size, image_info,
> -			       descriptor_version, descriptor_count,
> -			       descriptor_size,
> -			       package_version, package_version_name,
> -			       &efi_firmware_image_type_uboot_raw);
> +	ret = efi_fill_image_desc_array(image_info_size, image_info,
> +					descriptor_version, descriptor_count,
> +					descriptor_size, package_version,
> +					package_version_name);
>  
>  	return EFI_EXIT(ret);
>  }
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data
  2022-03-31 15:08   ` Ilias Apalodimas
@ 2022-03-31 16:29     ` Sughosh Ganu
  0 siblings, 0 replies; 20+ messages in thread
From: Sughosh Ganu @ 2022-03-31 16:29 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

hi Ilias,

On Thu, 31 Mar 2022 at 20:38, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sughosh,
>
> On Thu, Mar 31, 2022 at 06:57:44PM +0530, Sughosh Ganu wrote:
> > Currently, the image descriptor array that has been passed to the
> > GetImageInfo function of the Firmware Management Protocol(FMP) gets
> > populated through the data stored with the dfu framework. The
> > dfu data is not restricted to contain information only of the images
> > updatable through the capsule update mechanism, but it also contains
> > information on other images.
> >
> > The image descriptor array is also parsed by the ESRT generation code,
> > and thus the ESRT table contains entries for other images that are not
> > being handled by the FMP for the capsule updates.
> >
> > The other issue fixed is assignment of a separate GUID for all images
> > in the image descriptor array. The UEFI specification mandates that
> > all entries in the ESRT table should have a unique GUID value as part
> > of the FwClass member of the EFI_SYSTEM_RESOURCE_ENTRY. Currently, all
> > images are assigned a single GUID value, either an FIT GUID or a raw
> > image GUID. This is fixed by obtaining the GUID values from the
> > efi_fw_images array defined per platform.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >
> > Changes since V3: None
> >
> >  include/efi_loader.h          |  3 ++
> >  lib/efi_loader/efi_firmware.c | 91 +++++++++++------------------------
> >  2 files changed, 30 insertions(+), 64 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 284d64547b..9704397bd7 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -997,6 +997,9 @@ struct efi_fw_images {
> >       u8 image_index;
> >  };
> >
> > +extern struct efi_fw_images fw_images[];
> > +extern u8 num_image_type_guids;
> > +
> >  /**
> >   * Install the ESRT system table.
> >   *
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index a5ff32f121..169f3a29bb 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -97,91 +97,60 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported(
> >  }
> >
> >  /**
> > - * efi_get_dfu_info - return information about the current firmware image
> > + * efi_fill_image_desc_array - populate image descriptor array
> >   * @this:                    Protocol instance
> >   * @image_info_size:         Size of @image_info
> >   * @image_info:                      Image information
> >   * @descriptor_version:              Pointer to version number
> > - * @descriptor_count:                Pointer to number of descriptors
> > + * @descriptor_count:                Image count
> >   * @descriptor_size:         Pointer to descriptor size
> > - * package_version:          Package version
> > - * package_version_name:     Package version's name
> > - * image_type:                       Image type GUID
> > + * @package_version:         Package version
> > + * @package_version_name:    Package version's name
> >   *
> >   * Return information bout the current firmware image in @image_info.
> >   * @image_info will consist of a number of descriptors.
> > - * Each descriptor will be created based on "dfu_alt_info" variable.
> > + * Each descriptor will be created based on "efi_fw_images" variable.
> >   *
> >   * Return            status code
> >   */
> > -static efi_status_t efi_get_dfu_info(
> > +static efi_status_t efi_fill_image_desc_array(
> >       efi_uintn_t *image_info_size,
> >       struct efi_firmware_image_descriptor *image_info,
> >       u32 *descriptor_version,
> >       u8 *descriptor_count,
> >       efi_uintn_t *descriptor_size,
> >       u32 *package_version,
> > -     u16 **package_version_name,
> > -     const efi_guid_t *image_type)
> > +     u16 **package_version_name)
> >  {
> > -     struct dfu_entity *dfu;
> >       size_t names_len, total_size;
> > -     int dfu_num, i;
> > -     u16 *name, *next;
> > -     int ret;
> > -
> > -     ret = dfu_init_env_entities(NULL, NULL);
> > -     if (ret)
> > -             return EFI_SUCCESS;
> > +     struct efi_fw_images *fw_array;
> > +     u8 image_count;
> > +     int i;
> >
> > +     fw_array = &fw_images[0];
>
> nit, but fw_array = fw_images looks better

Okay. Will change.

>
> > +     *descriptor_count = image_count = num_image_type_guids;
>
> We could get rid of image_count here

Will remove.

>
> >       names_len = 0;
> > -     dfu_num = 0;
> > -     list_for_each_entry(dfu, &dfu_list, list) {
> > -             names_len += (utf8_utf16_strlen(dfu->name) + 1) * 2;
> > -             dfu_num++;
> > -     }
> > -     if (!dfu_num) {
> > -             log_warning("No entities in dfu_alt_info\n");
> > -             *image_info_size = 0;
> > -             dfu_free_entities();
> >
> > -             return EFI_SUCCESS;
> > -     }
> > +     total_size = sizeof(*image_info) * image_count;
> >
> > -     total_size = sizeof(*image_info) * dfu_num + names_len;
> > -     /*
> > -      * we will assume that sizeof(*image_info) * dfu_name
> > -      * is, at least, a multiple of 2. So the start address for
> > -      * image_id_name would be aligned with 2 bytes.
> > -      */
> >       if (*image_info_size < total_size) {
> >               *image_info_size = total_size;
> > -             dfu_free_entities();
> >
> >               return EFI_BUFFER_TOO_SMALL;
> >       }
> >       *image_info_size = total_size;
> >
> >       *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> > -     *descriptor_count = dfu_num;
> >       *descriptor_size = sizeof(*image_info);
> >       *package_version = 0xffffffff; /* not supported */
> >       *package_version_name = NULL; /* not supported */
> >
> > -     /* DFU alt number should correspond to image_index */
> > -     i = 0;
> > -     /* Name area starts just after descriptors */
> > -     name = (u16 *)((u8 *)image_info + sizeof(*image_info) * dfu_num);
> > -     next = name;
> > -     list_for_each_entry(dfu, &dfu_list, list) {
> > -             image_info[i].image_index = dfu->alt + 1;
> > -             image_info[i].image_type_id = *image_type;
> > -             image_info[i].image_id = dfu->alt;
> > -
> > -             /* copy the DFU entity name */
> > -             utf8_utf16_strcpy(&next, dfu->name);
> > -             image_info[i].image_id_name = name;
> > -             name = ++next;
> > +     for (i = 0; i < image_count; i++) {
> > +             image_info[i].image_index = fw_array[i].image_index;
> > +             image_info[i].image_type_id = fw_array[i].image_type_id;
> > +             image_info[i].image_id = fw_array[i].image_index;
> > +
> > +             image_info[i].image_id_name = (u16 *)fw_array[i].fw_name;
>
> We shouldn't be casting const qualifications away.  In theory no one will
> even modify that in this case and nothing bad will happen.  Since the efi
> spec defines the image_id_name as u16 * are we better of removing the const
> from efi_fw_images?

Sure. I will change the structure member to a u16 *. Thanks.

-sughosh

>
> Thanks
> /Ilias
> >
> >               image_info[i].version = 0; /* not supported */
> >               image_info[i].version_name = NULL; /* not supported */
> > @@ -202,12 +171,8 @@ static efi_status_t efi_get_dfu_info(
> >               image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> >               image_info[i].hardware_instance = 1;
> >               image_info[i].dependencies = NULL;
> > -
> > -             i++;
> >       }
> >
> > -     dfu_free_entities();
> > -
> >       return EFI_SUCCESS;
> >  }
> >
> > @@ -267,11 +232,10 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
> >            !descriptor_size || !package_version || !package_version_name))
> >               return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > -     ret = efi_get_dfu_info(image_info_size, image_info,
> > -                            descriptor_version, descriptor_count,
> > -                            descriptor_size,
> > -                            package_version, package_version_name,
> > -                            &efi_firmware_image_type_uboot_fit);
> > +     ret = efi_fill_image_desc_array(image_info_size, image_info,
> > +                                     descriptor_version, descriptor_count,
> > +                                     descriptor_size, package_version,
> > +                                     package_version_name);
> >
> >       return EFI_EXIT(ret);
> >  }
> > @@ -376,11 +340,10 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
> >            !descriptor_size || !package_version || !package_version_name))
> >               return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > -     ret = efi_get_dfu_info(image_info_size, image_info,
> > -                            descriptor_version, descriptor_count,
> > -                            descriptor_size,
> > -                            package_version, package_version_name,
> > -                            &efi_firmware_image_type_uboot_raw);
> > +     ret = efi_fill_image_desc_array(image_info_size, image_info,
> > +                                     descriptor_version, descriptor_count,
> > +                                     descriptor_size, package_version,
> > +                                     package_version_name);
> >
> >       return EFI_EXIT(ret);
> >  }
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images
  2022-03-31 13:27 ` [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images Sughosh Ganu
@ 2022-03-31 18:10   ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 18:10 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

On Thu, Mar 31, 2022 at 06:57:48PM +0530, Sughosh Ganu wrote:
> The capsule update code has been modified for getting the image GUID
> values from the platform code. With this, each image now has a unique
> GUID value. With this change, there is no longer a need for defining
> GUIDs for FIT and raw images. Remove these GUID values.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> 
> Changes since V3: None
> 
>  include/efi_api.h             | 8 --------
>  lib/efi_loader/efi_firmware.c | 4 ----
>  2 files changed, 12 deletions(-)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 982c200172..c7f7873b5d 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -1967,14 +1967,6 @@ struct efi_signature_list {
>  	EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \
>  		 0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)
>  
> -#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
> -	EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
> -		 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
> -
> -#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
> -	EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
> -		 0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
> -
>  #define IMAGE_ATTRIBUTE_IMAGE_UPDATABLE		0x0000000000000001
>  #define IMAGE_ATTRIBUTE_RESET_REQUIRED		0x0000000000000002
>  #define IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED	0x0000000000000004
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 169f3a29bb..7734d9f353 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -185,8 +185,6 @@ static efi_status_t efi_fill_image_desc_array(
>   *   - versioning of firmware image
>   *   - package information
>   */
> -const efi_guid_t efi_firmware_image_type_uboot_fit =
> -	EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
>  
>  /**
>   * efi_firmware_fit_get_image_info - return information about the current
> @@ -293,8 +291,6 @@ const struct efi_firmware_management_protocol efi_fmp_fit = {
>   * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update
>   * method with raw data.
>   */
> -const efi_guid_t efi_firmware_image_type_uboot_raw =
> -	EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>  
>  /**
>   * efi_firmware_raw_get_image_info - return information about the current
> -- 
> 2.25.1
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types
  2022-03-31 13:27 ` [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types Sughosh Ganu
@ 2022-03-31 18:10   ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 18:10 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

On Thu, Mar 31, 2022 at 06:57:49PM +0530, Sughosh Ganu wrote:
> While building a capsule, the GUID value of that specific image is to
> be passed through the --guid command option to the mkeficapsule
> tool. This renders the EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID and
> EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID values superfluous. Remove the
> --raw and --fit command line options as well.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> 
> Changes since V3: None
> 
>  tools/eficapsule.h   |  8 --------
>  tools/mkeficapsule.c | 26 +-------------------------
>  2 files changed, 1 insertion(+), 33 deletions(-)
> 
> diff --git a/tools/eficapsule.h b/tools/eficapsule.h
> index 69c9c58c2f..d63b831443 100644
> --- a/tools/eficapsule.h
> +++ b/tools/eficapsule.h
> @@ -37,14 +37,6 @@ typedef struct {
>  	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
>  		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
>  
> -#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
> -	EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
> -		 0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
> -
> -#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
> -	EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
> -		 0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
> -
>  #define EFI_CERT_TYPE_PKCS7_GUID \
>  	EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \
>  		 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index c118335b93..5f74d23b9e 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -27,17 +27,11 @@
>  static const char *tool_name = "mkeficapsule";
>  
>  efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> -efi_guid_t efi_guid_image_type_uboot_fit =
> -		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
> -efi_guid_t efi_guid_image_type_uboot_raw =
> -		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>  efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>  
> -static const char *opts_short = "frg:i:I:v:p:c:m:dh";
> +static const char *opts_short = "g:i:I:v:p:c:m:dh";
>  
>  static struct option options[] = {
> -	{"fit", no_argument, NULL, 'f'},
> -	{"raw", no_argument, NULL, 'r'},
>  	{"guid", required_argument, NULL, 'g'},
>  	{"index", required_argument, NULL, 'i'},
>  	{"instance", required_argument, NULL, 'I'},
> @@ -54,8 +48,6 @@ static void print_usage(void)
>  	fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
>  		"Options:\n"
>  
> -		"\t-f, --fit                   FIT image type\n"
> -		"\t-r, --raw                   raw image type\n"
>  		"\t-g, --guid <guid string>    guid for image blob type\n"
>  		"\t-i, --index <index>         update image index\n"
>  		"\t-I, --instance <instance>   update hardware instance\n"
> @@ -606,22 +598,6 @@ int main(int argc, char **argv)
>  			break;
>  
>  		switch (c) {
> -		case 'f':
> -			if (guid) {
> -				fprintf(stderr,
> -					"Image type already specified\n");
> -				exit(EXIT_FAILURE);
> -			}
> -			guid = &efi_guid_image_type_uboot_fit;
> -			break;
> -		case 'r':
> -			if (guid) {
> -				fprintf(stderr,
> -					"Image type already specified\n");
> -				exit(EXIT_FAILURE);
> -			}
> -			guid = &efi_guid_image_type_uboot_raw;
> -			break;
>  		case 'g':
>  			if (guid) {
>  				fprintf(stderr,
> -- 
> 2.25.1
> 

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation
  2022-03-31 13:27 ` [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation Sughosh Ganu
@ 2022-03-31 18:14   ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 18:14 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

On Thu, Mar 31, 2022 at 06:57:50PM +0530, Sughosh Ganu wrote:
> Update the capsule update functionality related documentation to
> refect the additional definitions that need to be made per platform
> for supporting the capsule update feature.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> 
> Changes since V3:
> * Rephrase the commit message to indicate that the doc changes are not
>   just limited to adding the GUID values, but other info as well.
> * Elaborate with an example on the relation between the dfu alt number
>   and the image index 
> 
>  doc/develop/uefi/uefi.rst | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index fe337c88bd..d886635cc3 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -312,8 +312,8 @@ Run the following command
>  .. code-block:: console
>  
>      $ mkeficapsule \
> -      --index 1 --instance 0 \
> -      [--fit <FIT image> | --raw <raw image>] \
> +      --index <index> --instance 0 \
> +      --guid <image GUID> \
>        <capsule_file_name>
>  
>  Performing the update
> @@ -333,6 +333,26 @@ won't be taken over across the reboot. If this is the case, you can skip
>  this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS)
>  set.
>  
> +A few things need to be defined in the board file for performing the
> +capsule upadte. The first is defining the function set_dfu_alt_info in

s/upadte/update/

> +the board file. This function sets the environment variable
> +dfu_alt_info. Instead of taking the variable from the environment, the
> +capsule update feature requires that the variable be set through the

I think we should also mention that allowing a user to change the location
of the firmware updates in flash is not a good security practice.  
Having that baked in the firmware (as long as a prior stage boot loader verifies it)
is a better approach.

Thanks
/Ilias
> +board function, since that is more robust. Secondly, define GUID
> +values and image index of the images that are to be updated through
> +the capsule update feature in the board file. Both the values are to
> +be defined as part of the fw_images array. These GUID values would be
> +used by the Firmware Management Protocol(FMP) to populate the image
> +descriptor array and also displayed as part of the ESRT table. The
> +image index values defined in the array should be one greater than the
> +dfu alt number that corresponds to the firmware image. So, if the dfu
> +alt number for an image is 2, the value of image index in the
> +fw_images array for that image should be 3. The dfu alt number can be
> +obtained by running the following command::
> +
> +    dfu list
> +
> +
>  Finally, the capsule update can be initiated by rebooting the board.
>  
>  Enabling Capsule Authentication
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/8] capsule: Put a check for image index before the update
  2022-03-31 13:27 ` [PATCH v4 3/8] capsule: Put a check for image index before the update Sughosh Ganu
@ 2022-03-31 18:47   ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 18:47 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

On Thu, Mar 31, 2022 at 06:57:45PM +0530, Sughosh Ganu wrote:
> The current capsule update code compares the image GUID value in the
> capsule header with the image GUID value obtained from the
> GetImageInfo function of the Firmware Management Protocol(FMP). This
> comparison is done to ascertain if the FMP's SetImage function can be
> called for the update. Make this checking more robust by comparing the
> image_index value passed through the capsule with that returned by the
> FMP's GetImageInfo function. This protects against the scenario of the
> firmware being updated in a wrong partition/location on the storage
> device if an incorrect value has been passed through the capsule,
> since the image_index is used to determine the location of the update
> on the storage device.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
> 
> Changes since V3: None
> 
>  lib/efi_loader/efi_capsule.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index f00440163d..f03f4c9044 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -128,6 +128,7 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
>  /**
>   * efi_fmp_find - search for Firmware Management Protocol drivers
>   * @image_type:		Image type guid
> + * @image_index:	Image Index
>   * @instance:		Instance number
>   * @handles:		Handles of FMP drivers
>   * @no_handles:		Number of handles
> @@ -141,8 +142,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
>   * * NULL		- on failure
>   */
>  static struct efi_firmware_management_protocol *
> -efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
> -	     efi_uintn_t no_handles)
> +efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance,
> +	     efi_handle_t *handles, efi_uintn_t no_handles)
>  {
>  	efi_handle_t *handle;
>  	struct efi_firmware_management_protocol *fmp;
> @@ -203,6 +204,7 @@ efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
>  			log_debug("+++ desc[%d] index: %d, name: %ls\n",
>  				  j, desc->image_index, desc->image_id_name);
>  			if (!guidcmp(&desc->image_type_id, image_type) &&
> +			    (desc->image_index == image_index) &&
>  			    (!instance ||
>  			     !desc->hardware_instance ||
>  			      desc->hardware_instance == instance))
> @@ -449,8 +451,8 @@ static efi_status_t efi_capsule_update_firmware(
>  		}
>  
>  		/* find a device for update firmware */
> -		/* TODO: should we pass index as well, or nothing but type? */
>  		fmp = efi_fmp_find(&image->update_image_type_id,
> +				   image->update_image_index,
>  				   image->update_hardware_instance,
>  				   handles, no_handles);
>  		if (!fmp) {
> -- 
> 2.25.1
> 

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
  2022-03-31 13:27 ` [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled Sughosh Ganu
  2022-03-31 14:08   ` Masami Hiramatsu
@ 2022-03-31 19:35   ` Ilias Apalodimas
  2022-04-01  6:58     ` Sughosh Ganu
  1 sibling, 1 reply; 20+ messages in thread
From: Ilias Apalodimas @ 2022-03-31 19:35 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

Hi Sughosh, 

Some nots below

On Thu, Mar 31, 2022 at 06:57:46PM +0530, Sughosh Ganu wrote:
> Currently, there are a bunch of boards which enable the UEFI capsule
> update feature. The actual update of the firmware images is done
> through the dfu framework which uses the dfu_alt_info environment
> variable for getting information on the update, like device, partition
> number/address etc. Currently, these boards define the dfu_alt_info
> variable in the board config header, as an environment variable. With
> this, the variable can be modified from the u-boot command line and
> this can cause an incorrect update.
> 
> To prevent this from happening, define the set_dfu_alt_info function
> in the board file, and select SET_DFU_ALT_INFO for all platforms which
> enable the capsule update feature. With the function defined, the dfu
> framework populates the dfu_alt_info variable through the board file,
> instead of fetching the variable from the environment, thus making the
> update more robust.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> 
> Changes since V3:
> * Do not remove the existing dfu_alt_info definitions made by
>   platforms in the config files, as discussed with Masami.
> * Squash the selection of the SET_DFU_ALT_INFO config symbol for
>   capsule update feature as part of this patch.
> 
> 
>  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 24 +++++++++++++++++
>  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 24 +++++++++++++++++
>  board/emulation/common/qemu_dfu.c             |  6 ++---
>  board/kontron/pitx_imx8m/pitx_imx8m.c         | 24 +++++++++++++++++
>  board/kontron/sl-mx8mm/sl-mx8mm.c             | 24 +++++++++++++++++
>  board/kontron/sl28/sl28.c                     | 25 ++++++++++++++++++
>  board/sandbox/sandbox.c                       | 26 +++++++++++++++++++
>  board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++++
>  board/xilinx/zynq/board.c                     |  5 ++--
>  board/xilinx/zynqmp/zynqmp.c                  |  5 ++--
>  lib/efi_loader/Kconfig                        |  2 ++
>  11 files changed, 184 insertions(+), 7 deletions(-)
> 
> diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> index 1c953ba195..41154ca9f3 100644
> --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> @@ -5,10 +5,12 @@
>   */
>  
>  #include <common.h>
> +#include <dfu.h>
>  #include <dwc3-uboot.h>
>  #include <efi.h>
>  #include <efi_loader.h>
>  #include <errno.h>
> +#include <memalign.h>
>  #include <miiphy.h>
>  #include <netdev.h>
>  #include <spl.h>
> @@ -24,6 +26,7 @@
>  #include <asm/mach-imx/dma.h>
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
> +#include <linux/sizes.h>
>  #include <power/pmic.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
>  	}
>  }
>  #endif /* CONFIG_SPL_MMC_SUPPORT */
> +
> +#if defined(CONFIG_SET_DFU_ALT_INFO)
> +
> +#define DFU_ALT_BUF_LEN		SZ_1K
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> +	if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> +	    env_get("dfu_alt_info"))
> +		return;

Just add a helper function with this since we need to repeat it for every
board. Something like 'bool needs_runtime_dfu_alt_info() '
> +
> +	memset(buf, 0, DFU_ALT_BUF_LEN);

I'd prefer sizeof(buf) instead of explicitly calling the length.

Otherwise LGTM, but I'd prefer if board maintainer had a look as well 

Thanks
/Ilias

> +
> +	snprintf(buf, DFU_ALT_BUF_LEN,
> +		 "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1");
> +
> +	env_set("dfu_alt_info", buf);
> +}
> +#endif /* CONFIG_SET_DFU_ALT_INFO */

[...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
  2022-03-31 19:35   ` Ilias Apalodimas
@ 2022-04-01  6:58     ` Sughosh Ganu
  2022-04-01  9:55       ` Ilias Apalodimas
  0 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2022-04-01  6:58 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

hi Ilias,

On Fri, 1 Apr 2022 at 01:05, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sughosh,
>
> Some nots below
>
> On Thu, Mar 31, 2022 at 06:57:46PM +0530, Sughosh Ganu wrote:
> > Currently, there are a bunch of boards which enable the UEFI capsule
> > update feature. The actual update of the firmware images is done
> > through the dfu framework which uses the dfu_alt_info environment
> > variable for getting information on the update, like device, partition
> > number/address etc. Currently, these boards define the dfu_alt_info
> > variable in the board config header, as an environment variable. With
> > this, the variable can be modified from the u-boot command line and
> > this can cause an incorrect update.
> >
> > To prevent this from happening, define the set_dfu_alt_info function
> > in the board file, and select SET_DFU_ALT_INFO for all platforms which
> > enable the capsule update feature. With the function defined, the dfu
> > framework populates the dfu_alt_info variable through the board file,
> > instead of fetching the variable from the environment, thus making the
> > update more robust.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >
> > Changes since V3:
> > * Do not remove the existing dfu_alt_info definitions made by
> >   platforms in the config files, as discussed with Masami.
> > * Squash the selection of the SET_DFU_ALT_INFO config symbol for
> >   capsule update feature as part of this patch.
> >
> >
> >  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 24 +++++++++++++++++
> >  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 24 +++++++++++++++++
> >  board/emulation/common/qemu_dfu.c             |  6 ++---
> >  board/kontron/pitx_imx8m/pitx_imx8m.c         | 24 +++++++++++++++++
> >  board/kontron/sl-mx8mm/sl-mx8mm.c             | 24 +++++++++++++++++
> >  board/kontron/sl28/sl28.c                     | 25 ++++++++++++++++++
> >  board/sandbox/sandbox.c                       | 26 +++++++++++++++++++
> >  board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++++
> >  board/xilinx/zynq/board.c                     |  5 ++--
> >  board/xilinx/zynqmp/zynqmp.c                  |  5 ++--
> >  lib/efi_loader/Kconfig                        |  2 ++
> >  11 files changed, 184 insertions(+), 7 deletions(-)
> >
> > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > index 1c953ba195..41154ca9f3 100644
> > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > @@ -5,10 +5,12 @@
> >   */
> >
> >  #include <common.h>
> > +#include <dfu.h>
> >  #include <dwc3-uboot.h>
> >  #include <efi.h>
> >  #include <efi_loader.h>
> >  #include <errno.h>
> > +#include <memalign.h>
> >  #include <miiphy.h>
> >  #include <netdev.h>
> >  #include <spl.h>
> > @@ -24,6 +26,7 @@
> >  #include <asm/mach-imx/dma.h>
> >  #include <linux/delay.h>
> >  #include <linux/kernel.h>
> > +#include <linux/sizes.h>
> >  #include <power/pmic.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> > @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
> >       }
> >  }
> >  #endif /* CONFIG_SPL_MMC_SUPPORT */
> > +
> > +#if defined(CONFIG_SET_DFU_ALT_INFO)
> > +
> > +#define DFU_ALT_BUF_LEN              SZ_1K
> > +
> > +void set_dfu_alt_info(char *interface, char *devstr)
> > +{
> > +     ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > +
> > +     if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> > +         env_get("dfu_alt_info"))
> > +             return;
>
> Just add a helper function with this since we need to repeat it for every
> board. Something like 'bool needs_runtime_dfu_alt_info() '

Okay

> > +
> > +     memset(buf, 0, DFU_ALT_BUF_LEN);
>
> I'd prefer sizeof(buf) instead of explicitly calling the length.

So the current boards are using this wrong. The
ALLOC_CACHE_ALIGN_BUFFER actually defines an array named __buf, and a
char *buf which points to the array. So the existing code in some
boards was zeroing out only the sizeof(char *) number of bytes. The
sandbox clang build throws up this warning, which is how I found it.

-sughosh

>
> Otherwise LGTM, but I'd prefer if board maintainer had a look as well
>
> Thanks
> /Ilias
>
> > +
> > +     snprintf(buf, DFU_ALT_BUF_LEN,
> > +              "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1");
> > +
> > +     env_set("dfu_alt_info", buf);
> > +}
> > +#endif /* CONFIG_SET_DFU_ALT_INFO */
>
> [...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
  2022-04-01  6:58     ` Sughosh Ganu
@ 2022-04-01  9:55       ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2022-04-01  9:55 UTC (permalink / raw)
  To: Sughosh Ganu
  Cc: u-boot, Heinrich Schuchardt, AKASHI Takahiro, Ying-Chun Liu,
	Tuomas Tynkkynen, Heiko Thiery, Frieder Schrempf, Michael Walle,
	Masami Hiramatsu, Jassi Brar, Michal Simek, Michal Simek

Hi Sughosh

On Fri, 1 Apr 2022 at 09:58, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Ilias,
>
> On Fri, 1 Apr 2022 at 01:05, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Sughosh,
> >
> > Some nots below
> >
> > On Thu, Mar 31, 2022 at 06:57:46PM +0530, Sughosh Ganu wrote:
> > > Currently, there are a bunch of boards which enable the UEFI capsule
> > > update feature. The actual update of the firmware images is done
> > > through the dfu framework which uses the dfu_alt_info environment
> > > variable for getting information on the update, like device, partition
> > > number/address etc. Currently, these boards define the dfu_alt_info
> > > variable in the board config header, as an environment variable. With
> > > this, the variable can be modified from the u-boot command line and
> > > this can cause an incorrect update.
> > >
> > > To prevent this from happening, define the set_dfu_alt_info function
> > > in the board file, and select SET_DFU_ALT_INFO for all platforms which
> > > enable the capsule update feature. With the function defined, the dfu
> > > framework populates the dfu_alt_info variable through the board file,
> > > instead of fetching the variable from the environment, thus making the
> > > update more robust.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > >
> > > Changes since V3:
> > > * Do not remove the existing dfu_alt_info definitions made by
> > >   platforms in the config files, as discussed with Masami.
> > > * Squash the selection of the SET_DFU_ALT_INFO config symbol for
> > >   capsule update feature as part of this patch.
> > >
> > >
> > >  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 24 +++++++++++++++++
> > >  .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 24 +++++++++++++++++
> > >  board/emulation/common/qemu_dfu.c             |  6 ++---
> > >  board/kontron/pitx_imx8m/pitx_imx8m.c         | 24 +++++++++++++++++
> > >  board/kontron/sl-mx8mm/sl-mx8mm.c             | 24 +++++++++++++++++
> > >  board/kontron/sl28/sl28.c                     | 25 ++++++++++++++++++
> > >  board/sandbox/sandbox.c                       | 26 +++++++++++++++++++
> > >  board/socionext/developerbox/developerbox.c   | 26 +++++++++++++++++++
> > >  board/xilinx/zynq/board.c                     |  5 ++--
> > >  board/xilinx/zynqmp/zynqmp.c                  |  5 ++--
> > >  lib/efi_loader/Kconfig                        |  2 ++
> > >  11 files changed, 184 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > > index 1c953ba195..41154ca9f3 100644
> > > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
> > > @@ -5,10 +5,12 @@
> > >   */
> > >
> > >  #include <common.h>
> > > +#include <dfu.h>
> > >  #include <dwc3-uboot.h>
> > >  #include <efi.h>
> > >  #include <efi_loader.h>
> > >  #include <errno.h>
> > > +#include <memalign.h>
> > >  #include <miiphy.h>
> > >  #include <netdev.h>
> > >  #include <spl.h>
> > > @@ -24,6 +26,7 @@
> > >  #include <asm/mach-imx/dma.h>
> > >  #include <linux/delay.h>
> > >  #include <linux/kernel.h>
> > > +#include <linux/sizes.h>
> > >  #include <power/pmic.h>
> > >
> > >  DECLARE_GLOBAL_DATA_PTR;
> > > @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc)
> > >       }
> > >  }
> > >  #endif /* CONFIG_SPL_MMC_SUPPORT */
> > > +
> > > +#if defined(CONFIG_SET_DFU_ALT_INFO)
> > > +
> > > +#define DFU_ALT_BUF_LEN              SZ_1K
> > > +
> > > +void set_dfu_alt_info(char *interface, char *devstr)
> > > +{
> > > +     ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > > +
> > > +     if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) &&
> > > +         env_get("dfu_alt_info"))
> > > +             return;
> >
> > Just add a helper function with this since we need to repeat it for every
> > board. Something like 'bool needs_runtime_dfu_alt_info() '
>
> Okay
>
> > > +
> > > +     memset(buf, 0, DFU_ALT_BUF_LEN);
> >
> > I'd prefer sizeof(buf) instead of explicitly calling the length.
>
> So the current boards are using this wrong. The
> ALLOC_CACHE_ALIGN_BUFFER actually defines an array named __buf, and a
> char *buf which points to the array. So the existing code in some
> boards was zeroing out only the sizeof(char *) number of bytes. The
> sandbox clang build throws up this warning, which is how I found it.
>
> -sughosh

Ah right, then that's fine.

Thinking about the common function a bit more can we slightly change
what you have?
On the fw_images struct add a const char* with the dfu_alt_str.
Instead of defining a string literal and snprintf it, use the per
board struct definition and get the string you need.  So the common
case can be abstracted in a __weak function for all the boards to use
and if some platform needs something more it can replace it with it's
own function at link time.

Cheers
/Ilias
>
> >
> > Otherwise LGTM, but I'd prefer if board maintainer had a look as well
> >
> > Thanks
> > /Ilias
> >
> > > +
> > > +     snprintf(buf, DFU_ALT_BUF_LEN,
> > > +              "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1");
> > > +
> > > +     env_set("dfu_alt_info", buf);
> > > +}
> > > +#endif /* CONFIG_SET_DFU_ALT_INFO */
> >
> > [...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-04-01  9:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 13:27 [PATCH v4 0/8] efi: capsule: Capsule Update fixes and enhancements Sughosh Ganu
2022-03-31 13:27 ` [PATCH v4 1/8] capsule: Add Image GUIDs and image index for platforms using capsule updates Sughosh Ganu
2022-03-31 14:03   ` Ilias Apalodimas
2022-03-31 13:27 ` [PATCH v4 2/8] capsule: FMP: Populate the image descriptor array from platform data Sughosh Ganu
2022-03-31 15:08   ` Ilias Apalodimas
2022-03-31 16:29     ` Sughosh Ganu
2022-03-31 13:27 ` [PATCH v4 3/8] capsule: Put a check for image index before the update Sughosh Ganu
2022-03-31 18:47   ` Ilias Apalodimas
2022-03-31 13:27 ` [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled Sughosh Ganu
2022-03-31 14:08   ` Masami Hiramatsu
2022-03-31 19:35   ` Ilias Apalodimas
2022-04-01  6:58     ` Sughosh Ganu
2022-04-01  9:55       ` Ilias Apalodimas
2022-03-31 13:27 ` [PATCH v4 5/8] test: capsule: Modify the capsule tests to use GUID values for sandbox Sughosh Ganu
2022-03-31 13:27 ` [PATCH v4 6/8] FMP: Remove GUIDs for FIT and raw images Sughosh Ganu
2022-03-31 18:10   ` Ilias Apalodimas
2022-03-31 13:27 ` [PATCH v4 7/8] mkeficapsule: Remove raw and FIT GUID types Sughosh Ganu
2022-03-31 18:10   ` Ilias Apalodimas
2022-03-31 13:27 ` [PATCH v4 8/8] doc: uefi: Update the capsule update related documentation Sughosh Ganu
2022-03-31 18:14   ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.