xen/arm: ffa: Transmit RXTX buffers to the SPMC

When an RXTX buffer is mapped by a VM transmit it to the SPMC when it
supports RX_ACQUIRE.
As a consequence of that, we must acquire the RX buffer of a VM from the
SPMC when we want to use it:
- create a generic acquire and release function to get the rx buffer of
  a VM which gets it from the SPMC when supported
- rename the rx_acquire to hyp_rx_acquire to remove confusion
- rework the rx_lock to only lock access to rx_is_free and only allow
  usage of the rx buffer to one who managed to acquire it, thus removing
  the trylock and returning busy if rx_is_free is false

As part of this change move some structure definition to ffa_private
from ffa_shm as those are need for the MAP call with the SPMC.

While there also fix ffa_handle_rxtx_map which was testing the wrong
variable after getting the page for the rx buffer, testing tx_pg
instead of rx_pg.

Fixes: be75f686eb ("xen/arm: ffa: separate rxtx buffer routines")

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

xen/arch/arm/tee/ffa.c

@@ -347,7 +347,7 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
         ffa_handle_partition_info_get(regs);
         return true;
     case FFA_RX_RELEASE:
-        e = ffa_handle_rx_release();
+        e = ffa_rx_release(d);
         break;
     case FFA_MSG_SEND_DIRECT_REQ_32:
     case FFA_MSG_SEND_DIRECT_REQ_64:

xen/arch/arm/tee/ffa_partinfo.c

@@ -121,11 +121,9 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
         goto out;
     }
 
-    if ( !spin_trylock(&ctx->rx_lock) )
-    {
-        ret = FFA_RET_BUSY;
+    ret = ffa_rx_acquire(d);
+    if ( ret != FFA_RET_OK )
         goto out;
-    }
 
     dst_buf = ctx->rx;
 
@@ -135,22 +133,16 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
         goto out_rx_release;
     }
 
-    if ( !ctx->page_count || !ctx->rx_is_free )
-    {
-        ret = FFA_RET_DENIED;
-        goto out_rx_release;
-    }
-
     spin_lock(&ffa_rx_buffer_lock);
 
     ret = ffa_partition_info_get(uuid, 0, &ffa_sp_count, &src_size);
 
     if ( ret )
-        goto out_rx_buf_unlock;
+        goto out_rx_hyp_unlock;
 
     /*
      * ffa_partition_info_get() succeeded so we now own the RX buffer we
-     * share with the SPMC. We must give it back using ffa_rx_release()
+     * share with the SPMC. We must give it back using ffa_hyp_rx_release()
      * once we've copied the content.
      */
 
@@ -190,15 +182,20 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs)
         }
     }
 
-    ctx->rx_is_free = false;
-
 out_rx_hyp_release:
-    ffa_rx_release();
-out_rx_buf_unlock:
+    ffa_hyp_rx_release();
+out_rx_hyp_unlock:
     spin_unlock(&ffa_rx_buffer_lock);
 out_rx_release:
-    spin_unlock(&ctx->rx_lock);
-
+    /*
+     * The calling VM RX buffer only contains data to be used by the VM if the
+     * call was successful, in which case the VM has to release the buffer
+     * once it has used the data.
+     * If something went wrong during the call, we have to release the RX
+     * buffer back to the SPMC as the VM will not do it.
+     */
+    if ( ret != FFA_RET_OK )
+        ffa_rx_release(d);
 out:
     if ( ret )
         ffa_set_regs_error(regs, ret);
@@ -365,8 +362,7 @@ bool ffa_partinfo_init(void)
     ret = init_subscribers(count, fpi_size);
 
 out:
-    ffa_rx_release();
-
+    ffa_hyp_rx_release();
     return ret;
 }
 

xen/arch/arm/tee/ffa_private.h

@@ -265,6 +265,21 @@
 #define FFA_ABI_BITNUM(id)    ((FFA_ABI_ID(id) - FFA_ABI_MIN) << 1 | \
                                FFA_ABI_CONV(id))
 
+/* Constituent memory region descriptor */
+struct ffa_address_range {
+    uint64_t address;
+    uint32_t page_count;
+    uint32_t reserved;
+};
+
+/* Composite memory region descriptor */
+struct ffa_mem_region {
+    uint32_t total_page_count;
+    uint32_t address_range_count;
+    uint64_t reserved;
+    struct ffa_address_range address_range_array[];
+};
+
 struct ffa_ctx_notif {
     bool enabled;
 
@@ -292,7 +307,7 @@ struct ffa_ctx {
     struct ffa_ctx_notif notif;
     /*
      * tx_lock is used to serialize access to tx
-     * rx_lock is used to serialize access to rx
+     * rx_lock is used to serialize access to rx_is_free
      * lock is used for the rest in this struct
      */
     spinlock_t tx_lock;
@@ -331,7 +346,8 @@ void ffa_rxtx_domain_destroy(struct domain *d);
 uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
 			     register_t rx_addr, uint32_t page_count);
 uint32_t ffa_handle_rxtx_unmap(void);
-int32_t ffa_handle_rx_release(void);
+int32_t ffa_rx_acquire(struct domain *d);
+int32_t ffa_rx_release(struct domain *d);
 
 void ffa_notif_init(void);
 void ffa_notif_init_interrupt(void);
@@ -420,7 +436,7 @@ static inline int32_t ffa_simple_call(uint32_t fid, register_t a1,
     return ffa_get_ret_code(&resp);
 }
 
-static inline int32_t ffa_rx_release(void)
+static inline int32_t ffa_hyp_rx_release(void)
 {
     return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
 }

xen/arch/arm/tee/ffa_rxtx.c

@@ -30,6 +30,17 @@ struct ffa_endpoint_rxtx_descriptor_1_1 {
     uint32_t tx_region_offs;
 };
 
+static int32_t ffa_rxtx_map(paddr_t tx_addr, paddr_t rx_addr,
+                            uint32_t page_count)
+{
+    return ffa_simple_call(FFA_RXTX_MAP_64, tx_addr, rx_addr, page_count, 0);
+}
+
+static int32_t ffa_rxtx_unmap(uint16_t id)
+{
+    return ffa_simple_call(FFA_RXTX_UNMAP, ((uint64_t)id) << 16, 0, 0, 0);
+}
+
 uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
 			     register_t rx_addr, uint32_t page_count)
 {
@@ -42,6 +53,9 @@ uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
     void *rx;
     void *tx;
 
+    /* The code is considering that we only get one page for now */
+    BUILD_BUG_ON(FFA_MAX_RXTX_PAGE_COUNT != 1);
+
     if ( !smccc_is_conv_64(fid) )
     {
         /*
@@ -72,7 +86,7 @@ uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
         goto err_put_tx_pg;
 
     rx_pg = get_page_from_gfn(d, gfn_x(gaddr_to_gfn(rx_addr)), &t, P2M_ALLOC);
-    if ( !tx_pg )
+    if ( !rx_pg )
         goto err_put_tx_pg;
 
     /* Only normal RW RAM for now */
@@ -87,6 +101,66 @@ uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
     if ( !rx )
         goto err_unmap_tx;
 
+    /*
+     * Transmit the RX/TX buffer information to the SPM if acquire is supported
+     * as the spec says that if not there is not need to acquire/release/map
+     * rxtx buffers from the SPMC
+     */
+    if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) )
+    {
+        struct ffa_endpoint_rxtx_descriptor_1_1 *rxtx_desc;
+        struct ffa_mem_region *mem_reg;
+
+        /* All must fit in our TX buffer */
+        BUILD_BUG_ON(sizeof(*rxtx_desc) + sizeof(*mem_reg) * 2 +
+                     sizeof(struct ffa_address_range) * 2 >
+                     FFA_MAX_RXTX_PAGE_COUNT * FFA_PAGE_SIZE);
+
+        spin_lock(&ffa_tx_buffer_lock);
+        rxtx_desc = ffa_tx;
+
+        /*
+         * We have only one page for each so we pack everything:
+         * - rx region descriptor
+         * - rx region range
+         * - tx region descriptor
+         * - tx region range
+         */
+        rxtx_desc->sender_id = ffa_get_vm_id(d);
+        rxtx_desc->reserved = 0;
+        rxtx_desc->rx_region_offs = sizeof(*rxtx_desc);
+        rxtx_desc->tx_region_offs = sizeof(*rxtx_desc) +
+                                    offsetof(struct ffa_mem_region,
+                                             address_range_array[1]);
+
+        /* rx buffer */
+        mem_reg = ffa_tx + sizeof(*rxtx_desc);
+        mem_reg->total_page_count = 1;
+        mem_reg->address_range_count = 1;
+        mem_reg->reserved = 0;
+
+        mem_reg->address_range_array[0].address = page_to_maddr(rx_pg);
+        mem_reg->address_range_array[0].page_count = 1;
+        mem_reg->address_range_array[0].reserved = 0;
+
+        /* tx buffer */
+        mem_reg = ffa_tx + rxtx_desc->tx_region_offs;
+        mem_reg->total_page_count = 1;
+        mem_reg->address_range_count = 1;
+        mem_reg->reserved = 0;
+
+        mem_reg->address_range_array[0].address = page_to_maddr(tx_pg);
+        mem_reg->address_range_array[0].page_count = 1;
+        mem_reg->address_range_array[0].reserved = 0;
+
+        ret = ffa_rxtx_map(0, 0, 0);
+
+        spin_unlock(&ffa_tx_buffer_lock);
+
+        if ( ret != FFA_RET_OK )
+            goto err_unmap_rx;
+    }
+
     ctx->rx = rx;
     ctx->tx = tx;
     ctx->rx_pg = rx_pg;
@@ -95,6 +169,8 @@ uint32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr,
     ctx->rx_is_free = true;
     return FFA_RET_OK;
 
+err_unmap_rx:
+    unmap_domain_page_global(rx);
 err_unmap_tx:
     unmap_domain_page_global(tx);
 err_put_rx_pg:
@@ -105,8 +181,22 @@ err_put_tx_pg:
     return ret;
 }
 
-static void rxtx_unmap(struct ffa_ctx *ctx)
+static uint32_t  rxtx_unmap(struct domain *d)
 {
+    struct ffa_ctx *ctx = d->arch.tee;
+
+    if ( !ctx->page_count )
+        return FFA_RET_INVALID_PARAMETERS;
+
+    if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) )
+    {
+        uint32_t ret;
+
+        ret = ffa_rxtx_unmap(ffa_get_vm_id(d));
+        if ( ret != FFA_RET_OK )
+            return ret;
+    }
+
     unmap_domain_page_global(ctx->rx);
     unmap_domain_page_global(ctx->tx);
     put_page(ctx->rx_pg);
@@ -117,32 +207,63 @@ static void rxtx_unmap(struct ffa_ctx *ctx)
     ctx->tx_pg = NULL;
     ctx->page_count = 0;
     ctx->rx_is_free = false;
-}
-
-uint32_t ffa_handle_rxtx_unmap(void)
-{
-    struct domain *d = current->domain;
-    struct ffa_ctx *ctx = d->arch.tee;
-
-    if ( !ctx->rx )
-        return FFA_RET_INVALID_PARAMETERS;
-
-    rxtx_unmap(ctx);
 
     return FFA_RET_OK;
 }
 
-int32_t ffa_handle_rx_release(void)
+uint32_t ffa_handle_rxtx_unmap(void)
 {
-    int32_t ret = FFA_RET_DENIED;
-    struct domain *d = current->domain;
+    return rxtx_unmap(current->domain);
+}
+
+int32_t ffa_rx_acquire(struct domain *d)
+{
+    int32_t ret = FFA_RET_OK;
     struct ffa_ctx *ctx = d->arch.tee;
 
-    if ( !spin_trylock(&ctx->rx_lock) )
-        return FFA_RET_BUSY;
+    spin_lock(&ctx->rx_lock);
+
+    if ( !ctx->page_count )
+    {
+        ret = FFA_RET_DENIED;
+        goto out;
+    }
+
+    if ( !ctx->rx_is_free )
+    {
+        ret = FFA_RET_BUSY;
+        goto out;
+    }
+
+    if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) )
+    {
+        ret = ffa_simple_call(FFA_RX_ACQUIRE, ffa_get_vm_id(d), 0, 0, 0);
+        if ( ret != FFA_RET_OK )
+            goto out;
+    }
+    ctx->rx_is_free = false;
+out:
+    spin_unlock(&ctx->rx_lock);
+
+    return ret;
+}
+
+int32_t ffa_rx_release(struct domain *d)
+{
+    int32_t ret = FFA_RET_DENIED;
+    struct ffa_ctx *ctx = d->arch.tee;
+
+    spin_lock(&ctx->rx_lock);
 
     if ( !ctx->page_count || ctx->rx_is_free )
         goto out;
+
+    if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) )
+    {
+        ret = ffa_simple_call(FFA_RX_RELEASE, ffa_get_vm_id(d), 0, 0, 0);
+        if ( ret != FFA_RET_OK )
+            goto out;
+    }
     ret = FFA_RET_OK;
     ctx->rx_is_free = true;
 out:
@@ -151,23 +272,9 @@ out:
     return ret;
 }
 
-static int32_t ffa_rxtx_map(paddr_t tx_addr, paddr_t rx_addr,
-                            uint32_t page_count)
-{
-    return ffa_simple_call(FFA_RXTX_MAP_64, tx_addr, rx_addr, page_count, 0);
-}
-
-static int32_t ffa_rxtx_unmap(void)
-{
-    return ffa_simple_call(FFA_RXTX_UNMAP, 0, 0, 0, 0);
-}
-
 void ffa_rxtx_domain_destroy(struct domain *d)
 {
-    struct ffa_ctx *ctx = d->arch.tee;
-
-    if ( ctx->rx )
-        rxtx_unmap(ctx);
+    rxtx_unmap(d);
 }
 
 void ffa_rxtx_destroy(void)
@@ -186,7 +293,7 @@ void ffa_rxtx_destroy(void)
     }
 
     if ( need_unmap )
-        ffa_rxtx_unmap();
+        ffa_rxtx_unmap(0);
 }
 
 bool ffa_rxtx_init(void)

xen/arch/arm/tee/ffa_shm.c

@@ -16,21 +16,6 @@
 
 #include "ffa_private.h"
 
-/* Constituent memory region descriptor */
-struct ffa_address_range {
-    uint64_t address;
-    uint32_t page_count;
-    uint32_t reserved;
-};
-
-/* Composite memory region descriptor */
-struct ffa_mem_region {
-    uint32_t total_page_count;
-    uint32_t address_range_count;
-    uint64_t reserved;
-    struct ffa_address_range address_range_array[];
-};
-
 /* Memory access permissions descriptor */
 struct ffa_mem_access_perm {
     uint16_t endpoint_id;