Fix initrd length miscalculation in bootm command

The do_bootm_linux() function was using the same variable ('len') to calculate the the dtu length and the initrd length, which meant that the initrd length was incorrect. This patch creates renames 'len' and 'data' to 'initrd_len' and 'initrd_data', thereby preventing any future confusion. It also deletes 'len' and 'data' because the dtu calculations don't actually need them.

Quoted 5 lines · In message <[email protected]> you wrote:

The do_bootm_linux() function was using the same variable ('len') to calculate the the dtu length and the initrd length, which meant that the initrd length was incorrect. This patch creates renames 'len' and 'data' to 'initrd_len' and 'initrd_data', thereby preventing any future confusion. It also deletes 'len' and 'data' because the dtu calculations don't actually need them.

I can't parse this. 'len' was used twice, but actually it wasn't used in the second case, but you renamed the only remaining use it had?

Quoted 13 lines · Wolfgang Denk

In message <[email protected]> you

Quoted 10 lines · wrote:

The do_bootm_linux() function was using the same variable ('len') to calculate
the the dtu length and the initrd length, which meant that the initrd length
was incorrect. This patch creates renames 'len' and 'data' to 'initrd_len'
and 'initrd_data', thereby preventing any future confusion. It also deletes
'len' and 'data' because the dtu calculations don't actually need them.

Quoted 2 lines

I can't parse this. 'len' was used twice, but actually it wasn't used in the second case, but you renamed the only remaining use it had?

Sorry, it looks like I have a few typos in my description. Here's a new one:

"The do_bootm_linux() function was using the same variable ('len') to calculate
the dtu length and the initrd length. This meant that the initrd length was
incorrect when it came time to book the kernel. This resulted in the inability

to boot with an initrd on an OF-based kernel. This patch renames the local
variables 'len' and 'data' to 'initrd_len' and 'initrd_data', respectively, and
it uses initrd_len and initrd_data only for initrd calculations.

In addition, there were a few places where 'len' and 'data' where being used as
temporary variables. The code was modified to eliminate the need for temporary
variables."

Does this explain everything? Do you want me to resubmit the patch with the new
changelog text?

Quoted 2 lines

incorrect when it came time to book the kernel. This resulted in the inability

book -> boot

Did you test this with a multi image as well? (kernel + initrd + dtb)

Quoted 3 lines · Kumar Gala

Quoted 2 lines

incorrect when it came time to book the kernel. This resulted in the inability

book -> boot

Ugh.

Quoted 1 line

Did you test this with a multi image as well? (kernel + initrd + dtb)

I've never heard of multi image before. How do I make one? The README doesn't explain it.

mkimage -A ppc -O Linux -T multi -C gzip -n 'Linux Multiboot-Image' -e 0 -a 0 -d vmlinux.gz:rootfs.ext2.gz:oftree.dtb kern+fs+dtb.bin

Quoted 2 lines

I've never heard of multi image before. How do I make one? The README doesn't explain it.

mkimage -A ppc -O Linux -T multi -C gzip -n 'Linux Multiboot-Image' -e 0 -a 0 -d vmlinux.gz:rootfs.ext2.gz:oftree.dtb kern+fs+dtb.bin

Quoted 5 lines · Kumar Gala

Quoted 2 lines

I've never heard of multi image before. How do I make one? The README doesn't explain it.

mkimage -A ppc -O Linux -T multi -C gzip -n 'Linux Multiboot- Image' -e 0 -a 0 -d vmlinux.gz:rootfs.ext2.gz:oftree.dtb kern+fs +dtb.bin

I need some more help getting the multi-image to work. I have the following files:

vmlinux.bin.gz (built myself)
rootfs.ext2.gz.uboot (from the BSP, because I don't know how to build a root file system. I don't know what the ".uboot" is for.) mpc8349emitx.dtb (built myself)

I create the multi image per you instructions, tftp them to $loadaddr (200000), and do "bootm $loadaddr". I get this output:

bootm $loadaddr
## Booting image at 00200000 ...

   Image Name:   Linux Multiboot-Image
   Created:      2007-02-19  23:46:03 UTC
   Image Type:   PowerPC Linux Multi-File Image (gzip compressed)
   Data Size:    20407814 Bytes = 19.5 MB
   Load Address: 00000000
   Entry Point:  00000000
   Contents:
   Image 0:  1416615 Bytes =  1.4 MB
   Image 1: 18987128 Bytes = 18.1 MB
   Image 2:     4054 Bytes =  4 kB
   Verifying Checksum ... OK
   Uncompressing Multi-File Image ... OK

U-Boot 1.2.0-g2e892316-dirty (Feb 9 2007 - 15:56:37) MPC83XX

So the image appears to be okay, but it doesn't boot at all. Is there something obvious I'm doing wrong, or will I have to debug this?

Sounds right, I'd double check that your rootfs.ext2.gz.uboot isn't already a uImage, but the actual ext2 image gziped.

Also, I was wondering if 200000 is too low of a load address. Doesn't the kernel lock that memory or something?

Quoted 2 lines · In message <[email protected]> you wrote:

Also, I was wondering if 200000 is too low of a load address. Doesn't the kernel lock that memory or something?

If you hve a big kernel, that's definitely too low.

Quoted 2 lines · In message <[email protected]> you wrote:

Also, I was wondering if 200000 is too low of a load address. Doesn't the kernel lock that memory or something?

If you hve a big kernel, that's definitely too low.

I can confirm that multi-images work both with and without my patch. So if there are no objections, please apply this patch. Thanks.

Quoted 2 lines · In message <[email protected]> you wrote:

I can confirm that multi-images work both with and without my patch. So if there are no objections, please apply this patch. Thanks.

Since it doesn't fix a problem I see little reason to change the code.

Quoted 2 lines · In message <[email protected]> you wrote:

I can confirm that multi-images work both with and without my patch. So if there are no objections, please apply this patch. Thanks.

Since it doesn't fix a problem I see little reason to change the code.

Quoted 2 lines · Wolfgang Denk

Quoted 1 line · In message <[email protected]> you wrote:

I can confirm that multi-images work both with and without my patch. So if

Quoted 4 lines

Quoted 1 line

there are no objections, please apply this patch. Thanks.

Since it doesn't fix a problem I see little reason to change the code.

Wolfgang, I didn't get a response from you about my last email on this thread,

so I just want to make sure that you understand that my patch, "Fix initrd length miscalculation in bootm command," does fix a real bug in do_bootm_linux().

Here's the changelog for the patch:

I remember the changelog, but your statement that "multi-images work both with and without" your patch. Confuses me. If it works without your patch, then what's the problem?

Now that I think about it, I guess that statement doesn't make any sense. I was trying to say that I didn't *break* multi-images. The patch fixes a problem with initrd + OF. If you're not using an initrd, then my patch doesn't change anything, so I just wanted to make sure that things that worked before my patch still work after my patch.

Quoted 4 lines · Timur Tabi

Now that I think about it, I guess that statement doesn't make any sense. I was trying to say that I didn't *break* multi-images. The patch fixes a problem with initrd + OF. If you're not using an initrd, then my patch doesn't change anything, so I just wanted to make sure that things that worked before my patch still work after my patch.

Wolfgang,

I just wanted to make sure that you understood that this patch does indeed fix a real and severe problem with U-Boot, and therefore should be applied. I submitted this patch over a month ago, and it's still not in your repository. Without this patch, you cannot use an
initrd on any ARCH=powerpc kernel.

Quoted 9 lines · Timur Tabi

Quoted 4 lines · Timur Tabi

Now that I think about it, I guess that statement doesn't make any sense. I was trying to say that I didn't *break* multi-images. The patch fixes a problem with initrd + OF. If you're not using an initrd, then my patch doesn't change anything, so I just wanted to make sure that things that worked before my patch still work after my patch.

Wolfgang,

Quoted 4 lines

I just wanted to make sure that you understood that this patch does indeed fix a real and severe problem with U-Boot, and therefore should be applied. I submitted this patch over a month ago, and it's still not in your repository. Without this patch, you cannot use an
initrd on any ARCH=powerpc kernel.

Just want to confirm that the booting of "arch/powerpc" kernels with an initrd is currently broken in u-boot 1.2.0. And, that Timur's patch fixes the issue.

One more note to add to that. The only part of Timur's patch that is really important is this bit:

(quoted diff) +1 -2

@@ -771,9 +767,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int fl
 
                       checksum = ntohl(hdr->ih_dcrc);
                       addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
-                       len = ntohl(hdr->ih_size);
 
-                       if(checksum != crc32(0, (uchar *)addr, len)) {
+                       if(checksum != crc32(0, (uchar *)addr,
 ntohl(hdr->ih_size))) {
                               printf("ERROR: Flat Device Tree
 checksum is invalid\n");

                               return;
                       }

The "len" variable should not be used for the DTB image.