[EXT] Re: [PATCH v4 03/16] i.MX8M: crypto: updated device tree for supporting DM in SPL
ZHIZHIKIN Andrey
andrey.zhizhikin at leica-geosystems.com
Mon Nov 8 09:48:19 CET 2021
Hello Gurav,
I'll Cc: Michael Walle here for the reason described below.
> -----Original Message-----
> From: Gaurav Jain <gaurav.jain at nxp.com>
> Sent: Monday, November 8, 2021 9:21 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; Adam Ford
> <aford173 at gmail.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; 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>; 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 v4 03/16] 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: Wednesday, November 3, 2021 5:05 PM
> > To: Adam Ford <aford173 at gmail.com>; Gaurav Jain <gaurav.jain at nxp.com>
> > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; 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>; 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 v4 03/16] i.MX8M: crypto: updated device
> > tree for supporting DM in SPL
> >
> > Caution: EXT Email
> >
> > Hello Gurav/Adam ,
> >
> > > -----Original Message-----
> > > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Adam Ford
> > > Sent: Tuesday, November 2, 2021 12:19 PM
> > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; 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>; 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 v4 03/16] i.MX8M: crypto: updated
> > > device tree for supporting DM in SPL
> > >
> > >
> > > On Tue, Nov 2, 2021 at 3:17 AM Gaurav Jain <gaurav.jain at nxp.com> wrote:
> > > >
> > > > Hello Adam
> > > >
> > > > > -----Original Message-----
> > > > > From: Adam Ford <aford173 at gmail.com>
> > > > > Sent: Monday, November 1, 2021 6:30 PM
> > > > > To: Gaurav Jain <gaurav.jain at nxp.com>
> > > > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; 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>; 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 v4 03/16] i.MX8M: crypto: updated
> > > > > device tree for supporting DM in SPL
> > > > >
> > > > > Caution: EXT Email
> > > > >
> > > > > On Tue, Oct 26, 2021 at 1:57 AM Gaurav Jain
> > > > > <gaurav.jain at nxp.com>
> > wrote:
> > > > > >
> > > > > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for
> > > > > > secure boot.
> > > > > >
> > > > > > Signed-off-by: Gaurav Jain <gaurav.jain at nxp.com>
> > > > > > Reviewed-by: Ye Li <ye.li at nxp.com>
> > > > > > ---
> > > > > > arch/arm/dts/imx8mm-evk-u-boot.dtsi | 18 +++++++++++++++++-
> > > > > > arch/arm/dts/imx8mm.dtsi | 1 +
> > > > > > arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 18
> +++++++++++++++++-
> > > > > > arch/arm/dts/imx8mn.dtsi | 1 +
> > > > > > arch/arm/dts/imx8mp-evk-u-boot.dtsi | 18 +++++++++++++++++-
> > > > > > arch/arm/dts/imx8mp.dtsi | 1 +
> > > > > > arch/arm/dts/imx8mq.dtsi | 1 +
> > > > > > 7 files changed, 55 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > > b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > > index 3c75415e8f..40f5cfda9a 100644
> > > > > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > > > @@ -1,6 +1,6 @@
> > > > > > // SPDX-License-Identifier: GPL-2.0+
> > > > > > /*
> > > > > > - * Copyright 2019 NXP
> > > > > > + * Copyright 2019, 2021 NXP
> > > > > > */
> > > > > >
> > > > > > #include "imx8mm-u-boot.dtsi"
> > > > > > @@ -72,6 +72,22 @@
> > > > > > u-boot,dm-spl;
> > > > > > };
> > > > > >
> > > > > > +&crypto {
> > > > > > + u-boot,dm-spl;
> > > > > > +};
> > > > > > +
> > > > > > +&sec_jr0 {
> > > > > > + u-boot,dm-spl;
> > > > > > +};
> > > > > > +
> > > > > > +&sec_jr1 {
> > > > > > + u-boot,dm-spl;
> > > > > > +};
> > > > > > +
> > > > > > +&sec_jr2 {
> > > > > > + u-boot,dm-spl;
> > > > > > +};
> > > > > > +
> > > > > > &usdhc1 {
> > > > > > u-boot,dm-spl;
> > > > > > };
> > > > > > diff --git a/arch/arm/dts/imx8mm.dtsi
> > > > > > b/arch/arm/dts/imx8mm.dtsi index b142b80734..009999bf3a 100644
> > > > > > --- a/arch/arm/dts/imx8mm.dtsi
> > > > > > +++ b/arch/arm/dts/imx8mm.dtsi
> > > > > > @@ -824,6 +824,7 @@
> > > > > > compatible = "fsl,sec-v4.0-job-ring";
> > > > > > reg = <0x1000 0x1000>;
> > > > > > interrupts = <GIC_SPI
> > > > > > 105 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > + status = "disabled";
> > > > >
> > > > > Changing the SoC DTSI files makes future re-synchronizing difficult.
> > > > > If you mark these as disabled in the respective u-boot.dtsi
> > > > > files, it will create less work in the future. You're already
> > > > > enabling a bunch of new features in the respective -u-boot.dtsi
> > > > > files, I would suggest
> > > doing the disable in the same way.
> > > >
> > > > JR0 status is marked as disabled, as it is reserved in ATF and
> > > > cannot be accessed
> > > in SPL and Uboot.
> > > > For JR driver model(new feature) to work, at least one Jobring has
> > > > to be
> > > initialized, which is not possible with JR0.
> > > > JR1 will be initialized.
> > >
> > > I wasn't suggesting that it should not be disabled. I was asking
> > > that it be disabled in the -u-boot.dtsi file instead of the
> > > imx8mm.dtsi file because when the imx8mm.dtsi file gets resync'd
> > > with Linux, it might be lost. Having it done in the - u-boot.dtsi
> > > file instead helps reduce the likelihood of being undone later, and
> > > that is the purpose of the -u-
> > boot.dtsi files.
> >
> > If I may add my 2c here:
> > It seems to me that this patch should be split into at least 2 separate patches:
> > one that disables the JR0 node, and one - for the rest.
> >
> > The reason being: if the JR0 node is reserved for ATF and should not
> > be available
> > - then it should be disabled in the Kernel DTB as well as here.
> >
> > This part is missing in upstream Kernel, so the boot process throws
> > following errors during JR 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
> > ----
> >
> > Disabling the JR0 node in the kernel makes this error go away, and
> > decreases the JR count to 2 which can be observed in the NXP vendor kernel.
> >
> > I suggest you to extract the node disabling from this patch and send it to Kernel.
> > Once accepted in upstream - this change would be picked up by the
> > U-Boot at next DTB re-sync.
> >
> > -- andrey
> >
>
> Removing the JR0 disable change from this patch will cause CAAM failed to
> initialize on iMX8M series.
> As Adam suggested, I will disable the JR0 in the - u-boot.dtsi. I will also send a
> patch to kernel for disabling the JRO.
> When U-boot DTB will be synced with kernel, I will remove the JR0 disable change
> from - u-boot.dtsi
I'm currently working on dynamic recognition of Job Rings which are reserved for TZ
at the early boot, there is a patch proposed to address this [1]. I was planning to re-work
this to also include a small clean-up of the CAAM driver in a way how JRs are probed.
The patch does not require nodes to be permanently disabled in DTB, rather they would
need to be omitted from the probing instead.
I was wondering perhaps similar changes can be introduced in the U-Boot, since it would
allow to account for cases where JRs reserved for specific purposes (e.g. HAB) can be
dynamically changed and would not require modifications of DTB to align.
In generic case, any JR can be "taken away" into TZ. Those modifications should be
sync'ed with DTB changes, while having this dynamically recognized would have a
benefit of not touching the DTB at all.
>
> Regards
> Gaurav Jain
>
Link: [1]: https://lore.kernel.org/lkml/20211104162114.2019509-1-andrey.zhizhikin@leica-geosystems.com/
-- andrey
More information about the U-Boot
mailing list