[PATCH 2/5] selftests/powerpc: Add test to check TM ucontext creation

Daniel Axtens dja at axtens.net
Thu Jun 9 15:12:51 AEST 2016


I'm trying not to be too nit picky or difficult on tests, so here's a
pre-written commit message for you:

"The kernel sets up two sets of ucontexts if the signal was to be
delivered while the thread was in a transaction. Expected behaviour is
that the currently executing code is in the first and the checkpointed
state (the state that will be rolled back to) is in the uc_link
ucontext.

The reason for this is that:

 - code which is not TM aware and installs a signal handler will expect
   to see/modify its currently running state in the uc.

 - but, that TM-unaware code may have dynamicially linked against code
   which is TM aware and is doing HTM under the hood, so the
   checkpointed state needs to be made available somewhere

Test if the live and checkpointed state is made stored correctly.
Test:
 - GPRs
 - FP registers
 - VMX
 - VSX
"

> +#define TBEGIN .long 0x7C00051D
> +#define TSUSPEND .long 0x7C0005DD
> +#define TRESUME .long 0x7C2005DD
You define these 3 opcodes in a number of files. I assume you're going
to consolidate them in v2?

> + * The kernel sets up two sets of ucontexts if the signal was to be delivered
> + * while the thread was in a transaction. Expected behaviour is that the
> + * currently executing code is in the first and the checkpointed state (the
> + * state that will be rolled back to) is in the uc_link ucontext.
> + *
> + * The reason for this is that code which is not TM aware and installs a signal
> + * handler will expect to see/modify its currently running state in the uc,
> + * this code may have dynamicially linked against code which is TM aware and is
> + * doing HTM under the hood.

I had real trouble parsing this sentence the first few times. I think
it's missing a while:

The reason for this is that _while_ code which is not TM aware...

(Although it would be better in several sentences :P)

> +++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-chk-gpr.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright 2016, Cyril Bur, IBM Corp.
> + * Licensed under GPLv2.
Ironically, it seems this now needs to be GPLv2+, probably with the
regular license grant paragraph.

> +	/* Always be 64bit, don't really care about 32bit */
Forgive my ignorance of the test suite: are we guaranteed this by the
build system, or should we add a SKIP_IF() for it?
> +	for (i = 0; i < NV_GPR_REGS && !fail; i++) {
> +		fail = (ucp->uc_mcontext.gp_regs[i + 14] != gps[i]);
> +		fail |= (tm_ucp->uc_mcontext.gp_regs[i + 14] != gps[i + NV_GPR_REGS]);
> +	}
> +	if (fail)
> +		printf("Failed on %d GPR %lu or %lu\n", i - 1,
> +				ucp->uc_mcontext.gp_regs[i + 13], tm_ucp->uc_mcontext.gp_regs[i + 13]);
> +}
> +

Looking good otherwise!

Regards,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 859 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20160609/531dfc93/attachment.sig>


More information about the Linuxppc-dev mailing list