diff mbox

[v4,REPOST,3/5] omap4: Unconditionally require l2x0 L2 cache controller support

Message ID 1323862781-3465-4-git-send-email-dave.martin@linaro.org
State Superseded
Headers show

Commit Message

Dave Martin Dec. 14, 2011, 11:39 a.m. UTC
If running in the Normal World on a TrustZone-enabled SoC, Linux
does not have complete control over the L2 cache controller
configuration.  The kernel cannot work reliably on such platforms
without the l2x0 cache support code built in.

This patch unconditionally enables l2x0 support for the OMAP4 SoCs.

Thanks to Rob Herring for this suggestion. [1]

Signed-off-by: Dave Martin <dave.martin@linaro.org>

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074495.html
---
 arch/arm/mach-omap2/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Tony Lindgren Dec. 14, 2011, 6:14 p.m. UTC | #1
* Dave Martin <dave.martin@linaro.org> [111214 03:08]:
> If running in the Normal World on a TrustZone-enabled SoC, Linux
> does not have complete control over the L2 cache controller
> configuration.  The kernel cannot work reliably on such platforms
> without the l2x0 cache support code built in.

There are HS and GP omaps (High Security and General Purpose).
GP omaps do have full control of the L2. Also HS omaps most likely
provide control over enabling and disabling L2 depending how the
secure code is implemented.

BTW, the real problem is that because the secure code is implemented
in various ways, we don't really have any handling for it in Linux.

The SMI instruction numbers don't seem to be standardized at all,
and can mean different things on different boards, even different
board versions :(

Sounds like devicetree is the only safe way to deal with the L2
control options.

Regards,

Tony

 
> This patch unconditionally enables l2x0 support for the OMAP4 SoCs.
> 
> Thanks to Rob Herring for this suggestion. [1]
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074495.html
> ---
>  arch/arm/mach-omap2/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index bb1b670..94e568a 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -41,11 +41,11 @@ config ARCH_OMAP4
>  	bool "TI OMAP4"
>  	default y
>  	depends on ARCH_OMAP2PLUS
> +	select CACHE_L2X0
>  	select CPU_V7
>  	select ARM_GIC
>  	select HAVE_SMP
>  	select LOCAL_TIMERS if SMP
> -	select MIGHT_HAVE_CACHE_L2X0
>  	select PL310_ERRATA_588369
>  	select PL310_ERRATA_727915
>  	select ARM_ERRATA_720789
> -- 
> 1.7.4.1
>
Dave Martin Dec. 14, 2011, 6:30 p.m. UTC | #2
On Wed, Dec 14, 2011 at 10:14:25AM -0800, Tony Lindgren wrote:
> * Dave Martin <dave.martin@linaro.org> [111214 03:08]:
> > If running in the Normal World on a TrustZone-enabled SoC, Linux
> > does not have complete control over the L2 cache controller
> > configuration.  The kernel cannot work reliably on such platforms
> > without the l2x0 cache support code built in.
> 
> There are HS and GP omaps (High Security and General Purpose).
> GP omaps do have full control of the L2. Also HS omaps most likely
> provide control over enabling and disabling L2 depending how the
> secure code is implemented.
> 
> BTW, the real problem is that because the secure code is implemented
> in various ways, we don't really have any handling for it in Linux.
> 
> The SMI instruction numbers don't seem to be standardized at all,
> and can mean different things on different boards, even different
> board versions :(
> 
> Sounds like devicetree is the only safe way to deal with the L2
> control options.
> 
> Regards,
> 
> Tony
> 
>  
> > This patch unconditionally enables l2x0 support for the OMAP4 SoCs.
> > 
> > Thanks to Rob Herring for this suggestion. [1]
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > 
> > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074495.html
> > ---
> >  arch/arm/mach-omap2/Kconfig |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index bb1b670..94e568a 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -41,11 +41,11 @@ config ARCH_OMAP4
> >  	bool "TI OMAP4"
> >  	default y
> >  	depends on ARCH_OMAP2PLUS
> > +	select CACHE_L2X0
> >  	select CPU_V7
> >  	select ARM_GIC
> >  	select HAVE_SMP
> >  	select LOCAL_TIMERS if SMP
> > -	select MIGHT_HAVE_CACHE_L2X0
> >  	select PL310_ERRATA_588369
> >  	select PL310_ERRATA_727915
> >  	select ARM_ERRATA_720789
> > -- 
> > 1.7.4.1
> > 

To clarify, are you suggesting we retain this patch, or not?

(If we only know what l2x0 support will be needed once the dts is
parsed at runtime, there could be an argument for keeping the
select CACHE_L2X0 here -- unless we have specific kconfigs for
the different security variants of omap.)

Cheers
---Dave
Tony Lindgren Dec. 14, 2011, 6:39 p.m. UTC | #3
* Dave Martin <dave.martin@linaro.org> [111214 09:58]:
> On Wed, Dec 14, 2011 at 10:14:25AM -0800, Tony Lindgren wrote:
> > * Dave Martin <dave.martin@linaro.org> [111214 03:08]:
> > > If running in the Normal World on a TrustZone-enabled SoC, Linux
> > > does not have complete control over the L2 cache controller
> > > configuration.  The kernel cannot work reliably on such platforms
> > > without the l2x0 cache support code built in.
> > 
> > There are HS and GP omaps (High Security and General Purpose).
> > GP omaps do have full control of the L2. Also HS omaps most likely
> > provide control over enabling and disabling L2 depending how the
> > secure code is implemented.
> > 
> > BTW, the real problem is that because the secure code is implemented
> > in various ways, we don't really have any handling for it in Linux.
> > 
> > The SMI instruction numbers don't seem to be standardized at all,
> > and can mean different things on different boards, even different
> > board versions :(
> > 
> > Sounds like devicetree is the only safe way to deal with the L2
> > control options.
> > 
> > Regards,
> > 
> > Tony
> > 
> >  
> > > This patch unconditionally enables l2x0 support for the OMAP4 SoCs.
> > > 
> > > Thanks to Rob Herring for this suggestion. [1]
> > > 
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > > 
> > > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074495.html
> > > ---
> > >  arch/arm/mach-omap2/Kconfig |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > > index bb1b670..94e568a 100644
> > > --- a/arch/arm/mach-omap2/Kconfig
> > > +++ b/arch/arm/mach-omap2/Kconfig
> > > @@ -41,11 +41,11 @@ config ARCH_OMAP4
> > >  	bool "TI OMAP4"
> > >  	default y
> > >  	depends on ARCH_OMAP2PLUS
> > > +	select CACHE_L2X0
> > >  	select CPU_V7
> > >  	select ARM_GIC
> > >  	select HAVE_SMP
> > >  	select LOCAL_TIMERS if SMP
> > > -	select MIGHT_HAVE_CACHE_L2X0
> > >  	select PL310_ERRATA_588369
> > >  	select PL310_ERRATA_727915
> > >  	select ARM_ERRATA_720789
> > > -- 
> > > 1.7.4.1
> > > 
> 
> To clarify, are you suggesting we retain this patch, or not?

I think we should keep L2 configurable for omaps until we have some
way of getting the configuration dynamically or from device tree.
 
> (If we only know what l2x0 support will be needed once the dts is
> parsed at runtime, there could be an argument for keeping the
> select CACHE_L2X0 here -- unless we have specific kconfigs for
> the different security variants of omap.)

Well we can detect if it's an HS omap, but we may not know what
SMI it uses for enabling and disabling L2.. That can depend on
the board version.

Is there some problem keeping MIGHT_HAVE_CACHE_L2X0? This is
pretty important from debugging cache issues point of view.

Regards,

Tony
Rob Herring Dec. 14, 2011, 9:01 p.m. UTC | #4
On 12/14/2011 12:39 PM, Tony Lindgren wrote:
> * Dave Martin <dave.martin@linaro.org> [111214 09:58]:
>> On Wed, Dec 14, 2011 at 10:14:25AM -0800, Tony Lindgren wrote:
>>> * Dave Martin <dave.martin@linaro.org> [111214 03:08]:
>>>> If running in the Normal World on a TrustZone-enabled SoC, Linux
>>>> does not have complete control over the L2 cache controller
>>>> configuration.  The kernel cannot work reliably on such platforms
>>>> without the l2x0 cache support code built in.
>>>
>>> There are HS and GP omaps (High Security and General Purpose).
>>> GP omaps do have full control of the L2. Also HS omaps most likely
>>> provide control over enabling and disabling L2 depending how the
>>> secure code is implemented.
>>>
>>> BTW, the real problem is that because the secure code is implemented
>>> in various ways, we don't really have any handling for it in Linux.
>>>
>>> The SMI instruction numbers don't seem to be standardized at all,
>>> and can mean different things on different boards, even different
>>> board versions :(
>>>
>>> Sounds like devicetree is the only safe way to deal with the L2
>>> control options.
>>>
>>> Regards,
>>>
>>> Tony
>>>
>>>  
>>>> This patch unconditionally enables l2x0 support for the OMAP4 SoCs.
>>>>
>>>> Thanks to Rob Herring for this suggestion. [1]
>>>>
>>>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>>>
>>>> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-November/074495.html
>>>> ---
>>>>  arch/arm/mach-omap2/Kconfig |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>>>> index bb1b670..94e568a 100644
>>>> --- a/arch/arm/mach-omap2/Kconfig
>>>> +++ b/arch/arm/mach-omap2/Kconfig
>>>> @@ -41,11 +41,11 @@ config ARCH_OMAP4
>>>>  	bool "TI OMAP4"
>>>>  	default y
>>>>  	depends on ARCH_OMAP2PLUS
>>>> +	select CACHE_L2X0
>>>>  	select CPU_V7
>>>>  	select ARM_GIC
>>>>  	select HAVE_SMP
>>>>  	select LOCAL_TIMERS if SMP
>>>> -	select MIGHT_HAVE_CACHE_L2X0
>>>>  	select PL310_ERRATA_588369
>>>>  	select PL310_ERRATA_727915
>>>>  	select ARM_ERRATA_720789
>>>> -- 
>>>> 1.7.4.1
>>>>
>>
>> To clarify, are you suggesting we retain this patch, or not?
> 
> I think we should keep L2 configurable for omaps until we have some
> way of getting the configuration dynamically or from device tree.
>  

This already exists with l2x0_of_init. OMAP just needs to start using
it. It will initialize the l2 if present in the DT and skip it if not
present.

Rob

>> (If we only know what l2x0 support will be needed once the dts is
>> parsed at runtime, there could be an argument for keeping the
>> select CACHE_L2X0 here -- unless we have specific kconfigs for
>> the different security variants of omap.)
> 
> Well we can detect if it's an HS omap, but we may not know what
> SMI it uses for enabling and disabling L2.. That can depend on
> the board version.
> 
> Is there some problem keeping MIGHT_HAVE_CACHE_L2X0? This is
> pretty important from debugging cache issues point of view.
> 
> Regards,
> 
> Tony
Tony Lindgren Dec. 14, 2011, 9:48 p.m. UTC | #5
* Rob Herring <robherring2@gmail.com> [111214 12:30]:
> 
> On 12/14/2011 12:39 PM, Tony Lindgren wrote:
> > 
> > I think we should keep L2 configurable for omaps until we have some
> > way of getting the configuration dynamically or from device tree.
> >  
> 
> This already exists with l2x0_of_init. OMAP just needs to start using
> it. It will initialize the l2 if present in the DT and skip it if not
> present.

That's great, that will allow doing the right thing with setting
up how to enable and disable it :) Considering that, this patch
should be OK to apply:

Acked-by: Tony Lindgren <tony@atomide.com>
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index bb1b670..94e568a 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -41,11 +41,11 @@  config ARCH_OMAP4
 	bool "TI OMAP4"
 	default y
 	depends on ARCH_OMAP2PLUS
+	select CACHE_L2X0
 	select CPU_V7
 	select ARM_GIC
 	select HAVE_SMP
 	select LOCAL_TIMERS if SMP
-	select MIGHT_HAVE_CACHE_L2X0
 	select PL310_ERRATA_588369
 	select PL310_ERRATA_727915
 	select ARM_ERRATA_720789