From: Tero Kristo <hidden> Date: 2016-06-01 08:56:01
Hi,
This series adds support for crypto hardware accelerators on TI DRA7xx
and AM43xx SoCs, and fixes a number of bugs in the existing codebase.
This series also addresses performance issues with the AES / SHA
accelerators, doing some optimizations on these.
Patch #7 and #13 are generic crypto API implementation changes.
Without #7, omap-sham export/import does not work, #13 is kind
of nice to have.
Patches 16+ should be picked-up / acked by Tony, but they have
dependencies on the preceding patches; at least the AES dual core
support must be in before applying the rest, otherwise bad things
will happen.
-Tero
From: Tero Kristo <hidden> Date: 2016-06-01 08:56:02
From: Lokesh Vutla <redacted>
Algorithms can be registered only once. So skip registration of
algorithms if already registered (i.e. in case we have two AES cores
in the system.)
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-aes.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:42
From: Lokesh Vutla <redacted>
The extra call to dmaengine_terminate_all is not needed, as the DMA
is not running at this point. This improves performance slightly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-aes.c | 2 --
drivers/crypto/omap-sham.c | 1 -
2 files changed, 3 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:42
Change crypto queue size from 1 to 10 for omap SHA driver. This should
allow clients to enqueue requests more effectively to avoid serializing
whole crypto sequences, giving extra performance.
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:44
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers can
cause a series of crypto operations divided by page. The runtime PM API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session (cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime get/put
is only called during a crypto session which completes usually quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
@@ -1239,6 +1229,7 @@ static int omap_sham_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base){structomap_sham_ctx*tctx=crypto_tfm_ctx(tfm);constchar*alg_name=crypto_tfm_alg_name(tfm);+structomap_sham_dev*dd;/* Allocate a fallback and abort if it failed. */tctx->fallback=crypto_alloc_shash(alg_name,0,
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:46
From: Bin Liu <b-liu@ti.com>
Adds software fallback support for small crypto requests. In these cases,
it is undesirable to use DMA, as setting it up itself is rather heavy
operation. Gives about 40% extra performance in ipsec usecase.
Signed-off-by: Bin Liu <b-liu@ti.com>
[t-kristo at ti.com: dropped the extra traces, updated some comments
on the code]
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -1082,7 +1082,7 @@ static int omap_sham_update(struct ahash_request *req)ctx->offset=0;if(ctx->flags&BIT(FLAGS_FINUP)){-if((ctx->digcnt+ctx->bufcnt+ctx->total)<9){+if((ctx->digcnt+ctx->bufcnt+ctx->total)<240){/**OMAPHWaccelworksonlywithbuffers>=9*willswitchtobypassinfinal()
@@ -1138,9 +1138,13 @@ static int omap_sham_final(struct ahash_request *req)if(ctx->flags&BIT(FLAGS_ERROR))return0;/* uncompleted hash is not needed */-/* OMAP HW accel works only with buffers >= 9 */-/* HMAC is always >= 9 because ipad == block size */-if((ctx->digcnt+ctx->bufcnt)<9)+/*+*OMAPHWaccelworksonlywithbuffers>=9.+*HMACisalways>=9becauseipad==blocksize.+*Ifbuffersizeislessthan240,weusefallbackSWencoding,+*asusingDMA+HWinthiscasedoesn'tprovideanybenefit.+*/+if((ctx->digcnt+ctx->bufcnt)<240)returnomap_sham_final_shash(req);elseif(ctx->bufcnt)returnomap_sham_enqueue(req,OP_FINAL);
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:47
The statesize is used to determine the maximum size for saved ahash
context. In some cases, this can be much larger than what is currently
allocated for it, for example omap-sham driver uses a buffer size of
PAGE_SIZE. Increase the statesize to accommodate this.
Signed-off-by: Tero Kristo <redacted>
---
crypto/ahash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:48
Some of the call paths of OMAP SHA driver can avoid executing the next
step of the crypto queue under tasklet; instead, execute the next step
directly via function call. This avoids a costly round-trip via the
scheduler giving a slight performance boost.
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:51
Context export/import are now required for ahash algorithms due to
required support in algif_hash. Implement these for OMAP SHA driver,
saving and restoring the internal state of the driver.
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:53
From: Lokesh Vutla <redacted>
For cases where total length of an input SGs is not same as
length of the input data for encryption, omap-des driver
crashes. This happens in the case when IPsec is trying to use
omap-des driver.
To avoid this, we copy all the pages from the input SG list
into a contiguous buffer and prepare a single element SG list
for this buffer with length as the total bytes to crypt, which is
similar thing that is done in case of unaligned lengths.
Signed-off-by: Lokesh Vutla <redacted>
Tested-by: Aparna Balasubramanian <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-des.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
@@ -521,29 +521,36 @@ static int omap_des_crypt_dma_stop(struct omap_des_dev *dd)return0;}-staticintomap_des_copy_needed(structscatterlist*sg)+staticintomap_des_copy_needed(structscatterlist*sg,inttotal){+intlen=0;++if(!IS_ALIGNED(total,DES_BLOCK_SIZE))+return-1;+while(sg){if(!IS_ALIGNED(sg->offset,4))return-1;if(!IS_ALIGNED(sg->length,DES_BLOCK_SIZE))return-1;++len+=sg->length;sg=sg_next(sg);}++if(len!=total)+return-1;+return0;}staticintomap_des_copy_sgs(structomap_des_dev*dd){void*buf_in,*buf_out;-intpages;--pages=dd->total>>PAGE_SHIFT;--if(dd->total&(PAGE_SIZE-1))-pages++;+intpages,total;-BUG_ON(!pages);+total=ALIGN(dd->total,DES_BLOCK_SIZE);+pages=get_order(total);buf_in=(void*)__get_free_pages(GFP_ATOMIC,pages);buf_out=(void*)__get_free_pages(GFP_ATOMIC,pages);
@@ -595,8 +602,8 @@ static int omap_des_prepare_req(struct crypto_engine *engine,dd->in_sg=req->src;dd->out_sg=req->dst;-if(omap_des_copy_needed(dd->in_sg)||-omap_des_copy_needed(dd->out_sg)){+if(omap_des_copy_needed(dd->in_sg,dd->total)||+omap_des_copy_needed(dd->out_sg,dd->total)){if(omap_des_copy_sgs(dd))pr_err("Failed to copy SGs for unaligned cases\n");dd->sgs_copied=1;
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:53
From: Lokesh Vutla <redacted>
Enable clocks for all cores before starting session.
Driver has to pic the aes core dynamically based on the queue length.
Signed-off-by: Lokesh Vutla <redacted>
---
drivers/crypto/omap-aes.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
@@ -760,18 +760,13 @@ static int omap_aes_cra_init(struct crypto_tfm *tfm)structomap_aes_dev*dd=NULL;interr;-/* Find AES device, currently picks the first device */-spin_lock_bh(&list_lock);list_for_each_entry(dd,&dev_list,list){-break;-}-spin_unlock_bh(&list_lock);--err=pm_runtime_get_sync(dd->dev);-if(err<0){-dev_err(dd->dev,"%s: failed to get_sync(%d)\n",-__func__,err);-returnerr;+err=pm_runtime_get_sync(dd->dev);+if(err<0){+dev_err(dd->dev,"%s: failed to get_sync(%d)\n",+__func__,err);+returnerr;+}}tfm->crt_ablkcipher.reqsize=sizeof(structomap_aes_reqctx);
@@ -783,14 +778,10 @@ static void omap_aes_cra_exit(struct crypto_tfm *tfm){structomap_aes_dev*dd=NULL;-/* Find AES device, currently picks the first device */-spin_lock_bh(&list_lock);list_for_each_entry(dd,&dev_list,list){-break;+pm_runtime_put_sync(dd->dev);}-spin_unlock_bh(&list_lock);-pm_runtime_put_sync(dd->dev);}/* ********************** ALGS ************************************ */
From: Tero Kristo <hidden> Date: 2016-06-01 08:57:58
From: Lokesh Vutla <redacted>
Some SoCs like omap4/omap5/dra7 contain multiple AES crypto accelerator
cores. Adapt the driver to support this. The driver picks the last used
device from a list of AES devices.
Signed-off-by: Lokesh Vutla <redacted>
[t-kristo at ti.com: forward ported to 4.7 kernel]
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-aes.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
@@ -319,20 +319,12 @@ static void omap_aes_dma_stop(struct omap_aes_dev *dd)staticstructomap_aes_dev*omap_aes_find_dev(structomap_aes_ctx*ctx){-structomap_aes_dev*dd=NULL,*tmp;+structomap_aes_dev*dd;spin_lock_bh(&list_lock);-if(!ctx->dd){-list_for_each_entry(tmp,&dev_list,list){-/* FIXME: take fist available aes core */-dd=tmp;-break;-}-ctx->dd=dd;-}else{-/* already found before */-dd=ctx->dd;-}+dd=list_first_entry(&dev_list,structomap_aes_dev,list);+list_move_tail(&dd->list,&dev_list);+ctx->dd=dd;spin_unlock_bh(&list_lock);returndd;
@@ -600,7 +592,7 @@ static int omap_aes_prepare_req(struct crypto_engine *engine,{structomap_aes_ctx*ctx=crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));-structomap_aes_dev*dd=omap_aes_find_dev(ctx);+structomap_aes_dev*dd=ctx->dd;structomap_aes_reqctx*rctx;intlen;
@@ -644,7 +636,7 @@ static int omap_aes_crypt_req(struct crypto_engine *engine,{structomap_aes_ctx*ctx=crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));-structomap_aes_dev*dd=omap_aes_find_dev(ctx);+structomap_aes_dev*dd=ctx->dd;if(!dd)return-ENODEV;
From: Tero Kristo <hidden> Date: 2016-06-01 08:58:05
From: Lokesh Vutla <redacted>
As setting up the DMA operations is quite costly, add software fallback
support for requests smaller than 200 bytes. This change gives some 10%
extra performance in ipsec use case.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/Kconfig | 3 +++
drivers/crypto/omap-aes.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 45 insertions(+), 3 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:58:16
Crypto engine will now hi-jack the currently running thread for executing
crypto functionality. Only if we are not running a thread (in interrupt
context) the kthread will be scheduled.
This will improve performance of crypto operations using crypto engine.
Signed-off-by: Tero Kristo <redacted>
---
crypto/crypto_engine.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:58:16
The crypto engine must be initialized before registering algorithms,
otherwise the test manager will crash as it attempts to execute
tests for the algos while they are being registered.
Fixes: f1b77aaca85a ("crypto: omap-des - Integrate with the crypto engine framework")
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-des.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 08:58:16
The crypto engine must be initialized before registering algorithms,
otherwise the test manager will crash as it attempts to execute
tests for the algos while they are being registered.
Fixes: 0529900a01cb ("crypto: omap-aes - Support crypto engine framework")
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-aes.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
From: Tero Kristo <hidden> Date: 2016-06-01 09:05:31
From: Joel Fernandes <redacted>
DRA7 SoC contains DES crypto hardware accelerator. Add hwmod data for
this IP so that it can be utilized by crypto frameworks.
Signed-off-by: Joel Fernandes <redacted>
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 37 +++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:12
From: Joel Fernandes <redacted>
DRA7 SoC contains AES crypto hardware accelerator. Add hwmod data for
this IP so that it can be utilized by crypto frameworks.
Signed-off-by: Joel Fernandes <redacted>
Signed-off-by: Lokesh Vutla <redacted>
[t-kristo at ti.com: squash in support for both AES1 and AES2 cores]
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 62 +++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:23
From: Lokesh Vutla <redacted>
DRA7 SoC contains SHA crypto hardware accelerator. Add hwmod data for
this IP so that it can be utilized by crypto frameworks.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 37 +++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:26
From: Joel Fernandes <redacted>
DRA7 SoC contains hardware random number generator. Add hwmod data for
this IP so that it can be utilized.
Signed-off-by: Joel Fernandes <redacted>
Signed-off-by: Lokesh Vutla <redacted>
[t-kristo at ti.com: squashed the RNG hwmod IP flag fixes from Lokesh,
squashed the HS chip fix from Daniel Allred]
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:42
From: Lokesh Vutla <redacted>
AM43xx SoC contains DES crypto hardware accelerator. Add hwmod data for
this IP so that it can be utilized by crypto frameworks.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/omap_hwmod_43xx_data.c | 33 ++++++++++++++++++++++++++++++
arch/arm/mach-omap2/prcm43xx.h | 1 +
2 files changed, 34 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:46
From: Joel Fernandes <redacted>
Using HWSUP for l4sec clock domain is causing warnings in HWMOD code for
DRA7. Based on some observations, once the clock domain goes into an IDLE
state (because of no activity etc), the IDLEST for the module goes to '0x2'
value which means Interface IDLE condition. So far so go, however once the
MODULEMODE is set to disabled for the particular IP, the IDLEST for the
module should go to '0x3', per the HW AUTO IDLE protocol. However this is
not observed and there is no reason per the protocl for the transition to
not happen. This could potentially be a bug in the HW AUTO state-machine.
Work around for this is to use SWSUP only for the particular clockdomain.
With this all the transitions of IDLEST happen correctly and warnings
don't occur.
Signed-off-by: Joel Fernandes <redacted>
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:49
From: Joel Fernandes <redacted>
DRA7 SoC has the same AES IP as OMAP4. Add DT entries for both AES cores.
Signed-off-by: Joel Fernandes <redacted>
Signed-off-by: Lokesh Vutla <redacted>
[t-kristo at ti.com: squashed in the change to use EDMA, squashed in
support for two AES cores]
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/boot/dts/dra7.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:50
From: Joel Fernandes <redacted>
DRA7xx SoCs have a DES3DES IP. Add DT data for the same.
Signed-off-by: Joel Fernandes <redacted>
---
arch/arm/boot/dts/dra7.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
From: Tero Kristo <hidden> Date: 2016-06-01 09:07:53
From: Lokesh Vutla <redacted>
DRA7 SoC has the same SHA IP as OMAP5. Add DT entry for the same.
Signed-off-by: Lokesh Vutla <redacted>
[t-kristo at ti.com: changed SHA to use EDMA instead of SDMA]
Signed-off-by: Tero Kristo <redacted>
---
arch/arm/boot/dts/dra7.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers can
cause a series of crypto operations divided by page. The runtime PM API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session (cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime get/put
is only called during a crypto session which completes usually quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
@@ -1239,6 +1229,7 @@ static int omap_sham_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base){structomap_sham_ctx*tctx=crypto_tfm_ctx(tfm);constchar*alg_name=crypto_tfm_alg_name(tfm);+structomap_sham_dev*dd;/* Allocate a fallback and abort if it failed. */tctx->fallback=crypto_alloc_shash(alg_name,0,
From: Dave Gerlach <hidden> Date: 2016-06-01 23:04:37
On 06/01/2016 04:53 AM, Grygorii Strashko wrote:
On 06/01/2016 11:56 AM, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers can
cause a series of crypto operations divided by page. The runtime PM API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session (cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime get/put
is only called during a crypto session which completes usually quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
ahash_request *req)
static int omap_sham_hw_init(struct omap_sham_dev *dd)
{
- int err;
-
- err = pm_runtime_get_sync(dd->dev);
- if (err < 0) {
- dev_err(dd->dev, "failed to get sync: %d\n", err);
- return err;
- }
-
Would it be worth it to investigate a pm_runtime autosuspend approach
rather than knocking runtime PM out here completely? I am not clear if
the overhead is coming from the pm_runtime calls themselves or the
actual idling of the IP, but if it's the idling of the IP causing the
slowdown, with a large enough autosuspend_delay we don't actually sleep
between each block but after a long enough period of idle time we would
actually suspend.
Regards,
Dave
quoted
if (!test_bit(FLAGS_INIT, &dd->flags)) {
set_bit(FLAGS_INIT, &dd->flags);
dd->err = 0;
From: Herbert Xu <herbert@gondor.apana.org.au> Date: 2016-06-07 10:08:37
On Wed, Jun 01, 2016 at 06:03:52PM -0500, Dave Gerlach wrote:
On 06/01/2016 04:53 AM, Grygorii Strashko wrote:
quoted
On 06/01/2016 11:56 AM, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers can
cause a series of crypto operations divided by page. The runtime PM API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session (cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime get/put
is only called during a crypto session which completes usually quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
ahash_request *req)
static int omap_sham_hw_init(struct omap_sham_dev *dd)
{
- int err;
-
- err = pm_runtime_get_sync(dd->dev);
- if (err < 0) {
- dev_err(dd->dev, "failed to get sync: %d\n", err);
- return err;
- }
-
Would it be worth it to investigate a pm_runtime autosuspend
approach rather than knocking runtime PM out here completely? I am
not clear if the overhead is coming from the pm_runtime calls
themselves or the actual idling of the IP, but if it's the idling of
the IP causing the slowdown, with a large enough autosuspend_delay
we don't actually sleep between each block but after a long enough
period of idle time we would actually suspend.
From: Herbert Xu <herbert@gondor.apana.org.au> Date: 2016-06-07 10:48:37
On Wed, Jun 01, 2016 at 11:56:02AM +0300, Tero Kristo wrote:
From: Lokesh Vutla <redacted>
Algorithms can be registered only once. So skip registration of
algorithms if already registered (i.e. in case we have two AES cores
in the system.)
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
From: Tero Kristo <hidden> Date: 2016-06-07 11:53:51
On 07/06/16 13:08, Herbert Xu wrote:
On Wed, Jun 01, 2016 at 06:03:52PM -0500, Dave Gerlach wrote:
quoted
On 06/01/2016 04:53 AM, Grygorii Strashko wrote:
quoted
On 06/01/2016 11:56 AM, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers can
cause a series of crypto operations divided by page. The runtime PM API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session (cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime get/put
is only called during a crypto session which completes usually quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
ahash_request *req)
static int omap_sham_hw_init(struct omap_sham_dev *dd)
{
- int err;
-
- err = pm_runtime_get_sync(dd->dev);
- if (err < 0) {
- dev_err(dd->dev, "failed to get sync: %d\n", err);
- return err;
- }
-
Would it be worth it to investigate a pm_runtime autosuspend
approach rather than knocking runtime PM out here completely? I am
not clear if the overhead is coming from the pm_runtime calls
themselves or the actual idling of the IP, but if it's the idling of
the IP causing the slowdown, with a large enough autosuspend_delay
we don't actually sleep between each block but after a long enough
period of idle time we would actually suspend.
Indeed, I think this patch is bogus. cra_init is associated
with the tfm object which is usually long-lived. So doing power
management there makes no sense.
Cheers,
I can investigate this further, but I believe this patch itself gave a
noticeable performance boost.
This is an optimization anyway, and not critical for functionality.
-Tero
On Wed, Jun 01, 2016 at 06:03:52PM -0500, Dave Gerlach wrote:
quoted
On 06/01/2016 04:53 AM, Grygorii Strashko wrote:
quoted
On 06/01/2016 11:56 AM, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers
can
cause a series of crypto operations divided by page. The runtime PM
API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session
(cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime
get/put
is only called during a crypto session which completes usually
quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
ahash_request *req)
static int omap_sham_hw_init(struct omap_sham_dev *dd)
{
- int err;
-
- err = pm_runtime_get_sync(dd->dev);
- if (err < 0) {
- dev_err(dd->dev, "failed to get sync: %d\n", err);
- return err;
- }
-
Would it be worth it to investigate a pm_runtime autosuspend
approach rather than knocking runtime PM out here completely? I am
not clear if the overhead is coming from the pm_runtime calls
themselves or the actual idling of the IP, but if it's the idling of
the IP causing the slowdown, with a large enough autosuspend_delay
we don't actually sleep between each block but after a long enough
period of idle time we would actually suspend.
Indeed, I think this patch is bogus. cra_init is associated
with the tfm object which is usually long-lived. So doing power
management there makes no sense.
Cheers,
I can investigate this further, but I believe this patch itself gave a
noticeable performance boost.
This is an optimization anyway, and not critical for functionality.
It is not critical only if below code would not introduce races
+ spin_lock_bh(&sham.lock);
+ list_for_each_entry(dd, &sham.dev_list, list) {
+ break;
+ }
+ spin_unlock_bh(&sham.lock);
Is it guaranteed that dd will alive always at this moment?
+
+ pm_runtime_get_sync(dd->dev);
--
regards,
-grygorii
From: Tero Kristo <hidden> Date: 2016-06-20 12:11:54
On 07/06/16 13:48, Herbert Xu wrote:
On Wed, Jun 01, 2016 at 11:56:02AM +0300, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Algorithms can be registered only once. So skip registration of
algorithms if already registered (i.e. in case we have two AES cores
in the system.)
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
Patch applied. Thanks.
Thanks,
Did you check the rest of the series? I only got feedback for this and
patch #2 on the series, shall I repost the remainder of the series as a
whole or...?
-Tero
From: Herbert Xu <herbert@gondor.apana.org.au> Date: 2016-06-20 23:49:29
On Mon, Jun 20, 2016 at 03:11:54PM +0300, Tero Kristo wrote:
Did you check the rest of the series? I only got feedback for this
and patch #2 on the series, shall I repost the remainder of the
series as a whole or...?
From: Tero Kristo <hidden> Date: 2016-06-21 17:56:44
On 10/06/16 14:38, Tony Lindgren wrote:
* Tero Kristo [off-list ref] [160601 02:09]:
quoted
From: Joel Fernandes <redacted>
DRA7xx SoCs have a DES3DES IP. Add DT data for the same.
Are these dts changes safe to apply separately or do they
cause issues like extra warnings during boot?
DTS changes are fine to merge as is separately, some crypto
functionality might not work properly though (well, the support is
somewhat broken on am43xx/dra7 anyways.) I just gave the kernel a boot
test with am43xx/dra7 platforms with the DT data only applied, and that
part worked fine.
I noticed that if you merge the hwmod changes before you have the DTS
data though, you will get some extra boot warnings of the following kind
on am43xx/dra7:
<snip>
[ 0.315623] omap_hwmod: aes1: no dt node
[ 0.315631] ------------[ cut here ]------------
[ 0.315656] WARNING: CPU: 0 PID: 1 at
arch/arm/mach-omap2/omap_hwmod.c:2497 _init+0x1d0/0x41c
[ 0.315663] omap_hwmod: aes1: doesn't have mpu register target base
<snip>
Do you want to pick-up the DTS changes from this revision of series as
is or shall I repost those also? I think the series would require a
re-ordering of posting the DTS changes before the hwmod data.
-Tero
From: Tony Lindgren <tony@atomide.com> Date: 2016-06-22 07:58:40
* Tero Kristo [off-list ref] [160621 10:58]:
Do you want to pick-up the DTS changes from this revision of series as is or
shall I repost those also? I think the series would require a re-ordering of
posting the DTS changes before the hwmod data.
I'll pick the dts changes from this series into omap-for-v4.8/dt
thanks. Please split the rest of the patches into separate hwmod
and driver changes and describe if there's a dependency with the
order they should get merged.
Regards,
Tony
From: Tero Kristo <hidden> Date: 2016-06-22 09:17:09
On 07/06/16 15:24, Grygorii Strashko wrote:
On 06/07/2016 02:52 PM, Tero Kristo wrote:
quoted
On 07/06/16 13:08, Herbert Xu wrote:
quoted
On Wed, Jun 01, 2016 at 06:03:52PM -0500, Dave Gerlach wrote:
quoted
On 06/01/2016 04:53 AM, Grygorii Strashko wrote:
quoted
On 06/01/2016 11:56 AM, Tero Kristo wrote:
quoted
From: Lokesh Vutla <redacted>
Calling runtime PM API for every block causes serious perf hit to
crypto operations that are done on a long buffer.
As crypto is performed on a page boundary, encrypting large buffers
can
cause a series of crypto operations divided by page. The runtime PM
API
is also called those many times.
We call runtime_pm_get_sync only at beginning on the session
(cra_init)
and runtime_pm_put at the end. This result in upto a 50% speedup.
This doesn't make the driver to keep the system awake as runtime
get/put
is only called during a crypto session which completes usually
quickly.
Signed-off-by: Lokesh Vutla <redacted>
Signed-off-by: Tero Kristo <redacted>
---
drivers/crypto/omap-sham.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
ahash_request *req)
static int omap_sham_hw_init(struct omap_sham_dev *dd)
{
- int err;
-
- err = pm_runtime_get_sync(dd->dev);
- if (err < 0) {
- dev_err(dd->dev, "failed to get sync: %d\n", err);
- return err;
- }
-
Would it be worth it to investigate a pm_runtime autosuspend
approach rather than knocking runtime PM out here completely? I am
not clear if the overhead is coming from the pm_runtime calls
themselves or the actual idling of the IP, but if it's the idling of
the IP causing the slowdown, with a large enough autosuspend_delay
we don't actually sleep between each block but after a long enough
period of idle time we would actually suspend.
Indeed, I think this patch is bogus. cra_init is associated
with the tfm object which is usually long-lived. So doing power
management there makes no sense.
Cheers,
I can investigate this further, but I believe this patch itself gave a
noticeable performance boost.
This is an optimization anyway, and not critical for functionality.
It is not critical only if below code would not introduce races
I don't get your point here. This patch is an optimization, and the
driver works fine without it.
+ spin_lock_bh(&sham.lock);
+ list_for_each_entry(dd, &sham.dev_list, list) {
+ break;
+ }
+ spin_unlock_bh(&sham.lock);
Is it guaranteed that dd will alive always at this moment?
Typically yes, but I think there might be a race condition here if the
driver is removed during operation. Anyway, I'll drop this patch and
change the optimization to use autosuspend as Dave suggested; that gives
almost the same performance boost as this one (I miss a couple of
percent in the overall performance, but I can live with that.)
-Tero