Re: [PATCH] PowerPC: Clear MSR_VSX for getcontext

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Oct 17, 2013 at 08:56:21AM -0300, Adhemerval Zanella wrote:
> This patch makes getcontext clear the MSR_VSX (indicating VSX usage)
> from gregs[PT_MSR] so a following setcontext/swapcontext/makecontext
> will not fail with invalid argument.
> 
> This patch fixes the testcases:
> 
> * stdlib/tst-setcontext,
> * stdlib/tst-makecontext3, and
> * tst-setcontext-fpscr
> 
> When building for PPC32 with --with-cpu=power7.

This is not sufficient, while what getcontext will return will be fine,
what swapcontext returns (well, fills in *oucp) will often not.  So, if you
say use some VSX instruction somewhere (thus modify VSX state), do
getcontext (&uctx1); makecontext (&uctx1, fn, 0); swapcontext (&uctx2, &uctx1);
this will all succeed, but uctx2 might have MSR_VSX bit set in it.  Thus
if you say in fn call setcontext (&uctx2); or swapcontext (&uctx1, &uctx2);
etc., those will fail.  And, for swapcontext there is no (easy) way to run
code to actually clear the bit in the new context.

Which is why I'm afraid this is much more easily fixable in the kernel,
say through something like:
--- signal_32.c 2013-01-16 11:26:18.125461647 +0100
+++ signal_32.c    2013-10-11 11:41:40.288026929 +0200
@@ -449,7 +449,8 @@ static int save_user_regs(struct pt_regs
                if (copy_vsx_to_user(&frame->mc_vsregs, current))
                        return 1;
                msr |= MSR_VSX;
-       }
+       } else if (!ctx_has_vsx_region)
+               msr &= ~MSR_VSX;
 #endif /* CONFIG_VSX */
 #ifdef CONFIG_SPE
        /* save spe registers */
(complete untested) - the point, if somebody asks with getcontext
or swapcontext for context being filled and it is small enough that
the VSX state can't be stored into it, just clear the MSR_VSX bit
so that you can actually successfully swapcontext to it.

Or the other option is to clear the MSR_VSX bit not in userland
getcontext (where it is possible) and swapcontext (where it is not
possible), but instead in setcontext and swapcontext in the structure
with the new context.  Except that the argument points to const ucontext_t,
and it might be undesirable or impossible to modify it, so it would need
to check if the MSR_VSX bit is set, and if it is, copy to a temporary
ucontext_t on the stack, clear the MSR_VSX bit there, and use address of the
copy instead of the original as argument to the swapcontext syscall.
This could be perhaps improved by also clearing the MSR_VSX bit in
getcontext, so if you never use swapcontext function call, no copying would be
ever needed.

	Jakub