| Message ID | 1413313448-5835-1-git-send-email-dinguyen@opensource.altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 14, 2014 at 02:04:08PM -0500, dinguyen@opensource.altera.com wrote: > 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 r1, [r0] > - bx r1 > + adr r0, 1f > + ldmia r0, {r1, r2} > + sub r2, r2, #PAGE_OFFSET > + ldr r3, [r2] > + ldr r4, [r3] > + bx r4 > > + .align > +1: .long . > + .long socfpga_cpu1start_addr I'm still not happy with this. The old code used to take the socfpga_cpu1start_addr address (which will probably have both bits 31 and 30 set), and clear bit 30. That made the address in the range of 0x80000000 - 0xbfffffff. What you're doing above is taking the socfpga_cpu1start_addr address and subtracting PAGE_OFFSET from it. This will give an address in the 0x00000000 - 0x3fffffff range. PAGE_OFFSET is *not* the offset between physical and virtual addresses. It is the offset from _zero_ of the start of the kernel lowmem mapping. In other words, if lowmem starts at 0xc0000000, then PAGE_OFFSET will be 0xc0000000 whether or not your physical memory starts at 0, 0x10000000, 0x40000000, 0x50000000 or whatever. So, what this new code implies is that the start of physical memory is at zero on this platform. A comment to that effect in the code would be nice so that people understand what's going on here, and also know not to copy this code...
On 10/14/2014 02:18 PM, Russell King - ARM Linux wrote: > On Tue, Oct 14, 2014 at 02:04:08PM -0500, dinguyen@opensource.altera.com wrote: >> 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 r1, [r0] >> - bx r1 >> + adr r0, 1f >> + ldmia r0, {r1, r2} >> + sub r2, r2, #PAGE_OFFSET >> + ldr r3, [r2] >> + ldr r4, [r3] >> + bx r4 >> >> + .align >> +1: .long . >> + .long socfpga_cpu1start_addr > > I'm still not happy with this. > > The old code used to take the socfpga_cpu1start_addr address (which will > probably have both bits 31 and 30 set), and clear bit 30. That made the > address in the range of 0x80000000 - 0xbfffffff. That's correct. And with the our previous settings in the SDRAM controller, it was working because it appears that the access was just wrapping around when the controller was accessing memory outside of it's settings. Now that we've changed the memory controller's setting to block accesses beyond the max configured memory, simply clearing bit 30 will not work. > > What you're doing above is taking the socfpga_cpu1start_addr address and > subtracting PAGE_OFFSET from it. This will give an address in the > 0x00000000 - 0x3fffffff range. > > PAGE_OFFSET is *not* the offset between physical and virtual addresses. > It is the offset from _zero_ of the start of the kernel lowmem mapping. > > In other words, if lowmem starts at 0xc0000000, then PAGE_OFFSET will be > 0xc0000000 whether or not your physical memory starts at 0, 0x10000000, > 0x40000000, 0x50000000 or whatever. > > So, what this new code implies is that the start of physical memory is > at zero on this platform. A comment to that effect in the code would > be nice so that people understand what's going on here, and also know > not to copy this code... > Yes, the physical start of CPU1 is will always be zero for this platform. I will add a comment of something to that affect for V3. Thanks, Dinh
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..8c616e1 100644 --- a/arch/arm/mach-socfpga/headsmp.S +++ b/arch/arm/mach-socfpga/headsmp.S @@ -9,21 +9,21 @@ */ #include <linux/linkage.h> #include <linux/init.h> +#include <asm/memory.h> .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 r1, [r0] - bx r1 + adr r0, 1f + ldmia r0, {r1, r2} + sub r2, r2, #PAGE_OFFSET + ldr r3, [r2] + ldr r4, [r3] + bx r4 + .align +1: .long . + .long socfpga_cpu1start_addr 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..383d61e 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -29,7 +29,7 @@ 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; +unsigned long socfpga_cpu1start_addr; static struct map_desc scu_io_desc __initdata = { .virtual = SOCFPGA_SCU_VIRT_BASE, @@ -70,7 +70,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);