[U-Boot] [PATCH v2 0/4] rockchip: rk3399: Update SPL generation for the RK3399

[U-Boot] [PATCH v2 0/4] rockchip: rk3399: Update SPL generation for the RK3399

[Philipp Tomsich]

For the RK3399, which boots in AArch64 mode, the Boot ROM still starts
execution at the instruction following the SPL boot-magic
(i.e. 'RK33'). This puts the first executed instruction on an odd
address (+0x4).

However, we can't reasonably use for the start of the .text-section on
AArch64, as it would violate natural alignment... which in turn
invites trouble down the line if a linker assumes natural alignment or
some assembly snippet tries to be smart.

This series implements both approaches for dealing with this odd
alignment during SPL image creation:
* patches 1 and 2 take an SPL image which starts at (+0x8) and
  prepends it with a AArch64 'nop' during image assembly
* patches 3 and 4 change this to using a BOOT0_HOOK to insert a single
  4-byte word at the beginning of the SPL build to make space for the
  'RK33' magic and then have the mkimage-tool overwrite this word in
  the 'set_header' function.

In doing so, the rkimage code is refactored:
- to remove duplication between rksd.c and rkspi.c
- to dynamically allocate the space for headers from vrec

Changes in v2:
- Use BOOT0_HOOK to insert space into the SPL payload that can be
  overwritten with the boot magic (e.g. 'RK33') by rkimage
- Change rkimage to overwrite this padding for the RK3399 instead of
  inserting an artificial 'nop'

Philipp Tomsich (4):
  rockchip: mkimage: simplify start/size calculation for rc4_encode
  rockchip: mkimage: pad the header to 8-bytes (using a 'nop') for
    RK3399
  rockchip: spl: RK3399: use boot0 hook to create space for SPL magic
  rockchip: mkimage: update rkimage to support pre-padded payloads

 arch/arm/include/asm/arch-rockchip/boot0.h | 18 ++++++
 arch/arm/mach-rockchip/Kconfig             |  1 +
 include/configs/rk3399_common.h            |  2 +-
 tools/rkcommon.c                           | 90 ++++++++++++++++++++++++++----
 tools/rkcommon.h                           | 10 ++++
 tools/rksd.c                               | 17 ++----
 tools/rkspi.c                              | 17 ++----
 7 files changed, 119 insertions(+), 36 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-rockchip/boot0.h

-- 
1.9.1

[U-Boot] [PATCH v2 1/4] rockchip: mkimage: simplify start/size calculation for rc4_encode

[Philipp Tomsich]

The RC4 encoding works on full blocks, but the calculation of the
starting offset and size are needlessly complicated by using a
reference value known to be offset into a block by the size of the
header and then correcting for the (hard-coded) size of the header
(i.e. 4 bytes).

We change this over to use the RK_SPL_HDR_START directly (which is
known to be on a block boundary).

X-AffectedPlatforms: RK3399-Q7
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
---

Changes in v2: None

 tools/rksd.c  | 4 ++--
 tools/rkspi.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/rksd.c b/tools/rksd.c
index ff2233f..e55c522 100644
--- a/tools/rksd.c
+++ b/tools/rksd.c
@@ -43,8 +43,8 @@ static void rksd_set_header(void *buf,  struct stat *sbuf,  int ifd,
 	       RK_SPL_HDR_SIZE);
 
 	if (rkcommon_need_rc4_spl(params))
-		rkcommon_rc4_encode_spl(buf, RK_SPL_START - 4,
-					params->file_size - RK_SPL_START + 4);
+		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
+					params->file_size - RK_SPL_HDR_START);
 }
 
 static int rksd_extract_subimage(void *buf,  struct image_tool_params *params)
diff --git a/tools/rkspi.c b/tools/rkspi.c
index 0271d2e..9fa43e8 100644
--- a/tools/rkspi.c
+++ b/tools/rkspi.c
@@ -49,8 +49,8 @@ static void rkspi_set_header(void *buf, struct stat *sbuf, int ifd,
 	       RK_SPL_HDR_SIZE);
 
 	if (rkcommon_need_rc4_spl(params))
-		rkcommon_rc4_encode_spl(buf, RK_SPL_START - 4,
-					params->file_size - RK_SPL_START + 4);
+		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
+					params->file_size - RK_SPL_HDR_START);
 
 	/*
 	 * Spread the image out so we only use the first 2KB of each 4KB
-- 
1.9.1

[U-Boot] [PATCH v2 2/4] rockchip: mkimage: pad the header to 8-bytes (using a 'nop') for RK3399

[Philipp Tomsich]

The RK3399 boot code (running as AArch64) poses a bit of a challenge
for SPL image generation:
 * The BootROM will start execution right after the 4-byte header (at
   the odd instruction word loaded into SRAM at 0xff8c2004, with the
   'RK33' boot magic residing at 0xff8c2000).
 * The default padding (during ELF generation) for AArch64 is 0x0,
   which is an illegal instruction and the .text section needs to be
   naturally aligned (someone might locate a 64bit constant relative
   to the section start and unaligned loads trigger a fault for all
   privileged modes of an ARMv8)... so we can't simply define the
   CONFIG_SPL_TEXT_BASE option to the odd address (0xff8c2004).
 * Finally, we don't want to change the values used for padding of
   the SPL .text section for all ARMv8 targets to the instruction
   word encoding 'nop', as this would affect all padding in this
   section and might hide errors that would otherwise quickly trigger
   an illegal insn exception.

To deal with this situation, we modify the rkimage generation to
 - understand the fact that the RK3399 needs to pad the header to an
   8 byte boundary using an AArch64 'nop'
 - the necessary logic to adjust the header_size (which controls the
   location where the payload is copied into the image) and to insert
   this padding (AArch64 insn words are always little-endian) into
   the image following the 4-byte header magic.

X-AffectedPlatforms: RK3399-Q7
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
---

Changes in v2: None

 tools/rkcommon.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 tools/rkcommon.h | 23 ++++++++++++++
 tools/rksd.c     | 17 +++-------
 tools/rkspi.c    | 17 +++-------
 4 files changed, 118 insertions(+), 36 deletions(-)

diff --git a/tools/rkcommon.c b/tools/rkcommon.c
index 6595e02..1ea072b 100644
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -41,25 +41,36 @@ struct header0_info {
 };
 
 /**
+ * struct header1_info
+ */
+struct header1_info {
+	uint32_t magic;
+	uint32_t first_insn;
+};
+
+/**
  * struct spl_info - spl info for each chip
  *
  * @imagename:		Image name(passed by "mkimage -n")
  * @spl_hdr:		Boot ROM requires a 4-bytes spl header
  * @spl_size:		Spl size(include extra 4-bytes spl header)
  * @spl_rc4:		RC4 encode the SPL binary (same key as header)
+ * @spl_aarch64:        Pad the header with an AArch64 'nop's to 8-bytes
  */
+
 struct spl_info {
 	const char *imagename;
 	const char *spl_hdr;
 	const uint32_t spl_size;
 	const bool spl_rc4;
+	const bool spl_aarch64;
 };
 
 static struct spl_info spl_infos[] = {
-	{ "rk3036", "RK30", 0x1000, false },
-	{ "rk3188", "RK31", 0x8000 - 0x800, true },
-	{ "rk3288", "RK32", 0x8000, false },
-	{ "rk3399", "RK33", 0x20000, false },
+	{ "rk3036", "RK30", 0x1000, false, false },
+	{ "rk3188", "RK31", 0x8000 - 0x800, true, false },
+	{ "rk3288", "RK32", 0x8000, false, false },
+	{ "rk3399", "RK33", 0x20000, false, true },
 };
 
 static unsigned char rc4_key[16] = {
@@ -106,6 +117,16 @@ const char *rkcommon_get_spl_hdr(struct image_tool_params *params)
 	return info->spl_hdr;
 }
 
+const bool rkcommon_get_spl_hdr_padto8(struct image_tool_params *params)
+{
+	struct spl_info *info = rkcommon_get_spl_info(params->imagename);
+
+	/*
+	 * info would not be NULL, because of we checked params before.
+	 */
+	return info->spl_aarch64;
+}
+
 int rkcommon_get_spl_size(struct image_tool_params *params)
 {
 	struct spl_info *info = rkcommon_get_spl_info(params->imagename);
@@ -126,16 +147,12 @@ bool rkcommon_need_rc4_spl(struct image_tool_params *params)
 	return info->spl_rc4;
 }
 
-int rkcommon_set_header(void *buf, uint file_size,
-			struct image_tool_params *params)
+static void rkcommon_set_header0(void *buf, uint file_size,
+				 struct image_tool_params *params)
 {
-	struct header0_info *hdr;
+	struct header0_info *hdr = buf;
 
-	if (file_size > rkcommon_get_spl_size(params))
-		return -ENOSPC;
-
-	memset(buf,  '\0', RK_INIT_OFFSET * RK_BLK_SIZE);
-	hdr = (struct header0_info *)buf;
+	memset(buf, '\0', RK_INIT_OFFSET * RK_BLK_SIZE);
 	hdr->signature = RK_SIGNATURE;
 	hdr->disable_rc4 = !rkcommon_need_rc4_spl(params);
 	hdr->init_offset = RK_INIT_OFFSET;
@@ -145,6 +162,31 @@ int rkcommon_set_header(void *buf, uint file_size,
 	hdr->init_boot_size = hdr->init_size + RK_MAX_BOOT_SIZE / RK_BLK_SIZE;
 
 	rc4_encode(buf, RK_BLK_SIZE, rc4_key);
+}
+
+int rkcommon_set_header(void *buf, uint file_size,
+			struct image_tool_params *params)
+{
+	struct header1_info *hdr = buf + RK_SPL_HDR_START;
+
+	if (file_size > rkcommon_get_spl_size(params))
+		return -ENOSPC;
+
+	rkcommon_set_header0(buf, file_size, params);
+
+	/* Set up the SPL name and add the AArch64 'nop' padding, if needed */
+	memcpy(&hdr->magic, rkcommon_get_spl_hdr(params), RK_SPL_HDR_SIZE);
+
+	/*
+	 * Pad the 4-byte header to 8-bytes using an AArch64 'nop'.
+	 * Note that AArch64 insns are always encoded as little-endian.
+	 */
+	if (rkcommon_get_spl_hdr_padto8(params))
+		hdr->first_insn = cpu_to_le32(0xd503201f);
+
+	if (rkcommon_need_rc4_spl(params))
+		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
+					params->file_size - RK_SPL_HDR_START);
 
 	return 0;
 }
@@ -161,3 +203,34 @@ void rkcommon_rc4_encode_spl(void *buf, unsigned int offset, unsigned int size)
 		remaining -= step;
 	}
 }
+
+void rkcommon_vrec_header(struct image_tool_params *params,
+			  struct image_type_params *tparams)
+{
+	/*
+	 * The SPL image looks as follows:
+	 *
+	 * 0x0    header0 (see rkcommon.c)
+	 * 0x800  spl_name ('RK30', ..., 'RK33')
+	 * 0x804  first instruction to be executed
+	 *        (image start for AArch32, 'nop' for AArch64))
+	 * 0x808  second instruction to be executed
+	 *        (image start for AArch64)
+	 *
+	 * For AArch64 (ARMv8) payloads, we receive an input file that
+	 * needs to start on an 8-byte boundary (natural alignment), so
+	 * we need to put a NOP at 0x804.
+	 *
+	 * Depending on this, the header is either 0x804 or 0x808 bytes
+	 * in length.
+	 */
+	if (rkcommon_get_spl_hdr_padto8(params))
+		tparams->header_size = RK_SPL_HDR_START + 8;
+	else
+		tparams->header_size = RK_SPL_HDR_START + 4;
+
+	/* Allocate, clear and install the header */
+	tparams->hdr = malloc(tparams->header_size);
+	memset(tparams->hdr, 0, tparams->header_size);
+	tparams->header_size = tparams->header_size;
+}
diff --git a/tools/rkcommon.h b/tools/rkcommon.h
index b4f6f32..3d64516 100644
--- a/tools/rkcommon.h
+++ b/tools/rkcommon.h
@@ -34,6 +34,19 @@ int rkcommon_check_params(struct image_tool_params *params);
 const char *rkcommon_get_spl_hdr(struct image_tool_params *params);
 
 /**
+ * rkcommon_get_spl_hdr_padto8() - check if we need to pad to 8 bytes
+ *
+ * Rockchip's bootrom starts execution right after the SPL header (i.e.
+ * at offset 4), but we can not reasonably align the test section of
+ * an AArch64 SPL at 4 bytes (as this would break natural alignment
+ * and any embedded constants might cause an alignment exception, which
+ * is illegal in privileged modes).
+ *
+ * Padding is (for now) assumed to occur with a single AArch64 'nop'.
+ */
+const bool rkcommon_get_spl_hdr_padto8(struct image_tool_params *params);
+
+/**
  * rkcommon_get_spl_size() - get spl size for a Rockchip boot image
  *
  * Different chip may have different sram size. And if we want to jump
@@ -77,4 +90,14 @@ bool rkcommon_need_rc4_spl(struct image_tool_params *params);
  */
 void rkcommon_rc4_encode_spl(void *buf, unsigned int offset, unsigned int size);
 
+/**
+ * rkcommon_vrec_header() - allocate memory for the header
+ *
+ * @params:     Pointer to the tool params structure
+ * @tparams:    Pointer tot the image type structure (for setting
+ *              the header and header_size)
+ */
+void rkcommon_vrec_header(struct image_tool_params *params,
+			  struct image_type_params *tparams);
+
 #endif
diff --git a/tools/rksd.c b/tools/rksd.c
index e55c522..ac8a67d 100644
--- a/tools/rksd.c
+++ b/tools/rksd.c
@@ -13,8 +13,6 @@
 #include "mkimage.h"
 #include "rkcommon.h"
 
-static char dummy_hdr[RK_IMAGE_HEADER_LEN];
-
 static int rksd_verify_header(unsigned char *buf,  int size,
 				 struct image_tool_params *params)
 {
@@ -38,13 +36,6 @@ static void rksd_set_header(void *buf,  struct stat *sbuf,  int ifd,
 		printf("Warning: SPL image is too large (size %#x) and will not boot\n",
 		       size);
 	}
-
-	memcpy(buf + RK_SPL_HDR_START, rkcommon_get_spl_hdr(params),
-	       RK_SPL_HDR_SIZE);
-
-	if (rkcommon_need_rc4_spl(params))
-		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
-					params->file_size - RK_SPL_HDR_START);
 }
 
 static int rksd_extract_subimage(void *buf,  struct image_tool_params *params)
@@ -66,10 +57,12 @@ static int rksd_vrec_header(struct image_tool_params *params,
 {
 	int pad_size;
 
+	rkcommon_vrec_header(params, tparams);
+
 	pad_size = RK_SPL_HDR_START + rkcommon_get_spl_size(params);
 	debug("pad_size %x\n", pad_size);
 
-	return pad_size - params->file_size;
+	return pad_size - params->file_size - tparams->header_size;
 }
 
 /*
@@ -78,8 +71,8 @@ static int rksd_vrec_header(struct image_tool_params *params,
 U_BOOT_IMAGE_TYPE(
 	rksd,
 	"Rockchip SD Boot Image support",
-	RK_IMAGE_HEADER_LEN,
-	dummy_hdr,
+	0,
+	NULL,
 	rkcommon_check_params,
 	rksd_verify_header,
 	rksd_print_header,
diff --git a/tools/rkspi.c b/tools/rkspi.c
index 9fa43e8..d2d3fdd 100644
--- a/tools/rkspi.c
+++ b/tools/rkspi.c
@@ -17,8 +17,6 @@ enum {
 	RKSPI_SECT_LEN		= RK_BLK_SIZE * 4,
 };
 
-static char dummy_hdr[RK_IMAGE_HEADER_LEN];
-
 static int rkspi_verify_header(unsigned char *buf, int size,
 			       struct image_tool_params *params)
 {
@@ -45,13 +43,6 @@ static void rkspi_set_header(void *buf, struct stat *sbuf, int ifd,
 		       size);
 	}
 
-	memcpy(buf + RK_SPL_HDR_START, rkcommon_get_spl_hdr(params),
-	       RK_SPL_HDR_SIZE);
-
-	if (rkcommon_need_rc4_spl(params))
-		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
-					params->file_size - RK_SPL_HDR_START);
-
 	/*
 	 * Spread the image out so we only use the first 2KB of each 4KB
 	 * region. This is a feature of the SPI format required by the Rockchip
@@ -86,6 +77,8 @@ static int rkspi_vrec_header(struct image_tool_params *params,
 {
 	int pad_size;
 
+	rkcommon_vrec_header(params, tparams);
+
 	pad_size = (rkcommon_get_spl_size(params) + 0x7ff) / 0x800 * 0x800;
 	params->orig_file_size = pad_size;
 
@@ -94,7 +87,7 @@ static int rkspi_vrec_header(struct image_tool_params *params,
 	pad_size += RK_SPL_HDR_START;
 	debug("pad_size %x\n", pad_size);
 
-	return pad_size - params->file_size;
+	return pad_size - params->file_size - tparams->header_size;
 }
 
 /*
@@ -103,8 +96,8 @@ static int rkspi_vrec_header(struct image_tool_params *params,
 U_BOOT_IMAGE_TYPE(
 	rkspi,
 	"Rockchip SPI Boot Image support",
-	RK_IMAGE_HEADER_LEN,
-	dummy_hdr,
+	0,
+	NULL,
 	rkcommon_check_params,
 	rkspi_verify_header,
 	rkspi_print_header,
-- 
1.9.1

[U-Boot] [PATCH v2 3/4] rockchip: spl: RK3399: use boot0 hook to create space for SPL magic

[Philipp Tomsich]

The SPL binary needs to be prefixed with the boot magic ('RK33' for
the RK3399) on the Rockchip platform and starts execution of the
instruction word following immediately after this boot magic.

This poses a challenge for AArch64 (ARMv8) binaries, as the .text
section would need to start on the odd address, violating natural
alignment (and potentially triggering a fault for any code that
tries to access 64bit values embedded in the .text section).

A quick and easy fix is to have the .text section include the 'RK33'
magic and pad it with a boot0 hook to insert 4 bytes of padding at the
start of the section (with the intention of having mkimage overwrite
this padding with the appropriate boot magic). This avoids having to
modify the linker scripts or more complex logic in mkimage.

X-AffectedPlatforms: RK3399-Q7
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>

---

Changes in v2:
- Use BOOT0_HOOK to insert space into the SPL payload that can be
  overwritten with the boot magic (e.g. 'RK33') by rkimage

 arch/arm/include/asm/arch-rockchip/boot0.h | 18 ++++++++++++++++++
 arch/arm/mach-rockchip/Kconfig             |  1 +
 include/configs/rk3399_common.h            |  2 +-
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/asm/arch-rockchip/boot0.h

diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
new file mode 100644
index 0000000..8d7bc9a
--- /dev/null
+++ b/arch/arm/include/asm/arch-rockchip/boot0.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2017 Theobroma Systems Design und Consulting GmbH
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/*
+ * Execution starts on the instruction following this 4-byte header
+ * (containing the magic 'RK33').
+ *
+ * To make life easier for everyone, we build the SPL binary with
+ * space for this 4-byte header already included in the binary.
+ */
+
+#ifdef CONFIG_SPL_BUILD
+	.space 0x4         /* space for the 'RK33' */
+#endif
+	b reset
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index bf8e6be..af0796d 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -54,6 +54,7 @@ config ROCKCHIP_RK3399
 	select SUPPORT_SPL
 	select SPL
 	select SPL_SEPARATE_BSS
+	select ENABLE_ARM_SOC_BOOT0_HOOK
 	help
 	  The Rockchip RK3399 is a ARM-based SoC with a dual-core Cortex-A72
 	  and quad-core Cortex-A53.
diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
index eb38376..bc91eb6 100644
--- a/include/configs/rk3399_common.h
+++ b/include/configs/rk3399_common.h
@@ -28,7 +28,7 @@
 #define CONFIG_SYS_INIT_SP_ADDR		0x00300000
 #define CONFIG_SYS_LOAD_ADDR		0x00800800
 #define CONFIG_SPL_STACK		0xff8effff
-#define CONFIG_SPL_TEXT_BASE		0xff8c2008
+#define CONFIG_SPL_TEXT_BASE		0xff8c2000
 #define CONFIG_SPL_MAX_SIZE		0x30000
 /*  BSS setup */
 #define CONFIG_SPL_BSS_START_ADDR       0xff8e0000
-- 
1.9.1

[U-Boot] [PATCH v2 4/4] rockchip: mkimage: update rkimage to support pre-padded payloads

[Philipp Tomsich]

To simplify the creation of AArch64 SPL images for the RK3399, we
use the ENABLE_ARM_SOC_BOOT0_HOOK option and prepend 4 bytes of
padding at the start of the text section. This makes it easy for
mkimage to rewrite this word with the 'RK33' boot magic.

This change brings logic to calculate the header size and allocate
the header back in sync. For the RK3399 we now limit the header to
before the payload (i.e. the 'header0' and the padding up to the
actual image) and overwrite the first word (inserted by the
boot0-hook for this purpose) with the 'RK33' magic in-place.

X-AffectedPlatforms: RK3399-Q7
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>

---

Changes in v2:
- Change rkimage to overwrite this padding for the RK3399 instead of
  inserting an artificial 'nop'

 tools/rkcommon.c | 31 +++++++++++++------------------
 tools/rkcommon.h | 13 -------------
 2 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/tools/rkcommon.c b/tools/rkcommon.c
index 1ea072b..6cdb749 100644
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -45,7 +45,6 @@ struct header0_info {
  */
 struct header1_info {
 	uint32_t magic;
-	uint32_t first_insn;
 };
 
 /**
@@ -55,7 +54,9 @@ struct header1_info {
  * @spl_hdr:		Boot ROM requires a 4-bytes spl header
  * @spl_size:		Spl size(include extra 4-bytes spl header)
  * @spl_rc4:		RC4 encode the SPL binary (same key as header)
- * @spl_aarch64:        Pad the header with an AArch64 'nop's to 8-bytes
+ * @spl_boot0:          A new-style (ARM_SOC_BOOT0_HOOK) image that should
+ *                      have the boot magic (e.g. 'RK33') written to its first
+ *                      word.
  */
 
 struct spl_info {
@@ -63,7 +64,7 @@ struct spl_info {
 	const char *spl_hdr;
 	const uint32_t spl_size;
 	const bool spl_rc4;
-	const bool spl_aarch64;
+	const bool spl_boot0;
 };
 
 static struct spl_info spl_infos[] = {
@@ -117,34 +118,35 @@ const char *rkcommon_get_spl_hdr(struct image_tool_params *params)
 	return info->spl_hdr;
 }
 
-const bool rkcommon_get_spl_hdr_padto8(struct image_tool_params *params)
+
+int rkcommon_get_spl_size(struct image_tool_params *params)
 {
 	struct spl_info *info = rkcommon_get_spl_info(params->imagename);
 
 	/*
 	 * info would not be NULL, because of we checked params before.
 	 */
-	return info->spl_aarch64;
+	return info->spl_size;
 }
 
-int rkcommon_get_spl_size(struct image_tool_params *params)
+bool rkcommon_need_rc4_spl(struct image_tool_params *params)
 {
 	struct spl_info *info = rkcommon_get_spl_info(params->imagename);
 
 	/*
 	 * info would not be NULL, because of we checked params before.
 	 */
-	return info->spl_size;
+	return info->spl_rc4;
 }
 
-bool rkcommon_need_rc4_spl(struct image_tool_params *params)
+bool rkcommon_spl_is_boot0(struct image_tool_params *params)
 {
 	struct spl_info *info = rkcommon_get_spl_info(params->imagename);
 
 	/*
 	 * info would not be NULL, because of we checked params before.
 	 */
-	return info->spl_rc4;
+	return info->spl_boot0;
 }
 
 static void rkcommon_set_header0(void *buf, uint file_size,
@@ -177,13 +179,6 @@ int rkcommon_set_header(void *buf, uint file_size,
 	/* Set up the SPL name and add the AArch64 'nop' padding, if needed */
 	memcpy(&hdr->magic, rkcommon_get_spl_hdr(params), RK_SPL_HDR_SIZE);
 
-	/*
-	 * Pad the 4-byte header to 8-bytes using an AArch64 'nop'.
-	 * Note that AArch64 insns are always encoded as little-endian.
-	 */
-	if (rkcommon_get_spl_hdr_padto8(params))
-		hdr->first_insn = cpu_to_le32(0xd503201f);
-
 	if (rkcommon_need_rc4_spl(params))
 		rkcommon_rc4_encode_spl(buf, RK_SPL_HDR_START,
 					params->file_size - RK_SPL_HDR_START);
@@ -224,8 +219,8 @@ void rkcommon_vrec_header(struct image_tool_params *params,
 	 * Depending on this, the header is either 0x804 or 0x808 bytes
 	 * in length.
 	 */
-	if (rkcommon_get_spl_hdr_padto8(params))
-		tparams->header_size = RK_SPL_HDR_START + 8;
+	if (rkcommon_spl_is_boot0(params))
+		tparams->header_size = RK_SPL_HDR_START;
 	else
 		tparams->header_size = RK_SPL_HDR_START + 4;
 
diff --git a/tools/rkcommon.h b/tools/rkcommon.h
index 3d64516..cc161a6 100644
--- a/tools/rkcommon.h
+++ b/tools/rkcommon.h
@@ -34,19 +34,6 @@ int rkcommon_check_params(struct image_tool_params *params);
 const char *rkcommon_get_spl_hdr(struct image_tool_params *params);
 
 /**
- * rkcommon_get_spl_hdr_padto8() - check if we need to pad to 8 bytes
- *
- * Rockchip's bootrom starts execution right after the SPL header (i.e.
- * at offset 4), but we can not reasonably align the test section of
- * an AArch64 SPL at 4 bytes (as this would break natural alignment
- * and any embedded constants might cause an alignment exception, which
- * is illegal in privileged modes).
- *
- * Padding is (for now) assumed to occur with a single AArch64 'nop'.
- */
-const bool rkcommon_get_spl_hdr_padto8(struct image_tool_params *params);
-
-/**
  * rkcommon_get_spl_size() - get spl size for a Rockchip boot image
  *
  * Different chip may have different sram size. And if we want to jump
-- 
1.9.1

[U-Boot] [PATCH v2 0/4] rockchip: rk3399: Update SPL generation for the RK3399

[Kever Yang]

Hi Philipp,

     It's great to see your patch fix the align issue in rk3399, reserve 
space
for magic tag is a good idea which is the same with our internal code.
In this case, you do not need to separate for rk3399 and other Rockchips 
SoCs:
1. SPL code: reserve 4-byte space for all rockchip SoCs's SPL,
2. mkimage: override the first 4 byte with magic number, no need to 
insert 'nop'
     for rk3399, rk3399 is the same with other SoCs
3. doc: maybe you can help to update doc/README.rockchip?

Thanks,
- Kever
On 03/15/2017 07:08 PM, Philipp Tomsich wrote:
> For the RK3399, which boots in AArch64 mode, the Boot ROM still starts
> execution at the instruction following the SPL boot-magic
> (i.e. 'RK33'). This puts the first executed instruction on an odd
> address (+0x4).
>
> However, we can't reasonably use for the start of the .text-section on
> AArch64, as it would violate natural alignment... which in turn
> invites trouble down the line if a linker assumes natural alignment or
> some assembly snippet tries to be smart.
>
> This series implements both approaches for dealing with this odd
> alignment during SPL image creation:
> * patches 1 and 2 take an SPL image which starts at (+0x8) and
>    prepends it with a AArch64 'nop' during image assembly
> * patches 3 and 4 change this to using a BOOT0_HOOK to insert a single
>    4-byte word at the beginning of the SPL build to make space for the
>    'RK33' magic and then have the mkimage-tool overwrite this word in
>    the 'set_header' function.
>
> In doing so, the rkimage code is refactored:
> - to remove duplication between rksd.c and rkspi.c
> - to dynamically allocate the space for headers from vrec
>
> Changes in v2:
> - Use BOOT0_HOOK to insert space into the SPL payload that can be
>    overwritten with the boot magic (e.g. 'RK33') by rkimage
> - Change rkimage to overwrite this padding for the RK3399 instead of
>    inserting an artificial 'nop'
>
> Philipp Tomsich (4):
>    rockchip: mkimage: simplify start/size calculation for rc4_encode
>    rockchip: mkimage: pad the header to 8-bytes (using a 'nop') for
>      RK3399
>    rockchip: spl: RK3399: use boot0 hook to create space for SPL magic
>    rockchip: mkimage: update rkimage to support pre-padded payloads
>
>   arch/arm/include/asm/arch-rockchip/boot0.h | 18 ++++++
>   arch/arm/mach-rockchip/Kconfig             |  1 +
>   include/configs/rk3399_common.h            |  2 +-
>   tools/rkcommon.c                           | 90 ++++++++++++++++++++++++++----
>   tools/rkcommon.h                           | 10 ++++
>   tools/rksd.c                               | 17 ++----
>   tools/rkspi.c                              | 17 ++----
>   7 files changed, 119 insertions(+), 36 deletions(-)
>   create mode 100644 arch/arm/include/asm/arch-rockchip/boot0.h
>

[U-Boot] [PATCH v2 1/4] rockchip: mkimage: simplify start/size calculation for rc4_encode

[Simon Glass]

On 15 March 2017 at 05:08, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The RC4 encoding works on full blocks, but the calculation of the
> starting offset and size are needlessly complicated by using a
> reference value known to be offset into a block by the size of the
> header and then correcting for the (hard-coded) size of the header
> (i.e. 4 bytes).
>
> We change this over to use the RK_SPL_HDR_START directly (which is
> known to be on a block boundary).
>
> X-AffectedPlatforms: RK3399-Q7
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
> ---
>
> Changes in v2: None
>
>  tools/rksd.c  | 4 ++--
>  tools/rkspi.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 4/4] rockchip: mkimage: update rkimage to support pre-padded payloads

[Simon Glass]

On 15 March 2017 at 05:08, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> To simplify the creation of AArch64 SPL images for the RK3399, we
> use the ENABLE_ARM_SOC_BOOT0_HOOK option and prepend 4 bytes of
> padding at the start of the text section. This makes it easy for
> mkimage to rewrite this word with the 'RK33' boot magic.
>
> This change brings logic to calculate the header size and allocate
> the header back in sync. For the RK3399 we now limit the header to
> before the payload (i.e. the 'header0' and the padding up to the
> actual image) and overwrite the first word (inserted by the
> boot0-hook for this purpose) with the 'RK33' magic in-place.
>
> X-AffectedPlatforms: RK3399-Q7
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
>
> ---
>
> Changes in v2:
> - Change rkimage to overwrite this padding for the RK3399 instead of
>   inserting an artificial 'nop'
>
>  tools/rkcommon.c | 31 +++++++++++++------------------
>  tools/rkcommon.h | 13 -------------
>  2 files changed, 13 insertions(+), 31 deletions(-)

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 3/4] rockchip: spl: RK3399: use boot0 hook to create space for SPL magic

[Simon Glass]

On 15 March 2017 at 05:08, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The SPL binary needs to be prefixed with the boot magic ('RK33' for
> the RK3399) on the Rockchip platform and starts execution of the
> instruction word following immediately after this boot magic.
>
> This poses a challenge for AArch64 (ARMv8) binaries, as the .text
> section would need to start on the odd address, violating natural
> alignment (and potentially triggering a fault for any code that
> tries to access 64bit values embedded in the .text section).
>
> A quick and easy fix is to have the .text section include the 'RK33'
> magic and pad it with a boot0 hook to insert 4 bytes of padding at the
> start of the section (with the intention of having mkimage overwrite
> this padding with the appropriate boot magic). This avoids having to
> modify the linker scripts or more complex logic in mkimage.
>
> X-AffectedPlatforms: RK3399-Q7
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
>
> ---
>
> Changes in v2:
> - Use BOOT0_HOOK to insert space into the SPL payload that can be
>   overwritten with the boot magic (e.g. 'RK33') by rkimage
>
>  arch/arm/include/asm/arch-rockchip/boot0.h | 18 ++++++++++++++++++
>  arch/arm/mach-rockchip/Kconfig             |  1 +
>  include/configs/rk3399_common.h            |  2 +-
>  3 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/include/asm/arch-rockchip/boot0.h

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 2/4] rockchip: mkimage: pad the header to 8-bytes (using a 'nop') for RK3399

[Simon Glass]

On 15 March 2017 at 05:08, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The RK3399 boot code (running as AArch64) poses a bit of a challenge
> for SPL image generation:
>  * The BootROM will start execution right after the 4-byte header (at
>    the odd instruction word loaded into SRAM at 0xff8c2004, with the
>    'RK33' boot magic residing at 0xff8c2000).
>  * The default padding (during ELF generation) for AArch64 is 0x0,
>    which is an illegal instruction and the .text section needs to be
>    naturally aligned (someone might locate a 64bit constant relative
>    to the section start and unaligned loads trigger a fault for all
>    privileged modes of an ARMv8)... so we can't simply define the
>    CONFIG_SPL_TEXT_BASE option to the odd address (0xff8c2004).
>  * Finally, we don't want to change the values used for padding of
>    the SPL .text section for all ARMv8 targets to the instruction
>    word encoding 'nop', as this would affect all padding in this
>    section and might hide errors that would otherwise quickly trigger
>    an illegal insn exception.
>
> To deal with this situation, we modify the rkimage generation to
>  - understand the fact that the RK3399 needs to pad the header to an
>    8 byte boundary using an AArch64 'nop'
>  - the necessary logic to adjust the header_size (which controls the
>    location where the payload is copied into the image) and to insert
>    this padding (AArch64 insn words are always little-endian) into
>    the image following the 4-byte header magic.
>
> X-AffectedPlatforms: RK3399-Q7
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
> ---
>
> Changes in v2: None
>
>  tools/rkcommon.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-------
>  tools/rkcommon.h | 23 ++++++++++++++
>  tools/rksd.c     | 17 +++-------
>  tools/rkspi.c    | 17 +++-------
>  4 files changed, 118 insertions(+), 36 deletions(-)
>

Applied to u-boot-rockchip, thanks!