Hi,
On 19/09/2024 14:19, Bertrand Marquis wrote:
Store the list of ABI we need in a list and go through the list instead
of having a list of conditions inside the code.
No functional change.
Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
---
xen/arch/arm/tee/ffa.c | 61 +++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index 7c84aa6aa43d..7ff2529b2055 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -74,6 +74,24 @@
/* Negotiated FF-A version to use with the SPMC, 0 if not there or supported
*/
static uint32_t __ro_after_init ffa_fw_version;
+/* List of ABI we use from the firmware */
+static const uint32_t ffa_fw_feat_needed[] = {
+ FFA_VERSION,
+ FFA_FEATURES,
+ FFA_NOTIFICATION_BITMAP_CREATE,
+ FFA_NOTIFICATION_BITMAP_DESTROY,
+ FFA_PARTITION_INFO_GET,
+ FFA_NOTIFICATION_INFO_GET_64,
+ FFA_NOTIFICATION_GET,
+ FFA_RX_RELEASE,
+ FFA_RXTX_MAP_64,
+ FFA_RXTX_UNMAP,
+ FFA_MEM_SHARE_32,
+ FFA_MEM_SHARE_64,
+ FFA_MEM_RECLAIM,
+ FFA_MSG_SEND_DIRECT_REQ_32,
+ FFA_MSG_SEND_DIRECT_REQ_64,
+};
NIT: As we are creating an array, could be take the opportunity to
provide a name for each feature (we could use macro for that)? This
would make easier for the user to know which feature is missing.
/*
* Our rx/tx buffers shared with the SPMC. FFA_RXTX_PAGE_COUNT is the
@@ -112,20 +130,9 @@ static bool ffa_get_version(uint32_t *vers)
return true;
}
-static int32_t ffa_features(uint32_t id)
+static bool ffa_feature_supported(uint32_t id)
{
- return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
-}
-
-static bool check_mandatory_feature(uint32_t id)
-{
- int32_t ret = ffa_features(id);
-
- if ( ret )
- printk(XENLOG_ERR "ffa: mandatory feature id %#x missing: error %d\n",
- id, ret);
-
- return !ret;
+ return !ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
}
static void handle_version(struct cpu_user_regs *regs)
@@ -529,24 +536,6 @@ static bool ffa_probe(void)
goto err_no_fw;
}
- /*
- * 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
- * features supported.
- */
You removed this TODO but I don't think it was addressed. Can you clarify?
- if ( !check_mandatory_feature(FFA_PARTITION_INFO_GET) ||
- !check_mandatory_feature(FFA_RX_RELEASE) ||
- !check_mandatory_feature(FFA_RXTX_MAP_64) ||
- !check_mandatory_feature(FFA_MEM_SHARE_64) ||
- !check_mandatory_feature(FFA_RXTX_UNMAP) ||
- !check_mandatory_feature(FFA_MEM_SHARE_32) ||
- !check_mandatory_feature(FFA_MEM_RECLAIM) ||
- !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
- {
- printk(XENLOG_ERR "ffa: Mandatory feature not supported by fw\n");
- goto err_no_fw;
- }
-
major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT)
& FFA_VERSION_MAJOR_MASK;
minor_vers = vers & FFA_VERSION_MINOR_MASK;
@@ -555,6 +544,16 @@ static bool ffa_probe(void)
ffa_fw_version = vers;
+ for ( int i = 0; i < ARRAY_SIZE(ffa_fw_feat_needed); i++ )
This is an index, so please use unsigned int (even though it should
technically be size_t).
+ {
+ if ( !ffa_feature_supported(ffa_fw_feat_needed[i]) )
+ {
+ printk(XENLOG_INFO "ARM FF-A Firmware does not support 0x%08x\n",
+ ffa_fw_feat_needed[i]);
+ goto err_no_fw;
+ }
+ }
+
if ( !ffa_rxtx_init() )
{
printk(XENLOG_ERR "ffa: Error during RXTX buffer init\n");
Cheers,
--
Julien Grall
