[PATCH 3/4] xen/arm: ffa: Drop SP subscriber lists

Bertrand Marquis posted 4 patches 5 days, 17 hours ago
There is a newer version of this series
[PATCH 3/4] xen/arm: ffa: Drop SP subscriber lists
Posted by Bertrand Marquis 5 days, 17 hours ago
The init-time SP cache already contains partition properties, but the
code still builds separate subscriber arrays for VM created/destroyed
notifications. That duplicates state and allocation.

Use the cached SP list directly to:
- decide which SPs receive created/destroyed notifications
- build the per-domain destroy bitmap
- skip destroy notifications for SPs not notified on create

No functional changes.

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

diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
index 8a3eac25f99f..d7f9b9f7153c 100644
--- a/xen/arch/arm/tee/ffa_partinfo.c
+++ b/xen/arch/arm/tee/ffa_partinfo.c
@@ -28,12 +28,6 @@ struct ffa_partition_info_1_1 {
     uint8_t uuid[16];
 };
 
-/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
-static uint16_t *subscr_vm_created __read_mostly;
-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;
@@ -434,14 +428,6 @@ static void ffa_sp_list_cache_free(void)
     sp_list_entry_size = 0;
 }
 
-static void uninit_subscribers(void)
-{
-        subscr_vm_created_count = 0;
-        subscr_vm_destroyed_count = 0;
-        XFREE(subscr_vm_created);
-        XFREE(subscr_vm_destroyed);
-}
-
 static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
                                    uint32_t fpi_size)
 {
@@ -504,79 +490,6 @@ static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
     return true;
 }
 
-static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
-{
-    uint16_t n;
-    uint16_t c_pos;
-    uint16_t d_pos;
-    struct ffa_partition_info_1_1 *fpi;
-
-    if ( fpi_size < sizeof(struct ffa_partition_info_1_1) )
-    {
-        printk(XENLOG_ERR "ffa: partition info size invalid: %u\n", fpi_size);
-        return false;
-    }
-
-    subscr_vm_created_count = 0;
-    subscr_vm_destroyed_count = 0;
-    for ( n = 0; n < count; n++ )
-    {
-        fpi = buf + n * fpi_size;
-
-        /*
-         * We need to have secure partitions using bit 15 set convention for
-         * secure partition IDs.
-         * Inform the user with a log and discard giving created or destroy
-         * event to those IDs.
-         */
-        if ( !FFA_ID_IS_SECURE(fpi->id) )
-        {
-            printk_once(XENLOG_ERR
-                        "ffa: Firmware is not using bit 15 convention for IDs !!\n");
-            printk(XENLOG_ERR
-                   "ffa: Secure partition with id 0x%04x cannot be used\n",
-                   fpi->id);
-        }
-        else
-        {
-            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED )
-                subscr_vm_created_count++;
-            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
-                subscr_vm_destroyed_count++;
-        }
-    }
-
-    if ( subscr_vm_created_count )
-        subscr_vm_created = xzalloc_array(uint16_t, subscr_vm_created_count);
-    if ( subscr_vm_destroyed_count )
-        subscr_vm_destroyed = xzalloc_array(uint16_t,
-                                            subscr_vm_destroyed_count);
-    if ( (subscr_vm_created_count && !subscr_vm_created) ||
-         (subscr_vm_destroyed_count && !subscr_vm_destroyed) )
-    {
-        printk(XENLOG_ERR "ffa: Failed to allocate subscription lists\n");
-        uninit_subscribers();
-        return false;
-    }
-
-    for ( c_pos = 0, d_pos = 0, n = 0; n < count; n++ )
-    {
-        fpi = buf + n * fpi_size;
-
-        if ( FFA_ID_IS_SECURE(fpi->id) )
-        {
-            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED )
-                subscr_vm_created[c_pos++] = fpi->id;
-            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
-                subscr_vm_destroyed[d_pos++] = fpi->id;
-        }
-    }
-
-    return true;
-}
-
-
-
 bool ffa_partinfo_init(void)
 {
     bool ret = false;
@@ -616,48 +529,43 @@ bool ffa_partinfo_init(void)
         goto out;
     }
 
-    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
+    if ( sp_list_entry_size < sizeof(struct ffa_partition_info_1_1) )
+    {
+        printk(XENLOG_ERR "ffa: partition info size invalid: %u\n",
+               sp_list_entry_size);
+        goto out;
+    }
+    ret = true;
 
 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;
 }
 
-static bool is_in_subscr_list(const uint16_t *subscr, uint16_t start,
-                              uint16_t end, uint16_t sp_id)
+static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
+                                   unsigned int first_unnotified)
 {
     unsigned int n;
+    struct ffa_partition_info_1_1 *fpi;
 
-    for ( n = start; n < end; n++ )
+    for ( n = 0; n < sp_list_count; n++ )
     {
-        if ( subscr[n] == sp_id )
-            return true;
-    }
-
-    return false;
-}
+        fpi = sp_list + n * sp_list_entry_size;
 
-static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
-                                   unsigned int create_signal_count)
-{
-    unsigned int n;
+        if ( !(fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED) )
+            continue;
 
-    for ( n = 0; n < subscr_vm_destroyed_count; n++ )
-    {
         /*
          * Skip SPs subscribed to the VM created event that never was
          * notified of the VM creation due to an error during
          * ffa_domain_init().
          */
-        if ( is_in_subscr_list(subscr_vm_created, create_signal_count,
-                               subscr_vm_created_count,
-                               subscr_vm_destroyed[n]) )
+        if ( (fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED) &&
+             n >= first_unnotified )
             continue;
 
         set_bit(n, ctx->vm_destroy_bitmap);
@@ -666,32 +574,39 @@ static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
 
 int32_t ffa_partinfo_domain_init(struct domain *d)
 {
-    unsigned int count = BITS_TO_LONGS(subscr_vm_destroyed_count);
+    unsigned int count = BITS_TO_LONGS(sp_list_count);
     struct ffa_ctx *ctx = d->arch.tee;
     unsigned int n;
+    unsigned int first_unnotified = sp_list_count;
     int32_t res;
+    struct ffa_partition_info_1_1 *fpi;
 
-    if ( !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) )
+    if ( !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) || !sp_list_count )
         return 0;
 
     ctx->vm_destroy_bitmap = xzalloc_array(unsigned long, count);
     if ( !ctx->vm_destroy_bitmap )
         return -ENOMEM;
 
-    for ( n = 0; n < subscr_vm_created_count; n++ )
+    for ( n = 0; n < sp_list_count; n++ )
     {
-        res = ffa_direct_req_send_vm(subscr_vm_created[n], ffa_get_vm_id(d),
+        fpi = sp_list + n * sp_list_entry_size;
+        if ( !(fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED) )
+            continue;
+
+        res = ffa_direct_req_send_vm(fpi->id, ffa_get_vm_id(d),
                                      FFA_MSG_SEND_VM_CREATED);
         if ( res )
         {
             printk(XENLOG_ERR "ffa: Failed to report creation of vm_id %u to  %u: res %d\n",
-                   ffa_get_vm_id(d), subscr_vm_created[n], res);
+                   ffa_get_vm_id(d), fpi->id, res);
+            first_unnotified = n;
             break;
         }
     }
-    vm_destroy_bitmap_init(ctx, n);
+    vm_destroy_bitmap_init(ctx, first_unnotified);
 
-    if ( n != subscr_vm_created_count )
+    if ( first_unnotified != sp_list_count )
         return -EIO;
 
     return 0;
@@ -702,22 +617,24 @@ bool ffa_partinfo_domain_destroy(struct domain *d)
     struct ffa_ctx *ctx = d->arch.tee;
     unsigned int n;
     int32_t res;
+    struct ffa_partition_info_1_1 *fpi;
 
     if ( !ctx->vm_destroy_bitmap )
         return true;
 
-    for ( n = 0; n < subscr_vm_destroyed_count; n++ )
+    for ( n = 0; n < sp_list_count; n++ )
     {
         if ( !test_bit(n, ctx->vm_destroy_bitmap) )
             continue;
 
-        res = ffa_direct_req_send_vm(subscr_vm_destroyed[n], ffa_get_vm_id(d),
+        fpi = sp_list + n * sp_list_entry_size;
+        res = ffa_direct_req_send_vm(fpi->id, ffa_get_vm_id(d),
                                      FFA_MSG_SEND_VM_DESTROYED);
 
         if ( res && printk_ratelimit() )
             printk(XENLOG_WARNING
                    "%pd: ffa: Failed to report destruction of vm_id %u to %u: res %d\n",
-                   d, ffa_get_vm_id(d), subscr_vm_destroyed[n], res);
+                   d, ffa_get_vm_id(d), fpi->id, res);
 
         /*
          * For these two error codes the hypervisor is expected to resend
@@ -729,7 +646,7 @@ bool ffa_partinfo_domain_destroy(struct domain *d)
             clear_bit(n, ctx->vm_destroy_bitmap);
     }
 
-    if ( bitmap_empty(ctx->vm_destroy_bitmap, subscr_vm_destroyed_count) )
+    if ( bitmap_empty(ctx->vm_destroy_bitmap, sp_list_count) )
         XFREE(ctx->vm_destroy_bitmap);
 
     return !ctx->vm_destroy_bitmap;
-- 
2.52.0
Re: [PATCH 3/4] xen/arm: ffa: Drop SP subscriber lists
Posted by Jens Wiklander 3 days, 16 hours ago
Hi Bertrand,

On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> The init-time SP cache already contains partition properties, but the
> code still builds separate subscriber arrays for VM created/destroyed
> notifications. That duplicates state and allocation.
>
> Use the cached SP list directly to:
> - decide which SPs receive created/destroyed notifications
> - build the per-domain destroy bitmap
> - skip destroy notifications for SPs not notified on create
>
> No functional changes.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
>  xen/arch/arm/tee/ffa_partinfo.c | 155 ++++++++------------------------
>  1 file changed, 36 insertions(+), 119 deletions(-)

Looks good:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Cheers,
Jens

>
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index 8a3eac25f99f..d7f9b9f7153c 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -28,12 +28,6 @@ struct ffa_partition_info_1_1 {
>      uint8_t uuid[16];
>  };
>
> -/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
> -static uint16_t *subscr_vm_created __read_mostly;
> -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;
> @@ -434,14 +428,6 @@ static void ffa_sp_list_cache_free(void)
>      sp_list_entry_size = 0;
>  }
>
> -static void uninit_subscribers(void)
> -{
> -        subscr_vm_created_count = 0;
> -        subscr_vm_destroyed_count = 0;
> -        XFREE(subscr_vm_created);
> -        XFREE(subscr_vm_destroyed);
> -}
> -
>  static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
>                                     uint32_t fpi_size)
>  {
> @@ -504,79 +490,6 @@ static bool ffa_sp_list_cache_init(const void *buf, uint32_t count,
>      return true;
>  }
>
> -static bool init_subscribers(void *buf, uint16_t count, uint32_t fpi_size)
> -{
> -    uint16_t n;
> -    uint16_t c_pos;
> -    uint16_t d_pos;
> -    struct ffa_partition_info_1_1 *fpi;
> -
> -    if ( fpi_size < sizeof(struct ffa_partition_info_1_1) )
> -    {
> -        printk(XENLOG_ERR "ffa: partition info size invalid: %u\n", fpi_size);
> -        return false;
> -    }
> -
> -    subscr_vm_created_count = 0;
> -    subscr_vm_destroyed_count = 0;
> -    for ( n = 0; n < count; n++ )
> -    {
> -        fpi = buf + n * fpi_size;
> -
> -        /*
> -         * We need to have secure partitions using bit 15 set convention for
> -         * secure partition IDs.
> -         * Inform the user with a log and discard giving created or destroy
> -         * event to those IDs.
> -         */
> -        if ( !FFA_ID_IS_SECURE(fpi->id) )
> -        {
> -            printk_once(XENLOG_ERR
> -                        "ffa: Firmware is not using bit 15 convention for IDs !!\n");
> -            printk(XENLOG_ERR
> -                   "ffa: Secure partition with id 0x%04x cannot be used\n",
> -                   fpi->id);
> -        }
> -        else
> -        {
> -            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED )
> -                subscr_vm_created_count++;
> -            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
> -                subscr_vm_destroyed_count++;
> -        }
> -    }
> -
> -    if ( subscr_vm_created_count )
> -        subscr_vm_created = xzalloc_array(uint16_t, subscr_vm_created_count);
> -    if ( subscr_vm_destroyed_count )
> -        subscr_vm_destroyed = xzalloc_array(uint16_t,
> -                                            subscr_vm_destroyed_count);
> -    if ( (subscr_vm_created_count && !subscr_vm_created) ||
> -         (subscr_vm_destroyed_count && !subscr_vm_destroyed) )
> -    {
> -        printk(XENLOG_ERR "ffa: Failed to allocate subscription lists\n");
> -        uninit_subscribers();
> -        return false;
> -    }
> -
> -    for ( c_pos = 0, d_pos = 0, n = 0; n < count; n++ )
> -    {
> -        fpi = buf + n * fpi_size;
> -
> -        if ( FFA_ID_IS_SECURE(fpi->id) )
> -        {
> -            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED )
> -                subscr_vm_created[c_pos++] = fpi->id;
> -            if ( fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
> -                subscr_vm_destroyed[d_pos++] = fpi->id;
> -        }
> -    }
> -
> -    return true;
> -}
> -
> -
> -
>  bool ffa_partinfo_init(void)
>  {
>      bool ret = false;
> @@ -616,48 +529,43 @@ bool ffa_partinfo_init(void)
>          goto out;
>      }
>
> -    ret = init_subscribers(sp_list, sp_list_count, sp_list_entry_size);
> +    if ( sp_list_entry_size < sizeof(struct ffa_partition_info_1_1) )
> +    {
> +        printk(XENLOG_ERR "ffa: partition info size invalid: %u\n",
> +               sp_list_entry_size);
> +        goto out;
> +    }
> +    ret = true;
>
>  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;
>  }
>
> -static bool is_in_subscr_list(const uint16_t *subscr, uint16_t start,
> -                              uint16_t end, uint16_t sp_id)
> +static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
> +                                   unsigned int first_unnotified)
>  {
>      unsigned int n;
> +    struct ffa_partition_info_1_1 *fpi;
>
> -    for ( n = start; n < end; n++ )
> +    for ( n = 0; n < sp_list_count; n++ )
>      {
> -        if ( subscr[n] == sp_id )
> -            return true;
> -    }
> -
> -    return false;
> -}
> +        fpi = sp_list + n * sp_list_entry_size;
>
> -static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
> -                                   unsigned int create_signal_count)
> -{
> -    unsigned int n;
> +        if ( !(fpi->partition_properties & FFA_PART_PROP_NOTIF_DESTROYED) )
> +            continue;
>
> -    for ( n = 0; n < subscr_vm_destroyed_count; n++ )
> -    {
>          /*
>           * Skip SPs subscribed to the VM created event that never was
>           * notified of the VM creation due to an error during
>           * ffa_domain_init().
>           */
> -        if ( is_in_subscr_list(subscr_vm_created, create_signal_count,
> -                               subscr_vm_created_count,
> -                               subscr_vm_destroyed[n]) )
> +        if ( (fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED) &&
> +             n >= first_unnotified )
>              continue;
>
>          set_bit(n, ctx->vm_destroy_bitmap);
> @@ -666,32 +574,39 @@ static void vm_destroy_bitmap_init(struct ffa_ctx *ctx,
>
>  int32_t ffa_partinfo_domain_init(struct domain *d)
>  {
> -    unsigned int count = BITS_TO_LONGS(subscr_vm_destroyed_count);
> +    unsigned int count = BITS_TO_LONGS(sp_list_count);
>      struct ffa_ctx *ctx = d->arch.tee;
>      unsigned int n;
> +    unsigned int first_unnotified = sp_list_count;
>      int32_t res;
> +    struct ffa_partition_info_1_1 *fpi;
>
> -    if ( !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) )
> +    if ( !ffa_fw_supports_fid(FFA_MSG_SEND_DIRECT_REQ_32) || !sp_list_count )
>          return 0;
>
>      ctx->vm_destroy_bitmap = xzalloc_array(unsigned long, count);
>      if ( !ctx->vm_destroy_bitmap )
>          return -ENOMEM;
>
> -    for ( n = 0; n < subscr_vm_created_count; n++ )
> +    for ( n = 0; n < sp_list_count; n++ )
>      {
> -        res = ffa_direct_req_send_vm(subscr_vm_created[n], ffa_get_vm_id(d),
> +        fpi = sp_list + n * sp_list_entry_size;
> +        if ( !(fpi->partition_properties & FFA_PART_PROP_NOTIF_CREATED) )
> +            continue;
> +
> +        res = ffa_direct_req_send_vm(fpi->id, ffa_get_vm_id(d),
>                                       FFA_MSG_SEND_VM_CREATED);
>          if ( res )
>          {
>              printk(XENLOG_ERR "ffa: Failed to report creation of vm_id %u to  %u: res %d\n",
> -                   ffa_get_vm_id(d), subscr_vm_created[n], res);
> +                   ffa_get_vm_id(d), fpi->id, res);
> +            first_unnotified = n;
>              break;
>          }
>      }
> -    vm_destroy_bitmap_init(ctx, n);
> +    vm_destroy_bitmap_init(ctx, first_unnotified);
>
> -    if ( n != subscr_vm_created_count )
> +    if ( first_unnotified != sp_list_count )
>          return -EIO;
>
>      return 0;
> @@ -702,22 +617,24 @@ bool ffa_partinfo_domain_destroy(struct domain *d)
>      struct ffa_ctx *ctx = d->arch.tee;
>      unsigned int n;
>      int32_t res;
> +    struct ffa_partition_info_1_1 *fpi;
>
>      if ( !ctx->vm_destroy_bitmap )
>          return true;
>
> -    for ( n = 0; n < subscr_vm_destroyed_count; n++ )
> +    for ( n = 0; n < sp_list_count; n++ )
>      {
>          if ( !test_bit(n, ctx->vm_destroy_bitmap) )
>              continue;
>
> -        res = ffa_direct_req_send_vm(subscr_vm_destroyed[n], ffa_get_vm_id(d),
> +        fpi = sp_list + n * sp_list_entry_size;
> +        res = ffa_direct_req_send_vm(fpi->id, ffa_get_vm_id(d),
>                                       FFA_MSG_SEND_VM_DESTROYED);
>
>          if ( res && printk_ratelimit() )
>              printk(XENLOG_WARNING
>                     "%pd: ffa: Failed to report destruction of vm_id %u to %u: res %d\n",
> -                   d, ffa_get_vm_id(d), subscr_vm_destroyed[n], res);
> +                   d, ffa_get_vm_id(d), fpi->id, res);
>
>          /*
>           * For these two error codes the hypervisor is expected to resend
> @@ -729,7 +646,7 @@ bool ffa_partinfo_domain_destroy(struct domain *d)
>              clear_bit(n, ctx->vm_destroy_bitmap);
>      }
>
> -    if ( bitmap_empty(ctx->vm_destroy_bitmap, subscr_vm_destroyed_count) )
> +    if ( bitmap_empty(ctx->vm_destroy_bitmap, sp_list_count) )
>          XFREE(ctx->vm_destroy_bitmap);
>
>      return !ctx->vm_destroy_bitmap;
> --
> 2.52.0
>