Hi Jens, > On 9 Feb 2026, at 15:24, Jens Wiklander <[email protected]> wrote: > > 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()?
Yes that would make more sense. I will rename spmc_ok to notify_fw in v2. > > Either way, please add: > Reviewed-by: Jens Wiklander <[email protected]> Thanks Cheers Bertrand > > 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)
