[PATCH] PPC64: large INITRD causes kernel not to boot [UPDATE]

[PATCH] PPC64: large INITRD causes kernel not to boot [UPDATE]

[Mark Bellon]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

In PPC64 there are number of problems in arch/ppc64/boot/main.c that 
prevent a kernel from making use of a large (greater than ~16MB) INITRD. 
This is 64 bit architecture and really large INITRD images should be 
possible.

Simply put the existing code has a fixed reservation (claim) address and 
once the kernel plus initrd image are large enough to pass this address 
all sorts of bad things occur. The fix is the dynamically establish the 
first claim address above the loaded kernel plus initrd (plus some 
"padding" and rounding). If PROG_START is defined this will be used as 
the minimum safe address - currently known to be 0x01400000 for the 
firmwares tested so far.

We've talked about this in linuxppc64-dev@ozlabs.org and this is what 
seems to have settled out.

mark

Signed-off-by: Mark Bellon <mbellon@mvista.com>


[-- Attachment #2: initrd-patch --]
[-- Type: text/plain, Size: 2638 bytes --]

diff -Naur linux-2.6.13-orig/arch/ppc64/boot/main.c linux-2.6.13/arch/ppc64/boot/main.c
--- linux-2.6.13-orig/arch/ppc64/boot/main.c	2005-08-28 16:41:01.000000000 -0700
+++ linux-2.6.13/arch/ppc64/boot/main.c	2005-09-06 15:42:22.000000000 -0700
@@ -20,7 +20,7 @@
 extern void printf(const char *fmt, ...);
 extern int sprintf(char *buf, const char *fmt, ...);
 void gunzip(void *, int, unsigned char *, int *);
-void *claim(unsigned int, unsigned int, unsigned int);
+void *claim(unsigned long, unsigned long, unsigned long);
 void flush_cache(void *, unsigned long);
 void pause(void);
 extern void exit(void);
@@ -31,7 +31,8 @@
 
 /* Value picked to match that used by yaboot */
 #define PROG_START	0x01400000
-#define RAM_END		(256<<20) // Fixme: use OF */
+#define RAM_END		(512<<20) // Fixme: use OF */
+#define	ONE_MB		0x100000
 
 char *avail_ram;
 char *begin_avail, *end_avail;
@@ -40,6 +41,7 @@
 unsigned int heap_max;
 
 extern char _start[];
+extern char _end[];
 extern char _vmlinux_start[];
 extern char _vmlinux_end[];
 extern char _initrd_start[];
@@ -73,13 +75,13 @@
 
 #undef DEBUG
 
-static unsigned long claim_base = PROG_START;
+static unsigned long claim_base;
 
 static unsigned long try_claim(unsigned long size)
 {
 	unsigned long addr = 0;
 
-	for(; claim_base < RAM_END; claim_base += 0x100000) {
+	for(; claim_base < RAM_END; claim_base += ONE_MB) {
 #ifdef DEBUG
 		printf("    trying: 0x%08lx\n\r", claim_base);
 #endif
@@ -110,7 +112,26 @@
 	if (getprop(chosen_handle, "stdin", &stdin, sizeof(stdin)) != 4)
 		exit();
 
-	printf("\n\rzImage starting: loaded at 0x%x\n\r", (unsigned)_start);
+	printf("\n\rzImage starting: loaded at 0x%lx\n\r", (unsigned long) _start);
+
+	/*
+	 * The first available claim_base must be above the end of the
+	 * the loaded kernel wrapper file (_start to _end includes the
+	 * initrd image if it is present) and rounded up to a nice
+	 * 1 MB boundary for good measure.
+	 */
+
+	claim_base = _ALIGN_UP((unsigned long)_end, ONE_MB);
+
+#if defined(PROG_START)
+	/*
+	 * Maintain a "magic" minimum address. This keeps some older
+	 * firmware platforms running.
+	 */
+
+	if (claim_base < PROG_START)
+		claim_base = PROG_START;
+#endif
 
 	/*
 	 * Now we try to claim some memory for the kernel itself
@@ -120,7 +141,7 @@
 	 * size... In practice we add 1Mb, that is enough, but we should really
 	 * consider fixing the Makefile to put a _raw_ kernel in there !
 	 */
-	vmlinux_memsize += 0x100000;
+	vmlinux_memsize += ONE_MB;
 	printf("Allocating 0x%lx bytes for kernel ...\n\r", vmlinux_memsize);
 	vmlinux.addr = try_claim(vmlinux_memsize);
 	if (vmlinux.addr == 0) {

Re: [PATCH] PPC64: large INITRD causes kernel not to boot [UPDATE]

[Paul Mackerras]

Mark Bellon writes:

> Simply put the existing code has a fixed reservation (claim) address and 
> once the kernel plus initrd image are large enough to pass this address 
> all sorts of bad things occur. The fix is the dynamically establish the 
> first claim address above the loaded kernel plus initrd (plus some 
> "padding" and rounding). If PROG_START is defined this will be used as 
> the minimum safe address - currently known to be 0x01400000 for the 
> firmwares tested so far.

The idea is fine, but I have some questions about the actual patch:

> -void *claim(unsigned int, unsigned int, unsigned int);
> +void *claim(unsigned long, unsigned long, unsigned long);

What was the motivation for this change?  Since the zImage wrapper is
a 32-bit executable, int and long are both 32 bits.  I would prefer to
leave the parameters as unsigned int to force people to realize that
the parameters are 32 bits (even if said people have been working on
64-bit programs recently).

> +	claim_base = _ALIGN_UP((unsigned long)_end, ONE_MB);
> +
> +#if defined(PROG_START)
> +	/*
> +	 * Maintain a "magic" minimum address. This keeps some older
> +	 * firmware platforms running.
> +	 */
> +
> +	if (claim_base < PROG_START)
> +		claim_base = PROG_START;
> +#endif

This appears to be the meat of the patch, the rest is "cleanup",
right?

Paul.

Re: [PATCH] PPC64: large INITRD causes kernel not to boot [UPDATE]

[Mark Bellon]

Paul Mackerras wrote:

>Mark Bellon writes:
>
>  
>
>>Simply put the existing code has a fixed reservation (claim) address and 
>>once the kernel plus initrd image are large enough to pass this address 
>>all sorts of bad things occur. The fix is the dynamically establish the 
>>first claim address above the loaded kernel plus initrd (plus some 
>>"padding" and rounding). If PROG_START is defined this will be used as 
>>the minimum safe address - currently known to be 0x01400000 for the 
>>firmwares tested so far.
>>    
>>
>
>The idea is fine, but I have some questions about the actual patch:
>
>  
>
>>-void *claim(unsigned int, unsigned int, unsigned int);
>>+void *claim(unsigned long, unsigned long, unsigned long);
>>    
>>
>
>What was the motivation for this change?  Since the zImage wrapper is
>a 32-bit executable, int and long are both 32 bits.  I would prefer to
>leave the parameters as unsigned int to force people to realize that
>the parameters are 32 bits (even if said people have been working on
>64-bit programs recently).
>
>  
>
The function, claim, is found in prom.c uses longs. The long is the 
usual idiom for hiding a pointer, not an int, so I fixed accordingly. 
I'm open to further discussion of course.

On a 64 bit machine long and int are different sizes. This would make 
things "proper" if things changed in the future.

>>+	claim_base = _ALIGN_UP((unsigned long)_end, ONE_MB);
>>+
>>+#if defined(PROG_START)
>>+	/*
>>+	 * Maintain a "magic" minimum address. This keeps some older
>>+	 * firmware platforms running.
>>+	 */
>>+
>>+	if (claim_base < PROG_START)
>>+		claim_base = PROG_START;
>>+#endif
>>    
>>
>
>This appears to be the meat of the patch, the rest is "cleanup", right?
>  
>

Correct. The preceding comment explains what is going on. Removing the 
magic numbers seemed like a good idea.

mark

>Paul.
>  
>