date:20240604

Re: [PATCH v3 6/7] tools: add genguid tool

2024-06-04 Thread Heinrich Schuchardt


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 
---
  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 ..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:
+    ----
+
+.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 
+
+.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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+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"
+    "  -

Re: [PATCH v3 6/7] tools: add genguid tool

2024-06-04 Thread Heinrich Schuchardt


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 
---
  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 ..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:
+----
+
+.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 
+
+.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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+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

Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs

2024-06-04 Thread Heinrich Schuchardt

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_iwjjnha4gmf897mqyzndbgjfg8k4kwgstxwuy72wkyli...@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-c26f31057...@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-e8154e00e...@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

// Caleb (they/them)

Re: [PATCH 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL

2024-06-04 Thread Ilias Apalodimas

Hi Heinrich

On Wed, 5 Jun 2024 at 07:09, Heinrich Schuchardt  wrote:
>
> On 6/5/24 05:25, Simon Glass wrote:
> > It does not make sense to enable all SHA algorithms unless they are
> > needed. It bloats the code and in this case, causes chromebook_link to
> > fail to build.
>
> Why would chromebook_link fail to build?
> Is TPM used by U-Boot on that board at all?
>
> >
> > Add a condition to TPM to correct this. Note that the original commit
> > combines refactoring and new features, which makes it hard to see what
> > is going on.
> >
> > Fixes: 97707f12fda tpm: Support boot measurements
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >   lib/Kconfig | 8 
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 189e6eb31aa..70b32362ada 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -438,10 +438,10 @@ config TPM
> >   bool "Trusted Platform Module (TPM) Support"
> >   depends on DM
> >   imply DM_RNG
> > - select SHA1
> > - select SHA256
> > - select SHA384
> > - select SHA512
> > + select SHA1 if EFI_TCG2_PROTOCOL
> > + select SHA256 if EFI_TCG2_PROTOCOL
> > + select SHA384 if EFI_TCG2_PROTOCOL
> > + select SHA512 if EFI_TCG2_PROTOCOL
>
> You need to consider CONFIG_MEASURED_BOOT which allows measured boot in
> the non-UEFI case.
>
> Please, take into account
>
> lib/tpm-v1.c:20:
> #error "TPM_AUTH_SESSIONS require SHA1 to be configured, too"
>
> This #error should be replaced by a Kconfig constraint.
>
> I would prefer the select statements to be in lib/efi_loader/Kconfig
> under EFI_TCG2_PROTOCOL.
>
> @Ilias, Eddie:
>
> Why do we require SHA1 which is considered insecure?
>
> Shouldn't we change tpm2_supported_algorithms[] to include only
> algorithms selected in the configuration? This would allow replacing all
> the select statements in Simon's patch by imply.

The algorithms used and configured by the TPM are device runtime
decisions, not compile-time ones and to my knowledge, there are no
devices out there that disable SHA1.

Failing to extend a PCR in an active bank is a security vulnerability.
It would allow the unsealing of secrets if an attacker can replay a
good set of measurements into an unused bank.

We could potentially fix that in the future since we can configure the
TPM active banks on boot, but IIRC we don't support that yet.

Regards
/Ilias

>
> Best regards
>
> Heinrich
>
> >   help
> > This enables support for TPMs which can be used to provide security
> > features for your board. The TPM can be connected via LPC or I2C
>

Re: [PATCH v3 2/7] efi: add a helper to generate dynamic UUIDs

2024-06-04 Thread Heinrich Schuchardt


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 
---
  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 
  #include 
  #include 
  #include 
+#include 

  #include 
  #include 
  #include 
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

Re: [PATCH v3 1/7] lib: uuid: add UUID v5 support

2024-06-04 Thread Heinrich Schuchardt


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 
---
  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 
+#include 

  /*
   * 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 
  #include 
  #include 
  #include 
+#include 

  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_VERSION0x4

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];

Re: [PATCH 5/9] fdt: Correct condition for bloblist existing

2024-06-04 Thread Ilias Apalodimas

Hi Simon,

On Wed, 5 Jun 2024 at 06:26, Simon Glass  wrote:
>
> On some boards, the bloblist is created in SPL once SDRAM is ready. It
> cannot be accessed until that point, so is not available early in SPL.
>
> Add a condition to avoid a hang in this case.
>
> This fixes a hang in chromebook_coral
>
> Fixes: 70fe2385943 ("fdt: Allow the devicetree to come from a bloblist")
>
> Signed-off-by: Simon Glass 
> ---
>
>  lib/fdtdec.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index b2c59ab3818..b141244e3b9 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1669,8 +1669,16 @@ int fdtdec_setup(void)
>  {
> int ret = -ENOENT;
>
> -   /* If allowing a bloblist, check that first */
> -   if (CONFIG_IS_ENABLED(BLOBLIST)) {
> +   /*
> +* If allowing a bloblist, check that first. This would be better
> +* handled with an OF_BLOBLIST Kconfig, but that caused far too much
> +* argument, so add a hack here, used e.g. by chromebook_coral
> +* The necessary test is whether the previous stage passed a bloblist,
> +* not whether this one creates one.
> +*/
> +   if (CONFIG_IS_ENABLED(OF_BLOBLIST) &&
> +   (spl_prev_phase() != PHASE_TPL ||
> +!IS_ENABLED(CONFIG_TPL_BLOBLIST))) {

The same condition exists in common/bloblist.c.
Carve out a function --e.g

bool can can_enable_bloblist(void)
 return 

instead of open coding that

Thanks
/Ilias
> ret = bloblist_maybe_init();
> if (!ret) {
> gd->fdt_blob = bloblist_find(BLOBLISTT_CONTROL_FDT, 
> 0);
> --
> 2.34.1
>

Re: [PATCH v3 0/8] qcom: implement capsule updates

2024-06-04 Thread Sumit Garg

Hi Caleb,

On Mon, 3 Jun 2024 at 18:19, Caleb Connolly  wrote:
>
> Hook up support for capsule updates loaded from disk on Qualcomm
> platforms.
>
> Most Qualcomm devices have an A/B partition layout, with most partitions
> duplicated. The metadata on which slot is active is stored in the GPT
> headers in the vendor-specific attribute bits of each partition.

It's good to see capsule updates support coming up for Qualcomm
platforms. AFAICS, with this series we only update U-Boot on the
current active partition. IOW, real A/B support is still not
supported. Do you think it is possible for U-Boot to update metadata
in GPT headers and for proprietary bootloaders to pick up the U-Boot
from the updated partition?

-Sumit

>
> Add support for reading this attributes via the disk_partition struct
> and using them to determine which boot partition U-Boot is flashed to
> and generate the appropriate DFU string.
>
> This logic is gated behind a check to ensure that U-Boot is actually
> being chainloaded and not run via some other mechanism.
>
> SCSI support for most Qualcomm platforms is not yet enabled upstream,
> but will follow in future patches.
>
> This series enables capsule updates on the RB2, however [1] is required
> for it to work properly (as otherwise MMC won't be available).
>
> [1]: 
> https://lore.kernel.org/u-boot/20240527-b4-clk-stub-v2-0-29013855e...@linaro.org/
>
> To: Tom Rini 
> To: Simon Glass 
> To: Lukasz Majewski 
> To: Mattijs Korpershoek 
> To: Caleb Connolly 
> To: Neil Armstrong 
> To: Sumit Garg 
> Cc: Ilias Apalodimas 
> Cc: u-boot@lists.denx.de
> Cc: u-boot-q...@groups.io
>
> Changes in v3:
> - Address comments in scsi dfu support
> - enable CONFIG_DFU_SCSI for qcom
> - Link to v2: 
> https://lore.kernel.org/r/20240527-b4-qcom-capsule-updates-v2-0-47583d7ad...@linaro.org
>
> Changes in v2:
> - Add qcom capsule update support patches
> - Link to v1: 
> https://lore.kernel.org/r/20240409-b4-dfu-scsi-v1-0-3e1441a60...@linaro.org
>
> ---
> Caleb Connolly (8):
>   dfu: add scsi backend
>   disk: expose partition type flags
>   mmc: msm_sdhci: work around a bug when writing
>   mach-snapdragon: implement capsule update support
>   qcom_defconfig: savedefconfig
>   mach-snapdragon: use SYSRESET_PSCI
>   mach-snapdragon: bump up heap size
>   qcom_defconfig: enable capsule update support
>
>  arch/arm/Kconfig  |   2 +
>  arch/arm/mach-snapdragon/Kconfig  |   3 +
>  arch/arm/mach-snapdragon/Makefile |   1 +
>  arch/arm/mach-snapdragon/board.c  |   8 +-
>  arch/arm/mach-snapdragon/capsule_update.c | 147 ++
>  arch/arm/mach-snapdragon/qcom-priv.h  |   6 +
>  configs/qcom_defconfig|  19 +-
>  disk/part_efi.c   |   1 +
>  doc/usage/dfu.rst |  32 +++
>  drivers/dfu/Kconfig   |   7 +
>  drivers/dfu/Makefile  |   1 +
>  drivers/dfu/dfu.c |   5 +-
>  drivers/dfu/dfu_scsi.c| 435 
> ++
>  drivers/mmc/msm_sdhci.c   |   7 +
>  include/configs/qcom.h|   5 +
>  include/dfu.h |  26 ++
>  include/part.h|   1 +
>  17 files changed, 695 insertions(+), 11 deletions(-)
> ---
> change-id: 20240523-b4-qcom-capsule-updates-ea2e4f8f0ff0
> base-commit: 5d8881a0801241d68701e8644d495f1d535506f0
>
> // Caleb (they/them)
>

Re: [PATCH v3 4/8] mach-snapdragon: implement capsule update support

2024-06-04 Thread Sumit Garg

Hi Caleb,

On Mon, 3 Jun 2024 at 18:19, Caleb Connolly  wrote:
>
> Qualcomm boards flash U-Boot to the boot partition, implement support
> for determining which slot U-Boot is running from and finding the
> correct boot partition for that slot and configuring the appropriate DFU
> string.
>
> For now this only supports boards with SCSI/UFS storage where U-Boot is
> flashed to the boot partition, and only U-Boot itself is updated. In the
> future we may also support updating additional firmware components (tz,
> hyp, xbl) as well as having U-Boot installed to other partitions (e.g.
> as a first-stage bootloader).
>
> Signed-off-by: Caleb Connolly 
> ---
>  arch/arm/mach-snapdragon/Makefile |   1 +
>  arch/arm/mach-snapdragon/board.c  |   3 +
>  arch/arm/mach-snapdragon/capsule_update.c | 147 
> ++
>  arch/arm/mach-snapdragon/qcom-priv.h  |   6 ++
>  include/configs/qcom.h|   5 +
>  5 files changed, 162 insertions(+)
>
> diff --git a/arch/arm/mach-snapdragon/Makefile 
> b/arch/arm/mach-snapdragon/Makefile
> index 7a4495c8108f..343e825c6fdd 100644
> --- a/arch/arm/mach-snapdragon/Makefile
> +++ b/arch/arm/mach-snapdragon/Makefile
> @@ -2,5 +2,6 @@
>  #
>  # (C) Copyright 2015 Mateusz Kulikowski 
>
>  obj-y += board.o
> +obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += capsule_update.o
>  obj-$(CONFIG_OF_LIVE) += of_fixup.o
> diff --git a/arch/arm/mach-snapdragon/board.c 
> b/arch/arm/mach-snapdragon/board.c
> index b439a19ec7eb..c4a3394706e6 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -299,8 +299,11 @@ int board_late_init(void)
>
> configure_env();
> qcom_late_init();
>
> +   /* Configure the dfu_string for capsule updates */
> +   qcom_configure_capsule_updates();
> +
> return 0;
>  }
>
>  static void build_mem_map(void)
> diff --git a/arch/arm/mach-snapdragon/capsule_update.c 
> b/arch/arm/mach-snapdragon/capsule_update.c
> new file mode 100644
> index ..505f5bf5ae07
> --- /dev/null
> +++ b/arch/arm/mach-snapdragon/capsule_update.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Common initialisation for Qualcomm Snapdragon boards.
> + *
> + * Copyright (c) 2024 Linaro Ltd.
> + * Author: Caleb Connolly 
> + */
> +
> +#define pr_fmt(fmt) "QCOM-FMP: " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qcom-priv.h"
> +
> +struct efi_fw_image fw_images[] = {
> +   {
> +   .image_type_id = QUALCOMM_UBOOT_BOOT_IMAGE_GUID,
> +   .fw_name = u"QUALCOMM-UBOOT",
> +   .image_index = 1,
> +   },
> +};
> +
> +struct efi_capsule_update_info update_info = {
> +   /* Filled in by configure_dfu_string() */
> +   .dfu_string = NULL,
> +   .num_images = ARRAY_SIZE(fw_images),
> +   .images = fw_images,
> +};
> +
> +/* LSB first */
> +struct part_slot_status {
> +   u16: 2;
> +   u16 active : 1;
> +   u16: 3;
> +   u16 successful : 1;
> +   u16 unbootable : 1;
> +   u16 tries_remaining : 4;
> +};
> +
> +static int find_boot_partition(const char *partname, struct blk_desc 
> *blk_dev, char *name)
> +{
> +   int ret;
> +   int partnum;
> +   struct disk_partition info;
> +   struct part_slot_status *slot_status;
> +
> +   for (partnum = 1;; partnum++) {
> +   ret = part_get_info(blk_dev, partnum, &info);
> +   if (ret)
> +   return ret;
> +
> +   slot_status = (struct part_slot_status *)&info.type_flags;
> +   log_io("%16s: Active: %1d, Successful: %1d, Unbootable: %1d, 
> Tries left: %1d\n",
> +  info.name, slot_status->active,
> +  slot_status->successful, slot_status->unbootable,
> +  slot_status->tries_remaining);
> +   if (!strncmp(info.name, partname, strlen(partname)) && 
> slot_status->active) {
> +   log_debug("Found active %s partition: '%s'!\n", 
> partname, info.name);
> +   strlcpy(name, info.name, sizeof(info.name));
> +   return partnum;
> +   }
> +   }
> +
> +   return -1;
> +}
> +
> +/**
> + * qcom_configure_capsule_updates() - Configure the DFU string for capsule 
> updates
> + *
> + * U-Boot is flashed to the boot partition on Qualcomm boards. In most cases 
> there
> + * are two boot partitions, boot_a and boot_b. As we don't currently support 
> doing
> + * full A/B updates, we only support updating the currently active boot 
> partition.
> + *
> + * So we need to find the current slot suffix and the associated boot 
> partition.
> + * We do this by looking for the boot partition that has the 'active' flag 
> set
> + * in the GPT partition vendor attribute bits.
> + */
> +void qcom_configure_capsule_updates(void)
> +{
> +   struct blk

Re: [PATCH 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL

2024-06-04 Thread Heinrich Schuchardt


On 6/5/24 05:25, Simon Glass wrote:

It does not make sense to enable all SHA algorithms unless they are
needed. It bloats the code and in this case, causes chromebook_link to
fail to build.


Why would chromebook_link fail to build?
Is TPM used by U-Boot on that board at all?



Add a condition to TPM to correct this. Note that the original commit
combines refactoring and new features, which makes it hard to see what
is going on.

Fixes: 97707f12fda tpm: Support boot measurements

Signed-off-by: Simon Glass 
---

  lib/Kconfig | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa..70b32362ada 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -438,10 +438,10 @@ config TPM
bool "Trusted Platform Module (TPM) Support"
depends on DM
imply DM_RNG
-   select SHA1
-   select SHA256
-   select SHA384
-   select SHA512
+   select SHA1 if EFI_TCG2_PROTOCOL
+   select SHA256 if EFI_TCG2_PROTOCOL
+   select SHA384 if EFI_TCG2_PROTOCOL
+   select SHA512 if EFI_TCG2_PROTOCOL


You need to consider CONFIG_MEASURED_BOOT which allows measured boot in
the non-UEFI case.

Please, take into account

lib/tpm-v1.c:20:
#error "TPM_AUTH_SESSIONS require SHA1 to be configured, too"

This #error should be replaced by a Kconfig constraint.

I would prefer the select statements to be in lib/efi_loader/Kconfig
under EFI_TCG2_PROTOCOL.

@Ilias, Eddie:

Why do we require SHA1 which is considered insecure?

Shouldn't we change tpm2_supported_algorithms[] to include only
algorithms selected in the configuration? This would allow replacing all
the select statements in Simon's patch by imply.

Best regards

Heinrich


help
  This enables support for TPMs which can be used to provide security
  features for your board. The TPM can be connected via LPC or I2C

Re: [PATCH] ufs: core: remove link_startup_again logic

2024-06-04 Thread Neha Malcom Francis


Hi Neil,

On 28/05/24 14:06, Neil Armstrong wrote:

The link_startup_again logic was added in Linux to handle device
that were set in LinkDown state, which should not be the case since U-boot
doesn't set LinkDown state are init, and Linux sets the device active


s/are/at ?


in ufshcd_init() for the first link startup.

While it worked to far, it breaks link startup for Qualcomm Controllers v5,
let's just remove the logic.



I don't see any repercussions ATM, but will need to get a wider opinion with 
different controllers before acking.



Signed-off-by: Neil Armstrong 
---
  drivers/ufs/ufs.c | 8 
  1 file changed, 8 deletions(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index be64bf971f1..28c244b70e5 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -456,9 +456,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
  {
int ret;
int retries = DME_LINKSTARTUP_RETRIES;
-   bool link_startup_again = true;
  
-link_startup:

do {
ufshcd_ops_link_startup_notify(hba, PRE_CHANGE);
  
@@ -484,12 +482,6 @@ link_startup:

/* failed to get the link up... retire */
goto out;
  
-	if (link_startup_again) {

-   link_startup_again = false;
-   retries = DME_LINKSTARTUP_RETRIES;
-   goto link_startup;
-   }
-
/* Mark that link is up in PWM-G1, 1-lane, SLOW-AUTO mode */
ufshcd_init_pwr_info(hba);
  


---
base-commit: 7e52d6ccfb76e2afc2d183b357abe2a2e2f948cf
change-id: 20240528-topic-sm8x50-ufs-core-link-startup-again-bc2cf907c164

Best regards,


--
Thanking You
Neha Malcom Francis

1 2 >

1 - 100 of 106 matches

Mail list logo