* [PATCH 1/8] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
From: Ram Pai <linuxram@us.ibm.com>
VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
is enabled. Powerpc also needs these bits. Hence lets define the
VM_PKEY_BITx bits for any architecture that enables
CONFIG_ARCH_HAS_PKEYS.
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/pkeys.h | 2 ++
fs/proc/task_mmu.c | 4 ++--
include/linux/mm.h | 9 +++++----
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 0409c80c32c0..18ef59a9886d 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -26,6 +26,8 @@ extern u32 initial_allocation_mask; /* bits set for reserved keys */
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
+#elif !defined(VM_PKEY_BIT4)
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
#endif
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index c486ad4b43f0..541392a62608 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -673,13 +673,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_MERGEABLE)] = "mg",
[ilog2(VM_UFFD_MISSING)]= "um",
[ilog2(VM_UFFD_WP)] = "uw",
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#ifdef CONFIG_ARCH_HAS_PKEYS
/* These come out via ProtectionKey: */
[ilog2(VM_PKEY_BIT0)] = "",
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1ac1f06a4be6..c6a6f2492c1b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -228,15 +228,16 @@ extern unsigned int kobjsize(const void *objp);
#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
-#if defined(CONFIG_X86)
-# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
-#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
+#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
+
+#if defined(CONFIG_X86)
+# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
#elif defined(CONFIG_PPC)
# define VM_SAO VM_ARCH_1 /* Strong Access Ordering (powerpc) */
#elif defined(CONFIG_PARISC)
--
2.14.1
^ permalink raw reply related
* [PATCH 2/8] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
From: Ram Pai <linuxram@us.ibm.com>
Currently only 4bits are allocated in the vma flags to hold 16
keys. This is sufficient for x86. PowerPC supports 32 keys,
which needs 5bits. This patch allocates an additional bit.
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
[mpe: Fold in #if VM_PKEY_BIT4 as noticed by Dave Hansen]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
fs/proc/task_mmu.c | 3 +++
include/linux/mm.h | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 541392a62608..c2163606e6fb 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -679,6 +679,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
+#if VM_PKEY_BIT4
+ [ilog2(VM_PKEY_BIT4)] = "",
+#endif
#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index c6a6f2492c1b..abfd758ff83a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -231,9 +231,10 @@ extern unsigned int kobjsize(const void *objp);
#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
-# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
+# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 /* on x86 and 5-bit value on ppc64 */
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
#endif /* CONFIG_ARCH_HAS_PKEYS */
#if defined(CONFIG_X86)
--
2.14.1
^ permalink raw reply related
* [PATCH 3/8] mm/pkeys: Remove include of asm/mmu_context.h from pkeys.h
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
While trying to unify the pkey handling in show_smap() between x86 and
powerpc we stumbled across various build failures due to the order of
includes between the two arches.
Part of the problem is that linux/pkeys.h includes asm/mmu_context.h,
and the relationship between asm/mmu_context.h and asm/pkeys.h is not
consistent between the two arches.
It would be cleaner if linux/pkeys.h only included asm/pkeys.h,
creating a single integration point for the arch pkey definitions.
So this patch removes the include of asm/mmu_context.h from
linux/pkeys.h.
We can't prove that this is safe in the general case, but it passes
all the build tests I've thrown at it. Also asm/mmu_context.h is
included widely while linux/pkeys.h is not, so most likely any code
that is including linux/pkeys.h is already getting asm/mmu_context.h
from elsewhere.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
include/linux/pkeys.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 0794ca78c379..ed06e1a67bfa 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -3,7 +3,6 @@
#define _LINUX_PKEYS_H
#include <linux/mm_types.h>
-#include <asm/mmu_context.h>
#ifdef CONFIG_ARCH_HAS_PKEYS
#include <asm/pkeys.h>
--
2.14.1
^ permalink raw reply related
* [PATCH 4/8] mm/pkeys, powerpc, x86: Provide an empty vma_pkey() in linux/pkeys.h
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
Consolidate the pkey handling by providing a common empty definition
of vma_pkey() in pkeys.h when CONFIG_ARCH_HAS_PKEYS=n.
This also removes another entanglement of pkeys.h and
asm/mmu_context.h.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/mmu_context.h | 5 -----
arch/x86/include/asm/mmu_context.h | 5 -----
include/linux/pkeys.h | 5 +++++
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 1835ca1505d6..896efa559996 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -250,11 +250,6 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
#define thread_pkey_regs_restore(new_thread, old_thread)
#define thread_pkey_regs_init(thread)
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
-
static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
{
return 0x0UL;
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 57e3785d0d26..3d748bdf44a7 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -296,11 +296,6 @@ static inline int vma_pkey(struct vm_area_struct *vma)
return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
}
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
#endif
/*
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index ed06e1a67bfa..aad54663763b 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -13,6 +13,11 @@
#define PKEY_DEDICATED_EXECUTE_ONLY 0
#define ARCH_VM_PKEY_FLAGS 0
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+ return 0;
+}
+
static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
{
return (pkey == 0);
--
2.14.1
^ permalink raw reply related
* [PATCH 5/8] x86/pkeys: Move vma_pkey() into asm/pkeys.h
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
Move the last remaining pkey helper, vma_pkey() into asm/pkeys.h
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/x86/include/asm/mmu_context.h | 10 ----------
arch/x86/include/asm/pkeys.h | 8 ++++++++
include/linux/pkeys.h | 2 +-
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 3d748bdf44a7..59e1fadef401 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -288,16 +288,6 @@ static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
mpx_notify_unmap(mm, vma, start, end);
}
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
- VM_PKEY_BIT2 | VM_PKEY_BIT3;
-
- return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
-}
-#endif
-
/*
* We only want to enforce protection keys on the current process
* because we effectively have no access to PKRU for other
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ffda0df..0e5f749158e4 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -106,4 +106,12 @@ extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
extern void copy_init_pkru_to_fpregs(void);
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+ unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
+ VM_PKEY_BIT2 | VM_PKEY_BIT3;
+
+ return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
+}
+
#endif /*_ASM_X86_PKEYS_H */
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index aad54663763b..946cb773b79f 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -2,7 +2,7 @@
#ifndef _LINUX_PKEYS_H
#define _LINUX_PKEYS_H
-#include <linux/mm_types.h>
+#include <linux/mm.h>
#ifdef CONFIG_ARCH_HAS_PKEYS
#include <asm/pkeys.h>
--
2.14.1
^ permalink raw reply related
* [PATCH 6/8] x86/pkeys: Add arch_pkeys_enabled()
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
This will be used in future patches to check for arch support for
pkeys in generic code.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/x86/include/asm/pkeys.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index 0e5f749158e4..c1957f8f7c1b 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -7,6 +7,11 @@
extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
+static inline bool arch_pkeys_enabled(void)
+{
+ return boot_cpu_has(X86_FEATURE_OSPKE);
+}
+
/*
* Try to dedicate one of the protection keys to be used as an
* execute-only protection key.
--
2.14.1
^ permalink raw reply related
* [PATCH 7/8] mm/pkeys: Add an empty arch_pkeys_enabled()
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
Add an empty arch_pkeys_enabled() in linux/pkeys.h for the
CONFIG_ARCH_HAS_PKEYS=n case.
Split out of a patch by Ram Pai <linuxram@us.ibm.com>.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
include/linux/pkeys.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 946cb773b79f..2955ba976048 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -39,6 +39,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
return 0;
}
+static inline bool arch_pkeys_enabled(void)
+{
+ return false;
+}
+
static inline void copy_init_pkru_to_fpregs(void)
{
}
--
2.14.1
^ permalink raw reply related
* [PATCH 8/8] mm/pkeys, x86, powerpc: Display pkey in smaps if arch supports pkeys
From: Michael Ellerman @ 2018-05-08 14:59 UTC (permalink / raw)
To: linuxram; +Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-1-mpe@ellerman.id.au>
From: Ram Pai <linuxram@us.ibm.com>
Currently the architecture specific code is expected to display the
protection keys in smap for a given vma. This can lead to redundant
code and possibly to divergent formats in which the key gets
displayed.
This patch changes the implementation. It displays the pkey only if
the architecture support pkeys, i.e arch_pkeys_enabled() returns true.
x86 arch_show_smap() function is not needed anymore, delete it.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
[mpe: Split out from larger patch, rebased on header changes]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/x86/kernel/setup.c | 8 --------
fs/proc/task_mmu.c | 8 +++-----
2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 5c623dfe39d1..2f86d883dd95 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1312,11 +1312,3 @@ static int __init register_kernel_offset_dumper(void)
return 0;
}
__initcall(register_kernel_offset_dumper);
-
-void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
- if (!boot_cpu_has(X86_FEATURE_OSPKE))
- return;
-
- seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
-}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index c2163606e6fb..93cea7b07a80 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -18,6 +18,7 @@
#include <linux/page_idle.h>
#include <linux/shmem_fs.h>
#include <linux/uaccess.h>
+#include <linux/pkeys.h>
#include <asm/elf.h>
#include <asm/tlb.h>
@@ -730,10 +731,6 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
}
#endif /* HUGETLB_PAGE */
-void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
-}
-
#define SEQ_PUT_DEC(str, val) \
seq_put_decimal_ull_width(m, str, (val) >> 10, 8)
static int show_smap(struct seq_file *m, void *v, int is_pid)
@@ -838,7 +835,8 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
seq_puts(m, " kB\n");
}
if (!rollup_mode) {
- arch_show_smap(m, vma);
+ if (arch_pkeys_enabled())
+ seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
show_smap_vma_flags(m, vma);
}
m_cache_vma(m, vma);
--
2.14.1
^ permalink raw reply related
* Re: [PATCH 2/8] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Ram Pai @ 2018-05-08 16:18 UTC (permalink / raw)
To: Michael Ellerman
Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-3-mpe@ellerman.id.au>
On Wed, May 09, 2018 at 12:59:42AM +1000, Michael Ellerman wrote:
> From: Ram Pai <linuxram@us.ibm.com>
>
> Currently only 4bits are allocated in the vma flags to hold 16
> keys. This is sufficient for x86. PowerPC supports 32 keys,
> which needs 5bits. This patch allocates an additional bit.
>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> [mpe: Fold in #if VM_PKEY_BIT4 as noticed by Dave Hansen]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> fs/proc/task_mmu.c | 3 +++
> include/linux/mm.h | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 541392a62608..c2163606e6fb 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -679,6 +679,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> [ilog2(VM_PKEY_BIT1)] = "",
> [ilog2(VM_PKEY_BIT2)] = "",
> [ilog2(VM_PKEY_BIT3)] = "",
> +#if VM_PKEY_BIT4
> + [ilog2(VM_PKEY_BIT4)] = "",
> +#endif
> #endif /* CONFIG_ARCH_HAS_PKEYS */
> };
> size_t i;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c6a6f2492c1b..abfd758ff83a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -231,9 +231,10 @@ extern unsigned int kobjsize(const void *objp);
> #ifdef CONFIG_ARCH_HAS_PKEYS
> # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
> # define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
> -# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
> +# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 /* on x86 and 5-bit value on ppc64 */
> # define VM_PKEY_BIT2 VM_HIGH_ARCH_2
> # define VM_PKEY_BIT3 VM_HIGH_ARCH_3
> +# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
> #endif /* CONFIG_ARCH_HAS_PKEYS */
this has to be:
+#if defined(CONFIG_PPC)
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
+#else
+# define VM_PKEY_BIT4 0
+#endif
^ permalink raw reply
* Re: [PATCH 4/8] mm/pkeys, powerpc, x86: Provide an empty vma_pkey() in linux/pkeys.h
From: Ram Pai @ 2018-05-08 16:27 UTC (permalink / raw)
To: Michael Ellerman
Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel, dave.hansen
In-Reply-To: <20180508145948.9492-5-mpe@ellerman.id.au>
On Wed, May 09, 2018 at 12:59:44AM +1000, Michael Ellerman wrote:
> Consolidate the pkey handling by providing a common empty definition
> of vma_pkey() in pkeys.h when CONFIG_ARCH_HAS_PKEYS=n.
>
> This also removes another entanglement of pkeys.h and
> asm/mmu_context.h.
>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/mmu_context.h | 5 -----
> arch/x86/include/asm/mmu_context.h | 5 -----
> include/linux/pkeys.h | 5 +++++
> 3 files changed, 5 insertions(+), 10 deletions(-)
RP
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Ram Pai @ 2018-05-08 16:38 UTC (permalink / raw)
To: Michal Suchánek
Cc: fweimer, Thomas Gleixner, Ulrich.Weigand, mhocko, Dave Hansen,
paulus, aneesh.kumar, bauerman, Andrew Morton, linuxppc-dev,
Ingo Molnar
In-Reply-To: <20180507132149.3fc1dcc5@kitsune.suse.cz>
On Mon, May 07, 2018 at 01:21:49PM +0200, Michal Suchánek wrote:
> On Sun, 6 May 2018 13:10:43 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
>
> > On Sat, May 05, 2018 at 02:39:56PM +0200, Michal Suchánek wrote:
> > > On Fri, 4 May 2018 14:45:07 -0700
> > > Ram Pai <linuxram@us.ibm.com> wrote:
> > >
> > > > On Fri, May 04, 2018 at 02:31:10PM -0700, Dave Hansen wrote:
> > > > > On 05/04/2018 02:26 PM, Michal Suchánek wrote:
> > > > > > If it is not ok to change permissions of pkey 0 is it ok to
> > > > > > free it?
> > > > >
> > > > > It's pretty much never OK to free it on x86 or ppc. But, we're
> > > > > not going to put code in to keep userspace from shooting itself
> > > > > in the foot, at least on x86.
> > > >
> > > > and on powerpc aswell.
> > >
> > > But once it's free it can be re-allocated. So you are moving the
> > > special-casing from free code to code dealing with allocation.
> >
> > Actually if an application frees key-0, it has potentially opened up a
> > can-of-worms. It could step on anything that explodes.
> >
> > Its choice between imposing policies on an application v/s freeing it
> > up to choose its own policy. I think the kernel should impose some
> > form of mild-policy. But others think there should be none.
>
> It is a problem of the application when it frees a key it still needs.
> I am not for or against allowing that. The problem is with the
> interface change once the key is freed.
>
> >
> > >
> > > If you want something like allocate_exec_only_pkey then the function
> > > (either in kernel or in userspace) needs to make sure it is not
> > > getting/requesting key 0 on powerpc.
> >
> > Yes. makes sense. I will put in some checks towards that.
>
> Suppose an application is adapted to take advantage of freeing key
> 0, perhaps to revoke access to any code and data used at runtime
> initialization which is not longer needed.
>
> As I understand it with the proposed change any address range not
> associated with non-default key becomes inaccessible and key 0 becomes
> available for allocation again.
The above sentence is difficult to parse. I think what you are saying is --
Any address range associated with key-0 become inaccessible if key-0
is freed. Is that a correct simplification of the above
statement?
Assuming I got it right --
Yes. key-0 when freed becomes available for allocation again.
No. When key-0 is freed any address range
associated with key-0 will continue to be accessible. It was accessible
to begin with and will continue to be accessible. It was accessible,
because kernel; during task-creation, never disables any permissions on
key-0, and also never lets userspace disable any permissions on
key-0.
The problem arises, when key-0 gets reallocated at a later time. If the
key-0 at that point(during reallocation) is treated like any other key;
allowing userspace to change its permissions, it can explode on
access to any page associated with key-0. (BTW: almost everything is
associated with key-0 by default).
> This is fine on x86 where key 0 is fully functional.
>
> However, on powerpc the application now has key 0 available and it is
> not fully a fully functional key. So the application now needs to check
> that the key it is allocating is not key 0. Otherwise further key
> operations may unexpectedly fail.
>
> To prevent this I would suggest
>
> a) do not allow allocating key 0. If it is freed it becomes reserved
Certainly yes. We never promised any particular key to the user space. So
not returning key-0 during allocation should break no semantics.
>
> b) never expose key 0 to applications. Allocate key 2 as the default
> key and present the key numbers with an offset to userspace so key 2
> appears as key 0 to user applications.
this is complicated. offseting the number is not easy. if kernel
has allocated key-2 and tells the user space to use key-0, userspace
will use bit-0 and bit-1 of the AMR register to program the permissions
on the key, and cpu will start applying those changes on key-0.
Basically kernel, userspace and cpu will be out-of-sync. This is
possible if the userspace uses a shift-aware library function to program
the AMR. BTW: A library function integrated into glibc is yet to be
defined AFAICK.
>
> Thanks
>
> Michal
--
Ram Pai
^ permalink raw reply
* Re: [PATCH 4/8] mm/pkeys, powerpc, x86: Provide an empty vma_pkey() in linux/pkeys.h
From: Dave Hansen @ 2018-05-08 16:45 UTC (permalink / raw)
To: Michael Ellerman, linuxram
Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel
In-Reply-To: <20180508145948.9492-5-mpe@ellerman.id.au>
On 05/08/2018 07:59 AM, Michael Ellerman wrote:
> Consolidate the pkey handling by providing a common empty definition
> of vma_pkey() in pkeys.h when CONFIG_ARCH_HAS_PKEYS=n.
>
> This also removes another entanglement of pkeys.h and
> asm/mmu_context.h.
Looks fine to me. Thanks for consolidating these.
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
^ permalink raw reply
* Re: [PATCH 5/8] x86/pkeys: Move vma_pkey() into asm/pkeys.h
From: Dave Hansen @ 2018-05-08 16:45 UTC (permalink / raw)
To: Michael Ellerman, linuxram
Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel
In-Reply-To: <20180508145948.9492-6-mpe@ellerman.id.au>
On 05/08/2018 07:59 AM, Michael Ellerman wrote:
> Move the last remaining pkey helper, vma_pkey() into asm/pkeys.h
Fine with me, as long as it compiles. :)
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
^ permalink raw reply
* Re: [PATCH 8/8] mm/pkeys, x86, powerpc: Display pkey in smaps if arch supports pkeys
From: Dave Hansen @ 2018-05-08 16:47 UTC (permalink / raw)
To: Michael Ellerman, linuxram
Cc: mingo, linuxppc-dev, linux-mm, x86, linux-kernel
In-Reply-To: <20180508145948.9492-9-mpe@ellerman.id.au>
On 05/08/2018 07:59 AM, Michael Ellerman wrote:
> Currently the architecture specific code is expected to display the
> protection keys in smap for a given vma. This can lead to redundant
> code and possibly to divergent formats in which the key gets
> displayed.
>
> This patch changes the implementation. It displays the pkey only if
> the architecture support pkeys, i.e arch_pkeys_enabled() returns true.
For this, along with 6/8 and 7/8:
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Michal Suchánek @ 2018-05-08 17:03 UTC (permalink / raw)
To: Ram Pai
Cc: fweimer, Dave Hansen, Ingo Molnar, mhocko, Ulrich.Weigand, paulus,
aneesh.kumar, bauerman, Thomas Gleixner, linuxppc-dev,
Andrew Morton
In-Reply-To: <20180508163801.GC5474@ram.oc3035372033.ibm.com>
On Tue, 8 May 2018 09:38:01 -0700
Ram Pai <linuxram@us.ibm.com> wrote:
> On Mon, May 07, 2018 at 01:21:49PM +0200, Michal Such=C3=A1nek wrote:
> > On Sun, 6 May 2018 13:10:43 -0700
> > Ram Pai <linuxram@us.ibm.com> wrote:
> > =20
> > > On Sat, May 05, 2018 at 02:39:56PM +0200, Michal Such=C3=A1nek wrote:=
=20
> > > > On Fri, 4 May 2018 14:45:07 -0700
> > > > Ram Pai <linuxram@us.ibm.com> wrote:
...
> > Suppose an application is adapted to take advantage of freeing key
> > 0, perhaps to revoke access to any code and data used at runtime
> > initialization which is not longer needed.=20
> >=20
> > As I understand it with the proposed change any address range not
> > associated with non-default key becomes inaccessible and key 0
> > becomes available for allocation again. =20
>=20
> The above sentence is difficult to parse. I think what you are saying
> is -- Any address range associated with key-0 become inaccessible if
> key-0 is freed. Is that a correct simplification of the above
> statement?
>=20
> Assuming I got it right -- =20
>=20
> Yes. key-0 when freed becomes available for allocation again.
> No. When key-0 is freed any address range
> associated with key-0 will continue to be accessible. It was
> accessible to begin with and will continue to be accessible. It was
> accessible, because kernel; during task-creation, never disables any
> permissions on key-0, and also never lets userspace disable any
> permissions on key-0.
Oh, right. The result of freeing a used key is not defined. When it is
not defined expecting a sane result is too much.
>=20
> The problem arises, when key-0 gets reallocated at a later time. If
> the key-0 at that point(during reallocation) is treated like any
> other key; allowing userspace to change its permissions, it can
> explode on access to any page associated with key-0. (BTW: almost
> everything is associated with key-0 by default).
>=20
> > This is fine on x86 where key 0 is fully functional. =20
>=20
> >=20
> > However, on powerpc the application now has key 0 available and it
> > is not fully a fully functional key. So the application now needs
> > to check that the key it is allocating is not key 0. Otherwise
> > further key operations may unexpectedly fail.
> >=20
> > To prevent this I would suggest
> >=20
> > a) do not allow allocating key 0. If it is freed it becomes
> > reserved =20
>=20
> Certainly yes. We never promised any particular key to the user
> space. So not returning key-0 during allocation should break no
> semantics.
>=20
> >=20
> > b) never expose key 0 to applications. Allocate key 2 as the default
> > key and present the key numbers with an offset to userspace so key 2
> > appears as key 0 to user applications. =20
>=20
> this is complicated. offseting the number is not easy. if kernel
> has allocated key-2 and tells the user space to use key-0, userspace
> will use bit-0 and bit-1 of the AMR register to program the
> permissions on the key, and cpu will start applying those changes on
> key-0. Basically kernel, userspace and cpu will be out-of-sync. This
> is possible if the userspace uses a shift-aware library function to
> program the AMR. BTW: A library function integrated into glibc is yet
> to be defined AFAICK.
If userspace can program the hardware directly then it is not possible
to lie about the key numbers, unfortunately. An alternative is to
reserve a different key for the kernel so applications get default key
as 0 and can set permissions on it on any platform.
How is the application denied setting the permissions on key 0 if it
can program the register directly?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v3] powerpc, pkey: make protection key 0 less special
From: Ram Pai @ 2018-05-08 18:38 UTC (permalink / raw)
To: Michal Suchánek
Cc: fweimer, Dave Hansen, Ingo Molnar, mhocko, Ulrich.Weigand, paulus,
aneesh.kumar, bauerman, Thomas Gleixner, linuxppc-dev,
Andrew Morton
In-Reply-To: <20180508190336.361a508b@naga.suse.cz>
On Tue, May 08, 2018 at 07:03:36PM +0200, Michal Suchánek wrote:
>
> How is the application denied setting the permissions on key 0 if it
> can program the register directly?
There is a UAMOR register. The userspace can change the permissions of a
given key; by modifying the bits in AMOR register, only if the kernel
has set the corresponding bit in the UAMOR register. For key-0 the
kernel does not set the corresponding UAMOR bit.
RP
^ permalink raw reply
* Re: [PATCH v3] ppc64le livepatch: implement reliable stacktrace for newer consistency models
From: Josh Poimboeuf @ 2018-05-09 0:14 UTC (permalink / raw)
To: Torsten Duwe
Cc: Michael Ellerman, Jiri Kosina, linuxppc-dev, linux-kernel,
Nicholas Piggin, live-patching
In-Reply-To: <20180508103832.085378b9@blackhole.lan>
On Tue, May 08, 2018 at 10:38:32AM +0200, Torsten Duwe wrote:
> On Mon, 7 May 2018 10:42:08 -0500
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> > The subject doesn't actively describe what the patch does, maybe
> > change it to something like:
> >
> > powerpc: Add support for HAVE_RELIABLE_STACKTRACE
> >
> > or maybe
> >
> > powerpc: Add support for livepatch consistency model
>
> Maybe $SUBJECT? You're absolutely right, the old subject was just a
> leftover of my original attempt to just set the flag, before Miroslav
> corrected me. I just kept on copying it without a second thought. Thanks
> for noting.
Generally we refer to it as "the consistency model". So I would
propose a minor edit:
ppc64le/livepatch: Implement reliable stack tracing for the consistency model
> > Otherwise it looks great to me.
> >
> > Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
> >
> Thanks.
>
> Torsten
>
--
Josh
^ permalink raw reply
* [PATCH v3 1/7] powerpc: Add TIDR CPU feature for POWER9
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
This patch adds a CPU feature bit to show whether the CPU has
the TIDR register available, enabling as_notify/wait in userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
arch/powerpc/include/asm/cputable.h | 3 ++-
arch/powerpc/kernel/dt_cpu_ftrs.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 4e332f3531c5..54c4cbbe57b4 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -215,6 +215,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_TM_HV_ASSIST LONG_ASM_CONST(0x0000100000000000)
#define CPU_FTR_P9_TM_XER_SO_BUG LONG_ASM_CONST(0x0000200000000000)
#define CPU_FTR_P9_TLBIE_BUG LONG_ASM_CONST(0x0000400000000000)
+#define CPU_FTR_P9_TIDR LONG_ASM_CONST(0x0000800000000000)
#ifndef __ASSEMBLY__
@@ -462,7 +463,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
- CPU_FTR_P9_TLBIE_BUG)
+ CPU_FTR_P9_TLBIE_BUG | CPU_FTR_P9_TIDR)
#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
(~CPU_FTR_SAO))
#define CPU_FTRS_POWER9_DD2_0 CPU_FTRS_POWER9
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 11a3a4fed3fb..10f8b7f55637 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -722,6 +722,7 @@ static __init void cpufeatures_cpu_quirks(void)
if ((version & 0xffff0000) == 0x004e0000) {
cur_cpu_spec->cpu_features &= ~(CPU_FTR_DAWR);
cur_cpu_spec->cpu_features |= CPU_FTR_P9_TLBIE_BUG;
+ cur_cpu_spec->cpu_features |= CPU_FTR_P9_TIDR;
}
}
--
2.14.3
^ permalink raw reply related
* [PATCH v3 2/7] powerpc: Use TIDR CPU feature to control TIDR allocation
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Switch the use of TIDR on it's CPU feature, rather than assuming it
is available based on architecture.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
arch/powerpc/kernel/process.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1237f13fed51..3b00da47699b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1154,7 +1154,7 @@ static inline void restore_sprs(struct thread_struct *old_thread,
mtspr(SPRN_TAR, new_thread->tar);
}
- if (cpu_has_feature(CPU_FTR_ARCH_300) &&
+ if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
old_thread->tidr != new_thread->tidr)
mtspr(SPRN_TIDR, new_thread->tidr);
#endif
@@ -1570,7 +1570,7 @@ void clear_thread_tidr(struct task_struct *t)
if (!t->thread.tidr)
return;
- if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (!cpu_has_feature(CPU_FTR_P9_TIDR)) {
WARN_ON_ONCE(1);
return;
}
@@ -1593,7 +1593,7 @@ int set_thread_tidr(struct task_struct *t)
{
int rc;
- if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ if (!cpu_has_feature(CPU_FTR_P9_TIDR))
return -EINVAL;
if (t != current)
--
2.14.3
^ permalink raw reply related
* [PATCH v3 4/7] ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The function removes the process element from NPU cache.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
arch/powerpc/include/asm/pnv-ocxl.h | 2 +-
arch/powerpc/platforms/powernv/ocxl.c | 4 ++--
drivers/misc/ocxl/link.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index f6945d3bc971..208b5503f4ed 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -28,7 +28,7 @@ extern int pnv_ocxl_map_xsl_regs(struct pci_dev *dev, void __iomem **dsisr,
extern int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask,
void **platform_data);
extern void pnv_ocxl_spa_release(void *platform_data);
-extern int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle);
+extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
extern void pnv_ocxl_free_xive_irq(u32 irq);
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index fa9b53af3c7b..8c65aacda9c8 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -475,7 +475,7 @@ void pnv_ocxl_spa_release(void *platform_data)
}
EXPORT_SYMBOL_GPL(pnv_ocxl_spa_release);
-int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle)
+int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
{
struct spa_data *data = (struct spa_data *) platform_data;
int rc;
@@ -483,7 +483,7 @@ int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle)
rc = opal_npu_spa_clear_cache(data->phb_opal_id, data->bdfn, pe_handle);
return rc;
}
-EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe);
+EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe_from_cache);
int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
{
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index f30790582dc0..656e8610eec2 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -599,7 +599,7 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
* On powerpc, the entry needs to be cleared from the context
* cache of the NPU.
*/
- rc = pnv_ocxl_spa_remove_pe(link->platform_data, pe_handle);
+ rc = pnv_ocxl_spa_remove_pe_from_cache(link->platform_data, pe_handle);
WARN_ON(rc);
pe_data = radix_tree_delete(&spa->pe_tree, pe_handle);
--
2.14.3
^ permalink raw reply related
* [PATCH v3 0/7] ocxl: Implement Power9 as_notify/wait for OpenCAPI
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180417020950.21446-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The Power 9 as_notify/wait feature provides a lower latency way to
signal a thread that work is complete. This series enables the use of
this feature from OpenCAPI adapters, as well as addressing a potential
starvation issue when allocating thread IDs.
Changelog:
v3:
Fix references to POWER9
Remove stray whitespace edit from docs
Add more details to commit message for "use task_pid_nr()"
Retitle patch 6 to indicate OCXL rather than CPU features
v2:
Rename get_platform IOCTL to get_features
Move stray edit from patch 1 to patch 3
Alastair D'Silva (7):
powerpc: Add TIDR CPU feature for POWER9
powerpc: Use TIDR CPU feature to control TIDR allocation
powerpc: use task_pid_nr() for TID allocation
ocxl: Rename pnv_ocxl_spa_remove_pe to clarify it's action
ocxl: Expose the thread_id needed for wait on POWER9
ocxl: Add an IOCTL so userspace knows what OCXL features are available
ocxl: Document new OCXL IOCTLs
Documentation/accelerators/ocxl.rst | 11 ++++
arch/powerpc/include/asm/cputable.h | 3 +-
arch/powerpc/include/asm/pnv-ocxl.h | 2 +-
arch/powerpc/include/asm/switch_to.h | 1 -
arch/powerpc/kernel/dt_cpu_ftrs.c | 1 +
arch/powerpc/kernel/process.c | 101 +---------------------------------
arch/powerpc/platforms/powernv/ocxl.c | 4 +-
drivers/misc/ocxl/context.c | 5 +-
drivers/misc/ocxl/file.c | 78 ++++++++++++++++++++++++++
drivers/misc/ocxl/link.c | 38 ++++++++++++-
drivers/misc/ocxl/ocxl_internal.h | 1 +
include/misc/ocxl.h | 9 +++
include/uapi/misc/ocxl.h | 14 +++++
13 files changed, 163 insertions(+), 105 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH v3 3/7] powerpc: use task_pid_nr() for TID allocation
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The current implementation of TID allocation, using a global IDR, may
result in an errant process starving the system of available TIDs.
Instead, use task_pid_nr(), as mentioned by the original author. The
scenario described which prevented it's use is not applicable, as
set_thread_tidr can only be called after the task struct has been
populated.
In the unlikely event that 2 threads share the TID and are waiting,
one of the following cases will happen:
1. The correct thread is running, the wrong thread is not
In this situation, the correct thread is woken and proceeds to pass it's
condition check.
2. Neither threads are running
In this situation, neither thread will be woken. When scheduled, the waiting
threads will execute either a wait, which will return immediately, followed
by a condition check, which will pass for the correct thread and fail
for the wrong thread, or they will execute the condition check immediately.
3. The wrong thread is running, the correct thread is not
The wrong thread will be woken, but will fail it's condition check and
re-execute wait. The correct thread, when scheduled, will execute either
it's condition check (which will pass), or wait, which returns immediately
when called the first time after the thread is scheduled, followed by it's
condition check (which will pass).
4. Both threads are running
Both threads will be woken. The wrong thread will fail it's condition check
and execute another wait, while the correct thread will pass it's condition
check.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
arch/powerpc/include/asm/switch_to.h | 1 -
arch/powerpc/kernel/process.c | 97 +-----------------------------------
2 files changed, 1 insertion(+), 97 deletions(-)
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index be8c9fa23983..5b03d8a82409 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -94,6 +94,5 @@ static inline void clear_task_ebb(struct task_struct *t)
extern int set_thread_uses_vas(void);
extern int set_thread_tidr(struct task_struct *t);
-extern void clear_thread_tidr(struct task_struct *t);
#endif /* _ASM_POWERPC_SWITCH_TO_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 3b00da47699b..87f047fd2762 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1496,103 +1496,12 @@ int set_thread_uses_vas(void)
}
#ifdef CONFIG_PPC64
-static DEFINE_SPINLOCK(vas_thread_id_lock);
-static DEFINE_IDA(vas_thread_ida);
-
-/*
- * We need to assign a unique thread id to each thread in a process.
- *
- * This thread id, referred to as TIDR, and separate from the Linux's tgid,
- * is intended to be used to direct an ASB_Notify from the hardware to the
- * thread, when a suitable event occurs in the system.
- *
- * One such event is a "paste" instruction in the context of Fast Thread
- * Wakeup (aka Core-to-core wake up in the Virtual Accelerator Switchboard
- * (VAS) in POWER9.
- *
- * To get a unique TIDR per process we could simply reuse task_pid_nr() but
- * the problem is that task_pid_nr() is not yet available copy_thread() is
- * called. Fixing that would require changing more intrusive arch-neutral
- * code in code path in copy_process()?.
- *
- * Further, to assign unique TIDRs within each process, we need an atomic
- * field (or an IDR) in task_struct, which again intrudes into the arch-
- * neutral code. So try to assign globally unique TIDRs for now.
- *
- * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
- * For now, only threads that expect to be notified by the VAS
- * hardware need a TIDR value and we assign values > 0 for those.
- */
-#define MAX_THREAD_CONTEXT ((1 << 16) - 1)
-static int assign_thread_tidr(void)
-{
- int index;
- int err;
- unsigned long flags;
-
-again:
- if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
- return -ENOMEM;
-
- spin_lock_irqsave(&vas_thread_id_lock, flags);
- err = ida_get_new_above(&vas_thread_ida, 1, &index);
- spin_unlock_irqrestore(&vas_thread_id_lock, flags);
-
- if (err == -EAGAIN)
- goto again;
- else if (err)
- return err;
-
- if (index > MAX_THREAD_CONTEXT) {
- spin_lock_irqsave(&vas_thread_id_lock, flags);
- ida_remove(&vas_thread_ida, index);
- spin_unlock_irqrestore(&vas_thread_id_lock, flags);
- return -ENOMEM;
- }
-
- return index;
-}
-
-static void free_thread_tidr(int id)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&vas_thread_id_lock, flags);
- ida_remove(&vas_thread_ida, id);
- spin_unlock_irqrestore(&vas_thread_id_lock, flags);
-}
-
-/*
- * Clear any TIDR value assigned to this thread.
- */
-void clear_thread_tidr(struct task_struct *t)
-{
- if (!t->thread.tidr)
- return;
-
- if (!cpu_has_feature(CPU_FTR_P9_TIDR)) {
- WARN_ON_ONCE(1);
- return;
- }
-
- mtspr(SPRN_TIDR, 0);
- free_thread_tidr(t->thread.tidr);
- t->thread.tidr = 0;
-}
-
-void arch_release_task_struct(struct task_struct *t)
-{
- clear_thread_tidr(t);
-}
-
/*
* Assign a unique TIDR (thread id) for task @t and set it in the thread
* structure. For now, we only support setting TIDR for 'current' task.
*/
int set_thread_tidr(struct task_struct *t)
{
- int rc;
-
if (!cpu_has_feature(CPU_FTR_P9_TIDR))
return -EINVAL;
@@ -1602,11 +1511,7 @@ int set_thread_tidr(struct task_struct *t)
if (t->thread.tidr)
return 0;
- rc = assign_thread_tidr();
- if (rc < 0)
- return rc;
-
- t->thread.tidr = rc;
+ t->thread.tidr = (u16)task_pid_nr(t);
mtspr(SPRN_TIDR, t->thread.tidr);
return 0;
--
2.14.3
^ permalink raw reply related
* [PATCH v3 7/7] ocxl: Document new OCXL IOCTLs
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Documentation/accelerators/ocxl.rst | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index ddcc58d01cfb..14cefc020e2d 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -157,6 +157,17 @@ OCXL_IOCTL_GET_METADATA:
Obtains configuration information from the card, such at the size of
MMIO areas, the AFU version, and the PASID for the current context.
+OCXL_IOCTL_ENABLE_P9_WAIT:
+
+ Allows the AFU to wake a userspace thread executing 'wait'. Returns
+ information to userspace to allow it to configure the AFU. Note that
+ this is only available on POWER9.
+
+OCXL_IOCTL_GET_FEATURES:
+
+ Reports on which CPU features that affect OpenCAPI are usable from
+ userspace.
+
mmap
----
--
2.14.3
^ permalink raw reply related
* [PATCH v3 6/7] ocxl: Add an IOCTL so userspace knows what OCXL features are available
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
In order for a userspace AFU driver to call the POWER9 specific
OCXL_IOCTL_ENABLE_P9_WAIT, it needs to verify that it can actually
make that call.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 25 +++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 4 ++++
2 files changed, 29 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index eb409a469f21..33ae46ce0a8a 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -168,12 +168,32 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
}
#endif
+
+static long afu_ioctl_get_features(struct ocxl_context *ctx,
+ struct ocxl_ioctl_features __user *uarg)
+{
+ struct ocxl_ioctl_features arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+#ifdef CONFIG_PPC64
+ if (cpu_has_feature(CPU_FTR_P9_TIDR))
+ arg.flags[0] |= OCXL_IOCTL_FEATURES_FLAGS0_P9_WAIT;
+#endif
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
x == OCXL_IOCTL_ENABLE_P9_WAIT ? "ENABLE_P9_WAIT" : \
+ x == OCXL_IOCTL_GET_FEATURES ? "GET_FEATURES" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -239,6 +259,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
break;
#endif
+ case OCXL_IOCTL_GET_FEATURES:
+ rc = afu_ioctl_get_features(ctx,
+ (struct ocxl_ioctl_features __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 8d2748e69c84..bb80f294b429 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -55,6 +55,9 @@ struct ocxl_ioctl_p9_wait {
__u64 reserved3[3];
};
+#define OCXL_IOCTL_FEATURES_FLAGS0_P9_WAIT 0x01
+struct ocxl_ioctl_features {
+ __u64 flags[4];
};
struct ocxl_ioctl_irq_fd {
@@ -72,5 +75,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_metadata)
#define OCXL_IOCTL_ENABLE_P9_WAIT _IOR(OCXL_MAGIC, 0x15, struct ocxl_ioctl_p9_wait)
+#define OCXL_IOCTL_GET_FEATURES _IOR(OCXL_MAGIC, 0x16, struct ocxl_ioctl_platform)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* [PATCH v3 5/7] ocxl: Expose the thread_id needed for wait on POWER9
From: Alastair D'Silva @ 2018-05-09 0:42 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, linux-doc, mikey, vaibhav, aneesh.kumar, malat,
felix, pombredanne, sukadev, npiggin, gregkh, arnd,
andrew.donnellan, fbarrat, corbet, Alastair D'Silva
In-Reply-To: <20180509004212.4506-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
In order to successfully issue as_notify, an AFU needs to know the TID
to notify, which in turn means that this information should be
available in userspace so it can be communicated to the AFU.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/context.c | 5 +++-
drivers/misc/ocxl/file.c | 53 +++++++++++++++++++++++++++++++++++++++
drivers/misc/ocxl/link.c | 36 ++++++++++++++++++++++++++
drivers/misc/ocxl/ocxl_internal.h | 1 +
include/misc/ocxl.h | 9 +++++++
include/uapi/misc/ocxl.h | 10 ++++++++
6 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 909e8807824a..95f74623113e 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -34,6 +34,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
mutex_init(&ctx->xsl_error_lock);
mutex_init(&ctx->irq_lock);
idr_init(&ctx->irq_idr);
+ ctx->tidr = 0;
+
/*
* Keep a reference on the AFU to make sure it's valid for the
* duration of the life of the context
@@ -65,6 +67,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
{
int rc;
+ // Locks both status & tidr
mutex_lock(&ctx->status_mutex);
if (ctx->status != OPENED) {
rc = -EIO;
@@ -72,7 +75,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
}
rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid,
- current->mm->context.id, 0, amr, current->mm,
+ current->mm->context.id, ctx->tidr, amr, current->mm,
xsl_fault_error, ctx);
if (rc)
goto out;
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 038509e5d031..eb409a469f21 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -5,6 +5,8 @@
#include <linux/sched/signal.h>
#include <linux/uaccess.h>
#include <uapi/misc/ocxl.h>
+#include <asm/reg.h>
+#include <asm/switch_to.h>
#include "ocxl_internal.h"
@@ -123,11 +125,55 @@ static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
return 0;
}
+#ifdef CONFIG_PPC64
+static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
+ struct ocxl_ioctl_p9_wait __user *uarg)
+{
+ struct ocxl_ioctl_p9_wait arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ if (cpu_has_feature(CPU_FTR_P9_TIDR)) {
+ enum ocxl_context_status status;
+
+ // Locks both status & tidr
+ mutex_lock(&ctx->status_mutex);
+ if (!ctx->tidr) {
+ if (set_thread_tidr(current))
+ return -ENOENT;
+
+ ctx->tidr = current->thread.tidr;
+ }
+
+ status = ctx->status;
+ mutex_unlock(&ctx->status_mutex);
+
+ if (status == ATTACHED) {
+ int rc;
+ struct link *link = ctx->afu->fn->link;
+
+ rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
+ if (rc)
+ return rc;
+ }
+
+ arg.thread_id = ctx->tidr;
+ } else
+ return -ENOENT;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+#endif
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
+ x == OCXL_IOCTL_ENABLE_P9_WAIT ? "ENABLE_P9_WAIT" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -186,6 +232,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
(struct ocxl_ioctl_metadata __user *) args);
break;
+#ifdef CONFIG_PPC64
+ case OCXL_IOCTL_ENABLE_P9_WAIT:
+ rc = afu_ioctl_enable_p9_wait(ctx,
+ (struct ocxl_ioctl_p9_wait __user *) args);
+ break;
+#endif
+
default:
rc = -EINVAL;
}
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 656e8610eec2..88876ae8f330 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -544,6 +544,42 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
}
EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
+{
+ struct link *link = (struct link *) link_handle;
+ struct spa *spa = link->spa;
+ struct ocxl_process_element *pe;
+ int pe_handle, rc;
+
+ if (pasid > SPA_PASID_MAX)
+ return -EINVAL;
+
+ pe_handle = pasid & SPA_PE_MASK;
+ pe = spa->spa_mem + pe_handle;
+
+ mutex_lock(&spa->spa_lock);
+
+ pe->tid = tid;
+
+ /*
+ * The barrier makes sure the PE is updated
+ * before we clear the NPU context cache below, so that the
+ * old PE cannot be reloaded erroneously.
+ */
+ mb();
+
+ /*
+ * hook to platform code
+ * On powerpc, the entry needs to be cleared from the context
+ * cache of the NPU.
+ */
+ rc = pnv_ocxl_spa_remove_pe_from_cache(link->platform_data, pe_handle);
+ WARN_ON(rc);
+
+ mutex_unlock(&spa->spa_lock);
+ return rc;
+}
+
int ocxl_link_remove_pe(void *link_handle, int pasid)
{
struct link *link = (struct link *) link_handle;
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 5d421824afd9..a32f2151029f 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -77,6 +77,7 @@ struct ocxl_context {
struct ocxl_xsl_error xsl_error;
struct mutex irq_lock;
struct idr irq_idr;
+ u16 tidr; // Thread ID used for P9 wait implementation
};
struct ocxl_process_element {
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 51ccf76db293..9ff6ddc28e22 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -188,6 +188,15 @@ extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
+/**
+ * Update values within a Process Element
+ *
+ * link_handle: the link handle associated with the process element
+ * pasid: the PASID for the AFU context
+ * tid: the new thread id for the process element
+ */
+extern int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+
/*
* Remove a Process Element from the Shared Process Area for a link
*/
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 0af83d80fb3e..8d2748e69c84 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -48,6 +48,15 @@ struct ocxl_ioctl_metadata {
__u64 reserved[13]; // Total of 16*u64
};
+struct ocxl_ioctl_p9_wait {
+ __u16 thread_id; // The thread ID required to wake this thread
+ __u16 reserved1;
+ __u32 reserved2;
+ __u64 reserved3[3];
+};
+
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -62,5 +71,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_metadata)
+#define OCXL_IOCTL_ENABLE_P9_WAIT _IOR(OCXL_MAGIC, 0x15, struct ocxl_ioctl_p9_wait)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox