All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3] xen: arm: correctly handle continuations for 64-bit guests
@ 2015-03-25 15:34 Ian Campbell
  2015-03-25 15:41 ` Andrew Cooper
  2015-03-25 22:39 ` Julien Grall
  0 siblings, 2 replies; 5+ messages in thread
From: Ian Campbell @ 2015-03-25 15:34 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

The 64-bit ABI is different to 32-bit:

 - uses x16 as the op register rather than r12.
 - arguments in x0..x5 and not r0..r5. Using rN here potentially
   truncates.
 - return value goes in x0, not r0.

Hypercalls can only be made directly from kernel space, so checking
the domain's size is sufficient.

The update of regs->pc is duplicated in both halves because the 32-bit
case is going to need fixing to handle Thumb mode (next patch).

Spotted due to spurious -EFAULT when destroying a domain, due to the
hypercall's pointer argument being truncated. I'm unclear why I am
only seeing this now.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
I imagine this needs backporting everywhere...

For reference the git diff -b version of this is:
	@@ -357,6 +357,36 @@ unsigned long hypercall_create_continuation(
	     else
	     {
		 regs = guest_cpu_user_regs();
	+
	+#ifdef CONFIG_ARM_64
	+        if ( !is_32bit_domain(current->domain) )
	+        {
	+            regs->x16 = op;
	+
	+            /* Ensure the hypercall trap instruction is re-executed. */
	+            regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
	+
	+            for ( i = 0; *p != '\0'; i++ )
	+            {
	+                arg = next_arg(p, args);
	+
	+                switch ( i )
	+                {
	+                case 0: regs->x0 = arg; break;
	+                case 1: regs->x1 = arg; break;
	+                case 2: regs->x2 = arg; break;
	+                case 3: regs->x3 = arg; break;
	+                case 4: regs->x4 = arg; break;
	+                case 5: regs->x5 = arg; break;
	+                }
	+            }
	+
	+            /* Return value gets written back to x0 */
	+            rc = regs->x0;
	+        }
	+        else
	+#endif
	+        {
		     regs->r12 = op;

		     /* Ensure the hypercall trap instruction is re-executed.
	 * */
	@@ -380,6 +410,7 @@ unsigned long hypercall_create_continuation(
             /* Return value gets written back to r0 */
             rc = regs->r0;
         }
	+    }

	     va_end(args);
---
 xen/arch/arm/domain.c |   63 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 939d8cd..10f13e4 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -356,29 +356,60 @@ unsigned long hypercall_create_continuation(
     }
     else
     {
-        regs      = guest_cpu_user_regs();
-        regs->r12 = op;
+        regs = guest_cpu_user_regs();
 
-        /* Ensure the hypercall trap instruction is re-executed. */
-        regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
-
-        for ( i = 0; *p != '\0'; i++ )
+#ifdef CONFIG_ARM_64
+        if ( !is_32bit_domain(current->domain) )
         {
-            arg = next_arg(p, args);
+            regs->x16 = op;
 
-            switch ( i )
+            /* Ensure the hypercall trap instruction is re-executed. */
+            regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
+
+            for ( i = 0; *p != '\0'; i++ )
             {
-            case 0: regs->r0 = arg; break;
-            case 1: regs->r1 = arg; break;
-            case 2: regs->r2 = arg; break;
-            case 3: regs->r3 = arg; break;
-            case 4: regs->r4 = arg; break;
-            case 5: regs->r5 = arg; break;
+                arg = next_arg(p, args);
+
+                switch ( i )
+                {
+                case 0: regs->x0 = arg; break;
+                case 1: regs->x1 = arg; break;
+                case 2: regs->x2 = arg; break;
+                case 3: regs->x3 = arg; break;
+                case 4: regs->x4 = arg; break;
+                case 5: regs->x5 = arg; break;
+                }
             }
+
+            /* Return value gets written back to x0 */
+            rc = regs->x0;
         }
+        else
+#endif
+        {
+            regs->r12 = op;
+
+            /* Ensure the hypercall trap instruction is re-executed. */
+            regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
+
+            for ( i = 0; *p != '\0'; i++ )
+            {
+                arg = next_arg(p, args);
+
+                switch ( i )
+                {
+                case 0: regs->r0 = arg; break;
+                case 1: regs->r1 = arg; break;
+                case 2: regs->r2 = arg; break;
+                case 3: regs->r3 = arg; break;
+                case 4: regs->r4 = arg; break;
+                case 5: regs->r5 = arg; break;
+                }
+            }
 
-        /* Return value gets written back to r0 */
-        rc = regs->r0;
+            /* Return value gets written back to r0 */
+            rc = regs->r0;
+        }
     }
 
     va_end(args);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-26 10:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-25 15:34 [PATCH 2/3] xen: arm: correctly handle continuations for 64-bit guests Ian Campbell
2015-03-25 15:41 ` Andrew Cooper
2015-03-25 15:50   ` Ian Campbell
2015-03-25 22:39 ` Julien Grall
2015-03-26 10:04   ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.