Hi, This patch implements the 6 arguments hypercalls. The sixth argument is passed using ebp or r9. Signed-off-by: Jean Guyader <jean.guyader@citrix.com> I tried to make this as clean as I could, but the hack we have to do to use ebp doesn''t really help. Please let me know if you could think of a cleaner way to do it. http://lists.xensource.com/archives/html/xen-devel/2010-12/msg00945.html Jean _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 24/06/2011 12:36, "Jean Guyader" <jean.guyader@eu.citrix.com> wrote:> Hi, > > This patch implements the 6 arguments hypercalls. > The sixth argument is passed using ebp or r9. > > Signed-off-by: Jean Guyader <jean.guyader@citrix.com> > > I tried to make this as clean as I could, but the hack we have > to do to use ebp doesn''t really help. > > Please let me know if you could think of a cleaner way to do it. > > http://lists.xensource.com/archives/html/xen-devel/2010-12/msg00945.htmlWhere is your intended destination for this patch? Sending to the list without explicitly approaching/ccing an appropriate Linux maintainer will likely result in it being totally ignored. -- Keir> Jean > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, This patch implements the 6 arguments hypercalls. The sixth argument is passed using ebp or r9. Signed-off-by: Jean Guyader <jean.guyader@citrix.com> I tried to make this as clean as I could, but the hack we have to do to use ebp doesn''t really help. Please let me know if you could think of a cleaner way to do it. http://lists.xensource.com/archives/html/xen-devel/2010-12/msg00945.html Jean _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jun-24 12:34 UTC
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@eu.citrix.com> > To: > xen-devel@lists.xensource.com > <xen-devel@lists.xensource.com> > Subject: > [Xen-devel] [PATCH] 6 arguments > hypercall for Linux > Date: > Fri, 24 Jun 2011 12:36:38 +0100 > Message-id: > <20110624113638.GB14099@spongy.cam.xci-test.com> > > > Hi, > > This patch implements the 6 arguments hypercalls. > The sixth argument is passed using ebp or r9. > > Signed-off-by: Jean Guyader <jean.guyader@citrix.com> > > 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@lists.xensource.com http://lists.xensource.com/xen-devel