* [PATCH v4 2/4] async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
From: Anup Patel @ 2017-02-14 6:51 UTC (permalink / raw)
To: Vinod Koul, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Jassi Brar
Cc: Dan Williams, Ray Jui, Scott Branden, Jon Mason, Rob Rice,
bcm-kernel-feedback-list, dmaengine, devicetree, linux-arm-kernel,
linux-kernel, linux-crypto, linux-raid, Anup Patel
In-Reply-To: <1487055112-5185-1-git-send-email-anup.patel@broadcom.com>
The DMA_PREP_FENCE is to be used when preparing Tx descriptor if output
of Tx descriptor is to be used by next/dependent Tx descriptor.
The DMA_PREP_FENSE will not be set correctly in do_async_gen_syndrome()
when calling dma->device_prep_dma_pq() under following conditions:
1. ASYNC_TX_FENCE not set in submit->flags
2. DMA_PREP_FENCE not set in dma_flags
3. src_cnt (= (disks - 2)) is greater than dma_maxpq(dma, dma_flags)
This patch fixes DMA_PREP_FENCE usage in do_async_gen_syndrome() taking
inspiration from do_async_xor() implementation.
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
crypto/async_tx/async_pq.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/crypto/async_tx/async_pq.c b/crypto/async_tx/async_pq.c
index f83de99..56bd612 100644
--- a/crypto/async_tx/async_pq.c
+++ b/crypto/async_tx/async_pq.c
@@ -62,9 +62,6 @@ do_async_gen_syndrome(struct dma_chan *chan,
dma_addr_t dma_dest[2];
int src_off = 0;
- if (submit->flags & ASYNC_TX_FENCE)
- dma_flags |= DMA_PREP_FENCE;
-
while (src_cnt > 0) {
submit->flags = flags_orig;
pq_src_cnt = min(src_cnt, dma_maxpq(dma, dma_flags));
@@ -83,6 +80,8 @@ do_async_gen_syndrome(struct dma_chan *chan,
if (cb_fn_orig)
dma_flags |= DMA_PREP_INTERRUPT;
}
+ if (submit->flags & ASYNC_TX_FENCE)
+ dma_flags |= DMA_PREP_FENCE;
/* Drivers force forward progress in case they can not provide
* a descriptor
--
2.7.4
^ permalink raw reply related
* [bug report] crypto: brcm - Add Broadcom SPU driver
From: Dan Carpenter @ 2017-02-14 7:55 UTC (permalink / raw)
To: rob.rice; +Cc: linux-crypto
Hello Rob Rice,
The patch 9d12ba86f818: "crypto: brcm - Add Broadcom SPU driver" from
Feb 3, 2017, leads to the following static checker warning:
drivers/crypto/bcm/cipher.c:2340 ahash_finup()
warn: 'tmpbuf' was already freed.
drivers/crypto/bcm/cipher.c
2316 /* Copy data from req scatterlist to tmp buffer */
2317 gfp = (req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
2318 CRYPTO_TFM_REQ_MAY_SLEEP)) ? GFP_KERNEL : GFP_ATOMIC;
2319 tmpbuf = kmalloc(req->nbytes, gfp);
2320 if (!tmpbuf) {
2321 ret = -ENOMEM;
2322 goto ahash_finup_exit;
2323 }
2324
2325 if (sg_copy_to_buffer(req->src, nents, tmpbuf, req->nbytes) !=
2326 req->nbytes) {
2327 ret = -EINVAL;
2328 goto ahash_finup_free;
2329 }
2330
2331 /* Call synchronous update */
2332 ret = crypto_shash_finup(ctx->shash, tmpbuf, req->nbytes,
2333 req->result);
2334 kfree(tmpbuf);
^^^^^^^^^^^^^
2335 } else {
2336 /* Otherwise call the internal function which uses SPU hw */
2337 return __ahash_finup(req);
2338 }
2339 ahash_finup_free:
2340 kfree(tmpbuf);
^^^^^^^^^^^^^
I'm only working a 30 minutes per day to keep a hand in. I'm not
sending patches this month.
2341
2342 ahash_finup_exit:
2343 /* Done with hash, can deallocate it now */
2344 crypto_free_shash(ctx->shash->tfm);
2345 kfree(ctx->shash);
2346 return ret;
2347 }
regards,
dan carpenter
^ permalink raw reply
* [bug report] crypto: cavium - Add Support for Octeon-tx CPT Engine
From: Dan Carpenter @ 2017-02-14 8:28 UTC (permalink / raw)
To: george.cherian; +Cc: linux-crypto
Hello George Cherian,
The patch 9e2c7d99941d: "crypto: cavium - Add Support for Octeon-tx
CPT Engine" from Feb 7, 2017, leads to the following static checker
warning:
drivers/crypto/cavium/cpt/cptpf_mbox.c:70 cpt_bind_vq_to_grp()
warn: signedness bug returning '(-22)'
drivers/crypto/cavium/cpt/cptpf_mbox.c
62 static u8 cpt_bind_vq_to_grp(struct cpt_device *cpt, u8 q, u8 grp)
^^
63 {
64 struct microcode *mcode = cpt->mcode;
65 union cptx_pf_qx_ctl pf_qx_ctl;
66 struct device *dev = &cpt->pdev->dev;
67
68 if (q >= CPT_MAX_VF_NUM) {
69 dev_err(dev, "Queues are more than cores in the group");
70 return -EINVAL;
^^^^^^^^^^^^^^^
71 }
72 if (grp >= CPT_MAX_CORE_GROUPS) {
73 dev_err(dev, "Request group is more than possible groups");
74 return -EINVAL;
^^^^^^^^^^^^^^^
75 }
76 if (grp >= cpt->next_mc_idx) {
77 dev_err(dev, "Request group is higher than available functional groups");
78 return -EINVAL;
^^^^^^^^^^^^^^^
79 }
80 pf_qx_ctl.u = cpt_read_csr64(cpt->reg_base, CPTX_PF_QX_CTL(0, q));
81 pf_qx_ctl.s.grp = mcode[grp].group;
82 cpt_write_csr64(cpt->reg_base, CPTX_PF_QX_CTL(0, q), pf_qx_ctl.u);
83 dev_dbg(dev, "VF %d TYPE %s", q, (mcode[grp].is_ae ? "AE" : "SE"));
84
85 return mcode[grp].is_ae ? AE_TYPES : SE_TYPES;
86 }
regards,
dan carpenter
^ permalink raw reply
* [PATCH] drivers: crypto: cpt: cpt_bind_vq_to_grp could return an error code
From: George Cherian @ 2017-02-14 9:23 UTC (permalink / raw)
To: herbert, davem, dan.carpenter; +Cc: linux-kernel, linux-crypto, George Cherian
cpt_bind_vq_to_grp() could return an error code. However, it currently
returns a u8. This produce the static checker warning.
drivers/crypto/cavium/cpt/cptpf_mbox.c:70 cpt_bind_vq_to_grp() warn: signedness bug returning '(-22)'
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
---
drivers/crypto/cavium/cpt/cptpf_mbox.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/cavium/cpt/cptpf_mbox.c b/drivers/crypto/cavium/cpt/cptpf_mbox.c
index 5818b41..20f2c6e 100644
--- a/drivers/crypto/cavium/cpt/cptpf_mbox.c
+++ b/drivers/crypto/cavium/cpt/cptpf_mbox.c
@@ -59,7 +59,7 @@ static void cpt_cfg_vq_priority(struct cpt_device *cpt, int vf, u32 pri)
cpt_write_csr64(cpt->reg_base, CPTX_PF_QX_CTL(0, vf), pf_qx_ctl.u);
}
-static u8 cpt_bind_vq_to_grp(struct cpt_device *cpt, u8 q, u8 grp)
+static int cpt_bind_vq_to_grp(struct cpt_device *cpt, u8 q, u8 grp)
{
struct microcode *mcode = cpt->mcode;
union cptx_pf_qx_ctl pf_qx_ctl;
@@ -90,7 +90,7 @@ static void cpt_handle_mbox_intr(struct cpt_device *cpt, int vf)
{
struct cpt_vf_info *vfx = &cpt->vfinfo[vf];
struct cpt_mbox mbx = {};
- u8 vftype;
+ int vftype;
struct device *dev = &cpt->pdev->dev;
/*
* MBOX[0] contains msg
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3] crypto: algapi - make crypto_xor() and crypto_inc() alignment agnostic
From: Ard Biesheuvel @ 2017-02-14 9:24 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: Linux Crypto Mailing List, Eric Biggers, Herbert Xu
In-Reply-To: <CAHmME9o9Rg2bcb1dibAyTUCSYGuzicSU__62P_G9G8ARJkgs=Q@mail.gmail.com>
On 13 February 2017 at 21:55, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Sun, Feb 5, 2017 at 11:06 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ||
>> + !((unsigned long)b & (__alignof__(*b) - 1)))
>
> Why not simply use the IS_ALIGNED macro?
>
Good point.
> Also, are you might consider checking to see if this is a constant, so
> that you can avoid an unnecessary branch.
Check if what is a constant? The buffer address?
> Alternatively, if you want
> to use the branch, I'd be interested in you writing back saying, "I
> tested both cases, and branching is faster than always using the slow
> unaligned path."
When using the 4-byte wide path, the loop terminates after 1 iteration
unless there is a carry in the low word. The likelihood of the loop
iterating multiple times in the 1-byte wide path is 1 in 256.
I suppose we could add this if there is concern about the branching
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 6b52e8f0b95f..03670390a2e4 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -967,7 +967,7 @@ void crypto_inc(u8 *a, unsigned int size)
for (; size >= 4; size -= 4) {
c = be32_to_cpu(*--b) + 1;
*b = cpu_to_be32(c);
- if (c)
+ if (likely(c))
return;
}
but other than that, I see little reason to introduce complicated logic here.
>> + while (((unsigned long)dst & (relalign - 1)) && len > 0) {
>
> IS_ALIGNED
>
>> +static inline void crypto_xor(u8 *dst, const u8 *src, unsigned int size)
>> +{
>> + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
>> + __builtin_constant_p(size) &&
>> + (size % sizeof(unsigned long)) == 0) {
>
> You can expand this condition to be:
>
> if ( (is_constant(size) && size%sizeof(ulong)==0) &&
> (efficient_unaligned || (is_constant(dst) && is_constant(src) &&
> is_aligned(dst) && is_aligned(src))) )
>
> It might seem complex, but it all gets compiled out.
Care to explain how? Could you point me to any references to
crypto_xor() where either of the buffer addresses are compile time
(not link time) constants?
^ permalink raw reply related
* [PATCH 1/2] crypto: arm/aes-neonbs - resolve fallback cipher at runtime
From: Ard Biesheuvel @ 2017-02-14 10:03 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
Currently, the bit sliced NEON AES code for ARM has a link time
dependency on the scalar ARM asm implementation, which it uses as a
fallback to perform CBC encryption and the encryption of the initial
XTS tweak.
The bit sliced NEON code is both fast and time invariant, which makes
it a reasonable default on hardware that supports it. However, the
ARM asm code it pulls in is not time invariant, and due to the way it
is linked in, cannot be overridden by the new generic time invariant
driver. In fact, it will not be used at all, given that the ARM asm
code registers itself as a cipher with a priority that exceeds the
priority of the fixed time cipher.
So remove the link time dependency, and allocate the fallback cipher
via the crypto API. Note that this requires this driver's module_init
call to be replaced with late_initcall, so that the (possibly generic)
fallback cipher is guaranteed to be available when the builtin test
is performed at registration time.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/crypto/Kconfig | 2 +-
arch/arm/crypto/aes-neonbs-glue.c | 65 ++++++++++++++++++++++++++++++---------
2 files changed, 51 insertions(+), 16 deletions(-)
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index a8fce93137fb..b9adedcc5b2e 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -73,7 +73,7 @@ config CRYPTO_AES_ARM_BS
depends on KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_SIMD
- select CRYPTO_AES_ARM
+ select CRYPTO_AES
help
Use a faster and more secure NEON based implementation of AES in CBC,
CTR and XTS modes
diff --git a/arch/arm/crypto/aes-neonbs-glue.c b/arch/arm/crypto/aes-neonbs-glue.c
index 2920b96dbd36..6a2a30b9e4f5 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -42,9 +42,6 @@ asmlinkage void aesbs_xts_encrypt(u8 out[], u8 const in[], u8 const rk[],
asmlinkage void aesbs_xts_decrypt(u8 out[], u8 const in[], u8 const rk[],
int rounds, int blocks, u8 iv[]);
-asmlinkage void __aes_arm_encrypt(const u32 rk[], int rounds, const u8 in[],
- u8 out[]);
-
struct aesbs_ctx {
int rounds;
u8 rk[13 * (8 * AES_BLOCK_SIZE) + 32] __aligned(AES_BLOCK_SIZE);
@@ -52,12 +49,12 @@ struct aesbs_ctx {
struct aesbs_cbc_ctx {
struct aesbs_ctx key;
- u32 enc[AES_MAX_KEYLENGTH_U32];
+ struct crypto_cipher *enc_tfm;
};
struct aesbs_xts_ctx {
struct aesbs_ctx key;
- u32 twkey[AES_MAX_KEYLENGTH_U32];
+ struct crypto_cipher *tweak_tfm;
};
static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
@@ -132,20 +129,18 @@ static int aesbs_cbc_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
ctx->key.rounds = 6 + key_len / 4;
- memcpy(ctx->enc, rk.key_enc, sizeof(ctx->enc));
-
kernel_neon_begin();
aesbs_convert_key(ctx->key.rk, rk.key_enc, ctx->key.rounds);
kernel_neon_end();
- return 0;
+ return crypto_cipher_setkey(ctx->enc_tfm, in_key, key_len);
}
static void cbc_encrypt_one(struct crypto_skcipher *tfm, const u8 *src, u8 *dst)
{
struct aesbs_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
- __aes_arm_encrypt(ctx->enc, ctx->key.rounds, src, dst);
+ crypto_cipher_encrypt_one(ctx->enc_tfm, dst, src);
}
static int cbc_encrypt(struct skcipher_request *req)
@@ -181,6 +176,23 @@ static int cbc_decrypt(struct skcipher_request *req)
return err;
}
+static int cbc_init(struct crypto_tfm *tfm)
+{
+ struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->enc_tfm = crypto_alloc_cipher("aes", 0, 0);
+ if (IS_ERR(ctx->enc_tfm))
+ return PTR_ERR(ctx->enc_tfm);
+ return 0;
+}
+
+static void cbc_exit(struct crypto_tfm *tfm)
+{
+ struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ crypto_free_cipher(ctx->enc_tfm);
+}
+
static int ctr_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
@@ -228,7 +240,6 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
unsigned int key_len)
{
struct aesbs_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
- struct crypto_aes_ctx rk;
int err;
err = xts_verify_key(tfm, in_key, key_len);
@@ -236,13 +247,33 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
return err;
key_len /= 2;
- err = crypto_aes_expand_key(&rk, in_key + key_len, key_len);
+ err = crypto_cipher_setkey(ctx->tweak_tfm, in_key + key_len, key_len);
if (err)
return err;
- memcpy(ctx->twkey, rk.key_enc, sizeof(ctx->twkey));
+ err = aesbs_setkey(tfm, in_key, key_len);
+ if (err)
+ return err;
+
+ return 0;
+
+}
+
+static int xts_init(struct crypto_tfm *tfm)
+{
+ struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->tweak_tfm = crypto_alloc_cipher("aes", 0, 0);
+ if (IS_ERR(ctx->tweak_tfm))
+ return PTR_ERR(ctx->tweak_tfm);
+ return 0;
+}
+
+static void xts_exit(struct crypto_tfm *tfm)
+{
+ struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
- return aesbs_setkey(tfm, in_key, key_len);
+ crypto_free_cipher(ctx->tweak_tfm);
}
static int __xts_crypt(struct skcipher_request *req,
@@ -256,7 +287,7 @@ static int __xts_crypt(struct skcipher_request *req,
err = skcipher_walk_virt(&walk, req, true);
- __aes_arm_encrypt(ctx->twkey, ctx->key.rounds, walk.iv, walk.iv);
+ crypto_cipher_encrypt_one(ctx->tweak_tfm, walk.iv, walk.iv);
kernel_neon_begin();
while (walk.nbytes >= AES_BLOCK_SIZE) {
@@ -309,6 +340,8 @@ static struct skcipher_alg aes_algs[] = { {
.base.cra_ctxsize = sizeof(struct aesbs_cbc_ctx),
.base.cra_module = THIS_MODULE,
.base.cra_flags = CRYPTO_ALG_INTERNAL,
+ .base.cra_init = cbc_init,
+ .base.cra_exit = cbc_exit,
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
@@ -342,6 +375,8 @@ static struct skcipher_alg aes_algs[] = { {
.base.cra_ctxsize = sizeof(struct aesbs_xts_ctx),
.base.cra_module = THIS_MODULE,
.base.cra_flags = CRYPTO_ALG_INTERNAL,
+ .base.cra_init = xts_init,
+ .base.cra_exit = xts_exit,
.min_keysize = 2 * AES_MIN_KEY_SIZE,
.max_keysize = 2 * AES_MAX_KEY_SIZE,
@@ -402,5 +437,5 @@ static int __init aes_init(void)
return err;
}
-module_init(aes_init);
+late_initcall(aes_init);
module_exit(aes_exit);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] crypto: algapi - annotate expected branch behavior in crypto_inc()
From: Ard Biesheuvel @ 2017-02-14 10:04 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel, Jason A . Donenfeld
In-Reply-To: <1487066640-17886-1-git-send-email-ard.biesheuvel@linaro.org>
To prevent unnecessary branching, mark the exit condition of the
primary loop as likely(), given that a carry in a 32-bit counter
occurs very rarely.
On arm64, the resulting code is emitted by GCC as
9a8: cmp w1, #0x3
9ac: add x3, x0, w1, uxtw
9b0: b.ls 9e0 <crypto_inc+0x38>
9b4: ldr w2, [x3,#-4]!
9b8: rev w2, w2
9bc: add w2, w2, #0x1
9c0: rev w4, w2
9c4: str w4, [x3]
9c8: cbz w2, 9d0 <crypto_inc+0x28>
9cc: ret
where the two remaining branch conditions (one for size < 4 and one for
the carry) are statically predicted as non-taken, resulting in optimal
execution in the vast majority of cases.
Also, replace the open coded alignment test with IS_ALIGNED().
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
crypto/algapi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 6b52e8f0b95f..9eed4ef9c971 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -963,11 +963,11 @@ void crypto_inc(u8 *a, unsigned int size)
u32 c;
if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ||
- !((unsigned long)b & (__alignof__(*b) - 1)))
+ IS_ALIGNED((unsigned long)b, __alignof__(*b)))
for (; size >= 4; size -= 4) {
c = be32_to_cpu(*--b) + 1;
*b = cpu_to_be32(c);
- if (c)
+ if (likely(c))
return;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] lz4: fix performance regressions
From: Sven Schmidt @ 2017-02-14 10:33 UTC (permalink / raw)
To: Eric Biggers
Cc: minchan, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
gregkh, linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170212233802.GA29621@zzz>
Hey Eric,
On Sun, Feb 12, 2017 at 03:38:02PM -0800, Eric Biggers wrote:
> Hi Sven,
>
> On Sun, Feb 12, 2017 at 12:16:18PM +0100, Sven Schmidt wrote:
> > /*-************************************
> > * Reading and writing into memory
> > **************************************/
> > +typedef union {
> > + U16 u16;
> > + U32 u32;
> > + size_t uArch;
> > +} __packed unalign;
> >
> > -static inline U16 LZ4_read16(const void *memPtr)
> > +static FORCE_INLINE __maybe_unused U16 LZ4_read16(const void *ptr)
> > {
> > - U16 val;
> > -
> > - memcpy(&val, memPtr, sizeof(val));
> > -
> > - return val;
> > + return ((const unalign *)ptr)->u16;
> > }
> >
> > -static inline U32 LZ4_read32(const void *memPtr)
> > +static FORCE_INLINE __maybe_unused U32 LZ4_read32(const void *ptr)
> > {
> > - U32 val;
> > -
> > - memcpy(&val, memPtr, sizeof(val));
> > -
> > - return val;
> > + return ((const unalign *)ptr)->u32;
> > }
> >
> > -static inline size_t LZ4_read_ARCH(const void *memPtr)
> > +static FORCE_INLINE __maybe_unused size_t LZ4_read_ARCH(const void *ptr)
> > {
> > - size_t val;
> > -
> > - memcpy(&val, memPtr, sizeof(val));
> > -
> > - return val;
> > + return ((const unalign *)ptr)->uArch;
> > }
> >
> > -static inline void LZ4_write16(void *memPtr, U16 value)
> > +static FORCE_INLINE __maybe_unused void LZ4_write16(void *memPtr, U16 value)
> > {
> > - memcpy(memPtr, &value, sizeof(value));
> > + ((unalign *)memPtr)->u16 = value;
> > }
> >
> > -static inline void LZ4_write32(void *memPtr, U32 value)
> > -{
> > - memcpy(memPtr, &value, sizeof(value));
> > +static FORCE_INLINE __maybe_unused void LZ4_write32(void *memPtr, U32 value) {
> > + ((unalign *)memPtr)->u32 = value;
> > }
> >
> > -static inline U16 LZ4_readLE16(const void *memPtr)
> > +static FORCE_INLINE __maybe_unused U16 LZ4_readLE16(const void *memPtr)
> > {
> > -#ifdef __LITTLE_ENDIAN__
> > +#if LZ4_LITTLE_ENDIAN
> > return LZ4_read16(memPtr);
> > #else
> > const BYTE *p = (const BYTE *)memPtr;
> > @@ -137,19 +143,19 @@ static inline U16 LZ4_readLE16(const void *memPtr)
> > #endif
> > }
>
> Since upstream LZ4 is intended to be compiled at -O3, this may allow it to get
> away with using memcpy() for unaligned memory accesses. The reason it uses
> memcpy() is that, other than a byte-by-byte copy, it is the only portable way to
> express unaligned memory accesses. But the Linux kernel is sometimes compiled
> optimized for size (-Os), and I wouldn't be *too* surprised if some of the
> memcpy()'s don't always get inlined then, which could be causing the performance
> regression being observed. (Of course, this could be verified by checking
> whether CONFIG_CC_OPTIMIZE_FOR_SIZE=y is set, then reading the assembly.)
>
> But I don't think accessing a __packed structure directly is the right
> alternative. Instead, Linux already includes macros for unaligned memory
> accesses which have been optimized for every supported architecture. Those
> should just be used instead, e.g. like this:
>
> static FORCE_INLINE U16 LZ4_read16(const void *ptr)
> {
> return get_unaligned((const u16 *)ptr);
> }
>
> static FORCE_INLINE U32 LZ4_read32(const void *ptr)
> {
> return get_unaligned((const u32 *)ptr);
> }
>
> static FORCE_INLINE size_t LZ4_read_ARCH(const void *ptr)
> {
> return get_unaligned((const size_t *)ptr);
> }
>
> static FORCE_INLINE void LZ4_write16(void *memPtr, U16 value)
> {
> put_unaligned(value, (u16 *)memPtr);
> }
>
> static FORCE_INLINE void LZ4_write32(void *memPtr, U32 value)
> {
> put_unaligned(value, (u32 *)memPtr);
> }
>
> static FORCE_INLINE U16 LZ4_readLE16(const void *memPtr)
> {
> return get_unaligned_le16(memPtr);
> }
>
> static FORCE_INLINE void LZ4_writeLE16(void *memPtr, U16 value)
> {
> return put_unaligned_le16(value, memPtr);
> }
>
> static FORCE_INLINE void LZ4_copy8(void *dst, const void *src)
> {
> if (LZ4_64bits()) {
> u64 a = get_unaligned((const u64 *)src);
> put_unaligned(a, (u64 *)dst);
> } else {
> u32 a = get_unaligned((const u32 *)src);
> u32 b = get_unaligned((const u32 *)src + 1);
> put_unaligned(a, (u32 *)dst);
> put_unaligned(b, (u32 *)dst + 1);
> }
> }
>
>
> Note that I dropped __maybe_unused as it's not needed on inline functions.
> That should be done everywhere else the patch proposes to add it too.
>
> > -#if LZ4_ARCH64
> > -#ifdef __BIG_ENDIAN__
> > -#define LZ4_NBCOMMONBYTES(val) (__builtin_clzll(val) >> 3)
> > +static FORCE_INLINE unsigned int LZ4_NbCommonBytes(register size_t val)
> > +{
> > +#if LZ4_LITTLE_ENDIAN
> > +#if LZ4_ARCH64 /* 64 Bits Little Endian */
> > +#if defined(LZ4_FORCE_SW_BITCOUNT)
> > + static const int DeBruijnBytePos[64] = {
> > + 0, 0, 0, 0, 0, 1, 1, 2, 0, 3, 1, 3, 1, 4, 2, 7,
> > + 0, 2, 3, 6, 1, 5, 3, 5, 1, 3, 4, 4, 2, 5, 6, 7,
> > + 7, 0, 1, 2, 3, 3, 4, 6, 2, 6, 5, 5, 3, 4, 5, 6,
> > + 7, 1, 2, 4, 6, 4, 4, 5, 7, 2, 6, 5, 7, 6, 7, 7
> > + };
> > +
> > + return DeBruijnBytePos[((U64)((val & -(long long)val)
> > + * 0x0218A392CDABBD3FULL)) >> 58];
> > #else
> > -#define LZ4_NBCOMMONBYTES(val) (__builtin_ctzll(val) >> 3)
> > -#endif
> > + return (__builtin_ctzll((U64)val) >> 3);
> > +#endif /* defined(LZ4_FORCE_SW_BITCOUNT) */
> > +#else /* 32 Bits Little Endian */
> > +#if defined(LZ4_FORCE_SW_BITCOUNT)
> > + static const int DeBruijnBytePos[32] = {
> > + 0, 0, 3, 0, 3, 1, 3, 0, 3, 2, 2, 1, 3, 2, 0, 1,
> > + 3, 3, 1, 2, 2, 2, 2, 0, 3, 1, 2, 0, 1, 0, 1, 1
> > + };
> > +
> > + return DeBruijnBytePos[((U32)((val & -(S32)val)
> > + * 0x077CB531U)) >> 27];
> > #else
> > -#ifdef __BIG_ENDIAN__
> > -#define LZ4_NBCOMMONBYTES(val) (__builtin_clz(val) >> 3)
> > + return (__builtin_ctz((U32)val) >> 3);
> > +#endif /* defined(LZ4_FORCE_SW_BITCOUNT) */
> > +#endif /* LZ4_ARCH64 */
> > +#else /* Big Endian */
> > +#if LZ4_ARCH64 /* 64 Bits Big Endian */
> > +#if defined(LZ4_FORCE_SW_BITCOUNT)
> > + unsigned int r;
> > +
> > + if (!(val >> 32)) {
> > + r = 4;
> > + } else {
> > + r = 0;
> > + val >>= 32;
> > + }
> > +
> > + if (!(val >> 16)) {
> > + r += 2;
> > + val >>= 8;
> > + } else {
> > + val >>= 24;
> > + }
> > +
> > + r += (!val);
> > +
> > + return r;
> > #else
> > -#define LZ4_NBCOMMONBYTES(val) (__builtin_ctz(val) >> 3)
> > -#endif
> > -#endif
> > + return (__builtin_clzll((U64)val) >> 3);
> > +#endif /* defined(LZ4_FORCE_SW_BITCOUNT) */
> > +#else /* 32 Bits Big Endian */
> > +#if defined(LZ4_FORCE_SW_BITCOUNT)
> > + unsigned int r;
> > +
> > + if (!(val >> 16)) {
> > + r = 2;
> > + val >>= 8;
> > + } else {
> > + r = 0;
> > + val >>= 24;
> > + }
> > +
> > + r += (!val);
> > +
> > + return r;
> > +#else
> > + return (__builtin_clz((U32)val) >> 3);
> > +#endif /* defined(LZ4_FORCE_SW_BITCOUNT) */
> > +#endif /* LZ4_ARCH64 */
> > +#endif /* LZ4_LITTLE_ENDIAN */
> > +}
>
> The reason LZ4_NbCommonBytes() in upstream LZ4 is so complicated is that it
> needs to provide portable fallbacks that work on *any* platform and compiler.
> This isn't needed in the Linux kernel, and it should just call the functions
> already defined that do the right thing:
>
> static FORCE_INLINE unsigned int LZ4_NbCommonBytes(register size_t val)
> {
> if (LZ4_isLittleEndian())
> return __ffs(val) >> 3;
> else
> return (BITS_PER_LONG - 1 - __fls(val)) >> 3;
> }
>
> To be clear, when I said that upstream LZ4 shouldn't generally be changed, I'm
> primarily talking about the core code, not the platform-specific parts. What we
> need to do is define platform-specific stuff, like LZ4_read*(), LZ4_write*(),
> LZ4_NbCommonBytes(), LZ4_64bits(), and FORCE_INLINE, in a way that makes sense
> for the Linux kernel and the environment it's compiled in. Also I think it's
> fine, and maybe even necessary for performance, to *add* inline or FORCE_INLINE
> in some places too, given that LZ4 in Linux may get compiled with a lower
> optimization level than that intended to be used for upstream LZ4 --- though it
> may be worth considering updating the Makefile to just always compile the LZ4
> files with -O3 instead. What should be avoided is making unnecessary changes to
> the *users* of the platform-specific code or to the core (de)compression
> parameters or templates.
>
> Eric
I'm very thankful for your effort here! I included all your proposed changes (as well as -O3 as compiler flag in the Makefile)
and noticed further improvements concerning the performance in ZRAM (compared to current LZ4 in 4.10 RC8 as well as the previous changes discussed
here). I would never have thought that the NBcommonBytes function can be re-written in such an easy way, since the current LZ4 in the kernel
uses similar approach checking for 64/32 bit and endianess.
Also, thanks for your very detailed remarks. That's really helpful to me.
Regards,
Sven
^ permalink raw reply
* [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp framework support
From: Mahipal Challa @ 2017-02-14 15:40 UTC (permalink / raw)
To: herbert, sjenning, davem
Cc: linux-crypto, linux-kernel, linux-mm, pathreya, vnair,
Mahipal Challa, Vishnu Nair
In-Reply-To: <1487086821-5880-1-git-send-email-Mahipal.Challa@cavium.com>
This adds the support for kernel's crypto new acomp/scomp framework
to zswap.
Signed-off-by: Mahipal Challa <Mahipal.Challa@cavium.com>
Signed-off-by: Vishnu Nair <Vishnu.Nair@cavium.com>
---
mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 99 insertions(+), 30 deletions(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 067a0d6..d08631b 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -33,6 +33,8 @@
#include <linux/rbtree.h>
#include <linux/swap.h>
#include <linux/crypto.h>
+#include <crypto/acompress.h>
+#include <linux/scatterlist.h>
#include <linux/mempool.h>
#include <linux/zpool.h>
@@ -114,7 +116,8 @@ static int zswap_compressor_param_set(const char *,
struct zswap_pool {
struct zpool *zpool;
- struct crypto_comp * __percpu *tfm;
+ struct crypto_acomp * __percpu *acomp;
+ struct acomp_req * __percpu *acomp_req;
struct kref kref;
struct list_head list;
struct work_struct work;
@@ -379,30 +382,49 @@ static int zswap_dstmem_dead(unsigned int cpu)
static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
{
struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
- struct crypto_comp *tfm;
+ struct crypto_acomp *acomp;
+ struct acomp_req *acomp_req;
- if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu)))
+ if (WARN_ON(*per_cpu_ptr(pool->acomp, cpu)))
return 0;
+ if (WARN_ON(*per_cpu_ptr(pool->acomp_req, cpu)))
+ return 0;
+
+ acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0);
+ if (IS_ERR_OR_NULL(acomp)) {
+ pr_err("could not alloc crypto acomp %s : %ld\n",
+ pool->tfm_name, PTR_ERR(acomp));
+ return -ENOMEM;
+ }
+ *per_cpu_ptr(pool->acomp, cpu) = acomp;
- tfm = crypto_alloc_comp(pool->tfm_name, 0, 0);
- if (IS_ERR_OR_NULL(tfm)) {
- pr_err("could not alloc crypto comp %s : %ld\n",
- pool->tfm_name, PTR_ERR(tfm));
+ acomp_req = acomp_request_alloc(acomp);
+ if (IS_ERR_OR_NULL(acomp_req)) {
+ pr_err("could not alloc crypto acomp %s : %ld\n",
+ pool->tfm_name, PTR_ERR(acomp));
return -ENOMEM;
}
- *per_cpu_ptr(pool->tfm, cpu) = tfm;
+ *per_cpu_ptr(pool->acomp_req, cpu) = acomp_req;
+
return 0;
}
static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
{
struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
- struct crypto_comp *tfm;
+ struct crypto_acomp *acomp;
+ struct acomp_req *acomp_req;
+
+ acomp_req = *per_cpu_ptr(pool->acomp_req, cpu);
+ if (!IS_ERR_OR_NULL(acomp_req))
+ acomp_request_free(acomp_req);
+ *per_cpu_ptr(pool->acomp_req, cpu) = NULL;
+
+ acomp = *per_cpu_ptr(pool->acomp, cpu);
+ if (!IS_ERR_OR_NULL(acomp))
+ crypto_free_acomp(acomp);
+ *per_cpu_ptr(pool->acomp, cpu) = NULL;
- tfm = *per_cpu_ptr(pool->tfm, cpu);
- if (!IS_ERR_OR_NULL(tfm))
- crypto_free_comp(tfm);
- *per_cpu_ptr(pool->tfm, cpu) = NULL;
return 0;
}
@@ -503,8 +525,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
strlcpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
- pool->tfm = alloc_percpu(struct crypto_comp *);
- if (!pool->tfm) {
+ pool->acomp = alloc_percpu(struct crypto_acomp *);
+ if (!pool->acomp) {
+ pr_err("percpu alloc failed\n");
+ goto error;
+ }
+
+ pool->acomp_req = alloc_percpu(struct acomp_req *);
+ if (!pool->acomp_req) {
pr_err("percpu alloc failed\n");
goto error;
}
@@ -526,7 +554,8 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
return pool;
error:
- free_percpu(pool->tfm);
+ free_percpu(pool->acomp_req);
+ free_percpu(pool->acomp);
if (pool->zpool)
zpool_destroy_pool(pool->zpool);
kfree(pool);
@@ -566,7 +595,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool)
zswap_pool_debug("destroying", pool);
cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
- free_percpu(pool->tfm);
+ free_percpu(pool->acomp_req);
+ free_percpu(pool->acomp);
zpool_destroy_pool(pool->zpool);
kfree(pool);
}
@@ -763,7 +793,8 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
pgoff_t offset;
struct zswap_entry *entry;
struct page *page;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
u8 *src, *dst;
unsigned int dlen;
int ret;
@@ -803,14 +834,23 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
case ZSWAP_SWAPCACHE_NEW: /* page is locked */
/* decompress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
dlen = PAGE_SIZE;
src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
ZPOOL_MM_RO) + sizeof(struct zswap_header);
dst = kmap_atomic(page);
- tfm = *get_cpu_ptr(entry->pool->tfm);
- ret = crypto_comp_decompress(tfm, src, entry->length,
- dst, &dlen);
- put_cpu_ptr(entry->pool->tfm);
+
+ sg_init_one(&input, src, entry->length);
+ sg_init_one(&output, dst, dlen);
+ acomp_request_set_params(req, &input, &output, entry->length,
+ dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+ NULL, NULL);
+
+ ret = crypto_acomp_decompress(req);
+
+ dlen = req->dlen;
+ put_cpu_ptr(entry->pool->acomp_req);
kunmap_atomic(dst);
zpool_unmap_handle(entry->pool->zpool, entry->handle);
BUG_ON(ret);
@@ -886,7 +926,8 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
{
struct zswap_tree *tree = zswap_trees[type];
struct zswap_entry *entry, *dupentry;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
int ret;
unsigned int dlen = PAGE_SIZE, len;
unsigned long handle;
@@ -925,12 +966,27 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
}
/* compress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
+ if (!req) {
+ put_cpu_ptr(entry->pool->acomp_req);
+ ret = -EINVAL;
+ goto freepage;
+ }
+
dst = get_cpu_var(zswap_dstmem);
- tfm = *get_cpu_ptr(entry->pool->tfm);
src = kmap_atomic(page);
- ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
+
+ sg_init_one(&input, src, PAGE_SIZE);
+ /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
+ sg_init_one(&output, dst, PAGE_SIZE * 2);
+ acomp_request_set_params(req, &input, &output, PAGE_SIZE, dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
+ NULL);
+
+ ret = crypto_acomp_compress(req);
kunmap_atomic(src);
- put_cpu_ptr(entry->pool->tfm);
+ put_cpu_ptr(entry->pool->acomp_req);
+ dlen = req->dlen;
if (ret) {
ret = -EINVAL;
goto put_dstmem;
@@ -998,7 +1054,8 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
{
struct zswap_tree *tree = zswap_trees[type];
struct zswap_entry *entry;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
u8 *src, *dst;
unsigned int dlen;
int ret;
@@ -1014,13 +1071,25 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
spin_unlock(&tree->lock);
/* decompress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
+ if (!req) {
+ put_cpu_ptr(entry->pool->acomp_req);
+ return -1;
+ }
dlen = PAGE_SIZE;
src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
ZPOOL_MM_RO) + sizeof(struct zswap_header);
dst = kmap_atomic(page);
- tfm = *get_cpu_ptr(entry->pool->tfm);
- ret = crypto_comp_decompress(tfm, src, entry->length, dst, &dlen);
- put_cpu_ptr(entry->pool->tfm);
+
+ sg_init_one(&input, src, entry->length);
+ sg_init_one(&output, dst, dlen);
+ acomp_request_set_params(req, &input, &output, entry->length, dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
+ NULL);
+
+ ret = crypto_acomp_decompress(req);
+
+ put_cpu_ptr(entry->pool->acomp_req);
kunmap_atomic(dst);
zpool_unmap_handle(entry->pool->zpool, entry->handle);
BUG_ON(ret);
--
1.8.3.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v1 0/1] mm: zswap - crypto acomp/scomp support
From: Mahipal Challa @ 2017-02-14 15:40 UTC (permalink / raw)
To: herbert, sjenning, davem
Cc: linux-crypto, linux-kernel, linux-mm, pathreya, vnair,
Mahipal Challa
Hi Seth, Herbert,
This series adds support for kernel's new crypto acomp/scomp compression &
decompression framework to zswap. We verified these changes using the
kernel's crypto deflate-scomp, lzo-scomp modules and Cavium's ThunderX
ZIP driver (We will post the Cavium's ThunderX ZIP driver v2 patches with
acomp/scomp support soon).
Patch is on top of 'crypto-2.6' branch.
please provide your comments.
Regards,
Mahipal
Mahipal Challa (1):
mm: zswap - Add crypto acomp/scomp framework support
mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 99 insertions(+), 30 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp framework support
From: Seth Jennings @ 2017-02-14 16:20 UTC (permalink / raw)
To: Mahipal Challa
Cc: herbert, davem, linux-crypto, LKML, Linux-MM, pathreya, vnair,
Mahipal Challa, Vishnu Nair
In-Reply-To: <1487086821-5880-2-git-send-email-Mahipal.Challa@cavium.com>
On Tue, Feb 14, 2017 at 9:40 AM, Mahipal Challa
<mahipalreddy2006@gmail.com> wrote:
> This adds the support for kernel's crypto new acomp/scomp framework
> to zswap.
>
> Signed-off-by: Mahipal Challa <Mahipal.Challa@cavium.com>
> Signed-off-by: Vishnu Nair <Vishnu.Nair@cavium.com>
> ---
> mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 99 insertions(+), 30 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 067a0d6..d08631b 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -33,6 +33,8 @@
> #include <linux/rbtree.h>
> #include <linux/swap.h>
> #include <linux/crypto.h>
> +#include <crypto/acompress.h>
> +#include <linux/scatterlist.h>
> #include <linux/mempool.h>
> #include <linux/zpool.h>
>
> @@ -114,7 +116,8 @@ static int zswap_compressor_param_set(const char *,
>
> struct zswap_pool {
> struct zpool *zpool;
> - struct crypto_comp * __percpu *tfm;
> + struct crypto_acomp * __percpu *acomp;
> + struct acomp_req * __percpu *acomp_req;
> struct kref kref;
> struct list_head list;
> struct work_struct work;
> @@ -379,30 +382,49 @@ static int zswap_dstmem_dead(unsigned int cpu)
> static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
> {
> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
> - struct crypto_comp *tfm;
> + struct crypto_acomp *acomp;
> + struct acomp_req *acomp_req;
>
> - if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu)))
> + if (WARN_ON(*per_cpu_ptr(pool->acomp, cpu)))
> return 0;
> + if (WARN_ON(*per_cpu_ptr(pool->acomp_req, cpu)))
> + return 0;
> +
> + acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0);
> + if (IS_ERR_OR_NULL(acomp)) {
> + pr_err("could not alloc crypto acomp %s : %ld\n",
> + pool->tfm_name, PTR_ERR(acomp));
> + return -ENOMEM;
> + }
> + *per_cpu_ptr(pool->acomp, cpu) = acomp;
>
> - tfm = crypto_alloc_comp(pool->tfm_name, 0, 0);
> - if (IS_ERR_OR_NULL(tfm)) {
> - pr_err("could not alloc crypto comp %s : %ld\n",
> - pool->tfm_name, PTR_ERR(tfm));
> + acomp_req = acomp_request_alloc(acomp);
> + if (IS_ERR_OR_NULL(acomp_req)) {
> + pr_err("could not alloc crypto acomp %s : %ld\n",
> + pool->tfm_name, PTR_ERR(acomp));
> return -ENOMEM;
> }
> - *per_cpu_ptr(pool->tfm, cpu) = tfm;
> + *per_cpu_ptr(pool->acomp_req, cpu) = acomp_req;
> +
> return 0;
> }
>
> static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
> {
> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
> - struct crypto_comp *tfm;
> + struct crypto_acomp *acomp;
> + struct acomp_req *acomp_req;
> +
> + acomp_req = *per_cpu_ptr(pool->acomp_req, cpu);
> + if (!IS_ERR_OR_NULL(acomp_req))
> + acomp_request_free(acomp_req);
> + *per_cpu_ptr(pool->acomp_req, cpu) = NULL;
> +
> + acomp = *per_cpu_ptr(pool->acomp, cpu);
> + if (!IS_ERR_OR_NULL(acomp))
> + crypto_free_acomp(acomp);
> + *per_cpu_ptr(pool->acomp, cpu) = NULL;
>
> - tfm = *per_cpu_ptr(pool->tfm, cpu);
> - if (!IS_ERR_OR_NULL(tfm))
> - crypto_free_comp(tfm);
> - *per_cpu_ptr(pool->tfm, cpu) = NULL;
> return 0;
> }
>
> @@ -503,8 +525,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
>
> strlcpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
> - pool->tfm = alloc_percpu(struct crypto_comp *);
> - if (!pool->tfm) {
> + pool->acomp = alloc_percpu(struct crypto_acomp *);
> + if (!pool->acomp) {
> + pr_err("percpu alloc failed\n");
> + goto error;
> + }
> +
> + pool->acomp_req = alloc_percpu(struct acomp_req *);
> + if (!pool->acomp_req) {
> pr_err("percpu alloc failed\n");
> goto error;
> }
> @@ -526,7 +554,8 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> return pool;
>
> error:
> - free_percpu(pool->tfm);
> + free_percpu(pool->acomp_req);
> + free_percpu(pool->acomp);
> if (pool->zpool)
> zpool_destroy_pool(pool->zpool);
> kfree(pool);
> @@ -566,7 +595,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool)
> zswap_pool_debug("destroying", pool);
>
> cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
> - free_percpu(pool->tfm);
> + free_percpu(pool->acomp_req);
> + free_percpu(pool->acomp);
> zpool_destroy_pool(pool->zpool);
> kfree(pool);
> }
> @@ -763,7 +793,8 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
> pgoff_t offset;
> struct zswap_entry *entry;
> struct page *page;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> u8 *src, *dst;
> unsigned int dlen;
> int ret;
> @@ -803,14 +834,23 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>
> case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> /* decompress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> dlen = PAGE_SIZE;
> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
> ZPOOL_MM_RO) + sizeof(struct zswap_header);
> dst = kmap_atomic(page);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> - ret = crypto_comp_decompress(tfm, src, entry->length,
> - dst, &dlen);
> - put_cpu_ptr(entry->pool->tfm);
> +
> + sg_init_one(&input, src, entry->length);
> + sg_init_one(&output, dst, dlen);
> + acomp_request_set_params(req, &input, &output, entry->length,
> + dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> + NULL, NULL);
> +
> + ret = crypto_acomp_decompress(req);
I assume all of these crypto_acomp_[compress|decompress] calls are
actually synchronous,
not asynchronous as the name suggests. Otherwise, this would blow up
quite spectacularly
since all the resources we use in the call get derefed/unmapped below.
Could an async algorithm be implement/used that would break this assumption?
Seth
> +
> + dlen = req->dlen;
> + put_cpu_ptr(entry->pool->acomp_req);
> kunmap_atomic(dst);
> zpool_unmap_handle(entry->pool->zpool, entry->handle);
> BUG_ON(ret);
> @@ -886,7 +926,8 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> {
> struct zswap_tree *tree = zswap_trees[type];
> struct zswap_entry *entry, *dupentry;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> int ret;
> unsigned int dlen = PAGE_SIZE, len;
> unsigned long handle;
> @@ -925,12 +966,27 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> }
>
> /* compress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> + if (!req) {
> + put_cpu_ptr(entry->pool->acomp_req);
> + ret = -EINVAL;
> + goto freepage;
> + }
> +
> dst = get_cpu_var(zswap_dstmem);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> src = kmap_atomic(page);
> - ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
> +
> + sg_init_one(&input, src, PAGE_SIZE);
> + /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
> + sg_init_one(&output, dst, PAGE_SIZE * 2);
> + acomp_request_set_params(req, &input, &output, PAGE_SIZE, dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
> + NULL);
> +
> + ret = crypto_acomp_compress(req);
> kunmap_atomic(src);
> - put_cpu_ptr(entry->pool->tfm);
> + put_cpu_ptr(entry->pool->acomp_req);
> + dlen = req->dlen;
> if (ret) {
> ret = -EINVAL;
> goto put_dstmem;
> @@ -998,7 +1054,8 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> {
> struct zswap_tree *tree = zswap_trees[type];
> struct zswap_entry *entry;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> u8 *src, *dst;
> unsigned int dlen;
> int ret;
> @@ -1014,13 +1071,25 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> spin_unlock(&tree->lock);
>
> /* decompress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> + if (!req) {
> + put_cpu_ptr(entry->pool->acomp_req);
> + return -1;
> + }
> dlen = PAGE_SIZE;
> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
> ZPOOL_MM_RO) + sizeof(struct zswap_header);
> dst = kmap_atomic(page);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> - ret = crypto_comp_decompress(tfm, src, entry->length, dst, &dlen);
> - put_cpu_ptr(entry->pool->tfm);
> +
> + sg_init_one(&input, src, entry->length);
> + sg_init_one(&output, dst, dlen);
> + acomp_request_set_params(req, &input, &output, entry->length, dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
> + NULL);
> +
> + ret = crypto_acomp_decompress(req);
> +
> + put_cpu_ptr(entry->pool->acomp_req);
> kunmap_atomic(dst);
> zpool_unmap_handle(entry->pool->zpool, entry->handle);
> BUG_ON(ret);
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
From: Dan Williams @ 2017-02-14 16:34 UTC (permalink / raw)
To: Anup Patel
Cc: Vinod Koul, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Jassi Brar, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback,
dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Device Tree,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, linux-raid
In-Reply-To: <1487055112-5185-4-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> The Broadcom stream buffer accelerator (SBA) provides offloading
> capabilities for RAID operations. This SBA offload engine is
> accessible via Broadcom SoC specific ring manager.
>
> This patch adds Broadcom SBA RAID driver which provides one
> DMA device with RAID capabilities using one or more Broadcom
> SoC specific ring manager channels. The SBA RAID driver in its
> current shape implements memcpy, xor, and pq operations.
>
> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
> drivers/dma/Kconfig | 13 +
> drivers/dma/Makefile | 1 +
> drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1708 insertions(+)
> create mode 100644 drivers/dma/bcm-sba-raid.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 263495d..bf8fb84 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -99,6 +99,19 @@ config AXI_DMAC
> controller is often used in Analog Device's reference designs for FPGA
> platforms.
>
> +config BCM_SBA_RAID
> + tristate "Broadcom SBA RAID engine support"
> + depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
> + select DMA_ENGINE
> + select DMA_ENGINE_RAID
> + select ASYNC_TX_ENABLE_CHANNEL_SWITCH
I thought you agreed to drop this. Its usage is broken.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] crypto: cavium: fix Kconfig dependencies
From: Arnd Bergmann @ 2017-02-14 17:07 UTC (permalink / raw)
To: George Cherian, Herbert Xu, David S. Miller
Cc: Arnd Bergmann, David Daney, linux-crypto, linux-kernel
The driver fails to build if MSI support is disabled:
In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete element type 'struct msix_entry'
struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
^~~~~~~~~~~~
drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of function 'pci_enable_msix';did you mean 'cpt_enable_msix'? [-Werror=implicit-function-declaration]
On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
so let's allow compile-testing to catch this kind of problem more easily.
The 64-bit dependency is needed for the use of readq/writeq.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/crypto/cavium/cpt/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/cavium/cpt/Kconfig b/drivers/crypto/cavium/cpt/Kconfig
index 247f1cbbefc1..cbd51b1aa046 100644
--- a/drivers/crypto/cavium/cpt/Kconfig
+++ b/drivers/crypto/cavium/cpt/Kconfig
@@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT
config CAVIUM_CPT
tristate "Cavium Cryptographic Accelerator driver"
- depends on ARCH_THUNDER
+ depends on ARCH_THUNDER || COMPILE_TEST
+ depends on PCI_MSI && 64BIT
select CRYPTO_DEV_CPT
help
Support for Cavium CPT block found in octeon-tx series of
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] crypto: cavium: fix Kconfig dependencies
From: David Daney @ 2017-02-14 17:09 UTC (permalink / raw)
To: Arnd Bergmann, George Cherian, Herbert Xu, David S. Miller
Cc: David Daney, linux-crypto, linux-kernel
In-Reply-To: <20170214170739.2687818-1-arnd@arndb.de>
On 02/14/2017 09:07 AM, Arnd Bergmann wrote:
> The driver fails to build if MSI support is disabled:
>
> In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
> drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete element type 'struct msix_entry'
> struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
> ^~~~~~~~~~~~
> drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
> drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of function 'pci_enable_msix';did you mean 'cpt_enable_msix'? [-Werror=implicit-function-declaration]
>
> On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
> so let's allow compile-testing to catch this kind of problem more easily.
> The 64-bit dependency is needed for the use of readq/writeq.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/crypto/cavium/cpt/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/cavium/cpt/Kconfig b/drivers/crypto/cavium/cpt/Kconfig
> index 247f1cbbefc1..cbd51b1aa046 100644
> --- a/drivers/crypto/cavium/cpt/Kconfig
> +++ b/drivers/crypto/cavium/cpt/Kconfig
> @@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT
>
> config CAVIUM_CPT
> tristate "Cavium Cryptographic Accelerator driver"
> - depends on ARCH_THUNDER
> + depends on ARCH_THUNDER || COMPILE_TEST
> + depends on PCI_MSI && 64BIT
Perhaps we should select PCI and PCI_MSI instead.
These systems cannot function without those.
> select CRYPTO_DEV_CPT
> help
> Support for Cavium CPT block found in octeon-tx series of
>
^ permalink raw reply
* [PATCH 1/1] crypto: brcm - Avoid double free in ahash_finup()
From: Rob Rice @ 2017-02-14 17:45 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Dan Carpenter, linux-crypto,
linux-kernel, bcm-kernel-feedback-list
Cc: Rob Rice
In Broadcom SPU driver, in case where incremental hash
is done in software in ahash_finup(), tmpbuf was freed
twice.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rob Rice <rob.rice@broadcom.com>
---
drivers/crypto/bcm/cipher.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/crypto/bcm/cipher.c b/drivers/crypto/bcm/cipher.c
index a654a01..cc0d5b9 100644
--- a/drivers/crypto/bcm/cipher.c
+++ b/drivers/crypto/bcm/cipher.c
@@ -2331,7 +2331,6 @@ static int ahash_finup(struct ahash_request *req)
/* Call synchronous update */
ret = crypto_shash_finup(ctx->shash, tmpbuf, req->nbytes,
req->result);
- kfree(tmpbuf);
} else {
/* Otherwise call the internal function which uses SPU hw */
return __ahash_finup(req);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] crypto: cavium: fix Kconfig dependencies
From: Randy Dunlap @ 2017-02-14 18:26 UTC (permalink / raw)
To: David Daney, Arnd Bergmann, George Cherian, Herbert Xu,
David S. Miller
Cc: David Daney, linux-crypto, linux-kernel
In-Reply-To: <09cdd8d4-f84b-9664-15e2-4fe923d0b744@caviumnetworks.com>
On 02/14/17 09:09, David Daney wrote:
> On 02/14/2017 09:07 AM, Arnd Bergmann wrote:
>> The driver fails to build if MSI support is disabled:
>>
>> In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
>> drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete element type 'struct msix_entry'
>> struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
>> ^~~~~~~~~~~~
>> drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
>> drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of function 'pci_enable_msix';did you mean 'cpt_enable_msix'? [-Werror=implicit-function-declaration]
>>
>> On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
>> so let's allow compile-testing to catch this kind of problem more easily.
>> The 64-bit dependency is needed for the use of readq/writeq.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/crypto/cavium/cpt/Kconfig | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/cavium/cpt/Kconfig b/drivers/crypto/cavium/cpt/Kconfig
>> index 247f1cbbefc1..cbd51b1aa046 100644
>> --- a/drivers/crypto/cavium/cpt/Kconfig
>> +++ b/drivers/crypto/cavium/cpt/Kconfig
>> @@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT
>>
>> config CAVIUM_CPT
>> tristate "Cavium Cryptographic Accelerator driver"
>> - depends on ARCH_THUNDER
>> + depends on ARCH_THUNDER || COMPILE_TEST
>> + depends on PCI_MSI && 64BIT
>
>
> Perhaps we should select PCI and PCI_MSI instead.
>
> These systems cannot function without those.
Then the "depends" (and hence the patch) is correct.
A driver should not enable all of PCI if it is disabled
for some other reason.
>> select CRYPTO_DEV_CPT
>> help
>> Support for Cavium CPT block found in octeon-tx series of
>>
>
--
~Randy
^ permalink raw reply
* Re: [PATCH] crypto: cavium: fix Kconfig dependencies
From: David Daney @ 2017-02-14 18:30 UTC (permalink / raw)
To: Randy Dunlap, Arnd Bergmann, George Cherian, Herbert Xu,
David S. Miller
Cc: David Daney, linux-crypto, linux-kernel
In-Reply-To: <47441e97-99be-15e7-ea15-b35a4a827139@infradead.org>
On 02/14/2017 10:26 AM, Randy Dunlap wrote:
> On 02/14/17 09:09, David Daney wrote:
>> On 02/14/2017 09:07 AM, Arnd Bergmann wrote:
>>> The driver fails to build if MSI support is disabled:
>>>
>>> In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
>>> drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete element type 'struct msix_entry'
>>> struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
>>> ^~~~~~~~~~~~
>>> drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
>>> drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of function 'pci_enable_msix';did you mean 'cpt_enable_msix'? [-Werror=implicit-function-declaration]
>>>
>>> On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
>>> so let's allow compile-testing to catch this kind of problem more easily.
>>> The 64-bit dependency is needed for the use of readq/writeq.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> drivers/crypto/cavium/cpt/Kconfig | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/cavium/cpt/Kconfig b/drivers/crypto/cavium/cpt/Kconfig
>>> index 247f1cbbefc1..cbd51b1aa046 100644
>>> --- a/drivers/crypto/cavium/cpt/Kconfig
>>> +++ b/drivers/crypto/cavium/cpt/Kconfig
>>> @@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT
>>>
>>> config CAVIUM_CPT
>>> tristate "Cavium Cryptographic Accelerator driver"
>>> - depends on ARCH_THUNDER
>>> + depends on ARCH_THUNDER || COMPILE_TEST
>>> + depends on PCI_MSI && 64BIT
>>
>>
>> Perhaps we should select PCI and PCI_MSI instead.
>>
>> These systems cannot function without those.
>
> Then the "depends" (and hence the patch) is correct.
>
> A driver should not enable all of PCI if it is disabled
> for some other reason.
I see your point. In that case, this patch:
Acked-by: David Daney <david.daney@cavium.com>
>
>>> select CRYPTO_DEV_CPT
>>> help
>>> Support for Cavium CPT block found in octeon-tx series of
>>>
>>
>
>
^ permalink raw reply
* Re: [PATCH 1/2] crypto: arm/aes-neonbs - resolve fallback cipher at runtime
From: Ard Biesheuvel @ 2017-02-14 19:11 UTC (permalink / raw)
To: linux-crypto@vger.kernel.org, Herbert Xu; +Cc: Ard Biesheuvel
In-Reply-To: <1487066640-17886-1-git-send-email-ard.biesheuvel@linaro.org>
On 14 February 2017 at 10:03, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Currently, the bit sliced NEON AES code for ARM has a link time
> dependency on the scalar ARM asm implementation, which it uses as a
> fallback to perform CBC encryption and the encryption of the initial
> XTS tweak.
>
> The bit sliced NEON code is both fast and time invariant, which makes
> it a reasonable default on hardware that supports it. However, the
> ARM asm code it pulls in is not time invariant, and due to the way it
> is linked in, cannot be overridden by the new generic time invariant
> driver. In fact, it will not be used at all, given that the ARM asm
> code registers itself as a cipher with a priority that exceeds the
> priority of the fixed time cipher.
>
> So remove the link time dependency, and allocate the fallback cipher
> via the crypto API. Note that this requires this driver's module_init
> call to be replaced with late_initcall, so that the (possibly generic)
> fallback cipher is guaranteed to be available when the builtin test
> is performed at registration time.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm/crypto/Kconfig | 2 +-
> arch/arm/crypto/aes-neonbs-glue.c | 65 ++++++++++++++++++++++++++++++---------
> 2 files changed, 51 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
> index a8fce93137fb..b9adedcc5b2e 100644
> --- a/arch/arm/crypto/Kconfig
> +++ b/arch/arm/crypto/Kconfig
> @@ -73,7 +73,7 @@ config CRYPTO_AES_ARM_BS
> depends on KERNEL_MODE_NEON
> select CRYPTO_BLKCIPHER
> select CRYPTO_SIMD
> - select CRYPTO_AES_ARM
> + select CRYPTO_AES
> help
> Use a faster and more secure NEON based implementation of AES in CBC,
> CTR and XTS modes
> diff --git a/arch/arm/crypto/aes-neonbs-glue.c b/arch/arm/crypto/aes-neonbs-glue.c
> index 2920b96dbd36..6a2a30b9e4f5 100644
> --- a/arch/arm/crypto/aes-neonbs-glue.c
> +++ b/arch/arm/crypto/aes-neonbs-glue.c
> @@ -42,9 +42,6 @@ asmlinkage void aesbs_xts_encrypt(u8 out[], u8 const in[], u8 const rk[],
> asmlinkage void aesbs_xts_decrypt(u8 out[], u8 const in[], u8 const rk[],
> int rounds, int blocks, u8 iv[]);
>
> -asmlinkage void __aes_arm_encrypt(const u32 rk[], int rounds, const u8 in[],
> - u8 out[]);
> -
> struct aesbs_ctx {
> int rounds;
> u8 rk[13 * (8 * AES_BLOCK_SIZE) + 32] __aligned(AES_BLOCK_SIZE);
> @@ -52,12 +49,12 @@ struct aesbs_ctx {
>
> struct aesbs_cbc_ctx {
> struct aesbs_ctx key;
> - u32 enc[AES_MAX_KEYLENGTH_U32];
> + struct crypto_cipher *enc_tfm;
> };
>
> struct aesbs_xts_ctx {
> struct aesbs_ctx key;
> - u32 twkey[AES_MAX_KEYLENGTH_U32];
> + struct crypto_cipher *tweak_tfm;
> };
>
> static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
> @@ -132,20 +129,18 @@ static int aesbs_cbc_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
>
> ctx->key.rounds = 6 + key_len / 4;
>
> - memcpy(ctx->enc, rk.key_enc, sizeof(ctx->enc));
> -
> kernel_neon_begin();
> aesbs_convert_key(ctx->key.rk, rk.key_enc, ctx->key.rounds);
> kernel_neon_end();
>
> - return 0;
> + return crypto_cipher_setkey(ctx->enc_tfm, in_key, key_len);
> }
>
> static void cbc_encrypt_one(struct crypto_skcipher *tfm, const u8 *src, u8 *dst)
> {
> struct aesbs_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
>
> - __aes_arm_encrypt(ctx->enc, ctx->key.rounds, src, dst);
> + crypto_cipher_encrypt_one(ctx->enc_tfm, dst, src);
> }
>
> static int cbc_encrypt(struct skcipher_request *req)
> @@ -181,6 +176,23 @@ static int cbc_decrypt(struct skcipher_request *req)
> return err;
> }
>
> +static int cbc_init(struct crypto_tfm *tfm)
> +{
> + struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
> +
> + ctx->enc_tfm = crypto_alloc_cipher("aes", 0, 0);
> + if (IS_ERR(ctx->enc_tfm))
> + return PTR_ERR(ctx->enc_tfm);
> + return 0;
> +}
> +
> +static void cbc_exit(struct crypto_tfm *tfm)
> +{
> + struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
> +
> + crypto_free_cipher(ctx->enc_tfm);
> +}
> +
> static int ctr_encrypt(struct skcipher_request *req)
> {
> struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> @@ -228,7 +240,6 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
> unsigned int key_len)
> {
> struct aesbs_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
> - struct crypto_aes_ctx rk;
> int err;
>
> err = xts_verify_key(tfm, in_key, key_len);
> @@ -236,13 +247,33 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
> return err;
>
> key_len /= 2;
> - err = crypto_aes_expand_key(&rk, in_key + key_len, key_len);
> + err = crypto_cipher_setkey(ctx->tweak_tfm, in_key + key_len, key_len);
> if (err)
> return err;
>
> - memcpy(ctx->twkey, rk.key_enc, sizeof(ctx->twkey));
> + err = aesbs_setkey(tfm, in_key, key_len);
> + if (err)
> + return err;
> +
> + return 0;
> +
This looks a bit silly, especially because there is no need to change
it in the first place. Will spin a v2
> +}
> +
> +static int xts_init(struct crypto_tfm *tfm)
> +{
> + struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
> +
> + ctx->tweak_tfm = crypto_alloc_cipher("aes", 0, 0);
> + if (IS_ERR(ctx->tweak_tfm))
> + return PTR_ERR(ctx->tweak_tfm);
> + return 0;
> +}
> +
> +static void xts_exit(struct crypto_tfm *tfm)
> +{
> + struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
>
> - return aesbs_setkey(tfm, in_key, key_len);
> + crypto_free_cipher(ctx->tweak_tfm);
> }
>
> static int __xts_crypt(struct skcipher_request *req,
> @@ -256,7 +287,7 @@ static int __xts_crypt(struct skcipher_request *req,
>
> err = skcipher_walk_virt(&walk, req, true);
>
> - __aes_arm_encrypt(ctx->twkey, ctx->key.rounds, walk.iv, walk.iv);
> + crypto_cipher_encrypt_one(ctx->tweak_tfm, walk.iv, walk.iv);
>
> kernel_neon_begin();
> while (walk.nbytes >= AES_BLOCK_SIZE) {
> @@ -309,6 +340,8 @@ static struct skcipher_alg aes_algs[] = { {
> .base.cra_ctxsize = sizeof(struct aesbs_cbc_ctx),
> .base.cra_module = THIS_MODULE,
> .base.cra_flags = CRYPTO_ALG_INTERNAL,
> + .base.cra_init = cbc_init,
> + .base.cra_exit = cbc_exit,
>
> .min_keysize = AES_MIN_KEY_SIZE,
> .max_keysize = AES_MAX_KEY_SIZE,
> @@ -342,6 +375,8 @@ static struct skcipher_alg aes_algs[] = { {
> .base.cra_ctxsize = sizeof(struct aesbs_xts_ctx),
> .base.cra_module = THIS_MODULE,
> .base.cra_flags = CRYPTO_ALG_INTERNAL,
> + .base.cra_init = xts_init,
> + .base.cra_exit = xts_exit,
>
> .min_keysize = 2 * AES_MIN_KEY_SIZE,
> .max_keysize = 2 * AES_MAX_KEY_SIZE,
> @@ -402,5 +437,5 @@ static int __init aes_init(void)
> return err;
> }
>
> -module_init(aes_init);
> +late_initcall(aes_init);
> module_exit(aes_exit);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 0/4] crypto: gf128mul cleanups
From: Eric Biggers @ 2017-02-14 21:43 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu, David S . Miller, Eric Biggers
This patchset makes a few cleanups to the generic GF(2^128) multiplication code
to make it slightly easier to understand and modify. No functional changes are
intended.
Eric Biggers (4):
crypto: gf128mul - fix some comments
crypto: gf128mul - remove xx() macro
crypto: gf128mul - rename the byte overflow tables
crypto: gf128mul - constify 4k and 64k multiplication tables
crypto/gf128mul.c | 86 +++++++++++++++++++++++++++--------------------
include/crypto/gf128mul.h | 32 +++++++++---------
2 files changed, 67 insertions(+), 51 deletions(-)
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply
* [PATCH 1/4] crypto: gf128mul - fix some comments
From: Eric Biggers @ 2017-02-14 21:43 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu, David S . Miller, Eric Biggers, Alex Cope
In-Reply-To: <20170214214330.99845-1-ebiggers@google.com>
Fix incorrect references to GF(128) instead of GF(2^128), as these are
two entirely different fields, and fix a few other incorrect comments.
Cc: Alex Cope <alexcope@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/gf128mul.c | 13 +++++++------
include/crypto/gf128mul.h | 26 ++++++++++++++------------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 72015fee533d..d9e3eecc218a 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -44,7 +44,7 @@
---------------------------------------------------------------------------
Issue 31/01/2006
- This file provides fast multiplication in GF(128) as required by several
+ This file provides fast multiplication in GF(2^128) as required by several
cryptographic authentication modes
*/
@@ -116,9 +116,10 @@
static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);
static const u16 gf128mul_table_bbe[256] = gf128mul_dat(xda_bbe);
-/* These functions multiply a field element by x, by x^4 and by x^8
- * in the polynomial field representation. It uses 32-bit word operations
- * to gain speed but compensates for machine endianess and hence works
+/*
+ * The following functions multiply a field element by x or by x^8 in
+ * the polynomial field representation. They use 64-bit word operations
+ * to gain speed but compensate for machine endianness and hence work
* correctly on both styles of machine.
*/
@@ -251,7 +252,7 @@ EXPORT_SYMBOL(gf128mul_bbe);
/* This version uses 64k bytes of table space.
A 16 byte buffer has to be multiplied by a 16 byte key
- value in GF(128). If we consider a GF(128) value in
+ value in GF(2^128). If we consider a GF(2^128) value in
the buffer's lowest byte, we can construct a table of
the 256 16 byte values that result from the 256 values
of this byte. This requires 4096 bytes. But we also
@@ -330,7 +331,7 @@ EXPORT_SYMBOL(gf128mul_64k_bbe);
/* This version uses 4k bytes of table space.
A 16 byte buffer has to be multiplied by a 16 byte key
- value in GF(128). If we consider a GF(128) value in a
+ value in GF(2^128). If we consider a GF(2^128) value in a
single byte, we can construct a table of the 256 16 byte
values that result from the 256 values of this byte.
This requires 4096 bytes. If we take the highest byte in
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 592d47e565a8..9662c4538873 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -43,7 +43,7 @@
---------------------------------------------------------------------------
Issue Date: 31/01/2006
- An implementation of field multiplication in Galois Field GF(128)
+ An implementation of field multiplication in Galois Field GF(2^128)
*/
#ifndef _CRYPTO_GF128MUL_H
@@ -65,7 +65,7 @@
* are left and the lsb's are right. char b[16] is an array and b[0] is
* the first octet.
*
- * 80000000 00000000 00000000 00000000 .... 00000000 00000000 00000000
+ * 10000000 00000000 00000000 00000000 .... 00000000 00000000 00000000
* b[0] b[1] b[2] b[3] b[13] b[14] b[15]
*
* Every bit is a coefficient of some power of X. We can store the bits
@@ -85,15 +85,17 @@
* Both of the above formats are easy to implement on big-endian
* machines.
*
- * EME (which is patent encumbered) uses the ble format (bits are stored
- * in big endian order and the bytes in little endian). The above buffer
- * represents X^7 in this case and the primitive polynomial is b[0] = 0x87.
+ * XTS and EME (the latter of which is patent encumbered) use the ble
+ * format (bits are stored in big endian order and the bytes in little
+ * endian). The above buffer represents X^7 in this case and the
+ * primitive polynomial is b[0] = 0x87.
*
* The common machine word-size is smaller than 128 bits, so to make
* an efficient implementation we must split into machine word sizes.
- * This file uses one 32bit for the moment. Machine endianness comes into
- * play. The lle format in relation to machine endianness is discussed
- * below by the original author of gf128mul Dr Brian Gladman.
+ * This implementation uses 64-bit words for the moment. Machine
+ * endianness comes into play. The lle format in relation to machine
+ * endianness is discussed below by the original author of gf128mul Dr
+ * Brian Gladman.
*
* Let's look at the bbe and ble format on a little endian machine.
*
@@ -127,10 +129,10 @@
* machines this will automatically aligned to wordsize and on a 64-bit
* machine also.
*/
-/* Multiply a GF128 field element by x. Field elements are held in arrays
- of bytes in which field bits 8n..8n + 7 are held in byte[n], with lower
- indexed bits placed in the more numerically significant bit positions
- within bytes.
+/* Multiply a GF(2^128) field element by x. Field elements are
+ held in arrays of bytes in which field bits 8n..8n + 7 are held in
+ byte[n], with lower indexed bits placed in the more numerically
+ significant bit positions within bytes.
On little endian machines the bit indexes translate into the bit
positions within four 32-bit words in the following way
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 2/4] crypto: gf128mul - remove xx() macro
From: Eric Biggers @ 2017-02-14 21:43 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu, David S . Miller, Eric Biggers, Alex Cope
In-Reply-To: <20170214214330.99845-1-ebiggers@google.com>
The xx() macro serves no purpose and can be removed.
Cc: Alex Cope <alexcope@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/gf128mul.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index d9e3eecc218a..c050cf6f5aa9 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -97,20 +97,18 @@
the table above
*/
-#define xx(p, q) 0x##p##q
-
#define xda_bbe(i) ( \
- (i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
- (i & 0x20 ? xx(10, e0) : 0) ^ (i & 0x10 ? xx(08, 70) : 0) ^ \
- (i & 0x08 ? xx(04, 38) : 0) ^ (i & 0x04 ? xx(02, 1c) : 0) ^ \
- (i & 0x02 ? xx(01, 0e) : 0) ^ (i & 0x01 ? xx(00, 87) : 0) \
+ (i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
+ (i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
+ (i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
+ (i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
)
#define xda_lle(i) ( \
- (i & 0x80 ? xx(e1, 00) : 0) ^ (i & 0x40 ? xx(70, 80) : 0) ^ \
- (i & 0x20 ? xx(38, 40) : 0) ^ (i & 0x10 ? xx(1c, 20) : 0) ^ \
- (i & 0x08 ? xx(0e, 10) : 0) ^ (i & 0x04 ? xx(07, 08) : 0) ^ \
- (i & 0x02 ? xx(03, 84) : 0) ^ (i & 0x01 ? xx(01, c2) : 0) \
+ (i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
+ (i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
+ (i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
+ (i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
)
static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 3/4] crypto: gf128mul - rename the byte overflow tables
From: Eric Biggers @ 2017-02-14 21:43 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu, David S . Miller, Eric Biggers, Alex Cope
In-Reply-To: <20170214214330.99845-1-ebiggers@google.com>
Though the GF(2^128) byte overflow tables were named the "lle" and "bbe"
tables, they are not actually tied to these element formats
specifically, but rather to particular a "bit endianness". For example,
the bbe table is actually used for both bbe and ble multiplication.
Therefore, rename the tables to "le" and "be" and update the comment to
explain this.
Cc: Alex Cope <alexcope@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/gf128mul.c | 49 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 17 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index c050cf6f5aa9..1fde1c79ffa5 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -88,31 +88,46 @@
q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
}
-/* Given the value i in 0..255 as the byte overflow when a field element
- in GHASH is multiplied by x^8, this function will return the values that
- are generated in the lo 16-bit word of the field value by applying the
- modular polynomial. The values lo_byte and hi_byte are returned via the
- macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
- memory as required by a suitable definition of this macro operating on
- the table above
-*/
+/*
+ * Given a value i in 0..255 as the byte overflow when a field element
+ * in GF(2^128) is multiplied by x^8, the following macro returns the
+ * 16-bit value that must be XOR-ed into the low-degree end of the
+ * product to reduce it modulo the polynomial x^128 + x^7 + x^2 + x + 1.
+ *
+ * There are two versions of the macro, and hence two tables: one for
+ * the "be" convention where the highest-order bit is the coefficient of
+ * the highest-degree polynomial term, and one for the "le" convention
+ * where the highest-order bit is the coefficient of the lowest-degree
+ * polynomial term. In both cases the values are stored in CPU byte
+ * endianness such that the coefficients are ordered consistently across
+ * bytes, i.e. in the "be" table bits 15..0 of the stored value
+ * correspond to the coefficients of x^15..x^0, and in the "le" table
+ * bits 15..0 correspond to the coefficients of x^0..x^15.
+ *
+ * Therefore, provided that the appropriate byte endianness conversions
+ * are done by the multiplication functions (and these must be in place
+ * anyway to support both little endian and big endian CPUs), the "be"
+ * table can be used for multiplications of both "bbe" and "ble"
+ * elements, and the "le" table can be used for multiplications of both
+ * "lle" and "lbe" elements.
+ */
-#define xda_bbe(i) ( \
+#define xda_be(i) ( \
(i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
(i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
(i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
(i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
)
-#define xda_lle(i) ( \
+#define xda_le(i) ( \
(i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
(i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
(i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
(i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
)
-static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);
-static const u16 gf128mul_table_bbe[256] = gf128mul_dat(xda_bbe);
+static const u16 gf128mul_table_le[256] = gf128mul_dat(xda_le);
+static const u16 gf128mul_table_be[256] = gf128mul_dat(xda_be);
/*
* The following functions multiply a field element by x or by x^8 in
@@ -125,7 +140,7 @@ static void gf128mul_x_lle(be128 *r, const be128 *x)
{
u64 a = be64_to_cpu(x->a);
u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_lle[(b << 7) & 0xff];
+ u64 _tt = gf128mul_table_le[(b << 7) & 0xff];
r->b = cpu_to_be64((b >> 1) | (a << 63));
r->a = cpu_to_be64((a >> 1) ^ (_tt << 48));
@@ -135,7 +150,7 @@ static void gf128mul_x_bbe(be128 *r, const be128 *x)
{
u64 a = be64_to_cpu(x->a);
u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_bbe[a >> 63];
+ u64 _tt = gf128mul_table_be[a >> 63];
r->a = cpu_to_be64((a << 1) | (b >> 63));
r->b = cpu_to_be64((b << 1) ^ _tt);
@@ -145,7 +160,7 @@ void gf128mul_x_ble(be128 *r, const be128 *x)
{
u64 a = le64_to_cpu(x->a);
u64 b = le64_to_cpu(x->b);
- u64 _tt = gf128mul_table_bbe[b >> 63];
+ u64 _tt = gf128mul_table_be[b >> 63];
r->a = cpu_to_le64((a << 1) ^ _tt);
r->b = cpu_to_le64((b << 1) | (a >> 63));
@@ -156,7 +171,7 @@ static void gf128mul_x8_lle(be128 *x)
{
u64 a = be64_to_cpu(x->a);
u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_lle[b & 0xff];
+ u64 _tt = gf128mul_table_le[b & 0xff];
x->b = cpu_to_be64((b >> 8) | (a << 56));
x->a = cpu_to_be64((a >> 8) ^ (_tt << 48));
@@ -166,7 +181,7 @@ static void gf128mul_x8_bbe(be128 *x)
{
u64 a = be64_to_cpu(x->a);
u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_bbe[a >> 56];
+ u64 _tt = gf128mul_table_be[a >> 56];
x->a = cpu_to_be64((a << 8) | (b >> 56));
x->b = cpu_to_be64((b << 8) ^ _tt);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH 4/4] crypto: gf128mul - constify 4k and 64k multiplication tables
From: Eric Biggers @ 2017-02-14 21:43 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu, David S . Miller, Eric Biggers, Alex Cope
In-Reply-To: <20170214214330.99845-1-ebiggers@google.com>
Constify the multiplication tables passed to the 4k and 64k
multiplication functions, as they are not modified by these functions.
Cc: Alex Cope <alexcope@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/gf128mul.c | 6 +++---
include/crypto/gf128mul.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 1fde1c79ffa5..04facc0690aa 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -329,7 +329,7 @@ void gf128mul_free_64k(struct gf128mul_64k *t)
}
EXPORT_SYMBOL(gf128mul_free_64k);
-void gf128mul_64k_bbe(be128 *a, struct gf128mul_64k *t)
+void gf128mul_64k_bbe(be128 *a, const struct gf128mul_64k *t)
{
u8 *ap = (u8 *)a;
be128 r[1];
@@ -402,7 +402,7 @@ struct gf128mul_4k *gf128mul_init_4k_bbe(const be128 *g)
}
EXPORT_SYMBOL(gf128mul_init_4k_bbe);
-void gf128mul_4k_lle(be128 *a, struct gf128mul_4k *t)
+void gf128mul_4k_lle(be128 *a, const struct gf128mul_4k *t)
{
u8 *ap = (u8 *)a;
be128 r[1];
@@ -417,7 +417,7 @@ void gf128mul_4k_lle(be128 *a, struct gf128mul_4k *t)
}
EXPORT_SYMBOL(gf128mul_4k_lle);
-void gf128mul_4k_bbe(be128 *a, struct gf128mul_4k *t)
+void gf128mul_4k_bbe(be128 *a, const struct gf128mul_4k *t)
{
u8 *ap = (u8 *)a;
be128 r[1];
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 9662c4538873..0bc9b5f1c45e 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -174,8 +174,8 @@ struct gf128mul_4k {
struct gf128mul_4k *gf128mul_init_4k_lle(const be128 *g);
struct gf128mul_4k *gf128mul_init_4k_bbe(const be128 *g);
-void gf128mul_4k_lle(be128 *a, struct gf128mul_4k *t);
-void gf128mul_4k_bbe(be128 *a, struct gf128mul_4k *t);
+void gf128mul_4k_lle(be128 *a, const struct gf128mul_4k *t);
+void gf128mul_4k_bbe(be128 *a, const struct gf128mul_4k *t);
static inline void gf128mul_free_4k(struct gf128mul_4k *t)
{
@@ -196,6 +196,6 @@ struct gf128mul_64k {
*/
struct gf128mul_64k *gf128mul_init_64k_bbe(const be128 *g);
void gf128mul_free_64k(struct gf128mul_64k *t);
-void gf128mul_64k_bbe(be128 *a, struct gf128mul_64k *t);
+void gf128mul_64k_bbe(be128 *a, const struct gf128mul_64k *t);
#endif /* _CRYPTO_GF128MUL_H */
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH v2 1/2] crypto: arm/aes-neonbs - resolve fallback cipher at runtime
From: Ard Biesheuvel @ 2017-02-14 21:51 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
Currently, the bit sliced NEON AES code for ARM has a link time
dependency on the scalar ARM asm implementation, which it uses as a
fallback to perform CBC encryption and the encryption of the initial
XTS tweak.
The bit sliced NEON code is both fast and time invariant, which makes
it a reasonable default on hardware that supports it. However, the
ARM asm code it pulls in is not time invariant, and due to the way it
is linked in, cannot be overridden by the new generic time invariant
driver. In fact, it will not be used at all, given that the ARM asm
code registers itself as a cipher with a priority that exceeds the
priority of the fixed time cipher.
So remove the link time dependency, and allocate the fallback cipher
via the crypto API. Note that this requires this driver's module_init
call to be replaced with late_initcall, so that the (possibly generic)
fallback cipher is guaranteed to be available when the builtin test
is performed at registration time.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: remove spurious change from aesbs_xts_setkey()
arch/arm/crypto/Kconfig | 2 +-
arch/arm/crypto/aes-neonbs-glue.c | 60 +++++++++++++++-----
2 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index a8fce93137fb..b9adedcc5b2e 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -73,7 +73,7 @@ config CRYPTO_AES_ARM_BS
depends on KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_SIMD
- select CRYPTO_AES_ARM
+ select CRYPTO_AES
help
Use a faster and more secure NEON based implementation of AES in CBC,
CTR and XTS modes
diff --git a/arch/arm/crypto/aes-neonbs-glue.c b/arch/arm/crypto/aes-neonbs-glue.c
index 2920b96dbd36..c76377961444 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -42,9 +42,6 @@ asmlinkage void aesbs_xts_encrypt(u8 out[], u8 const in[], u8 const rk[],
asmlinkage void aesbs_xts_decrypt(u8 out[], u8 const in[], u8 const rk[],
int rounds, int blocks, u8 iv[]);
-asmlinkage void __aes_arm_encrypt(const u32 rk[], int rounds, const u8 in[],
- u8 out[]);
-
struct aesbs_ctx {
int rounds;
u8 rk[13 * (8 * AES_BLOCK_SIZE) + 32] __aligned(AES_BLOCK_SIZE);
@@ -52,12 +49,12 @@ struct aesbs_ctx {
struct aesbs_cbc_ctx {
struct aesbs_ctx key;
- u32 enc[AES_MAX_KEYLENGTH_U32];
+ struct crypto_cipher *enc_tfm;
};
struct aesbs_xts_ctx {
struct aesbs_ctx key;
- u32 twkey[AES_MAX_KEYLENGTH_U32];
+ struct crypto_cipher *tweak_tfm;
};
static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
@@ -132,20 +129,18 @@ static int aesbs_cbc_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
ctx->key.rounds = 6 + key_len / 4;
- memcpy(ctx->enc, rk.key_enc, sizeof(ctx->enc));
-
kernel_neon_begin();
aesbs_convert_key(ctx->key.rk, rk.key_enc, ctx->key.rounds);
kernel_neon_end();
- return 0;
+ return crypto_cipher_setkey(ctx->enc_tfm, in_key, key_len);
}
static void cbc_encrypt_one(struct crypto_skcipher *tfm, const u8 *src, u8 *dst)
{
struct aesbs_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
- __aes_arm_encrypt(ctx->enc, ctx->key.rounds, src, dst);
+ crypto_cipher_encrypt_one(ctx->enc_tfm, dst, src);
}
static int cbc_encrypt(struct skcipher_request *req)
@@ -181,6 +176,23 @@ static int cbc_decrypt(struct skcipher_request *req)
return err;
}
+static int cbc_init(struct crypto_tfm *tfm)
+{
+ struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->enc_tfm = crypto_alloc_cipher("aes", 0, 0);
+ if (IS_ERR(ctx->enc_tfm))
+ return PTR_ERR(ctx->enc_tfm);
+ return 0;
+}
+
+static void cbc_exit(struct crypto_tfm *tfm)
+{
+ struct aesbs_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ crypto_free_cipher(ctx->enc_tfm);
+}
+
static int ctr_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
@@ -228,7 +240,6 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
unsigned int key_len)
{
struct aesbs_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
- struct crypto_aes_ctx rk;
int err;
err = xts_verify_key(tfm, in_key, key_len);
@@ -236,15 +247,30 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
return err;
key_len /= 2;
- err = crypto_aes_expand_key(&rk, in_key + key_len, key_len);
+ err = crypto_cipher_setkey(ctx->tweak_tfm, in_key + key_len, key_len);
if (err)
return err;
- memcpy(ctx->twkey, rk.key_enc, sizeof(ctx->twkey));
-
return aesbs_setkey(tfm, in_key, key_len);
}
+static int xts_init(struct crypto_tfm *tfm)
+{
+ struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->tweak_tfm = crypto_alloc_cipher("aes", 0, 0);
+ if (IS_ERR(ctx->tweak_tfm))
+ return PTR_ERR(ctx->tweak_tfm);
+ return 0;
+}
+
+static void xts_exit(struct crypto_tfm *tfm)
+{
+ struct aesbs_xts_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ crypto_free_cipher(ctx->tweak_tfm);
+}
+
static int __xts_crypt(struct skcipher_request *req,
void (*fn)(u8 out[], u8 const in[], u8 const rk[],
int rounds, int blocks, u8 iv[]))
@@ -256,7 +282,7 @@ static int __xts_crypt(struct skcipher_request *req,
err = skcipher_walk_virt(&walk, req, true);
- __aes_arm_encrypt(ctx->twkey, ctx->key.rounds, walk.iv, walk.iv);
+ crypto_cipher_encrypt_one(ctx->tweak_tfm, walk.iv, walk.iv);
kernel_neon_begin();
while (walk.nbytes >= AES_BLOCK_SIZE) {
@@ -309,6 +335,8 @@ static struct skcipher_alg aes_algs[] = { {
.base.cra_ctxsize = sizeof(struct aesbs_cbc_ctx),
.base.cra_module = THIS_MODULE,
.base.cra_flags = CRYPTO_ALG_INTERNAL,
+ .base.cra_init = cbc_init,
+ .base.cra_exit = cbc_exit,
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
@@ -342,6 +370,8 @@ static struct skcipher_alg aes_algs[] = { {
.base.cra_ctxsize = sizeof(struct aesbs_xts_ctx),
.base.cra_module = THIS_MODULE,
.base.cra_flags = CRYPTO_ALG_INTERNAL,
+ .base.cra_init = xts_init,
+ .base.cra_exit = xts_exit,
.min_keysize = 2 * AES_MIN_KEY_SIZE,
.max_keysize = 2 * AES_MAX_KEY_SIZE,
@@ -402,5 +432,5 @@ static int __init aes_init(void)
return err;
}
-module_init(aes_init);
+late_initcall(aes_init);
module_exit(aes_exit);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/2] crypto: algapi - annotate expected branch behavior in crypto_inc()
From: Ard Biesheuvel @ 2017-02-14 21:51 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel, Jason A . Donenfeld
In-Reply-To: <1487109062-3419-1-git-send-email-ard.biesheuvel@linaro.org>
To prevent unnecessary branching, mark the exit condition of the
primary loop as likely(), given that a carry in a 32-bit counter
occurs very rarely.
On arm64, the resulting code is emitted by GCC as
9a8: cmp w1, #0x3
9ac: add x3, x0, w1, uxtw
9b0: b.ls 9e0 <crypto_inc+0x38>
9b4: ldr w2, [x3,#-4]!
9b8: rev w2, w2
9bc: add w2, w2, #0x1
9c0: rev w4, w2
9c4: str w4, [x3]
9c8: cbz w2, 9d0 <crypto_inc+0x28>
9cc: ret
where the two remaining branch conditions (one for size < 4 and one for
the carry) are statically predicted as non-taken, resulting in optimal
execution in the vast majority of cases.
Also, replace the open coded alignment test with IS_ALIGNED().
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: no change
crypto/algapi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 6b52e8f0b95f..9eed4ef9c971 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -963,11 +963,11 @@ void crypto_inc(u8 *a, unsigned int size)
u32 c;
if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ||
- !((unsigned long)b & (__alignof__(*b) - 1)))
+ IS_ALIGNED((unsigned long)b, __alignof__(*b)))
for (; size >= 4; size -= 4) {
c = be32_to_cpu(*--b) + 1;
*b = cpu_to_be32(c);
- if (c)
+ if (likely(c))
return;
}
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox