| Message ID | 1395919740-20774-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 27, 2014 at 12:29:00PM +0100, Ard Biesheuvel wrote: > The GHASH setkey() function uses SSE registers but fails to call > kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and > then having to deal with the restriction that they cannot be called from > interrupt context, move the setkey() implementation to the C domain. Note that setkey cannot be called from interrupt context since allocation/setkey is supposed to be slow-path material. But your approach is fine by me. > Note that setkey() does not use any particular SSE features and is not > expected to become a performance bottleneck. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > I suppose this should be marked for stable as well? Sure I'll add the cc. Thanks,
On 27 March 2014 12:36, Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Thu, Mar 27, 2014 at 12:29:00PM +0100, Ard Biesheuvel wrote: >> The GHASH setkey() function uses SSE registers but fails to call >> kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and >> then having to deal with the restriction that they cannot be called from >> interrupt context, move the setkey() implementation to the C domain. > > Note that setkey cannot be called from interrupt context since > allocation/setkey is supposed to be slow-path material. > > But your approach is fine by me. > I agree that it makes little sense to call this from atomic context, but that still means (I think, but the x86 guys should confirm) that you are supposed to call kernel_fpu_begin() and kernel_fpu_end(). >> Note that setkey() does not use any particular SSE features and is not >> expected to become a performance bottleneck. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> >> I suppose this should be marked for stable as well? > > Sure I'll add the cc. > Perhaps wait for an ack from team x86?
On 27 March 2014 12:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 27 March 2014 12:36, Herbert Xu <herbert@gondor.apana.org.au> wrote: >> On Thu, Mar 27, 2014 at 12:29:00PM +0100, Ard Biesheuvel wrote: >>> The GHASH setkey() function uses SSE registers but fails to call >>> kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and >>> then having to deal with the restriction that they cannot be called from >>> interrupt context, move the setkey() implementation to the C domain. >> >> Note that setkey cannot be called from interrupt context since >> allocation/setkey is supposed to be slow-path material. >> >> But your approach is fine by me. >> > > I agree that it makes little sense to call this from atomic context, > but that still means (I think, but the x86 guys should confirm) that > you are supposed to call kernel_fpu_begin() and kernel_fpu_end(). > >>> Note that setkey() does not use any particular SSE features and is not >>> expected to become a performance bottleneck. >>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> >>> I suppose this should be marked for stable as well? >> >> Sure I'll add the cc. >> > > Perhaps wait for an ack from team x86? > ... oh, and I noticed that I forgot to remove the forward declaration of clmul_ghash_setkey() as well. Sorry.
On 03/27/2014 04:46 AM, Ard Biesheuvel wrote: > On 27 March 2014 12:36, Herbert Xu <herbert@gondor.apana.org.au> wrote: >> On Thu, Mar 27, 2014 at 12:29:00PM +0100, Ard Biesheuvel wrote: >>> The GHASH setkey() function uses SSE registers but fails to call >>> kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and >>> then having to deal with the restriction that they cannot be called from >>> interrupt context, move the setkey() implementation to the C domain. >> >> Note that setkey cannot be called from interrupt context since >> allocation/setkey is supposed to be slow-path material. >> >> But your approach is fine by me. > > I agree that it makes little sense to call this from atomic context, > but that still means (I think, but the x86 guys should confirm) that > you are supposed to call kernel_fpu_begin() and kernel_fpu_end(). > Yes. I'm suspecting calling kernel_fpu_begin() for a single GF operation is probably not worth it, so I'm fine with reimplementing it in integer logic. Acked-by: H. Peter Anvin <hpa@linux.intel.com> -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/crypto/ghash-clmulni-intel_asm.S b/arch/x86/crypto/ghash-clmulni-intel_asm.S index 586f41aac361..185fad49d86f 100644 --- a/arch/x86/crypto/ghash-clmulni-intel_asm.S +++ b/arch/x86/crypto/ghash-clmulni-intel_asm.S @@ -24,10 +24,6 @@ .align 16 .Lbswap_mask: .octa 0x000102030405060708090a0b0c0d0e0f -.Lpoly: - .octa 0xc2000000000000000000000000000001 -.Ltwo_one: - .octa 0x00000001000000000000000000000001 #define DATA %xmm0 #define SHASH %xmm1 @@ -134,28 +130,3 @@ ENTRY(clmul_ghash_update) .Lupdate_just_ret: ret ENDPROC(clmul_ghash_update) - -/* - * void clmul_ghash_setkey(be128 *shash, const u8 *key); - * - * Calculate hash_key << 1 mod poly - */ -ENTRY(clmul_ghash_setkey) - movaps .Lbswap_mask, BSWAP - movups (%rsi), %xmm0 - PSHUFB_XMM BSWAP %xmm0 - movaps %xmm0, %xmm1 - psllq $1, %xmm0 - psrlq $63, %xmm1 - movaps %xmm1, %xmm2 - pslldq $8, %xmm1 - psrldq $8, %xmm2 - por %xmm1, %xmm0 - # reduction - pshufd $0b00100100, %xmm2, %xmm1 - pcmpeqd .Ltwo_one, %xmm1 - pand .Lpoly, %xmm1 - pxor %xmm1, %xmm0 - movups %xmm0, (%rdi) - ret -ENDPROC(clmul_ghash_setkey) diff --git a/arch/x86/crypto/ghash-clmulni-intel_glue.c b/arch/x86/crypto/ghash-clmulni-intel_glue.c index 6759dd1135be..3945d8095e80 100644 --- a/arch/x86/crypto/ghash-clmulni-intel_glue.c +++ b/arch/x86/crypto/ghash-clmulni-intel_glue.c @@ -58,13 +58,23 @@ static int ghash_setkey(struct crypto_shash *tfm, const u8 *key, unsigned int keylen) { struct ghash_ctx *ctx = crypto_shash_ctx(tfm); + be128 *x = (be128 *)key; + u64 a, b; if (keylen != GHASH_BLOCK_SIZE) { crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN); return -EINVAL; } - clmul_ghash_setkey(&ctx->shash, key); + /* perform multiplication by 'x' in GF(2^128) */ + a = be64_to_cpu(x->a); + b = be64_to_cpu(x->b); + + ctx->shash.a = (__be64)((b << 1) ^ (a >> 63)); + ctx->shash.b = (__be64)((a << 1) | (b >> 63)); + + if (a >> 63) + ctx->shash.b ^= cpu_to_be64(0xc2); return 0; }
The GHASH setkey() function uses SSE registers but fails to call kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and then having to deal with the restriction that they cannot be called from interrupt context, move the setkey() implementation to the C domain. Note that setkey() does not use any particular SSE features and is not expected to become a performance bottleneck. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- I suppose this should be marked for stable as well? arch/x86/crypto/ghash-clmulni-intel_asm.S | 29 ----------------------------- arch/x86/crypto/ghash-clmulni-intel_glue.c | 12 +++++++++++- 2 files changed, 11 insertions(+), 30 deletions(-)