Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
On Fri, Aug 11, 2017 at 04:34:19PM +1000, Michael Ellerman wrote:
> Thiago Jung Bauermann writes:
>
> > Ram Pai writes:
> >> --- a/arch/powerpc/kernel/process.c
> >> +++ b/arch/powerpc/kernel/process.c
> >> @@ -42,6 +42,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> #include
> >> #include
> >> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct
> >> *t)
> >>t->tar = mfspr(SPRN_TAR);
> >>}
> >> #endif
> >> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >> + if (arch_pkeys_enabled()) {
> >> + t->amr = mfspr(SPRN_AMR);
> >> + t->iamr = mfspr(SPRN_IAMR);
> >> + t->uamor = mfspr(SPRN_UAMOR);
> >> + }
> >> +#endif
> >> }
> >
> > Is it worth having a flag in thread_struct saying whether it has every
> > called pkey_alloc and only do the mfsprs if it did?
Yes. This will further optimize the code; a great thing!
>
> Yes, in fact there's a programming note in the UAMOR section of the arch
> that says exactly that.
>
> On the write side you have to be a bit more careful. You have to make
> sure you set the UAMOR to 0 when you're switching from a process that
> has used keys to one that isn't.
Currently we save and restore AMR/IAMR/UAMOR if the OS has enabled pkeys.
This means the UAMOR will get restored to 0 if the application has not
used any keys.
But if we do optimize the code further; as suggested by Thiago, we will
have to be careful with initializing UAMOR while switching back task
that has not used the keys yet.
RP
>
> cheers
--
Ram Pai
Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Thiago Jung Bauermann writes:
> Ram Pai writes:
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -42,6 +42,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
>> t->tar = mfspr(SPRN_TAR);
>> }
>> #endif
>> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>> +if (arch_pkeys_enabled()) {
>> +t->amr = mfspr(SPRN_AMR);
>> +t->iamr = mfspr(SPRN_IAMR);
>> +t->uamor = mfspr(SPRN_UAMOR);
>> +}
>> +#endif
>> }
>
> Is it worth having a flag in thread_struct saying whether it has every
> called pkey_alloc and only do the mfsprs if it did?
Yes, in fact there's a programming note in the UAMOR section of the arch
that says exactly that.
On the write side you have to be a bit more careful. You have to make
sure you set the UAMOR to 0 when you're switching from a process that
has used keys to one that isn't.
cheers
Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Ram Pai writes:
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -42,6 +42,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
> t->tar = mfspr(SPRN_TAR);
> }
> #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> + if (arch_pkeys_enabled()) {
> + t->amr = mfspr(SPRN_AMR);
> + t->iamr = mfspr(SPRN_IAMR);
> + t->uamor = mfspr(SPRN_UAMOR);
> + }
> +#endif
> }
Is it worth having a flag in thread_struct saying whether it has every
called pkey_alloc and only do the mfsprs if it did?
> @@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct
> *old_thread,
> mtspr(SPRN_TAR, new_thread->tar);
> }
> #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> + if (arch_pkeys_enabled()) {
> + if (old_thread->amr != new_thread->amr)
> + mtspr(SPRN_AMR, new_thread->amr);
> + if (old_thread->iamr != new_thread->iamr)
> + mtspr(SPRN_IAMR, new_thread->iamr);
> + if (old_thread->uamor != new_thread->uamor)
> + mtspr(SPRN_UAMOR, new_thread->uamor);
> + }
> +#endif
> }
>
> struct task_struct *__switch_to(struct task_struct *prev,
> @@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long
> start, unsigned long sp)
> current->thread.tm_tfiar = 0;
> current->thread.load_tm = 0;
> #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> + if (arch_pkeys_enabled()) {
> + current->thread.amr = 0x0ul;
> + current->thread.iamr = 0x0ul;
> + current->thread.uamor = 0x0ul;
> + }
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> }
> EXPORT_SYMBOL(start_thread);
--
Thiago Jung Bauermann
IBM Linux Technology Center
[RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Store and restore the AMR, IAMR and UAMOR register state of the task
before scheduling out and after scheduling in, respectively.
Signed-off-by: Ram Pai
---
arch/powerpc/include/asm/pkeys.h |5 +
arch/powerpc/include/asm/processor.h |5 +
arch/powerpc/kernel/process.c| 25 +
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 9d59619..dac8751 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -147,6 +147,11 @@ static inline int arch_set_user_pkey_access(struct
task_struct *tsk, int pkey,
return __arch_set_user_pkey_access(tsk, pkey, init_val);
}
+static inline bool arch_pkeys_enabled(void)
+{
+ return pkey_inited;
+}
+
static inline void pkey_mm_init(struct mm_struct *mm)
{
if (!pkey_inited)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index 1189d04..dcb1cf0 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -309,6 +309,11 @@ struct thread_struct {
struct thread_vr_state ckvr_state; /* Checkpointed VR state */
unsigned long ckvrsave; /* Checkpointed VRSAVE */
#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+ unsigned long amr;
+ unsigned long iamr;
+ unsigned long uamor;
+#endif
#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
void* kvm_shadow_vcpu; /* KVM internal data */
#endif /* CONFIG_KVM_BOOK3S_32_HANDLER */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 2ad725e..af373f4 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
t->tar = mfspr(SPRN_TAR);
}
#endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+ if (arch_pkeys_enabled()) {
+ t->amr = mfspr(SPRN_AMR);
+ t->iamr = mfspr(SPRN_IAMR);
+ t->uamor = mfspr(SPRN_UAMOR);
+ }
+#endif
}
static inline void restore_sprs(struct thread_struct *old_thread,
@@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct
*old_thread,
mtspr(SPRN_TAR, new_thread->tar);
}
#endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+ if (arch_pkeys_enabled()) {
+ if (old_thread->amr != new_thread->amr)
+ mtspr(SPRN_AMR, new_thread->amr);
+ if (old_thread->iamr != new_thread->iamr)
+ mtspr(SPRN_IAMR, new_thread->iamr);
+ if (old_thread->uamor != new_thread->uamor)
+ mtspr(SPRN_UAMOR, new_thread->uamor);
+ }
+#endif
}
struct task_struct *__switch_to(struct task_struct *prev,
@@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long
start, unsigned long sp)
current->thread.tm_tfiar = 0;
current->thread.load_tm = 0;
#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+ if (arch_pkeys_enabled()) {
+ current->thread.amr = 0x0ul;
+ current->thread.iamr = 0x0ul;
+ current->thread.uamor = 0x0ul;
+ }
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
}
EXPORT_SYMBOL(start_thread);
--
1.7.1
