| Message ID | 20190423101948.24898-3-bryan.odonoghue@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | imx: Implement job-ring context switching | expand |
Hi Bryan, Em ter, 23 de abr de 2019 às 07:20, Bryan O'Donoghue <bryan.odonoghue@linaro.org> escreveu: > > Use __sec_set_jr_context_normal() to set job-ring ownership rather than the > current in-line array walk. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/crypto/fsl/jr.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c > index 7b13aa4a61..65982b8369 100644 > --- a/drivers/crypto/fsl/jr.c > +++ b/drivers/crypto/fsl/jr.c > @@ -616,7 +616,6 @@ 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; We may also need to remove this variable otherwise we get build warning below: drivers/crypto/fsl/jr.c: In function 'sec_init_idx': drivers/crypto/fsl/jr.c:625:6: warning: unused variable 'i' [-Wunused-variable] int i; ^ Thanks for submitting this patch set. I couldn't get encrypted boot working in my first attempt, doing the exact same procedure with commit 22191ac35344 ("drivers/crypto/fsl: assign job-rings to non-TrustZone") reverted works fine. I will take a better look in your patch set and let you know if I find something. Best Regards, Breno Matheus Lima
On 25/04/2019 04:24, Breno Matheus Lima wrote: > I couldn't get encrypted boot working in my first attempt, doing the > exact same procedure with commit 22191ac35344 ("drivers/crypto/fsl: > assign job-rings to non-TrustZone") reverted works fine. Hi Breno, I noticed another patch from you re: dek blob, does that address this issue for you are is this still a live thing ? If you are running in secure-world, and the BootROM dek blob stuff validates job-ring ownership it _should_ be possible to flip the ownership bits to what the BootROM expects and then back again. If its not working, presumably its because we aren't flipping ownership at the right time. Maybe better to set permissions to secure-world while we are in u-boot and then switch to normal world before we hand over to the next boot phase. --- bod
On 30/04/2019 02:28, Bryan O'Donoghue wrote: > > > On 25/04/2019 04:24, Breno Matheus Lima wrote: >> I couldn't get encrypted boot working in my first attempt, doing the >> exact same procedure with commit 22191ac35344 ("drivers/crypto/fsl: >> assign job-rings to non-TrustZone") reverted works fine. > > Hi Breno, > > I noticed another patch from you re: dek blob, does that address this > issue for you are is this still a live thing ? > > If you are running in secure-world, and the BootROM dek blob stuff > validates job-ring ownership it _should_ be possible to flip the > ownership bits to what the BootROM expects and then back again. > > If its not working, presumably its because we aren't flipping ownership > at the right time. It occurred to me after I went to bed. The right thing to do is leave the BootROM settings up until we hand-off and then set the required post-boot settings. Something I reckon can be ~easily done in some sort of architectural handover preparation function. I'll spin that patchset. --- bod
Hi Bryan, Em ter, 30 de abr de 2019 às 05:13, Bryan O'Donoghue <bryan.odonoghue@linaro.org> escreveu: > > > > On 30/04/2019 02:28, Bryan O'Donoghue wrote: > > > > > > On 25/04/2019 04:24, Breno Matheus Lima wrote: > >> I couldn't get encrypted boot working in my first attempt, doing the > >> exact same procedure with commit 22191ac35344 ("drivers/crypto/fsl: > >> assign job-rings to non-TrustZone") reverted works fine. > > > > Hi Breno, > > > > I noticed another patch from you re: dek blob, does that address this > > issue for you are is this still a live thing ? No, the patch I have recently submitted does not address the JR TrustZone issue we are currently seeing with DEK blob decapsulation at ROM level. I was not following AN12056 when I tried so I couldn't see this other issue at first moment. > > > > If you are running in secure-world, and the BootROM dek blob stuff > > validates job-ring ownership it _should_ be possible to flip the > > ownership bits to what the BootROM expects and then back again. > > > > If its not working, presumably its because we aren't flipping ownership > > at the right time. > > It occurred to me after I went to bed. > > The right thing to do is leave the BootROM settings up until we hand-off > and then set the required post-boot settings. > > Something I reckon can be ~easily done in some sort of architectural > handover preparation function. > > I'll spin that patchset. Thanks for preparing a second version for this patchset, I see that you have also replied to my other e-mail in "[PATCH 1/4] crypto/fsl: Introduce API to save/restore job-ring context". Your new proposal looks fine to me, I can test again. Thanks, Breno Lima
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index 7b13aa4a61..65982b8369 100644 --- a/drivers/crypto/fsl/jr.c +++ b/drivers/crypto/fsl/jr.c @@ -616,7 +616,6 @@ 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; @@ -674,11 +673,7 @@ int sec_init_idx(uint8_t sec_idx) #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); - } + __sec_set_jr_context_normal(sec_idx); ret = jr_init(sec_idx); if (ret < 0) {
Use __sec_set_jr_context_normal() to set job-ring ownership rather than the current in-line array walk. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/crypto/fsl/jr.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)