[PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[Sascha Hauer]

Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.

When barebox starts we have to guess if we have to start OP-TEE or not.
As we can't detect the exception level on ARMv7 we do this by checking
if a first stage loader has passed us a device tree.

First of all the device tree is passed in r2, not in r0, so fix the
register we use.

Then we have to check if r2 is within the SDRAM. We check against
MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
base address of the SDRAM instead.

Finally we manipulate the TZASC which we are obviously not allowed in
EL1, so move the manipulation to the code which is only executed in EL2.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/tqma6ulx/lowlevel.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boards/tqma6ulx/lowlevel.c b/arch/arm/boards/tqma6ulx/lowlevel.c
index 5fd997d2ec..ce330c37af 100644
--- a/arch/arm/boards/tqma6ulx/lowlevel.c
+++ b/arch/arm/boards/tqma6ulx/lowlevel.c
@@ -66,7 +66,7 @@ static void *read_eeprom(void)
 	return fdt;
 }
 
-static void noinline start_mba6ulx(u32 r0)
+static void noinline start_mba6ulx(u32 r2)
 {
 	void *fdt;
 	int tee_size;
@@ -76,16 +76,15 @@ static void noinline start_mba6ulx(u32 r0)
 
 	fdt = read_eeprom();
 
-	/* Enable normal/secure r/w for TZC380 region0 */
-	writel(0xf0000000, 0x021D0108);
-
 	/*
 	 * Chainloading barebox will pass a device tree within the RAM in r0,
 	 * skip OP-TEE early loading in this case
 	 */
 	if (IS_ENABLED(CONFIG_FIRMWARE_TQMA6UL_OPTEE) &&
-	    !(r0 > MX6_MMDC_P0_BASE_ADDR &&
-	      r0 < MX6_MMDC_P0_BASE_ADDR + SZ_256M)) {
+	    !(r2 > MX6_MMDC_PORT0_BASE_ADDR && r2 < MX6_MMDC_PORT0_BASE_ADDR + SZ_256M)) {
+		/* Enable normal/secure r/w for TZC380 region0 */
+		writel(0xf0000000, 0x021D0108);
+
 		get_builtin_firmware(mba6ul_optee_bin, &tee, &tee_size);
 
 		memset((void *)OPTEE_OVERLAY_LOCATION, 0, 0x1000);
@@ -112,5 +111,5 @@ ENTRY_FUNCTION(start_imx6ul_mba6ulx, r0, r1, r2)
 	setup_c();
 	barrier();
 
-	start_mba6ulx(r0);
+	start_mba6ulx(r2);
 }
-- 
2.39.5

[PATCH 2/2] ARM: i.MX: Webasto ccbv2: fix barebox chainloading with OP-TEE enabled

[Sascha Hauer]

Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.

When barebox starts we have to guess if we have to start OP-TEE or not.
As we can't detect the exception level on ARMv7 we do this by checking
if a first stage loader has passed us a device tree.

First of all the device tree is passed in r2, not in r0, so fix the
register we use.

Then we have to check if r2 is within the SDRAM. We check against
MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
base address of the SDRAM instead.

Finally we manipulate the TZASC which we are obviously not allowed in
EL1, so move the manipulation to the code which is only executed in EL2.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/webasto-ccbv2/lowlevel.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boards/webasto-ccbv2/lowlevel.c b/arch/arm/boards/webasto-ccbv2/lowlevel.c
index 7a198bd801..17264479c2 100644
--- a/arch/arm/boards/webasto-ccbv2/lowlevel.c
+++ b/arch/arm/boards/webasto-ccbv2/lowlevel.c
@@ -31,23 +31,21 @@ static void configure_uart(void)
 
 }
 
-static void noinline start_ccbv2(u32 r0, unsigned long mem_size, char *fdt)
+static void noinline start_ccbv2(u32 r2, unsigned long mem_size, char *fdt)
 {
 	int tee_size;
 	void *tee;
 
-	/* Enable normal/secure r/w for TZC380 region0 */
-	writel(0xf0000000, 0x021D0108);
-
 	configure_uart();
 
 	/*
 	 * Chainloading barebox will pass a device tree within the RAM in r0,
 	 * skip OP-TEE early loading in this case
 	 */
-	if(IS_ENABLED(CONFIG_FIRMWARE_CCBV2_OPTEE)
-	   && !(r0 > MX6_MMDC_P0_BASE_ADDR
-	        &&  r0 < MX6_MMDC_P0_BASE_ADDR + mem_size)) {
+	if(IS_ENABLED(CONFIG_FIRMWARE_CCBV2_OPTEE) &&
+	   !(r2 > MX6_MMDC_PORT0_BASE_ADDR && r2 < MX6_MMDC_PORT0_BASE_ADDR + mem_size)) {
+		/* Enable normal/secure r/w for TZC380 region0 */
+		writel(0xf0000000, 0x021D0108);
 		get_builtin_firmware(ccbv2_optee_bin, &tee, &tee_size);
 
 		memset((void *)OPTEE_OVERLAY_LOCATION, 0, 0x1000);
@@ -70,7 +68,7 @@ ENTRY_FUNCTION(start_imx6ul_ccbv2_256m, r0, r1, r2)
 	setup_c();
 	barrier();
 
-	start_ccbv2(r0, SZ_256M, __dtb_z_imx6ul_webasto_ccbv2_start);
+	start_ccbv2(r2, SZ_256M, __dtb_z_imx6ul_webasto_ccbv2_start);
 }
 
 ENTRY_FUNCTION(start_imx6ul_ccbv2_512m, r0, r1, r2)
@@ -83,7 +81,7 @@ ENTRY_FUNCTION(start_imx6ul_ccbv2_512m, r0, r1, r2)
 	setup_c();
 	barrier();
 
-	start_ccbv2(r0, SZ_512M, __dtb_z_imx6ul_webasto_ccbv2_start);
+	start_ccbv2(r2, SZ_512M, __dtb_z_imx6ul_webasto_ccbv2_start);
 }
 
 extern char __dtb_z_imx6ul_webasto_marvel_start[];
@@ -97,5 +95,5 @@ ENTRY_FUNCTION(start_imx6ul_marvel, r0, r1, r2)
 	setup_c();
 	barrier();
 
-	start_ccbv2(r0, SZ_512M, __dtb_z_imx6ul_webasto_marvel_start);
+	start_ccbv2(r2, SZ_512M, __dtb_z_imx6ul_webasto_marvel_start);
 }
-- 
2.39.5

Re: [PATCH 2/2] ARM: i.MX: Webasto ccbv2: fix barebox chainloading with OP-TEE enabled

[Marco Felsch]

On 25-06-26, Sascha Hauer wrote:
> Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.
> 
> When barebox starts we have to guess if we have to start OP-TEE or not.
> As we can't detect the exception level on ARMv7 we do this by checking
> if a first stage loader has passed us a device tree.
> 
> First of all the device tree is passed in r2, not in r0, so fix the
> register we use.
> 
> Then we have to check if r2 is within the SDRAM. We check against
> MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
> base address of the SDRAM instead.
> 
> Finally we manipulate the TZASC which we are obviously not allowed in
> EL1, so move the manipulation to the code which is only executed in EL2.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Feel free to add my:

Acked-by: Marco Felsch <m.felsch@pengutronix.de>

Regards,
  Marco

Re: [PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[Marco Felsch]

On 25-06-26, Sascha Hauer wrote:
> Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.
> 
> When barebox starts we have to guess if we have to start OP-TEE or not.
> As we can't detect the exception level on ARMv7 we do this by checking
> if a first stage loader has passed us a device tree.
> 
> First of all the device tree is passed in r2, not in r0, so fix the
> register we use.
> 
> Then we have to check if r2 is within the SDRAM. We check against
> MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
> base address of the SDRAM instead.
> 
> Finally we manipulate the TZASC which we are obviously not allowed in
> EL1, so move the manipulation to the code which is only executed in EL2.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Feel free to add my:

Acked-by: Marco Felsch <m.felsch@pengutronix.de>

Regards,
  Marco

Re: [PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[Ahmad Fatoum]

Hello Sascha,

On 6/26/25 16:03, Sascha Hauer wrote:
> Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.
> 
> When barebox starts we have to guess if we have to start OP-TEE or not.
> As we can't detect the exception level on ARMv7 we do this by checking
> if a first stage loader has passed us a device tree.
> 
> First of all the device tree is passed in r2, not in r0, so fix the
> register we use.
> 
> Then we have to check if r2 is within the SDRAM. We check against
> MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
> base address of the SDRAM instead.
> 
> Finally we manipulate the TZASC which we are obviously not allowed in
> EL1, so move the manipulation to the code which is only executed in EL2.

EL1/EL2 are ARMv8-specific.

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/boards/tqma6ulx/lowlevel.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boards/tqma6ulx/lowlevel.c b/arch/arm/boards/tqma6ulx/lowlevel.c
> index 5fd997d2ec..ce330c37af 100644
> --- a/arch/arm/boards/tqma6ulx/lowlevel.c
> +++ b/arch/arm/boards/tqma6ulx/lowlevel.c
> @@ -66,7 +66,7 @@ static void *read_eeprom(void)
>  	return fdt;
>  }
>  
> -static void noinline start_mba6ulx(u32 r0)
> +static void noinline start_mba6ulx(u32 r2)
>  {
>  	void *fdt;
>  	int tee_size;
> @@ -76,16 +76,15 @@ static void noinline start_mba6ulx(u32 r0)
>  
>  	fdt = read_eeprom();
>  
> -	/* Enable normal/secure r/w for TZC380 region0 */
> -	writel(0xf0000000, 0x021D0108);
> -
>  	/*
>  	 * Chainloading barebox will pass a device tree within the RAM in r0,
>  	 * skip OP-TEE early loading in this case
>  	 */
>  	if (IS_ENABLED(CONFIG_FIRMWARE_TQMA6UL_OPTEE) &&
> -	    !(r0 > MX6_MMDC_P0_BASE_ADDR &&
> -	      r0 < MX6_MMDC_P0_BASE_ADDR + SZ_256M)) {
> +	    !(r2 > MX6_MMDC_PORT0_BASE_ADDR && r2 < MX6_MMDC_PORT0_BASE_ADDR + SZ_256M)) {
> +		/* Enable normal/secure r/w for TZC380 region0 */
> +		writel(0xf0000000, 0x021D0108);

I think this is problematic:

  - robustness-wise: We have no guarantee that there isn't some lesser
    used BootROM code path that happens to leave a suitable DRAM
    look-alike address that would trick us here.

  - security wise, even if we check for FDT header if r2 points into
    DRAM, a compromised OS could spray RAM with FDT magic,
    trigger a warm reset that has the BootROM produce a DRAM lookalike
    pointer in r2 and then OP-TEE loading is skipped and the kernel
    starts in the highest privilege level.

To address this, we need some way to set a sticky bit that's cleared
only on reset. One way, would be to set up an IVT and try to access the
L2 cache controller while data_abort_mask() is active, like
imx6_cannot_write_l2x0 is doing.

Cheers,
Ahmad

> +
>  		get_builtin_firmware(mba6ul_optee_bin, &tee, &tee_size);
>  
>  		memset((void *)OPTEE_OVERLAY_LOCATION, 0, 0x1000);
> @@ -112,5 +111,5 @@ ENTRY_FUNCTION(start_imx6ul_mba6ulx, r0, r1, r2)
>  	setup_c();
>  	barrier();
>  
> -	start_mba6ulx(r0);
> +	start_mba6ulx(r2);
>  }

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[Marco Felsch]

On 25-06-26, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 6/26/25 16:03, Sascha Hauer wrote:
> > Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.
> > 
> > When barebox starts we have to guess if we have to start OP-TEE or not.
> > As we can't detect the exception level on ARMv7 we do this by checking
> > if a first stage loader has passed us a device tree.
> > 
> > First of all the device tree is passed in r2, not in r0, so fix the
> > register we use.
> > 
> > Then we have to check if r2 is within the SDRAM. We check against
> > MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
> > base address of the SDRAM instead.
> > 
> > Finally we manipulate the TZASC which we are obviously not allowed in
> > EL1, so move the manipulation to the code which is only executed in EL2.
> 
> EL1/EL2 are ARMv8-specific.
> 
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/boards/tqma6ulx/lowlevel.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm/boards/tqma6ulx/lowlevel.c b/arch/arm/boards/tqma6ulx/lowlevel.c
> > index 5fd997d2ec..ce330c37af 100644
> > --- a/arch/arm/boards/tqma6ulx/lowlevel.c
> > +++ b/arch/arm/boards/tqma6ulx/lowlevel.c
> > @@ -66,7 +66,7 @@ static void *read_eeprom(void)
> >  	return fdt;
> >  }
> >  
> > -static void noinline start_mba6ulx(u32 r0)
> > +static void noinline start_mba6ulx(u32 r2)
> >  {
> >  	void *fdt;
> >  	int tee_size;
> > @@ -76,16 +76,15 @@ static void noinline start_mba6ulx(u32 r0)
> >  
> >  	fdt = read_eeprom();
> >  
> > -	/* Enable normal/secure r/w for TZC380 region0 */
> > -	writel(0xf0000000, 0x021D0108);
> > -
> >  	/*
> >  	 * Chainloading barebox will pass a device tree within the RAM in r0,
> >  	 * skip OP-TEE early loading in this case
> >  	 */
> >  	if (IS_ENABLED(CONFIG_FIRMWARE_TQMA6UL_OPTEE) &&
> > -	    !(r0 > MX6_MMDC_P0_BASE_ADDR &&
> > -	      r0 < MX6_MMDC_P0_BASE_ADDR + SZ_256M)) {
> > +	    !(r2 > MX6_MMDC_PORT0_BASE_ADDR && r2 < MX6_MMDC_PORT0_BASE_ADDR + SZ_256M)) {
> > +		/* Enable normal/secure r/w for TZC380 region0 */
> > +		writel(0xf0000000, 0x021D0108);
> 
> I think this is problematic:
> 
>   - robustness-wise: We have no guarantee that there isn't some lesser
>     used BootROM code path that happens to leave a suitable DRAM
>     look-alike address that would trick us here.
> 
>   - security wise, even if we check for FDT header if r2 points into
>     DRAM, a compromised OS could spray RAM with FDT magic,
>     trigger a warm reset that has the BootROM produce a DRAM lookalike
>     pointer in r2 and then OP-TEE loading is skipped and the kernel
>     starts in the highest privilege level.

These are good points and should be addressed, but I think the patch is
still good because obvious broken code gets repaired.

> To address this, we need some way to set a sticky bit that's cleared
> only on reset. One way, would be to set up an IVT and try to access the
> L2 cache controller while data_abort_mask() is active, like
> imx6_cannot_write_l2x0 is doing.

Nice trick!

One could also argue that skip the OP-TEE loading (to chainload barebox)
is a dev-feature which should be disabled once the INSECURE=n.

Regards,
  Marco

> Cheers,
> Ahmad
> 
> > +
> >  		get_builtin_firmware(mba6ul_optee_bin, &tee, &tee_size);
> >  
> >  		memset((void *)OPTEE_OVERLAY_LOCATION, 0, 0x1000);
> > @@ -112,5 +111,5 @@ ENTRY_FUNCTION(start_imx6ul_mba6ulx, r0, r1, r2)
> >  	setup_c();
> >  	barrier();
> >  
> > -	start_mba6ulx(r0);
> > +	start_mba6ulx(r2);
> >  }
> 
> -- 
> Pengutronix e.K.                  |                             |
> Steuerwalder Str. 21              | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |
> 
> 
>