From: Eric Biggers <[email protected]>
The x86 SHA-256 module contains four implementations: SSSE3, AVX, AVX2,
and SHA-NI. Commit 1c43c0f1f84a ("crypto: x86/sha - load modules based
on CPU features") made the module be autoloaded when SSSE3, AVX, or AVX2
is detected. The omission of SHA-NI appears to be an oversight, perhaps
because of the outdated file-level comment. This patch fixes this,
though in practice this makes no difference because SSSE3 is a subset of
the other three features anyway. Indeed, sha256_ni_transform() executes
SSSE3 instructions such as pshufb.
Cc: Roxana Nicolescu <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
arch/x86/crypto/sha256_ssse3_glue.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/sha256_ssse3_glue.c b/arch/x86/crypto/sha256_ssse3_glue.c
index 4c0383a90e11..a135cf9baca3 100644
--- a/arch/x86/crypto/sha256_ssse3_glue.c
+++ b/arch/x86/crypto/sha256_ssse3_glue.c
@@ -1,15 +1,15 @@
/*
* Cryptographic API.
*
- * Glue code for the SHA256 Secure Hash Algorithm assembler
- * implementation using supplemental SSE3 / AVX / AVX2 instructions.
+ * Glue code for the SHA256 Secure Hash Algorithm assembler implementations
+ * using SSSE3, AVX, AVX2, and SHA-NI instructions.
*
* This file is based on sha256_generic.c
*
* Copyright (C) 2013 Intel Corporation.
*
* Author:
* Tim Chen <[email protected]>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
@@ -38,20 +38,21 @@
#include <crypto/sha2.h>
#include <crypto/sha256_base.h>
#include <linux/string.h>
#include <asm/cpu_device_id.h>
#include <asm/simd.h>
asmlinkage void sha256_transform_ssse3(struct sha256_state *state,
const u8 *data, int blocks);
static const struct x86_cpu_id module_cpu_ids[] = {
+ X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
X86_MATCH_FEATURE(X86_FEATURE_AVX2, NULL),
X86_MATCH_FEATURE(X86_FEATURE_AVX, NULL),
X86_MATCH_FEATURE(X86_FEATURE_SSSE3, NULL),
{}
};
MODULE_DEVICE_TABLE(x86cpu, module_cpu_ids);
static int _sha256_update(struct shash_desc *desc, const u8 *data,
unsigned int len, sha256_block_fn *sha256_xform)
{
base-commit: f2b88bab69c86d4dab2bfd25a0e741d7df411f7a
--
2.42.0
On 29/10/2023 06:15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> The x86 SHA-256 module contains four implementations: SSSE3, AVX, AVX2,
> and SHA-NI. Commit 1c43c0f1f84a ("crypto: x86/sha - load modules based
> on CPU features") made the module be autoloaded when SSSE3, AVX, or AVX2
> is detected. The omission of SHA-NI appears to be an oversight, perhaps
> because of the outdated file-level comment. This patch fixes this,
> though in practice this makes no difference because SSSE3 is a subset of
> the other three features anyway. Indeed, sha256_ni_transform() executes
> SSSE3 instructions such as pshufb.
>
> Cc: Roxana Nicolescu <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
Indeed, it was an oversight.
Reviewed-by: Roxana Nicolescu <[email protected]>
> ---
> arch/x86/crypto/sha256_ssse3_glue.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/crypto/sha256_ssse3_glue.c b/arch/x86/crypto/sha256_ssse3_glue.c
> index 4c0383a90e11..a135cf9baca3 100644
> --- a/arch/x86/crypto/sha256_ssse3_glue.c
> +++ b/arch/x86/crypto/sha256_ssse3_glue.c
> @@ -1,15 +1,15 @@
> /*
> * Cryptographic API.
> *
> - * Glue code for the SHA256 Secure Hash Algorithm assembler
> - * implementation using supplemental SSE3 / AVX / AVX2 instructions.
> + * Glue code for the SHA256 Secure Hash Algorithm assembler implementations
> + * using SSSE3, AVX, AVX2, and SHA-NI instructions.
> *
> * This file is based on sha256_generic.c
> *
> * Copyright (C) 2013 Intel Corporation.
> *
> * Author:
> * Tim Chen <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License as published by the Free
> @@ -38,20 +38,21 @@
> #include <crypto/sha2.h>
> #include <crypto/sha256_base.h>
> #include <linux/string.h>
> #include <asm/cpu_device_id.h>
> #include <asm/simd.h>
>
> asmlinkage void sha256_transform_ssse3(struct sha256_state *state,
> const u8 *data, int blocks);
>
> static const struct x86_cpu_id module_cpu_ids[] = {
> + X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
> X86_MATCH_FEATURE(X86_FEATURE_AVX2, NULL),
> X86_MATCH_FEATURE(X86_FEATURE_AVX, NULL),
> X86_MATCH_FEATURE(X86_FEATURE_SSSE3, NULL),
> {}
> };
> MODULE_DEVICE_TABLE(x86cpu, module_cpu_ids);
>
> static int _sha256_update(struct shash_desc *desc, const u8 *data,
> unsigned int len, sha256_block_fn *sha256_xform)
> {
>
> base-commit: f2b88bab69c86d4dab2bfd25a0e741d7df411f7a
> -----Original Message-----
> From: Roxana Nicolescu <[email protected]>
> Sent: Tuesday, October 31, 2023 8:19 AM
> To: Eric Biggers <[email protected]>; [email protected]
> Subject: Re: [PATCH] crypto: x86/sha256 - autoload if SHA-NI detected
>
> On 29/10/2023 06:15, Eric Biggers wrote:
> > From: Eric Biggers <[email protected]>
> >
> > The x86 SHA-256 module contains four implementations: SSSE3, AVX, AVX2,
> > and SHA-NI. Commit 1c43c0f1f84a ("crypto: x86/sha - load modules based
> > on CPU features") made the module be autoloaded when SSSE3, AVX, or AVX2
> > is detected. The omission of SHA-NI appears to be an oversight, perhaps
> > because of the outdated file-level comment. This patch fixes this,
> > though in practice this makes no difference because SSSE3 is a subset of
> > the other three features anyway. Indeed, sha256_ni_transform() executes
> > SSSE3 instructions such as pshufb.
> >
> > Cc: Roxana Nicolescu <[email protected]>
> > Signed-off-by: Eric Biggers <[email protected]>
>
> Indeed, it was an oversight.
>
> Reviewed-by: Roxana Nicolescu <[email protected]>
> > ---
> > arch/x86/crypto/sha256_ssse3_glue.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/crypto/sha256_ssse3_glue.c
> b/arch/x86/crypto/sha256_ssse3_glue.c
> > index 4c0383a90e11..a135cf9baca3 100644
> > --- a/arch/x86/crypto/sha256_ssse3_glue.c
...
> >
> > static const struct x86_cpu_id module_cpu_ids[] = {
> > + X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
Unless something else has changed, this needs to be inside ifdefs, as discovered
in the proposed patch series last year:
for sha1_sse3_glue.c:
#ifdef CONFIG_AS_SHA1_NI
X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
#endif
for sha256_sse3_glue.c:
+#ifdef CONFIG_AS_SHA256_NI
+ X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
+#endif
> > X86_MATCH_FEATURE(X86_FEATURE_AVX2, NULL),
> > X86_MATCH_FEATURE(X86_FEATURE_AVX, NULL),
> > X86_MATCH_FEATURE(X86_FEATURE_SSSE3, NULL),
> > {}
> > };
> > MODULE_DEVICE_TABLE(x86cpu, module_cpu_ids);
On Tue, Oct 31, 2023 at 02:52:53PM +0000, Elliott, Robert (Servers) wrote:
> > -----Original Message-----
> > From: Roxana Nicolescu <[email protected]>
> > Sent: Tuesday, October 31, 2023 8:19 AM
> > To: Eric Biggers <[email protected]>; [email protected]
> > Subject: Re: [PATCH] crypto: x86/sha256 - autoload if SHA-NI detected
> >
> > On 29/10/2023 06:15, Eric Biggers wrote:
> > > From: Eric Biggers <[email protected]>
> > >
> > > The x86 SHA-256 module contains four implementations: SSSE3, AVX, AVX2,
> > > and SHA-NI. Commit 1c43c0f1f84a ("crypto: x86/sha - load modules based
> > > on CPU features") made the module be autoloaded when SSSE3, AVX, or AVX2
> > > is detected. The omission of SHA-NI appears to be an oversight, perhaps
> > > because of the outdated file-level comment. This patch fixes this,
> > > though in practice this makes no difference because SSSE3 is a subset of
> > > the other three features anyway. Indeed, sha256_ni_transform() executes
> > > SSSE3 instructions such as pshufb.
> > >
> > > Cc: Roxana Nicolescu <[email protected]>
> > > Signed-off-by: Eric Biggers <[email protected]>
> >
> > Indeed, it was an oversight.
> >
> > Reviewed-by: Roxana Nicolescu <[email protected]>
> > > ---
> > > arch/x86/crypto/sha256_ssse3_glue.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/crypto/sha256_ssse3_glue.c
> > b/arch/x86/crypto/sha256_ssse3_glue.c
> > > index 4c0383a90e11..a135cf9baca3 100644
> > > --- a/arch/x86/crypto/sha256_ssse3_glue.c
> ...
> > >
> > > static const struct x86_cpu_id module_cpu_ids[] = {
> > > + X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
>
> Unless something else has changed, this needs to be inside ifdefs, as discovered
> in the proposed patch series last year:
>
> for sha1_sse3_glue.c:
> #ifdef CONFIG_AS_SHA1_NI
> X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
> #endif
>
> for sha256_sse3_glue.c:
> +#ifdef CONFIG_AS_SHA256_NI
> + X86_MATCH_FEATURE(X86_FEATURE_SHA_NI, NULL),
> +#endif
Right, thanks for pointing that out. It compiles either way, but we shouldn't
autoload on SHA-NI when the code using SHA-NI isn't being built. Sent out v2
with this fixed.
- Eric