[PATCH v3 00/13] KVM: arm64: Userspace SMCCC call filtering

[PATCH v3 00/13] KVM: arm64: Userspace SMCCC call filtering

[Oliver Upton]

The Arm SMCCC is rather prescriptive in regards to the allocation of
SMCCC function ID ranges. Many of the hypercall ranges have an
associated specification from Arm (FF-A, PSCI, SDEI, etc.) with some
room for vendor-specific implementations.

The ever-expanding SMCCC surface leaves a lot of work within KVM for
providing new features. Furthermore, KVM implements its own
vendor-specific ABI, with little room for other implementations (like
Hyper-V, for example). Rather than cramming it all into the kernel we
should provide a way for userspace to handle hypercalls.

It would appear that vCPU hotplug [*] has a legitimate use case for
something like this, sending PSCI calls to userspace (where they
should have gone in the first place).

[*] https://lore.kernel.org/kvmarm/20230203135043.409192-1-james.morse@arm.com/

=> We have these new hypercall bitmap registers, why not use that?

The hypercall bitmap registers aren't necessarily aimed at the same
problem. The bitmap registers allow a VMM to preserve the ABI the guest
gets from KVM by default when migrating between hosts. By default KVM
exposes the entire feature set to the guest, whereas user SMCCC calls
need explicit opt-in from userspace.

Applies to 6.3-rc3. 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/20230320221002.4191007-1-oliver.upton@linux.dev/
v2: https://lore.kernel.org/kvmarm/20230330154918.4014761-1-oliver.upton@linux.dev/

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

diff --cc arch/arm64/include/asm/kvm_host.h
index a8e2c52b44aa,2682b3fd0881..000000000000
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@@ -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		8
  	unsigned long flags;
  
  	/*
diff --cc arch/arm64/kvm/arm.c
index 0e5a3ff8cc5a,efee032c9560..000000000000
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@@ -1513,13 -1502,18 +1536,25 @@@ long kvm_arch_vm_ioctl(struct file *fil
  			return -EFAULT;
  		return kvm_vm_ioctl_mte_copy_tags(kvm, &copy_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;
  	}
diff --cc arch/arm64/kvm/pmu-emul.c
index 240168416838,a0fc569fdbca..000000000000
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@@ -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);
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index ba1adc68d695..22938e4d947c 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -164,7 +164,7 @@ static int kvm_smccc_set_filter(struct kvm *kvm, struct kvm_smccc_filter __user
 	if (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 __user
 	set_bit(KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED, &kvm->arch.flags);
 
 out_unlock:
-	mutex_unlock(&kvm->lock);
+	mutex_unlock(&kvm->arch.config_lock);
 	return r;
 }
 

-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 01/13] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL

[Oliver Upton]

The 'longmode' field is a bit annoying as it blows an entire __u32 to
represent a boolean value. Since other architectures are looking to add
support for KVM_EXIT_HYPERCALL, now is probably a good time to clean it
up.

Redefine the field (and the remaining padding) as a set of flags.
Preserve the existing ABI by using bit 0 to indicate if the guest was in
long mode and requiring that the remaining 31 bits must be zero.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 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(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 62de0768d6aa..9b01e3d0e757 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -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
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 808c292ad3f4..15bda40517ff 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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 */
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7f467fe05d42..1a6a1f987949 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -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 */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7713420abab0..27a1d5c1a018 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d77aef872a0a..dd42d7dfb86c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -341,8 +341,13 @@ struct kvm_run {
 			__u64 nr;
 			__u64 args[6];
 			__u64 ret;
-			__u32 longmode;
-			__u32 pad;
+
+			union {
+#ifndef __KERNEL__
+				__u32 longmode;
+#endif
+				__u64 flags;
+			};
 		} hypercall;
 		/* KVM_EXIT_TPR_ACCESS */
 		struct {
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 02/13] KVM: arm64: Add a helper to check if a VM has ran once

[Oliver Upton]

The test_bit(...) pattern is quite a lot of keystrokes. Replace
existing callsites with a helper.

No functional change intended.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h | 3 +++
 arch/arm64/kvm/hypercalls.c       | 3 +--
 arch/arm64/kvm/pmu-emul.c         | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bcd774d74f34..d091d1c9890b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1061,6 +1061,9 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);
 	(system_supports_32bit_el0() &&				\
 	 !static_branch_unlikely(&arm64_mismatched_32bit_el0))
 
+#define kvm_vm_has_ran_once(kvm)					\
+	(test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &(kvm)->arch.flags))
+
 int kvm_trng_call(struct kvm_vcpu *vcpu);
 #ifdef CONFIG_KVM
 extern phys_addr_t hyp_mem_base;
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 5da884e11337..a09a526a7d7c 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -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;
 	}
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 24908400e190..a0fc569fdbca 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -880,7 +880,7 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu, int pmu_id)
 	list_for_each_entry(entry, &arm_pmus, entry) {
 		arm_pmu = entry->arm_pmu;
 		if (arm_pmu->pmu.type == pmu_id) {
-			if (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags) ||
+			if (kvm_vm_has_ran_once(kvm) ||
 			    (kvm->arch.pmu_filter && kvm->arch.arm_pmu != arm_pmu)) {
 				ret = -EBUSY;
 				break;
@@ -963,7 +963,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
 
 		mutex_lock(&kvm->lock);
 
-		if (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags)) {
+		if (kvm_vm_has_ran_once(kvm)) {
 			mutex_unlock(&kvm->lock);
 			return -EBUSY;
 		}
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 03/13] KVM: arm64: Add vm fd device attribute accessors

[Oliver Upton]

A subsequent change will allow userspace to convey a filter for
hypercalls through a vm device attribute. Add the requisite boilerplate
for vm attribute accessors.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/arm.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3bd732eaf087..b6e26c0e65e5 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1439,11 +1439,28 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
 	}
 }
 
+static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	switch (attr->group) {
+	default:
+		return -ENXIO;
+	}
+}
+
+static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	switch (attr->group) {
+	default:
+		return -ENXIO;
+	}
+}
+
 long kvm_arch_vm_ioctl(struct file *filp,
 		       unsigned int ioctl, unsigned long arg)
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
+	struct kvm_device_attr attr;
 
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
@@ -1479,6 +1496,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			return -EFAULT;
 		return kvm_vm_ioctl_mte_copy_tags(kvm, &copy_tags);
 	}
+	case KVM_HAS_DEVICE_ATTR: {
+		if (copy_from_user(&attr, argp, sizeof(attr)))
+			return -EFAULT;
+
+		return kvm_vm_has_attr(kvm, &attr);
+	}
+	case KVM_SET_DEVICE_ATTR: {
+		if (copy_from_user(&attr, argp, sizeof(attr)))
+			return -EFAULT;
+
+		return kvm_vm_set_attr(kvm, &attr);
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 04/13] KVM: arm64: Rename SMC/HVC call handler to reflect reality

[Oliver Upton]

KVM handles SMCCC calls from virtual EL2 that use the SMC instruction
since commit bd36b1a9eb5a ("KVM: arm64: nv: Handle SMCs taken from
virtual EL2"). Thus, the function name of the handler no longer reflects
reality.

Normalize the name on SMCCC, since that's the only hypercall interface
KVM supports in the first place. No fuctional change intended.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/handle_exit.c | 4 ++--
 arch/arm64/kvm/hypercalls.c  | 2 +-
 include/kvm/arm_hypercalls.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index a798c0b4d717..5e4f9737cbd5 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -52,7 +52,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu)
 		return 1;
 	}
 
-	ret = kvm_hvc_call_handler(vcpu);
+	ret = kvm_smccc_call_handler(vcpu);
 	if (ret < 0) {
 		vcpu_set_reg(vcpu, 0, ~0UL);
 		return 1;
@@ -89,7 +89,7 @@ static int handle_smc(struct kvm_vcpu *vcpu)
 	 * at Non-secure EL1 is trapped to EL2 if HCR_EL2.TSC==1, rather than
 	 * being treated as UNDEFINED.
 	 */
-	ret = kvm_hvc_call_handler(vcpu);
+	ret = kvm_smccc_call_handler(vcpu);
 	if (ret < 0)
 		vcpu_set_reg(vcpu, 0, ~0UL);
 
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index a09a526a7d7c..5ead6c6afff0 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -121,7 +121,7 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
 	}
 }
 
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
+int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 {
 	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
 	u32 func_id = smccc_get_function(vcpu);
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index 1188f116cf4e..8f4e33bc43e8 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -6,7 +6,7 @@
 
 #include <asm/kvm_emulate.h>
 
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+int kvm_smccc_call_handler(struct kvm_vcpu *vcpu);
 
 static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
 {
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 05/13] KVM: arm64: Start handling SMCs from EL1

[Oliver Upton]

Whelp, the architecture gods have spoken and confirmed that the function
ID space is common between SMCs and HVCs. Not only that, the expectation
is that hypervisors handle calls to both SMC and HVC conduits. KVM
recently picked up support for SMCCCs in commit bd36b1a9eb5a ("KVM:
arm64: nv: Handle SMCs taken from virtual EL2") but scoped it only to a
nested hypervisor.

Let's just open the floodgates and let EL1 access our SMCCC
implementation with the SMC instruction as well.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/handle_exit.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 5e4f9737cbd5..68f95dcd41a1 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -72,13 +72,15 @@ static int handle_smc(struct kvm_vcpu *vcpu)
 	 *
 	 * We need to advance the PC after the trap, as it would
 	 * otherwise return to the same address...
-	 *
-	 * Only handle SMCs from the virtual EL2 with an immediate of zero and
-	 * skip it otherwise.
 	 */
-	if (!vcpu_is_el2(vcpu) || kvm_vcpu_hvc_get_imm(vcpu)) {
+	kvm_incr_pc(vcpu);
+
+	/*
+	 * SMCs with a nonzero immediate are reserved according to DEN0028E 2.9
+	 * "SMC and HVC immediate value".
+	 */
+	if (kvm_vcpu_hvc_get_imm(vcpu)) {
 		vcpu_set_reg(vcpu, 0, ~0UL);
-		kvm_incr_pc(vcpu);
 		return 1;
 	}
 
@@ -93,8 +95,6 @@ static int handle_smc(struct kvm_vcpu *vcpu)
 	if (ret < 0)
 		vcpu_set_reg(vcpu, 0, ~0UL);
 
-	kvm_incr_pc(vcpu);
-
 	return ret;
 }
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 06/13] KVM: arm64: Refactor hvc filtering to support different actions

[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 <suzuki.poulose@arm.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/uapi/asm/kvm.h |  9 +++++++++
 arch/arm64/kvm/hypercalls.c       | 26 ++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index f8129c624b07..f9672ef1159a 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -469,6 +469,15 @@ enum {
 /* run->fail_entry.hardware_entry_failure_reason codes. */
 #define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED	(1ULL << 0)
 
+enum kvm_smccc_filter_action {
+	KVM_SMCCC_FILTER_HANDLE = 0,
+	KVM_SMCCC_FILTER_DENY,
+
+#ifdef __KERNEL__
+	NR_SMCCC_FILTER_ACTIONS
+#endif
+};
+
 #endif
 
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 5ead6c6afff0..0be974e2f1fc 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -65,7 +65,7 @@ static void kvm_ptp_get_time(struct kvm_vcpu *vcpu, u64 *val)
 	val[3] = lower_32_bits(cycles);
 }
 
-static bool kvm_hvc_call_default_allowed(u32 func_id)
+static bool kvm_smccc_default_allowed(u32 func_id)
 {
 	switch (func_id) {
 	/*
@@ -93,7 +93,7 @@ static bool kvm_hvc_call_default_allowed(u32 func_id)
 	}
 }
 
-static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
+static bool kvm_smccc_test_fw_bmap(struct kvm_vcpu *vcpu, u32 func_id)
 {
 	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
 
@@ -117,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:
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 07/13] KVM: arm64: Use a maple tree to represent the SMCCC filter

[Oliver Upton]

Maple tree is an efficient B-tree implementation that is intended for
storing non-overlapping intervals. Such a data structure is a good fit
for the SMCCC filter as it is desirable to sparsely allocate the 32 bit
function ID space.

To that end, add a maple tree to kvm_arch and correctly init/teardown
along with the VM. Wire in a test against the hypercall filter for HVCs
which does nothing until the controls are exposed to userspace.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  5 ++-
 arch/arm64/kvm/arm.c              |  2 ++
 arch/arm64/kvm/hypercalls.c       | 57 +++++++++++++++++++++++++++++++
 include/kvm/arm_hypercalls.h      |  1 +
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index d091d1c9890b..2682b3fd0881 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -16,6 +16,7 @@
 #include <linux/types.h>
 #include <linux/jump_label.h>
 #include <linux/kvm_types.h>
+#include <linux/maple_tree.h>
 #include <linux/percpu.h>
 #include <linux/psci.h>
 #include <asm/arch_gicv3.h>
@@ -221,7 +222,8 @@ struct kvm_arch {
 #define KVM_ARCH_FLAG_EL1_32BIT				4
 	/* PSCI SYSTEM_SUSPEND enabled for the guest */
 #define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED		5
-
+	/* SMCCC filter initialized for the VM */
+#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED		6
 	unsigned long flags;
 
 	/*
@@ -242,6 +244,7 @@ struct kvm_arch {
 
 	/* Hypercall features firmware registers' descriptor */
 	struct kvm_smccc_features smccc_feat;
+	struct maple_tree smccc_filter;
 
 	/*
 	 * For an untrusted host VM, 'pkvm.handle' is used to lookup
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index b6e26c0e65e5..1202ac03bee0 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -192,6 +192,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	kvm_destroy_vcpus(kvm);
 
 	kvm_unshare_hyp(kvm, kvm + 1);
+
+	kvm_arm_teardown_hypercalls(kvm);
 }
 
 int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 0be974e2f1fc..ba7cd84c6668 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -121,8 +121,58 @@ static bool kvm_smccc_test_fw_bmap(struct kvm_vcpu *vcpu, u32 func_id)
 	}
 }
 
+#define SMCCC_ARCH_RANGE_BEGIN	ARM_SMCCC_VERSION_FUNC_ID
+#define SMCCC_ARCH_RANGE_END				\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
+			   ARM_SMCCC_SMC_32,		\
+			   0, ARM_SMCCC_FUNC_MASK)
+
+static void init_smccc_filter(struct kvm *kvm)
+{
+	int r;
+
+	mt_init(&kvm->arch.smccc_filter);
+
+	/*
+	 * Prevent userspace from handling any SMCCC calls in the architecture
+	 * range, avoiding the risk of misrepresenting Spectre mitigation status
+	 * to the guest.
+	 */
+	r = mtree_insert_range(&kvm->arch.smccc_filter,
+			       SMCCC_ARCH_RANGE_BEGIN, SMCCC_ARCH_RANGE_END,
+			       xa_mk_value(KVM_SMCCC_FILTER_HANDLE),
+			       GFP_KERNEL_ACCOUNT);
+	WARN_ON_ONCE(r);
+}
+
+static u8 kvm_smccc_filter_get_action(struct kvm *kvm, u32 func_id)
+{
+	unsigned long idx = func_id;
+	void *val;
+
+	if (!test_bit(KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED, &kvm->arch.flags))
+		return KVM_SMCCC_FILTER_HANDLE;
+
+	/*
+	 * But where's the error handling, you say?
+	 *
+	 * mt_find() returns NULL if no entry was found, which just so happens
+	 * to match KVM_SMCCC_FILTER_HANDLE.
+	 */
+	val = mt_find(&kvm->arch.smccc_filter, &idx, idx);
+	return xa_to_value(val);
+}
+
 static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
 {
+	/*
+	 * Intervening actions in the SMCCC filter take precedence over the
+	 * pseudo-firmware register bitmaps.
+	 */
+	u8 action = kvm_smccc_filter_get_action(vcpu->kvm, func_id);
+	if (action != KVM_SMCCC_FILTER_HANDLE)
+		return action;
+
 	if (kvm_smccc_test_fw_bmap(vcpu, func_id) ||
 	    kvm_smccc_default_allowed(func_id))
 		return KVM_SMCCC_FILTER_HANDLE;
@@ -263,6 +313,13 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
 	smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
 	smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
 	smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
+
+	init_smccc_filter(kvm);
+}
+
+void kvm_arm_teardown_hypercalls(struct kvm *kvm)
+{
+	mtree_destroy(&kvm->arch.smccc_filter);
 }
 
 int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index 8f4e33bc43e8..fe6c31575b05 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -43,6 +43,7 @@ static inline void smccc_set_retval(struct kvm_vcpu *vcpu,
 struct kvm_one_reg;
 
 void kvm_arm_init_hypercalls(struct kvm *kvm);
+void kvm_arm_teardown_hypercalls(struct kvm *kvm);
 int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_fw_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices);
 int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Oliver Upton]

In anticipation of user hypercall filters, add the necessary plumbing to
get SMCCC calls out to userspace. Even though the exit structure has
space for KVM to pass register arguments, let's just avoid it altogether
and let userspace poke at the registers via KVM_GET_ONE_REG.

This deliberately stretches the definition of a 'hypercall' to cover
SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
support EL1 calls into secure services, but now we can paint that as a
userspace problem and be done with it.

Finally, we need a flag to let userspace know what conduit instruction
was used (i.e. SMC vs. HVC).

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 Documentation/virt/kvm/api.rst    | 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(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 9b01e3d0e757..9497792c4ee5 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -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 */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index f9672ef1159a..f86446c5a7e3 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -472,12 +472,16 @@ enum {
 enum kvm_smccc_filter_action {
 	KVM_SMCCC_FILTER_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__ */
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 68f95dcd41a1..3f43e20c48b6 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
 	 * Trap exception, not a Secure Monitor Call exception [...]"
 	 *
 	 * We need to advance the PC after the trap, as it would
-	 * otherwise return to the same address...
+	 * otherwise return to the same address. Furthermore, pre-incrementing
+	 * the PC before potentially exiting to userspace maintains the same
+	 * abstraction for both SMCs and HVCs.
 	 */
 	kvm_incr_pc(vcpu);
 
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index ba7cd84c6668..2db53709bec1 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -180,6 +180,19 @@ static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
 	return KVM_SMCCC_FILTER_DENY;
 }
 
+static void kvm_prepare_hypercall_exit(struct kvm_vcpu *vcpu, u32 func_id)
+{
+	u8 ec = ESR_ELx_EC(kvm_vcpu_get_esr(vcpu));
+	struct kvm_run *run = vcpu->run;
+
+	run->exit_reason = KVM_EXIT_HYPERCALL;
+	run->hypercall.nr = func_id;
+	run->hypercall.flags = 0;
+
+	if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64)
+		run->hypercall.flags |= KVM_HYPERCALL_EXIT_SMC;
+}
+
 int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 {
 	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
@@ -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;
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 09/13] KVM: arm64: Introduce support for userspace SMCCC filtering

[Oliver Upton]

As the SMCCC (and related specifications) march towards an 'everything
and the kitchen sink' interface for interacting with a system it becomes
less likely that KVM will support every related feature. We could do
better by letting userspace have a crack at it instead.

Allow userspace to define an 'SMCCC filter' that applies to both HVCs
and SMCs initiated by the guest. Supporting both conduits with this
interface is important for a couple of reasons. Guest SMC usage is table
stakes for a nested guest, as HVCs are always taken to the virtual EL2.
Additionally, guests may want to interact with a service on the secure
side which can now be proxied by userspace.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 Documentation/virt/kvm/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(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 9497792c4ee5..c8ab2f730945 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -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.
diff --git a/Documentation/virt/kvm/devices/vm.rst b/Documentation/virt/kvm/devices/vm.rst
index 147efec626e5..9d726e60ec47 100644
--- a/Documentation/virt/kvm/devices/vm.rst
+++ b/Documentation/virt/kvm/devices/vm.rst
@@ -321,3 +321,82 @@ 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:
+
+        ======  ===========================================
+        EEXIST  Range intersects with a previously inserted
+                or reserved 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_HANDLE = 0,
+            KVM_SMCCC_FILTER_DENY,
+            KVM_SMCCC_FILTER_FWD_TO_USER,
+    };
+
+    struct kvm_smccc_filter {
+            __u32 base;
+            __u32 nr_functions;
+            __u8 action;
+            __u8 pad[15];
+    };
+
+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 range is not allowed to wrap,
+i.e. userspace cannot rely on ``base + nr_functions`` overflowing.
+
+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_HANDLE``: 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``.
+
+The ``pad`` field is reserved for future use and must be zero. KVM may
+return ``-EINVAL`` if the field is nonzero.
+
+KVM reserves the 'Arm Architecture Calls' range of function IDs and
+will reject attempts to define a filter for any portion of these ranges:
+
+        =========== ===============
+        Start       End (inclusive)
+        =========== ===============
+        0x8000_0000 0x8000_FFFF
+        0xC000_0000 0xC000_FFFF
+        =========== ===============
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index f86446c5a7e3..3dcfa4bfdf83 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -372,6 +372,10 @@ enum {
 #endif
 };
 
+/* Device Control API on vm fd */
+#define KVM_ARM_VM_SMCCC_CTRL		0
+#define   KVM_ARM_VM_SMCCC_FILTER	0
+
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
@@ -479,6 +483,13 @@ enum kvm_smccc_filter_action {
 #endif
 };
 
+struct kvm_smccc_filter {
+	__u32 base;
+	__u32 nr_functions;
+	__u8 action;
+	__u8 pad[15];
+};
+
 /* arm64-specific KVM_EXIT_HYPERCALL flags */
 #define KVM_HYPERCALL_EXIT_SMC	(1U << 0)
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 1202ac03bee0..efee032c9560 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1444,6 +1444,8 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
 static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	switch (attr->group) {
+	case KVM_ARM_VM_SMCCC_CTRL:
+		return kvm_vm_smccc_has_attr(kvm, attr);
 	default:
 		return -ENXIO;
 	}
@@ -1452,6 +1454,8 @@ static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 static int kvm_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	switch (attr->group) {
+	case KVM_ARM_VM_SMCCC_CTRL:
+		return kvm_vm_smccc_set_attr(kvm, attr);
 	default:
 		return -ENXIO;
 	}
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 2db53709bec1..9a35d6d18193 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -145,6 +145,44 @@ 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)
+{
+	const void *zero_page = page_to_virt(ZERO_PAGE(0));
+	struct kvm_smccc_filter filter;
+	u32 start, end;
+	int r;
+
+	if (copy_from_user(&filter, uaddr, sizeof(filter)))
+		return -EFAULT;
+
+	if (memcmp(filter.pad, zero_page, sizeof(filter.pad)))
+		return -EINVAL;
+
+	start = filter.base;
+	end = start + filter.nr_functions - 1;
+
+	if (end < start || filter.action >= NR_SMCCC_FILTER_ACTIONS)
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+
+	if (kvm_vm_has_ran_once(kvm)) {
+		r = -EBUSY;
+		goto out_unlock;
+	}
+
+	r = mtree_insert_range(&kvm->arch.smccc_filter, start, end,
+			       xa_mk_value(filter.action), GFP_KERNEL_ACCOUNT);
+	if (r)
+		goto out_unlock;
+
+	set_bit(KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED, &kvm->arch.flags);
+
+out_unlock:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
+
 static u8 kvm_smccc_filter_get_action(struct kvm *kvm, u32 func_id)
 {
 	unsigned long idx = func_id;
@@ -569,3 +607,25 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 
 	return -EINVAL;
 }
+
+int kvm_vm_smccc_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	switch (attr->attr) {
+	case KVM_ARM_VM_SMCCC_FILTER:
+		return 0;
+	default:
+		return -ENXIO;
+	}
+}
+
+int kvm_vm_smccc_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	void __user *uaddr = (void __user *)attr->addr;
+
+	switch (attr->attr) {
+	case KVM_ARM_VM_SMCCC_FILTER:
+		return kvm_smccc_set_filter(kvm, uaddr);
+	default:
+		return -ENXIO;
+	}
+}
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index fe6c31575b05..2df152207ccd 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -49,4 +49,7 @@ int kvm_arm_copy_fw_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices);
 int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
 int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
 
+int kvm_vm_smccc_has_attr(struct kvm *kvm, struct kvm_device_attr *attr);
+int kvm_vm_smccc_set_attr(struct kvm *kvm, struct kvm_device_attr *attr);
+
 #endif
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 10/13] KVM: arm64: Return NOT_SUPPORTED to guest for unknown PSCI version

[Oliver Upton]

A 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.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/psci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
index 7fbc4c1b9df0..aff54b106c30 100644
--- a/arch/arm64/kvm/psci.c
+++ b/arch/arm64/kvm/psci.c
@@ -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;
 	}
 }
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 11/13] KVM: arm64: Let errors from SMCCC emulation to reach userspace

[Oliver Upton]

Typically 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.

Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/handle_exit.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 3f43e20c48b6..6dcd6604b6bc 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -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);
 }
 
 /*
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 12/13] KVM: selftests: Add a helper for SMCCC calls with SMC instruction

[Oliver Upton]

Build a helper for doing SMCs in selftests by macro-izing the current
HVC implementation and taking the conduit instruction as an argument.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 .../selftests/kvm/include/aarch64/processor.h | 13 +++++
 .../selftests/kvm/lib/aarch64/processor.c     | 52 ++++++++++++-------
 2 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 5f977528e09c..cb537253a6b9 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -214,6 +214,19 @@ void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
 	       uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
 	       uint64_t arg6, struct arm_smccc_res *res);
 
+/**
+ * smccc_smc - Invoke a SMCCC function using the smc conduit
+ * @function_id: the SMCCC function to be called
+ * @arg0-arg6: SMCCC function arguments, corresponding to registers x1-x7
+ * @res: pointer to write the return values from registers x0-x3
+ *
+ */
+void smccc_smc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
+	       uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
+	       uint64_t arg6, struct arm_smccc_res *res);
+
+
+
 uint32_t guest_get_vcpuid(void);
 
 #endif /* SELFTEST_KVM_PROCESSOR_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 5972a23b2765..24e8122307f4 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -508,29 +508,43 @@ void aarch64_get_supported_page_sizes(uint32_t ipa,
 	close(kvm_fd);
 }
 
+#define __smccc_call(insn, function_id, arg0, arg1, arg2, arg3, arg4, arg5,	\
+		     arg6, res)							\
+	asm volatile("mov   w0, %w[function_id]\n"				\
+		     "mov   x1, %[arg0]\n"					\
+		     "mov   x2, %[arg1]\n"					\
+		     "mov   x3, %[arg2]\n"					\
+		     "mov   x4, %[arg3]\n"					\
+		     "mov   x5, %[arg4]\n"					\
+		     "mov   x6, %[arg5]\n"					\
+		     "mov   x7, %[arg6]\n"					\
+		     #insn  "#0\n"						\
+		     "mov   %[res0], x0\n"					\
+		     "mov   %[res1], x1\n"					\
+		     "mov   %[res2], x2\n"					\
+		     "mov   %[res3], x3\n"					\
+		     : [res0] "=r"(res->a0), [res1] "=r"(res->a1),		\
+		       [res2] "=r"(res->a2), [res3] "=r"(res->a3)		\
+		     : [function_id] "r"(function_id), [arg0] "r"(arg0),	\
+		       [arg1] "r"(arg1), [arg2] "r"(arg2), [arg3] "r"(arg3),	\
+		       [arg4] "r"(arg4), [arg5] "r"(arg5), [arg6] "r"(arg6)	\
+		     : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7")
+
+
 void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
 	       uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
 	       uint64_t arg6, struct arm_smccc_res *res)
 {
-	asm volatile("mov   w0, %w[function_id]\n"
-		     "mov   x1, %[arg0]\n"
-		     "mov   x2, %[arg1]\n"
-		     "mov   x3, %[arg2]\n"
-		     "mov   x4, %[arg3]\n"
-		     "mov   x5, %[arg4]\n"
-		     "mov   x6, %[arg5]\n"
-		     "mov   x7, %[arg6]\n"
-		     "hvc   #0\n"
-		     "mov   %[res0], x0\n"
-		     "mov   %[res1], x1\n"
-		     "mov   %[res2], x2\n"
-		     "mov   %[res3], x3\n"
-		     : [res0] "=r"(res->a0), [res1] "=r"(res->a1),
-		       [res2] "=r"(res->a2), [res3] "=r"(res->a3)
-		     : [function_id] "r"(function_id), [arg0] "r"(arg0),
-		       [arg1] "r"(arg1), [arg2] "r"(arg2), [arg3] "r"(arg3),
-		       [arg4] "r"(arg4), [arg5] "r"(arg5), [arg6] "r"(arg6)
-		     : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7");
+	__smccc_call(hvc, function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+		     arg6, res);
+}
+
+void smccc_smc(uint32_t function_id, uint64_t arg0, uint64_t arg1,
+	       uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
+	       uint64_t arg6, struct arm_smccc_res *res)
+{
+	__smccc_call(smc, function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+		     arg6, res);
 }
 
 void kvm_selftest_arch_init(void)
-- 
2.40.0.348.gf938b09366-goog

[PATCH v3 13/13] KVM: selftests: Add test for SMCCC filter

[Oliver Upton]

Add a selftest for the SMCCC filter, ensuring basic UAPI constraints
(e.g. reserved ranges, non-overlapping ranges) are upheld. Additionally,
test that the DENIED and FWD_TO_USER work as intended.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/smccc_filter.c      | 260 ++++++++++++++++++
 2 files changed, 261 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/smccc_filter.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 84a627c43795..d66a0642cffd 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -141,6 +141,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
 TEST_GEN_PROGS_aarch64 += aarch64/psci_test
+TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
 TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
diff --git a/tools/testing/selftests/kvm/aarch64/smccc_filter.c b/tools/testing/selftests/kvm/aarch64/smccc_filter.c
new file mode 100644
index 000000000000..0f9db0641847
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/smccc_filter.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * smccc_filter - Tests for the SMCCC filter UAPI.
+ *
+ * Copyright (c) 2023 Google LLC
+ *
+ * This test includes:
+ *  - Tests that the UAPI constraints are upheld by KVM. For example, userspace
+ *    is prevented from filtering the architecture range of SMCCC calls.
+ *  - Test that the filter actions (DENIED, FWD_TO_USER) work as intended.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/psci.h>
+#include <stdint.h>
+
+#include "processor.h"
+#include "test_util.h"
+
+enum smccc_conduit {
+	HVC_INSN,
+	SMC_INSN,
+};
+
+#define for_each_conduit(conduit)					\
+	for (conduit = HVC_INSN; conduit <= SMC_INSN; conduit++)
+
+static void guest_main(uint32_t func_id, enum smccc_conduit conduit)
+{
+	struct arm_smccc_res res;
+
+	if (conduit == SMC_INSN)
+		smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+	else
+		smccc_hvc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+
+	GUEST_SYNC(res.a0);
+}
+
+static int __set_smccc_filter(struct kvm_vm *vm, uint32_t start, uint32_t nr_functions,
+			      enum kvm_smccc_filter_action action)
+{
+	struct kvm_smccc_filter filter = {
+		.base		= start,
+		.nr_functions	= nr_functions,
+		.action		= action,
+	};
+
+	return __kvm_device_attr_set(vm->fd, KVM_ARM_VM_SMCCC_CTRL,
+				     KVM_ARM_VM_SMCCC_FILTER, &filter);
+}
+
+static void set_smccc_filter(struct kvm_vm *vm, uint32_t start, uint32_t nr_functions,
+			     enum kvm_smccc_filter_action action)
+{
+	int ret = __set_smccc_filter(vm, start, nr_functions, action);
+
+	TEST_ASSERT(!ret, "failed to configure SMCCC filter: %d", ret);
+}
+
+static struct kvm_vm *setup_vm(struct kvm_vcpu **vcpu)
+{
+	struct kvm_vcpu_init init;
+	struct kvm_vm *vm;
+
+	vm = vm_create(1);
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
+
+	/*
+	 * Enable in-kernel emulation of PSCI to ensure that calls are denied
+	 * due to the SMCCC filter, not because of KVM.
+	 */
+	init.features[0] |= (1 << KVM_ARM_VCPU_PSCI_0_2);
+
+	*vcpu = aarch64_vcpu_add(vm, 0, &init, guest_main);
+	return vm;
+}
+
+static void test_pad_must_be_zero(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	struct kvm_smccc_filter filter = {
+		.base		= PSCI_0_2_FN_PSCI_VERSION,
+		.nr_functions	= 1,
+		.action		= KVM_SMCCC_FILTER_DENY,
+		.pad		= { -1 },
+	};
+	int r;
+
+	r = __kvm_device_attr_set(vm->fd, KVM_ARM_VM_SMCCC_CTRL,
+				  KVM_ARM_VM_SMCCC_FILTER, &filter);
+	TEST_ASSERT(r < 0 && errno == EINVAL,
+		    "Setting filter with nonzero padding should return EINVAL");
+}
+
+/* Ensure that userspace cannot filter the Arm Architecture SMCCC range */
+static void test_filter_reserved_range(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	int r;
+
+	r = __set_smccc_filter(vm, ARM_SMCCC_ARCH_WORKAROUND_1,
+			       1, KVM_SMCCC_FILTER_DENY);
+	TEST_ASSERT(r < 0 && errno == EEXIST,
+		    "Attempt to filter reserved range should return EEXIST");
+
+	kvm_vm_free(vm);
+}
+
+static void test_invalid_nr_functions(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	int r;
+
+	r = __set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 0, KVM_SMCCC_FILTER_DENY);
+	TEST_ASSERT(r < 0 && errno == EINVAL,
+		    "Attempt to filter 0 functions should return EINVAL");
+
+	kvm_vm_free(vm);
+}
+
+static void test_overflow_nr_functions(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	int r;
+
+	r = __set_smccc_filter(vm, ~0, ~0, KVM_SMCCC_FILTER_DENY);
+	TEST_ASSERT(r < 0 && errno == EINVAL,
+		    "Attempt to overflow filter range should return EINVAL");
+
+	kvm_vm_free(vm);
+}
+
+static void test_reserved_action(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	int r;
+
+	r = __set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 1, -1);
+	TEST_ASSERT(r < 0 && errno == EINVAL,
+		    "Attempt to use reserved filter action should return EINVAL");
+
+	kvm_vm_free(vm);
+}
+
+
+/* Test that overlapping configurations of the SMCCC filter are rejected */
+static void test_filter_overlap(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = setup_vm(&vcpu);
+	int r;
+
+	set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 1, KVM_SMCCC_FILTER_DENY);
+
+	r = __set_smccc_filter(vm, PSCI_0_2_FN64_CPU_ON, 1, KVM_SMCCC_FILTER_DENY);
+	TEST_ASSERT(r < 0 && errno == EEXIST,
+		    "Attempt to filter already configured range should return EEXIST");
+
+	kvm_vm_free(vm);
+}
+
+static void expect_call_denied(struct kvm_vcpu *vcpu)
+{
+	struct ucall uc;
+
+	if (get_ucall(vcpu, &uc) != UCALL_SYNC)
+		TEST_FAIL("Unexpected ucall: %lu\n", uc.cmd);
+
+	TEST_ASSERT(uc.args[1] == SMCCC_RET_NOT_SUPPORTED,
+		    "Unexpected SMCCC return code: %lu", uc.args[1]);
+}
+
+/* Denied SMCCC calls have a return code of SMCCC_RET_NOT_SUPPORTED */
+static void test_filter_denied(void)
+{
+	enum smccc_conduit conduit;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	for_each_conduit(conduit) {
+		vm = setup_vm(&vcpu);
+
+		set_smccc_filter(vm, PSCI_0_2_FN_PSCI_VERSION, 1, KVM_SMCCC_FILTER_DENY);
+		vcpu_args_set(vcpu, 2, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+		vcpu_run(vcpu);
+		expect_call_denied(vcpu);
+
+		kvm_vm_free(vm);
+	}
+}
+
+static void expect_call_fwd_to_user(struct kvm_vcpu *vcpu, uint32_t func_id,
+				    enum smccc_conduit conduit)
+{
+	struct kvm_run *run = vcpu->run;
+
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_HYPERCALL,
+		    "Unexpected exit reason: %u", run->exit_reason);
+	TEST_ASSERT(run->hypercall.nr == func_id,
+		    "Unexpected SMCCC function: %llu", run->hypercall.nr);
+
+	if (conduit == SMC_INSN)
+		TEST_ASSERT(run->hypercall.flags & KVM_HYPERCALL_EXIT_SMC,
+			    "KVM_HYPERCALL_EXIT_SMC is not set");
+	else
+		TEST_ASSERT(!(run->hypercall.flags & KVM_HYPERCALL_EXIT_SMC),
+			    "KVM_HYPERCAL_EXIT_SMC is set");
+}
+
+/* SMCCC calls forwarded to userspace cause KVM_EXIT_HYPERCALL exits */
+static void test_filter_fwd_to_user(void)
+{
+	enum smccc_conduit conduit;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	for_each_conduit(conduit) {
+		vm = setup_vm(&vcpu);
+
+		set_smccc_filter(vm, PSCI_0_2_FN_PSCI_VERSION, 1, KVM_SMCCC_FILTER_FWD_TO_USER);
+		vcpu_args_set(vcpu, 2, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+		vcpu_run(vcpu);
+		expect_call_fwd_to_user(vcpu, PSCI_0_2_FN_PSCI_VERSION, conduit);
+
+		kvm_vm_free(vm);
+	}
+}
+
+static bool kvm_supports_smccc_filter(void)
+{
+	struct kvm_vm *vm = vm_create_barebones();
+	int r;
+
+	r = __kvm_has_device_attr(vm->fd, KVM_ARM_VM_SMCCC_CTRL, KVM_ARM_VM_SMCCC_FILTER);
+
+	kvm_vm_free(vm);
+	return !r;
+}
+
+int main(void)
+{
+	TEST_REQUIRE(kvm_supports_smccc_filter());
+
+	test_pad_must_be_zero();
+	test_invalid_nr_functions();
+	test_overflow_nr_functions();
+	test_reserved_action();
+	test_filter_reserved_range();
+	test_filter_overlap();
+	test_filter_denied();
+	test_filter_fwd_to_user();
+}
-- 
2.40.0.348.gf938b09366-goog

Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Marc Zyngier]

On Tue, 04 Apr 2023 16:40:45 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> In anticipation of user hypercall filters, add the necessary plumbing to
> get SMCCC calls out to userspace. Even though the exit structure has
> space for KVM to pass register arguments, let's just avoid it altogether
> and let userspace poke at the registers via KVM_GET_ONE_REG.
> 
> This deliberately stretches the definition of a 'hypercall' to cover
> SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
> support EL1 calls into secure services, but now we can paint that as a
> userspace problem and be done with it.
> 
> Finally, we need a flag to let userspace know what conduit instruction
> was used (i.e. SMC vs. HVC).
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Documentation/virt/kvm/api.rst    | 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(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 9b01e3d0e757..9497792c4ee5 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -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 */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index f9672ef1159a..f86446c5a7e3 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -472,12 +472,16 @@ enum {
>  enum kvm_smccc_filter_action {
>  	KVM_SMCCC_FILTER_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__ */
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 68f95dcd41a1..3f43e20c48b6 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
>  	 * Trap exception, not a Secure Monitor Call exception [...]"
>  	 *
>  	 * We need to advance the PC after the trap, as it would
> -	 * otherwise return to the same address...
> +	 * otherwise return to the same address. Furthermore, pre-incrementing
> +	 * the PC before potentially exiting to userspace maintains the same
> +	 * abstraction for both SMCs and HVCs.

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.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Marc Zyngier]

On Wed, 05 Apr 2023 08:35:05 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Tue, 04 Apr 2023 16:40:45 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > In anticipation of user hypercall filters, add the necessary plumbing to
> > get SMCCC calls out to userspace. Even though the exit structure has
> > space for KVM to pass register arguments, let's just avoid it altogether
> > and let userspace poke at the registers via KVM_GET_ONE_REG.
> > 
> > This deliberately stretches the definition of a 'hypercall' to cover
> > SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
> > support EL1 calls into secure services, but now we can paint that as a
> > userspace problem and be done with it.
> > 
> > Finally, we need a flag to let userspace know what conduit instruction
> > was used (i.e. SMC vs. HVC).
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  Documentation/virt/kvm/api.rst    | 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(-)
> > 
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 9b01e3d0e757..9497792c4ee5 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -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 */
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index f9672ef1159a..f86446c5a7e3 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -472,12 +472,16 @@ enum {
> >  enum kvm_smccc_filter_action {
> >  	KVM_SMCCC_FILTER_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__ */
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > index 68f95dcd41a1..3f43e20c48b6 100644
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
> >  	 * Trap exception, not a Secure Monitor Call exception [...]"
> >  	 *
> >  	 * We need to advance the PC after the trap, as it would
> > -	 * otherwise return to the same address...
> > +	 * otherwise return to the same address. Furthermore, pre-incrementing
> > +	 * the PC before potentially exiting to userspace maintains the same
> > +	 * abstraction for both SMCs and HVCs.
> 
> 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?

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 <maz@kernel.org>
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.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 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(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index c8ab2f730945..103f945959ed 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -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 */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 3dcfa4bfdf83..b1c1edf85480 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -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
 
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 9a35d6d18193..3b6523f25afc 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -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)
-- 
2.34.1


-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Oliver Upton]

Hey Marc,

On Wed, Apr 05, 2023 at 12:59:20PM +0100, Marc Zyngier wrote:

[...]

> > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > > index 68f95dcd41a1..3f43e20c48b6 100644
> > > --- a/arch/arm64/kvm/handle_exit.c
> > > +++ b/arch/arm64/kvm/handle_exit.c
> > > @@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
> > >  	 * Trap exception, not a Secure Monitor Call exception [...]"
> > >  	 *
> > >  	 * We need to advance the PC after the trap, as it would
> > > -	 * otherwise return to the same address...
> > > +	 * otherwise return to the same address. Furthermore, pre-incrementing
> > > +	 * the PC before potentially exiting to userspace maintains the same
> > > +	 * abstraction for both SMCs and HVCs.
> > 
> > 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?
> 
> 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 :)

> From 9b830e7a3819c2771074bebe66c1d5f20394e3cc Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> 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.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

> ---
>  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(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index c8ab2f730945..103f945959ed 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -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 */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 3dcfa4bfdf83..b1c1edf85480 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -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
>  
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 9a35d6d18193..3b6523f25afc 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -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)
> -- 
> 2.34.1
> 
> 
> -- 
> Without deviation from the norm, progress is not possible.

-- 
Thanks,
Oliver

Re: [PATCH v3 00/13] KVM: arm64: Userspace SMCCC call filtering

[Marc Zyngier]

On Tue, 4 Apr 2023 15:40:37 +0000, Oliver Upton wrote:
> 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.
> 
> [...]

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: 60e7dade498eb881bcdf0d9a420c97625f73acc1

I've also added the extra patch to deal with with AArch32 T1
encodings of HVC/SMC. Please check the conflict resolution!

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Salil Mehta]

Hi 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.

> From: Marc Zyngier <maz@kernel.org>
> Sent: Wednesday, April 5, 2023 12:59 PM
> To: Oliver Upton <oliver.upton@linux.dev>
> Cc: kvmarm@lists.linux.dev; kvm@vger.kernel.org; Paolo Bonzini
> <pbonzini@redhat.com>; James Morse <james.morse@arm.com>; Suzuki K Poulose
> <suzuki.poulose@arm.com>; yuzenghui <yuzenghui@huawei.com>; Sean
> Christopherson <seanjc@google.com>; Salil Mehta <salil.mehta@huawei.com>
> Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support for
> KVM_EXIT_HYPERCALL
> 
> On Wed, 05 Apr 2023 08:35:05 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Tue, 04 Apr 2023 16:40:45 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > >
> > > In anticipation of user hypercall filters, add the necessary plumbing to
> > > get SMCCC calls out to userspace. Even though the exit structure has
> > > space for KVM to pass register arguments, let's just avoid it altogether
> > > and let userspace poke at the registers via KVM_GET_ONE_REG.
> > >
> > > This deliberately stretches the definition of a 'hypercall' to cover
> > > SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
> > > support EL1 calls into secure services, but now we can paint that as a
> > > userspace problem and be done with it.
> > >
> > > Finally, we need a flag to let userspace know what conduit instruction
> > > was used (i.e. SMC vs. HVC).
> > >
> > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > > ---
> > >  Documentation/virt/kvm/api.rst    | 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(-)
> > >
> > > diff --git a/Documentation/virt/kvm/api.rst  b/Documentation/virt/kvm/api.rst
> > > index 9b01e3d0e757..9497792c4ee5 100644
> > > --- a/Documentation/virt/kvm/api.rst
> > > +++ b/Documentation/virt/kvm/api.rst
> > > @@ -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 */
> > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > > index f9672ef1159a..f86446c5a7e3 100644
> > > --- a/arch/arm64/include/uapi/asm/kvm.h
> > > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > > @@ -472,12 +472,16 @@ enum {
> > >  enum kvm_smccc_filter_action {
> > >  	KVM_SMCCC_FILTER_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__ */
> > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > > index 68f95dcd41a1..3f43e20c48b6 100644
> > > --- a/arch/arm64/kvm/handle_exit.c
> > > +++ b/arch/arm64/kvm/handle_exit.c
> > > @@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
> > >  	 * Trap exception, not a Secure Monitor Call exception [...]"
> > >  	 *
> > >  	 * We need to advance the PC after the trap, as it would
> > > -	 * otherwise return to the same address...
> > > +	 * otherwise return to the same address. Furthermore, pre-incrementing
> > > +	 * the PC before potentially exiting to userspace maintains the same
> > > +	 * abstraction for both SMCs and HVCs.
> >
> > 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.


> 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 <maz@kernel.org>
> 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.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  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(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst
> b/Documentation/virt/kvm/api.rst
> index c8ab2f730945..103f945959ed 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -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 */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h
> b/arch/arm64/include/uapi/asm/kvm.h
> index 3dcfa4bfdf83..b1c1edf85480 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -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
> 
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 9a35d6d18193..3b6523f25afc 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -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,


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

[Oliver Upton]

Hi Salil,

On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:

[...]

> > > 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? 

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.

-- 
Thanks,
Oliver

Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Marc Zyngier]

On Wed, 17 May 2023 19:38:14 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hi Salil,
> 
> On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
> 
> [...]
> 
> > > > 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? 
> 
> 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.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Salil Mehta]

Hi Oliver,

> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Wednesday, May 17, 2023 7:38 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: Marc Zyngier <maz@kernel.org>; kvmarm@lists.linux.dev;
> kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>; James Morse
> <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>; yuzenghui
> <yuzenghui@huawei.com>; Sean Christopherson <seanjc@google.com>
> Subject: 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:
> 
> [...]
> 
> > > > 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?
> 
> 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.

> 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.


> 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

RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Salil Mehta]

Hi Marc,

> From: Marc Zyngier <maz@kernel.org>
> Sent: Thursday, May 18, 2023 9:06 AM
> To: Oliver Upton <oliver.upton@linux.dev>
> Cc: Salil Mehta <salil.mehta@huawei.com>; kvmarm@lists.linux.dev;
> kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>; James Morse
> <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>; yuzenghui
> <yuzenghui@huawei.com>; Sean Christopherson <seanjc@google.com>
> Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support for
> KVM_EXIT_HYPERCALL
> 
> On Wed, 17 May 2023 19:38:14 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > Hi Salil,
> >
> > On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
> >
> > [...]
> >
> > > > > 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?
> >
> > 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.


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.


> 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

Re: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Marc Zyngier]

On Thu, 18 May 2023 10:08:46 +0100,
Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> Hi Marc,
> 
> > From: Marc Zyngier <maz@kernel.org>
> > Sent: Thursday, May 18, 2023 9:06 AM
> > To: Oliver Upton <oliver.upton@linux.dev>
> > Cc: Salil Mehta <salil.mehta@huawei.com>; kvmarm@lists.linux.dev;
> > kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>; James Morse
> > <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>; yuzenghui
> > <yuzenghui@huawei.com>; Sean Christopherson <seanjc@google.com>
> > Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support for
> > KVM_EXIT_HYPERCALL
> > 
> > On Wed, 17 May 2023 19:38:14 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > >
> > > Hi Salil,
> > >
> > > On Wed, May 17, 2023 at 06:00:18PM +0000, Salil Mehta wrote:
> > >
> > > [...]
> > >
> > > > > > 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?
> > >
> > > 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.
> 
> 
> 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.

> > 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.

-- 
Without deviation from the norm, progress is not possible.

RE: [PATCH v3 08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

[Salil Mehta]

Hi Marc,

> From: Marc Zyngier <maz@kernel.org>
> Sent: Thursday, May 18, 2023 10:43 AM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: Oliver Upton <oliver.upton@linux.dev>; kvmarm@lists.linux.dev;
> kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>; James Morse
> <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>; yuzenghui
> <yuzenghui@huawei.com>; Sean Christopherson <seanjc@google.com>
> Subject: Re: [PATCH v3 08/13] KVM: arm64: Add support for
> KVM_EXIT_HYPERCALL
> 

[...]


> > > > > > > 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?
> > > >
> > > > 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.
> >
> >
> > 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.
> 
> > > 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.


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.


> 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.