Re: [PATCH v7 3/3] crypto: Add Inside Secure SafeXcel EIP-93 crypto engine support

[Christian Marangi <ansuelsmth@gmail.com>]

On Tue, Dec 10, 2024 at 11:23:24AM +0800, Herbert Xu wrote:
> On Tue, Nov 12, 2024 at 02:59:00AM +0100, Christian Marangi wrote:
> >
> > +static int eip93_hash_export(struct ahash_request *req, void *out)
> > +{
> > +	struct eip93_hash_reqctx *rctx = ahash_request_ctx(req);
> > +	struct eip93_hash_export_state *state = out;
> > +	DECLARE_CRYPTO_WAIT(wait);
> > +	int ret;
> > +
> > +	crypto_init_wait(&wait);
> > +	/* Set the req callback for hash_partial_final wait */
> > +	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> > +				   crypto_req_done, &wait);
> > +
> > +	/* Set for partial hash generation */
> > +	rctx->partial_hash = true;
> > +	rctx->export_state = true;
> > +	rctx->state = state;
> > +
> > +	/* Save the first block in state data */
> > +	if (rctx->left_last || rctx->len) {
> > +		struct mkt_hash_block *block;
> > +
> > +		block = list_first_entry(&rctx->blocks,
> > +					 struct mkt_hash_block,
> > +					 list);
> > +
> > +		memcpy(state->data, block->data,
> > +		       SHA256_BLOCK_SIZE - rctx->left_last);
> > +	}
> > +
> > +	/* Call hash_partial_final.
> > +	 * This will send a dummpy 0 length packet. This is done to
> > +	 * wait for every descriptor to being handled and sync the sa_state
> > +	 * from the host. Partial hash and any other data will be copied in
> > +	 * eip93_hash_handle_result()
> > +	 */
> > +	ret = crypto_wait_req(eip93_hash_partial_final(req), &wait);
> 
> Sorry, you can't do that here.  Your hash state should have been
> exported when the request previously completed.  The export function
> must not sleep.
>

Ah that unfortunate but doesn't that goes against the async idea of ahash
OPs? Anyway is busy loop acceptable here?

The main problem here is that .update only enqueue packet to be
processed and we don't wait for it to finish as that would result in
really bad performance.

To export the state with the previous request (.update) we would have to
wait for each packet to complete or we would export the wrong partial
hash.

The process is fast enough so busy loop here should not be problematic
but waiting for every packet sent to .update on being processed might
introduce big performance regression.

Thanks for checking this hope you can help me understand this.

-- 
	Ansuel