Re: [PATCH] MIPS: mm: tlbex: Fix potential HTW race on TLBL/M/S handlers

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On 27/11/14 11:13, Markos Chandras wrote:
> From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> 
> There is a potential race when probing the TLB in TLBL/M/S exception
> handlers for a matching entry. Between the time we hit a TLBL/S/M
> exception and the time we get to execute the TLBP instruction, the

More specifically it is between the exception being triggered and the
actual start of exception handling. HTW is disabled while at exception
level so it isn't a problem after the handler has actually started.

> HTW may have killed the TLB entry we are interested in hence the TLB

maybe s/killed/replaced/

Sorry to be picky, but I think it's worth getting that wording as
specific as possible for future reference.

> probe may fail. However, in the existing handlers, we never checked the
> status of the TLBP (ie check the result in the C0/Index register). We
> fix this by adding such a check when the core implements the HTW. If
> we couldn't find a matching entry, we return back and try again.
> 
> Cc: <stable@vger.kernel.org> # v3.17+
> Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
> ---
>  arch/mips/mm/tlbex.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index 7994368f96c4..3978a3d81366 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -1872,8 +1872,16 @@ build_r4000_tlbchange_handler_head(u32 **p, struct uasm_label **l,
>  	uasm_l_smp_pgtable_change(l, *p);
>  #endif
>  	iPTE_LW(p, wr.r1, wr.r2); /* get even pte */
> -	if (!m4kc_tlbp_war())
> +	if (!m4kc_tlbp_war()) {
>  		build_tlb_probe_entry(p);
> +		if (cpu_has_htw) {
> +			/* race condition happens, leaving */

How about expanding this comment a bit for people trying to figure out
the code.

Technically though:
Reviewed-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

> +			uasm_i_ehb(p);
> +			uasm_i_mfc0(p, wr.r3, C0_INDEX);
> +			uasm_il_bltz(p, r, wr.r3, label_leave);
> +			uasm_i_nop(p);
> +		}
> +	}
>  	return wr;
>  }
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]