hOn Mon, 2015-11-16 at 21:24 +1100, Michael Ellerman wrote:
> On Fri, 2015-11-13 at 15:57 +1100, Michael Neuling wrote:
>
> > Test the kernel's signal return code to ensure that it doesn't
> > crash
> > when both the transactional and suspend MSR bits are set in the
> > signal
> > context.
> >
> > Signed-off-by: Michael Neuling <mi...@neuling.org>
> > ---
> > tools/testing/selftests/powerpc/tm/Makefile | 2 +-
> > .../selftests/powerpc/tm/tm-signal-msr-resv.c | 64
> > ++++++++++++++++++++++
> > 2 files changed, 65 insertions(+), 1 deletion(-)
> > create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal
> > -msr-resv.c
> >
> > diff --git a/tools/testing/selftests/powerpc/tm/Makefile
> > b/tools/testing/selftests/powerpc/tm/Makefile
> > index 4bea62a..0c28db7 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
> > +TEST_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv
> >
> > all: $(TEST_PROGS)
> >
> > diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-msr
> > -resv.c b/tools/testing/selftests/powerpc/tm/tm-signal-msr-resv.c
> > new file mode 100644
> > index 0000000..14705ae
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/tm/tm-signal-msr-resv.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * Copyright 2015, Michael Neuling, IBM Corp.
> > + * Licensed under GPLv2.
> > + *
> > + * Test the kernel's signal return code to ensure that it doesn't
> > + * crash when both the transactional and suspend MSR bits are set
> > in
> > + * the signal context.
>
> That's a good start, a little blurb on how we do the test is nice
> too. For our
> future selves.
>
> eg. We send ourselves a SIGUSR1, and in the handler we write the
> illegal MSR
> value, then when we return from the signal the kernel should detect
> that and
> kill us with a SIGSEGV.
Ok, done.
>
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <signal.h>
> > +
> > +#include "utils.h"
> > +
> > +struct sigaction act;
> > +
> > +void signal_segv(int signum, siginfo_t *info, void *uc)
> > +{
> > + printf("PASSED\n");
> > + exit(0);
>
> You're not allowed to call exit() from a signal handler :D
It worked :-D
I'll move to _exit().
> You can call _exit(), I guess you can't just set a flag and return
> like you'd
> normally do in a handler?
No I can't as we keep segving when we return.
>
> > +}
> > +
> > +void signal_usr1(int signum, siginfo_t *info, void *uc)
> > +{
> > + ucontext_t *ucp = uc;
> > +
> > + /* link tm checkpointed context to normal context */
> > + ucp->uc_link = ucp;
> > + /* set all TM bits */
> > +#ifdef __powerpc64__
> > + ucp->uc_mcontext.gp_regs[PT_MSR] |= (7ULL << 32);
> > +#else
> > + ucp->uc_mcontext.regs->gpr[PT_MSR] |= (7ULL);
> > +#endif
> > + /* Should segv on return becuase of invalid context */
>
> On return of what? This handler or sigaction() below ?
Because the kernel detects the illegal MSR setting above.
>
> > + act.sa_sigaction = signal_segv;
> > + if (sigaction(SIGSEGV, &act, NULL) < 0) {
> > + perror("sigaction sigsegv");
> > + exit(1);
> > + }
>
> I'm not clear why we're setting up the handler for SEGV in the
> handler for
> USR1. Is that required to trip the bug? (not that I understood).
I did it here to minimise the chance of taking the segv earlier and
falsely passing the test.
I've changed this to a flag now so I can register it earlier.
> > +}
> > +
> > +int tm_signal_msr_resv()
> > +{
> > +
> > + act.sa_sigaction = signal_usr1;
> > + sigemptyset(&act.sa_mask);
> > + act.sa_flags = SA_SIGINFO;
> > + if (sigaction(SIGUSR1, &act, NULL) < 0) {
> > + perror("sigaction sigusr1");
> > + exit(1);
> > + }
> > +
> > + raise(SIGUSR1);
> > +
> > + printf("FAILED\n");
>
> The harness will print a failure message for you, so this is not
> really
> helpful. What is helpful is a message that says *why* it failed. eg:
> "Expected
> to take SEGV but didn't?" or something like that.
I've removed it for now.
I've also added the compiled binary to .gitignore.
Mikey
>
> > + return 1;
> > +}
> > +
> > +int main(void)
> > +{
> > + return test_harness(tm_signal_msr_resv,
> > "tm_signal_msr_resv");
> > +}
>
> cheers
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
