mbox series

[v2,0/7] crypto: sun4i-ss: prevent always fallback for ciphers

Message ID 1600627038-40000-1-git-send-email-clabbe@baylibre.com
Headers show
Series crypto: sun4i-ss: prevent always fallback for ciphers | expand

Message

Corentin Labbe Sept. 20, 2020, 6:37 p.m. UTC 
Hello

For help testing on "crypto: sun4i-ss - Fix sparse endianness markers",
I have added "stats" support like other allwinner's crypto drivers.
Seeing stats showed a clear problem, the ciphers function were not used
at all.
This is due to the not-inialized need_fallback which is "init" as true
everytime.
So basicly, since the patch introduced it, this probem hidden some bugs.

This serie fixes all hidden problems, then fix the initialization of
"need_fallback" and then add the stats like other allwinner drivers.

Regards

Changes since v1:
- patch #4 is sufficient to fix BE problem (removed todo)

Corentin Labbe (7):
  crypto: sun4i-ss: linearize buffers content must be kept
  crypto: sun4i-ss: checking sg length is not sufficient
  crypto: sun4i-ss: IV register does not work on A10 and A13
  crypto: sun4i-ss: handle BigEndian for cipher
  crypto: sun4i-ss: initialize need_fallback
  crypto: sun4i-ss: enabled stats via debugfs
  crypto: sun4i-ss: add SPDX header and remove blank lines

 drivers/crypto/allwinner/Kconfig              |  9 ++
 .../allwinner/sun4i-ss/sun4i-ss-cipher.c      | 82 ++++++++++++++-----
 .../crypto/allwinner/sun4i-ss/sun4i-ss-core.c | 54 ++++++++++++
 .../crypto/allwinner/sun4i-ss/sun4i-ss-hash.c |  8 ++
 .../crypto/allwinner/sun4i-ss/sun4i-ss-prng.c |  6 ++
 drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h  | 11 +++
 6 files changed, 148 insertions(+), 22 deletions(-)

Comments

Herbert Xu Sept. 25, 2020, 7:36 a.m. UTC | #1 
On Sun, Sep 20, 2020 at 06:37:17PM +0000, Corentin Labbe wrote:
>

> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> index f3bdf465b02e..2ec359eaa4cf 100644

> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> @@ -34,6 +34,10 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)

>  	struct sg_mapping_iter mi, mo;

>  	unsigned int oi, oo; /* offset for in and out */

>  	unsigned long flags;

> +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG

> +	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);

> +	struct sun4i_ss_alg_template *algt;

> +#endif

>  

>  	if (!areq->cryptlen)

>  		return 0;

> @@ -50,6 +54,12 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)

>  		scatterwalk_map_and_copy(backup_iv, areq->src, areq->cryptlen - ivsize, ivsize, 0);

>  	}

>  

> +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG

> +	algt = container_of(alg, struct sun4i_ss_alg_template, alg.crypto);

> +	algt->stat_opti++;

> +	algt->stat_bytes += areq->cryptlen;

> +#endif


Please do

	if (IS_ENABLED(CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG)) {
		...
	}

instead so this always gets compiled.

> +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG

> +	/* Ignore error of debugfs */

> +	ss->dbgfs_dir = debugfs_create_dir("sun4i-ss", NULL);

> +	ss->dbgfs_stats = debugfs_create_file("stats", 0444, ss->dbgfs_dir, ss,

> +					      &sun4i_ss_debugfs_fops);

> +#endif


The calls too be done under an if statement.  Only the assignments
need to use the ifdef.

> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h

> index 163962f9e284..a98a2c05089b 100644

> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h

> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h

> @@ -13,6 +13,7 @@

>  

>  #include <linux/clk.h>

>  #include <linux/crypto.h>

> +#include <linux/debugfs.h>


Please add this into the c files that actually use debugfs, and
not here.

>  #include <linux/io.h>

>  #include <linux/module.h>

>  #include <linux/of.h>

> @@ -152,6 +153,10 @@ struct sun4i_ss_ctx {

>  #ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG

>  	u32 seed[SS_SEED_LEN / BITS_PER_LONG];

>  #endif

> +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG

> +	struct dentry *dbgfs_dir;

> +	struct dentry *dbgfs_stats;

> +#endif


You just need to add forward declarations for struct dentry.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt