Re: [PATCH v1 15/24] kvm: arm64: Bootstrap PSCI SMC handler in nVHE EL2
From: Marc Zyngier
Date: Wed Nov 11 2020 - 08:18:39 EST
On 2020-11-09 11:32, David Brazdil wrote:
Add a handler of PSCI SMCs in nVHE hyp code. The handler is initialized
with the version used by the host's PSCI driver and the function IDs it
was configured with. If the SMC function ID matches one of the
configured PSCI calls (for v0.1) or falls into the PSCI function ID
range (for v0.2+), the SMC is handled by the PSCI handler. For now, all
SMCs return PSCI_RET_NOT_SUPPORTED.
Signed-off-by: David Brazdil <dbrazdil@xxxxxxxxxx>
---
arch/arm64/include/asm/kvm_hyp.h | 4 ++
arch/arm64/kvm/arm.c | 13 ++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 4 ++
arch/arm64/kvm/hyp/nvhe/psci.c | 102 +++++++++++++++++++++++++++++
include/uapi/linux/psci.h | 1 +
6 files changed, 125 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/psci.c
diff --git a/arch/arm64/include/asm/kvm_hyp.h
b/arch/arm64/include/asm/kvm_hyp.h
index a3289071f3d8..95a2bbbcc7e1 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -96,6 +96,10 @@ void deactivate_traps_vhe_put(void);
u64 __guest_enter(struct kvm_vcpu *vcpu);
+#ifdef __KVM_NVHE_HYPERVISOR__
+bool kvm_host_psci_handler(struct kvm_cpu_context *host_ctxt);
+#endif
+
void __noreturn hyp_panic(void);
#ifdef __KVM_NVHE_HYPERVISOR__
void __noreturn __hyp_do_panic(bool restore_host, u64 spsr, u64 elr,
u64 par);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 1a57b6025937..28e3bc056225 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -19,6 +19,7 @@
#include <linux/kvm_irqfd.h>
#include <linux/irqbypass.h>
#include <linux/sched/stat.h>
+#include <linux/psci.h>
#include <trace/events/kvm.h>
#define CREATE_TRACE_POINTS
@@ -1498,6 +1499,17 @@ static void init_cpu_logical_map(void)
CHOOSE_NVHE_SYM(__cpu_logical_map)[cpu] = cpu_logical_map(cpu);
}
+static void init_psci(void)
nit: init_psci_relay?
+{
+ extern u32 kvm_nvhe_sym(kvm_host_psci_version);
+ extern u32 kvm_nvhe_sym(kvm_host_psci_function_id)[PSCI_FN_MAX];
+ int i;
+
+ CHOOSE_NVHE_SYM(kvm_host_psci_version) = psci_driver_version();
+ for (i = 0; i < PSCI_FN_MAX; ++i)
+ CHOOSE_NVHE_SYM(kvm_host_psci_function_id)[i] =
psci_get_function_id(i);
+}
+
static int init_common_resources(void)
{
return kvm_set_ipa_limit();
@@ -1677,6 +1689,7 @@ static int init_hyp_mode(void)
}
init_cpu_logical_map();
+ init_psci();
return 0;
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile
b/arch/arm64/kvm/hyp/nvhe/Makefile
index c45f440cce51..647b63337a51 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,7 +7,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 \
- hyp-main.o percpu.o
+ hyp-main.o percpu.o psci.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/hyp-main.c
b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 8661bc7deaa9..69f34d4f2773 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -134,6 +134,10 @@ static void handle_host_smc(struct
kvm_cpu_context *host_ctxt)
*/
skip_host_instruction();
+ /* Try to handle host's PSCI SMCs. */
+ if (kvm_host_psci_handler(host_ctxt))
+ return;
+
/* Forward SMC not handled in EL2 to EL3. */
forward_host_smc(host_ctxt);
}
diff --git a/arch/arm64/kvm/hyp/nvhe/psci.c
b/arch/arm64/kvm/hyp/nvhe/psci.c
new file mode 100644
index 000000000000..82d3b2c89658
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/psci.c
nit: can we please name this psci-relay.c, or psci-proxy.c?
We already have a psci.c in the tree, and having the same file name
messes
with my editor... ;-)
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 - Google LLC
+ * Author: David Brazdil <dbrazdil@xxxxxxxxxx>
+ */
+
+#include <asm/kvm_asm.h>
+#include <asm/kvm_hyp.h>
+#include <asm/kvm_mmu.h>
+#include <kvm/arm_hypercalls.h>
+#include <linux/arm-smccc.h>
+#include <linux/psci.h>
+#include <kvm/arm_psci.h>
+#include <uapi/linux/psci.h>
+
+/* Config options set by the host. */
+u32 kvm_host_psci_version = PSCI_VERSION(0, 0);
+u32 kvm_host_psci_function_id[PSCI_FN_MAX];
+
+static u64 get_psci_func_id(struct kvm_cpu_context *host_ctxt)
+{
+ return host_ctxt->regs.regs[0];
+}
+
+static bool is_psci_0_1_call(u64 func_id)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(kvm_host_psci_function_id); ++i) {
+ if (func_id == kvm_host_psci_function_id[i])
+ return true;
+ }
+ return false;
+}
+
+static bool is_psci_0_2_fn_call(u64 func_id)
+{
+ u64 base = func_id & ~PSCI_0_2_FN_ID_MASK;
+
+ return base == PSCI_0_2_FN_BASE || base == PSCI_0_2_FN64_BASE;
I couldn't spot in the spec where PSCI reserves 16bit worth of IDs in
each range.
+}
+
+static bool is_psci_call(u64 func_id)
+{
+ if (kvm_host_psci_version == PSCI_VERSION(0, 0))
+ return false;
+ else if (kvm_host_psci_version == PSCI_VERSION(0, 1))
+ return is_psci_0_1_call(func_id);
+ else
+ return is_psci_0_2_fn_call(func_id);
Consider using switch/case constructs for readability.
+}
+
+static unsigned long psci_0_1_handler(u64 func_id, struct
kvm_cpu_context *host_ctxt)
+{
+ return PSCI_RET_NOT_SUPPORTED;
+}
+
+static unsigned long psci_0_2_handler(u64 func_id, struct
kvm_cpu_context *host_ctxt)
+{
+ switch (func_id) {
+ default:
+ return PSCI_RET_NOT_SUPPORTED;
+ }
+}
+
+static unsigned long psci_1_0_handler(u64 func_id, struct
kvm_cpu_context *host_ctxt)
+{
+ int ret;
+
+ ret = psci_0_2_handler(func_id, host_ctxt);
+ if (ret != PSCI_RET_NOT_SUPPORTED)
+ return ret;
+
+ switch (func_id) {
+ default:
+ return PSCI_RET_NOT_SUPPORTED;
+ }
It would probably help to adopt the same structure as we have in the
KVM PSCI implementation:
switch(psci_fn) {
case PSCI_0_2_FN_PSCI_VERSION:
val = KVM_ARM_PSCI_1_0;
break;
[...]
default:
return kvm_psci_0_2_call(vcpu);
which allows 1.0 to override some 0.2 functions, and otherwise leave
it to the 0.2 backend.
+}
+
+bool kvm_host_psci_handler(struct kvm_cpu_context *host_ctxt)
+{
+ u64 func_id = get_psci_func_id(host_ctxt);
+ unsigned long ret;
+
+ if (!is_psci_call(func_id))
+ return false;
+
+ if (kvm_host_psci_version == PSCI_VERSION(0, 1))
+ ret = psci_0_1_handler(func_id, host_ctxt);
+ else if (kvm_host_psci_version == PSCI_VERSION(0, 2))
+ ret = psci_0_2_handler(func_id, host_ctxt);
+ else if (PSCI_VERSION_MAJOR(kvm_host_psci_version) >= 1)
+ ret = psci_1_0_handler(func_id, host_ctxt);
+ else
+ ret = PSCI_RET_NOT_SUPPORTED;
Same remark about the use of switch/case.
+
+ host_ctxt->regs.regs[0] = ret;
+ host_ctxt->regs.regs[1] = 0;
+ host_ctxt->regs.regs[2] = 0;
+ host_ctxt->regs.regs[3] = 0;
+ return true;
+}
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 2fcad1dd0b0e..0d52b8dbe8c2 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -29,6 +29,7 @@
#define PSCI_0_2_FN64_BASE \
(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
#define PSCI_0_2_FN64(n) (PSCI_0_2_FN64_BASE + (n))
+#define PSCI_0_2_FN_ID_MASK 0xffff
#define PSCI_0_2_FN_PSCI_VERSION PSCI_0_2_FN(0)
#define PSCI_0_2_FN_CPU_SUSPEND PSCI_0_2_FN(1)
Thanks,
M.
--
Jazz is not dead. It just smells funny...