[PATCHv2] arm: socfpga: fix fetching cpu1start_addr for system with > 2GB of ram

[PATCHv2] arm: socfpga: fix fetching cpu1start_addr for system with > 2GB of ram

[dinguyen at opensource.altera.com]

From: Dinh Nguyen <dinguyen@opensource.altera.com>

When CPU1 is brought out of reset, it's MMU is not turned yet, so it will only
be able to use physical addresses. For systems with 1GB or less, clearing
0x40000000 will work just fine, as it was just converting the virtual address
of &cpu1start_addr into a physical address. But for systems with > 2BG, bit
clearing 0x40000000 is not enough to get the physical address of &cpu1start_addr
correctly.

This patch fixes the secondary_trampoline code to correctly fetch the physical
address of cpu1start_addr directly.

While at it, change the name of cpu1start_addr to socfpga_cpu1start_addr
to avoid any future naming collisions for multiplatform image.

Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
v2: Correctly get the physical address instead of just a BIC hack.
---
 arch/arm/mach-socfpga/core.h    |  2 +-
 arch/arm/mach-socfpga/headsmp.S | 13 +++++--------
 arch/arm/mach-socfpga/platsmp.c |  5 +++--
 arch/arm/mach-socfpga/socfpga.c |  3 +--
 4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
index 572b8f7..60c443d 100644
--- a/arch/arm/mach-socfpga/core.h
+++ b/arch/arm/mach-socfpga/core.h
@@ -40,7 +40,7 @@ extern void __iomem *rst_manager_base_addr;
 extern struct smp_operations socfpga_smp_ops;
 extern char secondary_trampoline, secondary_trampoline_end;
 
-extern unsigned long cpu1start_addr;
+extern unsigned long socfpga_cpu1start_addr;
 
 #define SOCFPGA_SCU_VIRT_BASE   0xfffec000
 
diff --git a/arch/arm/mach-socfpga/headsmp.S b/arch/arm/mach-socfpga/headsmp.S
index 95c115d..43cbbf5 100644
--- a/arch/arm/mach-socfpga/headsmp.S
+++ b/arch/arm/mach-socfpga/headsmp.S
@@ -13,17 +13,14 @@
 	.arch	armv7-a
 
 ENTRY(secondary_trampoline)
-	movw	r2, #:lower16:cpu1start_addr
-	movt  r2, #:upper16:cpu1start_addr
-
-	/* The socfpga VT cannot handle a 0xC0000000 page offset when loading
-		the cpu1start_addr, we bit clear it. Tested on HW and VT. */
-	bic	r2, r2, #0x40000000
-
-	ldr	r0, [r2]
+	ldr	r0, 1f
 	ldr	r1, [r0]
 	bx	r1
 
+	.globl	socfpga_cpu1start_addr
+socfpga_cpu1start_addr:
+1:	.space	4
+
 ENTRY(secondary_trampoline_end)
 
 ENTRY(socfpga_secondary_startup)
diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c
index 5356a72..ba84c58 100644
--- a/arch/arm/mach-socfpga/platsmp.c
+++ b/arch/arm/mach-socfpga/platsmp.c
@@ -33,11 +33,12 @@ static int socfpga_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
 
-	if (cpu1start_addr) {
+	if (socfpga_cpu1start_addr) {
 		memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size);
 
 		__raw_writel(virt_to_phys(socfpga_secondary_startup),
-			(sys_manager_base_addr + (cpu1start_addr & 0x000000ff)));
+			(sys_manager_base_addr + (socfpga_cpu1start_addr
+						  & 0x000000ff)));
 
 		flush_cache_all();
 		smp_wmb();
diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c
index adbf383..a6fdb98 100644
--- a/arch/arm/mach-socfpga/socfpga.c
+++ b/arch/arm/mach-socfpga/socfpga.c
@@ -29,7 +29,6 @@
 void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE));
 void __iomem *sys_manager_base_addr;
 void __iomem *rst_manager_base_addr;
-unsigned long cpu1start_addr;
 
 static struct map_desc scu_io_desc __initdata = {
 	.virtual	= SOCFPGA_SCU_VIRT_BASE,
@@ -70,7 +69,7 @@ void __init socfpga_sysmgr_init(void)
 	np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr");
 
 	if (of_property_read_u32(np, "cpu1-start-addr",
-			(u32 *) &cpu1start_addr))
+			(u32 *) &socfpga_cpu1start_addr))
 		pr_err("SMP: Need cpu1-start-addr in device tree.\n");
 
 	sys_manager_base_addr = of_iomap(np, 0);
-- 
2.0.3

[PATCHv2] arm: socfpga: fix fetching cpu1start_addr for system with > 2GB of ram

[Russell King - ARM Linux]

On Fri, Oct 03, 2014 at 04:09:00PM -0500, dinguyen at opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
> 
> When CPU1 is brought out of reset, it's MMU is not turned yet, so it will only
> be able to use physical addresses. For systems with 1GB or less, clearing
> 0x40000000 will work just fine, as it was just converting the virtual address
> of &cpu1start_addr into a physical address. But for systems with > 2BG, bit
> clearing 0x40000000 is not enough to get the physical address of &cpu1start_addr
> correctly.
> 
> This patch fixes the secondary_trampoline code to correctly fetch the physical
> address of cpu1start_addr directly.
> 
> While at it, change the name of cpu1start_addr to socfpga_cpu1start_addr
> to avoid any future naming collisions for multiplatform image.

I'm still not happy with this.  I see why my suggestion didn't work, but
this is still pretty dire - and will break when we add read-only text
support to the kernel (you are writing into the text segment.)

The reason my suggestion broke is because you aren't executing the code
in the kernel, you're copying it into SRAM and executing it there.  I
think you need to come up with some way to locate the physical address
in the SRAM, either before or after the function you're copying there.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCHv2] arm: socfpga: fix fetching cpu1start_addr for system with > 2GB of ram

[Dinh Nguyen]

Hi Russell,

On 10/03/2014 04:58 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 03, 2014 at 04:09:00PM -0500, dinguyen at opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> When CPU1 is brought out of reset, it's MMU is not turned yet, so it will only
>> be able to use physical addresses. For systems with 1GB or less, clearing
>> 0x40000000 will work just fine, as it was just converting the virtual address
>> of &cpu1start_addr into a physical address. But for systems with > 2BG, bit
>> clearing 0x40000000 is not enough to get the physical address of &cpu1start_addr
>> correctly.
>>
>> This patch fixes the secondary_trampoline code to correctly fetch the physical
>> address of cpu1start_addr directly.
>>
>> While at it, change the name of cpu1start_addr to socfpga_cpu1start_addr
>> to avoid any future naming collisions for multiplatform image.
> 
> I'm still not happy with this.  I see why my suggestion didn't work, but
> this is still pretty dire - and will break when we add read-only text
> support to the kernel (you are writing into the text segment.)
> 
> The reason my suggestion broke is because you aren't executing the code
> in the kernel, you're copying it into SRAM and executing it there.  I
> think you need to come up with some way to locate the physical address
> in the SRAM, either before or after the function you're copying there.
> 

This seems to also work. What do you think?

ENTRY(secondary_trampoline)
        adr     r0, 1f
        ldmia   r0, {r1, r2}
        sub     r2, r2, #PAGE_OFFSET
        ldr     r3, [r2]
        ldr     r4, [r3]
        bx      r4

        .align
1:      .long   .
        .long   cpu1start_addr
ENTRY(secondary_trampoline_end)