* [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
@ 2024-05-31 13:50 Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
` (8 more replies)
0 siblings, 9 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
As more boards adopt support for the EFI CapsuleUpdate mechanism, there
is a growing issue of being able to target updates to them properly. The
current mechanism of hardcoding UUIDs for each board at compile time is
unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
In this series, I propose that we adopt v5 GUIDs, these are generated
by using a well-known salt GUID as well as board specific information
(like the model/revision), these are hashed together and the result is
truncated to form a new UUID.
The well-known salt GUID can be specific to the architecture (SoC
vendor), or OEM. It is defined in the board defconfig so that vendors
can easily bring their own.
Specifically, the following fields are used to generate a GUID for a
particular fw_image:
* namespace salt
* board compatible (usually the first entry in the dt root compatible
array).
* fw_image name (the string identifying the specific image, especially
relevant for board that can update multiple images).
== Usage ==
Boards can integrate dynamic UUID support as follows:
1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if
EFI_HAVE_CAPSULE_SUPPORT.
2. Skip setting the fw_images image_type_id property.
3. Generate a UUID and set CONFIG_EFI_CAPSULE_NAMESPACE_UUID in your
defconfig.
== Limitations ==
* Changing GUIDs
The primary limitation with this approach is that if any of the source
fields change, so will the GUID for the board. It is therefore pretty
important to ensure that GUID changes are caught during development.
* Supporting multiple boards with a single image
This now requires having an entry with the GUID for every board which
might lead to larger UpdateCapsule images.
== Tooling ==
This series introduces a new tool: genguid. This can be used to generate
the same GUIDs that the board would at runtime.
This series follows a related discussion started by Ilias:
https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72WkYLizrw@mail.gmail.com/
---
Changes in v3:
- Add manpage for genguid
- Add dedicated CONFIG_TOOLS_GENGUID option
- Minor code fixes addressing v2 feedback
- Link to v2: https://lore.kernel.org/r/20240529-b4-dynamic-uuid-v2-0-c26f31057bbe@linaro.org
Changes in v2:
- Move namespace UUID to be defined in defconfig
- Add tests and tooling
- Only use the first board compatible to generate UUID.
- Link to v1: https://lore.kernel.org/r/20240426-b4-dynamic-uuid-v1-0-e8154e00ec44@linaro.org
---
Caleb Connolly (7):
lib: uuid: add UUID v5 support
efi: add a helper to generate dynamic UUIDs
doc: uefi: document dynamic UUID generation
sandbox: switch to dynamic UUIDs
lib: uuid: supporting building as part of host tools
tools: add genguid tool
test: lib/uuid: add unit tests for dynamic UUIDs
arch/Kconfig | 1 +
board/sandbox/sandbox.c | 16 ---
configs/sandbox_defconfig | 1 +
configs/sandbox_flattree_defconfig | 1 +
doc/develop/uefi/uefi.rst | 31 +++++
doc/genguid.1 | 52 +++++++
include/sandbox_efi_capsule.h | 6 +-
include/uuid.h | 21 ++-
lib/Kconfig | 8 ++
lib/efi_loader/Kconfig | 23 +++
lib/efi_loader/efi_capsule.c | 1 +
lib/efi_loader/efi_firmware.c | 66 +++++++++
lib/uuid.c | 81 +++++++++--
test/lib/uuid.c | 88 ++++++++++++
.../test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
.../test_efi_capsule/test_capsule_firmware_raw.py | 8 +-
.../test_capsule_firmware_signed_fit.py | 2 +-
.../test_capsule_firmware_signed_raw.py | 4 +-
test/py/tests/test_efi_capsule/version.dts | 6 +-
tools/Kconfig | 7 +
tools/Makefile | 3 +
tools/binman/etype/efi_capsule.py | 2 +-
tools/binman/ftest.py | 2 +-
tools/genguid.c | 154 +++++++++++++++++++++
24 files changed, 538 insertions(+), 48 deletions(-)
---
change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27
base-commit: 2e682a4a406fc81ef32e05c28542cc8067f1e15f
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 1/7] lib: uuid: add UUID v5 support
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-05-31 16:11 ` Ilias Apalodimas
` (2 more replies)
2024-05-31 13:50 ` [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs Caleb Connolly
` (7 subsequent siblings)
8 siblings, 3 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Add support for generating version 5 UUIDs, these are determistic and work
by hashing a "namespace" UUID together with some unique data. One intended
usecase is to allow for dynamically generate payload UUIDs for UEFI
capsule updates, so that supported boards can have their own UUIDs
without needing to hardcode them.
Tests for this are added in an upcoming patch.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
include/uuid.h | 17 +++++++++++++++++
lib/Kconfig | 8 ++++++++
lib/uuid.c | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/include/uuid.h b/include/uuid.h
index f5a941250f48..539affaa47b9 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -10,8 +10,9 @@
#ifndef __UUID_H__
#define __UUID_H__
#include <linux/bitops.h>
+#include <linux/kconfig.h>
/*
* UUID - Universally Unique IDentifier - 128 bits unique number.
* There are 5 versions and one variant of UUID defined by RFC4122
@@ -142,8 +143,24 @@ void gen_rand_uuid(unsigned char *uuid_bin);
* @param - uuid output type: UUID - 0, GUID - 1
*/
void gen_rand_uuid_str(char *uuid_str, int str_format);
+#if IS_ENABLED(CONFIG_UUID_GEN_V5)
+/**
+ * gen_uuid_v5() - generate UUID v5 from namespace and other seed data.
+ *
+ * @namespace: pointer to UUID namespace salt
+ * @uuid: pointer to allocated UUID output
+ * @...: NULL terminated list of seed data as pairs of pointers
+ * to data and their lengths
+ */
+void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...);
+#else
+static inline void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
+{
+}
+#endif
+
/**
* uuid_str_to_le_bin() - Convert string UUID to little endian binary data.
* @uuid_str: pointer to UUID string
* @uuid_bin: pointer to allocated array for little endian output [16B]
diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa1..2941532f25cf 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -80,8 +80,16 @@ config RANDOM_UUID
help
Enable the generation of partitions with random UUIDs if none
are provided.
+config UUID_GEN_V5
+ bool "Enable UUID version 5 generation"
+ select LIB_UUID
+ depends on SHA1
+ help
+ Enable the generation of version 5 UUIDs, these are determistic and
+ generated from a namespace UUID, and a string (such as a board name).
+
config SPL_LIB_UUID
depends on SPL
bool
diff --git a/lib/uuid.c b/lib/uuid.c
index dfa2320ba267..2df0523e717f 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -21,8 +21,9 @@
#include <part_efi.h>
#include <malloc.h>
#include <dm/uclass.h>
#include <rng.h>
+#include <u-boot/sha1.h>
int uuid_str_valid(const char *uuid)
{
int i, valid;
@@ -368,8 +369,44 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str,
}
}
}
+#if IS_ENABLED(CONFIG_UUID_GEN_V5)
+void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
+{
+ sha1_context ctx;
+ va_list args;
+ const uint8_t *data;
+ uint8_t hash[SHA1_SUM_LEN];
+ uint32_t tmp;
+
+ sha1_starts(&ctx);
+ /* Hash the namespace UUID as salt */
+ sha1_update(&ctx, (unsigned char *)namespace, UUID_BIN_LEN);
+ va_start(args, uuid);
+
+ while ((data = va_arg(args, const uint8_t *))) {
+ unsigned int len = va_arg(args, size_t);
+
+ sha1_update(&ctx, data, len);
+ }
+
+ va_end(args);
+ sha1_finish(&ctx, hash);
+
+ /* Truncate the hash into output UUID, it is already big endian */
+ memcpy(uuid, hash, sizeof(*uuid));
+
+ /* Configure variant/version bits */
+ tmp = be32_to_cpu(uuid->time_hi_and_version);
+ tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
+ uuid->time_hi_and_version = cpu_to_be32(tmp);
+
+ uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
+ uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
+}
+#endif
+
#if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
void gen_rand_uuid(unsigned char *uuid_bin)
{
u32 ptr[4];
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-06-05 5:52 ` Heinrich Schuchardt
2024-05-31 13:50 ` [PATCH v3 3/7] doc: uefi: document dynamic UUID generation Caleb Connolly
` (6 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Introduce a new helper efi_capsule_update_info_gen_ids() which populates
the capsule update fw images image_type_id field. This allows for
determinstic UUIDs to be used that can scale to a large number of
different boards and board variants without the need to maintain a big
list.
We call this from efi_fill_image_desc_array() to populate the UUIDs
lazily on-demand.
This is behind an additional config option as it depends on V5 UUIDs and
the SHA1 implementation.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
lib/efi_loader/Kconfig | 23 +++++++++++++++
lib/efi_loader/efi_capsule.c | 1 +
lib/efi_loader/efi_firmware.c | 66 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 90 insertions(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 430bb7f0f7dc..e90caf4f8e14 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -235,8 +235,31 @@ config EFI_CAPSULE_ON_DISK_EARLY
If this option is enabled, capsules will be enforced to be
executed as part of U-Boot initialisation so that they will
surely take place whatever is set to distro_bootcmd.
+config EFI_CAPSULE_DYNAMIC_UUIDS
+ bool "Dynamic UUIDs for capsules"
+ depends on EFI_HAVE_CAPSULE_SUPPORT
+ select UUID_GEN_V5
+ help
+ Select this option if you want to use dynamically generated v5
+ UUIDs for your board. To make use of this feature, your board
+ code should call efi_capsule_update_info_gen_ids() with a seed
+ UUID to generate the image_type_id field for each fw_image.
+
+ The CapsuleUpdate payloads are expected to generate matching UUIDs
+ using the same scheme.
+
+config EFI_CAPSULE_NAMESPACE_UUID
+ string "Namespace UUID for dynamic UUIDs"
+ depends on EFI_CAPSULE_DYNAMIC_UUIDS
+ help
+ Define the namespace or "salt" UUID used to generate the per-image
+ UUIDs. This should be a UUID in the standard 8-4-4-4-12 format.
+
+ Device vendors are expected to generate their own namespace UUID
+ to avoid conflicts with existing products.
+
config EFI_CAPSULE_FIRMWARE
bool
config EFI_CAPSULE_FIRMWARE_MANAGEMENT
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 0937800e588f..ac02e79ae7d8 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -19,8 +19,9 @@
#include <mapmem.h>
#include <sort.h>
#include <sysreset.h>
#include <asm/global_data.h>
+#include <uuid.h>
#include <crypto/pkcs7.h>
#include <crypto/pkcs7_parser.h>
#include <linux/err.h>
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index ba5aba098c0f..a8dafe4f01a5 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -244,8 +244,71 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
free(var_state);
}
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
+/**
+ * efi_capsule_update_info_gen_ids - generate GUIDs for the images
+ *
+ * Generate the image_type_id for each image in the update_info.images array
+ * using the first compatible from the device tree and a salt
+ * UUID defined at build time.
+ *
+ * Returns: status code
+ */
+static efi_status_t efi_capsule_update_info_gen_ids(void)
+{
+ int ret, i;
+ struct uuid namespace;
+ const char *compatible; /* Full array including null bytes */
+ struct efi_fw_image *fw_array;
+
+ fw_array = update_info.images;
+ /* Check if we need to run (there are images and we didn't already generate their IDs) */
+ if (!update_info.num_images ||
+ memchr_inv(&fw_array[0].image_type_id, 0, sizeof(fw_array[0].image_type_id)))
+ return EFI_SUCCESS;
+
+ ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
+ (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
+ if (ret) {
+ log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid: %d\n", __func__, ret);
+ return EFI_UNSUPPORTED;
+ }
+
+ compatible = ofnode_read_string(ofnode_root(), "compatible");
+
+ if (!compatible) {
+ log_debug("%s: model or compatible not defined\n", __func__);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (!update_info.num_images) {
+ log_debug("%s: no fw_images, make sure update_info.num_images is set\n", __func__);
+ return -ENODATA;
+ }
+
+ for (i = 0; i < update_info.num_images; i++) {
+ gen_uuid_v5(&namespace,
+ (struct uuid *)&fw_array[i].image_type_id,
+ compatible, strlen(compatible),
+ fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
+ - sizeof(uint16_t),
+ NULL);
+
+ log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
+ &fw_array[i].image_type_id);
+ }
+
+ return EFI_SUCCESS;
+}
+#else
+static efi_status_t efi_capsule_update_info_gen_ids(void)
+{
+ return EFI_SUCCESS;
+}
+#endif
+
/**
* efi_fill_image_desc_array - populate image descriptor array
* @image_info_size: Size of @image_info
* @image_info: Image information
@@ -282,8 +345,11 @@ static efi_status_t efi_fill_image_desc_array(
return EFI_BUFFER_TOO_SMALL;
}
*image_info_size = total_size;
+ if (efi_capsule_update_info_gen_ids() != EFI_SUCCESS)
+ return EFI_UNSUPPORTED;
+
fw_array = update_info.images;
*descriptor_count = update_info.num_images;
*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
*descriptor_size = sizeof(*image_info);
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 3/7] doc: uefi: document dynamic UUID generation
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 4/7] sandbox: switch to dynamic UUIDs Caleb Connolly
` (5 subsequent siblings)
8 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Document how platforms can generate GUIDs at runtime rather than
maintaining a list of UUIDs per-board.
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
doc/develop/uefi/uefi.rst | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 0389b269c01b..0b60702c052a 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -448,8 +448,39 @@ the location of the firmware updates is not a very secure
practice. Getting this information from the firmware itself is more
secure, assuming the firmware has been verified by a previous stage
boot loader.
+The image_type_id contains a GUID value which is specific to the image
+and board being updated, that is to say it should uniquely identify the
+board model (and revision if relevant) and image pair. Traditionally,
+these GUIDs are generated manually and hardcoded on a per-board basis,
+however this scheme makes it difficult to scale up to support many
+boards.
+
+To address this, v5 GUIDs can be used to generate board-specific GUIDs
+at runtime, based on a set of persistent identifiable information:
+
+.. code-block:: c
+
+ /**
+ * efi_capsule_update_info_gen_ids - generate GUIDs for the images
+ *
+ * Generate the image_type_id for each image in the update_info.images array
+ * using the model and compatible strings from the device tree and a salt
+ * UUID defined at build time.
+ *
+ * Returns: status code
+ */
+ static efi_status_t efi_capsule_update_info_gen_ids(void);
+
+These strings are combined with the fw_image name to generate GUIDs for
+each image. Support for dynamic UUIDs can be enabled by turning on
+CONFIG_EFI_CAPSULE_DYNAMIC_UUIDS, generating a new namespace UUID and
+setting CONFIG_EFI_CAPSULE_NAMESPACE_UUID to it.
+
+The genguid tool can be used to determine the GUIDs for a particular board
+and image. It can be found in the tools directory.
+
The firmware images structure defines the GUID values, image index
values and the name of the images that are to be updated through
the capsule update feature. These values are to be defined as part of
an array. These GUID values would be used by the Firmware Management
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 4/7] sandbox: switch to dynamic UUIDs
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (2 preceding siblings ...)
2024-05-31 13:50 ` [PATCH v3 3/7] doc: uefi: document dynamic UUID generation Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-05-31 15:43 ` Ilias Apalodimas
2024-05-31 13:50 ` [PATCH v3 5/7] lib: uuid: supporting building as part of host tools Caleb Connolly
` (4 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Migrate sandbox over to generating it's capsule update image GUIDs
dynamically from the namespace and board/image info. Update the
reference and tests to use the new GUIDs.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
arch/Kconfig | 1 +
board/sandbox/sandbox.c | 16 ----------------
configs/sandbox_defconfig | 1 +
configs/sandbox_flattree_defconfig | 1 +
include/sandbox_efi_capsule.h | 6 +++---
.../tests/test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
.../tests/test_efi_capsule/test_capsule_firmware_raw.py | 8 ++++----
.../test_efi_capsule/test_capsule_firmware_signed_fit.py | 2 +-
.../test_efi_capsule/test_capsule_firmware_signed_raw.py | 4 ++--
test/py/tests/test_efi_capsule/version.dts | 6 +++---
tools/binman/etype/efi_capsule.py | 2 +-
tools/binman/ftest.py | 2 +-
12 files changed, 19 insertions(+), 32 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index abd406d48841..0558c90540b6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -164,8 +164,9 @@ config SANDBOX
select SYS_CACHE_SHIFT_4
select IRQ
select SUPPORT_EXTENSION_SCAN if CMDLINE
select SUPPORT_ACPI
+ select EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT
imply BITREVERSE
select BLOBLIST
imply LTO
imply CMD_DM
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 802596569c64..d97945e58fcf 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -31,36 +31,20 @@
*/
gd_t *gd;
#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)
-/* GUIDs for capsule updatable firmware images */
-#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)
-
struct efi_fw_image 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
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 93b52f2de5cf..58775b271600 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -349,8 +349,9 @@ CONFIG_TPM=y
CONFIG_ERRNO_STR=y
CONFIG_GETOPT=y
CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_NAMESPACE_UUID="09D7CF52-0720-4710-91D1-08469B7FE9C8"
CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
CONFIG_EFI_CAPSULE_ESL_FILE="board/sandbox/capsule_pub_esl_good.esl"
CONFIG_EFI_SECURE_BOOT=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 6bf8874e722e..85ae63da8881 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -224,8 +224,9 @@ CONFIG_TPM=y
CONFIG_ZSTD=y
CONFIG_ERRNO_STR=y
CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_NAMESPACE_UUID="09d7cf52-0720-4710-91d1-08469b7fe9c8"
CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
CONFIG_EFI_CAPSULE_ESL_FILE="board/sandbox/capsule_pub_esl_good.esl"
CONFIG_UNIT_TEST=y
diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h
index 3e288e8a84a2..25ac496ea24f 100644
--- a/include/sandbox_efi_capsule.h
+++ b/include/sandbox_efi_capsule.h
@@ -5,11 +5,11 @@
#if !defined(_SANDBOX_EFI_CAPSULE_H_)
#define _SANDBOX_EFI_CAPSULE_H_
-#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8"
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0"
-#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937"
+#define SANDBOX_UBOOT_IMAGE_GUID "fd5db83c-12f3-a46b-80a9-e3007c7ff56e"
+#define SANDBOX_UBOOT_ENV_IMAGE_GUID "935fe837-fac8-4394-c008-737d8852c60d"
+#define SANDBOX_FIT_IMAGE_GUID "ffd97379-0956-fa94-c003-8bfcf5cc097b"
#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4"
#define UBOOT_FIT_IMAGE "u-boot_bin_env.itb"
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
index 11bcdc2bb293..746da4602085 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
@@ -146,9 +146,9 @@ class TestEfiCapsuleFirmwareFit():
verify_content(u_boot_console, '100000', 'u-boot:Old')
verify_content(u_boot_console, '150000', 'u-boot-env:Old')
else:
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '3673B45D-6A7C-46F3-9E60-ADABB03F7937' in ''.join(output)
+ assert '5AF91295-5A99-F62B-80D7-E9574DE87170' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
verify_content(u_boot_console, '100000', 'u-boot:New')
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
index a5b5c8a3853a..1866b8086573 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
@@ -133,12 +133,12 @@ class TestEfiCapsuleFirmwareRaw:
'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 SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
+ assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
+ assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
check_file_removed(u_boot_console, disk_img, capsule_files)
expected = 'u-boot:Old' if capsule_auth else 'u-boot:New'
@@ -187,14 +187,14 @@ class TestEfiCapsuleFirmwareRaw:
verify_content(u_boot_console, '100000', 'u-boot:Old')
verify_content(u_boot_console, '150000', 'u-boot-env:Old')
else:
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
+ assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
# ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
+ assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
assert 'ESRT: fw_version=10' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
verify_content(u_boot_console, '100000', 'u-boot:New')
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
index 44a58baa3106..a4e0a3bc73f5 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
@@ -156,9 +156,9 @@ class TestEfiCapsuleFirmwareSignedFit():
'u-boot-env raw 0x150000 0x200000"',
'efidebug capsule esrt'])
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '3673B45D-6A7C-46F3-9E60-ADABB03F7937' in ''.join(output)
+ assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
verify_content(u_boot_console, '100000', 'u-boot:New')
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
index 83a10e160b8c..260c71860632 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
@@ -150,14 +150,14 @@ class TestEfiCapsuleFirmwareSignedRaw():
'u-boot-env raw 0x150000 0x200000"',
'efidebug capsule esrt'])
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
+ assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
# ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
+ assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
assert 'ESRT: fw_version=10' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
verify_content(u_boot_console, '100000', 'u-boot:New')
diff --git a/test/py/tests/test_efi_capsule/version.dts b/test/py/tests/test_efi_capsule/version.dts
index 07850cc6064c..3f0698bf7280 100644
--- a/test/py/tests/test_efi_capsule/version.dts
+++ b/test/py/tests/test_efi_capsule/version.dts
@@ -7,18 +7,18 @@
firmware-version {
image1 {
lowest-supported-version = <3>;
image-index = <1>;
- image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+ image-type-id = "FD5DB83C-12F3-A46B-80A9-E3007C7FF56E";
};
image2 {
lowest-supported-version = <7>;
image-index = <2>;
- image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+ image-type-id = "935FE837-FAC8-4394-C008-737D8852C60D";
};
image3 {
lowest-supported-version = <3>;
image-index = <1>;
- image-type-id = "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
+ image-type-id = "FFD97379-0956-FA94-C003-8BFCF5CC097B";
};
};
};
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py
index e32037178221..da1f9b0a381a 100644
--- a/tools/binman/etype/efi_capsule.py
+++ b/tools/binman/etype/efi_capsule.py
@@ -23,9 +23,9 @@ def get_binman_test_guid(type_str):
Returns:
The actual GUID value (str)
"""
TYPE_TO_GUID = {
- 'binman-test' : '09d7cf52-0720-4710-91d1-08469b7fe9c8'
+ 'binman-test' : 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
}
return TYPE_TO_GUID[type_str]
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8a44bc051b36..dc602b95ecd5 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -123,9 +123,9 @@ TEE_ADDR = 0x5678
# Firmware Management Protocol(FMP) GUID
FW_MGMT_GUID = '6dcbd5ed-e82d-4c44-bda1-7194199ad92a'
# Image GUID specified in the DTS
-CAPSULE_IMAGE_GUID = '09d7cf52-0720-4710-91d1-08469b7fe9c8'
+CAPSULE_IMAGE_GUID = 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
# Windows cert GUID
WIN_CERT_TYPE_EFI_GUID = '4aafd29d-68df-49ee-8aa9-347d375665a7'
# Empty capsule GUIDs
EMPTY_CAPSULE_ACCEPT_GUID = '0c996046-bcc0-4d04-85ec-e1fcedf1c6f8'
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 5/7] lib: uuid: supporting building as part of host tools
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (3 preceding siblings ...)
2024-05-31 13:50 ` [PATCH v3 4/7] sandbox: switch to dynamic UUIDs Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
` (3 subsequent siblings)
8 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Adjust the UUID library code so that it can be compiled as part of a
host tool.
This removes the one redundant log_debug() call, as well as the
incorrectly defined LOG_CATEGORY.
In general this is a fairly trivial change, just adjusting includes and
disabling list_guid.
This will be used by a new genguid tool to generate v5 GUIDs that match
those generated by U-Boot at runtime.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
include/uuid.h | 4 ++--
lib/uuid.c | 44 ++++++++++++++++++++++++++++++--------------
2 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/include/uuid.h b/include/uuid.h
index 539affaa47b9..b38b20d957ef 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -69,10 +69,10 @@ struct uuid {
} __packed;
/* Bits of a bitmask specifying the output format for GUIDs */
#define UUID_STR_FORMAT_STD 0
-#define UUID_STR_FORMAT_GUID BIT(0)
-#define UUID_STR_UPPER_CASE BIT(1)
+#define UUID_STR_FORMAT_GUID 0x1
+#define UUID_STR_UPPER_CASE 0x2
/* Use UUID_STR_LEN + 1 for string space */
#define UUID_STR_LEN 36
#define UUID_BIN_LEN sizeof(struct uuid)
diff --git a/lib/uuid.c b/lib/uuid.c
index 2df0523e717f..89911b06ccc0 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -6,25 +6,38 @@
* Authors:
* Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
*/
-#define LOG_CATEGOT LOGC_CORE
-
+#ifndef USE_HOSTCC
#include <command.h>
#include <efi_api.h>
#include <env.h>
#include <rand.h>
#include <time.h>
-#include <uuid.h>
-#include <linux/ctype.h>
-#include <errno.h>
#include <asm/io.h>
#include <part_efi.h>
#include <malloc.h>
#include <dm/uclass.h>
#include <rng.h>
+#include <linux/ctype.h>
+#include <hexdump.h>
+#else
+#include <stdarg.h>
+#include <stdint.h>
+#include <eficapsule.h>
+#include <ctype.h>
+#endif
+#include <linux/types.h>
+#include <errno.h>
+#include <linux/kconfig.h>
+#include <uuid.h>
#include <u-boot/sha1.h>
+#ifdef USE_HOSTCC
+/* polyfill hextoul to avoid pulling in strto.c */
+#define hextoul(cp, endp) strtoul(cp, endp, 16)
+#endif
+
int uuid_str_valid(const char *uuid)
{
int i, valid;
@@ -51,8 +64,9 @@ int uuid_str_valid(const char *uuid)
static const struct {
const char *string;
efi_guid_t guid;
} list_guid[] = {
+#ifndef USE_HOSTCC
#ifdef CONFIG_PARTITION_TYPE_GUID
{"system", PARTITION_SYSTEM_GUID},
{"mbr", LEGACY_MBR_PARTITION_GUID},
{"msft", PARTITION_MSFT_RESERVED_GUID},
@@ -231,8 +245,9 @@ static const struct {
{ "EFI_MEMORY_TYPE", EFI_MEMORY_TYPE },
{ "EFI_MEM_STATUS_CODE_REC", EFI_MEM_STATUS_CODE_REC },
{ "EFI_GUID_EFI_ACPI1", EFI_GUID_EFI_ACPI1 },
#endif
+#endif /* !USE_HOSTCC */
};
int uuid_guid_get_bin(const char *guid_str, unsigned char *guid_bin)
{
@@ -266,9 +281,8 @@ int uuid_str_to_bin(const char *uuid_str, unsigned char *uuid_bin,
uint32_t tmp32;
uint64_t tmp64;
if (!uuid_str_valid(uuid_str)) {
- log_debug("not valid\n");
#ifdef CONFIG_PARTITION_TYPE_GUID
if (!uuid_guid_get_bin(uuid_str, uuid_bin))
return 0;
#endif
@@ -297,19 +311,19 @@ int uuid_str_to_bin(const char *uuid_str, unsigned char *uuid_bin,
tmp16 = cpu_to_be16(hextoul(uuid_str + 19, NULL));
memcpy(uuid_bin + 8, &tmp16, 2);
- tmp64 = cpu_to_be64(simple_strtoull(uuid_str + 24, NULL, 16));
+ tmp64 = cpu_to_be64(hextoul(uuid_str + 24, NULL));
memcpy(uuid_bin + 10, (char *)&tmp64 + 2, 6);
return 0;
}
int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin)
{
- u16 tmp16;
- u32 tmp32;
- u64 tmp64;
+ uint16_t tmp16;
+ uint32_t tmp32;
+ uint64_t tmp64;
if (!uuid_str_valid(uuid_str) || !uuid_bin)
return -EINVAL;
@@ -324,22 +338,22 @@ int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin)
tmp16 = cpu_to_le16(hextoul(uuid_str + 19, NULL));
memcpy(uuid_bin + 8, &tmp16, 2);
- tmp64 = cpu_to_le64(simple_strtoull(uuid_str + 24, NULL, 16));
+ tmp64 = cpu_to_le64(hextoul(uuid_str + 24, NULL));
memcpy(uuid_bin + 10, &tmp64, 6);
return 0;
}
void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str,
int str_format)
{
- const u8 uuid_char_order[UUID_BIN_LEN] = {0, 1, 2, 3, 4, 5, 6, 7, 8,
+ const uint8_t uuid_char_order[UUID_BIN_LEN] = {0, 1, 2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15};
- const u8 guid_char_order[UUID_BIN_LEN] = {3, 2, 1, 0, 5, 4, 7, 6, 8,
+ const uint8_t guid_char_order[UUID_BIN_LEN] = {3, 2, 1, 0, 5, 4, 7, 6, 8,
9, 10, 11, 12, 13, 14, 15};
- const u8 *char_order;
+ const uint8_t *char_order;
const char *format;
int i;
/*
@@ -405,8 +419,9 @@ void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
}
#endif
+#ifndef USE_HOSTCC
#if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
void gen_rand_uuid(unsigned char *uuid_bin)
{
u32 ptr[4];
@@ -494,4 +509,5 @@ U_BOOT_CMD(guid, CONFIG_SYS_MAXARGS, 1, do_uuid,
"e.g. guid guid_env"
);
#endif /* CONFIG_CMD_UUID */
#endif /* CONFIG_RANDOM_UUID || CONFIG_CMD_UUID */
+#endif /* !USE_HOSTCC */
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 6/7] tools: add genguid tool
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (4 preceding siblings ...)
2024-05-31 13:50 ` [PATCH v3 5/7] lib: uuid: supporting building as part of host tools Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-06-05 2:13 ` Simon Glass
` (2 more replies)
2024-05-31 13:50 ` [PATCH v3 7/7] test: lib/uuid: add unit tests for dynamic UUIDs Caleb Connolly
` (2 subsequent siblings)
8 siblings, 3 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Add a tool that can generate GUIDs that match those generated internally
by U-Boot for capsule update fw_images.
Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
with the board model, compatible, and fw_image name.
This tool accepts the same inputs and will produce the same GUID as
U-Boot would at runtime.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
doc/genguid.1 | 52 +++++++++++++++++++
tools/Kconfig | 7 +++
tools/Makefile | 3 ++
tools/genguid.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 216 insertions(+)
diff --git a/doc/genguid.1 b/doc/genguid.1
new file mode 100644
index 000000000000..4128055b3a9a
--- /dev/null
+++ b/doc/genguid.1
@@ -0,0 +1,52 @@
+.\" SPDX-License-Identifier: GPL-2.0+
+.\" Copyright (c) 2024, Linaro Limited
+.TH GENGUID 1 "May 2024"
+
+.SH NAME
+genguid \- Generate deterministic EFI capsule image GUIDs for a board
+
+.SH SYNOPSIS
+.B genguid
+.RI GUID " " [ -vj ] " " -c " " COMPAT " " NAME...
+
+.SH "DESCRIPTION"
+The
+.B genguid
+command is used to determine the update image GUIDs for a board using
+dynamic UUIDs. The command takes a namespace GUID (defined in the boards
+defconfig), the boards first compatible string, and the names of the
+firmware images. The command will output the GUIDs for each image.
+
+As the dynamic UUID mechanism generates GUIDs at runtime, it would be
+necessary to actually boot U-Boot on the board and enable debug logs
+to retrieve the generated GUIDs. This tools just simplifies that process.
+
+.SH "OPTIONS"
+
+.TP
+.BI GUID
+The namespace/salt GUID, same as CONFIG_EFI_CAPSULE_NAMESPACE_UUID.
+The format is:
+ xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+
+.TP
+.BI "-v\fR,\fB --verbose "
+Print additional information to stderr.
+
+.TP
+.BI "-j\fR,\fB --json "
+Output the results in JSON format (array of object with name/uuid properties).
+
+.TP
+.BI "-c\fR,\fB --compat " COMPAT
+The first entry in the boards root compatible property.
+
+.TP
+.BI NAME...
+The names of the firmware images to generate GUIDs for (e.g. "SANDBOX-UBOOT-ENV").
+
+.SH AUTHORS
+Written by Caleb Connolly <caleb.connolly@linaro.org>
+
+.SH HOMEPAGE
+https://u-boot.org
diff --git a/tools/Kconfig b/tools/Kconfig
index 667807b33173..13201ff61fd4 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -103,8 +103,15 @@ config TOOLS_MKEFICAPSULE
This command allows users to create a UEFI capsule file and,
optionally sign that file. If you want to enable UEFI capsule
update feature on your target, you certainly need this.
+config TOOLS_GENGUID
+ bool "Build genguid command"
+ default y if EFI_CAPSULE_DYNAMIC_UUIDS
+ help
+ This command allows users to generate the GUIDs that a given
+ board would use for UEFI capsule update feature.
+
menuconfig FSPI_CONF_HEADER
bool "FlexSPI Header Configuration"
help
FSPI Header Configuration
diff --git a/tools/Makefile b/tools/Makefile
index 6a4280e3668f..29e9a93b0f24 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -253,8 +253,11 @@ HOSTLDLIBS_mkeficapsule += \
HOSTLDLIBS_mkeficapsule += \
$(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
+genguid-objs := generated/lib/uuid.o generated/lib/sha1.o genguid.o
+hostprogs-$(CONFIG_TOOLS_GENGUID) += genguid
+
mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
HOSTLDLIBS_mkfwumdata += -luuid
hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
diff --git a/tools/genguid.c b/tools/genguid.c
new file mode 100644
index 000000000000..e71bc1d48f95
--- /dev/null
+++ b/tools/genguid.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2024 Linaro Ltd.
+ * Author: Caleb Connolly
+ */
+
+#include <getopt.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/types.h>
+
+#include <uuid.h>
+
+static struct option options[] = {
+ {"dtb", required_argument, NULL, 'd'},
+ {"compat", required_argument, NULL, 'c'},
+ {"help", no_argument, NULL, 'h'},
+ {"verbose", no_argument, NULL, 'v'},
+ {"json", no_argument, NULL, 'j'},
+ {NULL, 0, NULL, 0},
+};
+
+static void usage(const char *progname)
+{
+ fprintf(stderr, "Usage: %s GUID [-v] -c COMPAT NAME...\n", progname);
+ fprintf(stderr,
+ "Generate a v5 GUID for one of more U-Boot fw_images the same way U-Boot does at runtime.\n");
+ fprintf(stderr,
+ "\nOptions:\n"
+ " GUID namespace/salt GUID in 8-4-4-4-12 format\n"
+ " -h, --help display this help and exit\n"
+ " -c, --compat=COMPAT first compatible property in the board devicetree\n"
+ " -v, --verbose print debug messages\n"
+ " -j, --json output in JSON format\n"
+ " NAME... one or more names of fw_images to generate GUIDs for\n"
+ );
+ fprintf(stderr, "\nExample:\n");
+ fprintf(stderr, " %s 2a5aa852-b856-4d97-baa9-5c5f4421551f \\\n"
+ "\t-c \"qcom,qrb4210-rb2\" \\\n"
+ "\tQUALCOMM-UBOOT\n", progname);
+}
+
+static size_t u16_strsize(const uint16_t *in)
+{
+ size_t i = 0, count = UINT16_MAX;
+
+ while (count-- && in[i])
+ i++;
+
+ return (i + 1) * sizeof(uint16_t);
+}
+
+int main(int argc, char **argv)
+{
+ struct uuid namespace;
+ char *namespace_str;
+ char uuid_str[37];
+ char **image_uuids;
+ char *compatible = NULL;
+ uint16_t **images_u16;
+ char **images;
+ int c, n_images;
+ bool debug = false, json = false;
+
+ if (argc < 2) {
+ usage(argv[0]);
+ return 1;
+ }
+
+ namespace_str = argv[1];
+
+ /* The first arg is the GUID so skip it */
+ while ((c = getopt_long(argc, argv, "c:hvj", options, NULL)) != -1) {
+ switch (c) {
+ case 'c':
+ compatible = strdup(optarg);
+ break;
+ case 'h':
+ usage(argv[0]);
+ return 0;
+ case 'v':
+ debug = true;
+ break;
+ case 'j':
+ json = true;
+ break;
+ default:
+ usage(argv[0]);
+ return 1;
+ }
+ }
+
+ if (!compatible) {
+ fprintf(stderr, "ERROR: Please specify the compatible property.\n\n");
+ usage(argv[0]);
+ return 1;
+ }
+
+ if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_GUID)) {
+ fprintf(stderr, "ERROR: Check that your UUID is formatted correctly.\n");
+ exit(EXIT_FAILURE);
+ }
+
+ /* This is probably not the best way to convert a string to a "u16" string */
+ n_images = argc - optind - 1;
+ images = argv + optind + 1;
+ images_u16 = calloc(n_images, sizeof(char *));
+ for (int i = 0; i < n_images; i++) {
+ images_u16[i] = calloc(1, strlen(images[i]) * 2 + 2);
+ for (int j = 0; j < strlen(images[i]); j++)
+ images_u16[i][j] = (uint16_t)images[i][j];
+ }
+
+ if (debug) {
+ fprintf(stderr, "GUID: ");
+ uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_GUID);
+ fprintf(stderr, "%s\n", uuid_str);
+ fprintf(stderr, "Compatible: \"%s\"\n", compatible);
+ fprintf(stderr, "Images: ");
+ for (int i = 0; i < n_images; i++)
+ fprintf(stderr, "\"%s\"%s", argv[optind + i + 1],
+ i == n_images - 1 ? "\n" : ", ");
+ }
+
+ image_uuids = calloc(n_images, sizeof(char *));
+ for (int i = 0; i < n_images; i++) {
+ struct uuid image_type_id;
+
+ gen_uuid_v5(&namespace, &image_type_id,
+ compatible, strlen(compatible),
+ images_u16[i], u16_strsize(images_u16[i]) - sizeof(uint16_t),
+ NULL);
+
+ uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_GUID);
+ image_uuids[i] = strdup(uuid_str);
+ }
+
+ if (json) {
+ printf("[\n");
+ for (int i = 0; i < n_images; i++)
+ printf("\t{\"name\": \"%s\", \"uuid\": \"%s\"}%s\n", images[i], image_uuids[i],
+ i == n_images - 1 ? "" : ",");
+ printf("]\n");
+ } else {
+ for (int i = 0; i < n_images; i++)
+ printf("%-24s| %s\n", images[i], image_uuids[i]);
+ }
+
+ return 0;
+}
+
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 7/7] test: lib/uuid: add unit tests for dynamic UUIDs
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (5 preceding siblings ...)
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
@ 2024-05-31 13:50 ` Caleb Connolly
2024-06-05 5:59 ` [PATCH v3 0/7] efi: CapsuleUpdate: support " Heinrich Schuchardt
2024-06-19 14:02 ` Vincent Stehlé
8 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-05-31 13:50 UTC (permalink / raw)
To: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot, Caleb Connolly
Add some basic unit tests to validate that the UUID generation behaves
as expected. This matches the implementation in efi_loader for sandbox
and a Qualcomm board and should catch any regressions.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
test/lib/uuid.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/test/lib/uuid.c b/test/lib/uuid.c
index 0914f2c47e77..dc485189ae68 100644
--- a/test/lib/uuid.c
+++ b/test/lib/uuid.c
@@ -7,15 +7,20 @@
* Authors:
* Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
*/
+#include <charset.h>
#include <uuid.h>
#include <test/lib.h>
#include <test/test.h>
#include <test/ut.h>
/* test UUID */
#define TEST_SVC_UUID "ed32d533-4209-99e6-2d72-cdd998a79cc0"
+/* Sandbox namespace UUID */
+#define SANDBOX_NAMESPACE_UUID "09d7cf52-0720-4710-91d1-08469b7fe9c8"
+/* Qcom namespace UUID */
+#define QCOM_NAMESPACE_UUID "2a5aa852-b856-4d97-baa9-5c5f4421551f"
#define UUID_SIZE 16
/* The UUID binary data (little-endian format) */
@@ -37,4 +42,87 @@ static int lib_test_uuid_to_le(struct unit_test_state *uts)
return 0;
}
LIB_TEST(lib_test_uuid_to_le, 0);
+
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
+struct dynamic_uuid_test_data {
+ const char *namespace;
+ const char *compatible;
+ const u16 *images[4];
+ const char *expected_uuids[4];
+};
+
+static int lib_test_dynamic_uuid_case(struct unit_test_state *uts,
+ const struct dynamic_uuid_test_data *data)
+{
+ struct uuid namespace;
+ int j;
+
+ ut_assertok(uuid_str_to_bin(data->namespace, (unsigned char *)&namespace,
+ UUID_STR_FORMAT_GUID));
+
+ for (j = 0; data->images[j]; j++) {
+ const char *expected_uuid = data->expected_uuids[j];
+ const u16 *image = data->images[j];
+ struct uuid uuid;
+ char uuid_str[37];
+
+ gen_uuid_v5(&namespace, &uuid,
+ data->compatible, strlen(data->compatible),
+ image, u16_strsize(image) - sizeof(uint16_t),
+ NULL);
+ uuid_bin_to_str((unsigned char *)&uuid, uuid_str, UUID_STR_FORMAT_GUID);
+
+ ut_asserteq_str(expected_uuid, uuid_str);
+ }
+
+ return 0;
+}
+
+static int lib_test_dynamic_uuid(struct unit_test_state *uts)
+{
+ int ret, i;
+ const struct dynamic_uuid_test_data test_data[] = {
+ {
+ .compatible = "sandbox",
+ .namespace = SANDBOX_NAMESPACE_UUID,
+ .images = {
+ u"SANDBOX-UBOOT",
+ u"SANDBOX-UBOOT-ENV",
+ u"SANDBOX-FIT",
+ NULL,
+ },
+ .expected_uuids = {
+ "fd5db83c-12f3-a46b-80a9-e3007c7ff56e",
+ "935fe837-fac8-4394-c008-737d8852c60d",
+ "ffd97379-0956-fa94-c003-8bfcf5cc097b",
+ NULL,
+ }
+ },
+ {
+ .compatible = "qcom,qrb4210-rb2",
+ .namespace = QCOM_NAMESPACE_UUID,
+ .images = {
+ u"QUALCOMM-UBOOT",
+ NULL,
+ },
+ .expected_uuids = {
+ "8ee418dc-7e00-e156-80a7-274fbbc05ba8",
+ NULL,
+ }
+ },
+ };
+
+ for (i = 0; i < ARRAY_SIZE(test_data); i++) {
+ ret = lib_test_dynamic_uuid_case(uts, &test_data[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+LIB_TEST(lib_test_dynamic_uuid, 0);
+
+#endif /* CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) */
+
--
2.45.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3 4/7] sandbox: switch to dynamic UUIDs
2024-05-31 13:50 ` [PATCH v3 4/7] sandbox: switch to dynamic UUIDs Caleb Connolly
@ 2024-05-31 15:43 ` Ilias Apalodimas
0 siblings, 0 replies; 36+ messages in thread
From: Ilias Apalodimas @ 2024-05-31 15:43 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Simon Glass, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes, u-boot
On Fri, 31 May 2024 at 16:50, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Migrate sandbox over to generating it's capsule update image GUIDs
> dynamically from the namespace and board/image info. Update the
> reference and tests to use the new GUIDs.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> arch/Kconfig | 1 +
> board/sandbox/sandbox.c | 16 ----------------
> configs/sandbox_defconfig | 1 +
> configs/sandbox_flattree_defconfig | 1 +
> include/sandbox_efi_capsule.h | 6 +++---
> .../tests/test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
> .../tests/test_efi_capsule/test_capsule_firmware_raw.py | 8 ++++----
> .../test_efi_capsule/test_capsule_firmware_signed_fit.py | 2 +-
> .../test_efi_capsule/test_capsule_firmware_signed_raw.py | 4 ++--
> test/py/tests/test_efi_capsule/version.dts | 6 +++---
> tools/binman/etype/efi_capsule.py | 2 +-
> tools/binman/ftest.py | 2 +-
> 12 files changed, 19 insertions(+), 32 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index abd406d48841..0558c90540b6 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -164,8 +164,9 @@ config SANDBOX
> select SYS_CACHE_SHIFT_4
> select IRQ
> select SUPPORT_EXTENSION_SCAN if CMDLINE
> select SUPPORT_ACPI
> + select EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT
> imply BITREVERSE
> select BLOBLIST
> imply LTO
> imply CMD_DM
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index 802596569c64..d97945e58fcf 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -31,36 +31,20 @@
> */
> gd_t *gd;
>
> #if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)
> -/* GUIDs for capsule updatable firmware images */
> -#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)
> -
> struct efi_fw_image 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
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 93b52f2de5cf..58775b271600 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -349,8 +349,9 @@ CONFIG_TPM=y
> CONFIG_ERRNO_STR=y
> CONFIG_GETOPT=y
> CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> +CONFIG_EFI_CAPSULE_NAMESPACE_UUID="09D7CF52-0720-4710-91D1-08469B7FE9C8"
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> CONFIG_EFI_CAPSULE_ESL_FILE="board/sandbox/capsule_pub_esl_good.esl"
> CONFIG_EFI_SECURE_BOOT=y
> diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
> index 6bf8874e722e..85ae63da8881 100644
> --- a/configs/sandbox_flattree_defconfig
> +++ b/configs/sandbox_flattree_defconfig
> @@ -224,8 +224,9 @@ CONFIG_TPM=y
> CONFIG_ZSTD=y
> CONFIG_ERRNO_STR=y
> CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> +CONFIG_EFI_CAPSULE_NAMESPACE_UUID="09d7cf52-0720-4710-91d1-08469b7fe9c8"
> CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> CONFIG_EFI_CAPSULE_ESL_FILE="board/sandbox/capsule_pub_esl_good.esl"
> CONFIG_UNIT_TEST=y
> diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h
> index 3e288e8a84a2..25ac496ea24f 100644
> --- a/include/sandbox_efi_capsule.h
> +++ b/include/sandbox_efi_capsule.h
> @@ -5,11 +5,11 @@
>
> #if !defined(_SANDBOX_EFI_CAPSULE_H_)
> #define _SANDBOX_EFI_CAPSULE_H_
>
> -#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8"
> -#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0"
> -#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937"
> +#define SANDBOX_UBOOT_IMAGE_GUID "fd5db83c-12f3-a46b-80a9-e3007c7ff56e"
> +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "935fe837-fac8-4394-c008-737d8852c60d"
> +#define SANDBOX_FIT_IMAGE_GUID "ffd97379-0956-fa94-c003-8bfcf5cc097b"
> #define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4"
>
> #define UBOOT_FIT_IMAGE "u-boot_bin_env.itb"
>
> 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
> index 11bcdc2bb293..746da4602085 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
> @@ -146,9 +146,9 @@ class TestEfiCapsuleFirmwareFit():
> verify_content(u_boot_console, '100000', 'u-boot:Old')
> verify_content(u_boot_console, '150000', 'u-boot-env:Old')
> else:
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '3673B45D-6A7C-46F3-9E60-ADABB03F7937' in ''.join(output)
> + assert '5AF91295-5A99-F62B-80D7-E9574DE87170' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> verify_content(u_boot_console, '100000', 'u-boot:New')
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> index a5b5c8a3853a..1866b8086573 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> @@ -133,12 +133,12 @@ class TestEfiCapsuleFirmwareRaw:
> '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 SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
> + assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
> + assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
>
> check_file_removed(u_boot_console, disk_img, capsule_files)
>
> expected = 'u-boot:Old' if capsule_auth else 'u-boot:New'
> @@ -187,14 +187,14 @@ class TestEfiCapsuleFirmwareRaw:
> verify_content(u_boot_console, '100000', 'u-boot:Old')
> verify_content(u_boot_console, '150000', 'u-boot-env:Old')
> else:
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
> + assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
> + assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
> assert 'ESRT: fw_version=10' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
>
> verify_content(u_boot_console, '100000', 'u-boot:New')
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> index 44a58baa3106..a4e0a3bc73f5 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> @@ -156,9 +156,9 @@ class TestEfiCapsuleFirmwareSignedFit():
> 'u-boot-env raw 0x150000 0x200000"',
> 'efidebug capsule esrt'])
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '3673B45D-6A7C-46F3-9E60-ADABB03F7937' in ''.join(output)
> + assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> verify_content(u_boot_console, '100000', 'u-boot:New')
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> index 83a10e160b8c..260c71860632 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> @@ -150,14 +150,14 @@ class TestEfiCapsuleFirmwareSignedRaw():
> 'u-boot-env raw 0x150000 0x200000"',
> 'efidebug capsule esrt'])
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '09D7CF52-0720-4710-91D1-08469B7FE9C8' in ''.join(output)
> + assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '5A7021F5-FEF2-48B4-AABA-832E777418C0' in ''.join(output)
> + assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
> assert 'ESRT: fw_version=10' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
>
> verify_content(u_boot_console, '100000', 'u-boot:New')
> diff --git a/test/py/tests/test_efi_capsule/version.dts b/test/py/tests/test_efi_capsule/version.dts
> index 07850cc6064c..3f0698bf7280 100644
> --- a/test/py/tests/test_efi_capsule/version.dts
> +++ b/test/py/tests/test_efi_capsule/version.dts
> @@ -7,18 +7,18 @@
> firmware-version {
> image1 {
> lowest-supported-version = <3>;
> image-index = <1>;
> - image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> + image-type-id = "FD5DB83C-12F3-A46B-80A9-E3007C7FF56E";
> };
> image2 {
> lowest-supported-version = <7>;
> image-index = <2>;
> - image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> + image-type-id = "935FE837-FAC8-4394-C008-737D8852C60D";
> };
> image3 {
> lowest-supported-version = <3>;
> image-index = <1>;
> - image-type-id = "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
> + image-type-id = "FFD97379-0956-FA94-C003-8BFCF5CC097B";
> };
> };
> };
> diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py
> index e32037178221..da1f9b0a381a 100644
> --- a/tools/binman/etype/efi_capsule.py
> +++ b/tools/binman/etype/efi_capsule.py
> @@ -23,9 +23,9 @@ def get_binman_test_guid(type_str):
> Returns:
> The actual GUID value (str)
> """
> TYPE_TO_GUID = {
> - 'binman-test' : '09d7cf52-0720-4710-91d1-08469b7fe9c8'
> + 'binman-test' : 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
> }
>
> return TYPE_TO_GUID[type_str]
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 8a44bc051b36..dc602b95ecd5 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -123,9 +123,9 @@ TEE_ADDR = 0x5678
>
> # Firmware Management Protocol(FMP) GUID
> FW_MGMT_GUID = '6dcbd5ed-e82d-4c44-bda1-7194199ad92a'
> # Image GUID specified in the DTS
> -CAPSULE_IMAGE_GUID = '09d7cf52-0720-4710-91d1-08469b7fe9c8'
> +CAPSULE_IMAGE_GUID = 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
> # Windows cert GUID
> WIN_CERT_TYPE_EFI_GUID = '4aafd29d-68df-49ee-8aa9-347d375665a7'
> # Empty capsule GUIDs
> EMPTY_CAPSULE_ACCEPT_GUID = '0c996046-bcc0-4d04-85ec-e1fcedf1c6f8'
>
> --
> 2.45.0
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/7] lib: uuid: add UUID v5 support
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
@ 2024-05-31 16:11 ` Ilias Apalodimas
2024-06-05 2:13 ` Simon Glass
2024-06-05 5:33 ` Heinrich Schuchardt
2 siblings, 0 replies; 36+ messages in thread
From: Ilias Apalodimas @ 2024-05-31 16:11 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Simon Glass, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes, u-boot
Hi Caleb,
[...]
> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> +{
> + sha1_context ctx;
> + va_list args;
> + const uint8_t *data;
> + uint8_t hash[SHA1_SUM_LEN];
> + uint32_t tmp;
> +
> + sha1_starts(&ctx);
> + /* Hash the namespace UUID as salt */
> + sha1_update(&ctx, (unsigned char *)namespace, UUID_BIN_LEN);
> + va_start(args, uuid);
> +
> + while ((data = va_arg(args, const uint8_t *))) {
> + unsigned int len = va_arg(args, size_t);
> +
> + sha1_update(&ctx, data, len);
> + }
> +
> + va_end(args);
> + sha1_finish(&ctx, hash);
> +
> + /* Truncate the hash into output UUID, it is already big endian */
> + memcpy(uuid, hash, sizeof(*uuid));
> +
> + /* Configure variant/version bits */
> + tmp = be32_to_cpu(uuid->time_hi_and_version);
uuid->time_hi_and_version is a u16 so this should be better off with a
be16_to_cpu?
OTOH your initial implementation was using clrsetbits_be16 and clrsetbits_8.
Looking at what we have the same calls are done in gen_rand_uuid().
I think it's better if you make a function call for this and replace
both your code and the gen_rand_uuid() instead of open coding that
Thanks
/Ilias
> + tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> + uuid->time_hi_and_version = cpu_to_be32(tmp);
> +
> + uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
> + uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> +}
> +#endif
> +
> #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
> void gen_rand_uuid(unsigned char *uuid_bin)
> {
> u32 ptr[4];
>
> --
> 2.45.0
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
@ 2024-06-05 2:13 ` Simon Glass
2024-06-05 6:36 ` Heinrich Schuchardt
2024-06-20 14:58 ` Vincent Stehlé
2 siblings, 0 replies; 36+ messages in thread
From: Simon Glass @ 2024-06-05 2:13 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes, u-boot
Hi Caleb,
On Fri, 31 May 2024 at 07:50, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Add a tool that can generate GUIDs that match those generated internally
> by U-Boot for capsule update fw_images.
>
> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> with the board model, compatible, and fw_image name.
>
> This tool accepts the same inputs and will produce the same GUID as
> U-Boot would at runtime.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> doc/genguid.1 | 52 +++++++++++++++++++
> tools/Kconfig | 7 +++
> tools/Makefile | 3 ++
> tools/genguid.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 216 insertions(+)
Reviewed-by: Simon Glass <sjg@chromium.org>
Can I suggest you look at patman, which will provide a per-patch change list?
Regards,
SImon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/7] lib: uuid: add UUID v5 support
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
2024-05-31 16:11 ` Ilias Apalodimas
@ 2024-06-05 2:13 ` Simon Glass
2024-06-05 12:06 ` Caleb Connolly
2024-06-05 5:33 ` Heinrich Schuchardt
2 siblings, 1 reply; 36+ messages in thread
From: Simon Glass @ 2024-06-05 2:13 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes, u-boot
Hi Caleb,
On Fri, 31 May 2024 at 07:50, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Add support for generating version 5 UUIDs, these are determistic and work
spelling
> by hashing a "namespace" UUID together with some unique data. One intended
> usecase is to allow for dynamically generate payload UUIDs for UEFI
> capsule updates, so that supported boards can have their own UUIDs
> without needing to hardcode them.
>
> Tests for this are added in an upcoming patch.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> include/uuid.h | 17 +++++++++++++++++
> lib/Kconfig | 8 ++++++++
> lib/uuid.c | 37 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 62 insertions(+)
>
> diff --git a/include/uuid.h b/include/uuid.h
> index f5a941250f48..539affaa47b9 100644
> --- a/include/uuid.h
> +++ b/include/uuid.h
> @@ -10,8 +10,9 @@
> #ifndef __UUID_H__
> #define __UUID_H__
>
> #include <linux/bitops.h>
> +#include <linux/kconfig.h>
>
> /*
> * UUID - Universally Unique IDentifier - 128 bits unique number.
> * There are 5 versions and one variant of UUID defined by RFC4122
> @@ -142,8 +143,24 @@ void gen_rand_uuid(unsigned char *uuid_bin);
> * @param - uuid output type: UUID - 0, GUID - 1
> */
> void gen_rand_uuid_str(char *uuid_str, int str_format);
>
> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
> +/**
> + * gen_uuid_v5() - generate UUID v5 from namespace and other seed data.
> + *
> + * @namespace: pointer to UUID namespace salt
> + * @uuid: pointer to allocated UUID output
> + * @...: NULL terminated list of seed data as pairs of pointers
> + * to data and their lengths
> + */
> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...);
> +#else
> +static inline void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> +{
> +}
> +#endif
Can you explain somewhere why the static inline is needed?
> +
> /**
> * uuid_str_to_le_bin() - Convert string UUID to little endian binary data.
> * @uuid_str: pointer to UUID string
> * @uuid_bin: pointer to allocated array for little endian output [16B]
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 189e6eb31aa1..2941532f25cf 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -80,8 +80,16 @@ config RANDOM_UUID
> help
> Enable the generation of partitions with random UUIDs if none
> are provided.
>
> +config UUID_GEN_V5
> + bool "Enable UUID version 5 generation"
> + select LIB_UUID
> + depends on SHA1
> + help
> + Enable the generation of version 5 UUIDs, these are determistic and
spelling
> + generated from a namespace UUID, and a string (such as a board name).
> +
> config SPL_LIB_UUID
> depends on SPL
> bool
>
> diff --git a/lib/uuid.c b/lib/uuid.c
> index dfa2320ba267..2df0523e717f 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -21,8 +21,9 @@
> #include <part_efi.h>
> #include <malloc.h>
> #include <dm/uclass.h>
> #include <rng.h>
> +#include <u-boot/sha1.h>
>
> int uuid_str_valid(const char *uuid)
> {
> int i, valid;
> @@ -368,8 +369,44 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str,
> }
> }
> }
>
> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> +{
> + sha1_context ctx;
> + va_list args;
> + const uint8_t *data;
> + uint8_t hash[SHA1_SUM_LEN];
> + uint32_t tmp;
> +
> + sha1_starts(&ctx);
> + /* Hash the namespace UUID as salt */
> + sha1_update(&ctx, (unsigned char *)namespace, UUID_BIN_LEN);
> + va_start(args, uuid);
> +
> + while ((data = va_arg(args, const uint8_t *))) {
> + unsigned int len = va_arg(args, size_t);
> +
> + sha1_update(&ctx, data, len);
> + }
> +
> + va_end(args);
> + sha1_finish(&ctx, hash);
> +
> + /* Truncate the hash into output UUID, it is already big endian */
> + memcpy(uuid, hash, sizeof(*uuid));
> +
> + /* Configure variant/version bits */
> + tmp = be32_to_cpu(uuid->time_hi_and_version);
> + tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> + uuid->time_hi_and_version = cpu_to_be32(tmp);
> +
> + uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
> + uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> +}
> +#endif
> +
> #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
> void gen_rand_uuid(unsigned char *uuid_bin)
> {
> u32 ptr[4];
>
> --
> 2.45.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/7] lib: uuid: add UUID v5 support
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
2024-05-31 16:11 ` Ilias Apalodimas
2024-06-05 2:13 ` Simon Glass
@ 2024-06-05 5:33 ` Heinrich Schuchardt
2 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-05 5:33 UTC (permalink / raw)
To: Caleb Connolly
Cc: Richard Hughes, u-boot, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
On 5/31/24 15:50, Caleb Connolly wrote:
> Add support for generating version 5 UUIDs, these are determistic and work
> by hashing a "namespace" UUID together with some unique data. One intended
> usecase is to allow for dynamically generate payload UUIDs for UEFI
> capsule updates, so that supported boards can have their own UUIDs
> without needing to hardcode them.
>
> Tests for this are added in an upcoming patch.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> include/uuid.h | 17 +++++++++++++++++
> lib/Kconfig | 8 ++++++++
> lib/uuid.c | 37 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 62 insertions(+)
>
> diff --git a/include/uuid.h b/include/uuid.h
> index f5a941250f48..539affaa47b9 100644
> --- a/include/uuid.h
> +++ b/include/uuid.h
> @@ -10,8 +10,9 @@
> #ifndef __UUID_H__
> #define __UUID_H__
>
> #include <linux/bitops.h>
> +#include <linux/kconfig.h>
>
> /*
> * UUID - Universally Unique IDentifier - 128 bits unique number.
> * There are 5 versions and one variant of UUID defined by RFC4122
> @@ -142,8 +143,24 @@ void gen_rand_uuid(unsigned char *uuid_bin);
> * @param - uuid output type: UUID - 0, GUID - 1
> */
> void gen_rand_uuid_str(char *uuid_str, int str_format);
>
> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
> +/**
> + * gen_uuid_v5() - generate UUID v5 from namespace and other seed data.
> + *
> + * @namespace: pointer to UUID namespace salt
> + * @uuid: pointer to allocated UUID output
> + * @...: NULL terminated list of seed data as pairs of pointers
> + * to data and their lengths
It is unclear what this might mean.
According to your description ... could be
struct {
void *data;
u32 *length;
} ...[] = {
{ &data_1, &length_1 },
{ &data_2, &length_2 },
{ NULL, NULL }
}
Do we have pointer to lengths?
Which data type do lengths have?
Please, provide an unambiguous description.
An example would help.
> + */
> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...);
> +#else
> +static inline void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> +{
> +}
> +#endif
> +
> /**
> * uuid_str_to_le_bin() - Convert string UUID to little endian binary data.
> * @uuid_str: pointer to UUID string
> * @uuid_bin: pointer to allocated array for little endian output [16B]
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 189e6eb31aa1..2941532f25cf 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -80,8 +80,16 @@ config RANDOM_UUID
> help
> Enable the generation of partitions with random UUIDs if none
> are provided.
>
> +config UUID_GEN_V5
> + bool "Enable UUID version 5 generation"
> + select LIB_UUID
> + depends on SHA1
'select SHA1' might make things easier.
> + help
> + Enable the generation of version 5 UUIDs, these are determistic and
%s/determistic/deterministic/
> + generated from a namespace UUID, and a string (such as a board name).
> +
> config SPL_LIB_UUID
> depends on SPL
> bool
>
> diff --git a/lib/uuid.c b/lib/uuid.c
> index dfa2320ba267..2df0523e717f 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -21,8 +21,9 @@
> #include <part_efi.h>
> #include <malloc.h>
> #include <dm/uclass.h>
> #include <rng.h>
> +#include <u-boot/sha1.h>
>
> int uuid_str_valid(const char *uuid)
> {
> int i, valid;
> @@ -368,8 +369,44 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str,
> }
> }
> }
>
> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> +{
> + sha1_context ctx;
> + va_list args;
> + const uint8_t *data;
> + uint8_t hash[SHA1_SUM_LEN];
> + uint32_t tmp;
> +
> + sha1_starts(&ctx);
> + /* Hash the namespace UUID as salt */
> + sha1_update(&ctx, (unsigned char *)namespace, UUID_BIN_LEN);
> + va_start(args, uuid);
> +
> + while ((data = va_arg(args, const uint8_t *))) {
> + unsigned int len = va_arg(args, size_t);
> +
> + sha1_update(&ctx, data, len);
> + }
> +
> + va_end(args);
> + sha1_finish(&ctx, hash);
> +
> + /* Truncate the hash into output UUID, it is already big endian */
> + memcpy(uuid, hash, sizeof(*uuid));
> +
> + /* Configure variant/version bits */
> + tmp = be32_to_cpu(uuid->time_hi_and_version);
> + tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
Currently we have
#define UUID_VERSION 0x4
Shouldn't we create
#define UUID_VARIANT_4 0x4 - to be used in gen_rand_uuid()
#define UUID_VARIANT_5 0x5 - to be used in gen_uuid_v5()
instead?
Please, provide a unit test for gen_uuid_v5() that passes multiple
strings to gen_uuid_v5() and checks the hash.
Best regards
Heinrich
> + uuid->time_hi_and_version = cpu_to_be32(tmp);
> +
> + uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
> + uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> +}
> +#endif
> +
> #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
> void gen_rand_uuid(unsigned char *uuid_bin)
> {
> u32 ptr[4];
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs
2024-05-31 13:50 ` [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs Caleb Connolly
@ 2024-06-05 5:52 ` Heinrich Schuchardt
2024-06-05 12:22 ` Caleb Connolly
0 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-05 5:52 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot
On 5/31/24 15:50, Caleb Connolly wrote:
> Introduce a new helper efi_capsule_update_info_gen_ids() which populates
> the capsule update fw images image_type_id field. This allows for
> determinstic UUIDs to be used that can scale to a large number of
> different boards and board variants without the need to maintain a big
> list.
>
> We call this from efi_fill_image_desc_array() to populate the UUIDs
> lazily on-demand.
>
> This is behind an additional config option as it depends on V5 UUIDs and
> the SHA1 implementation.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> lib/efi_loader/Kconfig | 23 +++++++++++++++
> lib/efi_loader/efi_capsule.c | 1 +
> lib/efi_loader/efi_firmware.c | 66 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 90 insertions(+)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 430bb7f0f7dc..e90caf4f8e14 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -235,8 +235,31 @@ config EFI_CAPSULE_ON_DISK_EARLY
> If this option is enabled, capsules will be enforced to be
> executed as part of U-Boot initialisation so that they will
> surely take place whatever is set to distro_bootcmd.
>
> +config EFI_CAPSULE_DYNAMIC_UUIDS
> + bool "Dynamic UUIDs for capsules"
> + depends on EFI_HAVE_CAPSULE_SUPPORT
> + select UUID_GEN_V5
This select will create a build error if CONFIG_SHA1=n as
CONFIG_UUID_GEN_V5 depends on it.
> + help
> + Select this option if you want to use dynamically generated v5
> + UUIDs for your board. To make use of this feature, your board
> + code should call efi_capsule_update_info_gen_ids() with a seed
> + UUID to generate the image_type_id field for each fw_image.
> +
> + The CapsuleUpdate payloads are expected to generate matching UUIDs
> + using the same scheme.
> +
> +config EFI_CAPSULE_NAMESPACE_UUID
> + string "Namespace UUID for dynamic UUIDs"
> + depends on EFI_CAPSULE_DYNAMIC_UUIDS
> + help
> + Define the namespace or "salt" UUID used to generate the per-image
> + UUIDs. This should be a UUID in the standard 8-4-4-4-12 format.
As UUIDs can be formatted low-endian or big-endian I would not know how
the value will be interpreted.
Why do we need a vendor specific name-space if we are using compatible
strings which are vendor specific themselves?
> +
> + Device vendors are expected to generate their own namespace UUID
> + to avoid conflicts with existing products.
> +
> config EFI_CAPSULE_FIRMWARE
> bool
>
> config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 0937800e588f..ac02e79ae7d8 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -19,8 +19,9 @@
> #include <mapmem.h>
> #include <sort.h>
> #include <sysreset.h>
> #include <asm/global_data.h>
> +#include <uuid.h>
>
> #include <crypto/pkcs7.h>
> #include <crypto/pkcs7_parser.h>
> #include <linux/err.h>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index ba5aba098c0f..a8dafe4f01a5 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -244,8 +244,71 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
>
> free(var_state);
> }
>
> +#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
> +/**
> + * efi_capsule_update_info_gen_ids - generate GUIDs for the images
> + *
> + * Generate the image_type_id for each image in the update_info.images array
> + * using the first compatible from the device tree and a salt
> + * UUID defined at build time.
> + *
> + * Returns: status code
> + */
> +static efi_status_t efi_capsule_update_info_gen_ids(void)
> +{
> + int ret, i;
> + struct uuid namespace;
> + const char *compatible; /* Full array including null bytes */
> + struct efi_fw_image *fw_array;
> +
> + fw_array = update_info.images;
> + /* Check if we need to run (there are images and we didn't already generate their IDs) */
> + if (!update_info.num_images ||
> + memchr_inv(&fw_array[0].image_type_id, 0, sizeof(fw_array[0].image_type_id)))
> + return EFI_SUCCESS;
> +
> + ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
> + (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
> + if (ret) {
> + log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid: %d\n", __func__, ret);
> + return EFI_UNSUPPORTED;
> + }
You don't want end-users to be the first to find this issue. This must
be a build time check.
> +
> + compatible = ofnode_read_string(ofnode_root(), "compatible");
> +
> + if (!compatible) {
> + log_debug("%s: model or compatible not defined\n", __func__);
You are not reading model here.
> + return EFI_UNSUPPORTED;
> + }
> +
> + if (!update_info.num_images) {
> + log_debug("%s: no fw_images, make sure update_info.num_images is set\n", __func__);
I thought we were using capsules without images in A/B updates and need
to process them?
> + return -ENODATA;
> + }
> +
> + for (i = 0; i < update_info.num_images; i++) {
> + gen_uuid_v5(&namespace,
> + (struct uuid *)&fw_array[i].image_type_id,
> + compatible, strlen(compatible),
> + fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> + - sizeof(uint16_t),
> + NULL);
> +
> + log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
> + &fw_array[i].image_type_id);
> + }
> +
> + return EFI_SUCCESS;
> +}
> +#else
> +static efi_status_t efi_capsule_update_info_gen_ids(void)
> +{
> + return EFI_SUCCESS;
Why should we have a case were we don't generate the image UUIDs?
Don't we get rid of capsule GUIDs in the device-trees?
Best regards
Heinrich
> +}
> +#endif
> +
> /**
> * efi_fill_image_desc_array - populate image descriptor array
> * @image_info_size: Size of @image_info
> * @image_info: Image information
> @@ -282,8 +345,11 @@ static efi_status_t efi_fill_image_desc_array(
> return EFI_BUFFER_TOO_SMALL;
> }
> *image_info_size = total_size;
>
> + if (efi_capsule_update_info_gen_ids() != EFI_SUCCESS)
> + return EFI_UNSUPPORTED;
> +
> fw_array = update_info.images;
> *descriptor_count = update_info.num_images;
> *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> *descriptor_size = sizeof(*image_info);
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (6 preceding siblings ...)
2024-05-31 13:50 ` [PATCH v3 7/7] test: lib/uuid: add unit tests for dynamic UUIDs Caleb Connolly
@ 2024-06-05 5:59 ` Heinrich Schuchardt
2024-06-05 17:02 ` Caleb Connolly
2024-06-19 14:02 ` Vincent Stehlé
8 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-05 5:59 UTC (permalink / raw)
To: Caleb Connolly
Cc: Richard Hughes, u-boot, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
On 5/31/24 15:50, Caleb Connolly wrote:
> As more boards adopt support for the EFI CapsuleUpdate mechanism, there
> is a growing issue of being able to target updates to them properly. The
> current mechanism of hardcoding UUIDs for each board at compile time is
> unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
>
> In this series, I propose that we adopt v5 GUIDs, these are generated
> by using a well-known salt GUID as well as board specific information
> (like the model/revision), these are hashed together and the result is
> truncated to form a new UUID.
>
> The well-known salt GUID can be specific to the architecture (SoC
> vendor), or OEM. It is defined in the board defconfig so that vendors
> can easily bring their own.
>
> Specifically, the following fields are used to generate a GUID for a
> particular fw_image:
>
> * namespace salt
> * board compatible (usually the first entry in the dt root compatible
> array).
> * fw_image name (the string identifying the specific image, especially
> relevant for board that can update multiple images).
>
> == Usage ==
>
> Boards can integrate dynamic UUID support as follows:
>
> 1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if
> EFI_HAVE_CAPSULE_SUPPORT.
> 2. Skip setting the fw_images image_type_id property.
> 3. Generate a UUID and set CONFIG_EFI_CAPSULE_NAMESPACE_UUID in your
> defconfig.
Why should we have two alternatives?
If we have the dynamic UUIDs, please, get rid of static ones.
>
> == Limitations ==
>
> * Changing GUIDs
>
> The primary limitation with this approach is that if any of the source
> fields change, so will the GUID for the board. It is therefore pretty
> important to ensure that GUID changes are caught during development.
>
> * Supporting multiple boards with a single image
>
> This now requires having an entry with the GUID for every board which
> might lead to larger UpdateCapsule images.
Do we have a test case were a capsule contains images that shall not be
installed?
>
> == Tooling ==
>
> This series introduces a new tool: genguid. This can be used to generate
> the same GUIDs that the board would at runtime.
the GUIDs that the board requires at runtime.
Best regards
Heinrich
>
> This series follows a related discussion started by Ilias:
> https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72WkYLizrw@mail.gmail.com/
>
> ---
> Changes in v3:
> - Add manpage for genguid
> - Add dedicated CONFIG_TOOLS_GENGUID option
> - Minor code fixes addressing v2 feedback
> - Link to v2: https://lore.kernel.org/r/20240529-b4-dynamic-uuid-v2-0-c26f31057bbe@linaro.org
>
> Changes in v2:
> - Move namespace UUID to be defined in defconfig
> - Add tests and tooling
> - Only use the first board compatible to generate UUID.
> - Link to v1: https://lore.kernel.org/r/20240426-b4-dynamic-uuid-v1-0-e8154e00ec44@linaro.org
>
> ---
> Caleb Connolly (7):
> lib: uuid: add UUID v5 support
> efi: add a helper to generate dynamic UUIDs
> doc: uefi: document dynamic UUID generation
> sandbox: switch to dynamic UUIDs
> lib: uuid: supporting building as part of host tools
> tools: add genguid tool
> test: lib/uuid: add unit tests for dynamic UUIDs
>
> arch/Kconfig | 1 +
> board/sandbox/sandbox.c | 16 ---
> configs/sandbox_defconfig | 1 +
> configs/sandbox_flattree_defconfig | 1 +
> doc/develop/uefi/uefi.rst | 31 +++++
> doc/genguid.1 | 52 +++++++
> include/sandbox_efi_capsule.h | 6 +-
> include/uuid.h | 21 ++-
> lib/Kconfig | 8 ++
> lib/efi_loader/Kconfig | 23 +++
> lib/efi_loader/efi_capsule.c | 1 +
> lib/efi_loader/efi_firmware.c | 66 +++++++++
> lib/uuid.c | 81 +++++++++--
> test/lib/uuid.c | 88 ++++++++++++
> .../test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
> .../test_efi_capsule/test_capsule_firmware_raw.py | 8 +-
> .../test_capsule_firmware_signed_fit.py | 2 +-
> .../test_capsule_firmware_signed_raw.py | 4 +-
> test/py/tests/test_efi_capsule/version.dts | 6 +-
> tools/Kconfig | 7 +
> tools/Makefile | 3 +
> tools/binman/etype/efi_capsule.py | 2 +-
> tools/binman/ftest.py | 2 +-
> tools/genguid.c | 154 +++++++++++++++++++++
> 24 files changed, 538 insertions(+), 48 deletions(-)
> ---
> change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27
> base-commit: 2e682a4a406fc81ef32e05c28542cc8067f1e15f
>
> // Caleb (they/them)
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
2024-06-05 2:13 ` Simon Glass
@ 2024-06-05 6:36 ` Heinrich Schuchardt
2024-06-05 6:38 ` Heinrich Schuchardt
2024-06-05 9:25 ` Ilias Apalodimas
2024-06-20 14:58 ` Vincent Stehlé
2 siblings, 2 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-05 6:36 UTC (permalink / raw)
To: Caleb Connolly
Cc: Richard Hughes, u-boot, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
On 5/31/24 15:50, Caleb Connolly wrote:
> Add a tool that can generate GUIDs that match those generated internally
> by U-Boot for capsule update fw_images.
>
> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> with the board model, compatible, and fw_image name.
>
> This tool accepts the same inputs and will produce the same GUID as
> U-Boot would at runtime.
This functionality should be integrated into the mkeficapsule.
Just pass the device-tree into mkeficapsule and generate the GUIDs
whereever they are needed.
Best regards
Heinrich
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> doc/genguid.1 | 52 +++++++++++++++++++
> tools/Kconfig | 7 +++
> tools/Makefile | 3 ++
> tools/genguid.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 216 insertions(+)
>
> diff --git a/doc/genguid.1 b/doc/genguid.1
> new file mode 100644
> index 000000000000..4128055b3a9a
> --- /dev/null
> +++ b/doc/genguid.1
> @@ -0,0 +1,52 @@
> +.\" SPDX-License-Identifier: GPL-2.0+
> +.\" Copyright (c) 2024, Linaro Limited
> +.TH GENGUID 1 "May 2024"
> +
> +.SH NAME
> +genguid \- Generate deterministic EFI capsule image GUIDs for a board
> +
> +.SH SYNOPSIS
> +.B genguid
> +.RI GUID " " [ -vj ] " " -c " " COMPAT " " NAME...
> +
> +.SH "DESCRIPTION"
> +The
> +.B genguid
> +command is used to determine the update image GUIDs for a board using
> +dynamic UUIDs. The command takes a namespace GUID (defined in the boards
> +defconfig), the boards first compatible string, and the names of the
> +firmware images. The command will output the GUIDs for each image.
> +
> +As the dynamic UUID mechanism generates GUIDs at runtime, it would be
> +necessary to actually boot U-Boot on the board and enable debug logs
> +to retrieve the generated GUIDs. This tools just simplifies that process.
> +
> +.SH "OPTIONS"
> +
> +.TP
> +.BI GUID
> +The namespace/salt GUID, same as CONFIG_EFI_CAPSULE_NAMESPACE_UUID.
> +The format is:
> + xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> +
> +.TP
> +.BI "-v\fR,\fB --verbose "
> +Print additional information to stderr.
> +
> +.TP
> +.BI "-j\fR,\fB --json "
> +Output the results in JSON format (array of object with name/uuid properties).
> +
> +.TP
> +.BI "-c\fR,\fB --compat " COMPAT
> +The first entry in the boards root compatible property.
> +
> +.TP
> +.BI NAME...
> +The names of the firmware images to generate GUIDs for (e.g. "SANDBOX-UBOOT-ENV").
> +
> +.SH AUTHORS
> +Written by Caleb Connolly <caleb.connolly@linaro.org>
> +
> +.SH HOMEPAGE
> +https://u-boot.org
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 667807b33173..13201ff61fd4 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -103,8 +103,15 @@ config TOOLS_MKEFICAPSULE
> This command allows users to create a UEFI capsule file and,
> optionally sign that file. If you want to enable UEFI capsule
> update feature on your target, you certainly need this.
>
> +config TOOLS_GENGUID
> + bool "Build genguid command"
> + default y if EFI_CAPSULE_DYNAMIC_UUIDS
Distros have a package u-boot-tools. You want this package to contain
all tools.
Please, ensure that the new tool is built by tools-only_defconfig.
> + help
> + This command allows users to generate the GUIDs that a given
> + board would use for UEFI capsule update feature.
> +
> menuconfig FSPI_CONF_HEADER
> bool "FlexSPI Header Configuration"
> help
> FSPI Header Configuration
> diff --git a/tools/Makefile b/tools/Makefile
> index 6a4280e3668f..29e9a93b0f24 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -253,8 +253,11 @@ HOSTLDLIBS_mkeficapsule += \
> HOSTLDLIBS_mkeficapsule += \
> $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
> hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
> +genguid-objs := generated/lib/uuid.o generated/lib/sha1.o genguid.o
> +hostprogs-$(CONFIG_TOOLS_GENGUID) += genguid
> +
> mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
> HOSTLDLIBS_mkfwumdata += -luuid
> hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
>
> diff --git a/tools/genguid.c b/tools/genguid.c
> new file mode 100644
> index 000000000000..e71bc1d48f95
> --- /dev/null
> +++ b/tools/genguid.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 Linaro Ltd.
> + * Author: Caleb Connolly
> + */
> +
> +#include <getopt.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/types.h>
> +
> +#include <uuid.h>
> +
> +static struct option options[] = {
> + {"dtb", required_argument, NULL, 'd'},
> + {"compat", required_argument, NULL, 'c'},
> + {"help", no_argument, NULL, 'h'},
> + {"verbose", no_argument, NULL, 'v'},
> + {"json", no_argument, NULL, 'j'},
> + {NULL, 0, NULL, 0},
> +};
> +
> +static void usage(const char *progname)
> +{
> + fprintf(stderr, "Usage: %s GUID [-v] -c COMPAT NAME...\n", progname);
> + fprintf(stderr,
> + "Generate a v5 GUID for one of more U-Boot fw_images the same way U-Boot does at runtime.\n");
> + fprintf(stderr,
> + "\nOptions:\n"
> + " GUID namespace/salt GUID in 8-4-4-4-12 format\n"
> + " -h, --help display this help and exit\n"
> + " -c, --compat=COMPAT first compatible property in the board devicetree\n"
We don't need the first compatible string but the one in the root node.
> + " -v, --verbose print debug messages\n"
> + " -j, --json output in JSON format\n"
> + " NAME... one or more names of fw_images to generate GUIDs for\n"
> + );
> + fprintf(stderr, "\nExample:\n");
> + fprintf(stderr, " %s 2a5aa852-b856-4d97-baa9-5c5f4421551f \\\n"
> + "\t-c \"qcom,qrb4210-rb2\" \\\n"
> + "\tQUALCOMM-UBOOT\n", progname);
> +}
> +
> +static size_t u16_strsize(const uint16_t *in)
> +{
> + size_t i = 0, count = UINT16_MAX;
> +
> + while (count-- && in[i])
> + i++;
> +
> + return (i + 1) * sizeof(uint16_t);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + struct uuid namespace;
> + char *namespace_str;
> + char uuid_str[37];
> + char **image_uuids;
> + char *compatible = NULL;
> + uint16_t **images_u16;
> + char **images;
> + int c, n_images;
> + bool debug = false, json = false;
> +
> + if (argc < 2) {
> + usage(argv[0]);
> + return 1;
> + }
> +
> + namespace_str = argv[1];
> +
> + /* The first arg is the GUID so skip it */
> + while ((c = getopt_long(argc, argv, "c:hvj", options, NULL)) != -1) {
> + switch (c) {
> + case 'c':
> + compatible = strdup(optarg);
> + break;
> + case 'h':
> + usage(argv[0]);
> + return 0;
> + case 'v':
> + debug = true;
> + break;
> + case 'j':
> + json = true;
> + break;
> + default:
> + usage(argv[0]);
> + return 1;
> + }
> + }
> +
> + if (!compatible) {
> + fprintf(stderr, "ERROR: Please specify the compatible property.\n\n");
> + usage(argv[0]);
> + return 1;
> + }
> +
> + if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_GUID)) {
> + fprintf(stderr, "ERROR: Check that your UUID is formatted correctly.\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* This is probably not the best way to convert a string to a "u16" string */
Do you mean UTF-16?
Instead of writing "not the best way", please, describe the restrictions.
> + n_images = argc - optind - 1;
> + images = argv + optind + 1;
> + images_u16 = calloc(n_images, sizeof(char *));
Please, check that the result in not NULL.
> + for (int i = 0; i < n_images; i++) {
> + images_u16[i] = calloc(1, strlen(images[i]) * 2 + 2);
ditto
> + for (int j = 0; j < strlen(images[i]); j++)
> + images_u16[i][j] = (uint16_t)images[i][j];
This is definitively not working for non-ASCII characters. You should
throw an error for non-ASCII or provide a conversion routine.
Best regards
Heinrich
> + }
> +
> + if (debug) {
> + fprintf(stderr, "GUID: ");
> + uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_GUID);
> + fprintf(stderr, "%s\n", uuid_str);
> + fprintf(stderr, "Compatible: \"%s\"\n", compatible);
> + fprintf(stderr, "Images: ");
> + for (int i = 0; i < n_images; i++)
> + fprintf(stderr, "\"%s\"%s", argv[optind + i + 1],
> + i == n_images - 1 ? "\n" : ", ");
> + }
> +
> + image_uuids = calloc(n_images, sizeof(char *));
> + for (int i = 0; i < n_images; i++) {
> + struct uuid image_type_id;
> +
> + gen_uuid_v5(&namespace, &image_type_id,
> + compatible, strlen(compatible),
> + images_u16[i], u16_strsize(images_u16[i]) - sizeof(uint16_t),
> + NULL);
> +
> + uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_GUID);
> + image_uuids[i] = strdup(uuid_str);
> + }
> +
> + if (json) {
> + printf("[\n");
> + for (int i = 0; i < n_images; i++)
> + printf("\t{\"name\": \"%s\", \"uuid\": \"%s\"}%s\n", images[i], image_uuids[i],
> + i == n_images - 1 ? "" : ",");
> + printf("]\n");
> + } else {
> + for (int i = 0; i < n_images; i++)
> + printf("%-24s| %s\n", images[i], image_uuids[i]);
> + }
> +
> + return 0;
> +}
> +
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-06-05 6:36 ` Heinrich Schuchardt
@ 2024-06-05 6:38 ` Heinrich Schuchardt
2024-06-05 9:25 ` Ilias Apalodimas
1 sibling, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-05 6:38 UTC (permalink / raw)
To: Caleb Connolly
Cc: Richard Hughes, u-boot, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
On 6/5/24 08:36, Heinrich Schuchardt wrote:
> On 5/31/24 15:50, Caleb Connolly wrote:
>> Add a tool that can generate GUIDs that match those generated internally
>> by U-Boot for capsule update fw_images.
>>
>> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
>> with the board model, compatible, and fw_image name.
>>
>> This tool accepts the same inputs and will produce the same GUID as
>> U-Boot would at runtime.
>
> This functionality should be integrated into the mkeficapsule.
>
> Just pass the device-tree into mkeficapsule and generate the GUIDs
> whereever they are needed.
>
> Best regards
>
> Heinrich
>
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>> doc/genguid.1 | 52 +++++++++++++++++++
>> tools/Kconfig | 7 +++
>> tools/Makefile | 3 ++
>> tools/genguid.c | 154
A change to MAINTAINERS is missing.
Best regards
Heinrich
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 216 insertions(+)
>>
>> diff --git a/doc/genguid.1 b/doc/genguid.1
>> new file mode 100644
>> index 000000000000..4128055b3a9a
>> --- /dev/null
>> +++ b/doc/genguid.1
>> @@ -0,0 +1,52 @@
>> +.\" SPDX-License-Identifier: GPL-2.0+
>> +.\" Copyright (c) 2024, Linaro Limited
>> +.TH GENGUID 1 "May 2024"
>> +
>> +.SH NAME
>> +genguid \- Generate deterministic EFI capsule image GUIDs for a board
>> +
>> +.SH SYNOPSIS
>> +.B genguid
>> +.RI GUID " " [ -vj ] " " -c " " COMPAT " " NAME...
>> +
>> +.SH "DESCRIPTION"
>> +The
>> +.B genguid
>> +command is used to determine the update image GUIDs for a board using
>> +dynamic UUIDs. The command takes a namespace GUID (defined in the boards
>> +defconfig), the boards first compatible string, and the names of the
>> +firmware images. The command will output the GUIDs for each image.
>> +
>> +As the dynamic UUID mechanism generates GUIDs at runtime, it would be
>> +necessary to actually boot U-Boot on the board and enable debug logs
>> +to retrieve the generated GUIDs. This tools just simplifies that
>> process.
>> +
>> +.SH "OPTIONS"
>> +
>> +.TP
>> +.BI GUID
>> +The namespace/salt GUID, same as CONFIG_EFI_CAPSULE_NAMESPACE_UUID.
>> +The format is:
>> + xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
>> +
>> +.TP
>> +.BI "-v\fR,\fB --verbose "
>> +Print additional information to stderr.
>> +
>> +.TP
>> +.BI "-j\fR,\fB --json "
>> +Output the results in JSON format (array of object with name/uuid
>> properties).
>> +
>> +.TP
>> +.BI "-c\fR,\fB --compat " COMPAT
>> +The first entry in the boards root compatible property.
>> +
>> +.TP
>> +.BI NAME...
>> +The names of the firmware images to generate GUIDs for (e.g.
>> "SANDBOX-UBOOT-ENV").
>> +
>> +.SH AUTHORS
>> +Written by Caleb Connolly <caleb.connolly@linaro.org>
>> +
>> +.SH HOMEPAGE
>> +https://u-boot.org
>> diff --git a/tools/Kconfig b/tools/Kconfig
>> index 667807b33173..13201ff61fd4 100644
>> --- a/tools/Kconfig
>> +++ b/tools/Kconfig
>> @@ -103,8 +103,15 @@ config TOOLS_MKEFICAPSULE
>> This command allows users to create a UEFI capsule file and,
>> optionally sign that file. If you want to enable UEFI capsule
>> update feature on your target, you certainly need this.
>>
>> +config TOOLS_GENGUID
>> + bool "Build genguid command"
>> + default y if EFI_CAPSULE_DYNAMIC_UUIDS
>
> Distros have a package u-boot-tools. You want this package to contain
> all tools.
>
> Please, ensure that the new tool is built by tools-only_defconfig.
>
>> + help
>> + This command allows users to generate the GUIDs that a given
>> + board would use for UEFI capsule update feature.
>> +
>> menuconfig FSPI_CONF_HEADER
>> bool "FlexSPI Header Configuration"
>> help
>> FSPI Header Configuration
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 6a4280e3668f..29e9a93b0f24 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -253,8 +253,11 @@ HOSTLDLIBS_mkeficapsule += \
>> HOSTLDLIBS_mkeficapsule += \
>> $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
>> hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>>
>> +genguid-objs := generated/lib/uuid.o generated/lib/sha1.o genguid.o
>> +hostprogs-$(CONFIG_TOOLS_GENGUID) += genguid
>> +
>> mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
>> HOSTLDLIBS_mkfwumdata += -luuid
>> hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
>>
>> diff --git a/tools/genguid.c b/tools/genguid.c
>> new file mode 100644
>> index 000000000000..e71bc1d48f95
>> --- /dev/null
>> +++ b/tools/genguid.c
>> @@ -0,0 +1,154 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2024 Linaro Ltd.
>> + * Author: Caleb Connolly
>> + */
>> +
>> +#include <getopt.h>
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <linux/types.h>
>> +
>> +#include <uuid.h>
>> +
>> +static struct option options[] = {
>> + {"dtb", required_argument, NULL, 'd'},
>> + {"compat", required_argument, NULL, 'c'},
>> + {"help", no_argument, NULL, 'h'},
>> + {"verbose", no_argument, NULL, 'v'},
>> + {"json", no_argument, NULL, 'j'},
>> + {NULL, 0, NULL, 0},
>> +};
>> +
>> +static void usage(const char *progname)
>> +{
>> + fprintf(stderr, "Usage: %s GUID [-v] -c COMPAT NAME...\n",
>> progname);
>> + fprintf(stderr,
>> + "Generate a v5 GUID for one of more U-Boot fw_images the same
>> way U-Boot does at runtime.\n");
>> + fprintf(stderr,
>> + "\nOptions:\n"
>> + " GUID namespace/salt GUID in 8-4-4-4-12
>> format\n"
>> + " -h, --help display this help and exit\n"
>> + " -c, --compat=COMPAT first compatible property in the
>> board devicetree\n"
>
> We don't need the first compatible string but the one in the root node.
>
>> + " -v, --verbose print debug messages\n"
>> + " -j, --json output in JSON format\n"
>> + " NAME... one or more names of fw_images to
>> generate GUIDs for\n"
>> + );
>> + fprintf(stderr, "\nExample:\n");
>> + fprintf(stderr, " %s 2a5aa852-b856-4d97-baa9-5c5f4421551f \\\n"
>> + "\t-c \"qcom,qrb4210-rb2\" \\\n"
>> + "\tQUALCOMM-UBOOT\n", progname);
>> +}
>> +
>> +static size_t u16_strsize(const uint16_t *in)
>> +{
>> + size_t i = 0, count = UINT16_MAX;
>> +
>> + while (count-- && in[i])
>> + i++;
>> +
>> + return (i + 1) * sizeof(uint16_t);
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> + struct uuid namespace;
>> + char *namespace_str;
>> + char uuid_str[37];
>> + char **image_uuids;
>> + char *compatible = NULL;
>> + uint16_t **images_u16;
>> + char **images;
>> + int c, n_images;
>> + bool debug = false, json = false;
>> +
>> + if (argc < 2) {
>> + usage(argv[0]);
>> + return 1;
>> + }
>> +
>> + namespace_str = argv[1];
>> +
>> + /* The first arg is the GUID so skip it */
>> + while ((c = getopt_long(argc, argv, "c:hvj", options, NULL)) !=
>> -1) {
>> + switch (c) {
>> + case 'c':
>> + compatible = strdup(optarg);
>> + break;
>> + case 'h':
>> + usage(argv[0]);
>> + return 0;
>> + case 'v':
>> + debug = true;
>> + break;
>> + case 'j':
>> + json = true;
>> + break;
>> + default:
>> + usage(argv[0]);
>> + return 1;
>> + }
>> + }
>> +
>> + if (!compatible) {
>> + fprintf(stderr, "ERROR: Please specify the compatible
>> property.\n\n");
>> + usage(argv[0]);
>> + return 1;
>> + }
>> +
>> + if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace,
>> UUID_STR_FORMAT_GUID)) {
>> + fprintf(stderr, "ERROR: Check that your UUID is formatted
>> correctly.\n");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + /* This is probably not the best way to convert a string to a
>> "u16" string */
>
> Do you mean UTF-16?
>
> Instead of writing "not the best way", please, describe the restrictions.
>
>> + n_images = argc - optind - 1;
>> + images = argv + optind + 1;
>> + images_u16 = calloc(n_images, sizeof(char *));
>
> Please, check that the result in not NULL.
>
>> + for (int i = 0; i < n_images; i++) {
>> + images_u16[i] = calloc(1, strlen(images[i]) * 2 + 2);
>
> ditto
>
>> + for (int j = 0; j < strlen(images[i]); j++)
>> + images_u16[i][j] = (uint16_t)images[i][j];
>
> This is definitively not working for non-ASCII characters. You should
> throw an error for non-ASCII or provide a conversion routine.
>
> Best regards
>
> Heinrich
>
>> + }
>> +
>> + if (debug) {
>> + fprintf(stderr, "GUID: ");
>> + uuid_bin_to_str((uint8_t *)&namespace, uuid_str,
>> UUID_STR_FORMAT_GUID);
>> + fprintf(stderr, "%s\n", uuid_str);
>> + fprintf(stderr, "Compatible: \"%s\"\n", compatible);
>> + fprintf(stderr, "Images: ");
>> + for (int i = 0; i < n_images; i++)
>> + fprintf(stderr, "\"%s\"%s", argv[optind + i + 1],
>> + i == n_images - 1 ? "\n" : ", ");
>> + }
>> +
>> + image_uuids = calloc(n_images, sizeof(char *));
>> + for (int i = 0; i < n_images; i++) {
>> + struct uuid image_type_id;
>> +
>> + gen_uuid_v5(&namespace, &image_type_id,
>> + compatible, strlen(compatible),
>> + images_u16[i], u16_strsize(images_u16[i]) -
>> sizeof(uint16_t),
>> + NULL);
>> +
>> + uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str,
>> UUID_STR_FORMAT_GUID);
>> + image_uuids[i] = strdup(uuid_str);
>> + }
>> +
>> + if (json) {
>> + printf("[\n");
>> + for (int i = 0; i < n_images; i++)
>> + printf("\t{\"name\": \"%s\", \"uuid\": \"%s\"}%s\n",
>> images[i], image_uuids[i],
>> + i == n_images - 1 ? "" : ",");
>> + printf("]\n");
>> + } else {
>> + for (int i = 0; i < n_images; i++)
>> + printf("%-24s| %s\n", images[i], image_uuids[i]);
>> + }
>> +
>> + return 0;
>> +}
>> +
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-06-05 6:36 ` Heinrich Schuchardt
2024-06-05 6:38 ` Heinrich Schuchardt
@ 2024-06-05 9:25 ` Ilias Apalodimas
2024-06-05 12:29 ` Caleb Connolly
1 sibling, 1 reply; 36+ messages in thread
From: Ilias Apalodimas @ 2024-06-05 9:25 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Caleb Connolly, Richard Hughes, u-boot, Tom Rini, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Hi Heinrich
On Wed, 5 Jun 2024 at 09:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 5/31/24 15:50, Caleb Connolly wrote:
> > Add a tool that can generate GUIDs that match those generated internally
> > by U-Boot for capsule update fw_images.
> >
> > Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> > with the board model, compatible, and fw_image name.
> >
> > This tool accepts the same inputs and will produce the same GUID as
> > U-Boot would at runtime.
>
> This functionality should be integrated into the mkeficapsule.
>
> Just pass the device-tree into mkeficapsule and generate the GUIDs
> whereever they are needed.
I like the idea of moving this mkeficapsule which is already part of
distro packages but, isn't it easier to pass the compatible string
node and the namespace?
Thanks
/Ilias
>
> Best regards
>
> Heinrich
[...]
Thanks
/Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/7] lib: uuid: add UUID v5 support
2024-06-05 2:13 ` Simon Glass
@ 2024-06-05 12:06 ` Caleb Connolly
0 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-06-05 12:06 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes, u-boot
Hi Simon,
On 05/06/2024 04:13, Simon Glass wrote:
> Hi Caleb,
>
> On Fri, 31 May 2024 at 07:50, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> Add support for generating version 5 UUIDs, these are determistic and work
>
> spelling
>
>> by hashing a "namespace" UUID together with some unique data. One intended
>> usecase is to allow for dynamically generate payload UUIDs for UEFI
>> capsule updates, so that supported boards can have their own UUIDs
>> without needing to hardcode them.
>>
>> Tests for this are added in an upcoming patch.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>> include/uuid.h | 17 +++++++++++++++++
>> lib/Kconfig | 8 ++++++++
>> lib/uuid.c | 37 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 62 insertions(+)
>>
>> diff --git a/include/uuid.h b/include/uuid.h
>> index f5a941250f48..539affaa47b9 100644
>> --- a/include/uuid.h
>> +++ b/include/uuid.h
>> @@ -10,8 +10,9 @@
>> #ifndef __UUID_H__
>> #define __UUID_H__
>>
>> #include <linux/bitops.h>
>> +#include <linux/kconfig.h>
>>
>> /*
>> * UUID - Universally Unique IDentifier - 128 bits unique number.
>> * There are 5 versions and one variant of UUID defined by RFC4122
>> @@ -142,8 +143,24 @@ void gen_rand_uuid(unsigned char *uuid_bin);
>> * @param - uuid output type: UUID - 0, GUID - 1
>> */
>> void gen_rand_uuid_str(char *uuid_str, int str_format);
>>
>> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
>> +/**
>> + * gen_uuid_v5() - generate UUID v5 from namespace and other seed data.
>> + *
>> + * @namespace: pointer to UUID namespace salt
>> + * @uuid: pointer to allocated UUID output
>> + * @...: NULL terminated list of seed data as pairs of pointers
>> + * to data and their lengths
>> + */
>> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...);
>> +#else
>> +static inline void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
>> +{
>> +}
>> +#endif
>
> Can you explain somewhere why the static inline is needed?
The explicit "inline" certainly isn't, but this must be static because
it's defined in a header, so we'd otherwise have multiple definitions
for every #include.
>
>> +
>> /**
>> * uuid_str_to_le_bin() - Convert string UUID to little endian binary data.
>> * @uuid_str: pointer to UUID string
>> * @uuid_bin: pointer to allocated array for little endian output [16B]
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 189e6eb31aa1..2941532f25cf 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -80,8 +80,16 @@ config RANDOM_UUID
>> help
>> Enable the generation of partitions with random UUIDs if none
>> are provided.
>>
>> +config UUID_GEN_V5
>> + bool "Enable UUID version 5 generation"
>> + select LIB_UUID
>> + depends on SHA1
>> + help
>> + Enable the generation of version 5 UUIDs, these are determistic and
>
> spelling
>
>> + generated from a namespace UUID, and a string (such as a board name).
>> +
>> config SPL_LIB_UUID
>> depends on SPL
>> bool
>>
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index dfa2320ba267..2df0523e717f 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -21,8 +21,9 @@
>> #include <part_efi.h>
>> #include <malloc.h>
>> #include <dm/uclass.h>
>> #include <rng.h>
>> +#include <u-boot/sha1.h>
>>
>> int uuid_str_valid(const char *uuid)
>> {
>> int i, valid;
>> @@ -368,8 +369,44 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str,
>> }
>> }
>> }
>>
>> +#if IS_ENABLED(CONFIG_UUID_GEN_V5)
>> +void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
>> +{
>> + sha1_context ctx;
>> + va_list args;
>> + const uint8_t *data;
>> + uint8_t hash[SHA1_SUM_LEN];
>> + uint32_t tmp;
>> +
>> + sha1_starts(&ctx);
>> + /* Hash the namespace UUID as salt */
>> + sha1_update(&ctx, (unsigned char *)namespace, UUID_BIN_LEN);
>> + va_start(args, uuid);
>> +
>> + while ((data = va_arg(args, const uint8_t *))) {
>> + unsigned int len = va_arg(args, size_t);
>> +
>> + sha1_update(&ctx, data, len);
>> + }
>> +
>> + va_end(args);
>> + sha1_finish(&ctx, hash);
>> +
>> + /* Truncate the hash into output UUID, it is already big endian */
>> + memcpy(uuid, hash, sizeof(*uuid));
>> +
>> + /* Configure variant/version bits */
>> + tmp = be32_to_cpu(uuid->time_hi_and_version);
>> + tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
>> + uuid->time_hi_and_version = cpu_to_be32(tmp);
>> +
>> + uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
>> + uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
>> +}
>> +#endif
>> +
>> #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID)
>> void gen_rand_uuid(unsigned char *uuid_bin)
>> {
>> u32 ptr[4];
>>
>> --
>> 2.45.0
>>
>
> Regards,
> Simon
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs
2024-06-05 5:52 ` Heinrich Schuchardt
@ 2024-06-05 12:22 ` Caleb Connolly
0 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-06-05 12:22 UTC (permalink / raw)
To: Heinrich Schuchardt, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
Cc: Richard Hughes, u-boot
On 05/06/2024 07:52, Heinrich Schuchardt wrote:
> On 5/31/24 15:50, Caleb Connolly wrote:
>> Introduce a new helper efi_capsule_update_info_gen_ids() which populates
>> the capsule update fw images image_type_id field. This allows for
>> determinstic UUIDs to be used that can scale to a large number of
>> different boards and board variants without the need to maintain a big
>> list.
>>
>> We call this from efi_fill_image_desc_array() to populate the UUIDs
>> lazily on-demand.
>>
>> This is behind an additional config option as it depends on V5 UUIDs and
>> the SHA1 implementation.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>> lib/efi_loader/Kconfig | 23 +++++++++++++++
>> lib/efi_loader/efi_capsule.c | 1 +
>> lib/efi_loader/efi_firmware.c | 66
>> +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 90 insertions(+)
>>
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index 430bb7f0f7dc..e90caf4f8e14 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -235,8 +235,31 @@ config EFI_CAPSULE_ON_DISK_EARLY
>> If this option is enabled, capsules will be enforced to be
>> executed as part of U-Boot initialisation so that they will
>> surely take place whatever is set to distro_bootcmd.
>>
>> +config EFI_CAPSULE_DYNAMIC_UUIDS
>> + bool "Dynamic UUIDs for capsules"
>> + depends on EFI_HAVE_CAPSULE_SUPPORT
>> + select UUID_GEN_V5
>
> This select will create a build error if CONFIG_SHA1=n as
> CONFIG_UUID_GEN_V5 depends on it.
Ack
>
>> + help
>> + Select this option if you want to use dynamically generated v5
>> + UUIDs for your board. To make use of this feature, your board
>> + code should call efi_capsule_update_info_gen_ids() with a seed
>> + UUID to generate the image_type_id field for each fw_image.
>> +
>> + The CapsuleUpdate payloads are expected to generate matching UUIDs
>> + using the same scheme.
>> +
>> +config EFI_CAPSULE_NAMESPACE_UUID
>> + string "Namespace UUID for dynamic UUIDs"
>> + depends on EFI_CAPSULE_DYNAMIC_UUIDS
>> + help
>> + Define the namespace or "salt" UUID used to generate the per-image
>> + UUIDs. This should be a UUID in the standard 8-4-4-4-12 format.
>
> As UUIDs can be formatted low-endian or big-endian I would not know how
> the value will be interpreted.
Arguably it doesn't matter, it's just salt which is roughly UUID shaped :D
>
> Why do we need a vendor specific name-space if we are using compatible
> strings which are vendor specific themselves?
If vendor A and vendor B both build embedded products based on a
Qualcomm RB2 (for example), they might both use the same DTB but
customise other parts of U-Boot. We want them both to be able to use
LVFS to provide updates, so they need a way to avoid conflicting UUIDs.
I think requiring them to use a custom compatible might be a bit
arbitrary, especially if they aren't using any custom hardware.
>
>> +
>> + Device vendors are expected to generate their own namespace UUID
>> + to avoid conflicts with existing products.
>> +
>> config EFI_CAPSULE_FIRMWARE
>> bool
>>
>> config EFI_CAPSULE_FIRMWARE_MANAGEMENT
>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>> index 0937800e588f..ac02e79ae7d8 100644
>> --- a/lib/efi_loader/efi_capsule.c
>> +++ b/lib/efi_loader/efi_capsule.c
>> @@ -19,8 +19,9 @@
>> #include <mapmem.h>
>> #include <sort.h>
>> #include <sysreset.h>
>> #include <asm/global_data.h>
>> +#include <uuid.h>
>>
>> #include <crypto/pkcs7.h>
>> #include <crypto/pkcs7_parser.h>
>> #include <linux/err.h>
>> diff --git a/lib/efi_loader/efi_firmware.c
>> b/lib/efi_loader/efi_firmware.c
>> index ba5aba098c0f..a8dafe4f01a5 100644
>> --- a/lib/efi_loader/efi_firmware.c
>> +++ b/lib/efi_loader/efi_firmware.c
>> @@ -244,8 +244,71 @@ void efi_firmware_fill_version_info(struct
>> efi_firmware_image_descriptor *image_
>>
>> free(var_state);
>> }
>>
>> +#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
>> +/**
>> + * efi_capsule_update_info_gen_ids - generate GUIDs for the images
>> + *
>> + * Generate the image_type_id for each image in the
>> update_info.images array
>> + * using the first compatible from the device tree and a salt
>> + * UUID defined at build time.
>> + *
>> + * Returns: status code
>> + */
>> +static efi_status_t efi_capsule_update_info_gen_ids(void)
>> +{
>> + int ret, i;
>> + struct uuid namespace;
>> + const char *compatible; /* Full array including null bytes */
>> + struct efi_fw_image *fw_array;
>> +
>> + fw_array = update_info.images;
>> + /* Check if we need to run (there are images and we didn't
>> already generate their IDs) */
>> + if (!update_info.num_images ||
>> + memchr_inv(&fw_array[0].image_type_id, 0,
>> sizeof(fw_array[0].image_type_id)))
>> + return EFI_SUCCESS;
>> +
>> + ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
>> + (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
>> + if (ret) {
>> + log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid:
>> %d\n", __func__, ret);
>> + return EFI_UNSUPPORTED;
>> + }
>
> You don't want end-users to be the first to find this issue. This must
> be a build time check.
Simply boot testing a production build should hit this error path... But
sure a built time check would be better. Could you suggest how I might
do this? Can we have host tools that are used during build? Where abouts
should I add this?
>
>> +
>> + compatible = ofnode_read_string(ofnode_root(), "compatible");
>> +
>> + if (!compatible) {
>> + log_debug("%s: model or compatible not defined\n", __func__);
>
> You are not reading model here.
Ah yeah.
>
>> + return EFI_UNSUPPORTED;
>> + }
>> +
>> + if (!update_info.num_images) {
>> + log_debug("%s: no fw_images, make sure update_info.num_images
>> is set\n", __func__);
>
> I thought we were using capsules without images in A/B updates and need
> to process them?
I'm not sure I understand. This is just a sanity check, though it's
actually duplicated above and this one should be dropped (it's a
holdover from a previous version of this patch).
>
>> + return -ENODATA;
>> + }
>> +
>> + for (i = 0; i < update_info.num_images; i++) {
>> + gen_uuid_v5(&namespace,
>> + (struct uuid *)&fw_array[i].image_type_id,
>> + compatible, strlen(compatible),
>> + fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
>> + - sizeof(uint16_t),
>> + NULL);
>> +
>> + log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
>> + &fw_array[i].image_type_id);
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +#else
>> +static efi_status_t efi_capsule_update_info_gen_ids(void)
>> +{
>> + return EFI_SUCCESS;
>
> Why should we have a case were we don't generate the image UUIDs?
> Don't we get rid of capsule GUIDs in the device-trees?
Many boards have hardcoded GUIDs, which may or may not be in used. So we
can't just break them all by migrating them over. This will have to be
done incrementally by device maintainers.
>
> Best regards
>
> Heinrich
>
>> +}
>> +#endif
>> +
>> /**
>> * efi_fill_image_desc_array - populate image descriptor array
>> * @image_info_size: Size of @image_info
>> * @image_info: Image information
>> @@ -282,8 +345,11 @@ static efi_status_t efi_fill_image_desc_array(
>> return EFI_BUFFER_TOO_SMALL;
>> }
>> *image_info_size = total_size;
>>
>> + if (efi_capsule_update_info_gen_ids() != EFI_SUCCESS)
>> + return EFI_UNSUPPORTED;
>> +
>> fw_array = update_info.images;
>> *descriptor_count = update_info.num_images;
>> *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
>> *descriptor_size = sizeof(*image_info);
>>
>
Kind regards,
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-06-05 9:25 ` Ilias Apalodimas
@ 2024-06-05 12:29 ` Caleb Connolly
2024-06-05 13:43 ` Ilias Apalodimas
0 siblings, 1 reply; 36+ messages in thread
From: Caleb Connolly @ 2024-06-05 12:29 UTC (permalink / raw)
To: Ilias Apalodimas, Heinrich Schuchardt
Cc: Richard Hughes, u-boot, Tom Rini, Simon Glass, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi
On 05/06/2024 11:25, Ilias Apalodimas wrote:
> Hi Heinrich
>
> On Wed, 5 Jun 2024 at 09:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 5/31/24 15:50, Caleb Connolly wrote:
>>> Add a tool that can generate GUIDs that match those generated internally
>>> by U-Boot for capsule update fw_images.
>>>
>>> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
>>> with the board model, compatible, and fw_image name.
>>>
>>> This tool accepts the same inputs and will produce the same GUID as
>>> U-Boot would at runtime.
>>
>> This functionality should be integrated into the mkeficapsule.
>>
>> Just pass the device-tree into mkeficapsule and generate the GUIDs
>> whereever they are needed.
>
> I like the idea of moving this mkeficapsule which is already part of
> distro packages but, isn't it easier to pass the compatible string
> node and the namespace?
Yeah, we only need the first entry in the compatible property, so
parsing the whole DT is a bit overkill.
I can look at combining these tools, can mkeficapsule generate a capsule
for multiple images?
Would we want it to work such that you provide the namespace GUID,
compatible, and image names, and have it generate a capsule with the
right image GUIDs?
Or would it be simple enough to just incorporate the functionality of
genguid?
Kind regards,
>
> Thanks
> /Ilias
>>
>> Best regards
>>
>> Heinrich
>
> [...]
>
> Thanks
> /Ilias
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-06-05 12:29 ` Caleb Connolly
@ 2024-06-05 13:43 ` Ilias Apalodimas
0 siblings, 0 replies; 36+ messages in thread
From: Ilias Apalodimas @ 2024-06-05 13:43 UTC (permalink / raw)
To: Caleb Connolly, Sughosh Ganu
Cc: Heinrich Schuchardt, Richard Hughes, u-boot, Tom Rini,
Simon Glass, Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
+ CC Sughosh
On Wed, 5 Jun 2024 at 15:29, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
>
> On 05/06/2024 11:25, Ilias Apalodimas wrote:
> > Hi Heinrich
> >
> > On Wed, 5 Jun 2024 at 09:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 5/31/24 15:50, Caleb Connolly wrote:
> >>> Add a tool that can generate GUIDs that match those generated internally
> >>> by U-Boot for capsule update fw_images.
> >>>
> >>> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> >>> with the board model, compatible, and fw_image name.
> >>>
> >>> This tool accepts the same inputs and will produce the same GUID as
> >>> U-Boot would at runtime.
> >>
> >> This functionality should be integrated into the mkeficapsule.
> >>
> >> Just pass the device-tree into mkeficapsule and generate the GUIDs
> >> whereever they are needed.
> >
> > I like the idea of moving this mkeficapsule which is already part of
> > distro packages but, isn't it easier to pass the compatible string
> > node and the namespace?
>
> Yeah, we only need the first entry in the compatible property, so
> parsing the whole DT is a bit overkill.
>
> I can look at combining these tools, can mkeficapsule generate a capsule
> for multiple images?
Not yet. There's [0] which adds that functionality but we are
discussing parsing a .yaml file instead of a custom format
>
> Would we want it to work such that you provide the namespace GUID,
> compatible, and image names, and have it generate a capsule with the
> right image GUIDs?
>
> Or would it be simple enough to just incorporate the functionality of
> genguid?
>
I'd argue for both since people might choose other tools to generate a capsule.
So you want 2 flags ideally (random flags)
-x: dump the autogenerated GUIDs
-y use the autogenerated GUIDs on capsule generation
Since mkeficapsule can't generate a capsule with multiple payloads
yet, you will be limited to just one, but I guess it's easy to expand
it once [0] is merged
[0] https://lore.kernel.org/u-boot/20240419065542.1160527-1-sughosh.ganu@linaro.org/
Thanks
/Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-05 5:59 ` [PATCH v3 0/7] efi: CapsuleUpdate: support " Heinrich Schuchardt
@ 2024-06-05 17:02 ` Caleb Connolly
0 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-06-05 17:02 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Richard Hughes, u-boot, Tom Rini, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi
On 05/06/2024 07:59, Heinrich Schuchardt wrote:
> On 5/31/24 15:50, Caleb Connolly wrote:
>> As more boards adopt support for the EFI CapsuleUpdate mechanism, there
>> is a growing issue of being able to target updates to them properly. The
>> current mechanism of hardcoding UUIDs for each board at compile time is
>> unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
>>
>> In this series, I propose that we adopt v5 GUIDs, these are generated
>> by using a well-known salt GUID as well as board specific information
>> (like the model/revision), these are hashed together and the result is
>> truncated to form a new UUID.
>>
>> The well-known salt GUID can be specific to the architecture (SoC
>> vendor), or OEM. It is defined in the board defconfig so that vendors
>> can easily bring their own.
>>
>> Specifically, the following fields are used to generate a GUID for a
>> particular fw_image:
>>
>> * namespace salt
>> * board compatible (usually the first entry in the dt root compatible
>> array).
>> * fw_image name (the string identifying the specific image, especially
>> relevant for board that can update multiple images).
>>
>> == Usage ==
>>
>> Boards can integrate dynamic UUID support as follows:
>>
>> 1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if
>> EFI_HAVE_CAPSULE_SUPPORT.
>> 2. Skip setting the fw_images image_type_id property.
>> 3. Generate a UUID and set CONFIG_EFI_CAPSULE_NAMESPACE_UUID in your
>> defconfig.
>
> Why should we have two alternatives?
>
> If we have the dynamic UUIDs, please, get rid of static ones.
I don't think this is possible to do without the risk of breaking some
boards. Can we assume that ALL existing capsule update image GUIDs are
safe to just scrap?
>
>>
>> == Limitations ==
>>
>> * Changing GUIDs
>>
>> The primary limitation with this approach is that if any of the source
>> fields change, so will the GUID for the board. It is therefore pretty
>> important to ensure that GUID changes are caught during development.
>>
>> * Supporting multiple boards with a single image
>>
>> This now requires having an entry with the GUID for every board which
>> might lead to larger UpdateCapsule images.
>
> Do we have a test case were a capsule contains images that shall not be
> installed?
>
>>
>> == Tooling ==
>>
>> This series introduces a new tool: genguid. This can be used to generate
>> the same GUIDs that the board would at runtime.
>
> the GUIDs that the board requires at runtime.
>
> Best regards
>
> Heinrich
>
>>
>> This series follows a related discussion started by Ilias:
>> https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72WkYLizrw@mail.gmail.com/
>>
>> ---
>> Changes in v3:
>> - Add manpage for genguid
>> - Add dedicated CONFIG_TOOLS_GENGUID option
>> - Minor code fixes addressing v2 feedback
>> - Link to v2:
>> https://lore.kernel.org/r/20240529-b4-dynamic-uuid-v2-0-c26f31057bbe@linaro.org
>>
>> Changes in v2:
>> - Move namespace UUID to be defined in defconfig
>> - Add tests and tooling
>> - Only use the first board compatible to generate UUID.
>> - Link to v1:
>> https://lore.kernel.org/r/20240426-b4-dynamic-uuid-v1-0-e8154e00ec44@linaro.org
>>
>> ---
>> Caleb Connolly (7):
>> lib: uuid: add UUID v5 support
>> efi: add a helper to generate dynamic UUIDs
>> doc: uefi: document dynamic UUID generation
>> sandbox: switch to dynamic UUIDs
>> lib: uuid: supporting building as part of host tools
>> tools: add genguid tool
>> test: lib/uuid: add unit tests for dynamic UUIDs
>>
>> arch/Kconfig | 1 +
>> board/sandbox/sandbox.c | 16 ---
>> configs/sandbox_defconfig | 1 +
>> configs/sandbox_flattree_defconfig | 1 +
>> doc/develop/uefi/uefi.rst | 31 +++++
>> doc/genguid.1 | 52 +++++++
>> include/sandbox_efi_capsule.h | 6 +-
>> include/uuid.h | 21 ++-
>> lib/Kconfig | 8 ++
>> lib/efi_loader/Kconfig | 23 +++
>> lib/efi_loader/efi_capsule.c | 1 +
>> lib/efi_loader/efi_firmware.c | 66 +++++++++
>> lib/uuid.c | 81 +++++++++--
>> test/lib/uuid.c | 88 ++++++++++++
>> .../test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
>> .../test_efi_capsule/test_capsule_firmware_raw.py | 8 +-
>> .../test_capsule_firmware_signed_fit.py | 2 +-
>> .../test_capsule_firmware_signed_raw.py | 4 +-
>> test/py/tests/test_efi_capsule/version.dts | 6 +-
>> tools/Kconfig | 7 +
>> tools/Makefile | 3 +
>> tools/binman/etype/efi_capsule.py | 2 +-
>> tools/binman/ftest.py | 2 +-
>> tools/genguid.c | 154
>> +++++++++++++++++++++
>> 24 files changed, 538 insertions(+), 48 deletions(-)
>> ---
>> change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27
>> base-commit: 2e682a4a406fc81ef32e05c28542cc8067f1e15f
>>
>> // Caleb (they/them)
>>
>
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
` (7 preceding siblings ...)
2024-06-05 5:59 ` [PATCH v3 0/7] efi: CapsuleUpdate: support " Heinrich Schuchardt
@ 2024-06-19 14:02 ` Vincent Stehlé
2024-06-19 19:15 ` Ilias Apalodimas
8 siblings, 1 reply; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-19 14:02 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:
> As more boards adopt support for the EFI CapsuleUpdate mechanism, there
> is a growing issue of being able to target updates to them properly. The
> current mechanism of hardcoding UUIDs for each board at compile time is
> unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
>
> In this series, I propose that we adopt v5 GUIDs, these are generated
> by using a well-known salt GUID as well as board specific information
> (like the model/revision), these are hashed together and the result is
> truncated to form a new UUID.
Dear Caleb,
Thank you for working on this proposal, this looks very useful.
Indeed, we found out during SystemReady certifications that it is easy for
unique ids to be inadvertently re-used, making them thus non-unique.
I have a doubt regarding the format of the generated UUIDs, which end up in the
ESRT, though.
Here is a quick experiment on the sandbox booting with a DTB using the efidebug
command.
With the patch series, rebased on the master branch:
$ make sandbox_defconfig
$ make
$ ./u-boot --default_fdt
...
U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
...
Model: sandbox
...
Hit any key to stop autoboot: 0
=> efidebug capsule esrt
...
========================================
ESRT: fw_resource_count=2
ESRT: fw_resource_count_max=2
ESRT: fw_resource_version=1
[entry 0]==============================
ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
...
[entry 1]==============================
ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
...
========================================
$ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
SIV: 336781303264349553179461347850802165102
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 10 (unknown)
content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
(not decipherable: unknown UUID version)
Version 10 does not look right.
$ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
SIV: 195894493536133784175416063449172723213
decode: variant: reserved (Microsoft GUID)
version: 4 (random data based)
content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
(no semantics: random data only)
A reserved Microsoft GUID variant does not look right.
With the master branch, the (hardcoded) GUIDs are ok:
$ ./u-boot --default_fdt
...
U-Boot 2024.07-rc4-00021-gfe2ce09a075 (Jun 19 2024 - 15:35:08 +0200)
...
Model: sandbox
...
=> efidebug capsule esrt
...
========================================
ESRT: fw_resource_count=2
ESRT: fw_resource_count_max=2
ESRT: fw_resource_version=1
[entry 0]==============================
ESRT: fw_class=09D7CF52-0720-4710-91D1-08469B7FE9C8
...
[entry 1]==============================
ESRT: fw_class=5A7021F5-FEF2-48B4-AABA-832E777418C0
...
========================================
$ uuid -d 09D7CF52-0720-4710-91D1-08469B7FE9C8
encode: STR: 09d7cf52-0720-4710-91d1-08469b7fe9c8
SIV: 13083600744351929150374221048734280136
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 4 (random data based)
content: 09:D7:CF:52:07:20:07:10:11:D1:08:46:9B:7F:E9:C8
(no semantics: random data only)
$ uuid -d 5A7021F5-FEF2-48B4-AABA-832E777418C0
encode: STR: 5a7021f5-fef2-48b4-aaba-832e777418c0
SIV: 120212745678117161641696128857923655872
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 4 (random data based)
content: 5A:70:21:F5:FE:F2:08:B4:2A:BA:83:2E:77:74:18:C0
(no semantics: random data only)
Also, this is what to expect for a v5 UUID [1]:
$ uuid -d a8f6ae40-d8a7-58f0-be05-a22f94eca9ec
encode: STR: a8f6ae40-d8a7-58f0-be05-a22f94eca9ec
SIV: 224591142595989943290477237735758014956
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 5 (name based, SHA-1)
content: A8:F6:AE:40:D8:A7:08:F0:3E:05:A2:2F:94:EC:A9:EC
(not decipherable: truncated SHA-1 message digest only)
Best regards,
Vincent.
[1] https://uuid.ramsey.dev/en/stable/rfc4122/version5.html
>
> The well-known salt GUID can be specific to the architecture (SoC
> vendor), or OEM. It is defined in the board defconfig so that vendors
> can easily bring their own.
>
> Specifically, the following fields are used to generate a GUID for a
> particular fw_image:
>
> * namespace salt
> * board compatible (usually the first entry in the dt root compatible
> array).
> * fw_image name (the string identifying the specific image, especially
> relevant for board that can update multiple images).
>
> == Usage ==
>
> Boards can integrate dynamic UUID support as follows:
>
> 1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if
> EFI_HAVE_CAPSULE_SUPPORT.
> 2. Skip setting the fw_images image_type_id property.
> 3. Generate a UUID and set CONFIG_EFI_CAPSULE_NAMESPACE_UUID in your
> defconfig.
>
> == Limitations ==
>
> * Changing GUIDs
>
> The primary limitation with this approach is that if any of the source
> fields change, so will the GUID for the board. It is therefore pretty
> important to ensure that GUID changes are caught during development.
>
> * Supporting multiple boards with a single image
>
> This now requires having an entry with the GUID for every board which
> might lead to larger UpdateCapsule images.
>
> == Tooling ==
>
> This series introduces a new tool: genguid. This can be used to generate
> the same GUIDs that the board would at runtime.
>
> This series follows a related discussion started by Ilias:
> https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72WkYLizrw@mail.gmail.com/
>
> ---
> Changes in v3:
> - Add manpage for genguid
> - Add dedicated CONFIG_TOOLS_GENGUID option
> - Minor code fixes addressing v2 feedback
> - Link to v2: https://lore.kernel.org/r/20240529-b4-dynamic-uuid-v2-0-c26f31057bbe@linaro.org
>
> Changes in v2:
> - Move namespace UUID to be defined in defconfig
> - Add tests and tooling
> - Only use the first board compatible to generate UUID.
> - Link to v1: https://lore.kernel.org/r/20240426-b4-dynamic-uuid-v1-0-e8154e00ec44@linaro.org
>
> ---
> Caleb Connolly (7):
> lib: uuid: add UUID v5 support
> efi: add a helper to generate dynamic UUIDs
> doc: uefi: document dynamic UUID generation
> sandbox: switch to dynamic UUIDs
> lib: uuid: supporting building as part of host tools
> tools: add genguid tool
> test: lib/uuid: add unit tests for dynamic UUIDs
>
> arch/Kconfig | 1 +
> board/sandbox/sandbox.c | 16 ---
> configs/sandbox_defconfig | 1 +
> configs/sandbox_flattree_defconfig | 1 +
> doc/develop/uefi/uefi.rst | 31 +++++
> doc/genguid.1 | 52 +++++++
> include/sandbox_efi_capsule.h | 6 +-
> include/uuid.h | 21 ++-
> lib/Kconfig | 8 ++
> lib/efi_loader/Kconfig | 23 +++
> lib/efi_loader/efi_capsule.c | 1 +
> lib/efi_loader/efi_firmware.c | 66 +++++++++
> lib/uuid.c | 81 +++++++++--
> test/lib/uuid.c | 88 ++++++++++++
> .../test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
> .../test_efi_capsule/test_capsule_firmware_raw.py | 8 +-
> .../test_capsule_firmware_signed_fit.py | 2 +-
> .../test_capsule_firmware_signed_raw.py | 4 +-
> test/py/tests/test_efi_capsule/version.dts | 6 +-
> tools/Kconfig | 7 +
> tools/Makefile | 3 +
> tools/binman/etype/efi_capsule.py | 2 +-
> tools/binman/ftest.py | 2 +-
> tools/genguid.c | 154 +++++++++++++++++++++
> 24 files changed, 538 insertions(+), 48 deletions(-)
> ---
> change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27
> base-commit: 2e682a4a406fc81ef32e05c28542cc8067f1e15f
>
> // Caleb (they/them)
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-19 14:02 ` Vincent Stehlé
@ 2024-06-19 19:15 ` Ilias Apalodimas
2024-06-21 9:12 ` Vincent Stehlé
0 siblings, 1 reply; 36+ messages in thread
From: Ilias Apalodimas @ 2024-06-19 19:15 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini, Heinrich Schuchardt, Ilias Apalodimas,
Simon Glass, Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi,
Richard Hughes, u-boot
Allô Vincent,
Thanks for testing!
On Wed, 19 Jun 2024 at 17:02, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>
> On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:
> > As more boards adopt support for the EFI CapsuleUpdate mechanism, there
> > is a growing issue of being able to target updates to them properly. The
> > current mechanism of hardcoding UUIDs for each board at compile time is
> > unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
> >
> > In this series, I propose that we adopt v5 GUIDs, these are generated
> > by using a well-known salt GUID as well as board specific information
> > (like the model/revision), these are hashed together and the result is
> > truncated to form a new UUID.
>
> Dear Caleb,
>
> Thank you for working on this proposal, this looks very useful.
> Indeed, we found out during SystemReady certifications that it is easy for
> unique ids to be inadvertently re-used, making them thus non-unique.
>
> I have a doubt regarding the format of the generated UUIDs, which end up in the
> ESRT, though.
>
> Here is a quick experiment on the sandbox booting with a DTB using the efidebug
> command.
>
> With the patch series, rebased on the master branch:
>
> $ make sandbox_defconfig
> $ make
> $ ./u-boot --default_fdt
> ...
> U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
> ...
> Model: sandbox
> ...
> Hit any key to stop autoboot: 0
> => efidebug capsule esrt
> ...
> ========================================
> ESRT: fw_resource_count=2
> ESRT: fw_resource_count_max=2
> ESRT: fw_resource_version=1
> [entry 0]==============================
> ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> ...
> [entry 1]==============================
> ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
> ...
> ========================================
>
> $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
> SIV: 336781303264349553179461347850802165102
> decode: variant: DCE 1.1, ISO/IEC 11578:1996
> version: 10 (unknown)
> content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
> (not decipherable: unknown UUID version)
>
> Version 10 does not look right.
So, this seems to be an endianess problem.
Looking at RFC4122 only the space ID needs to be in BE.
>
> $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
> encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
> SIV: 195894493536133784175416063449172723213
> decode: variant: reserved (Microsoft GUID)
> version: 4 (random data based)
> content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
> (no semantics: random data only)
>
> A reserved Microsoft GUID variant does not look right.
This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
in the variant as
1 1 0 Reserved, Microsoft Corporation backward
compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
The patch below should work for you (on top of Calebs')
diff --git a/include/uuid.h b/include/uuid.h
index b38b20d957ef..78ed5839d2d6 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -81,7 +81,7 @@ struct uuid {
#define UUID_VERSION_SHIFT 12
#define UUID_VERSION 0x4
-#define UUID_VARIANT_MASK 0xc0
+#define UUID_VARIANT_MASK 0xb0
#define UUID_VARIANT_SHIFT 7
#define UUID_VARIANT 0x1
diff --git a/lib/uuid.c b/lib/uuid.c
index 89911b06ccc0..73251eaa397e 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
struct uuid *uuid, ...)
memcpy(uuid, hash, sizeof(*uuid));
/* Configure variant/version bits */
- tmp = be32_to_cpu(uuid->time_hi_and_version);
+ tmp = uuid->time_hi_and_version;
tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
- uuid->time_hi_and_version = cpu_to_be32(tmp);
+ uuid->time_hi_and_version = tmp;
uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
Can you give it a shot?
What I am afraid of is breaking existing use cases using a different
variant mask....
If that's the case we might need to keep the buggy existing
UUID_VARIANT_MASK and use the new one only on v5 and newer code
Thanks
/Ilias
>
> With the master branch, the (hardcoded) GUIDs are ok:
>
> $ ./u-boot --default_fdt
> ...
> U-Boot 2024.07-rc4-00021-gfe2ce09a075 (Jun 19 2024 - 15:35:08 +0200)
> ...
> Model: sandbox
> ...
> => efidebug capsule esrt
> ...
> ========================================
> ESRT: fw_resource_count=2
> ESRT: fw_resource_count_max=2
> ESRT: fw_resource_version=1
> [entry 0]==============================
> ESRT: fw_class=09D7CF52-0720-4710-91D1-08469B7FE9C8
> ...
> [entry 1]==============================
> ESRT: fw_class=5A7021F5-FEF2-48B4-AABA-832E777418C0
> ...
> ========================================
>
> $ uuid -d 09D7CF52-0720-4710-91D1-08469B7FE9C8
> encode: STR: 09d7cf52-0720-4710-91d1-08469b7fe9c8
> SIV: 13083600744351929150374221048734280136
> decode: variant: DCE 1.1, ISO/IEC 11578:1996
> version: 4 (random data based)
> content: 09:D7:CF:52:07:20:07:10:11:D1:08:46:9B:7F:E9:C8
> (no semantics: random data only)
>
> $ uuid -d 5A7021F5-FEF2-48B4-AABA-832E777418C0
> encode: STR: 5a7021f5-fef2-48b4-aaba-832e777418c0
> SIV: 120212745678117161641696128857923655872
> decode: variant: DCE 1.1, ISO/IEC 11578:1996
> version: 4 (random data based)
> content: 5A:70:21:F5:FE:F2:08:B4:2A:BA:83:2E:77:74:18:C0
> (no semantics: random data only)
>
> Also, this is what to expect for a v5 UUID [1]:
>
> $ uuid -d a8f6ae40-d8a7-58f0-be05-a22f94eca9ec
> encode: STR: a8f6ae40-d8a7-58f0-be05-a22f94eca9ec
> SIV: 224591142595989943290477237735758014956
> decode: variant: DCE 1.1, ISO/IEC 11578:1996
> version: 5 (name based, SHA-1)
> content: A8:F6:AE:40:D8:A7:08:F0:3E:05:A2:2F:94:EC:A9:EC
> (not decipherable: truncated SHA-1 message digest only)
>
> Best regards,
> Vincent.
>
> [1] https://uuid.ramsey.dev/en/stable/rfc4122/version5.html
>
> >
> > The well-known salt GUID can be specific to the architecture (SoC
> > vendor), or OEM. It is defined in the board defconfig so that vendors
> > can easily bring their own.
> >
> > Specifically, the following fields are used to generate a GUID for a
> > particular fw_image:
> >
> > * namespace salt
> > * board compatible (usually the first entry in the dt root compatible
> > array).
> > * fw_image name (the string identifying the specific image, especially
> > relevant for board that can update multiple images).
> >
> > == Usage ==
> >
> > Boards can integrate dynamic UUID support as follows:
> >
> > 1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if
> > EFI_HAVE_CAPSULE_SUPPORT.
> > 2. Skip setting the fw_images image_type_id property.
> > 3. Generate a UUID and set CONFIG_EFI_CAPSULE_NAMESPACE_UUID in your
> > defconfig.
> >
> > == Limitations ==
> >
> > * Changing GUIDs
> >
> > The primary limitation with this approach is that if any of the source
> > fields change, so will the GUID for the board. It is therefore pretty
> > important to ensure that GUID changes are caught during development.
> >
> > * Supporting multiple boards with a single image
> >
> > This now requires having an entry with the GUID for every board which
> > might lead to larger UpdateCapsule images.
> >
> > == Tooling ==
> >
> > This series introduces a new tool: genguid. This can be used to generate
> > the same GUIDs that the board would at runtime.
> >
> > This series follows a related discussion started by Ilias:
> > https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72WkYLizrw@mail.gmail.com/
> >
> > ---
> > Changes in v3:
> > - Add manpage for genguid
> > - Add dedicated CONFIG_TOOLS_GENGUID option
> > - Minor code fixes addressing v2 feedback
> > - Link to v2: https://lore.kernel.org/r/20240529-b4-dynamic-uuid-v2-0-c26f31057bbe@linaro.org
> >
> > Changes in v2:
> > - Move namespace UUID to be defined in defconfig
> > - Add tests and tooling
> > - Only use the first board compatible to generate UUID.
> > - Link to v1: https://lore.kernel.org/r/20240426-b4-dynamic-uuid-v1-0-e8154e00ec44@linaro.org
> >
> > ---
> > Caleb Connolly (7):
> > lib: uuid: add UUID v5 support
> > efi: add a helper to generate dynamic UUIDs
> > doc: uefi: document dynamic UUID generation
> > sandbox: switch to dynamic UUIDs
> > lib: uuid: supporting building as part of host tools
> > tools: add genguid tool
> > test: lib/uuid: add unit tests for dynamic UUIDs
> >
> > arch/Kconfig | 1 +
> > board/sandbox/sandbox.c | 16 ---
> > configs/sandbox_defconfig | 1 +
> > configs/sandbox_flattree_defconfig | 1 +
> > doc/develop/uefi/uefi.rst | 31 +++++
> > doc/genguid.1 | 52 +++++++
> > include/sandbox_efi_capsule.h | 6 +-
> > include/uuid.h | 21 ++-
> > lib/Kconfig | 8 ++
> > lib/efi_loader/Kconfig | 23 +++
> > lib/efi_loader/efi_capsule.c | 1 +
> > lib/efi_loader/efi_firmware.c | 66 +++++++++
> > lib/uuid.c | 81 +++++++++--
> > test/lib/uuid.c | 88 ++++++++++++
> > .../test_efi_capsule/test_capsule_firmware_fit.py | 2 +-
> > .../test_efi_capsule/test_capsule_firmware_raw.py | 8 +-
> > .../test_capsule_firmware_signed_fit.py | 2 +-
> > .../test_capsule_firmware_signed_raw.py | 4 +-
> > test/py/tests/test_efi_capsule/version.dts | 6 +-
> > tools/Kconfig | 7 +
> > tools/Makefile | 3 +
> > tools/binman/etype/efi_capsule.py | 2 +-
> > tools/binman/ftest.py | 2 +-
> > tools/genguid.c | 154 +++++++++++++++++++++
> > 24 files changed, 538 insertions(+), 48 deletions(-)
> > ---
> > change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27
> > base-commit: 2e682a4a406fc81ef32e05c28542cc8067f1e15f
> >
> > // Caleb (they/them)
> >
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3 6/7] tools: add genguid tool
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
2024-06-05 2:13 ` Simon Glass
2024-06-05 6:36 ` Heinrich Schuchardt
@ 2024-06-20 14:58 ` Vincent Stehlé
2 siblings, 0 replies; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-20 14:58 UTC (permalink / raw)
To: Caleb Connolly
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On Fri, May 31, 2024 at 03:50:40PM +0200, Caleb Connolly wrote:
> Add a tool that can generate GUIDs that match those generated internally
> by U-Boot for capsule update fw_images.
Hi Caleb,
Thanks for working on this.
I think there is a leftover "dtb" option; see below.
Best regards,
Vincent.
>
> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> with the board model, compatible, and fw_image name.
>
> This tool accepts the same inputs and will produce the same GUID as
> U-Boot would at runtime.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> doc/genguid.1 | 52 +++++++++++++++++++
> tools/Kconfig | 7 +++
> tools/Makefile | 3 ++
> tools/genguid.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 216 insertions(+)
(...)
> diff --git a/tools/genguid.c b/tools/genguid.c
> new file mode 100644
> index 000000000000..e71bc1d48f95
> --- /dev/null
> +++ b/tools/genguid.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 Linaro Ltd.
> + * Author: Caleb Connolly
> + */
> +
> +#include <getopt.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/types.h>
> +
> +#include <uuid.h>
> +
> +static struct option options[] = {
> + {"dtb", required_argument, NULL, 'd'},
I think this is unused.
> + {"compat", required_argument, NULL, 'c'},
> + {"help", no_argument, NULL, 'h'},
> + {"verbose", no_argument, NULL, 'v'},
> + {"json", no_argument, NULL, 'j'},
> + {NULL, 0, NULL, 0},
> +};
> +
> +static void usage(const char *progname)
> +{
> + fprintf(stderr, "Usage: %s GUID [-v] -c COMPAT NAME...\n", progname);
> + fprintf(stderr,
> + "Generate a v5 GUID for one of more U-Boot fw_images the same way U-Boot does at runtime.\n");
> + fprintf(stderr,
> + "\nOptions:\n"
> + " GUID namespace/salt GUID in 8-4-4-4-12 format\n"
> + " -h, --help display this help and exit\n"
> + " -c, --compat=COMPAT first compatible property in the board devicetree\n"
> + " -v, --verbose print debug messages\n"
> + " -j, --json output in JSON format\n"
> + " NAME... one or more names of fw_images to generate GUIDs for\n"
> + );
> + fprintf(stderr, "\nExample:\n");
> + fprintf(stderr, " %s 2a5aa852-b856-4d97-baa9-5c5f4421551f \\\n"
> + "\t-c \"qcom,qrb4210-rb2\" \\\n"
> + "\tQUALCOMM-UBOOT\n", progname);
> +}
> +
> +static size_t u16_strsize(const uint16_t *in)
> +{
> + size_t i = 0, count = UINT16_MAX;
> +
> + while (count-- && in[i])
> + i++;
> +
> + return (i + 1) * sizeof(uint16_t);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + struct uuid namespace;
> + char *namespace_str;
> + char uuid_str[37];
> + char **image_uuids;
> + char *compatible = NULL;
> + uint16_t **images_u16;
> + char **images;
> + int c, n_images;
> + bool debug = false, json = false;
> +
> + if (argc < 2) {
> + usage(argv[0]);
> + return 1;
> + }
> +
> + namespace_str = argv[1];
> +
> + /* The first arg is the GUID so skip it */
> + while ((c = getopt_long(argc, argv, "c:hvj", options, NULL)) != -1) {
> + switch (c) {
> + case 'c':
> + compatible = strdup(optarg);
> + break;
> + case 'h':
> + usage(argv[0]);
> + return 0;
> + case 'v':
> + debug = true;
> + break;
> + case 'j':
> + json = true;
> + break;
> + default:
> + usage(argv[0]);
> + return 1;
> + }
> + }
> +
> + if (!compatible) {
> + fprintf(stderr, "ERROR: Please specify the compatible property.\n\n");
> + usage(argv[0]);
> + return 1;
> + }
> +
> + if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_GUID)) {
> + fprintf(stderr, "ERROR: Check that your UUID is formatted correctly.\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* This is probably not the best way to convert a string to a "u16" string */
> + n_images = argc - optind - 1;
> + images = argv + optind + 1;
> + images_u16 = calloc(n_images, sizeof(char *));
> + for (int i = 0; i < n_images; i++) {
> + images_u16[i] = calloc(1, strlen(images[i]) * 2 + 2);
> + for (int j = 0; j < strlen(images[i]); j++)
> + images_u16[i][j] = (uint16_t)images[i][j];
> + }
> +
> + if (debug) {
> + fprintf(stderr, "GUID: ");
> + uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_GUID);
> + fprintf(stderr, "%s\n", uuid_str);
> + fprintf(stderr, "Compatible: \"%s\"\n", compatible);
> + fprintf(stderr, "Images: ");
> + for (int i = 0; i < n_images; i++)
> + fprintf(stderr, "\"%s\"%s", argv[optind + i + 1],
> + i == n_images - 1 ? "\n" : ", ");
> + }
> +
> + image_uuids = calloc(n_images, sizeof(char *));
> + for (int i = 0; i < n_images; i++) {
> + struct uuid image_type_id;
> +
> + gen_uuid_v5(&namespace, &image_type_id,
> + compatible, strlen(compatible),
> + images_u16[i], u16_strsize(images_u16[i]) - sizeof(uint16_t),
> + NULL);
> +
> + uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_GUID);
> + image_uuids[i] = strdup(uuid_str);
> + }
> +
> + if (json) {
> + printf("[\n");
> + for (int i = 0; i < n_images; i++)
> + printf("\t{\"name\": \"%s\", \"uuid\": \"%s\"}%s\n", images[i], image_uuids[i],
> + i == n_images - 1 ? "" : ",");
> + printf("]\n");
> + } else {
> + for (int i = 0; i < n_images; i++)
> + printf("%-24s| %s\n", images[i], image_uuids[i]);
> + }
> +
> + return 0;
> +}
> +
>
> --
> 2.45.0
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-19 19:15 ` Ilias Apalodimas
@ 2024-06-21 9:12 ` Vincent Stehlé
2024-06-21 11:00 ` Heinrich Schuchardt
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-21 9:12 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Caleb Connolly, Tom Rini, Heinrich Schuchardt, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On Wed, Jun 19, 2024 at 10:15:32PM +0300, Ilias Apalodimas wrote:
> Allô Vincent,
Hi Ilias :)
>
> Thanks for testing!
>
> On Wed, 19 Jun 2024 at 17:02, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> >
> > On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:
> > > As more boards adopt support for the EFI CapsuleUpdate mechanism, there
> > > is a growing issue of being able to target updates to them properly. The
> > > current mechanism of hardcoding UUIDs for each board at compile time is
> > > unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
> > >
> > > In this series, I propose that we adopt v5 GUIDs, these are generated
> > > by using a well-known salt GUID as well as board specific information
> > > (like the model/revision), these are hashed together and the result is
> > > truncated to form a new UUID.
> >
> > Dear Caleb,
> >
> > Thank you for working on this proposal, this looks very useful.
> > Indeed, we found out during SystemReady certifications that it is easy for
> > unique ids to be inadvertently re-used, making them thus non-unique.
> >
> > I have a doubt regarding the format of the generated UUIDs, which end up in the
> > ESRT, though.
> >
> > Here is a quick experiment on the sandbox booting with a DTB using the efidebug
> > command.
> >
> > With the patch series, rebased on the master branch:
> >
> > $ make sandbox_defconfig
> > $ make
> > $ ./u-boot --default_fdt
> > ...
> > U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
> > ...
> > Model: sandbox
> > ...
> > Hit any key to stop autoboot: 0
> > => efidebug capsule esrt
> > ...
> > ========================================
> > ESRT: fw_resource_count=2
> > ESRT: fw_resource_count_max=2
> > ESRT: fw_resource_version=1
> > [entry 0]==============================
> > ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> > ...
> > [entry 1]==============================
> > ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
> > ...
> > ========================================
> >
> > $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> > encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
> > SIV: 336781303264349553179461347850802165102
> > decode: variant: DCE 1.1, ISO/IEC 11578:1996
> > version: 10 (unknown)
> > content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
> > (not decipherable: unknown UUID version)
> >
> > Version 10 does not look right.
>
> So, this seems to be an endianess problem.
> Looking at RFC4122 only the space ID needs to be in BE.
>
> >
> > $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
> > encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
> > SIV: 195894493536133784175416063449172723213
> > decode: variant: reserved (Microsoft GUID)
> > version: 4 (random data based)
> > content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
> > (no semantics: random data only)
> >
> > A reserved Microsoft GUID variant does not look right.
>
> This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
> in the variant as
> 1 1 0 Reserved, Microsoft Corporation backward
> compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
I think the variant mask 0xc0 is correct:
- The variant field is in the top three bits of the "clock seq high and
reserved" byte, but...
- The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
With the mask 0xc0 we can clear the top two bits as we set the top most bit just
after anyway.
...the mask needs to be used correctly, though; see below.
>
> The patch below should work for you (on top of Calebs')
>
> diff --git a/include/uuid.h b/include/uuid.h
> index b38b20d957ef..78ed5839d2d6 100644
> --- a/include/uuid.h
> +++ b/include/uuid.h
> @@ -81,7 +81,7 @@ struct uuid {
> #define UUID_VERSION_SHIFT 12
> #define UUID_VERSION 0x4
>
> -#define UUID_VARIANT_MASK 0xc0
> +#define UUID_VARIANT_MASK 0xb0
> #define UUID_VARIANT_SHIFT 7
> #define UUID_VARIANT 0x1
>
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 89911b06ccc0..73251eaa397e 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
> struct uuid *uuid, ...)
> memcpy(uuid, hash, sizeof(*uuid));
>
> /* Configure variant/version bits */
> - tmp = be32_to_cpu(uuid->time_hi_and_version);
> + tmp = uuid->time_hi_and_version;
Not caring for the endianness at all does not look right.
Indeed, while this does work on my side in little-endian, this does not work on
on big-endian simulators.
Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
use the be16 functions.
> tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> - uuid->time_hi_and_version = cpu_to_be32(tmp);
> + uuid->time_hi_and_version = tmp;
>
> uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
We need to mask with ~UUID_VARIANT_MASK, I think.
> uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
>
> Can you give it a shot?
This does indeed work on my little-endian machines, but not on big-endian
simulators.
For testing on big-endian, I suggest using only genguid as the sandbox will not
help there:
$ make sandbox_defconfig
$ make tools-only
$ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
-c "qcom,qrb4210-rb2" \
QUALCOMM-UBOOT
...and feed the resulting UUID to `uuid -d'.
(The genguid command is the online help example.)
> What I am afraid of is breaking existing use cases using a different
> variant mask....
> If that's the case we might need to keep the buggy existing
> UUID_VARIANT_MASK and use the new one only on v5 and newer code
I tried to debug further and I suspect that:
- Operations on 8b clock_seq_hi_and_reserved might need further casts.
- My understanding is that we are generating the v5 UUID as big-endian in
memory; if this is indeed the case, genguid should not print it with the GUID
byte order.
For the moment I am unable to make the code work in all the following cases:
- genguid on little-endian
- genguid on big-endian
- sandbox ESRT on little-endian
I will let you and Caleb know if I make any progress.
Best regards,
Vincent.
>
> Thanks
> /Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 9:12 ` Vincent Stehlé
@ 2024-06-21 11:00 ` Heinrich Schuchardt
2024-06-21 17:06 ` Vincent Stehlé
2024-06-21 11:01 ` Ilias Apalodimas
2024-06-27 9:55 ` [PATCH] Proposed changes to dynamic UUIDs v3 Vincent Stehlé
2 siblings, 1 reply; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-21 11:00 UTC (permalink / raw)
To: Vincent Stehlé
Cc: Ilias Apalodimas, Caleb Connolly, Tom Rini, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On 21.06.24 11:12, Vincent Stehlé wrote:
> On Wed, Jun 19, 2024 at 10:15:32PM +0300, Ilias Apalodimas wrote:
>> Allô Vincent,
>
> Hi Ilias :)
>
>>
>> Thanks for testing!
>>
>> On Wed, 19 Jun 2024 at 17:02, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>>>
>>> On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:
>>>> As more boards adopt support for the EFI CapsuleUpdate mechanism, there
>>>> is a growing issue of being able to target updates to them properly. The
>>>> current mechanism of hardcoding UUIDs for each board at compile time is
>>>> unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
>>>>
>>>> In this series, I propose that we adopt v5 GUIDs, these are generated
>>>> by using a well-known salt GUID as well as board specific information
>>>> (like the model/revision), these are hashed together and the result is
>>>> truncated to form a new UUID.
>>>
>>> Dear Caleb,
>>>
>>> Thank you for working on this proposal, this looks very useful.
>>> Indeed, we found out during SystemReady certifications that it is easy for
>>> unique ids to be inadvertently re-used, making them thus non-unique.
>>>
>>> I have a doubt regarding the format of the generated UUIDs, which end up in the
>>> ESRT, though.
>>>
>>> Here is a quick experiment on the sandbox booting with a DTB using the efidebug
>>> command.
>>>
>>> With the patch series, rebased on the master branch:
>>>
>>> $ make sandbox_defconfig
>>> $ make
>>> $ ./u-boot --default_fdt
>>> ...
>>> U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
>>> ...
>>> Model: sandbox
>>> ...
>>> Hit any key to stop autoboot: 0
>>> => efidebug capsule esrt
>>> ...
>>> ========================================
>>> ESRT: fw_resource_count=2
>>> ESRT: fw_resource_count_max=2
>>> ESRT: fw_resource_version=1
>>> [entry 0]==============================
>>> ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
>>> ...
>>> [entry 1]==============================
>>> ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
>>> ...
>>> ========================================
>>>
>>> $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
>>> encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
>>> SIV: 336781303264349553179461347850802165102
>>> decode: variant: DCE 1.1, ISO/IEC 11578:1996
>>> version: 10 (unknown)
>>> content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
>>> (not decipherable: unknown UUID version)
>>>
>>> Version 10 does not look right.
>>
>> So, this seems to be an endianess problem.
>> Looking at RFC4122 only the space ID needs to be in BE.
>>
>>>
>>> $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
>>> encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
>>> SIV: 195894493536133784175416063449172723213
>>> decode: variant: reserved (Microsoft GUID)
>>> version: 4 (random data based)
>>> content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
>>> (no semantics: random data only)
>>>
>>> A reserved Microsoft GUID variant does not look right.
>>
>> This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
>> in the variant as
>> 1 1 0 Reserved, Microsoft Corporation backward
>> compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
>
> I think the variant mask 0xc0 is correct:
>
> - The variant field is in the top three bits of the "clock seq high and
> reserved" byte, but...
> - The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
>
> With the mask 0xc0 we can clear the top two bits as we set the top most bit just
> after anyway.
>
> ...the mask needs to be used correctly, though; see below.
>
>>
>> The patch below should work for you (on top of Calebs')
>>
>> diff --git a/include/uuid.h b/include/uuid.h
>> index b38b20d957ef..78ed5839d2d6 100644
>> --- a/include/uuid.h
>> +++ b/include/uuid.h
>> @@ -81,7 +81,7 @@ struct uuid {
>> #define UUID_VERSION_SHIFT 12
>> #define UUID_VERSION 0x4
>>
>> -#define UUID_VARIANT_MASK 0xc0
>> +#define UUID_VARIANT_MASK 0xb0
>> #define UUID_VARIANT_SHIFT 7
>> #define UUID_VARIANT 0x1
>>
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index 89911b06ccc0..73251eaa397e 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
>> struct uuid *uuid, ...)
>> memcpy(uuid, hash, sizeof(*uuid));
>>
>> /* Configure variant/version bits */
>> - tmp = be32_to_cpu(uuid->time_hi_and_version);
>> + tmp = uuid->time_hi_and_version;
>
> Not caring for the endianness at all does not look right.
> Indeed, while this does work on my side in little-endian, this does not work on
> on big-endian simulators.
> Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
> use the be16 functions.
>
>> tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
>> - uuid->time_hi_and_version = cpu_to_be32(tmp);
>> + uuid->time_hi_and_version = tmp;
>>
>> uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
>
> We need to mask with ~UUID_VARIANT_MASK, I think.
>
>> uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
>>
>> Can you give it a shot?
>
> This does indeed work on my little-endian machines, but not on big-endian
> simulators.
> For testing on big-endian, I suggest using only genguid as the sandbox will not
> help there:
>
> $ make sandbox_defconfig
> $ make tools-only
> $ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
> -c "qcom,qrb4210-rb2" \
> QUALCOMM-UBOOT
>
> ...and feed the resulting UUID to `uuid -d'.
> (The genguid command is the online help example.)
>
>> What I am afraid of is breaking existing use cases using a different
>> variant mask....
>> If that's the case we might need to keep the buggy existing
>> UUID_VARIANT_MASK and use the new one only on v5 and newer code
>
> I tried to debug further and I suspect that:
>
> - Operations on 8b clock_seq_hi_and_reserved might need further casts.
>
> - My understanding is that we are generating the v5 UUID as big-endian in
> memory; if this is indeed the case, genguid should not print it with the GUID
> byte order.
The current specification is in RFC 9562, 4.1, "Variant field"
"The variant field consists of a variable number of the most significant
bits of octet 8 of the UUID.
...
Specifically for UUIDs in this document, bits 64 and 65 of the UUID
(bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
of Table 1."
This reference to byte 8 does not depend on endianness.
U-Boot's include/uuid.h has:
/* This is structure is in big-endian */
struct uuid {
The field time_hi_and_version needs to be stored in big-endian fashion.
tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
typical in the EFI context but not understood by 'uuid -d'. Maybe we
should add a parameter for the output format.
Best regards
Heinrich
>
> For the moment I am unable to make the code work in all the following cases:
>
> - genguid on little-endian
> - genguid on big-endian
> - sandbox ESRT on little-endian
>
> I will let you and Caleb know if I make any progress.
>
> Best regards,
> Vincent.
>
>>
>> Thanks
>> /Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 9:12 ` Vincent Stehlé
2024-06-21 11:00 ` Heinrich Schuchardt
@ 2024-06-21 11:01 ` Ilias Apalodimas
2024-06-21 11:25 ` Ilias Apalodimas
2024-06-27 9:55 ` [PATCH] Proposed changes to dynamic UUIDs v3 Vincent Stehlé
2 siblings, 1 reply; 36+ messages in thread
From: Ilias Apalodimas @ 2024-06-21 11:01 UTC (permalink / raw)
To: Ilias Apalodimas, Caleb Connolly, Tom Rini, Heinrich Schuchardt,
Simon Glass, Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi,
Richard Hughes, u-boot
Hi Vincent,
[...]
> > > $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
> > > encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
> > > SIV: 195894493536133784175416063449172723213
> > > decode: variant: reserved (Microsoft GUID)
> > > version: 4 (random data based)
> > > content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
> > > (no semantics: random data only)
> > >
> > > A reserved Microsoft GUID variant does not look right.
> >
> > This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
> > in the variant as
> > 1 1 0 Reserved, Microsoft Corporation backward
> > compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
>
> I think the variant mask 0xc0 is correct:
>
> - The variant field is in the top three bits of the "clock seq high and
> reserved" byte, but...
> - The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
>
> With the mask 0xc0 we can clear the top two bits as we set the top most bit just
> after anyway.
>
> ...the mask needs to be used correctly, though; see below.
Ah yes, the current code is using it in clrsetbits_8, which inverts it
internally, so it's indeed correct.
>
> >
> > The patch below should work for you (on top of Calebs')
> >
> > diff --git a/include/uuid.h b/include/uuid.h
> > index b38b20d957ef..78ed5839d2d6 100644
> > --- a/include/uuid.h
> > +++ b/include/uuid.h
> > @@ -81,7 +81,7 @@ struct uuid {
> > #define UUID_VERSION_SHIFT 12
> > #define UUID_VERSION 0x4
> >
> > -#define UUID_VARIANT_MASK 0xc0
> > +#define UUID_VARIANT_MASK 0xb0
> > #define UUID_VARIANT_SHIFT 7
> > #define UUID_VARIANT 0x1
> >
> > diff --git a/lib/uuid.c b/lib/uuid.c
> > index 89911b06ccc0..73251eaa397e 100644
> > --- a/lib/uuid.c
> > +++ b/lib/uuid.c
> > @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
> > struct uuid *uuid, ...)
> > memcpy(uuid, hash, sizeof(*uuid));
> >
> > /* Configure variant/version bits */
> > - tmp = be32_to_cpu(uuid->time_hi_and_version);
> > + tmp = uuid->time_hi_and_version;
>
> Not caring for the endianness at all does not look right.
> Indeed, while this does work on my side in little-endian, this does not work on
> on big-endian simulators.
Yes, we need the conversions
> Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
> use the be16 functions.
>
Yep I already pointed that out earlier.
> > tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> > - uuid->time_hi_and_version = cpu_to_be32(tmp);
> > + uuid->time_hi_and_version = tmp;
> >
> > uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
>
> We need to mask with ~UUID_VARIANT_MASK, I think.
>
> > uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> >
> > Can you give it a shot?
>
> This does indeed work on my little-endian machines, but not on big-endian
> simulators.
> For testing on big-endian, I suggest using only genguid as the sandbox will not
> help there:
>
> $ make sandbox_defconfig
> $ make tools-only
> $ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
> -c "qcom,qrb4210-rb2" \
> QUALCOMM-UBOOT
>
> ...and feed the resulting UUID to `uuid -d'.
> (The genguid command is the online help example.)
>
> > What I am afraid of is breaking existing use cases using a different
> > variant mask....
> > If that's the case we might need to keep the buggy existing
> > UUID_VARIANT_MASK and use the new one only on v5 and newer code
>
> I tried to debug further and I suspect that:
>
> - Operations on 8b clock_seq_hi_and_reserved might need further casts.
>
> - My understanding is that we are generating the v5 UUID as big-endian in
> memory; if this is indeed the case, genguid should not print it with the GUID
> byte order.
RFC4122 says that
"put name space ID in network byte order so it hashes the same no
matter what endian machine we're on "
the EFI spec says
"It should also be noted that TimeLow, TimeMid, TimeHighAndVersion
fields in the EFI are encoded as little endian. The following table
defines the format of an EFI GUID (128 bits)."
Which is lovely....
I'll send a patch with the changes
Regards
/Ilias
>
> For the moment I am unable to make the code work in all the following cases:
>
> - genguid on little-endian
> - genguid on big-endian
> - sandbox ESRT on little-endian
>
> I will let you and Caleb know if I make any progress.
>
> Best regards,
> Vincent.
>
> >
> > Thanks
> > /Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 11:01 ` Ilias Apalodimas
@ 2024-06-21 11:25 ` Ilias Apalodimas
2024-06-21 13:16 ` Heinrich Schuchardt
0 siblings, 1 reply; 36+ messages in thread
From: Ilias Apalodimas @ 2024-06-21 11:25 UTC (permalink / raw)
To: Ilias Apalodimas, Caleb Connolly, Tom Rini, Heinrich Schuchardt,
Simon Glass, Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi,
Richard Hughes, u-boot
On Fri, 21 Jun 2024 at 14:01, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Vincent,
>
> [...]
>
> > > > $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
> > > > encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
> > > > SIV: 195894493536133784175416063449172723213
> > > > decode: variant: reserved (Microsoft GUID)
> > > > version: 4 (random data based)
> > > > content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
> > > > (no semantics: random data only)
> > > >
> > > > A reserved Microsoft GUID variant does not look right.
> > >
> > > This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
> > > in the variant as
> > > 1 1 0 Reserved, Microsoft Corporation backward
> > > compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
> >
> > I think the variant mask 0xc0 is correct:
> >
> > - The variant field is in the top three bits of the "clock seq high and
> > reserved" byte, but...
> > - The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
> >
> > With the mask 0xc0 we can clear the top two bits as we set the top most bit just
> > after anyway.
> >
> > ...the mask needs to be used correctly, though; see below.
>
> Ah yes, the current code is using it in clrsetbits_8, which inverts it
> internally, so it's indeed correct.
>
> >
> > >
> > > The patch below should work for you (on top of Calebs')
> > >
> > > diff --git a/include/uuid.h b/include/uuid.h
> > > index b38b20d957ef..78ed5839d2d6 100644
> > > --- a/include/uuid.h
> > > +++ b/include/uuid.h
> > > @@ -81,7 +81,7 @@ struct uuid {
> > > #define UUID_VERSION_SHIFT 12
> > > #define UUID_VERSION 0x4
> > >
> > > -#define UUID_VARIANT_MASK 0xc0
> > > +#define UUID_VARIANT_MASK 0xb0
> > > #define UUID_VARIANT_SHIFT 7
> > > #define UUID_VARIANT 0x1
> > >
> > > diff --git a/lib/uuid.c b/lib/uuid.c
> > > index 89911b06ccc0..73251eaa397e 100644
> > > --- a/lib/uuid.c
> > > +++ b/lib/uuid.c
> > > @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
> > > struct uuid *uuid, ...)
> > > memcpy(uuid, hash, sizeof(*uuid));
> > >
> > > /* Configure variant/version bits */
> > > - tmp = be32_to_cpu(uuid->time_hi_and_version);
> > > + tmp = uuid->time_hi_and_version;
> >
> > Not caring for the endianness at all does not look right.
> > Indeed, while this does work on my side in little-endian, this does not work on
> > on big-endian simulators.
>
> Yes, we need the conversions
>
> > Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
> > use the be16 functions.
> >
>
> Yep I already pointed that out earlier.
>
> > > tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> > > - uuid->time_hi_and_version = cpu_to_be32(tmp);
> > > + uuid->time_hi_and_version = tmp;
> > >
> > > uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
> >
> > We need to mask with ~UUID_VARIANT_MASK, I think.
> >
> > > uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> > >
> > > Can you give it a shot?
> >
> > This does indeed work on my little-endian machines, but not on big-endian
> > simulators.
> > For testing on big-endian, I suggest using only genguid as the sandbox will not
> > help there:
> >
> > $ make sandbox_defconfig
> > $ make tools-only
> > $ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
> > -c "qcom,qrb4210-rb2" \
> > QUALCOMM-UBOOT
> >
> > ...and feed the resulting UUID to `uuid -d'.
> > (The genguid command is the online help example.)
> >
> > > What I am afraid of is breaking existing use cases using a different
> > > variant mask....
> > > If that's the case we might need to keep the buggy existing
> > > UUID_VARIANT_MASK and use the new one only on v5 and newer code
> >
> > I tried to debug further and I suspect that:
> >
> > - Operations on 8b clock_seq_hi_and_reserved might need further casts.
> >
> > - My understanding is that we are generating the v5 UUID as big-endian in
> > memory; if this is indeed the case, genguid should not print it with the GUID
> > byte order.
>
> RFC4122 says that
> "put name space ID in network byte order so it hashes the same no
> matter what endian machine we're on "
> the EFI spec says
> "It should also be noted that TimeLow, TimeMid, TimeHighAndVersion
> fields in the EFI are encoded as little endian. The following table
> defines the format of an EFI GUID (128 bits)."
>
> Which is lovely....
>
But this brings up something that Heinrich also mentioned. Since the
efi spec and RFC4122 interpret the endianess differently, how do you
expect uuid -d to work?
Thanks
/Ilias
> I'll send a patch with the changes
>
> Regards
> /Ilias
> >
> > For the moment I am unable to make the code work in all the following cases:
> >
> > - genguid on little-endian
> > - genguid on big-endian
> > - sandbox ESRT on little-endian
> >
> > I will let you and Caleb know if I make any progress.
> >
> > Best regards,
> > Vincent.
> >
> > >
> > > Thanks
> > > /Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 11:25 ` Ilias Apalodimas
@ 2024-06-21 13:16 ` Heinrich Schuchardt
0 siblings, 0 replies; 36+ messages in thread
From: Heinrich Schuchardt @ 2024-06-21 13:16 UTC (permalink / raw)
To: Ilias Apalodimas, Caleb Connolly, Tom Rini, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On 21.06.24 13:25, Ilias Apalodimas wrote:
> On Fri, 21 Jun 2024 at 14:01, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
>>
>> Hi Vincent,
>>
>> [...]
>>
>>>>> $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
>>>>> encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
>>>>> SIV: 195894493536133784175416063449172723213
>>>>> decode: variant: reserved (Microsoft GUID)
>>>>> version: 4 (random data based)
>>>>> content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
>>>>> (no semantics: random data only)
>>>>>
>>>>> A reserved Microsoft GUID variant does not look right.
>>>>
>>>> This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
>>>> in the variant as
>>>> 1 1 0 Reserved, Microsoft Corporation backward
>>>> compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
>>>
>>> I think the variant mask 0xc0 is correct:
>>>
>>> - The variant field is in the top three bits of the "clock seq high and
>>> reserved" byte, but...
>>> - The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
>>>
>>> With the mask 0xc0 we can clear the top two bits as we set the top most bit just
>>> after anyway.
>>>
>>> ...the mask needs to be used correctly, though; see below.
>>
>> Ah yes, the current code is using it in clrsetbits_8, which inverts it
>> internally, so it's indeed correct.
>>
>>>
>>>>
>>>> The patch below should work for you (on top of Calebs')
>>>>
>>>> diff --git a/include/uuid.h b/include/uuid.h
>>>> index b38b20d957ef..78ed5839d2d6 100644
>>>> --- a/include/uuid.h
>>>> +++ b/include/uuid.h
>>>> @@ -81,7 +81,7 @@ struct uuid {
>>>> #define UUID_VERSION_SHIFT 12
>>>> #define UUID_VERSION 0x4
>>>>
>>>> -#define UUID_VARIANT_MASK 0xc0
>>>> +#define UUID_VARIANT_MASK 0xb0
>>>> #define UUID_VARIANT_SHIFT 7
>>>> #define UUID_VARIANT 0x1
>>>>
>>>> diff --git a/lib/uuid.c b/lib/uuid.c
>>>> index 89911b06ccc0..73251eaa397e 100644
>>>> --- a/lib/uuid.c
>>>> +++ b/lib/uuid.c
>>>> @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
>>>> struct uuid *uuid, ...)
>>>> memcpy(uuid, hash, sizeof(*uuid));
>>>>
>>>> /* Configure variant/version bits */
>>>> - tmp = be32_to_cpu(uuid->time_hi_and_version);
>>>> + tmp = uuid->time_hi_and_version;
>>>
>>> Not caring for the endianness at all does not look right.
>>> Indeed, while this does work on my side in little-endian, this does not work on
>>> on big-endian simulators.
>>
>> Yes, we need the conversions
>>
>>> Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
>>> use the be16 functions.
>>>
>>
>> Yep I already pointed that out earlier.
>>
>>>> tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
>>>> - uuid->time_hi_and_version = cpu_to_be32(tmp);
>>>> + uuid->time_hi_and_version = tmp;
>>>>
>>>> uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
>>>
>>> We need to mask with ~UUID_VARIANT_MASK, I think.
>>>
>>>> uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
>>>>
>>>> Can you give it a shot?
>>>
>>> This does indeed work on my little-endian machines, but not on big-endian
>>> simulators.
>>> For testing on big-endian, I suggest using only genguid as the sandbox will not
>>> help there:
>>>
>>> $ make sandbox_defconfig
>>> $ make tools-only
>>> $ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
>>> -c "qcom,qrb4210-rb2" \
>>> QUALCOMM-UBOOT
>>>
>>> ...and feed the resulting UUID to `uuid -d'.
>>> (The genguid command is the online help example.)
>>>
>>>> What I am afraid of is breaking existing use cases using a different
>>>> variant mask....
>>>> If that's the case we might need to keep the buggy existing
>>>> UUID_VARIANT_MASK and use the new one only on v5 and newer code
>>>
>>> I tried to debug further and I suspect that:
>>>
>>> - Operations on 8b clock_seq_hi_and_reserved might need further casts.
>>>
>>> - My understanding is that we are generating the v5 UUID as big-endian in
>>> memory; if this is indeed the case, genguid should not print it with the GUID
>>> byte order.
>>
>> RFC4122 says that
>> "put name space ID in network byte order so it hashes the same no
>> matter what endian machine we're on "
>> the EFI spec says
>> "It should also be noted that TimeLow, TimeMid, TimeHighAndVersion
>> fields in the EFI are encoded as little endian. The following table
>> defines the format of an EFI GUID (128 bits)."
>>
>> Which is lovely....
>>
>
> But this brings up something that Heinrich also mentioned. Since the
> efi spec and RFC4122 interpret the endianess differently, how do you
> expect uuid -d to work?
The binary format does depend on endianness:
$ echo -n -e
"\\xF8\\x1D\\x4F\\xAE\\x7D\\xEC\\x51\\xD0\\xA7\\x65\\x00\\xA0\\xC9\\x1E\\x6B\\xF6"
\
> | uuid -d -F BIN -
encode: STR: f81d4fae-7dec-51d0-a765-00a0c91e6bf6
SIV: 329800735698586931527096882168799849462
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 5 (name based, SHA-1)
content: F8:1D:4F:AE:7D:EC:01:D0:27:65:00:A0:C9:1E:6B:F6
(not decipherable: truncated SHA-1 message digest only)
Best regards
Heinrich
>
> Thanks
> /Ilias
>> I'll send a patch with the changes
>>
>> Regards
>> /Ilias
>>>
>>> For the moment I am unable to make the code work in all the following cases:
>>>
>>> - genguid on little-endian
>>> - genguid on big-endian
>>> - sandbox ESRT on little-endian
>>>
>>> I will let you and Caleb know if I make any progress.
>>>
>>> Best regards,
>>> Vincent.
>>>
>>>>
>>>> Thanks
>>>> /Ilias
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 11:00 ` Heinrich Schuchardt
@ 2024-06-21 17:06 ` Vincent Stehlé
2024-06-21 17:11 ` Caleb Connolly
0 siblings, 1 reply; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-21 17:06 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, Caleb Connolly, Tom Rini, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
u-boot
On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
(..)
> The current specification is in RFC 9562, 4.1, "Variant field"
>
> "The variant field consists of a variable number of the most significant
> bits of octet 8 of the UUID.
>
> ...
>
> Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> of Table 1."
>
> This reference to byte 8 does not depend on endianness.
Hi Heinrich,
Agreed, variant is not concerned by the endianness.
>
> U-Boot's include/uuid.h has:
>
> /* This is structure is in big-endian */
> struct uuid {
>
> The field time_hi_and_version needs to be stored in big-endian fashion.
Thanks! I thought this structure was used to hold either a big-endian UUID or a
little-endian GUID, but now you have convinced me.
This confirms that the generation of the dynamic GUID is missing something:
gen_uuid_v5(&namespace,
(struct uuid *)&fw_array[i].image_type_id,
compatible, strlen(compatible),
fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
- sizeof(uint16_t),
NULL);
It is not possible to cast the little-endian efi_guid_t .image_type_id as the
big-endian struct uuid output of gen_uuid_v5() like this; we need to convert the
three time fields from big to little endianness.
>
>
> tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> typical in the EFI context but not understood by 'uuid -d'. Maybe we
> should add a parameter for the output format.
My understanding is that there is a single universal string format for both
UUIDs and GUIDs, which uuid -d understands, and which has no notion of
endianness. (Only the structures in memory have an endianness.)
This means we do not need an output format parameter.
genguid is calling the new gen_uuid_v5() function, which outputs a big-endian
struct uuid. Therefore, genguid should print it with 'STD format, not 'GUID.
Best regards,
Vincent.
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 17:06 ` Vincent Stehlé
@ 2024-06-21 17:11 ` Caleb Connolly
2024-06-24 9:14 ` Vincent Stehlé
0 siblings, 1 reply; 36+ messages in thread
From: Caleb Connolly @ 2024-06-21 17:11 UTC (permalink / raw)
To: Heinrich Schuchardt, Ilias Apalodimas, Caleb Connolly, Tom Rini,
Simon Glass, Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi,
Richard Hughes, U-Boot Mailing List
On Fri, 21 Jun 2024, 19:06 Vincent Stehlé, <vincent.stehle@arm.com> wrote:
> On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
> (..)
> > The current specification is in RFC 9562, 4.1, "Variant field"
> >
> > "The variant field consists of a variable number of the most significant
> > bits of octet 8 of the UUID.
> >
> > ...
> >
> > Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> > (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> > of Table 1."
> >
> > This reference to byte 8 does not depend on endianness.
>
> Hi Heinrich,
>
> Agreed, variant is not concerned by the endianness.
>
> >
> > U-Boot's include/uuid.h has:
> >
> > /* This is structure is in big-endian */
> > struct uuid {
> >
> > The field time_hi_and_version needs to be stored in big-endian fashion.
>
> Thanks! I thought this structure was used to hold either a big-endian UUID
> or a
> little-endian GUID, but now you have convinced me.
>
> This confirms that the generation of the dynamic GUID is missing something:
>
> gen_uuid_v5(&namespace,
> (struct uuid *)&fw_array[i].image_type_id,
> compatible, strlen(compatible),
> fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> - sizeof(uint16_t),
> NULL);
>
> It is not possible to cast the little-endian efi_guid_t .image_type_id as
> the
> big-endian struct uuid output of gen_uuid_v5() like this; we need to
> convert the
> three time fields from big to little endianness.
>
I'm inclined to disagree, the comment above struct uuid in include/uuid.h
state clearly that the format in memory is always big endian, but that a
GUID when printed has some fields converted to little endian.
> >
> >
> > tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> > typical in the EFI context but not understood by 'uuid -d'. Maybe we
> > should add a parameter for the output format.
>
> My understanding is that there is a single universal string format for both
> UUIDs and GUIDs, which uuid -d understands, and which has no notion of
> endianness. (Only the structures in memory have an endianness.)
> This means we do not need an output format parameter.
>
> genguid is calling the new gen_uuid_v5() function, which outputs a
> big-endian
> struct uuid. Therefore, genguid should print it with 'STD format, not
> 'GUID.
>
> Best regards,
> Vincent.
>
> >
> > Best regards
> >
> > Heinrich
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
2024-06-21 17:11 ` Caleb Connolly
@ 2024-06-24 9:14 ` Vincent Stehlé
0 siblings, 0 replies; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-24 9:14 UTC (permalink / raw)
To: Caleb Connolly
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes,
U-Boot Mailing List
On Fri, Jun 21, 2024 at 07:11:28PM +0200, Caleb Connolly wrote:
> On Fri, 21 Jun 2024, 19:06 Vincent Stehlé, <vincent.stehle@arm.com> wrote:
>
> > On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
> > (..)
> > > The current specification is in RFC 9562, 4.1, "Variant field"
> > >
> > > "The variant field consists of a variable number of the most significant
> > > bits of octet 8 of the UUID.
> > >
> > > ...
> > >
> > > Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> > > (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> > > of Table 1."
> > >
> > > This reference to byte 8 does not depend on endianness.
> >
> > Hi Heinrich,
> >
> > Agreed, variant is not concerned by the endianness.
> >
> > >
> > > U-Boot's include/uuid.h has:
> > >
> > > /* This is structure is in big-endian */
> > > struct uuid {
> > >
> > > The field time_hi_and_version needs to be stored in big-endian fashion.
> >
> > Thanks! I thought this structure was used to hold either a big-endian UUID
> > or a
> > little-endian GUID, but now you have convinced me.
> >
> > This confirms that the generation of the dynamic GUID is missing something:
> >
> > gen_uuid_v5(&namespace,
> > (struct uuid *)&fw_array[i].image_type_id,
> > compatible, strlen(compatible),
> > fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> > - sizeof(uint16_t),
> > NULL);
> >
> > It is not possible to cast the little-endian efi_guid_t .image_type_id as
> > the
> > big-endian struct uuid output of gen_uuid_v5() like this; we need to
> > convert the
> > three time fields from big to little endianness.
> >
>
> I'm inclined to disagree, the comment above struct uuid in include/uuid.h
> state clearly that the format in memory is always big endian, but that a
> GUID when printed has some fields converted to little endian.
Hi Caleb,
I read the comments above struct uuid differently: the GUID has some fields
little-endian when stored in memory (and can thus not be stored in a struct
uuid after Heinrich's comments).
This is consistent with how it is done in U-Boot in various locations; for
example, the EFI_GUID() macro stores a GUID with its time fields little-endian
in memory. Similarly, the callers of uuid_str_to_bin() or uuid_bin_to_str() with
format UUID_STR_FORMAT_GUID have indeed a little-endian GUID in memory (most
often an efi_guid_t). This is also consistent with the UEFI specification's
16-byte buffer format. [1]
When you have a big-endian UUID in memory, the version field is stored in byte
6, which is consistent with the RFC 9562. [2]
If you convert this big-endian UUID with uuid_bin_to_str() and
UUID_STR_FORMAT_GUID as in genguid, the "time high and version" field's bytes
will be printed with byte 7 first and then byte 6, as per guid_char_order[].
This is in contradiction with the RFC, which shows that the version field ("M")
should be printed first.
If you print the UUID with format 'STD, you will indeed have byte 6 containing
the version field printed first as it should (uuid_char_order[]).
This is confirmed by "uuid -d".
Best regards,
Vincent.
[1] https://uefi.org/specs/UEFI/2.10/Apx_A_GUID_and_Time_Formats.html
[2] https://www.rfc-editor.org/rfc/rfc9562
>
>
> > >
> > >
> > > tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> > > typical in the EFI context but not understood by 'uuid -d'. Maybe we
> > > should add a parameter for the output format.
> >
> > My understanding is that there is a single universal string format for both
> > UUIDs and GUIDs, which uuid -d understands, and which has no notion of
> > endianness. (Only the structures in memory have an endianness.)
> > This means we do not need an output format parameter.
> >
> > genguid is calling the new gen_uuid_v5() function, which outputs a
> > big-endian
> > struct uuid. Therefore, genguid should print it with 'STD format, not
> > 'GUID.
> >
> > Best regards,
> > Vincent.
> >
> > >
> > > Best regards
> > >
> > > Heinrich
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] Proposed changes to dynamic UUIDs v3
2024-06-21 9:12 ` Vincent Stehlé
2024-06-21 11:00 ` Heinrich Schuchardt
2024-06-21 11:01 ` Ilias Apalodimas
@ 2024-06-27 9:55 ` Vincent Stehlé
2024-07-02 13:49 ` Caleb Connolly
2 siblings, 1 reply; 36+ messages in thread
From: Vincent Stehlé @ 2024-06-27 9:55 UTC (permalink / raw)
To: u-boot
Cc: Vincent Stehlé, Caleb Connolly, Tom Rini,
Heinrich Schuchardt, Ilias Apalodimas, Simon Glass, Mario Six,
Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes
Here are the changes that I would like to suggest for the "efi:
CapsuleUpdate: support for dynamic UUIDs" v3 patch series:
- Convert from big-endian UUID to little-endian GUID in
efi_capsule_update_info_gen_ids().
- Fix tmp size and masking in gen_uuid_v5().
- Use UUID_STR_FORMAT_STD in all places where we are dealing with a
big-endian UUID.
- Update all GUIDs constants in the code and in the tests accordingly. This
gets rid of the following broken UUIDs:
5af91295-5a99-f62b-80d7-e9574de87170
8ee418dc-7e00-e156-80a7-274fbbc05ba8
935fe837-fac8-4394-c008-737d8852c60d
fd5db83c-12f3-a46b-80a9-e3007c7ff56e
ffd97379-0956-fa94-c003-8bfcf5cc097b
- Also, a few minor modifications here and there.
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Richard Hughes <hughsient@gmail.com>
---
include/sandbox_efi_capsule.h | 6 +++---
lib/efi_loader/efi_firmware.c | 14 +++++++++++---
lib/uuid.c | 8 ++++----
test/lib/uuid.c | 12 ++++++------
.../test_efi_capsule/test_capsule_firmware_fit.py | 4 ++--
.../test_efi_capsule/test_capsule_firmware_raw.py | 8 ++++----
.../test_capsule_firmware_signed_fit.py | 2 +-
.../test_capsule_firmware_signed_raw.py | 4 ++--
test/py/tests/test_efi_capsule/version.dts | 6 +++---
tools/.gitignore | 1 +
tools/binman/etype/efi_capsule.py | 2 +-
tools/binman/ftest.py | 2 +-
tools/genguid.c | 7 +++----
13 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h
index 25ac496ea24..6f0de5a1e25 100644
--- a/include/sandbox_efi_capsule.h
+++ b/include/sandbox_efi_capsule.h
@@ -6,9 +6,9 @@
#if !defined(_SANDBOX_EFI_CAPSULE_H_)
#define _SANDBOX_EFI_CAPSULE_H_
-#define SANDBOX_UBOOT_IMAGE_GUID "fd5db83c-12f3-a46b-80a9-e3007c7ff56e"
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID "935fe837-fac8-4394-c008-737d8852c60d"
-#define SANDBOX_FIT_IMAGE_GUID "ffd97379-0956-fa94-c003-8bfcf5cc097b"
+#define SANDBOX_UBOOT_IMAGE_GUID "50980990-5af9-5522-86e2-8f05f4d7313c"
+#define SANDBOX_UBOOT_ENV_IMAGE_GUID "3554b655-b9f0-5240-ace2-6f34c2f7fcca"
+#define SANDBOX_FIT_IMAGE_GUID "8b38adc7-df0c-5769-8b89-c090ca3d07a7"
#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4"
#define UBOOT_FIT_IMAGE "u-boot_bin_env.itb"
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index a8dafe4f01a..f0d0c3fa972 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -258,7 +258,7 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
static efi_status_t efi_capsule_update_info_gen_ids(void)
{
int ret, i;
- struct uuid namespace;
+ struct uuid namespace, type;
const char *compatible; /* Full array including null bytes */
struct efi_fw_image *fw_array;
@@ -269,7 +269,7 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
return EFI_SUCCESS;
ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
- (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
+ (unsigned char *)&namespace, UUID_STR_FORMAT_STD);
if (ret) {
log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid: %d\n", __func__, ret);
return EFI_UNSUPPORTED;
@@ -289,12 +289,20 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
for (i = 0; i < update_info.num_images; i++) {
gen_uuid_v5(&namespace,
- (struct uuid *)&fw_array[i].image_type_id,
+ &type,
compatible, strlen(compatible),
fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
- sizeof(uint16_t),
NULL);
+ /* Convert to little-endian GUID. */
+ fw_array[i].image_type_id = (efi_guid_t)EFI_GUID(
+ be32_to_cpu(type.time_low), be16_to_cpu(type.time_mid),
+ be16_to_cpu(type.time_hi_and_version),
+ type.clock_seq_hi_and_reserved, type.clock_seq_low,
+ type.node[0], type.node[1], type.node[2], type.node[3],
+ type.node[4], type.node[5]);
+
log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
&fw_array[i].image_type_id);
}
diff --git a/lib/uuid.c b/lib/uuid.c
index 89911b06ccc..a8c3a504090 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -391,7 +391,7 @@ void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
va_list args;
const uint8_t *data;
uint8_t hash[SHA1_SUM_LEN];
- uint32_t tmp;
+ uint16_t tmp;
sha1_starts(&ctx);
/* Hash the namespace UUID as salt */
@@ -411,11 +411,11 @@ void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
memcpy(uuid, hash, sizeof(*uuid));
/* Configure variant/version bits */
- tmp = be32_to_cpu(uuid->time_hi_and_version);
+ tmp = be16_to_cpu(uuid->time_hi_and_version);
tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
- uuid->time_hi_and_version = cpu_to_be32(tmp);
+ uuid->time_hi_and_version = cpu_to_be16(tmp);
- uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
+ uuid->clock_seq_hi_and_reserved &= ~UUID_VARIANT_MASK;
uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
}
#endif
diff --git a/test/lib/uuid.c b/test/lib/uuid.c
index 0bcb332e534..b5940fa855c 100644
--- a/test/lib/uuid.c
+++ b/test/lib/uuid.c
@@ -60,7 +60,7 @@ static int lib_test_dynamic_uuid_case(struct unit_test_state *uts,
int j;
ut_assertok(uuid_str_to_bin(data->namespace, (unsigned char *)&namespace,
- UUID_STR_FORMAT_GUID));
+ UUID_STR_FORMAT_STD));
for (j = 0; data->images[j]; j++) {
const char *expected_uuid = data->expected_uuids[j];
@@ -72,7 +72,7 @@ static int lib_test_dynamic_uuid_case(struct unit_test_state *uts,
data->compatible, strlen(data->compatible),
image, u16_strsize(image) - sizeof(uint16_t),
NULL);
- uuid_bin_to_str((unsigned char *)&uuid, uuid_str, UUID_STR_FORMAT_GUID);
+ uuid_bin_to_str((unsigned char *)&uuid, uuid_str, UUID_STR_FORMAT_STD);
ut_asserteq_str(expected_uuid, uuid_str);
}
@@ -94,9 +94,9 @@ static int lib_test_dynamic_uuid(struct unit_test_state *uts)
NULL,
},
.expected_uuids = {
- "fd5db83c-12f3-a46b-80a9-e3007c7ff56e",
- "935fe837-fac8-4394-c008-737d8852c60d",
- "ffd97379-0956-fa94-c003-8bfcf5cc097b",
+ "50980990-5af9-5522-86e2-8f05f4d7313c",
+ "3554b655-b9f0-5240-ace2-6f34c2f7fcca",
+ "8b38adc7-df0c-5769-8b89-c090ca3d07a7",
NULL,
}
},
@@ -108,7 +108,7 @@ static int lib_test_dynamic_uuid(struct unit_test_state *uts)
NULL,
},
.expected_uuids = {
- "8ee418dc-7e00-e156-80a7-274fbbc05ba8",
+ "14c399c8-4e16-5ba4-b720-44426d3a0bb9",
NULL,
}
},
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
index 746da460208..9701acebbe3 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
@@ -146,8 +146,8 @@ class TestEfiCapsuleFirmwareFit():
verify_content(u_boot_console, '100000', 'u-boot:Old')
verify_content(u_boot_console, '150000', 'u-boot-env:Old')
else:
- # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert '5AF91295-5A99-F62B-80D7-E9574DE87170' in ''.join(output)
+ # ensure that SANDBOX_FIT_IMAGE_GUID is in the ESRT.
+ assert '8B38ADC7-DF0C-5769-8B89-C090CA3D07A7' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
index 1866b808657..cedb3a43591 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
@@ -134,10 +134,10 @@ class TestEfiCapsuleFirmwareRaw:
'efidebug capsule esrt'])
# ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
+ assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
+ assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
check_file_removed(u_boot_console, disk_img, capsule_files)
@@ -188,12 +188,12 @@ class TestEfiCapsuleFirmwareRaw:
verify_content(u_boot_console, '150000', 'u-boot-env:Old')
else:
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
+ assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
# ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
+ assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
assert 'ESRT: fw_version=10' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
index a4e0a3bc73f..10eb8281457 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
@@ -157,7 +157,7 @@ class TestEfiCapsuleFirmwareSignedFit():
'efidebug capsule esrt'])
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
+ assert '8B38ADC7-DF0C-5769-8B89-C090CA3D07A7' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
index 260c7186063..01e5f3b3405 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
@@ -151,12 +151,12 @@ class TestEfiCapsuleFirmwareSignedRaw():
'efidebug capsule esrt'])
# ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
- assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
+ assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
assert 'ESRT: fw_version=5' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
# ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
- assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
+ assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
assert 'ESRT: fw_version=10' in ''.join(output)
assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
diff --git a/test/py/tests/test_efi_capsule/version.dts b/test/py/tests/test_efi_capsule/version.dts
index 3f0698bf728..c447a3d8199 100644
--- a/test/py/tests/test_efi_capsule/version.dts
+++ b/test/py/tests/test_efi_capsule/version.dts
@@ -8,17 +8,17 @@
image1 {
lowest-supported-version = <3>;
image-index = <1>;
- image-type-id = "FD5DB83C-12F3-A46B-80A9-E3007C7FF56E";
+ image-type-id = "50980990-5AF9-5522-86E2-8F05F4D7313C";
};
image2 {
lowest-supported-version = <7>;
image-index = <2>;
- image-type-id = "935FE837-FAC8-4394-C008-737D8852C60D";
+ image-type-id = "3554B655-B9F0-5240-ACE2-6F34C2F7FCCA";
};
image3 {
lowest-supported-version = <3>;
image-index = <1>;
- image-type-id = "FFD97379-0956-FA94-C003-8BFCF5CC097B";
+ image-type-id = "8B38ADC7-DF0C-5769-8B89-C090CA3D07A7";
};
};
};
diff --git a/tools/.gitignore b/tools/.gitignore
index 0108c567309..6b7d7b89c39 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -15,6 +15,7 @@
/gdb/gdbsend
/gen_eth_addr
/gen_ethaddr_crc
+/genguid
/ifdtool
/ifwitool
/img2srec
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py
index da1f9b0a381..f9f4fda5f71 100644
--- a/tools/binman/etype/efi_capsule.py
+++ b/tools/binman/etype/efi_capsule.py
@@ -24,7 +24,7 @@ def get_binman_test_guid(type_str):
The actual GUID value (str)
"""
TYPE_TO_GUID = {
- 'binman-test' : 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
+ 'binman-test' : '50980990-5af9-5522-86e2-8f05f4d7313c'
}
return TYPE_TO_GUID[type_str]
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index dc602b95ecd..5610afc26de 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -124,7 +124,7 @@ TEE_ADDR = 0x5678
# Firmware Management Protocol(FMP) GUID
FW_MGMT_GUID = '6dcbd5ed-e82d-4c44-bda1-7194199ad92a'
# Image GUID specified in the DTS
-CAPSULE_IMAGE_GUID = 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
+CAPSULE_IMAGE_GUID = '50980990-5af9-5522-86e2-8f05f4d7313c'
# Windows cert GUID
WIN_CERT_TYPE_EFI_GUID = '4aafd29d-68df-49ee-8aa9-347d375665a7'
# Empty capsule GUIDs
diff --git a/tools/genguid.c b/tools/genguid.c
index e71bc1d48f9..1e365399721 100644
--- a/tools/genguid.c
+++ b/tools/genguid.c
@@ -15,7 +15,6 @@
#include <uuid.h>
static struct option options[] = {
- {"dtb", required_argument, NULL, 'd'},
{"compat", required_argument, NULL, 'c'},
{"help", no_argument, NULL, 'h'},
{"verbose", no_argument, NULL, 'v'},
@@ -99,7 +98,7 @@ int main(int argc, char **argv)
return 1;
}
- if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_GUID)) {
+ if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_STD)) {
fprintf(stderr, "ERROR: Check that your UUID is formatted correctly.\n");
exit(EXIT_FAILURE);
}
@@ -116,7 +115,7 @@ int main(int argc, char **argv)
if (debug) {
fprintf(stderr, "GUID: ");
- uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_GUID);
+ uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_STD);
fprintf(stderr, "%s\n", uuid_str);
fprintf(stderr, "Compatible: \"%s\"\n", compatible);
fprintf(stderr, "Images: ");
@@ -134,7 +133,7 @@ int main(int argc, char **argv)
images_u16[i], u16_strsize(images_u16[i]) - sizeof(uint16_t),
NULL);
- uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_GUID);
+ uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_STD);
image_uuids[i] = strdup(uuid_str);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] Proposed changes to dynamic UUIDs v3
2024-06-27 9:55 ` [PATCH] Proposed changes to dynamic UUIDs v3 Vincent Stehlé
@ 2024-07-02 13:49 ` Caleb Connolly
0 siblings, 0 replies; 36+ messages in thread
From: Caleb Connolly @ 2024-07-02 13:49 UTC (permalink / raw)
To: Vincent Stehlé, u-boot
Cc: Tom Rini, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
Mario Six, Alper Nebi Yasak, Abdellatif El Khlifi, Richard Hughes
Hi Vincent,
On 27/06/2024 11:55, Vincent Stehlé wrote:
> Here are the changes that I would like to suggest for the "efi:
> CapsuleUpdate: support for dynamic UUIDs" v3 patch series:
>
> - Convert from big-endian UUID to little-endian GUID in
> efi_capsule_update_info_gen_ids().
>
> - Fix tmp size and masking in gen_uuid_v5().
>
> - Use UUID_STR_FORMAT_STD in all places where we are dealing with a
> big-endian UUID.
>
> - Update all GUIDs constants in the code and in the tests accordingly. This
> gets rid of the following broken UUIDs:
>
> 5af91295-5a99-f62b-80d7-e9574de87170
> 8ee418dc-7e00-e156-80a7-274fbbc05ba8
> 935fe837-fac8-4394-c008-737d8852c60d
> fd5db83c-12f3-a46b-80a9-e3007c7ff56e
> ffd97379-0956-fa94-c003-8bfcf5cc097b
>
> - Also, a few minor modifications here and there.
Thanks, this was really helpful for prepping v4. I decided to go with a
slightly different approach and just make the the v5 generator produce a
little endian GUID rather than a BE UUID.
V4 is here:
https://lore.kernel.org/u-boot/20240702-b4-dynamic-uuid-v4-0-a00c82d1f504@linaro.org
Kind regards,
>
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Richard Hughes <hughsient@gmail.com>
> ---
> include/sandbox_efi_capsule.h | 6 +++---
> lib/efi_loader/efi_firmware.c | 14 +++++++++++---
> lib/uuid.c | 8 ++++----
> test/lib/uuid.c | 12 ++++++------
> .../test_efi_capsule/test_capsule_firmware_fit.py | 4 ++--
> .../test_efi_capsule/test_capsule_firmware_raw.py | 8 ++++----
> .../test_capsule_firmware_signed_fit.py | 2 +-
> .../test_capsule_firmware_signed_raw.py | 4 ++--
> test/py/tests/test_efi_capsule/version.dts | 6 +++---
> tools/.gitignore | 1 +
> tools/binman/etype/efi_capsule.py | 2 +-
> tools/binman/ftest.py | 2 +-
> tools/genguid.c | 7 +++----
> 13 files changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h
> index 25ac496ea24..6f0de5a1e25 100644
> --- a/include/sandbox_efi_capsule.h
> +++ b/include/sandbox_efi_capsule.h
> @@ -6,9 +6,9 @@
> #if !defined(_SANDBOX_EFI_CAPSULE_H_)
> #define _SANDBOX_EFI_CAPSULE_H_
>
> -#define SANDBOX_UBOOT_IMAGE_GUID "fd5db83c-12f3-a46b-80a9-e3007c7ff56e"
> -#define SANDBOX_UBOOT_ENV_IMAGE_GUID "935fe837-fac8-4394-c008-737d8852c60d"
> -#define SANDBOX_FIT_IMAGE_GUID "ffd97379-0956-fa94-c003-8bfcf5cc097b"
> +#define SANDBOX_UBOOT_IMAGE_GUID "50980990-5af9-5522-86e2-8f05f4d7313c"
> +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "3554b655-b9f0-5240-ace2-6f34c2f7fcca"
> +#define SANDBOX_FIT_IMAGE_GUID "8b38adc7-df0c-5769-8b89-c090ca3d07a7"
> #define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4"
>
> #define UBOOT_FIT_IMAGE "u-boot_bin_env.itb"
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index a8dafe4f01a..f0d0c3fa972 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -258,7 +258,7 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
> static efi_status_t efi_capsule_update_info_gen_ids(void)
> {
> int ret, i;
> - struct uuid namespace;
> + struct uuid namespace, type;
> const char *compatible; /* Full array including null bytes */
> struct efi_fw_image *fw_array;
>
> @@ -269,7 +269,7 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
> return EFI_SUCCESS;
>
> ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
> - (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
> + (unsigned char *)&namespace, UUID_STR_FORMAT_STD);
> if (ret) {
> log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid: %d\n", __func__, ret);
> return EFI_UNSUPPORTED;
> @@ -289,12 +289,20 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
>
> for (i = 0; i < update_info.num_images; i++) {
> gen_uuid_v5(&namespace,
> - (struct uuid *)&fw_array[i].image_type_id,
> + &type,
> compatible, strlen(compatible),
> fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> - sizeof(uint16_t),
> NULL);
>
> + /* Convert to little-endian GUID. */
> + fw_array[i].image_type_id = (efi_guid_t)EFI_GUID(
> + be32_to_cpu(type.time_low), be16_to_cpu(type.time_mid),
> + be16_to_cpu(type.time_hi_and_version),
> + type.clock_seq_hi_and_reserved, type.clock_seq_low,
> + type.node[0], type.node[1], type.node[2], type.node[3],
> + type.node[4], type.node[5]);
> +
> log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
> &fw_array[i].image_type_id);
> }
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 89911b06ccc..a8c3a504090 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -391,7 +391,7 @@ void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> va_list args;
> const uint8_t *data;
> uint8_t hash[SHA1_SUM_LEN];
> - uint32_t tmp;
> + uint16_t tmp;
>
> sha1_starts(&ctx);
> /* Hash the namespace UUID as salt */
> @@ -411,11 +411,11 @@ void gen_uuid_v5(const struct uuid *namespace, struct uuid *uuid, ...)
> memcpy(uuid, hash, sizeof(*uuid));
>
> /* Configure variant/version bits */
> - tmp = be32_to_cpu(uuid->time_hi_and_version);
> + tmp = be16_to_cpu(uuid->time_hi_and_version);
> tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
> - uuid->time_hi_and_version = cpu_to_be32(tmp);
> + uuid->time_hi_and_version = cpu_to_be16(tmp);
>
> - uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;
> + uuid->clock_seq_hi_and_reserved &= ~UUID_VARIANT_MASK;
> uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;
> }
> #endif
> diff --git a/test/lib/uuid.c b/test/lib/uuid.c
> index 0bcb332e534..b5940fa855c 100644
> --- a/test/lib/uuid.c
> +++ b/test/lib/uuid.c
> @@ -60,7 +60,7 @@ static int lib_test_dynamic_uuid_case(struct unit_test_state *uts,
> int j;
>
> ut_assertok(uuid_str_to_bin(data->namespace, (unsigned char *)&namespace,
> - UUID_STR_FORMAT_GUID));
> + UUID_STR_FORMAT_STD));
>
> for (j = 0; data->images[j]; j++) {
> const char *expected_uuid = data->expected_uuids[j];
> @@ -72,7 +72,7 @@ static int lib_test_dynamic_uuid_case(struct unit_test_state *uts,
> data->compatible, strlen(data->compatible),
> image, u16_strsize(image) - sizeof(uint16_t),
> NULL);
> - uuid_bin_to_str((unsigned char *)&uuid, uuid_str, UUID_STR_FORMAT_GUID);
> + uuid_bin_to_str((unsigned char *)&uuid, uuid_str, UUID_STR_FORMAT_STD);
>
> ut_asserteq_str(expected_uuid, uuid_str);
> }
> @@ -94,9 +94,9 @@ static int lib_test_dynamic_uuid(struct unit_test_state *uts)
> NULL,
> },
> .expected_uuids = {
> - "fd5db83c-12f3-a46b-80a9-e3007c7ff56e",
> - "935fe837-fac8-4394-c008-737d8852c60d",
> - "ffd97379-0956-fa94-c003-8bfcf5cc097b",
> + "50980990-5af9-5522-86e2-8f05f4d7313c",
> + "3554b655-b9f0-5240-ace2-6f34c2f7fcca",
> + "8b38adc7-df0c-5769-8b89-c090ca3d07a7",
> NULL,
> }
> },
> @@ -108,7 +108,7 @@ static int lib_test_dynamic_uuid(struct unit_test_state *uts)
> NULL,
> },
> .expected_uuids = {
> - "8ee418dc-7e00-e156-80a7-274fbbc05ba8",
> + "14c399c8-4e16-5ba4-b720-44426d3a0bb9",
> NULL,
> }
> },
> 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
> index 746da460208..9701acebbe3 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
> @@ -146,8 +146,8 @@ class TestEfiCapsuleFirmwareFit():
> verify_content(u_boot_console, '100000', 'u-boot:Old')
> verify_content(u_boot_console, '150000', 'u-boot-env:Old')
> else:
> - # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert '5AF91295-5A99-F62B-80D7-E9574DE87170' in ''.join(output)
> + # ensure that SANDBOX_FIT_IMAGE_GUID is in the ESRT.
> + assert '8B38ADC7-DF0C-5769-8B89-C090CA3D07A7' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> index 1866b808657..cedb3a43591 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
> @@ -134,10 +134,10 @@ class TestEfiCapsuleFirmwareRaw:
> 'efidebug capsule esrt'])
>
> # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
> + assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> + assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
>
> check_file_removed(u_boot_console, disk_img, capsule_files)
>
> @@ -188,12 +188,12 @@ class TestEfiCapsuleFirmwareRaw:
> verify_content(u_boot_console, '150000', 'u-boot-env:Old')
> else:
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> + assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
> + assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
> assert 'ESRT: fw_version=10' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
>
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> index a4e0a3bc73f..10eb8281457 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> @@ -157,7 +157,7 @@ class TestEfiCapsuleFirmwareSignedFit():
> 'efidebug capsule esrt'])
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> + assert '8B38ADC7-DF0C-5769-8B89-C090CA3D07A7' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> index 260c7186063..01e5f3b3405 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> @@ -151,12 +151,12 @@ class TestEfiCapsuleFirmwareSignedRaw():
> 'efidebug capsule esrt'])
>
> # ensure that SANDBOX_UBOOT_IMAGE_GUID is in the ESRT.
> - assert 'FD5DB83C-12F3-A46B-80A9-E3007C7FF56E' in ''.join(output)
> + assert '50980990-5AF9-5522-86E2-8F05F4D7313C' in ''.join(output)
> assert 'ESRT: fw_version=5' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=3' in ''.join(output)
>
> # ensure that SANDBOX_UBOOT_ENV_IMAGE_GUID is in the ESRT.
> - assert '935FE837-FAC8-4394-C008-737D8852C60D' in ''.join(output)
> + assert '3554B655-B9F0-5240-ACE2-6F34C2F7FCCA' in ''.join(output)
> assert 'ESRT: fw_version=10' in ''.join(output)
> assert 'ESRT: lowest_supported_fw_version=7' in ''.join(output)
>
> diff --git a/test/py/tests/test_efi_capsule/version.dts b/test/py/tests/test_efi_capsule/version.dts
> index 3f0698bf728..c447a3d8199 100644
> --- a/test/py/tests/test_efi_capsule/version.dts
> +++ b/test/py/tests/test_efi_capsule/version.dts
> @@ -8,17 +8,17 @@
> image1 {
> lowest-supported-version = <3>;
> image-index = <1>;
> - image-type-id = "FD5DB83C-12F3-A46B-80A9-E3007C7FF56E";
> + image-type-id = "50980990-5AF9-5522-86E2-8F05F4D7313C";
> };
> image2 {
> lowest-supported-version = <7>;
> image-index = <2>;
> - image-type-id = "935FE837-FAC8-4394-C008-737D8852C60D";
> + image-type-id = "3554B655-B9F0-5240-ACE2-6F34C2F7FCCA";
> };
> image3 {
> lowest-supported-version = <3>;
> image-index = <1>;
> - image-type-id = "FFD97379-0956-FA94-C003-8BFCF5CC097B";
> + image-type-id = "8B38ADC7-DF0C-5769-8B89-C090CA3D07A7";
> };
> };
> };
> diff --git a/tools/.gitignore b/tools/.gitignore
> index 0108c567309..6b7d7b89c39 100644
> --- a/tools/.gitignore
> +++ b/tools/.gitignore
> @@ -15,6 +15,7 @@
> /gdb/gdbsend
> /gen_eth_addr
> /gen_ethaddr_crc
> +/genguid
> /ifdtool
> /ifwitool
> /img2srec
> diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py
> index da1f9b0a381..f9f4fda5f71 100644
> --- a/tools/binman/etype/efi_capsule.py
> +++ b/tools/binman/etype/efi_capsule.py
> @@ -24,7 +24,7 @@ def get_binman_test_guid(type_str):
> The actual GUID value (str)
> """
> TYPE_TO_GUID = {
> - 'binman-test' : 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
> + 'binman-test' : '50980990-5af9-5522-86e2-8f05f4d7313c'
> }
>
> return TYPE_TO_GUID[type_str]
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index dc602b95ecd..5610afc26de 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -124,7 +124,7 @@ TEE_ADDR = 0x5678
> # Firmware Management Protocol(FMP) GUID
> FW_MGMT_GUID = '6dcbd5ed-e82d-4c44-bda1-7194199ad92a'
> # Image GUID specified in the DTS
> -CAPSULE_IMAGE_GUID = 'fd5db83c-12f3-a46b-80a9-e3007c7ff56e'
> +CAPSULE_IMAGE_GUID = '50980990-5af9-5522-86e2-8f05f4d7313c'
> # Windows cert GUID
> WIN_CERT_TYPE_EFI_GUID = '4aafd29d-68df-49ee-8aa9-347d375665a7'
> # Empty capsule GUIDs
> diff --git a/tools/genguid.c b/tools/genguid.c
> index e71bc1d48f9..1e365399721 100644
> --- a/tools/genguid.c
> +++ b/tools/genguid.c
> @@ -15,7 +15,6 @@
> #include <uuid.h>
>
> static struct option options[] = {
> - {"dtb", required_argument, NULL, 'd'},
> {"compat", required_argument, NULL, 'c'},
> {"help", no_argument, NULL, 'h'},
> {"verbose", no_argument, NULL, 'v'},
> @@ -99,7 +98,7 @@ int main(int argc, char **argv)
> return 1;
> }
>
> - if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_GUID)) {
> + if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace, UUID_STR_FORMAT_STD)) {
> fprintf(stderr, "ERROR: Check that your UUID is formatted correctly.\n");
> exit(EXIT_FAILURE);
> }
> @@ -116,7 +115,7 @@ int main(int argc, char **argv)
>
> if (debug) {
> fprintf(stderr, "GUID: ");
> - uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_GUID);
> + uuid_bin_to_str((uint8_t *)&namespace, uuid_str, UUID_STR_FORMAT_STD);
> fprintf(stderr, "%s\n", uuid_str);
> fprintf(stderr, "Compatible: \"%s\"\n", compatible);
> fprintf(stderr, "Images: ");
> @@ -134,7 +133,7 @@ int main(int argc, char **argv)
> images_u16[i], u16_strsize(images_u16[i]) - sizeof(uint16_t),
> NULL);
>
> - uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_GUID);
> + uuid_bin_to_str((uint8_t *)&image_type_id, uuid_str, UUID_STR_FORMAT_STD);
> image_uuids[i] = strdup(uuid_str);
> }
>
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2024-07-02 13:49 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-31 13:50 [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 1/7] lib: uuid: add UUID v5 support Caleb Connolly
2024-05-31 16:11 ` Ilias Apalodimas
2024-06-05 2:13 ` Simon Glass
2024-06-05 12:06 ` Caleb Connolly
2024-06-05 5:33 ` Heinrich Schuchardt
2024-05-31 13:50 ` [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs Caleb Connolly
2024-06-05 5:52 ` Heinrich Schuchardt
2024-06-05 12:22 ` Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 3/7] doc: uefi: document dynamic UUID generation Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 4/7] sandbox: switch to dynamic UUIDs Caleb Connolly
2024-05-31 15:43 ` Ilias Apalodimas
2024-05-31 13:50 ` [PATCH v3 5/7] lib: uuid: supporting building as part of host tools Caleb Connolly
2024-05-31 13:50 ` [PATCH v3 6/7] tools: add genguid tool Caleb Connolly
2024-06-05 2:13 ` Simon Glass
2024-06-05 6:36 ` Heinrich Schuchardt
2024-06-05 6:38 ` Heinrich Schuchardt
2024-06-05 9:25 ` Ilias Apalodimas
2024-06-05 12:29 ` Caleb Connolly
2024-06-05 13:43 ` Ilias Apalodimas
2024-06-20 14:58 ` Vincent Stehlé
2024-05-31 13:50 ` [PATCH v3 7/7] test: lib/uuid: add unit tests for dynamic UUIDs Caleb Connolly
2024-06-05 5:59 ` [PATCH v3 0/7] efi: CapsuleUpdate: support " Heinrich Schuchardt
2024-06-05 17:02 ` Caleb Connolly
2024-06-19 14:02 ` Vincent Stehlé
2024-06-19 19:15 ` Ilias Apalodimas
2024-06-21 9:12 ` Vincent Stehlé
2024-06-21 11:00 ` Heinrich Schuchardt
2024-06-21 17:06 ` Vincent Stehlé
2024-06-21 17:11 ` Caleb Connolly
2024-06-24 9:14 ` Vincent Stehlé
2024-06-21 11:01 ` Ilias Apalodimas
2024-06-21 11:25 ` Ilias Apalodimas
2024-06-21 13:16 ` Heinrich Schuchardt
2024-06-27 9:55 ` [PATCH] Proposed changes to dynamic UUIDs v3 Vincent Stehlé
2024-07-02 13:49 ` Caleb Connolly
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.