[PATCH v4 2/5] KVM: arm64: Zero x4-x7 in ffa_set_retval

[Will Deacon]

On Fri, May 16, 2025 at 12:14:01PM +0000, Per Larsen via B4 Relay wrote:
> From: Per Larsen <perlarsen at google.com>
> 
> FF-A 1.0 references SMCCC 1.2 (reference [4] on page xi). The results of
> an SMC32/HVC32 are returned in W1-W7 in SMCCC 1.2. However, W4-W7 must be
> preserved unless they contain results according to the function definition.
> 
> ffa_set_retval is commonly used to report FFA_SUCCESS or FFA_ERROR back to
> the caller. FFA_SUCCESS requires that x2-x7 must be zero (MBZ) unless a
> register is explicitly used in the function response. FFA_ERROR requires
> x3-x7 MBZ.
> 
> ffa_set_retval is also used to return results of:
> - FFA_VERSION which requires x1-x7 MBZ
> - FFA_MEM_FRAG_TX which returns FFA_MEM_FRAG_RX or FFA_ERROR.
>   FFA_MEM_FRAG_RAX always requires that x5-x7 MBZ and x4 MBZ
>   at any virtual FF-A instance (applies to kvm).
> 
> Messaging interfaces such as FFA_MSG_SEND_DIRECT_{REQ,REQ2}, will not use
> ffa_set_retval.
> 
> Given the above, it is safe to zero x4-x7 in ffa_set_retval.
> 
> Signed-off-by: Per Larsen <perlarsen at google.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/ffa.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 2c199d40811efb5bfae199c4a67d8ae3d9307357..b3d016bee404ce3f8c72cc57befb4ef4e6c1657f 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -101,6 +101,10 @@ static void ffa_set_retval(struct kvm_cpu_context *ctxt,
>  	cpu_reg(ctxt, 1) = res->a1;
>  	cpu_reg(ctxt, 2) = res->a2;
>  	cpu_reg(ctxt, 3) = res->a3;
> +	cpu_reg(ctxt, 4) = 0;
> +	cpu_reg(ctxt, 5) = 0;
> +	cpu_reg(ctxt, 6) = 0;
> +	cpu_reg(ctxt, 7) = 0;
>  }

Hmm. On reflection, isn't the root of the problem actually that we're
using SMCCC 1.1 to communicate with TZ, whereas FF-A requires SMCCC 1.2
(and we even enforce that in hyp_ffa_init())?

If we moved all of the hyp FF-A code over to SMCCC 1.2 unconditionally,
then we could just assign everything from res back to the CPU context
without having to worry about zeroing. That would also simplify your
later patches because we wouldn't have to deal with SMCCC 1.1 at all.

I'm aware that we're carrying a patch in Android to permit SMCCC 1.1,
but that's only for Pixel 6 iirc and we shouldn't worry about that
upstream.

Will