| Message ID | 1516741853-12458-1-git-send-email-bryan.odonoghue@linaro.org |
|---|---|
| Headers | show |
| Series | Fix CAAM for TrustZone enable for warp7 | expand |
On Tue, 2018-01-23 at 21:10 +0000, Bryan O'Donoghue wrote: > 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. > I did have the same issue, not with booting OPTEE, but booting Linux in non-secure mode. #2 and #3 are required to handle this problem, but #1 is not needed. The CAAM kernel driver always tries to instantiate all RNG state handles directly using DEC0 (which is only accessible from secure mode). The u-boot driver however only instantiates the first state handle, which is why the kernel driver then goes on and tries to instantiate the second one. This is solved by simply instantiating all RNG state handles in the CAAM u-boot driver. I can prepare a patch to implement this, if there is interest. This is tested to work with the mainline kernel and I assume that it would work with the NXP kernel as well. Further patches are however needed to support the imx7 in the CAAM kernel driver. > 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. This is actually quite simple to solve, since each job ring has a separate device tree node. Simply disabling all job rings used by OPTEE / secure world software should be sufficient. Thanks, Lukas > [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 | 1 + > 3 files changed, 15 insertions(+), 1 deletion(-) >
On 23 January 2018 at 21:10, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > 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 > This series: Tested-by: Ryan Harkin <ryan.harkin@linaro.org> > > board/warp7/warp7.c | 6 +++++- > drivers/crypto/fsl/jr.c | 9 +++++++++ > drivers/crypto/fsl/jr.h | 1 + > 3 files changed, 15 insertions(+), 1 deletion(-) > > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
On 24/01/18 12:52, Auer, Lukas wrote: > On Tue, 2018-01-23 at 21:10 +0000, Bryan O'Donoghue wrote: >> 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. >> > > I did have the same issue, not with booting OPTEE, but booting Linux in > non-secure mode. #2 and #3 are required to handle this problem, but #1 > is not needed. > > The CAAM kernel driver always tries to instantiate all RNG state > handles directly using DEC0 (which is only accessible from secure > mode). The u-boot driver however only instantiates the first state > handle, which is why the kernel driver then goes on and tries to > instantiate the second one. This is solved by simply instantiating all > RNG state handles in the CAAM u-boot driver. > > I can prepare a patch to implement this, if there is interest. This is > tested to work with the mainline kernel and I assume that it would work > with the NXP kernel as well. Further patches are however needed to > support the imx7 in the CAAM kernel driver. So I just sent out a patch-set for i.MX7 covering this - I added you to the CC list for that. The guidance from NXP (and my experience) is that the current kernel driver bugs-out when it can't touch the deco registers. I have a bunch of fixes around that. > >> 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. > > This is actually quite simple to solve, since each job ring has a > separate device tree node. Simply disabling all job rings used by OPTEE > / secure world software should be sufficient. Yes I agree. Then again there is currently no CAAM/OPTEE driver so nothing to-do.
On Wed, 2018-01-24 at 14:35 +0000, Bryan O'Donoghue wrote: > > On 24/01/18 12:52, Auer, Lukas wrote: > > On Tue, 2018-01-23 at 21:10 +0000, Bryan O'Donoghue wrote: > > > 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. > > > > > > > I did have the same issue, not with booting OPTEE, but booting > > Linux in > > non-secure mode. #2 and #3 are required to handle this problem, but > > #1 > > is not needed. > > > > The CAAM kernel driver always tries to instantiate all RNG state > > handles directly using DEC0 (which is only accessible from secure > > mode). The u-boot driver however only instantiates the first state > > handle, which is why the kernel driver then goes on and tries to > > instantiate the second one. This is solved by simply instantiating > > all > > RNG state handles in the CAAM u-boot driver. > > > > I can prepare a patch to implement this, if there is interest. This > > is > > tested to work with the mainline kernel and I assume that it would > > work > > with the NXP kernel as well. Further patches are however needed to > > support the imx7 in the CAAM kernel driver. > > So I just sent out a patch-set for i.MX7 covering this - I added you > to > the CC list for that. > > The guidance from NXP (and my experience) is that the current kernel > driver bugs-out when it can't touch the deco registers. > > I have a bunch of fixes around that. Thanks for adding me to the CC list. I have experienced the same thing regarding the dec0 registers. However, I don't understand why you want to detect secure mode in the kernel driver to skip RNG instantiation instead of instantiating all RNG state handles in the u-boot driver. This way, the kernel driver would skip both state handles (instead of just the first one as with the current u-boot driver) in the probe call. The dec0 registers would therefore never be used. In addition, skipping RNG instantiation in the kernel driver means that the second state handle never gets instantiated. > > > > > 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. > > > > This is actually quite simple to solve, since each job ring has a > > separate device tree node. Simply disabling all job rings used by > > OPTEE > > / secure world software should be sufficient. > > Yes I agree. Then again there is currently no CAAM/OPTEE driver so > nothing to-do.
On 24/01/18 17:41, Auer, Lukas wrote: > Thanks for adding me to the CC list. > I have experienced the same thing regarding the dec0 registers. > However, I don't understand why you want to detect secure mode in the > kernel driver to skip RNG instantiation instead of instantiating all > RNG state handles in the u-boot driver. That's what we are doing though. This set instantiates everything in u-boot and then detects and skips in the kernel if-and-only if 1. Trust zone is detected 2. It looks to the Linux CAAM driver as if u-boot has initialised the h/w For #2 I actually have to variants 1. Which passes a DT parameter which indicates the kernel should skip RNG init 2. A module parameter which indicates the kernel should skip rng init Could we discuss the kernel changes in the kernel thread ? I believe we agree the u-boot side is right ?
On Wed, 2018-01-24 at 19:41 +0000, Bryan O'Donoghue wrote: > > On 24/01/18 17:41, Auer, Lukas wrote: > > Thanks for adding me to the CC list. > > I have experienced the same thing regarding the dec0 registers. > > However, I don't understand why you want to detect secure mode in > > the > > kernel driver to skip RNG instantiation instead of instantiating > > all > > RNG state handles in the u-boot driver. > > That's what we are doing though. > > This set instantiates everything in u-boot and then detects and skips > in > the kernel if-and-only if > > 1. Trust zone is detected > 2. It looks to the Linux CAAM driver as if u-boot has initialised the > h/w > > For #2 I actually have to variants > > 1. Which passes a DT parameter which indicates the kernel should > skip > RNG init > > 2. A module parameter which indicates the kernel should skip rng init > > Could we discuss the kernel changes in the kernel thread ? > > I believe we agree the u-boot side is right ? Sorry, I haven't explained what I mean very well. You are right in that sec_init() must be called to instantiate the RNG, however the CAAM u-boot driver only partially does so. If you look at function instantiate_rng() in both u-boot (drivers/crypto/fsl/jr.c) and the kernel (drivers/crypto/caam/ctrl.c), you'll see that the kernel loops over all available state handles whereas u-boot does not. Fixing this in u-boot should mean that you can drop patch 5 and 6 from your kernel series since the kernel should then skip over all state handles. I can send out a patch later today to fix this on the u-boot side. Thanks, Lukas
On 25/01/18 09:14, Auer, Lukas wrote: > On Wed, 2018-01-24 at 19:41 +0000, Bryan O'Donoghue wrote: >> >> On 24/01/18 17:41, Auer, Lukas wrote: >>> Thanks for adding me to the CC list. >>> I have experienced the same thing regarding the dec0 registers. >>> However, I don't understand why you want to detect secure mode in >>> the >>> kernel driver to skip RNG instantiation instead of instantiating >>> all >>> RNG state handles in the u-boot driver. >> >> That's what we are doing though. >> >> This set instantiates everything in u-boot and then detects and skips >> in >> the kernel if-and-only if >> >> 1. Trust zone is detected >> 2. It looks to the Linux CAAM driver as if u-boot has initialised the >> h/w >> >> For #2 I actually have to variants >> >> 1. Which passes a DT parameter which indicates the kernel should >> skip >> RNG init >> >> 2. A module parameter which indicates the kernel should skip rng init >> >> Could we discuss the kernel changes in the kernel thread ? >> >> I believe we agree the u-boot side is right ? > > Sorry, I haven't explained what I mean very well. > > You are right in that sec_init() must be called to instantiate the RNG, > however the CAAM u-boot driver only partially does so. If you look at > function instantiate_rng() in both u-boot (drivers/crypto/fsl/jr.c) and > the kernel (drivers/crypto/caam/ctrl.c), you'll see that the kernel > loops over all available state handles whereas u-boot does not. > > Fixing this in u-boot should mean that you can drop patch 5 and 6 from > your kernel series since the kernel should then skip over all state > handles. Are you sure about that ? It looks to me as if we will hit this block of code fairly decisively without #5 and #6 clrsetbits_32(&ctrl->deco_rq, 0, DECORR_RQD0ENABLE); while (!(rd_reg32(&ctrl->deco_rq) & DECORR_DEN0) && --timeout) cpu_relax(); if (!timeout) { dev_err(ctrldev, "failed to acquire DECO 0\n"); clrsetbits_32(&ctrl->deco_rq, DECORR_RQD0ENABLE, 0); return -ENODEV; } ... but again let's discuss that in the kernel thread. > I can send out a patch later today to fix this on the u-boot side. I'll certainly try out your patch on top of these patches. > Thanks, > Lukas >
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 | 1 + 3 files changed, 15 insertions(+), 1 deletion(-)