| Message ID | 20211207230206.14637-3-andrey.zhizhikin@leica-geosystems.com |
|---|---|
| State | New |
| Headers | show |
| Series | CAAM Driver: re-factor and set proper JR status | expand |
Hello Rouven, > -----Original Message----- > From: Rouven Czerwinski <r.czerwinski@pengutronix.de> > Sent: Friday, January 7, 2022 12:56 PM [snip] > > Yes, upstream TF-A does not implement the NXP specific SIPs to > communicate with the ROM code to do further image authentication. > Thats also the reason why all JRs are released to normal world, there > is no possible HAB use after TF-A has started. > > > I've been following the build documentation in U-Boot, where the downstream > > TF-A is listed as build prequisites [2] without any mentioning of upstream, > > hence I received a readout that matched the BootROM "1-to-1". > > Yes, since the downstream TF-A is required to authenticate further > images. > > Aside from this the bootrom HAB interface on i.MX8MQ was broken in > interesting ways, I had to implement parsing of the HAB status SRAM > area by hand within barebox. > > > I believe that if the information from NXP regarding JR0 reservation for > > HAB is correct (and I have no reasons to doubt it), then upstream TF-A > > should be adapted in order for HAB feature to work, and in that case this > > patch brings correct HW state description as seeing from Linux. > > JR0 for HAB in S-World makes sense to me, otherwise the bootrom will > probably refuse to work with the JR. > > > And in contrary, if the upstream TF-A is not adjusted to include HAB > > support - then applying this patch would effectively just "remove" one > > JR device, still keeping 2 additional available nodes for HW crypto > > operations in Kernel, with added benefit that both upstream and > > downstream TF-A can be used during the boot without seeing issues later > > in the Kernel. > > Even with the downstream TF-A there is no reason to keep JR0 asigned to > the secure world, unless you are running OP-TEE. JR0 assignement for > HAB shouldn't be required after the bootloader has run, unless you want > to HAB authenticate images from a running Linux kernel. Then this probably should be communicated in U-Boot as there is a patchset proposed in U-Boot, and one of the patches in that series [1] disabled JR0 as well. Once merged - the JR0 is not going to be available for Linux, regardless of the fact that TF-A would set DIDs to 0x1. > > The reason NXP hardcodes the configuration downstream is of course to > support HAB & OP-TEE, but this is still not a reason to hardcode this > assignement into the kernel device tree. They probably also hardcode it > in their downstream kernel versions. Actually, I've checked the downstream NXP Kernel version, and none of the Job Ring nodes (including JR0) are disabled there. > > I can understand that it seems easier to hardcode this in the kernel, > but as I said before, if you are running OP-TEE you need to adjust the > DT anyway since nodes need to be added and you might as well adjust the > jobring configuration than. My first version of this patch has been targeting dynamic register readout to check if DID is set for either S or NS Worlds, but that was rejected due to unreliable readout in HW. Hence this attempt to statically disable node was made. Please note, that there are combinations out there which do have HAB, TF-A but no OP-TEE. In that case patching DT is not an option, and would cause the probing error at boot. Frankly speaking, I'm not sure how to proceed with this further... Clearly, there is an issue that JR devices are not available in certain combination of SW entities that are setting different permissions on JR: upstream TF-A does not do any reservation but does not support HAB (and this is aligned with current DT nodes description); downstream TF-A does perform reservation and support HAB, but does not release JR0 to NS-World causing error on the boot at JR probing. Since those 2 combinations are orthogonal - the only solution that I see (including the patch proposed in U-Boot ML) is to reserve the JR0 for all combinations, loosing it in Linux but covering both TF-A (HAB and non-HAB) at the same time. If you have any other suggestions here - I'm fully opened to receive those! > > Kind regards, > Rouven > > Regards, Andrey Link: [1]: https://lore.kernel.org/u-boot/20211207074129.10955-3-gaurav.jain@nxp.com/
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi index 5b9c2cca9ac4..51465974c4ea 100644 --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi @@ -914,18 +914,22 @@ sec_jr0: jr@1000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x1000 0x1000>; interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + secure-status = "okay"; }; sec_jr1: jr@2000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x2000 0x1000>; interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "okay"; }; sec_jr2: jr@3000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x3000 0x1000>; interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "disabled"; }; }; diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi index ba23b416b5e6..e5edf14319b1 100644 --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi @@ -808,18 +808,22 @@ sec_jr0: jr@1000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x1000 0x1000>; interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + secure-status = "okay"; }; sec_jr1: jr@2000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x2000 0x1000>; interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "okay"; }; sec_jr2: jr@3000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x3000 0x1000>; interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "disabled"; }; }; diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index 977783784342..3c23bf5c3910 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi @@ -661,18 +661,22 @@ sec_jr0: jr@1000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x1000 0x1000>; interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + secure-status = "okay"; }; sec_jr1: jr@2000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x2000 0x1000>; interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "okay"; }; sec_jr2: jr@3000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x3000 0x1000>; interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "disabled"; }; }; diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi index 95d8b95d6120..16c4c9110ce7 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi @@ -999,18 +999,22 @@ sec_jr0: jr@1000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x1000 0x1000>; interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + secure-status = "okay"; }; sec_jr1: jr@2000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x2000 0x1000>; interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "okay"; }; sec_jr2: jr@3000 { compatible = "fsl,sec-v4.0-job-ring"; reg = <0x3000 0x1000>; interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; + secure-status = "disabled"; }; };
CAAM JR nodes are configured by BootROM and are used by various software entities during the boot process before they reach the Kernel. Default BootROM configuration have JR0 and JR1 reserved for S-only access, while JR2 is generally available for both S and NS access. HAB feature of i.MX8M family does require that JR0 is reserved exclusively in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE can later reclaim the JR2 via dt_enable_secure_status() call, and modify the DID to hold it in S-World only. The above setup has been discovered during review of CAAM patchset presented to U-Boot integration [1], and does not correspond to the status on jr nodes in FDT. This missing status settings leads to the following error message during jr node probing: [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 JR register readout after BootROM execution shows the following values: JR0DID_MS = 0x8011 JR1DID_MS = 0x8011 JR2DID_MS = 0x0 This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be reserved for S-World, while JR2 remains accessible from NS-World. Provide the correct status for JR nodes in imx8m derivatives, which have a following meaning: - JR0: S-only - JR1: visible in both - JR2: NS-only Note, that JR2 is initially marked to be NS-only which does correspond to DID readout when OP-TEE is not present. Once present, OP-TEE will reclaim the JR2 and set both "status" and "secure-status" to claim JR2 for S-only access. Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com> Link: [1]: https://lore.kernel.org/u-boot/AM6PR06MB4691FC905FE5658BE4B15C11A6609@AM6PR06MB4691.eurprd06.prod.outlook.com/ --- Changes in V3: - No change, new patch introduced arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 ++++ arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 ++++ arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++ arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++++ 4 files changed, 16 insertions(+)