Re: [PATCH] powerpc/book3s: Remove a few page table update interfaces.

[Nicholas Piggin <npiggin@gmail.com>]

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
> 
>