Hi Michael,
On 12/20/18 10:51 AM, Michael Ellerman wrote:
> Breno Leitao <lei...@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 might also add a macro to address ucontext->mcontext MSR. This will avoid
problems like that in the future.
index ae43a614835d..7636bf45d5d5 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -102,8 +102,10 @@ do {
#if defined(__powerpc64__)
#define UCONTEXT_NIA(UC) (UC)->uc_mcontext.gp_regs[PT_NIP]
+#define UCONTEXT_MSR(UC) (UC)->uc_mcontext.gp_regs[PT_MSR]
#elif defined(__powerpc__)
#define UCONTEXT_NIA(UC) (UC)->uc_mcontext.uc_regs->gregs[PT_NIP]
+#define UCONTEXT_MSR(UC) (UC)->uc_mcontext.uc_regs->gregs[PT_MSR]
#else
#error implement UCONTEXT_NIA
#endif
> And using powerpc64le-linux-gnu-gcc I get:
>
> In file included from /usr/powerpc64le-linux-gnu/include/string.h:494,
> from tm-signal-force-msr.c:10:
> In function 'memcpy',
> inlined from 'trap_signal_handler' at tm-signal-force-msr.c:39:2:
> /usr/powerpc64le-linux-gnu/include/bits/string_fortified.h:34:10: error:
> '__builtin_memcpy' accessing 1272 bytes at offsets 8 and 168 overlaps 1112
> bytes at offset 168 [-Werror=restrict]
> return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Damn, that is because I do not know how to use C pointers. Fixing it on v3 also.
