Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Gaurav, On Fri, Jun 10, 2022 at 8:51 AM Gaurav Jain wrote: > > Hi Fabio > > I can not see the patch in my mails. > Can you include me using my mail id. I added your email, when submitting: http://lists.infradead.org/pipermail/linux-arm-kernel/2022-June/749625.html Regards, Fabio Estevam
RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Fabio I can not see the patch in my mails. Can you include me using my mail id. Regards Gaurav Jain > -Original Message- > From: Fabio Estevam > Sent: Friday, June 10, 2022 1:36 AM > To: Gaurav Jain > Cc: ZHIZHIKIN Andrey ; u- > b...@lists.denx.de; Stefano Babic ; Tommaso Merciai > ; Michael Trimarchi > ; Marek Vasut ; Simon > Glass ; Patrick Delaunay ; > Stefan Roese ; Horia Geanta ; Pankaj > Gupta ; Varun Sethi ; Ye Li > ; Michael Walle ; dl-uboot-imx i...@nxp.com> > Subject: Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot > > Caution: EXT Email > > Hi Gaurav, > > On Thu, Jun 9, 2022 at 10:12 AM Gaurav Jain wrote: > > > > 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. > > Yes, if you have a chance please send your Tested-by or Reviewed-by to that > patch. > > Thanks
Re: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Gaurav, On Thu, Jun 9, 2022 at 10:12 AM Gaurav Jain wrote: > > 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. Yes, if you have a chance please send your Tested-by or Reviewed-by to that patch. Thanks
RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hello Gaurav,
> -Original Message-
> From: U-Boot On Behalf Of Gaurav Jain
> Sent: Thursday, June 9, 2022 3:13 PM
> To: ZHIZHIKIN Andrey ; u-
> b...@lists.denx.de; Stefano Babic ; Fabio Estevam
> ; Tommaso Merciai ;
> Michael Trimarchi ; Marek Vasut ;
> Simon Glass ; Patrick Delaunay
> ;
> Stefan Roese ; Horia Geanta ; Pankaj Gupta
> ; Varun Sethi ; Ye Li ;
> Michael Walle
> Cc: dl-uboot-imx
> Subject: RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
>
> Hello Andrey
>
> > -Original Message-
> > From: ZHIZHIKIN Andrey
> > Sent: Wednesday, June 8, 2022 8:31 PM
> > To: Gaurav Jain ; u-boot@lists.denx.de; Stefano Babic
> > ; Fabio Estevam ; Tommaso Merciai
> > ; Michael Trimarchi
> > ; Marek Vasut ; Simon
> > Glass ; Patrick Delaunay ;
> > Stefan Roese ; Horia Geanta ; Pankaj
> > Gupta ; Varun Sethi ; Ye Li
> > ; Michael Walle
> > Cc: dl-uboot-imx
> > 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
> > > Sent: Wednesday, June 8, 2022 3:34 PM
> > > To: u-boot@lists.denx.de; Stefano Babic ; Fabio
> > > Estevam ; Tommaso Merciai
> > > ;
> > > ZHIZHIKIN Andrey ; Michael
> > > Trimarchi ; Marek Vasut ;
> > > Simon Glass ; Patrick Delaunay
> > > ; Stefan Roese ; Horia
> > > Geanta ; Pankaj Gupta ;
> > > Varun Sethi ; Ye Li
> > > Cc: NXP i . MX U-Boot Team ; Gaurav Jain
> > >
> > > 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
> > > ---
> > > 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
> &
RE: [EXT] RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hello Andrey
> -Original Message-
> From: ZHIZHIKIN Andrey
> Sent: Wednesday, June 8, 2022 8:31 PM
> To: Gaurav Jain ; u-boot@lists.denx.de; Stefano Babic
> ; Fabio Estevam ; Tommaso Merciai
> ; Michael Trimarchi
> ; Marek Vasut ; Simon
> Glass ; Patrick Delaunay ;
> Stefan Roese ; Horia Geanta ; Pankaj
> Gupta ; Varun Sethi ; Ye Li
> ; Michael Walle
> Cc: dl-uboot-imx
> 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
> > Sent: Wednesday, June 8, 2022 3:34 PM
> > To: u-boot@lists.denx.de; Stefano Babic ; Fabio
> > Estevam ; Tommaso Merciai
> > ;
> > ZHIZHIKIN Andrey ; Michael
> > Trimarchi ; Marek Vasut ;
> > Simon Glass ; Patrick Delaunay
> > ; Stefan Roese ; Horia
> > Geanta ; Pankaj Gupta ;
> > Varun Sethi ; Ye Li
> > Cc: NXP i . MX U-Boot Team ; Gaurav Jain
> >
> > 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
> > ---
> > 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.
>
> > 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)
> > #define CONFIG_SYS_FSL_JR0_ADDR (CONFIG_SYS_FSL_SEC_ADDR + \
> > CONFIG_SYS_FSL_JR0_OFFSET)
&
Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Andrey, On Wed, Jun 8, 2022 at 12:00 PM ZHIZHIKIN Andrey wrote: > 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. Yes, agreed. Just submitted a patch doing this: http://lists.infradead.org/pipermail/linux-arm-kernel/2022-June/749625.html Thanks
RE: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
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
> Sent: Wednesday, June 8, 2022 3:34 PM
> To: u-boot@lists.denx.de; Stefano Babic ; Fabio Estevam
> ; Tommaso Merciai ;
> ZHIZHIKIN Andrey ; Michael Trimarchi
> ; Marek Vasut ; Simon Glass
> ; Patrick Delaunay ; Stefan
> Roese
> ; Horia Geanta ; Pankaj Gupta
> ; Varun Sethi ; Ye Li
> Cc: NXP i . MX U-Boot Team ; Gaurav Jain
>
> 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
> ---
> 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.
> 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)
> #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
I believe this whole macro can be simplified, isn't it?
> 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
> CONFI
Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Gaurav, On Wed, Jun 8, 2022 at 10:56 AM Fabio Estevam wrote: > Thanks for your patch. > > With your patch applied, I can use the NXP TF-A lf_v2.4 successfully: > no more CAAM sha256 errors are seen when launching a fitImage. > > Tested on a imx8mm-evk. > > Tested-by: Fabio Estevam I noticed a different issue when using the NXP TF-A lf_v2.4: There is a job ring probe error in the kernel (upstream 5.15.43): # dmesg | grep jr [2.449204] caam_jr 30901000.jr: failed to flush job ring 0 [2.454867] caam_jr: probe of 30901000.jr failed with error -5 This problem does not happen if I use NXP TF-A imx_5.4.47_2.2.0 version. If I disable sec_jr0 in the Linux devicetree, then the error does not happen. Should U-Boot disable sec_jr0 of the Linux devicetree too? What is a proper solution for this issue? Thanks, Fabio Estevam
Re: [PATCH] i.MX8M: crypto: disable JR0 in SPL, U-Boot
Hi Gaurav, On Wed, Jun 8, 2022 at 10:34 AM Gaurav Jain wrote: > > disabled use of JR0 in SPL and uboot, as JR0 is reserved > for HAB in TF-A. > > Signed-off-by: Gaurav Jain Thanks for your patch. With your patch applied, I can use the NXP TF-A lf_v2.4 successfully: no more CAAM sha256 errors are seen when launching a fitImage. Tested on a imx8mm-evk. Tested-by: Fabio Estevam
