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 Bauermannwrites: > > > 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 Bauermannwrites: > 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 Paiwrites: > --- 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