| Message ID | 20190730054833.24192-1-horia.geanta@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam - fix concurrency issue in givencrypt descriptor | expand |
On Tue, Jul 30, 2019 at 08:48:33AM +0300, Horia Geantă wrote: > IV transfer from ofifo to class2 (set up at [29][30]) is not guaranteed > to be scheduled before the data transfer from ofifo to external memory > (set up at [38]: > > [29] 10FA0004 ld: ind-nfifo (len=4) imm > [30] 81F00010 <nfifo_entry: ofifo->class2 type=msg len=16> > [31] 14820004 ld: ccb2-datasz len=4 offs=0 imm > [32] 00000010 data:0x00000010 > [33] 8210010D operation: cls1-op aes cbc init-final enc > [34] A8080B04 math: (seqin + math0)->vseqout len=4 > [35] 28000010 seqfifold: skip len=16 > [36] A8080A04 math: (seqin + math0)->vseqin len=4 > [37] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz > [38] 69300000 seqfifostr: msg len=vseqoutsz > [39] 5C20000C seqstr: ccb2 ctx len=12 offs=0 > > If ofifo -> external memory transfer happens first, DECO will hang > (issuing a Watchdog Timeout error, if WDOG is enabled) waiting for > data availability in ofifo for the ofifo -> c2 ififo transfer. > > Make sure IV transfer happens first by waiting for all CAAM internal > transfers to end before starting payload transfer. > > New descriptor with jump command inserted at [37]: > > [..] > [36] A8080A04 math: (seqin + math0)->vseqin len=4 > [37] A1000401 jump: jsl1 all-match[!nfifopend] offset=[01] local->[38] > [38] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz > [39] 69300000 seqfifostr: msg len=vseqoutsz > [40] 5C20000C seqstr: ccb2 ctx len=12 offs=0 > > [Note: the issue is present in the descriptor from the very beginning > (cf. Fixes tag). However I've marked it v4.19+ since it's the oldest > maintained kernel that the patch applies clean against.] > > Cc: <stable@vger.kernel.org> # v4.19+ > Fixes: 1acebad3d8db8 ("crypto: caam - faster aead implementation") > Signed-off-by: Horia Geantă <horia.geanta@nxp.com> > --- > drivers/crypto/caam/caamalg_desc.c | 9 +++++++++ > drivers/crypto/caam/caamalg_desc.h | 2 +- > 2 files changed, 10 insertions(+), 1 deletion(-) Patch applied. Thanks.
On Tue, Jul 30, 2019 at 08:48:33AM +0300, Horia Geantă wrote: > IV transfer from ofifo to class2 (set up at [29][30]) is not guaranteed > to be scheduled before the data transfer from ofifo to external memory > (set up at [38]: > > [29] 10FA0004 ld: ind-nfifo (len=4) imm > [30] 81F00010 <nfifo_entry: ofifo->class2 type=msg len=16> > [31] 14820004 ld: ccb2-datasz len=4 offs=0 imm > [32] 00000010 data:0x00000010 > [33] 8210010D operation: cls1-op aes cbc init-final enc > [34] A8080B04 math: (seqin + math0)->vseqout len=4 > [35] 28000010 seqfifold: skip len=16 > [36] A8080A04 math: (seqin + math0)->vseqin len=4 > [37] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz > [38] 69300000 seqfifostr: msg len=vseqoutsz > [39] 5C20000C seqstr: ccb2 ctx len=12 offs=0 > > If ofifo -> external memory transfer happens first, DECO will hang > (issuing a Watchdog Timeout error, if WDOG is enabled) waiting for > data availability in ofifo for the ofifo -> c2 ififo transfer. > > Make sure IV transfer happens first by waiting for all CAAM internal > transfers to end before starting payload transfer. > > New descriptor with jump command inserted at [37]: > > [..] > [36] A8080A04 math: (seqin + math0)->vseqin len=4 > [37] A1000401 jump: jsl1 all-match[!nfifopend] offset=[01] local->[38] > [38] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz > [39] 69300000 seqfifostr: msg len=vseqoutsz > [40] 5C20000C seqstr: ccb2 ctx len=12 offs=0 > > [Note: the issue is present in the descriptor from the very beginning > (cf. Fixes tag). However I've marked it v4.19+ since it's the oldest > maintained kernel that the patch applies clean against.] > > Cc: <stable@vger.kernel.org> # v4.19+ > Fixes: 1acebad3d8db8 ("crypto: caam - faster aead implementation") > Signed-off-by: Horia Geantă <horia.geanta@nxp.com> > --- > drivers/crypto/caam/caamalg_desc.c | 9 +++++++++ > drivers/crypto/caam/caamalg_desc.h | 2 +- > 2 files changed, 10 insertions(+), 1 deletion(-) Patch applied. Thanks.
diff --git a/drivers/crypto/caam/caamalg_desc.c b/drivers/crypto/caam/caamalg_desc.c index 72531837571e..28ecef7a481c 100644 --- a/drivers/crypto/caam/caamalg_desc.c +++ b/drivers/crypto/caam/caamalg_desc.c @@ -503,6 +503,7 @@ void cnstr_shdsc_aead_givencap(u32 * const desc, struct alginfo *cdata, const bool is_qi, int era) { u32 geniv, moveiv; + u32 *wait_cmd; /* Note: Context registers are saved. */ init_sh_desc_key_aead(desc, cdata, adata, is_rfc3686, nonce, era); @@ -598,6 +599,14 @@ void cnstr_shdsc_aead_givencap(u32 * const desc, struct alginfo *cdata, /* Will read cryptlen */ append_math_add(desc, VARSEQINLEN, SEQINLEN, REG0, CAAM_CMD_SZ); + + /* + * Wait for IV transfer (ofifo -> class2) to finish before starting + * ciphertext transfer (ofifo -> external memory). + */ + wait_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL | JUMP_COND_NIFP); + set_jump_tgt_here(desc, wait_cmd); + append_seq_fifo_load(desc, 0, FIFOLD_CLASS_BOTH | KEY_VLF | FIFOLD_TYPE_MSG1OUT2 | FIFOLD_TYPE_LASTBOTH); append_seq_fifo_store(desc, 0, FIFOST_TYPE_MESSAGE_DATA | KEY_VLF); diff --git a/drivers/crypto/caam/caamalg_desc.h b/drivers/crypto/caam/caamalg_desc.h index da4a4ee60c80..706007624d82 100644 --- a/drivers/crypto/caam/caamalg_desc.h +++ b/drivers/crypto/caam/caamalg_desc.h @@ -12,7 +12,7 @@ #define DESC_AEAD_BASE (4 * CAAM_CMD_SZ) #define DESC_AEAD_ENC_LEN (DESC_AEAD_BASE + 11 * CAAM_CMD_SZ) #define DESC_AEAD_DEC_LEN (DESC_AEAD_BASE + 15 * CAAM_CMD_SZ) -#define DESC_AEAD_GIVENC_LEN (DESC_AEAD_ENC_LEN + 7 * CAAM_CMD_SZ) +#define DESC_AEAD_GIVENC_LEN (DESC_AEAD_ENC_LEN + 8 * CAAM_CMD_SZ) #define DESC_QI_AEAD_ENC_LEN (DESC_AEAD_ENC_LEN + 3 * CAAM_CMD_SZ) #define DESC_QI_AEAD_DEC_LEN (DESC_AEAD_DEC_LEN + 3 * CAAM_CMD_SZ) #define DESC_QI_AEAD_GIVENC_LEN (DESC_AEAD_GIVENC_LEN + 3 * CAAM_CMD_SZ)
IV transfer from ofifo to class2 (set up at [29][30]) is not guaranteed to be scheduled before the data transfer from ofifo to external memory (set up at [38]: [29] 10FA0004 ld: ind-nfifo (len=4) imm [30] 81F00010 <nfifo_entry: ofifo->class2 type=msg len=16> [31] 14820004 ld: ccb2-datasz len=4 offs=0 imm [32] 00000010 data:0x00000010 [33] 8210010D operation: cls1-op aes cbc init-final enc [34] A8080B04 math: (seqin + math0)->vseqout len=4 [35] 28000010 seqfifold: skip len=16 [36] A8080A04 math: (seqin + math0)->vseqin len=4 [37] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz [38] 69300000 seqfifostr: msg len=vseqoutsz [39] 5C20000C seqstr: ccb2 ctx len=12 offs=0 If ofifo -> external memory transfer happens first, DECO will hang (issuing a Watchdog Timeout error, if WDOG is enabled) waiting for data availability in ofifo for the ofifo -> c2 ififo transfer. Make sure IV transfer happens first by waiting for all CAAM internal transfers to end before starting payload transfer. New descriptor with jump command inserted at [37]: [..] [36] A8080A04 math: (seqin + math0)->vseqin len=4 [37] A1000401 jump: jsl1 all-match[!nfifopend] offset=[01] local->[38] [38] 2F1E0000 seqfifold: both msg1->2-last2-last1 len=vseqinsz [39] 69300000 seqfifostr: msg len=vseqoutsz [40] 5C20000C seqstr: ccb2 ctx len=12 offs=0 [Note: the issue is present in the descriptor from the very beginning (cf. Fixes tag). However I've marked it v4.19+ since it's the oldest maintained kernel that the patch applies clean against.] Cc: <stable@vger.kernel.org> # v4.19+ Fixes: 1acebad3d8db8 ("crypto: caam - faster aead implementation") Signed-off-by: Horia Geantă <horia.geanta@nxp.com> --- drivers/crypto/caam/caamalg_desc.c | 9 +++++++++ drivers/crypto/caam/caamalg_desc.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-)