[RFC,v6,20/62] powerpc: store and restore the pkey state across context switches

Message ID 1500177424-13695-21-git-send-email-linuxram@us.ibm.com
State RFC
Headers show

Commit Message

Ram Pai July 16, 2017, 3:56 a.m.
Store and restore the AMR, IAMR and UMOR register state of the task
before scheduling out and after scheduling in, respectively.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/processor.h |    5 +++++
 arch/powerpc/kernel/process.c        |   18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

Comments

Thiago Jung Bauermann July 27, 2017, 5:32 p.m. | #1
Ram Pai <linuxram@us.ibm.com> writes:

> Store and restore the AMR, IAMR and UMOR register state of the task
> before scheduling out and after scheduling in, respectively.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>

s/UMOR/UAMOR/

> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 2ad725e..9429361 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1096,6 +1096,11 @@ static inline void save_sprs(struct thread_struct *t)
>  		t->tar = mfspr(SPRN_TAR);
>  	}
>  #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	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 +1136,14 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>  			mtspr(SPRN_TAR, new_thread->tar);
>  	}
>  #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	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
>  }

Shouldn't the saving and restoring of the SPRs be guarded by a check for
whether memory protection keys are enabled? What happens when trying to
access these registers on a CPU which doesn't have them?
Ram Pai July 29, 2017, 11:31 p.m. | #2
On Thu, Jul 27, 2017 at 02:32:59PM -0300, Thiago Jung Bauermann wrote:
> 
> Ram Pai <linuxram@us.ibm.com> writes:
> 
> > Store and restore the AMR, IAMR and UMOR register state of the task
> > before scheduling out and after scheduling in, respectively.
> >
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> 
> s/UMOR/UAMOR/
> 
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 2ad725e..9429361 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1096,6 +1096,11 @@ static inline void save_sprs(struct thread_struct *t)
> >  		t->tar = mfspr(SPRN_TAR);
> >  	}
> >  #endif
> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > +	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 +1136,14 @@ static inline void restore_sprs(struct thread_struct *old_thread,
> >  			mtspr(SPRN_TAR, new_thread->tar);
> >  	}
> >  #endif
> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > +	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
> >  }
> 
> Shouldn't the saving and restoring of the SPRs be guarded by a check for
> whether memory protection keys are enabled? What happens when trying to
> access these registers on a CPU which doesn't have them?

Good point. need to guard it.  However; i think, these registers have been
available since power6.

RP
Michael Ellerman July 31, 2017, 1 p.m. | #3
Ram Pai <linuxram@us.ibm.com> writes:
> On Thu, Jul 27, 2017 at 02:32:59PM -0300, Thiago Jung Bauermann wrote:
>> Ram Pai <linuxram@us.ibm.com> writes:
>> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> > index 2ad725e..9429361 100644
>> > --- a/arch/powerpc/kernel/process.c
>> > +++ b/arch/powerpc/kernel/process.c
>> > @@ -1096,6 +1096,11 @@ static inline void save_sprs(struct thread_struct *t)
>> >  		t->tar = mfspr(SPRN_TAR);
>> >  	}
>> >  #endif
>> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>> > +	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 +1136,14 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>> >  			mtspr(SPRN_TAR, new_thread->tar);
>> >  	}
>> >  #endif
>> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>> > +	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
>> >  }
>> 
>> Shouldn't the saving and restoring of the SPRs be guarded by a check for
>> whether memory protection keys are enabled? What happens when trying to
>> access these registers on a CPU which doesn't have them?
>
> Good point. need to guard it.  However; i think, these registers have been
> available since power6.

The kernel runs on CPUs much older than that.

IAMR was added on Power8.

And performance is also an issue, so we should only switch them when we
need to.

cheers

Patch

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..9429361 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1096,6 +1096,11 @@  static inline void save_sprs(struct thread_struct *t)
 		t->tar = mfspr(SPRN_TAR);
 	}
 #endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	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 +1136,14 @@  static inline void restore_sprs(struct thread_struct *old_thread,
 			mtspr(SPRN_TAR, new_thread->tar);
 	}
 #endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	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 +1702,11 @@  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
+	current->thread.amr   = 0x0ul;
+	current->thread.iamr  = 0x0ul;
+	current->thread.uamor = 0x0ul;
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 }
 EXPORT_SYMBOL(start_thread);