[PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[David Daney]

Some CPUs implement mipsr2, but because they are a super-set of
mips64r2 do not define CONFIG_CPU_MIPS64_R2.  Cavium OCTEON falls into
this category.  We would still like to use the optimized
implementation, so since we have already checked for
CONFIG_CPU_MIPSR2, checking for CONFIG_64BIT instead of
CONFIG_CPU_MIPS64_R2 is sufficient.

Change from v1: Add comments about why the change is safe.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/include/asm/swab.h |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/swab.h b/arch/mips/include/asm/swab.h
index 99993c0..97c2f81 100644
--- a/arch/mips/include/asm/swab.h
+++ b/arch/mips/include/asm/swab.h
@@ -38,7 +38,11 @@ static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
 }
 #define __arch_swab32 __arch_swab32
 
-#ifdef CONFIG_CPU_MIPS64_R2
+/*
+ * Having already checked for CONFIG_CPU_MIPSR2, enable the
+ * optimized version for 64-bit kernel on r2 CPUs.
+ */
+#ifdef CONFIG_64BIT
 static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
 {
 	__asm__(
@@ -50,6 +54,6 @@ static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
 	return x;
 }
 #define __arch_swab64 __arch_swab64
-#endif /* CONFIG_CPU_MIPS64_R2 */
+#endif /* CONFIG_64BIT */
 #endif /* CONFIG_CPU_MIPSR2 */
 #endif /* _ASM_SWAB_H */
-- 
1.6.0.6

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[Ralf Baechle]

On Mon, Jun 29, 2009 at 09:54:15AM -0700, David Daney wrote:

> Some CPUs implement mipsr2, but because they are a super-set of
> mips64r2 do not define CONFIG_CPU_MIPS64_R2.  Cavium OCTEON falls into
> this category.  We would still like to use the optimized
> implementation, so since we have already checked for
> CONFIG_CPU_MIPSR2, checking for CONFIG_64BIT instead of
> CONFIG_CPU_MIPS64_R2 is sufficient.
> 
> Change from v1: Add comments about why the change is safe.

Thanks, applied.  Though this sort of patch make me thing that maybe we
rather should have treated the cnMIPS cores differently.

  Ralf

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[Maciej W. Rozycki]

On Mon, 29 Jun 2009, Ralf Baechle wrote:

> > Some CPUs implement mipsr2, but because they are a super-set of
> > mips64r2 do not define CONFIG_CPU_MIPS64_R2.  Cavium OCTEON falls into
> > this category.  We would still like to use the optimized
> > implementation, so since we have already checked for
> > CONFIG_CPU_MIPSR2, checking for CONFIG_64BIT instead of
> > CONFIG_CPU_MIPS64_R2 is sufficient.
> > 
> > Change from v1: Add comments about why the change is safe.
> 
> Thanks, applied.  Though this sort of patch make me thing that maybe we
> rather should have treated the cnMIPS cores differently.

 This is a pure code generation option and it asks for "select 
CPU_MIPS64_R2" under CPU_OCTEON (or whatever option is used for that 
chip).  Or something like "select ISA_MIPS64_R2" actually, as we want to 
keep CPU_foo as the -march=, etc. designator.  IOW it looks like we lack 
ISA supersetting along the lines of how tools handle it.

  Maciej

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[David Daney]

Maciej W. Rozycki wrote:
> On Mon, 29 Jun 2009, Ralf Baechle wrote:
> 
>>> Some CPUs implement mipsr2, but because they are a super-set of
>>> mips64r2 do not define CONFIG_CPU_MIPS64_R2.  Cavium OCTEON falls into
>>> this category.  We would still like to use the optimized
>>> implementation, so since we have already checked for
>>> CONFIG_CPU_MIPSR2, checking for CONFIG_64BIT instead of
>>> CONFIG_CPU_MIPS64_R2 is sufficient.
>>>
>>> Change from v1: Add comments about why the change is safe.
>> Thanks, applied.  Though this sort of patch make me thing that maybe we
>> rather should have treated the cnMIPS cores differently.
> 
>  This is a pure code generation option and it asks for "select 
> CPU_MIPS64_R2" under CPU_OCTEON (or whatever option is used for that 
> chip).  Or something like "select ISA_MIPS64_R2" actually, as we want to 
> keep CPU_foo as the -march=, etc. designator.  IOW it looks like we lack 
> ISA supersetting along the lines of how tools handle it.
> 

The problem with CPU_MIPS64_R2 in the kernel is that it means two 
unrelated things:

1) The cpu can execute all mips64r2 ISA instructions.

2) The cpu requires that all worse case cache and execution hazards are 
handled.

In the case of the Octeon processors, #1 is true, but we can get better 
performance by omitting many of the hazard barriers because they are 
unneeded.

David Daney

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[Maciej W. Rozycki]

On Tue, 30 Jun 2009, David Daney wrote:

> The problem with CPU_MIPS64_R2 in the kernel is that it means two unrelated
> things:
> 
> 1) The cpu can execute all mips64r2 ISA instructions.
> 
> 2) The cpu requires that all worse case cache and execution hazards are
> handled.
> 
> In the case of the Octeon processors, #1 is true, but we can get better
> performance by omitting many of the hazard barriers because they are unneeded.

 Which is why I think a split of the semantics would be a good idea.

  Maciej

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[Ralf Baechle]

On Wed, Jul 01, 2009 at 02:36:31AM +0100, Maciej W. Rozycki wrote:

> > The problem with CPU_MIPS64_R2 in the kernel is that it means two unrelated
> > things:
> > 
> > 1) The cpu can execute all mips64r2 ISA instructions.
> > 
> > 2) The cpu requires that all worse case cache and execution hazards are
> > handled.
> > 
> > In the case of the Octeon processors, #1 is true, but we can get better
> > performance by omitting many of the hazard barriers because they are unneeded.
> 
>  Which is why I think a split of the semantics would be a good idea.

That's the idea since a long time.  There are far less uses of CONFIG_CPU_*
than there used to be historically - and usually they're a bug or at least
should be considered one.

  Ralf

Re: [PATCH] MIPS: Define __arch_swab64 for all mips r2 cpus (v2).

[Ralf Baechle]

On Tue, Jun 30, 2009 at 06:13:41PM -0700, David Daney wrote:

> The problem with CPU_MIPS64_R2 in the kernel is that it means two  
> unrelated things:
>
> 1) The cpu can execute all mips64r2 ISA instructions.
>
> 2) The cpu requires that all worse case cache and execution hazards are  
> handled.
>
> In the case of the Octeon processors, #1 is true, but we can get better  
> performance by omitting many of the hazard barriers because they are  
> unneeded.

The most performance sensitive hazard barriers are the ones in the TLB
exception handlers and they're now being handled in C code in tlbex.c
which mostly does runtime decissions.  I suspect the remaining hazard
barriers are not a big performance thing anymore.

  Ralf