Xen devel - Jun 2011 - [PATCH] Linux: 6 arguments hypercall v2

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>

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 Fri, 2011-06-24 at 15:00 +0100, Jean Guyader wrote:
> Hi,
> 
> This patch implements the 6 arguments hypercalls.
> The sixth argument is passed using ebp or r9.
Looks pretty good.

I was able to get rid of more of the ifdefery etc with the following. It
appears to do the right thing (by inspection of compiled code, not
actual testing).

Defining both __res and __arg6 as register("eax") seems slightly
dubious, as is including them both in  __HYPERCALL_6PARAM, but gcc
doesn''t seem to mind and it makes  __HYPERCALL_6PARAM the same in both
cases and gets rid of __HYPERCALL_DECLS6.

Your version is probably less likely to tickle ggc bugs/behaviour
changes in the future, so it''s likely safer I think.

diff --git a/arch/x86/include/asm/xen/hypercall.h
b/arch/x86/include/asm/xen/hypercall.h
index d240ea9..cacd1b9 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[];
@@ -91,6 +90,18 @@ 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.
+ */
+#define __HYPERCALL_ARG6REG	"eax"
+#define __HYPERCALL6_PRE        "push %%ebp ; mov %%eax, %%ebp ; "
+#define __HYPERCALL6_POST       ";" "pop %%ebp"
 #else
 #define __HYPERCALL_RETREG	"rax"
 #define __HYPERCALL_ARG1REG	"rdi"
@@ -98,6 +109,9 @@ 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       ""
 #endif
 
 #define __HYPERCALL_DECLS						\
@@ -106,7 +120,8 @@ 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;
+	register unsigned long __arg5 asm(__HYPERCALL_ARG5REG) = __arg5; \
+	register unsigned long __arg6 asm(__HYPERCALL_ARG6REG) = __arg6;
 
 #define __HYPERCALL_0PARAM	"=r" (__res)
 #define __HYPERCALL_1PARAM	__HYPERCALL_0PARAM, "+r" (__arg1)
@@ -114,6 +129,7 @@ extern struct { char _entry[32]; } hypercall_page[];
 #define __HYPERCALL_3PARAM	__HYPERCALL_2PARAM, "+r" (__arg3)
 #define __HYPERCALL_4PARAM	__HYPERCALL_3PARAM, "+r" (__arg4)
 #define __HYPERCALL_5PARAM	__HYPERCALL_4PARAM, "+r" (__arg5)
+#define __HYPERCALL_6PARAM	__HYPERCALL_5PARAM, "+r" (__arg6)
 
 #define __HYPERCALL_0ARG()
 #define __HYPERCALL_1ARG(a1)						\
@@ -126,7 +142,10 @@ extern struct { char _entry[32]; } hypercall_page[];
 	__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_6ARG(a1,a2,a3,a4,a5,a6)				\
+	__HYPERCALL_5ARG(a1,a2,a3,a4,a5) __arg6 = (unsigned long)(a6);
 
+#define __HYPERCALL_CLOBBER6	"memory"
 #define __HYPERCALL_CLOBBER5	"memory"
 #define __HYPERCALL_CLOBBER4	__HYPERCALL_CLOBBER5, __HYPERCALL_ARG5REG
 #define __HYPERCALL_CLOBBER3	__HYPERCALL_CLOBBER4, __HYPERCALL_ARG4REG
@@ -200,6 +219,17 @@ 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				\
+		      : __HYPERCALL_ENTRY(name)				\
+		      : __HYPERCALL_CLOBBER6);				\
+	(type)__res;							\
+})
+
 static inline long
 privcmd_call(unsigned call,
 	     unsigned long a1, unsigned long a2,
@@ -220,6 +250,7 @@ privcmd_call(unsigned call,
 static inline int
 HYPERVISOR_set_trap_table(struct trap_info *table)
 {
+	return _hypercall6(int, set_trap_table, 1, 2, 3, 4, 5, 6);
 	return _hypercall1(int, set_trap_table, table);
 }
 
> 
> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
> 
> 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 03:46, Ian Campbell wrote:
> On Fri, 2011-06-24 at 15:00 +0100, Jean Guyader wrote:
> > Hi,
> > 
> > This patch implements the 6 arguments hypercalls.
> > The sixth argument is passed using ebp or r9.
> 
> Looks pretty good.
> 
> I was able to get rid of more of the ifdefery etc with the following. It
> appears to do the right thing (by inspection of compiled code, not
> actual testing).
> 
> Defining both __res and __arg6 as register("eax") seems slightly
> dubious, as is including them both in  __HYPERCALL_6PARAM, but gcc
> doesn''t seem to mind and it makes  __HYPERCALL_6PARAM the same in
both
> cases and gets rid of __HYPERCALL_DECLS6.
> 
> Your version is probably less likely to tickle ggc bugs/behaviour
> changes in the future, so it''s likely safer I think.
> 
Thanks that makes the code much nicer, I didn''t really want to declare
to variable in eax but if that works fine.

Also my patch modifies  HYPERVISOR_set_trap_table by mistake but
shouldn''t. I only used that for testing _hypercall6. I''ll send
another version with your changes in but without the mistake
at the end.

Jean

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2011-06-24 at 15:59 +0100, Jean Guyader wrote:
> On 24/06 03:46, Ian Campbell wrote:
> > On Fri, 2011-06-24 at 15:00 +0100, Jean Guyader wrote:
> > > Hi,
> > > 
> > > This patch implements the 6 arguments hypercalls.
> > > The sixth argument is passed using ebp or r9.
> > 
> > Looks pretty good.
> > 
> > I was able to get rid of more of the ifdefery etc with the following.
It
> > appears to do the right thing (by inspection of compiled code, not
> > actual testing).
> > 
> > Defining both __res and __arg6 as register("eax") seems
slightly
> > dubious, as is including them both in  __HYPERCALL_6PARAM, but gcc
> > doesn''t seem to mind and it makes  __HYPERCALL_6PARAM the
same in both
> > cases and gets rid of __HYPERCALL_DECLS6.
> > 
> > Your version is probably less likely to tickle ggc bugs/behaviour
> > changes in the future, so it''s likely safer I think.
> > 
> 
> Thanks that makes the code much nicer, I didn''t really want to
declare
> to variable in eax but if that works fine.
Well, with my specific version of gcc (4.4.5-8)... I didn''t check the
docs to see if it supposed to be ok or not or anything like that.
> Also my patch modifies  HYPERVISOR_set_trap_table by mistake but
> shouldn''t. I only used that for testing _hypercall6.
I did the same ;-)
>  I''ll send
> another version with your changes in but without the mistake
> at the end.
Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel