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

2016-10-12 Thread Heiner Kallweit 
Am 12.10.2016 um 06:26 schrieb Oliver O'Halloran:
> On Tue, Oct 11, 2016 at 7:06 AM, Heiner Kallweit  wrote:
>>> IMHO in case of using cuboot no CONFIG_KERNEL_ 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.
>>>
>>
>> This would be the patch implementing the idea. Advantage is that all
>> the unnecessary decompression code isn't built. Works fine for me.
> 
> I don't think this approach is viable. The wrapper code is shared
> among the various output image formats some of which *will* contain a
> compressed kernel image so we can't simply remove the decompressor
> from the wrapper. A random example I found in the makefile was
> CONFIG_BAMBOO:
> 
>> image-$(CONFIG_BAMBOO) += treeImage.bamboo cuImage.bamboo
> 
> When building for this platform Kbuild will produce treeboot and a
> cuboot image. Unlike uboot, Treeboot doesn't do any decompression so
> the wrapper needs to decompress the kernel itself. The probing
> solution more or less matches the old behaviour (which we know works)
> so I think we should just stick with that.
> 
> - Oliver
> 
Indeed, I also figured that out later. As you say, then let's stick
with re-introducing the probing. I'll send the patch for this.

Heiner

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

2016-10-11 Thread Oliver O'Halloran 
On Tue, Oct 11, 2016 at 7:06 AM, Heiner Kallweit  wrote:
>> IMHO in case of using cuboot no CONFIG_KERNEL_ 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.
>>
>
> This would be the patch implementing the idea. Advantage is that all
> the unnecessary decompression code isn't built. Works fine for me.

I don't think this approach is viable. The wrapper code is shared
among the various output image formats some of which *will* contain a
compressed kernel image so we can't simply remove the decompressor
from the wrapper. A random example I found in the makefile was
CONFIG_BAMBOO:

> image-$(CONFIG_BAMBOO) += treeImage.bamboo cuImage.bamboo

When building for this platform Kbuild will produce treeboot and a
cuboot image. Unlike uboot, Treeboot doesn't do any decompression so
the wrapper needs to decompress the kernel itself. The probing
solution more or less matches the old behaviour (which we know works)
so I think we should just stick with that.

- Oliver

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

2016-10-10 Thread Heiner Kallweit 
Am 10.10.2016 um 08:10 schrieb Heiner Kallweit:
> Am 10.10.2016 um 06:41 schrieb Michael Ellerman:
>> Heiner Kallweit  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_ 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.
> 

This would be the patch implementing the idea. Advantage is that all
the unnecessary decompression code isn't built. Works fine for me.

---
 arch/powerpc/boot/Makefile |  7 ++-
 arch/powerpc/boot/main.c   | 15 ---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index eae2dc8..1f18847 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -19,6 +19,7 @@
 
 all: $(obj)/zImage
 
+compress-y := CONFIG_KERNEL_UNCOMPRESSED
 compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP
 compress-$(CONFIG_KERNEL_XZ)   := CONFIG_KERNEL_XZ
 
@@ -95,12 +96,15 @@ libfdtheader := fdt.h libfdt.h libfdt_internal.h
 $(addprefix $(obj)/,$(libfdt) libfdt-wrapper.o simpleboot.o epapr.o opal.o): \
$(addprefix $(obj)/,$(libfdtheader))
 
-src-wlib-y := string.S crt0.S crtsavres.S stdio.c decompress.c main.c \
+src-wlib-y := string.S crt0.S crtsavres.S stdio.c main.c \
$(libfdt) libfdt-wrapper.c \
ns16550.c serial.c simple_alloc.c div64.S util.S \
elf_util.c $(zlib-y) devtree.c stdlib.c \
oflib.c ofconsole.c cuboot.c mpsc.c cpm-serial.c \
uartlite.c mpc52xx-psc.c opal.c opal-calls.S
+ifneq ($(compress-y),CONFIG_KERNEL_UNCOMPRESSED)
+src-wlib-y += decompress.c
+endif
 src-wlib-$(CONFIG_40x) += 4xx.c planetcore.c
 src-wlib-$(CONFIG_44x) += 4xx.c ebony.c bamboo.c
 src-wlib-$(CONFIG_8xx) += mpc8xx.c planetcore.c fsl-soc.c
@@ -226,6 +230,7 @@ CROSSWRAP := -C "$(CROSS_COMPILE)"
 endif
 endif
 
+compressor-y := none
 compressor-$(CONFIG_KERNEL_GZIP) := gz
 compressor-$(CONFIG_KERNEL_XZ)   := xz
 
diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index f7a184b..5a28c18 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -28,14 +28,19 @@ static struct addr_range prep_kernel(void)
 {
char elfheader[256];
unsigned char *vmlinuz_addr = (unsigned char *)_vmlinux_start;
-   unsigned long vmlinuz_size = _vmlinux_end - _vmlinux_start;
void *addr = 0;
struct elf_info ei;
+#ifndef CONFIG_KERNEL_UNCOMPRESSED
+   unsigned long vmlinuz_size = _vmlinux_end - _vmlinux_start;
long len;
+#endif
 
+#ifdef CONFIG_KERNEL_UNCOMPRESSED
+   memcpy(elfheader, vmlinuz_addr, sizeof(elfheader));
+#else
partial_decompress(vmlinuz_addr, vmlinuz_size,
elfheader, sizeof(elfheader), 0);
-
+#endif
if (!parse_elf64(elfheader, ) && !parse_elf32(elfheader, ))
fatal("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
 
@@ -67,6 +72,10 @@ static struct addr_range prep_kernel(void)
"device tree\n\r");
}
 
+#ifdef CONFIG_KERNEL_UNCOMPRESSED
+   memcpy(addr, vmlinuz_addr + ei.elfoffset,

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

2016-10-10 Thread Heiner Kallweit 
Am 10.10.2016 um 06:41 schrieb Michael Ellerman:
> Heiner Kallweit  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_ 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

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

2016-10-09 Thread Oliver O'Halloran 
On Mon, Oct 10, 2016 at 3:41 PM, Michael Ellerman  wrote:
> Heiner Kallweit  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

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

2016-10-09 Thread Michael Ellerman 
Heiner Kallweit  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

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

2016-10-08 Thread Heiner Kallweit 
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

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, ) && !parse_elf32(elfheader, ))
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};
-- 
2.10.0

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

2016-10-07 Thread Heiner Kallweit 
This commit results in boot stopping after the following messages.
Any additional information I could provide?


U-Boot 2010.12-svn19826 (Apr 24 2013 - 20:01:21)

CPU:   P1014, Version: 1.0, (0x80f10110)
Core:  E500, Version: 5.1, (0x80212151)
Clock Configuration:
   CPU0:800  MHz,
   CCB:400  MHz,
   DDR:333.333 MHz (666.667 MT/s data rate) (Asynchronous), IFC:100  MHz
L1:D-cache 32 kB enabled
   I-cache 32 kB enabled
Board: P1014RDB
SPI:   ready
DRAM:  128 MiB
L2:256 KB enabled
Using default environment

PCIe1: Root Complex of mini PCIe Slot, x1, regs @ 0xffe0a000
  01:00.0 - 168c:abcd - Network controller
PCIe1: Bus 00 - 01
PCIe2: Root Complex of PCIe Slot, x1, regs @ 0xffe09000
  03:00.0 - 168c:0033 - Network controller
PCIe2: Bus 02 - 03
In:serial
Out:   serial
Err:   serial
Net:   initialization for Atheros AR8327/AR8328
eTSEC1
auto update firmware: is_auto_upload_firmware = 0!
Autobooting in 1 seconds
SF: Detected S25FL128S_64K with page size 256, total 16 MiB
16384 KiB S25FL128S_64K at 0:0 is now current device
SPI flash read successful
SPI flash read successful
## Booting kernel from Legacy Image at 0200 ...
   Image Name:   Linux-4.9.0-rc0
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:2009549 Bytes = 1.9 MiB
   Load Address: 0100
   Entry Point:  01000524
   Verifying Checksum ... OK
## Flattened Device Tree blob at 0300
   Booting using the fdt blob at 0x300
   Uncompressing Kernel Image ... OK
   Loading Device Tree to 00ffa000, end 00ff ... OK

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

2016-10-06 Thread 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