Fix FFA version negotiation with the firmware to follow the
specification guidance more closely (see FF-A Specification Version 1.1
in chapter 13.2.1).
When the firmware returns OK we can have several cases:
- the version requested is accepted but the firmware supports a greater
one in the same major.
- the firmware supports a greater major version. It could still return
OK even if the version requested is not accepted. Reject it.
- the firmware supports a lower version. It will return OK and give that
version. Check if we support it and use it or reject it if we do not.
Adapt the code to:
- reject any version lower than the one we support or not with the same
major version
- use the version returned if in our supported range (currently 1.1
only)
- use 1.1 if the version returned is greater.
Also adapt the handling of version requests from VM:
- use our version if same major but greater minor is requested
- use requested version if same major but lower minor is requested
- do not use if incompatible major is requested
- always return our version without error to the requester
[1] https://developer.arm.com/documentation/den0077/e/
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
---
Changes in v3:
- add Jens R-b
Changes in v2:
- add link in commit message and code to the version and chapter of
the spec.
- fix comment coding style
- introduce macros to get the major and minor of a version
- add BUILD_BUG_ON to validate that the SPMC version we want is
compatible with our own version
- rework version testing and selecting to be clearer by discarding
different major or version lower than what we want and then selecting
based on the minor version.
- fix get_version handling to be more generic
---
xen/arch/arm/tee/ffa.c | 53 +++++++++++++++++++++++++---------
xen/arch/arm/tee/ffa_private.h | 3 ++
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index ade6aaa6fd1d..c5dcb4fe240c 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -148,13 +148,20 @@ static void handle_version(struct cpu_user_regs *regs)
struct ffa_ctx *ctx = d->arch.tee;
uint32_t vers = get_user_reg(regs, 1);
- if ( vers < FFA_VERSION_1_1 )
- vers = FFA_VERSION_1_0;
- else
- vers = FFA_VERSION_1_1;
-
- ctx->guest_vers = vers;
- ffa_set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
+ /*
+ * Guest will use the version it requested if it is our major and minor
+ * lower or equals to ours. If the minor is greater, our version will be
+ * used.
+ * In any case return our version to the caller.
+ */
+ if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
+ {
+ if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
+ ctx->guest_vers = FFA_MY_VERSION;
+ else
+ ctx->guest_vers = vers;
+ }
+ ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
}
static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid)
@@ -537,17 +544,39 @@ static bool ffa_probe(void)
goto err_no_fw;
}
- if ( vers < FFA_MIN_SPMC_VERSION || vers > FFA_MY_VERSION )
+ /* Some sanity check in case we update the version we support */
+ BUILD_BUG_ON(FFA_MIN_SPMC_VERSION > FFA_MY_VERSION);
+ BUILD_BUG_ON(FFA_VERSION_MAJOR(FFA_MIN_SPMC_VERSION) !=
+ FFA_MY_VERSION_MAJOR);
+
+ major_vers = FFA_VERSION_MAJOR(vers);
+ minor_vers = FFA_VERSION_MINOR(vers);
+
+ if ( major_vers != FFA_MY_VERSION_MAJOR ||
+ minor_vers < FFA_VERSION_MINOR(FFA_MIN_SPMC_VERSION) )
{
- printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
+ printk(XENLOG_ERR "ffa: Incompatible firmware version %u.%u\n",
+ major_vers, minor_vers);
goto err_no_fw;
}
- major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;
- minor_vers = vers & FFA_VERSION_MINOR_MASK;
printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
major_vers, minor_vers);
+ /*
+ * If the call succeed and the version returned is higher or equal to
+ * the one Xen requested, the version requested by Xen will be the one
+ * used. If the version returned is lower but compatible with Xen, Xen
+ * will use that version instead.
+ * A version with a different major or lower than the minimum version
+ * we support is rejected before.
+ * See https://developer.arm.com/documentation/den0077/e/ chapter 13.2.1
+ */
+ if ( minor_vers > FFA_MY_VERSION_MINOR )
+ ffa_fw_version = FFA_MY_VERSION;
+ else
+ ffa_fw_version = vers;
+
/*
* At the moment domains must support the same features used by Xen.
* TODO: Rework the code to allow domain to use a subset of the
@@ -563,8 +592,6 @@ static bool ffa_probe(void)
}
}
- ffa_fw_version = vers;
-
if ( !ffa_rxtx_init() )
{
printk(XENLOG_ERR "ffa: Error during RXTX buffer init\n");
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 7c6b06f686fc..045d9c4a0b56 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -35,6 +35,9 @@
#define MAKE_FFA_VERSION(major, minor) \
((((major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) | \
((minor) & FFA_VERSION_MINOR_MASK))
+#define FFA_VERSION_MAJOR(vers) (((vers) >> FFA_VERSION_MAJOR_SHIFT) & \
+ FFA_VERSION_MAJOR_MASK)
+#define FFA_VERSION_MINOR(vers) ((vers) & FFA_VERSION_MINOR_MASK)
#define FFA_VERSION_1_0 MAKE_FFA_VERSION(1, 0)
#define FFA_VERSION_1_1 MAKE_FFA_VERSION(1, 1)
--
2.47.0