From: Marc Zyngier <maz@kernel.org>
To: Andrew Scull <ascull@google.com>
Cc: kernel-team@android.com, suzuki.poulose@arm.com,
catalin.marinas@arm.com, james.morse@arm.com,
linux-arm-kernel@lists.infradead.org,
Sudeep Holla <sudeep.holla@arm.com>,
will@kernel.org, kvmarm@lists.cs.columbia.edu,
julien.thierry.kdev@gmail.com
Subject: Re: [PATCH v3 12/18] KVM: arm64: nVHE: Switch to hyp context for EL2
Date: Mon, 07 Sep 2020 14:02:54 +0100 [thread overview]
Message-ID: <87v9gploxt.wl-maz@kernel.org> (raw)
In-Reply-To: <20200903135307.251331-13-ascull@google.com>
Hi Andrew,
On Thu, 03 Sep 2020 14:53:01 +0100,
Andrew Scull <ascull@google.com> wrote:
>
> Save and restore the host context when switching to and from hyp. This
> gives hyp its own context that the host will not see as a step towards a
> full trust boundary between the two.
>
> SP_EL0 and pointer authentication keys are currently shared between the
> host and hyp so don't need to be switched yet.
>
> Signed-off-by: Andrew Scull <ascull@google.com>
> ---
> arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +
> arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
> arch/arm64/kvm/hyp/nvhe/host.S | 68 ++++++++++++++++++-------
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 35 +++++++++++++
> 4 files changed, 88 insertions(+), 19 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/nvhe/hyp-main.c
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 821721b78ad9..4536b50ddc06 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -372,6 +372,8 @@ static inline bool esr_is_ptrauth_trap(u32 esr)
> ctxt_sys_reg(ctxt, key ## KEYHI_EL1) = __val; \
> } while(0)
>
> +DECLARE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
> +
> static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
> {
> struct kvm_cpu_context *ctxt;
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index ddf98eb07b9d..46c89e8c30bc 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -6,7 +6,7 @@
> asflags-y := -D__KVM_NVHE_HYPERVISOR__
> ccflags-y := -D__KVM_NVHE_HYPERVISOR__
>
> -obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o
> +obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o hyp-main.o
> obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> ../fpsimd.o ../hyp-entry.o
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> index d4e8b8084020..1062547853db 100644
> --- a/arch/arm64/kvm/hyp/nvhe/host.S
> +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> @@ -12,6 +12,55 @@
>
> .text
>
> +SYM_FUNC_START(__host_exit)
> + stp x0, x1, [sp, #-16]!
> +
> + get_host_ctxt x0, x1
> +
> + ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +
> + /* Store the guest regs x2 and x3 */
These comments are massively confusing. Please stick with the
conventional KVM terminology, where the host isn't a guest.
> + stp x2, x3, [x0, #CPU_XREG_OFFSET(2)]
> +
> + /* Retrieve the guest regs x0-x1 from the stack */
> + ldp x2, x3, [sp], #16 // x0, x1
> +
> + // Store the guest regs x0-x1 and x4-x17
> + stp x2, x3, [x0, #CPU_XREG_OFFSET(0)]
> + stp x4, x5, [x0, #CPU_XREG_OFFSET(4)]
> + stp x6, x7, [x0, #CPU_XREG_OFFSET(6)]
> + stp x8, x9, [x0, #CPU_XREG_OFFSET(8)]
> + stp x10, x11, [x0, #CPU_XREG_OFFSET(10)]
> + stp x12, x13, [x0, #CPU_XREG_OFFSET(12)]
> + stp x14, x15, [x0, #CPU_XREG_OFFSET(14)]
> + stp x16, x17, [x0, #CPU_XREG_OFFSET(16)]
> +
> + /* Store the guest regs x18-x29, lr */
> + save_callee_saved_regs x0
> +
> + /* Save the host context pointer in x29 across the function call */
> + mov x29, x0
> + bl handle_trap
> +
> + /* Restore guest regs x0-x17 */
> + ldp x0, x1, [x29, #CPU_XREG_OFFSET(0)]
> + ldp x2, x3, [x29, #CPU_XREG_OFFSET(2)]
> + ldp x4, x5, [x29, #CPU_XREG_OFFSET(4)]
> + ldp x6, x7, [x29, #CPU_XREG_OFFSET(6)]
> + ldp x8, x9, [x29, #CPU_XREG_OFFSET(8)]
> + ldp x10, x11, [x29, #CPU_XREG_OFFSET(10)]
> + ldp x12, x13, [x29, #CPU_XREG_OFFSET(12)]
> + ldp x14, x15, [x29, #CPU_XREG_OFFSET(14)]
> + ldp x16, x17, [x29, #CPU_XREG_OFFSET(16)]
> +
> + /* Restore guest regs x18-x29, lr */
> + restore_callee_saved_regs x29
This is a lot of save/restoring on each and every HVC. And I fear that
at some stage, you will want to restore some EL2-specific registers
too, adding even more to the overhead.
I'll have a go a measuring by how much we regress with this.
> +
> + /* Do not touch any register after this! */
> + eret
> + sb
> +SYM_FUNC_END(__host_exit)
> +
> SYM_FUNC_START(__hyp_do_panic)
> mov lr, #(PSR_F_BIT | PSR_I_BIT | PSR_A_BIT | PSR_D_BIT |\
> PSR_MODE_EL1h)
> @@ -35,7 +84,7 @@ SYM_FUNC_END(__hyp_do_panic)
>
> /* Check for a stub HVC call */
> cmp x0, #HVC_STUB_HCALL_NR
> - b.hs 1f
> + b.hs __host_exit
>
> /*
> * Compute the idmap address of __kvm_handle_stub_hvc and
> @@ -51,23 +100,6 @@ SYM_FUNC_END(__hyp_do_panic)
> /* x5 = __pa(x5) */
> sub x5, x5, x6
> br x5
> -
> -1:
> - /*
> - * Shuffle the parameters before calling the function
> - * pointed to in x0. Assumes parameters in x[1,2,3].
> - */
> - kern_hyp_va x0
> - str lr, [sp, #-16]!
> - mov lr, x0
> - mov x0, x1
> - mov x1, x2
> - mov x2, x3
> - blr lr
> - ldr lr, [sp], #16
> -
> - eret
> - sb
> .endm
>
> .macro invalid_host_vect
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> new file mode 100644
> index 000000000000..c8938e09f585
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 - Google Inc
> + * Author: Andrew Scull <ascull@google.com>
> + */
> +
> +#include <hyp/switch.h>
> +
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_emulate.h>
> +#include <asm/kvm_host.h>
> +#include <asm/kvm_hyp.h>
> +#include <asm/kvm_mmu.h>
> +
> +typedef unsigned long (*hypcall_fn_t)
> + (unsigned long, unsigned long, unsigned long);
> +
> +void handle_trap(struct kvm_cpu_context *host_ctxt) {
Coding style.
> + u64 esr = read_sysreg_el2(SYS_ESR);
> + hypcall_fn_t func;
> + unsigned long ret;
> +
> + if (ESR_ELx_EC(esr) != ESR_ELx_EC_HVC64)
> + hyp_panic();
> +
> + /*
> + * __kvm_call_hyp takes a pointer in the host address space and
> + * up to three arguments.
> + */
> + func = (hypcall_fn_t)kern_hyp_va(host_ctxt->regs.regs[0]);
> + ret = func(host_ctxt->regs.regs[1],
> + host_ctxt->regs.regs[2],
> + host_ctxt->regs.regs[3]);
> + host_ctxt->regs.regs[0] = ret;
> +}
> --
> 2.28.0.402.g5ffc5be6b7-goog
>
>
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-07 13:04 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 13:52 [PATCH v3 00/18] Introduce separate nVHE hyp context Andrew Scull
2020-09-03 13:52 ` [PATCH v3 01/18] KVM: arm64: Remove __activate_vm wrapper Andrew Scull
2020-09-03 13:52 ` [PATCH v3 02/18] KVM: arm64: Remove hyp_panic arguments Andrew Scull
2020-09-07 10:21 ` Marc Zyngier
2020-09-03 13:52 ` [PATCH v3 03/18] KVM: arm64: Remove kvm_host_data_t typedef Andrew Scull
2020-09-03 13:52 ` [PATCH v3 04/18] KVM: arm64: Restrict symbol aliasing to outside nVHE Andrew Scull
2020-09-07 10:38 ` Marc Zyngier
2020-09-08 10:13 ` Andrew Scull
2020-09-03 13:52 ` [PATCH v3 05/18] KVM: arm64: Save chosen hyp vector to a percpu variable Andrew Scull
2020-09-03 13:52 ` [PATCH v3 06/18] KVM: arm64: nVHE: Use separate vector for the host Andrew Scull
2020-09-07 11:38 ` Marc Zyngier
2020-09-08 10:29 ` Andrew Scull
2020-09-03 13:52 ` [PATCH v3 07/18] KVM: arm64: nVHE: Don't consume host SErrors with ESB Andrew Scull
2020-09-07 11:46 ` Marc Zyngier
2020-09-03 13:52 ` [PATCH v3 08/18] KVM: arm64: Introduce hyp context Andrew Scull
2020-09-07 13:29 ` Marc Zyngier
2020-09-08 10:52 ` Andrew Scull
2020-09-03 13:52 ` [PATCH v3 09/18] KVM: arm64: Update context references from host to hyp Andrew Scull
2020-09-03 13:52 ` [PATCH v3 10/18] KVM: arm64: Restore hyp when panicking in guest context Andrew Scull
2020-09-03 13:53 ` [PATCH v3 11/18] KVM: arm64: Share context save and restore macros Andrew Scull
2020-09-03 13:53 ` [PATCH v3 12/18] KVM: arm64: nVHE: Switch to hyp context for EL2 Andrew Scull
2020-09-07 13:02 ` Marc Zyngier [this message]
2020-09-08 10:42 ` Andrew Scull
2020-09-03 13:53 ` [PATCH v3 13/18] KVM: arm64: nVHE: Handle hyp panics Andrew Scull
2020-09-07 13:24 ` Marc Zyngier
2020-09-03 13:53 ` [PATCH v3 14/18] smccc: Cast arguments to unsigned long Andrew Scull
2020-09-07 13:33 ` Marc Zyngier
2020-09-08 10:58 ` Andrew Scull
2020-09-03 13:53 ` [PATCH v3 15/18] KVM: arm64: nVHE: Pass pointers consistently to hyp-init Andrew Scull
2020-09-03 13:53 ` [PATCH v3 16/18] KVM: arm64: nVHE: Migrate hyp interface to SMCCC Andrew Scull
2020-09-07 13:47 ` Marc Zyngier
2020-09-07 14:20 ` Marc Zyngier
2020-09-08 11:02 ` Andrew Scull
2020-09-09 8:30 ` Andrew Scull
2020-09-03 13:53 ` [PATCH v3 17/18] KVM: arm64: nVHE: Migrate hyp-init " Andrew Scull
2020-09-03 13:53 ` [PATCH v3 18/18] KVM: arm64: nVHE: Fix pointers during SMCCC convertion Andrew Scull
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v9gploxt.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=ascull@google.com \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=sudeep.holla@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).