Skip to contentSkip to conversation

Update OMAP5 "unmap first page" patch series

U-Boot New Vincent Stehlé 2 participants 7 messages Jan 7, 2013 – Jan 8, 2013 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 92476020c6e1ef888207ca6f661c48068050efbf

Vincent Stehlé

author
#1 lore raw

Hi,

Here is a proposed updated patch series to unmap page 0 on OMAP5, rebased on denx/master:

[PATCH v4 1/3] ARM: cache: declare set_section_dcache [PATCH v4 2/3] ARM: cache: introduce weak arm_setup_identity_mapping [PATCH v4 3/3] ARM: OMAP5: redefine arm_setup_identity_mapping

Necessary adaptations have been done to follow set_section_dcache() changes.

Happy new year,

V.

author
#2 lore raw

PATCH v4 1/3 ARM: cache: declare set_section_dcache

We declare the set_section_dcache function globally in the cache header, for later use by e.g. machine specific code.

Signed-off-by Vincent Stehl? <[email protected]>
Cc Tom Rini <[email protected]>
---
 arch/arm/include/asm/cache.h |    1 +
 1 file changed, 1 insertion(+)
arch/arm/include/asm/cache.h +1 -0
index eef6a5a..416d2c8 100644
@@ -41,6 +41,7 @@ static inline void invalidate_l2_cache(void)
 
 void l2_cache_enable(void);
 void l2_cache_disable(void);
+void set_section_dcache(int section, enum dcache_option option);
 
 /*
  * The current upper bound for ARM L1 data cache line sizes is 64 bytes.  We
Signature · 1 line
1.7.9.5
author
#3 lore raw

PATCH v4 2/3 ARM: cache: introduce weak arm_setup_identity_mapping

In reply to Vincent Stehlé

Separate the MMU identity mapping for ARM in a weak function, to allow redefinition with platform specific function.

This is motivated by the need 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? <[email protected]>
Cc Tom Rini <[email protected]>

---
Changes for v4;
- Use set_section_dcache() function
- Remove page_table argument

Changes for v3:
- Add definition of __arm_setup_identity_mapping() into asm/cache.h
- Fix comments style

 arch/arm/include/asm/cache.h |    1 +
 arch/arm/lib/cache-cp15.c    |   22 +++++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)
arch/arm/include/asm/cache.h +1 -0
index 416d2c8..b898dc5 100644
@@ -42,6 +42,7 @@ static inline void invalidate_l2_cache(void)
 void l2_cache_enable(void);
 void l2_cache_disable(void);
 void set_section_dcache(int section, enum dcache_option option);
+void __arm_setup_identity_mapping(void);
 
 /*
  * The current upper bound for ARM L1 data cache line sizes is 64 bytes.  We
arch/arm/lib/cache-cp15.c +19 -3
index 6edf815..fa7b0c5 100644
@@ -23,6 +23,8 @@
 
 #include <common.h>
 #include <asm/system.h>
+#include <asm/cache.h>
+#include <linux/compiler.h>
 
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
 

@@ -34,6 +36,17 @@ void __arm_init_before_mmu(void)
 void arm_init_before_mmu(void)
 	__attribute__((weak, alias("__arm_init_before_mmu")));
 
+void __arm_setup_identity_mapping(void)
+{
+	int i;
+
+	/* Set up an identity-mapping for all 4GB, rw for everyone */
+	for (i = 0; i < 4096; i++)
+		set_section_dcache(i, DCACHE_OFF);
+}
+__weak void arm_setup_identity_mapping(void)
+	__attribute__((alias("__arm_setup_identity_mapping")));
+
 static void cp_delay (void)
 {
 	volatile int i;

@@ -101,9 +114,12 @@ static inline void mmu_setup(void)
 	u32 reg;
 
 	arm_init_before_mmu();
-	/* Set up an identity-mapping for all 4GB, rw for everyone */
-	for (i = 0; i < 4096; i++)
-		set_section_dcache(i, DCACHE_OFF);
+
+	/*
+	 * Set up an identity-mapping. Default version maps all 4GB rw for
+	 * everyone
+	 */
+	arm_setup_identity_mapping();
 
 	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
 		dram_bank_mmu_setup(i);
Signature · 1 line
1.7.9.5
author
#4 lore raw

PATCH v4 3/3 ARM: OMAP5: redefine arm_setup_identity_mapping

In reply to Vincent Stehlé

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? <[email protected]>
Cc Tom Rini <[email protected]>

---
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
arch/arm/cpu/armv7/omap5/Makefile +1 -0
index 9b261c4..49c454c 100644
@@ -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))
arch/arm/cpu/armv7/omap5/cache-cp15.c new file +52 -0

52 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#5 lore raw

PATCH v4 3/3 ARM: OMAP5: redefine arm_setup_identity_mapping

Hi Vincent,

Quoted 86 lines · Vincent Stehl?

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? <[email protected]>
Cc Tom Rini <[email protected]>

---
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

arch/arm/cpu/armv7/omap5/Makefile +1 -0
index 9b261c4..49c454c 100644
@@ -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))
arch/arm/cpu/armv7/omap5/cache-cp15.c new file +41 -0
new file mode 100644
index 0000000..a9666f7
@@ -0,0 +1,52 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehl?, Texas Instruments, v-stehle at 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.

arch/arm/cpu/armv7/omap5/cache-cp15.c line 51

Quoted 10 lines
arch/arm/cpu/armv7/omap5/cache-cp15.c new file +10 -0
+	 */
+	__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 at lists.denx.de/msg102709.html

Please let me know your take on that.

Regards,
Sricharan

author
#6 lore raw

PATCH v4 3/3 ARM: OMAP5: redefine arm_setup_identity_mapping

On 01/08/2013 12:14 PM, R Sricharan wrote:
(..)

Quoted 10 lines

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.

(..)

Quoted 2 lines

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? <[email protected]>

Best regards,

V.

#7 lore raw

PATCH v4 3/3 ARM: OMAP5: redefine arm_setup_identity_mapping

Hi Vincent,

Quoted 16 lines · Vincent Stehl?

On 01/08/2013 12:14 PM, R Sricharan wrote:
(..)

Quoted 10 lines

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.

(..)

Quoted 2 lines

Please let me know your take on that.

Quoted 8 lines

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