Skip to contentSkip to conversation

of: Specify initrd location using 64-bit

PATCH In review Santosh Shilimkar 8 participants 16 messages Jun 21, 2013 – Jul 1, 2013

Subsystems (10)
Arm (32-bit) x86 PowerPC MIPS +6 more subsystems
lore
Share & tools
lore

Santosh Shilimkar

author
#0 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

On some PAE architectures, the entire range of physical memory could reside outside the 32-bit limit. These systems need the ability to specify the initrd location using 64-bit numbers.

This patch globally modifies the early_init_dt_setup_initrd_arch() function to use 64-bit numbers instead of the current unsigned long.

Patch is refreshed version of Cyril's original patch against 3.10-rcx which has few more arch's which needs to be patched. Have to drop Cyril's email id since its broken now.

Boot tested on ARM platform and just build tested for x86 platform. Technically it should not break anything but cc list check if it creates any issue for your arch.

Cc Vineet Gupta <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Salter <[email protected]>
Cc Aurelien Jacquiot <[email protected]>
Cc James Hogan <[email protected]>
Cc Michal Simek <[email protected]>
Cc Ralf Baechle <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc x86 at kernel.org
Cc arm at kernel.org
Cc Chris Zankel <[email protected]>
Cc Max Filippov <[email protected]>
Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc bigeasy at linutronix.de
Cc robherring2 at gmail.com
Cc Nicolas Pitre <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-c6x-dev at linux-c6x.org
Cc linux-mips at linux-mips.org
Cc linuxppc-dev at lists.ozlabs.org
Cc linux-xtensa at linux-xtensa.org
Cc devicetree-discuss at lists.ozlabs.org
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arc/mm/init.c            |    3 +--
 arch/arm/mm/init.c            |    2 +-
 arch/arm64/mm/init.c          |    3 +--
 arch/c6x/kernel/devicetree.c  |    3 +--
 arch/metag/mm/init.c          |    3 +--
 arch/microblaze/kernel/prom.c |    3 +--
 arch/mips/kernel/prom.c       |    3 +--
 arch/openrisc/kernel/prom.c   |    3 +--
 arch/powerpc/kernel/prom.c    |    3 +--
 arch/x86/kernel/devicetree.c  |    3 +--
 arch/xtensa/kernel/setup.c    |    3 +--
 drivers/of/fdt.c              |   10 ++++++----
 include/linux/of_fdt.h        |    3 +--
 13 files changed, 18 insertions(+), 27 deletions(-)
arch/arc/mm/init.c +1 -2
index 4a17736..3640c74 100644
@@ -157,8 +157,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	pr_err("%s(%lx, %lx)\n", __func__, start, end);
 }
arch/arm/mm/init.c +1 -1
index 9a5cdc0..afeaef7 100644
@@ -76,7 +76,7 @@ static int __init parse_tag_initrd2(const struct tag *tag)
 __tagtable(ATAG_INITRD2, parse_tag_initrd2);
 
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start, unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	phys_initrd_start = start;
 	phys_initrd_size = end - start;
arch/arm64/mm/init.c +1 -2
index f497ca7..7047708 100644
@@ -44,8 +44,7 @@ static unsigned long phys_initrd_size __initdata = 0;
 
 phys_addr_t memstart_addr __read_mostly = 0;
 
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	phys_initrd_start = start;
 	phys_initrd_size = end - start;
arch/c6x/kernel/devicetree.c +1 -2
index bdb56f0..287d0e6 100644
@@ -33,8 +33,7 @@ void __init early_init_devtree(void *params)
 
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/metag/mm/init.c +1 -2
index d05b845..07b4412 100644
@@ -419,8 +419,7 @@ void free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	pr_err("%s(%lx, %lx)\n",
 	       __func__, start, end);
arch/microblaze/kernel/prom.c +1 -2
index 0a2c68f..62e2e8f 100644
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/mips/kernel/prom.c +1 -2
index 5712bb5..32b8788 100644
@@ -58,8 +58,7 @@ void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/openrisc/kernel/prom.c +1 -2
index 5869e3f..150215a 100644
@@ -96,8 +96,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/powerpc/kernel/prom.c +1 -2
index 8b6f7a9..2f3e252 100644
@@ -550,8 +550,7 @@ void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/x86/kernel/devicetree.c +1 -2
index b158152..2fbad6b 100644
@@ -52,8 +52,7 @@ void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
arch/xtensa/kernel/setup.c +1 -2
index 6dd25ec..d45e602 100644
@@ -170,8 +170,7 @@ static int __init parse_tag_fdt(const bp_tag_t *tag)
 
 __tagtable(BP_TAG_FDT, parse_tag_fdt);
 
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (void *)__va(start);
 	initrd_end = (void *)__va(end);
drivers/of/fdt.c +6 -4
index 808be06..21123b8 100644
@@ -550,7 +550,8 @@ int __init of_flat_dt_match(unsigned long node, const char *const *compat)
  */
 void __init early_init_dt_check_for_initrd(unsigned long node)
 {
-	unsigned long start, end, len;
+	u64 start, end;
+	unsigned long len;
 	__be32 *prop;
 
 	pr_debug("Looking for initrd properties... ");

@@ -558,15 +559,16 @@ void __init early_init_dt_check_for_initrd(unsigned long node)
 	prop = of_get_flat_dt_prop(node, "linux,initrd-start", &len);
 	if (!prop)
 		return;
-	start = of_read_ulong(prop, len/4);
+	start = of_read_number(prop, len/4);
 
 	prop = of_get_flat_dt_prop(node, "linux,initrd-end", &len);
 	if (!prop)
 		return;
-	end = of_read_ulong(prop, len/4);
+	end = of_read_number(prop, len/4);
 
 	early_init_dt_setup_initrd_arch(start, end);
-	pr_debug("initrd_start=0x%lx  initrd_end=0x%lx\n", start, end);
+	pr_debug("initrd_start=0x%llx  initrd_end=0x%llx\n",
+		 (unsigned long long)start, (unsigned long long)end);
 }
 #else
 inline void early_init_dt_check_for_initrd(unsigned long node)
include/linux/of_fdt.h +1 -2
index ed136ad..4a17939 100644
@@ -106,8 +106,7 @@ extern u64 dt_mem_next_cell(int s, __be32 **cellp);
  * physical addresses.
  */
 #ifdef CONFIG_BLK_DEV_INITRD
-extern void early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end);
+extern void early_init_dt_setup_initrd_arch(u64 start, u64 end);
 #endif
 
 /* Early flat tree scan hooks */
Signature · 1 line
1.7.9.5
#1 lore raw

Hi Santosh,

Quoted 51 lines · Santosh Shilimkar
Cc Vineet Gupta <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Salter <[email protected]>
Cc Aurelien Jacquiot <[email protected]>
Cc James Hogan <[email protected]>
Cc Michal Simek <[email protected]>
Cc Ralf Baechle <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc x86 at kernel.org
Cc arm at kernel.org
Cc Chris Zankel <[email protected]>
Cc Max Filippov <[email protected]>
Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc bigeasy at linutronix.de
Cc robherring2 at gmail.com
Cc Nicolas Pitre <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-c6x-dev at linux-c6x.org
Cc linux-mips at linux-mips.org
Cc linuxppc-dev at lists.ozlabs.org
Cc linux-xtensa at linux-xtensa.org
Cc devicetree-discuss at lists.ozlabs.org
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arc/mm/init.c            |    3 +--
 arch/arm/mm/init.c            |    2 +-
 arch/arm64/mm/init.c          |    3 +--
 arch/c6x/kernel/devicetree.c  |    3 +--
 arch/metag/mm/init.c          |    3 +--
 arch/microblaze/kernel/prom.c |    3 +--
 arch/mips/kernel/prom.c       |    3 +--
 arch/openrisc/kernel/prom.c   |    3 +--
 arch/powerpc/kernel/prom.c    |    3 +--
 arch/x86/kernel/devicetree.c  |    3 +--
 arch/xtensa/kernel/setup.c    |    3 +--
 drivers/of/fdt.c              |   10 ++++++----
 include/linux/of_fdt.h        |    3 +--
 13 files changed, 18 insertions(+), 27 deletions(-)
arch/arc/mm/init.c +0 -0
index 4a17736..3640c74 100644

arch/arc/mm/init.c void __init free_initrd_mem(unsigned long start, unsigned long end) · line 163

Quoted 9 lines
arch/arc/mm/init.c +1 -2
@@ -157,8 +157,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	pr_err("%s(%lx, %lx)\n", __func__, start, end);
 }

To avoid gcc warnings, you need to fix the print format specifiers too.

Thx,
-Vineet

#2 lore raw
Quoted 56 lines · Vineet Gupta

Hi Santosh,

Quoted 53 lines · Santosh Shilimkar
Cc Vineet Gupta <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Salter <[email protected]>
Cc Aurelien Jacquiot <[email protected]>
Cc James Hogan <[email protected]>
Cc Michal Simek <[email protected]>
Cc Ralf Baechle <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc x86 at kernel.org
Cc arm at kernel.org
Cc Chris Zankel <[email protected]>
Cc Max Filippov <[email protected]>
Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc bigeasy at linutronix.de
Cc robherring2 at gmail.com
Cc Nicolas Pitre <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-c6x-dev at linux-c6x.org
Cc linux-mips at linux-mips.org
Cc linuxppc-dev at lists.ozlabs.org
Cc linux-xtensa at linux-xtensa.org
Cc devicetree-discuss at lists.ozlabs.org
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arc/mm/init.c            |    3 +--
 arch/arm/mm/init.c            |    2 +-
 arch/arm64/mm/init.c          |    3 +--
 arch/c6x/kernel/devicetree.c  |    3 +--
 arch/metag/mm/init.c          |    3 +--
 arch/microblaze/kernel/prom.c |    3 +--
 arch/mips/kernel/prom.c       |    3 +--
 arch/openrisc/kernel/prom.c   |    3 +--
 arch/powerpc/kernel/prom.c    |    3 +--
 arch/x86/kernel/devicetree.c  |    3 +--
 arch/xtensa/kernel/setup.c    |    3 +--
 drivers/of/fdt.c              |   10 ++++++----
 include/linux/of_fdt.h        |    3 +--
 13 files changed, 18 insertions(+), 27 deletions(-)
arch/arc/mm/init.c +0 -0
index 4a17736..3640c74 100644
@@ -157,8 +157,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
Quoted 9 lines

arch/arc/mm/init.c void __init free_initrd_mem(unsigned long start, unsigned long end) · line 163

Quoted 7 lines
arch/arc/mm/init.c +1 -2
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	pr_err("%s(%lx, %lx)\n", __func__, start, end);
 }

To avoid gcc warnings, you need to fix the print format specifiers too.

The same thing goes for arch/metag too.

Cheers
James

author
#3 lore raw

Vineet, James,

Quoted 67 lines · James Hogan
Quoted 56 lines · Vineet Gupta

Hi Santosh,

Quoted 53 lines · Santosh Shilimkar
Cc Vineet Gupta <[email protected]>
Cc Russell King <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Salter <[email protected]>
Cc Aurelien Jacquiot <[email protected]>
Cc James Hogan <[email protected]>
Cc Michal Simek <[email protected]>
Cc Ralf Baechle <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc x86 at kernel.org
Cc arm at kernel.org
Cc Chris Zankel <[email protected]>
Cc Max Filippov <[email protected]>
Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc bigeasy at linutronix.de
Cc robherring2 at gmail.com
Cc Nicolas Pitre <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Cc linux-c6x-dev at linux-c6x.org
Cc linux-mips at linux-mips.org
Cc linuxppc-dev at lists.ozlabs.org
Cc linux-xtensa at linux-xtensa.org
Cc devicetree-discuss at lists.ozlabs.org
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arc/mm/init.c            |    3 +--
 arch/arm/mm/init.c            |    2 +-
 arch/arm64/mm/init.c          |    3 +--
 arch/c6x/kernel/devicetree.c  |    3 +--
 arch/metag/mm/init.c          |    3 +--
 arch/microblaze/kernel/prom.c |    3 +--
 arch/mips/kernel/prom.c       |    3 +--
 arch/openrisc/kernel/prom.c   |    3 +--
 arch/powerpc/kernel/prom.c    |    3 +--
 arch/x86/kernel/devicetree.c  |    3 +--
 arch/xtensa/kernel/setup.c    |    3 +--
 drivers/of/fdt.c              |   10 ++++++----
 include/linux/of_fdt.h        |    3 +--
 13 files changed, 18 insertions(+), 27 deletions(-)
arch/arc/mm/init.c +0 -0
index 4a17736..3640c74 100644
@@ -157,8 +157,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
Quoted 9 lines

arch/arc/mm/init.c void __init free_initrd_mem(unsigned long start, unsigned long end) · line 163

Quoted 7 lines
arch/arc/mm/init.c +1 -2
 #ifdef CONFIG_OF_FLATTREE
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-					    unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	pr_err("%s(%lx, %lx)\n", __func__, start, end);
 }

To avoid gcc warnings, you need to fix the print format specifiers too.

Quoted 2 lines

The same thing goes for arch/metag too.

Will update that in next version based on what we will end after the dicussion. i.e u64 or phys_addr_t

Thanks for comments.

Regards,
Santosh

#4 lore raw
In reply to Santosh Shilimkar
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

Sebastian

author
#5 lore raw
Quoted 22 lines · Sebastian Andrzej Siewior
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

Quoted 4 lines

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

That pretty much means phys_addr_t. It will work for me as well but in last thread from consistency with memory and reserved node, Rob insisted to keep it as u64. So before I re-spin another version, would like to here what Rob has to say considering the x86 requirement.

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

Regards,
Santosh

#6 lore raw
Quoted 27 lines · Santosh Shilimkar
Quoted 22 lines · Sebastian Andrzej Siewior
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-		unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

Quoted 4 lines

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

Quoted 8 lines

That pretty much means phys_addr_t. It will work for me as well but in last thread from consistency with memory and reserved node, Rob insisted to keep it as u64. So before I re-spin another version, would like to here what Rob has to say considering the x86 requirement.

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

Rob

#7 lore raw
Quoted 5 lines · Rob Herring
Quoted 3 lines

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

The physical address space is represented by phys_addr_t and not u64 within the kernel. If you go for u64 you may waste 32bit and you need to check if the running kernel can deal with this.
Why was u64 such a good thing?

Quoted 1 line

Rob

Sebastian

#8 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

In reply to Rob Herring
Quoted 33 lines · Rob Herring
Quoted 32 lines · Santosh Shilimkar
Quoted 22 lines · Sebastian Andrzej Siewior
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-           unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
    initrd_start = (unsigned long)__va(start);
    initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

Quoted 4 lines

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

That pretty much means phys_addr_t. It will work for me as well but in last thread from consistency with memory and reserved node, Rob insisted to keep it as u64. So before I re-spin another version, would like to here what Rob has to say considering the x86 requirement.

Quoted 5 lines
Quoted 3 lines

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

+1

g.

#9 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

Quoted 34 lines · Grant Likely
Quoted 33 lines · Rob Herring
Quoted 32 lines · Santosh Shilimkar
Quoted 22 lines · Sebastian Andrzej Siewior
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-           unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
    initrd_start = (unsigned long)__va(start);
    initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

Quoted 4 lines

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

That pretty much means phys_addr_t. It will work for me as well but in last thread from consistency with memory and reserved node, Rob insisted to keep it as u64. So before I re-spin another version, would like to here what Rob has to say considering the x86 requirement.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

+1

+1

fix type

Best Regards,
J.

author
#10 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

Sebastian,

Quoted 39 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 38 lines · Grant Likely
Quoted 37 lines · Rob Herring
Quoted 35 lines · Santosh Shilimkar
Quoted 22 lines · Sebastian Andrzej Siewior
Quoted 5 lines · Santosh Shilimkar
arch/microblaze/kernel/prom.c +0 -0
index 0a2c68f..62e2e8f 100644

arch/microblaze/kernel/prom.c void __init early_init_devtree(void *params) · line 142

Quoted 9 lines
arch/microblaze/kernel/prom.c +1 -2
@@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-void __init early_init_dt_setup_initrd_arch(unsigned long start,
-           unsigned long end)
+void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
 {
    initrd_start = (unsigned long)__va(start);
    initrd_end = (unsigned long)__va(end);

I think it would better to go here for phys_addr_t instead of u64. This would force you in of_flat_dt_match() to check if the value passed from DT specifies a memory address outside of 32bit address space and the kernel can't deal with this because its phys_addr_t is 32bit only due to a Kconfig switch.

Quoted 4 lines

For x86, the initrd has to remain in the 32bit address space so passing the initrd in the upper range would violate the ABI. Not sure if this is true for other archs as well (ARM obviously not).

That pretty much means phys_addr_t. It will work for me as well but in last thread from consistency with memory and reserved node, Rob insisted to keep it as u64. So before I re-spin another version, would like to here what Rob has to say considering the x86 requirement.

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

No. I still think it should be u64 for same reasons I said originally.

+1

+1

fix type

Apart from waste of 32bit, what is the other concern you have ? I really want to converge on this patch because it has been a open ended discussion for quite some time. Does that really break any thing on x86 or your concern is more from semantics of the physical address.

Thanks for help.

Regards,
Santosh

#11 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

Quoted 10 lines · Santosh Shilimkar
Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Rob,
Are you ok with phys_addr_t since your concern was about rest of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

+1

+1

Quoted 7 lines
Quoted 2 lines

fix type

Apart from waste of 32bit, what is the other concern you have ? I really want to converge on this patch because it has been a open ended discussion for quite some time. Does that really break any thing on x86 or your concern is more from semantics of the physical address.

As the "original reasons" were not in this thread, I had to search a bit. I suppose you mean this one: https://lkml.org/lkml/2012/9/13/544 ?

Summarized:

| The address to load the initrd is decided by the bootloader/user and set
| at that point later in time.
| The dtb should not be tied to the kernel you are booting.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#12 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

In reply to Santosh Shilimkar
Quoted 5 lines · Santosh Shilimkar

Sebastian,

Apart from waste of 32bit, what is the other concern you have ?

You pass a u64 as a physical address which is represented in other parts of the kernel (for a good reason) by phys_addr_t.

Quoted 4 lines

I really want to converge on this patch because it has been a open ended discussion for quite some time. Does that really break any thing on x86 or your concern is more from semantics of the physical address.

You want to have your code in so you can continue with your work, that is okay. The other two arguments why u64 here is a good thing was "due to what I said earlier" and "+1" and I don't have the time to look that up.

There should be no problems on x86 if this goes in as it is now.

But think about this: What happens if you boot your ARM device without PAE and your initrd is in the upper region? If you are lucky the kernel looks at a different place where it also has a read permission, notices nothing sane is there, writes a message and continues. And if it is not allowed to read? It is clearly the user's fault for booting a non-PAE kernel.

Quoted 5 lines

Thanks for help.

Regards,
Santosh

Sebastian

#13 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

Quoted 16 lines · Sebastian Andrzej Siewior
Quoted 2 lines · Santosh Shilimkar

Apart from waste of 32bit, what is the other concern you have ?

You pass a u64 as a physical address which is represented in other parts of the kernel (for a good reason) by phys_addr_t.

Quoted 4 lines

I really want to converge on this patch because it has been a open ended discussion for quite some time. Does that really break any thing on x86 or your concern is more from semantics of the physical address.

You want to have your code in so you can continue with your work, that is okay. The other two arguments why u64 here is a good thing was "due to what I said earlier" and "+1" and I don't have the time to look that up.

Quoted 8 lines

There should be no problems on x86 if this goes in as it is now.

But think about this: What happens if you boot your ARM device without PAE and your initrd is in the upper region? If you are lucky the kernel looks at a different place where it also has a read permission, notices nothing sane is there, writes a message and continues. And if it is not allowed to read? It is clearly the user's fault for booting a non-PAE kernel.

That's actual the original reason: DT has it as 64 bit, and passes it to a 32 bit kernel when running in 32 bit mode without PAE.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#14 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

Quoted 2 lines · Geert Uytterhoeven

That's actual the original reason: DT has it as 64 bit, and passes it to a 32 bit kernel when running in 32 bit mode without PAE.

And I think the DT code should check if the u64 fits in phys_addr_t and if does not it should write an error message and act like no initrd was specified (instead of passing "something" to the architecture).

Quoted 5 lines

Gr{oetje,eeting}s,

Geert

Sebastian

author
#15 lore raw

PATCH 1/1 of: Specify initrd location using 64-bit

In reply to Geert Uytterhoeven
Quoted 27 lines · Geert Uytterhoeven
Quoted 16 lines · Sebastian Andrzej Siewior
Quoted 2 lines · Santosh Shilimkar

Apart from waste of 32bit, what is the other concern you have ?

You pass a u64 as a physical address which is represented in other parts of the kernel (for a good reason) by phys_addr_t.

Quoted 4 lines

I really want to converge on this patch because it has been a open ended discussion for quite some time. Does that really break any thing on x86 or your concern is more from semantics of the physical address.

You want to have your code in so you can continue with your work, that is okay. The other two arguments why u64 here is a good thing was "due to what I said earlier" and "+1" and I don't have the time to look that up.

Quoted 8 lines

There should be no problems on x86 if this goes in as it is now.

But think about this: What happens if you boot your ARM device without PAE and your initrd is in the upper region? If you are lucky the kernel looks at a different place where it also has a read permission, notices nothing sane is there, writes a message and continues. And if it is not allowed to read? It is clearly the user's fault for booting a non-PAE kernel.

Quoted 3 lines

That's actual the original reason: DT has it as 64 bit, and passes it to a 32 bit kernel when running in 32 bit mode without PAE.

Thanks all for comments and useful discussion. I will resubmit the patch with update to fix the printk warnings reported by Vineet and James post the $subject change.

Am assuming the patch will go via Grant Likely's tree.

Regards,
Santosh