[PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Ard Biesheuvel]

The kmap_local conversion broke the ARM architecture, because the new
code assumes that all PTEs used for creating kmaps form a linear array
in memory, and uses array indexing to look up the kmap PTE belonging to
a certain kmap index.

On ARM, this cannot work, not only because the PTE pages may be
non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
entries and extended entries (carrying software-only bits) in a way that
is not compatible with array indexing.

Fortunately, this only seems to affect configurations with more than 8
CPUs, due to the way the per-CPU kmap slots are organized in memory.

Work around this by permitting an architecture to set a Kconfig symbol
that signifies that the kmap PTEs do not form a lineary array in memory,
and so the only way to locate the appropriate one is to walk the page
tables.

Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/Kconfig |  1 +
 mm/Kconfig       |  3 +++
 mm/highmem.c     | 32 +++++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 727c00c7d616..9aa0528f85de 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1467,6 +1467,7 @@ config HIGHMEM
 	bool "High Memory Support"
 	depends on MMU
 	select KMAP_LOCAL
+	select KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
 	help
 	  The address space of ARM processors is only 4 Gigabytes large
 	  and it has to accommodate user address space, kernel address
diff --git a/mm/Kconfig b/mm/Kconfig
index d16ba9249bc5..c048dea7e342 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -887,6 +887,9 @@ config MAPPING_DIRTY_HELPERS
 config KMAP_LOCAL
 	bool
 
+config KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
+	bool
+
 # struct io_mapping based helper.  Selected by drivers that need them
 config IO_MAPPING
 	bool
diff --git a/mm/highmem.c b/mm/highmem.c
index 4212ad0e4a19..1f0c8a52fd80 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -504,16 +504,22 @@ static inline int kmap_local_calc_idx(int idx)
 
 static pte_t *__kmap_pte;
 
-static pte_t *kmap_get_pte(void)
+static pte_t *kmap_get_pte(unsigned long vaddr, int idx)
 {
+	if (IS_ENABLED(CONFIG_KMAP_LOCAL_NON_LINEAR_PTE_ARRAY))
+		/*
+		 * Set by the arch if __kmap_pte[-idx] does not produce
+		 * the correct entry.
+		 */
+		return virt_to_kpte(vaddr);
 	if (!__kmap_pte)
 		__kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
-	return __kmap_pte;
+	return &__kmap_pte[-idx];
 }
 
 void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
 {
-	pte_t pteval, *kmap_pte = kmap_get_pte();
+	pte_t pteval, *kmap_pte;
 	unsigned long vaddr;
 	int idx;
 
@@ -525,9 +531,10 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
 	preempt_disable();
 	idx = arch_kmap_local_map_idx(kmap_local_idx_push(), pfn);
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-	BUG_ON(!pte_none(*(kmap_pte - idx)));
+	kmap_pte = kmap_get_pte(vaddr, idx);
+	BUG_ON(!pte_none(*kmap_pte));
 	pteval = pfn_pte(pfn, prot);
-	arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte - idx, pteval);
+	arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte, pteval);
 	arch_kmap_local_post_map(vaddr, pteval);
 	current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
 	preempt_enable();
@@ -560,7 +567,7 @@ EXPORT_SYMBOL(__kmap_local_page_prot);
 void kunmap_local_indexed(void *vaddr)
 {
 	unsigned long addr = (unsigned long) vaddr & PAGE_MASK;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int idx;
 
 	if (addr < __fix_to_virt(FIX_KMAP_END) ||
@@ -585,8 +592,9 @@ void kunmap_local_indexed(void *vaddr)
 	idx = arch_kmap_local_unmap_idx(kmap_local_idx(), addr);
 	WARN_ON_ONCE(addr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
 
+	kmap_pte = kmap_get_pte(addr, idx);
 	arch_kmap_local_pre_unmap(addr);
-	pte_clear(&init_mm, addr, kmap_pte - idx);
+	pte_clear(&init_mm, addr, kmap_pte);
 	arch_kmap_local_post_unmap(addr);
 	current->kmap_ctrl.pteval[kmap_local_idx()] = __pte(0);
 	kmap_local_idx_pop();
@@ -608,7 +616,7 @@ EXPORT_SYMBOL(kunmap_local_indexed);
 void __kmap_local_sched_out(void)
 {
 	struct task_struct *tsk = current;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int i;
 
 	/* Clear kmaps */
@@ -635,8 +643,9 @@ void __kmap_local_sched_out(void)
 		idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
 
 		addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
+		kmap_pte = kmap_get_pte(addr, idx);
 		arch_kmap_local_pre_unmap(addr);
-		pte_clear(&init_mm, addr, kmap_pte - idx);
+		pte_clear(&init_mm, addr, kmap_pte);
 		arch_kmap_local_post_unmap(addr);
 	}
 }
@@ -644,7 +653,7 @@ void __kmap_local_sched_out(void)
 void __kmap_local_sched_in(void)
 {
 	struct task_struct *tsk = current;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int i;
 
 	/* Restore kmaps */
@@ -664,7 +673,8 @@ void __kmap_local_sched_in(void)
 		/* See comment in __kmap_local_sched_out() */
 		idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
 		addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-		set_pte_at(&init_mm, addr, kmap_pte - idx, pteval);
+		kmap_pte = kmap_get_pte(addr, idx);
+		set_pte_at(&init_mm, addr, kmap_pte, pteval);
 		arch_kmap_local_post_map(addr, pteval);
 	}
 }
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Ard Biesheuvel]

On Tue, 26 Oct 2021 at 15:13, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The kmap_local conversion broke the ARM architecture, because the new
> code assumes that all PTEs used for creating kmaps form a linear array
> in memory, and uses array indexing to look up the kmap PTE belonging to
> a certain kmap index.
>
> On ARM, this cannot work, not only because the PTE pages may be
> non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> entries and extended entries (carrying software-only bits) in a way that
> is not compatible with array indexing.
>
> Fortunately, this only seems to affect configurations with more than 8
> CPUs, due to the way the per-CPU kmap slots are organized in memory.
>
> Work around this by permitting an architecture to set a Kconfig symbol
> that signifies that the kmap PTEs do not form a lineary array in memory,
> and so the only way to locate the appropriate one is to walk the page
> tables.
>
> Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---

Ping? Can we get this fixed please?


>  arch/arm/Kconfig |  1 +
>  mm/Kconfig       |  3 +++
>  mm/highmem.c     | 32 +++++++++++++++++++++-----------
>  3 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 727c00c7d616..9aa0528f85de 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1467,6 +1467,7 @@ config HIGHMEM
>         bool "High Memory Support"
>         depends on MMU
>         select KMAP_LOCAL
> +       select KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
>         help
>           The address space of ARM processors is only 4 Gigabytes large
>           and it has to accommodate user address space, kernel address
> diff --git a/mm/Kconfig b/mm/Kconfig
> index d16ba9249bc5..c048dea7e342 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -887,6 +887,9 @@ config MAPPING_DIRTY_HELPERS
>  config KMAP_LOCAL
>         bool
>
> +config KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
> +       bool
> +
>  # struct io_mapping based helper.  Selected by drivers that need them
>  config IO_MAPPING
>         bool
> diff --git a/mm/highmem.c b/mm/highmem.c
> index 4212ad0e4a19..1f0c8a52fd80 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -504,16 +504,22 @@ static inline int kmap_local_calc_idx(int idx)
>
>  static pte_t *__kmap_pte;
>
> -static pte_t *kmap_get_pte(void)
> +static pte_t *kmap_get_pte(unsigned long vaddr, int idx)
>  {
> +       if (IS_ENABLED(CONFIG_KMAP_LOCAL_NON_LINEAR_PTE_ARRAY))
> +               /*
> +                * Set by the arch if __kmap_pte[-idx] does not produce
> +                * the correct entry.
> +                */
> +               return virt_to_kpte(vaddr);
>         if (!__kmap_pte)
>                 __kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
> -       return __kmap_pte;
> +       return &__kmap_pte[-idx];
>  }
>
>  void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
>  {
> -       pte_t pteval, *kmap_pte = kmap_get_pte();
> +       pte_t pteval, *kmap_pte;
>         unsigned long vaddr;
>         int idx;
>
> @@ -525,9 +531,10 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
>         preempt_disable();
>         idx = arch_kmap_local_map_idx(kmap_local_idx_push(), pfn);
>         vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
> -       BUG_ON(!pte_none(*(kmap_pte - idx)));
> +       kmap_pte = kmap_get_pte(vaddr, idx);
> +       BUG_ON(!pte_none(*kmap_pte));
>         pteval = pfn_pte(pfn, prot);
> -       arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte - idx, pteval);
> +       arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte, pteval);
>         arch_kmap_local_post_map(vaddr, pteval);
>         current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
>         preempt_enable();
> @@ -560,7 +567,7 @@ EXPORT_SYMBOL(__kmap_local_page_prot);
>  void kunmap_local_indexed(void *vaddr)
>  {
>         unsigned long addr = (unsigned long) vaddr & PAGE_MASK;
> -       pte_t *kmap_pte = kmap_get_pte();
> +       pte_t *kmap_pte;
>         int idx;
>
>         if (addr < __fix_to_virt(FIX_KMAP_END) ||
> @@ -585,8 +592,9 @@ void kunmap_local_indexed(void *vaddr)
>         idx = arch_kmap_local_unmap_idx(kmap_local_idx(), addr);
>         WARN_ON_ONCE(addr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
>
> +       kmap_pte = kmap_get_pte(addr, idx);
>         arch_kmap_local_pre_unmap(addr);
> -       pte_clear(&init_mm, addr, kmap_pte - idx);
> +       pte_clear(&init_mm, addr, kmap_pte);
>         arch_kmap_local_post_unmap(addr);
>         current->kmap_ctrl.pteval[kmap_local_idx()] = __pte(0);
>         kmap_local_idx_pop();
> @@ -608,7 +616,7 @@ EXPORT_SYMBOL(kunmap_local_indexed);
>  void __kmap_local_sched_out(void)
>  {
>         struct task_struct *tsk = current;
> -       pte_t *kmap_pte = kmap_get_pte();
> +       pte_t *kmap_pte;
>         int i;
>
>         /* Clear kmaps */
> @@ -635,8 +643,9 @@ void __kmap_local_sched_out(void)
>                 idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
>
>                 addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
> +               kmap_pte = kmap_get_pte(addr, idx);
>                 arch_kmap_local_pre_unmap(addr);
> -               pte_clear(&init_mm, addr, kmap_pte - idx);
> +               pte_clear(&init_mm, addr, kmap_pte);
>                 arch_kmap_local_post_unmap(addr);
>         }
>  }
> @@ -644,7 +653,7 @@ void __kmap_local_sched_out(void)
>  void __kmap_local_sched_in(void)
>  {
>         struct task_struct *tsk = current;
> -       pte_t *kmap_pte = kmap_get_pte();
> +       pte_t *kmap_pte;
>         int i;
>
>         /* Restore kmaps */
> @@ -664,7 +673,8 @@ void __kmap_local_sched_in(void)
>                 /* See comment in __kmap_local_sched_out() */
>                 idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
>                 addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
> -               set_pte_at(&init_mm, addr, kmap_pte - idx, pteval);
> +               kmap_pte = kmap_get_pte(addr, idx);
> +               set_pte_at(&init_mm, addr, kmap_pte, pteval);
>                 arch_kmap_local_post_map(addr, pteval);
>         }
>  }
> --
> 2.30.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Russell King (Oracle)]

On Fri, Nov 05, 2021 at 03:21:00PM +0100, Ard Biesheuvel wrote:
> On Tue, 26 Oct 2021 at 15:13, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > The kmap_local conversion broke the ARM architecture, because the new
> > code assumes that all PTEs used for creating kmaps form a linear array
> > in memory, and uses array indexing to look up the kmap PTE belonging to
> > a certain kmap index.
> >
> > On ARM, this cannot work, not only because the PTE pages may be
> > non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> > entries and extended entries (carrying software-only bits) in a way that
> > is not compatible with array indexing.
> >
> > Fortunately, this only seems to affect configurations with more than 8
> > CPUs, due to the way the per-CPU kmap slots are organized in memory.
> >
> > Work around this by permitting an architecture to set a Kconfig symbol
> > that signifies that the kmap PTEs do not form a lineary array in memory,
> > and so the only way to locate the appropriate one is to walk the page
> > tables.
> >
> > Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> 
> Ping? Can we get this fixed please?

Who are you expecting to apply it? It seems to be touching only core
code, so I don't think it's up to me - not without some kind of review
from mm guys.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Ard Biesheuvel]

On Fri, 5 Nov 2021 at 15:50, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Fri, Nov 05, 2021 at 03:21:00PM +0100, Ard Biesheuvel wrote:
> > On Tue, 26 Oct 2021 at 15:13, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > The kmap_local conversion broke the ARM architecture, because the new
> > > code assumes that all PTEs used for creating kmaps form a linear array
> > > in memory, and uses array indexing to look up the kmap PTE belonging to
> > > a certain kmap index.
> > >
> > > On ARM, this cannot work, not only because the PTE pages may be
> > > non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> > > entries and extended entries (carrying software-only bits) in a way that
> > > is not compatible with array indexing.
> > >
> > > Fortunately, this only seems to affect configurations with more than 8
> > > CPUs, due to the way the per-CPU kmap slots are organized in memory.
> > >
> > > Work around this by permitting an architecture to set a Kconfig symbol
> > > that signifies that the kmap PTEs do not form a lineary array in memory,
> > > and so the only way to locate the appropriate one is to walk the page
> > > tables.
> > >
> > > Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> >
> > Ping? Can we get this fixed please?
>
> Who are you expecting to apply it? It seems to be touching only core
> code, so I don't think it's up to me - not without some kind of review
> from mm guys.
>

I was hoping Thomas would respond, given that he introduced the
problem in the first place, and can either carry the fix himself, or
tell us whether he wants it fixed in a different way.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Linus Walleij]

On Tue, Oct 26, 2021 at 3:13 PM Ard Biesheuvel <ardb@kernel.org> wrote:

> The kmap_local conversion broke the ARM architecture, because the new
> code assumes that all PTEs used for creating kmaps form a linear array
> in memory, and uses array indexing to look up the kmap PTE belonging to
> a certain kmap index.
>
> On ARM, this cannot work, not only because the PTE pages may be
> non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> entries and extended entries (carrying software-only bits) in a way that
> is not compatible with array indexing.
>
> Fortunately, this only seems to affect configurations with more than 8
> CPUs, due to the way the per-CPU kmap slots are organized in memory.
>
> Work around this by permitting an architecture to set a Kconfig symbol
> that signifies that the kmap PTEs do not form a lineary array in memory,
> and so the only way to locate the appropriate one is to walk the page
> tables.
>
> Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Ouch, nice workarounding.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I wonder if there is some Fixes: that applies to this?
Maybe TGLX can add one when he applies it.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Russell King (Oracle)]

[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]

On Fri, Nov 05, 2021 at 04:01:20PM +0100, Ard Biesheuvel wrote:
> On Fri, 5 Nov 2021 at 15:50, Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Fri, Nov 05, 2021 at 03:21:00PM +0100, Ard Biesheuvel wrote:
> > > On Tue, 26 Oct 2021 at 15:13, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > The kmap_local conversion broke the ARM architecture, because the new
> > > > code assumes that all PTEs used for creating kmaps form a linear array
> > > > in memory, and uses array indexing to look up the kmap PTE belonging to
> > > > a certain kmap index.
> > > >
> > > > On ARM, this cannot work, not only because the PTE pages may be
> > > > non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> > > > entries and extended entries (carrying software-only bits) in a way that
> > > > is not compatible with array indexing.
> > > >
> > > > Fortunately, this only seems to affect configurations with more than 8
> > > > CPUs, due to the way the per-CPU kmap slots are organized in memory.
> > > >
> > > > Work around this by permitting an architecture to set a Kconfig symbol
> > > > that signifies that the kmap PTEs do not form a lineary array in memory,
> > > > and so the only way to locate the appropriate one is to walk the page
> > > > tables.
> > > >
> > > > Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > ---
> > >
> > > Ping? Can we get this fixed please?
> >
> > Who are you expecting to apply it? It seems to be touching only core
> > code, so I don't think it's up to me - not without some kind of review
> > from mm guys.
> >
> 
> I was hoping Thomas would respond, given that he introduced the
> problem in the first place, and can either carry the fix himself, or
> tell us whether he wants it fixed in a different way.

We're two weeks on, and Thomas hasn't responded - not on IRC nor via
email. Can we get akpm to pick the patch up? (Original patch message
attached.)

I think I have one fix ("fix early early_iounmap()") that needs to go
to Linus, so alternatively I could send it with that... if someone
wants to drop it in the patch system.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

[-- Attachment #2: Type: message/rfc822, Size: 7550 bytes --]

From: Ard Biesheuvel <ardb@kernel.org>
To: linux@armlinux.org.uk
Cc: linus.walleij@linaro.org, arnd@arndb.de, linux-arm-kernel@lists.infradead.org, tglx@linutronix.de, quanyang.wang@windriver.com, Ard Biesheuvel <ardb@kernel.org>
Subject: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory
Date: Tue, 26 Oct 2021 15:12:49 +0200
Message-ID: <20211026131249.3731275-1-ardb@kernel.org>

The kmap_local conversion broke the ARM architecture, because the new
code assumes that all PTEs used for creating kmaps form a linear array
in memory, and uses array indexing to look up the kmap PTE belonging to
a certain kmap index.

On ARM, this cannot work, not only because the PTE pages may be
non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
entries and extended entries (carrying software-only bits) in a way that
is not compatible with array indexing.

Fortunately, this only seems to affect configurations with more than 8
CPUs, due to the way the per-CPU kmap slots are organized in memory.

Work around this by permitting an architecture to set a Kconfig symbol
that signifies that the kmap PTEs do not form a lineary array in memory,
and so the only way to locate the appropriate one is to walk the page
tables.

Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/Kconfig |  1 +
 mm/Kconfig       |  3 +++
 mm/highmem.c     | 32 +++++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 727c00c7d616..9aa0528f85de 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1467,6 +1467,7 @@ config HIGHMEM
 	bool "High Memory Support"
 	depends on MMU
 	select KMAP_LOCAL
+	select KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
 	help
 	  The address space of ARM processors is only 4 Gigabytes large
 	  and it has to accommodate user address space, kernel address
diff --git a/mm/Kconfig b/mm/Kconfig
index d16ba9249bc5..c048dea7e342 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -887,6 +887,9 @@ config MAPPING_DIRTY_HELPERS
 config KMAP_LOCAL
 	bool
 
+config KMAP_LOCAL_NON_LINEAR_PTE_ARRAY
+	bool
+
 # struct io_mapping based helper.  Selected by drivers that need them
 config IO_MAPPING
 	bool
diff --git a/mm/highmem.c b/mm/highmem.c
index 4212ad0e4a19..1f0c8a52fd80 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -504,16 +504,22 @@ static inline int kmap_local_calc_idx(int idx)
 
 static pte_t *__kmap_pte;
 
-static pte_t *kmap_get_pte(void)
+static pte_t *kmap_get_pte(unsigned long vaddr, int idx)
 {
+	if (IS_ENABLED(CONFIG_KMAP_LOCAL_NON_LINEAR_PTE_ARRAY))
+		/*
+		 * Set by the arch if __kmap_pte[-idx] does not produce
+		 * the correct entry.
+		 */
+		return virt_to_kpte(vaddr);
 	if (!__kmap_pte)
 		__kmap_pte = virt_to_kpte(__fix_to_virt(FIX_KMAP_BEGIN));
-	return __kmap_pte;
+	return &__kmap_pte[-idx];
 }
 
 void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
 {
-	pte_t pteval, *kmap_pte = kmap_get_pte();
+	pte_t pteval, *kmap_pte;
 	unsigned long vaddr;
 	int idx;
 
@@ -525,9 +531,10 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
 	preempt_disable();
 	idx = arch_kmap_local_map_idx(kmap_local_idx_push(), pfn);
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-	BUG_ON(!pte_none(*(kmap_pte - idx)));
+	kmap_pte = kmap_get_pte(vaddr, idx);
+	BUG_ON(!pte_none(*kmap_pte));
 	pteval = pfn_pte(pfn, prot);
-	arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte - idx, pteval);
+	arch_kmap_local_set_pte(&init_mm, vaddr, kmap_pte, pteval);
 	arch_kmap_local_post_map(vaddr, pteval);
 	current->kmap_ctrl.pteval[kmap_local_idx()] = pteval;
 	preempt_enable();
@@ -560,7 +567,7 @@ EXPORT_SYMBOL(__kmap_local_page_prot);
 void kunmap_local_indexed(void *vaddr)
 {
 	unsigned long addr = (unsigned long) vaddr & PAGE_MASK;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int idx;
 
 	if (addr < __fix_to_virt(FIX_KMAP_END) ||
@@ -585,8 +592,9 @@ void kunmap_local_indexed(void *vaddr)
 	idx = arch_kmap_local_unmap_idx(kmap_local_idx(), addr);
 	WARN_ON_ONCE(addr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
 
+	kmap_pte = kmap_get_pte(addr, idx);
 	arch_kmap_local_pre_unmap(addr);
-	pte_clear(&init_mm, addr, kmap_pte - idx);
+	pte_clear(&init_mm, addr, kmap_pte);
 	arch_kmap_local_post_unmap(addr);
 	current->kmap_ctrl.pteval[kmap_local_idx()] = __pte(0);
 	kmap_local_idx_pop();
@@ -608,7 +616,7 @@ EXPORT_SYMBOL(kunmap_local_indexed);
 void __kmap_local_sched_out(void)
 {
 	struct task_struct *tsk = current;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int i;
 
 	/* Clear kmaps */
@@ -635,8 +643,9 @@ void __kmap_local_sched_out(void)
 		idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
 
 		addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
+		kmap_pte = kmap_get_pte(addr, idx);
 		arch_kmap_local_pre_unmap(addr);
-		pte_clear(&init_mm, addr, kmap_pte - idx);
+		pte_clear(&init_mm, addr, kmap_pte);
 		arch_kmap_local_post_unmap(addr);
 	}
 }
@@ -644,7 +653,7 @@ void __kmap_local_sched_out(void)
 void __kmap_local_sched_in(void)
 {
 	struct task_struct *tsk = current;
-	pte_t *kmap_pte = kmap_get_pte();
+	pte_t *kmap_pte;
 	int i;
 
 	/* Restore kmaps */
@@ -664,7 +673,8 @@ void __kmap_local_sched_in(void)
 		/* See comment in __kmap_local_sched_out() */
 		idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
 		addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
-		set_pte_at(&init_mm, addr, kmap_pte - idx, pteval);
+		kmap_pte = kmap_get_pte(addr, idx);
+		set_pte_at(&init_mm, addr, kmap_pte, pteval);
 		arch_kmap_local_post_map(addr, pteval);
 	}
 }
-- 
2.30.2



[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Ard Biesheuvel]

On Tue, 9 Nov 2021 at 12:47, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Fri, Nov 05, 2021 at 04:01:20PM +0100, Ard Biesheuvel wrote:
> > On Fri, 5 Nov 2021 at 15:50, Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Fri, Nov 05, 2021 at 03:21:00PM +0100, Ard Biesheuvel wrote:
> > > > On Tue, 26 Oct 2021 at 15:13, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > The kmap_local conversion broke the ARM architecture, because the new
> > > > > code assumes that all PTEs used for creating kmaps form a linear array
> > > > > in memory, and uses array indexing to look up the kmap PTE belonging to
> > > > > a certain kmap index.
> > > > >
> > > > > On ARM, this cannot work, not only because the PTE pages may be
> > > > > non-adjacent in memory, but also because ARM/!LPAE interleaves hardware
> > > > > entries and extended entries (carrying software-only bits) in a way that
> > > > > is not compatible with array indexing.
> > > > >
> > > > > Fortunately, this only seems to affect configurations with more than 8
> > > > > CPUs, due to the way the per-CPU kmap slots are organized in memory.
> > > > >
> > > > > Work around this by permitting an architecture to set a Kconfig symbol
> > > > > that signifies that the kmap PTEs do not form a lineary array in memory,
> > > > > and so the only way to locate the appropriate one is to walk the page
> > > > > tables.
> > > > >
> > > > > Reported-by: Quanyang Wang <quanyang.wang@windriver.com>
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > >
> > > > Ping? Can we get this fixed please?
> > >
> > > Who are you expecting to apply it? It seems to be touching only core
> > > code, so I don't think it's up to me - not without some kind of review
> > > from mm guys.
> > >
> >
> > I was hoping Thomas would respond, given that he introduced the
> > problem in the first place, and can either carry the fix himself, or
> > tell us whether he wants it fixed in a different way.
>
> We're two weeks on, and Thomas hasn't responded - not on IRC nor via
> email. Can we get akpm to pick the patch up? (Original patch message
> attached.)
>
> I think I have one fix ("fix early early_iounmap()") that needs to go
> to Linus, so alternatively I could send it with that... if someone
> wants to drop it in the patch system.
>

Submitted as 9157/1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] kmap_local: don't assume kmap PTEs are linear arrays in memory

[Andrew Morton]

On Mon, 15 Nov 2021 08:54:33 +0100 Ard Biesheuvel <ardb@kernel.org> wrote:

> > We're two weeks on, and Thomas hasn't responded - not on IRC nor via
> > email. Can we get akpm to pick the patch up? (Original patch message
> > attached.)

Sure, I can handle that.  But I'd prefer a resend, please.  With a
cc:linux-mm.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel