Rework access to VM RX/TX buffer to use acquire/release functions equivalent to the ones used for the SPMC RX/TX buffers and remove all direct access to ctx->tx or ctx->rx by giving back the buffer pointer and size back in acquire.
This design ensures that rx or page_count is not accessed without the lock held and limit direct usage of the context rx/tx buffer info to ffa_rxtx.c Modify msg, partinfo and shm code to use the new RX/TX buffer acquire/release functions and remove all direct accesses to rx/tx and page_count so that any access is done only with the lock taken. Signed-off-by: Bertrand Marquis <[email protected]> --- Changes in v1: - patch introduced --- xen/arch/arm/tee/ffa.c | 2 +- xen/arch/arm/tee/ffa_msg.c | 32 ++++++++++++------------ xen/arch/arm/tee/ffa_partinfo.c | 8 +++--- xen/arch/arm/tee/ffa_private.h | 6 +++-- xen/arch/arm/tee/ffa_rxtx.c | 43 ++++++++++++++++++++++++++++----- xen/arch/arm/tee/ffa_shm.c | 18 +++++++------- 6 files changed, 72 insertions(+), 37 deletions(-) diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c index 43af49d1c011..69a5e1e876ce 100644 --- a/xen/arch/arm/tee/ffa.c +++ b/xen/arch/arm/tee/ffa.c @@ -345,7 +345,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_rx_release(d); + e = ffa_rx_release(ctx); break; case FFA_MSG_SEND_DIRECT_REQ_32: case FFA_MSG_SEND_DIRECT_REQ_64: diff --git a/xen/arch/arm/tee/ffa_msg.c b/xen/arch/arm/tee/ffa_msg.c index 2c2ebc9c5cd6..d60eed6d8811 100644 --- a/xen/arch/arm/tee/ffa_msg.c +++ b/xen/arch/arm/tee/ffa_msg.c @@ -94,6 +94,8 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf, struct domain *dst_d; struct ffa_ctx *dst_ctx; struct ffa_part_msg_rxtx *dst_msg; + void *rx_buf; + size_t rx_size; int err; int32_t ret; @@ -120,20 +122,19 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf, } /* This also checks that destination has set a Rx buffer */ - ret = ffa_rx_acquire(dst_d); + ret = ffa_rx_acquire(dst_ctx , &rx_buf, &rx_size); if ( ret ) goto out_unlock; /* we need to have enough space in the destination buffer */ - if ( (dst_ctx->page_count * FFA_PAGE_SIZE - - sizeof(struct ffa_part_msg_rxtx)) < src_msg->msg_size ) + if ( (rx_size - sizeof(struct ffa_part_msg_rxtx)) < src_msg->msg_size ) { ret = FFA_RET_NO_MEMORY; - ffa_rx_release(dst_d); + ffa_rx_release(dst_ctx); goto out_unlock; } - dst_msg = dst_ctx->rx; + dst_msg = rx_buf; /* prepare destination header */ dst_msg->flags = 0; @@ -142,7 +143,7 @@ static int32_t ffa_msg_send2_vm(uint16_t dst_id, const void *src_buf, dst_msg->send_recv_id = src_msg->send_recv_id; dst_msg->msg_size = src_msg->msg_size; - memcpy(dst_ctx->rx + sizeof(struct ffa_part_msg_rxtx), + memcpy(rx_buf + sizeof(struct ffa_part_msg_rxtx), src_buf + src_msg->msg_offset, src_msg->msg_size); /* receiver rx buffer will be released by the receiver*/ @@ -159,17 +160,20 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs) { struct domain *src_d = current->domain; struct ffa_ctx *src_ctx = src_d->arch.tee; + const void *tx_buf; + size_t tx_size; struct ffa_part_msg_rxtx src_msg; uint16_t dst_id, src_id; int32_t ret; BUILD_BUG_ON(sizeof(struct ffa_part_msg_rxtx) >= FFA_PAGE_SIZE); - if ( !spin_trylock(&src_ctx->tx_lock) ) - return FFA_RET_BUSY; + ret = ffa_tx_acquire(src_ctx, &tx_buf, &tx_size); + if ( ret != FFA_RET_OK ) + return ret; /* create a copy of the message header */ - memcpy(&src_msg, src_ctx->tx, sizeof(src_msg)); + memcpy(&src_msg, tx_buf, sizeof(src_msg)); src_id = src_msg.send_recv_id >> 16; dst_id = src_msg.send_recv_id & GENMASK(15,0); @@ -182,10 +186,8 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs) /* check source message fits in buffer */ if ( src_msg.msg_offset < sizeof(struct ffa_part_msg_rxtx) || - src_msg.msg_size == 0 || - src_msg.msg_offset > src_ctx->page_count * FFA_PAGE_SIZE || - src_msg.msg_size > (src_ctx->page_count * FFA_PAGE_SIZE - - src_msg.msg_offset) ) + src_msg.msg_size == 0 || src_msg.msg_offset > tx_size || + src_msg.msg_size > (tx_size - src_msg.msg_offset) ) { ret = FFA_RET_INVALID_PARAMETERS; goto out; @@ -206,12 +208,12 @@ int32_t ffa_handle_msg_send2(struct cpu_user_regs *regs) else if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) ) { /* Message for a VM */ - ret = ffa_msg_send2_vm(dst_id, src_ctx->tx, &src_msg); + ret = ffa_msg_send2_vm(dst_id, tx_buf, &src_msg); } else ret = FFA_RET_INVALID_PARAMETERS; out: - spin_unlock(&src_ctx->tx_lock); + ffa_tx_release(src_ctx); return ret; } diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c index 145b869957b0..16c905cb12b8 100644 --- a/xen/arch/arm/tee/ffa_partinfo.c +++ b/xen/arch/arm/tee/ffa_partinfo.c @@ -224,6 +224,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs) get_user_reg(regs, 4), }; uint32_t dst_size = 0; + size_t buf_size; void *dst_buf, *end_buf; uint32_t ffa_vm_count = 0, ffa_sp_count = 0; @@ -268,12 +269,11 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs) } /* Get the RX buffer to write the list of partitions */ - ret = ffa_rx_acquire(d); + ret = ffa_rx_acquire(ctx, &dst_buf, &buf_size); if ( ret != FFA_RET_OK ) goto out; - dst_buf = ctx->rx; - end_buf = ctx->rx + ctx->page_count * FFA_PAGE_SIZE; + end_buf = dst_buf + buf_size; /* An entry should be smaller than a page */ BUILD_BUG_ON(sizeof(struct ffa_partition_info_1_1) > FFA_PAGE_SIZE); @@ -304,7 +304,7 @@ void ffa_handle_partition_info_get(struct cpu_user_regs *regs) out_rx_release: if ( ret ) - ffa_rx_release(d); + ffa_rx_release(ctx); out: if ( ret ) ffa_set_regs_error(regs, ret); diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h index 8797a62abd01..0d1bab6cc700 100644 --- a/xen/arch/arm/tee/ffa_private.h +++ b/xen/arch/arm/tee/ffa_private.h @@ -444,8 +444,10 @@ void ffa_rxtx_domain_destroy(struct domain *d); int32_t ffa_handle_rxtx_map(uint32_t fid, register_t tx_addr, register_t rx_addr, uint32_t page_count); int32_t ffa_handle_rxtx_unmap(void); -int32_t ffa_rx_acquire(struct domain *d); -int32_t ffa_rx_release(struct domain *d); +int32_t ffa_rx_acquire(struct ffa_ctx *ctx, void **buf, size_t *buf_size); +int32_t ffa_rx_release(struct ffa_ctx *ctx); +int32_t ffa_tx_acquire(struct ffa_ctx *ctx, const void **buf, size_t *buf_size); +int32_t ffa_tx_release(struct ffa_ctx *ctx); void ffa_notif_init(void); void ffa_notif_init_interrupt(void); diff --git a/xen/arch/arm/tee/ffa_rxtx.c b/xen/arch/arm/tee/ffa_rxtx.c index e325eae07bda..f79f2cf406aa 100644 --- a/xen/arch/arm/tee/ffa_rxtx.c +++ b/xen/arch/arm/tee/ffa_rxtx.c @@ -257,10 +257,9 @@ int32_t ffa_handle_rxtx_unmap(void) return rxtx_unmap(current->domain); } -int32_t ffa_rx_acquire(struct domain *d) +int32_t ffa_rx_acquire(struct ffa_ctx *ctx, void **buf, size_t *buf_size) { int32_t ret = FFA_RET_OK; - struct ffa_ctx *ctx = d->arch.tee; spin_lock(&ctx->rx_lock); @@ -278,21 +277,22 @@ int32_t ffa_rx_acquire(struct domain *d) if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) ) { - ret = ffa_simple_call(FFA_RX_ACQUIRE, ffa_get_vm_id(d), 0, 0, 0); + ret = ffa_simple_call(FFA_RX_ACQUIRE, ctx->ffa_id, 0, 0, 0); if ( ret != FFA_RET_OK ) goto out; } ctx->rx_is_free = false; + *buf = ctx->rx; + *buf_size = ctx->page_count * FFA_PAGE_SIZE; out: spin_unlock(&ctx->rx_lock); return ret; } -int32_t ffa_rx_release(struct domain *d) +int32_t ffa_rx_release(struct ffa_ctx *ctx) { int32_t ret = FFA_RET_DENIED; - struct ffa_ctx *ctx = d->arch.tee; spin_lock(&ctx->rx_lock); @@ -301,7 +301,7 @@ int32_t ffa_rx_release(struct domain *d) if ( ffa_fw_supports_fid(FFA_RX_ACQUIRE) ) { - ret = ffa_simple_call(FFA_RX_RELEASE, ffa_get_vm_id(d), 0, 0, 0); + ret = ffa_simple_call(FFA_RX_RELEASE, ctx->ffa_id, 0, 0, 0); if ( ret != FFA_RET_OK ) goto out; } @@ -313,6 +313,37 @@ out: return ret; } +int32_t ffa_tx_acquire(struct ffa_ctx *ctx, const void **buf, size_t *buf_size) +{ + int32_t ret = FFA_RET_DENIED; + + if ( !spin_trylock(&ctx->tx_lock) ) + return FFA_RET_BUSY; + + if ( !ctx->page_count ) + goto err_unlock; + + if ( !ctx->tx ) + goto err_unlock; + + *buf = ctx->tx; + *buf_size = ctx->page_count * FFA_PAGE_SIZE; + return FFA_RET_OK; + +err_unlock: + spin_unlock(&ctx->tx_lock); + + return ret; +} + +int32_t ffa_tx_release(struct ffa_ctx *ctx) +{ + ASSERT(spin_is_locked(&ctx->tx_lock)); + + spin_unlock(&ctx->tx_lock); + return FFA_RET_OK; +} + int32_t ffa_rxtx_domain_init(struct domain *d) { struct ffa_ctx *ctx = d->arch.tee; diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c index e275d3769d9b..b862578c553c 100644 --- a/xen/arch/arm/tee/ffa_shm.c +++ b/xen/arch/arm/tee/ffa_shm.c @@ -460,6 +460,8 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs) struct ffa_mem_transaction_int trans; struct domain *d = current->domain; struct ffa_ctx *ctx = d->arch.tee; + const void *tx_buf; + size_t tx_size; struct ffa_shm_mem *shm = NULL; register_t handle_hi = 0; register_t handle_lo = 0; @@ -498,16 +500,14 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs) goto out_set_ret; } - if ( !spin_trylock(&ctx->tx_lock) ) - { - ret = FFA_RET_BUSY; + ret = ffa_tx_acquire(ctx, &tx_buf, &tx_size); + if ( ret != FFA_RET_OK ) goto out_set_ret; - } - if ( frag_len > ctx->page_count * FFA_PAGE_SIZE ) + if ( frag_len > tx_size ) goto out_unlock; - ret = read_mem_transaction(ACCESS_ONCE(ctx->guest_vers), ctx->tx, + ret = read_mem_transaction(ACCESS_ONCE(ctx->guest_vers), tx_buf, frag_len, &trans); if ( ret ) goto out_unlock; @@ -535,7 +535,7 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs) if ( trans.mem_access_offs + trans.mem_access_size > frag_len ) goto out_unlock; - mem_access = ctx->tx + trans.mem_access_offs; + mem_access = tx_buf + trans.mem_access_offs; dst_id = ACCESS_ONCE(mem_access->access_perm.endpoint_id); if ( !FFA_ID_IS_SECURE(dst_id) ) @@ -558,7 +558,7 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs) goto out_unlock; } - region_descr = ctx->tx + region_offs; + region_descr = tx_buf + region_offs; range_count = ACCESS_ONCE(region_descr->address_range_count); page_count = ACCESS_ONCE(region_descr->total_page_count); @@ -605,7 +605,7 @@ out: if ( ret ) free_ffa_shm_mem(d, shm); out_unlock: - spin_unlock(&ctx->tx_lock); + ffa_tx_release(ctx); out_set_ret: if ( ret == 0) -- 2.51.2
