[2/5] X86-64 should not uniquely require a third ELF package to build.

  x86-64 is the only architecture that can't compile without an extra
ELF library installed on the host. (The kernel already has multiple ELF
parse implementations built-in, so requiring another one is questionable
at best.) You can switch it back on in menuconfig if you want to, this
just stops it being mandatory.

See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html

Signed-off-by: Rob Landley <rob@landley.net>
---
 arch/x86/Kconfig | 1 -
 1 file changed, 1 deletion(-)
  

Rob!

On Tue, Feb 21 2023 at 14:56, Rob Landley wrote:

The subject line is not compliant to documentation. Please read and
follow Documentation/process/* especially submitting-patches.rst and
maintainer-tip.rst

> x86-64 is the only architecture that can't compile without an extra
> ELF library installed on the host. (The kernel already has multiple ELF
> parse implementations built-in, so requiring another one is questionable
> at best.)

How are ELF parsers in the kernel itself related to a build requirement
for a tool, which is part of the kernel build process?

Next time you ask for removal of perl, python or whatever the kernel
requires to build.

> You can switch it back on in menuconfig if you want to, this
> just stops it being mandatory.

How do you switch on CONFIG_HAVE_OBJTOOL in menuconfig?

config HAVE_OBJTOOL
        bool

There is no knob.

> See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
> and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html

Please use https://lore.kernel.org/lkml/ links.

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 3604074a878b..b63510d79baf 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -243,7 +243,6 @@ config X86
>  	select HAVE_NOINSTR_HACK		if HAVE_OBJTOOL
>  	select HAVE_NMI
>  	select HAVE_NOINSTR_VALIDATION		if HAVE_OBJTOOL
> -	select HAVE_OBJTOOL			if X86_64

This prevents runtime features, optimizations, mitigations and build
time validations rom being selected as you can see from your patch
context.

Just to be clear: objtool is mandatory for x86_64 builds.

git grep 'select HAVE_OBJTOOL' will tell you that your claim about
x86_64 being the only architecture is slightly wrong.

Thanks,

        tglx
  

On 2/21/23 17:12, Thomas Gleixner wrote:
> Rob!

Thomas!

> On Tue, Feb 21 2023 at 14:56, Rob Landley wrote:
> 
> The subject line is not compliant to documentation. Please read and
> follow Documentation/process/* especially submitting-patches.rst and
> maintainer-tip.rst

I've read them multiple times over the years, but I'm not good at remembering
extensive bureaucracy over long periods. Let's see...

Are you complaining it doesn't have "RESEND"? These apply to 6.2, not 6.1, and a
couple things got tweaked. What's the "RESEND" threshold?

Or is this a "subsystem:" prefix thing where I have to guess what subsystem the
top level makefile or mktimeconst fall under? Is "init" a subsystem?

$ grep -i subsystem MAINTAINERS  | wc
     85     286    2216
$ grep -i subsystem MAINTAINERS  | grep -i init
$

Apparently not. I would have thought get_maintainer.pl would emit this sort of
info if it's important, but I guess I'm remembering back when Linux had
hobbyists who worked on things that didn't require justifying the time on a
spreadsheet to their boss in a budget allocation meeting before modifying any
code...

>> x86-64 is the only architecture that can't compile without an extra
>> ELF library installed on the host. (The kernel already has multiple ELF
>> parse implementations built-in, so requiring another one is questionable
>> at best.)
> 
> How are ELF parsers in the kernel itself related to a build requirement
> for a tool, which is part of the kernel build process?

The project already has multiple instances of code that traverses ELF data
structures. A definition of "code reuse" that pulls in an additional build
dependency which the project was not already using is just "code use" without a
"re".

My response was "this doesn't need to be done at all, it's just being used for
an optional stack unwinder, and even if you _want_ a stack unwinder there are
multiple other implementations without this dependency".

They just tangled up the kconfig plubmbing so even when nothing uses it, it
still tries to build the tool it won't run. If I'd tried to FIX the tool,
factoring out existing ELF code so it could be built on the host and used at
build time might have made sense. But the thing it's being used to do is not a
thing I need on the systems I'm building.

> Next time you ask for removal of perl, python or whatever the kernel
> requires to build.

Kernel doesn't require python to build, my perl removal series got merged 10
years ago, and other people have sent follow-up perl removal patches since.

>> You can switch it back on in menuconfig if you want to, this
>> just stops it being mandatory.
> 
> How do you switch on CONFIG_HAVE_OBJTOOL in menuconfig?
> 
> config HAVE_OBJTOOL
>         bool
> 
> There is no knob.

Sigh, I traced through all this stuff at one point. It's been a few releases
since then (and the symbol got renamed), and it looks like it's developed some
more dependencies off of it in kernel/trace/Kconfig.

Alright, how about this:

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..70923305d596 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -243,7 +243,8 @@ config X86
        select HAVE_NOINSTR_HACK                if HAVE_OBJTOOL
        select HAVE_NMI
        select HAVE_NOINSTR_VALIDATION          if HAVE_OBJTOOL
-       select HAVE_OBJTOOL                     if X86_64
+       select HAVE_OBJTOOL                     if X86_64 && \
+         $(success,echo "#include <gelf.h>" | $(HOSTCC) -xc -o /dev/null -c -)
        select HAVE_OPTPROBES
        select HAVE_PCSPKR_PLATFORM
        select HAVE_PERF_EVENTS

Actually TEST if the build environment has it, and set the symbol appropriately.

Would that with "test for optional build dependency" work? I'm aware people want
to use this plumbing for more thorough spectre/meltdown style mitigations, but
Linux has CONFIG_MULTIUSER which can remove support for non-root users. (I'm off
at the embedded end of things: we're weird.)

>> See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
>> and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html
> 
> Please use https://lore.kernel.org/lkml/ links.

Yeah, check_patch.pl complained about that too. Google's getting unreliable
enough it's not always easy to map between them, but I'll put it on the todo
list for the 6.3 repost of the series...

>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 3604074a878b..b63510d79baf 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -243,7 +243,6 @@ config X86
>>  	select HAVE_NOINSTR_HACK		if HAVE_OBJTOOL
>>  	select HAVE_NMI
>>  	select HAVE_NOINSTR_VALIDATION		if HAVE_OBJTOOL
>> -	select HAVE_OBJTOOL			if X86_64
> 
> This prevents runtime features, optimizations, mitigations and build
> time validations rom being selected as you can see from your patch
> context.
> 
> Just to be clear: objtool is mandatory for x86_64 builds.

Because without this patch it's forced on, yes. Works fine without it in my testing?

The llvm guys built a whole compiler to get away from GPLv3 and
https://sourceware.org/elfutils is GPLv3, which is not _my_ motivation for
trying to avoid it, but I can see it being "of interest". (If the goal is to
push people away from x86-64 faster...)

No other architecture I tested had this requirement. I admit I'm only building
arm, arm64, x86, x86-64, m68k, mips, ppc32, ppc64, s390x, and superh kernels at
the moment. I'm only doing a userspace version of hexagon because they didn't
have qemu-system-hexagon working last I poked Taylor Simpson so I'd have nothing
to test the kernel on. Musl-libc's xtensa support is not just an out of tree
fork but _very_stale_ (the one I have, anyway). Musl doesn't support
alpha/arc/csky/itanic/mips-looongarch/nios2/parisc/sparc yet. Microblaze
binaries segfault in the ELF _start code for some reason (I should track down
why). I'm halfway to a working cortex-m qemu config and need to get back to that
(qemu SHOULD have a decent board, there's like 5 options now). Building or1k is
on the todo list...

> git grep 'select HAVE_OBJTOOL' will tell you that your claim about
> x86_64 being the only architecture is slightly wrong.

There's two hits, and the other is PPC32, which was building fine for me without
this patch last I checked?

Ah, commit e52ec98c5ab1 from 3 months ago. So 6.1 built fine without it but 6.2
also requires... no, hang on, I test built 6.2 with this patch series more than
once, and my patches don't modify the arch/powerpc directory? I still have the
binaries:

$ ./run-qemu.sh
...
Kernel memory protection not selected by kernel config.
Run /init as init process
Type exit when done.
# cat /proc/version
Linux version 6.2.0 (landley@driftwood) (powerpc-linux-musl-cc (GCC) 9.4.0, GNU
ld (GNU Binutils) 2.33.1) #1 Tue Feb 21 14:07:21 CST 2023
# cat /proc/cpuinfo
processor	: 0
cpu		: 740/750
clock		: 266.000000MHz
revision	: 3.1 (pvr 0008 0301)
bogomips	: 33.20

timebase	: 16603616
platform	: PowerMac
model		: Power Macintosh
machine		: Power Macintosh
motherboard	: AAPL,PowerMac G3 MacRISC
detected as	: 49 (PowerMac G3 (Silk))
pmac flags	: 00000000
pmac-generation	: OldWorld
Memory		: 256 MB
# file /bin/toybox
/bin/toybox: ELF executable, 32-bit MSB ppc, static, stripped


That's 6.2, G3 was 7xx series which is 32 bit... Yup, powerpc 32 bit built and
ran fine without this. I haven't got libelf-dev installed on my laptop, so it
CAN'T have required it,the build would break if it tried...

Which other architecture build breaks for you without libelf-dev installed?

> Thanks,
> 
>         tglx

Rob
  

Rob!

On Wed, Feb 22 2023 at 12:14, Rob Landley wrote:
> On 2/21/23 17:12, Thomas Gleixner wrote:
>> The subject line is not compliant to documentation. Please read and
>> follow Documentation/process/* especially submitting-patches.rst and
>> maintainer-tip.rst
>
> I've read them multiple times over the years, but I'm not good at remembering
> extensive bureaucracy over long periods. Let's see...
>
> Are you complaining it doesn't have "RESEND"? These apply to 6.2, not 6.1, and a
> couple things got tweaked. What's the "RESEND" threshold?
>
> Or is this a "subsystem:" prefix thing where I have to guess what subsystem the
> top level makefile or mktimeconst fall under? Is "init" a subsystem?
>
> $ grep -i subsystem MAINTAINERS  | wc
>      85     286    2216
> $ grep -i subsystem MAINTAINERS  | grep -i init
> $

What has a patch against arch/x86 to do with init? I gave you two files
which extensively describe what we ask people to do.

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-subject

Do you need some more explicit links?

These rules have been set in place to make the maintainer work
scalable. If you ignore them, then you are wasting other peoples
time. The logical consequence is that those people will ignore you too.

> Apparently not. I would have thought get_maintainer.pl would emit this sort of
> info if it's important, but I guess I'm remembering back when Linux had
> hobbyists who worked on things that didn't require justifying the time on a
> spreadsheet to their boss in a budget allocation meeting before modifying any
> code...

Your unrelated and non-sensical rants are not getting you anywhere.

>>> x86-64 is the only architecture that can't compile without an extra
>>> ELF library installed on the host. (The kernel already has multiple ELF
>>> parse implementations built-in, so requiring another one is questionable
>>> at best.)
>> 
>> How are ELF parsers in the kernel itself related to a build requirement
>> for a tool, which is part of the kernel build process?
>
> The project already has multiple instances of code that traverses ELF data
> structures. A definition of "code reuse" that pulls in an additional build
> dependency which the project was not already using is just "code use" without a
> "re".

Feel free to send patches against objtool to make use of those multiple
instances, which you gracefully omit to name.

> My response was "this doesn't need to be done at all, it's just being used for
> an optional stack unwinder, and even if you _want_ a stack unwinder there are
> multiple other implementations without this dependency".

Care to figure out what objtool really does and why it is required to
enable certain features? You might not care, but most people do for very
good reasons.

>> Just to be clear: objtool is mandatory for x86_64 builds.
>
> Because without this patch it's forced on, yes. Works fine without it
> in my testing?

Works for me does not cut it and you know that.

>> git grep 'select HAVE_OBJTOOL' will tell you that your claim about
>> x86_64 being the only architecture is slightly wrong.
>
> There's two hits, and the other is PPC32, which was building fine for me without
> this patch last I checked?

Select the right options on PPC32 or PPC64 and it requires objtool.

Just accept it. Your patch _is_ broken and it's not my problem to come
up with a solution which causes regressions for people who care and
depend on the features. It's just more work than removing a single line
in a Kconfig file and claiming that uncomprehensible word salad
qualifies as change log.

Please stop this right here and come back when you have something which
is worth to look at. I gave you enough pointers by now and I'm not going
to waste another second of my time on reading your rants.

Thanks,

        tglx