Skip to contentSkip to conversation

arm: socfpga: reserve the region at start of phys mem

PATCH New Sebastian Andrzej Siewior 4 participants 8 messages Feb 18, 2014 – Apr 30, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

The SMP bringup code copies trampline code to the physical location 0x0. If somebody allocated memory from this location then it will be overwritten.
This patch reserves the few bytes so that it won't be used by the memory allocator.

---
 arch/arm/mach-socfpga/socfpga.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/arm/mach-socfpga/socfpga.c +13 -0
index a9050e6..2ba992a 100644
@@ -26,6 +26,7 @@
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
 #include <linux/sys_soc.h>
+#include <linux/memblock.h>
 
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
@@ -315,6 +316,17 @@ static void __init socfpga_cyclone5_init(void)
 	socfpga_soc_device_init();
 }
 
+static void __init socfmpga_smp_reserve(void)
+{
+#ifdef CONFIG_SMP
+	int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
+	int ret;
+
+	ret = memblock_reserve(0, trampoline_size);
+	WARN_ON(ret);
+#endif
+}
+
 static const char *altera_dt_match[] = {
 	"altr,socfpga",
 	NULL
@@ -327,4 +339,5 @@ DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA")
 	.init_machine	= socfpga_cyclone5_init,
 	.restart	= socfpga_cyclone5_restart,
 	.dt_compat	= altera_dt_match,
+	.reserve	= socfmpga_smp_reserve,
 MACHINE_END
Signature · 1 line
1.9.0.rc3
#1 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

Quoted 24 lines · Sebastian Andrzej Siewior

The SMP bringup code copies trampline code to the physical location 0x0. If somebody allocated memory from this location then it will be overwritten.
This patch reserves the few bytes so that it won't be used by the memory allocator.

---
 arch/arm/mach-socfpga/socfpga.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/arm/mach-socfpga/socfpga.c +1 -0
index a9050e6..2ba992a 100644
@@ -26,6 +26,7 @@
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
 #include <linux/sys_soc.h>
+#include <linux/memblock.h>
 
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>

arch/arm/mach-socfpga/socfpga.c static void __init socfpga_cyclone5_init(void) · line 319

Quoted 4 lines
arch/arm/mach-socfpga/socfpga.c +1 -0
@@ -315,6 +316,17 @@ static void __init socfpga_cyclone5_init(void)
 	socfpga_soc_device_init();
 }
 
+static void __init socfmpga_smp_reserve(void)

s/socfmpga/socfpga

Quoted 14 lines
arch/arm/mach-socfpga/socfpga.c +10 -0
+{
+#ifdef CONFIG_SMP
+	int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
+	int ret;
+
+	ret = memblock_reserve(0, trampoline_size);
+	WARN_ON(ret);
+#endif
+}
+
 static const char *altera_dt_match[] = {
 	"altr,socfpga",
 	NULL

arch/arm/mach-socfpga/socfpga.c DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA") · line 342

Quoted 4 lines
arch/arm/mach-socfpga/socfpga.c +1 -0
@@ -327,4 +339,5 @@ DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA")
 	.init_machine	= socfpga_cyclone5_init,
 	.restart	= socfpga_cyclone5_restart,
 	.dt_compat	= altera_dt_match,
+	.reserve	= socfmpga_smp_reserve,

same..

I've applied this patch with the above edits.

Thanks,
Dinh

Quoted 1 line
arch/arm/mach-socfpga/socfpga.c +0 -0
 MACHINE_END
#2 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

Quoted 1 line · Dinh Nguyen

I've applied this patch with the above edits.

Thank you.

Quoted 3 lines

Thanks,
Dinh

Sebastian

#3 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

In reply to Dinh Nguyen
Quoted 1 line · Dinh Nguyen

I've applied this patch with the above edits.

I could find this patch v3.15-rc3 and I couldn't find it in your next git tree. Did I miss something?

Quoted 3 lines

Thanks,
Dinh

Sebastian

#4 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

In reply to Sebastian Andrzej Siewior
Quoted 7 lines · Sebastian Andrzej Siewior

The SMP bringup code copies trampline code to the physical location 0x0. If somebody allocated memory from this location then it will be overwritten.
This patch reserves the few bytes so that it won't be used by the memory allocator.

[..]

Quoted 15 lines
arch/arm/mach-socfpga/socfpga.c +11 -0
+static void __init socfmpga_smp_reserve(void)
+{
+#ifdef CONFIG_SMP
+	int trampoline_size = &secondary_trampoline_end - &secondary_trampoline;
+	int ret;
+
+	ret = memblock_reserve(0, trampoline_size);
+	WARN_ON(ret);
+#endif
+}
+
 static const char *altera_dt_match[] = {
 	"altr,socfpga",
 	NULL

arch/arm/mach-socfpga/socfpga.c DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA") · line 343

Quoted 5 lines
arch/arm/mach-socfpga/socfpga.c +1 -0
@@ -327,4 +339,5 @@ DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA")
 	.init_machine	= socfpga_cyclone5_init,
 	.restart	= socfpga_cyclone5_restart,
 	.dt_compat	= altera_dt_match,
+	.reserve	= socfmpga_smp_reserve,
 MACHINE_END

Is there a particular reason why you aren't describing this reservation in devicetree using a /memreserve/ node?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#5 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

Quoted 2 lines · Josh Cartwright

Is there a particular reason why you aren't describing this reservation in devicetree using a /memreserve/ node?

Because the kernel is using memory at a specific address it did not allocate. If the memory address where the second CPU come up could be set then this node wouldn't be required and a simple kmalloc() would do it, too.

Sebastian

#6 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

Quoted 8 lines · Sebastian Andrzej Siewior
Quoted 2 lines · Josh Cartwright

Is there a particular reason why you aren't describing this reservation in devicetree using a /memreserve/ node?

Because the kernel is using memory at a specific address it did not allocate. If the memory address where the second CPU come up could be set then this node wouldn't be required and a simple kmalloc() would do it, too.

I understand the need to reserve the area, but questioning the mechanism used to do so. How is the socfpga case different from Highbank, for example, which makes use of /memreserve/ for what appears to be a very similar purpose (see arch/arm/boot/dts/highbank.dts):

	/* First 4KB has pen for secondary cores. */
	/memreserve/ 0x00000000 0x0001000;
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#7 lore raw

PATCH 1/1 arm: socfpga: reserve the region at start of phys mem

Hi Sebastian,

Quoted 8 lines · Josh Cartwright
Quoted 7 lines · Sebastian Andrzej Siewior
Quoted 2 lines · Josh Cartwright

Is there a particular reason why you aren't describing this reservation in devicetree using a /memreserve/ node?

Because the kernel is using memory at a specific address it did not allocate. If the memory address where the second CPU come up could be set then this node wouldn't be required and a simple kmalloc() would do it, too.

Quoted 8 lines

I understand the need to reserve the area, but questioning the mechanism used to do so. How is the socfpga case different from Highbank, for example, which makes use of /memreserve/ for what appears to be a very similar purpose (see arch/arm/boot/dts/highbank.dts):

	/* First 4KB has pen for secondary cores. */
	/memreserve/ 0x00000000 0x0001000;

I apologize but it looks like I accidently dropped this patch. But in light of Josh's
comments, I think doing the /memreserve/ is the correct way.

Dinh