diff mbox

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

Message ID 20230404154050.2270077-1-oliver.upton@linux.dev (mailing list archive)
State New, archived
Headers show

Commit Message

Oliver Upton April 4, 2023, 3:40 p.m. UTC
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

+++ 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;
  
  	/*
+++ 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;
  	}
+++ 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);

Comments

Marc Zyngier April 5, 2023, 6:45 p.m. UTC | #1
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.
diff mbox

Patch

diff --cc arch/arm64/include/asm/kvm_host.h
index a8e2c52b44aa,2682b3fd0881..000000000000
--- a/arch/arm64/include/asm/kvm_host.h
diff --cc arch/arm64/kvm/arm.c
index 0e5a3ff8cc5a,efee032c9560..000000000000
--- a/arch/arm64/kvm/arm.c
diff --cc arch/arm64/kvm/pmu-emul.c
index 240168416838,a0fc569fdbca..000000000000
--- a/arch/arm64/kvm/pmu-emul.c
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;
 }