| Message ID | 1347465937-7056-1-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 9/12/2012 12:16 PM, Geert Uytterhoeven wrote: > On Wed, Sep 12, 2012 at 6:05 PM, Cyril Chemparathy <cyril@ti.com> wrote: >> 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. > >> -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) > > Why not phys_addr_t? > The rest of the memory specific bits of the device-tree code use u64 for addresses, and I kept it the same for consistency. > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 >
On 09/12/2012 08:02 PM, Cyril Chemparathy wrote: >>> -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) >> >> Why not phys_addr_t? >> > > The rest of the memory specific bits of the device-tree code use u64 for > addresses, and I kept it the same for consistency. Geert is right here. If it is a physical address, it should be phys_addr_t. Sebastian
On 09/12/2012 11:05 AM, Cyril Chemparathy wrote: > 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. S-o-B? > --- > arch/arm/mm/init.c | 2 +- > arch/c6x/kernel/devicetree.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 +-- > drivers/of/fdt.c | 10 ++++++---- > include/linux/of_fdt.h | 3 +-- > 9 files changed, 14 insertions(+), 19 deletions(-) > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > index ad722f1..579792c 100644 > --- a/arch/arm/mm/init.c > +++ b/arch/arm/mm/init.c > @@ -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/size need to change too. Not sure about similar things on other arches. Does u-boot need similar fixes? > { > phys_initrd_start = start; > phys_initrd_size = end - start; > diff --git a/arch/c6x/kernel/devicetree.c b/arch/c6x/kernel/devicetree.c > index bdb56f0..287d0e6 100644 > --- a/arch/c6x/kernel/devicetree.c > +++ b/arch/c6x/kernel/devicetree.c > @@ -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); > diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c > index 4a764cc..cecd42c 100644 > --- a/arch/microblaze/kernel/prom.c > +++ b/arch/microblaze/kernel/prom.c > @@ -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); > diff --git a/arch/mips/kernel/prom.c b/arch/mips/kernel/prom.c > index 028f6f8..e37e0dc 100644 > --- a/arch/mips/kernel/prom.c > +++ b/arch/mips/kernel/prom.c > @@ -41,8 +41,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); > diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c > index 5869e3f..150215a 100644 > --- a/arch/openrisc/kernel/prom.c > +++ b/arch/openrisc/kernel/prom.c > @@ -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); > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 37725e8..ac15f63 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -549,8 +549,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); > diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c > index b158152..2fbad6b 100644 > --- a/arch/x86/kernel/devicetree.c > +++ b/arch/x86/kernel/devicetree.c > @@ -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); > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index 91a375f..2ff8b7a 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -554,7 +554,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... "); > @@ -562,15 +563,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) > diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h > index ed136ad..4a17939 100644 > --- a/include/linux/of_fdt.h > +++ b/include/linux/of_fdt.h > @@ -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 */ >
On Wed, 12 Sep 2012, Rob Herring wrote: > On 09/12/2012 11:05 AM, Cyril Chemparathy wrote: > > 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. > > S-o-B? > > > --- > > arch/arm/mm/init.c | 2 +- > > arch/c6x/kernel/devicetree.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 +-- > > drivers/of/fdt.c | 10 ++++++---- > > include/linux/of_fdt.h | 3 +-- > > 9 files changed, 14 insertions(+), 19 deletions(-) > > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > > index ad722f1..579792c 100644 > > --- a/arch/arm/mm/init.c > > +++ b/arch/arm/mm/init.c > > @@ -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/size need to change too. Not sure about similar things > on other arches. size ? Nicolas
On 09/12/2012 03:31 PM, Nicolas Pitre wrote: > On Wed, 12 Sep 2012, Rob Herring wrote: > >> On 09/12/2012 11:05 AM, Cyril Chemparathy wrote: >>> 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. >> >> S-o-B? >> >>> --- >>> arch/arm/mm/init.c | 2 +- >>> arch/c6x/kernel/devicetree.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 +-- >>> drivers/of/fdt.c | 10 ++++++---- >>> include/linux/of_fdt.h | 3 +-- >>> 9 files changed, 14 insertions(+), 19 deletions(-) >>> >>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c >>> index ad722f1..579792c 100644 >>> --- a/arch/arm/mm/init.c >>> +++ b/arch/arm/mm/init.c >>> @@ -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/size need to change too. Not sure about similar things >> on other arches. > > size ? phys_initrd_size. Arguably, we'll never have a >4GB initrd with a PAE system or perhaps run into other issues first (like space to decompress it), but technically the DTS could specify one. Rob
On 09/12/2012 02:58 PM, Sebastian Andrzej Siewior wrote: > On 09/12/2012 08:02 PM, Cyril Chemparathy wrote: >>>> -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) >>> >>> Why not phys_addr_t? >>> >> >> The rest of the memory specific bits of the device-tree code use u64 for >> addresses, and I kept it the same for consistency. > > Geert is right here. If it is a physical address, it should be > phys_addr_t. While generally true, for the DT specific code I think it should be a fixed u64. The size of the address is defined by the FDT, not the kernel. It is very likely we could have a FDT that specifies addresses in 64-bit values, but then we boot a kernel is compiled for !LPAE. phys_addr_t is currently sized based on LPAE setting. Also, this is how the memory and reserved nodes are handled currently, so we should be consistent. Rob
On 9/12/2012 4:23 PM, Rob Herring wrote: > On 09/12/2012 11:05 AM, Cyril Chemparathy wrote: >> 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. > > S-o-B? > Sorry about that, will include in v2. [...] >> --- a/arch/arm/mm/init.c >> +++ b/arch/arm/mm/init.c >> @@ -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/size need to change too. Not sure about similar things > on other arches. > I've fixed phys_initrd_start (not size) in another patch, please see [1]. > Does u-boot need similar fixes? > We aren't there yet :-) We are currently running this platform without u-boot. [1] http://permalink.gmane.org/gmane.linux.kernel/1356713
On 09/13/2012 12:08 AM, Rob Herring wrote: >> Geert is right here. If it is a physical address, it should be >> phys_addr_t. > > While generally true, for the DT specific code I think it should be a > fixed u64. The size of the address is defined by the FDT, not the > kernel. It is very likely we could have a FDT that specifies addresses > in 64-bit values, but then we boot a kernel is compiled for !LPAE. > phys_addr_t is currently sized based on LPAE setting. If your kernel is 32bit without PAE and your DTB address is >32ibt than you can't handle it. If you don't notice this in your dt code than you remap the wrong memory ioremap(). > > Rob > Sebastian
On 09/13/2012 01:47 AM, Sebastian Andrzej Siewior wrote: > On 09/13/2012 12:08 AM, Rob Herring wrote: >>> Geert is right here. If it is a physical address, it should be >>> phys_addr_t. >> >> While generally true, for the DT specific code I think it should be a >> fixed u64. The size of the address is defined by the FDT, not the >> kernel. It is very likely we could have a FDT that specifies addresses >> in 64-bit values, but then we boot a kernel is compiled for !LPAE. >> phys_addr_t is currently sized based on LPAE setting. > > If your kernel is 32bit without PAE and your DTB address is >32ibt than > you can't handle it. If you don't notice this in your dt code than you > remap the wrong memory ioremap(). The size of the initrd fields are set by #address-cells properties and determined when you create the dtb. 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. Obviously, if you want to boot a non-PAE kernel, everything has to be placed at <4GB. I can boot i386 and i386-pae kernels on an i386-pae machines. I expect to generally be able to do that on ARM. Perhaps some SOCs like this one will not allow that, it is not always true. Rob
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index ad722f1..579792c 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -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; diff --git a/arch/c6x/kernel/devicetree.c b/arch/c6x/kernel/devicetree.c index bdb56f0..287d0e6 100644 --- a/arch/c6x/kernel/devicetree.c +++ b/arch/c6x/kernel/devicetree.c @@ -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); diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c index 4a764cc..cecd42c 100644 --- a/arch/microblaze/kernel/prom.c +++ b/arch/microblaze/kernel/prom.c @@ -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); diff --git a/arch/mips/kernel/prom.c b/arch/mips/kernel/prom.c index 028f6f8..e37e0dc 100644 --- a/arch/mips/kernel/prom.c +++ b/arch/mips/kernel/prom.c @@ -41,8 +41,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); diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c index 5869e3f..150215a 100644 --- a/arch/openrisc/kernel/prom.c +++ b/arch/openrisc/kernel/prom.c @@ -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); diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 37725e8..ac15f63 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -549,8 +549,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); diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c index b158152..2fbad6b 100644 --- a/arch/x86/kernel/devicetree.c +++ b/arch/x86/kernel/devicetree.c @@ -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); diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 91a375f..2ff8b7a 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -554,7 +554,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... "); @@ -562,15 +563,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) diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index ed136ad..4a17939 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -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 */