diff mbox

Commit 1b7898ee276b "powerpc/boot: Use the pre-boot decompression API" breaks boot

Message ID 80c9c42c-45d3-9f24-b3e1-5a5042686b1b@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Heiner Kallweit Oct. 8, 2016, 10:13 p.m. UTC
Am 07.10.2016 um 21:26 schrieb Heiner Kallweit:
> Am 07.10.2016 um 07:51 schrieb Oliver O'Halloran:
>> Hi, Heiner
>>
>> Could you send me a copy of the kernel .config (or which defconfig)
>> that you're using, the name of the HW platform that you're using and
>> if possible the kernel image itself?
>>
>> Thanks,
>> Oliver
>>
> Thanks for the quick reply. Attached are .config and cuImage.
> HW is a TP-Link TL-WDR4900 WiFi router (P1014-based) running OpenWRT.
> 
> Rgds, Heiner
> 
After further checking I think I found the issue. The old gunzip code
handled uncompressed data transparently whilst the new one bails out
if it doesn't find a proper gzip header.
And in my case the actual kernel image is uncompressed.
With the following patch the system boots fine again (at least for me).

Rgds, Heiner

Comments

Michael Ellerman Oct. 10, 2016, 4:41 a.m. UTC | #1
Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 07.10.2016 um 21:26 schrieb Heiner Kallweit:
>> Am 07.10.2016 um 07:51 schrieb Oliver O'Halloran:
>>> Hi, Heiner
>>>
>>> Could you send me a copy of the kernel .config (or which defconfig)
>>> that you're using, the name of the HW platform that you're using and
>>> if possible the kernel image itself?
>>>
>>> Thanks,
>>> Oliver
>>>
>> Thanks for the quick reply. Attached are .config and cuImage.
>> HW is a TP-Link TL-WDR4900 WiFi router (P1014-based) running OpenWRT.
>> 
> After further checking I think I found the issue. The old gunzip code
> handled uncompressed data transparently whilst the new one bails out
> if it doesn't find a proper gzip header.
> And in my case the actual kernel image is uncompressed.
> With the following patch the system boots fine again (at least for me).

Thanks for testing and tracking it down.

I wonder why the actual image is uncompressed? Or alternately why do we
tell uboot the image is compressed when it's not?

cheers
Oliver O'Halloran Oct. 10, 2016, 5:33 a.m. UTC | #2
On Mon, Oct 10, 2016 at 3:41 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
>
>> Am 07.10.2016 um 21:26 schrieb Heiner Kallweit:
>>> Am 07.10.2016 um 07:51 schrieb Oliver O'Halloran:
>>>> Hi, Heiner
>>>>
>>>> Could you send me a copy of the kernel .config (or which defconfig)
>>>> that you're using, the name of the HW platform that you're using and
>>>> if possible the kernel image itself?
>>>>
>>>> Thanks,
>>>> Oliver
>>>>
>>> Thanks for the quick reply. Attached are .config and cuImage.
>>> HW is a TP-Link TL-WDR4900 WiFi router (P1014-based) running OpenWRT.
>>>
>> After further checking I think I found the issue. The old gunzip code
>> handled uncompressed data transparently whilst the new one bails out
>> if it doesn't find a proper gzip header.
>> And in my case the actual kernel image is uncompressed.
>> With the following patch the system boots fine again (at least for me).
>
> Thanks for testing and tracking it down.

Yeah thanks for that. I was putting off looking at it until Monday :)

>
> I wonder why the actual image is uncompressed? Or alternately why do we
> tell uboot the image is compressed when it's not?

The uboot payload (wrapper, kernel, initrd) as a whole is compressed
as a single blob. Modern uboot can just decompress the payload and
jump straight into the kernel and I'd assumed that all uboot platforms
did this. The problem is that the compatible uboot (cuboot) images do
use the wrapper and the vmlinux baked into the wrapper is
uncompressed.

Oliver
Heiner Kallweit Oct. 10, 2016, 6:10 a.m. UTC | #3
Am 10.10.2016 um 06:41 schrieb Michael Ellerman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Am 07.10.2016 um 21:26 schrieb Heiner Kallweit:
>>> Am 07.10.2016 um 07:51 schrieb Oliver O'Halloran:
>>>> Hi, Heiner
>>>>
>>>> Could you send me a copy of the kernel .config (or which defconfig)
>>>> that you're using, the name of the HW platform that you're using and
>>>> if possible the kernel image itself?
>>>>
>>>> Thanks,
>>>> Oliver
>>>>
>>> Thanks for the quick reply. Attached are .config and cuImage.
>>> HW is a TP-Link TL-WDR4900 WiFi router (P1014-based) running OpenWRT.
>>>
>> After further checking I think I found the issue. The old gunzip code
>> handled uncompressed data transparently whilst the new one bails out
>> if it doesn't find a proper gzip header.
>> And in my case the actual kernel image is uncompressed.
>> With the following patch the system boots fine again (at least for me).
> 
> Thanks for testing and tracking it down.
> 
> I wonder why the actual image is uncompressed? Or alternately why do we
> tell uboot the image is compressed when it's not?
> 
Uboot is provided with a compressed image, but what gets compressed is
not the pure kernel image but the resulting image incl. boot wrapper code,
see this part of the wrapper script:

cuboot*)
    gzip -n -f -9 "$ofile"
    ${MKIMAGE} -A ppc -O linux -T kernel -C gzip -a "$base" -e "$entry" \
            $uboot_version -d "$ofile".gz "$ofile"

And this resulting image is decompressed by uboot already during boot.
Therefore the boot wrapper code sees an uncompressed kernel image.

IMHO in case of using cuboot no CONFIG_KERNEL_<COMPR TYPE> config option
should be set and Makefile + code in arch/powerpc/boot should be able
to deal with this situation:
- don't copy and build the decompression stuff
- use an alternative version of prep_kernel() in main.c which doesn't
  attempt to decompress the kernel image

This should be a cleaner solution than probing the kernel image whether
it's compressed or not.

Rgds, Heiner
diff mbox

Patch

diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index f7a184b..9796491 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -32,9 +32,16 @@  static struct addr_range prep_kernel(void)
 	void *addr = 0;
 	struct elf_info ei;
 	long len;
+	int uncompressed_image = 0;
 
-	partial_decompress(vmlinuz_addr, vmlinuz_size,
+	len = partial_decompress(vmlinuz_addr, vmlinuz_size,
 		elfheader, sizeof(elfheader), 0);
+	/* assume uncompressed data if -1 is returned */
+	if (len == -1) {
+		uncompressed_image = 1;
+		memcpy(elfheader, vmlinuz_addr, sizeof(elfheader));
+		printf("No valid compressed data found, assume uncompressed data\n\r");
+	}
 
 	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei))
 		fatal("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
@@ -67,6 +74,12 @@  static struct addr_range prep_kernel(void)
 					"device tree\n\r");
 	}
 
+	if (uncompressed_image) {
+		memcpy(addr, vmlinuz_addr + ei.elfoffset, ei.loadsize);
+		printf("%ld bytes of uncompressed data copied\n\r", ei.loadsize);
+		goto out;
+	}
+
 	/* Finally, decompress the kernel */
 	printf("Decompressing (0x%p <- 0x%p:0x%p)...\n\r", addr,
 	       vmlinuz_addr, vmlinuz_addr+vmlinuz_size);
@@ -82,7 +95,7 @@  static struct addr_range prep_kernel(void)
 			 len, ei.loadsize);
 
 	printf("Done! Decompressed 0x%lx bytes\n\r", len);
-
+out:
 	flush_cache(addr, ei.loadsize);
 
 	return (struct addr_range){addr, ei.memsize};