Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
Benjamin Herrenschmidt writes: > On Tue, 2017-06-13 at 20:04 +1000, Michael Ellerman wrote: >> > 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. >> >> > 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. > > At this stage I wouldn't worry too much about it. What we can do is > write a pre-cooked value (from reading it earlier once at boot) if we > are paranoid or just do what Nick does and put the onus on future > designs that might want to re-use it for other things to add a mode > bits to configure the new feature in. Fine by me. cheers
Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
On Tue, 2017-06-13 at 20:04 +1000, Michael Ellerman wrote: > > 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. > > > 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. At this stage I wouldn't worry too much about it. What we can do is write a pre-cooked value (from reading it earlier once at boot) if we are paranoid or just do what Nick does and put the onus on future designs that might want to re-use it for other things to add a mode bits to configure the new feature in. Ben.
Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
On Tue, 13 Jun 2017 20:04:27 +1000
Michael Ellerman wrote:
> Vaidyanathan Srinivasan writes:
> > * Nicholas Piggin [2017-06-12 09:58:34]:
> >
> >> 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
> >
> >> 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.
>
> > 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.
Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
Vaidyanathan Srinivasan writes:
> * Nicholas Piggin [2017-06-12 09:58:34]:
>
>> 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
>
>> 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.
> 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.
cheers
Re: [PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
* Nicholas Piggin [2017-06-12 09:58:34]:
> 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
Reviewed-by: Vaidyanathan Srinivasan
> ---
> arch/powerpc/kernel/process.c | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> 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
> @@ -1960,12 +1960,8 @@ void show_stack(struct task_struct *tsk, unsigned long
> *stack)
> void notrace __ppc64_runlatch_on(void)
> {
> struct thread_info *ti = current_thread_info();
> - unsigned long ctrl;
> -
> - ctrl = mfspr(SPRN_CTRLF);
> - ctrl |= CTRL_RUNLATCH;
> - mtspr(SPRN_CTRLT, ctrl);
>
> + mtspr(SPRN_CTRLT, CTRL_RUNLATCH);
> ti->local_flags |= _TLF_RUNLATCH;
> }
>
> @@ -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? Anton and Ben kept the
mfspr/mtspr part in earlier updates to this routine.
--Vaidy
[PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
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
---
arch/powerpc/kernel/process.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
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
@@ -1960,12 +1960,8 @@ void show_stack(struct task_struct *tsk, unsigned long
*stack)
void notrace __ppc64_runlatch_on(void)
{
struct thread_info *ti = current_thread_info();
- unsigned long ctrl;
-
- ctrl = mfspr(SPRN_CTRLF);
- ctrl |= CTRL_RUNLATCH;
- mtspr(SPRN_CTRLT, ctrl);
+ mtspr(SPRN_CTRLT, CTRL_RUNLATCH);
ti->local_flags |= _TLF_RUNLATCH;
}
@@ -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);
}
#endif /* CONFIG_PPC64 */
--
2.11.0
[PATCH 13/14] powerpc/64: runlatch CTRL[RUN] set optimisation
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
---
arch/powerpc/kernel/process.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 6273b5d5baec..29865c817b02 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1991,12 +1991,8 @@ void show_stack(struct task_struct *tsk, unsigned long
*stack)
void notrace __ppc64_runlatch_on(void)
{
struct thread_info *ti = current_thread_info();
- unsigned long ctrl;
-
- ctrl = mfspr(SPRN_CTRLF);
- ctrl |= CTRL_RUNLATCH;
- mtspr(SPRN_CTRLT, ctrl);
+ mtspr(SPRN_CTRLT, CTRL_RUNLATCH);
ti->local_flags |= _TLF_RUNLATCH;
}
@@ -2004,13 +2000,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);
}
#endif /* CONFIG_PPC64 */
--
2.11.0
