[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] 6 arguments hypercall for Linux
On Fri, 2011-06-24 at 12:36 +0100, Jean Guyader wrote: > From: > Jean Guyader > <jean.guyader@xxxxxxxxxxxxx> > To: > xen-devel@xxxxxxxxxxxxxxxxxxx > <xen-devel@xxxxxxxxxxxxxxxxxxx> > Subject: > [Xen-devel] [PATCH] 6 arguments > hypercall for Linux > Date: > Fri, 24 Jun 2011 12:36:38 +0100 > Message-id: > <20110624113638.GB14099@xxxxxxxxxxxxxxxxxxxxxxx> > > > Hi, > > This patch implements the 6 arguments hypercalls. > The sixth argument is passed using ebp or r9. > > Signed-off-by: Jean Guyader <jean.guyader@xxxxxxxxxx> > > I tried to make this as clean as I could, but the hack we have > to do to use ebp doesn't really help. You seem to have included a whole bunch of code motion and whitespace changes in this patch. It makes it really hard to review. I guess the code motion is to allow moving some stuff into the existing #ifdef? I think it would be cleaner to simply repeat the ifdef in the __HYPERCALL_<N>PARAM and ARG etc blocks as necessary. The benefit of keeping those together I think outweighs the benefit of having a single ifdef. > > diff --git a/arch/x86/include/asm/xen/hypercall.h > b/arch/x86/include/asm/xen/hypercall.h > index d240ea9..18327a7 100644 > --- a/arch/x86/include/asm/xen/hypercall.h > +++ b/arch/x86/include/asm/xen/hypercall.h > @@ -74,8 +74,7 @@ > * - clobber the rest > * > * The result certainly isn't pretty, and it really shows up cpp's > - * weakness as as macro language. Sorry. (But let's just give thanks > - * there aren't more than 5 arguments...) > + * weakness as as macro language. Sorry. > */ > > extern struct { char _entry[32]; } hypercall_page[]; > @@ -84,6 +83,13 @@ extern struct { char _entry[32]; } hypercall_page[]; > #define __HYPERCALL_ENTRY(x) \ > [offset] "i" (__HYPERVISOR_##x * sizeof(hypercall_page[0])) > > +#define __HYPERCALL_0PARAM "=r" (__res) > +#define __HYPERCALL_1PARAM __HYPERCALL_0PARAM, "+r" (__arg1) > +#define __HYPERCALL_2PARAM __HYPERCALL_1PARAM, "+r" (__arg2) > +#define __HYPERCALL_3PARAM __HYPERCALL_2PARAM, "+r" (__arg3) > +#define __HYPERCALL_4PARAM __HYPERCALL_3PARAM, "+r" (__arg4) > +#define __HYPERCALL_5PARAM __HYPERCALL_4PARAM, "+r" (__arg5) Why did these need to move and/or why is __HYPERCALL_6PARAM not declared here too? > + > #ifdef CONFIG_X86_32 > #define __HYPERCALL_RETREG "eax" > #define __HYPERCALL_ARG1REG "ebx" > @@ -91,6 +97,23 @@ extern struct { char _entry[32]; } hypercall_page[]; > #define __HYPERCALL_ARG3REG "edx" > #define __HYPERCALL_ARG4REG "esi" > #define __HYPERCALL_ARG5REG "edi" > + > +/* > +** On 32b we are out of free registers to pass in > +** the 6th argument of the hypercall, the last one > +** available is ebp. > +** %ebp is already being used by linux so we save it > +** then we move %eax which is the 6th argument in %ebp. > +** On the way back of the hypercall we restore %ebp. > +*/ Linux comment style is: /* * Blah blah blah blah blah * blah blah blah. */ > +#define __HYPERCALL6_PRE "push %%ebp ; mov %%eax, %%ebp ; " > +#define __HYPERCALL6_POST ";" "pop %%ebp" HYPERCALL_6... for consistency. > +#define __HYPERCALL_6PARAM __HYPERCALL_5PARAM > +#define __HYPERCALL6_ENTRY(x, a6) \ > + __HYPERCALL_ENTRY(x) , "0" ((long)(a6)) > +#define __HYPERCALL_6ARG(a1,a2,a3,a4,a5, a6) \ > + __HYPERCALL_5ARG(a1,a2,a3,a4, a5) Can't you define this as "__HYPERCALL_5ARG(....) __res = (unsigned long)a6"? Then the the special HYPERCALL6_ENTRY goes away? HYPERCALL6_PRE remains the same. > +#define __HYPERCALL_DECLS6 (void)0; > #else > #define __HYPERCALL_RETREG "rax" > #define __HYPERCALL_ARG1REG "rdi" > @@ -98,6 +121,15 @@ extern struct { char _entry[32]; } hypercall_page[]; > #define __HYPERCALL_ARG3REG "rdx" > #define __HYPERCALL_ARG4REG "r10" > #define __HYPERCALL_ARG5REG "r8" > +#define __HYPERCALL_ARG6REG "r9" > +#define __HYPERCALL6_PRE "" > +#define __HYPERCALL6_POST "" > +#define __HYPERCALL6_ENTRY(x, a6) __HYPERCALL_ENTRY(x) > +#define __HYPERCALL_6ARG(a1,a2,a3,a4,a5, a6) > \ > + __HYPERCALL_5ARG(a1,a2,a3,a4, a5) __arg6 = (unsigned long)(a6); > +#define __HYPERCALL_6PARAM __HYPERCALL_5PARAM, "+r" (__arg6) > +#define __HYPERCALL_DECLS6 > \ > + register unsigned long __arg6 asm(__HYPERCALL_ARG6REG) = __arg6; > #endif > > #define __HYPERCALL_DECLS \ > @@ -106,27 +138,22 @@ extern struct { char _entry[32]; } hypercall_page[]; > register unsigned long __arg2 asm(__HYPERCALL_ARG2REG) = __arg2; \ > register unsigned long __arg3 asm(__HYPERCALL_ARG3REG) = __arg3; \ > register unsigned long __arg4 asm(__HYPERCALL_ARG4REG) = __arg4; \ > - register unsigned long __arg5 asm(__HYPERCALL_ARG5REG) = __arg5; > - > -#define __HYPERCALL_0PARAM "=r" (__res) > -#define __HYPERCALL_1PARAM __HYPERCALL_0PARAM, "+r" (__arg1) > -#define __HYPERCALL_2PARAM __HYPERCALL_1PARAM, "+r" (__arg2) > -#define __HYPERCALL_3PARAM __HYPERCALL_2PARAM, "+r" (__arg3) > -#define __HYPERCALL_4PARAM __HYPERCALL_3PARAM, "+r" (__arg4) > -#define __HYPERCALL_5PARAM __HYPERCALL_4PARAM, "+r" (__arg5) > + register unsigned long __arg5 asm(__HYPERCALL_ARG5REG) = __arg5; \ > + __HYPERCALL_DECLS6 > > #define __HYPERCALL_0ARG() > -#define __HYPERCALL_1ARG(a1) \ > - __HYPERCALL_0ARG() __arg1 = (unsigned long)(a1); > -#define __HYPERCALL_2ARG(a1,a2) > \ > - __HYPERCALL_1ARG(a1) __arg2 = (unsigned long)(a2); > -#define __HYPERCALL_3ARG(a1,a2,a3) \ > - __HYPERCALL_2ARG(a1,a2) __arg3 = (unsigned long)(a3); > -#define __HYPERCALL_4ARG(a1,a2,a3,a4) \ > - __HYPERCALL_3ARG(a1,a2,a3) __arg4 = (unsigned long)(a4); > -#define __HYPERCALL_5ARG(a1,a2,a3,a4,a5) \ > - __HYPERCALL_4ARG(a1,a2,a3,a4) __arg5 = (unsigned long)(a5); > - > +#define __HYPERCALL_1ARG(a1) > \ > + __HYPERCALL_0ARG() __arg1 = (unsigned long)(a1); > +#define __HYPERCALL_2ARG(a1,a2) > \ > + __HYPERCALL_1ARG(a1) __arg2 = (unsigned long)(a2); > +#define __HYPERCALL_3ARG(a1,a2,a3) > \ > + __HYPERCALL_2ARG(a1,a2) __arg3 = (unsigned long)(a3); > +#define __HYPERCALL_4ARG(a1,a2,a3,a4) > \ > + __HYPERCALL_3ARG(a1,a2,a3) __arg4 = (unsigned long)(a4); > +#define __HYPERCALL_5ARG(a1,a2,a3,a4,a5) > \ > + __HYPERCALL_4ARG(a1,a2,a3,a4) __arg5 = (unsigned long)(a5); You didn't actually change anything other than whitespace here? > + > +#define __HYPERCALL_CLOBBER6 "memory" > #define __HYPERCALL_CLOBBER5 "memory" > #define __HYPERCALL_CLOBBER4 __HYPERCALL_CLOBBER5, __HYPERCALL_ARG5REG > #define __HYPERCALL_CLOBBER3 __HYPERCALL_CLOBBER4, __HYPERCALL_ARG4REG > @@ -200,6 +227,19 @@ extern struct { char _entry[32]; } hypercall_page[]; > (type)__res; \ > }) > > +#define _hypercall6(type, name, a1, a2, a3, a4, a5, a6) \ > +({ \ > + __HYPERCALL_DECLS; \ > + __HYPERCALL_6ARG(a1, a2, a3, a4, a5, a6); \ > + asm volatile ( __HYPERCALL6_PRE \ > + __HYPERCALL \ > + __HYPERCALL6_POST \ > + : __HYPERCALL_6PARAM \ > + : __HYPERCALL6_ENTRY(name, a6) \ > + : __HYPERCALL_CLOBBER6); \ > + (type)__res; \ > +}) > + > static inline long > privcmd_call(unsigned call, > unsigned long a1, unsigned long a2, > > > > > > > > plain text document attachment (ATT00001.txt) > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |