| Message ID | 20250311110616.148682-10-ardb+git@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/build: Get rid of vmlinux postlink step | expand |
+ Petr for 9d9173e9ceb6 @Petr, you can find the whole thread on lore. On Tue, Mar 11, 2025 at 12:06:21PM +0100, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > Instead of generating the vmlinux.relocs file (needed by the > decompressor build to construct the KASLR relocation tables) as a > vmlinux postlink step, which is dubious because it depends on data that > is stripped from vmlinux before the build completes, generate it from > vmlinux.unstripped, which has been introduced specifically for this > purpose. > > This ensures that each artifact is rebuilt as needed, rather than as a > side effect of another build rule. > > This effectively reverts commit > > 9d9173e9ceb6 ("x86/build: Avoid relocation information in final vmlinux") > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/Makefile.postlink | 38 -------------------- > arch/x86/boot/compressed/Makefile | 9 +++-- > 2 files changed, 6 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink > deleted file mode 100644 > index 445fce66630f..000000000000 > --- a/arch/x86/Makefile.postlink > +++ /dev/null > @@ -1,38 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -# =========================================================================== > -# Post-link x86 pass > -# =========================================================================== > -# > -# 1. Separate relocations from vmlinux into vmlinux.relocs. > -# 2. Strip relocations from vmlinux. > - > -PHONY := __archpost > -__archpost: > - > --include include/config/auto.conf > -include $(srctree)/scripts/Kbuild.include > - > -CMD_RELOCS = arch/x86/tools/relocs > -OUT_RELOCS = arch/x86/boot/compressed > -quiet_cmd_relocs = RELOCS $(OUT_RELOCS)/vmlinux.relocs > - cmd_relocs = \ > - mkdir -p $(OUT_RELOCS); \ > - $(CMD_RELOCS) $@ > $(OUT_RELOCS)/vmlinux.relocs; \ > - $(CMD_RELOCS) --abs-relocs $@ > - > -# `@true` prevents complaint when there is nothing to be done > - > -vmlinux vmlinux.unstripped: FORCE > - @true > -ifeq ($(CONFIG_X86_NEED_RELOCS),y) > - $(call cmd,relocs) > -endif > - > -clean: > - @rm -f $(OUT_RELOCS)/vmlinux.relocs > - > -PHONY += FORCE clean > - > -FORCE: > - > -.PHONY: $(PHONY) > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 606c74f27459..5edee7a9786c 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -117,9 +117,12 @@ $(obj)/vmlinux.bin: vmlinux FORCE > > targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs > > -# vmlinux.relocs is created by the vmlinux postlink step. > -$(obj)/vmlinux.relocs: vmlinux > - @true > +CMD_RELOCS = arch/x86/tools/relocs > +quiet_cmd_relocs = RELOCS $@ > + cmd_relocs = $(CMD_RELOCS) $< > $@;$(CMD_RELOCS) --abs-relocs $< > + > +$(obj)/vmlinux.relocs: vmlinux.unstripped FORCE > + $(call if_changed,relocs) > > vmlinux.bin.all-y := $(obj)/vmlinux.bin > vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs > -- > 2.49.0.rc0.332.g42c0ae87b1-goog >
On 3/11/25 12:17, Borislav Petkov wrote: > + Petr for 9d9173e9ceb6 > > @Petr, you can find the whole thread on lore. > > On Tue, Mar 11, 2025 at 12:06:21PM +0100, Ard Biesheuvel wrote: >> From: Ard Biesheuvel <ardb@kernel.org> >> >> Instead of generating the vmlinux.relocs file (needed by the >> decompressor build to construct the KASLR relocation tables) as a >> vmlinux postlink step, which is dubious because it depends on data that >> is stripped from vmlinux before the build completes, generate it from >> vmlinux.unstripped, which has been introduced specifically for this >> purpose. >> >> This ensures that each artifact is rebuilt as needed, rather than as a >> side effect of another build rule. >> >> This effectively reverts commit >> >> 9d9173e9ceb6 ("x86/build: Avoid relocation information in final vmlinux") >> >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> For what it's worth, the changes look to me as an improvement over the previous implementation.
On Tue, Mar 11, 2025 at 8:06 PM Ard Biesheuvel <ardb+git@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Instead of generating the vmlinux.relocs file (needed by the > decompressor build to construct the KASLR relocation tables) as a > vmlinux postlink step, which is dubious because it depends on data that > is stripped from vmlinux before the build completes, generate it from > vmlinux.unstripped, which has been introduced specifically for this > purpose. > > This ensures that each artifact is rebuilt as needed, rather than as a > side effect of another build rule. > > This effectively reverts commit > > 9d9173e9ceb6 ("x86/build: Avoid relocation information in final vmlinux") > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/Makefile.postlink | 38 -------------------- > arch/x86/boot/compressed/Makefile | 9 +++-- > 2 files changed, 6 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink > deleted file mode 100644 > index 445fce66630f..000000000000 > --- a/arch/x86/Makefile.postlink > +++ /dev/null > @@ -1,38 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -# =========================================================================== > -# Post-link x86 pass > -# =========================================================================== > -# > -# 1. Separate relocations from vmlinux into vmlinux.relocs. > -# 2. Strip relocations from vmlinux. > - > -PHONY := __archpost > -__archpost: > - > --include include/config/auto.conf > -include $(srctree)/scripts/Kbuild.include > - > -CMD_RELOCS = arch/x86/tools/relocs > -OUT_RELOCS = arch/x86/boot/compressed > -quiet_cmd_relocs = RELOCS $(OUT_RELOCS)/vmlinux.relocs > - cmd_relocs = \ > - mkdir -p $(OUT_RELOCS); \ > - $(CMD_RELOCS) $@ > $(OUT_RELOCS)/vmlinux.relocs; \ > - $(CMD_RELOCS) --abs-relocs $@ > - > -# `@true` prevents complaint when there is nothing to be done > - > -vmlinux vmlinux.unstripped: FORCE > - @true > -ifeq ($(CONFIG_X86_NEED_RELOCS),y) > - $(call cmd,relocs) > -endif > - > -clean: > - @rm -f $(OUT_RELOCS)/vmlinux.relocs > - > -PHONY += FORCE clean > - > -FORCE: > - > -.PHONY: $(PHONY) > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 606c74f27459..5edee7a9786c 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -117,9 +117,12 @@ $(obj)/vmlinux.bin: vmlinux FORCE > > targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs > > -# vmlinux.relocs is created by the vmlinux postlink step. > -$(obj)/vmlinux.relocs: vmlinux > - @true > +CMD_RELOCS = arch/x86/tools/relocs > +quiet_cmd_relocs = RELOCS $@ > + cmd_relocs = $(CMD_RELOCS) $< > $@;$(CMD_RELOCS) --abs-relocs $< > + > +$(obj)/vmlinux.relocs: vmlinux.unstripped FORCE > + $(call if_changed,relocs) Perhaps, it may make sense to rebuild vmlinux.relocs when arch/x86/tools/relocs is changed, but I do not see such dependency in the other arch/x86/realmode/rm/Makefile. https://github.com/torvalds/linux/blob/v6.14-rc5/arch/x86/realmode/rm/Makefile#L61 So, I decided it is ok. If you mind, you can send v3.
On Thu, 13 Mar 2025 at 03:09, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Mar 11, 2025 at 8:06 PM Ard Biesheuvel <ardb+git@google.com> wrote: > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Instead of generating the vmlinux.relocs file (needed by the > > decompressor build to construct the KASLR relocation tables) as a > > vmlinux postlink step, which is dubious because it depends on data that > > is stripped from vmlinux before the build completes, generate it from > > vmlinux.unstripped, which has been introduced specifically for this > > purpose. > > > > This ensures that each artifact is rebuilt as needed, rather than as a > > side effect of another build rule. > > > > This effectively reverts commit > > > > 9d9173e9ceb6 ("x86/build: Avoid relocation information in final vmlinux") > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > arch/x86/Makefile.postlink | 38 -------------------- > > arch/x86/boot/compressed/Makefile | 9 +++-- > > 2 files changed, 6 insertions(+), 41 deletions(-) > > > > diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink > > deleted file mode 100644 > > index 445fce66630f..000000000000 > > --- a/arch/x86/Makefile.postlink > > +++ /dev/null > > @@ -1,38 +0,0 @@ > > -# SPDX-License-Identifier: GPL-2.0 > > -# =========================================================================== > > -# Post-link x86 pass > > -# =========================================================================== > > -# > > -# 1. Separate relocations from vmlinux into vmlinux.relocs. > > -# 2. Strip relocations from vmlinux. > > - > > -PHONY := __archpost > > -__archpost: > > - > > --include include/config/auto.conf > > -include $(srctree)/scripts/Kbuild.include > > - > > -CMD_RELOCS = arch/x86/tools/relocs > > -OUT_RELOCS = arch/x86/boot/compressed > > -quiet_cmd_relocs = RELOCS $(OUT_RELOCS)/vmlinux.relocs > > - cmd_relocs = \ > > - mkdir -p $(OUT_RELOCS); \ > > - $(CMD_RELOCS) $@ > $(OUT_RELOCS)/vmlinux.relocs; \ > > - $(CMD_RELOCS) --abs-relocs $@ > > - > > -# `@true` prevents complaint when there is nothing to be done > > - > > -vmlinux vmlinux.unstripped: FORCE > > - @true > > -ifeq ($(CONFIG_X86_NEED_RELOCS),y) > > - $(call cmd,relocs) > > -endif > > - > > -clean: > > - @rm -f $(OUT_RELOCS)/vmlinux.relocs > > - > > -PHONY += FORCE clean > > - > > -FORCE: > > - > > -.PHONY: $(PHONY) > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > > index 606c74f27459..5edee7a9786c 100644 > > --- a/arch/x86/boot/compressed/Makefile > > +++ b/arch/x86/boot/compressed/Makefile > > @@ -117,9 +117,12 @@ $(obj)/vmlinux.bin: vmlinux FORCE > > > > targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs > > > > -# vmlinux.relocs is created by the vmlinux postlink step. > > -$(obj)/vmlinux.relocs: vmlinux > > - @true > > +CMD_RELOCS = arch/x86/tools/relocs > > +quiet_cmd_relocs = RELOCS $@ > > + cmd_relocs = $(CMD_RELOCS) $< > $@;$(CMD_RELOCS) --abs-relocs $< > > + > > +$(obj)/vmlinux.relocs: vmlinux.unstripped FORCE > > + $(call if_changed,relocs) > > Perhaps, it may make sense to rebuild vmlinux.relocs > when arch/x86/tools/relocs is changed, but > I do not see such dependency in the other > arch/x86/realmode/rm/Makefile. > > https://github.com/torvalds/linux/blob/v6.14-rc5/arch/x86/realmode/rm/Makefile#L61 > > > So, I decided it is ok. > If you mind, you can send v3. > No I think that's fine: this patch reverts the changes to arch/x86/boot/compressed/Makefile; if we need to improve it, it should be a separate change in any case.
diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink deleted file mode 100644 index 445fce66630f..000000000000 --- a/arch/x86/Makefile.postlink +++ /dev/null @@ -1,38 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0 -# =========================================================================== -# Post-link x86 pass -# =========================================================================== -# -# 1. Separate relocations from vmlinux into vmlinux.relocs. -# 2. Strip relocations from vmlinux. - -PHONY := __archpost -__archpost: - --include include/config/auto.conf -include $(srctree)/scripts/Kbuild.include - -CMD_RELOCS = arch/x86/tools/relocs -OUT_RELOCS = arch/x86/boot/compressed -quiet_cmd_relocs = RELOCS $(OUT_RELOCS)/vmlinux.relocs - cmd_relocs = \ - mkdir -p $(OUT_RELOCS); \ - $(CMD_RELOCS) $@ > $(OUT_RELOCS)/vmlinux.relocs; \ - $(CMD_RELOCS) --abs-relocs $@ - -# `@true` prevents complaint when there is nothing to be done - -vmlinux vmlinux.unstripped: FORCE - @true -ifeq ($(CONFIG_X86_NEED_RELOCS),y) - $(call cmd,relocs) -endif - -clean: - @rm -f $(OUT_RELOCS)/vmlinux.relocs - -PHONY += FORCE clean - -FORCE: - -.PHONY: $(PHONY) diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 606c74f27459..5edee7a9786c 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -117,9 +117,12 @@ $(obj)/vmlinux.bin: vmlinux FORCE targets += $(patsubst $(obj)/%,%,$(vmlinux-objs-y)) vmlinux.bin.all vmlinux.relocs -# vmlinux.relocs is created by the vmlinux postlink step. -$(obj)/vmlinux.relocs: vmlinux - @true +CMD_RELOCS = arch/x86/tools/relocs +quiet_cmd_relocs = RELOCS $@ + cmd_relocs = $(CMD_RELOCS) $< > $@;$(CMD_RELOCS) --abs-relocs $< + +$(obj)/vmlinux.relocs: vmlinux.unstripped FORCE + $(call if_changed,relocs) vmlinux.bin.all-y := $(obj)/vmlinux.bin vmlinux.bin.all-$(CONFIG_X86_NEED_RELOCS) += $(obj)/vmlinux.relocs