* [PATCH v2] xen: arm: correctly handle continuations for 64-bit guests
@ 2015-03-26 10:54 Ian Campbell
2015-03-26 11:58 ` Julien Grall
0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2015-03-26 10:54 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.
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>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
---
I imagine this needs backporting everywhere...
v2: Pull regs->pc update out of the conditional blocks, no need to
mess around with thumb.
For reference the git diff -b version of this is:
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 939d8cd..887fc10 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -357,11 +357,38 @@ unsigned long hypercall_create_continuation(
else
{
regs = guest_cpu_user_regs();
- regs->r12 = op;
/* Ensure the hypercall trap instruction is re-executed. */
regs->pc -= 4; /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
+#ifdef CONFIG_ARM_64
+ if ( !is_32bit_domain(current->domain) )
+ {
+ regs->x16 = op;
+
+ 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;
+
for ( i = 0; *p != '\0'; i++ )
{
arg = next_arg(p, args);
@@ -380,6 +407,7 @@ unsigned long hypercall_create_continuation(
/* Return value gets written back to r0 */
rc = regs->r0;
}
+ }
va_end(args);
---
xen/arch/arm/domain.c | 54 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 939d8cd..887fc10 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -356,29 +356,57 @@ 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 )
+ 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;
- /* Return value gets written back to r0 */
- rc = regs->r0;
+ 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;
+ }
}
va_end(args);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xen: arm: correctly handle continuations for 64-bit guests
2015-03-26 10:54 [PATCH v2] xen: arm: correctly handle continuations for 64-bit guests Ian Campbell
@ 2015-03-26 11:58 ` Julien Grall
2015-03-30 9:07 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Julien Grall @ 2015-03-26 11:58 UTC (permalink / raw)
To: Ian Campbell, xen-devel; +Cc: tim, stefano.stabellini
Hi Ian,
On 26/03/2015 10:54, Ian Campbell wrote:
> 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.
>
> 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>
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> ---
> I imagine this needs backporting everywhere...
>
> v2: Pull regs->pc update out of the conditional blocks, no need to
> mess around with thumb.
FWIW, with this change I confirm my Reviewed-by.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xen: arm: correctly handle continuations for 64-bit guests
2015-03-26 11:58 ` Julien Grall
@ 2015-03-30 9:07 ` Ian Campbell
0 siblings, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2015-03-30 9:07 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, xen-devel
On Thu, 2015-03-26 at 11:58 +0000, Julien Grall wrote:
> Hi Ian,
>
> On 26/03/2015 10:54, Ian Campbell wrote:
> > 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.
> >
> > 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>
> > Reviewed-by: Julien Grall <julien.grall@linaro.org>
> > ---
> > I imagine this needs backporting everywhere...
> >
> > v2: Pull regs->pc update out of the conditional blocks, no need to
> > mess around with thumb.
>
> FWIW, with this change I confirm my Reviewed-by.
Thanks, applied.
Ian.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-03-30 9:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 10:54 [PATCH v2] xen: arm: correctly handle continuations for 64-bit guests Ian Campbell
2015-03-26 11:58 ` Julien Grall
2015-03-30 9:07 ` 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.