[U-Boot,v4,3/3] ARM: OMAP5: redefine arm_setup_identity_mapping

We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.

We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.

Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
Cc: Tom Rini <trini@ti.com>
---
Changes for v4:
  - Protect functions according to cache config
  - Remove page_table argument
  - Declare global data ptr and use it to retrieve page_table

Changes for v3:
  - Use definition of __arm_setup_identity_mapping() from asm/cache.h

Changes for v2:
  - Fix missing page_table argument
  - Add extern definition to fix compilation warning

 arch/arm/cpu/armv7/omap5/Makefile     |    1 +
 arch/arm/cpu/armv7/omap5/cache-cp15.c |   52 +++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c

Hi Vincent,

On Monday 07 January 2013 08:14 PM, Vincent Stehlé wrote:
> We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
> makes the first page of the identity mapping invalid.
>
> We want to unmap the region near address zero on HS OMAP devices, to avoid
> speculative accesses. Accessing this region causes security violations, which
> we want to avoid.
>
> Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
> Cc: Tom Rini <trini@ti.com>
> ---
> Changes for v4:
>    - Protect functions according to cache config
>    - Remove page_table argument
>    - Declare global data ptr and use it to retrieve page_table
>
> Changes for v3:
>    - Use definition of __arm_setup_identity_mapping() from asm/cache.h
>
> Changes for v2:
>    - Fix missing page_table argument
>    - Add extern definition to fix compilation warning
>
>   arch/arm/cpu/armv7/omap5/Makefile     |    1 +
>   arch/arm/cpu/armv7/omap5/cache-cp15.c |   52 +++++++++++++++++++++++++++++++++
>   2 files changed, 53 insertions(+)
>   create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
>
> diff --git a/arch/arm/cpu/armv7/omap5/Makefile b/arch/arm/cpu/armv7/omap5/Makefile
> index 9b261c4..49c454c 100644
> --- a/arch/arm/cpu/armv7/omap5/Makefile
> +++ b/arch/arm/cpu/armv7/omap5/Makefile
> @@ -29,6 +29,7 @@ COBJS	+= hwinit.o
>   COBJS	+= clocks.o
>   COBJS	+= emif.o
>   COBJS	+= sdram.o
> +COBJS	+= cache-cp15.o
>
>   SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
>   OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
> diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c b/arch/arm/cpu/armv7/omap5/cache-cp15.c
> new file mode 100644
> index 0000000..a9666f7
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
> @@ -0,0 +1,52 @@
> +/*
> + * (C) Copyright 2002
> + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> + *
> + * (C) Copyright 2012
> + * Vincent Stehlé, Texas Instruments, v-stehle@ti.com.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <asm/cache.h>
> +
> +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* OMAP5 specific function to set up the identity mapping. */
> +void arm_setup_identity_mapping(void)
> +{
> +	u32 *page_table = (u32 *)gd->tlb_addr;
> +
> +	/*
> +	 * Perform default mapping, which sets up an identity-mapping for all
> +	 * 4GB, rw for everyone.
> +	 */
> +	__arm_setup_identity_mapping();
> +
> +	/*
> +	 * First page (starting at 0x0) is made invalid to avoid speculative
> +	 * accesses in secure rom.
> +	 * TODO: use second level descriptors for finer grained mapping.
> +	 */
> +	page_table[0] = 0;
> +}

  First, really sorry for the late response on this.

  We had this problem of speculative aborts in the kernel uncompress code
  as well, which maps all of 4GB address space. It was solved by setting
  the non-DRAM region as non-executable(XN) and with client permissions
  to the domain in the DACR register.

  This way speculative prefetches are avoided not only to the page 0,
  but also to other read sensitive I/O regions.

  I have created a similar patch in u-boot and posted a RFC now.
  I was using your first patch [1] and rest from me.

  [1] http://www.mail-archive.com/u-boot@lists.denx.de/msg102709.html

  Please let me know your take on that.

Regards,
  Sricharan

On 01/08/2013 12:14 PM, R Sricharan wrote:
(..)
>  We had this problem of speculative aborts in the kernel uncompress code
>  as well, which maps all of 4GB address space. It was solved by setting
>  the non-DRAM region as non-executable(XN) and with client permissions
>  to the domain in the DACR register.
> 
>  This way speculative prefetches are avoided not only to the page 0,
>  but also to other read sensitive I/O regions.
> 
>  I have created a similar patch in u-boot and posted a RFC now.
>  I was using your first patch [1] and rest from me.
(..)
> 
>  Please let me know your take on that.

Hi Sricharan,

Your solution to this issue looks more elegant to me than my unmapping
page 0 completely.
  I tested your patches and they work for me on both GP (without
security) and EMU (with security) OMAP5 ES1.0 devices. I'll keep them,
thanks :) You can add my 'Tested-by' if you want:

  Tested-by: Vincent Stehlé <v-stehle@ti.com>

Best regards,

V.

Hi Vincent,

On Tuesday 08 January 2013 05:57 PM, Vincent Stehlé wrote:
> On 01/08/2013 12:14 PM, R Sricharan wrote:
> (..)
>>   We had this problem of speculative aborts in the kernel uncompress code
>>   as well, which maps all of 4GB address space. It was solved by setting
>>   the non-DRAM region as non-executable(XN) and with client permissions
>>   to the domain in the DACR register.
>>
>>   This way speculative prefetches are avoided not only to the page 0,
>>   but also to other read sensitive I/O regions.
>>
>>   I have created a similar patch in u-boot and posted a RFC now.
>>   I was using your first patch [1] and rest from me.
> (..)
>>
>>   Please let me know your take on that.
>
> Hi Sricharan,
>
> Your solution to this issue looks more elegant to me than my unmapping
> page 0 completely.
>    I tested your patches and they work for me on both GP (without
> security) and EMU (with security) OMAP5 ES1.0 devices. I'll keep them,
> thanks :) You can add my 'Tested-by' if you want:
>
  Thanks for the testing and confirming.
  Will add your <Tested-by> in the re post.

Regards,
  Sricharan