All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
@ 2018-01-12 12:39 Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int Bryan O'Donoghue
                   ` (25 more replies)
  0 siblings, 26 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

v6:
- Added patch 21/25 return zero on open (unlocked) board when
  calling authenticate_image() - Breno
  
- Added Tested-by: Breno Matheus Lima <brenomatheus@gmail.com>
  as indicated for remainder 24/25 patches

- Added Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
  as indicated for remainder 24/25 patches

v5:
- Drop dcache disable across HAB call.
  We can't replicate this error on the current codebase and the available
  images. We'll have to wait for the error to crop up again before pushing
  that patch any further.

v4:
- No change mixed extra patches @ v3 unnoticed with previous
  git-send

v3:
- Only call into ROM if headers are verified. - Bryan

- Print HAB event log if and only if a call was made to HAB
  and a meaningful status code has been obtained. - Breno

v2:
- Fix compilation warnings and errors in SPL highlighted by 
  Breno Matheus Lima

- Add CC: Breno Matheus Lima <brenomatheus@gmail.com> to all patches

v1:
This patchset updates the i.MX HAB layer in u-boot to fix a list of
identified issues and then to add and extend existing functionality.

The first block of patches 0001-0006 deal with fixing existing code,

- Fixes indentation
- Fixes the treatment of input parameters to hab_auth_image.

The second block of patches 0007-0013 are about tidying up the HAB code

- Remove reliance on hard-coding to specific offsets
- IVT header drives locating CSF
- Continue to support existing boards

Patches 0014 onwards extend out the HAB functionality.

- hab_rvt_check_target is a recommended check in the NXP documents to
  perform prior to hab_rvt_authenticate_image
- hab_rvt_failsafe is a useful function to set the board into BootROM
  USB recovery mode.



Bryan O'Donoghue (25):
  arm: imx: hab: Make authenticate_image return int
  arm: imx: hab: Fix authenticate_image result code
  arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail
  arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail
  arm: imx: hab: Move IVT_SIZE to hab.h
  arm: imx: hab: Move CSF_PAD_SIZE to hab.h
  arm: imx: hab: Fix authenticate_image input parameters
  arm: imx: hab: Add IVT header definitions
  arm: imx: hab: Add IVT header verification
  arm: imx: hab: Verify IVT self matches calculated address
  arm: imx: hab: Only call ROM once headers are verified
  arm: imx: hab: Print CSF based on IVT descriptor
  arm: imx: hab: Print additional IVT elements during debug
  arm: imx: hab: Define rvt_check_target()
  arm: imx: hab: Implement hab_rvt_check_target
  arm: imx: hab: Add a hab_rvt_check_target to image auth
  arm: imx: hab: Print HAB event log only after calling ROM
  arm: imx: hab: Make internal functions and data static
  arm: imx: hab: Prefix authenticate_image with imx_hab
  arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled
  arm: imx: hab: Make authenticate_image() return zero on open boards
  arm: imx: hab: Make imx_hab_is_enabled global
  arm: imx: hab: Define rvt_failsafe()
  arm: imx: hab: Implement hab_rvt_failsafe
  arm: imx: hab: Add hab_failsafe console command

 arch/arm/include/asm/mach-imx/hab.h |  46 +++-
 arch/arm/mach-imx/hab.c             | 461 +++++++++++++++++++++---------------
 arch/arm/mach-imx/spl.c             |  38 ++-
 3 files changed, 354 insertions(+), 191 deletions(-)

-- 
2.7.4

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

* [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
@ 2018-01-12 12:39 ` Bryan O'Donoghue
  2018-01-14 16:44   ` Stefano Babic
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 02/25] arm: imx: hab: Fix authenticate_image result code Bryan O'Donoghue
                   ` (24 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

Both usages of authenticate_image treat the result code as a simple binary.
The command line usage of authenticate_image directly returns the result
code of authenticate_image as a success/failure code.

Right now when calling hab_auth_img and test the result code in a shell a
passing hab_auth_img will appear to the shell as a fail.

The first step in fixing this behaviour is to fix-up the result code return
by authenticate_image() itself, subsequent patches fix the interpretation
of authenticate_image so that zero will return CMD_RET_SUCCESS and non-zero
will return CMD_RET_FAILURE.

The first step is fixing the return type in authenticate_image() so do that
now.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 2 +-
 arch/arm/mach-imx/hab.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index e0ff459..1b7a5e4 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -145,6 +145,6 @@ typedef void hapi_clock_init_t(void);
 
 /* ----------- end of HAB API updates ------------*/
 
-uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size);
+int authenticate_image(uint32_t ddr_start, uint32_t image_size);
 
 #endif
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 02c7ae4..09892a6 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -410,7 +410,7 @@ static bool is_hab_enabled(void)
 	return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
 }
 
-uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size)
+int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 {
 	uint32_t load_addr = 0;
 	size_t bytes;
-- 
2.7.4

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

* [U-Boot] [PATCH v6 02/25] arm: imx: hab: Fix authenticate_image result code
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int Bryan O'Donoghue
@ 2018-01-12 12:39 ` Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 03/25] arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail Bryan O'Donoghue
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

authenticate_image returns 1 for success and 0 for failure. That result
code is mapped directly to the result code for the command line function
hab_auth_img - which means when hab_auth_img succeeds it is returning
CMD_RET_FAILURE (1) instead of CMD_RET_SUCCESS (0).

This patch fixes this behaviour by making authenticate_image() return 0 for
success and 1 for failure. Both users of authenticate_image() as a result
have some minimal churn. The upshot is once done when hab_auth_img is
called from the command line we set $? in the standard way for scripting
functions to act on.

Fixes: 36c1ca4d46ef ("imx: Support i.MX6 High Assurance Boot
authentication")

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 9 ++++++---
 arch/arm/mach-imx/spl.c | 4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 09892a6..9fe6d43 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -373,7 +373,10 @@ static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
 	ivt_offset = simple_strtoul(argv[2], NULL, 16);
 
 	rcode = authenticate_image(addr, ivt_offset);
-
+	if (rcode == 0)
+		rcode = CMD_RET_SUCCESS;
+	else
+		rcode = CMD_RET_FAILURE;
 	return rcode;
 }
 
@@ -415,7 +418,7 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 	uint32_t load_addr = 0;
 	size_t bytes;
 	ptrdiff_t ivt_offset = 0;
-	int result = 0;
+	int result = 1;
 	ulong start;
 	hab_rvt_authenticate_image_t *hab_rvt_authenticate_image;
 	hab_rvt_entry_t *hab_rvt_entry;
@@ -510,7 +513,7 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 	}
 
 	if ((!is_hab_enabled()) || (load_addr != 0))
-		result = 1;
+		result = 0;
 
 	return result;
 }
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index d0d1b73..6e930b3 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -163,8 +163,8 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 
 	/* HAB looks for the CSF at the end of the authenticated data therefore,
 	 * we need to subtract the size of the CSF from the actual filesize */
-	if (authenticate_image(spl_image->load_addr,
-			       spl_image->size - CONFIG_CSF_SIZE)) {
+	if (!authenticate_image(spl_image->load_addr,
+				spl_image->size - CONFIG_CSF_SIZE)) {
 		image_entry();
 	} else {
 		puts("spl: ERROR:  image authentication unsuccessful\n");
-- 
2.7.4

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

* [U-Boot] [PATCH v6 03/25] arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 02/25] arm: imx: hab: Fix authenticate_image result code Bryan O'Donoghue
@ 2018-01-12 12:39 ` Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 04/25] arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail Bryan O'Donoghue
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

There is no need to call is_enabled() twice in authenticate_image - it does
nothing but add an additional layer of indentation.

We can check for is_enabled() at the start of the function and return the
result code directly.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 138 ++++++++++++++++++++++++------------------------
 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 9fe6d43..6f86c02 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -428,91 +428,91 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 	hab_rvt_entry = hab_rvt_entry_p;
 	hab_rvt_exit = hab_rvt_exit_p;
 
-	if (is_hab_enabled()) {
-		printf("\nAuthenticate image from DDR location 0x%x...\n",
-		       ddr_start);
+	if (!is_hab_enabled()) {
+		puts("hab fuse not enabled\n");
+		return result;
+	}
 
-		hab_caam_clock_enable(1);
+	printf("\nAuthenticate image from DDR location 0x%x...\n",
+	       ddr_start);
 
-		if (hab_rvt_entry() == HAB_SUCCESS) {
-			/* If not already aligned, Align to ALIGN_SIZE */
-			ivt_offset = (image_size + ALIGN_SIZE - 1) &
-					~(ALIGN_SIZE - 1);
+	hab_caam_clock_enable(1);
 
-			start = ddr_start;
-			bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
+	if (hab_rvt_entry() == HAB_SUCCESS) {
+		/* If not already aligned, Align to ALIGN_SIZE */
+		ivt_offset = (image_size + ALIGN_SIZE - 1) &
+				~(ALIGN_SIZE - 1);
+
+		start = ddr_start;
+		bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
 #ifdef DEBUG
-			printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n",
-			       ivt_offset, ddr_start + ivt_offset);
-			puts("Dumping IVT\n");
-			print_buffer(ddr_start + ivt_offset,
-				     (void *)(ddr_start + ivt_offset),
-				     4, 0x8, 0);
-
-			puts("Dumping CSF Header\n");
-			print_buffer(ddr_start + ivt_offset+IVT_SIZE,
-				     (void *)(ddr_start + ivt_offset+IVT_SIZE),
-				     4, 0x10, 0);
+		printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n",
+		       ivt_offset, ddr_start + ivt_offset);
+		puts("Dumping IVT\n");
+		print_buffer(ddr_start + ivt_offset,
+			     (void *)(ddr_start + ivt_offset),
+			     4, 0x8, 0);
+
+		puts("Dumping CSF Header\n");
+		print_buffer(ddr_start + ivt_offset + IVT_SIZE,
+			     (void *)(ddr_start + ivt_offset + IVT_SIZE),
+			     4, 0x10, 0);
 
 #if  !defined(CONFIG_SPL_BUILD)
-			get_hab_status();
+		get_hab_status();
 #endif
 
-			puts("\nCalling authenticate_image in ROM\n");
-			printf("\tivt_offset = 0x%x\n", ivt_offset);
-			printf("\tstart = 0x%08lx\n", start);
-			printf("\tbytes = 0x%x\n", bytes);
+		puts("\nCalling authenticate_image in ROM\n");
+		printf("\tivt_offset = 0x%x\n", ivt_offset);
+		printf("\tstart = 0x%08lx\n", start);
+		printf("\tbytes = 0x%x\n", bytes);
 #endif
-			/*
-			 * If the MMU is enabled, we have to notify the ROM
-			 * code, or it won't flush the caches when needed.
-			 * This is done, by setting the "pu_irom_mmu_enabled"
-			 * word to 1. You can find its address by looking in
-			 * the ROM map. This is critical for
-			 * authenticate_image(). If MMU is enabled, without
-			 * setting this bit, authentication will fail and may
-			 * crash.
-			 */
-			/* Check MMU enabled */
-			if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) {
-				if (is_mx6dq()) {
-					/*
-					 * This won't work on Rev 1.0.0 of
-					 * i.MX6Q/D, since their ROM doesn't
-					 * do cache flushes. don't think any
-					 * exist, so we ignore them.
-					 */
-					if (!is_mx6dqp())
-						writel(1, MX6DQ_PU_IROM_MMU_EN_VAR);
-				} else if (is_mx6sdl()) {
-					writel(1, MX6DLS_PU_IROM_MMU_EN_VAR);
-				} else if (is_mx6sl()) {
-					writel(1, MX6SL_PU_IROM_MMU_EN_VAR);
-				}
+		/*
+		 * If the MMU is enabled, we have to notify the ROM
+		 * code, or it won't flush the caches when needed.
+		 * This is done, by setting the "pu_irom_mmu_enabled"
+		 * word to 1. You can find its address by looking in
+		 * the ROM map. This is critical for
+		 * authenticate_image(). If MMU is enabled, without
+		 * setting this bit, authentication will fail and may
+		 * crash.
+		 */
+		/* Check MMU enabled */
+		if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) {
+			if (is_mx6dq()) {
+				/*
+				 * This won't work on Rev 1.0.0 of
+				 * i.MX6Q/D, since their ROM doesn't
+				 * do cache flushes. don't think any
+				 * exist, so we ignore them.
+				 */
+				if (!is_mx6dqp())
+					writel(1, MX6DQ_PU_IROM_MMU_EN_VAR);
+			} else if (is_mx6sdl()) {
+				writel(1, MX6DLS_PU_IROM_MMU_EN_VAR);
+			} else if (is_mx6sl()) {
+				writel(1, MX6SL_PU_IROM_MMU_EN_VAR);
 			}
+		}
 
-			load_addr = (uint32_t)hab_rvt_authenticate_image(
-					HAB_CID_UBOOT,
-					ivt_offset, (void **)&start,
-					(size_t *)&bytes, NULL);
-			if (hab_rvt_exit() != HAB_SUCCESS) {
-				puts("hab exit function fail\n");
-				load_addr = 0;
-			}
-		} else {
-			puts("hab entry function fail\n");
+		load_addr = (uint32_t)hab_rvt_authenticate_image(
+				HAB_CID_UBOOT,
+				ivt_offset, (void **)&start,
+				(size_t *)&bytes, NULL);
+		if (hab_rvt_exit() != HAB_SUCCESS) {
+			puts("hab exit function fail\n");
+			load_addr = 0;
 		}
+	} else {
+		puts("hab entry function fail\n");
+	}
 
-		hab_caam_clock_enable(0);
+	hab_caam_clock_enable(0);
 
 #if !defined(CONFIG_SPL_BUILD)
-		get_hab_status();
+	get_hab_status();
 #endif
-	} else {
-		puts("hab fuse not enabled\n");
-	}
-
-	if ((!is_hab_enabled()) || (load_addr != 0))
+	if (load_addr != 0)
 		result = 0;
 
 	return result;
-- 
2.7.4

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

* [U-Boot] [PATCH v6 04/25] arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (2 preceding siblings ...)
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 03/25] arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail Bryan O'Donoghue
@ 2018-01-12 12:39 ` Bryan O'Donoghue
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 05/25] arm: imx: hab: Move IVT_SIZE to hab.h Bryan O'Donoghue
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

The current code disjoins an entire block of code on hab_entry pass/fail
resulting in a large chunk of authenticate_image being offset to the right.

Fix this by checking hab_entry() pass/failure and exiting the function
directly if in an error state.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 118 ++++++++++++++++++++++++------------------------
 1 file changed, 60 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 6f86c02..f878b7b 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -438,75 +438,77 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 
 	hab_caam_clock_enable(1);
 
-	if (hab_rvt_entry() == HAB_SUCCESS) {
-		/* If not already aligned, Align to ALIGN_SIZE */
-		ivt_offset = (image_size + ALIGN_SIZE - 1) &
-				~(ALIGN_SIZE - 1);
+	if (hab_rvt_entry() != HAB_SUCCESS) {
+		puts("hab entry function fail\n");
+		goto hab_caam_clock_disable;
+	}
 
-		start = ddr_start;
-		bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
+	/* If not already aligned, Align to ALIGN_SIZE */
+	ivt_offset = (image_size + ALIGN_SIZE - 1) &
+			~(ALIGN_SIZE - 1);
+
+	start = ddr_start;
+	bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
 #ifdef DEBUG
-		printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n",
-		       ivt_offset, ddr_start + ivt_offset);
-		puts("Dumping IVT\n");
-		print_buffer(ddr_start + ivt_offset,
-			     (void *)(ddr_start + ivt_offset),
-			     4, 0x8, 0);
-
-		puts("Dumping CSF Header\n");
-		print_buffer(ddr_start + ivt_offset + IVT_SIZE,
-			     (void *)(ddr_start + ivt_offset + IVT_SIZE),
-			     4, 0x10, 0);
+	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n",
+	       ivt_offset, ddr_start + ivt_offset);
+	puts("Dumping IVT\n");
+	print_buffer(ddr_start + ivt_offset,
+		     (void *)(ddr_start + ivt_offset),
+		     4, 0x8, 0);
+
+	puts("Dumping CSF Header\n");
+	print_buffer(ddr_start + ivt_offset + IVT_SIZE,
+		     (void *)(ddr_start + ivt_offset + IVT_SIZE),
+		     4, 0x10, 0);
 
 #if  !defined(CONFIG_SPL_BUILD)
-		get_hab_status();
+	get_hab_status();
 #endif
 
-		puts("\nCalling authenticate_image in ROM\n");
-		printf("\tivt_offset = 0x%x\n", ivt_offset);
-		printf("\tstart = 0x%08lx\n", start);
-		printf("\tbytes = 0x%x\n", bytes);
+	puts("\nCalling authenticate_image in ROM\n");
+	printf("\tivt_offset = 0x%x\n", ivt_offset);
+	printf("\tstart = 0x%08lx\n", start);
+	printf("\tbytes = 0x%x\n", bytes);
 #endif
-		/*
-		 * If the MMU is enabled, we have to notify the ROM
-		 * code, or it won't flush the caches when needed.
-		 * This is done, by setting the "pu_irom_mmu_enabled"
-		 * word to 1. You can find its address by looking in
-		 * the ROM map. This is critical for
-		 * authenticate_image(). If MMU is enabled, without
-		 * setting this bit, authentication will fail and may
-		 * crash.
-		 */
-		/* Check MMU enabled */
-		if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) {
-			if (is_mx6dq()) {
-				/*
-				 * This won't work on Rev 1.0.0 of
-				 * i.MX6Q/D, since their ROM doesn't
-				 * do cache flushes. don't think any
-				 * exist, so we ignore them.
-				 */
-				if (!is_mx6dqp())
-					writel(1, MX6DQ_PU_IROM_MMU_EN_VAR);
-			} else if (is_mx6sdl()) {
-				writel(1, MX6DLS_PU_IROM_MMU_EN_VAR);
-			} else if (is_mx6sl()) {
-				writel(1, MX6SL_PU_IROM_MMU_EN_VAR);
-			}
+	/*
+	 * If the MMU is enabled, we have to notify the ROM
+	 * code, or it won't flush the caches when needed.
+	 * This is done, by setting the "pu_irom_mmu_enabled"
+	 * word to 1. You can find its address by looking in
+	 * the ROM map. This is critical for
+	 * authenticate_image(). If MMU is enabled, without
+	 * setting this bit, authentication will fail and may
+	 * crash.
+	 */
+	/* Check MMU enabled */
+	if (is_soc_type(MXC_SOC_MX6) && get_cr() & CR_M) {
+		if (is_mx6dq()) {
+			/*
+			 * This won't work on Rev 1.0.0 of
+			 * i.MX6Q/D, since their ROM doesn't
+			 * do cache flushes. don't think any
+			 * exist, so we ignore them.
+			 */
+			if (!is_mx6dqp())
+				writel(1, MX6DQ_PU_IROM_MMU_EN_VAR);
+		} else if (is_mx6sdl()) {
+			writel(1, MX6DLS_PU_IROM_MMU_EN_VAR);
+		} else if (is_mx6sl()) {
+			writel(1, MX6SL_PU_IROM_MMU_EN_VAR);
 		}
+	}
 
-		load_addr = (uint32_t)hab_rvt_authenticate_image(
-				HAB_CID_UBOOT,
-				ivt_offset, (void **)&start,
-				(size_t *)&bytes, NULL);
-		if (hab_rvt_exit() != HAB_SUCCESS) {
-			puts("hab exit function fail\n");
-			load_addr = 0;
-		}
-	} else {
-		puts("hab entry function fail\n");
+	load_addr = (uint32_t)hab_rvt_authenticate_image(
+			HAB_CID_UBOOT,
+			ivt_offset, (void **)&start,
+			(size_t *)&bytes, NULL);
+	if (hab_rvt_exit() != HAB_SUCCESS) {
+		puts("hab exit function fail\n");
+		load_addr = 0;
 	}
 
+hab_caam_clock_disable:
 	hab_caam_clock_enable(0);
 
 #if !defined(CONFIG_SPL_BUILD)
-- 
2.7.4

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

* [U-Boot] [PATCH v6 05/25] arm: imx: hab: Move IVT_SIZE to hab.h
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (3 preceding siblings ...)
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 04/25] arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail Bryan O'Donoghue
@ 2018-01-12 12:39 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 06/25] arm: imx: hab: Move CSF_PAD_SIZE " Bryan O'Donoghue
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:39 UTC (permalink / raw)
  To: u-boot

The size of the IVT header should be defined in hab.h move it there now.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 2 ++
 arch/arm/mach-imx/hab.c             | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 1b7a5e4..3c19d2e 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -143,6 +143,8 @@ typedef void hapi_clock_init_t(void);
 #define HAB_CID_ROM 0 /**< ROM Caller ID */
 #define HAB_CID_UBOOT 1 /**< UBOOT Caller ID*/
 
+#define IVT_SIZE			0x20
+
 /* ----------- end of HAB API updates ------------*/
 
 int authenticate_image(uint32_t ddr_start, uint32_t image_size);
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index f878b7b..6367562 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -70,7 +70,6 @@
 	((hab_rvt_exit_t *)HAB_RVT_EXIT)			\
 )
 
-#define IVT_SIZE		0x20
 #define ALIGN_SIZE		0x1000
 #define CSF_PAD_SIZE		0x2000
 #define MX6DQ_PU_IROM_MMU_EN_VAR	0x009024a8
-- 
2.7.4

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

* [U-Boot] [PATCH v6 06/25] arm: imx: hab: Move CSF_PAD_SIZE to hab.h
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (4 preceding siblings ...)
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 05/25] arm: imx: hab: Move IVT_SIZE to hab.h Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 07/25] arm: imx: hab: Fix authenticate_image input parameters Bryan O'Donoghue
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

CSF_PAD_SIZE should be defined in hab.h, move it to that location now.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 1 +
 arch/arm/mach-imx/hab.c             | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 3c19d2e..91dda42 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -144,6 +144,7 @@ typedef void hapi_clock_init_t(void);
 #define HAB_CID_UBOOT 1 /**< UBOOT Caller ID*/
 
 #define IVT_SIZE			0x20
+#define CSF_PAD_SIZE			0x2000
 
 /* ----------- end of HAB API updates ------------*/
 
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 6367562..039a017 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -71,7 +71,6 @@
 )
 
 #define ALIGN_SIZE		0x1000
-#define CSF_PAD_SIZE		0x2000
 #define MX6DQ_PU_IROM_MMU_EN_VAR	0x009024a8
 #define MX6DLS_PU_IROM_MMU_EN_VAR	0x00901dd0
 #define MX6SL_PU_IROM_MMU_EN_VAR	0x00900a18
-- 
2.7.4

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

* [U-Boot] [PATCH v6 07/25] arm: imx: hab: Fix authenticate_image input parameters
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (5 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 06/25] arm: imx: hab: Move CSF_PAD_SIZE " Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 08/25] arm: imx: hab: Add IVT header definitions Bryan O'Donoghue
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

u-boot command "hab_auth_img" tells a user that it takes

- addr - image hex address
- offset - hex offset of IVT in the image

but in fact the callback hab_auth_img makes to authenticate_image treats
the second 'offset' parameter as an image length.

Furthermore existing code requires the IVT header to be appended to the end
of the image which is not actually a requirement of HABv4.

This patch fixes this situation by

1: Adding a new parameter to hab_auth_img
   - addr   : image hex address
   - length : total length of the image
   - offset : offset of IVT from addr

2: Updates the existing call into authenticate_image() in
   arch/arm/mach-imx/spl.c:jump_to_image_no_args() to pass
   addr, length and IVT offset respectively.

This allows then hab_auth_img to actually operate the way it was specified
in the help text and should still allow existing code to work.

It has the added advantage that the IVT header doesn't have to be appended
to an image given to HAB - it can be prepended for example.

Note prepending the IVT is what u-boot will do when making an IVT for the
BootROM. It should be possible for u-boot properly authenticate images
made by mkimage via HAB.

This patch is the first step in making that happen subsequent patches will
focus on removing hard-coded offsets to the IVT, which again is not
mandated to live at the end of a .imx image.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h |  3 +-
 arch/arm/mach-imx/hab.c             | 73 +++++++++++--------------------------
 arch/arm/mach-imx/spl.c             | 35 +++++++++++++++++-
 3 files changed, 57 insertions(+), 54 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 91dda42..b2a8031 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -148,6 +148,7 @@ typedef void hapi_clock_init_t(void);
 
 /* ----------- end of HAB API updates ------------*/
 
-int authenticate_image(uint32_t ddr_start, uint32_t image_size);
+int authenticate_image(uint32_t ddr_start, uint32_t image_size,
+		       uint32_t ivt_offset);
 
 #endif
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 039a017..2a40d06 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -78,37 +78,6 @@
 	(is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 :	\
 	 (is_soc_type(MXC_SOC_MX7) ? 0x2000000 : 0x2))
 
-/*
- * +------------+  0x0 (DDR_UIMAGE_START) -
- * |   Header   |                          |
- * +------------+  0x40                    |
- * |            |                          |
- * |            |                          |
- * |            |                          |
- * |            |                          |
- * | Image Data |                          |
- * .            |                          |
- * .            |                           > Stuff to be authenticated ----+
- * .            |                          |                                |
- * |            |                          |                                |
- * |            |                          |                                |
- * +------------+                          |                                |
- * |            |                          |                                |
- * | Fill Data  |                          |                                |
- * |            |                          |                                |
- * +------------+ Align to ALIGN_SIZE      |                                |
- * |    IVT     |                          |                                |
- * +------------+ + IVT_SIZE              -                                 |
- * |            |                                                           |
- * |  CSF DATA  | <---------------------------------------------------------+
- * |            |
- * +------------+
- * |            |
- * | Fill Data  |
- * |            |
- * +------------+ + CSF_PAD_SIZE
- */
-
 static bool is_hab_enabled(void);
 
 #if !defined(CONFIG_SPL_BUILD)
@@ -361,20 +330,22 @@ int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
 				char * const argv[])
 {
-	ulong	addr, ivt_offset;
+	ulong	addr, length, ivt_offset;
 	int	rcode = 0;
 
-	if (argc < 3)
+	if (argc < 4)
 		return CMD_RET_USAGE;
 
 	addr = simple_strtoul(argv[1], NULL, 16);
-	ivt_offset = simple_strtoul(argv[2], NULL, 16);
+	length = simple_strtoul(argv[2], NULL, 16);
+	ivt_offset = simple_strtoul(argv[3], NULL, 16);
 
-	rcode = authenticate_image(addr, ivt_offset);
+	rcode = authenticate_image(addr, length, ivt_offset);
 	if (rcode == 0)
 		rcode = CMD_RET_SUCCESS;
 	else
 		rcode = CMD_RET_FAILURE;
+
 	return rcode;
 }
 
@@ -385,10 +356,11 @@ U_BOOT_CMD(
 	  );
 
 U_BOOT_CMD(
-		hab_auth_img, 3, 0, do_authenticate_image,
+		hab_auth_img, 4, 0, do_authenticate_image,
 		"authenticate image via HAB",
-		"addr ivt_offset\n"
+		"addr length ivt_offset\n"
 		"addr - image hex address\n"
+		"length - image hex length\n"
 		"ivt_offset - hex offset of IVT in the image"
 	  );
 
@@ -411,11 +383,12 @@ static bool is_hab_enabled(void)
 	return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
 }
 
-int authenticate_image(uint32_t ddr_start, uint32_t image_size)
+int authenticate_image(uint32_t ddr_start, uint32_t image_size,
+		       uint32_t ivt_offset)
 {
 	uint32_t load_addr = 0;
 	size_t bytes;
-	ptrdiff_t ivt_offset = 0;
+	uint32_t ivt_addr = 0;
 	int result = 1;
 	ulong start;
 	hab_rvt_authenticate_image_t *hab_rvt_authenticate_image;
@@ -441,24 +414,18 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 		goto hab_caam_clock_disable;
 	}
 
-	/* If not already aligned, Align to ALIGN_SIZE */
-	ivt_offset = (image_size + ALIGN_SIZE - 1) &
-			~(ALIGN_SIZE - 1);
-
+	/* Calculate IVT address header */
+	ivt_addr = ddr_start + ivt_offset;
 	start = ddr_start;
-	bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE;
+	bytes = image_size;
 #ifdef DEBUG
-	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n",
-	       ivt_offset, ddr_start + ivt_offset);
+	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr);
 	puts("Dumping IVT\n");
-	print_buffer(ddr_start + ivt_offset,
-		     (void *)(ddr_start + ivt_offset),
-		     4, 0x8, 0);
+	print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);
 
 	puts("Dumping CSF Header\n");
-	print_buffer(ddr_start + ivt_offset + IVT_SIZE,
-		     (void *)(ddr_start + ivt_offset + IVT_SIZE),
-		     4, 0x10, 0);
+	print_buffer(ivt_addr + IVT_SIZE, (void *)(ivt_addr + IVT_SIZE), 4,
+		     0x10, 0);
 
 #if  !defined(CONFIG_SPL_BUILD)
 	get_hab_status();
@@ -468,6 +435,8 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
 	printf("\tivt_offset = 0x%x\n", ivt_offset);
 	printf("\tstart = 0x%08lx\n", start);
 	printf("\tbytes = 0x%x\n", bytes);
+#else
+	(void)ivt_addr;
 #endif
 	/*
 	 * If the MMU is enabled, we have to notify the ROM
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 6e930b3..e5d0c35 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -152,9 +152,41 @@ u32 spl_boot_mode(const u32 boot_device)
 
 #if defined(CONFIG_SECURE_BOOT)
 
+/*
+ * +------------+  0x0 (DDR_UIMAGE_START) -
+ * |   Header   |                          |
+ * +------------+  0x40                    |
+ * |            |                          |
+ * |            |                          |
+ * |            |                          |
+ * |            |                          |
+ * | Image Data |                          |
+ * .            |                          |
+ * .            |                           > Stuff to be authenticated ----+
+ * .            |                          |                                |
+ * |            |                          |                                |
+ * |            |                          |                                |
+ * +------------+                          |                                |
+ * |            |                          |                                |
+ * | Fill Data  |                          |                                |
+ * |            |                          |                                |
+ * +------------+ Align to ALIGN_SIZE      |                                |
+ * |    IVT     |                          |                                |
+ * +------------+ + IVT_SIZE              -                                 |
+ * |            |                                                           |
+ * |  CSF DATA  | <---------------------------------------------------------+
+ * |            |
+ * +------------+
+ * |            |
+ * | Fill Data  |
+ * |            |
+ * +------------+ + CSF_PAD_SIZE
+ */
+
 __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 {
 	typedef void __noreturn (*image_entry_noargs_t)(void);
+	uint32_t offset;
 
 	image_entry_noargs_t image_entry =
 		(image_entry_noargs_t)(unsigned long)spl_image->entry_point;
@@ -163,8 +195,9 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 
 	/* HAB looks for the CSF at the end of the authenticated data therefore,
 	 * we need to subtract the size of the CSF from the actual filesize */
+	offset = spl_image->size - CONFIG_CSF_SIZE;
 	if (!authenticate_image(spl_image->load_addr,
-				spl_image->size - CONFIG_CSF_SIZE)) {
+				offset + IVT_SIZE + CSF_PAD_SIZE, offset)) {
 		image_entry();
 	} else {
 		puts("spl: ERROR:  image authentication unsuccessful\n");
-- 
2.7.4

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

* [U-Boot] [PATCH v6 08/25] arm: imx: hab: Add IVT header definitions
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (6 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 07/25] arm: imx: hab: Fix authenticate_image input parameters Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 09/25] arm: imx: hab: Add IVT header verification Bryan O'Donoghue
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The various i.MX BootROMs containing the High Assurance Boot (HAB) block
rely on a data structure called the Image Vector Table (IVT) to describe to
the BootROM where to locate various data-structures used by HAB during
authentication.

This patch adds a definition of the IVT header for use in later patches,
where we will break the current incorrect dependence on fixed offsets in
favour of an IVT described parsing of incoming binaries.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index b2a8031..28cde38 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -10,6 +10,34 @@
 
 #include <linux/types.h>
 
+/*
+ * IVT header definitions
+ * Security Reference Manual for i.MX 7Dual and 7Solo Applications Processors,
+ * Rev. 0, 03/2017
+ * Section : 6.7.1.1
+ */
+#define IVT_HEADER_MAGIC	0xD1
+#define IVT_TOTAL_LENGTH	0x20
+#define IVT_HEADER_V1		0x40
+#define IVT_HEADER_V2		0x41
+
+struct ivt_header {
+	uint8_t		magic;
+	uint16_t	length;
+	uint8_t		version;
+} __attribute__((packed));
+
+struct ivt {
+	struct ivt_header hdr;	/* IVT header above */
+	uint32_t entry;		/* Absolute address of first instruction */
+	uint32_t reserved1;	/* Reserved should be zero */
+	uint32_t dcd;		/* Absolute address of the image DCD */
+	uint32_t boot;		/* Absolute address of the boot data */
+	uint32_t self;		/* Absolute address of the IVT */
+	uint32_t csf;		/* Absolute address of the CSF */
+	uint32_t reserved2;	/* Reserved should be zero */
+};
+
 /* -------- start of HAB API updates ------------*/
 /* The following are taken from HAB4 SIS */
 
-- 
2.7.4

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

* [U-Boot] [PATCH v6 09/25] arm: imx: hab: Add IVT header verification
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (7 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 08/25] arm: imx: hab: Add IVT header definitions Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 10/25] arm: imx: hab: Verify IVT self matches calculated address Bryan O'Donoghue
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The IVT header contains a magic number, fixed length and one of two version
identifiers. Validate these settings before doing anything with a putative
IVT binary.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 2a40d06..998d253 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -80,6 +80,31 @@
 
 static bool is_hab_enabled(void);
 
+static int ivt_header_error(const char *err_str, struct ivt_header *ivt_hdr)
+{
+	printf("%s magic=0x%x length=0x%02x version=0x%x\n", err_str,
+	       ivt_hdr->magic, ivt_hdr->length, ivt_hdr->version);
+
+	return 1;
+}
+
+static int verify_ivt_header(struct ivt_header *ivt_hdr)
+{
+	int result = 0;
+
+	if (ivt_hdr->magic != IVT_HEADER_MAGIC)
+		result = ivt_header_error("bad magic", ivt_hdr);
+
+	if (be16_to_cpu(ivt_hdr->length) != IVT_TOTAL_LENGTH)
+		result = ivt_header_error("bad length", ivt_hdr);
+
+	if (ivt_hdr->version != IVT_HEADER_V1 &&
+	    ivt_hdr->version != IVT_HEADER_V2)
+		result = ivt_header_error("bad version", ivt_hdr);
+
+	return result;
+}
+
 #if !defined(CONFIG_SPL_BUILD)
 
 #define MAX_RECORD_BYTES     (8*1024) /* 4 kbytes */
@@ -394,6 +419,8 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	hab_rvt_authenticate_image_t *hab_rvt_authenticate_image;
 	hab_rvt_entry_t *hab_rvt_entry;
 	hab_rvt_exit_t *hab_rvt_exit;
+	struct ivt *ivt;
+	struct ivt_header *ivt_hdr;
 
 	hab_rvt_authenticate_image = hab_rvt_authenticate_image_p;
 	hab_rvt_entry = hab_rvt_entry_p;
@@ -416,6 +443,13 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	/* Calculate IVT address header */
 	ivt_addr = ddr_start + ivt_offset;
+	ivt = (struct ivt *)ivt_addr;
+	ivt_hdr = &ivt->hdr;
+
+	/* Verify IVT header bugging out on error */
+	if (verify_ivt_header(ivt_hdr))
+		goto hab_caam_clock_disable;
+
 	start = ddr_start;
 	bytes = image_size;
 #ifdef DEBUG
@@ -435,8 +469,6 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	printf("\tivt_offset = 0x%x\n", ivt_offset);
 	printf("\tstart = 0x%08lx\n", start);
 	printf("\tbytes = 0x%x\n", bytes);
-#else
-	(void)ivt_addr;
 #endif
 	/*
 	 * If the MMU is enabled, we have to notify the ROM
-- 
2.7.4

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

* [U-Boot] [PATCH v6 10/25] arm: imx: hab: Verify IVT self matches calculated address
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (8 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 09/25] arm: imx: hab: Add IVT header verification Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 11/25] arm: imx: hab: Only call ROM once headers are verified Bryan O'Donoghue
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The IVT is a self-describing structure which contains a self field. The
self field is the absolute physical base address the IVT ought to be at in
memory. Use the IVT self field to validate the calculated ivt_addr bugging
out if the two values differ.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 998d253..39f8f2d 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -450,6 +450,13 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	if (verify_ivt_header(ivt_hdr))
 		goto hab_caam_clock_disable;
 
+	/* Verify IVT body */
+	if (ivt->self != ivt_addr) {
+		printf("ivt->self 0x%08x pointer is 0x%08x\n",
+		       ivt->self, ivt_addr);
+		goto hab_caam_clock_disable;
+	}
+
 	start = ddr_start;
 	bytes = image_size;
 #ifdef DEBUG
-- 
2.7.4

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

* [U-Boot] [PATCH v6 11/25] arm: imx: hab: Only call ROM once headers are verified
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (9 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 10/25] arm: imx: hab: Verify IVT self matches calculated address Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 12/25] arm: imx: hab: Print CSF based on IVT descriptor Bryan O'Donoghue
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

Previous patches added IVT header verification steps. We shouldn't call
hab_rvt_entry() until we have done the basic header verification steps.

This patch changes the time we make the hab_rvt_entry() call so that it
only takes place if we are happy with the IVT header sanity checks.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 39f8f2d..a8e3e79 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -436,11 +436,6 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	hab_caam_clock_enable(1);
 
-	if (hab_rvt_entry() != HAB_SUCCESS) {
-		puts("hab entry function fail\n");
-		goto hab_caam_clock_disable;
-	}
-
 	/* Calculate IVT address header */
 	ivt_addr = ddr_start + ivt_offset;
 	ivt = (struct ivt *)ivt_addr;
@@ -459,6 +454,12 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	start = ddr_start;
 	bytes = image_size;
+
+	if (hab_rvt_entry() != HAB_SUCCESS) {
+		puts("hab entry function fail\n");
+		goto hab_caam_clock_disable;
+	}
+
 #ifdef DEBUG
 	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr);
 	puts("Dumping IVT\n");
-- 
2.7.4

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

* [U-Boot] [PATCH v6 12/25] arm: imx: hab: Print CSF based on IVT descriptor
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (10 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 11/25] arm: imx: hab: Only call ROM once headers are verified Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 13/25] arm: imx: hab: Print additional IVT elements during debug Bryan O'Donoghue
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The IVT gives the absolute address of the CSF. There is no requirement for
the CSF to be located adjacent to the IVT so lets use the address provided
in the IVT header instead of the hard-coded fixed CSF offset currently in
place.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index a8e3e79..229c723 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -466,8 +466,7 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);
 
 	puts("Dumping CSF Header\n");
-	print_buffer(ivt_addr + IVT_SIZE, (void *)(ivt_addr + IVT_SIZE), 4,
-		     0x10, 0);
+	print_buffer(ivt->csf, (void *)(ivt->csf), 4, 0x10, 0);
 
 #if  !defined(CONFIG_SPL_BUILD)
 	get_hab_status();
-- 
2.7.4

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

* [U-Boot] [PATCH v6 13/25] arm: imx: hab: Print additional IVT elements during debug
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (11 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 12/25] arm: imx: hab: Print CSF based on IVT descriptor Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 14/25] arm: imx: hab: Define rvt_check_target() Bryan O'Donoghue
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

This patch enables printout of the IVT entry, dcd and csf data fields.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 229c723..364bd6b 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -462,6 +462,8 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 #ifdef DEBUG
 	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr);
+	printf("ivt entry = 0x%08x, dcd = 0x%08x, csf = 0x%08x\n", ivt->entry,
+	       ivt->dcd, ivt->csf);
 	puts("Dumping IVT\n");
 	print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);
 
-- 
2.7.4

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

* [U-Boot] [PATCH v6 14/25] arm: imx: hab: Define rvt_check_target()
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (12 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 13/25] arm: imx: hab: Print additional IVT elements during debug Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 15/25] arm: imx: hab: Implement hab_rvt_check_target Bryan O'Donoghue
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The hab_rvt_check_target() callback according to the HABv4 documentation:

"This function reports whether or not a given target region is allowed for
 either peripheral configuration or image loading in memory. It is intended
 for use by post-ROM boot stage components, via the ROM Vector Table, in
 order to avoid configuring security-sensitive peripherals, or loading
 images over sensitive memory regions or outside recognized memory devices
 in the address map."

It is a useful function to support as a precursor to calling into
authenticate_image() to validate the target memory region is good.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 28cde38..14e1220 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -113,6 +113,12 @@ enum hab_context {
 	HAB_CTX_MAX
 };
 
+enum hab_target {
+	HAB_TGT_MEMORY		= 0x0f,
+	HAB_TGT_PERIPHERAL	= 0xf0,
+	HAB_TGT_ANY		= 0x55,
+};
+
 struct imx_sec_config_fuse_t {
 	int bank;
 	int word;
@@ -132,6 +138,8 @@ typedef enum hab_status hab_rvt_entry_t(void);
 typedef enum hab_status hab_rvt_exit_t(void);
 typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_t,
 		void **, size_t *, hab_loader_callback_f_t);
+typedef enum hab_status hab_rvt_check_target_t(enum hab_target, const void *,
+					       size_t);
 typedef void hapi_clock_init_t(void);
 
 #define HAB_ENG_ANY		0x00   /* Select first compatible engine */
@@ -158,6 +166,7 @@ typedef void hapi_clock_init_t(void);
 
 #define HAB_RVT_ENTRY			(*(uint32_t *)(HAB_RVT_BASE + 0x04))
 #define HAB_RVT_EXIT			(*(uint32_t *)(HAB_RVT_BASE + 0x08))
+#define HAB_RVT_CHECK_TARGET		(*(uint32_t *)(HAB_RVT_BASE + 0x0C))
 #define HAB_RVT_AUTHENTICATE_IMAGE	(*(uint32_t *)(HAB_RVT_BASE + 0x10))
 #define HAB_RVT_REPORT_EVENT		(*(uint32_t *)(HAB_RVT_BASE + 0x20))
 #define HAB_RVT_REPORT_STATUS		(*(uint32_t *)(HAB_RVT_BASE + 0x24))
-- 
2.7.4

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

* [U-Boot] [PATCH v6 15/25] arm: imx: hab: Implement hab_rvt_check_target
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (13 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 14/25] arm: imx: hab: Define rvt_check_target() Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 16/25] arm: imx: hab: Add a hab_rvt_check_target to image auth Bryan O'Donoghue
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

This patch implements the basic callback hooks for hab_rvt_check_target()
for BootROM code using the older BootROM address layout - in my test case
the i.MX7. Code based on new BootROM callbacks will just have HAB_SUCCESS
as a result code. Adding support for the new BootROM callbacks is a TODO.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 364bd6b..2a18ea2 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -70,6 +70,24 @@
 	((hab_rvt_exit_t *)HAB_RVT_EXIT)			\
 )
 
+static inline enum hab_status hab_rvt_check_target_new(enum hab_target target,
+						       const void *start,
+						       size_t bytes)
+{
+	return HAB_SUCCESS;
+}
+
+#define hab_rvt_check_target_p					\
+(								\
+	(is_mx6dqp()) ?						\
+	((hab_rvt_check_target_t *)hab_rvt_check_target_new) :	\
+	(is_mx6dq() && (soc_rev() >= CHIP_REV_1_5)) ?		\
+	((hab_rvt_check_target_t *)hab_rvt_check_target_new) :	\
+	(is_mx6sdl() &&	(soc_rev() >= CHIP_REV_1_2)) ?		\
+	((hab_rvt_check_target_t *)hab_rvt_check_target_new) :	\
+	((hab_rvt_check_target_t *)HAB_RVT_CHECK_TARGET)	\
+)
+
 #define ALIGN_SIZE		0x1000
 #define MX6DQ_PU_IROM_MMU_EN_VAR	0x009024a8
 #define MX6DLS_PU_IROM_MMU_EN_VAR	0x00901dd0
-- 
2.7.4

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

* [U-Boot] [PATCH v6 16/25] arm: imx: hab: Add a hab_rvt_check_target to image auth
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (14 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 15/25] arm: imx: hab: Implement hab_rvt_check_target Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 17/25] arm: imx: hab: Print HAB event log only after calling ROM Bryan O'Donoghue
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

Add a hab_rvt_check_target() step to authenticate_image() as a sanity
check for the target memory region authenticate_image() will run over,
prior to making the BootROM authentication callback itself.

This check is recommended by the HAB documentation so it makes sense to
adhere to the guidance and perform that check as directed.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 2a18ea2..079423a 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -437,12 +437,15 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	hab_rvt_authenticate_image_t *hab_rvt_authenticate_image;
 	hab_rvt_entry_t *hab_rvt_entry;
 	hab_rvt_exit_t *hab_rvt_exit;
+	hab_rvt_check_target_t *hab_rvt_check_target;
 	struct ivt *ivt;
 	struct ivt_header *ivt_hdr;
+	enum hab_status status;
 
 	hab_rvt_authenticate_image = hab_rvt_authenticate_image_p;
 	hab_rvt_entry = hab_rvt_entry_p;
 	hab_rvt_exit = hab_rvt_exit_p;
+	hab_rvt_check_target = hab_rvt_check_target_p;
 
 	if (!is_hab_enabled()) {
 		puts("hab fuse not enabled\n");
@@ -478,6 +481,12 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 		goto hab_caam_clock_disable;
 	}
 
+	status = hab_rvt_check_target(HAB_TGT_MEMORY, (void *)ddr_start, bytes);
+	if (status != HAB_SUCCESS) {
+		printf("HAB check target 0x%08x-0x%08x fail\n",
+		       ddr_start, ddr_start + bytes);
+		goto hab_caam_clock_disable;
+	}
 #ifdef DEBUG
 	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr);
 	printf("ivt entry = 0x%08x, dcd = 0x%08x, csf = 0x%08x\n", ivt->entry,
-- 
2.7.4

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

* [U-Boot] [PATCH v6 17/25] arm: imx: hab: Print HAB event log only after calling ROM
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (15 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 16/25] arm: imx: hab: Add a hab_rvt_check_target to image auth Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 18/25] arm: imx: hab: Make internal functions and data static Bryan O'Donoghue
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The current flow of authenticate_image() will print the HAB event log even
if we reject an element of the IVT header before ever calling into the ROM.
This can be confusing.

This patch changes the flow of the code so that the HAB event log is only
printed out if we have called into the ROM and received some sort of status
code.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Suggested-by: Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 079423a..3ae88a4 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -478,14 +478,14 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	if (hab_rvt_entry() != HAB_SUCCESS) {
 		puts("hab entry function fail\n");
-		goto hab_caam_clock_disable;
+		goto hab_exit_failure_print_status;
 	}
 
 	status = hab_rvt_check_target(HAB_TGT_MEMORY, (void *)ddr_start, bytes);
 	if (status != HAB_SUCCESS) {
 		printf("HAB check target 0x%08x-0x%08x fail\n",
 		       ddr_start, ddr_start + bytes);
-		goto hab_caam_clock_disable;
+		goto hab_exit_failure_print_status;
 	}
 #ifdef DEBUG
 	printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", ivt_offset, ivt_addr);
@@ -543,12 +543,14 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size,
 		load_addr = 0;
 	}
 
-hab_caam_clock_disable:
-	hab_caam_clock_enable(0);
-
+hab_exit_failure_print_status:
 #if !defined(CONFIG_SPL_BUILD)
 	get_hab_status();
 #endif
+
+hab_caam_clock_disable:
+	hab_caam_clock_enable(0);
+
 	if (load_addr != 0)
 		result = 0;
 
-- 
2.7.4

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

* [U-Boot] [PATCH v6 18/25] arm: imx: hab: Make internal functions and data static
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (16 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 17/25] arm: imx: hab: Print HAB event log only after calling ROM Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 19/25] arm: imx: hab: Prefix authenticate_image with imx_hab Bryan O'Donoghue
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

There is no need to export these functions and data structures externally.
Make them all static now.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 159 +++++++++++++++++++++++++-----------------------
 1 file changed, 84 insertions(+), 75 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 3ae88a4..ec85548 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -135,73 +135,81 @@ struct record {
 	bool	 any_rec_flag;
 };
 
-char *rsn_str[] = {"RSN = HAB_RSN_ANY (0x00)\n",
-				   "RSN = HAB_ENG_FAIL (0x30)\n",
-				   "RSN = HAB_INV_ADDRESS (0x22)\n",
-				   "RSN = HAB_INV_ASSERTION (0x0C)\n",
-				   "RSN = HAB_INV_CALL (0x28)\n",
-				   "RSN = HAB_INV_CERTIFICATE (0x21)\n",
-				   "RSN = HAB_INV_COMMAND (0x06)\n",
-				   "RSN = HAB_INV_CSF (0x11)\n",
-				   "RSN = HAB_INV_DCD (0x27)\n",
-				   "RSN = HAB_INV_INDEX (0x0F)\n",
-				   "RSN = HAB_INV_IVT (0x05)\n",
-				   "RSN = HAB_INV_KEY (0x1D)\n",
-				   "RSN = HAB_INV_RETURN (0x1E)\n",
-				   "RSN = HAB_INV_SIGNATURE (0x18)\n",
-				   "RSN = HAB_INV_SIZE (0x17)\n",
-				   "RSN = HAB_MEM_FAIL (0x2E)\n",
-				   "RSN = HAB_OVR_COUNT (0x2B)\n",
-				   "RSN = HAB_OVR_STORAGE (0x2D)\n",
-				   "RSN = HAB_UNS_ALGORITHM (0x12)\n",
-				   "RSN = HAB_UNS_COMMAND (0x03)\n",
-				   "RSN = HAB_UNS_ENGINE (0x0A)\n",
-				   "RSN = HAB_UNS_ITEM (0x24)\n",
-				   "RSN = HAB_UNS_KEY (0x1B)\n",
-				   "RSN = HAB_UNS_PROTOCOL (0x14)\n",
-				   "RSN = HAB_UNS_STATE (0x09)\n",
-				   "RSN = INVALID\n",
-				   NULL};
-
-char *sts_str[] = {"STS = HAB_SUCCESS (0xF0)\n",
-				   "STS = HAB_FAILURE (0x33)\n",
-				   "STS = HAB_WARNING (0x69)\n",
-				   "STS = INVALID\n",
-				   NULL};
-
-char *eng_str[] = {"ENG = HAB_ENG_ANY (0x00)\n",
-				   "ENG = HAB_ENG_SCC (0x03)\n",
-				   "ENG = HAB_ENG_RTIC (0x05)\n",
-				   "ENG = HAB_ENG_SAHARA (0x06)\n",
-				   "ENG = HAB_ENG_CSU (0x0A)\n",
-				   "ENG = HAB_ENG_SRTC (0x0C)\n",
-				   "ENG = HAB_ENG_DCP (0x1B)\n",
-				   "ENG = HAB_ENG_CAAM (0x1D)\n",
-				   "ENG = HAB_ENG_SNVS (0x1E)\n",
-				   "ENG = HAB_ENG_OCOTP (0x21)\n",
-				   "ENG = HAB_ENG_DTCP (0x22)\n",
-				   "ENG = HAB_ENG_ROM (0x36)\n",
-				   "ENG = HAB_ENG_HDCP (0x24)\n",
-				   "ENG = HAB_ENG_RTL (0x77)\n",
-				   "ENG = HAB_ENG_SW (0xFF)\n",
-				   "ENG = INVALID\n",
-				   NULL};
-
-char *ctx_str[] = {"CTX = HAB_CTX_ANY(0x00)\n",
-				   "CTX = HAB_CTX_FAB (0xFF)\n",
-				   "CTX = HAB_CTX_ENTRY (0xE1)\n",
-				   "CTX = HAB_CTX_TARGET (0x33)\n",
-				   "CTX = HAB_CTX_AUTHENTICATE (0x0A)\n",
-				   "CTX = HAB_CTX_DCD (0xDD)\n",
-				   "CTX = HAB_CTX_CSF (0xCF)\n",
-				   "CTX = HAB_CTX_COMMAND (0xC0)\n",
-				   "CTX = HAB_CTX_AUT_DAT (0xDB)\n",
-				   "CTX = HAB_CTX_ASSERT (0xA0)\n",
-				   "CTX = HAB_CTX_EXIT (0xEE)\n",
-				   "CTX = INVALID\n",
-				   NULL};
-
-uint8_t hab_statuses[5] = {
+static char *rsn_str[] = {
+			  "RSN = HAB_RSN_ANY (0x00)\n",
+			  "RSN = HAB_ENG_FAIL (0x30)\n",
+			  "RSN = HAB_INV_ADDRESS (0x22)\n",
+			  "RSN = HAB_INV_ASSERTION (0x0C)\n",
+			  "RSN = HAB_INV_CALL (0x28)\n",
+			  "RSN = HAB_INV_CERTIFICATE (0x21)\n",
+			  "RSN = HAB_INV_COMMAND (0x06)\n",
+			  "RSN = HAB_INV_CSF (0x11)\n",
+			  "RSN = HAB_INV_DCD (0x27)\n",
+			  "RSN = HAB_INV_INDEX (0x0F)\n",
+			  "RSN = HAB_INV_IVT (0x05)\n",
+			  "RSN = HAB_INV_KEY (0x1D)\n",
+			  "RSN = HAB_INV_RETURN (0x1E)\n",
+			  "RSN = HAB_INV_SIGNATURE (0x18)\n",
+			  "RSN = HAB_INV_SIZE (0x17)\n",
+			  "RSN = HAB_MEM_FAIL (0x2E)\n",
+			  "RSN = HAB_OVR_COUNT (0x2B)\n",
+			  "RSN = HAB_OVR_STORAGE (0x2D)\n",
+			  "RSN = HAB_UNS_ALGORITHM (0x12)\n",
+			  "RSN = HAB_UNS_COMMAND (0x03)\n",
+			  "RSN = HAB_UNS_ENGINE (0x0A)\n",
+			  "RSN = HAB_UNS_ITEM (0x24)\n",
+			  "RSN = HAB_UNS_KEY (0x1B)\n",
+			  "RSN = HAB_UNS_PROTOCOL (0x14)\n",
+			  "RSN = HAB_UNS_STATE (0x09)\n",
+			  "RSN = INVALID\n",
+			  NULL
+};
+
+static char *sts_str[] = {
+			  "STS = HAB_SUCCESS (0xF0)\n",
+			  "STS = HAB_FAILURE (0x33)\n",
+			  "STS = HAB_WARNING (0x69)\n",
+			  "STS = INVALID\n",
+			  NULL
+};
+
+static char *eng_str[] = {
+			  "ENG = HAB_ENG_ANY (0x00)\n",
+			  "ENG = HAB_ENG_SCC (0x03)\n",
+			  "ENG = HAB_ENG_RTIC (0x05)\n",
+			  "ENG = HAB_ENG_SAHARA (0x06)\n",
+			  "ENG = HAB_ENG_CSU (0x0A)\n",
+			  "ENG = HAB_ENG_SRTC (0x0C)\n",
+			  "ENG = HAB_ENG_DCP (0x1B)\n",
+			  "ENG = HAB_ENG_CAAM (0x1D)\n",
+			  "ENG = HAB_ENG_SNVS (0x1E)\n",
+			  "ENG = HAB_ENG_OCOTP (0x21)\n",
+			  "ENG = HAB_ENG_DTCP (0x22)\n",
+			  "ENG = HAB_ENG_ROM (0x36)\n",
+			  "ENG = HAB_ENG_HDCP (0x24)\n",
+			  "ENG = HAB_ENG_RTL (0x77)\n",
+			  "ENG = HAB_ENG_SW (0xFF)\n",
+			  "ENG = INVALID\n",
+			  NULL
+};
+
+static char *ctx_str[] = {
+			  "CTX = HAB_CTX_ANY(0x00)\n",
+			  "CTX = HAB_CTX_FAB (0xFF)\n",
+			  "CTX = HAB_CTX_ENTRY (0xE1)\n",
+			  "CTX = HAB_CTX_TARGET (0x33)\n",
+			  "CTX = HAB_CTX_AUTHENTICATE (0x0A)\n",
+			  "CTX = HAB_CTX_DCD (0xDD)\n",
+			  "CTX = HAB_CTX_CSF (0xCF)\n",
+			  "CTX = HAB_CTX_COMMAND (0xC0)\n",
+			  "CTX = HAB_CTX_AUT_DAT (0xDB)\n",
+			  "CTX = HAB_CTX_ASSERT (0xA0)\n",
+			  "CTX = HAB_CTX_EXIT (0xEE)\n",
+			  "CTX = INVALID\n",
+			  NULL
+};
+
+static uint8_t hab_statuses[5] = {
 	HAB_STS_ANY,
 	HAB_FAILURE,
 	HAB_WARNING,
@@ -209,7 +217,7 @@ uint8_t hab_statuses[5] = {
 	-1
 };
 
-uint8_t hab_reasons[26] = {
+static uint8_t hab_reasons[26] = {
 	HAB_RSN_ANY,
 	HAB_ENG_FAIL,
 	HAB_INV_ADDRESS,
@@ -238,7 +246,7 @@ uint8_t hab_reasons[26] = {
 	-1
 };
 
-uint8_t hab_contexts[12] = {
+static uint8_t hab_contexts[12] = {
 	HAB_CTX_ANY,
 	HAB_CTX_FAB,
 	HAB_CTX_ENTRY,
@@ -253,7 +261,7 @@ uint8_t hab_contexts[12] = {
 	-1
 };
 
-uint8_t hab_engines[16] = {
+static uint8_t hab_engines[16] = {
 	HAB_ENG_ANY,
 	HAB_ENG_SCC,
 	HAB_ENG_RTIC,
@@ -284,7 +292,7 @@ static inline uint8_t get_idx(uint8_t *list, uint8_t tgt)
 	return -1;
 }
 
-void process_event_record(uint8_t *event_data, size_t bytes)
+static void process_event_record(uint8_t *event_data, size_t bytes)
 {
 	struct record *rec = (struct record *)event_data;
 
@@ -294,7 +302,7 @@ void process_event_record(uint8_t *event_data, size_t bytes)
 	printf("%s", eng_str[get_idx(hab_engines, rec->contents[3])]);
 }
 
-void display_event(uint8_t *event_data, size_t bytes)
+static void display_event(uint8_t *event_data, size_t bytes)
 {
 	uint32_t i;
 
@@ -313,7 +321,7 @@ void display_event(uint8_t *event_data, size_t bytes)
 	process_event_record(event_data, bytes);
 }
 
-int get_hab_status(void)
+static int get_hab_status(void)
 {
 	uint32_t index = 0; /* Loop index */
 	uint8_t event_data[128]; /* Event data buffer */
@@ -358,7 +366,8 @@ int get_hab_status(void)
 	return 0;
 }
 
-int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
 {
 	if ((argc != 1)) {
 		cmd_usage(cmdtp);
@@ -371,7 +380,7 @@ int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
-				char * const argv[])
+				 char * const argv[])
 {
 	ulong	addr, length, ivt_offset;
 	int	rcode = 0;
-- 
2.7.4

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

* [U-Boot] [PATCH v6 19/25] arm: imx: hab: Prefix authenticate_image with imx_hab
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (17 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 18/25] arm: imx: hab: Make internal functions and data static Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 20/25] arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled Bryan O'Donoghue
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

Tidy up the HAB namespace a bit by prefixing external functions with
imx_hab. All external facing functions past this point will be prefixed in
the same way to make the fact we are doing IMX HAB activities clear from
reading the code. authenticate_image() could mean anything
imx_hab_authenticate_image() is on the other hand very explicit.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 4 ++--
 arch/arm/mach-imx/hab.c             | 6 +++---
 arch/arm/mach-imx/spl.c             | 5 +++--
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 14e1220..98bc1bd 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -185,7 +185,7 @@ typedef void hapi_clock_init_t(void);
 
 /* ----------- end of HAB API updates ------------*/
 
-int authenticate_image(uint32_t ddr_start, uint32_t image_size,
-		       uint32_t ivt_offset);
+int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
+			       uint32_t ivt_offset);
 
 #endif
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index ec85548..7c2f828 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -392,7 +392,7 @@ static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
 	length = simple_strtoul(argv[2], NULL, 16);
 	ivt_offset = simple_strtoul(argv[3], NULL, 16);
 
-	rcode = authenticate_image(addr, length, ivt_offset);
+	rcode = imx_hab_authenticate_image(addr, length, ivt_offset);
 	if (rcode == 0)
 		rcode = CMD_RET_SUCCESS;
 	else
@@ -435,8 +435,8 @@ static bool is_hab_enabled(void)
 	return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
 }
 
-int authenticate_image(uint32_t ddr_start, uint32_t image_size,
-		       uint32_t ivt_offset)
+int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
+			       uint32_t ivt_offset)
 {
 	uint32_t load_addr = 0;
 	size_t bytes;
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index e5d0c35..a5478ce 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -196,8 +196,9 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 	/* HAB looks for the CSF at the end of the authenticated data therefore,
 	 * we need to subtract the size of the CSF from the actual filesize */
 	offset = spl_image->size - CONFIG_CSF_SIZE;
-	if (!authenticate_image(spl_image->load_addr,
-				offset + IVT_SIZE + CSF_PAD_SIZE, offset)) {
+	if (!imx_hab_authenticate_image(spl_image->load_addr,
+					offset + IVT_SIZE + CSF_PAD_SIZE,
+					offset)) {
 		image_entry();
 	} else {
 		puts("spl: ERROR:  image authentication unsuccessful\n");
-- 
2.7.4

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

* [U-Boot] [PATCH v6 20/25] arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (18 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 19/25] arm: imx: hab: Prefix authenticate_image with imx_hab Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards Bryan O'Donoghue
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

Understanding if the HAB is enabled is something that we want to
interrogate and report on outside of the HAB layer. First step to that is
renaming the relevant function to match the previously introduced external
naming convention imx_hab_function()

The name imx_hab_is_hab_enabled() is a tautology. A more logical name is
imx_hab_is_enabled().

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 7c2f828..d917ac3 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -96,7 +96,7 @@ static inline enum hab_status hab_rvt_check_target_new(enum hab_target target,
 	(is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 :	\
 	 (is_soc_type(MXC_SOC_MX7) ? 0x2000000 : 0x2))
 
-static bool is_hab_enabled(void);
+static bool imx_hab_is_enabled(void);
 
 static int ivt_header_error(const char *err_str, struct ivt_header *ivt_hdr)
 {
@@ -334,7 +334,7 @@ static int get_hab_status(void)
 	hab_rvt_report_event = hab_rvt_report_event_p;
 	hab_rvt_report_status = hab_rvt_report_status_p;
 
-	if (is_hab_enabled())
+	if (imx_hab_is_enabled())
 		puts("\nSecure boot enabled\n");
 	else
 		puts("\nSecure boot disabled\n");
@@ -419,7 +419,7 @@ U_BOOT_CMD(
 
 #endif /* !defined(CONFIG_SPL_BUILD) */
 
-static bool is_hab_enabled(void)
+static bool imx_hab_is_enabled(void)
 {
 	struct imx_sec_config_fuse_t *fuse =
 		(struct imx_sec_config_fuse_t *)&imx_sec_config_fuse;
@@ -456,7 +456,7 @@ int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
 	hab_rvt_exit = hab_rvt_exit_p;
 	hab_rvt_check_target = hab_rvt_check_target_p;
 
-	if (!is_hab_enabled()) {
+	if (!imx_hab_is_enabled()) {
 		puts("hab fuse not enabled\n");
 		return result;
 	}
-- 
2.7.4

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

* [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (19 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 20/25] arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 15:37   ` Breno Matheus Lima
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 22/25] arm: imx: hab: Make imx_hab_is_enabled global Bryan O'Donoghue
                   ` (4 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The BootROM will not successfully process a HAB image passed by u-boot
unless the board has been set into locked mode. Some of the existing usages
of authenticate_image() expect and rely on unlocked boards doing the
following

1. Not calling into the BootROM authenticate_image() callback
2. Returning a pass status for authenticate_image() calls anyway

A previous patch removed the necessity to call into imx_hab_is_enabled()
twice. This patch ensures the reliance on authenticate_image() returning
zero is maintained.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Suggested-by: Breno Matheus Lima <brenomatheus@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
---
 arch/arm/mach-imx/hab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index d917ac3..3b19a7e 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -458,7 +458,7 @@ int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
 
 	if (!imx_hab_is_enabled()) {
 		puts("hab fuse not enabled\n");
-		return result;
+		return 0;
 	}
 
 	printf("\nAuthenticate image from DDR location 0x%x...\n",
-- 
2.7.4

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

* [U-Boot] [PATCH v6 22/25] arm: imx: hab: Make imx_hab_is_enabled global
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (20 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 23/25] arm: imx: hab: Define rvt_failsafe() Bryan O'Donoghue
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

It will be helpful to boot commands to know if the HAB is enabled. Export
imx_hab_is_enabled() now to facilitate further work with this data-point in
a secure-boot context.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 1 +
 arch/arm/mach-imx/hab.c             | 4 +---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 98bc1bd..5c13aff 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -187,5 +187,6 @@ typedef void hapi_clock_init_t(void);
 
 int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size,
 			       uint32_t ivt_offset);
+bool imx_hab_is_enabled(void);
 
 #endif
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 3b19a7e..d1c5f69 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -96,8 +96,6 @@ static inline enum hab_status hab_rvt_check_target_new(enum hab_target target,
 	(is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 :	\
 	 (is_soc_type(MXC_SOC_MX7) ? 0x2000000 : 0x2))
 
-static bool imx_hab_is_enabled(void);
-
 static int ivt_header_error(const char *err_str, struct ivt_header *ivt_hdr)
 {
 	printf("%s magic=0x%x length=0x%02x version=0x%x\n", err_str,
@@ -419,7 +417,7 @@ U_BOOT_CMD(
 
 #endif /* !defined(CONFIG_SPL_BUILD) */
 
-static bool imx_hab_is_enabled(void)
+bool imx_hab_is_enabled(void)
 {
 	struct imx_sec_config_fuse_t *fuse =
 		(struct imx_sec_config_fuse_t *)&imx_sec_config_fuse;
-- 
2.7.4

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

* [U-Boot] [PATCH v6 23/25] arm: imx: hab: Define rvt_failsafe()
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (21 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 22/25] arm: imx: hab: Make imx_hab_is_enabled global Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 24/25] arm: imx: hab: Implement hab_rvt_failsafe Bryan O'Donoghue
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

The hab_rvt_failsafe() callback according to the HABv4 documentation:

"This function provides a safe path when image authentication has failed
and all possible boot paths have been exhausted. It is intended for use by
post-ROM boot stage components, via the ROM Vector Table."

Once invoked the part will drop down to its BootROM USB recovery mode.
Should it be the case that the part is in secure boot mode - only an
appropriately signed binary will be accepted by the ROM and subsequently
executed.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 5c13aff..a0cb19d 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -140,6 +140,7 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_t,
 		void **, size_t *, hab_loader_callback_f_t);
 typedef enum hab_status hab_rvt_check_target_t(enum hab_target, const void *,
 					       size_t);
+typedef void hab_rvt_failsafe_t(void);
 typedef void hapi_clock_init_t(void);
 
 #define HAB_ENG_ANY		0x00   /* Select first compatible engine */
@@ -170,6 +171,7 @@ typedef void hapi_clock_init_t(void);
 #define HAB_RVT_AUTHENTICATE_IMAGE	(*(uint32_t *)(HAB_RVT_BASE + 0x10))
 #define HAB_RVT_REPORT_EVENT		(*(uint32_t *)(HAB_RVT_BASE + 0x20))
 #define HAB_RVT_REPORT_STATUS		(*(uint32_t *)(HAB_RVT_BASE + 0x24))
+#define HAB_RVT_FAILSAFE		(*(uint32_t *)(HAB_RVT_BASE + 0x28))
 
 #define HAB_RVT_REPORT_EVENT_NEW               (*(uint32_t *)0x000000B8)
 #define HAB_RVT_REPORT_STATUS_NEW              (*(uint32_t *)0x000000BC)
-- 
2.7.4

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

* [U-Boot] [PATCH v6 24/25] arm: imx: hab: Implement hab_rvt_failsafe
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (22 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 23/25] arm: imx: hab: Define rvt_failsafe() Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 25/25] arm: imx: hab: Add hab_failsafe console command Bryan O'Donoghue
  2018-02-08 12:43 ` [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Jagan Teki
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

This patch implements the basic callback hooks for
hab_rvt_check_failsafe for BootROM code using the older BootROM address
layout - in my test case the i.MX7. Code based on new BootROM callbacks
will just do nothing and there's definitely a TODO to implement that extra
functionality on the alternative BootROM API.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index d1c5f69..1236717 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -70,6 +70,21 @@
 	((hab_rvt_exit_t *)HAB_RVT_EXIT)			\
 )
 
+static inline void hab_rvt_failsafe_new(void)
+{
+}
+
+#define hab_rvt_failsafe_p				\
+(							\
+	(is_mx6dqp()) ?					\
+	((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) :	\
+	(is_mx6dq() && (soc_rev() >= CHIP_REV_1_5)) ?	\
+	((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) :	\
+	(is_mx6sdl() && (soc_rev() >= CHIP_REV_1_2)) ?	\
+	((hab_rvt_failsafe_t *)hab_rvt_failsafe_new) :	\
+	((hab_rvt_failsafe_t *)HAB_RVT_FAILSAFE)	\
+)
+
 static inline enum hab_status hab_rvt_check_target_new(enum hab_target target,
 						       const void *start,
 						       size_t bytes)
-- 
2.7.4

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

* [U-Boot] [PATCH v6 25/25] arm: imx: hab: Add hab_failsafe console command
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (23 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 24/25] arm: imx: hab: Implement hab_rvt_failsafe Bryan O'Donoghue
@ 2018-01-12 12:40 ` Bryan O'Donoghue
  2018-02-08 12:43 ` [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Jagan Teki
  25 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-01-12 12:40 UTC (permalink / raw)
  To: u-boot

hab_failsafe when called puts the part into BootROM recovery mode.
This will allow u-boot scripts to script the dropping down into recovery
mode.

=> hab_failsafe

Shows the i.MX7 appear as "hiddev0,hidraw5: USB HID v1.10 Device [Freescale
SemiConductor Inc  SP Blank ULT1] " in a Linux dmesg thus allowing download
of a new image via the BootROM USB download protocol routine.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Breno Matheus Lima <brenomatheus@gmail.com>
Tested-by: Breno Lima <breno.lima@nxp.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/mach-imx/hab.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 1236717..5f19777 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -414,6 +414,22 @@ static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
 	return rcode;
 }
 
+static int do_hab_failsafe(cmd_tbl_t *cmdtp, int flag, int argc,
+			   char * const argv[])
+{
+	hab_rvt_failsafe_t *hab_rvt_failsafe;
+
+	if (argc != 1) {
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	hab_rvt_failsafe = hab_rvt_failsafe_p;
+	hab_rvt_failsafe();
+
+	return 0;
+}
+
 U_BOOT_CMD(
 		hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status,
 		"display HAB status",
@@ -429,6 +445,11 @@ U_BOOT_CMD(
 		"ivt_offset - hex offset of IVT in the image"
 	  );
 
+U_BOOT_CMD(
+		hab_failsafe, CONFIG_SYS_MAXARGS, 1, do_hab_failsafe,
+		"run BootROM failsafe routine",
+		""
+	  );
 
 #endif /* !defined(CONFIG_SPL_BUILD) */
 
-- 
2.7.4

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

* [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards Bryan O'Donoghue
@ 2018-01-12 15:37   ` Breno Matheus Lima
  0 siblings, 0 replies; 34+ messages in thread
From: Breno Matheus Lima @ 2018-01-12 15:37 UTC (permalink / raw)
  To: u-boot

Hi Bryan,

2018-01-12 10:40 GMT-02:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
> The BootROM will not successfully process a HAB image passed by u-boot
> unless the board has been set into locked mode. Some of the existing usages
> of authenticate_image() expect and rely on unlocked boards doing the
> following
>
> 1. Not calling into the BootROM authenticate_image() callback
> 2. Returning a pass status for authenticate_image() calls anyway
>
> A previous patch removed the necessity to call into imx_hab_is_enabled()
> twice. This patch ensures the reliance on authenticate_image() returning
> zero is maintained.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Suggested-by: Breno Matheus Lima <brenomatheus@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
> Cc: George McCollister <george.mccollister@gmail.com>

Thanks for adding this patch on the series, I tested on an open
mx6ulevk board and it's working fine.

Tested-by: Breno Lima <breno.lima@nxp.com>

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

* [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int
  2018-01-12 12:39 ` [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int Bryan O'Donoghue
@ 2018-01-14 16:44   ` Stefano Babic
  0 siblings, 0 replies; 34+ messages in thread
From: Stefano Babic @ 2018-01-14 16:44 UTC (permalink / raw)
  To: u-boot

On 12/01/2018 13:39, Bryan O'Donoghue wrote:
> Both usages of authenticate_image treat the result code as a simple binary.
> The command line usage of authenticate_image directly returns the result
> code of authenticate_image as a success/failure code.
> 
> Right now when calling hab_auth_img and test the result code in a shell a
> passing hab_auth_img will appear to the shell as a fail.
> 
> The first step in fixing this behaviour is to fix-up the result code return
> by authenticate_image() itself, subsequent patches fix the interpretation
> of authenticate_image so that zero will return CMD_RET_SUCCESS and non-zero
> will return CMD_RET_FAILURE.
> 
> The first step is fixing the return type in authenticate_image() so do that
> now.


Thanks for your series - applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Sven Ebenfeld <sven.ebenfeld@gmail.com>
> Cc: George McCollister <george.mccollister@gmail.com>
> Cc: Breno Matheus Lima <brenomatheus@gmail.com>
> Tested-by: Breno Lima <breno.lima@nxp.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  arch/arm/include/asm/mach-imx/hab.h | 2 +-
>  arch/arm/mach-imx/hab.c             | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
> index e0ff459..1b7a5e4 100644
> --- a/arch/arm/include/asm/mach-imx/hab.h
> +++ b/arch/arm/include/asm/mach-imx/hab.h
> @@ -145,6 +145,6 @@ typedef void hapi_clock_init_t(void);
>  
>  /* ----------- end of HAB API updates ------------*/
>  
> -uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size);
> +int authenticate_image(uint32_t ddr_start, uint32_t image_size);
>  
>  #endif
> diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
> index 02c7ae4..09892a6 100644
> --- a/arch/arm/mach-imx/hab.c
> +++ b/arch/arm/mach-imx/hab.c
> @@ -410,7 +410,7 @@ static bool is_hab_enabled(void)
>  	return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
>  }
>  
> -uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size)
> +int authenticate_image(uint32_t ddr_start, uint32_t image_size)
>  {
>  	uint32_t load_addr = 0;
>  	size_t bytes;
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
                   ` (24 preceding siblings ...)
  2018-01-12 12:40 ` [U-Boot] [PATCH v6 25/25] arm: imx: hab: Add hab_failsafe console command Bryan O'Donoghue
@ 2018-02-08 12:43 ` Jagan Teki
  2018-02-08 14:39   ` Bryan O'Donoghue
  2018-02-08 15:05   ` Breno Matheus Lima
  25 siblings, 2 replies; 34+ messages in thread
From: Jagan Teki @ 2018-02-08 12:43 UTC (permalink / raw)
  To: u-boot

On Fri, Jan 12, 2018 at 6:09 PM, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
> v6:
> - Added patch 21/25 return zero on open (unlocked) board when
>   calling authenticate_image() - Breno
>
> - Added Tested-by: Breno Matheus Lima <brenomatheus@gmail.com>
>   as indicated for remainder 24/25 patches
>
> - Added Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>   as indicated for remainder 24/25 patches
>
> v5:
> - Drop dcache disable across HAB call.
>   We can't replicate this error on the current codebase and the available
>   images. We'll have to wait for the error to crop up again before pushing
>   that patch any further.
>
> v4:
> - No change mixed extra patches @ v3 unnoticed with previous
>   git-send
>
> v3:
> - Only call into ROM if headers are verified. - Bryan
>
> - Print HAB event log if and only if a call was made to HAB
>   and a meaningful status code has been obtained. - Breno
>
> v2:
> - Fix compilation warnings and errors in SPL highlighted by
>   Breno Matheus Lima
>
> - Add CC: Breno Matheus Lima <brenomatheus@gmail.com> to all patches
>
> v1:
> This patchset updates the i.MX HAB layer in u-boot to fix a list of
> identified issues and then to add and extend existing functionality.
>
> The first block of patches 0001-0006 deal with fixing existing code,
>
> - Fixes indentation
> - Fixes the treatment of input parameters to hab_auth_image.
>
> The second block of patches 0007-0013 are about tidying up the HAB code
>
> - Remove reliance on hard-coding to specific offsets
> - IVT header drives locating CSF
> - Continue to support existing boards
>
> Patches 0014 onwards extend out the HAB functionality.
>
> - hab_rvt_check_target is a recommended check in the NXP documents to
>   perform prior to hab_rvt_authenticate_image
> - hab_rvt_failsafe is a useful function to set the board into BootROM
>   USB recovery mode.
>
>
>
> Bryan O'Donoghue (25):
>   arm: imx: hab: Make authenticate_image return int
>   arm: imx: hab: Fix authenticate_image result code
>   arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail
>   arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail
>   arm: imx: hab: Move IVT_SIZE to hab.h
>   arm: imx: hab: Move CSF_PAD_SIZE to hab.h
>   arm: imx: hab: Fix authenticate_image input parameters
>   arm: imx: hab: Add IVT header definitions
>   arm: imx: hab: Add IVT header verification
>   arm: imx: hab: Verify IVT self matches calculated address
>   arm: imx: hab: Only call ROM once headers are verified
>   arm: imx: hab: Print CSF based on IVT descriptor
>   arm: imx: hab: Print additional IVT elements during debug
>   arm: imx: hab: Define rvt_check_target()
>   arm: imx: hab: Implement hab_rvt_check_target
>   arm: imx: hab: Add a hab_rvt_check_target to image auth
>   arm: imx: hab: Print HAB event log only after calling ROM
>   arm: imx: hab: Make internal functions and data static
>   arm: imx: hab: Prefix authenticate_image with imx_hab
>   arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled
>   arm: imx: hab: Make authenticate_image() return zero on open boards
>   arm: imx: hab: Make imx_hab_is_enabled global
>   arm: imx: hab: Define rvt_failsafe()
>   arm: imx: hab: Implement hab_rvt_failsafe
>   arm: imx: hab: Add hab_failsafe console command
>
>  arch/arm/include/asm/mach-imx/hab.h |  46 +++-
>  arch/arm/mach-imx/hab.c             | 461 +++++++++++++++++++++---------------
>  arch/arm/mach-imx/spl.c             |  38 ++-
>  3 files changed, 354 insertions(+), 191 deletions(-)

I tried Secure boot before[1] with SPL and U-Boot proper and work well.

I'm observing authentication issue while loading U-Boot proper, U-Boot
proper now have features like SPL DM and SPL FIT etc

U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
Trying to boot from MMC1
Expected Linux image is not found. Trying to start U-boot

Authenticate image from DDR location 0x17800000...
bad magic magic=0xb8 length=0x841b version=0x17
bad length magic=0xb8 length=0x841b version=0x17
bad version magic=0xb8 length=0x841b version=0x17
spl: ERROR:  image authentication unsuccessful
### ERROR ### Please RESET the board ###

Please let me know where I missed, I'm authenticating SPL and
u-boot-dtb.img now.

[1] https://openedev.amarulasolutions.com/display/ODUBOOT/SPL+HABv4

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-02-08 12:43 ` [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Jagan Teki
@ 2018-02-08 14:39   ` Bryan O'Donoghue
  2018-02-08 15:05   ` Breno Matheus Lima
  1 sibling, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-02-08 14:39 UTC (permalink / raw)
  To: u-boot



On 08/02/18 12:43, Jagan Teki wrote:
> On Fri, Jan 12, 2018 at 6:09 PM, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>> v6:
>> - Added patch 21/25 return zero on open (unlocked) board when
>>    calling authenticate_image() - Breno
>>
>> - Added Tested-by: Breno Matheus Lima <brenomatheus@gmail.com>
>>    as indicated for remainder 24/25 patches
>>
>> - Added Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>>    as indicated for remainder 24/25 patches
>>
>> v5:
>> - Drop dcache disable across HAB call.
>>    We can't replicate this error on the current codebase and the available
>>    images. We'll have to wait for the error to crop up again before pushing
>>    that patch any further.
>>
>> v4:
>> - No change mixed extra patches @ v3 unnoticed with previous
>>    git-send
>>
>> v3:
>> - Only call into ROM if headers are verified. - Bryan
>>
>> - Print HAB event log if and only if a call was made to HAB
>>    and a meaningful status code has been obtained. - Breno
>>
>> v2:
>> - Fix compilation warnings and errors in SPL highlighted by
>>    Breno Matheus Lima
>>
>> - Add CC: Breno Matheus Lima <brenomatheus@gmail.com> to all patches
>>
>> v1:
>> This patchset updates the i.MX HAB layer in u-boot to fix a list of
>> identified issues and then to add and extend existing functionality.
>>
>> The first block of patches 0001-0006 deal with fixing existing code,
>>
>> - Fixes indentation
>> - Fixes the treatment of input parameters to hab_auth_image.
>>
>> The second block of patches 0007-0013 are about tidying up the HAB code
>>
>> - Remove reliance on hard-coding to specific offsets
>> - IVT header drives locating CSF
>> - Continue to support existing boards
>>
>> Patches 0014 onwards extend out the HAB functionality.
>>
>> - hab_rvt_check_target is a recommended check in the NXP documents to
>>    perform prior to hab_rvt_authenticate_image
>> - hab_rvt_failsafe is a useful function to set the board into BootROM
>>    USB recovery mode.
>>
>>
>>
>> Bryan O'Donoghue (25):
>>    arm: imx: hab: Make authenticate_image return int
>>    arm: imx: hab: Fix authenticate_image result code
>>    arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail
>>    arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail
>>    arm: imx: hab: Move IVT_SIZE to hab.h
>>    arm: imx: hab: Move CSF_PAD_SIZE to hab.h
>>    arm: imx: hab: Fix authenticate_image input parameters
>>    arm: imx: hab: Add IVT header definitions
>>    arm: imx: hab: Add IVT header verification
>>    arm: imx: hab: Verify IVT self matches calculated address
>>    arm: imx: hab: Only call ROM once headers are verified
>>    arm: imx: hab: Print CSF based on IVT descriptor
>>    arm: imx: hab: Print additional IVT elements during debug
>>    arm: imx: hab: Define rvt_check_target()
>>    arm: imx: hab: Implement hab_rvt_check_target
>>    arm: imx: hab: Add a hab_rvt_check_target to image auth
>>    arm: imx: hab: Print HAB event log only after calling ROM
>>    arm: imx: hab: Make internal functions and data static
>>    arm: imx: hab: Prefix authenticate_image with imx_hab
>>    arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled
>>    arm: imx: hab: Make authenticate_image() return zero on open boards
>>    arm: imx: hab: Make imx_hab_is_enabled global
>>    arm: imx: hab: Define rvt_failsafe()
>>    arm: imx: hab: Implement hab_rvt_failsafe
>>    arm: imx: hab: Add hab_failsafe console command
>>
>>   arch/arm/include/asm/mach-imx/hab.h |  46 +++-
>>   arch/arm/mach-imx/hab.c             | 461 +++++++++++++++++++++---------------
>>   arch/arm/mach-imx/spl.c             |  38 ++-
>>   3 files changed, 354 insertions(+), 191 deletions(-)
> 
> I tried Secure boot before[1] with SPL and U-Boot proper and work well.
> 
> I'm observing authentication issue while loading U-Boot proper, U-Boot
> proper now have features like SPL DM and SPL FIT etc
> 
> U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
> Trying to boot from MMC1
> Expected Linux image is not found. Trying to start U-boot
> 
> Authenticate image from DDR location 0x17800000...
> bad magic magic=0xb8 length=0x841b version=0x17
> bad length magic=0xb8 length=0x841b version=0x17
> bad version magic=0xb8 length=0x841b version=0x17
> spl: ERROR:  image authentication unsuccessful
> ### ERROR ### Please RESET the board ###
> 
> Please let me know where I missed, I'm authenticating SPL and
> u-boot-dtb.img now.

Can you send

1. The load address of the binary
2. The command you are using for authenticate image ?

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-02-08 12:43 ` [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Jagan Teki
  2018-02-08 14:39   ` Bryan O'Donoghue
@ 2018-02-08 15:05   ` Breno Matheus Lima
  2018-02-08 16:17     ` Bryan O'Donoghue
  1 sibling, 1 reply; 34+ messages in thread
From: Breno Matheus Lima @ 2018-02-08 15:05 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

2018-02-08 10:43 GMT-02:00 Jagan Teki <jagan@amarulasolutions.com>:
> On Fri, Jan 12, 2018 at 6:09 PM, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>> v6:
>> - Added patch 21/25 return zero on open (unlocked) board when
>>   calling authenticate_image() - Breno
>>
>> - Added Tested-by: Breno Matheus Lima <brenomatheus@gmail.com>
>>   as indicated for remainder 24/25 patches
>>
>> - Added Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>>   as indicated for remainder 24/25 patches
>>
>> v5:
>> - Drop dcache disable across HAB call.
>>   We can't replicate this error on the current codebase and the available
>>   images. We'll have to wait for the error to crop up again before pushing
>>   that patch any further.
>>
>> v4:
>> - No change mixed extra patches @ v3 unnoticed with previous
>>   git-send
>>
>> v3:
>> - Only call into ROM if headers are verified. - Bryan
>>
>> - Print HAB event log if and only if a call was made to HAB
>>   and a meaningful status code has been obtained. - Breno
>>
>> v2:
>> - Fix compilation warnings and errors in SPL highlighted by
>>   Breno Matheus Lima
>>
>> - Add CC: Breno Matheus Lima <brenomatheus@gmail.com> to all patches
>>
>> v1:
>> This patchset updates the i.MX HAB layer in u-boot to fix a list of
>> identified issues and then to add and extend existing functionality.
>>
>> The first block of patches 0001-0006 deal with fixing existing code,
>>
>> - Fixes indentation
>> - Fixes the treatment of input parameters to hab_auth_image.
>>
>> The second block of patches 0007-0013 are about tidying up the HAB code
>>
>> - Remove reliance on hard-coding to specific offsets
>> - IVT header drives locating CSF
>> - Continue to support existing boards
>>
>> Patches 0014 onwards extend out the HAB functionality.
>>
>> - hab_rvt_check_target is a recommended check in the NXP documents to
>>   perform prior to hab_rvt_authenticate_image
>> - hab_rvt_failsafe is a useful function to set the board into BootROM
>>   USB recovery mode.
>>
>>
>>
>> Bryan O'Donoghue (25):
>>   arm: imx: hab: Make authenticate_image return int
>>   arm: imx: hab: Fix authenticate_image result code
>>   arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail
>>   arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail
>>   arm: imx: hab: Move IVT_SIZE to hab.h
>>   arm: imx: hab: Move CSF_PAD_SIZE to hab.h
>>   arm: imx: hab: Fix authenticate_image input parameters
>>   arm: imx: hab: Add IVT header definitions
>>   arm: imx: hab: Add IVT header verification
>>   arm: imx: hab: Verify IVT self matches calculated address
>>   arm: imx: hab: Only call ROM once headers are verified
>>   arm: imx: hab: Print CSF based on IVT descriptor
>>   arm: imx: hab: Print additional IVT elements during debug
>>   arm: imx: hab: Define rvt_check_target()
>>   arm: imx: hab: Implement hab_rvt_check_target
>>   arm: imx: hab: Add a hab_rvt_check_target to image auth
>>   arm: imx: hab: Print HAB event log only after calling ROM
>>   arm: imx: hab: Make internal functions and data static
>>   arm: imx: hab: Prefix authenticate_image with imx_hab
>>   arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled
>>   arm: imx: hab: Make authenticate_image() return zero on open boards
>>   arm: imx: hab: Make imx_hab_is_enabled global
>>   arm: imx: hab: Define rvt_failsafe()
>>   arm: imx: hab: Implement hab_rvt_failsafe
>>   arm: imx: hab: Add hab_failsafe console command
>>
>>  arch/arm/include/asm/mach-imx/hab.h |  46 +++-
>>  arch/arm/mach-imx/hab.c             | 461 +++++++++++++++++++++---------------
>>  arch/arm/mach-imx/spl.c             |  38 ++-
>>  3 files changed, 354 insertions(+), 191 deletions(-)
>
> I tried Secure boot before[1] with SPL and U-Boot proper and work well.
>
> I'm observing authentication issue while loading U-Boot proper, U-Boot
> proper now have features like SPL DM and SPL FIT etc
>
> U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
> Trying to boot from MMC1
> Expected Linux image is not found. Trying to start U-boot
>
> Authenticate image from DDR location 0x17800000...
> bad magic magic=0xb8 length=0x841b version=0x17
> bad length magic=0xb8 length=0x841b version=0x17
> bad version magic=0xb8 length=0x841b version=0x17
> spl: ERROR:  image authentication unsuccessful
> ### ERROR ### Please RESET the board ###
>
> Please let me know where I missed, I'm authenticating SPL and
> u-boot-dtb.img now.

Can you please check if the generated u-boot-dtb.img contains a IVT
table appended in the end of the image?

The mx6slevk_spl_defconfig target also generates SPL + u-boot-dtb.img
but I have to use the u-boot-ivt.img binary instead. In my case
u-boot-dtb.img does not includes a IVT table.

Best Regards,
Breno Lima

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-02-08 15:05   ` Breno Matheus Lima
@ 2018-02-08 16:17     ` Bryan O'Donoghue
  2018-02-09  7:27       ` Jagan Teki
  0 siblings, 1 reply; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-02-08 16:17 UTC (permalink / raw)
  To: u-boot


>>
>> I'm observing authentication issue while loading U-Boot proper, U-Boot
>> proper now have features like SPL DM and SPL FIT etc
>>
>> U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
>> Trying to boot from MMC1
>> Expected Linux image is not found. Trying to start U-boot
>>
>> Authenticate image from DDR location 0x17800000...
>> bad magic magic=0xb8 length=0x841b version=0x17
>> bad length magic=0xb8 length=0x841b version=0x17
>> bad version magic=0xb8 length=0x841b version=0x17
>> spl: ERROR:  image authentication unsuccessful
>> ### ERROR ### Please RESET the board ###
>>
>> Please let me know where I missed, I'm authenticating SPL and
>> u-boot-dtb.img now.
> 
> Can you please check if the generated u-boot-dtb.img contains a IVT
> table appended in the end of the image?
> 
> The mx6slevk_spl_defconfig target also generates SPL + u-boot-dtb.img
> but I have to use the u-boot-ivt.img binary instead. In my case
> u-boot-dtb.img does not includes a IVT table.
> 
> Best Regards,
> Breno Lima
> 

At a guess I'd say it's the fix we did for hab_auth_img - I guess Jagan 
you have an out-of-tree implementation here ?

If you have a command in your environment that looks like this

hab_auth_img 0x17800000 0x10000

that should now be

hab_auth_img 0x17800000 0x10000 0xF400

assuming the CSF footer is aprox 0xC00 bytes padded.

git show c5800b2

arm: imx: hab: Fix authenticate_image input parameters

1: Adding a new parameter to hab_auth_img
        - addr   : image hex address
        - length : total length of the image
        - offset : offset of IVT from addr

---
bod

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-02-08 16:17     ` Bryan O'Donoghue
@ 2018-02-09  7:27       ` Jagan Teki
  2018-02-09 10:01         ` Bryan O'Donoghue
  0 siblings, 1 reply; 34+ messages in thread
From: Jagan Teki @ 2018-02-09  7:27 UTC (permalink / raw)
  To: u-boot

On Thu, Feb 8, 2018 at 9:47 PM, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
>>>
>>> I'm observing authentication issue while loading U-Boot proper, U-Boot
>>> proper now have features like SPL DM and SPL FIT etc
>>>
>>> U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
>>> Trying to boot from MMC1
>>> Expected Linux image is not found. Trying to start U-boot
>>>
>>> Authenticate image from DDR location 0x17800000...
>>> bad magic magic=0xb8 length=0x841b version=0x17
>>> bad length magic=0xb8 length=0x841b version=0x17
>>> bad version magic=0xb8 length=0x841b version=0x17
>>> spl: ERROR:  image authentication unsuccessful
>>> ### ERROR ### Please RESET the board ###
>>>
>>> Please let me know where I missed, I'm authenticating SPL and
>>> u-boot-dtb.img now.
>>
>>
>> Can you please check if the generated u-boot-dtb.img contains a IVT
>> table appended in the end of the image?
>>
>> The mx6slevk_spl_defconfig target also generates SPL + u-boot-dtb.img
>> but I have to use the u-boot-ivt.img binary instead. In my case
>> u-boot-dtb.img does not includes a IVT table.
>>
>> Best Regards,
>> Breno Lima
>>
>
> At a guess I'd say it's the fix we did for hab_auth_img - I guess Jagan you
> have an out-of-tree implementation here ?

Basically I'm trying to compare this with implementation before, look
like issue is IVT image signature is missing for when
CONFIG_SPL_LOAD_FIT defined.  It's working without SPL_LOAD_FIT.

>
> If you have a command in your environment that looks like this
>
> hab_auth_img 0x17800000 0x10000
>
> that should now be
>
> hab_auth_img 0x17800000 0x10000 0xF400
>
> assuming the CSF footer is aprox 0xC00 bytes padded.
>
> git show c5800b2
>
> arm: imx: hab: Fix authenticate_image input parameters
>
> 1: Adding a new parameter to hab_auth_img
>        - addr   : image hex address
>        - length : total length of the image
>        - offset : offset of IVT from addr
>

I've created u-boot-ivt.image which we did in previous releases[2] and
padded 0x2000 to CSF to align the size of CONFIG_CSF_SIZE

Image Name:   U-Boot 2018.03-rc1-00182-gb81f7c
Created:      Fri Feb  9 11:00:05 2018
Image Type:   ARM U-Boot Firmware with HABv4 IVT (uncompressed)
Data Size:    360384 Bytes = 351.94 KiB = 0.34 MiB
Load Address: 17800000
Entry Point:  00000000
HAB Blocks:   0x177fffc0   0x0000   0x00056020

icorem6qdl-rqs> hab_auth_img 0x177fffc0 0x58020 0x56020

Authenticate image from DDR location 0x177fffc0...
bad magic magic=0xd4 length=0x5000 version=0x41
bad length magic=0xd4 length=0x5000 version=0x41

[2] https://openedev.amarulasolutions.com/display/ODUBOOT/i.MX6+HABv4#i.MX6HABv4-SignedBoot-Usage

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

* [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer
  2018-02-09  7:27       ` Jagan Teki
@ 2018-02-09 10:01         ` Bryan O'Donoghue
  0 siblings, 0 replies; 34+ messages in thread
From: Bryan O'Donoghue @ 2018-02-09 10:01 UTC (permalink / raw)
  To: u-boot



On 09/02/18 07:27, Jagan Teki wrote:
> On Thu, Feb 8, 2018 at 9:47 PM, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>>>>
>>>> I'm observing authentication issue while loading U-Boot proper, U-Boot
>>>> proper now have features like SPL DM and SPL FIT etc
>>>>
>>>> U-Boot SPL 2018.03-rc1-00182-gb81f7c9 (Feb 08 2018 - 17:19:03 +0530)
>>>> Trying to boot from MMC1
>>>> Expected Linux image is not found. Trying to start U-boot
>>>>
>>>> Authenticate image from DDR location 0x17800000...
>>>> bad magic magic=0xb8 length=0x841b version=0x17
>>>> bad length magic=0xb8 length=0x841b version=0x17
>>>> bad version magic=0xb8 length=0x841b version=0x17
>>>> spl: ERROR:  image authentication unsuccessful
>>>> ### ERROR ### Please RESET the board ###
>>>>
>>>> Please let me know where I missed, I'm authenticating SPL and
>>>> u-boot-dtb.img now.
>>>
>>>
>>> Can you please check if the generated u-boot-dtb.img contains a IVT
>>> table appended in the end of the image?
>>>
>>> The mx6slevk_spl_defconfig target also generates SPL + u-boot-dtb.img
>>> but I have to use the u-boot-ivt.img binary instead. In my case
>>> u-boot-dtb.img does not includes a IVT table.
>>>
>>> Best Regards,
>>> Breno Lima
>>>
>>
>> At a guess I'd say it's the fix we did for hab_auth_img - I guess Jagan you
>> have an out-of-tree implementation here ?
> 
> Basically I'm trying to compare this with implementation before, look
> like issue is IVT image signature is missing for when
> CONFIG_SPL_LOAD_FIT defined.  It's working without SPL_LOAD_FIT.
> 
>>
>> If you have a command in your environment that looks like this
>>
>> hab_auth_img 0x17800000 0x10000
>>
>> that should now be
>>
>> hab_auth_img 0x17800000 0x10000 0xF400
>>
>> assuming the CSF footer is aprox 0xC00 bytes padded.
>>
>> git show c5800b2
>>
>> arm: imx: hab: Fix authenticate_image input parameters
>>
>> 1: Adding a new parameter to hab_auth_img
>>         - addr   : image hex address
>>         - length : total length of the image
>>         - offset : offset of IVT from addr
>>
> 
> I've created u-boot-ivt.image which we did in previous releases[2] and
> padded 0x2000 to CSF to align the size of CONFIG_CSF_SIZE
> 
> Image Name:   U-Boot 2018.03-rc1-00182-gb81f7c
> Created:      Fri Feb  9 11:00:05 2018
> Image Type:   ARM U-Boot Firmware with HABv4 IVT (uncompressed)
> Data Size:    360384 Bytes = 351.94 KiB = 0.34 MiB
> Load Address: 17800000
> Entry Point:  00000000
> HAB Blocks:   0x177fffc0   0x0000   0x00056020
> 
> icorem6qdl-rqs> hab_auth_img 0x177fffc0 0x58020 0x56020
> 
> Authenticate image from DDR location 0x177fffc0...
> bad magic magic=0xd4 length=0x5000 version=0x41
> bad length magic=0xd4 length=0x5000 version=0x41
> 
> [2] https://openedev.amarulasolutions.com/display/ODUBOOT/i.MX6+HABv4#i.MX6HABv4-SignedBoot-Usage
> 

Ah... is that diagram accurate ?

You are perpending the IVT to your image header

In which case your command should be

icorem6qdl-rqs> hab_auth_img 0x177fffc0 0x58020 0

Incidentially you are pointed at the CSF there not the IVT.

High Assurance Boot Version 4 Application Programming Interface 
Reference Manual section 6.2

tag = 0xD4 => CSF
tag = 0xD1 => IVT

---
bod

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

end of thread, other threads:[~2018-02-09 10:01 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-12 12:39 [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Bryan O'Donoghue
2018-01-12 12:39 ` [U-Boot] [PATCH v6 01/25] arm: imx: hab: Make authenticate_image return int Bryan O'Donoghue
2018-01-14 16:44   ` Stefano Babic
2018-01-12 12:39 ` [U-Boot] [PATCH v6 02/25] arm: imx: hab: Fix authenticate_image result code Bryan O'Donoghue
2018-01-12 12:39 ` [U-Boot] [PATCH v6 03/25] arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail Bryan O'Donoghue
2018-01-12 12:39 ` [U-Boot] [PATCH v6 04/25] arm: imx: hab: Optimise flow of authenticate_image on hab_entry fail Bryan O'Donoghue
2018-01-12 12:39 ` [U-Boot] [PATCH v6 05/25] arm: imx: hab: Move IVT_SIZE to hab.h Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 06/25] arm: imx: hab: Move CSF_PAD_SIZE " Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 07/25] arm: imx: hab: Fix authenticate_image input parameters Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 08/25] arm: imx: hab: Add IVT header definitions Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 09/25] arm: imx: hab: Add IVT header verification Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 10/25] arm: imx: hab: Verify IVT self matches calculated address Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 11/25] arm: imx: hab: Only call ROM once headers are verified Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 12/25] arm: imx: hab: Print CSF based on IVT descriptor Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 13/25] arm: imx: hab: Print additional IVT elements during debug Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 14/25] arm: imx: hab: Define rvt_check_target() Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 15/25] arm: imx: hab: Implement hab_rvt_check_target Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 16/25] arm: imx: hab: Add a hab_rvt_check_target to image auth Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 17/25] arm: imx: hab: Print HAB event log only after calling ROM Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 18/25] arm: imx: hab: Make internal functions and data static Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 19/25] arm: imx: hab: Prefix authenticate_image with imx_hab Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 20/25] arm: imx: hab: Rename is_hab_enabled imx_hab_is_enabled Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 21/25] arm: imx: hab: Make authenticate_image() return zero on open boards Bryan O'Donoghue
2018-01-12 15:37   ` Breno Matheus Lima
2018-01-12 12:40 ` [U-Boot] [PATCH v6 22/25] arm: imx: hab: Make imx_hab_is_enabled global Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 23/25] arm: imx: hab: Define rvt_failsafe() Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 24/25] arm: imx: hab: Implement hab_rvt_failsafe Bryan O'Donoghue
2018-01-12 12:40 ` [U-Boot] [PATCH v6 25/25] arm: imx: hab: Add hab_failsafe console command Bryan O'Donoghue
2018-02-08 12:43 ` [U-Boot] [PATCH v6 00/25] Fix and extend i.MX HAB layer Jagan Teki
2018-02-08 14:39   ` Bryan O'Donoghue
2018-02-08 15:05   ` Breno Matheus Lima
2018-02-08 16:17     ` Bryan O'Donoghue
2018-02-09  7:27       ` Jagan Teki
2018-02-09 10:01         ` Bryan O'Donoghue

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