Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1
@ 2020-06-22 15:50 Sedat Dilek
  2020-06-22 16:52 ` Sedat Dilek
  2020-06-22 20:32 ` Nick Desaulniers
  0 siblings, 2 replies; 4+ messages in thread
From: Sedat Dilek @ 2020-06-22 15:50 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, linux-crypto, linux-kernel,
	clang-built-linux
  Cc: Sedat Dilek, Craig Topper, Craig Topper

When building with LLVM_IAS=1 means using Clang's Integrated Assembly (IAS)
from LLVM/Clang >= v10.0.1-rc1+ instead of GNU/as from GNU/binutils
I see the following breakage in Debian/testing AMD64:

<instantiation>:15:74: error: too many positional arguments
 PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
                                                                         ^
 arch/x86/crypto/aesni-intel_asm.S:1598:2: note: while in macro instantiation
 GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
 ^
<instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
 GHASH_4_ENCRYPT_4_PARALLEL_dec %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
 ^
arch/x86/crypto/aesni-intel_asm.S:1599:2: note: while in macro instantiation
 GCM_ENC_DEC dec
 ^
<instantiation>:15:74: error: too many positional arguments
 PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
                                                                         ^
arch/x86/crypto/aesni-intel_asm.S:1686:2: note: while in macro instantiation
 GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
 ^
<instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
 GHASH_4_ENCRYPT_4_PARALLEL_enc %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
 ^
arch/x86/crypto/aesni-intel_asm.S:1687:2: note: while in macro instantiation
 GCM_ENC_DEC enc
 
Craig Topper suggested me in ClangBuiltLinux issue #1050:

> I think the "too many positional arguments" is because the parser isn't able
> to handle the trailing commas.
> 
> The "unknown use of instruction mnemonic" is because the macro was named
> GHASH_4_ENCRYPT_4_PARALLEL_DEC but its being instantiated with 
> GHASH_4_ENCRYPT_4_PARALLEL_dec I guess gas ignores case on the
> macro instantiation, but llvm doesn't.

First, I removed the trailing comma in the PRECOMPUTE line.

Second, I substituted:
1. GCM_ENC_DEC dec -> GCM_ENC_DEC DEC
2. GCM_ENC_DEC enc -> GCM_ENC_DEC ENC

With these changes I was able to build with LLVM_IAS=1 and boot on bare metal.

As llvm-toolchain I used v10.0.1-rc1+ and v11.0.0-git pre-releases:
1. release/10.x Git: 2dc664d578f0e9c8ea5975eed745e322fa77bffe
2.       master Git: 8da5b9083691b557f50f72ab099598bb291aec5f (default)

Just for the sake of completeness:
1. CONFIG_DEBUG_INFO_DWARF4=y
2. OBJDUMP=llvm-objdump (passed to my make-line)

Please have a look into "llvm.rst" kernel-doc for further informations and
how to pass LLVM kbuild-options to your make-line.

I confirmed that this works with Linux-kernel v5.7.3 and v5.7.5 final.

NOTE: This patch is on top of Linux v5.7 final.

Thanks to Craig and the folks from the ClangBuiltLinux project.

Cc: Craig Topper <craig.topper@intel.com>
Cc: Craig Topper <craig.topper@gmail.com>
Cc: Nick Desaulniers ndesaulniers@google.com
Cc: "ClangBuiltLinux" <clang-built-linux@googlegroups.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1050
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/llvm.rst

---
 arch/x86/crypto/aesni-intel_asm.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index cad6e1bfa7d5..983eb2eec51a 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -266,7 +266,7 @@ ALL_F:      .octa 0xffffffffffffffffffffffffffffffff
 	PSHUFB_XMM %xmm2, %xmm0
 	movdqu %xmm0, CurCount(%arg2) # ctx_data.current_counter = iv
 
-	PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
+	PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7
 	movdqu HashKey(%arg2), %xmm13
 
 	CALC_AAD_HASH %xmm13, \AAD, \AADLEN, %xmm0, %xmm1, %xmm2, %xmm3, \
@@ -1596,7 +1596,7 @@ SYM_FUNC_START(aesni_gcm_dec)
 	FUNC_SAVE
 
 	GCM_INIT %arg6, arg7, arg8, arg9
-	GCM_ENC_DEC dec
+	GCM_ENC_DEC DEC
 	GCM_COMPLETE arg10, arg11
 	FUNC_RESTORE
 	ret
@@ -1684,7 +1684,7 @@ SYM_FUNC_START(aesni_gcm_enc)
 	FUNC_SAVE
 
 	GCM_INIT %arg6, arg7, arg8, arg9
-	GCM_ENC_DEC enc
+	GCM_ENC_DEC ENC
 
 	GCM_COMPLETE arg10, arg11
 	FUNC_RESTORE
@@ -1719,7 +1719,7 @@ SYM_FUNC_END(aesni_gcm_init)
 */
 SYM_FUNC_START(aesni_gcm_enc_update)
 	FUNC_SAVE
-	GCM_ENC_DEC enc
+	GCM_ENC_DEC ENC
 	FUNC_RESTORE
 	ret
 SYM_FUNC_END(aesni_gcm_enc_update)
@@ -1734,7 +1734,7 @@ SYM_FUNC_END(aesni_gcm_enc_update)
 */
 SYM_FUNC_START(aesni_gcm_dec_update)
 	FUNC_SAVE
-	GCM_ENC_DEC dec
+	GCM_ENC_DEC DEC
 	FUNC_RESTORE
 	ret
 SYM_FUNC_END(aesni_gcm_dec_update)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1
  2020-06-22 15:50 [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1 Sedat Dilek
@ 2020-06-22 16:52 ` Sedat Dilek
  2020-06-22 20:32 ` Nick Desaulniers
  1 sibling, 0 replies; 4+ messages in thread
From: Sedat Dilek @ 2020-06-22 16:52 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin, linux-crypto, linux-kernel,
	Clang-Built-Linux ML
  Cc: Craig Topper, Craig Topper

On Mon, Jun 22, 2020 at 5:50 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:

> Craig Topper suggested me in ClangBuiltLinux issue #1050:
>
> > I think the "too many positional arguments" is because the parser isn't able
> > to handle the trailing commas.
> >
> > The "unknown use of instruction mnemonic" is because the macro was named
> > GHASH_4_ENCRYPT_4_PARALLEL_DEC but its being instantiated with
> > GHASH_4_ENCRYPT_4_PARALLEL_dec I guess gas ignores case on the
> > macro instantiation, but llvm doesn't.
>

> Cc: Craig Topper <craig.topper@intel.com>
> Cc: Craig Topper <craig.topper@gmail.com>

That should be a s/Cc/Suggested-by/.

- Sedat -

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1
  2020-06-22 15:50 [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1 Sedat Dilek
  2020-06-22 16:52 ` Sedat Dilek
@ 2020-06-22 20:32 ` Nick Desaulniers
  2020-06-23  3:15   ` Sedat Dilek
  1 sibling, 1 reply; 4+ messages in thread
From: Nick Desaulniers @ 2020-06-22 20:32 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	LKML, clang-built-linux, Craig Topper, Craig Topper, Jian Cai

On Mon, Jun 22, 2020 at 8:50 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> When building with LLVM_IAS=1 means using Clang's Integrated Assembly (IAS)
> from LLVM/Clang >= v10.0.1-rc1+ instead of GNU/as from GNU/binutils
> I see the following breakage in Debian/testing AMD64:
>
> <instantiation>:15:74: error: too many positional arguments
>  PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
>                                                                          ^
>  arch/x86/crypto/aesni-intel_asm.S:1598:2: note: while in macro instantiation
>  GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
>  ^
> <instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
>  GHASH_4_ENCRYPT_4_PARALLEL_dec %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
>  ^
> arch/x86/crypto/aesni-intel_asm.S:1599:2: note: while in macro instantiation
>  GCM_ENC_DEC dec
>  ^
> <instantiation>:15:74: error: too many positional arguments
>  PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
>                                                                          ^
> arch/x86/crypto/aesni-intel_asm.S:1686:2: note: while in macro instantiation
>  GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
>  ^
> <instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
>  GHASH_4_ENCRYPT_4_PARALLEL_enc %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
>  ^
> arch/x86/crypto/aesni-intel_asm.S:1687:2: note: while in macro instantiation
>  GCM_ENC_DEC enc

=== I think from here to...


>
> Craig Topper suggested me in ClangBuiltLinux issue #1050:
>
> > I think the "too many positional arguments" is because the parser isn't able
> > to handle the trailing commas.
> >
> > The "unknown use of instruction mnemonic" is because the macro was named
> > GHASH_4_ENCRYPT_4_PARALLEL_DEC but its being instantiated with
> > GHASH_4_ENCRYPT_4_PARALLEL_dec I guess gas ignores case on the
> > macro instantiation, but llvm doesn't.

Yep, see also:
commit 6f5459da2b87 ("arm64: alternative: fix build with clang
integrated assembler")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6f5459da2b8736720afdbd67c4bd2d1edba7d0e3

>
> First, I removed the trailing comma in the PRECOMPUTE line.
>
> Second, I substituted:
> 1. GCM_ENC_DEC dec -> GCM_ENC_DEC DEC
> 2. GCM_ENC_DEC enc -> GCM_ENC_DEC ENC
>
> With these changes I was able to build with LLVM_IAS=1 and boot on bare metal.
>
> As llvm-toolchain I used v10.0.1-rc1+ and v11.0.0-git pre-releases:
> 1. release/10.x Git: 2dc664d578f0e9c8ea5975eed745e322fa77bffe
> 2.       master Git: 8da5b9083691b557f50f72ab099598bb291aec5f (default)
>
> Just for the sake of completeness:
> 1. CONFIG_DEBUG_INFO_DWARF4=y
> 2. OBJDUMP=llvm-objdump (passed to my make-line)
>
> Please have a look into "llvm.rst" kernel-doc for further informations and
> how to pass LLVM kbuild-options to your make-line.
>
> I confirmed that this works with Linux-kernel v5.7.3 and v5.7.5 final.
>
> NOTE: This patch is on top of Linux v5.7 final.
>
> Thanks to Craig and the folks from the ClangBuiltLinux project.

===...here can be cut out from the commit message.

>
> Cc: Craig Topper <craig.topper@intel.com>
> Cc: Craig Topper <craig.topper@gmail.com>

I'd pick one or the other email addresses, and just use that one.
Craig seems to commit to LLVM with craig.topper@intel.com, so I
recommend that one.

> Cc: Nick Desaulniers ndesaulniers@google.com

Thanks for the explicit CC, though I do monitor the below list actively.

> Cc: "ClangBuiltLinux" <clang-built-linux@googlegroups.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1050
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/llvm.rst

^ probably don't need that link either.

>
> ---
>  arch/x86/crypto/aesni-intel_asm.S | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
> index cad6e1bfa7d5..983eb2eec51a 100644
> --- a/arch/x86/crypto/aesni-intel_asm.S
> +++ b/arch/x86/crypto/aesni-intel_asm.S
> @@ -266,7 +266,7 @@ ALL_F:      .octa 0xffffffffffffffffffffffffffffffff
>         PSHUFB_XMM %xmm2, %xmm0
>         movdqu %xmm0, CurCount(%arg2) # ctx_data.current_counter = iv
>
> -       PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
> +       PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7
>         movdqu HashKey(%arg2), %xmm13
>
>         CALC_AAD_HASH %xmm13, \AAD, \AADLEN, %xmm0, %xmm1, %xmm2, %xmm3, \


There's a comparison on L386
 386 .ifc \operation, dec
Also, L407, L393, L672, L808, L841, L935, L941, L947,

If we change the `\operation` macro parameter to be `DEC` instead of
`dec`, does this comparison still hold true?  I would expect not if
LLVM's integrated assembler is case sensitive?  Otherwise we're
probably missing instructions for the case of `DEC`.  In that case,
that should probably get fixed.

Interesting, looks like GAS *is* case sensitive for `.ifc` directives?
```
$ cat foo.s
.macro foo op
.ifc \op, dec
  subq r8, r9
.else
  addq $0, r8
.endif
.endm
foo DEC

$ clang foo.s -c
$ llvm-objdump -d foo.o
...
       0: 48 83 04 25 00 00 00 00 00    addq    $0, 0
$ as foo.s -c
$ llvm-objdump -d foo.o
...
       0: 48 83 04 25 00 00 00 00 00    addq    $0, 0
```
In that case, it seems that this patch probably breaks the DEC case
for both toolchains.

Just to triple check, forgetting LLVM for a minute, let's compare the
disassembly before and after.

$ make -j71 defconfig
$ make -j71 menuconfig
<enable CRYPTO_AES_NI_INTEL>
$ make -j71 arch/x86/crypto/aesni-intel_asm.o
$ llvm-objdump -dr arch/x86/crypto/aesni-intel_asm.o > prepatch.txt
<apply your patch>
$ make -j71 arch/x86/crypto/aesni-intel_asm.o
$ llvm-objdump -dr arch/x86/crypto/aesni-intel_asm.o > postpatch.txt
$ diff -u <(cat prepatch.txt | tr -s ' ' | cut -d '   ' -f 2-) <(cat
postpatch.txt| tr -s ' ' | cut -d '      ' -f 2-) | less

It's not the easiest to tell, since I should have left off `-r` for
relocations from objdump, but you can clearly see cases of different
disassembly.  Even the address of labels changes due to missing
instructions.

In that case, it's probably a smaller change to just renamed the macros
GHASH_4_ENCRYPT_4_PARALLEL_ENC
GHASH_4_ENCRYPT_4_PARALLEL_DEC

to

GHASH_4_ENCRYPT_4_PARALLEL_enc
GHASH_4_ENCRYPT_4_PARALLEL_dec

respectively, though using ALL CAPS is likely more consistent at the
cost of more lines changed.  Also, sorry I didn't catch this on the
earlier thread; my mistake.

> @@ -1596,7 +1596,7 @@ SYM_FUNC_START(aesni_gcm_dec)
>         FUNC_SAVE
>
>         GCM_INIT %arg6, arg7, arg8, arg9
> -       GCM_ENC_DEC dec
> +       GCM_ENC_DEC DEC
>         GCM_COMPLETE arg10, arg11
>         FUNC_RESTORE
>         ret
> @@ -1684,7 +1684,7 @@ SYM_FUNC_START(aesni_gcm_enc)
>         FUNC_SAVE
>
>         GCM_INIT %arg6, arg7, arg8, arg9
> -       GCM_ENC_DEC enc
> +       GCM_ENC_DEC ENC
>
>         GCM_COMPLETE arg10, arg11
>         FUNC_RESTORE
> @@ -1719,7 +1719,7 @@ SYM_FUNC_END(aesni_gcm_init)
>  */
>  SYM_FUNC_START(aesni_gcm_enc_update)
>         FUNC_SAVE
> -       GCM_ENC_DEC enc
> +       GCM_ENC_DEC ENC
>         FUNC_RESTORE
>         ret
>  SYM_FUNC_END(aesni_gcm_enc_update)
> @@ -1734,7 +1734,7 @@ SYM_FUNC_END(aesni_gcm_enc_update)
>  */
>  SYM_FUNC_START(aesni_gcm_dec_update)
>         FUNC_SAVE
> -       GCM_ENC_DEC dec
> +       GCM_ENC_DEC DEC
>         FUNC_RESTORE
>         ret
>  SYM_FUNC_END(aesni_gcm_dec_update)
> --

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1
  2020-06-22 20:32 ` Nick Desaulniers
@ 2020-06-23  3:15   ` Sedat Dilek
  0 siblings, 0 replies; 4+ messages in thread
From: Sedat Dilek @ 2020-06-23  3:15 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	LKML, clang-built-linux, Craig Topper, Craig Topper, Jian Cai

On Mon, Jun 22, 2020 at 10:33 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Jun 22, 2020 at 8:50 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > When building with LLVM_IAS=1 means using Clang's Integrated Assembly (IAS)
> > from LLVM/Clang >= v10.0.1-rc1+ instead of GNU/as from GNU/binutils
> > I see the following breakage in Debian/testing AMD64:
> >
> > <instantiation>:15:74: error: too many positional arguments
> >  PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
> >                                                                          ^
> >  arch/x86/crypto/aesni-intel_asm.S:1598:2: note: while in macro instantiation
> >  GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
> >  ^
> > <instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
> >  GHASH_4_ENCRYPT_4_PARALLEL_dec %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
> >  ^
> > arch/x86/crypto/aesni-intel_asm.S:1599:2: note: while in macro instantiation
> >  GCM_ENC_DEC dec
> >  ^
> > <instantiation>:15:74: error: too many positional arguments
> >  PRECOMPUTE 8*3+8(%rsp), %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
> >                                                                          ^
> > arch/x86/crypto/aesni-intel_asm.S:1686:2: note: while in macro instantiation
> >  GCM_INIT %r9, 8*3 +8(%rsp), 8*3 +16(%rsp), 8*3 +24(%rsp)
> >  ^
> > <instantiation>:47:2: error: unknown use of instruction mnemonic without a size suffix
> >  GHASH_4_ENCRYPT_4_PARALLEL_enc %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7, %xmm8, enc
> >  ^
> > arch/x86/crypto/aesni-intel_asm.S:1687:2: note: while in macro instantiation
> >  GCM_ENC_DEC enc
>
> === I think from here to...
>
>
> >
> > Craig Topper suggested me in ClangBuiltLinux issue #1050:
> >
> > > I think the "too many positional arguments" is because the parser isn't able
> > > to handle the trailing commas.
> > >
> > > The "unknown use of instruction mnemonic" is because the macro was named
> > > GHASH_4_ENCRYPT_4_PARALLEL_DEC but its being instantiated with
> > > GHASH_4_ENCRYPT_4_PARALLEL_dec I guess gas ignores case on the
> > > macro instantiation, but llvm doesn't.
>
> Yep, see also:
> commit 6f5459da2b87 ("arm64: alternative: fix build with clang
> integrated assembler")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6f5459da2b8736720afdbd67c4bd2d1edba7d0e3
>
> >
> > First, I removed the trailing comma in the PRECOMPUTE line.
> >
> > Second, I substituted:
> > 1. GCM_ENC_DEC dec -> GCM_ENC_DEC DEC
> > 2. GCM_ENC_DEC enc -> GCM_ENC_DEC ENC
> >
> > With these changes I was able to build with LLVM_IAS=1 and boot on bare metal.
> >
> > As llvm-toolchain I used v10.0.1-rc1+ and v11.0.0-git pre-releases:
> > 1. release/10.x Git: 2dc664d578f0e9c8ea5975eed745e322fa77bffe
> > 2.       master Git: 8da5b9083691b557f50f72ab099598bb291aec5f (default)
> >
> > Just for the sake of completeness:
> > 1. CONFIG_DEBUG_INFO_DWARF4=y
> > 2. OBJDUMP=llvm-objdump (passed to my make-line)
> >
> > Please have a look into "llvm.rst" kernel-doc for further informations and
> > how to pass LLVM kbuild-options to your make-line.
> >
> > I confirmed that this works with Linux-kernel v5.7.3 and v5.7.5 final.
> >
> > NOTE: This patch is on top of Linux v5.7 final.
> >
> > Thanks to Craig and the folks from the ClangBuiltLinux project.
>
> ===...here can be cut out from the commit message.
>
> >
> > Cc: Craig Topper <craig.topper@intel.com>
> > Cc: Craig Topper <craig.topper@gmail.com>
>
> I'd pick one or the other email addresses, and just use that one.
> Craig seems to commit to LLVM with craig.topper@intel.com, so I
> recommend that one.
>
> > Cc: Nick Desaulniers ndesaulniers@google.com
>
> Thanks for the explicit CC, though I do monitor the below list actively.
>
> > Cc: "ClangBuiltLinux" <clang-built-linux@googlegroups.com>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1050
> > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/llvm.rst
>
> ^ probably don't need that link either.
>
> >
> > ---
> >  arch/x86/crypto/aesni-intel_asm.S | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
> > index cad6e1bfa7d5..983eb2eec51a 100644
> > --- a/arch/x86/crypto/aesni-intel_asm.S
> > +++ b/arch/x86/crypto/aesni-intel_asm.S
> > @@ -266,7 +266,7 @@ ALL_F:      .octa 0xffffffffffffffffffffffffffffffff
> >         PSHUFB_XMM %xmm2, %xmm0
> >         movdqu %xmm0, CurCount(%arg2) # ctx_data.current_counter = iv
> >
> > -       PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
> > +       PRECOMPUTE \SUBKEY, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7
> >         movdqu HashKey(%arg2), %xmm13
> >
> >         CALC_AAD_HASH %xmm13, \AAD, \AADLEN, %xmm0, %xmm1, %xmm2, %xmm3, \
>
>
> There's a comparison on L386
>  386 .ifc \operation, dec
> Also, L407, L393, L672, L808, L841, L935, L941, L947,
>
> If we change the `\operation` macro parameter to be `DEC` instead of
> `dec`, does this comparison still hold true?  I would expect not if
> LLVM's integrated assembler is case sensitive?  Otherwise we're
> probably missing instructions for the case of `DEC`.  In that case,
> that should probably get fixed.
>
> Interesting, looks like GAS *is* case sensitive for `.ifc` directives?
> ```
> $ cat foo.s
> .macro foo op
> .ifc \op, dec
>   subq r8, r9
> .else
>   addq $0, r8
> .endif
> .endm
> foo DEC
>
> $ clang foo.s -c
> $ llvm-objdump -d foo.o
> ...
>        0: 48 83 04 25 00 00 00 00 00    addq    $0, 0
> $ as foo.s -c
> $ llvm-objdump -d foo.o
> ...
>        0: 48 83 04 25 00 00 00 00 00    addq    $0, 0
> ```
> In that case, it seems that this patch probably breaks the DEC case
> for both toolchains.
>
> Just to triple check, forgetting LLVM for a minute, let's compare the
> disassembly before and after.
>
> $ make -j71 defconfig
> $ make -j71 menuconfig
> <enable CRYPTO_AES_NI_INTEL>
> $ make -j71 arch/x86/crypto/aesni-intel_asm.o
> $ llvm-objdump -dr arch/x86/crypto/aesni-intel_asm.o > prepatch.txt
> <apply your patch>
> $ make -j71 arch/x86/crypto/aesni-intel_asm.o
> $ llvm-objdump -dr arch/x86/crypto/aesni-intel_asm.o > postpatch.txt
> $ diff -u <(cat prepatch.txt | tr -s ' ' | cut -d '   ' -f 2-) <(cat
> postpatch.txt| tr -s ' ' | cut -d '      ' -f 2-) | less
>
> It's not the easiest to tell, since I should have left off `-r` for
> relocations from objdump, but you can clearly see cases of different
> disassembly.  Even the address of labels changes due to missing
> instructions.
>
> In that case, it's probably a smaller change to just renamed the macros
> GHASH_4_ENCRYPT_4_PARALLEL_ENC
> GHASH_4_ENCRYPT_4_PARALLEL_DEC
>
> to
>
> GHASH_4_ENCRYPT_4_PARALLEL_enc
> GHASH_4_ENCRYPT_4_PARALLEL_dec
>
> respectively, though using ALL CAPS is likely more consistent at the
> cost of more lines changed.  Also, sorry I didn't catch this on the
> earlier thread; my mistake.
>
> > @@ -1596,7 +1596,7 @@ SYM_FUNC_START(aesni_gcm_dec)
> >         FUNC_SAVE
> >
> >         GCM_INIT %arg6, arg7, arg8, arg9
> > -       GCM_ENC_DEC dec
> > +       GCM_ENC_DEC DEC
> >         GCM_COMPLETE arg10, arg11
> >         FUNC_RESTORE
> >         ret
> > @@ -1684,7 +1684,7 @@ SYM_FUNC_START(aesni_gcm_enc)
> >         FUNC_SAVE
> >
> >         GCM_INIT %arg6, arg7, arg8, arg9
> > -       GCM_ENC_DEC enc
> > +       GCM_ENC_DEC ENC
> >
> >         GCM_COMPLETE arg10, arg11
> >         FUNC_RESTORE
> > @@ -1719,7 +1719,7 @@ SYM_FUNC_END(aesni_gcm_init)
> >  */
> >  SYM_FUNC_START(aesni_gcm_enc_update)
> >         FUNC_SAVE
> > -       GCM_ENC_DEC enc
> > +       GCM_ENC_DEC ENC
> >         FUNC_RESTORE
> >         ret
> >  SYM_FUNC_END(aesni_gcm_enc_update)
> > @@ -1734,7 +1734,7 @@ SYM_FUNC_END(aesni_gcm_enc_update)
> >  */
> >  SYM_FUNC_START(aesni_gcm_dec_update)
> >         FUNC_SAVE
> > -       GCM_ENC_DEC dec
> > +       GCM_ENC_DEC DEC
> >         FUNC_RESTORE
> >         ret
> >  SYM_FUNC_END(aesni_gcm_dec_update)
> > --
>

Thanks for your feedback and double-checking Nick.

I have submitted a v3 of my patch - should have all relevant material.

- Sedat -

[1] https://lore.kernel.org/lkml/20200623030600.18848-1-sedat.dilek@gmail.com/
[2] https://lore.kernel.org/patchwork/patch/1261340/
[3] https://github.com/ClangBuiltLinux/linux/issues/1050#issuecomment-647879758

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 15:50 [PATCH 5.7] x86/crypto: aesni: Fix build with LLVM_IAS=1 Sedat Dilek
2020-06-22 16:52 ` Sedat Dilek
2020-06-22 20:32 ` Nick Desaulniers
2020-06-23  3:15   ` Sedat Dilek

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git