Messages in this thread | | | Date | Wed, 27 Oct 2021 06:39:18 +0200 | Subject | Re: [PATCH 1/3] powerpc/nohash: Fix __ptep_set_access_flags() and ptep_set_wrprotect() | From | Christophe Leroy <> |
| |
Le 27/10/2021 à 06:23, Nicholas Piggin a écrit : > Excerpts from Christophe Leroy's message of October 26, 2021 3:39 pm: >> Commit 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code") >> changed those two functions to use pte helpers to determine which >> bits to clear and which bits to set. >> >> This change was based on the assumption that bits to be set/cleared >> are always the same and can be determined by applying the pte >> manipulation helpers on __pte(0). >> >> But on platforms like book3e, the bits depend on whether the page >> is a user page or not. >> >> For the time being it more or less works because of _PAGE_EXEC being >> used for user pages only and exec right being set at all time on >> kernel page. But following patch will clean that and output of >> pte_mkexec() will depend on the page being a user or kernel page. >> >> Instead of trying to make an even more complicated helper where bits >> would become dependent on the final pte value, come back to a more >> static situation like before commit 26973fa5ac0e ("powerpc/mm: use >> pte helpers in generic code"), by introducing an 8xx specific >> version of __ptep_set_access_flags() and ptep_set_wrprotect(). > > What is this actually fixing? Does it change anything itself, or > just a preparation patch?
Just a preparation patch I think.
I didn't flag it for stable.
Once patch 2 is applied, __ptep_set_access_flags() doesn't work anymore without this patch, because then pte_mkexec(__pte(0)) sets SX and clears UX while pte_mkexec(__pte(~0)) sets UX and clears SX
Christophe
> > Thanks, > Nick > >> >> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code") >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v3: No change >> v2: New >> --- >> arch/powerpc/include/asm/nohash/32/pgtable.h | 17 +++++++-------- >> arch/powerpc/include/asm/nohash/32/pte-8xx.h | 22 ++++++++++++++++++++ >> 2 files changed, 30 insertions(+), 9 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h >> index 34ce50da1850..11c6849f7864 100644 >> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h >> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h >> @@ -306,30 +306,29 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr, >> } >> >> #define __HAVE_ARCH_PTEP_SET_WRPROTECT >> +#ifndef ptep_set_wrprotect >> static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr, >> pte_t *ptep) >> { >> - unsigned long clr = ~pte_val(pte_wrprotect(__pte(~0))); >> - unsigned long set = pte_val(pte_wrprotect(__pte(0))); >> - >> - pte_update(mm, addr, ptep, clr, set, 0); >> + pte_update(mm, addr, ptep, _PAGE_RW, 0, 0); >> } >> +#endif >> >> +#ifndef __ptep_set_access_flags >> static inline void __ptep_set_access_flags(struct vm_area_struct *vma, >> pte_t *ptep, pte_t entry, >> unsigned long address, >> int psize) >> { >> - pte_t pte_set = pte_mkyoung(pte_mkdirty(pte_mkwrite(pte_mkexec(__pte(0))))); >> - pte_t pte_clr = pte_mkyoung(pte_mkdirty(pte_mkwrite(pte_mkexec(__pte(~0))))); >> - unsigned long set = pte_val(entry) & pte_val(pte_set); >> - unsigned long clr = ~pte_val(entry) & ~pte_val(pte_clr); >> + unsigned long set = pte_val(entry) & >> + (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC); >> int huge = psize > mmu_virtual_psize ? 1 : 0; >> >> - pte_update(vma->vm_mm, address, ptep, clr, set, huge); >> + pte_update(vma->vm_mm, address, ptep, 0, set, huge); >> >> flush_tlb_page(vma, address); >> } >> +#endif >> >> static inline int pte_young(pte_t pte) >> { >> diff --git a/arch/powerpc/include/asm/nohash/32/pte-8xx.h b/arch/powerpc/include/asm/nohash/32/pte-8xx.h >> index fcc48d590d88..1a89ebdc3acc 100644 >> --- a/arch/powerpc/include/asm/nohash/32/pte-8xx.h >> +++ b/arch/powerpc/include/asm/nohash/32/pte-8xx.h >> @@ -136,6 +136,28 @@ static inline pte_t pte_mkhuge(pte_t pte) >> >> #define pte_mkhuge pte_mkhuge >> >> +static inline pte_basic_t pte_update(struct mm_struct *mm, unsigned long addr, pte_t *p, >> + unsigned long clr, unsigned long set, int huge); >> + >> +static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr, pte_t *ptep) >> +{ >> + pte_update(mm, addr, ptep, 0, _PAGE_RO, 0); >> +} >> +#define ptep_set_wrprotect ptep_set_wrprotect >> + >> +static inline void __ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep, >> + pte_t entry, unsigned long address, int psize) >> +{ >> + unsigned long set = pte_val(entry) & (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_EXEC); >> + unsigned long clr = ~pte_val(entry) & _PAGE_RO; >> + int huge = psize > mmu_virtual_psize ? 1 : 0; >> + >> + pte_update(vma->vm_mm, address, ptep, clr, set, huge); >> + >> + flush_tlb_page(vma, address); >> +} >> +#define __ptep_set_access_flags __ptep_set_access_flags >> + >> static inline unsigned long pgd_leaf_size(pgd_t pgd) >> { >> if (pgd_val(pgd) & _PMD_PAGE_8M) >> -- >> 2.31.1 >> >>
| |