FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
RX buffer every time. The SP list is expected to be static, so repeated
firmware calls and validation are unnecessary.

Cache the SPMC partition-info list at init time, keeping only secure
endpoints, and reuse the cached entries for SP count and partition-info
responses. Initialize the VM create/destroy subscriber lists from the cached
list and free the cache on init failure.

SP partition info now reflects the init-time snapshot and will not change.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
 1 file changed, 138 insertions(+), 67 deletions(-)

diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
index 6a6f3ffb822e..8a3eac25f99f 100644
--- a/xen/arch/arm/tee/ffa_partinfo.c
+++ b/xen/arch/arm/tee/ffa_partinfo.c
@@ -6,6 +6,7 @@
 #include <xen/const.h>
 #include <xen/sizes.h>
 #include <xen/types.h>
+#include <xen/xmalloc.h>
 
 #include <asm/smccc.h>
 #include <asm/regs.h>
@@ -33,6 +34,10 @@ static uint16_t subscr_vm_created_count __read_mostly;
 static uint16_t *subscr_vm_destroyed __read_mostly;
 static uint16_t subscr_vm_destroyed_count __read_mostly;
 
+/* SP list cache (secure endpoints only); populated at init. */
+static void *sp_list __read_mostly;
+static uint32_t sp_list_count __read_mostly;
+static uint32_t sp_list_entry_size __read_mostly;
 static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
                                       uint32_t *count, uint32_t *fpi_size)
 {
@@ -79,92 +84,78 @@ static int32_t ffa_copy_info(void **dst, void *dst_end, const void *src,
     return FFA_RET_OK;
 }
 
-static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
+static bool ffa_sp_entry_matches_uuid(const void *entry, struct ffa_uuid uuid)
 {
-    uint32_t src_size;
+    const struct ffa_partition_info_1_1 *fpi = entry;
+    struct ffa_uuid sp_uuid;
+
+    if ( ffa_uuid_is_nil(uuid) )
+        return true;
 
-    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
-                                  sp_count, &src_size);
+    if ( sp_list_entry_size < sizeof(*fpi) )
+        return false;
+
+    memcpy(&sp_uuid, fpi->uuid, sizeof(sp_uuid));
+    return ffa_uuid_equal(uuid, sp_uuid);
 }
 
-static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
-                                   void **dst_buf, void *end_buf,
-                                   uint32_t dst_size)
+static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
 {
-    int32_t ret;
-    int32_t release_ret;
-    uint32_t src_size, real_sp_count;
-    void *src_buf;
     uint32_t count = 0;
-    bool notify_fw = false;
-
-    /* We need to use the RX buffer to receive the list */
-    src_buf = ffa_rxtx_spmc_rx_acquire();
-    if ( !src_buf )
-        return FFA_RET_DENIED;
-
-    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
-    if ( ret )
-        goto out;
-    notify_fw = true;
+    uint32_t n;
 
-    /* Validate the src_size we got */
-    if ( src_size < sizeof(struct ffa_partition_info_1_0) ||
-         src_size >= FFA_PAGE_SIZE )
+    for ( n = 0; n < sp_list_count; n++ )
     {
-        ret = FFA_RET_NOT_SUPPORTED;
-        goto out;
+        void *entry = sp_list + n * sp_list_entry_size;
+
+        if ( ffa_sp_entry_matches_uuid(entry, uuid) )
+            count++;
     }
 
-    /*
-     * Limit the maximum time we hold the CPU by limiting the number of SPs.
-     * We just ignore the extra ones as this is tested during init in
-     * ffa_partinfo_init so the only possible reason is SP have been added
-     * since boot.
-     */
-    if ( real_sp_count > FFA_MAX_NUM_SP )
-        real_sp_count = FFA_MAX_NUM_SP;
+    *sp_count = count;
 
-    /* Make sure the data fits in our buffer */
-    if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size )
-    {
-        ret = FFA_RET_NOT_SUPPORTED;
-        goto out;
-    }
+    if ( !ffa_uuid_is_nil(uuid) && !count )
+        return FFA_RET_INVALID_PARAMETERS;
 
-    for ( uint32_t sp_num = 0; sp_num < real_sp_count; sp_num++ )
-    {
-        struct ffa_partition_info_1_1 *fpi = src_buf;
+    return FFA_RET_OK;
+}
 
-        /* filter out SP not following bit 15 convention if any */
-        if ( FFA_ID_IS_SECURE(fpi->id) )
-        {
-            /*
-             * If VM is 1.0 but firmware is 1.1 we could have several entries
-             * with the same ID but different UUIDs. In this case the VM will
-             * get a list with several time the same ID.
-             * This is a non-compliance to the specification but 1.0 VMs should
-             * handle that on their own to simplify Xen implementation.
-             */
+static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
+                                   void **dst_buf, void *end_buf,
+                                   uint32_t dst_size)
+{
+    int32_t ret;
+    uint32_t count = 0;
+    uint32_t n;
 
-            ret = ffa_copy_info(dst_buf, end_buf, src_buf, dst_size, src_size);
-            if ( ret )
-                goto out;
+    for ( n = 0; n < sp_list_count; n++ )
+    {
+        void *entry = sp_list + n * sp_list_entry_size;
 
-            count++;
-        }
+        if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
+            continue;
 
-        src_buf += src_size;
+        /*
+         * If VM is 1.0 but firmware is 1.1 we could have several entries
+         * with the same ID but different UUIDs. In this case the VM will
+         * get a list with several time the same ID.
+         * This is a non-compliance to the specification but 1.0 VMs should
+         * handle that on their own to simplify Xen implementation.
+         */
+        ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
+                            sp_list_entry_size);
+        if ( ret )
+            return ret;
+
+        count++;
     }
 
     *sp_count = count;
 
-out:
-    release_ret = ffa_rxtx_spmc_rx_release(notify_fw);
-    if ( release_ret )
-        gprintk(XENLOG_WARNING,
-                "ffa: Error releasing SPMC RX buffer: %d\n", release_ret);
-    return ret;
+    if ( !ffa_uuid_is_nil(uuid) && !count )
+        return FFA_RET_INVALID_PARAMETERS;
+
+    return FFA_RET_OK;
 }
 
 static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t start_index,
@@ -435,6 +426,14 @@ static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
     return res;
 }
 
+static void ffa_sp_list_cache_free(void)
+{
+    XFREE(sp_list);
+    sp_list = NULL;
+    sp_list_count = 0;
+    sp_list_entry_size = 0;
+}
+
 static void uninit_subscribers(void)
 {
         subscr_vm_created_count = 0;
@@ -443,6 +442,68 @@ static void uninit_subscribers(void)
         XFREE(subscr_vm_destroyed);
 }
 
+static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
+                                   uint32_t fpi_size)
+{
+    const uint8_t *src = buf;
+    uint32_t secure_count = 0;
+    uint32_t n, idx = 0;
+    bool warned = false;
+
+    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
+         fpi_size >= FFA_PAGE_SIZE )
+        return false;
+
+    if ( count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / fpi_size )
+        return false;
+
+    for ( n = 0; n < count; n++ )
+    {
+        const struct ffa_partition_info_1_0 *fpi =
+            (const void *)(src + n * fpi_size);
+
+        if ( !FFA_ID_IS_SECURE(fpi->id) )
+        {
+            if ( !warned )
+            {
+                printk_once(XENLOG_ERR
+                            "ffa: Firmware is not using bit 15 convention for IDs !!\n");
+                warned = true;
+            }
+            printk(XENLOG_ERR
+                   "ffa: Secure partition with id 0x%04x cannot be used\n",
+                   fpi->id);
+            continue;
+        }
+
+        secure_count++;
+    }
+
+    if ( secure_count )
+    {
+        sp_list = xzalloc_bytes(secure_count * fpi_size);
+        if ( !sp_list )
+            return false;
+    }
+
+    sp_list_count = secure_count;
+    sp_list_entry_size = fpi_size;
+
+    for ( n = 0; n < count; n++ )
+    {
+        const struct ffa_partition_info_1_0 *fpi =
+            (const void *)(src + n * fpi_size);
+
+        if ( !FFA_ID_IS_SECURE(fpi->id) )
+            continue;
+
+        memcpy(sp_list + idx * fpi_size, fpi, fpi_size);
+        idx++;
+    }
+
+    return true;
+}
+
 static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
 {
     uint16_t n;
@@ -549,12 +610,22 @@ bool ffa_partinfo_init(void)
         goto out;
     }
 
-    ret = init_subscribers(spmc_rx, count, fpi_size);
+    if ( !ffa_sp_list_cache_init(spmc_rx, count, fpi_size) )
+    {
+        printk(XENLOG_ERR "ffa: Failed to cache SP list\n");
+        goto out;
+    }
+
+    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
 
 out:
     e = ffa_rxtx_spmc_rx_release(notify_fw);
     if ( e )
         printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", e);
+    if ( !ret )
+        uninit_subscribers();
+    if ( !ret )
+        ffa_sp_list_cache_free();
     return ret;
 }
 
-- 
2.52.0

Hi Bertrand,

On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
> RX buffer every time. The SP list is expected to be static, so repeated
> firmware calls and validation are unnecessary.
>
> Cache the SPMC partition-info list at init time, keeping only secure
> endpoints, and reuse the cached entries for SP count and partition-info
> responses. Initialize the VM create/destroy subscriber lists from the cached
> list and free the cache on init failure.
>
> SP partition info now reflects the init-time snapshot and will not change.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
>  xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
>  1 file changed, 138 insertions(+), 67 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index 6a6f3ffb822e..8a3eac25f99f 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -6,6 +6,7 @@
>  #include <xen/const.h>
>  #include <xen/sizes.h>
>  #include <xen/types.h>
> +#include <xen/xmalloc.h>
>
>  #include <asm/smccc.h>
>  #include <asm/regs.h>
> @@ -33,6 +34,10 @@ static uint16_t subscr_vm_created_count __read_mostly;
>  static uint16_t *subscr_vm_destroyed __read_mostly;
>  static uint16_t subscr_vm_destroyed_count __read_mostly;
>
> +/* SP list cache (secure endpoints only); populated at init. */
> +static void *sp_list __read_mostly;
> +static uint32_t sp_list_count __read_mostly;
> +static uint32_t sp_list_entry_size __read_mostly;
>  static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
>                                        uint32_t *count, uint32_t *fpi_size)
>  {
> @@ -79,92 +84,78 @@ static int32_t ffa_copy_info(void **dst, void *dst_end, const void *src,
>      return FFA_RET_OK;
>  }
>
> -static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
> +static bool ffa_sp_entry_matches_uuid(const void *entry, struct ffa_uuid uuid)
>  {
> -    uint32_t src_size;
> +    const struct ffa_partition_info_1_1 *fpi = entry;
> +    struct ffa_uuid sp_uuid;
> +
> +    if ( ffa_uuid_is_nil(uuid) )
> +        return true;
>
> -    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
> -                                  sp_count, &src_size);
> +    if ( sp_list_entry_size < sizeof(*fpi) )
> +        return false;

This can never happen since we don't accept SPMC below version 1.1. We
even have a check to ensure the SPMC doesn't return a too-small
spi_size.

> +
> +    memcpy(&sp_uuid, fpi->uuid, sizeof(sp_uuid));
> +    return ffa_uuid_equal(uuid, sp_uuid);
>  }
>
> -static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
> -                                   void **dst_buf, void *end_buf,
> -                                   uint32_t dst_size)
> +static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
>  {
> -    int32_t ret;
> -    int32_t release_ret;
> -    uint32_t src_size, real_sp_count;
> -    void *src_buf;
>      uint32_t count = 0;
> -    bool notify_fw = false;
> -
> -    /* We need to use the RX buffer to receive the list */
> -    src_buf = ffa_rxtx_spmc_rx_acquire();
> -    if ( !src_buf )
> -        return FFA_RET_DENIED;
> -
> -    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
> -    if ( ret )
> -        goto out;
> -    notify_fw = true;
> +    uint32_t n;
>
> -    /* Validate the src_size we got */
> -    if ( src_size < sizeof(struct ffa_partition_info_1_0) ||
> -         src_size >= FFA_PAGE_SIZE )
> +    for ( n = 0; n < sp_list_count; n++ )
>      {
> -        ret = FFA_RET_NOT_SUPPORTED;
> -        goto out;
> +        void *entry = sp_list + n * sp_list_entry_size;
> +
> +        if ( ffa_sp_entry_matches_uuid(entry, uuid) )
> +            count++;
>      }
>
> -    /*
> -     * Limit the maximum time we hold the CPU by limiting the number of SPs.
> -     * We just ignore the extra ones as this is tested during init in
> -     * ffa_partinfo_init so the only possible reason is SP have been added
> -     * since boot.
> -     */
> -    if ( real_sp_count > FFA_MAX_NUM_SP )
> -        real_sp_count = FFA_MAX_NUM_SP;
> +    *sp_count = count;
>
> -    /* Make sure the data fits in our buffer */
> -    if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size )
> -    {
> -        ret = FFA_RET_NOT_SUPPORTED;
> -        goto out;
> -    }
> +    if ( !ffa_uuid_is_nil(uuid) && !count )
> +        return FFA_RET_INVALID_PARAMETERS;
>
> -    for ( uint32_t sp_num = 0; sp_num < real_sp_count; sp_num++ )
> -    {
> -        struct ffa_partition_info_1_1 *fpi = src_buf;
> +    return FFA_RET_OK;
> +}
>
> -        /* filter out SP not following bit 15 convention if any */
> -        if ( FFA_ID_IS_SECURE(fpi->id) )
> -        {
> -            /*
> -             * If VM is 1.0 but firmware is 1.1 we could have several entries
> -             * with the same ID but different UUIDs. In this case the VM will
> -             * get a list with several time the same ID.
> -             * This is a non-compliance to the specification but 1.0 VMs should
> -             * handle that on their own to simplify Xen implementation.
> -             */
> +static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
> +                                   void **dst_buf, void *end_buf,
> +                                   uint32_t dst_size)
> +{
> +    int32_t ret;
> +    uint32_t count = 0;
> +    uint32_t n;
>
> -            ret = ffa_copy_info(dst_buf, end_buf, src_buf, dst_size, src_size);
> -            if ( ret )
> -                goto out;
> +    for ( n = 0; n < sp_list_count; n++ )
> +    {
> +        void *entry = sp_list + n * sp_list_entry_size;
>
> -            count++;
> -        }
> +        if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
> +            continue;
>
> -        src_buf += src_size;
> +        /*
> +         * If VM is 1.0 but firmware is 1.1 we could have several entries
> +         * with the same ID but different UUIDs. In this case the VM will
> +         * get a list with several time the same ID.
> +         * This is a non-compliance to the specification but 1.0 VMs should
> +         * handle that on their own to simplify Xen implementation.
> +         */
> +        ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
> +                            sp_list_entry_size);
> +        if ( ret )
> +            return ret;
> +
> +        count++;
>      }
>
>      *sp_count = count;
>
> -out:
> -    release_ret = ffa_rxtx_spmc_rx_release(notify_fw);
> -    if ( release_ret )
> -        gprintk(XENLOG_WARNING,
> -                "ffa: Error releasing SPMC RX buffer: %d\n", release_ret);
> -    return ret;
> +    if ( !ffa_uuid_is_nil(uuid) && !count )
> +        return FFA_RET_INVALID_PARAMETERS;
> +
> +    return FFA_RET_OK;
>  }
>
>  static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t start_index,
> @@ -435,6 +426,14 @@ static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>      return res;
>  }
>
> +static void ffa_sp_list_cache_free(void)
> +{
> +    XFREE(sp_list);
> +    sp_list = NULL;

XFREE() is already setting sp_list to NULL.

> +    sp_list_count = 0;
> +    sp_list_entry_size = 0;
> +}
> +
>  static void uninit_subscribers(void)
>  {
>          subscr_vm_created_count = 0;
> @@ -443,6 +442,68 @@ static void uninit_subscribers(void)
>          XFREE(subscr_vm_destroyed);
>  }
>
> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
> +                                   uint32_t fpi_size)
> +{
> +    const uint8_t *src = buf;
> +    uint32_t secure_count = 0;
> +    uint32_t n, idx = 0;
> +    bool warned = false;
> +
> +    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
> +         fpi_size >= FFA_PAGE_SIZE )
> +        return false;

Would it make sense to check that fpi_size is well aligned with struct
ffa_partition_info_1_0? If it's an odd size, we'll make unaligned
accesses below with memcpy(). But perhaps that's a bit much. The SPMC
isn't supposed to provide garbage.

> +
> +    if ( count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / fpi_size )
> +        return false;
> +
> +    for ( n = 0; n < count; n++ )
> +    {
> +        const struct ffa_partition_info_1_0 *fpi =
> +            (const void *)(src + n * fpi_size);
> +
> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
> +        {
> +            if ( !warned )

Is this needed? Doesn't printk_once() already ensure this? Or did you
happen to leave printk_once() by mistake and meant for this to be
printed once each time ffa_sp_list_cache_init() is called, since
"warned" isn't static.

> +            {
> +                printk_once(XENLOG_ERR
> +                            "ffa: Firmware is not using bit 15 convention for IDs !!\n");
> +                warned = true;
> +            }
> +            printk(XENLOG_ERR
> +                   "ffa: Secure partition with id 0x%04x cannot be used\n",
> +                   fpi->id);
> +            continue;
> +        }
> +
> +        secure_count++;
> +    }
> +
> +    if ( secure_count )
> +    {
> +        sp_list = xzalloc_bytes(secure_count * fpi_size);
> +        if ( !sp_list )
> +            return false;
> +    }
> +
> +    sp_list_count = secure_count;
> +    sp_list_entry_size = fpi_size;
> +
> +    for ( n = 0; n < count; n++ )
> +    {
> +        const struct ffa_partition_info_1_0 *fpi =
> +            (const void *)(src + n * fpi_size);
> +
> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
> +            continue;
> +
> +        memcpy(sp_list + idx * fpi_size, fpi, fpi_size);
> +        idx++;
> +    }
> +
> +    return true;
> +}
> +
>  static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
>  {
>      uint16_t n;
> @@ -549,12 +610,22 @@ bool ffa_partinfo_init(void)
>          goto out;
>      }
>
> -    ret = init_subscribers(spmc_rx, count, fpi_size);
> +    if ( !ffa_sp_list_cache_init(spmc_rx, count, fpi_size) )
> +    {
> +        printk(XENLOG_ERR "ffa: Failed to cache SP list\n");
> +        goto out;
> +    }
> +
> +    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
>
>  out:
>      e = ffa_rxtx_spmc_rx_release(notify_fw);
>      if ( e )
>          printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", e);
> +    if ( !ret )
> +        uninit_subscribers();

ret is initially false and can only be set to true if
init_subscribers() returns true. So there's never any point in calling
uninit_subscribers().

> +    if ( !ret )
> +        ffa_sp_list_cache_free();

ret can be false even if ffa_sp_list_cache_init() hasn't been called
yet. Calling ffa_sp_list_cache_free() anyway is harmless, but not
elegant.

Cheers,
Jens

>      return ret;
>  }
>
> --
> 2.52.0
>

Hi Jens,

> On 27 Feb 2026, at 11:39, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
> <bertrand.marquis@arm.com> wrote:
>> 
>> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
>> RX buffer every time. The SP list is expected to be static, so repeated
>> firmware calls and validation are unnecessary.
>> 
>> Cache the SPMC partition-info list at init time, keeping only secure
>> endpoints, and reuse the cached entries for SP count and partition-info
>> responses. Initialize the VM create/destroy subscriber lists from the cached
>> list and free the cache on init failure.
>> 
>> SP partition info now reflects the init-time snapshot and will not change.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
>> 1 file changed, 138 insertions(+), 67 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
>> index 6a6f3ffb822e..8a3eac25f99f 100644
>> --- a/xen/arch/arm/tee/ffa_partinfo.c
>> +++ b/xen/arch/arm/tee/ffa_partinfo.c
>> @@ -6,6 +6,7 @@
>> #include <xen/const.h>
>> #include <xen/sizes.h>
>> #include <xen/types.h>
>> +#include <xen/xmalloc.h>
>> 
>> #include <asm/smccc.h>
>> #include <asm/regs.h>
>> @@ -33,6 +34,10 @@ static uint16_t subscr_vm_created_count __read_mostly;
>> static uint16_t *subscr_vm_destroyed __read_mostly;
>> static uint16_t subscr_vm_destroyed_count __read_mostly;
>> 
>> +/* SP list cache (secure endpoints only); populated at init. */
>> +static void *sp_list __read_mostly;
>> +static uint32_t sp_list_count __read_mostly;
>> +static uint32_t sp_list_entry_size __read_mostly;
>> static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
>>                                       uint32_t *count, uint32_t *fpi_size)
>> {
>> @@ -79,92 +84,78 @@ static int32_t ffa_copy_info(void **dst, void *dst_end, const void *src,
>>     return FFA_RET_OK;
>> }
>> 
>> -static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
>> +static bool ffa_sp_entry_matches_uuid(const void *entry, struct ffa_uuid uuid)
>> {
>> -    uint32_t src_size;
>> +    const struct ffa_partition_info_1_1 *fpi = entry;
>> +    struct ffa_uuid sp_uuid;
>> +
>> +    if ( ffa_uuid_is_nil(uuid) )
>> +        return true;
>> 
>> -    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
>> -                                  sp_count, &src_size);
>> +    if ( sp_list_entry_size < sizeof(*fpi) )
>> +        return false;
> 
> This can never happen since we don't accept SPMC below version 1.1. We
> even have a check to ensure the SPMC doesn't return a too-small
> spi_size.

I tried to make the code forward compatible. It is not really a possible case right now 
but we could fall into this in the future.

> 
>> +
>> +    memcpy(&sp_uuid, fpi->uuid, sizeof(sp_uuid));
>> +    return ffa_uuid_equal(uuid, sp_uuid);
>> }
>> 
>> -static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
>> -                                   void **dst_buf, void *end_buf,
>> -                                   uint32_t dst_size)
>> +static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
>> {
>> -    int32_t ret;
>> -    int32_t release_ret;
>> -    uint32_t src_size, real_sp_count;
>> -    void *src_buf;
>>     uint32_t count = 0;
>> -    bool notify_fw = false;
>> -
>> -    /* We need to use the RX buffer to receive the list */
>> -    src_buf = ffa_rxtx_spmc_rx_acquire();
>> -    if ( !src_buf )
>> -        return FFA_RET_DENIED;
>> -
>> -    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
>> -    if ( ret )
>> -        goto out;
>> -    notify_fw = true;
>> +    uint32_t n;
>> 
>> -    /* Validate the src_size we got */
>> -    if ( src_size < sizeof(struct ffa_partition_info_1_0) ||
>> -         src_size >= FFA_PAGE_SIZE )
>> +    for ( n = 0; n < sp_list_count; n++ )
>>     {
>> -        ret = FFA_RET_NOT_SUPPORTED;
>> -        goto out;
>> +        void *entry = sp_list + n * sp_list_entry_size;
>> +
>> +        if ( ffa_sp_entry_matches_uuid(entry, uuid) )
>> +            count++;
>>     }
>> 
>> -    /*
>> -     * Limit the maximum time we hold the CPU by limiting the number of SPs.
>> -     * We just ignore the extra ones as this is tested during init in
>> -     * ffa_partinfo_init so the only possible reason is SP have been added
>> -     * since boot.
>> -     */
>> -    if ( real_sp_count > FFA_MAX_NUM_SP )
>> -        real_sp_count = FFA_MAX_NUM_SP;
>> +    *sp_count = count;
>> 
>> -    /* Make sure the data fits in our buffer */
>> -    if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size )
>> -    {
>> -        ret = FFA_RET_NOT_SUPPORTED;
>> -        goto out;
>> -    }
>> +    if ( !ffa_uuid_is_nil(uuid) && !count )
>> +        return FFA_RET_INVALID_PARAMETERS;
>> 
>> -    for ( uint32_t sp_num = 0; sp_num < real_sp_count; sp_num++ )
>> -    {
>> -        struct ffa_partition_info_1_1 *fpi = src_buf;
>> +    return FFA_RET_OK;
>> +}
>> 
>> -        /* filter out SP not following bit 15 convention if any */
>> -        if ( FFA_ID_IS_SECURE(fpi->id) )
>> -        {
>> -            /*
>> -             * If VM is 1.0 but firmware is 1.1 we could have several entries
>> -             * with the same ID but different UUIDs. In this case the VM will
>> -             * get a list with several time the same ID.
>> -             * This is a non-compliance to the specification but 1.0 VMs should
>> -             * handle that on their own to simplify Xen implementation.
>> -             */
>> +static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
>> +                                   void **dst_buf, void *end_buf,
>> +                                   uint32_t dst_size)
>> +{
>> +    int32_t ret;
>> +    uint32_t count = 0;
>> +    uint32_t n;
>> 
>> -            ret = ffa_copy_info(dst_buf, end_buf, src_buf, dst_size, src_size);
>> -            if ( ret )
>> -                goto out;
>> +    for ( n = 0; n < sp_list_count; n++ )
>> +    {
>> +        void *entry = sp_list + n * sp_list_entry_size;
>> 
>> -            count++;
>> -        }
>> +        if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
>> +            continue;
>> 
>> -        src_buf += src_size;
>> +        /*
>> +         * If VM is 1.0 but firmware is 1.1 we could have several entries
>> +         * with the same ID but different UUIDs. In this case the VM will
>> +         * get a list with several time the same ID.
>> +         * This is a non-compliance to the specification but 1.0 VMs should
>> +         * handle that on their own to simplify Xen implementation.
>> +         */
>> +        ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
>> +                            sp_list_entry_size);
>> +        if ( ret )
>> +            return ret;
>> +
>> +        count++;
>>     }
>> 
>>     *sp_count = count;
>> 
>> -out:
>> -    release_ret = ffa_rxtx_spmc_rx_release(notify_fw);
>> -    if ( release_ret )
>> -        gprintk(XENLOG_WARNING,
>> -                "ffa: Error releasing SPMC RX buffer: %d\n", release_ret);
>> -    return ret;
>> +    if ( !ffa_uuid_is_nil(uuid) && !count )
>> +        return FFA_RET_INVALID_PARAMETERS;
>> +
>> +    return FFA_RET_OK;
>> }
>> 
>> static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t start_index,
>> @@ -435,6 +426,14 @@ static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>>     return res;
>> }
>> 
>> +static void ffa_sp_list_cache_free(void)
>> +{
>> +    XFREE(sp_list);
>> +    sp_list = NULL;
> 
> XFREE() is already setting sp_list to NULL.

Ack will fix in v2

> 
>> +    sp_list_count = 0;
>> +    sp_list_entry_size = 0;
>> +}
>> +
>> static void uninit_subscribers(void)
>> {
>>         subscr_vm_created_count = 0;
>> @@ -443,6 +442,68 @@ static void uninit_subscribers(void)
>>         XFREE(subscr_vm_destroyed);
>> }
>> 
>> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
>> +                                   uint32_t fpi_size)
>> +{
>> +    const uint8_t *src = buf;
>> +    uint32_t secure_count = 0;
>> +    uint32_t n, idx = 0;
>> +    bool warned = false;
>> +
>> +    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
>> +         fpi_size >= FFA_PAGE_SIZE )
>> +        return false;
> 
> Would it make sense to check that fpi_size is well aligned with struct
> ffa_partition_info_1_0? If it's an odd size, we'll make unaligned
> accesses below with memcpy(). But perhaps that's a bit much. The SPMC
> isn't supposed to provide garbage.

Memcpy should prevent issues even if accesses are not aligned.
If we had this test, we cannot return an error to the SPMC so we would have to
generate one to the caller. It is simpler i think to handle non-aligned as we do not
expect the SPMC to generate such a case.
Tell me if you agree.

> 
>> +
>> +    if ( count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / fpi_size )
>> +        return false;
>> +
>> +    for ( n = 0; n < count; n++ )
>> +    {
>> +        const struct ffa_partition_info_1_0 *fpi =
>> +            (const void *)(src + n * fpi_size);
>> +
>> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
>> +        {
>> +            if ( !warned )
> 
> Is this needed? Doesn't printk_once() already ensure this? Or did you
> happen to leave printk_once() by mistake and meant for this to be
> printed once each time ffa_sp_list_cache_init() is called, since
> "warned" isn't static.

Very right, i need to remove the warned, printk_once should be enough here.

> 
>> +            {
>> +                printk_once(XENLOG_ERR
>> +                            "ffa: Firmware is not using bit 15 convention for IDs !!\n");
>> +                warned = true;
>> +            }
>> +            printk(XENLOG_ERR
>> +                   "ffa: Secure partition with id 0x%04x cannot be used\n",
>> +                   fpi->id);
>> +            continue;
>> +        }
>> +
>> +        secure_count++;
>> +    }
>> +
>> +    if ( secure_count )
>> +    {
>> +        sp_list = xzalloc_bytes(secure_count * fpi_size);
>> +        if ( !sp_list )
>> +            return false;
>> +    }
>> +
>> +    sp_list_count = secure_count;
>> +    sp_list_entry_size = fpi_size;
>> +
>> +    for ( n = 0; n < count; n++ )
>> +    {
>> +        const struct ffa_partition_info_1_0 *fpi =
>> +            (const void *)(src + n * fpi_size);
>> +
>> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
>> +            continue;
>> +
>> +        memcpy(sp_list + idx * fpi_size, fpi, fpi_size);
>> +        idx++;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>> static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
>> {
>>     uint16_t n;
>> @@ -549,12 +610,22 @@ bool ffa_partinfo_init(void)
>>         goto out;
>>     }
>> 
>> -    ret = init_subscribers(spmc_rx, count, fpi_size);
>> +    if ( !ffa_sp_list_cache_init(spmc_rx, count, fpi_size) )
>> +    {
>> +        printk(XENLOG_ERR "ffa: Failed to cache SP list\n");
>> +        goto out;
>> +    }
>> +
>> +    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
>> 
>> out:
>>     e = ffa_rxtx_spmc_rx_release(notify_fw);
>>     if ( e )
>>         printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", e);
>> +    if ( !ret )
>> +        uninit_subscribers();
> 
> ret is initially false and can only be set to true if
> init_subscribers() returns true. So there's never any point in calling
> uninit_subscribers().

True, I will fix that.

> 
>> +    if ( !ret )
>> +        ffa_sp_list_cache_free();
> 
> ret can be false even if ffa_sp_list_cache_init() hasn't been called
> yet. Calling ffa_sp_list_cache_free() anyway is harmless, but not
> elegant.

yes, i will rework a bit the cleanup logic here.

Thanks for the review

Cheers
Bertrand

> 
> Cheers,
> Jens
> 
>>     return ret;
>> }
>> 
>> --
>> 2.52.0

Hi Bertrand,

On Mon, Mar 2, 2026 at 9:51 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 27 Feb 2026, at 11:39, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi Bertrand,
> >
> > On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
> > <bertrand.marquis@arm.com> wrote:
> >>
> >> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
> >> RX buffer every time. The SP list is expected to be static, so repeated
> >> firmware calls and validation are unnecessary.
> >>
> >> Cache the SPMC partition-info list at init time, keeping only secure
> >> endpoints, and reuse the cached entries for SP count and partition-info
> >> responses. Initialize the VM create/destroy subscriber lists from the cached
> >> list and free the cache on init failure.
> >>
> >> SP partition info now reflects the init-time snapshot and will not change.
> >>
> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >> ---
> >> xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
> >> 1 file changed, 138 insertions(+), 67 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> >> index 6a6f3ffb822e..8a3eac25f99f 100644
> >> --- a/xen/arch/arm/tee/ffa_partinfo.c
> >> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> >> @@ -6,6 +6,7 @@
> >> #include <xen/const.h>
> >> #include <xen/sizes.h>
> >> #include <xen/types.h>
> >> +#include <xen/xmalloc.h>
> >>
> >> #include <asm/smccc.h>
> >> #include <asm/regs.h>
> >> @@ -33,6 +34,10 @@ static uint16_t subscr_vm_created_count __read_mostly;
> >> static uint16_t *subscr_vm_destroyed __read_mostly;
> >> static uint16_t subscr_vm_destroyed_count __read_mostly;
> >>
> >> +/* SP list cache (secure endpoints only); populated at init. */
> >> +static void *sp_list __read_mostly;
> >> +static uint32_t sp_list_count __read_mostly;
> >> +static uint32_t sp_list_entry_size __read_mostly;
> >> static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
> >>                                       uint32_t *count, uint32_t *fpi_size)
> >> {
> >> @@ -79,92 +84,78 @@ static int32_t ffa_copy_info(void **dst, void *dst_end, const void *src,
> >>     return FFA_RET_OK;
> >> }
> >>
> >> -static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
> >> +static bool ffa_sp_entry_matches_uuid(const void *entry, struct ffa_uuid uuid)
> >> {
> >> -    uint32_t src_size;
> >> +    const struct ffa_partition_info_1_1 *fpi = entry;
> >> +    struct ffa_uuid sp_uuid;
> >> +
> >> +    if ( ffa_uuid_is_nil(uuid) )
> >> +        return true;
> >>
> >> -    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
> >> -                                  sp_count, &src_size);
> >> +    if ( sp_list_entry_size < sizeof(*fpi) )
> >> +        return false;
> >
> > This can never happen since we don't accept SPMC below version 1.1. We
> > even have a check to ensure the SPMC doesn't return a too-small
> > spi_size.
>
> I tried to make the code forward compatible. It is not really a possible case right now
> but we could fall into this in the future.

At the moment, it's only ffa_sp_list_cache_init(), which initializes
and puts things in the SP. So as long as ffa_sp_list_cache_init() and
this function are in sync, there should be no problem. It simplifies
things if we can trust the SP cache to be usable.

>
> >
> >> +
> >> +    memcpy(&sp_uuid, fpi->uuid, sizeof(sp_uuid));
> >> +    return ffa_uuid_equal(uuid, sp_uuid);
> >> }
> >>
> >> -static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
> >> -                                   void **dst_buf, void *end_buf,
> >> -                                   uint32_t dst_size)
> >> +static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
> >> {
> >> -    int32_t ret;
> >> -    int32_t release_ret;
> >> -    uint32_t src_size, real_sp_count;
> >> -    void *src_buf;
> >>     uint32_t count = 0;
> >> -    bool notify_fw = false;
> >> -
> >> -    /* We need to use the RX buffer to receive the list */
> >> -    src_buf = ffa_rxtx_spmc_rx_acquire();
> >> -    if ( !src_buf )
> >> -        return FFA_RET_DENIED;
> >> -
> >> -    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
> >> -    if ( ret )
> >> -        goto out;
> >> -    notify_fw = true;
> >> +    uint32_t n;
> >>
> >> -    /* Validate the src_size we got */
> >> -    if ( src_size < sizeof(struct ffa_partition_info_1_0) ||
> >> -         src_size >= FFA_PAGE_SIZE )
> >> +    for ( n = 0; n < sp_list_count; n++ )
> >>     {
> >> -        ret = FFA_RET_NOT_SUPPORTED;
> >> -        goto out;
> >> +        void *entry = sp_list + n * sp_list_entry_size;
> >> +
> >> +        if ( ffa_sp_entry_matches_uuid(entry, uuid) )
> >> +            count++;
> >>     }
> >>
> >> -    /*
> >> -     * Limit the maximum time we hold the CPU by limiting the number of SPs.
> >> -     * We just ignore the extra ones as this is tested during init in
> >> -     * ffa_partinfo_init so the only possible reason is SP have been added
> >> -     * since boot.
> >> -     */
> >> -    if ( real_sp_count > FFA_MAX_NUM_SP )
> >> -        real_sp_count = FFA_MAX_NUM_SP;
> >> +    *sp_count = count;
> >>
> >> -    /* Make sure the data fits in our buffer */
> >> -    if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size )
> >> -    {
> >> -        ret = FFA_RET_NOT_SUPPORTED;
> >> -        goto out;
> >> -    }
> >> +    if ( !ffa_uuid_is_nil(uuid) && !count )
> >> +        return FFA_RET_INVALID_PARAMETERS;
> >>
> >> -    for ( uint32_t sp_num = 0; sp_num < real_sp_count; sp_num++ )
> >> -    {
> >> -        struct ffa_partition_info_1_1 *fpi = src_buf;
> >> +    return FFA_RET_OK;
> >> +}
> >>
> >> -        /* filter out SP not following bit 15 convention if any */
> >> -        if ( FFA_ID_IS_SECURE(fpi->id) )
> >> -        {
> >> -            /*
> >> -             * If VM is 1.0 but firmware is 1.1 we could have several entries
> >> -             * with the same ID but different UUIDs. In this case the VM will
> >> -             * get a list with several time the same ID.
> >> -             * This is a non-compliance to the specification but 1.0 VMs should
> >> -             * handle that on their own to simplify Xen implementation.
> >> -             */
> >> +static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
> >> +                                   void **dst_buf, void *end_buf,
> >> +                                   uint32_t dst_size)
> >> +{
> >> +    int32_t ret;
> >> +    uint32_t count = 0;
> >> +    uint32_t n;
> >>
> >> -            ret = ffa_copy_info(dst_buf, end_buf, src_buf, dst_size, src_size);
> >> -            if ( ret )
> >> -                goto out;
> >> +    for ( n = 0; n < sp_list_count; n++ )
> >> +    {
> >> +        void *entry = sp_list + n * sp_list_entry_size;
> >>
> >> -            count++;
> >> -        }
> >> +        if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
> >> +            continue;
> >>
> >> -        src_buf += src_size;
> >> +        /*
> >> +         * If VM is 1.0 but firmware is 1.1 we could have several entries
> >> +         * with the same ID but different UUIDs. In this case the VM will
> >> +         * get a list with several time the same ID.
> >> +         * This is a non-compliance to the specification but 1.0 VMs should
> >> +         * handle that on their own to simplify Xen implementation.
> >> +         */
> >> +        ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
> >> +                            sp_list_entry_size);
> >> +        if ( ret )
> >> +            return ret;
> >> +
> >> +        count++;
> >>     }
> >>
> >>     *sp_count = count;
> >>
> >> -out:
> >> -    release_ret = ffa_rxtx_spmc_rx_release(notify_fw);
> >> -    if ( release_ret )
> >> -        gprintk(XENLOG_WARNING,
> >> -                "ffa: Error releasing SPMC RX buffer: %d\n", release_ret);
> >> -    return ret;
> >> +    if ( !ffa_uuid_is_nil(uuid) && !count )
> >> +        return FFA_RET_INVALID_PARAMETERS;
> >> +
> >> +    return FFA_RET_OK;
> >> }
> >>
> >> static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t start_index,
> >> @@ -435,6 +426,14 @@ static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> >>     return res;
> >> }
> >>
> >> +static void ffa_sp_list_cache_free(void)
> >> +{
> >> +    XFREE(sp_list);
> >> +    sp_list = NULL;
> >
> > XFREE() is already setting sp_list to NULL.
>
> Ack will fix in v2
>
> >
> >> +    sp_list_count = 0;
> >> +    sp_list_entry_size = 0;
> >> +}
> >> +
> >> static void uninit_subscribers(void)
> >> {
> >>         subscr_vm_created_count = 0;
> >> @@ -443,6 +442,68 @@ static void uninit_subscribers(void)
> >>         XFREE(subscr_vm_destroyed);
> >> }
> >>
> >> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
> >> +                                   uint32_t fpi_size)
> >> +{
> >> +    const uint8_t *src = buf;
> >> +    uint32_t secure_count = 0;
> >> +    uint32_t n, idx = 0;
> >> +    bool warned = false;
> >> +
> >> +    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
> >> +         fpi_size >= FFA_PAGE_SIZE )
> >> +        return false;
> >
> > Would it make sense to check that fpi_size is well aligned with struct
> > ffa_partition_info_1_0? If it's an odd size, we'll make unaligned
> > accesses below with memcpy(). But perhaps that's a bit much. The SPMC
> > isn't supposed to provide garbage.
>
> Memcpy should prevent issues even if accesses are not aligned.
> If we had this test, we cannot return an error to the SPMC so we would have to
> generate one to the caller. It is simpler i think to handle non-aligned as we do not
> expect the SPMC to generate such a case.
> Tell me if you agree.

We dereference fpi below, and depending on compiler flags and pointer
types, memcpy() might not be safe with unaligned pointers.
From 6.3.2.3 Pointers, paragraph 7, in the C standard:
"A pointer to an object type may be converted to a pointer to a
different object type. If the
resulting pointer is not correctly aligned for the referenced type,
the behavior is
undefined."

I've seen past examples where the compiler optimized memcpy() in a way
that breaks with unaligned pointers.

We don't expect the test above to fail, but if it does we will not use
the secure firmware. I think refusing unexpected sizes is even
simpler. It should make finding eventual errors much easier.

So my question above is whether it's worth checking that fpi_size is
well-aligned, or if it's so unlikely that we don't need to consider
it.

Cheers,
Jens

>
> >
> >> +
> >> +    if ( count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / fpi_size )
> >> +        return false;
> >> +
> >> +    for ( n = 0; n < count; n++ )
> >> +    {
> >> +        const struct ffa_partition_info_1_0 *fpi =
> >> +            (const void *)(src + n * fpi_size);
> >> +
> >> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
> >> +        {
> >> +            if ( !warned )
> >
> > Is this needed? Doesn't printk_once() already ensure this? Or did you
> > happen to leave printk_once() by mistake and meant for this to be
> > printed once each time ffa_sp_list_cache_init() is called, since
> > "warned" isn't static.
>
> Very right, i need to remove the warned, printk_once should be enough here.
>
> >
> >> +            {
> >> +                printk_once(XENLOG_ERR
> >> +                            "ffa: Firmware is not using bit 15 convention for IDs !!\n");
> >> +                warned = true;
> >> +            }
> >> +            printk(XENLOG_ERR
> >> +                   "ffa: Secure partition with id 0x%04x cannot be used\n",
> >> +                   fpi->id);
> >> +            continue;
> >> +        }
> >> +
> >> +        secure_count++;
> >> +    }
> >> +
> >> +    if ( secure_count )
> >> +    {
> >> +        sp_list = xzalloc_bytes(secure_count * fpi_size);
> >> +        if ( !sp_list )
> >> +            return false;
> >> +    }
> >> +
> >> +    sp_list_count = secure_count;
> >> +    sp_list_entry_size = fpi_size;
> >> +
> >> +    for ( n = 0; n < count; n++ )
> >> +    {
> >> +        const struct ffa_partition_info_1_0 *fpi =
> >> +            (const void *)(src + n * fpi_size);
> >> +
> >> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
> >> +            continue;
> >> +
> >> +        memcpy(sp_list + idx * fpi_size, fpi, fpi_size);
> >> +        idx++;
> >> +    }
> >> +
> >> +    return true;
> >> +}
> >> +
> >> static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
> >> {
> >>     uint16_t n;
> >> @@ -549,12 +610,22 @@ bool ffa_partinfo_init(void)
> >>         goto out;
> >>     }
> >>
> >> -    ret = init_subscribers(spmc_rx, count, fpi_size);
> >> +    if ( !ffa_sp_list_cache_init(spmc_rx, count, fpi_size) )
> >> +    {
> >> +        printk(XENLOG_ERR "ffa: Failed to cache SP list\n");
> >> +        goto out;
> >> +    }
> >> +
> >> +    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
> >>
> >> out:
> >>     e = ffa_rxtx_spmc_rx_release(notify_fw);
> >>     if ( e )
> >>         printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", e);
> >> +    if ( !ret )
> >> +        uninit_subscribers();
> >
> > ret is initially false and can only be set to true if
> > init_subscribers() returns true. So there's never any point in calling
> > uninit_subscribers().
>
> True, I will fix that.
>
> >
> >> +    if ( !ret )
> >> +        ffa_sp_list_cache_free();
> >
> > ret can be false even if ffa_sp_list_cache_init() hasn't been called
> > yet. Calling ffa_sp_list_cache_free() anyway is harmless, but not
> > elegant.
>
> yes, i will rework a bit the cleanup logic here.
>
> Thanks for the review
>
> Cheers
> Bertrand
>
> >
> > Cheers,
> > Jens
> >
> >>     return ret;
> >> }
> >>
> >> --
> >> 2.52.0
>
>

Hi Jens,

> On 2 Mar 2026, at 10:32, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Mon, Mar 2, 2026 at 9:51 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>> 
>> Hi Jens,
>> 
>>> On 27 Feb 2026, at 11:39, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>> 
>>> Hi Bertrand,
>>> 
>>> On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
>>> <bertrand.marquis@arm.com> wrote:
>>>> 
>>>> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
>>>> RX buffer every time. The SP list is expected to be static, so repeated
>>>> firmware calls and validation are unnecessary.
>>>> 
>>>> Cache the SPMC partition-info list at init time, keeping only secure
>>>> endpoints, and reuse the cached entries for SP count and partition-info
>>>> responses. Initialize the VM create/destroy subscriber lists from the cached
>>>> list and free the cache on init failure.
>>>> 
>>>> SP partition info now reflects the init-time snapshot and will not change.
>>>> 
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> ---
>>>> xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
>>>> 1 file changed, 138 insertions(+), 67 deletions(-)
>>>> 
>>>> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
>>>> index 6a6f3ffb822e..8a3eac25f99f 100644
>>>> --- a/xen/arch/arm/tee/ffa_partinfo.c
>>>> +++ b/xen/arch/arm/tee/ffa_partinfo.c
>>>> @@ -6,6 +6,7 @@
>>>> #include <xen/const.h>
>>>> #include <xen/sizes.h>
>>>> #include <xen/types.h>
>>>> +#include <xen/xmalloc.h>
>>>> 
>>>> #include <asm/smccc.h>
>>>> #include <asm/regs.h>
>>>> @@ -33,6 +34,10 @@ static uint16_t subscr_vm_created_count __read_mostly;
>>>> static uint16_t *subscr_vm_destroyed __read_mostly;
>>>> static uint16_t subscr_vm_destroyed_count __read_mostly;
>>>> 
>>>> +/* SP list cache (secure endpoints only); populated at init. */
>>>> +static void *sp_list __read_mostly;
>>>> +static uint32_t sp_list_count __read_mostly;
>>>> +static uint32_t sp_list_entry_size __read_mostly;
>>>> static int32_t ffa_partition_info_get(struct ffa_uuid uuid, uint32_t flags,
>>>>                                      uint32_t *count, uint32_t *fpi_size)
>>>> {
>>>> @@ -79,92 +84,78 @@ static int32_t ffa_copy_info(void **dst, void *dst_end, const void *src,
>>>>    return FFA_RET_OK;
>>>> }
>>>> 
>>>> -static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
>>>> +static bool ffa_sp_entry_matches_uuid(const void *entry, struct ffa_uuid uuid)
>>>> {
>>>> -    uint32_t src_size;
>>>> +    const struct ffa_partition_info_1_1 *fpi = entry;
>>>> +    struct ffa_uuid sp_uuid;
>>>> +
>>>> +    if ( ffa_uuid_is_nil(uuid) )
>>>> +        return true;
>>>> 
>>>> -    return ffa_partition_info_get(uuid, FFA_PARTITION_INFO_GET_COUNT_FLAG,
>>>> -                                  sp_count, &src_size);
>>>> +    if ( sp_list_entry_size < sizeof(*fpi) )
>>>> +        return false;
>>> 
>>> This can never happen since we don't accept SPMC below version 1.1. We
>>> even have a check to ensure the SPMC doesn't return a too-small
>>> spi_size.
>> 
>> I tried to make the code forward compatible. It is not really a possible case right now
>> but we could fall into this in the future.
> 
> At the moment, it's only ffa_sp_list_cache_init(), which initializes
> and puts things in the SP. So as long as ffa_sp_list_cache_init() and
> this function are in sync, there should be no problem. It simplifies
> things if we can trust the SP cache to be usable.

Agree, we should not come into a situation where the SP cache content cannot be used.
Anything to be check must have been checked or converted when creating the cache not
when using it.

> 
>> 
>>> 
>>>> +
>>>> +    memcpy(&sp_uuid, fpi->uuid, sizeof(sp_uuid));
>>>> +    return ffa_uuid_equal(uuid, sp_uuid);
>>>> }
>>>> 
>>>> -static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
>>>> -                                   void **dst_buf, void *end_buf,
>>>> -                                   uint32_t dst_size)
>>>> +static int32_t ffa_get_sp_count(struct ffa_uuid uuid, uint32_t *sp_count)
>>>> {
>>>> -    int32_t ret;
>>>> -    int32_t release_ret;
>>>> -    uint32_t src_size, real_sp_count;
>>>> -    void *src_buf;
>>>>    uint32_t count = 0;
>>>> -    bool notify_fw = false;
>>>> -
>>>> -    /* We need to use the RX buffer to receive the list */
>>>> -    src_buf = ffa_rxtx_spmc_rx_acquire();
>>>> -    if ( !src_buf )
>>>> -        return FFA_RET_DENIED;
>>>> -
>>>> -    ret = ffa_partition_info_get(uuid, 0, &real_sp_count, &src_size);
>>>> -    if ( ret )
>>>> -        goto out;
>>>> -    notify_fw = true;
>>>> +    uint32_t n;
>>>> 
>>>> -    /* Validate the src_size we got */
>>>> -    if ( src_size < sizeof(struct ffa_partition_info_1_0) ||
>>>> -         src_size >= FFA_PAGE_SIZE )
>>>> +    for ( n = 0; n < sp_list_count; n++ )
>>>>    {
>>>> -        ret = FFA_RET_NOT_SUPPORTED;
>>>> -        goto out;
>>>> +        void *entry = sp_list + n * sp_list_entry_size;
>>>> +
>>>> +        if ( ffa_sp_entry_matches_uuid(entry, uuid) )
>>>> +            count++;
>>>>    }
>>>> 
>>>> -    /*
>>>> -     * Limit the maximum time we hold the CPU by limiting the number of SPs.
>>>> -     * We just ignore the extra ones as this is tested during init in
>>>> -     * ffa_partinfo_init so the only possible reason is SP have been added
>>>> -     * since boot.
>>>> -     */
>>>> -    if ( real_sp_count > FFA_MAX_NUM_SP )
>>>> -        real_sp_count = FFA_MAX_NUM_SP;
>>>> +    *sp_count = count;
>>>> 
>>>> -    /* Make sure the data fits in our buffer */
>>>> -    if ( real_sp_count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / src_size )
>>>> -    {
>>>> -        ret = FFA_RET_NOT_SUPPORTED;
>>>> -        goto out;
>>>> -    }
>>>> +    if ( !ffa_uuid_is_nil(uuid) && !count )
>>>> +        return FFA_RET_INVALID_PARAMETERS;
>>>> 
>>>> -    for ( uint32_t sp_num = 0; sp_num < real_sp_count; sp_num++ )
>>>> -    {
>>>> -        struct ffa_partition_info_1_1 *fpi = src_buf;
>>>> +    return FFA_RET_OK;
>>>> +}
>>>> 
>>>> -        /* filter out SP not following bit 15 convention if any */
>>>> -        if ( FFA_ID_IS_SECURE(fpi->id) )
>>>> -        {
>>>> -            /*
>>>> -             * If VM is 1.0 but firmware is 1.1 we could have several entries
>>>> -             * with the same ID but different UUIDs. In this case the VM will
>>>> -             * get a list with several time the same ID.
>>>> -             * This is a non-compliance to the specification but 1.0 VMs should
>>>> -             * handle that on their own to simplify Xen implementation.
>>>> -             */
>>>> +static int32_t ffa_get_sp_partinfo(struct ffa_uuid uuid, uint32_t *sp_count,
>>>> +                                   void **dst_buf, void *end_buf,
>>>> +                                   uint32_t dst_size)
>>>> +{
>>>> +    int32_t ret;
>>>> +    uint32_t count = 0;
>>>> +    uint32_t n;
>>>> 
>>>> -            ret = ffa_copy_info(dst_buf, end_buf, src_buf, dst_size, src_size);
>>>> -            if ( ret )
>>>> -                goto out;
>>>> +    for ( n = 0; n < sp_list_count; n++ )
>>>> +    {
>>>> +        void *entry = sp_list + n * sp_list_entry_size;
>>>> 
>>>> -            count++;
>>>> -        }
>>>> +        if ( !ffa_sp_entry_matches_uuid(entry, uuid) )
>>>> +            continue;
>>>> 
>>>> -        src_buf += src_size;
>>>> +        /*
>>>> +         * If VM is 1.0 but firmware is 1.1 we could have several entries
>>>> +         * with the same ID but different UUIDs. In this case the VM will
>>>> +         * get a list with several time the same ID.
>>>> +         * This is a non-compliance to the specification but 1.0 VMs should
>>>> +         * handle that on their own to simplify Xen implementation.
>>>> +         */
>>>> +        ret = ffa_copy_info(dst_buf, end_buf, entry, dst_size,
>>>> +                            sp_list_entry_size);
>>>> +        if ( ret )
>>>> +            return ret;
>>>> +
>>>> +        count++;
>>>>    }
>>>> 
>>>>    *sp_count = count;
>>>> 
>>>> -out:
>>>> -    release_ret = ffa_rxtx_spmc_rx_release(notify_fw);
>>>> -    if ( release_ret )
>>>> -        gprintk(XENLOG_WARNING,
>>>> -                "ffa: Error releasing SPMC RX buffer: %d\n", release_ret);
>>>> -    return ret;
>>>> +    if ( !ffa_uuid_is_nil(uuid) && !count )
>>>> +        return FFA_RET_INVALID_PARAMETERS;
>>>> +
>>>> +    return FFA_RET_OK;
>>>> }
>>>> 
>>>> static int32_t ffa_get_vm_partinfo(struct ffa_uuid uuid, uint32_t start_index,
>>>> @@ -435,6 +426,14 @@ static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>>>>    return res;
>>>> }
>>>> 
>>>> +static void ffa_sp_list_cache_free(void)
>>>> +{
>>>> +    XFREE(sp_list);
>>>> +    sp_list = NULL;
>>> 
>>> XFREE() is already setting sp_list to NULL.
>> 
>> Ack will fix in v2
>> 
>>> 
>>>> +    sp_list_count = 0;
>>>> +    sp_list_entry_size = 0;
>>>> +}
>>>> +
>>>> static void uninit_subscribers(void)
>>>> {
>>>>        subscr_vm_created_count = 0;
>>>> @@ -443,6 +442,68 @@ static void uninit_subscribers(void)
>>>>        XFREE(subscr_vm_destroyed);
>>>> }
>>>> 
>>>> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
>>>> +                                   uint32_t fpi_size)
>>>> +{
>>>> +    const uint8_t *src = buf;
>>>> +    uint32_t secure_count = 0;
>>>> +    uint32_t n, idx = 0;
>>>> +    bool warned = false;
>>>> +
>>>> +    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
>>>> +         fpi_size >= FFA_PAGE_SIZE )
>>>> +        return false;
>>> 
>>> Would it make sense to check that fpi_size is well aligned with struct
>>> ffa_partition_info_1_0? If it's an odd size, we'll make unaligned
>>> accesses below with memcpy(). But perhaps that's a bit much. The SPMC
>>> isn't supposed to provide garbage.
>> 
>> Memcpy should prevent issues even if accesses are not aligned.
>> If we had this test, we cannot return an error to the SPMC so we would have to
>> generate one to the caller. It is simpler i think to handle non-aligned as we do not
>> expect the SPMC to generate such a case.
>> Tell me if you agree.
> 
> We dereference fpi below, and depending on compiler flags and pointer
> types, memcpy() might not be safe with unaligned pointers.
> From 6.3.2.3 Pointers, paragraph 7, in the C standard:
> "A pointer to an object type may be converted to a pointer to a
> different object type. If the
> resulting pointer is not correctly aligned for the referenced type,
> the behavior is
> undefined."
> 
> I've seen past examples where the compiler optimized memcpy() in a way
> that breaks with unaligned pointers.
> 
> We don't expect the test above to fail, but if it does we will not use
> the secure firmware. I think refusing unexpected sizes is even
> simpler. It should make finding eventual errors much easier.

In the ffa spec, the size can grow and this is why there is a size field.
FF-A expect that we either ignore or copy without looking the extra
content.
I think we should not be dependent on any alignment so we need to
make sure we do the copy in a way that is robust to non alignments.

> 
> So my question above is whether it's worth checking that fpi_size is
> well-aligned, or if it's so unlikely that we don't need to consider
> it.

I think we need to find a solution where we handle properly things even
if the size is not aligned. I though using memcpy would protect against
that but maybe we need to use a stronger solution to ensure that works
even if data is unaligned.

Cheers
Bertrand

> 
> Cheers,
> Jens
> 
>> 
>>> 
>>>> +
>>>> +    if ( count > (FFA_RXTX_PAGE_COUNT * FFA_PAGE_SIZE) / fpi_size )
>>>> +        return false;
>>>> +
>>>> +    for ( n = 0; n < count; n++ )
>>>> +    {
>>>> +        const struct ffa_partition_info_1_0 *fpi =
>>>> +            (const void *)(src + n * fpi_size);
>>>> +
>>>> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
>>>> +        {
>>>> +            if ( !warned )
>>> 
>>> Is this needed? Doesn't printk_once() already ensure this? Or did you
>>> happen to leave printk_once() by mistake and meant for this to be
>>> printed once each time ffa_sp_list_cache_init() is called, since
>>> "warned" isn't static.
>> 
>> Very right, i need to remove the warned, printk_once should be enough here.
>> 
>>> 
>>>> +            {
>>>> +                printk_once(XENLOG_ERR
>>>> +                            "ffa: Firmware is not using bit 15 convention for IDs !!\n");
>>>> +                warned = true;
>>>> +            }
>>>> +            printk(XENLOG_ERR
>>>> +                   "ffa: Secure partition with id 0x%04x cannot be used\n",
>>>> +                   fpi->id);
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        secure_count++;
>>>> +    }
>>>> +
>>>> +    if ( secure_count )
>>>> +    {
>>>> +        sp_list = xzalloc_bytes(secure_count * fpi_size);
>>>> +        if ( !sp_list )
>>>> +            return false;
>>>> +    }
>>>> +
>>>> +    sp_list_count = secure_count;
>>>> +    sp_list_entry_size = fpi_size;
>>>> +
>>>> +    for ( n = 0; n < count; n++ )
>>>> +    {
>>>> +        const struct ffa_partition_info_1_0 *fpi =
>>>> +            (const void *)(src + n * fpi_size);
>>>> +
>>>> +        if ( !FFA_ID_IS_SECURE(fpi->id) )
>>>> +            continue;
>>>> +
>>>> +        memcpy(sp_list + idx * fpi_size, fpi, fpi_size);
>>>> +        idx++;
>>>> +    }
>>>> +
>>>> +    return true;
>>>> +}
>>>> +
>>>> static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
>>>> {
>>>>    uint16_t n;
>>>> @@ -549,12 +610,22 @@ bool ffa_partinfo_init(void)
>>>>        goto out;
>>>>    }
>>>> 
>>>> -    ret = init_subscribers(spmc_rx, count, fpi_size);
>>>> +    if ( !ffa_sp_list_cache_init(spmc_rx, count, fpi_size) )
>>>> +    {
>>>> +        printk(XENLOG_ERR "ffa: Failed to cache SP list\n");
>>>> +        goto out;
>>>> +    }
>>>> +
>>>> +    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
>>>> 
>>>> out:
>>>>    e = ffa_rxtx_spmc_rx_release(notify_fw);
>>>>    if ( e )
>>>>        printk(XENLOG_WARNING "ffa: Error releasing SPMC RX buffer: %d\n", e);
>>>> +    if ( !ret )
>>>> +        uninit_subscribers();
>>> 
>>> ret is initially false and can only be set to true if
>>> init_subscribers() returns true. So there's never any point in calling
>>> uninit_subscribers().
>> 
>> True, I will fix that.
>> 
>>> 
>>>> +    if ( !ret )
>>>> +        ffa_sp_list_cache_free();
>>> 
>>> ret can be false even if ffa_sp_list_cache_init() hasn't been called
>>> yet. Calling ffa_sp_list_cache_free() anyway is harmless, but not
>>> elegant.
>> 
>> yes, i will rework a bit the cleanup logic here.
>> 
>> Thanks for the review
>> 
>> Cheers
>> Bertrand
>> 
>>> 
>>> Cheers,
>>> Jens
>>> 
>>>>    return ret;
>>>> }
>>>> 
>>>> --
>>>> 2.52.0

Hi Bertrand,

On Mon, Mar 2, 2026 at 10:58 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 2 Mar 2026, at 10:32, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi Bertrand,
> >
> > On Mon, Mar 2, 2026 at 9:51 AM Bertrand Marquis
> > <Bertrand.Marquis@arm.com> wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 27 Feb 2026, at 11:39, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >>>
> >>> Hi Bertrand,
> >>>
> >>> On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
> >>> <bertrand.marquis@arm.com> wrote:
> >>>>
> >>>> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks the
> >>>> RX buffer every time. The SP list is expected to be static, so repeated
> >>>> firmware calls and validation are unnecessary.
> >>>>
> >>>> Cache the SPMC partition-info list at init time, keeping only secure
> >>>> endpoints, and reuse the cached entries for SP count and partition-info
> >>>> responses. Initialize the VM create/destroy subscriber lists from the cached
> >>>> list and free the cache on init failure.
> >>>>
> >>>> SP partition info now reflects the init-time snapshot and will not change.
> >>>>
> >>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >>>> ---
> >>>> xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++-----------
> >>>> 1 file changed, 138 insertions(+), 67 deletions(-)
> >>>>
[snip]
> >>>> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
> >>>> +                                   uint32_t fpi_size)
> >>>> +{
> >>>> +    const uint8_t *src = buf;
> >>>> +    uint32_t secure_count = 0;
> >>>> +    uint32_t n, idx = 0;
> >>>> +    bool warned = false;
> >>>> +
> >>>> +    if ( fpi_size < sizeof(struct ffa_partition_info_1_0) ||
> >>>> +         fpi_size >= FFA_PAGE_SIZE )
> >>>> +        return false;
> >>>
> >>> Would it make sense to check that fpi_size is well aligned with struct
> >>> ffa_partition_info_1_0? If it's an odd size, we'll make unaligned
> >>> accesses below with memcpy(). But perhaps that's a bit much. The SPMC
> >>> isn't supposed to provide garbage.
> >>
> >> Memcpy should prevent issues even if accesses are not aligned.
> >> If we had this test, we cannot return an error to the SPMC so we would have to
> >> generate one to the caller. It is simpler i think to handle non-aligned as we do not
> >> expect the SPMC to generate such a case.
> >> Tell me if you agree.
> >
> > We dereference fpi below, and depending on compiler flags and pointer
> > types, memcpy() might not be safe with unaligned pointers.
> > From 6.3.2.3 Pointers, paragraph 7, in the C standard:
> > "A pointer to an object type may be converted to a pointer to a
> > different object type. If the
> > resulting pointer is not correctly aligned for the referenced type,
> > the behavior is
> > undefined."
> >
> > I've seen past examples where the compiler optimized memcpy() in a way
> > that breaks with unaligned pointers.
> >
> > We don't expect the test above to fail, but if it does we will not use
> > the secure firmware. I think refusing unexpected sizes is even
> > simpler. It should make finding eventual errors much easier.
>
> In the ffa spec, the size can grow and this is why there is a size field.
> FF-A expect that we either ignore or copy without looking the extra
> content.
> I think we should not be dependent on any alignment so we need to
> make sure we do the copy in a way that is robust to non alignments.
>
> >
> > So my question above is whether it's worth checking that fpi_size is
> > well-aligned, or if it's so unlikely that we don't need to consider
> > it.
>
> I think we need to find a solution where we handle properly things even
> if the size is not aligned.

I agree, that's the most robust.

> I though using memcpy would protect against
> that but maybe we need to use a stronger solution to ensure that works
> even if data is unaligned.

memcpy() can be used, but the pointer values must not pass through a struct
ffa_partition_info_1_0 pointer or such.

Cheers,
Jens