| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAK7LNASFR4XtyNKDtMabm6gd+oeMefZsg4nDMaY+Qmg2KkN8ew@mail.gmail.com>
Date: Sun, 25 Feb 2024 17:40:39 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Simon Glass <sjg@...omium.org>
Cc: linux-arm-kernel@...ts.infradead.org,
Ahmad Fatoum <a.fatoum@...gutronix.de>, Nicolas Schier <nicolas@...sle.eu>,
Catalin Marinas <catalin.marinas@....com>, Jonathan Corbet <corbet@....net>,
Nathan Chancellor <nathan@...nel.org>, Nick Terrell <terrelln@...com>, Will Deacon <will@...nel.org>,
linux-doc@...r.kernel.org, linux-kbuild@...r.kernel.org,
linux-kernel@...r.kernel.org, workflows@...r.kernel.org
Subject: Re: [PATCH v10 2/2] arm64: boot: Support Flat Image Tree
On Wed, Feb 21, 2024 at 9:37 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Sat, Feb 3, 2024 at 2:30 AM Simon Glass <sjg@...omium.org> wrote:
> >
> > Add a script which produces a Flat Image Tree (FIT), a single file
> > containing the built kernel and associated devicetree files.
> > Compression defaults to gzip which gives a good balance of size and
> > performance.
> >
> > The files compress from about 86MB to 24MB using this approach.
> >
> > The FIT can be used by bootloaders which support it, such as U-Boot
> > and Linuxboot. It permits automatic selection of the correct
> > devicetree, matching the compatible string of the running board with
> > the closest compatible string in the FIT. There is no need for
> > filenames or other workarounds.
> >
> > Add a 'make image.fit' build target for arm64, as well.
> >
> > The FIT can be examined using 'dumpimage -l'.
> >
> > This uses the 'dtbs-list' file but processes only .dtb files, ignoring
> > the overlay .dtbo files.
> >
> > This features requires pylibfdt (use 'pip install libfdt'). It also
> > requires compression utilities for the algorithm being used. Supported
> > compression options are the same as the Image.xxx files. Use
> > FIT_COMPRESSION to select an algorithm other than gzip.
> >
> > While FIT supports a ramdisk / initrd, no attempt is made to support
> > this here, since it must be built separately from the Linux build.
> >
> > Signed-off-by: Simon Glass <sjg@...omium.org>
> > ---
> >
> > Changes in v10:
> > - Make use of dtbs-list file
> > - Mention dtbs-list and FIT_COMPRESSION
> > - Update copyright year
> > - Update cover letter to take account of an applied patch
> >
> > Changes in v9:
> > - Move the compression control into Makefile.lib
> >
> > Changes in v8:
> > - Drop compatible string in FDT node
> > - Correct sorting of MAINTAINERS to before ARM64 PORT
> > - Turn compress part of the make_fit.py comment in to a sentence
> > - Add two blank lines before parse_args() and setup_fit()
> > - Use 'image.fit: dtbs' instead of BUILD_DTBS var
> > - Use '$(<D)/dts' instead of '$(dir $<)dts'
> > - Add 'mkimage' details Documentation/process/changes.rst
> > - Allow changing the compression used
> > - Tweak cover letter since there is only one clean-up patch
> >
> > Changes in v7:
> > - Add Image as a dependency of image.fit
> > - Drop kbuild tag
> > - Add dependency on dtbs
> > - Drop unnecessary path separator for dtbs
> > - Rebase to -next
> >
> > Changes in v5:
> > - Drop patch previously applied
> > - Correct compression rule which was broken in v4
> >
> > Changes in v4:
> > - Use single quotes for UIMAGE_NAME
> >
> > Changes in v3:
> > - Drop temporary file image.itk
> > - Drop patch 'Use double quotes for image name'
> > - Drop double quotes in use of UIMAGE_NAME
> > - Drop unnecessary CONFIG_EFI_ZBOOT condition for help
> > - Avoid hard-coding "arm64" for the DT architecture
> >
> > Changes in v2:
> > - Drop patch previously applied
> > - Add .gitignore file
> > - Move fit rule to Makefile.lib using an intermediate file
> > - Drop dependency on CONFIG_EFI_ZBOOT
> > - Pick up .dtb files separately from the kernel
> > - Correct pylint too-many-args warning for write_kernel()
> > - Include the kernel image in the file count
> > - Add a pointer to the FIT spec and mention of its wide industry usage
> > - Mention the kernel version in the FIT description
> >
> > Documentation/process/changes.rst | 9 +
> > MAINTAINERS | 7 +
> > arch/arm64/Makefile | 7 +-
> > arch/arm64/boot/.gitignore | 1 +
> > arch/arm64/boot/Makefile | 6 +-
> > scripts/Makefile.lib | 16 ++
> > scripts/make_fit.py | 298 ++++++++++++++++++++++++++++++
> > 7 files changed, 341 insertions(+), 3 deletions(-)
> > create mode 100755 scripts/make_fit.py
> >
> > diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> > index 50b3d1cb1115..a8110965e4e1 100644
> > --- a/Documentation/process/changes.rst
> > +++ b/Documentation/process/changes.rst
> > @@ -62,6 +62,7 @@ Sphinx\ [#f1]_ 2.4.4 sphinx-build --version
> > cpio any cpio --version
> > GNU tar 1.28 tar --version
> > gtags (optional) 6.6.5 gtags --version
> > +mkimage (optional) 2017.01 mkimage --version
> > ====================== =============== ========================================
> >
> > .. [#f1] Sphinx is needed only to build the Kernel documentation
> > @@ -189,6 +190,14 @@ The kernel build requires GNU GLOBAL version 6.6.5 or later to generate
> > tag files through ``make gtags``. This is due to its use of the gtags
> > ``-C (--directory)`` flag.
> >
> > +mkimage
> > +-------
> > +
> > +This tool is used when building a Flat Image Tree (FIT), commonly used on ARM
> > +platforms. The tool is available via the ``u-boot-tools`` package or can be
> > +built from the U-Boot source code. See the instructions at
> > +https://docs.u-boot.org/en/latest/build/tools.html#building-tools-for-linux
> > +
> > System utilities
> > ****************
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 61117c3afa80..10c2753d7bcc 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3026,6 +3026,13 @@ F: drivers/mmc/host/sdhci-of-arasan.c
> > N: zynq
> > N: xilinx
> >
> > +ARM64 FIT SUPPORT
> > +M: Simon Glass <sjg@...omium.org>
> > +L: linux-arm-kernel@...ts.infradead.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: arch/arm64/boot/Makefile
> > +F: scripts/make_fit.py
> > +
> > ARM64 PORT (AARCH64 ARCHITECTURE)
> > M: Catalin Marinas <catalin.marinas@....com>
> > M: Will Deacon <will@...nel.org>
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 83cd2b7234b9..5de2b02f549a 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -150,7 +150,7 @@ libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > # Default target when executing plain make
> > boot := arch/arm64/boot
> >
> > -BOOT_TARGETS := Image vmlinuz.efi
> > +BOOT_TARGETS := Image vmlinuz.efi image.fit
> >
> > PHONY += $(BOOT_TARGETS)
> >
> > @@ -162,7 +162,9 @@ endif
> >
> > all: $(notdir $(KBUILD_IMAGE))
> >
> > -vmlinuz.efi: Image
> > +image.fit: dtbs
> > +
> > +vmlinuz.efi image.fit: Image
> > $(BOOT_TARGETS): vmlinux
> > $(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
> >
> > @@ -215,6 +217,7 @@ virtconfig:
> > define archhelp
> > echo '* Image.gz - Compressed kernel image (arch/$(ARCH)/boot/Image.gz)'
> > echo ' Image - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
> > + echo ' image.fit - Flat Image Tree (arch/$(ARCH)/boot/image.fit)'
> > echo ' install - Install uncompressed kernel'
> > echo ' zinstall - Install compressed kernel'
> > echo ' Install using (your) ~/bin/installkernel or'
> > diff --git a/arch/arm64/boot/.gitignore b/arch/arm64/boot/.gitignore
> > index af5dc61f8b43..abaae9de1bdd 100644
> > --- a/arch/arm64/boot/.gitignore
> > +++ b/arch/arm64/boot/.gitignore
> > @@ -2,3 +2,4 @@
> > Image
> > Image.gz
> > vmlinuz*
> > +image.fit
> > diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile
> > index a5a787371117..ab21af82913e 100644
> > --- a/arch/arm64/boot/Makefile
> > +++ b/arch/arm64/boot/Makefile
> > @@ -16,7 +16,8 @@
> >
> > OBJCOPYFLAGS_Image :=-O binary -R .note -R .note.gnu.build-id -R .comment -S
> >
> > -targets := Image Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo Image.zst
> > +targets := Image Image.bz2 Image.gz Image.lz4 Image.lzma Image.lzo \
> > + Image.zst image.fit
> >
> > $(obj)/Image: vmlinux FORCE
> > $(call if_changed,objcopy)
> > @@ -39,6 +40,9 @@ $(obj)/Image.lzo: $(obj)/Image FORCE
> > $(obj)/Image.zst: $(obj)/Image FORCE
> > $(call if_changed,zstd)
> >
> > +$(obj)/image.fit: $(obj)/Image FORCE
> > + $(call cmd,fit)
>
>
>
> The point for using dtbs-list is
> to avoid rebuilding image.fit needlessly.
>
>
> This should be:
>
> $(obj)/image.fit: $(obj)/Image $(obj)/dts/dtbs-list FORCE
> $(call if_changed,fit)
>
>
>
>
>
> > +
> > EFI_ZBOOT_PAYLOAD := Image
> > EFI_ZBOOT_BFD_TARGET := elf64-littleaarch64
> > EFI_ZBOOT_MACH_TYPE := ARM64
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index b35d39022a30..4efb5ad07fd7 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -502,6 +502,22 @@ quiet_cmd_uimage = UIMAGE $@
> > -a $(UIMAGE_LOADADDR) -e $(UIMAGE_ENTRYADDR) \
> > -n '$(UIMAGE_NAME)' -d $< $@
> >
> > +# Flat Image Tree (FIT)
> > +# This allows for packaging of a kernel and all devicetrees files, using
> > +# compression.
> > +# ---------------------------------------------------------------------------
> > +
> > +MAKE_FIT := $(srctree)/scripts/make_fit.py
> > +
> > +# Use this to override the compression algorithm
> > +FIT_COMPRESSION ?= gzip
> > +
> > +quiet_cmd_fit = FIT $@
> > + cmd_fit = $(MAKE_FIT) -f $@ --arch $(UIMAGE_ARCH) --os linux \
> > + --name '$(UIMAGE_NAME)' \
> > + --compress $(FIT_COMPRESSION) -k $< \
> > + @arch/$(SRCARCH)/boot/dts/dtbs-list
>
>
>
>
> cmd_fit = $(MAKE_FIT) -f $@ --arch $(UIMAGE_ARCH) --os linux \
> --name '$(UIMAGE_NAME)' \
> --compress $(FIT_COMPRESSION) -k $< @$(word 2,$^)
>
>
>
>
>
> > +
> > # XZ
> > # ---------------------------------------------------------------------------
> > # Use xzkern to compress the kernel image and xzmisc to compress other things.
> > diff --git a/scripts/make_fit.py b/scripts/make_fit.py
> > new file mode 100755
> > index 000000000000..69eee32960ae
> > --- /dev/null
> > +++ b/scripts/make_fit.py
> > @@ -0,0 +1,298 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2024 Google LLC
> > +# Written by Simon Glass <sjg@...omium.org>
> > +#
> > +
> > +"""Build a FIT containing a lot of devicetree files
> > +
> > +Usage:
> > + make_fit.py -A arm64 -n 'Linux-6.6' -O linux
> > + -f arch/arm64/boot/image.fit -k /tmp/kern/arch/arm64/boot/image.itk
> > + @arch/arm64/boot/dts/dtbs-list -E -c gzip
> > +
> > +Creates a FIT containing the supplied kernel and a set of devicetree files,
> > +either specified individually or listed in a file (with an '@' prefix) Files
> > +which don't end in '.dtb' are silently ignored.
>
>
> Why do you need to check the suffix?
>
>
>
> > +
> > +Use -E to generate an external FIT (where the data is placed after the
> > +FIT data structure). This allows parsing of the data without loading
> > +the entire FIT.
> > +
> > +Use -c to compress the data, using bzip2, gzip, lz4, lzma, lzo and
> > +zstd algorithms.
> > +
> > +The resulting FIT can be booted by bootloaders which support FIT, such
> > +as U-Boot, Linuxboot, Tianocore, etc.
> > +
> > +Note that this tool does not yet support adding a ramdisk / initrd.
> > +"""
> > +
> > +import argparse
> > +import collections
> > +import os
> > +import subprocess
> > +import sys
> > +import tempfile
> > +import time
> > +
> > +import libfdt
> > +
> > +
> > +# Tool extension and the name of the command-line tools
> > +CompTool = collections.namedtuple('CompTool', 'ext,tools')
> > +
> > +COMP_TOOLS = {
> > + 'bzip2': CompTool('.bz2', 'bzip2'),
> > + 'gzip': CompTool('.gz', 'pigz,gzip'),
> > + 'lz4': CompTool('.lz4', 'lz4'),
> > + 'lzma': CompTool('.lzma', 'lzma'),
> > + 'lzo': CompTool('.lzo', 'lzop'),
> > + 'zstd': CompTool('.zstd', 'zstd'),
> > +}
> > +
> > +
> > +def parse_args():
> > + """Parse the program ArgumentParser
> > +
> > + Returns:
> > + Namespace object containing the arguments
> > + """
> > + epilog = 'Build a FIT from a directory tree containing .dtb files'
> > + parser = argparse.ArgumentParser(epilog=epilog)
> > + parser.add_argument('-A', '--arch', type=str, required=True,
> > + help='Specifies the architecture')
> > + parser.add_argument('-c', '--compress', type=str, default='none',
> > + help='Specifies the compression')
> > + parser.add_argument('-E', '--external', action='store_true',
> > + help='Convert the FIT to use external data')
> > + parser.add_argument('-n', '--name', type=str, required=True,
> > + help='Specifies the name')
> > + parser.add_argument('-O', '--os', type=str, required=True,
> > + help='Specifies the operating system')
> > + parser.add_argument('-f', '--fit', type=str, required=True,
> > + help='Specifies the output file (.fit)')
>
>
>
> I like -o (--output) to specify the output file.
>
>
>
> parser.add_argument('-o', '--output', type=str, required=True,
> help='Specifies the output file (.fit)')
>
>
>
>
>
> > + parser.add_argument('-k', '--kernel', type=str, required=True,
> > + help='Specifies the (uncompressed) kernel input file (.itk)')
> > + parser.add_argument('srcdir', type=str, nargs='*',
> > + help='Specifies the directory tree that contains .dtb files')
>
>
> srcdir?
>
> You changed the positional parameters to take dtb files.
>
>
>
>
>
>
> > +
> > + return parser.parse_args()
> > +
> > +
> > +def setup_fit(fsw, name):
> > + """Make a start on writing the FIT
> > +
> > + Outputs the root properties and the 'images' node
> > +
> > + Args:
> > + fsw (libfdt.FdtSw): Object to use for writing
> > + name (str): Name of kernel image
> > + """
> > + fsw.INC_SIZE = 65536
> > + fsw.finish_reservemap()
> > + fsw.begin_node('')
> > + fsw.property_string('description', f'{name} with devicetree set')
> > + fsw.property_u32('#address-cells', 1)
> > +
> > + fsw.property_u32('timestamp', int(time.time()))
> > + fsw.begin_node('images')
> > +
> > +
> > +def write_kernel(fsw, data, args):
> > + """Write out the kernel image
> > +
> > + Writes a kernel node along with the required properties
> > +
> > + Args:
> > + fsw (libfdt.FdtSw): Object to use for writing
> > + data (bytes): Data to write (possibly compressed)
> > + args (Namespace): Contains necessary strings:
> > + arch: FIT architecture, e.g. 'arm64'
> > + fit_os: Operating Systems, e.g. 'linux'
> > + name: Name of OS, e.g. 'Linux-6.6.0-rc7'
> > + compress: Compression algorithm to use, e.g. 'gzip'
> > + """
> > + with fsw.add_node('kernel'):
> > + fsw.property_string('description', args.name)
> > + fsw.property_string('type', 'kernel_noload')
> > + fsw.property_string('arch', args.arch)
> > + fsw.property_string('os', args.os)
> > + fsw.property_string('compression', args.compress)
> > + fsw.property('data', data)
> > + fsw.property_u32('load', 0)
> > + fsw.property_u32('entry', 0)
> > +
> > +
> > +def finish_fit(fsw, entries):
> > + """Finish the FIT ready for use
> > +
> > + Writes the /configurations node and subnodes
> > +
> > + Args:
> > + fsw (libfdt.FdtSw): Object to use for writing
> > + entries (list of tuple): List of configurations:
> > + str: Description of model
> > + str: Compatible stringlist
> > + """
> > + fsw.end_node()
> > + seq = 0
> > + with fsw.add_node('configurations'):
> > + for model, compat in entries:
> > + seq += 1
> > + with fsw.add_node(f'conf-{seq}'):
> > + fsw.property('compatible', bytes(compat))
> > + fsw.property_string('description', model)
> > + fsw.property_string('fdt', f'fdt-{seq}')
> > + fsw.property_string('kernel', 'kernel')
> > + fsw.end_node()
> > +
> > +
> > +def compress_data(inf, compress):
> > + """Compress data using a selected algorithm
> > +
> > + Args:
> > + inf (IOBase): Filename containing the data to compress
> > + compress (str): Compression algorithm, e.g. 'gzip'
> > +
> > + Return:
> > + bytes: Compressed data
> > + """
> > + if compress == 'none':
> > + return inf.read()
> > +
> > + comp = COMP_TOOLS.get(compress)
> > + if not comp:
> > + raise ValueError(f"Unknown compression algorithm '{compress}'")
> > +
> > + with tempfile.NamedTemporaryFile() as comp_fname:
> > + with open(comp_fname.name, 'wb') as outf:
> > + done = False
> > + for tool in comp.tools.split(','):
> > + try:
> > + subprocess.call([tool, '-c'], stdin=inf, stdout=outf)
> > + done = True
> > + break
> > + except FileNotFoundError:
> > + pass
> > + if not done:
> > + raise ValueError(f'Missing tool(s): {comp.tools}\n')
> > + with open(comp_fname.name, 'rb') as compf:
> > + comp_data = compf.read()
> > + return comp_data
> > +
> > +
> > +def output_dtb(fsw, seq, fname, arch, compress):
> > + """Write out a single devicetree to the FIT
> > +
> > + Args:
> > + fsw (libfdt.FdtSw): Object to use for writing
> > + seq (int): Sequence number (1 for first)
> > + fmame (str): Filename containing the DTB
> > + arch: FIT architecture, e.g. 'arm64'
> > + compress (str): Compressed algorithm, e.g. 'gzip'
> > +
> > + Returns:
> > + tuple:
> > + str: Model name
> > + bytes: Compatible stringlist
> > + """
> > + with fsw.add_node(f'fdt-{seq}'):
> > + # Get the compatible / model information
> > + with open(fname, 'rb') as inf:
> > + data = inf.read()
> > + fdt = libfdt.FdtRo(data)
> > + model = fdt.getprop(0, 'model').as_str()
> > + compat = fdt.getprop(0, 'compatible')
> > +
> > + fsw.property_string('description', model)
> > + fsw.property_string('type', 'flat_dt')
> > + fsw.property_string('arch', arch)
> > + fsw.property_string('compression', compress)
> > + fsw.property('compatible', bytes(compat))
> > +
> > + with open(fname, 'rb') as inf:
> > + compressed = compress_data(inf, compress)
> > + fsw.property('data', compressed)
> > + return model, compat
> > +
> > +
> > +def build_fit(args):
> > + """Build the FIT from the provided files and arguments
> > +
> > + Args:
> > + args (Namespace): Program arguments
> > +
> > + Returns:
> > + tuple:
> > + bytes: FIT data
> > + int: Number of configurations generated
> > + size: Total uncompressed size of data
> > + """
> > + def add_file(fname):
> > + nonlocal seq, size
> > +
> > + if os.path.splitext(fname)[1] == '.dtb':
> > + seq += 1
> > + size += os.path.getsize(fname)
> > + model, compat = output_dtb(fsw, seq, fname, args.arch,
> > + args.compress)
> > + entries.append([model, compat])
> > + return True
> > +
> > + seq = 0
> > + size = 0
> > + fsw = libfdt.FdtSw()
> > + setup_fit(fsw, args.name)
> > + entries = []
> > +
> > + # Handle the kernel
> > + with open(args.kernel, 'rb') as inf:
> > + comp_data = compress_data(inf, args.compress)
> > + size += os.path.getsize(args.kernel)
> > + write_kernel(fsw, comp_data, args)
> > +
> > + for path in args.srcdir:
> > + # Handle a list of devicetree files
> > + if path.startswith('@'):
> > + with open(path[1:], 'r', encoding='utf-8') as inf:
> > + for fname in inf.read().splitlines():
> > + add_file(fname)
>
>
>
>
> You missed the point of my suggestion.
>
>
> I did not mean the "@file" syntax
> specifically for containing the device trees.
>
>
> It is common for tools to support the "@file" syntax
> to avoid "Argument list too long" error.
>
>
>
> See "man ar", "man ld", etc. for example.
>
>
> @file
> Read command‐line options from file. The options read are inserted in
> place of the original @file option. If file does not exist, or cannot
> be read, then the option will be treated literally, and not removed.
>
>
>
>
> It must be generic enough to contain any command line parameters.
>
>
> And, you do not even implement it yourself because
> it is just a matter of adding fromfile_prefix_chars='@'
>
>
> See the document.
>
> https://docs.python.org/3/library/argparse.html#fromfile-prefix-chars
Or, if you prefer using the 'file' only for containing DTB files,
I think it is better to use a standard short/long option instead of
the @file syntax.
For example, 'tar' can take files from FILE instead of the
positional parameters of the command line.
'man tar':
-T, --files-from=FILE
Get names to extract or create from FILE.
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists