| Message ID | 20231218080127.907460-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | efi/libstub: zboot: do not use $(shell ...) in cmd_copy_and_pad | expand |
On Mon, 18 Dec 2023 at 09:01, Masahiro Yamada <masahiroy@kernel.org> wrote: > > You do not need to use $(shell ...) in recipe lines, as they are already > executed in a shell. An alternative solution is $$(...), which is an > escaped sequence of the shell's command substituion, $(...). > > For this case, there is a reason to avoid $(shell ...). > > Kbuild detects command changes by using the if_changed macro, which > compares the previous command recorded in .*.cmd with the current > command from Makefile. If they differ, Kbuild re-runs the build rule. > > To diff the commands, Make must expand $(shell ...) first. It means that > hexdump is executed every time, even when nothing needs rebuilding. If > Kbuild determines that vmlinux.bin needs rebuilding, hexdump will be > executed again to evaluate the 'cmd' macro, one more time to really > build vmlinux.bin, and finally yet again to record the expanded command > into .*.cmd. > > Replace $(shell ...) with $$(...) to avoid multiple, unnecessay shell > evaluations. Since Make is agnostic about the shell code, $(...), the > if_changed macro compares the string "$(hexdump -s16 -n4 ...)" verbatim, > so hexdump is run only for building vmlinux.bin. > > For the same reason, $(shell ...) in EFI_ZBOOT_OBJCOPY_FLAGS should be > eliminated. > > While I was here, I replaced '&&' with ';' because a command for > if_changed is executed with 'set -e'. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Ard Biesheuvel <ardb@kernel.org> > --- > > arch/arm64/boot/Makefile | 2 +- > drivers/firmware/efi/libstub/Makefile.zboot | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile > index 1761f5972443..a5a787371117 100644 > --- a/arch/arm64/boot/Makefile > +++ b/arch/arm64/boot/Makefile > @@ -44,7 +44,7 @@ EFI_ZBOOT_BFD_TARGET := elf64-littleaarch64 > EFI_ZBOOT_MACH_TYPE := ARM64 > EFI_ZBOOT_FORWARD_CFI := $(CONFIG_ARM64_BTI_KERNEL) > > -EFI_ZBOOT_OBJCOPY_FLAGS = --add-symbol zboot_code_size=0x$(shell \ > +EFI_ZBOOT_OBJCOPY_FLAGS = --add-symbol zboot_code_size=0x$$( \ > $(NM) vmlinux|grep _kernel_codesize|cut -d' ' -f1) > > include $(srctree)/drivers/firmware/efi/libstub/Makefile.zboot > diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot > index 2c489627a807..65ffd0b760b2 100644 > --- a/drivers/firmware/efi/libstub/Makefile.zboot > +++ b/drivers/firmware/efi/libstub/Makefile.zboot > @@ -5,8 +5,8 @@ > # EFI_ZBOOT_FORWARD_CFI > > quiet_cmd_copy_and_pad = PAD $@ > - cmd_copy_and_pad = cp $< $@ && \ > - truncate -s $(shell hexdump -s16 -n4 -e '"%u"' $<) $@ > + cmd_copy_and_pad = cp $< $@; \ > + truncate -s $$(hexdump -s16 -n4 -e '"%u"' $<) $@ > > # Pad the file to the size of the uncompressed image in memory, including BSS > $(obj)/vmlinux.bin: $(obj)/$(EFI_ZBOOT_PAYLOAD) FORCE > -- > 2.40.1 > >
On Mon, 18 Dec 2023 17:01:27 +0900, Masahiro Yamada wrote: > You do not need to use $(shell ...) in recipe lines, as they are already > executed in a shell. An alternative solution is $$(...), which is an > escaped sequence of the shell's command substituion, $(...). > > For this case, there is a reason to avoid $(shell ...). > > Kbuild detects command changes by using the if_changed macro, which > compares the previous command recorded in .*.cmd with the current > command from Makefile. If they differ, Kbuild re-runs the build rule. > > [...] Applied to arm64 (for-next/kbuild), thanks! [1/1] efi/libstub: zboot: do not use $(shell ...) in cmd_copy_and_pad https://git.kernel.org/arm64/c/97ba4416d6dd Cheers,
diff --git a/arch/arm64/boot/Makefile b/arch/arm64/boot/Makefile index 1761f5972443..a5a787371117 100644 --- a/arch/arm64/boot/Makefile +++ b/arch/arm64/boot/Makefile @@ -44,7 +44,7 @@ EFI_ZBOOT_BFD_TARGET := elf64-littleaarch64 EFI_ZBOOT_MACH_TYPE := ARM64 EFI_ZBOOT_FORWARD_CFI := $(CONFIG_ARM64_BTI_KERNEL) -EFI_ZBOOT_OBJCOPY_FLAGS = --add-symbol zboot_code_size=0x$(shell \ +EFI_ZBOOT_OBJCOPY_FLAGS = --add-symbol zboot_code_size=0x$$( \ $(NM) vmlinux|grep _kernel_codesize|cut -d' ' -f1) include $(srctree)/drivers/firmware/efi/libstub/Makefile.zboot diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot index 2c489627a807..65ffd0b760b2 100644 --- a/drivers/firmware/efi/libstub/Makefile.zboot +++ b/drivers/firmware/efi/libstub/Makefile.zboot @@ -5,8 +5,8 @@ # EFI_ZBOOT_FORWARD_CFI quiet_cmd_copy_and_pad = PAD $@ - cmd_copy_and_pad = cp $< $@ && \ - truncate -s $(shell hexdump -s16 -n4 -e '"%u"' $<) $@ + cmd_copy_and_pad = cp $< $@; \ + truncate -s $$(hexdump -s16 -n4 -e '"%u"' $<) $@ # Pad the file to the size of the uncompressed image in memory, including BSS $(obj)/vmlinux.bin: $(obj)/$(EFI_ZBOOT_PAYLOAD) FORCE
You do not need to use $(shell ...) in recipe lines, as they are already executed in a shell. An alternative solution is $$(...), which is an escaped sequence of the shell's command substituion, $(...). For this case, there is a reason to avoid $(shell ...). Kbuild detects command changes by using the if_changed macro, which compares the previous command recorded in .*.cmd with the current command from Makefile. If they differ, Kbuild re-runs the build rule. To diff the commands, Make must expand $(shell ...) first. It means that hexdump is executed every time, even when nothing needs rebuilding. If Kbuild determines that vmlinux.bin needs rebuilding, hexdump will be executed again to evaluate the 'cmd' macro, one more time to really build vmlinux.bin, and finally yet again to record the expanded command into .*.cmd. Replace $(shell ...) with $$(...) to avoid multiple, unnecessay shell evaluations. Since Make is agnostic about the shell code, $(...), the if_changed macro compares the string "$(hexdump -s16 -n4 ...)" verbatim, so hexdump is run only for building vmlinux.bin. For the same reason, $(shell ...) in EFI_ZBOOT_OBJCOPY_FLAGS should be eliminated. While I was here, I replaced '&&' with ';' because a command for if_changed is executed with 'set -e'. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- arch/arm64/boot/Makefile | 2 +- drivers/firmware/efi/libstub/Makefile.zboot | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)