[U-Boot] [PATCH 0/5] Secure EMIF firewall and memory reservation on DRA7xx/AM57x devices

[U-Boot] [PATCH 0/5] Secure EMIF firewall and memory reservation on DRA7xx/AM57x devices

[Daniel Allred]

These patches to add secure memory reservations and EMIF firewall config to SDRAM
init code. The reservation and firewall config is done using PPA installed HAL APIs,
so they are not common to all platforms (so they are put in omap5 path instead of
omap-common).

With these patches applied, a secure memory reservation can be specified with the
following configs:

    CONFIG_TI_SECURE_EMIF_REGION_START - start location of region. If it is
        not specified, then the region will be placed at the end of the SDRAM.
    CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE - total size of complete region
    CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE - size (less than total) made
        secure using secure firewalls. The secured region begins at the start
        location (it comes first), and anything left over will be non-secure
        but still reserved from use by u-boot and the kernel

The secure_emif_reserve() API will make use of the above configs to make
a part of the SDRAM secure, with the lowest enforcing priority, giving access
to the ARM TrustZone world only. The secure_emif_firewall_setup() API is
also introduced. This API allows setting other EMIF firewall regions with
particular permissions (for other cores, etc). The current code does not
use this API, but it exists to help satisfy particular system requirements
that users might need.

After all the configuration is done, the secure_emif_firewall_lock() API
should be called to make the previous two APIs stop working. This is important
in order to prevent a later compromise of public supervisor code from being
able to modify the EMIF firewalls. This API is in the code, so any use of the
secure_emif_firewall_setup() API must be inserted before the lock API is called.

Daniel Allred (5):
  ti: omap5: Add Kconfig options for secure EMIF reservations
  arm: omap5: secure API for EMIF memory reservations
  ARM: DRA7: Add secure emif setup calls
  ti_omap5_common: mark region of DRAM protected on HS parts
  ARM: omap5: add fdt secure dram reservation fixup

 arch/arm/cpu/armv7/omap-common/emif-common.c |  15 ++++
 arch/arm/cpu/armv7/omap5/Kconfig             |  26 ++++++
 arch/arm/cpu/armv7/omap5/Makefile            |   1 +
 arch/arm/cpu/armv7/omap5/fdt.c               |  64 +++++++++++++-
 arch/arm/cpu/armv7/omap5/sec-fxns.c          | 126 +++++++++++++++++++++++++++
 arch/arm/include/asm/omap_sec_common.h       |  24 +++++
 include/configs/ti_omap5_common.h            |   8 ++
 7 files changed, 262 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/omap5/sec-fxns.c

-- 
2.7.4

[U-Boot] [PATCH 1/5] ti: omap5: Add Kconfig options for secure EMIF reservations

[Daniel Allred]

Adds start address and size config options for setting aside
a portion of the EMIF memory space for usage by security software
(like a secure OS/TEE). There are two sizes, a total size and a
protected size. The region is divided into protected (secure) and
unprotected (public) regions, that are contiguous and start at the
start address given. If the start address is zero, the intention
is that the region will be automatically placed at the end of the
available external DRAM space.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 arch/arm/cpu/armv7/omap5/Kconfig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/cpu/armv7/omap5/Kconfig b/arch/arm/cpu/armv7/omap5/Kconfig
index a8600b1..25474ed 100644
--- a/arch/arm/cpu/armv7/omap5/Kconfig
+++ b/arch/arm/cpu/armv7/omap5/Kconfig
@@ -24,6 +24,32 @@ endchoice
 config SYS_SOC
 	default "omap5"
 
+config TI_SECURE_EMIF_REGION_START
+	hex "Reserved EMIF region start address"
+	depends on TI_SECURE_DEVICE
+	default 0x0
+	help
+	  Reserved EMIF region start address. Set to "0" to auto-select
+	  to be at the end of the external memory region.
+
+config TI_SECURE_EMIF_TOTAL_REGION_SIZE
+	hex "Reserved EMIF region size"
+	depends on TI_SECURE_DEVICE
+	default 0x0
+	help
+	  Total reserved EMIF region size. Default is 0, which means no reserved EMIF
+	  region on secure devices.
+
+config TI_SECURE_EMIF_PROTECTED_REGION_SIZE
+	hex "Size of protected region within reserved EMIF region"
+	depends on TI_SECURE_DEVICE
+	default 0x0
+	help
+	  This config option is used to specify the size of the portion of the total
+	  reserved EMIF region set aside for secure OS needs that will  be protected
+	  using hardware memory firewalls. This value must be smaller than the
+	  TI_SECURE_EMIF_TOTAL_REGION_SIZE value.
+
 source "board/compulab/cm_t54/Kconfig"
 source "board/ti/omap5_uevm/Kconfig"
 source "board/ti/dra7xx/Kconfig"
-- 
2.7.4

[U-Boot] [PATCH 2/5] arm: omap5: secure API for EMIF memory reservations

[Daniel Allred]

Create a few public APIs which rely on secure world ROM/HAL
APIs for their implementation. These are intended to be used
to reserve a portion of the EMIF memory and configure hardware
firewalls around that region to prevent public code from
manipulating or interfering with that memory.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 arch/arm/cpu/armv7/omap5/Makefile      |   1 +
 arch/arm/cpu/armv7/omap5/sec-fxns.c    | 126 +++++++++++++++++++++++++++++++++
 arch/arm/include/asm/omap_sec_common.h |  24 +++++++
 3 files changed, 151 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/omap5/sec-fxns.c

diff --git a/arch/arm/cpu/armv7/omap5/Makefile b/arch/arm/cpu/armv7/omap5/Makefile
index 3caba86..0212df7 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -14,3 +14,4 @@ obj-y	+= hw_data.o
 obj-y	+= abb.o
 obj-y	+= fdt.o
 obj-$(CONFIG_IODELAY_RECALIBRATION) += dra7xx_iodelay.o
+obj-$(CONFIG_TI_SECURE_DEVICE) += sec-fxns.o
diff --git a/arch/arm/cpu/armv7/omap5/sec-fxns.c b/arch/arm/cpu/armv7/omap5/sec-fxns.c
new file mode 100644
index 0000000..33d4ea4
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/sec-fxns.c
@@ -0,0 +1,126 @@
+/*
+ *
+ * Security related functions for OMAP5 class devices
+ *
+ * (C) Copyright 2016
+ * Texas Instruments, <www.ti.com>
+ *
+ * Daniel Allred <d-allred@ti.com>
+ * Harinarayan Bhatta <harinarayan@ti.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <stdarg.h>
+
+#include <asm/arch/sys_proto.h>
+#include <asm/omap_common.h>
+#include <asm/omap_sec_common.h>
+#include <asm/spl.h>
+#include <spl.h>
+
+/* Index for signature PPA-based TI HAL APIs */
+#define PPA_HAL_SERVICES_START_INDEX        (0x200)
+#define PPA_SERV_HAL_SETUP_SEC_RESVD_REGION (PPA_HAL_SERVICES_START_INDEX + 25)
+#define PPA_SERV_HAL_SETUP_EMIF_FW_REGION   (PPA_HAL_SERVICES_START_INDEX + 26)
+#define PPA_SERV_HAL_LOCK_EMIF_FW           (PPA_HAL_SERVICES_START_INDEX + 27)
+
+static u32 get_sec_mem_start(void)
+{
+	u32 sec_mem_start = CONFIG_TI_SECURE_EMIF_REGION_START;
+	u32 sec_mem_size = CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE;
+	/*
+	 * Total reserved region is all contiguous with protected
+	 * region coming first, followed by the non-secure region.
+	 * If 0x0 start address is given, we simply put the reserved
+	 * region at the end of the external DRAM.
+	 */
+	if (sec_mem_start == 0)
+		sec_mem_start =
+			(CONFIG_SYS_SDRAM_BASE +
+			(omap_sdram_size() - sec_mem_size));
+	return sec_mem_start;
+}
+
+int secure_emif_firewall_setup(uint8_t region_num, uint32_t start_addr,
+			       uint32_t size, uint32_t access_perm,
+			       uint32_t initiator_perm)
+{
+	int result = 1;
+
+	/*
+	 * Call PPA HAL API to do any other general firewall
+	 * configuration for regions 1-6 of the EMIF firewall.
+	 */
+	debug("%s: regionNum = %x, startAddr = %x, size = %x", __func__,
+	      region_num, start_addr, size);
+
+	result = secure_rom_call(
+			PPA_SERV_HAL_SETUP_EMIF_FW_REGION, 0, 0, 4,
+			(start_addr & 0xFFFFFFF0) | (region_num & 0x0F),
+			size, access_perm, initiator_perm);
+
+	if (result != 0) {
+		puts("Secure EMIF Firewall Setup failed!\n");
+		debug("Return Value = %x\n", result);
+	}
+
+	return result;
+}
+
+#if	(CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE <  \
+	CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE)
+#error	"TI Secure EMIF: Protected size cannot be larger than total size."
+#endif
+int secure_emif_reserve(void)
+{
+	int result = 1;
+	u32 sec_mem_start = get_sec_mem_start();
+	u32 sec_prot_size = CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE;
+
+	/* If there is no protected region, there is no reservation to make */
+	if (sec_prot_size == 0)
+		return 0;
+
+	/*
+	 * Call PPA HAL API to reserve a chunk of EMIF SDRAM
+	 * for secure world use. This region should be carved out
+	 * from use by any public code. EMIF firewall region 7
+	 * will be used to protect this block of memory.
+	 */
+	result = secure_rom_call(
+			PPA_SERV_HAL_SETUP_SEC_RESVD_REGION,
+			0, 0, 2, sec_mem_start, sec_prot_size);
+
+	if (result != 0) {
+		puts("SDRAM Firewall: Secure memory reservation failed!\n");
+		debug("Return Value = %x\n", result);
+	}
+
+	return result;
+}
+
+int secure_emif_firewall_lock(void)
+{
+	int result = 1;
+
+	/*
+	 * Call PPA HAL API to lock the EMIF firewall configurations.
+	 * After this API is called, none of the PPA HAL APIs for
+	 * configuring the EMIF firewalls will be usable again (that
+	 * is, calls to those APIs will return failure and have no
+	 * effect).
+	 */
+
+	result = secure_rom_call(
+			PPA_SERV_HAL_LOCK_EMIF_FW,
+			0, 0, 0);
+
+	if (result != 0) {
+		puts("Secure EMIF Firewall Lock failed!\n");
+		debug("Return Value = %x\n", result);
+	}
+
+	return result;
+}
diff --git a/arch/arm/include/asm/omap_sec_common.h b/arch/arm/include/asm/omap_sec_common.h
index 842f2af..4bde93f 100644
--- a/arch/arm/include/asm/omap_sec_common.h
+++ b/arch/arm/include/asm/omap_sec_common.h
@@ -27,4 +27,28 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...);
  */
 int secure_boot_verify_image(void **p_image, size_t *p_size);
 
+/*
+ * Invoke a secure HAL API that allows configuration of the external memory
+ * firewall regions.
+ */
+int secure_emif_firewall_setup(uint8_t region_num, uint32_t start_addr,
+			       uint32_t size, uint32_t access_perm,
+			       uint32_t initiator_perm);
+
+/*
+ * Invoke a secure HAL API on high-secure (HS) device variants that reserves a
+ * region of external memory for secure world use, and protects it using memory
+ * firewalls that prevent public world access. This API is intended to setaside
+ * memory that will be used for a secure world OS/TEE.
+ */
+int secure_emif_reserve(void);
+
+/*
+ * Invoke a secure HAL API to lock the external memory firewall configurations.
+ * After this API is called, none of the HAL APIs for configuring the that
+ * firewall will be usable (calls to those APIs will return failure and have
+ * no effect).
+ */
+int secure_emif_firewall_lock(void);
+
 #endif /* _OMAP_SEC_COMMON_H_ */
-- 
2.7.4

[U-Boot] [PATCH 3/5] ARM: DRA7: Add secure emif setup calls

[Daniel Allred]

After EMIF DRAM is configured, but before it is used,
calls are made on secure devices to reserve any configured
memory region needed by the secure world and then to lock the
EMIF firewall configuration. If any other firewall
configuration needs to be applied, it must happen before the
lock call.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c
index 2b79010..b26984e 100644
--- a/arch/arm/cpu/armv7/omap-common/emif-common.c
+++ b/arch/arm/cpu/armv7/omap-common/emif-common.c
@@ -14,6 +14,7 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/omap_common.h>
+#include <asm/omap_sec_common.h>
 #include <asm/utils.h>
 #include <linux/compiler.h>
 
@@ -1477,6 +1478,20 @@ void sdram_init(void)
 			debug("get_ram_size() successful");
 	}
 
+#if defined(CONFIG_TI_SECURE_DEVICE)
+	/*
+	 * On HS devices, do static EMIF firewall configuration
+	 * but only do it if not already running in SDRAM
+	 */
+	if (!in_sdram)
+		if (0 != secure_emif_reserve())
+			hang();
+
+	/* On HS devices, ensure static EMIF firewall APIs are locked */
+	if (0 != secure_emif_firewall_lock())
+		hang();
+#endif
+
 	if (sdram_type == EMIF_SDRAM_TYPE_DDR3 &&
 	    (!in_sdram && !warm_reset()) && (!is_dra7xx())) {
 		if (emif1_enabled)
-- 
2.7.4

[U-Boot] [PATCH 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Daniel Allred]

If the ending portion of the DRAM is reserved for secure
world use, then u-boot cannot use this memory for its relocation
purposes. To prevent issues, we mark this memory as PRAM and this
prevents it from being used by u-boot at all.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 include/configs/ti_omap5_common.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
index e42c88e..70fdc6e 100644
--- a/include/configs/ti_omap5_common.h
+++ b/include/configs/ti_omap5_common.h
@@ -146,6 +146,14 @@
  */
 #define TI_OMAP5_SECURE_BOOT_RESV_SRAM_SZ	0x1000
 #define CONFIG_SPL_TEXT_BASE	0x40301350
+/* If no specific start address is specified then the secure EMIF
+ * region will be placed at the end of the DDR space. In order to prevent
+ * the main u-boot relocation from clobbering that memory and causing a
+ * firewall violation, we tell u-boot that memory is protected RAM (PRAM)
+ */
+#if (CONFIG_TI_SECURE_EMIF_REGION_START == 0)
+#define CONFIG_PRAM (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE) >> 10
+#endif
 #else
 /*
  * For all booting on GP parts, the flash loader image is
-- 
2.7.4

[U-Boot] [PATCH 5/5] ARM: omap5: add fdt secure dram reservation fixup

[Daniel Allred]

Adds a secure dram reservation fixup for secure
devices, when a region in the emif has been set aside
for secure world use. The size is defined by the
CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE config option.

Signed-off-by: Daniel Allred <d-allred@ti.com>
---
 arch/arm/cpu/armv7/omap5/fdt.c | 64 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap5/fdt.c b/arch/arm/cpu/armv7/omap5/fdt.c
index 0493cd1..da8d59b 100644
--- a/arch/arm/cpu/armv7/omap5/fdt.c
+++ b/arch/arm/cpu/armv7/omap5/fdt.c
@@ -153,13 +153,73 @@ static int ft_hs_fixup_sram(void *fdt, bd_t *bd)
 static int ft_hs_fixup_sram(void *fdt, bd_t *bd) { return 0; }
 #endif
 
+#if (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE != 0)
+static int ft_hs_fixup_dram(void *fdt, bd_t *bd)
+{
+	const char *path, *subpath;
+	int offs;
+	u32 sec_mem_start = CONFIG_TI_SECURE_EMIF_REGION_START;
+	u32 sec_mem_size = CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE;
+	fdt64_t temp[2];
+
+	/* If start address is zero, place at end of DRAM */
+	if (0 == sec_mem_start)
+		sec_mem_start =
+			(CONFIG_SYS_SDRAM_BASE +
+			(omap_sdram_size() - sec_mem_size));
+
+	/* Delete any original secure_reserved node */
+	path = "/reserved-memory/secure_reserved";
+	offs = fdt_path_offset(fdt, path);
+	if (offs >= 0)
+		fdt_del_node(fdt, offs);
+
+	/* Add new secure_reserved node */
+	path = "/reserved-memory";
+	offs = fdt_path_offset(fdt, path);
+	if (offs < 0) {
+		debug("Node %s not found\n", path);
+		path = "/";
+		subpath = "reserved-memory";
+		fdt_path_offset(fdt, path);
+		offs = fdt_add_subnode(fdt, offs, subpath);
+		if (offs < 0) {
+			printf("Could not create %s%s node.\n", path, subpath);
+			return 1;
+		}
+		path = "/reserved-memory";
+		offs = fdt_path_offset(fdt, path);
+	}
+
+	subpath = "secure_reserved";
+	offs = fdt_add_subnode(fdt, offs, subpath);
+	if (offs < 0) {
+		printf("Could not create %s%s node.\n", path, subpath);
+		return 1;
+	}
+
+	temp[0] = cpu_to_fdt64(((u64)sec_mem_start));
+	temp[1] = cpu_to_fdt64(((u64)sec_mem_size));
+	fdt_setprop_string(fdt, offs, "compatible",
+			   "ti,dra7-secure-memory");
+	fdt_setprop_string(fdt, offs, "status", "okay");
+	fdt_setprop(fdt, offs, "no-map", NULL, 0);
+	fdt_setprop(fdt, offs, "reg", temp, sizeof(temp));
+
+	return 0;
+}
+#else
+static int ft_hs_fixup_dram(void *fdt, bd_t *bd) { return 0; }
+#endif
+
 static void ft_hs_fixups(void *fdt, bd_t *bd)
 {
 	/* Check we are running on an HS/EMU device type */
 	if (GP_DEVICE != get_device_type()) {
 		if ((ft_hs_fixup_crossbar(fdt, bd) == 0) &&
 		    (ft_hs_disable_rng(fdt, bd) == 0) &&
-		    (ft_hs_fixup_sram(fdt, bd) == 0))
+		    (ft_hs_fixup_sram(fdt, bd) == 0) &&
+		    (ft_hs_fixup_dram(fdt, bd) == 0))
 			return;
 	} else {
 		printf("ERROR: Incorrect device type (GP) detected!");
@@ -171,7 +231,7 @@ static void ft_hs_fixups(void *fdt, bd_t *bd)
 static void ft_hs_fixups(void *fdt, bd_t *bd)
 {
 }
-#endif
+#endif /* #ifdef CONFIG_TI_SECURE_DEVICE */
 
 /*
  * Place for general cpu/SoC FDT fixups. Board specific
-- 
2.7.4

[U-Boot] [PATCH 1/5] ti: omap5: Add Kconfig options for secure EMIF reservations

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:20AM -0500, Daniel Allred wrote:

> Adds start address and size config options for setting aside
> a portion of the EMIF memory space for usage by security software
> (like a secure OS/TEE). There are two sizes, a total size and a
> protected size. The region is divided into protected (secure) and
> unprotected (public) regions, that are contiguous and start at the
> start address given. If the start address is zero, the intention
> is that the region will be automatically placed at the end of the
> available external DRAM space.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  arch/arm/cpu/armv7/omap5/Kconfig | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/omap5/Kconfig b/arch/arm/cpu/armv7/omap5/Kconfig
> index a8600b1..25474ed 100644
> --- a/arch/arm/cpu/armv7/omap5/Kconfig
> +++ b/arch/arm/cpu/armv7/omap5/Kconfig
> @@ -24,6 +24,32 @@ endchoice
>  config SYS_SOC
>  	default "omap5"

I think we should hide these based on some config option and then have a
menu here with these entries.  That also means that you don't need to
have "0 to disable" for the size.

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

[U-Boot] [PATCH 2/5] arm: omap5: secure API for EMIF memory reservations

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:21AM -0500, Daniel Allred wrote:

> Create a few public APIs which rely on secure world ROM/HAL
> APIs for their implementation. These are intended to be used
> to reserve a portion of the EMIF memory and configure hardware
> firewalls around that region to prevent public code from
> manipulating or interfering with that memory.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>

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

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

[U-Boot] [PATCH 3/5] ARM: DRA7: Add secure emif setup calls

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote:
> After EMIF DRAM is configured, but before it is used,
> calls are made on secure devices to reserve any configured
> memory region needed by the secure world and then to lock the
> EMIF firewall configuration. If any other firewall
> configuration needs to be applied, it must happen before the
> lock call.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c
> index 2b79010..b26984e 100644
> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c
> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c
> @@ -14,6 +14,7 @@
>  #include <asm/arch/clock.h>
>  #include <asm/arch/sys_proto.h>
>  #include <asm/omap_common.h>
> +#include <asm/omap_sec_common.h>
>  #include <asm/utils.h>
>  #include <linux/compiler.h>
>  
> @@ -1477,6 +1478,20 @@ void sdram_init(void)
>  			debug("get_ram_size() successful");
>  	}
>  
> +#if defined(CONFIG_TI_SECURE_DEVICE)
> +	/*
> +	 * On HS devices, do static EMIF firewall configuration
> +	 * but only do it if not already running in SDRAM
> +	 */
> +	if (!in_sdram)
> +		if (0 != secure_emif_reserve())
> +			hang();
> +
> +	/* On HS devices, ensure static EMIF firewall APIs are locked */
> +	if (0 != secure_emif_firewall_lock())
> +		hang();

Those are awkward tests (should be func() != val), and since it's just
checking for function return status, we should just write that normally.
Thanks!

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

[U-Boot] [PATCH 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:23AM -0500, Daniel Allred wrote:

> If the ending portion of the DRAM is reserved for secure
> world use, then u-boot cannot use this memory for its relocation
> purposes. To prevent issues, we mark this memory as PRAM and this
> prevents it from being used by u-boot at all.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  include/configs/ti_omap5_common.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> index e42c88e..70fdc6e 100644
> --- a/include/configs/ti_omap5_common.h
> +++ b/include/configs/ti_omap5_common.h
> @@ -146,6 +146,14 @@
>   */
>  #define TI_OMAP5_SECURE_BOOT_RESV_SRAM_SZ	0x1000
>  #define CONFIG_SPL_TEXT_BASE	0x40301350
> +/* If no specific start address is specified then the secure EMIF
> + * region will be placed at the end of the DDR space. In order to prevent
> + * the main u-boot relocation from clobbering that memory and causing a
> + * firewall violation, we tell u-boot that memory is protected RAM (PRAM)
> + */
> +#if (CONFIG_TI_SECURE_EMIF_REGION_START == 0)
> +#define CONFIG_PRAM (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE) >> 10
> +#endif
>  #else
>  /*
>   * For all booting on GP parts, the flash loader image is

Can you move PRAM to Kconfig please?  Thinking about this more, I think
what you want for the size option is CONFIG_PRAM and then to use that to
expand out the size.

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

[U-Boot] [PATCH 5/5] ARM: omap5: add fdt secure dram reservation fixup

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:24AM -0500, Daniel Allred wrote:

> Adds a secure dram reservation fixup for secure
> devices, when a region in the emif has been set aside
> for secure world use. The size is defined by the
> CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE config option.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>

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

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

[U-Boot] [PATCH 1/5] ti: omap5: Add Kconfig options for secure EMIF reservations

[Andrew F. Davis]

On 09/02/2016 12:40 AM, Daniel Allred wrote:
> Adds start address and size config options for setting aside
> a portion of the EMIF memory space for usage by security software
> (like a secure OS/TEE). There are two sizes, a total size and a
> protected size. The region is divided into protected (secure) and
> unprotected (public) regions, that are contiguous and start at the
> start address given. If the start address is zero, the intention
> is that the region will be automatically placed at the end of the
> available external DRAM space.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  arch/arm/cpu/armv7/omap5/Kconfig | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/omap5/Kconfig b/arch/arm/cpu/armv7/omap5/Kconfig
> index a8600b1..25474ed 100644
> --- a/arch/arm/cpu/armv7/omap5/Kconfig
> +++ b/arch/arm/cpu/armv7/omap5/Kconfig
> @@ -24,6 +24,32 @@ endchoice
>  config SYS_SOC
>  	default "omap5"
>  
> +config TI_SECURE_EMIF_REGION_START
> +	hex "Reserved EMIF region start address"
> +	depends on TI_SECURE_DEVICE
> +	default 0x0
> +	help
> +	  Reserved EMIF region start address. Set to "0" to auto-select
> +	  to be at the end of the external memory region.

The secure image that is placed in this location (OPTEE OS for now) will
probably not be relocatable, what happens when we add RAM to a board
(some boards take regular ram sticks), will our secure world suddenly
break? What do we gain from having the location auto-selected?

> +
> +config TI_SECURE_EMIF_TOTAL_REGION_SIZE

Instead of having the total combined size of both the protected and
non-protected secure region then subtracting out the protected below to
get the non-protected, why not have:

TI_SECURE_EMIF_PUBLIC_REGION_SIZE
TI_SECURE_EMIF_PROTECTED_REGION_SIZE

then add the two to get the total region size? This would also remove
some checks to make sure the total isn't smaller than one part, plus it
removes the need for any mental math in figuring the size of the public
region.

> +	hex "Reserved EMIF region size"
> +	depends on TI_SECURE_DEVICE
> +	default 0x0
> +	help
> +	  Total reserved EMIF region size. Default is 0, which means no reserved EMIF
> +	  region on secure devices.
> +
> +config TI_SECURE_EMIF_PROTECTED_REGION_SIZE
> +	hex "Size of protected region within reserved EMIF region"
> +	depends on TI_SECURE_DEVICE
> +	default 0x0
> +	help
> +	  This config option is used to specify the size of the portion of the total
> +	  reserved EMIF region set aside for secure OS needs that will  be protected
> +	  using hardware memory firewalls. This value must be smaller than the
> +	  TI_SECURE_EMIF_TOTAL_REGION_SIZE value.
> +
>  source "board/compulab/cm_t54/Kconfig"
>  source "board/ti/omap5_uevm/Kconfig"
>  source "board/ti/dra7xx/Kconfig"
> 

[U-Boot] [PATCH 3/5] ARM: DRA7: Add secure emif setup calls

[Andrew F. Davis]

On 09/02/2016 12:40 AM, Daniel Allred wrote:
> After EMIF DRAM is configured, but before it is used,
> calls are made on secure devices to reserve any configured
> memory region needed by the secure world and then to lock the
> EMIF firewall configuration. If any other firewall
> configuration needs to be applied, it must happen before the
> lock call.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c
> index 2b79010..b26984e 100644
> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c
> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c
> @@ -14,6 +14,7 @@
>  #include <asm/arch/clock.h>
>  #include <asm/arch/sys_proto.h>
>  #include <asm/omap_common.h>
> +#include <asm/omap_sec_common.h>
>  #include <asm/utils.h>
>  #include <linux/compiler.h>
>  
> @@ -1477,6 +1478,20 @@ void sdram_init(void)
>  			debug("get_ram_size() successful");
>  	}
>  
> +#if defined(CONFIG_TI_SECURE_DEVICE)
> +	/*
> +	 * On HS devices, do static EMIF firewall configuration
> +	 * but only do it if not already running in SDRAM
> +	 */
> +	if (!in_sdram)

Should we hang in this case?

> +		if (0 != secure_emif_reserve())

"0 != " could be removed here and below.

> +			hang();
> +
> +	/* On HS devices, ensure static EMIF firewall APIs are locked */
> +	if (0 != secure_emif_firewall_lock())
> +		hang();
> +#endif
> +
>  	if (sdram_type == EMIF_SDRAM_TYPE_DDR3 &&
>  	    (!in_sdram && !warm_reset()) && (!is_dra7xx())) {
>  		if (emif1_enabled)
> 

[U-Boot] [PATCH 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Andrew F. Davis]

On 09/02/2016 12:40 AM, Daniel Allred wrote:
> If the ending portion of the DRAM is reserved for secure
> world use, then u-boot cannot use this memory for its relocation
> purposes. To prevent issues, we mark this memory as PRAM and this
> prevents it from being used by u-boot at all.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  include/configs/ti_omap5_common.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> index e42c88e..70fdc6e 100644
> --- a/include/configs/ti_omap5_common.h
> +++ b/include/configs/ti_omap5_common.h
> @@ -146,6 +146,14 @@
>   */
>  #define TI_OMAP5_SECURE_BOOT_RESV_SRAM_SZ	0x1000
>  #define CONFIG_SPL_TEXT_BASE	0x40301350
> +/* If no specific start address is specified then the secure EMIF
> + * region will be placed at the end of the DDR space. In order to prevent
> + * the main u-boot relocation from clobbering that memory and causing a
> + * firewall violation, we tell u-boot that memory is protected RAM (PRAM)
> + */
> +#if (CONFIG_TI_SECURE_EMIF_REGION_START == 0)

What about if we set the start address manually, what prevents u-boot
from over-writing that memory?

> +#define CONFIG_PRAM (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE) >> 10
> +#endif
>  #else
>  /*
>   * For all booting on GP parts, the flash loader image is
> 

[U-Boot] [PATCH 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Tom Rini]

On Tue, Sep 06, 2016 at 03:54:31PM -0500, Andrew F. Davis wrote:
> On 09/02/2016 12:40 AM, Daniel Allred wrote:
> > If the ending portion of the DRAM is reserved for secure
> > world use, then u-boot cannot use this memory for its relocation
> > purposes. To prevent issues, we mark this memory as PRAM and this
> > prevents it from being used by u-boot at all.
> > 
> > Signed-off-by: Daniel Allred <d-allred@ti.com>
> > ---
> >  include/configs/ti_omap5_common.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> > index e42c88e..70fdc6e 100644
> > --- a/include/configs/ti_omap5_common.h
> > +++ b/include/configs/ti_omap5_common.h
> > @@ -146,6 +146,14 @@
> >   */
> >  #define TI_OMAP5_SECURE_BOOT_RESV_SRAM_SZ	0x1000
> >  #define CONFIG_SPL_TEXT_BASE	0x40301350
> > +/* If no specific start address is specified then the secure EMIF
> > + * region will be placed at the end of the DDR space. In order to prevent
> > + * the main u-boot relocation from clobbering that memory and causing a
> > + * firewall violation, we tell u-boot that memory is protected RAM (PRAM)
> > + */
> > +#if (CONFIG_TI_SECURE_EMIF_REGION_START == 0)
> 
> What about if we set the start address manually, what prevents u-boot
> from over-writing that memory?

I think that's another reason this needs to be made to use the existing
pram mechanism as we make sure that if pram isn't set in the environment
we set it to the default of CONFIG_PRAM.

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

[U-Boot] [PATCH 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Allred, Daniel]

On 9/6/2016 3:54 PM, Andrew F. Davis wrote:
> On 09/02/2016 12:40 AM, Daniel Allred wrote:
>> If the ending portion of the DRAM is reserved for secure
>> world use, then u-boot cannot use this memory for its relocation
>> purposes. To prevent issues, we mark this memory as PRAM and this
>> prevents it from being used by u-boot at all.
>>
>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>> ---
>>  include/configs/ti_omap5_common.h | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
>> index e42c88e..70fdc6e 100644
>> --- a/include/configs/ti_omap5_common.h
>> +++ b/include/configs/ti_omap5_common.h
>> @@ -146,6 +146,14 @@
>>   */
>>  #define TI_OMAP5_SECURE_BOOT_RESV_SRAM_SZ	0x1000
>>  #define CONFIG_SPL_TEXT_BASE	0x40301350
>> +/* If no specific start address is specified then the secure EMIF
>> + * region will be placed at the end of the DDR space. In order to prevent
>> + * the main u-boot relocation from clobbering that memory and causing a
>> + * firewall violation, we tell u-boot that memory is protected RAM (PRAM)
>> + */
>> +#if (CONFIG_TI_SECURE_EMIF_REGION_START == 0)
> 
> What about if we set the start address manually, what prevents u-boot
> from over-writing that memory?
Nothing. The PRAM mechanism, as it is currently defined in the u-boot code, can only protect the RAM at the end. It prevents u-boot from seeing that memory so that it won't try to relocate itself there during the u-boot relocation. We found this was needed because the firewalls are warm-reset insensitive and so u-boot could cause firewall violations after a warm reset. So we really need to put this memory somewhere where it can be set aside from all other uses, and the CONFIG_PRAM mechanism accomplishes this. 

If you manually place it anywhere else in the DRAM, you have to make sure that memory will not be used by any u-boot code. I think we saw one case early on in development where the location of the secure OS ended up conflicting with the Ethernet buffer memory, thus breaking network/NFS booting (that was when we were loading the TEE under Linux, so we only saw the violation on warm reset, not the initial boot). Since all other DRAM users in u-boot avoid the end of DRAM because of its use for u-boot relocation, using the PRAM option to put it there avoids all possible issues. So manual placement can work, but it has more risks. But you can then place the secure OS/TEE at a fixed location, which could be helpful if it is not PIC and you want to support it across quite a few platforms with different memory sizes without a re-link. 

Regards, Daniel
> 
>> +#define CONFIG_PRAM (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE) >> 10
>> +#endif
>>  #else
>>  /*
>>   * For all booting on GP parts, the flash loader image is
>>

[U-Boot] [PATCH 3/5] ARM: DRA7: Add secure emif setup calls

[Allred, Daniel]

On 9/2/2016 9:54 AM, Tom Rini wrote:
> On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote:
>> After EMIF DRAM is configured, but before it is used,
>> calls are made on secure devices to reserve any configured
>> memory region needed by the secure world and then to lock the
>> EMIF firewall configuration. If any other firewall
>> configuration needs to be applied, it must happen before the
>> lock call.
>>
>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>> ---
>>  arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c
>> index 2b79010..b26984e 100644
>> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c
>> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c
>> @@ -14,6 +14,7 @@
>>  #include <asm/arch/clock.h>
>>  #include <asm/arch/sys_proto.h>
>>  #include <asm/omap_common.h>
>> +#include <asm/omap_sec_common.h>
>>  #include <asm/utils.h>
>>  #include <linux/compiler.h>
>>  
>> @@ -1477,6 +1478,20 @@ void sdram_init(void)
>>  			debug("get_ram_size() successful");
>>  	}
>>  
>> +#if defined(CONFIG_TI_SECURE_DEVICE)
>> +	/*
>> +	 * On HS devices, do static EMIF firewall configuration
>> +	 * but only do it if not already running in SDRAM
>> +	 */
>> +	if (!in_sdram)
>> +		if (0 != secure_emif_reserve())
>> +			hang();
>> +
>> +	/* On HS devices, ensure static EMIF firewall APIs are locked */
>> +	if (0 != secure_emif_firewall_lock())
>> +		hang();
> 
> Those are awkward tests (should be func() != val), and since it's just
> checking for function return status, we should just write that normally.
> Thanks!
> 
Fair point, will change.

[U-Boot] [PATCH 3/5] ARM: DRA7: Add secure emif setup calls

[Allred, Daniel]

On 9/6/2016 3:41 PM, Andrew F. Davis wrote:
> On 09/02/2016 12:40 AM, Daniel Allred wrote:
>> After EMIF DRAM is configured, but before it is used,
>> calls are made on secure devices to reserve any configured
>> memory region needed by the secure world and then to lock the
>> EMIF firewall configuration. If any other firewall
>> configuration needs to be applied, it must happen before the
>> lock call.
>>
>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>> ---
>>  arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c
>> index 2b79010..b26984e 100644
>> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c
>> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c
>> @@ -14,6 +14,7 @@
>>  #include <asm/arch/clock.h>
>>  #include <asm/arch/sys_proto.h>
>>  #include <asm/omap_common.h>
>> +#include <asm/omap_sec_common.h>
>>  #include <asm/utils.h>
>>  #include <linux/compiler.h>
>>  
>> @@ -1477,6 +1478,20 @@ void sdram_init(void)
>>  			debug("get_ram_size() successful");
>>  	}
>>  
>> +#if defined(CONFIG_TI_SECURE_DEVICE)
>> +	/*
>> +	 * On HS devices, do static EMIF firewall configuration
>> +	 * but only do it if not already running in SDRAM
>> +	 */
>> +	if (!in_sdram)
> 
> Should we hang in this case?
The check is supposed to ensure that if we are already in DRAM, we don't apply the firewall config. It might make sense to still (attempt to) do the lock. I need to think a little more on it, but I don't think hanging if in sdram is the right thing.
> 
>> +		if (0 != secure_emif_reserve())
> 
> "0 != " could be removed here and below.
> 
>> +			hang();
>> +
>> +	/* On HS devices, ensure static EMIF firewall APIs are locked */
>> +	if (0 != secure_emif_firewall_lock())
>> +		hang();
>> +#endif
>> +
>>  	if (sdram_type == EMIF_SDRAM_TYPE_DDR3 &&
>>  	    (!in_sdram && !warm_reset()) && (!is_dra7xx())) {
>>  		if (emif1_enabled)
>>

[U-Boot] [PATCH 1/5] ti: omap5: Add Kconfig options for secure EMIF reservations

[Allred, Daniel]

On 9/6/2016 3:23 PM, Andrew F. Davis wrote:
> On 09/02/2016 12:40 AM, Daniel Allred wrote:
>> Adds start address and size config options for setting aside
>> a portion of the EMIF memory space for usage by security software
>> (like a secure OS/TEE). There are two sizes, a total size and a
>> protected size. The region is divided into protected (secure) and
>> unprotected (public) regions, that are contiguous and start at the
>> start address given. If the start address is zero, the intention
>> is that the region will be automatically placed at the end of the
>> available external DRAM space.
>>
>> Signed-off-by: Daniel Allred <d-allred@ti.com>
>> ---
>>  arch/arm/cpu/armv7/omap5/Kconfig | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/omap5/Kconfig b/arch/arm/cpu/armv7/omap5/Kconfig
>> index a8600b1..25474ed 100644
>> --- a/arch/arm/cpu/armv7/omap5/Kconfig
>> +++ b/arch/arm/cpu/armv7/omap5/Kconfig
>> @@ -24,6 +24,32 @@ endchoice
>>  config SYS_SOC
>>  	default "omap5"
>>  
>> +config TI_SECURE_EMIF_REGION_START
>> +	hex "Reserved EMIF region start address"
>> +	depends on TI_SECURE_DEVICE
>> +	default 0x0
>> +	help
>> +	  Reserved EMIF region start address. Set to "0" to auto-select
>> +	  to be at the end of the external memory region.
> 
> The secure image that is placed in this location (OPTEE OS for now) will
> probably not be relocatable, what happens when we add RAM to a board
> (some boards take regular ram sticks), will our secure world suddenly
> break? What do we gain from having the location auto-selected?
See the answer I provided to the other patch about PRAM. Auto-selection isn't the key, but putting it at the end out of the way of all potential conflicts is. If the secure OS/TEE is not PIC, then re-linking will be required if the RAM size changes. But I personally see a change in RAM size as a new platform/board, so I don't think rebuilding some of the SW for that platform is completely unreasonable. Ultimately, it will depend on the secure OS/TEE.
> 
>> +
>> +config TI_SECURE_EMIF_TOTAL_REGION_SIZE
> 
> Instead of having the total combined size of both the protected and
> non-protected secure region then subtracting out the protected below to
> get the non-protected, why not have:
> 
> TI_SECURE_EMIF_PUBLIC_REGION_SIZE
> TI_SECURE_EMIF_PROTECTED_REGION_SIZE
> 
> then add the two to get the total region size? This would also remove
> some checks to make sure the total isn't smaller than one part, plus it
> removes the need for any mental math in figuring the size of the public
> region.
We had it this way previously, but the code ended up more complicated (though maybe the preprocessor checks were simpler). Having seen it both ways, I think it is a wash.

Daniel
> 
>> +	hex "Reserved EMIF region size"
>> +	depends on TI_SECURE_DEVICE
>> +	default 0x0
>> +	help
>> +	  Total reserved EMIF region size. Default is 0, which means no reserved EMIF
>> +	  region on secure devices.
>> +
>> +config TI_SECURE_EMIF_PROTECTED_REGION_SIZE
>> +	hex "Size of protected region within reserved EMIF region"
>> +	depends on TI_SECURE_DEVICE
>> +	default 0x0
>> +	help
>> +	  This config option is used to specify the size of the portion of the total
>> +	  reserved EMIF region set aside for secure OS needs that will  be protected
>> +	  using hardware memory firewalls. This value must be smaller than the
>> +	  TI_SECURE_EMIF_TOTAL_REGION_SIZE value.
>> +
>>  source "board/compulab/cm_t54/Kconfig"
>>  source "board/ti/omap5_uevm/Kconfig"
>>  source "board/ti/dra7xx/Kconfig"
>>

[U-Boot] [PATCH 5/5] ARM: omap5: add fdt secure dram reservation fixup

[Andrew F. Davis]

On 09/02/2016 12:40 AM, Daniel Allred wrote:
> Adds a secure dram reservation fixup for secure
> devices, when a region in the emif has been set aside
> for secure world use. The size is defined by the
> CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE config option.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> ---
>  arch/arm/cpu/armv7/omap5/fdt.c | 64 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/omap5/fdt.c b/arch/arm/cpu/armv7/omap5/fdt.c
> index 0493cd1..da8d59b 100644
> --- a/arch/arm/cpu/armv7/omap5/fdt.c
> +++ b/arch/arm/cpu/armv7/omap5/fdt.c
> @@ -153,13 +153,73 @@ static int ft_hs_fixup_sram(void *fdt, bd_t *bd)
>  static int ft_hs_fixup_sram(void *fdt, bd_t *bd) { return 0; }
>  #endif
>  
> +#if (CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE != 0)
> +static int ft_hs_fixup_dram(void *fdt, bd_t *bd)
> +{
> +	const char *path, *subpath;
> +	int offs;
> +	u32 sec_mem_start = CONFIG_TI_SECURE_EMIF_REGION_START;
> +	u32 sec_mem_size = CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE;
> +	fdt64_t temp[2];
> +
> +	/* If start address is zero, place at end of DRAM */
> +	if (0 == sec_mem_start)
> +		sec_mem_start =
> +			(CONFIG_SYS_SDRAM_BASE +
> +			(omap_sdram_size() - sec_mem_size));
> +
> +	/* Delete any original secure_reserved node */
> +	path = "/reserved-memory/secure_reserved";
> +	offs = fdt_path_offset(fdt, path);
> +	if (offs >= 0)
> +		fdt_del_node(fdt, offs);
> +
> +	/* Add new secure_reserved node */
> +	path = "/reserved-memory";
> +	offs = fdt_path_offset(fdt, path);
> +	if (offs < 0) {
> +		debug("Node %s not found\n", path);
> +		path = "/";
> +		subpath = "reserved-memory";
> +		fdt_path_offset(fdt, path);

Are we supposed to be doing something with the result of this call? Who
sets 'offs' in this case?

> +		offs = fdt_add_subnode(fdt, offs, subpath);
> +		if (offs < 0) {
> +			printf("Could not create %s%s node.\n", path, subpath);
> +			return 1;
> +		}
> +		path = "/reserved-memory";
> +		offs = fdt_path_offset(fdt, path);
> +	}
> +
> +	subpath = "secure_reserved";
> +	offs = fdt_add_subnode(fdt, offs, subpath);
> +	if (offs < 0) {
> +		printf("Could not create %s%s node.\n", path, subpath);
> +		return 1;
> +	}
> +
> +	temp[0] = cpu_to_fdt64(((u64)sec_mem_start));
> +	temp[1] = cpu_to_fdt64(((u64)sec_mem_size));
> +	fdt_setprop_string(fdt, offs, "compatible",
> +			   "ti,dra7-secure-memory");
> +	fdt_setprop_string(fdt, offs, "status", "okay");
> +	fdt_setprop(fdt, offs, "no-map", NULL, 0);
> +	fdt_setprop(fdt, offs, "reg", temp, sizeof(temp));
> +
> +	return 0;
> +}
> +#else
> +static int ft_hs_fixup_dram(void *fdt, bd_t *bd) { return 0; }
> +#endif
> +
>  static void ft_hs_fixups(void *fdt, bd_t *bd)
>  {
>  	/* Check we are running on an HS/EMU device type */
>  	if (GP_DEVICE != get_device_type()) {
>  		if ((ft_hs_fixup_crossbar(fdt, bd) == 0) &&
>  		    (ft_hs_disable_rng(fdt, bd) == 0) &&
> -		    (ft_hs_fixup_sram(fdt, bd) == 0))
> +		    (ft_hs_fixup_sram(fdt, bd) == 0) &&
> +		    (ft_hs_fixup_dram(fdt, bd) == 0))
>  			return;
>  	} else {
>  		printf("ERROR: Incorrect device type (GP) detected!");
> @@ -171,7 +231,7 @@ static void ft_hs_fixups(void *fdt, bd_t *bd)
>  static void ft_hs_fixups(void *fdt, bd_t *bd)
>  {
>  }
> -#endif
> +#endif /* #ifdef CONFIG_TI_SECURE_DEVICE */
>  
>  /*
>   * Place for general cpu/SoC FDT fixups. Board specific
> 

[U-Boot] [U-Boot, 1/5] ti: omap5: Add Kconfig options for secure EMIF reservations

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:20AM -0500, Daniel Allred wrote:

> Adds start address and size config options for setting aside
> a portion of the EMIF memory space for usage by security software
> (like a secure OS/TEE). There are two sizes, a total size and a
> protected size. The region is divided into protected (secure) and
> unprotected (public) regions, that are contiguous and start at the
> start address given. If the start address is zero, the intention
> is that the region will be automatically placed at the end of the
> available external DRAM space.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>

Applied to u-boot/master, thanks!

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

[U-Boot] [U-Boot, 2/5] arm: omap5: secure API for EMIF memory reservations

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:21AM -0500, Daniel Allred wrote:

> Create a few public APIs which rely on secure world ROM/HAL
> APIs for their implementation. These are intended to be used
> to reserve a portion of the EMIF memory and configure hardware
> firewalls around that region to prevent public code from
> manipulating or interfering with that memory.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

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

[U-Boot] [U-Boot,3/5] ARM: DRA7: Add secure emif setup calls

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote:

> After EMIF DRAM is configured, but before it is used,
> calls are made on secure devices to reserve any configured
> memory region needed by the secure world and then to lock the
> EMIF firewall configuration. If any other firewall
> configuration needs to be applied, it must happen before the
> lock call.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>

Applied to u-boot/master, thanks!

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

[U-Boot] [U-Boot, 4/5] ti_omap5_common: mark region of DRAM protected on HS parts

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:23AM -0500, Daniel Allred wrote:

> If the ending portion of the DRAM is reserved for secure
> world use, then u-boot cannot use this memory for its relocation
> purposes. To prevent issues, we mark this memory as PRAM and this
> prevents it from being used by u-boot at all.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>

Applied to u-boot/master, thanks!

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

[U-Boot] [U-Boot, 5/5] ARM: omap5: add fdt secure dram reservation fixup

[Tom Rini]

On Fri, Sep 02, 2016 at 12:40:24AM -0500, Daniel Allred wrote:

> Adds a secure dram reservation fixup for secure
> devices, when a region in the emif has been set aside
> for secure world use. The size is defined by the
> CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE config option.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

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