Re: [PATCH v2] MIPS: Optimise TLB handlers for MIPS32/64 R2 cores.

[David Daney <ddaney.cavm@gmail.com>]

On 01/03/2013 01:45 PM, Steven J. Hill wrote:
> From: "Steven J. Hill" <sjhill@mips.com>
>
> The EXT and INS instructions can be used to decrease code size and
> thus speed up TLB handlers on MIPS32R2 and MIPS64R2 cores.
>
[...]
> @@ -1012,6 +1019,24 @@ static void __cpuinit build_adjust_context(u32 **p, unsigned int ctx)
>
>   static void __cpuinit build_get_ptep(u32 **p, unsigned int tmp, unsigned int ptr)
>   {
> +	if (cpu_has_mips_r2) {
> +		/* PTE ptr offset is obtained from BadVAddr */
> +		UASM_i_MFC0(p, tmp, C0_BADVADDR);
> +		UASM_i_LW(p, ptr, 0, ptr);
> +#ifdef CONFIG_CPU_MIPS64

Is this the right condition?  Is is correct for a 32-bit kernel running 
on a 64-bit CPU?  Will OCTEON be covered? (no, but it should)

> +		uasm_i_dext(p, tmp, tmp, (PAGE_SHIFT + 1),
> +			(PAGE_SHIFT - PTE_ORDER - PTE_T_LOG2 - 1));
> +		uasm_i_dins(p, ptr, tmp, (PTE_T_LOG2 + 1),
> +			(PAGE_SHIFT - PTE_ORDER - PTE_T_LOG2 - 1));
> +#else
> +		uasm_i_ext(p, tmp, tmp, (PAGE_SHIFT + 1),
> +			(PGDIR_SHIFT - PAGE_SHIFT - 1);

Did you even compile this?  It looks like a mismatch in the number of 
'(' and ')'.

> +		uasm_i_ins(p, ptr, tmp, (PTE_T_LOG2 + 1),
> +			(PGDIR_SHIFT - PAGE_SHIFT - 1);
> +#endif
> +		return;
> +	}


Can this whole thing be made more clear by defining UASM_i_EXT(...) that 
does the proper thing for either 32 or 64 bit kernels as the rest of the 
capitolized versions of the macros do?


Is (PAGE_SHIFT - PTE_ORDER - PTE_T_LOG2 - 1) != (PGDIR_SHIFT - 
PAGE_SHIFT - 1) for any combinations of config options?  Why are they 
different for the two cases?

David Daney