[PATCH v7 2/5] KVM: arm64: Use SMCCC 1.2 for FF-A initialization and in host handler

[Will Deacon]

[Sudeep & Mark to To:]

On Mon, Jul 21, 2025 at 05:20:01PM -0700, Per Larsen wrote:
> On 7/21/25 4:01 AM, Arve Hjønnevåg wrote:
> > On Fri, Jul 18, 2025 at 10:54 PM Per Larsen <perl at immunant.com> wrote:
> > > On 7/18/25 6:37 AM, Will Deacon wrote:
> > > > On Mon, Jul 07, 2025 at 05:06:23PM -0700, Per Larsen wrote:
> > > > > On 7/3/25 5:38 AM, Marc Zyngier wrote:
> > > > > > On Tue, 01 Jul 2025 23:06:35 +0100,
> > > > > > Per Larsen via B4 Relay <devnull+perlarsen.google.com at kernel.org> wrote:
> > > > > > > +  /*
> > > > > > > +   * DEN0028C 2.6: SMC32/HVC32 call from aarch64 must preserve x8-x30.
> > > > > > > +   *
> > > > > > > +   * The most straightforward approach is to look at the function ID
> > > > > > > +   * sent by the caller. However, the caller could send FFA_MSG_WAIT
> > > > > > > +   * which is a 32-bit interface but the reply could very well be 64-bit
> > > > > > > +   * such as FFA_FN64_MSG_SEND_DIRECT_REQ or FFA_MSG_SEND_DIRECT_REQ2.
> > > > > > > +   *
> > > > > > > +   * Instead, we could look at the function ID in the response (a0) but
> > > > > > > +   * that doesn't work either as FFA_VERSION responses put the version
> > > > > > > +   * number (or error code) in w0.
> > > > > > > +   *
> > > > > > > +   * Set x8-x17 iff response contains 64-bit function ID in a0.
> > > > > > > +   */
> > > > > > > +  if (func_id != FFA_VERSION && ARM_SMCCC_IS_64(res->a0)) {
> > > > > > > +          cpu_reg(ctxt, 8) = res->a8;
> > > > > > > +          cpu_reg(ctxt, 9) = res->a9;
> > > > > > > +          cpu_reg(ctxt, 10) = res->a10;
> > > > > > > +          cpu_reg(ctxt, 11) = res->a11;
> > > > > > > +          cpu_reg(ctxt, 12) = res->a12;
> > > > > > > +          cpu_reg(ctxt, 13) = res->a13;
> > > > > > > +          cpu_reg(ctxt, 14) = res->a14;
> > > > > > > +          cpu_reg(ctxt, 15) = res->a15;
> > > > > > > +          cpu_reg(ctxt, 16) = res->a16;
> > > > > > > +          cpu_reg(ctxt, 17) = res->a17;
> > > > > > > +  }
> > > > > > >     }
> > > > > > 
> > > > > > I don't see how that can ever work.
> > > > > > 
> > > > > > Irrespective of how FFA_MSG_WAIT actually works (and I couldn't find
> > > > > > anything in the spec that supports the above), the requester will
> > > > > > fully expect its registers to be preserved based on the initial
> > > > > > function type, and that alone. No ifs, no buts.
> > > > > > 
> > > > > > If what you describe can happen (please provide a convincing example),
> > > > > > then the spec is doomed.
> > > > > 
> > > > > DEN0077A 1.2 Section 8.2 (Runtime Model for FFA_RUN) and 8.3 (Runtime Model
> > > > > for Direct request ABIs) contains Figures 8.1 and 8.2. Each figure shows
> > > > > transitions between states "waiting", "blocked", "running", and "preempted".
> > > > > Key to my understanding is that the waiting state in Figure 8.1 and Figure
> > > > > 8.2 is the exact same state. This appears to be the case per Section 4.10.
> > > > > 
> > > > > So we have to consider the ways to get in and out of the waiting state by
> > > > > joining the state machines in Figures 8.1 and 8.2. Figure 8.1 has an edge
> > > > > between "running" and "waiting" caused by FFA_MSG_WAIT. Figure 8.2 has an
> > > > > edge between "waiting" and "running" caused by a "Direct request ABI".
> > > > > 
> > > > > Direct request ABIs include FFA_MSG_SEND_DIRECT_REQ2 which is why the FF-A
> > > > > 1.2 spec, in my read, permits the response to a 32-bit FFA_MSG_WAIT call can
> > > > > be a 64-bit FFA_MSG_SEND_DIRECT_REQ2 reply.
> > > > 
> > > > That seems bonkers to me and I agree with Marc's assessment above. The
> > > > SMCCC is crystal clear that a caller using the SM32/HVC32 calling
> > > > convention can rely on x8-x30 (as well as the stack pointers) being
> > > > preserved. If FF-A has a problem with that then it needs to be fixed
> > > > there and not bodged in Linux.
> > > Ack. Patchset v8 addresses Marc's feedback by using the func_id detect
> > > SMC64 instead.>
> > > > Setting that aside, I'm still not sure I follow this part of your check:
> > > > 
> > > >        if (... && ARM_SMCCC_IS_64(res->a0))
> > > > 
> > > > won't res->a0 contain something like FFA_SUCCESS? The FFA spec states:
> > > > 
> > > >     FFA_SUCCESS64, is used only if any result register encodes a 64-bit
> > > >     parameter.
> > > > 
> > > > which doesn't really seem related to whether or not the initial call
> > > > was using SMC32 or SMC64. What am I missing?I agree that we cannot use res->a0 to distinguish SMC32/SMC64 for the
> > > reason you stated.
> > 
> > I don't think using the function-id of the original call works
> > correctly in this context though. If you look at
> > drivers/firmware/arm_ffa/driver.c:ffa_msg_send_direct_req2 it has the
> > same problem as the FFA_MSG_WAIT example in your comment. In the
> > simple case it will use FFA_MSG_SEND_DIRECT_REQ2 for the call and
> > FFA_MSG_SEND_DIRECT_RESP2 for the response, both 64 bit opcodes, and
> > either approach here will have the same correct result. However if
> > FFA_MSG_SEND_DIRECT_REQ2 responds with FFA_INTERRUPT or FFA_YIELD,
> > then the driver will resume the call with FFA_RUN, a 32 bit opcode,
> > and still expect a 64 bit FFA_MSG_SEND_DIRECT_RESP2 response with a
> > full response in x4-17. If you use ARM_SMCCC_IS_64(func_id) here
> > instead of ARM_SMCCC_IS_64(res->a0), then the part of response in
> > x8-x17 will be lost.

Can't we just update the FF-A driver? This is clearly all the result of
a half-baked spec...

Sudeep -- any chance you can add support for the hot-off-the-press
64-bit FFA_RUN call to the Linux driver, please? Without that, I don't
see how the REQ2/RESP2 calls can work properly.

> > The FF-A 1.3 ALP2 fixes this by adding a 64 bit FF-A run opcode, but
> > at the current patchstack only adds ff-a 1.2 support and the linux
> > ff-a driver does not yet support the new 1.3 ALP2 call flow either so
> > I think the current v7 patch here is the best option for now.
> > 
> FFA_RUN is passed through to EL3 by kvm_host_ffa_handler so I'm not sure
> there is a code path where func_id == FFA_RUN in set_ffa_retval.

That's actually a really interesting point...

The passthrough code in __kvm_hyp_host_forward_smc() assumes SMC64 and
populates/reloads X0-X17 across the SMC. If the firmware replies to an
SMC32 call with an SMC64, then so be it, but it's not the hypervisor's
job to fix that up and the same problem would presumably happen even if
the hypervisor wasn't present.

So perhaps there's an argument that the proxy should be consistent with
that behaviour, otherwise we end up with different semantics depending
on whether we chose to proxy the call or pass it through. That would
mean always populating X8-X17 in the return value, regardless of the
function ID or anything else.

The spec should still be fixed because the current wording makes very
little sense in this area, but in the meantime maybe it's best just to
pass the hot potato between the host and the firmware rather than try to
fix it up ourselves.

Marc -- what do you think? We're damned if we do, damned if we don't,
but there's something to be said for being consistent when we get into
this messy situation.

Will