Patchwork [RFC] powerpc/boot: compare _start against ei.loadsize instead ei.memsize

login
register
mail settings
Submitter Sebastian Siewior
Date Sept. 23, 2008, 8:38 p.m.
Message ID <20080923203857.GB10935@www.tglx.de>
Download mbox | patch
Permalink /patch/1173/
State RFC
Headers show

Comments

Sebastian Siewior - Sept. 23, 2008, 8:38 p.m.
My mylinux binary incl. bss is ~5 MiB without bss less than 4 MiB.
Therefore I though that I could replace ei.memsize with ei.loadsize. It
didn't work. I'm not sure why it did not work but I guess that the
memset() of bss in the initial kernel code overwrote the cuimage code
which is required for some reason. Maybe some device-tree callbacks.

My current (working) solution is to move cuImage from 4 MiB to 8 MiB.
Something similar has been done for pSeries in 9b09c6d "powerpc: Change
the default link address for pSeries zImage kernels". Would it be
appropriate to move initial address to 64 MiB as the default loading
address or do we have here some boards which have only 64 MiB of memory?
Does someone have another idea?
Milton Miller - Sept. 24, 2008, 1:46 a.m.
On Wed Sep 24 at about 06:38:57 EST in 2008, Sebastian Siewior wrote:
> My mylinux binary incl. bss is ~5 MiB without bss less than 4 MiB.
> Therefore I though that I could replace ei.memsize with ei.loadsize. It
> didn't work. I'm not sure why it did not work but I guess that the
> memset() of bss in the initial kernel code overwrote the cuimage code
> which is required for some reason. Maybe some device-tree callbacks.

probably because the bss extended beyond the cuboot _end to include
where your device tree was copied (just a malloc and we start
simple_malloc at the boot _end on most platforms).

> 
> My current (working) solution is to move cuImage from 4 MiB to 8 MiB.
> Something similar has been done for pSeries in 9b09c6d "powerpc: Change
> the default link address for pSeries zImage kernels". Would it be
> appropriate to move initial address to 64 MiB as the default loading
> address or do we have here some boards which have only 64 MiB of memory?

I think there are some boards that have even less (8xx?).  Is the
link address a wrapper option yet?

> Does someone have another idea?
> 
> --- a/arch/powerpc/boot/main.c
> +++ b/arch/powerpc/boot/main.c
> @@ -56,7 +56,7 @@ static struct addr_range prep_kernel(void)
>  	if (platform_ops.vmlinux_alloc) {
>  		addr = platform_ops.vmlinux_alloc(ei.memsize);
>  	} else {
> -		if ((unsigned long)_start < ei.memsize)
> +		if ((unsigned long)_start < ei.loadsize)


I think this needs to be a two part test (add approprate casts):  
	_start < ei.loadsize || _end < ei.memsize

.. and a comment explaining why.

We could do better if we kept allocating more memory until we got
above it, but the current code has no such provision.  (prpmc2800, or
one of those, actually decompresses the header to peek at memsize
before starting the simple_alloc code).

>  			fatal("Insufficient memory for kernel at address 0!"
>  			       " (_start=%p)\n\r", _start);
>  	}
> -- 
> 
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -138,7 +138,7 @@ objflags=-S
>  tmp=$tmpdir/zImage.$$.o
>  ksection=.kernel:vmlinux.strip
>  isection=.kernel:initrd
> -link_address='0x400000'
> +link_address='0x800000'
>  
>  case "$platform" in
>  pseries)
> --
Sebastian Siewior - Sept. 25, 2008, 10:21 a.m.
Milton Miller wrote:
>> My current (working) solution is to move cuImage from 4 MiB to 8 MiB.
>> Something similar has been done for pSeries in 9b09c6d "powerpc: Change
>> the default link address for pSeries zImage kernels". Would it be
>> appropriate to move initial address to 64 MiB as the default loading
>> address or do we have here some boards which have only 64 MiB of memory?
> 
> I think there are some boards that have even less (8xx?).  Is the
> link address a wrapper option yet?

Nope. The wrapper script distinguishes between platforms. Default is 4MiB, 
pSeries has 64MiB, coff does 5MiB and the PS3 has other magic.

>> --- a/arch/powerpc/boot/main.c
>> +++ b/arch/powerpc/boot/main.c
>> @@ -56,7 +56,7 @@ static struct addr_range prep_kernel(void)
>>  	if (platform_ops.vmlinux_alloc) {
>>  		addr = platform_ops.vmlinux_alloc(ei.memsize);
>>  	} else {
>> -		if ((unsigned long)_start < ei.memsize)
>> +		if ((unsigned long)_start < ei.loadsize)
> 
> 
> I think this needs to be a two part test (add approprate casts):  
> 	_start < ei.loadsize || _end < ei.memsize
> 
> .. and a comment explaining why.

okay.

> We could do better if we kept allocating more memory until we got
> above it, but the current code has no such provision.  (prpmc2800, or
> one of those, actually decompresses the header to peek at memsize
> before starting the simple_alloc code).

I don't thing I can follow. Do you want to move the bootwrapper code above 
the limit and recall it?

Sebastian
Scott Wood - Sept. 25, 2008, 6:15 p.m.
On Tue, Sep 23, 2008 at 10:38:57PM +0200, Sebastian Siewior wrote:
> My current (working) solution is to move cuImage from 4 MiB to 8 MiB.
> Something similar has been done for pSeries in 9b09c6d "powerpc: Change
> the default link address for pSeries zImage kernels". Would it be
> appropriate to move initial address to 64 MiB as the default loading
> address or do we have here some boards which have only 64 MiB of memory?
> Does someone have another idea?

We have at least one supported board with only 8 MiB.  

Some options:
1. Dynamically relocate the wrapper to the top of RAM.
2. Relink based on the uncompressed size of the image bing wrapped.
3. Try to fall back on regular malloc() rather than fail if there's no
vmlinux_alloc() and there's no room at zero.

-Scott

Patch

--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -56,7 +56,7 @@  static struct addr_range prep_kernel(void)
 	if (platform_ops.vmlinux_alloc) {
 		addr = platform_ops.vmlinux_alloc(ei.memsize);
 	} else {
-		if ((unsigned long)_start < ei.memsize)
+		if ((unsigned long)_start < ei.loadsize)
 			fatal("Insufficient memory for kernel at address 0!"
 			       " (_start=%p)\n\r", _start);
 	}
-- 

--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -138,7 +138,7 @@  objflags=-S
 tmp=$tmpdir/zImage.$$.o
 ksection=.kernel:vmlinux.strip
 isection=.kernel:initrd
-link_address='0x400000'
+link_address='0x800000'
 
 case "$platform" in
 pseries)