selftests/powerpc: Add test to check if TAR is corrupted

If the transaction is aborted, the TAR should be rolled back to the
checkpointed value before the transaction began.

Signed-off-by: Rashmica Gupta <rashmicy@gmail.com>
---
To check this yourself, undo the changes from the patch "powerpc/tm: Fix
context switching TAR, PPR and DSCR SPRs".

 tools/testing/selftests/powerpc/tm/.gitignore |  1 +
 tools/testing/selftests/powerpc/tm/Makefile   |  2 +-
 tools/testing/selftests/powerpc/tm/tm-tar.c   | 90 +++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-tar.c

On 12/02/2015 08:52 AM, Rashmica Gupta wrote:
> If the transaction is aborted, the TAR should be rolled back to the
> checkpointed value before the transaction began.

And if it does not abort, it should continue to hold the changed
(inside the transaction) TAR value after the transaction finishes.
The test here does check for both these cases and hence the commit
message should reflect that as well. This test can also accommodate
checks for PPR and DSCR register in one go.

In the TM ptrace patch series (https://lkml.org/lkml/2015/10/8/522)
which is still under review, there are couple of tests related
to (TAR, PPR, DSCR) registers with (TM running & TM suspended) and
without TM. There, the values are being checked using ptrace interface
instead. Can you please check them and just make sure that we are not
duplicating anything here.

> 
> Signed-off-by: Rashmica Gupta <rashmicy@gmail.com>
> ---
> To check this yourself, undo the changes from the patch "powerpc/tm: Fix
> context switching TAR, PPR and DSCR SPRs".
> 
>  tools/testing/selftests/powerpc/tm/.gitignore |  1 +
>  tools/testing/selftests/powerpc/tm/Makefile   |  2 +-
>  tools/testing/selftests/powerpc/tm/tm-tar.c   | 90 +++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/tm/tm-tar.c
> 
> diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore
> index 76eae258feeb..9bf6749550e3 100644
> --- a/tools/testing/selftests/powerpc/tm/.gitignore
> +++ b/tools/testing/selftests/powerpc/tm/.gitignore
> @@ -4,3 +4,4 @@ tm-signal-msr-resv
>  tm-signal-stack
>  tm-fork
>  tm-dscr
> +tm-tar
> diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
> index 59eec240339d..ab2992f69b4c 100644
> --- a/tools/testing/selftests/powerpc/tm/Makefile
> +++ b/tools/testing/selftests/powerpc/tm/Makefile
> @@ -1,4 +1,4 @@
> -TEST_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack tm-fork tm-dscr
> +TEST_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack tm-fork tm-dscr tm-tar
>  
>  all: $(TEST_PROGS)
>  
> diff --git a/tools/testing/selftests/powerpc/tm/tm-tar.c b/tools/testing/selftests/powerpc/tm/tm-tar.c
> new file mode 100644
> index 000000000000..22a0c3485412
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/tm/tm-tar.c
> @@ -0,0 +1,90 @@
> +/* 
> + * Copyright 2015, Michael Neuling, IBM Corp. 
> + * Licensed under GPLv2.
> + * Original: Michael Neuling 19/7/2013
> + * Edited: Rashmica Gupta 01/12/2015
> + *
> + * Do some transactions, see if the tar is corrupted.

It can use a better explanation here.

> + *
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include "tm.h"
> +#include "utils.h"
> +
> +#define SPRN_TAR	0x32f
> +
> +int	num_loops	= 10000;
> +
> +int test_tar(void)
> +{
> +	int i;
> +
> +	SKIP_IF(!have_htm());
> +
> +	for (i = 0; i < num_loops; i++)
> +	{
> +		uint64_t result = 0;
> +		asm __volatile__(
> +			"li	7, 1;"
> +			"mtspr	%[tar], 7;"	// tar = 1

Please use /* */ kind of comment sections not //.

> +			"tbegin.;"	
> +			"beq	3f;"
> +			"li	4, 0x7000;"	// Loop lots, to use time
> +			"2:;"			// Start loop

Here as well.

> +			"li	7, 2;"
> +			"mtspr	%[tar], 7;"	// tar = 2
> +			"tsuspend.;"
> +			"li	7, 3;"
> +			"mtspr	%[tar], 7;"	// tar = 3
> +			"tresume.;"
> +			"subi	4, 4, 1;"
> +			"cmpdi	4, 0;"
> +			"bne	2b;"
> +			"tend.;"
> +
> +			// Transaction sucess! TAR should be 3.
> +			"mfspr  7, %[tar];"
> +			"ori	%[res], 7, 4;"  // res = 3|4 = 7
> +			"b	4f;"
> +
> +			// Abort handler. TAR should be rolled back to 1.
> +			"3:;"
> +			"mfspr  7, %[tar];"
> +			"ori	%[res], 7, 8;"	// res = 1|8 = 9
> +			"4:;"
> +
> +			: [res]"=r"(result)
> +			: [tar]"i"(SPRN_TAR)
> +			   : "memory", "r0", "r4", "r7");
> +
> +		// If result is anything else other than 7 or 9, the tar
> +		// value must have been corrupted.

Here as well.

> +		if ((result != 7) && (result != 9)) 
> +			return 1;
> +		
> +	}
> +	return 0;
> +}
> +
> +int main(int argc, char *argv[])
> +
> +	// A low number of iterations (eg 100) can cause a false pass.

Here as well.

> +	if (argc > 1) {
> +		if (strcmp(argv[1], "-h") == 0) {
> +			printf("Syntax:\n\t%s [<num loops>]\n",
> +			       argv[0]);
> +			return 0;
> +		} else {
> +			num_loops = atoi(argv[1]);
> +		}
> +	}
> +
> +	printf("Starting, %d loops\n", num_loops);
> +
> +	test_harness(test_tar, "tm_tar");
> +}
>

> Re: [PATCH] selftests/powerpc: Add test to check if TAR is corrupted

The patch description should mention TM too.


On Wed, 2015-12-02 at 10:42 +0530, Anshuman Khandual wrote:
> On 12/02/2015 08:52 AM, Rashmica Gupta wrote:
> > If the transaction is aborted, the TAR should be rolled back to the
> > checkpointed value before the transaction began.
> 
> And if it does not abort, it should continue to hold the changed
> (inside the transaction) TAR value after the transaction finishes.
> The test here does check for both these cases and hence the commit
> message should reflect that as well. This test can also accommodate
> checks for PPR and DSCR register in one go.

Good point.  

> In the TM ptrace patch series (https://lkml.org/lkml/2015/10/8/522)
> which is still under review, there are couple of tests related
> to (TAR, PPR, DSCR) registers with (TM running & TM suspended) and
> without TM. There, the values are being checked using ptrace
> interface
> instead. Can you please check them and just make sure that we are not
> duplicating anything here.

Even if it is duplicated, I'd prefer to have this separate test as
well.  This is much simpler than that ptrace test.

Thanks for the review.

Mikey

> 
> > 
> > Signed-off-by: Rashmica Gupta <rashmicy@gmail.com>
> > ---
> > To check this yourself, undo the changes from the patch
> > "powerpc/tm: Fix
> > context switching TAR, PPR and DSCR SPRs".
> > 
> >  tools/testing/selftests/powerpc/tm/.gitignore |  1 +
> >  tools/testing/selftests/powerpc/tm/Makefile   |  2 +-
> >  tools/testing/selftests/powerpc/tm/tm-tar.c   | 90
> > +++++++++++++++++++++++++++
> >  3 files changed, 92 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/powerpc/tm/tm-tar.c
> > 
> > diff --git a/tools/testing/selftests/powerpc/tm/.gitignore
> > b/tools/testing/selftests/powerpc/tm/.gitignore
> > index 76eae258feeb..9bf6749550e3 100644
> > --- a/tools/testing/selftests/powerpc/tm/.gitignore
> > +++ b/tools/testing/selftests/powerpc/tm/.gitignore
> > @@ -4,3 +4,4 @@ tm-signal-msr-resv
> >  tm-signal-stack
> >  tm-fork
> >  tm-dscr
> > +tm-tar
> > diff --git a/tools/testing/selftests/powerpc/tm/Makefile
> > b/tools/testing/selftests/powerpc/tm/Makefile
> > index 59eec240339d..ab2992f69b4c 100644
> > --- a/tools/testing/selftests/powerpc/tm/Makefile
> > +++ b/tools/testing/selftests/powerpc/tm/Makefile
> > @@ -1,4 +1,4 @@
> > -TEST_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm
> > -signal-stack tm-fork tm-dscr
> > +TEST_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm
> > -signal-stack tm-fork tm-dscr tm-tar
> >  
> >  all: $(TEST_PROGS)
> >  
> > diff --git a/tools/testing/selftests/powerpc/tm/tm-tar.c
> > b/tools/testing/selftests/powerpc/tm/tm-tar.c
> > new file mode 100644
> > index 000000000000..22a0c3485412
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/tm/tm-tar.c
> > @@ -0,0 +1,90 @@
> > +/* 
> > + * Copyright 2015, Michael Neuling, IBM Corp. 
> > + * Licensed under GPLv2.
> > + * Original: Michael Neuling 19/7/2013
> > + * Edited: Rashmica Gupta 01/12/2015
> > + *
> > + * Do some transactions, see if the tar is corrupted.
> 
> It can use a better explanation here.
> 
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +
> > +#include "tm.h"
> > +#include "utils.h"
> > +
> > +#define SPRN_TAR	0x32f
> > +
> > +int	num_loops	= 10000;
> > +
> > +int test_tar(void)
> > +{
> > +	int i;
> > +
> > +	SKIP_IF(!have_htm());
> > +
> > +	for (i = 0; i < num_loops; i++)
> > +	{
> > +		uint64_t result = 0;
> > +		asm __volatile__(
> > +			"li	7, 1;"
> > +			"mtspr	%[tar], 7;"	// tar =
> > 1
> 
> Please use /* */ kind of comment sections not //.
> 
> > +			"tbegin.;"	
> > +			"beq	3f;"
> > +			"li	4, 0x7000;"	// Loop
> > lots, to use time
> > +			"2:;"			// Start loop
> 
> Here as well.
> 
> > +			"li	7, 2;"
> > +			"mtspr	%[tar], 7;"	// tar =
> > 2
> > +			"tsuspend.;"
> > +			"li	7, 3;"
> > +			"mtspr	%[tar], 7;"	// tar =
> > 3
> > +			"tresume.;"
> > +			"subi	4, 4, 1;"
> > +			"cmpdi	4, 0;"
> > +			"bne	2b;"
> > +			"tend.;"
> > +
> > +			// Transaction sucess! TAR should be 3.
> > +			"mfspr  7, %[tar];"
> > +			"ori	%[res], 7, 4;"  // res = 3|4 =
> > 7
> > +			"b	4f;"
> > +
> > +			// Abort handler. TAR should be rolled
> > back to 1.
> > +			"3:;"
> > +			"mfspr  7, %[tar];"
> > +			"ori	%[res], 7, 8;"	// res =
> > 1|8 = 9
> > +			"4:;"
> > +
> > +			: [res]"=r"(result)
> > +			: [tar]"i"(SPRN_TAR)
> > +			   : "memory", "r0", "r4", "r7");
> > +
> > +		// If result is anything else other than 7 or 9,
> > the tar
> > +		// value must have been corrupted.
> 
> Here as well.
> 
> > +		if ((result != 7) && (result != 9)) 
> > +			return 1;
> > +		
> > +	}
> > +	return 0;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +
> > +	// A low number of iterations (eg 100) can cause a false
> > pass.
> 
> Here as well.
> 
> > +	if (argc > 1) {
> > +		if (strcmp(argv[1], "-h") == 0) {
> > +			printf("Syntax:\n\t%s [<num loops>]\n",
> > +			       argv[0]);
> > +			return 0;
> > +		} else {
> > +			num_loops = atoi(argv[1]);
> > +		}
> > +	}
> > +
> > +	printf("Starting, %d loops\n", num_loops);
> > +
> > +	test_harness(test_tar, "tm_tar");
> > +}
> > 
>