| Message ID | 20211207230206.14637-3-andrey.zhizhikin@leica-geosystems.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] crypto: caam - convert to use capabilities | expand |
On Thu, 2022-01-06 at 14:08 +0000, ZHIZHIKIN Andrey wrote: > Hello Rouven, > > > -----Original Message----- > > From: Rouven Czerwinski <r.czerwinski@pengutronix.de> > > Sent: Thursday, January 6, 2022 12:27 PM > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux- > > kernel@vger.kernel.org > > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org; > > frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com; > > marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com; > > aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com; > > devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de; > > pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com; > > martink@posteo.de; daniel.baluta@nxp.com; linux-arm-kernel@lists.infradead.org; > > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com; > > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org; > > kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org; davem@davemloft.net; > > l.stach@pengutronix.de > > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr > > > > Hi Andrey, > > > > On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote: > > > 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. > > > > While I can understand that you want to fix your use case for when HAB > > is enabled, note that this is disabling JR0 in the none-HAB case as > > well. > > This is not totally correct, as this patch does address the reservation of > JR0 by BootROM in both HAB and non-HAB configurations. My current setup does > not include HAB functionality enabled, and I still do observe boot errors > that are listed in commit message. This is due to the fact that the BootROM > does not release JR0 to NS-World regardless of whether HAB is enabled or not. > This has been discussed in the U-Boot thread I provided the link in the patch. > > This patch does rather bring the correct HW module description as seeing > from Linux. I took a look into i.MX8MQ, the bootrom indeed sets 0x8011 for JR0 & JR1: JR0:0000000000008011 JR1:0000000000008011 JR2:0000000000000000 TF-A >CAAM JR0:0000000000000001 JR1:0000000000000001 JR2:0000000000000001 However at least the upstream TF-A reconfigures the DIDs to 1 for all i.MX8M* derivates. So while it is technically correct to change the DT values as you describe, the downstream TF-A and upstream TF-A seem to differ in their configuration. I also think it's a bad move to hardcode the JR configuration to the boot ROM config, since AFAIK i.MX8M* can not be run without TF-A. And IMO the upstream kernel should follow what the upstream TF-A does in this case. > > > IMO this should be handled correctly by the bootloader and/or OP- > > TEE. The default upstream configuration for OP-TEE is to not use the > > CAAM at runtime as well, since linux runtime PM disablement of the CAAM > > will lock up OP-TEE when it tries to access the CAAM. > > If by handling you mean releasing JR0 reservation - then yes, it should be > done by either SPL or TF-A as they do run in S World. In such a case, DTB > bindings need to be adapted further according to the new state. Until this > done - this patch does provide a correct state of HW to the Kernel. Upstream TF-A simply releases all JRs to the normal world, matching the current DTB description. Kind Regards, Rouven Czerwinski > > > > Kind regards, > > Rouven Czerwinski > > > > > > > > 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(+) > > > > > > 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"; > > > }; > > > }; > > > > > > > -- > > Pengutronix e.K. | Rouven Czerwinski | > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > > -- andrey
Hi Rouven,
Am 2022-01-07 10:46, schrieb Rouven Czerwinski:
> .. since AFAIK i.MX8M* can not be run without TF-A.
Are you sure? There probably aren't any boards out there
without TF-A, but why shouldn't it work without it?
-michael
On Fri, 2022-01-07 at 10:40 +0000, ZHIZHIKIN Andrey wrote: > Hello Rouven, > > > -----Original Message----- > > From: Rouven Czerwinski <r.czerwinski@pengutronix.de> > > Sent: Friday, January 7, 2022 10:46 AM > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux- > > kernel@vger.kernel.org > > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org; > > krzk@kernel.org; leonard.crestez@nxp.com; festevam@gmail.com; marex@denx.de; > > herbert@gondor.apana.org.au; horia.geanta@nxp.com; daniel.baluta@nxp.com; > > frieder.schrempf@kontron.de; linux-imx@nxp.com; devicetree@vger.kernel.org; > > hongxing.zhu@nxp.com; s.hauer@pengutronix.de; pankaj.gupta@nxp.com; > > robh+dt@kernel.org; thunder.leizhen@huawei.com; martink@posteo.de; > > aford173@gmail.com; linux-arm-kernel@lists.infradead.org; > > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com; > > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org; > > kernel@pengutronix.de; l.stach@pengutronix.de; shawnguo@kernel.org; > > davem@davemloft.net; jun.li@nxp.com > > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr > > > > > > On Thu, 2022-01-06 at 14:08 +0000, ZHIZHIKIN Andrey wrote: > > > Hello Rouven, > > > > > > > -----Original Message----- > > > > From: Rouven Czerwinski <r.czerwinski@pengutronix.de> > > > > Sent: Thursday, January 6, 2022 12:27 PM > > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux- > > > > kernel@vger.kernel.org > > > > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org; > > > > frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com; > > > > marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com; > > > > aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com; > > > > devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de; > > > > pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com; > > > > martink@posteo.de; daniel.baluta@nxp.com; linux-arm- > > kernel@lists.infradead.org; > > > > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com; > > > > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux- > > crypto@vger.kernel.org; > > > > kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org; > > davem@davemloft.net; > > > > l.stach@pengutronix.de > > > > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam > > jr > > > > > > > > Hi Andrey, > > > > > > > > On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote: > > > > > 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. > > > > > > > > While I can understand that you want to fix your use case for when HAB > > > > is enabled, note that this is disabling JR0 in the none-HAB case as > > > > well. > > > > > > This is not totally correct, as this patch does address the reservation of > > > JR0 by BootROM in both HAB and non-HAB configurations. My current setup does > > > not include HAB functionality enabled, and I still do observe boot errors > > > that are listed in commit message. This is due to the fact that the BootROM > > > does not release JR0 to NS-World regardless of whether HAB is enabled or not. > > > This has been discussed in the U-Boot thread I provided the link in the patch. > > > > > > This patch does rather bring the correct HW module description as seeing > > > from Linux. > > > > I took a look into i.MX8MQ, the bootrom indeed sets 0x8011 for JR0 & > > JR1: > > JR0:0000000000008011 > > JR1:0000000000008011 > > JR2:0000000000000000 > > TF-A > > > CAAM > > JR0:0000000000000001 > > JR1:0000000000000001 > > JR2:0000000000000001 > > > > However at least the upstream TF-A reconfigures the DIDs to 1 for all > > i.MX8M* derivates. So while it is technically correct to change the DT > > values as you describe, the downstream TF-A and upstream TF-A seem to > > differ in their configuration. I also think it's a bad move to hardcode > > the JR configuration to the boot ROM config, since AFAIK i.MX8M* can > > not be run without TF-A. And IMO the upstream kernel should follow what > > the upstream TF-A does in this case. > > This is indeed an interesting piece of information, thanks a lot! > > From what I understood in previous discussions for this series here in > the Kernel, and on U-Boot ML: JR0 is required to be held reserved in > S-World for HAB to operate, and this is clearly communicated by NXP in [1]. > If my understanding is correct, then upstream TF-A either does not support > or breaks the HAB feature. 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. 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. 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. Kind regards, Rouven > > > > > > > > > > IMO this should be handled correctly by the bootloader and/or OP- > > > > TEE. The default upstream configuration for OP-TEE is to not use the > > > > CAAM at runtime as well, since linux runtime PM disablement of the CAAM > > > > will lock up OP-TEE when it tries to access the CAAM. > > > > > > If by handling you mean releasing JR0 reservation - then yes, it should be > > > done by either SPL or TF-A as they do run in S World. In such a case, DTB > > > bindings need to be adapted further according to the new state. Until this > > > done - this patch does provide a correct state of HW to the Kernel. > > > > Upstream TF-A simply releases all JRs to the normal world, matching the > > current DTB description. > > > > Kind Regards, > > Rouven Czerwinski > > > > > [snip] > > Regards, > Andrey > > Link: [1]: https://lore.kernel.org/u-boot/VI1PR04MB5342C8C6E651EC2CC4477EB5E79A9@VI1PR04MB5342.eurprd04.prod.outlook.com/ > Link: [2]: https://source.denx.de/u-boot/u-boot/-/blob/master/doc/board/nxp/imx8mm_evk.rst
Am 2022-01-07 12:58, schrieb Lucas Stach: > Am Freitag, dem 07.01.2022 um 12:47 +0100 schrieb Michael Walle: >> Hi Rouven, >> >> Am 2022-01-07 10:46, schrieb Rouven Czerwinski: >> > .. since AFAIK i.MX8M* can not be run without TF-A. >> >> Are you sure? There probably aren't any boards out there >> without TF-A, but why shouldn't it work without it? > > PSCI, i.e. the only means to start the secondary CPUs, is implemented > in TF-A, so it's very unlikely that anyone would want to run a system > without TF-A. Also quite a bit of the lowlevel SoC initialization is > implemented in TF-A. Doesn't mean u-boot cannot implement PSCI; actually you doesn't need it at all, you can still use spin tables. I just keep hearing the same arguments for the LS1028A SoC and yet there is one board without TF-A ;) Anyway, I admit it's rather unlikely. -michael
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(+)