[patch 0/4] mm/highmem: Fix fallout from generic kmap_local conversions

[patch 0/4] mm/highmem: Fix fallout from generic kmap_local conversions

[Thomas Gleixner]

The kmap_local conversion wreckaged sparc, mips and powerpc as it missed
some of the details in the original implementation.

The following series addresses that.

Thanks,

	tglx
---
 arch/mips/include/asm/highmem.h      |    1 +
 arch/sparc/include/asm/highmem.h     |    9 +++++----
 b/arch/powerpc/include/asm/highmem.h |    2 ++
 mm/highmem.c                         |    7 ++++++-
 4 files changed, 14 insertions(+), 5 deletions(-)

[patch 1/4] sparc/mm/highmem: Flush cache and TLB

[Thomas Gleixner]

The recent conversion to the generic kmap_local infrastructure failed to
assign the proper pre/post map/unmap flush operations for sparc.

Sparc requires cache flush before map/unmap and tlb flush afterwards.

Fixes: 3293efa97807 ("sparc/mm/highmem: Switch to generic kmap atomic")
Reported-by: Andreas Larsson <andreas@gaisler.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/include/asm/highmem.h |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/arch/sparc/include/asm/highmem.h
+++ b/arch/sparc/include/asm/highmem.h
@@ -50,10 +50,11 @@ extern pte_t *pkmap_page_table;
 
 #define flush_cache_kmaps()	flush_cache_all()
 
-/* FIXME: Use __flush_tlb_one(vaddr) instead of flush_cache_all() -- Anton */
-#define arch_kmap_local_post_map(vaddr, pteval)	flush_cache_all()
-#define arch_kmap_local_post_unmap(vaddr)	flush_cache_all()
-
+/* FIXME: Use __flush_*_one(vaddr) instead of flush_*_all() -- Anton */
+#define arch_kmap_local_pre_map(vaddr, pteval)	flush_cache_all()
+#define arch_kmap_local_pre_unmap(vaddr)	flush_cache_all()
+#define arch_kmap_local_post_map(vaddr, pteval)	flush_tlb_all()
+#define arch_kmap_local_post_unmap(vaddr)	flush_tlb_all()
 
 #endif /* __KERNEL__ */
 

Re: [patch 1/4] sparc/mm/highmem: Flush cache and TLB

[Andreas Larsson]

On 2021-01-12 18:01, Thomas Gleixner wrote:
> The recent conversion to the generic kmap_local infrastructure failed to
> assign the proper pre/post map/unmap flush operations for sparc.
> 
> Sparc requires cache flush before map/unmap and tlb flush afterwards.
> 
> Fixes: 3293efa97807 ("sparc/mm/highmem: Switch to generic kmap atomic")
> Reported-by: Andreas Larsson <andreas@gaisler.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> ---
>   arch/sparc/include/asm/highmem.h |    9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> --- a/arch/sparc/include/asm/highmem.h
> +++ b/arch/sparc/include/asm/highmem.h
> @@ -50,10 +50,11 @@ extern pte_t *pkmap_page_table;
>   
>   #define flush_cache_kmaps()	flush_cache_all()
>   
> -/* FIXME: Use __flush_tlb_one(vaddr) instead of flush_cache_all() -- Anton */
> -#define arch_kmap_local_post_map(vaddr, pteval)	flush_cache_all()
> -#define arch_kmap_local_post_unmap(vaddr)	flush_cache_all()
> -
> +/* FIXME: Use __flush_*_one(vaddr) instead of flush_*_all() -- Anton */
> +#define arch_kmap_local_pre_map(vaddr, pteval)	flush_cache_all()
> +#define arch_kmap_local_pre_unmap(vaddr)	flush_cache_all()
> +#define arch_kmap_local_post_map(vaddr, pteval)	flush_tlb_all()
> +#define arch_kmap_local_post_unmap(vaddr)	flush_tlb_all()
>   
>   #endif /* __KERNEL__ */

Yes! I found, just an hour before your patched was posted, that an
equivalent fix helped back where the switch-to-generic patch first
occurred. This patch was successfully tested on master.

Tested-by: Andreas Larsson <andreas@gaisler.com>

-- 
Andreas Larsson
Cobham Gaisler

[patch 2/4] mm/highmem: Prepare for overriding set_pte_at()

[Thomas Gleixner]

The generic kmap_local() map function uses set_pte_at(), but MIPS requires
set_pte() and PowerPC wants __set_pte_at().

Provide arch_kmap_local_set_pte() and default it to set_pte_at().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 mm/highmem.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -473,6 +473,11 @@ static inline void *arch_kmap_local_high
 }
 #endif
 
+#ifndef arch_kmap_local_set_pte
+#define arch_kmap_local_set_pte(mm, vaddr, ptep, ptev)	\
+	set_pte_at(mm, vaddr, ptep, ptev)
+#endif
+
 /* Unmap a local mapping which was obtained by kmap_high_get() */
 static inline bool kmap_high_unmap_local(unsigned long vaddr)
 {
@@ -515,7 +520,7 @@ void *__kmap_local_pfn_prot(unsigned lon
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
 	BUG_ON(!pte_none(*(kmap_pte - idx)));
 	pteval = pfn_pte(pfn, prot);
-	set_pte_at(&init_mm, vaddr, kmap_pte - idx, pteval);
+	arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte - idx, pteval);
 	arch_kmap_local_post_map(vaddr, pteval);
 	current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
 	preempt_enable();

[patch 3/4] mips/mm/highmem: Use set_pte() for kmap_local()

[Thomas Gleixner]

set_pte_at() on MIPS invokes update_cache() which might recurse into
kmap_local(). Use set_pte() like the original MIPS highmem implementation
did.

Fixes: a4c33e83bca1 ("mips/mm/highmem: Switch to generic kmap atomic")
Reported-by: Paul Cercueil <paul@crapouillou.net>
Reported-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/mips/include/asm/highmem.h |    1 +
 1 file changed, 1 insertion(+)

--- a/arch/mips/include/asm/highmem.h
+++ b/arch/mips/include/asm/highmem.h
@@ -51,6 +51,7 @@ extern void kmap_flush_tlb(unsigned long
 
 #define flush_cache_kmaps()	BUG_ON(cpu_has_dc_aliases)
 
+#define arch_kmap_local_set_pte(mm, vaddr, ptep, ptev)	set_pte(ptep, ptev)
 #define arch_kmap_local_post_map(vaddr, pteval)	local_flush_tlb_one(vaddr)
 #define arch_kmap_local_post_unmap(vaddr)	local_flush_tlb_one(vaddr)
 

Re: [patch 3/4] mips/mm/highmem: Use set_pte() for kmap_local()

[Thomas Bogendoerfer]

On Tue, Jan 12, 2021 at 06:01:39PM +0100, Thomas Gleixner wrote:
> set_pte_at() on MIPS invokes update_cache() which might recurse into
> kmap_local(). Use set_pte() like the original MIPS highmem implementation
> did.
> 
> Fixes: a4c33e83bca1 ("mips/mm/highmem: Switch to generic kmap atomic")
> Reported-by: Paul Cercueil <paul@crapouillou.net>
> Reported-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/mips/include/asm/highmem.h |    1 +
>  1 file changed, 1 insertion(+)

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

[patch 4/4] powerpc/mm/highmem: Use __set_pte_at() for kmap_local()

[Thomas Gleixner]

The original PowerPC highmem mapping function used __set_pte_at() to denote
that the mapping is per CPU. This got lost with the conversion to the
generic implementation.

Override the default map function.

Fixes: 47da42b27a56 ("powerpc/mm/highmem: Switch to generic kmap atomic")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/include/asm/highmem.h |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/powerpc/include/asm/highmem.h
+++ b/arch/powerpc/include/asm/highmem.h
@@ -58,6 +58,8 @@ extern pte_t *pkmap_page_table;
 
 #define flush_cache_kmaps()	flush_cache_all()
 
+#define arch_kmap_local_set_pte(mm, vaddr, ptep, ptev)	\
+	__set_pte_at(mm, vaddr, ptep, ptev, 1)
 #define arch_kmap_local_post_map(vaddr, pteval)	\
 	local_flush_tlb_page(NULL, vaddr)
 #define arch_kmap_local_post_unmap(vaddr)	\