On 11/03/2015 02:36 PM, LABBE Corentin wrote: > On Tue, Nov 03, 2015 at 12:39:57PM +0200, Stanimir Varbanov wrote: >> Hi, >> >> I know that this patch has been queued up, but ... >> >> On 10/02/2015 09:01 AM, LABBE Corentin wrote: >>> The qce driver use two dma_map_sg path according to SG are chained >>> or not. >>> Since dma_map_sg can handle both case, clean the code with all >>> references to sg chained. >>> >>> Thus removing qce_mapsg, qce_unmapsg and qce_countsg functions. >>> >>> Signed-off-by: LABBE Corentin <clabbe.montjoie@xxxxxxxxx> >>> --- >>> drivers/crypto/qce/ablkcipher.c | 30 ++++++++---------------- >>> drivers/crypto/qce/cipher.h | 4 ---- >>> drivers/crypto/qce/dma.c | 52 ----------------------------------------- >>> drivers/crypto/qce/dma.h | 5 ---- >>> drivers/crypto/qce/sha.c | 18 ++++++-------- >>> drivers/crypto/qce/sha.h | 2 -- >>> 6 files changed, 17 insertions(+), 94 deletions(-) >>> >> >> <snip> >> >>> diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c >>> index be2f504..0c9973e 100644 >>> --- a/drivers/crypto/qce/sha.c >>> +++ b/drivers/crypto/qce/sha.c >>> @@ -51,9 +51,8 @@ static void qce_ahash_done(void *data) >>> if (error) >>> dev_dbg(qce->dev, "ahash dma termination error (%d)\n", error); >>> >>> - qce_unmapsg(qce->dev, req->src, rctx->src_nents, DMA_TO_DEVICE, >>> - rctx->src_chained); >>> - qce_unmapsg(qce->dev, &rctx->result_sg, 1, DMA_FROM_DEVICE, 0); >>> + dma_unmap_sg(qce->dev, req->src, rctx->src_nents, DMA_TO_DEVICE); >>> + dma_unmap_sg(qce->dev, &rctx->result_sg, 1, DMA_FROM_DEVICE); >>> >>> memcpy(rctx->digest, result->auth_iv, digestsize); >>> if (req->result) >>> @@ -92,16 +91,14 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req) >>> rctx->authklen = AES_KEYSIZE_128; >>> } >>> >>> - rctx->src_nents = qce_countsg(req->src, req->nbytes, >>> - &rctx->src_chained); >>> - ret = qce_mapsg(qce->dev, req->src, rctx->src_nents, DMA_TO_DEVICE, >>> - rctx->src_chained); >>> + rctx->src_nents = sg_nents_for_len(req->src, req->nbytes); >> >> sg_nents_for_len can return -EINVAL, and this error should be handled. >> >> After added a check for the error I'm observing below: >> >> #insmod tcrypt.ko mode=403 (test_ahash_speed("sha1")) >> >> test 21 ( 8192 byte blocks, 8192 bytes per update, 1 updates): >> qcrypto 73a000.crypto: sg_nents_for_len failed (-22) (nbytes:8192, >> sg_len:4096) >> hashing failed ret=-22 >> >> It seems that something is wrong with the test case? Looking further in >> test_hash_sg_init() we can see: >> >> #define TVMEMSIZE 4 >> >> sg_init_table(sg, TVMEMSIZE); >> for (i = 0; i < TVMEMSIZE; i++) { >> sg_set_buf(sg + i, tvmem[i], PAGE_SIZE); >> memset(tvmem[i], 0xff, PAGE_SIZE); >> } >> >> so we have 4 SGs with sg->length = 4096, thus sg_nents_for_len() should >> return 2 entries with 4096 bytes each. >> >> What is wrong here? >> > > Hello > > It is a shame that I forgot to check the return value. I guess that you expected dma_map_sg() to handle the error, and infact it does - trow out BUG_ON :)) More interesting thing to me is how the above test case is expected to work without sg chaining. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html