Re: [PATCH v2] crypto: caam - fix IV loading for authenc (giv)decryption

[Horia Ioan Geanta Neag]

On 8/26/2016 6:33 PM, Horia Geantă wrote:
> For algorithms that implement IV generators before the crypto ops,
> the IV needed for decryption is initially located in req->src
> scatterlist, not in req->iv.
> 
> Avoid copying the IV into req->iv by modifying the (givdecrypt)
> descriptors to load it directly from req->src.
> aead_givdecrypt() is no longer needed and goes away.
> 
> Cc:  # 4.3+
> Fixes: 479bcc7c5b9e ("crypto: caam - Convert authenc to new AEAD interface")
> Signed-off-by: Horia Geantă 
> ---
> 
>  drivers/crypto/caam/caamalg.c | 77 
> +--
>  1 file changed, 37 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index 6dc597126b79..775b8b524913 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -556,7 +556,10 @@ skip_enc:
>  
>   /* Read and write assoclen bytes */
>   append_math_add(desc, VARSEQINLEN, ZERO, REG3, CAAM_CMD_SZ);
> - append_math_add(desc, VARSEQOUTLEN, ZERO, REG3, CAAM_CMD_SZ);
> + if (alg->caam.geniv)
> + append_math_add_imm_u32(desc, VARSEQOUTLEN, REG3, IMM, ivsize);
> + else
> + append_math_add(desc, VARSEQOUTLEN, ZERO, REG3, CAAM_CMD_SZ);
>  
>   /* Skip assoc data */
>   append_seq_fifo_store(desc, 0, FIFOST_TYPE_SKIP | FIFOLDST_VLF);
> @@ -565,6 +568,14 @@ skip_enc:
>   append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS2 | FIFOLD_TYPE_MSG |
>KEY_VLF);
>  
> + if (alg->caam.geniv) {
> + append_seq_load(desc, ivsize, LDST_CLASS_1_CCB |
> + LDST_SRCDST_BYTE_CONTEXT |
> + (ctx1_iv_off << LDST_OFFSET_SHIFT));
> + append_move(desc, MOVE_SRC_CLASS1CTX | MOVE_DEST_CLASS2INFIFO |
> + (ctx1_iv_off << MOVE_OFFSET_SHIFT) | ivsize);
> + }
> +
>   /* Load Counter into CONTEXT1 reg */
>   if (is_rfc3686)
>   append_load_imm_u32(desc, be32_to_cpu(1), LDST_IMM |
> @@ -2150,7 +2161,7 @@ static void init_authenc_job(struct aead_request *req,
>  
>   init_aead_job(req, edesc, all_contig, encrypt);
>  
> - if (ivsize && (is_rfc3686 || !(alg->caam.geniv && encrypt)))
> + if (ivsize && !alg->caam.geniv)

This condition update is incorrect, since IV won't be loaded neither
here nor in the givencrypt aead descriptor for rfc3686 case.
I'll send v3 shortly.

>   append_load_as_imm(desc, req->iv, ivsize,
>  LDST_CLASS_1_CCB |
>  LDST_SRCDST_BYTE_CONTEXT |
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

caam - IV source for IPsec decryption

[Horia Ioan Geanta Neag]

Herbert,

Commits
7021b2e1cddd "esp4: Switch to new AEAD interface"
000ae7b2690e "esp6: Switch to new AEAD interface"
removed the following:
/* Get ivec. This can be wrong, check against another impls. */
iv = esph->enc_data;
from IPsec decryption - esp{4,6}_input(),
so the IV in req->iv received by the implementer is no longer valid.

Thus, the load of IV in caam driver - caamalg.c, init_authenc_job():
if (ivsize && (is_rfc3686 || !(alg->caam.geniv && encrypt)))
append_load_as_imm(desc, req->iv, ivsize,
   LDST_CLASS_1_CCB |
   LDST_SRCDST_BYTE_CONTEXT |
   (ivoffset << LDST_OFFSET_SHIFT));
is not suited for case mentioned above.

Instead, the IV should be read from the req->src scatterlist
(which consists of assoc data, iv, ciphertext).
Please let me know if this is accurate, so I could prepare a fix.

Thanks,
Horia
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

caam - IV source for IPsec decryption

[Horia Ioan Geanta Neag]

Herbert,

Commits
7021b2e1cddd "esp4: Switch to new AEAD interface"
000ae7b2690e "esp6: Switch to new AEAD interface"
removed the following:
/* Get ivec. This can be wrong, check against another impls. */
iv = esph->enc_data;
from IPsec decryption - esp{4,6}_input(),
so the IV in req->iv received by the implementer is no longer valid.

Thus, the load of IV in caam driver - caamalg.c, init_authenc_job():
if (ivsize && (is_rfc3686 || !(alg->caam.geniv && encrypt)))
append_load_as_imm(desc, req->iv, ivsize,
   LDST_CLASS_1_CCB |
   LDST_SRCDST_BYTE_CONTEXT |
   (ivoffset << LDST_OFFSET_SHIFT));
is not suited for case mentioned above.

Instead, the IV should be read from the req->src scatterlist
(which consists of assoc data, iv, ciphertext).
Please let me know if this is accurate, so I could prepare a fix.

Thanks,
Horia
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: caam - replace deprecated EXTRA_CFLAGS

[Horia Ioan Geanta Neag]

On 6/16/2016 4:33 PM, Tudor Ambarus wrote:
> EXTRA_CFLAGS is still supported but its usage is deprecated.
> 
> Signed-off-by: Tudor Ambarus 

Reviewed-by: Horia Geantă 

> ---
>  drivers/crypto/caam/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 399ad55..3e9d3e1 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -2,7 +2,7 @@
>  # Makefile for the CAAM backend and dependent components
>  #
>  ifeq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG), y)
> - EXTRA_CFLAGS := -DDEBUG
> + ccflags-y := -DDEBUG
>  endif
>  
>  ccflags-y += -I$(srctree)/crypto
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: caam: fix misspelled upper_32_bits

[Horia Ioan Geanta Neag]

On 6/16/2016 12:04 PM, Arnd Bergmann wrote:
> An endianess fix mistakenly used higher_32_bits() instead of
> upper_32_bits(), and that doesn't exist:
> 
> drivers/crypto/caam/desc_constr.h: In function 'append_ptr':
> drivers/crypto/caam/desc_constr.h:84:75: error: implicit declaration of 
> function 'higher_32_bits' [-Werror=implicit-function-declaration]
>   *offset = cpu_to_caam_dma(ptr);
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 261ea058f016 ("crypto: caam - handle core endianness != caam 
> endianness")

Oops... thanks Arnd!
Reviewed-by: Horia Geantă 

> ---
>  drivers/crypto/caam/regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> index 8c766cf9202c..b3c5016f6458 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -183,10 +183,10 @@ static inline u64 rd_reg64(void __iomem *reg)
>  #ifdef CONFIG_SOC_IMX7D
>  #define cpu_to_caam_dma(value) \
>   (((u64)cpu_to_caam32(lower_32_bits(value)) << 32) | \
> -  (u64)cpu_to_caam32(higher_32_bits(value)))
> +   (u64)cpu_to_caam32(upper_32_bits(value)))
>  #define caam_dma_to_cpu(value) \
>   (((u64)caam32_to_cpu(lower_32_bits(value)) << 32) | \
> -  (u64)caam32_to_cpu(higher_32_bits(value)))
> +   (u64)caam32_to_cpu(upper_32_bits(value)))
>  #else
>  #define cpu_to_caam_dma(value) cpu_to_caam64(value)
>  #define caam_dma_to_cpu(value) caam64_to_cpu(value)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: HWCAP_CRYPTO define for ARMv8?

[Horia Ioan Geanta Neag]

On 5/15/2016 1:13 PM, Jeffrey Walton wrote:
> Hi Everyone,
> 
> It appears defines like HWCAP_CRC32 fall under the purview of the
> kernel. Confer, http://www.google.com/search?q="HWCAP_CRC32; (my
> apologies if this is not the case).
> 
> We use getauxval(AT_HWCAP) and HWCAP_CRC32 for runtime detection of
> processor support for CRC. However, I can't find a similar symbol in
> the context of the crypto instructions. Confer,
> http://www.google.com/search?q="HWCAP_CRYPTO;.
> 
> My question is, what are the equivalent defines for Crypto features?
> 
Take a look at arch/arm64/include/uapi/asm/hwcap.h
#define HWCAP_AES   (1 << 3)
#define HWCAP_PMULL (1 << 4)
#define HWCAP_SHA1  (1 << 5)
#define HWCAP_SHA2  (1 << 6)

Horia

> Thanks in advance.
> 
> *
> 
> Below is from a 64-bit LeMaker HiKey
> (http://www.lemaker.org/product-hikey-index.html). It responds to
> getauxval(AT_HWCAP) and HWCAP_CRC32.
> 
> $ cat /proc/cpuinfo
> Processor: AArch64 Processor rev 3 (aarch64)
> processor: 0
> ...
> processor: 7
> Features: fp asimd evtstrm aes pmull sha1 sha2 crc32
> CPU implementer: 0x41
> CPU architecture: AArch64
> CPU variant: 0x0
> CPU part: 0xd03
> CPU revision: 3
> 
> Hardware: HiKey Development Board
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] crypto: caam - add support for RSA algorithm

[Horia Ioan Geanta Neag]

On 5/19/2016 3:15 PM, Tudor Ambarus wrote:
> Depends on:
> [PATCH v2] crypto: rsa - return raw integers for the ASN.1 parser
> 
> Changes in v6:
> - write descriptor PDB fields with inline append
> - move Protocol Data Block (pdb) structures to pdb.h
> - move setting of PDB fields in new functions
> - unmap sec4_sg_dma on done callback
> - remove redundant clean code on error path
> - fix doc typos
> 
> Changes in v5:
> - sync with ASN.1 parser
> 
> Changes in v4:
> - sync with ASN.1 parser
> 
> Changes in v3:
> - sync with ASN.1 parser
> 
> Changes in v2:
> - fix memory leaks on error path
> - rename struct akcipher_alg rsa to caam_rsa
> 
> 
> Tudor Ambarus (3):
>   crypto: scatterwak - Add scatterwalk_sg_copychunks
>   crypto: scatterwalk - export scatterwalk_pagedone
>   crypto: caam - add support for RSA algorithm
> 
>  crypto/scatterwalk.c  |  31 ++-
>  drivers/crypto/caam/Kconfig   |  12 +
>  drivers/crypto/caam/Makefile  |   4 +
>  drivers/crypto/caam/caampkc.c | 569 
> ++
>  drivers/crypto/caam/caampkc.h |  56 
>  drivers/crypto/caam/desc.h|   2 +
>  drivers/crypto/caam/desc_constr.h |   7 +
>  drivers/crypto/caam/pdb.h |  51 +++-
>  drivers/crypto/caam/pkc_desc.c|  35 +++
>  include/crypto/scatterwalk.h  |   4 +
>  10 files changed, 768 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/crypto/caam/caampkc.c
>  create mode 100644 drivers/crypto/caam/caampkc.h
>  create mode 100644 drivers/crypto/caam/pkc_desc.c
> 
Checked that when adding on top the patch set from here:
[PATCH v3 0/8] crypto: caam - add support for LS1043A SoC
https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg19096.html

RSA works on LS1043A, so

Tested-by: Horia Geantă 

Horia

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 3/3] crypto: caam - add support for RSA algorithm

[Horia Ioan Geanta Neag]

On 5/19/2016 3:15 PM, Tudor Ambarus wrote:
> Add RSA support to caam driver.
> 
> Coauthored-by: Yashpal Dutta 
> 
> Signed-off-by: Tudor Ambarus 

Reviewed-by: Horia Geantă 

Horia

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/8] crypto: caam - add support for LS1043A SoC

[Horia Ioan Geanta Neag]

On 5/5/2016 6:34 PM, Horia Geantă wrote:
> v2:
> As suggested by Arnd, patch 1 fixes io{read,write}{16,32}be accessors
> to prevent the case when {read,write}{w,l} are overriden by arch-specific
> ones having barriers, while the BE accessors previously mentioned are not
> (thus behaving differently, having no barriers).
> 
> Hi,
> 
> [Patches 2-4 add io{read,write}64[be] accessors (generic, arm64, ppc64),
> such that CAAM's accessors in regs.h are simplified a bit.
> Patch 8 adds crypto node for LS1043A platform.
> Let me know if it's ok to go with these through the cryptodev-2.6 tree.]
> 
Herbert,

Could you apply the patch set, but without patch 8/8 I guess - since DT
maintainers haven't ack-ed it?

I assume it's too late for 4.7, however applying the patches would solve
dependencies b/w on-going caam development.

Thanks,
Horia


> This is a follow-up on the following RFC patch set:
> crypto: caam - Revamp I/O accessors
> https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg15878.html
> 
> There are platforms such as LS1043A (or LS1012A) where core endianness
> does not match CAAM/SEC endianness (LE vs. BE).
> Add support in caam driver for these cases.
> 
> Current patch set detects device endianness at runtime (as opposed to
> compile-time endianness), in order to support multiplatform kernels.
> Detection of device endianness is not device-tree based.
> Instead, SSTA ("SEC STAtus") register has a property such that
> reading it in any endianness and masking it properly, it's possible
> to deduce device endianness.
> 
> The performance drop due to the runtime detection is < 1.0%.
> (An alternative implementation using function pointers has been tried,
> but lead to a bigger performance drop.)
> 
> Thanks,
> Horia
> 
> Cristian Stoica (1):
>   crypto: caam - fix offset field in hw sg entries
> 
> Horia Geantă (7):
>   asm-generic/io.h: allow barriers in io{read,write}{16,32}be
>   asm-generic/io.h: add io{read,write}64 accessors
>   arm64: add io{read,write}64be accessors
>   powerpc: add io{read,write}64 accessors
>   crypto: caam - handle core endianness != caam endianness
>   crypto: caam - add ARCH_LAYERSCAPE to supported architectures
>   arm64: dts: ls1043a: add crypto node
> 
>  arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts |   4 +
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi|  43 ++
>  arch/arm64/include/asm/io.h   |   4 +-
>  arch/powerpc/kernel/iomap.c   |  24 
>  drivers/crypto/caam/Kconfig   |   6 +-
>  drivers/crypto/caam/caamhash.c|   5 +-
>  drivers/crypto/caam/ctrl.c| 125 +++---
>  drivers/crypto/caam/desc.h|   9 +-
>  drivers/crypto/caam/desc_constr.h |  44 ---
>  drivers/crypto/caam/jr.c  |  22 ++--
>  drivers/crypto/caam/pdb.h | 137 +++-
>  drivers/crypto/caam/regs.h| 151 
> +++---
>  drivers/crypto/caam/sg_sw_sec4.h  |  17 +--
>  include/asm-generic/io.h  |  71 +-
>  include/asm-generic/iomap.h   |   8 ++
>  15 files changed, 494 insertions(+), 176 deletions(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] crypto: caam - add support for RSA algorithm

[Horia Ioan Geanta Neag]

On 5/12/2016 6:06 PM, Tudor Ambarus wrote:
> Add RSA support to caam driver.
> 
> Coauthored-by: Yashpal Dutta 
> 
> Signed-off-by: Tudor Ambarus 
> ---
>  drivers/crypto/caam/Kconfig|  12 ++
>  drivers/crypto/caam/Makefile   |   4 +
>  drivers/crypto/caam/caampkc.c  | 466 
> +
>  drivers/crypto/caam/caampkc.h  |  94 +
>  drivers/crypto/caam/desc.h |   2 +
>  drivers/crypto/caam/pdb.h  |  16 +-
>  drivers/crypto/caam/pkc_desc.c | 138 
>  7 files changed, 731 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/caam/caampkc.c
>  create mode 100644 drivers/crypto/caam/caampkc.h
>  create mode 100644 drivers/crypto/caam/pkc_desc.c
> 
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index 5652a53..9ba 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -99,6 +99,18 @@ config CRYPTO_DEV_FSL_CAAM_AHASH_API
> To compile this as a module, choose M here: the module
> will be called caamhash.
>  
> +config CRYPTO_DEV_FSL_CAAM_PKC_API
> +tristate "Register public key cryptography implementations with 
> Crypto API"
> +depends on CRYPTO_DEV_FSL_CAAM && CRYPTO_DEV_FSL_CAAM_JR
> +default y
> +select CRYPTO_RSA
> +help
> +  Selecting this will allow SEC Public key support for RSA.
> +  Supported cryptographic primitives: encryption, decryption,
> +  signature and verification.
> +  To compile this as a module, choose M here: the module
> +  will be called caam_pkc.
> +
>  config CRYPTO_DEV_FSL_CAAM_RNG_API
>   tristate "Register caam device for hwrng API"
>   depends on CRYPTO_DEV_FSL_CAAM && CRYPTO_DEV_FSL_CAAM_JR
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 550758a..399ad55 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -5,11 +5,15 @@ ifeq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG), y)
>   EXTRA_CFLAGS := -DDEBUG
>  endif
>  
> +ccflags-y += -I$(srctree)/crypto
> +
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_JR) += caam_jr.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
> +obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caam_pkc.o
>  
>  caam-objs := ctrl.o
>  caam_jr-objs := jr.o key_gen.o error.o
> +caam_pkc-y := caampkc.o pkc_desc.o
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> new file mode 100644
> index 000..34ffa87
> --- /dev/null
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -0,0 +1,466 @@
> +/*
> + * caam - Freescale FSL CAAM support for Public Key Cryptography
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * There is no Shared Descriptor for PKC so that the Job Descriptor must 
> carry
> + * all the desired key parameters, input and output pointers.
> + */
> +#include 
> +#include 
> +#include "compat.h"
> +#include "caampkc.h"
> +#include "sg_sw_sec4.h"
> +#include "regs.h"
> +#include "intern.h"
> +#include "jr.h"
> +#include "error.h"
> +
> +static void rsa_io_unmap(struct device *dev, struct rsa_edesc *edesc,
> +  struct akcipher_request *req)
> +{
> + dma_unmap_sg(dev, req->dst, edesc->dst_nents, DMA_FROM_DEVICE);
> + dma_unmap_sg(dev, req->src, edesc->src_nents, DMA_TO_DEVICE);
> +}
> +
> +static void rsa_pub_unmap(struct device *dev, struct rsa_edesc *edesc,
> +   struct akcipher_request *req)
> +{
> + struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> + struct caam_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
> + struct rsa_key *key = >key;
> + struct rsa_pub_desc *hw_desc = (struct rsa_pub_desc *)edesc->hw_desc;
> +
> + dma_unmap_single(dev, hw_desc->n_dma, key->n_sz, DMA_TO_DEVICE);
> + dma_unmap_single(dev, hw_desc->e_dma, key->e_sz, DMA_TO_DEVICE);
> +}
> +
> +static void rsa_priv_f1_unmap(struct device *dev, struct rsa_edesc *edesc,
> +   struct akcipher_request *req)
> +{
> + struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> + struct caam_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
> + struct rsa_key *key = >key;
> + struct rsa_priv_f1_desc *hw_desc =
> + (struct rsa_priv_f1_desc *)edesc->hw_desc;
> +
> + dma_unmap_single(dev, hw_desc->n_dma, key->n_sz, DMA_TO_DEVICE);
> + dma_unmap_single(dev, hw_desc->d_dma, key->n_sz, DMA_TO_DEVICE);
> +}
> +
> +static size_t skip_to_nonzero(u8 *ptr, size_t nbytes)
> +{
> + size_t nr_zeros = 0;
> +
> + while (!(*ptr) && nbytes) {
> + nbytes--;
> + ptr++;
> + nr_zeros++;
> + }
> +
> + return nr_zeros;
> +}
> +
> +static size_t scatterwalk_skip_zeros(struct scatter_walk 

Re: [PATCH] crypto: caam: add backlogging support

[Horia Ioan Geanta Neag]

On 5/6/2016 4:19 PM, Catalin Vasile wrote:
> caam_jr_enqueue() function returns -EBUSY once there are no more slots
> available in the JR, but it doesn't actually save the current request.
> This breaks the functionality of users that expect that even if there is
> no more space for the request, it is at least queued for later
> execution. In other words, all crypto transformations that request
> backlogging (i.e. have CRYPTO_TFM_REQ_MAY_BACKLOG set), will hang. Such
> an example is dm-crypt. The current patch solves this issue by setting a
> threshold after which caam_jr_enqueue() returns -EBUSY, but since the HW
> job ring isn't actually full, the job is enqueued.
> 
The commit message should mention that *both* number of tfms / Job Ring
*and* the number of available Job Ring slots (configured by
CRYPTO_DEV_FSL_CAAM_RINGSIZE) is being limited by JOBR_THRESH:
tfms / Job Ring < JOBR_THRESH
available (free) Job Ring slots >= JOBR_THRESH

Shouldn't caam_jr_enqueue() from key_gen.c be changed too?
Generating a split key is supposed to be done on behalf of the
underlying tfm, thus the MAY_BACKLOG flag should be checked here too.

> Signed-off-by: Alexandru Porosanu 
> Tested-by: Catalin Vasile 
> ---
>  drivers/crypto/caam/caamalg.c  |  88 --
>  drivers/crypto/caam/caamhash.c | 101 +++--
>  drivers/crypto/caam/intern.h   |   7 ++
>  drivers/crypto/caam/jr.c   | 196 
> +
>  drivers/crypto/caam/jr.h   |   5 ++
>  5 files changed, 343 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index ea8189f..62bce17 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -1921,6 +1921,9 @@ static void aead_encrypt_done(struct device *jrdev, u32 
> *desc, u32 err,
>  
>   edesc = container_of(desc, struct aead_edesc, hw_desc[0]);
>  
> + if (err == -EINPROGRESS)
> + goto out_bklogged;

Checking that err is -EINPROGRESS should be the first thing to do in
*_done callbacks.

> +
>   if (err)
>   caam_jr_strstatus(jrdev, err);
>  
> @@ -1928,6 +1931,7 @@ static void aead_encrypt_done(struct device *jrdev, u32 
> *desc, u32 err,
>  
>   kfree(edesc);
>  
> +out_bklogged:
>   aead_request_complete(req, err);
>  }
>  
> @@ -1943,6 +1947,9 @@ static void aead_decrypt_done(struct device *jrdev, u32 
> *desc, u32 err,
>  
>   edesc = container_of(desc, struct aead_edesc, hw_desc[0]);
>  
> + if (err == -EINPROGRESS)
> + goto out_bklogged;
> +
>   if (err)
>   caam_jr_strstatus(jrdev, err);
>  
> @@ -1956,6 +1963,7 @@ static void aead_decrypt_done(struct device *jrdev, u32 
> *desc, u32 err,
>  
>   kfree(edesc);
>  
> +out_bklogged:
>   aead_request_complete(req, err);
>  }
>  
> @@ -1974,6 +1982,9 @@ static void ablkcipher_encrypt_done(struct device 
> *jrdev, u32 *desc, u32 err,
>   edesc = (struct ablkcipher_edesc *)((char *)desc -
>offsetof(struct ablkcipher_edesc, hw_desc));
>  
> + if (err == -EINPROGRESS)
> + goto out_bklogged;
> +
>   if (err)
>   caam_jr_strstatus(jrdev, err);
>  
> @@ -1989,6 +2000,7 @@ static void ablkcipher_encrypt_done(struct device 
> *jrdev, u32 *desc, u32 err,
>   ablkcipher_unmap(jrdev, edesc, req);
>   kfree(edesc);
>  
> +out_bklogged:
>   ablkcipher_request_complete(req, err);
>  }
>  
> @@ -2006,6 +2018,10 @@ static void ablkcipher_decrypt_done(struct device 
> *jrdev, u32 *desc, u32 err,
>  
>   edesc = (struct ablkcipher_edesc *)((char *)desc -
>offsetof(struct ablkcipher_edesc, hw_desc));
> +
> + if (err == -EINPROGRESS)
> + goto out_bklogged;
> +
>   if (err)
>   caam_jr_strstatus(jrdev, err);
>  
> @@ -2021,6 +2037,7 @@ static void ablkcipher_decrypt_done(struct device 
> *jrdev, u32 *desc, u32 err,
>   ablkcipher_unmap(jrdev, edesc, req);
>   kfree(edesc);
>  
> +out_bklogged:
>   ablkcipher_request_complete(req, err);
>  }
>  
> @@ -2394,7 +2411,15 @@ static int gcm_encrypt(struct aead_request *req)
>  #endif
>  
>   desc = edesc->hw_desc;
> - ret = caam_jr_enqueue(jrdev, desc, aead_encrypt_done, req);
> + if (req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG) {
> + ret = caam_jr_enqueue_bklog(jrdev, desc, aead_encrypt_done,
> + req);
> + if (ret == -EBUSY)
> + return ret;
> + } else {
> + ret = caam_jr_enqueue(jrdev, desc, aead_encrypt_done, req);
> + }
> +
>   if (!ret) {
>   ret = -EINPROGRESS;
>   } else {
> @@ -2438,7 +2463,15 @@ static int aead_encrypt(struct aead_request *req)
>  #endif
>  
>   desc = edesc->hw_desc;
> - ret = caam_jr_enqueue(jrdev, desc, aead_encrypt_done, req);
> + if (req->base.flags & 

Re: [PATCH v2 8/8] arm64: dts: ls1043a: add crypto node

[Horia Ioan Geanta Neag]

+Shawn

On 5/5/2016 6:39 PM, Horia Geantă wrote:
> LS1043A has a SEC v5.4 security engine.
> For now don't add rtic or sec_mon subnodes, since these features
> haven't been tested yet.
> 
> Signed-off-by: Horia Geantă 

Shawn,

IIUC, you are the de facto maintainer of arch/arm64/boot/dts/freescale
entry, thus adding you to the loop.
I'd like to get the Ack for this patch.

As stated in the cover letter: https://lkml.org/lkml/2016/5/5/340
my intent is to go with the whole patch set via cryptodev-2.6 tree.

Thanks,
Horia

> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts |  4 +++
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi| 43 
> +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts 
> b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
> index ce235577e90f..9b5b75a4f02a 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
> @@ -49,6 +49,10 @@
>  
>  / {
>   model = "LS1043A RDB Board";
> +
> + aliases {
> + crypto = 
> + };
>  };
>  
>   {
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi 
> b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index be72bf5b58b5..529c198494d5 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -159,6 +159,49 @@
>   big-endian;
>   };
>  
> + crypto: crypto@170 {
> + compatible = "fsl,sec-v5.4", "fsl,sec-v5.0",
> +  "fsl,sec-v4.0";
> + fsl,sec-era = <3>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x00 0x170 0x10>;
> + reg = <0x00 0x170 0x0 0x10>;
> + interrupts = <0 75 0x4>;
> +
> + sec_jr0: jr@1 {
> + compatible = "fsl,sec-v5.4-job-ring",
> +  "fsl,sec-v5.0-job-ring",
> +  "fsl,sec-v4.0-job-ring";
> + reg= <0x1 0x1>;
> + interrupts = <0 71 0x4>;
> + };
> +
> + sec_jr1: jr@2 {
> + compatible = "fsl,sec-v5.4-job-ring",
> +  "fsl,sec-v5.0-job-ring",
> +  "fsl,sec-v4.0-job-ring";
> + reg= <0x2 0x1>;
> + interrupts = <0 72 0x4>;
> + };
> +
> + sec_jr2: jr@3 {
> + compatible = "fsl,sec-v5.4-job-ring",
> +  "fsl,sec-v5.0-job-ring",
> +  "fsl,sec-v4.0-job-ring";
> + reg= <0x3 0x1>;
> + interrupts = <0 73 0x4>;
> + };
> +
> + sec_jr3: jr@4 {
> + compatible = "fsl,sec-v5.4-job-ring",
> +  "fsl,sec-v5.0-job-ring",
> +  "fsl,sec-v4.0-job-ring";
> + reg= <0x4 0x1>;
> + interrupts = <0 74 0x4>;
> + };
> + };
> +
>   dcfg: dcfg@1ee {
>   compatible = "fsl,ls1043a-dcfg", "syscon";
>   reg = <0x0 0x1ee 0x0 0x1>;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/8] powerpc: add io{read,write}64 accessors

[Horia Ioan Geanta Neag]

On 5/5/2016 6:37 PM, Horia Geantă wrote:
> This will allow device drivers to consistently use io{read,write}XX
> also for 64-bit accesses.
> 
> Signed-off-by: Horia Geantă 

It would be great if PPC maintainers could Ack this patch.

As stated in the cover letter: https://lkml.org/lkml/2016/5/5/340
I'd like to go with the whole patch set via cryptodev-2.6 tree.

Thanks,
Horia

> ---
>  arch/powerpc/kernel/iomap.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/iomap.c b/arch/powerpc/kernel/iomap.c
> index 12e48d56f771..3963f0b68d52 100644
> --- a/arch/powerpc/kernel/iomap.c
> +++ b/arch/powerpc/kernel/iomap.c
> @@ -38,6 +38,18 @@ EXPORT_SYMBOL(ioread16);
>  EXPORT_SYMBOL(ioread16be);
>  EXPORT_SYMBOL(ioread32);
>  EXPORT_SYMBOL(ioread32be);
> +#ifdef __powerpc64__
> +u64 ioread64(void __iomem *addr)
> +{
> + return readq(addr);
> +}
> +u64 ioread64be(void __iomem *addr)
> +{
> + return readq_be(addr);
> +}
> +EXPORT_SYMBOL(ioread64);
> +EXPORT_SYMBOL(ioread64be);
> +#endif /* __powerpc64__ */
>  
>  void iowrite8(u8 val, void __iomem *addr)
>  {
> @@ -64,6 +76,18 @@ EXPORT_SYMBOL(iowrite16);
>  EXPORT_SYMBOL(iowrite16be);
>  EXPORT_SYMBOL(iowrite32);
>  EXPORT_SYMBOL(iowrite32be);
> +#ifdef __powerpc64__
> +void iowrite64(u64 val, void __iomem *addr)
> +{
> + writeq(val, addr);
> +}
> +void iowrite64be(u64 val, void __iomem *addr)
> +{
> + writeq_be(val, addr);
> +}
> +EXPORT_SYMBOL(iowrite64);
> +EXPORT_SYMBOL(iowrite64be);
> +#endif /* __powerpc64__ */
>  
>  /*
>   * These are the "repeat read/write" functions. Note the
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AEAD in TALITOS SEC1 versus TALITOS SEC2

[Horia Ioan Geanta Neag]

On 4/22/2016 6:45 PM, Kim Phillips wrote:
> On Thu, 21 Apr 2016 13:31:47 +
> Horia Ioan Geanta Neag <horia.gea...@nxp.com> wrote:
> 
>> On 4/20/2016 3:04 PM, Christophe Leroy wrote:
>>> What's the best way to implement the selection of the proper descriptor
>>> type ?
>>> * We can duplicate the templates but it means that when both types are
>>> supported the driver with try to register each AEAD alg twice
>>> * We can "on the fly" change the DESC_HDR_TYPE_IPSEC_ESP type into
>>> DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU type ?
>>> * We can alter the templates at startup when we know we are on a SEC1,
>>> changing all templates based on DESC_HDR_TYPE_IPSEC_ESP into templates
>>> based on DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU
>>>
>>> What would be the best approach from your point of view ?
>>>
>> I would go with altering the relevant entries in the template array, of
>> course before the hw_supports() check.
> 
> was wondering whether assigning a different cra_priority were another
> option?
> 
AFAICT yes, this would work combined with the first choice mentioned by
Christophe.

Thanks,
Horia

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AEAD in TALITOS SEC1 versus TALITOS SEC2

[Horia Ioan Geanta Neag]

On 4/20/2016 3:04 PM, Christophe Leroy wrote:
> Today, in Talitos driver crypto alg registration is based on predefined 
> templates with a predefined descriptor type and verification against the 
> descriptors supported by the HW. This works well for ALG that require a 
> unique descriptor. But for IPsec this is slightly different:
> * IPsec can be performed with both DESC_HDR_TYPE_IPSEC_ESP and 
> DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU
> * DESC_HDR_TYPE_IPSEC_ESP is supported only by SEC2
> * DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU is supported by both SEC1 and SEC2
> * DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU is less performant than 
> DESC_HDR_TYPE_IPSEC_ESP
> So it is natural to use DESC_HDR_TYPE_IPSEC_ESP when it is supported and 
> use DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU otherwise ?
> 
> What's the best way to implement the selection of the proper descriptor 
> type ?
> * We can duplicate the templates but it means that when both types are 
> supported the driver with try to register each AEAD alg twice
> * We can "on the fly" change the DESC_HDR_TYPE_IPSEC_ESP type into 
> DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU type ?
> * We can alter the templates at startup when we know we are on a SEC1, 
> changing all templates based on DESC_HDR_TYPE_IPSEC_ESP into templates 
> based on DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU
> 
> What would be the best approach from your point of view ?
> 
I would go with altering the relevant entries in the template array, of
course before the hw_supports() check.

IIUC, the "on the fly" option won't work. There has to be a valid
descriptor type for each template entry before hw_supports().

Regards,
Horia
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ppc/talitos oops on call to crypto_alloc_aead

[Horia Ioan Geanta Neag]

On 4/18/2016 12:36 PM, Herbert Xu wrote:
> On Fri, Apr 15, 2016 at 10:02:21PM +0800, Herbert Xu wrote:
>> Jonas Eymann  wrote:
>>>
>>> running a current 4.4.6 kernel on a board using a Freescale P1020, I ran 
>>> into an oops when calling crypto_alloc_aead using the talitos driver. I 
>>> could also reproduce this using the run-time self tests:
>>
>> Thanks for the patch.  Could you please add a Signed-off-by?
>>  
Thanks Jonas.

It looks there's a typo, please consider this incremental patch:

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 4f09395dd5f2..7c16051599fd 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2657,7 +2657,7 @@ static int talitos_cra_init(struct crypto_tfm *tfm)
struct talitos_ctx *ctx = crypto_tfm_ctx(tfm);

if ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
CRYPTO_ALG_TYPE_AHASH)
-   talitos_alg = container_of(__crypto_ahash_alg(tfm),
+   talitos_alg = container_of(__crypto_ahash_alg(alg),
   struct talitos_crypto_alg,
   algt.alg.hash);
else


>>> In a first attempt to fix this problem using the patch further below, I 
>>> could prevent the oops, though the self tests still fail:
>>>
>>> [...]
>>> [1.145414] alg: aead: Test 1 failed on encryption for 
>>> authenc-hmac-sha1-cbc-aes-talitos
>>> [1.153564] : 53 69 6e 67 6c 65 20 62 6c 6f 63 6b 20 6d 73 67
>>> [1.160041] 0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> [1.166509] 0020: 00 00 00 00
>>> [1.170041] alg: aead: Test 1 failed on encryption for 
>>> authenc-hmac-sha1-cbc-3des-talitos
>>> [1.178276] : 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
>>> [1.184751] 0010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
>>> [1.191220] 0020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
>>> [1.197689] 0030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
>>> [1.204158] 0040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
>>> [1.210626] 0050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
>>> [1.217095] 0060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
>>> [1.223563] 0070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
>>> [1.230038] 0080: de ad be ef de ad be ef de ad be ef de ad be ef
>>> [1.236515] 0090: de ad be ef
> 
> Any chance you can let me know whether 4.2 works on this machine?
> 
4.2 works fine. 4.3+ have issues.

This might be related to:
aeb4c132f33d crypto: talitos - Convert to new AEAD interface
Before this commit, self-tests pass.

Wrt. hash algorithms registration failure, indeed the problem is due to
lack of import/export functionality.

Regards,
Horia
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] crypto: caam - performance fixes/improvements

[Horia Ioan Geanta Neag]

On 1/12/2016 5:14 PM, Horia Geantă wrote:
> The following patches increase/fix CAAM performance by modifying
> the configuration of MCFGR (Master Configuration Register):
> -1st patch fixes a ~ 5% performance drop on PPC platforms
> -2nd patch improves performance in some use cases, since CAAM DMA
> transfers are optimized
> 
> 
> Note: AWCACHE[0] (AXI3 "bufferable") and AWCACHE[1] (AXI3 "cacheable")
> are set irrespective of platform, since:
> -for ARM-based SoCs: the interconnect IP ignores AWCACHE[0]
> -for PPC-based SoCs: PAMU handles coherency control, not AWCACHE[1]
> 
> Horia Geantă (2):
>   crypto: caam - make write transactions bufferable on PPC platforms
>   crypto: caam - enable LARGE_BURST for enhancing DMA transactions size

Herbert, I see that you've sent the crypto fixes for 4.5 pull request.
Any reason why these patches were not included?
I was expecting at least the first one to get it, it's even Cc-ing stable.

Thanks,
Horia