[RFC 00/10] xen/arm: Implement the hypercall handling in assembly

[RFC 00/10] xen/arm: Implement the hypercall handling in assembly

[Julien Grall]

Hi all,

This patch aims is a first attempt to write the hypercall dispatch in assembly
in order to avoid reloading the hypercall arguments from the stack.

I've posted an early version to get comments and only the ARM64 side has been
implemented.

A branch with the code can be found here:

git://xenbits.xen.org/people/julieng/xen-unstable.git branch asm-hypercall-rfc

Sincerely yours,

Julien Grall (10):
  xen/arm: move lr, push and pop in a separate header
  xen/arm: Drop unused defines in asm/bug.h
  xen/arm: Implement assembly version of WARN/BUG/ASSERT_FAILED...
  xen/arm64: Add an assembly macro for perf counter
  xen/arm: gic_inject: Introduce a local variable to store current
  xen/arm: gic: Allow the LRs to be cleared lazily
  xen/arm: Implement the code to dispatch the hypercall in assembly
  xen/arm64: Implement the hypercall handing fully in assembly
  xen/arm: Remove the C version of do_trap_hypercall
  xen/arm: Factorize the C code to dispatch HVC

 xen/arch/arm/Makefile                 |   1 +
 xen/arch/arm/arm64/Makefile           |   1 +
 xen/arch/arm/arm64/asm-offsets.c      |  11 +++
 xen/arch/arm/arm64/entry.S            | 165 +++++++++++++++++++++++++++++----
 xen/arch/arm/arm64/hypercall.S        |  27 ++++++
 xen/arch/arm/gic.c                    |  17 +++-
 xen/arch/arm/hypercall.S              |  88 ++++++++++++++++++
 xen/arch/arm/traps.c                  | 167 +++++-----------------------------
 xen/include/asm-arm/arm64/asm_defns.h |  62 +++++++++++++
 xen/include/asm-arm/arm64/bug.h       |   4 +
 xen/include/asm-arm/asm_defns.h       |   3 +
 xen/include/asm-arm/bug.h             |  39 ++++++--
 xen/include/asm-arm/domain.h          |   7 +-
 xen/include/asm-arm/hypercall.h       |   9 ++
 xen/include/asm-arm/processor.h       |   3 +
 15 files changed, 432 insertions(+), 172 deletions(-)
 create mode 100644 xen/arch/arm/arm64/hypercall.S
 create mode 100644 xen/arch/arm/hypercall.S
 create mode 100644 xen/include/asm-arm/arm64/asm_defns.h

-- 
2.1.4

[RFC 01/10] xen/arm: move lr, push and pop in a separate header

[Julien Grall]

The shorthand lr and the macros push/pop are useful for any assembly
code.

Move them in asm/arm64/asm_defns.h

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/arm64/entry.S            | 17 -----------------
 xen/include/asm-arm/arm64/asm_defns.h | 26 ++++++++++++++++++++++++++
 xen/include/asm-arm/asm_defns.h       |  3 +++
 3 files changed, 29 insertions(+), 17 deletions(-)
 create mode 100644 xen/include/asm-arm/arm64/asm_defns.h

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 9cda8f1..2b9d4cd 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -4,23 +4,6 @@
 #include <public/xen.h>
 
 /*
- * Register aliases.
- */
-lr      .req    x30             // link register
-
-/*
- * Stack pushing/popping (register pairs only). Equivalent to store decrement
- * before, load increment after.
- */
-        .macro  push, xreg1, xreg2
-        stp     \xreg1, \xreg2, [sp, #-16]!
-        .endm
-
-        .macro  pop, xreg1, xreg2
-        ldp     \xreg1, \xreg2, [sp], #16
-        .endm
-
-/*
  * Save/restore guest mode specific state, outer stack frame
  */
         .macro  entry_guest, compat
diff --git a/xen/include/asm-arm/arm64/asm_defns.h b/xen/include/asm-arm/arm64/asm_defns.h
new file mode 100644
index 0000000..7085c26
--- /dev/null
+++ b/xen/include/asm-arm/arm64/asm_defns.h
@@ -0,0 +1,26 @@
+#ifndef __ASM_ARM_ARM64_DEFNS_H__
+#define __ASM_ARM_ARM64_DEFNS_H__
+
+#ifdef __ASSEMBLY__
+
+/*
+ * Register aliases.
+ */
+lr      .req    x30             // link register
+
+/*
+ * Stack pushing/popping (register pairs only). Equivalent to store decrement
+ * before, load increment after.
+ */
+.macro  push, xreg1, xreg2
+stp     \xreg1, \xreg2, [sp, #-16]!
+.endm
+
+.macro  pop, xreg1, xreg2
+ldp     \xreg1, \xreg2, [sp], #16
+.endm
+
+#endif /* !__ASSEMBLY__ */
+
+
+#endif /* __ASM_ARM_ARM64_DEFNS_H__ */
diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
index 02be83e..365263b 100644
--- a/xen/include/asm-arm/asm_defns.h
+++ b/xen/include/asm-arm/asm_defns.h
@@ -12,6 +12,9 @@
 # define __OP32
 #elif defined(CONFIG_ARM_64)
 # define __OP32 "w"
+
+# include <asm/arm64/asm_defns.h>
+
 #else
 # error "unknown ARM variant"
 #endif
-- 
2.1.4

[RFC 02/10] xen/arm: Drop unused defines in asm/bug.h

[Julien Grall]

The defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH and BUG_LINE_HI_WIDTH have
been introduced by mistake in a previous patch. They are not used at all
so drop them.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/include/asm-arm/bug.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index ab9e811..fd3e95e 100644
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -11,10 +11,6 @@
 # error "unknown ARM variant"
 #endif
 
-#define BUG_DISP_WIDTH    24
-#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
-#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
-
 struct bug_frame {
     signed int loc_disp;    /* Relative address to the bug address */
     signed int file_disp;   /* Relative address to the filename */
-- 
2.1.4

[RFC 03/10] xen/arm: Implement assembly version of WARN/BUG/ASSERT_FAILED...

[Julien Grall]

... to produce clear error message in assembly code.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
    TODO:
        - Implement the arm32 bits
---
 xen/arch/arm/arm64/entry.S      |  1 +
 xen/include/asm-arm/arm64/bug.h |  4 ++++
 xen/include/asm-arm/bug.h       | 37 +++++++++++++++++++++++++++++++++----
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 2b9d4cd..93c80ff 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -1,5 +1,6 @@
 #include <xen/config.h>
 #include <asm/asm_defns.h>
+#include <asm/bug.h>
 #include <asm/regs.h>
 #include <public/xen.h>
 
diff --git a/xen/include/asm-arm/arm64/bug.h b/xen/include/asm-arm/arm64/bug.h
index 42b0e4f..9670505 100644
--- a/xen/include/asm-arm/arm64/bug.h
+++ b/xen/include/asm-arm/arm64/bug.h
@@ -5,6 +5,10 @@
 
 #define BRK_BUG_FRAME 1
 
+#ifndef __ASSEMBLY__
 #define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME)
+#else
+#define BUG_INSTR brk BRK_BUG_FRAME
+#endif
 
 #endif /* __ARM_ARM64_BUG_H__ */
diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index fd3e95e..ffd7f6c 100644
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -11,6 +11,12 @@
 # error "unknown ARM variant"
 #endif
 
+#define BUGFRAME_warn   0
+#define BUGFRAME_bug    1
+#define BUGFRAME_assert 2
+
+#ifndef __ASSEMBLY__
+
 struct bug_frame {
     signed int loc_disp;    /* Relative address to the bug address */
     signed int file_disp;   /* Relative address to the filename */
@@ -24,10 +30,6 @@ struct bug_frame {
 #define bug_line(b) ((b)->line)
 #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
 
-#define BUGFRAME_warn   0
-#define BUGFRAME_bug    1
-#define BUGFRAME_assert 2
-
 /* Many versions of GCC doesn't support the asm %c parameter which would
  * be preferable to this unpleasantness. We use mergeable string
  * sections to avoid multiple copies of the string appearing in the
@@ -71,6 +73,33 @@ extern const struct bug_frame __start_bug_frames[],
 
 int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc);
 
+#else /* !__ASSEMBLY__ */
+
+.macro BUG_FRAME type, line , file, has_msg, msg
+.L\@bug:BUG_INSTR
+.pushsection .rodata.str, "aMS", %progbits, 1
+    .L\@s1:.asciz "\file"
+    .L\@s2:
+    .if \has_msg
+    .asciz "\msg"
+    .endif
+.popsection
+.pushsection .bug_frames.\type, "a", %progbits
+    .L\@bf:
+    .long (.L\@bug - .L\@bf)
+    .long (.L\@s1 - .L\@bf)
+    .long (.L\@s2 - .L\@bf)
+    .hword \line, 0
+.popsection
+.endm
+
+#define WARN    BUG_FRAME BUGFRAME_warn, __LINE__, __FILE__, 0, 0
+#define BUG     BUG_FRAME BUGFRAME_bug, __LINE__, __FILE__, 0, 0
+#define ASSERT_FAILED(msg)                                          \
+    BUG_FRAME BUGFRAME_assert, __LINE__, __FILE__, 1, msg
+
+#endif
+
 #endif /* __ARM_BUG_H__ */
 /*
  * Local variables:
-- 
2.1.4

[RFC 04/10] xen/arm64: Add an assembly macro for perf counter

[Julien Grall]

Allow assembly code to use performance counter.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/include/asm-arm/arm64/asm_defns.h | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/xen/include/asm-arm/arm64/asm_defns.h b/xen/include/asm-arm/arm64/asm_defns.h
index 7085c26..36442b8 100644
--- a/xen/include/asm-arm/arm64/asm_defns.h
+++ b/xen/include/asm-arm/arm64/asm_defns.h
@@ -20,6 +20,42 @@ stp     \xreg1, \xreg2, [sp, #-16]!
 ldp     \xreg1, \xreg2, [sp], #16
 .endm
 
+#ifdef PERF_COUNTERS
+
+.macro perfc_incr, name, idx
+push    x0, x1                  /* x0 and x1 are used by the macro -> save them */
+
+/*
+ * Compute the address of the perf counters
+ *
+ * Note that each perf counters are 4 bytes and stored percpu.
+ */
+mov     x0, \idx                /* x0 <- index from ASM_PERFC_* */
+add     x0, x0, #\name          /* x0 <- index in the percpu array */
+lsl     x0, x0, 2               /* x0 <- offset from per_cpu__perfcounters. */
+ldr     x1, =per_cpu__perfcounters
+add     x0, x0, x1              /* x0 <- offset in the percpu area */
+mrs     x1, TPIDR_EL2           /* x1 <- per cpu offset */
+add     x0, x0, x1              /* x0 <- address of the perf counter for this cpu */
+
+/*
+ * Increment the perf counters.
+ *
+ * Note it's not atomic so does the C version.
+ */
+ldr     x1, [x0]                /* x1 <- current value of the counter */
+add     x1, x1, 1               /* x1 <- increment the counter */
+str     x1, [x0]                /* Store the counters incremented */
+
+pop     x0, x1                  /* Restore x0, x1 */
+.endm
+
+#define PERFC_INCR(_name, _idx) perfc_incr  ASM_PERFC_##_name, _idx
+
+#else
+#define PERFC_INCR(_name, _idx)
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 
-- 
2.1.4

[RFC 05/10] xen/arm: gic_inject: Introduce a local variable to store current

[Julien Grall]

current is a define which is replaced by a read to a system register.
Rather than reading the system registers multiple store the result in a
local variable.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/gic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 1e1e5ba..37e579b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -596,11 +596,13 @@ out:
 
 void gic_inject(void)
 {
+    struct vcpu *v = current;
+
     ASSERT(!local_irq_is_enabled());
 
-    gic_restore_pending_irqs(current);
+    gic_restore_pending_irqs(v);
 
-    if ( !list_empty(&current->arch.vgic.lr_pending) && lr_all_full() )
+    if ( !list_empty(&v->arch.vgic.lr_pending) && lr_all_full() )
         gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1);
 }
 
-- 
2.1.4

[RFC 06/10] xen/arm: gic: Allow the LRs to be cleared lazily

[Julien Grall]

Currently the LRs are cleared every time we entered in Xen from a guest
before any action is taken by the hypervisor. This requiring to reload
the guest registers from the stack when an hypercall is handled for
instance.

However, we only need to clear the LRs for the following actions:
    - An interrupt is received and injected
    - Checking hypercall preemption
    - Before re-entering in the guest

A new flag has been introduced per vCPU in vgic.flags to know whether
the LRs has been cleared since the vCPU has been running. The flag is
always cleared before switching back to the guest vCPU.

As clearing the LRs is now lazy, we have to check if they are cleared
every time we enter in the hypervisor and not only when we entered from
a lower exception state.

Note that actually no one is taking advantage of clearing the LRs
lazily. A follow-up patch will enable this possibility.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/gic.c           | 11 +++++++++++
 xen/arch/arm/traps.c         |  9 ++++++++-
 xen/include/asm-arm/domain.h |  7 ++++++-
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 37e579b..5d70251 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -476,6 +476,13 @@ void gic_clear_lrs(struct vcpu *v)
     if ( is_idle_vcpu(v) )
         return;
 
+    /*
+     * Check the LRs has already been cleared for this vCPU since the
+     * last time it has running.
+     */
+    if ( test_and_set_bit(_VGIC_LRS_CLEARED, &v->arch.vgic.flags) )
+        return;
+
     gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 0);
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
@@ -566,6 +573,8 @@ int gic_events_need_delivery(void)
     int active_priority;
     int rc = 0;
 
+    gic_clear_lrs(v);
+
     mask_priority = gic_hw_ops->read_vmcr_priority();
     active_priority = find_next_bit(&apr, 32, 0);
 
@@ -602,6 +611,8 @@ void gic_inject(void)
 
     gic_restore_pending_irqs(v);
 
+    clear_bit(_VGIC_LRS_CLEARED, &v->arch.vgic.flags);
+
     if ( !list_empty(&v->arch.vgic.lr_pending) && lr_all_full() )
         gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1);
 }
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c49bd3f..f222d96 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2451,7 +2451,12 @@ bad_data_abort:
 
 static void enter_hypervisor_head(struct cpu_user_regs *regs)
 {
-    if ( guest_mode(regs) )
+    /*
+     * enter_hypervisor_head is called by most of the traps generated by the
+     * processor. It could be taken from the hypervisor or the guest.
+     * However current is only valid after Xen has finished to boot.
+     */
+    if ( likely(system_state > SYS_STATE_active) )
         gic_clear_lrs(current);
 }
 
@@ -2602,6 +2607,8 @@ asmlinkage void do_trap_fiq(struct cpu_user_regs *regs)
 
 asmlinkage void leave_hypervisor_tail(void)
 {
+    gic_clear_lrs(current);
+
     while (1)
     {
         local_irq_disable();
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index e7e40da..401b4d0 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -245,7 +245,12 @@ struct arch_vcpu
 
         /* GICv3: redistributor base and flags for this vCPU */
         paddr_t rdist_base;
-#define VGIC_V3_RDIST_LAST  (1 << 0)        /* last vCPU of the rdist */
+/* Last vCPU of the rdist */
+#define _VGIC_V3_RDIST_LAST (0)
+#define VGIC_V3_RDIST_LAST (1 << _VGIC_V3_RDIST_LAST)
+/* LRs have been cleared for this vCPU */
+#define _VGIC_LRS_CLEARED   (1)
+#define VGIC_LRS_CLEARED    (1 << _VGIC_LRS_CLEARED)
         uint8_t flags;
     } vgic;
 
-- 
2.1.4

[RFC 07/10] xen/arm: Implement the code to dispatch the hypercall in assembly

[Julien Grall]

Each hypercalls have different number of arguments. To avoid worry about
the number of arguments, all the hypercall handler are cast to a
5-arguments function. However, based on the C-spec (see 6.3.2.3 paragraph 8
and 6.7.5.3 paraph 2) the behavior is undefined.

This is also the first step to handle the hypercall directly in
assembly.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
    TODO:
        - Implement do_dispatch_hypercall for ARM32
---
 xen/arch/arm/Makefile           |  1 +
 xen/arch/arm/arm64/Makefile     |  1 +
 xen/arch/arm/arm64/hypercall.S  | 27 +++++++++++++
 xen/arch/arm/hypercall.S        | 88 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c            | 90 ++++++-----------------------------------
 xen/include/asm-arm/hypercall.h |  9 +++++
 6 files changed, 139 insertions(+), 77 deletions(-)
 create mode 100644 xen/arch/arm/arm64/hypercall.S
 create mode 100644 xen/arch/arm/hypercall.S

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 4ac5edd..3bf7455 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@ obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += hypercall.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c7243f5..1255b4b 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -8,5 +8,6 @@ obj-y += vfp.o
 obj-y += smpboot.o
 obj-y += domctl.o
 obj-y += cache.o
+obj-y += hypercall.o
 
 obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm64/hypercall.S b/xen/arch/arm/arm64/hypercall.S
new file mode 100644
index 0000000..d831181
--- /dev/null
+++ b/xen/arch/arm/arm64/hypercall.S
@@ -0,0 +1,27 @@
+#include <xen/config.h>
+#include <asm/asm_defns.h>
+
+/*
+ * int do_dispatch_hypercall(register_t arg1, register_t arg2,
+ *                           register_t arg3, register_t arg4,
+ *                           register_t arg5, register_t nr)
+ *
+ * x0 - arg1
+ * x1 - arg2
+ * x2 - arg3
+ * x3 - arg4
+ * x4 - arg5
+ * x5 - nr
+ *
+ * return the hypercall result in x0.
+ */
+ENTRY(do_dispatch_hypercall)
+    push    x19, lr                 /* Save lr (x19 is here because the stack need to be 16 bytes aligned */
+    ldr     x10, =hypercall_table
+    ldr     x10, [x10, x5, lsl #3]  /* x10 := pointer to hypercall function */
+
+    blr     x10                     /* Call the hypercall */
+
+    pop     x19, lr                 /* Restore lr */
+
+    ret
diff --git a/xen/arch/arm/hypercall.S b/xen/arch/arm/hypercall.S
new file mode 100644
index 0000000..c8680eb
--- /dev/null
+++ b/xen/arch/arm/hypercall.S
@@ -0,0 +1,88 @@
+#include <xen/config.h>
+#include <public/xen.h>
+
+#ifdef CONFIG_ARM_64
+#define ventry .quad
+#else
+#define ventry .word
+#endif
+
+ENTRY(hypercall_table)
+        /* Hypercalls 0-5 are not implemented on ARM */
+        .rept 6
+        ventry do_ni_hypercall
+        .endr
+        ventry do_deprecated_hypercall
+        ventry do_platform_op
+        ventry do_ni_hypercall
+        ventry do_ni_hypercall
+        ventry do_ni_hypercall       /* 10 */
+        ventry do_ni_hypercall
+        ventry do_memory_op
+        ventry do_multicall
+        ventry do_ni_hypercall
+        ventry do_ni_hypercall       /* 15 */
+        ventry do_deprecated_hypercall
+        ventry do_xen_version
+        ventry do_console_io
+        ventry do_deprecated_hypercall
+        ventry do_grant_table_op     /* 20 */
+        ventry do_ni_hypercall
+        ventry do_ni_hypercall
+        ventry do_ni_hypercall
+        ventry do_vcpu_op
+        ventry do_ni_hypercall       /* 25 */
+        ventry do_ni_hypercall
+        ventry do_xsm_op
+        ventry do_ni_hypercall
+        ventry do_sched_op
+        ventry do_ni_hypercall        /* 30 */
+        ventry do_ni_hypercall
+        ventry do_event_channel_op
+        ventry do_physdev_op
+        ventry do_hvm_op
+        ventry do_sysctl             /* 35 */
+        ventry do_domctl
+        .rept NR_hypercalls-((.-hypercall_table)/BYTES_PER_LONG)
+        ventry do_ni_hypercall
+        .endr
+
+ENTRY(hypercall_args_table)
+        /* Hypercalls 0-5 are not implemented on ARM */
+        .rept 6
+        .byte 0 /* do_ni_hypercall          */
+        .endr
+        .byte 2 /* do_deprecated_hypercall  */
+        .byte 1 /* do_platform_op           */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 0 /* do_ni_hypercall          */ /* 10 */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 2 /* do_memory_op             */
+        .byte 2 /* do_multicall             */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 1 /* do_deprecated_hypercall  */
+        .byte 2 /* do_xen_version           */
+        .byte 3 /* do_console_io            */
+        .byte 1 /* do_deprecated_hypercall  */
+        .byte 3 /* do_grant_table_op        */  /* 20 */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 3 /* do_vcpu_op               */
+        .byte 0 /* do_ni_hypercall          */ /* 25 */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 1 /* do_xsm_op                */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 2 /* do_sched_op              */
+        .byte 0 /* do_ni_hypercall          */ /* 30 */
+        .byte 0 /* do_ni_hypercall          */
+        .byte 2 /* do_event_channel_op      */
+        .byte 2 /* do_physdev_op            */
+        .byte 2 /* do_hvm_op                */
+        .byte 1 /* do_sysctl                */  /* 35 */
+        .byte 1 /* do_domctl                */
+        .rept NR_hypercalls-(.-hypercall_args_table)
+        .byte 0 /* do_ni_hypercall      */
+        .endr
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f222d96..cc67d23 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1171,7 +1171,7 @@ die:
 }
 #endif
 
-static register_t do_deprecated_hypercall(void)
+register_t do_deprecated_hypercall(void)
 {
     struct cpu_user_regs *regs = guest_cpu_user_regs();
     const register_t op =
@@ -1187,56 +1187,6 @@ static register_t do_deprecated_hypercall(void)
     return -ENOSYS;
 }
 
-typedef register_t (*arm_hypercall_fn_t)(
-    register_t, register_t, register_t, register_t, register_t);
-
-typedef struct {
-    arm_hypercall_fn_t fn;
-    int nr_args;
-} arm_hypercall_t;
-
-#define HYPERCALL(_name, _nr_args)                                   \
-    [ __HYPERVISOR_ ## _name ] =  {                                  \
-        .fn = (arm_hypercall_fn_t) &do_ ## _name,                    \
-        .nr_args = _nr_args,                                         \
-    }
-
-#define HYPERCALL_ARM(_name, _nr_args)                        \
-    [ __HYPERVISOR_ ## _name ] =  {                                  \
-        .fn = (arm_hypercall_fn_t) &do_arm_ ## _name,                \
-        .nr_args = _nr_args,                                         \
-    }
-/*
- * Only use this for hypercalls which were deprecated (i.e. replaced
- * by something else) before Xen on ARM was created, i.e. *not* for
- * hypercalls which are simply not yet used on ARM.
- */
-#define HYPERCALL_DEPRECATED(_name, _nr_args)                   \
-    [ __HYPERVISOR_##_name ] = {                                \
-        .fn = (arm_hypercall_fn_t) &do_deprecated_hypercall,    \
-        .nr_args = _nr_args,                                    \
-    }
-
-static arm_hypercall_t arm_hypercall_table[] = {
-    HYPERCALL(memory_op, 2),
-    HYPERCALL(domctl, 1),
-    HYPERCALL(sched_op, 2),
-    HYPERCALL_DEPRECATED(sched_op_compat, 2),
-    HYPERCALL(console_io, 3),
-    HYPERCALL(xen_version, 2),
-    HYPERCALL(xsm_op, 1),
-    HYPERCALL(event_channel_op, 2),
-    HYPERCALL_DEPRECATED(event_channel_op_compat, 1),
-    HYPERCALL(physdev_op, 2),
-    HYPERCALL_DEPRECATED(physdev_op_compat, 1),
-    HYPERCALL(sysctl, 2),
-    HYPERCALL(hvm_op, 2),
-    HYPERCALL(grant_table_op, 3),
-    HYPERCALL(multicall, 2),
-    HYPERCALL(platform_op, 1),
-    HYPERCALL_ARM(vcpu_op, 3),
-};
-
 #ifndef NDEBUG
 static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
 {
@@ -1406,17 +1356,14 @@ static void do_trap_psci(struct cpu_user_regs *regs)
 static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
                               unsigned long iss)
 {
-    arm_hypercall_fn_t call = NULL;
 #ifndef NDEBUG
     register_t orig_pc = regs->pc;
 #endif
 
-    BUILD_BUG_ON(NR_hypercalls < ARRAY_SIZE(arm_hypercall_table) );
-
     if ( iss != XEN_HYPERCALL_TAG )
         domain_crash_synchronous();
 
-    if ( *nr >= ARRAY_SIZE(arm_hypercall_table) )
+    if ( *nr >= NR_hypercalls )
     {
         perfc_incr(invalid_hypercalls);
         HYPERCALL_RESULT_REG(regs) = -ENOSYS;
@@ -1424,14 +1371,9 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
     }
 
     perfc_incra(hypercalls, *nr);
-    call = arm_hypercall_table[*nr].fn;
-    if ( call == NULL )
-    {
-        HYPERCALL_RESULT_REG(regs) = -ENOSYS;
-        return;
-    }
 
-    HYPERCALL_RESULT_REG(regs) = call(HYPERCALL_ARGS(regs));
+    HYPERCALL_RESULT_REG(regs) = do_dispatch_hypercall(HYPERCALL_ARGS(regs),
+                                                       *nr);
 
 #ifndef NDEBUG
     /*
@@ -1440,13 +1382,16 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
      */
     if ( orig_pc == regs->pc )
     {
-        switch ( arm_hypercall_table[*nr].nr_args ) {
+        switch ( hypercall_args_table[*nr] ) {
         case 5: HYPERCALL_ARG5(regs) = 0xDEADBEEF;
         case 4: HYPERCALL_ARG4(regs) = 0xDEADBEEF;
         case 3: HYPERCALL_ARG3(regs) = 0xDEADBEEF;
         case 2: HYPERCALL_ARG2(regs) = 0xDEADBEEF;
         case 1: /* Don't clobber x0/r0 -- it's the return value */
             break;
+        case 0:
+            ASSERT(hypercall_table[*nr] == do_ni_hypercall);
+            break;
         default: BUG();
         }
         *nr = 0xDEADBEEF;
@@ -1458,7 +1403,7 @@ static bool_t check_multicall_32bit_clean(struct multicall_entry *multi)
 {
     int i;
 
-    for ( i = 0; i < arm_hypercall_table[multi->op].nr_args; i++ )
+    for ( i = 0; i < hypercall_args_table[multi->op]; i++ )
     {
         if ( unlikely(multi->args[i] & 0xffffffff00000000ULL) )
         {
@@ -1474,16 +1419,7 @@ static bool_t check_multicall_32bit_clean(struct multicall_entry *multi)
 
 void do_multicall_call(struct multicall_entry *multi)
 {
-    arm_hypercall_fn_t call = NULL;
-
-    if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
-    {
-        multi->result = -ENOSYS;
-        return;
-    }
-
-    call = arm_hypercall_table[multi->op].fn;
-    if ( call == NULL )
+    if ( multi->op >= NR_hypercalls )
     {
         multi->result = -ENOSYS;
         return;
@@ -1493,9 +1429,9 @@ void do_multicall_call(struct multicall_entry *multi)
          !check_multicall_32bit_clean(multi) )
         return;
 
-    multi->result = call(multi->args[0], multi->args[1],
-                         multi->args[2], multi->args[3],
-                         multi->args[4]);
+    multi->result = do_dispatch_hypercall(multi->args[0], multi->args[1],
+                                          multi->args[2], multi->args[3],
+                                          multi->args[4], multi->op);
 }
 
 /*
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index a0c5a31..41e43ec 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -9,6 +9,15 @@ long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) a
 long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
                        XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
 
+register_t do_deprecated_hypercall(void);
+
+register_t do_dispatch_hypercall(register_t arg0, register_t arg1,
+                                 register_t arg2, register_t arg3,
+                                 register_t arg4, register_t nr);
+
+extern const uint8_t hypercall_args_table[];
+extern const void *hypercall_table[];
+
 #endif /* __ASM_ARM_HYPERCALL_H__ */
 /*
  * Local variables:
-- 
2.1.4

[RFC 08/10] xen/arm64: Implement the hypercall handing fully in assembly

[Julien Grall]

Currently when the guest is issuing an hypercall, all the registers are
saved in the stack and then later reload from the stack before calling
the hypercall dispatcher.

To avoid spending time loading from the stack the arguments, the
hypercall handler can be implemented in assembly.

This patch implements the assembly version of do_trap_hypercall for
AArch64.

Note that the C version of do_trap_hypercall will be dropped in a
follow-up patch.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/arm64/asm-offsets.c |  11 +++
 xen/arch/arm/arm64/entry.S       | 147 ++++++++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/processor.h  |   3 +
 3 files changed, 159 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index a3ce816..6f0e8f1 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -23,6 +23,12 @@
 void __dummy__(void)
 {
    OFFSET(UREGS_X0, struct cpu_user_regs, x0);
+   OFFSET(UREGS_X1, struct cpu_user_regs, x1);
+   OFFSET(UREGS_X2, struct cpu_user_regs, x2);
+   OFFSET(UREGS_X3, struct cpu_user_regs, x3);
+   OFFSET(UREGS_X4, struct cpu_user_regs, x4);
+   OFFSET(UREGS_X12, struct cpu_user_regs, x12);
+   OFFSET(UREGS_X16, struct cpu_user_regs, x16);
    OFFSET(UREGS_LR, struct cpu_user_regs, lr);
 
    OFFSET(UREGS_SP, struct cpu_user_regs, sp);
@@ -50,6 +56,11 @@ void __dummy__(void)
 
    BLANK();
    OFFSET(INITINFO_stack, struct init_info, stack);
+
+#ifdef PERF_COUNTERS
+   BLANK();
+   DEFINE(ASM_PERFC_hypercalls, PERFC_hypercalls);
+#endif
 }
 
 /*
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 93c80ff..5092533 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -1,4 +1,5 @@
 #include <xen/config.h>
+#include <xen/errno.h>
 #include <asm/asm_defns.h>
 #include <asm/bug.h>
 #include <asm/regs.h>
@@ -174,8 +175,9 @@ hyp_irq:
 guest_sync:
         entry   hyp=0, compat=0
         msr     daifclr, #2
+        mov     x7, x0
         mov     x0, sp
-        bl      do_trap_hypervisor
+        bl      do_trap_guest_sync
         exit    hyp=0, compat=0
 
 guest_irq:
@@ -195,8 +197,9 @@ guest_error_invalid:
 guest_sync_compat:
         entry   hyp=0, compat=1
         msr     daifclr, #2
+        mov     x7, x0
         mov     x0, sp
-        bl      do_trap_hypervisor
+        bl      do_trap_guest_sync
         exit    hyp=0, compat=1
 
 guest_irq_compat:
@@ -277,6 +280,146 @@ ENTRY(hyp_traps_vector)
         ventry  guest_fiq_invalid_compat        // FIQ 32-bit EL0/EL1
         ventry  guest_error_invalid_compat      // Error 32-bit EL0/EL1
 
+
+/*
+ * x0/sp - cpu_user_regs
+ * x7 - x0 of the guest (possible arg1 of an hypercall)
+ * x12 - hypercall number for HVC32
+ * x16 - hypercall number for HVC64
+ *
+ * Callee-saved register used within the function for specific purpose:
+ *
+ * x19 - save x30 (lr)
+ * x20 - save the guest pc (only for debug build)
+ * x21 - save the hypercall number
+ * x22 - ESR_EL2.EC
+ *
+ * Note that x1 - x4 shouldn't be clobbered as they contain the
+ * arguments of the hypercall if the guest issued an hypercall.
+ */
+do_trap_guest_sync:
+        mrs     x5, ESR_EL2
+        lsr     x22, x5, #HSR_EC_SHIFT      /* x22 <- ESR.EL2.EC */
+        /* The trap may be an hypercall if EC == HVC32 || HVC64 */
+        cmp     x22, #HSR_EC_HVC32
+        beq     do_trap_hvc32               /* EC == HVC32 -> HVC call */
+        cmp     x22, #HSR_EC_HVC64
+        beq     do_trap_hvc                 /* EC == HVC64 -> HVC call */
+
+        b do_trap_hypervisor                /* Not an HVC call -> generic dispatch */
+
+do_trap_hvc32:
+        /*
+         * For HVC32, the hypercall number is in x12, load it in x16
+         * to have the HVC dispatcher common.
+         */
+        mov     x16, x12
+
+do_trap_hvc:
+        /*
+         * Only HVC call with ISS == XEN_HYPERCALL_TAG is handled in
+         * assembly, the rest is handled in C by the generic dispatch.
+         */
+        and     x6, x5, #HSR_ISS_MASK       /* x6 <- ESR_EL2.ISS */
+        cmp     x6, #XEN_HYPERCALL_TAG
+        bne     do_trap_hypervisor          /* Not an hypercall -> generic dispatch */
+
+do_trap_hypercall:
+        /* The guest issued an hypercall */
+
+        /* Check if the hypercall number is valid */
+        cmp     x16, #(NR_hypercalls-1)
+        bhi     invalid_hypercall           /* Hypercall number invalid -> bail out */
+
+        PERFC_INCR(hypercalls, x16)
+
+#ifndef NDEBUG
+        /* Save guest PC to detect hypercall continuation */
+        ldr     x20, [sp, #UREGS_PC]        /* x20 <- regs->pc */
+#endif
+
+        /*
+         * Call the hypercall dispatcher
+         * The arguments arg2 - arg5 are already in the correct
+         * registers (i.e x1 - x4).
+         */
+        mov     x19, lr                     /* save LR in x19 */
+        mov     x21, x16                    /* save the hypercall number in x21 */
+        mov     x0, x7                      /* x0 <- arg1 */
+        mov     x5, x16                     /* x5 <- Hypercall number */
+        bl      do_dispatch_hypercall
+        mov     lr, x19                     /* Restore LR from x19 */
+
+        str     x0, [sp, #UREGS_X0]         /* regs->x0 <- do_dispatch_hypercall(...) */
+
+#ifndef NDEBUG
+        /*
+         * Clobber arguments register only if pc is unchanged, otherwise
+         * this is a hypercall continuation.
+         */
+        ldr     x0, [sp, #UREGS_PC]         /* x0 <- regs->pc */
+        cmp     x0, x20                     /* Compare regs->pc (x0) with orig_pc (x20) */
+        bne     clobber_done                /* different => no need to clobber */
+
+        ldr     x6, =0xdeadbeefdeadbeef     /* x6 <- poison for registers */
+
+        ldr     x5, =hypercall_args_table
+        ldr     x5, [x5, x21]               /* x5 <- number of args */
+        cmp     x5, #5; b   .Largs5
+        cmp     x5, #4; b   .Largs4
+        cmp     x5, #3; b   .Largs3
+        cmp     x5, #2; b   .Largs2
+        cmp     x5, #1; b   .Largs1
+        cmp     x5, #0; b   .Largs0         /* Only for hypercall not implemented */
+        BUG                                 /* Number of arguments invalid */
+.Largs5:
+        ldr     x6, [sp, #UREGS_X4]
+.Largs4:
+        ldr     x6, [sp, #UREGS_X3]
+.Largs3:
+        ldr     x6, [sp, #UREGS_X2]
+.Largs2:
+        ldr     x6, [sp, #UREGS_X1]
+.Largs1:
+        /* Don't clobber x0 -- it's the return value */
+        b       .Lclobber_nr
+.Largs0:
+        /*
+         * Can only happen when the hypercall is not implemented.
+         * i.e hypercall_table[*nr] == do_ni_hypercall
+         */
+        ldr     x6, =hypercall_table
+        ldr     x6, [x6, x21, lsl #3]       /* x10 := pointer to hypercall function */
+        ldr     x5, do_ni_hypercall
+        cmp     x5, x6
+        beq     .Lclobber_nr                /* Hypercall function == do_ni_hypercall => Good */
+        ASSERT_FAILED("hypercall_table[*nr] == do_ni_hypercall")
+
+.Lclobber_nr:
+        /*
+         * Clobber the hypercall number register. The register depends
+         * on HSR.EL2 (stored in x22):
+         *      HSR_EC_HVC32 -> x12
+         *      HSR_EC_HVC64 -> x16
+         */
+        cmp     x22, #HSR_EC_HVC32
+        beq     .Lclobber_hvc32
+.Lclobber_hvc64:
+        str     x6, [sp, #UREGS_X16]
+        b       clobber_done
+.Lclobber_hvc32:
+        str     x6, [sp, #UREGS_X12]
+
+clobber_done:
+#endif
+
+        ret
+
+invalid_hypercall:
+        mov     x0, #-ENOSYS
+        str     x0, [sp, #UREGS_X0]         /* regs->x0 <- -ENOSYS */
+        ret
+
 /*
  * struct vcpu *__context_switch(struct vcpu *prev, struct vcpu *next)
  *
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 7e6eb66..da02599 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -483,6 +483,9 @@ union hsr {
 };
 #endif
 
+#define HSR_EC_SHIFT    26
+#define HSR_ISS_MASK    0x00ffffff
+
 /* HSR.EC == HSR_CP{15,14,10}_32 */
 #define HSR_CP32_OP2_MASK (0x000e0000)
 #define HSR_CP32_OP2_SHIFT (17)
-- 
2.1.4

[RFC 09/10] xen/arm: Remove the C version of do_trap_hypercall

[Julien Grall]

This has been fully implemented in assembly so it's not necessary to
keep it anymore.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---

    It's possible to factorize the code to handle HVC32/HVC64, but this will
    be done in the next patch.
---
 xen/arch/arm/traps.c | 74 ++++++----------------------------------------------
 1 file changed, 8 insertions(+), 66 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index cc67d23..3cd8992 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1335,70 +1335,6 @@ static void do_trap_psci(struct cpu_user_regs *regs)
     }
 }
 
-#ifdef CONFIG_ARM_64
-#define HYPERCALL_RESULT_REG(r) (r)->x0
-#define HYPERCALL_ARG1(r) (r)->x0
-#define HYPERCALL_ARG2(r) (r)->x1
-#define HYPERCALL_ARG3(r) (r)->x2
-#define HYPERCALL_ARG4(r) (r)->x3
-#define HYPERCALL_ARG5(r) (r)->x4
-#define HYPERCALL_ARGS(r) (r)->x0, (r)->x1, (r)->x2, (r)->x3, (r)->x4
-#else
-#define HYPERCALL_RESULT_REG(r) (r)->r0
-#define HYPERCALL_ARG1(r) (r)->r0
-#define HYPERCALL_ARG2(r) (r)->r1
-#define HYPERCALL_ARG3(r) (r)->r2
-#define HYPERCALL_ARG4(r) (r)->r3
-#define HYPERCALL_ARG5(r) (r)->r4
-#define HYPERCALL_ARGS(r) (r)->r0, (r)->r1, (r)->r2, (r)->r3, (r)->r4
-#endif
-
-static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
-                              unsigned long iss)
-{
-#ifndef NDEBUG
-    register_t orig_pc = regs->pc;
-#endif
-
-    if ( iss != XEN_HYPERCALL_TAG )
-        domain_crash_synchronous();
-
-    if ( *nr >= NR_hypercalls )
-    {
-        perfc_incr(invalid_hypercalls);
-        HYPERCALL_RESULT_REG(regs) = -ENOSYS;
-        return;
-    }
-
-    perfc_incra(hypercalls, *nr);
-
-    HYPERCALL_RESULT_REG(regs) = do_dispatch_hypercall(HYPERCALL_ARGS(regs),
-                                                       *nr);
-
-#ifndef NDEBUG
-    /*
-     * Clobber argument registers only if pc is unchanged, otherwise
-     * this is a hypercall continuation.
-     */
-    if ( orig_pc == regs->pc )
-    {
-        switch ( hypercall_args_table[*nr] ) {
-        case 5: HYPERCALL_ARG5(regs) = 0xDEADBEEF;
-        case 4: HYPERCALL_ARG4(regs) = 0xDEADBEEF;
-        case 3: HYPERCALL_ARG3(regs) = 0xDEADBEEF;
-        case 2: HYPERCALL_ARG2(regs) = 0xDEADBEEF;
-        case 1: /* Don't clobber x0/r0 -- it's the return value */
-            break;
-        case 0:
-            ASSERT(hypercall_table[*nr] == do_ni_hypercall);
-            break;
-        default: BUG();
-        }
-        *nr = 0xDEADBEEF;
-    }
-#endif
-}
-
 static bool_t check_multicall_32bit_clean(struct multicall_entry *multi)
 {
     int i;
@@ -2476,7 +2412,10 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 #endif
         if ( hsr.iss == 0 )
             return do_trap_psci(regs);
-        do_trap_hypercall(regs, (register_t *)&regs->r12, hsr.iss);
+
+        /* The ISS is not supported */
+        domain_crash_synchronous();
+
         break;
 #ifdef CONFIG_ARM_64
     case HSR_EC_HVC64:
@@ -2488,7 +2427,10 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 #endif
         if ( hsr.iss == 0 )
             return do_trap_psci(regs);
-        do_trap_hypercall(regs, &regs->x16, hsr.iss);
+
+        /* The ISS is not supported */
+        domain_crash_synchronous();
+
         break;
     case HSR_EC_SMC64:
         /*
-- 
2.1.4

[RFC 10/10] xen/arm: Factorize the C code to dispatch HVC

[Julien Grall]

Now that Xen hypercall is fully dispatched in assembly, the code to
handle HVC32/HVC64 is mostly the same.

Create an helper to avoid duplicating the code.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/traps.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3cd8992..a49530a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1335,6 +1335,20 @@ static void do_trap_psci(struct cpu_user_regs *regs)
     }
 }
 
+static void do_trap_hvc(struct cpu_user_regs *regs, uint32_t iss)
+{
+
+#ifndef NDEBUG
+    if ( (iss & 0xff00) == 0xff00 )
+        return do_debug_trap(regs, iss & 0x00ff);
+#endif
+    if ( iss == 0 )
+        return do_trap_psci(regs);
+
+    /* The ISS is not supported */
+    domain_crash_synchronous();
+}
+
 static bool_t check_multicall_32bit_clean(struct multicall_entry *multi)
 {
     int i;
@@ -2406,31 +2420,13 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
     case HSR_EC_HVC32:
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_hvc32);
-#ifndef NDEBUG
-        if ( (hsr.iss & 0xff00) == 0xff00 )
-            return do_debug_trap(regs, hsr.iss & 0x00ff);
-#endif
-        if ( hsr.iss == 0 )
-            return do_trap_psci(regs);
-
-        /* The ISS is not supported */
-        domain_crash_synchronous();
-
+        do_trap_hvc(regs, hsr.iss);
         break;
 #ifdef CONFIG_ARM_64
     case HSR_EC_HVC64:
         GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_hvc64);
-#ifndef NDEBUG
-        if ( (hsr.iss & 0xff00) == 0xff00 )
-            return do_debug_trap(regs, hsr.iss & 0x00ff);
-#endif
-        if ( hsr.iss == 0 )
-            return do_trap_psci(regs);
-
-        /* The ISS is not supported */
-        domain_crash_synchronous();
-
+        do_trap_hvc(regs, hsr.iss);
         break;
     case HSR_EC_SMC64:
         /*
-- 
2.1.4

Re: [RFC 00/10] xen/arm: Implement the hypercall handling in assembly

[Andrew Cooper]

On 15/12/15 17:51, Julien Grall wrote:
> Hi all,
>
> This patch aims is a first attempt to write the hypercall dispatch in assembly
> in order to avoid reloading the hypercall arguments from the stack.

Why?  A couple of stack operations are in the noise, performance wise.

I have half a patch series to do exactly the opposite for x86 PV guests,
as C is far more maintainable than asm.

~Andrew

Re: [RFC 00/10] xen/arm: Implement the hypercall handling in assembly

[Ian Campbell]

On Tue, 2015-12-15 at 18:33 +0000, Andrew Cooper wrote:
> On 15/12/15 17:51, Julien Grall wrote:
> > Hi all,
> > 
> > This patch aims is a first attempt to write the hypercall dispatch in
> > assembly
> > in order to avoid reloading the hypercall arguments from the stack.
> 
> Why?  A couple of stack operations are in the noise, performance wise.

At least on ARM It's quite a bit more than a couple (dozens but not
hundreds I would say, and I've not looked at the patch yet to see how much
it saves out of those), but clearly it would need to be measured at some
point before taking it too much further than an RFC.

There's a second issue which is that the current C dispatch table approach
which uses a common typedef is not strictly speaking valid C, since it is
undefined behaviour to call a function through a typedef which has a
different prototype (Ian J can probably explain it better than I, with
chapter and verse etc).

Ian.

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