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

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

[Per Larsen]

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.906.g1f30a19c02-goog

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

[Marc Zyngier]

On Fri, 02 May 2025 04:52:39 +0100,
Per Larsen <perl@immunant.com> wrote:
> 
> 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).

I can't parse this sentence. Missing words?

> + *
> + * 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;
>   }
> 

Something has gone seriously wrong with your email, and the patches
are badly mangled and unusable. They are also sent as individual
patches and not as a thread, which is a sign that you didn't send them
using git. Please fix this for your next posting.

More to the meat of the patches: why should the hypervisor paper over
anything if the spec is broken? Why can't the host just as well decide
for itself what to do?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

[Per Larsen]

From: Per Larsen <perlarsen@google.com>

On Fri, May 02, 2025 at 09:47:45AM +0100, Marc Zyngier wrote:
> On Fri, 02 May 2025 04:52:39 +0100,
> Per Larsen <perl@immunant.com> wrote:
> > 
> > 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).
> 
> I can't parse this sentence. Missing words?
> 

Yes, I will fix this in v2.

> > + *
> > + * 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;
> >   }
> > 
> 
> Something has gone seriously wrong with your email, and the patches
> are badly mangled and unusable. They are also sent as individual
> patches and not as a thread, which is a sign that you didn't send them
> using git. Please fix this for your next posting.
> 

Yes, my apologies. I will use git send-email to post v2.

> More to the meat of the patches: why should the hypervisor paper over
> anything if the spec is broken? Why can't the host just as well decide
> for itself what to do?
> 

Asssuming we drop this patch from the series and apply the rest, the
hypervisor and host can negotiate FF-A 1.2. If the host then calls
FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
return version 1.2 (without this patch). Per the spec, that means the
host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
way in 1.B if B>A). 

However, the hypervisor negotiated version stays at 1.2 so it will use
SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
might as well assume that the hypervisor was implemented to fall back to
SMCCC 1.1 in this particular case. 

I don't even know that the host will ever try to renegotiate as it is
explicitly not allowed by the FF-A spec. There is no way for the
hypervisor to say, "stay at the negotiated version" so we must return
NOT_SUPPORTED. 

> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 
Thanks,
Per

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

[Marc Zyngier]

On Tue, 06 May 2025 10:29:41 +0100,
Per Larsen <perl@immunant.com> wrote:
> 
> From: Per Larsen <perlarsen@google.com>
> 
> On Fri, May 02, 2025 at 09:47:45AM +0100, Marc Zyngier wrote:
> > On Fri, 02 May 2025 04:52:39 +0100,
> > Per Larsen <perl@immunant.com> wrote:
> > > 
> > > 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).
> > 
> > I can't parse this sentence. Missing words?
> > 
> 
> Yes, I will fix this in v2.
> 
> > > + *
> > > + * 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;
> > >   }
> > > 
> > 
> > Something has gone seriously wrong with your email, and the patches
> > are badly mangled and unusable. They are also sent as individual
> > patches and not as a thread, which is a sign that you didn't send them
> > using git. Please fix this for your next posting.
> > 
> 
> Yes, my apologies. I will use git send-email to post v2.
> 
> > More to the meat of the patches: why should the hypervisor paper over
> > anything if the spec is broken? Why can't the host just as well decide
> > for itself what to do?
> > 
> 
> Asssuming we drop this patch from the series and apply the rest, the
> hypervisor and host can negotiate FF-A 1.2. If the host then calls
> FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> return version 1.2 (without this patch).

Why would it do that? Once a particular version has been negotiated, I
expect to be immutable.

> Per the spec, that means the
> host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
> ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
> way in 1.B if B>A).

I don't interpret this as "you can switch between versions" after the
initial negotiation.

> However, the hypervisor negotiated version stays at 1.2 so it will use
> SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
> might as well assume that the hypervisor was implemented to fall back to
> SMCCC 1.1 in this particular case. 
> 
> I don't even know that the host will ever try to renegotiate as it is
> explicitly not allowed by the FF-A spec. There is no way for the
> hypervisor to say, "stay at the negotiated version" so we must return
> NOT_SUPPORTED. 

If it is not allowed, why should we do *anything*? And if the host is
broken, let's fix the host rather than adding pointless validation
code to EL2.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

[Sudeep Holla]

(just adding some additional info not particularly impacting the $subject
 change implementation)

On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> On Tue, 06 May 2025 10:29:41 +0100,
> Per Larsen <perl@immunant.com> wrote:
> > 

[...]

> > Asssuming we drop this patch from the series and apply the rest, the
> > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > return version 1.2 (without this patch).
> 
> Why would it do that? Once a particular version has been negotiated, I
> expect to be immutable.
> 

Not suggesting that we need to support this, but it is technically possible
today by loading FF-A as a module—first inserting and removing a module with
v1.2 support, then loading one with v1.1 support. It can ever throw error
as not supported to keep it simple.

> > Per the spec, that means the
> > host is can use the compatibility rules (DEN0077A Sec 13.2.1) to go
> > ahead and use FF-A 1.1 (every function in 1.A must work in a compatible
> > way in 1.B if B>A).
> 
> I don't interpret this as "you can switch between versions" after the
> initial negotiation.
> 

Agreed.

> > However, the hypervisor negotiated version stays at 1.2 so it will use
> > SMCCC 1.2 for 64-bit interfaces. The host has no way of knowing this and
> > might as well assume that the hypervisor was implemented to fall back to
> > SMCCC 1.1 in this particular case. 
> > 
> > I don't even know that the host will ever try to renegotiate as it is
> > explicitly not allowed by the FF-A spec. There is no way for the
> > hypervisor to say, "stay at the negotiated version" so we must return
> > NOT_SUPPORTED. 
> 
> If it is not allowed, why should we do *anything*? And if the host is
> broken, let's fix the host rather than adding pointless validation
> code to EL2.
> 

Agreed, it is *not yet" allowed. There were some thoughts for a different
use-case IIUC, need to check the status. IIRC, it was bootloader vs OS
where bootloader like UEFI might negotiate one version(usually older) and
then OS comes and request newer version. To support such a setup, we do
need some additional support in the spec and the current latest v1.2 is not
sufficient.

-- 
Regards,
Sudeep

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

[Arve Hjønnevåg]

On Thu, May 8, 2025 at 2:27 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> (just adding some additional info not particularly impacting the $subject
>  change implementation)
>
> On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> > On Tue, 06 May 2025 10:29:41 +0100,
> > Per Larsen <perl@immunant.com> wrote:
> > >
>
> [...]
>
> > > Asssuming we drop this patch from the series and apply the rest, the
> > > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > > return version 1.2 (without this patch).
> >
> > Why would it do that? Once a particular version has been negotiated, I
> > expect to be immutable.
> >
>
> Not suggesting that we need to support this, but it is technically possible
> today by loading FF-A as a module—first inserting and removing a module with
> v1.2 support, then loading one with v1.1 support. It can ever throw error
> as not supported to keep it simple.
>

I'm not sure how what you are suggesting here is different from what
this patch does. This patch does not alter what versions the host can
negotiate. The hypervisor already disallows negotiating a different
version once has_version_negotiated is set, the return code just
doesn't always reflect this. If you try to load the 1.0 driver in the
host after unloading the 1.1 driver similar to what you describe
above, then this CL will let the 1.0 driver know that the hypervisor
does not support 1.0 (I use 1.1 to 1.0 as the example here since this
is an issue even without the next CL in this patch series that bumps
the hypervisor supported version to 1.2). Without this CL, the 1.0
driver will now proceed making other ffa calls using 1.0 data
structures that the hypervisor will incorrectly interpret as 1.1 data
structures.

With this CL, loading a 1.2 driver after the initial 1.1 driver will
work as it did before by returning version 1.1 to the 1.2 driver to
let it know that _it_ needs to downgrade to 1.1. if it wants to
proceed. Loading the 1.0 driver after 1.1 will now fail at the version
negotiation stage however. This will be clearer, and more correct,
than getting FFA_RET_INVALID_PARAMETERS return codes from other ffa
calls when passing valid 1.0 parameters to those calls.

--
Arve Hjønnevåg

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

[Sudeep Holla]

On Thu, May 08, 2025 at 08:45:01AM -0700, Arve Hjønnevåg wrote:
> On Thu, May 8, 2025 at 2:27 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > (just adding some additional info not particularly impacting the $subject
> >  change implementation)
> >
> > On Thu, May 08, 2025 at 09:55:05AM +0100, Marc Zyngier wrote:
> > > On Tue, 06 May 2025 10:29:41 +0100,
> > > Per Larsen <perl@immunant.com> wrote:
> > > >
> >
> > [...]
> >
> > > > Asssuming we drop this patch from the series and apply the rest, the
> > > > hypervisor and host can negotiate FF-A 1.2. If the host then calls
> > > > FFA_VERSION a second time to request FF-A 1.1, the hypervisor would
> > > > return version 1.2 (without this patch).
> > >
> > > Why would it do that? Once a particular version has been negotiated, I
> > > expect to be immutable.
> > >
> >
> > Not suggesting that we need to support this, but it is technically possible
> > today by loading FF-A as a module—first inserting and removing a module with
> > v1.2 support, then loading one with v1.1 support. It can ever throw error
> > as not supported to keep it simple.
> >
> 
> I'm not sure how what you are suggesting here is different from what
> this patch does. This patch does not alter what versions the host can
> negotiate. The hypervisor already disallows negotiating a different
> version once has_version_negotiated is set, the return code just
> doesn't always reflect this. If you try to load the 1.0 driver in the
> host after unloading the 1.1 driver similar to what you describe
> above, then this CL will let the 1.0 driver know that the hypervisor
> does not support 1.0 (I use 1.1 to 1.0 as the example here since this
> is an issue even without the next CL in this patch series that bumps
> the hypervisor supported version to 1.2). Without this CL, the 1.0
> driver will now proceed making other ffa calls using 1.0 data
> structures that the hypervisor will incorrectly interpret as 1.1 data
> structures.
> 
> With this CL, loading a 1.2 driver after the initial 1.1 driver will
> work as it did before by returning version 1.1 to the 1.2 driver to
> let it know that _it_ needs to downgrade to 1.1. if it wants to
> proceed. Loading the 1.0 driver after 1.1 will now fail at the version
> negotiation stage however. This will be clearer, and more correct,
> than getting FFA_RET_INVALID_PARAMETERS return codes from other ffa
> calls when passing valid 1.0 parameters to those calls.
> 

Thanks for the detailed explanation. Sorry I didn't look at the change itself
and I might have made assumptions as I just read

  |  If the host then calls FFA_VERSION a second time to request FF-A 1.1,
  |  the hypervisor would return version 1.2 (without this patch).

So, my point was just that it is OK to even report it as NOT_SUPPORTED
if FF-A proxy doesn't want to deal with all the compatibility for simplicity.

-- 
Regards,
Sudeep

[PATCH 0/3] KVM: arm64: Support FF-A 1.2 and SEND_DIRECT2 ABI

[Per Larsen]

Hi,

The FF-A 1.2 specification introduces a new SEND_DIRECT2 ABI which
allows registers x4-x17 to be used for the message payload. This patch
set prevents the host from using a lower FF-A version than what has
already been negotiated with the hypervisor. This is necessary because
the hypervisor does not have the necessary compatibility paths to
translate from the hypervisor FF-A version to a previous version.

Support for FF-A 1.2 in the hypervisor is added as a precursor to the
addition of the FFA_MSG_SEND_DIRECT_REQ2 messaging interface. The bulk
of this change has to do with the upgrade to SMCCC 1.2 required by
FF-A 1.2. Additionally, unimplemented FF-A 1.2 interfaces are added to
the list of unsupported functions.

Tested by booting Android under QEMU and loading Trusty as the guest
VM and observing the SEND_DIRECT2 ABI being used successfully during
guest boot.

(This is my second attempt at sending out these patches; sorry about
my broken first try and the resulting duplicate messages.)

Thanks,
Per Larsen

Per Larsen (3):
  KVM: arm64: Restrict FF-A host version renegotiation
  KVM: arm64: Bump the supported version of FF-A to 1.2
  KVM: arm64: Support FFA_MSG_SEND_DIRECT_REQ2 in host handler

 arch/arm64/kvm/hyp/nvhe/Makefile |   1 +
 arch/arm64/kvm/hyp/nvhe/ffa.c    | 235 ++++++++++++++++++++++++++++---
 include/linux/arm_ffa.h          |   2 +
 3 files changed, 221 insertions(+), 17 deletions(-)

--
2.49.0

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

[Per Larsen]

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

[Sebastian Ene]

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