[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1473828680.2554.10.camel@gmail.com>
Date: Wed, 14 Sep 2016 14:51:20 +1000
From: Cyril Bur <cyrilbur@...il.com>
To: wei.guo.simon@...il.com, linuxppc-dev@...ts.ozlabs.org
Cc: Shuah Khan <shuah@...nel.org>,
Michael Ellerman <mpe@...erman.id.au>,
Chris Smart <chris@...troguy.com>,
Suraj Jitindar Singh <sjitindarsingh@...il.com>,
Michael Neuling <mikey@...ling.org>,
Anshuman Khandual <khandual@...ux.vnet.ibm.com>,
Jack Miller <jack@...ezen.org>,
Rashmica Gupta <rashmicy@...il.com>,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v14 05/15] selftests/powerpc: Add ptrace tests for
GPR/FPR registers in TM
On Mon, 2016-09-12 at 15:33 +0800, wei.guo.simon@...il.com wrote:
> From: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
>
> This patch adds ptrace interface test for GPR/FPR registers
> inside TM context. This adds ptrace interface based helper
> functions related to checkpointed GPR/FPR access.
>
> Signed-off-by: Anshuman Khandual <khandual@...ux.vnet.ibm.com>
> Signed-off-by: Simon Guo <wei.guo.simon@...il.com>
> ---
> tools/testing/selftests/powerpc/ptrace/Makefile | 3 +-
> .../selftests/powerpc/ptrace/ptrace-tm-gpr.c | 296
> +++++++++++++++++++++
> 2 files changed, 298 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-tm-
> gpr.c
>
> diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile
> b/tools/testing/selftests/powerpc/ptrace/Makefile
> index 31e8e33..170683a 100644
> --- a/tools/testing/selftests/powerpc/ptrace/Makefile
> +++ b/tools/testing/selftests/powerpc/ptrace/Makefile
> @@ -1,8 +1,9 @@
> -TEST_PROGS := ptrace-ebb ptrace-gpr
> +TEST_PROGS := ptrace-ebb ptrace-gpr ptrace-tm-gpr
>
> all: $(TEST_PROGS)
> CFLAGS += -m64
> $(TEST_PROGS): ../harness.c ptrace.S ../utils.c ptrace.h
> ptrace-ebb: ../pmu/event.c ../pmu/lib.c ../pmu/ebb/ebb_handler.S
> ../pmu/ebb/busy_loop.S
> +
> clean:
> rm -f $(TEST_PROGS) *.o
> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> new file mode 100644
> index 0000000..8417d04
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-gpr.c
> @@ -0,0 +1,296 @@
> +/*
> + * Ptrace test for GPR/FPR registers in TM context
> + *
> + * Copyright (C) 2015 Anshuman Khandual, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +#include "ptrace.h"
> +#include "ptrace-gpr.h"
> +
> +/* Tracer and Tracee Shared Data */
> +int shm_id;
> +volatile unsigned long *cptr, *pptr;
> +
> +float a = FPR_1;
> +float b = FPR_2;
> +float c = FPR_3;
> +
> +void tm_gpr(void)
> +{
> + unsigned long gpr_buf[18];
> + unsigned long result, texasr;
> + float fpr_buf[32];
> +
> + printf("Starting the child\n");
> + cptr = (unsigned long *)shmat(shm_id, NULL, 0);
> +
> +trans:
> + cptr[1] = 0;
> + asm __volatile__(
> +
> + "li 14, %[gpr_1];"
> + "li 15, %[gpr_1];"
> + "li 16, %[gpr_1];"
> + "li 17, %[gpr_1];"
> + "li 18, %[gpr_1];"
> + "li 19, %[gpr_1];"
> + "li 20, %[gpr_1];"
> + "li 21, %[gpr_1];"
> + "li 22, %[gpr_1];"
> + "li 23, %[gpr_1];"
> + "li 24, %[gpr_1];"
> + "li 25, %[gpr_1];"
> + "li 26, %[gpr_1];"
> + "li 27, %[gpr_1];"
> + "li 28, %[gpr_1];"
> + "li 29, %[gpr_1];"
> + "li 30, %[gpr_1];"
> + "li 31, %[gpr_1];"
> +
> + "lfs 0, 0(%[flt_1]);"
> + "lfs 1, 0(%[flt_1]);"
> + "lfs 2, 0(%[flt_1]);"
> + "lfs 3, 0(%[flt_1]);"
> + "lfs 4, 0(%[flt_1]);"
> + "lfs 5, 0(%[flt_1]);"
> + "lfs 6, 0(%[flt_1]);"
> + "lfs 7, 0(%[flt_1]);"
> + "lfs 8, 0(%[flt_1]);"
> + "lfs 9, 0(%[flt_1]);"
> + "lfs 10, 0(%[flt_1]);"
> + "lfs 11, 0(%[flt_1]);"
> + "lfs 12, 0(%[flt_1]);"
> + "lfs 13, 0(%[flt_1]);"
> + "lfs 14, 0(%[flt_1]);"
> + "lfs 15, 0(%[flt_1]);"
> + "lfs 16, 0(%[flt_1]);"
> + "lfs 17, 0(%[flt_1]);"
> + "lfs 18, 0(%[flt_1]);"
> + "lfs 19, 0(%[flt_1]);"
> + "lfs 20, 0(%[flt_1]);"
> + "lfs 21, 0(%[flt_1]);"
> + "lfs 22, 0(%[flt_1]);"
> + "lfs 23, 0(%[flt_1]);"
> + "lfs 24, 0(%[flt_1]);"
> + "lfs 25, 0(%[flt_1]);"
> + "lfs 26, 0(%[flt_1]);"
> + "lfs 27, 0(%[flt_1]);"
> + "lfs 28, 0(%[flt_1]);"
> + "lfs 29, 0(%[flt_1]);"
> + "lfs 30, 0(%[flt_1]);"
> + "lfs 31, 0(%[flt_1]);"
> +
There was this in the previous patch? Can we consolidate?
> + "1: ;"
> + TBEGIN
tbegin. is probably fine
> + "beq 2f;"
> +
> + "li 14, %[gpr_2];"
> + "li 15, %[gpr_2];"
> + "li 16, %[gpr_2];"
> + "li 17, %[gpr_2];"
> + "li 18, %[gpr_2];"
> + "li 19, %[gpr_2];"
> + "li 20, %[gpr_2];"
> + "li 21, %[gpr_2];"
> + "li 22, %[gpr_2];"
> + "li 23, %[gpr_2];"
> + "li 24, %[gpr_2];"
> + "li 25, %[gpr_2];"
> + "li 26, %[gpr_2];"
> + "li 27, %[gpr_2];"
> + "li 28, %[gpr_2];"
> + "li 29, %[gpr_2];"
> + "li 30, %[gpr_2];"
> + "li 31, %[gpr_2];"
> +
> +
> + "lfs 0, 0(%[flt_2]);"
> + "lfs 1, 0(%[flt_2]);"
> + "lfs 2, 0(%[flt_2]);"
> + "lfs 3, 0(%[flt_2]);"
> + "lfs 4, 0(%[flt_2]);"
> + "lfs 5, 0(%[flt_2]);"
> + "lfs 6, 0(%[flt_2]);"
> + "lfs 7, 0(%[flt_2]);"
> + "lfs 8, 0(%[flt_2]);"
> + "lfs 9, 0(%[flt_2]);"
> + "lfs 10, 0(%[flt_2]);"
> + "lfs 11, 0(%[flt_2]);"
> + "lfs 12, 0(%[flt_2]);"
> + "lfs 13, 0(%[flt_2]);"
> + "lfs 14, 0(%[flt_2]);"
> + "lfs 15, 0(%[flt_2]);"
> + "lfs 16, 0(%[flt_2]);"
> + "lfs 17, 0(%[flt_2]);"
> + "lfs 18, 0(%[flt_2]);"
> + "lfs 19, 0(%[flt_2]);"
> + "lfs 20, 0(%[flt_2]);"
> + "lfs 21, 0(%[flt_2]);"
> + "lfs 22, 0(%[flt_2]);"
> + "lfs 23, 0(%[flt_2]);"
> + "lfs 24, 0(%[flt_2]);"
> + "lfs 25, 0(%[flt_2]);"
> + "lfs 26, 0(%[flt_2]);"
> + "lfs 27, 0(%[flt_2]);"
> + "lfs 28, 0(%[flt_2]);"
> + "lfs 29, 0(%[flt_2]);"
> + "lfs 30, 0(%[flt_2]);"
> + "lfs 31, 0(%[flt_2]);"
> + TSUSPEND
tsuspend.
> + "li 7, 1;"
> + "stw 7, 0(%[cptr1]);"
> + TRESUME
tresume.
> + "b .;"
> +
> + TEND
...
> + "li 0, 0;"
> + "ori %[res], 0, 0;"
> + "b 3f;"
> +
> + /* Transaction abort handler */
> + "2: ;"
> + "li 0, 1;"
> + "ori %[res], 0, 0;"
> + "mfspr %[texasr], %[sprn_texasr];"
> +
> + "3: ;"
> + : [res] "=r" (result), [texasr] "=r" (texasr)
> + : [gpr_1]"i"(GPR_1), [gpr_2]"i"(GPR_2),
> + [sprn_texasr] "i" (SPRN_TEXASR), [flt_1] "r" (&a),
Thanks for actually passing through texasr like this (as opposed to
simply hardcoding it), made this easier to read.
> + [flt_2] "r" (&b), [cptr1] "r" (&cptr[1])
> + : "memory", "r7", "r8", "r9", "r10",
> + "r11", "r12", "r13", "r14", "r15", "r16",
> + "r17", "r18", "r19", "r20", "r21", "r22",
> + "r23", "r24", "r25", "r26", "r27", "r28",
> + "r29", "r30", "r31"
> + );
> +
> + if (result) {
> + if (!cptr[0])
> + goto trans;
> +
> + shmdt((void *)cptr);
> + store_gpr(gpr_buf);
> + store_fpr(fpr_buf);
> +
> + if (validate_gpr(gpr_buf, GPR_3))
> + exit(1);
> +
> + if (validate_fpr_float(fpr_buf, c))
> + exit(1);
> +
> + exit(0);
> + }
> + shmdt((void *)cptr);
> + exit(1);
> +}
> +
> +int trace_tm_gpr(pid_t child)
> +{
> + unsigned long gpr[18];
> + unsigned long fpr[32];
> + int ret;
> +
> + ret = start_trace(child);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = show_gpr(child, gpr);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = validate_gpr(gpr, GPR_2);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = show_fpr(child, fpr);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = validate_fpr(fpr, FPR_2_REP);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = show_ckpt_fpr(child, fpr);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = validate_fpr(fpr, FPR_1_REP);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = show_ckpt_gpr(child, gpr);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = validate_gpr(gpr, GPR_1);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = write_ckpt_gpr(child, GPR_3);
> + if (ret)
> + return TEST_FAIL;
> +
> + ret = write_ckpt_fpr(child, FPR_3_REP);
> + if (ret)
> + return TEST_FAIL;
> +
> + pptr[0] = 1;
> + ret = stop_trace(child);
> + if (ret)
> + return TEST_FAIL;
> +
> + return TEST_PASS;
I'll repeat this somewhere else perhaps as it seems to be a theme, it
might be wise to use the TEST_FAIL(x) macro from utils.h as it will
give information to more quickly narrow down where the test failed.
> +}
> +
> +int ptrace_tm_gpr(void)
> +{
> + pid_t pid;
> + int ret, status;
> +
> + SKIP_IF(!((long)get_auxv_entry(AT_HWCAP2) &
> PPC_FEATURE2_HTM));
Theres a function (have_htm()) in tm/tm.h that might be nice to move
more generally which does this...
> + shm_id = shmget(IPC_PRIVATE, sizeof(int) * 2,
> 0777|IPC_CREAT);
> + pid = fork();
> + if (pid < 0) {
> + perror("fork() failed");
> + return TEST_FAIL;
> + }
> + if (pid == 0)
> + tm_gpr();
> +
> + if (pid) {
> + pptr = (unsigned long *)shmat(shm_id, NULL, 0);
> +
> + while (!pptr[1]);
asm volatile("" : : : "memory");
See: Documentation/volatile-considered-harmful.txt also in some
selftests I wrote I had exactly what you had and just declaring
volatile didn't *work* [do that I thought it would which it seems is
what you want here too]!
> + kill(pid, SIGTERM);
> + return TEST_FAIL;
> + }
> +
> + shmdt((void *)pptr);
> +
> + ret = wait(&status);
> + shmctl(shm_id, IPC_RMID, NULL);
> + if (ret != pid) {
> + printf("Child's exit status not
> captured\n");
> + return TEST_FAIL;
> + }
> +
> + if (WIFEXITED(status)) {
> + if (WEXITSTATUS(status))
> + return TEST_FAIL;
> + }
> + return TEST_PASS;
return (WIFEXITED(status) && WEXITSTATUS(status)) ? TEST_FAIL :
TEST_PASS;
> + }
> + return TEST_PASS;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + return test_harness(ptrace_tm_gpr, "ptrace_tm_gpr");
> +}
Powered by blists - more mailing lists