[PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Baoquan He]

Add hooks arch_ioremap() and arch_iounmap() for operisc's special
operation when ioremap() and iounmap.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Stafford Horne <shorne@gmail.com>
Cc: openrisc@lists.librecores.org
---
 arch/openrisc/Kconfig          |  1 +
 arch/openrisc/include/asm/io.h | 16 ++++++++---
 arch/openrisc/mm/ioremap.c     | 51 +++++++++++-----------------------
 3 files changed, 29 insertions(+), 39 deletions(-)

diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index c7f282f60f64..fd9bb76a610b 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -21,6 +21,7 @@ config OPENRISC
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_SHOW
 	select GENERIC_PCI_IOMAP
+	select GENERIC_IOREMAP
 	select GENERIC_CPU_DEVICES
 	select HAVE_PCI
 	select HAVE_UID16
diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
index ee6043a03173..9db67938bfc4 100644
--- a/arch/openrisc/include/asm/io.h
+++ b/arch/openrisc/include/asm/io.h
@@ -15,6 +15,8 @@
 #define __ASM_OPENRISC_IO_H
 
 #include <linux/types.h>
+#include <asm/pgtable.h>
+#include <asm/pgalloc.h>
 
 /*
  * PCI: We do not use IO ports in OpenRISC
@@ -27,11 +29,17 @@
 #define PIO_OFFSET		0
 #define PIO_MASK		0
 
-#define ioremap ioremap
-void __iomem *ioremap(phys_addr_t offset, unsigned long size);
+/*
+ * 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
+
+int arch_iounmap(void __iomem *addr);
+#define arch_iounmap arch_iounmap
 
-#define iounmap iounmap
-extern void iounmap(volatile void __iomem *addr);
+#define _PAGE_IOREMAP (pgprot_val(PAGE_KERNEL) | _PAGE_CI)
 
 #include <asm-generic/io.h>
 
diff --git a/arch/openrisc/mm/ioremap.c b/arch/openrisc/mm/ioremap.c
index 8ec0dafecf25..bc41660e1fb0 100644
--- a/arch/openrisc/mm/ioremap.c
+++ b/arch/openrisc/mm/ioremap.c
@@ -24,26 +24,18 @@ extern int mem_init_done;
 
 static unsigned int fixmaps_used __initdata;
 
-/*
- * Remap an arbitrary physical address space into the kernel virtual
- * address space. Needed when the kernel wants to access high addresses
- * directly.
- *
- * 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 *__ref ioremap(phys_addr_t addr, unsigned long size)
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
 {
 	phys_addr_t p;
 	unsigned long v;
-	unsigned long offset, last_addr;
-	struct vm_struct *area = NULL;
+	unsigned long offset, last_addr, addr = *paddr;
+	int ret = -EINVAL;
 
 	/* Don't allow wraparound or zero size */
 	last_addr = addr + size - 1;
 	if (!size || last_addr < addr)
-		return NULL;
+		return IOMEM_ERR_PTR(ret);
 
 	/*
 	 * Mappings have to be page-aligned
@@ -52,32 +44,24 @@ void __iomem *__ref ioremap(phys_addr_t addr, unsigned long size)
 	p = addr & PAGE_MASK;
 	size = PAGE_ALIGN(last_addr + 1) - p;
 
-	if (likely(mem_init_done)) {
-		area = get_vm_area(size, VM_IOREMAP);
-		if (!area)
-			return NULL;
-		v = (unsigned long)area->addr;
-	} else {
+	if (unlikely(!mem_init_done)) {
 		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
-			return NULL;
+			return IOMEM_ERR_PTR(ret);
 		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
 		fixmaps_used += (size >> PAGE_SHIFT);
-	}
 
-	if (ioremap_page_range(v, v + size, p,
-			__pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_CI))) {
-		if (likely(mem_init_done))
-			vfree(area->addr);
-		else
+		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
 			fixmaps_used -= (size >> PAGE_SHIFT);
-		return NULL;
+			return IOMEM_ERR_PTR(ret);
+		}
+
+		return (void __iomem *)(offset + (char *)v);
 	}
 
-	return (void __iomem *)(offset + (char *)v);
+	return NULL;
 }
-EXPORT_SYMBOL(ioremap);
 
-void iounmap(volatile void __iomem *addr)
+int arch_iounmap(void __iomem *addr)
 {
 	/* If the page is from the fixmap pool then we just clear out
 	 * the fixmap mapping.
@@ -97,13 +81,10 @@ void iounmap(volatile void __iomem *addr)
 		 *   ii) invalid accesses to the freed areas aren't made
 		 */
 		flush_tlb_all();
-		return;
+		return -EINVAL;
 	}
-
-	return vfree((void *)(PAGE_MASK & (unsigned long)addr));
+	return 0;
 }
-EXPORT_SYMBOL(iounmap);
-
 /**
  * OK, this one's a bit tricky... ioremap can get called before memory is
  * initialized (early serial console does this) and will want to alloc a page
-- 
2.34.1

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Christoph Hellwig]

> +	if (unlikely(!mem_init_done)) {
>  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> +			return IOMEM_ERR_PTR(ret);
>  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
>  		fixmaps_used += (size >> PAGE_SHIFT);
>  
> +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
>  			fixmaps_used -= (size >> PAGE_SHIFT);
> +			return IOMEM_ERR_PTR(ret);
> +		}
> +
> +		return (void __iomem *)(offset + (char *)v);
>  	}

This code needs to go away, and all very early boot uses of ioremap
need to switch to use early_ioremap insted.

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Baoquan He]

On 08/21/22 at 12:03am, Christoph Hellwig wrote:
> > +	if (unlikely(!mem_init_done)) {
> >  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> > +			return IOMEM_ERR_PTR(ret);
> >  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
> >  		fixmaps_used += (size >> PAGE_SHIFT);
> >  
> > +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
> >  			fixmaps_used -= (size >> PAGE_SHIFT);
> > +			return IOMEM_ERR_PTR(ret);
> > +		}
> > +
> > +		return (void __iomem *)(offset + (char *)v);
> >  	}
> 
> This code needs to go away, and all very early boot uses of ioremap
> need to switch to use early_ioremap insted.

Makes sense. On openrisc, the thing is I didn't find one place where
ioremap() is called in arch code. I can cut the early ioremap out and
wrap into a separate early_ioremap() function, however I don't know
where to put it. Not sure if I miss anything or openrisc doesn't really
need early ioremap.

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Stafford Horne]

On Mon, Aug 29, 2022 at 09:40:24AM +0800, Baoquan He wrote:
> On 08/21/22 at 12:03am, Christoph Hellwig wrote:
> > > +	if (unlikely(!mem_init_done)) {
> > >  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> > > +			return IOMEM_ERR_PTR(ret);
> > >  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
> > >  		fixmaps_used += (size >> PAGE_SHIFT);
> > >  
> > > +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
> > >  			fixmaps_used -= (size >> PAGE_SHIFT);
> > > +			return IOMEM_ERR_PTR(ret);
> > > +		}
> > > +
> > > +		return (void __iomem *)(offset + (char *)v);
> > >  	}
> > 
> > This code needs to go away, and all very early boot uses of ioremap
> > need to switch to use early_ioremap insted.
> 
> Makes sense. On openrisc, the thing is I didn't find one place where
> ioremap() is called in arch code. I can cut the early ioremap out and
> wrap into a separate early_ioremap() function, however I don't know
> where to put it. Not sure if I miss anything or openrisc doesn't really
> need early ioremap.

Hi,

I don't know of any early_ioremap usage either in openrisc, maybe some drivers
use it?  However, we do not initialize any early_ioremap infrastructure in
openrisc so that may cause issues if it is used.

We can try to remove it all.  I tested these below additional changes and they
work, if you want to add them we can see if kbuild robots pick anything up.

-Stafford

diff --git a/arch/openrisc/mm/ioremap.c b/arch/openrisc/mm/ioremap.c
index bc41660e1fb0..ffe6d5e2b5fe 100644
--- a/arch/openrisc/mm/ioremap.c
+++ b/arch/openrisc/mm/ioremap.c
@@ -22,14 +22,10 @@
 
 extern int mem_init_done;
 
-static unsigned int fixmaps_used __initdata;
-
 void __iomem *
 arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
 {
-       phys_addr_t p;
-       unsigned long v;
-       unsigned long offset, last_addr, addr = *paddr;
+       unsigned long last_addr, addr = *paddr;
        int ret = -EINVAL;
 
        /* Don't allow wraparound or zero size */
@@ -37,27 +33,6 @@ arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
        if (!size || last_addr < addr)
                return IOMEM_ERR_PTR(ret);
 
-       /*
-        * Mappings have to be page-aligned
-        */
-       offset = addr & ~PAGE_MASK;
-       p = addr & PAGE_MASK;
-       size = PAGE_ALIGN(last_addr + 1) - p;
-
-       if (unlikely(!mem_init_done)) {
-               if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
-                       return IOMEM_ERR_PTR(ret);
-               v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
-               fixmaps_used += (size >> PAGE_SHIFT);
-
-               if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
-                       fixmaps_used -= (size >> PAGE_SHIFT);
-                       return IOMEM_ERR_PTR(ret);
-               }
-
-               return (void __iomem *)(offset + (char *)v);
-       }
-
        return NULL;
 }
 

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Baoquan He]

On 08/29/22 at 06:42am, Stafford Horne wrote:
> On Mon, Aug 29, 2022 at 09:40:24AM +0800, Baoquan He wrote:
> > On 08/21/22 at 12:03am, Christoph Hellwig wrote:
> > > > +	if (unlikely(!mem_init_done)) {
> > > >  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> > > > +			return IOMEM_ERR_PTR(ret);
> > > >  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
> > > >  		fixmaps_used += (size >> PAGE_SHIFT);
> > > >  
> > > > +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
> > > >  			fixmaps_used -= (size >> PAGE_SHIFT);
> > > > +			return IOMEM_ERR_PTR(ret);
> > > > +		}
> > > > +
> > > > +		return (void __iomem *)(offset + (char *)v);
> > > >  	}
> > > 
> > > This code needs to go away, and all very early boot uses of ioremap
> > > need to switch to use early_ioremap insted.
> > 
> > Makes sense. On openrisc, the thing is I didn't find one place where
> > ioremap() is called in arch code. I can cut the early ioremap out and
> > wrap into a separate early_ioremap() function, however I don't know
> > where to put it. Not sure if I miss anything or openrisc doesn't really
> > need early ioremap.
> 
> Hi,
> 
> I don't know of any early_ioremap usage either in openrisc, maybe some drivers
> use it?  However, we do not initialize any early_ioremap infrastructure in
> openrisc so that may cause issues if it is used.

The variable mem_init_done indicates if mem_init() is called or not.
Driver should only initialize after mem_init(). With my understanding,
the early ioremap is only needed by arch code.
> 
> We can try to remove it all.  I tested these below additional changes and they
> work, if you want to add them we can see if kbuild robots pick anything up.

Very helpful information. Then I will remove the !mem_init_done part
code as you listed below, let's see how test robots react. Thanks a lot.

> 
> -Stafford
> 
> diff --git a/arch/openrisc/mm/ioremap.c b/arch/openrisc/mm/ioremap.c
> index bc41660e1fb0..ffe6d5e2b5fe 100644
> --- a/arch/openrisc/mm/ioremap.c
> +++ b/arch/openrisc/mm/ioremap.c
> @@ -22,14 +22,10 @@
>  
>  extern int mem_init_done;
>  
> -static unsigned int fixmaps_used __initdata;
> -
>  void __iomem *
>  arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
>  {
> -       phys_addr_t p;
> -       unsigned long v;
> -       unsigned long offset, last_addr, addr = *paddr;
> +       unsigned long last_addr, addr = *paddr;
>         int ret = -EINVAL;
>  
>         /* Don't allow wraparound or zero size */
> @@ -37,27 +33,6 @@ arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
>         if (!size || last_addr < addr)
>                 return IOMEM_ERR_PTR(ret);
>  
> -       /*
> -        * Mappings have to be page-aligned
> -        */
> -       offset = addr & ~PAGE_MASK;
> -       p = addr & PAGE_MASK;
> -       size = PAGE_ALIGN(last_addr + 1) - p;
> -
> -       if (unlikely(!mem_init_done)) {
> -               if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> -                       return IOMEM_ERR_PTR(ret);
> -               v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
> -               fixmaps_used += (size >> PAGE_SHIFT);
> -
> -               if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
> -                       fixmaps_used -= (size >> PAGE_SHIFT);
> -                       return IOMEM_ERR_PTR(ret);
> -               }
> -
> -               return (void __iomem *)(offset + (char *)v);
> -       }
> -
>         return NULL;
>  }
>  
> 

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Christophe Leroy]

Hi Christoph,

Le 21/08/2022 à 09:03, Christoph Hellwig a écrit :
>> +	if (unlikely(!mem_init_done)) {
>>   		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
>> +			return IOMEM_ERR_PTR(ret);
>>   		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
>>   		fixmaps_used += (size >> PAGE_SHIFT);
>>   
>> +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
>>   			fixmaps_used -= (size >> PAGE_SHIFT);
>> +			return IOMEM_ERR_PTR(ret);
>> +		}
>> +
>> +		return (void __iomem *)(offset + (char *)v);
>>   	}
> 
> This code needs to go away, and all very early boot uses of ioremap
> need to switch to use early_ioremap insted.
> 

early_ioremap() is not that easy to use. I don't know in this particular 
openrisc context, but in powerpc we have many places that use ioremap() 
very early.

Three years ago we implemented EARLY_IOREMAP, commit d538aadc2718 
("powerpc/ioremap: warn on early use of ioremap()") added a warning to 
identify all places using ioremap() early with the target being to get 
rid of early usage of ioremap. But it turned that using early_ioremap() 
is not that easy.

The problem with early_ioremap() is that it installs ephemeral mappings 
that must be freed before the end of init. Most early uses of ioremap() 
in powerpc are to install mappings that will last for life.

If we could have something that works like memblock_alloc() and allows 
to keep the allocation forever, then it would be a lot easier to use 
early_ioremap() and we could get rid of the early handling of ioremap() 
in powerpc.

Christophe

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Stafford Horne]

On Sat, Aug 20, 2022 at 08:31:21AM +0800, Baoquan He wrote:
> Add hooks arch_ioremap() and arch_iounmap() for operisc's special
> operation when ioremap() and iounmap.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Cc: Jonas Bonn <jonas@southpole.se>
> Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> Cc: Stafford Horne <shorne@gmail.com>
> Cc: openrisc@lists.librecores.org
> ---
>  arch/openrisc/Kconfig          |  1 +
>  arch/openrisc/include/asm/io.h | 16 ++++++++---
>  arch/openrisc/mm/ioremap.c     | 51 +++++++++++-----------------------
>  3 files changed, 29 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index c7f282f60f64..fd9bb76a610b 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -21,6 +21,7 @@ config OPENRISC
>  	select GENERIC_IRQ_PROBE
>  	select GENERIC_IRQ_SHOW
>  	select GENERIC_PCI_IOMAP
> +	select GENERIC_IOREMAP
>  	select GENERIC_CPU_DEVICES
>  	select HAVE_PCI
>  	select HAVE_UID16
> diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
> index ee6043a03173..9db67938bfc4 100644
> --- a/arch/openrisc/include/asm/io.h
> +++ b/arch/openrisc/include/asm/io.h
> @@ -15,6 +15,8 @@
>  #define __ASM_OPENRISC_IO_H
>  
>  #include <linux/types.h>
> +#include <asm/pgtable.h>
> +#include <asm/pgalloc.h>

This seems to cause a compilation issue when building virt_defconig:

      CC      kernel/irq/generic-chip.o
    In file included from ./include/asm-generic/pgtable-nopud.h:7,
		     from ./include/asm-generic/pgtable-nopmd.h:7,
		     from ./arch/openrisc/include/asm/pgtable.h:24,
		     from ./arch/openrisc/include/asm/io.h:18,
		     from ./include/linux/io.h:13,
		     from kernel/irq/generic-chip.c:7:
    ./include/asm-generic/pgtable-nop4d.h:9:18: error: unknown type name 'pgd_t'
	9 | typedef struct { pgd_t pgd; } p4d_t;
	  |                  ^~~~~

It works if we swap the order arround:

    +#include <asm/pgalloc.h>
    +#include <asm/pgtable.h>

Otherwise we need to add an asm/page.h include to asm/pgtable.h (which is more
correct) but means you would touch more files.

>  /*
>   * PCI: We do not use IO ports in OpenRISC
> @@ -27,11 +29,17 @@
>  #define PIO_OFFSET		0
>  #define PIO_MASK		0
>  
> -#define ioremap ioremap
> -void __iomem *ioremap(phys_addr_t offset, unsigned long size);
> +/*
> + * 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
> +
> +int arch_iounmap(void __iomem *addr);
> +#define arch_iounmap arch_iounmap
>  
> -#define iounmap iounmap
> -extern void iounmap(volatile void __iomem *addr);
> +#define _PAGE_IOREMAP (pgprot_val(PAGE_KERNEL) | _PAGE_CI)
>  
>  #include <asm-generic/io.h>
>  
> diff --git a/arch/openrisc/mm/ioremap.c b/arch/openrisc/mm/ioremap.c
> index 8ec0dafecf25..bc41660e1fb0 100644
> --- a/arch/openrisc/mm/ioremap.c
> +++ b/arch/openrisc/mm/ioremap.c
> @@ -24,26 +24,18 @@ extern int mem_init_done;
>  
>  static unsigned int fixmaps_used __initdata;
>  
> -/*
> - * Remap an arbitrary physical address space into the kernel virtual
> - * address space. Needed when the kernel wants to access high addresses
> - * directly.
> - *
> - * 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 *__ref ioremap(phys_addr_t addr, unsigned long size)
> +void __iomem *
> +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
>  {
>  	phys_addr_t p;
>  	unsigned long v;
> -	unsigned long offset, last_addr;
> -	struct vm_struct *area = NULL;
> +	unsigned long offset, last_addr, addr = *paddr;
> +	int ret = -EINVAL;
>  
>  	/* Don't allow wraparound or zero size */
>  	last_addr = addr + size - 1;
>  	if (!size || last_addr < addr)
> -		return NULL;
> +		return IOMEM_ERR_PTR(ret);
>  
>  	/*
>  	 * Mappings have to be page-aligned
> @@ -52,32 +44,24 @@ void __iomem *__ref ioremap(phys_addr_t addr, unsigned long size)
>  	p = addr & PAGE_MASK;
>  	size = PAGE_ALIGN(last_addr + 1) - p;
>  
> -	if (likely(mem_init_done)) {
> -		area = get_vm_area(size, VM_IOREMAP);
> -		if (!area)
> -			return NULL;
> -		v = (unsigned long)area->addr;
> -	} else {
> +	if (unlikely(!mem_init_done)) {
>  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> -			return NULL;
> +			return IOMEM_ERR_PTR(ret);
>  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
>  		fixmaps_used += (size >> PAGE_SHIFT);
> -	}
>  
> -	if (ioremap_page_range(v, v + size, p,
> -			__pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_CI))) {
> -		if (likely(mem_init_done))
> -			vfree(area->addr);
> -		else
> +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
>  			fixmaps_used -= (size >> PAGE_SHIFT);
> -		return NULL;
> +			return IOMEM_ERR_PTR(ret);
> +		}
> +
> +		return (void __iomem *)(offset + (char *)v);
>  	}
>  
> -	return (void __iomem *)(offset + (char *)v);
> +	return NULL;
>  }
> -EXPORT_SYMBOL(ioremap);
>  
> -void iounmap(volatile void __iomem *addr)
> +int arch_iounmap(void __iomem *addr)
>  {
>  	/* If the page is from the fixmap pool then we just clear out
>  	 * the fixmap mapping.
> @@ -97,13 +81,10 @@ void iounmap(volatile void __iomem *addr)
>  		 *   ii) invalid accesses to the freed areas aren't made
>  		 */
>  		flush_tlb_all();
> -		return;
> +		return -EINVAL;
>  	}
> -
> -	return vfree((void *)(PAGE_MASK & (unsigned long)addr));
> +	return 0;
>  }
> -EXPORT_SYMBOL(iounmap);
> -
>  /**
>   * OK, this one's a bit tricky... ioremap can get called before memory is
>   * initialized (early serial console does this) and will want to alloc a page
> -- 
> 2.34.1

Other than that compiler issue, I fixed it and test booted this and it works
well.

Acked-by: Stafford Horne <shorne@gmail.com>

Re: [PATCH v2 07/11] openrisc: mm: Convert to GENERIC_IOREMAP

[Baoquan He]

On 08/29/22 at 06:32am, Stafford Horne wrote:
> On Sat, Aug 20, 2022 at 08:31:21AM +0800, Baoquan He wrote:
> > Add hooks arch_ioremap() and arch_iounmap() for operisc's special
> > operation when ioremap() and iounmap.
> > 
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Cc: Jonas Bonn <jonas@southpole.se>
> > Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> > Cc: Stafford Horne <shorne@gmail.com>
> > Cc: openrisc@lists.librecores.org
> > ---
> >  arch/openrisc/Kconfig          |  1 +
> >  arch/openrisc/include/asm/io.h | 16 ++++++++---
> >  arch/openrisc/mm/ioremap.c     | 51 +++++++++++-----------------------
> >  3 files changed, 29 insertions(+), 39 deletions(-)
> > 
> > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > index c7f282f60f64..fd9bb76a610b 100644
> > --- a/arch/openrisc/Kconfig
> > +++ b/arch/openrisc/Kconfig
> > @@ -21,6 +21,7 @@ config OPENRISC
> >  	select GENERIC_IRQ_PROBE
> >  	select GENERIC_IRQ_SHOW
> >  	select GENERIC_PCI_IOMAP
> > +	select GENERIC_IOREMAP
> >  	select GENERIC_CPU_DEVICES
> >  	select HAVE_PCI
> >  	select HAVE_UID16
> > diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
> > index ee6043a03173..9db67938bfc4 100644
> > --- a/arch/openrisc/include/asm/io.h
> > +++ b/arch/openrisc/include/asm/io.h
> > @@ -15,6 +15,8 @@
> >  #define __ASM_OPENRISC_IO_H
> >  
> >  #include <linux/types.h>
> > +#include <asm/pgtable.h>
> > +#include <asm/pgalloc.h>
> 
> This seems to cause a compilation issue when building virt_defconig:
> 
>       CC      kernel/irq/generic-chip.o
>     In file included from ./include/asm-generic/pgtable-nopud.h:7,
> 		     from ./include/asm-generic/pgtable-nopmd.h:7,
> 		     from ./arch/openrisc/include/asm/pgtable.h:24,
> 		     from ./arch/openrisc/include/asm/io.h:18,
> 		     from ./include/linux/io.h:13,
> 		     from kernel/irq/generic-chip.c:7:
>     ./include/asm-generic/pgtable-nop4d.h:9:18: error: unknown type name 'pgd_t'
> 	9 | typedef struct { pgd_t pgd; } p4d_t;
> 	  |                  ^~~~~
> 
> It works if we swap the order arround:
> 
>     +#include <asm/pgalloc.h>
>     +#include <asm/pgtable.h>

Thanks a lot, add this change to patch.

> 
> Otherwise we need to add an asm/page.h include to asm/pgtable.h (which is more
> correct) but means you would touch more files.
> 
> >  /*
> >   * PCI: We do not use IO ports in OpenRISC
> > @@ -27,11 +29,17 @@
> >  #define PIO_OFFSET		0
> >  #define PIO_MASK		0
> >  
> > -#define ioremap ioremap
> > -void __iomem *ioremap(phys_addr_t offset, unsigned long size);
> > +/*
> > + * 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
> > +
> > +int arch_iounmap(void __iomem *addr);
> > +#define arch_iounmap arch_iounmap
> >  
> > -#define iounmap iounmap
> > -extern void iounmap(volatile void __iomem *addr);
> > +#define _PAGE_IOREMAP (pgprot_val(PAGE_KERNEL) | _PAGE_CI)
> >  
> >  #include <asm-generic/io.h>
> >  
> > diff --git a/arch/openrisc/mm/ioremap.c b/arch/openrisc/mm/ioremap.c
> > index 8ec0dafecf25..bc41660e1fb0 100644
> > --- a/arch/openrisc/mm/ioremap.c
> > +++ b/arch/openrisc/mm/ioremap.c
> > @@ -24,26 +24,18 @@ extern int mem_init_done;
> >  
> >  static unsigned int fixmaps_used __initdata;
> >  
> > -/*
> > - * Remap an arbitrary physical address space into the kernel virtual
> > - * address space. Needed when the kernel wants to access high addresses
> > - * directly.
> > - *
> > - * 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 *__ref ioremap(phys_addr_t addr, unsigned long size)
> > +void __iomem *
> > +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
> >  {
> >  	phys_addr_t p;
> >  	unsigned long v;
> > -	unsigned long offset, last_addr;
> > -	struct vm_struct *area = NULL;
> > +	unsigned long offset, last_addr, addr = *paddr;
> > +	int ret = -EINVAL;
> >  
> >  	/* Don't allow wraparound or zero size */
> >  	last_addr = addr + size - 1;
> >  	if (!size || last_addr < addr)
> > -		return NULL;
> > +		return IOMEM_ERR_PTR(ret);
> >  
> >  	/*
> >  	 * Mappings have to be page-aligned
> > @@ -52,32 +44,24 @@ void __iomem *__ref ioremap(phys_addr_t addr, unsigned long size)
> >  	p = addr & PAGE_MASK;
> >  	size = PAGE_ALIGN(last_addr + 1) - p;
> >  
> > -	if (likely(mem_init_done)) {
> > -		area = get_vm_area(size, VM_IOREMAP);
> > -		if (!area)
> > -			return NULL;
> > -		v = (unsigned long)area->addr;
> > -	} else {
> > +	if (unlikely(!mem_init_done)) {
> >  		if ((fixmaps_used + (size >> PAGE_SHIFT)) > FIX_N_IOREMAPS)
> > -			return NULL;
> > +			return IOMEM_ERR_PTR(ret);
> >  		v = fix_to_virt(FIX_IOREMAP_BEGIN + fixmaps_used);
> >  		fixmaps_used += (size >> PAGE_SHIFT);
> > -	}
> >  
> > -	if (ioremap_page_range(v, v + size, p,
> > -			__pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_CI))) {
> > -		if (likely(mem_init_done))
> > -			vfree(area->addr);
> > -		else
> > +		if (ioremap_page_range(v, v + size, p, __pgprot(*prot_val))) {
> >  			fixmaps_used -= (size >> PAGE_SHIFT);
> > -		return NULL;
> > +			return IOMEM_ERR_PTR(ret);
> > +		}
> > +
> > +		return (void __iomem *)(offset + (char *)v);
> >  	}
> >  
> > -	return (void __iomem *)(offset + (char *)v);
> > +	return NULL;
> >  }
> > -EXPORT_SYMBOL(ioremap);
> >  
> > -void iounmap(volatile void __iomem *addr)
> > +int arch_iounmap(void __iomem *addr)
> >  {
> >  	/* If the page is from the fixmap pool then we just clear out
> >  	 * the fixmap mapping.
> > @@ -97,13 +81,10 @@ void iounmap(volatile void __iomem *addr)
> >  		 *   ii) invalid accesses to the freed areas aren't made
> >  		 */
> >  		flush_tlb_all();
> > -		return;
> > +		return -EINVAL;
> >  	}
> > -
> > -	return vfree((void *)(PAGE_MASK & (unsigned long)addr));
> > +	return 0;
> >  }
> > -EXPORT_SYMBOL(iounmap);
> > -
> >  /**
> >   * OK, this one's a bit tricky... ioremap can get called before memory is
> >   * initialized (early serial console does this) and will want to alloc a page
> > -- 
> > 2.34.1
> 
> Other than that compiler issue, I fixed it and test booted this and it works
> well.
> 
> Acked-by: Stafford Horne <shorne@gmail.com>

Thanks, will add this tag when repost.