| Message ID | 1500443053-1264-1-git-send-email-gromero@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cd63f3cf1d59b7ad8419eba1cac8f9126e79cc43 |
| Headers | show |
On Wed, 2017-07-19 at 01:44 -0400, Gustavo Romero wrote: > Currently flush_tmregs_to_thread() does not update accordingly the thread > structures from live state before a core dump rendering wrong values of > THFAR, TFIAR, and TEXASR in core dump files. > > That commit fixes it by copying from live state to the appropriate thread > structures when it's necessary. > > Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> Gustavo was nice enough to provide me with a simple test case: int main(void) { __builtin_set_texasr(0x4841434b); __builtin_set_tfhar(0xbfee00); __builtin_set_tfiar(0x4841434b); asm volatile (".long 0x0"); return 0; } Running this binary in a loop and inspecting the resulting core file with a modified elfutils also provided by Gustavo (https://sourceware.o rg/ml/elfutils-devel/2017-q3/msg00030.html) should always observe the values that those __builtin functions set. __builtin_set_{texasr,tfhar,tfiar} are just wrappers around the corresponding mtspr instruction. On an unmodified 4.13-rc1 it takes in the order of 10 executions of the test to observe an incorrect TM SPR values in the core file (typically zero). The above test was run on the same 4.13-rc1 with this patch applied for a over 48 hours. The test was executed at a rate of about one run per second. An incorrect value was never observed. This gives me confidence that this patch is correct. Running the kernel selftests does not detect any regressions. Reviewed-by: Cyril Bur <cyrilbur@gmail.com> > --- > arch/powerpc/kernel/ptrace.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 925a4ef..660ed39 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -127,12 +127,19 @@ static void flush_tmregs_to_thread(struct task_struct *tsk) > * If task is not current, it will have been flushed already to > * it's thread_struct during __switch_to(). > * > - * A reclaim flushes ALL the state. > + * A reclaim flushes ALL the state or if not in TM save TM SPRs > + * in the appropriate thread structures from live. > */ > > - if (tsk == current && MSR_TM_SUSPENDED(mfmsr())) > - tm_reclaim_current(TM_CAUSE_SIGNAL); > + if (tsk != current) > + return; > > + if (MSR_TM_SUSPENDED(mfmsr())) { > + tm_reclaim_current(TM_CAUSE_SIGNAL); > + } else { > + tm_enable(); > + tm_save_sprs(&(tsk->thread)); > + } > } > #else > static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
On Wed, 2017-07-19 at 05:44:13 UTC, Gustavo Romero wrote: > Currently flush_tmregs_to_thread() does not update accordingly the thread > structures from live state before a core dump rendering wrong values of > THFAR, TFIAR, and TEXASR in core dump files. > > That commit fixes it by copying from live state to the appropriate thread > structures when it's necessary. > > Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> > Reviewed-by: Cyril Bur <cyrilbur@gmail.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/cd63f3cf1d59b7ad8419eba1cac8f9 cheers
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 925a4ef..660ed39 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -127,12 +127,19 @@ static void flush_tmregs_to_thread(struct task_struct *tsk) * If task is not current, it will have been flushed already to * it's thread_struct during __switch_to(). * - * A reclaim flushes ALL the state. + * A reclaim flushes ALL the state or if not in TM save TM SPRs + * in the appropriate thread structures from live. */ - if (tsk == current && MSR_TM_SUSPENDED(mfmsr())) - tm_reclaim_current(TM_CAUSE_SIGNAL); + if (tsk != current) + return; + if (MSR_TM_SUSPENDED(mfmsr())) { + tm_reclaim_current(TM_CAUSE_SIGNAL); + } else { + tm_enable(); + tm_save_sprs(&(tsk->thread)); + } } #else static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
Currently flush_tmregs_to_thread() does not update accordingly the thread structures from live state before a core dump rendering wrong values of THFAR, TFIAR, and TEXASR in core dump files. That commit fixes it by copying from live state to the appropriate thread structures when it's necessary. Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> --- arch/powerpc/kernel/ptrace.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)