NestMMU pte upgrade workaround for mprotect
Subsystems (12)
v4 cover NestMMU pte upgrade workaround for mprotect
We can upgrade pte access (R -> RW transition) via mprotect. We need
to make sure we follow the recommended pte update sequence as outlined in commit bd5050e38aec ("powerpc/mm/radix: Change pte relax sequence to handle nest MMU hang") for such updates. This patch series do that.
Changes from V3:
* Build fix for x86
Changes from V2:
* Update commit message for patch 4
* use radix tlb flush routines directly.
Changes from V1:
* Restrict ths only for R->RW upgrade. We don't need to do this for Autonuma
* Restrict this only for radix translation mode.
Aneesh Kumar K.V (5):
mm: Update ptep_modify_prot_start/commit to take vm_area_struct as arg
mm: update ptep_modify_prot_commit to take old pte value as arg arch/powerpc/mm: Nest MMU workaround for mprotect RW upgrade. mm/hugetlb: Add prot_modify_start/commit sequence for hugetlb update arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW
upgrade
arch/powerpc/include/asm/book3s/64/hugetlb.h | 12 +++++++++ arch/powerpc/include/asm/book3s/64/pgtable.h | 18 +++++++++++++ arch/powerpc/include/asm/book3s/64/radix.h | 4 +++ arch/powerpc/mm/hugetlbpage-hash64.c | 27 ++++++++++++++++++++ arch/powerpc/mm/hugetlbpage-radix.c | 17 ++++++++++++ arch/powerpc/mm/pgtable-book3s64.c | 27 ++++++++++++++++++++ arch/powerpc/mm/pgtable-radix.c | 18 +++++++++++++ arch/s390/include/asm/pgtable.h | 5 ++-- arch/s390/mm/pgtable.c | 8 +++--- arch/x86/include/asm/paravirt.h | 13 +++++----- arch/x86/include/asm/paravirt_types.h | 5 ++-- arch/x86/xen/mmu.h | 4 +-- arch/x86/xen/mmu_pv.c | 8 +++--- fs/proc/task_mmu.c | 8 +++--- include/asm-generic/pgtable.h | 18 ++++++------- include/linux/hugetlb.h | 20 +++++++++++++++ mm/hugetlb.c | 8 +++--- mm/memory.c | 8 +++--- mm/mprotect.c | 6 ++--- 19 files changed, 193 insertions(+), 41 deletions(-)
Signature · 1 line
2.19.2
PATCH v4 1/5 mm: Update ptep_modify_prot_start/commit to take vm_area_struct as arg
Some architecture may want to call flush_tlb_range from these helpers.
Signed-off-by Aneesh Kumar K.V <[email protected]>
--- arch/s390/include/asm/pgtable.h | 4 ++-- arch/s390/mm/pgtable.c | 6 ++++-- arch/x86/include/asm/paravirt.h | 11 ++++++----- arch/x86/include/asm/paravirt_types.h | 5 +++-- arch/x86/xen/mmu.h | 4 ++-- arch/x86/xen/mmu_pv.c | 8 ++++---- fs/proc/task_mmu.c | 4 ++-- include/asm-generic/pgtable.h | 16 ++++++++-------- mm/memory.c | 4 ++-- mm/mprotect.c | 4 ++-- 10 files changed, 35 insertions(+), 31 deletions(-)
arch/s390/include/asm/pgtable.h +2 -2
@@ -1069,8 +1069,8 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm, }#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION -pte_t ptep_modify_prot_start (struct mm_struct *,unsigned long ,pte_t *); -void ptep_modify_prot_commit (struct mm_struct *,unsigned long ,pte_t *,pte_t ); +pte_t ptep_modify_prot_start (struct vm_area_struct *,unsigned long ,pte_t *); +void ptep_modify_prot_commit (struct vm_area_struct *,unsigned long ,pte_t *,pte_t );#define __HAVE_ARCH_PTEP_CLEAR_FLUSH static inline pte_t ptep_clear_flush(struct vm_area_struct *vma,
arch/s390/mm/pgtable.c +4 -2
@@ -301,12 +301,13 @@ pte_t ptep_xchg_lazy(struct mm_struct *mm, unsigned long addr, }EXPORT_SYMBOL (ptep_xchg_lazy); -pte_t ptep_modify_prot_start (struct mm_struct *mm,unsigned long addr, +pte_t ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {pgste_t pgste;pte_t old;int nodat; +struct mm_struct *mm = vma->vm_mm;preempt_disable (); pgste =ptep_xchg_start (mm, addr, ptep);
@@ -320,10 +321,11 @@ pte_t ptep_modify_prot_start(struct mm_struct *mm, unsigned long addr, }EXPORT_SYMBOL (ptep_modify_prot_start); -void ptep_modify_prot_commit (struct mm_struct *mm,unsigned long addr, +void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte) {pgste_t pgste; +struct mm_struct *mm = vma->vm_mm;if (!MACHINE_HAS_NX )pte_val (pte) &= ~_PAGE_NOEXEC;
arch/x86/include/asm/paravirt.h +6 -5
@@ -417,25 +417,26 @@ static inline pgdval_t pgd_val(pgd_t pgd) }#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION -static inline pte_t ptep_modify_prot_start (struct mm_struct *mm,unsigned long addr, +static inline pte_t ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {pteval_t ret; - ret =PVOP_CALL3 (pteval_t, mmu.ptep_modify_prot_start, mm, addr, ptep); + ret =PVOP_CALL3 (pteval_t, mmu.ptep_modify_prot_start, vma, addr, ptep);return (pte_t ) { .pte = ret }; } -static inline void ptep_modify_prot_commit (struct mm_struct *mm,unsigned long addr, +static inline void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte) { +if (sizeof (pteval_t) >sizeof (long ))/* 5 arg words */ - pv_ops.mmu.ptep_modify_prot_commit (mm, addr, ptep, pte); + pv_ops.mmu.ptep_modify_prot_commit (vma, addr, ptep, pte);else PVOP_VCALL4 (mmu.ptep_modify_prot_commit, - mm, addr, ptep, pte.pte); + vma, addr, ptep, pte.pte); }static inline void set_pte (pte_t *ptep,pte_t pte)
arch/x86/include/asm/paravirt_types.h +3 -2
@@ -55,6 +55,7 @@ struct task_struct;struct cpumask ;struct flush_tlb_info ;struct mmu_gather ; +struct vm_area_struct ;/* * Wrapper type for pointers to code which uses the non-standard
@@ -254,9 +255,9 @@ struct pv_mmu_ops { pte_t *ptep, pte_t pteval);void (*set_pmd)(pmd_t *pmdp,pmd_t pmdval); -pte_t (*ptep_modify_prot_start)(struct mm_struct *mm,unsigned long addr, +pte_t (*ptep_modify_prot_start)(struct vm_area_struct *vma,unsigned long addr,pte_t *ptep); -void (*ptep_modify_prot_commit)(struct mm_struct *mm,unsigned long addr, +void (*ptep_modify_prot_commit)(struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte);struct paravirt_callee_save pte_val;
arch/x86/xen/mmu.h +2 -2
@@ -17,8 +17,8 @@ bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn);void set_pte_mfn (unsigned long vaddr,unsigned long pfn,pgprot_t flags); -pte_t xen_ptep_modify_prot_start (struct mm_struct *mm,unsigned long addr,pte_t *ptep); -void xen_ptep_modify_prot_commit (struct mm_struct *mm,unsigned long addr, +pte_t xen_ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep); +void xen_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte);unsigned long xen_read_cr2_direct (void );
arch/x86/xen/mmu_pv.c +4 -4
@@ -306,20 +306,20 @@ static void xen_set_pte_at(struct mm_struct *mm, unsigned long addr,__xen_set_pte (ptep, pteval); } -pte_t xen_ptep_modify_prot_start (struct mm_struct *mm, +pte_t xen_ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {/* Just return the pte as-is. We preserve the bits on commit */ -trace_xen_mmu_ptep_modify_prot_start (mm, addr, ptep, *ptep); +trace_xen_mmu_ptep_modify_prot_start (vma->vm_mm, addr, ptep, *ptep);return *ptep; } -void xen_ptep_modify_prot_commit (struct mm_struct *mm,unsigned long addr, +void xen_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte) {struct mmu_update u; -trace_xen_mmu_ptep_modify_prot_commit (mm, addr, ptep, pte); +trace_xen_mmu_ptep_modify_prot_commit (vma->vm_mm, addr, ptep, pte);xen_mc_batch (); u.ptr =virt_to_machine (ptep).maddr |MMU_PT_UPDATE_PRESERVE_AD ;
fs/proc/task_mmu.c +2 -2
@@ -940,10 +940,10 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,pte_t ptent = *pte;if (pte_present (ptent)) { - ptent =ptep_modify_prot_start (vma->vm_mm, addr, pte); + ptent =ptep_modify_prot_start (vma, addr, pte); ptent =pte_wrprotect (ptent); ptent =pte_clear_soft_dirty (ptent); -ptep_modify_prot_commit (vma->vm_mm, addr, pte, ptent); +ptep_modify_prot_commit (vma, addr, pte, ptent); }else if (is_swap_pte (ptent)) { ptent =pte_swp_clear_soft_dirty (ptent);set_pte_at (vma->vm_mm, addr, pte, ptent);
include/asm-generic/pgtable.h +8 -8
@@ -568,7 +568,7 @@ static inline int pmd_none_or_clear_bad(pmd_t *pmd)return 0 ; } -static inline pte_t __ptep_modify_prot_start (struct mm_struct *mm, +static inline pte_t __ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {
@@ -577,10 +577,10 @@ static inline pte_t __ptep_modify_prot_start(struct mm_struct *mm,* non-present, preventing the hardware from asynchronously * updating it. */ -return ptep_get_and_clear (mm, addr, ptep); +return ptep_get_and_clear (vma->vm_mm, addr, ptep); } -static inline void __ptep_modify_prot_commit (struct mm_struct *mm, +static inline void __ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte) {
@@ -588,7 +588,7 @@ static inline void __ptep_modify_prot_commit(struct mm_struct *mm,* The pte is non-present, so there's no hardware state to * preserve. */ -set_pte_at (mm, addr, ptep, pte); +set_pte_at (vma->vm_mm, addr, ptep, pte); }#ifndef __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
@@ -606,22 +606,22 @@ static inline void __ptep_modify_prot_commit(struct mm_struct *mm,* queue the update to be done at some later time. The update must be * actually committed before the pte lock is released, however. */ -static inline pte_t ptep_modify_prot_start (struct mm_struct *mm, +static inline pte_t ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) { -return __ptep_modify_prot_start (mm, addr, ptep); +return __ptep_modify_prot_start (vma, addr, ptep); }/* * Commit an update to a pte, leaving any hardware-controlled bits in * the PTE unmodified. */ -static inline void ptep_modify_prot_commit (struct mm_struct *mm, +static inline void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep,pte_t pte) { -__ptep_modify_prot_commit (mm, addr, ptep, pte); +__ptep_modify_prot_commit (vma, addr, ptep, pte); }#endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ #endif /* CONFIG_MMU */
mm/memory.c +2 -2
@@ -3588,12 +3588,12 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)* Make it present again, Depending on how arch implementes non * accessible ptes, some can allow access by kernel mode. */ - pte =ptep_modify_prot_start (vma->vm_mm, vmf->address, vmf->pte); + pte =ptep_modify_prot_start (vma, vmf->address, vmf->pte); pte =pte_modify (pte, vma->vm_page_prot); pte =pte_mkyoung (pte);if (was_writable) pte =pte_mkwrite (pte); -ptep_modify_prot_commit (vma->vm_mm, vmf->address, vmf->pte, pte); +ptep_modify_prot_commit (vma, vmf->address, vmf->pte, pte);update_mmu_cache (vma, vmf->address, vmf->pte); page =vm_normal_page (vma, vmf->address, pte);
mm/mprotect.c +2 -2
@@ -110,7 +110,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,continue ; } - ptent =ptep_modify_prot_start (mm, addr, pte); + ptent =ptep_modify_prot_start (vma, addr, pte); ptent =pte_modify (ptent, newprot);if (preserve_write) ptent =pte_mk_savedwrite (ptent);
@@ -121,7 +121,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd, !(vma->vm_flags & VM_SOFTDIRTY))) { ptent =pte_mkwrite (ptent); } -ptep_modify_prot_commit (mm, addr, pte, ptent); +ptep_modify_prot_commit (vma, addr, pte, ptent); pages++; }else if (IS_ENABLED (CONFIG_MIGRATION )) {swp_entry_t entry =pte_to_swp_entry (oldpte);
Signature · 1 line
2.19.2
PATCH v4 2/5 mm: update ptep_modify_prot_commit to take old pte value as arg
In reply to Aneesh Kumar K.VArchitectures like ppc64 requires to do a conditional tlb flush based on the old and new value of pte. Enable that by passing old pte value as the arg.
Signed-off-by Aneesh Kumar K.V <[email protected]>
--- arch/s390/include/asm/pgtable.h | 3 ++- arch/s390/mm/pgtable.c | 2 +- arch/x86/include/asm/paravirt.h | 2 +- fs/proc/task_mmu.c | 8 +++++--- include/asm-generic/pgtable.h | 2 +- mm/memory.c | 8 ++++---- mm/mprotect.c | 6 +++--- 7 files changed, 17 insertions(+), 14 deletions(-)
arch/s390/include/asm/pgtable.h +2 -1
@@ -1070,7 +1070,8 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm,#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTIONpte_t ptep_modify_prot_start (struct vm_area_struct *,unsigned long ,pte_t *); -void ptep_modify_prot_commit (struct vm_area_struct *,unsigned long ,pte_t *,pte_t ); +void ptep_modify_prot_commit (struct vm_area_struct *,unsigned long , +pte_t *,pte_t ,pte_t );#define __HAVE_ARCH_PTEP_CLEAR_FLUSH static inline pte_t ptep_clear_flush(struct vm_area_struct *vma,
arch/s390/mm/pgtable.c +1 -1
@@ -322,7 +322,7 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,EXPORT_SYMBOL (ptep_modify_prot_start);void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, -pte_t *ptep,pte_t pte) +pte_t *ptep,pte_t old_pte,pte_t pte) {pgste_t pgste;struct mm_struct *mm = vma->vm_mm;
arch/x86/include/asm/paravirt.h +1 -1
@@ -428,7 +428,7 @@ static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned }static inline void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, -pte_t *ptep,pte_t pte) +pte_t *ptep,pte_t old_pte,pte_t pte) {if (sizeof (pteval_t) >sizeof (long ))
fs/proc/task_mmu.c +5 -3
@@ -940,10 +940,12 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,pte_t ptent = *pte;if (pte_present (ptent)) { - ptent =ptep_modify_prot_start (vma, addr, pte); - ptent =pte_wrprotect (ptent); +pte_t old_pte; + + old_pte =ptep_modify_prot_start (vma, addr, pte); + ptent =pte_wrprotect (old_pte); ptent =pte_clear_soft_dirty (ptent); -ptep_modify_prot_commit (vma, addr, pte, ptent); +ptep_modify_prot_commit (vma, addr, pte, old_pte, ptent); }else if (is_swap_pte (ptent)) { ptent =pte_swp_clear_soft_dirty (ptent);set_pte_at (vma->vm_mm, addr, pte, ptent);
include/asm-generic/pgtable.h +1 -1
@@ -619,7 +619,7 @@ static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,*/ static inline void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, -pte_t *ptep,pte_t pte) +pte_t *ptep,pte_t old_pte,pte_t pte) {__ptep_modify_prot_commit (vma, addr, ptep, pte); }
mm/memory.c +4 -4
@@ -3568,7 +3568,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)int last_cpupid;int target_nid;bool migrated = false; -pte_t pte; +pte_t pte, old_pte;bool was_writable =pte_savedwrite (vmf->orig_pte);int flags =0 ;
@@ -3588,12 +3588,12 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)* Make it present again, Depending on how arch implementes non * accessible ptes, some can allow access by kernel mode. */ - pte =ptep_modify_prot_start (vma, vmf->address, vmf->pte); - pte =pte_modify (pte, vma->vm_page_prot); + old_pte =ptep_modify_prot_start (vma, vmf->address, vmf->pte); + pte =pte_modify (old_pte, vma->vm_page_prot); pte =pte_mkyoung (pte);if (was_writable) pte =pte_mkwrite (pte); -ptep_modify_prot_commit (vma, vmf->address, vmf->pte, pte); +ptep_modify_prot_commit (vma, vmf->address, vmf->pte, old_pte, pte);update_mmu_cache (vma, vmf->address, vmf->pte); page =vm_normal_page (vma, vmf->address, pte);
mm/mprotect.c +3 -3
@@ -110,8 +110,8 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,continue ; } - ptent =ptep_modify_prot_start (vma, addr, pte); - ptent =pte_modify (ptent, newprot); + oldpte =ptep_modify_prot_start (vma, addr, pte); + ptent =pte_modify (oldpte, newprot);if (preserve_write) ptent =pte_mk_savedwrite (ptent);
@@ -121,7 +121,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd, !(vma->vm_flags & VM_SOFTDIRTY))) { ptent =pte_mkwrite (ptent); } -ptep_modify_prot_commit (vma, addr, pte, ptent); +ptep_modify_prot_commit (vma, addr, pte, oldpte, ptent); pages++; }else if (IS_ENABLED (CONFIG_MIGRATION )) {swp_entry_t entry =pte_to_swp_entry (oldpte);
Signature · 1 line
2.19.2
PATCH v4 3/5 arch/powerpc/mm: Nest MMU workaround for mprotect RW upgrade.
In reply to Aneesh Kumar K.VNestMMU requires us to mark the pte invalid and flush the tlb when we do a RW upgrade of pte. We fixed a variant of this in the fault path in commit
Fixes bd5050e38aec ("powerpc/mm/radix: Change pte relax sequence to handle nest MMU hang")
Do the same for mprotect upgrades.
Hugetlb is handled in the next patch.
Signed-off-by Aneesh Kumar K.V <[email protected]>
--- arch/powerpc/include/asm/book3s/64/pgtable.h | 18 +++++++++++++ arch/powerpc/include/asm/book3s/64/radix.h | 4 +++ arch/powerpc/mm/pgtable-book3s64.c | 27 ++++++++++++++++++++ arch/powerpc/mm/pgtable-radix.c | 18 +++++++++++++ 4 files changed, 67 insertions(+)
arch/powerpc/include/asm/book3s/64/pgtable.h +18 -0
@@ -1314,6 +1314,24 @@ static inline int pud_pfn(pud_t pud)BUILD_BUG ();return 0 ; } +#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION +pte_t ptep_modify_prot_start (struct vm_area_struct *,unsigned long ,pte_t *); +void ptep_modify_prot_commit (struct vm_area_struct *,unsigned long , +pte_t *,pte_t ,pte_t ); + +/* +* Returns true for a R -> RW upgrade of pte +*/ +static inline bool is_pte_rw_upgrade (unsigned long old_val,unsigned long new_val) +{ +if (!(old_val & _PAGE_READ)) +return false; + +if ((!(old_val & _PAGE_WRITE)) && (new_val & _PAGE_WRITE)) +return true; + +return false; +}#endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
arch/powerpc/include/asm/book3s/64/radix.h +4 -0
@@ -127,6 +127,10 @@ extern void radix__ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep pte_t entry, unsigned long address, int psize); +extern void radix__ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t pte); +static inline unsigned long __radix_pte_update (pte_t *ptep,unsigned long clr,unsigned long set) {
arch/powerpc/mm/pgtable-book3s64.c +27 -0
@@ -400,3 +400,30 @@ void arch_report_meminfo(struct seq_file *m) atomic_long_read(&direct_pages_count[MMU_PAGE_1G]) << 20); }#endif /* CONFIG_PROC_FS */ + +pte_t ptep_modify_prot_start (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep) +{ +unsigned long pte_val; + +/* +* Clear the _PAGE_PRESENT so that no hardware parallel update is +* possible. Also keep the pte_present true so that we don't take +* wrong fault. +*/ + pte_val =pte_update (vma->vm_mm, addr, ptep, _PAGE_PRESENT, _PAGE_INVALID,0 ); + +return __pte (pte_val); + +} +EXPORT_SYMBOL (ptep_modify_prot_start); + +void ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ +if (radix_enabled ()) +return radix__ptep_modify_prot_commit (vma, addr, + ptep, old_pte, pte); +set_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (ptep_modify_prot_commit);
arch/powerpc/mm/pgtable-radix.c +18 -0
@@ -1063,3 +1063,21 @@ void radix__ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep, }/* See ptesync comment in radix__set_pte_at */ } + +void radix__ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t pte) +{ +struct mm_struct *mm = vma->vm_mm; + +/* +* To avoid NMMU hang while relaxing access we need to flush the tlb before +* we set the new value. We need to do this only for radix, because hash +* translation does flush when updating the linux pte. +*/ +if (is_pte_rw_upgrade (pte_val (old_pte),pte_val (pte)) && + (atomic_read (&mm->context.copros) >0 )) +radix__flush_tlb_page (vma, addr); + +set_pte_at (mm, addr, ptep, pte); +}
Signature · 1 line
2.19.2
PATCH v4 4/5 mm/hugetlb: Add prot_modify_start/commit sequence for hugetlb update
In reply to Aneesh Kumar K.VArchitectures like ppc64 requires to do a conditional tlb flush based on the old and new value of pte. Follow the regular pte change protection sequence for hugetlb too. This allow the architectures to override the update sequence.
Signed-off-by Aneesh Kumar K.V <[email protected]>
--- include/linux/hugetlb.h | 20 ++++++++++++++++++++ mm/hugetlb.c | 8 +++++--- 2 files changed, 25 insertions(+), 3 deletions(-)
include/linux/hugetlb.h +20 -0
@@ -543,6 +543,26 @@ static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr set_huge_pte_at(mm, addr, ptep, pte); }#endif + +#ifndef huge_ptep_modify_prot_start +#define huge_ptep_modify_prot_start huge_ptep_modify_prot_start +static inline pte_t huge_ptep_modify_prot_start (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) +{ +return huge_ptep_get_and_clear (vma->vm_mm, addr, ptep); +} +#endif + +#ifndef huge_ptep_modify_prot_commit +#define huge_ptep_modify_prot_commit huge_ptep_modify_prot_commit +static inline void huge_ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t pte) +{ +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +#endif +#else /* CONFIG_HUGETLB_PAGE */ struct hstate {};#define alloc_huge_page (v, a, r) NULL
mm/hugetlb.c +5 -3
@@ -4388,10 +4388,12 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,continue ; }if (!huge_pte_none (pte)) { - pte =huge_ptep_get_and_clear (mm, address, ptep); - pte =pte_mkhuge (huge_pte_modify (pte, newprot)); +pte_t old_pte; + + old_pte =huge_ptep_modify_prot_start (vma, address, ptep); + pte =pte_mkhuge (huge_pte_modify (old_pte, newprot)); pte =arch_make_huge_pte (pte, vma,NULL ,0 ); -set_huge_pte_at (mm, address, ptep, pte); +huge_ptep_modify_prot_commit (vma, address, ptep, old_pte, pte); pages++; }spin_unlock (ptl);
Signature · 1 line
2.19.2
PATCH v4 5/5 arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
In reply to Aneesh Kumar K.VNestMMU requires us to mark the pte invalid and flush the tlb when we do a RW upgrade of pte. We fixed a variant of this in the fault path in commit
Fixes bd5050e38aec ("powerpc/mm/radix: Change pte relax sequence to handle nest MMU hang")
Signed-off-by Aneesh Kumar K.V <[email protected]>
--- arch/powerpc/include/asm/book3s/64/hugetlb.h | 12 +++++++++ arch/powerpc/mm/hugetlbpage-hash64.c | 27 ++++++++++++++++++++ arch/powerpc/mm/hugetlbpage-radix.c | 17 ++++++++++++ 3 files changed, 56 insertions(+)
arch/powerpc/include/asm/book3s/64/hugetlb.h +12 -0
@@ -13,6 +13,10 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); +extern void radix__huge_ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t pte); +static inline int hstate_get_psize (struct hstate *hstate) {unsigned long shift;
@@ -42,4 +46,12 @@ static inline bool gigantic_page_supported(void)/* hugepd entry valid bit */ #define HUGEPD_VAL_BITS (0x8000000000000000UL) +#define huge_ptep_modify_prot_start huge_ptep_modify_prot_start +extern pte_t huge_ptep_modify_prot_start (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep); + +#define huge_ptep_modify_prot_commit huge_ptep_modify_prot_commit +extern void huge_ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t new_pte);#endif
arch/powerpc/mm/hugetlbpage-hash64.c +27 -0
@@ -121,3 +121,30 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, *ptep =__pte (new_pte & ~H_PAGE_BUSY );return 0 ; } + +pte_t huge_ptep_modify_prot_start (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) +{ +unsigned long pte_val; +/* +* Clear the _PAGE_PRESENT so that no hardware parallel update is +* possible. Also keep the pte_present true so that we don't take +* wrong fault. +*/ + pte_val =pte_update (vma->vm_mm, addr, ptep, + _PAGE_PRESENT, _PAGE_INVALID,1 ); + +return __pte (pte_val); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_start); + +void huge_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ + +if (radix_enabled ()) +return radix__huge_ptep_modify_prot_commit (vma, addr, ptep, + old_pte, pte); +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_commit);
arch/powerpc/mm/hugetlbpage-radix.c +17 -0
@@ -90,3 +90,20 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,return vm_unmapped_area (&info); } + +void radix__huge_ptep_modify_prot_commit (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep, +pte_t old_pte,pte_t pte) +{ +struct mm_struct *mm = vma->vm_mm; + +/* +* To avoid NMMU hang while relaxing access we need to flush the tlb before +* we set the new value. +*/ +if (is_pte_rw_upgrade (pte_val (old_pte),pte_val (pte)) && + (atomic_read (&mm->context.copros) >0 )) +radix__flush_hugetlb_page (vma, addr); + +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +}
Signature · 1 line
2.19.2
Re: [PATCH V4 5/5] arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
Quoted 1 line · Aneesh Kumar K.V
+EXPORT_SYMBOL(huge_ptep_modify_prot_start);
The only user of this function is the one you added in the last patch in mm/hugetlb.c, so there is no need to export this function.
arch/powerpc/mm/hugetlbpage-hash64.c int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, · line 150
Quoted 11 lines
arch/powerpc/mm/hugetlbpage-hash64.c +11
-0+ +void huge_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ + +if (radix_enabled ()) +return radix__huge_ptep_modify_prot_commit (vma, addr, ptep, + old_pte, pte); +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_commit);
Same here.
Re: [PATCH V4 5/5] arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
Quoted 6 lines · Christoph Hellwig
Quoted 1 line · Aneesh Kumar K.V
+EXPORT_SYMBOL(huge_ptep_modify_prot_start);
The only user of this function is the one you added in the last patch in
mm/hugetlb.c, so there is no need to export this function.
Quoted 13 lines
arch/powerpc/mm/hugetlbpage-hash64.c int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, · line 150
Quoted 11 lines
arch/powerpc/mm/hugetlbpage-hash64.c +11
-0+ +void huge_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ + +if (radix_enabled ()) +return radix__huge_ptep_modify_prot_commit (vma, addr, ptep, + old_pte, pte); +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_commit);Same here.
That was done considering that ptep_modify_prot_start/commit was defined in asm-generic/pgtable.h. I was trying to make sure I didn't break anything with the patch. Also s390 do have that EXPORT_SYMBOL() for the same. hugetlb just inherited that.
If you feel strongly about it, I can drop the EXPORT_SYMBOL().
-aneesh
Re: [PATCH V4 5/5] arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
Quoted 6 lines · Aneesh Kumar K.V
That was done considering that ptep_modify_prot_start/commit was defined in
asm-generic/pgtable.h. I was trying to make sure I didn't break anything with the patch. Also s390 do have thatEXPORT_SYMBOL()for the same. hugetlb just inherited that.If you feel strongly about it, I can drop the
EXPORT_SYMBOL().
Yes. And we should probably remove the s390 as well as it isn't used either.
Re: [PATCH V4 5/5] arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
In reply to Aneesh Kumar K.VQuoted 22 lines · Aneesh Kumar K.V
Quoted 6 lines · Christoph Hellwig
Quoted 1 line · Aneesh Kumar K.V
+EXPORT_SYMBOL(huge_ptep_modify_prot_start);
The only user of this function is the one you added in the last patch in
mm/hugetlb.c, so there is no need to export this function.Quoted 13 lines
arch/powerpc/mm/hugetlbpage-hash64.c int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, · line 150
Quoted 11 lines
arch/powerpc/mm/hugetlbpage-hash64.c +11
-0+ +void huge_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ + +if (radix_enabled ()) +return radix__huge_ptep_modify_prot_commit (vma, addr, ptep, + old_pte, pte); +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_commit);Same here.
Quoted 6 lines
That was done considering that ptep_modify_prot_start/commit was defined in
asm-generic/pgtable.h. I was trying to make sure I didn't break anything with the patch. Also s390 do have thatEXPORT_SYMBOL()for the same. hugetlb just inherited that.If you feel strongly about it, I can drop the
EXPORT_SYMBOL().
At the very least it should be _GPL
Cheers,
Ben.
Re: [PATCH V4 5/5] arch/powerpc/mm/hugetlb: NestMMU workaround for hugetlb mprotect RW upgrade
Quoted 23 lines · Benjamin Herrenschmidt
Quoted 22 lines · Aneesh Kumar K.V
Quoted 19 lines · Christoph Hellwig
Quoted 1 line · Aneesh Kumar K.V
+EXPORT_SYMBOL(huge_ptep_modify_prot_start);
The only user of this function is the one you added in the last patch in
mm/hugetlb.c, so there is no need to export this function.arch/powerpc/mm/hugetlbpage-hash64.c int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, · line 150
Quoted 11 lines
arch/powerpc/mm/hugetlbpage-hash64.c +11
-0+ +void huge_ptep_modify_prot_commit (struct vm_area_struct *vma,unsigned long addr, +pte_t *ptep,pte_t old_pte,pte_t pte) +{ + +if (radix_enabled ()) +return radix__huge_ptep_modify_prot_commit (vma, addr, ptep, + old_pte, pte); +set_huge_pte_at (vma->vm_mm, addr, ptep, pte); +} +EXPORT_SYMBOL (huge_ptep_modify_prot_commit);Same here.
Quoted 8 lines
Quoted 6 lines
That was done considering that ptep_modify_prot_start/commit was defined in
asm-generic/pgtable.h. I was trying to make sure I didn't break anything with the patch. Also s390 do have thatEXPORT_SYMBOL()for the same. hugetlb just inherited that.If you feel strongly about it, I can drop the
EXPORT_SYMBOL().At the very least it should be
_GPL
In general yes, but in this case it just needs to go. The s390 maintainers already removed a patch to remove their ptep_modify_prot_start / ptep_modify_prot_commit exports.
This series seems to miss patches 1 and 2.
Quoted 1 line · Christoph Hellwig
This series seems to miss patches 1 and 2.
https://lore.kernel.org/linuxppc-dev/[email protected]/
https://lore.kernel.org/linuxppc-dev/[email protected]/
-aneesh
Quoted 1 line · Christoph Hellwig
This series seems to miss patches 1 and 2.
Odd, I got them...
Ben.