[EXT] RE: [PATCH v10 02/14] i.MX8M: crypto: updated device tree for supporting DM in SPL

[ZHIZHIKIN Andrey]

Hello Gaurav,

Sorry, I somehow missed you last reply here.

Cc: Rouven

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Gaurav Jain
> Sent: Friday, February 11, 2022 10:48 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; u-
> boot at lists.denx.de
> Cc: Stefano Babic <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Michael Walle
> <michael at walle.cc>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>;
> Horia Geanta <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> <franck.lenormand at nxp.com>; Silvano Di Ninno <silvano.dininno at nxp.com>; Sahil
> Malhotra <sahil.malhotra at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>; Varun
> Sethi <V.Sethi at nxp.com>; dl-uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu
> <shengzhou.liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat
> <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim
> Khan <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod Kumar
> <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>; Adrian Alonso
> <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: RE: [EXT] RE: [PATCH v10 02/14] i.MX8M: crypto: updated device tree for
> supporting DM in SPL
> 
> Hello Andrey
> 
> I responded to your comments on this patch.
> If you are good, can this patch series merged?
> 
> Regards
> Gaurav Jain
> 
> > -----Original Message-----
> > From: Gaurav Jain
> > Sent: Thursday, February 3, 2022 10:58 AM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; u-
> > boot at lists.denx.de
> > Cc: Stefano Babic <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>;
> > Peng Fan <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Michael
> > Walle <michael at walle.cc>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> > <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> > <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>; Silvano Di
> > Ninno <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> > Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-
> > uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>;
> > Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> > Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> > <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> > Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> > Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> > Subject: RE: [EXT] RE: [PATCH v10 02/14] i.MX8M: crypto: updated device tree
> > for supporting DM in SPL
> >
> > Hello Andrey
> >
> > > -----Original Message-----
> > > From: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> > > Sent: Tuesday, February 1, 2022 3:15 AM
> > > To: Gaurav Jain <gaurav.jain at nxp.com>; u-boot at lists.denx.de
> > > Cc: Stefano Babic <sbabic at denx.de>; Fabio Estevam
> > > <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>; Simon Glass
> > > <sjg at chromium.org>; Michael Walle <michael at walle.cc>; Priyanka Jain
> > > <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> > > <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> > > <franck.lenormand at nxp.com>; Silvano Di Ninno
> > > <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> > > Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>;
> > > dl- uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu
> > > <shengzhou.liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>; Rajesh
> > > Bhagat <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> > > <meenakshi.aggarwal at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> > Alison
> > > Wang <alison.wang at nxp.com>; Pramod Kumar
> > <pramod.kumar_1 at nxp.com>;
> > > Andy Tang <andy.tang at nxp.com>; Adrian Alonso <adrian.alonso at nxp.com>;
> > > Vladimir Oltean <olteanv at gmail.com>
> > > Subject: [EXT] RE: [PATCH v10 02/14] i.MX8M: crypto: updated device
> > > tree for supporting DM in SPL
> > >
> > > Caution: EXT Email
> > >
> > > Hello Gaurav,
> > >
> > > > -----Original Message-----
> > > > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Gaurav Jain
> > > > Sent: Wednesday, January 12, 2022 2:31 PM
> > > > To: u-boot at lists.denx.de
> > > > Cc: Stefano Babic <sbabic at denx.de>; Fabio Estevam
> > > > <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>; Simon Glass
> > > > <sjg at chromium.org>; Michael Walle <michael at walle.cc>; Priyanka Jain
> > > > <priyanka.jain at nxp.com>; Ye Li <ye.li at nxp.com>; Horia Geanta
> > > > <horia.geanta at nxp.com>; Ji Luo <ji.luo at nxp.com>; Franck Lenormand
> > > > <franck.lenormand at nxp.com>; Silvano Di Ninno
> > > > <silvano.dininno at nxp.com>; Sahil malhotra <sahil.malhotra at nxp.com>;
> > > > Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>;
> > > > NXP i . MX U-Boot Team <uboot-imx at nxp.com>; Shengzhou Liu
> > > > <Shengzhou.Liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>; Rajesh
> > > > Bhagat <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> > > > <meenakshi.aggarwal at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> > > Alison
> > > > Wang <alison.wang at nxp.com>; Pramod Kumar
> > > <pramod.kumar_1 at nxp.com>;
> > > > Tang Yuantian <andy.tang at nxp.com>; Adrian Alonso
> > > > <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>; Gaurav
> > > > Jain <gaurav.jain at nxp.com>
> > > > Subject: [PATCH v10 02/14] i.MX8M: crypto: updated device tree for
> > > > supporting DM in SPL
> > > >
> > > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for secure
> > > > boot.
> > >
> > > I'd like to return the original question here, which was not
> > > completely clarified during previous reviews: where does the reservation
> > restriction is coming from?
> >
> > As mentioned earlier JR0 is being used by high assurance boot (HAB). This
> > prevents JR0 being used by any other software layer.

From what I understood reading AN4581 and companion AN12056, HAB requires
JR only in case when Encrypted Boot is performed. For image signature
verification it is not required, hence this reservation does sound a bit
artificial to me. Please correct me here if my understanding is not complete,
I was not able to get this information from all the answers you provided
before on the purpose of this reservation.

This fact, coupled with the one that upstream TF-A does release all JR to
NS-World, suggests that the reservation is not actually necessary, and
moreover - it will not reflect what the HW configuration it will have in
all combinations of BootROM->SPL->TF-A->(potentially OP-TEE)->Kernel.

Please note, that in case of downstream NXP TF-A is used here - this
reservation is rather valid, but it leads to "vendor locking" of
upstream U-Boot as it cannot be expected that all board/device vendors
are using downstream TF-A.

You do suggest below that this TF-A "mismatch" will be addressed in
upstream, but until this is done it is not reasonable to imply this
reservation IMHO.

As a side note: I had a patch series in the Kernel that performs
exactly the same reservation, but had to hold it back exactly because
of the reason I stated above: upstream TF-A does behave different from
downstream NXP one, hence booting Kernel with either one of those would
have different results with respect to JR0 reservation, see [1]. So you
can say that I'm and "interested party" here, since depending on how this
is solved in the boot chain would influence my next version of the patch
set to the Kernel.

> > >
> > > BootROM does reserve the JR0 and JR1, which are later released by ATF.
> > > NXP downstream ATF keeps the JR0 reserved, but upstream ATF does
> > > release *all* JRs to NS World.
> > >
> > > If this reservation is taken like the patch proposes and U-Boot is
> > > built with upstream ATF - this would eventually lead to the situation
> > > where the HW configuration is not aligned with what DTB indicates.
> >
> > I agree with you, we will address this in the upstream ATF code base as well.

That would be really great! I would personally consider this as a first
step before at least this reservation is performed. This would ensure the
consistency of HW description that DT provides.

> > >
> > > Please note, that recent OP-TEE release has also re-mapped the JR it
> > > uses from
> > > JR0 to JR2, which can also lead to usage of the JR which is already
> > > taken by OP- TEE. There is an ongoing PR in OP-TEE to disable JR nodes
> > > via DT overlay for Linux [1], but I'm not sure if the same applies to U-Boot
> as
> > well.
> >
> > Yes, Sahil would be addressing this issue in the next version of  his patch
> set.

Michael had a share of comments regarding the approach chosen for LS family,
and I think once that is solved - same solution can be applied to imx8 family.

> >
> > Regards
> > Gaurav Jain
> > >

Link: [1]: https://lore.kernel.org/lkml/AM6PR06MB4691D4FCA4D82284E70A9F53A64E9@AM6PR06MB4691.eurprd06.prod.outlook.com/

-- andrey