| Message ID | 20240116094405.744466-1-gaurav.jain@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam/hash - fix asynchronous hash | expand |
On Tue, Jan 16, 2024 at 03:14:05PM +0530, Gaurav Jain wrote: > ahash_alg->setkey is updated to ahash_nosetkey in ahash.c > so updating the handling of setkey in caam driver. > > Fixes: 2f1f34c1bf7b ("crypto: ahash - optimize performance when wrapping shash") > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 4 ++-- > drivers/crypto/caam/caamhash.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c > index a148ff1f0872..93a400e286b4 100644 > --- a/drivers/crypto/caam/caamalg_qi2.c > +++ b/drivers/crypto/caam/caamalg_qi2.c > @@ -4571,7 +4571,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) > > ctx->dev = caam_hash->dev; > > - if (alg->setkey) { > + if (crypto_hash_alg_has_setkey(halg)) { > ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key, > ARRAY_SIZE(ctx->key), > DMA_TO_DEVICE, > @@ -4611,7 +4611,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) > * For keyed hash algorithms shared descriptors > * will be created later in setkey() callback > */ > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > + return crypto_hash_alg_has_setkey(halg) ? 0 : ahash_set_sh_desc(ahash); > } > > static void caam_hash_cra_exit(struct crypto_tfm *tfm) > diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c > index 290c8500c247..4d50356b593c 100644 > --- a/drivers/crypto/caam/caamhash.c > +++ b/drivers/crypto/caam/caamhash.c > @@ -1804,7 +1804,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) > } else { > if (priv->era >= 6) { > ctx->dir = DMA_BIDIRECTIONAL; > - ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE; > + ctx->key_dir = crypto_hash_alg_has_setkey(halg) ? DMA_TO_DEVICE : DMA_NONE; > } else { > ctx->dir = DMA_TO_DEVICE; > ctx->key_dir = DMA_NONE; > @@ -1862,7 +1862,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) > * For keyed hash algorithms shared descriptors > * will be created later in setkey() callback > */ > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > + return crypto_hash_alg_has_setkey(halg) ? 0 : ahash_set_sh_desc(ahash); > } > Thanks. Did you also consider putting something in struct caam_hash_alg (the struct in which this driver embeds its ahash_alg structure) that indicates whether the algorithm is an HMAC or not? Other drivers use that solution. - Eric
> -----Original Message----- > From: Eric Biggers <ebiggers@kernel.org> > Sent: Wednesday, January 17, 2024 10:03 AM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > <davem@davemloft.net>; Horia Geanta <horia.geanta@nxp.com>; Pankaj > Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > Aggarwal <meenakshi.aggarwal@nxp.com>; Aisheng Dong > <aisheng.dong@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux- > imx@nxp.com> > Subject: [EXT] Re: [PATCH] crypto: caam/hash - fix asynchronous hash > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > On Tue, Jan 16, 2024 at 03:14:05PM +0530, Gaurav Jain wrote: > > ahash_alg->setkey is updated to ahash_nosetkey in ahash.c so updating > > the handling of setkey in caam driver. > > > > Fixes: 2f1f34c1bf7b ("crypto: ahash - optimize performance when > > wrapping shash") > > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > > --- > > drivers/crypto/caam/caamalg_qi2.c | 4 ++-- > > drivers/crypto/caam/caamhash.c | 4 ++-- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/crypto/caam/caamalg_qi2.c > > b/drivers/crypto/caam/caamalg_qi2.c > > index a148ff1f0872..93a400e286b4 100644 > > --- a/drivers/crypto/caam/caamalg_qi2.c > > +++ b/drivers/crypto/caam/caamalg_qi2.c > > @@ -4571,7 +4571,7 @@ static int caam_hash_cra_init(struct crypto_tfm > > *tfm) > > > > ctx->dev = caam_hash->dev; > > > > - if (alg->setkey) { > > + if (crypto_hash_alg_has_setkey(halg)) { > > ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key, > > ARRAY_SIZE(ctx->key), > > DMA_TO_DEVICE, > > @@ -4611,7 +4611,7 @@ static int caam_hash_cra_init(struct crypto_tfm > *tfm) > > * For keyed hash algorithms shared descriptors > > * will be created later in setkey() callback > > */ > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > + ahash_set_sh_desc(ahash); > > } > > > > static void caam_hash_cra_exit(struct crypto_tfm *tfm) diff --git > > a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c > > index 290c8500c247..4d50356b593c 100644 > > --- a/drivers/crypto/caam/caamhash.c > > +++ b/drivers/crypto/caam/caamhash.c > > @@ -1804,7 +1804,7 @@ static int caam_hash_cra_init(struct crypto_tfm > *tfm) > > } else { > > if (priv->era >= 6) { > > ctx->dir = DMA_BIDIRECTIONAL; > > - ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE; > > + ctx->key_dir = crypto_hash_alg_has_setkey(halg) > > + ? DMA_TO_DEVICE : DMA_NONE; > > } else { > > ctx->dir = DMA_TO_DEVICE; > > ctx->key_dir = DMA_NONE; @@ -1862,7 +1862,7 @@ > > static int caam_hash_cra_init(struct crypto_tfm *tfm) > > * For keyed hash algorithms shared descriptors > > * will be created later in setkey() callback > > */ > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > + ahash_set_sh_desc(ahash); > > } > > > > Thanks. Did you also consider putting something in struct caam_hash_alg (the > struct in which this driver embeds its ahash_alg structure) that indicates whether > the algorithm is an HMAC or not? Other drivers use that solution. Crypto/ahash.c has this API to check the setkey so I used to differentiate between HMAC & only hash. Let me know if this change is not sufficient, will add the flag in caam_hash_alg. Gaurav > > - Eric
On Wed, Jan 17, 2024 at 10:48:39AM +0000, Gaurav Jain wrote: > > > > -----Original Message----- > > From: Eric Biggers <ebiggers@kernel.org> > > Sent: Wednesday, January 17, 2024 10:03 AM > > To: Gaurav Jain <gaurav.jain@nxp.com> > > Cc: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > > <davem@davemloft.net>; Horia Geanta <horia.geanta@nxp.com>; Pankaj > > Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > > Aggarwal <meenakshi.aggarwal@nxp.com>; Aisheng Dong > > <aisheng.dong@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>; linux- > > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux- > > imx@nxp.com> > > Subject: [EXT] Re: [PATCH] crypto: caam/hash - fix asynchronous hash > > > > Caution: This is an external email. Please take care when clicking links or > > opening attachments. When in doubt, report the message using the 'Report this > > email' button > > > > > > On Tue, Jan 16, 2024 at 03:14:05PM +0530, Gaurav Jain wrote: > > > ahash_alg->setkey is updated to ahash_nosetkey in ahash.c so updating > > > the handling of setkey in caam driver. > > > > > > Fixes: 2f1f34c1bf7b ("crypto: ahash - optimize performance when > > > wrapping shash") > > > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > > > --- > > > drivers/crypto/caam/caamalg_qi2.c | 4 ++-- > > > drivers/crypto/caam/caamhash.c | 4 ++-- > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/crypto/caam/caamalg_qi2.c > > > b/drivers/crypto/caam/caamalg_qi2.c > > > index a148ff1f0872..93a400e286b4 100644 > > > --- a/drivers/crypto/caam/caamalg_qi2.c > > > +++ b/drivers/crypto/caam/caamalg_qi2.c > > > @@ -4571,7 +4571,7 @@ static int caam_hash_cra_init(struct crypto_tfm > > > *tfm) > > > > > > ctx->dev = caam_hash->dev; > > > > > > - if (alg->setkey) { > > > + if (crypto_hash_alg_has_setkey(halg)) { > > > ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key, > > > ARRAY_SIZE(ctx->key), > > > DMA_TO_DEVICE, > > > @@ -4611,7 +4611,7 @@ static int caam_hash_cra_init(struct crypto_tfm > > *tfm) > > > * For keyed hash algorithms shared descriptors > > > * will be created later in setkey() callback > > > */ > > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > > + ahash_set_sh_desc(ahash); > > > } > > > > > > static void caam_hash_cra_exit(struct crypto_tfm *tfm) diff --git > > > a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c > > > index 290c8500c247..4d50356b593c 100644 > > > --- a/drivers/crypto/caam/caamhash.c > > > +++ b/drivers/crypto/caam/caamhash.c > > > @@ -1804,7 +1804,7 @@ static int caam_hash_cra_init(struct crypto_tfm > > *tfm) > > > } else { > > > if (priv->era >= 6) { > > > ctx->dir = DMA_BIDIRECTIONAL; > > > - ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE; > > > + ctx->key_dir = crypto_hash_alg_has_setkey(halg) > > > + ? DMA_TO_DEVICE : DMA_NONE; > > > } else { > > > ctx->dir = DMA_TO_DEVICE; > > > ctx->key_dir = DMA_NONE; @@ -1862,7 +1862,7 @@ > > > static int caam_hash_cra_init(struct crypto_tfm *tfm) > > > * For keyed hash algorithms shared descriptors > > > * will be created later in setkey() callback > > > */ > > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > > + ahash_set_sh_desc(ahash); > > > } > > > > > > > Thanks. Did you also consider putting something in struct caam_hash_alg (the > > struct in which this driver embeds its ahash_alg structure) that indicates whether > > the algorithm is an HMAC or not? Other drivers use that solution. > > Crypto/ahash.c has this API to check the setkey so I used to differentiate between HMAC & only hash. > Let me know if this change is not sufficient, will add the flag in caam_hash_alg. > Currently crypto_hash_alg_has_setkey() isn't used outside of crypto/ahash.c. Since there's an alternative that matches what the other drivers do, I think I'd prefer that you did that. We can then make crypto_hash_alg_has_setkey() a static function, private to crypto/ahash.c. - Eric
> -----Original Message----- > From: Eric Biggers <ebiggers@kernel.org> > Sent: Thursday, January 18, 2024 9:14 AM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > <davem@davemloft.net>; Horia Geanta <horia.geanta@nxp.com>; Pankaj > Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > Aggarwal <meenakshi.aggarwal@nxp.com>; Aisheng Dong > <aisheng.dong@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux- > imx@nxp.com> > Subject: Re: [EXT] Re: [PATCH] crypto: caam/hash - fix asynchronous hash > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > On Wed, Jan 17, 2024 at 10:48:39AM +0000, Gaurav Jain wrote: > > > > > > > -----Original Message----- > > > From: Eric Biggers <ebiggers@kernel.org> > > > Sent: Wednesday, January 17, 2024 10:03 AM > > > To: Gaurav Jain <gaurav.jain@nxp.com> > > > Cc: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > > > <davem@davemloft.net>; Horia Geanta <horia.geanta@nxp.com>; Pankaj > > > Gupta <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; > > > Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Aisheng Dong > > > <aisheng.dong@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>; > > > linux- crypto@vger.kernel.org; linux-kernel@vger.kernel.org; > > > dl-linux-imx <linux- imx@nxp.com> > > > Subject: [EXT] Re: [PATCH] crypto: caam/hash - fix asynchronous hash > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the 'Report this email' button > > > > > > > > > On Tue, Jan 16, 2024 at 03:14:05PM +0530, Gaurav Jain wrote: > > > > ahash_alg->setkey is updated to ahash_nosetkey in ahash.c so > > > > updating the handling of setkey in caam driver. > > > > > > > > Fixes: 2f1f34c1bf7b ("crypto: ahash - optimize performance when > > > > wrapping shash") > > > > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > > > > --- > > > > drivers/crypto/caam/caamalg_qi2.c | 4 ++-- > > > > drivers/crypto/caam/caamhash.c | 4 ++-- > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/crypto/caam/caamalg_qi2.c > > > > b/drivers/crypto/caam/caamalg_qi2.c > > > > index a148ff1f0872..93a400e286b4 100644 > > > > --- a/drivers/crypto/caam/caamalg_qi2.c > > > > +++ b/drivers/crypto/caam/caamalg_qi2.c > > > > @@ -4571,7 +4571,7 @@ static int caam_hash_cra_init(struct > > > > crypto_tfm > > > > *tfm) > > > > > > > > ctx->dev = caam_hash->dev; > > > > > > > > - if (alg->setkey) { > > > > + if (crypto_hash_alg_has_setkey(halg)) { > > > > ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key, > > > > ARRAY_SIZE(ctx->key), > > > > > > > > DMA_TO_DEVICE, @@ -4611,7 +4611,7 @@ static int > > > > caam_hash_cra_init(struct crypto_tfm > > > *tfm) > > > > * For keyed hash algorithms shared descriptors > > > > * will be created later in setkey() callback > > > > */ > > > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > > > + ahash_set_sh_desc(ahash); > > > > } > > > > > > > > static void caam_hash_cra_exit(struct crypto_tfm *tfm) diff --git > > > > a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c > > > > index 290c8500c247..4d50356b593c 100644 > > > > --- a/drivers/crypto/caam/caamhash.c > > > > +++ b/drivers/crypto/caam/caamhash.c > > > > @@ -1804,7 +1804,7 @@ static int caam_hash_cra_init(struct > > > > crypto_tfm > > > *tfm) > > > > } else { > > > > if (priv->era >= 6) { > > > > ctx->dir = DMA_BIDIRECTIONAL; > > > > - ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE; > > > > + ctx->key_dir = > > > > + crypto_hash_alg_has_setkey(halg) ? DMA_TO_DEVICE : DMA_NONE; > > > > } else { > > > > ctx->dir = DMA_TO_DEVICE; > > > > ctx->key_dir = DMA_NONE; @@ -1862,7 +1862,7 > > > > @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) > > > > * For keyed hash algorithms shared descriptors > > > > * will be created later in setkey() callback > > > > */ > > > > - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); > > > > + return crypto_hash_alg_has_setkey(halg) ? 0 : > > > > + ahash_set_sh_desc(ahash); > > > > } > > > > > > > > > > Thanks. Did you also consider putting something in struct > > > caam_hash_alg (the struct in which this driver embeds its ahash_alg > > > structure) that indicates whether the algorithm is an HMAC or not? Other > drivers use that solution. > > > > Crypto/ahash.c has this API to check the setkey so I used to differentiate > between HMAC & only hash. > > Let me know if this change is not sufficient, will add the flag in caam_hash_alg. > > > > Currently crypto_hash_alg_has_setkey() isn't used outside of crypto/ahash.c. > Since there's an alternative that matches what the other drivers do, I think I'd > prefer that you did that. We can then make crypto_hash_alg_has_setkey() a > static function, private to crypto/ahash.c. Understood. Changes done in v2. Thanks Gaurav > > - Eric
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c index a148ff1f0872..93a400e286b4 100644 --- a/drivers/crypto/caam/caamalg_qi2.c +++ b/drivers/crypto/caam/caamalg_qi2.c @@ -4571,7 +4571,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) ctx->dev = caam_hash->dev; - if (alg->setkey) { + if (crypto_hash_alg_has_setkey(halg)) { ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key, ARRAY_SIZE(ctx->key), DMA_TO_DEVICE, @@ -4611,7 +4611,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) * For keyed hash algorithms shared descriptors * will be created later in setkey() callback */ - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); + return crypto_hash_alg_has_setkey(halg) ? 0 : ahash_set_sh_desc(ahash); } static void caam_hash_cra_exit(struct crypto_tfm *tfm) diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index 290c8500c247..4d50356b593c 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -1804,7 +1804,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) } else { if (priv->era >= 6) { ctx->dir = DMA_BIDIRECTIONAL; - ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE; + ctx->key_dir = crypto_hash_alg_has_setkey(halg) ? DMA_TO_DEVICE : DMA_NONE; } else { ctx->dir = DMA_TO_DEVICE; ctx->key_dir = DMA_NONE; @@ -1862,7 +1862,7 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm) * For keyed hash algorithms shared descriptors * will be created later in setkey() callback */ - return alg->setkey ? 0 : ahash_set_sh_desc(ahash); + return crypto_hash_alg_has_setkey(halg) ? 0 : ahash_set_sh_desc(ahash); } static void caam_hash_cra_exit(struct crypto_tfm *tfm)
ahash_alg->setkey is updated to ahash_nosetkey in ahash.c so updating the handling of setkey in caam driver. Fixes: 2f1f34c1bf7b ("crypto: ahash - optimize performance when wrapping shash") Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> --- drivers/crypto/caam/caamalg_qi2.c | 4 ++-- drivers/crypto/caam/caamhash.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)