Hi Bertrand, On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis <[email protected]> wrote: > > On PARTITION_INFO_GET error paths, Xen unconditionally called > FFA_RX_RELEASE for the SPMC RX buffer. If the SPMC didn't grant RX > ownership (i.e., the call failed early), this issues a spurious release > that returns DENIED and produces warnings. > > Modify ffa_rxtx_spmc_rx_release() to return the release status and let > callers choose whether to log it. Only issue FFA_RX_RELEASE after a > successful PARTINFO SMC, while always releasing the local RX lock to > avoid deadlocks. > > Update handle_partition_info_get() to only release the SPMC RX buffer > after successful fw_ret checks, and ignore release errors during the > error path. > > Functional impact: eliminates spurious FFA_RX_RELEASE calls and > associated DENIED warnings when PARTITION_INFO_GET fails before > obtaining SPMC RX buffer ownership. > > Signed-off-by: Bertrand Marquis <[email protected]> > --- > xen/arch/arm/tee/ffa_partinfo.c | 14 ++++++++++++-- > xen/arch/arm/tee/ffa_private.h | 2 +- > xen/arch/arm/tee/ffa_rxtx.c | 14 +++++++++----- > 3 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c > index bf906ed0c88f..6b01c4abe915 100644 > --- a/xen/arch/arm/tee/ffa_partinfo.c > +++ b/xen/arch/arm/tee/ffa_partinfo.c > @@ -92,9 +92,11 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, > uint32_t *sp_count, > uint32_t dst_size) > { > int32_t ret; > + int32_t release_ret; > uint32_t src_size, real_sp_count; > void *src_buf; > uint32_t count = 0; > + bool spmc_ok = false;
Wouldn't notify_fw be clearer, and the same in ffa_partinfo_init()? Either way, please add: Reviewed-by: Jens Wiklander <[email protected]> Cheers, Jens > > /* We need to use the RX buffer to receive the list */ > src_buf = ffa_rxtx_spmc_rx_acquire(); > @@ -104,6 +106,7 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, > uint32_t *sp_count, > ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size); > if ( ret ) > goto out; > + spmc_ok = true; > > /* Validate the src_size we got */ > if ( src_size < sizeof(struct ffa_partition_info_1_0) || > @@ -157,7 +160,10 @@ static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, > uint32_t *sp_count, > *sp_count = count; > > out: > - ffa_rxtx_spmc_rx_release(); > + release_ret = ffa_rxtx_spmc_rx_release(spmc_ok); > + if ( release_ret ) > + gprintk(XENLOG_WARNING, > + "ffa: Error releasing SPMC RX buffer: %d\n", release_ret); > return ret; > } > > @@ -507,6 +513,7 @@ bool ffa_partinfo_init(void) > int32_t e; > void *spmc_rx; > struct ffa_uuid nil_uuid = { .val = { 0ULL, 0ULL } }; > + bool spmc_ok = false; > > if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) || > !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32)) > @@ -522,6 +529,7 @@ bool ffa_partinfo_init(void) > printk(XENLOG_ERR "ffa: Failed to get list of SPs: %d\n", e); > goto out; > } > + spmc_ok = true; > > if ( count >= FFA_MAX_NUM_SP ) > { > @@ -533,7 +541,9 @@ bool ffa_partinfo_init(void) > ret = init_subscribers(spmc_rx, count, fpi_size); > > out: > - ffa_rxtx_spmc_rx_release(); > + e = ffa_rxtx_spmc_rx_release(spmc_ok); > + if ( e ) > + printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", > e); > return ret; > } > > diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h > index 58562d8e733c..461e87f6f9c4 100644 > --- a/xen/arch/arm/tee/ffa_private.h > +++ b/xen/arch/arm/tee/ffa_private.h > @@ -458,7 +458,7 @@ int32_t ffa_endpoint_domain_lookup(uint16_t endpoint_id, > struct domain **d_out, > bool ffa_rxtx_spmc_init(void); > void ffa_rxtx_spmc_destroy(void); > void *ffa_rxtx_spmc_rx_acquire(void); > -void ffa_rxtx_spmc_rx_release(void); > +int32_t ffa_rxtx_spmc_rx_release(bool notify_fw); > void *ffa_rxtx_spmc_tx_acquire(void); > void ffa_rxtx_spmc_tx_release(void); > > diff --git a/xen/arch/arm/tee/ffa_rxtx.c b/xen/arch/arm/tee/ffa_rxtx.c > index 7d8bb4f4d031..50758fb57cdf 100644 > --- a/xen/arch/arm/tee/ffa_rxtx.c > +++ b/xen/arch/arm/tee/ffa_rxtx.c > @@ -375,18 +375,22 @@ void *ffa_rxtx_spmc_rx_acquire(void) > return NULL; > } > > -void ffa_rxtx_spmc_rx_release(void) > +int32_t ffa_rxtx_spmc_rx_release(bool notify_fw) > { > int32_t ret; > > ASSERT(spin_is_locked(&ffa_spmc_rx_lock)); > > - /* Inform the SPMC that we are done with our RX buffer */ > - ret = ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0); > - if ( ret != FFA_RET_OK ) > - printk(XENLOG_DEBUG "Error releasing SPMC RX buffer: %d\n", ret); > + if ( notify_fw ) > + { > + /* Inform the SPMC that we are done with our RX buffer */ > + ret = ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0); > + } > + else > + ret = FFA_RET_OK; > > spin_unlock(&ffa_spmc_rx_lock); > + return ret; > } > > void *ffa_rxtx_spmc_tx_acquire(void) > -- > 2.50.1 (Apple Git-155) >
