| Message ID | 20230523153421.1528359-1-meenakshi.aggarwal@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove CRYPTO_ALG_ALLOCATES_MEMORY flag | expand |
On Tue, May 23, 2023 at 05:34:16PM +0200, meenakshi.aggarwal@nxp.com wrote: > CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to dm-crypt use-cases, > which seems to be 4 entries maximum. This isn't mentioned in the documentation for CRYPTO_ALG_ALLOCATES_MEMORY. So it's not part of the contract of CRYPTO_ALG_ALLOCATES_MEMORY currently. Please don't make this change without updating the documentation. If you'd like to make this change, please update the documentation for CRYPTO_ALG_ALLOCATES_MEMORY to mention this additional exception. - Eric
Hi Eric, We are not updating the functioning of CRYPTO_ALG_ALLOCATES_MEMORY flag. We have introduced the changes in CAAM driver and this flag is no longer required for CAAM use-cases. I will update the description of my patches in next version. Thanks, Meenakshi > -----Original Message----- > From: Eric Biggers <ebiggers@kernel.org> > Sent: Tuesday, May 23, 2023 10:25 PM > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Cc: Horia Geanta <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; > Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>; > herbert@gondor.apana.org.au; davem@davemloft.net; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; Iuliana Prodan > <iuliana.prodan@nxp.com> > Subject: Re: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > On Tue, May 23, 2023 at 05:34:16PM +0200, meenakshi.aggarwal@nxp.com > wrote: > > CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to dm-crypt > > use-cases, which seems to be 4 entries maximum. > > This isn't mentioned in the documentation for > CRYPTO_ALG_ALLOCATES_MEMORY. So it's not part of the contract of > CRYPTO_ALG_ALLOCATES_MEMORY currently. > > Please don't make this change without updating the documentation. > > If you'd like to make this change, please update the documentation for > CRYPTO_ALG_ALLOCATES_MEMORY to mention this additional exception. > > - Eric
On Tue, May 23, 2023 at 04:55:03PM +0000, Eric Biggers wrote: > > This isn't mentioned in the documentation for CRYPTO_ALG_ALLOCATES_MEMORY. So > it's not part of the contract of CRYPTO_ALG_ALLOCATES_MEMORY currently. > > Please don't make this change without updating the documentation. > > If you'd like to make this change, please update the documentation for > CRYPTO_ALG_ALLOCATES_MEMORY to mention this additional exception. Agreed. We talked about this at the time of the original patch-set, but never made any changes in this direction. Apparently the users are still the same so it should be possible but it has to be done in the right order as Eric suggested. Of course if the drivers don't actually need this then all the better and let's not even bother with adding such a restriction. Thanks,
On Thu, Jun 01, 2023 at 11:33:48AM +0100, Herbert Xu wrote: > On Tue, May 23, 2023 at 04:55:03PM +0000, Eric Biggers wrote: > > > > This isn't mentioned in the documentation for CRYPTO_ALG_ALLOCATES_MEMORY. So > > it's not part of the contract of CRYPTO_ALG_ALLOCATES_MEMORY currently. > > > > Please don't make this change without updating the documentation. > > > > If you'd like to make this change, please update the documentation for > > CRYPTO_ALG_ALLOCATES_MEMORY to mention this additional exception. > > Agreed. We talked about this at the time of the original patch-set, > but never made any changes in this direction. > > Apparently the users are still the same so it should be possible > but it has to be done in the right order as Eric suggested. BTW, some time ago we did an assessment of the users of !CRYPTO_ALG_ALLOCATES_MEMORY and we came to the conclusion that we cannot just update the documentation. dm-crypt uses scatterlists with at most 4 entries. dm-integrity, instead, might allocate memory for scatterlists with an arbitrary number of entries. https://www.spinics.net/lists/linux-crypto/msg65282.html Any suggestion on how to move forward for drivers that allocate memory? Regards,
Reviewed-By: Pankaj Gupta <pankaj.gupta@nxp.com> > -----Original Message----- > From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Sent: Tuesday, May 23, 2023 9:04 PM > To: Horia Geanta <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; > Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>; > herbert@gondor.apana.org.au; davem@davemloft.net; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; Iuliana Prodan > <iuliana.prodan@nxp.com> > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Subject: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > > This series includes patches to remove CRYPTO_ALG_ALLOCATES_MEMORY > flag and allocate the required memory within the crypto request object. > > CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to dm-crypt use-cases, > which seems to be 4 entries maximum. > Therefore in reqsize we allocate memory for maximum 4 entries for src and 1 > for IV, and the same for dst, both aligned. > If the driver needs more than the 4 entries maximum, the memory is > dynamically allocated, at runtime. > > Meenakshi Aggarwal (5): > crypto:caam - avoid allocating memory at crypto request runtime for > skcipher > crypto:caam - avoid allocating memory at crypto request runtime for > aead > crypto: caam - avoid allocating memory at crypto request runtime for > hash > crypto: caam/qi - avoid allocating memory at crypto request runtime > crypto: caam/qi2 - avoid allocating memory at crypto request runtime > > drivers/crypto/caam/caamalg.c | 138 +++++++--- > drivers/crypto/caam/caamalg_qi.c | 131 +++++++--- > drivers/crypto/caam/caamalg_qi2.c | 421 ++++++++++++++++++++---------- > drivers/crypto/caam/caamalg_qi2.h | 6 + > drivers/crypto/caam/caamhash.c | 77 ++++-- > 5 files changed, 542 insertions(+), 231 deletions(-) > > -- > 2.25.1
On Thu, Jun 01, 2023 at 12:23:58PM +0100, Giovanni Cabiddu wrote: > > BTW, some time ago we did an assessment of the users of > !CRYPTO_ALG_ALLOCATES_MEMORY and we came to the conclusion that we > cannot just update the documentation. > dm-crypt uses scatterlists with at most 4 entries. dm-integrity, > instead, might allocate memory for scatterlists with an arbitrary number > of entries. dm-integrity shouldn't be using ALLOCATES_MEMORY at all. It's using GFP_KERNEL allocations right next to the crypto operations. But those are some seriously big crypto operations, 16 thousand 4K pages in one hit? Cheers,
On Fri, Jun 09, 2023 at 05:24:04PM +0800, Herbert Xu wrote: > On Thu, Jun 01, 2023 at 12:23:58PM +0100, Giovanni Cabiddu wrote: > > BTW, some time ago we did an assessment of the users of > > !CRYPTO_ALG_ALLOCATES_MEMORY and we came to the conclusion that we > > cannot just update the documentation. > > dm-crypt uses scatterlists with at most 4 entries. dm-integrity, > > instead, might allocate memory for scatterlists with an arbitrary number > > of entries. > > dm-integrity shouldn't be using ALLOCATES_MEMORY at all. It's > using GFP_KERNEL allocations right next to the crypto operations. If you all agree, I can send a patch to remove CRYPTO_ALG_ALLOCATES_MEMORY from dm-integrity and update the documentation in crypto.h. > But those are some seriously big crypto operations, 16 thousand > 4K pages in one hit? Regards,
Hi Eric, Herbert, I think Giovanni should send the patch with suggested changes. Please share your thoughts. Thanks, Meenakshi > -----Original Message----- > From: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > Sent: Saturday, June 10, 2023 2:11 PM > To: Eric Biggers <ebiggers@kernel.org>; Herbert Xu > <herbert@gondor.apana.org.au> > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Horia Geanta > <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Pankaj Gupta > <pankaj.gupta@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>; > davem@davemloft.net; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org; Iuliana Prodan <iuliana.prodan@nxp.com>; > lucas.segarra.fernandez@intel.com > Subject: Re: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > On Fri, Jun 09, 2023 at 05:24:04PM +0800, Herbert Xu wrote: > > On Thu, Jun 01, 2023 at 12:23:58PM +0100, Giovanni Cabiddu wrote: > > > BTW, some time ago we did an assessment of the users of > > > !CRYPTO_ALG_ALLOCATES_MEMORY and we came to the conclusion that > we > > > cannot just update the documentation. > > > dm-crypt uses scatterlists with at most 4 entries. dm-integrity, > > > instead, might allocate memory for scatterlists with an arbitrary > > > number of entries. > > > > dm-integrity shouldn't be using ALLOCATES_MEMORY at all. It's using > > GFP_KERNEL allocations right next to the crypto operations. > If you all agree, I can send a patch to remove > CRYPTO_ALG_ALLOCATES_MEMORY from dm-integrity and update the > documentation in crypto.h. > > > But those are some seriously big crypto operations, 16 thousand 4K > > pages in one hit? > > Regards, > > -- > Giovanni
On Sat, Jun 10, 2023 at 09:41:28AM +0100, Giovanni Cabiddu wrote: > > If you all agree, I can send a patch to remove CRYPTO_ALG_ALLOCATES_MEMORY > from dm-integrity and update the documentation in crypto.h. Yes please. Cheers,
Hi Giovanni, When are you planning to send the patches? Thanks, Meenakshi > -----Original Message----- > From: Herbert Xu <herbert@gondor.apana.org.au> > Sent: Wednesday, June 14, 2023 3:19 PM > To: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > Cc: Eric Biggers <ebiggers@kernel.org>; Meenakshi Aggarwal > <meenakshi.aggarwal@nxp.com>; Horia Geanta <horia.geanta@nxp.com>; > Varun Sethi <V.Sethi@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; > Gaurav Jain <gaurav.jain@nxp.com>; davem@davemloft.net; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; Iuliana Prodan > <iuliana.prodan@nxp.com>; lucas.segarra.fernandez@intel.com > Subject: Re: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > On Sat, Jun 10, 2023 at 09:41:28AM +0100, Giovanni Cabiddu wrote: > > > > If you all agree, I can send a patch to remove > > CRYPTO_ALG_ALLOCATES_MEMORY from dm-integrity and update the > documentation in crypto.h. > > Yes please. > > Cheers, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: > http://gondor.ap/ > ana.org.au%2F~herbert%2F&data=05%7C01%7Cmeenakshi.aggarwal%40nxp.co > m%7C6af00291047c46f2325e08db6cbca189%7C686ea1d3bc2b4c6fa92cd99c5c > 301635%7C0%7C0%7C638223329655306217%7CUnknown%7CTWFpbGZsb3d8e > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 > C3000%7C%7C%7C&sdata=Nrj0YEjJrFUNWY25h0iy5sF7tUnHSDun8KhHxnhUhh0 > %3D&reserved=0 > PGP Key: > http://gondor.ap/ > ana.org.au%2F~herbert%2Fpubkey.txt&data=05%7C01%7Cmeenakshi.aggarwal > %40nxp.com%7C6af00291047c46f2325e08db6cbca189%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C0%7C638223329655306217%7CUnknown%7CTWFp > bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn0%3D%7C3000%7C%7C%7C&sdata=lBaQiOM%2BXaax2lMroHtqbtBbjXBlkkKj > ms8mgBnou%2BU%3D&reserved=0
On Tue, Jul 04, 2023 at 09:19:28AM +0000, Meenakshi Aggarwal wrote:
> When are you planning to send the patches?
Done today.
https://patchwork.kernel.org/project/linux-crypto/list/?series=762772
Regards,
Thank You > -----Original Message----- > From: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > Sent: Wednesday, July 5, 2023 11:21 PM > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au>; Eric Biggers > <ebiggers@kernel.org>; Horia Geanta <horia.geanta@nxp.com>; Varun Sethi > <V.Sethi@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain > <gaurav.jain@nxp.com>; davem@davemloft.net; linux-crypto@vger.kernel.org; > linux-kernel@vger.kernel.org; Iuliana Prodan <iuliana.prodan@nxp.com>; > lucas.segarra.fernandez@intel.com > Subject: Re: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > On Tue, Jul 04, 2023 at 09:19:28AM +0000, Meenakshi Aggarwal wrote: > > When are you planning to send the patches? > Done today. > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwo > rk.kernel.org%2Fproject%2Flinux- > crypto%2Flist%2F%3Fseries%3D762772&data=05%7C01%7Cmeenakshi.aggarwa > l%40nxp.com%7C7bf37c3fd13e41b6a45408db7d807937%7C686ea1d3bc2b4c6f > a92cd99c5c301635%7C0%7C0%7C638241762971432376%7CUnknown%7CTWF > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn0%3D%7C3000%7C%7C%7C&sdata=Ha8TNlFgKo4sten%2BQ8wIe2z%2FzvbjI > npuLwxHAdLVcS4%3D&reserved=0 > > Regards, > > -- > Giovanni > > > > > Thanks, > > Meenakshi > > > > > -----Original Message----- > > > From: Herbert Xu <herbert@gondor.apana.org.au> > > > Sent: Wednesday, June 14, 2023 3:19 PM > > > To: Giovanni Cabiddu <giovanni.cabiddu@intel.com> > > > Cc: Eric Biggers <ebiggers@kernel.org>; Meenakshi Aggarwal > > > <meenakshi.aggarwal@nxp.com>; Horia Geanta <horia.geanta@nxp.com>; > > > Varun Sethi <V.Sethi@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; > > > Gaurav Jain <gaurav.jain@nxp.com>; davem@davemloft.net; linux- > > > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; Iuliana Prodan > > > <iuliana.prodan@nxp.com>; lucas.segarra.fernandez@intel.com > > > Subject: Re: [PATCH 0/5] Remove CRYPTO_ALG_ALLOCATES_MEMORY flag > > > > > > On Sat, Jun 10, 2023 at 09:41:28AM +0100, Giovanni Cabiddu wrote: > > > > > > > > If you all agree, I can send a patch to remove > > > > CRYPTO_ALG_ALLOCATES_MEMORY from dm-integrity and update the > > > documentation in crypto.h. > > > > > > Yes please. > > > > > > Cheers, > > > -- > > > Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgon > > > > dor.ap%2F&data=05%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C7bf37c3fd > 13e > > > > 41b6a45408db7d807937%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C63 > > > > 8241762971432376%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL > CJQIjo > > > > iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ED3 > Qi5 > > > P0i31gXXPgUOcpVD8YcPhaT1gir3lzZT6C0L8%3D&reserved=0 > > > > ana.org.au%2F~herbert%2F&data=05%7C01%7Cmeenakshi.aggarwal%40nxp.co > > > > m%7C6af00291047c46f2325e08db6cbca189%7C686ea1d3bc2b4c6fa92cd99c5c > > > > 301635%7C0%7C0%7C638223329655306217%7CUnknown%7CTWFpbGZsb3d8e > > > > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 > > > > C3000%7C%7C%7C&sdata=Nrj0YEjJrFUNWY25h0iy5sF7tUnHSDun8KhHxnhUhh0 > > > %3D&reserved=0 > > > PGP Key: > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgon > > > > dor.ap%2F&data=05%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C7bf37c3fd > 13e > > > > 41b6a45408db7d807937%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C63 > > > > 8241762971432376%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL > CJQIjo > > > > iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ED3 > Qi5 > > > P0i31gXXPgUOcpVD8YcPhaT1gir3lzZT6C0L8%3D&reserved=0 > > > > ana.org.au%2F~herbert%2Fpubkey.txt&data=05%7C01%7Cmeenakshi.aggarwal > > > %40nxp.com%7C6af00291047c46f2325e08db6cbca189%7C686ea1d3bc2b4c > 6fa > > > > 92cd99c5c301635%7C0%7C0%7C638223329655306217%7CUnknown%7CTWFp > > > > bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > > Mn0%3D%7C3000%7C%7C%7C&sdata=lBaQiOM%2BXaax2lMroHtqbtBbjXBlkkKj > > > ms8mgBnou%2BU%3D&reserved=0
From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> This series includes patches to remove CRYPTO_ALG_ALLOCATES_MEMORY flag and allocate the required memory within the crypto request object. CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to dm-crypt use-cases, which seems to be 4 entries maximum. Therefore in reqsize we allocate memory for maximum 4 entries for src and 1 for IV, and the same for dst, both aligned. If the driver needs more than the 4 entries maximum, the memory is dynamically allocated, at runtime. Meenakshi Aggarwal (5): crypto:caam - avoid allocating memory at crypto request runtime for skcipher crypto:caam - avoid allocating memory at crypto request runtime for aead crypto: caam - avoid allocating memory at crypto request runtime for hash crypto: caam/qi - avoid allocating memory at crypto request runtime crypto: caam/qi2 - avoid allocating memory at crypto request runtime drivers/crypto/caam/caamalg.c | 138 +++++++--- drivers/crypto/caam/caamalg_qi.c | 131 +++++++--- drivers/crypto/caam/caamalg_qi2.c | 421 ++++++++++++++++++++---------- drivers/crypto/caam/caamalg_qi2.h | 6 + drivers/crypto/caam/caamhash.c | 77 ++++-- 5 files changed, 542 insertions(+), 231 deletions(-)