KVM: arm64: Userspace SMCCC call filtering
Subsystems (7)
v3 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]/
v2: https://lore.kernel.org/kvmarm/[email protected]/
v2 -> v3:
- Collect Sean's Ack (thanks!)
- s/ALLOW/HANDLE/g (Marc)
- Test that the padding is zero (Marc)
- Doc fixes, typos, etc. (Marc)
- Test that the filter range has not overflowed
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: Introduce 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 | 79 ++++++ 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 | 164 ++++++++++- 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 | 260 ++++++++++++++++++ .../selftests/kvm/include/aarch64/processor.h | 13 + .../selftests/kvm/lib/aarch64/processor.c | 52 ++-- 18 files changed, 676 insertions(+), 63 deletions(-) create mode 100644 tools/testing/selftests/kvm/aarch64/smccc_filter.c
base-commit: e8d018dd0257f744ca50a729e3d042cf2ec9da65
arch/arm64/include/asm/kvm_host.h +0 -0
@@@ -224,11 -222,8 +225,12 @@@ 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 + /* VM counter offset */ +#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6 + /* Timer PPIs made immutable */ +#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 7 - +/* SMCCC filter initialized for the VM */ -#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED 6 ++#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED 8unsigned long flags;/*
arch/arm64/kvm/arm.c +0 -0
@@@ -1513,13 -1502,18 +1536,25 @@@ long kvm_arch_vm_ioctl(struct file *filreturn -EFAULT ;return kvm_vm_ioctl_mte_copy_tags (kvm, ©_tags); } + case KVM_ARM_SET_COUNTER_OFFSET: { + struct kvm_arm_counter_offset offset; + + if (copy_from_user(&offset, argp, sizeof(offset))) + return -EFAULT ; + return kvm_vm_ioctl_set_counter_offset(kvm, &offset); + } +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 ; }
arch/arm64/kvm/pmu-emul.c +0 -0
@@@ -958,8 -961,12 +958,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
@@ -164,7 +164,7 @@ static int kvm_smccc_set_filter(struct kvm *kvm, struct kvm_smccc_filter __userif (end < start || filter.action >=NR_SMCCC_FILTER_ACTIONS )return -EINVAL ; -mutex_lock (&kvm->lock); +mutex_lock (&kvm->arch.config_lock);if (kvm_vm_has_ran_once (kvm)) { r = -EBUSY ;
@@ -179,7 +179,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 v3 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
PATCH v3 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 v3 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 v3 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 v3 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 v3 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 | 26 ++++++++++++++++++++++---- 2 files changed, 31 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_HANDLE =0 , +KVM_SMCCC_FILTER_DENY , + +#ifdef __KERNEL__ +NR_SMCCC_FILTER_ACTIONS +#endif +}; +#endif #endif /* __ARM_KVM_H__ */
arch/arm64/kvm/hypercalls.c +22 -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,20 +117,38 @@ 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_HANDLE ; + +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); +switch (action) { +case KVM_SMCCC_FILTER_HANDLE : +break ; +case KVM_SMCCC_FILTER_DENY : + goto out; +default : +WARN_RATELIMIT (1 ,"Unhandled SMCCC filter action: %d\n" , action); goto out; + }switch (func_id) {case ARM_SMCCC_VERSION_FUNC_ID :
Signature · 1 line
2.40.0.348.gf938b09366-goog
PATCH v3 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 v3 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 | 18 ++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)
Documentation/virt/kvm/api.rst +16 -2
@@ -6221,11 +6221,25 @@ 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: +---------- + +``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_HANDLE =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 +16 -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;
@@ -195,6 +208,9 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)break ;case KVM_SMCCC_FILTER_DENY : goto out; +case KVM_SMCCC_FILTER_FWD_TO_USER : +kvm_prepare_hypercall_exit (vcpu, func_id); +return 0 ;default :WARN_RATELIMIT (1 ,"Unhandled SMCCC filter action: %d\n" , action); goto out;
Signature · 1 line
2.40.0.348.gf938b09366-goog
Marc Zyngier
maintainerRe: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 80 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 | 18 ++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)Documentation/virt/kvm/api.rst +16
-2@@ -6221,11 +6221,25 @@ 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: +---------- + +``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_HANDLE =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 +0
-0
arch/arm64/kvm/handle_exit.c static int handle_smc(struct kvm_vcpu *vcpu) · line 76
Quoted 7 lines
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.
nit: this comment really needs to find its way in the documentation so that a VMM author can determine the PC of the SMC/HVC. This is specially important for 32bit, which has a 16bit encodings for SMC/HVC.
And thinking of it, this outlines a small flaw in this API. If luserspace needs to find out about the address of the HVC/SMC, it needs to know the *size* of the instruction. But we don't propagate the ESR value. I think this still works by construction (userspace can check PSTATE and work out whether we're in ARM or Thumb mode), but this feels fragile.
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 103 lines · Marc Zyngier
Quoted 80 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 | 18 ++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)Documentation/virt/kvm/api.rst +16
-2@@ -6221,11 +6221,25 @@ 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: +---------- + +``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_HANDLE =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 +0
-0arch/arm64/kvm/handle_exit.c static int handle_smc(struct kvm_vcpu *vcpu) · line 76
Quoted 7 lines
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. nit: this comment really needs to find its way in the documentation so that a VMM author can determine the PC of the SMC/HVC. This is specially important for 32bit, which has a 16bit encodings for SMC/HVC.
And thinking of it, this outlines a small flaw in this API. If luserspace needs to find out about the address of the HVC/SMC, it needs to know the *size* of the instruction. But we don't propagate the ESR value. I think this still works by construction (userspace can check PSTATE and work out whether we're in ARM or Thumb mode), but this feels fragile.
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
Just to make this a quicker round trip, I hacked the following together. If you agree with it, I'll stick it on top and get the ball rolling.
Thanks,
M.
From 9b830e7a3819c2771074bebe66c1d5f20394e3cc Mon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Wed, 5 Apr 2023 12:48:58 +0100
Subject: [PATCH] KVM: arm64: Expose SMC/HVC width to userspace
When returning to userspace to handle a SMCCC call, we consistently set PC to point to the instruction immediately after the HVC/SMC.
However, should userspace need to know the exact address of the trapping instruction, it needs to know about the *size* of that instruction. For AArch64, this is pretty easy. For AArch32, this is a bit more funky, as Thumb has 16bit encodings for both HVC and SMC.
Expose this to userspace with a new flag that directly derives from ESR_EL2.IL. Also update the documentation to reflect the PC state at the point of exit.
Finally, this fixes a small buglet where the hypercall.{args,ret} fields would not be cleared on exit, and could contain some random junk.
--- Documentation/virt/kvm/api.rst | 8 ++++++++ arch/arm64/include/uapi/asm/kvm.h | 3 ++- arch/arm64/kvm/hypercalls.c | 16 +++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-)
Documentation/virt/kvm/api.rst +8 -0
@@ -6244,6 +6244,14 @@ Definition of ``flags``: conduit to initiate the SMCCC call. If this bit is 0 then the guest used the HVC conduit for the SMCCC call. + - ``KVM_HYPERCALL_EXIT_16BIT``: Indicates that the guest used a 16bit + instruction to initiate the SMCCC call. If this bit is 0 then the + guest used a 32bit instruction. An AArch64 guest always has this + bit set to 0. + +At the point of exit, PC points to the instruction immediately following +the trapping instruction. + :: /* KVM_EXIT_TPR_ACCESS */
arch/arm64/include/uapi/asm/kvm.h +2 -1
@@ -491,7 +491,8 @@ struct kvm_smccc_filter { };/* arm64-specific KVM_EXIT_HYPERCALL flags */ -#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_16BIT (1U << 1)#endif
arch/arm64/kvm/hypercalls.c +11 -5
@@ -222,13 +222,19 @@ 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 ; +u64 flags =0 ;if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64) - run->hypercall.flags |=KVM_HYPERCALL_EXIT_SMC ; + flags |=KVM_HYPERCALL_EXIT_SMC ; + +if (!kvm_vcpu_trap_il_is32bit (vcpu)) + flags |=KVM_HYPERCALL_EXIT_16BIT ; + + run->exit_reason =KVM_EXIT_HYPERCALL ; + run->hypercall = (typeof(run->hypercall)) { + .nr = func_id, + .flags = flags, + }; }int kvm_smccc_call_handler (struct kvm_vcpu *vcpu)
Signature · 5 lines
2.34.1 -- Without deviation from the norm, progress is not possible.
Oliver Upton
Reviewed-byRe: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Hey Marc,
On Wed, Apr 05, 2023 at 12:59:20PM +0100, Marc Zyngier wrote:
[...]
Quoted 25 lines
Quoted 25 lines
arch/arm64/kvm/handle_exit.c static int handle_smc(struct kvm_vcpu *vcpu) · line 76
Quoted 12 lines
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. nit: this comment really needs to find its way in the documentation so that a VMM author can determine the PC of the SMC/HVC. This is specially important for 32bit, which has a 16bit encodings for SMC/HVC.
And thinking of it, this outlines a small flaw in this API. If luserspace needs to find out about the address of the HVC/SMC, it needs to know the *size* of the instruction. But we don't propagate the ESR value. I think this still works by construction (userspace can check PSTATE and work out whether we're in ARM or Thumb mode), but this feels fragile.
Quoted 6 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
Just to make this a quicker round trip, I hacked the following together. If you agree with it, I'll stick it on top and get the ball rolling.
Less work for me? How could I say no :)
Quoted 18 lines
From
9b830e7a3819c2771074bebe66c1d5f20394e3ccMon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Wed, 5 Apr 2023 12:48:58 +0100
Subject: [PATCH] KVM: arm64: Expose SMC/HVC width to userspaceWhen returning to userspace to handle a SMCCC call, we consistently set PC to point to the instruction immediately after the HVC/SMC.
However, should userspace need to know the exact address of the trapping instruction, it needs to know about the *size* of that instruction. For AArch64, this is pretty easy. For AArch32, this is a bit more funky, as Thumb has 16bit encodings for both HVC and SMC.
Expose this to userspace with a new flag that directly derives from ESR_EL2.IL. Also update the documentation to reflect the PC state at the point of exit.
Quoted 5 lines
Finally, this fixes a small buglet where the hypercall.{args,ret} fields would not be cleared on exit, and could contain some random junk.
Signed-off-by Marc Zyngier <[email protected]>
Quoted 74 lines
--- Documentation/virt/kvm/api.rst | 8 ++++++++ arch/arm64/include/uapi/asm/kvm.h | 3 ++- arch/arm64/kvm/hypercalls.c | 16 +++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-)Documentation/virt/kvm/api.rst +8
-0@@ -6244,6 +6244,14 @@ Definition of ``flags``: conduit to initiate the SMCCC call. If this bit is 0 then the guest used the HVC conduit for the SMCCC call. + - ``KVM_HYPERCALL_EXIT_16BIT``: Indicates that the guest used a 16bit + instruction to initiate the SMCCC call. If this bit is 0 then the + guest used a 32bit instruction. An AArch64 guest always has this + bit set to 0. + +At the point of exit, PC points to the instruction immediately following +the trapping instruction. + :: /* KVM_EXIT_TPR_ACCESS */arch/arm64/include/uapi/asm/kvm.h +2
-1@@ -491,7 +491,8 @@ struct kvm_smccc_filter { };/* arm64-specific KVM_EXIT_HYPERCALL flags */ -#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_16BIT (1U << 1)#endif arch/arm64/kvm/hypercalls.c +11
-5@@ -222,13 +222,19 @@ 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 ; +u64 flags =0 ;if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64) - run->hypercall.flags |=KVM_HYPERCALL_EXIT_SMC ; + flags |=KVM_HYPERCALL_EXIT_SMC ; + +if (!kvm_vcpu_trap_il_is32bit (vcpu)) + flags |=KVM_HYPERCALL_EXIT_16BIT ; + + run->exit_reason =KVM_EXIT_HYPERCALL ; + run->hypercall = (typeof(run->hypercall)) { + .nr = func_id, + .flags = flags, + }; }int kvm_smccc_call_handler (struct kvm_vcpu *vcpu)Signature · 5 lines
2.34.1 -- Without deviation from the norm, progress is not possible.
Signature · 2 lines
Thanks, Oliver
RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
In reply to Marc ZyngierHi Marc,
Sorry for jumping late. I am updating the Qemu code for the VCPU hotplug to reflect Oliver's SMCCC filtering changes and I have few doubts.
Please scroll below.
Quoted 10 lines
From: Marc Zyngier <[email protected]>
Sent: Wednesday, April 5, 2023 12:59 PM
To: Oliver Upton <[email protected]>
Cc: [email protected]; [email protected]; Paolo Bonzini <[email protected]>; James Morse <[email protected]>; Suzuki K Poulose <[email protected]>; yuzenghui <[email protected]>; Sean Christopherson <[email protected]>; Salil Mehta <[email protected]>
Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support forKVM_EXIT_HYPERCALL
Quoted 107 lines
Quoted 105 lines · Marc Zyngier
Quoted 80 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 | 18 ++++++++++++++++-- arch/arm64/include/uapi/asm/kvm.h | 4 ++++ arch/arm64/kvm/handle_exit.c | 4 +++- arch/arm64/kvm/hypercalls.c | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)Documentation/virt/kvm/api.rst +16
-2@@ -6221,11 +6221,25 @@ 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: +---------- + +``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_HANDLE =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 +0
-0arch/arm64/kvm/handle_exit.c static int handle_smc(struct kvm_vcpu *vcpu) · line 76
Quoted 7 lines
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. nit: this comment really needs to find its way in the documentation so that a VMM author can determine the PC of the SMC/HVC. This is specially important for 32bit, which has a 16bit encodings for SMC/HVC.
And thinking of it, this outlines a small flaw in this API. If luserspace needs to find out about the address of the HVC/SMC, it needs to know the *size* of the instruction. But we don't propagate the ESR value. I think this still works by construction (userspace can check PSTATE and work out whether we're in ARM or Thumb mode), but this feels fragile.
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the ESR_EL2 register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?
Please correct me if I am missing anything here.
Quoted 87 lines
Just to make this a quicker round trip, I hacked the following together. If you agree with it, I'll stick it on top and get the ball rolling.
Thanks,
M.From
9b830e7a3819c2771074bebe66c1d5f20394e3ccMon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Wed, 5 Apr 2023 12:48:58 +0100
Subject: [PATCH] KVM: arm64: Expose SMC/HVC width to userspaceWhen returning to userspace to handle a SMCCC call, we consistently set PC to point to the instruction immediately after the HVC/SMC.
However, should userspace need to know the exact address of the trapping instruction, it needs to know about the *size* of that instruction. For AArch64, this is pretty easy. For AArch32, this is a bit more funky, as Thumb has 16bit encodings for both HVC and SMC.
Expose this to userspace with a new flag that directly derives from ESR_EL2.IL. Also update the documentation to reflect the PC state at the point of exit.
Finally, this fixes a small buglet where the hypercall.{args,ret} fields would not be cleared on exit, and could contain some random junk.
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/virt/kvm/api.rst | 8 ++++++++ arch/arm64/include/uapi/asm/kvm.h | 3 ++- arch/arm64/kvm/hypercalls.c | 16 +++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-)Documentation/virt/kvm/api.rst +8
-0@@ -6244,6 +6244,14 @@ Definition of ``flags``: conduit to initiate the SMCCC call. If this bit is 0 then the guest used the HVC conduit for the SMCCC call. + - ``KVM_HYPERCALL_EXIT_16BIT``: Indicates that the guest used a 16bit + instruction to initiate the SMCCC call. If this bit is 0 then the + guest used a 32bit instruction. An AArch64 guest always has this + bit set to 0. + +At the point of exit, PC points to the instruction immediately following +the trapping instruction. + :: /* KVM_EXIT_TPR_ACCESS */arch/arm64/include/uapi/asm/kvm.h +2
-1@@ -491,7 +491,8 @@ struct kvm_smccc_filter { };/* arm64-specific KVM_EXIT_HYPERCALL flags */ -#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_SMC (1U << 0) +#define KVM_HYPERCALL_EXIT_16BIT (1U << 1)#endif arch/arm64/kvm/hypercalls.c +1
-4@@ -222,13 +222,19 @@ 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 ; +u64 flags =0 ;
arch/arm64/kvm/hypercalls.c static void kvm_prepare_hypercall_exit(struct · line 236
Quoted 10 lines
arch/arm64/kvm/hypercalls.c +8
-1if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64) - run->hypercall.flags |=KVM_HYPERCALL_EXIT_SMC ; + flags |=KVM_HYPERCALL_EXIT_SMC ; + +if (!kvm_vcpu_trap_il_is32bit (vcpu)) + flags |=KVM_HYPERCALL_EXIT_16BIT ; + + run->exit_reason =KVM_EXIT_HYPERCALL ; + run->hypercall = (typeof(run->hypercall)) { + .nr = func_id,
Earlier this was the place holder for the "immediate value"
Best regards
Salil
Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Hi Salil,
On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
[...]
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?
The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Is there a legitimate use case for hypercalls with a nonzero immediate? They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 15 lines · Oliver Upton
Hi Salil,
On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
[...]
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?
Quoted 7 lines
The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Is there a legitimate use case for hypercalls with a nonzero immediate? They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
I agree. The use of non-zero immediate has long been deprecated. I guess we should actually reject non-zero immediate for HVC just like we do for SMC.
If there is an actual need for a non-zero immediate to be propagated to userspace (want to emulate Xen's infamous 'HVC #0xEA1'?), then this should be an extension to the current API.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Hi Marc,
Quoted 31 lines
From: Marc Zyngier <[email protected]>
Sent: Thursday, May 18, 2023 9:06 AM
To: Oliver Upton <[email protected]>
Cc: Salil Mehta <[email protected]>; [email protected]; [email protected]; Paolo Bonzini <[email protected]>; James Morse <[email protected]>; Suzuki K Poulose <[email protected]>; yuzenghui <[email protected]>; Sean Christopherson <[email protected]>
Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support forKVM_EXIT_HYPERCALLQuoted 19 lines · Oliver Upton
Hi Salil,
On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
[...]
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Quoted 7 lines
Quoted 3 lines
Is there a legitimate use case for hypercalls with a nonzero immediate? They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
I agree. The use of non-zero immediate has long been deprecated. I guess we should actually reject non-zero immediate for HVC just like we do for SMC.
Ok. Maybe I will hard code Immediate value as 0 to create a syndrome value at the VMM/Qemu and will also put a note stating non-zero immediate for HVC/SVC are not supported/deprecated.
Quoted 3 lines
If there is an actual need for a non-zero immediate to be propagated to userspace (want to emulate Xen's infamous 'HVC #0xEA1'?), then this should be an extension to the current API.
Oh ok, then perhaps this new extension change should be simultaneously committed to avoid breaking Xen?
Thanks
Salil
Marc Zyngier
maintainerRe: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Quoted 38 lines · Salil Mehta
Hi Marc,
Quoted 35 lines
From: Marc Zyngier <[email protected]>
Sent: Thursday, May 18, 2023 9:06 AM
To: Oliver Upton <[email protected]>
Cc: Salil Mehta <[email protected]>; [email protected]; [email protected]; Paolo Bonzini <[email protected]>; James Morse <[email protected]>; Suzuki K Poulose <[email protected]>; yuzenghui <[email protected]>; Sean Christopherson <[email protected]>
Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support forKVM_EXIT_HYPERCALLQuoted 22 lines · Oliver Upton
Hi Salil,
On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
[...]
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Is there a legitimate use case for hypercalls with a nonzero immediate? They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
Quoted 8 lines
Quoted 3 lines
I agree. The use of non-zero immediate has long been deprecated. I guess we should actually reject non-zero immediate for HVC just like we do for SMC.
Ok. Maybe I will hard code Immediate value as 0 to create a syndrome value at the VMM/Qemu and will also put a note stating non-zero immediate for HVC/SVC are not supported/deprecated.
Yes, because this should be the only situation where you should see such an exit to userspace.
Quoted 6 lines
Quoted 3 lines
If there is an actual need for a non-zero immediate to be propagated to userspace (want to emulate Xen's infamous 'HVC #0xEA1'?), then this should be an extension to the current API.
Oh ok, then perhaps this new extension change should be simultaneously committed to avoid breaking Xen?
How would that break Xen? I don't have any plan to emulate Xen in any shape or form, and I don't think anyone want to do that in userspace either.
I really want to see an actual use case to expand this stuff. Because so far, we follow the strict SMCCC spec, and nothing else. But if we admit deviations, do we also have to expose SMC and HVC as different instructions?
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
Hi Marc,
Quoted 10 lines
From: Marc Zyngier <[email protected]>
Sent: Thursday, May 18, 2023 10:43 AM
To: Salil Mehta <[email protected]>
Cc: Oliver Upton <[email protected]>; [email protected]; [email protected]; Paolo Bonzini <[email protected]>; James Morse <[email protected]>; Suzuki K Poulose <[email protected]>; yuzenghui <[email protected]>; Sean Christopherson <[email protected]>
Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support forKVM_EXIT_HYPERCALL
[...]
Quoted 32 lines
Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Is there a legitimate use case for hypercalls with a nonzero immediate? They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
Quoted 8 lines
Quoted 3 lines
I agree. The use of non-zero immediate has long been deprecated. I guess we should actually reject non-zero immediate for HVC just like we do for SMC.
Ok. Maybe I will hard code Immediate value as 0 to create a syndrome value at the VMM/Qemu and will also put a note stating non-zero immediate for HVC/SVC are not supported/deprecated.
Yes, because this should be the only situation where you should see such an exit to userspace.
Quoted 4 lines
Quoted 3 lines
If there is an actual need for a non-zero immediate to be propagated to userspace (want to emulate Xen's infamous 'HVC #0xEA1'?), then this should be an extension to the current API.
Quoted 6 lines
Quoted 2 lines
Oh ok, then perhaps this new extension change should be simultaneously committed to avoid breaking Xen?
How would that break Xen? I don't have any plan to emulate Xen in any shape or form, and I don't think anyone want to do that in userspace either.
Ok. I thought you are suggestion an exception which is already in use and the current change might break it but now that does not seems to be the case.
Quoted 4 lines
I really want to see an actual use case to expand this stuff. Because so far, we follow the strict SMCCC spec, and nothing else. But if we admit deviations, do we also have to expose SMC and HVC as different instructions?
Ok got it. No problem and makes sense what you just said. Maybe in future if it is really required then we can always come back to it. For now, I will go ahead with the agreed change in the qemu.
Many thanks
Salil.
RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
In reply to Oliver UptonHi Oliver,
Quoted 20 lines
From: Oliver Upton <[email protected]>
Sent: Wednesday, May 17, 2023 7:38 PM
To: Salil Mehta <[email protected]>
Cc: Marc Zyngier <[email protected]>; [email protected]; [email protected]; Paolo Bonzini <[email protected]>; James Morse <[email protected]>; Suzuki K Poulose <[email protected]>; yuzenghui <[email protected]>; Sean Christopherson <[email protected]>
Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support forKVM_EXIT_HYPERCALLHi Salil,
On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
[...]
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Should we expose the ESR, or at least ESR_EL2.IL as an additional flag?
Quoted 7 lines
Quoted 3 lines
I think we would need "Immediate value" of the
ESR_EL2register in the user-space/VMM to be able to construct the syndrome value. I cannot see where it is being sent?The immediate value is not exposed to userspace, although by definition the immediate value must be zero. The SMCCC spec requires all compliant calls to use an immediate of zero (DEN0028E 2.9).
Sure. I do understand this.
Quoted 1 line
Is there a legitimate use case for hypercalls with a nonzero immediate?
To be frank I was not sure of this either and therefore I thought it would be safe to keep the handling in user-space/Qemu generic as it is now by constructing a syndrome value depending upon immediate value and other accompanying parameters from the KVM.
Also, I am not sure what it could break or what platforms it could break. I think we need some Qemu folks to pitch-in and comment on this.
Quoted 2 lines
They would no longer be considered SMCCC calls at that point, so they wouldn't work with the new UAPI.
True. So should we do this change now?
Thanks
Salil
PATCH v3 9/13 KVM: arm64: Introduce 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/api.rst | 4 ++ Documentation/virt/kvm/devices/vm.rst | 79 +++++++++++++++++++++++++++ arch/arm64/include/uapi/asm/kvm.h | 11 ++++ arch/arm64/kvm/arm.c | 4 ++ arch/arm64/kvm/hypercalls.c | 60 ++++++++++++++++++++ include/kvm/arm_hypercalls.h | 3 + 6 files changed, 161 insertions(+)
Documentation/virt/kvm/api.rst +4 -0
@@ -6231,6 +6231,10 @@ requires a guest to interact with host userpace. 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.
Documentation/virt/kvm/devices/vm.rst +79 -0
82 lines not on this page · raw message
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[15 ]; +}; +/* 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 +60 -0
69 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
PATCH v3 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 v3 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 v3 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 v3 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 | 260 ++++++++++++++++++ 2 files changed, 261 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 +260 -0
260 lines not on this page · raw message
Signature · 1 line
2.40.0.348.gf938b09366-goog
Quoted 5 lines · 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.
Quoted 7 lines
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.
[...]
Applied to next, thanks!
[01/13] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL
commit: e65733b5c59a1ea20324a03494364958bef3fc68
[02/13] KVM: arm64: Add a helper to check if a VM has ran once
commit: de40bb8abb764f6866d82c4e2a43acdb22892cf4
[03/13] KVM: arm64: Add vm fd device attribute accessors
commit: e0fc6b21616dd917899ee4a2d4126b4a963c0871
[04/13] KVM: arm64: Rename SMC/HVC call handler to reflect reality
commit: aac94968126beb9846c12a940f1302ece7849b4f
[05/13] KVM: arm64: Start handling SMCs from EL1
commit: c2d2e9b3d8ce9db825a5630d9d52d542f5138ae0
[06/13] KVM: arm64: Refactor hvc filtering to support different actions
commit: a8308b3fc9494953c453480fb277e24f82f7d2b9
[07/13] KVM: arm64: Use a maple tree to represent the SMCCC filter
commit: fb88707dd39bd1d5ec4a058776de9ee99bcc7b72
[08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL
commit: d824dff1919bbd523d4d5c860437d043c0ad121d
[09/13] KVM: arm64: Introduce support for userspace SMCCC filtering
commit: 821d935c87bc95253f82deec3cbb457ccf3de003
[10/13] KVM: arm64: Return NOT_SUPPORTED to guest for unknown PSCI version
commit: 7e484d2785e2a2e526a6b2679d3e4c1402ffe0ec
[11/13] KVM: arm64: Let errors from SMCCC emulation to reach userspace
commit: 37c8e494794786aa8e4acba1f0f5b45f37b11699
[12/13] KVM: selftests: Add a helper for SMCCC calls with SMC instruction
commit: fab19915f498b0e76fabd4d78841c99b7b6d7851
[13/13] KVM: selftests: Add test for SMCCC filter
commit: 60e7dade498eb881bcdf0d9a420c97625f73acc1I've also added the extra patch to deal with with AArch32 T1 encodings of HVC/SMC. Please check the conflict resolution!
Cheers,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.