[PATCH 1/5] sh: rename suffix-y to suffix_y

[PATCH 1/5] sh: rename suffix-y to suffix_y

[Masahiro Yamada]

'export suffix-y' does not work reliably because hyphens are disallowed
in shell variables.

A similar issue was fixed by commit 2bfbe7881ee0 ("kbuild: Do not use
hyphen in exported variable name").

If I do similar in dash, ARCH=sh fails to build.

  $ mv linux linux~
  $ cd linux~
  $ dash
  $ make O=foo/bar ARCH=sh CROSS_COMPILE=sh4-linux-gnu- defconfig all
  make[1]: Entering directory '/home/masahiro/linux~/foo/bar'
    [ snip ]
  make[4]: *** No rule to make target 'arch/sh/boot/compressed/vmlinux.bin.', needed by 'arch/sh/boot/compressed/piggy.o'.  Stop.
  make[3]: *** [/home/masahiro/linux~/arch/sh/boot/Makefile:40: arch/sh/boot/compressed/vmlinux] Error 2
  make[2]: *** [/home/masahiro/linux~/arch/sh/Makefile:194: zImage] Error 2
  make[1]: *** [/home/masahiro/linux~/Makefile:350: __build_one_by_one] Error 2
  make[1]: Leaving directory '/home/masahiro/linux~/foo/bar'
  make: *** [Makefile:219: __sub-make] Error 2

The maintainer of GNU Make stated that there is no consistent way to
export variables that do not meet the shell's naming criteria.
(https://savannah.gnu.org/bugs/?55719)

Consequently, you cannot use hyphens in exported variables.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/sh/boot/Makefile            | 16 ++++++++--------
 arch/sh/boot/compressed/Makefile |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/sh/boot/Makefile b/arch/sh/boot/Makefile
index 5c123f5b2797..1f5d2df3c7e0 100644
--- a/arch/sh/boot/Makefile
+++ b/arch/sh/boot/Makefile
@@ -19,12 +19,12 @@ CONFIG_ZERO_PAGE_OFFSET	?= 0x00001000
 CONFIG_ENTRY_OFFSET	?= 0x00001000
 CONFIG_PHYSICAL_START	?= $(CONFIG_MEMORY_START)
 
-suffix-y := bin
-suffix-$(CONFIG_KERNEL_GZIP)	:= gz
-suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
-suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
-suffix-$(CONFIG_KERNEL_XZ)	:= xz
-suffix-$(CONFIG_KERNEL_LZO)	:= lzo
+suffix_y := bin
+suffix_$(CONFIG_KERNEL_GZIP)	:= gz
+suffix_$(CONFIG_KERNEL_BZIP2)	:= bz2
+suffix_$(CONFIG_KERNEL_LZMA)	:= lzma
+suffix_$(CONFIG_KERNEL_XZ)	:= xz
+suffix_$(CONFIG_KERNEL_LZO)	:= lzo
 
 targets := zImage vmlinux.srec romImage uImage uImage.srec uImage.gz \
 	   uImage.bz2 uImage.lzma uImage.xz uImage.lzo uImage.bin \
@@ -106,10 +106,10 @@ OBJCOPYFLAGS_uImage.srec := -I binary -O srec
 $(obj)/uImage.srec: $(obj)/uImage FORCE
 	$(call if_changed,objcopy)
 
-$(obj)/uImage: $(obj)/uImage.$(suffix-y)
+$(obj)/uImage: $(obj)/uImage.$(suffix_y)
 	@ln -sf $(notdir $<) $@
 	@echo '  Image $@ is ready'
 
 export CONFIG_PAGE_OFFSET CONFIG_MEMORY_START CONFIG_BOOT_LINK_OFFSET \
        CONFIG_PHYSICAL_START CONFIG_ZERO_PAGE_OFFSET CONFIG_ENTRY_OFFSET \
-       KERNEL_MEMORY suffix-y
+       KERNEL_MEMORY suffix_y
diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index cf3174df7859..c1eb9a62de55 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -64,5 +64,5 @@ OBJCOPYFLAGS += -R .empty_zero_page
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(ld-bfd) -T
 
-$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
+$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix_y) FORCE
 	$(call if_changed,ld)
-- 
2.32.0

[PATCH 2/5] kbuild: drop $(size_append) from cmd_zstd

[Masahiro Yamada]

The appended file size is only used by the decompressors, which some
architectures support.

As the comment "zstd22 is used for kernel compression" says, cmd_zstd22
is used in arch/{mips,s390,x86}/boot/compressed/Makefile.

On the other hand, there is no good reason to append the file size to
cmd_zstd since it is used for other purposes.

Actually cmd_zstd is only used in usr/Makefile, where the appended file
size is rather harmful.

The initramfs with its file size appended is considered as corrupted
data, so commit 65e00e04e5ae ("initramfs: refactor the initramfs build
rules") added 'override size_append := :' to make it no-op.

As a conclusion, this $(size_append) should not exist here.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index d1f865b8c0cb..5366466ea0e4 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -473,7 +473,7 @@ quiet_cmd_xzmisc = XZMISC  $@
 # be used because it would require zstd to allocate a 128 MB buffer.
 
 quiet_cmd_zstd = ZSTD    $@
-      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
+      cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
       cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
-- 
2.32.0

Re: [PATCH 2/5] kbuild: drop $(size_append) from cmd_zstd

[Nicolas Schier]

On Mon, Jan 10, 2022 at 03:15:26AM +0900, Masahiro Yamada wrote:
> The appended file size is only used by the decompressors, which some
> architectures support.
> 
> As the comment "zstd22 is used for kernel compression" says, cmd_zstd22
> is used in arch/{mips,s390,x86}/boot/compressed/Makefile.
> 
> On the other hand, there is no good reason to append the file size to
> cmd_zstd since it is used for other purposes.
> 
> Actually cmd_zstd is only used in usr/Makefile, where the appended file
> size is rather harmful.
> 
> The initramfs with its file size appended is considered as corrupted
> data, so commit 65e00e04e5ae ("initramfs: refactor the initramfs build
> rules") added 'override size_append := :' to make it no-op.
> 
> As a conclusion, this $(size_append) should not exist here.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>


> 
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index d1f865b8c0cb..5366466ea0e4 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -473,7 +473,7 @@ quiet_cmd_xzmisc = XZMISC  $@
>  # be used because it would require zstd to allocate a 128 MB buffer.
>  
>  quiet_cmd_zstd = ZSTD    $@
> -      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@
> +      cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
>  
>  quiet_cmd_zstd22 = ZSTD22  $@
>        cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> -- 
> 2.32.0
> 

[PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Masahiro Yamada]

GZIP-compressed files end with 4 byte data that represents the size
of the original input. The decompressors (the self-extracting kernel)
exploit it to know the vmlinux size beforehand. To mimic the GZIP's
trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}.
Unfortunately these macros are used everywhere despite the appended
size data is only useful for the decompressors.

There is no guarantee that such hand-crafted trailers are safely ignored.
In fact, the kernel refuses compressed initramdisks with the garbage
data. That is why usr/Makefile overrides size_append to make it no-op.

To limit the use of such broken compressed files, this commit renames
the existing macros as follows:

  cmd_bzip2   --> cmd_bzip2_with_size
  cmd_lzma    --> cmd_lzma_with_size
  cmd_lzo     --> cmd_lzo_with_size
  cmd_lz4     --> cmd_lz4_with_size
  cmd_xzkern  --> cmd_xzkern_with_size
  cmd_zstd22  --> cmd_zstd22_with_size

To keep the decompressors working, I updated the following Makefiles
accordingly:

  arch/arm/boot/compressed/Makefile
  arch/h8300/boot/compressed/Makefile
  arch/mips/boot/compressed/Makefile
  arch/parisc/boot/compressed/Makefile
  arch/s390/boot/compressed/Makefile
  arch/sh/boot/compressed/Makefile
  arch/x86/boot/compressed/Makefile

I reused the current macro names for the normal usecases; they produce
the compressed data in the proper format.

I did not touch the following:

  arch/arc/boot/Makefile
  arch/arm64/boot/Makefile
  arch/csky/boot/Makefile
  arch/mips/boot/Makefile
  arch/riscv/boot/Makefile
  arch/sh/boot/Makefile
  kernel/Makefile

This means those Makefiles will stop appending the size data.

I dropped the 'override size_append' hack from usr/Makefile.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arm/boot/compressed/Makefile    |  8 ++++----
 arch/h8300/boot/compressed/Makefile  |  4 +++-
 arch/mips/boot/compressed/Makefile   | 12 +++++------
 arch/parisc/boot/compressed/Makefile | 10 +++++-----
 arch/s390/boot/compressed/Makefile   | 12 +++++------
 arch/sh/boot/compressed/Makefile     |  8 ++++----
 arch/x86/boot/compressed/Makefile    | 12 +++++------
 scripts/Makefile.lib                 | 30 ++++++++++++++++++++++------
 usr/Makefile                         |  5 -----
 9 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 91265e7ff672..adc0e318a1ea 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -77,10 +77,10 @@ CPPFLAGS_vmlinux.lds += -DTEXT_OFFSET="$(TEXT_OFFSET)"
 CPPFLAGS_vmlinux.lds += -DMALLOC_SIZE="$(MALLOC_SIZE)"
 
 compress-$(CONFIG_KERNEL_GZIP) = gzip
-compress-$(CONFIG_KERNEL_LZO)  = lzo
-compress-$(CONFIG_KERNEL_LZMA) = lzma
-compress-$(CONFIG_KERNEL_XZ)   = xzkern
-compress-$(CONFIG_KERNEL_LZ4)  = lz4
+compress-$(CONFIG_KERNEL_LZO)  = lzo_with_size
+compress-$(CONFIG_KERNEL_LZMA) = lzma_with_size
+compress-$(CONFIG_KERNEL_XZ)   = xzkern_with_size
+compress-$(CONFIG_KERNEL_LZ4)  = lz4_with_size
 
 libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
 
diff --git a/arch/h8300/boot/compressed/Makefile b/arch/h8300/boot/compressed/Makefile
index 5942793f77a0..6ab2fa5ba105 100644
--- a/arch/h8300/boot/compressed/Makefile
+++ b/arch/h8300/boot/compressed/Makefile
@@ -30,9 +30,11 @@ $(obj)/vmlinux.bin: vmlinux FORCE
 
 suffix-$(CONFIG_KERNEL_GZIP)    := gzip
 suffix-$(CONFIG_KERNEL_LZO)     := lzo
+compress-$(CONFIG_KERNEL_GZIP)  := gzip
+compress-$(CONFIG_KERNEL_LZO)   := lzo_with_size
 
 $(obj)/vmlinux.bin.$(suffix-y): $(obj)/vmlinux.bin FORCE
-	$(call if_changed,$(suffix-y))
+	$(call if_changed,$(compress-y))
 
 LDFLAGS_piggy.o := -r --format binary --oformat elf32-h8300-linux -T
 OBJCOPYFLAGS := -O binary
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index f27cf31b4140..832f8001d7d9 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -64,12 +64,12 @@ $(obj)/vmlinux.bin: $(KBUILD_IMAGE) FORCE
 	$(call if_changed,objcopy)
 
 tool_$(CONFIG_KERNEL_GZIP)    = gzip
-tool_$(CONFIG_KERNEL_BZIP2)   = bzip2
-tool_$(CONFIG_KERNEL_LZ4)     = lz4
-tool_$(CONFIG_KERNEL_LZMA)    = lzma
-tool_$(CONFIG_KERNEL_LZO)     = lzo
-tool_$(CONFIG_KERNEL_XZ)      = xzkern
-tool_$(CONFIG_KERNEL_ZSTD)    = zstd22
+tool_$(CONFIG_KERNEL_BZIP2)   = bzip2_with_size
+tool_$(CONFIG_KERNEL_LZ4)     = lz4_with_size
+tool_$(CONFIG_KERNEL_LZMA)    = lzma_with_size
+tool_$(CONFIG_KERNEL_LZO)     = lzo_with_size
+tool_$(CONFIG_KERNEL_XZ)      = xzkern_with_size
+tool_$(CONFIG_KERNEL_ZSTD)    = zstd22_with_size
 
 targets += vmlinux.bin.z
 
diff --git a/arch/parisc/boot/compressed/Makefile b/arch/parisc/boot/compressed/Makefile
index bf4f2891d0b7..2640f72d69ce 100644
--- a/arch/parisc/boot/compressed/Makefile
+++ b/arch/parisc/boot/compressed/Makefile
@@ -70,15 +70,15 @@ suffix-$(CONFIG_KERNEL_XZ)  := xz
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
 $(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
diff --git a/arch/s390/boot/compressed/Makefile b/arch/s390/boot/compressed/Makefile
index 3b860061e84d..8ea880b7c3ec 100644
--- a/arch/s390/boot/compressed/Makefile
+++ b/arch/s390/boot/compressed/Makefile
@@ -71,17 +71,17 @@ suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
 $(obj)/piggy.o: $(obj)/vmlinux.bin$(suffix-y) FORCE
diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index c1eb9a62de55..a6808a403f4b 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -52,13 +52,13 @@ vmlinux.bin.all-y := $(obj)/vmlinux.bin
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page
 
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 431bf7f846c3..2825c74bcae3 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -123,17 +123,17 @@ vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs
 $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
 	$(call if_changed,gzip)
 $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,bzip2)
+	$(call if_changed,bzip2_with_size)
 $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzma)
+	$(call if_changed,lzma_with_size)
 $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,xzkern)
+	$(call if_changed,xzkern_with_size)
 $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lzo)
+	$(call if_changed,lzo_with_size)
 $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,lz4)
+	$(call if_changed,lz4_with_size)
 $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
-	$(call if_changed,zstd22)
+	$(call if_changed,zstd22_with_size)
 
 suffix-$(CONFIG_KERNEL_GZIP)	:= gz
 suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 5366466ea0e4..4207a72d429f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -395,19 +395,31 @@ printf "%08x\n" $$dec_size |						\
 )
 
 quiet_cmd_bzip2 = BZIP2   $@
-      cmd_bzip2 = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
+      cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
+
+quiet_cmd_bzip2_with_size = BZIP2   $@
+      cmd_bzip2_with_size = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
 
 # Lzma
 # ---------------------------------------------------------------------------
 
 quiet_cmd_lzma = LZMA    $@
-      cmd_lzma = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
+      cmd_lzma = cat $(real-prereqs) | $(LZMA) -9 > $@
+
+quiet_cmd_lzma_with_size = LZMA    $@
+      cmd_lzma_with_size = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
 
 quiet_cmd_lzo = LZO     $@
-      cmd_lzo = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
+      cmd_lzo = cat $(real-prereqs) | $(KLZOP) -9 > $@
+
+quiet_cmd_lzo_with_size = LZO     $@
+      cmd_lzo_with_size = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
 
 quiet_cmd_lz4 = LZ4     $@
-      cmd_lz4 = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
+      cmd_lz4 = cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout > $@
+
+quiet_cmd_lz4_with_size = LZ4     $@
+      cmd_lz4_with_size = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
                   $(size_append); } > $@
 
 # U-Boot mkimage
@@ -450,7 +462,10 @@ quiet_cmd_uimage = UIMAGE  $@
 # big dictionary would increase the memory usage too much in the multi-call
 # decompression mode. A BCJ filter isn't used either.
 quiet_cmd_xzkern = XZKERN  $@
-      cmd_xzkern = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
+      cmd_xzkern = cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh > $@
+
+quiet_cmd_xzkern_with_size = XZKERN  $@
+      cmd_xzkern_with_size = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
                      $(size_append); } > $@
 
 quiet_cmd_xzmisc = XZMISC  $@
@@ -476,7 +491,10 @@ quiet_cmd_zstd = ZSTD    $@
       cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
 
 quiet_cmd_zstd22 = ZSTD22  $@
-      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
+      cmd_zstd22 = cat $(real-prereqs) | $(ZSTD) -22 --ultra > $@
+
+quiet_cmd_zstd22_with_size = ZSTD22  $@
+      cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
 
 # ASM offsets
 # ---------------------------------------------------------------------------
diff --git a/usr/Makefile b/usr/Makefile
index b1a81a40eab1..7b89c0175a3a 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -3,11 +3,6 @@
 # kbuild file for usr/ - including initramfs image
 #
 
-# cmd_bzip2, cmd_lzma, cmd_lzo, cmd_lz4 from scripts/Makefile.lib appends the
-# size at the end of the compressed file, which unfortunately does not work
-# with unpack_to_rootfs(). Make size_append no-op.
-override size_append := :
-
 compress-y					:= shipped
 compress-$(CONFIG_INITRAMFS_COMPRESSION_GZIP)	:= gzip
 compress-$(CONFIG_INITRAMFS_COMPRESSION_BZIP2)	:= bzip2
-- 
2.32.0

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Nicolas Schier]

On Mon, Jan 10, 2022 at 03:15:27AM +0900, Masahiro Yamada wrote:
> GZIP-compressed files end with 4 byte data that represents the size
> of the original input. The decompressors (the self-extracting kernel)
> exploit it to know the vmlinux size beforehand. To mimic the GZIP's
> trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}.
> Unfortunately these macros are used everywhere despite the appended
> size data is only useful for the decompressors.
> 
> There is no guarantee that such hand-crafted trailers are safely ignored.
> In fact, the kernel refuses compressed initramdisks with the garbage
> data. That is why usr/Makefile overrides size_append to make it no-op.
> 
> To limit the use of such broken compressed files, this commit renames
> the existing macros as follows:
> 
>   cmd_bzip2   --> cmd_bzip2_with_size
>   cmd_lzma    --> cmd_lzma_with_size
>   cmd_lzo     --> cmd_lzo_with_size
>   cmd_lz4     --> cmd_lz4_with_size
>   cmd_xzkern  --> cmd_xzkern_with_size
>   cmd_zstd22  --> cmd_zstd22_with_size
> 
> To keep the decompressors working, I updated the following Makefiles
> accordingly:
> 
>   arch/arm/boot/compressed/Makefile
>   arch/h8300/boot/compressed/Makefile
>   arch/mips/boot/compressed/Makefile
>   arch/parisc/boot/compressed/Makefile
>   arch/s390/boot/compressed/Makefile
>   arch/sh/boot/compressed/Makefile
>   arch/x86/boot/compressed/Makefile
> 
> I reused the current macro names for the normal usecases; they produce
> the compressed data in the proper format.
> 
> I did not touch the following:
> 
>   arch/arc/boot/Makefile
>   arch/arm64/boot/Makefile
>   arch/csky/boot/Makefile
>   arch/mips/boot/Makefile
>   arch/riscv/boot/Makefile
>   arch/sh/boot/Makefile
>   kernel/Makefile
> 
> This means those Makefiles will stop appending the size data.
> 
> I dropped the 'override size_append' hack from usr/Makefile.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>

> 
>  arch/arm/boot/compressed/Makefile    |  8 ++++----
>  arch/h8300/boot/compressed/Makefile  |  4 +++-
>  arch/mips/boot/compressed/Makefile   | 12 +++++------
>  arch/parisc/boot/compressed/Makefile | 10 +++++-----
>  arch/s390/boot/compressed/Makefile   | 12 +++++------
>  arch/sh/boot/compressed/Makefile     |  8 ++++----
>  arch/x86/boot/compressed/Makefile    | 12 +++++------
>  scripts/Makefile.lib                 | 30 ++++++++++++++++++++++------
>  usr/Makefile                         |  5 -----
>  9 files changed, 58 insertions(+), 43 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 91265e7ff672..adc0e318a1ea 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -77,10 +77,10 @@ CPPFLAGS_vmlinux.lds += -DTEXT_OFFSET="$(TEXT_OFFSET)"
>  CPPFLAGS_vmlinux.lds += -DMALLOC_SIZE="$(MALLOC_SIZE)"
>  
>  compress-$(CONFIG_KERNEL_GZIP) = gzip
> -compress-$(CONFIG_KERNEL_LZO)  = lzo
> -compress-$(CONFIG_KERNEL_LZMA) = lzma
> -compress-$(CONFIG_KERNEL_XZ)   = xzkern
> -compress-$(CONFIG_KERNEL_LZ4)  = lz4
> +compress-$(CONFIG_KERNEL_LZO)  = lzo_with_size
> +compress-$(CONFIG_KERNEL_LZMA) = lzma_with_size
> +compress-$(CONFIG_KERNEL_XZ)   = xzkern_with_size
> +compress-$(CONFIG_KERNEL_LZ4)  = lz4_with_size
>  
>  libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
>  
> diff --git a/arch/h8300/boot/compressed/Makefile b/arch/h8300/boot/compressed/Makefile
> index 5942793f77a0..6ab2fa5ba105 100644
> --- a/arch/h8300/boot/compressed/Makefile
> +++ b/arch/h8300/boot/compressed/Makefile
> @@ -30,9 +30,11 @@ $(obj)/vmlinux.bin: vmlinux FORCE
>  
>  suffix-$(CONFIG_KERNEL_GZIP)    := gzip
>  suffix-$(CONFIG_KERNEL_LZO)     := lzo
> +compress-$(CONFIG_KERNEL_GZIP)  := gzip
> +compress-$(CONFIG_KERNEL_LZO)   := lzo_with_size
>  
>  $(obj)/vmlinux.bin.$(suffix-y): $(obj)/vmlinux.bin FORCE
> -	$(call if_changed,$(suffix-y))
> +	$(call if_changed,$(compress-y))
>  
>  LDFLAGS_piggy.o := -r --format binary --oformat elf32-h8300-linux -T
>  OBJCOPYFLAGS := -O binary
> diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
> index f27cf31b4140..832f8001d7d9 100644
> --- a/arch/mips/boot/compressed/Makefile
> +++ b/arch/mips/boot/compressed/Makefile
> @@ -64,12 +64,12 @@ $(obj)/vmlinux.bin: $(KBUILD_IMAGE) FORCE
>  	$(call if_changed,objcopy)
>  
>  tool_$(CONFIG_KERNEL_GZIP)    = gzip
> -tool_$(CONFIG_KERNEL_BZIP2)   = bzip2
> -tool_$(CONFIG_KERNEL_LZ4)     = lz4
> -tool_$(CONFIG_KERNEL_LZMA)    = lzma
> -tool_$(CONFIG_KERNEL_LZO)     = lzo
> -tool_$(CONFIG_KERNEL_XZ)      = xzkern
> -tool_$(CONFIG_KERNEL_ZSTD)    = zstd22
> +tool_$(CONFIG_KERNEL_BZIP2)   = bzip2_with_size
> +tool_$(CONFIG_KERNEL_LZ4)     = lz4_with_size
> +tool_$(CONFIG_KERNEL_LZMA)    = lzma_with_size
> +tool_$(CONFIG_KERNEL_LZO)     = lzo_with_size
> +tool_$(CONFIG_KERNEL_XZ)      = xzkern_with_size
> +tool_$(CONFIG_KERNEL_ZSTD)    = zstd22_with_size
>  
>  targets += vmlinux.bin.z
>  
> diff --git a/arch/parisc/boot/compressed/Makefile b/arch/parisc/boot/compressed/Makefile
> index bf4f2891d0b7..2640f72d69ce 100644
> --- a/arch/parisc/boot/compressed/Makefile
> +++ b/arch/parisc/boot/compressed/Makefile
> @@ -70,15 +70,15 @@ suffix-$(CONFIG_KERNEL_XZ)  := xz
>  $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
>  	$(call if_changed,gzip)
>  $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,bzip2)
> +	$(call if_changed,bzip2_with_size)
>  $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lz4)
> +	$(call if_changed,lz4_with_size)
>  $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzma)
> +	$(call if_changed,lzma_with_size)
>  $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzo)
> +	$(call if_changed,lzo_with_size)
>  $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,xzkern)
> +	$(call if_changed,xzkern_with_size)
>  
>  LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
>  $(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
> diff --git a/arch/s390/boot/compressed/Makefile b/arch/s390/boot/compressed/Makefile
> index 3b860061e84d..8ea880b7c3ec 100644
> --- a/arch/s390/boot/compressed/Makefile
> +++ b/arch/s390/boot/compressed/Makefile
> @@ -71,17 +71,17 @@ suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
>  $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
>  	$(call if_changed,gzip)
>  $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,bzip2)
> +	$(call if_changed,bzip2_with_size)
>  $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lz4)
> +	$(call if_changed,lz4_with_size)
>  $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzma)
> +	$(call if_changed,lzma_with_size)
>  $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzo)
> +	$(call if_changed,lzo_with_size)
>  $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,xzkern)
> +	$(call if_changed,xzkern_with_size)
>  $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,zstd22)
> +	$(call if_changed,zstd22_with_size)
>  
>  OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
>  $(obj)/piggy.o: $(obj)/vmlinux.bin$(suffix-y) FORCE
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index c1eb9a62de55..a6808a403f4b 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -52,13 +52,13 @@ vmlinux.bin.all-y := $(obj)/vmlinux.bin
>  $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
>  	$(call if_changed,gzip)
>  $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,bzip2)
> +	$(call if_changed,bzip2_with_size)
>  $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzma)
> +	$(call if_changed,lzma_with_size)
>  $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,xzkern)
> +	$(call if_changed,xzkern_with_size)
>  $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzo)
> +	$(call if_changed,lzo_with_size)
>  
>  OBJCOPYFLAGS += -R .empty_zero_page
>  
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 431bf7f846c3..2825c74bcae3 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -123,17 +123,17 @@ vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs
>  $(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
>  	$(call if_changed,gzip)
>  $(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,bzip2)
> +	$(call if_changed,bzip2_with_size)
>  $(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzma)
> +	$(call if_changed,lzma_with_size)
>  $(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,xzkern)
> +	$(call if_changed,xzkern_with_size)
>  $(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lzo)
> +	$(call if_changed,lzo_with_size)
>  $(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,lz4)
> +	$(call if_changed,lz4_with_size)
>  $(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
> -	$(call if_changed,zstd22)
> +	$(call if_changed,zstd22_with_size)
>  
>  suffix-$(CONFIG_KERNEL_GZIP)	:= gz
>  suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5366466ea0e4..4207a72d429f 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -395,19 +395,31 @@ printf "%08x\n" $$dec_size |						\
>  )
>  
>  quiet_cmd_bzip2 = BZIP2   $@
> -      cmd_bzip2 = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
> +      cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
> +
> +quiet_cmd_bzip2_with_size = BZIP2   $@
> +      cmd_bzip2_with_size = { cat $(real-prereqs) | $(KBZIP2) -9; $(size_append); } > $@
>  
>  # Lzma
>  # ---------------------------------------------------------------------------
>  
>  quiet_cmd_lzma = LZMA    $@
> -      cmd_lzma = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
> +      cmd_lzma = cat $(real-prereqs) | $(LZMA) -9 > $@
> +
> +quiet_cmd_lzma_with_size = LZMA    $@
> +      cmd_lzma_with_size = { cat $(real-prereqs) | $(LZMA) -9; $(size_append); } > $@
>  
>  quiet_cmd_lzo = LZO     $@
> -      cmd_lzo = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
> +      cmd_lzo = cat $(real-prereqs) | $(KLZOP) -9 > $@
> +
> +quiet_cmd_lzo_with_size = LZO     $@
> +      cmd_lzo_with_size = { cat $(real-prereqs) | $(KLZOP) -9; $(size_append); } > $@
>  
>  quiet_cmd_lz4 = LZ4     $@
> -      cmd_lz4 = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
> +      cmd_lz4 = cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout > $@
> +
> +quiet_cmd_lz4_with_size = LZ4     $@
> +      cmd_lz4_with_size = { cat $(real-prereqs) | $(LZ4) -l -c1 stdin stdout; \
>                    $(size_append); } > $@
>  
>  # U-Boot mkimage
> @@ -450,7 +462,10 @@ quiet_cmd_uimage = UIMAGE  $@
>  # big dictionary would increase the memory usage too much in the multi-call
>  # decompression mode. A BCJ filter isn't used either.
>  quiet_cmd_xzkern = XZKERN  $@
> -      cmd_xzkern = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
> +      cmd_xzkern = cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh > $@
> +
> +quiet_cmd_xzkern_with_size = XZKERN  $@
> +      cmd_xzkern_with_size = { cat $(real-prereqs) | sh $(srctree)/scripts/xz_wrap.sh; \
>                       $(size_append); } > $@
>  
>  quiet_cmd_xzmisc = XZMISC  $@
> @@ -476,7 +491,10 @@ quiet_cmd_zstd = ZSTD    $@
>        cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@
>  
>  quiet_cmd_zstd22 = ZSTD22  $@
> -      cmd_zstd22 = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
> +      cmd_zstd22 = cat $(real-prereqs) | $(ZSTD) -22 --ultra > $@
> +
> +quiet_cmd_zstd22_with_size = ZSTD22  $@
> +      cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@
>  
>  # ASM offsets
>  # ---------------------------------------------------------------------------
> diff --git a/usr/Makefile b/usr/Makefile
> index b1a81a40eab1..7b89c0175a3a 100644
> --- a/usr/Makefile
> +++ b/usr/Makefile
> @@ -3,11 +3,6 @@
>  # kbuild file for usr/ - including initramfs image
>  #
>  
> -# cmd_bzip2, cmd_lzma, cmd_lzo, cmd_lz4 from scripts/Makefile.lib appends the
> -# size at the end of the compressed file, which unfortunately does not work
> -# with unpack_to_rootfs(). Make size_append no-op.
> -override size_append := :
> -
>  compress-y					:= shipped
>  compress-$(CONFIG_INITRAMFS_COMPRESSION_GZIP)	:= gzip
>  compress-$(CONFIG_INITRAMFS_COMPRESSION_BZIP2)	:= bzip2
> -- 
> 2.32.0
> 

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Eugeniu Rosca]

Hello Yamada-san,
Hello Nicolas,
Cc: SzuWei Lin (committer of the patch in AOSP [1])
Cc: Kbuild

On Mon, Jan 10, 2022 at 12:33:15PM +0100, Nicolas Schier wrote:
> On Mon, Jan 10, 2022 at 03:15:27AM +0900, Masahiro Yamada wrote:
> > GZIP-compressed files end with 4 byte data that represents the size
> > of the original input. The decompressors (the self-extracting kernel)
> > exploit it to know the vmlinux size beforehand. To mimic the GZIP's
> > trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}.
> > Unfortunately these macros are used everywhere despite the appended
> > size data is only useful for the decompressors.
> > 
> > There is no guarantee that such hand-crafted trailers are safely ignored.
> > In fact, the kernel refuses compressed initramdisks with the garbage
> > data. That is why usr/Makefile overrides size_append to make it no-op.
> > 
> > To limit the use of such broken compressed files, this commit renames
> > the existing macros as follows:
> > 
> >   cmd_bzip2   --> cmd_bzip2_with_size
> >   cmd_lzma    --> cmd_lzma_with_size
> >   cmd_lzo     --> cmd_lzo_with_size
> >   cmd_lz4     --> cmd_lz4_with_size
> >   cmd_xzkern  --> cmd_xzkern_with_size
> >   cmd_zstd22  --> cmd_zstd22_with_size
> > 
> > To keep the decompressors working, I updated the following Makefiles
> > accordingly:
> > 
> >   arch/arm/boot/compressed/Makefile
> >   arch/h8300/boot/compressed/Makefile
> >   arch/mips/boot/compressed/Makefile
> >   arch/parisc/boot/compressed/Makefile
> >   arch/s390/boot/compressed/Makefile
> >   arch/sh/boot/compressed/Makefile
> >   arch/x86/boot/compressed/Makefile
> > 
> > I reused the current macro names for the normal usecases; they produce
> > the compressed data in the proper format.
> > 
> > I did not touch the following:
> > 
> >   arch/arc/boot/Makefile
> >   arch/arm64/boot/Makefile
> >   arch/csky/boot/Makefile
> >   arch/mips/boot/Makefile
> >   arch/riscv/boot/Makefile
> >   arch/sh/boot/Makefile
> >   kernel/Makefile
> > 
> > This means those Makefiles will stop appending the size data.
> > 
> > I dropped the 'override size_append' hack from usr/Makefile.
> > 
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> 
> Reviewed-by: Nicolas Schier <n.schier@avm.de>

If you don't mind, I would like to report another instance of
"/bin/sh: Argument list too long" while building some out-of-tree *ko
in a number of downstream v5.15.78+ kernels containing [1].

For some time now, we've been living with ugly hacks to overcome it.

Fortunately, recent git bisecting efforts apparently reveal that
current v5.17-rc1 commit (and its backports in downstream) look to
act as the culprit (confirmed on several host machines). So, I
started to have some hopes of a long-term solution and hence
sharing the findings as a first step.

I am not entirely clear how to properly trace this behavior, since no
amount of "make V=1/V=2" uncovers more details. Purely by accident, I
looked into the top/htop output (while running the repro) and
noticed several processes doing:

/bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
  fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
  dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
  | sed 's/\(..\)/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
  $ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }

As it was the case in the recent report [2], the above command seems
to require/assume generous amount of space for the shell arguments.

I still haven't compared the exact traces before and after this commit,
to quantify by how much the shell argument list is increased (TODO).

Another aspect is that current commit seems to introduce the
regression in a multi-threaded make only. The issue is apparently
masked by 'make -j1' (TBC), which adds another level of complexity.

Unfortunately, the build use-case is highly tailored to downstream
and is not repeatable against vanilla out of the box.

I will continue to increase my understanding behind what's happening.
In case there are already any suggestions, would appreciate those.

[1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
    ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")

[2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

-- 
Best regards,
Eugeniu Rosca

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Eugeniu Rosca]

Hello Yamada-san,

On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:
> Hello Yamada-san,
> Hello Nicolas,
> Cc: SzuWei Lin (committer of the patch in AOSP [1])
> Cc: Kbuild
> 
> On Mon, Jan 10, 2022 at 12:33:15PM +0100, Nicolas Schier wrote:
> > On Mon, Jan 10, 2022 at 03:15:27AM +0900, Masahiro Yamada wrote:
> > > GZIP-compressed files end with 4 byte data that represents the size
> > > of the original input. The decompressors (the self-extracting kernel)
> > > exploit it to know the vmlinux size beforehand. To mimic the GZIP's
> > > trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}.
> > > Unfortunately these macros are used everywhere despite the appended
> > > size data is only useful for the decompressors.
> > > 
> > > There is no guarantee that such hand-crafted trailers are safely ignored.
> > > In fact, the kernel refuses compressed initramdisks with the garbage
> > > data. That is why usr/Makefile overrides size_append to make it no-op.
> > > 
> > > To limit the use of such broken compressed files, this commit renames
> > > the existing macros as follows:
> > > 
> > >   cmd_bzip2   --> cmd_bzip2_with_size
> > >   cmd_lzma    --> cmd_lzma_with_size
> > >   cmd_lzo     --> cmd_lzo_with_size
> > >   cmd_lz4     --> cmd_lz4_with_size
> > >   cmd_xzkern  --> cmd_xzkern_with_size
> > >   cmd_zstd22  --> cmd_zstd22_with_size
> > > 
> > > To keep the decompressors working, I updated the following Makefiles
> > > accordingly:
> > > 
> > >   arch/arm/boot/compressed/Makefile
> > >   arch/h8300/boot/compressed/Makefile
> > >   arch/mips/boot/compressed/Makefile
> > >   arch/parisc/boot/compressed/Makefile
> > >   arch/s390/boot/compressed/Makefile
> > >   arch/sh/boot/compressed/Makefile
> > >   arch/x86/boot/compressed/Makefile
> > > 
> > > I reused the current macro names for the normal usecases; they produce
> > > the compressed data in the proper format.
> > > 
> > > I did not touch the following:
> > > 
> > >   arch/arc/boot/Makefile
> > >   arch/arm64/boot/Makefile
> > >   arch/csky/boot/Makefile
> > >   arch/mips/boot/Makefile
> > >   arch/riscv/boot/Makefile
> > >   arch/sh/boot/Makefile
> > >   kernel/Makefile
> > > 
> > > This means those Makefiles will stop appending the size data.
> > > 
> > > I dropped the 'override size_append' hack from usr/Makefile.
> > > 
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > 
> > Reviewed-by: Nicolas Schier <n.schier@avm.de>
> 
> If you don't mind, I would like to report another instance of
> "/bin/sh: Argument list too long" while building some out-of-tree *ko
> in a number of downstream v5.15.78+ kernels containing [1].
> 
> For some time now, we've been living with ugly hacks to overcome it.
> 
> Fortunately, recent git bisecting efforts apparently reveal that
> current v5.17-rc1 commit (and its backports in downstream) look to
> act as the culprit (confirmed on several host machines). So, I
> started to have some hopes of a long-term solution and hence
> sharing the findings as a first step.
> 
> I am not entirely clear how to properly trace this behavior, since no
> amount of "make V=1/V=2" uncovers more details. Purely by accident, I
> looked into the top/htop output (while running the repro) and
> noticed several processes doing:
> 
> /bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
>   fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
>   dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
>   | sed 's/\(..\)/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
>   $ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }
> 
> As it was the case in the recent report [2], the above command seems
> to require/assume generous amount of space for the shell arguments.
> 
> I still haven't compared the exact traces before and after this commit,
> to quantify by how much the shell argument list is increased (TODO).
> 
> Another aspect is that current commit seems to introduce the
> regression in a multi-threaded make only. The issue is apparently
> masked by 'make -j1' (TBC), which adds another level of complexity.
> 
> Unfortunately, the build use-case is highly tailored to downstream
> and is not repeatable against vanilla out of the box.
> 
> I will continue to increase my understanding behind what's happening.
> In case there are already any suggestions, would appreciate those.

JFYI, we've got confirmation from Qualcomm Customer Support interface
that reverting [1] heals the issue on QC end as well. However, it looks
like none of us has clear understanding how to properly
troubleshoot/trace/compare the behavior before and after the commit.

I would happily follow any suggestions.

> [1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
>     ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
> 
> [2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

-- 
Best regards,
Eugeniu Rosca

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Masahiro Yamada]

On Thu, Jul 20, 2023 at 4:09 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
>
> Hello Yamada-san,
>
> On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:
> > Hello Yamada-san,
> > Hello Nicolas,
> > Cc: SzuWei Lin (committer of the patch in AOSP [1])
> > Cc: Kbuild
> >
> > On Mon, Jan 10, 2022 at 12:33:15PM +0100, Nicolas Schier wrote:
> > > On Mon, Jan 10, 2022 at 03:15:27AM +0900, Masahiro Yamada wrote:
> > > > GZIP-compressed files end with 4 byte data that represents the size
> > > > of the original input. The decompressors (the self-extracting kernel)
> > > > exploit it to know the vmlinux size beforehand. To mimic the GZIP's
> > > > trailer, Kbuild provides cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}.
> > > > Unfortunately these macros are used everywhere despite the appended
> > > > size data is only useful for the decompressors.
> > > >
> > > > There is no guarantee that such hand-crafted trailers are safely ignored.
> > > > In fact, the kernel refuses compressed initramdisks with the garbage
> > > > data. That is why usr/Makefile overrides size_append to make it no-op.
> > > >
> > > > To limit the use of such broken compressed files, this commit renames
> > > > the existing macros as follows:
> > > >
> > > >   cmd_bzip2   --> cmd_bzip2_with_size
> > > >   cmd_lzma    --> cmd_lzma_with_size
> > > >   cmd_lzo     --> cmd_lzo_with_size
> > > >   cmd_lz4     --> cmd_lz4_with_size
> > > >   cmd_xzkern  --> cmd_xzkern_with_size
> > > >   cmd_zstd22  --> cmd_zstd22_with_size
> > > >
> > > > To keep the decompressors working, I updated the following Makefiles
> > > > accordingly:
> > > >
> > > >   arch/arm/boot/compressed/Makefile
> > > >   arch/h8300/boot/compressed/Makefile
> > > >   arch/mips/boot/compressed/Makefile
> > > >   arch/parisc/boot/compressed/Makefile
> > > >   arch/s390/boot/compressed/Makefile
> > > >   arch/sh/boot/compressed/Makefile
> > > >   arch/x86/boot/compressed/Makefile
> > > >
> > > > I reused the current macro names for the normal usecases; they produce
> > > > the compressed data in the proper format.
> > > >
> > > > I did not touch the following:
> > > >
> > > >   arch/arc/boot/Makefile
> > > >   arch/arm64/boot/Makefile
> > > >   arch/csky/boot/Makefile
> > > >   arch/mips/boot/Makefile
> > > >   arch/riscv/boot/Makefile
> > > >   arch/sh/boot/Makefile
> > > >   kernel/Makefile
> > > >
> > > > This means those Makefiles will stop appending the size data.
> > > >
> > > > I dropped the 'override size_append' hack from usr/Makefile.
> > > >
> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > > ---
> > >
> > > Reviewed-by: Nicolas Schier <n.schier@avm.de>
> >
> > If you don't mind, I would like to report another instance of
> > "/bin/sh: Argument list too long" while building some out-of-tree *ko
> > in a number of downstream v5.15.78+ kernels containing [1].
> >
> > For some time now, we've been living with ugly hacks to overcome it.
> >
> > Fortunately, recent git bisecting efforts apparently reveal that
> > current v5.17-rc1 commit (and its backports in downstream) look to
> > act as the culprit (confirmed on several host machines). So, I
> > started to have some hopes of a long-term solution and hence
> > sharing the findings as a first step.
> >
> > I am not entirely clear how to properly trace this behavior, since no
> > amount of "make V=1/V=2" uncovers more details. Purely by accident, I
> > looked into the top/htop output (while running the repro) and
> > noticed several processes doing:
> >
> > /bin/sh -c dec_size=0; for F in <humongous list of filenames>; do \
> >   fsize=$(sh /abs/path/to/scripts/file-size.sh $F); \
> >   dec_size=$(expr $dec_size + $fsize); done; printf "%08x\n" $dec_size \
> >   | sed 's/\(..\)/\1 /g' | { read ch0 ch1 ch2 ch3; for ch in \
> >   $ch3 $ch2 $ch1 $ch0; do printf '%s%03o' '\\' $((0x$ch)); done; }
> >
> > As it was the case in the recent report [2], the above command seems
> > to require/assume generous amount of space for the shell arguments.
> >
> > I still haven't compared the exact traces before and after this commit,
> > to quantify by how much the shell argument list is increased (TODO).
> >
> > Another aspect is that current commit seems to introduce the
> > regression in a multi-threaded make only. The issue is apparently
> > masked by 'make -j1' (TBC), which adds another level of complexity.
> >
> > Unfortunately, the build use-case is highly tailored to downstream
> > and is not repeatable against vanilla out of the box.
> >
> > I will continue to increase my understanding behind what's happening.
> > In case there are already any suggestions, would appreciate those.
>
> JFYI, we've got confirmation from Qualcomm Customer Support interface
> that reverting [1] heals the issue on QC end as well. However, it looks
> like none of us has clear understanding how to properly
> troubleshoot/trace/compare the behavior before and after the commit.
>
> I would happily follow any suggestions.
>
> > [1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
> >     ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
> >
> > [2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/
>
> --
> Best regards,
> Eugeniu Rosca





The only suspicious code I found in the Android common kernel
is the following line in scripts/Makefile.lib



quiet_cmd_zstd = ZSTD    $@
      cmd_zstd = { cat $(real-prereqs) | $(ZSTD) -19; $(size_append); } > $@







If you see the corresponding line in the mainline kernel,
it looks as follows:


quiet_cmd_zstd = ZSTD    $@
      cmd_zstd = cat $(real-prereqs) | $(ZSTD) -19 > $@








7ce7e984ab2b218d6e92d5165629022fe2daf9ee depends on
64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c


But, Android common kernel back-ported only
7ce7e984ab2b218d6e92d5165629022fe2daf9ee



Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c
and see if the problem goes away.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Eugeniu Rosca]

Hello Yamada-san,

Appreciate your willingness to support. Some findings below.

On Sun, Jul 23, 2023 at 01:08:46AM +0900, Masahiro Yamada wrote:
> On Thu, Jul 20, 2023 at 4:09 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> > On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:

[..]

> > > I will continue to increase my understanding behind what's happening.
> > > In case there are already any suggestions, would appreciate those.
> >
> > JFYI, we've got confirmation from Qualcomm Customer Support interface
> > that reverting [1] heals the issue on QC end as well. However, it looks
> > like none of us has clear understanding how to properly
> > troubleshoot/trace/compare the behavior before and after the commit.
> >
> > I would happily follow any suggestions.
> >
> > > [1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
> > >     ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
> > >
> > > [2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/

[..]

> Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c
> and see if the problem goes away.

Unfortunately, the problem remains after backporting the above commit.

After some more bisecting and some more trial-and-error, I finally came
up with a reproduction scenario against vanilla. It also shows that
after reverting 7ce7e984ab2b21 ("kbuild: rename
cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}"), the problem goes away.

It takes <30 seconds to reproduce the issue on my machine (on 2nd run).

In order to make the test self-sufficient, it also clones the Linux
sources (only during 1st run, with --depth 1, for minimal footprint),
hence ~1.8 GB free space is required in /tmp .

The repro.sh script:
 => https://gist.github.com/erosca/1372fdc24126dc98031444613450c494

Output against vanilla on 1st run (always OK, matches real-life case):
 => https://gist.github.com/erosca/0f5b8e0a00a256d80f0c8a6364d81568

Output against vanilla on 2nd/Nth run (NOK: Argument list too long):
 => https://gist.github.com/erosca/e5c2c6479cc32244cc38d308deea4cf5

Output against vanilla + revert_of_7ce7e984ab2b2 on Nth run (always OK):
 => https://gist.github.com/erosca/57e114f92ea20132e19fc7f5a46e7c65

Would it be possible to get your thoughts on the above?

-- 
Best regards,
Eugeniu Rosca

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Masahiro Yamada]

[-- Attachment #1: Type: text/plain, Size: 3287 bytes --]

On Tue, Jul 25, 2023 at 6:24 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
>
> Hello Yamada-san,
>
> Appreciate your willingness to support. Some findings below.
>
> On Sun, Jul 23, 2023 at 01:08:46AM +0900, Masahiro Yamada wrote:
> > On Thu, Jul 20, 2023 at 4:09 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> > > On Fri, Jun 23, 2023 at 04:45:44PM +0200, Eugeniu Rosca wrote:
>
> [..]
>
> > > > I will continue to increase my understanding behind what's happening.
> > > > In case there are already any suggestions, would appreciate those.
> > >
> > > JFYI, we've got confirmation from Qualcomm Customer Support interface
> > > that reverting [1] heals the issue on QC end as well. However, it looks
> > > like none of us has clear understanding how to properly
> > > troubleshoot/trace/compare the behavior before and after the commit.
> > >
> > > I would happily follow any suggestions.
> > >
> > > > [1] https://android.googlesource.com/kernel/common/+/bc6d3d83539512
> > > >     ("UPSTREAM: kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}")
> > > >
> > > > [2] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/
>
> [..]
>
> > Please backport 64d8aaa4ef388b22372de4dc9ce3b9b3e5f45b6c
> > and see if the problem goes away.
>
> Unfortunately, the problem remains after backporting the above commit.
>
> After some more bisecting and some more trial-and-error, I finally came
> up with a reproduction scenario against vanilla. It also shows that
> after reverting 7ce7e984ab2b21 ("kbuild: rename
> cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}"), the problem goes away.
>
> It takes <30 seconds to reproduce the issue on my machine (on 2nd run).
>
> In order to make the test self-sufficient, it also clones the Linux
> sources (only during 1st run, with --depth 1, for minimal footprint),
> hence ~1.8 GB free space is required in /tmp .
>
> The repro.sh script:
>  => https://gist.github.com/erosca/1372fdc24126dc98031444613450c494
>
> Output against vanilla on 1st run (always OK, matches real-life case):
>  => https://gist.github.com/erosca/0f5b8e0a00a256d80f0c8a6364d81568
>
> Output against vanilla on 2nd/Nth run (NOK: Argument list too long):
>  => https://gist.github.com/erosca/e5c2c6479cc32244cc38d308deea4cf5
>
> Output against vanilla + revert_of_7ce7e984ab2b2 on Nth run (always OK):
>  => https://gist.github.com/erosca/57e114f92ea20132e19fc7f5a46e7c65
>
> Would it be possible to get your thoughts on the above?
>
> --
> Best regards,
> Eugeniu Rosca





Indeed, reverting 7ce7e984ab2b218d6e92d5165629022fe2daf9ee
makes qcom's external module build successfully
(but rebuilding is super slow).

Interestingly, revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee
then apply the attached patch, then
'Argument list too long' will come back.

So, this is unrelated to the actual build commands.




I suspect bare 'export', which expands all variables
while apparently most of them are not meant exported.



Insert the following in your reproducer, then it will work.


# qcom's audio-kernel sprinkles 'export' everywhere.
# Remove bare use of 'export'
find "$ABS_KMOD" -name Kbuild | xargs sed -i '/export$/d'




--
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-Add-dummy-commands-to-make-qcom-s-external-module-fa.patch --]
[-- Type: text/x-patch, Size: 1640 bytes --]

From a1d641238851dbde00a37b7b203b110e786edd1d Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Wed, 2 Aug 2023 18:11:32 +0900
Subject: [PATCH] Add dummy commands to make qcom's external module fail

Revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee and
apply this instead.

I see

/bin/sh: Argument list too long

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
 scripts/Makefile.lib | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 2270ed819a29..c481018d1b5f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -559,3 +559,36 @@ define filechk_offsets
 	 echo ""; \
 	 echo "#endif"
 endef
+
+
+# These dummy commands are not used anywhere, but
+# make qcom's external module fail to build.
+# qcom's Makefile use bare 'export'.
+# Having more and more shell commands may flood variable expansions.
+
+quiet_cmd_dummy1 = FOO    $@
+      cmd_dummy1 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy2 = FOO    $@
+      cmd_dummy2 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy3 = FOO    $@
+      cmd_dummy3 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy4 = FOO    $@
+      cmd_dummy4 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy5 = FOO    $@
+      cmd_dummy5 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy6 = FOO    $@
+      cmd_dummy6 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy7 = FOO    $@
+      cmd_dummy7 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy8 = FOO    $@
+      cmd_dummy8 = cat $(real-prereqs) > $@
+
+quiet_cmd_dummy9 = FOO    $@
+      cmd_dummy9 = cat $(real-prereqs) > $@
-- 
2.39.2

Re: [PATCH 3/5] kbuild: rename cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22}

[Eugeniu Rosca]

Hello Yamada-san,

Appreciate your generous support, which allowed finding the root-cause.

On Wed, Aug 02, 2023 at 06:21:14PM +0900, Masahiro Yamada wrote:

[..]

> Indeed, reverting 7ce7e984ab2b218d6e92d5165629022fe2daf9ee
> makes qcom's external module build successfully
> (but rebuilding is super slow).

Same observation.

> 
> Interestingly, revert 7ce7e984ab2b218d6e92d5165629022fe2daf9ee
> then apply the attached patch, then
> 'Argument list too long' will come back.
> 
> So, this is unrelated to the actual build commands.
> 
> I suspect bare 'export', which expands all variables
> while apparently most of them are not meant exported.

Indeed, that seems to be the case and more evidence supports this:

 * Several pre-existing commits point out to the same root-cause:

    https://git.codelinaro.org/clo/la/platform/vendor/qcom/opensource/audio-kernel-ar/-/commit/4025a25a2479fc34
    ("makefile: kona: remove make export <all variables> instances")

    https://github.com/sonyxperiadev/kernel-techpack-audio/commit/02f00754120df2
    ("audio-kernel: Fix build time issue")

 * The more recent GNU Make 4.4.1 (called with -ddd) reveals more
    details just before hitting 'Argument list too long':

    scripts/Makefile.lib:431: not recursively expanding size_append to export to shell function
    scripts/Makefile.lib:447: not recursively expanding cmd_file_size to export to shell function
    scripts/Makefile.lib:453: not recursively expanding cmd_bzip2_with_size to export to shell function
    scripts/Makefile.lib:462: not recursively expanding cmd_lzma_with_size to export to shell function
    scripts/Makefile.lib:468: not recursively expanding cmd_lzo_with_size to export to shell function
    scripts/Makefile.lib:474: not recursively expanding cmd_lz4_with_size to export to shell function
    scripts/Makefile.lib:520: not recursively expanding cmd_xzkern_with_size to export to shell function
    scripts/Makefile.lib:549: not recursively expanding cmd_zstd22_with_size to export to shell function

    The "not recursively expanding" messages above come from GNU
    make commit https://gnu.googlesource.com/make/+/7d484017077089a
    ("[SV 63016] Don't fail exporting to $(shell ...)"), which appears
    to be saying that our issue is caused by exporting a makefile
    variable which contains the 'shell' directive in its definition.

    I think it would be too much of a burden for Kbuild not to make use
    of any variables using the 'shell' keyword, so I tend to agree that
    defending against bare 'export' statements in Kbuild is rather not
    feasible and it should be fixed in the makefiles of out-of-tree *ko.

> Insert the following in your reproducer, then it will work.

Thanks for the demo code snippet.

-- 
Best regards,
Eugeniu Rosca

[PATCH 4/5] arch: decompressor: remove useless vmlinux.bin.all-y

[Masahiro Yamada]

Presumably, arch/{parisc,s390,sh}/boot/compressed/Makefile copied
arch/x86/boot/compressed/Makefile, but vmlinux.bin.all-y is useless
here because it is the same as $(obj)/vmlinux.bin.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/parisc/boot/compressed/Makefile | 14 ++++++--------
 arch/s390/boot/compressed/Makefile   | 16 +++++++---------
 arch/sh/boot/compressed/Makefile     | 12 +++++-------
 3 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/arch/parisc/boot/compressed/Makefile b/arch/parisc/boot/compressed/Makefile
index 2640f72d69ce..877a7099b5e1 100644
--- a/arch/parisc/boot/compressed/Makefile
+++ b/arch/parisc/boot/compressed/Makefile
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -R .comment -R .note -S
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := gz
 suffix-$(CONFIG_KERNEL_BZIP2) := bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := lz4
@@ -67,17 +65,17 @@ suffix-$(CONFIG_KERNEL_LZMA)  := lzma
 suffix-$(CONFIG_KERNEL_LZO)  := lzo
 suffix-$(CONFIG_KERNEL_XZ)  := xz
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
 
 LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
diff --git a/arch/s390/boot/compressed/Makefile b/arch/s390/boot/compressed/Makefile
index 8ea880b7c3ec..d04e0e7de0b3 100644
--- a/arch/s390/boot/compressed/Makefile
+++ b/arch/s390/boot/compressed/Makefile
@@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -O binary --remove-section=.comment --remove-section
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
 suffix-$(CONFIG_KERNEL_GZIP)  := .gz
 suffix-$(CONFIG_KERNEL_BZIP2) := .bz2
 suffix-$(CONFIG_KERNEL_LZ4)  := .lz4
@@ -68,19 +66,19 @@ suffix-$(CONFIG_KERNEL_LZO)  := .lzo
 suffix-$(CONFIG_KERNEL_XZ)  := .xz
 suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
 
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lz4_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.zst: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,zstd22_with_size)
 
 OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index a6808a403f4b..591125c42d49 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -47,17 +47,15 @@ $(obj)/vmlinux: $(addprefix $(obj)/, $(OBJECTS)) FORCE
 $(obj)/vmlinux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
 
-vmlinux.bin.all-y := $(obj)/vmlinux.bin
-
-$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,gzip)
-$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,bzip2_with_size)
-$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzma_with_size)
-$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,xzkern_with_size)
-$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
+$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
 	$(call if_changed,lzo_with_size)
 
 OBJCOPYFLAGS += -R .empty_zero_page
-- 
2.32.0

Re: [PATCH 4/5] arch: decompressor: remove useless vmlinux.bin.all-y

[Nicolas Schier]

On Mon, Jan 10, 2022 at 03:15:28AM +0900, Masahiro Yamada wrote:
> Presumably, arch/{parisc,s390,sh}/boot/compressed/Makefile copied
> arch/x86/boot/compressed/Makefile, but vmlinux.bin.all-y is useless
> here because it is the same as $(obj)/vmlinux.bin.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>

> 
>  arch/parisc/boot/compressed/Makefile | 14 ++++++--------
>  arch/s390/boot/compressed/Makefile   | 16 +++++++---------
>  arch/sh/boot/compressed/Makefile     | 12 +++++-------
>  3 files changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/parisc/boot/compressed/Makefile b/arch/parisc/boot/compressed/Makefile
> index 2640f72d69ce..877a7099b5e1 100644
> --- a/arch/parisc/boot/compressed/Makefile
> +++ b/arch/parisc/boot/compressed/Makefile
> @@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -R .comment -R .note -S
>  $(obj)/vmlinux.bin: vmlinux FORCE
>  	$(call if_changed,objcopy)
>  
> -vmlinux.bin.all-y := $(obj)/vmlinux.bin
> -
>  suffix-$(CONFIG_KERNEL_GZIP)  := gz
>  suffix-$(CONFIG_KERNEL_BZIP2) := bz2
>  suffix-$(CONFIG_KERNEL_LZ4)  := lz4
> @@ -67,17 +65,17 @@ suffix-$(CONFIG_KERNEL_LZMA)  := lzma
>  suffix-$(CONFIG_KERNEL_LZO)  := lzo
>  suffix-$(CONFIG_KERNEL_XZ)  := xz
>  
> -$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,gzip)
> -$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,bzip2_with_size)
> -$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lz4_with_size)
> -$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzma_with_size)
> -$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzo_with_size)
> -$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,xzkern_with_size)
>  
>  LDFLAGS_piggy.o := -r --format binary --oformat $(LD_BFD) -T
> diff --git a/arch/s390/boot/compressed/Makefile b/arch/s390/boot/compressed/Makefile
> index 8ea880b7c3ec..d04e0e7de0b3 100644
> --- a/arch/s390/boot/compressed/Makefile
> +++ b/arch/s390/boot/compressed/Makefile
> @@ -58,8 +58,6 @@ OBJCOPYFLAGS_vmlinux.bin := -O binary --remove-section=.comment --remove-section
>  $(obj)/vmlinux.bin: vmlinux FORCE
>  	$(call if_changed,objcopy)
>  
> -vmlinux.bin.all-y := $(obj)/vmlinux.bin
> -
>  suffix-$(CONFIG_KERNEL_GZIP)  := .gz
>  suffix-$(CONFIG_KERNEL_BZIP2) := .bz2
>  suffix-$(CONFIG_KERNEL_LZ4)  := .lz4
> @@ -68,19 +66,19 @@ suffix-$(CONFIG_KERNEL_LZO)  := .lzo
>  suffix-$(CONFIG_KERNEL_XZ)  := .xz
>  suffix-$(CONFIG_KERNEL_ZSTD)  := .zst
>  
> -$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,gzip)
> -$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,bzip2_with_size)
> -$(obj)/vmlinux.bin.lz4: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lz4: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lz4_with_size)
> -$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzma_with_size)
> -$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzo_with_size)
> -$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,xzkern_with_size)
> -$(obj)/vmlinux.bin.zst: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.zst: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,zstd22_with_size)
>  
>  OBJCOPYFLAGS_piggy.o := -I binary -O elf64-s390 -B s390:64-bit --rename-section .data=.vmlinux.bin.compressed
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index a6808a403f4b..591125c42d49 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -47,17 +47,15 @@ $(obj)/vmlinux: $(addprefix $(obj)/, $(OBJECTS)) FORCE
>  $(obj)/vmlinux.bin: vmlinux FORCE
>  	$(call if_changed,objcopy)
>  
> -vmlinux.bin.all-y := $(obj)/vmlinux.bin
> -
> -$(obj)/vmlinux.bin.gz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.gz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,gzip)
> -$(obj)/vmlinux.bin.bz2: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.bz2: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,bzip2_with_size)
> -$(obj)/vmlinux.bin.lzma: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzma: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzma_with_size)
> -$(obj)/vmlinux.bin.xz: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.xz: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,xzkern_with_size)
> -$(obj)/vmlinux.bin.lzo: $(vmlinux.bin.all-y) FORCE
> +$(obj)/vmlinux.bin.lzo: $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,lzo_with_size)
>  
>  OBJCOPYFLAGS += -R .empty_zero_page
> -- 
> 2.32.0
> 

[PATCH 5/5] kbuild: add cmd_file_size

[Masahiro Yamada]

Some architectures support self-extracting kernel, which embeds the
compressed vmlinux.

It has 4 byte data at the end so the decompressor can know the vmlinux
size beforehand.

GZIP natively has it in the trailer, but for the other compression
algorithms, the hand-crafted trailer is added.

It is unneeded to generate such _corrupted_ compressed files because
it is possible to pass the size data separately.

For example, the assembly code:

     .incbin "compressed-vmlinux-with-size-data-appended"

can be transformed to:

     .incbin "compressed-vmlinux"
     .incbin "size-data"

My hope is, after some reworks of the decompressors, the macros
cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22} will go away.

This new macro, cmd_file_size, will be useful to generate a separate
size-data file.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 4207a72d429f..05ca77706f6b 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -394,6 +394,9 @@ printf "%08x\n" $$dec_size |						\
 	}								\
 )
 
+quiet_cmd_file_size = GEN     $@
+      cmd_file_size = $(size_append) > $@
+
 quiet_cmd_bzip2 = BZIP2   $@
       cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
 
-- 
2.32.0

Re: [PATCH 5/5] kbuild: add cmd_file_size

[Nicolas Schier]

On Mon, Jan 10, 2022 at 03:15:29AM +0900, Masahiro Yamada wrote:
> Some architectures support self-extracting kernel, which embeds the
> compressed vmlinux.
> 
> It has 4 byte data at the end so the decompressor can know the vmlinux
> size beforehand.
> 
> GZIP natively has it in the trailer, but for the other compression
> algorithms, the hand-crafted trailer is added.
> 
> It is unneeded to generate such _corrupted_ compressed files because
> it is possible to pass the size data separately.
> 
> For example, the assembly code:
> 
>      .incbin "compressed-vmlinux-with-size-data-appended"
> 
> can be transformed to:
> 
>      .incbin "compressed-vmlinux"
>      .incbin "size-data"
> 
> My hope is, after some reworks of the decompressors, the macros
> cmd_{bzip2,lzma,lzo,lz4,xzkern,zstd22} will go away.
> 
> This new macro, cmd_file_size, will be useful to generate a separate
> size-data file.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>

> 
>  scripts/Makefile.lib | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4207a72d429f..05ca77706f6b 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -394,6 +394,9 @@ printf "%08x\n" $$dec_size |						\
>  	}								\
>  )
>  
> +quiet_cmd_file_size = GEN     $@
> +      cmd_file_size = $(size_append) > $@
> +
>  quiet_cmd_bzip2 = BZIP2   $@
>        cmd_bzip2 = cat $(real-prereqs) | $(KBZIP2) -9 > $@
>  
> -- 
> 2.32.0
> 

Re: [PATCH 1/5] sh: rename suffix-y to suffix_y

[Nicolas Schier]

On Mon, Jan 10, 2022 at 03:15:25AM +0900, Masahiro Yamada wrote:
> 'export suffix-y' does not work reliably because hyphens are disallowed
> in shell variables.
> 
> A similar issue was fixed by commit 2bfbe7881ee0 ("kbuild: Do not use
> hyphen in exported variable name").
> 
> If I do similar in dash, ARCH=sh fails to build.
> 
>   $ mv linux linux~
>   $ cd linux~
>   $ dash
>   $ make O=foo/bar ARCH=sh CROSS_COMPILE=sh4-linux-gnu- defconfig all
>   make[1]: Entering directory '/home/masahiro/linux~/foo/bar'
>     [ snip ]
>   make[4]: *** No rule to make target 'arch/sh/boot/compressed/vmlinux.bin.', needed by 'arch/sh/boot/compressed/piggy.o'.  Stop.
>   make[3]: *** [/home/masahiro/linux~/arch/sh/boot/Makefile:40: arch/sh/boot/compressed/vmlinux] Error 2
>   make[2]: *** [/home/masahiro/linux~/arch/sh/Makefile:194: zImage] Error 2
>   make[1]: *** [/home/masahiro/linux~/Makefile:350: __build_one_by_one] Error 2
>   make[1]: Leaving directory '/home/masahiro/linux~/foo/bar'
>   make: *** [Makefile:219: __sub-make] Error 2
> 
> The maintainer of GNU Make stated that there is no consistent way to
> export variables that do not meet the shell's naming criteria.
> (https://savannah.gnu.org/bugs/?55719)
> 
> Consequently, you cannot use hyphens in exported variables.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>

> 
>  arch/sh/boot/Makefile            | 16 ++++++++--------
>  arch/sh/boot/compressed/Makefile |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/sh/boot/Makefile b/arch/sh/boot/Makefile
> index 5c123f5b2797..1f5d2df3c7e0 100644
> --- a/arch/sh/boot/Makefile
> +++ b/arch/sh/boot/Makefile
> @@ -19,12 +19,12 @@ CONFIG_ZERO_PAGE_OFFSET	?= 0x00001000
>  CONFIG_ENTRY_OFFSET	?= 0x00001000
>  CONFIG_PHYSICAL_START	?= $(CONFIG_MEMORY_START)
>  
> -suffix-y := bin
> -suffix-$(CONFIG_KERNEL_GZIP)	:= gz
> -suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
> -suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
> -suffix-$(CONFIG_KERNEL_XZ)	:= xz
> -suffix-$(CONFIG_KERNEL_LZO)	:= lzo
> +suffix_y := bin
> +suffix_$(CONFIG_KERNEL_GZIP)	:= gz
> +suffix_$(CONFIG_KERNEL_BZIP2)	:= bz2
> +suffix_$(CONFIG_KERNEL_LZMA)	:= lzma
> +suffix_$(CONFIG_KERNEL_XZ)	:= xz
> +suffix_$(CONFIG_KERNEL_LZO)	:= lzo
>  
>  targets := zImage vmlinux.srec romImage uImage uImage.srec uImage.gz \
>  	   uImage.bz2 uImage.lzma uImage.xz uImage.lzo uImage.bin \
> @@ -106,10 +106,10 @@ OBJCOPYFLAGS_uImage.srec := -I binary -O srec
>  $(obj)/uImage.srec: $(obj)/uImage FORCE
>  	$(call if_changed,objcopy)
>  
> -$(obj)/uImage: $(obj)/uImage.$(suffix-y)
> +$(obj)/uImage: $(obj)/uImage.$(suffix_y)
>  	@ln -sf $(notdir $<) $@
>  	@echo '  Image $@ is ready'
>  
>  export CONFIG_PAGE_OFFSET CONFIG_MEMORY_START CONFIG_BOOT_LINK_OFFSET \
>         CONFIG_PHYSICAL_START CONFIG_ZERO_PAGE_OFFSET CONFIG_ENTRY_OFFSET \
> -       KERNEL_MEMORY suffix-y
> +       KERNEL_MEMORY suffix_y
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index cf3174df7859..c1eb9a62de55 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -64,5 +64,5 @@ OBJCOPYFLAGS += -R .empty_zero_page
>  
>  LDFLAGS_piggy.o := -r --format binary --oformat $(ld-bfd) -T
>  
> -$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix-y) FORCE
> +$(obj)/piggy.o: $(obj)/vmlinux.scr $(obj)/vmlinux.bin.$(suffix_y) FORCE
>  	$(call if_changed,ld)
> -- 
> 2.32.0
>