Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation

On Tue, 13 Jun 2017 20:04:27 +1000
Michael Ellerman [off-list ref] wrote:

Vaidyanathan Srinivasan [off-list ref] writes:

quoted

* Nicholas Piggin [off-list ref] [2017-06-12 09:58:34]:
 

quoted

The CTRL register is read-only except bit 63 which is the run latch
control. This means it can be updated with a mtspr rather than
mfspr/mtspr.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>  

quoted

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index baae104b16c7..a44ea034c226 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c

@@ -1973,13 +1969,9 @@ void notrace __ppc64_runlatch_on(void)
 void notrace __ppc64_runlatch_off(void)
 {
 	struct thread_info *ti = current_thread_info();
-	unsigned long ctrl;

 	ti->local_flags &= ~_TLF_RUNLATCH;
-
-	ctrl = mfspr(SPRN_CTRLF);
-	ctrl &= ~CTRL_RUNLATCH;
-	mtspr(SPRN_CTRLT, ctrl);
+	mtspr(SPRN_CTRLT, 0);  

Good idea.  Writing to CTRL register can change only the RUN field.
Was this any different in older generations?  

No AFAICS back to 2.02.

quoted

Anton and Ben kept the mfspr/mtspr part in earlier updates to this
routine.  

Doing the read/modify write is forward compatible vs a new writable
field, whereas writing the whole register with a known value is not.

Can we call that an incompatible arch change and not worry about
it? ISA says we may expect TS (read-only) field to expand, but I
guess they could shoehorn something else in there.