The linux-firmware packages on Gentoo, Fedora, Arch, and others compress
the firmware files. This works well with CONFIG_FW_LOADER_COMPRESS, but
does not work with CONFIG_EXTRA_FIRMWARE. This patch allows the build
system to decompress firmware files specified by CONFIG_EXTRA_FIRMWARE.
PATCH 1/2 adds decompression routines next to the compression routines
in scripts/Makefile.lib. That patch is then used by PATCH 2/2 to
decompress files before compiling them into the kernel.
The patch works by copying or decompressing the specified firmware files
into the build directory, then compiling them in from there. I would
prefer to not copy any uncompressed files, but I have not found a clean
way to do that.
Kevin Martin (2):
kbuild: Enable decompression for use by EXTRA_FIRMWARE The build
system can currently only compress files. This patch adds the
functionality to decompress files. Decompression is needed for
building firmware files into the kernel if those files are
compressed on the filesystem. Compressed firmware files are in use
by Gentoo, Fedora, Arch, and others.
firmware_loader: Enable compressed files with EXTRA_FIRMWARE The
linux-firmware packages on Gentoo, Fedora, Arch, and others compress
the firmware files. This works well with CONFIG_FW_LOADER_COMPRESS
but does not work with CONFIG_EXTRA_FIRMWARE. This patch allows the
build system to decompress firmware files specified by
CONFIG_EXTRA_FIRMWARE. Uncompressed files are used first, then the
compressed files are used.
drivers/base/firmware_loader/Kconfig | 5 ++++-
drivers/base/firmware_loader/builtin/Makefile | 16 ++++++++++++----
scripts/Makefile.lib | 6 ++++++
3 files changed, 22 insertions(+), 5 deletions(-)
--
2.41.0
The linux-firmware packages on Gentoo, Fedora, Arch, and others compress
the firmware files. This works well with CONFIG_FW_LOADER_COMPRESS, but
does not work with CONFIG_EXTRA_FIRMWARE. This patch allows the build
system to decompress firmware files specified by CONFIG_EXTRA_FIRMWARE.
Uncompressed files are used first, then the compressed files are used.
Signed-off-by: Kevin Martin <[email protected]>
---
drivers/base/firmware_loader/Kconfig | 5 ++++-
drivers/base/firmware_loader/builtin/Makefile | 16 ++++++++++++----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 5ca00e02f..b7a908bff 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -76,7 +76,10 @@ config EXTRA_FIRMWARE
image since it combines both GPL and non-GPL work. You should
consult a lawyer of your own before distributing such an image.
- NOTE: Compressed files are not supported in EXTRA_FIRMWARE.
+ NOTE: Compressed files are supported by EXTRA_FIRMWARE. The build
+ system will look for uncompressed files first then fall back to
+ searching for compressed files in a similar way to
+ CONFIG_FW_LOADER_COMPRESS.
config EXTRA_FIRMWARE_DIR
string "Firmware blobs root directory"
diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 6c067dedc..cc60eb441 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -20,7 +20,7 @@ filechk_fwbin = \
echo " .section .rodata" ;\
echo " .p2align 4" ;\
echo "_fw_$(FWSTR)_bin:" ;\
- echo " .incbin \"$(fwdir)/$(FWNAME)\"" ;\
+ echo " .incbin \"$(obj)/$(FWNAME)\"" ;\
echo "_fw_end:" ;\
echo " .section .rodata.str,\"aMS\",$(PROGBITS),1" ;\
echo " .p2align $(ASM_ALIGN)" ;\
@@ -36,7 +36,15 @@ $(obj)/%.gen.S: FORCE
$(call filechk,fwbin)
# The .o files depend on the binaries directly; the .S files don't.
-$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
+$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(obj)/%
-targets := $(patsubst $(obj)/%,%, \
- $(shell find $(obj) -name \*.gen.S 2>/dev/null))
+$(obj)/% : $(fwdir)/% FORCE
+ $(call if_changed,copy)
+
+$(obj)/% : $(fwdir)/%.xz FORCE
+ $(call if_changed,xzdec)
+
+$(obj)/% : $(fwdir)/%.zst FORCE
+ $(call if_changed,zstddec)
+
+targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)
--
2.41.0
Signed-off-by: Kevin Martin <[email protected]>
---
scripts/Makefile.lib | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1a965fe68..d043be3dc 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@
quiet_cmd_xzmisc = XZMISC $@
cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
+quiet_cmd_xzdec = XZDEC $@
+ cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@
+
# ZSTD
# ---------------------------------------------------------------------------
# Appends the uncompressed size of the data using size_append. The .zst
@@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@
quiet_cmd_zstd22_with_size = ZSTD22 $@
cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+quiet_cmd_zstddec = ZSTDDEC $@
+ cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@
+
# ASM offsets
# ---------------------------------------------------------------------------
--
2.41.0
Hi Kevin,
> Subject: Re: [PATCH 1/2] kbuild: Enable decompression for use by
> EXTRA_FIRMWARE The build system can currently only compress files. This
> patch adds the functionality to decompress files. Decompression is needed
> for building firmware files into the kernel if those files are compressed on
> the filesystem. Compressed firmware files are in use by Gentoo, Fedora,
> Arch, and others.
patch description is squashed into the subject. Did your tooling
accidentially remove the empty line between?
The patch itself looks good to me.
Tested-by: Nicolas Schier <[email protected]>
Kind regards,
Nicolas
On Wed, Dec 20, 2023 at 05:22:50AM -0500, Kevin Martin wrote:
> Signed-off-by: Kevin Martin <[email protected]>
> ---
> scripts/Makefile.lib | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1a965fe68..d043be3dc 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@
> quiet_cmd_xzmisc = XZMISC $@
> cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
>
> +quiet_cmd_xzdec = XZDEC $@
> + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@
> +
> # ZSTD
> # ---------------------------------------------------------------------------
> # Appends the uncompressed size of the data using size_append. The .zst
> @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@
> quiet_cmd_zstd22_with_size = ZSTD22 $@
> cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
>
> +quiet_cmd_zstddec = ZSTDDEC $@
> + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@
> +
> # ASM offsets
> # ---------------------------------------------------------------------------
>
> --
> 2.41.0
>
On Wed, Dec 20, 2023 at 05:29:33AM -0500, Kevin Martin wrote:
> The linux-firmware packages on Gentoo, Fedora, Arch, and others compress
> the firmware files. This works well with CONFIG_FW_LOADER_COMPRESS, but
> does not work with CONFIG_EXTRA_FIRMWARE. This patch allows the build
> system to decompress firmware files specified by CONFIG_EXTRA_FIRMWARE.
> Uncompressed files are used first, then the compressed files are used.
>
> Signed-off-by: Kevin Martin <[email protected]>
> ---
Tested-by: Nicolas Schier <[email protected]>
On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <[email protected]> wrote:
>
> Signed-off-by: Kevin Martin <[email protected]>
> ---
> scripts/Makefile.lib | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1a965fe68..d043be3dc 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@
> quiet_cmd_xzmisc = XZMISC $@
> cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
>
> +quiet_cmd_xzdec = XZDEC $@
> + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@
> +
Please do not fork the meaningless 'cat' process.
This should be a single process to take just one input file.
cmd_xzdec = $(XZ) --decompress --stdout $< > $@
Commit d3dd3b5a29bb9582957451531fed461628dfc834
was a very bad commit.
The 'cat' and compression/decompression must be
separate rules.
We should not repeat the mistake in the past.
> # ZSTD
> # ---------------------------------------------------------------------------
> # Appends the uncompressed size of the data using size_append. The .zst
> @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@
> quiet_cmd_zstd22_with_size = ZSTD22 $@
> cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
>
> +quiet_cmd_zstddec = ZSTDDEC $@
> + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@
> +
Same here.
Please make this a single process:
cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $<
One small concern in the future is, if we end up with adding
quiet_cmd_bzip2dec, we will run out of the 7-column of the short log.
quiet_cmd_bzip2dec = BZIP2DEC$@
We can increase the column size if needed, so I do not think
it is a big issue.
> # ASM offsets
> # ---------------------------------------------------------------------------
>
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
On Wed, Dec 20, 2023 at 7:33 PM Kevin Martin <[email protected]> wrote:
>
> The linux-firmware packages on Gentoo, Fedora, Arch, and others compress
> the firmware files. This works well with CONFIG_FW_LOADER_COMPRESS, but
> does not work with CONFIG_EXTRA_FIRMWARE. This patch allows the build
> system to decompress firmware files specified by CONFIG_EXTRA_FIRMWARE.
> Uncompressed files are used first, then the compressed files are used.
>
> Signed-off-by: Kevin Martin <[email protected]>
> ---
> drivers/base/firmware_loader/Kconfig | 5 ++++-
> drivers/base/firmware_loader/builtin/Makefile | 16 ++++++++++++----
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 5ca00e02f..b7a908bff 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -76,7 +76,10 @@ config EXTRA_FIRMWARE
> image since it combines both GPL and non-GPL work. You should
> consult a lawyer of your own before distributing such an image.
>
> - NOTE: Compressed files are not supported in EXTRA_FIRMWARE.
> + NOTE: Compressed files are supported by EXTRA_FIRMWARE. The build
> + system will look for uncompressed files first then fall back to
> + searching for compressed files in a similar way to
> + CONFIG_FW_LOADER_COMPRESS.
>
> config EXTRA_FIRMWARE_DIR
> string "Firmware blobs root directory"
> diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
> index 6c067dedc..cc60eb441 100644
> --- a/drivers/base/firmware_loader/builtin/Makefile
> +++ b/drivers/base/firmware_loader/builtin/Makefile
> @@ -20,7 +20,7 @@ filechk_fwbin = \
> echo " .section .rodata" ;\
> echo " .p2align 4" ;\
> echo "_fw_$(FWSTR)_bin:" ;\
> - echo " .incbin \"$(fwdir)/$(FWNAME)\"" ;\
> + echo " .incbin \"$(obj)/$(FWNAME)\"" ;\
Insert a tab to keep the alignment of ";\"
Also, please update the .gitignore because
any arbitrary firmware file might be copied to
this directory.
All files except the 3 check-in files must be
ignored.
@@ -1,2 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-*.gen.S
+*
+!.gitignore
+!Makefile
+!main.c
--
Best Regards
Masahiro Yamada
On Wed, 3 Jan 2024, Masahiro Yamada wrote:
> On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <[email protected]> wrote:
> >
> > Signed-off-by: Kevin Martin <[email protected]>
> > ---
> > scripts/Makefile.lib | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 1a965fe68..d043be3dc 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@
> > quiet_cmd_xzmisc = XZMISC $@
> > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
> >
> > +quiet_cmd_xzdec = XZDEC $@
> > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@
> > +
>
>
>
> Please do not fork the meaningless 'cat' process.
>
> This should be a single process to take just one input file.
>
> cmd_xzdec = $(XZ) --decompress --stdout $< > $@
>
>
>
>
> Commit d3dd3b5a29bb9582957451531fed461628dfc834
> was a very bad commit.
>
> The 'cat' and compression/decompression must be
> separate rules.
>
> We should not repeat the mistake in the past.
>
Would it be preferable to change all of the compression rules or just the
new decompression rules?
I could change just the new ones and then begin working on a different
patch to clean up the 'cat' processes in the compression rules.
>
>
> > # ZSTD
> > # ---------------------------------------------------------------------------
> > # Appends the uncompressed size of the data using size_append. The .zst
> > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@
> > quiet_cmd_zstd22_with_size = ZSTD22 $@
> > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> >
> > +quiet_cmd_zstddec = ZSTDDEC $@
> > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@
> > +
>
>
> Same here.
> Please make this a single process:
>
> cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $<
>
>
>
>
>
>
> One small concern in the future is, if we end up with adding
> quiet_cmd_bzip2dec, we will run out of the 7-column of the short log.
>
> quiet_cmd_bzip2dec = BZIP2DEC$@
>
> We can increase the column size if needed, so I do not think
> it is a big issue.
>
>
>
>
>
>
>
>
>
>
> > # ASM offsets
> > # ---------------------------------------------------------------------------
> >
> > --
> > 2.41.0
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
On Thu, Jan 4, 2024 at 4:04 PM Kevin Martin <[email protected]> wrote:
>
>
> On Wed, 3 Jan 2024, Masahiro Yamada wrote:
>
> > On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <[email protected]> wrote:
> > >
> > > Signed-off-by: Kevin Martin <[email protected]>
> > > ---
> > > scripts/Makefile.lib | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > index 1a965fe68..d043be3dc 100644
> > > --- a/scripts/Makefile.lib
> > > +++ b/scripts/Makefile.lib
> > > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@
> > > quiet_cmd_xzmisc = XZMISC $@
> > > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@
> > >
> > > +quiet_cmd_xzdec = XZDEC $@
> > > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@
> > > +
> >
> >
> >
> > Please do not fork the meaningless 'cat' process.
> >
> > This should be a single process to take just one input file.
> >
> > cmd_xzdec = $(XZ) --decompress --stdout $< > $@
> >
> >
> >
> >
> > Commit d3dd3b5a29bb9582957451531fed461628dfc834
> > was a very bad commit.
> >
> > The 'cat' and compression/decompression must be
> > separate rules.
> >
> > We should not repeat the mistake in the past.
> >
>
> Would it be preferable to change all of the compression rules or just the
> new decompression rules?
I do not require you to change the existing code.
For decompression, it is unlikely that the recipe
takes multiple input files.
So, 'cat' is unneeded.
> I could change just the new ones and then begin working on a different
> patch to clean up the 'cat' processes in the compression rules.
If you get rid of the 'cat', you need to refactor the user code.
arch/x86/boot/compressed/Makefile relies on 'cat' and 'compress',
but please double-check no other Makefile uses it.
Also, you might need some research about the potential
impact onto the reproducible builds.
Without 'cat |', the compressed archive might encode
the timestamp of the original file.
GZIP records the timestamp in the header.
>
> >
> >
> > > # ZSTD
> > > # ---------------------------------------------------------------------------
> > > # Appends the uncompressed size of the data using size_append. The .zst
> > > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@
> > > quiet_cmd_zstd22_with_size = ZSTD22 $@
> > > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> > >
> > > +quiet_cmd_zstddec = ZSTDDEC $@
> > > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@
> > > +
> >
> >
> > Same here.
> > Please make this a single process:
> >
> > cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $<
> >
> >
> >
> >
> >
> >
> > One small concern in the future is, if we end up with adding
> > quiet_cmd_bzip2dec, we will run out of the 7-column of the short log.
> >
> > quiet_cmd_bzip2dec = BZIP2DEC$@
> >
> > We can increase the column size if needed, so I do not think
> > it is a big issue.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > # ASM offsets
> > > # ---------------------------------------------------------------------------
> > >
> > > --
> > > 2.41.0
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
--
Best Regards
Masahiro Yamada