linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm: l2c: unlock ways when in non-secure mode
@ 2017-11-26 12:25 Peng Fan
  2017-11-26 13:18 ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Peng Fan @ 2017-11-26 12:25 UTC (permalink / raw)
  To: linux-arm-kernel

To boot Linux in Non-secure mode with l2x0, the l2x0 controller
is enabled in secure mode and ways locked to make it seems L2 cache
disabled during linux boot process. So during l2x0 initialization,
need to unlock the ways to make l2x0 could cache data/inst.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Chris Brandt <chris.brandt@renesas.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/cache-l2x0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 808efbb89b88..de8eed0871ec 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -879,6 +879,10 @@ static int __init __l2c_init(const struct l2c_init_data *data,
 		l2x0_saved_regs.aux_ctrl = aux;
 
 		data->enable(l2x0_base, data->num_lock);
+	} else {
+		pr_info("%s: unlock cache controller\n", data->type);
+
+		data->unlock(l2x0_base, data->num_lock);
 	}
 
 	outer_cache = fns;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-26 12:25 [PATCH] arm: l2c: unlock ways when in non-secure mode Peng Fan
@ 2017-11-26 13:18 ` Russell King - ARM Linux
  2017-11-26 23:56   ` Peng Fan
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2017-11-26 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> To boot Linux in Non-secure mode with l2x0, the l2x0 controller
> is enabled in secure mode and ways locked to make it seems L2 cache
> disabled during linux boot process. So during l2x0 initialization,
> need to unlock the ways to make l2x0 could cache data/inst.

Why was this chosen instead of doing what everyone else does?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-26 13:18 ` Russell King - ARM Linux
@ 2017-11-26 23:56   ` Peng Fan
  2017-11-27  9:19     ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Peng Fan @ 2017-11-26 23:56 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

> Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> 
> On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > To boot Linux in Non-secure mode with l2x0, the l2x0 controller is
> > enabled in secure mode and ways locked to make it seems L2 cache
> > disabled during linux boot process. So during l2x0 initialization,
> > need to unlock the ways to make l2x0 could cache data/inst.
> 
> Why was this chosen instead of doing what everyone else does?

I am not aware of how other platform handles the l2x0 unlock in non
secure mode. Could you please share with me what others choose?

Thanks,
Peng.

> 
> --
> RMK's Patch system:
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=02%7C01%7Cpeng.fan%4
> 0nxp.com%7C277f5508e3ef412c3dc908d534d031ab%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C636472991153380699&sdata=6QBrJCdmtU%2B4GO8
> YsYMvcvRwwk5ST2B87%2BWY%2BhPRxP8%3D&reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-26 23:56   ` Peng Fan
@ 2017-11-27  9:19     ` Russell King - ARM Linux
  2017-11-27  9:43       ` Peng Fan
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2017-11-27  9:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Nov 26, 2017 at 11:56:10PM +0000, Peng Fan wrote:
> Hi Russell,
> 
> > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> > 
> > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > > To boot Linux in Non-secure mode with l2x0, the l2x0 controller is
> > > enabled in secure mode and ways locked to make it seems L2 cache
> > > disabled during linux boot process. So during l2x0 initialization,
> > > need to unlock the ways to make l2x0 could cache data/inst.
> > 
> > Why was this chosen instead of doing what everyone else does?
> 
> I am not aware of how other platform handles the l2x0 unlock in non
> secure mode. Could you please share with me what others choose?

That's not what I was asking.

Everyone else provides a way for the l2x0 controller to be enabled and
disabled from non-secure mode.

Why have you decided to enable the l2x0 controller and leave it enabled,
and then lock down all the cache ways - which means you need the kernel
to do something entirely different for your platform.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-27  9:19     ` Russell King - ARM Linux
@ 2017-11-27  9:43       ` Peng Fan
  2017-11-27 10:19         ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Peng Fan @ 2017-11-27  9:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> Sent: Monday, November 27, 2017 5:20 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> van.freenix at gmail.com; Mark Rutland <mark.rutland@arm.com>; Thomas
> Gleixner <tglx@linutronix.de>; Chris Brandt <chris.brandt@renesas.com>; Will
> Deacon <will.deacon@arm.com>
> Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> 
> On Sun, Nov 26, 2017 at 11:56:10PM +0000, Peng Fan wrote:
> > Hi Russell,
> >
> > > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> > >
> > > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > > > To boot Linux in Non-secure mode with l2x0, the l2x0 controller is
> > > > enabled in secure mode and ways locked to make it seems L2 cache
> > > > disabled during linux boot process. So during l2x0 initialization,
> > > > need to unlock the ways to make l2x0 could cache data/inst.
> > >
> > > Why was this chosen instead of doing what everyone else does?
> >
> > I am not aware of how other platform handles the l2x0 unlock in non
> > secure mode. Could you please share with me what others choose?
> 
> That's not what I was asking.
> 
> Everyone else provides a way for the l2x0 controller to be enabled and disabled
> from non-secure mode.

Thanks for the information. I see that some platforms implements l2c_write_sec.

> 
> Why have you decided to enable the l2x0 controller and leave it enabled, and
> then lock down all the cache ways - which means you need the kernel to do
> something entirely different for your platform.

Currently we are running OP-TEE on i.MX6/7 with Linux in non-secure mode. See
In https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L428
Pl310 is enabled. And In
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L461
pl310 locked before returning back to Linux.

I see ti platform not enabled pl310 in OP-TEE, leaving Linux to enable it. platform-sam/stm/ zynq7k/imx
Have pl310 enabled in OP-TEE.

I could switch to use l2c_write_sec dedicated for i.MX. But I think this patch is also a valid point.
What do you suggest?

Thanks,
Peng. 

> 
> --
> RMK's Patch system:
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=02%7C01%7Cpeng.fan%4
> 0nxp.com%7C35e84f76d1614037232008d535780198%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C636473711907495083&sdata=8aFy%2Fhi6hXQLyrlO
> 93uABaplCKM6QOP5n9DvX7S1uJ4%3D&reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-27  9:43       ` Peng Fan
@ 2017-11-27 10:19         ` Russell King - ARM Linux
  2017-11-28  1:57           ` Peng Fan
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2017-11-27 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 27, 2017 at 09:43:41AM +0000, Peng Fan wrote:
> Hi Russell,
> 
> > -----Original Message-----
> > From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> > Sent: Monday, November 27, 2017 5:20 PM
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> > van.freenix at gmail.com; Mark Rutland <mark.rutland@arm.com>; Thomas
> > Gleixner <tglx@linutronix.de>; Chris Brandt <chris.brandt@renesas.com>; Will
> > Deacon <will.deacon@arm.com>
> > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> > 
> > On Sun, Nov 26, 2017 at 11:56:10PM +0000, Peng Fan wrote:
> > > Hi Russell,
> > >
> > > > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> > > >
> > > > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > > > > To boot Linux in Non-secure mode with l2x0, the l2x0 controller is
> > > > > enabled in secure mode and ways locked to make it seems L2 cache
> > > > > disabled during linux boot process. So during l2x0 initialization,
> > > > > need to unlock the ways to make l2x0 could cache data/inst.
> > > >
> > > > Why was this chosen instead of doing what everyone else does?
> > >
> > > I am not aware of how other platform handles the l2x0 unlock in non
> > > secure mode. Could you please share with me what others choose?
> > 
> > That's not what I was asking.
> > 
> > Everyone else provides a way for the l2x0 controller to be enabled and disabled
> > from non-secure mode.
> 
> Thanks for the information. I see that some platforms implements l2c_write_sec.
> 
> > 
> > Why have you decided to enable the l2x0 controller and leave it enabled, and
> > then lock down all the cache ways - which means you need the kernel to do
> > something entirely different for your platform.
> 
> Currently we are running OP-TEE on i.MX6/7 with Linux in non-secure mode. See
> In https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L428
> Pl310 is enabled. And In
> https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L461
> pl310 locked before returning back to Linux.
> 
> I see ti platform not enabled pl310 in OP-TEE, leaving Linux to enable it. platform-sam/stm/ zynq7k/imx
> Have pl310 enabled in OP-TEE.
> 
> I could switch to use l2c_write_sec dedicated for i.MX. But I think this patch is also a valid point.
> What do you suggest?

What I'm concerned about is that there's a valid scenario where the L2
cache would be enabled and left enabled by the secure mode code - that
is if the secure mode wishes to take advantage of the L2 cache, and has
locked down some ways for its own use.

In this scenario, the secure world would have set the L2 cache up to
prevent the non-secure side unlocking those ways.  This would mean
that the NS_LOCKDOWN bit in the auxiliary control register would be
clear.  The PL310 TRM has this to say:

"On reset the Non-Secure Lockdown Enable bit is set to 0 and Lockdown
 Registers are not permitted to be modified by non-secure accesses. In
 that configuration, if a non-secure access tries to write to those
 registers, the write response returns a DECERR response."

which means that if we blindly try and unlock the ways, we will end
up triggering an exception, and that will crash the kernel.

Given that the kernel does _not_ handle this scenario today, I fail to
see why OP-TEE would decide that, on ARM by default, it will enable
the L2 cache and lock all ways.

As you have already found, at least OMAP has decided to do things
sensibly.  I fail to see why everyone else can't also decide to do the
sensible thing.

Please talk to the OP-TEE folk to see whether the OP-TEE behaviour can
be changed first.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-27 10:19         ` Russell King - ARM Linux
@ 2017-11-28  1:57           ` Peng Fan
  2017-12-03 11:20             ` Peng Fan
  0 siblings, 1 reply; 9+ messages in thread
From: Peng Fan @ 2017-11-28  1:57 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Russell,

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> Sent: Monday, November 27, 2017 6:19 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> van.freenix at gmail.com; Mark Rutland <mark.rutland@arm.com>; Thomas
> Gleixner <tglx@linutronix.de>; Chris Brandt <chris.brandt@renesas.com>; Will
> Deacon <will.deacon@arm.com>
> Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> 
> On Mon, Nov 27, 2017 at 09:43:41AM +0000, Peng Fan wrote:
> > Hi Russell,
> >
> > > -----Original Message-----
> > > From: Russell King - ARM Linux [mailto:linux at armlinux.org.uk]
> > > Sent: Monday, November 27, 2017 5:20 PM
> > > To: Peng Fan <peng.fan@nxp.com>
> > > Cc: linux-arm-kernel at lists.infradead.org;
> > > linux-kernel at vger.kernel.org; van.freenix at gmail.com; Mark Rutland
> > > <mark.rutland@arm.com>; Thomas Gleixner <tglx@linutronix.de>; Chris
> > > Brandt <chris.brandt@renesas.com>; Will Deacon <will.deacon@arm.com>
> > > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure mode
> > >
> > > On Sun, Nov 26, 2017 at 11:56:10PM +0000, Peng Fan wrote:
> > > > Hi Russell,
> > > >
> > > > > Subject: Re: [PATCH] arm: l2c: unlock ways when in non-secure
> > > > > mode
> > > > >
> > > > > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > > > > > To boot Linux in Non-secure mode with l2x0, the l2x0
> > > > > > controller is enabled in secure mode and ways locked to make
> > > > > > it seems L2 cache disabled during linux boot process. So
> > > > > > during l2x0 initialization, need to unlock the ways to make l2x0 could
> cache data/inst.
> > > > >
> > > > > Why was this chosen instead of doing what everyone else does?
> > > >
> > > > I am not aware of how other platform handles the l2x0 unlock in
> > > > non secure mode. Could you please share with me what others choose?
> > >
> > > That's not what I was asking.
> > >
> > > Everyone else provides a way for the l2x0 controller to be enabled
> > > and disabled from non-secure mode.
> >
> > Thanks for the information. I see that some platforms implements
> l2c_write_sec.
> >
> > >
> > > Why have you decided to enable the l2x0 controller and leave it
> > > enabled, and then lock down all the cache ways - which means you
> > > need the kernel to do something entirely different for your platform.
> >
> > Currently we are running OP-TEE on i.MX6/7 with Linux in non-secure
> > mode. See In
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > hub.com%2FOP-
> TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
> >
> el%2Fgeneric_entry_a32.S%23L428&data=02%7C01%7Cpeng.fan%40nxp.com%
> 7C32
> >
> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%
> >
> 7C0%7C636473747645673295&sdata=ZGaxxhs8mPNcqk5l2aSiStkPRFNxLzFFj45w
> kj%
> > 2Ff%2Fu4%3D&reserved=0
> > Pl310 is enabled. And In
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > hub.com%2FOP-
> TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
> >
> el%2Fgeneric_entry_a32.S%23L461&data=02%7C01%7Cpeng.fan%40nxp.com%
> 7C32
> >
> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%
> >
> 7C0%7C636473747645673295&sdata=rO1LG3639lfclvtgzZRTTPcSAGDQNG0Clqb
> D1wC
> > 4wGk%3D&reserved=0
> > pl310 locked before returning back to Linux.
> >
> > I see ti platform not enabled pl310 in OP-TEE, leaving Linux to enable
> > it. platform-sam/stm/ zynq7k/imx Have pl310 enabled in OP-TEE.
> >
> > I could switch to use l2c_write_sec dedicated for i.MX. But I think this patch is
> also a valid point.
> > What do you suggest?
> 
> What I'm concerned about is that there's a valid scenario where the L2 cache
> would be enabled and left enabled by the secure mode code - that is if the
> secure mode wishes to take advantage of the L2 cache, and has locked down
> some ways for its own use.
> 
> In this scenario, the secure world would have set the L2 cache up to prevent
> the non-secure side unlocking those ways.  This would mean that the
> NS_LOCKDOWN bit in the auxiliary control register would be clear.  The PL310
> TRM has this to say:
> 
> "On reset the Non-Secure Lockdown Enable bit is set to 0 and Lockdown
> Registers are not permitted to be modified by non-secure accesses. In  that
> configuration, if a non-secure access tries to write to those  registers, the write
> response returns a DECERR response."
> 
> which means that if we blindly try and unlock the ways, we will end up
> triggering an exception, and that will crash the kernel.

Currently, we set auxiliary control register to let NS could unlock. BIT26 set to 1.
But you bring a valid point is if TEE would like to lock down some ways for its own
use, l2c_write_sec should be used, to avoid Linux to directly unlock.

> 
> Given that the kernel does _not_ handle this scenario today, I fail to see why
> OP-TEE would decide that, on ARM by default, it will enable the L2 cache and
> lock all ways.
> 
> As you have already found, at least OMAP has decided to do things sensibly.  I
> fail to see why everyone else can't also decide to do the sensible thing.

Most platforms just set BIT26 to allow non-secure unlock ways without considering
reserving ways dedicated to TEE.

i.MX also has BIT26 set, so if l2c_init is not a good place, do you think moving unlock
to imx_init_l2cache is ok? But this means "enabling(unlock)" L2C earlier which is before l2c_init

> 
> Please talk to the OP-TEE folk to see whether the OP-TEE behaviour can be
> changed first.

+OP-TEE maintainers Etienne, Jens
Do you have comments on this?

Thanks,
Peng.

> 
> --
> RMK's Patch system:
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=02%7C01%7Cpeng.fan%4
> 0nxp.com%7C32e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C636473747645673295&sdata=4i8a6dYNkHlMXiYQZ
> N9Ej4b68q%2FZfMCZvIUfJtFy0Jc%3D&reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-11-28  1:57           ` Peng Fan
@ 2017-12-03 11:20             ` Peng Fan
  2017-12-05 16:03               ` Etienne Carriere
  0 siblings, 1 reply; 9+ messages in thread
From: Peng Fan @ 2017-12-03 11:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

> > > > > >
> > > > > > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
> > > > > > > To boot Linux in Non-secure mode with l2x0, the l2x0
> > > > > > > controller is enabled in secure mode and ways locked to make
> > > > > > > it seems L2 cache disabled during linux boot process. So
> > > > > > > during l2x0 initialization, need to unlock the ways to make
> > > > > > > l2x0 could
> > cache data/inst.
> > > > > >
> > > > > > Why was this chosen instead of doing what everyone else does?
> > > > >
> > > > > I am not aware of how other platform handles the l2x0 unlock in
> > > > > non secure mode. Could you please share with me what others choose?
> > > >
> > > > That's not what I was asking.
> > > >
> > > > Everyone else provides a way for the l2x0 controller to be enabled
> > > > and disabled from non-secure mode.
> > >
> > > Thanks for the information. I see that some platforms implements
> > l2c_write_sec.
> > >
> > > >
> > > > Why have you decided to enable the l2x0 controller and leave it
> > > > enabled, and then lock down all the cache ways - which means you
> > > > need the kernel to do something entirely different for your platform.
> > >
> > > Currently we are running OP-TEE on i.MX6/7 with Linux in non-secure
> > > mode. See In
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > > it
> > > hub.com%2FOP-
> > TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
> > >
> > el%2Fgeneric_entry_a32.S%23L428&data=02%7C01%7Cpeng.fan%40nxp.com%
> > 7C32
> > >
> >
> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> > C0%
> > >
> >
> 7C0%7C636473747645673295&sdata=ZGaxxhs8mPNcqk5l2aSiStkPRFNxLzFFj45w
> > kj%
> > > 2Ff%2Fu4%3D&reserved=0
> > > Pl310 is enabled. And In
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > > it
> > > hub.com%2FOP-
> > TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
> > >
> > el%2Fgeneric_entry_a32.S%23L461&data=02%7C01%7Cpeng.fan%40nxp.com%
> > 7C32
> > >
> >
> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> > C0%
> > >
> >
> 7C0%7C636473747645673295&sdata=rO1LG3639lfclvtgzZRTTPcSAGDQNG0Clqb
> > D1wC
> > > 4wGk%3D&reserved=0
> > > pl310 locked before returning back to Linux.
> > >
> > > I see ti platform not enabled pl310 in OP-TEE, leaving Linux to
> > > enable it. platform-sam/stm/ zynq7k/imx Have pl310 enabled in OP-TEE.
> > >
> > > I could switch to use l2c_write_sec dedicated for i.MX. But I think
> > > this patch is
> > also a valid point.
> > > What do you suggest?
> >
> > What I'm concerned about is that there's a valid scenario where the L2
> > cache would be enabled and left enabled by the secure mode code - that
> > is if the secure mode wishes to take advantage of the L2 cache, and
> > has locked down some ways for its own use.
> >
> > In this scenario, the secure world would have set the L2 cache up to
> > prevent the non-secure side unlocking those ways.  This would mean
> > that the NS_LOCKDOWN bit in the auxiliary control register would be
> > clear.  The PL310 TRM has this to say:
> >
> > "On reset the Non-Secure Lockdown Enable bit is set to 0 and Lockdown
> > Registers are not permitted to be modified by non-secure accesses. In
> > that configuration, if a non-secure access tries to write to those
> > registers, the write response returns a DECERR response."
> >
> > which means that if we blindly try and unlock the ways, we will end up
> > triggering an exception, and that will crash the kernel.

Just have a follow up question. If implementing l2c_write_sec, the kernel image
could not  only running in non-secure world. If we want the image to support
running in secure and non-secure world, do you have any suggestions about
the l2c things?

Thanks,
Peng.

> 
> Currently, we set auxiliary control register to let NS could unlock. BIT26 set to 1.
> But you bring a valid point is if TEE would like to lock down some ways for its
> own use, l2c_write_sec should be used, to avoid Linux to directly unlock.
> 
> >
> > Given that the kernel does _not_ handle this scenario today, I fail to
> > see why OP-TEE would decide that, on ARM by default, it will enable
> > the L2 cache and lock all ways.
> >
> > As you have already found, at least OMAP has decided to do things
> > sensibly.  I fail to see why everyone else can't also decide to do the sensible
> thing.
> 
> Most platforms just set BIT26 to allow non-secure unlock ways without
> considering reserving ways dedicated to TEE.
> 
> i.MX also has BIT26 set, so if l2c_init is not a good place, do you think moving
> unlock to imx_init_l2cache is ok? But this means "enabling(unlock)" L2C earlier
> which is before l2c_init
> 
> >
> > Please talk to the OP-TEE folk to see whether the OP-TEE behaviour can
> > be changed first.
> 
> +OP-TEE maintainers Etienne, Jens
> Do you have comments on this?
> 
> Thanks,
> Peng.
> 
> >
> > --
> > RMK's Patch system:
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> >
> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=02%7C01%7Cpeng.fan%4
> >
> 0nxp.com%7C32e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92c
> >
> d99c5c301635%7C0%7C0%7C636473747645673295&sdata=4i8a6dYNkHlMXiYQZ
> > N9Ej4b68q%2FZfMCZvIUfJtFy0Jc%3D&reserved=0
> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down
> > 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] arm: l2c: unlock ways when in non-secure mode
  2017-12-03 11:20             ` Peng Fan
@ 2017-12-05 16:03               ` Etienne Carriere
  0 siblings, 0 replies; 9+ messages in thread
From: Etienne Carriere @ 2017-12-05 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 3 December 2017 at 12:20, Peng Fan <peng.fan@nxp.com> wrote:
> Hi Russell,
>
>> > > > > >
>> > > > > > On Sun, Nov 26, 2017 at 08:25:30PM +0800, Peng Fan wrote:
>> > > > > > > To boot Linux in Non-secure mode with l2x0, the l2x0
>> > > > > > > controller is enabled in secure mode and ways locked to make
>> > > > > > > it seems L2 cache disabled during linux boot process. So
>> > > > > > > during l2x0 initialization, need to unlock the ways to make
>> > > > > > > l2x0 could
>> > cache data/inst.
>> > > > > >
>> > > > > > Why was this chosen instead of doing what everyone else does?
>> > > > >
>> > > > > I am not aware of how other platform handles the l2x0 unlock in
>> > > > > non secure mode. Could you please share with me what others choose?
>> > > >
>> > > > That's not what I was asking.
>> > > >
>> > > > Everyone else provides a way for the l2x0 controller to be enabled
>> > > > and disabled from non-secure mode.
>> > >
>> > > Thanks for the information. I see that some platforms implements
>> > l2c_write_sec.
>> > >
>> > > >
>> > > > Why have you decided to enable the l2x0 controller and leave it
>> > > > enabled, and then lock down all the cache ways - which means you
>> > > > need the kernel to do something entirely different for your platform.
>> > >
>> > > Currently we are running OP-TEE on i.MX6/7 with Linux in non-secure
>> > > mode. See In
>> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
>> > > it
>> > > hub.com%2FOP-
>> > TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
>> > >
>> > el%2Fgeneric_entry_a32.S%23L428&data=02%7C01%7Cpeng.fan%40nxp.com%
>> > 7C32
>> > >
>> >
>> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
>> > C0%
>> > >
>> >
>> 7C0%7C636473747645673295&sdata=ZGaxxhs8mPNcqk5l2aSiStkPRFNxLzFFj45w
>> > kj%
>> > > 2Ff%2Fu4%3D&reserved=0
>> > > Pl310 is enabled. And In
>> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
>> > > it
>> > > hub.com%2FOP-
>> > TEE%2Foptee_os%2Fblob%2Fmaster%2Fcore%2Farch%2Farm%2Fkern
>> > >
>> > el%2Fgeneric_entry_a32.S%23L461&data=02%7C01%7Cpeng.fan%40nxp.com%
>> > 7C32
>> > >
>> >
>> e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
>> > C0%
>> > >
>> >
>> 7C0%7C636473747645673295&sdata=rO1LG3639lfclvtgzZRTTPcSAGDQNG0Clqb
>> > D1wC
>> > > 4wGk%3D&reserved=0
>> > > pl310 locked before returning back to Linux.
>> > >
>> > > I see ti platform not enabled pl310 in OP-TEE, leaving Linux to
>> > > enable it. platform-sam/stm/ zynq7k/imx Have pl310 enabled in OP-TEE.
>> > >
>> > > I could switch to use l2c_write_sec dedicated for i.MX. But I think
>> > > this patch is
>> > also a valid point.
>> > > What do you suggest?
>> >
>> > What I'm concerned about is that there's a valid scenario where the L2
>> > cache would be enabled and left enabled by the secure mode code - that
>> > is if the secure mode wishes to take advantage of the L2 cache, and
>> > has locked down some ways for its own use.
>> >
>> > In this scenario, the secure world would have set the L2 cache up to
>> > prevent the non-secure side unlocking those ways.  This would mean
>> > that the NS_LOCKDOWN bit in the auxiliary control register would be
>> > clear.  The PL310 TRM has this to say:
>> >
>> > "On reset the Non-Secure Lockdown Enable bit is set to 0 and Lockdown
>> > Registers are not permitted to be modified by non-secure accesses. In
>> > that configuration, if a non-secure access tries to write to those
>> > registers, the write response returns a DECERR response."
>> >
>> > which means that if we blindly try and unlock the ways, we will end up
>> > triggering an exception, and that will crash the kernel.
>
> Just have a follow up question. If implementing l2c_write_sec, the kernel image
> could not  only running in non-secure world. If we want the image to support
> running in secure and non-secure world, do you have any suggestions about
> the l2c things?
>
> Thanks,
> Peng.
>

The weird part is that there is no HW means on ARM cores for a
privileged (or not) execution level to know whether it runs in secure
or non-secure state.

Moreover, the split between secure and non-secure wolrd is really
related the the boot stages.
A boot scheme can decide to split secure/non-secure world (i.e run
optee in secure and linux in non-secure), or can decide to boot the
Linux kernel in secure state.

So, IMO this is pure SW configuration. How to handle that?
Static built-in configuration of the Kernel is not flexible enough.
Device tree (or kernel cmdline) could be mean for the boot stage to
inform the Linux kernel on its execution state.

>>
>> Currently, we set auxiliary control register to let NS could unlock. BIT26 set to 1.
>> But you bring a valid point is if TEE would like to lock down some ways for its
>> own use, l2c_write_sec should be used, to avoid Linux to directly unlock.
>>
>> >
>> > Given that the kernel does _not_ handle this scenario today, I fail to
>> > see why OP-TEE would decide that, on ARM by default, it will enable
>> > the L2 cache and lock all ways.
>> >
>> > As you have already found, at least OMAP has decided to do things
>> > sensibly.  I fail to see why everyone else can't also decide to do the sensible
>> thing.
>>
>> Most platforms just set BIT26 to allow non-secure unlock ways without
>> considering reserving ways dedicated to TEE.
>>
>> i.MX also has BIT26 set, so if l2c_init is not a good place, do you think moving
>> unlock to imx_init_l2cache is ok? But this means "enabling(unlock)" L2C earlier
>> which is before l2c_init
>>
>> >
>> > Please talk to the OP-TEE folk to see whether the OP-TEE behaviour can
>> > be changed first.
>>
>> +OP-TEE maintainers Etienne, Jens
>> Do you have comments on this?
>>

The current OP-TEE implementation choose this 'locked/invalidated'
state for the L2 cache because it is quite easy to integrate and
currently fits the OP-TEE needs.
Not a very convincing argument I must admit.
A cleaner implementation would rather require the secure side to
provide an API for the Linux kernel the enable/disable/configure the
L2.

Some standardized ARM SMC would be nice for that. Otherwise, each
platform must provide it own means.

Note however, that if the Linux kernel relies on some SMC to access L2
cache services, this expects there is a SMC handler (aka secure
monitor) installed, which may not be the case if the bootloader boots
the Linux kernel without installing such a secure monitor: the
execution of the SMC instruction would likely to crash the kernel.
This is about the same as for the PSCI support.

Regards,
etienne

>> Thanks,
>> Peng.
>>
>> >
>> > --
>> > RMK's Patch system:
>> >
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
>> >
>> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=02%7C01%7Cpeng.fan%4
>> >
>> 0nxp.com%7C32e10e1e643f4def0d9508d535805486%7C686ea1d3bc2b4c6fa92c
>> >
>> d99c5c301635%7C0%7C0%7C636473747645673295&sdata=4i8a6dYNkHlMXiYQZ
>> > N9Ej4b68q%2FZfMCZvIUfJtFy0Jc%3D&reserved=0
>> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down
>> > 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-12-05 16:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-26 12:25 [PATCH] arm: l2c: unlock ways when in non-secure mode Peng Fan
2017-11-26 13:18 ` Russell King - ARM Linux
2017-11-26 23:56   ` Peng Fan
2017-11-27  9:19     ` Russell King - ARM Linux
2017-11-27  9:43       ` Peng Fan
2017-11-27 10:19         ` Russell King - ARM Linux
2017-11-28  1:57           ` Peng Fan
2017-12-03 11:20             ` Peng Fan
2017-12-05 16:03               ` Etienne Carriere

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).