[v2,2/2] warp7 : run sec_init for CAAM RNG

Message ID 1516932578-19992-3-git-send-email-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Fix CAAM for TrustZone enable for warp7
Related show

Commit Message

Bryan O'Donoghue Jan. 26, 2018, 2:09 a.m.
This patch adds a sec_init call into board_init. Doing so in conjunction
with the patch "drivers/crypto/fsl: assign job-rings to non-TrustZone"
enables use of the CAAM in Linux when OPTEE/TrustZone is active.

u-boot will initialise the RNG and assign ownership of the job-ring
registers to a non-TrustZone context. Linux then simply has to detect or be
told to skip RNG initialisation.

This change is safe both for the OPTEE/TrustZone boot path and the regular
non-OPTEE/TrustZone boot path.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Marco Franchi <marco.franchi@nxp.com>
Cc: Vanessa Maegima <vanessa.maegima@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 board/warp7/warp7.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Auer, Lukas Jan. 26, 2018, 9:09 a.m. | #1
On Fri, 2018-01-26 at 02:09 +0000, Bryan O'Donoghue wrote:
> This patch adds a sec_init call into board_init. Doing so in

> conjunction

> with the patch "drivers/crypto/fsl: assign job-rings to non-

> TrustZone"

> enables use of the CAAM in Linux when OPTEE/TrustZone is active.

> 

> u-boot will initialise the RNG and assign ownership of the job-ring

> registers to a non-TrustZone context. Linux then simply has to detect

> or be

> told to skip RNG initialisation.

> 

> This change is safe both for the OPTEE/TrustZone boot path and the

> regular

> non-OPTEE/TrustZone boot path.

> 

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> Cc: Fabio Estevam <fabio.estevam@nxp.com>

> Cc: Peng Fan <peng.fan@nxp.com>

> Cc: Marco Franchi <marco.franchi@nxp.com>

> Cc: Vanessa Maegima <vanessa.maegima@nxp.com>

> Cc: Stefano Babic <sbabic@denx.de>

> ---

>  board/warp7/warp7.c | 6 +++++-

>  1 file changed, 5 insertions(+), 1 deletion(-)

> 

> diff --git a/board/warp7/warp7.c b/board/warp7/warp7.c

> index 337e76b..219ab6f 100644

> --- a/board/warp7/warp7.c

> +++ b/board/warp7/warp7.c

> @@ -16,6 +16,7 @@

>  #include <asm/io.h>

>  #include <common.h>

>  #include <fsl_esdhc.h>

> +#include <fsl_sec.h>

>  #include <i2c.h>

>  #include <mmc.h>

>  #include <asm/arch/crm_regs.h>

> @@ -225,6 +226,10 @@ int board_init(void)

>  		setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,

> &i2c_pad_info1);

>  	#endif

>  

> +	#ifdef CONFIG_FSL_CAAM

> +		sec_init();

> +	#endif

> +

>  	return 0;

>  }

>  

> @@ -366,5 +371,4 @@ int g_dnl_bind_fixup(struct usb_device_descriptor

> *dev, const char *name)

>  

>  	return 0;

>  }

> -

>  #endif /* ifdef CONFIG_USB_GADGET */


Hi Bryan,

this fails to apply for me on current HEAD. It seems like you have
additional modifications to wrap7.c in your tree (there is no
CONFIG_USB_GADGET on master).

Regarding the patch, would it make sense to put sec_init() somewhere
else, so that it does not have to be duplicated in the board file for
all platforms with CAAM?

Thanks,
Lukas
Bryan O'Donoghue Jan. 26, 2018, 11:32 a.m. | #2
On 26/01/18 09:09, Auer, Lukas wrote:
> Hi Bryan,
> 
> this fails to apply for me on current HEAD. It seems like you have
> additional modifications to wrap7.c in your tree (there is no
> CONFIG_USB_GADGET on master).

I'm carrying a few patches locally and upstreaming gradually - got 
caught out here...

> Regarding the patch, would it make sense to put sec_init() somewhere
> else, so that it does not have to be duplicated in the board file for
> all platforms with CAAM?

It does... to me. Looking at these .. I'd say leave the old 
powerpc/freescale stuff alone.

This works for me as an alternative when I tested it

diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c
index d160e80..d399fd8 100644
--- a/arch/arm/mach-imx/mx7/soc.c
+++ b/arch/arm/mach-imx/mx7/soc.c
@@ -261,6 +261,9 @@ int arch_misc_init(void)
         else
                 env_set("soc", "imx7s");
  #endif
+       #ifdef CONFIG_FSL_CAAM
+               sec_init();
+       #endif

         return 0;
  }

perhaps this would work for other i.mx processors

diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
index 43cb581..679c23b 100644
--- a/arch/arm/mach-imx/mx6/soc.c
+++ b/arch/arm/mach-imx/mx6/soc.c
@@ -515,6 +515,10 @@ int board_postclk_init(void)

         set_ldo_voltage(LDO_SOC, 1175); /* Set VDDSOC to 1.175V */

+#ifdef CONFIG_FSL_CAAM
+       sec_init();
+#endif
+
         return 0;
  }

diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c
index 454665a..dc3d601 100644
--- a/arch/arm/mach-imx/mx7ulp/soc.c
+++ b/arch/arm/mach-imx/mx7ulp/soc.c
@@ -57,6 +57,11 @@ int arch_cpu_init(void)
  #ifdef CONFIG_BOARD_POSTCLK_INIT
  int board_postclk_init(void)
  {
+
+#ifdef CONFIG_FSL_CAAM
+       sec_init();
+#endif
+
         return 0;
  }
  #endif

I'd say the right thing to do is - fix it for all i.MX7D/S and let 
others with access to mx6/mx7ulp etc test/patch themselves.

Anyway I'll send a generic patch for i.mx7s/d in arch_misc_init()

---
bod
Auer, Lukas Jan. 26, 2018, 12:30 p.m. | #3
On Fri, 2018-01-26 at 11:32 +0000, Bryan O'Donoghue wrote:
> 

> On 26/01/18 09:09, Auer, Lukas wrote:

> > Hi Bryan,

> > 

> > this fails to apply for me on current HEAD. It seems like you have

> > additional modifications to wrap7.c in your tree (there is no

> > CONFIG_USB_GADGET on master).

> 

> I'm carrying a few patches locally and upstreaming gradually - got 

> caught out here...

> 

> > Regarding the patch, would it make sense to put sec_init()

> > somewhere

> > else, so that it does not have to be duplicated in the board file

> > for

> > all platforms with CAAM?

> 

> It does... to me. Looking at these .. I'd say leave the old 

> powerpc/freescale stuff alone.

> 

> This works for me as an alternative when I tested it

> 

> diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-

> imx/mx7/soc.c

> index d160e80..d399fd8 100644

> --- a/arch/arm/mach-imx/mx7/soc.c

> +++ b/arch/arm/mach-imx/mx7/soc.c

> @@ -261,6 +261,9 @@ int arch_misc_init(void)

>          else

>                  env_set("soc", "imx7s");

>   #endif

> +       #ifdef CONFIG_FSL_CAAM

> +               sec_init();

> +       #endif

> 

>          return 0;

>   }

> 

> perhaps this would work for other i.mx processors

> 

> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-

> imx/mx6/soc.c

> index 43cb581..679c23b 100644

> --- a/arch/arm/mach-imx/mx6/soc.c

> +++ b/arch/arm/mach-imx/mx6/soc.c

> @@ -515,6 +515,10 @@ int board_postclk_init(void)

> 

>          set_ldo_voltage(LDO_SOC, 1175); /* Set VDDSOC to 1.175V */

> 

> +#ifdef CONFIG_FSL_CAAM

> +       sec_init();

> +#endif

> +

>          return 0;

>   }

> 

> diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-

> imx/mx7ulp/soc.c

> index 454665a..dc3d601 100644

> --- a/arch/arm/mach-imx/mx7ulp/soc.c

> +++ b/arch/arm/mach-imx/mx7ulp/soc.c

> @@ -57,6 +57,11 @@ int arch_cpu_init(void)

>   #ifdef CONFIG_BOARD_POSTCLK_INIT

>   int board_postclk_init(void)

>   {

> +

> +#ifdef CONFIG_FSL_CAAM

> +       sec_init();

> +#endif

> +

>          return 0;

>   }

>   #endif

> 

> I'd say the right thing to do is - fix it for all i.MX7D/S and let 

> others with access to mx6/mx7ulp etc test/patch themselves.

> 

> Anyway I'll send a generic patch for i.mx7s/d in arch_misc_init()

> 

> ---

> bod


I agree, that's a good way to add the initialization code.

Patch

diff --git a/board/warp7/warp7.c b/board/warp7/warp7.c
index 337e76b..219ab6f 100644
--- a/board/warp7/warp7.c
+++ b/board/warp7/warp7.c
@@ -16,6 +16,7 @@ 
 #include <asm/io.h>
 #include <common.h>
 #include <fsl_esdhc.h>
+#include <fsl_sec.h>
 #include <i2c.h>
 #include <mmc.h>
 #include <asm/arch/crm_regs.h>
@@ -225,6 +226,10 @@  int board_init(void)
 		setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1);
 	#endif
 
+	#ifdef CONFIG_FSL_CAAM
+		sec_init();
+	#endif
+
 	return 0;
 }
 
@@ -366,5 +371,4 @@  int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
 
 	return 0;
 }
-
 #endif /* ifdef CONFIG_USB_GADGET */