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: > >> - /* 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
