RFR(S) 8162369: PPC64: Wrong ucontext used after SIGTRAP while in HTM critical section

Mon Aug 8 12:30:23 UTC 2016

Hi Gustavo,

thanks a lot for the nice example and the detailed explanation.
The new webrev looks good - Martin will push it.

Regards,
Volker

On Fri, Aug 5, 2016 at 4:58 AM, Gustavo Romero
<gromero at linux.vnet.ibm.com> wrote:
> Hi Volker,
>
> Thanks a lot for reviewing.
>
> On 04-08-2016 13:17, Volker Simonis wrote:
>> thanks for the new webrev. It looks good in general. However I think we
>> must check for uc being NULL. Something like:
>>
>> +  if (uc && uc->uc_link) {+    ucontext_t* second_uc = uc->uc_link;+
>
> Fixed.
>
>
>> I also didn't fully understand the comment about LSB/MSB. Doesn't it mean
>> that we need to compare the msr register against different masks depending
>> on the endianess?
>
> It's just matter of notation, a different way to point out which bit is in
> position 0 in a register. It's not related to the endianness because we are not
> dealing how a word (word could mean 16 bits [Intel], 32 bits [POWER], or
> anything else) or a register value is stored in memory, i.e the order bytes
> composing the value in a register will be stored in memory. The MSR (a 64-bit
> register) value can be stored in memory as BE or LE but when it is loaded back
> into another 64-bit GPR register it (the value) is the same, regardless
> the endianness.
>
> That's why in the Linux kernel, for instance, MSR Transactional State bits are
> defined just once and not specific to any endianness (note that they differ here
> from Power ISA, since C/C++ expect LSB 0 notation by convention in bitwise
> operations, regardless of endianness):
> https://github.com/torvalds/linux/blob/master/arch/powerpc/include/asm/reg.h#L32-L33
>
> MSB 0 is also known as IBM bit ordering/numbering. For example, as we can find
> in this subtle comment in the code regarding the implementation of HTM
> intrinsics on gcc:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/config/rs6000/htmintrin.h#L44-L45
>
> I wrote a simple example to show that the value of MSR won't change from BE to
> LE: https://github.com/gromero/htm/blob/master/threads10.c
>
> This code when compiled and run in BE will produce the same result as in LE (you
> can see the MSR TS bits that match the mask 0x600000000 flipping):
>
> [LE]
>
> gromero at gromero2:~/git/htm$ uname -a
> Linux gromero2 4.4.0-28-generic #47-Ubuntu SMP Fri Jun 24 10:09:20 UTC 2016 ppc64le ppc64le ppc64le GNU/Linux
> gromero at gromero2:~/git/htm$ make threads10
> gcc -c -g -O0 threads10.c -o threads10.o
> gcc -pthread -mhtm -mcpu=power8 -g -O0 threads10.o -o threads10
> gromero at gromero2:~/git/htm$ ./threads10
> => I'm thread 3fffa078f1a0. I'll perform JUST A TRAP every 2 seconds
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d033
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP but NOT in transaction: uc->uc_mcontext.regs->msr = 0x800000010282d033
> => I'm thread 3fffa078f1a0. I'll perform JUST A TRAP every 2 seconds
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d033
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d033
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP but NOT in transaction: uc->uc_mcontext.regs->msr = 0x800000010282d033
> => I'm thread 3fffa078f1a0. I'll perform JUST A TRAP every 2 seconds
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d033
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d033
> => I'm thread 3fff9ff8f1a0. I'll perform A TRAP IN TRANSACTION every 1 second
> ...
>
> [BE]
>
> [root at localhost htm]# uname -a
> Linux localhost 3.10.0-327.18.2.el7.ppc64 #1 SMP Fri Apr 8 05:12:29 EDT 2016 ppc64 ppc64 ppc64 GNU/Linux
> [root at localhost htm]# make threads10
> gcc -c -g -O0 threads10.c -o threads10.o
> gcc -pthread -mhtm -mcpu=power8 -g -O0 threads10.o -o threads10
> [root at localhost htm]# ./threads10
> => I'm thread 3fffa902f1d0. I'll perform JUST A TRAP every 2 seconds
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d032
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP but NOT in transaction: uc->uc_mcontext.regs->msr = 0x800000010282d032
> => I'm thread 3fffa902f1d0. I'll perform JUST A TRAP every 2 seconds
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d032
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d032
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP but NOT in transaction: uc->uc_mcontext.regs->msr = 0x800000010282d032
> => I'm thread 3fffa902f1d0. I'll perform JUST A TRAP every 2 seconds
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d032
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> * Caught a SIGTRAP in transaction:         uc->uc_mcontext.regs->msr = 0x800000050282d032
> => I'm thread 3fffa882f1d0. I'll perform A TRAP IN TRANSACTION every 1 second
> ...
>
> new webrev:
> http://81.de.7a9f.ip4.static.sl-reverse.com/8162369/v3
>
> Thank you!
>
> Regards,
> Gustavo
>