Re: [PATCH v25 2/4] crypto: spacc - Add SPAcc ahash support

[Herbert Xu]

On Mon, Sep 07, 2026 at 09:29:14PM +0530, Pavitrakumar Managutte wrote:
>
> +static int spacc_hash_init(struct ahash_request *req)
> +{
> +	int rc;
> +	struct crypto_ahash *tfm        = crypto_ahash_reqtfm(req);
> +	struct spacc_crypto_ctx *tctx   = crypto_ahash_ctx(tfm);
> +	struct spacc_crypto_reqctx *ctx = ahash_request_ctx(req);
> +
> +	memset(ctx->state_buffer, 0, sizeof(ctx->state_buffer));
> +
> +	if (tctx->shash_fb) {
> +		SHASH_DESC_ON_STACK(desc, tctx->shash_fb);
> +
> +		desc->tfm = tctx->shash_fb;
> +		rc = crypto_shash_init(desc);
> +		if (!rc)
> +			rc = crypto_shash_export(desc, ctx->state_buffer);
> +		shash_desc_zero(desc);
> +	} else {
> +		HASH_FBREQ_ON_STACK(fbreq, req);
> +
> +		rc = crypto_ahash_init(fbreq);
> +		if (!rc)
> +			rc = crypto_ahash_export(fbreq, ctx->state_buffer);
> +		HASH_REQUEST_ZERO(fbreq);
> +	}

Why do we need two fallback paths? It would appear that you can
just use the else clause in all cases.

> +static const struct ahash_engine_alg spacc_hash_template = {
> +	.base = {
> +		.init     = spacc_hash_init,
> +		.update   = spacc_hash_update,
> +		.final    = spacc_hash_final,
> +		.finup    = spacc_hash_finup,
> +		.digest   = spacc_hash_digest,
> +		.setkey   = spacc_hash_setkey,
> +		.export   = spacc_hash_export,
> +		.import   = spacc_hash_import,
> +		.init_tfm = spacc_hash_init_tfm,
> +		.exit_tfm = spacc_hash_exit_tfm,
> +
> +		.halg.base = {
> +			.cra_priority	= 300,
> +			.cra_module	= THIS_MODULE,
> +			.cra_ctxsize	= sizeof(struct spacc_crypto_ctx),
> +			.cra_reqsize	= sizeof(struct spacc_crypto_reqctx),
> +			.cra_destroy	= spacc_hash_alg_destroy,
> +			.cra_flags	= CRYPTO_ALG_TYPE_AHASH    |
> +					  CRYPTO_ALG_ASYNC

Please use BLOCK_ONLY for all new drivers.  See aspeed for an
example on how it works.

> +	/*
> +	 * CMAC/XCBC-AES have a dedicated shash fallback (allocated in
> +	 * init_tfm).  They must not advertise CRYPTO_ALG_OPTIONAL_KEY or
> +	 * the crypto API may try to use the generic ahash fallback for them,
> +	 * which would recurse back into this driver.
> +	 */

How can it recurse back into the driver? The fallback allocated
by the API is always synchronous so unless your driver is sync
(which it is not) it cannot recurse.

> +		salg->alg.hash.base.halg.base.cra_flags =
> +			CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_ASYNC |
> +			CRYPTO_ALG_NEED_FALLBACK;

There is no need to set NEED_FALLBACK as the API now allocates
fallbacks for all async drivers.

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt