KVM: arm64: Userspace SMCCC call filtering
Subsystems (7)
v2 cover KVM: arm64: Userspace SMCCC call filtering
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/[email protected]/
=> 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. Note that the kvm->lock is used in this series to guard the SMCCC filter and should be converted to use kvm->arch.config_lock when that is available. Marc, you can find my conflict resolution below between these two series, and I've also pushed the result to:
git://git.kernel.org/pub/scm/linux/kernel/git/oupton/linux.git kvm-arm64/for-6.4
Presumptive, I know! :)
v1: https://lore.kernel.org/kvmarm/[email protected]/
v1 -> v2:
- Only set bit 0 for longmode, requiring the remaining lower 31 bits be 0 (Sean)
- Let errors from kvm_smccc_call_handler() reach userspace (Suzuki)
- Use the kvm_vm_has_ran_once() helper in the hypercall bitmap register handlers
- Collect Suzuki's R-bs (thanks!)
Oliver Upton (13):
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: arm64: Return NOT_SUPPORTED to guest for unknown PSCI version
KVM: arm64: Let errors from SMCCC emulation to reach userspace
KVM: selftests: Add a helper for SMCCC calls with SMC instruction
KVM: selftests: Add test for SMCCC filter
Documentation/virt/kvm/api.rst | 25 ++- Documentation/virt/kvm/devices/vm.rst | 76 +++++++ 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 | 36 ++-- arch/arm64/kvm/hypercalls.c | 156 +++++++++++++- arch/arm64/kvm/pmu-emul.c | 4 +- arch/arm64/kvm/psci.c | 7 +- arch/x86/include/asm/kvm_host.h | 7 + arch/x86/include/uapi/asm/kvm.h | 3 + arch/x86/kvm/x86.c | 6 +- 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 +++-- 18 files changed, 601 insertions(+), 63 deletions(-) create mode 100644 tools/testing/selftests/kvm/aarch64/smccc_filter.c
base-commit: e8d018dd0257f744ca50a729e3d042cf2ec9da65
arch/arm64/kvm/pmu-emul.c +0 -0
@@@ -959,8 -961,12 +959,8 @@@ int kvm_arm_pmu_v3_set_attr(struct kvm_ filter.action != KVM_PMU_EVENT_DENY))return -EINVAL ; -if (test_bit (KVM_ARCH_FLAG_HAS_RAN_ONCE , &kvm->arch.flags)) -mutex_lock (&kvm->lock); - - if (kvm_vm_has_ran_once(kvm)) { -mutex_unlock (&kvm->lock); ++ if (kvm_vm_has_ran_once(kvm)) return -EBUSY; - }if (!kvm->arch.pmu_filter) { kvm->arch.pmu_filter =bitmap_alloc (nr_events,GFP_KERNEL_ACCOUNT );
arch/arm64/kvm/hypercalls.c +2 -2
@@ -154,7 +154,7 @@ static int kvm_smccc_set_filter(struct kvm *kvm, struct kvm_smccc_filter __userif (copy_from_user (&filter, uaddr,sizeof (filter)))return -EFAULT ; -mutex_lock (&kvm->lock); +mutex_lock (&kvm->arch.config_lock);if (kvm_vm_has_ran_once (kvm)) { r = -EBUSY ;
@@ -177,7 +177,7 @@ static int kvm_smccc_set_filter(struct kvm *kvm, struct kvm_smccc_filter __userset_bit (KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED , &kvm->arch.flags); out_unlock: -mutex_unlock (&kvm->lock); +mutex_unlock (&kvm->arch.config_lock);return r; }
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 1/13 KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
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 bit 0 to indicate if the guest was in long mode and requiring that the remaining 31 bits must be zero.
--- Documentation/virt/kvm/api.rst | 3 +-- arch/x86/include/asm/kvm_host.h | 7 +++++++ arch/x86/include/uapi/asm/kvm.h | 3 +++ arch/x86/kvm/x86.c | 6 +++++- include/uapi/linux/kvm.h | 9 +++++++-- 5 files changed, 23 insertions(+), 5 deletions(-)
Documentation/virt/kvm/api.rst +1 -2
@@ -6218,8 +6218,7 @@ to the byte array. __u64 nr; __u64 args[6]; __u64 ret; - __u32 longmode; - __u32 pad; + __u64 flags; } hypercall; Unused. This was once used for 'hypercall to userspace'. To implement
arch/x86/include/asm/kvm_host.h +7 -0
@@ -2204,4 +2204,11 @@ int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages); KVM_X86_QUIRK_FIX_HYPERCALL_INSN | \ KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS) +/* +* KVM previously used a u32 field in kvm_run to indicate the hypercall was +* initiated from long mode. KVM now sets bit 0 to indicate long mode, but the +* remaining 31 lower bits must be 0 to preserve ABI. +*/ +#define KVM_EXIT_HYPERCALL_MBZ GENMASK_ULL(31, 1) +#endif /* _ASM_X86_KVM_HOST_H */
arch/x86/include/uapi/asm/kvm.h +3 -0
@@ -559,4 +559,7 @@ 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. */ +#define KVM_EXIT_HYPERCALL_LONG_MODE BIT(0) +#endif /* _ASM_X86_KVM_H */
arch/x86/kvm/x86.c +5 -1
@@ -9803,7 +9803,11 @@ 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 ; + +WARN_ON_ONCE (vcpu->run->hypercall.flags &KVM_EXIT_HYPERCALL_MBZ ); vcpu->arch.complete_userspace_io = complete_hypercall_exit;return 0 ; }
include/uapi/linux/kvm.h +7 -2
@@ -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 {
Signature · 1 line
2.40.0.348.gf938b09366-goog
Re: [PATCH v2 01/13] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
Quoted 5 lines · Oliver Upton
The 'longmode' field is a bit annoying as it blows an entire
__u32to represent a boolean value. Since other architectures are looking to add support forKVM_EXIT_HYPERCALL, now is probably a good time to clean it up.
Quoted 8 lines
Redefine the field (and the remaining padding) as a set of flags. Preserve the existing ABI by using bit 0 to indicate if the guest was in long mode and requiring that the remaining 31 bits must be zero.
Cc Paolo Bonzini <[email protected]>Cc Sean Christopherson <[email protected]>Signed-off-by Oliver Upton <[email protected]>---
Quoted 12 lines
Documentation/virt/kvm/api.rst | 3 +-- arch/x86/include/asm/kvm_host.h | 7 +++++++ arch/x86/include/uapi/asm/kvm.h | 3 +++ arch/x86/kvm/x86.c | 6 +++++- include/uapi/linux/kvm.h | 9 +++++++-- 5 files changed, 23 insertions(+), 5 deletions(-)Documentation/virt/kvm/api.rst +0
-0
Documentation/virt/kvm/api.rst to the byte array. · line 6224
Quoted 9 lines
Documentation/virt/kvm/api.rst +1
-2@@ -6218,8 +6218,7 @@ to the byte array. __u64 nr; __u64 args[6]; __u64 ret; - __u32 longmode; - __u32 pad; + __u64 flags; } hypercall; Unused. This was once used for 'hypercall to userspace'. To implement
Note to self, this needs to be updated as "hypercall" is longer unused.
PATCH v2 2/13 KVM: arm64: Add a helper to check if a VM has ran once
In reply to Oliver UptonThe test_bit(...) pattern is quite a lot of keystrokes. Replace existing callsites with a helper.
No functional change intended.
--- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/hypercalls.c | 3 +-- arch/arm64/kvm/pmu-emul.c | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -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;
arch/arm64/kvm/hypercalls.c +1 -2
@@ -379,8 +379,7 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)mutex_lock (&kvm->lock); -if (test_bit (KVM_ARCH_FLAG_HAS_RAN_ONCE , &kvm->arch.flags) && - val != *fw_reg_bmap) { +if (kvm_vm_has_ran_once (kvm) && val != *fw_reg_bmap) { ret = -EBUSY ; goto out; }
arch/arm64/kvm/pmu-emul.c +2 -2
@@ -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 ; }
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 3/13 KVM: arm64: Add vm fd device attribute accessors
In reply to Oliver UptonA subsequent change will allow userspace to convey a filter for hypercalls through a vm device attribute. Add the requisite boilerplate for vm attribute accessors.
--- arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
arch/arm64/kvm/arm.c +29 -0
@@ -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 ; }
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 4/13 KVM: arm64: Rename SMC/HVC call handler to reflect reality
In reply to Oliver UptonKVM 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.
--- 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(-)
arch/arm64/kvm/handle_exit.c +2 -2
@@ -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 );
arch/arm64/kvm/hypercalls.c +1 -1
@@ -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);
include/kvm/arm_hypercalls.h +1 -1
@@ -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) {
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 5/13 KVM: arm64: Start handling SMCs from EL1
In reply to Oliver UptonWhelp, 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.
--- arch/arm64/kvm/handle_exit.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
arch/arm64/kvm/handle_exit.c +7 -7
@@ -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; }
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 6/13 KVM: arm64: Refactor hvc filtering to support different actions
In reply to Oliver UptonKVM 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.
--- arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++ arch/arm64/kvm/hypercalls.c | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-)
arch/arm64/include/uapi/asm/kvm.h +9 -0
@@ -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__ */
arch/arm64/kvm/hypercalls.c +15 -4
@@ -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) {
Signature · 1 line
2.40.0.348.gf938b09366-goog
Marc Zyngier
maintainerRe: [PATCH v2 06/13] KVM: arm64: Refactor hvc filtering to support different actions
Quoted 27 lines · 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.
Reviewed-by Suzuki K Poulose <[email protected]>Signed-off-by Oliver Upton <[email protected]>--- arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++ arch/arm64/kvm/hypercalls.c | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-)arch/arm64/include/uapi/asm/kvm.h +0
-0@@ -469,6 +469,15 @@ enum {/* run->fail_entry.hardware_entry_failure_reason codes. */ #define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED (1ULL << 0)
arch/arm64/include/uapi/asm/kvm.h enum { · line 480
Quoted 10 lines
arch/arm64/include/uapi/asm/kvm.h +9
-0+enum kvm_smccc_filter_action { +KVM_SMCCC_FILTER_ALLOW =0 , +KVM_SMCCC_FILTER_DENY , + +#ifdef __KERNEL__ +NR_SMCCC_FILTER_ACTIONS +#endif +}; +
One thing I find myself wondering is what "ALLOW" mean here: Allow the handling of the hypercall? Or allow its forwarding? My guess is that this is the former, but I'd love a comment to clarify it, or even a clearer name ("HANDLE" instead of "ALLOW", for example, but YMMV...).
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH v2 06/13] KVM: arm64: Refactor hvc filtering to support different actions
Quoted 30 lines · Marc Zyngier
Quoted 28 lines · 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.
Reviewed-by Suzuki K Poulose <[email protected]>Signed-off-by Oliver Upton <[email protected]>--- arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++ arch/arm64/kvm/hypercalls.c | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-)arch/arm64/include/uapi/asm/kvm.h +0
-0@@ -469,6 +469,15 @@ enum {/* run->fail_entry.hardware_entry_failure_reason codes. */ #define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED (1ULL << 0)
Quoted 14 lines
arch/arm64/include/uapi/asm/kvm.h enum { · line 480
Quoted 9 lines
arch/arm64/include/uapi/asm/kvm.h +9
-0+enum kvm_smccc_filter_action { +KVM_SMCCC_FILTER_ALLOW =0 , +KVM_SMCCC_FILTER_DENY , + +#ifdef __KERNEL__ +NR_SMCCC_FILTER_ACTIONS +#endif +}; +One thing I find myself wondering is what "ALLOW" mean here: Allow the handling of the hypercall? Or allow its forwarding? My guess is that this is the former, but I'd love a comment to clarify it, or even a clearer name ("HANDLE" instead of "ALLOW", for example, but YMMV...).
Yeah, I prefer calling it HANDLE as you suggest.
Signature · 2 lines
Thanks, Oliver
PATCH v2 7/13 KVM: arm64: Use a maple tree to represent the SMCCC filter
In reply to Oliver UptonMaple 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.
--- 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(-)
arch/arm64/include/asm/kvm_host.h +4 -1
@@ -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 6unsigned 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
arch/arm64/kvm/arm.c +2 -0
@@ -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)
arch/arm64/kvm/hypercalls.c +57 -0
71 lines not on this page · raw message
include/kvm/arm_hypercalls.h +1 -0
@@ -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);
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 8/13 KVM: arm64: Add support for KVM_EXIT_HYPERCALL
In reply to Oliver UptonIn 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).
--- Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-)
Documentation/virt/kvm/api.rst +20 -2
@@ -6221,11 +6221,29 @@ to the byte array. __u64 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 */
arch/arm64/include/uapi/asm/kvm.h +4 -0
@@ -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__ */
arch/arm64/kvm/handle_exit.c +3 -1
@@ -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);
arch/arm64/kvm/hypercalls.c +17 -0
@@ -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 :
Signature · 1 line
2.40.0.348.gf938b09366-goog
Marc Zyngier
maintainerRe: [PATCH v2 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 35 lines · 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 <[email protected]>--- Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-)Documentation/virt/kvm/api.rst +3
-2@@ -6221,11 +6221,29 @@ to the byte array. __u64 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
Documentation/virt/kvm/api.rst to the byte array. · line 6236
Quoted 10 lines
Documentation/virt/kvm/api.rst +7
-0+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.
Maybe this hunk should come with the following patch which actually adds that doc.
Quoted 74 lines
Documentation/virt/kvm/api.rst +10
-0+ +``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 */arch/arm64/include/uapi/asm/kvm.h +4
-0@@ -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__ */ arch/arm64/kvm/handle_exit.c +3
-1@@ -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);arch/arm64/kvm/hypercalls.c +13
-0@@ -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;
arch/arm64/kvm/hypercalls.c int kvm_smccc_call_handler(struct kvm_vcpu *vcpu) · line 211
Quoted 7 lines
arch/arm64/kvm/hypercalls.c +4
-0@@ -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 ; + }
nit: maybe write this as a switch statement?
arch/arm64/kvm/hypercalls.c int kvm_smccc_call_handler(struct kvm_vcpu *vcpu) · line 214
Quoted 3 lines
arch/arm64/kvm/hypercalls.c +0
-0switch (func_id) {case ARM_SMCCC_VERSION_FUNC_ID :
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH v2 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 41 lines · Marc Zyngier
Quoted 39 lines · 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 <[email protected]>--- Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-)Documentation/virt/kvm/api.rst +4
-2@@ -6221,11 +6221,29 @@ to the byte array. __u64 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.
Quoted 9 lines
Documentation/virt/kvm/api.rst to the byte array. · line 6236
Quoted 6 lines
Documentation/virt/kvm/api.rst +6
-0+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.Maybe this hunk should come with the following patch which actually adds that doc.
Heh, you caught me being lazy :) Can do.
Quoted 26 lines
Quoted 26 lines
arch/arm64/kvm/hypercalls.c +13
-0@@ -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);
Quoted 8 lines
arch/arm64/kvm/hypercalls.c int kvm_smccc_call_handler(struct kvm_vcpu *vcpu) · line 211
Quoted 6 lines
arch/arm64/kvm/hypercalls.c +4
-0if (action ==KVM_SMCCC_FILTER_DENY ) goto out; +if (action ==KVM_SMCCC_FILTER_FWD_TO_USER ) { +kvm_prepare_hypercall_exit (vcpu, func_id); +return 0 ; + }nit: maybe write this as a switch statement?
Sure thing. I'll get a new spin on the list sometime in the next day or two that addresses your feedback. Appreciate the review
Signature · 2 lines
Thanks, Oliver
PATCH v2 9/13 KVM: arm64: Indroduce support for userspace SMCCC filtering
In reply to Oliver UptonAs 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.
--- Documentation/virt/kvm/devices/vm.rst | 76 +++++++++++++++++++++++++++ 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, 152 insertions(+)
Documentation/virt/kvm/devices/vm.rst +76 -0
@@ -321,3 +321,79 @@ 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. The SMCCC filter gates the in-kernel emulation of SMCCC calls +and as such takes effect before other interfaces that interact with +SMCCC calls (e.g. hypercall bitmap registers). + +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 + =========== ===============
arch/arm64/include/uapi/asm/kvm.h +11 -0
@@ -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)
arch/arm64/kvm/arm.c +4 -0
@@ -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 ; }
arch/arm64/kvm/hypercalls.c +58 -0
67 lines not on this page · raw message
include/kvm/arm_hypercalls.h +3 -0
@@ -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
Signature · 1 line
2.40.0.348.gf938b09366-goog
Marc Zyngier
maintainerRe: [PATCH v2 09/13] KVM: arm64: Indroduce support for userspace SMCCC filtering
On Thu, 30 Mar 2023 16:49:14 +0100,
Oliver Upton <[email protected]> wrote:
nit: typo in the subject. Also, this isn't so much about filtering, but forwarding.
Quoted 56 lines
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 <[email protected]>--- Documentation/virt/kvm/devices/vm.rst | 76 +++++++++++++++++++++++++++ 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, 152 insertions(+)Documentation/virt/kvm/devices/vm.rst +26
-0@@ -321,3 +321,79 @@ 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 {
Documentation/virt/kvm/devices/vm.rst Allows userspace to query the status of migration mode. · line 359
Quoted 10 lines
Documentation/virt/kvm/devices/vm.rst +10
-0+ 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];
If I could add one word (pun intended) here, I'd add an extra 64bit of padding, just in case we end-up needing to pass an address, or something else.
Quoted 30 lines
Documentation/virt/kvm/devices/vm.rst +30
-0+ }; + +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. The SMCCC filter gates the in-kernel emulation of SMCCC calls +and as such takes effect before other interfaces that interact with +SMCCC calls (e.g. hypercall bitmap registers). + +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``.
Documentation/virt/kvm/devices/vm.rst Allows userspace to query the status of migration mode. · line 399
Quoted 10 lines
Documentation/virt/kvm/devices/vm.rst +10
-0+ +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 + =========== ===============
You also may want to call out that all padding fields must be set to 0.
Quoted 69 lines
arch/arm64/include/uapi/asm/kvm.h +11
-0@@ -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)arch/arm64/kvm/arm.c +4
-0@@ -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 ; }arch/arm64/kvm/hypercalls.c +10
-0@@ -145,6 +145,42 @@ static void init_smccc_filter(struct kvm *kvm)WARN_ON_ONCE (r); } +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);
arch/arm64/kvm/hypercalls.c static void init_smccc_filter(struct kvm *kvm) · line 167
Quoted 10 lines
arch/arm64/kvm/hypercalls.c +10
-0+ +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; + }
Also check that the padding fields are set to 0 until we have a better use for them.
Quoted 6 lines
arch/arm64/kvm/hypercalls.c +6
-0+ + 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 );
arch/arm64/kvm/hypercalls.c static void init_smccc_filter(struct kvm *kvm) · line 183
Quoted 10 lines
arch/arm64/kvm/hypercalls.c +10
-0+if (r) + goto out_unlock; + +set_bit (KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED , &kvm->arch.flags); + +out_unlock: +mutex_unlock (&kvm->lock); +return r; +} +
Just a though: it'd be interesting to have a debugfs file dumping the content of the maple tree, if only to figure out what it going on when doing some userspace hacking. Not something that's needed right now though.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH v2 09/13] KVM: arm64: Indroduce support for userspace SMCCC filtering
Quoted 5 lines · Marc Zyngier
On Thu, 30 Mar 2023 16:49:14 +0100,
Oliver Upton <[email protected]> wrote:nit: typo in the subject. Also, this isn't so much about filtering, but forwarding.
Oops, thanks for catching the typo. If you don't mind, I'd prefer to retain the term 'filtering', as this patch provides UAPI for _both_ forwarding and outright denying SMCCC calls.
Ack to the rest of your feedback here, I'll have it addressed in the next spin of the series.
Signature · 2 lines
Thanks, Oliver
PATCH v2 10/13 KVM: arm64: Return NOT_SUPPORTED to guest for unknown PSCI version
In reply to Oliver UptonA subsequent change to KVM will allow negative returns from SMCCC handlers to exit to userspace. Make way for this change by explicitly returning SMCCC_RET_NOT_SUPPORTED to the guest if the VM is configured to use an unknown PSCI version. Add a WARN since this is undoubtedly a KVM bug.
--- arch/arm64/kvm/psci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
arch/arm64/kvm/psci.c +5 -2
@@ -435,6 +435,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)int kvm_psci_call (struct kvm_vcpu *vcpu) {u32 psci_fn =smccc_get_function (vcpu); +int version =kvm_psci_version (vcpu);unsigned long val; val =kvm_psci_check_allowed_function (vcpu, psci_fn);
@@ -443,7 +444,7 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)return 1 ; } -switch (kvm_psci_version (vcpu)) { +switch (version) {case KVM_ARM_PSCI_1_1 :return kvm_psci_1_x_call (vcpu,1 );case KVM_ARM_PSCI_1_0 :
@@ -453,6 +454,8 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)case KVM_ARM_PSCI_0_1 :return kvm_psci_0_1_call (vcpu);default : -return -EINVAL ; +WARN_ONCE (1 ,"Unknown PSCI version %d" , version); +smccc_set_retval (vcpu,SMCCC_RET_NOT_SUPPORTED ,0 ,0 ,0 ); +return 1 ; } }
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 11/13 KVM: arm64: Let errors from SMCCC emulation to reach userspace
In reply to Oliver UptonTypically a negative return from an exit handler is used to request a return to userspace with the specified error. KVM's handling of SMCCC emulation (i.e. both HVCs and SMCs) deviates from the trend and resumes the guest instead.
Stop handling negative returns this way and instead let the error percolate to userspace.
--- arch/arm64/kvm/handle_exit.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
arch/arm64/kvm/handle_exit.c +2 -16
@@ -36,8 +36,6 @@ static void kvm_handle_guest_serror(struct kvm_vcpu *vcpu, u64 esr)static int handle_hvc (struct kvm_vcpu *vcpu) { -int ret; -trace_kvm_hvc_arm64 (*vcpu_pc (vcpu),vcpu_get_reg (vcpu,0 ),kvm_vcpu_hvc_get_imm (vcpu)); vcpu->stat.hvc_exit_stat++;
@@ -52,19 +50,11 @@ static int handle_hvc(struct kvm_vcpu *vcpu)return 1 ; } - ret =kvm_smccc_call_handler (vcpu); -if (ret <0 ) { -vcpu_set_reg (vcpu,0 , ~0UL ); -return 1 ; - } - -return ret; +return kvm_smccc_call_handler (vcpu); }static int handle_smc (struct kvm_vcpu *vcpu) { -int ret; -/* * "If an SMC instruction executed at Non-secure EL1 is * trapped to EL2 because HCR_EL2.TSC is 1, the exception is a
@@ -93,11 +83,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_smccc_call_handler (vcpu); -if (ret <0 ) -vcpu_set_reg (vcpu,0 , ~0UL ); - -return ret; +return kvm_smccc_call_handler (vcpu); }/*
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 12/13 KVM: selftests: Add a helper for SMCCC calls with SMC instruction
In reply to Oliver UptonBuild a helper for doing SMCs in selftests by macro-izing the current HVC implementation and taking the conduit instruction as an argument.
--- .../selftests/kvm/include/aarch64/processor.h | 13 +++++ .../selftests/kvm/lib/aarch64/processor.c | 52 ++++++++++++------- 2 files changed, 46 insertions(+), 19 deletions(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +13 -0
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/processor.c +33 -19
62 lines not on this page · raw message
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v2 13/13 KVM: selftests: Add test for SMCCC filter
In reply to Oliver UptonAdd 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.
--- 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
tools/testing/selftests/kvm/Makefile +1 -0
@@ -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
tools/testing/selftests/kvm/aarch64/smccc_filter.c new file +196 -0
196 lines not on this page · raw message
Signature · 1 line
2.40.0.348.gf938b09366-goog