RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-18 Thread Santosh Shilimkar 
 -Original Message-
 From: catalin.mari...@gmail.com [mailto:catalin.mari...@gmail.com]
 On Behalf Of Catalin Marinas
 Sent: Wednesday, February 16, 2011 9:24 PM
 To: Santosh Shilimkar
 Cc: linux-arm-ker...@lists.infradead.org; Andrei Warkentin; Kevin
 Hilman; t...@atomide.com; linux-omap@vger.kernel.org
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 On 15 February 2011 07:14, Santosh Shilimkar
 santosh.shilim...@ti.com wrote:
  --- a/arch/arm/Kconfig
  +++ b/arch/arm/Kconfig
  @@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231
 
   config PL310_ERRATA_588369
         bool Clean  Invalidate maintenance operations do not
 invalidate
  clean lines
  -       depends on CACHE_L2X0  ARCH_OMAP4
  +       depends on CACHE_L2X0  CACHE_PL310

 It can just depend on CACHE_PL310 as this depends on CACHE_L2X0.

With CACHE_PL310 alone, I get below warning.

warning: (ARCH_OMAP4) selects PL310_ERRATA_588369 which has unmet direct
dependencies (CACHE_PL310)

This is coming because Pl310 can't be selected on V6 and OMAP
common build has V6 and V7 both enabled.

So to avoid the warning and also able to select the errata in
OMAP build, I am making the errata's depends on CACHE_L2X0
with comment.

Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-16 Thread Santosh Shilimkar 
Catalin,
 -Original Message-
 From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com]
 Sent: Tuesday, February 15, 2011 12:44 PM
 To: linux-arm-ker...@lists.infradead.org; Andrei Warkentin
 Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
 Catalin Marinas
 Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

  -Original Message-
  From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com]
  Sent: Monday, February 14, 2011 10:39 AM
  To: Andrei Warkentin
  Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
  linux-arm-ker...@lists.infradead.org; Catalin Marinas
  Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
  operation can cause data corruption
 

 []

   ...
  I understood that from first comment. But I am not in favor
  of polluting common ARM files with SOC specific #ifdeffery.
  We have gone over this when first errata support
  was added for PL310
 
  I have a better way to handle this scenario.
  Expect an updated patch for this.
 

 Below is the updated version which should remove any
 OMAP dependency on these errata's. Attached same.

 
 From: Santosh Shilimkar santosh.shilim...@ti.com
 Date: Fri, 14 Jan 2011 14:16:04 +0530
 Subject: [v2 PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation
 can cause data corruption

 PL310 implements the Clean  Invalidate by Way L2 cache maintenance
 operation (offset 0x7FC). This operation runs in background so that
 PL310 can handle normal accesses while it is in progress. Under very
 rare circumstances, due to this erratum, write data can be lost when
 PL310 treats a cacheable write transaction during a Clean 
 Invalidate
 by Way operation.

 Workaround:
 Disable Write-Back and Cache Linefill (Debug Control Register)
 Clean  Invalidate by Way (0x7FC)
 Re-enable Write-Back and Cache Linefill (Debug Control Register)

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Catalin Marinas catalin.mari...@arm.com
 ---

Ack , Nak ?

  arch/arm/Kconfig   |   13 -
  arch/arm/include/asm/outercache.h  |1 +
  arch/arm/mach-omap2/Kconfig|3 +++
  arch/arm/mach-omap2/omap4-common.c |7 +++
  arch/arm/mm/cache-l2x0.c   |   28 +++--
 ---
  5 files changed, 38 insertions(+), 14 deletions(-)

 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index 5cff165..ebadd95 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231

  config PL310_ERRATA_588369
   bool Clean  Invalidate maintenance operations do not
 invalidate
 clean lines
 - depends on CACHE_L2X0  ARCH_OMAP4
 + depends on CACHE_L2X0  CACHE_PL310
   help
  The PL310 L2 cache controller implements three types of
 Clean 
  Invalidate maintenance operations: by Physical Address
 @@ -1177,6 +1177,17 @@ config ARM_ERRATA_743622
 visible impact on the overall performance or power
 consumption
 of the
 processor.

 +config PL310_ERRATA_727915
 + bool Background Clean  Invalidate by Way operation can cause
 data corruption
 + depends on CACHE_L2X0  CACHE_PL310
 + help
 +   PL310 implements the Clean  Invalidate by Way L2 cache
 maintenance
 +   operation (offset 0x7FC). This operation runs in background
 so
 that
 +   PL310 can handle normal accesses while it is in progress.
 Under
 very
 +   rare circumstances, due to this erratum, write data can be
 lost
 when
 +   PL310 treats a cacheable write transaction during a Clean 
 +   Invalidate by Way operation Note that this errata uses Texas
 +   Instrument's secure monitor api to implement the work
 around.
  endmenu

  source arch/arm/common/Kconfig
 diff --git a/arch/arm/include/asm/outercache.h
 b/arch/arm/include/asm/outercache.h
 index fc19009..348d513 100644
 --- a/arch/arm/include/asm/outercache.h
 +++ b/arch/arm/include/asm/outercache.h
 @@ -31,6 +31,7 @@ struct outer_cache_fns {
  #ifdef CONFIG_OUTER_CACHE_SYNC
   void (*sync)(void);
  #endif
 + void (*set_debug)(unsigned long);
  };

  #ifdef CONFIG_OUTER_CACHE
 diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-
 omap2/Kconfig
 index f285dd7..fd11ab4 100644
 --- a/arch/arm/mach-omap2/Kconfig
 +++ b/arch/arm/mach-omap2/Kconfig
 @@ -45,7 +45,10 @@ config ARCH_OMAP4
   select CPU_V7
   select ARM_GIC
   select LOCAL_TIMERS
 + select CACHE_L2X0
 + select CACHE_PL310
   select PL310_ERRATA_588369
 + select PL310_ERRATA_727915
   select ARM_ERRATA_720789
   select ARCH_HAS_OPP
   select PM_OPP if PM
 diff --git a/arch/arm/mach-omap2/omap4-common.c
 b/arch/arm/mach-omap2/omap4-common.c
 index 1926864..9ef8c29 100644
 --- a/arch/arm/mach-omap2/omap4-common.c
 +++ b/arch/arm/mach-omap2/omap4-common.c
 @@ -52,6 +52,12 @@ static void omap4_l2x0_disable(void)
   omap_smc1(0x102, 0x0);
  }

 +static void omap4_l2x0_set_debug

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-16 Thread Catalin Marinas 
On 15 February 2011 07:14, Santosh Shilimkar santosh.shilim...@ti.com wrote:
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231

  config PL310_ERRATA_588369
        bool Clean  Invalidate maintenance operations do not invalidate
 clean lines
 -       depends on CACHE_L2X0  ARCH_OMAP4
 +       depends on CACHE_L2X0  CACHE_PL310

It can just depend on CACHE_PL310 as this depends on CACHE_L2X0.

 +config PL310_ERRATA_727915
 +       bool Background Clean  Invalidate by Way operation can cause
 data corruption
 +       depends on CACHE_L2X0  CACHE_PL310

Same here.

 --- a/arch/arm/mach-omap2/Kconfig
 +++ b/arch/arm/mach-omap2/Kconfig
 @@ -45,7 +45,10 @@ config ARCH_OMAP4
        select CPU_V7
        select ARM_GIC
        select LOCAL_TIMERS
 +       select CACHE_L2X0

CACHE_L2X0 has a long dependency list. You could add ARCH_OMAP4 in
there or just change the other platforms to select a HAVE_CACHE_L2X0.
Ideally we would like this option to be selectable in config just in
case you want to debug some issues.

 --- a/arch/arm/mach-omap2/omap4-common.c
 +++ b/arch/arm/mach-omap2/omap4-common.c
 @@ -52,6 +52,12 @@ static void omap4_l2x0_disable(void)
        omap_smc1(0x102, 0x0);
  }

 +static void omap4_l2x0_set_debug(unsigned long val)
 +{
 +       /* Program PL310 L2 Cache controller debug register */
 +       omap_smc1(0x100, val);
 +}

This part together with the Kconfig changes for OMAP4 could be a
separate patch, OMAP-specific.

The rest seems fine.

-- 
Catalin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-16 Thread Santosh Shilimkar 
 -Original Message-
 From: catalin.mari...@gmail.com [mailto:catalin.mari...@gmail.com]
 On Behalf Of Catalin Marinas
 Sent: Wednesday, February 16, 2011 9:24 PM
 To: Santosh Shilimkar
 Cc: linux-arm-ker...@lists.infradead.org; Andrei Warkentin; Kevin
 Hilman; t...@atomide.com; linux-omap@vger.kernel.org
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 On 15 February 2011 07:14, Santosh Shilimkar
 santosh.shilim...@ti.com wrote:
  --- a/arch/arm/Kconfig
  +++ b/arch/arm/Kconfig
  @@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231
 
   config PL310_ERRATA_588369
         bool Clean  Invalidate maintenance operations do not
 invalidate
  clean lines
  -       depends on CACHE_L2X0  ARCH_OMAP4
  +       depends on CACHE_L2X0  CACHE_PL310

 It can just depend on CACHE_PL310 as this depends on CACHE_L2X0.

Ok.
  +config PL310_ERRATA_727915
  +       bool Background Clean  Invalidate by Way operation can
 cause
  data corruption
  +       depends on CACHE_L2X0  CACHE_PL310

 Same here.

  --- a/arch/arm/mach-omap2/Kconfig
  +++ b/arch/arm/mach-omap2/Kconfig
  @@ -45,7 +45,10 @@ config ARCH_OMAP4
         select CPU_V7
         select ARM_GIC
         select LOCAL_TIMERS
  +       select CACHE_L2X0

 CACHE_L2X0 has a long dependency list. You could add ARCH_OMAP4 in
 there or just change the other platforms to select a
 HAVE_CACHE_L2X0.
 Ideally we would like this option to be selectable in config just in
 case you want to debug some issues.

I will add ARCH_OMAP4 under CACHE_L2X0.

  --- a/arch/arm/mach-omap2/omap4-common.c
  +++ b/arch/arm/mach-omap2/omap4-common.c
  @@ -52,6 +52,12 @@ static void omap4_l2x0_disable(void)
         omap_smc1(0x102, 0x0);
   }
 
  +static void omap4_l2x0_set_debug(unsigned long val)
  +{
  +       /* Program PL310 L2 Cache controller debug register */
  +       omap_smc1(0x100, val);
  +}

 This part together with the Kconfig changes for OMAP4 could be a
 separate patch, OMAP-specific.

Agree. I will split this patch and repost.

 The rest seems fine.

Thanks for the feedback.

Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-15 Thread Andrei Warkentin 
On Tue, Feb 15, 2011 at 1:14 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 -Original Message-
 From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com]
 Sent: Monday, February 14, 2011 10:39 AM
 To: Andrei Warkentin
 Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption


 []

  ...
 I understood that from first comment. But I am not in favor
 of polluting common ARM files with SOC specific #ifdeffery.
 We have gone over this when first errata support
 was added for PL310

 I have a better way to handle this scenario.
 Expect an updated patch for this.


 Below is the updated version which should remove any
 OMAP dependency on these errata's. Attached same.

 
 From: Santosh Shilimkar santosh.shilim...@ti.com
 Date: Fri, 14 Jan 2011 14:16:04 +0530
 Subject: [v2 PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation
 can cause data corruption

 PL310 implements the Clean  Invalidate by Way L2 cache maintenance
 operation (offset 0x7FC). This operation runs in background so that
 PL310 can handle normal accesses while it is in progress. Under very
 rare circumstances, due to this erratum, write data can be lost when
 PL310 treats a cacheable write transaction during a Clean  Invalidate
 by Way operation.

 Workaround:
 Disable Write-Back and Cache Linefill (Debug Control Register)
 Clean  Invalidate by Way (0x7FC)
 Re-enable Write-Back and Cache Linefill (Debug Control Register)

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Catalin Marinas catalin.mari...@arm.com
 ---
  arch/arm/Kconfig                   |   13 -
  arch/arm/include/asm/outercache.h  |    1 +
  arch/arm/mach-omap2/Kconfig        |    3 +++
  arch/arm/mach-omap2/omap4-common.c |    7 +++
  arch/arm/mm/cache-l2x0.c           |   28 +++-
  5 files changed, 38 insertions(+), 14 deletions(-)

 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index 5cff165..ebadd95 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231

  config PL310_ERRATA_588369
        bool Clean  Invalidate maintenance operations do not invalidate
 clean lines
 -       depends on CACHE_L2X0  ARCH_OMAP4
 +       depends on CACHE_L2X0  CACHE_PL310
        help
           The PL310 L2 cache controller implements three types of Clean 
           Invalidate maintenance operations: by Physical Address
 @@ -1177,6 +1177,17 @@ config ARM_ERRATA_743622
          visible impact on the overall performance or power consumption
 of the
          processor.

 +config PL310_ERRATA_727915
 +       bool Background Clean  Invalidate by Way operation can cause
 data corruption
 +       depends on CACHE_L2X0  CACHE_PL310
 +       help
 +         PL310 implements the Clean  Invalidate by Way L2 cache
 maintenance
 +         operation (offset 0x7FC). This operation runs in background so
 that
 +         PL310 can handle normal accesses while it is in progress. Under
 very
 +         rare circumstances, due to this erratum, write data can be lost
 when
 +         PL310 treats a cacheable write transaction during a Clean 
 +         Invalidate by Way operation Note that this errata uses Texas
 +         Instrument's secure monitor api to implement the work around.
  endmenu

  source arch/arm/common/Kconfig
 diff --git a/arch/arm/include/asm/outercache.h
 b/arch/arm/include/asm/outercache.h
 index fc19009..348d513 100644
 --- a/arch/arm/include/asm/outercache.h
 +++ b/arch/arm/include/asm/outercache.h
 @@ -31,6 +31,7 @@ struct outer_cache_fns {
  #ifdef CONFIG_OUTER_CACHE_SYNC
        void (*sync)(void);
  #endif
 +       void (*set_debug)(unsigned long);
  };

  #ifdef CONFIG_OUTER_CACHE
 diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
 index f285dd7..fd11ab4 100644
 --- a/arch/arm/mach-omap2/Kconfig
 +++ b/arch/arm/mach-omap2/Kconfig
 @@ -45,7 +45,10 @@ config ARCH_OMAP4
        select CPU_V7
        select ARM_GIC
        select LOCAL_TIMERS
 +       select CACHE_L2X0
 +       select CACHE_PL310
        select PL310_ERRATA_588369
 +       select PL310_ERRATA_727915
        select ARM_ERRATA_720789
        select ARCH_HAS_OPP
        select PM_OPP if PM
 diff --git a/arch/arm/mach-omap2/omap4-common.c
 b/arch/arm/mach-omap2/omap4-common.c
 index 1926864..9ef8c29 100644
 --- a/arch/arm/mach-omap2/omap4-common.c
 +++ b/arch/arm/mach-omap2/omap4-common.c
 @@ -52,6 +52,12 @@ static void omap4_l2x0_disable(void)
        omap_smc1(0x102, 0x0);
  }

 +static void omap4_l2x0_set_debug(unsigned long val)
 +{
 +       /* Program PL310 L2 Cache controller debug register */
 +       omap_smc1(0x100, val);
 +}
 +
  static int __init omap_l2_cache_init(void)
  {
        u32 aux_ctrl = 0;
 @@ -99,6 +105,7 @@ static int __init omap_l2_cache_init(void)
         * specific one

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-15 Thread Santosh Shilimkar 
 -Original Message-
 From: Andrei Warkentin [mailto:andr...@motorola.com]
 Sent: Tuesday, February 15, 2011 2:40 PM
 To: Santosh Shilimkar
 Cc: linux-arm-ker...@lists.infradead.org; linux-
 o...@vger.kernel.org; Kevin Hilman; t...@atomide.com; Catalin
 Marinas
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 On Tue, Feb 15, 2011 at 1:14 AM, Santosh Shilimkar
 santosh.shilim...@ti.com wrote:
  -Original Message-
  From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com]
  Sent: Monday, February 14, 2011 10:39 AM
  To: Andrei Warkentin
  Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
  linux-arm-ker...@lists.infradead.org; Catalin Marinas
  Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
  operation can cause data corruption
 
 

[]

 

 Why set by default to NULL, why not have a normal l2x0_set_debug
 which
 just does a write to L2X0_DEBUG_CTRL, and then just invoke the
 function pointer when you wish to manipulate the DCR? That way you
 don't need the runtime check, and it's just clearer, I think.

I though about it. There more changes in the file and hence I
avoided it. This can be done though.

 Also, why not do something like -
 
 do_stuff();
 #ifdef CONFIG_NEED_ERRATA_1234
 do_errata_stuff();
 #endif
 do_more_stuff();
 ...

Which makes code completely ugly.

 instead of  -

 ...
 #ifdef CONFIG_NEED_ERRATA_1234
 do_some_stuf() {
 bar();
 }
 #else
 {
 do_some_stuff() {
 }
 // nothing
 }

We have already discussed this. The code becomes ugly. If you
are interested in the reasoning, please check archives.

Russell and Catalin has suggested above.

If you understand the errata in first place, you could
understand the comment.

I let Catalin, Russell comment on it more, but unnecessary
CONFIG options and polluting every function with #If, else
checks don't make sense. Rest of your comments are related
to this.

Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-14 Thread Andrei Warkentin 
On Sun, Feb 13, 2011 at 11:08 PM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 -Original Message-
 From: Andrei Warkentin [mailto:andr...@motorola.com]
 Sent: Sunday, February 13, 2011 4:48 AM
 To: Santosh Shilimkar
 Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 On Sat, Feb 12, 2011 at 11:59 AM, Santosh Shilimkar
 santosh.shilim...@ti.com wrote:
  -Original Message-
  From: Andrei Warkentin [mailto:andr...@motorola.com]
  Sent: Saturday, February 12, 2011 11:20 PM
  To: Santosh Shilimkar
  Cc: linux-omap@vger.kernel.org; khil...@ti.com; t...@atomide.com;
  linux-arm-ker...@lists.infradead.org; Catalin Marinas
  Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
  operation can cause data corruption
 
  []
 
 
  Can these PL310 errata fixes be made more generic? PL310 is
 present
  in
  non-OMAP platforms too, which lack the TI hypervisor. And these
  platforms might have the same PL310 rev, and suffer the same
  glitches.
  While ideally there is some kind of hypervisor_ops to modify the
  protected register, at the very least there should be the generic
  debug_write handling the  I  can write all PL310 regs case. If
  you're interested I have a patch someplace that tried to do this,
  hopefully I can still find it.
 
  They are kind of generic. If you look at it, the only change
  Which is arch specific is the implementation of debug_writel
 function.
  Today this code is not in generic PL310 code, but
  OMAP specific.
 
  May be we can make this as exported function pointer, which
  arch's can populate.
 
  Will that work for you ?
 
  Regards,
  Santosh
 

 Ie something like the following what do you think???

 #define L2X0_DCR (0xF40)

 static void debug_writel(unsigned long val)
 {
 #ifdef CONFIG_ARCH_OMAP4
        omap_smc1(0x100, val);
 #else
        writel_relaxed(val, l2x0_base + L2X0_DCR);
 #endif
 }
 ...
 I understood that from first comment. But I am not in favor
 of polluting common ARM files with SOC specific #ifdeffery.
 We have gone over this when first errata support
 was added for PL310

 I have a better way to handle this scenario.
 Expect an updated patch for this.

 Regards,
 Santosh


Fair enough, but you're doing it right now :-). I believe the smarter
approach would be to start abstracting all accesses to secure-only
resources (like the DCR reg). This would be your hypervisor
interface. Then provide an implementation for your TI secure monitor.
Obviously over time :).
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-14 Thread Andrei Warkentin 
On Mon, Feb 14, 2011 at 1:33 PM, Andrei Warkentin andr...@motorola.com wrote:

 Fair enough, but you're doing it right now :-). I believe the smarter
 approach would be to start abstracting all accesses to secure-only
 resources (like the DCR reg). This would be your hypervisor
 interface. Then provide an implementation for your TI secure monitor.
 Obviously over time :).


Santosh,

Maybe this can influence you in some ways. I have this old patch
sitting around, although it does #ifdef around the TI stuff, simply
because I wasn't really interested in moving it out.
I group the errata by revs so it's simpler to see what you need and
what you don't. Obviously there aren't that many, but it does provide
a pattern to follow...

Thanks,
A
From be41c69f48d5886c76148c2ad378dae78e590534 Mon Sep 17 00:00:00 2001
From: Andrei Warkentin andr...@motorola.com
Date: Mon, 14 Feb 2011 15:34:06 -0600
Subject: [PATCH] ARM PL310: Cleanup errata handling for cache controller.

Adds a revision option for PL310 cache controller. All errata
now depend on the revision selected. Picking a particular revision
results in selecting all required errata. CACHE_PL310_REV_UNKNOWN
is used when manual control is desired.

Signed-off-by: Andrei Warkentin andr...@motorola.com

Change-Id: I8804af5d7a476737ce259b5d6a2a132c50082d66
---
 arch/arm/Kconfig|   36 ---
 arch/arm/configs/omap_4430sdp_defconfig |5 ++-
 arch/arm/mm/Kconfig |   52 +++
 arch/arm/mm/cache-l2x0.c|   58 +++---
 4 files changed, 122 insertions(+), 29 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cd2c427..20af15f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1014,6 +1014,28 @@ if !MMU
 source arch/arm/Kconfig-nommu
 endif
 
+config ARM_TRUSTZONE_UNSECURE
+   bool TrustZone: Support for running in an unsecure domain
+   depends on CPU_V6 || CPU_V7
+   help
+ This enables TZ-specific behaviors with respect to modifying state
+	 that is secure-only in a TrustZone system. You will need to select
+	 a specific Hypervisor/Secure Monitor.
+
+choice
+   prompt TrustZone Hypervisor/Secure Monitor
+   depends on ARM_TRUSTZONE_UNSECURE
+   default ARM_TRUSTZONE_UNSECURE_UNKNOWN
+   help
+ Pick the TrustZone Hypervisor/Secure Monitor, under which Linux
+	 will run in an unsecure domain.
+config ARM_TRUSTZONE_UNSECURE_UNKNOWN
+   bool Unknown
+config ARM_TRUSTZONE_UNSECURE_TI
+   bool Texas Instruments Secure Monitor API
+   depends on ARCH_OMAP4
+endchoice
+
 config ARM_ERRATA_411920
 	bool ARM errata: Invalidation of the Instruction Cache operation can fail
 	depends on CPU_V6  !SMP
@@ -1090,20 +1112,6 @@ config ARM_ERRATA_742231
 	  register of the Cortex-A9 which reduces the linefill issuing
 	  capabilities of the processor.
 
-config PL310_ERRATA_588369
-	bool Clean  Invalidate maintenance operations do not invalidate clean lines
-	depends on CACHE_L2X0  ARCH_OMAP4
-	help
-	   The PL310 L2 cache controller implements three types of Clean 
-	   Invalidate maintenance operations: by Physical Address
-	   (offset 0x7F0), by Index/Way (0x7F8) and by Way (0x7FC).
-	   They are architecturally defined to behave as the execution of a
-	   clean operation followed immediately by an invalidate operation,
-	   both performing to the same memory location. This functionality
-	   is not correctly implemented in PL310 as clean lines are not
-	   invalidated as a result of these operations. Note that this errata
-	   uses Texas Instrument's secure monitor api.
-
 config ARM_ERRATA_720789
 	bool ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID
 	depends on CPU_V7  SMP
diff --git a/arch/arm/configs/omap_4430sdp_defconfig b/arch/arm/configs/omap_4430sdp_defconfig
index 14c1e18..ac4091c 100644
--- a/arch/arm/configs/omap_4430sdp_defconfig
+++ b/arch/arm/configs/omap_4430sdp_defconfig
@@ -13,6 +13,8 @@ CONFIG_MODULE_SRCVERSION_ALL=y
 # CONFIG_BLK_DEV_BSG is not set
 CONFIG_ARCH_OMAP=y
 CONFIG_ARCH_OMAP4=y
+CONFIG_ARM_TRUSTZONE_UNSECURE=y
+CONFIG_ARM_TRUSTZONE_UNSECURE_TI=y
 # CONFIG_ARCH_OMAP2PLUS_TYPICAL is not set
 # CONFIG_ARCH_OMAP2 is not set
 # CONFIG_ARCH_OMAP3 is not set
@@ -21,7 +23,8 @@ CONFIG_OMAP_32K_TIMER=y
 CONFIG_OMAP_DM_TIMER=y
 CONFIG_MACH_OMAP_4430SDP=y
 # CONFIG_ARM_THUMB is not set
-CONFIG_PL310_ERRATA_588369=y
+CONFIG_CACHE_PL310_REV_UNKNOWN=y
+CONFIG_CACHE_PL310_ERRATA_588369=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=2
 # CONFIG_LOCAL_TIMERS is not set
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index cc6f9d6..057fa76 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -786,6 +786,58 @@ config CACHE_PL310
 	help
 	  This option enables support for the PL310 cache controller.
 
+choice
+	prompt PL310 revision
+	depends on CACHE_PL310
+	default CACHE_PL310_REV_UNKNOWN
+	help
+	  This option selects the specific revision of the

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-14 Thread Santosh Shilimkar 
 -Original Message-
 From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com]
 Sent: Monday, February 14, 2011 10:39 AM
 To: Andrei Warkentin
 Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption


[]

  ...
 I understood that from first comment. But I am not in favor
 of polluting common ARM files with SOC specific #ifdeffery.
 We have gone over this when first errata support
 was added for PL310

 I have a better way to handle this scenario.
 Expect an updated patch for this.


Below is the updated version which should remove any
OMAP dependency on these errata's. Attached same.


From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Fri, 14 Jan 2011 14:16:04 +0530
Subject: [v2 PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation
can cause data corruption

PL310 implements the Clean  Invalidate by Way L2 cache maintenance
operation (offset 0x7FC). This operation runs in background so that
PL310 can handle normal accesses while it is in progress. Under very
rare circumstances, due to this erratum, write data can be lost when
PL310 treats a cacheable write transaction during a Clean  Invalidate
by Way operation.

Workaround:
Disable Write-Back and Cache Linefill (Debug Control Register)
Clean  Invalidate by Way (0x7FC)
Re-enable Write-Back and Cache Linefill (Debug Control Register)

Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Catalin Marinas catalin.mari...@arm.com
---
 arch/arm/Kconfig   |   13 -
 arch/arm/include/asm/outercache.h  |1 +
 arch/arm/mach-omap2/Kconfig|3 +++
 arch/arm/mach-omap2/omap4-common.c |7 +++
 arch/arm/mm/cache-l2x0.c   |   28 +++-
 5 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5cff165..ebadd95 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1140,7 +1140,7 @@ config ARM_ERRATA_742231

 config PL310_ERRATA_588369
bool Clean  Invalidate maintenance operations do not invalidate
clean lines
-   depends on CACHE_L2X0  ARCH_OMAP4
+   depends on CACHE_L2X0  CACHE_PL310
help
   The PL310 L2 cache controller implements three types of Clean 
   Invalidate maintenance operations: by Physical Address
@@ -1177,6 +1177,17 @@ config ARM_ERRATA_743622
  visible impact on the overall performance or power consumption
of the
  processor.

+config PL310_ERRATA_727915
+   bool Background Clean  Invalidate by Way operation can cause
data corruption
+   depends on CACHE_L2X0  CACHE_PL310
+   help
+ PL310 implements the Clean  Invalidate by Way L2 cache
maintenance
+ operation (offset 0x7FC). This operation runs in background so
that
+ PL310 can handle normal accesses while it is in progress. Under
very
+ rare circumstances, due to this erratum, write data can be lost
when
+ PL310 treats a cacheable write transaction during a Clean 
+ Invalidate by Way operation Note that this errata uses Texas
+ Instrument's secure monitor api to implement the work around.
 endmenu

 source arch/arm/common/Kconfig
diff --git a/arch/arm/include/asm/outercache.h
b/arch/arm/include/asm/outercache.h
index fc19009..348d513 100644
--- a/arch/arm/include/asm/outercache.h
+++ b/arch/arm/include/asm/outercache.h
@@ -31,6 +31,7 @@ struct outer_cache_fns {
 #ifdef CONFIG_OUTER_CACHE_SYNC
void (*sync)(void);
 #endif
+   void (*set_debug)(unsigned long);
 };

 #ifdef CONFIG_OUTER_CACHE
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index f285dd7..fd11ab4 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -45,7 +45,10 @@ config ARCH_OMAP4
select CPU_V7
select ARM_GIC
select LOCAL_TIMERS
+   select CACHE_L2X0
+   select CACHE_PL310
select PL310_ERRATA_588369
+   select PL310_ERRATA_727915
select ARM_ERRATA_720789
select ARCH_HAS_OPP
select PM_OPP if PM
diff --git a/arch/arm/mach-omap2/omap4-common.c
b/arch/arm/mach-omap2/omap4-common.c
index 1926864..9ef8c29 100644
--- a/arch/arm/mach-omap2/omap4-common.c
+++ b/arch/arm/mach-omap2/omap4-common.c
@@ -52,6 +52,12 @@ static void omap4_l2x0_disable(void)
omap_smc1(0x102, 0x0);
 }

+static void omap4_l2x0_set_debug(unsigned long val)
+{
+   /* Program PL310 L2 Cache controller debug register */
+   omap_smc1(0x100, val);
+}
+
 static int __init omap_l2_cache_init(void)
 {
u32 aux_ctrl = 0;
@@ -99,6 +105,7 @@ static int __init omap_l2_cache_init(void)
 * specific one
*/
outer_cache.disable = omap4_l2x0_disable;
+   outer_cache.set_debug = omap4_l2x0_set_debug;

return 0;
 }
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-13 Thread Santosh Shilimkar 
 -Original Message-
 From: Andrei Warkentin [mailto:andr...@motorola.com]
 Sent: Sunday, February 13, 2011 4:48 AM
 To: Santosh Shilimkar
 Cc: linux-omap@vger.kernel.org; Kevin Hilman; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 On Sat, Feb 12, 2011 at 11:59 AM, Santosh Shilimkar
 santosh.shilim...@ti.com wrote:
  -Original Message-
  From: Andrei Warkentin [mailto:andr...@motorola.com]
  Sent: Saturday, February 12, 2011 11:20 PM
  To: Santosh Shilimkar
  Cc: linux-omap@vger.kernel.org; khil...@ti.com; t...@atomide.com;
  linux-arm-ker...@lists.infradead.org; Catalin Marinas
  Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
  operation can cause data corruption
 
  []
 
 
  Can these PL310 errata fixes be made more generic? PL310 is
 present
  in
  non-OMAP platforms too, which lack the TI hypervisor. And these
  platforms might have the same PL310 rev, and suffer the same
  glitches.
  While ideally there is some kind of hypervisor_ops to modify the
  protected register, at the very least there should be the generic
  debug_write handling the  I  can write all PL310 regs case. If
  you're interested I have a patch someplace that tried to do this,
  hopefully I can still find it.
 
  They are kind of generic. If you look at it, the only change
  Which is arch specific is the implementation of debug_writel
 function.
  Today this code is not in generic PL310 code, but
  OMAP specific.
 
  May be we can make this as exported function pointer, which
  arch's can populate.
 
  Will that work for you ?
 
  Regards,
  Santosh
 

 Ie something like the following what do you think???

 #define L2X0_DCR (0xF40)

 static void debug_writel(unsigned long val)
 {
 #ifdef CONFIG_ARCH_OMAP4
omap_smc1(0x100, val);
 #else
writel_relaxed(val, l2x0_base + L2X0_DCR);
 #endif
 }
 ...
I understood that from first comment. But I am not in favor
of polluting common ARM files with SOC specific #ifdeffery.
We have gone over this when first errata support
was added for PL310

I have a better way to handle this scenario.
Expect an updated patch for this.

Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-12 Thread Santosh Shilimkar 
 -Original Message-
 From: Andrei Warkentin [mailto:andr...@motorola.com]
 Sent: Saturday, February 12, 2011 11:20 PM
 To: Santosh Shilimkar
 Cc: linux-omap@vger.kernel.org; khil...@ti.com; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

[]


 Can these PL310 errata fixes be made more generic? PL310 is present
 in
 non-OMAP platforms too, which lack the TI hypervisor. And these
 platforms might have the same PL310 rev, and suffer the same
 glitches.
 While ideally there is some kind of hypervisor_ops to modify the
 protected register, at the very least there should be the generic
 debug_write handling the  I  can write all PL310 regs case. If
 you're interested I have a patch someplace that tried to do this,
 hopefully I can still find it.

They are kind of generic. If you look at it, the only change
Which is arch specific is the implementation of debug_writel function.
Today this code is not in generic PL310 code, but
OMAP specific.

May be we can make this as exported function pointer, which
arch's can populate.

Will that work for you ?

Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-12 Thread Andrei Warkentin 
On Sat, Feb 12, 2011 at 5:29 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 PL310 implements the Clean  Invalidate by Way L2 cache maintenance
 operation (offset 0x7FC). This operation runs in background so that
 PL310 can handle normal accesses while it is in progress. Under very
 rare circumstances, due to this erratum, write data can be lost when
 PL310 treats a cacheable write transaction during a Clean  Invalidate
 by Way operation.

 Workaround:
 Disable Write-Back and Cache Linefill (Debug Control Register)
 Clean  Invalidate by Way (0x7FC)
 Re-enable Write-Back and Cache Linefill (Debug Control Register)

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Catalin Marinas catalin.mari...@arm.com
 ---
  arch/arm/Kconfig            |   11 +++
  arch/arm/mach-omap2/Kconfig |    1 +
  arch/arm/mm/cache-l2x0.c    |   16 ++--
  3 files changed, 22 insertions(+), 6 deletions(-)

 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index 5cff165..2e6b879 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -1177,6 +1177,17 @@ config ARM_ERRATA_743622
          visible impact on the overall performance or power consumption of the
          processor.

 +config PL310_ERRATA_727915
 +       bool Background Clean  Invalidate by Way operation can cause data 
 corruption
 +       depends on CACHE_L2X0  ARCH_OMAP4
 +       help
 +         PL310 implements the Clean  Invalidate by Way L2 cache maintenance
 +         operation (offset 0x7FC). This operation runs in background so that
 +         PL310 can handle normal accesses while it is in progress. Under very
 +         rare circumstances, due to this erratum, write data can be lost when
 +         PL310 treats a cacheable write transaction during a Clean 
 +         Invalidate by Way operation Note that this errata uses Texas
 +         Instrument's secure monitor api to implement the work around.
  endmenu

  source arch/arm/common/Kconfig
 diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
 index f285dd7..1f0ff75 100644
 --- a/arch/arm/mach-omap2/Kconfig
 +++ b/arch/arm/mach-omap2/Kconfig
 @@ -46,6 +46,7 @@ config ARCH_OMAP4
        select ARM_GIC
        select LOCAL_TIMERS
        select PL310_ERRATA_588369
 +       select PL310_ERRATA_727915
        select ARM_ERRATA_720789
        select ARCH_HAS_OPP
        select PM_OPP if PM
 diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
 index 170c9bb..c7c8fbe 100644
 --- a/arch/arm/mm/cache-l2x0.c
 +++ b/arch/arm/mm/cache-l2x0.c
 @@ -67,7 +67,7 @@ static inline void l2x0_inv_line(unsigned long addr)
        writel_relaxed(addr, base + L2X0_INV_LINE_PA);
  }

 -#ifdef CONFIG_PL310_ERRATA_588369
 +#if defined(CONFIG_PL310_ERRATA_588369) || 
 defined(CONFIG_PL310_ERRATA_727915)
  static void debug_writel(unsigned long val)
  {
        extern void omap_smc1(u32 fn, u32 arg);
 @@ -78,7 +78,14 @@ static void debug_writel(unsigned long val)
         */
        omap_smc1(0x100, val);
  }
 +#else
 +/* Optimised out for non-errata case */
 +static inline void debug_writel(unsigned long val)
 +{
 +}
 +#endif

 +#ifdef CONFIG_PL310_ERRATA_588369
  static inline void l2x0_flush_line(unsigned long addr)
  {
        void __iomem *base = l2x0_base;
 @@ -91,11 +98,6 @@ static inline void l2x0_flush_line(unsigned long addr)
  }
  #else

 -/* Optimised out for non-errata case */
 -static inline void debug_writel(unsigned long val)
 -{
 -}
 -
  static inline void l2x0_flush_line(unsigned long addr)
  {
        void __iomem *base = l2x0_base;
 @@ -119,9 +121,11 @@ static void l2x0_flush_all(void)

        /* clean all ways */
        spin_lock_irqsave(l2x0_lock, flags);
 +       debug_writel(0x03);
        writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_INV_WAY);
        cache_wait_way(l2x0_base + L2X0_CLEAN_INV_WAY, l2x0_way_mask);
        cache_sync();
 +       debug_writel(0x00);
        spin_unlock_irqrestore(l2x0_lock, flags);
  }

 --
 1.6.0.4


 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


Can these PL310 errata fixes be made more generic? PL310 is present in
non-OMAP platforms too, which lack the TI hypervisor. And these
platforms might have the same PL310 rev, and suffer the same glitches.
While ideally there is some kind of hypervisor_ops to modify the
protected register, at the very least there should be the generic
debug_write handling the  I  can write all PL310 regs case. If
you're interested I have a patch someplace that tried to do this,
hopefully I can still find it.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

2011-02-12 Thread Andrei Warkentin 
On Sat, Feb 12, 2011 at 11:59 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 -Original Message-
 From: Andrei Warkentin [mailto:andr...@motorola.com]
 Sent: Saturday, February 12, 2011 11:20 PM
 To: Santosh Shilimkar
 Cc: linux-omap@vger.kernel.org; khil...@ti.com; t...@atomide.com;
 linux-arm-ker...@lists.infradead.org; Catalin Marinas
 Subject: Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way
 operation can cause data corruption

 []


 Can these PL310 errata fixes be made more generic? PL310 is present
 in
 non-OMAP platforms too, which lack the TI hypervisor. And these
 platforms might have the same PL310 rev, and suffer the same
 glitches.
 While ideally there is some kind of hypervisor_ops to modify the
 protected register, at the very least there should be the generic
 debug_write handling the  I  can write all PL310 regs case. If
 you're interested I have a patch someplace that tried to do this,
 hopefully I can still find it.

 They are kind of generic. If you look at it, the only change
 Which is arch specific is the implementation of debug_writel function.
 Today this code is not in generic PL310 code, but
 OMAP specific.

 May be we can make this as exported function pointer, which
 arch's can populate.

 Will that work for you ?

 Regards,
 Santosh


Ie something like the following what do you think???

#define L2X0_DCR (0xF40)

static void debug_writel(unsigned long val)
{
#ifdef CONFIG_ARCH_OMAP4
   omap_smc1(0x100, val);
#else
   writel_relaxed(val, l2x0_base + L2X0_DCR);
#endif
}
...
...

   /* clean all ways */
   spin_lock_irqsave(l2x0_lock, flags);
#ifdef CONFIG_PL310_ERRATA_727915
   debug_writel(DCR_DWB | DCR_DCL);  not 0x3, but self-documenting
#endif
   writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_INV_WAY);
   cache_wait_way(l2x0_base + L2X0_CLEAN_INV_WAY, l2x0_way_mask);
   cache_sync();
#ifdef CONFIG_PL310_ERRATA_727915
   debug_writel(0x00);
#endif
   spin_unlock_irqrestore(l2x0_lock, flags);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html