| Message ID | 20190214064540.26597-1-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c746ca00f5eac6224eda02f39ebdc48fabfad3c5 |
| Headers | show |
| Series | powerpc/book3s: Remove a few page table update interfaces. | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/build-ppc64le | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64be | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64e | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-pmac32 | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 63 lines checked |
Aneesh Kumar K.V's on February 14, 2019 4:45 pm: > When updating page tables, we need to make sure we fill the page table > entry valid bit. We should be using page table populate interface for > updating the table entries. The page table 'set' interface allows > updating the raw value of page table entry. This can result in > updating the entry wrongly. Remove the 'set' interface so that we avoid > its future usage. Removing a pointless indirection is fine by me, but I can't figure out how this change could help to prevent entry being updated incorrectly. Thanks, Nick > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- > arch/powerpc/include/asm/book3s/64/pgalloc.h | 8 ++++---- > arch/powerpc/include/asm/book3s/64/pgtable.h | 14 -------------- > 2 files changed, 4 insertions(+), 18 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h > index 9c1173283b96..138bc2ecc0c4 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgalloc.h > +++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h > @@ -111,7 +111,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd) > > static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud) > { > - pgd_set(pgd, __pgtable_ptr_val(pud) | PGD_VAL_BITS); > + *pgd = __pgd(__pgtable_ptr_val(pud) | PGD_VAL_BITS); > } > > static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr) > @@ -138,7 +138,7 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud) > > static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd) > { > - pud_set(pud, __pgtable_ptr_val(pmd) | PUD_VAL_BITS); > + *pud = __pud(__pgtable_ptr_val(pmd) | PUD_VAL_BITS); > } > > static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud, > @@ -176,13 +176,13 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd, > static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd, > pte_t *pte) > { > - pmd_set(pmd, __pgtable_ptr_val(pte) | PMD_VAL_BITS); > + *pmd = __pmd(__pgtable_ptr_val(pte) | PMD_VAL_BITS); > } > > static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd, > pgtable_t pte_page) > { > - pmd_set(pmd, __pgtable_ptr_val(pte_page) | PMD_VAL_BITS); > + *pmd = __pmd(__pgtable_ptr_val(pte_page) | PMD_VAL_BITS); > } > > static inline pgtable_t pmd_pgtable(pmd_t pmd) > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h > index dc71e2b92003..a24e00fb7fa7 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > @@ -853,11 +853,6 @@ static inline bool pte_ci(pte_t pte) > return false; > } > > -static inline void pmd_set(pmd_t *pmdp, unsigned long val) > -{ > - *pmdp = __pmd(val); > -} > - > static inline void pmd_clear(pmd_t *pmdp) > { > *pmdp = __pmd(0); > @@ -889,11 +884,6 @@ static inline int pmd_bad(pmd_t pmd) > return hash__pmd_bad(pmd); > } > > -static inline void pud_set(pud_t *pudp, unsigned long val) > -{ > - *pudp = __pud(val); > -} > - > static inline void pud_clear(pud_t *pudp) > { > *pudp = __pud(0); > @@ -936,10 +926,6 @@ static inline bool pud_access_permitted(pud_t pud, bool write) > } > > #define pgd_write(pgd) pte_write(pgd_pte(pgd)) > -static inline void pgd_set(pgd_t *pgdp, unsigned long val) > -{ > - *pgdp = __pgd(val); > -} > > static inline void pgd_clear(pgd_t *pgdp) > { > -- > 2.20.1 > >
On 2/19/19 11:23 AM, Nicholas Piggin wrote: > Aneesh Kumar K.V's on February 14, 2019 4:45 pm: >> When updating page tables, we need to make sure we fill the page table >> entry valid bit. We should be using page table populate interface for >> updating the table entries. The page table 'set' interface allows >> updating the raw value of page table entry. This can result in >> updating the entry wrongly. Remove the 'set' interface so that we avoid >> its future usage. > > Removing a pointless indirection is fine by me, but I can't > figure out how this change could help to prevent entry being updated > incorrectly. I guess my commit message is confusing. What I meant is this avoid the usage of pgd/pud_set from other kernel code without updating the page table entry valid bits. The idea is to make pgd_populate as the only interface for update pgd entries. -aneesh
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h index 9c1173283b96..138bc2ecc0c4 100644 --- a/arch/powerpc/include/asm/book3s/64/pgalloc.h +++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h @@ -111,7 +111,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd) static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud) { - pgd_set(pgd, __pgtable_ptr_val(pud) | PGD_VAL_BITS); + *pgd = __pgd(__pgtable_ptr_val(pud) | PGD_VAL_BITS); } static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr) @@ -138,7 +138,7 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud) static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd) { - pud_set(pud, __pgtable_ptr_val(pmd) | PUD_VAL_BITS); + *pud = __pud(__pgtable_ptr_val(pmd) | PUD_VAL_BITS); } static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud, @@ -176,13 +176,13 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd, static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd, pte_t *pte) { - pmd_set(pmd, __pgtable_ptr_val(pte) | PMD_VAL_BITS); + *pmd = __pmd(__pgtable_ptr_val(pte) | PMD_VAL_BITS); } static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd, pgtable_t pte_page) { - pmd_set(pmd, __pgtable_ptr_val(pte_page) | PMD_VAL_BITS); + *pmd = __pmd(__pgtable_ptr_val(pte_page) | PMD_VAL_BITS); } static inline pgtable_t pmd_pgtable(pmd_t pmd) diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index dc71e2b92003..a24e00fb7fa7 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -853,11 +853,6 @@ static inline bool pte_ci(pte_t pte) return false; } -static inline void pmd_set(pmd_t *pmdp, unsigned long val) -{ - *pmdp = __pmd(val); -} - static inline void pmd_clear(pmd_t *pmdp) { *pmdp = __pmd(0); @@ -889,11 +884,6 @@ static inline int pmd_bad(pmd_t pmd) return hash__pmd_bad(pmd); } -static inline void pud_set(pud_t *pudp, unsigned long val) -{ - *pudp = __pud(val); -} - static inline void pud_clear(pud_t *pudp) { *pudp = __pud(0); @@ -936,10 +926,6 @@ static inline bool pud_access_permitted(pud_t pud, bool write) } #define pgd_write(pgd) pte_write(pgd_pte(pgd)) -static inline void pgd_set(pgd_t *pgdp, unsigned long val) -{ - *pgdp = __pgd(val); -} static inline void pgd_clear(pgd_t *pgdp) {
When updating page tables, we need to make sure we fill the page table entry valid bit. We should be using page table populate interface for updating the table entries. The page table 'set' interface allows updating the raw value of page table entry. This can result in updating the entry wrongly. Remove the 'set' interface so that we avoid its future usage. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- arch/powerpc/include/asm/book3s/64/pgalloc.h | 8 ++++---- arch/powerpc/include/asm/book3s/64/pgtable.h | 14 -------------- 2 files changed, 4 insertions(+), 18 deletions(-)