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

From: David Daney <[email-masked]>
Date: Wed, 16 Sep 2009 11:09:35 -0700

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 <[email-masked]>
> 

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);
>  
>