2022-04-28 19:43:07

by Ahmad Fatoum

[permalink] [raw]
Subject: [PATCH v8 0/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

Series applies on top of v5.18-rc4. Would be great if this could make it
into v5.19.

v7 was here:
https://lore.kernel.org/linux-integrity/[email protected]

Changelog is beneath each individual patch. No code changes (except for
comments) compared to v7.


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:

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 Agarwal 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. This time
it uses CAAM "black blobs" instead of "red blobs", so key material stays
within the CAAM and isn't exposed to kernel in plaintext.
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.

- 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.

The CAAM bits are based on Steffen's initial patch from 2015. His work had been
used in the field for some years now, so I preferred not to deviate too much from it.

This series has been tested with dmcrypt[5] on an i.MX6Q/DL and an i.MX8M[6].

Looking forward to your feedback.

Cheers,
Ahmad

[1]: https://lore.kernel.org/linux-crypto/[email protected]/
[2]: https://lore.kernel.org/linux-integrity/[email protected]/
[3]: https://lore.kernel.org/lkml/[email protected]/
[4]: https://lore.kernel.org/lkml/[email protected]/
[5]: https://lore.kernel.org/linux-integrity/[email protected]/
[6]: https://lore.kernel.org/linux-integrity/DU2PR04MB8630D83FE9BBC0D782C4FAF595089@DU2PR04MB8630.eurprd04.prod.outlook.com/

---
To: Jarkko Sakkinen <[email protected]>
To: "Horia Geantă" <[email protected]>
To: Mimi Zohar <[email protected]>
To: Pankaj Gupta <[email protected]>
To: Herbert Xu <[email protected]>
To: "David S. Miller" <[email protected]>
To: James Bottomley <[email protected]>
Cc: David Howells <[email protected]>
Cc: James Morris <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: Steffen Trumtrar <[email protected]>
Cc: Jan Luebbe <[email protected]>
Cc: David Gstir <[email protected]>
Cc: Eric Biggers <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Franck LENORMAND <[email protected]>
Cc: Sumit Garg <[email protected]>
Cc: Andreas Rammhold <[email protected]>
Cc: Tim Harvey <[email protected]>
Cc: Matthias Schiffer <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Ahmad Fatoum (6):
KEYS: trusted: allow use of TEE as backend without TCG_TPM support
KEYS: trusted: allow use of kernel RNG for key material
crypto: caam - add in-kernel interface for blob generator
KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
doc: trusted-encrypted: describe new CAAM trust source
MAINTAINERS: add myself as CAAM trusted key maintainer

.../admin-guide/kernel-parameters.txt | 11 ++
.../security/keys/trusted-encrypted.rst | 60 ++++++-
MAINTAINERS | 9 +
drivers/crypto/caam/Kconfig | 3 +
drivers/crypto/caam/Makefile | 1 +
drivers/crypto/caam/blob_gen.c | 164 ++++++++++++++++++
include/keys/trusted-type.h | 2 +-
include/keys/trusted_caam.h | 11 ++
include/soc/fsl/caam-blob.h | 102 +++++++++++
security/keys/Kconfig | 18 +-
security/keys/trusted-keys/Kconfig | 38 ++++
security/keys/trusted-keys/Makefile | 10 +-
security/keys/trusted-keys/trusted_caam.c | 82 +++++++++
security/keys/trusted-keys/trusted_core.c | 45 ++++-
14 files changed, 527 insertions(+), 29 deletions(-)
create mode 100644 drivers/crypto/caam/blob_gen.c
create mode 100644 include/keys/trusted_caam.h
create mode 100644 include/soc/fsl/caam-blob.h
create mode 100644 security/keys/trusted-keys/Kconfig
create mode 100644 security/keys/trusted-keys/trusted_caam.c

--
2.30.2


2022-04-29 06:31:33

by Ahmad Fatoum

[permalink] [raw]
Subject: [PATCH v8 3/6] crypto: caam - add in-kernel interface for blob generator

The NXP Cryptographic Acceleration and Assurance Module (CAAM)
can be used to protect user-defined data across system reboot:

- When the system is fused and boots into secure state, the master
key is a unique never-disclosed device-specific key
- random key is encrypted by key derived from master key
- data is encrypted using the random key
- encrypted data and its encrypted random key are stored alongside
- This blob can now be safely stored in non-volatile memory

On next power-on:
- blob is loaded into CAAM
- CAAM writes decrypted data either into memory or key register

Add functions to realize encrypting and decrypting into memory alongside
the CAAM driver.

They will be used in a later commit as a source for the trusted key
seal/unseal mechanism.

Reviewed-by: David Gstir <[email protected]>
Reviewed-by: Pankaj Gupta <[email protected]>
Tested-by: Tim Harvey <[email protected]>
Tested-by: Matthias Schiffer <[email protected]>
Tested-by: Pankaj Gupta <[email protected]>
Signed-off-by: Steffen Trumtrar <[email protected]>
Signed-off-by: Ahmad Fatoum <[email protected]>
---
v7 -> v8:
- remove unneeded new line in kernel doc (Jarkko)
- Make comments parse as kernel-doc and fix associated warnings
- add Pankaj's Tested-by
v6 -> v7:
- Added more verbose comment on how CAAM_BLOB_DESC_BYTES_MAX adds up.
- remove error message on kzalloc failure (checkpatch)
- Replaced buffer arguments with structure containing them (Pankaj)
v5 -> v6:
- Dropped caam_blob_alloc_desc() in favor of kzalloc() with fixed size.
This simplifies code and wastes at most 12 bytes which are freed
at the end of the function anyway.
- Factored out common code between caam_encap_blob and caam_decap_blob
as both functions were largely identical
- use append_seq_(in|out)_ptr_intlen for both encap/decap as a result
- use reverse christmas tree order for caam_process_blob variable
definitions.
v4 -> v5:
- Collected Reviewed-by's and Tested-by's
- Note in CAAM patch what CAAM is (Jarkko)
v3 -> v4:
- Collected Acked-by's, Reviewed-by's and Tested-by
- Fixed typo spotted by David
v2 -> v3:
- No change
v1 -> v2:
- Enforce maximum keymod size (Horia)
- Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
(Horia)
- Make blobifier handle private to CAAM glue code file (Horia)

To: "Horia Geantă" <[email protected]>
To: Pankaj Gupta <[email protected]>
To: Herbert Xu <[email protected]>
To: "David S. Miller" <[email protected]>
Cc: James Bottomley <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: Mimi Zohar <[email protected]>
Cc: David Howells <[email protected]>
Cc: James Morris <[email protected]>
Cc: Eric Biggers <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: Jan Luebbe <[email protected]>
Cc: David Gstir <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Franck LENORMAND <[email protected]>
Cc: Matthias Schiffer <[email protected]>
Cc: Sumit Garg <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/caam/Kconfig | 3 +
drivers/crypto/caam/Makefile | 1 +
drivers/crypto/caam/blob_gen.c | 164 +++++++++++++++++++++++++++++++++
include/soc/fsl/caam-blob.h | 102 ++++++++++++++++++++
4 files changed, 270 insertions(+)
create mode 100644 drivers/crypto/caam/blob_gen.c
create mode 100644 include/soc/fsl/caam-blob.h

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index 84ea7cba5ee5..ea9f8b1ae981 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -151,6 +151,9 @@ 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_BLOB_GEN
+ bool
+
endif # CRYPTO_DEV_FSL_CAAM_JR

endif # CRYPTO_DEV_FSL_CAAM
diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 3570286eb9ce..25f7ae5a4642 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -21,6 +21,7 @@ caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o pkc_desc.o
+caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o

caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o
ifneq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),)
diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
new file mode 100644
index 000000000000..d0b1a0015308
--- /dev/null
+++ b/drivers/crypto/caam/blob_gen.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2015 Pengutronix, Steffen Trumtrar <[email protected]>
+ * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <[email protected]>
+ */
+
+#include <linux/device.h>
+#include <soc/fsl/caam-blob.h>
+
+#include "compat.h"
+#include "desc_constr.h"
+#include "desc.h"
+#include "error.h"
+#include "intern.h"
+#include "jr.h"
+#include "regs.h"
+
+#define CAAM_BLOB_DESC_BYTES_MAX \
+ /* Command to initialize & stating length of descriptor */ \
+ (CAAM_CMD_SZ + \
+ /* Command to append the key-modifier + key-modifier data */ \
+ CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
+ /* Command to include input key + pointer to the input key */ \
+ CAAM_CMD_SZ + CAAM_PTR_SZ_MAX + \
+ /* Command to include output key + pointer to the output key */ \
+ CAAM_CMD_SZ + CAAM_PTR_SZ_MAX + \
+ /* Command describing the operation to perform */ \
+ CAAM_CMD_SZ)
+
+struct caam_blob_priv {
+ struct device jrdev;
+};
+
+struct caam_blob_job_result {
+ int err;
+ struct completion completion;
+};
+
+static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err, void *context)
+{
+ struct caam_blob_job_result *res = context;
+ int ecode = 0;
+
+ dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
+
+ if (err)
+ ecode = caam_jr_strstatus(dev, err);
+
+ res->err = ecode;
+
+ /*
+ * Upon completion, desc points to a buffer containing a CAAM job
+ * descriptor which encapsulates data into an externally-storable
+ * blob.
+ */
+ complete(&res->completion);
+}
+
+int caam_process_blob(struct caam_blob_priv *priv,
+ struct caam_blob_info *info, bool encap)
+{
+ struct caam_blob_job_result testres;
+ struct device *jrdev = &priv->jrdev;
+ dma_addr_t dma_in, dma_out;
+ int op = OP_PCLID_BLOB;
+ size_t output_len;
+ u32 *desc;
+ int ret;
+
+ if (info->key_mod_len > CAAM_BLOB_KEYMOD_LENGTH)
+ return -EINVAL;
+
+ if (encap) {
+ op |= OP_TYPE_ENCAP_PROTOCOL;
+ output_len = info->input_len + CAAM_BLOB_OVERHEAD;
+ } else {
+ op |= OP_TYPE_DECAP_PROTOCOL;
+ output_len = info->input_len - CAAM_BLOB_OVERHEAD;
+ }
+
+ desc = kzalloc(CAAM_BLOB_DESC_BYTES_MAX, GFP_KERNEL | GFP_DMA);
+ if (!desc)
+ return -ENOMEM;
+
+ dma_in = dma_map_single(jrdev, info->input, info->input_len,
+ 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, info->output, output_len,
+ 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, info->key_mod, info->key_mod_len,
+ info->key_mod_len, CLASS_2 | KEY_DEST_CLASS_REG);
+ append_seq_in_ptr_intlen(desc, dma_in, info->input_len, 0);
+ append_seq_out_ptr_intlen(desc, dma_out, output_len, 0);
+ append_operation(desc, op);
+
+ print_hex_dump_debug("data@"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 1, info->input,
+ info->input_len, false);
+ print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 1, desc,
+ desc_bytes(desc), false);
+
+ testres.err = 0;
+ init_completion(&testres.completion);
+
+ ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
+ if (ret == -EINPROGRESS) {
+ wait_for_completion(&testres.completion);
+ ret = testres.err;
+ print_hex_dump_debug("output@"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 1, info->output,
+ output_len, false);
+ }
+
+ if (ret == 0)
+ info->output_len = output_len;
+
+ dma_unmap_single(jrdev, dma_out, output_len, DMA_FROM_DEVICE);
+out_unmap_in:
+ dma_unmap_single(jrdev, dma_in, info->input_len, DMA_TO_DEVICE);
+out_free:
+ kfree(desc);
+
+ return ret;
+}
+EXPORT_SYMBOL(caam_process_blob);
+
+struct caam_blob_priv *caam_blob_gen_init(void)
+{
+ struct device *jrdev;
+
+ jrdev = caam_jr_alloc();
+ if (IS_ERR(jrdev))
+ return ERR_CAST(jrdev);
+
+ return container_of(jrdev, struct caam_blob_priv, jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_init);
+
+void caam_blob_gen_exit(struct caam_blob_priv *priv)
+{
+ caam_jr_free(&priv->jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_exit);
diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
new file mode 100644
index 000000000000..ec57eec4f2d2
--- /dev/null
+++ b/include/soc/fsl/caam-blob.h
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <[email protected]>
+ */
+
+#ifndef __CAAM_BLOB_GEN
+#define __CAAM_BLOB_GEN
+
+#include <linux/types.h>
+#include <linux/errno.h>
+
+#define CAAM_BLOB_KEYMOD_LENGTH 16
+#define CAAM_BLOB_OVERHEAD (32 + 16)
+#define CAAM_BLOB_MAX_LEN 4096
+
+struct caam_blob_priv;
+
+/**
+ * struct caam_blob_info - information for CAAM blobbing
+ * @input: pointer to input buffer (must be DMAable)
+ * @input_len: length of @input buffer in bytes.
+ * @output: pointer to output buffer (must be DMAable)
+ * @output_len: length of @output buffer in bytes.
+ * @key_mod: key modifier
+ * @key_mod_len: length of @key_mod in bytes.
+ * May not exceed %CAAM_BLOB_KEYMOD_LENGTH
+ */
+struct caam_blob_info {
+ void *input;
+ size_t input_len;
+
+ void *output;
+ size_t output_len;
+
+ const void *key_mod;
+ size_t key_mod_len;
+};
+
+/**
+ * caam_blob_gen_init - initialize blob generation
+ * Return: pointer to new caam_blob_priv instance on success
+ * and error pointer otherwise
+ */
+struct caam_blob_priv *caam_blob_gen_init(void);
+
+/**
+ * caam_blob_gen_exit - free blob generation resources
+ * @priv: instance returned by caam_blob_gen_init
+ */
+void caam_blob_gen_exit(struct caam_blob_priv *priv);
+
+/**
+ * caam_process_blob - encapsulate or decapsulate blob
+ * @priv: instance returned by caam_blob_gen_init
+ * @info: pointer to blobbing info describing key, blob and
+ * key modifier buffers.
+ * @encap: true for encapsulation, false for decapsulation
+ *
+ * Return: %0 and sets info->output_len on success and a negative
+ * error code otherwise.
+ */
+int caam_process_blob(struct caam_blob_priv *priv,
+ struct caam_blob_info *info, bool encap);
+
+/**
+ * caam_encap_blob - encapsulate blob
+ * @priv: instance returned by caam_blob_gen_init
+ * @info: pointer to blobbing info describing input key,
+ * output blob and key modifier buffers.
+ *
+ * Return: %0 and sets @info->output_len on success and
+ * a negative error code otherwise.
+ */
+static inline int caam_encap_blob(struct caam_blob_priv *priv,
+ struct caam_blob_info *info)
+{
+ if (info->output_len < info->input_len + CAAM_BLOB_OVERHEAD)
+ return -EINVAL;
+
+ return caam_process_blob(priv, info, true);
+}
+
+/**
+ * caam_decap_blob - decapsulate blob
+ * @priv: instance returned by caam_blob_gen_init
+ * @info: pointer to blobbing info describing output key,
+ * input blob and key modifier buffers.
+ *
+ * Return: %0 and sets @info->output_len on success and
+ * a negative error code otherwise.
+ */
+static inline int caam_decap_blob(struct caam_blob_priv *priv,
+ struct caam_blob_info *info)
+{
+ if (info->input_len < CAAM_BLOB_OVERHEAD ||
+ info->output_len < info->input_len - CAAM_BLOB_OVERHEAD)
+ return -EINVAL;
+
+ return caam_process_blob(priv, info, false);
+}
+
+#endif
--
2.30.2

2022-05-05 22:15:01

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 0/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

Hello John,

On 05.05.22 16:58, John Ernberg wrote:
> Gave this a go on iMX8QXP with Linux 5.17.5 and I can't quite get it working.
>
> I get -ENODEV from add_key() via keyctl. When I traced it in dmesg I couldn't
> get an as clear picture as I would like but CAAM (and thus possibly JRs?)
> initialzing after trusted_key.
>
> dmesg snips:
> [ 1.296772] trusted_key: Job Ring Device allocation for transform failed
> ...
> [ 1.799768] caam 31400000.crypto: device ID = 0x0a16040000000100 (Era 9)
> [ 1.807142] caam 31400000.crypto: job rings = 2, qi = 0
> [ 1.822667] caam algorithms registered in /proc/crypto
> [ 1.830541] caam 31400000.crypto: caam pkc algorithms registered in /proc/crypto
> [ 1.841807] caam 31400000.crypto: registering rng-caam
>
> I didn't quite have the time to get a better trace than that.

I don't see a crypto@31400000 node upstream. Where can I see your device tree?
Initcall ordering does the right thing, but if CAAM device probe is deferred beyond
late_initcall, then it won't help.

This is a general limitation with trusted keys at the moment. Anything that's
not there by the time of the late_initcall won't be tried again. You can work
around it by having trusted keys as a module. We might be able to do something
with fw_devlinks in the future and a look into your device tree would help here,
but I think that should be separate from this patch series.

Please let me know if the module build improves the situation for you.

Cheers,
Ahmad

>
> Best regards // John Ernberg


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-05-09 01:26:49

by John Ernberg

[permalink] [raw]
Subject: Re: [PATCH v8 0/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

Hi Ahmad,

> >
> > dmesg snips:
> > [ 1.296772] trusted_key: Job Ring Device allocation for transform failed
> > ...
> > [ 1.799768] caam 31400000.crypto: device ID = 0x0a16040000000100 (Era 9)
> > [ 1.807142] caam 31400000.crypto: job rings = 2, qi = 0
> > [ 1.822667] caam algorithms registered in /proc/crypto
> > [ 1.830541] caam 31400000.crypto: caam pkc algorithms registered in /proc/crypto
> > [ 1.841807] caam 31400000.crypto: registering rng-caam
> >
> > I didn't quite have the time to get a better trace than that.
>
> I don't see a crypto@31400000 node upstream. Where can I see your device tree?

Apologies for forgetting to mention that, I took it from the NXP tree
while removing the SM and SECO bits [1].
I also had to rebase some of their patches onto 5.17 for the CAAM to
probe, as the SCU makes some register pages unavailable.

> Initcall ordering does the right thing, but if CAAM device probe is deferred beyond
> late_initcall, then it won't help.
>
> This is a general limitation with trusted keys at the moment. Anything that's
> not there by the time of the late_initcall won't be tried again. You can work
> around it by having trusted keys as a module. We might be able to do something
> with fw_devlinks in the future and a look into your device tree would help here,
> but I think that should be separate from this patch series.

Thank for you the explanation, it makes sense, and I agree that such work
would be a different patch set.

>
> Please let me know if the module build improves the situation for you.
>

After I changed trusted keys to a module I got it working. Which is good
enough for me as QXP CAAM support is not upstream yet.

Feel free to add my tested by if you need to make another spin.
Tested-by: John Ernberg <[email protected]> # iMX8QXP

I didn't test v9 as I would have to patch around the new patch due to
the SCU.

Best regards // John Ernberg

[1]: https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8-ss-security.dtsi?h=lf-5.10.y

2022-05-11 11:37:05

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 0/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

Hello John,

On 07.05.22 23:30, John Ernberg wrote:
> Hi Ahmad,
>
>>>
>>> dmesg snips:
>>> [ 1.296772] trusted_key: Job Ring Device allocation for transform failed
>>> ...
>>> [ 1.799768] caam 31400000.crypto: device ID = 0x0a16040000000100 (Era 9)
>>> [ 1.807142] caam 31400000.crypto: job rings = 2, qi = 0
>>> [ 1.822667] caam algorithms registered in /proc/crypto
>>> [ 1.830541] caam 31400000.crypto: caam pkc algorithms registered in /proc/crypto
>>> [ 1.841807] caam 31400000.crypto: registering rng-caam
>>>
>>> I didn't quite have the time to get a better trace than that.
>>
>> I don't see a crypto@31400000 node upstream. Where can I see your device tree?
>
> Apologies for forgetting to mention that, I took it from the NXP tree
> while removing the SM and SECO bits [1].
> I also had to rebase some of their patches onto 5.17 for the CAAM to
> probe, as the SCU makes some register pages unavailable.

If the CAAM has a dependency on some SCU-provided resource, this
would explain why the driver probes it that late.

>> Initcall ordering does the right thing, but if CAAM device probe is deferred beyond
>> late_initcall, then it won't help.
>>
>> This is a general limitation with trusted keys at the moment. Anything that's
>> not there by the time of the late_initcall won't be tried again. You can work
>> around it by having trusted keys as a module. We might be able to do something
>> with fw_devlinks in the future and a look into your device tree would help here,
>> but I think that should be separate from this patch series.
>
> Thank for you the explanation, it makes sense, and I agree that such work
> would be a different patch set.
>
>>
>> Please let me know if the module build improves the situation for you.
>>
>
> After I changed trusted keys to a module I got it working. Which is good
> enough for me as QXP CAAM support is not upstream yet.

Great!

> Feel free to add my tested by if you need to make another spin.
> Tested-by: John Ernberg <[email protected]> # iMX8QXP
>
> I didn't test v9 as I would have to patch around the new patch due to
> the SCU.

Thanks for the test. I will add it to v10 except for

- "crypto: caam - determine whether CAAM supports blob encap/decap", which
was new in v9
- "doc: trusted-encrypted: describe new CAAM trust source",
"MAINTAINERS: add KEYS-TRUSTED-CAAM" as runtime test isn't affected by these.

Cheers,
Ahmad

>
> Best regards // John Ernberg
>
> [1]: https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8-ss-security.dtsi?h=lf-5.10.y


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |