[U-Boot] [PATCH 0/9] Secure Boot by Authenticating/Decrypting SPL FIT blobs

[U-Boot] [PATCH 0/9] Secure Boot by Authenticating/Decrypting SPL FIT blobs

[Andreas Dannenberg]

This patch series is derived from an earlier RFC [1] with (most of the)
feedback implemented that was provided by Simon, Tom, and Lokesh (see change
log at the end). There are still some loose ends namely affecting the make
process (see discussion here [2], maybe Yamada-san has some inputs?) as well as
the conversion of the "weak" post-process function call to a Kconfig option. We
did experiment with a few options regarding the Kconfig conversion but couldn't
really settle on a good way. Simon or Tom- if you could provide a tiny bit of
additional direction here we will be happy to re-visit this and change the
current implementation.

Here's again the high-level summary of what we are trying to achieve:

A method is introduced that uses a "weak" post-process function call that's
injected into the SPL FIT loading process after each blob has been extracted
(U-Boot firmware, selected DTB) which is populated with a platform specific
function. In case of TI high-security (HS) device variants a ROM API call is
performed (via SMC) that transparently handles authentication and optional
decryption. This essentially allows authenticating (and optionally decrypting)
U-Boot from SPL. The post-processing injection is implemented such to enable a
more universal use of this feature by other platforms or for other purposes.

Furthermore the build process has been modified to automatically invoke a TI
blob signing/encryption tool through the $TI_SECURE_DEV_PKG env variable
already introduced into U-Boot. This singing/encryption step happens for each
artifact that gets bundled into the final u-boot.img FIT blob.

Why do we need this for our platforms if some generic form of verified boot
already exists in U-Boot? The approach proposed here provides support for
decryption which is currently not available in U-Boot (and may not be easily
implementable generically for example due to the need for keeping symmetric
keys secure). Furthermore it allows for a consistent build as well as runtime
flow no matter authentication and/or decryption is used (as opposed to using
existing U-Boot verified boot for authentication and an additional TI-specific
ROM API call for decryption for example). It also allows for a faster and more
efficient boot process (auth and decryption can be performed in a single step
by the ROM APIs also utilizing crypto HW accelerators in a completely
transparent fashion). However anyone can still use the standard U-Boot verified
boot scheme from U-Boot onwards if they so chose and only need authentication.


I also just re-rested the patch series on actual AM437x HS, DRA7 HS, DRA72 HS,
and AM57 HS device variants by building and running SPL/U-Boot. I've also
build-tested the corresponding non-HS device variant configurations to make
sure nothing is broken there.


Changes RFC->PATCH:
- Update of README.ti-secure
- Unification of some of the secure ROM API call stuff between AM43xx and
  OMAP5-based platforms by moving those into common files
- Replacement of puts() with printf()
- Minor build simplification/cleanup
- Addition of "Reviewed-by:" comments for files that were pretty much carried
  over from the RFC as-is
- Addition of AM437x HS device build support (was missing in RFC)
- Removal of some redundant conditional compile directives
- Rebased on upstream U-Boot commit "Prepare v2016.07-rc2"

Regards,
Andreas Dannenberg

[1] https://www.mail-archive.com/u-boot at lists.denx.de/msg216299.html
[2] https://www.mail-archive.com/u-boot at lists.denx.de/msg216298.html


Andreas Dannenberg (4):
  arm: omap-common: add secure rom call API for secure devices
  arm: omap-common: secure ROM signature verify API
  arm: omap-common: Update to generate secure U-Boot FIT blob
  arm: omap5: add U-Boot FIT signing and SPL image post-processing

Daniel Allred (4):
  arm: cache: add missing dummy functions for when dcache disabled
  spl: fit: add support for post-processing of images
  arm: omap-common: add secure smc entry
  doc: Update info on using secure devices from TI

Madan Srinivas (1):
  arm: am4x: add U-Boot FIT signing and SPL image post-processing

 arch/arm/cpu/armv7/am33xx/config.mk             |   1 +
 arch/arm/cpu/armv7/cache_v7.c                   |   8 ++
 arch/arm/cpu/armv7/omap-common/Makefile         |   2 +
 arch/arm/cpu/armv7/omap-common/config_secure.mk |  57 +++++++-
 arch/arm/cpu/armv7/omap-common/lowlevel_init.S  |  45 +++++--
 arch/arm/cpu/armv7/omap-common/sec-common.c     | 125 +++++++++++++++++
 arch/arm/cpu/armv7/omap5/config.mk              |   3 +
 arch/arm/include/asm/omap_common.h              |  22 +++
 board/ti/am43xx/board.c                         |   7 +
 board/ti/am57xx/board.c                         |   7 +
 board/ti/dra7xx/evm.c                           |   7 +
 common/spl/spl_fit.c                            |  21 ++-
 doc/README.ti-secure                            | 170 ++++++++++++++++--------
 include/image.h                                 |  15 +++
 14 files changed, 420 insertions(+), 70 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/omap-common/sec-common.c

-- 
2.6.4

[U-Boot] [PATCH 1/9] arm: cache: add missing dummy functions for when dcache disabled

[Andreas Dannenberg]

From: Daniel Allred <d-allred@ti.com>

Adds missing flush_dcache_range and invalidate_dcache_range dummy
(empty) placeholder functions to the #else portion of the #ifndef
CONFIG_SYS_DCACHE_OFF, where full implementations of these functions
are defined.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 arch/arm/cpu/armv7/cache_v7.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index dc309da..24fe0c5 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -195,6 +195,14 @@ void flush_dcache_all(void)
 {
 }
 
+void invalidate_dcache_range(unsigned long start, unsigned long stop)
+{
+}
+
+void flush_dcache_range(unsigned long start, unsigned long stop)
+{
+}
+
 void arm_init_before_mmu(void)
 {
 }
-- 
2.6.4

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Andreas Dannenberg]

From: Daniel Allred <d-allred@ti.com>

The next stage boot loader image and the selected FDT can be
post-processed by board/platform/device-specific code, which can include
modifying the size and altering the starting source address before
copying these binary blobs to their final desitination. This might be
desired to do things like strip headers or footers attached to the
images before they were packeaged into the FIT, or to perform operations
such as decryption or authentication.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
 common/spl/spl_fit.c | 21 ++++++++++++++++-----
 include/image.h      | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 9874708..ecbcb97 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	return (data_size + info->bl_len - 1) / info->bl_len;
 }
 
+void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
+{
+}
+
 int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 {
 	int sectors;
@@ -132,7 +136,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	int data_offset, data_size;
 	int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
 	int src_sector;
-	void *dst;
+	void *dst, *src;
 
 	/*
 	 * Figure out where the external images start. This is the base for the
@@ -206,8 +210,11 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 		return -EIO;
 	debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
 	      data_size);
-	memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
-	       data_size);
+	src = dst + get_aligned_image_overhead(info, data_offset);
+
+	board_fit_image_post_process((void **)&src, (size_t *)&data_size);
+
+	memcpy(dst, src, data_size);
 
 	/* Figure out which device tree the board wants to use */
 	fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
@@ -236,8 +243,12 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 */
 	debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
 	      fdt_len);
-	memcpy(load_ptr + data_size,
-	       dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
+	src = dst + get_aligned_image_overhead(info, fdt_offset);
+	dst = load_ptr + data_size;
+
+	board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
+
+	memcpy(dst, src, fdt_len);
 
 	return 0;
 }
diff --git a/include/image.h b/include/image.h
index a8f6bd1..9536874 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1172,4 +1172,19 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr);
  */
 int board_fit_config_name_match(const char *name);
 
+/**
+ * board_fit_image_post_process() - Do any post-process on FIT binary data
+ *
+ * This is used to do any sort of image manipulation, verification, decryption
+ * etc. in a platform or board specific way. Obviously, anything done here would
+ * need to be comprehended in how the images were prepared before being injected
+ * into the FIT creation (i.e. the binary blobs would have been pre-processed
+ * before being added to the FIT image).
+ *
+ * @image: pointer to the image start pointer
+ * @size: pointer to the image size
+ * @return no return value (failure should be handled internally)
+ */
+void board_fit_image_post_process(void **p_image, size_t *p_size);
+
 #endif	/* __IMAGE_H__ */
-- 
2.6.4

[U-Boot] [PATCH 3/9] arm: omap-common: add secure smc entry

[Andreas Dannenberg]

From: Daniel Allred <d-allred@ti.com>

Add an interface for calling secure ROM APIs across a range of OMAP and
OMAP compatible high-security (HS) device variants. While at it, also
perform minor cleanup/alignment without any change in functionality.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/omap-common/lowlevel_init.S | 45 ++++++++++++++++++++------
 arch/arm/include/asm/omap_common.h             |  6 ++++
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap-common/lowlevel_init.S b/arch/arm/cpu/armv7/omap-common/lowlevel_init.S
index 5283135..66a3b3d 100644
--- a/arch/arm/cpu/armv7/omap-common/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/omap-common/lowlevel_init.S
@@ -16,9 +16,10 @@
 #include <asm/arch/spl.h>
 #include <linux/linkage.h>
 
+.arch_extension sec
+
 #ifdef CONFIG_SPL
 ENTRY(save_boot_params)
-
 	ldr	r1, =OMAP_SRAM_SCRATCH_BOOT_PARAMS
 	str	r0, [r1]
 	b	save_boot_params_ret
@@ -26,14 +27,40 @@ ENDPROC(save_boot_params)
 #endif
 
 ENTRY(omap_smc1)
-	PUSH	{r4-r12, lr}	@ save registers - ROM code may pollute
+	push	{r4-r12, lr}	@ save registers - ROM code may pollute
 				@ our registers
-	MOV	r12, r0		@ Service
-	MOV	r0, r1		@ Argument
-	DSB
-	DMB
-	.word	0xe1600070	@ SMC #0 - hand assembled for GCC versions
-				@ call ROM Code API for the service requested
+	mov	r12, r0		@ Service
+	mov	r0, r1		@ Argument
 
-	POP	{r4-r12, pc}
+	dsb
+	dmb
+	smc	0		@ SMC #0 to enter monitor mode
+				@ call ROM Code API for the service requested
+	pop	{r4-r12, pc}
 ENDPROC(omap_smc1)
+
+ENTRY(omap_smc_sec)
+	push	{r4-r12, lr}	@ save registers - ROM code may pollute
+				@ our registers
+	mov	r6, #0xFF	@ Indicate new Task call
+	mov	r12, #0x00	@ Secure Service ID in R12
+
+	dsb
+	dmb
+	smc	0		@ SMC #0 to enter monitor mode
+
+	b	omap_smc_sec_end @ exit at end of the service execution
+	nop
+
+	@ In case of IRQ happening in Secure, then ARM will branch here.
+	@ At that moment, IRQ will be pending and ARM will jump to Non Secure
+	@ IRQ handler
+	mov	r12, #0xFE
+
+	dsb
+	dmb
+	smc	0		@ SMC #0 to enter monitor mode
+
+omap_smc_sec_end:
+	pop	{r4-r12, pc}
+ENDPROC(omap_smc_sec)
diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
index 07f3848..605c549 100644
--- a/arch/arm/include/asm/omap_common.h
+++ b/arch/arm/include/asm/omap_common.h
@@ -627,6 +627,12 @@ void recalibrate_iodelay(void);
 
 void omap_smc1(u32 service, u32 val);
 
+/*
+ * Low-level helper function used when performing secure ROM calls on high-
+ * security (HS) device variants by doing a specially-formed smc entry.
+ */
+u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
+
 void enable_edma3_clocks(void);
 void disable_edma3_clocks(void);
 
-- 
2.6.4

[U-Boot] [PATCH 4/9] arm: omap-common: add secure rom call API for secure devices

[Andreas Dannenberg]

Adds a generic C-callable API for making secure ROM calls on OMAP and
OMAP-compatible devices. This API provides the important function of
flushing the ROM call arguments to memory from the cache, so that the
secure world will have a coherent view of those arguments. Then is
simply calls the omap_smc_sec routine.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/omap-common/Makefile     |  2 ++
 arch/arm/cpu/armv7/omap-common/sec-common.c | 49 +++++++++++++++++++++++++++++
 arch/arm/include/asm/omap_common.h          |  7 +++++
 3 files changed, 58 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/omap-common/sec-common.c

diff --git a/arch/arm/cpu/armv7/omap-common/Makefile b/arch/arm/cpu/armv7/omap-common/Makefile
index 87a7ac0..3172bae 100644
--- a/arch/arm/cpu/armv7/omap-common/Makefile
+++ b/arch/arm/cpu/armv7/omap-common/Makefile
@@ -36,3 +36,5 @@ obj-y	+= boot-common.o
 obj-y	+= lowlevel_init.o
 
 obj-y	+= mem-common.o
+
+obj-$(CONFIG_TI_SECURE_DEVICE) += sec-common.o
diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
new file mode 100644
index 0000000..b9c0a42
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
@@ -0,0 +1,49 @@
+/*
+ *
+ * Common security related functions for OMAP devices
+ *
+ * (C) Copyright 2016
+ * Texas Instruments, <www.ti.com>
+ *
+ * Daniel Allred    <d-allred@ti.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <stdarg.h>
+
+#include <asm/arch/sys_proto.h>
+#include <asm/omap_common.h>
+
+static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
+
+u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
+{
+	int i;
+	u32 num_args;
+	va_list ap;
+
+	va_start(ap, flag);
+
+	num_args = va_arg(ap, u32);
+
+	if (num_args > 4)
+		return 1;
+
+	/* Copy args to aligned args structure */
+	for (i = 0; i < num_args; i++)
+		secure_rom_call_args[i + 1] = va_arg(ap, u32);
+
+	secure_rom_call_args[0] = num_args;
+
+	va_end(ap);
+
+	/* if data cache is enabled, flush the aligned args structure */
+	flush_dcache_range(
+		(unsigned int)&secure_rom_call_args[0],
+		(unsigned int)&secure_rom_call_args[0] +
+		roundup(sizeof(secure_rom_call_args), ARCH_DMA_MINALIGN));
+
+	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
+}
diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
index 605c549..293fc72 100644
--- a/arch/arm/include/asm/omap_common.h
+++ b/arch/arm/include/asm/omap_common.h
@@ -633,6 +633,13 @@ void omap_smc1(u32 service, u32 val);
  */
 u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
 
+/*
+ * Invoke secure ROM API on high-security (HS) device variants. It formats
+ * the variable argument list into the format expected by the ROM code before
+ * triggering the actual low-level smc entry.
+ */
+u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
+
 void enable_edma3_clocks(void);
 void disable_edma3_clocks(void);
 
-- 
2.6.4

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Andreas Dannenberg]

Adds an API that verifies a signature attached to an image (binary
blob). This API is basically a entry to a secure ROM service provided by
the device and accessed via an SMC call, using a particular calling
convention.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
 arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
 arch/arm/include/asm/omap_common.h          |  9 ++++
 2 files changed, 85 insertions(+)

diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
index b9c0a42..dbb9078 100644
--- a/arch/arm/cpu/armv7/omap-common/sec-common.c
+++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
@@ -16,6 +16,9 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/omap_common.h>
 
+/* Index for signature verify ROM API */
+#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
+
 static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
 
 u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
@@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
 
 	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
 }
+
+static u32 find_sig_start(char *image, size_t size)
+{
+	char *image_end = image + size;
+	char *sig_start_magic = "CERT_";
+	int magic_str_len = strlen(sig_start_magic);
+	char *ch;
+
+	while (--image_end > image) {
+		if (*image_end == '_') {
+			ch = image_end - magic_str_len + 1;
+			if (!strncmp(ch, sig_start_magic, magic_str_len))
+				return (u32)ch;
+		}
+	}
+	return 0;
+}
+
+int secure_boot_verify_image(void **image, size_t *size)
+{
+	int result = 1;
+	u32 cert_addr, sig_addr;
+	size_t cert_size;
+
+	/* Perform cache writeback on input buffer */
+	flush_dcache_range(
+		(u32)*image,
+		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
+
+	cert_addr = (uint32_t)*image;
+	sig_addr = find_sig_start((char *)*image, *size);
+
+	if (sig_addr == 0) {
+		printf("No signature found in image.\n");
+		result = 1;
+		goto auth_exit;
+	}
+
+	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
+	cert_size = *size;
+
+	/* Check if image load address is 32-bit aligned */
+	if (0 != (0x3 & cert_addr)) {
+		printf("Image is not 4-byte aligned.\n");
+		result = 1;
+		goto auth_exit;
+	}
+
+	/* Image size also should be multiple of 4 */
+	if (0 != (0x3 & cert_size)) {
+		printf("Image size is not 4-byte aligned.\n");
+		result = 1;
+		goto auth_exit;
+	}
+
+	/* Call ROM HAL API to verify certificate signature */
+	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
+	      cert_addr, cert_size, sig_addr);
+
+	result = secure_rom_call(
+		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
+		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
+auth_exit:
+	if (result != 0) {
+		printf("Authentication failed!\n");
+		printf("Return Value = %08X\n", result);
+		hang();
+	}
+
+	printf("Authentication passed: %s\n", (char *)sig_addr);
+
+	return result;
+}
diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
index 293fc72..8887335 100644
--- a/arch/arm/include/asm/omap_common.h
+++ b/arch/arm/include/asm/omap_common.h
@@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
  */
 u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
 
+/*
+ * Invoke a secure ROM API on high-secure (HS) device variants that can be used
+ * to verify a secure blob by authenticating and optionally decrypting it. The
+ * exact operation performed depends on how the certificate that was embedded
+ * into the blob during the signing/encryption step when the secure blob was
+ * first created.
+ */
+int secure_boot_verify_image(void **p_image, size_t *p_size);
+
 void enable_edma3_clocks(void);
 void disable_edma3_clocks(void);
 
-- 
2.6.4

[U-Boot] [PATCH 6/9] arm: omap-common: Update to generate secure U-Boot FIT blob

[Andreas Dannenberg]

Adds commands so that when a secure device is in use and the SPL is
built to load a FIT image (with combined U-Boot binary and various
DTBs), these components that get fed into the FIT are all processed to
be signed/encrypted/etc. as per the operations performed by the
secure-binary-image.sh script of the TI SECDEV package.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap-common/config_secure.mk b/arch/arm/cpu/armv7/omap-common/config_secure.mk
index c7bb101..c4514ad 100644
--- a/arch/arm/cpu/armv7/omap-common/config_secure.mk
+++ b/arch/arm/cpu/armv7/omap-common/config_secure.mk
@@ -12,8 +12,8 @@ cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \
 	$(if $(KBUILD_VERBOSE:1=), >/dev/null)
 else
 cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \
-    $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \
-    $(if $(KBUILD_VERBOSE:1=), >/dev/null)
+	$(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \
+	$(if $(KBUILD_VERBOSE:1=), >/dev/null)
 endif
 else
 cmd_mkomapsecimg = echo "WARNING:" \
@@ -25,6 +25,26 @@ cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
 	"variable must be defined for TI secure devices. $@ was NOT created!"
 endif
 
+ifdef CONFIG_SPL_LOAD_FIT
+quiet_cmd_omapsecureimg = SECURE  $@
+ifneq ($(TI_SECURE_DEV_PKG),)
+ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)
+cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
+	$< $@ \
+	$(if $(KBUILD_VERBOSE:1=), >/dev/null)
+else
+cmd_omapsecureimg = echo "WARNING:" \
+	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
+	"$@ was NOT created!"; cp $< $@
+endif
+else
+cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
+	"variable must be defined for TI secure devices." \
+	"$@ was NOT created!"; cp $< $@
+endif
+endif
+
+
 # Standard X-LOADER target (QPSI, NOR flash)
 u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin
 	$(call if_changed,mkomapsecimg)
@@ -64,3 +84,36 @@ u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin
 # the mkomapsecimg command looks for a u-boot-HS_* prefix
 u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin
 	$(call if_changed,mkomapsecimg)
+
+# For supporting the SPL loading and interpreting
+# of FIT images whose components are pre-processed
+# before being integrated into the FIT image in order
+# to secure them in some way
+ifdef CONFIG_SPL_LOAD_FIT
+
+MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
+	-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
+	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
+	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
+
+OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
+$(OF_LIST_TARGETS): dtbs
+
+%_HS.dtb: %.dtb
+	$(call if_changed,omapsecureimg)
+	$(Q)if [ -f $@ ]; then \
+		cp -f $@ $<; \
+	fi
+
+u-boot-nodtb_HS.bin: u-boot-nodtb.bin
+	$(call if_changed,omapsecureimg)
+
+u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))
+	$(call if_changed,mkimage)
+	$(Q)if [ -f $@ ]; then \
+		cp -f $@ u-boot.img; \
+	fi
+
+.NOTPARALLEL: dtbs
+
+endif
-- 
2.6.4

[U-Boot] [PATCH 7/9] arm: omap5: add U-Boot FIT signing and SPL image post-processing

[Andreas Dannenberg]

Modify the SPL build procedure for AM57xx and DRA7xx high-security (HS)
device variants to create a secure u-boot_HS.img FIT blob that contains
U-Boot and DTB artifacts signed with a TI-specific process based on the
CONFIG_TI_SECURE_DEVICE config option and the externally-provided image
signing tool.

Also populate the corresponding FIT image post processing call to be
performed during SPL runtime.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
 arch/arm/cpu/armv7/omap5/config.mk | 3 +++
 board/ti/am57xx/board.c            | 7 +++++++
 board/ti/dra7xx/evm.c              | 7 +++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/arm/cpu/armv7/omap5/config.mk b/arch/arm/cpu/armv7/omap5/config.mk
index a7e55a5..d245572 100644
--- a/arch/arm/cpu/armv7/omap5/config.mk
+++ b/arch/arm/cpu/armv7/omap5/config.mk
@@ -15,5 +15,8 @@ else
 ALL-y	+= MLO
 endif
 else
+ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
+ALL-y   += u-boot_HS.img
+endif
 ALL-y	+= u-boot.img
 endif
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c
index 08cf14d..8ded453 100644
--- a/board/ti/am57xx/board.c
+++ b/board/ti/am57xx/board.c
@@ -750,3 +750,10 @@ int board_fit_config_name_match(const char *name)
 		return -1;
 }
 #endif
+
+#ifdef CONFIG_TI_SECURE_DEVICE
+void board_fit_image_post_process(void **p_image, size_t *p_size)
+{
+	secure_boot_verify_image(p_image, p_size);
+}
+#endif
diff --git a/board/ti/dra7xx/evm.c b/board/ti/dra7xx/evm.c
index 0394e4e..a8176b8 100644
--- a/board/ti/dra7xx/evm.c
+++ b/board/ti/dra7xx/evm.c
@@ -830,3 +830,10 @@ int board_fit_config_name_match(const char *name)
 		return -1;
 }
 #endif
+
+#ifdef CONFIG_TI_SECURE_DEVICE
+void board_fit_image_post_process(void **p_image, size_t *p_size)
+{
+	secure_boot_verify_image(p_image, p_size);
+}
+#endif
-- 
2.6.4

[U-Boot] [PATCH 8/9] arm: am4x: add U-Boot FIT signing and SPL image post-processing

[Andreas Dannenberg]

From: Madan Srinivas <madans@ti.com>

Modify the SPL build procedure for AM437x high-security (HS) device
variants to create a secure u-boot_HS.img FIT blob that contains U-Boot
and DTB artifacts signed (and optionally encrypted) with a TI-specific
process based on the CONFIG_TI_SECURE_DEVICE config option and the
externally-provided image signing tool.

Also populate the corresponding FIT image post processing call to be
performed during SPL runtime.

Signed-off-by: Madan Srinivas <madans@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
 arch/arm/cpu/armv7/am33xx/config.mk | 1 +
 board/ti/am43xx/board.c             | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/cpu/armv7/am33xx/config.mk b/arch/arm/cpu/armv7/am33xx/config.mk
index 6d95d32..ab94708 100644
--- a/arch/arm/cpu/armv7/am33xx/config.mk
+++ b/arch/arm/cpu/armv7/am33xx/config.mk
@@ -26,6 +26,7 @@ endif
 else
 ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
 ALL-$(CONFIG_QSPI_BOOT) += u-boot_HS_XIP_X-LOADER
+ALL-y	+= u-boot_HS.img
 endif
 ALL-y	+= u-boot.img
 endif
diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c
index f005762..fc0b38b 100644
--- a/board/ti/am43xx/board.c
+++ b/board/ti/am43xx/board.c
@@ -862,3 +862,10 @@ int board_fit_config_name_match(const char *name)
 		return -1;
 }
 #endif
+
+#ifdef CONFIG_TI_SECURE_DEVICE
+void board_fit_image_post_process(void **p_image, size_t *p_size)
+{
+	secure_boot_verify_image(p_image, p_size);
+}
+#endif
-- 
2.6.4

[U-Boot] [PATCH 9/9] doc: Update info on using secure devices from TI

[Andreas Dannenberg]

From: Daniel Allred <d-allred@ti.com>

Adds information regarding SPL handling the loading
and processing of secured u-boot images as part of
the second stage boot the SPL does. Introduces the
description of a new interface script in the TI
SECDEV Package which handles the creation and prep
of secured binary images.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 doc/README.ti-secure | 170 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 116 insertions(+), 54 deletions(-)

diff --git a/doc/README.ti-secure b/doc/README.ti-secure
index 7fc9b9b..e9d212b 100644
--- a/doc/README.ti-secure
+++ b/doc/README.ti-secure
@@ -19,69 +19,80 @@ control restrictions. Access must be requested and granted by TI before the
 package is viewable and downloadable. Contact TI, either online or by way
 of a local TI representative, to request access.
 
-When CONFIG_TI_SECURE_DEVICE is set, the U-Boot SPL build process requires
-the presence and use of these tools in order to create a viable boot image.
-The build process will look for the environment variable TI_SECURE_DEV_PKG,
-which should be the path of the installed SECDEV package. If the
-TI_SECURE_DEV_PKG variable is not defined or if it is defined but doesn't
-point to a valid SECDEV package, a warning is issued during the build to
-indicate that a final secure bootable image was not created.
-
-Within the SECDEV package exists an image creation script:
-
-${TI_SECURE_DEV_PKG}/scripts/create-boot-image.sh
-
-This is called as part of the SPL/u-boot build process. As the secure boot
-image formats and requirements differ between secure SOC from TI, the
-purpose of this script is to abstract these details as much as possible.
-
-The script is basically the only required interface to the TI SECDEV package
-for secure TI devices.
-
-Invoking the script for AM43xx Secure Devices
-=============================================
-
-create-boot-image.sh <IMAGE_FLAG> <INPUT_FILE> <OUTPUT_FILE> <SPL_LOAD_ADDR>
-
-<IMAGE_FLAG> is a value that specifies the type of the image to generate OR
-the action the image generation tool will take. Valid values are:
-	SPI_X-LOADER - Generates an image for SPI flash (byte swapped)
-	XIP_X-LOADER - Generates a single stage u-boot for NOR/QSPI XiP
-	ISSW - Generates an image for all other boot modes
-
-<INPUT_FILE> is the full path and filename of the public world boot loader
-binary file (depending on the boot media, this is usually either
-u-boot-spl.bin or u-boot.bin).
-
-<OUTPUT_FILE> is the full path and filename of the final secure image. The
-output binary images should be used in place of the standard non-secure
-binary images (see the platform-specific user's guides and releases notes
-for how the non-secure images are typically used)
+Booting of U-Boot SPL
+=====================
+
+	When CONFIG_TI_SECURE_DEVICE is set, the U-Boot SPL build process
+	requires the presence and use of these tools in order to create a
+	viable boot image. The build process will look for the environment
+	variable TI_SECURE_DEV_PKG, which should be the path of the installed
+	SECDEV package. If the TI_SECURE_DEV_PKG variable is not defined or
+	if it is defined but doesn't point to a valid SECDEV package, a
+	warning is issued during the build to indicate that a final secure
+	bootable image was not created.
+
+	Within the SECDEV package exists an image creation script:
+
+	${TI_SECURE_DEV_PKG}/scripts/create-boot-image.sh
+
+	This is called as part of the SPL/u-boot build process. As the secure
+	boot image formats and requirements differ between secure SOC from TI,
+	the purpose of this script is to abstract these details as much as
+	possible.
+
+	The script is basically the only required interface to the TI SECDEV
+	package for creating a bootable SPL image for secure TI devices.
+
+	Invoking the script for AM43xx Secure Devices
+	=============================================
+
+	create-boot-image.sh \
+		<IMAGE_FLAG> <INPUT_FILE> <OUTPUT_FILE> <SPL_LOAD_ADDR>
+
+	<IMAGE_FLAG> is a value that specifies the type of the image to
+	generate OR the action the image generation tool will take. Valid
+	values are:
+		SPI_X-LOADER - Generates an image for SPI flash (byte
+			swapped)
+		XIP_X-LOADER - Generates a single stage u-boot for
+			NOR/QSPI XiP
+		ISSW - Generates an image for all other boot modes
+
+	<INPUT_FILE> is the full path and filename of the public world boot
+	loaderbinary file (depending on the boot media, this is usually
+	either u-boot-spl.bin or u-boot.bin).
+
+	<OUTPUT_FILE> is the full path and filename of the final secure
+	image. The output binary images should be used in place of the standard
+	non-secure binary images (see the platform-specific user's guides and
+	releases notes for how the non-secure images are typically used)
 	u-boot-spl_HS_SPI_X-LOADER - byte swapped boot image for SPI flash
 	u-boot_HS_XIP_X-LOADER - boot image for NOR or QSPI flash
 	u-boot-spl_HS_ISSW - boot image for all other boot media
 
-<SPL_LOAD_ADDR> is the address at which SOC ROM should load the <INPUT_FILE>
+	<SPL_LOAD_ADDR> is the address at which SOC ROM should load the
+	<INPUT_FILE>
 
-Invoking the script for DRA7xx/AM57xx Secure Devices
-====================================================
+	Invoking the script for DRA7xx/AM57xx Secure Devices
+	====================================================
 
-create-boot-image.sh <IMAGE_TYPE> <INPUT_FILE> <OUTPUT_FILE>
+	create-boot-image.sh <IMAGE_TYPE> <INPUT_FILE> <OUTPUT_FILE>
 
-<IMAGE_TYPE> is a value that specifies the type of the image to generate OR
-the action the image generation tool will take. Valid values are:
-	X-LOADER - Generates an image for NOR or QSPI boot modes
-	MLO - Generates an image for SD/MMC/eMMC boot modes
-	ULO - Generates an image for USB/UART peripheral boot modes
-	Note: ULO is not yet used by the u-boot build process
+	<IMAGE_TYPE> is a value that specifies the type of the image to
+	generate OR the action the image generation tool will take. Valid
+	values are:
+		X-LOADER - Generates an image for NOR or QSPI boot modes
+		MLO - Generates an image for SD/MMC/eMMC boot modes
+		ULO - Generates an image for USB/UART peripheral boot modes
+		Note: ULO is not yet used by the u-boot build process
 
-<INPUT_FILE> is the full path and filename of the public world boot loader
-binary file (for this platform, this is always u-boot-spl.bin).
+	<INPUT_FILE> is the full path and filename of the public world boot
+	loader binary file (for this platform, this is always u-boot-spl.bin).
 
-<OUTPUT_FILE> is the full path and filename of the final secure image. The
-output binary images should be used in place of the standard non-secure
-binary images (see the platform-specific user's guides and releases notes
-for how the non-secure images are typically used)
+	<OUTPUT_FILE> is the full path and filename of the final secure image.
+	The output binary images should be used in place of the standard
+	non-secure binary images (see the platform-specific user's guides
+	and releases notes for how the non-secure images are typically used)
 	u-boot-spl_HS_MLO - boot image for SD/MMC/eMMC. This image is
 		copied to a file named MLO, which is the name that
 		the device ROM bootloader requires for loading from
@@ -89,3 +100,54 @@ for how the non-secure images are typically used)
 		non-secure devices)
 	u-boot-spl_HS_X-LOADER - boot image for all other flash memories
 		including QSPI and NOR flash
+
+Booting of Primary U-Boot (u-boot.img)
+======================================
+
+	The SPL image is responsible for loading the next stage boot loader,
+	which is the main u-boot image. For secure TI devices, the SPL will
+	be authenticated, as described above, as part of the particular
+	device's ROM boot process. In order to continue the secure boot
+	process, the authenticated SPL must authenticate the main u-boot
+	image that it loads.
+
+	The configurations for secure TI platforms are written to make the boot
+	process use the FIT image format for the u-boot.img (CONFIG_SPL_FRAMEWORK
+	and CONFIG_SPL_LOAD_FIT). With these configurations the binary
+	components that the SPL loads include a specific DTB image and u-boot
+	image. These DTB image may be one of many available to the boot
+	process. In order to secure these components so that they can be
+	authenticated by the SPL as they are loaded from the FIT image,	the
+	build procedure for secure TI devices will secure these images before
+	they are integrated into the FIT image. When those images are extracted
+	from the FIT image at boot time, they are post-processed to verify that
+	they are still secure.
+
+	The exact details of the how the images are secured is handled by the
+	SECDEV package. Within the SECDEV package exists a script to process
+	an input binary image:
+
+	${TI_SECURE_DEV_PKG}/scripts/secure-binary-image.sh
+
+	This is called as part of the u-boot build process. As the secure
+	image formats and requirements can differ between the various secure
+	SOCs from TI, this script in the SECDEV package abstracts these
+	details. This script is essentially the only required interface to the
+	TI SECDEV package for creating a u-boot.img image for secure TI
+	devices.
+
+	The SPL/u-boot code contains calls to dedicated secure ROM functions
+	to perform the validation on the secured images. The details of the
+	interface to those functions is shown in the code. The summary
+	is that they are accessed by invoking an ARM secure monitor call to
+	the device's secure ROM (fixed read-only-memory that is secure and
+	only accessible when the ARM core is operating in the secure mode).
+
+	Invoking the secure-binary-image script for Secure Devices
+	==========================================================
+
+	secure-binary-image.sh <INPUT_FILE> <OUTPUT_FILE>
+
+	<INPUT_FILE> is the full path and filename of the input binary image
+
+	<OUTPUT_FILE> is the full path and filename of the output secure image.
-- 
2.6.4

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Lokesh Vutla]

On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> Adds an API that verifies a signature attached to an image (binary
> blob). This API is basically a entry to a secure ROM service provided by
> the device and accessed via an SMC call, using a particular calling
> convention.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
>  arch/arm/include/asm/omap_common.h          |  9 ++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> index b9c0a42..dbb9078 100644
> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> @@ -16,6 +16,9 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm/omap_common.h>
>  
> +/* Index for signature verify ROM API */
> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> +
>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
>  
>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
>  
>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
>  }
> +
> +static u32 find_sig_start(char *image, size_t size)
> +{
> +	char *image_end = image + size;
> +	char *sig_start_magic = "CERT_";
> +	int magic_str_len = strlen(sig_start_magic);
> +	char *ch;
> +
> +	while (--image_end > image) {
> +		if (*image_end == '_') {
> +			ch = image_end - magic_str_len + 1;
> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> +				return (u32)ch;
> +		}
> +	}
> +	return 0;
> +}
> +
> +int secure_boot_verify_image(void **image, size_t *size)
> +{
> +	int result = 1;
> +	u32 cert_addr, sig_addr;
> +	size_t cert_size;
> +
> +	/* Perform cache writeback on input buffer */
> +	flush_dcache_range(
> +		(u32)*image,
> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> +
> +	cert_addr = (uint32_t)*image;
> +	sig_addr = find_sig_start((char *)*image, *size);
> +
> +	if (sig_addr == 0) {
> +		printf("No signature found in image.\n");
> +		result = 1;
> +		goto auth_exit;
> +	}
> +
> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> +	cert_size = *size;
> +
> +	/* Check if image load address is 32-bit aligned */
> +	if (0 != (0x3 & cert_addr)) {

	if (!IS_ALIGNED(cert_addr, 4)) { ?

> +		printf("Image is not 4-byte aligned.\n");
> +		result = 1;
> +		goto auth_exit;
> +	}
> +
> +	/* Image size also should be multiple of 4 */
> +	if (0 != (0x3 & cert_size)) {

	if (!IS_ALIGNED(cert_size, 4)) { ?

> +		printf("Image size is not 4-byte aligned.\n");
> +		result = 1;
> +		goto auth_exit;
> +	}
> +
> +	/* Call ROM HAL API to verify certificate signature */
> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> +	      cert_addr, cert_size, sig_addr);
> +
> +	result = secure_rom_call(
> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> +auth_exit:
> +	if (result != 0) {
> +		printf("Authentication failed!\n");
> +		printf("Return Value = %08X\n", result);
> +		hang();
> +	}
> +
> +	printf("Authentication passed: %s\n", (char *)sig_addr);

Uart boot will break because of these prints during the FIT loading. Can
you make this as debug?

Prints in the failed case is fine as we need to know if it is failed.

Thanks and regards,
Lokesh

> +
> +	return result;
> +}
> diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
> index 293fc72..8887335 100644
> --- a/arch/arm/include/asm/omap_common.h
> +++ b/arch/arm/include/asm/omap_common.h
> @@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
>   */
>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
>  
> +/*
> + * Invoke a secure ROM API on high-secure (HS) device variants that can be used
> + * to verify a secure blob by authenticating and optionally decrypting it. The
> + * exact operation performed depends on how the certificate that was embedded
> + * into the blob during the signing/encryption step when the secure blob was
> + * first created.
> + */
> +int secure_boot_verify_image(void **p_image, size_t *p_size);
> +
>  void enable_edma3_clocks(void);
>  void disable_edma3_clocks(void);
>  
> 

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Andreas Dannenberg]

Hi Lokesh--- comment inlined...

On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
> 
> 
> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> > Adds an API that verifies a signature attached to an image (binary
> > blob). This API is basically a entry to a secure ROM service provided by
> > the device and accessed via an SMC call, using a particular calling
> > convention.
> > 
> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> >  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
> >  arch/arm/include/asm/omap_common.h          |  9 ++++
> >  2 files changed, 85 insertions(+)
> > 
> > diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > index b9c0a42..dbb9078 100644
> > --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> > +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > @@ -16,6 +16,9 @@
> >  #include <asm/arch/sys_proto.h>
> >  #include <asm/omap_common.h>
> >  
> > +/* Index for signature verify ROM API */
> > +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> > +
> >  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
> >  
> >  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> >  
> >  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
> >  }
> > +
> > +static u32 find_sig_start(char *image, size_t size)
> > +{
> > +	char *image_end = image + size;
> > +	char *sig_start_magic = "CERT_";
> > +	int magic_str_len = strlen(sig_start_magic);
> > +	char *ch;
> > +
> > +	while (--image_end > image) {
> > +		if (*image_end == '_') {
> > +			ch = image_end - magic_str_len + 1;
> > +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> > +				return (u32)ch;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +int secure_boot_verify_image(void **image, size_t *size)
> > +{
> > +	int result = 1;
> > +	u32 cert_addr, sig_addr;
> > +	size_t cert_size;
> > +
> > +	/* Perform cache writeback on input buffer */
> > +	flush_dcache_range(
> > +		(u32)*image,
> > +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> > +
> > +	cert_addr = (uint32_t)*image;
> > +	sig_addr = find_sig_start((char *)*image, *size);
> > +
> > +	if (sig_addr == 0) {
> > +		printf("No signature found in image.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> > +	cert_size = *size;
> > +
> > +	/* Check if image load address is 32-bit aligned */
> > +	if (0 != (0x3 & cert_addr)) {
> 
> 	if (!IS_ALIGNED(cert_addr, 4)) { ?

Good suggestion! Will simplify.

> > +		printf("Image is not 4-byte aligned.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	/* Image size also should be multiple of 4 */
> > +	if (0 != (0x3 & cert_size)) {
> 
> 	if (!IS_ALIGNED(cert_size, 4)) { ?

ditto.

> > +		printf("Image size is not 4-byte aligned.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	/* Call ROM HAL API to verify certificate signature */
> > +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> > +	      cert_addr, cert_size, sig_addr);
> > +
> > +	result = secure_rom_call(
> > +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> > +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> > +auth_exit:
> > +	if (result != 0) {
> > +		printf("Authentication failed!\n");
> > +		printf("Return Value = %08X\n", result);
> > +		hang();
> > +	}
> > +
> > +	printf("Authentication passed: %s\n", (char *)sig_addr);
> 
> Uart boot will break because of these prints during the FIT loading. Can
> you make this as debug?
> 
> Prints in the failed case is fine as we need to know if it is failed.

Oh yes, the UART boot. We actually explicitly added this printf for
successful authentication as the regular U-Boot verified boot also uses
some form of "positive feedback" (in form of a '+' sign or so) and we
felt its important feedback to _always_ provide some assurance that the
chain of trust is actually established (and not interrupted by a build
or setup error or something else).

So let's say we do want to keep this output, are there any other
options? Like suppressing it only in case of SPL UART boot, but not in
any of the other modes (sounds ugly but maybe there is another way)?

Thanks,
Andreas


> 
> Thanks and regards,
> Lokesh
> 
> > +
> > +	return result;
> > +}
> > diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
> > index 293fc72..8887335 100644
> > --- a/arch/arm/include/asm/omap_common.h
> > +++ b/arch/arm/include/asm/omap_common.h
> > @@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
> >   */
> >  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
> >  
> > +/*
> > + * Invoke a secure ROM API on high-secure (HS) device variants that can be used
> > + * to verify a secure blob by authenticating and optionally decrypting it. The
> > + * exact operation performed depends on how the certificate that was embedded
> > + * into the blob during the signing/encryption step when the secure blob was
> > + * first created.
> > + */
> > +int secure_boot_verify_image(void **p_image, size_t *p_size);
> > +
> >  void enable_edma3_clocks(void);
> >  void disable_edma3_clocks(void);
> >  
> > 

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Lokesh Vutla]

On Tuesday 21 June 2016 10:32 AM, Andreas Dannenberg wrote:
> Hi Lokesh--- comment inlined...
> 
> On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
>>
>>
>> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
>>> Adds an API that verifies a signature attached to an image (binary
>>> blob). This API is basically a entry to a secure ROM service provided by
>>> the device and accessed via an SMC call, using a particular calling
>>> convention.
>>>
>>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
>>> ---
>>>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
>>>  arch/arm/include/asm/omap_common.h          |  9 ++++
>>>  2 files changed, 85 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> index b9c0a42..dbb9078 100644
>>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> @@ -16,6 +16,9 @@
>>>  #include <asm/arch/sys_proto.h>
>>>  #include <asm/omap_common.h>
>>>  
>>> +/* Index for signature verify ROM API */
>>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
>>> +
>>>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
>>>  
>>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
>>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
>>>  
>>>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
>>>  }
>>> +
>>> +static u32 find_sig_start(char *image, size_t size)
>>> +{
>>> +	char *image_end = image + size;
>>> +	char *sig_start_magic = "CERT_";
>>> +	int magic_str_len = strlen(sig_start_magic);
>>> +	char *ch;
>>> +
>>> +	while (--image_end > image) {
>>> +		if (*image_end == '_') {
>>> +			ch = image_end - magic_str_len + 1;
>>> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
>>> +				return (u32)ch;
>>> +		}
>>> +	}
>>> +	return 0;
>>> +}
>>> +
>>> +int secure_boot_verify_image(void **image, size_t *size)
>>> +{
>>> +	int result = 1;
>>> +	u32 cert_addr, sig_addr;
>>> +	size_t cert_size;
>>> +
>>> +	/* Perform cache writeback on input buffer */
>>> +	flush_dcache_range(
>>> +		(u32)*image,
>>> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
>>> +
>>> +	cert_addr = (uint32_t)*image;
>>> +	sig_addr = find_sig_start((char *)*image, *size);
>>> +
>>> +	if (sig_addr == 0) {
>>> +		printf("No signature found in image.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
>>> +	cert_size = *size;
>>> +
>>> +	/* Check if image load address is 32-bit aligned */
>>> +	if (0 != (0x3 & cert_addr)) {
>>
>> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
> 
> Good suggestion! Will simplify.
> 
>>> +		printf("Image is not 4-byte aligned.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	/* Image size also should be multiple of 4 */
>>> +	if (0 != (0x3 & cert_size)) {
>>
>> 	if (!IS_ALIGNED(cert_size, 4)) { ?
> 
> ditto.
> 
>>> +		printf("Image size is not 4-byte aligned.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	/* Call ROM HAL API to verify certificate signature */
>>> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
>>> +	      cert_addr, cert_size, sig_addr);
>>> +
>>> +	result = secure_rom_call(
>>> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
>>> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
>>> +auth_exit:
>>> +	if (result != 0) {
>>> +		printf("Authentication failed!\n");
>>> +		printf("Return Value = %08X\n", result);
>>> +		hang();
>>> +	}
>>> +
>>> +	printf("Authentication passed: %s\n", (char *)sig_addr);
>>
>> Uart boot will break because of these prints during the FIT loading. Can
>> you make this as debug?
>>
>> Prints in the failed case is fine as we need to know if it is failed.
> 
> Oh yes, the UART boot. We actually explicitly added this printf for
> successful authentication as the regular U-Boot verified boot also uses
> some form of "positive feedback" (in form of a '+' sign or so) and we
> felt its important feedback to _always_ provide some assurance that the
> chain of trust is actually established (and not interrupted by a build
> or setup error or something else).
> 
> So let's say we do want to keep this output, are there any other
> options? Like suppressing it only in case of SPL UART boot, but not in
> any of the other modes (sounds ugly but maybe there is another way)?

Hmm.. you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the
config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do.

Thanks and regards,
Lokesh

> 
> Thanks,
> Andreas
> 
> 
>>
>> Thanks and regards,
>> Lokesh
>>
>>> +
>>> +	return result;
>>> +}
>>> diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
>>> index 293fc72..8887335 100644
>>> --- a/arch/arm/include/asm/omap_common.h
>>> +++ b/arch/arm/include/asm/omap_common.h
>>> @@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params);
>>>   */
>>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
>>>  
>>> +/*
>>> + * Invoke a secure ROM API on high-secure (HS) device variants that can be used
>>> + * to verify a secure blob by authenticating and optionally decrypting it. The
>>> + * exact operation performed depends on how the certificate that was embedded
>>> + * into the blob during the signing/encryption step when the secure blob was
>>> + * first created.
>>> + */
>>> +int secure_boot_verify_image(void **p_image, size_t *p_size);
>>> +
>>>  void enable_edma3_clocks(void);
>>>  void disable_edma3_clocks(void);
>>>  
>>>

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Tom Rini]

On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
> 
> 
> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> > Adds an API that verifies a signature attached to an image (binary
> > blob). This API is basically a entry to a secure ROM service provided by
> > the device and accessed via an SMC call, using a particular calling
> > convention.
> > 
> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> >  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
> >  arch/arm/include/asm/omap_common.h          |  9 ++++
> >  2 files changed, 85 insertions(+)
> > 
> > diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > index b9c0a42..dbb9078 100644
> > --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> > +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > @@ -16,6 +16,9 @@
> >  #include <asm/arch/sys_proto.h>
> >  #include <asm/omap_common.h>
> >  
> > +/* Index for signature verify ROM API */
> > +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> > +
> >  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
> >  
> >  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> >  
> >  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
> >  }
> > +
> > +static u32 find_sig_start(char *image, size_t size)
> > +{
> > +	char *image_end = image + size;
> > +	char *sig_start_magic = "CERT_";
> > +	int magic_str_len = strlen(sig_start_magic);
> > +	char *ch;
> > +
> > +	while (--image_end > image) {
> > +		if (*image_end == '_') {
> > +			ch = image_end - magic_str_len + 1;
> > +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> > +				return (u32)ch;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +int secure_boot_verify_image(void **image, size_t *size)
> > +{
> > +	int result = 1;
> > +	u32 cert_addr, sig_addr;
> > +	size_t cert_size;
> > +
> > +	/* Perform cache writeback on input buffer */
> > +	flush_dcache_range(
> > +		(u32)*image,
> > +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> > +
> > +	cert_addr = (uint32_t)*image;
> > +	sig_addr = find_sig_start((char *)*image, *size);
> > +
> > +	if (sig_addr == 0) {
> > +		printf("No signature found in image.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> > +	cert_size = *size;
> > +
> > +	/* Check if image load address is 32-bit aligned */
> > +	if (0 != (0x3 & cert_addr)) {
> 
> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
> 
> > +		printf("Image is not 4-byte aligned.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	/* Image size also should be multiple of 4 */
> > +	if (0 != (0x3 & cert_size)) {
> 
> 	if (!IS_ALIGNED(cert_size, 4)) { ?
> 
> > +		printf("Image size is not 4-byte aligned.\n");
> > +		result = 1;
> > +		goto auth_exit;
> > +	}
> > +
> > +	/* Call ROM HAL API to verify certificate signature */
> > +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> > +	      cert_addr, cert_size, sig_addr);
> > +
> > +	result = secure_rom_call(
> > +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> > +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> > +auth_exit:
> > +	if (result != 0) {
> > +		printf("Authentication failed!\n");
> > +		printf("Return Value = %08X\n", result);
> > +		hang();
> > +	}
> > +
> > +	printf("Authentication passed: %s\n", (char *)sig_addr);
> 
> Uart boot will break because of these prints during the FIT loading. Can
> you make this as debug?

Are you sure it will break?  There's usually a print in between loading
SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough
to re-transmit.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/9185317e/attachment.sig>

[U-Boot] [PATCH 4/9] arm: omap-common: add secure rom call API for secure devices

[Tom Rini]

On Mon, Jun 20, 2016 at 10:34:07PM -0500, Andreas Dannenberg wrote:

> Adds a generic C-callable API for making secure ROM calls on OMAP and
> OMAP-compatible devices. This API provides the important function of
> flushing the ROM call arguments to memory from the cache, so that the
> secure world will have a coherent view of those arguments. Then is
> simply calls the omap_smc_sec routine.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/4317a526/attachment.sig>

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Tom Rini]

On Mon, Jun 20, 2016 at 10:34:05PM -0500, Andreas Dannenberg wrote:

> From: Daniel Allred <d-allred@ti.com>
> 
> The next stage boot loader image and the selected FDT can be
> post-processed by board/platform/device-specific code, which can include
> modifying the size and altering the starting source address before
> copying these binary blobs to their final desitination. This might be
> desired to do things like strip headers or footers attached to the
> images before they were packeaged into the FIT, or to perform operations
> such as decryption or authentication.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/d49c0100/attachment.sig>

[U-Boot] [PATCH 3/9] arm: omap-common: add secure smc entry

[Tom Rini]

On Mon, Jun 20, 2016 at 10:34:06PM -0500, Andreas Dannenberg wrote:

> From: Daniel Allred <d-allred@ti.com>
> 
> Add an interface for calling secure ROM APIs across a range of OMAP and
> OMAP compatible high-security (HS) device variants. While at it, also
> perform minor cleanup/alignment without any change in functionality.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/8b8f4860/attachment.sig>

[U-Boot] [PATCH 8/9] arm: am4x: add U-Boot FIT signing and SPL image post-processing

[Tom Rini]

On Mon, Jun 20, 2016 at 10:34:11PM -0500, Andreas Dannenberg wrote:

> From: Madan Srinivas <madans@ti.com>
> 
> Modify the SPL build procedure for AM437x high-security (HS) device
> variants to create a secure u-boot_HS.img FIT blob that contains U-Boot
> and DTB artifacts signed (and optionally encrypted) with a TI-specific
> process based on the CONFIG_TI_SECURE_DEVICE config option and the
> externally-provided image signing tool.
> 
> Also populate the corresponding FIT image post processing call to be
> performed during SPL runtime.
> 
> Signed-off-by: Madan Srinivas <madans@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/a72f26aa/attachment.sig>

[U-Boot] [PATCH 7/9] arm: omap5: add U-Boot FIT signing and SPL image post-processing

[Tom Rini]

On Mon, Jun 20, 2016 at 10:34:10PM -0500, Andreas Dannenberg wrote:

> Modify the SPL build procedure for AM57xx and DRA7xx high-security (HS)
> device variants to create a secure u-boot_HS.img FIT blob that contains
> U-Boot and DTB artifacts signed with a TI-specific process based on the
> CONFIG_TI_SECURE_DEVICE config option and the externally-provided image
> signing tool.
> 
> Also populate the corresponding FIT image post processing call to be
> performed during SPL runtime.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160621/4c453fb5/attachment.sig>

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Lokesh Vutla]

On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote:
> On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
>>
>>
>> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
>>> Adds an API that verifies a signature attached to an image (binary
>>> blob). This API is basically a entry to a secure ROM service provided by
>>> the device and accessed via an SMC call, using a particular calling
>>> convention.
>>>
>>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
>>> ---
>>>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
>>>  arch/arm/include/asm/omap_common.h          |  9 ++++
>>>  2 files changed, 85 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> index b9c0a42..dbb9078 100644
>>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
>>> @@ -16,6 +16,9 @@
>>>  #include <asm/arch/sys_proto.h>
>>>  #include <asm/omap_common.h>
>>>  
>>> +/* Index for signature verify ROM API */
>>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
>>> +
>>>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
>>>  
>>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
>>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
>>>  
>>>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
>>>  }
>>> +
>>> +static u32 find_sig_start(char *image, size_t size)
>>> +{
>>> +	char *image_end = image + size;
>>> +	char *sig_start_magic = "CERT_";
>>> +	int magic_str_len = strlen(sig_start_magic);
>>> +	char *ch;
>>> +
>>> +	while (--image_end > image) {
>>> +		if (*image_end == '_') {
>>> +			ch = image_end - magic_str_len + 1;
>>> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
>>> +				return (u32)ch;
>>> +		}
>>> +	}
>>> +	return 0;
>>> +}
>>> +
>>> +int secure_boot_verify_image(void **image, size_t *size)
>>> +{
>>> +	int result = 1;
>>> +	u32 cert_addr, sig_addr;
>>> +	size_t cert_size;
>>> +
>>> +	/* Perform cache writeback on input buffer */
>>> +	flush_dcache_range(
>>> +		(u32)*image,
>>> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
>>> +
>>> +	cert_addr = (uint32_t)*image;
>>> +	sig_addr = find_sig_start((char *)*image, *size);
>>> +
>>> +	if (sig_addr == 0) {
>>> +		printf("No signature found in image.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
>>> +	cert_size = *size;
>>> +
>>> +	/* Check if image load address is 32-bit aligned */
>>> +	if (0 != (0x3 & cert_addr)) {
>>
>> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
>>
>>> +		printf("Image is not 4-byte aligned.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	/* Image size also should be multiple of 4 */
>>> +	if (0 != (0x3 & cert_size)) {
>>
>> 	if (!IS_ALIGNED(cert_size, 4)) { ?
>>
>>> +		printf("Image size is not 4-byte aligned.\n");
>>> +		result = 1;
>>> +		goto auth_exit;
>>> +	}
>>> +
>>> +	/* Call ROM HAL API to verify certificate signature */
>>> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
>>> +	      cert_addr, cert_size, sig_addr);
>>> +
>>> +	result = secure_rom_call(
>>> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
>>> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
>>> +auth_exit:
>>> +	if (result != 0) {
>>> +		printf("Authentication failed!\n");
>>> +		printf("Return Value = %08X\n", result);
>>> +		hang();
>>> +	}
>>> +
>>> +	printf("Authentication passed: %s\n", (char *)sig_addr);
>>
>> Uart boot will break because of these prints during the FIT loading. Can
>> you make this as debug?
> 
> Are you sure it will break?  There's usually a print in between loading
> SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough
> to re-transmit.
> 

Yes, if the print is in between while Y-MODEM is transferring. The above
print falls in this case.

Thanks and regards,
Lokesh

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Andreas Dannenberg]

On Wed, Jun 22, 2016 at 03:13:04PM +0530, Lokesh Vutla wrote:
> 
> 
> On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote:
> > On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
> >>
> >>
> >> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> >>> Adds an API that verifies a signature attached to an image (binary
> >>> blob). This API is basically a entry to a secure ROM service provided by
> >>> the device and accessed via an SMC call, using a particular calling
> >>> convention.
> >>>
> >>> Signed-off-by: Daniel Allred <d-allred@ti.com>
> >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> >>> ---
> >>>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
> >>>  arch/arm/include/asm/omap_common.h          |  9 ++++
> >>>  2 files changed, 85 insertions(+)
> >>>
> >>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> >>> index b9c0a42..dbb9078 100644
> >>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> >>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> >>> @@ -16,6 +16,9 @@
> >>>  #include <asm/arch/sys_proto.h>
> >>>  #include <asm/omap_common.h>
> >>>  
> >>> +/* Index for signature verify ROM API */
> >>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> >>> +
> >>>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
> >>>  
> >>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> >>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> >>>  
> >>>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
> >>>  }
> >>> +
> >>> +static u32 find_sig_start(char *image, size_t size)
> >>> +{
> >>> +	char *image_end = image + size;
> >>> +	char *sig_start_magic = "CERT_";
> >>> +	int magic_str_len = strlen(sig_start_magic);
> >>> +	char *ch;
> >>> +
> >>> +	while (--image_end > image) {
> >>> +		if (*image_end == '_') {
> >>> +			ch = image_end - magic_str_len + 1;
> >>> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> >>> +				return (u32)ch;
> >>> +		}
> >>> +	}
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +int secure_boot_verify_image(void **image, size_t *size)
> >>> +{
> >>> +	int result = 1;
> >>> +	u32 cert_addr, sig_addr;
> >>> +	size_t cert_size;
> >>> +
> >>> +	/* Perform cache writeback on input buffer */
> >>> +	flush_dcache_range(
> >>> +		(u32)*image,
> >>> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> >>> +
> >>> +	cert_addr = (uint32_t)*image;
> >>> +	sig_addr = find_sig_start((char *)*image, *size);
> >>> +
> >>> +	if (sig_addr == 0) {
> >>> +		printf("No signature found in image.\n");
> >>> +		result = 1;
> >>> +		goto auth_exit;
> >>> +	}
> >>> +
> >>> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> >>> +	cert_size = *size;
> >>> +
> >>> +	/* Check if image load address is 32-bit aligned */
> >>> +	if (0 != (0x3 & cert_addr)) {
> >>
> >> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
> >>
> >>> +		printf("Image is not 4-byte aligned.\n");
> >>> +		result = 1;
> >>> +		goto auth_exit;
> >>> +	}
> >>> +
> >>> +	/* Image size also should be multiple of 4 */
> >>> +	if (0 != (0x3 & cert_size)) {
> >>
> >> 	if (!IS_ALIGNED(cert_size, 4)) { ?
> >>
> >>> +		printf("Image size is not 4-byte aligned.\n");
> >>> +		result = 1;
> >>> +		goto auth_exit;
> >>> +	}
> >>> +
> >>> +	/* Call ROM HAL API to verify certificate signature */
> >>> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> >>> +	      cert_addr, cert_size, sig_addr);
> >>> +
> >>> +	result = secure_rom_call(
> >>> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> >>> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> >>> +auth_exit:
> >>> +	if (result != 0) {
> >>> +		printf("Authentication failed!\n");
> >>> +		printf("Return Value = %08X\n", result);
> >>> +		hang();
> >>> +	}
> >>> +
> >>> +	printf("Authentication passed: %s\n", (char *)sig_addr);
> >>
> >> Uart boot will break because of these prints during the FIT loading. Can
> >> you make this as debug?
> > 
> > Are you sure it will break?  There's usually a print in between loading
> > SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough
> > to re-transmit.
> > 
> 
> Yes, if the print is in between while Y-MODEM is transferring. The above
> print falls in this case.

Tom et al.,
so if this really breaks stuff I need to do something about it. As said
I'd really like to keep the "Authentication passed: <certificate name>"
message in the boot log. So if I implement something along the lines
what Lokesh suggested:

"...you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the                           
config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do..."

to selectivly suppress the message in case of UART boot, would this be
acceptable? Or is there a better way?

Thanks and Regards,
Andreas

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Tom Rini]

On Wed, Jun 22, 2016 at 09:21:28AM -0500, Andreas Dannenberg wrote:
> On Wed, Jun 22, 2016 at 03:13:04PM +0530, Lokesh Vutla wrote:
> > 
> > 
> > On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote:
> > > On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
> > >>
> > >>
> > >> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> > >>> Adds an API that verifies a signature attached to an image (binary
> > >>> blob). This API is basically a entry to a secure ROM service provided by
> > >>> the device and accessed via an SMC call, using a particular calling
> > >>> convention.
> > >>>
> > >>> Signed-off-by: Daniel Allred <d-allred@ti.com>
> > >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > >>> ---
> > >>>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
> > >>>  arch/arm/include/asm/omap_common.h          |  9 ++++
> > >>>  2 files changed, 85 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > >>> index b9c0a42..dbb9078 100644
> > >>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> > >>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > >>> @@ -16,6 +16,9 @@
> > >>>  #include <asm/arch/sys_proto.h>
> > >>>  #include <asm/omap_common.h>
> > >>>  
> > >>> +/* Index for signature verify ROM API */
> > >>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> > >>> +
> > >>>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
> > >>>  
> > >>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > >>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > >>>  
> > >>>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
> > >>>  }
> > >>> +
> > >>> +static u32 find_sig_start(char *image, size_t size)
> > >>> +{
> > >>> +	char *image_end = image + size;
> > >>> +	char *sig_start_magic = "CERT_";
> > >>> +	int magic_str_len = strlen(sig_start_magic);
> > >>> +	char *ch;
> > >>> +
> > >>> +	while (--image_end > image) {
> > >>> +		if (*image_end == '_') {
> > >>> +			ch = image_end - magic_str_len + 1;
> > >>> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> > >>> +				return (u32)ch;
> > >>> +		}
> > >>> +	}
> > >>> +	return 0;
> > >>> +}
> > >>> +
> > >>> +int secure_boot_verify_image(void **image, size_t *size)
> > >>> +{
> > >>> +	int result = 1;
> > >>> +	u32 cert_addr, sig_addr;
> > >>> +	size_t cert_size;
> > >>> +
> > >>> +	/* Perform cache writeback on input buffer */
> > >>> +	flush_dcache_range(
> > >>> +		(u32)*image,
> > >>> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> > >>> +
> > >>> +	cert_addr = (uint32_t)*image;
> > >>> +	sig_addr = find_sig_start((char *)*image, *size);
> > >>> +
> > >>> +	if (sig_addr == 0) {
> > >>> +		printf("No signature found in image.\n");
> > >>> +		result = 1;
> > >>> +		goto auth_exit;
> > >>> +	}
> > >>> +
> > >>> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> > >>> +	cert_size = *size;
> > >>> +
> > >>> +	/* Check if image load address is 32-bit aligned */
> > >>> +	if (0 != (0x3 & cert_addr)) {
> > >>
> > >> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
> > >>
> > >>> +		printf("Image is not 4-byte aligned.\n");
> > >>> +		result = 1;
> > >>> +		goto auth_exit;
> > >>> +	}
> > >>> +
> > >>> +	/* Image size also should be multiple of 4 */
> > >>> +	if (0 != (0x3 & cert_size)) {
> > >>
> > >> 	if (!IS_ALIGNED(cert_size, 4)) { ?
> > >>
> > >>> +		printf("Image size is not 4-byte aligned.\n");
> > >>> +		result = 1;
> > >>> +		goto auth_exit;
> > >>> +	}
> > >>> +
> > >>> +	/* Call ROM HAL API to verify certificate signature */
> > >>> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> > >>> +	      cert_addr, cert_size, sig_addr);
> > >>> +
> > >>> +	result = secure_rom_call(
> > >>> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> > >>> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> > >>> +auth_exit:
> > >>> +	if (result != 0) {
> > >>> +		printf("Authentication failed!\n");
> > >>> +		printf("Return Value = %08X\n", result);
> > >>> +		hang();
> > >>> +	}
> > >>> +
> > >>> +	printf("Authentication passed: %s\n", (char *)sig_addr);
> > >>
> > >> Uart boot will break because of these prints during the FIT loading. Can
> > >> you make this as debug?
> > > 
> > > Are you sure it will break?  There's usually a print in between loading
> > > SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough
> > > to re-transmit.
> > > 
> > 
> > Yes, if the print is in between while Y-MODEM is transferring. The above
> > print falls in this case.

... but Y-MODEM (the protocol) does retransmit.  It should recover from
this message.

> Tom et al.,
> so if this really breaks stuff I need to do something about it. As said
> I'd really like to keep the "Authentication passed: <certificate name>"
> message in the boot log. So if I implement something along the lines
> what Lokesh suggested:
> 
> "...you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the                           
> config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do..."
> 
> to selectivly suppress the message in case of UART boot, would this be
> acceptable? Or is there a better way?

At worst case, yes, we can case this around !CONFIG_SPL_YMODEM_SUPPORT.
But I keep thinking the world should recover from this too.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160622/8bd9dd5c/attachment.sig>

[U-Boot] [PATCH 5/9] arm: omap-common: secure ROM signature verify API

[Andreas Dannenberg]

On Wed, Jun 22, 2016 at 10:36:27AM -0400, Tom Rini wrote:
> On Wed, Jun 22, 2016 at 09:21:28AM -0500, Andreas Dannenberg wrote:
> > On Wed, Jun 22, 2016 at 03:13:04PM +0530, Lokesh Vutla wrote:
> > > 
> > > 
> > > On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote:
> > > > On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote:
> > > >>
> > > >>
> > > >> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote:
> > > >>> Adds an API that verifies a signature attached to an image (binary
> > > >>> blob). This API is basically a entry to a secure ROM service provided by
> > > >>> the device and accessed via an SMC call, using a particular calling
> > > >>> convention.
> > > >>>
> > > >>> Signed-off-by: Daniel Allred <d-allred@ti.com>
> > > >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > > >>> ---
> > > >>>  arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++
> > > >>>  arch/arm/include/asm/omap_common.h          |  9 ++++
> > > >>>  2 files changed, 85 insertions(+)
> > > >>>
> > > >>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > > >>> index b9c0a42..dbb9078 100644
> > > >>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c
> > > >>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c
> > > >>> @@ -16,6 +16,9 @@
> > > >>>  #include <asm/arch/sys_proto.h>
> > > >>>  #include <asm/omap_common.h>
> > > >>>  
> > > >>> +/* Index for signature verify ROM API */
> > > >>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX	(0x0000000E)
> > > >>> +
> > > >>>  static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN);
> > > >>>  
> > > >>>  u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > > >>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...)
> > > >>>  
> > > >>>  	return omap_smc_sec(service, proc_id, flag, secure_rom_call_args);
> > > >>>  }
> > > >>> +
> > > >>> +static u32 find_sig_start(char *image, size_t size)
> > > >>> +{
> > > >>> +	char *image_end = image + size;
> > > >>> +	char *sig_start_magic = "CERT_";
> > > >>> +	int magic_str_len = strlen(sig_start_magic);
> > > >>> +	char *ch;
> > > >>> +
> > > >>> +	while (--image_end > image) {
> > > >>> +		if (*image_end == '_') {
> > > >>> +			ch = image_end - magic_str_len + 1;
> > > >>> +			if (!strncmp(ch, sig_start_magic, magic_str_len))
> > > >>> +				return (u32)ch;
> > > >>> +		}
> > > >>> +	}
> > > >>> +	return 0;
> > > >>> +}
> > > >>> +
> > > >>> +int secure_boot_verify_image(void **image, size_t *size)
> > > >>> +{
> > > >>> +	int result = 1;
> > > >>> +	u32 cert_addr, sig_addr;
> > > >>> +	size_t cert_size;
> > > >>> +
> > > >>> +	/* Perform cache writeback on input buffer */
> > > >>> +	flush_dcache_range(
> > > >>> +		(u32)*image,
> > > >>> +		(u32)*image + roundup(*size, ARCH_DMA_MINALIGN));
> > > >>> +
> > > >>> +	cert_addr = (uint32_t)*image;
> > > >>> +	sig_addr = find_sig_start((char *)*image, *size);
> > > >>> +
> > > >>> +	if (sig_addr == 0) {
> > > >>> +		printf("No signature found in image.\n");
> > > >>> +		result = 1;
> > > >>> +		goto auth_exit;
> > > >>> +	}
> > > >>> +
> > > >>> +	*size = sig_addr - cert_addr;	/* Subtract out the signature size */
> > > >>> +	cert_size = *size;
> > > >>> +
> > > >>> +	/* Check if image load address is 32-bit aligned */
> > > >>> +	if (0 != (0x3 & cert_addr)) {
> > > >>
> > > >> 	if (!IS_ALIGNED(cert_addr, 4)) { ?
> > > >>
> > > >>> +		printf("Image is not 4-byte aligned.\n");
> > > >>> +		result = 1;
> > > >>> +		goto auth_exit;
> > > >>> +	}
> > > >>> +
> > > >>> +	/* Image size also should be multiple of 4 */
> > > >>> +	if (0 != (0x3 & cert_size)) {
> > > >>
> > > >> 	if (!IS_ALIGNED(cert_size, 4)) { ?
> > > >>
> > > >>> +		printf("Image size is not 4-byte aligned.\n");
> > > >>> +		result = 1;
> > > >>> +		goto auth_exit;
> > > >>> +	}
> > > >>> +
> > > >>> +	/* Call ROM HAL API to verify certificate signature */
> > > >>> +	debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__,
> > > >>> +	      cert_addr, cert_size, sig_addr);
> > > >>> +
> > > >>> +	result = secure_rom_call(
> > > >>> +		API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0,
> > > >>> +		4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF);
> > > >>> +auth_exit:
> > > >>> +	if (result != 0) {
> > > >>> +		printf("Authentication failed!\n");
> > > >>> +		printf("Return Value = %08X\n", result);
> > > >>> +		hang();
> > > >>> +	}
> > > >>> +
> > > >>> +	printf("Authentication passed: %s\n", (char *)sig_addr);
> > > >>
> > > >> Uart boot will break because of these prints during the FIT loading. Can
> > > >> you make this as debug?
> > > > 
> > > > Are you sure it will break?  There's usually a print in between loading
> > > > SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough
> > > > to re-transmit.
> > > > 
> > > 
> > > Yes, if the print is in between while Y-MODEM is transferring. The above
> > > print falls in this case.
> 
> ... but Y-MODEM (the protocol) does retransmit.  It should recover from
> this message.
> 
> > Tom et al.,
> > so if this really breaks stuff I need to do something about it. As said
> > I'd really like to keep the "Authentication passed: <certificate name>"
> > message in the boot log. So if I implement something along the lines
> > what Lokesh suggested:
> > 
> > "...you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the                           
> > config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do..."
> > 
> > to selectivly suppress the message in case of UART boot, would this be
> > acceptable? Or is there a better way?
> 
> At worst case, yes, we can case this around !CONFIG_SPL_YMODEM_SUPPORT.
> But I keep thinking the world should recover from this too.

...hmmm, but it's so ugly :)

Well I'm going to spend some time to play with it. Thanks for all your
feedback.

Andreas

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Masahiro Yamada]

2016-06-21 12:34 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:
> From: Daniel Allred <d-allred@ti.com>
>
> The next stage boot loader image and the selected FDT can be
> post-processed by board/platform/device-specific code, which can include
> modifying the size and altering the starting source address before
> copying these binary blobs to their final desitination. This might be
> desired to do things like strip headers or footers attached to the
> images before they were packeaged into the FIT, or to perform operations
> such as decryption or authentication.
>
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---

Typos?

desitination -> destination
packeaged    -> packaged


-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Andreas Dannenberg]

On Thu, Jun 23, 2016 at 11:38:30AM +0900, Masahiro Yamada wrote:
> 2016-06-21 12:34 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:
> > From: Daniel Allred <d-allred@ti.com>
> >
> > The next stage boot loader image and the selected FDT can be
> > post-processed by board/platform/device-specific code, which can include
> > modifying the size and altering the starting source address before
> > copying these binary blobs to their final desitination. This might be
> > desired to do things like strip headers or footers attached to the
> > images before they were packeaged into the FIT, or to perform operations
> > such as decryption or authentication.
> >
> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> 
> Typos?
> 
> desitination -> destination
> packeaged    -> packaged
> 

Good catch! Missed running the spell checker here... Will fix.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Simon Glass]

Hi Andreas,

On 20 June 2016 at 21:34, Andreas Dannenberg <dannenberg@ti.com> wrote:
> From: Daniel Allred <d-allred@ti.com>
>
> The next stage boot loader image and the selected FDT can be
> post-processed by board/platform/device-specific code, which can include
> modifying the size and altering the starting source address before
> copying these binary blobs to their final desitination. This might be
> desired to do things like strip headers or footers attached to the
> images before they were packeaged into the FIT, or to perform operations
> such as decryption or authentication.
>
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
>  common/spl/spl_fit.c | 21 ++++++++++++++++-----
>  include/image.h      | 15 +++++++++++++++
>  2 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9874708..ecbcb97 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>         return (data_size + info->bl_len - 1) / info->bl_len;
>  }
>
> +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
> +{
> +}

Can you make this a Kconfig option instead of a weak function? It's
hard enough to trace what a particular board is doing...

> +
>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  {
>         int sectors;
> @@ -132,7 +136,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>         int data_offset, data_size;
>         int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
>         int src_sector;
> -       void *dst;
> +       void *dst, *src;
>
>         /*
>          * Figure out where the external images start. This is the base for the
> @@ -206,8 +210,11 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>                 return -EIO;
>         debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
>               data_size);
> -       memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
> -              data_size);
> +       src = dst + get_aligned_image_overhead(info, data_offset);
> +
> +       board_fit_image_post_process((void **)&src, (size_t *)&data_size);
> +
> +       memcpy(dst, src, data_size);
>
>         /* Figure out which device tree the board wants to use */
>         fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> @@ -236,8 +243,12 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>          */
>         debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
>               fdt_len);
> -       memcpy(load_ptr + data_size,
> -              dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
> +       src = dst + get_aligned_image_overhead(info, fdt_offset);
> +       dst = load_ptr + data_size;
> +
> +       board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
> +
> +       memcpy(dst, src, fdt_len);
>
>         return 0;
>  }
> diff --git a/include/image.h b/include/image.h
> index a8f6bd1..9536874 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1172,4 +1172,19 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr);
>   */
>  int board_fit_config_name_match(const char *name);
>
> +/**
> + * board_fit_image_post_process() - Do any post-process on FIT binary data
> + *
> + * This is used to do any sort of image manipulation, verification, decryption
> + * etc. in a platform or board specific way. Obviously, anything done here would
> + * need to be comprehended in how the images were prepared before being injected
> + * into the FIT creation (i.e. the binary blobs would have been pre-processed
> + * before being added to the FIT image).
> + *
> + * @image: pointer to the image start pointer
> + * @size: pointer to the image size
> + * @return no return value (failure should be handled internally)
> + */
> +void board_fit_image_post_process(void **p_image, size_t *p_size);
> +
>  #endif /* __IMAGE_H__ */
> --
> 2.6.4
>

Regards,
Simon

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Andreas Dannenberg]

On Thu, Jun 23, 2016 at 07:57:13AM -0600, Simon Glass wrote:
> Hi Andreas,
> 
> On 20 June 2016 at 21:34, Andreas Dannenberg <dannenberg@ti.com> wrote:
> > From: Daniel Allred <d-allred@ti.com>
> >
> > The next stage boot loader image and the selected FDT can be
> > post-processed by board/platform/device-specific code, which can include
> > modifying the size and altering the starting source address before
> > copying these binary blobs to their final desitination. This might be
> > desired to do things like strip headers or footers attached to the
> > images before they were packeaged into the FIT, or to perform operations
> > such as decryption or authentication.
> >
> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> >  common/spl/spl_fit.c | 21 ++++++++++++++++-----
> >  include/image.h      | 15 +++++++++++++++
> >  2 files changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> > index 9874708..ecbcb97 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
> >         return (data_size + info->bl_len - 1) / info->bl_len;
> >  }
> >
> > +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
> > +{
> > +}
> 
> Can you make this a Kconfig option instead of a weak function? It's
> hard enough to trace what a particular board is doing...

Hi Simon,
I understand your point and we did look into doing it but could not come
up with a good way (certainly I don't want to clutter this global file
with any TI stuff) but maybe we were over-thinking it :)

So (roughly) how would you like to see this Kconfig option to be
implemented? I'll be happy to change the implementation if given a
little direction.

Thanks,
Andreas


> 
> > +
> >  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
> >  {
> >         int sectors;
> > @@ -132,7 +136,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
> >         int data_offset, data_size;
> >         int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
> >         int src_sector;
> > -       void *dst;
> > +       void *dst, *src;
> >
> >         /*
> >          * Figure out where the external images start. This is the base for the
> > @@ -206,8 +210,11 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
> >                 return -EIO;
> >         debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
> >               data_size);
> > -       memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
> > -              data_size);
> > +       src = dst + get_aligned_image_overhead(info, data_offset);
> > +
> > +       board_fit_image_post_process((void **)&src, (size_t *)&data_size);
> > +
> > +       memcpy(dst, src, data_size);
> >
> >         /* Figure out which device tree the board wants to use */
> >         fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> > @@ -236,8 +243,12 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
> >          */
> >         debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
> >               fdt_len);
> > -       memcpy(load_ptr + data_size,
> > -              dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
> > +       src = dst + get_aligned_image_overhead(info, fdt_offset);
> > +       dst = load_ptr + data_size;
> > +
> > +       board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
> > +
> > +       memcpy(dst, src, fdt_len);
> >
> >         return 0;
> >  }
> > diff --git a/include/image.h b/include/image.h
> > index a8f6bd1..9536874 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1172,4 +1172,19 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr);
> >   */
> >  int board_fit_config_name_match(const char *name);
> >
> > +/**
> > + * board_fit_image_post_process() - Do any post-process on FIT binary data
> > + *
> > + * This is used to do any sort of image manipulation, verification, decryption
> > + * etc. in a platform or board specific way. Obviously, anything done here would
> > + * need to be comprehended in how the images were prepared before being injected
> > + * into the FIT creation (i.e. the binary blobs would have been pre-processed
> > + * before being added to the FIT image).
> > + *
> > + * @image: pointer to the image start pointer
> > + * @size: pointer to the image size
> > + * @return no return value (failure should be handled internally)
> > + */
> > +void board_fit_image_post_process(void **p_image, size_t *p_size);
> > +
> >  #endif /* __IMAGE_H__ */
> > --
> > 2.6.4
> >
> 
> Regards,
> Simon

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Simon Glass]

Hi Andreas,

On 23 June 2016 at 08:19, Andreas Dannenberg <dannenberg@ti.com> wrote:
> On Thu, Jun 23, 2016 at 07:57:13AM -0600, Simon Glass wrote:
>> Hi Andreas,
>>
>> On 20 June 2016 at 21:34, Andreas Dannenberg <dannenberg@ti.com> wrote:
>> > From: Daniel Allred <d-allred@ti.com>
>> >
>> > The next stage boot loader image and the selected FDT can be
>> > post-processed by board/platform/device-specific code, which can include
>> > modifying the size and altering the starting source address before
>> > copying these binary blobs to their final desitination. This might be
>> > desired to do things like strip headers or footers attached to the
>> > images before they were packeaged into the FIT, or to perform operations
>> > such as decryption or authentication.
>> >
>> > Signed-off-by: Daniel Allred <d-allred@ti.com>
>> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
>> > ---
>> >  common/spl/spl_fit.c | 21 ++++++++++++++++-----
>> >  include/image.h      | 15 +++++++++++++++
>> >  2 files changed, 31 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> > index 9874708..ecbcb97 100644
>> > --- a/common/spl/spl_fit.c
>> > +++ b/common/spl/spl_fit.c
>> > @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>> >         return (data_size + info->bl_len - 1) / info->bl_len;
>> >  }
>> >
>> > +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
>> > +{
>> > +}
>>
>> Can you make this a Kconfig option instead of a weak function? It's
>> hard enough to trace what a particular board is doing...
>
> Hi Simon,
> I understand your point and we did look into doing it but could not come
> up with a good way (certainly I don't want to clutter this global file
> with any TI stuff) but maybe we were over-thinking it :)
>
> So (roughly) how would you like to see this Kconfig option to be
> implemented? I'll be happy to change the implementation if given a
> little direction.

Add  Kconfig like FIT_IMAGE_POST_PROCESS

Then in the code:

if (IS_ENABLED(FIT_IMAGE_POST_PROCESS))
   board_fit_image_post_process(...)

(or use #ifdef)

and enable it for your board.

You are already cluttering the code with TI stuff, but at least this
way it is clear whether a particular board is using this feature.
Perhaps others will find it useful too.

With enough weak functions we would reach a point where no one can
figure out what is happened without linking the code and looking at
the link map!

Regards,
Simon

>
> Thanks,
> Andreas
[snip]

[U-Boot] [PATCH 2/9] spl: fit: add support for post-processing of images

[Andreas Dannenberg]

On Thu, Jun 23, 2016 at 08:45:54AM -0600, Simon Glass wrote:
> Hi Andreas,
> 
> On 23 June 2016 at 08:19, Andreas Dannenberg <dannenberg@ti.com> wrote:
> > On Thu, Jun 23, 2016 at 07:57:13AM -0600, Simon Glass wrote:
> >> Hi Andreas,
> >>
> >> On 20 June 2016 at 21:34, Andreas Dannenberg <dannenberg@ti.com> wrote:
> >> > From: Daniel Allred <d-allred@ti.com>
> >> >
> >> > The next stage boot loader image and the selected FDT can be
> >> > post-processed by board/platform/device-specific code, which can include
> >> > modifying the size and altering the starting source address before
> >> > copying these binary blobs to their final desitination. This might be
> >> > desired to do things like strip headers or footers attached to the
> >> > images before they were packeaged into the FIT, or to perform operations
> >> > such as decryption or authentication.
> >> >
> >> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> >> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> >> > ---
> >> >  common/spl/spl_fit.c | 21 ++++++++++++++++-----
> >> >  include/image.h      | 15 +++++++++++++++
> >> >  2 files changed, 31 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> >> > index 9874708..ecbcb97 100644
> >> > --- a/common/spl/spl_fit.c
> >> > +++ b/common/spl/spl_fit.c
> >> > @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
> >> >         return (data_size + info->bl_len - 1) / info->bl_len;
> >> >  }
> >> >
> >> > +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
> >> > +{
> >> > +}
> >>
> >> Can you make this a Kconfig option instead of a weak function? It's
> >> hard enough to trace what a particular board is doing...
> >
> > Hi Simon,
> > I understand your point and we did look into doing it but could not come
> > up with a good way (certainly I don't want to clutter this global file
> > with any TI stuff) but maybe we were over-thinking it :)
> >
> > So (roughly) how would you like to see this Kconfig option to be
> > implemented? I'll be happy to change the implementation if given a
> > little direction.
> 
> Add  Kconfig like FIT_IMAGE_POST_PROCESS
> 
> Then in the code:
> 
> if (IS_ENABLED(FIT_IMAGE_POST_PROCESS))
>    board_fit_image_post_process(...)
> 
> (or use #ifdef)
> 
> and enable it for your board.
> 
> You are already cluttering the code with TI stuff, but at least this
> way it is clear whether a particular board is using this feature.
> Perhaps others will find it useful too.

Simon,
Yes that was the idea, to allow other use cases here as well. And thanks
for the suggestion, that's actually simpler than what we had thought of
(while also helping the re-use aspect). I'm also going to make this a
SPL_FIT_IMAGE_POST_PROCESS to narrow the scope a little and to follow a
similar scheme like the other SPL FIT related options.

Regards,
Andreas

> With enough weak functions we would reach a point where no one can
> figure out what is happened without linking the code and looking at
> the link map!
> 
> Regards,
> Simon