* [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform
@ 2020-11-26 18:40 Sughosh Ganu
2020-11-26 18:40 ` [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined Sughosh Ganu
` (13 more replies)
0 siblings, 14 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:40 UTC (permalink / raw)
To: u-boot
The following series adds support for the uefi capsule update feature
on the qemu arm64 platform, along with adding support for the capsule
authentication feature.
The capsule update feature is supported on a platform configuration
booting in a non-secure mode, i.e with -machine virt,secure=off option
set. This results in the platform booting u-boot directly without
the presence of trusted firmware(tf-a). Steps that need to be followed
for using this feature have been provided as part of the documentation.
Support has also been added for enabling the capsule authentication
feature. Capsule authentication, as defined by the uefi
specification is very much on similar lines to the logic used for
variable authentication. As a result, most of the signature
verification code already in use for variable authentication has been
used for capsule authentication.
Storage of the public key certificate, needed for the signature
verification process is in form of the efi signature list(esl)
structure. This public key is stored on the platform's device tree
blob. The public key esl file can be embedded into the dtb using the
mkeficapsule utility that has been added as part of the capsule update
support series[1]. Steps needed for enabling capsule authentication
have been provided as part of the documentation.
This patch series needs to be applied on top of the capsule update
support patch series from Takahiro Akashi[1]
[1] -
https://patchwork.ozlabs.org/project/uboot/cover/20201117002805.13902-1-takahiro.akashi at linaro.org/
Sughosh Ganu (14):
qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
mkeficapsule: Add support for embedding public key in a dtb
qemu: arm: Scan the pci bus in board_init
crypto: Fix the logic to calculate hash with authattributes set
qemu: arm64: Add support for dynamic mtdparts for the platform
qemu: arm64: Set dfu_alt_info variable for the platform
efi_loader: Add config option to indicate fmp header presence
dfu_mtd: Add provision to unlock mtd device
efi_loader: Make the pkcs7 header parsing function an extern
efi_loader: Re-factor code to build the signature store from efi
signature list
efi: capsule: Add support for uefi capsule authentication
efi_loader: Enable uefi capsule authentication
efidebug: capsule: Add a command to update capsule on disk
qemu: arm64: Add documentation for capsule update
board/emulation/qemu-arm/qemu-arm.c | 170 ++++++++++++++++++++++++
cmd/efidebug.c | 14 ++
doc/board/emulation/qemu-arm.rst | 157 ++++++++++++++++++++++
drivers/dfu/dfu_mtd.c | 20 ++-
include/configs/qemu-arm.h | 8 ++
include/efi_api.h | 18 +++
include/efi_loader.h | 12 ++
lib/crypto/pkcs7_verify.c | 37 ++++--
lib/efi_loader/Kconfig | 24 ++++
lib/efi_loader/efi_capsule.c | 122 +++++++++++++++++
lib/efi_loader/efi_firmware.c | 49 ++++++-
lib/efi_loader/efi_signature.c | 192 ++++++++++++++++++++-------
lib/efi_loader/efi_variable.c | 93 +------------
tools/Makefile | 1 +
tools/mkeficapsule.c | 198 ++++++++++++++++++++++++++--
15 files changed, 954 insertions(+), 161 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
@ 2020-11-26 18:40 ` Sughosh Ganu
2020-12-05 9:31 ` Heinrich Schuchardt
2020-11-26 18:40 ` [PATCH 02/14] mkeficapsule: Add support for embedding public key in a dtb Sughosh Ganu
` (12 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:40 UTC (permalink / raw)
To: u-boot
The Qemu platform emulator generates a device tree blob and places it
at the start of the dram, which is then used by u-boot. Use this dtb
only if CONFIG_OF_BOARD is defined. This allows using a different
device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
to the u-boot binary as a u-boot-fdt.bin file
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
board/emulation/qemu-arm/qemu-arm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index f18f2ed7da..e146d1cc50 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -89,11 +89,13 @@ int dram_init_banksize(void)
return 0;
}
+#if defined(CONFIG_OF_BOARD)
void *board_fdt_blob_setup(void)
{
/* QEMU loads a generated DTB for us at the start of RAM. */
return (void *)CONFIG_SYS_SDRAM_BASE;
}
+#endif
void enable_caches(void)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 02/14] mkeficapsule: Add support for embedding public key in a dtb
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
2020-11-26 18:40 ` [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined Sughosh Ganu
@ 2020-11-26 18:40 ` Sughosh Ganu
2020-11-26 18:40 ` [PATCH 03/14] qemu: arm: Scan the pci bus in board_init Sughosh Ganu
` (11 subsequent siblings)
13 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:40 UTC (permalink / raw)
To: u-boot
Add options for embedding the public key esl(efi signature list) file
to the platform's dtb. The esl file is then retrieved and used for
authenticating the capsule to be used for updating firmare components
on the platform.
The esl file can now be embedded in the dtb by invoking the following
command
mkeficapsule -K <pub_key.esl> -D <dtb>
This will create a node named 'signature' in the dtb, and the esl file
will be stored as 'capsule-key'
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
tools/Makefile | 1 +
tools/mkeficapsule.c | 198 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 186 insertions(+), 13 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile
index 66d9376803..6d7b48fb57 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -218,6 +218,7 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler
HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
+mkeficapsule-objs := mkeficapsule.o $(LIBFDT_OBJS)
hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
# We build some files with extra pedantic flags to try to minimize things
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 45e27d74a5..ce05da12d6 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -4,16 +4,22 @@
* Author: AKASHI Takahiro
*/
+#include <errno.h>
#include <getopt.h>
#include <malloc.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
#include <linux/types.h>
+
+#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
+#include "fdt_host.h"
+
typedef __u8 u8;
typedef __u16 u16;
typedef __u32 u32;
@@ -23,6 +29,8 @@ typedef __s32 s32;
#define aligned_u64 __aligned_u64
+#define SIGNATURE_NODENAME "signature"
+
#ifndef __packed
#define __packed __attribute__((packed))
#endif
@@ -43,6 +51,8 @@ static struct option options[] = {
{"raw", required_argument, NULL, 'r'},
{"index", required_argument, NULL, 'i'},
{"instance", required_argument, NULL, 'I'},
+ {"dtb", required_argument, NULL, 'D'},
+ {"public key", required_argument, NULL, 'K'},
{"version", required_argument, NULL, 'v'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0},
@@ -52,15 +62,154 @@ static void print_usage(void)
{
printf("Usage: %s [options] <output file>\n"
"Options:\n"
- "\t--fit <fit image> new FIT image file\n"
- "\t--raw <raw image> new raw image file\n"
- "\t--index <index> update image index\n"
- "\t--instance <instance> update hardware instance\n"
- "\t--version <version> firmware version\n"
- "\t--help print a help message\n",
+ "\t--fit <fit image> new FIT image file\n"
+ "\t--raw <raw image> new raw image file\n"
+ "\t--index <index> update image index\n"
+ "\t--instance <instance> update hardware instance\n"
+ "\t--version <version> firmware version\n"
+ "\t--public-key <key file> public key esl file\n"
+ "\t--dtb <dtb file> dtb file\n"
+ "\t--help print a help message\n",
tool_name);
}
+static int fdt_add_pub_key_data(void *sptr, void *dptr, size_t key_size)
+{
+ int parent;
+ int ret = 0;
+
+ parent = fdt_subnode_offset(dptr, 0, SIGNATURE_NODENAME);
+ if (parent == -FDT_ERR_NOTFOUND) {
+ parent = fdt_add_subnode(dptr, 0, SIGNATURE_NODENAME);
+ if (parent < 0) {
+ ret = parent;
+ if (ret != -FDT_ERR_NOSPACE) {
+ fprintf(stderr,
+ "Couldn't create signature node: %s\n",
+ fdt_strerror(parent));
+ }
+ }
+ }
+ if (ret)
+ goto done;
+
+ /* Write the key to the FDT node */
+ ret = fdt_setprop(dptr, parent, "capsule-key",
+ sptr, key_size);
+
+done:
+ if (ret)
+ ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
+
+ return ret;
+}
+
+static int add_public_key(const char *pkey_file, const char *dtb_file)
+{
+ int ret;
+ int srcfd = 0;
+ int destfd = 0;
+ void *sptr = NULL;
+ void *dptr = NULL;
+ off_t src_size;
+ struct stat pub_key;
+ struct stat dtb;
+
+ /* Find out the size of the public key */
+ srcfd = open(pkey_file, O_RDONLY);
+ if (srcfd == -1) {
+ fprintf(stderr, "%s: Can't open %s: %s\n",
+ __func__, pkey_file, strerror(errno));
+ goto err;
+ }
+
+ ret = fstat(srcfd, &pub_key);
+ if (ret == -1) {
+ fprintf(stderr, "%s: Can't stat %s: %s\n",
+ __func__, pkey_file, strerror(errno));
+ goto err;
+ }
+
+ src_size = pub_key.st_size;
+
+ /* mmap the public key esl file */
+ sptr = mmap(0, src_size, PROT_READ, MAP_SHARED, srcfd, 0);
+ if ((sptr == MAP_FAILED) || (errno != 0)) {
+ fprintf(stderr, "%s: Failed to mmap %s:%s\n",
+ __func__, pkey_file, strerror(errno));
+ goto err;
+ }
+
+ /* Open the dest FDT */
+ destfd = open(dtb_file, O_RDWR);
+ if (destfd == -1) {
+ fprintf(stderr, "%s: Can't open %s: %s\n",
+ __func__, dtb_file, strerror(errno));
+ goto err;
+ }
+
+ ret = fstat(destfd, &dtb);
+ if (ret == -1) {
+ fprintf(stderr, "%s: Can't stat %s: %s\n",
+ __func__, dtb_file, strerror(errno));
+ goto err;
+ }
+
+ dtb.st_size += src_size;
+ if (ftruncate(destfd, dtb.st_size)) {
+ fprintf(stderr, "%s: Can't expand %s: %s\n",
+ __func__, dtb_file, strerror(errno));
+ goto err;;
+ }
+
+ errno = 0;
+ /* mmap the dtb file */
+ dptr = mmap(0, dtb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED,
+ destfd, 0);
+ if ((dptr == MAP_FAILED) || (errno != 0)) {
+ fprintf(stderr, "%s: Failed to mmap %s:%s\n",
+ __func__, dtb_file, strerror(errno));
+ goto err;
+ }
+
+ if (fdt_check_header(dptr)) {
+ fprintf(stderr, "%s: Invalid FDT header\n", __func__);
+ goto err;
+ }
+
+ ret = fdt_open_into(dptr, dptr, dtb.st_size);
+ if (ret) {
+ fprintf(stderr, "%s: Cannot expand FDT: %s\n",
+ __func__, fdt_strerror(ret));
+ goto err;
+ }
+
+ /* Copy the esl file to the expanded FDT */
+ ret = fdt_add_pub_key_data(sptr, dptr, src_size);
+ if (ret < 0) {
+ fprintf(stderr, "%s: Unable to add public key to the FDT\n",
+ __func__);
+ goto err;
+ }
+
+ return 0;
+
+err:
+ if (sptr)
+ munmap(sptr, src_size);
+
+ if (dptr)
+ munmap(dptr, dtb.st_size);
+
+ if (srcfd >= 0)
+ close(srcfd);
+
+ if (destfd >= 0)
+ close(destfd);
+
+ return -1;
+}
+
static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
unsigned long version, unsigned long index,
unsigned long instance)
@@ -171,17 +320,22 @@ err_1:
int main(int argc, char **argv)
{
char *file;
+ char *pkey_file;
+ char *dtb_file;
efi_guid_t *guid;
unsigned long index, instance, version;
int c, idx;
+ int ret;
file = NULL;
+ pkey_file = NULL;
+ dtb_file = NULL;
guid = NULL;
index = 0;
instance = 0;
version = 0;
for (;;) {
- c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
+ c = getopt_long(argc, argv, "f:r:i:I:v:D:K:h", options, &idx);
if (c == -1)
break;
@@ -211,22 +365,40 @@ int main(int argc, char **argv)
case 'v':
version = strtoul(optarg, NULL, 0);
break;
+ case 'K':
+ if (pkey_file) {
+ printf("Public Key already specified\n");
+ return -1;
+ }
+ pkey_file = optarg;
+ break;
+ case 'D':
+ if (dtb_file) {
+ printf("DTB file already specified\n");
+ return -1;
+ }
+ dtb_file = optarg;
+ break;
case 'h':
print_usage();
return 0;
}
}
- /* need a output file */
- if (argc != optind + 1) {
+ /* need a fit image file or raw image file */
+ if (!file && !pkey_file && !dtb_file) {
print_usage();
return -1;
}
- /* need a fit image file or raw image file */
- if (!file) {
- print_usage();
- return -1;
+ if (pkey_file && dtb_file) {
+ ret = add_public_key(pkey_file, dtb_file);
+ if (ret == -1) {
+ printf("Adding public key to the dtb failed\n");
+ return -1;
+ } else {
+ return 0;
+ }
}
if (create_fwbin(argv[optind], file, guid, version, index, instance)
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 03/14] qemu: arm: Scan the pci bus in board_init
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
2020-11-26 18:40 ` [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined Sughosh Ganu
2020-11-26 18:40 ` [PATCH 02/14] mkeficapsule: Add support for embedding public key in a dtb Sughosh Ganu
@ 2020-11-26 18:40 ` Sughosh Ganu
2020-12-05 9:45 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set Sughosh Ganu
` (10 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:40 UTC (permalink / raw)
To: u-boot
Scan the pci bus in board_init routine before scanning the virtio
devices. This enumerates all the virtio devices, including devices
found on the pci bus.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
board/emulation/qemu-arm/qemu-arm.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index e146d1cc50..b3d5b3d5c2 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -65,6 +65,14 @@ struct mm_region *mem_map = qemu_arm64_mem_map;
int board_init(void)
{
+
+ /*
+ * Scan the pci bus before calling virtio_init. This
+ * enumerates all virtio devices, including devices
+ * on the pci bus.
+ */
+ pci_init();
+
/*
* Make sure virtio bus is enumerated so that peripherals
* on the virtio bus can be discovered by their drivers
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (2 preceding siblings ...)
2020-11-26 18:40 ` [PATCH 03/14] qemu: arm: Scan the pci bus in board_init Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:21 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform Sughosh Ganu
` (9 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
RFC 2315 Section 9.3 describes the message digesting process. The
digest calculated depends on whether the authenticated attributes are
present. In case of a scenario where the authenticated attributes are
present, the message digest that gets signed and is part of the pkcs7
message is computed from the auth attributes rather than the contents
field.
Check if the auth attributes are present, and if set, use the auth
attributes to compute the hash that would be compared with the
encrypted hash on the pkcs7 message.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
lib/crypto/pkcs7_verify.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
index 320ba49f79..58683ef614 100644
--- a/lib/crypto/pkcs7_verify.c
+++ b/lib/crypto/pkcs7_verify.c
@@ -50,8 +50,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
struct image_region regions[2];
int ret = 0;
- /* The digest was calculated already. */
- if (sig->digest)
+ /*
+ * [RFC2315 9.3]
+ * If the authenticated attributes are present,
+ * the message-digest is calculated on the
+ * attributes present in the
+ * authenticatedAttributes field and not just
+ * the contents field
+ */
+ if (!sinfo->authattrs && sig->digest)
return 0;
if (!sinfo->sig->hash_algo)
@@ -63,17 +70,25 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
else
return -ENOPKG;
- sig->digest = calloc(1, sig->digest_size);
- if (!sig->digest) {
- pr_warn("Sig %u: Out of memory\n", sinfo->index);
- return -ENOMEM;
- }
+ /*
+ * Calculate the hash only if the data is present.
+ * In case of authenticated variable and capsule,
+ * the hash has already been calculated on the
+ * efi_image_regions and populated
+ */
+ if (pkcs7->data) {
+ sig->digest = calloc(1, sig->digest_size);
+ if (!sig->digest) {
+ pr_warn("Sig %u: Out of memory\n", sinfo->index);
+ return -ENOMEM;
+ }
- regions[0].data = pkcs7->data;
- regions[0].size = pkcs7->data_len;
+ regions[0].data = pkcs7->data;
+ regions[0].size = pkcs7->data_len;
- /* Digest the message [RFC2315 9.3] */
- hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
+ /* Digest the message [RFC2315 9.3] */
+ hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
+ }
/* However, if there are authenticated attributes, there must be a
* message digest attribute amongst them which corresponds to the
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (3 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:29 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable " Sughosh Ganu
` (8 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Add support for setting the default values for mtd partitions on the
platform for the nor flash. This would be used for updating the
firmware image using uefi capsule update with the dfu mtd backend
driver.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
board/emulation/qemu-arm/qemu-arm.c | 70 +++++++++++++++++++++++++++++
include/configs/qemu-arm.h | 7 +++
2 files changed, 77 insertions(+)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index b3d5b3d5c2..d5ed3eebaf 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -197,3 +197,73 @@ void flash_write32(u32 value, void *addr)
{
asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
}
+
+#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
+
+#include <mtd.h>
+
+static void board_get_mtdparts(const char *dev, const char *partition,
+ char *mtdids, char *mtdparts)
+{
+ /* mtdids: "<dev>=<dev>, ...." */
+ if (mtdids[0] != '\0')
+ strcat(mtdids, ",");
+ strcat(mtdids, dev);
+ strcat(mtdids, "=");
+ strcat(mtdids, dev);
+
+ /* mtdparts: "mtdparts=<dev>:<mtdparts_<dev>>;..." */
+ if (mtdparts[0] != '\0')
+ strncat(mtdparts, ";", MTDPARTS_LEN);
+ else
+ strcat(mtdparts, "mtdparts=");
+
+ strncat(mtdparts, dev, MTDPARTS_LEN);
+ strncat(mtdparts, ":", MTDPARTS_LEN);
+ strncat(mtdparts, partition, MTDPARTS_LEN);
+}
+
+void board_mtdparts_default(const char **mtdids, const char **mtdparts)
+{
+ struct mtd_info *mtd;
+ struct udevice *dev;
+ const char *mtd_partition;
+ static char parts[3 * MTDPARTS_LEN + 1];
+ static char ids[MTDIDS_LEN + 1];
+ static bool mtd_initialized;
+
+ if (mtd_initialized) {
+ *mtdids = ids;
+ *mtdparts = parts;
+ return;
+ }
+
+ memset(parts, 0, sizeof(parts));
+ memset(ids, 0, sizeof(ids));
+
+ /* probe all MTD devices */
+ for (uclass_first_device(UCLASS_MTD, &dev); dev;
+ uclass_next_device(&dev)) {
+ debug("mtd device = %s\n", dev->name);
+ }
+
+ mtd = get_mtd_device_nm("nor0");
+ if (!IS_ERR_OR_NULL(mtd)) {
+ mtd_partition = MTDPARTS_NOR0;
+ board_get_mtdparts("nor0", mtd_partition, ids, parts);
+ put_mtd_device(mtd);
+ }
+
+ mtd = get_mtd_device_nm("nor1");
+ if (!IS_ERR_OR_NULL(mtd)) {
+ mtd_partition = MTDPARTS_NOR1;
+ board_get_mtdparts("nor1", mtd_partition, ids, parts);
+ put_mtd_device(mtd);
+ }
+
+ mtd_initialized = true;
+ *mtdids = ids;
+ *mtdparts = parts;
+ debug("%s:mtdids=%s & mtdparts=%s\n", __func__, ids, parts);
+}
+#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */
diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index 273fa1a7d7..69ff329434 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -32,6 +32,13 @@
#include <config_distro_bootcmd.h>
+#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
+#define CONFIG_SYS_MTDPARTS_RUNTIME
+#endif
+
+#define MTDPARTS_NOR0 "64m(u-boot)\0"
+#define MTDPARTS_NOR1 "64m(u-boot-env)\0"
+
#define CONFIG_EXTRA_ENV_SETTINGS \
"fdt_high=0xffffffff\0" \
"initrd_high=0xffffffff\0" \
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (4 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:31 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence Sughosh Ganu
` (7 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
The dfu framework uses the dfu_alt_info environment variable to get
information that is needed for performing the firmware update. Set the
dfu_alt_info for the platform to reflect the two mtd partitions
created for the u-boot env and the firmware image.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
board/emulation/qemu-arm/qemu-arm.c | 55 +++++++++++++++++++++++++++++
include/configs/qemu-arm.h | 1 +
2 files changed, 56 insertions(+)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index d5ed3eebaf..8cad54c76f 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
+#include <memalign.h>
#include <mtd.h>
+#define MTDPARTS_LEN 256
+#define MTDIDS_LEN 128
+
+#define DFU_ALT_BUF_LEN SZ_1K
+
+static void board_get_alt_info(struct mtd_info *mtd, char *buf)
+{
+ struct mtd_info *part;
+ bool first = true;
+ const char *name;
+ int len, partnum = 0;
+
+ name = mtd->name;
+ len = strlen(buf);
+
+ if (buf[0] != '\0')
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
+ "mtd %s=", name);
+
+ list_for_each_entry(part, &mtd->partitions, node) {
+ partnum++;
+ if (!first)
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, ";");
+ first = false;
+
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
+ "%s part %d",
+ part->name, partnum);
+ }
+}
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+ struct mtd_info *mtd;
+
+ ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
+
+ if (env_get("dfu_alt_info"))
+ return;
+
+ memset(buf, 0, sizeof(buf));
+
+ /* probe all MTD devices */
+ mtd_probe_devices();
+
+ mtd = get_mtd_device_nm("nor0");
+ if (!IS_ERR_OR_NULL(mtd))
+ board_get_alt_info(mtd, buf);
+
+ env_set("dfu_alt_info", buf);
+ printf("dfu_alt_info set\n");
+}
+
static void board_get_mtdparts(const char *dev, const char *partition,
char *mtdids, char *mtdparts)
{
diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index 69ff329434..726f985d35 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -33,6 +33,7 @@
#include <config_distro_bootcmd.h>
#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
+#define CONFIG_SET_DFU_ALT_INFO
#define CONFIG_SYS_MTDPARTS_RUNTIME
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (5 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable " Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:34 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 08/14] dfu_mtd: Add provision to unlock mtd device Sughosh Ganu
` (6 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
When building the capsule using scripts in edk2, an fmp header is
added on top of the binary payload. Add a config option to indicate
the presence of the header. When enabled, the pointer to the image
needs to be adjusted as per the size of the header to point to the
actual binary payload.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
lib/efi_loader/Kconfig | 7 +++++++
lib/efi_loader/efi_firmware.c | 12 ++++++++++++
2 files changed, 19 insertions(+)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 0d1b1b5356..55e4787e32 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -138,6 +138,13 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT
Select this option if you want to enable capsule-based
firmware update using Firmware Management Protocol.
+config EFI_CAPSULE_FMP_HEADER
+ bool "Capsule uses FMP header"
+ depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+ help
+ Select this option if the capsule is built using the
+ scripts in edk2.
+
config EFI_CAPSULE_FIRMWARE_FIT
bool "FMP driver for FIT image"
depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 7e56077383..6c97604d8b 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -385,10 +385,22 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
if (!image)
return EFI_EXIT(EFI_INVALID_PARAMETER);
+ if (CONFIG_IS_ENABLED(EFI_CAPSULE_FMP_HEADER)) {
+ /*
+ * When building the capsule with the scripts in
+ * edk2, a FMP header is inserted above the capsule
+ * payload. Compensate for this header to get the
+ * actual payload that is to be updated.
+ */
+ image += 0x10;
+ image_size -= 0x10;
+ }
+
if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
NULL, NULL))
return EFI_EXIT(EFI_DEVICE_ERROR);
+ printf("%s: Capsule update complete!\n", __func__);
return EFI_EXIT(EFI_SUCCESS);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 08/14] dfu_mtd: Add provision to unlock mtd device
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (6 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern Sughosh Ganu
` (5 subsequent siblings)
13 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Prior to writing to an mtd device, mtd_erase is called. This call
fails in case the sector being erased is locked. Call mtd_unlock to
unlock the region which is to be erased and later written to. Lock the
region once the write to the region has completed.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
drivers/dfu/dfu_mtd.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
index 36cd4e945b..b34975dbb0 100644
--- a/drivers/dfu/dfu_mtd.c
+++ b/drivers/dfu/dfu_mtd.c
@@ -21,7 +21,7 @@ static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu,
u64 offset, void *buf, long *len)
{
- u64 off, lim, remaining;
+ u64 off, lim, remaining, lock_ofs, lock_len;
struct mtd_info *mtd = dfu->data.mtd.info;
struct mtd_oob_ops io_op = {};
int ret = 0;
@@ -34,7 +34,7 @@ static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu,
return 0;
}
- off = dfu->data.mtd.start + offset + dfu->bad_skip;
+ off = lock_ofs = dfu->data.mtd.start + offset + dfu->bad_skip;
lim = dfu->data.mtd.start + dfu->data.mtd.size;
if (off >= lim) {
@@ -56,12 +56,19 @@ static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu,
if (op == DFU_OP_WRITE) {
struct erase_info erase_op = {};
- remaining = round_up(*len, mtd->erasesize);
+ remaining = lock_len = round_up(*len, mtd->erasesize);
erase_op.mtd = mtd;
erase_op.addr = off;
erase_op.len = mtd->erasesize;
erase_op.scrub = 0;
+ debug("Unlocking the mtd device\n");
+ ret = mtd_unlock(mtd, lock_ofs, lock_len);
+ if (ret && ret != -EOPNOTSUPP) {
+ printf("MTD device unlock failed\n");
+ return 0;
+ }
+
while (remaining) {
if (erase_op.addr + remaining > lim) {
printf("Limit reached 0x%llx while erasing at offset 0x%llx\n",
@@ -139,6 +146,13 @@ static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu,
io_op.len = mtd->writesize;
}
+ if (op == DFU_OP_WRITE) {
+ /* Write done, lock again */
+ debug("Locking the mtd device\n");
+ ret = mtd_lock(mtd, lock_ofs, lock_len);
+ if (ret && ret != -EOPNOTSUPP)
+ printf("MTD device lock failed\n");
+ }
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (7 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 08/14] dfu_mtd: Add provision to unlock mtd device Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:40 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 10/14] efi_loader: Re-factor code to build the signature store from efi signature list Sughosh Ganu
` (4 subsequent siblings)
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
The pkcs7 header parsing functionality is pretty generic, and can be
used by other features like capsule authentication. Make the function
an extern, also changing it's name to efi_parse_pkcs7_header
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
include/efi_loader.h | 4 ++
lib/efi_loader/efi_signature.c | 85 +++++++++++++++++++++++++++++++
lib/efi_loader/efi_variable.c | 93 ++--------------------------------
3 files changed, 93 insertions(+), 89 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 76cd2b36f2..b9226208f5 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -810,6 +810,10 @@ bool efi_secure_boot_enabled(void);
bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
WIN_CERTIFICATE **auth, size_t *auth_len);
+struct pkcs7_message *efi_parse_pkcs7_header(const void *buf,
+ size_t buflen,
+ u8 **tmpbuf);
+
/* runtime implementation of memcpy() */
void efi_memcpy_runtime(void *dest, const void *src, size_t n);
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index 79dee27421..9ab071b611 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -27,6 +27,91 @@ const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;
const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
#ifdef CONFIG_EFI_SECURE_BOOT
+static u8 pkcs7_hdr[] = {
+ /* SEQUENCE */
+ 0x30, 0x82, 0x05, 0xc7,
+ /* OID: pkcs7-signedData */
+ 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
+ /* Context Structured? */
+ 0xa0, 0x82, 0x05, 0xb8,
+};
+
+/**
+ * efi_parse_pkcs7_header - parse a signature in payload
+ * @buf: Pointer to payload's value
+ * @buflen: Length of @buf
+ * @tmpbuf: Pointer to temporary buffer
+ *
+ * Parse a signature embedded in payload's value and instantiate
+ * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
+ * pkcs7's signedData, some header needed be prepended for correctly
+ * parsing authentication data
+ * A temporary buffer will be allocated if needed, and it should be
+ * kept valid during the authentication because some data in the buffer
+ * will be referenced by efi_signature_verify().
+ *
+ * Return: Pointer to pkcs7_message structure on success, NULL on error
+ */
+struct pkcs7_message *efi_parse_pkcs7_header(const void *buf,
+ size_t buflen,
+ u8 **tmpbuf)
+{
+ u8 *ebuf;
+ size_t ebuflen, len;
+ struct pkcs7_message *msg;
+
+ /*
+ * This is the best assumption to check if the binary is
+ * already in a form of pkcs7's signedData.
+ */
+ if (buflen > sizeof(pkcs7_hdr) &&
+ !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
+ msg = pkcs7_parse_message(buf, buflen);
+ if (IS_ERR(msg))
+ return NULL;
+ return msg;
+ }
+
+ /*
+ * Otherwise, we should add a dummy prefix sequence for pkcs7
+ * message parser to be able to process.
+ * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
+ * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
+ * TODO:
+ * The header should be composed in a more refined manner.
+ */
+ EFI_PRINT("Makeshift prefix added to authentication data\n");
+ ebuflen = sizeof(pkcs7_hdr) + buflen;
+ if (ebuflen <= 0x7f) {
+ EFI_PRINT("Data is too short\n");
+ return NULL;
+ }
+
+ ebuf = malloc(ebuflen);
+ if (!ebuf) {
+ EFI_PRINT("Out of memory\n");
+ return NULL;
+ }
+
+ memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
+ memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
+ len = ebuflen - 4;
+ ebuf[2] = (len >> 8) & 0xff;
+ ebuf[3] = len & 0xff;
+ len = ebuflen - 0x13;
+ ebuf[0x11] = (len >> 8) & 0xff;
+ ebuf[0x12] = len & 0xff;
+
+ msg = pkcs7_parse_message(ebuf, ebuflen);
+
+ if (IS_ERR(msg)) {
+ free(ebuf);
+ return NULL;
+ }
+
+ *tmpbuf = ebuf;
+ return msg;
+}
/**
* efi_hash_regions - calculate a hash value
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 0c689cfb47..ba0874e9e7 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -24,91 +24,6 @@
#include <asm/sections.h>
#ifdef CONFIG_EFI_SECURE_BOOT
-static u8 pkcs7_hdr[] = {
- /* SEQUENCE */
- 0x30, 0x82, 0x05, 0xc7,
- /* OID: pkcs7-signedData */
- 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
- /* Context Structured? */
- 0xa0, 0x82, 0x05, 0xb8,
-};
-
-/**
- * efi_variable_parse_signature - parse a signature in variable
- * @buf: Pointer to variable's value
- * @buflen: Length of @buf
- * @tmpbuf: Pointer to temporary buffer
- *
- * Parse a signature embedded in variable's value and instantiate
- * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
- * pkcs7's signedData, some header needed be prepended for correctly
- * parsing authentication data, particularly for variable's.
- * A temporary buffer will be allocated if needed, and it should be
- * kept valid during the authentication because some data in the buffer
- * will be referenced by efi_signature_verify().
- *
- * Return: Pointer to pkcs7_message structure on success, NULL on error
- */
-static struct pkcs7_message *efi_variable_parse_signature(const void *buf,
- size_t buflen,
- u8 **tmpbuf)
-{
- u8 *ebuf;
- size_t ebuflen, len;
- struct pkcs7_message *msg;
-
- /*
- * This is the best assumption to check if the binary is
- * already in a form of pkcs7's signedData.
- */
- if (buflen > sizeof(pkcs7_hdr) &&
- !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
- msg = pkcs7_parse_message(buf, buflen);
- if (IS_ERR(msg))
- return NULL;
- return msg;
- }
-
- /*
- * Otherwise, we should add a dummy prefix sequence for pkcs7
- * message parser to be able to process.
- * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
- * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
- * TODO:
- * The header should be composed in a more refined manner.
- */
- EFI_PRINT("Makeshift prefix added to authentication data\n");
- ebuflen = sizeof(pkcs7_hdr) + buflen;
- if (ebuflen <= 0x7f) {
- EFI_PRINT("Data is too short\n");
- return NULL;
- }
-
- ebuf = malloc(ebuflen);
- if (!ebuf) {
- EFI_PRINT("Out of memory\n");
- return NULL;
- }
-
- memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
- memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
- len = ebuflen - 4;
- ebuf[2] = (len >> 8) & 0xff;
- ebuf[3] = len & 0xff;
- len = ebuflen - 0x13;
- ebuf[0x11] = (len >> 8) & 0xff;
- ebuf[0x12] = len & 0xff;
-
- msg = pkcs7_parse_message(ebuf, ebuflen);
-
- if (IS_ERR(msg)) {
- free(ebuf);
- return NULL;
- }
-
- *tmpbuf = ebuf;
- return msg;
-}
/**
* efi_variable_authenticate - authenticate a variable
@@ -215,10 +130,10 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
goto err;
/* ebuf should be kept valid during the authentication */
- var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
- auth->auth_info.hdr.dwLength
- - sizeof(auth->auth_info),
- &ebuf);
+ var_sig = efi_parse_pkcs7_header(auth->auth_info.cert_data,
+ auth->auth_info.hdr.dwLength
+ - sizeof(auth->auth_info),
+ &ebuf);
if (!var_sig) {
EFI_PRINT("Parsing variable's signature failed\n");
goto err;
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 10/14] efi_loader: Re-factor code to build the signature store from efi signature list
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (8 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 11/14] efi: capsule: Add support for uefi capsule authentication Sughosh Ganu
` (3 subsequent siblings)
13 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
The efi_sigstore_parse_sigdb function reads the uefi authenticated
variable, stored in the signature database format and builds the
signature store structure. Factor out the code for building
the signature store. This can then be used by the capsule
authentication routine to build the signature store even when the
signature database is not stored as an uefi authenticated variable
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
include/efi_loader.h | 2 +
lib/efi_loader/efi_signature.c | 103 +++++++++++++++++++--------------
2 files changed, 63 insertions(+), 42 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index b9226208f5..8d8a6649b5 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -803,6 +803,8 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs,
int nocheck);
void efi_sigstore_free(struct efi_signature_store *sigstore);
+struct efi_signature_store *efi_build_signature_store(void *sig_list,
+ efi_uintn_t size);
struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
bool efi_secure_boot_enabled(void);
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index 9ab071b611..87525bdc80 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -736,6 +736,63 @@ err:
return NULL;
}
+/**
+ * efi_sigstore_parse_sigdb - parse the signature list and populate
+ * the signature store
+ *
+ * @sig_list: Pointer to the signature list
+ * @size: Size of the signature list
+ *
+ * Parse the efi signature list and instantiate a signature store
+ * structure.
+ *
+ * Return: Pointer to signature store on success, NULL on error
+ */
+struct efi_signature_store *efi_build_signature_store(void *sig_list,
+ efi_uintn_t size)
+{
+ struct efi_signature_list *esl;
+ struct efi_signature_store *sigstore = NULL, *siglist;
+
+ esl = sig_list;
+ while (size > 0) {
+ /* List must exist if there is remaining data. */
+ if (size < sizeof(*esl)) {
+ EFI_PRINT("Signature list in wrong format\n");
+ goto err;
+ }
+
+ if (size < esl->signature_list_size) {
+ EFI_PRINT("Signature list in wrong format\n");
+ goto err;
+ }
+
+ /* Parse a single siglist. */
+ siglist = efi_sigstore_parse_siglist(esl);
+ if (!siglist) {
+ EFI_PRINT("Parsing of signature list of failed\n");
+ goto err;
+ }
+
+ /* Append siglist */
+ siglist->next = sigstore;
+ sigstore = siglist;
+
+ /* Next */
+ size -= esl->signature_list_size;
+ esl = (void *)esl + esl->signature_list_size;
+ }
+ free(sig_list);
+
+ return sigstore;
+
+err:
+ efi_sigstore_free(sigstore);
+ free(sig_list);
+
+ return NULL;
+}
+
/**
* efi_sigstore_parse_sigdb - parse a signature database variable
* @name: Variable's name
@@ -747,8 +804,7 @@ err:
*/
struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name)
{
- struct efi_signature_store *sigstore = NULL, *siglist;
- struct efi_signature_list *esl;
+ struct efi_signature_store *sigstore = NULL;
const efi_guid_t *vendor;
void *db;
efi_uintn_t db_size;
@@ -784,47 +840,10 @@ struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name)
ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, db));
if (ret != EFI_SUCCESS) {
EFI_PRINT("Getting variable, %ls, failed\n", name);
- goto err;
- }
-
- /* Parse siglist list */
- esl = db;
- while (db_size > 0) {
- /* List must exist if there is remaining data. */
- if (db_size < sizeof(*esl)) {
- EFI_PRINT("variable, %ls, in wrong format\n", name);
- goto err;
- }
-
- if (db_size < esl->signature_list_size) {
- EFI_PRINT("variable, %ls, in wrong format\n", name);
- goto err;
- }
-
- /* Parse a single siglist. */
- siglist = efi_sigstore_parse_siglist(esl);
- if (!siglist) {
- EFI_PRINT("Parsing signature list of %ls failed\n",
- name);
- goto err;
- }
-
- /* Append siglist */
- siglist->next = sigstore;
- sigstore = siglist;
-
- /* Next */
- db_size -= esl->signature_list_size;
- esl = (void *)esl + esl->signature_list_size;
+ free(db);
+ return NULL;
}
- free(db);
-
- return sigstore;
-err:
- efi_sigstore_free(sigstore);
- free(db);
-
- return NULL;
+ return efi_build_signature_store(db, db_size);
}
#endif /* CONFIG_EFI_SECURE_BOOT */
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 11/14] efi: capsule: Add support for uefi capsule authentication
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (9 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 10/14] efi_loader: Re-factor code to build the signature store from efi signature list Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 12/14] efi_loader: Enable " Sughosh Ganu
` (2 subsequent siblings)
13 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Add support for authenticating uefi capsules. Most of the signature
verification functionality is shared with the uefi secure boot
feature.
The root certificate containing the public key used for the signature
verification is stored as part of the device tree blob. The root
certificate is stored as an efi signature list(esl) file -- this file
contains the x509 certificate which is the root certificate.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
board/emulation/qemu-arm/qemu-arm.c | 35 ++++++++
include/efi_api.h | 18 ++++
include/efi_loader.h | 6 ++
lib/efi_loader/Kconfig | 17 ++++
lib/efi_loader/efi_capsule.c | 122 ++++++++++++++++++++++++++++
lib/efi_loader/efi_signature.c | 4 +-
6 files changed, 200 insertions(+), 2 deletions(-)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 8cad54c76f..5794d4c669 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -208,6 +208,41 @@ void flash_write32(u32 value, void *addr)
#define DFU_ALT_BUF_LEN SZ_1K
+int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
+{
+ const void *fdt_blob = gd->fdt_blob;
+ const void *blob;
+ const char *cnode_name = "capsule-key";
+ const char *snode_name = "signature";
+ int sig_node;
+ int len;
+
+ sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
+ if (sig_node < 0) {
+ EFI_PRINT("Unable to get signature node offset\n");
+ return -FDT_ERR_NOTFOUND;
+ }
+
+ blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len);
+
+ if (!blob || len < 0) {
+ EFI_PRINT("Unable to get capsule-key value\n");
+ *pkey = NULL;
+ *pkey_len = 0;
+ return -FDT_ERR_NOTFOUND;
+ }
+
+ *pkey = (void *)blob;
+ *pkey_len = len;
+
+ return 0;
+}
+
+bool efi_capsule_auth_enabled(void)
+{
+ return env_get("capsule_authentication_enabled") != NULL ? true : false;
+}
+
static void board_get_alt_info(struct mtd_info *mtd, char *buf)
{
struct mtd_info *part;
diff --git a/include/efi_api.h b/include/efi_api.h
index c7038f863a..bcf6de6629 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1808,6 +1808,24 @@ struct efi_variable_authentication_2 {
struct win_certificate_uefi_guid auth_info;
} __attribute__((__packed__));
+/**
+ * efi_firmware_image_authentication - Capsule authentication method
+ * descriptor
+ *
+ * This structure describes an authentication information for
+ * a capsule with IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED set
+ * and should be included as part of the capsule.
+ * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
+ *
+ * @monotonic_count: Count to prevent replay
+ * @auth_info: Authentication info
+ */
+struct efi_firmware_image_authentication {
+ uint64_t monotonic_count;
+ struct win_certificate_uefi_guid auth_info;
+} __attribute__((__packed__));
+
+
/**
* efi_signature_data - A format of signature
*
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 8d8a6649b5..d311317fc1 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -809,6 +809,8 @@ struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
bool efi_secure_boot_enabled(void);
+bool efi_capsule_auth_enabled(void);
+
bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
WIN_CERTIFICATE **auth, size_t *auth_len);
@@ -836,6 +838,10 @@ efi_status_t EFIAPI efi_query_capsule_caps(
u64 *maximum_capsule_size,
u32 *reset_type);
+efi_status_t efi_capsule_authenticate(const void *capsule,
+ efi_uintn_t capsule_size,
+ void **image, efi_uintn_t *image_size);
+
#define EFI_CAPSULE_DIR L"\\EFI\\UpdateCapsule\\"
/* Hook@initialization */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 55e4787e32..597ad6b2a7 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -145,6 +145,23 @@ config EFI_CAPSULE_FMP_HEADER
Select this option if the capsule is built using the
scripts in edk2.
+config EFI_CAPSULE_AUTHENTICATE
+ bool "Update Capsule authentication"
+ depends on EFI_CAPSULE_FIRMWARE
+ depends on EFI_CAPSULE_ON_DISK
+ depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+ select SHA256
+ select RSA
+ select RSA_VERIFY
+ select RSA_VERIFY_WITH_PKEY
+ select X509_CERTIFICATE_PARSER
+ select PKCS7_MESSAGE_PARSER
+ select PKCS7_VERIFY
+ default n
+ help
+ Select this option if you want to enable capsule
+ authentication
+
config EFI_CAPSULE_FIRMWARE_FIT
bool "FMP driver for FIT image"
depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index f385e58378..ad2036fe93 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -14,6 +14,10 @@
#include <mapmem.h>
#include <sort.h>
+#include <crypto/pkcs7.h>
+#include <crypto/pkcs7_parser.h>
+#include <linux/err.h>
+
const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
static const efi_guid_t efi_guid_firmware_management_capsule_id =
EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
@@ -191,6 +195,124 @@ skip:
return NULL;
}
+#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
+
+const efi_guid_t efi_guid_capsule_root_cert_guid =
+ EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
+
+__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
+{
+ /* The platform is supposed to provide
+ * a method for getting the public key
+ * stored in the form of efi signature
+ * list
+ */
+ return 0;
+}
+
+efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
+ void **image, efi_uintn_t *image_size)
+{
+ u8 *buf;
+ int ret;
+ void *fdt_pkey, *pkey;
+ efi_uintn_t pkey_len;
+ uint64_t monotonic_count;
+ struct efi_signature_store *truststore;
+ struct pkcs7_message *capsule_sig;
+ struct efi_image_regions *regs;
+ struct efi_firmware_image_authentication *auth_hdr;
+ efi_status_t status;
+
+ status = EFI_SECURITY_VIOLATION;
+ capsule_sig = NULL;
+ truststore = NULL;
+ regs = NULL;
+
+ /* Sanity checks */
+ if (capsule == NULL || capsule_size == 0)
+ goto out;
+
+ auth_hdr = (struct efi_firmware_image_authentication *)capsule;
+ if (capsule_size < sizeof(*auth_hdr))
+ goto out;
+
+ if (auth_hdr->auth_info.hdr.dwLength <=
+ offsetof(struct win_certificate_uefi_guid, cert_data))
+ goto out;
+
+ if (guidcmp(&auth_hdr->auth_info.cert_type, &efi_guid_cert_type_pkcs7))
+ goto out;
+
+ *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) +
+ auth_hdr->auth_info.hdr.dwLength;
+ *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength -
+ sizeof(auth_hdr->monotonic_count);
+ memcpy(&monotonic_count, &auth_hdr->monotonic_count,
+ sizeof(monotonic_count));
+
+ /* data to be digested */
+ regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, 1);
+ if (!regs)
+ goto out;
+
+ regs->max = 2;
+ efi_image_region_add(regs, (uint8_t *)*image,
+ (uint8_t *)*image + *image_size, 1);
+
+ efi_image_region_add(regs, (uint8_t *)&monotonic_count,
+ (uint8_t *)&monotonic_count + sizeof(monotonic_count),
+ 1);
+
+ capsule_sig = efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data,
+ auth_hdr->auth_info.hdr.dwLength
+ - sizeof(auth_hdr->auth_info),
+ &buf);
+ if (IS_ERR(capsule_sig)) {
+ debug("Parsing variable's pkcs7 header failed\n");
+ capsule_sig = NULL;
+ goto out;
+ }
+
+ ret = efi_get_public_key_data(&fdt_pkey, &pkey_len);
+ if (ret < 0)
+ goto out;
+
+ pkey = malloc(pkey_len);
+ if (!pkey)
+ goto out;
+
+ memcpy(pkey, fdt_pkey, pkey_len);
+ truststore = efi_build_signature_store(pkey, pkey_len);
+ if (!truststore)
+ goto out;
+
+ /* verify signature */
+ if (efi_signature_verify(regs, capsule_sig, truststore, NULL)) {
+ debug("Verified\n");
+ } else {
+ debug("Verifying variable's signature failed\n");
+ goto out;
+ }
+
+ status = EFI_SUCCESS;
+
+out:
+ efi_sigstore_free(truststore);
+ pkcs7_free_message(capsule_sig);
+ free(regs);
+
+ return status;
+}
+#else
+efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size,
+ void **image, efi_uintn_t *image_size)
+{
+ return EFI_UNSUPPORTED;
+}
+#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+
+
/**
* efi_capsule_update_firmware - update firmware from capsule
* @capsule_data: Capsule
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index 87525bdc80..c7ec275414 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -26,7 +26,7 @@ const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID;
const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;
const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
-#ifdef CONFIG_EFI_SECURE_BOOT
+#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE)
static u8 pkcs7_hdr[] = {
/* SEQUENCE */
0x30, 0x82, 0x05, 0xc7,
@@ -846,4 +846,4 @@ struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name)
return efi_build_signature_store(db, db_size);
}
-#endif /* CONFIG_EFI_SECURE_BOOT */
+#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 12/14] efi_loader: Enable uefi capsule authentication
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (10 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 11/14] efi: capsule: Add support for uefi capsule authentication Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:47 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 13/14] efidebug: capsule: Add a command to update capsule on disk Sughosh Ganu
2020-11-26 18:41 ` [PATCH 14/14] qemu: arm64: Add documentation for capsule update Sughosh Ganu
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Add support for enabling uefi capsule authentication. This feature is
enabled by setting the environment variable
"capsule_authentication_enabled".
The following configs are needed for enabling uefi capsule update and
capsule authentication features on the platform.
CONFIG_EFI_HAVE_CAPSULE_SUPPORT=y
CONFIG_EFI_CAPSULE_ON_DISK=y
CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
CONFIG_EFI_CAPSULE_FIRMWARE=y
CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
lib/efi_loader/efi_firmware.c | 37 ++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 6c97604d8b..5e17b2ab5a 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -162,9 +162,16 @@ static efi_status_t efi_get_dfu_info(
image_info[i].version_name = NULL; /* not supported */
image_info[i].size = 0;
image_info[i].attributes_supported =
- IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
+ IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
+ IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
image_info[i].attributes_setting =
IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
+
+ /* Check if the capsule authentication is enabled */
+ if (env_get("capsule_authentication_enabled"))
+ image_info[0].attributes_setting |=
+ IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
+
image_info[i].lowest_supported_image_version = 0;
image_info[i].last_attempt_version = 0;
image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
@@ -379,12 +386,40 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
efi_status_t (*progress)(efi_uintn_t completion),
u16 **abort_reason)
{
+ void *capsule_payload;
+ efi_status_t status;
+ efi_uintn_t capsule_payload_size;
+
EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image,
image_size, vendor_code, progress, abort_reason);
if (!image)
return EFI_EXIT(EFI_INVALID_PARAMETER);
+ /* Authenticate the capsule if authentication enabled */
+ if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
+ env_get("capsule_authentication_enabled")) {
+ capsule_payload = NULL;
+ capsule_payload_size = 0;
+ status = efi_capsule_authenticate(image, image_size,
+ &capsule_payload,
+ &capsule_payload_size);
+
+ if (status == EFI_SECURITY_VIOLATION) {
+ printf("Capsule authentication check failed. Aborting update\n");
+ return EFI_EXIT(status);
+ } else if (status != EFI_SUCCESS) {
+ return EFI_EXIT(status);
+ }
+
+ debug("Capsule authentication successfull\n");
+ image = capsule_payload;
+ image_size = capsule_payload_size;
+ } else {
+ debug("Capsule authentication disabled. ");
+ debug("Updating capsule without authenticating.\n");
+ }
+
if (CONFIG_IS_ENABLED(EFI_CAPSULE_FMP_HEADER)) {
/*
* When building the capsule with the scripts in
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 13/14] efidebug: capsule: Add a command to update capsule on disk
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (11 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 12/14] efi_loader: Enable " Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 14/14] qemu: arm64: Add documentation for capsule update Sughosh Ganu
13 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Add a efidebug subcommand to initiate a firmware update using the efi
firmware management protocol(fmp) set_image routine.
The firmware update can be initiated through
'efidebug capsule disk-update'
This would locate the efi capsule file on the efi system partition,
and call the platform's set_image fmp routine to initiate the firmware
update.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
cmd/efidebug.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 7d327c8268..b8fe9ad6d5 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -79,6 +79,16 @@ static int do_efi_capsule_update(struct cmd_tbl *cmdtp, int flag,
return CMD_RET_SUCCESS;
}
+static int do_efi_capsule_on_disk_update(struct cmd_tbl *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ efi_status_t ret;
+
+ ret = efi_launch_capsules();
+
+ return ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
+}
+
/**
* do_efi_capsule_show() - show capsule information
*
@@ -207,6 +217,8 @@ static struct cmd_tbl cmd_efidebug_capsule_sub[] = {
"", ""),
U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
"", ""),
+ U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
+ "", ""),
U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
"", ""),
};
@@ -1540,6 +1552,8 @@ static char efidebug_help_text[] =
#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
"efidebug capsule update [-v] <capsule address>\n"
" - process a capsule\n"
+ "efidebug capsule disk-update\n"
+ " - update a capsule from disk\n"
"efidebug capsule show <capsule address>\n"
" - show capsule information\n"
"efidebug capsule result [<capsule result var>]\n"
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 14/14] qemu: arm64: Add documentation for capsule update
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
` (12 preceding siblings ...)
2020-11-26 18:41 ` [PATCH 13/14] efidebug: capsule: Add a command to update capsule on disk Sughosh Ganu
@ 2020-11-26 18:41 ` Sughosh Ganu
2020-12-05 10:16 ` Heinrich Schuchardt
13 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-11-26 18:41 UTC (permalink / raw)
To: u-boot
Add documentation highlighting the steps for using the uefi capsule
update feature for updating the u-boot firmware image.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
doc/board/emulation/qemu-arm.rst | 157 +++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 8d7fda10f1..3978c13269 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -90,3 +90,160 @@ The debug UART on the ARM virt board uses these settings::
CONFIG_DEBUG_UART_PL010=y
CONFIG_DEBUG_UART_BASE=0x9000000
CONFIG_DEBUG_UART_CLOCK=0
+
+Enabling Uefi Capsule Update feature
+------------------------------------
+
+Support has been added for the uefi capsule update feature which
+enables updating the u-boot image using the uefi firmware management
+protocol (fmp). The capsules are not passed to the firmware through
+the UpdateCapsule runtime service. Instead, capsule-on-disk
+functionality is used for fetching the capsule from the EFI System
+Partition (ESP).
+
+Currently, support has been added for updating the u-boot binary as a
+raw image when the platform is booted in non-secure mode, i.e with
+CONFIG_TFABOOT disabled. For this configuration, the qemu platform
+needs to be booted with 'secure=off'. The u-boot binary placed on the
+first bank of the Nor Flash at offset 0x0. The u-boot environment is
+placed on the second Nor Flash bank at offset 0x4000000.
+
+The capsule update feature is enabled with the following configs::
+
+ CONFIG_MTD=y
+ CONFIG_FLASH_CFI_MTD=y
+ CONFIG_CMD_MTDPARTS=y
+ CONFIG_CMD_DFU=y
+ CONFIG_DFU_MTD=y
+ CONFIG_EFI_CAPSULE_ON_DISK=y
+ CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
+ CONFIG_EFI_CAPSULE_FIRMWARE=y
+ CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+ CONFIG_EFI_CAPSULE_FMP_HEADER=y
+
+In addition, the following config needs to be disabled::
+ CONFIG_TFABOOT
+
+The capsule file can be generated by using the GenerateCapsule.py
+script in edk2::
+
+ $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
+ <capsule_file_name> --fw-version <val> --lsv <val> --guid \
+ e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose --update-image-index \
+ <val> --verbose <u-boot.bin>
+
+If the above edk2 script is being used for generating the capsule, the
+following additional config needs to be enabled::
+ CONFIG_EFI_CAPSULE_FMP_HEADER=y
+
+As per the uefi specification, the capsule file needs to be placed on
+the EFI System Partition, under the EFI/UpdateCapsule/ directory. The
+EFI System Partition can be a virtio-blk-device.
+
+Before initiating the firmware update, the efi variables BootNext,
+BootXXXX and OsIndications need to be set. The BootXXXX variable needs
+to be pointing to the EFI System Partition which contains the capsule
+file. The BootNext, BootXXXX and OsIndications variables can be set
+using the following commands::
+
+ => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
+ => efidebug boot next 0
+ => setenv -e -nv -bs -rt -v OsIndications =0x04
+ => saveenv
+
+Finally, the capsule update can be initiated with the following
+command::
+
+ => efidebug capsule disk-update
+
+The updated u-boot image will be booted on subsequent boot.
+
+Enabling Capsule Authentication
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The uefi specification defines a way of authenticating the capsule to
+be updated by verifying the capsule signature. The capsule signature
+is computed and prepended to the capsule payload at the time of
+capsule generation. This signature is then verified by using the
+public key stored as part of the X509 certificate. This certificate is
+in the form of an efi signature list (esl) file, which is embedded as
+part of the platform's device tree blob using the mkeficapsule
+utility.
+
+The capsule authentication feature can be enabled through the
+following config, in addition to the configs listed above for capsule
+update::
+
+ CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+
+The public key esl file can be embedded in the dtb with the following
+command::
+ ./tools/mkeficapsule -K <pub_key.esl> -D <dtb>
+
+Running the above command results in the creation of a 'signature'
+node in the dtb, under which the public key is stored as a
+'capsule-key' property.
+
+Once the esl file has been embedded as part of the dtb, the platform
+needs to be be booted with this dtb. This can be done by disabling the
+CONFIG_OF_BOARD option, and then, passing the above dtb file to the
+u-boot build.
+
+The capsule update with authentication can be enabled on the platform
+with the following steps
+
+1. Install utility commands on your host
+ * openssl
+ * efitools
+
+2. Create signing keys and certificate files on your host::
+
+ $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
+ -keyout CRT.key -out CRT.crt -nodes -days 365
+ $ cert-to-efi-sig-list CRT.crt CRT.esl
+
+ $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
+ $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
+
+ $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
+ $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
+
+3. Store the esl file generated above as part of the dtb::
+
+ $ ./tools/mkeficapsule -K <pub_key.esl> -D <dtb>
+
+4. The capsule file can be generated by using the GenerateCapsule.py
+ script in edk2::
+
+ $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
+ <capsule_file_name> --monotonic-count <val> --fw-version \
+ <val> --lsv <val> --guid \
+ e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
+ --update-image-index <val> --signer-private-cert \
+ /path/to/CRT.pem --trusted-public-cert \
+ /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
+ <u-boot.bin>
+
+Once the capsule has been generated, use the same instructions as
+mentioned above for placing the capsule on the EFI System Partition
+
+5. Building u-boot with the following steps::
+
+ $ make qemu_arm64_defconfig
+ $ make menuconfig
+ Disable CONFIG_OF_BOARD and CONFIG_TFABOOT
+ $ make EXT_DTB=<dtb> all
+
+6. Enable capsule authentication by setting the following env
+ variable::
+
+ => setenv capsule_authentication_enabled 1
+ => saveenv
+
+Setting the environment variable capsule_authentication_enabled
+enables the capsule authentication.
+
+Once the capsule has been placed on the EFI System Partition and the
+above env variable has been set, along with the BootXXXX and the
+BootNext variables, the capsule update can be initiated
+using the same command as that shown above.
--
2.17.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-11-26 18:40 ` [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined Sughosh Ganu
@ 2020-12-05 9:31 ` Heinrich Schuchardt
2020-12-07 5:15 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 9:31 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> The Qemu platform emulator generates a device tree blob and places it
> at the start of the dram, which is then used by u-boot. Use this dtb
> only if CONFIG_OF_BOARD is defined. This allows using a different
> device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
> to the u-boot binary as a u-boot-fdt.bin file
Dear Sughosh,
thank your for this series which will allow us to better demonstrate and
test capsule updates.
I am not sure if the approach that you take at device-trees here is the
right one.
On QEMU the device-tree is generated on the fly by the QEMU binary
depending on which devices the user has specified.
Your idea is to replace this device-tree completely to be able to add
extra elements (the EFI signature list, see patch 2/14). Thus a
device-tree might be loaded that does not match the user selected devices.
An alternative approach would be to apply all additions to the
device-tree as an FDT overlay (or fixup). This would allow the dynamic
parts of the QEMU device-tree still to be passed through.
Could you, please, assess the pros and cons of the two approaches with
respect to:
* usability for capsule updates
* applicability for non-QEMU systems
* integration of DTB overlays with FIT images for other use cases
Best regards
Heinrich
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> board/emulation/qemu-arm/qemu-arm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index f18f2ed7da..e146d1cc50 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -89,11 +89,13 @@ int dram_init_banksize(void)
> return 0;
> }
>
> +#if defined(CONFIG_OF_BOARD)
> void *board_fdt_blob_setup(void)
> {
> /* QEMU loads a generated DTB for us at the start of RAM. */
> return (void *)CONFIG_SYS_SDRAM_BASE;
> }
> +#endif
>
> void enable_caches(void)
> {
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 03/14] qemu: arm: Scan the pci bus in board_init
2020-11-26 18:40 ` [PATCH 03/14] qemu: arm: Scan the pci bus in board_init Sughosh Ganu
@ 2020-12-05 9:45 ` Heinrich Schuchardt
2020-12-07 5:16 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 9:45 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> Scan the pci bus in board_init routine before scanning the virtio
> devices. This enumerates all the virtio devices, including devices
> found on the pci bus.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> board/emulation/qemu-arm/qemu-arm.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index e146d1cc50..b3d5b3d5c2 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -65,6 +65,14 @@ struct mm_region *mem_map = qemu_arm64_mem_map;
>
> int board_init(void)
> {
> +
> + /*
> + * Scan the pci bus before calling virtio_init. This
> + * enumerates all virtio devices, including devices
> + * on the pci bus.
> + */
> + pci_init();
This does not compile if CONFIG_PCI=n.
aarch64-linux-gnu-ld.bfd:
board/emulation/qemu-arm/built-in.o:
in function `board_init':
board/emulation/qemu-arm/qemu-arm.c:74:
undefined reference to `pci_init'
Cf.
arch/arm/mach-mvebu/arm64-common.c-106-#ifdef CONFIG_DM_PCI
board/socrates/socrates.c-134-#if defined(CONFIG_DM_PCI)
I found these lines in common/board_r.c:250
#ifdef CONFIG_PCI
static int initr_pci(void)
{
????????if (IS_ENABLED(CONFIG_PCI_INIT_R))
????????????????pci_init();
????????return 0;
}
#endif
Would selecting CONFIG_PCI_INIT_R be enough to solve your problem?
Best regards
Heinrich
> +
> /*
> * Make sure virtio bus is enumerated so that peripherals
> * on the virtio bus can be discovered by their drivers
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 14/14] qemu: arm64: Add documentation for capsule update
2020-11-26 18:41 ` [PATCH 14/14] qemu: arm64: Add documentation for capsule update Sughosh Ganu
@ 2020-12-05 10:16 ` Heinrich Schuchardt
0 siblings, 0 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:16 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> Add documentation highlighting the steps for using the uefi capsule
> update feature for updating the u-boot firmware image.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> doc/board/emulation/qemu-arm.rst | 157 +++++++++++++++++++++++++++++++
Thank you for carefully documenting your enhancement.
Unfortunately this does not build with 'make htmldocs'. (You will need
python3-sphinx with a version < 3 for building due to incompatible
changes in python3-sphinx.)
Warning, treated as error:
doc/board/emulation/qemu-arm.rst:137:Unexpected indentation.
make[1]: *** [doc/Makefile:69: htmldocs] Error 2
make: *** [Makefile:2167: htmldocs] Error 2
Please, run your series through Travis CI before resubmitting.
Takahiro's patches have been added to origin/next. So this is what your
series should be based on until the next is merged into master in January.
I am missing a documentation for mkeficapsule. Could you, please, try to
set one up together with Takahiro. I guess we should create a new
directory doc/tools/.
Best regards
Heinrich
> 1 file changed, 157 insertions(+)
>
> diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
> index 8d7fda10f1..3978c13269 100644
> --- a/doc/board/emulation/qemu-arm.rst
> +++ b/doc/board/emulation/qemu-arm.rst
> @@ -90,3 +90,160 @@ The debug UART on the ARM virt board uses these settings::
> CONFIG_DEBUG_UART_PL010=y
> CONFIG_DEBUG_UART_BASE=0x9000000
> CONFIG_DEBUG_UART_CLOCK=0
> +
> +Enabling Uefi Capsule Update feature
> +------------------------------------
> +
> +Support has been added for the uefi capsule update feature which
> +enables updating the u-boot image using the uefi firmware management
> +protocol (fmp). The capsules are not passed to the firmware through
> +the UpdateCapsule runtime service. Instead, capsule-on-disk
> +functionality is used for fetching the capsule from the EFI System
> +Partition (ESP).
> +
> +Currently, support has been added for updating the u-boot binary as a
> +raw image when the platform is booted in non-secure mode, i.e with
> +CONFIG_TFABOOT disabled. For this configuration, the qemu platform
> +needs to be booted with 'secure=off'. The u-boot binary placed on the
> +first bank of the Nor Flash at offset 0x0. The u-boot environment is
> +placed on the second Nor Flash bank at offset 0x4000000.
> +
> +The capsule update feature is enabled with the following configs::
> +
> + CONFIG_MTD=y
> + CONFIG_FLASH_CFI_MTD=y
> + CONFIG_CMD_MTDPARTS=y
> + CONFIG_CMD_DFU=y
> + CONFIG_DFU_MTD=y
> + CONFIG_EFI_CAPSULE_ON_DISK=y
> + CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> + CONFIG_EFI_CAPSULE_FIRMWARE=y
> + CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> + CONFIG_EFI_CAPSULE_FMP_HEADER=y
> +
> +In addition, the following config needs to be disabled::
> + CONFIG_TFABOOT
> +
> +The capsule file can be generated by using the GenerateCapsule.py
> +script in edk2::
> +
> + $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> + <capsule_file_name> --fw-version <val> --lsv <val> --guid \
> + e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose --update-image-index \
> + <val> --verbose <u-boot.bin>
> +
> +If the above edk2 script is being used for generating the capsule, the
> +following additional config needs to be enabled::
> + CONFIG_EFI_CAPSULE_FMP_HEADER=y
> +
> +As per the uefi specification, the capsule file needs to be placed on
> +the EFI System Partition, under the EFI/UpdateCapsule/ directory. The
> +EFI System Partition can be a virtio-blk-device.
> +
> +Before initiating the firmware update, the efi variables BootNext,
> +BootXXXX and OsIndications need to be set. The BootXXXX variable needs
> +to be pointing to the EFI System Partition which contains the capsule
> +file. The BootNext, BootXXXX and OsIndications variables can be set
> +using the following commands::
> +
> + => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
> + => efidebug boot next 0
> + => setenv -e -nv -bs -rt -v OsIndications =0x04
> + => saveenv
> +
> +Finally, the capsule update can be initiated with the following
> +command::
> +
> + => efidebug capsule disk-update
> +
> +The updated u-boot image will be booted on subsequent boot.
> +
> +Enabling Capsule Authentication
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The uefi specification defines a way of authenticating the capsule to
> +be updated by verifying the capsule signature. The capsule signature
> +is computed and prepended to the capsule payload at the time of
> +capsule generation. This signature is then verified by using the
> +public key stored as part of the X509 certificate. This certificate is
> +in the form of an efi signature list (esl) file, which is embedded as
> +part of the platform's device tree blob using the mkeficapsule
> +utility.
> +
> +The capsule authentication feature can be enabled through the
> +following config, in addition to the configs listed above for capsule
> +update::
> +
> + CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> +
> +The public key esl file can be embedded in the dtb with the following
> +command::
> + ./tools/mkeficapsule -K <pub_key.esl> -D <dtb>
> +
> +Running the above command results in the creation of a 'signature'
> +node in the dtb, under which the public key is stored as a
> +'capsule-key' property.
> +
> +Once the esl file has been embedded as part of the dtb, the platform
> +needs to be be booted with this dtb. This can be done by disabling the
> +CONFIG_OF_BOARD option, and then, passing the above dtb file to the
> +u-boot build.
> +
> +The capsule update with authentication can be enabled on the platform
> +with the following steps
> +
> +1. Install utility commands on your host
> + * openssl
> + * efitools
> +
> +2. Create signing keys and certificate files on your host::
> +
> + $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
> + -keyout CRT.key -out CRT.crt -nodes -days 365
> + $ cert-to-efi-sig-list CRT.crt CRT.esl
> +
> + $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> + $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> +
> + $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> + $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> +
> +3. Store the esl file generated above as part of the dtb::
> +
> + $ ./tools/mkeficapsule -K <pub_key.esl> -D <dtb>
> +
> +4. The capsule file can be generated by using the GenerateCapsule.py
> + script in edk2::
> +
> + $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> + <capsule_file_name> --monotonic-count <val> --fw-version \
> + <val> --lsv <val> --guid \
> + e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> + --update-image-index <val> --signer-private-cert \
> + /path/to/CRT.pem --trusted-public-cert \
> + /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> + <u-boot.bin>
> +
> +Once the capsule has been generated, use the same instructions as
> +mentioned above for placing the capsule on the EFI System Partition
> +
> +5. Building u-boot with the following steps::
> +
> + $ make qemu_arm64_defconfig
> + $ make menuconfig
> + Disable CONFIG_OF_BOARD and CONFIG_TFABOOT
> + $ make EXT_DTB=<dtb> all
> +
> +6. Enable capsule authentication by setting the following env
> + variable::
> +
> + => setenv capsule_authentication_enabled 1
> + => saveenv
> +
> +Setting the environment variable capsule_authentication_enabled
> +enables the capsule authentication.
> +
> +Once the capsule has been placed on the EFI System Partition and the
> +above env variable has been set, along with the BootXXXX and the
> +BootNext variables, the capsule update can be initiated
> +using the same command as that shown above.
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set
2020-11-26 18:41 ` [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set Sughosh Ganu
@ 2020-12-05 10:21 ` Heinrich Schuchardt
0 siblings, 0 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:21 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> RFC 2315 Section 9.3 describes the message digesting process. The
> digest calculated depends on whether the authenticated attributes are
> present. In case of a scenario where the authenticated attributes are
> present, the message digest that gets signed and is part of the pkcs7
> message is computed from the auth attributes rather than the contents
> field.
>
> Check if the auth attributes are present, and if set, use the auth
> attributes to compute the hash that would be compared with the
> encrypted hash on the pkcs7 message.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Dear Takahiro,
Could you, please, review this patch.
Best regards
Heinrich
> ---
> lib/crypto/pkcs7_verify.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> index 320ba49f79..58683ef614 100644
> --- a/lib/crypto/pkcs7_verify.c
> +++ b/lib/crypto/pkcs7_verify.c
> @@ -50,8 +50,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
> struct image_region regions[2];
> int ret = 0;
>
> - /* The digest was calculated already. */
> - if (sig->digest)
> + /*
> + * [RFC2315 9.3]
> + * If the authenticated attributes are present,
> + * the message-digest is calculated on the
> + * attributes present in the
> + * authenticatedAttributes field and not just
> + * the contents field
> + */
> + if (!sinfo->authattrs && sig->digest)
> return 0;
>
> if (!sinfo->sig->hash_algo)
> @@ -63,17 +70,25 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
> else
> return -ENOPKG;
>
> - sig->digest = calloc(1, sig->digest_size);
> - if (!sig->digest) {
> - pr_warn("Sig %u: Out of memory\n", sinfo->index);
> - return -ENOMEM;
> - }
> + /*
> + * Calculate the hash only if the data is present.
> + * In case of authenticated variable and capsule,
> + * the hash has already been calculated on the
> + * efi_image_regions and populated
> + */
> + if (pkcs7->data) {
> + sig->digest = calloc(1, sig->digest_size);
> + if (!sig->digest) {
> + pr_warn("Sig %u: Out of memory\n", sinfo->index);
> + return -ENOMEM;
> + }
>
> - regions[0].data = pkcs7->data;
> - regions[0].size = pkcs7->data_len;
> + regions[0].data = pkcs7->data;
> + regions[0].size = pkcs7->data_len;
>
> - /* Digest the message [RFC2315 9.3] */
> - hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
> + /* Digest the message [RFC2315 9.3] */
> + hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
> + }
>
> /* However, if there are authenticated attributes, there must be a
> * message digest attribute amongst them which corresponds to the
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform
2020-11-26 18:41 ` [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform Sughosh Ganu
@ 2020-12-05 10:29 ` Heinrich Schuchardt
2020-12-07 5:30 ` Sughosh Ganu
2020-12-07 18:44 ` Tom Rini
0 siblings, 2 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:29 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> Add support for setting the default values for mtd partitions on the
> platform for the nor flash. This would be used for updating the
> firmware image using uefi capsule update with the dfu mtd backend
> driver.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> board/emulation/qemu-arm/qemu-arm.c | 70 +++++++++++++++++++++++++++++
> include/configs/qemu-arm.h | 7 +++
> 2 files changed, 77 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index b3d5b3d5c2..d5ed3eebaf 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
Is this development really QEMU specific or is it something we would
could reuse for other systems too? If yes, we should put it into another
directory (drivers/mtd/ or drivers/dfu/).
Best regards
Heinrich
> @@ -197,3 +197,73 @@ void flash_write32(u32 value, void *addr)
> {
> asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
> }
> +
> +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> +
> +#include <mtd.h>
> +
> +static void board_get_mtdparts(const char *dev, const char *partition,
> + char *mtdids, char *mtdparts)
> +{
> + /* mtdids: "<dev>=<dev>, ...." */
> + if (mtdids[0] != '\0')
> + strcat(mtdids, ",");
> + strcat(mtdids, dev);
> + strcat(mtdids, "=");
> + strcat(mtdids, dev);
> +
> + /* mtdparts: "mtdparts=<dev>:<mtdparts_<dev>>;..." */
> + if (mtdparts[0] != '\0')
> + strncat(mtdparts, ";", MTDPARTS_LEN);
> + else
> + strcat(mtdparts, "mtdparts=");
> +
> + strncat(mtdparts, dev, MTDPARTS_LEN);
> + strncat(mtdparts, ":", MTDPARTS_LEN);
> + strncat(mtdparts, partition, MTDPARTS_LEN);
> +}
> +
> +void board_mtdparts_default(const char **mtdids, const char **mtdparts)
> +{
> + struct mtd_info *mtd;
> + struct udevice *dev;
> + const char *mtd_partition;
> + static char parts[3 * MTDPARTS_LEN + 1];
> + static char ids[MTDIDS_LEN + 1];
> + static bool mtd_initialized;
> +
> + if (mtd_initialized) {
> + *mtdids = ids;
> + *mtdparts = parts;
> + return;
> + }
> +
> + memset(parts, 0, sizeof(parts));
> + memset(ids, 0, sizeof(ids));
> +
> + /* probe all MTD devices */
> + for (uclass_first_device(UCLASS_MTD, &dev); dev;
> + uclass_next_device(&dev)) {
> + debug("mtd device = %s\n", dev->name);
> + }
> +
> + mtd = get_mtd_device_nm("nor0");
> + if (!IS_ERR_OR_NULL(mtd)) {
> + mtd_partition = MTDPARTS_NOR0;
> + board_get_mtdparts("nor0", mtd_partition, ids, parts);
> + put_mtd_device(mtd);
> + }
> +
> + mtd = get_mtd_device_nm("nor1");
> + if (!IS_ERR_OR_NULL(mtd)) {
> + mtd_partition = MTDPARTS_NOR1;
> + board_get_mtdparts("nor1", mtd_partition, ids, parts);
> + put_mtd_device(mtd);
> + }
> +
> + mtd_initialized = true;
> + *mtdids = ids;
> + *mtdparts = parts;
> + debug("%s:mtdids=%s & mtdparts=%s\n", __func__, ids, parts);
> +}
> +#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 273fa1a7d7..69ff329434 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -32,6 +32,13 @@
>
> #include <config_distro_bootcmd.h>
>
> +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> +#define CONFIG_SYS_MTDPARTS_RUNTIME
> +#endif
> +
> +#define MTDPARTS_NOR0 "64m(u-boot)\0"
> +#define MTDPARTS_NOR1 "64m(u-boot-env)\0"
> +
> #define CONFIG_EXTRA_ENV_SETTINGS \
> "fdt_high=0xffffffff\0" \
> "initrd_high=0xffffffff\0" \
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-11-26 18:41 ` [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable " Sughosh Ganu
@ 2020-12-05 10:31 ` Heinrich Schuchardt
2020-12-07 5:42 ` Sughosh Ganu
2020-12-07 18:47 ` Tom Rini
0 siblings, 2 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:31 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> The dfu framework uses the dfu_alt_info environment variable to get
> information that is needed for performing the firmware update. Set the
> dfu_alt_info for the platform to reflect the two mtd partitions
> created for the u-boot env and the firmware image.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
I can't see anything QEMU specific in this patch. Why is the code under
board/emulation/?
Best regards
Heinrich
> ---
> board/emulation/qemu-arm/qemu-arm.c | 55 +++++++++++++++++++++++++++++
> include/configs/qemu-arm.h | 1 +
> 2 files changed, 56 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index d5ed3eebaf..8cad54c76f 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
>
> #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
>
> +#include <memalign.h>
> #include <mtd.h>
>
> +#define MTDPARTS_LEN 256
> +#define MTDIDS_LEN 128
> +
> +#define DFU_ALT_BUF_LEN SZ_1K
> +
> +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> +{
> + struct mtd_info *part;
> + bool first = true;
> + const char *name;
> + int len, partnum = 0;
> +
> + name = mtd->name;
> + len = strlen(buf);
> +
> + if (buf[0] != '\0')
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> + "mtd %s=", name);
> +
> + list_for_each_entry(part, &mtd->partitions, node) {
> + partnum++;
> + if (!first)
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, ";");
> + first = false;
> +
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> + "%s part %d",
> + part->name, partnum);
> + }
> +}
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> + struct mtd_info *mtd;
> +
> + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> + if (env_get("dfu_alt_info"))
> + return;
> +
> + memset(buf, 0, sizeof(buf));
> +
> + /* probe all MTD devices */
> + mtd_probe_devices();
> +
> + mtd = get_mtd_device_nm("nor0");
> + if (!IS_ERR_OR_NULL(mtd))
> + board_get_alt_info(mtd, buf);
> +
> + env_set("dfu_alt_info", buf);
> + printf("dfu_alt_info set\n");
> +}
> +
> static void board_get_mtdparts(const char *dev, const char *partition,
> char *mtdids, char *mtdparts)
> {
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 69ff329434..726f985d35 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -33,6 +33,7 @@
> #include <config_distro_bootcmd.h>
>
> #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> +#define CONFIG_SET_DFU_ALT_INFO
> #define CONFIG_SYS_MTDPARTS_RUNTIME
> #endif
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence
2020-11-26 18:41 ` [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence Sughosh Ganu
@ 2020-12-05 10:34 ` Heinrich Schuchardt
2020-12-07 6:02 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:34 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> When building the capsule using scripts in edk2, an fmp header is
> added on top of the binary payload. Add a config option to indicate
> the presence of the header. When enabled, the pointer to the image
> needs to be adjusted as per the size of the header to point to the
> actual binary payload.
Why do we need a config option? Can't we detect the header?
Can we harmonize the capsule format in EDK II and U-Boot?
Best regards
Heinrich
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> lib/efi_loader/Kconfig | 7 +++++++
> lib/efi_loader/efi_firmware.c | 12 ++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 0d1b1b5356..55e4787e32 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -138,6 +138,13 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> Select this option if you want to enable capsule-based
> firmware update using Firmware Management Protocol.
>
> +config EFI_CAPSULE_FMP_HEADER
> + bool "Capsule uses FMP header"
> + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> + help
> + Select this option if the capsule is built using the
> + scripts in edk2.
> +
> config EFI_CAPSULE_FIRMWARE_FIT
> bool "FMP driver for FIT image"
> depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 7e56077383..6c97604d8b 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -385,10 +385,22 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> if (!image)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> + if (CONFIG_IS_ENABLED(EFI_CAPSULE_FMP_HEADER)) {
> + /*
> + * When building the capsule with the scripts in
> + * edk2, a FMP header is inserted above the capsule
> + * payload. Compensate for this header to get the
> + * actual payload that is to be updated.
> + */
> + image += 0x10;
> + image_size -= 0x10;
> + }
> +
> if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
> NULL, NULL))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> + printf("%s: Capsule update complete!\n", __func__);
> return EFI_EXIT(EFI_SUCCESS);
> }
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern
2020-11-26 18:41 ` [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern Sughosh Ganu
@ 2020-12-05 10:40 ` Heinrich Schuchardt
0 siblings, 0 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:40 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> The pkcs7 header parsing functionality is pretty generic, and can be
> used by other features like capsule authentication. Make the function
> an extern, also changing it's name to efi_parse_pkcs7_header
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> include/efi_loader.h | 4 ++
> lib/efi_loader/efi_signature.c | 85 +++++++++++++++++++++++++++++++
> lib/efi_loader/efi_variable.c | 93 ++--------------------------------
> 3 files changed, 93 insertions(+), 89 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 76cd2b36f2..b9226208f5 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -810,6 +810,10 @@ bool efi_secure_boot_enabled(void);
> bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> WIN_CERTIFICATE **auth, size_t *auth_len);
>
> +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf,
> + size_t buflen,
> + u8 **tmpbuf);
> +
> /* runtime implementation of memcpy() */
> void efi_memcpy_runtime(void *dest, const void *src, size_t n);
>
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index 79dee27421..9ab071b611 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -27,6 +27,91 @@ const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;
> const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>
> #ifdef CONFIG_EFI_SECURE_BOOT
> +static u8 pkcs7_hdr[] = {
> + /* SEQUENCE */
> + 0x30, 0x82, 0x05, 0xc7,
> + /* OID: pkcs7-signedData */
> + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
> + /* Context Structured? */
> + 0xa0, 0x82, 0x05, 0xb8,
Dear Takahiro,
From where are these magic numbers taken? Could you, please, provide a
reference that we can add as a comment.
Best regards
Heinrich
> +};
> +
> +/**
> + * efi_parse_pkcs7_header - parse a signature in payload
> + * @buf: Pointer to payload's value
> + * @buflen: Length of @buf
> + * @tmpbuf: Pointer to temporary buffer
> + *
> + * Parse a signature embedded in payload's value and instantiate
> + * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
> + * pkcs7's signedData, some header needed be prepended for correctly
> + * parsing authentication data
> + * A temporary buffer will be allocated if needed, and it should be
> + * kept valid during the authentication because some data in the buffer
> + * will be referenced by efi_signature_verify().
> + *
> + * Return: Pointer to pkcs7_message structure on success, NULL on error
> + */
> +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf,
> + size_t buflen,
> + u8 **tmpbuf)
> +{
> + u8 *ebuf;
> + size_t ebuflen, len;
> + struct pkcs7_message *msg;
> +
> + /*
> + * This is the best assumption to check if the binary is
> + * already in a form of pkcs7's signedData.
> + */
> + if (buflen > sizeof(pkcs7_hdr) &&
> + !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
> + msg = pkcs7_parse_message(buf, buflen);
> + if (IS_ERR(msg))
> + return NULL;
> + return msg;
> + }
> +
> + /*
> + * Otherwise, we should add a dummy prefix sequence for pkcs7
> + * message parser to be able to process.
> + * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> + * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> + * TODO:
> + * The header should be composed in a more refined manner.
> + */
> + EFI_PRINT("Makeshift prefix added to authentication data\n");
> + ebuflen = sizeof(pkcs7_hdr) + buflen;
> + if (ebuflen <= 0x7f) {
> + EFI_PRINT("Data is too short\n");
> + return NULL;
> + }
> +
> + ebuf = malloc(ebuflen);
> + if (!ebuf) {
> + EFI_PRINT("Out of memory\n");
> + return NULL;
> + }
> +
> + memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> + memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> + len = ebuflen - 4;
> + ebuf[2] = (len >> 8) & 0xff;
> + ebuf[3] = len & 0xff;
> + len = ebuflen - 0x13;
> + ebuf[0x11] = (len >> 8) & 0xff;
> + ebuf[0x12] = len & 0xff;
> +
> + msg = pkcs7_parse_message(ebuf, ebuflen);
> +
> + if (IS_ERR(msg)) {
> + free(ebuf);
> + return NULL;
> + }
> +
> + *tmpbuf = ebuf;
> + return msg;
> +}
>
> /**
> * efi_hash_regions - calculate a hash value
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 0c689cfb47..ba0874e9e7 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -24,91 +24,6 @@
> #include <asm/sections.h>
>
> #ifdef CONFIG_EFI_SECURE_BOOT
> -static u8 pkcs7_hdr[] = {
> - /* SEQUENCE */
> - 0x30, 0x82, 0x05, 0xc7,
> - /* OID: pkcs7-signedData */
> - 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
> - /* Context Structured? */
> - 0xa0, 0x82, 0x05, 0xb8,
> -};
> -
> -/**
> - * efi_variable_parse_signature - parse a signature in variable
> - * @buf: Pointer to variable's value
> - * @buflen: Length of @buf
> - * @tmpbuf: Pointer to temporary buffer
> - *
> - * Parse a signature embedded in variable's value and instantiate
> - * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
> - * pkcs7's signedData, some header needed be prepended for correctly
> - * parsing authentication data, particularly for variable's.
> - * A temporary buffer will be allocated if needed, and it should be
> - * kept valid during the authentication because some data in the buffer
> - * will be referenced by efi_signature_verify().
> - *
> - * Return: Pointer to pkcs7_message structure on success, NULL on error
> - */
> -static struct pkcs7_message *efi_variable_parse_signature(const void *buf,
> - size_t buflen,
> - u8 **tmpbuf)
> -{
> - u8 *ebuf;
> - size_t ebuflen, len;
> - struct pkcs7_message *msg;
> -
> - /*
> - * This is the best assumption to check if the binary is
> - * already in a form of pkcs7's signedData.
> - */
> - if (buflen > sizeof(pkcs7_hdr) &&
> - !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
> - msg = pkcs7_parse_message(buf, buflen);
> - if (IS_ERR(msg))
> - return NULL;
> - return msg;
> - }
> -
> - /*
> - * Otherwise, we should add a dummy prefix sequence for pkcs7
> - * message parser to be able to process.
> - * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> - * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> - * TODO:
> - * The header should be composed in a more refined manner.
> - */
> - EFI_PRINT("Makeshift prefix added to authentication data\n");
> - ebuflen = sizeof(pkcs7_hdr) + buflen;
> - if (ebuflen <= 0x7f) {
> - EFI_PRINT("Data is too short\n");
> - return NULL;
> - }
> -
> - ebuf = malloc(ebuflen);
> - if (!ebuf) {
> - EFI_PRINT("Out of memory\n");
> - return NULL;
> - }
> -
> - memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> - memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> - len = ebuflen - 4;
> - ebuf[2] = (len >> 8) & 0xff;
> - ebuf[3] = len & 0xff;
> - len = ebuflen - 0x13;
> - ebuf[0x11] = (len >> 8) & 0xff;
> - ebuf[0x12] = len & 0xff;
> -
> - msg = pkcs7_parse_message(ebuf, ebuflen);
> -
> - if (IS_ERR(msg)) {
> - free(ebuf);
> - return NULL;
> - }
> -
> - *tmpbuf = ebuf;
> - return msg;
> -}
>
> /**
> * efi_variable_authenticate - authenticate a variable
> @@ -215,10 +130,10 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
> goto err;
>
> /* ebuf should be kept valid during the authentication */
> - var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
> - auth->auth_info.hdr.dwLength
> - - sizeof(auth->auth_info),
> - &ebuf);
> + var_sig = efi_parse_pkcs7_header(auth->auth_info.cert_data,
> + auth->auth_info.hdr.dwLength
> + - sizeof(auth->auth_info),
> + &ebuf);
> if (!var_sig) {
> EFI_PRINT("Parsing variable's signature failed\n");
> goto err;
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 12/14] efi_loader: Enable uefi capsule authentication
2020-11-26 18:41 ` [PATCH 12/14] efi_loader: Enable " Sughosh Ganu
@ 2020-12-05 10:47 ` Heinrich Schuchardt
0 siblings, 0 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-05 10:47 UTC (permalink / raw)
To: u-boot
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> Add support for enabling uefi capsule authentication. This feature is
> enabled by setting the environment variable
> "capsule_authentication_enabled".
>
> The following configs are needed for enabling uefi capsule update and
> capsule authentication features on the platform.
>
> CONFIG_EFI_HAVE_CAPSULE_SUPPORT=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> CONFIG_EFI_CAPSULE_FIRMWARE=y
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> CONFIG_EFI_CAPSULE_AUTHENTICATE=y
Dear Takahiro, dear Sughosh,
could you, please, provide a documentation for capsule updates in /doc/uefi.
Best regards
Heinrich
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> lib/efi_loader/efi_firmware.c | 37 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 6c97604d8b..5e17b2ab5a 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -162,9 +162,16 @@ static efi_status_t efi_get_dfu_info(
> image_info[i].version_name = NULL; /* not supported */
> image_info[i].size = 0;
> image_info[i].attributes_supported =
> - IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
> + IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
> + IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
> image_info[i].attributes_setting =
> IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
> +
> + /* Check if the capsule authentication is enabled */
> + if (env_get("capsule_authentication_enabled"))
> + image_info[0].attributes_setting |=
> + IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
> +
> image_info[i].lowest_supported_image_version = 0;
> image_info[i].last_attempt_version = 0;
> image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> @@ -379,12 +386,40 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> efi_status_t (*progress)(efi_uintn_t completion),
> u16 **abort_reason)
> {
> + void *capsule_payload;
> + efi_status_t status;
> + efi_uintn_t capsule_payload_size;
> +
> EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image,
> image_size, vendor_code, progress, abort_reason);
>
> if (!image)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> + /* Authenticate the capsule if authentication enabled */
> + if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
> + env_get("capsule_authentication_enabled")) {
> + capsule_payload = NULL;
> + capsule_payload_size = 0;
> + status = efi_capsule_authenticate(image, image_size,
> + &capsule_payload,
> + &capsule_payload_size);
> +
> + if (status == EFI_SECURITY_VIOLATION) {
> + printf("Capsule authentication check failed. Aborting update\n");
> + return EFI_EXIT(status);
> + } else if (status != EFI_SUCCESS) {
> + return EFI_EXIT(status);
> + }
> +
> + debug("Capsule authentication successfull\n");
> + image = capsule_payload;
> + image_size = capsule_payload_size;
> + } else {
> + debug("Capsule authentication disabled. ");
> + debug("Updating capsule without authenticating.\n");
> + }
> +
> if (CONFIG_IS_ENABLED(EFI_CAPSULE_FMP_HEADER)) {
> /*
> * When building the capsule with the scripts in
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-05 9:31 ` Heinrich Schuchardt
@ 2020-12-07 5:15 ` Sughosh Ganu
2020-12-07 12:50 ` Heinrich Schuchardt
0 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 5:15 UTC (permalink / raw)
To: u-boot
hello Heinrich,
On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > The Qemu platform emulator generates a device tree blob and places it
> > at the start of the dram, which is then used by u-boot. Use this dtb
> > only if CONFIG_OF_BOARD is defined. This allows using a different
> > device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
> > to the u-boot binary as a u-boot-fdt.bin file
>
> Dear Sughosh,
>
> thank your for this series which will allow us to better demonstrate and
> test capsule updates.
>
> I am not sure if the approach that you take at device-trees here is the
> right one.
>
> On QEMU the device-tree is generated on the fly by the QEMU binary
> depending on which devices the user has specified.
>
> Your idea is to replace this device-tree completely to be able to add
> extra elements (the EFI signature list, see patch 2/14). Thus a
> device-tree might be loaded that does not match the user selected devices.
>
> An alternative approach would be to apply all additions to the
> device-tree as an FDT overlay (or fixup). This would allow the dynamic
> parts of the QEMU device-tree still to be passed through.
>
I will take a look at storing the public key as part of the fdt overlay,
with a runtime fixup. Although, I think the issue that you are pointing to
would be specific to Qemu and not other platforms. But I do see the merit
in having the public-key certificate stored as part of an overlay. If I hit
any issues while implementing this, I will get back to you. Thanks.
-sughosh
> Could you, please, assess the pros and cons of the two approaches with
> respect to:
>
> * usability for capsule updates
> * applicability for non-QEMU systems
> * integration of DTB overlays with FIT images for other use cases
>
> Best regards
>
> Heinrich
>
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index f18f2ed7da..e146d1cc50 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -89,11 +89,13 @@ int dram_init_banksize(void)
> > return 0;
> > }
> >
> > +#if defined(CONFIG_OF_BOARD)
> > void *board_fdt_blob_setup(void)
> > {
> > /* QEMU loads a generated DTB for us at the start of RAM. */
> > return (void *)CONFIG_SYS_SDRAM_BASE;
> > }
> > +#endif
> >
> > void enable_caches(void)
> > {
> >
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 03/14] qemu: arm: Scan the pci bus in board_init
2020-12-05 9:45 ` Heinrich Schuchardt
@ 2020-12-07 5:16 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 5:16 UTC (permalink / raw)
To: u-boot
On Sat, 5 Dec 2020 at 15:15, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > Scan the pci bus in board_init routine before scanning the virtio
> > devices. This enumerates all the virtio devices, including devices
> > found on the pci bus.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index e146d1cc50..b3d5b3d5c2 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -65,6 +65,14 @@ struct mm_region *mem_map = qemu_arm64_mem_map;
> >
> > int board_init(void)
> > {
> > +
> > + /*
> > + * Scan the pci bus before calling virtio_init. This
> > + * enumerates all virtio devices, including devices
> > + * on the pci bus.
> > + */
> > + pci_init();
>
> This does not compile if CONFIG_PCI=n.
>
> aarch64-linux-gnu-ld.bfd:
> board/emulation/qemu-arm/built-in.o:
> in function `board_init':
> board/emulation/qemu-arm/qemu-arm.c:74:
> undefined reference to `pci_init'
>
> Cf.
> arch/arm/mach-mvebu/arm64-common.c-106-#ifdef CONFIG_DM_PCI
> board/socrates/socrates.c-134-#if defined(CONFIG_DM_PCI)
>
> I found these lines in common/board_r.c:250
>
> #ifdef CONFIG_PCI
> static int initr_pci(void)
> {
> if (IS_ENABLED(CONFIG_PCI_INIT_R))
> pci_init();
>
> return 0;
> }
> #endif
>
> Would selecting CONFIG_PCI_INIT_R be enough to solve your problem?
>
Will check this out. Thanks.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform
2020-12-05 10:29 ` Heinrich Schuchardt
@ 2020-12-07 5:30 ` Sughosh Ganu
2020-12-07 18:44 ` Tom Rini
1 sibling, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 5:30 UTC (permalink / raw)
To: u-boot
On Sat, 5 Dec 2020 at 16:00, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > Add support for setting the default values for mtd partitions on the
> > platform for the nor flash. This would be used for updating the
> > firmware image using uefi capsule update with the dfu mtd backend
> > driver.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 70 +++++++++++++++++++++++++++++
> > include/configs/qemu-arm.h | 7 +++
> > 2 files changed, 77 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index b3d5b3d5c2..d5ed3eebaf 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
>
> Is this development really QEMU specific or is it something we would
> could reuse for other systems too? If yes, we should put it into another
> directory (drivers/mtd/ or drivers/dfu/).
>
I think the functionality added in this patch is specific for a particular
board, given that the number and type of mtd devices would be specific to a
board. Even if the mtd device detection can be done dynamically, it would
be required to get the partition information from the board specific files.
Merging all the functionality under a common directory like drivers/mtd/,
if done, should be taken up as a separate effort. Thanks.
-sughosh
> Best regards
>
> Heinrich
>
> > @@ -197,3 +197,73 @@ void flash_write32(u32 value, void *addr)
> > {
> > asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
> > }
> > +
> > +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +
> > +#include <mtd.h>
> > +
> > +static void board_get_mtdparts(const char *dev, const char *partition,
> > + char *mtdids, char *mtdparts)
> > +{
> > + /* mtdids: "<dev>=<dev>, ...." */
> > + if (mtdids[0] != '\0')
> > + strcat(mtdids, ",");
> > + strcat(mtdids, dev);
> > + strcat(mtdids, "=");
> > + strcat(mtdids, dev);
> > +
> > + /* mtdparts: "mtdparts=<dev>:<mtdparts_<dev>>;..." */
> > + if (mtdparts[0] != '\0')
> > + strncat(mtdparts, ";", MTDPARTS_LEN);
> > + else
> > + strcat(mtdparts, "mtdparts=");
> > +
> > + strncat(mtdparts, dev, MTDPARTS_LEN);
> > + strncat(mtdparts, ":", MTDPARTS_LEN);
> > + strncat(mtdparts, partition, MTDPARTS_LEN);
> > +}
> > +
> > +void board_mtdparts_default(const char **mtdids, const char **mtdparts)
> > +{
> > + struct mtd_info *mtd;
> > + struct udevice *dev;
> > + const char *mtd_partition;
> > + static char parts[3 * MTDPARTS_LEN + 1];
> > + static char ids[MTDIDS_LEN + 1];
> > + static bool mtd_initialized;
> > +
> > + if (mtd_initialized) {
> > + *mtdids = ids;
> > + *mtdparts = parts;
> > + return;
> > + }
> > +
> > + memset(parts, 0, sizeof(parts));
> > + memset(ids, 0, sizeof(ids));
> > +
> > + /* probe all MTD devices */
> > + for (uclass_first_device(UCLASS_MTD, &dev); dev;
> > + uclass_next_device(&dev)) {
> > + debug("mtd device = %s\n", dev->name);
> > + }
> > +
> > + mtd = get_mtd_device_nm("nor0");
> > + if (!IS_ERR_OR_NULL(mtd)) {
> > + mtd_partition = MTDPARTS_NOR0;
> > + board_get_mtdparts("nor0", mtd_partition, ids, parts);
> > + put_mtd_device(mtd);
> > + }
> > +
> > + mtd = get_mtd_device_nm("nor1");
> > + if (!IS_ERR_OR_NULL(mtd)) {
> > + mtd_partition = MTDPARTS_NOR1;
> > + board_get_mtdparts("nor1", mtd_partition, ids, parts);
> > + put_mtd_device(mtd);
> > + }
> > +
> > + mtd_initialized = true;
> > + *mtdids = ids;
> > + *mtdparts = parts;
> > + debug("%s:mtdids=%s & mtdparts=%s\n", __func__, ids, parts);
> > +}
> > +#endif /* CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT */
> > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > index 273fa1a7d7..69ff329434 100644
> > --- a/include/configs/qemu-arm.h
> > +++ b/include/configs/qemu-arm.h
> > @@ -32,6 +32,13 @@
> >
> > #include <config_distro_bootcmd.h>
> >
> > +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +#define CONFIG_SYS_MTDPARTS_RUNTIME
> > +#endif
> > +
> > +#define MTDPARTS_NOR0 "64m(u-boot)\0"
> > +#define MTDPARTS_NOR1 "64m(u-boot-env)\0"
> > +
> > #define CONFIG_EXTRA_ENV_SETTINGS \
> > "fdt_high=0xffffffff\0" \
> > "initrd_high=0xffffffff\0" \
> >
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-05 10:31 ` Heinrich Schuchardt
@ 2020-12-07 5:42 ` Sughosh Ganu
2020-12-07 6:56 ` Heinrich Schuchardt
2020-12-07 18:47 ` Tom Rini
1 sibling, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 5:42 UTC (permalink / raw)
To: u-boot
On Sat, 5 Dec 2020 at 16:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > The dfu framework uses the dfu_alt_info environment variable to get
> > information that is needed for performing the firmware update. Set the
> > dfu_alt_info for the platform to reflect the two mtd partitions
> > created for the u-boot env and the firmware image.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>
> I can't see anything QEMU specific in this patch. Why is the code under
> board/emulation/?
>
The device that the board wants to use to set the dfu_alt_info to would be
very much board specific. For example, the dfu_alt_info is being set for
the qemu platform for an mtd as the interface, with nor0 as the device.
Different platforms might have different requirements, like the setting of
the variable done for st platforms(board/st/common/stm32mp_dfu.c). I think
this information is very much platform specific.
-sughosh
>
> Best regards
>
> Heinrich
>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 55 +++++++++++++++++++++++++++++
> > include/configs/qemu-arm.h | 1 +
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index d5ed3eebaf..8cad54c76f 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> >
> > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> >
> > +#include <memalign.h>
> > #include <mtd.h>
> >
> > +#define MTDPARTS_LEN 256
> > +#define MTDIDS_LEN 128
> > +
> > +#define DFU_ALT_BUF_LEN SZ_1K
> > +
> > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > +{
> > + struct mtd_info *part;
> > + bool first = true;
> > + const char *name;
> > + int len, partnum = 0;
> > +
> > + name = mtd->name;
> > + len = strlen(buf);
> > +
> > + if (buf[0] != '\0')
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > + "mtd %s=", name);
> > +
> > + list_for_each_entry(part, &mtd->partitions, node) {
> > + partnum++;
> > + if (!first)
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> ";");
> > + first = false;
> > +
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > + "%s part %d",
> > + part->name, partnum);
> > + }
> > +}
> > +
> > +void set_dfu_alt_info(char *interface, char *devstr)
> > +{
> > + struct mtd_info *mtd;
> > +
> > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > +
> > + if (env_get("dfu_alt_info"))
> > + return;
> > +
> > + memset(buf, 0, sizeof(buf));
> > +
> > + /* probe all MTD devices */
> > + mtd_probe_devices();
> > +
> > + mtd = get_mtd_device_nm("nor0");
> > + if (!IS_ERR_OR_NULL(mtd))
> > + board_get_alt_info(mtd, buf);
> > +
> > + env_set("dfu_alt_info", buf);
> > + printf("dfu_alt_info set\n");
> > +}
> > +
> > static void board_get_mtdparts(const char *dev, const char *partition,
> > char *mtdids, char *mtdparts)
> > {
> > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > index 69ff329434..726f985d35 100644
> > --- a/include/configs/qemu-arm.h
> > +++ b/include/configs/qemu-arm.h
> > @@ -33,6 +33,7 @@
> > #include <config_distro_bootcmd.h>
> >
> > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +#define CONFIG_SET_DFU_ALT_INFO
> > #define CONFIG_SYS_MTDPARTS_RUNTIME
> > #endif
> >
> >
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence
2020-12-05 10:34 ` Heinrich Schuchardt
@ 2020-12-07 6:02 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 6:02 UTC (permalink / raw)
To: u-boot
On Sat, 5 Dec 2020 at 16:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > When building the capsule using scripts in edk2, an fmp header is
> > added on top of the binary payload. Add a config option to indicate
> > the presence of the header. When enabled, the pointer to the image
> > needs to be adjusted as per the size of the header to point to the
> > actual binary payload.
>
> Why do we need a config option? Can't we detect the header?
>
We do have a header signature that can be read, so yes this can be detected
at runtime.
>
> Can we harmonize the capsule format in EDK II and U-Boot?
>
I am not sure about this though. The FMP payload header that gets added by
the edk2 scripts is not defined by the uefi specification but is a edk2
specific structure. I am not sure if defining it in u-boot would make sense.
-sughosh
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > lib/efi_loader/Kconfig | 7 +++++++
> > lib/efi_loader/efi_firmware.c | 12 ++++++++++++
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 0d1b1b5356..55e4787e32 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -138,6 +138,13 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> > Select this option if you want to enable capsule-based
> > firmware update using Firmware Management Protocol.
> >
> > +config EFI_CAPSULE_FMP_HEADER
> > + bool "Capsule uses FMP header"
> > + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> > + help
> > + Select this option if the capsule is built using the
> > + scripts in edk2.
> > +
> > config EFI_CAPSULE_FIRMWARE_FIT
> > bool "FMP driver for FIT image"
> > depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> > diff --git a/lib/efi_loader/efi_firmware.c
> b/lib/efi_loader/efi_firmware.c
> > index 7e56077383..6c97604d8b 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -385,10 +385,22 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > if (!image)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > + if (CONFIG_IS_ENABLED(EFI_CAPSULE_FMP_HEADER)) {
> > + /*
> > + * When building the capsule with the scripts in
> > + * edk2, a FMP header is inserted above the capsule
> > + * payload. Compensate for this header to get the
> > + * actual payload that is to be updated.
> > + */
> > + image += 0x10;
> > + image_size -= 0x10;
> > + }
> > +
> > if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
> > NULL, NULL))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + printf("%s: Capsule update complete!\n", __func__);
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> >
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-07 5:42 ` Sughosh Ganu
@ 2020-12-07 6:56 ` Heinrich Schuchardt
2020-12-07 7:45 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-07 6:56 UTC (permalink / raw)
To: u-boot
On 12/7/20 6:42 AM, Sughosh Ganu wrote:
>
> On Sat, 5 Dec 2020 at 16:01, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > The dfu framework uses the dfu_alt_info environment variable to get
> > information that is needed for performing the firmware update.
> Set the
> > dfu_alt_info for the platform to reflect the two mtd partitions
> > created for the u-boot env and the firmware image.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> <mailto:sughosh.ganu@linaro.org>>
>
> I can't see anything QEMU specific in this patch. Why is the code under
> board/emulation/?
>
>
> The device that the board wants to use to set the dfu_alt_info to would
> be very much board specific. For example, the dfu_alt_info is being set
> for the qemu platform for an mtd as the interface, with nor0 as the
> device. Different platforms might have different requirements, like the
> setting of the variable done for st
> platforms(board/st/common/stm32mp_dfu.c). I think this information is
> very much platform specific.
In the function below you simply collect MTD partitions. Isn't this
something that could be reused for other boards?
On other boards (e.g. board/samsung/common/misc.c)
CONFIG_SET_DFU_ALT_INFO controls if set_dfu_alt_info() exists. Should
the same be done for QEMU ARM?
Reading through patch 14/14 it is unclear to me how you control to which
partition you write TF-A, SPL, U-Boot depending on which of these exists
on the MTD device.
Wouldn't it make more sense to encode in the capsule to where it will be
written?
Best regards
Heinrich
>
> -sughosh
>
>
> Best regards
>
> Heinrich
>
> > ---
> >? ?board/emulation/qemu-arm/qemu-arm.c | 55
> +++++++++++++++++++++++++++++
> >? ?include/configs/qemu-arm.h? ? ? ? ? |? 1 +
> >? ?2 files changed, 56 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index d5ed3eebaf..8cad54c76f 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> >
> >? ?#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> >
> > +#include <memalign.h>
> >? ?#include <mtd.h>
> >
> > +#define MTDPARTS_LEN? ? ? ? ?256
> > +#define MTDIDS_LEN? ? ? ? ? ?128
> > +
> > +#define DFU_ALT_BUF_LEN? ? ? ? ? ? ? SZ_1K
> > +
> > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > +{
> > +? ? ?struct mtd_info *part;
> > +? ? ?bool first = true;
> > +? ? ?const char *name;
> > +? ? ?int len, partnum = 0;
> > +
> > +? ? ?name = mtd->name;
> > +? ? ?len = strlen(buf);
> > +
> > +? ? ?if (buf[0] != '\0')
> > +? ? ? ? ? ? ?len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > +? ? ?len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > +? ? ? ? ? ? ? ? ? ? ?"mtd %s=", name);
> > +
> > +? ? ?list_for_each_entry(part, &mtd->partitions, node) {
> > +? ? ? ? ? ? ?partnum++;
> > +? ? ? ? ? ? ?if (!first)
> > +? ? ? ? ? ? ? ? ? ? ?len += snprintf(buf + len, DFU_ALT_BUF_LEN
> - len, ";");
> > +? ? ? ? ? ? ?first = false;
> > +
> > +? ? ? ? ? ? ?len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"%s part %d",
> > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ?part->name, partnum);
> > +? ? ?}
> > +}
> > +
> > +void set_dfu_alt_info(char *interface, char *devstr)
> > +{
> > +? ? ?struct mtd_info *mtd;
> > +
> > +? ? ?ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > +
> > +? ? ?if (env_get("dfu_alt_info"))
> > +? ? ? ? ? ? ?return;
> > +
> > +? ? ?memset(buf, 0, sizeof(buf));
> > +
> > +? ? ?/* probe all MTD devices */
> > +? ? ?mtd_probe_devices();
> > +
> > +? ? ?mtd = get_mtd_device_nm("nor0");
> > +? ? ?if (!IS_ERR_OR_NULL(mtd))
> > +? ? ? ? ? ? ?board_get_alt_info(mtd, buf);
> > +
> > +? ? ?env_set("dfu_alt_info", buf);
> > +? ? ?printf("dfu_alt_info set\n");
> > +}
> > +
> >? ?static void board_get_mtdparts(const char *dev, const char
> *partition,
> >? ? ? ? ? ? ? ? ? ? ? ? ? ? ? char *mtdids, char *mtdparts)
> >? ?{
> > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > index 69ff329434..726f985d35 100644
> > --- a/include/configs/qemu-arm.h
> > +++ b/include/configs/qemu-arm.h
> > @@ -33,6 +33,7 @@
> >? ?#include <config_distro_bootcmd.h>
> >
> >? ?#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +#define CONFIG_SET_DFU_ALT_INFO
> >? ?#define CONFIG_SYS_MTDPARTS_RUNTIME
> >? ?#endif
> >
> >
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-07 6:56 ` Heinrich Schuchardt
@ 2020-12-07 7:45 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-07 7:45 UTC (permalink / raw)
To: u-boot
On Mon, 7 Dec 2020 at 12:26, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 12/7/20 6:42 AM, Sughosh Ganu wrote:
> >
> > On Sat, 5 Dec 2020 at 16:01, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > > The dfu framework uses the dfu_alt_info environment variable to
> get
> > > information that is needed for performing the firmware update.
> > Set the
> > > dfu_alt_info for the platform to reflect the two mtd partitions
> > > created for the u-boot env and the firmware image.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> > <mailto:sughosh.ganu@linaro.org>>
> >
> > I can't see anything QEMU specific in this patch. Why is the code
> under
> > board/emulation/?
> >
> >
> > The device that the board wants to use to set the dfu_alt_info to would
> > be very much board specific. For example, the dfu_alt_info is being set
> > for the qemu platform for an mtd as the interface, with nor0 as the
> > device. Different platforms might have different requirements, like the
> > setting of the variable done for st
> > platforms(board/st/common/stm32mp_dfu.c). I think this information is
> > very much platform specific.
>
> In the function below you simply collect MTD partitions. Isn't this
> something that could be reused for other boards?
>
The function probes all mtd devices, but uses only the nor0 device for
setting up of the dfu_alt_info variable. The other nor1 device is not used,
since, on this platform configuration(non-secure boot), it has the u-boot
environment. So which of the mtd devices to use for setting of the
dfu_alt_info is very much platform specific.
> On other boards (e.g. board/samsung/common/misc.c)
> CONFIG_SET_DFU_ALT_INFO controls if set_dfu_alt_info() exists. Should
> the same be done for QEMU ARM?
>
Yes, I can add a check for this on the lines that you mention. Will do.
>
> Reading through patch 14/14 it is unclear to me how you control to which
> partition you write TF-A, SPL, U-Boot depending on which of these exists
> on the MTD device.
>
This gets added in patch 05/14. This sets the mtd partitions that are to be
set for the platform. On this particular configuration of the
platform(non-secure boot), the partition 1 is used for the u-boot image,
and the partition 2 is used for the u-boot environment.
> Wouldn't it make more sense to encode in the capsule to where it will be
> written?
>
We have discussed this earlier[1] that there is no way to encode the target
device and partition information as part of the capsule, the way the
capsule structure is defined by the uefi specification. That is the reason
that it was decided to use the dfu backend for the update using the
dfu_alt_info variable to indicate the partitions and device to be used for
the update.
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2020-May/411236.html
> Best regards
>
> Heinrich
>
> >
> > -sughosh
> >
> >
> > Best regards
> >
> > Heinrich
> >
> > > ---
> > > board/emulation/qemu-arm/qemu-arm.c | 55
> > +++++++++++++++++++++++++++++
> > > include/configs/qemu-arm.h | 1 +
> > > 2 files changed, 56 insertions(+)
> > >
> > > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> > b/board/emulation/qemu-arm/qemu-arm.c
> > > index d5ed3eebaf..8cad54c76f 100644
> > > --- a/board/emulation/qemu-arm/qemu-arm.c
> > > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> > >
> > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > >
> > > +#include <memalign.h>
> > > #include <mtd.h>
> > >
> > > +#define MTDPARTS_LEN 256
> > > +#define MTDIDS_LEN 128
> > > +
> > > +#define DFU_ALT_BUF_LEN SZ_1K
> > > +
> > > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > > +{
> > > + struct mtd_info *part;
> > > + bool first = true;
> > > + const char *name;
> > > + int len, partnum = 0;
> > > +
> > > + name = mtd->name;
> > > + len = strlen(buf);
> > > +
> > > + if (buf[0] != '\0')
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> "&");
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > + "mtd %s=", name);
> > > +
> > > + list_for_each_entry(part, &mtd->partitions, node) {
> > > + partnum++;
> > > + if (!first)
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN
> > - len, ";");
> > > + first = false;
> > > +
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > + "%s part %d",
> > > + part->name, partnum);
> > > + }
> > > +}
> > > +
> > > +void set_dfu_alt_info(char *interface, char *devstr)
> > > +{
> > > + struct mtd_info *mtd;
> > > +
> > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > > +
> > > + if (env_get("dfu_alt_info"))
> > > + return;
> > > +
> > > + memset(buf, 0, sizeof(buf));
> > > +
> > > + /* probe all MTD devices */
> > > + mtd_probe_devices();
> > > +
> > > + mtd = get_mtd_device_nm("nor0");
> > > + if (!IS_ERR_OR_NULL(mtd))
> > > + board_get_alt_info(mtd, buf);
> > > +
> > > + env_set("dfu_alt_info", buf);
> > > + printf("dfu_alt_info set\n");
> > > +}
> > > +
> > > static void board_get_mtdparts(const char *dev, const char
> > *partition,
> > > char *mtdids, char *mtdparts)
> > > {
> > > diff --git a/include/configs/qemu-arm.h
> b/include/configs/qemu-arm.h
> > > index 69ff329434..726f985d35 100644
> > > --- a/include/configs/qemu-arm.h
> > > +++ b/include/configs/qemu-arm.h
> > > @@ -33,6 +33,7 @@
> > > #include <config_distro_bootcmd.h>
> > >
> > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > +#define CONFIG_SET_DFU_ALT_INFO
> > > #define CONFIG_SYS_MTDPARTS_RUNTIME
> > > #endif
> > >
> > >
> >
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-07 5:15 ` Sughosh Ganu
@ 2020-12-07 12:50 ` Heinrich Schuchardt
2020-12-07 17:58 ` Heinrich Schuchardt
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-07 12:50 UTC (permalink / raw)
To: u-boot
On 07.12.20 06:15, Sughosh Ganu wrote:
> hello Heinrich,
>
> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > The Qemu platform emulator generates a device tree blob and places it
> > at the start of the dram, which is then used by u-boot. Use this dtb
> > only if CONFIG_OF_BOARD is defined. This allows using a different
> > device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
> > to the u-boot binary as a u-boot-fdt.bin file
>
> Dear Sughosh,
>
> thank your for this series which will allow us to better demonstrate and
> test capsule updates.
>
> I am not sure if the approach that you take at device-trees here is the
> right one.
>
> On QEMU the device-tree is generated on the fly by the QEMU binary
> depending on which devices the user has specified.
>
> Your idea is to replace this device-tree completely to be able to add
> extra elements (the EFI signature list, see patch 2/14). Thus a
> device-tree might be loaded that does not match the user selected
> devices.
>
> An alternative approach would be to apply all additions to the
> device-tree as an FDT overlay (or fixup). This would allow the dynamic
> parts of the QEMU device-tree still to be passed through.
>
>
> I will take a look at storing the public key as part of the fdt overlay,
> with a runtime fixup. Although, I think the issue that you are pointing
> to would be specific to Qemu and not other platforms. But I do see the
> merit in having the public-key certificate stored as part of an overlay.
> If I hit any issues while implementing this, I will get back to you. Thanks.
>
> -sughosh
OpenSBI can supply a device-tree to U-Boot. So this would be an
equivalent case.
Best regards
Heinrich
>
>
> Could you, please, assess the pros and cons of the two approaches with
> respect to:
>
> * usability for capsule updates
> * applicability for non-QEMU systems
> * integration of DTB overlays with FIT images for other use cases
>
> Best regards
>
> Heinrich
>
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> <mailto:sughosh.ganu@linaro.org>>
> > ---
> >? ?board/emulation/qemu-arm/qemu-arm.c | 2 ++
> >? ?1 file changed, 2 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index f18f2ed7da..e146d1cc50 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -89,11 +89,13 @@ int dram_init_banksize(void)
> >? ? ? ?return 0;
> >? ?}
> >
> > +#if defined(CONFIG_OF_BOARD)
> >? ?void *board_fdt_blob_setup(void)
> >? ?{
> >? ? ? ?/* QEMU loads a generated DTB for us at the start of RAM. */
> >? ? ? ?return (void *)CONFIG_SYS_SDRAM_BASE;
> >? ?}
> > +#endif
> >
> >? ?void enable_caches(void)
> >? ?{
> >
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-07 12:50 ` Heinrich Schuchardt
@ 2020-12-07 17:58 ` Heinrich Schuchardt
2020-12-08 5:28 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-07 17:58 UTC (permalink / raw)
To: u-boot
On 07.12.20 13:50, Heinrich Schuchardt wrote:
> On 07.12.20 06:15, Sughosh Ganu wrote:
>> hello Heinrich,
>>
>> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt <xypron.glpk@gmx.de
>> <mailto:xypron.glpk@gmx.de>> wrote:
>>
>> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
>> > The Qemu platform emulator generates a device tree blob and places it
>> > at the start of the dram, which is then used by u-boot. Use this dtb
>> > only if CONFIG_OF_BOARD is defined. This allows using a different
>> > device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
>> > to the u-boot binary as a u-boot-fdt.bin file
>>
>> Dear Sughosh,
>>
>> thank your for this series which will allow us to better demonstrate and
>> test capsule updates.
>>
>> I am not sure if the approach that you take at device-trees here is the
>> right one.
>>
>> On QEMU the device-tree is generated on the fly by the QEMU binary
>> depending on which devices the user has specified.
>>
>> Your idea is to replace this device-tree completely to be able to add
>> extra elements (the EFI signature list, see patch 2/14). Thus a
>> device-tree might be loaded that does not match the user selected
>> devices.
>>
>> An alternative approach would be to apply all additions to the
>> device-tree as an FDT overlay (or fixup). This would allow the dynamic
>> parts of the QEMU device-tree still to be passed through.
>>
>>
>> I will take a look at storing the public key as part of the fdt overlay,
>> with a runtime fixup. Although, I think the issue that you are pointing
>> to would be specific to Qemu and not other platforms. But I do see the
>> merit in having the public-key certificate stored as part of an overlay.
>> If I hit any issues while implementing this, I will get back to you. Thanks.
>>
>> -sughosh
>
> OpenSBI can supply a device-tree to U-Boot. So this would be an
> equivalent case.
>
> Best regards
>
> Heinrich
<sng_>: "I am unable to think of a simple and elegant way to generate
the overlay dtb, since this would require creation of a corresponding
dts file, compiling it into a dtb and then using this on the platform."
Jean-Jacques' patch series introduced some of the necessary code:
[PATCH PATCH v6 00/13] Add support for applications of overlays in SPL
https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be compiled.
You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
CONFIG_SPL_LOAD_FIT dependency.
Best regards
Heinrich
>
>>
>>
>> Could you, please, assess the pros and cons of the two approaches with
>> respect to:
>>
>> * usability for capsule updates
>> * applicability for non-QEMU systems
>> * integration of DTB overlays with FIT images for other use cases
>>
>> Best regards
>>
>> Heinrich
>>
>>
>> >
>> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
>> <mailto:sughosh.ganu@linaro.org>>
>> > ---
>> >? ?board/emulation/qemu-arm/qemu-arm.c | 2 ++
>> >? ?1 file changed, 2 insertions(+)
>> >
>> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
>> b/board/emulation/qemu-arm/qemu-arm.c
>> > index f18f2ed7da..e146d1cc50 100644
>> > --- a/board/emulation/qemu-arm/qemu-arm.c
>> > +++ b/board/emulation/qemu-arm/qemu-arm.c
>> > @@ -89,11 +89,13 @@ int dram_init_banksize(void)
>> >? ? ? ?return 0;
>> >? ?}
>> >
>> > +#if defined(CONFIG_OF_BOARD)
>> >? ?void *board_fdt_blob_setup(void)
>> >? ?{
>> >? ? ? ?/* QEMU loads a generated DTB for us at the start of RAM. */
>> >? ? ? ?return (void *)CONFIG_SYS_SDRAM_BASE;
>> >? ?}
>> > +#endif
>> >
>> >? ?void enable_caches(void)
>> >? ?{
>> >
>>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform
2020-12-05 10:29 ` Heinrich Schuchardt
2020-12-07 5:30 ` Sughosh Ganu
@ 2020-12-07 18:44 ` Tom Rini
2020-12-08 5:12 ` Sughosh Ganu
1 sibling, 1 reply; 49+ messages in thread
From: Tom Rini @ 2020-12-07 18:44 UTC (permalink / raw)
To: u-boot
On Sat, Dec 05, 2020 at 11:29:58AM +0100, Heinrich Schuchardt wrote:
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > Add support for setting the default values for mtd partitions on the
> > platform for the nor flash. This would be used for updating the
> > firmware image using uefi capsule update with the dfu mtd backend
> > driver.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 70 +++++++++++++++++++++++++++++
> > include/configs/qemu-arm.h | 7 +++
> > 2 files changed, 77 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> > index b3d5b3d5c2..d5ed3eebaf 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
[snip]
> > +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +#define CONFIG_SYS_MTDPARTS_RUNTIME
> > +#endif
This symbol is in Kconfig and needs to be enabled that way.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201207/daae2f88/attachment.sig>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-05 10:31 ` Heinrich Schuchardt
2020-12-07 5:42 ` Sughosh Ganu
@ 2020-12-07 18:47 ` Tom Rini
2020-12-08 5:18 ` Sughosh Ganu
1 sibling, 1 reply; 49+ messages in thread
From: Tom Rini @ 2020-12-07 18:47 UTC (permalink / raw)
To: u-boot
On Sat, Dec 05, 2020 at 11:31:49AM +0100, Heinrich Schuchardt wrote:
> On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > The dfu framework uses the dfu_alt_info environment variable to get
> > information that is needed for performing the firmware update. Set the
> > dfu_alt_info for the platform to reflect the two mtd partitions
> > created for the u-boot env and the firmware image.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>
> I can't see anything QEMU specific in this patch. Why is the code under
> board/emulation/?
>
> Best regards
>
> Heinrich
>
> > ---
> > board/emulation/qemu-arm/qemu-arm.c | 55 +++++++++++++++++++++++++++++
> > include/configs/qemu-arm.h | 1 +
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> > index d5ed3eebaf..8cad54c76f 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> >
> > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> >
> > +#include <memalign.h>
> > #include <mtd.h>
> >
> > +#define MTDPARTS_LEN 256
> > +#define MTDIDS_LEN 128
> > +
> > +#define DFU_ALT_BUF_LEN SZ_1K
> > +
> > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > +{
> > + struct mtd_info *part;
> > + bool first = true;
> > + const char *name;
> > + int len, partnum = 0;
> > +
> > + name = mtd->name;
> > + len = strlen(buf);
> > +
> > + if (buf[0] != '\0')
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > + "mtd %s=", name);
> > +
> > + list_for_each_entry(part, &mtd->partitions, node) {
> > + partnum++;
> > + if (!first)
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, ";");
> > + first = false;
> > +
> > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > + "%s part %d",
> > + part->name, partnum);
> > + }
> > +}
> > +
> > +void set_dfu_alt_info(char *interface, char *devstr)
> > +{
> > + struct mtd_info *mtd;
> > +
> > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > +
> > + if (env_get("dfu_alt_info"))
> > + return;
> > +
> > + memset(buf, 0, sizeof(buf));
> > +
> > + /* probe all MTD devices */
> > + mtd_probe_devices();
> > +
> > + mtd = get_mtd_device_nm("nor0");
> > + if (!IS_ERR_OR_NULL(mtd))
> > + board_get_alt_info(mtd, buf);
> > +
> > + env_set("dfu_alt_info", buf);
> > + printf("dfu_alt_info set\n");
> > +}
> > +
> > static void board_get_mtdparts(const char *dev, const char *partition,
> > char *mtdids, char *mtdparts)
> > {
> > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > index 69ff329434..726f985d35 100644
> > --- a/include/configs/qemu-arm.h
> > +++ b/include/configs/qemu-arm.h
> > @@ -33,6 +33,7 @@
> > #include <config_distro_bootcmd.h>
> >
> > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > +#define CONFIG_SET_DFU_ALT_INFO
> > #define CONFIG_SYS_MTDPARTS_RUNTIME
First, CONFIG_SET_DFU_ALT_INFO is in Kconfig and needs to be enabled
that way. Second, typically we just set the information in the
environment part of the header. This is especially true if in both this
case and the previous patch to add mtdparts, we don't really have this
being dynamic? Or are we really expecting / supporting arbitrary sized
flash as this is qemu? Thanks.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201207/297e48b6/attachment.sig>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform
2020-12-07 18:44 ` Tom Rini
@ 2020-12-08 5:12 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-08 5:12 UTC (permalink / raw)
To: u-boot
On Tue, 8 Dec 2020 at 00:14, Tom Rini <trini@konsulko.com> wrote:
> On Sat, Dec 05, 2020 at 11:29:58AM +0100, Heinrich Schuchardt wrote:
> > On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > > Add support for setting the default values for mtd partitions on the
> > > platform for the nor flash. This would be used for updating the
> > > firmware image using uefi capsule update with the dfu mtd backend
> > > driver.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > > board/emulation/qemu-arm/qemu-arm.c | 70
> +++++++++++++++++++++++++++++
> > > include/configs/qemu-arm.h | 7 +++
> > > 2 files changed, 77 insertions(+)
> > >
> > > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > > index b3d5b3d5c2..d5ed3eebaf 100644
> > > --- a/board/emulation/qemu-arm/qemu-arm.c
> > > +++ b/board/emulation/qemu-arm/qemu-arm.c
> [snip]
> > > +#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > +#define CONFIG_SYS_MTDPARTS_RUNTIME
> > > +#endif
>
> This symbol is in Kconfig and needs to be enabled that way.
>
Will make the necessary change in the next version. Thanks.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-07 18:47 ` Tom Rini
@ 2020-12-08 5:18 ` Sughosh Ganu
2020-12-08 12:20 ` Tom Rini
0 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-08 5:18 UTC (permalink / raw)
To: u-boot
On Tue, 8 Dec 2020 at 00:17, Tom Rini <trini@konsulko.com> wrote:
> On Sat, Dec 05, 2020 at 11:31:49AM +0100, Heinrich Schuchardt wrote:
> > On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > > The dfu framework uses the dfu_alt_info environment variable to get
> > > information that is needed for performing the firmware update. Set the
> > > dfu_alt_info for the platform to reflect the two mtd partitions
> > > created for the u-boot env and the firmware image.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> >
> > I can't see anything QEMU specific in this patch. Why is the code under
> > board/emulation/?
> >
> > Best regards
> >
> > Heinrich
> >
> > > ---
> > > board/emulation/qemu-arm/qemu-arm.c | 55
> +++++++++++++++++++++++++++++
> > > include/configs/qemu-arm.h | 1 +
> > > 2 files changed, 56 insertions(+)
> > >
> > > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > > index d5ed3eebaf..8cad54c76f 100644
> > > --- a/board/emulation/qemu-arm/qemu-arm.c
> > > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> > >
> > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > >
> > > +#include <memalign.h>
> > > #include <mtd.h>
> > >
> > > +#define MTDPARTS_LEN 256
> > > +#define MTDIDS_LEN 128
> > > +
> > > +#define DFU_ALT_BUF_LEN SZ_1K
> > > +
> > > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > > +{
> > > + struct mtd_info *part;
> > > + bool first = true;
> > > + const char *name;
> > > + int len, partnum = 0;
> > > +
> > > + name = mtd->name;
> > > + len = strlen(buf);
> > > +
> > > + if (buf[0] != '\0')
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > + "mtd %s=", name);
> > > +
> > > + list_for_each_entry(part, &mtd->partitions, node) {
> > > + partnum++;
> > > + if (!first)
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> ";");
> > > + first = false;
> > > +
> > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > + "%s part %d",
> > > + part->name, partnum);
> > > + }
> > > +}
> > > +
> > > +void set_dfu_alt_info(char *interface, char *devstr)
> > > +{
> > > + struct mtd_info *mtd;
> > > +
> > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > > +
> > > + if (env_get("dfu_alt_info"))
> > > + return;
> > > +
> > > + memset(buf, 0, sizeof(buf));
> > > +
> > > + /* probe all MTD devices */
> > > + mtd_probe_devices();
> > > +
> > > + mtd = get_mtd_device_nm("nor0");
> > > + if (!IS_ERR_OR_NULL(mtd))
> > > + board_get_alt_info(mtd, buf);
> > > +
> > > + env_set("dfu_alt_info", buf);
> > > + printf("dfu_alt_info set\n");
> > > +}
> > > +
> > > static void board_get_mtdparts(const char *dev, const char
> *partition,
> > > char *mtdids, char *mtdparts)
> > > {
> > > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > > index 69ff329434..726f985d35 100644
> > > --- a/include/configs/qemu-arm.h
> > > +++ b/include/configs/qemu-arm.h
> > > @@ -33,6 +33,7 @@
> > > #include <config_distro_bootcmd.h>
> > >
> > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > +#define CONFIG_SET_DFU_ALT_INFO
> > > #define CONFIG_SYS_MTDPARTS_RUNTIME
>
> First, CONFIG_SET_DFU_ALT_INFO is in Kconfig and needs to be enabled
> that way.
Will change in the next version.
> Second, typically we just set the information in the
> environment part of the header. This is especially true if in both this
> case and the previous patch to add mtdparts, we don't really have this
> being dynamic? Or are we really expecting / supporting arbitrary sized
> flash as this is qemu? Thanks.
>
I am not sure I understand this. One thing that can be done is to move the
setting of the mtd partitions to the Kconfig file, on similar lines to what
is being done for the st platforms, e.g MTDPARTS_NOR0_TEE. I can move the
mtd partition definitions to the Kconfig file if that is what you are
asking for.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-07 17:58 ` Heinrich Schuchardt
@ 2020-12-08 5:28 ` Sughosh Ganu
2020-12-08 9:02 ` Heinrich Schuchardt
0 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-08 5:28 UTC (permalink / raw)
To: u-boot
On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > On 07.12.20 06:15, Sughosh Ganu wrote:
> >> hello Heinrich,
> >>
> >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt <xypron.glpk@gmx.de
> >> <mailto:xypron.glpk@gmx.de>> wrote:
> >>
> >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> >> > The Qemu platform emulator generates a device tree blob and
> places it
> >> > at the start of the dram, which is then used by u-boot. Use this
> dtb
> >> > only if CONFIG_OF_BOARD is defined. This allows using a different
> >> > device tree, using the CONFIG_OF_SEPARATE option. This dtb is
> attached
> >> > to the u-boot binary as a u-boot-fdt.bin file
> >>
> >> Dear Sughosh,
> >>
> >> thank your for this series which will allow us to better
> demonstrate and
> >> test capsule updates.
> >>
> >> I am not sure if the approach that you take at device-trees here is
> the
> >> right one.
> >>
> >> On QEMU the device-tree is generated on the fly by the QEMU binary
> >> depending on which devices the user has specified.
> >>
> >> Your idea is to replace this device-tree completely to be able to
> add
> >> extra elements (the EFI signature list, see patch 2/14). Thus a
> >> device-tree might be loaded that does not match the user selected
> >> devices.
> >>
> >> An alternative approach would be to apply all additions to the
> >> device-tree as an FDT overlay (or fixup). This would allow the
> dynamic
> >> parts of the QEMU device-tree still to be passed through.
> >>
> >>
> >> I will take a look at storing the public key as part of the fdt overlay,
> >> with a runtime fixup. Although, I think the issue that you are pointing
> >> to would be specific to Qemu and not other platforms. But I do see the
> >> merit in having the public-key certificate stored as part of an overlay.
> >> If I hit any issues while implementing this, I will get back to you.
> Thanks.
> >>
> >> -sughosh
> >
> > OpenSBI can supply a device-tree to U-Boot. So this would be an
> > equivalent case.
> >
> > Best regards
> >
> > Heinrich
>
> <sng_>: "I am unable to think of a simple and elegant way to generate
> the overlay dtb, since this would require creation of a corresponding
> dts file, compiling it into a dtb and then using this on the platform."
>
> Jean-Jacques' patch series introduced some of the necessary code:
>
> [PATCH PATCH v6 00/13] Add support for applications of overlays in SPL
> https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
>
> CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be compiled.
> You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> CONFIG_SPL_LOAD_FIT dependency.
What i meant was that the process to generate the fdt overlay on the host,
embed it with the public-key certificate is more cumbersome than the
current solution. So, for comparing the embedding the pub-key cert in the
fdt overlay, as against the platform dtb
Embedding the certificate in the overlay
1) Generate a skeleton overlay dts file
2) Convert it to a dtb
3) Run it through the mkeficapsule utility to embed the pub-key certificate
in the overlay(dtb)
4) Store the overlay dtb on some nv storage on the platform(ESP partition?)
5) Load the overlay and apply it to the platform's dtb
6) Retrieve the certificate from the dtb at the time of capsule
authentication
Embedding the certificate in the platform's dtb
1) Run the platform's dtb through the mkeficapsule utility to embed
the pub-key certificate
2) Boot the platform with the platform's dtb
3) Retrieve the certificate from the dtb at the time of capsule
authentication
You had mentioned OpenSBI passing the dtb to u-boot. Does the OpenSBI
generate the device tree for the platform on the fly even for non-qemu
platforms. If it does not, the dtb that OpenSBI uses can be run through the
mkeficapsule utility to embed the certificate.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-08 5:28 ` Sughosh Ganu
@ 2020-12-08 9:02 ` Heinrich Schuchardt
2020-12-08 9:19 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-08 9:02 UTC (permalink / raw)
To: u-boot
On 08.12.20 06:28, Sughosh Ganu wrote:
>
> On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > On 07.12.20 06:15, Sughosh Ganu wrote:
> >> hello Heinrich,
> >>
> >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> >>
> >>? ? ?On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> >>? ? ?> The Qemu platform emulator generates a device tree blob and
> places it
> >>? ? ?> at the start of the dram, which is then used by u-boot. Use
> this dtb
> >>? ? ?> only if CONFIG_OF_BOARD is defined. This allows using a
> different
> >>? ? ?> device tree, using the CONFIG_OF_SEPARATE option. This dtb
> is attached
> >>? ? ?> to the u-boot binary as a u-boot-fdt.bin file
> >>
> >>? ? ?Dear Sughosh,
> >>
> >>? ? ?thank your for this series which will allow us to better
> demonstrate and
> >>? ? ?test capsule updates.
> >>
> >>? ? ?I am not sure if the approach that you take at device-trees
> here is the
> >>? ? ?right one.
> >>
> >>? ? ?On QEMU the device-tree is generated on the fly by the QEMU
> binary
> >>? ? ?depending on which devices the user has specified.
> >>
> >>? ? ?Your idea is to replace this device-tree completely to be
> able to add
> >>? ? ?extra elements (the EFI signature list, see patch 2/14). Thus a
> >>? ? ?device-tree might be loaded that does not match the user selected
> >>? ? ?devices.
> >>
> >>? ? ?An alternative approach would be to apply all additions to the
> >>? ? ?device-tree as an FDT overlay (or fixup). This would allow
> the dynamic
> >>? ? ?parts of the QEMU device-tree still to be passed through.
> >>
> >>
> >> I will take a look at storing the public key as part of the fdt
> overlay,
> >> with a runtime fixup. Although, I think the issue that you are
> pointing
> >> to would be specific to Qemu and not other platforms. But I do
> see the
> >> merit in having the public-key certificate stored as part of an
> overlay.
> >> If I hit any issues while implementing this, I will get back to
> you. Thanks.
> >>
> >> -sughosh
> >
> > OpenSBI can supply a device-tree to U-Boot. So this would be an
> > equivalent case.
> >
> > Best regards
> >
> > Heinrich
>
> <sng_>: "I am unable to think of a simple and elegant way to generate
> the overlay dtb, since this would require creation of a corresponding
> dts file, compiling it into a dtb and then using this on the platform."
>
> Jean-Jacques' patch series introduced some of the necessary code:
>
> [PATCH PATCH v6 00/13] Add support for applications of overlays in SPL
> https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>
>
> CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be compiled.
> You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> CONFIG_SPL_LOAD_FIT dependency.
>
>
> What i meant was that the process to generate the fdt overlay on the
> host, embed it with the public-key certificate is more cumbersome than
> the current solution. So, for comparing the embedding the pub-key cert
> in the fdt overlay, as against the platform dtb
>
> Embedding the certificate in the overlay
> 1) Generate a skeleton overlay dts file
> 2) Convert it to a dtb
> 3) Run it through the mkeficapsule utility to embed the pub-key
> certificate in the overlay(dtb)
> 4) Store the overlay dtb on some nv storage on the platform(ESP partition?)
> 5) Load the overlay and apply it to the platform's dtb
> 6) Retrieve the certificate from the dtb at the time of capsule
> authentication
>
> Embedding the certificate in the platform's dtb
> 1) Run the platform's dtb through the mkeficapsule utility to embed
> the?pub-key certificate
> 2) Boot the platform with the platform's dtb
> 3) Retrieve the certificate from the dtb at the time of capsule
> authentication
>
> You had mentioned OpenSBI passing the dtb to u-boot. Does the OpenSBI
> generate the device tree for the platform on the fly even for non-qemu
> platforms.?If it does not, the dtb that OpenSBI uses can be run through
> the mkeficapsule utility to embed the certificate.
OpenSBI applies fix-ups before passing the device-tree. The prototype
device-tree can either be built into OpenSBI or can be passed from an
earlier firmware stage.
Best regards
Heinrich
>
> -sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-08 9:02 ` Heinrich Schuchardt
@ 2020-12-08 9:19 ` Sughosh Ganu
2020-12-08 21:54 ` Heinrich Schuchardt
0 siblings, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-08 9:19 UTC (permalink / raw)
To: u-boot
On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08.12.20 06:28, Sughosh Ganu wrote:
> >
> > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > > On 07.12.20 06:15, Sughosh Ganu wrote:
> > >> hello Heinrich,
> > >>
> > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> > >>
> > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > >> > The Qemu platform emulator generates a device tree blob and
> > places it
> > >> > at the start of the dram, which is then used by u-boot. Use
> > this dtb
> > >> > only if CONFIG_OF_BOARD is defined. This allows using a
> > different
> > >> > device tree, using the CONFIG_OF_SEPARATE option. This dtb
> > is attached
> > >> > to the u-boot binary as a u-boot-fdt.bin file
> > >>
> > >> Dear Sughosh,
> > >>
> > >> thank your for this series which will allow us to better
> > demonstrate and
> > >> test capsule updates.
> > >>
> > >> I am not sure if the approach that you take at device-trees
> > here is the
> > >> right one.
> > >>
> > >> On QEMU the device-tree is generated on the fly by the QEMU
> > binary
> > >> depending on which devices the user has specified.
> > >>
> > >> Your idea is to replace this device-tree completely to be
> > able to add
> > >> extra elements (the EFI signature list, see patch 2/14). Thus
> a
> > >> device-tree might be loaded that does not match the user
> selected
> > >> devices.
> > >>
> > >> An alternative approach would be to apply all additions to the
> > >> device-tree as an FDT overlay (or fixup). This would allow
> > the dynamic
> > >> parts of the QEMU device-tree still to be passed through.
> > >>
> > >>
> > >> I will take a look at storing the public key as part of the fdt
> > overlay,
> > >> with a runtime fixup. Although, I think the issue that you are
> > pointing
> > >> to would be specific to Qemu and not other platforms. But I do
> > see the
> > >> merit in having the public-key certificate stored as part of an
> > overlay.
> > >> If I hit any issues while implementing this, I will get back to
> > you. Thanks.
> > >>
> > >> -sughosh
> > >
> > > OpenSBI can supply a device-tree to U-Boot. So this would be an
> > > equivalent case.
> > >
> > > Best regards
> > >
> > > Heinrich
> >
> > <sng_>: "I am unable to think of a simple and elegant way to generate
> > the overlay dtb, since this would require creation of a corresponding
> > dts file, compiling it into a dtb and then using this on the
> platform."
> >
> > Jean-Jacques' patch series introduced some of the necessary code:
> >
> > [PATCH PATCH v6 00/13] Add support for applications of overlays in
> SPL
> > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> >
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >
> >
> > CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be
> compiled.
> > You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> > CONFIG_SPL_LOAD_FIT dependency.
> >
> >
> > What i meant was that the process to generate the fdt overlay on the
> > host, embed it with the public-key certificate is more cumbersome than
> > the current solution. So, for comparing the embedding the pub-key cert
> > in the fdt overlay, as against the platform dtb
> >
> > Embedding the certificate in the overlay
> > 1) Generate a skeleton overlay dts file
> > 2) Convert it to a dtb
> > 3) Run it through the mkeficapsule utility to embed the pub-key
> > certificate in the overlay(dtb)
> > 4) Store the overlay dtb on some nv storage on the platform(ESP
> partition?)
> > 5) Load the overlay and apply it to the platform's dtb
> > 6) Retrieve the certificate from the dtb at the time of capsule
> > authentication
> >
> > Embedding the certificate in the platform's dtb
> > 1) Run the platform's dtb through the mkeficapsule utility to embed
> > the pub-key certificate
> > 2) Boot the platform with the platform's dtb
> > 3) Retrieve the certificate from the dtb at the time of capsule
> > authentication
> >
> > You had mentioned OpenSBI passing the dtb to u-boot. Does the OpenSBI
> > generate the device tree for the platform on the fly even for non-qemu
> > platforms. If it does not, the dtb that OpenSBI uses can be run through
> > the mkeficapsule utility to embed the certificate.
>
> OpenSBI applies fix-ups before passing the device-tree. The prototype
> device-tree can either be built into OpenSBI or can be passed from an
> earlier firmware stage.
So, if qemu if the only platform where the device tree is generated on the
fly, and passed along, based on the comparison that i have stated above for
the two scenarios(overlay vs platform dtb), I feel that using the
platform's dtb and embedding the certificate is more easier to use than
using the overlay.
Even on the qemu platform, I retrieved the dtb from the platform, for the
given set of devices and interfaces used, and then ran the dtb through the
mkeficapsule utility.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-08 5:18 ` Sughosh Ganu
@ 2020-12-08 12:20 ` Tom Rini
2020-12-08 17:03 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Tom Rini @ 2020-12-08 12:20 UTC (permalink / raw)
To: u-boot
On Tue, Dec 08, 2020 at 10:48:57AM +0530, Sughosh Ganu wrote:
> On Tue, 8 Dec 2020 at 00:17, Tom Rini <trini@konsulko.com> wrote:
>
> > On Sat, Dec 05, 2020 at 11:31:49AM +0100, Heinrich Schuchardt wrote:
> > > On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > > > The dfu framework uses the dfu_alt_info environment variable to get
> > > > information that is needed for performing the firmware update. Set the
> > > > dfu_alt_info for the platform to reflect the two mtd partitions
> > > > created for the u-boot env and the firmware image.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > >
> > > I can't see anything QEMU specific in this patch. Why is the code under
> > > board/emulation/?
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > ---
> > > > board/emulation/qemu-arm/qemu-arm.c | 55
> > +++++++++++++++++++++++++++++
> > > > include/configs/qemu-arm.h | 1 +
> > > > 2 files changed, 56 insertions(+)
> > > >
> > > > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> > b/board/emulation/qemu-arm/qemu-arm.c
> > > > index d5ed3eebaf..8cad54c76f 100644
> > > > --- a/board/emulation/qemu-arm/qemu-arm.c
> > > > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > > > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> > > >
> > > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > >
> > > > +#include <memalign.h>
> > > > #include <mtd.h>
> > > >
> > > > +#define MTDPARTS_LEN 256
> > > > +#define MTDIDS_LEN 128
> > > > +
> > > > +#define DFU_ALT_BUF_LEN SZ_1K
> > > > +
> > > > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > > > +{
> > > > + struct mtd_info *part;
> > > > + bool first = true;
> > > > + const char *name;
> > > > + int len, partnum = 0;
> > > > +
> > > > + name = mtd->name;
> > > > + len = strlen(buf);
> > > > +
> > > > + if (buf[0] != '\0')
> > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > > + "mtd %s=", name);
> > > > +
> > > > + list_for_each_entry(part, &mtd->partitions, node) {
> > > > + partnum++;
> > > > + if (!first)
> > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > ";");
> > > > + first = false;
> > > > +
> > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > > + "%s part %d",
> > > > + part->name, partnum);
> > > > + }
> > > > +}
> > > > +
> > > > +void set_dfu_alt_info(char *interface, char *devstr)
> > > > +{
> > > > + struct mtd_info *mtd;
> > > > +
> > > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > > > +
> > > > + if (env_get("dfu_alt_info"))
> > > > + return;
> > > > +
> > > > + memset(buf, 0, sizeof(buf));
> > > > +
> > > > + /* probe all MTD devices */
> > > > + mtd_probe_devices();
> > > > +
> > > > + mtd = get_mtd_device_nm("nor0");
> > > > + if (!IS_ERR_OR_NULL(mtd))
> > > > + board_get_alt_info(mtd, buf);
> > > > +
> > > > + env_set("dfu_alt_info", buf);
> > > > + printf("dfu_alt_info set\n");
> > > > +}
> > > > +
> > > > static void board_get_mtdparts(const char *dev, const char
> > *partition,
> > > > char *mtdids, char *mtdparts)
> > > > {
> > > > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > > > index 69ff329434..726f985d35 100644
> > > > --- a/include/configs/qemu-arm.h
> > > > +++ b/include/configs/qemu-arm.h
> > > > @@ -33,6 +33,7 @@
> > > > #include <config_distro_bootcmd.h>
> > > >
> > > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > > +#define CONFIG_SET_DFU_ALT_INFO
> > > > #define CONFIG_SYS_MTDPARTS_RUNTIME
> >
> > First, CONFIG_SET_DFU_ALT_INFO is in Kconfig and needs to be enabled
> > that way.
>
>
> Will change in the next version.
>
>
> > Second, typically we just set the information in the
> > environment part of the header. This is especially true if in both this
> > case and the previous patch to add mtdparts, we don't really have this
> > being dynamic? Or are we really expecting / supporting arbitrary sized
> > flash as this is qemu? Thanks.
> >
>
> I am not sure I understand this. One thing that can be done is to move the
> setting of the mtd partitions to the Kconfig file, on similar lines to what
> is being done for the st platforms, e.g MTDPARTS_NOR0_TEE. I can move the
> mtd partition definitions to the Kconfig file if that is what you are
> asking for.
Environment manipulation via C code is possible, but discouraged. What
can be set via Kconfig should be set that way, and what can be put in
the environment part of the header should be done that way. Does that
help clarify?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201208/f6a6772a/attachment.sig>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform
2020-12-08 12:20 ` Tom Rini
@ 2020-12-08 17:03 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-08 17:03 UTC (permalink / raw)
To: u-boot
On Tue, 8 Dec 2020 at 17:50, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Dec 08, 2020 at 10:48:57AM +0530, Sughosh Ganu wrote:
> > On Tue, 8 Dec 2020 at 00:17, Tom Rini <trini@konsulko.com> wrote:
> >
> > > On Sat, Dec 05, 2020 at 11:31:49AM +0100, Heinrich Schuchardt wrote:
> > > > On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> > > > > The dfu framework uses the dfu_alt_info environment variable to get
> > > > > information that is needed for performing the firmware update. Set
> the
> > > > > dfu_alt_info for the platform to reflect the two mtd partitions
> > > > > created for the u-boot env and the firmware image.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > >
> > > > I can't see anything QEMU specific in this patch. Why is the code
> under
> > > > board/emulation/?
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > > ---
> > > > > board/emulation/qemu-arm/qemu-arm.c | 55
> > > +++++++++++++++++++++++++++++
> > > > > include/configs/qemu-arm.h | 1 +
> > > > > 2 files changed, 56 insertions(+)
> > > > >
> > > > > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> > > b/board/emulation/qemu-arm/qemu-arm.c
> > > > > index d5ed3eebaf..8cad54c76f 100644
> > > > > --- a/board/emulation/qemu-arm/qemu-arm.c
> > > > > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > > > > @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
> > > > >
> > > > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > > >
> > > > > +#include <memalign.h>
> > > > > #include <mtd.h>
> > > > >
> > > > > +#define MTDPARTS_LEN 256
> > > > > +#define MTDIDS_LEN 128
> > > > > +
> > > > > +#define DFU_ALT_BUF_LEN SZ_1K
> > > > > +
> > > > > +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> > > > > +{
> > > > > + struct mtd_info *part;
> > > > > + bool first = true;
> > > > > + const char *name;
> > > > > + int len, partnum = 0;
> > > > > +
> > > > > + name = mtd->name;
> > > > > + len = strlen(buf);
> > > > > +
> > > > > + if (buf[0] != '\0')
> > > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> > > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > > > + "mtd %s=", name);
> > > > > +
> > > > > + list_for_each_entry(part, &mtd->partitions, node) {
> > > > > + partnum++;
> > > > > + if (!first)
> > > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN -
> len,
> > > ";");
> > > > > + first = false;
> > > > > +
> > > > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> > > > > + "%s part %d",
> > > > > + part->name, partnum);
> > > > > + }
> > > > > +}
> > > > > +
> > > > > +void set_dfu_alt_info(char *interface, char *devstr)
> > > > > +{
> > > > > + struct mtd_info *mtd;
> > > > > +
> > > > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > > > > +
> > > > > + if (env_get("dfu_alt_info"))
> > > > > + return;
> > > > > +
> > > > > + memset(buf, 0, sizeof(buf));
> > > > > +
> > > > > + /* probe all MTD devices */
> > > > > + mtd_probe_devices();
> > > > > +
> > > > > + mtd = get_mtd_device_nm("nor0");
> > > > > + if (!IS_ERR_OR_NULL(mtd))
> > > > > + board_get_alt_info(mtd, buf);
> > > > > +
> > > > > + env_set("dfu_alt_info", buf);
> > > > > + printf("dfu_alt_info set\n");
> > > > > +}
> > > > > +
> > > > > static void board_get_mtdparts(const char *dev, const char
> > > *partition,
> > > > > char *mtdids, char *mtdparts)
> > > > > {
> > > > > diff --git a/include/configs/qemu-arm.h
> b/include/configs/qemu-arm.h
> > > > > index 69ff329434..726f985d35 100644
> > > > > --- a/include/configs/qemu-arm.h
> > > > > +++ b/include/configs/qemu-arm.h
> > > > > @@ -33,6 +33,7 @@
> > > > > #include <config_distro_bootcmd.h>
> > > > >
> > > > > #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> > > > > +#define CONFIG_SET_DFU_ALT_INFO
> > > > > #define CONFIG_SYS_MTDPARTS_RUNTIME
> > >
> > > First, CONFIG_SET_DFU_ALT_INFO is in Kconfig and needs to be enabled
> > > that way.
> >
> >
> > Will change in the next version.
> >
> >
> > > Second, typically we just set the information in the
> > > environment part of the header. This is especially true if in both
> this
> > > case and the previous patch to add mtdparts, we don't really have this
> > > being dynamic? Or are we really expecting / supporting arbitrary sized
> > > flash as this is qemu? Thanks.
> > >
> >
> > I am not sure I understand this. One thing that can be done is to move
> the
> > setting of the mtd partitions to the Kconfig file, on similar lines to
> what
> > is being done for the st platforms, e.g MTDPARTS_NOR0_TEE. I can move the
> > mtd partition definitions to the Kconfig file if that is what you are
> > asking for.
>
> Environment manipulation via C code is possible, but discouraged. What
> can be set via Kconfig should be set that way, and what can be put in
> the environment part of the header should be done that way. Does that
> help clarify?
>
Yes, that does clarify. I will move the settings of the partitions used for
mtdparts in the Kconfig, similar to the way it is being done for the st
platforms. Thanks.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-08 9:19 ` Sughosh Ganu
@ 2020-12-08 21:54 ` Heinrich Schuchardt
2020-12-09 5:25 ` Sughosh Ganu
2020-12-15 11:10 ` Sughosh Ganu
0 siblings, 2 replies; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-08 21:54 UTC (permalink / raw)
To: u-boot
On 12/8/20 10:19 AM, Sughosh Ganu wrote:
>
> On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 08.12.20 06:28, Sughosh Ganu wrote:
> >
> > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> >
> >? ? ?On 07.12.20 13:50, Heinrich Schuchardt wrote:
> >? ? ?> On 07.12.20 06:15, Sughosh Ganu wrote:
> >? ? ?>> hello Heinrich,
> >? ? ?>>
> >? ? ?>> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> >? ? ?<xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> >? ? ?>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> >? ? ?>>
> >? ? ?>>? ? ?On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> >? ? ?>>? ? ?> The Qemu platform emulator generates a device tree
> blob and
> >? ? ?places it
> >? ? ?>>? ? ?> at the start of the dram, which is then used by
> u-boot. Use
> >? ? ?this dtb
> >? ? ?>>? ? ?> only if CONFIG_OF_BOARD is defined. This allows using a
> >? ? ?different
> >? ? ?>>? ? ?> device tree, using the CONFIG_OF_SEPARATE option.
> This dtb
> >? ? ?is attached
> >? ? ?>>? ? ?> to the u-boot binary as a u-boot-fdt.bin file
> >? ? ?>>
> >? ? ?>>? ? ?Dear Sughosh,
> >? ? ?>>
> >? ? ?>>? ? ?thank your for this series which will allow us to better
> >? ? ?demonstrate and
> >? ? ?>>? ? ?test capsule updates.
> >? ? ?>>
> >? ? ?>>? ? ?I am not sure if the approach that you take at
> device-trees
> >? ? ?here is the
> >? ? ?>>? ? ?right one.
> >? ? ?>>
> >? ? ?>>? ? ?On QEMU the device-tree is generated on the fly by the
> QEMU
> >? ? ?binary
> >? ? ?>>? ? ?depending on which devices the user has specified.
> >? ? ?>>
> >? ? ?>>? ? ?Your idea is to replace this device-tree completely to be
> >? ? ?able to add
> >? ? ?>>? ? ?extra elements (the EFI signature list, see patch
> 2/14). Thus a
> >? ? ?>>? ? ?device-tree might be loaded that does not match the
> user selected
> >? ? ?>>? ? ?devices.
> >? ? ?>>
> >? ? ?>>? ? ?An alternative approach would be to apply all
> additions to the
> >? ? ?>>? ? ?device-tree as an FDT overlay (or fixup). This would allow
> >? ? ?the dynamic
> >? ? ?>>? ? ?parts of the QEMU device-tree still to be passed through.
> >? ? ?>>
> >? ? ?>>
> >? ? ?>> I will take a look at storing the public key as part of
> the fdt
> >? ? ?overlay,
> >? ? ?>> with a runtime fixup. Although, I think the issue that you are
> >? ? ?pointing
> >? ? ?>> to would be specific to Qemu and not other platforms. But I do
> >? ? ?see the
> >? ? ?>> merit in having the public-key certificate stored as part
> of an
> >? ? ?overlay.
> >? ? ?>> If I hit any issues while implementing this, I will get
> back to
> >? ? ?you. Thanks.
> >? ? ?>>
> >? ? ?>> -sughosh
> >? ? ?>
> >? ? ?> OpenSBI can supply a device-tree to U-Boot. So this would be an
> >? ? ?> equivalent case.
> >? ? ?>
> >? ? ?> Best regards
> >? ? ?>
> >? ? ?> Heinrich
> >
> >? ? ?<sng_>: "I am unable to think of a simple and elegant way to
> generate
> >? ? ?the overlay dtb, since this would require creation of a
> corresponding
> >? ? ?dts file, compiling it into a dtb and then using this on the
> platform."
> >
> >? ? ?Jean-Jacques' patch series introduced some of the necessary code:
> >
> >? ? ?[PATCH PATCH v6 00/13] Add support for applications of
> overlays in SPL
> > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> >
> ?<https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
> >
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>
> >
> ?<https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>>
> >
> >? ? ?CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be
> compiled.
> >? ? ?You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> >? ? ?CONFIG_SPL_LOAD_FIT dependency.
> >
> >
> > What i meant was that the process to generate the fdt overlay on the
> > host, embed it with the public-key certificate is more cumbersome
> than
> > the current solution. So, for comparing the embedding the pub-key
> cert
> > in the fdt overlay, as against the platform dtb
> >
> > Embedding the certificate in the overlay
> > 1) Generate a skeleton overlay dts file
> > 2) Convert it to a dtb
> > 3) Run it through the mkeficapsule utility to embed the pub-key
> > certificate in the overlay(dtb)
> > 4) Store the overlay dtb on some nv storage on the platform(ESP
> partition?)
> > 5) Load the overlay and apply it to the platform's dtb
> > 6) Retrieve the certificate from the dtb at the time of capsule
> > authentication
> >
> > Embedding the certificate in the platform's dtb
> > 1) Run the platform's dtb through the mkeficapsule utility to embed
> > the?pub-key certificate
> > 2) Boot the platform with the platform's dtb
> > 3) Retrieve the certificate from the dtb at the time of capsule
> > authentication
> >
> > You had mentioned OpenSBI passing the dtb to u-boot. Does the OpenSBI
> > generate the device tree for the platform on the fly even for
> non-qemu
> > platforms.?If it does not, the dtb that OpenSBI uses can be run
> through
> > the mkeficapsule utility to embed the certificate.
>
> OpenSBI applies fix-ups before passing the device-tree. The prototype
> device-tree can either be built into OpenSBI or can be passed from an
> earlier firmware stage.
>
>
> So, if qemu if the only platform where the device tree is generated on
> the fly, and passed along, based on the comparison that i have stated
> above for the two scenarios(overlay vs platform dtb), I feel that using
> the platform's dtb and embedding the certificate is more easier to use
> than using the overlay.
>
> Even on the qemu platform, I retrieved the dtb from the platform, for
> the given set of devices and interfaces used, and then ran the dtb
> through the mkeficapsule utility.
>
> -sughosh
As I understand the whole series is not about productive firmware
updates for QEMU but about a demonstration how it could be used and
allowing testing.
Wouldn't it be the enough for testing to apply an overlay with the
public key using the fdt command?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-08 21:54 ` Heinrich Schuchardt
@ 2020-12-09 5:25 ` Sughosh Ganu
2020-12-09 7:26 ` Heinrich Schuchardt
2020-12-15 11:10 ` Sughosh Ganu
1 sibling, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-09 5:25 UTC (permalink / raw)
To: u-boot
On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 12/8/20 10:19 AM, Sughosh Ganu wrote:
> >
> > On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 08.12.20 06:28, Sughosh Ganu wrote:
> > >
> > > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> > >
> > > On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > > > On 07.12.20 06:15, Sughosh Ganu wrote:
> > > >> hello Heinrich,
> > > >>
> > > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> > > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> > > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> > > >>
> > > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > > >> > The Qemu platform emulator generates a device tree
> > blob and
> > > places it
> > > >> > at the start of the dram, which is then used by
> > u-boot. Use
> > > this dtb
> > > >> > only if CONFIG_OF_BOARD is defined. This allows
> using a
> > > different
> > > >> > device tree, using the CONFIG_OF_SEPARATE option.
> > This dtb
> > > is attached
> > > >> > to the u-boot binary as a u-boot-fdt.bin file
> > > >>
> > > >> Dear Sughosh,
> > > >>
> > > >> thank your for this series which will allow us to
> better
> > > demonstrate and
> > > >> test capsule updates.
> > > >>
> > > >> I am not sure if the approach that you take at
> > device-trees
> > > here is the
> > > >> right one.
> > > >>
> > > >> On QEMU the device-tree is generated on the fly by the
> > QEMU
> > > binary
> > > >> depending on which devices the user has specified.
> > > >>
> > > >> Your idea is to replace this device-tree completely to
> be
> > > able to add
> > > >> extra elements (the EFI signature list, see patch
> > 2/14). Thus a
> > > >> device-tree might be loaded that does not match the
> > user selected
> > > >> devices.
> > > >>
> > > >> An alternative approach would be to apply all
> > additions to the
> > > >> device-tree as an FDT overlay (or fixup). This would
> allow
> > > the dynamic
> > > >> parts of the QEMU device-tree still to be passed
> through.
> > > >>
> > > >>
> > > >> I will take a look at storing the public key as part of
> > the fdt
> > > overlay,
> > > >> with a runtime fixup. Although, I think the issue that you
> are
> > > pointing
> > > >> to would be specific to Qemu and not other platforms. But
> I do
> > > see the
> > > >> merit in having the public-key certificate stored as part
> > of an
> > > overlay.
> > > >> If I hit any issues while implementing this, I will get
> > back to
> > > you. Thanks.
> > > >>
> > > >> -sughosh
> > > >
> > > > OpenSBI can supply a device-tree to U-Boot. So this would
> be an
> > > > equivalent case.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > >
> > > <sng_>: "I am unable to think of a simple and elegant way to
> > generate
> > > the overlay dtb, since this would require creation of a
> > corresponding
> > > dts file, compiling it into a dtb and then using this on the
> > platform."
> > >
> > > Jean-Jacques' patch series introduced some of the necessary
> code:
> > >
> > > [PATCH PATCH v6 00/13] Add support for applications of
> > overlays in SPL
> > > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> > >
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
> > >
> >
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >
> > >
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>
> > >
> > > CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be
> > compiled.
> > > You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY
> and
> > > CONFIG_SPL_LOAD_FIT dependency.
> > >
> > >
> > > What i meant was that the process to generate the fdt overlay on
> the
> > > host, embed it with the public-key certificate is more cumbersome
> > than
> > > the current solution. So, for comparing the embedding the pub-key
> > cert
> > > in the fdt overlay, as against the platform dtb
> > >
> > > Embedding the certificate in the overlay
> > > 1) Generate a skeleton overlay dts file
> > > 2) Convert it to a dtb
> > > 3) Run it through the mkeficapsule utility to embed the pub-key
> > > certificate in the overlay(dtb)
> > > 4) Store the overlay dtb on some nv storage on the platform(ESP
> > partition?)
> > > 5) Load the overlay and apply it to the platform's dtb
> > > 6) Retrieve the certificate from the dtb at the time of capsule
> > > authentication
> > >
> > > Embedding the certificate in the platform's dtb
> > > 1) Run the platform's dtb through the mkeficapsule utility to
> embed
> > > the pub-key certificate
> > > 2) Boot the platform with the platform's dtb
> > > 3) Retrieve the certificate from the dtb at the time of capsule
> > > authentication
> > >
> > > You had mentioned OpenSBI passing the dtb to u-boot. Does the
> OpenSBI
> > > generate the device tree for the platform on the fly even for
> > non-qemu
> > > platforms. If it does not, the dtb that OpenSBI uses can be run
> > through
> > > the mkeficapsule utility to embed the certificate.
> >
> > OpenSBI applies fix-ups before passing the device-tree. The prototype
> > device-tree can either be built into OpenSBI or can be passed from an
> > earlier firmware stage.
> >
> >
> > So, if qemu if the only platform where the device tree is generated on
> > the fly, and passed along, based on the comparison that i have stated
> > above for the two scenarios(overlay vs platform dtb), I feel that using
> > the platform's dtb and embedding the certificate is more easier to use
> > than using the overlay.
> >
> > Even on the qemu platform, I retrieved the dtb from the platform, for
> > the given set of devices and interfaces used, and then ran the dtb
> > through the mkeficapsule utility.
> >
> > -sughosh
>
> As I understand the whole series is not about productive firmware
> updates for QEMU but about a demonstration how it could be used and
> allowing testing.
>
True, but the process of embedding the public key certificate into the
platform's dtb for use in capsule authentication can be common for all
platforms. If you are fine, we can keep this patch to be used on other
non-qemu platforms, while i work on the overlay method for qemu. Will you
be fine with this approach.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-09 5:25 ` Sughosh Ganu
@ 2020-12-09 7:26 ` Heinrich Schuchardt
2020-12-09 8:26 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-09 7:26 UTC (permalink / raw)
To: u-boot
On 12/9/20 6:25 AM, Sughosh Ganu wrote:
> On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>> On 12/8/20 10:19 AM, Sughosh Ganu wrote:
>>>
>>> On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de
>>> <mailto:xypron.glpk@gmx.de>> wrote:
>>>
>>> On 08.12.20 06:28, Sughosh Ganu wrote:
>>> >
>>> > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
>>> <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
>>> >
>>> > On 07.12.20 13:50, Heinrich Schuchardt wrote:
>>> > > On 07.12.20 06:15, Sughosh Ganu wrote:
>>> > >> hello Heinrich,
>>> > >>
>>> > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
>>> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
>>> > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
>>> > >>
>>> > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
>>> > >> > The Qemu platform emulator generates a device tree
>>> blob and
>>> > places it
>>> > >> > at the start of the dram, which is then used by
>>> u-boot. Use
>>> > this dtb
>>> > >> > only if CONFIG_OF_BOARD is defined. This allows
>> using a
>>> > different
>>> > >> > device tree, using the CONFIG_OF_SEPARATE option.
>>> This dtb
>>> > is attached
>>> > >> > to the u-boot binary as a u-boot-fdt.bin file
>>> > >>
>>> > >> Dear Sughosh,
>>> > >>
>>> > >> thank your for this series which will allow us to
>> better
>>> > demonstrate and
>>> > >> test capsule updates.
>>> > >>
>>> > >> I am not sure if the approach that you take at
>>> device-trees
>>> > here is the
>>> > >> right one.
>>> > >>
>>> > >> On QEMU the device-tree is generated on the fly by the
>>> QEMU
>>> > binary
>>> > >> depending on which devices the user has specified.
>>> > >>
>>> > >> Your idea is to replace this device-tree completely to
>> be
>>> > able to add
>>> > >> extra elements (the EFI signature list, see patch
>>> 2/14). Thus a
>>> > >> device-tree might be loaded that does not match the
>>> user selected
>>> > >> devices.
>>> > >>
>>> > >> An alternative approach would be to apply all
>>> additions to the
>>> > >> device-tree as an FDT overlay (or fixup). This would
>> allow
>>> > the dynamic
>>> > >> parts of the QEMU device-tree still to be passed
>> through.
>>> > >>
>>> > >>
>>> > >> I will take a look at storing the public key as part of
>>> the fdt
>>> > overlay,
>>> > >> with a runtime fixup. Although, I think the issue that you
>> are
>>> > pointing
>>> > >> to would be specific to Qemu and not other platforms. But
>> I do
>>> > see the
>>> > >> merit in having the public-key certificate stored as part
>>> of an
>>> > overlay.
>>> > >> If I hit any issues while implementing this, I will get
>>> back to
>>> > you. Thanks.
>>> > >>
>>> > >> -sughosh
>>> > >
>>> > > OpenSBI can supply a device-tree to U-Boot. So this would
>> be an
>>> > > equivalent case.
>>> > >
>>> > > Best regards
>>> > >
>>> > > Heinrich
>>> >
>>> > <sng_>: "I am unable to think of a simple and elegant way to
>>> generate
>>> > the overlay dtb, since this would require creation of a
>>> corresponding
>>> > dts file, compiling it into a dtb and then using this on the
>>> platform."
>>> >
>>> > Jean-Jacques' patch series introduced some of the necessary
>> code:
>>> >
>>> > [PATCH PATCH v6 00/13] Add support for applications of
>>> overlays in SPL
>>> > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
>>> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
>>> >
>>> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
>>> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
>>> >
>>>
>> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
>>> <
>> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
>>>
>>> >
>>> <
>> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
>> <
>> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
>>>>
>>> >
>>> > CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be
>>> compiled.
>>> > You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY
>> and
>>> > CONFIG_SPL_LOAD_FIT dependency.
>>> >
>>> >
>>> > What i meant was that the process to generate the fdt overlay on
>> the
>>> > host, embed it with the public-key certificate is more cumbersome
>>> than
>>> > the current solution. So, for comparing the embedding the pub-key
>>> cert
>>> > in the fdt overlay, as against the platform dtb
>>> >
>>> > Embedding the certificate in the overlay
>>> > 1) Generate a skeleton overlay dts file
>>> > 2) Convert it to a dtb
>>> > 3) Run it through the mkeficapsule utility to embed the pub-key
>>> > certificate in the overlay(dtb)
>>> > 4) Store the overlay dtb on some nv storage on the platform(ESP
>>> partition?)
>>> > 5) Load the overlay and apply it to the platform's dtb
>>> > 6) Retrieve the certificate from the dtb at the time of capsule
>>> > authentication
>>> >
>>> > Embedding the certificate in the platform's dtb
>>> > 1) Run the platform's dtb through the mkeficapsule utility to
>> embed
>>> > the pub-key certificate
>>> > 2) Boot the platform with the platform's dtb
>>> > 3) Retrieve the certificate from the dtb at the time of capsule
>>> > authentication
>>> >
>>> > You had mentioned OpenSBI passing the dtb to u-boot. Does the
>> OpenSBI
>>> > generate the device tree for the platform on the fly even for
>>> non-qemu
>>> > platforms. If it does not, the dtb that OpenSBI uses can be run
>>> through
>>> > the mkeficapsule utility to embed the certificate.
>>>
>>> OpenSBI applies fix-ups before passing the device-tree. The prototype
>>> device-tree can either be built into OpenSBI or can be passed from an
>>> earlier firmware stage.
>>>
>>>
>>> So, if qemu if the only platform where the device tree is generated on
>>> the fly, and passed along, based on the comparison that i have stated
>>> above for the two scenarios(overlay vs platform dtb), I feel that using
>>> the platform's dtb and embedding the certificate is more easier to use
>>> than using the overlay.
>>>
>>> Even on the qemu platform, I retrieved the dtb from the platform, for
>>> the given set of devices and interfaces used, and then ran the dtb
>>> through the mkeficapsule utility.
>>>
>>> -sughosh
>>
>> As I understand the whole series is not about productive firmware
>> updates for QEMU but about a demonstration how it could be used and
>> allowing testing.
>>
>
> True, but the process of embedding the public key certificate into the
> platform's dtb for use in capsule authentication can be common for all
> platforms. If you are fine, we can keep this patch to be used on other
> non-qemu platforms, while i work on the overlay method for qemu. Will you
> be fine with this approach.
>
> -sughosh
>
This patch only changes board/emulation/qemu-arm/qemu-arm.c. Hence "keep
this patch to be used on other non-qemu platforms" is not applicable to
this specific patch.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-09 7:26 ` Heinrich Schuchardt
@ 2020-12-09 8:26 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-09 8:26 UTC (permalink / raw)
To: u-boot
On Wed, 9 Dec 2020 at 12:56, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 12/9/20 6:25 AM, Sughosh Ganu wrote:
> > On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >
> >> On 12/8/20 10:19 AM, Sughosh Ganu wrote:
> >>>
> >>> On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de
> >>> <mailto:xypron.glpk@gmx.de>> wrote:
> >>>
> >>> On 08.12.20 06:28, Sughosh Ganu wrote:
> >>> >
> >>> > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> >>> <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>
> wrote:
> >>> >
> >>> > On 07.12.20 13:50, Heinrich Schuchardt wrote:
> >>> > > On 07.12.20 06:15, Sughosh Ganu wrote:
> >>> > >> hello Heinrich,
> >>> > >>
> >>> > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> >>> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> >>> > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> >>> > >>
> >>> > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> >>> > >> > The Qemu platform emulator generates a device tree
> >>> blob and
> >>> > places it
> >>> > >> > at the start of the dram, which is then used by
> >>> u-boot. Use
> >>> > this dtb
> >>> > >> > only if CONFIG_OF_BOARD is defined. This allows
> >> using a
> >>> > different
> >>> > >> > device tree, using the CONFIG_OF_SEPARATE option.
> >>> This dtb
> >>> > is attached
> >>> > >> > to the u-boot binary as a u-boot-fdt.bin file
> >>> > >>
> >>> > >> Dear Sughosh,
> >>> > >>
> >>> > >> thank your for this series which will allow us to
> >> better
> >>> > demonstrate and
> >>> > >> test capsule updates.
> >>> > >>
> >>> > >> I am not sure if the approach that you take at
> >>> device-trees
> >>> > here is the
> >>> > >> right one.
> >>> > >>
> >>> > >> On QEMU the device-tree is generated on the fly by
> the
> >>> QEMU
> >>> > binary
> >>> > >> depending on which devices the user has specified.
> >>> > >>
> >>> > >> Your idea is to replace this device-tree completely
> to
> >> be
> >>> > able to add
> >>> > >> extra elements (the EFI signature list, see patch
> >>> 2/14). Thus a
> >>> > >> device-tree might be loaded that does not match the
> >>> user selected
> >>> > >> devices.
> >>> > >>
> >>> > >> An alternative approach would be to apply all
> >>> additions to the
> >>> > >> device-tree as an FDT overlay (or fixup). This would
> >> allow
> >>> > the dynamic
> >>> > >> parts of the QEMU device-tree still to be passed
> >> through.
> >>> > >>
> >>> > >>
> >>> > >> I will take a look at storing the public key as part of
> >>> the fdt
> >>> > overlay,
> >>> > >> with a runtime fixup. Although, I think the issue that
> you
> >> are
> >>> > pointing
> >>> > >> to would be specific to Qemu and not other platforms.
> But
> >> I do
> >>> > see the
> >>> > >> merit in having the public-key certificate stored as
> part
> >>> of an
> >>> > overlay.
> >>> > >> If I hit any issues while implementing this, I will get
> >>> back to
> >>> > you. Thanks.
> >>> > >>
> >>> > >> -sughosh
> >>> > >
> >>> > > OpenSBI can supply a device-tree to U-Boot. So this would
> >> be an
> >>> > > equivalent case.
> >>> > >
> >>> > > Best regards
> >>> > >
> >>> > > Heinrich
> >>> >
> >>> > <sng_>: "I am unable to think of a simple and elegant way
> to
> >>> generate
> >>> > the overlay dtb, since this would require creation of a
> >>> corresponding
> >>> > dts file, compiling it into a dtb and then using this on
> the
> >>> platform."
> >>> >
> >>> > Jean-Jacques' patch series introduced some of the necessary
> >> code:
> >>> >
> >>> > [PATCH PATCH v6 00/13] Add support for applications of
> >>> overlays in SPL
> >>> >
> https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> >>> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> >>> >
> >>> <
> https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> >>> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> >>
> >>> >
> >>>
> >>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>> <
> >>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>>
> >>> >
> >>> <
> >>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >> <
> >>
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>>>
> >>> >
> >>> > CONFIG_OF_OVERLAY_LIST is used to specify the overlays to
> be
> >>> compiled.
> >>> > You will have to remove the
> CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY
> >> and
> >>> > CONFIG_SPL_LOAD_FIT dependency.
> >>> >
> >>> >
> >>> > What i meant was that the process to generate the fdt overlay
> on
> >> the
> >>> > host, embed it with the public-key certificate is more
> cumbersome
> >>> than
> >>> > the current solution. So, for comparing the embedding the
> pub-key
> >>> cert
> >>> > in the fdt overlay, as against the platform dtb
> >>> >
> >>> > Embedding the certificate in the overlay
> >>> > 1) Generate a skeleton overlay dts file
> >>> > 2) Convert it to a dtb
> >>> > 3) Run it through the mkeficapsule utility to embed the pub-key
> >>> > certificate in the overlay(dtb)
> >>> > 4) Store the overlay dtb on some nv storage on the platform(ESP
> >>> partition?)
> >>> > 5) Load the overlay and apply it to the platform's dtb
> >>> > 6) Retrieve the certificate from the dtb at the time of capsule
> >>> > authentication
> >>> >
> >>> > Embedding the certificate in the platform's dtb
> >>> > 1) Run the platform's dtb through the mkeficapsule utility to
> >> embed
> >>> > the pub-key certificate
> >>> > 2) Boot the platform with the platform's dtb
> >>> > 3) Retrieve the certificate from the dtb at the time of capsule
> >>> > authentication
> >>> >
> >>> > You had mentioned OpenSBI passing the dtb to u-boot. Does the
> >> OpenSBI
> >>> > generate the device tree for the platform on the fly even for
> >>> non-qemu
> >>> > platforms. If it does not, the dtb that OpenSBI uses can be run
> >>> through
> >>> > the mkeficapsule utility to embed the certificate.
> >>>
> >>> OpenSBI applies fix-ups before passing the device-tree. The
> prototype
> >>> device-tree can either be built into OpenSBI or can be passed
> from an
> >>> earlier firmware stage.
> >>>
> >>>
> >>> So, if qemu if the only platform where the device tree is generated on
> >>> the fly, and passed along, based on the comparison that i have stated
> >>> above for the two scenarios(overlay vs platform dtb), I feel that using
> >>> the platform's dtb and embedding the certificate is more easier to use
> >>> than using the overlay.
> >>>
> >>> Even on the qemu platform, I retrieved the dtb from the platform, for
> >>> the given set of devices and interfaces used, and then ran the dtb
> >>> through the mkeficapsule utility.
> >>>
> >>> -sughosh
> >>
> >> As I understand the whole series is not about productive firmware
> >> updates for QEMU but about a demonstration how it could be used and
> >> allowing testing.
> >>
> >
> > True, but the process of embedding the public key certificate into the
> > platform's dtb for use in capsule authentication can be common for all
> > platforms. If you are fine, we can keep this patch to be used on other
> > non-qemu platforms, while i work on the overlay method for qemu. Will you
> > be fine with this approach.
> >
> > -sughosh
> >
>
> This patch only changes board/emulation/qemu-arm/qemu-arm.c. Hence "keep
> this patch to be used on other non-qemu platforms" is not applicable to
> this specific patch.
>
Sorry, i did not mean this patch, but patch 02/14, "mkeficapsule: Add
support for embedding public key in a dtb". I meant that that patch can be
used for other non-qemu platforms.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-08 21:54 ` Heinrich Schuchardt
2020-12-09 5:25 ` Sughosh Ganu
@ 2020-12-15 11:10 ` Sughosh Ganu
2020-12-15 12:55 ` Heinrich Schuchardt
1 sibling, 1 reply; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-15 11:10 UTC (permalink / raw)
To: u-boot
On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 12/8/20 10:19 AM, Sughosh Ganu wrote:
> >
> > On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 08.12.20 06:28, Sughosh Ganu wrote:
> > >
> > > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> > >
> > > On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > > > On 07.12.20 06:15, Sughosh Ganu wrote:
> > > >> hello Heinrich,
> > > >>
> > > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> > > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> > > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> > > >>
> > > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > > >> > The Qemu platform emulator generates a device tree
> > blob and
> > > places it
> > > >> > at the start of the dram, which is then used by
> > u-boot. Use
> > > this dtb
> > > >> > only if CONFIG_OF_BOARD is defined. This allows
> using a
> > > different
> > > >> > device tree, using the CONFIG_OF_SEPARATE option.
> > This dtb
> > > is attached
> > > >> > to the u-boot binary as a u-boot-fdt.bin file
> > > >>
> > > >> Dear Sughosh,
> > > >>
> > > >> thank your for this series which will allow us to
> better
> > > demonstrate and
> > > >> test capsule updates.
> > > >>
> > > >> I am not sure if the approach that you take at
> > device-trees
> > > here is the
> > > >> right one.
> > > >>
> > > >> On QEMU the device-tree is generated on the fly by the
> > QEMU
> > > binary
> > > >> depending on which devices the user has specified.
> > > >>
> > > >> Your idea is to replace this device-tree completely to
> be
> > > able to add
> > > >> extra elements (the EFI signature list, see patch
> > 2/14). Thus a
> > > >> device-tree might be loaded that does not match the
> > user selected
> > > >> devices.
> > > >>
> > > >> An alternative approach would be to apply all
> > additions to the
> > > >> device-tree as an FDT overlay (or fixup). This would
> allow
> > > the dynamic
> > > >> parts of the QEMU device-tree still to be passed
> through.
> > > >>
> > > >>
> > > >> I will take a look at storing the public key as part of
> > the fdt
> > > overlay,
> > > >> with a runtime fixup. Although, I think the issue that you
> are
> > > pointing
> > > >> to would be specific to Qemu and not other platforms. But
> I do
> > > see the
> > > >> merit in having the public-key certificate stored as part
> > of an
> > > overlay.
> > > >> If I hit any issues while implementing this, I will get
> > back to
> > > you. Thanks.
> > > >>
> > > >> -sughosh
> > > >
> > > > OpenSBI can supply a device-tree to U-Boot. So this would
> be an
> > > > equivalent case.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > >
> > > <sng_>: "I am unable to think of a simple and elegant way to
> > generate
> > > the overlay dtb, since this would require creation of a
> > corresponding
> > > dts file, compiling it into a dtb and then using this on the
> > platform."
> > >
> > > Jean-Jacques' patch series introduced some of the necessary
> code:
> > >
> > > [PATCH PATCH v6 00/13] Add support for applications of
> > overlays in SPL
> > > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> > >
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
> > >
> >
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >
> > >
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>
> > >
> > > CONFIG_OF_OVERLAY_LIST is used to specify the overlays to be
> > compiled.
> > > You will have to remove the CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY
> and
> > > CONFIG_SPL_LOAD_FIT dependency.
> > >
> > >
> > > What i meant was that the process to generate the fdt overlay on
> the
> > > host, embed it with the public-key certificate is more cumbersome
> > than
> > > the current solution. So, for comparing the embedding the pub-key
> > cert
> > > in the fdt overlay, as against the platform dtb
> > >
> > > Embedding the certificate in the overlay
> > > 1) Generate a skeleton overlay dts file
> > > 2) Convert it to a dtb
> > > 3) Run it through the mkeficapsule utility to embed the pub-key
> > > certificate in the overlay(dtb)
> > > 4) Store the overlay dtb on some nv storage on the platform(ESP
> > partition?)
> > > 5) Load the overlay and apply it to the platform's dtb
> > > 6) Retrieve the certificate from the dtb at the time of capsule
> > > authentication
> > >
> > > Embedding the certificate in the platform's dtb
> > > 1) Run the platform's dtb through the mkeficapsule utility to
> embed
> > > the pub-key certificate
> > > 2) Boot the platform with the platform's dtb
> > > 3) Retrieve the certificate from the dtb at the time of capsule
> > > authentication
> > >
> > > You had mentioned OpenSBI passing the dtb to u-boot. Does the
> OpenSBI
> > > generate the device tree for the platform on the fly even for
> > non-qemu
> > > platforms. If it does not, the dtb that OpenSBI uses can be run
> > through
> > > the mkeficapsule utility to embed the certificate.
> >
> > OpenSBI applies fix-ups before passing the device-tree. The prototype
> > device-tree can either be built into OpenSBI or can be passed from an
> > earlier firmware stage.
> >
> >
> > So, if qemu if the only platform where the device tree is generated on
> > the fly, and passed along, based on the comparison that i have stated
> > above for the two scenarios(overlay vs platform dtb), I feel that using
> > the platform's dtb and embedding the certificate is more easier to use
> > than using the overlay.
> >
> > Even on the qemu platform, I retrieved the dtb from the platform, for
> > the given set of devices and interfaces used, and then ran the dtb
> > through the mkeficapsule utility.
> >
> > -sughosh
>
> As I understand the whole series is not about productive firmware
> updates for QEMU but about a demonstration how it could be used and
> allowing testing.
>
> Wouldn't it be the enough for testing to apply an overlay with the
> public key using the fdt command?
Is there a way I can build qemu such that the device-tree that is generated
is with the __symbols__ node. Without this node, I am unable to apply the
fdt overlay on the base platform devicetree.
-sughosh
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-15 11:10 ` Sughosh Ganu
@ 2020-12-15 12:55 ` Heinrich Schuchardt
2020-12-15 15:35 ` Sughosh Ganu
0 siblings, 1 reply; 49+ messages in thread
From: Heinrich Schuchardt @ 2020-12-15 12:55 UTC (permalink / raw)
To: u-boot
On 15.12.20 12:10, Sughosh Ganu wrote:
>
> On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 12/8/20 10:19 AM, Sughosh Ganu wrote:
> >
> > On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt
> <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> >
> >? ? ?On 08.12.20 06:28, Sughosh Ganu wrote:
> >? ? ? >
> >? ? ? > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> >? ? ?<xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> >? ? ? > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> >? ? ? >
> >? ? ? >? ? ?On 07.12.20 13:50, Heinrich Schuchardt wrote:
> >? ? ? >? ? ?> On 07.12.20 06:15, Sughosh Ganu wrote:
> >? ? ? >? ? ?>> hello Heinrich,
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> >? ? ? >? ? ?<xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> >? ? ?<mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>
> >? ? ? >? ? ?>> <mailto:xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>>
> >? ? ?<mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>>> wrote:
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> >? ? ? >? ? ?>>? ? ?> The Qemu platform emulator generates a device tree
> >? ? ?blob and
> >? ? ? >? ? ?places it
> >? ? ? >? ? ?>>? ? ?> at the start of the dram, which is then used by
> >? ? ?u-boot. Use
> >? ? ? >? ? ?this dtb
> >? ? ? >? ? ?>>? ? ?> only if CONFIG_OF_BOARD is defined. This
> allows using a
> >? ? ? >? ? ?different
> >? ? ? >? ? ?>>? ? ?> device tree, using the CONFIG_OF_SEPARATE option.
> >? ? ?This dtb
> >? ? ? >? ? ?is attached
> >? ? ? >? ? ?>>? ? ?> to the u-boot binary as a u-boot-fdt.bin file
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?Dear Sughosh,
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?thank your for this series which will allow us
> to better
> >? ? ? >? ? ?demonstrate and
> >? ? ? >? ? ?>>? ? ?test capsule updates.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?I am not sure if the approach that you take at
> >? ? ?device-trees
> >? ? ? >? ? ?here is the
> >? ? ? >? ? ?>>? ? ?right one.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?On QEMU the device-tree is generated on the fly
> by the
> >? ? ?QEMU
> >? ? ? >? ? ?binary
> >? ? ? >? ? ?>>? ? ?depending on which devices the user has specified.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?Your idea is to replace this device-tree
> completely to be
> >? ? ? >? ? ?able to add
> >? ? ? >? ? ?>>? ? ?extra elements (the EFI signature list, see patch
> >? ? ?2/14). Thus a
> >? ? ? >? ? ?>>? ? ?device-tree might be loaded that does not match the
> >? ? ?user selected
> >? ? ? >? ? ?>>? ? ?devices.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>? ? ?An alternative approach would be to apply all
> >? ? ?additions to the
> >? ? ? >? ? ?>>? ? ?device-tree as an FDT overlay (or fixup). This
> would allow
> >? ? ? >? ? ?the dynamic
> >? ? ? >? ? ?>>? ? ?parts of the QEMU device-tree still to be passed
> through.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>> I will take a look at storing the public key as part of
> >? ? ?the fdt
> >? ? ? >? ? ?overlay,
> >? ? ? >? ? ?>> with a runtime fixup. Although, I think the issue
> that you are
> >? ? ? >? ? ?pointing
> >? ? ? >? ? ?>> to would be specific to Qemu and not other
> platforms. But I do
> >? ? ? >? ? ?see the
> >? ? ? >? ? ?>> merit in having the public-key certificate stored as
> part
> >? ? ?of an
> >? ? ? >? ? ?overlay.
> >? ? ? >? ? ?>> If I hit any issues while implementing this, I will get
> >? ? ?back to
> >? ? ? >? ? ?you. Thanks.
> >? ? ? >? ? ?>>
> >? ? ? >? ? ?>> -sughosh
> >? ? ? >? ? ?>
> >? ? ? >? ? ?> OpenSBI can supply a device-tree to U-Boot. So this
> would be an
> >? ? ? >? ? ?> equivalent case.
> >? ? ? >? ? ?>
> >? ? ? >? ? ?> Best regards
> >? ? ? >? ? ?>
> >? ? ? >? ? ?> Heinrich
> >? ? ? >
> >? ? ? >? ? ?<sng_>: "I am unable to think of a simple and elegant
> way to
> >? ? ?generate
> >? ? ? >? ? ?the overlay dtb, since this would require creation of a
> >? ? ?corresponding
> >? ? ? >? ? ?dts file, compiling it into a dtb and then using this
> on the
> >? ? ?platform."
> >? ? ? >
> >? ? ? >? ? ?Jean-Jacques' patch series introduced some of the
> necessary code:
> >? ? ? >
> >? ? ? >? ? ?[PATCH PATCH v6 00/13] Add support for applications of
> >? ? ?overlays in SPL
> >? ? ? >
> https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> >? ?
> ?<https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
> >? ? ? >
> >? ? ?
> ?<https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> >? ?
> ?<https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>>
> >? ? ? >
> >? ?
> ?https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>
> >? ?
> ?<https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>>
> >? ? ? >
> >? ? ?
> ?<https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> <https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both>>>
> >? ? ? >
> >? ? ? >? ? ?CONFIG_OF_OVERLAY_LIST is used to specify the overlays
> to be
> >? ? ?compiled.
> >? ? ? >? ? ?You will have to remove the
> CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> >? ? ? >? ? ?CONFIG_SPL_LOAD_FIT dependency.
> >? ? ? >
> >? ? ? >
> >? ? ? > What i meant was that the process to generate the fdt
> overlay on the
> >? ? ? > host, embed it with the public-key certificate is more
> cumbersome
> >? ? ?than
> >? ? ? > the current solution. So, for comparing the embedding the
> pub-key
> >? ? ?cert
> >? ? ? > in the fdt overlay, as against the platform dtb
> >? ? ? >
> >? ? ? > Embedding the certificate in the overlay
> >? ? ? > 1) Generate a skeleton overlay dts file
> >? ? ? > 2) Convert it to a dtb
> >? ? ? > 3) Run it through the mkeficapsule utility to embed the pub-key
> >? ? ? > certificate in the overlay(dtb)
> >? ? ? > 4) Store the overlay dtb on some nv storage on the platform(ESP
> >? ? ?partition?)
> >? ? ? > 5) Load the overlay and apply it to the platform's dtb
> >? ? ? > 6) Retrieve the certificate from the dtb at the time of capsule
> >? ? ? > authentication
> >? ? ? >
> >? ? ? > Embedding the certificate in the platform's dtb
> >? ? ? > 1) Run the platform's dtb through the mkeficapsule utility
> to embed
> >? ? ? > the?pub-key certificate
> >? ? ? > 2) Boot the platform with the platform's dtb
> >? ? ? > 3) Retrieve the certificate from the dtb at the time of capsule
> >? ? ? > authentication
> >? ? ? >
> >? ? ? > You had mentioned OpenSBI passing the dtb to u-boot. Does
> the OpenSBI
> >? ? ? > generate the device tree for the platform on the fly even for
> >? ? ?non-qemu
> >? ? ? > platforms.?If it does not, the dtb that OpenSBI uses can be run
> >? ? ?through
> >? ? ? > the mkeficapsule utility to embed the certificate.
> >
> >? ? ?OpenSBI applies fix-ups before passing the device-tree. The
> prototype
> >? ? ?device-tree can either be built into OpenSBI or can be passed
> from an
> >? ? ?earlier firmware stage.
> >
> >
> > So, if qemu if the only platform where the device tree is generated on
> > the fly, and passed along, based on the comparison that i have stated
> > above for the two scenarios(overlay vs platform dtb), I feel that
> using
> > the platform's dtb and embedding the certificate is more easier to use
> > than using the overlay.
> >
> > Even on the qemu platform, I retrieved the dtb from the platform, for
> > the given set of devices and interfaces used, and then ran the dtb
> > through the mkeficapsule utility.
> >
> > -sughosh
>
> As I understand the whole series is not about productive firmware
> updates for QEMU but about a demonstration how it could be used and
> allowing testing.
>
> Wouldn't it be the enough for testing to apply an overlay with the
> public key using the fdt command?
>
>
> Is there a way I can build qemu such that the device-tree that is
> generated is with the __symbols__ node. Without this node, I am unable
> to apply the fdt overlay on the base platform devicetree.
According to
https://www.kernel.org/doc/Documentation/devicetree/overlay-notes.txt
__symbols__ is only needed if you refer to phandles in the overlay. When
using paths instead __symbols__ is not required.
I have not tested if this holds true for U-Boot too.
Best regards
Heinrich
>
> -sughosh?
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined
2020-12-15 12:55 ` Heinrich Schuchardt
@ 2020-12-15 15:35 ` Sughosh Ganu
0 siblings, 0 replies; 49+ messages in thread
From: Sughosh Ganu @ 2020-12-15 15:35 UTC (permalink / raw)
To: u-boot
On Tue, 15 Dec 2020 at 18:25, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 15.12.20 12:10, Sughosh Ganu wrote:
> >
> > On Wed, 9 Dec 2020 at 03:24, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 12/8/20 10:19 AM, Sughosh Ganu wrote:
> > >
> > > On Tue, 8 Dec 2020 at 14:32, Heinrich Schuchardt
> > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> wrote:
> > >
> > > On 08.12.20 06:28, Sughosh Ganu wrote:
> > > >
> > > > On Mon, 7 Dec 2020 at 23:28, Heinrich Schuchardt
> > > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> wrote:
> > > >
> > > > On 07.12.20 13:50, Heinrich Schuchardt wrote:
> > > > > On 07.12.20 06:15, Sughosh Ganu wrote:
> > > > >> hello Heinrich,
> > > > >>
> > > > >> On Sat, 5 Dec 2020 at 15:01, Heinrich Schuchardt
> > > > <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>
> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>
> > > > >> <mailto:xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>>
> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>>> wrote:
> > > > >>
> > > > >> On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> > > > >> > The Qemu platform emulator generates a device
> tree
> > > blob and
> > > > places it
> > > > >> > at the start of the dram, which is then used by
> > > u-boot. Use
> > > > this dtb
> > > > >> > only if CONFIG_OF_BOARD is defined. This
> > allows using a
> > > > different
> > > > >> > device tree, using the CONFIG_OF_SEPARATE
> option.
> > > This dtb
> > > > is attached
> > > > >> > to the u-boot binary as a u-boot-fdt.bin file
> > > > >>
> > > > >> Dear Sughosh,
> > > > >>
> > > > >> thank your for this series which will allow us
> > to better
> > > > demonstrate and
> > > > >> test capsule updates.
> > > > >>
> > > > >> I am not sure if the approach that you take at
> > > device-trees
> > > > here is the
> > > > >> right one.
> > > > >>
> > > > >> On QEMU the device-tree is generated on the fly
> > by the
> > > QEMU
> > > > binary
> > > > >> depending on which devices the user has
> specified.
> > > > >>
> > > > >> Your idea is to replace this device-tree
> > completely to be
> > > > able to add
> > > > >> extra elements (the EFI signature list, see patch
> > > 2/14). Thus a
> > > > >> device-tree might be loaded that does not match
> the
> > > user selected
> > > > >> devices.
> > > > >>
> > > > >> An alternative approach would be to apply all
> > > additions to the
> > > > >> device-tree as an FDT overlay (or fixup). This
> > would allow
> > > > the dynamic
> > > > >> parts of the QEMU device-tree still to be passed
> > through.
> > > > >>
> > > > >>
> > > > >> I will take a look at storing the public key as part
> of
> > > the fdt
> > > > overlay,
> > > > >> with a runtime fixup. Although, I think the issue
> > that you are
> > > > pointing
> > > > >> to would be specific to Qemu and not other
> > platforms. But I do
> > > > see the
> > > > >> merit in having the public-key certificate stored as
> > part
> > > of an
> > > > overlay.
> > > > >> If I hit any issues while implementing this, I will
> get
> > > back to
> > > > you. Thanks.
> > > > >>
> > > > >> -sughosh
> > > > >
> > > > > OpenSBI can supply a device-tree to U-Boot. So this
> > would be an
> > > > > equivalent case.
> > > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > >
> > > > <sng_>: "I am unable to think of a simple and elegant
> > way to
> > > generate
> > > > the overlay dtb, since this would require creation of a
> > > corresponding
> > > > dts file, compiling it into a dtb and then using this
> > on the
> > > platform."
> > > >
> > > > Jean-Jacques' patch series introduced some of the
> > necessary code:
> > > >
> > > > [PATCH PATCH v6 00/13] Add support for applications of
> > > overlays in SPL
> > > >
> > https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> > >
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>
> > > >
> > >
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>
> > >
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html
> > <https://lists.denx.de/pipermail/u-boot/2019-October/387653.html>>>
> > > >
> > >
> >
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >
> > >
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>
> > > >
> > >
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> > <
> https://patchwork.ozlabs.org/project/uboot/list/?series=137810&state=%2A&archive=both
> >>>
> > > >
> > > > CONFIG_OF_OVERLAY_LIST is used to specify the overlays
> > to be
> > > compiled.
> > > > You will have to remove the
> > CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY and
> > > > CONFIG_SPL_LOAD_FIT dependency.
> > > >
> > > >
> > > > What i meant was that the process to generate the fdt
> > overlay on the
> > > > host, embed it with the public-key certificate is more
> > cumbersome
> > > than
> > > > the current solution. So, for comparing the embedding the
> > pub-key
> > > cert
> > > > in the fdt overlay, as against the platform dtb
> > > >
> > > > Embedding the certificate in the overlay
> > > > 1) Generate a skeleton overlay dts file
> > > > 2) Convert it to a dtb
> > > > 3) Run it through the mkeficapsule utility to embed the
> pub-key
> > > > certificate in the overlay(dtb)
> > > > 4) Store the overlay dtb on some nv storage on the
> platform(ESP
> > > partition?)
> > > > 5) Load the overlay and apply it to the platform's dtb
> > > > 6) Retrieve the certificate from the dtb at the time of
> capsule
> > > > authentication
> > > >
> > > > Embedding the certificate in the platform's dtb
> > > > 1) Run the platform's dtb through the mkeficapsule utility
> > to embed
> > > > the pub-key certificate
> > > > 2) Boot the platform with the platform's dtb
> > > > 3) Retrieve the certificate from the dtb at the time of
> capsule
> > > > authentication
> > > >
> > > > You had mentioned OpenSBI passing the dtb to u-boot. Does
> > the OpenSBI
> > > > generate the device tree for the platform on the fly even
> for
> > > non-qemu
> > > > platforms. If it does not, the dtb that OpenSBI uses can be
> run
> > > through
> > > > the mkeficapsule utility to embed the certificate.
> > >
> > > OpenSBI applies fix-ups before passing the device-tree. The
> > prototype
> > > device-tree can either be built into OpenSBI or can be passed
> > from an
> > > earlier firmware stage.
> > >
> > >
> > > So, if qemu if the only platform where the device tree is
> generated on
> > > the fly, and passed along, based on the comparison that i have
> stated
> > > above for the two scenarios(overlay vs platform dtb), I feel that
> > using
> > > the platform's dtb and embedding the certificate is more easier to
> use
> > > than using the overlay.
> > >
> > > Even on the qemu platform, I retrieved the dtb from the platform,
> for
> > > the given set of devices and interfaces used, and then ran the dtb
> > > through the mkeficapsule utility.
> > >
> > > -sughosh
> >
> > As I understand the whole series is not about productive firmware
> > updates for QEMU but about a demonstration how it could be used and
> > allowing testing.
> >
> > Wouldn't it be the enough for testing to apply an overlay with the
> > public key using the fdt command?
> >
> >
> > Is there a way I can build qemu such that the device-tree that is
> > generated is with the __symbols__ node. Without this node, I am unable
> > to apply the fdt overlay on the base platform devicetree.
>
> According to
> https://www.kernel.org/doc/Documentation/devicetree/overlay-notes.txt
> __symbols__ is only needed if you refer to phandles in the overlay. When
> using paths instead __symbols__ is not required.
>
> I have not tested if this holds true for U-Boot too.
>
This does work. Thanks. I was referring to the document mentioned in the
README.fdt-overlays[1] and that does not have mention of target-path.
-sughosh
[1] -
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/Documentation/dt-object-internal.txt
^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2020-12-15 15:35 UTC | newest]
Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 18:40 [PATCH 00/14] qemu: arm64: Add support for uefi capsule update on qemu arm64 platform Sughosh Ganu
2020-11-26 18:40 ` [PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined Sughosh Ganu
2020-12-05 9:31 ` Heinrich Schuchardt
2020-12-07 5:15 ` Sughosh Ganu
2020-12-07 12:50 ` Heinrich Schuchardt
2020-12-07 17:58 ` Heinrich Schuchardt
2020-12-08 5:28 ` Sughosh Ganu
2020-12-08 9:02 ` Heinrich Schuchardt
2020-12-08 9:19 ` Sughosh Ganu
2020-12-08 21:54 ` Heinrich Schuchardt
2020-12-09 5:25 ` Sughosh Ganu
2020-12-09 7:26 ` Heinrich Schuchardt
2020-12-09 8:26 ` Sughosh Ganu
2020-12-15 11:10 ` Sughosh Ganu
2020-12-15 12:55 ` Heinrich Schuchardt
2020-12-15 15:35 ` Sughosh Ganu
2020-11-26 18:40 ` [PATCH 02/14] mkeficapsule: Add support for embedding public key in a dtb Sughosh Ganu
2020-11-26 18:40 ` [PATCH 03/14] qemu: arm: Scan the pci bus in board_init Sughosh Ganu
2020-12-05 9:45 ` Heinrich Schuchardt
2020-12-07 5:16 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 04/14] crypto: Fix the logic to calculate hash with authattributes set Sughosh Ganu
2020-12-05 10:21 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 05/14] qemu: arm64: Add support for dynamic mtdparts for the platform Sughosh Ganu
2020-12-05 10:29 ` Heinrich Schuchardt
2020-12-07 5:30 ` Sughosh Ganu
2020-12-07 18:44 ` Tom Rini
2020-12-08 5:12 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 06/14] qemu: arm64: Set dfu_alt_info variable " Sughosh Ganu
2020-12-05 10:31 ` Heinrich Schuchardt
2020-12-07 5:42 ` Sughosh Ganu
2020-12-07 6:56 ` Heinrich Schuchardt
2020-12-07 7:45 ` Sughosh Ganu
2020-12-07 18:47 ` Tom Rini
2020-12-08 5:18 ` Sughosh Ganu
2020-12-08 12:20 ` Tom Rini
2020-12-08 17:03 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 07/14] efi_loader: Add config option to indicate fmp header presence Sughosh Ganu
2020-12-05 10:34 ` Heinrich Schuchardt
2020-12-07 6:02 ` Sughosh Ganu
2020-11-26 18:41 ` [PATCH 08/14] dfu_mtd: Add provision to unlock mtd device Sughosh Ganu
2020-11-26 18:41 ` [PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern Sughosh Ganu
2020-12-05 10:40 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 10/14] efi_loader: Re-factor code to build the signature store from efi signature list Sughosh Ganu
2020-11-26 18:41 ` [PATCH 11/14] efi: capsule: Add support for uefi capsule authentication Sughosh Ganu
2020-11-26 18:41 ` [PATCH 12/14] efi_loader: Enable " Sughosh Ganu
2020-12-05 10:47 ` Heinrich Schuchardt
2020-11-26 18:41 ` [PATCH 13/14] efidebug: capsule: Add a command to update capsule on disk Sughosh Ganu
2020-11-26 18:41 ` [PATCH 14/14] qemu: arm64: Add documentation for capsule update Sughosh Ganu
2020-12-05 10:16 ` Heinrich Schuchardt
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.