[PATCH v3 04/10] xen/arm: ffa: Fine granular call support

Bertrand Marquis posted 10 patches 1 week ago
[PATCH v3 04/10] xen/arm: ffa: Fine granular call support
Posted by Bertrand Marquis 1 week ago
Create a bitmap to store which feature is supported or not by the
firmware and use it to filter which calls are done to the firmware.

While there reoder ABI definition by numbers to easily find the min and
max ones.

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:
- rename fw_feat to abi and macros to FFA_ABI to be coherent with the
  abi needed change done before
- rework the macros to be simpler by directly defining MIN and MAX using
  only Function ids
- check that requested function ids do not go over the bitmap size in
  ffa_fw_supports_fid
- add an ASSERT to make sure that we do not try to set bits outside of
  the bitmap
- turn off FF-A if there is not firmware support and adapt the commit
  message to reflect this
- add a compile time check that FFA_ABI_MIN < FFA_ABI_MAX
- remove spurious line removal
- restore proper cleanup of rxtx init in case of error
- reorder ABI by numbers
---
 xen/arch/arm/tee/ffa.c          | 28 +++++++++++++++---------
 xen/arch/arm/tee/ffa_notif.c    |  7 ++++++
 xen/arch/arm/tee/ffa_partinfo.c | 30 +++++++++++++++++++++++++-
 xen/arch/arm/tee/ffa_private.h  | 38 ++++++++++++++++++++++++++++-----
 xen/arch/arm/tee/ffa_rxtx.c     |  4 ++++
 xen/arch/arm/tee/ffa_shm.c      | 12 +++++++++++
 6 files changed, 103 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index c5dcb4fe240c..1b0919041dc1 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -72,7 +72,10 @@
 #include "ffa_private.h"
 
 /* Negotiated FF-A version to use with the SPMC, 0 if not there or supported */
-static uint32_t __ro_after_init ffa_fw_version;
+uint32_t __ro_after_init ffa_fw_version;
+
+/* Features supported by the SPMC or secure world when present */
+DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
 
 struct ffa_fw_abi {
     uint32_t id;
@@ -177,6 +180,13 @@ static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid)
     else
         mask = GENMASK_ULL(31, 0);
 
+    if ( !ffa_fw_supports_fid(fid) )
+    {
+        resp.a0 = FFA_ERROR;
+        resp.a2 = FFA_RET_NOT_SUPPORTED;
+        goto out;
+    }
+
     src_dst = get_user_reg(regs, 1);
     if ( (src_dst >> 16) != ffa_get_vm_id(d) )
     {
@@ -577,19 +587,16 @@ static bool ffa_probe(void)
     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
-     * features supported.
-     */
     for ( unsigned int i = 0; i < ARRAY_SIZE(ffa_fw_abi_needed); i++ )
     {
-        if ( !ffa_abi_supported(ffa_fw_abi_needed[i].id) )
-        {
+        ASSERT(FFA_ABI_BITNUM(ffa_fw_abi_needed[i].id) < FFA_ABI_BITMAP_SIZE);
+
+        if ( ffa_abi_supported(ffa_fw_abi_needed[i].id) )
+            set_bit(FFA_ABI_BITNUM(ffa_fw_abi_needed[i].id),
+                    ffa_fw_abi_supported);
+        else
             printk(XENLOG_INFO "ARM FF-A Firmware does not support %s\n",
                    ffa_fw_abi_needed[i].name);
-            goto err_no_fw;
-        }
     }
 
     if ( !ffa_rxtx_init() )
@@ -611,6 +618,7 @@ err_rxtx_destroy:
     ffa_rxtx_destroy();
 err_no_fw:
     ffa_fw_version = 0;
+    bitmap_zero(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
     printk(XENLOG_WARNING "ARM FF-A No firmware support\n");
 
     return false;
diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
index 541e61d2f606..4b3e46318f4b 100644
--- a/xen/arch/arm/tee/ffa_notif.c
+++ b/xen/arch/arm/tee/ffa_notif.c
@@ -377,6 +377,13 @@ void ffa_notif_init(void)
     unsigned int irq;
     int ret;
 
+    /* Only enable fw notification if all ABIs we need are supported */
+    if ( !(ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_CREATE) &&
+           ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_DESTROY) &&
+           ffa_fw_supports_fid(FFA_NOTIFICATION_GET) &&
+           ffa_fw_supports_fid(FFA_NOTIFICATION_INFO_GET_64)) )
+        return;
+
     arm_smccc_1_2_smc(&arg, &resp);
     if ( resp.a0 != FFA_SUCCESS_32 )
         return;
diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
index 93a03c6bc672..99c48f0e5c05 100644
--- a/xen/arch/arm/tee/ffa_partinfo.c
+++ b/xen/arch/arm/tee/ffa_partinfo.c
@@ -77,7 +77,15 @@ int32_t ffa_handle_partition_info_get(uint32_t w1, uint32_t w2, uint32_t w3,
      */
     if ( w5 == FFA_PARTITION_INFO_GET_COUNT_FLAG &&
          ctx->guest_vers == FFA_VERSION_1_1 )
-        return ffa_partition_info_get(w1, w2, w3, w4, w5, count, fpi_size);
+    {
+        if ( ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
+            return ffa_partition_info_get(w1, w2, w3, w4, w5, count, fpi_size);
+        else
+        {
+            *count = 0;
+            return FFA_RET_OK;
+        }
+    }
     if ( w5 )
         return FFA_RET_INVALID_PARAMETERS;
 
@@ -87,6 +95,18 @@ int32_t ffa_handle_partition_info_get(uint32_t w1, uint32_t w2, uint32_t w3,
     if ( !spin_trylock(&ctx->rx_lock) )
         return FFA_RET_BUSY;
 
+    if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
+    {
+        if ( ctx->guest_vers == FFA_VERSION_1_0 )
+            *fpi_size = sizeof(struct ffa_partition_info_1_0);
+        else
+            *fpi_size = sizeof(struct ffa_partition_info_1_1);
+
+        *count = 0;
+        ret = FFA_RET_OK;
+        goto out;
+    }
+
     if ( !ctx->page_count || !ctx->rx_is_free )
         goto out;
     spin_lock(&ffa_rx_buffer_lock);
@@ -250,6 +270,11 @@ bool ffa_partinfo_init(void)
     uint32_t count;
     int e;
 
+    if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) ||
+         !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) ||
+         !ffa_rx || !ffa_tx )
+        return false;
+
     e = ffa_partition_info_get(0, 0, 0, 0, 0, &count, &fpi_size);
     if ( e )
     {
@@ -313,6 +338,9 @@ int ffa_partinfo_domain_init(struct domain *d)
     unsigned int n;
     int32_t res;
 
+    if ( !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) )
+        return 0;
+
     ctx->vm_destroy_bitmap = xzalloc_array(unsigned long, count);
     if ( !ctx->vm_destroy_bitmap )
         return -ENOMEM;
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 045d9c4a0b56..85eb61c13464 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -14,6 +14,7 @@
 #include <xen/spinlock.h>
 #include <xen/sched.h>
 #include <xen/time.h>
+#include <xen/bitmap.h>
 
 /* Error codes */
 #define FFA_RET_OK                      0
@@ -201,18 +202,17 @@
 #define FFA_INTERRUPT                   0x84000062U
 #define FFA_VERSION                     0x84000063U
 #define FFA_FEATURES                    0x84000064U
-#define FFA_RX_ACQUIRE                  0x84000084U
 #define FFA_RX_RELEASE                  0x84000065U
 #define FFA_RXTX_MAP_32                 0x84000066U
 #define FFA_RXTX_MAP_64                 0xC4000066U
 #define FFA_RXTX_UNMAP                  0x84000067U
 #define FFA_PARTITION_INFO_GET          0x84000068U
 #define FFA_ID_GET                      0x84000069U
-#define FFA_SPM_ID_GET                  0x84000085U
+#define FFA_MSG_POLL                    0x8400006AU
 #define FFA_MSG_WAIT                    0x8400006BU
 #define FFA_MSG_YIELD                   0x8400006CU
 #define FFA_RUN                         0x8400006DU
-#define FFA_MSG_SEND2                   0x84000086U
+#define FFA_MSG_SEND                    0x8400006EU
 #define FFA_MSG_SEND_DIRECT_REQ_32      0x8400006FU
 #define FFA_MSG_SEND_DIRECT_REQ_64      0xC400006FU
 #define FFA_MSG_SEND_DIRECT_RESP_32     0x84000070U
@@ -230,8 +230,6 @@
 #define FFA_MEM_RECLAIM                 0x84000077U
 #define FFA_MEM_FRAG_RX                 0x8400007AU
 #define FFA_MEM_FRAG_TX                 0x8400007BU
-#define FFA_MSG_SEND                    0x8400006EU
-#define FFA_MSG_POLL                    0x8400006AU
 #define FFA_NOTIFICATION_BITMAP_CREATE  0x8400007DU
 #define FFA_NOTIFICATION_BITMAP_DESTROY 0x8400007EU
 #define FFA_NOTIFICATION_BIND           0x8400007FU
@@ -240,6 +238,25 @@
 #define FFA_NOTIFICATION_GET            0x84000082U
 #define FFA_NOTIFICATION_INFO_GET_32    0x84000083U
 #define FFA_NOTIFICATION_INFO_GET_64    0xC4000083U
+#define FFA_RX_ACQUIRE                  0x84000084U
+#define FFA_SPM_ID_GET                  0x84000085U
+#define FFA_MSG_SEND2                   0x84000086U
+
+/**
+ * Encoding of features supported or not by the fw in a bitmap:
+ * - Function IDs are going from 0x60 to 0xFF
+ * - A function can be supported in 32 and/or 64bit
+ * The bitmap has one bit for each function in 32 and 64 bit.
+ */
+#define FFA_ABI_ID(id)        ((id) & ARM_SMCCC_FUNC_MASK)
+#define FFA_ABI_CONV(id)      (((id) >> ARM_SMCCC_CONV_SHIFT) & BIT(0,U))
+
+#define FFA_ABI_MIN           FFA_ABI_ID(FFA_ERROR)
+#define FFA_ABI_MAX           FFA_ABI_ID(FFA_MSG_SEND2)
+
+#define FFA_ABI_BITMAP_SIZE   (2 * (FFA_ABI_MAX - FFA_ABI_MIN + 1))
+#define FFA_ABI_BITNUM(id)    ((FFA_ABI_ID(id) - FFA_ABI_MIN) << 1 | \
+                               FFA_ABI_CONV(id))
 
 struct ffa_ctx_notif {
     bool enabled;
@@ -289,6 +306,8 @@ extern void *ffa_rx;
 extern void *ffa_tx;
 extern spinlock_t ffa_rx_buffer_lock;
 extern spinlock_t ffa_tx_buffer_lock;
+extern uint32_t __ro_after_init ffa_fw_version;
+extern DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
 
 bool ffa_shm_domain_destroy(struct domain *d);
 void ffa_handle_mem_share(struct cpu_user_regs *regs);
@@ -401,4 +420,13 @@ static inline int32_t ffa_rx_release(void)
     return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
 }
 
+static inline bool ffa_fw_supports_fid(uint32_t fid)
+{
+    BUILD_BUG_ON(FFA_ABI_MIN > FFA_ABI_MAX);
+
+    if ( FFA_ABI_BITNUM(fid) > FFA_ABI_BITMAP_SIZE)
+        return false;
+    return test_bit(FFA_ABI_BITNUM(fid), ffa_fw_abi_supported);
+}
+
 #endif /*__FFA_PRIVATE_H__*/
diff --git a/xen/arch/arm/tee/ffa_rxtx.c b/xen/arch/arm/tee/ffa_rxtx.c
index c8bc516a19be..132a7982407b 100644
--- a/xen/arch/arm/tee/ffa_rxtx.c
+++ b/xen/arch/arm/tee/ffa_rxtx.c
@@ -193,6 +193,10 @@ bool ffa_rxtx_init(void)
 {
     int e;
 
+    /* Firmware not there or not supporting */
+    if ( !ffa_fw_supports_fid(FFA_RXTX_MAP_64) )
+        return false;
+
     ffa_rx = alloc_xenheap_pages(get_order_from_pages(FFA_RXTX_PAGE_COUNT), 0);
     if ( !ffa_rx )
         return false;
diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
index 370d83ec5cf8..efa5b67db8e1 100644
--- a/xen/arch/arm/tee/ffa_shm.c
+++ b/xen/arch/arm/tee/ffa_shm.c
@@ -149,6 +149,9 @@ static int32_t ffa_mem_share(uint32_t tot_len, uint32_t frag_len,
 static int32_t ffa_mem_reclaim(uint32_t handle_lo, uint32_t handle_hi,
                                uint32_t flags)
 {
+    if ( !ffa_fw_supports_fid(FFA_MEM_RECLAIM) )
+        return FFA_RET_NOT_SUPPORTED;
+
     return ffa_simple_call(FFA_MEM_RECLAIM, handle_lo, handle_hi, flags, 0);
 }
 
@@ -467,6 +470,12 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
     uint32_t range_count;
     uint32_t region_offs;
 
+    if ( !ffa_fw_supports_fid(FFA_MEM_SHARE_64) )
+    {
+        ret = FFA_RET_NOT_SUPPORTED;
+        goto out_set_ret;
+    }
+
     /*
      * We're only accepting memory transaction descriptors via the rx/tx
      * buffer.
@@ -621,6 +630,9 @@ int ffa_handle_mem_reclaim(uint64_t handle, uint32_t flags)
     register_t handle_lo;
     int ret;
 
+    if ( !ffa_fw_supports_fid(FFA_MEM_RECLAIM) )
+        return FFA_RET_NOT_SUPPORTED;
+
     spin_lock(&ctx->lock);
     shm = find_shm_mem(ctx, handle);
     if ( shm )
-- 
2.47.0