Re: [U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in on-chip RAM

[Henry Beberman]

Hi Stefano,

> -Original Message-
> From: Stefano Babic 
> Sent: Tuesday, August 7, 2018 5:17 AM
> To: Henry Beberman ; u-
> b...@lists.denx.de
> Cc: Stefano Babic ; Fabio Estevam
> 
> Subject: Re: [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in
> on-chip RAM
> 
> Hi Henry,
> 
> On 14/07/2018 02:11, Henry Beberman wrote:
> > From: Henry Beberman 
> >
> > This patch is part of the i.MX Windows 10 IoT Core boot flow.
> >
> > It adds a modified linker script for SPL to keep all segments in
> > on-chip ram. This is to harden the device against potential leaks of
> > device secrets by keeping them out of DRAM.
> >
> > Additionally if CONFIG_SYS_SPL_MALLOC_START is defined, it will
> > override the CONFIG_SPL_SYS_MALLOC_SIMPLE and allocate space in
> DRAM
> > instead of on-chip ram. This patch prevents the definition of those
> > values for i.MX6 and i.MX7 SPL if CONFIG_OPTEE_SPL_BOOT is selected.
> >
> 
> I guess there should be some kind of restrictions to be set according to the
> i.MX6 variant. I have already had some projects where I get rid of all space
> available on OCRAM. The smaller i.MX6 has just 64KB of RAM - have you
> tested also on them ? I wonder if there is enough space for all i.MX variants,
> specially if some other options are enabled.

As configured now our SPL binary is only about 32KB. We haven’t run it on
any i.MX6 platforms with 64KB of OCRAM, so I'm not sure if we're bypassing
64KB at runtime.

Which i.MX6 configurations have 64KB of OCRAM?

> 
> > Signed-off-by: Henry Beberman 
> > Cc: Stefano Babic 
> > Cc: Fabio Estevam 
> > ---
> >  arch/arm/mach-imx/u-boot-spl-sram.lds | 59
> +++
> >  include/configs/imx6_spl.h|  2 ++
> >  include/configs/imx7_spl.h|  2 ++
> >  3 files changed, 63 insertions(+)
> >  create mode 100644 arch/arm/mach-imx/u-boot-spl-sram.lds
> >
> > diff --git a/arch/arm/mach-imx/u-boot-spl-sram.lds
> > b/arch/arm/mach-imx/u-boot-spl-sram.lds
> > new file mode 100644
> > index 00..dfbb4aef5d
> > --- /dev/null
> > +++ b/arch/arm/mach-imx/u-boot-spl-sram.lds
> > @@ -0,0 +1,59 @@
> > +/*
> > + * (C) Copyright 2002
> > + * Gary Jennejohn, DENX Software Engineering, 
> > + *
> > + * (C) Copyright 2010
> > + * Texas Instruments,
>  a=02%7C01%7CHenry.Beberman%40microsoft.com%7Cd96d8086ca0a4fd87d
> 5308d5fc5fb1db%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6366
> 92410300862003&sdata=RFzlv9uQ5%2BBDtudKWw4aacKgE4B%2Bn1ENE
> xvM9v2ANew%3D&reserved=0>
> > + * Aneesh V 
> > + *
> > + * (C) Copyright 2018 Microsoft Corporation
> > + *
> > + * SPDX-License-Identifier:GPL-2.0+
> > + */
> > +
> > +MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
> > +   LENGTH = CONFIG_SPL_MAX_SIZE }
> > +
> > +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm",
> > +"elf32-littlearm")
> > +OUTPUT_ARCH(arm)
> > +ENTRY(_start)
> > +SECTIONS
> > +{
> > +   .text  :
> > +   {
> > +   __start = .;
> > +   *(.vectors)
> > +   arch/arm/cpu/armv7/start.o  (.text*)
> > +   *(.text*)
> > +   } >.sram
> > +
> > +   . = ALIGN(4);
> > +   .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
> > +
> > +   . = ALIGN(4);
> > +   .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> > +
> > +   . = ALIGN(4);
> > +   .u_boot_list : {
> > +   KEEP(*(SORT(.u_boot_list*)));
> > +   } >.sram
> > +
> > +   . = ALIGN(4);
> > +   __image_copy_end = .;
> > +
> > +   .end :
> > +   {
> > +   *(.__end)
> > +   }
> > +
> > +   _image_binary_end = .;
> > +
> > +   .bss :
> > +   {
> > +   . = ALIGN(4);
> > +   __bss_start = .;
> > +   *(.bss*)
> > +   . = ALIGN(4);
> > +   __bss_end = .;
> > +   } >.sram
> > +}
> 
> This is more or less a copy of the armv8 version + bss in sram instead of
> sdram. In any case, mach-imx is not the right place because it is quite SOC
> independent. The whole i.MX do not use own lds but they are using the
> general scripts from ARM 32bit.

I agree that this isn’t i.MX specific. I took a look at the general ARM32 lds
and since it's not splitting the bss into a different region it looks like I 
just need
to set CONFIG_SPL_TEXT_BASE correctly. I'll check with my team to see why
we added this lds in the first place, but hopefully I can just get rid of it. 

Thanks,
Henry

> 
> > diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> > index 720ff045a7..4088e8a936 100644
> > --- a/include/configs/imx6_spl.h
> > +++ b/include/configs/imx6_spl.h
> > @@ -51,6 +51,7 @@
> >  # endif
> >  #endif
> >
> > +#ifndef CONFIG_OPTEE_SPL_BOOT
> >  #if defined(CONFIG_MX6SX) || defined(CONFIG_MX6SL) || \
> > defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> >  #define CONFIG_SPL_BSS_START_ADDR  0x8820
> > @@ -63,6 +64,7 @@
> >  #define CONFIG_SYS_SPL_MALLOC_START0x1830
> >  #define CONFIG_SYS_SPL_MALLOC_SIZE 0x10

Re: [U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in on-chip RAM

[Stefano Babic]

Hi Henry,

On 14/07/2018 02:11, Henry Beberman wrote:
> From: Henry Beberman 
> 
> This patch is part of the i.MX Windows 10 IoT Core boot flow.
> 
> It adds a modified linker script for SPL to keep all segments in
> on-chip ram. This is to harden the device against potential leaks of
> device secrets by keeping them out of DRAM.
> 
> Additionally if CONFIG_SYS_SPL_MALLOC_START is defined, it will
> override the CONFIG_SPL_SYS_MALLOC_SIMPLE and allocate space in DRAM
> instead of on-chip ram. This patch prevents the definition of those
> values for i.MX6 and i.MX7 SPL if CONFIG_OPTEE_SPL_BOOT is selected.
> 

I guess there should be some kind of restrictions to be set according to
the i.MX6 variant. I have already had some projects where I get rid of
all space available on OCRAM. The smaller i.MX6 has just 64KB of RAM -
have you tested also on them ? I wonder if there is enough space for all
i.MX variants, specially if some other options are enabled.

> Signed-off-by: Henry Beberman 
> Cc: Stefano Babic 
> Cc: Fabio Estevam 
> ---
>  arch/arm/mach-imx/u-boot-spl-sram.lds | 59 
> +++
>  include/configs/imx6_spl.h|  2 ++
>  include/configs/imx7_spl.h|  2 ++
>  3 files changed, 63 insertions(+)
>  create mode 100644 arch/arm/mach-imx/u-boot-spl-sram.lds
> 
> diff --git a/arch/arm/mach-imx/u-boot-spl-sram.lds 
> b/arch/arm/mach-imx/u-boot-spl-sram.lds
> new file mode 100644
> index 00..dfbb4aef5d
> --- /dev/null
> +++ b/arch/arm/mach-imx/u-boot-spl-sram.lds
> @@ -0,0 +1,59 @@
> +/*
> + * (C) Copyright 2002
> + * Gary Jennejohn, DENX Software Engineering, 
> + *
> + * (C) Copyright 2010
> + * Texas Instruments, 
> + *   Aneesh V 
> + *
> + * (C) Copyright 2018 Microsoft Corporation
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
> + LENGTH = CONFIG_SPL_MAX_SIZE }
> +
> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> + .text  :
> + {
> + __start = .;
> + *(.vectors)
> + arch/arm/cpu/armv7/start.o  (.text*)
> + *(.text*)
> + } >.sram
> +
> + . = ALIGN(4);
> + .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
> +
> + . = ALIGN(4);
> + .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> +
> + . = ALIGN(4);
> + .u_boot_list : {
> + KEEP(*(SORT(.u_boot_list*)));
> + } >.sram
> +
> + . = ALIGN(4);
> + __image_copy_end = .;
> +
> + .end :
> + {
> + *(.__end)
> + }
> +
> + _image_binary_end = .;
> +
> + .bss :
> + {
> + . = ALIGN(4);
> + __bss_start = .;
> + *(.bss*)
> + . = ALIGN(4);
> + __bss_end = .;
> + } >.sram
> +}

This is more or less a copy of the armv8 version + bss in sram instead
of sdram. In any case, mach-imx is not the right place because it is
quite SOC independent. The whole i.MX do not use own lds but they are
using the general scripts from ARM 32bit.

> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> index 720ff045a7..4088e8a936 100644
> --- a/include/configs/imx6_spl.h
> +++ b/include/configs/imx6_spl.h
> @@ -51,6 +51,7 @@
>  # endif
>  #endif
>  
> +#ifndef CONFIG_OPTEE_SPL_BOOT
>  #if defined(CONFIG_MX6SX) || defined(CONFIG_MX6SL) || \
>   defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
>  #define CONFIG_SPL_BSS_START_ADDR  0x8820
> @@ -63,6 +64,7 @@
>  #define CONFIG_SYS_SPL_MALLOC_START  0x1830
>  #define CONFIG_SYS_SPL_MALLOC_SIZE   0x10/* 1 MB */
>  #endif
> +#endif /* !CONFIG_OPTEE_SPL_BOOT */
>  #endif
>  
>  #endif
> diff --git a/include/configs/imx7_spl.h b/include/configs/imx7_spl.h
> index 1eb6cd894d..5dd4aed652 100644
> --- a/include/configs/imx7_spl.h
> +++ b/include/configs/imx7_spl.h
> @@ -46,10 +46,12 @@
>  # endif
>  #endif
>  
> +#ifndef CONFIG_OPTEE_SPL_BOOT
>  #define CONFIG_SPL_BSS_START_ADDR  0x8820
>  #define CONFIG_SPL_BSS_MAX_SIZE0x10  /* 1 MB */
>  #define CONFIG_SYS_SPL_MALLOC_START0x8830
>  #define CONFIG_SYS_SPL_MALLOC_SIZE 0x10  /* 1 MB */
> +#endif /* !CONFIG_OPTEE_SPL_BOOT */
>  
>  #endif /* CONFIG_SPL */

Best regards,
Stefano Babic

-- 
=
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: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in on-chip RAM

[Henry Beberman]

Hi Trent,

> -Original Message-
> From: Trent Piepho 
> Sent: Monday, July 16, 2018 10:33 AM
> To: Henry Beberman ; u-
> b...@lists.denx.de
> Cc: fabio.este...@nxp.com
> Subject: Re: [U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL
> entirely in on-chip RAM
> 
> On Sat, 2018-07-14 at 00:11 +, Henry Beberman wrote:
> > From: Henry Beberman 
> >
> > This patch is part of the i.MX Windows 10 IoT Core boot flow.
> >
> > It adds a modified linker script for SPL to keep all segments in
> > on-chip ram. This is to harden the device against potential leaks of
> > device secrets by keeping them out of DRAM.
> >
> > Additionally if CONFIG_SYS_SPL_MALLOC_START is defined, it will
> > override the CONFIG_SPL_SYS_MALLOC_SIMPLE and allocate space in
> DRAM
> > instead of on-chip ram. This patch prevents the definition of those
> > values for i.MX6 and i.MX7 SPL if CONFIG_OPTEE_SPL_BOOT is selected.
> 
> Is booting SPL from entirely from SRAM only useful in concert with OPTEE?
> 
> For instance, if I'm building a device that doesn't use OPTEE and yet want it 
> to
> be secure, would I want to keep the SPL entirely in SRAM?

We're adding this in anticipation of implementing the Trusted Computing Group's 
Device Identifier Composition Engine (DICE). The reason we're adding it now is 
to ensure that the SPL we're building fits within the size limitations imposed 
by SRAM.

The switch to SRAM is primarily to protect the Unique Device Secret (UDS), 
which is a device specific identifier that must only be readable by DICE. The 
UDS must not be visible in DRAM at any point. You can find more information on 
the DICE requirements in the "Hardware Requirements for a Device Identifier 
Composition Engine" specification. 
(https://trustedcomputinggroup.org/resource/hardware-requirements-for-a-device-identifier-composition-engine/)

There's value in other devices keeping SPL entirely in SDRAM as an additional 
layer of security, but it’s a tradeoff against image size and stack/malloc 
space.

Thanks,
Henry
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in on-chip RAM

[Trent Piepho]

On Sat, 2018-07-14 at 00:11 +, Henry Beberman wrote:
> From: Henry Beberman 
> 
> This patch is part of the i.MX Windows 10 IoT Core boot flow.
> 
> It adds a modified linker script for SPL to keep all segments in
> on-chip ram. This is to harden the device against potential leaks of
> device secrets by keeping them out of DRAM.
> 
> Additionally if CONFIG_SYS_SPL_MALLOC_START is defined, it will
> override the CONFIG_SPL_SYS_MALLOC_SIMPLE and allocate space in DRAM
> instead of on-chip ram. This patch prevents the definition of those
> values for i.MX6 and i.MX7 SPL if CONFIG_OPTEE_SPL_BOOT is selected.

Is booting SPL from entirely from SRAM only useful in concert with
OPTEE?

For instance, if I'm building a device that doesn't use OPTEE and yet
want it to be secure, would I want to keep the SPL entirely in SRAM?
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 04/11] spl: imx: Add optional lds to keep SPL entirely in on-chip RAM

[Henry Beberman]

From: Henry Beberman 

This patch is part of the i.MX Windows 10 IoT Core boot flow.

It adds a modified linker script for SPL to keep all segments in
on-chip ram. This is to harden the device against potential leaks of
device secrets by keeping them out of DRAM.

Additionally if CONFIG_SYS_SPL_MALLOC_START is defined, it will
override the CONFIG_SPL_SYS_MALLOC_SIMPLE and allocate space in DRAM
instead of on-chip ram. This patch prevents the definition of those
values for i.MX6 and i.MX7 SPL if CONFIG_OPTEE_SPL_BOOT is selected.

Signed-off-by: Henry Beberman 
Cc: Stefano Babic 
Cc: Fabio Estevam 
---
 arch/arm/mach-imx/u-boot-spl-sram.lds | 59 +++
 include/configs/imx6_spl.h|  2 ++
 include/configs/imx7_spl.h|  2 ++
 3 files changed, 63 insertions(+)
 create mode 100644 arch/arm/mach-imx/u-boot-spl-sram.lds

diff --git a/arch/arm/mach-imx/u-boot-spl-sram.lds 
b/arch/arm/mach-imx/u-boot-spl-sram.lds
new file mode 100644
index 00..dfbb4aef5d
--- /dev/null
+++ b/arch/arm/mach-imx/u-boot-spl-sram.lds
@@ -0,0 +1,59 @@
+/*
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering, 
+ *
+ * (C) Copyright 2010
+ * Texas Instruments, 
+ * Aneesh V 
+ *
+ * (C) Copyright 2018 Microsoft Corporation
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
+   LENGTH = CONFIG_SPL_MAX_SIZE }
+
+OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
+OUTPUT_ARCH(arm)
+ENTRY(_start)
+SECTIONS
+{
+   .text  :
+   {
+   __start = .;
+   *(.vectors)
+   arch/arm/cpu/armv7/start.o  (.text*)
+   *(.text*)
+   } >.sram
+
+   . = ALIGN(4);
+   .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
+
+   . = ALIGN(4);
+   .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
+
+   . = ALIGN(4);
+   .u_boot_list : {
+   KEEP(*(SORT(.u_boot_list*)));
+   } >.sram
+
+   . = ALIGN(4);
+   __image_copy_end = .;
+
+   .end :
+   {
+   *(.__end)
+   }
+
+   _image_binary_end = .;
+
+   .bss :
+   {
+   . = ALIGN(4);
+   __bss_start = .;
+   *(.bss*)
+   . = ALIGN(4);
+   __bss_end = .;
+   } >.sram
+}
diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
index 720ff045a7..4088e8a936 100644
--- a/include/configs/imx6_spl.h
+++ b/include/configs/imx6_spl.h
@@ -51,6 +51,7 @@
 # endif
 #endif
 
+#ifndef CONFIG_OPTEE_SPL_BOOT
 #if defined(CONFIG_MX6SX) || defined(CONFIG_MX6SL) || \
defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
 #define CONFIG_SPL_BSS_START_ADDR  0x8820
@@ -63,6 +64,7 @@
 #define CONFIG_SYS_SPL_MALLOC_START0x1830
 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */
 #endif
+#endif /* !CONFIG_OPTEE_SPL_BOOT */
 #endif
 
 #endif
diff --git a/include/configs/imx7_spl.h b/include/configs/imx7_spl.h
index 1eb6cd894d..5dd4aed652 100644
--- a/include/configs/imx7_spl.h
+++ b/include/configs/imx7_spl.h
@@ -46,10 +46,12 @@
 # endif
 #endif
 
+#ifndef CONFIG_OPTEE_SPL_BOOT
 #define CONFIG_SPL_BSS_START_ADDR  0x8820
 #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */
 #define CONFIG_SYS_SPL_MALLOC_START0x8830
 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */
+#endif /* !CONFIG_OPTEE_SPL_BOOT */
 
 #endif /* CONFIG_SPL */
 
-- 
2.16.2.gvfs.1.33.gf5370f1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot