bi_record and initrd

bi_record and initrd

[Magnus Damm]

Hi all,

The 2.4.19 kernel and the current rsync kernel from
source.mvista.com::linuxppc_2_4 does not work with large initrds.

The bi_record setup code in linux/arch/ppc/boot/simple/misc-embedded.c
doesn't care about the initrd size. So, if you are using a initrd that
crosses a megabyte-boundary you will have your initrd overwritten with
the bi_record. This results in crc error when unpacking the initrd.

The quick fix is to add a offset to the boot-code and to
arch/ppc/kernel/setup.c.

So, what's the long term solution?
Passing a pointer to the kernel?

/ magnus

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Tom Rini]

On Thu, Nov 14, 2002 at 04:54:47PM +0100, Magnus Damm wrote:

> The bi_record setup code in linux/arch/ppc/boot/simple/misc-embedded.c
> doesn't care about the initrd size. So, if you are using a initrd that
> crosses a megabyte-boundary you will have your initrd overwritten with
> the bi_record. This results in crc error when unpacking the initrd.
>
> The quick fix is to add a offset to the boot-code and to
> arch/ppc/kernel/setup.c.
>
> So, what's the long term solution?
> Passing a pointer to the kernel?

Can you give more information about where everything is loaded up at?  I
thought this was a non-issue, but it's been a while since I tested
initrds.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Magnus Damm]

> Can you give more information about where everything is loaded up at?  I
> thought this was a non-issue, but it's been a while since I tested
> initrds.

Sure.
The first example below does not trigger the bug.
The second one shows the bug.

First example:

loaded at:     00180000 0023C1D0
board data at: 00239124 00239140
relocated to:  001852FC 00185318
zimage at:     00185880 00214ADE
initrd at:     00215000 002388D9
avail ram:     0023D000 04000000

gunzip() unzips the zimage to address 0 and sets zimage_size to 0x00148A6C.
I'm not sure about the _ALIGN() macro and how it's used, but in this case
rec gets calculated to 0x00300000.
I thought the idea was to get a address that is aligned to the nearest
1-megabyte boundary.
(2 megabyte in this case - but that would overwrite the zimage...)

anyhow, the rec at 3 megabytes is inside of the available ram - good.

this mail says something about typos or misuse of _ALIGN().
http://www.geocrawler.com/archives/3/8358/2002/9/0/9715261/

If _ALIGN() now is used correctly, then the align definition
maybe chould be changed from
#define _ALIGN(addr,size)        (((addr)+size-1)&(~(size-1)))
to
#define _ALIGN(addr,size)        (((addr)+(size)-1)&(~((size)-1)))
to make sure that size is treated correctly. Or maybe it's a feature. =)

Second example:

loaded at:     FE2226F0 FE3098C0
relocated to:  00180000 002671D0
board data at: 00264124 00264140
relocated to:  001852E8 00185304
zimage at:     00185840 001CCEAB
initrd at:     001CD000 00263A21
avail ram:     00268000 04000000

I have not been able to output the value of zimage_size for this case, but I'm
sure that my initrd gets overwritten with the bi_record at 0x00200000.
We read out the initrd memory area with a BDM debugger and compared with the
original compressed filesystem image. So rec gets set to 2 megabytes.
This results in a initrd with bad crc. If it would help you I could get the value
of zimage_size, but it's on another machine and I don't have the time today.

Another thing - why is the second argument (dstlen) to gunzip() always 4 megabytes?
Maybe it could be set to the address that the image is loaded at / relocated to?
(0x180000 above) That way the gunzip function wouldn't overwrite the running code,
if I understand the dstlen argument correctly that is.

I'm using a 2.4.18 kernel patched up to 2.4.19-pre9 with more patches to match
linuxppc_2_4 (downloaded from ppckernel.org). So if you've made any changes
recently then I don't have them. But I compared my bi_record code with the
latest code available via rsync and they look identical.

thanks /

magnus

Tom Rini wrote:
>
> On Thu, Nov 14, 2002 at 04:54:47PM +0100, Magnus Damm wrote:
>
> > The bi_record setup code in linux/arch/ppc/boot/simple/misc-embedded.c
> > doesn't care about the initrd size. So, if you are using a initrd that
> > crosses a megabyte-boundary you will have your initrd overwritten with
> > the bi_record. This results in crc error when unpacking the initrd.
> >
> > The quick fix is to add a offset to the boot-code and to
> > arch/ppc/kernel/setup.c.
> >
> > So, what's the long term solution?
> > Passing a pointer to the kernel?
>
> Can you give more information about where everything is loaded up at?  I
> thought this was a non-issue, but it's been a while since I tested
> initrds.
>
> --
> Tom Rini (TR1265)
> http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Cort Dougan]

This would be a good opportunity to switch-over to allowing the bootloader
stick the initrd where it wants and just pass the physical pointer in via
bi_record in all cases.  It would certainly simplify these cases.

} > Can you give more information about where everything is loaded up at?  I
} > thought this was a non-issue, but it's been a while since I tested
} > initrds.
}
} Sure.
} The first example below does not trigger the bug.
} The second one shows the bug.
}
} First example:
}
} loaded at:     00180000 0023C1D0
} board data at: 00239124 00239140
} relocated to:  001852FC 00185318
} zimage at:     00185880 00214ADE
} initrd at:     00215000 002388D9
} avail ram:     0023D000 04000000
}
} gunzip() unzips the zimage to address 0 and sets zimage_size to 0x00148A6C.
} I'm not sure about the _ALIGN() macro and how it's used, but in this case
} rec gets calculated to 0x00300000.
} I thought the idea was to get a address that is aligned to the nearest
} 1-megabyte boundary.
} (2 megabyte in this case - but that would overwrite the zimage...)
}
} anyhow, the rec at 3 megabytes is inside of the available ram - good.
}
} this mail says something about typos or misuse of _ALIGN().
} http://www.geocrawler.com/archives/3/8358/2002/9/0/9715261/
}
} If _ALIGN() now is used correctly, then the align definition
} maybe chould be changed from
} #define _ALIGN(addr,size)        (((addr)+size-1)&(~(size-1)))
} to
} #define _ALIGN(addr,size)        (((addr)+(size)-1)&(~((size)-1)))
} to make sure that size is treated correctly. Or maybe it's a feature. =)
}
} Second example:
}
} loaded at:     FE2226F0 FE3098C0
} relocated to:  00180000 002671D0
} board data at: 00264124 00264140
} relocated to:  001852E8 00185304
} zimage at:     00185840 001CCEAB
} initrd at:     001CD000 00263A21
} avail ram:     00268000 04000000
}
} I have not been able to output the value of zimage_size for this case, but I'm
} sure that my initrd gets overwritten with the bi_record at 0x00200000.
} We read out the initrd memory area with a BDM debugger and compared with the
} original compressed filesystem image. So rec gets set to 2 megabytes.
} This results in a initrd with bad crc. If it would help you I could get the value
} of zimage_size, but it's on another machine and I don't have the time today.
}
} Another thing - why is the second argument (dstlen) to gunzip() always 4 megabytes?
} Maybe it could be set to the address that the image is loaded at / relocated to?
} (0x180000 above) That way the gunzip function wouldn't overwrite the running code,
} if I understand the dstlen argument correctly that is.
}
} I'm using a 2.4.18 kernel patched up to 2.4.19-pre9 with more patches to match
} linuxppc_2_4 (downloaded from ppckernel.org). So if you've made any changes
} recently then I don't have them. But I compared my bi_record code with the
} latest code available via rsync and they look identical.
}
} thanks /
}
} magnus
}
} Tom Rini wrote:
} >
} > On Thu, Nov 14, 2002 at 04:54:47PM +0100, Magnus Damm wrote:
} >
} > > The bi_record setup code in linux/arch/ppc/boot/simple/misc-embedded.c
} > > doesn't care about the initrd size. So, if you are using a initrd that
} > > crosses a megabyte-boundary you will have your initrd overwritten with
} > > the bi_record. This results in crc error when unpacking the initrd.
} > >
} > > The quick fix is to add a offset to the boot-code and to
} > > arch/ppc/kernel/setup.c.
} > >
} > > So, what's the long term solution?
} > > Passing a pointer to the kernel?
} >
} > Can you give more information about where everything is loaded up at?  I
} > thought this was a non-issue, but it's been a while since I tested
} > initrds.
} >
} > --
} > Tom Rini (TR1265)
} > http://gate.crashing.org/~trini/
}

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Magnus Damm]

> This would be a good opportunity to switch-over to allowing the bootloader
> stick the initrd where it wants and just pass the physical pointer in via
> bi_record in all cases.  It would certainly simplify these cases.

So, the following registers passed to the kernel could change from

 *   r3: ptr to board info data
 *   r4: initrd_start or if no initrd then 0
 *   r5: initrd_end - unused if r4 is 0
 *   r6: Start of command line string
 *   r7: End of command line string

to

 *   r3: ptr to bi_record

If the initrd is going to be passed with the bi_record, and the command line
already is there, then it's just the board info left. (for the 8xx anyhow)

Or maybe there are zillions of applications booting the kernel directly
without the bootloader..

/ magnus

Cort Dougan wrote:
>
> This would be a good opportunity to switch-over to allowing the bootloader
> stick the initrd where it wants and just pass the physical pointer in via
> bi_record in all cases.  It would certainly simplify these cases.
>
> } > Can you give more information about where everything is loaded up at?  I
> } > thought this was a non-issue, but it's been a while since I tested
> } > initrds.
> }
> } Sure.
> } The first example below does not trigger the bug.
> } The second one shows the bug.
> }
> } First example:
> }
> } loaded at:     00180000 0023C1D0
> } board data at: 00239124 00239140
> } relocated to:  001852FC 00185318
> } zimage at:     00185880 00214ADE
> } initrd at:     00215000 002388D9
> } avail ram:     0023D000 04000000
> }
> } gunzip() unzips the zimage to address 0 and sets zimage_size to 0x00148A6C.
> } I'm not sure about the _ALIGN() macro and how it's used, but in this case
> } rec gets calculated to 0x00300000.
> } I thought the idea was to get a address that is aligned to the nearest
> } 1-megabyte boundary.
> } (2 megabyte in this case - but that would overwrite the zimage...)
> }
> } anyhow, the rec at 3 megabytes is inside of the available ram - good.
> }
> } this mail says something about typos or misuse of _ALIGN().
> } http://www.geocrawler.com/archives/3/8358/2002/9/0/9715261/
> }
> } If _ALIGN() now is used correctly, then the align definition
> } maybe chould be changed from
> } #define _ALIGN(addr,size)        (((addr)+size-1)&(~(size-1)))
> } to
> } #define _ALIGN(addr,size)        (((addr)+(size)-1)&(~((size)-1)))
> } to make sure that size is treated correctly. Or maybe it's a feature. =)
> }
> } Second example:
> }
> } loaded at:     FE2226F0 FE3098C0
> } relocated to:  00180000 002671D0
> } board data at: 00264124 00264140
> } relocated to:  001852E8 00185304
> } zimage at:     00185840 001CCEAB
> } initrd at:     001CD000 00263A21
> } avail ram:     00268000 04000000
> }
> } I have not been able to output the value of zimage_size for this case, but I'm
> } sure that my initrd gets overwritten with the bi_record at 0x00200000.
> } We read out the initrd memory area with a BDM debugger and compared with the
> } original compressed filesystem image. So rec gets set to 2 megabytes.
> } This results in a initrd with bad crc. If it would help you I could get the value
> } of zimage_size, but it's on another machine and I don't have the time today.
> }
> } Another thing - why is the second argument (dstlen) to gunzip() always 4 megabytes?
> } Maybe it could be set to the address that the image is loaded at / relocated to?
> } (0x180000 above) That way the gunzip function wouldn't overwrite the running code,
> } if I understand the dstlen argument correctly that is.
> }
> } I'm using a 2.4.18 kernel patched up to 2.4.19-pre9 with more patches to match
> } linuxppc_2_4 (downloaded from ppckernel.org). So if you've made any changes
> } recently then I don't have them. But I compared my bi_record code with the
> } latest code available via rsync and they look identical.
> }
> } thanks /
> }
> } magnus
> }
> } Tom Rini wrote:
> } >
> } > On Thu, Nov 14, 2002 at 04:54:47PM +0100, Magnus Damm wrote:
> } >
> } > > The bi_record setup code in linux/arch/ppc/boot/simple/misc-embedded.c
> } > > doesn't care about the initrd size. So, if you are using a initrd that
> } > > crosses a megabyte-boundary you will have your initrd overwritten with
> } > > the bi_record. This results in crc error when unpacking the initrd.
> } > >
> } > > The quick fix is to add a offset to the boot-code and to
> } > > arch/ppc/kernel/setup.c.
> } > >
> } > > So, what's the long term solution?
> } > > Passing a pointer to the kernel?
> } >
> } > Can you give more information about where everything is loaded up at?  I
> } > thought this was a non-issue, but it's been a while since I tested
> } > initrds.
> } >
> } > --
> } > Tom Rini (TR1265)
> } > http://gate.crashing.org/~trini/
> }

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Tom Rini]

On Mon, Nov 18, 2002 at 09:19:24AM +0100, Magnus Damm wrote:
> > This would be a good opportunity to switch-over to allowing the bootloader
> > stick the initrd where it wants and just pass the physical pointer in via
> > bi_record in all cases.  It would certainly simplify these cases.
>
> So, the following registers passed to the kernel could change from
>
>  *   r3: ptr to board info data
>  *   r4: initrd_start or if no initrd then 0
>  *   r5: initrd_end - unused if r4 is 0
>  *   r6: Start of command line string
>  *   r7: End of command line string

In arch/ppc/boot, only r3 is used now, FWIW.

> to
>
>  *   r3: ptr to bi_record

Or so, yes.  Look at the archives for discussions / flamewars about
bi_records.

> If the initrd is going to be passed with the bi_record, and the command line
> already is there, then it's just the board info left. (for the 8xx anyhow)

The initrd already is, right now.  And yes, the bd_t would be passed, or
phased out, see the above mentioned discussions / flamewars.

> Or maybe there are zillions of applications booting the kernel directly
> without the bootloader..

Something _always_ loads the kernel someplace.  Call it what you will,
but the thing moving the kernel to 0 and pressing go is the bootloader.
Oh, and see the above mentioned discissions / flamewars :)

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Murray Jensen]

On Mon, 18 Nov 2002 09:19:24 +0100, Magnus Damm <damm@opensource.se> writes:
>Or maybe there are zillions of applications booting the kernel directly
>without the bootloader..

ppcboot for one ... and it only takes one application, if there are "zillions"
of people using it :-) Cheers!
								Murray...

PS: it doesn't really matter what the mechanism for passing info to the kernel
is, as long as it is an open standard - ppcboot will simply just conform to it.
But this will require effort on someone's part so changes shouldn't be done
lightly or gratuitously - they should be discussed in the list first and the
changes should "add value" (so to speak :-)
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Tom Rini]

On Fri, Nov 15, 2002 at 10:08:57AM -0700, Cort Dougan wrote:

> This would be a good opportunity to switch-over to allowing the bootloader
> stick the initrd where it wants and just pass the physical pointer in via
> bi_record in all cases.  It would certainly simplify these cases.

I'm not so sure.  In the case of arch/ppc/boot/ and not on OF, there
aren't many places we really know are 'good' that we can throw things.
I think what might help, and that we can do in 2.5 even, is allow the
bootloader to put the bi_recs wherever it wants to, pass that along in
r3 or r4 or so, and then in this case, we would align the bi_recs at
_ALIGN(0+zimage)+_ALIGN(initrd), and if that's > 8MB, make sure that we
don't allow the end of the initrd to be overwritten too.

And keep in mind it's Monday, and I haven't had any coffee yet, if
there's any horribly silly mistakes above :)

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Tom Rini]

On Fri, Nov 15, 2002 at 01:02:57PM +0100, Magnus Damm wrote:

> > Can you give more information about where everything is loaded up at?  I
> > thought this was a non-issue, but it's been a while since I tested
> > initrds.
>
> Sure.
[snip]
> this mail says something about typos or misuse of _ALIGN().
> http://www.geocrawler.com/archives/3/8358/2002/9/0/9715261/

I _think_ this is intentional, ie make sure we have bigger than needed
alignments, possibly to try and avoid what we've hit here.

> If _ALIGN() now is used correctly, then the align definition
> maybe chould be changed from
> #define _ALIGN(addr,size)        (((addr)+size-1)&(~(size-1)))
> to
> #define _ALIGN(addr,size)        (((addr)+(size)-1)&(~((size)-1)))
> to make sure that size is treated correctly. Or maybe it's a feature. =)

Fixing that now, thanks.

> Second example:
[snip]
> I have not been able to output the value of zimage_size for this case, but I'm
> sure that my initrd gets overwritten with the bi_record at 0x00200000.

Yeah, I'm sure too.  Here's an untested patch vs current
linuxppc_2_4_devel, which will relocate the initrd if needed.  The
current bi_rec code in the kernel shouldn't care if the initrd is moved
(This was broken in the past and then pointed out / fixed later, so I'm
rather confident of this part :)) so it should be OK.

For 2.5, I'm pondering going back and re-reading all of the discussions
and maybe even starting on it, with an arbitrary location for the
bi_recs which would let this case be much simpiler.

> Another thing - why is the second argument (dstlen) to gunzip() always 4 megabytes?
> Maybe it could be set to the address that the image is loaded at / relocated to?
> (0x180000 above) That way the gunzip function wouldn't overwrite the running code,
> if I understand the dstlen argument correctly that is.

I'm not sure about this part.  Tested patches are welcome of course. :)

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: bi_record and initrd

[Tom Rini]

On Tue, Nov 19, 2002 at 11:11:07AM -0700, Tom Rini wrote:

[snip]
> Yeah, I'm sure too.  Here's an untested patch vs current
> linuxppc_2_4_devel, which will relocate the initrd if needed.  The

And here it actually is.

===== arch/ppc/boot/common/misc-simple.c 1.24 vs edited =====
--- 1.24/arch/ppc/boot/common/misc-simple.c	Fri Oct 25 17:26:48 2002
+++ edited/arch/ppc/boot/common/misc-simple.c	Tue Nov 19 11:03:16 2002
@@ -82,6 +82,7 @@
 	char *cp, ch;
 	struct bi_record *rec, *birecs;
 	unsigned long TotalMemory = 0;
+	unsigned long initrd_loc;

 	serial_fixups();
 	com_port = serial_init(0, NULL);
@@ -145,6 +146,18 @@
 		puts("initrd at:     ");
 		puthex((unsigned long)(&__ramdisk_begin));
 		puts(" "); puthex((unsigned long)(&__ramdisk_end));puts("\n");
+		/* Make sure we don't put the bi_recs inside the initrd. */
+		if ((_ALIGN((unsigned long)(zimage_size) + (1 << 20) - 1,
+						(1 << 20))) <=
+				(unsigned long)(&__ramdisk_end)) {
+			initrd_loc = _ALIGN((unsigned long)(initrd_size) +
+					(2 << 20) - 1, (2 << 20));
+			memcpy(&initrd_loc, &__ramdisk_end, initrd_size);
+			puts("relocated to:  "); puthex(initrd_loc);
+			puts(" "); puthex(initrd_loc + initrd_size);
+			puts("\n");
+		} else
+			initrd_loc = (unsigned long)(&__ramdisk_begin);
 	}

 	avail_ram = (char *)0x00400000;
@@ -234,7 +247,7 @@

 	if ( initrd_size ) {
 		rec->tag = BI_INITRD;
-		rec->data[0] = (unsigned long)(&__ramdisk_begin);
+		rec->data[0] = initrd_loc;
 		rec->data[1] = initrd_size;
 		rec->size = sizeof(struct bi_record) + 2 *
 			sizeof(unsigned long);

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: bi_record and initrd

[Richard Laing]

The following is a diff context patch for misc-embedded.c, it fixes a
problem where the initrd ramdisk image is corrupted during the boot process.
It is based on the untested patch from Tom Rini for misc-simple.c. The patch
corrects a bug in the Tom's original patch. The patch is against the current
linuxppc_2_4_devel branch. The only change from the original patch the
memcpy line

memcpy((void *)initrd_loc, &__ramdisk_begin, initrd_size);

I have tested this patch with my build and it works fine...

Cheers
Richard


*** arch/ppc/boot/simple/old.c	2002-11-28 09:00:08.000000000 +1300
--- arch/ppc/boot/simple/misc-embedded.c	2002-11-28
09:50:21.000000000 +1300
***************
*** 83,88 ****
--- 83,89 ----
  	char *cp, ch;
  	int timer = 0, zimage_size;
  	unsigned long initrd_size;
+ 	unsigned long initrd_loc;

  	/* First, capture the embedded board information.  Then
  	 * initialize the serial console port.
***************
*** 145,150 ****
--- 146,162 ----
  		puts("initrd at:     ");
  		puthex((unsigned long)(&__ramdisk_begin));
  		puts(" "); puthex((unsigned
long)(&__ramdisk_end));puts("\n");
+
+ 		if ((_ALIGN((unsigned long)(zimage_size) + (1 << 20) - 1,
+ 			(1 << 20))) <= (unsigned long)(&__ramdisk_end)) {
+ 			initrd_loc = _ALIGN((unsigned long)(initrd_size) +(2
<< 20) - 1, (2 << 20));
+ 			 memcpy((void *)initrd_loc, &__ramdisk_begin,
initrd_size);
+ 			 puts("relocated to:  "); puthex(initrd_loc);
+ 			 puts(" "); puthex(initrd_loc + initrd_size);
+ 			 puts("\n");
+ 		} else {
+ 			initrd_loc = (unsigned long)(&__ramdisk_begin);
+ 	        }
  	}

  	/*
***************
*** 225,231 ****

  		if ( initrd_size ) {
  			rec->tag = BI_INITRD;
! 			rec->data[0] = (unsigned long)(&__ramdisk_begin);
  			rec->data[1] = initrd_size;
  			rec->size = sizeof(struct bi_record) + 2 *
  				sizeof(unsigned long);
--- 237,243 ----

  		if ( initrd_size ) {
  			rec->tag = BI_INITRD;
! 			rec->data[0] = initrd_loc;
  			rec->data[1] = initrd_size;
  			rec->size = sizeof(struct bi_record) + 2 *
  				sizeof(unsigned long);


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/