On Tue, Jun 07, 2016 at 11:58:29AM +0100, Marc Zyngier wrote:
> Now that we only have the "merged page tables" case to deal with,
> there is a bunch of things we can simplify in the HYP code (both
> at init and teardown time).
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 12 ++------
> arch/arm64/kvm/hyp-init.S | 61 +++++----------------------------------
> arch/arm64/kvm/hyp/entry.S | 19 ------------
> arch/arm64/kvm/hyp/hyp-entry.S | 15 ++++++++++
> arch/arm64/kvm/reset.c | 11 -------
> 5 files changed, 26 insertions(+), 92 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 49095fc..88462c3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -48,7 +48,6 @@
> int __attribute_const__ kvm_target_cpu(void);
> int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
> int kvm_arch_dev_ioctl_check_extension(long ext);
> -unsigned long kvm_hyp_reset_entry(void);
> void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t idmap_start);
>
> struct kvm_arch {
> @@ -357,19 +356,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t boot_pgd_ptr,
> * Call initialization code, and switch to the full blown
> * HYP code.
> */
> - __kvm_call_hyp((void *)boot_pgd_ptr, pgd_ptr,
> - hyp_stack_ptr, vector_ptr);
> + __kvm_call_hyp((void *)pgd_ptr, hyp_stack_ptr, vector_ptr);
> }
>
> +void __kvm_hyp_teardown(void);
> static inline void __cpu_reset_hyp_mode(phys_addr_t boot_pgd_ptr,
> phys_addr_t phys_idmap_start)
> {
> - /*
> - * Call reset code, and switch back to stub hyp vectors.
> - * Uses __kvm_call_hyp() to avoid kaslr's kvm_ksym_ref() translation.
> - */
> - __kvm_call_hyp((void *)kvm_hyp_reset_entry(),
> - boot_pgd_ptr, phys_idmap_start);
> + kvm_call_hyp(__kvm_hyp_teardown, phys_idmap_start);
> }
>
> static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
> index a873a6d..6b29d3d 100644
> --- a/arch/arm64/kvm/hyp-init.S
> +++ b/arch/arm64/kvm/hyp-init.S
> @@ -53,10 +53,9 @@ __invalid:
> b .
>
> /*
> - * x0: HYP boot pgd
> - * x1: HYP pgd
> - * x2: HYP stack
> - * x3: HYP vectors
> + * x0: HYP pgd
> + * x1: HYP stack
> + * x2: HYP vectors
> */
> __do_hyp_init:
>
> @@ -110,71 +109,27 @@ __do_hyp_init:
> msr sctlr_el2, x4
> isb
>
> - /* Skip the trampoline dance if we merged the boot and runtime PGDs */
> - cmp x0, x1
> - b.eq merged
> -
> - /* MMU is now enabled. Get ready for the trampoline dance */
> - ldr x4, =TRAMPOLINE_VA
> - adr x5, target
> - bfi x4, x5, #0, #PAGE_SHIFT
> - br x4
> -
> -target: /* We're now in the trampoline code, switch page tables */
> - msr ttbr0_el2, x1
> - isb
> -
> - /* Invalidate the old TLBs */
> - tlbi alle2
> - dsb sy
> -
> -merged:
> /* Set the stack and new vectors */
> + kern_hyp_va x1
> + mov sp, x1
> kern_hyp_va x2
> - mov sp, x2
> - kern_hyp_va x3
> - msr vbar_el2, x3
> + msr vbar_el2, x2
>
> /* Hello, World! */
> eret
> ENDPROC(__kvm_hyp_init)
>
> /*
> - * Reset kvm back to the hyp stub. This is the trampoline dance in
> - * reverse. If kvm used an extended idmap, __extended_idmap_trampoline
> - * calls this code directly in the idmap. In this case switching to the
> - * boot tables is a no-op.
> - *
> - * x0: HYP boot pgd
> - * x1: HYP phys_idmap_start
> + * Reset kvm back to the hyp stub.
> */
> ENTRY(__kvm_hyp_reset)
> - /* We're in trampoline code in VA, switch back to boot page tables */
> - msr ttbr0_el2, x0
> - isb
> -
> - /* Ensure the PA branch doesn't find a stale tlb entry or stale code. */
> - ic iallu
> - tlbi alle2
> - dsb sy
> - isb
> -
> - /* Branch into PA space */
> - adr x0, 1f
> - bfi x1, x0, #0, #PAGE_SHIFT
> - br x1
> -
> /* We're now in idmap, disable MMU */
> -1: mrs x0, sctlr_el2
> + mrs x0, sctlr_el2
> ldr x1, =SCTLR_ELx_FLAGS
> bic x0, x0, x1 // Clear SCTL_M and etc
> msr sctlr_el2, x0
> isb
>
> - /* Invalidate the old TLBs */
> - tlbi alle2
> - dsb sy
> -
why can we get rid of the above two lines now?
> /* Install stub vectors */
> adr_l x0, __hyp_stub_vectors
> msr vbar_el2, x0
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 70254a6..ce9e5e5 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -164,22 +164,3 @@ alternative_endif
>
> eret
> ENDPROC(__fpsimd_guest_restore)
> -
> -/*
> - * When using the extended idmap, we don't have a trampoline page we can use
> - * while we switch pages tables during __kvm_hyp_reset. Accessing the idmap
> - * directly would be ideal, but if we're using the extended idmap then the
> - * idmap is located above HYP_PAGE_OFFSET, and the address will be masked by
> - * kvm_call_hyp using kern_hyp_va.
> - *
> - * x0: HYP boot pgd
> - * x1: HYP phys_idmap_start
> - */
> -ENTRY(__extended_idmap_trampoline)
> - mov x4, x1
> - adr_l x3, __kvm_hyp_reset
> -
> - /* insert __kvm_hyp_reset()s offset into phys_idmap_start */
> - bfi x4, x3, #0, #PAGE_SHIFT
> - br x4
> -ENDPROC(__extended_idmap_trampoline)
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> index 2d87f36..f6d9694 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -62,6 +62,21 @@ ENTRY(__vhe_hyp_call)
> isb
> ret
> ENDPROC(__vhe_hyp_call)
> +
> +/*
> + * Compute the idmap address of __kvm_hyp_reset based on the idmap
> + * start passed as a parameter, and jump there.
> + *
> + * x0: HYP phys_idmap_start
> + */
> +ENTRY(__kvm_hyp_teardown)
> + mov x4, x0
> + adr_l x3, __kvm_hyp_reset
> +
> + /* insert __kvm_hyp_reset()s offset into phys_idmap_start */
> + bfi x4, x3, #0, #PAGE_SHIFT
> + br x4
> +ENDPROC(__kvm_hyp_teardown)
>
> el1_sync: // Guest trapped into EL2
> save_x0_to_x3
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index d044ca3..deee1b1 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -132,14 +132,3 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> /* Reset timer */
> return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> }
> -
> -unsigned long kvm_hyp_reset_entry(void)
> -{
> - /*
> - * KVM is running with merged page tables, which don't have the
> - * trampoline page mapped. We know the idmap is still mapped,
> - * but can't be called into directly. Use
> - * __extended_idmap_trampoline to do the call.
> - */
> - return (unsigned long)kvm_ksym_ref(__extended_idmap_trampoline);
> -}
> --
> 2.1.4
>
I'm not sure I understand why we needed the kvm_hyp_reset_entry
indirection before, but the resulting code here looks good to me.
Thanks,
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
