From: Per Larsen <perlarsen@google.com>
FF-A version 1.2 introduces the DIRECT_REQ2 ABI. Bump the FF-A version
preferred by the hypervisor as a precursor to implementing the 1.2-only
FFA_MSG_SEND_DIRECT_REQ2 and FFA_MSG_SEND_RESP2 messaging interfaces.
We must also use SMCCC 1.2 for 64-bit SMCs if hypervisor negotiated FF-A
1.2, so ffa_set_retval is updated and a new function to call 64-bit smcs
using SMCCC 1.2 with fallback to SMCCC 1.1 is introduced.
Update ffa_call_supported to mark FF-A 1.2 interfaces as unsupported
lest they get forwarded.
Co-developed-by: Ayrton Munoz <ayrton@google.com>
Signed-off-by: Ayrton Munoz <ayrton@google.com>
Signed-off-by: Per Larsen <perlarsen@google.com>
---
arch/arm64/kvm/hyp/nvhe/ffa.c | 18 ++++++++++++++----
include/linux/arm_ffa.h | 1 +
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 5fd6474d96ae4b90d99796ee81bb36373219afc4..79d834120a3f3d26e17e9170c60012b60c6f5a5e 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -678,6 +678,13 @@ static bool ffa_call_supported(u64 func_id)
case FFA_NOTIFICATION_SET:
case FFA_NOTIFICATION_GET:
case FFA_NOTIFICATION_INFO_GET:
+ /* Optional interfaces added in FF-A 1.2 */
+ case FFA_MSG_SEND_DIRECT_REQ2: /* Optional per 7.5.1 */
+ case FFA_MSG_SEND_DIRECT_RESP2: /* Optional per 7.5.1 */
+ case FFA_CONSOLE_LOG: /* Optional per 13.1: not in Table 13.1 */
+ case FFA_PARTITION_INFO_GET_REGS: /* Optional for virtual instances per 13.1 */
+ /* Unsupported interfaces added in FF-A 1.2 */
+ case FFA_EL3_INTR_HANDLE: /* Only valid for secure physical instances */
return false;
}
@@ -734,7 +741,10 @@ static int hyp_ffa_post_init(void)
if (res.a0 != FFA_SUCCESS)
return -EOPNOTSUPP;
- switch (res.a2) {
+ if ((res.a2 & GENMASK(15, 2)) != 0 || res.a3 != 0)
+ return -EINVAL;
+
+ switch (res.a2 & FFA_FEAT_RXTX_MIN_SZ_MASK) {
case FFA_FEAT_RXTX_MIN_SZ_4K:
min_rxtx_sz = SZ_4K;
break;
@@ -931,7 +941,7 @@ int hyp_ffa_init(void *pages)
arm_smccc_1_2_smc(&(struct arm_smccc_1_2_regs) {
.a0 = FFA_VERSION,
- .a1 = FFA_VERSION_1_1,
+ .a1 = FFA_VERSION_1_2,
}, &res);
if (res.a0 == FFA_RET_NOT_SUPPORTED)
return 0;
@@ -952,10 +962,10 @@ int hyp_ffa_init(void *pages)
if (FFA_MAJOR_VERSION(res.a0) != 1)
return -EOPNOTSUPP;
- if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))
+ if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_2))
hyp_ffa_version = res.a0;
else
- hyp_ffa_version = FFA_VERSION_1_1;
+ hyp_ffa_version = FFA_VERSION_1_2;
tx = pages;
pages += KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE;
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 5bded24dc24fea8cdcbe42bf79c7c025c3fa5f4b..2b760bede9c0a4bcb58bf697681c32e77c08a5d9 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -128,6 +128,7 @@
#define FFA_FEAT_RXTX_MIN_SZ_4K 0
#define FFA_FEAT_RXTX_MIN_SZ_64K 1
#define FFA_FEAT_RXTX_MIN_SZ_16K 2
+#define FFA_FEAT_RXTX_MIN_SZ_MASK GENMASK(1, 0)
/* FFA Bus/Device/Driver related */
struct ffa_device {
--
2.50.0.727.gbf7dc18ff4-goog
On Tue, Jul 01, 2025 at 10:06:37PM +0000, Per Larsen via B4 Relay wrote:
> From: Per Larsen <perlarsen@google.com>
>
> FF-A version 1.2 introduces the DIRECT_REQ2 ABI. Bump the FF-A version
> preferred by the hypervisor as a precursor to implementing the 1.2-only
> FFA_MSG_SEND_DIRECT_REQ2 and FFA_MSG_SEND_RESP2 messaging interfaces.
>
> We must also use SMCCC 1.2 for 64-bit SMCs if hypervisor negotiated FF-A
> 1.2, so ffa_set_retval is updated and a new function to call 64-bit smcs
> using SMCCC 1.2 with fallback to SMCCC 1.1 is introduced.
>
> Update ffa_call_supported to mark FF-A 1.2 interfaces as unsupported
> lest they get forwarded.
>
> Co-developed-by: Ayrton Munoz <ayrton@google.com>
> Signed-off-by: Ayrton Munoz <ayrton@google.com>
> Signed-off-by: Per Larsen <perlarsen@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/ffa.c | 18 ++++++++++++++----
> include/linux/arm_ffa.h | 1 +
> 2 files changed, 15 insertions(+), 4 deletions(-)
This patch needs to be split into smaller chunks as it's doing a number
of things in one go.
> diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> index 5fd6474d96ae4b90d99796ee81bb36373219afc4..79d834120a3f3d26e17e9170c60012b60c6f5a5e 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> @@ -678,6 +678,13 @@ static bool ffa_call_supported(u64 func_id)
> case FFA_NOTIFICATION_SET:
> case FFA_NOTIFICATION_GET:
> case FFA_NOTIFICATION_INFO_GET:
> + /* Optional interfaces added in FF-A 1.2 */
> + case FFA_MSG_SEND_DIRECT_REQ2: /* Optional per 7.5.1 */
> + case FFA_MSG_SEND_DIRECT_RESP2: /* Optional per 7.5.1 */
> + case FFA_CONSOLE_LOG: /* Optional per 13.1: not in Table 13.1 */
> + case FFA_PARTITION_INFO_GET_REGS: /* Optional for virtual instances per 13.1 */
> + /* Unsupported interfaces added in FF-A 1.2 */
> + case FFA_EL3_INTR_HANDLE: /* Only valid for secure physical instances */
> return false;
> }
This could be a standalone change ^^^
>
> @@ -734,7 +741,10 @@ static int hyp_ffa_post_init(void)
> if (res.a0 != FFA_SUCCESS)
> return -EOPNOTSUPP;
>
> - switch (res.a2) {
> + if ((res.a2 & GENMASK(15, 2)) != 0 || res.a3 != 0)
> + return -EINVAL;
Why are you checking bits a2[15:2] and a3? The spec says they MBZ,
so we shouldn't care about enforcing that. In fact, adding the check
probably means we'll fail if those bits get allocated in future.
> +
> + switch (res.a2 & FFA_FEAT_RXTX_MIN_SZ_MASK) {
That makes sense, and can be its own patch.
> case FFA_FEAT_RXTX_MIN_SZ_4K:
> min_rxtx_sz = SZ_4K;
> break;
> @@ -931,7 +941,7 @@ int hyp_ffa_init(void *pages)
>
> arm_smccc_1_2_smc(&(struct arm_smccc_1_2_regs) {
> .a0 = FFA_VERSION,
> - .a1 = FFA_VERSION_1_1,
> + .a1 = FFA_VERSION_1_2,
>
> }, &res);
> if (res.a0 == FFA_RET_NOT_SUPPORTED)
> return 0;
> @@ -952,10 +962,10 @@ int hyp_ffa_init(void *pages)
> if (FFA_MAJOR_VERSION(res.a0) != 1)
> return -EOPNOTSUPP;
>
> - if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_1))
> + if (FFA_MINOR_VERSION(res.a0) < FFA_MINOR_VERSION(FFA_VERSION_1_2))
> hyp_ffa_version = res.a0;
> else
> - hyp_ffa_version = FFA_VERSION_1_1;
> + hyp_ffa_version = FFA_VERSION_1_2;
The move to v1.2 can also be its own patch. So you end up with three in
total.
Will
On Fri, 18 Jul 2025 14:45:17 +0100,
Will Deacon <will@kernel.org> wrote:
>
> On Tue, Jul 01, 2025 at 10:06:37PM +0000, Per Larsen via B4 Relay wrote:
> > From: Per Larsen <perlarsen@google.com>
> >
> > FF-A version 1.2 introduces the DIRECT_REQ2 ABI. Bump the FF-A version
> > preferred by the hypervisor as a precursor to implementing the 1.2-only
> > FFA_MSG_SEND_DIRECT_REQ2 and FFA_MSG_SEND_RESP2 messaging interfaces.
> >
> > We must also use SMCCC 1.2 for 64-bit SMCs if hypervisor negotiated FF-A
> > 1.2, so ffa_set_retval is updated and a new function to call 64-bit smcs
> > using SMCCC 1.2 with fallback to SMCCC 1.1 is introduced.
> >
> > Update ffa_call_supported to mark FF-A 1.2 interfaces as unsupported
> > lest they get forwarded.
> >
> > Co-developed-by: Ayrton Munoz <ayrton@google.com>
> > Signed-off-by: Ayrton Munoz <ayrton@google.com>
> > Signed-off-by: Per Larsen <perlarsen@google.com>
> > ---
> > arch/arm64/kvm/hyp/nvhe/ffa.c | 18 ++++++++++++++----
> > include/linux/arm_ffa.h | 1 +
> > 2 files changed, 15 insertions(+), 4 deletions(-)
>
[..]
Late catching up on this, as we seem to get a version a day, probably
in the hope that it will keep *something* away,...
> > @@ -734,7 +741,10 @@ static int hyp_ffa_post_init(void)
> > if (res.a0 != FFA_SUCCESS)
> > return -EOPNOTSUPP;
> >
> > - switch (res.a2) {
> > + if ((res.a2 & GENMASK(15, 2)) != 0 || res.a3 != 0)
> > + return -EINVAL;
>
> Why are you checking bits a2[15:2] and a3? The spec says they MBZ,
> so we shouldn't care about enforcing that. In fact, adding the check
> probably means we'll fail if those bits get allocated in future.
I have the exact opposite approach. If we don't check that they are 0
for v1.2 and previous versions, we won't be able to tell what they
mean when they are finally allocated to mean something in version
1.337.
Until we support such version, MBZ should be enforced, because we
otherwise don't understand what the "client" is trying to say. And we
don't understand, we're guaranteed to do the wrong thing.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Hey Marc,
(we discussed this very briefly offline but I wanted to reply for the
benefit of everybody else and also because I don't recall quite where we
ended up)
On Thu, Jul 31, 2025 at 08:56:59AM +0100, Marc Zyngier wrote:
> On Fri, 18 Jul 2025 14:45:17 +0100,
> Will Deacon <will@kernel.org> wrote:
> > On Tue, Jul 01, 2025 at 10:06:37PM +0000, Per Larsen via B4 Relay wrote:
> > > From: Per Larsen <perlarsen@google.com>
> > > @@ -734,7 +741,10 @@ static int hyp_ffa_post_init(void)
> > > if (res.a0 != FFA_SUCCESS)
> > > return -EOPNOTSUPP;
> > >
> > > - switch (res.a2) {
> > > + if ((res.a2 & GENMASK(15, 2)) != 0 || res.a3 != 0)
> > > + return -EINVAL;
> >
> > Why are you checking bits a2[15:2] and a3? The spec says they MBZ,
> > so we shouldn't care about enforcing that. In fact, adding the check
> > probably means we'll fail if those bits get allocated in future.
>
> I have the exact opposite approach. If we don't check that they are 0
> for v1.2 and previous versions, we won't be able to tell what they
> mean when they are finally allocated to mean something in version
> 1.337.
>
> Until we support such version, MBZ should be enforced, because we
> otherwise don't understand what the "client" is trying to say. And we
> don't understand, we're guaranteed to do the wrong thing.
We've lost a bunch of context in the diff here, but there are two
important things to keep in mind at this point:
1. We've negotiated a known version of FF-A, so it won't be v1.337 and
we _should_ be able rely on the spec authors not breaking stuff
retrospectively (famous last words...)
2. The response we're parsing here is something that has come back
from TZ after we (the hypervisor) have called FFA_FEATURES. If
those MBZ bits are non-zero, I think should just ignore them.
Cheers,
Will
© 2016 - 2025 Red Hat, Inc.