[PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Jan Beulich]

While LLVM's lld is supposed to be a drop-in replacement for GNU ld [1],
it appears to not understand quoted section names as operands to e.g.
ADDR(). Therefore the original workaround broke the build in
environments where ld is actually LLVM's, like on FreeBSD.

Fixes: 58ad654ebce7 ("x86: work around build issue with GNU ld 2.37")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

[1] https://lld.llvm.org/
---
I haven't been able to find an environment where I could actually try
with lld (ld.lld); all testing was with GNU ld (ld.bfd).

--- a/.gitignore
+++ b/.gitignore
@@ -306,6 +306,7 @@
 xen/.config.old
 xen/.xen.elf32
 xen/System.map
+xen/arch/x86/.check.*
 xen/arch/x86/asm-macros.i
 xen/arch/x86/boot/mkelf32
 xen/arch/x86/boot/cmdline.S
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
 
 ifneq ($(build_id_linker),)
 notes_phdrs = --notes
+# Determine whether to engage a workaround for GNU ld 2.37.
+build-id-ld-good = $(shell echo 'void test(void) {}' \
+                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
+                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
+                           && echo y)
 else
 ifeq ($(CONFIG_PVH_GUEST),y)
 notes_phdrs = --notes
 endif
+build-id-ld-good := y
 endif
 
 ifdef CONFIG_LIVEPATCH
@@ -291,6 +297,10 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
 	$(call move-if-changed,$@.new,$@)
 
 efi.lds: AFLAGS-y += -DEFI
+xen.lds: Makefile
+ifneq ($(build-id-ld-good),y)
+xen.lds: CFLAGS-y += -DQUOTE_SECTION_NAMES
+endif
 xen.lds efi.lds: xen.lds.S
 	$(CPP) -P $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
 
@@ -302,7 +312,7 @@ hweight.o: CFLAGS-y += $(foreach reg,cx
 
 .PHONY: clean
 clean::
-	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
+	rm -f ???.lds *.new .check.* boot/*.o boot/*~ boot/core boot/mkelf32
 	rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
 	rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
 	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
--- /dev/null
+++ b/xen/arch/x86/check.lds
@@ -0,0 +1,11 @@
+/*
+ * Minimal linker script used to check whether the linker understands section
+ * names containing a dash (like in .note.gnu.build-id) inside ADDR() and alike
+ * without quoting them.
+ */
+OUTPUT_FORMAT("elf64-x86-64")
+OUTPUT_ARCH("i386:x86-64")
+SECTIONS
+{
+ .text-1 : AT(ADDR(.text-1)) { *(.text) }
+}
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -18,7 +18,12 @@ ENTRY(efi_start)
 #else /* !EFI */
 
 #define FORMAT "elf64-x86-64"
+
+#if defined(QUOTE_SECTION_NAMES) /* GNU ld 2.37 workaround */
 #define DECL_SECTION(x) x : AT(ADDR(#x) - __XEN_VIRT_START)
+#else
+#define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START)
+#endif
 
 ENTRY(start_pa)
 

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Roger Pau Monné]

On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
> While LLVM's lld is supposed to be a drop-in replacement for GNU ld [1],
> it appears to not understand quoted section names as operands to e.g.
> ADDR(). Therefore the original workaround broke the build in
> environments where ld is actually LLVM's, like on FreeBSD.
> 
> Fixes: 58ad654ebce7 ("x86: work around build issue with GNU ld 2.37")
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> [1] https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flld.llvm.org%2F&amp;data=04%7C01%7Croger.pau%40citrix.com%7C07abc3240fc14a3f095408d9739f3eb3%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637667950073151096%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Sa0BBWP2cWOhWsB3b4FQIZ1J3KvfWP%2BnINiqyXdChD0%3D&amp;reserved=0
> ---
> I haven't been able to find an environment where I could actually try
> with lld (ld.lld); all testing was with GNU ld (ld.bfd).

Thanks for fixing this. I've been able to test with LLVM ld and the
workaround is fine.

> --- a/.gitignore
> +++ b/.gitignore
> @@ -306,6 +306,7 @@
>  xen/.config.old
>  xen/.xen.elf32
>  xen/System.map
> +xen/arch/x86/.check.*
>  xen/arch/x86/asm-macros.i
>  xen/arch/x86/boot/mkelf32
>  xen/arch/x86/boot/cmdline.S
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
>  
>  ifneq ($(build_id_linker),)
>  notes_phdrs = --notes
> +# Determine whether to engage a workaround for GNU ld 2.37.
> +build-id-ld-good = $(shell echo 'void test(void) {}' \
> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
> +                           && echo y)

Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
then use is here?

We already have compiler and assembler checks in x86/Kconfig, so it
would seem more natural to place it there.

>  else
>  ifeq ($(CONFIG_PVH_GUEST),y)
>  notes_phdrs = --notes
>  endif
> +build-id-ld-good := y
>  endif

I also wonder whether we need to make the quoting tied to the usage of
build-id. I guess we don't add sections with dashes and instead
use underscores, but it might be prudent to always quote to be on the
safe side if dashes are not supported.

>  
>  ifdef CONFIG_LIVEPATCH
> @@ -291,6 +297,10 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
>  	$(call move-if-changed,$@.new,$@)
>  
>  efi.lds: AFLAGS-y += -DEFI
> +xen.lds: Makefile
> +ifneq ($(build-id-ld-good),y)
> +xen.lds: CFLAGS-y += -DQUOTE_SECTION_NAMES
> +endif
>  xen.lds efi.lds: xen.lds.S
>  	$(CPP) -P $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
>  
> @@ -302,7 +312,7 @@ hweight.o: CFLAGS-y += $(foreach reg,cx
>  
>  .PHONY: clean
>  clean::
> -	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
> +	rm -f ???.lds *.new .check.* boot/*.o boot/*~ boot/core boot/mkelf32
>  	rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
>  	rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
>  	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
> --- /dev/null
> +++ b/xen/arch/x86/check.lds

I would maybe name this check-dash.lds, in case we need to add more ld
build tests.

Thanks, Roger.

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Jan Beulich]

On 13.09.2021 15:37, Roger Pau Monné wrote:
> On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
>> I haven't been able to find an environment where I could actually try
>> with lld (ld.lld); all testing was with GNU ld (ld.bfd).
> 
> Thanks for fixing this. I've been able to test with LLVM ld and the
> workaround is fine.

Oh, good, thanks for trying this out.

>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
>>  
>>  ifneq ($(build_id_linker),)
>>  notes_phdrs = --notes
>> +# Determine whether to engage a workaround for GNU ld 2.37.
>> +build-id-ld-good = $(shell echo 'void test(void) {}' \
>> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
>> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
>> +                           && echo y)
> 
> Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
> then use is here?
> 
> We already have compiler and assembler checks in x86/Kconfig, so it
> would seem more natural to place it there.

The question of whether to record tool chain capabilities in .config
is still pending. I'm not convinced this is a good idea, Andrew keeps
shouting me out for that, and an actual discussion doesn't really
happen. Yet unlike back at the time when I first raised my concern,
Anthony meanwhile supports me in at least the question (to Andrew) of
when such a discussion would have happened: Neither of us is aware,
yet Andrew claims it did happen, but so far didn't point out where
one could read about what was discussed and decided there.

For the few uses we've accumulated I gave (if at all) an ack for
things happening under some sort of pressure, with the request that
aformentioned discussion would happen subsequently (and, depending on
outcome, these would be converted to another approach if need be). I
have meanwhile realized that it was a mistake to allow such things in
on this basis - the more of them we gain, the more I'm hearing "we've
already got some".

>>  else
>>  ifeq ($(CONFIG_PVH_GUEST),y)
>>  notes_phdrs = --notes
>>  endif
>> +build-id-ld-good := y
>>  endif
> 
> I also wonder whether we need to make the quoting tied to the usage of
> build-id. I guess we don't add sections with dashes and instead
> use underscores, but it might be prudent to always quote to be on the
> safe side if dashes are not supported.

If quoting was uniformly supported, I might have considered that. But
it not being uniformly supported is the reason for this change in the
first place. Hence I'd prefer to generalize this only if really needed.

>> --- /dev/null
>> +++ b/xen/arch/x86/check.lds
> 
> I would maybe name this check-dash.lds, in case we need to add more ld
> build tests.

I sincerely hope it was a one-off that a binutils release got cut with
this sort of a supposedly prominent bug. Considering that the dash is
merely what we're after in this specific case, but breakage was wider
(presumably about any printable char that's not alnum or underscore),
I'd consider check-dash too specific a name. You only say "maybe"; if
you were sufficiently convinced, this is an adjustment I'd be willing
to make. Yet even better would be if I / we could just be done with
this.

Jan

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Roger Pau Monné]

On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
>  .PHONY: clean
>  clean::
> -	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
> +	rm -f ???.lds *.new .check.* boot/*.o boot/*~ boot/core boot/mkelf32

Forgot to mention in my previous reply, but what's the point of using
??? instead of just typing xen.lds?

AFAICT there are no other .lds artifacts, so doing a 3-letter .lds
wildcard seems weird.

Thanks, Roger.

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Roger Pau Monné]

On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
> On 13.09.2021 15:37, Roger Pau Monné wrote:
> > On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
> >> I haven't been able to find an environment where I could actually try
> >> with lld (ld.lld); all testing was with GNU ld (ld.bfd).
> > 
> > Thanks for fixing this. I've been able to test with LLVM ld and the
> > workaround is fine.
> 
> Oh, good, thanks for trying this out.
> 
> >> --- a/xen/arch/x86/Makefile
> >> +++ b/xen/arch/x86/Makefile
> >> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
> >>  
> >>  ifneq ($(build_id_linker),)
> >>  notes_phdrs = --notes
> >> +# Determine whether to engage a workaround for GNU ld 2.37.
> >> +build-id-ld-good = $(shell echo 'void test(void) {}' \
> >> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
> >> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
> >> +                           && echo y)
> > 
> > Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
> > then use is here?
> > 
> > We already have compiler and assembler checks in x86/Kconfig, so it
> > would seem more natural to place it there.
> 
> The question of whether to record tool chain capabilities in .config
> is still pending. I'm not convinced this is a good idea, Andrew keeps
> shouting me out for that, and an actual discussion doesn't really
> happen. Yet unlike back at the time when I first raised my concern,
> Anthony meanwhile supports me in at least the question (to Andrew) of
> when such a discussion would have happened: Neither of us is aware,
> yet Andrew claims it did happen, but so far didn't point out where
> one could read about what was discussed and decided there.
> 
> For the few uses we've accumulated I gave (if at all) an ack for
> things happening under some sort of pressure, with the request that
> aformentioned discussion would happen subsequently (and, depending on
> outcome, these would be converted to another approach if need be). I
> have meanwhile realized that it was a mistake to allow such things in
> on this basis - the more of them we gain, the more I'm hearing "we've
> already got some".

I see, that's not an ideal situation from a review PoV, as we don't
have a clear placement for those and that will just cause confusion
(and more work since there are potentially two places to check).

What's the benefit of placing the checks in Kconfig instead of the
Makefiles?

As I understand Kconfig is run only once, while the Makefile could run
the check multiple times.

The downfall of having them in .config is that .config could suddenly
change as tools are updated or as it's moved around different systems
(yet .config already contains specific compiler version information).

> >>  else
> >>  ifeq ($(CONFIG_PVH_GUEST),y)
> >>  notes_phdrs = --notes
> >>  endif
> >> +build-id-ld-good := y
> >>  endif
> > 
> > I also wonder whether we need to make the quoting tied to the usage of
> > build-id. I guess we don't add sections with dashes and instead
> > use underscores, but it might be prudent to always quote to be on the
> > safe side if dashes are not supported.
> 
> If quoting was uniformly supported, I might have considered that. But
> it not being uniformly supported is the reason for this change in the
> first place. Hence I'd prefer to generalize this only if really needed.

OK, FE. As said I don't we would deliberately add sections with
slashes.

> >> --- /dev/null
> >> +++ b/xen/arch/x86/check.lds
> > 
> > I would maybe name this check-dash.lds, in case we need to add more ld
> > build tests.
> 
> I sincerely hope it was a one-off that a binutils release got cut with
> this sort of a supposedly prominent bug. Considering that the dash is
> merely what we're after in this specific case, but breakage was wider
> (presumably about any printable char that's not alnum or underscore),
> I'd consider check-dash too specific a name. You only say "maybe"; if
> you were sufficiently convinced, this is an adjustment I'd be willing
> to make. Yet even better would be if I / we could just be done with
> this.

Ack, we can always rename at a later point if there's a need to,
hopefully we are not being too optimistic.

Thanks, Roger.

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Jan Beulich]

On 13.09.2021 16:20, Roger Pau Monné wrote:
> On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
>>  .PHONY: clean
>>  clean::
>> -	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
>> +	rm -f ???.lds *.new .check.* boot/*.o boot/*~ boot/core boot/mkelf32
> 
> Forgot to mention in my previous reply, but what's the point of using
> ??? instead of just typing xen.lds?
> 
> AFAICT there are no other .lds artifacts, so doing a 3-letter .lds
> wildcard seems weird.

efi.lds is the 2nd one to match.

Jan

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Jan Beulich]

On 13.09.2021 16:33, Roger Pau Monné wrote:
> On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
>> On 13.09.2021 15:37, Roger Pau Monné wrote:
>>> On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
>>>> I haven't been able to find an environment where I could actually try
>>>> with lld (ld.lld); all testing was with GNU ld (ld.bfd).
>>>
>>> Thanks for fixing this. I've been able to test with LLVM ld and the
>>> workaround is fine.
>>
>> Oh, good, thanks for trying this out.
>>
>>>> --- a/xen/arch/x86/Makefile
>>>> +++ b/xen/arch/x86/Makefile
>>>> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
>>>>  
>>>>  ifneq ($(build_id_linker),)
>>>>  notes_phdrs = --notes
>>>> +# Determine whether to engage a workaround for GNU ld 2.37.
>>>> +build-id-ld-good = $(shell echo 'void test(void) {}' \
>>>> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
>>>> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
>>>> +                           && echo y)
>>>
>>> Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
>>> then use is here?
>>>
>>> We already have compiler and assembler checks in x86/Kconfig, so it
>>> would seem more natural to place it there.
>>
>> The question of whether to record tool chain capabilities in .config
>> is still pending. I'm not convinced this is a good idea, Andrew keeps
>> shouting me out for that, and an actual discussion doesn't really
>> happen. Yet unlike back at the time when I first raised my concern,
>> Anthony meanwhile supports me in at least the question (to Andrew) of
>> when such a discussion would have happened: Neither of us is aware,
>> yet Andrew claims it did happen, but so far didn't point out where
>> one could read about what was discussed and decided there.
>>
>> For the few uses we've accumulated I gave (if at all) an ack for
>> things happening under some sort of pressure, with the request that
>> aformentioned discussion would happen subsequently (and, depending on
>> outcome, these would be converted to another approach if need be). I
>> have meanwhile realized that it was a mistake to allow such things in
>> on this basis - the more of them we gain, the more I'm hearing "we've
>> already got some".
> 
> I see, that's not an ideal situation from a review PoV, as we don't
> have a clear placement for those and that will just cause confusion
> (and more work since there are potentially two places to check).
> 
> What's the benefit of placing the checks in Kconfig instead of the
> Makefiles?
> 
> As I understand Kconfig is run only once, while the Makefile could run
> the check multiple times.

Right - as many times as a directory would be entered for building,
times the number of evaluations of a respective variable.

> The downfall of having them in .config is that .config could suddenly
> change as tools are updated or as it's moved around different systems
> (yet .config already contains specific compiler version information).

Correct: Tool chain specific information may get updated, but then
further options may get silently turned off. Plus to update tool
chain specific information there needs to be a trigger to invoke
kconfig in the first place. Merely relying on make dependencies is
not enough there. Iirc we don't have any means in place yet to
actually enforce this even when there's no other reason to run
kconfig in the course of re-building a previously built tree.

Jan

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Jan Beulich]

On 13.09.2021 16:33, Roger Pau Monné wrote:
> On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
>> On 13.09.2021 15:37, Roger Pau Monné wrote:
>>>> --- a/xen/arch/x86/Makefile
>>>> +++ b/xen/arch/x86/Makefile
>>>> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
>>>>  
>>>>  ifneq ($(build_id_linker),)
>>>>  notes_phdrs = --notes
>>>> +# Determine whether to engage a workaround for GNU ld 2.37.
>>>> +build-id-ld-good = $(shell echo 'void test(void) {}' \
>>>> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
>>>> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
>>>> +                           && echo y)
>>>
>>> Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
>>> then use is here?
>>>
>>> We already have compiler and assembler checks in x86/Kconfig, so it
>>> would seem more natural to place it there.
>>
>> The question of whether to record tool chain capabilities in .config
>> is still pending. I'm not convinced this is a good idea, Andrew keeps
>> shouting me out for that, and an actual discussion doesn't really
>> happen. Yet unlike back at the time when I first raised my concern,
>> Anthony meanwhile supports me in at least the question (to Andrew) of
>> when such a discussion would have happened: Neither of us is aware,
>> yet Andrew claims it did happen, but so far didn't point out where
>> one could read about what was discussed and decided there.
>>
>> For the few uses we've accumulated I gave (if at all) an ack for
>> things happening under some sort of pressure, with the request that
>> aformentioned discussion would happen subsequently (and, depending on
>> outcome, these would be converted to another approach if need be). I
>> have meanwhile realized that it was a mistake to allow such things in
>> on this basis - the more of them we gain, the more I'm hearing "we've
>> already got some".
> 
> I see, that's not an ideal situation from a review PoV, as we don't
> have a clear placement for those and that will just cause confusion
> (and more work since there are potentially two places to check).
> 
> What's the benefit of placing the checks in Kconfig instead of the
> Makefiles?
> 
> As I understand Kconfig is run only once, while the Makefile could run
> the check multiple times.
> 
> The downfall of having them in .config is that .config could suddenly
> change as tools are updated or as it's moved around different systems
> (yet .config already contains specific compiler version information).

I should have added in the earlier reply: Besides the pros and cons
there is, to me at least, also the abstract question of whether such
information belongs in .config in the first place. To me this file
has always been a record of build-meister decisions only. Quite
different from the output of userland ./configure, where checking
various aspects of the environment is the primary goal.

Jan

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Roger Pau Monné]

On Mon, Sep 13, 2021 at 05:10:54PM +0200, Jan Beulich wrote:
> On 13.09.2021 16:33, Roger Pau Monné wrote:
> > On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
> >> On 13.09.2021 15:37, Roger Pau Monné wrote:
> >>>> --- a/xen/arch/x86/Makefile
> >>>> +++ b/xen/arch/x86/Makefile
> >>>> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
> >>>>  
> >>>>  ifneq ($(build_id_linker),)
> >>>>  notes_phdrs = --notes
> >>>> +# Determine whether to engage a workaround for GNU ld 2.37.
> >>>> +build-id-ld-good = $(shell echo 'void test(void) {}' \
> >>>> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
> >>>> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
> >>>> +                           && echo y)
> >>>
> >>> Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
> >>> then use is here?
> >>>
> >>> We already have compiler and assembler checks in x86/Kconfig, so it
> >>> would seem more natural to place it there.
> >>
> >> The question of whether to record tool chain capabilities in .config
> >> is still pending. I'm not convinced this is a good idea, Andrew keeps
> >> shouting me out for that, and an actual discussion doesn't really
> >> happen. Yet unlike back at the time when I first raised my concern,
> >> Anthony meanwhile supports me in at least the question (to Andrew) of
> >> when such a discussion would have happened: Neither of us is aware,
> >> yet Andrew claims it did happen, but so far didn't point out where
> >> one could read about what was discussed and decided there.
> >>
> >> For the few uses we've accumulated I gave (if at all) an ack for
> >> things happening under some sort of pressure, with the request that
> >> aformentioned discussion would happen subsequently (and, depending on
> >> outcome, these would be converted to another approach if need be). I
> >> have meanwhile realized that it was a mistake to allow such things in
> >> on this basis - the more of them we gain, the more I'm hearing "we've
> >> already got some".
> > 
> > I see, that's not an ideal situation from a review PoV, as we don't
> > have a clear placement for those and that will just cause confusion
> > (and more work since there are potentially two places to check).
> > 
> > What's the benefit of placing the checks in Kconfig instead of the
> > Makefiles?
> > 
> > As I understand Kconfig is run only once, while the Makefile could run
> > the check multiple times.
> > 
> > The downfall of having them in .config is that .config could suddenly
> > change as tools are updated or as it's moved around different systems
> > (yet .config already contains specific compiler version information).
> 
> I should have added in the earlier reply: Besides the pros and cons
> there is, to me at least, also the abstract question of whether such
> information belongs in .config in the first place. To me this file
> has always been a record of build-meister decisions only. Quite
> different from the output of userland ./configure, where checking
> various aspects of the environment is the primary goal.

Well, userland ./configure is also used for setting build time options
for the tools (we have a bunch of --{disable/enable}-foo), so I think
the line is quite fuzzy there also.

I think it would be better if we could split Kconfig settings into
.config and .tools for example, to clearly differentiate between
user-selected options vs environment information, and then it would
also be fairly easy to regenerate .tools on every build. Sadly I don't
think Kconfig is capable of this in it's current state.

Thanks, Roger.

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37

[Roger Pau Monné]

On Mon, Sep 13, 2021 at 05:07:54PM +0200, Jan Beulich wrote:
> On 13.09.2021 16:33, Roger Pau Monné wrote:
> > On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
> >> On 13.09.2021 15:37, Roger Pau Monné wrote:
> >>> On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
> >>>> I haven't been able to find an environment where I could actually try
> >>>> with lld (ld.lld); all testing was with GNU ld (ld.bfd).
> >>>
> >>> Thanks for fixing this. I've been able to test with LLVM ld and the
> >>> workaround is fine.
> >>
> >> Oh, good, thanks for trying this out.
> >>
> >>>> --- a/xen/arch/x86/Makefile
> >>>> +++ b/xen/arch/x86/Makefile
> >>>> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
> >>>>  
> >>>>  ifneq ($(build_id_linker),)
> >>>>  notes_phdrs = --notes
> >>>> +# Determine whether to engage a workaround for GNU ld 2.37.
> >>>> +build-id-ld-good = $(shell echo 'void test(void) {}' \
> >>>> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 2>.check.err \
> >>>> +                           && $(LD) -T check.lds -o .check.elf .check.o 2>>.check.err \
> >>>> +                           && echo y)
> >>>
> >>> Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
> >>> then use is here?
> >>>
> >>> We already have compiler and assembler checks in x86/Kconfig, so it
> >>> would seem more natural to place it there.
> >>
> >> The question of whether to record tool chain capabilities in .config
> >> is still pending. I'm not convinced this is a good idea, Andrew keeps
> >> shouting me out for that, and an actual discussion doesn't really
> >> happen. Yet unlike back at the time when I first raised my concern,
> >> Anthony meanwhile supports me in at least the question (to Andrew) of
> >> when such a discussion would have happened: Neither of us is aware,
> >> yet Andrew claims it did happen, but so far didn't point out where
> >> one could read about what was discussed and decided there.
> >>
> >> For the few uses we've accumulated I gave (if at all) an ack for
> >> things happening under some sort of pressure, with the request that
> >> aformentioned discussion would happen subsequently (and, depending on
> >> outcome, these would be converted to another approach if need be). I
> >> have meanwhile realized that it was a mistake to allow such things in
> >> on this basis - the more of them we gain, the more I'm hearing "we've
> >> already got some".
> > 
> > I see, that's not an ideal situation from a review PoV, as we don't
> > have a clear placement for those and that will just cause confusion
> > (and more work since there are potentially two places to check).
> > 
> > What's the benefit of placing the checks in Kconfig instead of the
> > Makefiles?
> > 
> > As I understand Kconfig is run only once, while the Makefile could run
> > the check multiple times.
> 
> Right - as many times as a directory would be entered for building,
> times the number of evaluations of a respective variable.
> 
> > The downfall of having them in .config is that .config could suddenly
> > change as tools are updated or as it's moved around different systems
> > (yet .config already contains specific compiler version information).
> 
> Correct: Tool chain specific information may get updated, but then
> further options may get silently turned off. Plus to update tool
> chain specific information there needs to be a trigger to invoke
> kconfig in the first place. Merely relying on make dependencies is
> not enough there. Iirc we don't have any means in place yet to
> actually enforce this even when there's no other reason to run
> kconfig in the course of re-building a previously built tree.

Indeed, we would need something to trigger the (re)evaluation of
Kconfig options on every run, regardless of whether they have been
set, unless there's already some magic for options using cc-option or
shell test macros that does it. Linux will surely have the same
problem with this, and they recommend to use Kconfig to check for
compiler capabilities [0].

My opinion would be to go for Kconfig because it's IMO cleaner to
represent the options there rather than mixed into Makefiles, and
should also prove to be faster regarding build times due to the single
evaluation. We can always bring the environment related issues to the
Kconfig developers in order to try to find a solution for it.

Thanks, Roger.

[0] https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html#adding-features-that-need-compiler-support