Re: [PATCH v2 5/6] xen/arm: ffa: Track VM notification bindings locally

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertrand,

On Wed, Apr 29, 2026 at 7:44 AM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> VM-to-VM notifications need receiver-side bind state so Xen can validate
> which sender owns each notification bit. Non-secure BIND and UNBIND
> requests currently have no local state and cannot enforce that contract.
>
> Add per-bit VM notification binding state to struct ffa_ctx_notif and
> use it to handle non-secure BIND and UNBIND requests when
> CONFIG_FFA_VM_TO_VM is enabled. The update helper validates the whole
> request under notif_lock before mutating anything, denies bind or
> unbind when a bit is pending, rejects rebinding to a different sender,
> and keeps rebinding to the same sender idempotent.
>
> Promote vm_pending to a bitmap so the bind logic can reason per
> notification ID, use that bitmap directly when reporting pending state,
> and initialize and clear the new VM notification state during domain
> init and teardown.
>
> Functional impact: when CONFIG_FFA_VM_TO_VM is enabled, Xen tracks VM
> notification bindings locally and validates non-secure bind and unbind
> requests against that state.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> Changes since v1:
> - use memset() to clear vm_bind[] in init/destroy
> - replace the file-scope #error check with BUILD_BUG_ON()
> ---
>  xen/arch/arm/tee/ffa_notif.c   | 96 ++++++++++++++++++++++++++++++----
>  xen/arch/arm/tee/ffa_private.h | 11 ++--
>  2 files changed, 94 insertions(+), 13 deletions(-)

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

Cheers,
Jens

>
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index e1cd852d1c53..a841c8f8d747 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -8,6 +8,7 @@
>  #include <xen/list.h>
>  #include <xen/notifier.h>
>  #include <xen/spinlock.h>
> +#include <xen/string.h>
>  #include <xen/tasklet.h>
>  #include <xen/types.h>
>
> @@ -58,6 +59,54 @@ static int32_t ffa_notif_validate_params(uint16_t dom_id, 
> uint16_t caller_id,
>      return FFA_RET_OK;
>  }
>
> +static int32_t ffa_notif_update_vm_binding(struct ffa_ctx *ctx,
> +                                           uint16_t dest_id, uint64_t bitmap,
> +                                           bool bind)
> +{
> +    unsigned int id;
> +    int32_t ret = FFA_RET_OK;
> +
> +    spin_lock(&ctx->notif.notif_lock);
> +
> +    for ( id = 0; id < FFA_NUM_VM_NOTIF; id++ )
> +    {
> +        if ( !(bitmap & BIT(id, ULL)) )
> +            continue;
> +
> +        if ( ctx->notif.vm_pending & BIT(id, ULL) )
> +        {
> +            ret = FFA_RET_DENIED;
> +            goto out_unlock;
> +        }
> +
> +        if ( bind )
> +        {
> +            if ( ctx->notif.vm_bind[id] != 0 &&
> +                 ctx->notif.vm_bind[id] != dest_id )
> +            {
> +                ret = FFA_RET_DENIED;
> +                goto out_unlock;
> +            }
> +        }
> +        else if ( ctx->notif.vm_bind[id] != dest_id )
> +        {
> +            ret = FFA_RET_DENIED;
> +            goto out_unlock;
> +        }
> +    }
> +
> +    for ( id = 0; id < FFA_NUM_VM_NOTIF; id++ )
> +    {
> +        if ( bitmap & BIT(id, ULL) )
> +            ctx->notif.vm_bind[id] = bind ? dest_id : 0;
> +    }
> +
> +out_unlock:
> +    spin_unlock(&ctx->notif.notif_lock);
> +
> +    return ret;
> +}
> +
>  int32_t ffa_handle_notification_bind(struct cpu_user_regs *regs)
>  {
>      struct domain *d = current->domain;
> @@ -78,11 +127,21 @@ int32_t ffa_handle_notification_bind(struct 
> cpu_user_regs *regs)
>      if ( ret )
>          return ret;
>
> -    if ( FFA_ID_IS_SECURE(dest_id) && fw_notif_enabled )
> -        return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags,
> -                               bitmap_lo, bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(dest_id) )
> +    {
> +        if ( fw_notif_enabled )
> +            return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags,
> +                                   bitmap_lo, bitmap_hi);
>
> -    return FFA_RET_NOT_SUPPORTED;
> +        return FFA_RET_NOT_SUPPORTED;
> +    }
> +
> +    if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> +        return FFA_RET_NOT_SUPPORTED;
> +
> +    return ffa_notif_update_vm_binding(ctx, dest_id,
> +                                       ((uint64_t)bitmap_hi << 32) | 
> bitmap_lo,
> +                                       true);
>  }
>
>  int32_t ffa_handle_notification_unbind(struct cpu_user_regs *regs)
> @@ -101,11 +160,21 @@ int32_t ffa_handle_notification_unbind(struct 
> cpu_user_regs *regs)
>      if ( ret )
>          return ret;
>
> -    if ( FFA_ID_IS_SECURE(dest_id) && fw_notif_enabled )
> -        return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, 
> bitmap_lo,
> -                               bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(dest_id) )
> +    {
> +        if ( fw_notif_enabled )
> +            return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0,
> +                                   bitmap_lo, bitmap_hi);
>
> -    return FFA_RET_NOT_SUPPORTED;
> +        return FFA_RET_NOT_SUPPORTED;
> +    }
> +
> +    if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> +        return FFA_RET_NOT_SUPPORTED;
> +
> +    return ffa_notif_update_vm_binding(ctx, dest_id,
> +                                       ((uint64_t)bitmap_hi << 32) | 
> bitmap_lo,
> +                                       false);
>  }
>
>  void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
> @@ -127,9 +196,10 @@ void ffa_handle_notification_info_get(struct 
> cpu_user_regs *regs)
>
>      if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>      {
> -        notif_pending |= test_and_clear_bool(ctx->notif.vm_pending);
> -
>          spin_lock(&ctx->notif.notif_lock);
> +        if ( ctx->notif.vm_pending )
> +            notif_pending = true;
> +
>          if ( ctx->notif.hyp_pending )
>              notif_pending = true;
>          spin_unlock(&ctx->notif.notif_lock);
> @@ -498,9 +568,13 @@ int ffa_notif_domain_init(struct domain *d)
>      struct ffa_ctx *ctx = d->arch.tee;
>      int32_t res;
>
> +    BUILD_BUG_ON(FFA_NUM_VM_NOTIF > 64);
> +
>      spin_lock_init(&ctx->notif.notif_lock);
>      ctx->notif.notif_irq_raised = false;
>      ctx->notif.secure_pending = false;
> +    ctx->notif.vm_pending = 0;
> +    memset(ctx->notif.vm_bind, 0, sizeof(ctx->notif.vm_bind));
>      ctx->notif.hyp_pending = 0;
>
>      if ( fw_notif_enabled )
> @@ -520,6 +594,8 @@ void ffa_notif_domain_destroy(struct domain *d)
>      spin_lock(&ctx->notif.notif_lock);
>      ctx->notif.notif_irq_raised = false;
>      ctx->notif.secure_pending = false;
> +    ctx->notif.vm_pending = 0;
> +    memset(ctx->notif.vm_bind, 0, sizeof(ctx->notif.vm_bind));
>      ctx->notif.hyp_pending = 0;
>      spin_unlock(&ctx->notif.notif_lock);
>
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index 9ddda3f88986..78a0a9815d56 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -236,6 +236,7 @@
>  #define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
>
>  #define FFA_NOTIF_RX_BUFFER_FULL        BIT(0, U)
> +#define FFA_NUM_VM_NOTIF                64U
>
>  /* Feature IDs used with FFA_FEATURES */
>  #define FFA_FEATURE_NOTIF_PEND_INTR     0x1U
> @@ -334,10 +335,14 @@ struct ffa_ctx_notif {
>      bool secure_pending;
>
>      /*
> -     * True if domain is reported by FFA_NOTIFICATION_INFO_GET to have
> -     * pending notifications from VMs (including framework ones).
> +     * Bitmap of pending notifications from VMs (including framework ones).
> +     */
> +    uint64_t vm_pending;
> +
> +    /*
> +     * Source endpoint bound to each VM notification ID (0 means unbound).
>       */
> -    bool vm_pending;
> +    uint16_t vm_bind[FFA_NUM_VM_NOTIF];
>
>      /*
>       * Lock protecting the hypervisor-managed notification state.
> --
> 2.53.0
>