Re: [PATCH v6 21/38] powerpc: Implement the new page table range API
On Thu, Aug 03, 2023 at 04:38:14PM -0700, Nathan Chancellor wrote: > > -EXPORT_SYMBOL(flush_dcache_icache_page); > > Apologies if this has already been fixed or reported, I searched lore > and did not find anything. The dropping of this export in combination > with the conversion above appears to cause ARCH=powerpc allmodconfig to > fail with: > > ERROR: modpost: "flush_dcache_icache_folio" [arch/powerpc/kvm/kvm-pr.ko] > undefined! > > I don't know if this should be re-exported or not but that does > obviously resolve the issue. Well, that was clumsy of me. I (and the Intel build bot) did test several build combos, but clearly didn't manage to find a config that showed this problem. Andrew, a fix patch for you to integrate, if you would: diff --git a/arch/powerpc/mm/cacheflush.c b/arch/powerpc/mm/cacheflush.c index 8760d2223abe..15189592da09 100644 --- a/arch/powerpc/mm/cacheflush.c +++ b/arch/powerpc/mm/cacheflush.c @@ -172,6 +172,7 @@ void flush_dcache_icache_folio(struct folio *folio) flush_dcache_icache_phys((pfn + i) * PAGE_SIZE); } } +EXPORT_SYMBOL(flush_dcache_icache_folio); void clear_user_page(void *page, unsigned long vaddr, struct page *pg) {
Re: [PATCH v6 21/38] powerpc: Implement the new page table range API
Hi Matthew, On Wed, Aug 02, 2023 at 04:13:49PM +0100, Matthew Wilcox (Oracle) wrote: > Add set_ptes(), update_mmu_cache_range() and flush_dcache_folio(). > Change the PG_arch_1 (aka PG_dcache_dirty) flag from being per-page to > per-folio. > > Signed-off-by: Matthew Wilcox (Oracle) > Acked-by: Mike Rapoport (IBM) > Cc: Michael Ellerman > Cc: Nicholas Piggin > Cc: Christophe Leroy > Cc: linuxppc-dev@lists.ozlabs.org ... > diff --git a/arch/powerpc/include/asm/kvm_ppc.h > b/arch/powerpc/include/asm/kvm_ppc.h > index d16d80ad2ae4..b4da8514af43 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -894,7 +894,7 @@ void kvmppc_init_lpid(unsigned long nr_lpids); > > static inline void kvmppc_mmu_flush_icache(kvm_pfn_t pfn) > { > - struct page *page; > + struct folio *folio; > /* >* We can only access pages that the kernel maps >* as memory. Bail out for unmapped ones. > @@ -903,10 +903,10 @@ static inline void kvmppc_mmu_flush_icache(kvm_pfn_t > pfn) > return; > > /* Clear i-cache for new pages */ > - page = pfn_to_page(pfn); > - if (!test_bit(PG_dcache_clean, >flags)) { > - flush_dcache_icache_page(page); > - set_bit(PG_dcache_clean, >flags); > + folio = page_folio(pfn_to_page(pfn)); > + if (!test_bit(PG_dcache_clean, >flags)) { > + flush_dcache_icache_folio(folio); > + set_bit(PG_dcache_clean, >flags); > } > } ... > diff --git a/arch/powerpc/mm/cacheflush.c b/arch/powerpc/mm/cacheflush.c > index 0e9b4879c0f9..8760d2223abe 100644 > --- a/arch/powerpc/mm/cacheflush.c > +++ b/arch/powerpc/mm/cacheflush.c > @@ -148,44 +148,30 @@ static void __flush_dcache_icache(void *p) > invalidate_icache_range(addr, addr + PAGE_SIZE); > } > > -static void flush_dcache_icache_hugepage(struct page *page) > +void flush_dcache_icache_folio(struct folio *folio) > { > - int i; > - int nr = compound_nr(page); > + unsigned int i, nr = folio_nr_pages(folio); > > - if (!PageHighMem(page)) { > + if (flush_coherent_icache()) > + return; > + > + if (!folio_test_highmem(folio)) { > + void *addr = folio_address(folio); > for (i = 0; i < nr; i++) > - __flush_dcache_icache(lowmem_page_address(page + i)); > - } else { > + __flush_dcache_icache(addr + i * PAGE_SIZE); > + } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > > sizeof(void *)) { > for (i = 0; i < nr; i++) { > - void *start = kmap_local_page(page + i); > + void *start = kmap_local_folio(folio, i * PAGE_SIZE); > > __flush_dcache_icache(start); > kunmap_local(start); > } > - } > -} > - > -void flush_dcache_icache_page(struct page *page) > -{ > - if (flush_coherent_icache()) > - return; > - > - if (PageCompound(page)) > - return flush_dcache_icache_hugepage(page); > - > - if (!PageHighMem(page)) { > - __flush_dcache_icache(lowmem_page_address(page)); > - } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > > sizeof(void *)) { > - void *start = kmap_local_page(page); > - > - __flush_dcache_icache(start); > - kunmap_local(start); > } else { > - flush_dcache_icache_phys(page_to_phys(page)); > + unsigned long pfn = folio_pfn(folio); > + for (i = 0; i < nr; i++) > + flush_dcache_icache_phys((pfn + i) * PAGE_SIZE); > } > } > -EXPORT_SYMBOL(flush_dcache_icache_page); Apologies if this has already been fixed or reported, I searched lore and did not find anything. The dropping of this export in combination with the conversion above appears to cause ARCH=powerpc allmodconfig to fail with: ERROR: modpost: "flush_dcache_icache_folio" [arch/powerpc/kvm/kvm-pr.ko] undefined! I don't know if this should be re-exported or not but that does obviously resolve the issue. Cheers, Nathan
[PATCH v6 21/38] powerpc: Implement the new page table range API
Add set_ptes(), update_mmu_cache_range() and flush_dcache_folio(). Change the PG_arch_1 (aka PG_dcache_dirty) flag from being per-page to per-folio. Signed-off-by: Matthew Wilcox (Oracle) Acked-by: Mike Rapoport (IBM) Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: linuxppc-dev@lists.ozlabs.org --- arch/powerpc/include/asm/book3s/32/pgtable.h | 5 -- arch/powerpc/include/asm/book3s/64/pgtable.h | 6 +-- arch/powerpc/include/asm/book3s/pgtable.h| 11 ++-- arch/powerpc/include/asm/cacheflush.h| 14 -- arch/powerpc/include/asm/kvm_ppc.h | 10 ++-- arch/powerpc/include/asm/nohash/pgtable.h| 16 ++ arch/powerpc/include/asm/pgtable.h | 12 + arch/powerpc/mm/book3s64/hash_utils.c| 11 ++-- arch/powerpc/mm/cacheflush.c | 40 +-- arch/powerpc/mm/nohash/e500_hugetlbpage.c| 3 +- arch/powerpc/mm/pgtable.c| 53 11 files changed, 88 insertions(+), 93 deletions(-) diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h index 7bf1fe7297c6..5f12b9382909 100644 --- a/arch/powerpc/include/asm/book3s/32/pgtable.h +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h @@ -462,11 +462,6 @@ static inline pte_t pfn_pte(unsigned long pfn, pgprot_t pgprot) pgprot_val(pgprot)); } -static inline unsigned long pte_pfn(pte_t pte) -{ - return pte_val(pte) >> PTE_RPN_SHIFT; -} - /* Generic modifiers for PTE bits */ static inline pte_t pte_wrprotect(pte_t pte) { diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index a8204566cfd0..8269b231c533 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -104,6 +104,7 @@ * and every thing below PAGE_SHIFT; */ #define PTE_RPN_MASK (((1UL << _PAGE_PA_MAX) - 1) & (PAGE_MASK)) +#define PTE_RPN_SHIFT PAGE_SHIFT /* * set of bits not changed in pmd_modify. Even though we have hash specific bits * in here, on radix we expect them to be zero. @@ -569,11 +570,6 @@ static inline pte_t pfn_pte(unsigned long pfn, pgprot_t pgprot) return __pte(((pte_basic_t)pfn << PAGE_SHIFT) | pgprot_val(pgprot) | _PAGE_PTE); } -static inline unsigned long pte_pfn(pte_t pte) -{ - return (pte_val(pte) & PTE_RPN_MASK) >> PAGE_SHIFT; -} - /* Generic modifiers for PTE bits */ static inline pte_t pte_wrprotect(pte_t pte) { diff --git a/arch/powerpc/include/asm/book3s/pgtable.h b/arch/powerpc/include/asm/book3s/pgtable.h index d18b748ea3ae..3b7bd36a2321 100644 --- a/arch/powerpc/include/asm/book3s/pgtable.h +++ b/arch/powerpc/include/asm/book3s/pgtable.h @@ -9,13 +9,6 @@ #endif #ifndef __ASSEMBLY__ -/* Insert a PTE, top-level function is out of line. It uses an inline - * low level function in the respective pgtable-* files - */ -extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, - pte_t pte); - - #define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS extern int ptep_set_access_flags(struct vm_area_struct *vma, unsigned long address, pte_t *ptep, pte_t entry, int dirty); @@ -36,7 +29,9 @@ void __update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t * corresponding HPTE into the hash table ahead of time, instead of * waiting for the inevitable extra hash-table miss exception. */ -static inline void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep) +static inline void update_mmu_cache_range(struct vm_fault *vmf, + struct vm_area_struct *vma, unsigned long address, + pte_t *ptep, unsigned int nr) { if (IS_ENABLED(CONFIG_PPC32) && !mmu_has_feature(MMU_FTR_HPTE_TABLE)) return; diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h index 7564dd4fd12b..ef7d2de33b89 100644 --- a/arch/powerpc/include/asm/cacheflush.h +++ b/arch/powerpc/include/asm/cacheflush.h @@ -35,13 +35,19 @@ static inline void flush_cache_vmap(unsigned long start, unsigned long end) * It just marks the page as not i-cache clean. We do the i-cache * flush later when the page is given to a user process, if necessary. */ -static inline void flush_dcache_page(struct page *page) +static inline void flush_dcache_folio(struct folio *folio) { if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) return; /* avoid an atomic op if possible */ - if (test_bit(PG_dcache_clean, >flags)) - clear_bit(PG_dcache_clean, >flags); + if (test_bit(PG_dcache_clean, >flags)) + clear_bit(PG_dcache_clean, >flags); +} +#define flush_dcache_folio flush_dcache_folio + +static inline void flush_dcache_page(struct page *page) +{ + flush_dcache_folio(page_folio(page)); } void flush_icache_range(unsigned long start,