Re: [PATCH 2/3] powerpc ioremap_prot

Andrew Morton
2008-04-29T18:18:30Z

On Tue, 29 Apr 2008 11:33:48 -0400
Rik van Riel <riel@redhat.com> wrote:

> This adds ioremap_prot and pte_pgprot() so that one can extract
> protection bits from a PTE and use them to ioremap_prot() (in
> order to support ptrace of VM_IO | VM_PFNMAP as per Rik's patch).
>
> This moves a couple of flag checks around in the ioremap
> implementations of arch/powerpc. There's a side effect of allowing
> non-cacheable and non-guarded mappings on ppc32 which before would
> always have _PAGE_GUARDED set whenever _PAGE_NO_CACHE is.
>
> (standard ioremap will still set _PAGE_GUARDED, but ioremap_prot
> will be capable of setting such a non guarded mapping).
>
> ...

This depends upon [patch 1/3]. I assume that [3/3] does as well. With a
suitable ack from Paul I can merge all three in one hit when the time
comes, save some paperwork?

> --- linux-work.orig/include/asm-powerpc/io.h 2008-04-29 10:08:39.000000000 +1000
> +++ linux-work/include/asm-powerpc/io.h 2008-04-29 13:21:00.000000000 +1000
> @@ -585,7 +585,8 @@ static inline void iosync(void)
> * and can be hooked by the platform via ppc_md
> *
> * * ioremap_flags allows to specify the page flags as an argument and can
> - * also be hooked by the platform via ppc_md
> + * also be hooked by the platform via ppc_md. ioremap_prot is the exact
> + * same thing as ioremap_flags.
> *
> * * ioremap_nocache is identical to ioremap
> *
> @@ -607,6 +608,9 @@ extern void __iomem *ioremap(phys_addr_t
> extern void __iomem *ioremap_flags(phys_addr_t address, unsigned long size,
> unsigned long flags);
> #define ioremap_nocache(addr, size) ioremap((addr), (size))
> +#define ioremap_prot(addr, size, prot) ioremap_flags((addr), (size), (prot))
> +#define _HAVE_ARCH_IOREMAP_PROT 1

Given that x86 implements ioremap_prot() as a regular C function, it would
be nicer to require that all architectures do that. Especially as macros
suck.

Your powerpc implementation of ioremap_prot() has a different signature
from the x86 one: `phys_addr_t address' versus `resource_size_t phys_addr'.
Can that be improved?

> extern void iounmap(volatile void __iomem *addr);
>
> extern void __iomem *__ioremap(phys_addr_t, unsigned long size,
> Index: linux-work/include/asm-powerpc/pgtable-ppc32.h
> ===================================================================
> --- linux-work.orig/include/asm-powerpc/pgtable-ppc32.h 2008-04-29 10:08:39.000000000 +1000
> +++ linux-work/include/asm-powerpc/pgtable-ppc32.h 2008-04-29 13:04:03.000000000 +1000
> @@ -381,6 +381,12 @@ extern int icache_44x_need_flush;
> #ifndef _PAGE_EXEC
> #define _PAGE_EXEC 0
> #endif
> +#ifndef _PAGE_ENDIAN
> +#define _PAGE_ENDIAN 0
> +#endif
> +#ifndef _PAGE_COHERENT
> +#define _PAGE_COHERENT 0
> +#endif
> #ifndef _PMD_PRESENT_MASK
> #define _PMD_PRESENT_MASK _PMD_PRESENT
> #endif
> @@ -391,6 +397,12 @@ extern int icache_44x_need_flush;
>
> #define _PAGE_CHG_MASK (PAGE_MASK | _PAGE_ACCESSED | _PAGE_DIRTY)
>
> +
> +#define PAGE_PROT_BITS __pgprot(_PAGE_GUARDED | _PAGE_COHERENT | _PAGE_NO_CACHE | \
> + _PAGE_WRITETHRU | _PAGE_ENDIAN | \
> + _PAGE_USER | _PAGE_ACCESSED | \
> + _PAGE_RW | _PAGE_HWWRITE | _PAGE_DIRTY | \
> + _PAGE_EXEC | _PAGE_HWEXEC)
> /*
> * Note: the _PAGE_COHERENT bit automatically gets set in the hardware
> * PTE if CONFIG_SMP is defined (hash_page does this); there is no need
> @@ -524,6 +536,8 @@ static inline pte_t pte_mkyoung(pte_t pt
> pte_val(pte) |= _PAGE_ACCESSED; return pte; }
> static inline pte_t pte_mkspecial(pte_t pte) {
> return pte; }
> +static inline unsigned long pte_pgprot(pte_t pte) {
> + return __pgprot(pte_val(pte)) & PAGE_PROT_BITS; }

ick. \n's are cheap.

> +static inline unsigned long pte_pgprot(pte_t pte) {
> + return __pgprot(pte_val(pte)) & PAGE_PROT_BITS; }