[PATCH 0/2] LTO: two more fixes

[PATCH 0/2] LTO: two more fixes

[Arnd Bergmann]

Hi Andi,

Here are two more fixes for your LTO series. I still hope there
is a nicer way to get make LTO see the relocations from inline
asm, but my patch at least works.

For the modpost case, this is probably the right solution,
but I'm still testing it to make sure it's complete.

      Arnd

Arnd Bergmann (2):
  modpost: don't add warnings for LTO-generated symbols
  x86: make __ibp_barrier visible

 arch/x86/kernel/cpu/bugs.c | 2 +-
 scripts/mod/modpost.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.9.0

[PATCH 1/2] modpost: don't add warnings for LTO-generated symbols

[Arnd Bergmann]

WARNING: vmlinux.o(.data+0x12e0): Section mismatch in reference from the variable pfkey_net_ops.lto_priv.2992 to the function .init.text:pfkey_net_init.lto_priv.2977()
The variable pfkey_net_ops.lto_priv.2992 references
the function __init pfkey_net_init.lto_priv.2977()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 scripts/mod/modpost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f75224d59294..8a8a473cee08 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1027,7 +1027,7 @@ static const struct sectioncheck sectioncheck[] = {
 	.bad_tosec = { INIT_SECTIONS, NULL },
 	.mismatch = DATA_TO_ANY_INIT,
 	.symbol_white_list = {
-		"*_template", "*_timer", "*_sht", "*_ops",
+		"*_template", "*_timer", "*_sht", "*_ops", "*_ops.lto_priv*",
 		"*_probe", "*_probe_one", "*_console", NULL
 	},
 },
-- 
2.9.0

[PATCH 2/2] x86: make __ibp_barrier visible

[Arnd Bergmann]

LTO linking was broken by the addition of __ibp_barrier, which only
has relocations from assembler code, and not from C, so the linker
decides to drop it:

./ccfzUJ1l.ltrans0.ltrans.o:(.altinstr_replacement+0x42): undefined reference to `__ibp_barrier'

Fixes: 64e16720ea08 ("x86/speculation: Simplify indirect_branch_prediction_barrier()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/kernel/cpu/bugs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 71949bf2de5a..c3f5fe1c6e83 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -338,7 +338,7 @@ ssize_t cpu_show_spectre_v2(struct device *dev,
 }
 #endif
 
-void __ibp_barrier(void)
+void __visible __ibp_barrier(void)
 {
 	__wrmsr(MSR_IA32_PRED_CMD, PRED_CMD_IBPB, 0);
 }
-- 
2.9.0

Re: [PATCH 1/2] modpost: don't add warnings for LTO-generated symbols

[Andi Kleen]

On Mon, Feb 05, 2018 at 04:12:52PM +0100, Arnd Bergmann wrote:
> WARNING: vmlinux.o(.data+0x12e0): Section mismatch in reference from the variable pfkey_net_ops.lto_priv.2992 to the function .init.text:pfkey_net_init.lto_priv.2977()
> The variable pfkey_net_ops.lto_priv.2992 references
> the function __init pfkey_net_init.lto_priv.2977()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

A better fix would be to ensure modpost always runs on already LD_FINALed
objects, so it never sees LTO.

Otherwise you would need to teach modpost about the LTO symbol table
and some other magic. I did that once, but it turned out to be very
ugly.

-Andi

Re: [PATCH 1/2] modpost: don't add warnings for LTO-generated symbols

[Arnd Bergmann]

On Mon, Feb 5, 2018 at 4:50 PM, Andi Kleen <ak@linux.intel.com> wrote:
> On Mon, Feb 05, 2018 at 04:12:52PM +0100, Arnd Bergmann wrote:
>> WARNING: vmlinux.o(.data+0x12e0): Section mismatch in reference from the variable pfkey_net_ops.lto_priv.2992 to the function .init.text:pfkey_net_init.lto_priv.2977()
>> The variable pfkey_net_ops.lto_priv.2992 references
>> the function __init pfkey_net_init.lto_priv.2977()
>> If the reference is valid then annotate the
>> variable with __init* or __refdata (see linux/init.h) or name the variable:
>> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
> A better fix would be to ensure modpost always runs on already LD_FINALed
> objects, so it never sees LTO.
>
> Otherwise you would need to teach modpost about the LTO symbol table
> and some other magic. I did that once, but it turned out to be very
> ugly.

I'm not sure I understand what that means. Do you mean that with LD_FINAL,
those symbol references are completely eliminated so we don't need to worry
about them any more?

I got about a dozen section mismatch errors with LTO in cases where the
calling function gets a specialized version of a structure, and my oneline
patch above addresses them all.

        Arnd

Re: [PATCH 1/2] modpost: don't add warnings for LTO-generated symbols

[Andi Kleen]

On Mon, Feb 05, 2018 at 04:59:32PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 5, 2018 at 4:50 PM, Andi Kleen <ak@linux.intel.com> wrote:
> > On Mon, Feb 05, 2018 at 04:12:52PM +0100, Arnd Bergmann wrote:
> >> WARNING: vmlinux.o(.data+0x12e0): Section mismatch in reference from the variable pfkey_net_ops.lto_priv.2992 to the function .init.text:pfkey_net_init.lto_priv.2977()
> >> The variable pfkey_net_ops.lto_priv.2992 references
> >> the function __init pfkey_net_init.lto_priv.2977()
> >> If the reference is valid then annotate the
> >> variable with __init* or __refdata (see linux/init.h) or name the variable:
> >> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
> >
> > A better fix would be to ensure modpost always runs on already LD_FINALed
> > objects, so it never sees LTO.
> >
> > Otherwise you would need to teach modpost about the LTO symbol table
> > and some other magic. I did that once, but it turned out to be very
> > ugly.
> 
> I'm not sure I understand what that means. Do you mean that with LD_FINAL,
> those symbol references are completely eliminated so we don't need to worry
> about them any more?

Yes there should not be any .lto_priv references after LDFINAL, it will
be just like any other ELF object. We also need to do the same
thing before objtool.

> 
> I got about a dozen section mismatch errors with LTO in cases where the
> calling function gets a specialized version of a structure, and my oneline
> patch above addresses them all.

Not sure I follow here. .lto_priv should be anything LTO, not just
some specific optimizations.

-Andi

Re: [PATCH 1/2] modpost: don't add warnings for LTO-generated symbols

[Arnd Bergmann]

On Mon, Feb 5, 2018 at 5:22 PM, Andi Kleen <ak@linux.intel.com> wrote:
> On Mon, Feb 05, 2018 at 04:59:32PM +0100, Arnd Bergmann wrote:
>> On Mon, Feb 5, 2018 at 4:50 PM, Andi Kleen <ak@linux.intel.com> wrote:
>> > On Mon, Feb 05, 2018 at 04:12:52PM +0100, Arnd Bergmann wrote:
>> >> WARNING: vmlinux.o(.data+0x12e0): Section mismatch in reference from the variable pfkey_net_ops.lto_priv.2992 to the function .init.text:pfkey_net_init.lto_priv.2977()
>> >> The variable pfkey_net_ops.lto_priv.2992 references
>> >> the function __init pfkey_net_init.lto_priv.2977()
>> >> If the reference is valid then annotate the
>> >> variable with __init* or __refdata (see linux/init.h) or name the variable:
>> >> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>> >
>> > A better fix would be to ensure modpost always runs on already LD_FINALed
>> > objects, so it never sees LTO.
>> >
>> > Otherwise you would need to teach modpost about the LTO symbol table
>> > and some other magic. I did that once, but it turned out to be very
>> > ugly.
>>
>> I'm not sure I understand what that means. Do you mean that with LD_FINAL,
>> those symbol references are completely eliminated so we don't need to worry
>> about them any more?
>
> Yes there should not be any .lto_priv references after LDFINAL, it will
> be just like any other ELF object. We also need to do the same
> thing before objtool.

>> I got about a dozen section mismatch errors with LTO in cases where the
>> calling function gets a specialized version of a structure, and my oneline
>> patch above addresses them all.
>
> Not sure I follow here. .lto_priv should be anything LTO, not just
> some specific optimizations.

I think it's a bit different from my first interpretation, this is what I see
in the output of 'objdump -Dr vmlinux' after the final LTO link:

b1da26f0 <amba_console.lto_priv.21118>:
b1da26f0:       74747941        cmnmi   r9, r4, ror r4
b1da26f4:       4d000000        andeq   r0, r0, sp, asr #32
        ...
b1da2700:       b0bdfa08        ldmeq   sl!, {r4, r5, r7, r8, sl, fp,
ip, sp, pc}^
b1da2704:       00000000        andeq   r0, r0, r0
b1da2708:       b0bcc790        strhls  fp, [r7], #192  ; 0xc0
b1da270c:       00000000        andeq   r0, r0, r0
b1da2710:       b1a2bb18        ldmne   fp!, {r0, r4, r5, r7, r9, sp, pc}
b1da2714:       00000000        andeq   r0, r0, r0
b1da2718:       0001ffff                        ; <UNDEFINED>
instruction: 0xffff0100
b1da271c:       00000000        andeq   r0, r0, r0
b1da2720:       b1da0938        stmdacc r9, {r0, r4, r5, r7, r9, fp, ip, lr, pc}
b1da2724:       00000000        andeq   r0, r0, r0


b1da2728 <amba_console.lto_priv.21119>:
b1da2728:       74747941        cmnmi   r9, r4, ror r4
b1da272c:       4d410000        andeq   r4, r0, sp, asr #2
        ...
b1da2738:       b0be163c        ldccc   14, cr11, [r6], {176}   ; 0xb0
b1da273c:       00000000        andeq   r0, r0, r0
b1da2740:       b0bcc790        strhls  fp, [r7], #192  ; 0xc0
b1da2744:       00000000        andeq   r0, r0, r0
b1da2748:       b1a2b8b8        ldmlt   r8!, {r0, r4, r5, r7, r9, sp, pc}
b1da274c:       b1a2e6d8        stmiale r6!, {r0, r4, r5, r7, r9, sp, pc}^
b1da2750:       0011ffff                        ; <UNDEFINED>
instruction: 0xffff1100
b1da2754:       00000000        andeq   r0, r0, r0
b1da2758:       b1d9fed8        ldmle   lr!, {r0, r4, r5, r7, r8, fp,
ip, lr, pc}^
b1da275c:       00000000        andeq   r0, r0, r0

I got a warning from scripts/mod/modpost about vmlinux.o before it,
which has relocations from each of these two into an init function.

My interpretation now is that LTO adds the .lto_priv.21118 suffix for
the two structures to disambiguate them, as one comes from
amba-pl010.c and the other one is from amba-pl011.c.
This means that the whitelist in modpost no longer works, as it
checks the name against the "*_console" glob string.

Almost all other symbols are not affected because the names of the
static symbols are unique, and LTO gets to drop the suffix.

         Arnd