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

[Gustavo Romero]

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