Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

And an additional comment regarding the rtas bit:

>  #ifdef CONFIG_PPC_SMLPAR
>  void arch_free_page(struct page *page, int order);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index d5ca823..07cf2cf 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
>  	return 0;
>  }
>  
> +int page_is_rtas(unsigned long pfn)
> +{
> +	unsigned long paddr = (pfn << PAGE_SHIFT);
> +
> +	if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
> +		return 1;
> +
> +	return 0;
> +}

So this only exist whenever rtas.c is compiled... but ...

> +
>  /*
>   * Call early during boot, before mem init or bootmem, to retrieve the RTAS
>   * informations from the device-tree and allocate the RMO buffer for userland
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index c781bbc..d21dbc6 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
>  	hash_preload(vma->vm_mm, address, access, trap);
>  #endif /* CONFIG_PPC_STD_MMU */
>  }
> +
> +extern int page_is_rtas(unsigned long pfn);
> +
> +/*
> + * devmem_is_allowed(): check to see if /dev/mem access to a certain address
> + * is valid. The argument is a physical page number.
> + *
> + * Access has to be given to non-kernel-ram areas as well, these contain the
> + * PCI mmio resources as well as potential bios/acpi data regions.
> + */
> +int devmem_is_allowed(unsigned long pfn)
> +{
> +	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
> +		return 0;
> +	if (!page_is_ram(pfn))
> +		return 1;
> +	if (page_is_rtas(pfn))
> +		return 1;
> +	return 0;
> +}

This calls it unconditionally... you just broke the build of all !rtas
platforms. Additionally, putting an extern definition like that in a .c
file is gross at best....

Please instead, put in a header something like

#ifdef CONFIG_PPC_RTAS
extern int page_is_rtas(unsigned long pfn);
#else
static inline int page_is_rtas(unsigned long pfn) { }
#endif

And while at it, call it page_is_rtas_user_buf(); to make it clear what
we are talking about, ie, not RTAS core per-se but specifically the RMO
buffer.

Cheers,
Ben.