Skip to contentSkip to conversation

armv8/cache: Fix page table creation

PATCH New Thierry Reding 4 participants 20 messages Mar 20, 2015 – Jul 3, 2015 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/6 armv8/cache: Fix page table creation

From: Thierry Reding <[email protected]>

While generating the page tables, a running integer index is shifted by SECTION_SHIFT (29) and causes overflow for any integer bigger than 7. The page tables therefore alias to the same 8 sections and cause U-Boot to hang once the MMU is enabled.

Fix this by making the index a 64-bit unsigned integer and so avoid the overflow.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/cache_v8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +2 -2
index c5ec5297cd39..254a629a3b8c 100644
@@ -25,9 +25,9 @@ void set_pgtable_section(u64 *page_table, u64 index, u64 section,
 /* to activate the MMU we need to set up virtual memory */
 static void mmu_setup(void)
 {
-	int i, j, el;
 	bd_t *bd = gd->bd;
-	u64 *page_table = (u64 *)gd->arch.tlb_addr;
+	u64 *page_table = (u64 *)gd->arch.tlb_addr, i, j;
+	int el;
 
 	/* Setup an identity-mapping for all spaces */
 	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
Signature · 1 line
2.3.2
#1 lore raw

PATCH 2/6 armv8: Implement CONFIG_SYS_MALLOC_F_LEN support

From: Thierry Reding <[email protected]>

Implement early malloc() support in a similar way as on 32-bit ARM. This is required for 64-bit Tegra SoCs that initialize from the device tree just like the earlier 32-bit SoCs.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/config.h |  4 ----
 arch/arm/lib/crt0_64.S        | 16 ++++++++++++++--
 2 files changed, 14 insertions(+), 6 deletions(-)
arch/arm/include/asm/config.h +0 -4
index be80434dee4d..7a34a0186cb1 100644
@@ -7,10 +7,6 @@
 #ifndef _ASM_CONFIG_H_
 #define _ASM_CONFIG_H_
 
-#ifdef __aarch64__
-#define CONFIG_SYS_GENERIC_GLOBAL_DATA
-#endif
-
 #define CONFIG_LMB
 #define CONFIG_SYS_BOOT_RAMDISK_HIGH
 
arch/arm/lib/crt0_64.S +14 -2
index 77563967e517..010efdbf8f3f 100644
@@ -62,9 +62,21 @@ ENTRY(_main)
  * Set up initial C runtime environment and call board_init_f(0).
  */
 	ldr	x0, =(CONFIG_SYS_INIT_SP_ADDR)
+	mov	x1, x0
 	sub	x0, x0, #GD_SIZE	/* allocate one GD above SP */
-	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
-	mov	x18, sp			/* GD is above SP */
+	bic	x0, x0, #0xf		/* 16-byte alignment for ABI compliance */
+	mov	x18, x0			/* GD is above SP */
+	mov	sp, x0
+clr_gd:
+	str	xzr, [x0], #8
+	cmp	x0, x1
+	b.lo	clr_gd
+#if defined(CONFIG_SYS_MALLOC_F_LEN)
+	ldr	x0, =CONFIG_SYS_MALLOC_F_LEN
+	sub	x0, sp, x0
+	str	x0, [x18, #GD_MALLOC_BASE]
+#endif
+3:
 	mov	x0, #0
 	bl	board_init_f
 
Signature · 1 line
2.3.2
#2 lore raw

PATCH 2/6 armv8: Implement CONFIG_SYS_MALLOC_F_LEN support

hi Thierry,

Quoted 49 lines

-----Original Messages-----
From: "Thierry Reding" <[email protected]>
Sent Time: 2015-03-20 19:47:49 (Friday)
To: u-boot at lists.denx.de
Cc: "Marc Zyngier" <[email protected]>
Subject: [U-Boot] [PATCH 2/6] armv8: Implement CONFIG_SYS_MALLOC_F_LEN support

From: Thierry Reding <[email protected]>

Implement early malloc() support in a similar way as on 32-bit ARM. This is required for 64-bit Tegra SoCs that initialize from the device tree just like the earlier 32-bit SoCs.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/config.h |  4 ----
 arch/arm/lib/crt0_64.S        | 16 ++++++++++++++--
 2 files changed, 14 insertions(+), 6 deletions(-)
arch/arm/include/asm/config.h +0 -4
index be80434dee4d..7a34a0186cb1 100644
@@ -7,10 +7,6 @@
 #ifndef _ASM_CONFIG_H_
 #define _ASM_CONFIG_H_
 
-#ifdef __aarch64__
-#define CONFIG_SYS_GENERIC_GLOBAL_DATA
-#endif
-
 #define CONFIG_LMB
 #define CONFIG_SYS_BOOT_RAMDISK_HIGH
 
arch/arm/lib/crt0_64.S +2 -2
index 77563967e517..010efdbf8f3f 100644
@@ -62,9 +62,21 @@ ENTRY(_main)
  * Set up initial C runtime environment and call board_init_f(0).
  */
 	ldr	x0, =(CONFIG_SYS_INIT_SP_ADDR)
+	mov	x1, x0
 	sub	x0, x0, #GD_SIZE	/* allocate one GD above SP */
-	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
-	mov	x18, sp			/* GD is above SP */
+	bic	x0, x0, #0xf		/* 16-byte alignment for ABI compliance */

arch/arm/lib/crt0_64.S ENTRY(_main) · line 77

Quoted 10 lines
arch/arm/lib/crt0_64.S +10 -0
+	mov	x18, x0			/* GD is above SP */
+	mov	sp, x0
+clr_gd:
+	str	xzr, [x0], #8
+	cmp	x0, x1
+	b.lo	clr_gd
+#if defined(CONFIG_SYS_MALLOC_F_LEN)
+	ldr	x0, =CONFIG_SYS_MALLOC_F_LEN
+	sub	x0, sp, x0
+	str	x0, [x18, #GD_MALLOC_BASE]

sp should substract CONFIG_SYS_MALLOC_F_LEN also. Actually my previous patch did the same work, but got no reply.

Quoted 12 lines
arch/arm/lib/crt0_64.S +2 -0
+#endif
+3:
 	mov	x0, #0
 	bl	board_init_f
 
Signature · 6 lines
2.3.2

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
#3 lore raw

PATCH 3/6 armv8/mmu: Clean up TCR programming

In reply to Thierry Reding

From: Thierry Reding <[email protected]>

Use the inner shareable attribute for memory, which makes more sense considering that this code is called when caches are being enabled.

While at it, fix the values for the shareability attribute field to match the documentation.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/armv8/mmu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/include/asm/armv8/mmu.h +4 -4
index 4b9cb5296572..6d42f5533a74 100644
@@ -93,8 +93,8 @@
 #define TCR_ORGN_WBNWA		(3 << 10)
 #define TCR_ORGN_MASK		(3 << 10)
 #define TCR_SHARED_NON		(0 << 12)
-#define TCR_SHARED_OUTER	(1 << 12)
-#define TCR_SHARED_INNER	(2 << 12)
+#define TCR_SHARED_OUTER	(2 << 12)
+#define TCR_SHARED_INNER	(3 << 12)
 #define TCR_TG0_4K		(0 << 14)
 #define TCR_TG0_64K		(1 << 14)
 #define TCR_TG0_16K		(2 << 14)
@@ -102,9 +102,9 @@
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 
-/* PTWs cacheable, inner/outer WBWA and non-shareable */
+/* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
-				TCR_SHARED_NON |	\
+				TCR_SHARED_INNER |	\
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
Signature · 1 line
2.3.2
#4 lore raw

PATCH 3/6 armv8/mmu: Clean up TCR programming

Quoted 33 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Use the inner shareable attribute for memory, which makes more sense considering that this code is called when caches are being enabled.

While at it, fix the values for the shareability attribute field to match the documentation.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/armv8/mmu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/include/asm/armv8/mmu.h +2 -2
index 4b9cb5296572..6d42f5533a74 100644
@@ -93,8 +93,8 @@
 #define TCR_ORGN_WBNWA		(3 << 10)
 #define TCR_ORGN_MASK		(3 << 10)
 #define TCR_SHARED_NON		(0 << 12)
-#define TCR_SHARED_OUTER	(1 << 12)
-#define TCR_SHARED_INNER	(2 << 12)
+#define TCR_SHARED_OUTER	(2 << 12)
+#define TCR_SHARED_INNER	(3 << 12)
 #define TCR_TG0_4K		(0 << 14)
 #define TCR_TG0_64K		(1 << 14)
 #define TCR_TG0_16K		(2 << 14)
@@ -102,9 +102,9 @@
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */

arch/arm/include/asm/armv8/mmu.h line 110

Quoted 10 lines
arch/arm/include/asm/armv8/mmu.h +2 -2
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 
-/* PTWs cacheable, inner/outer WBWA and non-shareable */
+/* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
-				TCR_SHARED_NON |	\
+				TCR_SHARED_INNER |	\
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))

One thing though: the architecture doesn't mandate 64k pages to be implemented by the HW. Actually, it doesn't mandate any particular page size, you just have to implement at least one (4k, 16k or 64k).

It would be good to test if 64k pages are implemented (by testing ID_AA64MMFR0_EL1) and not try to enable caches if not, possibly displaying a warning for the unsuspecting u-boot hacker.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#5 lore raw

PATCH 3/6 armv8/mmu: Clean up TCR programming

Hello Marc,

Quoted 47 lines · Marc Zyngier
Quoted 33 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Use the inner shareable attribute for memory, which makes more sense considering that this code is called when caches are being enabled.

While at it, fix the values for the shareability attribute field to match the documentation.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/armv8/mmu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/include/asm/armv8/mmu.h +2 -2
index 4b9cb5296572..6d42f5533a74 100644
@@ -93,8 +93,8 @@
 #define TCR_ORGN_WBNWA		(3 << 10)
 #define TCR_ORGN_MASK		(3 << 10)
 #define TCR_SHARED_NON		(0 << 12)
-#define TCR_SHARED_OUTER	(1 << 12)
-#define TCR_SHARED_INNER	(2 << 12)
+#define TCR_SHARED_OUTER	(2 << 12)
+#define TCR_SHARED_INNER	(3 << 12)
 #define TCR_TG0_4K		(0 << 14)
 #define TCR_TG0_64K		(1 << 14)
 #define TCR_TG0_16K		(2 << 14)
@@ -102,9 +102,9 @@
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */

arch/arm/include/asm/armv8/mmu.h line 110

Quoted 10 lines
arch/arm/include/asm/armv8/mmu.h +2 -2
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 
-/* PTWs cacheable, inner/outer WBWA and non-shareable */
+/* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
-				TCR_SHARED_NON |	\
+				TCR_SHARED_INNER |	\
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
Quoted 7 lines

One thing though: the architecture doesn't mandate 64k pages to be implemented by the HW. Actually, it doesn't mandate any particular page size, you just have to implement at least one (4k, 16k or 64k).

It would be good to test if 64k pages are implemented (by testing ID_AA64MMFR0_EL1) and not try to enable caches if not, possibly displaying a warning for the unsuspecting u-boot hacker.

So Marc, is this a request for a change, or is the patch applicable as it is?

Amicalement,

Signature · 1 line
Albert.
#6 lore raw

PATCH 3/6 armv8/mmu: Clean up TCR programming

Hi Albert,

Quoted 55 lines · Albert ARIBAUD

Hello Marc,

Quoted 51 lines · Marc Zyngier
Quoted 33 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Use the inner shareable attribute for memory, which makes more sense considering that this code is called when caches are being enabled.

While at it, fix the values for the shareability attribute field to match the documentation.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/armv8/mmu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/include/asm/armv8/mmu.h +2 -2
index 4b9cb5296572..6d42f5533a74 100644
@@ -93,8 +93,8 @@
 #define TCR_ORGN_WBNWA		(3 << 10)
 #define TCR_ORGN_MASK		(3 << 10)
 #define TCR_SHARED_NON		(0 << 12)
-#define TCR_SHARED_OUTER	(1 << 12)
-#define TCR_SHARED_INNER	(2 << 12)
+#define TCR_SHARED_OUTER	(2 << 12)
+#define TCR_SHARED_INNER	(3 << 12)
 #define TCR_TG0_4K		(0 << 14)
 #define TCR_TG0_64K		(1 << 14)
 #define TCR_TG0_16K		(2 << 14)
@@ -102,9 +102,9 @@
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */

arch/arm/include/asm/armv8/mmu.h line 110

Quoted 10 lines
arch/arm/include/asm/armv8/mmu.h +2 -2
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 
-/* PTWs cacheable, inner/outer WBWA and non-shareable */
+/* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
-				TCR_SHARED_NON |	\
+				TCR_SHARED_INNER |	\
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))

One thing though: the architecture doesn't mandate 64k pages to be implemented by the HW. Actually, it doesn't mandate any particular page size, you just have to implement at least one (4k, 16k or 64k).

Quoted 6 lines
Quoted 3 lines

It would be good to test if 64k pages are implemented (by testing ID_AA64MMFR0_EL1) and not try to enable caches if not, possibly displaying a warning for the unsuspecting u-boot hacker.

So Marc, is this a request for a change, or is the patch applicable as it is?

No, the patch is extremely valuable as it is, and should be applied. My remark is probably not applicable for generally available CPUs, and could be implemented as a later improvement.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#7 lore raw

PATCH 3/6 armv8/mmu: Clean up TCR programming

In reply to Thierry Reding
Quoted 54 lines

-----Original Messages-----
From: "Thierry Reding" <[email protected]>
Sent Time: 2015-03-20 19:47:50 (Friday)
To: u-boot at lists.denx.de
Cc: "Marc Zyngier" <[email protected]>
Subject: [U-Boot] [PATCH 3/6] armv8/mmu: Clean up TCR programming

From: Thierry Reding <[email protected]>

Use the inner shareable attribute for memory, which makes more sense considering that this code is called when caches are being enabled.

While at it, fix the values for the shareability attribute field to match the documentation.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/include/asm/armv8/mmu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/include/asm/armv8/mmu.h +4 -4
index 4b9cb5296572..6d42f5533a74 100644
@@ -93,8 +93,8 @@
 #define TCR_ORGN_WBNWA		(3 << 10)
 #define TCR_ORGN_MASK		(3 << 10)
 #define TCR_SHARED_NON		(0 << 12)
-#define TCR_SHARED_OUTER	(1 << 12)
-#define TCR_SHARED_INNER	(2 << 12)
+#define TCR_SHARED_OUTER	(2 << 12)
+#define TCR_SHARED_INNER	(3 << 12)
 #define TCR_TG0_4K		(0 << 14)
 #define TCR_TG0_64K		(1 << 14)
 #define TCR_TG0_16K		(2 << 14)
@@ -102,9 +102,9 @@
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 
-/* PTWs cacheable, inner/outer WBWA and non-shareable */
+/* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
-				TCR_SHARED_NON |	\
+				TCR_SHARED_INNER |	\
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
Signature · 1 line
2.3.2
#8 lore raw

PATCH 4/6 armv8/mmu: Set bits marked RES1 in TCR

In reply to Thierry Reding

From: Thierry Reding <[email protected]>

For EL3 and EL2, the documentation says that bits 31 and 23 are reserved but should be written as 1.

For EL1, only bit 23 is not reserved, so only write bit 31 as 1.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/cache_v8.c    | 6 +++---
 arch/arm/include/asm/armv8/mmu.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +3 -3
index 254a629a3b8c..f9b04057f696 100644
@@ -50,15 +50,15 @@ static void mmu_setup(void)
 	el = current_el();
 	if (el == 1) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL1_IPS_BITS,
+				  TCR_EL1_RSVD | TCR_FLAGS | TCR_EL1_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else if (el == 2) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL2_IPS_BITS,
+				  TCR_EL2_RSVD | TCR_FLAGS | TCR_EL2_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL3_IPS_BITS,
+				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	}
 	/* enable the mmu */
arch/arm/include/asm/armv8/mmu.h +4 -0
index 6d42f5533a74..8e577b34e4ba 100644
@@ -109,6 +109,10 @@
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
 
+#define TCR_EL1_RSVD		(1 << 31)
+#define TCR_EL2_RSVD		(1 << 31 | 1 << 23)
+#define TCR_EL3_RSVD		(1 << 31 | 1 << 23)
+
 #ifndef __ASSEMBLY__
 void set_pgtable_section(u64 *page_table, u64 index,
 			 u64 section, u64 memory_type);
Signature · 1 line
2.3.2
#9 lore raw

PATCH 4/6 armv8/mmu: Set bits marked RES1 in TCR

Quoted 44 lines · Thierry Reding

From: Thierry Reding <[email protected]>

For EL3 and EL2, the documentation says that bits 31 and 23 are reserved but should be written as 1.

For EL1, only bit 23 is not reserved, so only write bit 31 as 1.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/cache_v8.c    | 6 +++---
 arch/arm/include/asm/armv8/mmu.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +3 -3
index 254a629a3b8c..f9b04057f696 100644
@@ -50,15 +50,15 @@ static void mmu_setup(void)
 	el = current_el();
 	if (el == 1) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL1_IPS_BITS,
+				  TCR_EL1_RSVD | TCR_FLAGS | TCR_EL1_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else if (el == 2) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL2_IPS_BITS,
+				  TCR_EL2_RSVD | TCR_FLAGS | TCR_EL2_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL3_IPS_BITS,
+				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	}
 	/* enable the mmu */
arch/arm/include/asm/armv8/mmu.h +0 -0
index 6d42f5533a74..8e577b34e4ba 100644

arch/arm/include/asm/armv8/mmu.h line 118

Quoted 10 lines
arch/arm/include/asm/armv8/mmu.h +4 -0
@@ -109,6 +109,10 @@
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
 
+#define TCR_EL1_RSVD		(1 << 31)
+#define TCR_EL2_RSVD		(1 << 31 | 1 << 23)
+#define TCR_EL3_RSVD		(1 << 31 | 1 << 23)
+
 #ifndef __ASSEMBLY__
 void set_pgtable_section(u64 *page_table, u64 index,
 			 u64 section, u64 memory_type);
	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#10 lore raw

PATCH 4/6 armv8/mmu: Set bits marked RES1 in TCR

In reply to Thierry Reding
Quoted 65 lines

-----Original Messages-----
From: "Thierry Reding" <[email protected]>
Sent Time: 2015-03-20 19:47:51 (Friday)
To: u-boot at lists.denx.de
Cc: "Marc Zyngier" <[email protected]>
Subject: [U-Boot] [PATCH 4/6] armv8/mmu: Set bits marked RES1 in TCR

From: Thierry Reding <[email protected]>

For EL3 and EL2, the documentation says that bits 31 and 23 are reserved but should be written as 1.

For EL1, only bit 23 is not reserved, so only write bit 31 as 1.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/cache_v8.c    | 6 +++---
 arch/arm/include/asm/armv8/mmu.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +3 -3
index 254a629a3b8c..f9b04057f696 100644
@@ -50,15 +50,15 @@ static void mmu_setup(void)
 	el = current_el();
 	if (el == 1) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL1_IPS_BITS,
+				  TCR_EL1_RSVD | TCR_FLAGS | TCR_EL1_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else if (el == 2) {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL2_IPS_BITS,
+				  TCR_EL2_RSVD | TCR_FLAGS | TCR_EL2_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	} else {
 		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_FLAGS | TCR_EL3_IPS_BITS,
+				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	}
 	/* enable the mmu */
arch/arm/include/asm/armv8/mmu.h +4 -0
index 6d42f5533a74..8e577b34e4ba 100644
@@ -109,6 +109,10 @@
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
 
+#define TCR_EL1_RSVD		(1 << 31)
+#define TCR_EL2_RSVD		(1 << 31 | 1 << 23)
+#define TCR_EL3_RSVD		(1 << 31 | 1 << 23)
+
 #ifndef __ASSEMBLY__
 void set_pgtable_section(u64 *page_table, u64 index,
 			 u64 section, u64 memory_type);
Signature · 1 line
2.3.2
#11 lore raw

PATCH 5/6 armv8/gic: Fix GIC v2 initialization

In reply to Thierry Reding

From: Thierry Reding <[email protected]>

Initialize all GICD_IGROUPRn registers and set up GICC_CTLR to enable interrupts to the primary CPU. This fixes issues seen after booting a Linux kernel from U-Boot.

---
 arch/arm/lib/gic_64.S | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
arch/arm/lib/gic_64.S +9 -1
index a3e18f7713e5..62d0022408bc 100644
@@ -46,11 +46,19 @@ ENTRY(gic_init_secure)
 	ldr	w9, [x0, GICD_TYPER]
 	and	w10, w9, #0x1f		/* ITLinesNumber */
 	cbz	w10, 1f			/* No SPIs */
-	add	x11, x0, (GICD_IGROUPRn + 4)
+	add	x11, x0, GICD_IGROUPRn
 	mov	w9, #~0			/* Config SPIs as Grp1 */
+	str	w9, [x11], #0x4
 0:	str	w9, [x11], #0x4
 	sub	w10, w10, #0x1
 	cbnz	w10, 0b
+
+	ldr	x1, =GICC_BASE		/* GICC_CTLR */
+	mov	w0, #3			/* EnableGrp0 | EnableGrp1 */
+	str	w0, [x1]
+
+	mov	w0, #1 << 7		/* allow NS access to GICC_PMR */
+	str	w0, [x1, #4]		/* GICC_PMR */
 #endif
 1:
 	ret
Signature · 1 line
2.3.2
#12 lore raw

PATCH 5/6 armv8/gic: Fix GIC v2 initialization

Hi Thierry,

Quoted 32 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Initialize all GICD_IGROUPRn registers and set up GICC_CTLR to enable interrupts to the primary CPU. This fixes issues seen after booting a Linux kernel from U-Boot.

---
 arch/arm/lib/gic_64.S | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
arch/arm/lib/gic_64.S +2 -1
index a3e18f7713e5..62d0022408bc 100644
@@ -46,11 +46,19 @@ ENTRY(gic_init_secure)
 	ldr	w9, [x0, GICD_TYPER]
 	and	w10, w9, #0x1f		/* ITLinesNumber */
 	cbz	w10, 1f			/* No SPIs */
-	add	x11, x0, (GICD_IGROUPRn + 4)
+	add	x11, x0, GICD_IGROUPRn
 	mov	w9, #~0			/* Config SPIs as Grp1 */
+	str	w9, [x11], #0x4
 0:	str	w9, [x11], #0x4
 	sub	w10, w10, #0x1
 	cbnz	w10, 0b

arch/arm/lib/gic_64.S ENTRY(gic_init_secure) · line 64

Quoted 10 lines
arch/arm/lib/gic_64.S +7 -0
+
+	ldr	x1, =GICC_BASE		/* GICC_CTLR */
+	mov	w0, #3			/* EnableGrp0 | EnableGrp1 */
+	str	w0, [x1]
+
+	mov	w0, #1 << 7		/* allow NS access to GICC_PMR */
+	str	w0, [x1, #4]		/* GICC_PMR */
 #endif
 1:
 	ret

Looking at the code that currently sits in the u-boot repo, it looks like you are duplicating functionality that is provided by gic_init_secure_percpu.

Could your boot failure be a case of this function not being called on all CPUs?

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#13 lore raw

PATCH 6/6 armv8: Allow SoCs to override the generic timer

In reply to Thierry Reding

From: Thierry Reding <[email protected]>

Some SoCs come with a custom timer interface, so allow them to use that instead.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/generic_timer.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/cpu/armv8/generic_timer.c +2 -0
index 223b95e210ed..ab8573fc7cef 100644
@@ -9,6 +9,7 @@
 #include <command.h>
 #include <asm/system.h>
 
+#ifndef CONFIG_SYS_TIMER_COUNTER
 /*
  * Generic timer implementation of get_tbclk()
  */
@@ -29,3 +30,4 @@ unsigned long timer_read_counter(void)
 	asm volatile("mrs %0, cntpct_el0" : "=r" (cntpct));
 	return cntpct;
 }
+#endif
Signature · 1 line
2.3.2
#14 lore raw

PATCH 6/6 armv8: Allow SoCs to override the generic timer

Quoted 26 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Some SoCs come with a custom timer interface, so allow them to use that instead.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/generic_timer.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/cpu/armv8/generic_timer.c +1 -0
index 223b95e210ed..ab8573fc7cef 100644
@@ -9,6 +9,7 @@
 #include <command.h>
 #include <asm/system.h>
 
+#ifndef CONFIG_SYS_TIMER_COUNTER
 /*
  * Generic timer implementation of get_tbclk()
  */

arch/arm/cpu/armv8/generic_timer.c unsigned long timer_read_counter(void) · line 33

Quoted 4 lines
arch/arm/cpu/armv8/generic_timer.c +1 -0
@@ -29,3 +30,4 @@ unsigned long timer_read_counter(void)
 	asm volatile("mrs %0, cntpct_el0" : "=r" (cntpct));
 	return cntpct;
 }
+#endif

Does it mean that in this case, the generic timers are not in a working state? For ARMv8, it would make a lot more sense to make sure that the basic CPU stuff is actually in a working state, and avoid the madness that we have on ARMv7...

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#15 lore raw

PATCH 6/6 armv8: Allow SoCs to override the generic timer

In reply to Thierry Reding

hi Thierry,

Quoted 11 lines

-----Original Messages-----
From: "Thierry Reding" <[email protected]>
Sent Time: 2015-03-20 19:47:53 (Friday)
To: u-boot at lists.denx.de
Cc: "Marc Zyngier" <[email protected]>
Subject: [U-Boot] [PATCH 6/6] armv8: Allow SoCs to override the generic timer

From: Thierry Reding <[email protected]>

Some SoCs come with a custom timer interface, so allow them to use that instead.

Arch timer is always available when core running,
It's better to use arch timer instead of other custom timer interface.

Quoted 28 lines
Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/generic_timer.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/cpu/armv8/generic_timer.c +2 -0
index 223b95e210ed..ab8573fc7cef 100644
@@ -9,6 +9,7 @@
 #include <command.h>
 #include <asm/system.h>
 
+#ifndef CONFIG_SYS_TIMER_COUNTER
 /*
  * Generic timer implementation of get_tbclk()
  */
@@ -29,3 +30,4 @@ unsigned long timer_read_counter(void)
 	asm volatile("mrs %0, cntpct_el0" : "=r" (cntpct));
 	return cntpct;
 }
+#endif
Signature · 1 line
2.3.2

Yours.

#16 lore raw

PATCH 1/6 armv8/cache: Fix page table creation

In reply to Thierry Reding
Quoted 7 lines · Thierry Reding

From: Thierry Reding <[email protected]>

While generating the page tables, a running integer index is shifted by SECTION_SHIFT (29) and causes overflow for any integer bigger than 7. The page tables therefore alias to the same 8 sections and cause U-Boot to hang once the MMU is enabled.

Quoted 6 lines

Fix this by making the index a 64-bit unsigned integer and so avoid the overflow.

Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
Quoted 21 lines
---
 arch/arm/cpu/armv8/cache_v8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +2 -2
index c5ec5297cd39..254a629a3b8c 100644
@@ -25,9 +25,9 @@ void set_pgtable_section(u64 *page_table, u64 index, u64 section,
 /* to activate the MMU we need to set up virtual memory */
 static void mmu_setup(void)
 {
-	int i, j, el;
 	bd_t *bd = gd->bd;
-	u64 *page_table = (u64 *)gd->arch.tlb_addr;
+	u64 *page_table = (u64 *)gd->arch.tlb_addr, i, j;
+	int el;
 
 	/* Setup an identity-mapping for all spaces */
 	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
Signature · 1 line
Jazz is not dead. It just smells funny...
#17 lore raw

PATCH 1/6 armv8/cache: Fix page table creation

In reply to Thierry Reding
Quoted 12 lines

-----Original Messages-----
From: "Thierry Reding" <[email protected]>
Sent Time: 2015-03-20 19:47:48 (Friday)
To: u-boot at lists.denx.de
Cc: "Marc Zyngier" <[email protected]>
Subject: [U-Boot] [PATCH 1/6] armv8/cache: Fix page table creation

From: Thierry Reding <[email protected]>

Quoted 8 lines

While generating the page tables, a running integer index is shifted by SECTION_SHIFT (29) and causes overflow for any integer bigger than 7. The page tables therefore alias to the same 8 sections and cause U-Boot to hang once the MMU is enabled.

Fix this by making the index a 64-bit unsigned integer and so avoid the overflow.

Quoted 26 lines
Cc Albert Aribaud <[email protected]>
Cc Marc Zyngier <[email protected]>
---
 arch/arm/cpu/armv8/cache_v8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +2 -2
index c5ec5297cd39..254a629a3b8c 100644
@@ -25,9 +25,9 @@ void set_pgtable_section(u64 *page_table, u64 index, u64 section,
 /* to activate the MMU we need to set up virtual memory */
 static void mmu_setup(void)
 {
-	int i, j, el;
 	bd_t *bd = gd->bd;
-	u64 *page_table = (u64 *)gd->arch.tlb_addr;
+	u64 *page_table = (u64 *)gd->arch.tlb_addr, i, j;
+	int el;
 
 	/* Setup an identity-mapping for all spaces */
 	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
Signature · 1 line
2.3.2

A previous patch did the same work, but got no reply.

Yours.

#18 lore raw

PATCH 1/6 armv8/cache: Fix page table creation

In reply to Thierry Reding

Hello Thierry,

I assume there will be a v2 series?

(asking so that I can mark the series "Changes Requested")

Amicalement,

Signature · 1 line
Albert.
#19 lore raw

PATCH 1/6 armv8/cache: Fix page table creation

Hello Albert,

Quoted 5 lines · Albert ARIBAUD

Hello Thierry,

I assume there will be a v2 series?

(asking so that I can mark the series "Changes Requested")

Ping.

Amicalement,

Signature · 1 line
Albert.