Hello Andrey
-----Original Message----- From: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com Sent: Monday, November 22, 2021 10:51 PM To: Gaurav Jain gaurav.jain@nxp.com; u-boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com; Shengzhou Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com; Pramod Kumar pramod.kumar_1@nxp.com; Andy Tang andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com; Michael Walle michael@walle.cc Subject: RE: [EXT] RE: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
Caution: EXT Email
Hello Gaurav,
-----Original Message----- From: Gaurav Jain gaurav.jain@nxp.com Sent: Monday, November 22, 2021 8:29 AM To: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com; u- boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com; Shengzhou Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com;
Alison
Wang alison.wang@nxp.com; Pramod Kumar
Andy Tang andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com Subject: RE: [EXT] RE: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
Hello Andrey
-----Original Message----- From: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com Sent: Wednesday, November 17, 2021 6:33 PM To: Gaurav Jain gaurav.jain@nxp.com; u-boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com; Shengzhou Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com; Pramod Kumar pramod.kumar_1@nxp.com; Andy Tang andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com Subject: RE: [EXT] RE: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
Caution: EXT Email
Hello Gaurav,
-----Original Message----- From: Gaurav Jain gaurav.jain@nxp.com Sent: Wednesday, November 17, 2021 12:26 PM To: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com; u- boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com;
Silvano
Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com;
Varun
Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com;
Shengzhou
Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com;
Rajesh
Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com;
Alison
Wang alison.wang@nxp.com; Pramod Kumar
Andy Tang andy.tang@nxp.com; Adrian Alonso
Vladimir Oltean olteanv@gmail.com Subject: RE: [EXT] RE: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
Hello Andrey
-----Original Message----- From: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com Sent: Tuesday, November 16, 2021 9:24 PM To: Gaurav Jain gaurav.jain@nxp.com; u-boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com;
Varun
Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com;
Shengzhou
Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com;
Rajesh
Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com; Pramod Kumar pramod.kumar_1@nxp.com; Andy Tang andy.tang@nxp.com;
Adrian
Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com Subject: [EXT] RE: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
Caution: EXT Email
Hello Gaurav,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Gaurav Jain Sent: Monday, November 15, 2021 8:00 AM To: u-boot@lists.denx.de Cc: Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; NXP i . MX U-Boot Team uboot-imx@nxp.com; Shengzhou Liu Shengzhou.Liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com;
Meenakshi
Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com;
Pramod
Kumar
pramod.kumar_1@nxp.com; Tang Yuantian andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com; Gaurav Jain gaurav.jain@nxp.com Subject: [PATCH v5 01/16] crypto/fsl: Add support for CAAM Job ring driver model
added device tree support for job ring driver. sec is initialized based on job ring information processed from device tree.
Signed-off-by: Gaurav Jain gaurav.jain@nxp.com Reviewed-by: Ye Li ye.li@nxp.com
cmd/Kconfig | 1 + drivers/crypto/fsl/Kconfig | 7 + drivers/crypto/fsl/Makefile | 4 +- drivers/crypto/fsl/jr.c | 316 +++++++++++++++++++++++------------- drivers/crypto/fsl/jr.h | 14 ++ 5 files changed, 232 insertions(+), 110 deletions(-)
[snip]
sec_out32(&sec->mcfgr, mcr);+#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_IMX8M)
This would effectively reserve the JR0 on _all_ i.MX8M derivatives is S
World.
This code is to set any JR DID in SPL so that the job ring can be configured.
Current implementation only has JR0 reserved in S World on imx8mm derivative, but this new addition extends this to imx8mn, imx8mp and
imx8mq.
Current implementation do not initialize CAAM for i.MX8M derivatives. It is not based on driver model approach and only using JR0.
OK, but then I do not have on explanation on why do I see following results from reading JRaDID_MS registers on imx8m derivatives:
- imx8mm: JR0DID_MS = 0x8011 JR1DID_MS = 0x0 JR2DID_MS = 0x0
- imx8mn: JR0DID_MS = 0x0 JR1DID_MS = 0x0 JR2DID_MS = 0x0
- imx8mp: JR0DID_MS = 0x0 JR1DID_MS = 0x0 JR2DID_MS = 0x0
This readout is taken at Kernel boot, and it clearly shows that only JR0 has TZ_OWN, PRIM_TZ and PRIM_DID bits set, and it is only done on
imx8mm.
HAB is a code that is part of the ROM code which set the JR DID for all i.mx8M. I took the dumps on SPL boot which actually shows the JR DID set by HAB. Dump taken by you on kernel boot does not show the values set by ROM. IMX8MM JR0DID_MS = 0x8011 JR1DID_MS = 0x8011 JR2DID_MS = 0x0
IMX8MN JR0DID_MS = 0x8011 JR1DID_MS = 0x8011 JR2DID_MS = 0x0
IMX8MP JR0DID_MS = 0x8011 JR1DID_MS = 0x8011 JR2DID_MS = 0x0
This is an interesting piece of information, thanks a lot for the readout! So it does look like that BootROM on all derivatives reserves JR0 and JR1 at the beginning, letting the ATF to release only JR1 to NS world...
Does IMX8MQ have the same reservation as well?
With New implementation CAAM is enabled for i.MX8M derivative. Any JR whose DID is written in ATF, can be used in Uboot. JR0 is reserved for HAB so JR1 will be used for all i.MX8M derivatives.
I'm wondering about several points here:
- Why does current implementation on have this reservation done
on imx8mm and where does this happen? None of the code pieces suggests that it is
done in
U-Boot, is it performed in BootROM?
I cannot see if current implementation(SPL/Uboot) has reservation done for imx8mm. In ATF, we are reserving the JR0.
I was not able to identify which part of ATF code is responsible to program JR0DID_MS on imx8mm, the only thing I saw was the part where the JR0 is held in S World *if* the JR0DID_MS is set to 0x8011. Can you point out where is this performed in ATF code?
If it is not in the ATF, then my question above still stands: which component (HW or SW) programs JR0DID_MS, and why is it only done on imx8mm derivative?
HAB which is part of the ROM code sets the JR DID for all i.mx8M.
- What is the intention of having JR0 reserved for all
derivatives? Is
this
the part of a bigger change that stretches across different SW
components
(e.g. ATF, OP-TEE, etc.)? If that is the case - then a more detailed description would be appreciated here.
ATF code already accounts for this reservation in commit: a83a7c65e ("TEE-639 plat: imx8m: Do not release JR0 to NS if HAB is using it") [1], but there is no description on why is this required though.
If this is required for HAB feature, then the question is: should it be kept in
S
World when U-Boot starts, or SPL can release it after the binary is verified
and
crypto facilities are not in use anymore?
Commit: a83a7c65e reserves JR0 for HAB and not released to NS but JR1, JR2 are released to NS.
Then I believe this change should be in-sync with ATF implementation, because of the fact that your change can have any arbitrary JR to be held in S World.
What would happen if for example JR1 is programmed with TZ_OWN, but ATF releases it to NS world? Can it be used by Kernel afterwards? Or should the node be disabled here so that Kernel does not even see JR1 during
boot?
Since JR0 is marked as disabled in DT, so SPL is only accessing single job ring and setting the JR1 DID as 0x8011. After SPL boots successfully, ATF is releasing JR1 and JR2 to NS by modifying the JRDID_MS as 0x1. Uboot is also accessing single jobring which is JR1. JR0 is only reserved for secure boot.
Is it safe to assume that JR1 is then accessible from both S and NS Worlds?
If that is the case, then that would actually mean that JRx status on DT should be set as following:
&sec_jr0 { status = "disabled"; secure-status = "okay"; };
&sec_jr1 { secure-status = "okay"; };
&sec_jr2 { secure-status = "disabled"; };
This would effectively mean: JR0 - S-only, JR1 - visible in both JR2 - NS-only
Please note, that as this configuration is applicable to both Kernel and U-Boot - the above block should be defined in Kernel DT for all i.MX8M derivatives, and picked up with the next U-Boot DTB re-sync.
As I'm working on V3 for CAAM clean-up in the Kernel [1] - I can submit those configuration changes, but I would need a confirmation from you if this is an expected JR configuration, and whether IMX8MQ have the same setup.
IMX8MQ has same values. JR0DID_MS = 0x8011 JR1DID_MS = 0x8011 JR2DID_MS = 0x0
For now we are only reserving JR0 for secure boot. JR1 DID is later modified in ATF to 0x1. JR2 can be used by OPTEE which is secure and can set the DID before accessing the JR2. Setting secure-status as disabled for JR2 could break OPTEE. "secure-status" property is not used in uboot CAAM driver code so how this is going to affect the caam driver working in SPL/Uboot? I am not sure about the kernel caam driver how secure-status is processed. For kernel JR configuration I cannot confirm. I would suggest to take the opinion from kernel caam maintainers as well.
So far, ATF only examines the JR0DID_MS content, and not all the others...
HAB uses JR0 for secure boot on all i.MX8M derivatives. Uboot calls HAB API for authenticating kernel.
This implies then that the JR0 is permanently held in S World and stays there for entire device powercycle and cannot be reclaimed in NS
World?
Yes JR0 is held in S world.
In this
case, the DT node should be completely removed from DTB file so no SW entity can even see it (as it is in a total possession of HW mechanisms).
We can consider this change after this patch series is merged. Currently I have disabled the JR0 in device tree.
I guess with the proposed DT configuration this point would be covered as well, isn't it? There would be no need to remove the node, as it would be marked disabled in NS and enabled in S Worlds. I believe it is better to set the status as I proposed, because that information in DT is transparent for everyone (removing node raises questions regarding HW availability to me).
CAAM driver is used in spl, atf, optee, uboot, kernel. Spl and uboot can work with JR1 only. For other components it will be good to have their opinion.
jrdid_ms = JRDID_MS_TZ_OWN | JRDID_MS_PRIM_TZ |- JRDID_MS_PRIM_DID;
What is the intention of setting JRDID_MS_PRIM_TZ? Isn't setting JRDID_MS_TZ_OWN would be sufficient here?
PRIM_TZ bit is set to 1 to indicate that only SecureWorld can access registers in that Job Ring's register page
But would it not be enough just to set TZ_OWN? If I read SRM correct: only TZ_OWN is enough to hold the JR in S World.
HAB is also setting 0x8011 as JR DID. It is better to be in sync with HAB.
Do you know what is the reason for HAB to set PRIM_TZ bit? Is there any specific reason for this?
To restrict JR register page access to Secure World, PRIM_TZ bit is set. So later in ATF we can decide which JobRing to release to NS.
Regards Gaurav Jain
Regards Gaurav Jain
[snip]
-- andrey
-- andrey
Link: [1]: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern el.org%2Flkml%2F20211111164601.13135-1-andrey.zhizhikin%40leica- geosystems.com%2F&data=04%7C01%7Cgaurav.jain%40nxp.com%7C2266 10fc0dd44d2324b408d9addc6523%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 C0%7C0%7C637731984370210324%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a mp;sdata=SMTu0Nn0SCYFQ0H6IxLo%2F9p4AkbG%2FS1E%2BD7ojMx52WQ%3D &reserved=0