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

[Sebastian Ene <sebastianene@google.com>]

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