[Xen-devel] [PATCH 1/6] xen/tasklet: Fix return value truncation on arm64

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The use of return_reg() assumes ARM's 32bit ABI.  Therefore, a failure such as
-EINVAL will appear as a large positive number near 4 billion to a 64bit ARM
guest which happens to use continue_hypercall_on_cpu().

Introduce a new arch_hypercall_tasklet_result() hook which is implemented by
both architectures, and drop the return_reg() macros.  This logic will be
extended in a later change to make continuations out of the tasklet work.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>

This was posted on its own previously, but is reset back to v1 now it is in
its series.  This can't be made static inline due to header constraints, but
there is no inherent issue with doing so if the headers become less tangled.

Failing the call with -EINVAL for missing the correct CPU is very rude, and
addressed in a later patch.
---
 xen/arch/arm/traps.c       | 7 +++++++
 xen/arch/x86/hypercall.c   | 7 +++++++
 xen/common/domain.c        | 9 +++++++--
 xen/include/asm-arm/regs.h | 2 --
 xen/include/asm-x86/regs.h | 2 --
 xen/include/xen/domain.h   | 6 ++++++
 6 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d028ec9224..a20474f87c 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1485,6 +1485,13 @@ static void do_trap_hypercall(struct cpu_user_regs 
*regs, register_t *nr,
         regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
 }
 
+void arch_hypercall_tasklet_result(struct vcpu *v, long res)
+{
+    struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
+
+    HYPERCALL_RESULT_REG(regs) = res;
+}
+
 static bool check_multicall_32bit_clean(struct multicall_entry *multi)
 {
     int i;
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 1d42702c6a..7f299d45c6 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -166,6 +166,13 @@ unsigned long hypercall_create_continuation(
 
 #undef NEXT_ARG
 
+void arch_hypercall_tasklet_result(struct vcpu *v, long res)
+{
+    struct cpu_user_regs *regs = &v->arch.user_regs;
+
+    regs->rax = res;
+}
+
 int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
                                 unsigned int mask, ...)
 {
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 611116c7fc..ccf689fcbe 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1665,13 +1665,18 @@ static void continue_hypercall_tasklet_handler(unsigned 
long _info)
 {
     struct migrate_info *info = (struct migrate_info *)_info;
     struct vcpu *v = info->vcpu;
+    long res = -EINVAL;
 
     /* Wait for vcpu to sleep so that we can access its register state. */
     vcpu_sleep_sync(v);
 
     this_cpu(continue_info) = info;
-    return_reg(v) = (info->cpu == smp_processor_id())
-        ? info->func(info->data) : -EINVAL;
+
+    if ( likely(info->cpu == smp_processor_id()) )
+        res = info->func(info->data);
+
+    arch_hypercall_tasklet_result(v, res);
+
     this_cpu(continue_info) = NULL;
 
     if ( info->nest-- == 0 )
diff --git a/xen/include/asm-arm/regs.h b/xen/include/asm-arm/regs.h
index 0e3e56b452..ec091a28a2 100644
--- a/xen/include/asm-arm/regs.h
+++ b/xen/include/asm-arm/regs.h
@@ -57,8 +57,6 @@ static inline bool guest_mode(const struct cpu_user_regs *r)
     return (diff == 0);
 }
 
-#define return_reg(v) ((v)->arch.cpu_info->guest_cpu_user_regs.r0)
-
 register_t get_user_reg(struct cpu_user_regs *regs, int reg);
 void set_user_reg(struct cpu_user_regs *regs, int reg, register_t val);
 
diff --git a/xen/include/asm-x86/regs.h b/xen/include/asm-x86/regs.h
index 725a664e0a..dc00b854e3 100644
--- a/xen/include/asm-x86/regs.h
+++ b/xen/include/asm-x86/regs.h
@@ -15,6 +15,4 @@
     (diff == 0);                                                              \
 })
 
-#define return_reg(v) ((v)->arch.user_regs.rax)
-
 #endif /* __X86_REGS_H__ */
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 769302057b..1cb205d977 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -103,6 +103,12 @@ void domctl_lock_release(void);
 int continue_hypercall_on_cpu(
     unsigned int cpu, long (*func)(void *data), void *data);
 
+/*
+ * Companion to continue_hypercall_on_cpu(), to feed func()'s result back into
+ * vcpu regsiter state.
+ */
+void arch_hypercall_tasklet_result(struct vcpu *v, long res);
+
 extern unsigned int xen_processor_pmbits;
 
 extern bool_t opt_dom0_vcpus_pin;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel