[PATCH] x86, crypto: Check if gas supports CRC32

[PATCH] x86, crypto: Check if gas supports CRC32

[Borislav Petkov]

Building current kernel with some old toolchain (gcc 4.1.2 and gas 2.17)
chokes with:

arch/x86/crypto/crc32c-pcl-intel-asm_64.S: Assembler messages:
arch/x86/crypto/crc32c-pcl-intel-asm_64.S:128: Error: no such instruction: `crc32b %bl,%r8d'
arch/x86/crypto/crc32c-pcl-intel-asm_64.S:204: Error: no such instruction: `crc32q -i*8(%rcx),%r8'
...

due to the fact that gas doesn't know the CRC32 instruction. Check that
before building.

Cc: linux-crypto@vger.kernel.org
Signed-off-by: Borislav Petkov <bp@suse.de>
---

So this is one possibility to address this. Code already
has the ifdeffery around crc_pcl() which is implemented in
crc32c-pcl-intel-asm_64.S so we can piggyback on that and not build that
file if gas doesn't know CRC32.

If no CRC32 support, it still builds fine silently, however it would be
better to probably say that due to old toolchain, kernel doesn't include
fast CRC32 stuff in crc32c-intel.ko.

Hmmm.


 arch/x86/crypto/Makefile            |  8 +++++++-
 arch/x86/crypto/crc32c-intel_glue.c |  7 ++++---
 scripts/gas-can-do-crc32.sh         | 12 ++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100755 scripts/gas-can-do-crc32.sh

diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 61d6e281898b..707bf7ecb903 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -83,7 +83,13 @@ ifeq ($(avx2_supported),yes)
 sha1-ssse3-y += sha1_avx2_x86_64_asm.o
 endif
 crc32c-intel-y := crc32c-intel_glue.o
-crc32c-intel-$(CONFIG_64BIT) += crc32c-pcl-intel-asm_64.o
+
+gas_can_crc32 := $(srctree)/scripts/gas-can-do-crc32.sh
+CONFIG_GAS_SUPPORTS_CRC32=$(shell $(CONFIG_SHELL) $(gas_can_crc32) $(AS))
+
+ifdef CONFIG_64BIT
+crc32c-intel-$(CONFIG_GAS_SUPPORTS_CRC32) += crc32c-pcl-intel-asm_64.o
+endif
 crc32-pclmul-y := crc32-pclmul_asm.o crc32-pclmul_glue.o
 sha256-ssse3-y := sha256-ssse3-asm.o sha256-avx-asm.o sha256-avx2-asm.o sha256_ssse3_glue.o
 sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_ssse3_glue.o
diff --git a/arch/x86/crypto/crc32c-intel_glue.c b/arch/x86/crypto/crc32c-intel_glue.c
index 6812ad98355c..4ce8e2db2984 100644
--- a/arch/x86/crypto/crc32c-intel_glue.c
+++ b/arch/x86/crypto/crc32c-intel_glue.c
@@ -46,7 +46,7 @@
 #define REX_PRE
 #endif
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
 /*
  * use carryless multiply version of crc32c when buffer
  * size is >= 512 (when eager fpu is enabled) or
@@ -181,7 +181,7 @@ static int crc32c_intel_cra_init(struct crypto_tfm *tfm)
 	return 0;
 }
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
 static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,
 			       unsigned int len)
 {
@@ -257,7 +257,8 @@ static int __init crc32c_intel_mod_init(void)
 {
 	if (!x86_match_cpu(crc32c_cpu_id))
 		return -ENODEV;
-#ifdef CONFIG_X86_64
+
+#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
 	if (cpu_has_pclmulqdq) {
 		alg.update = crc32c_pcl_intel_update;
 		alg.finup = crc32c_pcl_intel_finup;
diff --git a/scripts/gas-can-do-crc32.sh b/scripts/gas-can-do-crc32.sh
new file mode 100755
index 000000000000..88ff476bd3cc
--- /dev/null
+++ b/scripts/gas-can-do-crc32.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+TMP=$(mktemp)
+
+echo "crc32 %rax,%rbx" | $* --warn - -o $TMP 2>/dev/null
+if [ "$?" -eq "0" ]; then
+	echo y
+else
+	echo n
+fi
+
+rm -f $TMP
-- 
2.0.0


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86, crypto: Check if gas supports CRC32

[Mathias Krause]

On Wed, Jul 30, 2014 at 06:47:01PM +0200, Borislav Petkov wrote:
> Building current kernel with some old toolchain (gcc 4.1.2 and gas 2.17)
> chokes with:
> 
> arch/x86/crypto/crc32c-pcl-intel-asm_64.S: Assembler messages:
> arch/x86/crypto/crc32c-pcl-intel-asm_64.S:128: Error: no such instruction: `crc32b %bl,%r8d'
> arch/x86/crypto/crc32c-pcl-intel-asm_64.S:204: Error: no such instruction: `crc32q -i*8(%rcx),%r8'
> ...
> 
> due to the fact that gas doesn't know the CRC32 instruction. Check that
> before building.
> 
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> 
> So this is one possibility to address this. Code already
> has the ifdeffery around crc_pcl() which is implemented in
> crc32c-pcl-intel-asm_64.S so we can piggyback on that and not build that
> file if gas doesn't know CRC32.
> 
> If no CRC32 support, it still builds fine silently, however it would be
> better to probably say that due to old toolchain, kernel doesn't include
> fast CRC32 stuff in crc32c-intel.ko.
> 
> Hmmm.
> 
> 
>  arch/x86/crypto/Makefile            |  8 +++++++-
>  arch/x86/crypto/crc32c-intel_glue.c |  7 ++++---
>  scripts/gas-can-do-crc32.sh         | 12 ++++++++++++
>  3 files changed, 23 insertions(+), 4 deletions(-)
>  create mode 100755 scripts/gas-can-do-crc32.sh
> 
> diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
> index 61d6e281898b..707bf7ecb903 100644
> --- a/arch/x86/crypto/Makefile
> +++ b/arch/x86/crypto/Makefile
> @@ -83,7 +83,13 @@ ifeq ($(avx2_supported),yes)
>  sha1-ssse3-y += sha1_avx2_x86_64_asm.o
>  endif
>  crc32c-intel-y := crc32c-intel_glue.o
> -crc32c-intel-$(CONFIG_64BIT) += crc32c-pcl-intel-asm_64.o
> +
> +gas_can_crc32 := $(srctree)/scripts/gas-can-do-crc32.sh
> +CONFIG_GAS_SUPPORTS_CRC32=$(shell $(CONFIG_SHELL) $(gas_can_crc32) $(AS))
> +

This looks too complicated. We do have as-instr for exactly those kind
of tests. And, in fact, looking at arch/x86/Makefile we already have one
for crc32:

  asinstr += $(call as-instr,crc32l %eax$(comma)%eax,-DCONFIG_AS_CRC32=1)

So you can just used CONFIG_AS_CRC32 for your tests and drop the shell
script.

> +ifdef CONFIG_64BIT
> +crc32c-intel-$(CONFIG_GAS_SUPPORTS_CRC32) += crc32c-pcl-intel-asm_64.o
> +endif

s/CONFIG_GAS_SUPPORTS_CRC32/CONFIG_AS_CRC32/ here and for all further
uses.

>  crc32-pclmul-y := crc32-pclmul_asm.o crc32-pclmul_glue.o
>  sha256-ssse3-y := sha256-ssse3-asm.o sha256-avx-asm.o sha256-avx2-asm.o sha256_ssse3_glue.o
>  sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_ssse3_glue.o
> diff --git a/arch/x86/crypto/crc32c-intel_glue.c b/arch/x86/crypto/crc32c-intel_glue.c
> index 6812ad98355c..4ce8e2db2984 100644
> --- a/arch/x86/crypto/crc32c-intel_glue.c
> +++ b/arch/x86/crypto/crc32c-intel_glue.c
> @@ -46,7 +46,7 @@
>  #define REX_PRE
>  #endif
>  
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
>  /*
>   * use carryless multiply version of crc32c when buffer
>   * size is >= 512 (when eager fpu is enabled) or
> @@ -181,7 +181,7 @@ static int crc32c_intel_cra_init(struct crypto_tfm *tfm)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
>  static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,
>  			       unsigned int len)
>  {
> @@ -257,7 +257,8 @@ static int __init crc32c_intel_mod_init(void)
>  {
>  	if (!x86_match_cpu(crc32c_cpu_id))
>  		return -ENODEV;
> -#ifdef CONFIG_X86_64
> +
> +#if defined(CONFIG_X86_64) && defined(CONFIG_GAS_SUPPORTS_CRC32)
>  	if (cpu_has_pclmulqdq) {
>  		alg.update = crc32c_pcl_intel_update;
>  		alg.finup = crc32c_pcl_intel_finup;
> diff --git a/scripts/gas-can-do-crc32.sh b/scripts/gas-can-do-crc32.sh
> new file mode 100755
> index 000000000000..88ff476bd3cc
> --- /dev/null
> +++ b/scripts/gas-can-do-crc32.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +
> +TMP=$(mktemp)
> +
> +echo "crc32 %rax,%rbx" | $* --warn - -o $TMP 2>/dev/null
> +if [ "$?" -eq "0" ]; then
> +	echo y
> +else
> +	echo n
> +fi
> +
> +rm -f $TMP
> -- 
> 2.0.0
> 

Regards,
Mathias

Re: [PATCH] x86, crypto: Check if gas supports CRC32

[Mathias Krause]

On Wed, Jul 30, 2014 at 11:19:37PM +0200, Mathias Krause wrote:
> On Wed, Jul 30, 2014 at 06:47:01PM +0200, Borislav Petkov wrote:
> > [...]
> 
> This looks too complicated. We do have as-instr for exactly those kind
> of tests. And, in fact, looking at arch/x86/Makefile we already have one
> for crc32:
> 
>   asinstr += $(call as-instr,crc32l %eax$(comma)%eax,-DCONFIG_AS_CRC32=1)
> 
> So you can just used CONFIG_AS_CRC32 for your tests and drop the shell
> script.
> 
> > +ifdef CONFIG_64BIT
> > +crc32c-intel-$(CONFIG_GAS_SUPPORTS_CRC32) += crc32c-pcl-intel-asm_64.o
> > +endif
> 
> s/CONFIG_GAS_SUPPORTS_CRC32/CONFIG_AS_CRC32/ here and for all further
> uses.
> 

Gah, CONFIG_AS_CRC32 gets defined as a preprocessor symbol only so
cannot be used in makefiles. So crc32c-pcl-intel-asm_64.S needs a
"#ifdef CONFIG_AS_CRC32" guard and still be compiled for CONFIG_64BIT,
as it is now. It'll be an empty object for older binutils versions not
supporting the crc32 instruction.

Sorry for the confusion.


Regards,
Mathias

Re: [PATCH] x86, crypto: Check if gas supports CRC32

[Borislav Petkov]

On Wed, Jul 30, 2014 at 11:28:14PM +0200, Mathias Krause wrote:
> Gah, CONFIG_AS_CRC32 gets defined as a preprocessor symbol only so
> cannot be used in makefiles. So crc32c-pcl-intel-asm_64.S needs a
> "#ifdef CONFIG_AS_CRC32" guard and still be compiled for CONFIG_64BIT,
> as it is now. It'll be an empty object for older binutils versions not
> supporting the crc32 instruction.

Yeah, that makes it all even simpler, thanks!

We're still b0rked though:

arch/x86/crypto/crct10dif-pcl-asm_64.S: Assembler messages:
arch/x86/crypto/crct10dif-pcl-asm_64.S:147: Error: no such instruction: `pclmulqdq $0x0,%xmm10,%xmm0'
arch/x86/crypto/crct10dif-pcl-asm_64.S:148: Error: no such instruction: `pclmulqdq $0x11,%xmm10,%xmm8'
arch/x86/crypto/crct10dif-pcl-asm_64.S:149: Error: no such instruction: `pclmulqdq $0x0,%xmm10,%xmm1'
...

and need checking for more instructions. I'll play with this more
tomorrow.

Good night :-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86, crypto: Check if gas supports CRC32

[Mathias Krause]

On 31 July 2014 00:11, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Jul 30, 2014 at 11:28:14PM +0200, Mathias Krause wrote:
> > Gah, CONFIG_AS_CRC32 gets defined as a preprocessor symbol only so
> > cannot be used in makefiles. So crc32c-pcl-intel-asm_64.S needs a
> > "#ifdef CONFIG_AS_CRC32" guard and still be compiled for CONFIG_64BIT,
> > as it is now. It'll be an empty object for older binutils versions not
> > supporting the crc32 instruction.
>
> Yeah, that makes it all even simpler, thanks!
>
> We're still b0rked though:
>
> arch/x86/crypto/crct10dif-pcl-asm_64.S: Assembler messages:
> arch/x86/crypto/crct10dif-pcl-asm_64.S:147: Error: no such instruction: `pclmulqdq $0x0,%xmm10,%xmm0'
> arch/x86/crypto/crct10dif-pcl-asm_64.S:148: Error: no such instruction: `pclmulqdq $0x11,%xmm10,%xmm8'
> arch/x86/crypto/crct10dif-pcl-asm_64.S:149: Error: no such instruction: `pclmulqdq $0x0,%xmm10,%xmm1'
> ...
>
> and need checking for more instructions. I'll play with this more
> tomorrow.
>

You probably can reuse the AVX test for this -- either the
CONFIG_AS_AVX preprocessor one or the $(avx_supported) make one, local
to arch/x86/crypto/Makefile.
Even though the CLMUL feature has not much to with AVX (it has a
dedicated CPUID feature bit), support for it in binutils was added
together with AVX support, see
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=c0f3af977b0f28a0dc5a620110b8dcf9d8286f84

Regards,
Mathias

> Good night :-)
>
> --
> Regards/Gruss,
>     Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --