Re: [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap()

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Fri, 2017-07-28 at 01:35 +1000, Oliver O'Halloran wrote:
> The ISA radix translation tree contains two different types of entry,
s/entry/entries
> > directories and leaves. The formats of the two entries are
> different
> with the directory entries containing no spare bits for use by
> software.

Rather than saying the directory entries contain no spare bits, would
it be better to say something like: the devmap property only relates to
pte (leaf) entries and so we shouldn't perform the check on/should
always return false for page directory entries?

> As a result we need to ensure that the *_devmap() family of functions
> check fail for everything except leaf (PTE) entries.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> "i'll just tweak the mbox before i sent it, what's the worst that can
> happen"
> *completely breaks KVM*
> "..."
> ---
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
> b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index d1da415..6bc6248 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -610,7 +610,9 @@ static inline pte_t pte_mkdevmap(pte_t pte)
>  
>  static inline int pte_devmap(pte_t pte)
>  {
> -	return !!(pte_raw(pte) & cpu_to_be64(_PAGE_DEVMAP));
> +	uint64_t mask = cpu_to_be64(_PAGE_DEVMAP | _PAGE_PTE);
> +
> +	return (pte_raw(pte) & mask) == mask;
>  }
>  
>  static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)

Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>