[PATCH] MIPS: Don't write ones to reserved entryhi bits.

[PATCH] MIPS: Don't write ones to reserved entryhi bits.

[David Daney]

According to the MIPS64 Privileged Resource Architecture manual, only
values of zero may be written to bits 8..10 of CP0 entryhi.  We need
to add masking by ASID_MASK.

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

diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index d7f3eb0..3899f99 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -169,7 +169,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	ehb(); /* Make sure it propagates to TCStatus */
 	evpe(mtflags);
 #else
-	write_c0_entryhi(cpu_context(cpu, next));
+	write_c0_entryhi(cpu_context(cpu, next) & ASID_MASK);
 #endif /* CONFIG_MIPS_MT_SMTC */
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
 
@@ -229,7 +229,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	ehb(); /* Make sure it propagates to TCStatus */
 	evpe(mtflags);
 #else
-	write_c0_entryhi(cpu_context(cpu, next));
+	write_c0_entryhi(cpu_context(cpu, next) & ASID_MASK);
 #endif /* CONFIG_MIPS_MT_SMTC */
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
 
-- 
1.6.0.6

Re: [PATCH] MIPS: Don't write ones to reserved entryhi bits.

[Ralf Baechle]

On Fri, May 08, 2009 at 12:52:10PM -0700, David Daney wrote:

> According to the MIPS64 Privileged Resource Architecture manual, only
> values of zero may be written to bits 8..10 of CP0 entryhi.  We need
> to add masking by ASID_MASK.

Yes, I've silently been relying on the hardware chopping off the excess
bits for no better reason that it saving an instruction.  One of the
functions you've touched is switch_mm() which is being used in context
switches and any changes to it will show up in context switching
benchmarks.

The patch you did (and along with that some older SMTC changes by Kevin)
can be done slightly more elegant because we already have:

#define cpu_asid(cpu, mm)       (cpu_context((cpu), (mm)) & ASID_MASK)

in <asm/mmu_context.h>.

We used to optimize the ASID managment code by code patching even, see
mmu_context.h in 78c388aed2b7184182c08428db1de6c872d815f5.

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/include/asm/mmu_context.h |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index d7f3eb0..25a50fa 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -164,12 +164,12 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	 * having ASID_MASK smaller than the hardware maximum,
 	 * make sure no "soft" bits become "hard"...
 	 */
-	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK)
-			| (cpu_context(cpu, next) & ASID_MASK));
+	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK) |
+			 cpu_asid(cpu, next));
 	ehb(); /* Make sure it propagates to TCStatus */
 	evpe(mtflags);
 #else
-	write_c0_entryhi(cpu_context(cpu, next));
+	write_c0_entryhi(cpu_asid(cpu, next));
 #endif /* CONFIG_MIPS_MT_SMTC */
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
 
@@ -225,11 +225,11 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	}
 	/* See comments for similar code above */
 	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK) |
-	                 (cpu_context(cpu, next) & ASID_MASK));
+	                 cpu_asid(cpu, next));
 	ehb(); /* Make sure it propagates to TCStatus */
 	evpe(mtflags);
 #else
-	write_c0_entryhi(cpu_context(cpu, next));
+	write_c0_entryhi(cpu_asid(cpu, next));
 #endif /* CONFIG_MIPS_MT_SMTC */
 	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
 

Re: [PATCH] MIPS: Don't write ones to reserved entryhi bits.

[David Daney]

Ralf Baechle wrote:
> On Fri, May 08, 2009 at 12:52:10PM -0700, David Daney wrote:
> 
>> According to the MIPS64 Privileged Resource Architecture manual, only
>> values of zero may be written to bits 8..10 of CP0 entryhi.  We need
>> to add masking by ASID_MASK.
> 
> Yes, I've silently been relying on the hardware chopping off the excess
> bits for no better reason that it saving an instruction.  One of the
> functions you've touched is switch_mm() which is being used in context
> switches and any changes to it will show up in context switching
> benchmarks.
> 
> The patch you did (and along with that some older SMTC changes by Kevin)
> can be done slightly more elegant because we already have:
> 
> #define cpu_asid(cpu, mm)       (cpu_context((cpu), (mm)) & ASID_MASK)
> 
> in <asm/mmu_context.h>.
> 
> We used to optimize the ASID managment code by code patching even, see
> mmu_context.h in 78c388aed2b7184182c08428db1de6c872d815f5.
> 
>   Ralf
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 

This is nice, but you never committed it.

David Daney.



>  arch/mips/include/asm/mmu_context.h |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
> index d7f3eb0..25a50fa 100644
> --- a/arch/mips/include/asm/mmu_context.h
> +++ b/arch/mips/include/asm/mmu_context.h
> @@ -164,12 +164,12 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  	 * having ASID_MASK smaller than the hardware maximum,
>  	 * make sure no "soft" bits become "hard"...
>  	 */
> -	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK)
> -			| (cpu_context(cpu, next) & ASID_MASK));
> +	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK) |
> +			 cpu_asid(cpu, next));
>  	ehb(); /* Make sure it propagates to TCStatus */
>  	evpe(mtflags);
>  #else
> -	write_c0_entryhi(cpu_context(cpu, next));
> +	write_c0_entryhi(cpu_asid(cpu, next));
>  #endif /* CONFIG_MIPS_MT_SMTC */
>  	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
>  
> @@ -225,11 +225,11 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
>  	}
>  	/* See comments for similar code above */
>  	write_c0_entryhi((read_c0_entryhi() & ~HW_ASID_MASK) |
> -	                 (cpu_context(cpu, next) & ASID_MASK));
> +	                 cpu_asid(cpu, next));
>  	ehb(); /* Make sure it propagates to TCStatus */
>  	evpe(mtflags);
>  #else
> -	write_c0_entryhi(cpu_context(cpu, next));
> +	write_c0_entryhi(cpu_asid(cpu, next));
>  #endif /* CONFIG_MIPS_MT_SMTC */
>  	TLBMISS_HANDLER_SETUP_PGD(next->pgd);
>  
> 

Re: [PATCH] MIPS: Don't write ones to reserved entryhi bits.

[Ralf Baechle]

On Wed, Sep 16, 2009 at 11:09:35AM -0700, David Daney wrote:

>>> According to the MIPS64 Privileged Resource Architecture manual, only
>>> values of zero may be written to bits 8..10 of CP0 entryhi.  We need
>>> to add masking by ASID_MASK.
>>
>> Yes, I've silently been relying on the hardware chopping off the excess
>> bits for no better reason that it saving an instruction.  One of the
>> functions you've touched is switch_mm() which is being used in context
>> switches and any changes to it will show up in context switching
>> benchmarks.
>>
>> The patch you did (and along with that some older SMTC changes by Kevin)
>> can be done slightly more elegant because we already have:
>>
>> #define cpu_asid(cpu, mm)       (cpu_context((cpu), (mm)) & ASID_MASK)
>>
>> in <asm/mmu_context.h>.
>>
>> We used to optimize the ASID managment code by code patching even, see
>> mmu_context.h in 78c388aed2b7184182c08428db1de6c872d815f5.
>>
>>   Ralf
>>
>> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>>
>
> This is nice, but you never committed it.

Waiting for people to test it - thanks!  Committing it now.

  Ralf