Nícolas F. R. A. Prado <nfraprado@collabora.com> writes:
> A couple tweaks to the commands in the regression documentation to make
> them up-to-date and less confusing.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> Changes in v2:
> - Reworded patch 1:
> - s/collon/colon/
> - Made title and message more straightforward
> - Link to v1: https://lore.kernel.org/r/20240308-regzbot-fixes-v1-0-577a4fe16e12@collabora.com
>
> ---
> Nícolas F. R. A. Prado (2):
> docs: *-regressions.rst: Add colon to regzbot commands
> docs: handling-regressions.rst: Update regzbot command fixed-by to fix
>
> Documentation/admin-guide/reporting-regressions.rst | 2 +-
> Documentation/process/handling-regressions.rst | 12 ++++++------
> 2 files changed, 7 insertions(+), 7 deletions(-)
Applied, thanks.
jon
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@chromium.org> --- Changes in v11: - Use dtbslist file in image.fit rule - Update cmd_fit rule as per Masahiro - Don't mention ignoring files without a .dtb prefix - Use argparse fromfile_prefix_chars feature - Add a -v option and use it for output (with make V=1) - rename srcdir to dtbs - Use -o for the output file instead of -f 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 | 15 ++ scripts/make_fit.py | 290 ++++++++++++++++++++++++++++++ 7 files changed, 332 insertions(+), 3 deletions(-) create mode 100755 scripts/make_fit.py diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst index 7ef8de58f7f8..3a39395bd9d3 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 b43102ca365d..4bba1419ca35 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3050,6 +3050,13 @@ F: drivers/mmc/host/sdhci-of-arasan.c N: zynq N: xilinx +ARM64 FIT SUPPORT +M: Simon Glass <sjg@chromium.org> +L: linux-arm-kernel@lists.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@arm.com> M: Will Deacon <will@kernel.org> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 1217d97998ac..b8b1d4f4a572 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -154,7 +154,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) @@ -166,7 +166,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)/$@ @@ -219,6 +221,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..607a67a649c4 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 $(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 3179747cbd2c..0138d0b82be5 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -504,6 +504,21 @@ 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) -o $@ --arch $(UIMAGE_ARCH) --os linux \ + --name '$(UIMAGE_NAME)' $(if $(V),-v) \ + --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..3de90c5a094b --- /dev/null +++ b/scripts/make_fit.py @@ -0,0 +1,290 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2024 Google LLC +# Written by Simon Glass <sjg@chromium.org> +# + +"""Build a FIT containing a lot of devicetree files + +Usage: + make_fit.py -A arm64 -n 'Linux-6.6' -O linux + -o 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). + +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, fromfile_prefix_chars='@') + 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', '--output', type=str, required=True, + help='Specifies the output file (.fit)') + parser.add_argument('-O', '--os', type=str, required=True, + help='Specifies the operating system') + parser.add_argument('-k', '--kernel', type=str, required=True, + help='Specifies the (uncompressed) kernel input file (.itk)') + parser.add_argument('-v', '--verbose', action='store_true', + help='Enable verbose output') + parser.add_argument('dtbs', type=str, nargs='*', + help='Specifies the devicetree files to process') + + 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 + """ + 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 fname in args.dtbs: + # Ignore overlay (.dtbo) files + 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]) + + finish_fit(fsw, entries) + + # Include the kernel itself in the returned file count + return fsw.as_fdt().as_bytearray(), seq + 1, size + + +def run_make_fit(): + """Run the tool's main logic""" + args = parse_args() + + out_data, count, size = build_fit(args) + with open(args.output, 'wb') as outf: + outf.write(out_data) + + ext_fit_size = None + if args.external: + mkimage = os.environ.get('MKIMAGE', 'mkimage') + subprocess.check_call([mkimage, '-E', '-F', args.output], + stdout=subprocess.DEVNULL) + + with open(args.output, 'rb') as inf: + data = inf.read() + ext_fit = libfdt.FdtRo(data) + ext_fit_size = ext_fit.totalsize() + + if args.verbose: + comp_size = len(out_data) + print(f'FIT size {comp_size:#x}/{comp_size / 1024 / 1024:.1f} MB', + end='') + if ext_fit_size: + print(f', header {ext_fit_size:#x}/{ext_fit_size / 1024:.1f} KB', + end='') + print(f', {count} files, uncompressed {size / 1024 / 1024:.1f} MB') + + +if __name__ == "__main__": + sys.exit(run_make_fit()) -- 2.34.1
Flat Image Tree (FIT) is a widely used file format for packaging a kernel and associated devicetree files[1]. It is not specific to any one bootloader, as it is supported by U-Boot, coreboot, Linuxboot, Tianocore and Barebox. This series adds support for building a FIT as part of the kernel build. This makes it easy to try out the kernel - just load the FIT onto your tftp server and it will run automatically on any supported arm64 board. The script is written in Python, since it is easy to build a FIT using the Python libfdt bindings. For now, no attempt is made to compress files in parallel, so building the 900-odd files takes a while, about 6 seconds with my testing. The series also includes a minor clean-up patch. [1] https://github.com/open-source-firmware/flat-image-tree Changes in v11: - Use dtbslist file in image.fit rule - Update cmd_fit rule as per Masahiro - Don't mention ignoring files without a .dtb prefix - Use argparse fromfile_prefix_chars feature - Add a -v option and use it for output (with make V=1) - rename srcdir to dtbs - Use -o for the output file instead of -f 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: - Drop the kbuild tag - 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 v6: - Drop the unwanted .gz suffix 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 Simon Glass (2): arm64: Add BOOT_TARGETS variable arm64: boot: Support Flat Image Tree Documentation/process/changes.rst | 9 + MAINTAINERS | 7 + arch/arm64/Makefile | 11 +- arch/arm64/boot/.gitignore | 1 + arch/arm64/boot/Makefile | 6 +- scripts/Makefile.lib | 15 ++ scripts/make_fit.py | 290 ++++++++++++++++++++++++++++++ 7 files changed, 336 insertions(+), 3 deletions(-) create mode 100755 scripts/make_fit.py -- 2.34.1
Hi-- On 3/1/24 05:46, Lukas Bulwahn wrote: > Submitting-patches is already assuming that git is used to prepare > patches. So, developers will use git format-patch and git send-email, and > this will take care that PATCH is usually in the subject line. Hence, the > 'include PATCH in the subject' does not deserve be an own section. > > Move this note into 'the canonical patch format' section, where it > currently fits best. > > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> > --- > Documentation/process/submitting-patches.rst | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst > index 66029999b587..2ec0c0d7d68f 100644 > --- a/Documentation/process/submitting-patches.rst > +++ b/Documentation/process/submitting-patches.rst > @@ -384,16 +384,6 @@ patch or patch series which have not been modified in any way from the > previous submission. > > > -Include PATCH in the subject > ------------------------------ > - > -Due to high e-mail traffic to Linus, and to linux-kernel, it is common > -convention to prefix your subject line with [PATCH]. This lets Linus > -and other kernel developers more easily distinguish patches from other > -e-mail discussions. > - > -``git send-email`` will do this for you automatically. > - > > Sign your work - the Developer's Certificate of Origin > ------------------------------------------------------ > @@ -616,6 +606,10 @@ The canonical patch subject line is:: > > Subject: [PATCH 001/123] subsystem: summary phrase > > +Prefix your subject line with [PATCH]. This allows to distinguish patches > +from other e-mail discussions. ``git send-email`` will do this for you > +automatically. Is this perhaps 'git format-patch' will do this for you automatically. ? I don't know, just asking. > + > The canonical patch message body contains the following: > > - A ``from`` line specifying the patch author, followed by an empty thanks. -- #Randy
Hi Masahiro, On Mon, 11 Mar 2024 at 02:31, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Mar 8, 2024 at 12:55 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Masahiro, > > > > On Thu, 22 Feb 2024 at 01:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sat, Feb 3, 2024 at 2:30 AM Simon Glass <sjg@chromium.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@chromium.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@chromium.org> > > > > +L: linux-arm-kernel@lists.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@arm.com> > > > > M: Will Deacon <will@kernel.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@chromium.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 > > > > > > > > > > > > > > > > > > > + else: > > > > + add_file(path) > > > > + > > > > + finish_fit(fsw, entries) > > > > + > > > > + # Include the kernel itself in the returned file count > > > > + return fsw.as_fdt().as_bytearray(), seq + 1, size > > > > + > > > > + > > > > +def run_make_fit(): > > > > + """Run the tool's main logic""" > > > > + args = parse_args() > > > > + > > > > + out_data, count, size = build_fit(args) > > > > + with open(args.fit, 'wb') as outf: > > > > + outf.write(out_data) > > > > + > > > > + ext_fit_size = None > > > > + if args.external: > > > > + mkimage = os.environ.get('MKIMAGE', 'mkimage') > > > > + subprocess.check_call([mkimage, '-E', '-F', args.fit], > > > > + stdout=subprocess.DEVNULL) > > > > + > > > > + with open(args.fit, 'rb') as inf: > > > > + data = inf.read() > > > > + ext_fit = libfdt.FdtRo(data) > > > > + ext_fit_size = ext_fit.totalsize() > > > > > > > > > > > > I still do not understand why mkimage is needed. > > > > > > > > > When external data is used, you can insert "data-size" > > > and "data-offset" to the dt structure, > > > and at the same time, concatenate the payload data. > > > Finally, you can combine the two. > > > Is it complex to implement? > > > > Yes it is somewhat complex, since there are options like alignment and > > the like which are implemented in that tool. > > > > So, do you mean you need to pass > not only -E, -F but also more options to mkimage? Possibly, in the future, yes. > > > > > Is there a problem with > > using the tool? After all, people are using it today and will be > > expecting this script to use it, I believe. If we start adding mkimage > > functionality here then we will have to maintain it in two places and > > worry about whether they are consistent. Also if we want to support > > signatures, etc. then it would be quite an undertaking to write all > > that code again in Python. > > > Not a problem, but I think it is unfortunate. > I thought this script would be a standalone, > self-contained tool to produce a FIT image. Well it already requires pylibfdt. I am not keen on duplicating any more of the mkimage functionality here, particularly as the features tend to change and expand over time. > > > > > > > > > > > > > > > > > > > > > > > > > + > > > > + comp_size = len(out_data) > > > > + print(f'FIT size {comp_size:#x}/{comp_size / 1024 / 1024:.1f} MB', end='') > > > > + if ext_fit_size: > > > > + print(f', header {ext_fit_size:#x}/{ext_fit_size / 1024:.1f} KB', end='') > > > > + print(f', {count} files, uncompressed {size / 1024 / 1024:.1f} MB') > > > > > > > > > Maybe, you can print this only when args.verbose == True ? > > > > This and other comments seem OK to me and I will address them in v11. > > > > > > > > > > > > > > > > > > > > > > > > > > At last, I still do not know how to distinguish the nodes > > > when the compatible strings are the same. > > > > It is like trying to distinguish two devices with the same compatible > > string. Really, it should not be done. If you would like me to change > > the script in some way, please suggest something. > > > I cannot suggest anything at this moment. > > I believe we need a way to distinguish two devices, > but I do not see any consensus. If you would like my opinion, it is that duplicate compatible strings are not distinguishable by design. It would be better to fix the source files. Regards, Simon
On the reference documentation for regzbot, the fixed-by command has been renamed to fix. Update the kernel documentation accordingly. Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md Link: https://gitlab.com/knurd42/regzbot/-/commit/6d8d30f6bda84e1b711121bb98a07a464d3f089a Reviewed-by: Thorsten Leemhuis <linux@leemhuis.info> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Documentation/process/handling-regressions.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst index 42b13f77b019..ce6753a674f3 100644 --- a/Documentation/process/handling-regressions.rst +++ b/Documentation/process/handling-regressions.rst @@ -432,7 +432,7 @@ or itself is a reply to that mail: * Mark a regression as fixed by a commit that is heading upstream or already landed:: - #regzbot fixed-by: 1f2e3d4c5d + #regzbot fix: 1f2e3d4c5d * Mark a regression as a duplicate of another one already tracked by regzbot:: -- 2.44.0
Use colon as command terminator everywhere for consistency, even though it's not strictly necessary. That way it will also match regzbot's reference documentation. Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md Reviewed-by: Thorsten Leemhuis <linux@leemhuis.info> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Documentation/admin-guide/reporting-regressions.rst | 2 +- Documentation/process/handling-regressions.rst | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/admin-guide/reporting-regressions.rst b/Documentation/admin-guide/reporting-regressions.rst index d8adccdae23f..76b246ecf21b 100644 --- a/Documentation/admin-guide/reporting-regressions.rst +++ b/Documentation/admin-guide/reporting-regressions.rst @@ -31,7 +31,7 @@ The important bits (aka "TL;DR") Linux kernel regression tracking bot "regzbot" track the issue by specifying when the regression started like this:: - #regzbot introduced v5.13..v5.14-rc1 + #regzbot introduced: v5.13..v5.14-rc1 All the details on Linux kernel regressions relevant for users diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst index 5d3c3de3f4ec..42b13f77b019 100644 --- a/Documentation/process/handling-regressions.rst +++ b/Documentation/process/handling-regressions.rst @@ -27,11 +27,11 @@ The important bits (aka "The TL;DR") is optional, but recommended): * For mailed reports, check if the reporter included a line like ``#regzbot - introduced v5.13..v5.14-rc1``. If not, send a reply (with the regressions + introduced: v5.13..v5.14-rc1``. If not, send a reply (with the regressions list in CC) containing a paragraph like the following, which tells regzbot when the issue started to happen:: - #regzbot ^introduced 1f2e3d4c5b6a + #regzbot ^introduced: 1f2e3d4c5b6a * When forwarding reports from a bug tracker to the regressions list (see above), include a paragraph like the following:: @@ -79,7 +79,7 @@ When doing either, consider making the Linux kernel regression tracking bot "regzbot" immediately start tracking the issue: * For mailed reports, check if the reporter included a "regzbot command" like - ``#regzbot introduced 1f2e3d4c5b6a``. If not, send a reply (with the + ``#regzbot introduced: 1f2e3d4c5b6a``. If not, send a reply (with the regressions list in CC) with a paragraph like the following::: #regzbot ^introduced: v5.13..v5.14-rc1 @@ -398,9 +398,9 @@ By using a 'regzbot command' in a direct or indirect reply to the mail with the regression report. These commands need to be in their own paragraph (IOW: they need to be separated from the rest of the mail using blank lines). -One such command is ``#regzbot introduced <version or commit>``, which makes +One such command is ``#regzbot introduced: <version or commit>``, which makes regzbot consider your mail as a regressions report added to the tracking, as -already described above; ``#regzbot ^introduced <version or commit>`` is another +already described above; ``#regzbot ^introduced: <version or commit>`` is another such command, which makes regzbot consider the parent mail as a report for a regression which it starts to track. -- 2.44.0
A couple tweaks to the commands in the regression documentation to make them up-to-date and less confusing. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Changes in v2: - Reworded patch 1: - s/collon/colon/ - Made title and message more straightforward - Link to v1: https://lore.kernel.org/r/20240308-regzbot-fixes-v1-0-577a4fe16e12@collabora.com --- Nícolas F. R. A. Prado (2): docs: *-regressions.rst: Add colon to regzbot commands docs: handling-regressions.rst: Update regzbot command fixed-by to fix Documentation/admin-guide/reporting-regressions.rst | 2 +- Documentation/process/handling-regressions.rst | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) --- base-commit: 11afac187274a6177a7ac82997f8691c0f469e41 change-id: 20240308-regzbot-fixes-43a28fd4b621 Best regards, -- Nícolas F. R. A. Prado <nfraprado@collabora.com>
On Mon, Mar 11, 2024 at 02:39:46PM +0100, Thorsten Leemhuis wrote:
> Thx for this!
>
> On 08.03.24 15:09, Nícolas F. R. A. Prado wrote:
> > All the examples in the reference documentation for regzbot have a
> > collon
>
> s/collon/colon/ here and a few lines below as well. And in the subject
> as well. Speaking of which: something like "docs: *-regressions.rst:
> add colon to regzbot commands" might be better.
>
> > after the "introduced" command, while on the kernel documentation
> > some have and others don't. This suggests both are acceptable,
>
> Yup.
>
> > but in
> > order to avoid confusion, add collons after all the commands to match
> > the reference docs.
>
> Yeah, good idea. I likely would have done this myself soon while doing a
> few other changes I plan, but whatever. :-D
>
> > Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> With the changes above:
>
> Reviewed-by: Thorsten Leemhuis <linux@leemhuis.info>
>
> Side note: I wonder if the commit message could come a bit quicker to
> the point (something along the lines of "Use colons as command
> terminator everywhere for consistency, even if it not strictly
> necessary. That way it will also match regzbot's reference
> documentation.". But not really important I guess. Up to John.
Yep, all great suggestions, thanks. Will apply them for v2.
Thanks,
Nícolas
Thx for this! On 08.03.24 15:10, Nícolas F. R. A. Prado wrote: > On the reference documentation for regzbot, the fixed-by command has > been renamed to fix. Update the kernel documentation accordingly. > > Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md > Link: https://gitlab.com/knurd42/regzbot/-/commit/6d8d30f6bda84e1b711121bb98a07a464d3f089a > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> Reviewed-by: Thorsten Leemhuis <linux@leemhuis.info> > --- > Documentation/process/handling-regressions.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst > index 42b13f77b019..ce6753a674f3 100644 > --- a/Documentation/process/handling-regressions.rst > +++ b/Documentation/process/handling-regressions.rst > @@ -432,7 +432,7 @@ or itself is a reply to that mail: > * Mark a regression as fixed by a commit that is heading upstream or already > landed:: > > - #regzbot fixed-by: 1f2e3d4c5d > + #regzbot fix: 1f2e3d4c5d > > * Mark a regression as a duplicate of another one already tracked by regzbot:: > >
Thx for this! On 08.03.24 15:09, Nícolas F. R. A. Prado wrote: > All the examples in the reference documentation for regzbot have a > collon s/collon/colon/ here and a few lines below as well. And in the subject as well. Speaking of which: something like "docs: *-regressions.rst: add colon to regzbot commands" might be better. > after the "introduced" command, while on the kernel documentation > some have and others don't. This suggests both are acceptable, Yup. > but in > order to avoid confusion, add collons after all the commands to match > the reference docs. Yeah, good idea. I likely would have done this myself soon while doing a few other changes I plan, but whatever. :-D > Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> With the changes above: Reviewed-by: Thorsten Leemhuis <linux@leemhuis.info> Side note: I wonder if the commit message could come a bit quicker to the point (something along the lines of "Use colons as command terminator everywhere for consistency, even if it not strictly necessary. That way it will also match regzbot's reference documentation.". But not really important I guess. Up to John. Ciao, Thorsten > --- > Documentation/admin-guide/reporting-regressions.rst | 2 +- > Documentation/process/handling-regressions.rst | 10 +++++----- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/admin-guide/reporting-regressions.rst b/Documentation/admin-guide/reporting-regressions.rst > index d8adccdae23f..76b246ecf21b 100644 > --- a/Documentation/admin-guide/reporting-regressions.rst > +++ b/Documentation/admin-guide/reporting-regressions.rst > @@ -31,7 +31,7 @@ The important bits (aka "TL;DR") > Linux kernel regression tracking bot "regzbot" track the issue by specifying > when the regression started like this:: > > - #regzbot introduced v5.13..v5.14-rc1 > + #regzbot introduced: v5.13..v5.14-rc1 > > > All the details on Linux kernel regressions relevant for users > diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst > index 5d3c3de3f4ec..42b13f77b019 100644 > --- a/Documentation/process/handling-regressions.rst > +++ b/Documentation/process/handling-regressions.rst > @@ -27,11 +27,11 @@ The important bits (aka "The TL;DR") > is optional, but recommended): > > * For mailed reports, check if the reporter included a line like ``#regzbot > - introduced v5.13..v5.14-rc1``. If not, send a reply (with the regressions > + introduced: v5.13..v5.14-rc1``. If not, send a reply (with the regressions > list in CC) containing a paragraph like the following, which tells regzbot > when the issue started to happen:: > > - #regzbot ^introduced 1f2e3d4c5b6a > + #regzbot ^introduced: 1f2e3d4c5b6a > > * When forwarding reports from a bug tracker to the regressions list (see > above), include a paragraph like the following:: > @@ -79,7 +79,7 @@ When doing either, consider making the Linux kernel regression tracking bot > "regzbot" immediately start tracking the issue: > > * For mailed reports, check if the reporter included a "regzbot command" like > - ``#regzbot introduced 1f2e3d4c5b6a``. If not, send a reply (with the > + ``#regzbot introduced: 1f2e3d4c5b6a``. If not, send a reply (with the > regressions list in CC) with a paragraph like the following::: > > #regzbot ^introduced: v5.13..v5.14-rc1 > @@ -398,9 +398,9 @@ By using a 'regzbot command' in a direct or indirect reply to the mail with the > regression report. These commands need to be in their own paragraph (IOW: they > need to be separated from the rest of the mail using blank lines). > > -One such command is ``#regzbot introduced <version or commit>``, which makes > +One such command is ``#regzbot introduced: <version or commit>``, which makes > regzbot consider your mail as a regressions report added to the tracking, as > -already described above; ``#regzbot ^introduced <version or commit>`` is another > +already described above; ``#regzbot ^introduced: <version or commit>`` is another > such command, which makes regzbot consider the parent mail as a report for a > regression which it starts to track. > >
[-- Attachment #1: Type: text/plain, Size: 3553 bytes --] Hi Nicolas, On Thu, Mar 07, 2024 at 01:05:12PM -0500, Nicolas Dufresne wrote: > Le jeudi 29 février 2024 à 10:02 +0100, Maxime Ripard a écrit : > > On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote: > > > This patch introduces a `.gitlab-ci` file along with a `ci/` folder, > > > defininga basic test pipeline triggered by code pushes to a GitLab-CI > > > instance. This initial version includes static checks (checkpatch and > > > smatch for now) and build tests across various architectures and > > > configurations. It leverages an integrated cache for efficient build > > > times and introduces a flexible 'scenarios' mechanism for > > > subsystem-specific extensions. > > > > > > [ci: add prerequisites to run check-patch on MRs] > > > Co-developed-by: Tales Aparecida <tales.aparecida@redhat.com> > > > Signed-off-by: Tales Aparecida <tales.aparecida@redhat.com> > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > --- > > > > > > Hey all, > > > > > > You can check the validation of this patchset on: > > > https://gitlab.collabora.com/koike/linux/-/pipelines/87035 > > > > > > I would appreciate your feedback on this work, what do you think? > > > > > > If you would rate from 0 to 5, where: > > > > > > [ ] 0. I don't think this is useful at all, and I doubt it will ever be. It doesn't seem worthwhile. > > > [ ] 1. I don't find it useful in its current form. > > > [ ] 2. It might be useful to others, but not for me. > > > [ ] 3. It has potential, but it's not yet something I can incorporate into my workflow. > > > [ ] 4. This is useful, but it needs some adjustments before I can include it in my workflow. > > > [ ] 5. This is really useful! I'm eager to start using it right away. Why didn't you send this earlier? :) > > > > > > Which rating would you select? > > > > 4.5 :) > > > > One thing I'm wondering here is how we're going to cope with the > > different requirements each user / framework has. > > > > Like, Linus probably want to have a different set of CI before merging a > > PR than (say) linux-next does, or stable, or before doing an actual > > release. > > > > Similarly, DRM probably has a different set of requirements than > > drm-misc, drm-amd or nouveau. > > > > I don't see how the current architecture could accomodate for that. I > > know that Gitlab allows to store issues template in a separate repo, > > maybe we could ask them to provide a feature where the actions would be > > separate from the main repo? That way, any gitlab project could provide > > its own set of tests, without conflicting with each others (and we could > > still share them if we wanted to) > > > > I know some of use had good relationship with Gitlab, so maybe it would > > be worth asking? > > As agreed, the .gitlab-ci.yaml file at the list will go away. Its a default > location, but not a required location. This way, each sub-system can have their > own (or not have one). The different sub-system forks will have to be configured > to point to their respective CI main configuration. > > Of course nothing prevents having common set of configuration for jobs and jobs > template. As an example, we could have a job template common for checkpatch, and > allow each subsystem adding their own sauce on top. It can save the duplicate > effort of parsing the tool results and reporting it in a format gitlab > understand. That makes total sense to me and would be incredibly useful indeed. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --]
On Fri, Mar 8, 2024 at 12:55 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Masahiro, > > On Thu, 22 Feb 2024 at 01:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sat, Feb 3, 2024 at 2:30 AM Simon Glass <sjg@chromium.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@chromium.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@chromium.org> > > > +L: linux-arm-kernel@lists.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@arm.com> > > > M: Will Deacon <will@kernel.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@chromium.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 > > > > > > > > > > > > > + else: > > > + add_file(path) > > > + > > > + finish_fit(fsw, entries) > > > + > > > + # Include the kernel itself in the returned file count > > > + return fsw.as_fdt().as_bytearray(), seq + 1, size > > > + > > > + > > > +def run_make_fit(): > > > + """Run the tool's main logic""" > > > + args = parse_args() > > > + > > > + out_data, count, size = build_fit(args) > > > + with open(args.fit, 'wb') as outf: > > > + outf.write(out_data) > > > + > > > + ext_fit_size = None > > > + if args.external: > > > + mkimage = os.environ.get('MKIMAGE', 'mkimage') > > > + subprocess.check_call([mkimage, '-E', '-F', args.fit], > > > + stdout=subprocess.DEVNULL) > > > + > > > + with open(args.fit, 'rb') as inf: > > > + data = inf.read() > > > + ext_fit = libfdt.FdtRo(data) > > > + ext_fit_size = ext_fit.totalsize() > > > > > > > > I still do not understand why mkimage is needed. > > > > > > When external data is used, you can insert "data-size" > > and "data-offset" to the dt structure, > > and at the same time, concatenate the payload data. > > Finally, you can combine the two. > > Is it complex to implement? > > Yes it is somewhat complex, since there are options like alignment and > the like which are implemented in that tool. So, do you mean you need to pass not only -E, -F but also more options to mkimage? > Is there a problem with > using the tool? After all, people are using it today and will be > expecting this script to use it, I believe. If we start adding mkimage > functionality here then we will have to maintain it in two places and > worry about whether they are consistent. Also if we want to support > signatures, etc. then it would be quite an undertaking to write all > that code again in Python. Not a problem, but I think it is unfortunate. I thought this script would be a standalone, self-contained tool to produce a FIT image. > > > > > > > > > > > > > > > + > > > + comp_size = len(out_data) > > > + print(f'FIT size {comp_size:#x}/{comp_size / 1024 / 1024:.1f} MB', end='') > > > + if ext_fit_size: > > > + print(f', header {ext_fit_size:#x}/{ext_fit_size / 1024:.1f} KB', end='') > > > + print(f', {count} files, uncompressed {size / 1024 / 1024:.1f} MB') > > > > > > Maybe, you can print this only when args.verbose == True ? > > This and other comments seem OK to me and I will address them in v11. > > > > > > > > > > > > > > > > > At last, I still do not know how to distinguish the nodes > > when the compatible strings are the same. > > It is like trying to distinguish two devices with the same compatible > string. Really, it should not be done. If you would like me to change > the script in some way, please suggest something. I cannot suggest anything at this moment. I believe we need a way to distinguish two devices, but I do not see any consensus. -- Best Regards Masahiro Yamada
On the reference documentation for regzbot, the fixed-by command has been renamed to fix. Update the kernel documentation accordingly. Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md Link: https://gitlab.com/knurd42/regzbot/-/commit/6d8d30f6bda84e1b711121bb98a07a464d3f089a Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Documentation/process/handling-regressions.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst index 42b13f77b019..ce6753a674f3 100644 --- a/Documentation/process/handling-regressions.rst +++ b/Documentation/process/handling-regressions.rst @@ -432,7 +432,7 @@ or itself is a reply to that mail: * Mark a regression as fixed by a commit that is heading upstream or already landed:: - #regzbot fixed-by: 1f2e3d4c5d + #regzbot fix: 1f2e3d4c5d * Mark a regression as a duplicate of another one already tracked by regzbot:: -- 2.44.0
All the examples in the reference documentation for regzbot have a collon after the "introduced" command, while on the kernel documentation some have and others don't. This suggests both are acceptable, but in order to avoid confusion, add collons after all the commands to match the reference docs. Link: https://gitlab.com/knurd42/regzbot/-/blob/main/docs/reference.md Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Documentation/admin-guide/reporting-regressions.rst | 2 +- Documentation/process/handling-regressions.rst | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/admin-guide/reporting-regressions.rst b/Documentation/admin-guide/reporting-regressions.rst index d8adccdae23f..76b246ecf21b 100644 --- a/Documentation/admin-guide/reporting-regressions.rst +++ b/Documentation/admin-guide/reporting-regressions.rst @@ -31,7 +31,7 @@ The important bits (aka "TL;DR") Linux kernel regression tracking bot "regzbot" track the issue by specifying when the regression started like this:: - #regzbot introduced v5.13..v5.14-rc1 + #regzbot introduced: v5.13..v5.14-rc1 All the details on Linux kernel regressions relevant for users diff --git a/Documentation/process/handling-regressions.rst b/Documentation/process/handling-regressions.rst index 5d3c3de3f4ec..42b13f77b019 100644 --- a/Documentation/process/handling-regressions.rst +++ b/Documentation/process/handling-regressions.rst @@ -27,11 +27,11 @@ The important bits (aka "The TL;DR") is optional, but recommended): * For mailed reports, check if the reporter included a line like ``#regzbot - introduced v5.13..v5.14-rc1``. If not, send a reply (with the regressions + introduced: v5.13..v5.14-rc1``. If not, send a reply (with the regressions list in CC) containing a paragraph like the following, which tells regzbot when the issue started to happen:: - #regzbot ^introduced 1f2e3d4c5b6a + #regzbot ^introduced: 1f2e3d4c5b6a * When forwarding reports from a bug tracker to the regressions list (see above), include a paragraph like the following:: @@ -79,7 +79,7 @@ When doing either, consider making the Linux kernel regression tracking bot "regzbot" immediately start tracking the issue: * For mailed reports, check if the reporter included a "regzbot command" like - ``#regzbot introduced 1f2e3d4c5b6a``. If not, send a reply (with the + ``#regzbot introduced: 1f2e3d4c5b6a``. If not, send a reply (with the regressions list in CC) with a paragraph like the following::: #regzbot ^introduced: v5.13..v5.14-rc1 @@ -398,9 +398,9 @@ By using a 'regzbot command' in a direct or indirect reply to the mail with the regression report. These commands need to be in their own paragraph (IOW: they need to be separated from the rest of the mail using blank lines). -One such command is ``#regzbot introduced <version or commit>``, which makes +One such command is ``#regzbot introduced: <version or commit>``, which makes regzbot consider your mail as a regressions report added to the tracking, as -already described above; ``#regzbot ^introduced <version or commit>`` is another +already described above; ``#regzbot ^introduced: <version or commit>`` is another such command, which makes regzbot consider the parent mail as a report for a regression which it starts to track. -- 2.44.0
A couple tweaks to the commands in the regression documentation to make them up-to-date and less confusing. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- Nícolas F. R. A. Prado (2): docs: *-regressions.rst: Use collon after regzbot introduced command docs: handling-regressions.rst: Update regzbot command fixed-by to fix Documentation/admin-guide/reporting-regressions.rst | 2 +- Documentation/process/handling-regressions.rst | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) --- base-commit: 11afac187274a6177a7ac82997f8691c0f469e41 change-id: 20240308-regzbot-fixes-43a28fd4b621 Best regards, -- Nícolas F. R. A. Prado <nfraprado@collabora.com>
Hi Masahiro, On Thu, 22 Feb 2024 at 01:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sat, Feb 3, 2024 at 2:30 AM Simon Glass <sjg@chromium.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@chromium.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@chromium.org> > > +L: linux-arm-kernel@lists.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@arm.com> > > M: Will Deacon <will@kernel.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@chromium.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 > > > > > > > + else: > > + add_file(path) > > + > > + finish_fit(fsw, entries) > > + > > + # Include the kernel itself in the returned file count > > + return fsw.as_fdt().as_bytearray(), seq + 1, size > > + > > + > > +def run_make_fit(): > > + """Run the tool's main logic""" > > + args = parse_args() > > + > > + out_data, count, size = build_fit(args) > > + with open(args.fit, 'wb') as outf: > > + outf.write(out_data) > > + > > + ext_fit_size = None > > + if args.external: > > + mkimage = os.environ.get('MKIMAGE', 'mkimage') > > + subprocess.check_call([mkimage, '-E', '-F', args.fit], > > + stdout=subprocess.DEVNULL) > > + > > + with open(args.fit, 'rb') as inf: > > + data = inf.read() > > + ext_fit = libfdt.FdtRo(data) > > + ext_fit_size = ext_fit.totalsize() > > > > I still do not understand why mkimage is needed. > > > When external data is used, you can insert "data-size" > and "data-offset" to the dt structure, > and at the same time, concatenate the payload data. > Finally, you can combine the two. > Is it complex to implement? Yes it is somewhat complex, since there are options like alignment and the like which are implemented in that tool. Is there a problem with using the tool? After all, people are using it today and will be expecting this script to use it, I believe. If we start adding mkimage functionality here then we will have to maintain it in two places and worry about whether they are consistent. Also if we want to support signatures, etc. then it would be quite an undertaking to write all that code again in Python. > > > > > > > > + > > + comp_size = len(out_data) > > + print(f'FIT size {comp_size:#x}/{comp_size / 1024 / 1024:.1f} MB', end='') > > + if ext_fit_size: > > + print(f', header {ext_fit_size:#x}/{ext_fit_size / 1024:.1f} KB', end='') > > + print(f', {count} files, uncompressed {size / 1024 / 1024:.1f} MB') > > > Maybe, you can print this only when args.verbose == True ? This and other comments seem OK to me and I will address them in v11. > > > > > > > > At last, I still do not know how to distinguish the nodes > when the compatible strings are the same. It is like trying to distinguish two devices with the same compatible string. Really, it should not be done. If you would like me to change the script in some way, please suggest something. > > > > > > > + > > +if __name__ == "__main__": > > + sys.exit(run_make_fit()) > > -- > > 2.34.1 > > > Regards, Simon
Hi Masahiro,
On Sun, 25 Feb 2024 at 21:41, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Feb 21, 2024 at 9:37 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Sat, Feb 3, 2024 at 2:30 AM Simon Glass <sjg@chromium.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@chromium.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@chromium.org>
> > > +L: linux-arm-kernel@lists.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@arm.com>
> > > M: Will Deacon <will@kernel.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@chromium.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.
No, I am fine with the @ thing. I didn't know it was built into argparse
Regards,
Simon
On Mon, 2024-03-04 at 18:45 -0300, Helen Koike wrote: > Hi Linus, > > Thank you for your reply and valuable inputs. > > On 01/03/2024 17:10, Linus Torvalds wrote: > > On Fri, 1 Mar 2024 at 02:27, Nikolai Kondrashov <spbnick@gmail.com> wrote: > > > > > > I agree, it's hard to imagine even a simple majority agreeing on how GitLab CI > > > should be done. Still, we would like to help people, who are interested in > > > this kind of thing, to set it up. How about we reframe this contribution as a > > > sort of template, or a reference for people to start their setup with, > > > assuming that most maintainers would want to tweak it? We would also be glad > > > to stand by for questions and help, as people try to use it. > > > > Ack. I think seeing it as a library for various gitlab CI models would > > be a lot more palatable. Particularly if you can then show that yes, > > it is also relevant to our currently existing drm case. > > Having it as a library would certainly make my work as the DRM-CI > maintainer easier and also simplify the process whenever we consider > integrating tests into other subsystems. > > > > > So I'm not objecting to having (for example) some kind of CI helper > > templates - I think a logical place would be in tools/ci/ which is > > kind of alongside our tools/testing subdirectory. > > Works for me. > > We can skip having a default .gitlab-ci.yml in the root directory and > instead include clear instructions in our documentation for using these > templates. From previous experience[1], I recommend this approach. This way it does not bother current Gitlab mirrors / personal repos, while allowing anyone to setup the CI from Gitlab menus just by changing: Repo -> Settings -> CI/CD -> General Pipelines -> CI/CD configuration file Thanks! Leo [1] Last year I implemented Gitlab-CI for the perfbook repo, and I came across some issues, including the disruption of .gitlab-ci.yml in the root of a repo. https://lore.kernel.org/perfbook/20230201201529.901316-1-leobras.c@gmail.com/ > > Thanks, > Helen Koike > > > > > (And then perhaps have a 'gitlab' directory under that. I'm not sure > > whether - and how much - commonality there might be between the > > different CI models of different hosts). > > > > Just to clarify: when I say "a logical place", I very much want to > > emphasize the "a" - maybe there are better places, and I'm not saying > > that is the only possible place. But it sounds more logical to me than > > some. > > > > Linus
Le jeudi 29 février 2024 à 10:02 +0100, Maxime Ripard a écrit :
> Hi Helen,
>
> Thanks for working on this
>
> On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote:
> > This patch introduces a `.gitlab-ci` file along with a `ci/` folder,
> > defininga basic test pipeline triggered by code pushes to a GitLab-CI
> > instance. This initial version includes static checks (checkpatch and
> > smatch for now) and build tests across various architectures and
> > configurations. It leverages an integrated cache for efficient build
> > times and introduces a flexible 'scenarios' mechanism for
> > subsystem-specific extensions.
> >
> > [ci: add prerequisites to run check-patch on MRs]
> > Co-developed-by: Tales Aparecida <tales.aparecida@redhat.com>
> > Signed-off-by: Tales Aparecida <tales.aparecida@redhat.com>
> > Signed-off-by: Helen Koike <helen.koike@collabora.com>
> >
> > ---
> >
> > Hey all,
> >
> > You can check the validation of this patchset on:
> > https://gitlab.collabora.com/koike/linux/-/pipelines/87035
> >
> > I would appreciate your feedback on this work, what do you think?
> >
> > If you would rate from 0 to 5, where:
> >
> > [ ] 0. I don't think this is useful at all, and I doubt it will ever be. It doesn't seem worthwhile.
> > [ ] 1. I don't find it useful in its current form.
> > [ ] 2. It might be useful to others, but not for me.
> > [ ] 3. It has potential, but it's not yet something I can incorporate into my workflow.
> > [ ] 4. This is useful, but it needs some adjustments before I can include it in my workflow.
> > [ ] 5. This is really useful! I'm eager to start using it right away. Why didn't you send this earlier? :)
> >
> > Which rating would you select?
>
> 4.5 :)
>
> One thing I'm wondering here is how we're going to cope with the
> different requirements each user / framework has.
>
> Like, Linus probably want to have a different set of CI before merging a
> PR than (say) linux-next does, or stable, or before doing an actual
> release.
>
> Similarly, DRM probably has a different set of requirements than
> drm-misc, drm-amd or nouveau.
>
> I don't see how the current architecture could accomodate for that. I
> know that Gitlab allows to store issues template in a separate repo,
> maybe we could ask them to provide a feature where the actions would be
> separate from the main repo? That way, any gitlab project could provide
> its own set of tests, without conflicting with each others (and we could
> still share them if we wanted to)
>
> I know some of use had good relationship with Gitlab, so maybe it would
> be worth asking?
As agreed, the .gitlab-ci.yaml file at the list will go away. Its a default
location, but not a required location. This way, each sub-system can have their
own (or not have one). The different sub-system forks will have to be configured
to point to their respective CI main configuration.
Of course nothing prevents having common set of configuration for jobs and jobs
template. As an example, we could have a job template common for checkpatch, and
allow each subsystem adding their own sauce on top. It can save the duplicate
effort of parsing the tool results and reporting it in a format gitlab
understand.
This also allow subsystems to offer different coverage, e.g. a fast vs a full
coverage. Or perhaps a configuration to focus on specific devices. But in
general, just not having a central config is enough to support the idea. What we
have now could be entirely drm specific and "commonized" later when other
subsystem wanting to use gitlab joins (Linux Media is among those).
Nicolas
On 05.03.24 13:59, Jonathan Corbet wrote:
> Thorsten Leemhuis <linux@leemhuis.info> writes:
>> On 03.03.24 17:31, Jonathan Corbet wrote:
>>> Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:
>>>> I wanted to clean up the development-process documentation. There is
>>>> however no easy way to break the ice here:
>>>>
>>>> The elephant in the room is that there is some unclear relation between
>>>> 5.Posting.rst, 6.Followthrough.rst and submitting-patches.rst.
>>>> (Yes, I know each document has its own history...; but let us put the
>>>> history aside for now.)
>>>
>>> FWIW, the objective of those two documents is quite different; one is a
>>> high-level overview of how the development process as a whole works, the
>>> other is a detailed guide to submitting work for consideration.
>>
>> Sorry, I'm slightly confused here, so I have to ask: which is which?
>>
>> Due to the "*essential*" in the headline of submitting-patches.rst and
>> its "For *detailed* information on how the kernel development process
>> works, see Documentation/process/development-process.rst" in the intro
>> make it sounds to me like submitting-patches.rst should be the one with
>> the high-level overview. But...
>>
>>> Again, let's remember the different purposes of these documents. The
>>> development-process document is an overall description of the process,
>>> so it doesn't need the details.
>>
>> ...this makes it sounds like you consider it the other way around. And
>> for me that feels the wrong, as why describe the overall process in
>> detail, but leave the most important part of the process to some other
>> document?
>>
>> /me wonders what he is missing
>
> The series of files starting with Documentation/process/1.Intro.rst was
> meant to describe the whole of the development process to a wider
> audience; I originally wrote it as a project for the Linux Foundation.
> It covers far more than the business of putting up patches for
> consideration - development cycles and all that.
>
> submitting-patches.rst, instead, covers the details of getting code
> considered for merging; it is intended to be read by the people actually
> trying to do that work.
>
> One document describes what the pieces of the car are and how they work
> together to get you to the pub. The other gives all of the steps for
> working on the brakes without causing accidents. They both fit as part
> of a larger body of documentation, but they are definitely not the same
> document.
Thx for the clarification. And of course both fit in a larger body. It
seems some of the word used in the intros of both documents made me
assume it was the other way around at some point in the past and then it
stuck. Wondering if that's just me or if that happened to others as
well. Whatever, if Lukas will realize his plans I guess the different
target audiences will become more obvious over time.
Thx again! Ciao, Thorsten
Thorsten Leemhuis <linux@leemhuis.info> writes:
> On 03.03.24 17:31, Jonathan Corbet wrote:
>> Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:
>>> I wanted to clean up the development-process documentation. There is
>>> however no easy way to break the ice here:
>>>
>>> The elephant in the room is that there is some unclear relation between
>>> 5.Posting.rst, 6.Followthrough.rst and submitting-patches.rst.
>>> (Yes, I know each document has its own history...; but let us put the
>>> history aside for now.)
>>
>> FWIW, the objective of those two documents is quite different; one is a
>> high-level overview of how the development process as a whole works, the
>> other is a detailed guide to submitting work for consideration.
>
> Sorry, I'm slightly confused here, so I have to ask: which is which?
>
> Due to the "*essential*" in the headline of submitting-patches.rst and
> its "For *detailed* information on how the kernel development process
> works, see Documentation/process/development-process.rst" in the intro
> make it sounds to me like submitting-patches.rst should be the one with
> the high-level overview. But...
>
>> Again, let's remember the different purposes of these documents. The
>> development-process document is an overall description of the process,
>> so it doesn't need the details.
>
> ...this makes it sounds like you consider it the other way around. And
> for me that feels the wrong, as why describe the overall process in
> detail, but leave the most important part of the process to some other
> document?
>
> /me wonders what he is missing
The series of files starting with Documentation/process/1.Intro.rst was
meant to describe the whole of the development process to a wider
audience; I originally wrote it as a project for the Linux Foundation.
It covers far more than the business of putting up patches for
consideration - development cycles and all that.
submitting-patches.rst, instead, covers the details of getting code
considered for merging; it is intended to be read by the people actually
trying to do that work.
One document describes what the pieces of the car are and how they work
together to get you to the pub. The other gives all of the steps for
working on the brakes without causing accidents. They both fit as part
of a larger body of documentation, but they are definitely not the same
document.
Make sense?
jon
On Sun, Mar 3, 2024 at 5:31 PM Jonathan Corbet <corbet@lwn.net> wrote: > > Lukas Bulwahn <lukas.bulwahn@gmail.com> writes: > > > Dear Jonathan, > > > > I wanted to clean up the development-process documentation. There is > > however no easy way to break the ice here: > > > > The elephant in the room is that there is some unclear relation between > > 5.Posting.rst, 6.Followthrough.rst and submitting-patches.rst. > > (Yes, I know each document has its own history...; but let us put the > > history aside for now.) > > FWIW, the objective of those two documents is quite different; one is a > high-level overview of how the development process as a whole works, the > other is a detailed guide to submitting work for consideration. > Yes, that _objective_ is clear when reading the documents. However, unfortunately, the detailed guide to submitting work for consideration in submitting-patches.rst really is not that much more detailed than what 5.Posting and 6.Followthrough already recommend. A lot of the "details" in submitting-patches.rst is then also just details on topics that are much more an explanation than actual recommendation for specific actions. Let me clean things up in submitting-patches, and then start a proper comparison. > > Submitting-patches.rst contains information largely put together from > > different initial starting points and is partly outdated due to common > > workflows with git format-patch and git send-email. > > You should have seen it before I thrashed it a few years back :) > > > For a simple experiment, I moved the larger parts on the tags > > (signed-off-by, co-developed-by, acked-by, reported-by, etc.) into a > > separate document and then ran the numbers on submitting-patches again: > > > > 4329 submitting-patches.rst > > > > Nowt, the size of submitting-patches is actually below Posting and > > Followthrough. > > I don't think we should be driven by word counts. I do think that > moving a bunch of information on tags to its own document could make > sense. > > > So, the difficult task to reach a coherent process description is to see > > some relation between these documents and then go through the editorial > > changes. I have come up with this kind of vision: > > > > Phase 1: Clean up submitting patches > > > > Topics/Statements that can be easily cleaned up first do not get in > > the way (at least mentally) when trying to understand the next steps. > > > > E.g., as an experiment I moved the details on tags into a separate > > document. > > Fine. > > > Phase 2: Make submitting-patches have one clear temporal flow. > > > > The top-level structure should basically be along the temporal order of > > things: Prepare a patch, Post a patch, Respond to review, Send reworked > > patches, Be patient before resending > > This makes sense as well. I wonder if splitting the document along some > of those lines might also be a good idea, with submitting-patches.rst > becoming a relatively short overview deferring details to the others. > This is one of the most important docs we have, and it's far too much > for people to engage with all at once. > I understand that people nowadays do not read prose from top to bottom, as soon as it exceeds a certain length. So, for sure, we can consider splitting the current content into multiple pieces and add links between them. However, I also want to avoid that we have say 15 documents of a hundred lines, and you are always jumping back-and-forth in your web browser while reading. I think the split is going to be into two or three documents if at all. I will do some experiments and suggest some splitting. > > Phase 3: Merge the pieces of content from Posting and Followthrough into > > submitting patches if it adds something to that document. > > > > When both documents roughly cover the topics of similar depth, we look > > fine-grained into how to construct the one document that has the best > > from both documents. > > > > Phase 4: Remove Posting and Followthrough and simply replace it in the > > process description with submitting patches. > > In broad terms, this seems like a good direction to me. > > Again, let's remember the different purposes of these documents. The > development-process document is an overall description of the process, > so it doesn't need the details. But when you say: > > > Posting will not be missed. > > I don't entirely agree. But I don't doubt it could be a fraction of > what it is now. > When I say "Posting will not be missed", I mean the name "5.Posting.rst" will not be missed, as the future submitting-patches, partially existent on my hard disk right now, includes the best of 5.Posting.rst as it is now, namely the natural flow of the explanation, the good style of writing, being precise and concise and the ability to address all audiences with a suitable text, e.g., newcomers and experienced kernel developers enjoy reading it. Some important information in 5.Posting.rst should really also be mentioned in submitting-patches.rst. I think if submitting-patches.rst is structured and written well, the development process description can go from 4. Getting the code right to "5.Submitting patches" and the readers would not even notice that they once originated from very different sources and authors. > > So, here are some first changes to Phase 1 and Phase 2. > > At a first glance, these changes seem fine. I think I'll hold them > until after the merge window so that others can think about what you're > up to, but I suspect there will be no reason not to apply this first set > then. > > Thanks for working on this material; it's some of the most important we > have and it definitely needs some attention. > I will continue working on it and see what I consider stable enough in moving around that it deserves to be posted to the mailing list. While working on the document, it is unfortunately a lot of temporary movement back and forth, or huge changes at once and it is a bit difficult to then extract the next natural change to propose, but I will see how I can present this best piece by piece. Lukas
On 2024-02-29 21:21, Linus Torvalds wrote: > On Thu, 29 Feb 2024 at 01:23, Nikolai Kondrashov <spbnick@gmail.com> wrote: >> >> However, I think a better approach would be *not* to add the .gitlab-ci.yaml >> file in the root of the source tree, but instead change the very same repo >> setting to point to a particular entry YAML, *inside* the repo (somewhere >> under "ci" directory) instead. > > I really don't want some kind of top-level CI for the base kernel project. > > We already have the situation that the drm people have their own ci > model. II'm ok with that, partly because then at least the maintainers > of that subsystem can agree on the rules for that one subsystem. > > I'm not at all interested in having something that people will then > either fight about, or - more likely - ignore, at the top level > because there isn't some global agreement about what the rules are. > > For example, even just running checkpatch is often a stylistic thing, > and not everybody agrees about all the checkpatch warnings. > > I would suggest the CI project be separate from the kernel. That would be missing a lot of the point / benefit of CI. A CI system which is separate from the kernel will tend to be out of sync, so it can't gate the merging of changes and thus can't prevent regressions from propagating. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
On 03.03.24 17:31, Jonathan Corbet wrote: > Lukas Bulwahn <lukas.bulwahn@gmail.com> writes: >> I wanted to clean up the development-process documentation. There is >> however no easy way to break the ice here: >> >> The elephant in the room is that there is some unclear relation between >> 5.Posting.rst, 6.Followthrough.rst and submitting-patches.rst. >> (Yes, I know each document has its own history...; but let us put the >> history aside for now.) > > FWIW, the objective of those two documents is quite different; one is a > high-level overview of how the development process as a whole works, the > other is a detailed guide to submitting work for consideration. Sorry, I'm slightly confused here, so I have to ask: which is which? Due to the "*essential*" in the headline of submitting-patches.rst and its "For *detailed* information on how the kernel development process works, see Documentation/process/development-process.rst" in the intro make it sounds to me like submitting-patches.rst should be the one with the high-level overview. But... > Again, let's remember the different purposes of these documents. The > development-process document is an overall description of the process, > so it doesn't need the details. ...this makes it sounds like you consider it the other way around. And for me that feels the wrong, as why describe the overall process in detail, but leave the most important part of the process to some other document? /me wonders what he is missing Ciao, Thorsten