[EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot

ZHIZHIKIN Andrey andrey.zhizhikin at leica-geosystems.com
Thu Jun 9 19:29:34 CEST 2022


Hello Gaurav,

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Gaurav Jain
> Sent: Thursday, June 9, 2022 3:13 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; u-
> boot at lists.denx.de; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; Tommaso Merciai <tommaso.merciai at amarulasolutions.com>;
> Michael Trimarchi <michael at amarulasolutions.com>; Marek Vasut <marex at denx.de>;
> Simon Glass <sjg at chromium.org>; Patrick Delaunay <patrick.delaunay at foss.st.com>;
> Stefan Roese <sr at denx.de>; Horia Geanta <horia.geanta at nxp.com>; Pankaj Gupta
> <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; Ye Li <ye.li at nxp.com>;
> Michael Walle <michael at walle.cc>
> Cc: dl-uboot-imx <uboot-imx at nxp.com>
> Subject: RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> 
> Hello Andrey
> 
> > -----Original Message-----
> > From: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> > Sent: Wednesday, June 8, 2022 8:31 PM
> > To: Gaurav Jain <gaurav.jain at nxp.com>; u-boot at lists.denx.de; Stefano Babic
> > <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Tommaso Merciai
> > <tommaso.merciai at amarulasolutions.com>; Michael Trimarchi
> > <michael at amarulasolutions.com>; Marek Vasut <marex at denx.de>; Simon
> > Glass <sjg at chromium.org>; Patrick Delaunay <patrick.delaunay at foss.st.com>;
> > Stefan Roese <sr at denx.de>; Horia Geanta <horia.geanta at nxp.com>; Pankaj
> > Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; Ye Li
> > <ye.li at nxp.com>; Michael Walle <michael at walle.cc>
> > Cc: dl-uboot-imx <uboot-imx at nxp.com>
> > Subject: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> >
> > Caution: EXT Email
> >
> > Hello Gaurav,
> >
> > Cc: Michael Walle here.
> >
> > I guess this is a re-incarnation of the previous discussions we had regarding
> the
> > JR reservation, see [1].
> >
> > > -----Original Message-----
> > > From: Gaurav Jain <gaurav.jain at nxp.com>
> > > Sent: Wednesday, June 8, 2022 3:34 PM
> > > To: u-boot at lists.denx.de; Stefano Babic <sbabic at denx.de>; Fabio
> > > Estevam <festevam at gmail.com>; Tommaso Merciai
> > > <tommaso.merciai at amarulasolutions.com>;
> > > ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; Michael
> > > Trimarchi <michael at amarulasolutions.com>; Marek Vasut <marex at denx.de>;
> > > Simon Glass <sjg at chromium.org>; Patrick Delaunay
> > > <patrick.delaunay at foss.st.com>; Stefan Roese <sr at denx.de>; Horia
> > > Geanta <horia.geanta at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>;
> > > Varun Sethi <V.Sethi at nxp.com>; Ye Li <ye.li at nxp.com>
> > > Cc: NXP i . MX U-Boot Team <uboot-imx at nxp.com>; Gaurav Jain
> > > <gaurav.jain at nxp.com>
> > > Subject: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
> > >
> > > disabled use of JR0 in SPL and uboot, as JR0 is reserved for HAB in
> > > TF-A.
> > >
> > > Signed-off-by: Gaurav Jain <gaurav.jain at nxp.com>
> > > ---
> > >  arch/arm/dts/imx8mm-evk-u-boot.dtsi        |  1 +
> > >  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   |  1 +
> > >  arch/arm/dts/imx8mp-evk-u-boot.dtsi        |  1 +
> > >  arch/arm/dts/imx8mq-evk-u-boot.dtsi        |  4 ++++
> >
> > Shall those DTB changes be sync'd with Kernel?
> >
> > Now that the JR0 reservation is done in both upstream and downstream TF-A -
> > Kernel would fail to initialize the JR0.
> >
> > This is what Fabio just noted and posted as a comment. :-)
> >
> > I suggest that this is submitted into Kernel, and then picked up during the
> next
> > DTB re-sync.
> 
> Ok. Fabio has already submitted a patch for this.

Great! Then I think DTB part will not be required in this patch.

> >
> > >  arch/arm/include/asm/arch-imx8m/imx-regs.h |  1 +
> > >  drivers/crypto/fsl/jr.c                    | 14 +++++++++++---
> > >  scripts/config_whitelist.txt               |  1 +
> > >  7 files changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mm-evk-u- boot.dtsi index e9fbf7b802..8cd37b5205
> > > 100644
> > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > @@ -74,6 +74,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mn-ddr4- evk-u-boot.dtsi index
> > > 4d0ecb07d4..0c31f2737a 100644
> > > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > @@ -114,6 +114,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mp-evk-u- boot.dtsi index f43eb6238d..28dce55fb9
> > > 100644
> > > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > @@ -77,6 +77,7 @@
> > >
> > >  &sec_jr0 {
> > >         u-boot,dm-spl;
> > > +       status = "disabled";
> > >  };
> > >
> > >  &sec_jr1 {
> > > diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mq-evk-u- boot.dtsi index 67da69a2eb..37364eb6b4
> > > 100644
> > > --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > > @@ -18,3 +18,7 @@
> > >  &uart1 {
> > >         u-boot,dm-spl;
> > >  };
> > > +
> > > +&sec_jr0 {
> > > +       status = "disabled";
> > > +};
> > > diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > index 1da75528d4..e6e2974df3 100644
> > > --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> > > @@ -89,6 +89,7 @@
> > >  #define CONFIG_SYS_FSL_SEC_ADDR         (CAAM_IPS_BASE_ADDR + \
> > >                                          CONFIG_SYS_FSL_SEC_OFFSET)
> > >  #define CONFIG_SYS_FSL_JR0_OFFSET       (0x1000)
> > > +#define CONFIG_SYS_FSL_JR1_OFFSET      (0x2000)

This shall be converted to config option, as each SoC family defines this
now differently.

Ideally, only JR ID shall be provided by config option, ADDR and OFFSET
can then be deducted based on the index.

> > >  #define CONFIG_SYS_FSL_JR0_ADDR         (CONFIG_SYS_FSL_SEC_ADDR + \
> > >                                          CONFIG_SYS_FSL_JR0_OFFSET)
> > >  #define CONFIG_SYS_FSL_MAX_NUM_OF_SEC   1
> > > diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index
> > > acd29924f7..66dd9cf365 100644
> > > --- a/drivers/crypto/fsl/jr.c
> > > +++ b/drivers/crypto/fsl/jr.c
> > > @@ -44,9 +44,17 @@ struct udevice *caam_dev;  #define SEC_ADDR(idx)  \
> > >         (ulong)((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx]))
> > >
> > > -#define SEC_JR0_ADDR(idx)      \
> > > +#ifndef CONFIG_IMX8M
> > > +#define SEC_JR_ADDR(idx)       \
> > >         (ulong)(SEC_ADDR(idx) + \
> > >          (CONFIG_SYS_FSL_JR0_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > > +#define JR_ID 0
> > > +#else
> > > +#define SEC_JR_ADDR(idx)       \
> > > +       (ulong)(SEC_ADDR(idx) + \
> > > +       (CONFIG_SYS_FSL_JR1_OFFSET - CONFIG_SYS_FSL_SEC_OFFSET))
> > > +#define JR_ID 1 #endif
> >

Once config option is introduced, it can be used here instead of macro.
Reserved JR ID can be used to deduct both ADDR and OFFSET parameters.
Then this re-use would not be necessary.

> > I believe this whole macro can be simplified, isn't it?
> 
> I reused the old macro to define for JR1.
> How you want to simplify?

See suggestions above.

> 
> Regards
> Gaurav Jain
> >
> > >  struct caam_regs caam_st;
> > >  #endif
> > >
> > > @@ -685,8 +693,8 @@ int sec_init_idx(uint8_t sec_idx)
> > >         caam = dev_get_priv(caam_dev);  #else
> > >         caam_st.sec = (void *)SEC_ADDR(sec_idx);
> > > -       caam_st.regs = (struct jr_regs *)SEC_JR0_ADDR(sec_idx);
> > > -       caam_st.jrid = 0;
> > > +       caam_st.regs = (struct jr_regs *)SEC_JR_ADDR(sec_idx);
> > > +       caam_st.jrid = JR_ID;
> > >         caam = &caam_st;
> > >  #endif
> > >  #if CONFIG_IS_ENABLED(OF_CONTROL)
> > > diff --git a/scripts/config_whitelist.txt
> > > b/scripts/config_whitelist.txt index cecdda6781..b99aeacbc4 100644
> > > --- a/scripts/config_whitelist.txt
> > > +++ b/scripts/config_whitelist.txt
> > > @@ -1040,6 +1040,7 @@ CONFIG_SYS_FSL_IFC_LE
> > CONFIG_SYS_FSL_ISBC_VER
> > > CONFIG_SYS_FSL_JR0_ADDR  CONFIG_SYS_FSL_JR0_OFFSET
> > > +CONFIG_SYS_FSL_JR1_OFFSET

This will not be necessary, and shall rather be replaced by JR ID config option.

> > >  CONFIG_SYS_FSL_LS1_CLK_ADDR
> > >  CONFIG_SYS_FSL_LSCH3_SERDES_ADDR
> > >  CONFIG_SYS_FSL_MAX_NUM_OF_SEC
> > > --
> > > 2.25.1
> >
> >
> > Link: [1]:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> > el.org%2Fu-
> > boot%2FVI1PR04MB53426E0295D8E2508BA01302E7089%40VI1PR04MB5342.e
> > urprd04.prod.outlook.com%2F&data=05%7C01%7Cgaurav.jain%40nxp.co
> > m%7Cc387babc3fed40edb32f08da495fa3f4%7C686ea1d3bc2b4c6fa92cd99c5c3
> > 01635%7C0%7C0%7C637902972380396116%7CUnknown%7CTWFpbGZsb3d8ey
> > JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> > 3000%7C%7C%7C&sdata=WETAijFtyFGHnrimKkeT4jICk0OQSXfMbRb8vWsK
> > Mp0%3D&reserved=0
> > -- andrey

-- andrey


More information about the U-Boot mailing list