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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jens,

> On 22 Apr 2026, at 15:21, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On Fri, Apr 17, 2026 at 3:41 PM 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>
>> ---
>> xen/arch/arm/tee/ffa_notif.c   | 97 ++++++++++++++++++++++++++++++----
>> xen/arch/arm/tee/ffa_private.h | 15 ++++--
>> 2 files changed, 99 insertions(+), 13 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>> index fff00ca2baec..4def701f0130 100644
>> --- a/xen/arch/arm/tee/ffa_notif.c
>> +++ b/xen/arch/arm/tee/ffa_notif.c
>> @@ -56,6 +56,54 @@ static int32_t ffa_notif_parse_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;
>> @@ -76,11 +124,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)
>> @@ -99,11 +157,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)
>> @@ -125,9 +193,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);
>> @@ -497,10 +566,14 @@ void ffa_notif_init(void)
>> int ffa_notif_domain_init(struct domain *d)
>> {
>>     struct ffa_ctx *ctx = d->arch.tee;
>> +    unsigned int i;
>>     int32_t res;
>> 
>>     spin_lock_init(&ctx->notif.notif_lock);
>>     ctx->notif.secure_pending = false;
>> +    ctx->notif.vm_pending = 0;
>> +    for ( i = 0; i < FFA_NUM_VM_NOTIF; i++ )
>> +        ctx->notif.vm_bind[i] = 0;
>>     ctx->notif.hyp_pending = 0;
>> 
>>     if ( fw_notif_enabled )
>> @@ -516,9 +589,13 @@ int ffa_notif_domain_init(struct domain *d)
>> void ffa_notif_domain_destroy(struct domain *d)
>> {
>>     struct ffa_ctx *ctx = d->arch.tee;
>> +    unsigned int i;
>> 
>>     spin_lock(&ctx->notif.notif_lock);
>>     ctx->notif.secure_pending = false;
>> +    ctx->notif.vm_pending = 0;
>> +    for ( i = 0; i < FFA_NUM_VM_NOTIF; i++ )
>> +        ctx->notif.vm_bind[i] = 0;
> 
> Why not memset(ctx->notif.vm_bind, 0, sizeof(ctx->notif.vm_bind)?

Definitely better to use memset, I will use it here and in domain_init
instead of an explicit loop.

> 
>>     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 5693772481ed..6d83afb3d00a 100644
>> --- a/xen/arch/arm/tee/ffa_private.h
>> +++ b/xen/arch/arm/tee/ffa_private.h
>> @@ -236,6 +236,11 @@
>> #define FFA_NOTIF_INFO_GET_ID_COUNT_MASK    0x1F
>> 
>> #define FFA_NOTIF_RX_BUFFER_FULL        BIT(0, U)
>> +#define FFA_NUM_VM_NOTIF                64U
>> +
>> +#if FFA_NUM_VM_NOTIF > 64
>> +#error "FFA_NUM_VM_NOTIF must be <= 64"
>> +#endif
> 
> BUILD_BUG_ON(FFA_NUM_VM_NOTIF > 64) ?

Yes that would be cleaner.
I will put it in ffa_notif_domain_init and remove that
from the header.

Cheers
Bertrand