[PATCH v5 1/9] selftests/powerpc: Test the preservation of FPU and VMX regs across syscall

[Naveen N. Rao]

On 2016/02/25 10:44AM, Cyril Bur wrote:
> On Wed, 24 Feb 2016 19:57:38 +0530
> "Naveen N. Rao" <naveen.n.rao at linux.vnet.ibm.com> wrote:
> 
> > On 2016/02/23 02:38PM, Cyril Bur wrote:
> > > Test that the non volatile floating point and Altivec registers get
> > > correctly preserved across the fork() syscall.
> > > 
> > > fork() works nicely for this purpose, the registers should be the same for
> > > both parent and child
> > > 
> > > Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
> > > ---

<snip>

> > > diff --git a/tools/testing/selftests/powerpc/basic_asm.h 
> > > b/tools/testing/selftests/powerpc/basic_asm.h
> > > new file mode 100644
> > > index 0000000..f56482f
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/powerpc/basic_asm.h
> > > @@ -0,0 +1,62 @@
> > > +#include <ppc-asm.h>
> > > +#include <asm/unistd.h>
> > > +
> > > +#if defined(_CALL_ELF) && _CALL_ELF == 2
> > > +#define STACK_FRAME_MIN_SIZE 32
> > > +#define STACK_FRAME_TOC_POS  24
> > > +#define __STACK_FRAME_PARAM(_param)  (32 + ((_param)*8))
> > > +#define __STACK_FRAME_LOCAL(_num_params,_var_num)  ((STACK_FRAME_PARAM(_num_params)) + ((_var_num)*8))
> > > +#else
> > > +#define STACK_FRAME_MIN_SIZE 112
> > > +#define STACK_FRAME_TOC_POS  40
> > > +#define __STACK_FRAME_PARAM(i)  (48 + ((i)*8))
> > > +/*
> > > + * Caveat: if a function passed more than 8 params, the caller will have
> > > + * made more space... this should be reflected by this C code.
> > > + * if (_num_params > 8)
> > > + *     total = 112 + ((_num_params - 8) * 8)
> > > + *
> > > + * And substitute the '112' for 'total' in the macro. Doable in preprocessor for ASM?
> > > + */  
> > 
> > Per my understanding, the parameter save area is only for parameters to 
> > functions that *we* call. And since we control that, I don't think we 
> > need to worry about having more than 8 parameters.
> > 
> 
> Yes, I just thought I'd put that there to prevent anyone blindly reusing this
> macro somewhere and getting stung. But you're correct, we don't need to worry
> about this caveat for this code.

Agreed, so probably a simpler warning will suffice. Also, it is worth 
noting that this is not necessarily 8 parameters, but 8 doublewords - we 
may have less number of parameters, but may still need more than 8 
doublewords.

> 
> > > +#define __STACK_FRAME_LOCAL(_num_params,_var_num)  (112 + ((_var_num)*8))
> > > +#endif
> > > +/* Parameter x saved to the stack */
> > > +#define STACK_FRAME_PARAM(var)    __STACK_FRAME_PARAM(var)
> > > +/* Local variable x saved to the stack after x parameters */
> > > +#define STACK_FRAME_LOCAL(num_params,var)    __STACK_FRAME_LOCAL(num_params,var)  
> > 
> > So this works, but I'm wondering if this is really worth the code 
> > complexity - every use needs to determine appropriate extra stack space, 
> > the number of parameters to save and so on.  This is after all for 
> > selftests and so, we probably don't need to be precise in stack space 
> > usage. We can get away using a larger fixed size stack.  That will 
> > simplify a lot of the code further on and future tests won't need to 
> > bother with all the details.
> > 
> 
> So I agree that we don't need to be precise about stack space at all (I'm sure
> you noticed all my stack pushes and pops are very overestimated in size).
> 
> I should probably go back to basics and explain how these macros started. In
> writing all this I got fed up of typing the PUSH_BASIC_STACK and
> POP_BASIC_STACK macro out at the start of each function. I wasn't trying to
> macro out the entire calling convention and honestly I don't think it is a good
> idea. The more easy to use/abstracted the macros get the less flexible they

The simplification here is largely in terms of the stack size and the 
fpu/gpr/vmx save areas. I don't think most tests would need more control 
there. For the few tests that may need it, they can always hand code or 
introduce different macros.

> become. These being selftests, I expect that people might want to do
> funky/questionable/hacky things, flexibility in the macros might help with that.

Sure, but having every test deal with the intricacies of the stack is 
not good. It's better to have simple macros that work for the common 
generic case and consider introducing more specific macros/hard coding 
where more control is desired.

> 
> I feel like if we want to go down the route of fully abstracting out everything
> then perhaps we should be considering C...

If that works for your tests, sure, I think that is not a bad idea.


- Naveen