From: Christophe Leroy <christophe.leroy@csgroup.eu>
Architectures like powerpc have a dedicated space for IOREMAP mappings.
If so, use it in generic_ioremap_pro().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
mm/ioremap.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/ioremap.c b/mm/ioremap.c
index 2fbe6b9bc50e..4a7749d85044 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -35,8 +35,13 @@ void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size,
if (!ioremap_allowed(phys_addr, size, pgprot_val(prot)))
return NULL;
+#ifdef IOREMAP_START
+ area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_START,
+ IOREMAP_END, __builtin_return_address(0));
+#else
area = get_vm_area_caller(size, VM_IOREMAP,
__builtin_return_address(0));
+#endif
if (!area)
return NULL;
vaddr = (unsigned long)area->addr;
@@ -66,7 +71,7 @@ void generic_iounmap(volatile void __iomem *addr)
if (!iounmap_allowed(vaddr))
return;
- if (is_vmalloc_addr(vaddr))
+ if (is_ioremap_addr(vaddr))
vunmap(vaddr);
}
--
2.34.1On Mon, May 15, 2023 at 05:08:45PM +0800, Baoquan He wrote: > @@ -35,8 +35,13 @@ void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, > if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) > return NULL; > > +#ifdef IOREMAP_START > + area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_START, > + IOREMAP_END, __builtin_return_address(0)); > +#else > area = get_vm_area_caller(size, VM_IOREMAP, > __builtin_return_address(0)); > +#endif I think this would be cleaner if we'd just always use __get_vm_area_caller and at the top of the file add a: #ifndef IOREMAP_START #define IOREMAP_START VMALLOC_START #define IOREMAP_END VMALLOC_END #endif Together with a little comment that ioremap often, but not always uses the generic vmalloc area.
On 05/16/23 at 11:41pm, Christoph Hellwig wrote: > On Mon, May 15, 2023 at 05:08:45PM +0800, Baoquan He wrote: > > @@ -35,8 +35,13 @@ void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, > > if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) > > return NULL; > > > > +#ifdef IOREMAP_START > > + area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_START, > > + IOREMAP_END, __builtin_return_address(0)); > > +#else > > area = get_vm_area_caller(size, VM_IOREMAP, > > __builtin_return_address(0)); > > +#endif > > I think this would be cleaner if we'd just always use > __get_vm_area_caller and at the top of the file add a: > > #ifndef IOREMAP_START > #define IOREMAP_START VMALLOC_START > #define IOREMAP_END VMALLOC_END > #endif > > Together with a little comment that ioremap often, but not always > uses the generic vmalloc area. Great idea, will do as suggested.
On Tue, May 16, 2023 at 11:41:26PM -0700, Christoph Hellwig wrote: > I think this would be cleaner if we'd just always use > __get_vm_area_caller and at the top of the file add a: > > #ifndef IOREMAP_START > #define IOREMAP_START VMALLOC_START > #define IOREMAP_END VMALLOC_END > #endif > > Together with a little comment that ioremap often, but not always > uses the generic vmalloc area. .. and with that we can also simply is_ioremap_addr by moving it to ioremap.c and making it always operate on the IOREMAP constants.
On 05/16/23 at 11:44pm, Christoph Hellwig wrote:
> On Tue, May 16, 2023 at 11:41:26PM -0700, Christoph Hellwig wrote:
> > I think this would be cleaner if we'd just always use
> > __get_vm_area_caller and at the top of the file add a:
> >
> > #ifndef IOREMAP_START
> > #define IOREMAP_START VMALLOC_START
> > #define IOREMAP_END VMALLOC_END
> > #endif
> >
> > Together with a little comment that ioremap often, but not always
> > uses the generic vmalloc area.
>
> .. and with that we can also simply is_ioremap_addr by moving it
> to ioremap.c and making it always operate on the IOREMAP constants.
In the current code, is_ioremap_addr() is being used in kernel/iomem.c.
However, mm/ioremap.c is only built in when CONFIG_GENERIC_IOREMAP is
enabled. This will impact those architectures which haven't taken
GENERIC_IOREMAP way.
[~]$ git grep is_ioremap_addr
arch/powerpc/include/asm/pgtable.h:#define is_ioremap_addr is_ioremap_addr
arch/powerpc/include/asm/pgtable.h:static inline bool is_ioremap_addr(const void *x)
include/linux/mm.h:static inline bool is_ioremap_addr(const void *x)
include/linux/mm.h:static inline bool is_ioremap_addr(const void *x)
kernel/iomem.c: if (is_ioremap_addr(addr))
mm/ioremap.c: if (is_ioremap_addr(vaddr))
[bhe@MiWiFi-R3L-srv linux-arm64]$ git grep ioremap mm/Makefile
mm/Makefile:obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
mm/Makefile:obj-$(CONFIG_GENERIC_IOREMAP) += ioremap.o
If we want to consolidate code, we can move is_ioremap_addr() to
include/linux/mm.h libe below. Not sure if it's fine. With it,
both kernel/iomem.c and mm/ioremap.c can use is_ioremap_addr().
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..0fbb94f0f025 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1041,9 +1041,25 @@ unsigned long vmalloc_to_pfn(const void *addr);
* On nommu, vmalloc/vfree wrap through kmalloc/kfree directly, so there
* is no special casing required.
*/
-
-#ifndef is_ioremap_addr
-#define is_ioremap_addr(x) is_vmalloc_addr(x)
+#if defined(CONFIG_HAS_IOMEM) || defined(CONFIG_GENERIC_IOREMAP)
+/*
+ * Ioremap often, but not always uses the generic vmalloc area. E.g on
+ * Power ARCH, it could have different ioremap space.
+ */
+#ifndef IOREMAP_START
+#define IOREMAP_START VMALLOC_START
+#define IOREMAP_END VMALLOC_END
+#endif
+static inline bool is_ioremap_addr(const void *x)
+{
+ unsigned long addr = (unsigned long)kasan_reset_tag(x);
+ return addr >= IOREMAP_START && addr < IOREMAP_END;
+}
+#else
+static inline bool is_ioremap_addr(const void *x)
+{
+ return false;
+}
#endif
#ifdef CONFIG_MMU
On Tue, May 30, 2023 at 05:37:23PM +0800, Baoquan He wrote: > If we want to consolidate code, we can move is_ioremap_addr() to > include/linux/mm.h libe below. Not sure if it's fine. With it, > both kernel/iomem.c and mm/ioremap.c can use is_ioremap_addr(). Can we just add a ne header for this given that no one else really needs it?
On 06/01/23 at 04:13am, Christoph Hellwig wrote:
> On Tue, May 30, 2023 at 05:37:23PM +0800, Baoquan He wrote:
> > If we want to consolidate code, we can move is_ioremap_addr() to
> > include/linux/mm.h libe below. Not sure if it's fine. With it,
> > both kernel/iomem.c and mm/ioremap.c can use is_ioremap_addr().
>
> Can we just add a ne header for this given that no one else really
> needs it?
Is it OK like below?
From fe5d4d25afa1e989fa82877c8466a97fc8bd8c93 Mon Sep 17 00:00:00 2001
From: Baoquan He <bhe@redhat.com>
Date: Fri, 2 Jun 2023 18:36:48 +0800
Subject: [PATCH] mm: move is_ioremap_addr() into new header file
Content-type: text/plain
Now is_ioremap_addr() is only used in kernel/iomem.c and gonna be used
in mm/ioremap.c. Move it into its own new header file linux/ioremap.h.
Signed-off-by: Baoquan He <bhe@redhat.com>
---
include/linux/ioremap.h | 29 +++++++++++++++++++++++++++++
include/linux/mm.h | 5 -----
kernel/iomem.c | 1 +
mm/ioremap.c | 1 +
4 files changed, 31 insertions(+), 5 deletions(-)
create mode 100644 include/linux/ioremap.h
diff --git a/include/linux/ioremap.h b/include/linux/ioremap.h
new file mode 100644
index 000000000000..2fd51a77ebdc
--- /dev/null
+++ b/include/linux/ioremap.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_IOREMAP_H
+#define _LINUX_IOREMAP_H
+
+#include <linux/kasan.h>
+#include <asm/pgtable.h>
+
+#if defined(CONFIG_HAS_IOMEM) || defined(CONFIG_GENERIC_IOREMAP)
+/*
+ * Ioremap often, but not always uses the generic vmalloc area. E.g on
+ * Power ARCH, it could have different ioremap space.
+ */
+#ifndef IOREMAP_START
+#define IOREMAP_START VMALLOC_START
+#define IOREMAP_END VMALLOC_END
+#endif
+static inline bool is_ioremap_addr(const void *x)
+{
+ unsigned long addr = (unsigned long)kasan_reset_tag(x);
+ return addr >= IOREMAP_START && addr < IOREMAP_END;
+}
+#else
+static inline bool is_ioremap_addr(const void *x)
+{
+ return false;
+}
+#endif
+
+#endif /* _LINUX_IOREMAP_H */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..7379f19768b4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1041,11 +1041,6 @@ unsigned long vmalloc_to_pfn(const void *addr);
* On nommu, vmalloc/vfree wrap through kmalloc/kfree directly, so there
* is no special casing required.
*/
-
-#ifndef is_ioremap_addr
-#define is_ioremap_addr(x) is_vmalloc_addr(x)
-#endif
-
#ifdef CONFIG_MMU
extern bool is_vmalloc_addr(const void *x);
extern int is_vmalloc_or_module_addr(const void *x);
diff --git a/kernel/iomem.c b/kernel/iomem.c
index 62c92e43aa0d..9682471e6471 100644
--- a/kernel/iomem.c
+++ b/kernel/iomem.c
@@ -3,6 +3,7 @@
#include <linux/types.h>
#include <linux/io.h>
#include <linux/mm.h>
+#include <linux/ioremap.h>
#ifndef ioremap_cache
/* temporary while we convert existing ioremap_cache users to memremap */
diff --git a/mm/ioremap.c b/mm/ioremap.c
index 0248e630561b..3dede3302eba 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -10,6 +10,7 @@
#include <linux/mm.h>
#include <linux/io.h>
#include <linux/export.h>
+#include <linux/ioremap.h>
/*
* Ioremap often, but not always uses the generic vmalloc area. E.g on
--
2.34.1
>
>
On Fri, Jun 02, 2023 at 06:42:59PM +0800, Baoquan He wrote: > On 06/01/23 at 04:13am, Christoph Hellwig wrote: > > On Tue, May 30, 2023 at 05:37:23PM +0800, Baoquan He wrote: > > > If we want to consolidate code, we can move is_ioremap_addr() to > > > include/linux/mm.h libe below. Not sure if it's fine. With it, > > > both kernel/iomem.c and mm/ioremap.c can use is_ioremap_addr(). > > > > Can we just add a ne header for this given that no one else really > > needs it? > > Is it OK like below? Looks good to me: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 05/16/23 at 11:44pm, Christoph Hellwig wrote: > On Tue, May 16, 2023 at 11:41:26PM -0700, Christoph Hellwig wrote: > > I think this would be cleaner if we'd just always use > > __get_vm_area_caller and at the top of the file add a: > > > > #ifndef IOREMAP_START > > #define IOREMAP_START VMALLOC_START > > #define IOREMAP_END VMALLOC_END > > #endif > > > > Together with a little comment that ioremap often, but not always > > uses the generic vmalloc area. > > .. and with that we can also simply is_ioremap_addr by moving it > to ioremap.c and making it always operate on the IOREMAP constants. Great idea too, will do. Put this into a separate patch?
On Sat, May 20, 2023 at 11:31:04AM +0800, Baoquan He wrote: > > > Together with a little comment that ioremap often, but not always > > > uses the generic vmalloc area. > > > > .. and with that we can also simply is_ioremap_addr by moving it > > to ioremap.c and making it always operate on the IOREMAP constants. > > Great idea too, will do. Put this into a separate patch? Yes.
On Mon, May 15, 2023 at 05:08:45PM +0800, Baoquan He wrote: > From: Christophe Leroy <christophe.leroy@csgroup.eu> > > Architectures like powerpc have a dedicated space for IOREMAP mappings. > > If so, use it in generic_ioremap_pro(). > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Baoquan He <bhe@redhat.com> Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org> > --- > mm/ioremap.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/ioremap.c b/mm/ioremap.c > index 2fbe6b9bc50e..4a7749d85044 100644 > --- a/mm/ioremap.c > +++ b/mm/ioremap.c > @@ -35,8 +35,13 @@ void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, > if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) > return NULL; > > +#ifdef IOREMAP_START > + area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_START, > + IOREMAP_END, __builtin_return_address(0)); > +#else > area = get_vm_area_caller(size, VM_IOREMAP, > __builtin_return_address(0)); > +#endif > if (!area) > return NULL; > vaddr = (unsigned long)area->addr; > @@ -66,7 +71,7 @@ void generic_iounmap(volatile void __iomem *addr) > if (!iounmap_allowed(vaddr)) > return; > > - if (is_vmalloc_addr(vaddr)) > + if (is_ioremap_addr(vaddr)) > vunmap(vaddr); > } > > -- > 2.34.1 > > -- Sincerely yours, Mike.
© 2016 - 2026 Red Hat, Inc.