Re: [PATCH] ARM: mach-imx: tzasc: keep default region 0 secure settings for i.MX8M

[Marco Felsch <m.felsch@pengutronix.de>]

On 25-09-04, Sascha Hauer wrote:
> On Wed, Sep 03, 2025 at 02:41:29PM +0200, Marco Felsch wrote:
> > On 25-09-03, Sascha Hauer wrote:
> > > On Mon, Sep 01, 2025 at 12:29:37PM +0200, Marco Felsch wrote:
> > > > The TZC-380 region 0 is the TZC default (fallback) region. This region
> > > > is used if access to a certain DRAM address was done which isn't
> > > > configured by any other region (see [1] for more information). Region 0
> > > > covers the complete AXI space from 0x0 to AXI-bus width. The access is
> > > > secure-only after reset.
> > > > 
> > > > The TZC-380 is not memory alias aware (see [1] for more information) and
> > > > due to the DDR controller, the i.MX8M allows memory alias access.
> > > > 
> > > > Configuring region 0 as secure + non-secure RW access opens the
> > > > potential security risk of allowing access to secure only memory e.g.
> > > > TEE memory area if the TEE didn't configure all memory aliases for its
> > > > memory. Because in such case region 0 could be used as fallback if an
> > > > attackers access the TEE memory via memory aliases.
> > > > 
> > > > Don't reconfigure TZC-380 default region 0 to allow secure and
> > > > non-secure access and instead setup an early non-secure region 1 which
> > > > covers the complete ram <= 4G size to fix this.
> > > > 
> > > > [1] https://developer.arm.com/documentation/ddi0431/c
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  arch/arm/mach-imx/tzasc.c | 19 ++++++++++++++-----
> > > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/tzasc.c b/arch/arm/mach-imx/tzasc.c
> > > > index 0fe7f6eb7f4a..31664bbf2b39 100644
> > > > --- a/arch/arm/mach-imx/tzasc.c
> > > > +++ b/arch/arm/mach-imx/tzasc.c
> > > > @@ -345,6 +345,7 @@ bool imx6_can_access_tzasc(void)
> > > >  void imx8m_tzc380_init(void)
> > > >  {
> > > >  	u32 __iomem *gpr = IOMEM(MX8M_IOMUXC_GPR_BASE_ADDR);
> > > > +	resource_size_t ram_sz;
> > > >  
> > > >  	/* Enable TZASC and lock setting */
> > > >  	setbits_le32(&gpr[10], GPR_TZASC_EN);
> > > > @@ -364,13 +365,21 @@ void imx8m_tzc380_init(void)
> > > >  	if (cpu_is_mx8mn() || cpu_is_mx8mp())
> > > >  		setbits_le32(&gpr[10], GPR_TZASC_ID_SWAP_BYPASS_LOCK);
> > > >  
> > > > +	/* All i.MX8M do have a 32-bit bus width except for the i.MX8M Nano */
> > > > +	ram_sz = imx8m_barebox_earlymem_size(32);
> > > > +	if (cpu_is_mx8mn())
> > > > +		ram_sz = imx8m_barebox_earlymem_size(16);
> > > 
> > > earlymem_size is limited to the 32bit address space. What about the DRAM
> > > above the 32bit address space? Don't we make this inaccessible with this
> > > patch?
> > 
> > A problem would arise if the USB Core uses memory above 4G. I've tested
> > the i.MX8MP USB download and it's still working with this patch. I
> > assume that the USB-Core is only 32-bit capable.
> 
> What about other bus masters, like LCDIF, SDMA, SDHCI?

These bus masters are not used during PBL stage. OP-TEE reconfigures
this to the actual memory size later on. So this should be fine.

> barebox itself won't be affected anyway because it only uses the 32bit
> space, but Linux uses the full address space.

Which is fine because OP-TEE reconfigures the TZASC anyway.

Regards,
  Marco