Received-SPF: pass (google.com: domain of linux-crypto+bounces-1481-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Gaurav Jain <gaurav.jain@nxp.com>
To: Eric Biggers <ebiggers@kernel.org>
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-crypto@vger.kernel.org>, "linux-kernel@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
Thread-Topic: [EXT] Re: [PATCH] crypto: caam/hash - fix asynchronous hash
Thread-Index: AQHaSGCYr0WDHP3yKEe2VG16zWQxl7Dda8kAgABkXzCAASAtAIAAYBmA
Date: Thu, 18 Jan 2024 09:28:28 +0000
Message-ID:
 <AM0PR04MB6004C56470D075284635E008E7712@AM0PR04MB6004.eurprd04.prod.outlook.com>
References: <20240116094405.744466-1-gaurav.jain@nxp.com>
 <20240117043308.GA1137@sol.localdomain>
 <AM0PR04MB6004E3B9AB6DB6BE3F6BE48BE7722@AM0PR04MB6004.eurprd04.prod.outlook.com>
 <20240118034348.GA1103@sol.localdomain>
In-Reply-To: <20240118034348.GA1103@sol.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?3WPe/YlxBSeAYWkqGc7sgYC8dc+ErBPg5mhN9MH09XP3KBH0OOkUOqzuriph?=
 =?us-ascii?Q?Frsz2Qa8+3Zb/sqSDzV1cYOLQUtOHEUrxhyETFWXQV2d2l9c8ijbLVoolhdB?=
 =?us-ascii?Q?0RnWp+OlPBBiLIqiQfF/6L06lW+cNcKRHW13oq7844iMrI+FbwV+A064i6EF?=
 =?us-ascii?Q?BqFUDNbAyTaIn3rMzCx84hiTs3D6Gu+8xpNvY9O6+ahIAIR+KAuiaXx/3VUJ?=
 =?us-ascii?Q?8iNqGOY8ShqjsheKkg34NgtXT410HKNCvVV4MobBlOUUvc9OYS2o0xxp1tlU?=
 =?us-ascii?Q?cg0aCVIcqR9NsfvEVUBoAdIiJ9FZ4DLt4glibfQITTq4T0qcNiyl9GgQ7yCo?=
 =?us-ascii?Q?UdOBn9jcp2MVnR6Dkif3iauMG8QXFnUtigNBbkljMvpOSMy63Hdz5eCvSPAL?=
 =?us-ascii?Q?H4kXQiHFeAWOIoP6/iixwW3pX1ZU0Nbik8pkOQNx5rtv02ClGXvbYOajPwZt?=
 =?us-ascii?Q?qSNjaEMBOQ6CfOF/5CqVmXFx3wvOgy09RK3THLP8HrZcSRsk6d53VkLNMaiQ?=
 =?us-ascii?Q?TytgESJM9q+yMKotwKVxGKmf4aDXr2U7rj/9FmATudx+nrGevedi7ngYvUxj?=
 =?us-ascii?Q?qvSKr5j6EGXUxmT1lsYoZMNX6WqM2KUUjInsWlEQw9oiuxVe4xRe/N/Pobfu?=
 =?us-ascii?Q?QLPqzxEGOe4SeeqGyyv7kLAOxN3ar4gw54SOWeeDa3CWHcyLf7zD29v1vgDI?=
 =?us-ascii?Q?htnviILGO1o8n0iPSuRiMgHzQ5QNS3vhs9wNJwc9kadpmDtVTASiSKaajpyA?=
 =?us-ascii?Q?C5hRIoDzFWyG9jUvdjxSck4FW2F6OyB4/wQipWlWf7U8bkWaSHo617nVbKE2?=
 =?us-ascii?Q?5sy+y8IZCbBA61PqDzf80wg3PWq20zA2gHSH8vBqRP1oVjgfrD+uo9wIgZkc?=
 =?us-ascii?Q?WNAzocPXkUjOb49ShK2/bzqgLtkXrT5LB5DlHiE4aZ038OthDy9TM2EIXgi7?=
 =?us-ascii?Q?lqTpA58K5SnN+Vu+gfipU5fF3RSKOBlRM0iEGjaf8St1iHBN9zX+gh9SvL3l?=
 =?us-ascii?Q?jPtrIx9vxCOGcXti1/f8k0fohycEvSJPVQo+D3Vn5WMTzHPmqllEe6pwgZkI?=
 =?us-ascii?Q?y0Kv31kwgJ7TL0WL7hEBXeLM9nfOcaeb8TyIcDRgdu0DizJaafig0WzCxlBh?=
 =?us-ascii?Q?eyawVKDaYsQqoILEO+lomqZ9yt3TfR7FSsVofhTQceZMIXzFffqpLsIfZg7H?=
 =?us-ascii?Q?+qoZtn/bEL9uHBsVMc8LUrktp7D+Pg3BKWf+Db3bAjieBFpStHkQDU5VIGeI?=
 =?us-ascii?Q?zk5Ne5XbdNYu9A4LZYxudb+CN4a9a1knvqUISj03u7X2M7Vgt4CuCmpPj+L8?=
 =?us-ascii?Q?dl1G1ay4g0HBEjwMsTIJVbWoEjPhIv2XCOoR6q90/FWNHUGaoWCbMF5ARFCo?=
 =?us-ascii?Q?NYJHpD08s2ddZd6KwfShsOaEGRlF5aL8zWbrB+sE6uc9hZRI24HSzSSAT71V?=
 =?us-ascii?Q?y3kdc6S94Ou3ax/MqdqfIjGQGJYYUjgv+9xY/i2yGLbqinIuJIW4RzTVjCf9?=
 =?us-ascii?Q?zaPiJpX5/Yju7mTWv7G4X2IfPQYF/cqELi2NoL3kPPwRZVBcg5uk43Vc0rju?=
 =?us-ascii?Q?rTbfxMY5WA27QVBlRGAUsfd7g9NPmLdCT27YyclS?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-crypto@vger.kernel.org
List-Id: <linux-crypto.vger.kernel.org>
List-Subscribe: <mailto:linux-crypto+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-crypto+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6004.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0b66e78d-9f33-45ca-3484-08dc1807d493
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Jan 2024 09:28:28.9834
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: aHFeKTHCwON7RjVP0uXQGfQ479eakS3VwqphdVL2yL71KBPsfnHmKCczIfbQsu01k4ZxtghwO0Efr7pLZ86rVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8318


> -----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
>=20
> 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
>=20
>=20
> 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 =3D caam_hash->dev;
> > > >
> > > > -     if (alg->setkey) {
> > > > +     if (crypto_hash_alg_has_setkey(halg)) {
> > > >               ctx->adata.key_dma =3D 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 >=3D 6) {
> > > >                       ctx->dir =3D DMA_BIDIRECTIONAL;
> > > > -                     ctx->key_dir =3D alg->setkey ? DMA_TO_DEVICE =
: DMA_NONE;
> > > > +                     ctx->key_dir =3D
> > > > + crypto_hash_alg_has_setkey(halg) ? DMA_TO_DEVICE : DMA_NONE;
> > > >               } else {
> > > >                       ctx->dir =3D DMA_TO_DEVICE;
> > > >                       ctx->key_dir =3D 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?  O=
ther
> drivers use that solution.
> >
> > Crypto/ahash.c has this API to check the setkey so I used to differenti=
ate
> between HMAC & only hash.
> > Let me know if this change is not sufficient, will add the flag in caam=
_hash_alg.
> >
>=20
> 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 th=
ink 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
>=20
> - Eric