All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
@ 2019-08-01  8:06 Heiko Schocher
  2019-08-01 12:33 ` Simon Goldschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Heiko Schocher @ 2019-08-01  8:06 UTC (permalink / raw)
  To: u-boot

Hello Julius,

since your commit (just gone into mainline):

b1307f884a91 ("fit: Support compression for non-kernel components (e.g. FDT)")

I see on an imx6ull based board when booting (FIT image with kernel, dtb
and initramdisk), fit image source created from yocto, see [1]:

## Loading kernel from FIT Image at 84000000 ...
    Using 'conf at imx6ull-k30rf.dtb' configuration
    Trying 'kernel at 1' kernel subimage
      Description:  Linux kernel
      Type:         Kernel Image
      Compression:  uncompressed
      Data Start:   0x840000e8
      Data Size:    5154568 Bytes = 4.9 MiB
      Architecture: ARM
      OS:           Linux
      Load Address: 0x82000000
      Entry Point:  0x82000000
      Hash algo:    sha1
      Hash value:   a3ba8a09b197c0d1acab834c69c5aea677d4a092
    Verifying Hash Integrity ... sha1+ OK
## Loading ramdisk from FIT Image at 84000000 ...
    Using 'conf at imx6ull-k30rf.dtb' configuration
    Trying 'ramdisk at 1' ramdisk subimage
      Description:  k30rf-image-initramfs
      Type:         RAMDisk Image
      Compression:  gzip compressed
      Data Start:   0x844f0cf8
      Data Size:    6769496 Bytes = 6.5 MiB
      Architecture: ARM
      OS:           Linux
      Load Address: unavailable
      Entry Point:  unavailable
      Hash algo:    sha1
      Hash value:   507d8b20b18f9bca0e9178c48f634df5da720f65
    Verifying Hash Integrity ... sha1+ OK
    Uncompressing RAMDisk Image
Error: Bad gzipped data
Error decompressing ramdisk
Ramdisk image is corrupt or invalid
=>

I found your commit with "git bisect":

hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect good
b1307f884a913f52a201491053b6d221c4204f60 is the first bad commit
commit b1307f884a913f52a201491053b6d221c4204f60
Author: Julius Werner <jwerner@chromium.org>
Date:   Wed Jul 24 19:37:55 2019 -0700

     fit: Support compression for non-kernel components (e.g. FDT)

     This patch adds support for compressing non-kernel image nodes in a FIT
     image (kernel nodes could already be compressed previously). This can
     reduce the size of FIT images and therefore improve boot times
     (especially when an image bundles many different kernel FDTs). The
     images will automatically be decompressed on load.

     This patch does not support extracting compatible strings from
     compressed FDTs, so it's not very helpful in conjunction with
     CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
     that select the configuration to load explicitly.

     Signed-off-by: Julius Werner <jwerner@chromium.org>
     Reviewed-by: Simon Glass <sjg@chromium.org>
     Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

:040000 040000 6280a41241c1c25a28845fa526c5c958e448e347 e3f8ed43853541f868676f077924e7ab242d4da6 M 
    common
:040000 040000 406b0c50c03633bad9d4a576d5662f4fa483fc6f 601624cee600c1c06640858519d93188cc312544 M 
    test
hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect log
git bisect start
# bad: [bbaf56eda0e63d6d28fbccae0f112ef88203eb4d] Merge tag 'u-boot-amlogic-20190731' of 
https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
git bisect bad bbaf56eda0e63d6d28fbccae0f112ef88203eb4d
# good: [a9aa4c5700c68c070d63a391b51ea8d341b6e8a6] Merge branch '2019-07-24-master-imports'
git bisect good a9aa4c5700c68c070d63a391b51ea8d341b6e8a6
# good: [75551c8bfc9545e31ec2ce238cac3857904007b8] Merge branch '2019-07-26-ti-imports'
git bisect good 75551c8bfc9545e31ec2ce238cac3857904007b8
# bad: [333755ef7b6f824366eed37ae068c20a4f25a123] Merge branch '2019-07-29-ti-imports'
git bisect bad 333755ef7b6f824366eed37ae068c20a4f25a123
# good: [26008cd42b590dc71ee9c1ca667a218542aab342] rockchip: rv1108: Migrate to use common board file
git bisect good 26008cd42b590dc71ee9c1ca667a218542aab342
# good: [92430b8fc8aac3b4ab92e9ca8a09d83c4788c609] Merge 
https://gitlab.denx.de/u-boot/custodians/u-boot-socfpga
git bisect good 92430b8fc8aac3b4ab92e9ca8a09d83c4788c609
# bad: [2d64a0f7e952f54375702fb2b854461e402ded9d] Merge branch '2019-07-29-master-imports'
git bisect bad 2d64a0f7e952f54375702fb2b854461e402ded9d
# bad: [49b10cb4926285b856b207c1f5bb40c75487f08b] gpio: fixes for gpio-hog support
git bisect bad 49b10cb4926285b856b207c1f5bb40c75487f08b
# bad: [e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d] tools/logos: remove black background of U-Boot logo
git bisect bad e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d
# bad: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for non-kernel components 
(e.g. FDT)
git bisect bad b1307f884a913f52a201491053b6d221c4204f60
# good: [2090854cd2f228bab546f2718ccdbe1664830d3c] common: Move bootm_decomp_image() to image.c (as 
image_decomp())
git bisect good 2090854cd2f228bab546f2718ccdbe1664830d3c
# first bad commit: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for 
non-kernel components (e.g. FDT)
hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $

base was current master:
*   bbaf56eda0 - Merge tag 'u-boot-amlogic-20190731' of 
https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic


I am currently not in my office to dig deeper into it ... may you have an idea?

Thanks!

bye,
Heiko

[1] fit image source (created from yocto)
/dts-v1/;

/ {
         description = "U-Boot fitImage for Poky (Yocto Project Reference Distro)/4.19.59/k30rf";
         #address-cells = <1>;

         images {
[...]
                 ramdisk at 1 {
                         description = "k30rf-image-initramfs";
                         data = 
/incbin/("/work/hs/tbot2go/k30rf/repo/k30rf/build_k30rf/tmp/deploy/images/k30rf/k30rf-image-initramfs-k30rf.cpio.gz");
                         type = "ramdisk";
                         arch = "arm";
                         os = "linux";
                         compression = "gzip";


-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01  8:06 [U-Boot] fit image: boot gzipped ramdisk does not work anymore Heiko Schocher
@ 2019-08-01 12:33 ` Simon Goldschmidt
  2019-08-01 14:38   ` Heiko Schocher
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Goldschmidt @ 2019-08-01 12:33 UTC (permalink / raw)
  To: u-boot

Am 01.08.2019 um 10:06 schrieb Heiko Schocher:
> Hello Julius,
> 
> since your commit (just gone into mainline):
> 
> b1307f884a91 ("fit: Support compression for non-kernel components (e.g. FDT)")
> 
> I see on an imx6ull based board when booting (FIT image with kernel, dtb
> and initramdisk), fit image source created from yocto, see [1]:
> 
> ## Loading kernel from FIT Image at 84000000 ...
>      Using 'conf at imx6ull-k30rf.dtb' configuration
>      Trying 'kernel at 1' kernel subimage
>        Description:  Linux kernel
>        Type:         Kernel Image
>        Compression:  uncompressed
>        Data Start:   0x840000e8
>        Data Size:    5154568 Bytes = 4.9 MiB
>        Architecture: ARM
>        OS:           Linux
>        Load Address: 0x82000000
>        Entry Point:  0x82000000
>        Hash algo:    sha1
>        Hash value:   a3ba8a09b197c0d1acab834c69c5aea677d4a092
>      Verifying Hash Integrity ... sha1+ OK
> ## Loading ramdisk from FIT Image at 84000000 ...
>      Using 'conf at imx6ull-k30rf.dtb' configuration
>      Trying 'ramdisk at 1' ramdisk subimage
>        Description:  k30rf-image-initramfs
>        Type:         RAMDisk Image
>        Compression:  gzip compressed
>        Data Start:   0x844f0cf8
>        Data Size:    6769496 Bytes = 6.5 MiB
>        Architecture: ARM
>        OS:           Linux
>        Load Address: unavailable
>        Entry Point:  unavailable
>        Hash algo:    sha1
>        Hash value:   507d8b20b18f9bca0e9178c48f634df5da720f65
>      Verifying Hash Integrity ... sha1+ OK
>      Uncompressing RAMDisk Image
> Error: Bad gzipped data
> Error decompressing ramdisk
> Ramdisk image is corrupt or invalid
> =>
> 
> I found your commit with "git bisect":
> 
> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect good
> b1307f884a913f52a201491053b6d221c4204f60 is the first bad commit
> commit b1307f884a913f52a201491053b6d221c4204f60
> Author: Julius Werner <jwerner@chromium.org>
> Date:   Wed Jul 24 19:37:55 2019 -0700
> 
>       fit: Support compression for non-kernel components (e.g. FDT)
> 
>       This patch adds support for compressing non-kernel image nodes in a FIT
>       image (kernel nodes could already be compressed previously). This can
>       reduce the size of FIT images and therefore improve boot times
>       (especially when an image bundles many different kernel FDTs). The
>       images will automatically be decompressed on load.
> 
>       This patch does not support extracting compatible strings from
>       compressed FDTs, so it's not very helpful in conjunction with
>       CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
>       that select the configuration to load explicitly.
> 
>       Signed-off-by: Julius Werner <jwerner@chromium.org>
>       Reviewed-by: Simon Glass <sjg@chromium.org>
>       Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> 
> :040000 040000 6280a41241c1c25a28845fa526c5c958e448e347 e3f8ed43853541f868676f077924e7ab242d4da6 M
>      common
> :040000 040000 406b0c50c03633bad9d4a576d5662f4fa483fc6f 601624cee600c1c06640858519d93188cc312544 M
>      test
> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect log
> git bisect start
> # bad: [bbaf56eda0e63d6d28fbccae0f112ef88203eb4d] Merge tag 'u-boot-amlogic-20190731' of
> https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
> git bisect bad bbaf56eda0e63d6d28fbccae0f112ef88203eb4d
> # good: [a9aa4c5700c68c070d63a391b51ea8d341b6e8a6] Merge branch '2019-07-24-master-imports'
> git bisect good a9aa4c5700c68c070d63a391b51ea8d341b6e8a6
> # good: [75551c8bfc9545e31ec2ce238cac3857904007b8] Merge branch '2019-07-26-ti-imports'
> git bisect good 75551c8bfc9545e31ec2ce238cac3857904007b8
> # bad: [333755ef7b6f824366eed37ae068c20a4f25a123] Merge branch '2019-07-29-ti-imports'
> git bisect bad 333755ef7b6f824366eed37ae068c20a4f25a123
> # good: [26008cd42b590dc71ee9c1ca667a218542aab342] rockchip: rv1108: Migrate to use common board file
> git bisect good 26008cd42b590dc71ee9c1ca667a218542aab342
> # good: [92430b8fc8aac3b4ab92e9ca8a09d83c4788c609] Merge
> https://gitlab.denx.de/u-boot/custodians/u-boot-socfpga
> git bisect good 92430b8fc8aac3b4ab92e9ca8a09d83c4788c609
> # bad: [2d64a0f7e952f54375702fb2b854461e402ded9d] Merge branch '2019-07-29-master-imports'
> git bisect bad 2d64a0f7e952f54375702fb2b854461e402ded9d
> # bad: [49b10cb4926285b856b207c1f5bb40c75487f08b] gpio: fixes for gpio-hog support
> git bisect bad 49b10cb4926285b856b207c1f5bb40c75487f08b
> # bad: [e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d] tools/logos: remove black background of U-Boot logo
> git bisect bad e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d
> # bad: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for non-kernel components
> (e.g. FDT)
> git bisect bad b1307f884a913f52a201491053b6d221c4204f60
> # good: [2090854cd2f228bab546f2718ccdbe1664830d3c] common: Move bootm_decomp_image() to image.c (as
> image_decomp())
> git bisect good 2090854cd2f228bab546f2718ccdbe1664830d3c
> # first bad commit: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for
> non-kernel components (e.g. FDT)
> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $
> 
> base was current master:
> *   bbaf56eda0 - Merge tag 'u-boot-amlogic-20190731' of
> https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
> 
> 
> I am currently not in my office to dig deeper into it ... may you have an idea?
> 
> Thanks!
> 
> bye,
> Heiko
> 
> [1] fit image source (created from yocto)
> /dts-v1/;
> 
> / {
>           description = "U-Boot fitImage for Poky (Yocto Project Reference Distro)/4.19.59/k30rf";
>           #address-cells = <1>;
> 
>           images {
> [...]
>                   ramdisk at 1 {
>                           description = "k30rf-image-initramfs";
>                           data =
> /incbin/("/work/hs/tbot2go/k30rf/repo/k30rf/build_k30rf/tmp/deploy/images/k30rf/k30rf-image-initramfs-k30rf.cpio.gz");
>                           type = "ramdisk";
>                           arch = "arm";
>                           os = "linux";
>                           compression = "gzip";

Back last year, when I started what Julius succeeded with this pach, we 
came to the conclusion that this "compression = " tag in the FIT image 
wasn't about the actual content but about what U-Boot should do with the 
content. I.e. in your case it means "U-Boot, please uncompress this!", 
while the old code just did not care about compression and left 
uncompression to the Kernel.

To retain this behaviour, it should be enough to delete the "compression 
= " line from your its. That way, U-Boot doesn't uncompress the ramdisk 
and the Kernel should do it like always.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 12:33 ` Simon Goldschmidt
@ 2019-08-01 14:38   ` Heiko Schocher
  2019-08-01 15:58     ` Tom Rini
  0 siblings, 1 reply; 14+ messages in thread
From: Heiko Schocher @ 2019-08-01 14:38 UTC (permalink / raw)
  To: u-boot

Hello Simon,

Am 01.08.2019 um 14:33 schrieb Simon Goldschmidt:
> Am 01.08.2019 um 10:06 schrieb Heiko Schocher:
>> Hello Julius,
>>
>> since your commit (just gone into mainline):
>>
>> b1307f884a91 ("fit: Support compression for non-kernel components (e.g. FDT)")
>>
>> I see on an imx6ull based board when booting (FIT image with kernel, dtb
>> and initramdisk), fit image source created from yocto, see [1]:
>>
>> ## Loading kernel from FIT Image at 84000000 ...
>>      Using 'conf at imx6ull-k30rf.dtb' configuration
>>      Trying 'kernel at 1' kernel subimage
>>        Description:  Linux kernel
>>        Type:         Kernel Image
>>        Compression:  uncompressed
>>        Data Start:   0x840000e8
>>        Data Size:    5154568 Bytes = 4.9 MiB
>>        Architecture: ARM
>>        OS:           Linux
>>        Load Address: 0x82000000
>>        Entry Point:  0x82000000
>>        Hash algo:    sha1
>>        Hash value:   a3ba8a09b197c0d1acab834c69c5aea677d4a092
>>      Verifying Hash Integrity ... sha1+ OK
>> ## Loading ramdisk from FIT Image at 84000000 ...
>>      Using 'conf at imx6ull-k30rf.dtb' configuration
>>      Trying 'ramdisk at 1' ramdisk subimage
>>        Description:  k30rf-image-initramfs
>>        Type:         RAMDisk Image
>>        Compression:  gzip compressed
>>        Data Start:   0x844f0cf8
>>        Data Size:    6769496 Bytes = 6.5 MiB
>>        Architecture: ARM
>>        OS:           Linux
>>        Load Address: unavailable
>>        Entry Point:  unavailable
>>        Hash algo:    sha1
>>        Hash value:   507d8b20b18f9bca0e9178c48f634df5da720f65
>>      Verifying Hash Integrity ... sha1+ OK
>>      Uncompressing RAMDisk Image
>> Error: Bad gzipped data
>> Error decompressing ramdisk
>> Ramdisk image is corrupt or invalid
>> =>
>>
>> I found your commit with "git bisect":
>>
>> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect good
>> b1307f884a913f52a201491053b6d221c4204f60 is the first bad commit
>> commit b1307f884a913f52a201491053b6d221c4204f60
>> Author: Julius Werner <jwerner@chromium.org>
>> Date:   Wed Jul 24 19:37:55 2019 -0700
>>
>>       fit: Support compression for non-kernel components (e.g. FDT)
>>
>>       This patch adds support for compressing non-kernel image nodes in a FIT
>>       image (kernel nodes could already be compressed previously). This can
>>       reduce the size of FIT images and therefore improve boot times
>>       (especially when an image bundles many different kernel FDTs). The
>>       images will automatically be decompressed on load.
>>
>>       This patch does not support extracting compatible strings from
>>       compressed FDTs, so it's not very helpful in conjunction with
>>       CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
>>       that select the configuration to load explicitly.
>>
>>       Signed-off-by: Julius Werner <jwerner@chromium.org>
>>       Reviewed-by: Simon Glass <sjg@chromium.org>
>>       Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>
>> :040000 040000 6280a41241c1c25a28845fa526c5c958e448e347 e3f8ed43853541f868676f077924e7ab242d4da6 M
>>      common
>> :040000 040000 406b0c50c03633bad9d4a576d5662f4fa483fc6f 601624cee600c1c06640858519d93188cc312544 M
>>      test
>> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect log
>> git bisect start
>> # bad: [bbaf56eda0e63d6d28fbccae0f112ef88203eb4d] Merge tag 'u-boot-amlogic-20190731' of
>> https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
>> git bisect bad bbaf56eda0e63d6d28fbccae0f112ef88203eb4d
>> # good: [a9aa4c5700c68c070d63a391b51ea8d341b6e8a6] Merge branch '2019-07-24-master-imports'
>> git bisect good a9aa4c5700c68c070d63a391b51ea8d341b6e8a6
>> # good: [75551c8bfc9545e31ec2ce238cac3857904007b8] Merge branch '2019-07-26-ti-imports'
>> git bisect good 75551c8bfc9545e31ec2ce238cac3857904007b8
>> # bad: [333755ef7b6f824366eed37ae068c20a4f25a123] Merge branch '2019-07-29-ti-imports'
>> git bisect bad 333755ef7b6f824366eed37ae068c20a4f25a123
>> # good: [26008cd42b590dc71ee9c1ca667a218542aab342] rockchip: rv1108: Migrate to use common board file
>> git bisect good 26008cd42b590dc71ee9c1ca667a218542aab342
>> # good: [92430b8fc8aac3b4ab92e9ca8a09d83c4788c609] Merge
>> https://gitlab.denx.de/u-boot/custodians/u-boot-socfpga
>> git bisect good 92430b8fc8aac3b4ab92e9ca8a09d83c4788c609
>> # bad: [2d64a0f7e952f54375702fb2b854461e402ded9d] Merge branch '2019-07-29-master-imports'
>> git bisect bad 2d64a0f7e952f54375702fb2b854461e402ded9d
>> # bad: [49b10cb4926285b856b207c1f5bb40c75487f08b] gpio: fixes for gpio-hog support
>> git bisect bad 49b10cb4926285b856b207c1f5bb40c75487f08b
>> # bad: [e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d] tools/logos: remove black background of U-Boot logo
>> git bisect bad e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d
>> # bad: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for non-kernel components
>> (e.g. FDT)
>> git bisect bad b1307f884a913f52a201491053b6d221c4204f60
>> # good: [2090854cd2f228bab546f2718ccdbe1664830d3c] common: Move bootm_decomp_image() to image.c (as
>> image_decomp())
>> git bisect good 2090854cd2f228bab546f2718ccdbe1664830d3c
>> # first bad commit: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for
>> non-kernel components (e.g. FDT)
>> hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $
>>
>> base was current master:
>> *   bbaf56eda0 - Merge tag 'u-boot-amlogic-20190731' of
>> https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
>>
>>
>> I am currently not in my office to dig deeper into it ... may you have an idea?
>>
>> Thanks!
>>
>> bye,
>> Heiko
>>
>> [1] fit image source (created from yocto)
>> /dts-v1/;
>>
>> / {
>>           description = "U-Boot fitImage for Poky (Yocto Project Reference Distro)/4.19.59/k30rf";
>>           #address-cells = <1>;
>>
>>           images {
>> [...]
>>                   ramdisk at 1 {
>>                           description = "k30rf-image-initramfs";
>>                           data =
>> /incbin/("/work/hs/tbot2go/k30rf/repo/k30rf/build_k30rf/tmp/deploy/images/k30rf/k30rf-image-initramfs-k30rf.cpio.gz"); 
>>
>>                           type = "ramdisk";
>>                           arch = "arm";
>>                           os = "linux";
>>                           compression = "gzip";
> 
> Back last year, when I started what Julius succeeded with this pach, we came to the conclusion that 
> this "compression = " tag in the FIT image wasn't about the actual content but about what U-Boot 
> should do with the content. I.e. in your case it means "U-Boot, please uncompress this!", while the 
> old code just did not care about compression and left uncompression to the Kernel.

Understand.

> To retain this behaviour, it should be enough to delete the "compression = " line from your its. 
> That way, U-Boot doesn't uncompress the ramdisk and the Kernel should do it like always.

Ah ... okay ... Hmm.. the its file gets created from yocto recipe:

https://git.yoctoproject.org/cgit.cgi/poky/plain/meta/classes/kernel-fitimage.bbclass

I never checked :-(

So this commit breaks now yocto builded FIT images (for boards which use
FIT image with an compressed ramdisk)

Added Marek, as he introduced "Add basic fitImage support" in yocto.

Hmm... thinking while writting ... this breaks also boards which only
do an U-Boot update, and want to boot "old" FIT images from a storage
device... Do we really want this?

Your arguments are correct, and the commit also, but we perhaps break
some boards, so U-Boot should be able to handle the old (wrong) way?

The "compression = " line in the generated its file is added in
kernel-fitimage.class from commit:

commit 4aa6644f92975ded908ef99cf313466e0845e071
Author: Rick Altherr <raltherr@google.com>
Date:   Fri Jan 20 11:28:53 2017 -0800

     kernel-fitimage: Use compressed ramdisks in FIT images if available

     kernel-fitimage:fitimage_assemble() was calling copy_initramfs from
     kernel.bbclass which decompresses the initramfs cpio.  Assume that if
     INITRAMFS_FSTYPES includes a compressed cpio, that is what it desired in
     the FIT image.

     (From OE-Core rev: 842ad404b36e00c89f615a3f7db4a2d30062effa)

     Signed-off-by: Rick Altherr <raltherr@google.com>
     Signed-off-by: Ross Burton <ross.burton@intel.com>
     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

So more than 2 years in, so for sure boards out, which we break now ...

Added Rick, Ross and Richard to cc too, may they have an idea, what we
should do.

I think we should do:

- U-Boot Kconfig option: to set U-Boot into old (wrong) way
   or at least, ignore compressed line in ramdisk section. Last idea
   seems the best option for me, may we do this always?

- correct yocto class, to not set "compressed =" line in its file.

Ideas?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 14:38   ` Heiko Schocher
@ 2019-08-01 15:58     ` Tom Rini
  2019-08-01 18:24       ` Julius Werner
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Rini @ 2019-08-01 15:58 UTC (permalink / raw)
  To: u-boot

On Thu, Aug 01, 2019 at 04:38:51PM +0200, Heiko Schocher wrote:
> Hello Simon,
> 
> Am 01.08.2019 um 14:33 schrieb Simon Goldschmidt:
> >Am 01.08.2019 um 10:06 schrieb Heiko Schocher:
> >>Hello Julius,
> >>
> >>since your commit (just gone into mainline):
> >>
> >>b1307f884a91 ("fit: Support compression for non-kernel components (e.g. FDT)")
> >>
> >>I see on an imx6ull based board when booting (FIT image with kernel, dtb
> >>and initramdisk), fit image source created from yocto, see [1]:
> >>
> >>## Loading kernel from FIT Image at 84000000 ...
> >>     Using 'conf at imx6ull-k30rf.dtb' configuration
> >>     Trying 'kernel at 1' kernel subimage
> >>       Description:  Linux kernel
> >>       Type:         Kernel Image
> >>       Compression:  uncompressed
> >>       Data Start:   0x840000e8
> >>       Data Size:    5154568 Bytes = 4.9 MiB
> >>       Architecture: ARM
> >>       OS:           Linux
> >>       Load Address: 0x82000000
> >>       Entry Point:  0x82000000
> >>       Hash algo:    sha1
> >>       Hash value:   a3ba8a09b197c0d1acab834c69c5aea677d4a092
> >>     Verifying Hash Integrity ... sha1+ OK
> >>## Loading ramdisk from FIT Image at 84000000 ...
> >>     Using 'conf at imx6ull-k30rf.dtb' configuration
> >>     Trying 'ramdisk at 1' ramdisk subimage
> >>       Description:  k30rf-image-initramfs
> >>       Type:         RAMDisk Image
> >>       Compression:  gzip compressed
> >>       Data Start:   0x844f0cf8
> >>       Data Size:    6769496 Bytes = 6.5 MiB
> >>       Architecture: ARM
> >>       OS:           Linux
> >>       Load Address: unavailable
> >>       Entry Point:  unavailable
> >>       Hash algo:    sha1
> >>       Hash value:   507d8b20b18f9bca0e9178c48f634df5da720f65
> >>     Verifying Hash Integrity ... sha1+ OK
> >>     Uncompressing RAMDisk Image
> >>Error: Bad gzipped data
> >>Error decompressing ramdisk
> >>Ramdisk image is corrupt or invalid
> >>=>
> >>
> >>I found your commit with "git bisect":
> >>
> >>hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect good
> >>b1307f884a913f52a201491053b6d221c4204f60 is the first bad commit
> >>commit b1307f884a913f52a201491053b6d221c4204f60
> >>Author: Julius Werner <jwerner@chromium.org>
> >>Date:   Wed Jul 24 19:37:55 2019 -0700
> >>
> >>      fit: Support compression for non-kernel components (e.g. FDT)
> >>
> >>      This patch adds support for compressing non-kernel image nodes in a FIT
> >>      image (kernel nodes could already be compressed previously). This can
> >>      reduce the size of FIT images and therefore improve boot times
> >>      (especially when an image bundles many different kernel FDTs). The
> >>      images will automatically be decompressed on load.
> >>
> >>      This patch does not support extracting compatible strings from
> >>      compressed FDTs, so it's not very helpful in conjunction with
> >>      CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
> >>      that select the configuration to load explicitly.
> >>
> >>      Signed-off-by: Julius Werner <jwerner@chromium.org>
> >>      Reviewed-by: Simon Glass <sjg@chromium.org>
> >>      Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>
> >>:040000 040000 6280a41241c1c25a28845fa526c5c958e448e347 e3f8ed43853541f868676f077924e7ab242d4da6 M
> >>     common
> >>:040000 040000 406b0c50c03633bad9d4a576d5662f4fa483fc6f 601624cee600c1c06640858519d93188cc312544 M
> >>     test
> >>hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $ git bisect log
> >>git bisect start
> >># bad: [bbaf56eda0e63d6d28fbccae0f112ef88203eb4d] Merge tag 'u-boot-amlogic-20190731' of
> >>https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
> >>git bisect bad bbaf56eda0e63d6d28fbccae0f112ef88203eb4d
> >># good: [a9aa4c5700c68c070d63a391b51ea8d341b6e8a6] Merge branch '2019-07-24-master-imports'
> >>git bisect good a9aa4c5700c68c070d63a391b51ea8d341b6e8a6
> >># good: [75551c8bfc9545e31ec2ce238cac3857904007b8] Merge branch '2019-07-26-ti-imports'
> >>git bisect good 75551c8bfc9545e31ec2ce238cac3857904007b8
> >># bad: [333755ef7b6f824366eed37ae068c20a4f25a123] Merge branch '2019-07-29-ti-imports'
> >>git bisect bad 333755ef7b6f824366eed37ae068c20a4f25a123
> >># good: [26008cd42b590dc71ee9c1ca667a218542aab342] rockchip: rv1108: Migrate to use common board file
> >>git bisect good 26008cd42b590dc71ee9c1ca667a218542aab342
> >># good: [92430b8fc8aac3b4ab92e9ca8a09d83c4788c609] Merge
> >>https://gitlab.denx.de/u-boot/custodians/u-boot-socfpga
> >>git bisect good 92430b8fc8aac3b4ab92e9ca8a09d83c4788c609
> >># bad: [2d64a0f7e952f54375702fb2b854461e402ded9d] Merge branch '2019-07-29-master-imports'
> >>git bisect bad 2d64a0f7e952f54375702fb2b854461e402ded9d
> >># bad: [49b10cb4926285b856b207c1f5bb40c75487f08b] gpio: fixes for gpio-hog support
> >>git bisect bad 49b10cb4926285b856b207c1f5bb40c75487f08b
> >># bad: [e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d] tools/logos: remove black background of U-Boot logo
> >>git bisect bad e35d533c79c75e9ad68a2e78da52c8ce58e6ce0d
> >># bad: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for non-kernel components
> >>(e.g. FDT)
> >>git bisect bad b1307f884a913f52a201491053b6d221c4204f60
> >># good: [2090854cd2f228bab546f2718ccdbe1664830d3c] common: Move bootm_decomp_image() to image.c (as
> >>image_decomp())
> >>git bisect good 2090854cd2f228bab546f2718ccdbe1664830d3c
> >># first bad commit: [b1307f884a913f52a201491053b6d221c4204f60] fit: Support compression for
> >>non-kernel components (e.g. FDT)
> >>hs at xmglap:u-boot  [(kein Branch, binäre Suche begonnen bei master)] $
> >>
> >>base was current master:
> >>*   bbaf56eda0 - Merge tag 'u-boot-amlogic-20190731' of
> >>https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic
> >>
> >>
> >>I am currently not in my office to dig deeper into it ... may you have an idea?
> >>
> >>Thanks!
> >>
> >>bye,
> >>Heiko
> >>
> >>[1] fit image source (created from yocto)
> >>/dts-v1/;
> >>
> >>/ {
> >>          description = "U-Boot fitImage for Poky (Yocto Project Reference Distro)/4.19.59/k30rf";
> >>          #address-cells = <1>;
> >>
> >>          images {
> >>[...]
> >>                  ramdisk at 1 {
> >>                          description = "k30rf-image-initramfs";
> >>                          data =
> >>/incbin/("/work/hs/tbot2go/k30rf/repo/k30rf/build_k30rf/tmp/deploy/images/k30rf/k30rf-image-initramfs-k30rf.cpio.gz");
> >>
> >>                          type = "ramdisk";
> >>                          arch = "arm";
> >>                          os = "linux";
> >>                          compression = "gzip";
> >
> >Back last year, when I started what Julius succeeded with this pach, we
> >came to the conclusion that this "compression = " tag in the FIT image
> >wasn't about the actual content but about what U-Boot should do with the
> >content. I.e. in your case it means "U-Boot, please uncompress this!",
> >while the old code just did not care about compression and left
> >uncompression to the Kernel.
> 
> Understand.
> 
> >To retain this behaviour, it should be enough to delete the "compression =
> >" line from your its. That way, U-Boot doesn't uncompress the ramdisk and
> >the Kernel should do it like always.
> 
> Ah ... okay ... Hmm.. the its file gets created from yocto recipe:
> 
> https://git.yoctoproject.org/cgit.cgi/poky/plain/meta/classes/kernel-fitimage.bbclass
> 
> I never checked :-(
> 
> So this commit breaks now yocto builded FIT images (for boards which use
> FIT image with an compressed ramdisk)
> 
> Added Marek, as he introduced "Add basic fitImage support" in yocto.
> 
> Hmm... thinking while writting ... this breaks also boards which only
> do an U-Boot update, and want to boot "old" FIT images from a storage
> device... Do we really want this?
> 
> Your arguments are correct, and the commit also, but we perhaps break
> some boards, so U-Boot should be able to handle the old (wrong) way?
> 
> The "compression = " line in the generated its file is added in
> kernel-fitimage.class from commit:
> 
> commit 4aa6644f92975ded908ef99cf313466e0845e071
> Author: Rick Altherr <raltherr@google.com>
> Date:   Fri Jan 20 11:28:53 2017 -0800
> 
>     kernel-fitimage: Use compressed ramdisks in FIT images if available
> 
>     kernel-fitimage:fitimage_assemble() was calling copy_initramfs from
>     kernel.bbclass which decompresses the initramfs cpio.  Assume that if
>     INITRAMFS_FSTYPES includes a compressed cpio, that is what it desired in
>     the FIT image.
> 
>     (From OE-Core rev: 842ad404b36e00c89f615a3f7db4a2d30062effa)
> 
>     Signed-off-by: Rick Altherr <raltherr@google.com>
>     Signed-off-by: Ross Burton <ross.burton@intel.com>
>     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> So more than 2 years in, so for sure boards out, which we break now ...
> 
> Added Rick, Ross and Richard to cc too, may they have an idea, what we
> should do.
> 
> I think we should do:
> 
> - U-Boot Kconfig option: to set U-Boot into old (wrong) way
>   or at least, ignore compressed line in ramdisk section. Last idea
>   seems the best option for me, may we do this always?
> 
> - correct yocto class, to not set "compressed =" line in its file.

First, we have a problem as we need to support what's out in the world.

Second, what is intended with what's out in the world?  Indeed before we
didn't do anything with the compressed data.  But now what, we're saying
it's compressed, but not providing enough details to U-Boot to
uncompress it?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190801/a9d451ad/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 15:58     ` Tom Rini
@ 2019-08-01 18:24       ` Julius Werner
  2019-08-01 18:34         ` Simon Goldschmidt
  2019-08-02  4:08         ` Heiko Schocher
  0 siblings, 2 replies; 14+ messages in thread
From: Julius Werner @ 2019-08-01 18:24 UTC (permalink / raw)
  To: u-boot

> First, we have a problem as we need to support what's out in the world.

How about I write a patch to change the decompression call in
fit_image_load() to fall back to a memcpy() if the decompression fails
(and print a big warning that the FIT image is wrong and should be
updated)? That should keep those old installations working.

> Second, what is intended with what's out in the world?  Indeed before we
> didn't do anything with the compressed data.  But now what, we're saying
> it's compressed, but not providing enough details to U-Boot to
> uncompress it?

Hmm... I'm not sure why it wouldn't just decompress correctly,
actually. Is there anything special with how gzipped data is formatted
for the kernel decompressor? Or is there any limitation in the U-Boot
gzip implementation?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 18:24       ` Julius Werner
@ 2019-08-01 18:34         ` Simon Goldschmidt
  2019-08-01 19:01           ` Julius Werner
  2019-08-02  4:08         ` Heiko Schocher
  1 sibling, 1 reply; 14+ messages in thread
From: Simon Goldschmidt @ 2019-08-01 18:34 UTC (permalink / raw)
  To: u-boot

Am 01.08.2019 um 20:24 schrieb Julius Werner:
>> First, we have a problem as we need to support what's out in the world.
> 
> How about I write a patch to change the decompression call in
> fit_image_load() to fall back to a memcpy() if the decompression fails
> (and print a big warning that the FIT image is wrong and should be
> updated)? That should keep those old installations working.

Well, I'm sure you'd find enough FIT images where the uncompression 
might succeed but the target space for decompression is not big enough 
(see bootm_size), would such an error be fixed as well?

Regards,
Simon

> 
>> Second, what is intended with what's out in the world?  Indeed before we
>> didn't do anything with the compressed data.  But now what, we're saying
>> it's compressed, but not providing enough details to U-Boot to
>> uncompress it?
> 
> Hmm... I'm not sure why it wouldn't just decompress correctly,
> actually. Is there anything special with how gzipped data is formatted
> for the kernel decompressor? Or is there any limitation in the U-Boot
> gzip implementation?
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 18:34         ` Simon Goldschmidt
@ 2019-08-01 19:01           ` Julius Werner
  2019-08-01 19:06             ` Simon Goldschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Julius Werner @ 2019-08-01 19:01 UTC (permalink / raw)
  To: u-boot

> Well, I'm sure you'd find enough FIT images where the uncompression
> might succeed but the target space for decompression is not big enough
> (see bootm_size), would such an error be fixed as well?

I think so? Assuming all the compression algorithms check for an
output buffer overrun (looks like they do), that should also just lead
to a compression error and go into the fallback path.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 19:01           ` Julius Werner
@ 2019-08-01 19:06             ` Simon Goldschmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Goldschmidt @ 2019-08-01 19:06 UTC (permalink / raw)
  To: u-boot

Am 01.08.2019 um 21:01 schrieb Julius Werner:
>> Well, I'm sure you'd find enough FIT images where the uncompression
>> might succeed but the target space for decompression is not big enough
>> (see bootm_size), would such an error be fixed as well?
> 
> I think so? Assuming all the compression algorithms check for an
> output buffer overrun (looks like they do), that should also just lead
> to a compression error and go into the fallback path.

Then I think that would be the best way to start and see if it's good 
enough for backwards compatibility.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-01 18:24       ` Julius Werner
  2019-08-01 18:34         ` Simon Goldschmidt
@ 2019-08-02  4:08         ` Heiko Schocher
  2019-08-02  6:00           ` Simon Goldschmidt
  1 sibling, 1 reply; 14+ messages in thread
From: Heiko Schocher @ 2019-08-02  4:08 UTC (permalink / raw)
  To: u-boot

Hello Julius,

Am 01.08.2019 um 20:24 schrieb Julius Werner:
>> First, we have a problem as we need to support what's out in the world.
> 
> How about I write a patch to change the decompression call in
> fit_image_load() to fall back to a memcpy() if the decompression fails
> (and print a big warning that the FIT image is wrong and should be
> updated)? That should keep those old installations working.

Hmm.. what happens if decompression does not fail ?
(I wonder, why I get this Error, but I go into vacation, so I have
  no chance to dig deeper into it soon ...)

And why should we decompress, if we "know" we do not have to (in ramdisk
case)? This cost only boottime ...

For the fast track, I prefer to ignore the compression property in ramdisk
node ... (may configurable through a Kconfig option with default to ignore)

>> Second, what is intended with what's out in the world?  Indeed before we
>> didn't do anything with the compressed data.  But now what, we're saying
>> it's compressed, but not providing enough details to U-Boot to
>> uncompress it?
> 
> Hmm... I'm not sure why it wouldn't just decompress correctly,
> actually. Is there anything special with how gzipped data is formatted
> for the kernel decompressor? Or is there any limitation in the U-Boot
> gzip implementation?
> 

I do not know.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-02  4:08         ` Heiko Schocher
@ 2019-08-02  6:00           ` Simon Goldschmidt
  2019-08-02 12:36             ` Tom Rini
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Goldschmidt @ 2019-08-02  6:00 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

On Fri, Aug 2, 2019 at 6:08 AM Heiko Schocher <hs@denx.de> wrote:
>
> Hello Julius,
>
> Am 01.08.2019 um 20:24 schrieb Julius Werner:
> >> First, we have a problem as we need to support what's out in the world.
> >
> > How about I write a patch to change the decompression call in
> > fit_image_load() to fall back to a memcpy() if the decompression fails
> > (and print a big warning that the FIT image is wrong and should be
> > updated)? That should keep those old installations working.
>
> Hmm.. what happens if decompression does not fail ?
> (I wonder, why I get this Error, but I go into vacation, so I have
>   no chance to dig deeper into it soon ...)
>
> And why should we decompress, if we "know" we do not have to (in ramdisk
> case)? This cost only boottime ...

It's correct we don't need to decompress since the kernel can decompress the
ramdisk. However, the FIT image is wrong (given our updated definition of the
"compressed" field) and it's only decompressed on "legacy" images.

>
> For the fast track, I prefer to ignore the compression property in ramdisk
> node ... (may configurable through a Kconfig option with default to ignore)

I'm not too fond of adding such extra cases, but you're right that
decompressing a ramdisk is probably never needed...

Regards,
Simon

>
> >> Second, what is intended with what's out in the world?  Indeed before we
> >> didn't do anything with the compressed data.  But now what, we're saying
> >> it's compressed, but not providing enough details to U-Boot to
> >> uncompress it?
> >
> > Hmm... I'm not sure why it wouldn't just decompress correctly,
> > actually. Is there anything special with how gzipped data is formatted
> > for the kernel decompressor? Or is there any limitation in the U-Boot
> > gzip implementation?
> >
>
> I do not know.
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-02  6:00           ` Simon Goldschmidt
@ 2019-08-02 12:36             ` Tom Rini
  2019-08-02 12:50               ` Simon Goldschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Rini @ 2019-08-02 12:36 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 02, 2019 at 08:00:22AM +0200, Simon Goldschmidt wrote:
> Hi Heiko,
> 
> On Fri, Aug 2, 2019 at 6:08 AM Heiko Schocher <hs@denx.de> wrote:
> >
> > Hello Julius,
> >
> > Am 01.08.2019 um 20:24 schrieb Julius Werner:
> > >> First, we have a problem as we need to support what's out in the world.
> > >
> > > How about I write a patch to change the decompression call in
> > > fit_image_load() to fall back to a memcpy() if the decompression fails
> > > (and print a big warning that the FIT image is wrong and should be
> > > updated)? That should keep those old installations working.
> >
> > Hmm.. what happens if decompression does not fail ?
> > (I wonder, why I get this Error, but I go into vacation, so I have
> >   no chance to dig deeper into it soon ...)
> >
> > And why should we decompress, if we "know" we do not have to (in ramdisk
> > case)? This cost only boottime ...
> 
> It's correct we don't need to decompress since the kernel can decompress the
> ramdisk. However, the FIT image is wrong (given our updated definition of the
> "compressed" field) and it's only decompressed on "legacy" images.
> 
> >
> > For the fast track, I prefer to ignore the compression property in ramdisk
> > node ... (may configurable through a Kconfig option with default to ignore)
> 
> I'm not too fond of adding such extra cases, but you're right that
> decompressing a ramdisk is probably never needed...

I think we have a case where intentions differed historically and now we
have to deal with it.  Back in the earlier thread this year where we
talked about this exact case, it was OK to start decompressing ramdisks
as that was the intention way back when, but it was silently broken
forever ago.  After it was broken is when FIT adoption finally took off
and we have the real life case of "set compression to the factual value,
but it's not used by U-Boot!" has now become "set compression to the
factual value and U-Boot decompresses it".

Maybe the best choice is to rework things so that we only check
compression on (and decompress) device trees and when we print ramdisk
information we note that it's not being decompressed?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190802/aca544c6/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-02 12:36             ` Tom Rini
@ 2019-08-02 12:50               ` Simon Goldschmidt
  2019-08-02 13:14                 ` Tom Rini
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Goldschmidt @ 2019-08-02 12:50 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 2, 2019 at 2:36 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Aug 02, 2019 at 08:00:22AM +0200, Simon Goldschmidt wrote:
> > Hi Heiko,
> >
> > On Fri, Aug 2, 2019 at 6:08 AM Heiko Schocher <hs@denx.de> wrote:
> > >
> > > Hello Julius,
> > >
> > > Am 01.08.2019 um 20:24 schrieb Julius Werner:
> > > >> First, we have a problem as we need to support what's out in the world.
> > > >
> > > > How about I write a patch to change the decompression call in
> > > > fit_image_load() to fall back to a memcpy() if the decompression fails
> > > > (and print a big warning that the FIT image is wrong and should be
> > > > updated)? That should keep those old installations working.
> > >
> > > Hmm.. what happens if decompression does not fail ?
> > > (I wonder, why I get this Error, but I go into vacation, so I have
> > >   no chance to dig deeper into it soon ...)
> > >
> > > And why should we decompress, if we "know" we do not have to (in ramdisk
> > > case)? This cost only boottime ...
> >
> > It's correct we don't need to decompress since the kernel can decompress the
> > ramdisk. However, the FIT image is wrong (given our updated definition of the
> > "compressed" field) and it's only decompressed on "legacy" images.
> >
> > >
> > > For the fast track, I prefer to ignore the compression property in ramdisk
> > > node ... (may configurable through a Kconfig option with default to ignore)
> >
> > I'm not too fond of adding such extra cases, but you're right that
> > decompressing a ramdisk is probably never needed...
>
> I think we have a case where intentions differed historically and now we
> have to deal with it.  Back in the earlier thread this year where we
> talked about this exact case, it was OK to start decompressing ramdisks
> as that was the intention way back when, but it was silently broken
> forever ago.  After it was broken is when FIT adoption finally took off
> and we have the real life case of "set compression to the factual value,
> but it's not used by U-Boot!" has now become "set compression to the
> factual value and U-Boot decompresses it".
>
> Maybe the best choice is to rework things so that we only check
> compression on (and decompress) device trees and when we print ramdisk
> information we note that it's not being decompressed?

While I can understand that point of view, please note that my intention is
not having a compressed devicetree (where we save some KByte) but having a
compressed FPGA image (where I save about 2 MByte!).

Given that, please don't start a positive list (e.g. only decompress
devicetree and Kernel). But if we actually need to do this differently per
type, just implement Heiko's suggestion and don't decompress ramdisks.

That way, we could show an error when seeing this setting for ramdisks and
probably remove this special case handling in some years...?

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-02 12:50               ` Simon Goldschmidt
@ 2019-08-02 13:14                 ` Tom Rini
  2019-08-02 21:57                   ` Julius Werner
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Rini @ 2019-08-02 13:14 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 02, 2019 at 02:50:40PM +0200, Simon Goldschmidt wrote:
> On Fri, Aug 2, 2019 at 2:36 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Aug 02, 2019 at 08:00:22AM +0200, Simon Goldschmidt wrote:
> > > Hi Heiko,
> > >
> > > On Fri, Aug 2, 2019 at 6:08 AM Heiko Schocher <hs@denx.de> wrote:
> > > >
> > > > Hello Julius,
> > > >
> > > > Am 01.08.2019 um 20:24 schrieb Julius Werner:
> > > > >> First, we have a problem as we need to support what's out in the world.
> > > > >
> > > > > How about I write a patch to change the decompression call in
> > > > > fit_image_load() to fall back to a memcpy() if the decompression fails
> > > > > (and print a big warning that the FIT image is wrong and should be
> > > > > updated)? That should keep those old installations working.
> > > >
> > > > Hmm.. what happens if decompression does not fail ?
> > > > (I wonder, why I get this Error, but I go into vacation, so I have
> > > >   no chance to dig deeper into it soon ...)
> > > >
> > > > And why should we decompress, if we "know" we do not have to (in ramdisk
> > > > case)? This cost only boottime ...
> > >
> > > It's correct we don't need to decompress since the kernel can decompress the
> > > ramdisk. However, the FIT image is wrong (given our updated definition of the
> > > "compressed" field) and it's only decompressed on "legacy" images.
> > >
> > > >
> > > > For the fast track, I prefer to ignore the compression property in ramdisk
> > > > node ... (may configurable through a Kconfig option with default to ignore)
> > >
> > > I'm not too fond of adding such extra cases, but you're right that
> > > decompressing a ramdisk is probably never needed...
> >
> > I think we have a case where intentions differed historically and now we
> > have to deal with it.  Back in the earlier thread this year where we
> > talked about this exact case, it was OK to start decompressing ramdisks
> > as that was the intention way back when, but it was silently broken
> > forever ago.  After it was broken is when FIT adoption finally took off
> > and we have the real life case of "set compression to the factual value,
> > but it's not used by U-Boot!" has now become "set compression to the
> > factual value and U-Boot decompresses it".
> >
> > Maybe the best choice is to rework things so that we only check
> > compression on (and decompress) device trees and when we print ramdisk
> > information we note that it's not being decompressed?
> 
> While I can understand that point of view, please note that my intention is
> not having a compressed devicetree (where we save some KByte) but having a
> compressed FPGA image (where I save about 2 MByte!).
> 
> Given that, please don't start a positive list (e.g. only decompress
> devicetree and Kernel). But if we actually need to do this differently per
> type, just implement Heiko's suggestion and don't decompress ramdisks.
> 
> That way, we could show an error when seeing this setting for ramdisks and
> probably remove this special case handling in some years...?

OK.  So lets special case ramdisk to just note that we see it's
compressed ("compression: %s, ignored" ?) and then we'll find out if
there's really a bunch of cases where people assumed it was decompressed
by us or not.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190802/26024af6/attachment.sig>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] fit image: boot gzipped ramdisk does not work anymore
  2019-08-02 13:14                 ` Tom Rini
@ 2019-08-02 21:57                   ` Julius Werner
  0 siblings, 0 replies; 14+ messages in thread
From: Julius Werner @ 2019-08-02 21:57 UTC (permalink / raw)
  To: u-boot

> OK.  So lets special case ramdisk to just note that we see it's
> compressed ("compression: %s, ignored" ?) and then we'll find out if
> there's really a bunch of cases where people assumed it was decompressed
> by us or not.

Sounds good! I'll send a patch.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-08-02 21:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01  8:06 [U-Boot] fit image: boot gzipped ramdisk does not work anymore Heiko Schocher
2019-08-01 12:33 ` Simon Goldschmidt
2019-08-01 14:38   ` Heiko Schocher
2019-08-01 15:58     ` Tom Rini
2019-08-01 18:24       ` Julius Werner
2019-08-01 18:34         ` Simon Goldschmidt
2019-08-01 19:01           ` Julius Werner
2019-08-01 19:06             ` Simon Goldschmidt
2019-08-02  4:08         ` Heiko Schocher
2019-08-02  6:00           ` Simon Goldschmidt
2019-08-02 12:36             ` Tom Rini
2019-08-02 12:50               ` Simon Goldschmidt
2019-08-02 13:14                 ` Tom Rini
2019-08-02 21:57                   ` Julius Werner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.