On Tue, 2018-11-06 at 10:40 -0200, Breno Leitao wrote:
> Currently the signal context restore code enables the bit on the MSR
> register without restoring the TM SPRs, which can cause undesired side
> effects.
>
> This is not correct because if TM is enabled in MSR, it means the TM SPR
> registers are valid and updated, which is not correct here. In fact, the
> live registers may contain previous' thread SPRs.
>
> Functions check if the register values are valid or not through looking
> if the facility is enabled at MSR, as MSR[TM] set means that the TM SPRs
> are hot.
>
> When just enabling MSR[TM] without updating the live SPRs, this can cause a
> crash, since current TM SPR from previous thread will be saved on the
> current thread, and might not have TEXASR[FS] set, for example.
>
> Signed-off-by: Breno Leitao <lei...@debian.org>
> ---
> arch/powerpc/kernel/signal_64.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index 487c3b6aa2e3..156b90e8ee78 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -478,8 +478,18 @@ static long restore_tm_sigcontexts(struct task_struct
> *tsk,
> * happened whilst in the signal handler and load_tm overflowed,
> * disabling the TM bit. In either case we can end up with an illegal
> * TM state leading to a TM Bad Thing when we return to userspace.
> + *
> + * Every time MSR_TM is enabled, mainly for the b) case, the TM SPRs
> + * must be restored in the live registers, since the live registers
> + * could contain garbage and later we want to read from live, since
> + * MSR_TM is enabled, and MSR[TM] is what is used to check if the
> + * TM SPRs live registers are valid or not.
> */
> - regs->msr |= MSR_TM;
> + if ((regs->msr & MSR_TM) == 0) {
> + regs->msr |= MSR_TM;
> + tm_enable();
> + tm_restore_sprs(&tsk->thread);
> + }
I'm wondering if we should put the save/restore TM registers in the early
entry/exit code too. We'd need to add the check on msr[tm]/load_tm.
Distributing the SPR save/restore throughout the kernel is just going to lead us
to similar problems that we are having now with reclaim/recheckpoint.
Mikey
>
> /* pull in MSR LE from user context */
> regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
