[U-Boot] [PATCH v2 0/2] Fix CAAM for TrustZone enable for warp7
V2: - Add an explicit assignment of JRMID when setting job-ring ownership Required on my reference part where the JRMID field is not set on the third job-ring
V1: This series is the u-boot fix to a problem we encountered when enabling OPTEE/TrustZone on the WaRP7. The symptom is once TrustZone is activated the first page of CAAM registers becomes read-only, read-zero from the perspective of Linux and other non TrustZone contexts.
Offlining the problem with Peng Fan[1] we eventually came to realise the problem could be worked around by
1. Making Linux skip RNG initialisation - a set of patches should be hitting LKML to do just that.
2. Initialising the RNG either from u-boot or OPTEE. In this case u-boot is the right place to-do that because there's upstream code in u-boot that just works. Patch #2 does that for the WaRP7.
3. Ensuring the job-ring registers are assigned to the non TrustZone mode. On the i.MX7 after the BootROM runs the job-ring registers are assigned to TrustZone. Patch #1 does that for all CAAM hardware.
On point #3 this ordinarily isn't a problem because unless TrustZone is activated the restrictions on the job-ring registers don't kick in, its only after enabling TrustZone that Linux will loose access to the job-ring registers.
Finally should OPTEE or another TEE want to do things with the job-ring registers it will have sufficient privilege to assign whichever job-ring registers it wants to OPTEE/TEE but will naturally then have to arbitrate with Linux to inform the Kernel CAAM driver which job-ring registers it can and cannot access.
That arbitration process is for a future putative OPTEE/TEE CAAM driver to solve and is out of scope of this patchset.
[1] Thanks for all of your help BTW - Peng, there's no way this would be working without you giving direction on how.
Bryan O'Donoghue (2): drivers/crypto/fsl: assign job-rings to non-TrustZone warp7 : run sec_init for CAAM RNG
board/warp7/warp7.c | 6 +++++- drivers/crypto/fsl/jr.c | 9 +++++++++ drivers/crypto/fsl/jr.h | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-)
After enabling TrustZone various parts of the CAAM silicon become inaccessible to non TrustZone contexts. The job-ring registers are designed to allow non TrustZone contexts like Linux to still submit jobs to CAAM even after TrustZone has been enabled.
The default job-ring permissions after the BootROM look like this for job-ring zero.
ms=0x00008001 ls=0x00008001
The MS field is JRaMIDR_MS (job ring MID most significant).
Referring to "Security Reference Manual for i.MX 7Dual and 7Solo Applications Processors, Rev. 0, 03/2017" section 8.10.4 we see that JROWN_NS controls whether or not a job-ring is accessible from non TrustZone.
Bit 15 (TrustZone) is the logical inverse of bit 3 hence the above value of 0x8001 shows that JROWN_NS=0 and TrustZone=1.
Clearly then as soon as TrustZone becomes active the job-ring registers are no longer accessible from Linux, which is not what we want.
This patch explicitly sets all job-ring registers to JROWN_NS=1 (non TrustZone) by default and to the Non-Secure MID 001. Both settings are required to successfully assign a job-ring to non-secure mode. If a piece of TrustZone firmware requires ownership of job-ring registers it can unset the JROWN_NS bit itself.
This patch in conjunction with a modification of the Linux kernel to skip HWRNG initialisation makes CAAM usable to Linux with TrustZone enabled.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Peng Fan peng.fan@nxp.com Cc: Alex Porosanu alexandru.porosanu@nxp.com Cc: Ruchika Gupta ruchika.gupta@nxp.com Cc: Aneesh Bansal aneesh.bansal@nxp.com Link: https://github.com/OP-TEE/optee_os/issues/1408 Link: https://tinyurl.com/yam5gv9a --- drivers/crypto/fsl/jr.c | 9 +++++++++ drivers/crypto/fsl/jr.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index a6dad01..34bd070 100644 --- a/drivers/crypto/fsl/jr.c +++ b/drivers/crypto/fsl/jr.c @@ -579,6 +579,8 @@ int sec_init_idx(uint8_t sec_idx) { ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx); uint32_t mcr = sec_in32(&sec->mcfgr); + uint32_t jrown_ns; + int i; int ret = 0;
#ifdef CONFIG_FSL_CORENET @@ -634,6 +636,13 @@ int sec_init_idx(uint8_t sec_idx) #endif #endif
+ /* Set ownership of job rings to non-TrustZone mode by default */ + for (i = 0; i < ARRAY_SIZE(sec->jrliodnr); i++) { + jrown_ns = sec_in32(&sec->jrliodnr[i].ms); + jrown_ns |= JROWN_NS | JRMID_NS; + sec_out32(&sec->jrliodnr[i].ms, jrown_ns); + } + ret = jr_init(sec_idx); if (ret < 0) { printf("SEC initialization failed\n"); diff --git a/drivers/crypto/fsl/jr.h b/drivers/crypto/fsl/jr.h index f546226..ef515e7 100644 --- a/drivers/crypto/fsl/jr.h +++ b/drivers/crypto/fsl/jr.h @@ -34,6 +34,8 @@ #define JRNSLIODN_MASK 0x0fff0000 #define JRSLIODN_SHIFT 0 #define JRSLIODN_MASK 0x00000fff +#define JROWN_NS 0x00000008 +#define JRMID_NS 0x00000001
#define JQ_DEQ_ERR -1 #define JQ_DEQ_TO_ERR -2
On Fri, 2018-01-26 at 02:09 +0000, Bryan O'Donoghue wrote:
After enabling TrustZone various parts of the CAAM silicon become inaccessible to non TrustZone contexts. The job-ring registers are designed to allow non TrustZone contexts like Linux to still submit jobs to CAAM even after TrustZone has been enabled.
The default job-ring permissions after the BootROM look like this for job-ring zero.
ms=0x00008001 ls=0x00008001
The MS field is JRaMIDR_MS (job ring MID most significant).
Referring to "Security Reference Manual for i.MX 7Dual and 7Solo Applications Processors, Rev. 0, 03/2017" section 8.10.4 we see that JROWN_NS controls whether or not a job-ring is accessible from non TrustZone.
Bit 15 (TrustZone) is the logical inverse of bit 3 hence the above value of 0x8001 shows that JROWN_NS=0 and TrustZone=1.
Clearly then as soon as TrustZone becomes active the job-ring registers are no longer accessible from Linux, which is not what we want.
This patch explicitly sets all job-ring registers to JROWN_NS=1 (non TrustZone) by default and to the Non-Secure MID 001. Both settings are required to successfully assign a job-ring to non-secure mode. If a piece of TrustZone firmware requires ownership of job-ring registers it can unset the JROWN_NS bit itself.
This patch in conjunction with a modification of the Linux kernel to skip HWRNG initialisation makes CAAM usable to Linux with TrustZone enabled.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Peng Fan peng.fan@nxp.com Cc: Alex Porosanu alexandru.porosanu@nxp.com Cc: Ruchika Gupta ruchika.gupta@nxp.com Cc: Aneesh Bansal aneesh.bansal@nxp.com Link: https://github.com/OP-TEE/optee_os/issues/1408 Link: https://tinyurl.com/yam5gv9a
drivers/crypto/fsl/jr.c | 9 +++++++++ drivers/crypto/fsl/jr.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index a6dad01..34bd070 100644 --- a/drivers/crypto/fsl/jr.c +++ b/drivers/crypto/fsl/jr.c @@ -579,6 +579,8 @@ int sec_init_idx(uint8_t sec_idx) { ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx); uint32_t mcr = sec_in32(&sec->mcfgr);
- uint32_t jrown_ns;
- int i; int ret = 0;
#ifdef CONFIG_FSL_CORENET @@ -634,6 +636,13 @@ int sec_init_idx(uint8_t sec_idx) #endif #endif
- /* Set ownership of job rings to non-TrustZone mode by
default */
- for (i = 0; i < ARRAY_SIZE(sec->jrliodnr); i++) {
jrown_ns = sec_in32(&sec->jrliodnr[i].ms);jrown_ns |= JROWN_NS | JRMID_NS;sec_out32(&sec->jrliodnr[i].ms, jrown_ns);- }
- ret = jr_init(sec_idx); if (ret < 0) { printf("SEC initialization failed\n");
diff --git a/drivers/crypto/fsl/jr.h b/drivers/crypto/fsl/jr.h index f546226..ef515e7 100644 --- a/drivers/crypto/fsl/jr.h +++ b/drivers/crypto/fsl/jr.h @@ -34,6 +34,8 @@ #define JRNSLIODN_MASK 0x0fff0000 #define JRSLIODN_SHIFT 0 #define JRSLIODN_MASK 0x00000fff +#define JROWN_NS 0x00000008 +#define JRMID_NS 0x00000001
#define JQ_DEQ_ERR -1 #define JQ_DEQ_TO_ERR -2
Hi Bryan,
just successfully tested this on my imx7d board. Your addition in v2 was quite important since job ring 2 fails to probe otherwise (was wondering why that happened yesterday). The CAAM and all job rings probe successfully now.
Tested-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
This patch adds a sec_init call into board_init. Doing so in conjunction with the patch "drivers/crypto/fsl: assign job-rings to non-TrustZone" enables use of the CAAM in Linux when OPTEE/TrustZone is active.
u-boot will initialise the RNG and assign ownership of the job-ring registers to a non-TrustZone context. Linux then simply has to detect or be told to skip RNG initialisation.
This change is safe both for the OPTEE/TrustZone boot path and the regular non-OPTEE/TrustZone boot path.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Peng Fan peng.fan@nxp.com Cc: Marco Franchi marco.franchi@nxp.com Cc: Vanessa Maegima vanessa.maegima@nxp.com Cc: Stefano Babic sbabic@denx.de --- board/warp7/warp7.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/board/warp7/warp7.c b/board/warp7/warp7.c index 337e76b..219ab6f 100644 --- a/board/warp7/warp7.c +++ b/board/warp7/warp7.c @@ -16,6 +16,7 @@ #include <asm/io.h> #include <common.h> #include <fsl_esdhc.h> +#include <fsl_sec.h> #include <i2c.h> #include <mmc.h> #include <asm/arch/crm_regs.h> @@ -225,6 +226,10 @@ int board_init(void) setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1); #endif
+ #ifdef CONFIG_FSL_CAAM + sec_init(); + #endif + return 0; }
@@ -366,5 +371,4 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
return 0; } - #endif /* ifdef CONFIG_USB_GADGET */
On Fri, 2018-01-26 at 02:09 +0000, Bryan O'Donoghue wrote:
This patch adds a sec_init call into board_init. Doing so in conjunction with the patch "drivers/crypto/fsl: assign job-rings to non- TrustZone" enables use of the CAAM in Linux when OPTEE/TrustZone is active.
u-boot will initialise the RNG and assign ownership of the job-ring registers to a non-TrustZone context. Linux then simply has to detect or be told to skip RNG initialisation.
This change is safe both for the OPTEE/TrustZone boot path and the regular non-OPTEE/TrustZone boot path.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Peng Fan peng.fan@nxp.com Cc: Marco Franchi marco.franchi@nxp.com Cc: Vanessa Maegima vanessa.maegima@nxp.com Cc: Stefano Babic sbabic@denx.de
board/warp7/warp7.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/board/warp7/warp7.c b/board/warp7/warp7.c index 337e76b..219ab6f 100644 --- a/board/warp7/warp7.c +++ b/board/warp7/warp7.c @@ -16,6 +16,7 @@ #include <asm/io.h> #include <common.h> #include <fsl_esdhc.h> +#include <fsl_sec.h> #include <i2c.h> #include <mmc.h> #include <asm/arch/crm_regs.h> @@ -225,6 +226,10 @@ int board_init(void) setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1); #endif
- #ifdef CONFIG_FSL_CAAM
sec_init();- #endif
- return 0;
}
@@ -366,5 +371,4 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
return 0; }
#endif /* ifdef CONFIG_USB_GADGET */
Hi Bryan,
this fails to apply for me on current HEAD. It seems like you have additional modifications to wrap7.c in your tree (there is no CONFIG_USB_GADGET on master).
Regarding the patch, would it make sense to put sec_init() somewhere else, so that it does not have to be duplicated in the board file for all platforms with CAAM?
Thanks, Lukas
On 26/01/18 09:09, Auer, Lukas wrote:
Hi Bryan,
this fails to apply for me on current HEAD. It seems like you have additional modifications to wrap7.c in your tree (there is no CONFIG_USB_GADGET on master).
I'm carrying a few patches locally and upstreaming gradually - got caught out here...
Regarding the patch, would it make sense to put sec_init() somewhere else, so that it does not have to be duplicated in the board file for all platforms with CAAM?
It does... to me. Looking at these .. I'd say leave the old powerpc/freescale stuff alone.
This works for me as an alternative when I tested it
diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c index d160e80..d399fd8 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -261,6 +261,9 @@ int arch_misc_init(void) else env_set("soc", "imx7s"); #endif + #ifdef CONFIG_FSL_CAAM + sec_init(); + #endif
return 0; }
perhaps this would work for other i.mx processors
diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c index 43cb581..679c23b 100644 --- a/arch/arm/mach-imx/mx6/soc.c +++ b/arch/arm/mach-imx/mx6/soc.c @@ -515,6 +515,10 @@ int board_postclk_init(void)
set_ldo_voltage(LDO_SOC, 1175); /* Set VDDSOC to 1.175V */
+#ifdef CONFIG_FSL_CAAM + sec_init(); +#endif + return 0; }
diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c index 454665a..dc3d601 100644 --- a/arch/arm/mach-imx/mx7ulp/soc.c +++ b/arch/arm/mach-imx/mx7ulp/soc.c @@ -57,6 +57,11 @@ int arch_cpu_init(void) #ifdef CONFIG_BOARD_POSTCLK_INIT int board_postclk_init(void) { + +#ifdef CONFIG_FSL_CAAM + sec_init(); +#endif + return 0; } #endif
I'd say the right thing to do is - fix it for all i.MX7D/S and let others with access to mx6/mx7ulp etc test/patch themselves.
Anyway I'll send a generic patch for i.mx7s/d in arch_misc_init()
--- bod
On Fri, 2018-01-26 at 11:32 +0000, Bryan O'Donoghue wrote:
On 26/01/18 09:09, Auer, Lukas wrote:
Hi Bryan,
this fails to apply for me on current HEAD. It seems like you have additional modifications to wrap7.c in your tree (there is no CONFIG_USB_GADGET on master).
I'm carrying a few patches locally and upstreaming gradually - got caught out here...
Regarding the patch, would it make sense to put sec_init() somewhere else, so that it does not have to be duplicated in the board file for all platforms with CAAM?
It does... to me. Looking at these .. I'd say leave the old powerpc/freescale stuff alone.
This works for me as an alternative when I tested it
diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach- imx/mx7/soc.c index d160e80..d399fd8 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -261,6 +261,9 @@ int arch_misc_init(void) else env_set("soc", "imx7s"); #endif
#ifdef CONFIG_FSL_CAAMsec_init();#endif return 0;}
perhaps this would work for other i.mx processors
diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach- imx/mx6/soc.c index 43cb581..679c23b 100644 --- a/arch/arm/mach-imx/mx6/soc.c +++ b/arch/arm/mach-imx/mx6/soc.c @@ -515,6 +515,10 @@ int board_postclk_init(void)
set_ldo_voltage(LDO_SOC, 1175); /* Set VDDSOC to 1.175V */+#ifdef CONFIG_FSL_CAAM
sec_init();+#endif
return 0;diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach- imx/mx7ulp/soc.c index 454665a..dc3d601 100644 --- a/arch/arm/mach-imx/mx7ulp/soc.c +++ b/arch/arm/mach-imx/mx7ulp/soc.c @@ -57,6 +57,11 @@ int arch_cpu_init(void) #ifdef CONFIG_BOARD_POSTCLK_INIT int board_postclk_init(void) {
+#ifdef CONFIG_FSL_CAAM
sec_init();+#endif
return 0;I'd say the right thing to do is - fix it for all i.MX7D/S and let others with access to mx6/mx7ulp etc test/patch themselves.
Anyway I'll send a generic patch for i.mx7s/d in arch_misc_init()
bod
I agree, that's a good way to add the initialization code.
participants (2)
-
Auer, Lukas -
Bryan O'Donoghue