2022-06-08 03:13:42

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()

1. Enhance generic ioremap to make it more useful.
2. Let's arm64 use GENERIC_IOREMAP to cleanup code.
3. Support HAVE_IOREMAP_PROT on arm64, which enable generic_access_phys(),
it is useful when debug(eg, gdb) via access_process_vm device memory
infrastructure.

v5:
- break long lines(> 80 cols), per Christoph Hellwig
- move is_vmalloc_addr() check from arm64 into generic ioremap, per
Christoph Hellwig
- make arm64's ioremap_cache as an inline function, per Christoph
- keep changes simple, make ioremap/iounmap_allowed return bool, per
Baoquan He
- simplify use 'void *' instead of 'void __iomem *' in iounmap, then
drop __force annotation

v4:
- update based on v5.19-rc1
- add generic arch_ioremap/arch_iounmap define, per Andrew Monrton
- simply return an int for arch_ioremap and rename arch_ioremap/arch_iounmap
to a better name, ioremap_allowed/iounmap_allowed, per Arnd Bergmann
- add __force annotation to slince sparse warning in vunmap()

Note,
1) after the renaming, the arm's change(patch1) is not the necessary
dependence for the following changes, but as a cleanup, still post
it here, hope it go in via the arm64 tree with reset of the series
directly if no object.
2) the changes in this version only influence on patch4/5, so retain
the ack/review.

v3:
- add cleanup patch to kill ARM's unused arch_iounmap(the naming will be
used in GENERIC_IOREMAP) and add comments for arch_ioremap/arch_iounmap
hooks, per Anshuman Khandual
- collect ack/review

v2:
- s/addr/phys_addr in ioremap_prot, suggested by Andrew Morton
- rename arch_ioremap/iounmap_check to arch_ioremap/iounmap
and change return value, per Christoph Hellwig and Andrew Morton
- and use 'ifndef arch_ioremap' instead of weak function, per Arnd Bergmann
- collect ack/review

Kefeng Wang (6):
ARM: mm: kill unused runtime hook arch_iounmap()
mm: ioremap: Use more sensibly name in ioremap_prot()
mm: ioremap: Setup phys_addr of struct vm_struct
mm: ioremap: Add ioremap/iounmap_allowed()
arm64: mm: Convert to GENERIC_IOREMAP
arm64: Add HAVE_IOREMAP_PROT support

.../features/vm/ioremap_prot/arch-support.txt | 2 +-
arch/arm/include/asm/io.h | 4 +-
arch/arm/mm/ioremap.c | 9 +-
arch/arm/mm/nommu.c | 9 +-
arch/arm64/Kconfig | 2 +
arch/arm64/include/asm/io.h | 24 +++--
arch/arm64/include/asm/pgtable.h | 10 +++
arch/arm64/kernel/acpi.c | 2 +-
arch/arm64/mm/hugetlbpage.c | 10 ---
arch/arm64/mm/ioremap.c | 90 ++-----------------
include/asm-generic/io.h | 29 +++++-
mm/ioremap.c | 26 ++++--
12 files changed, 90 insertions(+), 127 deletions(-)

--
2.35.3


2022-06-08 04:33:56

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v5 4/6] mm: ioremap: Add ioremap/iounmap_allowed()

Add special hook for architecture to verify addr, size or prot
when ioremap() or iounmap(), which will make the generic ioremap
more useful.

ioremap_allowed() return a bool,
- true means continue to remap
- false means skip remap and return directly
iounmap_allowed() return a bool,
- true means continue to vunmap
- false code means skip vunmap and return directly

Meanwhile, only vunmap the address when it is in vmalloc area
as the generic ioremap only returns vmalloc addresses.

Acked-by: Andrew Morton <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
include/asm-generic/io.h | 26 ++++++++++++++++++++++++++
mm/ioremap.c | 11 ++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index b76379628a02..db5b890eaff7 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -964,6 +964,32 @@ static inline void iounmap(volatile void __iomem *addr)
#elif defined(CONFIG_GENERIC_IOREMAP)
#include <linux/pgtable.h>

+/*
+ * Arch code can implement the following two hooks when using GENERIC_IOREMAP
+ * ioremap_allowed() return a bool,
+ * - true means continue to remap
+ * - false means skip remap and return directly
+ * iounmap_allowed() return a bool,
+ * - true means continue to vunmap
+ * - false means skip vunmap and return directly
+ */
+#ifndef ioremap_allowed
+#define ioremap_allowed ioremap_allowed
+static inline bool ioremap_allowed(phys_addr_t phys_addr, size_t size,
+ unsigned long prot)
+{
+ return true;
+}
+#endif
+
+#ifndef iounmap_allowed
+#define iounmap_allowed iounmap_allowed
+static inline bool iounmap_allowed(void *addr)
+{
+ return true;
+}
+#endif
+
void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
unsigned long prot);
void iounmap(volatile void __iomem *addr);
diff --git a/mm/ioremap.c b/mm/ioremap.c
index e1d008e8f87f..8652426282cc 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -28,6 +28,9 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
phys_addr -= offset;
size = PAGE_ALIGN(size + offset);

+ if (!ioremap_allowed(phys_addr, size, prot))
+ return NULL;
+
area = get_vm_area_caller(size, VM_IOREMAP,
__builtin_return_address(0));
if (!area)
@@ -47,6 +50,12 @@ EXPORT_SYMBOL(ioremap_prot);

void iounmap(volatile void __iomem *addr)
{
- vunmap((void *)((unsigned long)addr & PAGE_MASK));
+ void *vaddr = (void *)((unsigned long)addr & PAGE_MASK);
+
+ if (!iounmap_allowed(vaddr))
+ return;
+
+ if (is_vmalloc_addr(vaddr))
+ vunmap(vaddr);
}
EXPORT_SYMBOL(iounmap);
--
2.35.3

2022-06-08 05:13:49

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v5 2/6] mm: ioremap: Use more sensibly name in ioremap_prot()

Use more meaningful and sensibly naming phys_addr
instead addr in ioremap_prot().

Suggested-by: Andrew Morton <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
include/asm-generic/io.h | 3 ++-
mm/ioremap.c | 14 ++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ce93aaf69f8..b76379628a02 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -964,7 +964,8 @@ static inline void iounmap(volatile void __iomem *addr)
#elif defined(CONFIG_GENERIC_IOREMAP)
#include <linux/pgtable.h>

-void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
+void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
+ unsigned long prot);
void iounmap(volatile void __iomem *addr);

static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
diff --git a/mm/ioremap.c b/mm/ioremap.c
index 5fe598ecd9b7..2d754b48d230 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -11,20 +11,21 @@
#include <linux/io.h>
#include <linux/export.h>

-void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
+void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
+ unsigned long prot)
{
unsigned long offset, vaddr;
phys_addr_t last_addr;
struct vm_struct *area;

/* Disallow wrap-around or zero size */
- last_addr = addr + size - 1;
- if (!size || last_addr < addr)
+ last_addr = phys_addr + size - 1;
+ if (!size || last_addr < phys_addr)
return NULL;

/* Page-align mappings */
- offset = addr & (~PAGE_MASK);
- addr -= offset;
+ offset = phys_addr & (~PAGE_MASK);
+ phys_addr -= offset;
size = PAGE_ALIGN(size + offset);

area = get_vm_area_caller(size, VM_IOREMAP,
@@ -33,7 +34,8 @@ void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
return NULL;
vaddr = (unsigned long)area->addr;

- if (ioremap_page_range(vaddr, vaddr + size, addr, __pgprot(prot))) {
+ if (ioremap_page_range(vaddr, vaddr + size, phys_addr,
+ __pgprot(prot))) {
free_vm_area(area);
return NULL;
}
--
2.35.3

2022-06-08 08:22:32

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH v5 4/6] mm: ioremap: Add ioremap/iounmap_allowed()

On 06/07/22 at 08:50pm, Kefeng Wang wrote:
> Add special hook for architecture to verify addr, size or prot
> when ioremap() or iounmap(), which will make the generic ioremap
> more useful.
>
> ioremap_allowed() return a bool,
> - true means continue to remap
> - false means skip remap and return directly
> iounmap_allowed() return a bool,
> - true means continue to vunmap
> - false code means skip vunmap and return directly
>
> Meanwhile, only vunmap the address when it is in vmalloc area
> as the generic ioremap only returns vmalloc addresses.

LGTM,

Reviewed-by: Baoquan He <[email protected]>

>
> Acked-by: Andrew Morton <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> include/asm-generic/io.h | 26 ++++++++++++++++++++++++++
> mm/ioremap.c | 11 ++++++++++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index b76379628a02..db5b890eaff7 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -964,6 +964,32 @@ static inline void iounmap(volatile void __iomem *addr)
> #elif defined(CONFIG_GENERIC_IOREMAP)
> #include <linux/pgtable.h>
>
> +/*
> + * Arch code can implement the following two hooks when using GENERIC_IOREMAP
> + * ioremap_allowed() return a bool,
> + * - true means continue to remap
> + * - false means skip remap and return directly
> + * iounmap_allowed() return a bool,
> + * - true means continue to vunmap
> + * - false means skip vunmap and return directly
> + */
> +#ifndef ioremap_allowed
> +#define ioremap_allowed ioremap_allowed
> +static inline bool ioremap_allowed(phys_addr_t phys_addr, size_t size,
> + unsigned long prot)
> +{
> + return true;
> +}
> +#endif
> +
> +#ifndef iounmap_allowed
> +#define iounmap_allowed iounmap_allowed
> +static inline bool iounmap_allowed(void *addr)
> +{
> + return true;
> +}
> +#endif
> +
> void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> unsigned long prot);
> void iounmap(volatile void __iomem *addr);
> diff --git a/mm/ioremap.c b/mm/ioremap.c
> index e1d008e8f87f..8652426282cc 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -28,6 +28,9 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> phys_addr -= offset;
> size = PAGE_ALIGN(size + offset);
>
> + if (!ioremap_allowed(phys_addr, size, prot))
> + return NULL;
> +
> area = get_vm_area_caller(size, VM_IOREMAP,
> __builtin_return_address(0));
> if (!area)
> @@ -47,6 +50,12 @@ EXPORT_SYMBOL(ioremap_prot);
>
> void iounmap(volatile void __iomem *addr)
> {
> - vunmap((void *)((unsigned long)addr & PAGE_MASK));
> + void *vaddr = (void *)((unsigned long)addr & PAGE_MASK);
> +
> + if (!iounmap_allowed(vaddr))
> + return;
> +
> + if (is_vmalloc_addr(vaddr))
> + vunmap(vaddr);
> }
> EXPORT_SYMBOL(iounmap);
> --
> 2.35.3
>
>

2022-06-08 08:35:47

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v5 4/6] mm: ioremap: Add ioremap/iounmap_allowed()

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-06-08 08:42:19

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] mm: ioremap: Use more sensibly name in ioremap_prot()

On 06/08/22 at 12:16pm, Baoquan He wrote:
> On 06/07/22 at 08:50pm, Kefeng Wang wrote:
> > Use more meaningful and sensibly naming phys_addr
> > instead addr in ioremap_prot().
> >
> > Suggested-by: Andrew Morton <[email protected]>
> > Reviewed-by: Anshuman Khandual <[email protected]>
> > Signed-off-by: Kefeng Wang <[email protected]>
> > ---
> > include/asm-generic/io.h | 3 ++-
> > mm/ioremap.c | 14 ++++++++------
> > 2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index 7ce93aaf69f8..b76379628a02 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -964,7 +964,8 @@ static inline void iounmap(volatile void __iomem *addr)
> > #elif defined(CONFIG_GENERIC_IOREMAP)
> > #include <linux/pgtable.h>
> >
> > -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
> > +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> > + unsigned long prot);
> > void iounmap(volatile void __iomem *addr);
> >
> > static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
> > diff --git a/mm/ioremap.c b/mm/ioremap.c
> > index 5fe598ecd9b7..2d754b48d230 100644
> > --- a/mm/ioremap.c
> > +++ b/mm/ioremap.c
> > @@ -11,20 +11,21 @@
> > #include <linux/io.h>
> > #include <linux/export.h>
> >
> > -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> > +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> > + unsigned long prot)
> > {
> > unsigned long offset, vaddr;
> > phys_addr_t last_addr;
> > struct vm_struct *area;
> >
> > /* Disallow wrap-around or zero size */
> > - last_addr = addr + size - 1;
> > - if (!size || last_addr < addr)
> > + last_addr = phys_addr + size - 1;
> > + if (!size || last_addr < phys_addr)
> > return NULL;
> >
> > /* Page-align mappings */
> > - offset = addr & (~PAGE_MASK);
> > - addr -= offset;
> > + offset = phys_addr & (~PAGE_MASK);
> ~~~ use offset_in_page() instead?

Sorry, this patch only does s/addr/phys_addr/, please ignore this comment.

>
> Other than this nitpick, this looks good to me.
>
> Reviewed-by: Baoquan He <[email protected]>
>
> > + phys_addr -= offset;
> > size = PAGE_ALIGN(size + offset);
> >
> > area = get_vm_area_caller(size, VM_IOREMAP,
> > @@ -33,7 +34,8 @@ void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> > return NULL;
> > vaddr = (unsigned long)area->addr;
> >
> > - if (ioremap_page_range(vaddr, vaddr + size, addr, __pgprot(prot))) {
> > + if (ioremap_page_range(vaddr, vaddr + size, phys_addr,
> > + __pgprot(prot))) {
> > free_vm_area(area);
> > return NULL;
> > }
> > --
> > 2.35.3
> >
> >
>

2022-06-08 08:52:43

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] mm: ioremap: Use more sensibly name in ioremap_prot()

On 06/07/22 at 08:50pm, Kefeng Wang wrote:
> Use more meaningful and sensibly naming phys_addr
> instead addr in ioremap_prot().
>
> Suggested-by: Andrew Morton <[email protected]>
> Reviewed-by: Anshuman Khandual <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> include/asm-generic/io.h | 3 ++-
> mm/ioremap.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index 7ce93aaf69f8..b76379628a02 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -964,7 +964,8 @@ static inline void iounmap(volatile void __iomem *addr)
> #elif defined(CONFIG_GENERIC_IOREMAP)
> #include <linux/pgtable.h>
>
> -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
> +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> + unsigned long prot);
> void iounmap(volatile void __iomem *addr);
>
> static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
> diff --git a/mm/ioremap.c b/mm/ioremap.c
> index 5fe598ecd9b7..2d754b48d230 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -11,20 +11,21 @@
> #include <linux/io.h>
> #include <linux/export.h>
>
> -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> + unsigned long prot)
> {
> unsigned long offset, vaddr;
> phys_addr_t last_addr;
> struct vm_struct *area;
>
> /* Disallow wrap-around or zero size */
> - last_addr = addr + size - 1;
> - if (!size || last_addr < addr)
> + last_addr = phys_addr + size - 1;
> + if (!size || last_addr < phys_addr)
> return NULL;
>
> /* Page-align mappings */
> - offset = addr & (~PAGE_MASK);
> - addr -= offset;
> + offset = phys_addr & (~PAGE_MASK);
~~~ use offset_in_page() instead?

Other than this nitpick, this looks good to me.

Reviewed-by: Baoquan He <[email protected]>

> + phys_addr -= offset;
> size = PAGE_ALIGN(size + offset);
>
> area = get_vm_area_caller(size, VM_IOREMAP,
> @@ -33,7 +34,8 @@ void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> return NULL;
> vaddr = (unsigned long)area->addr;
>
> - if (ioremap_page_range(vaddr, vaddr + size, addr, __pgprot(prot))) {
> + if (ioremap_page_range(vaddr, vaddr + size, phys_addr,
> + __pgprot(prot))) {
> free_vm_area(area);
> return NULL;
> }
> --
> 2.35.3
>
>

2022-06-08 08:54:27

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH v5 2/6] mm: ioremap: Use more sensibly name in ioremap_prot()

On 06/07/22 at 08:50pm, Kefeng Wang wrote:
> Use more meaningful and sensibly naming phys_addr
~~ sensible, typo
and please fix the subject too.
> instead addr in ioremap_prot().
>
> Suggested-by: Andrew Morton <[email protected]>
> Reviewed-by: Anshuman Khandual <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> include/asm-generic/io.h | 3 ++-
> mm/ioremap.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index 7ce93aaf69f8..b76379628a02 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -964,7 +964,8 @@ static inline void iounmap(volatile void __iomem *addr)
> #elif defined(CONFIG_GENERIC_IOREMAP)
> #include <linux/pgtable.h>
>
> -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
> +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> + unsigned long prot);
> void iounmap(volatile void __iomem *addr);
>
> static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
> diff --git a/mm/ioremap.c b/mm/ioremap.c
> index 5fe598ecd9b7..2d754b48d230 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -11,20 +11,21 @@
> #include <linux/io.h>
> #include <linux/export.h>
>
> -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
> + unsigned long prot)
> {
> unsigned long offset, vaddr;
> phys_addr_t last_addr;
> struct vm_struct *area;
>
> /* Disallow wrap-around or zero size */
> - last_addr = addr + size - 1;
> - if (!size || last_addr < addr)
> + last_addr = phys_addr + size - 1;
> + if (!size || last_addr < phys_addr)
> return NULL;
>
> /* Page-align mappings */
> - offset = addr & (~PAGE_MASK);
> - addr -= offset;
> + offset = phys_addr & (~PAGE_MASK);
> + phys_addr -= offset;
> size = PAGE_ALIGN(size + offset);
>
> area = get_vm_area_caller(size, VM_IOREMAP,
> @@ -33,7 +34,8 @@ void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> return NULL;
> vaddr = (unsigned long)area->addr;
>
> - if (ioremap_page_range(vaddr, vaddr + size, addr, __pgprot(prot))) {
> + if (ioremap_page_range(vaddr, vaddr + size, phys_addr,
> + __pgprot(prot))) {
> free_vm_area(area);
> return NULL;
> }
> --
> 2.35.3
>
>

2022-06-10 10:34:28

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v5 resend 2/6] mm: ioremap: Use more sensible name in ioremap_prot()

Use more meaningful and sensible naming phys_addr
instead of addr in ioremap_prot().

Suggested-by: Andrew Morton <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Baoquan He <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
v5 resend:
- use sensible and add RB
include/asm-generic/io.h | 3 ++-
mm/ioremap.c | 14 ++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ce93aaf69f8..b76379628a02 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -964,7 +964,8 @@ static inline void iounmap(volatile void __iomem *addr)
#elif defined(CONFIG_GENERIC_IOREMAP)
#include <linux/pgtable.h>

-void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
+void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
+ unsigned long prot);
void iounmap(volatile void __iomem *addr);

static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
diff --git a/mm/ioremap.c b/mm/ioremap.c
index 5fe598ecd9b7..2d754b48d230 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -11,20 +11,21 @@
#include <linux/io.h>
#include <linux/export.h>

-void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
+void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
+ unsigned long prot)
{
unsigned long offset, vaddr;
phys_addr_t last_addr;
struct vm_struct *area;

/* Disallow wrap-around or zero size */
- last_addr = addr + size - 1;
- if (!size || last_addr < addr)
+ last_addr = phys_addr + size - 1;
+ if (!size || last_addr < phys_addr)
return NULL;

/* Page-align mappings */
- offset = addr & (~PAGE_MASK);
- addr -= offset;
+ offset = phys_addr & (~PAGE_MASK);
+ phys_addr -= offset;
size = PAGE_ALIGN(size + offset);

area = get_vm_area_caller(size, VM_IOREMAP,
@@ -33,7 +34,8 @@ void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
return NULL;
vaddr = (unsigned long)area->addr;

- if (ioremap_page_range(vaddr, vaddr + size, addr, __pgprot(prot))) {
+ if (ioremap_page_range(vaddr, vaddr + size, phys_addr,
+ __pgprot(prot))) {
free_vm_area(area);
return NULL;
}
--
2.35.3

2022-06-14 04:25:02

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()

Hi Catalin, could you help to pick up it, thanks.

On 2022/6/7 20:50, Kefeng Wang wrote:
> 1. Enhance generic ioremap to make it more useful.
> 2. Let's arm64 use GENERIC_IOREMAP to cleanup code.
> 3. Support HAVE_IOREMAP_PROT on arm64, which enable generic_access_phys(),
> it is useful when debug(eg, gdb) via access_process_vm device memory
> infrastructure.
>
> v5:
> - break long lines(> 80 cols), per Christoph Hellwig
> - move is_vmalloc_addr() check from arm64 into generic ioremap, per
> Christoph Hellwig
> - make arm64's ioremap_cache as an inline function, per Christoph
> - keep changes simple, make ioremap/iounmap_allowed return bool, per
> Baoquan He
> - simplify use 'void *' instead of 'void __iomem *' in iounmap, then
> drop __force annotation
>
> v4:
> - update based on v5.19-rc1
> - add generic arch_ioremap/arch_iounmap define, per Andrew Monrton
> - simply return an int for arch_ioremap and rename arch_ioremap/arch_iounmap
> to a better name, ioremap_allowed/iounmap_allowed, per Arnd Bergmann
> - add __force annotation to slince sparse warning in vunmap()
>
> Note,
> 1) after the renaming, the arm's change(patch1) is not the necessary
> dependence for the following changes, but as a cleanup, still post
> it here, hope it go in via the arm64 tree with reset of the series
> directly if no object.
> 2) the changes in this version only influence on patch4/5, so retain
> the ack/review.
>
> v3:
> - add cleanup patch to kill ARM's unused arch_iounmap(the naming will be
> used in GENERIC_IOREMAP) and add comments for arch_ioremap/arch_iounmap
> hooks, per Anshuman Khandual
> - collect ack/review
>
> v2:
> - s/addr/phys_addr in ioremap_prot, suggested by Andrew Morton
> - rename arch_ioremap/iounmap_check to arch_ioremap/iounmap
> and change return value, per Christoph Hellwig and Andrew Morton
> - and use 'ifndef arch_ioremap' instead of weak function, per Arnd Bergmann
> - collect ack/review
>
> Kefeng Wang (6):
> ARM: mm: kill unused runtime hook arch_iounmap()
> mm: ioremap: Use more sensibly name in ioremap_prot()
> mm: ioremap: Setup phys_addr of struct vm_struct
> mm: ioremap: Add ioremap/iounmap_allowed()
> arm64: mm: Convert to GENERIC_IOREMAP
> arm64: Add HAVE_IOREMAP_PROT support
>
> .../features/vm/ioremap_prot/arch-support.txt | 2 +-
> arch/arm/include/asm/io.h | 4 +-
> arch/arm/mm/ioremap.c | 9 +-
> arch/arm/mm/nommu.c | 9 +-
> arch/arm64/Kconfig | 2 +
> arch/arm64/include/asm/io.h | 24 +++--
> arch/arm64/include/asm/pgtable.h | 10 +++
> arch/arm64/kernel/acpi.c | 2 +-
> arch/arm64/mm/hugetlbpage.c | 10 ---
> arch/arm64/mm/ioremap.c | 90 ++-----------------
> include/asm-generic/io.h | 29 +++++-
> mm/ioremap.c | 26 ++++--
> 12 files changed, 90 insertions(+), 127 deletions(-)
>

2022-06-27 11:24:00

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()


On 2022/6/14 11:21, Kefeng Wang wrote:
> Hi Catalin, could you help to pick up it, thanks.
Kindly ping...
>
> On 2022/6/7 20:50, Kefeng Wang wrote:
>> 1. Enhance generic ioremap to make it more useful.
>> 2. Let's arm64 use GENERIC_IOREMAP to cleanup code.
>> 3. Support HAVE_IOREMAP_PROT on arm64, which enable
>> generic_access_phys(),
>>     it is useful when debug(eg, gdb) via access_process_vm device memory
>>     infrastructure.

2022-06-27 11:24:55

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()


On 2022/6/27 19:14, Will Deacon wrote:
> On Mon, Jun 27, 2022 at 07:06:41PM +0800, Kefeng Wang wrote:
>> On 2022/6/14 11:21, Kefeng Wang wrote:
>>> Hi Catalin, could you help to pick up it, thanks.
>> Kindly ping...
> Sorry, this is on my plate for 5.20. I'll try to get to it today.
Good to know it, thank you.
>
> Will
> .

2022-06-27 12:34:38

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()

On Mon, Jun 27, 2022 at 07:06:41PM +0800, Kefeng Wang wrote:
>
> On 2022/6/14 11:21, Kefeng Wang wrote:
> > Hi Catalin, could you help to pick up it, thanks.
> Kindly ping...

Sorry, this is on my plate for 5.20. I'll try to get to it today.

Will

2022-06-27 13:43:40

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()

On Tue, 7 Jun 2022 20:50:21 +0800, Kefeng Wang wrote:
> 1. Enhance generic ioremap to make it more useful.
> 2. Let's arm64 use GENERIC_IOREMAP to cleanup code.
> 3. Support HAVE_IOREMAP_PROT on arm64, which enable generic_access_phys(),
> it is useful when debug(eg, gdb) via access_process_vm device memory
> infrastructure.
>
> v5:
> - break long lines(> 80 cols), per Christoph Hellwig
> - move is_vmalloc_addr() check from arm64 into generic ioremap, per
> Christoph Hellwig
> - make arm64's ioremap_cache as an inline function, per Christoph
> - keep changes simple, make ioremap/iounmap_allowed return bool, per
> Baoquan He
> - simplify use 'void *' instead of 'void __iomem *' in iounmap, then
> drop __force annotation
>
> [...]

Applied to arm64 (for-next/ioremap), thanks!

[1/6] ARM: mm: kill unused runtime hook arch_iounmap()
https://git.kernel.org/arm64/c/d803336abdbc
[2/6] mm: ioremap: Use more sensibly name in ioremap_prot()
https://git.kernel.org/arm64/c/abc5992b9dd0
[3/6] mm: ioremap: Setup phys_addr of struct vm_struct
https://git.kernel.org/arm64/c/a14fff1c0379
[4/6] mm: ioremap: Add ioremap/iounmap_allowed()
https://git.kernel.org/arm64/c/18e780b4e6ab
[5/6] arm64: mm: Convert to GENERIC_IOREMAP
https://git.kernel.org/arm64/c/f23eab0bfaef
[6/6] arm64: Add HAVE_IOREMAP_PROT support
https://git.kernel.org/arm64/c/893dea9ccd08

Cheers,
--
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev