[permalink] [raw]

Subject: [PATCH v3 08/11] parisc: mm: Convert to GENERIC_IOREMAP

By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
iounmap() are visible and available to arch. Arch only needs to
provide implementation of arch_ioremap() or arch_iounmap() if there's
arch specific handling needed in its ioremap() or iounmap(). This
change will simplify implementation by removing duplicated codes with
generic ioremap() and iounmap(), and has the equivalent functioality
as before.

For parisc, add hook arch_ioremap() for parisc's special operation when
ioremap(), then ioremap_[wc|uc]() are converted to use ioremap_prot()
from GENERIC_IOREMAP.

Meanwhile, add macro ARCH_HAS_IOREMAP_WC since the added ioremap_wc()
will conflict with the one in include/asm-generic/iomap.h, then an
compiling error is seen:

./include/asm-generic/iomap.h:97: warning: "ioremap_wc" redefined
97 | #define ioremap_wc ioremap

And benefit from the commit 437b6b35362b ("parisc: Use the generic
IO helpers"), those macros don't need be added any more.

Signed-off-by: Baoquan He <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: [email protected]
---
v2->v3:
- Fix compiling error by adding macro definition, ARCH_HAS_IOREMAP_WC.
- Benefit from commit 437b6b35362b ("parisc: Use the generic IO helpers"),
those tons of port/mem io operation macro definitions are not needed
after rebasing to the latest upstream.

arch/parisc/Kconfig | 1 +
arch/parisc/include/asm/io.h | 19 ++++++++---
arch/parisc/mm/ioremap.c | 65 ++++--------------------------------
3 files changed, 21 insertions(+), 64 deletions(-)

diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index a98940e64243..0ed18e673aba 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -36,6 +36,7 @@ config PARISC
select GENERIC_ATOMIC64 if !64BIT
select GENERIC_IRQ_PROBE
select GENERIC_PCI_IOMAP
+ select GENERIC_IOREMAP
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ARCH_TOPOLOGY if SMP
diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index c05e781be2f5..1c54f83d4f78 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -2,6 +2,8 @@
#ifndef _ASM_IO_H
#define _ASM_IO_H

+#define ARCH_HAS_IOREMAP_WC
+
#include <linux/types.h>
#include <linux/pgtable.h>

@@ -125,12 +127,19 @@ static inline void gsc_writeq(unsigned long long val, unsigned long addr)
/*
* The standard PCI ioremap interfaces
*/
-void __iomem *ioremap(unsigned long offset, unsigned long size);
-#define ioremap_wc ioremap
-#define ioremap_uc ioremap
-#define pci_iounmap pci_iounmap
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val);
+#define arch_ioremap arch_ioremap
+
+#define _PAGE_IOREMAP (_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | \
+ _PAGE_ACCESSED | _PAGE_NO_CACHE)

-extern void iounmap(const volatile void __iomem *addr);
+#define ioremap_wc(addr, size) \
+ ioremap_prot((addr), (size), _PAGE_IOREMAP)
+#define ioremap_uc(addr, size) \
+ ioremap_prot((addr), (size), _PAGE_IOREMAP)
+
+#define pci_iounmap pci_iounmap

void memset_io(volatile void __iomem *addr, unsigned char val, int count);
void memcpy_fromio(void *dst, const volatile void __iomem *src, int count);
diff --git a/arch/parisc/mm/ioremap.c b/arch/parisc/mm/ioremap.c
index 345ff0b66499..28884757fad0 100644
--- a/arch/parisc/mm/ioremap.c
+++ b/arch/parisc/mm/ioremap.c
@@ -13,38 +13,19 @@
#include <linux/io.h>
#include <linux/mm.h>

-/*
- * Generic mapping function (not visible outside):
- */
-
-/*
- * Remap an arbitrary physical address space into the kernel virtual
- * address space.
- *
- * NOTE! We need to allow non-page-aligned mappings too: we will obviously
- * have to convert them into an offset in a page-aligned mapping, but the
- * caller shouldn't need to know that small detail.
- */
-void __iomem *ioremap(unsigned long phys_addr, unsigned long size)
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
{
- void __iomem *addr;
- struct vm_struct *area;
- unsigned long offset, last_addr;
- pgprot_t pgprot;
+ phys_addr_t phys_addr = *paddr;

#ifdef CONFIG_EISA
unsigned long end = phys_addr + size - 1;
/* Support EISA addresses */
if ((phys_addr >= 0x00080000 && end < 0x000fffff) ||
(phys_addr >= 0x00500000 && end < 0x03bfffff))
- phys_addr |= F_EXTEND(0xfc000000);
+ *paddr = phys_addr |= F_EXTEND(0xfc000000);
#endif

- /* Don't allow wraparound or zero size */
- last_addr = phys_addr + size - 1;
- if (!size || last_addr < phys_addr)
- return NULL;
-
/*
* Don't allow anybody to remap normal RAM that we're using..
*/
@@ -58,43 +39,9 @@ void __iomem *ioremap(unsigned long phys_addr, unsigned long size)
for (page = virt_to_page(t_addr);
page <= virt_to_page(t_end); page++) {
if(!PageReserved(page))
- return NULL;
+ return IOMEM_ERR_PTR(-EINVAL);
}
}

- pgprot = __pgprot(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY |
- _PAGE_ACCESSED | _PAGE_NO_CACHE);
-
- /*
- * Mappings have to be page-aligned
- */
- offset = phys_addr & ~PAGE_MASK;
- phys_addr &= PAGE_MASK;
- size = PAGE_ALIGN(last_addr + 1) - phys_addr;
-
- /*
- * Ok, go for it..
- */
- area = get_vm_area(size, VM_IOREMAP);
- if (!area)
- return NULL;
-
- addr = (void __iomem *) area->addr;
- if (ioremap_page_range((unsigned long)addr, (unsigned long)addr + size,
- phys_addr, pgprot)) {
- vunmap(addr);
- return NULL;
- }
-
- return (void __iomem *) (offset + (char __iomem *)addr);
-}
-EXPORT_SYMBOL(ioremap);
-
-void iounmap(const volatile void __iomem *io_addr)
-{
- unsigned long addr = (unsigned long)io_addr & PAGE_MASK;
-
- if (is_vmalloc_addr((void *)addr))
- vunmap((void *)addr);
+ return NULL;
}
-EXPORT_SYMBOL(iounmap);
--
2.34.1

2022-10-09 11:44:56

by Baoquan He

[permalink] [raw]

Subject: [PATCH v3 04/11] mm: ioremap: allow ARCH to have its own ioremap definition

Architectures like xtensa, arc, can be converted to GENERIC_IOREMAP,
to take standard ioremap_prot() and ioremap_xxx() way. But they have
ARCH specific handling for ioremap() method, than standard ioremap()
method.

In oder to convert them to take GENERIC_IOREMAP method, allow these
architecutres to have their own ioremap definition.

This is a preparation patch, no functionality change.

Signed-off-by: Baoquan He <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: Kefeng Wang <[email protected]>
---
include/asm-generic/io.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 2ae16906f3be..8878914579d8 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -1078,11 +1078,14 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size,
unsigned long prot);
void iounmap(volatile void __iomem *addr);

+#ifndef ioremap
+#define ioremap ioremap
static inline void __iomem *ioremap(phys_addr_t addr, size_t size)
{
/* _PAGE_IOREMAP needs to be supplied by the architecture */
return ioremap_prot(addr, size, _PAGE_IOREMAP);
}
+#endif
#endif /* !CONFIG_MMU || CONFIG_GENERIC_IOREMAP */

#ifndef ioremap_wc
--
2.34.1

2022-10-09 11:46:32

by Baoquan He

[permalink] [raw]

Subject: [PATCH v3 06/11] ia64: mm: Convert to GENERIC_IOREMAP

By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
iounmap() are visible and available to arch. Arch only needs to
provide implementation of arch_ioremap() or arch_iounmap() if there's
arch specific handling needed in its ioremap() or iounmap(). This
change will simplify implementation by removing duplicated codes with
generic ioremap() and iounmap(), and has the equivalent functioality
as before.

Here add hooks arch_ioremap() and arch_iounmap() for ia64's special
operation when ioremap() and iounmap(), then ioremap_cache() is
converted to use ioremap_prot() from GENERIC_IOREMAP.

The old ioremap_uc() is kept and add its macro definittion.

Signed-off-by: Baoquan He <[email protected]>
Cc: [email protected]
---
arch/ia64/Kconfig | 1 +
arch/ia64/include/asm/io.h | 26 ++++++++++++--------
arch/ia64/mm/ioremap.c | 50 +++++++++-----------------------------
3 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 26ac8ea15a9e..1ca18be5dc30 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -45,6 +45,7 @@ config IA64
select GENERIC_IRQ_LEGACY
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select GENERIC_IOMAP
+ select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select ARCH_TASK_STRUCT_ON_STACK
select ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index ce66dfc0e719..54378cea4b36 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -247,17 +247,23 @@ static inline void outsl(unsigned long port, const void *src,

# ifdef __KERNEL__

-extern void __iomem * ioremap(unsigned long offset, unsigned long size);
-extern void __iomem * ioremap_uc(unsigned long offset, unsigned long size);
-extern void iounmap (volatile void __iomem *addr);
-static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size)
-{
- return ioremap(phys_addr, size);
-}
-#define ioremap ioremap
-#define ioremap_cache ioremap_cache
+/*
+ * I/O memory mapping functions.
+ */
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val);
+#define arch_ioremap arch_ioremap
+
+bool arch_iounmap(void __iomem *addr);
+#define arch_iounmap arch_iounmap
+
+#define _PAGE_IOREMAP pgprot_val(PAGE_KERNEL)
+
+#define ioremap_cache(addr, size) \
+ ioremap_prot((addr), (size), pgprot_val(PAGE_KERNEL))
+
+void __iomem *ioremap_uc(unsigned long offset, unsigned long size);
#define ioremap_uc ioremap_uc
-#define iounmap iounmap

/*
* String version of IO memory access ops:
diff --git a/arch/ia64/mm/ioremap.c b/arch/ia64/mm/ioremap.c
index 55fd3eb753ff..a290c7dc7f1e 100644
--- a/arch/ia64/mm/ioremap.c
+++ b/arch/ia64/mm/ioremap.c
@@ -30,15 +30,12 @@ early_ioremap (unsigned long phys_addr, unsigned long size)
}

void __iomem *
-ioremap (unsigned long phys_addr, unsigned long size)
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
{
- void __iomem *addr;
- struct vm_struct *area;
- unsigned long offset;
- pgprot_t prot;
- u64 attr;
+ phys_addr_t phys_addr = *paddr;
unsigned long gran_base, gran_size;
unsigned long page_base;
+ u64 attr;

/*
* For things in kern_memmap, we must use the same attribute
@@ -69,35 +66,18 @@ ioremap (unsigned long phys_addr, unsigned long size)
page_base = phys_addr & PAGE_MASK;
size = PAGE_ALIGN(phys_addr + size) - page_base;
if (efi_mem_attribute(page_base, size) & EFI_MEMORY_WB) {
- prot = PAGE_KERNEL;
-
- /*
- * Mappings have to be page-aligned
- */
- offset = phys_addr & ~PAGE_MASK;
- phys_addr &= PAGE_MASK;
-
- /*
- * Ok, go for it..
- */
- area = get_vm_area(size, VM_IOREMAP);
- if (!area)
- return NULL;
-
- area->phys_addr = phys_addr;
- addr = (void __iomem *) area->addr;
- if (ioremap_page_range((unsigned long) addr,
- (unsigned long) addr + size, phys_addr, prot)) {
- vunmap((void __force *) addr);
- return NULL;
- }
-
- return (void __iomem *) (offset + (char __iomem *)addr);
+ return NULL;
}

return __ioremap_uc(phys_addr);
}
-EXPORT_SYMBOL(ioremap);
+
+bool arch_iounmap(void __iomem *addr)
+{
+ if (REGION_NUMBER(addr) != RGN_GATE)
+ return false;
+ return true;
+}

void __iomem *
ioremap_uc(unsigned long phys_addr, unsigned long size)
@@ -113,11 +93,3 @@ void
early_iounmap (volatile void __iomem *addr, unsigned long size)
{
}
-
-void
-iounmap (volatile void __iomem *addr)
-{
- if (REGION_NUMBER(addr) == RGN_GATE)
- vunmap((void *) ((unsigned long) addr & PAGE_MASK));
-}
-EXPORT_SYMBOL(iounmap);
--
2.34.1

2022-10-09 14:40:12

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v3 09/11] s390: mm: Convert to GENERIC_IOREMAP

Attachments:

(No filename) (2.21 kB)
config (64.84 kB)
Download all attachments

2022-10-09 16:26:01

Hi,

Le 09/10/2022 à 12:31, Baoquan He a écrit :
> Currently, many architecutres have't taken the standard GENERIC_IOREMAP
> way to implement ioremap_prot(), iounmap(), and ioremap_xx(), but make
> these functions specifically under each arch's folder. Those cause many
> duplicated codes of ioremap() and iounmap().
>
> In this patchset, firstly adapt the hooks io[re|un]map_allowed, then
> make use of them to convert those ARCH-es to take GENERIC_IOREMAP method.
> With these change, duplicated ioremap/iounmap() code uder ARCH-es are
> removed.
>
> This is suggested by Christoph in below discussion:
> https://lore.kernel.org/all/[email protected]/T/#u
>
> And it's basically further action after arm64 has converted to
> GENERIC_IOREMAP way in below patchset.
> [PATCH v5 0/6] arm64: Cleanup ioremap() and support ioremap_prot()
> https://lore.kernel.org/all/[email protected]/T/#u
>

I'm still puzzled with your series, it seems more churn than needed, and
I still deeply dislike the approach where a function called
arch_ioremap() says "Today I don't feel like doing the job so I return
NULL and you'll do it for me"

In order to better illustrate what I have in mind as discussed
previously, I have prepared a short RFC series based on your v3, taking
into account the first two architectures (ARC and IA64) of your series,
and also adding POWERPC architecture. I will send it out shortly.

Christophe

> v2->v3:
> - Rewrite log of all patches to add more details as Christoph suggested.
>
> - Merge the old patch 1 and 2 which adjusts return values and
> parameters of arch_ioremap() into one patch, namely the current
> patch 3. Christoph suggested this.
>
> - Change the return value of arch_iounmap() to bool type since we only
> do arch specific address filtering or address checking, bool value
> can reflect the checking better. This is pointed out by Niklas when
> he reviewed the s390 patch.
>
> - Put hexagon patch at the beginning of patchset since hexagon has the
> same ioremap() and iounmap() as standard ones, no arch_ioremap() and
> arch_iounmap() hooks need be introduced. So the later arch_ioremap
> and arch_iounmap() adjustment are not related in hexagon. Christophe
> suggested this.
>
> - Remove the early ioremap code from openrisc ioremap() firstly since
> openrisc doesn't have early ioremap handling in openrisc arch code.
> This simplifies the later converting to GENERIC_IOREMAP method.
> Christoph and Stafford suggersted this.
>
> - Fix compiling erorrs reported by lkp in parisc and sh patches.
> Adding macro defintions for those port|mem io functions in
> <asm/io.h> to avoid repeated definition in <asm-generic/io.h>.
>
> v1->v2:
> - Rename io[re|un]map_allowed() to arch_io[re|un]map() and made
> some minor changes in patch 1~2 as per Alexander and Kefeng's
> suggestions. Accordingly, adjust patches~4~11 because of the renaming
> arch_io[re|un]map().
>
> Testing:
> - It's running well on arm64 system with the latest upstream kernel
> and this patchset applied.
> - Cross compiling passed on arc, ia64, parisc, s390, sh, xtensa.
> - cross compiling is not tried on hexagon and openrisc because
> - Didn't find cross compiling tools for hexagon;
> - there's error with openrisc compiling, while I have no idea how to
> fix it. Please see below pasted log:
> ---------------------------------------------------------------------
> [root@intel-knightslanding-lb-02 linux]# make ARCH=openrisc defconfig
> *** Default configuration is based on 'or1ksim_defconfig'
> #
> # configuration written to .config
> #
> [root@intel-knightslanding-lb-02 linux]# make ARCH=openrisc -j320 CROSS_COMPILE=/usr/bin/openrisc-linux-gnu-
> SYNC include/config/auto.conf.cmd
> CC scripts/mod/empty.o
> ./scripts/check-local-export: /usr/bin/openrisc-linux-gnu-nm failed
> make[1]: *** [scripts/Makefile.build:250: scripts/mod/empty.o] Error 1
> make[1]: *** Deleting file 'scripts/mod/empty.o'
> make: *** [Makefile:1275: prepare0] Error 2
> ----------------------------------------------------------------------
>
> Baoquan He (11):
> hexagon: mm: Convert to GENERIC_IOREMAP
> openrisc: mm: remove unneeded early ioremap code
> mm/ioremap: change the return value of io[re|un]map_allowed and rename
> mm: ioremap: allow ARCH to have its own ioremap definition
> arc: mm: Convert to GENERIC_IOREMAP
> ia64: mm: Convert to GENERIC_IOREMAP
> openrisc: mm: Convert to GENERIC_IOREMAP
> parisc: mm: Convert to GENERIC_IOREMAP
> s390: mm: Convert to GENERIC_IOREMAP
> sh: mm: Convert to GENERIC_IOREMAP
> xtensa: mm: Convert to GENERIC_IOREMAP
>
> arch/arc/Kconfig | 1 +
> arch/arc/include/asm/io.h | 19 ++++++---
> arch/arc/mm/ioremap.c | 60 ++++-----------------------
> arch/arm64/include/asm/io.h | 5 ++-
> arch/arm64/mm/ioremap.c | 16 +++++---
> arch/hexagon/Kconfig | 1 +
> arch/hexagon/include/asm/io.h | 9 ++++-
> arch/hexagon/mm/ioremap.c | 44 --------------------
> arch/ia64/Kconfig | 1 +
> arch/ia64/include/asm/io.h | 26 +++++++-----
> arch/ia64/mm/ioremap.c | 50 +++++------------------
> arch/openrisc/Kconfig | 1 +
> arch/openrisc/include/asm/io.h | 12 ++++--
> arch/openrisc/mm/ioremap.c | 62 ++--------------------------
> arch/parisc/Kconfig | 1 +
> arch/parisc/include/asm/io.h | 19 ++++++---
> arch/parisc/mm/ioremap.c | 65 +++---------------------------
> arch/s390/Kconfig | 1 +
> arch/s390/include/asm/io.h | 25 +++++++-----
> arch/s390/pci/pci.c | 65 ++++++------------------------
> arch/sh/Kconfig | 1 +
> arch/sh/include/asm/io.h | 67 +++++++++++++++++--------------
> arch/sh/include/asm/io_noioport.h | 7 ++++
> arch/sh/mm/ioremap.c | 63 ++++++-----------------------
> arch/xtensa/Kconfig | 1 +
> arch/xtensa/include/asm/io.h | 39 ++++++++----------
> arch/xtensa/mm/ioremap.c | 56 ++++++--------------------
> include/asm-generic/io.h | 30 ++++++++------
> mm/ioremap.c | 13 ++++--
> 29 files changed, 248 insertions(+), 512 deletions(-)
> delete mode 100644 arch/hexagon/mm/ioremap.c
>