| Message ID | 20110421104954.GH13688@atomide.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 21 Apr 2011, Tony Lindgren wrote: > Otherwise we end up overwriting ourselves. This fixes booting > on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db > (ARM: 6750/1: improvements to compressed/head.S). > > Signed-off-by: Tony Lindgren <tony@atomide.com> I don't understand why this is needed. The copy loop is explicitly copying from the end going backward exactly to cope with this possibility. Hmmm... Nicolas
On Thu, 21 Apr 2011, Nicolas Pitre wrote: > On Thu, 21 Apr 2011, Tony Lindgren wrote: > > > Otherwise we end up overwriting ourselves. This fixes booting > > on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db > > (ARM: 6750/1: improvements to compressed/head.S). > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > I don't understand why this is needed. The copy loop is explicitly > copying from the end going backward exactly to cope with this > possibility. I think your patch is 1) unneeded (see the copy loop code and the comment before it), and 2) simply hiding the real bug. I just need to modify the code in compressed/misc.c slightly for the lzma decompressor to start or stop working randomly. It seems that this code might be sensitive to slight displacement in memory caused by modifications to totally unrelated code. I'm still trying to track this down. Nicolas
* Nicolas Pitre <nicolas.pitre@linaro.org> [110421 16:18]: > On Thu, 21 Apr 2011, Tony Lindgren wrote: > > > Otherwise we end up overwriting ourselves. This fixes booting > > on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db > > (ARM: 6750/1: improvements to compressed/head.S). > > > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > I don't understand why this is needed. The copy loop is explicitly > copying from the end going backward exactly to cope with this > possibility. > > Hmmm... Yeah that's what I'm wondering too.. This is probably not the right fix.. I'm also wondering that it should be possible to make uImage also not work by setting loadaddr just before the uncompressed kernel end. You would assume that only the running code would not survive relocation if some of it gets overwritten. But that should be only the beginning, no idea why the need to relocate all the way after the whole image? If stack was overlapping the zImage, I could see it corrupt the zImage but there not much happening between relocating and restarting of the bootloader. Tony
--- a/arch/arm/boot/compressed/head.S +++ b/arch/arm/boot/compressed/head.S @@ -282,6 +282,7 @@ dtb_check_done: /* * Check to see if we will overwrite ourselves. + * r1 = corrupted, temporary uncompressed kernel end * r4 = final kernel address * r5 = start of this image * r9 = size of decompressed image @@ -292,15 +293,24 @@ dtb_check_done: */ cmp r4, r10 bhs wont_overwrite - add r10, r4, r9 - cmp r10, r5 + add r1, r4, r9 + cmp r1, r5 bls wont_overwrite + /* + * Check if the compressed image end is past the uncompressed + * kernel end. In that case, relocate ourselves to the end + * of the compressed image instead of the uncompressed kernel + * end to avoid overwriting ourselves. + */ + cmp r10, r1 + movls r10, r1 + /* * Relocate ourselves past the end of the decompressed kernel. * r5 = start of this image * r6 = _edata - * r10 = end of the decompressed kernel + * r10 = end of the decompressed kernel or end of this image if larger * Because we always copy ahead, we need to do it from the end and go * backward in case the source and destination overlap. */
Otherwise we end up overwriting ourselves. This fixes booting on n900 after commit 6d7d0ae51574943bf571d269da3243257a2d15db (ARM: 6750/1: improvements to compressed/head.S). Signed-off-by: Tony Lindgren <tony@atomide.com>