[PATCH v2 0/5] arm64: Cleanup ioremap() and support ioremap_prot()

[PATCH v2 0/5] arm64: Cleanup ioremap() and support ioremap_prot()

[Kefeng Wang]

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.

v2:
- s/addr/phys_addr in ioremap_prot, suggested by Andrew Morton 
- rename arch_ioremap/iounmap_check to arch_ioremap/iounmad
  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 (5):
  mm: ioremap: Use more sensibly name in ioremap_prot()
  mm: ioremap: Setup phys_addr of struct vm_struct
  mm: ioremap: Add arch_ioremap/iounmap()
  arm64: mm: Convert to GENERIC_IOREMAP
  arm64: Add HAVE_IOREMAP_PROT support

 .../features/vm/ioremap_prot/arch-support.txt |  2 +-
 arch/arm64/Kconfig                            |  2 +
 arch/arm64/include/asm/io.h                   | 20 +++--
 arch/arm64/include/asm/pgtable.h              | 10 +++
 arch/arm64/kernel/acpi.c                      |  2 +-
 arch/arm64/mm/hugetlbpage.c                   | 10 ---
 arch/arm64/mm/ioremap.c                       | 85 +++----------------
 include/asm-generic/io.h                      | 16 +++-
 mm/ioremap.c                                  | 27 ++++--
 9 files changed, 74 insertions(+), 100 deletions(-)

-- 
2.35.3

[PATCH v2 1/5] mm: ioremap: Use more sensibly name in ioremap_prot()

[Kefeng Wang]

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

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/asm-generic/io.h |  2 +-
 mm/ioremap.c             | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ce93aaf69f8..e6ffa2519f08 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -964,7 +964,7 @@ 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..1f9597fbcc07 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -11,20 +11,20 @@
 #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 +33,7 @@ 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

[PATCH v2 2/5] mm: ioremap: Setup phys_addr of struct vm_struct

[Kefeng Wang]

Show physical address of each ioremap in /proc/vmallocinfo.

Acked-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/ioremap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/ioremap.c b/mm/ioremap.c
index 1f9597fbcc07..7cb9996b0c12 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -32,6 +32,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
 	if (!area)
 		return NULL;
 	vaddr = (unsigned long)area->addr;
+	area->phys_addr = phys_addr;
 
 	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, __pgprot(prot))) {
 		free_vm_area(area);
-- 
2.35.3

[PATCH v2 3/5] mm: ioremap: Add arch_ioremap/iounmap()

[Kefeng Wang]

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

  arch_ioremap() return a 'void __iomem *',
    - IS_ERR means return an error
    - NULL means continue to remap
    - a non-NULL, non-IS_ERR pointer is directly returned
  arch_iounmap() return a int value,
    - 0 means continue to vunmap
    - error code means skip vunmap and return directly

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/asm-generic/io.h | 14 ++++++++++++++
 mm/ioremap.c             | 14 +++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index e6ffa2519f08..f2f9aeedb5e8 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -964,6 +964,20 @@ static inline void iounmap(volatile void __iomem *addr)
 #elif defined(CONFIG_GENERIC_IOREMAP)
 #include <linux/pgtable.h>
 
+#ifndef arch_ioremap
+static inline void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
+{
+	return NULL;
+}
+#endif
+
+#ifndef arch_iounmap
+static inline int arch_iounmap(void __iomem *addr)
+{
+	return 0;
+}
+#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 7cb9996b0c12..de5a2e899e14 100644
--- a/mm/ioremap.c
+++ b/mm/ioremap.c
@@ -16,6 +16,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
 	unsigned long offset, vaddr;
 	phys_addr_t last_addr;
 	struct vm_struct *area;
+	void __iomem *base;
 
 	/* Disallow wrap-around or zero size */
 	last_addr = phys_addr + size - 1;
@@ -27,6 +28,12 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
 	phys_addr -= offset;
 	size = PAGE_ALIGN(size + offset);
 
+	base = arch_ioremap(phys_addr, size, prot);
+	if (IS_ERR(base))
+		return NULL;
+	else if (base)
+		return base;
+
 	area = get_vm_area_caller(size, VM_IOREMAP,
 			__builtin_return_address(0));
 	if (!area)
@@ -45,6 +52,11 @@ 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 (arch_iounmap(vaddr))
+		return;
+
+	vunmap(vaddr);
 }
 EXPORT_SYMBOL(iounmap);
-- 
2.35.3

[PATCH v2 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[Kefeng Wang]

Add hook for arm64's special operation when ioremap() and iounmap(),
then ioremap_wc/np/cache is converted to use ioremap_prot()
from GENERIC_IOREMAP, update the Copyright and kill the unused
inclusions.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm64/Kconfig          |  1 +
 arch/arm64/include/asm/io.h | 20 ++++++---
 arch/arm64/kernel/acpi.c    |  2 +-
 arch/arm64/mm/ioremap.c     | 85 +++++--------------------------------
 4 files changed, 27 insertions(+), 81 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 20ea89d9ac2f..56673209fdb9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -123,6 +123,7 @@ config ARM64
 	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_IDLE_POLL_SETUP
+	select GENERIC_IOREMAP
 	select GENERIC_IRQ_IPI
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_SHOW
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 7fd836bea7eb..042fa01940b8 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -163,13 +163,21 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
 /*
  * I/O memory mapping functions.
  */
-extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
-extern void iounmap(volatile void __iomem *addr);
-extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
 
-#define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
-#define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
-#define ioremap_np(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
+void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot);
+#define arch_ioremap arch_ioremap
+
+int arch_iounmap(void __iomem *addr);
+#define arch_iounmap arch_iounmap
+
+#define _PAGE_IOREMAP PROT_DEVICE_nGnRE
+
+#define ioremap_wc(addr, size)		ioremap_prot((addr), (size), PROT_NORMAL_NC)
+#define ioremap_np(addr, size)		ioremap_prot((addr), (size), PROT_DEVICE_nGnRnE)
+#define ioremap_cache(addr, size) ({							\
+	pfn_is_map_memory(__phys_to_pfn(addr)) ?					\
+	(void __iomem *)__phys_to_virt(addr) : ioremap_prot(addr, size, PROT_NORMAL);	\
+})
 
 /*
  * io{read,write}{16,32,64}be() macros
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e4dea8db6924..a5a256e3f9fe 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -351,7 +351,7 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
 				prot = __acpi_get_writethrough_mem_attribute();
 		}
 	}
-	return __ioremap(phys, size, prot);
+	return ioremap_prot(phys, size, pgprot_val(prot));
 }
 
 /*
diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
index b7c81dacabf0..a12e315726e7 100644
--- a/arch/arm64/mm/ioremap.c
+++ b/arch/arm64/mm/ioremap.c
@@ -1,96 +1,33 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * Based on arch/arm/mm/ioremap.c
- *
- * (C) Copyright 1995 1996 Linus Torvalds
- * Hacked for ARM by Phil Blundell <philb@gnu.org>
- * Hacked to allow all architectures to build, and various cleanups
- * by Russell King
- * Copyright (C) 2012 ARM Ltd.
- */
 
-#include <linux/export.h>
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 
-#include <asm/fixmap.h>
-#include <asm/tlbflush.h>
-
-static void __iomem *__ioremap_caller(phys_addr_t phys_addr, size_t size,
-				      pgprot_t prot, void *caller)
+void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
 {
-	unsigned long last_addr;
-	unsigned long offset = phys_addr & ~PAGE_MASK;
-	int err;
-	unsigned long addr;
-	struct vm_struct *area;
-
-	/*
-	 * Page align the mapping address and size, taking account of any
-	 * offset.
-	 */
-	phys_addr &= PAGE_MASK;
-	size = PAGE_ALIGN(size + offset);
+	unsigned long last_addr = phys_addr + size - 1;
+	int ret = -EINVAL;
 
-	/*
-	 * Don't allow wraparound, zero size or outside PHYS_MASK.
-	 */
-	last_addr = phys_addr + size - 1;
-	if (!size || last_addr < phys_addr || (last_addr & ~PHYS_MASK))
-		return NULL;
+	/* Don't allow outside PHYS_MASK */
+	if (last_addr & ~PHYS_MASK)
+		return ERR_PTR(ret);
 
-	/*
-	 * Don't allow RAM to be mapped.
-	 */
+	/* Don't allow RAM to be mapped. */
 	if (WARN_ON(pfn_is_map_memory(__phys_to_pfn(phys_addr))))
-		return NULL;
+		return ERR_PTR(ret);
 
-	area = get_vm_area_caller(size, VM_IOREMAP, caller);
-	if (!area)
-		return NULL;
-	addr = (unsigned long)area->addr;
-	area->phys_addr = phys_addr;
-
-	err = ioremap_page_range(addr, addr + size, phys_addr, prot);
-	if (err) {
-		vunmap((void *)addr);
-		return NULL;
-	}
-
-	return (void __iomem *)(offset + addr);
-}
-
-void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot)
-{
-	return __ioremap_caller(phys_addr, size, prot,
-				__builtin_return_address(0));
+	return NULL;
 }
-EXPORT_SYMBOL(__ioremap);
 
-void iounmap(volatile void __iomem *io_addr)
+int arch_iounmap(void __iomem *addr)
 {
-	unsigned long addr = (unsigned long)io_addr & PAGE_MASK;
-
 	/*
 	 * We could get an address outside vmalloc range in case
 	 * of ioremap_cache() reusing a RAM mapping.
 	 */
-	if (is_vmalloc_addr((void *)addr))
-		vunmap((void *)addr);
-}
-EXPORT_SYMBOL(iounmap);
-
-void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
-{
-	/* For normal memory we already have a cacheable mapping. */
-	if (pfn_is_map_memory(__phys_to_pfn(phys_addr)))
-		return (void __iomem *)__phys_to_virt(phys_addr);
-
-	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL),
-				__builtin_return_address(0));
+	return is_vmalloc_addr(addr) ? 0 : -EINVAL;
 }
-EXPORT_SYMBOL(ioremap_cache);
 
 /*
  * Must be called after early_fixmap_init
-- 
2.35.3

[PATCH v2 5/5] arm64: Add HAVE_IOREMAP_PROT support

[Kefeng Wang]

With ioremap_prot() defination from generic ioremap, also move
pte_pgprot() from hugetlbpage.c into pgtable.h, then arm64 could
have HAVE_IOREMAP_PROT, which will enable generic_access_phys()
code.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 .../features/vm/ioremap_prot/arch-support.txt          |  2 +-
 arch/arm64/Kconfig                                     |  1 +
 arch/arm64/include/asm/pgtable.h                       | 10 ++++++++++
 arch/arm64/mm/hugetlbpage.c                            | 10 ----------
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/Documentation/features/vm/ioremap_prot/arch-support.txt b/Documentation/features/vm/ioremap_prot/arch-support.txt
index a6dcbe5f47b6..b39ad5d61216 100644
--- a/Documentation/features/vm/ioremap_prot/arch-support.txt
+++ b/Documentation/features/vm/ioremap_prot/arch-support.txt
@@ -9,7 +9,7 @@
     |       alpha: | TODO |
     |         arc: |  ok  |
     |         arm: | TODO |
-    |       arm64: | TODO |
+    |       arm64: |  ok  |
     |        csky: | TODO |
     |       h8300: | TODO |
     |     hexagon: | TODO |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 56673209fdb9..5e5889049af0 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -186,6 +186,7 @@ config ARM64
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_GCC_PLUGINS
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
+	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KVM
 	select HAVE_NMI
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index dff2b483ea50..1402a2739024 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -402,6 +402,16 @@ static inline pgprot_t mk_pmd_sect_prot(pgprot_t prot)
 	return __pgprot((pgprot_val(prot) & ~PMD_TABLE_BIT) | PMD_TYPE_SECT);
 }
 
+/*
+ * Select all bits except the pfn
+ */
+static inline pgprot_t pte_pgprot(pte_t pte)
+{
+	unsigned long pfn = pte_pfn(pte);
+
+	return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
+}
+
 #ifdef CONFIG_NUMA_BALANCING
 /*
  * See the comment in include/linux/pgtable.h
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index cbace1c9e137..38d03406f6aa 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -100,16 +100,6 @@ int pud_huge(pud_t pud)
 #endif
 }
 
-/*
- * Select all bits except the pfn
- */
-static inline pgprot_t pte_pgprot(pte_t pte)
-{
-	unsigned long pfn = pte_pfn(pte);
-
-	return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
-}
-
 static int find_num_contig(struct mm_struct *mm, unsigned long addr,
 			   pte_t *ptep, size_t *pgsize)
 {
-- 
2.35.3

Re: [PATCH v2 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[kernel test robot]

Hi Kefeng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on hnaz-mm/master arnd-asm-generic/master v5.18-rc4 next-20220429]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Kefeng-Wang/arm64-Cleanup-ioremap-and-support-ioremap_prot/20220429-182215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-s032-20220429 (https://download.01.org/0day-ci/archive/20220430/202204300706.lSqtHOzz-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/intel-lab-lkp/linux/commit/77442d1fe8ed26b996b2b7d4464443a63097c434
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kefeng-Wang/arm64-Cleanup-ioremap-and-support-ioremap_prot/20220429-182215
        git checkout 77442d1fe8ed26b996b2b7d4464443a63097c434
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/mm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> arch/arm64/mm/ioremap.c:14:31: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void [noderef] __iomem * @@     got void * @@
   arch/arm64/mm/ioremap.c:14:31: sparse:     expected void [noderef] __iomem *
   arch/arm64/mm/ioremap.c:14:31: sparse:     got void *
   arch/arm64/mm/ioremap.c:18:31: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void [noderef] __iomem * @@     got void * @@
   arch/arm64/mm/ioremap.c:18:31: sparse:     expected void [noderef] __iomem *
   arch/arm64/mm/ioremap.c:18:31: sparse:     got void *
   arch/arm64/mm/ioremap.c:29:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const *x @@     got void [noderef] __iomem *addr @@
   arch/arm64/mm/ioremap.c:29:32: sparse:     expected void const *x
   arch/arm64/mm/ioremap.c:29:32: sparse:     got void [noderef] __iomem *addr

vim +14 arch/arm64/mm/ioremap.c

     6	
     7	void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
     8	{
     9		unsigned long last_addr = phys_addr + size - 1;
    10		int ret = -EINVAL;
    11	
    12		/* Don't allow outside PHYS_MASK */
    13		if (last_addr & ~PHYS_MASK)
  > 14			return ERR_PTR(ret);
    15	
    16		/* Don't allow RAM to be mapped. */
    17		if (WARN_ON(pfn_is_map_memory(__phys_to_pfn(phys_addr))))
    18			return ERR_PTR(ret);
    19	
    20		return NULL;
    21	}
    22	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH v2 resend 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[Kefeng Wang]

Add hook for arm64's special operation when ioremap() and iounmap(),
then ioremap_wc/np/cache is converted to use ioremap_prot()
from GENERIC_IOREMAP, update the Copyright and kill the unused
inclusions.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
v2 resend:
- use IOMEM_ERR_PTR to fix sparse warning found by lkp

 arch/arm64/Kconfig          |  1 +
 arch/arm64/include/asm/io.h | 20 ++++++---
 arch/arm64/kernel/acpi.c    |  2 +-
 arch/arm64/mm/ioremap.c     | 85 +++++--------------------------------
 4 files changed, 27 insertions(+), 81 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 20ea89d9ac2f..56673209fdb9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -123,6 +123,7 @@ config ARM64
 	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_IDLE_POLL_SETUP
+	select GENERIC_IOREMAP
 	select GENERIC_IRQ_IPI
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_SHOW
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 7fd836bea7eb..042fa01940b8 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -163,13 +163,21 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
 /*
  * I/O memory mapping functions.
  */
-extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
-extern void iounmap(volatile void __iomem *addr);
-extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
 
-#define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
-#define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
-#define ioremap_np(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
+void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot);
+#define arch_ioremap arch_ioremap
+
+int arch_iounmap(void __iomem *addr);
+#define arch_iounmap arch_iounmap
+
+#define _PAGE_IOREMAP PROT_DEVICE_nGnRE
+
+#define ioremap_wc(addr, size)		ioremap_prot((addr), (size), PROT_NORMAL_NC)
+#define ioremap_np(addr, size)		ioremap_prot((addr), (size), PROT_DEVICE_nGnRnE)
+#define ioremap_cache(addr, size) ({							\
+	pfn_is_map_memory(__phys_to_pfn(addr)) ?					\
+	(void __iomem *)__phys_to_virt(addr) : ioremap_prot(addr, size, PROT_NORMAL);	\
+})
 
 /*
  * io{read,write}{16,32,64}be() macros
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e4dea8db6924..a5a256e3f9fe 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -351,7 +351,7 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
 				prot = __acpi_get_writethrough_mem_attribute();
 		}
 	}
-	return __ioremap(phys, size, prot);
+	return ioremap_prot(phys, size, pgprot_val(prot));
 }
 
 /*
diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
index b7c81dacabf0..08fc30eb2721 100644
--- a/arch/arm64/mm/ioremap.c
+++ b/arch/arm64/mm/ioremap.c
@@ -1,96 +1,33 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * Based on arch/arm/mm/ioremap.c
- *
- * (C) Copyright 1995 1996 Linus Torvalds
- * Hacked for ARM by Phil Blundell <philb@gnu.org>
- * Hacked to allow all architectures to build, and various cleanups
- * by Russell King
- * Copyright (C) 2012 ARM Ltd.
- */
 
-#include <linux/export.h>
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 
-#include <asm/fixmap.h>
-#include <asm/tlbflush.h>
-
-static void __iomem *__ioremap_caller(phys_addr_t phys_addr, size_t size,
-				      pgprot_t prot, void *caller)
+void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
 {
-	unsigned long last_addr;
-	unsigned long offset = phys_addr & ~PAGE_MASK;
-	int err;
-	unsigned long addr;
-	struct vm_struct *area;
-
-	/*
-	 * Page align the mapping address and size, taking account of any
-	 * offset.
-	 */
-	phys_addr &= PAGE_MASK;
-	size = PAGE_ALIGN(size + offset);
+	unsigned long last_addr = phys_addr + size - 1;
+	int ret = -EINVAL;
 
-	/*
-	 * Don't allow wraparound, zero size or outside PHYS_MASK.
-	 */
-	last_addr = phys_addr + size - 1;
-	if (!size || last_addr < phys_addr || (last_addr & ~PHYS_MASK))
-		return NULL;
+	/* Don't allow outside PHYS_MASK */
+	if (last_addr & ~PHYS_MASK)
+		return IOMEM_ERR_PTR(ret);
 
-	/*
-	 * Don't allow RAM to be mapped.
-	 */
+	/* Don't allow RAM to be mapped. */
 	if (WARN_ON(pfn_is_map_memory(__phys_to_pfn(phys_addr))))
-		return NULL;
+		return IOMEM_ERR_PTR(ret);
 
-	area = get_vm_area_caller(size, VM_IOREMAP, caller);
-	if (!area)
-		return NULL;
-	addr = (unsigned long)area->addr;
-	area->phys_addr = phys_addr;
-
-	err = ioremap_page_range(addr, addr + size, phys_addr, prot);
-	if (err) {
-		vunmap((void *)addr);
-		return NULL;
-	}
-
-	return (void __iomem *)(offset + addr);
-}
-
-void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot)
-{
-	return __ioremap_caller(phys_addr, size, prot,
-				__builtin_return_address(0));
+	return NULL;
 }
-EXPORT_SYMBOL(__ioremap);
 
-void iounmap(volatile void __iomem *io_addr)
+int arch_iounmap(void __iomem *addr)
 {
-	unsigned long addr = (unsigned long)io_addr & PAGE_MASK;
-
 	/*
 	 * We could get an address outside vmalloc range in case
 	 * of ioremap_cache() reusing a RAM mapping.
 	 */
-	if (is_vmalloc_addr((void *)addr))
-		vunmap((void *)addr);
-}
-EXPORT_SYMBOL(iounmap);
-
-void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
-{
-	/* For normal memory we already have a cacheable mapping. */
-	if (pfn_is_map_memory(__phys_to_pfn(phys_addr)))
-		return (void __iomem *)__phys_to_virt(phys_addr);
-
-	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL),
-				__builtin_return_address(0));
+	return is_vmalloc_addr(addr) ? 0 : -EINVAL;
 }
-EXPORT_SYMBOL(ioremap_cache);
 
 /*
  * Must be called after early_fixmap_init
-- 
2.35.3

Re: [PATCH v2 1/5] mm: ioremap: Use more sensibly name in ioremap_prot()

[Anshuman Khandual]

On 4/29/22 16:02, Kefeng Wang wrote:
> Use more meaningful and sensibly naming phys_addr
> instead addr in ioremap_prot().
> 
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  include/asm-generic/io.h |  2 +-
>  mm/ioremap.c             | 12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index 7ce93aaf69f8..e6ffa2519f08 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -964,7 +964,7 @@ 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..1f9597fbcc07 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -11,20 +11,20 @@
>  #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 +33,7 @@ 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;
>  	}

Re: [PATCH v2 2/5] mm: ioremap: Setup phys_addr of struct vm_struct

[Anshuman Khandual]

On 4/29/22 16:02, Kefeng Wang wrote:
> Show physical address of each ioremap in /proc/vmallocinfo.
> 
> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  mm/ioremap.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/ioremap.c b/mm/ioremap.c
> index 1f9597fbcc07..7cb9996b0c12 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -32,6 +32,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
>  	if (!area)
>  		return NULL;
>  	vaddr = (unsigned long)area->addr;
> +	area->phys_addr = phys_addr;
>  
>  	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, __pgprot(prot))) {
>  		free_vm_area(area);

Re: [PATCH v2 0/5] arm64: Cleanup ioremap() and support ioremap_prot()

[Kefeng Wang]

Hello maintainers，kindly ping.

On 2022/4/29 18:32, 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.
>
> v2:
> - s/addr/phys_addr in ioremap_prot, suggested by Andrew Morton
> - rename arch_ioremap/iounmap_check to arch_ioremap/iounmad
>    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 (5):
>    mm: ioremap: Use more sensibly name in ioremap_prot()
>    mm: ioremap: Setup phys_addr of struct vm_struct
>    mm: ioremap: Add arch_ioremap/iounmap()
>    arm64: mm: Convert to GENERIC_IOREMAP
>    arm64: Add HAVE_IOREMAP_PROT support
>
>   .../features/vm/ioremap_prot/arch-support.txt |  2 +-
>   arch/arm64/Kconfig                            |  2 +
>   arch/arm64/include/asm/io.h                   | 20 +++--
>   arch/arm64/include/asm/pgtable.h              | 10 +++
>   arch/arm64/kernel/acpi.c                      |  2 +-
>   arch/arm64/mm/hugetlbpage.c                   | 10 ---
>   arch/arm64/mm/ioremap.c                       | 85 +++----------------
>   include/asm-generic/io.h                      | 16 +++-
>   mm/ioremap.c                                  | 27 ++++--
>   9 files changed, 74 insertions(+), 100 deletions(-)
>

Re: [PATCH v2 resend 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[Catalin Marinas]

On Mon, May 02, 2022 at 11:27:51AM +0800, Kefeng Wang wrote:
> Add hook for arm64's special operation when ioremap() and iounmap(),
> then ioremap_wc/np/cache is converted to use ioremap_prot()
> from GENERIC_IOREMAP, update the Copyright and kill the unused
> inclusions.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v2 5/5] arm64: Add HAVE_IOREMAP_PROT support

[Catalin Marinas]

On Fri, Apr 29, 2022 at 06:32:25PM +0800, Kefeng Wang wrote:
> With ioremap_prot() defination from generic ioremap, also move
> pte_pgprot() from hugetlbpage.c into pgtable.h, then arm64 could
> have HAVE_IOREMAP_PROT, which will enable generic_access_phys()
> code.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v2 0/5] arm64: Cleanup ioremap() and support ioremap_prot()

[Catalin Marinas]

On Fri, Apr 29, 2022 at 06:32:20PM +0800, Kefeng Wang wrote:
> Kefeng Wang (5):
>   mm: ioremap: Use more sensibly name in ioremap_prot()
>   mm: ioremap: Setup phys_addr of struct vm_struct
>   mm: ioremap: Add arch_ioremap/iounmap()
>   arm64: mm: Convert to GENERIC_IOREMAP
>   arm64: Add HAVE_IOREMAP_PROT support
> 
>  .../features/vm/ioremap_prot/arch-support.txt |  2 +-
>  arch/arm64/Kconfig                            |  2 +
>  arch/arm64/include/asm/io.h                   | 20 +++--
>  arch/arm64/include/asm/pgtable.h              | 10 +++
>  arch/arm64/kernel/acpi.c                      |  2 +-
>  arch/arm64/mm/hugetlbpage.c                   | 10 ---
>  arch/arm64/mm/ioremap.c                       | 85 +++----------------
>  include/asm-generic/io.h                      | 16 +++-
>  mm/ioremap.c                                  | 27 ++++--
>  9 files changed, 74 insertions(+), 100 deletions(-)

These patches touch the generic mm parts. Andrew, would you like to
merge these patches or are happy for them to go via the arm64 tree.

Thanks.

-- 
Catalin

Re: [PATCH v2 0/5] arm64: Cleanup ioremap() and support ioremap_prot()

[Kefeng Wang]

On 2022/5/17 6:51, Catalin Marinas wrote:
> On Fri, Apr 29, 2022 at 06:32:20PM +0800, Kefeng Wang wrote:
>> Kefeng Wang (5):
>>    mm: ioremap: Use more sensibly name in ioremap_prot()
>>    mm: ioremap: Setup phys_addr of struct vm_struct
>>    mm: ioremap: Add arch_ioremap/iounmap()
>>    arm64: mm: Convert to GENERIC_IOREMAP
>>    arm64: Add HAVE_IOREMAP_PROT support
>>
>>   .../features/vm/ioremap_prot/arch-support.txt |  2 +-
>>   arch/arm64/Kconfig                            |  2 +
>>   arch/arm64/include/asm/io.h                   | 20 +++--
>>   arch/arm64/include/asm/pgtable.h              | 10 +++
>>   arch/arm64/kernel/acpi.c                      |  2 +-
>>   arch/arm64/mm/hugetlbpage.c                   | 10 ---
>>   arch/arm64/mm/ioremap.c                       | 85 +++----------------
>>   include/asm-generic/io.h                      | 16 +++-
>>   mm/ioremap.c                                  | 27 ++++--
>>   9 files changed, 74 insertions(+), 100 deletions(-)
> These patches touch the generic mm parts. Andrew, would you like to
> merge these patches or are happy for them to go via the arm64 tree.
Hi Andrew, what's your preference ;)
> Thanks.
>

Re: [PATCH v2 3/5] mm: ioremap: Add arch_ioremap/iounmap()

[Anshuman Khandual]

Hi Kefeng,

On 4/29/22 16:02, Kefeng Wang wrote:
> Add special hook for architecture to verify addr, size and prot
> or setup when ioremap() or iounmap(), which will make the generic
> ioremap more useful.
> 
>   arch_ioremap() return a 'void __iomem *',
>     - IS_ERR means return an error
>     - NULL means continue to remap
>     - a non-NULL, non-IS_ERR pointer is directly returned
>   arch_iounmap() return a int value,
>     - 0 means continue to vunmap
>     - error code means skip vunmap and return directly

Should not these comments be also included as in-code documentation, possibly
near generic fall back stubs for arch_ioremap()/arch_iounmap() in the header
include/asm-generic/io.h ?

> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  include/asm-generic/io.h | 14 ++++++++++++++
>  mm/ioremap.c             | 14 +++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index e6ffa2519f08..f2f9aeedb5e8 100644
> --- a/include/asm-generic/io.h
> +++ b/arch_iounmap
> @@ -964,6 +964,20 @@ static inline void iounmap(volatile void __iomem *addr)
>  #elif defined(CONFIG_GENERIC_IOREMAP)
>  #include <linux/pgtable.h>
>  
> +#ifndef arch_ioremap
> +static inline void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +#ifndef arch_iounmap
> +static inline int arch_iounmap(void __iomem *addr)
> +{
> +	return 0;
> +}
> +#endif

There is a function in arch/arm/ with exact same name although the platform does
not enable GENERIC_IOREMAP. That function would require renaming for these arch
callbacks to be added here in GENERIC_IOREMAP path. Otherwise, it might be just
confusing later.

git grep "arch_iounmap" arch/arm/

arch/arm/include/asm/io.h:extern void (*arch_iounmap)(volatile void __iomem *);
arch/arm/mm/ioremap.c:void (*arch_iounmap)(volatile void __iomem *) = __iounmap;
arch/arm/mm/ioremap.c:  arch_iounmap(cookie);
arch/arm/mm/nommu.c:void (*arch_iounmap)(volatile void __iomem *);

> +
>  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 7cb9996b0c12..de5a2e899e14 100644
> --- a/mm/ioremap.c
> +++ b/mm/ioremap.c
> @@ -16,6 +16,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
>  	unsigned long offset, vaddr;
>  	phys_addr_t last_addr;
>  	struct vm_struct *area;
> +	void __iomem *base;
>  
>  	/* Disallow wrap-around or zero size */
>  	last_addr = phys_addr + size - 1;
> @@ -27,6 +28,12 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
>  	phys_addr -= offset;
>  	size = PAGE_ALIGN(size + offset);
>  
> +	base = arch_ioremap(phys_addr, size, prot);
> +	if (IS_ERR(base))
> +		return NULL;
> +	else if (base)
> +		return base;
> +
>  	area = get_vm_area_caller(size, VM_IOREMAP,
>  			__builtin_return_address(0));
>  	if (!area)
> @@ -45,6 +52,11 @@ 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);

Should not this variable be 'void __iomem *vaddr' instead, like above in
ioremap_prot(). Because arch_iounmap() takes 'void __iomem *' instead.

> +
> +	if (arch_iounmap(vaddr))
> +		return;
> +
> +	vunmap(vaddr);
>  }
>  EXPORT_SYMBOL(iounmap);
- Anshuman

Re: [PATCH v2 5/5] arm64: Add HAVE_IOREMAP_PROT support

[Anshuman Khandual]

On 4/29/22 16:02, Kefeng Wang wrote:
> With ioremap_prot() defination from generic ioremap, also move

s/defination/definition  ^^^^

> pte_pgprot() from hugetlbpage.c into pgtable.h, then arm64 could
> have HAVE_IOREMAP_PROT, which will enable generic_access_phys()
> code.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  .../features/vm/ioremap_prot/arch-support.txt          |  2 +-
>  arch/arm64/Kconfig                                     |  1 +
>  arch/arm64/include/asm/pgtable.h                       | 10 ++++++++++
>  arch/arm64/mm/hugetlbpage.c                            | 10 ----------
>  4 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/features/vm/ioremap_prot/arch-support.txt b/Documentation/features/vm/ioremap_prot/arch-support.txt
> index a6dcbe5f47b6..b39ad5d61216 100644
> --- a/Documentation/features/vm/ioremap_prot/arch-support.txt
> +++ b/Documentation/features/vm/ioremap_prot/arch-support.txt
> @@ -9,7 +9,7 @@
>      |       alpha: | TODO |
>      |         arc: |  ok  |
>      |         arm: | TODO |
> -    |       arm64: | TODO |
> +    |       arm64: |  ok  |
>      |        csky: | TODO |
>      |       h8300: | TODO |
>      |     hexagon: | TODO |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 56673209fdb9..5e5889049af0 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -186,6 +186,7 @@ config ARM64
>  	select HAVE_FUNCTION_GRAPH_TRACER
>  	select HAVE_GCC_PLUGINS
>  	select HAVE_HW_BREAKPOINT if PERF_EVENTS
> +	select HAVE_IOREMAP_PROT
>  	select HAVE_IRQ_TIME_ACCOUNTING
>  	select HAVE_KVM
>  	select HAVE_NMI
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index dff2b483ea50..1402a2739024 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -402,6 +402,16 @@ static inline pgprot_t mk_pmd_sect_prot(pgprot_t prot)
>  	return __pgprot((pgprot_val(prot) & ~PMD_TABLE_BIT) | PMD_TYPE_SECT);
>  }
>  
> +/*
> + * Select all bits except the pfn
> + */
> +static inline pgprot_t pte_pgprot(pte_t pte)
> +{
> +	unsigned long pfn = pte_pfn(pte);
> +
> +	return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
> +}
> +
>  #ifdef CONFIG_NUMA_BALANCING
>  /*
>   * See the comment in include/linux/pgtable.h
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index cbace1c9e137..38d03406f6aa 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -100,16 +100,6 @@ int pud_huge(pud_t pud)
>  #endif
>  }
>  
> -/*
> - * Select all bits except the pfn
> - */
> -static inline pgprot_t pte_pgprot(pte_t pte)
> -{
> -	unsigned long pfn = pte_pfn(pte);
> -
> -	return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
> -}
> -
>  static int find_num_contig(struct mm_struct *mm, unsigned long addr,
>  			   pte_t *ptep, size_t *pgsize)
>  {

Re: [PATCH v2 resend 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[Anshuman Khandual]

On 5/2/22 08:57, Kefeng Wang wrote:
> Add hook for arm64's special operation when ioremap() and iounmap(),
> then ioremap_wc/np/cache is converted to use ioremap_prot()
> from GENERIC_IOREMAP, update the Copyright and kill the unused
> inclusions.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> v2 resend:
> - use IOMEM_ERR_PTR to fix sparse warning found by lkp
> 
>  arch/arm64/Kconfig          |  1 +
>  arch/arm64/include/asm/io.h | 20 ++++++---
>  arch/arm64/kernel/acpi.c    |  2 +-
>  arch/arm64/mm/ioremap.c     | 85 +++++--------------------------------
>  4 files changed, 27 insertions(+), 81 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 20ea89d9ac2f..56673209fdb9 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -123,6 +123,7 @@ config ARM64
>  	select GENERIC_CPU_VULNERABILITIES
>  	select GENERIC_EARLY_IOREMAP
>  	select GENERIC_IDLE_POLL_SETUP
> +	select GENERIC_IOREMAP
>  	select GENERIC_IRQ_IPI
>  	select GENERIC_IRQ_PROBE
>  	select GENERIC_IRQ_SHOW
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 7fd836bea7eb..042fa01940b8 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -163,13 +163,21 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
>  /*
>   * I/O memory mapping functions.
>   */
> -extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
> -extern void iounmap(volatile void __iomem *addr);
> -extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
>  
> -#define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
> -#define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
> -#define ioremap_np(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
> +void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot);
> +#define arch_ioremap arch_ioremap
> +
> +int arch_iounmap(void __iomem *addr);
> +#define arch_iounmap arch_iounmap
> +
> +#define _PAGE_IOREMAP PROT_DEVICE_nGnRE

Small nit, should we have a comment here for the above components i.e
PAGE_IOREMAP and callbacks arch_ioremap()/arch_iounmap() are required
because of enabling GENERIC_IOREMAP ?

> +
> +#define ioremap_wc(addr, size)		ioremap_prot((addr), (size), PROT_NORMAL_NC)
> +#define ioremap_np(addr, size)		ioremap_prot((addr), (size), PROT_DEVICE_nGnRnE)
> +#define ioremap_cache(addr, size) ({							\
> +	pfn_is_map_memory(__phys_to_pfn(addr)) ?					\
> +	(void __iomem *)__phys_to_virt(addr) : ioremap_prot(addr, size, PROT_NORMAL);	\
> +})
>  
>  /*
>   * io{read,write}{16,32,64}be() macros
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index e4dea8db6924..a5a256e3f9fe 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -351,7 +351,7 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
>  				prot = __acpi_get_writethrough_mem_attribute();
>  		}
>  	}
> -	return __ioremap(phys, size, prot);
> +	return ioremap_prot(phys, size, pgprot_val(prot));
>  }
>  
>  /*
> diff --git a/arch/arm64/mm/ioremap.c b/arch/arm64/mm/ioremap.c
> index b7c81dacabf0..08fc30eb2721 100644
> --- a/arch/arm64/mm/ioremap.c
> +++ b/arch/arm64/mm/ioremap.c
> @@ -1,96 +1,33 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Based on arch/arm/mm/ioremap.c
> - *
> - * (C) Copyright 1995 1996 Linus Torvalds
> - * Hacked for ARM by Phil Blundell <philb@gnu.org>
> - * Hacked to allow all architectures to build, and various cleanups
> - * by Russell King
> - * Copyright (C) 2012 ARM Ltd.
> - */
>  
> -#include <linux/export.h>
>  #include <linux/mm.h>
>  #include <linux/vmalloc.h>
>  #include <linux/io.h>
>  
> -#include <asm/fixmap.h>
> -#include <asm/tlbflush.h>
> -
> -static void __iomem *__ioremap_caller(phys_addr_t phys_addr, size_t size,
> -				      pgprot_t prot, void *caller)
> +void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
>  {
> -	unsigned long last_addr;
> -	unsigned long offset = phys_addr & ~PAGE_MASK;
> -	int err;
> -	unsigned long addr;
> -	struct vm_struct *area;
> -
> -	/*
> -	 * Page align the mapping address and size, taking account of any
> -	 * offset.
> -	 */
> -	phys_addr &= PAGE_MASK;
> -	size = PAGE_ALIGN(size + offset);
> +	unsigned long last_addr = phys_addr + size - 1;
> +	int ret = -EINVAL;
>  
> -	/*
> -	 * Don't allow wraparound, zero size or outside PHYS_MASK.
> -	 */
> -	last_addr = phys_addr + size - 1;
> -	if (!size || last_addr < phys_addr || (last_addr & ~PHYS_MASK))
> -		return NULL;
> +	/* Don't allow outside PHYS_MASK */
> +	if (last_addr & ~PHYS_MASK)
> +		return IOMEM_ERR_PTR(ret);
>  
> -	/*
> -	 * Don't allow RAM to be mapped.
> -	 */
> +	/* Don't allow RAM to be mapped. */
>  	if (WARN_ON(pfn_is_map_memory(__phys_to_pfn(phys_addr))))
> -		return NULL;
> +		return IOMEM_ERR_PTR(ret);
>  
> -	area = get_vm_area_caller(size, VM_IOREMAP, caller);
> -	if (!area)
> -		return NULL;
> -	addr = (unsigned long)area->addr;
> -	area->phys_addr = phys_addr;
> -
> -	err = ioremap_page_range(addr, addr + size, phys_addr, prot);
> -	if (err) {
> -		vunmap((void *)addr);
> -		return NULL;
> -	}
> -
> -	return (void __iomem *)(offset + addr);
> -}
> -
> -void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot)
> -{
> -	return __ioremap_caller(phys_addr, size, prot,
> -				__builtin_return_address(0));
> +	return NULL;
>  }
> -EXPORT_SYMBOL(__ioremap);
>  
> -void iounmap(volatile void __iomem *io_addr)
> +int arch_iounmap(void __iomem *addr)
>  {
> -	unsigned long addr = (unsigned long)io_addr & PAGE_MASK;
> -
>  	/*
>  	 * We could get an address outside vmalloc range in case
>  	 * of ioremap_cache() reusing a RAM mapping.
>  	 */
> -	if (is_vmalloc_addr((void *)addr))
> -		vunmap((void *)addr);
> -}
> -EXPORT_SYMBOL(iounmap);
> -
> -void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size)
> -{
> -	/* For normal memory we already have a cacheable mapping. */
> -	if (pfn_is_map_memory(__phys_to_pfn(phys_addr)))
> -		return (void __iomem *)__phys_to_virt(phys_addr);
> -
> -	return __ioremap_caller(phys_addr, size, __pgprot(PROT_NORMAL),
> -				__builtin_return_address(0));
> +	return is_vmalloc_addr(addr) ? 0 : -EINVAL;
>  }
> -EXPORT_SYMBOL(ioremap_cache);
>  
>  /*
>   * Must be called after early_fixmap_init

Otherwise LGTM.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH v2 3/5] mm: ioremap: Add arch_ioremap/iounmap()

[Kefeng Wang]

On 2022/5/19 12:46, Anshuman Khandual wrote:
> Hi Kefeng,
>
> On 4/29/22 16:02, Kefeng Wang wrote:
>> Add special hook for architecture to verify addr, size and prot
>> or setup when ioremap() or iounmap(), which will make the generic
>> ioremap more useful.
>>
>>    arch_ioremap() return a 'void __iomem *',
>>      - IS_ERR means return an error
>>      - NULL means continue to remap
>>      - a non-NULL, non-IS_ERR pointer is directly returned
>>    arch_iounmap() return a int value,
>>      - 0 means continue to vunmap
>>      - error code means skip vunmap and return directly
> Should not these comments be also included as in-code documentation, possibly
> near generic fall back stubs for arch_ioremap()/arch_iounmap() in the header
> include/asm-generic/io.h ?
Ok, I will add some document in io.h
>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   include/asm-generic/io.h | 14 ++++++++++++++
>>   mm/ioremap.c             | 14 +++++++++++++-
>>   2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> index e6ffa2519f08..f2f9aeedb5e8 100644
>> --- a/include/asm-generic/io.h
>> +++ b/arch_iounmap
>> @@ -964,6 +964,20 @@ static inline void iounmap(volatile void __iomem *addr)
>>   #elif defined(CONFIG_GENERIC_IOREMAP)
>>   #include <linux/pgtable.h>
>>   
>> +#ifndef arch_ioremap
>> +static inline void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
>> +{
>> +	return NULL;
>> +}
>> +#endif
>> +
>> +#ifndef arch_iounmap
>> +static inline int arch_iounmap(void __iomem *addr)
>> +{
>> +	return 0;
>> +}
>> +#endif
> There is a function in arch/arm/ with exact same name although the platform does
> not enable GENERIC_IOREMAP. That function would require renaming for these arch
> callbacks to be added here in GENERIC_IOREMAP path. Otherwise, it might be just
> confusing later.
>
> git grep "arch_iounmap" arch/arm/
>
> arch/arm/include/asm/io.h:extern void (*arch_iounmap)(volatile void __iomem *);
> arch/arm/mm/ioremap.c:void (*arch_iounmap)(volatile void __iomem *) = __iounmap;
> arch/arm/mm/ioremap.c:  arch_iounmap(cookie);
> arch/arm/mm/nommu.c:void (*arch_iounmap)(volatile void __iomem *);

After

   59d3ae9a5bf60 ("ARM: remove Intel iop33x and iop13xx support") v5.4
   3e3f354bc383a ("ARM: remove ebsa110 platform") v5.11

arch_iounmap is useless, we could directly kill arch_iounmap/__iounmap 
on arm,

will add new cleanup patch in v3.


>
>> +
>>   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 7cb9996b0c12..de5a2e899e14 100644
>> --- a/mm/ioremap.c
>> +++ b/mm/ioremap.c
>> @@ -16,6 +16,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
>>   	unsigned long offset, vaddr;
>>   	phys_addr_t last_addr;
>>   	struct vm_struct *area;
>> +	void __iomem *base;
>>   
>>   	/* Disallow wrap-around or zero size */
>>   	last_addr = phys_addr + size - 1;
>> @@ -27,6 +28,12 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
>>   	phys_addr -= offset;
>>   	size = PAGE_ALIGN(size + offset);
>>   
>> +	base = arch_ioremap(phys_addr, size, prot);
>> +	if (IS_ERR(base))
>> +		return NULL;
>> +	else if (base)
>> +		return base;
>> +
>>   	area = get_vm_area_caller(size, VM_IOREMAP,
>>   			__builtin_return_address(0));
>>   	if (!area)
>> @@ -45,6 +52,11 @@ 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);
> Should not this variable be 'void __iomem *vaddr' instead, like above in
> ioremap_prot(). Because arch_iounmap() takes 'void __iomem *' instead.

Will do, thanks.

>> +
>> +	if (arch_iounmap(vaddr))
>> +		return;
>> +
>> +	vunmap(vaddr);
>>   }
>>   EXPORT_SYMBOL(iounmap);
> - Anshuman
> .

Re: [PATCH v2 resend 4/5] arm64: mm: Convert to GENERIC_IOREMAP

[Kefeng Wang]

On 2022/5/19 13:34, Anshuman Khandual wrote:
>
> On 5/2/22 08:57, Kefeng Wang wrote:
>> Add hook for arm64's special operation when ioremap() and iounmap(),
>> then ioremap_wc/np/cache is converted to use ioremap_prot()
>> from GENERIC_IOREMAP, update the Copyright and kill the unused
>> inclusions.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> v2 resend:
>> - use IOMEM_ERR_PTR to fix sparse warning found by lkp
>>
>>   arch/arm64/Kconfig          |  1 +
>>   arch/arm64/include/asm/io.h | 20 ++++++---
>>   arch/arm64/kernel/acpi.c    |  2 +-
>>   arch/arm64/mm/ioremap.c     | 85 +++++--------------------------------
>>   4 files changed, 27 insertions(+), 81 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 20ea89d9ac2f..56673209fdb9 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -123,6 +123,7 @@ config ARM64
>>   	select GENERIC_CPU_VULNERABILITIES
>>   	select GENERIC_EARLY_IOREMAP
>>   	select GENERIC_IDLE_POLL_SETUP
>> +	select GENERIC_IOREMAP
>>   	select GENERIC_IRQ_IPI
>>   	select GENERIC_IRQ_PROBE
>>   	select GENERIC_IRQ_SHOW
>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>> index 7fd836bea7eb..042fa01940b8 100644
>> --- a/arch/arm64/include/asm/io.h
>> +++ b/arch/arm64/include/asm/io.h
>> @@ -163,13 +163,21 @@ extern void __memset_io(volatile void __iomem *, int, size_t);
>>   /*
>>    * I/O memory mapping functions.
>>    */
>> -extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot);
>> -extern void iounmap(volatile void __iomem *addr);
>> -extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
>>   
>> -#define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
>> -#define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
>> -#define ioremap_np(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRnE))
>> +void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot);
>> +#define arch_ioremap arch_ioremap
>> +
>> +int arch_iounmap(void __iomem *addr);
>> +#define arch_iounmap arch_iounmap
>> +
>> +#define _PAGE_IOREMAP PROT_DEVICE_nGnRE
> Small nit, should we have a comment here for the above components i.e
> PAGE_IOREMAP and callbacks arch_ioremap()/arch_iounmap() are required
> because of enabling GENERIC_IOREMAP ?

There is a comment in  include/asm-generic/io.h:       /* _PAGE_IOREMAP 
needs to be supplied by the architecture */

so arch's callbacks could not add repeated comments.


> Otherwise LGTM.
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Thanks.
> .