[PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation

Per Larsen posted 3 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation
Posted by Per Larsen 9 months, 1 week ago
From: Per Larsen <perlarsen@google.com>

FF-A implementations with the same major version must interoperate with
earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not
backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).

If we return the negotiated hypervisor version when the host requests a
lesser minor version, the host will rely on the FF-A interoperability
rules. Since the hypervisor does not currently have the necessary
compatibility paths (e.g. to handle breaking changes to the SMC calling
convention), return NOT_SUPPORTED.

Signed-off-by: Per Larsen <perlarsen@google.com>
Signed-off-by: Per Larsen <perl@immunant.com>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 3369dd0c4009..10e88207b78e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
 
 	hyp_spin_lock(&version_lock);
 	if (has_version_negotiated) {
-		res->a0 = hyp_ffa_version;
+		/*
+		 * FF-A implementations with the same major version must
+		 * interoperate with earlier minor versions per DEN0077A 1.2
+		 * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
+		 * structures and 1.2 relies on SMCCC 1.2 is not backwards
+		 * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
+		 *
+		 * If we return the negotiated hypervisor version when the host
+		 * requests a lesser minor version, the host will rely on the
+		 * aforementioned FF-A interoperability rules. Since the
+		 * hypervisor does not currently have the necessary compatibility
+		 * paths (e.g. to paper over the above-mentioned calling
+		 * convention changes), return NOT_SUPPORTED.
+		 */
+		if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
+			res->a0 = FFA_RET_NOT_SUPPORTED;
+		else
+			res->a0 = hyp_ffa_version;
 		goto unlock;
 	}
 
-- 
2.49.0
Re: [PATCH 1/3] KVM: arm64: Restrict FF-A host version renegotiation
Posted by Sebastian Ene 9 months, 1 week ago
On Fri, May 02, 2025 at 02:21:06AM -0700, Per Larsen wrote:
> From: Per Larsen <perlarsen@google.com>
> 
> FF-A implementations with the same major version must interoperate with
> earlier minor versions per DEN0077A 1.2 REL0 13.2.1 but FF-A version 1.1
> broke the ABI on several structures and 1.2 relies on SMCCC 1.2 is not

The wording here is a bit hard to follow. Why don't we re-write to
something as simple as:

"Prevent the host from re-negotiating a smaller version with the
hypervisor. Once the hypervisor negotiates a version, that should
remain locked in. Fix the current behaviour by returning NOT_SUPPORTED
to avoid the FF-A ineroperability rules with ealier minor versions which
allows the host version to downgrade."

> backwards compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> 
> If we return the negotiated hypervisor version when the host requests a
> lesser minor version, the host will rely on the FF-A interoperability
> rules. Since the hypervisor does not currently have the necessary
> compatibility paths (e.g. to handle breaking changes to the SMC calling
> convention), return NOT_SUPPORTED.



> 
> Signed-off-by: Per Larsen <perlarsen@google.com>
> Signed-off-by: Per Larsen <perl@immunant.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 3369dd0c4009..10e88207b78e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -712,7 +712,24 @@ static void do_ffa_version(struct arm_smccc_res *res,
>  
>  	hyp_spin_lock(&version_lock);
>  	if (has_version_negotiated) {
> -		res->a0 = hyp_ffa_version;
> +		/*
> +		 * FF-A implementations with the same major version must
> +		 * interoperate with earlier minor versions per DEN0077A 1.2
> +		 * REL0 13.2.1 but FF-A version 1.1 broke the ABI on several
> +		 * structures and 1.2 relies on SMCCC 1.2 is not backwards
> +		 * compatible with SMCCC 1.2 (see DEN0028 1.6 G BET0 Appendix F).
> +		 *
> +		 * If we return the negotiated hypervisor version when the host
> +		 * requests a lesser minor version, the host will rely on the
> +		 * aforementioned FF-A interoperability rules. Since the
> +		 * hypervisor does not currently have the necessary compatibility
> +		 * paths (e.g. to paper over the above-mentioned calling
> +		 * convention changes), return NOT_SUPPORTED.
> +		 */

I would drop this comment as the commit message should be pretty
descriptive.

> +		if (FFA_MINOR_VERSION(ffa_req_version) < FFA_MINOR_VERSION(hyp_ffa_version))
> +			res->a0 = FFA_RET_NOT_SUPPORTED;
> +		else
> +			res->a0 = hyp_ffa_version;
>  		goto unlock;
>  	}
>  
> -- 
> 2.49.0
> 

Thanks,
Seb