Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 31 Jan 2020 09:38:13 +0000
From:   Will Deacon <will@kernel.org>
To:     Greg Ungerer <gerg@linux-m68k.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        linux-m68k@lists.linux-m68k.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/5] Rewrite Motorola MMU page-table layout
Message-ID: <20200131093813.GA3938@willie-the-truck>
References: <20200129103941.304769381@infradead.org>
 <8a81e075-d3bd-80c1-d869-9935fdd73162@linux-m68k.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8a81e075-d3bd-80c1-d869-9935fdd73162@linux-m68k.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Greg,

On Fri, Jan 31, 2020 at 04:31:48PM +1000, Greg Ungerer wrote:
> On 29/1/20 8:39 pm, Peter Zijlstra wrote:
> > In order to faciliate Will's READ_ONCE() patches:
> > 
> >    https://lkml.kernel.org/r/20200123153341.19947-1-will@kernel.org
> > 
> > we need to fix m68k/motorola to not have a giant pmd_t. These patches do so and
> > are tested using ARAnyM/68040.
> > 
> > It would be very good if someone can either test or tell us what emulator to
> > use for 020/030.
> 
> This series breaks compilation for the ColdFire (with MMU) variant of
> the m68k family:

[...]

> Easy to reproduce. Build for the m5475evb_defconfig.

I've hacked up a fix below, but I don't know how to test whether it actually
works (it does fix the build). However, I also notice that building for
m5475evb_defconfig with vanilla v5.5 triggers this scary looking warning
due to a mismatch between the pgd size and the (8k!) page size:


  | In function 'pgd_alloc.isra.111',
  |     inlined from 'mm_alloc_pgd' at kernel/fork.c:634:12,
  |     inlined from 'mm_init.isra.112' at kernel/fork.c:1043:6:
  | ./arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset [4097, 8192] is out of the bounds [0, 4096] of object 'kernel_pg_dir' with type 'pgd_t[1024]' {aka 'struct <anonymous>[1024]'} [-Warray-bounds]
  |  #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
  |                          ^~~~~~~~~~~~~~~~~~~~~~~~~
  | ./arch/m68k/include/asm/mcf_pgalloc.h:93:2: note: in expansion of macro 'memcpy'
  |   memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
  |   ^~~~~~


I think the correct fix is to add this:


diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
index 82ec54c2eaa4..c335e6a381a1 100644
--- a/arch/m68k/include/asm/mcf_pgalloc.h
+++ b/arch/m68k/include/asm/mcf_pgalloc.h
@@ -90,7 +90,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 	new_pgd = (pgd_t *)__get_free_page(GFP_DMA | __GFP_NOWARN);
 	if (!new_pgd)
 		return NULL;
-	memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
+	memcpy(new_pgd, swapper_pg_dir, PTRS_PER_PGD * sizeof(pgd_t));
 	memset(new_pgd, 0, PAGE_OFFSET >> PGDIR_SHIFT);
 	return new_pgd;
 }


but maybe it should be done as a separate patch give that it's not caused
by the rework we've been doing.

Will

--->8

diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
index 82ec54c2eaa4..955d54a6e973 100644
--- a/arch/m68k/include/asm/mcf_pgalloc.h
+++ b/arch/m68k/include/asm/mcf_pgalloc.h
@@ -28,21 +28,22 @@ extern inline pmd_t *pmd_alloc_kernel(pgd_t *pgd, unsigned long address)
 	return (pmd_t *) pgd;
 }
 
-#define pmd_populate(mm, pmd, page) (pmd_val(*pmd) = \
-	(unsigned long)(page_address(page)))
+#define pmd_populate(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
 
-#define pmd_populate_kernel(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
+#define pmd_populate_kernel pmd_populate
 
-#define pmd_pgtable(pmd) pmd_page(pmd)
+#define pmd_pgtable(pmd) pfn_to_virt(pmd_val(pmd) >> PAGE_SHIFT)
 
-static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t page,
+static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pgtable,
 				  unsigned long address)
 {
+	struct page *page = virt_to_page(pgtable);
+
 	pgtable_pte_page_dtor(page);
 	__free_page(page);
 }
 
-static inline struct page *pte_alloc_one(struct mm_struct *mm)
+static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
 {
 	struct page *page = alloc_pages(GFP_DMA, 0);
 	pte_t *pte;
@@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
 		return NULL;
 	}
 
-	pte = kmap(page);
-	if (pte) {
-		clear_page(pte);
-		__flush_page_to_ram(pte);
-		flush_tlb_kernel_page(pte);
-		nocache_page(pte);
-	}
-	kunmap(page);
+	pte = page_address(page);
+	clear_page(pte);
+	__flush_page_to_ram(pte);
+	flush_tlb_kernel_page(pte);
+	nocache_page(pte);
 
-	return page;
+	return pte;
 }
 
-static inline void pte_free(struct mm_struct *mm, struct page *page)
+static inline void pte_free(struct mm_struct *mm, pgtable_t pgtable)
 {
+	struct page *page = virt_to_page(pgtable);
+
 	pgtable_pte_page_dtor(page);
 	__free_page(page);
 }