Re: [PATCHv4 3/7] crypto: sa2ul: add sha1/sha256/sha512 support

[Tero Kristo <t-kristo@ti.com>]

On 26/06/2020 07:31, Herbert Xu wrote:
> On Mon, Jun 15, 2020 at 10:14:48AM +0300, Tero Kristo wrote:
>>
>> +static int sa_sha_update(struct ahash_request *req)
>> +{
>> +	struct sa_sha_req_ctx *rctx = ahash_request_ctx(req);
>> +	struct scatterlist *sg;
>> +	void *buf;
>> +	int pages;
>> +	struct page *pg;
>> +
>> +	if (!req->nbytes)
>> +		return 0;
>> +
>> +	if (rctx->buf_free >= req->nbytes) {
>> +		pg = sg_page(rctx->sg_next);
>> +		buf = kmap_atomic(pg);
>> +		scatterwalk_map_and_copy(buf + rctx->offset, req->src, 0,
>> +					 req->nbytes, 0);
>> +		kunmap_atomic(buf);
>> +		rctx->buf_free -= req->nbytes;
>> +		rctx->sg_next->length += req->nbytes;
>> +		rctx->offset += req->nbytes;
>> +	} else {
>> +		pages = get_order(req->nbytes);
>> +		buf = (void *)__get_free_pages(GFP_ATOMIC, pages);
>> +		if (!buf)
>> +			return -ENOMEM;
>> +
>> +		sg = kzalloc(sizeof(*sg) * 2, GFP_KERNEL);
>> +		if (!sg)
>> +			return -ENOMEM;
>> +
>> +		sg_init_table(sg, 1);
>> +		sg_set_buf(sg, buf, req->nbytes);
>> +		scatterwalk_map_and_copy(buf, req->src, 0, req->nbytes, 0);
>> +
>> +		rctx->buf_free = (PAGE_SIZE << pages) - req->nbytes;
>> +
>> +		if (rctx->sg_next) {
>> +			sg_unmark_end(rctx->sg_next);
>> +			sg_chain(rctx->sg_next, 2, sg);
>> +		} else {
>> +			rctx->src = sg;
>> +		}
>> +
>> +		rctx->sg_next = sg;
>> +		rctx->src_nents++;
>> +
>> +		rctx->offset = req->nbytes;
>> +	}
>> +
>> +	rctx->len += req->nbytes;
>> +
>> +	return 0;
>> +}
> 
> This is not how it's supposed to work.  To support the partial
> hashing interface, you must actually hash the data and not just
> save it in your context.  Otherwise your export is completely
> meaningless.

I have been experimenting with an alternate approach, where I have a 
small buffer within the context, this would be more like the way other 
drivers do this. If the buffer is closed before running out of space, I 
can push this to be processed by HW, otherwise I must fallback to SW. 
Does this sound like a better approach?

> If your hardware cannot export partially hashed state, then you
> should use a software fallback for everything but digest.

Yea, HW can't support partial hashes.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki