Re: [PATCH v4 4/6] mm: ioremap: Add ioremap/iounmap_allowed()

[Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>]

On 2022/6/6 23:14, Baoquan He wrote:
> On 06/06/22 at 03:48pm, Kefeng Wang wrote:
> > Add special hook for architecture to verify addr, size or prot
> > when ioremap() or iounmap(), which will make the generic ioremap
> > more useful.
> >
> >    ioremap_allowed() return an int,
> >      - NULL means continue to remap
> >      - error code means skip remap and return directly
> >    iounmap_allowed() return an int,
> >      - 0 means continue to vunmap
> >      - error code means skip vunmap and return directly
> Aren't they bool type function and better return bool value?

Ok, let's keep it simple, back to v1's version , use bool, if we want to 
extend

them,  do it later.

> > Acked-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> > ---
> >   include/asm-generic/io.h | 25 +++++++++++++++++++++++++
> >   mm/ioremap.c             | 13 ++++++++++---
> >   2 files changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index e6ffa2519f08..9429387a3e65 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -964,6 +964,31 @@ static inline void iounmap(volatile void __iomem *addr)
> >   #elif defined(CONFIG_GENERIC_IOREMAP)
> >   #include <linux/pgtable.h>
> >   
> > +/*
> > + * Arch code can implement the following two special hooks when using GENERIC_IOREMAP
> > + * ioremap_allowed() return an int,
> > + *   - 0 means continue to remap
> > + *   - error code means skip remap and return directly
> > + * iounmap_allowed() return an int,
> > + *   - 0 means continue to vunmap
> > + *   - error code means skip vunmap and return directly
> > + */
> > +#ifndef ioremap_allowed
> > +#define ioremap_allowed ioremap_allowed
> > +static inline int ioremap_allowed(phys_addr_t phys_addr, size_t size, unsigned long prot)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > +
> > +#ifndef iounmap_allowed
> > +#define iounmap_allowed iounmap_allowed
> > +static inline int iounmap_allowed(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..196c93c0beb8 100644
> > --- a/mm/ioremap.c
> > +++ b/mm/ioremap.c
> > @@ -27,8 +27,10 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro
> >   	phys_addr -= offset;
> >   	size = PAGE_ALIGN(size + offset);
> >   
> > -	area = get_vm_area_caller(size, VM_IOREMAP,
> > -			__builtin_return_address(0));
> > +	if (ioremap_allowed(phys_addr, size, prot))
> > +		return NULL;
> > +
> > +	area = get_vm_area_caller(size, VM_IOREMAP, __builtin_return_address(0));
> >   	if (!area)
> >   		return NULL;
> >   	vaddr = (unsigned long)area->addr;
> > @@ -45,6 +47,11 @@ EXPORT_SYMBOL(ioremap_prot);
> >   
> >   void iounmap(volatile void __iomem *addr)
> >   {
> > -	vunmap((void *)((unsigned long)addr & PAGE_MASK));
> > +	void __iomem *vaddr = (void __iomem *)((unsigned long)addr & PAGE_MASK);
> > +
> > +	if (iounmap_allowed(vaddr))
> > +		return;
> > +
> > +	vunmap((void __force *)vaddr);
> >   }
> >   EXPORT_SYMBOL(iounmap);
> > --
> > 2.35.3
> .