[PATCH v4] x86/entry: emit a symbol for register restoring thunk

[PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Nick Desaulniers]

Arnd found a randconfig that produces the warning:

arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
offset 0x3e

when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
notes:

  With the LLVM assembler not generating section symbols, objtool has no
  way to reference this code when it generates ORC unwinder entries,
  because this code is outside of any ELF function.

  The limitation now being imposed by objtool is that all code must be
  contained in an ELF symbol.  And .L symbols don't create such symbols.

  So basically, you can use an .L symbol *inside* a function or a code
  segment, you just can't use the .L symbol to contain the code using a
  SYM_*_START/END annotation pair.

Fangrui notes that this optimization is helpful for reducing image size
when compiling with -ffunction-sections and -fdata-sections. I have
observed on the order of tens of thousands of symbols for the kernel
images built with those flags.

A patch has been authored against GNU binutils to match this behavior,
so this will also become a problem for users of GNU binutils once they
upgrade to 2.36.

We can omit the .L prefix on a label so that the assembler will emit an
entry into the symbol table for the label, with STB_LOCAL binding.  This
enables objtool to generate proper unwind info here with LLVM_IAS=1 or
GNU binutils 2.36+.

Cc: Fangrui Song <maskray@google.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1209
Link: https://reviews.llvm.org/D93783
Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8
Reported-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes v3 -> v4:
* Add changes to Documentation/ and include/ as per Boris.
* Fix typos as per Josh.
* Replace link and note in commit message about
  --generate-unused-section-symbols=[yes|no] which was dropped from GNU
  binutils with link to actual commit in binutils-gdb.
* Add additional notes from Josh in commit message.
* Slightly reword commit message to indicate that section symbols are
  not emitted, rather than stripped.

Changes v2 -> v3:
* rework to use STB_LOCAL rather than STB_GLOBAL by dropping .L prefix,
  as per Josh.
* rename oneline to drop STB_GLOBAL in commit message.
* add link to GAS docs on .L prefix.
* drop Josh's ack since patch changed.

Changes v1 -> v2:
* Pick up Josh's Ack.
* Add commit message info about -ffunction-sections/-fdata-sections, and
  link to binutils patch.

 Documentation/asm-annotations.rst | 9 +++++++++
 arch/x86/entry/thunk_64.S         | 8 ++++----
 include/linux/linkage.h           | 5 ++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst
index 32ea57483378..e711ff98102a 100644
--- a/Documentation/asm-annotations.rst
+++ b/Documentation/asm-annotations.rst
@@ -153,6 +153,15 @@ This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.
   To some extent, this category corresponds to deprecated ``ENTRY`` and
   ``END``. Except ``END`` had several other meanings too.
 
+  Developers should avoid using local symbol names that are prefixed with
+  ``.L``, as this has special meaning for the assembler; a symbol entry will
+  not be emitted into the symbol table. This can prevent ``objtool`` from
+  generating correct unwind info. Symbols with STB_LOCAL binding may still be
+  used, and ``.L`` prefixed local symbol names are still generally useable
+  within a function, but ``.L`` prefixed local symbol names should not be used
+  to denote the beginning or end of code regions via
+  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
+
 * ``SYM_INNER_LABEL*`` is used to denote a label inside some
   ``SYM_{CODE,FUNC}_START`` and ``SYM_{CODE,FUNC}_END``.  They are very similar
   to C labels, except they can be made global. An example of use::
diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index ccd32877a3c4..c9a9fbf1655f 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -31,7 +31,7 @@ SYM_FUNC_START_NOALIGN(\name)
 	.endif
 
 	call \func
-	jmp  .L_restore
+	jmp  __thunk_restore
 SYM_FUNC_END(\name)
 	_ASM_NOKPROBE(\name)
 	.endm
@@ -44,7 +44,7 @@ SYM_FUNC_END(\name)
 #endif
 
 #ifdef CONFIG_PREEMPTION
-SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
+SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore)
 	popq %r11
 	popq %r10
 	popq %r9
@@ -56,6 +56,6 @@ SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
 	popq %rdi
 	popq %rbp
 	ret
-	_ASM_NOKPROBE(.L_restore)
-SYM_CODE_END(.L_restore)
+	_ASM_NOKPROBE(__thunk_restore)
+SYM_CODE_END(__thunk_restore)
 #endif
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index 5bcfbd972e97..11537ba9f512 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -270,7 +270,10 @@
 	SYM_END(name, SYM_T_FUNC)
 #endif
 
-/* SYM_CODE_START -- use for non-C (special) functions */
+/*
+ * SYM_CODE_START -- use for non-C (special) functions, avoid .L prefixed local
+ * symbol names which may not emit a symbol table entry.
+ */
 #ifndef SYM_CODE_START
 #define SYM_CODE_START(name)				\
 	SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Mark Brown]

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

On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:

This:

> when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> notes:

>   So basically, you can use an .L symbol *inside* a function or a code
>   segment, you just can't use the .L symbol to contain the code using a
>   SYM_*_START/END annotation pair.

is a stronger statement than this:

> +  Developers should avoid using local symbol names that are prefixed with
> +  ``.L``, as this has special meaning for the assembler; a symbol entry will
> +  not be emitted into the symbol table. This can prevent ``objtool`` from
> +  generating correct unwind info. Symbols with STB_LOCAL binding may still be
> +  used, and ``.L`` prefixed local symbol names are still generally useable
> +  within a function, but ``.L`` prefixed local symbol names should not be used
> +  to denote the beginning or end of code regions via
> +  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.

and seems more what I'd expect - SYM_FUNC* is also affected for example.
Even though other usages are probably not very likely it seems better to
keep the stronger statement in case someone comes up with one, and to
stop anyone spending time wondering why only SYM_CODE_START_LOCAL is
affected.

This also looks like a good candiate for a checkpatch rule, but that can
be done separately of course.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Josh Poimboeuf]

On Tue, Jan 12, 2021 at 09:01:54PM +0000, Mark Brown wrote:
> On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:
> 
> This:
> 
> > when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> > notes:
> 
> >   So basically, you can use an .L symbol *inside* a function or a code
> >   segment, you just can't use the .L symbol to contain the code using a
> >   SYM_*_START/END annotation pair.
> 
> is a stronger statement than this:
> 
> > +  Developers should avoid using local symbol names that are prefixed with
> > +  ``.L``, as this has special meaning for the assembler; a symbol entry will
> > +  not be emitted into the symbol table. This can prevent ``objtool`` from
> > +  generating correct unwind info. Symbols with STB_LOCAL binding may still be
> > +  used, and ``.L`` prefixed local symbol names are still generally useable
> > +  within a function, but ``.L`` prefixed local symbol names should not be used
> > +  to denote the beginning or end of code regions via
> > +  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
> 
> and seems more what I'd expect - SYM_FUNC* is also affected for example.
> Even though other usages are probably not very likely it seems better to
> keep the stronger statement in case someone comes up with one, and to
> stop anyone spending time wondering why only SYM_CODE_START_LOCAL is
> affected.

Agreed, I think the comment is misleading/wrong/unclear in multiple
ways.  In most cases the use of .L symbols is still fine.  What's no
longer fine is when they're used to contain code in any kind of
START/END pair.

> This also looks like a good candiate for a checkpatch rule, but that can
> be done separately of course.

I like the idea of a checkpatch rule.

-- 
Josh

[PATCH] Documentation: asm-annotation: clarify .L local symbol names

[Nick Desaulniers]

Use more precise language and move the text to a region in the docs to
show that this constraint is not just for SYM_CODE_START*.

Suggested-by: Mark Brown <broonie@kernel.org>
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Documentation/asm-annotations.rst | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst
index e711ff98102a..76424e0431f4 100644
--- a/Documentation/asm-annotations.rst
+++ b/Documentation/asm-annotations.rst
@@ -100,6 +100,11 @@ Instruction Macros
 ~~~~~~~~~~~~~~~~~~
 This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.
 
+``objtool`` requires that all code must be contained in an ELF symbol. Symbol
+names that have a ``.L`` prefix do not emit symbol table entries. ``.L``
+prefixed symbols can be used within a code region, but should be avoided for
+denoting a range of code via ``SYM_*_START/END`` annotations.
+
 * ``SYM_FUNC_START`` and ``SYM_FUNC_START_LOCAL`` are supposed to be **the
   most frequent markings**. They are used for functions with standard calling
   conventions -- global and local. Like in C, they both align the functions to
@@ -153,15 +158,6 @@ This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.
   To some extent, this category corresponds to deprecated ``ENTRY`` and
   ``END``. Except ``END`` had several other meanings too.
 
-  Developers should avoid using local symbol names that are prefixed with
-  ``.L``, as this has special meaning for the assembler; a symbol entry will
-  not be emitted into the symbol table. This can prevent ``objtool`` from
-  generating correct unwind info. Symbols with STB_LOCAL binding may still be
-  used, and ``.L`` prefixed local symbol names are still generally useable
-  within a function, but ``.L`` prefixed local symbol names should not be used
-  to denote the beginning or end of code regions via
-  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
-
 * ``SYM_INNER_LABEL*`` is used to denote a label inside some
   ``SYM_{CODE,FUNC}_START`` and ``SYM_{CODE,FUNC}_END``.  They are very similar
   to C labels, except they can be made global. An example of use::
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Nick Desaulniers]

On Wed, Jan 13, 2021 at 8:59 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Tue, Jan 12, 2021 at 09:01:54PM +0000, Mark Brown wrote:
> > On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:
> >
> > This:
> >
> > > when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> > > notes:
> >
> > >   So basically, you can use an .L symbol *inside* a function or a code
> > >   segment, you just can't use the .L symbol to contain the code using a
> > >   SYM_*_START/END annotation pair.
> >
> > is a stronger statement than this:
> >
> > > +  Developers should avoid using local symbol names that are prefixed with
> > > +  ``.L``, as this has special meaning for the assembler; a symbol entry will
> > > +  not be emitted into the symbol table. This can prevent ``objtool`` from
> > > +  generating correct unwind info. Symbols with STB_LOCAL binding may still be
> > > +  used, and ``.L`` prefixed local symbol names are still generally useable
> > > +  within a function, but ``.L`` prefixed local symbol names should not be used
> > > +  to denote the beginning or end of code regions via
> > > +  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
> >
> > and seems more what I'd expect - SYM_FUNC* is also affected for example.
> > Even though other usages are probably not very likely it seems better to
> > keep the stronger statement in case someone comes up with one, and to
> > stop anyone spending time wondering why only SYM_CODE_START_LOCAL is
> > affected.
>
> Agreed, I think the comment is misleading/wrong/unclear in multiple
> ways.  In most cases the use of .L symbols is still fine.  What's no
> longer fine is when they're used to contain code in any kind of
> START/END pair.

Apologies, that was not my intention.  I've sent a follow up in
https://lore.kernel.org/lkml/20210113174620.958429-1-ndesaulniers@google.com/T/#u
since BP picked up v3 in tip x86/entry:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/entry&id=bde718b7e154afc99e1956b18a848401ce8e1f8e

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] Documentation: asm-annotation: clarify .L local symbol names

[Mark Brown]

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

On Wed, Jan 13, 2021 at 09:46:20AM -0800, Nick Desaulniers wrote:
> Use more precise language and move the text to a region in the docs to
> show that this constraint is not just for SYM_CODE_START*.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Borislav Petkov]

On Wed, Jan 13, 2021 at 09:56:04AM -0800, Nick Desaulniers wrote:
> Apologies, that was not my intention.  I've sent a follow up in
> https://lore.kernel.org/lkml/20210113174620.958429-1-ndesaulniers@google.com/T/#u
> since BP picked up v3 in tip x86/entry:
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/entry&id=bde718b7e154afc99e1956b18a848401ce8e1f8e

It is the topmost patch so I can rebase...

Also, I replicated that text into linkage.h and removed the change over
SYM_CODE_START and I've got the below.

Further complaints?

---
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Tue, 12 Jan 2021 11:46:24 -0800
Subject: [PATCH] x86/entry: Emit a symbol for register restoring thunk

Arnd found a randconfig that produces the warning:

  arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
  offset 0x3e

when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
notes:

  With the LLVM assembler not generating section symbols, objtool has no
  way to reference this code when it generates ORC unwinder entries,
  because this code is outside of any ELF function.

  The limitation now being imposed by objtool is that all code must be
  contained in an ELF symbol.  And .L symbols don't create such symbols.

  So basically, you can use an .L symbol *inside* a function or a code
  segment, you just can't use the .L symbol to contain the code using a
  SYM_*_START/END annotation pair.

Fangrui notes that this optimization is helpful for reducing image size
when compiling with -ffunction-sections and -fdata-sections. I have
observed on the order of tens of thousands of symbols for the kernel
images built with those flags.

A patch has been authored against GNU binutils to match this behavior
of not generating unused section symbols ([1]), so this will
also become a problem for users of GNU binutils once they upgrade to 2.36.

Omit the .L prefix on a label so that the assembler will emit an entry
into the symbol table for the label, with STB_LOCAL binding. This
enables objtool to generate proper unwind info here with LLVM_IAS=1 or
GNU binutils 2.36+.

 [ bp: Massage commit message. ]

Reported-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20210112194625.4181814-1-ndesaulniers@google.com
Link: https://github.com/ClangBuiltLinux/linux/issues/1209
Link: https://reviews.llvm.org/D93783
Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8 [1]
---
 Documentation/asm-annotations.rst | 5 +++++
 arch/x86/entry/thunk_64.S         | 8 ++++----
 include/linux/linkage.h           | 5 +++++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst
index 32ea57483378..76424e0431f4 100644
--- a/Documentation/asm-annotations.rst
+++ b/Documentation/asm-annotations.rst
@@ -100,6 +100,11 @@ Instruction Macros
 ~~~~~~~~~~~~~~~~~~
 This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.
 
+``objtool`` requires that all code must be contained in an ELF symbol. Symbol
+names that have a ``.L`` prefix do not emit symbol table entries. ``.L``
+prefixed symbols can be used within a code region, but should be avoided for
+denoting a range of code via ``SYM_*_START/END`` annotations.
+
 * ``SYM_FUNC_START`` and ``SYM_FUNC_START_LOCAL`` are supposed to be **the
   most frequent markings**. They are used for functions with standard calling
   conventions -- global and local. Like in C, they both align the functions to
diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index ccd32877a3c4..c9a9fbf1655f 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -31,7 +31,7 @@ SYM_FUNC_START_NOALIGN(\name)
 	.endif
 
 	call \func
-	jmp  .L_restore
+	jmp  __thunk_restore
 SYM_FUNC_END(\name)
 	_ASM_NOKPROBE(\name)
 	.endm
@@ -44,7 +44,7 @@ SYM_FUNC_END(\name)
 #endif
 
 #ifdef CONFIG_PREEMPTION
-SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
+SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore)
 	popq %r11
 	popq %r10
 	popq %r9
@@ -56,6 +56,6 @@ SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
 	popq %rdi
 	popq %rbp
 	ret
-	_ASM_NOKPROBE(.L_restore)
-SYM_CODE_END(.L_restore)
+	_ASM_NOKPROBE(__thunk_restore)
+SYM_CODE_END(__thunk_restore)
 #endif
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index 5bcfbd972e97..dbf8506decca 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -178,6 +178,11 @@
  * Objtool generates debug info for both FUNC & CODE, but needs special
  * annotations for each CODE's start (to describe the actual stack frame).
  *
+ * Objtool requires that all code must be contained in an ELF symbol. Symbol
+ * names that have a  .L prefix do not emit symbol table entries. .L
+ * prefixed symbols can be used within a code region, but should be avoided for
+ * denoting a range of code via ``SYM_*_START/END`` annotations.
+ *
  * ALIAS -- does not generate debug info -- the aliased function will
  */
 
-- 
2.29.2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Peter Zijlstra]

On Thu, Jan 14, 2021 at 11:39:28AM +0100, Borislav Petkov wrote:
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Tue, 12 Jan 2021 11:46:24 -0800
> Subject: [PATCH] x86/entry: Emit a symbol for register restoring thunk
> 
> Arnd found a randconfig that produces the warning:
> 
>   arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
>   offset 0x3e
> 
> when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> notes:
> 
>   With the LLVM assembler not generating section symbols, objtool has no
>   way to reference this code when it generates ORC unwinder entries,
>   because this code is outside of any ELF function.
> 
>   The limitation now being imposed by objtool is that all code must be
>   contained in an ELF symbol.  And .L symbols don't create such symbols.
> 
>   So basically, you can use an .L symbol *inside* a function or a code
>   segment, you just can't use the .L symbol to contain the code using a
>   SYM_*_START/END annotation pair.
> 
> Fangrui notes that this optimization is helpful for reducing image size
> when compiling with -ffunction-sections and -fdata-sections. I have
> observed on the order of tens of thousands of symbols for the kernel
> images built with those flags.
> 
> A patch has been authored against GNU binutils to match this behavior
> of not generating unused section symbols ([1]), so this will
> also become a problem for users of GNU binutils once they upgrade to 2.36.
> 
> Omit the .L prefix on a label so that the assembler will emit an entry
> into the symbol table for the label, with STB_LOCAL binding. This
> enables objtool to generate proper unwind info here with LLVM_IAS=1 or
> GNU binutils 2.36+.
> 
>  [ bp: Massage commit message. ]
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
but that's another patch.

[PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument

[Borislav Petkov]

On Thu, Jan 14, 2021 at 12:36:45PM +0100, Peter Zijlstra wrote:
> And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
> but that's another patch.

---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 14 Jan 2021 14:25:35 +0100
Subject: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument

That logic is unused since

  320100a5ffe5 ("x86/entry: Remove the TRACE_IRQS cruft")

Remove it.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/entry/thunk_64.S | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index c9a9fbf1655f..496b11ec469d 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -10,7 +10,7 @@
 #include <asm/export.h>
 
 	/* rdi:	arg1 ... normal C conventions. rax is saved/restored. */
-	.macro THUNK name, func, put_ret_addr_in_rdi=0
+	.macro THUNK name, func
 SYM_FUNC_START_NOALIGN(\name)
 	pushq %rbp
 	movq %rsp, %rbp
@@ -25,11 +25,6 @@ SYM_FUNC_START_NOALIGN(\name)
 	pushq %r10
 	pushq %r11
 
-	.if \put_ret_addr_in_rdi
-	/* 8(%rbp) is return addr on stack */
-	movq 8(%rbp), %rdi
-	.endif
-
 	call \func
 	jmp  __thunk_restore
 SYM_FUNC_END(\name)
-- 
2.29.2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Josh Poimboeuf]

On Thu, Jan 14, 2021 at 11:39:28AM +0100, Borislav Petkov wrote:
> On Wed, Jan 13, 2021 at 09:56:04AM -0800, Nick Desaulniers wrote:
> > Apologies, that was not my intention.  I've sent a follow up in
> > https://lore.kernel.org/lkml/20210113174620.958429-1-ndesaulniers@google.com/T/#u
> > since BP picked up v3 in tip x86/entry:
> > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/entry&id=bde718b7e154afc99e1956b18a848401ce8e1f8e
> 
> It is the topmost patch so I can rebase...
> 
> Also, I replicated that text into linkage.h and removed the change over
> SYM_CODE_START and I've got the below.
> 
> Further complaints?
> 
> ---
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Tue, 12 Jan 2021 11:46:24 -0800
> Subject: [PATCH] x86/entry: Emit a symbol for register restoring thunk
> 
> Arnd found a randconfig that produces the warning:
> 
>   arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
>   offset 0x3e
> 
> when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> notes:
> 
>   With the LLVM assembler not generating section symbols, objtool has no
>   way to reference this code when it generates ORC unwinder entries,
>   because this code is outside of any ELF function.
> 
>   The limitation now being imposed by objtool is that all code must be
>   contained in an ELF symbol.  And .L symbols don't create such symbols.
> 
>   So basically, you can use an .L symbol *inside* a function or a code
>   segment, you just can't use the .L symbol to contain the code using a
>   SYM_*_START/END annotation pair.
> 
> Fangrui notes that this optimization is helpful for reducing image size
> when compiling with -ffunction-sections and -fdata-sections. I have
> observed on the order of tens of thousands of symbols for the kernel
> images built with those flags.
> 
> A patch has been authored against GNU binutils to match this behavior
> of not generating unused section symbols ([1]), so this will
> also become a problem for users of GNU binutils once they upgrade to 2.36.
> 
> Omit the .L prefix on a label so that the assembler will emit an entry
> into the symbol table for the label, with STB_LOCAL binding. This
> enables objtool to generate proper unwind info here with LLVM_IAS=1 or
> GNU binutils 2.36+.
> 
>  [ bp: Massage commit message. ]

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

Re: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument

[Peter Zijlstra]

On Thu, Jan 14, 2021 at 02:28:09PM +0100, Borislav Petkov wrote:
> On Thu, Jan 14, 2021 at 12:36:45PM +0100, Peter Zijlstra wrote:
> > And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
> > but that's another patch.
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Thu, 14 Jan 2021 14:25:35 +0100
> Subject: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument
> 
> That logic is unused since
> 
>   320100a5ffe5 ("x86/entry: Remove the TRACE_IRQS cruft")
> 
> Remove it.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>

Acked-by; Peter Zijlstra (Intel) <peterz@infradead.org>