[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.
Atish Patra
atishp at atishpatra.org
Mon Feb 17 01:48:22 CET 2020
On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra at wdc.com>
> > > > > > > wrote:
> > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > both
> > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > difficult
> > > > > > > > to calculate a safe address for every board where the
> > > > > > > > compressed
> > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > out
> > > > > > > > the
> > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > filesize
> > > > > > > > to
> > > > > > > > make this work.
> > > > > > > >
> > > > > > > > Following compression methods are supported for now.
> > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > >
> > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > compressed
> > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > and
> > > > > > > > decompress.
> > > > > > > >
> > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > Tested on Qemu for ARM64.
> > > > > > > >
> > > > > > > > Signed-off-by: Atish Patra <atish.patra at wdc.com>
> > > > > > > > ---
> > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > that
> > > > > > > > would be great.
> > > > > > > > ---
> > > > > > > > cmd/booti.c | 40 ++++++++++++++++++++++++++-
> > > > > > > > doc/README.distro | 12 +++++++++
> > > > > > > > doc/board/sifive/fu540.rst | 55
> > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > 3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > --- a/cmd/booti.c
> > > > > > > > +++ b/cmd/booti.c
> > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > #include <linux/kernel.h>
> > > > > > > > #include <linux/sizes.h>
> > > > > > > >
> > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > /*
> > > > > > > > * Image booting support
> > > > > > > > */
> > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > flag, int argc,
> > > > > > > > ulong ld;
> > > > > > > > ulong relocated_addr;
> > > > > > > > ulong image_size;
> > > > > > > > + uint8_t *temp;
> > > > > > > > + ulong dest;
> > > > > > > > + ulong dest_end;
> > > > > > > > + unsigned long comp_len;
> > > > > > > > + unsigned long decomp_len;
> > > > > > > > + int ctype;
> > > > > > > >
> > > > > > > > ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > BOOTM_STATE_START,
> > > > > > > > images, 1);
> > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > flag, int argc,
> > > > > > > > debug("* kernel: cmdline image address =
> > > > > > > > 0x%08lx\n", ld);
> > > > > > > > }
> > > > > > > >
> > > > > > > > + temp = map_sysmem(ld, 0);
> > > > > > > > + ctype = image_decomp_type(temp, 2);
> > > > > > > > + if (ctype > 0) {
> > > > > > > > + dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > 0);
> > > > > > > > + comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > + if (!dest || !comp_len) {
> > > > > > > > + puts("kernel_comp_addr_r or filesize is
> > > > > > > > not
> > > > > > > > provided!\n");
> > > > > > > > + return -EINVAL;
> > > > > > > > + }
> > > > > > > > + if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > {
> > > > > > > > + puts("kernel_comp_addr_r is outside of
> > > > > > > > DRAM
> > > > > > > > range!\n");
> > > > > > > > + return -EINVAL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + debug("kernel image compression type %d size =
> > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > + ctype, comp_len, (ulong)dest);
> > > > > > > > + decomp_len = comp_len * 10;
> > > > > > > > + ret = image_decomp(ctype, 0, ld,
> > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > + (void *)dest, (void *)ld,
> > > > > > > > comp_len,
> > > > > > > > + decomp_len, &dest_end);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + /* dest_end contains the uncompressed Image
> > > > > > > > size
> > > > > > > > */
> > > > > > > > + memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > + }
> > > > > > > > + unmap_sysmem((void *)ld);
> > > > > > > > +
> > > > > > > > ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > false);
> > > > > > > > if (ret != 0)
> > > > > > > > return 1;
> > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > int
> > > > > > > > argc, char * const argv[])
> > > > > > > > #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > static char booti_help_text[] =
> > > > > > > > "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > - " - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > + " - boot Linux flat or compressed 'Image' stored at
> > > > > > > > 'addr'\n"
> > > > > > > > "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > address\n"
> > > > > > > > "\tof an initrd in memory. The optional parameter
> > > > > > > > ':size'
> > > > > > > > allows\n"
> > > > > > > > "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > + "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > compression\n"
> > > > > > > > + "\ttypes are supported. In order to boot from any of
> > > > > > > > these
> > > > > > > > compressed\n"
> > > > > > > > + "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > filesize
> > > > > > > > enviornment\n"
> > > > > > > > + "\tvariables beforehand.\n"
> > > > > > > > #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > tree, a\n"
> > > > > > > > "\tthird argument providing the address of the device-
> > > > > > > > tree
> > > > > > > > blob\n"
> > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > --- a/doc/README.distro
> > > > > > > > +++ b/doc/README.distro
> > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > >
> > > > > > > > A size of 16MB for the kernel is likely adequate.
> > > > > > > >
> > > > > > > > +kernel_comp_addr_r:
> > > > > > > > + Optional. This is only required if user wants to boot Linux
> > > > > > > > from
> > > > > > > > a compressed
> > > > > > > > + Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > represents
> > > > > > > > the location
> > > > > > > > + in RAM where the compressed Image will be decompressed
> > > > > > > > temporarily. Once the
> > > > > > > > + decompression is complete, decompressed data will be moved
> > > > > > > > kernel_addr_r for
> > > > > > > > + booting.
> > > > > > > > +
> > > > > > > > +filesize:
> > > > > > > > + Optional. This is only required if user wants to boot Linux
> > > > > > > > from
> > > > > > > > a compressed
> > > > > > > > + Image using booti command. It represents the size of the
> > > > > > > > compressed file. The
> > > > > > > > + size has to at least the size of loaded image for
> > > > > > > > decompression
> > > > > > > > to succeed.
> > > > > > > > +
> > > > > > >
> > > > > > > I am not sure I like using filesize here compared to
> > > > > > > kernel_gz_size.
> > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > e.g.
> > > > > > > from mmc.
> > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > >
> > > > > > > The load happens in this order:
> > > > > > > - initrd
> > > > > > > - kernel
> > > > > > > - fdt
> > > > > > >
> > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > be
> > > > > > > wrong
> > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > kernel after
> > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > >
> > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > to
> > > > > > > max
> > > > > > > allowed size.
> > > > > > >
> > > > > > > david
> > > > > > >
> > > > > >
> > > > > > Ahh okay. There are two ways to fix it.
> > > > > >
> > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > later.
> > > > > >
> > > > > > But some other user may fall into the same issue without realizing
> > > > > > it
> > > > > > if the user loads any other image after compressed kernel Image.
> > > > > >
> > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > should
> > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > manually
> > > > > > before calling booti.
> > > > > >
> > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > size) to avoid any confusion.
> > > > > >
> > > > > > Any preference ?
> > > > > >
> > > > >
> > > > > Any thoughts ?
> > > > >
> > > >
> > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > should be the best approach to resolve the issue with pxe
> > > > implementation ?
> > >
> > > I think that we really need to be careful when adding "automagic"
> > > features like this. Thinking harder about it all, we can't re-use any
> > > existing variable as there's going to be some case of a setup breaking
> > > in strange silent ways.
> > >
> > > Can we uncompress a single chunk / page / something of the compressed
> > > image to get the header and thus know the uncompressed size? We would
> > > then know the compressed size is no larger than that and can progress
> > > from there?
> >
> > After a quick google on various compression formats I get impression this
> > is not a common thing to record in the headers. The kernels could be
> > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > riscv).
>
> Sorry I wasn't clear enough. The contents in question here contain a
> header at the start which does contain the uncompressed image size.
>
The objective of the patch was to support all the compressed image
type that kernel supports.
i.e. gz, bz2, lzma, lzo.
uncompressed image size in compressed header in the following file formats:
1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
2. bz2: No information in the header
3. lzma: Present in the header
(https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
To summarize, if we want to parse the compressed file to determine the
uncompressed size,
we have to add the support to generic compression library for all file
formats rather than just doing it here.
Additionally, not all format mandates the uncompressed size which
makes it difficult.
I have an alternate idea.
1. Save the compressed image size in an environment variable while
loading the image using tftpboot.
Thus, user doesn't have to save the compressed image size in that
variable. The variable can be just internal
and not exposed to the user.
> --
> Tom
--
Regards,
Atish
More information about the U-Boot
mailing list