2018-01-26 19:06:31

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 0/5] Enable CAAM on i.MX7s fix TrustZone issues

V2-resend:
- Patch 0005 lost in the ether - resending

V2:
- Endian detection is ok with TrustZone enabled Horia.
Endian detection logic tested with TrustZone enabled. The register that
this relies on though isn't affected by the lock-down in the first page.
Assuming set of affected registers is actually just the 'deco' registers
though there is no formal statement of that, that I am aware of.

- Moving of TrustZone work-around into u-boot
This set actually doesn't need to deal with TrustZone at all now but, for
the sake of consistency keeping thread title

https://patchwork.ozlabs.org/patch/866460/
https://patchwork.ozlabs.org/patch/866462/
https://patchwork.ozlabs.org/patch/865890/

- Reworded endless loop fix to read a bit better

- Fixes to DTS additions - Rui

- Fixes to number of clocks declared - Rui

V1:
This patch-set enables CAAM on the i.MX7s and fixes a number of issues
identified with the CAAM driver and hardware when TrustZone mode is
enabled.

The first block of patches are simple bug-fixes, followed by a second block
of patches which are simple enabling patches for the i.MX7Solo - note we
aren't enabling for the i.MX7Dual since we don't have hardware to test that
out but it should be a 1:1 mapping for others to enable when appropriate.

The final block in this series implements a fix for using the CAAM when
OPTEE/TrustZone is enabled. The various details are logged in these
threads.

Link: https://github.com/OP-TEE/optee_os/issues/1408
Link: https://tinyurl.com/yam5gv9a
Link: https://patchwork.ozlabs.org/cover/865042

In simple terms, when TrustZone is active the first page of the CAAM
becomes inaccessible to Linux as it has a special 'TZ bit' associated with
it that software cannot toggle or even view AFAIK.

The patches here then

1. Detect when TrustZone is active
2. Detect if u-boot (or OPTEE) has already initialized the RNG

and loads the CAAM driver in a different way - skipping over the RNG
initialization that Linux now no-longer has permissions to carry out.

Should #1 be true but #2 not be true, driver loading stops (and Rui's patch
for the NULL pointer dereference fixes a cash on this path). If #2 is true
but #1 is not then it's a NOP as Linux has full permission to rewrite the
deco registers in the first page of CAAM registers.

Finally then if #1 and #2 are true, the fixes here allow the CAAM to come
up and for the RNG to be useable again.

Bryan O'Donoghue (1):
crypto: caam: Fix endless loop when RNG is already initialized

Rui Miguel Silva (4):
crypto: caam: Fix null dereference at error path
crypto: caam: do not use mem and emi_slow clock for imx7x
clk: imx7d: add CAAM clock
ARM: dts: imx7s: add CAAM device node

arch/arm/boot/dts/imx7s.dtsi | 31 +++++++++++++++++++++++
drivers/clk/imx/clk-imx7d.c | 1 +
drivers/crypto/caam/ctrl.c | 45 ++++++++++++++++++++-------------
include/dt-bindings/clock/imx7d-clock.h | 3 ++-
4 files changed, 61 insertions(+), 19 deletions(-)

--
2.7.4



2018-01-26 19:06:12

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 2/5] crypto: caam: Fix endless loop when RNG is already initialized

commit 1005bccd7a4a ("crypto: caam - enable instantiation of all RNG4 state
handles") introduces a control when incrementing ent_delay which contains
the following comment above it:

/*
* If either SH were instantiated by somebody else
* (e.g. u-boot) then it is assumed that the entropy
* parameters are properly set and thus the function
* setting these (kick_trng(...)) is skipped.
* Also, if a handle was instantiated, do not change
* the TRNG parameters.
*/

This is a problem observed when sec_init() has been run in u-boot and
and TrustZone is enabled. We can fix this by instantiating all rng state
handles in u-boot but, on the Kernel side we should ensure that this
non-terminating path is dealt with.

Fixes: 1005bccd7a4a ("crypto: caam - enable instantiation of all RNG4 state
handles")

Reported-by: Ryan Harkin <[email protected]>
Cc: "Horia Geantă" <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Lukas Auer <[email protected]>
Cc: <[email protected]> # 4.12+
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/crypto/caam/ctrl.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 98986d3..0a1e96b 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -704,7 +704,10 @@ static int caam_probe(struct platform_device *pdev)
ent_delay);
kick_trng(pdev, ent_delay);
ent_delay += 400;
+ } else if (ctrlpriv->rng4_sh_init && inst_handles) {
+ ent_delay += 400;
}
+
/*
* if instantiate_rng(...) fails, the loop will rerun
* and the kick_trng(...) function will modfiy the
--
2.7.4


2018-01-26 19:06:24

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 1/5] crypto: caam: Fix null dereference at error path

From: Rui Miguel Silva <[email protected]>

caam_remove already removes the debugfs entry, so we need to remove the one
immediately before calling caam_remove.

This fix a NULL dereference at error paths is caam_probe fail.

[bod: changed name prefix to "crypto: caam: Fix .."]
[bod: added Fixes tag]

Fixes: 67c2315def06 ("crypto: caam - add Queue Interface (QI) backend
support")

Tested-by: Ryan Harkin <[email protected]>
Signed-off-by: Rui Miguel Silva <[email protected]>
Cc: "Horia Geantă" <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Lukas Auer <[email protected]>
Cc: <[email protected]> # 4.12+
Reviewed-by: Horia Geantă <[email protected]>
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/crypto/caam/ctrl.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 027e121..98986d3 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -809,9 +809,6 @@ static int caam_probe(struct platform_device *pdev)
return 0;

caam_remove:
-#ifdef CONFIG_DEBUG_FS
- debugfs_remove_recursive(ctrlpriv->dfs_root);
-#endif
caam_remove(pdev);
return ret;

--
2.7.4


2018-01-26 19:06:38

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 5/5] ARM: dts: imx7s: add CAAM device node

From: Rui Miguel Silva <[email protected]>

Add CAAM device node to the i.MX7s device tree.

Signed-off-by: Rui Miguel Silva <[email protected]>
Cc: "Horia Geantă" <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Lukas Auer <[email protected]>
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
arch/arm/boot/dts/imx7s.dtsi | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 82ad26e..e38c159 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -805,6 +805,37 @@
status = "disabled";
};

+ crypto: caam@30900000 {
+ compatible = "fsl,sec-v4.0";
+ fsl,sec-era = <4>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x30900000 0x40000>;
+ ranges = <0 0x30900000 0x40000>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7D_CAAM_CLK>,
+ <&clks IMX7D_AHB_CHANNEL_ROOT_CLK>;
+ clock-names = "ipg", "aclk";
+
+ sec_jr0: jr0@1000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x1000 0x1000>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr1: jr1@2000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x2000 0x1000>;
+ interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr2: jr1@3000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x3000 0x1000>;
+ interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
flexcan1: can@30a00000 {
compatible = "fsl,imx7d-flexcan", "fsl,imx6q-flexcan";
reg = <0x30a00000 0x10000>;
--
2.7.4


2018-01-26 19:07:11

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 3/5] crypto: caam: do not use mem and emi_slow clock for imx7x

From: Rui Miguel Silva <[email protected]>

I.MX7x only use two clocks for the CAAM module, so make sure we do not try to
use the mem and the emi_slow clock when running in that imx7d and imx7s machine
type.

[bod: fixed minor trailing whitespace issue]

Signed-off-by: Rui Miguel Silva <[email protected]>
Cc: "Horia Geantă" <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Lukas Auer <[email protected]>
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/crypto/caam/ctrl.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 0a1e96b..1f2dd6a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -333,7 +333,8 @@ static int caam_remove(struct platform_device *pdev)

/* shut clocks off before finalizing shutdown */
clk_disable_unprepare(ctrlpriv->caam_ipg);
- clk_disable_unprepare(ctrlpriv->caam_mem);
+ if (ctrlpriv->caam_mem)
+ clk_disable_unprepare(ctrlpriv->caam_mem);
clk_disable_unprepare(ctrlpriv->caam_aclk);
if (ctrlpriv->caam_emi_slow)
clk_disable_unprepare(ctrlpriv->caam_emi_slow);
@@ -462,14 +463,17 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_ipg = clk;

- clk = caam_drv_identify_clk(&pdev->dev, "mem");
- if (IS_ERR(clk)) {
- ret = PTR_ERR(clk);
- dev_err(&pdev->dev,
- "can't identify CAAM mem clk: %d\n", ret);
- return ret;
+ if (!of_machine_is_compatible("fsl,imx7d") &&
+ !of_machine_is_compatible("fsl,imx7s")) {
+ clk = caam_drv_identify_clk(&pdev->dev, "mem");
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ dev_err(&pdev->dev,
+ "can't identify CAAM mem clk: %d\n", ret);
+ return ret;
+ }
+ ctrlpriv->caam_mem = clk;
}
- ctrlpriv->caam_mem = clk;

clk = caam_drv_identify_clk(&pdev->dev, "aclk");
if (IS_ERR(clk)) {
@@ -480,7 +484,9 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_aclk = clk;

- if (!of_machine_is_compatible("fsl,imx6ul")) {
+ if (!of_machine_is_compatible("fsl,imx6ul") &&
+ !of_machine_is_compatible("fsl,imx7d") &&
+ !of_machine_is_compatible("fsl,imx7s")) {
clk = caam_drv_identify_clk(&pdev->dev, "emi_slow");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
@@ -497,11 +503,13 @@ static int caam_probe(struct platform_device *pdev)
return ret;
}

- ret = clk_prepare_enable(ctrlpriv->caam_mem);
- if (ret < 0) {
- dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
- ret);
- goto disable_caam_ipg;
+ if (ctrlpriv->caam_mem) {
+ ret = clk_prepare_enable(ctrlpriv->caam_mem);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
+ ret);
+ goto disable_caam_ipg;
+ }
}

ret = clk_prepare_enable(ctrlpriv->caam_aclk);
@@ -823,7 +831,8 @@ static int caam_probe(struct platform_device *pdev)
disable_caam_aclk:
clk_disable_unprepare(ctrlpriv->caam_aclk);
disable_caam_mem:
- clk_disable_unprepare(ctrlpriv->caam_mem);
+ if (ctrlpriv->caam_mem)
+ clk_disable_unprepare(ctrlpriv->caam_mem);
disable_caam_ipg:
clk_disable_unprepare(ctrlpriv->caam_ipg);
return ret;
--
2.7.4


2018-01-26 19:07:22

by Bryan O'Donoghue

[permalink] [raw]
Subject: [RESEND PATCH v2 4/5] clk: imx7d: add CAAM clock

From: Rui Miguel Silva <[email protected]>

Add CAAM clock so that we could use the Cryptographic Acceleration and
Assurance Module (CAAM) hardware block.

Signed-off-by: Rui Miguel Silva <[email protected]>
Cc: "Horia Geantă" <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Lukas Auer <[email protected]>
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/clk/imx/clk-imx7d.c | 1 +
include/dt-bindings/clock/imx7d-clock.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 80dc211..52ab096 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -795,6 +795,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = imx_clk_gate4("dram_phym_alt_root_clk", "dram_phym_alt_post_div", base + 0x4130, 0);
clks[IMX7D_DRAM_ALT_ROOT_CLK] = imx_clk_gate4("dram_alt_root_clk", "dram_alt_post_div", base + 0x4130, 0);
clks[IMX7D_OCOTP_CLK] = imx_clk_gate4("ocotp_clk", "ipg_root_clk", base + 0x4230, 0);
+ clks[IMX7D_CAAM_CLK] = imx_clk_gate4("caam_clk", "ipg_root_clk", base + 0x4240, 0);
clks[IMX7D_USB_HSIC_ROOT_CLK] = imx_clk_gate4("usb_hsic_root_clk", "usb_hsic_post_div", base + 0x4420, 0);
clks[IMX7D_SDMA_CORE_CLK] = imx_clk_gate4("sdma_root_clk", "ahb_root_clk", base + 0x4480, 0);
clks[IMX7D_PCIE_CTRL_ROOT_CLK] = imx_clk_gate4("pcie_ctrl_root_clk", "pcie_ctrl_post_div", base + 0x4600, 0);
diff --git a/include/dt-bindings/clock/imx7d-clock.h b/include/dt-bindings/clock/imx7d-clock.h
index e2f99ae..2bc5618 100644
--- a/include/dt-bindings/clock/imx7d-clock.h
+++ b/include/dt-bindings/clock/imx7d-clock.h
@@ -452,5 +452,6 @@
#define IMX7D_OCOTP_CLK 439
#define IMX7D_NAND_RAWNAND_CLK 440
#define IMX7D_NAND_USDHC_BUS_RAWNAND_CLK 441
-#define IMX7D_CLK_END 442
+#define IMX7D_CAAM_CLK 442
+#define IMX7D_CLK_END 443
#endif /* __DT_BINDINGS_CLOCK_IMX7D_H */
--
2.7.4


2018-01-27 00:43:57

by Fabio Estevam

[permalink] [raw]
Subject: Re: [RESEND PATCH v2 0/5] Enable CAAM on i.MX7s fix TrustZone issues

Hi Bryan,

On Fri, Jan 26, 2018 at 5:04 PM, Bryan O'Donoghue
<[email protected]> wrote:

> Bryan O'Donoghue (1):
> crypto: caam: Fix endless loop when RNG is already initialized
>
> Rui Miguel Silva (4):
> crypto: caam: Fix null dereference at error path
> crypto: caam: do not use mem and emi_slow clock for imx7x

Thanks for your series!

> clk: imx7d: add CAAM clock

This one should be posted to the clock driver maintainers.

> ARM: dts: imx7s: add CAAM device node

This one should be posted to the i.MX arch maintainers.