[PATCH 05/12] xen/arm: ffa: Fix PARTINFO RX release errors

Bertrand Marquis posted 12 patches 6 days, 7 hours ago
[PATCH 05/12] xen/arm: ffa: Fix PARTINFO RX release errors
Posted by Bertrand Marquis 6 days, 7 hours ago
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 <bertrand.marquis@arm.com>
---
 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;
 
     /* 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)
Re: [PATCH 05/12] xen/arm: ffa: Fix PARTINFO RX release errors
Posted by Jens Wiklander 10 hours ago
Hi Bertrand,

On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
<bertrand.marquis@arm.com> 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 <bertrand.marquis@arm.com>
> ---
>  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 <jens.wiklander@linaro.org>

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)
>
Re: [PATCH 05/12] xen/arm: ffa: Fix PARTINFO RX release errors
Posted by Bertrand Marquis 9 hours ago
Hi Jens,

> On 9 Feb 2026, at 15:24, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis
> <bertrand.marquis@arm.com> 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 <bertrand.marquis@arm.com>
>> ---
>> 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 <jens.wiklander@linaro.org>

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)