| Message ID | 20180714001117.14584-5-hebeberm@microsoft.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | Enable Windows 10 IoT Core on i.MX6 and i.MX7 | expand |
On Sat, 2018-07-14 at 00:11 +0000, Henry Beberman wrote: > From: Henry Beberman <henry.beberman@microsoft.com> > > 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?
Hi Trent, > -----Original Message----- > From: Trent Piepho <tpiepho@impinj.com> > Sent: Monday, July 16, 2018 10:33 AM > To: Henry Beberman <Henry.Beberman@microsoft.com>; u- > boot@lists.denx.de > Cc: fabio.estevam@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 +0000, Henry Beberman wrote: > > From: Henry Beberman <henry.beberman@microsoft.com> > > > > 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
Hi Henry, On 14/07/2018 02:11, Henry Beberman wrote: > From: Henry Beberman <henry.beberman@microsoft.com> > > 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 <henry.beberman@microsoft.com> > Cc: Stefano Babic <sbabic@denx.de> > Cc: Fabio Estevam <fabio.estevam@nxp.com> > --- > 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 0000000000..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, <garyj@denx.de> > + * > + * (C) Copyright 2010 > + * Texas Instruments, <www.ti.com> > + * Aneesh V <aneesh@ti.com> > + * > + * (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 0x88200000 > @@ -63,6 +64,7 @@ > #define CONFIG_SYS_SPL_MALLOC_START 0x18300000 > #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 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 0x88200000 > #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ > #define CONFIG_SYS_SPL_MALLOC_START 0x88300000 > #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 1 MB */ > +#endif /* !CONFIG_OPTEE_SPL_BOOT */ > > #endif /* CONFIG_SPL */ Best regards, Stefano Babic
Hi Stefano, > -----Original Message----- > From: Stefano Babic <sbabic@denx.de> > Sent: Tuesday, August 7, 2018 5:17 AM > To: Henry Beberman <Henry.Beberman@microsoft.com>; u- > boot@lists.denx.de > Cc: Stefano Babic <sbabic@denx.de>; Fabio Estevam > <fabio.estevam@nxp.com> > 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 <henry.beberman@microsoft.com> > > > > 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 <henry.beberman@microsoft.com> > > Cc: Stefano Babic <sbabic@denx.de> > > Cc: Fabio Estevam <fabio.estevam@nxp.com> > > --- > > 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 0000000000..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, <garyj@denx.de> > > + * > > + * (C) Copyright 2010 > > + * Texas Instruments, > <https://na01.safelinks.protection.outlook.com/?url=www.ti.com&dat > a=02%7C01%7CHenry.Beberman%40microsoft.com%7Cd96d8086ca0a4fd87d > 5308d5fc5fb1db%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6366 > 92410300862003&sdata=RFzlv9uQ5%2BBDtudKWw4aacKgE4B%2Bn1ENE > xvM9v2ANew%3D&reserved=0> > > + * Aneesh V <aneesh@ti.com> > > + * > > + * (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 0x88200000 > > @@ -63,6 +64,7 @@ > > #define CONFIG_SYS_SPL_MALLOC_START 0x18300000 > > #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 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 0x88200000 > > #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ > > #define CONFIG_SYS_SPL_MALLOC_START 0x88300000 > > #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 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: sbabic@denx.de > ========================================================== > ===========
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 0000000000..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, <garyj@denx.de> + * + * (C) Copyright 2010 + * Texas Instruments, <www.ti.com> + * Aneesh V <aneesh@ti.com> + * + * (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 0x88200000 @@ -63,6 +64,7 @@ #define CONFIG_SYS_SPL_MALLOC_START 0x18300000 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 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 0x88200000 #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START 0x88300000 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 /* 1 MB */ +#endif /* !CONFIG_OPTEE_SPL_BOOT */ #endif /* CONFIG_SPL */