KVM: arm64: Confine protected VM vCPU state to EL2

If a userspace process creates a VM and a memory slot, and then immediately deletes it via KVM_SET_USER_MEMORY_REGION with a zero memory size before the first KVM_RUN, this triggers kvm_arch_flush_shadow_memslot().

This unconditionally calls kvm_tlb_flush_vmid_range(), which issues an HVC call that ends up in handle___kvm_tlb_flush_vmid_range():

Since this is the exact same sequence described in the commit message (a host pointer unmapped at EL2 being dereferenced before the first run), should we also pin the mmu pointer around these TLB flush operations?

pKVM doesn't support steal time for protected guests, and KVM_CAP_STEAL_TIME already reads 0 for them on the VM file descriptor, but kvm_arm_pvtime_supported() doesn't know the VM, so the host still accepts the KVM_ARM_VCPU_PVTIME_CTRL attribute, whose IPA would point kvm_update_stolen_time() at the guest's private memory on every vCPU load. Pass the VM in and return false for a protected one, so the attribute returns -ENXIO as it does where steal time isn't implemented.

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/include/asm/kvm_host.h |  2 +-
 arch/arm64/kvm/arm.c              |  2 +-
 arch/arm64/kvm/pvtime.c           | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -1

index 27fe0cd5b2d7a..286489a69dff5 100644
@@ -1346,7 +1346,7 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
 gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu);
 void kvm_update_stolen_time(struct kvm_vcpu *vcpu);
 
-bool kvm_arm_pvtime_supported(void);
+bool kvm_arm_pvtime_supported(struct kvm *kvm);
 int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
 			    struct kvm_device_attr *attr);
 int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,

arch/arm64/kvm/arm.c +1 -1

index 8b080804bc90b..db36815630790 100644
@@ -447,7 +447,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		r = system_supports_mte();
 		break;
 	case KVM_CAP_STEAL_TIME:
-		r = kvm_arm_pvtime_supported();
+		r = kvm_arm_pvtime_supported(kvm);
 		break;
 	case KVM_CAP_ARM_EL1_32BIT:
 		r = cpus_have_final_cap(ARM64_HAS_32BIT_EL1);

arch/arm64/kvm/pvtime.c +5 -5

index 4ceabaa4c30bd..053fe831fa541 100644
@@ -67,9 +67,9 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)
 	return base;
 }
 
-bool kvm_arm_pvtime_supported(void)
+bool kvm_arm_pvtime_supported(struct kvm *kvm)
 {
-	return !!sched_info_on();
+	return !!sched_info_on() && (!kvm || !kvm_vm_is_protected(kvm));
 }
 
 int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,

@@ -81,7 +81,7 @@ int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
 	int ret = 0;
 	int idx;
 
-	if (!kvm_arm_pvtime_supported() ||
+	if (!kvm_arm_pvtime_supported(kvm) ||
 	    attr->attr != KVM_ARM_VCPU_PVTIME_IPA)
 		return -ENXIO;
 

@@ -110,7 +110,7 @@ int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
 	u64 __user *user = (u64 __user *)attr->addr;
 	u64 ipa;
 
-	if (!kvm_arm_pvtime_supported() ||
+	if (!kvm_arm_pvtime_supported(vcpu->kvm) ||
 	    attr->attr != KVM_ARM_VCPU_PVTIME_IPA)
 		return -ENXIO;
 

@@ -126,7 +126,7 @@ int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
 {
 	switch (attr->attr) {
 	case KVM_ARM_VCPU_PVTIME_IPA:
-		if (kvm_arm_pvtime_supported())
+		if (kvm_arm_pvtime_supported(vcpu->kvm))
 			return 0;
 	}
 	return -ENXIO;

Signature · 1 line

2.39.5

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Early return in flush_debug_state leaves the protected guest's debug_owner as VCPU_DEBUG_FREE, completely bypassing hardware debug register context switching and exposing host physical debug registers to the guest.

[Severity: Critical]
Does this early return leave the protected guest's debug_owner as VCPU_DEBUG_FREE (its zero-initialized value) instead of synchronizing it? This could bypass hardware debug register context switching, potentially exposing host physical debug registers to the guest.

[Severity: Critical]
By moving the mdcr_el2 copy into the non-protected branch, does the protected guest retain the EL2-computed mdcr_el2 where TDA is deliberately cleared?

EL2 implements PSCI 1.1 for protected VMs: CPU_ON, CPU_OFF, PSCI_VERSION and PSCI_FEATURES are decided at EL2 (CPU_ON and CPU_OFF still exit to the host, which only schedules or parks the target), AFFINITY_INFO, CPU_SUSPEND and the platform power operations are forwarded to the host, and anything else returns NOT_SUPPORTED, including the TRNG calls and the functions above 1.1, SYSTEM_OFF2 among them, that the host handled for a protected guest until now. TRNG for protected guests is a follow-up. AFFINITY_INFO stays with the host, which returns OFF only once it has parked the target:
the host is what a guest polls to see a CPU_OFF complete before it issues the next CPU_ON, as Linux does on hotplug.

Three consequences follow:

- A protected VM has one primary vCPU, the first whose hyp vCPU is created with mp_state RUNNABLE. A second one, or an mp_state other than RUNNABLE or STOPPED, fails that vCPU's first KVM_RUN with -EINVAL.

- CPU_ON finds its target among the hyp vCPUs, which exist from the target's first KVM_RUN; before that the guest gets INVALID_PARAMETERS.

- A vCPU EL2 holds powered off doesn't run: handle___kvm_vcpu_run() returns ARM_EXCEPTION_IL, reported as KVM_EXIT_FAIL_ENTRY. Its existing bail-outs return the same code instead of an -EINVAL that handle_exit() didn't recognise, for every hyp vCPU.

Non-protected VMs keep power_state ON and accept any mp_state.

Each protected vCPU is OFF, ON_PENDING or ON. CPU_ON moves the target to ON_PENDING, and the target's next run resets it and moves it to ON. The racing transitions are cmpxchg, and the reset state is published with a release/acquire pair, documented at each site. CPU_OFF publishes OFF with a release, so the target's clear of reset_state.reset is ordered before it and a CPU_ON that then wins on OFF republishes after the clear. Rolling a CPU_ON the host failed back to OFF needs the host's return value, which the per-EC marshalling patch delivers along with the rollback. Until then such a target stays ON_PENDING, and the reset has no observable effect: flush_hyp_vcpu() copies the host's context in on every entry until that patch removes the copy, so the target enters on the host's values rather than the ones EL2 reset.

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/kvm/hyp/include/nvhe/pkvm.h |  14 ++
 arch/arm64/kvm/hyp/nvhe/hyp-main.c     |  25 ++-
 arch/arm64/kvm/hyp/nvhe/pkvm.c         | 283 ++++++++++++++++++++++++-
 3 files changed, 311 insertions(+), 11 deletions(-)

arch/arm64/kvm/hyp/include/nvhe/pkvm.h +14 -0

index a04b7c04d5135..63b368baf0e72 100644
@@ -29,6 +29,12 @@ struct pkvm_hyp_vcpu {
 
 	/* The previous exit's ARM_EXCEPTION_* code. */
 	u32 exit_code;
+
+	/*
+	 * PSCI_0_2_AFFINITY_LEVEL_{OFF, ON_PENDING, ON}. A non-protected
+	 * vCPU is always ON.
+	 */
+	int power_state;
 };
 
 /*

@@ -46,6 +52,12 @@ struct pkvm_hyp_vm {
 	struct hyp_pool pool;
 	hyp_spinlock_t lock;
 
+	/*
+	 * The vCPU initialised RUNNABLE: claimed under vm_table_lock,
+	 * released only if its own init fails.
+	 */
+	struct pkvm_hyp_vcpu *primary_vcpu;
+
 	/* Array of the hyp vCPU structures for this VM. */
 	struct pkvm_hyp_vcpu *vcpus[];
 };

@@ -98,4 +110,6 @@ void kvm_init_pvm_id_regs(struct kvm_vcpu *vcpu);
 void kvm_reset_pvm_sys_regs(struct kvm_vcpu *vcpu);
 int kvm_check_pvm_sysreg_table(void);
 
+int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu);
+struct pkvm_hyp_vcpu *pkvm_mpidr_to_hyp_vcpu(struct pkvm_hyp_vm *vm, u64 mpidr);
 #endif /* __ARM64_KVM_NVHE_PKVM_H__ */

arch/arm64/kvm/hyp/nvhe/hyp-main.c +19 -6

index 9cc8ef16897c1..da8ab636063cf 100644
@@ -8,6 +8,7 @@
 #include <hyp/switch.h>
 
 #include <linux/irqchip/arm-gic-v3.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/pgtable-types.h>
 #include <asm/kvm_asm.h>

@@ -456,14 +457,12 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu;
 	struct kvm_vcpu *host_vcpu;
-	int ret;
+	int ret = ARM_EXCEPTION_IL;
 
 	host_vcpu = get_host_hyp_vcpus(host_ctxt, 1, &hyp_vcpu);
 
-	if (!host_vcpu) {
-		ret = -EINVAL;
+	if (!host_vcpu)
 		goto out;
-	}
 
 	if (unlikely(hyp_vcpu)) {
 		/*

@@ -472,8 +471,22 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 		 * loading a vcpu. Therefore, if SME features enabled the host
 		 * is misbehaving.
 		 */
-		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR))) {
-			ret = -EINVAL;
+		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR)))
+			goto out;
+
+		/*
+		 * ON has a single writer, pkvm_reset_vcpu() on this CPU, so
+		 * READ_ONCE suffices. ON_PENDING takes the reset; -ECANCELED
+		 * is a rollback that raced it.
+		 */
+		switch (READ_ONCE(hyp_vcpu->power_state)) {
+		case PSCI_0_2_AFFINITY_LEVEL_ON:
+			break;
+		case PSCI_0_2_AFFINITY_LEVEL_ON_PENDING:
+			if (pkvm_reset_vcpu(hyp_vcpu))
+				goto out;
+			break;
+		default:
 			goto out;
 		}
 

arch/arm64/kvm/hyp/nvhe/pkvm.c +40 -1

index 855cb77c8bba1..d970cba12ca47 100644
@@ -5,6 +5,7 @@
  */
 
 #include <kvm/arm_hypercalls.h>
+#include <kvm/arm_psci.h>
 
 #include <linux/kvm_host.h>
 #include <linux/mm.h>

@@ -433,6 +434,40 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
 		   allowed_features, KVM_VCPU_MAX_FEATURES);
 }
 
+static int pkvm_vcpu_init_psci(struct pkvm_hyp_vcpu *hyp_vcpu, u32 mp_state)
+{
+	struct vcpu_reset_state *reset_state = &hyp_vcpu->vcpu.arch.reset_state;
+	struct pkvm_hyp_vm *hyp_vm = pkvm_hyp_vcpu_to_hyp_vm(hyp_vcpu);
+	struct kvm_vcpu *host_vcpu;
+
+	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
+		/* The host manages a non-protected vCPU: always ON at EL2. */
+		hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_ON;
+		return 0;
+	}
+
+	if (mp_state != KVM_MP_STATE_RUNNABLE && mp_state != KVM_MP_STATE_STOPPED)
+		return -EINVAL;
+
+	if (mp_state == KVM_MP_STATE_STOPPED) {
+		reset_state->reset = false;
+		hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_OFF;
+		return 0;
+	}
+
+	hyp_assert_lock_held(&vm_table_lock);
+	if (hyp_vm->primary_vcpu)
+		return -EINVAL;
+	hyp_vm->primary_vcpu = hyp_vcpu;
+
+	host_vcpu = hyp_vcpu->host_vcpu;
+	reset_state->pc = READ_ONCE(host_vcpu->arch.ctxt.regs.pc);
+	reset_state->r0 = READ_ONCE(host_vcpu->arch.ctxt.regs.regs[0]);
+	reset_state->reset = true;
+	hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_ON_PENDING;
+	return 0;
+}
+
 static void unpin_host_vcpu(struct kvm_vcpu *host_vcpu)
 {
 	if (host_vcpu)

@@ -447,6 +482,9 @@ static void unpin_host_sve_state(struct pkvm_hyp_vcpu *hyp_vcpu)
 		return;
 
 	sve_state = hyp_vcpu->vcpu.arch.sve_state;
+	if (!sve_state)
+		return;
+
 	hyp_unpin_shared_mem(sve_state,
 			     sve_state + vcpu_sve_state_size(&hyp_vcpu->vcpu));
 }

@@ -559,10 +597,12 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 			      struct kvm_vcpu *host_vcpu)
 {
 	int ret = 0;
+	u32 mp_state;
 
 	if (hyp_pin_shared_mem(host_vcpu, host_vcpu + 1))
 		return -EBUSY;
 
+	mp_state = READ_ONCE(host_vcpu->arch.mp_state.mp_state);
 	hyp_vcpu->host_vcpu = host_vcpu;
 
 	hyp_vcpu->vcpu.kvm = &hyp_vm->kvm;

@@ -571,7 +611,6 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 
 	hyp_vcpu->vcpu.arch.hw_mmu = &hyp_vm->kvm.arch.mmu;
 	hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
-	hyp_vcpu->vcpu.arch.mp_state.mp_state = KVM_MP_STATE_STOPPED;
 
 	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
 		/*

arch/arm64/kvm/hyp/nvhe/pkvm.c +234 -3

 	return ret;
 }
 

@@ -980,13 +1022,22 @@ int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu,
 
 	ret = init_pkvm_hyp_vcpu(hyp_vcpu, hyp_vm, host_vcpu);
 	if (ret)
-		goto unlock;
+		goto unclaim;
 
 	ret = register_hyp_vcpu(hyp_vm, hyp_vcpu);
 	if (ret) {
 		unpin_host_vcpu(host_vcpu);
 		unpin_host_sve_state(hyp_vcpu);
+		goto unclaim;
 	}
+	goto unlock;
+unclaim:
+	/*
+	 * Under vm_table_lock, so no other claim can have landed: undo
+	 * this one.
+	 */
+	if (hyp_vm->primary_vcpu == hyp_vcpu)
+		hyp_vm->primary_vcpu = NULL;
 unlock:
 	hyp_spin_unlock(&vm_table_lock);
 

@@ -1178,6 +1229,228 @@ static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu)
 		ret[0] = SMCCC_RET_SUCCESS;
 }
 
+/*
+ * Reset the vCPU to its power-on state and commit ON_PENDING -> ON, on the
+ * target's own CPU. Returns -ECANCELED, with no side effects, if a rollback
+ * raced the reset.
+ */
+int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct vcpu_reset_state *reset_state = &hyp_vcpu->vcpu.arch.reset_state;
+	int prev;
+
+	/*
+	 * Pairs with smp_store_release(&reset_state->reset, true) in
+	 * pvm_psci_vcpu_on(). The acquire must precede the cmpxchg: reversed, a
+	 * winning cmpxchg with a false acquire would leave power_state == ON
+	 * with the reset skipped.
+	 */
+	if (!smp_load_acquire(&reset_state->reset))
+		return -ECANCELED;
+
+	prev = cmpxchg_relaxed(&hyp_vcpu->power_state,
+			       PSCI_0_2_AFFINITY_LEVEL_ON_PENDING,
+			       PSCI_0_2_AFFINITY_LEVEL_ON);
+	if (prev != PSCI_0_2_AFFINITY_LEVEL_ON_PENDING) {
+		/* The only other writer of ON_PENDING is the rollback. */
+		WARN_ON(prev != PSCI_0_2_AFFINITY_LEVEL_OFF);
+		return -ECANCELED;
+	}
+
+	kvm_reset_vcpu_core(&hyp_vcpu->vcpu);
+	kvm_reset_pvm_sys_regs(&hyp_vcpu->vcpu);
+
+	/* Must be done after resetting sys registers. */
+	kvm_reset_vcpu_psci(&hyp_vcpu->vcpu, reset_state);
+
+	hyp_vcpu->exit_code = 0;
+	/*
+	 * A source that raced a rollback can publish after this clear, and
+	 * the next cycle then passes the acquire on a stale flag: the pc,
+	 * r0 and be it reads are still the guest's own.
+	 */
+	reset_state->reset = false;
+	return 0;
+}
+
+struct pkvm_hyp_vcpu *pkvm_mpidr_to_hyp_vcpu(struct pkvm_hyp_vm *hyp_vm,
+					     u64 mpidr)
+{
+	struct pkvm_hyp_vcpu *hyp_vcpu;
+	int i;
+
+	mpidr &= MPIDR_HWID_BITMASK;
+
+	for (i = 0; i < hyp_vm->kvm.created_vcpus; i++) {
+		/* Pairs with smp_store_release() in register_hyp_vcpu(). */
+		hyp_vcpu = smp_load_acquire(&hyp_vm->vcpus[i]);
+
+		if (!hyp_vcpu)
+			continue;
+
+		if (mpidr == kvm_vcpu_get_mpidr_aff(&hyp_vcpu->vcpu))
+			return hyp_vcpu;
+	}
+
+	return NULL;
+}
+
+/*
+ * Returns true when handled at EL2, false when the host must wake the target
+ * vCPU.
+ */
+static bool pvm_psci_vcpu_on(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct pkvm_hyp_vm *hyp_vm = pkvm_hyp_vcpu_to_hyp_vm(hyp_vcpu);
+	struct vcpu_reset_state *reset_state;
+	struct pkvm_hyp_vcpu *target;
+	unsigned long cpu_id, ret;
+	int power_state;
+
+	cpu_id = smccc_get_arg1(&hyp_vcpu->vcpu);
+	if (!kvm_psci_valid_affinity(&hyp_vcpu->vcpu, cpu_id)) {
+		ret = PSCI_RET_INVALID_PARAMS;
+		goto error;
+	}
+
+	target = pkvm_mpidr_to_hyp_vcpu(hyp_vm, cpu_id);
+	if (!target) {
+		ret = PSCI_RET_INVALID_PARAMS;
+		goto error;
+	}
+
+	/*
+	 * vCPUs race to power on the same target. Relaxed: reset_state
+	 * is published by the release on reset_state.reset below.
+	 */
+	power_state = cmpxchg_relaxed(&target->power_state,
+				      PSCI_0_2_AFFINITY_LEVEL_OFF,
+				      PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);
+	switch (power_state) {
+	case PSCI_0_2_AFFINITY_LEVEL_ON_PENDING:
+		ret = PSCI_RET_ON_PENDING;
+		goto error;
+	case PSCI_0_2_AFFINITY_LEVEL_ON:
+		ret = PSCI_RET_ALREADY_ON;
+		goto error;
+	case PSCI_0_2_AFFINITY_LEVEL_OFF:
+		break;
+	default:
+		ret = PSCI_RET_INTERNAL_FAILURE;
+		goto error;
+	}
+
+	reset_state = &target->vcpu.arch.reset_state;
+	reset_state->pc = smccc_get_arg2(&hyp_vcpu->vcpu);
+	reset_state->r0 = smccc_get_arg3(&hyp_vcpu->vcpu);
+	reset_state->be = kvm_vcpu_is_be(&hyp_vcpu->vcpu);
+	/*
+	 * Publish reset_state.{pc, r0, be} to the target vCPU. Pairs with
+	 * smp_load_acquire(&reset_state->reset) in pkvm_reset_vcpu().
+	 */
+	smp_store_release(&reset_state->reset, true);
+
+	/* The host requests KVM_REQ_VCPU_RESET and wakes the target. */
+	return false;
+
+error:
+	smccc_set_retval(&hyp_vcpu->vcpu, ret, 0, 0, 0);
+	return true;
+}
+
+/*
+ * Returns true when handled at EL2, false when the host must stop scheduling
+ * the vCPU.
+ */
+static bool pvm_psci_vcpu_off(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	/* No other writer runs while this vCPU is ON and executing. */
+	WARN_ON(READ_ONCE(hyp_vcpu->power_state) != PSCI_0_2_AFFINITY_LEVEL_ON);
+
+	/*
+	 * Orders pkvm_reset_vcpu()'s clear of reset_state.reset before OFF, so
+	 * a CPU_ON that wins on OFF republishes after it. Pairs with the
+	 * cmpxchg in pvm_psci_vcpu_on().
+	 */
+	smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);
+
+	/* Return to the host so that it can finish powering off the vcpu. */
+	return false;
+}
+
+static bool pvm_psci_version(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	/* Nothing to be handled by the host. Go back to the guest. */
+	smccc_set_retval(&hyp_vcpu->vcpu, KVM_ARM_PSCI_1_1, 0, 0, 0);
+	return true;
+}
+
+static bool pvm_psci_features(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *vcpu = &hyp_vcpu->vcpu;
+	u32 feature = smccc_get_arg1(vcpu);
+	unsigned long val;
+
+	switch (feature) {
+	case PSCI_0_2_FN_PSCI_VERSION:
+	case PSCI_0_2_FN_CPU_SUSPEND:
+	case PSCI_0_2_FN64_CPU_SUSPEND:
+	case PSCI_0_2_FN_CPU_OFF:
+	case PSCI_0_2_FN_CPU_ON:
+	case PSCI_0_2_FN64_CPU_ON:
+	case PSCI_0_2_FN_AFFINITY_INFO:
+	case PSCI_0_2_FN64_AFFINITY_INFO:
+	case PSCI_0_2_FN_SYSTEM_OFF:
+	case PSCI_0_2_FN_SYSTEM_RESET:
+	case PSCI_1_0_FN_PSCI_FEATURES:
+	case PSCI_1_1_FN_SYSTEM_RESET2:
+	case PSCI_1_1_FN64_SYSTEM_RESET2:
+	case ARM_SMCCC_VERSION_FUNC_ID:
+		val = PSCI_RET_SUCCESS;
+		break;
+	default:
+		val = PSCI_RET_NOT_SUPPORTED;
+		break;
+	}
+
+	/* Nothing to be handled by the host. Go back to the guest. */
+	smccc_set_retval(vcpu, val, 0, 0, 0);
+	return true;
+}
+
+static bool pkvm_handle_psci(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *vcpu = &hyp_vcpu->vcpu;
+	u32 psci_fn = smccc_get_function(vcpu);
+
+	switch (psci_fn) {
+	case PSCI_0_2_FN_CPU_ON:
+		kvm_psci_narrow_to_32bit(vcpu);
+		fallthrough;
+	case PSCI_0_2_FN64_CPU_ON:
+		return pvm_psci_vcpu_on(hyp_vcpu);
+	case PSCI_0_2_FN_CPU_OFF:
+		return pvm_psci_vcpu_off(hyp_vcpu);
+	case PSCI_0_2_FN_PSCI_VERSION:
+		return pvm_psci_version(hyp_vcpu);
+	case PSCI_1_0_FN_PSCI_FEATURES:
+		return pvm_psci_features(hyp_vcpu);
+	case PSCI_0_2_FN_AFFINITY_INFO:
+	case PSCI_0_2_FN64_AFFINITY_INFO:
+	case PSCI_0_2_FN_SYSTEM_RESET:
+	case PSCI_0_2_FN_CPU_SUSPEND:
+	case PSCI_0_2_FN64_CPU_SUSPEND:
+	case PSCI_0_2_FN_SYSTEM_OFF:
+	case PSCI_1_1_FN_SYSTEM_RESET2:
+	case PSCI_1_1_FN64_SYSTEM_RESET2:
+		return false; /* Handled by the host. */
+	default:
+		/* Unknown PSCI calls are answered here, not forwarded. */
+		smccc_set_retval(vcpu, PSCI_RET_NOT_SUPPORTED, 0, 0, 0);
+		return true;
+	}
+}
+
 /*
  * Handler for protected VM HVC calls.
  *

@@ -1187,6 +1460,7 @@ static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu)
  */
 bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)
 {
+	struct pkvm_hyp_vcpu *hyp_vcpu = container_of(vcpu, struct pkvm_hyp_vcpu, vcpu);
 	u64 val[4] = { SMCCC_RET_INVALID_PARAMETER };
 	bool handled = true;
 	u32 feature;

@@ -1285,8 +1559,7 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code)
 		pkvm_memunshare_call(val, vcpu);
 		break;
 	default:
-		/* Punt everything else back to the host, for now. */
-		handled = false;
+		return pkvm_handle_psci(hyp_vcpu);
 	}
 
 	if (handled)

Signature · 1 line

2.39.5

Quoted 227 lines · Fuad Tabba

EL2 implements PSCI 1.1 for protected VMs: CPU_ON, CPU_OFF, PSCI_VERSION and PSCI_FEATURES are decided at EL2 (CPU_ON and CPU_OFF still exit to the host, which only schedules or parks the target), AFFINITY_INFO, CPU_SUSPEND and the platform power operations are forwarded to the host, and anything else returns NOT_SUPPORTED, including the TRNG calls and the functions above 1.1, SYSTEM_OFF2 among them, that the host handled for a protected guest until now. TRNG for protected guests is a follow-up. AFFINITY_INFO stays with the host, which returns OFF only once it has parked the target:
the host is what a guest polls to see a CPU_OFF complete before it issues the next CPU_ON, as Linux does on hotplug.

Three consequences follow:

- A protected VM has one primary vCPU, the first whose hyp vCPU is created with mp_state RUNNABLE. A second one, or an mp_state other than RUNNABLE or STOPPED, fails that vCPU's first KVM_RUN with -EINVAL.

- CPU_ON finds its target among the hyp vCPUs, which exist from the target's first KVM_RUN; before that the guest gets INVALID_PARAMETERS.

- A vCPU EL2 holds powered off doesn't run: handle___kvm_vcpu_run() returns ARM_EXCEPTION_IL, reported as KVM_EXIT_FAIL_ENTRY. Its existing bail-outs return the same code instead of an -EINVAL that handle_exit() didn't recognise, for every hyp vCPU.

Non-protected VMs keep power_state ON and accept any mp_state.

Each protected vCPU is OFF, ON_PENDING or ON. CPU_ON moves the target to ON_PENDING, and the target's next run resets it and moves it to ON. The racing transitions are cmpxchg, and the reset state is published with a release/acquire pair, documented at each site. CPU_OFF publishes OFF with a release, so the target's clear of reset_state.reset is ordered before it and a CPU_ON that then wins on OFF republishes after the clear. Rolling a CPU_ON the host failed back to OFF needs the host's return value, which the per-EC marshalling patch delivers along with the rollback. Until then such a target stays ON_PENDING, and the reset has no observable effect: flush_hyp_vcpu() copies the host's context in on every entry until that patch removes the copy, so the target enters on the host's values rather than the ones EL2 reset.

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/kvm/hyp/include/nvhe/pkvm.h |  14 ++
 arch/arm64/kvm/hyp/nvhe/hyp-main.c     |  25 ++-
 arch/arm64/kvm/hyp/nvhe/pkvm.c         | 283 ++++++++++++++++++++++++-
 3 files changed, 311 insertions(+), 11 deletions(-)

arch/arm64/kvm/hyp/include/nvhe/pkvm.h +14 -0

index a04b7c04d5135..63b368baf0e72 100644
@@ -29,6 +29,12 @@ struct pkvm_hyp_vcpu {
 
 	/* The previous exit's ARM_EXCEPTION_* code. */
 	u32 exit_code;
+
+	/*
+	 * PSCI_0_2_AFFINITY_LEVEL_{OFF, ON_PENDING, ON}. A non-protected
+	 * vCPU is always ON.
+	 */
+	int power_state;
 };
 
 /*

@@ -46,6 +52,12 @@ struct pkvm_hyp_vm {
 	struct hyp_pool pool;
 	hyp_spinlock_t lock;
 
+	/*
+	 * The vCPU initialised RUNNABLE: claimed under vm_table_lock,
+	 * released only if its own init fails.
+	 */
+	struct pkvm_hyp_vcpu *primary_vcpu;
+
 	/* Array of the hyp vCPU structures for this VM. */
 	struct pkvm_hyp_vcpu *vcpus[];
 };

@@ -98,4 +110,6 @@ void kvm_init_pvm_id_regs(struct kvm_vcpu *vcpu);
 void kvm_reset_pvm_sys_regs(struct kvm_vcpu *vcpu);
 int kvm_check_pvm_sysreg_table(void);
 
+int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu);
+struct pkvm_hyp_vcpu *pkvm_mpidr_to_hyp_vcpu(struct pkvm_hyp_vm *vm, u64 mpidr);
 #endif /* __ARM64_KVM_NVHE_PKVM_H__ */

arch/arm64/kvm/hyp/nvhe/hyp-main.c +19 -6

index 9cc8ef16897c1..da8ab636063cf 100644
@@ -8,6 +8,7 @@
 #include <hyp/switch.h>
 
 #include <linux/irqchip/arm-gic-v3.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/pgtable-types.h>
 #include <asm/kvm_asm.h>

@@ -456,14 +457,12 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu;
 	struct kvm_vcpu *host_vcpu;
-	int ret;
+	int ret = ARM_EXCEPTION_IL;
 
 	host_vcpu = get_host_hyp_vcpus(host_ctxt, 1, &hyp_vcpu);
 
-	if (!host_vcpu) {
-		ret = -EINVAL;
+	if (!host_vcpu)
 		goto out;
-	}
 
 	if (unlikely(hyp_vcpu)) {
 		/*

@@ -472,8 +471,22 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 		 * loading a vcpu. Therefore, if SME features enabled the host
 		 * is misbehaving.
 		 */
-		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR))) {
-			ret = -EINVAL;
+		if (unlikely(system_supports_sme() && read_sysreg_s(SYS_SVCR)))
+			goto out;
+
+		/*
+		 * ON has a single writer, pkvm_reset_vcpu() on this CPU, so
+		 * READ_ONCE suffices. ON_PENDING takes the reset; -ECANCELED
+		 * is a rollback that raced it.
+		 */
+		switch (READ_ONCE(hyp_vcpu->power_state)) {
+		case PSCI_0_2_AFFINITY_LEVEL_ON:
+			break;
+		case PSCI_0_2_AFFINITY_LEVEL_ON_PENDING:
+			if (pkvm_reset_vcpu(hyp_vcpu))
+				goto out;
+			break;
+		default:
 			goto out;
 		}
 

arch/arm64/kvm/hyp/nvhe/pkvm.c +40 -1

index 855cb77c8bba1..d970cba12ca47 100644
@@ -5,6 +5,7 @@
  */
 
 #include <kvm/arm_hypercalls.h>
+#include <kvm/arm_psci.h>
 
 #include <linux/kvm_host.h>
 #include <linux/mm.h>

@@ -433,6 +434,40 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
 		   allowed_features, KVM_VCPU_MAX_FEATURES);
 }
 
+static int pkvm_vcpu_init_psci(struct pkvm_hyp_vcpu *hyp_vcpu, u32 mp_state)
+{
+	struct vcpu_reset_state *reset_state = &hyp_vcpu->vcpu.arch.reset_state;
+	struct pkvm_hyp_vm *hyp_vm = pkvm_hyp_vcpu_to_hyp_vm(hyp_vcpu);
+	struct kvm_vcpu *host_vcpu;
+
+	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
+		/* The host manages a non-protected vCPU: always ON at EL2. */
+		hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_ON;
+		return 0;
+	}
+
+	if (mp_state != KVM_MP_STATE_RUNNABLE && mp_state != KVM_MP_STATE_STOPPED)
+		return -EINVAL;
+
+	if (mp_state == KVM_MP_STATE_STOPPED) {
+		reset_state->reset = false;
+		hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_OFF;
+		return 0;
+	}
+
+	hyp_assert_lock_held(&vm_table_lock);
+	if (hyp_vm->primary_vcpu)
+		return -EINVAL;
+	hyp_vm->primary_vcpu = hyp_vcpu;
+
+	host_vcpu = hyp_vcpu->host_vcpu;
+	reset_state->pc = READ_ONCE(host_vcpu->arch.ctxt.regs.pc);
+	reset_state->r0 = READ_ONCE(host_vcpu->arch.ctxt.regs.regs[0]);
+	reset_state->reset = true;
+	hyp_vcpu->power_state = PSCI_0_2_AFFINITY_LEVEL_ON_PENDING;
+	return 0;
+}
+
 static void unpin_host_vcpu(struct kvm_vcpu *host_vcpu)
 {
 	if (host_vcpu)

@@ -447,6 +482,9 @@ static void unpin_host_sve_state(struct pkvm_hyp_vcpu *hyp_vcpu)
 		return;
 
 	sve_state = hyp_vcpu->vcpu.arch.sve_state;
+	if (!sve_state)
+		return;
+
 	hyp_unpin_shared_mem(sve_state,
 			     sve_state + vcpu_sve_state_size(&hyp_vcpu->vcpu));
 }

@@ -559,10 +597,12 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 			      struct kvm_vcpu *host_vcpu)
 {
 	int ret = 0;
+	u32 mp_state;
 
 	if (hyp_pin_shared_mem(host_vcpu, host_vcpu + 1))
 		return -EBUSY;
 
+	mp_state = READ_ONCE(host_vcpu->arch.mp_state.mp_state);
 	hyp_vcpu->host_vcpu = host_vcpu;
 
 	hyp_vcpu->vcpu.kvm = &hyp_vm->kvm;

@@ -571,7 +611,6 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 
 	hyp_vcpu->vcpu.arch.hw_mmu = &hyp_vm->kvm.arch.mmu;
 	hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
-	hyp_vcpu->vcpu.arch.mp_state.mp_state = KVM_MP_STATE_STOPPED;
 
 	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
 		/*

@@ -601,9 +640,12 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
 

arch/arm64/kvm/hyp/nvhe/pkvm.c static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu, · line 648

Quoted 9 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +4 -1

 	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu))
 		kvm_reset_pvm_sys_regs(&hyp_vcpu->vcpu);
+	ret = pkvm_vcpu_init_psci(hyp_vcpu, mp_state);
 done:
-	if (ret)
+	if (ret) {
 		unpin_host_vcpu(host_vcpu);
+		unpin_host_sve_state(hyp_vcpu);
+	}

Was it planned for a different commit of the series?

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +0 -0

 	return ret;
 }
 

EL2 implements PSCI 1.1 for protected VMs: CPU_ON, CPU_OFF, PSCI_VERSION and PSCI_FEATURES are decided at EL2 (CPU_ON and CPU_OFF still exit to the host, which only schedules or parks the target), AFFINITY_INFO, CPU_SUSPEND and the platform power operations are forwarded to the host, and anything else returns NOT_SUPPORTED, including the TRNG calls and the functions above 1.1, SYSTEM_OFF2 among them, that the host handled for a protected guest until now. TRNG for protected guests is a follow-up. AFFINITY_INFO stays with the host, which returns OFF only once it has parked the target:
the host is what a guest polls to see a CPU_OFF complete before it issues the next CPU_ON, as Linux does on hotplug.

Three consequences follow:

- A protected VM has one primary vCPU, the first whose hyp vCPU is created with mp_state RUNNABLE. A second one, or an mp_state other than RUNNABLE or STOPPED, fails that vCPU's first KVM_RUN with -EINVAL.

- CPU_ON finds its target among the hyp vCPUs, which exist from the target's first KVM_RUN; before that the guest gets INVALID_PARAMETERS.

- A vCPU EL2 holds powered off doesn't run: handle___kvm_vcpu_run() returns ARM_EXCEPTION_IL, reported as KVM_EXIT_FAIL_ENTRY. Its existing bail-outs return the same code instead of an -EINVAL that handle_exit() didn't recognise, for every hyp vCPU.

Non-protected VMs keep power_state ON and accept any mp_state.

Each protected vCPU is OFF, ON_PENDING or ON. CPU_ON moves the target to ON_PENDING, and the target's next run resets it and moves it to ON. The racing transitions are cmpxchg, and the reset state is published with a release/acquire pair, documented at each site. CPU_OFF publishes OFF with a release, so the target's clear of reset_state.reset is ordered before it and a CPU_ON that then wins on OFF republishes after the clear. Rolling a CPU_ON the host failed back to OFF needs the host's return value, which the per-EC marshalling patch delivers along with the rollback. Until then such a target stays ON_PENDING, and the reset has no observable effect: flush_hyp_vcpu() copies the host's context in on every entry until that patch removes the copy, so the target enters on the host's values rather than the ones EL2 reset.

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/kvm/hyp/include/nvhe/pkvm.h |  14 ++
 arch/arm64/kvm/hyp/nvhe/hyp-main.c     |  25 ++-
 arch/arm64/kvm/hyp/nvhe/pkvm.c         | 283 ++++++++++++++++++++++++-
 3 files changed, 311 insertions(+), 11 deletions(-)

arch/arm64/kvm/hyp/nvhe/pkvm.c +69 -0

+
+/*
+ * Returns true when handled at EL2, false when the host must wake the target
+ * vCPU.
+ */
+static bool pvm_psci_vcpu_on(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct pkvm_hyp_vm *hyp_vm = pkvm_hyp_vcpu_to_hyp_vm(hyp_vcpu);
+	struct vcpu_reset_state *reset_state;
+	struct pkvm_hyp_vcpu *target;
+	unsigned long cpu_id, ret;
+	int power_state;
+
+	cpu_id = smccc_get_arg1(&hyp_vcpu->vcpu);
+	if (!kvm_psci_valid_affinity(&hyp_vcpu->vcpu, cpu_id)) {
+		ret = PSCI_RET_INVALID_PARAMS;
+		goto error;
+	}
+
+	target = pkvm_mpidr_to_hyp_vcpu(hyp_vm, cpu_id);
+	if (!target) {
+		ret = PSCI_RET_INVALID_PARAMS;
+		goto error;
+	}
+
+	/*
+	 * vCPUs race to power on the same target. Relaxed: reset_state
+	 * is published by the release on reset_state.reset below.
+	 */
+	power_state = cmpxchg_relaxed(&target->power_state,
+				      PSCI_0_2_AFFINITY_LEVEL_OFF,
+				      PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);
+	switch (power_state) {
+	case PSCI_0_2_AFFINITY_LEVEL_ON_PENDING:
+		ret = PSCI_RET_ON_PENDING;
+		goto error;
+	case PSCI_0_2_AFFINITY_LEVEL_ON:
+		ret = PSCI_RET_ALREADY_ON;
+		goto error;
+	case PSCI_0_2_AFFINITY_LEVEL_OFF:
+		break;
+	default:
+		ret = PSCI_RET_INTERNAL_FAILURE;
+		goto error;
+	}
+
+	reset_state = &target->vcpu.arch.reset_state;
+	reset_state->pc = smccc_get_arg2(&hyp_vcpu->vcpu);
+	reset_state->r0 = smccc_get_arg3(&hyp_vcpu->vcpu);
+	reset_state->be = kvm_vcpu_is_be(&hyp_vcpu->vcpu);
+	/*
+	 * Publish reset_state.{pc, r0, be} to the target vCPU. Pairs with
+	 * smp_load_acquire(&reset_state->reset) in pkvm_reset_vcpu().
+	 */
+	smp_store_release(&reset_state->reset, true);
+
+	/* The host requests KVM_REQ_VCPU_RESET and wakes the target. */
+	return false;
+
+error:
+	smccc_set_retval(&hyp_vcpu->vcpu, ret, 0, 0, 0);
+	return true;
+}
+
+/*
+ * Returns true when handled at EL2, false when the host must stop scheduling
+ * the vCPU.
+ */
+static bool pvm_psci_vcpu_off(struct pkvm_hyp_vcpu *hyp_vcpu)

arch/arm64/kvm/hyp/nvhe/pkvm.c +5 -0

+
+	/* Return to the host so that it can finish powering off the vcpu. */
+	return false;
+}
+

Quoted 10 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +10 -0

+/*
+ * Returns true when handled at EL2, false when the host must stop scheduling
+ * the vCPU.
+ */
+static bool pvm_psci_vcpu_off(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+        /* No other writer runs while this vCPU is ON and executing. */
+        WARN_ON(READ_ONCE(hyp_vcpu->power_state) != PSCI_0_2_AFFINITY_LEVEL_ON);
+
+        /*

arch/arm64/kvm/hyp/nvhe/pkvm.c static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu) · line 1375

Quoted 5 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +5 -0

+         * Orders pkvm_reset_vcpu()'s clear of reset_state.reset before OFF, so
+         * a CPU_ON that wins on OFF republishes after it. Pairs with the
+         * cmpxchg in pvm_psci_vcpu_on().
+         */
+        smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);

Is there an issue either with the comment or with pvm_psci_vcpu_on()? the cmpxchg is relaxed. I would have expected cmpxchg_acquire().

Hi Vincent,

Quoted 10 lines

Quoted 10 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +10 -0

+/*
+ * Returns true when handled at EL2, false when the host must stop scheduling
+ * the vCPU.
+ */
+static bool pvm_psci_vcpu_off(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+        /* No other writer runs while this vCPU is ON and executing. */
+        WARN_ON(READ_ONCE(hyp_vcpu->power_state) != PSCI_0_2_AFFINITY_LEVEL_ON);
+
+        /*

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu) · line 1375

Quoted 5 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +5 -0

+         * Orders pkvm_reset_vcpu()'s clear of reset_state.reset before OFF, so
+         * a CPU_ON that wins on OFF republishes after it. Pairs with the
+         * cmpxchg in pvm_psci_vcpu_on().
+         */
+        smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);

Is there an issue either with the comment or with pvm_psci_vcpu_on()? the cmpxchg is relaxed. I would have expected cmpxchg_acquire().

It's the comment. The ordering it describes holds with the relaxed cmpxchg. The release of OFF orders the clear before OFF. The winner's smp_store_release(&reset_state->reset, true) orders its cmpxchg before that store. So the clear can't land after the republish. The comment should name that store-release as the pair, not the cmpxchg.

Sorry, but I'm really confused by this and it appears to be different to what we've got in Android as well. Why do we need release semantics for the store to 'hyp_vcpu->power_state' in pvm_psci_vcpu_off()? What is it that we are publishing here? The comment talks about pkvm_reset_vcpu(), but how is that relevant to the vCPU _off_ path? You say the comment is wrong, but what _should_ it say?

I'm a bit baffled!

On Thu, 24 Sep 2026 09:30:06 +0100, Will Deacon <[email protected]> wrote:
[...]

Quoted 7 lines

Sorry, but I'm really confused by this and it appears to be different to what we've got in Android as well. Why do we need release semantics for the store to 'hyp_vcpu->power_state' in pvm_psci_vcpu_off()? What is it that we are publishing here? The comment talks about pkvm_reset_vcpu(), but how is that relevant to the vCPU _off_ path? You say the comment is wrong, but what _should_ it say?

Quoted 1 line

I'm a bit baffled!

It's not publishing data, it's handing reset_state back.

The CPU_ON winner writes reset_state.{pc, r0, be}, then reset_state.reset with a release. The target reads them and clears reset in pkvm_reset_vcpu() on its next run

and its CPU_OFF hands reset_state on to the next
CPU_ON. The release on OFF orders those reads and that clear before OFF, and an acquire on the winner's cmpxchg orders its writes after
it: release on the way out, acquire on the way in, like a lock.

Without the release, the target's clear of reset can become visible after the next winner's reset = true, and the target's next pkvm_reset_vcpu() then reads a clear flag and returns -ECANCELED, leaving the vCPU stuck at ON_PENDING.

The comment described the flag ordering, which holds through the winner's release on reset even with a relaxed cmpxchg, and named the cmpxchg as its pair when the cmpxchg wasn't an acquire. The acquire is for the winner's plain writes of pc/r0/be: nothing else orders them after the target's reads. v4 has cmpxchg_acquire() and the two comments name what's ordered and each other:

        /*
         * Orders pkvm_reset_vcpu()'s accesses to reset_state before OFF. Pairs
         * with the acquire cmpxchg in pvm_psci_vcpu_on().
         */
        smp_store_release(&hyp_vcpu->power_state, PSCI_0_2_AFFINITY_LEVEL_OFF);

and in pvm_psci_vcpu_on():

        /*
         * vCPUs race to power on the same target. The acquire pairs with the
         * release of OFF in pvm_psci_vcpu_off(): the target's accesses to
         * reset_state in pkvm_reset_vcpu() precede the writes below.
         */
        power_state = cmpxchg_acquire(&target->power_state,
                                      PSCI_0_2_AFFINITY_LEVEL_OFF,
                                      PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);

Quoted 1 line

It's not publishing data, it's handing reset_state back.

What's the difference? The usual pattern for acquire/release is:

<write data>
<store-release flag>

on one CPU and then on another:

<load-acquire flag> // If this reads from the release above...
<read data> // ... then this is guaranteed to read the written data

That's a message-passing shape and you would normally say that the first CPU (the producer) is publishing the data to the other CPU (the consumer).

Is this what is happening with the 'reset_state' (data) and the 'power_state' (flag)? If not, then what is the shape?

Quoted 3 lines

The CPU_ON winner writes reset_state.{pc, r0, be}, then reset_state.reset with a release. The target reads them and clears reset in pkvm_reset_vcpu() on its next run

By 'next run' you mean, at EL2 on the entry path into the guest following a successful CPU_ON operation?

Quoted 4 lines

and its CPU_OFF hands reset_state on to the next
CPU_ON. The release on OFF orders those reads and that clear before OFF, and an acquire on the winner's cmpxchg orders its writes after
it: release on the way out, acquire on the way in, like a lock.

That doesn't make sense to me, sorry. You're saying that the release store in the EL2 CPU_OFF hypercall handler is ordering stores that were made during the initial CPU_ON handling on that vCPU? Since then, we've been in and out of the guest. We really shouldn't need extra barriers to create order there.

Quoted 4 lines

Without the release, the target's clear of reset can become visible after the next winner's reset = true, and the target's next pkvm_reset_vcpu() then reads a clear flag and returns -ECANCELED, leaving the vCPU stuck at ON_PENDING.

This needs a litmus test because I can't see it myself. As above, CPU_OFF does not clear the reset state, so it's bizarre to put the release there.

Quoted 10 lines

and in pvm_psci_vcpu_on():

        /*
         * vCPUs race to power on the same target. The acquire pairs with the
         * release of OFF in pvm_psci_vcpu_off(): the target's accesses to
         * reset_state in pkvm_reset_vcpu() precede the writes below.
         */
        power_state = cmpxchg_acquire(&target->power_state,
                                      PSCI_0_2_AFFINITY_LEVEL_OFF,
                                      PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);

This confused me more :( Why are you talking about accesses preceding an acquire? An acquire only orders later accesses.

I really think we need some litmus tests to understand the general ordering problems we have here before adding the memory barriers.

On Thu, 24 Sep 2026 13:14:00 +0100, Will Deacon <[email protected]> wrote:
[...]

Quoted 9 lines

Quoted 1 line

It's not publishing data, it's handing reset_state back.

What's the difference? The usual pattern for acquire/release is:

<write data>
<store-release flag>

on one CPU and then on another:

Quoted 8 lines

<load-acquire flag> // If this reads from the release above...
<read data> // ... then this is guaranteed to read the written data

That's a message-passing shape and you would normally say that the first CPU (the producer) is publishing the data to the other CPU (the consumer).

Is this what is happening with the 'reset_state' (data) and the 'power_state' (flag)? If not, then what is the shape?

I don't think so, it's the other direction. That pattern is the pair on reset_state.reset, and it isn't in question. The pair on power_state, as I see it, runs the other way: the target's last accesses to reset_state are its reads of the payload and its clear of the flag in pkvm_reset_vcpu(), and the next CPU_ON's accesses are stores. Release on OFF, acquire on the cmpxchg: unlock then lock, with power_state as the lock word. The winner takes it with the cmpxchg, hands it to the target through reset, and the target gives it up at CPU_OFF. Nothing is published. What I was after is the winner's stores being ordered after the target's reads and its clear.

For the benefit of everybody else, we had a fire alarm in the office yesterday evening so Fuad and I sat outside a bar with a beer and a piece of paper and went through this together...

If we address (2), then I think the sequence becomes a little easier to reason about. The OFF->ON_PENDING transition on the requestor looks like:

which I think makes sense. WDYT? If you agree, I wonder if we can include a comment similar to the above in the code?

arch/arm64/kvm/hyp/nvhe/pkvm.c +15 -0

index 855cb77c8bba1..d970cba12ca47 100644
 
+/*
+ * Reset the vCPU to its power-on state and commit ON_PENDING -> ON, on the
+ * target's own CPU. Returns -ECANCELED, with no side effects, if a rollback
+ * raced the reset.
+ */
+int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct vcpu_reset_state *reset_state = &hyp_vcpu->vcpu.arch.reset_state;
+	int prev;
+
+	/*
+	 * Pairs with smp_store_release(&reset_state->reset, true) in
+	 * pvm_psci_vcpu_on(). The acquire must precede the cmpxchg: reversed, a
+	 * winning cmpxchg with a false acquire would leave power_state == ON
+	 * with the reset skipped.

arch/arm64/kvm/hyp/nvhe/pkvm.c static void pkvm_memunshare_call(u64 *ret, struct kvm_vcpu *vcpu) · line 1256

Quoted 3 lines

arch/arm64/kvm/hyp/nvhe/pkvm.c +3 -0

+    if (prev != PSCI_0_2_AFFINITY_LEVEL_ON_PENDING) {
+        /* The only other writer of ON_PENDING is the rollback. */
+        WARN_ON(prev != PSCI_0_2_AFFINITY_LEVEL_OFF);

If you go with the idea we're discussing later in the thread, then I think you need to drop this WARN_ON() and the comment that goes with it.

__kvm_adjust_pc() lets the host advance a vCPU's PC or inject an exception, which for a protected vCPU would let the host redirect guest execution. Drop the request there: the entry handlers apply the host's PC_UPDATE_REQ on re-entry, where EL2 allows it.

__kvm_adjust_pc() adjusts the vCPU a get/put pair returns, the one it was given outside pKVM. Both host callers hold the vCPU mutex, so a hyp vCPU loaded for the vCPU is loaded on the calling CPU. The request for a loaded protected vCPU is dropped. For a loaded non-protected vCPU, PKVM_HOST_STATE_DIRTY selects the copy, since adjusting the hyp vCPU while the host copy is authoritative loses the update at the next flush. Adjusting the hyp vCPU copies PC_UPDATE_REQ in and back out
again: without the copy back, INCREMENT_PC outlives the adjustment and the next KVM_SET_VCPU_EVENTS trips WARN_ON(INCREMENT_PC) in kvm_pend_exception(). With no hyp vCPU loaded, as under KVM_SET_VCPU_EVENTS, the host copy is adjusted as before.

Until the marshalling patch clears PC_UPDATE_REQ on the host copy at exit, a KVM_RUN that returns to userspace with INCREMENT_PC set leaves it on the host copy of a loaded protected vCPU, and a KVM_SET_VCPU_EVENTS before the next run then trips that WARN_ON.

Suggested-by Marc Zyngier <[email protected]>

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/kvm/hyp/exception.c             | 21 +++++++++-----
 arch/arm64/kvm/hyp/include/hyp/adjust_pc.h | 19 +++++++++++++
 arch/arm64/kvm/hyp/nvhe/hyp-main.c         | 32 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 7 deletions(-)

arch/arm64/kvm/hyp/exception.c +0 -0

index 6e60d890afa4a..7ea62e5304ae9 100644
@@ -353,12 +353,19 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
  */

arch/arm64/kvm/hyp/nvhe/hyp-main.c +3 -0

index da8ab636063cf..1dcc75261dc08 100644
@@ -642,6 +642,38 @@ static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt)
 	cpu_reg(host_ctxt, 1) =  ret;
 }
 
+/*
+ * PKVM_HOST_STATE_DIRTY names the authoritative copy, the host's when set.
+ * A loaded protected vCPU takes the request at its next entry instead.

arch/arm64/kvm/hyp/nvhe/hyp-main.c +8 -0

+
+	if (vcpu_get_flag(vcpu, PKVM_HOST_STATE_DIRTY))
+		return vcpu;
+
+	vcpu_copy_flag(&hyp_vcpu->vcpu, vcpu, PC_UPDATE_REQ);
+	return &hyp_vcpu->vcpu;
+}
+

nit: it is really odd to see this 'pkvm' prefix in generic code. The point of it is not only to abstract the pvkm complexity away, but also to have a wrapper that may be of use in other situations.

Don't respin the series just for this though, I may end-up changing this when applying it.

arch/arm64/kvm/hyp/nvhe/hyp-main.c static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt) · line 657

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c +2 -0

+   hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
+   if (!hyp_vcpu || hyp_vcpu->host_vcpu != vcpu)

Under which circumstances do we get hyp_vcpu->host_vcpu != vcpu?

arch/arm64/kvm/hyp/nvhe/hyp-main.c static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt) · line 661

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c +2 -0

+   if (pkvm_hyp_vcpu_is_protected(hyp_vcpu))
+           return NULL;

Is it always the case that a protected vcpu cannot see its PC adjusted at all? How is PC updated after an exit for MMIO? I feel there is an interaction with the above, but I'm not 100% certain...

Quoted 5 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt) · line 657

Quoted 3 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c +2 -0

+   hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
+   if (!hyp_vcpu || hyp_vcpu->host_vcpu != vcpu)

Under which circumstances do we get hyp_vcpu->host_vcpu != vcpu?

When EL2 calls __kvm_adjust_pc() on the hyp vCPU itself:
__kvm_vcpu_run() at guest entry and inject_sync64() in nvhe/sys_regs.c both pass &hyp_vcpu->vcpu. There the vCPU passed is the one to adjust, so it's returned as is.

Quoted 6 lines · Fuad Tabba

Quoted 5 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt) · line 657

Quoted 3 lines

arch/arm64/kvm/hyp/nvhe/hyp-main.c +2 -0

+   hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
+   if (!hyp_vcpu || hyp_vcpu->host_vcpu != vcpu)

Under which circumstances do we get hyp_vcpu->host_vcpu != vcpu?

Quoted 4 lines

When EL2 calls __kvm_adjust_pc() on the hyp vCPU itself:
__kvm_vcpu_run() at guest entry and inject_sync64() in nvhe/sys_regs.c both pass &hyp_vcpu->vcpu. There the vCPU passed is the one to adjust, so it's returned as is.

If this is the sole case, why not check for '&hyp_cpu->vcpu == vcpu' instead of this odd inequality?

Thanks,

        M.

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

Move a protected guest's state between the hyp vCPU and the host per exception class instead of copying the whole context. Add entry_hyp_pvm_handlers[] and exit_hyp_pvm_handlers[] for WFx, SYS64, IABT, DABT and HVC64, and route protected guests through them: on exit each handler copies out only what its class needs, and on re-entry only what the host may have changed: the PC update or exception it requested (PC_UPDATE_REQ), the value of a read it emulated, and a forwarded PSCI call's return value. entry_hyp_vm_handlers[] is
removed: a non-protected vCPU's iflags are copied wholesale.

The host's fault view is EL2's own syndrome with the guest register index withheld, the deferred SError syndrome (DISR_EL1), and the addresses each class needs. The value of a written register is passed in r0. MMIO data is clamped to the access width and, for a load, sign-extended at EL2 from EL2's syndrome. Endianness stays with the host. A data abort's PC update is taken for a completed MMIO access, and for a cache maintenance operation the host skips on unbacked memory, as it does for any guest.

The host's copy of a protected vCPU's PSTATE is a view the guest never runs from: its reset value has PSTATE.A set, and EL2 sets PSTATE.A in the mode it copies out regardless of the guest's, so, unless the VMM wrote the host copy's PSTATE or SCTLR2_EL1 before the first run, serror_is_masked() is true and kvm_inject_serror_esr() pends a host-injected SError through HCR_EL2.VSE, masked by the guest's own PSTATE.A, instead of emulating the entry on that copy.

Neither dispatch runs for a trap taken with an SError pending: EL2 doesn't handle it, and the guest replays it once the host has injected the SError. The exit handlers would otherwise marshal a trap EL2 never handled, and handle_pvm_exit_hvc64() would panic on an unfiltered function id.

For HVC64, only the PSCI calls EL2 forwards reach the host: the exit handler passes the function id and the arguments each call needs, and the entry handler returns the host's result. A CPU_ON the host failed is rolled back to OFF and returned as INTERNAL_FAILURE, or as ALREADY_ON when that's what the host returned: PSCI defines it as the retry signal for a CPU_ON that reaches the implementation before the target's CPU_OFF has been processed (DEN0022 section 6.6), which a guest that doesn't poll AFFINITY_INFO first can do. A target that already ran returns SUCCESS. The rollback leaves the published reset state in place: clearing it races a fresh CPU_ON's publication and wedges the target at ON_PENDING, and the entry point left behind is one the guest supplied. The rollback's cmpxchg carries no generation, so one that lands after a later CPU_ON has republished ON_PENDING cancels that cycle too. The target is then OFF at EL2 while the host has it runnable, and its CPU_ONs return ALREADY_ON until the VMM stops it again.

Signed-off-by Fuad Tabba <[email protected]>

---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 430 +++++++++++++++++++++++++++--
 1 file changed, 407 insertions(+), 23 deletions(-)

arch/arm64/kvm/hyp/nvhe/hyp-main.c +35 -4

index 1dcc75261dc08..3d59c4827c42a 100644
@@ -4,6 +4,8 @@
  * Author: Andrew Scull <[email protected]>
  */
 
+#include <kvm/arm_hypercalls.h>
+
 #include <hyp/adjust_pc.h>
 #include <hyp/switch.h>
 

@@ -34,13 +36,366 @@ void __kvm_hyp_host_forward_smc(struct kvm_cpu_context *host_ctxt);
 
 typedef void (*hyp_entry_exit_handler_fn)(struct pkvm_hyp_vcpu *);
 
-static void handle_vm_entry_generic(struct pkvm_hyp_vcpu *hyp_vcpu)
+static bool pvm_sys64_is_write(u64 esr)
 {
-	vcpu_copy_flag(&hyp_vcpu->vcpu, hyp_vcpu->host_vcpu, PC_UPDATE_REQ);
+	return (esr & ESR_ELx_SYS64_ISS_DIR_MASK) == ESR_ELx_SYS64_ISS_DIR_WRITE;
 }
 
-static const hyp_entry_exit_handler_fn entry_hyp_vm_handlers[] = {
-	[0 ... ESR_ELx_EC_MAX]		= handle_vm_entry_generic,
+static void handle_pvm_entry_wfx(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+
+	/* Exceptions have priority; the host injects none on WFx. */
+	if (vcpu_get_flag(host_vcpu, PENDING_EXCEPTION))
+		return;
+
+	if (vcpu_get_flag(host_vcpu, INCREMENT_PC)) {
+		vcpu_clear_flag(&hyp_vcpu->vcpu, PC_UPDATE_REQ);
+		kvm_incr_pc(&hyp_vcpu->vcpu);
+	}
+}
+
+static void handle_pvm_entry_sys64(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+	bool pc_update;
+
+	/* Exceptions have priority over anything else */
+	if (vcpu_get_flag(host_vcpu, PENDING_EXCEPTION)) {
+		/* A host-requested exception on SYS64 is always an UNDEF. */
+		u32 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT) | ESR_ELx_IL;
+
+		__vcpu_assign_sys_reg(&hyp_vcpu->vcpu, ESR_EL1, esr);
+		kvm_pend_exception(&hyp_vcpu->vcpu, EXCEPT_AA64_EL1_SYNC);
+		return;
+	}
+
+	/* Handle PC increment on a host-emulated access */
+	pc_update = vcpu_get_flag(host_vcpu, INCREMENT_PC);

arch/arm64/kvm/hyp/nvhe/hyp-main.c +5 -0

+static void handle_pvm_exit_sys64(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+	u32 esr_el2 = hyp_vcpu->vcpu.arch.fault.esr_el2;
+