Re: [RFC PATCH 1/2] objtool: Run objtool only if either of the config options are selected

Masahiro Yamada
2024-05-06T17:40:04Z

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

On Tue, Apr 23, 2024 at 1:19 AM Sathvika Vasireddy
<sv@linux.vnet.ibm.com> wrote:
>
> Hi Masahiro, thanks for reviewing.
>
> On 4/22/24 5:39 PM, Masahiro Yamada wrote:
>
> On Mon, Apr 22, 2024 at 6:25 PM Sathvika Vasireddy <sv@linux.ibm.com> wrote:
>
> Currently, when objtool is enabled and none of the supported options
> are triggered, kernel build errors out with the below error:
> error: objtool: At least one command required.
>
> Then, I think CONFIG_OBJTOOL should be disabled.
>
> A subsequent patch introduces --ftr-fixup as an option to objtool to do feature fixup at build-time via CONFIG_HAVE_OBJTOOL_FTR_FIXUP option. If CONFIG_OBJTOOL is not selected, then objtool cannot be used to pass --ftr-fixup option.
>
> In cases where none of the supported options (like --mcount on powerpc for example) is triggered, but still require --ftr-fixup option to be passed to objtool, we see "error: objtool: At least one command required" errors. So, to address this, run only when either of the config options are selected.
>
> Thanks,
> Sathvika

Same as my first comment.

Bad things happen because you select OBJTOOL.

Preferably, this should be a separate program
as in the first draft, but if you insist on
integrating it into objtool, I recommend keeping
CONFIG_OBJTOOL and CONFIG_HAVE_OBJTOOL_FTR_FIXUP
as separate, unlated options.

I attach a fix-up patch applicable on top of your work.

--
Best Regards
Masahiro Yamada

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 2947 bytes --]

diff --git a/Makefile b/Makefile
index 40fb2ca6fe4c..c5ac01274893 100644
--- a/Makefile
+++ b/Makefile
@@ -1327,6 +1327,13 @@ ifdef CONFIG_OBJTOOL
prepare: tools/objtool
endif

+# CONFIG_OBJTOOL and CONFIG_HAVE_OBJTOOL_FTR_FIXUP are unrelated, separate
+# options. It was integrated in objtool in order to borrow the elf parser,
+# but this is different from how the other objtool commands are used.
+ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
+prepare: tools/objtool
+endif
+
ifdef CONFIG_BPF
ifdef CONFIG_DEBUG_INFO_BTF
prepare: tools/bpf/resolve_btfids
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 806285a28231..564b73cbfa3d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -26,7 +26,7 @@ config 64BIT
config HAVE_OBJTOOL_FTR_FIXUP
bool
default y if CPU_LITTLE_ENDIAN && PPC64
- select OBJTOOL
+ # HAVE_OBJTOOL_FTR_FIXUP must not select OBJTOOL

config LIVEPATCH_64
def_bool PPC64
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8fff27b9bdcb..855ad097f85e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -257,10 +257,10 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__

-ifdef CONFIG_OBJTOOL
-
objtool := $(objtree)/tools/objtool/objtool

+ifdef CONFIG_OBJTOOL
+
objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK) += --hacks=jump_label
objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr
objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING) += --hacks=skylake
@@ -286,16 +286,7 @@ objtool-args = $(objtool-args-y) \

delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT))

-ifneq ($(objtool-args-y),)
cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool-args) $@)
-endif
-
-cmd_objtool_vmlinux :=
-ifeq ($(CONFIG_HAVE_OBJTOOL_FTR_FIXUP),y)
-cmd_objtool_vmlinux = $(if $(objtool-enabled), ; $(objtool) $(objtool-args) $@)
-vmlinux:
- $(cmd_objtool_vmlinux)
-endif

cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)

diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 2f4a7154e676..f02f99c6f355 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -58,10 +58,9 @@ existing-targets := $(wildcard $(sort $(targets)))
# ----------------------------------
#
# For feature fixup, objtool does not run on individual
-# translation units. Run this on vmlinux instead.
+# translation units. Run this on vmlinux instead. Only for PowerPC.
+# The other objtool commands work on individual objects or vmlinux.o.

-objtool-enabled := $(CONFIG_HAVE_OBJTOOL_FTR_FIXUP)
-
-vmlinux-objtool-args-$(CONFIG_HAVE_OBJTOOL_FTR_FIXUP) += --ftr-fixup
-
-objtool-args = $(vmlinux-objtool-args-y) --link
+ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
+cmd_objtool_vmlinux = ; $(objtool) --ftr-fixup --link $@
+endif