2020-11-03 10:35:47

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: [email protected]
---
V3: Remove the kmap types cruft
---
arch/mips/Kconfig | 1
arch/mips/include/asm/fixmap.h | 4 -
arch/mips/include/asm/highmem.h | 6 +-
arch/mips/include/asm/kmap_types.h | 13 ------
arch/mips/mm/highmem.c | 77 -------------------------------------
arch/mips/mm/init.c | 4 -
6 files changed, 6 insertions(+), 99 deletions(-)

--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2719,6 +2719,7 @@ config WAR_MIPS34K_MISSED_ITLB
config HIGHMEM
bool "High Memory Support"
depends on 32BIT && CPU_SUPPORTS_HIGHMEM && SYS_SUPPORTS_HIGHMEM && !CPU_MIPS32_3_5_EVA
+ select KMAP_LOCAL

config CPU_SUPPORTS_HIGHMEM
bool
--- a/arch/mips/include/asm/fixmap.h
+++ b/arch/mips/include/asm/fixmap.h
@@ -17,7 +17,7 @@
#include <spaces.h>
#ifdef CONFIG_HIGHMEM
#include <linux/threads.h>
-#include <asm/kmap_types.h>
+#include <asm/kmap_size.h>
#endif

/*
@@ -52,7 +52,7 @@ enum fixed_addresses {
#ifdef CONFIG_HIGHMEM
/* reserved pte's for temporary kernel mappings */
FIX_KMAP_BEGIN = FIX_CMAP_END + 1,
- FIX_KMAP_END = FIX_KMAP_BEGIN+(KM_TYPE_NR*NR_CPUS)-1,
+ FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
#endif
__end_of_fixed_addresses
};
--- a/arch/mips/include/asm/highmem.h
+++ b/arch/mips/include/asm/highmem.h
@@ -24,7 +24,7 @@
#include <linux/interrupt.h>
#include <linux/uaccess.h>
#include <asm/cpu-features.h>
-#include <asm/kmap_types.h>
+#include <asm/kmap_size.h>

/* declarations for highmem.c */
extern unsigned long highstart_pfn, highend_pfn;
@@ -48,11 +48,11 @@ extern pte_t *pkmap_page_table;

#define ARCH_HAS_KMAP_FLUSH_TLB
extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic_pfn(unsigned long pfn);

#define flush_cache_kmaps() BUG_ON(cpu_has_dc_aliases)

-extern void kmap_init(void);
+#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)

#endif /* __KERNEL__ */

--- a/arch/mips/include/asm/kmap_types.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-#define __WITH_KM_FENCE
-#endif
-
-#include <asm-generic/kmap_types.h>
-
-#undef __WITH_KM_FENCE
-
-#endif
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -8,8 +8,6 @@
#include <asm/fixmap.h>
#include <asm/tlbflush.h>

-static pte_t *kmap_pte;
-
unsigned long highstart_pfn, highend_pfn;

void kmap_flush_tlb(unsigned long addr)
@@ -17,78 +15,3 @@ void kmap_flush_tlb(unsigned long addr)
flush_tlb_one(addr);
}
EXPORT_SYMBOL(kmap_flush_tlb);
-
-void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
-{
- unsigned long vaddr;
- int idx, type;
-
- type = kmap_atomic_idx_push();
- idx = type + KM_TYPE_NR*smp_processor_id();
- vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-#ifdef CONFIG_DEBUG_HIGHMEM
- BUG_ON(!pte_none(*(kmap_pte - idx)));
-#endif
- set_pte(kmap_pte-idx, mk_pte(page, prot));
- local_flush_tlb_one((unsigned long)vaddr);
-
- return (void*) vaddr;
-}
-EXPORT_SYMBOL(kmap_atomic_high_prot);
-
-void kunmap_atomic_high(void *kvaddr)
-{
- unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
- int type __maybe_unused;
-
- if (vaddr < FIXADDR_START)
- return;
-
- type = kmap_atomic_idx();
-#ifdef CONFIG_DEBUG_HIGHMEM
- {
- int idx = type + KM_TYPE_NR * smp_processor_id();
-
- BUG_ON(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
-
- /*
- * force other mappings to Oops if they'll try to access
- * this pte without first remap it
- */
- pte_clear(&init_mm, vaddr, kmap_pte-idx);
- local_flush_tlb_one(vaddr);
- }
-#endif
- kmap_atomic_idx_pop();
-}
-EXPORT_SYMBOL(kunmap_atomic_high);
-
-/*
- * This is the same as kmap_atomic() but can map memory that doesn't
- * have a struct page associated with it.
- */
-void *kmap_atomic_pfn(unsigned long pfn)
-{
- unsigned long vaddr;
- int idx, type;
-
- preempt_disable();
- pagefault_disable();
-
- type = kmap_atomic_idx_push();
- idx = type + KM_TYPE_NR*smp_processor_id();
- vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
- set_pte(kmap_pte-idx, pfn_pte(pfn, PAGE_KERNEL));
- flush_tlb_one(vaddr);
-
- return (void*) vaddr;
-}
-
-void __init kmap_init(void)
-{
- unsigned long kmap_vstart;
-
- /* cache the first kmap pte */
- kmap_vstart = __fix_to_virt(FIX_KMAP_BEGIN);
- kmap_pte = virt_to_kpte(kmap_vstart);
-}
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -36,7 +36,6 @@
#include <asm/cachectl.h>
#include <asm/cpu.h>
#include <asm/dma.h>
-#include <asm/kmap_types.h>
#include <asm/maar.h>
#include <asm/mmu_context.h>
#include <asm/sections.h>
@@ -402,9 +401,6 @@ void __init paging_init(void)

pagetable_init();

-#ifdef CONFIG_HIGHMEM
- kmap_init();
-#endif
#ifdef CONFIG_ZONE_DMA
max_zone_pfns[ZONE_DMA] = MAX_DMA_PFN;
#endif


Subject: [tip: core/mm] mips/mm/highmem: Switch to generic kmap atomic

The following commit has been merged into the core/mm branch of tip:

Commit-ID: a4c33e83bca133ff979e13c784c7605e1ac143df
Gitweb: https://git.kernel.org/tip/a4c33e83bca133ff979e13c784c7605e1ac143df
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 03 Nov 2020 10:27:25 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Fri, 06 Nov 2020 23:14:56 +01:00

mips/mm/highmem: Switch to generic kmap atomic

No reason having the same code in every architecture

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
arch/mips/Kconfig | 1 +-
arch/mips/include/asm/fixmap.h | 4 +-
arch/mips/include/asm/highmem.h | 6 +-
arch/mips/include/asm/kmap_types.h | 13 +-----
arch/mips/mm/highmem.c | 77 +-----------------------------
arch/mips/mm/init.c | 4 +--
6 files changed, 6 insertions(+), 99 deletions(-)
delete mode 100644 arch/mips/include/asm/kmap_types.h

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 2000bb2..6b762be 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2719,6 +2719,7 @@ config WAR_MIPS34K_MISSED_ITLB
config HIGHMEM
bool "High Memory Support"
depends on 32BIT && CPU_SUPPORTS_HIGHMEM && SYS_SUPPORTS_HIGHMEM && !CPU_MIPS32_3_5_EVA
+ select KMAP_LOCAL

config CPU_SUPPORTS_HIGHMEM
bool
diff --git a/arch/mips/include/asm/fixmap.h b/arch/mips/include/asm/fixmap.h
index 743535b..beea147 100644
--- a/arch/mips/include/asm/fixmap.h
+++ b/arch/mips/include/asm/fixmap.h
@@ -17,7 +17,7 @@
#include <spaces.h>
#ifdef CONFIG_HIGHMEM
#include <linux/threads.h>
-#include <asm/kmap_types.h>
+#include <asm/kmap_size.h>
#endif

/*
@@ -52,7 +52,7 @@ enum fixed_addresses {
#ifdef CONFIG_HIGHMEM
/* reserved pte's for temporary kernel mappings */
FIX_KMAP_BEGIN = FIX_CMAP_END + 1,
- FIX_KMAP_END = FIX_KMAP_BEGIN+(KM_TYPE_NR*NR_CPUS)-1,
+ FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
#endif
__end_of_fixed_addresses
};
diff --git a/arch/mips/include/asm/highmem.h b/arch/mips/include/asm/highmem.h
index f1f788b..19edf8e 100644
--- a/arch/mips/include/asm/highmem.h
+++ b/arch/mips/include/asm/highmem.h
@@ -24,7 +24,7 @@
#include <linux/interrupt.h>
#include <linux/uaccess.h>
#include <asm/cpu-features.h>
-#include <asm/kmap_types.h>
+#include <asm/kmap_size.h>

/* declarations for highmem.c */
extern unsigned long highstart_pfn, highend_pfn;
@@ -48,11 +48,11 @@ extern pte_t *pkmap_page_table;

#define ARCH_HAS_KMAP_FLUSH_TLB
extern void kmap_flush_tlb(unsigned long addr);
-extern void *kmap_atomic_pfn(unsigned long pfn);

#define flush_cache_kmaps() BUG_ON(cpu_has_dc_aliases)

-extern void kmap_init(void);
+#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)

#endif /* __KERNEL__ */

diff --git a/arch/mips/include/asm/kmap_types.h b/arch/mips/include/asm/kmap_types.h
deleted file mode 100644
index 16665dc..0000000
--- a/arch/mips/include/asm/kmap_types.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_KMAP_TYPES_H
-#define _ASM_KMAP_TYPES_H
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-#define __WITH_KM_FENCE
-#endif
-
-#include <asm-generic/kmap_types.h>
-
-#undef __WITH_KM_FENCE
-
-#endif
diff --git a/arch/mips/mm/highmem.c b/arch/mips/mm/highmem.c
index 5fec7f4..57e2f08 100644
--- a/arch/mips/mm/highmem.c
+++ b/arch/mips/mm/highmem.c
@@ -8,8 +8,6 @@
#include <asm/fixmap.h>
#include <asm/tlbflush.h>

-static pte_t *kmap_pte;
-
unsigned long highstart_pfn, highend_pfn;

void kmap_flush_tlb(unsigned long addr)
@@ -17,78 +15,3 @@ void kmap_flush_tlb(unsigned long addr)
flush_tlb_one(addr);
}
EXPORT_SYMBOL(kmap_flush_tlb);
-
-void *kmap_atomic_high_prot(struct page *page, pgprot_t prot)
-{
- unsigned long vaddr;
- int idx, type;
-
- type = kmap_atomic_idx_push();
- idx = type + KM_TYPE_NR*smp_processor_id();
- vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-#ifdef CONFIG_DEBUG_HIGHMEM
- BUG_ON(!pte_none(*(kmap_pte - idx)));
-#endif
- set_pte(kmap_pte-idx, mk_pte(page, prot));
- local_flush_tlb_one((unsigned long)vaddr);
-
- return (void*) vaddr;
-}
-EXPORT_SYMBOL(kmap_atomic_high_prot);
-
-void kunmap_atomic_high(void *kvaddr)
-{
- unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
- int type __maybe_unused;
-
- if (vaddr < FIXADDR_START)
- return;
-
- type = kmap_atomic_idx();
-#ifdef CONFIG_DEBUG_HIGHMEM
- {
- int idx = type + KM_TYPE_NR * smp_processor_id();
-
- BUG_ON(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
-
- /*
- * force other mappings to Oops if they'll try to access
- * this pte without first remap it
- */
- pte_clear(&init_mm, vaddr, kmap_pte-idx);
- local_flush_tlb_one(vaddr);
- }
-#endif
- kmap_atomic_idx_pop();
-}
-EXPORT_SYMBOL(kunmap_atomic_high);
-
-/*
- * This is the same as kmap_atomic() but can map memory that doesn't
- * have a struct page associated with it.
- */
-void *kmap_atomic_pfn(unsigned long pfn)
-{
- unsigned long vaddr;
- int idx, type;
-
- preempt_disable();
- pagefault_disable();
-
- type = kmap_atomic_idx_push();
- idx = type + KM_TYPE_NR*smp_processor_id();
- vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
- set_pte(kmap_pte-idx, pfn_pte(pfn, PAGE_KERNEL));
- flush_tlb_one(vaddr);
-
- return (void*) vaddr;
-}
-
-void __init kmap_init(void)
-{
- unsigned long kmap_vstart;
-
- /* cache the first kmap pte */
- kmap_vstart = __fix_to_virt(FIX_KMAP_BEGIN);
- kmap_pte = virt_to_kpte(kmap_vstart);
-}
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 07e84a7..bc80893 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -36,7 +36,6 @@
#include <asm/cachectl.h>
#include <asm/cpu.h>
#include <asm/dma.h>
-#include <asm/kmap_types.h>
#include <asm/maar.h>
#include <asm/mmu_context.h>
#include <asm/sections.h>
@@ -402,9 +401,6 @@ void __init paging_init(void)

pagetable_init();

-#ifdef CONFIG_HIGHMEM
- kmap_init();
-#endif
#ifdef CONFIG_ZONE_DMA
max_zone_pfns[ZONE_DMA] = MAX_DMA_PFN;
#endif

2021-01-08 21:02:15

by Paul Cercueil

[permalink] [raw]
Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

Hi Thomas,

5.11 does not boot anymore on Ingenic SoCs, I bisected it to this
commit.

Any idea what could be happening?

Cheers,
-Paul


2021-01-09 00:02:17

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

On Fri, Jan 08, 2021 at 08:20:43PM +0000, Paul Cercueil wrote:
> Hi Thomas,
>
> 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this commit.
>
> Any idea what could be happening?

not yet, kernel crash log of a Malta QEMU is below.

Thomas.

Kernel bug detected[#1]:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.11.0-rc1-00017-gccb21774863a #2
$ 0 : 00000000 00000001 00000000 00000010
$ 4 : 00000001 000005cf 9e00059f 00000000
$ 8 : 00118173 809e6db8 9e00059f 00000000
$12 : 82023c00 00000001 810da04c 0212422f
$16 : 810da000 00027800 000005cf 80b4bf9c
$20 : 809e968c 82602400 810da000 0000000b
$24 : 021558f9 00000000
$28 : 820e0000 820e3928 80b10000 802710d0
Hi : 0000346c
Lo : 000002dd
epc : 80271114 __kmap_local_pfn_prot+0x78/0x1c0
ra : 802710d0 __kmap_local_pfn_prot+0x34/0x1c0
Status: 1000a403 KERNEL EXL IE
Cause : 00800034 (ExcCode 0d)
PrId : 0001a800 (MIPS P5600)
Modules linked in:
Process swapper/0 (pid: 1, threadinfo=(ptrval), task=(ptrval), tls=00000000)
Stack : 7fffffff 820c2408 820e3990 ffffff04 ffff0a00 80518224 000081a4 810da000
00000001 000005cf fff64000 8011c77c 820e3b26 ffffff04 ffff0a00 80518440
80b30000 80b4bf64 9e0005cf 000005cf fff64000 80271188 00000000 820e3a60
80b10000 80194478 0000005e 80954406 809e0000 810da000 00000001 000005cf
fff68000 8011c77c 8088fd44 809f6074 000000f4 00000000 00000000 80b4bf68
...
Call Trace:
[<80271114>] __kmap_local_pfn_prot+0x78/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<8011c77c>] __update_cache+0x16c/0x174
[<80271188>] __kmap_local_pfn_prot+0xec/0x1c0
[<802c49a0>] copy_string_kernel+0x168/0x264
[<802c5d18>] kernel_execve+0xd0/0x164
[<801006cc>] try_to_run_init_process+0x18/0x5c
[<80859e0c>] kernel_init+0xd0/0x120
[<801037f8>] ret_from_kernel_thread+0x14/0x1c

Code: 8c630564 28640010 38840001 <00040336> 8f82000c 2463ffff 00021100 00431021 2403ffbf

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

2021-01-09 00:36:34

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

On Sat, Jan 09, 2021 at 12:58:05AM +0100, Thomas Bogendoerfer wrote:
> On Fri, Jan 08, 2021 at 08:20:43PM +0000, Paul Cercueil wrote:
> > Hi Thomas,
> >
> > 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this commit.
> >
> > Any idea what could be happening?
>
> not yet, kernel crash log of a Malta QEMU is below.

update:

This dirty hack lets the Malta QEMU boot again:

diff --git a/mm/highmem.c b/mm/highmem.c
index c3a9ea7875ef..190cdda1149d 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -515,7 +515,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
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);
+ set_pte(kmap_pte - idx, pteval);
arch_kmap_local_post_map(vaddr, pteval);
current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
preempt_enable();

set_pte_at() tries to update cache and could do an kmap_atomic() there.
Not sure, if this is allowed at this point.

Thomas.

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

2021-01-10 11:37:33

by Paul Cercueil

[permalink] [raw]
Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

Hi Thomas,

Le sam. 9 janv. 2021 ? 1:33, Thomas Bogendoerfer
<[email protected]> a ?crit :
> On Sat, Jan 09, 2021 at 12:58:05AM +0100, Thomas Bogendoerfer wrote:
>> On Fri, Jan 08, 2021 at 08:20:43PM +0000, Paul Cercueil wrote:
>> > Hi Thomas,
>> >
>> > 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this
>> commit.
>> >
>> > Any idea what could be happening?
>>
>> not yet, kernel crash log of a Malta QEMU is below.
>
> update:
>
> This dirty hack lets the Malta QEMU boot again:
>
> diff --git a/mm/highmem.c b/mm/highmem.c
> index c3a9ea7875ef..190cdda1149d 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -515,7 +515,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn,
> pgprot_t prot)
> 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);
> + set_pte(kmap_pte - idx, pteval);
> arch_kmap_local_post_map(vaddr, pteval);
> current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
> preempt_enable();
>
> set_pte_at() tries to update cache and could do an kmap_atomic()
> there.
> Not sure, if this is allowed at this point.

Yes, I can confirm that your workaround works here too.

Cheers,
-Paul


2021-01-10 12:00:33

by H. Nikolaus Schaller

[permalink] [raw]
Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic


> Am 10.01.2021 um 12:35 schrieb Paul Cercueil <[email protected]>:
>
> Hi Thomas,
>
> Le sam. 9 janv. 2021 ? 1:33, Thomas Bogendoerfer <[email protected]> a ?crit :
>> On Sat, Jan 09, 2021 at 12:58:05AM +0100, Thomas Bogendoerfer wrote:
>>> On Fri, Jan 08, 2021 at 08:20:43PM +0000, Paul Cercueil wrote:
>>> > Hi Thomas,
>>> >
>>> > 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this commit.

Just for completeness, I have no such problems booting CI20/jz4780 or Skytone400/jz4730 (unpublished work) with 5.11-rc2.
But may depend on board capabilites (ram size, memory layout or something else).

>>> >
>>> > Any idea what could be happening?
>>> not yet, kernel crash log of a Malta QEMU is below.
>> update:
>> This dirty hack lets the Malta QEMU boot again:
>> diff --git a/mm/highmem.c b/mm/highmem.c
>> index c3a9ea7875ef..190cdda1149d 100644
>> --- a/mm/highmem.c
>> +++ b/mm/highmem.c
>> @@ -515,7 +515,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
>> 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);
>> + set_pte(kmap_pte - idx, pteval);
>> arch_kmap_local_post_map(vaddr, pteval);
>> current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
>> preempt_enable();
>> set_pte_at() tries to update cache and could do an kmap_atomic() there.
>> Not sure, if this is allowed at this point.
>
> Yes, I can confirm that your workaround works here too.
>
> Cheers,
> -Paul
>
>

Subject: Re: [patch V3 13/37] mips/mm/highmem: Switch to generic kmap atomic

On 2021-01-09 01:33:52 [+0100], Thomas Bogendoerfer wrote:
> On Sat, Jan 09, 2021 at 12:58:05AM +0100, Thomas Bogendoerfer wrote:
> > On Fri, Jan 08, 2021 at 08:20:43PM +0000, Paul Cercueil wrote:
> > > Hi Thomas,
> > >
> > > 5.11 does not boot anymore on Ingenic SoCs, I bisected it to this commit.
> > >
> > > Any idea what could be happening?
> >
> > not yet, kernel crash log of a Malta QEMU is below.
>
> update:
>
> This dirty hack lets the Malta QEMU boot again:
>
> diff --git a/mm/highmem.c b/mm/highmem.c
> index c3a9ea7875ef..190cdda1149d 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -515,7 +515,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
> 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);
> + set_pte(kmap_pte - idx, pteval);
> arch_kmap_local_post_map(vaddr, pteval);
> current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
> preempt_enable();
>
> set_pte_at() tries to update cache and could do an kmap_atomic() there.
So the old implementation used set_pte() while the new one uses
set_pte_at().

> Not sure, if this is allowed at this point.
The problem is the recursion
kmap_atomic() -> __update_cache() -> kmap_atomic()

and kmap_local_idx_push() runs out if index space before stack space.

I'm not sure if the __update_cache() worked for highmem. It has been
added for that in commit
f4281bba81810 ("MIPS: Handle highmem pages in __update_cache")

but it assumes that the address returned by kmap_atomic() is the same or
related enough for flush_data_cache_page() to work.

> Thomas.
>

Sebastian