Re: [PATCH] selftests/powerpc: New TM signal self test

[Michael Ellerman <mpe@ellerman.id.au>]

Breno Leitao <leitao@debian.org> writes:
> On 12/20/18 10:51 AM, Michael Ellerman wrote:
>> Breno Leitao <leitao@debian.org> writes:
>> 
>>> A new self test that forces MSR[TS] to be set without calling any TM
>>> instruction. This test also tries to cause a page fault at a signal
>>> handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing
>>> thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG
>>> when tm_recheckpoint() is called.
>>>
>>> This test is not deterministic since it is hard to guarantee that the page
>>> access will cause a page fault. Tests have shown that the bug could be
>>> exposed with few interactions in a buggy kernel. This test is configured to
>>> loop 5000x, having a good chance to hit the kernel issue in just one run.
>>> This self test takes less than two seconds to run.
>>>
>>> This test uses set/getcontext because the kernel will recheckpoint
>>> zeroed structures, causing the test to segfault, which is undesired because
>>> the test needs to rerun, so, there is a signal handler for SIGSEGV which
>>> will restart the test.
>> And reference the ucontext->mcontext MSR using UCONTEXT_MSR() macro.
>> Hi Breno,
>> 
>> Thanks for the test, some of these TM tests are getting pretty advanced! :)
>> 
>> Unfortunately it doesn't build in a few configurations.
>> 
>> On Ubuntu 18.10 built with powerpc-linux-gnu-gcc I get:
>> 
>>   tm-signal-force-msr.c: In function 'trap_signal_handler':
>>   tm-signal-force-msr.c:42:19: error: 'union uc_regs_ptr' has no member named 'gp_regs'; did you mean 'uc_regs'?
>>     ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S;
>>                      ^~~~~~~
>>                      uc_regs
>>   tm-signal-force-msr.c:17:29: error: left shift count >= width of type [-Werror=shift-count-overflow]
>>    #define __MASK(X)       (1UL<<(X))
>>                                ^~
>>   tm-signal-force-msr.c:20:25: note: in expansion of macro '__MASK'
>>    #define MSR_TS_S        __MASK(MSR_TS_S_LG) /* Transaction Suspended */
>>                            ^~~~~~
>>   tm-signal-force-msr.c:42:38: note: in expansion of macro 'MSR_TS_S'
>>     ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S;
>>                                         ^~~~~~~~
>> 
>
> That is because I missed the -m64 compilation flag on Makefile. I understand
> that this test only make sense when compiled in 64 bits. Do you agree?

I think the test could work as a 32-bit binary on a 64-bit kernel, but I
don't mind if you force it to build 64-bit.

cheers