[PATCH] powerpc/tm: fix live state of vs0/32 in tm_reclaim

[Christophe Leroy]

Le 04/07/2017 à 22:45, Gustavo Romero a écrit :
> Currently tm_reclaim() can return with a corrupted vs0 (fp0) or vs32 (v0)
> due to the fact vs0 is used to save FPSCR and vs32 is used to save VSCR.
> 
> Later, we recheckpoint trusting that the live state of FP and VEC are ok
> depending on the MSR.FP and MSR.VEC bits, i.e. if MSR.FP is enabled that
> means the FP registers checkpointed when we entered in TM are correct and
> after a treclaim. we can trust the FP live state. Similarly to VEC regs.
> However if tm_reclaim() does not return a sane state then tm_recheckpoint()
> will recheckpoint a corrupted state from live state back to the checkpoint
> area.
> 
> That commit fixes the corruption by restoring vs0 and vs32 from the
> ckfp_state and ckvr_state after they are used to save FPSCR and VSCR,
> respectively.
> 
> The effect of the issue described above is observed, for instance, once a
> VSX unavailable exception is caught in the middle of a transaction with
> MSR.FP = 1 or MSR.VEC = 1. If MSR.FP = 1, then after getting back to user
> space FP state is corrupted. If MSR.VEC = 1, then VEC state is corrupted.
> 
> The issue does not occur if MSR.FP = 0 and MSR.VEC = 0 because ckfp_state
> and ckvr_state are both copied from fp_state and vr_state, respectively,
> and on recheckpointing both states will be restored from these thread
> structures and not from the live state.
> 
> The issue does not occur also if MSR.FP = 1 and MSR.VEC = 1 because it
> implies MSR.VSX = 1 and in that case the VSX unavailable exception does not
> happen in the middle of the transactional block.
> 
> Finally, that commit also fixes the MSR used to check if FP and VEC bits
> are enabled once we are in tm_reclaim_thread(). ckpt_regs.msr is valid only
> if giveup_all() is called *before* using ckpt_regs.msr for checks because
> check_if_tm_restore_required() in giveup_all() will copy regs->msr to
> ckpt_regs.msr and so ckpt_regs.msr reflects exactly the MSR that the thread
> had when it came off the processor.
> 
> No regression was observed on powerpc/tm selftests after this fix.
> 
> Signed-off-by: Gustavo Romero <gromero at linux.vnet.ibm.com>
> Signed-off-by: Breno Leitao <leitao at debian.org>

This patch is still flaged as "New" in patchwork 
(https://patchwork.ozlabs.org/project/linuxppc-dev/patch/1499201115-22967-1-git-send-email-gromero@linux.vnet.ibm.com/)

I don't know why but the discussion that happened on this patch don't 
appear in patchwork.

I see two commit touching the same area of code done in the following 
monthes the same year:

eb5c3f1c8647 ("powerpc: Always save/restore checkpointed regs during 
treclaim/trecheckpoint")
91381b9cb1c3 ("powerpc: Force reload for recheckpoint during tm {fp, 
vec, vsx} unavailable exception")

So I'll mark this patch as "changes requested". Please raise hand if I'm 
wrong.

Christophe