Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCHv4 3/7] crypto: sa2ul: add sha1/sha256/sha512 support
To:     Herbert Xu <herbert@gondor.apana.org.au>
CC:     <davem@davemloft.net>, <linux-crypto@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <j-keerthy@ti.com>
References: <20200615071452.25141-1-t-kristo@ti.com>
 <20200615071452.25141-4-t-kristo@ti.com>
 <20200626043155.GA2683@gondor.apana.org.au>
From:   Tero Kristo <t-kristo@ti.com>
Message-ID: <2a89ea86-3b9e-06b5-fa8e-9dc6e5ad9aeb@ti.com>
Date:   Fri, 26 Jun 2020 12:15:42 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <20200626043155.GA2683@gondor.apana.org.au>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

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