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