Skip to contentSkip to conversation

KVM: arm64: Use acquire/release to communicate FF-A version negotiation

PATCH Applied Will Deacon 2 participants 2 messages Apr 7, 2025

Subsystems (2)
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

PATCH 1/1 KVM: arm64: Use acquire/release to communicate FF-A version negotiation

The pKVM FF-A proxy rejects FF-A requests other than FFA_VERSION until version negotiation is complete, which is signalled by setting the global 'has_version_negotiated' variable.

To avoid excessive locking, this variable is checked directly from kvm_host_ffa_handler() in response to an FF-A call, but this can race against another CPU performing the negotiation and potentially lead to reading a torn value (incredibly unlikely for a 'bool') or problematic re-ordering of the accesses to 'has_version_negotiated' and 'hyp_ffa_version' whereby a stale version number could be read by __do_ffa_mem_xfer().

Use acquire/release primitives when writing 'has_version_negotiated' with the version lock held and when reading without the lock held.

Cc Sebastian Ene <[email protected]>
Cc Sudeep Holla <[email protected]>
Cc Quentin Perret <[email protected]>
Cc Oliver Upton <[email protected]>
Cc Marc Zyngier <[email protected]>
Fixes c9c012625e12 ("KVM: arm64: Trap FFA_VERSION host call in pKVM")

---

Found by code inspection rather than any real issue in practice.

 arch/arm64/kvm/hyp/nvhe/ffa.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm64/kvm/hyp/nvhe/ffa.c +5 -4
index e433dfab882a..3369dd0c4009 100644
@@ -730,10 +730,10 @@ static void do_ffa_version(struct arm_smccc_res *res,
 		hyp_ffa_version = ffa_req_version;
 	}
 
-	if (hyp_ffa_post_init())
+	if (hyp_ffa_post_init()) {
 		res->a0 = FFA_RET_NOT_SUPPORTED;
-	else {
-		has_version_negotiated = true;
+	} else {
+		smp_store_release(&has_version_negotiated, true);
 		res->a0 = hyp_ffa_version;
 	}
 unlock:
@@ -809,7 +809,8 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id)
 	if (!is_ffa_call(func_id))
 		return false;
 
-	if (!has_version_negotiated && func_id != FFA_VERSION) {
+	if (func_id != FFA_VERSION &&
+	    !smp_load_acquire(&has_version_negotiated)) {
 		ffa_to_smccc_error(&res, FFA_RET_INVALID_PARAMETERS);
 		goto out_handled;
 	}
Signature · 1 line
2.49.0.504.g3bcea36a83-goog
Applied
#1 lore raw
Quoted 12 lines · Will Deacon

The pKVM FF-A proxy rejects FF-A requests other than FFA_VERSION until version negotiation is complete, which is signalled by setting the global 'has_version_negotiated' variable.

To avoid excessive locking, this variable is checked directly from kvm_host_ffa_handler() in response to an FF-A call, but this can race against another CPU performing the negotiation and potentially lead to reading a torn value (incredibly unlikely for a 'bool') or problematic re-ordering of the accesses to 'has_version_negotiated' and 'hyp_ffa_version' whereby a stale version number could be read by __do_ffa_mem_xfer().

Quoted 1 line

[...]

Applied to fixes, thanks!

[1/1] KVM: arm64: Use acquire/release to communicate FF-A version negotiation

      https://git.kernel.org/kvmarm/kvmarm/c/a344e258acb0

--
Best,
Oliver