mbox series

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

Message ID cover.1614182415.git.jpoimboe@redhat.com (mailing list archive)
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

Ard Biesheuvel Feb. 24, 2021, 5:10 p.m. UTC | #1
On Wed, 24 Feb 2021 at 17:29, Josh Poimboeuf <jpoimboe@redhat.com> 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
>

I gave this series a spin with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y,
and all the tests still pass.

Tested-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Ard Biesheuvel <ardb@kernel.org>



>  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(-)
>
> --
> 2.29.2
>
Sami Tolvanen Feb. 24, 2021, 9:48 p.m. UTC | #2
Hi Josh,

On Wed, Feb 24, 2021 at 8:29 AM Josh Poimboeuf <jpoimboe@redhat.com> 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(-)

Thank you for working on this! I can confirm that this series fixes
the objtool warnings in crypto code with allyesconfig + Clang LTO.

Tested-by: Sami Tolvanen <samitolvanen@google.com>

Sami
Peter Zijlstra Feb. 25, 2021, 9:46 a.m. UTC | #3
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 | #4
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.
Herbert Xu March 3, 2021, 8:47 a.m. UTC | #5
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>