SHA1 transform: x86_64 AVX2 optimization - glue & build-v2

PATCH In review chandramouli narayanan 3 participants 8 messages Mar 12, 2014 – Mar 17, 2014

Subsystems (2)

x86 Crypto API

lore

Share & tools

lore

chandramouli narayanan

author

2014-03-12 18:47 UTC #0 lore raw

PATCH 2/2 SHA1 transform: x86_64 AVX2 optimization - glue & build-v2

This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.

Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.

On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.

Signed-off-by Chandramouli Narayanan <[email protected]>

arch/x86/crypto/Makefile +3 -0

index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o
 aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
 ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o ghash-clmulni-intel_glue.o
 sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
+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
 crc32-pclmul-y := crc32-pclmul_asm.o crc32-pclmul_glue.o

arch/x86/crypto/sha1_ssse3_glue.c +43 -7

91 lines not on this page · raw message

crypto/Kconfig +2 -2

index 7bcb70d..ce4012a 100644
@@ -491,14 +491,14 @@ config CRYPTO_SHA1
 	  SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2).
 
 config CRYPTO_SHA1_SSSE3
-	tristate "SHA1 digest algorithm (SSSE3/AVX)"
+	tristate "SHA1 digest algorithm (SSSE3/AVX/AVX2)"
 	depends on X86 && 64BIT
 	select CRYPTO_SHA1
 	select CRYPTO_HASH
 	help
 	  SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2) implemented
 	  using Supplemental SSE3 (SSSE3) instructions or Advanced Vector
-	  Extensions (AVX), when available.
+	  Extensions (AVX/AVX2), when available.
 
 config CRYPTO_SHA256_SSSE3
 	tristate "SHA256 digest algorithm (SSSE3/AVX/AVX2)"

Marek Vasut

2014-03-14 05:40 UTC #1 lore raw

Quoted 27 lines · chandramouli narayanan

This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o
fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o

arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83

Quoted 2 lines

arch/x86/crypto/Makefile +2 -0

+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o

Use:

sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o

And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut

chandramouli narayanan

author

2014-03-17 15:52 UTC #2 lore raw

Quoted 33 lines · Marek Vasut

Quoted 27 lines · chandramouli narayanan
This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o
fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83
Quoted 2 lines
arch/x86/crypto/Makefile +2 -0
+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o
Use:

Quoted 6 lines

sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o
And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut

Sorry for the delayed reply. Agreed, I will fix the dependency.

thanks
- mouli

Marek Vasut

2014-03-17 16:06 UTC #3 lore raw

Quoted 36 lines · chandramouli narayanan

Quoted 35 lines · Marek Vasut
Quoted 29 lines · chandramouli narayanan
This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o
fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83
Quoted 2 lines
arch/x86/crypto/Makefile +2 -0
+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o
Use:

Quoted 8 lines

Quoted 6 lines
sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o
And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut
Sorry for the delayed reply. Agreed, I will fix the dependency.

No problem, thanks!

Best regards,
Marek Vasut

chandramouli narayanan

author

2014-03-17 16:53 UTC #4 lore raw

Quoted 44 lines · Marek Vasut

Quoted 43 lines · chandramouli narayanan
Quoted 41 lines · Marek Vasut
Quoted 29 lines · chandramouli narayanan
This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o
fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83
Quoted 2 lines
arch/x86/crypto/Makefile +2 -0
+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o
Use:
sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o
And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut

Quoted 6 lines

Quoted 1 line
Sorry for the delayed reply. Agreed, I will fix the dependency.
No problem, thanks!
Best regards,
Marek Vasut

On second thoughts, with sha1-sse3-(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o, I have build issues and sha1_transform_avx2 undefined in sha1-sss3.ko.

I can rid #ifdef CONFIG_AS_AVX2 in patch1. The following works though:
ifeq ($(avx2_supported),yes)
sha1-ssse3-y += sha1_avx2_x86_64_asm.o
endif

thanks
- mouli

H. Peter Anvin

reviewer

2014-03-17 18:37 UTC #5 lore raw

Quoted 8 lines · chandramouli narayanan

On second thoughts, with sha1-sse3-(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o, I have build issues and sha1_transform_avx2 undefined in sha1-sss3.ko.
I can rid #ifdef CONFIG_AS_AVX2 in patch1. The following works though:
ifeq ($(avx2_supported),yes)
sha1-ssse3-y += sha1_avx2_x86_64_asm.o
endif

Yes, the sad thing is that the CONFIG_AS_* things aren't real config symbols, despite the name. They might be in the future when Kconfig can run test probes (something we have needed for a very long time.)

The "yes" versus "y", though, is a total faceplant.

-hpa

Marek Vasut

2014-03-17 18:44 UTC #6 lore raw

In reply to chandramouli narayanan

Quoted 4 lines · chandramouli narayanan

Quoted 3 lines · Marek Vasut
Quoted 2 lines · chandramouli narayanan
Quoted 1 line · Marek Vasut
On Wednesday, March 12, 2014 at 07:47:50 PM, chandramouli narayanan

Quoted 49 lines · wrote:

Quoted 42 lines
Quoted 42 lines
Quoted 41 lines
Quoted 30 lines
This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o
aesni-intel_glue.o fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83
Quoted 2 lines
arch/x86/crypto/Makefile +2 -0
+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o
Use:
sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o
And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut
Quoted 6 lines
Quoted 1 line
Sorry for the delayed reply. Agreed, I will fix the dependency.
No problem, thanks!
Best regards,
Marek Vasut

Quoted 8 lines

On second thoughts, with sha1-sse3-(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o, I have build issues and sha1_transform_avx2 undefined in sha1-sss3.ko.
I can rid #ifdef CONFIG_AS_AVX2 in patch1. The following works though:
ifeq ($(avx2_supported),yes)
sha1-ssse3-y += sha1_avx2_x86_64_asm.o
endif

Looking throughout the arch/x86/crypto/Makefile , this sha1-ssse3.o thing is a bit odd I think. Why exactly does this not follow suit with the camellia or serpent ciphers ? I mean, look at their build rules, they handle all of SSE2/AVX/AVX2 implementation and it's build. Can we not clean up the SHA1-SSSE3 to do exactly the same ? But please note I might be just plain wrong and if that's the case, let me know ;-)

btw. I noticed another nit in the code. You use __sha1_transform_avx2() , but there previous function using AVX1 is called sha1_transform_avx() . Drop those two underscores please for consistency's sake.

Thanks!

Best regards,
Marek Vasut

chandramouli narayanan

author

2014-03-17 19:08 UTC #7 lore raw

Quoted 64 lines · Marek Vasut

Quoted 4 lines · chandramouli narayanan
Quoted 3 lines · Marek Vasut
Quoted 2 lines · chandramouli narayanan
Quoted 1 line · Marek Vasut
On Wednesday, March 12, 2014 at 07:47:50 PM, chandramouli narayanan
Quoted 49 lines · wrote:
Quoted 42 lines
Quoted 42 lines
Quoted 41 lines
Quoted 30 lines
This git patch adds the glue, build and configuration changes to include x86_64 AVX2 optimization of SHA1 transform to crypto support. The patch has been tested with 3.14.0-rc1 kernel.
Changes from the initial version of this patch are in
a) check for BMI2 in addition to AVX2 support since __sha1_transform_avx2() uses rorx
b) Since the module build has dependency on 64bit, it is redundant to check it in the code here.
On a Haswell desktop, with turbo disabled and all cpus running at maximum frequency, tcrypt shows AVX2 performance improvement from 3% for 256 bytes update to 16% for 1024 bytes update over AVX implementation.
Signed-off-by Chandramouli Narayanan <[email protected]>
arch/x86/crypto/Makefile +0 -0
index 6ba54d6..61d6e28 100644
@@ -79,6 +79,9 @@ aesni-intel-y := aesni-intel_asm.o
aesni-intel_glue.o fpu.o aesni-intel-$(CONFIG_64BIT) += aesni-intel_avx-x86_64.o
ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o
ghash-clmulni-intel_glue.o sha1-ssse3-y := sha1_ssse3_asm.o sha1_ssse3_glue.o
arch/x86/crypto/Makefile aesni-intel-y := aesni-intel_asm.o aesni-intel_glue.o fpu.o · line 83
Quoted 2 lines
arch/x86/crypto/Makefile +2 -0
+ifeq ($(avx2_supported),yes)
+sha1-ssse3-y += sha1_avx2_x86_64_asm.o
Use:
sha1-ssse3-$(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o
And you will not need the CONFIG_AS_AVX2 ifdef in your previous patch, no ? [...]
Best regards,
Marek Vasut
Quoted 6 lines
Quoted 1 line
Sorry for the delayed reply. Agreed, I will fix the dependency.
No problem, thanks!
Best regards,
Marek Vasut
Quoted 8 lines
On second thoughts, with sha1-sse3-(CONFIG_AS_AVX2) += sha1_avx2_x86_64_asm.o, I have build issues and sha1_transform_avx2 undefined in sha1-sss3.ko.
I can rid #ifdef CONFIG_AS_AVX2 in patch1. The following works though:
ifeq ($(avx2_supported),yes)
sha1-ssse3-y += sha1_avx2_x86_64_asm.o
endif

Quoted 6 lines

Looking throughout the arch/x86/crypto/Makefile , this sha1-ssse3.o thing is a bit odd I think. Why exactly does this not follow suit with the camellia or serpent ciphers ? I mean, look at their build rules, they handle all of SSE2/AVX/AVX2 implementation and it's build. Can we not clean up the SHA1-SSSE3 to do exactly the same ? But please note I might be just plain wrong and if that's the case, let me know ;-)

I appended AVX2 support to the existing sha1_ssse3_glue. I will see if it can be cleaned up.

Quoted 5 lines

btw. I noticed another nit in the code. You use __sha1_transform_avx2() , but there previous function using AVX1 is called sha1_transform_avx() . Drop those two underscores please for consistency's sake.

__sha1_transform_avx2() is merely an internal inline function. The code patch picks sha1_transform_avx() or sha1_transform_avx2() depending on the datablock size (based on the results from running tcrypt).

Quoted 4 lines

Thanks!
Best regards,
Marek Vasut

thanks
- mouli