Re: [PATCH v9 4/7] Add SPAcc AEAD support

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/09/2024 11:30, Pavitrakumar M wrote:
> Add AEAD support to SPAcc driver.
> Below are the supported AEAD algos:
> - ccm(sm4)
> - ccm(aes)
> - gcm(sm4)
> - gcm(aes)
> - rfc7539(chacha20,poly1305)

...

> +
> +struct spacc_iv_buf {
> +	unsigned char iv[SPACC_MAX_IV_SIZE];
> +	unsigned char spacc_adata[ADATA_BUF_SIZE];
> +	struct scatterlist sg[2], spacc_adata_sg[2];
> +	struct scatterlist *spacc_ptextsg, temp_aad[2];
> +};
> +
> +static struct kmem_cache *spacc_iv_pool;

How do you handle multiple devices? I don't see it...

> +
> +static struct mode_tab possible_aeads[] = {
> +	{ MODE_TAB_AEAD("rfc7539(chacha20,poly1305)",
> +			CRYPTO_MODE_CHACHA20_POLY1305, CRYPTO_MODE_NULL,
> +			16, 12, 1), .keylen = { 16, 24, 32 }
> +	},
> +	{ MODE_TAB_AEAD("gcm(aes)",
> +			CRYPTO_MODE_AES_GCM, CRYPTO_MODE_NULL,
> +			16, 12, 1), .keylen = { 16, 24, 32 }
> +	},
> +	{ MODE_TAB_AEAD("gcm(sm4)",
> +			CRYPTO_MODE_SM4_GCM, CRYPTO_MODE_NULL,
> +			16, 12, 1), .keylen = { 16 }
> +	},
> +	{ MODE_TAB_AEAD("ccm(aes)",
> +			CRYPTO_MODE_AES_CCM, CRYPTO_MODE_NULL,
> +			16, 16, 1), .keylen = { 16, 24, 32 }
> +	},
> +	{ MODE_TAB_AEAD("ccm(sm4)",
> +			CRYPTO_MODE_SM4_CCM, CRYPTO_MODE_NULL,
> +			16, 16, 1), .keylen = { 16, 24, 32 }
> +	},
> +};

...

> +
> +static int spacc_register_aead(unsigned int aead_mode,
> +			       struct platform_device *spacc_pdev)
> +{
> +	int rc;
> +	struct spacc_alg *salg;
> +
> +	salg = kmalloc(sizeof(*salg), GFP_KERNEL);
> +	if (!salg)
> +		return -ENOMEM;
> +
> +	salg->mode	= &possible_aeads[aead_mode];
> +	salg->dev[0]	= &spacc_pdev->dev;
> +	salg->dev[1]	= NULL;
> +	salg->calg	= &salg->alg.aead.base;
> +	salg->alg.aead	= spacc_aead_algs;
> +
> +	spacc_init_aead_alg(salg->calg, salg->mode);
> +
> +	salg->alg.aead.ivsize		  = salg->mode->ivlen;
> +	salg->alg.aead.maxauthsize	  = salg->mode->hashlen;
> +	salg->alg.aead.base.cra_blocksize = salg->mode->blocklen;
> +
> +	salg->keylen_mask = possible_aeads[aead_mode].keylen_mask;
> +
> +	if (salg->mode->aead.ciph & SPACC_MANGLE_IV_FLAG) {
> +		switch (salg->mode->aead.ciph & 0x7F00) {
> +		case SPACC_MANGLE_IV_RFC3686: /*CTR*/
> +		case SPACC_MANGLE_IV_RFC4106: /*GCM*/
> +		case SPACC_MANGLE_IV_RFC4543: /*GMAC*/
> +		case SPACC_MANGLE_IV_RFC4309: /*CCM*/
> +		case SPACC_MANGLE_IV_RFC8998: /*GCM/CCM*/
> +			salg->alg.aead.ivsize  = 12;
> +			break;
> +		}
> +	}
> +
> +	rc = crypto_register_aead(&salg->alg.aead);
> +	if (rc < 0) {
> +		kfree(salg);
> +		return rc;
> +	}
> +
> +	dev_dbg(salg->dev[0], "Registered %s\n", salg->mode->name);

Drop, too trivial.

> +
> +	mutex_lock(&spacc_aead_alg_mutex);
> +	list_add(&salg->list, &spacc_aead_alg_list);
> +	mutex_unlock(&spacc_aead_alg_mutex);
> +
> +	return 0;
> +}
> +
> +int probe_aeads(struct platform_device *spacc_pdev)
> +{
> +	int err;
> +	unsigned int x, y;
> +	struct spacc_priv *priv = NULL;
> +
> +	size_t alloc_size = max_t(unsigned long,
> +			roundup_pow_of_two(sizeof(struct spacc_iv_buf)),
> +			dma_get_cache_alignment());
> +
> +	spacc_iv_pool = kmem_cache_create("spacc-aead-iv", alloc_size,
> +					  alloc_size, 0, NULL);
> +
> +	if (!spacc_iv_pool)
> +		return -ENOMEM;
> +
> +	for (x = 0; x < ARRAY_SIZE(possible_aeads); x++) {
> +		possible_aeads[x].keylen_mask = 0;
> +		possible_aeads[x].valid       = 0;
> +	}
> +
> +	/* compute cipher key masks (over all devices) */
> +	priv = dev_get_drvdata(&spacc_pdev->dev);
> +
> +	for (x = 0; x < ARRAY_SIZE(possible_aeads); x++) {
> +		for (y = 0; y < ARRAY_SIZE(possible_aeads[x].keylen); y++) {
> +			if (spacc_isenabled(&priv->spacc,
> +					    possible_aeads[x].aead.ciph & 0xFF,
> +					possible_aeads[x].keylen[y]))
> +				possible_aeads[x].keylen_mask |= 1u << y;
> +		}
> +	}
> +
> +	/* scan for combined modes */
> +	priv = dev_get_drvdata(&spacc_pdev->dev);
> +
> +	for (x = 0; x < ARRAY_SIZE(possible_aeads); x++) {
> +		if (!possible_aeads[x].valid && possible_aeads[x].keylen_mask) {
> +			if (spacc_isenabled(&priv->spacc,
> +					    possible_aeads[x].aead.hash & 0xFF,
> +					possible_aeads[x].hashlen)) {
> +
> +				possible_aeads[x].valid = 1;
> +				err = spacc_register_aead(x, spacc_pdev);
> +				if (err < 0)
> +					goto error;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +
> +error:
> +	return err;
> +}
> +
> +int spacc_unregister_aead_algs(void)

Why do you make it global but without headers? Or you split the change
so weirdly that header is not here?

> +{
> +	struct spacc_alg *salg, *tmp;
> +
> +	mutex_lock(&spacc_aead_alg_mutex);
> +
> +	list_for_each_entry_safe(salg, tmp, &spacc_aead_alg_list, list) {
> +		crypto_unregister_alg(salg->calg);
> +		list_del(&salg->list);
> +		kfree(salg);
> +	}
> +
> +	mutex_unlock(&spacc_aead_alg_mutex);
> +
> +	kmem_cache_destroy(spacc_iv_pool);
> +
> +	return 0;
> +}

Best regards,
Krzysztof