Re: [PATCH] MIPS: Remove race window in page fault handling

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

On 05/31/2014 03:36 AM, Lars Persson wrote:
> Multicore MIPSes without I/D hardware coherency suffered from a race
> condition in the page fault handler. The page table entry was published
> before any pending lazy D-cache flush was committed, hence it allowed
> execution of stale page cache data by other VPEs in the system.
>

Shouldn't this only be done on machines that suffer from the problem?

There are many SMP MIPS machines that don't need this, so they shouldn't 
have to pay the price for doing this.

David Daney



> Signed-off-by: Lars Persson <larper@axis.com>
> ---
>   arch/mips/include/asm/pgtable.h |   17 +++++++++++++++--
>   arch/mips/mm/cache.c            |   10 ++++++++++
>   2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index 008324d..7b175e5 100644
> --- a/arch/mips/include/asm/pgtable.h
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -95,6 +95,9 @@ extern void paging_init(void);
>
>   #define pmd_page_vaddr(pmd)	pmd_val(pmd)
>
> +static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
> +	pte_t *ptep, pte_t pteval);
> +
>   #if defined(CONFIG_64BIT_PHYS_ADDR) && defined(CONFIG_CPU_MIPS32)
>
>   #define pte_none(pte)		(!(((pte).pte_low | (pte).pte_high) & ~_PAGE_GLOBAL))
> @@ -118,7 +121,6 @@ static inline void set_pte(pte_t *ptep, pte_t pte)
>   		}
>   	}
>   }
> -#define set_pte_at(mm, addr, ptep, pteval) set_pte(ptep, pteval)
>
>   static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>   {
> @@ -155,7 +157,6 @@ static inline void set_pte(pte_t *ptep, pte_t pteval)
>   	}
>   #endif
>   }
> -#define set_pte_at(mm, addr, ptep, pteval) set_pte(ptep, pteval)
>
>   static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>   {
> @@ -169,6 +170,18 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *pt
>   }
>   #endif
>
> +extern void mips_flush_dcache_from_pte(pte_t pteval);
> +
> +static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
> +	pte_t *ptep, pte_t pteval)
> +{
> +	/* Make code globally visible before publishing the page
> +	   table entry. */
> +	if (addr < TASK_SIZE && pte_present(pteval))
> +		mips_flush_dcache_from_pte(pteval);
> +	set_pte(ptep, pteval);
> +}
> +
>   /*
>    * (pmds are folded into puds so this doesn't get actually called,
>    * but the define is needed for a generic inline function.)
> diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
> index 9e67cde..1320afc 100644
> --- a/arch/mips/mm/cache.c
> +++ b/arch/mips/mm/cache.c
> @@ -137,6 +137,16 @@ void __update_cache(struct vm_area_struct *vma, unsigned long address,
>   	}
>   }
>
> +void mips_flush_dcache_from_pte(pte_t pteval)
> +{
> +	unsigned long pfn = pte_pfn(pteval);
> +	if (likely(pfn_valid(pfn))) {
> +		struct page *page = pfn_to_page(pfn);
> +		if (Page_dcache_dirty(page))
> +			flush_dcache_page(page);
> +	}
> +}
> +
>   unsigned long _page_cachable_default;
>   EXPORT_SYMBOL(_page_cachable_default);
>
>