Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
[...]
> +struct trusted_key_ops caam_trusted_key_ops = {
> + .migratable = 0, /* non-migratable */
> + .init = trusted_caam_init,
> + .seal = trusted_caam_seal,
> + .unseal = trusted_caam_unseal,
> + .exit = trusted_caam_exit,
> +};
caam has random number generation capabilities, so it's worth using that
by implementing .get_random.
Horia
Re: [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator
On 3/16/2021 7:01 PM, Ahmad Fatoum wrote:
> +int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
> + void *input, void *output, size_t length)
> +{
> + u32 *desc;
> + struct device *jrdev = >jrdev;
> + dma_addr_t dma_in, dma_out;
> + struct caam_blob_job_result testres;
> + size_t keymod_len = strlen(keymod);
> + int ret;
> +
> + if (length <= CAAM_BLOB_OVERHEAD)
> + return -EINVAL;
> +
> + desc = caam_blob_alloc_desc(keymod_len);
> + if (!desc) {
> + dev_err(jrdev, "unable to allocate desc\n");
> + return -ENOMEM;
> + }
> +
> + dma_in = dma_map_single(jrdev, input, length - CAAM_BLOB_OVERHEAD,
> DMA_TO_DEVICE);
> + if (dma_mapping_error(jrdev, dma_in)) {
> + dev_err(jrdev, "unable to map input DMA buffer\n");
> + ret = -ENOMEM;
> + goto out_free;
> + }
> +
> + dma_out = dma_map_single(jrdev, output, length, DMA_FROM_DEVICE);
> + if (dma_mapping_error(jrdev, dma_out)) {
> + dev_err(jrdev, "unable to map output DMA buffer\n");
> + ret = -ENOMEM;
> + goto out_unmap_in;
> + }
> +
> + /*
> + * A data blob is encrypted using a blob key (BK); a random number.
> + * The BK is used as an AES-CCM key. The initial block (B0) and the
> + * initial counter (Ctr0) are generated automatically and stored in
> + * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
> + * Class 1 Key Register. Operation Mode is set to AES-CCM.
> + */
> +
> + init_job_desc(desc, 0);
> + append_key_as_imm(desc, keymod, keymod_len, keymod_len,
> + CLASS_2 | KEY_DEST_CLASS_REG);
> + append_seq_in_ptr(desc, dma_in, length - CAAM_BLOB_OVERHEAD, 0);
> + append_seq_out_ptr(desc, dma_out, length, 0);
In case length is known to be < 2^16, it's recommended to use instead
append_seq_in_ptr_intlen, append_seq_out_ptr_intlen.
> + append_operation(desc, OP_TYPE_ENCAP_PROTOCOL | OP_PCLID_BLOB);
> +
> + print_hex_dump_debug("data@"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 1, input,
> + length - CAAM_BLOB_OVERHEAD, false);
> + print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 1, desc,
> + desc_bytes(desc), false);
> +
> + testres.err = 0;
> + init_completion();
> +
> + ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, );
> + if (ret == -EINPROGRESS) {
> + wait_for_completion();
> + ret = testres.err;
> + print_hex_dump_debug("output@"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 1, output,
> + length, false);
> + }
> +
> + dma_unmap_single(jrdev, dma_out, length, DMA_FROM_DEVICE);
> +out_unmap_in:
> + dma_unmap_single(jrdev, dma_in, length - CAAM_BLOB_OVERHEAD,
> DMA_TO_DEVICE);
> +out_free:
> + kfree(desc);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(caam_encap_blob);
> +
[...]
> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
> new file mode 100644
> index ..7eea0f543832
> --- /dev/null
> +++ b/include/soc/fsl/caam-blob.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum
> + */
> +
> +#ifndef __CAAM_BLOB_GEN
> +#define __CAAM_BLOB_GEN
> +
> +#include
> +
> +#define CAAM_BLOB_KEYMOD_LENGTH 16
The define isn't used here or on patch 3/3.
> +#define CAAM_BLOB_OVERHEAD (32 + 16)
> +#define CAAM_BLOB_MAX_LEN4096
> +
> +struct caam_blob_priv;
> +
> +/** caam_blob_gen_init - initialize blob generation
> + *
> + * returns either pointer to new caam_blob_priv instance
> + * or error pointer
> + */
> +struct caam_blob_priv *caam_blob_gen_init(void);
> +
> +/** caam_blob_gen_init - free blob generation resources
> + *
> + * @priv: instance returned by caam_blob_gen_init
> + */
> +void caam_blob_gen_exit(struct caam_blob_priv *priv);
> +
> +/** caam_encap_blob - encapsulate blob
> + *
> + * @priv: instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob encapsulation
> + * @input: buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
Is it guaranteed that input, output can be DMA-mapped?
Horia
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
On 3/16/2021 7:02 PM, Ahmad Fatoum wrote: > The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core > built into many newer i.MX and QorIQ SoCs by NXP. > > Its blob mechanism can AES encrypt/decrypt user data using a unique > never-disclosed device-specific key. There has been multiple > discussions on how to represent this within the kernel: > > - [RFC] crypto: caam - add red blobifier >Steffen implemented[1] a PoC sysfs driver to start a discussion on how to >best integrate the blob mechanism. >Mimi suggested that it could be used to implement trusted keys. >Trusted keys back then were a TPM-only feature. > > - security/keys/secure_key: Adds the secure key support based on CAAM. >Udit added[2] a new "secure" key type with the CAAM as backend. The key >material stays within the kernel only. >Mimi and James agreed that this needs a generic interface, not specific >to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as >basis for TEE-backed keys. > > - [RFC] drivers: crypto: caam: key: Add caam_tk key type >Franck added[3] a new "caam_tk" key type based on Udit's work. The key >material stays within the kernel only, but can optionally be user-set >instead of coming from RNG. James voiced the opinion that there should >be just one user-facing generic wrap/unwrap key type with multiple >possible handlers. David suggested trusted keys. > The whole point was to use caam "black blobs", with the main advantage of keys being kept encrypted in memory after "unsealing" the blobs. (Keys in blobs are encrypted with a persistent BKEK - blob KEK, derived from fuse-based OTPMK. OTOH black keys are keys encrypted with an ephemeral, random KEK that is stored in an internal caam register. When a black blob is unsealed, the key is practically rekeyed, the random key replacing the BKEK; key is never exposed in plaintext, rekeying happens in caam). Current implementation uses "red blobs", which means keys are left unprotected in memory after blobs are unsealed. > - Introduce TEE based Trusted Keys support >Sumit reworked[4] trusted keys to support multiple possible backends with >one chosen at boot time and added a new TEE backend along with TPM. >This now sits in Jarkko's master branch to be sent out for v5.13 > > This patch series builds on top of Sumit's rework to have the CAAM as yet > another > trusted key backend. > Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig) be updated to reflect the availability of multiple backends? Thanks, Horia
Re: [PATCH 07/10] crypto: caam: caampkc: Provide the name of the function and provide missing descriptions
On 3/18/2021 2:44 PM, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/crypto/caam/caampkc.c:199: warning: expecting prototype for from a > given scatterlist(). Prototype was for caam_rsa_count_leading_zeros() instead > drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member > 'xts_key_fallback' not described in 'caam_ctx' > drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member > 'fallback' not described in 'caam_ctx' > > Cc: "Horia Geantă" > Cc: Aymen Sghaier > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: linux-cry...@vger.kernel.org > Signed-off-by: Lee Jones Reviewed-by: Horia Geantă Thanks, Horia
[PATCH] ARM: dts: ls1021a: mark crypto engine dma coherent
Crypto engine (CAAM) on LS1021A platform is configured HW-coherent,
mark accordingly the DT node.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/ls1021a.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 007dd2bd0595..62c6eb87cfb7 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -246,6 +246,7 @@
reg = <0x0 0x170 0x0 0x10>;
ranges = <0x0 0x0 0x170 0x10>;
interrupts = ;
+ dma-coherent;
sec_jr0: jr@1 {
compatible = "fsl,sec-v5.0-job-ring",
base-commit: 498f8aee6ec000392d918ecbcbeb1b5ee3132006
--
2.17.1
[PATCH 3/3] arm64: dts: ls1012a: mark crypto engine dma coherent
Crypto engine (CAAM) on LS1012A platform is configured HW-coherent,
mark accordingly the DT node.
Lack of "dma-coherent" property for an IP that is configured HW-coherent
can lead to problems, similar to what has been reported for LS1046A.
Cc: # v4.12+
Fixes: 85b85c569507 ("arm64: dts: ls1012a: add crypto node")
Signed-off-by: Horia Geantă
---
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index 7de6b376d792..9058cfa4980f 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -198,6 +198,7 @@
ranges = <0x0 0x00 0x170 0x10>;
reg = <0x00 0x170 0x0 0x10>;
interrupts = ;
+ dma-coherent;
sec_jr0: jr@1 {
compatible = "fsl,sec-v5.4-job-ring",
--
2.17.1
[PATCH 2/3] arm64: dts: ls1043a: mark crypto engine dma coherent
Crypto engine (CAAM) on LS1043A platform is configured HW-coherent,
mark accordingly the DT node.
Lack of "dma-coherent" property for an IP that is configured HW-coherent
can lead to problems, similar to what has been reported for LS1046A.
Cc: # v4.8+
Fixes: 63dac35b58f4 ("arm64: dts: ls1043a: add crypto node")
Link:
https://lore.kernel.org/linux-crypto/fe6faa24-d8f7-d18f-adfa-44fa0caa1...@arm.com
Signed-off-by: Horia Geantă
---
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 5a8a1dc4262d..28c51e521cb2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -348,6 +348,7 @@
ranges = <0x0 0x00 0x170 0x10>;
reg = <0x00 0x170 0x0 0x10>;
interrupts = <0 75 0x4>;
+ dma-coherent;
sec_jr0: jr@1 {
compatible = "fsl,sec-v5.4-job-ring",
--
2.17.1
[PATCH 0/3] arm64: dts: ls: mark crypto engine dma coherent
This patch set adds "dma-coherent" property to the crypto node for NXP Layerscape platforms where the IP (CAAM) is configured HW-coherent. Horia Geantă (3): arm64: dts: ls1046a: mark crypto engine dma coherent arm64: dts: ls1043a: mark crypto engine dma coherent arm64: dts: ls1012a: mark crypto engine dma coherent arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 + arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 1 + arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 + 3 files changed, 3 insertions(+) base-commit: da1a6b8bec881b67f0e234ed19e8b7e2fb1e7812 -- 2.17.1
[PATCH 1/3] arm64: dts: ls1046a: mark crypto engine dma coherent
Crypto engine (CAAM) on LS1046A platform is configured HW-coherent,
mark accordingly the DT node.
As reported by Greg and Sascha, and explained by Robin, lack of
"dma-coherent" property for an IP that is configured HW-coherent
can lead to problems, e.g. on v5.11:
> kernel BUG at drivers/crypto/caam/jr.c:247!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 5.11.0-20210225-3-00039-g434215968816-dirty #12
> Hardware name: TQ TQMLS1046A SoM on Arkona AT1130 (C300) board (DT)
> pstate: 6005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> pc : caam_jr_dequeue+0x98/0x57c
> lr : caam_jr_dequeue+0x98/0x57c
> sp : 800010003d50
> x29: 800010003d50 x28: 8000118d4000
> x27: 8000118d4328 x26: 01f0
> x25: 0008022be480 x24: 0008022c6410
> x23: 01f1 x22: 8000118d4329
> x21: 4d80 x20: 01f1
> x19: 0001 x18: 0020
> x17: x16: 0015
> x15: 800011690230 x14: 2e2e2e2e2e2e2e2e
> x13: 2e2e2e2e2e2e2020 x12: 3030303030303030
> x11: 800011700a38 x10: f000
> x9 : 8000100ada30 x8 : 8000116a8a38
> x7 : 0001 x6 :
> x5 : x4 :
> x3 : x2 :
> x1 : x0 : 1800
> Call trace:
> caam_jr_dequeue+0x98/0x57c
> tasklet_action_common.constprop.0+0x164/0x18c
> tasklet_action+0x44/0x54
> __do_softirq+0x160/0x454
> __irq_exit_rcu+0x164/0x16c
> irq_exit+0x1c/0x30
> __handle_domain_irq+0xc0/0x13c
> gic_handle_irq+0x5c/0xf0
> el1_irq+0xb4/0x180
> arch_cpu_idle+0x18/0x30
> default_idle_call+0x3c/0x1c0
> do_idle+0x23c/0x274
> cpu_startup_entry+0x34/0x70
> rest_init+0xdc/0xec
> arch_call_rest_init+0x1c/0x28
> start_kernel+0x4ac/0x4e4
> Code: 91392021 912c2000 d377d8c6 97f24d96 (d421)
Cc: # v4.10+
Fixes: 8126d88162a5 ("arm64: dts: add QorIQ LS1046A SoC support")
Link:
https://lore.kernel.org/linux-crypto/fe6faa24-d8f7-d18f-adfa-44fa0caa1...@arm.com
Reported-by: Greg Ungerer
Reported-by: Sascha Hauer
Tested-by: Sascha Hauer
Signed-off-by: Horia Geantă
---
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index f581a6d1f881..37fec474673a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -354,6 +354,7 @@
ranges = <0x0 0x00 0x170 0x10>;
reg = <0x00 0x170 0x0 0x10>;
interrupts = ;
+ dma-coherent;
sec_jr0: jr@1 {
compatible = "fsl,sec-v5.4-job-ring",
--
2.17.1
Re: [PATCH 07/10] crypto: caam: caampkc: Provide the name of the function and provide missing descriptions
On 3/3/2021 4:35 PM, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
>
> drivers/crypto/caam/caampkc.c:199: warning: expecting prototype for from a
> given scatterlist(). Prototype was for caam_rsa_count_leading_zeros() instead
> drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member
> 'xts_key_fallback' not described in 'caam_ctx'
> drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member
> 'fallback' not described in 'caam_ctx'
>
> Cc: "Horia Geantă"
> Cc: Aymen Sghaier
> Cc: Herbert Xu
> Cc: "David S. Miller"
> Cc: linux-cry...@vger.kernel.org
> Signed-off-by: Lee Jones
> ---
> drivers/crypto/caam/caamalg_qi2.c | 2 ++
> drivers/crypto/caam/caampkc.c | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/caam/caamalg_qi2.c
> b/drivers/crypto/caam/caamalg_qi2.c
> index a780e627838ae..22e45c5bf2023 100644
> --- a/drivers/crypto/caam/caamalg_qi2.c
> +++ b/drivers/crypto/caam/caamalg_qi2.c
> @@ -71,6 +71,8 @@ struct caam_skcipher_alg {
> * @adata: authentication algorithm details
> * @cdata: encryption algorithm details
> * @authsize: authentication tag (a.k.a. ICV / MAC) size
> + * @xts_key_fallback: whether to set the fallback key
> + * @fallback: the fallback key
I'd prefer having the doc updated as mentioned in v1:
https://lore.kernel.org/linux-crypto/963b9e57-8077-5392-1d91-a5971e8d8...@nxp.com
Thanks,
Horia
Re: [PATCH 15/20] crypto: caam: caamalg_qi2: Supply a couple of 'fallback' related descriptions
On 2/4/2021 1:10 PM, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
>
> drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member
> 'xts_key_fallback' not described in 'caam_ctx'
> drivers/crypto/caam/caamalg_qi2.c:87: warning: Function parameter or member
> 'fallback' not described in 'caam_ctx'
>
> Cc: "Horia Geantă"
> Cc: Aymen Sghaier
> Cc: Herbert Xu
> Cc: "David S. Miller"
> Cc: linux-cry...@vger.kernel.org
> Signed-off-by: Lee Jones
> ---
> drivers/crypto/caam/caamalg_qi2.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/crypto/caam/caamalg_qi2.c
> b/drivers/crypto/caam/caamalg_qi2.c
> index a780e627838ae..22e45c5bf2023 100644
> --- a/drivers/crypto/caam/caamalg_qi2.c
> +++ b/drivers/crypto/caam/caamalg_qi2.c
> @@ -71,6 +71,8 @@ struct caam_skcipher_alg {
> * @adata: authentication algorithm details
> * @cdata: encryption algorithm details
> * @authsize: authentication tag (a.k.a. ICV / MAC) size
> + * @xts_key_fallback: whether to set the fallback key
true if fallback tfm needs to be used due to unsupported xts key lengths.
> + * @fallback: the fallback key
xts fallback tfm
Horia
Re: [PATCH 14/20] crypto: caam: caampkc: Provide the name of the function
On 2/4/2021 1:10 PM, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/crypto/caam/caampkc.c:199: warning: expecting prototype for from a > given scatterlist(). Prototype was for caam_rsa_count_leading_zeros() instead > > Cc: "Horia Geantă" > Cc: Aymen Sghaier > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: linux-cry...@vger.kernel.org > Signed-off-by: Lee Jones Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH v2] crypto: caam - Replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
On 2/2/2021 4:06 AM, Jiapeng Chong wrote: > Fix the following coccicheck warning: > > ./drivers/crypto/caam/debugfs.c:23:0-23: WARNING: caam_fops_u64_ro > should be defined with DEFINE_DEBUGFS_ATTRIBUTE. > > ./drivers/crypto/caam/debugfs.c:22:0-23: WARNING: caam_fops_u32_ro > should be defined with DEFINE_DEBUGFS_ATTRIBUTE. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH] crypto: caam -Replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
On 2/1/2021 10:20 AM, Jiapeng Chong wrote: > Fix the following coccicheck warning: > > ./drivers/crypto/caam/debugfs.c:23:0-23: WARNING: caam_fops_u64_ro > should be defined with DEFINE_DEBUGFS_ATTRIBUTE. > > ./drivers/crypto/caam/debugfs.c:22:0-23: WARNING: caam_fops_u32_ro > should be defined with DEFINE_DEBUGFS_ATTRIBUTE. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Reviewed-by: Horia Geantă Patch title nitpick: s/crypto: caam -Replace/crypto: caam - Replace Thanks, Horia
Re: [PATCH 0/5] crypto: caam - avoid allocating memory at crypto request runtime
On 12/3/2020 3:35 AM, Iuliana Prodan (OSS) wrote: > From: Iuliana Prodan > > This series removes CRYPTO_ALG_ALLOCATES_MEMORY flag and > allocates the memory needed by the driver, to fulfil a > request, within the crypto request object. > The extra size needed for base extended descriptor, hw > descriptor commands and link tables is added to the reqsize > field that indicates how much memory could be needed per request. > > CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to > dm-crypt use-cases, which seems to be 4 entries maximum. > Therefore in reqsize we allocate memory for maximum 4 entries > for src and 4 for dst, aligned. > If the driver needs more than the 4 entries maximum, the memory > is dynamically allocated, at runtime. > Moving the memory allocations from caam driver into the generic crypto API has the side effect of dropping the GFP_DMA allocation flag. For cases when caam device is limited to 32-bit address space and there's no IOMMU, this could lead to DMA API using bounce buffering. We need to measure the performance impact of this change before deciding what we should do next. Thanks, Horia
Re: [RFC PATCH 0/4] crypto: add CRYPTO_TFM_REQ_DMA flag
On 11/26/2020 9:09 AM, Ard Biesheuvel wrote: > On Wed, 25 Nov 2020 at 22:39, Iuliana Prodan wrote: >> >> On 11/25/2020 11:16 PM, Ard Biesheuvel wrote: >>> On Wed, 25 Nov 2020 at 22:14, Iuliana Prodan (OSS) >>> wrote: From: Iuliana Prodan Add the option to allocate the crypto request object plus any extra space needed by the driver into a DMA-able memory. Add CRYPTO_TFM_REQ_DMA flag to be used by backend implementations to indicate to crypto API the need to allocate GFP_DMA memory for private contexts of the crypto requests. >>> >>> These are always directional DMA mappings, right? So why can't we use >>> bounce buffering here? >>> >> The idea was to avoid allocating any memory in crypto drivers. >> We want to be able to use dm-crypt with CAAM, which needs DMA-able >> memory and increasing reqsize is not enough. > > But what does 'needs DMA-able memory' mean? DMA operations are > asynchronous by definition, and so the DMA layer should be able to > allocate bounce buffers when needed. This will cost some performance > in cases where the hardware cannot address all of memory directly, but > this is a consequence of the design, and I don't think we should > burden the generic API with this. > The performance loss due to bounce buffering is non-negligible. Previous experiments we did showed a 35% gain (when forcing all data, including I/O buffers, in ZONE_DMA32). I don't have the exact numbers for bounce buffering introduced by allowing only by the control data structures (descriptors etc.) in high memory, but I don't think it's fair to easily dismiss this topic, given the big performance drop and relatively low impact on the generic API. Thanks, Horia
Re: [PATCH] random: Don't freeze in add_hwgenerator_randomness() if stopping kthread
On 11/17/2019 2:57 AM, Herbert Xu wrote:
> On Sun, Nov 17, 2019 at 12:01:20AM +0100, Maciej S. Szmigiero wrote:
>>
>> If a reader (user space) task is frozen then it is no longer waiting
>> on this waitqueue - at least if I understand correctly how the freezer
>> works for user space tasks, that is by interrupting waits via a fake
>> signal.
>
> At this point I'm just going to revert the whole thing and we can
> sort it out in the next development cycle.
>
Working at adding power management support for caam crypto driver,
I've stumbled upon the issue of caam rng suspending while hwrng kthread
is still active.
For now I've fixed the issue by unregistering / re-registering caam rng
from hwrng at device suspend / resume time.
OTOH I see a few commits:
03a3bb7ae631 ("hwrng: core - Freeze khwrng thread during suspend")
ff296293b353 ("random: Support freezable kthreads in
add_hwgenerator_randomness()")
59b569480dc8 ("random: Use wait_event_freezable() in
add_hwgenerator_randomness()")
however they were reverted a bit later in commit
08e97aec700a ("Revert "hwrng: core - Freeze khwrng thread during suspend"")
I wasn't able to find a follow-up on this topic.
Could you advise on what's the proper thing to do going forward?
Thanks,
Horia
crypto: caam/qi - simplify error path for context allocation
On 11/6/2020 11:01 AM, Wang Qing wrote: > Fix passing zero to 'PTR_ERR' warning > Thanks. > Signed-off-by: Wang Qing > --- > drivers/crypto/caam/caamalg_qi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/caam/caamalg_qi.c > b/drivers/crypto/caam/caamalg_qi.c > index 66f60d7..add60e8 > --- a/drivers/crypto/caam/caamalg_qi.c > +++ b/drivers/crypto/caam/caamalg_qi.c > @@ -1166,7 +1166,7 @@ static inline int aead_crypt(struct aead_request *req, > bool encrypt) > /* allocate extended descriptor */ > edesc = aead_edesc_alloc(req, encrypt); > if (IS_ERR_OR_NULL(edesc)) > - return PTR_ERR(edesc); > + return PTR_ERR_OR_ZERO(edesc); > Digging a bit into the logic, it turns out aead_edesc_alloc() can't return a NULL pointer. -- >8 -- Subject: [PATCH] crypto: caam/qi - simplify error path for context allocation Wang Qing reports that IS_ERR_OR_NULL() should be matched with PTR_ERR_OR_ZERO(), not PTR_ERR(). As it turns out, the error path always returns an error code, i.e. NULL is never returned. Update the code accordingly - s/IS_ERR_OR_NULL/IS_ERR. Reported-by: Wang Qing Signed-off-by: Horia Geantă --- drivers/crypto/caam/caamalg_qi.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c index a24ae966df4a..189a7438b29c 100644 --- a/drivers/crypto/caam/caamalg_qi.c +++ b/drivers/crypto/caam/caamalg_qi.c @@ -852,7 +852,7 @@ static struct caam_drv_ctx *get_drv_ctx(struct caam_ctx *ctx, cpu = smp_processor_id(); drv_ctx = caam_drv_ctx_init(ctx->qidev, , desc); - if (!IS_ERR_OR_NULL(drv_ctx)) + if (!IS_ERR(drv_ctx)) drv_ctx->op_type = type; ctx->drv_ctx[type] = drv_ctx; @@ -955,7 +955,7 @@ static struct aead_edesc *aead_edesc_alloc(struct aead_request *req, struct caam_drv_ctx *drv_ctx; drv_ctx = get_drv_ctx(ctx, encrypt ? ENCRYPT : DECRYPT); - if (IS_ERR_OR_NULL(drv_ctx)) + if (IS_ERR(drv_ctx)) return (struct aead_edesc *)drv_ctx; /* allocate space for base edesc and hw desc commands, link tables */ @@ -1165,7 +1165,7 @@ static inline int aead_crypt(struct aead_request *req, bool encrypt) /* allocate extended descriptor */ edesc = aead_edesc_alloc(req, encrypt); - if (IS_ERR_OR_NULL(edesc)) + if (IS_ERR(edesc)) return PTR_ERR(edesc); /* Create and submit job descriptor */ @@ -1259,7 +1259,7 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req, struct caam_drv_ctx *drv_ctx; drv_ctx = get_drv_ctx(ctx, encrypt ? ENCRYPT : DECRYPT); - if (IS_ERR_OR_NULL(drv_ctx)) + if (IS_ERR(drv_ctx)) return (struct skcipher_edesc *)drv_ctx; src_nents = sg_nents_for_len(req->src, req->cryptlen); -- 2.17.1
[PATCH] crypto: caam - fix printing on xts fallback allocation error path
At the time xts fallback tfm allocation fails the device struct
hasn't been enabled yet in the caam xts tfm's private context.
Fix this by using the device struct from xts algorithm's private context
or, when not available, by replacing dev_err with pr_err.
Fixes: 9d9b14dbe077 ("crypto: caam/jr - add fallback for XTS with more than 8B
IV")
Fixes: 83e8aa912138 ("crypto: caam/qi - add fallback for XTS with more than 8B
IV")
Fixes: 36e2d7cfdcf1 ("crypto: caam/qi2 - add fallback for XTS with more than 8B
IV")
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caamalg.c | 4 ++--
drivers/crypto/caam/caamalg_qi.c | 4 ++--
drivers/crypto/caam/caamalg_qi2.c | 3 ++-
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index cf5bd7666dfc..8697ae53b063 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -3404,8 +3404,8 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
fallback = crypto_alloc_skcipher(tfm_name, 0,
CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
- dev_err(ctx->jrdev, "Failed to allocate %s fallback:
%ld\n",
- tfm_name, PTR_ERR(fallback));
+ pr_err("Failed to allocate %s fallback: %ld\n",
+ tfm_name, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 66f60d78bdc8..a24ae966df4a 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -2502,8 +2502,8 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
fallback = crypto_alloc_skcipher(tfm_name, 0,
CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
- dev_err(ctx->jrdev, "Failed to allocate %s fallback:
%ld\n",
- tfm_name, PTR_ERR(fallback));
+ pr_err("Failed to allocate %s fallback: %ld\n",
+ tfm_name, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 98c1ff1744bb..a780e627838a 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1611,7 +1611,8 @@ static int caam_cra_init_skcipher(struct crypto_skcipher
*tfm)
fallback = crypto_alloc_skcipher(tfm_name, 0,
CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
- dev_err(ctx->dev, "Failed to allocate %s fallback:
%ld\n",
+ dev_err(caam_alg->caam.dev,
+ "Failed to allocate %s fallback: %ld\n",
tfm_name, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
--
2.17.1
[PATCH] crypto: arm/aes-neonbs - fix usage of cbc(aes) fallback
Loading the module deadlocks since:
-local cbc(aes) implementation needs a fallback and
-crypto API tries to find one but the request_module() resolves back to
the same module
Fix this by changing the module alias for cbc(aes) and
using the NEED_FALLBACK flag when requesting for a fallback algorithm.
Fixes: 00b99ad2bac2 ("crypto: arm/aes-neonbs - Use generic cbc encryption path")
Signed-off-by: Horia Geantă
---
arch/arm/crypto/aes-neonbs-glue.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm/crypto/aes-neonbs-glue.c
b/arch/arm/crypto/aes-neonbs-glue.c
index bda8bf17631e..f70af1d0514b 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -19,7 +19,7 @@ MODULE_AUTHOR("Ard Biesheuvel ");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS_CRYPTO("ecb(aes)");
-MODULE_ALIAS_CRYPTO("cbc(aes)");
+MODULE_ALIAS_CRYPTO("cbc(aes)-all");
MODULE_ALIAS_CRYPTO("ctr(aes)");
MODULE_ALIAS_CRYPTO("xts(aes)");
@@ -191,7 +191,8 @@ static int cbc_init(struct crypto_skcipher *tfm)
struct aesbs_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
unsigned int reqsize;
- ctx->enc_tfm = crypto_alloc_skcipher("cbc(aes)", 0, CRYPTO_ALG_ASYNC);
+ ctx->enc_tfm = crypto_alloc_skcipher("cbc(aes)", 0, CRYPTO_ALG_ASYNC |
+CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(ctx->enc_tfm))
return PTR_ERR(ctx->enc_tfm);
@@ -441,7 +442,8 @@ static struct skcipher_alg aes_algs[] = { {
.base.cra_blocksize = AES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct aesbs_cbc_ctx),
.base.cra_module= THIS_MODULE,
- .base.cra_flags = CRYPTO_ALG_INTERNAL,
+ .base.cra_flags = CRYPTO_ALG_INTERNAL |
+ CRYPTO_ALG_NEED_FALLBACK,
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
--
2.17.1
Re: [PATCH] crypto: arm/aes-neonbs - fix usage of cbc(aes) fallback
On 10/28/2020 11:07 AM, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 10:03, Horia Geantă wrote:
>>
>> Loading the module deadlocks since:
>> -local cbc(aes) implementation needs a fallback and
>> -crypto API tries to find one but the request_module() resolves back to
>> the same module
>>
>> Fix this by changing the module alias for cbc(aes) and
>> using the NEED_FALLBACK flag when requesting for a fallback algorithm.
>>
>> Fixes: 00b99ad2bac2 ("crypto: arm/aes-neonbs - Use generic cbc encryption
>> path")
>> Signed-off-by: Horia Geantă
>
> Not sure what is happening here: IIRC the intention was to rely on the
> fact that only the sync cbc(aes) implementation needs the fallback,
> and therefore, allocating a sync skcipher explicitly would avoid this
> recursion.
>
My understanding is the following:
1. Local cbc_init() tries to allocate a fallback tfm for cbc(aes)
2. crypto API cbc(aes) tries to find a cbc(aes) algorithm implementation
crypto_alloc_skcipher ->
crypto_alloc_tfm ->
crypto_alloc_tfm_node ->
crypto_find_alg ->
crypto_alg_mod_lookup ->
crypto_larval_lookup
Here crypto_alg_lookup() fails to find a cbc(aes) implementation.
3. Next step is to try to dynamically load a module (request_module)
that supports this implementation. And here it deadlocks, since it tries
to load aes-arm-bs module...
The fix is providing a way to (partially) skip the dynamic module loading
in crypto_alg_lookup() and allow for the last method of finding an algorithm
implementation, which is creating the cbc(aes) on the fly
via the cbc template - see:
ok = crypto_probing_notify(CRYPTO_MSG_ALG_REQUEST, larval);
in crypto_alg_mod_lookup().
Horia
Re: [PATCH v2] crypto: caam - enable crypto-engine retry mechanism
On 10/26/2020 9:06 PM, Iuliana Prodan wrote: > Use the new crypto_engine_alloc_init_and_set() function to > initialize crypto-engine and enable retry mechanism. > > Set the maximum size for crypto-engine software queue based on > Job Ring size (JOBR_DEPTH) and a threshold (reserved for the > non-crypto-API requests that are not passed through crypto-engine). > > The callback for do_batch_requests is NULL, since CAAM > doesn't support linked requests. > > Signed-off-by: Iuliana Prodan Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH] crypto: caam - enable crypto-engine retry mechanism
On 10/26/2020 7:11 PM, Iuliana Prodan wrote: > On 10/26/2020 5:36 PM, Horia Geantă wrote: >> On 10/21/2020 11:07 PM, Iuliana Prodan wrote: [...] >>> +#define CRYPTO_ENGINE_MAX_QLEN (2 * (JOBR_DEPTH - THRESHOLD)) >>> + >> What's the logic behind multiplying by 2? > > I added the 2 as the number of Job Rings. > My logic was that crypto-engine is one per CAAM (x no of JRs), while > JOB_DEPTH is per JR. Currently there is one crypto-engine per JR. crypto_engine_alloc_init() is called when caam/jr driver is probing. > I know there are targets with other than 2 JRs. Therefore, is there a > way to get this number automatically? > Most SoCs have CAAM configured with 4 JRs. Of course what's visible to the kernel might be different. Given there's one crypto-engine / JR, determining the number of JRs shouldn't be required. Horia
Re: [PATCH] crypto: caam - enable crypto-engine retry mechanism
On 10/21/2020 11:07 PM, Iuliana Prodan wrote: > Use the new crypto_engine_alloc_init_and_set() function to > initialize crypto-engine and enable retry mechanism. > > Set the maximum size for crypto-engine software queue based on > Job Ring size (JOBR_DEPTH) and a threshold (reserved for the > non-crypto-API requests that are not pass through crypto-engine). ^ passed > > The callback for do_batch_requests is NULL, since CAAM > doesn't support linked requests. > > Signed-off-by: Iuliana Prodan > --- > drivers/crypto/caam/intern.h | 3 +++ > drivers/crypto/caam/jr.c | 6 +- > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h > index 9112279..44fe6ee 100644 > --- a/drivers/crypto/caam/intern.h > +++ b/drivers/crypto/caam/intern.h > @@ -16,6 +16,9 @@ > /* Currently comes from Kconfig param as a ^2 (driver-required) */ > #define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE) > > +#define THRESHOLD 15 Unless there's a comment added for THRESHOLD (e.g. what's in the commit message), I don't think the define is helpful. > +#define CRYPTO_ENGINE_MAX_QLEN (2 * (JOBR_DEPTH - THRESHOLD)) > + What's the logic behind multiplying by 2? > /* Kconfig params for interrupt coalescing if selected (else zero) */ > #ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_INTC > #define JOBR_INTC JRCFG_ICEN > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > index 6f66996..88540c9 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -550,7 +550,11 @@ static int caam_jr_probe(struct platform_device *pdev) > } > > /* Initialize crypto engine */ > - jrpriv->engine = crypto_engine_alloc_init(jrdev, false); > + jrpriv->engine = crypto_engine_alloc_init_and_set(jrdev, > + true, > + NULL, > + false, > + > CRYPTO_ENGINE_MAX_QLEN); Let's make the argument list more compact. Horia
Re: [PATCH v3 00/10] crypto: caam - xts(aes) updates
On 9/22/2020 7:03 PM, Andrei Botila (OSS) wrote: > From: Andrei Botila > > This patch series fixes some problems in CAAM's implementation of xts(aes): > - CAAM until Era 9 can't process XTS with 16B IV > - CAAM can only process in hardware XTS key lengths of 16B and 32B > - These hardware limitations are resolved through a fallback > - CAAM used to return 0 for XTS block length equal to zero > > This patch series also adds a new feature in CAAM's xts(aes): > - CAAM is now able to process XTS with 16B IV in HW > > Changes since v2: > - modified xts_skcipher_ivsize() based on comments > - squashed the previous 7-9/12 commits > > Changes since v1: > - use only get_unaligned() for calculating XTS IV size > - fixed the double calling of crypto_skcipher_set_reqsize() in case of XTS > - added a patch which modifies the return value for XTS when block length > is equal to zero > > Andrei Botila (10): > crypto: caam/jr - add fallback for XTS with more than 8B IV > crypto: caam/qi - add fallback for XTS with more than 8B IV > crypto: caam/qi2 - add fallback for XTS with more than 8B IV > crypto: caam/jr - add support for more XTS key lengths > crypto: caam/qi - add support for more XTS key lengths > crypto: caam/qi2 - add support for more XTS key lengths > crypto: caam - add xts check for block length equal to zero > crypto: caam/jr - add support for XTS with 16B IV > crypto: caam/qi - add support for XTS with 16B IV > crypto: caam/qi2 - add support for XTS with 16B IV > > drivers/crypto/caam/Kconfig| 3 + > drivers/crypto/caam/caamalg.c | 94 +--- > drivers/crypto/caam/caamalg_desc.c | 27 --- > drivers/crypto/caam/caamalg_qi.c | 94 +--- > drivers/crypto/caam/caamalg_qi2.c | 111 ++--- > drivers/crypto/caam/caamalg_qi2.h | 2 + > 6 files changed, 293 insertions(+), 38 deletions(-) > For the series: Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH v2 00/12] crypto: caam - xts(aes) updates
On 9/21/2020 10:32 AM, Andrei Botila (OSS) wrote: > From: Andrei Botila > > This patch series fixes some problems in CAAM's implementation of xts(aes): > - CAAM until Era 9 can't process XTS with 16B IV > - CAAM can only process in hardware XTS key lengths of 16B and 32B > - These hardware limitations are resolved through a fallback > - CAAM used to return 0 for XTS block length equal to zero > > This patch series also adds a new feature in CAAM's xts(aes): > - CAAM is now able to process XTS with 16B IV in HW > > Changes since v1: > - use only get_unaligned() for calculating XTS IV size > - fixed the double calling of crypto_skcipher_set_reqsize() in case of XTS > - added a patch which modifies the return value for XTS when block length > is equal to zero > Nitpick: The new patches are 7-9/12. Since they have the same Fixes tag and solve the same issue, it would probably be better to squash them into a single patch. Thanks, Horia
Re: [PATCH v2 01/12] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 9/21/2020 10:32 AM, Andrei Botila (OSS) wrote:
> +static bool xts_skcipher_ivsize(struct skcipher_request *req)
> +{
> + struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
> + unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
> + u64 size = 0;
> +
> + size = get_unaligned((u64 *)(req->iv + (ivsize / 2)));
> +
> + return !!size;
Just get rid of the "size" local variable and return !!get_unaligned(...).
Also the function should be inline.
> @@ -3344,13 +3379,30 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
> struct caam_skcipher_alg *caam_alg =
> container_of(alg, typeof(*caam_alg), skcipher);
> struct caam_ctx *ctx = crypto_skcipher_ctx(tfm);
> -
> - crypto_skcipher_set_reqsize(tfm, sizeof(struct caam_skcipher_req_ctx));
> + u32 alg_aai = caam_alg->caam.class1_alg_type & OP_ALG_AAI_MASK;
>
> ctx->enginectx.op.do_one_request = skcipher_do_one_req;
>
> - return caam_init_common(crypto_skcipher_ctx(tfm), _alg->caam,
> - false);
> + if (alg_aai == OP_ALG_AAI_XTS) {
> + const char *tfm_name = crypto_tfm_alg_name(>base);
> + struct crypto_skcipher *fallback;
> +
> + fallback = crypto_alloc_skcipher(tfm_name, 0,
> + CRYPTO_ALG_NEED_FALLBACK);
> + if (IS_ERR(fallback)) {
> + dev_err(ctx->jrdev, "Failed to allocate %s fallback:
> %ld\n",
> + tfm_name, PTR_ERR(fallback));
> + return PTR_ERR(fallback);
> + }
> +
> + ctx->fallback = fallback;
> + crypto_skcipher_set_reqsize(tfm, sizeof(struct
> caam_skcipher_req_ctx) +
> + crypto_skcipher_reqsize(fallback));
> + } else {
> + crypto_skcipher_set_reqsize(tfm, sizeof(struct
> caam_skcipher_req_ctx));
> + }
> +
> + return caam_init_common(ctx, _alg->caam, false);
In case caam_init_common() fails, fallback should be freed for XTS path.
Horia
Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms
On 9/16/2020 12:50 AM, Richard Weinberger wrote: > - Ursprüngliche Mail - >> Von: "horia geanta" How to use it with cryptsetup? I'm asking because it is not clear to me why you are not implementing a new kernel key type (KEYS subsystem) to utilize tagged keys. Many tools already support the keyctl userspace interface (cryptsetup, fscrypt, ...). >>> >>> *friendly ping* >>> >> We didn't include the key management part in this series, >> just the crypto API support for algorithms with protected keys, >> to get early feedback. >> >> Wrt. key management: >> The NXP vendor / downstream kernel (to be included in i.MX BSP Q3 release) >> will have support for protected keys generation. >> Besides this, a dedicated ioctl-based interface will allow userspace to >> generate and export these keys. After this, user can use standard keyctl >> to add a key (as user / logon type) in the keyring, such that it would be >> available to dm-crypt. >> >> We know that adding new ioctls is frowned upon, so before trying to upstream >> the ioctl-based solution the plan is checking the feasibility of >> extending keyctl as David Howells suggested: >> https://lore.kernel.org/lkml/8060.1533226...@warthog.procyon.org.uk >> (Note the difference b/w adding new key type - which was rejected - >> and a key "subtype extension".) > > We have also a keyctl based patch series which should go upstream. > Since we also added a new keytype, it got rejected so far. > Could you please point me to the discussion? > Do you have git repo with the WIP patches available? > Not that we do the work twice. :-) Unfortunately we haven't developed any code yet. > Our patch series also supports DCP beside of CAAM. > By looking at the DCP capabilities, I assume the OTP key that is copied in the key RAM at boot time is used as KEK. If you don't mind sharing, I could review the code. Thanks, Horia
Re: [PATCH -next v2] crypto: caam: Convert to DEFINE_SHOW_ATTRIBUTE
On 9/18/2020 4:30 AM, Qinglang Miao wrote: > Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code. > > Signed-off-by: Qinglang Miao Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms
On 9/14/2020 9:38 AM, Richard Weinberger wrote:
> On Thu, Jul 16, 2020 at 4:12 PM Richard Weinberger
> wrote:
>>
>> On Mon, Jul 13, 2020 at 12:09 AM Iuliana Prodan
>> wrote:
>>>
>>> Tagged keys are keys that contain metadata indicating what
>>> they are and how to handle them using tag_object API.
>>>
>>> Add support, for tagged keys, to skcipher algorithms by
>>> adding new transformations, with _tk_ prefix to distinguish
>>> between plaintext and tagged keys.
>>>
>>> For job descriptors a new option (key_cmd_opt) was added for KEY command.
>>> Tagged keys can be loaded using only a KEY command with ENC=1
>>> and the proper setting of the EKT bit. The EKT bit in the
>>> KEY command indicates which encryption algorithm (AES-ECB or
>>> AES-CCM) should be used to decrypt the key. These options will be kept in
>>> key_cmd_opt.
>>>
>>> The tk_ transformations can be used directly by their name:
>>> struct sockaddr_alg sa = {
>>> .salg_family = AF_ALG,
>>> .salg_type = "skcipher", /* this selects the symmetric cipher */
>>> .salg_name = "tk(cbc(aes))" /* this is the cipher name */
>>> };
>>> or for dm-crypt, e.g. using dmsetup:
>>> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
>>> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
>>> sector_size:512".
>>
>> How to use it with cryptsetup?
>> I'm asking because it is not clear to me why you are not implementing
>> a new kernel key type (KEYS subsystem)
>> to utilize tagged keys.
>> Many tools already support the keyctl userspace interface (cryptsetup,
>> fscrypt, ...).
>
> *friendly ping*
>
We didn't include the key management part in this series,
just the crypto API support for algorithms with protected keys,
to get early feedback.
Wrt. key management:
The NXP vendor / downstream kernel (to be included in i.MX BSP Q3 release)
will have support for protected keys generation.
Besides this, a dedicated ioctl-based interface will allow userspace to
generate and export these keys. After this, user can use standard keyctl
to add a key (as user / logon type) in the keyring, such that it would be
available to dm-crypt.
We know that adding new ioctls is frowned upon, so before trying to upstream
the ioctl-based solution the plan is checking the feasibility of
extending keyctl as David Howells suggested:
https://lore.kernel.org/lkml/8060.1533226...@warthog.procyon.org.uk
(Note the difference b/w adding new key type - which was rejected -
and a key "subtype extension".)
Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 9/15/2020 1:26 PM, Ard Biesheuvel wrote: > On Tue, 15 Sep 2020 at 13:02, Horia Geantă wrote: >> >> On 9/14/2020 9:20 PM, Ard Biesheuvel wrote: >>> On Mon, 14 Sep 2020 at 20:12, Horia Geantă wrote: >>>> >>>> On 9/14/2020 7:28 PM, Ard Biesheuvel wrote: >>>>> On Mon, 14 Sep 2020 at 19:24, Horia Geantă wrote: >>>>>> >>>>>> On 9/9/2020 1:10 AM, Herbert Xu wrote: >>>>>>> On Tue, Sep 08, 2020 at 01:35:04PM +0300, Horia Geantă wrote: >>>>>>>> >>>>>>>>> Just go with the get_unaligned unconditionally. >>>>>>>> >>>>>>>> Won't this lead to sub-optimal code for ARMv7 >>>>>>>> in case the IV is aligned? >>>>>>> >>>>>>> If this should be optimised in ARMv7 then that should be done >>>>>>> in get_unaligned itself and not open-coded. >>>>>>> >>>>>> I am not sure what's wrong with avoiding using the unaligned accessors >>>>>> in case data is aligned. >>>>>> >>>>>> Documentation/core-api/unaligned-memory-access.rst clearly states: >>>>>> These macros work for memory accesses of any length (not just 32 bits as >>>>>> in the examples above). Be aware that when compared to standard access of >>>>>> aligned memory, using these macros to access unaligned memory can be >>>>>> costly in >>>>>> terms of performance. >>>>>> >>>>>> So IMO it makes sense to use get_unaligned() only when needed. >>>>>> There are several cases of users doing this, e.g. siphash. >>>>>> >>>>> >>>>> For ARMv7 code, using the unaligned accessors unconditionally is fine, >>>>> and it will not affect performance. >>>>> >>>>> In general, when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is defined, >>>>> you can use the unaligned accessors. If it is not, it helps to have >>>>> different code paths. >>>>> >>>> arch/arm/include/asm/unaligned.h doesn't make use of >>>> linux/unaligned/access_ok.h, even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS >>>> is set. >>>> >>>> I understand the comment in the file, however using get_unaligned() >>>> unconditionally takes away the opportunity to generate optimized code >>>> (using ldrd/ldm) when data is aligned. >>>> >>> >>> But the minimal optimization that is possible here (one ldrd/ldm >>> instruction vs two ldr instructions) is defeated by the fact that you >>> are using a conditional branch to select between the two. And this is >>> not even a hot path to begin with, >>> >> This is actually on the hot path (encrypt/decrypt callbacks), >> but you're probably right that the conditional branching is going to offset >> the optimized code. >> > > This is called once per XTS request, right? And you are saying the > extra cycle makes a difference? > Yes, once per request and no, not super-important. >> To avoid branching, code could be rewritten as: >> >> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS >> size = *(u64 *)(req->iv + (ivsize / 2)); >> #else >> size = get_unaligned((u64 *)(req->iv + (ivsize / 2))); >> #endif >> >> however in this case ARMv7 would suffer since >> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y and >> ldrd/ldm for accesses not word-aligned are inefficient - lead to traps. >> > > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS means 'just use the unaligned > accessors as they are basically free'. Casting a potentially > misaligned u8* to a u64* is not permitted by the C standard. > Seems that I misunderstood CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. Looking at its usage, e.g. ether_addr_equal() or __crypto_memneq_*(), I see similar casts of pointers possibly misaligned. >> Would it be ok to use: >> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && !defined(CONFIG_ARM) >> to workaround the ARMv7 inconsistency? >> > > No, please just use the get_unaligned() accessor. > Ok. Thanks, Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 9/14/2020 9:20 PM, Ard Biesheuvel wrote: > On Mon, 14 Sep 2020 at 20:12, Horia Geantă wrote: >> >> On 9/14/2020 7:28 PM, Ard Biesheuvel wrote: >>> On Mon, 14 Sep 2020 at 19:24, Horia Geantă wrote: >>>> >>>> On 9/9/2020 1:10 AM, Herbert Xu wrote: >>>>> On Tue, Sep 08, 2020 at 01:35:04PM +0300, Horia Geantă wrote: >>>>>> >>>>>>> Just go with the get_unaligned unconditionally. >>>>>> >>>>>> Won't this lead to sub-optimal code for ARMv7 >>>>>> in case the IV is aligned? >>>>> >>>>> If this should be optimised in ARMv7 then that should be done >>>>> in get_unaligned itself and not open-coded. >>>>> >>>> I am not sure what's wrong with avoiding using the unaligned accessors >>>> in case data is aligned. >>>> >>>> Documentation/core-api/unaligned-memory-access.rst clearly states: >>>> These macros work for memory accesses of any length (not just 32 bits as >>>> in the examples above). Be aware that when compared to standard access of >>>> aligned memory, using these macros to access unaligned memory can be >>>> costly in >>>> terms of performance. >>>> >>>> So IMO it makes sense to use get_unaligned() only when needed. >>>> There are several cases of users doing this, e.g. siphash. >>>> >>> >>> For ARMv7 code, using the unaligned accessors unconditionally is fine, >>> and it will not affect performance. >>> >>> In general, when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is defined, >>> you can use the unaligned accessors. If it is not, it helps to have >>> different code paths. >>> >> arch/arm/include/asm/unaligned.h doesn't make use of >> linux/unaligned/access_ok.h, even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS >> is set. >> >> I understand the comment in the file, however using get_unaligned() >> unconditionally takes away the opportunity to generate optimized code >> (using ldrd/ldm) when data is aligned. >> > > But the minimal optimization that is possible here (one ldrd/ldm > instruction vs two ldr instructions) is defeated by the fact that you > are using a conditional branch to select between the two. And this is > not even a hot path to begin with, > This is actually on the hot path (encrypt/decrypt callbacks), but you're probably right that the conditional branching is going to offset the optimized code. To avoid branching, code could be rewritten as: #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS size = *(u64 *)(req->iv + (ivsize / 2)); #else size = get_unaligned((u64 *)(req->iv + (ivsize / 2))); #endif however in this case ARMv7 would suffer since CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y and ldrd/ldm for accesses not word-aligned are inefficient - lead to traps. Would it be ok to use: #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && !defined(CONFIG_ARM) to workaround the ARMv7 inconsistency? Thanks, Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 9/14/2020 7:28 PM, Ard Biesheuvel wrote: > On Mon, 14 Sep 2020 at 19:24, Horia Geantă wrote: >> >> On 9/9/2020 1:10 AM, Herbert Xu wrote: >>> On Tue, Sep 08, 2020 at 01:35:04PM +0300, Horia Geantă wrote: >>>> >>>>> Just go with the get_unaligned unconditionally. >>>> >>>> Won't this lead to sub-optimal code for ARMv7 >>>> in case the IV is aligned? >>> >>> If this should be optimised in ARMv7 then that should be done >>> in get_unaligned itself and not open-coded. >>> >> I am not sure what's wrong with avoiding using the unaligned accessors >> in case data is aligned. >> >> Documentation/core-api/unaligned-memory-access.rst clearly states: >> These macros work for memory accesses of any length (not just 32 bits as >> in the examples above). Be aware that when compared to standard access of >> aligned memory, using these macros to access unaligned memory can be costly >> in >> terms of performance. >> >> So IMO it makes sense to use get_unaligned() only when needed. >> There are several cases of users doing this, e.g. siphash. >> > > For ARMv7 code, using the unaligned accessors unconditionally is fine, > and it will not affect performance. > > In general, when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is defined, > you can use the unaligned accessors. If it is not, it helps to have > different code paths. > arch/arm/include/asm/unaligned.h doesn't make use of linux/unaligned/access_ok.h, even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set. I understand the comment in the file, however using get_unaligned() unconditionally takes away the opportunity to generate optimized code (using ldrd/ldm) when data is aligned. > This is a bit murky, and through the years, the interpretation of > unaligned-memory-access.rst has shifted a bit, but in this case, it > makes no sense to make the distinction. > Thanks, Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 9/9/2020 1:10 AM, Herbert Xu wrote: > On Tue, Sep 08, 2020 at 01:35:04PM +0300, Horia Geantă wrote: >> >>> Just go with the get_unaligned unconditionally. >> >> Won't this lead to sub-optimal code for ARMv7 >> in case the IV is aligned? > > If this should be optimised in ARMv7 then that should be done > in get_unaligned itself and not open-coded. > I am not sure what's wrong with avoiding using the unaligned accessors in case data is aligned. Documentation/core-api/unaligned-memory-access.rst clearly states: These macros work for memory accesses of any length (not just 32 bits as in the examples above). Be aware that when compared to standard access of aligned memory, using these macros to access unaligned memory can be costly in terms of performance. So IMO it makes sense to use get_unaligned() only when needed. There are several cases of users doing this, e.g. siphash. Thanks, Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 8/21/2020 6:47 AM, Herbert Xu wrote:
> On Thu, Aug 06, 2020 at 07:35:43PM +0300, Andrei Botila wrote:
>>
>> +static bool xts_skcipher_ivsize(struct skcipher_request *req)
>> +{
>> +struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
>> +unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
>> +u64 size = 0;
>> +
>> +if (IS_ALIGNED((unsigned long)req->iv, __alignof__(u64)))
>> +size = *(u64 *)(req->iv + (ivsize / 2));
>> +else
>> +size = get_unaligned((u64 *)(req->iv + (ivsize / 2)));
>> +
>> +return !!size;
>> +}
>
> Just go with the get_unaligned unconditionally.
>
Won't this lead to sub-optimal code for ARMv7
in case the IV is aligned?
Thanks,
Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 8/21/2020 6:47 AM, Herbert Xu wrote:
> On Tue, Aug 11, 2020 at 05:30:41PM +0300, Horia Geantă wrote:
>>
>>> + if (IS_ERR(fallback)) {
>>> + pr_err("Failed to allocate %s fallback: %ld\n",
>>> + tfm_name, PTR_ERR(fallback));
>>> + return PTR_ERR(fallback);
>> Shouldn't error out so early. It might be that the fallback won't be needed.
>> Let's postpone this until we're sure fallback is required.
>
> Why? The generic should always be there as otherwise you won't
> even pass the self-test. If we're OOM then we should error out
> ASAP.
>
self-tests don't check the cases where a fallback is needed,
so in theory things could go well for a misconfigured kernel
(where a fallback is not available).
But I agree, if driver is updated to select CRYPTO_XTS
then it's probably better to return an error here.
Thanks,
Horia
Re: [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
On 8/10/2020 8:03 PM, Eric Biggers wrote: > On Mon, Aug 10, 2020 at 05:33:39PM +0300, Horia Geantă wrote: >> On 8/10/2020 4:45 PM, Herbert Xu wrote: >>> On Mon, Aug 10, 2020 at 10:20:20AM +, Van Leeuwen, Pascal wrote: >>>> >>>> With all due respect, but this makes no sense. >>> >>> I agree. This is a lot of churn for no gain. >>> >> I would say the gain is that all skcipher algorithms would behave the same >> when input length equals zero - i.e. treat the request as a no-op. >> >> We can't say "no input" has any meaning to the other skcipher algorithms, >> but the convention is to accept this case and just return 0. >> I don't see why XTS has to be handled differently. >> > > CTS also rejects empty inputs. > > The rule it follows is just that all input lengths >= blocksize are allowed. > Input lengths < blocksize aren't allowed. > Indeed, thanks. What about, for example, CBC? AFAICT cbc(aes) with input length = 0 is valid. Same for CTR (with the note that blocksize = 1) and several other algorithms mentioned in the cover letter. What's the rule in these cases? Thanks, Horia
Re: [PATCH RESEND 4/9] crypto: caam/jr - add support for more XTS key lengths
On 8/6/2020 7:36 PM, Andrei Botila (OSS) wrote:
> @@ -1790,7 +1792,9 @@ static inline int skcipher_crypt(struct
> skcipher_request *req, bool encrypt)
> if (!req->cryptlen)
> return 0;
>
> - if (ctx->fallback && xts_skcipher_ivsize(req)) {
> + if (ctx->fallback && (xts_skcipher_ivsize(req) ||
> + (ctx->cdata.keylen != 2 * AES_KEYSIZE_128 &&
> +ctx->cdata.keylen != 2 * AES_KEYSIZE_256))) {
Let's avoid doing this check for every request.
This could be achieved by moving it into the .setkey callback and
setting a flag in the caam_ctx indicating if the fallback is needed or not
for this tfm.
Horia
Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
On 8/6/2020 7:36 PM, Andrei Botila (OSS) wrote:
> @@ -3344,12 +3382,30 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
> struct caam_skcipher_alg *caam_alg =
> container_of(alg, typeof(*caam_alg), skcipher);
> struct caam_ctx *ctx = crypto_skcipher_ctx(tfm);
> + u32 alg_aai = caam_alg->caam.class1_alg_type & OP_ALG_AAI_MASK;
>
> crypto_skcipher_set_reqsize(tfm, sizeof(struct caam_skcipher_req_ctx));
This is being called twice in case of XTS.
>
> ctx->enginectx.op.do_one_request = skcipher_do_one_req;
>
> - return caam_init_common(crypto_skcipher_ctx(tfm), _alg->caam,
> + if (alg_aai == OP_ALG_AAI_XTS) {
> + const char *tfm_name = crypto_tfm_alg_name(>base);
> + struct crypto_skcipher *fallback;
> +
> + fallback = crypto_alloc_skcipher(tfm_name, 0,
> + CRYPTO_ALG_NEED_FALLBACK);
Driver should select CRYPTO_XTS, such that at least the generic
xts implementation is available.
> + if (IS_ERR(fallback)) {
> + pr_err("Failed to allocate %s fallback: %ld\n",
> +tfm_name, PTR_ERR(fallback));
> + return PTR_ERR(fallback);
Shouldn't error out so early. It might be that the fallback won't be needed.
Let's postpone this until we're sure fallback is required.
Horia
Re: [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
On 8/10/2020 4:45 PM, Herbert Xu wrote: > On Mon, Aug 10, 2020 at 10:20:20AM +, Van Leeuwen, Pascal wrote: >> >> With all due respect, but this makes no sense. > > I agree. This is a lot of churn for no gain. > I would say the gain is that all skcipher algorithms would behave the same when input length equals zero - i.e. treat the request as a no-op. We can't say "no input" has any meaning to the other skcipher algorithms, but the convention is to accept this case and just return 0. I don't see why XTS has to be handled differently. Thanks, Horia
[PATCH] ARM: multi_v7_defconfig: enable caam crypto module
caam crypto module is included in several ARMv7-based SoCs from i.MX, Layerscape, Vybrid families. Signed-off-by: Horia Geantă --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index e9e76e32f10f..a024965ea35b 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -1121,6 +1121,7 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m CONFIG_CRYPTO_USER_API_RNG=m CONFIG_CRYPTO_USER_API_AEAD=m CONFIG_CRYPTO_DEV_SUN4I_SS=m +CONFIG_CRYPTO_DEV_FSL_CAAM=m CONFIG_CRYPTO_DEV_MARVELL_CESA=m CONFIG_CRYPTO_DEV_EXYNOS_RNG=m CONFIG_CRYPTO_DEV_S5P=m -- 2.17.1
[PATCH v3 1/3] dt-bindings: crypto: fsl-sec4: add snvs clock to pwrkey
From: André Draszik
On i.MX7 and i.MX8M, the SNVS requires a clock. This is similar to the
clock bound to the SNVS RTC node, but if the SNVS RTC driver isn't
enabled, then SNVS doesn't work, and as such the pwrkey driver doesn't
work (i.e. hangs the kernel, as the clock isn't enabled).
Also see commit ec2a844ef7c1
("ARM: dts: imx7s: add snvs rtc clock")
for a similar fix.
Signed-off-by: André Draszik
Acked-by: Rob Herring
Reviewed-by: Horia Geantă
Signed-off-by: Horia Geantă
---
.../devicetree/bindings/crypto/fsl-sec4.txt | 17 +
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
index 8f359f473ada..1800d57edb66 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -449,6 +449,19 @@ System ON/OFF key driver
Value type:
Definition: this is phandle to the register map node.
+ - clocks
+ Usage: optional, required if SNVS LP requires explicit
+ enablement of clocks
+ Value type:
+ Definition: a clock specifier describing the clock required for
+ enabling and disabling SNVS LP.
+
+ - clock-names
+ Usage: optional, required if SNVS LP requires explicit
+ enablement of clocks
+ Value type:
+ Definition: clock name string should be "snvs-pwrkey".
+
EXAMPLE:
snvs-pwrkey@020cc000 {
compatible = "fsl,sec-v4.0-pwrkey";
@@ -456,6 +469,8 @@ EXAMPLE:
interrupts = <0 4 0x4>
linux,keycode = <116>; /* KEY_POWER */
wakeup-source;
+ clocks = < IMX7D_SNVS_CLK>;
+ clock-names = "snvs-pwrkey";
};
=
@@ -547,6 +562,8 @@ FULL EXAMPLE
interrupts = <0 4 0x4>;
linux,keycode = <116>; /* KEY_POWER */
wakeup-source;
+ clocks = < IMX7D_SNVS_CLK>;
+ clock-names = "snvs-pwrkey";
};
};
--
2.17.1
[PATCH v3 2/3] Input: snvs_pwrkey - enable snvs clock as needed
From: André Draszik
At the moment, enabling this driver without the SNVS RTC driver
being active will hang the kernel as soon as the power button
is pressed.
The reason is that in that case the SNVS isn't enabled, and
any attempt to read the SNVS registers will simply hang forever.
Ensure the clock is enabled (during the interrupt handler) to
make this driver work.
Also see commit 7f8993995410 ("drivers/rtc/rtc-snvs: add clock support")
and commit edb190cb1734
("rtc: snvs: make sure clock is enabled for interrupt handle")
for similar updates to the snvs rtc driver.
Signed-off-by: André Draszik
Reviewed-by: Horia Geantă
Signed-off-by: Horia Geantă
---
drivers/input/keyboard/snvs_pwrkey.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c
b/drivers/input/keyboard/snvs_pwrkey.c
index 2f5e3ab5ed63..382d2ae82c9b 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -38,6 +39,7 @@ struct pwrkey_drv_data {
int wakeup;
struct timer_list check_timer;
struct input_dev *input;
+ struct clk *clk;
u8 minor_rev;
};
@@ -47,7 +49,10 @@ static void imx_imx_snvs_check_for_events(struct timer_list
*t)
struct input_dev *input = pdata->input;
u32 state;
+ clk_enable(pdata->clk);
regmap_read(pdata->snvs, SNVS_HPSR_REG, );
+ clk_disable(pdata->clk);
+
state = state & SNVS_HPSR_BTN ? 1 : 0;
/* only report new event if status changed */
@@ -74,11 +79,13 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
*dev_id)
pm_wakeup_event(input->dev.parent, 0);
+ clk_enable(pdata->clk);
+
regmap_read(pdata->snvs, SNVS_LPSR_REG, _status);
if (lp_status & SNVS_LPSR_SPO) {
if (pdata->minor_rev == 0) {
/*
-* The first generation i.MX6 SoCs only sends an
+* The first generation i.MX[6|7] SoCs only send an
* interrupt on button release. To mimic power-key
* usage, we'll prepend a press event.
*/
@@ -96,6 +103,8 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
*dev_id)
/* clear SPO status */
regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
+ clk_disable(pdata->clk);
+
return IRQ_HANDLED;
}
@@ -140,6 +149,23 @@ static int imx_snvs_pwrkey_probe(struct platform_device
*pdev)
if (pdata->irq < 0)
return -EINVAL;
+ pdata->clk = devm_clk_get_optional(>dev, "snvs-pwrkey");
+ if (IS_ERR(pdata->clk))
+ return PTR_ERR(pdata->clk);
+
+ error = clk_prepare(pdata->clk);
+ if (error) {
+ dev_err(>dev, "failed to prepare the snvs clock\n");
+ return error;
+ }
+ error = devm_add_action_or_reset(>dev,
+ (void(*)(void *))clk_unprepare,
+ pdata->clk);
+ if (error) {
+ dev_err(>dev, "failed to add reset action on
'snvs-pwrkey'");
+ return error;
+ }
+
regmap_read(pdata->snvs, SNVS_HPVIDR1_REG, );
pdata->minor_rev = vid & 0xff;
--
2.17.1
[PATCH v3 3/3] Input: snvs_pwrkey - only IRQ_HANDLED for our own events
From: André Draszik
The snvs_pwrkey shares the SNVS LPSR status register with the snvs_rtc.
This driver here should only return IRQ_HANDLED if the status register
indicates that the event we're handling in the irq handler was genuinely
intended for this driver. Otheriwse the interrupt subsystem will
assume the interrupt was handled successfully even though it wasn't
at all.
Signed-off-by: André Draszik
Reviewed-by: Horia Geantă
Signed-off-by: Horia Geantă
---
drivers/input/keyboard/snvs_pwrkey.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c
b/drivers/input/keyboard/snvs_pwrkey.c
index 382d2ae82c9b..980867886b34 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -82,7 +82,9 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
*dev_id)
clk_enable(pdata->clk);
regmap_read(pdata->snvs, SNVS_LPSR_REG, _status);
- if (lp_status & SNVS_LPSR_SPO) {
+ lp_status &= SNVS_LPSR_SPO;
+
+ if (lp_status) {
if (pdata->minor_rev == 0) {
/*
* The first generation i.MX[6|7] SoCs only send an
@@ -98,14 +100,14 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
*dev_id)
mod_timer(>check_timer,
jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
}
- }
- /* clear SPO status */
- regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
+ /* clear SPO status */
+ regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
+ }
clk_disable(pdata->clk);
- return IRQ_HANDLED;
+ return lp_status ? IRQ_HANDLED : IRQ_NONE;
}
static void imx_snvs_pwrkey_act(void *pdata)
--
2.17.1
[PATCH v3 0/3] input: i.MX snvs powerkey updates
Hi Herbert, Dmitry, This is a resend of v2 patches 1,5,6 that were not picked up https://lore.kernel.org/linux-input/20200225161201.1975-1-...@andred.net with collecting Acked-by, Reviewed-by. I skipped Robin's Reviewed-by since I prefer avoiding misintepreting the discussion between him and André. Thanks, Horia André Draszik (3): dt-bindings: crypto: fsl-sec4: add snvs clock to pwrkey Input: snvs_pwrkey - enable snvs clock as needed Input: snvs_pwrkey - only IRQ_HANDLED for our own events .../devicetree/bindings/crypto/fsl-sec4.txt | 17 + drivers/input/keyboard/snvs_pwrkey.c | 38 --- 2 files changed, 50 insertions(+), 5 deletions(-) -- 2.17.1
Re: [PATCH 6/7] crypto: caam - add more RNG hw error codes
On 7/22/2020 3:15 PM, Horia Geantă wrote: > In some cases, e.g. when TRNG is not properly configured, > the RNG module could issue a "Hardware error" at runtime. > > "Continuos check" error is emitted when some of the BISTs fail. > > Signed-off-by: Horia Geantă > Signed-off-by: Horia Geantă Oops, somehow the deprecated freescale address made its way through. If there won't be other objections to the patch set, maybe you could fix this so that a v2 is not needed. Thanks, Horia
[PATCH 5/7] crypto: caam/jr - remove incorrect reference to caam_jr_register()
From: Dan Douglass
caam_jr_register() function is no longer part of the driver since
commit 6dad41158db6 ("crypto: caam - Remove unused functions from Job Ring")
This patch removes a comment referencing the function.
Signed-off-by: Dan Douglass
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/jr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 4af22e7ceb4f..bf6b03b17251 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -339,8 +339,7 @@ EXPORT_SYMBOL(caam_jr_free);
* caam_jr_enqueue() - Enqueue a job descriptor head. Returns -EINPROGRESS
* if OK, -ENOSPC if the queue is full, -EIO if it cannot map the caller's
* descriptor.
- * @dev: device of the job ring to be used. This device should have
- *been assigned prior by caam_jr_register().
+ * @dev: struct device of the job ring to be used
* @desc: points to a job descriptor that execute our request. All
*descriptors (and all referenced data) must be in a DMAable
*region, and all data references must be physical addresses
--
2.17.1
[PATCH 3/7] crypto: caam/qi2 - create ahash shared descriptors only once
For keyed hash algorithms, shared descriptors are currently generated
twice:
-at tfm initialization time, in cra_init() callback
-in setkey() callback
Since it's mandatory to call setkey() for keyed algorithms, drop the
generation in cra_init().
This is similar to the change in caamhash (caam/jr top-level library)
commit 9a2537d0ebc9 ("crypto: caam - create ahash shared descriptors only once")
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caamalg_qi2.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 811d34eee4f2..1e901344db67 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -4500,7 +4500,11 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
sizeof(struct caam_hash_state));
- return ahash_set_sh_desc(ahash);
+ /*
+* For keyed hash algorithms shared descriptors
+* will be created later in setkey() callback
+*/
+ return alg->setkey ? 0 : ahash_set_sh_desc(ahash);
}
static void caam_hash_cra_exit(struct crypto_tfm *tfm)
--
2.17.1
[PATCH 7/7] crypto: caam/qi2 - add module alias
Add a module alias, to enable udev-based module autoloading:
$ modinfo -F alias drivers/crypto/caam/dpaa2_caam.ko
fsl-mc:v1957ddpseci
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caamalg_qi2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 700e1d50211f..66ae1d581168 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -5405,6 +5405,7 @@ static const struct fsl_mc_device_id
dpaa2_caam_match_id_table[] = {
},
{ .vendor = 0x0 }
};
+MODULE_DEVICE_TABLE(fslmc, dpaa2_caam_match_id_table);
static struct fsl_mc_driver dpaa2_caam_driver = {
.driver = {
--
2.17.1
[PATCH 1/7] crypto: caam - remove deadcode on 32-bit platforms
From: Franck LENORMAND
When building on a platform with a 32bit DMA address, taking the
upper 32 bits makes no sense.
Signed-off-by: Franck LENORMAND
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/regs.h | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 0f810bc13b2b..af61f3a2c0d4 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -173,9 +173,14 @@ static inline u64 rd_reg64(void __iomem *reg)
static inline u64 cpu_to_caam_dma64(dma_addr_t value)
{
- if (caam_imx)
- return (((u64)cpu_to_caam32(lower_32_bits(value)) << 32) |
-(u64)cpu_to_caam32(upper_32_bits(value)));
+ if (caam_imx) {
+ u64 ret_val = (u64)cpu_to_caam32(lower_32_bits(value)) << 32;
+
+ if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
+ ret_val |= (u64)cpu_to_caam32(upper_32_bits(value));
+
+ return ret_val;
+ }
return cpu_to_caam64(value);
}
--
2.17.1
[PATCH 6/7] crypto: caam - add more RNG hw error codes
In some cases, e.g. when TRNG is not properly configured,
the RNG module could issue a "Hardware error" at runtime.
"Continuos check" error is emitted when some of the BISTs fail.
Signed-off-by: Horia Geantă
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/error.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 17c6108b6d41..72db90176b1a 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -212,6 +212,9 @@ static const char * const rng_err_id_list[] = {
"Prediction resistance and test request",
"Uninstantiate",
"Secure key generation",
+ "",
+ "Hardware error",
+ "Continuous check"
};
static int report_ccb_status(struct device *jrdev, const u32 status,
--
2.17.1
[PATCH 2/7] crypto: caam/qi2 - fix error reporting for caam_hash_alloc
Fix error reporting when preparation of an hmac algorithm
for registration fails: print the hmac algorithm name, not the unkeyed
hash algorithm name.
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caamalg_qi2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 1b0c28675906..811d34eee4f2 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -5238,7 +5238,7 @@ static int dpaa2_caam_probe(struct fsl_mc_device
*dpseci_dev)
if (IS_ERR(t_alg)) {
err = PTR_ERR(t_alg);
dev_warn(dev, "%s hash alg allocation failed: %d\n",
-alg->driver_name, err);
+alg->hmac_driver_name, err);
continue;
}
--
2.17.1
[PATCH 4/7] crypto: caam - silence .setkey in case of bad key length
In case of bad key length, driver emits "key size mismatch" messages,
but only for xts(aes) algorithms.
Reduce verbosity by making them visible only when debugging.
This way crypto fuzz testing log cleans up a bit.
Signed-off-by: Horia Geantă
---
drivers/crypto/caam/caamalg.c | 2 +-
drivers/crypto/caam/caamalg_qi.c | 2 +-
drivers/crypto/caam/caamalg_qi2.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index e94e7f27f1d0..91feda5b63f6 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -832,7 +832,7 @@ static int xts_skcipher_setkey(struct crypto_skcipher
*skcipher, const u8 *key,
u32 *desc;
if (keylen != 2 * AES_MIN_KEY_SIZE && keylen != 2 * AES_MAX_KEY_SIZE) {
- dev_err(jrdev, "key size mismatch\n");
+ dev_dbg(jrdev, "key size mismatch\n");
return -EINVAL;
}
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index efe8f15a4a51..bb1c0106a95c 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -728,7 +728,7 @@ static int xts_skcipher_setkey(struct crypto_skcipher
*skcipher, const u8 *key,
int ret = 0;
if (keylen != 2 * AES_MIN_KEY_SIZE && keylen != 2 * AES_MAX_KEY_SIZE) {
- dev_err(jrdev, "key size mismatch\n");
+ dev_dbg(jrdev, "key size mismatch\n");
return -EINVAL;
}
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 1e901344db67..700e1d50211f 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1058,7 +1058,7 @@ static int xts_skcipher_setkey(struct crypto_skcipher
*skcipher, const u8 *key,
u32 *desc;
if (keylen != 2 * AES_MIN_KEY_SIZE && keylen != 2 * AES_MAX_KEY_SIZE) {
- dev_err(dev, "key size mismatch\n");
+ dev_dbg(dev, "key size mismatch\n");
return -EINVAL;
}
--
2.17.1
[PATCH 0/7] crypto: caam - updates for 5.9
Hi Herbert, This patch set contains a few caam driver updates. The fixes are minor and thus ok to go through the cryptodev tree. Dan Douglass (1): crypto: caam/jr - remove incorrect reference to caam_jr_register() Franck LENORMAND (1): crypto: caam - remove deadcode on 32-bit platforms Horia Geantă (5): crypto: caam/qi2 - fix error reporting for caam_hash_alloc crypto: caam/qi2 - create ahash shared descriptors only once crypto: caam - silence .setkey in case of bad key length crypto: caam - add more RNG hw error codes crypto: caam/qi2 - add module alias drivers/crypto/caam/caamalg.c | 2 +- drivers/crypto/caam/caamalg_qi.c | 2 +- drivers/crypto/caam/caamalg_qi2.c | 11 --- drivers/crypto/caam/error.c | 3 +++ drivers/crypto/caam/jr.c | 3 +-- drivers/crypto/caam/regs.h| 11 --- 6 files changed, 22 insertions(+), 10 deletions(-) -- 2.17.1
Re: [PATCH -next] crypto: caam: Convert to DEFINE_SHOW_ATTRIBUTE
On 7/16/2020 12:00 PM, Qinglang Miao wrote:
> From: Liu Shixin
>
> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>
> Signed-off-by: Liu Shixin
Reviewed-by: Horia Geantă
This patch depends on linux-next
commit 4d4901c6d748 ("seq_file: switch over direct seq_read method calls to
seq_read_iter")
Horia
Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms
On 7/16/2020 2:53 PM, Herbert Xu wrote: > On Thu, Jul 16, 2020 at 01:35:51PM +0300, Horia Geantă wrote: >> >> This patch set adds support only for some AES-based algorithms. >> However, going further the plan is to add all keyed algorithms >> supported by caam. >> >> Thus I wouldn't tie the name to AES. > > Yes but it's still exactly the same underlying feature as paes. > So I don't want to have two ways of doing the same thing in the > Crypto API. > So instead of tk(cbc(aes)) use paes(cbc(aes) or cbc(paes)? How would this work for hmac(sha512), paes(hmac(sha512)) or hmac(psha512), or even phmac(sha512)? Thanks, Horia
Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms
On 7/16/2020 10:36 AM, Herbert Xu wrote:
> On Mon, Jul 13, 2020 at 01:05:36AM +0300, Iuliana Prodan wrote:
>> Tagged keys are keys that contain metadata indicating what
>> they are and how to handle them using tag_object API.
>>
>> Add support, for tagged keys, to skcipher algorithms by
>> adding new transformations, with _tk_ prefix to distinguish
>> between plaintext and tagged keys.
>>
>> For job descriptors a new option (key_cmd_opt) was added for KEY command.
>> Tagged keys can be loaded using only a KEY command with ENC=1
>> and the proper setting of the EKT bit. The EKT bit in the
>> KEY command indicates which encryption algorithm (AES-ECB or
>> AES-CCM) should be used to decrypt the key. These options will be kept in
>> key_cmd_opt.
>>
>> The tk_ transformations can be used directly by their name:
>> struct sockaddr_alg sa = {
>> .salg_family = AF_ALG,
>> .salg_type = "skcipher", /* this selects the symmetric cipher */
>> .salg_name = "tk(cbc(aes))" /* this is the cipher name */
>> };
>> or for dm-crypt, e.g. using dmsetup:
>> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
>> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
>> sector_size:512".
>>
>> Signed-off-by: Franck LENORMAND
>> Signed-off-by: Iuliana Prodan
>
> Can this use the existing paes name instead of tk as done in
> other drivers?
>
This patch set adds support only for some AES-based algorithms.
However, going further the plan is to add all keyed algorithms
supported by caam.
Thus I wouldn't tie the name to AES.
Possible alternatives would be:
pk - protected keys
tk - with "t" standing for "trusted" instead of "tagged"
Wrt. "trusted", I am not sure this term should strictly be tied
to a TPM or not.
Thanks,
Horia
Re: [PATCH 1/2] crypto: caam - add tag object functionality
On 7/13/2020 1:05 AM, Iuliana Prodan wrote:
> A tag object represents the metadata (or simply a header/configuration)
> and the actual data (e.g. black key) obtained from hardware.
> Add functionality to tag an object with metadata:
> - validate metadata: check tag object header;
> - retrieve metadata: get tag object header configuration, black key
> configuration or tag object data.
>
> This API expects that the object (the actual data) from a tag object
> to be a buffer (defined by address and size).
>
> Signed-off-by: Franck LENORMAND
Signed-off-by: Horia Geantă
> Signed-off-by: Iuliana Prodan
> ---
> drivers/crypto/caam/Kconfig | 9 +++
> drivers/crypto/caam/Makefile | 1 +
> drivers/crypto/caam/desc.h | 4 +-
> drivers/crypto/caam/tag_object.c | 129
> +++
> drivers/crypto/caam/tag_object.h | 99 ++
> 5 files changed, 241 insertions(+), 1 deletion(-)
> create mode 100644 drivers/crypto/caam/tag_object.c
> create mode 100644 drivers/crypto/caam/tag_object.h
>
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index bc35aa0..73368d8 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -149,6 +149,15 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API
> Selecting this will register the SEC4 hardware rng to
> the hw_random API for supplying the kernel entropy pool.
>
> +config CRYPTO_DEV_FSL_CAAM_TK_API
> + bool "Register tagged key cryptography implementations with Crypto API"
> + depends on CRYPTO_DEV_FSL_CAAM_CRYPTO_API
> + help
> + Selecting this will register algorithms supporting tagged key.
> +
> + Tagged keys are keys that contain metadata indicating what
> + they are and how to handle them.
> +
Let's keep config options at minimum, we've got plenty of them already.
Please get rid of this entry (and the associated ifdeffery etc.).
> endif # CRYPTO_DEV_FSL_CAAM_JR
>
> endif # CRYPTO_DEV_FSL_CAAM
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 68d5cc0..192a88e 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC) +=
> caamhash_desc.o
>
> caam-y := ctrl.o
> caam_jr-y := jr.o key_gen.o
> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API) += tag_object.o
> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
> diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h
> index e134709..3001a8d 100644
> --- a/drivers/crypto/caam/desc.h
> +++ b/drivers/crypto/caam/desc.h
> @@ -152,7 +152,7 @@
> * with the TDKEK if TK is set
> */
> #define KEY_ENC 0x0040
> -
> +#define KEY_ENC_OFFSET 22
Missing empty line b/w groups of defines.
> /*
> * No Write Back - Do not allow key to be FIFO STOREd
> */
> @@ -162,11 +162,13 @@
> * Enhanced Encryption of Key
> */
> #define KEY_EKT 0x0010
> +#define KEY_EKT_OFFSET 20
>
> /*
> * Encrypted with Trusted Key
> */
> #define KEY_TK 0x8000
> +#define KEY_TK_OFFSET15
>
> /*
> * KDEST - Key Destination: 0 - class key register,
> diff --git a/drivers/crypto/caam/tag_object.c
> b/drivers/crypto/caam/tag_object.c
> new file mode 100644
> index ..55f41e9
> --- /dev/null
> +++ b/drivers/crypto/caam/tag_object.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> +/*
> + * Copyright 2018-2020 NXP
> + */
> +
> +#include
> +#include
Why is this include needed?
> +#include
> +
> +#include "tag_object.h"
> +#include "desc.h"
> +
> +/**
> + * is_black_key -Check if the tag object header is a black key
> + * @header: The tag object header configuration
> + *
> + * Return: True if is a black key, false otherwise
> + */
> +bool is_black_key(const struct header_conf *header)
> +{
> + u32 type = header->type;
> + /* Check type and color bitfields from tag object type */
> + return (type & (BIT(TAG_OBJ_COLOR_OFFSET) |
> + BIT(TAG_OBJ_TYPE_OFFSET))) == BIT(TAG_OBJ_COLOR_OFFSET);
> +}
> +EXPORT_SYMBOL(is_black_key);
Exported symbols should be named such that the probability of
collision with others is neglijible.
> +
> +/**
> + * is_valid_header_conf - Check if the header configuration is va
[PATCH v4 2/5] ARM: dts: imx6sl: fix rng node
rng DT node was added without a compatible string.
i.MX driver for RNGC (drivers/char/hw_random/imx-rngc.c) also claims
support for RNGB, and is currently used for i.MX25.
Let's use this driver also for RNGB block in i.MX6SL.
Fixes: e29fe21cff96 ("ARM: dts: add device tree source for imx6sl SoC")
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sl.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 1c7180f28539..91a8c54d5e11 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -939,8 +939,10 @@
};
rngb: rngb@21b4000 {
+ compatible = "fsl,imx6sl-rngb",
"fsl,imx25-rngb";
reg = <0x021b4000 0x4000>;
interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = < IMX6SL_CLK_DUMMY>;
};
weim: weim@21b8000 {
--
2.17.1
[PATCH v4 3/5] ARM: dts: imx6sll: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sll.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index fb5d3bc50c6b..0b622201a1f3 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -786,6 +786,13 @@
clocks = < IMX6SLL_CLK_MMDC_P0_IPG>;
};
+ rngb: rng@21b4000 {
+ compatible = "fsl,imx6sll-rngb",
"fsl,imx25-rngb";
+ reg = <0x021b4000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6SLL_CLK_DUMMY>;
+ };
+
ocotp: efuse@21bc000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.17.1
[PATCH v4 1/5] dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs
RNGB block is found in some i.MX6 SoCs - 6SL, 6SLL, 6ULL, 6ULZ. Add corresponding compatible strings. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Signed-off-by: Horia Geantă Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/rng/imx-rng.txt b/Documentation/devicetree/bindings/rng/imx-rng.txt index 405c2b00ccb0..659d4efdd664 100644 --- a/Documentation/devicetree/bindings/rng/imx-rng.txt +++ b/Documentation/devicetree/bindings/rng/imx-rng.txt @@ -5,6 +5,9 @@ Required properties: "fsl,imx21-rnga" "fsl,imx31-rnga" (backward compatible with "fsl,imx21-rnga") "fsl,imx25-rngb" + "fsl,imx6sl-rngb" (backward compatible with "fsl,imx25-rngb") + "fsl,imx6sll-rngb" (backward compatible with "fsl,imx25-rngb") + "fsl,imx6ull-rngb" (backward compatible with "fsl,imx25-rngb") "fsl,imx35-rngc" - reg : offset and length of the register set of this block - interrupts : the interrupt number for the RNG block -- 2.17.1
[PATCH v4 4/5] ARM: dts: imx6ull: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
Reviewed-by: Marco Felsch
---
arch/arm/boot/dts/imx6ull.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
index fcde7f77ae42..9bf67490ac49 100644
--- a/arch/arm/boot/dts/imx6ull.dtsi
+++ b/arch/arm/boot/dts/imx6ull.dtsi
@@ -68,6 +68,13 @@
clock-names = "dcp";
};
+ rngb: rng@2284000 {
+ compatible = "fsl,imx6ull-rngb",
"fsl,imx25-rngb";
+ reg = <0x02284000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6UL_CLK_DUMMY>;
+ };
+
iomuxc_snvs: iomuxc-snvs@229 {
compatible = "fsl,imx6ull-iomuxc-snvs";
reg = <0x0229 0x4000>;
--
2.17.1
[PATCH v4 5/5] hwrng: imx-rngc: enable driver for i.MX6
i.MX6 SL, SLL, ULL, ULZ SoCs have an RNGB block. Since imx-rngc driver supports also rngb, let's enable it for these SoCs too. Signed-off-by: Horia Geantă Reviewed-by: Martin Kaiser Reviewed-by: Marco Felsch --- drivers/char/hw_random/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index 8478eb757f3c..98f95a09ce55 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -255,7 +255,7 @@ config HW_RANDOM_MXC_RNGA config HW_RANDOM_IMX_RNGC tristate "Freescale i.MX RNGC Random Number Generator" depends on HAS_IOMEM && HAVE_CLK - depends on SOC_IMX25 || COMPILE_TEST + depends on SOC_IMX25 || SOC_IMX6SL || SOC_IMX6SLL || SOC_IMX6UL || COMPILE_TEST default HW_RANDOM help This driver provides kernel-side support for the Random Number -- 2.17.1
[PATCH v4 0/5] hwrng: add support for i.MX6 rngb
Add support for RNGB found in some i.MX6 SoCs (6SL, 6SLL, 6ULL, 6ULZ), based on RNGC driver (drivers/char/hw_random/imx-rngc.c). This driver claims support also for RNGB (besides RNGC), and is currently used only by i.MX25. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Changelog: v4 -remove unneeded compatible strings from the driver v3 -mention in the DT binding the compatibility with "fsl,imx25-rngb" -collected Reviewed-by v2 -update rngb DT binding with compatible strings for i.MX6 SoCs Horia Geantă (5): dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs ARM: dts: imx6sl: fix rng node ARM: dts: imx6sll: add rng ARM: dts: imx6ull: add rng hwrng: imx-rngc: enable driver for i.MX6 Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ arch/arm/boot/dts/imx6sl.dtsi | 2 ++ arch/arm/boot/dts/imx6sll.dtsi| 7 +++ arch/arm/boot/dts/imx6ull.dtsi| 7 +++ drivers/char/hw_random/Kconfig| 2 +- 5 files changed, 20 insertions(+), 1 deletion(-) -- 2.17.1
Re: [PATCH v3 5/5] hwrng: imx-rngc: enable driver for i.MX6
On 7/14/2020 3:48 PM, Arnd Bergmann wrote:
> On Tue, Jul 14, 2020 at 2:39 PM Horia Geantă wrote:
>> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
>> index 8478eb757f3c..98f95a09ce55 100644
>> --- a/drivers/char/hw_random/Kconfig
>> +++ b/drivers/char/hw_random/Kconfig
>> @@ -255,7 +255,7 @@ config HW_RANDOM_MXC_RNGA
>> config HW_RANDOM_IMX_RNGC
>> tristate "Freescale i.MX RNGC Random Number Generator"
>> depends on HAS_IOMEM && HAVE_CLK
>> - depends on SOC_IMX25 || COMPILE_TEST
>> + depends on SOC_IMX25 || SOC_IMX6SL || SOC_IMX6SLL || SOC_IMX6UL ||
>> COMPILE_TEST
>> default HW_RANDOM
>
> Are these the only chips that have it? If other i.MX variations have
> the same block,
> or might have it in the future, maybe just generialize the dependency
> to SOC_IMX6
> or ARCH_IMX?
>
Fabio also suggested this during v1, see discussion here:
https://lore.kernel.org/linux-crypto/292aafd1-7249-5b76-ccc3-77b153594...@nxp.com
The SoC list is relatively stable, to the best of my knowledge.
>> diff --git a/drivers/char/hw_random/imx-rngc.c
>> b/drivers/char/hw_random/imx-rngc.c
>> index 9c47e431ce90..84576d2fbf8c 100644
>> --- a/drivers/char/hw_random/imx-rngc.c
>> +++ b/drivers/char/hw_random/imx-rngc.c
>> @@ -350,6 +350,9 @@ static SIMPLE_DEV_PM_OPS(imx_rngc_pm_ops,
>> imx_rngc_suspend, imx_rngc_resume);
>>
>> static const struct of_device_id imx_rngc_dt_ids[] = {
>> { .compatible = "fsl,imx25-rngb", .data = NULL, },
>> + { .compatible = "fsl,imx6sl-rngb", .data = NULL, },
>> + { .compatible = "fsl,imx6sll-rngb", .data = NULL, },
>> + { .compatible = "fsl,imx6ull-rngb", .data = NULL, },
>> { /* sentinel */ }
>
> In the .dts file you describe the devices as compatible with fsl,imx25-rngb,
> so this change is not really needed, unless you want to add non-NULL
> .data fields in a follow-up. It is usually a good idea to have the more
> specialized compatible strings in the DT, but the driver change won't
> do anything here.
>
Indeed, this isn't needed.
Will remove it in v4.
Thanks,
Horia
[PATCH v3 5/5] hwrng: imx-rngc: enable driver for i.MX6
i.MX6 SL, SLL, ULL, ULZ SoCs have an RNGB block.
Since imx-rngc driver supports also rngb,
let's enable it for these SoCs too.
Signed-off-by: Horia Geantă
Reviewed-by: Martin Kaiser
Reviewed-by: Marco Felsch
---
drivers/char/hw_random/Kconfig| 2 +-
drivers/char/hw_random/imx-rngc.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 8478eb757f3c..98f95a09ce55 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -255,7 +255,7 @@ config HW_RANDOM_MXC_RNGA
config HW_RANDOM_IMX_RNGC
tristate "Freescale i.MX RNGC Random Number Generator"
depends on HAS_IOMEM && HAVE_CLK
- depends on SOC_IMX25 || COMPILE_TEST
+ depends on SOC_IMX25 || SOC_IMX6SL || SOC_IMX6SLL || SOC_IMX6UL ||
COMPILE_TEST
default HW_RANDOM
help
This driver provides kernel-side support for the Random Number
diff --git a/drivers/char/hw_random/imx-rngc.c
b/drivers/char/hw_random/imx-rngc.c
index 9c47e431ce90..84576d2fbf8c 100644
--- a/drivers/char/hw_random/imx-rngc.c
+++ b/drivers/char/hw_random/imx-rngc.c
@@ -350,6 +350,9 @@ static SIMPLE_DEV_PM_OPS(imx_rngc_pm_ops, imx_rngc_suspend,
imx_rngc_resume);
static const struct of_device_id imx_rngc_dt_ids[] = {
{ .compatible = "fsl,imx25-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6sl-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6sll-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6ull-rngb", .data = NULL, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, imx_rngc_dt_ids);
--
2.17.1
[PATCH v3 2/5] ARM: dts: imx6sl: fix rng node
rng DT node was added without a compatible string.
i.MX driver for RNGC (drivers/char/hw_random/imx-rngc.c) also claims
support for RNGB, and is currently used for i.MX25.
Let's use this driver also for RNGB block in i.MX6SL.
Fixes: e29fe21cff96 ("ARM: dts: add device tree source for imx6sl SoC")
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sl.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 1c7180f28539..91a8c54d5e11 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -939,8 +939,10 @@
};
rngb: rngb@21b4000 {
+ compatible = "fsl,imx6sl-rngb",
"fsl,imx25-rngb";
reg = <0x021b4000 0x4000>;
interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = < IMX6SL_CLK_DUMMY>;
};
weim: weim@21b8000 {
--
2.17.1
[PATCH v3 1/5] dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs
RNGB block is found in some i.MX6 SoCs - 6SL, 6SLL, 6ULL, 6ULZ. Add corresponding compatible strings. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Signed-off-by: Horia Geantă --- Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/rng/imx-rng.txt b/Documentation/devicetree/bindings/rng/imx-rng.txt index 405c2b00ccb0..659d4efdd664 100644 --- a/Documentation/devicetree/bindings/rng/imx-rng.txt +++ b/Documentation/devicetree/bindings/rng/imx-rng.txt @@ -5,6 +5,9 @@ Required properties: "fsl,imx21-rnga" "fsl,imx31-rnga" (backward compatible with "fsl,imx21-rnga") "fsl,imx25-rngb" + "fsl,imx6sl-rngb" (backward compatible with "fsl,imx25-rngb") + "fsl,imx6sll-rngb" (backward compatible with "fsl,imx25-rngb") + "fsl,imx6ull-rngb" (backward compatible with "fsl,imx25-rngb") "fsl,imx35-rngc" - reg : offset and length of the register set of this block - interrupts : the interrupt number for the RNG block -- 2.17.1
[PATCH v3 4/5] ARM: dts: imx6ull: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
Reviewed-by: Marco Felsch
---
arch/arm/boot/dts/imx6ull.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
index fcde7f77ae42..9bf67490ac49 100644
--- a/arch/arm/boot/dts/imx6ull.dtsi
+++ b/arch/arm/boot/dts/imx6ull.dtsi
@@ -68,6 +68,13 @@
clock-names = "dcp";
};
+ rngb: rng@2284000 {
+ compatible = "fsl,imx6ull-rngb",
"fsl,imx25-rngb";
+ reg = <0x02284000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6UL_CLK_DUMMY>;
+ };
+
iomuxc_snvs: iomuxc-snvs@229 {
compatible = "fsl,imx6ull-iomuxc-snvs";
reg = <0x0229 0x4000>;
--
2.17.1
[PATCH v3 0/5] hwrng: add support for i.MX6 rngb
Add support for RNGB found in some i.MX6 SoCs (6SL, 6SLL, 6ULL, 6ULZ), based on RNGC driver (drivers/char/hw_random/imx-rngc.c). This driver claims support also for RNGB (besides RNGC), and is currently used only by i.MX25. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Changelog: v3 -mention in the DT binding the compatibility with "fsl,imx25-rngb" -collected Reviewed-by v2 -update rngb DT binding with compatible strings for i.MX6 SoCs Horia Geantă (5): dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs ARM: dts: imx6sl: fix rng node ARM: dts: imx6sll: add rng ARM: dts: imx6ull: add rng hwrng: imx-rngc: enable driver for i.MX6 Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ arch/arm/boot/dts/imx6sl.dtsi | 2 ++ arch/arm/boot/dts/imx6sll.dtsi| 7 +++ arch/arm/boot/dts/imx6ull.dtsi| 7 +++ drivers/char/hw_random/Kconfig| 2 +- drivers/char/hw_random/imx-rngc.c | 3 +++ 6 files changed, 23 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH v3 3/5] ARM: dts: imx6sll: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sll.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index fb5d3bc50c6b..0b622201a1f3 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -786,6 +786,13 @@
clocks = < IMX6SLL_CLK_MMDC_P0_IPG>;
};
+ rngb: rng@21b4000 {
+ compatible = "fsl,imx6sll-rngb",
"fsl,imx25-rngb";
+ reg = <0x021b4000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6SLL_CLK_DUMMY>;
+ };
+
ocotp: efuse@21bc000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.17.1
Re: [PATCH v2 1/5] dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs
On 7/14/2020 3:03 AM, Rob Herring wrote: > On Sun, Jun 21, 2020 at 05:56:54PM +0300, Horia Geantă wrote: >> RNGB block is found in some i.MX6 SoCs - 6SL, 6SLL, 6ULL, 6ULZ. >> Add corresponding compatible strings. >> >> Note: >> >> Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) >> also have a RNGB, however it's part of the CAAM >> (Cryptograhic Accelerator and Assurance Module) crypto accelerator. >> In this case, RNGB is managed in the caam driver >> (drivers/crypto/caam/), since it's tightly related to >> the caam "job ring" interface, not to mention CAAM internally relying on >> RNGB as source of randomness. >> >> On the other hand, the i.MX6 SoCs with RNGB have a DCP >> (Data Co-Processor) crypto accelerator and this block and RNGB >> are independent. >> >> Signed-off-by: Horia Geantă >> --- >> Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/rng/imx-rng.txt >> b/Documentation/devicetree/bindings/rng/imx-rng.txt >> index 405c2b00ccb0..eb227db9e684 100644 >> --- a/Documentation/devicetree/bindings/rng/imx-rng.txt >> +++ b/Documentation/devicetree/bindings/rng/imx-rng.txt >> @@ -5,6 +5,9 @@ Required properties: >> "fsl,imx21-rnga" >> "fsl,imx31-rnga" (backward compatible with "fsl,imx21-rnga") >> "fsl,imx25-rngb" >> + "fsl,imx6sl-rngb" >> + "fsl,imx6sll-rngb" >> + "fsl,imx6ull-rngb" > > These are all different? IOW, no fallback compatible? > They are compatible with "fsl,imx25-rngb". I will clarify this in the binding in v3. Thanks, Horia
Re: [PATCH V3 0/3] ARM: imx: move cpu code to drivers/soc/imx
On 7/3/2020 3:25 PM, Peng Fan wrote: > Sorry to break LS1021A. But I wonder why i.MX code would affect LS? > imx_soc_device_init() was modified to be called for all SoCs under ARCH_MXC. multi_v7_defconfig, which is used for LS1021A, selects ARCH_MXC. Previously imx_soc_device_init() was called only for i.MX (and Vybrid) SoCs. Horia
Re: [PATCH V3 0/3] ARM: imx: move cpu code to drivers/soc/imx
On 5/20/2020 9:01 AM, Peng Fan wrote: > From: Peng Fan > > V3: > Rebased to latest next tree > Resolved the conflicts with vf610 soc patch > > V2: > Keep i.MX1/2/3/5 cpu type for completness > Correct return value in patch 1/3 > use CONFIG_ARM to guard compile soc-imx.c in patch 3/3 > > V1: > https://patchwork.kernel.org/cover/11433689/ > RFC version : > https://patchwork.kernel.org/cover/11336433/ > > Nothing changed in v1, just rename to formal patches > > Shawn, > The original concern has been eliminated in RFC discussion, > so this patchset is ready to be in next. > Thanks. > > Follow i.MX8, move the soc device register code to drivers/soc/imx > to simplify arch/arm/mach-imx/cpu.c > > I planned to use similar logic as soc-imx8m.c to restructure soc-imx.c > and merged the two files into one. But not sure, so still keep > the logic in cpu.c. > > There is one change is the platform devices are not under > /sys/devices/soc0 after patch 1/4. Actually ARM64 platform > devices are not under /sys/devices/soc0, such as i.MX8/8M. > So it should not hurt to let the platform devices under platform dir. > > Peng Fan (3): > ARM: imx: use device_initcall for imx_soc_device_init > ARM: imx: move cpu definitions into a header > soc: imx: move cpu code to drivers/soc/imx > This patch series has the side effect of LS1021A platform now reporting that it's part of "i.MX family". caam driver relies on the SoC bus / SoC attributes (ID, family) to determine if it's running on an i.MX SoC or other (Layerscape, QorIQ). With this patch set, driver fails to probe on LS1021A: [5.998928] caam 170.crypto: No clock data provided for i.MX SoC [6.005306] caam: probe of 170.crypto failed with error -22 Horia
[PATCH v2 2/5] ARM: dts: imx6sl: fix rng node
rng DT node was added without a compatible string.
i.MX driver for RNGC (drivers/char/hw_random/imx-rngc.c) also claims
support for RNGB, and is currently used for i.MX25.
Let's use this driver also for RNGB block in i.MX6SL.
Fixes: e29fe21cff96 ("ARM: dts: add device tree source for imx6sl SoC")
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sl.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 911d8cf77f2c..0339a46fa71c 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -939,8 +939,10 @@
};
rngb: rngb@21b4000 {
+ compatible = "fsl,imx6sl-rngb",
"fsl,imx25-rngb";
reg = <0x021b4000 0x4000>;
interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = < IMX6SL_CLK_DUMMY>;
};
weim: weim@21b8000 {
--
2.17.1
[PATCH v2 0/5] hwrng: add support for i.MX6 rngb
Add support for RNGB found in some i.MX6 SoCs (6SL, 6SLL, 6ULL, 6ULZ), based on RNGC driver (drivers/char/hw_random/imx-rngc.c). This driver claims support also for RNGB (besides RNGC), and is currently used only by i.MX25. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Changelog: -update rngb DT binding with compatible strings for i.MX6 SoCs Horia Geantă (5): dt-bindings: rng: add Freescale RNGB compatibles for i.MX6 SoCs ARM: dts: imx6sl: fix rng node ARM: dts: imx6sll: add rng ARM: dts: imx6ull: add rng hwrng: imx-rngc: enable driver for i.MX6 Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ arch/arm/boot/dts/imx6sl.dtsi | 2 ++ arch/arm/boot/dts/imx6sll.dtsi| 7 +++ arch/arm/boot/dts/imx6ull.dtsi| 7 +++ drivers/char/hw_random/Kconfig| 2 +- drivers/char/hw_random/imx-rngc.c | 3 +++ 6 files changed, 23 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH v2 4/5] ARM: dts: imx6ull: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6ull.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
index fcde7f77ae42..9bf67490ac49 100644
--- a/arch/arm/boot/dts/imx6ull.dtsi
+++ b/arch/arm/boot/dts/imx6ull.dtsi
@@ -68,6 +68,13 @@
clock-names = "dcp";
};
+ rngb: rng@2284000 {
+ compatible = "fsl,imx6ull-rngb",
"fsl,imx25-rngb";
+ reg = <0x02284000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6UL_CLK_DUMMY>;
+ };
+
iomuxc_snvs: iomuxc-snvs@229 {
compatible = "fsl,imx6ull-iomuxc-snvs";
reg = <0x0229 0x4000>;
--
2.17.1
[PATCH v2 3/5] ARM: dts: imx6sll: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sll.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index edd3abb9a9f1..1c5dbccca013 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -786,6 +786,13 @@
clocks = < IMX6SLL_CLK_MMDC_P0_IPG>;
};
+ rngb: rng@21b4000 {
+ compatible = "fsl,imx6sll-rngb",
"fsl,imx25-rngb";
+ reg = <0x021b4000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6SLL_CLK_DUMMY>;
+ };
+
ocotp: ocotp-ctrl@21bc000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.17.1
[PATCH v2 1/5] dt-bindings: rng: add RNGB compatibles for i.MX6 SoCs
RNGB block is found in some i.MX6 SoCs - 6SL, 6SLL, 6ULL, 6ULZ. Add corresponding compatible strings. Note: Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface, not to mention CAAM internally relying on RNGB as source of randomness. On the other hand, the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator and this block and RNGB are independent. Signed-off-by: Horia Geantă --- Documentation/devicetree/bindings/rng/imx-rng.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/rng/imx-rng.txt b/Documentation/devicetree/bindings/rng/imx-rng.txt index 405c2b00ccb0..eb227db9e684 100644 --- a/Documentation/devicetree/bindings/rng/imx-rng.txt +++ b/Documentation/devicetree/bindings/rng/imx-rng.txt @@ -5,6 +5,9 @@ Required properties: "fsl,imx21-rnga" "fsl,imx31-rnga" (backward compatible with "fsl,imx21-rnga") "fsl,imx25-rngb" + "fsl,imx6sl-rngb" + "fsl,imx6sll-rngb" + "fsl,imx6ull-rngb" "fsl,imx35-rngc" - reg : offset and length of the register set of this block - interrupts : the interrupt number for the RNG block -- 2.17.1
[PATCH v2 5/5] hwrng: imx-rngc: enable driver for i.MX6
i.MX6 SL, SLL, ULL, ULZ SoCs have an RNGB block.
Since imx-rngc driver supports also rngb,
let's enable it for these SoCs too.
Signed-off-by: Horia Geantă
---
drivers/char/hw_random/Kconfig| 2 +-
drivers/char/hw_random/imx-rngc.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 0ad17efc96df..53f6a7e4392f 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -245,7 +245,7 @@ config HW_RANDOM_MXC_RNGA
config HW_RANDOM_IMX_RNGC
tristate "Freescale i.MX RNGC Random Number Generator"
depends on HAS_IOMEM && HAVE_CLK
- depends on SOC_IMX25 || COMPILE_TEST
+ depends on SOC_IMX25 || SOC_IMX6SL || SOC_IMX6SLL || SOC_IMX6UL ||
COMPILE_TEST
default HW_RANDOM
help
This driver provides kernel-side support for the Random Number
diff --git a/drivers/char/hw_random/imx-rngc.c
b/drivers/char/hw_random/imx-rngc.c
index 9c47e431ce90..84576d2fbf8c 100644
--- a/drivers/char/hw_random/imx-rngc.c
+++ b/drivers/char/hw_random/imx-rngc.c
@@ -350,6 +350,9 @@ static SIMPLE_DEV_PM_OPS(imx_rngc_pm_ops, imx_rngc_suspend,
imx_rngc_resume);
static const struct of_device_id imx_rngc_dt_ids[] = {
{ .compatible = "fsl,imx25-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6sl-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6sll-rngb", .data = NULL, },
+ { .compatible = "fsl,imx6ull-rngb", .data = NULL, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, imx_rngc_dt_ids);
--
2.17.1
Re: [PATCH 4/4] hwrng: imx-rngc: enable driver for i.MX6
On 6/20/2020 12:49 AM, Fabio Estevam wrote:
> On Fri, Jun 19, 2020 at 6:46 PM Fabio Estevam wrote:
>
>> If in the future more SoCs will use this IP, then we will need to keep
>> extending this list over and over again.
>>
>> Maybe you could use:
>>
>> depends on MACH_IMX || COMPILE_TEST
>
> MACH_MXC is what I meant ;-)
>
Probably ARCH_MXC.
ARCH_MXC was originally the dependency for this driver, until it was changed
to a more specific one by
commit fcdba3c33a4d ("hwrng: imx-rngc - improve dependencies")
I don't think we'll see too many RNGB deployments in the future,
thus the list of SoCs shouldn't change that often.
Horia
[PATCH 2/4] ARM: dts: imx6sll: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sll.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index edd3abb9a9f1..e634cd45086b 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -786,6 +786,13 @@
clocks = < IMX6SLL_CLK_MMDC_P0_IPG>;
};
+ rngb: rng@21b4000 {
+ compatible = "fsl,imx25-rngb";
+ reg = <0x021b4000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6SLL_CLK_DUMMY>;
+ };
+
ocotp: ocotp-ctrl@21bc000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.17.1
[PATCH 4/4] hwrng: imx-rngc: enable driver for i.MX6
i.MX6 SL, SLL, ULL, ULZ SoCs have an RNGB block. Since imx-rngc driver supports also rngb, let's enable it for these SoCs too. Signed-off-by: Horia Geantă --- drivers/char/hw_random/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index 0ad17efc96df..53f6a7e4392f 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -245,7 +245,7 @@ config HW_RANDOM_MXC_RNGA config HW_RANDOM_IMX_RNGC tristate "Freescale i.MX RNGC Random Number Generator" depends on HAS_IOMEM && HAVE_CLK - depends on SOC_IMX25 || COMPILE_TEST + depends on SOC_IMX25 || SOC_IMX6SL || SOC_IMX6SLL || SOC_IMX6UL || COMPILE_TEST default HW_RANDOM help This driver provides kernel-side support for the Random Number -- 2.17.1
[PATCH 3/4] ARM: dts: imx6ull: add rng
Add node for the RNGB block.
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6ull.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
index fcde7f77ae42..70cc098adeee 100644
--- a/arch/arm/boot/dts/imx6ull.dtsi
+++ b/arch/arm/boot/dts/imx6ull.dtsi
@@ -68,6 +68,13 @@
clock-names = "dcp";
};
+ rngb: rng@2284000 {
+ compatible = "fsl,imx25-rngb";
+ reg = <0x02284000 0x4000>;
+ interrupts = ;
+ clocks = < IMX6UL_CLK_DUMMY>;
+ };
+
iomuxc_snvs: iomuxc-snvs@229 {
compatible = "fsl,imx6ull-iomuxc-snvs";
reg = <0x0229 0x4000>;
--
2.17.1
[PATCH 0/4] hwrng: add support for i.MX6 rngb
Add support for RNGB found in some i.MX6 SoCs (6SL, 6SLL, 6ULL, 6ULZ), based on RNGC driver (drivers/char/hw_random/imx-rngc.c). This driver claims support also for RNGB (besides RNGC), and is currently used only by i.MX25. Note: All the i.MX6 SoCs with RNGB have a DCP (Data Co-Processor) crypto accelerator. Several NXP SoC from QorIQ family (P1010, P1023, P4080, P3041, P5020) also have a RNGB, however it's part of the CAAM (Cryptograhic Accelerator and Assurance Module) crypto accelerator. In this case, RNGB is managed in the caam driver (drivers/crypto/caam/), since it's tightly related to the caam "job ring" interface. Horia Geantă (4): ARM: dts: imx6sl: fix rng node ARM: dts: imx6sll: add rng ARM: dts: imx6ull: add rng hwrng: imx-rngc: enable driver for i.MX6 arch/arm/boot/dts/imx6sl.dtsi | 2 ++ arch/arm/boot/dts/imx6sll.dtsi | 7 +++ arch/arm/boot/dts/imx6ull.dtsi | 7 +++ drivers/char/hw_random/Kconfig | 2 +- 4 files changed, 17 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH 1/4] ARM: dts: imx6sl: fix rng node
rng DT node was added without a compatible string.
i.MX driver for RNGC (drivers/char/hw_random/imx-rngc.c) also claims
support for RNGB, and is currently used for i.MX25.
Let's used this driver also for RNGB block in i.MX6SL.
Fixes: e29fe21cff96 ("ARM: dts: add device tree source for imx6sl SoC")
Signed-off-by: Horia Geantă
---
arch/arm/boot/dts/imx6sl.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 911d8cf77f2c..1f0f250ee175 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -939,8 +939,10 @@
};
rngb: rngb@21b4000 {
+ compatible = "fsl,imx25-rngb";
reg = <0x021b4000 0x4000>;
interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = < IMX6SL_CLK_DUMMY>;
};
weim: weim@21b8000 {
--
2.17.1
Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret
On 6/18/2020 2:00 PM, Herbert Xu wrote: > On Thu, Jun 18, 2020 at 01:54:55PM +0300, Horia Geantă wrote: >> >> The proper fix would be updating the ahash_finup_no_ctx() function >> to return the specific error code: >> return ret; >> instead of returning -ENOMEM for all error cases. >> >> For example error code returned by dpaa2_caam_enqueue() >> should be returned instead of -ENOMEM. > > You can do that as a follow-up. The patch is correct as is. > Just that the follow-up implies adding all the code back. Anyway, not a big deal... Thanks, Horia
Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret
On 6/11/2020 6:39 PM, Colin King wrote:
> From: Colin Ian King
>
> The variable ret is being assigned a value that is never read, the
> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning
> ret with -ENOMEM is redundamt.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/crypto/caam/caamalg_qi2.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamalg_qi2.c
> b/drivers/crypto/caam/caamalg_qi2.c
> index 28669cbecf77..ef2c4e095db3 100644
> --- a/drivers/crypto/caam/caamalg_qi2.c
> +++ b/drivers/crypto/caam/caamalg_qi2.c
> @@ -4044,7 +4044,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
> DMA_TO_DEVICE);
> if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) {
> dev_err(ctx->dev, "unable to map S/G table\n");
> - ret = -ENOMEM;
> goto unmap;
> }
> edesc->qm_sg_bytes = qm_sg_bytes;
> @@ -4055,7 +4054,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
> if (dma_mapping_error(ctx->dev, state->ctx_dma)) {
> dev_err(ctx->dev, "unable to map ctx\n");
> state->ctx_dma = 0;
> - ret = -ENOMEM;
> goto unmap;
> }
>
The proper fix would be updating the ahash_finup_no_ctx() function
to return the specific error code:
return ret;
instead of returning -ENOMEM for all error cases.
For example error code returned by dpaa2_caam_enqueue()
should be returned instead of -ENOMEM.
Horia
Re: [PATCH] crypto: caam/qi2: remove redundant assignment to ret
On 6/18/2020 10:58 AM, Herbert Xu wrote:
> On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote:
>> From: Colin Ian King
>>
>> The variable ret is being assigned a value that is never read, the
>> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning
>> ret with -ENOMEM is redundamt.
>>
>> Addresses-Coverity: ("Unused value")
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/crypto/caam/caamalg_qi2.c | 2 --
>> 1 file changed, 2 deletions(-)
>
> Patch applied. Thanks.
>
Unfortunately I missed this patch, and it doesn't look correct.
Do I need to send a revert?
Thanks,
Horia
Re: [PATCH v2 1/1] crypto: caam - fix typos
On 6/4/2020 1:40 PM, Heinrich Schuchardt wrote: > Fix CAAM related typos. > > Signed-off-by: Heinrich Schuchardt Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH 1/1] crypto: caam - fix typo
On 6/4/2020 5:41 AM, Heinrich Schuchardt wrote: > %s/suppying/supplying/ > This is good since it's not detected by the default kernel spellchecker. Would you be willing to append also the detected typos? CHECK: 'interrrupt' may be misspelled - perhaps 'interrupt'? #57: FILE: drivers/crypto/caam/ctrl.c:57: +* resets the done interrrupt and returns the RNG to idle. CHECK: 'occured' may be misspelled - perhaps 'occurred'? #159: FILE: drivers/crypto/caam/ctrl.c:159: +* If an error occured in the descriptor, then CHECK: 'aquired' may be misspelled - perhaps 'acquired'? #267: FILE: drivers/crypto/caam/ctrl.c:267: + * entropy being aquired. CHECK: 'paramters' may be misspelled - perhaps 'parameters'? #736: FILE: drivers/crypto/caam/ctrl.c:736: +* Read the Compile Time paramters and SCFGR to determine CHECK: 'modfiy' may be misspelled - perhaps 'modify'? #866: FILE: drivers/crypto/caam/ctrl.c:866: +* and the kick_trng(...) function will modfiy the CHECK: 'sucessful' may be misspelled - perhaps 'successful'? #868: FILE: drivers/crypto/caam/ctrl.c:868: +* interval, leading to a sucessful initialization of CHECK: 'Propogate' may be misspelled - perhaps 'Propagate'? #116: FILE: drivers/crypto/caam/desc.h:116: +/* Propogate DNR property to SharedDesc */ CHECK: 'defintions' may be misspelled - perhaps 'definitions'? #456: FILE: drivers/crypto/caam/pdb.h:456: + u32 sgf_ln; /* Use DSA_PDB_ defintions per above */ Thanks, Horia
Re: [PATCH v2] crypto: caam/qi2 - add support for dpseci_reset()
On 6/3/2020 11:47 AM, Andrei Botila (OSS) wrote: > From: Andrei Botila > > Add support for dpseci_reset() command for DPSECI objects. > For DPSECI DPAA2 objects with version lower than v5.4 reset command > was broken in MC f/w. > > Signed-off-by: Andrei Botila Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH] crypto: caam - add clock info for VFxxx SoCs
On 6/2/2020 2:07 AM, Andrey Smirnov wrote: > Add a small bit of plumbing necessary to use CAAM on VFxxx SoCs. > > Signed-off-by: Andrey Smirnov > Cc: Chris Healy > Cc: Horia Geantă > Cc: Herbert Xu > Cc: Fabio Estevam > Cc: linux-...@nxp.com > Cc: linux-cry...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org Reviewed-by: Horia Geantă Thanks, Horia
Re: [PATCH] crypto: caam/qi2 - add support for dpseci_reset()
On 5/28/2020 2:05 PM, Andrei Botila (OSS) wrote: > @@ -4698,6 +4698,9 @@ static void dpaa2_dpseci_free(struct dpaa2_caam_priv > *priv) > struct device *dev = priv->dev; > struct fsl_mc_device *ls_dev = to_fsl_mc_device(dev); > > + if (DPSECI_VER(priv->major_ver, priv->minor_ver) > DPSECI_VER(5, 3)) > + dpseci_reset(priv->mc_io, 0, ls_dev->mc_handle); Even though dpseci_close() should be called in all cases, a warning when reset fails would be useful. > + > dpaa2_dpseci_congestion_free(priv); > dpseci_close(priv->mc_io, 0, ls_dev->mc_handle); > }
Re: [PATCH] crypto: engine - do not requeue in case of fatal error
On 5/20/2020 1:17 AM, Iuliana Prodan wrote:
> Now, in crypto-engine, if hardware queue is full (-ENOSPC),
> requeue request regardless of MAY_BACKLOG flag.
> If hardware throws any other error code (like -EIO, -EINVAL,
> -ENOMEM, etc.) only MAY_BACKLOG requests are enqueued back into
> crypto-engine's queue, since the others can be dropped.
> The latter case can be fatal error, so those cannot be recovered from.
> For example, in CAAM driver, -EIO is returned in case the job descriptor
> is broken, so there is no possibility to fix the job descriptor.
> Therefore, these errors might be fatal error, so we shouldn’t
> requeue the request. This will just be pass back and forth between
> crypto-engine and hardware.
>
> Fixes: 6a89f492f8e5 ("crypto: engine - support for parallel requests based on
> retry mechanism")
> Signed-off-by: Iuliana Prodan
Reported-by: Horia Geantă
Reviewed-by: Horia Geantă
Thanks,
Horia
Re: [PATCH] crypto: caam - make soc match data optional
On 5/16/2020 7:23 AM, Andrey Smirnov wrote:
> Vyrbrid devices don't have any clock that need to be taken care of, so
> make clock data optional on i.MX.
>
Vybrid Security RM states that IPG clock used by CAAM
can be gated by CCM_CCGR11[CG176].
Clock driver needs to be updated accordingly,
and so will CAAM driver and DT node.
I don't have a board at hand, so patch below is not tested.
Horia
-- >8 --
Subject: [PATCH] clk: imx: vf610: add CAAM clock
According to Vybrid Security RM, CCM_CCGR11[CG176] can be used to
gate CAAM ipg clock.
Signed-off-by: Horia Geantă
---
drivers/clk/imx/clk-vf610.c | 2 ++
include/dt-bindings/clock/vf610-clock.h | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index cd04e7dc1878..4f3066cf1b89 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -439,6 +439,8 @@ static void __init vf610_clocks_init(struct device_node
*ccm_node)
clk[VF610_CLK_DAP] = imx_clk_gate("dap", "platform_bus", CCM_CCSR, 24);
clk[VF610_CLK_OCOTP] = imx_clk_gate("ocotp", "ipg_bus", CCM_CCGR6,
CCM_CCGRx_CGn(5));
+ clk[VF610_CLK_CAAM] = imx_clk_gate2("caam", "ipg_bus", CCM_CCGR11,
CCM_CCGRx_CGn(0));
+
imx_check_clocks(clk, ARRAY_SIZE(clk));
clk_set_parent(clk[VF610_CLK_QSPI0_SEL], clk[VF610_CLK_PLL1_PFD4]);
diff --git a/include/dt-bindings/clock/vf610-clock.h
b/include/dt-bindings/clock/vf610-clock.h
index 95394f35a74a..0f2d60e884dc 100644
--- a/include/dt-bindings/clock/vf610-clock.h
+++ b/include/dt-bindings/clock/vf610-clock.h
@@ -195,6 +195,7 @@
#define VF610_CLK_WKPU 186
#define VF610_CLK_TCON0187
#define VF610_CLK_TCON1188
-#define VF610_CLK_END 189
+#define VF610_CLK_CAAM 189
+#define VF610_CLK_END 190
#endif /* __DT_BINDINGS_CLOCK_VF610_H */
--
2.17.1
[PATCH] arm64: dts: ls1028a: add crypto node
LS1028A has a SEC v5.0 compatible security engine.
Signed-off-by: Horia Geantă
---
Tested with "arm-smmu.disable_bypass=0" kernel boot parameter,
since ICID (Isolation Context ID, out of which ARM SMMU stream ID
is derived) programming and DT fix-up support hasn't been added yet
in U-boot.
.../boot/dts/freescale/fsl-ls1028a-qds.dts| 1 +
.../boot/dts/freescale/fsl-ls1028a-rdb.dts| 1 +
.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 39 +++
3 files changed, 41 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index 4ed18287e077..c9765bb4bd7f 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -17,6 +17,7 @@
compatible = "fsl,ls1028a-qds", "fsl,ls1028a";
aliases {
+ crypto =
gpio0 =
gpio1 =
gpio2 =
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 4a203f7da598..745ec462bfae 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -16,6 +16,7 @@
compatible = "fsl,ls1028a-rdb", "fsl,ls1028a";
aliases {
+ crypto =
serial0 =
serial1 =
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index bb386dd1d1b1..0048c2610a09 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -388,6 +388,45 @@
,
;
};
+ crypto: crypto@800 {
+ compatible = "fsl,sec-v5.0", "fsl,sec-v4.0";
+ fsl,sec-era = <10>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x00 0x800 0x10>;
+ reg = <0x00 0x800 0x0 0x10>;
+ interrupts = ;
+ dma-coherent;
+
+ sec_jr0: jr@1 {
+ compatible = "fsl,sec-v5.0-job-ring",
+"fsl,sec-v4.0-job-ring";
+ reg = <0x1 0x1>;
+ interrupts = ;
+ };
+
+ sec_jr1: jr@2 {
+ compatible = "fsl,sec-v5.0-job-ring",
+"fsl,sec-v4.0-job-ring";
+ reg = <0x2 0x1>;
+ interrupts = ;
+ };
+
+ sec_jr2: jr@3 {
+ compatible = "fsl,sec-v5.0-job-ring",
+"fsl,sec-v4.0-job-ring";
+ reg = <0x3 0x1>;
+ interrupts = ;
+ };
+
+ sec_jr3: jr@4 {
+ compatible = "fsl,sec-v5.0-job-ring",
+"fsl,sec-v4.0-job-ring";
+ reg = <0x4 0x1>;
+ interrupts = ;
+ };
+ };
+
cluster1_core0_watchdog: watchdog@c00 {
compatible = "arm,sp805", "arm,primecell";
reg = <0x0 0xc00 0x0 0x1000>;
--
2.17.1
[PATCH RESEND v3] crypto: caam - add missing put_device() call
From: Wen Yang
The of_find_device_by_node() takes a reference to the underlying device
structure, we should release that reference.
Fixes: 35af64038623 ("crypto: caam - Check for CAAM block presence before
registering with crypto layer")
Fixes: b189817cf789 ("crypto: caam/qi - add ablkcipher and authenc algorithms")
Reviewed-by: Horia Geantă
Signed-off-by: Wen Yang
---
Looks like previous versions, for some unknown reason, did not reach
the mailing lists.
Resending v3 with the addition of a 2nd Fixes and Reviewed-by tags,
cf. https://lkml.org/lkml/2019/2/11/383
drivers/crypto/caam/caamalg.c| 12 +++-
drivers/crypto/caam/caamalg_qi.c | 11 ---
drivers/crypto/caam/caamhash.c | 18 +++---
drivers/crypto/caam/caampkc.c| 14 ++
drivers/crypto/caam/caamrng.c| 22 ++
5 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 9eac5099098e..579578498deb 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -3455,7 +3455,6 @@ static int __init caam_algapi_init(void)
{
struct device_node *dev_node;
struct platform_device *pdev;
- struct device *ctrldev;
struct caam_drv_private *priv;
int i = 0, err = 0;
u32 aes_vid, aes_inst, des_inst, md_vid, md_inst, ccha_inst, ptha_inst;
@@ -3476,16 +3475,17 @@ static int __init caam_algapi_init(void)
return -ENODEV;
}
- ctrldev = >dev;
- priv = dev_get_drvdata(ctrldev);
+ priv = dev_get_drvdata(>dev);
of_node_put(dev_node);
/*
* If priv is NULL, it's probably because the caam driver wasn't
* properly initialized (e.g. RNG4 init failed). Thus, bail out here.
*/
- if (!priv)
- return -ENODEV;
+ if (!priv) {
+ err = -ENODEV;
+ goto out_put_dev;
+ }
/*
@@ -3626,6 +3626,8 @@ static int __init caam_algapi_init(void)
if (registered)
pr_info("caam algorithms registered in /proc/crypto\n");
+out_put_dev:
+ put_device(>dev);
return err;
}
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index a15ce9213310..c61921d32489 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -2492,12 +2492,15 @@ static int __init caam_qi_algapi_init(void)
* If priv is NULL, it's probably because the caam driver wasn't
* properly initialized (e.g. RNG4 init failed). Thus, bail out here.
*/
- if (!priv || !priv->qi_present)
- return -ENODEV;
+ if (!priv || !priv->qi_present) {
+ err = -ENODEV;
+ goto out_put_dev;
+ }
if (caam_dpaa2) {
dev_info(ctrldev, "caam/qi frontend driver not suitable for
DPAA 2.x, aborting...\n");
- return -ENODEV;
+ err = -ENODEV;
+ goto out_put_dev;
}
/*
@@ -2610,6 +2613,8 @@ static int __init caam_qi_algapi_init(void)
if (registered)
dev_info(priv->qidev, "algorithms registered in
/proc/crypto\n");
+out_put_dev:
+ put_device(ctrldev);
return err;
}
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index d7483e4d0ce2..b1eadc6652b5 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1993,7 +1993,6 @@ static int __init caam_algapi_hash_init(void)
{
struct device_node *dev_node;
struct platform_device *pdev;
- struct device *ctrldev;
int i = 0, err = 0;
struct caam_drv_private *priv;
unsigned int md_limit = SHA512_DIGEST_SIZE;
@@ -2012,16 +2011,17 @@ static int __init caam_algapi_hash_init(void)
return -ENODEV;
}
- ctrldev = >dev;
- priv = dev_get_drvdata(ctrldev);
+ priv = dev_get_drvdata(>dev);
of_node_put(dev_node);
/*
* If priv is NULL, it's probably because the caam driver wasn't
* properly initialized (e.g. RNG4 init failed). Thus, bail out here.
*/
- if (!priv)
- return -ENODEV;
+ if (!priv) {
+ err = -ENODEV;
+ goto out_put_dev;
+ }
/*
* Register crypto algorithms the device supports. First, identify
@@ -2043,8 +2043,10 @@ static int __init caam_algapi_hash_init(void)
* Skip registration of any hashing algorithms if MD block
* is not present.
*/
- if (!md_inst)
- return -ENODEV;
+ if (!md_inst) {
+ err = -ENODEV;
+ goto out_put_dev;
+ }
/* Limit digest size based on LP256 */
if (md_vid == CHA_VER_VID_MD
[PATCH] soc: fsl: guts: make fsl_guts_get_svr() static
The export of fsl_guts_get_svr() is a left-over, it's currently used
only internally and users needing SoC information should use the generic
soc_device infrastructure.
Signed-off-by: Horia Geantă
---
drivers/soc/fsl/guts.c | 3 +--
include/linux/fsl/guts.h | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 4f9655087bd7..63f6df86f9e5 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -115,7 +115,7 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
return NULL;
}
-u32 fsl_guts_get_svr(void)
+static u32 fsl_guts_get_svr(void)
{
u32 svr = 0;
@@ -129,7 +129,6 @@ u32 fsl_guts_get_svr(void)
return svr;
}
-EXPORT_SYMBOL(fsl_guts_get_svr);
static int fsl_guts_probe(struct platform_device *pdev)
{
diff --git a/include/linux/fsl/guts.h b/include/linux/fsl/guts.h
index 941b11811f85..1fc0edd71c52 100644
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -135,8 +135,6 @@ struct ccsr_guts {
u32 srds2cr1; /* 0x.0f44 - SerDes2 Control Register 0 */
} __attribute__ ((packed));
-u32 fsl_guts_get_svr(void);
-
/* Alternate function signal multiplex control */
#define MPC85xx_PMUXCR_QE(x) (0x8000 >> (x))
--
2.16.2
[PATCH RESEND] crypto: caam - move shared symbols in a common location
There are several issues with symbols shared b/w:
-caam/jr and caam/qi drivers on one hand
-caam/qi2 driver on the other hand
Commit 52813ab24959 ("crypto: caam/qi2 - avoid double export") fixed
some of them, however compilation still fails for CRYPTO_DEV_FSL_CAAM=m
and CRYPTO_DEV_FSL_DPAA2_CAAM=y.
Another issue is related to dependency cycles reported by depmod when
CRYPTO_DEV_FSL_CAAM=n and CRYPTO_DEV_FSL_DPAA2_CAAM=m, as mentioned in
82c7b351be3f ("Revert "arm64: defconfig: Enable FSL_MC_BUS and FSL_MC_DPIO"")
To fix all these, move the symbols shared by these drivers in a common
location. The only existing possibility is error.c file (note that naming
doesn't help and should probably change).
Fixes: 52813ab24959 ("crypto: caam/qi2 - avoid double export")
Reported-by: Arnd Bergmann
Signed-off-by: Horia Geantă
---
Resending to fix patchwork tracking.
Initial submission done inline here:
https://patchwork.kernel.org/patch/10739071/
drivers/crypto/caam/caamalg_qi2.c | 7 ---
drivers/crypto/caam/ctrl.c| 4
drivers/crypto/caam/error.c | 6 ++
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/caam/caamalg_qi2.c
b/drivers/crypto/caam/caamalg_qi2.c
index 425d5d974613..cc59814afd29 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -25,13 +25,6 @@
#define CAAM_MAX_KEY_SIZE (AES_MAX_KEY_SIZE + CTR_RFC3686_NONCE_SIZE + \
SHA512_DIGEST_SIZE * 2)
-#if !IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM)
-bool caam_little_end;
-EXPORT_SYMBOL(caam_little_end);
-bool caam_imx;
-EXPORT_SYMBOL(caam_imx);
-#endif
-
/*
* This is a a cache of buffers, from which the users of CAAM QI driver
* can allocate short buffers. It's speedier than doing kmalloc on the hotpath.
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 16bbc72f041a..14fb09223156 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -18,12 +18,8 @@
#include "desc_constr.h"
#include "ctrl.h"
-bool caam_little_end;
-EXPORT_SYMBOL(caam_little_end);
bool caam_dpaa2;
EXPORT_SYMBOL(caam_dpaa2);
-bool caam_imx;
-EXPORT_SYMBOL(caam_imx);
#ifdef CONFIG_CAAM_QI
#include "qi.h"
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 7e8d690f2827..21a70fd32f5d 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -50,6 +50,12 @@ void caam_dump_sg(const char *level, const char *prefix_str,
int prefix_type,
#endif /* DEBUG */
EXPORT_SYMBOL(caam_dump_sg);
+bool caam_little_end;
+EXPORT_SYMBOL(caam_little_end);
+
+bool caam_imx;
+EXPORT_SYMBOL(caam_imx);
+
static const struct {
u8 value;
const char *error_text;
--
2.16.2
