Re: [RFC PATCH 09/17] arm64: remove arch_handle_exit

[Jan Kiszka]

On 06.11.18 00:05, Ralf Ramsauer wrote:

The final step: remove arch_handle_exit, and dispatch things directly in
assembly. As a consequence, we get a faster exit path, as we save the
double-dispatching.

We also save one instruction inside the interrupt vector. :-)

For the union registers, replace the exit_reason with __padding. For easier
handling, the padding is located at the beginning of the structure.

What do we need that padding still for?

Signed-off-by: Ralf Ramsauer <ralf.ramsa...@oth-regensburg.de>
---
  hypervisor/arch/arm64/entry.S                 | 18 +++++--------
  .../arch/arm64/include/arch/asm/traps.h       |  3 +++
  hypervisor/arch/arm64/include/asm/processor.h |  6 +----
  hypervisor/arch/arm64/traps.c                 | 26 ++-----------------
  4 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/hypervisor/arch/arm64/entry.S b/hypervisor/arch/arm64/entry.S
index e0240f00..498a8d64 100644
--- a/hypervisor/arch/arm64/entry.S
+++ b/hypervisor/arch/arm64/entry.S
@@ -271,19 +271,15 @@ bootstrap_vectors:
        ventry  .

-.macro handle_vmexit exit_reason

+.macro handle_vmexit handler
        .align  7
        /* We need to save EL1 context, reserve some space on the stack */
        sub     sp, sp, #(16 * 16)
        /* And push [x1-x4] early, we need registers to work on */
+       str         x0, [sp, #(1 * 8)]
        stp     x1, x2, [sp, #(1 * 16)]
        stp     x3, x4, [sp, #(2 * 16)]

- /* x1 is available, use it to hold the exit reason */

-       mov     x1, #\exit_reason
-       /* ... and push it together with x0 */
-       stp     x1, x0, [sp, #(0 * 16)]
-
        /* [x0-x4] may now be clobbered. */

/*

@@ -314,7 +310,7 @@ bootstrap_vectors:
        mov     x29, xzr        /* reset fp,lr */
        mov     x30, xzr
        mov     x0, sp
-       bl      arch_handle_exit
+       bl      \handler
        /* take the fast exit path, sp is already in place */
        b       __vmreturn
  .endm
@@ -326,13 +322,13 @@ hyp_vectors:
        ventry  .
        ventry  .

- handle_vmexit EXIT_REASON_EL2_ABORT

+       handle_vmexit arch_el2_abt
        ventry  .
        ventry  .
        ventry  .

- handle_vmexit EXIT_REASON_EL1_ABORT

-       handle_vmexit EXIT_REASON_EL1_IRQ
+       handle_vmexit arch_handle_trap
+       handle_vmexit irqchip_handle_irq
        ventry  .
        ventry  .

@@ -429,8 +425,8 @@ __vmreturn:

        ldp     x7, x8, [sp, #(4 * 16)]
        ldp     x5, x6, [sp, #(3 * 16)]
        ldp     x3, x4, [sp, #(2 * 16)]
-       ldp     x1, x0, [sp, #(0 * 16)] /* x1 is the exit_reason */
        ldp     x1, x2, [sp, #(1 * 16)]
+       ldr         x0, [sp, #(1 * 8)]
        add     sp, sp, #(16 * 16)
        eret
        .popsection
diff --git a/hypervisor/arch/arm64/include/arch/asm/traps.h 
b/hypervisor/arch/arm64/include/arch/asm/traps.h
index aec6de5a..e2164ad9 100644
--- a/hypervisor/arch/arm64/include/arch/asm/traps.h
+++ b/hypervisor/arch/arm64/include/arch/asm/traps.h
@@ -16,3 +16,6 @@ struct trap_context {
        u64 spsr;
        u64 sp;
  };
+
+void arch_handle_trap(union registers *guest_regs);
+void arch_el2_abt(union registers *regs);
diff --git a/hypervisor/arch/arm64/include/asm/processor.h 
b/hypervisor/arch/arm64/include/asm/processor.h
index 6396a356..5bedb20a 100644
--- a/hypervisor/arch/arm64/include/asm/processor.h
+++ b/hypervisor/arch/arm64/include/asm/processor.h
@@ -16,17 +16,13 @@
  #include <jailhouse/types.h>
  #include <jailhouse/utils.h>

-#define EXIT_REASON_EL2_ABORT 0x0

-#define EXIT_REASON_EL1_ABORT  0x1
-#define EXIT_REASON_EL1_IRQ    0x2
-
  #define NUM_USR_REGS          31

#ifndef __ASSEMBLY__ union registers {

        struct {
-               unsigned long exit_reason;
+               unsigned long __padding;
                unsigned long usr[NUM_USR_REGS];
        };
  };
diff --git a/hypervisor/arch/arm64/traps.c b/hypervisor/arch/arm64/traps.c
index 71f6da6f..0174edae 100644
--- a/hypervisor/arch/arm64/traps.c
+++ b/hypervisor/arch/arm64/traps.c
@@ -159,7 +159,7 @@ static const trap_handler trap_handlers[0x40] =
        [ESR_EC_DABT_LOW]       = arch_handle_dabt,
  };

-static void arch_handle_trap(union registers *guest_regs)

+void arch_handle_trap(union registers *guest_regs)
  {
        struct trap_context ctx;
        trap_handler handler;
@@ -182,7 +182,7 @@ static void arch_handle_trap(union registers *guest_regs)
        }
  }

-static void arch_el2_abt(union registers *regs)

+void arch_el2_abt(union registers *regs)
  {
        struct trap_context ctx;

@@ -193,25 +193,3 @@ static void arch_el2_abt(union registers *regs)

        dump_hyp_stack(&ctx);
        panic_stop();
  }
-
-union registers *arch_handle_exit(union registers *regs)
-{
-       switch (regs->exit_reason) {
-       case EXIT_REASON_EL1_IRQ:
-               irqchip_handle_irq();
-               break;
-
-       case EXIT_REASON_EL1_ABORT:
-               arch_handle_trap(regs);
-               break;
-
-       case EXIT_REASON_EL2_ABORT:
-               arch_el2_abt(regs);
-               break;
-
-       default:
-               panic_stop();
-       }
-
-       return regs;
-}

Nice refactoring!

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

--
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.