mbox series

[00/13] x86/crypto/asm: objtool support

Message ID cover.1614182415.git.jpoimboe@redhat.com
Headers show
Series x86/crypto/asm: objtool support | expand

Message

Josh Poimboeuf Feb. 24, 2021, 4:29 p.m. UTC
Standardize the crypto asm to make it resemble compiler-generated code,
so that objtool can understand it.

This magically enables ORC unwinding from crypto code.  It also fixes
the last known remaining objtool warnings on vmlinux.o, for LTO and
more.

Josh Poimboeuf (13):
  objtool: Support asm jump tables
  x86/crypto/aesni-intel_avx: Remove unused macros
  x86/crypto/aesni-intel_avx: Fix register usage comments
  x86/crypto/aesni-intel_avx: Standardize stack alignment prologue
  x86/crypto/camellia-aesni-avx2: Unconditionally allocate stack buffer
  x86/crypto/crc32c-pcl-intel: Standardize jump table
  x86/crypto/sha_ni: Standardize stack alignment prologue
  x86/crypto/sha1_avx2: Standardize stack alignment prologue
  x86/crypto/sha256-avx2: Standardize stack alignment prologue
  x86/crypto/sha512-avx: Standardize stack alignment prologue
  x86/crypto/sha512-avx2: Standardize stack alignment prologue
  x86/crypto/sha512-ssse3: Standardize stack alignment prologue
  x86/crypto: Enable objtool in crypto code

 arch/x86/crypto/Makefile                     |  2 -
 arch/x86/crypto/aesni-intel_avx-x86_64.S     | 28 +++++--------
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S |  5 +--
 arch/x86/crypto/crc32c-pcl-intel-asm_64.S    |  7 +---
 arch/x86/crypto/sha1_avx2_x86_64_asm.S       |  8 ++--
 arch/x86/crypto/sha1_ni_asm.S                |  8 ++--
 arch/x86/crypto/sha256-avx2-asm.S            | 13 +++---
 arch/x86/crypto/sha512-avx-asm.S             | 41 +++++++++----------
 arch/x86/crypto/sha512-avx2-asm.S            | 42 ++++++++++----------
 arch/x86/crypto/sha512-ssse3-asm.S           | 41 +++++++++----------
 tools/objtool/check.c                        | 14 ++++++-
 11 files changed, 98 insertions(+), 111 deletions(-)

Comments

Peter Zijlstra Feb. 25, 2021, 9:38 a.m. UTC | #1
On Wed, Feb 24, 2021 at 10:29:17AM -0600, Josh Poimboeuf wrote:
> Use RBP instead of R14 for saving the old stack pointer before

> realignment.  This resembles what compilers normally do.

> 

> This enables ORC unwinding by allowing objtool to understand the stack

> realignment.

> 

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

> ---

>  arch/x86/crypto/aesni-intel_avx-x86_64.S | 10 ++++------

>  1 file changed, 4 insertions(+), 6 deletions(-)

> 

> diff --git a/arch/x86/crypto/aesni-intel_avx-x86_64.S b/arch/x86/crypto/aesni-intel_avx-x86_64.S

> index 188f1848a730..98e3552b6e03 100644

> --- a/arch/x86/crypto/aesni-intel_avx-x86_64.S

> +++ b/arch/x86/crypto/aesni-intel_avx-x86_64.S

> @@ -251,22 +251,20 @@ VARIABLE_OFFSET = 16*8

>  .macro FUNC_SAVE

>          push    %r12

>          push    %r13

> -        push    %r14

>          push    %r15

>  

> -        mov     %rsp, %r14

> -

> -

> +	push	%rbp

> +	mov	%rsp, %rbp

>  

>          sub     $VARIABLE_OFFSET, %rsp

>          and     $~63, %rsp                    # align rsp to 64 bytes

>  .endm

>  

>  .macro FUNC_RESTORE

> -        mov     %r14, %rsp

> +        mov     %rbp, %rsp

> +	pop	%rbp

>  

>          pop     %r15

> -        pop     %r14

>          pop     %r13

>          pop     %r12

>  .endm


Urgh, I was about to say your patch is whitespace damaged, but it's the
original file :-/
Peter Zijlstra Feb. 25, 2021, 9:46 a.m. UTC | #2
On Wed, Feb 24, 2021 at 10:29:13AM -0600, Josh Poimboeuf wrote:
> Standardize the crypto asm to make it resemble compiler-generated code,

> so that objtool can understand it.

> 

> This magically enables ORC unwinding from crypto code.  It also fixes

> the last known remaining objtool warnings on vmlinux.o, for LTO and

> more.

> 

> Josh Poimboeuf (13):

>   objtool: Support asm jump tables

>   x86/crypto/aesni-intel_avx: Remove unused macros

>   x86/crypto/aesni-intel_avx: Fix register usage comments

>   x86/crypto/aesni-intel_avx: Standardize stack alignment prologue

>   x86/crypto/camellia-aesni-avx2: Unconditionally allocate stack buffer

>   x86/crypto/crc32c-pcl-intel: Standardize jump table

>   x86/crypto/sha_ni: Standardize stack alignment prologue

>   x86/crypto/sha1_avx2: Standardize stack alignment prologue

>   x86/crypto/sha256-avx2: Standardize stack alignment prologue

>   x86/crypto/sha512-avx: Standardize stack alignment prologue

>   x86/crypto/sha512-avx2: Standardize stack alignment prologue

>   x86/crypto/sha512-ssse3: Standardize stack alignment prologue

>   x86/crypto: Enable objtool in crypto code

> 

>  arch/x86/crypto/Makefile                     |  2 -

>  arch/x86/crypto/aesni-intel_avx-x86_64.S     | 28 +++++--------

>  arch/x86/crypto/camellia-aesni-avx2-asm_64.S |  5 +--

>  arch/x86/crypto/crc32c-pcl-intel-asm_64.S    |  7 +---

>  arch/x86/crypto/sha1_avx2_x86_64_asm.S       |  8 ++--

>  arch/x86/crypto/sha1_ni_asm.S                |  8 ++--

>  arch/x86/crypto/sha256-avx2-asm.S            | 13 +++---

>  arch/x86/crypto/sha512-avx-asm.S             | 41 +++++++++----------

>  arch/x86/crypto/sha512-avx2-asm.S            | 42 ++++++++++----------

>  arch/x86/crypto/sha512-ssse3-asm.S           | 41 +++++++++----------

>  tools/objtool/check.c                        | 14 ++++++-

>  11 files changed, 98 insertions(+), 111 deletions(-)


One nit, there's lots and lots of:

	mov %rbp, %rsp
	pop %rbp

and we have this 'leave' instruction that does exactly that, should we
be using it?

Otherwise:

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Josh Poimboeuf Feb. 25, 2021, 1:29 p.m. UTC | #3
On Thu, Feb 25, 2021 at 10:46:56AM +0100, Peter Zijlstra wrote:
> On Wed, Feb 24, 2021 at 10:29:13AM -0600, Josh Poimboeuf wrote:

> > Standardize the crypto asm to make it resemble compiler-generated code,

> > so that objtool can understand it.

> > 

> > This magically enables ORC unwinding from crypto code.  It also fixes

> > the last known remaining objtool warnings on vmlinux.o, for LTO and

> > more.

> > 

> > Josh Poimboeuf (13):

> >   objtool: Support asm jump tables

> >   x86/crypto/aesni-intel_avx: Remove unused macros

> >   x86/crypto/aesni-intel_avx: Fix register usage comments

> >   x86/crypto/aesni-intel_avx: Standardize stack alignment prologue

> >   x86/crypto/camellia-aesni-avx2: Unconditionally allocate stack buffer

> >   x86/crypto/crc32c-pcl-intel: Standardize jump table

> >   x86/crypto/sha_ni: Standardize stack alignment prologue

> >   x86/crypto/sha1_avx2: Standardize stack alignment prologue

> >   x86/crypto/sha256-avx2: Standardize stack alignment prologue

> >   x86/crypto/sha512-avx: Standardize stack alignment prologue

> >   x86/crypto/sha512-avx2: Standardize stack alignment prologue

> >   x86/crypto/sha512-ssse3: Standardize stack alignment prologue

> >   x86/crypto: Enable objtool in crypto code

> > 

> >  arch/x86/crypto/Makefile                     |  2 -

> >  arch/x86/crypto/aesni-intel_avx-x86_64.S     | 28 +++++--------

> >  arch/x86/crypto/camellia-aesni-avx2-asm_64.S |  5 +--

> >  arch/x86/crypto/crc32c-pcl-intel-asm_64.S    |  7 +---

> >  arch/x86/crypto/sha1_avx2_x86_64_asm.S       |  8 ++--

> >  arch/x86/crypto/sha1_ni_asm.S                |  8 ++--

> >  arch/x86/crypto/sha256-avx2-asm.S            | 13 +++---

> >  arch/x86/crypto/sha512-avx-asm.S             | 41 +++++++++----------

> >  arch/x86/crypto/sha512-avx2-asm.S            | 42 ++++++++++----------

> >  arch/x86/crypto/sha512-ssse3-asm.S           | 41 +++++++++----------

> >  tools/objtool/check.c                        | 14 ++++++-

> >  11 files changed, 98 insertions(+), 111 deletions(-)

> 

> One nit, there's lots and lots of:

> 

> 	mov %rbp, %rsp

> 	pop %rbp

> 

> and we have this 'leave' instruction that does exactly that, should we

> be using it?


I'd considered that, but LEAVE is more cryptic (no pun intended).  This
code often has "surprises", so I prefer the readability of the more
explicit instructions.

> Otherwise:

> 

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


Thanks.

-- 
Josh
Herbert Xu March 3, 2021, 8:47 a.m. UTC | #4
On Wed, Feb 24, 2021 at 10:29:13AM -0600, Josh Poimboeuf wrote:
> Standardize the crypto asm to make it resemble compiler-generated code,

> so that objtool can understand it.

> 

> This magically enables ORC unwinding from crypto code.  It also fixes

> the last known remaining objtool warnings on vmlinux.o, for LTO and

> more.

> 

> Josh Poimboeuf (13):

>   objtool: Support asm jump tables

>   x86/crypto/aesni-intel_avx: Remove unused macros

>   x86/crypto/aesni-intel_avx: Fix register usage comments

>   x86/crypto/aesni-intel_avx: Standardize stack alignment prologue

>   x86/crypto/camellia-aesni-avx2: Unconditionally allocate stack buffer

>   x86/crypto/crc32c-pcl-intel: Standardize jump table

>   x86/crypto/sha_ni: Standardize stack alignment prologue

>   x86/crypto/sha1_avx2: Standardize stack alignment prologue

>   x86/crypto/sha256-avx2: Standardize stack alignment prologue

>   x86/crypto/sha512-avx: Standardize stack alignment prologue

>   x86/crypto/sha512-avx2: Standardize stack alignment prologue

>   x86/crypto/sha512-ssse3: Standardize stack alignment prologue

>   x86/crypto: Enable objtool in crypto code

> 

>  arch/x86/crypto/Makefile                     |  2 -

>  arch/x86/crypto/aesni-intel_avx-x86_64.S     | 28 +++++--------

>  arch/x86/crypto/camellia-aesni-avx2-asm_64.S |  5 +--

>  arch/x86/crypto/crc32c-pcl-intel-asm_64.S    |  7 +---

>  arch/x86/crypto/sha1_avx2_x86_64_asm.S       |  8 ++--

>  arch/x86/crypto/sha1_ni_asm.S                |  8 ++--

>  arch/x86/crypto/sha256-avx2-asm.S            | 13 +++---

>  arch/x86/crypto/sha512-avx-asm.S             | 41 +++++++++----------

>  arch/x86/crypto/sha512-avx2-asm.S            | 42 ++++++++++----------

>  arch/x86/crypto/sha512-ssse3-asm.S           | 41 +++++++++----------

>  tools/objtool/check.c                        | 14 ++++++-

>  11 files changed, 98 insertions(+), 111 deletions(-)


Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt