Re: [PATCH v2 3/4] KVM: arm64: Map the hypervisor FF-A buffers on ffa init

Will Deacon
2025-03-13T12:21:40Z

On Wed, Mar 05, 2025 at 06:36:01PM +0000, Sebastian Ene wrote:
> On Wed, Mar 05, 2025 at 12:38:08AM +0000, Will Deacon wrote:
> > On Tue, Mar 04, 2025 at 05:38:02PM +0000, Sebastian Ene wrote:
> > > On Tue, Mar 04, 2025 at 01:56:35AM +0000, Will Deacon wrote:
> > > > | [...] negotiation of the version must happen before an invocation of
> > > > | any other FF-A ABI
> > > >
> > >
> > > We do that, as the hypervisor negotiates its own version in
> > > hyp_ffa_init.
> >
> > hyp_ffa_init() only issues FFA_VERSION afaict, which is the one call
> > that you're allowed to make during negotiation. So the existing code is
> > fine.
> >
> > > I think the host shouldn't be allowed to overwrite the
> > > hyp_ffa_version obtained from _init, this feels wrong as you
> > > can have a driver that forcefully downgrades the hypervisor to an old
> > > version.
> >
> > I think that's also fine. The FFA code in the hypervisor exists solely
> > to proxy requests from the host; it's not used for anything else and so,
> > from the host's persective, FFA should behave identically to the case in
> > which the proxy is not present (e.g. if we were just using VHE). That
> > means that we're doing the right thing by deferring to the host for
> > version negotation.
> >
> > Are you saying there's a bug in the current code if the host negotiates
> > the downgrade?
>
> It is an issue *only* for doing guest-ffa (which isn't posted here yet).
> If we allow the host to dictate the version & there is an issue with TZ
> FF-A dispatcher in that version => the guests will be affected by this
> as well.

When we get to guests doing FF-A, I still think the host should be
responsible for the version negotiation and guests should just be given
whatever has been negotiated. We could extend the hypervisor to marshall
between different versions, but I'd rather do that only if we actually
need it.

> > > We need to do three things, Sudeep & Will please correct me if I am
> > > wrong, but this is how I see it:
> > >
> > > - the hypervisor should act as a separate entity (it has a different ID and
> > > in the current implementation we don't do a distinction between host/hyp) and
> > > it should be able to lock its own version from init.
> >
> > I strongly disagree with that. The hypervisor isn't using FFA for
> > anything other than proxying the host and so we don't need to negotiate
> > a separate version.
> >
> > What would we gain by doing this? Is there a bug with what we're doing
> > at the moment?
>
> I think we need to make a distinction between the host and the
> hypervisor when we are adding support for guest-ffa. We currently have
> the same id (== 0) for both of them.

Right, and we currently don't support guest-ffa, so no problem :)

> > > - keep a separate version negotiated for the host
> > > - trap FFA_ID_GET from the host and return ID=1 because
> > > currently we forward the call to the TZ and it returns the same ID
> > > as the (hypervisor == 0).
> >
> > Why is this beneficial? It just looks like complexity at EL2 for no gain
> > to me, but maybe I'm missing something.
> >
>
> Because the host can impersonate the hypervisor using ff-a direct calls atm.
> and we are in a position to restrict the host from playing nasty games
> with TZ.

Can you give a specific example of why impersonating a direct call is a
problem? I agree that it sounds bad, but the hypervisor is still in the
middle and so it can check what the call is requesting.

Will