Re: [PATCH v2 1/2] powerpc/64s: system call scv tabort fix for corrupt irq soft-mask state

On Thu, Sep 02, 2021 at 01:33:10PM +1000, Nicholas Piggin wrote:

Excerpts from Christophe Leroy's message of September 2, 2021 3:21 am:

quoted

quoted

-	/* Firstly we need to enable TM in the kernel */
+	/* We need to enable TM in the kernel, and disable EE (for scv) */
  	mfmsr	r10
  	li	r9, 1
  	rldimi	r10, r9, MSR_TM_LG, 63-MSR_TM_LG
+	LOAD_REG_IMMEDIATE(r9, MSR_EE)
+	andc	r10, r10, r9

Why not use 'rlwinm' to mask out MSR_EE ?

Something like

	rlwinm	r10, r10, 0, ~MSR_EE

Mainly because I'm bad at powerpc assembly. Why do you think I'm trying 
to change as much as possible to C?

The actual bit (bit 31, i.e. with value 1UL << 32) cannot be cleared
with rlwinm (only the low 32 bits can).  There are many ways to do it
using two insns of course.

Actually there should really be no need for mfmsr either, I wanted to
rewrite the thing entirely as

	ld      r10,PACAKMSR(r13)
	LOAD_REG_IMMEDIATE(r9, MSR_TM)
	or	r10,r10,r9
	mtmsrd	r10

But I thought that's not a minimal bug fix.

That (LOAD_REG_IMMEDIATE+or) can be done with just two insns, not three
as written:
  li r0,1
  rldimi r10,r0,32,31
(you can write that last insns as
  rldimi r10,r0,MSR_TM_LG,MSR_TM_LG-1
if you insist :-) )

It isn't like a few integer computational insns will kill you here of
course, there are much more important cycles to shave off :-)


Segher