Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Herbert Xu 
On Tue, Nov 03, 2015 at 01:36:55PM +0100, LABBE Corentin wrote:
>
> Herbert, do you prefer to drop the patch series and I send an updated one, or 
> does I will send a new patch series for checking the return value of 
> sg_nents_for_len() ?

Your patch has already been applied so please send any fixes on
top of it.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Stanimir Varbanov 
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 
>>> ---
>>>  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(-)
>>>
>>
>> 
>>
>>> 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, >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, >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,
>>> - >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 TVMEMSIZE4
>>
>>  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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread LABBE Corentin 
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 
> > ---
> >  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(-)
> > 
> 
> 
> 
> > 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, >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, >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,
> > - >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.
Herbert, do you prefer to drop the patch series and I send an updated one, or 
does I will send a new patch series for checking the return value of 
sg_nents_for_len() ?

Regards

LABBE Corentin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Stanimir Varbanov 
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 
> ---
>  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(-)
> 



> 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, >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, >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,
> -   >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?

> + ret = dma_map_sg(qce->dev, req->src, rctx->src_nents, DMA_TO_DEVICE);
>   if (ret < 0)
>   return ret;
>  
>   sg_init_one(>result_sg, qce->dma.result_buf, QCE_RESULT_BUF_SZ);
>  
> - ret = qce_mapsg(qce->dev, >result_sg, 1, DMA_FROM_DEVICE, 0);
> + ret = dma_map_sg(qce->dev, >result_sg, 1, DMA_FROM_DEVICE);
>   if (ret < 0)
>   goto error_unmap_src;
>  




-- 
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread LABBE Corentin 
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 
> > ---
> >  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(-)
> > 
> 
> 
> 
> > 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, >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, >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,
> > - >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.
Herbert, do you prefer to drop the patch series and I send an updated one, or 
does I will send a new patch series for checking the return value of 
sg_nents_for_len() ?

Regards

LABBE Corentin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Stanimir Varbanov 
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 
> ---
>  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(-)
> 



> 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, >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, >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,
> -   >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?

> + ret = dma_map_sg(qce->dev, req->src, rctx->src_nents, DMA_TO_DEVICE);
>   if (ret < 0)
>   return ret;
>  
>   sg_init_one(>result_sg, qce->dma.result_buf, QCE_RESULT_BUF_SZ);
>  
> - ret = qce_mapsg(qce->dev, >result_sg, 1, DMA_FROM_DEVICE, 0);
> + ret = dma_map_sg(qce->dev, >result_sg, 1, DMA_FROM_DEVICE);
>   if (ret < 0)
>   goto error_unmap_src;
>  




-- 
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Herbert Xu 
On Tue, Nov 03, 2015 at 01:36:55PM +0100, LABBE Corentin wrote:
>
> Herbert, do you prefer to drop the patch series and I send an updated one, or 
> does I will send a new patch series for checking the return value of 
> sg_nents_for_len() ?

Your patch has already been applied so please send any fixes on
top of it.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-11-03 Thread Stanimir Varbanov 
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 
>>> ---
>>>  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(-)
>>>
>>
>> 
>>
>>> 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, >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, >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,
>>> - >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 TVMEMSIZE4
>>
>>  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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-10-08 Thread Herbert Xu 
On Fri, Oct 02, 2015 at 08:01:02AM +0200, 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 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-10-08 Thread Herbert Xu 
On Fri, Oct 02, 2015 at 08:01:02AM +0200, 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 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-10-02 Thread LABBE Corentin 
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 
---
 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(-)

diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..2c0d63d 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -44,10 +44,8 @@ static void qce_ablkcipher_done(void *data)
error);
 
if (diff_dst)
-   qce_unmapsg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src,
-   rctx->dst_chained);
-   qce_unmapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   dma_unmap_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src);
+   dma_unmap_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
 
sg_free_table(>dst_tbl);
 
@@ -80,15 +78,11 @@ qce_ablkcipher_async_req_handle(struct crypto_async_request 
*async_req)
dir_src = diff_dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
dir_dst = diff_dst ? DMA_FROM_DEVICE : DMA_BIDIRECTIONAL;
 
-   rctx->src_nents = qce_countsg(req->src, req->nbytes,
- >src_chained);
-   if (diff_dst) {
-   rctx->dst_nents = qce_countsg(req->dst, req->nbytes,
- >dst_chained);
-   } else {
+   rctx->src_nents = sg_nents_for_len(req->src, req->nbytes);
+   if (diff_dst)
+   rctx->dst_nents = sg_nents_for_len(req->dst, req->nbytes);
+   else
rctx->dst_nents = rctx->src_nents;
-   rctx->dst_chained = rctx->src_chained;
-   }
 
rctx->dst_nents += 1;
 
@@ -116,14 +110,12 @@ qce_ablkcipher_async_req_handle(struct 
crypto_async_request *async_req)
sg_mark_end(sg);
rctx->dst_sg = rctx->dst_tbl.sgl;
 
-   ret = qce_mapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   ret = dma_map_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
if (ret < 0)
goto error_free;
 
if (diff_dst) {
-   ret = qce_mapsg(qce->dev, req->src, rctx->src_nents, dir_src,
-   rctx->src_chained);
+   ret = dma_map_sg(qce->dev, req->src, rctx->src_nents, dir_src);
if (ret < 0)
goto error_unmap_dst;
rctx->src_sg = req->src;
@@ -149,11 +141,9 @@ error_terminate:
qce_dma_terminate_all(>dma);
 error_unmap_src:
if (diff_dst)
-   qce_unmapsg(qce->dev, req->src, rctx->src_nents, dir_src,
-   rctx->src_chained);
+   dma_unmap_sg(qce->dev, req->src, rctx->src_nents, dir_src);
 error_unmap_dst:
-   qce_unmapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   dma_unmap_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
 error_free:
sg_free_table(>dst_tbl);
return ret;
diff --git a/drivers/crypto/qce/cipher.h b/drivers/crypto/qce/cipher.h
index d5757cf..5c6a5f8 100644
--- a/drivers/crypto/qce/cipher.h
+++ b/drivers/crypto/qce/cipher.h
@@ -32,8 +32,6 @@ struct qce_cipher_ctx {
  * @ivsize: IV size
  * @src_nents: source entries
  * @dst_nents: destination entries
- * @src_chained: is source chained
- * @dst_chained: is destination chained
  * @result_sg: scatterlist used for result buffer
  * @dst_tbl: destination sg table
  * @dst_sg: destination sg pointer table beginning
@@ -47,8 +45,6 @@ struct qce_cipher_reqctx {
unsigned int ivsize;
int src_nents;
int dst_nents;
-   bool src_chained;
-   bool dst_chained;
struct scatterlist result_sg;
struct sg_table dst_tbl;
struct scatterlist *dst_sg;
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 378cb76..4797e79 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -54,58 +54,6 @@ void qce_dma_release(struct qce_dma_data *dma)
kfree(dma->result_buf);
 }
 
-int qce_mapsg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir, bool chained)
-{
-   int err;
-
-   if (chained) {
-   while (sg) {
-   err = dma_map_sg(dev, sg, 1, dir);
-   if (!err)
-   return -EFAULT;
-   sg = sg_next(sg);
-

[PATCH] crypto: qce: dma_map_sg can handle chained SG

2015-10-02 Thread LABBE Corentin 
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 
---
 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(-)

diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..2c0d63d 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -44,10 +44,8 @@ static void qce_ablkcipher_done(void *data)
error);
 
if (diff_dst)
-   qce_unmapsg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src,
-   rctx->dst_chained);
-   qce_unmapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   dma_unmap_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src);
+   dma_unmap_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
 
sg_free_table(>dst_tbl);
 
@@ -80,15 +78,11 @@ qce_ablkcipher_async_req_handle(struct crypto_async_request 
*async_req)
dir_src = diff_dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
dir_dst = diff_dst ? DMA_FROM_DEVICE : DMA_BIDIRECTIONAL;
 
-   rctx->src_nents = qce_countsg(req->src, req->nbytes,
- >src_chained);
-   if (diff_dst) {
-   rctx->dst_nents = qce_countsg(req->dst, req->nbytes,
- >dst_chained);
-   } else {
+   rctx->src_nents = sg_nents_for_len(req->src, req->nbytes);
+   if (diff_dst)
+   rctx->dst_nents = sg_nents_for_len(req->dst, req->nbytes);
+   else
rctx->dst_nents = rctx->src_nents;
-   rctx->dst_chained = rctx->src_chained;
-   }
 
rctx->dst_nents += 1;
 
@@ -116,14 +110,12 @@ qce_ablkcipher_async_req_handle(struct 
crypto_async_request *async_req)
sg_mark_end(sg);
rctx->dst_sg = rctx->dst_tbl.sgl;
 
-   ret = qce_mapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   ret = dma_map_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
if (ret < 0)
goto error_free;
 
if (diff_dst) {
-   ret = qce_mapsg(qce->dev, req->src, rctx->src_nents, dir_src,
-   rctx->src_chained);
+   ret = dma_map_sg(qce->dev, req->src, rctx->src_nents, dir_src);
if (ret < 0)
goto error_unmap_dst;
rctx->src_sg = req->src;
@@ -149,11 +141,9 @@ error_terminate:
qce_dma_terminate_all(>dma);
 error_unmap_src:
if (diff_dst)
-   qce_unmapsg(qce->dev, req->src, rctx->src_nents, dir_src,
-   rctx->src_chained);
+   dma_unmap_sg(qce->dev, req->src, rctx->src_nents, dir_src);
 error_unmap_dst:
-   qce_unmapsg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst,
-   rctx->dst_chained);
+   dma_unmap_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
 error_free:
sg_free_table(>dst_tbl);
return ret;
diff --git a/drivers/crypto/qce/cipher.h b/drivers/crypto/qce/cipher.h
index d5757cf..5c6a5f8 100644
--- a/drivers/crypto/qce/cipher.h
+++ b/drivers/crypto/qce/cipher.h
@@ -32,8 +32,6 @@ struct qce_cipher_ctx {
  * @ivsize: IV size
  * @src_nents: source entries
  * @dst_nents: destination entries
- * @src_chained: is source chained
- * @dst_chained: is destination chained
  * @result_sg: scatterlist used for result buffer
  * @dst_tbl: destination sg table
  * @dst_sg: destination sg pointer table beginning
@@ -47,8 +45,6 @@ struct qce_cipher_reqctx {
unsigned int ivsize;
int src_nents;
int dst_nents;
-   bool src_chained;
-   bool dst_chained;
struct scatterlist result_sg;
struct sg_table dst_tbl;
struct scatterlist *dst_sg;
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 378cb76..4797e79 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -54,58 +54,6 @@ void qce_dma_release(struct qce_dma_data *dma)
kfree(dma->result_buf);
 }
 
-int qce_mapsg(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir, bool chained)
-{
-   int err;
-
-   if (chained) {
-   while (sg) {
-   err = dma_map_sg(dev, sg, 1, dir);
-   if (!err)
-   return -EFAULT;
-   sg