[PATCH] powerpc/tm: fix TM SPRs in code dump file

[PATCH] powerpc/tm: fix TM SPRs in code dump file

[Gustavo Romero]

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(-)

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) { }
-- 
2.7.4

Re: [PATCH] powerpc/tm: fix TM SPRs in code dump file

[Cyril Bur]

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) { }

Re: powerpc/tm: fix TM SPRs in code dump file

[Michael Ellerman]

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