Skip to contentSkip to conversation

Fix Keystone 2 physical address switch

PATCH New Russell King - ARM Linux 4 participants 14 messages May 6, 2015 – Aug 6, 2015

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm/Texas instrument Keystone architecture
lore
Share & tools
lore

Russell King - ARM Linux

author
#0 lore raw

v2 cover Fix Keystone 2 physical address switch

For some time, the Keystone 2 physical address switch has been tainting the kernel ever since we decided that it was not safe.

This series re-works the address switch to be compliant architecturally.

There's still a niggle in the keystone2 code, where we set the arch_virt_to_idmap function pointer in keystone_pv_fixup() - something I'd ideally like to avoid.

I'm in two minds about how much of this code should be in SoC specific code, and how much should be in generic code - on one hand, I don't want to give a green light to this kind of sillyness by encouraging it, but I'd rather not end up with multiple implementations of this.

I've incorporated my change to the update of the PV offset constants into this series, and included the 7th patch in this set which I think is worth having - it's quite a large patch which gets rid of the TTBR1 setting that Keystone has to do. We basically pass the full 64-bits for TTBR0 in the assembly code so that Keystone doesn't have to re-set the page table pointer after secondary CPU initialisation. That needs us to juggle around some registers in the assembly code.

 arch/arm/include/asm/mach/arch.h  |   2 +-
 arch/arm/include/asm/memory.h     |  16 ----
 arch/arm/include/asm/proc-fns.h   |   7 --
 arch/arm/include/asm/smp.h        |   2 +-
 arch/arm/kernel/head-nommu.S      |   2 +-
 arch/arm/kernel/head.S            |  42 ++++++++---
 arch/arm/kernel/setup.c           |   7 +-
 arch/arm/kernel/smp.c             |  10 ++-
 arch/arm/mach-keystone/keystone.c |  41 +++++-----
 arch/arm/mach-keystone/platsmp.c  |  13 ----
 arch/arm/mm/Kconfig               |   4 +
 arch/arm/mm/Makefile              |   1 +
 arch/arm/mm/mmu.c                 | 153 ++++++++++++++++----------------------
 arch/arm/mm/nommu.c               |   9 ---
 arch/arm/mm/proc-v7-2level.S      |   6 +-
 arch/arm/mm/proc-v7-3level.S      |  14 ++--
 arch/arm/mm/proc-v7.S             |  26 +++----
 arch/arm/mm/pv-fixup-asm.S        |  88 ++++++++++++++++++++++
 18 files changed, 239 insertions(+), 204 deletions(-)
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#1 lore raw

PATCH 1/7 ARM: keystone2: move platform notifier initialisation into platform init

We ideally want the init_meminfo function to do nothing but return the delta to be applied to PHYS_OFFSET - it should do nothing else. As we can detect in platform init code whether we are running in the high physical address space, move the platform notifier initialisation entirely into the platform init code.

Acked-by Santosh Shilimkar <[email protected]>
Signed-off-by Russell King <[email protected]>
---
 arch/arm/mach-keystone/keystone.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
arch/arm/mach-keystone/keystone.c +9 -6
index 06620875813a..3d58a8f4dc7e 100644
@@ -27,7 +27,6 @@
 
 #include "keystone.h"
 
-static struct notifier_block platform_nb;
 static unsigned long keystone_dma_pfn_offset __read_mostly;
 
 static int keystone_platform_notifier(struct notifier_block *nb,

@@ -49,11 +48,18 @@ static int keystone_platform_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+static struct notifier_block platform_nb = {
+	.notifier_call = keystone_platform_notifier,
+};
+
 static void __init keystone_init(void)
 {
-	keystone_pm_runtime_init();
-	if (platform_nb.notifier_call)
+	if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
+		keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
+						   KEYSTONE_LOW_PHYS_START);
 		bus_register_notifier(&platform_bus_type, &platform_nb);
+	}
+	keystone_pm_runtime_init();
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 

@@ -96,9 +102,6 @@ static void __init keystone_init_meminfo(void)
 
 	/* Populate the arch idmap hook */
 	arch_virt_to_idmap = keystone_virt_to_idmap;
-	platform_nb.notifier_call = keystone_platform_notifier;
-	keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
-						KEYSTONE_LOW_PHYS_START);
 
 	pr_info("Switching to high address space at 0x%llx\n", (u64)offset);
 }
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 2/7 ARM: keystone2: move update of the phys-to-virt constants into generic code

In reply to Russell King - ARM Linux

Make the init_meminfo function return the offset to be applied to the phys-to-virt translation constants. This allows us to move the update into generic code, along with the requirements for this update.

This avoids platforms having to know the details of the phys-to-virt translation support.

Acked-by Santosh Shilimkar <[email protected]>
Signed-off-by Russell King <[email protected]>
---
 arch/arm/include/asm/mach/arch.h  |  2 +-
 arch/arm/mach-keystone/keystone.c | 27 ++++++++++-----------------
 arch/arm/mm/mmu.c                 | 26 ++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 22 deletions(-)
arch/arm/include/asm/mach/arch.h +1 -1
index 0406cb3f1af7..e881913f7c3e 100644
@@ -51,7 +51,7 @@ struct machine_desc {
 	bool			(*smp_init)(void);
 	void			(*fixup)(struct tag *, char **);
 	void			(*dt_fixup)(void);
-	void			(*init_meminfo)(void);
+	long long		(*init_meminfo)(void);
 	void			(*reserve)(void);/* reserve mem blocks	*/
 	void			(*map_io)(void);/* IO mapping function	*/
 	void			(*init_early)(void);
arch/arm/mach-keystone/keystone.c +10 -17
index 3d58a8f4dc7e..baa0fbc9803a 100644
@@ -68,11 +68,9 @@ static phys_addr_t keystone_virt_to_idmap(unsigned long x)
 	return (phys_addr_t)(x) - CONFIG_PAGE_OFFSET + KEYSTONE_LOW_PHYS_START;
 }
 
-static void __init keystone_init_meminfo(void)
+static long long __init keystone_init_meminfo(void)
 {
-	bool lpae = IS_ENABLED(CONFIG_ARM_LPAE);
-	bool pvpatch = IS_ENABLED(CONFIG_ARM_PATCH_PHYS_VIRT);
-	phys_addr_t offset = PHYS_OFFSET - KEYSTONE_LOW_PHYS_START;
+	long long offset;
 	phys_addr_t mem_start, mem_end;
 
 	mem_start = memblock_start_of_DRAM();

@@ -81,29 +79,24 @@ static void __init keystone_init_meminfo(void)
 	/* nothing to do if we are running out of the <32-bit space */
 	if (mem_start >= KEYSTONE_LOW_PHYS_START &&
 	    mem_end   <= KEYSTONE_LOW_PHYS_END)
-		return;
-
-	if (!lpae || !pvpatch) {
-		pr_crit("Enable %s%s%s to run outside 32-bit space\n",
-		      !lpae ? __stringify(CONFIG_ARM_LPAE) : "",
-		      (!lpae && !pvpatch) ? " and " : "",
-		      !pvpatch ? __stringify(CONFIG_ARM_PATCH_PHYS_VIRT) : "");
-	}
+		return 0;
 
 	if (mem_start < KEYSTONE_HIGH_PHYS_START ||
 	    mem_end   > KEYSTONE_HIGH_PHYS_END) {
 		pr_crit("Invalid address space for memory (%08llx-%08llx)\n",
-		      (u64)mem_start, (u64)mem_end);
+		        (u64)mem_start, (u64)mem_end);
+		return 0;
 	}
 
-	offset += KEYSTONE_HIGH_PHYS_START;
-	__pv_phys_pfn_offset = PFN_DOWN(offset);
-	__pv_offset = (offset - PAGE_OFFSET);
+	offset = KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START;
 
 	/* Populate the arch idmap hook */
 	arch_virt_to_idmap = keystone_virt_to_idmap;
 
-	pr_info("Switching to high address space at 0x%llx\n", (u64)offset);
+	pr_info("Switching to high address space at 0x%llx\n",
+	        (u64)PHYS_OFFSET + (u64)offset);
+
+	return offset;
 }
 
 static const char *const keystone_match[] __initconst = {
arch/arm/mm/mmu.c +22 -4
index 4e6ef896c619..38ccbdf6c322 100644
@@ -1387,7 +1387,7 @@ static void __init map_lowmem(void)
 	}
 }
 
-#ifdef CONFIG_ARM_LPAE
+#if defined(CONFIG_ARM_LPAE) && defined(CONFIG_ARM_PATCH_PHYS_VIRT)
 /*
  * early_paging_init() recreates boot time page table setup, allowing machines
  * to switch over to a high (>4G) address space on LPAE systems

@@ -1397,6 +1397,7 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 {
 	pmdval_t pmdprot = procinfo->__cpu_mm_mmu_flags;
 	unsigned long map_start, map_end;
+	long long offset;
 	pgd_t *pgd0, *pgdk;
 	pud_t *pud0, *pudk, *pud_start;
 	pmd_t *pmd0, *pmdk;

@@ -1419,7 +1420,13 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 	pudk = pud_offset(pgdk, map_start);
 	pmdk = pmd_offset(pudk, map_start);
 
-	mdesc->init_meminfo();
+	offset = mdesc->init_meminfo();
+	if (offset == 0)
+		return;
+
+	/* Re-set the phys pfn offset, and the pv offset */
+	__pv_offset += offset;
+	__pv_phys_pfn_offset += PFN_DOWN(offset);
 
 	/* Run the patch stub to update the constants */
 	fixup_pv_table(&__pv_table_begin,

@@ -1502,8 +1509,19 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 void __init early_paging_init(const struct machine_desc *mdesc,
 			      struct proc_info_list *procinfo)
 {
-	if (mdesc->init_meminfo)
-		mdesc->init_meminfo();
+	long long offset;
+
+	if (!mdesc->init_meminfo)
+		return;
+
+	offset = mdesc->init_meminfo();
+	if (offset == 0)
+		return;
+
+	pr_crit("Physical address space modification is only to support Keystone2.\n");
+	pr_crit("Please enable ARM_LPAE and ARM_PATCH_PHYS_VIRT support to use this\n");
+	pr_crit("feature. Your kernel may crash now, have a good day.\n");
+	add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
 }
 
 #endif
Signature · 1 line
1.8.3.1
#3 lore raw

PATCH 3/7 ARM: keystone2: move address space switch printk into generic code

In reply to Russell King - ARM Linux

There is no point platform code doing this, let's move it into the generic code so it doesn't get duplicated.

Acked-by Santosh Shilimkar <[email protected]>
Signed-off-by Russell King <[email protected]>
---
 arch/arm/mach-keystone/keystone.c | 3 ---
 arch/arm/mm/mmu.c                 | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-keystone/keystone.c +0 -3
index baa0fbc9803a..af8c92bc8188 100644
@@ -93,9 +93,6 @@ static long long __init keystone_init_meminfo(void)
 	/* Populate the arch idmap hook */
 	arch_virt_to_idmap = keystone_virt_to_idmap;
 
-	pr_info("Switching to high address space at 0x%llx\n",
-	        (u64)PHYS_OFFSET + (u64)offset);
-
 	return offset;
 }
arch/arm/mm/mmu.c +3 -0
index 38ccbdf6c322..91262d28a4c9 100644
@@ -1424,6 +1424,9 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 	if (offset == 0)
 		return;
 
+	pr_info("Switching physical address space to 0x%08llx\n",
+		(u64)PHYS_OFFSET + offset);
+
 	/* Re-set the phys pfn offset, and the pv offset */
 	__pv_offset += offset;
 	__pv_phys_pfn_offset += PFN_DOWN(offset);
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH 4/7 ARM: keystone2: rename init_meminfo to pv_fixup

In reply to Russell King - ARM Linux

The init_meminfo() method is not about initialising meminfo - it's about fixing up the physical to virtual translation so that we use a different physical address space, possibly above the 4GB physical address space. Therefore, the name "init_meminfo()" is confusing.

Rename it to pv_fixup() instead.

Acked-by Santosh Shilimkar <[email protected]>
Signed-off-by Russell King <[email protected]>
---
 arch/arm/include/asm/mach/arch.h  | 2 +-
 arch/arm/mach-keystone/keystone.c | 4 ++--
 arch/arm/mm/mmu.c                 | 8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)
arch/arm/include/asm/mach/arch.h +1 -1
index e881913f7c3e..cb3a40717edd 100644
@@ -51,7 +51,7 @@ struct machine_desc {
 	bool			(*smp_init)(void);
 	void			(*fixup)(struct tag *, char **);
 	void			(*dt_fixup)(void);
-	long long		(*init_meminfo)(void);
+	long long		(*pv_fixup)(void);
 	void			(*reserve)(void);/* reserve mem blocks	*/
 	void			(*map_io)(void);/* IO mapping function	*/
 	void			(*init_early)(void);
arch/arm/mach-keystone/keystone.c +2 -2
index af8c92bc8188..e288010522f9 100644
@@ -68,7 +68,7 @@ static phys_addr_t keystone_virt_to_idmap(unsigned long x)
 	return (phys_addr_t)(x) - CONFIG_PAGE_OFFSET + KEYSTONE_LOW_PHYS_START;
 }
 
-static long long __init keystone_init_meminfo(void)
+static long long __init keystone_pv_fixup(void)
 {
 	long long offset;
 	phys_addr_t mem_start, mem_end;

@@ -108,5 +108,5 @@ DT_MACHINE_START(KEYSTONE, "Keystone")
 	.smp		= smp_ops(keystone_smp_ops),
 	.init_machine	= keystone_init,
 	.dt_compat	= keystone_match,
-	.init_meminfo   = keystone_init_meminfo,
+	.pv_fixup	= keystone_pv_fixup,
 MACHINE_END
arch/arm/mm/mmu.c +4 -4
index 91262d28a4c9..0e5ed87221dd 100644
@@ -1404,7 +1404,7 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 	phys_addr_t phys;
 	int i;
 
-	if (!(mdesc->init_meminfo))
+	if (!mdesc->pv_fixup)
 		return;
 
 	/* remap kernel code and data */

@@ -1420,7 +1420,7 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 	pudk = pud_offset(pgdk, map_start);
 	pmdk = pmd_offset(pudk, map_start);
 
-	offset = mdesc->init_meminfo();
+	offset = mdesc->pv_fixup();
 	if (offset == 0)
 		return;
 

@@ -1514,10 +1514,10 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 {
 	long long offset;
 
-	if (!mdesc->init_meminfo)
+	if (!mdesc->pv_fixup)
 		return;
 
-	offset = mdesc->init_meminfo();
+	offset = mdesc->pv_fixup();
 	if (offset == 0)
 		return;
Signature · 1 line
1.8.3.1
#5 lore raw

PATCH 5/7 ARM: re-implement physical address space switching

In reply to Russell King - ARM Linux

Re-implement the physical address space switching to be architecturally compliant. This involves flushing the caches, disabling the MMU, and only then updating the page tables. Once that is complete, the system can be brought back up again.

Since we disable the MMU, we need to do the update in assembly code. Luckily, the entries which need updating are fairly trivial, and are all setup by the early assembly code. We can merely adjust each entry by the delta required.

Not only does this fix the code to be architecturally compliant, but it fixes a couple of bugs too:

1. The original code would only ever update the first L2 entry covering a fraction of the kernel; the remainder were left untouched.
2. The L2 entries covering the DTB blob were likewise untouched.

This solution fixes up all entries.

Signed-off-by Russell King <[email protected]>
---
 arch/arm/mm/Kconfig        |   4 ++
 arch/arm/mm/Makefile       |   1 +
 arch/arm/mm/mmu.c          | 124 +++++++++++++++------------------------------
 arch/arm/mm/pv-fixup-asm.S |  88 ++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+), 84 deletions(-)
 create mode 100644 arch/arm/mm/pv-fixup-asm.S
arch/arm/mm/Kconfig +4 -0
index b4f92b9a13ac..4dc661e2d3a6 100644
@@ -624,6 +624,10 @@ config ARM_LPAE
 
 	  If unsure, say N.
 
+config ARM_PV_FIXUP
+	def_bool y
+	depends on ARM_LPAE && ARM_PATCH_PHYS_VIRT && ARCH_KEYSTONE
+
 config ARCH_PHYS_ADDR_T_64BIT
 	def_bool ARM_LPAE
arch/arm/mm/Makefile +1 -0
index d3afdf9eb65a..4cc1ec9f6bb0 100644
@@ -18,6 +18,7 @@ obj-$(CONFIG_MODULES)		+= proc-syms.o
 obj-$(CONFIG_ALIGNMENT_TRAP)	+= alignment.o
 obj-$(CONFIG_HIGHMEM)		+= highmem.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
+obj-$(CONFIG_ARM_PV_FIXUP)	+= pv-fixup-asm.o
 
 obj-$(CONFIG_CPU_ABRT_NOMMU)	+= abort-nommu.o
 obj-$(CONFIG_CPU_ABRT_EV4)	+= abort-ev4.o
arch/arm/mm/mmu.c +40 -84

157 lines not on this page · raw message

arch/arm/mm/pv-fixup-asm.S new file +88 -0

88 lines not on this page · raw message

Signature · 1 line
1.8.3.1
maintainer
#6 lore raw

PATCH 5/7 ARM: re-implement physical address space switching

On 05/06/2015 05:30 AM, Russell King wrote:
[...]

Quoted 1 line
(quoted diff) +0 -0
@@ -1436,75 +1435,32 @@ void __init early_paging_init(const struct machine_desc *mdesc,

[...]

arch/arm/mm/mmu.c void __init early_paging_init(const struct machine_desc *mdesc, · line 1462

Quoted 5 lines
arch/arm/mm/mmu.c +2 -3
-	/* Finally flush any stale TLB values. */
-	local_flush_bp_all();
-	local_flush_tlb_all();
+	/* Re-enable the caches and cacheable TLB walks */
+	asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "ttbcr");

^^
on next-20150511, echo "CONFIG_ARM_LPAE=y">>arch/arm/configs/multi_v7_defconfig ;make multi_v7_defconfig;make arch/arm/mm/mmu.o
#

# configuration written to .config
#
scripts/kconfig/conf  --silentoldconfig Kconfig

<snip>
CC arch/arm/mm/mmu.o
arch/arm/mm/mmu.c: In function ?early_paging_init?:
arch/arm/mm/mmu.c:1461:54: error: expected ?(? before ?)? token make[1]: *** [arch/arm/mm/mmu.o] Error 1
make: *** [arch/arm/mm/mmu.o] Error 2
this was using [1]

using gcc 4.9[2]:
arch/arm/mm/mmu.c: In function ?early_paging_init?:
arch/arm/mm/mmu.c:1461:54: error: expected ?(? before ?)? token

  asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "ttbcr");
                                                      ^

scripts/Makefile.build:258: recipe for target 'arch/arm/mm/mmu.o' failed make[1]: *** [arch/arm/mm/mmu.o] Error 1
Makefile:1545: recipe for target 'arch/arm/mm/mmu.o' failed
make: *** [arch/arm/mm/mmu.o] Error 2

arch/arm/mm/mmu.c void __init early_paging_init(const struct machine_desc *mdesc, · line 1464

Quoted 2 lines
arch/arm/mm/mmu.c +1 -0
+	set_cr(cr);
 }

[1] $ arm-linux-gnueabi-gcc --version
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[2] $ arm-linux-gnu-gcc --version
arm-linux-gnu-gcc (GCC) 4.9.2 20150107 (Red Hat Cross 4.9.2-3) Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Signature · 2 lines
Regards,
Nishanth Menon
author
#7 lore raw

PATCH v2 5/7 ARM: re-implement physical address space switching

Quoted 19 lines · Nishanth Menon

On 05/06/2015 05:30 AM, Russell King wrote:
[...]

Quoted 1 line
(quoted diff) +0 -0
@@ -1436,75 +1435,32 @@ void __init early_paging_init(const struct machine_desc *mdesc,

[...]

arch/arm/mm/mmu.c void __init early_paging_init(const struct machine_desc *mdesc, · line 1462

Quoted 5 lines
arch/arm/mm/mmu.c +2 -3
-	/* Finally flush any stale TLB values. */
-	local_flush_bp_all();
-	local_flush_tlb_all();
+	/* Re-enable the caches and cacheable TLB walks */
+	asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "ttbcr");

^^
on next-20150511, echo "CONFIG_ARM_LPAE=y">>arch/arm/configs/multi_v7_defconfig ;make multi_v7_defconfig;make arch/arm/mm/mmu.o
#

# configuration written to .config
#
scripts/kconfig/conf  --silentoldconfig Kconfig
Quoted 7 lines

<snip>
CC arch/arm/mm/mmu.o
arch/arm/mm/mmu.c: In function ?early_paging_init?:
arch/arm/mm/mmu.c:1461:54: error: expected ?(? before ?)? token make[1]: *** [arch/arm/mm/mmu.o] Error 1
make: *** [arch/arm/mm/mmu.o] Error 2
this was using [1]

That's why I said I'd send you an updated version. Just replace the "ttbcr" with "r" (ttbcr)...

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
maintainer
#8 lore raw

PATCH 5/7 ARM: re-implement physical address space switching

Quoted 28 lines · Russell King - ARM Linux
Quoted 19 lines · Nishanth Menon

On 05/06/2015 05:30 AM, Russell King wrote:
[...]

Quoted 1 line
(quoted diff) +0 -0
@@ -1436,75 +1435,32 @@ void __init early_paging_init(const struct machine_desc *mdesc,

[...]

arch/arm/mm/mmu.c void __init early_paging_init(const struct machine_desc *mdesc, · line 1462

Quoted 5 lines
arch/arm/mm/mmu.c +2 -3
-   /* Finally flush any stale TLB values. */
-   local_flush_bp_all();
-   local_flush_tlb_all();
+   /* Re-enable the caches and cacheable TLB walks */
+   asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "ttbcr");

^^
on next-20150511, echo "CONFIG_ARM_LPAE=y">>arch/arm/configs/multi_v7_defconfig ;make multi_v7_defconfig;make arch/arm/mm/mmu.o
#

# configuration written to .config
#
scripts/kconfig/conf  --silentoldconfig Kconfig
Quoted 7 lines

<snip>
CC arch/arm/mm/mmu.o
arch/arm/mm/mmu.c: In function ?early_paging_init?:
arch/arm/mm/mmu.c:1461:54: error: expected ?(? before ?)? token make[1]: *** [arch/arm/mm/mmu.o] Error 1
make: *** [arch/arm/mm/mmu.o] Error 2
this was using [1]

Quoted 2 lines

That's why I said I'd send you an updated version. Just replace the "ttbcr" with "r" (ttbcr)...

Was reporting regression on 20150511. 20150512 seems to have built fine.

---
Regards,
Nishanth Menon

#9 lore raw

PATCH 6/7 ARM: cleanup early_paging_init() calling

In reply to Russell King - ARM Linux

Eliminate the needless nommu version of this function, and get rid of the proc_info_list structure argument - we no longer need this in order to fix up the page table entries.

Acked-by Santosh Shilimkar <[email protected]>
Signed-off-by Russell King <[email protected]>
---
 arch/arm/kernel/setup.c | 7 ++++---
 arch/arm/mm/mmu.c       | 6 ++----
 arch/arm/mm/nommu.c     | 9 ---------
 3 files changed, 6 insertions(+), 16 deletions(-)
arch/arm/kernel/setup.c +4 -3
index 6c777e908a24..979c1c5fe96a 100644
@@ -75,8 +75,7 @@ __setup("fpe=", fpe_setup);
 
 extern void init_default_cache_policy(unsigned long);
 extern void paging_init(const struct machine_desc *desc);
-extern void early_paging_init(const struct machine_desc *,
-			      struct proc_info_list *);
+extern void early_paging_init(const struct machine_desc *);
 extern void sanity_check_meminfo(void);
 extern enum reboot_mode reboot_mode;
 extern void setup_dma_zone(const struct machine_desc *desc);

@@ -936,7 +935,9 @@ void __init setup_arch(char **cmdline_p)
 
 	parse_early_param();
 
-	early_paging_init(mdesc, lookup_processor_type(read_cpuid_id()));
+#ifdef CONFIG_MMU
+	early_paging_init(mdesc);
+#endif
 	setup_dma_zone(mdesc);
 	sanity_check_meminfo();
 	arm_memblock_init(mdesc);
arch/arm/mm/mmu.c +2 -4
index 60e64209e7d6..d1d477958ea0 100644
@@ -1396,8 +1396,7 @@ pgtables_remap lpae_pgtables_remap_asm;
  * early_paging_init() recreates boot time page table setup, allowing machines
  * to switch over to a high (>4G) address space on LPAE systems
  */
-void __init early_paging_init(const struct machine_desc *mdesc,
-			      struct proc_info_list *procinfo)
+void __init early_paging_init(const struct machine_desc *mdesc)
 {
 	pgtables_remap *lpae_pgtables_remap;
 	unsigned long pa_pgd;

@@ -1465,8 +1464,7 @@ void __init early_paging_init(const struct machine_desc *mdesc,
 
 #else
 
-void __init early_paging_init(const struct machine_desc *mdesc,
-			      struct proc_info_list *procinfo)
+void __init early_paging_init(const struct machine_desc *mdesc)
 {
 	long long offset;
arch/arm/mm/nommu.c +0 -9
index a014dfacd5ca..afd7e05d95f1 100644
@@ -304,15 +304,6 @@ void __init sanity_check_meminfo(void)
 }
 
 /*
- * early_paging_init() recreates boot time page table setup, allowing machines
- * to switch over to a high (>4G) address space on LPAE systems
- */
-void __init early_paging_init(const struct machine_desc *mdesc,
-			      struct proc_info_list *procinfo)
-{
-}
-
-/*
  * paging_init() sets up the page tables, initialises the zone memory
  * maps, and sets up the zero page, bad page and bad page tables.
  */
Signature · 1 line
1.8.3.1
#10 lore raw

PATCH 7/7 ARM: redo TTBR setup code for LPAE

In reply to Russell King - ARM Linux

Re-engineer the LPAE TTBR setup code. Rather than passing some shifted address in order to fit in a CPU register, pass either a full physical address (in the case of r4, r5 for TTBR0) or a PFN (for TTBR1).

This removes the ARCH_PGD_SHIFT hack, and the last dangerous user of cpu_set_ttbr() in the secondary CPU startup code path (which was there to re-set TTBR1 to the appropriate high physical address space on Keystone2.

Signed-off-by Russell King <[email protected]>
---
 arch/arm/include/asm/memory.h    | 16 ---------------
 arch/arm/include/asm/proc-fns.h  |  7 -------
 arch/arm/include/asm/smp.h       |  2 +-
 arch/arm/kernel/head-nommu.S     |  2 +-
 arch/arm/kernel/head.S           | 42 +++++++++++++++++++++++++++++-----------
 arch/arm/kernel/smp.c            | 10 ++++++----
 arch/arm/mach-keystone/platsmp.c | 13 -------------
 arch/arm/mm/proc-v7-2level.S     |  6 +++---
 arch/arm/mm/proc-v7-3level.S     | 14 +++++---------
 arch/arm/mm/proc-v7.S            | 26 ++++++++++++-------------
 10 files changed, 60 insertions(+), 78 deletions(-)
arch/arm/include/asm/memory.h +0 -16
index 184def0e1652..3a72d69b3255 100644
@@ -18,8 +18,6 @@
 #include <linux/types.h>
 #include <linux/sizes.h>
 
-#include <asm/cache.h>
-
 #ifdef CONFIG_NEED_MACH_MEMORY_H
 #include <mach/memory.h>
 #endif

@@ -133,20 +131,6 @@
 #define phys_to_page(phys)	(pfn_to_page(__phys_to_pfn(phys)))
 
 /*
- * Minimum guaranted alignment in pgd_alloc().  The page table pointers passed
- * around in head.S and proc-*.S are shifted by this amount, in order to
- * leave spare high bits for systems with physical address extension.  This
- * does not fully accomodate the 40-bit addressing capability of ARM LPAE, but
- * gives us about 38-bits or so.
- */
-#ifdef CONFIG_ARM_LPAE
-#define ARCH_PGD_SHIFT		L1_CACHE_SHIFT
-#else
-#define ARCH_PGD_SHIFT		0
-#endif
-#define ARCH_PGD_MASK		((1 << ARCH_PGD_SHIFT) - 1)
-
-/*
  * PLAT_PHYS_OFFSET is the offset (from zero) of the start of physical
  * memory.  This is used for XIP and NoMMU kernels, and on platforms that don't
  * have CONFIG_ARM_PATCH_PHYS_VIRT. Assembly code must always use
arch/arm/include/asm/proc-fns.h +0 -7
index 5324c1112f3a..8877ad5ffe10 100644
@@ -125,13 +125,6 @@ extern void cpu_resume(void);
 		ttbr;						\
 	})
 
-#define cpu_set_ttbr(nr, val)					\
-	do {							\
-		u64 ttbr = val;					\
-		__asm__("mcrr	p15, " #nr ", %Q0, %R0, c2"	\
-			: : "r" (ttbr));			\
-	} while (0)
-
 #define cpu_get_pgd()	\
 	({						\
 		u64 pg = cpu_get_ttbr(0);		\
arch/arm/include/asm/smp.h +1 -1
index 18f5a554134f..487aa08f31ee 100644
@@ -61,7 +61,7 @@ asmlinkage void secondary_start_kernel(void);
 struct secondary_data {
 	union {
 		unsigned long mpu_rgn_szr;
-		unsigned long pgdir;
+		u64 pgdir;
 	};
 	unsigned long swapper_pg_dir;
 	void *stack;
arch/arm/kernel/head-nommu.S +1 -1
index aebfbf79a1a3..84da14b7cd04 100644
@@ -123,7 +123,7 @@ ENTRY(secondary_startup)
 ENDPROC(secondary_startup)
 
 ENTRY(__secondary_switched)
-	ldr	sp, [r7, #8]			@ set up the stack pointer
+	ldr	sp, [r7, #12]			@ set up the stack pointer
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
arch/arm/kernel/head.S +31 -11

93 lines not on this page · raw message

arch/arm/kernel/smp.c +6 -4
index cca5b8758185..90dfbedfbfb8 100644
@@ -86,9 +86,11 @@ void __init smp_set_ops(struct smp_operations *ops)
 
 static unsigned long get_arch_pgd(pgd_t *pgd)
 {
-	phys_addr_t pgdir = virt_to_idmap(pgd);
-	BUG_ON(pgdir & ARCH_PGD_MASK);
-	return pgdir >> ARCH_PGD_SHIFT;
+#ifdef CONFIG_ARM_LPAE
+	return __phys_to_pfn(virt_to_phys(pgd));
+#else
+	return virt_to_phys(pgd);
+#endif
 }
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)

@@ -108,7 +110,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 #endif
 
 #ifdef CONFIG_MMU
-	secondary_data.pgdir = get_arch_pgd(idmap_pgd);
+	secondary_data.pgdir = virt_to_phys(idmap_pgd);
 	secondary_data.swapper_pg_dir = get_arch_pgd(swapper_pg_dir);
 #endif
 	sync_cache_w(&secondary_data);
arch/arm/mach-keystone/platsmp.c +0 -13
index 5f46a7cf907b..4bbb18463bfd 100644
@@ -39,19 +39,6 @@ static int keystone_smp_boot_secondary(unsigned int cpu,
 	return error;
 }
 
-#ifdef CONFIG_ARM_LPAE
-static void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu)
-{
-	pgd_t *pgd0 = pgd_offset_k(0);
-	cpu_set_ttbr(1, __pa(pgd0) + TTBR1_OFFSET);
-	local_flush_tlb_all();
-}
-#else
-static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu)
-{}
-#endif
-
 struct smp_operations keystone_smp_ops __initdata = {
 	.smp_boot_secondary	= keystone_smp_boot_secondary,
-	.smp_secondary_init     = keystone_smp_secondary_initmem,
 };
arch/arm/mm/proc-v7-2level.S +3 -3
index 10405b8d31af..fa385140715f 100644
@@ -148,10 +148,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 * Macro for setting up the TTBRx and TTBCR registers.
 	 * - \ttb0 and \ttb1 updated with the corresponding flags.
 	 */
-	.macro	v7_ttb_setup, zero, ttbr0, ttbr1, tmp
+	.macro	v7_ttb_setup, zero, ttbr0l, ttbr0h, ttbr1, tmp
 	mcr	p15, 0, \zero, c2, c0, 2	@ TTB control register
-	ALT_SMP(orr	\ttbr0, \ttbr0, #TTB_FLAGS_SMP)
-	ALT_UP(orr	\ttbr0, \ttbr0, #TTB_FLAGS_UP)
+	ALT_SMP(orr	\ttbr0l, \ttbr0l, #TTB_FLAGS_SMP)
+	ALT_UP(orr	\ttbr0l, \ttbr0l, #TTB_FLAGS_UP)
 	ALT_SMP(orr	\ttbr1, \ttbr1, #TTB_FLAGS_SMP)
 	ALT_UP(orr	\ttbr1, \ttbr1, #TTB_FLAGS_UP)
 	mcr	p15, 0, \ttbr1, c2, c0, 1	@ load TTB1
arch/arm/mm/proc-v7-3level.S +5 -9
index d3daed0ae0ad..5e5720e8bc5f 100644
@@ -126,11 +126,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 * Macro for setting up the TTBRx and TTBCR registers.
 	 * - \ttbr1 updated.
 	 */
-	.macro	v7_ttb_setup, zero, ttbr0, ttbr1, tmp
+	.macro	v7_ttb_setup, zero, ttbr0l, ttbr0h, ttbr1, tmp
 	ldr	\tmp, =swapper_pg_dir		@ swapper_pg_dir virtual address
-	mov	\tmp, \tmp, lsr #ARCH_PGD_SHIFT
-	cmp	\ttbr1, \tmp			@ PHYS_OFFSET > PAGE_OFFSET?
-	mrc	p15, 0, \tmp, c2, c0, 2		@ TTB control register
+	cmp	\ttbr1, \tmp, lsr #12		@ PHYS_OFFSET > PAGE_OFFSET?
+	mrc	p15, 0, \tmp, c2, c0, 2		@ TTB control egister
 	orr	\tmp, \tmp, #TTB_EAE
 	ALT_SMP(orr	\tmp, \tmp, #TTB_FLAGS_SMP)
 	ALT_UP(orr	\tmp, \tmp, #TTB_FLAGS_UP)

@@ -143,13 +142,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 */
 	orrls	\tmp, \tmp, #TTBR1_SIZE				@ TTBCR.T1SZ
 	mcr	p15, 0, \tmp, c2, c0, 2				@ TTBCR
-	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
-	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
+	mov	\tmp, \ttbr1, lsr #20
+	mov	\ttbr1, \ttbr1, lsl #12
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
 	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
-	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
-	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
-	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0
 	.endm
 
 	/*
arch/arm/mm/proc-v7.S +13 -13
index 3d1054f11a8a..873230912894 100644
@@ -343,9 +343,9 @@ __v7_setup:
 	and	r10, r0, #0xff000000		@ ARM?
 	teq	r10, #0x41000000
 	bne	3f
-	and	r5, r0, #0x00f00000		@ variant
+	and	r3, r0, #0x00f00000		@ variant
 	and	r6, r0, #0x0000000f		@ revision
-	orr	r6, r6, r5, lsr #20-4		@ combine variant and revision
+	orr	r6, r6, r3, lsr #20-4		@ combine variant and revision
 	ubfx	r0, r0, #4, #12			@ primary part number
 
 	/* Cortex-A8 Errata */

@@ -354,7 +354,7 @@ __v7_setup:
 	bne	2f
 #if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM)
 
-	teq	r5, #0x00100000			@ only present in r1p*
+	teq	r3, #0x00100000			@ only present in r1p*
 	mrceq	p15, 0, r10, c1, c0, 1		@ read aux control register
 	orreq	r10, r10, #(1 << 6)		@ set IBE to 1
 	mcreq	p15, 0, r10, c1, c0, 1		@ write aux control register

@@ -395,7 +395,7 @@ __v7_setup:
 	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register
 #endif
 #ifdef CONFIG_ARM_ERRATA_743622
-	teq	r5, #0x00200000			@ only present in r2p*
+	teq	r3, #0x00200000			@ only present in r2p*
 	mrceq	p15, 0, r10, c15, c0, 1		@ read diagnostic register
 	orreq	r10, r10, #1 << 6		@ set bit #6
 	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register

@@ -425,10 +425,10 @@ __v7_setup:
 	mcr	p15, 0, r10, c7, c5, 0		@ I+BTB cache invalidate
 #ifdef CONFIG_MMU
 	mcr	p15, 0, r10, c8, c7, 0		@ invalidate I + D TLBs
-	v7_ttb_setup r10, r4, r8, r5		@ TTBCR, TTBRx setup
-	ldr	r5, =PRRR			@ PRRR
+	v7_ttb_setup r10, r4, r5, r8, r3	@ TTBCR, TTBRx setup
+	ldr	r3, =PRRR			@ PRRR
 	ldr	r6, =NMRR			@ NMRR
-	mcr	p15, 0, r5, c10, c2, 0		@ write PRRR
+	mcr	p15, 0, r3, c10, c2, 0		@ write PRRR
 	mcr	p15, 0, r6, c10, c2, 1		@ write NMRR
 #endif
 	dsb					@ Complete invalidations

@@ -437,22 +437,22 @@ __v7_setup:
 	and	r0, r0, #(0xf << 12)		@ ThumbEE enabled field
 	teq	r0, #(1 << 12)			@ check if ThumbEE is present
 	bne	1f
-	mov	r5, #0
-	mcr	p14, 6, r5, c1, c0, 0		@ Initialize TEEHBR to 0
+	mov	r3, #0
+	mcr	p14, 6, r3, c1, c0, 0		@ Initialize TEEHBR to 0
 	mrc	p14, 6, r0, c0, c0, 0		@ load TEECR
 	orr	r0, r0, #1			@ set the 1st bit in order to
 	mcr	p14, 6, r0, c0, c0, 0		@ stop userspace TEEHBR access
 1:
 #endif
-	adr	r5, v7_crval
-	ldmia	r5, {r5, r6}
+	adr	r3, v7_crval
+	ldmia	r3, {r3, r6}
  ARM_BE8(orr	r6, r6, #1 << 25)		@ big-endian page tables
 #ifdef CONFIG_SWP_EMULATE
-	orr     r5, r5, #(1 << 10)              @ set SW bit in "clear"
+	orr     r3, r3, #(1 << 10)              @ set SW bit in "clear"
 	bic     r6, r6, #(1 << 10)              @ clear it in "mmuset"
 #endif
    	mrc	p15, 0, r0, c1, c0, 0		@ read control register
-	bic	r0, r0, r5			@ clear bits them
+	bic	r0, r0, r3			@ clear bits them
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
Signature · 1 line
1.8.3.1
#11 lore raw

PATCH 7/7 ARM: redo TTBR setup code for LPAE

Hi Russell,

Quoted 8 lines · Russell King

Re-engineer the LPAE TTBR setup code. Rather than passing some shifted address in order to fit in a CPU register, pass either a full physical address (in the case of r4, r5 for TTBR0) or a PFN (for TTBR1).

This removes the ARCH_PGD_SHIFT hack, and the last dangerous user of cpu_set_ttbr() in the secondary CPU startup code path (which was there to re-set TTBR1 to the appropriate high physical address space on Keystone2.

It seems that since the merge of this commit the secondary CPUs on Armada XP failed to come online in Big Endian (my configuration is mvebu_v7_defconfig+CONFIG_CPU_BIG_ENDIAN=y). Once I found that this commit introduced the regression I also tested but with LPAE but I had the same result: only the first CPU boot.

I don't know if there is something wrong with this patch or if it reveals something which was wrong in the Armada XP port.

At a point I suspected something around the change in the get_arch_pgd function but I didn't find anything useful.

Any guidance would be greatly appreciate.

Thanks,

Gregory

Quoted 380 lines
Signed-off-by Russell King <[email protected]>
---
 arch/arm/include/asm/memory.h    | 16 ---------------
 arch/arm/include/asm/proc-fns.h  |  7 -------
 arch/arm/include/asm/smp.h       |  2 +-
 arch/arm/kernel/head-nommu.S     |  2 +-
 arch/arm/kernel/head.S           | 42 +++++++++++++++++++++++++++++-----------
 arch/arm/kernel/smp.c            | 10 ++++++----
 arch/arm/mach-keystone/platsmp.c | 13 -------------
 arch/arm/mm/proc-v7-2level.S     |  6 +++---
 arch/arm/mm/proc-v7-3level.S     | 14 +++++---------
 arch/arm/mm/proc-v7.S            | 26 ++++++++++++-------------
 10 files changed, 60 insertions(+), 78 deletions(-)
arch/arm/include/asm/memory.h +0 -16
index 184def0e1652..3a72d69b3255 100644
@@ -18,8 +18,6 @@
 #include <linux/types.h>
 #include <linux/sizes.h>
 
-#include <asm/cache.h>
-
 #ifdef CONFIG_NEED_MACH_MEMORY_H
 #include <mach/memory.h>
 #endif

@@ -133,20 +131,6 @@
 #define phys_to_page(phys)	(pfn_to_page(__phys_to_pfn(phys)))
 
 /*
- * Minimum guaranted alignment in pgd_alloc().  The page table pointers passed
- * around in head.S and proc-*.S are shifted by this amount, in order to
- * leave spare high bits for systems with physical address extension.  This
- * does not fully accomodate the 40-bit addressing capability of ARM LPAE, but
- * gives us about 38-bits or so.
- */
-#ifdef CONFIG_ARM_LPAE
-#define ARCH_PGD_SHIFT		L1_CACHE_SHIFT
-#else
-#define ARCH_PGD_SHIFT		0
-#endif
-#define ARCH_PGD_MASK		((1 << ARCH_PGD_SHIFT) - 1)
-
-/*
  * PLAT_PHYS_OFFSET is the offset (from zero) of the start of physical
  * memory.  This is used for XIP and NoMMU kernels, and on platforms that don't
  * have CONFIG_ARM_PATCH_PHYS_VIRT. Assembly code must always use
arch/arm/include/asm/proc-fns.h +0 -7
index 5324c1112f3a..8877ad5ffe10 100644
@@ -125,13 +125,6 @@ extern void cpu_resume(void);
 		ttbr;						\
 	})
 
-#define cpu_set_ttbr(nr, val)					\
-	do {							\
-		u64 ttbr = val;					\
-		__asm__("mcrr	p15, " #nr ", %Q0, %R0, c2"	\
-			: : "r" (ttbr));			\
-	} while (0)
-
 #define cpu_get_pgd()	\
 	({						\
 		u64 pg = cpu_get_ttbr(0);		\
arch/arm/include/asm/smp.h +1 -1
index 18f5a554134f..487aa08f31ee 100644
@@ -61,7 +61,7 @@ asmlinkage void secondary_start_kernel(void);
 struct secondary_data {
 	union {
 		unsigned long mpu_rgn_szr;
-		unsigned long pgdir;
+		u64 pgdir;
 	};
 	unsigned long swapper_pg_dir;
 	void *stack;
arch/arm/kernel/head-nommu.S +1 -1
index aebfbf79a1a3..84da14b7cd04 100644
@@ -123,7 +123,7 @@ ENTRY(secondary_startup)
 ENDPROC(secondary_startup)
 
 ENTRY(__secondary_switched)
-	ldr	sp, [r7, #8]			@ set up the stack pointer
+	ldr	sp, [r7, #12]			@ set up the stack pointer
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
arch/arm/kernel/head.S +31 -11
index 3637973a9708..7304b4c44b52 100644
@@ -131,13 +131,30 @@ ENTRY(stext)
 	 * The following calls CPU specific code in a position independent
 	 * manner.  See arch/arm/mm/proc-*.S for details.  r10 = base of
 	 * xxx_proc_info structure selected by __lookup_processor_type
-	 * above.  On return, the CPU will be ready for the MMU to be
-	 * turned on, and r0 will hold the CPU control register value.
+	 * above.
+	 *
+	 * The processor init function will be called with:
+	 *  r1 - machine type
+	 *  r2 - boot data (atags/dt) pointer
+	 *  r4 - translation table base (low word)
+	 *  r5 - translation table base (high word, if LPAE)
+	 *  r8 - translation table base 1 (pfn if LPAE)
+	 *  r9 - cpuid
+	 *  r13 - virtual address for __enable_mmu -> __turn_mmu_on
+	 *
+	 * On return, the CPU will be ready for the MMU to be turned on,
+	 * r0 will hold the CPU control register value, r1, r2, r4, and
+	 * r9 will be preserved.  r5 will also be preserved if LPAE.
 	 */
 	ldr	r13, =__mmap_switched		@ address to jump to after
 						@ mmu has been enabled
 	adr	lr, BSYM(1f)			@ return (PIC) address
+#ifdef CONFIG_ARM_LPAE
+	mov	r5, #0				@ high TTBR0
+	mov	r8, r4, lsr #12			@ TTBR1 is swapper_pg_dir pfn
+#else
 	mov	r8, r4				@ set TTBR1 to swapper_pg_dir
+#endif
 	ldr	r12, [r10, #PROCINFO_INITFUNC]
 	add	r12, r12, r10
 	ret	r12

@@ -158,7 +175,7 @@ ENDPROC(stext)
  *
  * Returns:
  *  r0, r3, r5-r7 corrupted
- *  r4 = page table (see ARCH_PGD_SHIFT in asm/memory.h)
+ *  r4 = physical page table address
  */
 __create_page_tables:
 	pgtbl	r4, r8				@ page table address

@@ -333,7 +350,6 @@ __create_page_tables:
 #endif
 #ifdef CONFIG_ARM_LPAE
 	sub	r4, r4, #0x1000		@ point to the PGD table
-	mov	r4, r4, lsr #ARCH_PGD_SHIFT
 #endif
 	ret	lr
 ENDPROC(__create_page_tables)

@@ -381,9 +397,9 @@ ENTRY(secondary_startup)
 	adr	r4, __secondary_data
 	ldmia	r4, {r5, r7, r12}		@ address to jump to after
 	sub	lr, r4, r5			@ mmu has been enabled
-	ldr	r4, [r7, lr]			@ get secondary_data.pgdir
-	add	r7, r7, #4
-	ldr	r8, [r7, lr]			@ get secondary_data.swapper_pg_dir
+	add	r3, r7, lr
+	ldrd	r4, [r3, #0]			@ get secondary_data.pgdir
+	ldr	r8, [r3, #8]			@ get secondary_data.swapper_pg_dir
 	adr	lr, BSYM(__enable_mmu)		@ return address
 	mov	r13, r12			@ __secondary_switched address
 	ldr	r12, [r10, #PROCINFO_INITFUNC]

@@ -397,7 +413,7 @@ ENDPROC(secondary_startup_arm)
 	 * r6  = &secondary_data
 	 */
 ENTRY(__secondary_switched)
-	ldr	sp, [r7, #4]			@ get secondary_data.stack
+	ldr	sp, [r7, #12]			@ get secondary_data.stack
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)

@@ -416,12 +432,14 @@ __secondary_data:
 /*
  * Setup common bits before finally enabling the MMU.  Essentially
  * this is just loading the page table pointer and domain access
- * registers.
+ * registers.  All these registers need to be preserved by the
+ * processor setup function (or set in the case of r0)
  *
  *  r0  = cp#15 control register
  *  r1  = machine ID
  *  r2  = atags or dtb pointer
- *  r4  = page table (see ARCH_PGD_SHIFT in asm/memory.h)
+ *  r4  = TTBR pointer (low word)
+ *  r5  = TTBR pointer (high word if LPAE)
  *  r9  = processor ID
  *  r13 = *virtual* address to jump to upon completion
  */

@@ -440,7 +458,9 @@ __enable_mmu:
 #ifdef CONFIG_CPU_ICACHE_DISABLE
 	bic	r0, r0, #CR_I
 #endif
-#ifndef CONFIG_ARM_LPAE
+#ifdef CONFIG_ARM_LPAE
+	mcrr	p15, 0, r4, r5, c2		@ load TTBR0
+#else
 	mov	r5, #(domain_val(DOMAIN_USER, DOMAIN_MANAGER) | \
 		      domain_val(DOMAIN_KERNEL, DOMAIN_MANAGER) | \
 		      domain_val(DOMAIN_TABLE, DOMAIN_MANAGER) | \
arch/arm/kernel/smp.c +6 -4
index cca5b8758185..90dfbedfbfb8 100644
@@ -86,9 +86,11 @@ void __init smp_set_ops(struct smp_operations *ops)
 
 static unsigned long get_arch_pgd(pgd_t *pgd)
 {
-	phys_addr_t pgdir = virt_to_idmap(pgd);
-	BUG_ON(pgdir & ARCH_PGD_MASK);
-	return pgdir >> ARCH_PGD_SHIFT;
+#ifdef CONFIG_ARM_LPAE
+	return __phys_to_pfn(virt_to_phys(pgd));
+#else
+	return virt_to_phys(pgd);
+#endif
 }
 
 int __cpu_up(unsigned int cpu, struct task_struct *idle)

@@ -108,7 +110,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 #endif
 
 #ifdef CONFIG_MMU
-	secondary_data.pgdir = get_arch_pgd(idmap_pgd);
+	secondary_data.pgdir = virt_to_phys(idmap_pgd);
 	secondary_data.swapper_pg_dir = get_arch_pgd(swapper_pg_dir);
 #endif
 	sync_cache_w(&secondary_data);
arch/arm/mach-keystone/platsmp.c +0 -13
index 5f46a7cf907b..4bbb18463bfd 100644
@@ -39,19 +39,6 @@ static int keystone_smp_boot_secondary(unsigned int cpu,
 	return error;
 }
 
-#ifdef CONFIG_ARM_LPAE
-static void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu)
-{
-	pgd_t *pgd0 = pgd_offset_k(0);
-	cpu_set_ttbr(1, __pa(pgd0) + TTBR1_OFFSET);
-	local_flush_tlb_all();
-}
-#else
-static inline void __cpuinit keystone_smp_secondary_initmem(unsigned int cpu)
-{}
-#endif
-
 struct smp_operations keystone_smp_ops __initdata = {
 	.smp_boot_secondary	= keystone_smp_boot_secondary,
-	.smp_secondary_init     = keystone_smp_secondary_initmem,
 };
arch/arm/mm/proc-v7-2level.S +3 -3
index 10405b8d31af..fa385140715f 100644
@@ -148,10 +148,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 * Macro for setting up the TTBRx and TTBCR registers.
 	 * - \ttb0 and \ttb1 updated with the corresponding flags.
 	 */
-	.macro	v7_ttb_setup, zero, ttbr0, ttbr1, tmp
+	.macro	v7_ttb_setup, zero, ttbr0l, ttbr0h, ttbr1, tmp
 	mcr	p15, 0, \zero, c2, c0, 2	@ TTB control register
-	ALT_SMP(orr	\ttbr0, \ttbr0, #TTB_FLAGS_SMP)
-	ALT_UP(orr	\ttbr0, \ttbr0, #TTB_FLAGS_UP)
+	ALT_SMP(orr	\ttbr0l, \ttbr0l, #TTB_FLAGS_SMP)
+	ALT_UP(orr	\ttbr0l, \ttbr0l, #TTB_FLAGS_UP)
 	ALT_SMP(orr	\ttbr1, \ttbr1, #TTB_FLAGS_SMP)
 	ALT_UP(orr	\ttbr1, \ttbr1, #TTB_FLAGS_UP)
 	mcr	p15, 0, \ttbr1, c2, c0, 1	@ load TTB1
arch/arm/mm/proc-v7-3level.S +5 -9
index d3daed0ae0ad..5e5720e8bc5f 100644
@@ -126,11 +126,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 * Macro for setting up the TTBRx and TTBCR registers.
 	 * - \ttbr1 updated.
 	 */
-	.macro	v7_ttb_setup, zero, ttbr0, ttbr1, tmp
+	.macro	v7_ttb_setup, zero, ttbr0l, ttbr0h, ttbr1, tmp
 	ldr	\tmp, =swapper_pg_dir		@ swapper_pg_dir virtual address
-	mov	\tmp, \tmp, lsr #ARCH_PGD_SHIFT
-	cmp	\ttbr1, \tmp			@ PHYS_OFFSET > PAGE_OFFSET?
-	mrc	p15, 0, \tmp, c2, c0, 2		@ TTB control register
+	cmp	\ttbr1, \tmp, lsr #12		@ PHYS_OFFSET > PAGE_OFFSET?
+	mrc	p15, 0, \tmp, c2, c0, 2		@ TTB control egister
 	orr	\tmp, \tmp, #TTB_EAE
 	ALT_SMP(orr	\tmp, \tmp, #TTB_FLAGS_SMP)
 	ALT_UP(orr	\tmp, \tmp, #TTB_FLAGS_UP)

@@ -143,13 +142,10 @@ ENDPROC(cpu_v7_set_pte_ext)
 	 */
 	orrls	\tmp, \tmp, #TTBR1_SIZE				@ TTBCR.T1SZ
 	mcr	p15, 0, \tmp, c2, c0, 2				@ TTBCR
-	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
-	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
+	mov	\tmp, \ttbr1, lsr #20
+	mov	\ttbr1, \ttbr1, lsl #12
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
 	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
-	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
-	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
-	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0
 	.endm
 
 	/*
arch/arm/mm/proc-v7.S +13 -13
index 3d1054f11a8a..873230912894 100644
@@ -343,9 +343,9 @@ __v7_setup:
 	and	r10, r0, #0xff000000		@ ARM?
 	teq	r10, #0x41000000
 	bne	3f
-	and	r5, r0, #0x00f00000		@ variant
+	and	r3, r0, #0x00f00000		@ variant
 	and	r6, r0, #0x0000000f		@ revision
-	orr	r6, r6, r5, lsr #20-4		@ combine variant and revision
+	orr	r6, r6, r3, lsr #20-4		@ combine variant and revision
 	ubfx	r0, r0, #4, #12			@ primary part number
 
 	/* Cortex-A8 Errata */

@@ -354,7 +354,7 @@ __v7_setup:
 	bne	2f
 #if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM)
 
-	teq	r5, #0x00100000			@ only present in r1p*
+	teq	r3, #0x00100000			@ only present in r1p*
 	mrceq	p15, 0, r10, c1, c0, 1		@ read aux control register
 	orreq	r10, r10, #(1 << 6)		@ set IBE to 1
 	mcreq	p15, 0, r10, c1, c0, 1		@ write aux control register

@@ -395,7 +395,7 @@ __v7_setup:
 	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register
 #endif
 #ifdef CONFIG_ARM_ERRATA_743622
-	teq	r5, #0x00200000			@ only present in r2p*
+	teq	r3, #0x00200000			@ only present in r2p*
 	mrceq	p15, 0, r10, c15, c0, 1		@ read diagnostic register
 	orreq	r10, r10, #1 << 6		@ set bit #6
 	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register

@@ -425,10 +425,10 @@ __v7_setup:
 	mcr	p15, 0, r10, c7, c5, 0		@ I+BTB cache invalidate
 #ifdef CONFIG_MMU
 	mcr	p15, 0, r10, c8, c7, 0		@ invalidate I + D TLBs
-	v7_ttb_setup r10, r4, r8, r5		@ TTBCR, TTBRx setup
-	ldr	r5, =PRRR			@ PRRR
+	v7_ttb_setup r10, r4, r5, r8, r3	@ TTBCR, TTBRx setup
+	ldr	r3, =PRRR			@ PRRR
 	ldr	r6, =NMRR			@ NMRR
-	mcr	p15, 0, r5, c10, c2, 0		@ write PRRR
+	mcr	p15, 0, r3, c10, c2, 0		@ write PRRR
 	mcr	p15, 0, r6, c10, c2, 1		@ write NMRR
 #endif
 	dsb					@ Complete invalidations

@@ -437,22 +437,22 @@ __v7_setup:
 	and	r0, r0, #(0xf << 12)		@ ThumbEE enabled field
 	teq	r0, #(1 << 12)			@ check if ThumbEE is present
 	bne	1f
-	mov	r5, #0
-	mcr	p14, 6, r5, c1, c0, 0		@ Initialize TEEHBR to 0
+	mov	r3, #0
+	mcr	p14, 6, r3, c1, c0, 0		@ Initialize TEEHBR to 0
 	mrc	p14, 6, r0, c0, c0, 0		@ load TEECR
 	orr	r0, r0, #1			@ set the 1st bit in order to
 	mcr	p14, 6, r0, c0, c0, 0		@ stop userspace TEEHBR access
 1:
 #endif
-	adr	r5, v7_crval
-	ldmia	r5, {r5, r6}
+	adr	r3, v7_crval
+	ldmia	r3, {r3, r6}
  ARM_BE8(orr	r6, r6, #1 << 25)		@ big-endian page tables
 #ifdef CONFIG_SWP_EMULATE
-	orr     r5, r5, #(1 << 10)              @ set SW bit in "clear"
+	orr     r3, r3, #(1 << 10)              @ set SW bit in "clear"
 	bic     r6, r6, #(1 << 10)              @ clear it in "mmuset"
 #endif
    	mrc	p15, 0, r0, c1, c0, 0		@ read control register
-	bic	r0, r0, r5			@ clear bits them
+	bic	r0, r0, r3			@ clear bits them
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#12 lore raw

PATCH v2 7/7 ARM: redo TTBR setup code for LPAE

Quoted 3 lines · Gregory CLEMENT

It seems that since the merge of this commit the secondary CPUs on Armada XP failed to come online in Big Endian (my configuration is mvebu_v7_defconfig+CONFIG_CPU_BIG_ENDIAN=y).

That's probably because I don't test big endian at all, never have done, and I suspect never will as I've never had systems using BE. So, I regard BE as something of a troublesome untestable burden on kernel development. (And I've always regarded BE to be an abomination.) In short, I loath big endian, which really doesn't bode well for me not constantly cocking it up...

My guess is that it all comes down to much of the new code expecting the value in r4/r5 to be the least significant 32bits in r4 and the most significant 32bits in r5. However, in the secondary code, we load this using ldrd, which on BE probably reverses that.

Quoted 2 lines

Once I found that this commit introduced the regression I also tested but with LPAE but I had the same result: only the first CPU boot.

Not surprising. I guess BE would need the two registers to mcrr reversing if they were a 64-bit value loaded with ldrd.

The simple answer may just be to swap r4/r5 after the ldrd. Trying to do something different is likely to get into very troublesome problems, ensuring that r5 doesn't get overwritten (eg, the code which loads the domain access register would have to switch to a different register - maybe r4? - but then that needs to be dependent on LE or BE...) and we're really not going to end up with lots of conditional register usage just because of annoying BE differences scattered throughout the code.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
#13 lore raw

PATCH 7/7 ARM: redo TTBR setup code for LPAE

Quoted 12 lines · Russell King - ARM Linux
Quoted 3 lines · Gregory CLEMENT

It seems that since the merge of this commit the secondary CPUs on Armada XP failed to come online in Big Endian (my configuration is mvebu_v7_defconfig+CONFIG_CPU_BIG_ENDIAN=y).

That's probably because I don't test big endian at all, never have done, and I suspect never will as I've never had systems using BE. So, I regard BE as something of a troublesome untestable burden on kernel development. (And I've always regarded BE to be an abomination.) In short, I loath big endian, which really doesn't bode well for me not constantly cocking it up...

Quoted 4 lines

My guess is that it all comes down to much of the new code expecting the value in r4/r5 to be the least significant 32bits in r4 and the most significant 32bits in r5. However, in the secondary code, we load this using ldrd, which on BE probably reverses that.

Thanks for the explanation.

Quoted 8 lines
Quoted 3 lines

Once I found that this commit introduced the regression I also tested but with LPAE but I had the same result: only the first CPU boot.

Not surprising. I guess BE would need the two registers to mcrr reversing if they were a 64-bit value loaded with ldrd.

The simple answer may just be to swap r4/r5 after the ldrd.

I tried this and it fixed the issue.
I will send a patch shortly and if you agree I will submit it to your patch system.

Thanks,

Gregory

Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com