* Re: [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap()
2017-07-27 15:35 [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap() Oliver O'Halloran
@ 2017-07-27 19:14 ` Laurent Vivier
2017-07-27 21:47 ` joserz
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2017-07-27 19:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: joserz, sjitindarsingh, Oliver O'Halloran
On 27/07/2017 17:35, oohall@gmail.com (Oliver O'Halloran) wrote:
> The ISA radix translation tree contains two different types of entry,
> directories and leaves. The formats of the two entries are different
> with the directory entries containing no spare bits for use by software.
> 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)
>
Tested-by: Laurent Vivier <lvivier@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap()
2017-07-27 15:35 [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap() Oliver O'Halloran
2017-07-27 19:14 ` Laurent Vivier
@ 2017-07-27 21:47 ` joserz
2017-07-28 1:14 ` Suraj Jitindar Singh
2017-07-28 10:15 ` [v2] " Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: joserz @ 2017-07-27 21:47 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev, mpe, sjitindarsingh
On Fri, Jul 28, 2017 at 01:35:53AM +1000, Oliver O'Halloran wrote:
> The ISA radix translation tree contains two different types of entry,
> directories and leaves. The formats of the two entries are different
> with the directory entries containing no spare bits for use by software.
> 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)
> --
> 2.7.4
>
Michael should thank you because I was about to send another insane
suggestion and ask him if that makes sense. :-D
Tested-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap()
2017-07-27 15:35 [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap() Oliver O'Halloran
2017-07-27 19:14 ` Laurent Vivier
2017-07-27 21:47 ` joserz
@ 2017-07-28 1:14 ` Suraj Jitindar Singh
2017-07-28 10:15 ` [v2] " Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: Suraj Jitindar Singh @ 2017-07-28 1:14 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev; +Cc: mpe, joserz
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [v2] powerpc/mm: Check for _PAGE_PTE in *_devmap()
2017-07-27 15:35 [PATCH v2] powerpc/mm: Check for _PAGE_PTE in *_devmap() Oliver O'Halloran
` (2 preceding siblings ...)
2017-07-28 1:14 ` Suraj Jitindar Singh
@ 2017-07-28 10:15 ` Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2017-07-28 10:15 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
Cc: Oliver O'Halloran, joserz, sjitindarsingh
On Thu, 2017-07-27 at 15:35:53 UTC, Oliver O'Halloran wrote:
> The ISA radix translation tree contains two different types of entry,
> directories and leaves. The formats of the two entries are different
> with the directory entries containing no spare bits for use by software.
> 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>
> Tested-by: Laurent Vivier <lvivier@redhat.com>
> Tested-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/c9c98bc5cc2ab4e0d5c9ad58286fa7
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread