| Message ID | 32dedfbe00c1da0114f66d6a43c56f4a16a85b64.1665568707.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: ioremap: Convert architectures to take GENERIC_IOREMAP way (Alternative) | expand |
On Wed, Oct 12, 2022 at 12:09:39PM +0200, Christophe Leroy wrote: > Define a generic version of ioremap_prot() and iounmap() that > architectures can call after they have performed the necessary > alteration to parameters and/or necessary verifications. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> [...] > diff --git a/mm/ioremap.c b/mm/ioremap.c > index 8652426282cc..9f34a8f90b58 100644 > --- a/mm/ioremap.c > +++ b/mm/ioremap.c > @@ -11,8 +11,8 @@ > #include <linux/io.h> > #include <linux/export.h> > > -void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > - unsigned long prot) > +void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, > + pgprot_t prot) > { > unsigned long offset, vaddr; > phys_addr_t last_addr; > @@ -28,7 +28,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > phys_addr -= offset; > size = PAGE_ALIGN(size + offset); > > - if (!ioremap_allowed(phys_addr, size, prot)) > + if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) > return NULL; It seems to me ioremap_allowed() is not needed anymore. Whatever is checked here would move to architecture- specific implementation. > area = get_vm_area_caller(size, VM_IOREMAP, > @@ -38,17 +38,24 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > vaddr = (unsigned long)area->addr; > area->phys_addr = phys_addr; > > - if (ioremap_page_range(vaddr, vaddr + size, phys_addr, > - __pgprot(prot))) { > + if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot)) { > free_vm_area(area); > return NULL; > } > > return (void __iomem *)(vaddr + offset); > } > + > +#ifndef ioremap_prot I guess, this is also needed: #define ioremap_prot ioremap_prot > +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > + unsigned long prot) > +{ > + return generic_ioremap_prot(phys_addr, size, __pgprot(prot)); > +} > EXPORT_SYMBOL(ioremap_prot); > +#endif > > -void iounmap(volatile void __iomem *addr) > +void generic_iounmap(volatile void __iomem *addr) > { > void *vaddr = (void *)((unsigned long)addr & PAGE_MASK); > > @@ -58,4 +65,11 @@ void iounmap(volatile void __iomem *addr) > if (is_vmalloc_addr(vaddr)) > vunmap(vaddr); > } > + > +#ifndef iounmap Same here. > +void iounmap(volatile void __iomem *addr) > +{ > + generic_iounmap(addr); > +} > EXPORT_SYMBOL(iounmap); > +#endif
Le 16/10/2022 à 10:14, Alexander Gordeev a écrit : > On Wed, Oct 12, 2022 at 12:09:39PM +0200, Christophe Leroy wrote: >> Define a generic version of ioremap_prot() and iounmap() that >> architectures can call after they have performed the necessary >> alteration to parameters and/or necessary verifications. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > [...] > >> diff --git a/mm/ioremap.c b/mm/ioremap.c >> index 8652426282cc..9f34a8f90b58 100644 >> --- a/mm/ioremap.c >> +++ b/mm/ioremap.c >> @@ -11,8 +11,8 @@ >> #include <linux/io.h> >> #include <linux/export.h> >> >> -void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, >> - unsigned long prot) >> +void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, >> + pgprot_t prot) >> { >> unsigned long offset, vaddr; >> phys_addr_t last_addr; >> @@ -28,7 +28,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, >> phys_addr -= offset; >> size = PAGE_ALIGN(size + offset); >> >> - if (!ioremap_allowed(phys_addr, size, prot)) >> + if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) >> return NULL; > > It seems to me ioremap_allowed() is not needed anymore. > Whatever is checked here would move to architecture- > specific implementation. Yes can probably be removed as a follow-up. I didn't want to change existing implementations at the first place, and see what it looks like once all architectures have been looked at. > >> area = get_vm_area_caller(size, VM_IOREMAP, >> @@ -38,17 +38,24 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, >> vaddr = (unsigned long)area->addr; >> area->phys_addr = phys_addr; >> >> - if (ioremap_page_range(vaddr, vaddr + size, phys_addr, >> - __pgprot(prot))) { >> + if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot)) { >> free_vm_area(area); >> return NULL; >> } >> >> return (void __iomem *)(vaddr + offset); >> } >> + >> +#ifndef ioremap_prot > > I guess, this is also needed: > > #define ioremap_prot ioremap_prot Why would it need that ? We are in ioremap.c, any define done here is exclusively seen here in this C file. It's not like a header file. > >> +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, >> + unsigned long prot) >> +{ >> + return generic_ioremap_prot(phys_addr, size, __pgprot(prot)); >> +} >> EXPORT_SYMBOL(ioremap_prot); >> +#endif >> >> -void iounmap(volatile void __iomem *addr) >> +void generic_iounmap(volatile void __iomem *addr) >> { >> void *vaddr = (void *)((unsigned long)addr & PAGE_MASK); >> >> @@ -58,4 +65,11 @@ void iounmap(volatile void __iomem *addr) >> if (is_vmalloc_addr(vaddr)) >> vunmap(vaddr); >> } >> + >> +#ifndef iounmap > > Same here. Same, I don't see what it would add. > >> +void iounmap(volatile void __iomem *addr) >> +{ >> + generic_iounmap(addr); >> +} >> EXPORT_SYMBOL(iounmap); >> +#endif
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index a68f8fbf423b..43eb4f62e954 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h @@ -1073,9 +1073,13 @@ static inline bool iounmap_allowed(void *addr) } #endif +void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, + pgprot_t prot); + void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long prot); void iounmap(volatile void __iomem *addr); +void generic_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 8652426282cc..9f34a8f90b58 100644 --- a/mm/ioremap.c +++ b/mm/ioremap.c @@ -11,8 +11,8 @@ #include <linux/io.h> #include <linux/export.h> -void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, - unsigned long prot) +void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size, + pgprot_t prot) { unsigned long offset, vaddr; phys_addr_t last_addr; @@ -28,7 +28,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, phys_addr -= offset; size = PAGE_ALIGN(size + offset); - if (!ioremap_allowed(phys_addr, size, prot)) + if (!ioremap_allowed(phys_addr, size, pgprot_val(prot))) return NULL; area = get_vm_area_caller(size, VM_IOREMAP, @@ -38,17 +38,24 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, vaddr = (unsigned long)area->addr; area->phys_addr = phys_addr; - if (ioremap_page_range(vaddr, vaddr + size, phys_addr, - __pgprot(prot))) { + if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot)) { free_vm_area(area); return NULL; } return (void __iomem *)(vaddr + offset); } + +#ifndef ioremap_prot +void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, + unsigned long prot) +{ + return generic_ioremap_prot(phys_addr, size, __pgprot(prot)); +} EXPORT_SYMBOL(ioremap_prot); +#endif -void iounmap(volatile void __iomem *addr) +void generic_iounmap(volatile void __iomem *addr) { void *vaddr = (void *)((unsigned long)addr & PAGE_MASK); @@ -58,4 +65,11 @@ void iounmap(volatile void __iomem *addr) if (is_vmalloc_addr(vaddr)) vunmap(vaddr); } + +#ifndef iounmap +void iounmap(volatile void __iomem *addr) +{ + generic_iounmap(addr); +} EXPORT_SYMBOL(iounmap); +#endif
Define a generic version of ioremap_prot() and iounmap() that architectures can call after they have performed the necessary alteration to parameters and/or necessary verifications. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- include/asm-generic/io.h | 4 ++++ mm/ioremap.c | 26 ++++++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-)