From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:15:59
This series cleans up and optimizes the code that translates between
scatterlists (the input to the API) and virtual addresses (what software
implementations operate on) for skcipher and aead algorithms.
This takes the form of cleanups and optimizations to the skcipher_walk
functions and a rework of the underlying scatter_walk functions.
This series is organized as follows:
- Patch 1-8 are cleanups and optimizations for skcipher_walk.
- Patch 9-10 are cleanups for drivers that were unnecessarily using
scatter_walk when simpler approaches existed.
- Patch 11-15 improve scatter_walk, introducing easier-to-use functions
and optimizing performance in some cases.
- Patch 16-27 convert users to use the new functions.
- Patch 28 removes functions that are no longer needed.
- Patch 29 optimizes the walker on !HIGHMEM platforms to start returning
data segments that can cross a page boundary. This can significantly
improve performance in cases where messages can cross pages, such as
IPsec. Previously there was a large overhead caused by packets being
unnecessarily divided into multiple parts by the walker, including
hitting skcipher_next_slow() which uses a single-block bounce buffer.
Changed in v2:
- Added comment to scatterwalk_done_dst().
- Added scatterwalk_get_sglist() and use it in net/tls/.
- Dropped the keywrap patch, as keywrap is being removed by
https://lore.kernel.org/r/20241227220802.92550-1-ebiggers@kernel.org
Eric Biggers (29):
crypto: skcipher - document skcipher_walk_done() and rename some vars
crypto: skcipher - remove unnecessary page alignment of bounce buffer
crypto: skcipher - remove redundant clamping to page size
crypto: skcipher - remove redundant check for SKCIPHER_WALK_SLOW
crypto: skcipher - fold skcipher_walk_skcipher() into
skcipher_walk_virt()
crypto: skcipher - clean up initialization of skcipher_walk::flags
crypto: skcipher - optimize initializing skcipher_walk fields
crypto: skcipher - call cond_resched() directly
crypto: omap - switch from scatter_walk to plain offset
crypto: powerpc/p10-aes-gcm - simplify handling of linear associated
data
crypto: scatterwalk - move to next sg entry just in time
crypto: scatterwalk - add new functions for skipping data
crypto: scatterwalk - add new functions for iterating through data
crypto: scatterwalk - add new functions for copying data
crypto: scatterwalk - add scatterwalk_get_sglist()
crypto: skcipher - use scatterwalk_start_at_pos()
crypto: aegis - use the new scatterwalk functions
crypto: arm/ghash - use the new scatterwalk functions
crypto: arm64 - use the new scatterwalk functions
crypto: nx - use the new scatterwalk functions
crypto: s390/aes-gcm - use the new scatterwalk functions
crypto: s5p-sss - use the new scatterwalk functions
crypto: stm32 - use the new scatterwalk functions
crypto: x86/aes-gcm - use the new scatterwalk functions
crypto: x86/aegis - use the new scatterwalk functions
net/tls: use the new scatterwalk functions
crypto: skcipher - use the new scatterwalk functions
crypto: scatterwalk - remove obsolete functions
crypto: scatterwalk - don't split at page boundaries when !HIGHMEM
arch/arm/crypto/ghash-ce-glue.c | 15 +-
arch/arm64/crypto/aes-ce-ccm-glue.c | 17 +-
arch/arm64/crypto/ghash-ce-glue.c | 16 +-
arch/arm64/crypto/sm4-ce-ccm-glue.c | 27 ++-
arch/arm64/crypto/sm4-ce-gcm-glue.c | 31 ++-
arch/powerpc/crypto/aes-gcm-p10-glue.c | 8 +-
arch/s390/crypto/aes_s390.c | 33 ++--
arch/x86/crypto/aegis128-aesni-glue.c | 10 +-
arch/x86/crypto/aesni-intel_glue.c | 28 +--
crypto/aegis128-core.c | 10 +-
crypto/scatterwalk.c | 91 +++++----
crypto/skcipher.c | 253 ++++++++++---------------
drivers/crypto/nx/nx-aes-ccm.c | 16 +-
drivers/crypto/nx/nx-aes-gcm.c | 17 +-
drivers/crypto/nx/nx.c | 31 +--
drivers/crypto/nx/nx.h | 3 -
drivers/crypto/omap-aes.c | 34 ++--
drivers/crypto/omap-aes.h | 6 +-
drivers/crypto/omap-des.c | 40 ++--
drivers/crypto/s5p-sss.c | 38 ++--
drivers/crypto/stm32/stm32-cryp.c | 34 ++--
include/crypto/internal/skcipher.h | 2 +-
include/crypto/scatterwalk.h | 203 ++++++++++++++++----
net/tls/tls_device_fallback.c | 31 +--
24 files changed, 473 insertions(+), 521 deletions(-)
base-commit: 7b6092ee7a4ce2d03dc65b87537889e8e1e0ab95
prerequisite-patch-id: a0414cca60a72ee1056cce0a74175103b19e0e77
--
2.47.1
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:15:59
From: Eric Biggers <redacted>
skcipher_walk_done() has an unusual calling convention, and some of its
local variables have unclear names. Document it and rename variables to
make it a bit clearer what is going on. No change in behavior.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 50 ++++++++++++++++++++----------
include/crypto/internal/skcipher.h | 2 +-
2 files changed, 35 insertions(+), 17 deletions(-)
@@ -87,21 +87,39 @@ static int skcipher_done_slow(struct skcipher_walk *walk, unsigned int bsize)addr=skcipher_get_spot(addr,bsize);scatterwalk_copychunks(addr,&walk->out,bsize,1);return0;}-intskcipher_walk_done(structskcipher_walk*walk,interr)+/**+*skcipher_walk_done()-finishonestepofaskcipher_walk+*@walk:theskcipher_walk+*@res:numberofbytes*not*processed(>=0)fromwalk->nbytes,+*ora-errnovaluetoterminatethewalkduetoanerror+*+*Thisfunctioncleansupafteronestepofwalkingthroughthesourceand+*destinationscatterlists,andadvancestothenextstepifapplicable.+*walk->nbytesissettothenumberofbytesavailableinthenextstep,+*walk->totalissettothenewtotalnumberofbytesremaining,and+*walk->{src,dst}.virt.addrissettothenextpairofdatapointers.Ifthere+*isnomoredata,orifanerroroccurred(i.e.-errnoreturn),then+*walk->nbytesandwalk->totalaresetto0andallresourcesownedbythe+*skcipher_walkarefreed.+*+*Return:0ora-errnovalue.If@reswasa-errnovaluethenitwillbe+*returned,butothererrorsmayoccurtoo.+*/+intskcipher_walk_done(structskcipher_walk*walk,intres){-unsignedintn=walk->nbytes;-unsignedintnbytes=0;+unsignedintn=walk->nbytes;/* num bytes processed this step */+unsignedinttotal=0;/* new total remaining */if(!n)gotofinish;-if(likely(err>=0)){-n-=err;-nbytes=walk->total-n;+if(likely(res>=0)){+n-=res;/* subtract num bytes *not* processed */+total=walk->total-n;}if(likely(!(walk->flags&(SKCIPHER_WALK_SLOW|SKCIPHER_WALK_COPY|SKCIPHER_WALK_DIFF)))){
@@ -113,35 +131,35 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err)}elseif(walk->flags&SKCIPHER_WALK_COPY){skcipher_map_dst(walk);memcpy(walk->dst.virt.addr,walk->page,n);skcipher_unmap_dst(walk);}elseif(unlikely(walk->flags&SKCIPHER_WALK_SLOW)){-if(err>0){+if(res>0){/**Didn'tprocessallbytes.Eitherthealgorithmis*broken,orthiswasthelaststepanditturnedout*themessagewasn'tevenlydivisibleintoblocksbut*thealgorithmrequiresit.*/-err=-EINVAL;-nbytes=0;+res=-EINVAL;+total=0;}elsen=skcipher_done_slow(walk,n);}-if(err>0)-err=0;+if(res>0)+res=0;-walk->total=nbytes;+walk->total=total;walk->nbytes=0;scatterwalk_advance(&walk->in,n);scatterwalk_advance(&walk->out,n);-scatterwalk_done(&walk->in,0,nbytes);-scatterwalk_done(&walk->out,1,nbytes);+scatterwalk_done(&walk->in,0,total);+scatterwalk_done(&walk->out,1,total);-if(nbytes){+if(total){crypto_yield(walk->flags&SKCIPHER_WALK_SLEEP?CRYPTO_TFM_REQ_MAY_SLEEP:0);returnskcipher_walk_next(walk);}
@@ -156,11 +174,11 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err)kfree(walk->buffer);if(walk->page)free_page((unsignedlong)walk->page);out:-returnerr;+returnres;}EXPORT_SYMBOL_GPL(skcipher_walk_done);staticintskcipher_next_slow(structskcipher_walk*walk,unsignedintbsize){
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:15:59
From: Eric Biggers <redacted>
In the slow path of skcipher_walk where it uses a slab bounce buffer for
the data and/or IV, do not bother to avoid crossing a page boundary in
the part(s) of this buffer that are used, and do not bother to allocate
extra space in the buffer for that purpose. The buffer is accessed only
by virtual address, so pages are irrelevant for it.
This logic may have been present due to the physical address support in
skcipher_walk, but that has now been removed. Or it may have been
present to be consistent with the fast path that currently does not hand
back addresses that span pages, but that behavior is a side effect of
the pages being "mapped" one by one and is not actually a requirement.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 62 ++++++++++++-----------------------------------
1 file changed, 15 insertions(+), 47 deletions(-)
@@ -61,32 +61,20 @@ static inline void skcipher_unmap_dst(struct skcipher_walk *walk)staticinlinegfp_tskcipher_walk_gfp(structskcipher_walk*walk){returnwalk->flags&SKCIPHER_WALK_SLEEP?GFP_KERNEL:GFP_ATOMIC;}-/* Get a spot of the specified length that does not straddle a page.-*Thecallerneedstoensurethatthereisenoughspaceforthisoperation.-*/-staticinlineu8*skcipher_get_spot(u8*start,unsignedintlen)-{-u8*end_page=(u8*)(((unsignedlong)(start+len-1))&PAGE_MASK);--returnmax(start,end_page);-}-staticinlinestructskcipher_alg*__crypto_skcipher_alg(structcrypto_alg*alg){returncontainer_of(alg,structskcipher_alg,base);}staticintskcipher_done_slow(structskcipher_walk*walk,unsignedintbsize){-u8*addr;+u8*addr=PTR_ALIGN(walk->buffer,walk->alignmask+1);-addr=(u8*)ALIGN((unsignedlong)walk->buffer,walk->alignmask+1);-addr=skcipher_get_spot(addr,bsize);scatterwalk_copychunks(addr,&walk->out,bsize,1);return0;}/**
@@ -181,37 +169,26 @@ int skcipher_walk_done(struct skcipher_walk *walk, int res)EXPORT_SYMBOL_GPL(skcipher_walk_done);staticintskcipher_next_slow(structskcipher_walk*walk,unsignedintbsize){unsignedalignmask=walk->alignmask;-unsigneda;unsignedn;u8*buffer;if(!walk->buffer)walk->buffer=walk->page;buffer=walk->buffer;-if(buffer)-gotook;--/* Start with the minimum alignment of kmalloc. */-a=crypto_tfm_ctx_alignment()-1;-n=bsize;--/* Minimum size to align buffer by alignmask. */-n+=alignmask&~a;--/* Minimum size to ensure buffer does not straddle a page. */-n+=(bsize-1)&~(alignmask|a);--buffer=kzalloc(n,skcipher_walk_gfp(walk));-if(!buffer)-returnskcipher_walk_done(walk,-ENOMEM);-walk->buffer=buffer;-ok:+if(!buffer){+/* Min size for a buffer of bsize bytes aligned to alignmask */+n=bsize+(alignmask&~(crypto_tfm_ctx_alignment()-1));++buffer=kzalloc(n,skcipher_walk_gfp(walk));+if(!buffer)+returnskcipher_walk_done(walk,-ENOMEM);+walk->buffer=buffer;+}walk->dst.virt.addr=PTR_ALIGN(buffer,alignmask+1);-walk->dst.virt.addr=skcipher_get_spot(walk->dst.virt.addr,bsize);walk->src.virt.addr=walk->dst.virt.addr;scatterwalk_copychunks(walk->src.virt.addr,&walk->in,bsize,0);walk->nbytes=bsize;
@@ -294,34 +271,25 @@ static int skcipher_walk_next(struct skcipher_walk *walk)returnskcipher_next_fast(walk);}staticintskcipher_copy_iv(structskcipher_walk*walk){-unsigneda=crypto_tfm_ctx_alignment()-1;unsignedalignmask=walk->alignmask;unsignedivsize=walk->ivsize;-unsignedbs=walk->stride;-unsignedaligned_bs;+unsignedaligned_stride=ALIGN(walk->stride,alignmask+1);unsignedsize;u8*iv;-aligned_bs=ALIGN(bs,alignmask+1);--/* Minimum size to align buffer by alignmask. */-size=alignmask&~a;--size+=aligned_bs+ivsize;--/* Minimum size to ensure buffer does not straddle a page. */-size+=(bs-1)&~(alignmask|a);+/* Min size for a buffer of stride + ivsize, aligned to alignmask */+size=aligned_stride+ivsize++(alignmask&~(crypto_tfm_ctx_alignment()-1));walk->buffer=kmalloc(size,skcipher_walk_gfp(walk));if(!walk->buffer)return-ENOMEM;-iv=PTR_ALIGN(walk->buffer,alignmask+1);-iv=skcipher_get_spot(iv,bs)+aligned_bs;+iv=PTR_ALIGN(walk->buffer,alignmask+1)+aligned_stride;walk->iv=memcpy(iv,walk->iv,walk->ivsize);return0;}
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:00
From: Eric Biggers <redacted>
In skcipher_walk_done(), remove the check for SKCIPHER_WALK_SLOW because
it is always true. All other flags (and lack thereof) were checked
earlier in the function, leaving SKCIPHER_WALK_SLOW as the only
remaining possibility.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:00
From: Eric Biggers <redacted>
In the case where skcipher_walk_next() allocates a bounce page, that
page by definition has size PAGE_SIZE. The number of bytes to copy 'n'
is guaranteed to fit in it, since earlier in the function it was clamped
to be at most a page. Therefore remove the unnecessary logic that tried
to clamp 'n' again to fit in the bounce page.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:00
From: Eric Biggers <redacted>
Fold skcipher_walk_skcipher() into skcipher_walk_virt() which is its
only remaining caller. No change in behavior.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:00
From: Eric Biggers <redacted>
- Initialize SKCIPHER_WALK_SLEEP in a consistent way, and check for
atomic=true at the same time as CRYPTO_TFM_REQ_MAY_SLEEP. Technically
atomic=true only needs to apply after the first step, but it is very
rarely used. We should optimize for the common case. So, check
'atomic' alongside CRYPTO_TFM_REQ_MAY_SLEEP. This is more efficient.
- Initialize flags other than SKCIPHER_WALK_SLEEP to 0 rather than
preserving them. No caller actually initializes the flags, which
makes it impossible to use their original values for anything.
Indeed, that does not happen and all meaningful flags get overridden
anyway. It may have been thought that just clearing one flag would be
faster than clearing all flags, but that's not the case as the former
is a read-write operation whereas the latter is just a write.
- Move the explicit clearing of SKCIPHER_WALK_SLOW, SKCIPHER_WALK_COPY,
and SKCIPHER_WALK_DIFF into skcipher_walk_done(), since it is now
only needed on non-first steps.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)
@@ -146,10 +146,12 @@ int skcipher_walk_done(struct skcipher_walk *walk, int res)scatterwalk_done(&walk->out,1,total);if(total){crypto_yield(walk->flags&SKCIPHER_WALK_SLEEP?CRYPTO_TFM_REQ_MAY_SLEEP:0);+walk->flags&=~(SKCIPHER_WALK_SLOW|SKCIPHER_WALK_COPY|+SKCIPHER_WALK_DIFF);returnskcipher_walk_next(walk);}finish:/* Short-circuit for the common/fast path. */
@@ -233,13 +235,10 @@ static int skcipher_next_fast(struct skcipher_walk *walk)staticintskcipher_walk_next(structskcipher_walk*walk){unsignedintbsize;unsignedintn;-walk->flags&=~(SKCIPHER_WALK_SLOW|SKCIPHER_WALK_COPY|-SKCIPHER_WALK_DIFF);-n=walk->total;bsize=min(walk->stride,max(n,walk->blocksize));n=scatterwalk_clamp(&walk->in,n);n=scatterwalk_clamp(&walk->out,n);
@@ -309,55 +308,53 @@ static int skcipher_walk_first(struct skcipher_walk *walk)intskcipher_walk_virt(structskcipher_walk*walk,structskcipher_request*req,boolatomic){structcrypto_skcipher*tfm=crypto_skcipher_reqtfm(req);structskcipher_alg*alg=crypto_skcipher_alg(tfm);-interr=0;might_sleep_if(req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP);walk->total=req->cryptlen;walk->nbytes=0;walk->iv=req->iv;walk->oiv=req->iv;+if((req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)&&!atomic)+walk->flags=SKCIPHER_WALK_SLEEP;+else+walk->flags=0;if(unlikely(!walk->total))-gotoout;+return0;scatterwalk_start(&walk->in,req->src);scatterwalk_start(&walk->out,req->dst);-walk->flags&=~SKCIPHER_WALK_SLEEP;-walk->flags|=req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP?-SKCIPHER_WALK_SLEEP:0;-walk->blocksize=crypto_skcipher_blocksize(tfm);walk->ivsize=crypto_skcipher_ivsize(tfm);walk->alignmask=crypto_skcipher_alignmask(tfm);if(alg->co.base.cra_type!=&crypto_skcipher_type)walk->stride=alg->co.chunksize;elsewalk->stride=alg->walksize;-err=skcipher_walk_first(walk);-out:-walk->flags&=atomic?~SKCIPHER_WALK_SLEEP:~0;--returnerr;+returnskcipher_walk_first(walk);}EXPORT_SYMBOL_GPL(skcipher_walk_virt);staticintskcipher_walk_aead_common(structskcipher_walk*walk,structaead_request*req,boolatomic){structcrypto_aead*tfm=crypto_aead_reqtfm(req);-interr;walk->nbytes=0;walk->iv=req->iv;walk->oiv=req->iv;+if((req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)&&!atomic)+walk->flags=SKCIPHER_WALK_SLEEP;+else+walk->flags=0;if(unlikely(!walk->total))return0;scatterwalk_start(&walk->in,req->src);
@@ -367,26 +364,16 @@ static int skcipher_walk_aead_common(struct skcipher_walk *walk,scatterwalk_copychunks(NULL,&walk->out,req->assoclen,2);scatterwalk_done(&walk->in,0,walk->total);scatterwalk_done(&walk->out,0,walk->total);-if(req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)-walk->flags|=SKCIPHER_WALK_SLEEP;-else-walk->flags&=~SKCIPHER_WALK_SLEEP;-walk->blocksize=crypto_aead_blocksize(tfm);walk->stride=crypto_aead_chunksize(tfm);walk->ivsize=crypto_aead_ivsize(tfm);walk->alignmask=crypto_aead_alignmask(tfm);-err=skcipher_walk_first(walk);--if(atomic)-walk->flags&=~SKCIPHER_WALK_SLEEP;--returnerr;+returnskcipher_walk_first(walk);}intskcipher_walk_aead_encrypt(structskcipher_walk*walk,structaead_request*req,boolatomic){
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:01
From: Eric Biggers <redacted>
In skcipher_walk_done(), instead of calling crypto_yield() which
requires a translation between flags, just call cond_resched() directly.
This has the same effect.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -144,12 +144,12 @@ int skcipher_walk_done(struct skcipher_walk *walk, int res)scatterwalk_advance(&walk->out,n);scatterwalk_done(&walk->in,0,total);scatterwalk_done(&walk->out,1,total);if(total){-crypto_yield(walk->flags&SKCIPHER_WALK_SLEEP?-CRYPTO_TFM_REQ_MAY_SLEEP:0);+if(walk->flags&SKCIPHER_WALK_SLEEP)+cond_resched();walk->flags&=~(SKCIPHER_WALK_SLOW|SKCIPHER_WALK_COPY|SKCIPHER_WALK_DIFF);returnskcipher_walk_next(walk);}
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:01
From: Eric Biggers <redacted>
The helper functions like crypto_skcipher_blocksize() take in a pointer
to a tfm object, but they actually return properties of the algorithm.
As the Linux kernel is compiled with -fno-strict-aliasing, the compiler
has to assume that the writes to struct skcipher_walk could clobber the
tfm's pointer to its algorithm. Thus it gets repeatedly reloaded in the
generated code. Therefore, replace the use of these helper functions
with staightforward accesses to the struct fields.
Note that while *users* of the skcipher and aead APIs are supposed to
use the helper functions, this particular code is part of the API
*implementation* in crypto/skcipher.c, which already accesses the
algorithm struct directly in many cases. So there is no reason to
prefer the helper functions here.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:01
From: Eric Biggers <redacted>
The omap driver was using struct scatter_walk, but only to maintain an
offset, rather than iterating through the virtual addresses of the data
contained in the scatterlist which is what scatter_walk is intended for.
Make it just use a plain offset instead. This is simpler and avoids
using struct scatter_walk in a way that is not well supported.
Signed-off-by: Eric Biggers <redacted>
---
drivers/crypto/omap-aes.c | 34 ++++++++++++++-------------------
drivers/crypto/omap-aes.h | 6 ++----
drivers/crypto/omap-des.c | 40 ++++++++++++++++-----------------------
3 files changed, 32 insertions(+), 48 deletions(-)
@@ -270,13 +269,13 @@ static int omap_aes_crypt_dma(struct omap_aes_dev *dd,structdma_async_tx_descriptor*tx_in,*tx_out=NULL,*cb_desc;structdma_slave_configcfg;intret;if(dd->pio_only){-scatterwalk_start(&dd->in_walk,dd->in_sg);+dd->in_sg_offset=0;if(out_sg_len)-scatterwalk_start(&dd->out_walk,dd->out_sg);+dd->out_sg_offset=0;/* Enable DATAIN interrupt and let it takecareoftherest*/omap_aes_write(dd,AES_REG_IRQ_ENABLE(dd),0x2);return0;
@@ -150,12 +147,12 @@ struct omap_des_dev {/* Buffers for copying for unaligned cases */structscatterlistin_sgl;structscatterlistout_sgl;structscatterlist*orig_out;-structscatter_walkin_walk;-structscatter_walkout_walk;+unsignedintin_sg_offset;+unsignedintout_sg_offset;structdma_chan*dma_lch_in;structdma_chan*dma_lch_out;intin_sg_len;intout_sg_len;intpio_only;
@@ -377,12 +374,12 @@ static int omap_des_crypt_dma(struct crypto_tfm *tfm,structdma_async_tx_descriptor*tx_in,*tx_out;structdma_slave_configcfg;intret;if(dd->pio_only){-scatterwalk_start(&dd->in_walk,dd->in_sg);-scatterwalk_start(&dd->out_walk,dd->out_sg);+dd->in_sg_offset=0;+dd->out_sg_offset=0;/* Enable DATAIN interrupt and let it takecareoftherest*/omap_des_write(dd,DES_REG_IRQ_ENABLE(dd),0x2);return0;
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:02
From: Eric Biggers <redacted>
p10_aes_gcm_crypt() is abusing the scatter_walk API to get the virtual
address for the first source scatterlist element. But this code is only
built for PPC64 which is a !HIGHMEM platform, and it can read past a
page boundary from the address returned by scatterwalk_map() which means
it already assumes the address is from the kernel's direct map. Thus,
just use sg_virt() instead to get the same result in a simpler way.
Cc: Christophe Leroy <redacted>
Cc: Danny Tsen <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
arch/powerpc/crypto/aes-gcm-p10-glue.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
@@ -232,12 +231,11 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8 *riv,memset(ivbuf,0,sizeof(ivbuf));memcpy(iv,riv,GCM_IV_SIZE);/* Linearize assoc, if not already linear */if(req->src->length>=assoclen&&req->src->length){-scatterwalk_start(&assoc_sg_walk,req->src);-assoc=scatterwalk_map(&assoc_sg_walk);+assoc=sg_virt(req->src);/* ppc64 is !HIGHMEM */}else{gfp_tflags=(req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)?GFP_KERNEL:GFP_ATOMIC;/* assoc can be any length, so must be on heap */
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:02
From: Eric Biggers <redacted>
Add scatterwalk_skip() to skip the given number of bytes in a
scatter_walk. Previously support for skipping was provided through
scatterwalk_copychunks(..., 2) followed by scatterwalk_done(), which was
confusing and less efficient.
Also add scatterwalk_start_at_pos() which starts a scatter_walk at the
given position, equivalent to scatterwalk_start() + scatterwalk_skip().
This addresses another common need in a more streamlined way.
Later patches will convert various users to use these functions.
Signed-off-by: Eric Biggers <redacted>
---
crypto/scatterwalk.c | 15 +++++++++++++++
include/crypto/scatterwalk.h | 18 ++++++++++++++++++
2 files changed, 33 insertions(+)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:02
From: Eric Biggers <redacted>
The scatterwalk_* functions are designed to advance to the next sg entry
only when there is more data from the request to process. Compared to
the alternative of advancing after each step if !sg_is_last(sg), this
has the advantage that it doesn't cause problems if users accidentally
don't terminate their scatterlist with the end marker (which is an easy
mistake to make, and there are examples of this).
Currently, the advance to the next sg entry happens in
scatterwalk_done(), which is called after each "step" of the walk. It
requires the caller to pass in a boolean 'more' that indicates whether
there is more data. This works when the caller immediately knows
whether there is more data, though it adds some complexity. However in
the case of scatterwalk_copychunks() it's not immediately known whether
there is more data, so the call to scatterwalk_done() has to happen
higher up the stack. This is error-prone, and indeed the needed call to
scatterwalk_done() is not always made, e.g. scatterwalk_copychunks() is
sometimes called multiple times in a row. This causes a zero-length
step to get added in some cases, which is unexpected and seems to work
only by accident.
This patch begins the switch to a less error-prone approach where the
advance to the next sg entry happens just in time instead. For now,
that means just doing the advance in scatterwalk_clamp() if it's needed
there. Initially this is redundant, but it's needed to keep the tree in
a working state as later patches change things to the final state.
Later patches will similarly move the dcache flushing logic out of
scatterwalk_done() and then remove scatterwalk_done() entirely.
Signed-off-by: Eric Biggers <redacted>
---
include/crypto/scatterwalk.h | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:03
From: Eric Biggers <redacted>
Add scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(). Also add scatterwalk_done_src() and
scatterwalk_done_dst() which consolidate scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done() or scatterwalk_pagedone().
A later patch will remove scatterwalk_done() and scatterwalk_pagedone().
The new code eliminates the error-prone 'more' parameter. Advancing to
the next sg entry now only happens just-in-time in scatterwalk_next().
The new code also pairs the dcache flush more closely with the actual
write, similar to memcpy_to_page(). Previously it was paired with
advancing to the next page. This is currently causing bugs where the
dcache flush is incorrectly being skipped, usually due to
scatterwalk_copychunks() being called without a following
scatterwalk_done(). The dcache flush may have been placed where it was
in order to not call flush_dcache_page() redundantly when visiting a
page more than once. However, that case is rare in practice, and most
architectures either do not implement flush_dcache_page() anyway or
implement it lazily where it just clears a page flag.
Another limitation of the old code was that by the time the flush
happened, there was no way to tell if more than one page needed to be
flushed. That has been sufficient because the code goes page by page,
but I would like to optimize that on !HIGHMEM platforms. The new code
makes this possible, and a later patch will implement this optimization.
Signed-off-by: Eric Biggers <redacted>
---
include/crypto/scatterwalk.h | 69 ++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 6 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:03
From: Eric Biggers <redacted>
Add memcpy_from_sglist() and memcpy_to_sglist() which are more readable
versions of scatterwalk_map_and_copy() with the 'out' argument 0 and 1
respectively. They follow the same argument order as memcpy_from_page()
and memcpy_to_page() from <linux/highmem.h>. Note that in the case of
memcpy_from_sglist(), this also happens to be the same argument order
that scatterwalk_map_and_copy() uses.
The new code is also faster, mainly because it builds the scatter_walk
directly without creating a temporary scatterlist. E.g., a 20%
performance improvement is seen for copying the AES-GCM auth tag.
Make scatterwalk_map_and_copy() be a wrapper around memcpy_from_sglist()
and memcpy_to_sglist(). Callers of scatterwalk_map_and_copy() should be
updated to call memcpy_from_sglist() or memcpy_to_sglist() directly, but
there are a lot of them so they aren't all being updated right away.
Also add functions memcpy_from_scatterwalk() and memcpy_to_scatterwalk()
which are similar but operate on a scatter_walk instead of a
scatterlist. These will replace scatterwalk_copychunks() with the 'out'
argument 0 and 1 respectively. Their behavior differs slightly from
scatterwalk_copychunks() in that they automatically take care of
flushing the dcache when needed, making them easier to use.
scatterwalk_copychunks() itself is left unchanged for now. It will be
removed after its callers are updated to use other functions instead.
Signed-off-by: Eric Biggers <redacted>
---
crypto/scatterwalk.c | 59 ++++++++++++++++++++++++++++++------
include/crypto/scatterwalk.h | 24 +++++++++++++--
2 files changed, 72 insertions(+), 11 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:04
From: Eric Biggers <redacted>
Add a function that creates a scatterlist that represents the remaining
data in a walk. This will be used to replace chain_to_walk() in
net/tls/tls_device_fallback.c so that it will no longer need to reach
into the internals of struct scatter_walk.
Cc: Boris Pismenny <borisp@nvidia.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
include/crypto/scatterwalk.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:04
From: Eric Biggers <redacted>
Use scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(), and use scatterwalk_done_src() which consolidates
scatterwalk_unmap(), scatterwalk_advance(), and scatterwalk_done().
Signed-off-by: Eric Biggers <redacted>
---
crypto/aegis128-core.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:04
From: Eric Biggers <redacted>
In skcipher_walk_aead_common(), use scatterwalk_start_at_pos() instead
of a sequence of scatterwalk_start(), scatterwalk_copychunks(..., 2),
and scatterwalk_done(). This is simpler and faster.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:05
From: Eric Biggers <redacted>
Use scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(), and use scatterwalk_done_src() which consolidates
scatterwalk_unmap(), scatterwalk_advance(), and scatterwalk_done().
Remove unnecessary code that seemed to be intended to advance to the
next sg entry, which is already handled by the scatterwalk functions.
Signed-off-by: Eric Biggers <redacted>
---
arch/arm/crypto/ghash-ce-glue.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:05
From: Eric Biggers <redacted>
Use scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(), and use scatterwalk_done_src() which consolidates
scatterwalk_unmap(), scatterwalk_advance(), and scatterwalk_done().
Remove unnecessary code that seemed to be intended to advance to the
next sg entry, which is already handled by the scatterwalk functions.
Adjust variable naming slightly to keep things consistent.
Signed-off-by: Eric Biggers <redacted>
---
arch/arm64/crypto/aes-ce-ccm-glue.c | 17 ++++------------
arch/arm64/crypto/ghash-ce-glue.c | 16 ++++-----------
arch/arm64/crypto/sm4-ce-ccm-glue.c | 27 ++++++++++---------------
arch/arm64/crypto/sm4-ce-gcm-glue.c | 31 ++++++++++++-----------------
4 files changed, 32 insertions(+), 59 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:05
From: Eric Biggers <redacted>
- In nx_walk_and_build(), use scatterwalk_start_at_pos() instead of a
more complex way to achieve the same result.
- Also in nx_walk_and_build(), use the new functions scatterwalk_next()
which consolidates scatterwalk_clamp() and scatterwalk_map(), and use
scatterwalk_done_src() which consolidates scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done(). Remove unnecessary
code that seemed to be intended to advance to the next sg entry, which
is already handled by the scatterwalk functions.
Note that nx_walk_and_build() does not actually read or write the
mapped virtual address, and thus it is misusing the scatter_walk API.
It really should just access the scatterlist directly. This patch
does not try to address this existing issue.
- In nx_gca(), use memcpy_from_sglist() instead of a more complex way to
achieve the same result.
- In various functions, replace calls to scatterwalk_map_and_copy() with
memcpy_from_sglist() or memcpy_to_sglist() as appropriate. Note that
this eliminates the confusing 'out' argument (which this driver had
tried to work around by defining the missing constants for it...)
Cc: Christophe Leroy <redacted>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
drivers/crypto/nx/nx-aes-ccm.c | 16 ++++++----------
drivers/crypto/nx/nx-aes-gcm.c | 17 ++++++-----------
drivers/crypto/nx/nx.c | 31 +++++--------------------------
drivers/crypto/nx/nx.h | 3 ---
4 files changed, 17 insertions(+), 50 deletions(-)
@@ -215,17 +215,15 @@ static int generate_pat(u8 *iv,*/if(b1){memset(b1,0,16);if(assoclen<=65280){*(u16*)b1=assoclen;-scatterwalk_map_and_copy(b1+2,req->src,0,-iauth_len,SCATTERWALK_FROM_SG);+memcpy_from_sglist(b1+2,req->src,0,iauth_len);}else{*(u16*)b1=(u16)(0xfffe);*(u32*)&b1[2]=assoclen;-scatterwalk_map_and_copy(b1+6,req->src,0,-iauth_len,SCATTERWALK_FROM_SG);+memcpy_from_sglist(b1+6,req->src,0,iauth_len);}}/* now copy any remaining AAD to scatterlist and call nx... */if(!assoclen){
@@ -339,13 +337,12 @@ static int ccm_nx_decrypt(struct aead_request *req,spin_lock_irqsave(&nx_ctx->lock,irq_flags);nbytes-=authsize;/* copy out the auth tag to compare with later */-scatterwalk_map_and_copy(priv->oauth_tag,-req->src,nbytes+req->assoclen,authsize,-SCATTERWALK_FROM_SG);+memcpy_from_sglist(priv->oauth_tag,req->src,nbytes+req->assoclen,+authsize);rc=generate_pat(iv,req,nx_ctx,authsize,nbytes,assoclen,csbcpb->cpb.aes_ccm.in_pat_or_b0);if(rc)gotoout;
@@ -463,13 +460,12 @@ static int ccm_nx_encrypt(struct aead_request *req,processed+=to_process;}while(processed<nbytes);/* copy out the auth tag */-scatterwalk_map_and_copy(csbcpb->cpb.aes_ccm.out_pat_or_mac,-req->dst,nbytes+req->assoclen,authsize,-SCATTERWALK_TO_SG);+memcpy_to_sglist(req->dst,nbytes+req->assoclen,+csbcpb->cpb.aes_ccm.out_pat_or_mac,authsize);out:spin_unlock_irqrestore(&nx_ctx->lock,irq_flags);returnrc;}
@@ -101,20 +101,17 @@ static int nx_gca(struct nx_crypto_ctx *nx_ctx,u8*out,unsignedintassoclen){intrc;structnx_csbcpb*csbcpb_aead=nx_ctx->csbcpb_aead;-structscatter_walkwalk;structnx_sg*nx_sg=nx_ctx->in_sg;unsignedintnbytes=assoclen;unsignedintprocessed=0,to_process;unsignedintmax_sg_len;if(nbytes<=AES_BLOCK_SIZE){-scatterwalk_start(&walk,req->src);-scatterwalk_copychunks(out,&walk,nbytes,SCATTERWALK_FROM_SG);-scatterwalk_done(&walk,SCATTERWALK_FROM_SG,0);+memcpy_from_sglist(out,req->src,0,nbytes);return0;}NX_CPB_FDM(csbcpb_aead)&=~NX_FDM_CONTINUATION;
@@ -389,23 +386,21 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int enc,}while(processed<nbytes);mac:if(enc){/* copy out the auth tag */-scatterwalk_map_and_copy(-csbcpb->cpb.aes_gcm.out_pat_or_mac,+memcpy_to_sglist(req->dst,req->assoclen+nbytes,-crypto_aead_authsize(crypto_aead_reqtfm(req)),-SCATTERWALK_TO_SG);+csbcpb->cpb.aes_gcm.out_pat_or_mac,+crypto_aead_authsize(crypto_aead_reqtfm(req)));}else{u8*itag=nx_ctx->priv.gcm.iauth_tag;u8*otag=csbcpb->cpb.aes_gcm.out_pat_or_mac;-scatterwalk_map_and_copy(+memcpy_from_sglist(itag,req->src,req->assoclen+nbytes,-crypto_aead_authsize(crypto_aead_reqtfm(req)),-SCATTERWALK_FROM_SG);+crypto_aead_authsize(crypto_aead_reqtfm(req)));rc=crypto_memneq(itag,otag,crypto_aead_authsize(crypto_aead_reqtfm(req)))?-EBADMSG:0;}out:
@@ -151,44 +151,23 @@ struct nx_sg *nx_walk_and_build(struct nx_sg *nx_dst,unsignedintstart,unsignedint*src_len){structscatter_walkwalk;structnx_sg*nx_sg=nx_dst;-unsignedintn,offset=0,len=*src_len;+unsignedintn,len=*src_len;char*dst;/* we need to fast forward through @start bytes first */-for(;;){-scatterwalk_start(&walk,sg_src);--if(start<offset+sg_src->length)-break;--offset+=sg_src->length;-sg_src=sg_next(sg_src);-}--/* start - offset is the number of bytes to advance in the scatterlist-*elementwe'recurrentlylookingat*/-scatterwalk_advance(&walk,start-offset);+scatterwalk_start_at_pos(&walk,sg_src,start);while(len&&(nx_sg-nx_dst)<sglen){-n=scatterwalk_clamp(&walk,len);-if(!n){-/* In cases where we have scatterlist chain sg_next-*handleswithitproperly*/-scatterwalk_start(&walk,sg_next(walk.sg));-n=scatterwalk_clamp(&walk,len);-}-dst=scatterwalk_map(&walk);+dst=scatterwalk_next(&walk,len,&n);nx_sg=nx_build_sg_list(nx_sg,dst,&n,sglen-(nx_sg-nx_dst));-len-=n;-scatterwalk_unmap(dst);-scatterwalk_advance(&walk,n);-scatterwalk_done(&walk,SCATTERWALK_FROM_SG,len);+scatterwalk_done_src(&walk,dst,n);+len-=n;}/* update to_process */*src_len-=len;/* return the moved destination pointer */
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:06
From: Eric Biggers <redacted>
Use scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(). Use scatterwalk_done_src() and
scatterwalk_done_dst() which consolidate scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done().
Besides the new functions being a bit easier to use, this is necessary
because scatterwalk_done() is planned to be removed.
Cc: Harald Freudenberger <freude@linux.ibm.com>
Cc: Holger Dengler <dengler@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
arch/s390/crypto/aes_s390.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:07
From: Eric Biggers <redacted>
In gcm_process_assoc(), use scatterwalk_next() which consolidates
scatterwalk_clamp() and scatterwalk_map(). Use scatterwalk_done_src()
which consolidates scatterwalk_unmap(), scatterwalk_advance(), and
scatterwalk_done().
Also rename some variables to avoid implying that anything is actually
mapped (it's not), or that the loop is going page by page (it is for
now, but nothing actually requires that to be the case).
Signed-off-by: Eric Biggers <redacted>
---
arch/x86/crypto/aesni-intel_glue.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:07
From: Eric Biggers <redacted>
s5p_sg_copy_buf() open-coded a copy from/to a scatterlist using
scatterwalk_* functions that are planned for removal. Replace it with
the new functions memcpy_from_sglist() and memcpy_to_sglist() instead.
Also take the opportunity to replace calls to scatterwalk_map_and_copy()
in the same file; this eliminates the confusing 'out' argument.
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: linux-samsung-soc@vger.kernel.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
drivers/crypto/s5p-sss.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
@@ -456,34 +456,21 @@ static void s5p_free_sg_cpy(struct s5p_aes_dev *dev, struct scatterlist **sg)kfree(*sg);*sg=NULL;}-staticvoids5p_sg_copy_buf(void*buf,structscatterlist*sg,-unsignedintnbytes,intout)-{-structscatter_walkwalk;--if(!nbytes)-return;--scatterwalk_start(&walk,sg);-scatterwalk_copychunks(buf,&walk,nbytes,out);-scatterwalk_done(&walk,out,0);-}-staticvoids5p_sg_done(structs5p_aes_dev*dev){structskcipher_request*req=dev->req;structs5p_aes_reqctx*reqctx=skcipher_request_ctx(req);if(dev->sg_dst_cpy){dev_dbg(dev->dev,"Copying %d bytes of output data back to original place\n",dev->req->cryptlen);-s5p_sg_copy_buf(sg_virt(dev->sg_dst_cpy),dev->req->dst,-dev->req->cryptlen,1);+memcpy_to_sglist(dev->req->dst,0,sg_virt(dev->sg_dst_cpy),+dev->req->cryptlen);}s5p_free_sg_cpy(dev,&dev->sg_src_cpy);s5p_free_sg_cpy(dev,&dev->sg_dst_cpy);if(reqctx->mode&FLAGS_AES_CBC)memcpy_fromio(req->iv,dev->aes_ioaddr+SSS_REG_AES_IV_DATA(0),AES_BLOCK_SIZE);
@@ -1251,13 +1236,12 @@ static int s5p_hash_prepare_request(struct ahash_request *req, bool update)xmit_len-=xmit_len&(BUFLEN-1);hash_later=ctx->total-xmit_len;/* copy hash_later bytes from end of req->src *//* previous bytes are in xmit_buf, so no overwrite */-scatterwalk_map_and_copy(ctx->buffer,req->src,-req->nbytes-hash_later,-hash_later,0);+memcpy_from_sglist(ctx->buffer,req->src,+req->nbytes-hash_later,hash_later);}if(xmit_len>BUFLEN){ret=s5p_hash_prepare_sgs(ctx,req->src,nbytes-hash_later,final);
@@ -1265,12 +1249,12 @@ static int s5p_hash_prepare_request(struct ahash_request *req, bool update)returnret;}else{/* have buffered data only */if(unlikely(!ctx->bufcnt)){/* first update didn't fill up buffer */-scatterwalk_map_and_copy(ctx->dd->xmit_buf,req->src,-0,xmit_len,0);+memcpy_from_sglist(ctx->dd->xmit_buf,req->src,+0,xmit_len);}sg_init_table(ctx->sgl,1);sg_set_buf(ctx->sgl,ctx->dd->xmit_buf,xmit_len);
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:07
From: Eric Biggers <redacted>
Replace calls to the deprecated function scatterwalk_copychunks() with
memcpy_from_scatterwalk(), memcpy_to_scatterwalk(), scatterwalk_skip(),
or scatterwalk_start_at_pos() as appropriate.
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Maxime Méré <redacted>
Cc: Thomas Bourgoin <redacted>
Cc: linux-stm32@st-md-mailman.stormreply.com
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
drivers/crypto/stm32/stm32-cryp.c | 34 +++++++++++++++----------------
1 file changed, 17 insertions(+), 17 deletions(-)
@@ -991,11 +991,11 @@ static int stm32_cryp_header_dma_start(struct stm32_cryp *cryp)tx_in->callback_param=cryp;tx_in->callback=stm32_cryp_header_dma_callback;/* Advance scatterwalk to not DMA'ed data */align_size=ALIGN_DOWN(cryp->header_in,cryp->hw_blocksize);-scatterwalk_copychunks(NULL,&cryp->in_walk,align_size,2);+scatterwalk_skip(&cryp->in_walk,align_size);cryp->header_in-=align_size;ret=dma_submit_error(dmaengine_submit(tx_in));if(ret<0){dev_err(cryp->dev,"DMA in submit failed\n");
@@ -1054,22 +1054,22 @@ static int stm32_cryp_dma_start(struct stm32_cryp *cryp)tx_out->callback=stm32_cryp_dma_callback;tx_out->callback_param=cryp;/* Advance scatterwalk to not DMA'ed data */align_size=ALIGN_DOWN(cryp->payload_in,cryp->hw_blocksize);-scatterwalk_copychunks(NULL,&cryp->in_walk,align_size,2);+scatterwalk_skip(&cryp->in_walk,align_size);cryp->payload_in-=align_size;ret=dma_submit_error(dmaengine_submit(tx_in));if(ret<0){dev_err(cryp->dev,"DMA in submit failed\n");returnret;}dma_async_issue_pending(cryp->dma_lch_in);/* Advance scatterwalk to not DMA'ed data */-scatterwalk_copychunks(NULL,&cryp->out_walk,align_size,2);+scatterwalk_skip(&cryp->out_walk,align_size);cryp->payload_out-=align_size;ret=dma_submit_error(dmaengine_submit(tx_out));if(ret<0){dev_err(cryp->dev,"DMA out submit failed\n");returnret;
@@ -1735,13 +1735,13 @@ static int stm32_cryp_prepare_req(struct skcipher_request *req,in_sg=areq->src;out_sg=areq->dst;scatterwalk_start(&cryp->in_walk,in_sg);-scatterwalk_start(&cryp->out_walk,out_sg);/* In output, jump after assoc data */-scatterwalk_copychunks(NULL,&cryp->out_walk,cryp->areq->assoclen,2);+scatterwalk_start_at_pos(&cryp->out_walk,out_sg,+areq->assoclen);ret=stm32_cryp_hw_init(cryp);if(ret)returnret;
@@ -1871,16 +1871,16 @@ static int stm32_cryp_read_auth_tag(struct stm32_cryp *cryp)if(is_encrypt(cryp)){u32out_tag[AES_BLOCK_32];/* Get and write tag */readsl(cryp->regs+cryp->caps->dout,out_tag,AES_BLOCK_32);-scatterwalk_copychunks(out_tag,&cryp->out_walk,cryp->authsize,1);+memcpy_to_scatterwalk(&cryp->out_walk,out_tag,cryp->authsize);}else{/* Get and check tag */u32in_tag[AES_BLOCK_32],out_tag[AES_BLOCK_32];-scatterwalk_copychunks(in_tag,&cryp->in_walk,cryp->authsize,0);+memcpy_from_scatterwalk(in_tag,&cryp->in_walk,cryp->authsize);readsl(cryp->regs+cryp->caps->dout,out_tag,AES_BLOCK_32);if(crypto_memneq(in_tag,out_tag,cryp->authsize))ret=-EBADMSG;}
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:08
From: Eric Biggers <redacted>
In crypto_aegis128_aesni_process_ad(), use scatterwalk_next() which
consolidates scatterwalk_clamp() and scatterwalk_map(). Use
scatterwalk_done_src() which consolidates scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done().
Signed-off-by: Eric Biggers <redacted>
---
arch/x86/crypto/aegis128-aesni-glue.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:08
From: Eric Biggers <redacted>
Replace calls to the deprecated function scatterwalk_copychunks() with
memcpy_from_scatterwalk(), memcpy_to_scatterwalk(), or
scatterwalk_skip() as appropriate. The new functions generally behave
more as expected and eliminate the need to call scatterwalk_done() or
scatterwalk_pagedone().
However, the new functions intentionally do not advance to the next sg
entry right away, which would have broken chain_to_walk() which is
accessing the fields of struct scatter_walk directly. To avoid this,
replace chain_to_walk() with scatterwalk_get_sglist() which supports the
needed functionality.
Cc: Boris Pismenny <borisp@nvidia.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
net/tls/tls_device_fallback.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
@@ -67,20 +56,17 @@ static int tls_enc_record(struct aead_request *aead_req,DEBUG_NET_WARN_ON_ONCE(!cipher_desc||!cipher_desc->offloadable);buf_size=TLS_HEADER_SIZE+cipher_desc->iv;len=min_t(int,*in_len,buf_size);-scatterwalk_copychunks(buf,in,len,0);-scatterwalk_copychunks(buf,out,len,1);+memcpy_from_scatterwalk(buf,in,len);+memcpy_to_scatterwalk(out,buf,len);*in_len-=len;if(!*in_len)return0;-scatterwalk_pagedone(in,0,1);-scatterwalk_pagedone(out,1,1);-len=buf[4]|(buf[3]<<8);len-=cipher_desc->iv;tls_make_aad(aad,len-cipher_desc->tag,(char*)&rcd_sn,buf[0],prot);
@@ -88,12 +74,12 @@ static int tls_enc_record(struct aead_request *aead_req,sg_init_table(sg_in,ARRAY_SIZE(sg_in));sg_init_table(sg_out,ARRAY_SIZE(sg_out));sg_set_buf(sg_in,aad,TLS_AAD_SPACE_SIZE);sg_set_buf(sg_out,aad,TLS_AAD_SPACE_SIZE);-chain_to_walk(sg_in+1,in);-chain_to_walk(sg_out+1,out);+scatterwalk_get_sglist(in,sg_in+1);+scatterwalk_get_sglist(out,sg_out+1);*in_len-=len;if(*in_len<0){*in_len+=cipher_desc->tag;/* the input buffer doesn't contain the entire record.
@@ -108,14 +94,12 @@ static int tls_enc_record(struct aead_request *aead_req,*in_len=0;}if(*in_len){-scatterwalk_copychunks(NULL,in,len,2);-scatterwalk_pagedone(in,0,1);-scatterwalk_copychunks(NULL,out,len,2);-scatterwalk_pagedone(out,1,1);+scatterwalk_skip(in,len);+scatterwalk_skip(out,len);}len-=cipher_desc->tag;aead_request_set_crypt(aead_req,sg_in,sg_out,len,iv);
@@ -160,13 +144,10 @@ static int tls_enc_records(struct aead_request *aead_req,cpu_to_be64(rcd_sn),&in,&out,&len,prot);rcd_sn++;}while(rc==0&&len);-scatterwalk_done(&in,0,0);-scatterwalk_done(&out,1,0);-returnrc;}/* Can't use icsk->icsk_af_ops->send_check here because the ip addresses*mighthavebeenchangedbyNAT.
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:08
From: Eric Biggers <redacted>
Remove various functions that are no longer used.
Signed-off-by: Eric Biggers <redacted>
---
crypto/scatterwalk.c | 37 ------------------------------------
include/crypto/scatterwalk.h | 25 ------------------------
2 files changed, 62 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:08
From: Eric Biggers <redacted>
Convert skcipher_walk to use the new scatterwalk functions.
This includes a few changes to exactly where the different parts of the
iteration happen. For example the dcache flush that previously happened
in scatterwalk_done() now happens in scatterwalk_dst_done() or in
memcpy_to_scatterwalk(). Advancing to the next sg entry now happens
just-in-time in scatterwalk_clamp() instead of in scatterwalk_done().
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 51 ++++++++++++++++++-----------------------------
1 file changed, 19 insertions(+), 32 deletions(-)
From: Eric Biggers <ebiggers@kernel.org> Date: 2024-12-30 00:16:09
From: Eric Biggers <redacted>
When !HIGHMEM, the kmap_local_page() in the scatterlist walker does not
actually map anything, and the address it returns is just the address
from the kernel's direct map, where each sg entry's data is virtually
contiguous. To improve performance, stop unnecessarily clamping data
segments to page boundaries in this case.
For now, still limit segments to PAGE_SIZE. This is needed to prevent
preemption from being disabled for too long when SIMD is used, and to
support the alignmask case which still uses a page-sized bounce buffer.
Even so, this change still helps a lot in cases where messages cross a
page boundary. For example, testing IPsec with AES-GCM on x86_64, the
messages are 1424 bytes which is less than PAGE_SIZE, but on the Rx side
over a third cross a page boundary. These ended up being processed in
three parts, with the middle part going through skcipher_next_slow which
uses a 16-byte bounce buffer. That was causing a significant amount of
overhead which unnecessarily reduced the performance benefit of the new
x86_64 AES-GCM assembly code. This change solves the problem; all these
messages now get passed to the assembly code in one part.
Signed-off-by: Eric Biggers <redacted>
---
crypto/skcipher.c | 4 +-
include/crypto/scatterwalk.h | 79 ++++++++++++++++++++++++++----------
2 files changed, 59 insertions(+), 24 deletions(-)
From: Eric Biggers <redacted>
p10_aes_gcm_crypt() is abusing the scatter_walk API to get the virtual
address for the first source scatterlist element. But this code is only
built for PPC64 which is a !HIGHMEM platform, and it can read past a
page boundary from the address returned by scatterwalk_map() which means
it already assumes the address is from the kernel's direct map. Thus,
just use sg_virt() instead to get the same result in a simpler way.
Cc: Christophe Leroy <redacted>
Cc: Danny Tsen <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
arch/powerpc/crypto/aes-gcm-p10-glue.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
@@ -232,12 +231,11 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8 *riv,memset(ivbuf,0,sizeof(ivbuf));memcpy(iv,riv,GCM_IV_SIZE);/* Linearize assoc, if not already linear */if(req->src->length>=assoclen&&req->src->length){-scatterwalk_start(&assoc_sg_walk,req->src);-assoc=scatterwalk_map(&assoc_sg_walk);+assoc=sg_virt(req->src);/* ppc64 is !HIGHMEM */}else{gfp_tflags=(req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)?GFP_KERNEL:GFP_ATOMIC;/* assoc can be any length, so must be on heap */
From: Eric Biggers <ebiggers@kernel.org> Date: 2025-01-02 17:24:46
On Thu, Jan 02, 2025 at 12:50:50PM +0100, Christophe Leroy wrote:
Le 30/12/2024 à 01:13, Eric Biggers a écrit :
quoted
From: Eric Biggers <redacted>
p10_aes_gcm_crypt() is abusing the scatter_walk API to get the virtual
address for the first source scatterlist element. But this code is only
built for PPC64 which is a !HIGHMEM platform, and it can read past a
page boundary from the address returned by scatterwalk_map() which means
it already assumes the address is from the kernel's direct map. Thus,
just use sg_virt() instead to get the same result in a simpler way.
Cc: Christophe Leroy <redacted>
Cc: Danny Tsen <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
arch/powerpc/crypto/aes-gcm-p10-glue.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
@@ -232,12 +231,11 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8 *riv,memset(ivbuf,0,sizeof(ivbuf));memcpy(iv,riv,GCM_IV_SIZE);/* Linearize assoc, if not already linear */if(req->src->length>=assoclen&&req->src->length){-scatterwalk_start(&assoc_sg_walk,req->src);-assoc=scatterwalk_map(&assoc_sg_walk);+assoc=sg_virt(req->src);/* ppc64 is !HIGHMEM */}else{gfp_tflags=(req->base.flags&CRYPTO_TFM_REQ_MAY_SLEEP)?GFP_KERNEL:GFP_ATOMIC;/* assoc can be any length, so must be on heap */
From: Harald Freudenberger <freude@linux.ibm.com> Date: 2025-01-08 15:06:30
On 2024-12-30 01:14, Eric Biggers wrote:
quoted hunk
From: Eric Biggers <redacted>
Use scatterwalk_next() which consolidates scatterwalk_clamp() and
scatterwalk_map(). Use scatterwalk_done_src() and
scatterwalk_done_dst() which consolidate scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done().
Besides the new functions being a bit easier to use, this is necessary
because scatterwalk_done() is planned to be removed.
Cc: Harald Freudenberger <freude@linux.ibm.com>
Cc: Holger Dengler <dengler@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Signed-off-by: Eric Biggers <redacted>
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series
and
did not receive it, please retrieve it from lore.kernel.org.
arch/s390/crypto/aes_s390.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)