[v1] Enable compressed files in EXTRA_FIRMWARE

[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.

Posted by Kevin Martin 2 years ago

Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
---
 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

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.

Posted by Masahiro Yamada 1 year, 11 months ago

On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
>
> Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> ---
>  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

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.

Posted by Kevin Martin 1 year, 11 months ago

On Wed, 3 Jan 2024, Masahiro Yamada wrote:

> On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
> >
> > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> > ---
> >  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
>

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.

Posted by Masahiro Yamada 1 year, 11 months ago

On Thu, Jan 4, 2024 at 4:04 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
>
>
> On Wed, 3 Jan 2024, Masahiro Yamada wrote:
>
> > On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
> > >
> > > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> > > ---
> > >  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

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.

Posted by Nicolas Schier 1 year, 12 months ago

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 <n.schier@avm.de>

Kind regards,
Nicolas

On Wed, Dec 20, 2023 at 05:22:50AM -0500, Kevin Martin wrote:
> Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> ---
>  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
>