* [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering
@ 2023-03-20 22:09 Oliver Upton
2023-03-20 22:09 ` [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL Oliver Upton
` (10 more replies)
0 siblings, 11 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
The Arm SMCCC is rather prescriptive in regards to the allocation of
SMCCC function ID ranges. Many of the hypercall ranges have an
associated specification from Arm (FF-A, PSCI, SDEI, etc.) with some
room for vendor-specific implementations.
The ever-expanding SMCCC surface leaves a lot of work within KVM for
providing new features. Furthermore, KVM implements its own
vendor-specific ABI, with little room for other implementations (like
Hyper-V, for example). Rather than cramming it all into the kernel we
should provide a way for userspace to handle hypercalls.
It would appear that vCPU hotplug [*] has a legitimate use case for
something like this, sending PSCI calls to userspace (where they
should have gone in the first place).
[*] https://lore.kernel.org/kvmarm/20230203135043.409192-1-james.morse@arm.com/
=> We have these new hypercall bitmap registers, why not use that?
The hypercall bitmap registers aren't necessarily aimed at the same
problem. The bitmap registers allow a VMM to preserve the ABI the guest
gets from KVM by default when migrating between hosts. By default KVM
exposes the entire feature set to the guest, whereas user SMCCC calls
need explicit opt-in from userspace.
Applies to 6.3-rc3.
RFCv2: https://lore.kernel.org/kvmarm/20230211013759.3556016-1-oliver.upton@linux.dev/
RFCv2 -> v1:
- Redefine kvm_run::hypercall::longmode as a flags field (Sean)
- Handle SMCs from EL1
- Pre-increment PC before exiting to userspace for an SMC
- A test!
Oliver Upton (11):
KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
KVM: arm64: Add a helper to check if a VM has ran once
KVM: arm64: Add vm fd device attribute accessors
KVM: arm64: Rename SMC/HVC call handler to reflect reality
KVM: arm64: Start handling SMCs from EL1
KVM: arm64: Refactor hvc filtering to support different actions
KVM: arm64: Use a maple tree to represent the SMCCC filter
KVM: arm64: Add support for KVM_EXIT_HYPERCALL
KVM: arm64: Indroduce support for userspace SMCCC filtering
KVM: selftests: Add a helper for SMCCC calls with SMC instruction
KVM: selftests: Add test for SMCCC filter
Documentation/virt/kvm/api.rst | 24 ++-
Documentation/virt/kvm/devices/vm.rst | 74 +++++++
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/uapi/asm/kvm.h | 24 +++
arch/arm64/kvm/arm.c | 35 ++++
arch/arm64/kvm/handle_exit.c | 22 +-
arch/arm64/kvm/hypercalls.c | 155 +++++++++++++-
arch/arm64/kvm/pmu-emul.c | 4 +-
arch/x86/include/uapi/asm/kvm.h | 9 +
arch/x86/kvm/x86.c | 5 +-
include/kvm/arm_hypercalls.h | 6 +-
include/uapi/linux/kvm.h | 9 +-
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/aarch64/smccc_filter.c | 196 ++++++++++++++++++
.../selftests/kvm/include/aarch64/processor.h | 13 ++
.../selftests/kvm/lib/aarch64/processor.c | 52 +++--
16 files changed, 593 insertions(+), 44 deletions(-)
create mode 100644 tools/testing/selftests/kvm/aarch64/smccc_filter.c
base-commit: e8d018dd0257f744ca50a729e3d042cf2ec9da65
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-21 15:53 ` Sean Christopherson
2023-03-20 22:09 ` [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once Oliver Upton
` (9 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
The 'longmode' field is a bit annoying as it blows an entire __u32 to
represent a boolean value. Since other architectures are looking to add
support for KVM_EXIT_HYPERCALL, now is probably a good time to clean it
up.
Redefine the field (and the remaining padding) as a set of flags.
Preserve the existing ABI by using the lower 32 bits of the field to
indicate if the guest was in long mode at the time of the hypercall.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/x86/include/uapi/asm/kvm.h | 9 +++++++++
arch/x86/kvm/x86.c | 5 ++++-
include/uapi/linux/kvm.h | 9 +++++++--
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7f467fe05d42..ab7b7b1d7c9d 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -559,4 +559,13 @@ struct kvm_pmu_event_filter {
#define KVM_VCPU_TSC_CTRL 0 /* control group for the timestamp counter (TSC) */
#define KVM_VCPU_TSC_OFFSET 0 /* attribute for the TSC offset */
+/*
+ * x86-specific KVM_EXIT_HYPERCALL flags.
+ *
+ * KVM previously used a u32 field to indicate the hypercall was initiated from
+ * long mode. As such, the lower 32 bits of the flags are used for long mode to
+ * preserve ABI.
+ */
+#define KVM_EXIT_HYPERCALL_LONG_MODE GENMASK_ULL(31, 0)
+
#endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7713420abab0..c61c2b0c73bd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9803,7 +9803,10 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
vcpu->run->hypercall.args[0] = gpa;
vcpu->run->hypercall.args[1] = npages;
vcpu->run->hypercall.args[2] = attrs;
- vcpu->run->hypercall.longmode = op_64_bit;
+ vcpu->run->hypercall.flags = 0;
+ if (op_64_bit)
+ vcpu->run->hypercall.flags |= KVM_EXIT_HYPERCALL_LONG_MODE;
+
vcpu->arch.complete_userspace_io = complete_hypercall_exit;
return 0;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d77aef872a0a..dd42d7dfb86c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -341,8 +341,13 @@ struct kvm_run {
__u64 nr;
__u64 args[6];
__u64 ret;
- __u32 longmode;
- __u32 pad;
+
+ union {
+#ifndef __KERNEL__
+ __u32 longmode;
+#endif
+ __u64 flags;
+ };
} hypercall;
/* KVM_EXIT_TPR_ACCESS */
struct {
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
2023-03-20 22:09 ` [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-21 9:42 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors Oliver Upton
` (8 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
The test_bit(...) pattern is quite a lot of keystrokes. Replace
existing callsites with a helper.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_host.h | 3 +++
arch/arm64/kvm/pmu-emul.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bcd774d74f34..d091d1c9890b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1061,6 +1061,9 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);
(system_supports_32bit_el0() && \
!static_branch_unlikely(&arm64_mismatched_32bit_el0))
+#define kvm_vm_has_ran_once(kvm) \
+ (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &(kvm)->arch.flags))
+
int kvm_trng_call(struct kvm_vcpu *vcpu);
#ifdef CONFIG_KVM
extern phys_addr_t hyp_mem_base;
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 24908400e190..a0fc569fdbca 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -880,7 +880,7 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu, int pmu_id)
list_for_each_entry(entry, &arm_pmus, entry) {
arm_pmu = entry->arm_pmu;
if (arm_pmu->pmu.type == pmu_id) {
- if (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags) ||
+ if (kvm_vm_has_ran_once(kvm) ||
(kvm->arch.pmu_filter && kvm->arch.arm_pmu != arm_pmu)) {
ret = -EBUSY;
break;
@@ -963,7 +963,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
mutex_lock(&kvm->lock);
- if (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags)) {
+ if (kvm_vm_has_ran_once(kvm)) {
mutex_unlock(&kvm->lock);
return -EBUSY;
}
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
2023-03-20 22:09 ` [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL Oliver Upton
2023-03-20 22:09 ` [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-21 9:53 ` Suzuki K Poulose
2023-03-28 8:40 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality Oliver Upton
` (7 subsequent siblings)
10 siblings, 2 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
A subsequent change will allow userspace to convey a filter for
hypercalls through a vm device attribute. Add the requisite boilerplate
for vm attribute accessors.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3bd732eaf087..b6e26c0e65e5 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1439,11 +1439,28 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
}
}
+static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ switch (attr->group) {
+ default:
+ return -ENXIO;
+ }
+}
+
+static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ switch (attr->group) {
+ default:
+ return -ENXIO;
+ }
+}
+
long kvm_arch_vm_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
struct kvm *kvm = filp->private_data;
void __user *argp = (void __user *)arg;
+ struct kvm_device_attr attr;
switch (ioctl) {
case KVM_CREATE_IRQCHIP: {
@@ -1479,6 +1496,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
return -EFAULT;
return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags);
}
+ case KVM_HAS_DEVICE_ATTR: {
+ if (copy_from_user(&attr, argp, sizeof(attr)))
+ return -EFAULT;
+
+ return kvm_vm_has_attr(kvm, &attr);
+ }
+ case KVM_SET_DEVICE_ATTR: {
+ if (copy_from_user(&attr, argp, sizeof(attr)))
+ return -EFAULT;
+
+ return kvm_vm_set_attr(kvm, &attr);
+ }
default:
return -EINVAL;
}
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (2 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-21 9:52 ` Suzuki K Poulose
2023-03-28 8:40 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1 Oliver Upton
` (6 subsequent siblings)
10 siblings, 2 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
KVM handles SMCCC calls from virtual EL2 that use the SMC instruction
since commit bd36b1a9eb5a ("KVM: arm64: nv: Handle SMCs taken from
virtual EL2"). Thus, the function name of the handler no longer reflects
reality.
Normalize the name on SMCCC, since that's the only hypercall interface
KVM supports in the first place. No fuctional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/handle_exit.c | 4 ++--
arch/arm64/kvm/hypercalls.c | 2 +-
include/kvm/arm_hypercalls.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index a798c0b4d717..5e4f9737cbd5 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -52,7 +52,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu)
return 1;
}
- ret = kvm_hvc_call_handler(vcpu);
+ ret = kvm_smccc_call_handler(vcpu);
if (ret < 0) {
vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
@@ -89,7 +89,7 @@ static int handle_smc(struct kvm_vcpu *vcpu)
* at Non-secure EL1 is trapped to EL2 if HCR_EL2.TSC==1, rather than
* being treated as UNDEFINED.
*/
- ret = kvm_hvc_call_handler(vcpu);
+ ret = kvm_smccc_call_handler(vcpu);
if (ret < 0)
vcpu_set_reg(vcpu, 0, ~0UL);
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 5da884e11337..05e588948e5a 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -121,7 +121,7 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
}
}
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
+int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
{
struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
u32 func_id = smccc_get_function(vcpu);
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index 1188f116cf4e..8f4e33bc43e8 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -6,7 +6,7 @@
#include <asm/kvm_emulate.h>
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+int kvm_smccc_call_handler(struct kvm_vcpu *vcpu);
static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
{
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (3 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-28 8:52 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions Oliver Upton
` (5 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
Whelp, the architecture gods have spoken and confirmed that the function
ID space is common between SMCs and HVCs. Not only that, the expectation
is that hypervisors handle calls to both SMC and HVC conduits. KVM
recently picked up support for SMCCCs in commit bd36b1a9eb5a ("KVM:
arm64: nv: Handle SMCs taken from virtual EL2") but scoped it only to a
nested hypervisor.
Let's just open the floodgates and let EL1 access our SMCCC
implementation with the SMC instruction as well.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/handle_exit.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 5e4f9737cbd5..68f95dcd41a1 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -72,13 +72,15 @@ static int handle_smc(struct kvm_vcpu *vcpu)
*
* We need to advance the PC after the trap, as it would
* otherwise return to the same address...
- *
- * Only handle SMCs from the virtual EL2 with an immediate of zero and
- * skip it otherwise.
*/
- if (!vcpu_is_el2(vcpu) || kvm_vcpu_hvc_get_imm(vcpu)) {
+ kvm_incr_pc(vcpu);
+
+ /*
+ * SMCs with a nonzero immediate are reserved according to DEN0028E 2.9
+ * "SMC and HVC immediate value".
+ */
+ if (kvm_vcpu_hvc_get_imm(vcpu)) {
vcpu_set_reg(vcpu, 0, ~0UL);
- kvm_incr_pc(vcpu);
return 1;
}
@@ -93,8 +95,6 @@ static int handle_smc(struct kvm_vcpu *vcpu)
if (ret < 0)
vcpu_set_reg(vcpu, 0, ~0UL);
- kvm_incr_pc(vcpu);
-
return ret;
}
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (4 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1 Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-28 9:19 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 07/11] KVM: arm64: Use a maple tree to represent the SMCCC filter Oliver Upton
` (4 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
KVM presently allows userspace to filter guest hypercalls with bitmaps
expressed via pseudo-firmware registers. These bitmaps have a narrow
scope and, of course, can only allow/deny a particular call. A
subsequent change to KVM will introduce a generalized UAPI for filtering
hypercalls, allowing functions to be forwarded to userspace.
Refactor the existing hypercall filtering logic to make room for more
than two actions. While at it, generalize the function names around
SMCCC as it is the basis for the upcoming UAPI.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++
arch/arm64/kvm/hypercalls.c | 19 +++++++++++++++----
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index f8129c624b07..bbab92402510 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -469,6 +469,15 @@ enum {
/* run->fail_entry.hardware_entry_failure_reason codes. */
#define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED (1ULL << 0)
+enum kvm_smccc_filter_action {
+ KVM_SMCCC_FILTER_ALLOW = 0,
+ KVM_SMCCC_FILTER_DENY,
+
+#ifdef __KERNEL__
+ NR_SMCCC_FILTER_ACTIONS
+#endif
+};
+
#endif
#endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 05e588948e5a..50145d2132ae 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -65,7 +65,7 @@ static void kvm_ptp_get_time(struct kvm_vcpu *vcpu, u64 *val)
val[3] = lower_32_bits(cycles);
}
-static bool kvm_hvc_call_default_allowed(u32 func_id)
+static bool kvm_smccc_default_allowed(u32 func_id)
{
switch (func_id) {
/*
@@ -93,7 +93,7 @@ static bool kvm_hvc_call_default_allowed(u32 func_id)
}
}
-static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
+static bool kvm_smccc_test_fw_bmap(struct kvm_vcpu *vcpu, u32 func_id)
{
struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
@@ -117,19 +117,30 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
return test_bit(KVM_REG_ARM_VENDOR_HYP_BIT_PTP,
&smccc_feat->vendor_hyp_bmap);
default:
- return kvm_hvc_call_default_allowed(func_id);
+ return false;
}
}
+static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
+{
+ if (kvm_smccc_test_fw_bmap(vcpu, func_id) ||
+ kvm_smccc_default_allowed(func_id))
+ return KVM_SMCCC_FILTER_ALLOW;
+
+ return KVM_SMCCC_FILTER_DENY;
+}
+
int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
{
struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
u32 func_id = smccc_get_function(vcpu);
u64 val[4] = {SMCCC_RET_NOT_SUPPORTED};
u32 feature;
+ u8 action;
gpa_t gpa;
- if (!kvm_hvc_call_allowed(vcpu, func_id))
+ action = kvm_smccc_get_action(vcpu, func_id);
+ if (action == KVM_SMCCC_FILTER_DENY)
goto out;
switch (func_id) {
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/11] KVM: arm64: Use a maple tree to represent the SMCCC filter
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (5 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-20 22:09 ` [PATCH 08/11] KVM: arm64: Add support for KVM_EXIT_HYPERCALL Oliver Upton
` (3 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
Maple tree is an efficient B-tree implementation that is intended for
storing non-overlapping intervals. Such a data structure is a good fit
for the SMCCC filter as it is desirable to sparsely allocate the 32 bit
function ID space.
To that end, add a maple tree to kvm_arch and correctly init/teardown
along with the VM. Wire in a test against the hypercall filter for HVCs
which does nothing until the controls are exposed to userspace.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/include/asm/kvm_host.h | 5 ++-
arch/arm64/kvm/arm.c | 2 ++
arch/arm64/kvm/hypercalls.c | 57 +++++++++++++++++++++++++++++++
include/kvm/arm_hypercalls.h | 1 +
4 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index d091d1c9890b..2682b3fd0881 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -16,6 +16,7 @@
#include <linux/types.h>
#include <linux/jump_label.h>
#include <linux/kvm_types.h>
+#include <linux/maple_tree.h>
#include <linux/percpu.h>
#include <linux/psci.h>
#include <asm/arch_gicv3.h>
@@ -221,7 +222,8 @@ struct kvm_arch {
#define KVM_ARCH_FLAG_EL1_32BIT 4
/* PSCI SYSTEM_SUSPEND enabled for the guest */
#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5
-
+ /* SMCCC filter initialized for the VM */
+#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED 6
unsigned long flags;
/*
@@ -242,6 +244,7 @@ struct kvm_arch {
/* Hypercall features firmware registers' descriptor */
struct kvm_smccc_features smccc_feat;
+ struct maple_tree smccc_filter;
/*
* For an untrusted host VM, 'pkvm.handle' is used to lookup
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index b6e26c0e65e5..1202ac03bee0 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -192,6 +192,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
kvm_destroy_vcpus(kvm);
kvm_unshare_hyp(kvm, kvm + 1);
+
+ kvm_arm_teardown_hypercalls(kvm);
}
int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 50145d2132ae..76d39297ed18 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -121,8 +121,58 @@ static bool kvm_smccc_test_fw_bmap(struct kvm_vcpu *vcpu, u32 func_id)
}
}
+#define SMCCC_ARCH_RANGE_BEGIN ARM_SMCCC_VERSION_FUNC_ID
+#define SMCCC_ARCH_RANGE_END \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_32, \
+ 0, ARM_SMCCC_FUNC_MASK)
+
+static void init_smccc_filter(struct kvm *kvm)
+{
+ int r;
+
+ mt_init(&kvm->arch.smccc_filter);
+
+ /*
+ * Prevent userspace from handling any SMCCC calls in the architecture
+ * range, avoiding the risk of misrepresenting Spectre mitigation status
+ * to the guest.
+ */
+ r = mtree_insert_range(&kvm->arch.smccc_filter,
+ SMCCC_ARCH_RANGE_BEGIN, SMCCC_ARCH_RANGE_END,
+ xa_mk_value(KVM_SMCCC_FILTER_ALLOW),
+ GFP_KERNEL_ACCOUNT);
+ KVM_BUG_ON(r, kvm);
+}
+
+static u8 kvm_smccc_filter_get_action(struct kvm *kvm, u32 func_id)
+{
+ unsigned long idx = func_id;
+ void *val;
+
+ if (!test_bit(KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED, &kvm->arch.flags))
+ return KVM_SMCCC_FILTER_ALLOW;
+
+ /*
+ * But where's the error handling, you say?
+ *
+ * mt_find() returns NULL if no entry was found, which just so happens
+ * to match KVM_SMCCC_FILTER_ALLOW.
+ */
+ val = mt_find(&kvm->arch.smccc_filter, &idx, idx);
+ return xa_to_value(val);
+}
+
static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
{
+ /*
+ * Intervening actions in the SMCCC filter take precedence over the
+ * pseudo-firmware register bitmaps.
+ */
+ u8 action = kvm_smccc_filter_get_action(vcpu->kvm, func_id);
+ if (action != KVM_SMCCC_FILTER_ALLOW)
+ return action;
+
if (kvm_smccc_test_fw_bmap(vcpu, func_id) ||
kvm_smccc_default_allowed(func_id))
return KVM_SMCCC_FILTER_ALLOW;
@@ -256,6 +306,13 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
+
+ init_smccc_filter(kvm);
+}
+
+void kvm_arm_teardown_hypercalls(struct kvm *kvm)
+{
+ mtree_destroy(&kvm->arch.smccc_filter);
}
int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index 8f4e33bc43e8..fe6c31575b05 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -43,6 +43,7 @@ static inline void smccc_set_retval(struct kvm_vcpu *vcpu,
struct kvm_one_reg;
void kvm_arm_init_hypercalls(struct kvm *kvm);
+void kvm_arm_teardown_hypercalls(struct kvm *kvm);
int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu);
int kvm_arm_copy_fw_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices);
int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/11] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (6 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 07/11] KVM: arm64: Use a maple tree to represent the SMCCC filter Oliver Upton
@ 2023-03-20 22:09 ` Oliver Upton
2023-03-20 22:10 ` [PATCH 09/11] KVM: arm64: Indroduce support for userspace SMCCC filtering Oliver Upton
` (2 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:09 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
In anticipation of user hypercall filters, add the necessary plumbing to
get SMCCC calls out to userspace. Even though the exit structure has
space for KVM to pass register arguments, let's just avoid it altogether
and let userspace poke at the registers via KVM_GET_ONE_REG.
This deliberately stretches the definition of a 'hypercall' to cover
SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
support EL1 calls into secure services, but now we can paint that as a
userspace problem and be done with it.
Finally, we need a flag to let userspace know what conduit instruction
was used (i.e. SMC vs. HVC).
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
Documentation/virt/kvm/api.rst | 24 +++++++++++++++++++++---
arch/arm64/include/uapi/asm/kvm.h | 4 ++++
arch/arm64/kvm/handle_exit.c | 4 +++-
arch/arm64/kvm/hypercalls.c | 19 +++++++++++++++++++
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 62de0768d6aa..3b43520003a2 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6219,14 +6219,32 @@ to the byte array.
__u64 args[6];
__u64 ret;
__u32 longmode;
- __u32 pad;
+ __u32 flags;
} hypercall;
-Unused. This was once used for 'hypercall to userspace'. To implement
-such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
+
+It is strongly recommended that userspace use ``KVM_EXIT_IO`` (x86) or
+``KVM_EXIT_MMIO`` (all except s390) to implement functionality that
+requires a guest to interact with host userpace.
.. note:: KVM_EXIT_IO is significantly faster than KVM_EXIT_MMIO.
+For arm64:
+----------
+
+SMCCC exits can be enabled depending on the configuration of the SMCCC
+filter. See the Documentation/virt/kvm/devices/vm.rst
+``KVM_ARM_SMCCC_FILTER`` for more details.
+
+``nr`` contains the function ID of the guest's SMCCC call. Userspace is
+expected to use the ``KVM_GET_ONE_REG`` ioctl to retrieve the call
+parameters from the vCPU's GPRs.
+
+Definition of ``flags``:
+ - ``KVM_HYPERCALL_EXIT_SMC``: Indicates that the guest used the SMC
+ conduit to initiate the SMCCC call. If this bit is 0 then the guest
+ used the HVC conduit for the SMCCC call.
+
::
/* KVM_EXIT_TPR_ACCESS */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index bbab92402510..1dabb7d05514 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -472,12 +472,16 @@ enum {
enum kvm_smccc_filter_action {
KVM_SMCCC_FILTER_ALLOW = 0,
KVM_SMCCC_FILTER_DENY,
+ KVM_SMCCC_FILTER_FWD_TO_USER,
#ifdef __KERNEL__
NR_SMCCC_FILTER_ACTIONS
#endif
};
+/* arm64-specific KVM_EXIT_HYPERCALL flags */
+#define KVM_HYPERCALL_EXIT_SMC (1U << 0)
+
#endif
#endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 68f95dcd41a1..3f43e20c48b6 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
* Trap exception, not a Secure Monitor Call exception [...]"
*
* We need to advance the PC after the trap, as it would
- * otherwise return to the same address...
+ * otherwise return to the same address. Furthermore, pre-incrementing
+ * the PC before potentially exiting to userspace maintains the same
+ * abstraction for both SMCs and HVCs.
*/
kvm_incr_pc(vcpu);
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 76d39297ed18..2843c8493c8a 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -180,6 +180,19 @@ static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
return KVM_SMCCC_FILTER_DENY;
}
+static void kvm_prepare_hypercall_exit(struct kvm_vcpu *vcpu, u32 func_id)
+{
+ u8 ec = ESR_ELx_EC(kvm_vcpu_get_esr(vcpu));
+ struct kvm_run *run = vcpu->run;
+
+ run->exit_reason = KVM_EXIT_HYPERCALL;
+ run->hypercall.nr = func_id;
+ run->hypercall.flags = 0;
+
+ if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64)
+ run->hypercall.flags |= KVM_HYPERCALL_EXIT_SMC;
+}
+
int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
{
struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
@@ -192,6 +205,10 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
action = kvm_smccc_get_action(vcpu, func_id);
if (action == KVM_SMCCC_FILTER_DENY)
goto out;
+ if (action == KVM_SMCCC_FILTER_FWD_TO_USER) {
+ kvm_prepare_hypercall_exit(vcpu, func_id);
+ return 0;
+ }
switch (func_id) {
case ARM_SMCCC_VERSION_FUNC_ID:
@@ -206,6 +223,8 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
break;
case SPECTRE_MITIGATED:
val[0] = SMCCC_RET_SUCCESS;
+ kvm_incr_pc(vcpu);
+
break;
case SPECTRE_UNAFFECTED:
val[0] = SMCCC_ARCH_WORKAROUND_RET_UNAFFECTED;
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/11] KVM: arm64: Indroduce support for userspace SMCCC filtering
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (7 preceding siblings ...)
2023-03-20 22:09 ` [PATCH 08/11] KVM: arm64: Add support for KVM_EXIT_HYPERCALL Oliver Upton
@ 2023-03-20 22:10 ` Oliver Upton
2023-03-20 22:10 ` [PATCH 10/11] KVM: selftests: Add a helper for SMCCC calls with SMC instruction Oliver Upton
2023-03-20 22:10 ` [PATCH 11/11] KVM: selftests: Add test for SMCCC filter Oliver Upton
10 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:10 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
As the SMCCC (and related specifications) march towards an 'everything
and the kitchen sink' interface for interacting with a system it becomes
less likely that KVM will support every related feature. We could do
better by letting userspace have a crack at it instead.
Allow userspace to define an 'SMCCC filter' that applies to both HVCs
and SMCs initiated by the guest. Supporting both conduits with this
interface is important for a couple of reasons. Guest SMC usage is table
stakes for a nested guest, as HVCs are always taken to the virtual EL2.
Additionally, guests may want to interact with a service on the secure
side which can now be proxied by userspace.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
Documentation/virt/kvm/devices/vm.rst | 74 +++++++++++++++++++++++++++
arch/arm64/include/uapi/asm/kvm.h | 11 ++++
arch/arm64/kvm/arm.c | 4 ++
arch/arm64/kvm/hypercalls.c | 58 +++++++++++++++++++++
include/kvm/arm_hypercalls.h | 3 ++
5 files changed, 150 insertions(+)
diff --git a/Documentation/virt/kvm/devices/vm.rst b/Documentation/virt/kvm/devices/vm.rst
index 147efec626e5..9bba8bacbd5d 100644
--- a/Documentation/virt/kvm/devices/vm.rst
+++ b/Documentation/virt/kvm/devices/vm.rst
@@ -321,3 +321,77 @@ Allows userspace to query the status of migration mode.
if it is enabled
:Returns: -EFAULT if the given address is not accessible from kernel space;
0 in case of success.
+
+6. GROUP: KVM_ARM_VM_SMCCC_CTRL
+===============================
+
+:Architectures: arm64
+
+6.1. ATTRIBUTE: KVM_ARM_VM_SMCCC_FILTER (w/o)
+---------------------------------------------
+
+:Parameters: Pointer to a ``struct kvm_smccc_filter``
+
+:Returns:
+
+ ======= ===========================================
+ -EPERM Range intersects with a reserved range
+ -EEXIST Range intersects with a previously inserted
+ range
+ -EBUSY A vCPU in the VM has already run
+ -EINVAL Invalid filter configuration
+ -ENOMEM Failed to allocate memory for the in-kernel
+ representation of the SMCCC filter
+ ======= ===========================================
+
+Requests the installation of an SMCCC call filter described as follows::
+
+ enum kvm_smccc_filter_action {
+ KVM_SMCCC_FILTER_ALLOW = 0,
+ KVM_SMCCC_FILTER_DENY,
+ KVM_SMCCC_FILTER_FWD_TO_USER,
+ };
+
+ struct kvm_smccc_filter {
+ __u32 base;
+ __u32 nr_functions;
+ __u8 action;
+ __u8 pad[7];
+ };
+
+The filter is defined as a set of non-overlapping ranges. Each
+range defines an action to be applied to SMCCC calls within the range.
+Userspace can insert multiple ranges into the filter by using
+successive calls to this attribute.
+
+The default configuration of KVM is such that all implemented SMCCC
+calls are allowed. Thus, the SMCCC filter can be defined sparsely
+by userspace, only describing ranges that modify the default behavior.
+
+The range expressed by ``struct kvm_smccc_filter`` is
+[``base``, ``base + nr_functions``).
+
+The SMCCC filter applies to both SMC and HVC calls initiated by the
+guest.
+
+Actions:
+
+ - ``KVM_SMCCC_FILTER_ALLOW``: Allows the guest SMCCC call to be
+ handled in-kernel. It is strongly recommended that userspace *not*
+ explicitly describe the allowed SMCCC call ranges.
+
+ - ``KVM_SMCCC_FILTER_DENY``: Rejects the guest SMCCC call in-kernel
+ and returns to the guest.
+
+ - ``KVM_SMCCC_FILTER_FWD_TO_USER``: The guest SMCCC call is forwarded
+ to userspace with an exit reason of ``KVM_EXIT_HYPERCALL``.
+
+KVM reserves the 'Arm Architecture Calls' range of function IDs and
+will reject attempts to define a filter for any portion of these ranges:
+
+ =========== ===============
+ Start End (inclusive)
+ =========== ===============
+ 0x8000_0000 0x8000_FFFF
+ 0xC000_0000 0xC000_FFFF
+ =========== ===============
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 1dabb7d05514..ba188562b7e0 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -372,6 +372,10 @@ enum {
#endif
};
+/* Device Control API on vm fd */
+#define KVM_ARM_VM_SMCCC_CTRL 0
+#define KVM_ARM_VM_SMCCC_FILTER 0
+
/* Device Control API: ARM VGIC */
#define KVM_DEV_ARM_VGIC_GRP_ADDR 0
#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
@@ -479,6 +483,13 @@ enum kvm_smccc_filter_action {
#endif
};
+struct kvm_smccc_filter {
+ __u32 base;
+ __u32 nr_functions;
+ __u8 action;
+ __u8 pad[7];
+};
+
/* arm64-specific KVM_EXIT_HYPERCALL flags */
#define KVM_HYPERCALL_EXIT_SMC (1U << 0)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 1202ac03bee0..efee032c9560 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1444,6 +1444,8 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
{
switch (attr->group) {
+ case KVM_ARM_VM_SMCCC_CTRL:
+ return kvm_vm_smccc_has_attr(kvm, attr);
default:
return -ENXIO;
}
@@ -1452,6 +1454,8 @@ static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
{
switch (attr->group) {
+ case KVM_ARM_VM_SMCCC_CTRL:
+ return kvm_vm_smccc_set_attr(kvm, attr);
default:
return -ENXIO;
}
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 2843c8493c8a..e30c3d59f9d7 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -145,6 +145,42 @@ static void init_smccc_filter(struct kvm *kvm)
KVM_BUG_ON(r, kvm);
}
+static int kvm_smccc_set_filter(struct kvm *kvm, struct kvm_smccc_filter __user *uaddr)
+{
+ struct kvm_smccc_filter filter;
+ unsigned long start, end;
+ int r;
+
+ if (copy_from_user(&filter, uaddr, sizeof(filter)))
+ return -EFAULT;
+
+ mutex_lock(&kvm->lock);
+
+ if (kvm_vm_has_ran_once(kvm)) {
+ r = -EBUSY;
+ goto out_unlock;
+ }
+
+ if (!filter.nr_functions || filter.action >= NR_SMCCC_FILTER_ACTIONS) {
+ r = -EINVAL;
+ goto out_unlock;
+ }
+
+ start = filter.base;
+ end = start + filter.nr_functions - 1;
+
+ r = mtree_insert_range(&kvm->arch.smccc_filter, start, end,
+ xa_mk_value(filter.action), GFP_KERNEL_ACCOUNT);
+ if (r)
+ goto out_unlock;
+
+ set_bit(KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED, &kvm->arch.flags);
+
+out_unlock:
+ mutex_unlock(&kvm->lock);
+ return r;
+}
+
static u8 kvm_smccc_filter_get_action(struct kvm *kvm, u32 func_id)
{
unsigned long idx = func_id;
@@ -566,3 +602,25 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
return -EINVAL;
}
+
+int kvm_vm_smccc_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ switch (attr->attr) {
+ case KVM_ARM_VM_SMCCC_FILTER:
+ return 0;
+ default:
+ return -ENXIO;
+ }
+}
+
+int kvm_vm_smccc_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ void __user *uaddr = (void __user *)attr->addr;
+
+ switch (attr->attr) {
+ case KVM_ARM_VM_SMCCC_FILTER:
+ return kvm_smccc_set_filter(kvm, uaddr);
+ default:
+ return -ENXIO;
+ }
+}
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index fe6c31575b05..2df152207ccd 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -49,4 +49,7 @@ int kvm_arm_copy_fw_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices);
int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
+int kvm_vm_smccc_has_attr(struct kvm *kvm, struct kvm_device_attr *attr);
+int kvm_vm_smccc_set_attr(struct kvm *kvm, struct kvm_device_attr *attr);
+
#endif
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/11] KVM: selftests: Add a helper for SMCCC calls with SMC instruction
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (8 preceding siblings ...)
2023-03-20 22:10 ` [PATCH 09/11] KVM: arm64: Indroduce support for userspace SMCCC filtering Oliver Upton
@ 2023-03-20 22:10 ` Oliver Upton
2023-03-20 22:10 ` [PATCH 11/11] KVM: selftests: Add test for SMCCC filter Oliver Upton
10 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:10 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
Build a helper for doing SMCs in selftests by macro-izing the current
HVC implementation and taking the conduit instruction as an argument.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
.../selftests/kvm/include/aarch64/processor.h | 13 +++++
.../selftests/kvm/lib/aarch64/processor.c | 52 ++++++++++++-------
2 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 5f977528e09c..cb537253a6b9 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -214,6 +214,19 @@ void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
uint64_t arg6, struct arm_smccc_res *res);
+/**
+ * smccc_smc - Invoke a SMCCC function using the smc conduit
+ * @function_id: the SMCCC function to be called
+ * @arg0-arg6: SMCCC function arguments, corresponding to registers x1-x7
+ * @res: pointer to write the return values from registers x0-x3
+ *
+ */
+void smccc_smc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
+ uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
+ uint64_t arg6, struct arm_smccc_res *res);
+
+
+
uint32_t guest_get_vcpuid(void);
#endif /* SELFTEST_KVM_PROCESSOR_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 5972a23b2765..24e8122307f4 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -508,29 +508,43 @@ void aarch64_get_supported_page_sizes(uint32_t ipa,
close(kvm_fd);
}
+#define __smccc_call(insn, function_id, arg0, arg1, arg2, arg3, arg4, arg5, \
+ arg6, res) \
+ asm volatile("mov w0, %w[function_id]\n" \
+ "mov x1, %[arg0]\n" \
+ "mov x2, %[arg1]\n" \
+ "mov x3, %[arg2]\n" \
+ "mov x4, %[arg3]\n" \
+ "mov x5, %[arg4]\n" \
+ "mov x6, %[arg5]\n" \
+ "mov x7, %[arg6]\n" \
+ #insn "#0\n" \
+ "mov %[res0], x0\n" \
+ "mov %[res1], x1\n" \
+ "mov %[res2], x2\n" \
+ "mov %[res3], x3\n" \
+ : [res0] "=r"(res->a0), [res1] "=r"(res->a1), \
+ [res2] "=r"(res->a2), [res3] "=r"(res->a3) \
+ : [function_id] "r"(function_id), [arg0] "r"(arg0), \
+ [arg1] "r"(arg1), [arg2] "r"(arg2), [arg3] "r"(arg3), \
+ [arg4] "r"(arg4), [arg5] "r"(arg5), [arg6] "r"(arg6) \
+ : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7")
+
+
void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
uint64_t arg6, struct arm_smccc_res *res)
{
- asm volatile("mov w0, %w[function_id]\n"
- "mov x1, %[arg0]\n"
- "mov x2, %[arg1]\n"
- "mov x3, %[arg2]\n"
- "mov x4, %[arg3]\n"
- "mov x5, %[arg4]\n"
- "mov x6, %[arg5]\n"
- "mov x7, %[arg6]\n"
- "hvc #0\n"
- "mov %[res0], x0\n"
- "mov %[res1], x1\n"
- "mov %[res2], x2\n"
- "mov %[res3], x3\n"
- : [res0] "=r"(res->a0), [res1] "=r"(res->a1),
- [res2] "=r"(res->a2), [res3] "=r"(res->a3)
- : [function_id] "r"(function_id), [arg0] "r"(arg0),
- [arg1] "r"(arg1), [arg2] "r"(arg2), [arg3] "r"(arg3),
- [arg4] "r"(arg4), [arg5] "r"(arg5), [arg6] "r"(arg6)
- : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7");
+ __smccc_call(hvc, function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+ arg6, res);
+}
+
+void smccc_smc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
+ uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
+ uint64_t arg6, struct arm_smccc_res *res)
+{
+ __smccc_call(smc, function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+ arg6, res);
}
void kvm_selftest_arch_init(void)
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/11] KVM: selftests: Add test for SMCCC filter
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
` (9 preceding siblings ...)
2023-03-20 22:10 ` [PATCH 10/11] KVM: selftests: Add a helper for SMCCC calls with SMC instruction Oliver Upton
@ 2023-03-20 22:10 ` Oliver Upton
10 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-20 22:10 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Suzuki K Poulose,
Zenghui Yu, Sean Christopherson, Salil Mehta, Oliver Upton
Add a selftest for the SMCCC filter, ensuring basic UAPI constraints
(e.g. reserved ranges, non-overlapping ranges) are upheld. Additionally,
test that the DENIED and FWD_TO_USER work as intended.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/aarch64/smccc_filter.c | 196 ++++++++++++++++++
2 files changed, 197 insertions(+)
create mode 100644 tools/testing/selftests/kvm/aarch64/smccc_filter.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 84a627c43795..d66a0642cffd 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -141,6 +141,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
TEST_GEN_PROGS_aarch64 += aarch64/psci_test
+TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
diff --git a/tools/testing/selftests/kvm/aarch64/smccc_filter.c b/tools/testing/selftests/kvm/aarch64/smccc_filter.c
new file mode 100644
index 000000000000..7f4950d18b42
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/smccc_filter.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * smccc_filter - Tests for the SMCCC filter UAPI.
+ *
+ * Copyright (c) 2023 Google LLC
+ *
+ * This test includes:
+ * - Tests that the UAPI constraints are upheld by KVM. For example, userspace
+ * is prevented from filtering the architecture range of SMCCC calls.
+ * - Test that the filter actions (DENIED, FWD_TO_USER) work as intended.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/psci.h>
+#include <stdint.h>
+
+#include "processor.h"
+#include "test_util.h"
+
+enum smccc_conduit {
+ HVC_INSN,
+ SMC_INSN,
+};
+
+#define for_each_conduit(conduit) \
+ for (conduit = HVC_INSN; conduit <= SMC_INSN; conduit++)
+
+static void guest_main(uint32_t func_id, enum smccc_conduit conduit)
+{
+ struct arm_smccc_res res;
+
+ if (conduit == SMC_INSN)
+ smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+ else
+ smccc_hvc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+
+ GUEST_SYNC(res.a0);
+}
+
+static int __set_smccc_filter(struct kvm_vm *vm, uint32_t start, uint32_t nr_functions,
+ enum kvm_smccc_filter_action action)
+{
+ struct kvm_smccc_filter filter = {
+ .base = start,
+ .nr_functions = nr_functions,
+ .action = action,
+ };
+
+ return __kvm_device_attr_set(vm->fd, KVM_ARM_VM_SMCCC_CTRL,
+ KVM_ARM_VM_SMCCC_FILTER, &filter);
+}
+
+static void set_smccc_filter(struct kvm_vm *vm, uint32_t start, uint32_t nr_functions,
+ enum kvm_smccc_filter_action action)
+{
+ int ret = __set_smccc_filter(vm, start, nr_functions, action);
+
+ TEST_ASSERT(!ret, "failed to configure SMCCC filter: %d", ret);
+}
+
+static struct kvm_vm *setup_vm(struct kvm_vcpu **vcpu)
+{
+ struct kvm_vcpu_init init;
+ struct kvm_vm *vm;
+
+ vm = vm_create(1);
+ vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
+
+ /*
+ * Enable in-kernel emulation of PSCI to ensure that calls are denied
+ * due to the SMCCC filter, not because of KVM.
+ */
+ init.features[0] |= (1 << KVM_ARM_VCPU_PSCI_0_2);
+
+ *vcpu = aarch64_vcpu_add(vm, 0, &init, guest_main);
+ return vm;
+}
+
+/* Ensure that userspace cannot filter the Arm Architecture SMCCC range */
+static void test_filter_reserved_range(void)
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm = setup_vm(&vcpu);
+ int r;
+
+ r = __set_smccc_filter(vm, ARM_SMCCC_ARCH_WORKAROUND_1,
+ 1, KVM_SMCCC_FILTER_DENY);
+ TEST_ASSERT(r, "Attempt to filter reserved range unexpectedly succeeded");
+
+ kvm_vm_free(vm);
+}
+
+/* Test that overlapping configurations of the SMCCC filter are rejected */
+static void test_filter_overlap(void)
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm = setup_vm(&vcpu);
+ int r;
+
+ set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 1, KVM_SMCCC_FILTER_DENY);
+
+ r = __set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 1, KVM_SMCCC_FILTER_DENY);
+ TEST_ASSERT(r, "Attempt to filter already configured range unexpectedly succeeded");
+
+ kvm_vm_free(vm);
+}
+
+static void expect_call_denied(struct kvm_vcpu *vcpu)
+{
+ struct ucall uc;
+
+ if (get_ucall(vcpu, &uc) != UCALL_SYNC)
+ TEST_FAIL("Unexpected ucall: %lu\n", uc.cmd);
+
+ TEST_ASSERT(uc.args[1] == SMCCC_RET_NOT_SUPPORTED,
+ "Unexpected SMCCC return code: %lu", uc.args[1]);
+}
+
+/* Denied SMCCC calls have a return code of SMCCC_RET_NOT_SUPPORTED */
+static void test_filter_denied(void)
+{
+ enum smccc_conduit conduit;
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+
+ for_each_conduit(conduit) {
+ vm = setup_vm(&vcpu);
+
+ set_smccc_filter(vm, PSCI_0_2_FN_PSCI_VERSION, 1, KVM_SMCCC_FILTER_DENY);
+ vcpu_args_set(vcpu, 2, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+ vcpu_run(vcpu);
+ expect_call_denied(vcpu);
+
+ kvm_vm_free(vm);
+ }
+}
+
+static void expect_call_fwd_to_user(struct kvm_vcpu *vcpu, uint32_t func_id,
+ enum smccc_conduit conduit)
+{
+ struct kvm_run *run = vcpu->run;
+
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_HYPERCALL,
+ "Unexpected exit reason: %u", run->exit_reason);
+ TEST_ASSERT(run->hypercall.nr == func_id,
+ "Unexpected SMCCC function: %llu", run->hypercall.nr);
+
+ if (conduit == SMC_INSN)
+ TEST_ASSERT(run->hypercall.flags & KVM_HYPERCALL_EXIT_SMC,
+ "KVM_HYPERCALL_EXIT_SMC is not set");
+ else
+ TEST_ASSERT(!(run->hypercall.flags & KVM_HYPERCALL_EXIT_SMC),
+ "KVM_HYPERCAL_EXIT_SMC is set");
+}
+
+/* SMCCC calls forwarded to userspace cause KVM_EXIT_HYPERCALL exits */
+static void test_filter_fwd_to_user(void)
+{
+ enum smccc_conduit conduit;
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+
+ for_each_conduit(conduit) {
+ vm = setup_vm(&vcpu);
+
+ set_smccc_filter(vm, PSCI_0_2_FN_PSCI_VERSION, 1, KVM_SMCCC_FILTER_FWD_TO_USER);
+ vcpu_args_set(vcpu, 2, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+ vcpu_run(vcpu);
+ expect_call_fwd_to_user(vcpu, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+ kvm_vm_free(vm);
+ }
+}
+
+static bool kvm_supports_smccc_filter(void)
+{
+ struct kvm_vm *vm = vm_create_barebones();
+ int r;
+
+ r = __kvm_has_device_attr(vm->fd, KVM_ARM_VM_SMCCC_CTRL, KVM_ARM_VM_SMCCC_FILTER);
+
+ kvm_vm_free(vm);
+ return !r;
+}
+
+int main(void)
+{
+ TEST_REQUIRE(kvm_supports_smccc_filter());
+
+ test_filter_reserved_range();
+ test_filter_overlap();
+ test_filter_denied();
+ test_filter_fwd_to_user();
+}
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once
2023-03-20 22:09 ` [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once Oliver Upton
@ 2023-03-21 9:42 ` Suzuki K Poulose
2023-03-21 16:29 ` Oliver Upton
0 siblings, 1 reply; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-21 9:42 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
Hi Oliver,
On 20/03/2023 22:09, Oliver Upton wrote:
> The test_bit(...) pattern is quite a lot of keystrokes. Replace
> existing callsites with a helper.
>
> No functional change intended.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/arm64/include/asm/kvm_host.h | 3 +++
> arch/arm64/kvm/pmu-emul.c | 4 ++--
There is one more instance in arch/arm64/kvm/hypercalls.c at
kvm_arm_set_fw_reg_bmap(). Is there a reason why that can't be replaced ?
Otherwise, looks good to me.
Suzuki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality
2023-03-20 22:09 ` [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality Oliver Upton
@ 2023-03-21 9:52 ` Suzuki K Poulose
2023-03-28 8:40 ` Suzuki K Poulose
1 sibling, 0 replies; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-21 9:52 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> KVM handles SMCCC calls from virtual EL2 that use the SMC instruction
> since commit bd36b1a9eb5a ("KVM: arm64: nv: Handle SMCs taken from
> virtual EL2"). Thus, the function name of the handler no longer reflects
> reality.
>
> Normalize the name on SMCCC, since that's the only hypercall interface
> KVM supports in the first place. No fuctional change intended.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors
2023-03-20 22:09 ` [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors Oliver Upton
@ 2023-03-21 9:53 ` Suzuki K Poulose
2023-03-21 16:49 ` Oliver Upton
2023-03-28 8:40 ` Suzuki K Poulose
1 sibling, 1 reply; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-21 9:53 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> A subsequent change will allow userspace to convey a filter for
> hypercalls through a vm device attribute. Add the requisite boilerplate
> for vm attribute accessors.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 3bd732eaf087..b6e26c0e65e5 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1439,11 +1439,28 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
> }
> }
>
> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> + switch (attr->group) {
> + default:
> + return -ENXIO;
> + }
> +}
> +
> +static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> + switch (attr->group) {
> + default:
> + return -ENXIO;
> + }
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> struct kvm *kvm = filp->private_data;
> void __user *argp = (void __user *)arg;
> + struct kvm_device_attr attr;
>
> switch (ioctl) {
> case KVM_CREATE_IRQCHIP: {
> @@ -1479,6 +1496,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
> return -EFAULT;
> return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags);
> }
> + case KVM_HAS_DEVICE_ATTR: {
> + if (copy_from_user(&attr, argp, sizeof(attr)))
> + return -EFAULT;
> +
> + return kvm_vm_has_attr(kvm, &attr);
> + }
> + case KVM_SET_DEVICE_ATTR: {
> + if (copy_from_user(&attr, argp, sizeof(attr)))
> + return -EFAULT;
> +
> + return kvm_vm_set_attr(kvm, &attr);
> + }
Is there a reason to exclude KVM_GET_DEVICE_ATTR handling ?
Suzuki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
2023-03-20 22:09 ` [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL Oliver Upton
@ 2023-03-21 15:53 ` Sean Christopherson
2023-03-21 17:36 ` Oliver Upton
0 siblings, 1 reply; 25+ messages in thread
From: Sean Christopherson @ 2023-03-21 15:53 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, kvm, Paolo Bonzini, Marc Zyngier, James Morse,
Suzuki K Poulose, Zenghui Yu, Salil Mehta
On Mon, Mar 20, 2023, Oliver Upton wrote:
> The 'longmode' field is a bit annoying as it blows an entire __u32 to
> represent a boolean value. Since other architectures are looking to add
> support for KVM_EXIT_HYPERCALL, now is probably a good time to clean it
> up.
>
> Redefine the field (and the remaining padding) as a set of flags.
> Preserve the existing ABI by using the lower 32 bits of the field to
> indicate if the guest was in long mode at the time of the hypercall.
Setting all of bits 31:0 doesn't strictly preserve the ABI, e.g. will be a
breaking change if userspace does something truly silly like
if (vcpu->run->hypercall.longmode == true)
...
It's likely unnecessary paranoia, but at the same time it's easy to avoid.
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/x86/include/uapi/asm/kvm.h | 9 +++++++++
> arch/x86/kvm/x86.c | 5 ++++-
> include/uapi/linux/kvm.h | 9 +++++++--
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 7f467fe05d42..ab7b7b1d7c9d 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -559,4 +559,13 @@ struct kvm_pmu_event_filter {
> #define KVM_VCPU_TSC_CTRL 0 /* control group for the timestamp counter (TSC) */
> #define KVM_VCPU_TSC_OFFSET 0 /* attribute for the TSC offset */
>
> +/*
> + * x86-specific KVM_EXIT_HYPERCALL flags.
> + *
> + * KVM previously used a u32 field to indicate the hypercall was initiated from
> + * long mode. As such, the lower 32 bits of the flags are used for long mode to
> + * preserve ABI.
> + */
> +#define KVM_EXIT_HYPERCALL_LONG_MODE GENMASK_ULL(31, 0)
For the uapi, I think it makes sense to do:
#define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
and then somewhere internally do:
/* Snarky comment goes here. */
#define KVM_EXIT_HYPERCALL_MBZ GENMASK_ULL(31, 1)
> #endif /* _ASM_X86_KVM_H */
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7713420abab0..c61c2b0c73bd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9803,7 +9803,10 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> vcpu->run->hypercall.args[0] = gpa;
> vcpu->run->hypercall.args[1] = npages;
> vcpu->run->hypercall.args[2] = attrs;
> - vcpu->run->hypercall.longmode = op_64_bit;
> + vcpu->run->hypercall.flags = 0;
> + if (op_64_bit)
> + vcpu->run->hypercall.flags |= KVM_EXIT_HYPERCALL_LONG_MODE;
> +
And add a runtime assertion to make sure we don't botch this in the future:
WARN_ON_ONCE(vcpu->run->hypercall.flags & KVM_EXIT_HYPERCALL_MBZ);
> vcpu->arch.complete_userspace_io = complete_hypercall_exit;
> return 0;
> }
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once
2023-03-21 9:42 ` Suzuki K Poulose
@ 2023-03-21 16:29 ` Oliver Upton
0 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-21 16:29 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: kvmarm, kvm, Paolo Bonzini, Marc Zyngier, James Morse,
Zenghui Yu, Sean Christopherson, Salil Mehta
Hi Suzuki,
On Tue, Mar 21, 2023 at 09:42:25AM +0000, Suzuki K Poulose wrote:
> Hi Oliver,
>
> On 20/03/2023 22:09, Oliver Upton wrote:
> > The test_bit(...) pattern is quite a lot of keystrokes. Replace
> > existing callsites with a helper.
> >
> > No functional change intended.
> >
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 3 +++
> > arch/arm64/kvm/pmu-emul.c | 4 ++--
>
> There is one more instance in arch/arm64/kvm/hypercalls.c at
> kvm_arm_set_fw_reg_bmap(). Is there a reason why that can't be replaced ?
Oops! Missed that one. I'll address it in v2.
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors
2023-03-21 9:53 ` Suzuki K Poulose
@ 2023-03-21 16:49 ` Oliver Upton
2023-03-28 8:39 ` Suzuki K Poulose
0 siblings, 1 reply; 25+ messages in thread
From: Oliver Upton @ 2023-03-21 16:49 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: kvmarm, kvm, Paolo Bonzini, Marc Zyngier, James Morse,
Zenghui Yu, Sean Christopherson, Salil Mehta
Hi Suzuki,
On Tue, Mar 21, 2023 at 09:53:06AM +0000, Suzuki K Poulose wrote:
> On 20/03/2023 22:09, Oliver Upton wrote:
> > A subsequent change will allow userspace to convey a filter for
> > hypercalls through a vm device attribute. Add the requisite boilerplate
> > for vm attribute accessors.
> >
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> > arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 3bd732eaf087..b6e26c0e65e5 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -1439,11 +1439,28 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
> > }
> > }
> > +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> > +{
> > + switch (attr->group) {
> > + default:
> > + return -ENXIO;
> > + }
> > +}
> > +
> > +static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> > +{
> > + switch (attr->group) {
> > + default:
> > + return -ENXIO;
> > + }
> > +}
> > +
> > long kvm_arch_vm_ioctl(struct file *filp,
> > unsigned int ioctl, unsigned long arg)
> > {
> > struct kvm *kvm = filp->private_data;
> > void __user *argp = (void __user *)arg;
> > + struct kvm_device_attr attr;
> > switch (ioctl) {
> > case KVM_CREATE_IRQCHIP: {
> > @@ -1479,6 +1496,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
> > return -EFAULT;
> > return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags);
> > }
> > + case KVM_HAS_DEVICE_ATTR: {
> > + if (copy_from_user(&attr, argp, sizeof(attr)))
> > + return -EFAULT;
> > +
> > + return kvm_vm_has_attr(kvm, &attr);
> > + }
> > + case KVM_SET_DEVICE_ATTR: {
> > + if (copy_from_user(&attr, argp, sizeof(attr)))
> > + return -EFAULT;
> > +
> > + return kvm_vm_set_attr(kvm, &attr);
> > + }
>
> Is there a reason to exclude KVM_GET_DEVICE_ATTR handling ?
The GET_DEVICE_ATTR would effectively be dead code, as the hypercall filter is
a write-only attribute. The filter is constructed through iterative calls to
the attribute, so conveying the end result to userspace w/ the same UAPI
is non-trivial.
Hopefully userspace remembers what it wrote to the field ;-)
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
2023-03-21 15:53 ` Sean Christopherson
@ 2023-03-21 17:36 ` Oliver Upton
0 siblings, 0 replies; 25+ messages in thread
From: Oliver Upton @ 2023-03-21 17:36 UTC (permalink / raw)
To: Sean Christopherson
Cc: kvmarm, kvm, Paolo Bonzini, Marc Zyngier, James Morse,
Suzuki K Poulose, Zenghui Yu, Salil Mehta
On Tue, Mar 21, 2023 at 08:53:55AM -0700, Sean Christopherson wrote:
> On Mon, Mar 20, 2023, Oliver Upton wrote:
> > The 'longmode' field is a bit annoying as it blows an entire __u32 to
> > represent a boolean value. Since other architectures are looking to add
> > support for KVM_EXIT_HYPERCALL, now is probably a good time to clean it
> > up.
> >
> > Redefine the field (and the remaining padding) as a set of flags.
> > Preserve the existing ABI by using the lower 32 bits of the field to
> > indicate if the guest was in long mode at the time of the hypercall.
>
> Setting all of bits 31:0 doesn't strictly preserve the ABI, e.g. will be a
> breaking change if userspace does something truly silly like
>
> if (vcpu->run->hypercall.longmode == true)
> ...
>
> It's likely unnecessary paranoia, but at the same time it's easy to avoid.
Argh, yeah. My route was just lazy.
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> > arch/x86/include/uapi/asm/kvm.h | 9 +++++++++
> > arch/x86/kvm/x86.c | 5 ++++-
> > include/uapi/linux/kvm.h | 9 +++++++--
> > 3 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index 7f467fe05d42..ab7b7b1d7c9d 100644
> > --- a/arch/x86/include/uapi/asm/kvm.h
> > +++ b/arch/x86/include/uapi/asm/kvm.h
> > @@ -559,4 +559,13 @@ struct kvm_pmu_event_filter {
> > #define KVM_VCPU_TSC_CTRL 0 /* control group for the timestamp counter (TSC) */
> > #define KVM_VCPU_TSC_OFFSET 0 /* attribute for the TSC offset */
> >
> > +/*
> > + * x86-specific KVM_EXIT_HYPERCALL flags.
> > + *
> > + * KVM previously used a u32 field to indicate the hypercall was initiated from
> > + * long mode. As such, the lower 32 bits of the flags are used for long mode to
> > + * preserve ABI.
> > + */
> > +#define KVM_EXIT_HYPERCALL_LONG_MODE GENMASK_ULL(31, 0)
>
> For the uapi, I think it makes sense to do:
>
> #define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0)
>
> and then somewhere internally do:
>
> /* Snarky comment goes here. */
> #define KVM_EXIT_HYPERCALL_MBZ GENMASK_ULL(31, 1)
>
> > #endif /* _ASM_X86_KVM_H */
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 7713420abab0..c61c2b0c73bd 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -9803,7 +9803,10 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > vcpu->run->hypercall.args[0] = gpa;
> > vcpu->run->hypercall.args[1] = npages;
> > vcpu->run->hypercall.args[2] = attrs;
> > - vcpu->run->hypercall.longmode = op_64_bit;
> > + vcpu->run->hypercall.flags = 0;
> > + if (op_64_bit)
> > + vcpu->run->hypercall.flags |= KVM_EXIT_HYPERCALL_LONG_MODE;
> > +
>
> And add a runtime assertion to make sure we don't botch this in the future:
>
> WARN_ON_ONCE(vcpu->run->hypercall.flags & KVM_EXIT_HYPERCALL_MBZ);
>
LGTM, I'll get something like this incorporated for v2.
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors
2023-03-21 16:49 ` Oliver Upton
@ 2023-03-28 8:39 ` Suzuki K Poulose
0 siblings, 0 replies; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-28 8:39 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, kvm, Paolo Bonzini, Marc Zyngier, James Morse,
Zenghui Yu, Sean Christopherson, Salil Mehta
On 21/03/2023 16:49, Oliver Upton wrote:
> Hi Suzuki,
>
> On Tue, Mar 21, 2023 at 09:53:06AM +0000, Suzuki K Poulose wrote:
>> On 20/03/2023 22:09, Oliver Upton wrote:
>>> A subsequent change will allow userspace to convey a filter for
>>> hypercalls through a vm device attribute. Add the requisite boilerplate
>>> for vm attribute accessors.
>>>
>>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>>> ---
>>> arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++
>>> 1 file changed, 29 insertions(+)
>>>
>>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>>> index 3bd732eaf087..b6e26c0e65e5 100644
>>> --- a/arch/arm64/kvm/arm.c
>>> +++ b/arch/arm64/kvm/arm.c
>>> @@ -1439,11 +1439,28 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
>>> }
>>> }
>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>> +{
>>> + switch (attr->group) {
>>> + default:
>>> + return -ENXIO;
>>> + }
>>> +}
>>> +
>>> +static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>> +{
>>> + switch (attr->group) {
>>> + default:
>>> + return -ENXIO;
>>> + }
>>> +}
>>> +
>>> long kvm_arch_vm_ioctl(struct file *filp,
>>> unsigned int ioctl, unsigned long arg)
>>> {
>>> struct kvm *kvm = filp->private_data;
>>> void __user *argp = (void __user *)arg;
>>> + struct kvm_device_attr attr;
>>> switch (ioctl) {
>>> case KVM_CREATE_IRQCHIP: {
>>> @@ -1479,6 +1496,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
>>> return -EFAULT;
>>> return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags);
>>> }
>>> + case KVM_HAS_DEVICE_ATTR: {
>>> + if (copy_from_user(&attr, argp, sizeof(attr)))
>>> + return -EFAULT;
>>> +
>>> + return kvm_vm_has_attr(kvm, &attr);
>>> + }
>>> + case KVM_SET_DEVICE_ATTR: {
>>> + if (copy_from_user(&attr, argp, sizeof(attr)))
>>> + return -EFAULT;
>>> +
>>> + return kvm_vm_set_attr(kvm, &attr);
>>> + }
>>
>> Is there a reason to exclude KVM_GET_DEVICE_ATTR handling ?
>
> The GET_DEVICE_ATTR would effectively be dead code, as the hypercall filter is
> a write-only attribute. The filter is constructed through iterative calls to
> the attribute, so conveying the end result to userspace w/ the same UAPI
> is non-trivial.
>
> Hopefully userspace remembers what it wrote to the field ;-)
Fair enough. I am thinking of using VM DEVICE_ATTR for some of the Arm
CCA Realm Configuration, which are now overloaded with ENABLE_CAP.
We could add GET_DEVICE_ATTR if and when we need it.
Thanks
Suzuki
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality
2023-03-20 22:09 ` [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality Oliver Upton
2023-03-21 9:52 ` Suzuki K Poulose
@ 2023-03-28 8:40 ` Suzuki K Poulose
1 sibling, 0 replies; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-28 8:40 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> KVM handles SMCCC calls from virtual EL2 that use the SMC instruction
> since commit bd36b1a9eb5a ("KVM: arm64: nv: Handle SMCs taken from
> virtual EL2"). Thus, the function name of the handler no longer reflects
> reality.
>
> Normalize the name on SMCCC, since that's the only hypercall interface
> KVM supports in the first place. No fuctional change intended.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors
2023-03-20 22:09 ` [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors Oliver Upton
2023-03-21 9:53 ` Suzuki K Poulose
@ 2023-03-28 8:40 ` Suzuki K Poulose
1 sibling, 0 replies; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-28 8:40 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> A subsequent change will allow userspace to convey a filter for
> hypercalls through a vm device attribute. Add the requisite boilerplate
> for vm attribute accessors.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1
2023-03-20 22:09 ` [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1 Oliver Upton
@ 2023-03-28 8:52 ` Suzuki K Poulose
2023-03-28 14:15 ` Marc Zyngier
0 siblings, 1 reply; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-28 8:52 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> Whelp, the architecture gods have spoken and confirmed that the function
> ID space is common between SMCs and HVCs. Not only that, the expectation
> is that hypervisors handle calls to both SMC and HVC conduits. KVM
> recently picked up support for SMCCCs in commit bd36b1a9eb5a ("KVM:
> arm64: nv: Handle SMCs taken from virtual EL2") but scoped it only to a
> nested hypervisor.
>
> Let's just open the floodgates and let EL1 access our SMCCC
> implementation with the SMC instruction as well.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
One minor observation below.
> ---
> arch/arm64/kvm/handle_exit.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 5e4f9737cbd5..68f95dcd41a1 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -72,13 +72,15 @@ static int handle_smc(struct kvm_vcpu *vcpu)
> *
> * We need to advance the PC after the trap, as it would
> * otherwise return to the same address...
> - *
> - * Only handle SMCs from the virtual EL2 with an immediate of zero and
> - * skip it otherwise.
> */
> - if (!vcpu_is_el2(vcpu) || kvm_vcpu_hvc_get_imm(vcpu)) {
> + kvm_incr_pc(vcpu);
> +
> + /*
> + * SMCs with a nonzero immediate are reserved according to DEN0028E 2.9
> + * "SMC and HVC immediate value".
> + */
> + if (kvm_vcpu_hvc_get_imm(vcpu)) {
> vcpu_set_reg(vcpu, 0, ~0UL);
> - kvm_incr_pc(vcpu);
> return 1;
> }
>
> @@ -93,8 +95,6 @@ static int handle_smc(struct kvm_vcpu *vcpu)
> if (ret < 0)
> vcpu_set_reg(vcpu, 0, ~0UL);
Nothing to do with this patch. But that check above is different
from how we handle HVC. i.e., we return back to guest for HVCs.
But for SMCs, we tend to return "ret" indicating an error (ret < 0).
Do we need to fix that ?
Suzuki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions
2023-03-20 22:09 ` [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions Oliver Upton
@ 2023-03-28 9:19 ` Suzuki K Poulose
0 siblings, 0 replies; 25+ messages in thread
From: Suzuki K Poulose @ 2023-03-28 9:19 UTC (permalink / raw)
To: Oliver Upton, kvmarm
Cc: kvm, Paolo Bonzini, Marc Zyngier, James Morse, Zenghui Yu,
Sean Christopherson, Salil Mehta
On 20/03/2023 22:09, Oliver Upton wrote:
> KVM presently allows userspace to filter guest hypercalls with bitmaps
> expressed via pseudo-firmware registers. These bitmaps have a narrow
> scope and, of course, can only allow/deny a particular call. A
> subsequent change to KVM will introduce a generalized UAPI for filtering
> hypercalls, allowing functions to be forwarded to userspace.
>
> Refactor the existing hypercall filtering logic to make room for more
> than two actions. While at it, generalize the function names around
> SMCCC as it is the basis for the upcoming UAPI.
>
> No functional change intended.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1
2023-03-28 8:52 ` Suzuki K Poulose
@ 2023-03-28 14:15 ` Marc Zyngier
0 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2023-03-28 14:15 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Oliver Upton, kvmarm, kvm, Paolo Bonzini, James Morse,
Zenghui Yu, Sean Christopherson, Salil Mehta
On 2023-03-28 09:52, Suzuki K Poulose wrote:
> On 20/03/2023 22:09, Oliver Upton wrote:
>> Whelp, the architecture gods have spoken and confirmed that the
>> function
>> ID space is common between SMCs and HVCs. Not only that, the
>> expectation
>> is that hypervisors handle calls to both SMC and HVC conduits. KVM
>> recently picked up support for SMCCCs in commit bd36b1a9eb5a ("KVM:
>> arm64: nv: Handle SMCs taken from virtual EL2") but scoped it only to
>> a
>> nested hypervisor.
>>
>> Let's just open the floodgates and let EL1 access our SMCCC
>> implementation with the SMC instruction as well.
>>
>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
> One minor observation below.
>
>> ---
>> arch/arm64/kvm/handle_exit.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/handle_exit.c
>> b/arch/arm64/kvm/handle_exit.c
>> index 5e4f9737cbd5..68f95dcd41a1 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -72,13 +72,15 @@ static int handle_smc(struct kvm_vcpu *vcpu)
>> *
>> * We need to advance the PC after the trap, as it would
>> * otherwise return to the same address...
>> - *
>> - * Only handle SMCs from the virtual EL2 with an immediate of zero
>> and
>> - * skip it otherwise.
>> */
>> - if (!vcpu_is_el2(vcpu) || kvm_vcpu_hvc_get_imm(vcpu)) {
>> + kvm_incr_pc(vcpu);
>> +
>> + /*
>> + * SMCs with a nonzero immediate are reserved according to DEN0028E
>> 2.9
>> + * "SMC and HVC immediate value".
>> + */
>> + if (kvm_vcpu_hvc_get_imm(vcpu)) {
>> vcpu_set_reg(vcpu, 0, ~0UL);
>> - kvm_incr_pc(vcpu);
>> return 1;
>> }
>> @@ -93,8 +95,6 @@ static int handle_smc(struct kvm_vcpu *vcpu)
>> if (ret < 0)
>> vcpu_set_reg(vcpu, 0, ~0UL);
>
> Nothing to do with this patch. But that check above is different
> from how we handle HVC. i.e., we return back to guest for HVCs.
> But for SMCs, we tend to return "ret" indicating an error (ret < 0).
>
> Do we need to fix that ?
I guess so. It is just that it is practically impossible to get
a negative value at the moment, but it isn't something we should
rely on.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2023-03-28 14:15 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-20 22:09 [PATCH 00/11] KVM: arm64: Userspace SMCCC call filtering Oliver Upton
2023-03-20 22:09 ` [PATCH 01/11] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL Oliver Upton
2023-03-21 15:53 ` Sean Christopherson
2023-03-21 17:36 ` Oliver Upton
2023-03-20 22:09 ` [PATCH 02/11] KVM: arm64: Add a helper to check if a VM has ran once Oliver Upton
2023-03-21 9:42 ` Suzuki K Poulose
2023-03-21 16:29 ` Oliver Upton
2023-03-20 22:09 ` [PATCH 03/11] KVM: arm64: Add vm fd device attribute accessors Oliver Upton
2023-03-21 9:53 ` Suzuki K Poulose
2023-03-21 16:49 ` Oliver Upton
2023-03-28 8:39 ` Suzuki K Poulose
2023-03-28 8:40 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 04/11] KVM: arm64: Rename SMC/HVC call handler to reflect reality Oliver Upton
2023-03-21 9:52 ` Suzuki K Poulose
2023-03-28 8:40 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 05/11] KVM: arm64: Start handling SMCs from EL1 Oliver Upton
2023-03-28 8:52 ` Suzuki K Poulose
2023-03-28 14:15 ` Marc Zyngier
2023-03-20 22:09 ` [PATCH 06/11] KVM: arm64: Refactor hvc filtering to support different actions Oliver Upton
2023-03-28 9:19 ` Suzuki K Poulose
2023-03-20 22:09 ` [PATCH 07/11] KVM: arm64: Use a maple tree to represent the SMCCC filter Oliver Upton
2023-03-20 22:09 ` [PATCH 08/11] KVM: arm64: Add support for KVM_EXIT_HYPERCALL Oliver Upton
2023-03-20 22:10 ` [PATCH 09/11] KVM: arm64: Indroduce support for userspace SMCCC filtering Oliver Upton
2023-03-20 22:10 ` [PATCH 10/11] KVM: selftests: Add a helper for SMCCC calls with SMC instruction Oliver Upton
2023-03-20 22:10 ` [PATCH 11/11] KVM: selftests: Add test for SMCCC filter Oliver Upton
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).