Hypervisor notifications are currently tracked with a dedicated
buff_full_pending boolean. That state only represents a single HYP
notification bit and keeps HYP bitmap handling tied to single-purpose
bookkeeping.

Replace the boolean with a hypervisor notification bitmap protected by
notif_lock. INFO_GET reports pending when the bitmap is non-zero, GET
returns and clears the HYP bitmap under the lock, and RX-buffer-full
sets FFA_NOTIF_RX_BUFFER_FULL in the bitmap instead of updating
separate state.

Initialize and clear the bitmap during domain lifecycle handling, and
use ctx->ffa_id for bitmap create and destroy so the notification state
stays tied to the cached FF-A endpoint ID.

No functional changes.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 xen/arch/arm/tee/ffa_notif.c   | 46 ++++++++++++++++++++++++++--------
 xen/arch/arm/tee/ffa_private.h |  9 +++++--
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
index 07bc5cb3a430..d15119409a25 100644
--- a/xen/arch/arm/tee/ffa_notif.c
+++ b/xen/arch/arm/tee/ffa_notif.c
@@ -94,8 +94,15 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
 
     notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
     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.hyp_pending )
+            notif_pending = true;
+        spin_unlock(&ctx->notif.notif_lock);
+    }
+
     if ( notif_pending )
     {
         /* A pending global notification for the guest */
@@ -174,12 +181,17 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
             w6 = resp.a6;
     }
 
-    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) &&
-          flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
-          test_and_clear_bool(ctx->notif.buff_full_pending) )
+    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
     {
-        ACCESS_ONCE(ctx->notif.vm_pending) = false;
-        w7 = FFA_NOTIF_RX_BUFFER_FULL;
+        spin_lock(&ctx->notif.notif_lock);
+
+        if ( (flags & FFA_NOTIF_FLAG_BITMAP_HYP) && ctx->notif.hyp_pending )
+        {
+            w7 = ctx->notif.hyp_pending;
+            ctx->notif.hyp_pending = 0;
+        }
+
+        spin_unlock(&ctx->notif.notif_lock);
     }
 
     ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
@@ -207,12 +219,17 @@ int32_t ffa_handle_notification_set(struct cpu_user_regs *regs)
 void ffa_raise_rx_buffer_full(struct domain *d)
 {
     struct ffa_ctx *ctx = d->arch.tee;
+    uint32_t prev_bitmap;
 
     if ( !ctx )
         return;
 
-    ACCESS_ONCE(ctx->notif.buff_full_pending) = true;
-    if ( !test_and_set_bool(ctx->notif.vm_pending) )
+    spin_lock(&ctx->notif.notif_lock);
+    prev_bitmap = ctx->notif.hyp_pending;
+    ctx->notif.hyp_pending |= FFA_NOTIF_RX_BUFFER_FULL;
+    spin_unlock(&ctx->notif.notif_lock);
+
+    if ( !(prev_bitmap & FFA_NOTIF_RX_BUFFER_FULL) )
         inject_notif_pending(d);
 }
 #endif
@@ -426,12 +443,15 @@ void ffa_notif_init(void)
 
 int ffa_notif_domain_init(struct domain *d)
 {
+    struct ffa_ctx *ctx = d->arch.tee;
     int32_t res;
 
+    spin_lock_init(&ctx->notif.notif_lock);
+    ctx->notif.hyp_pending = 0;
+
     if ( fw_notif_enabled )
     {
-
-        res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
+        res = ffa_notification_bitmap_create(ctx->ffa_id, d->max_vcpus);
         if ( res )
             return -ENOMEM;
     }
@@ -441,10 +461,16 @@ int ffa_notif_domain_init(struct domain *d)
 
 void ffa_notif_domain_destroy(struct domain *d)
 {
+    struct ffa_ctx *ctx = d->arch.tee;
+
+    spin_lock(&ctx->notif.notif_lock);
+    ctx->notif.hyp_pending = 0;
+    spin_unlock(&ctx->notif.notif_lock);
+
     /*
      * Call bitmap_destroy even if bitmap create failed as the SPMC will
      * return a DENIED error that we will ignore.
      */
     if ( fw_notif_enabled )
-        ffa_notification_bitmap_destroy(ffa_get_vm_id(d));
+        ffa_notification_bitmap_destroy(ctx->ffa_id);
 }
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index c291f32b56ff..5693772481ed 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -340,9 +340,14 @@ struct ffa_ctx_notif {
     bool vm_pending;
 
     /*
-     * True if domain has buffer full notification pending
+     * Lock protecting the hypervisor-managed notification state.
      */
-    bool buff_full_pending;
+    spinlock_t notif_lock;
+
+    /*
+     * Bitmap of pending hypervisor notifications (for HYP bitmap queries).
+     */
+    uint32_t hyp_pending;
 };
 
 struct ffa_ctx {
-- 
2.53.0

Hi Bertrand,

On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
<bertrand.marquis@arm.com> wrote:
>
> Hypervisor notifications are currently tracked with a dedicated
> buff_full_pending boolean. That state only represents a single HYP
> notification bit and keeps HYP bitmap handling tied to single-purpose
> bookkeeping.
>
> Replace the boolean with a hypervisor notification bitmap protected by
> notif_lock. INFO_GET reports pending when the bitmap is non-zero, GET
> returns and clears the HYP bitmap under the lock, and RX-buffer-full
> sets FFA_NOTIF_RX_BUFFER_FULL in the bitmap instead of updating
> separate state.
>
> Initialize and clear the bitmap during domain lifecycle handling, and
> use ctx->ffa_id for bitmap create and destroy so the notification state
> stays tied to the cached FF-A endpoint ID.
>
> No functional changes.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
>  xen/arch/arm/tee/ffa_notif.c   | 46 ++++++++++++++++++++++++++--------
>  xen/arch/arm/tee/ffa_private.h |  9 +++++--
>  2 files changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index 07bc5cb3a430..d15119409a25 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -94,8 +94,15 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>
>      notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
>      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.hyp_pending )
> +            notif_pending = true;
> +        spin_unlock(&ctx->notif.notif_lock);

Isn't this a functional change? Before this patch, we didn't consider
ctx->notif.buff_full_pending here. Am I missing something?

> +    }
> +
>      if ( notif_pending )
>      {
>          /* A pending global notification for the guest */
> @@ -174,12 +181,17 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>              w6 = resp.a6;
>      }
>
> -    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) &&
> -          flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
> -          test_and_clear_bool(ctx->notif.buff_full_pending) )
> +    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>      {
> -        ACCESS_ONCE(ctx->notif.vm_pending) = false;
> -        w7 = FFA_NOTIF_RX_BUFFER_FULL;
> +        spin_lock(&ctx->notif.notif_lock);
> +
> +        if ( (flags & FFA_NOTIF_FLAG_BITMAP_HYP) && ctx->notif.hyp_pending )
> +        {
> +            w7 = ctx->notif.hyp_pending;
> +            ctx->notif.hyp_pending = 0;
> +        }
> +
> +        spin_unlock(&ctx->notif.notif_lock);
>      }
>
>      ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
> @@ -207,12 +219,17 @@ int32_t ffa_handle_notification_set(struct cpu_user_regs *regs)
>  void ffa_raise_rx_buffer_full(struct domain *d)
>  {
>      struct ffa_ctx *ctx = d->arch.tee;
> +    uint32_t prev_bitmap;
>
>      if ( !ctx )
>          return;
>
> -    ACCESS_ONCE(ctx->notif.buff_full_pending) = true;
> -    if ( !test_and_set_bool(ctx->notif.vm_pending) )
> +    spin_lock(&ctx->notif.notif_lock);
> +    prev_bitmap = ctx->notif.hyp_pending;
> +    ctx->notif.hyp_pending |= FFA_NOTIF_RX_BUFFER_FULL;
> +    spin_unlock(&ctx->notif.notif_lock);
> +
> +    if ( !(prev_bitmap & FFA_NOTIF_RX_BUFFER_FULL) )

Do we need to check for FFA_NOTIF_RX_BUFFER_FULL? Isn't !prev_bitmap
more accurate, if any other bit would ever be used in the bitmap?

Cheers,
Jens

>          inject_notif_pending(d);
>  }
>  #endif
> @@ -426,12 +443,15 @@ void ffa_notif_init(void)
>
>  int ffa_notif_domain_init(struct domain *d)
>  {
> +    struct ffa_ctx *ctx = d->arch.tee;
>      int32_t res;
>
> +    spin_lock_init(&ctx->notif.notif_lock);
> +    ctx->notif.hyp_pending = 0;
> +
>      if ( fw_notif_enabled )
>      {
> -
> -        res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
> +        res = ffa_notification_bitmap_create(ctx->ffa_id, d->max_vcpus);
>          if ( res )
>              return -ENOMEM;
>      }
> @@ -441,10 +461,16 @@ int ffa_notif_domain_init(struct domain *d)
>
>  void ffa_notif_domain_destroy(struct domain *d)
>  {
> +    struct ffa_ctx *ctx = d->arch.tee;
> +
> +    spin_lock(&ctx->notif.notif_lock);
> +    ctx->notif.hyp_pending = 0;
> +    spin_unlock(&ctx->notif.notif_lock);
> +
>      /*
>       * Call bitmap_destroy even if bitmap create failed as the SPMC will
>       * return a DENIED error that we will ignore.
>       */
>      if ( fw_notif_enabled )
> -        ffa_notification_bitmap_destroy(ffa_get_vm_id(d));
> +        ffa_notification_bitmap_destroy(ctx->ffa_id);
>  }
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index c291f32b56ff..5693772481ed 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -340,9 +340,14 @@ struct ffa_ctx_notif {
>      bool vm_pending;
>
>      /*
> -     * True if domain has buffer full notification pending
> +     * Lock protecting the hypervisor-managed notification state.
>       */
> -    bool buff_full_pending;
> +    spinlock_t notif_lock;
> +
> +    /*
> +     * Bitmap of pending hypervisor notifications (for HYP bitmap queries).
> +     */
> +    uint32_t hyp_pending;
>  };
>
>  struct ffa_ctx {
> --
> 2.53.0
>

Hi Jens,

Thanks a lot for the review.

> On 22 Apr 2026, at 11:34, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
> <bertrand.marquis@arm.com> wrote:
>> 
>> Hypervisor notifications are currently tracked with a dedicated
>> buff_full_pending boolean. That state only represents a single HYP
>> notification bit and keeps HYP bitmap handling tied to single-purpose
>> bookkeeping.
>> 
>> Replace the boolean with a hypervisor notification bitmap protected by
>> notif_lock. INFO_GET reports pending when the bitmap is non-zero, GET
>> returns and clears the HYP bitmap under the lock, and RX-buffer-full
>> sets FFA_NOTIF_RX_BUFFER_FULL in the bitmap instead of updating
>> separate state.
>> 
>> Initialize and clear the bitmap during domain lifecycle handling, and
>> use ctx->ffa_id for bitmap create and destroy so the notification state
>> stays tied to the cached FF-A endpoint ID.
>> 
>> No functional changes.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> xen/arch/arm/tee/ffa_notif.c   | 46 ++++++++++++++++++++++++++--------
>> xen/arch/arm/tee/ffa_private.h |  9 +++++--
>> 2 files changed, 43 insertions(+), 12 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>> index 07bc5cb3a430..d15119409a25 100644
>> --- a/xen/arch/arm/tee/ffa_notif.c
>> +++ b/xen/arch/arm/tee/ffa_notif.c
>> @@ -94,8 +94,15 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>> 
>>     notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
>>     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.hyp_pending )
>> +            notif_pending = true;
>> +        spin_unlock(&ctx->notif.notif_lock);
> 
> Isn't this a functional change? Before this patch, we didn't consider
> ctx->notif.buff_full_pending here. Am I missing something?

We did consider it implicitly through vm_pending.

This patch makes that cleaner by using hyp_pending for the Hypervisor
framework notification itself. Previously, RX-buffer-full was made visible
indirectly via vm_pending, and FFA_NOTIFICATION_INFO_GET 
cleared that summary state.

As a result, the guest-visible pending indication could be lost before
the Hypervisor notification was actually retrieved with
FFA_NOTIFICATION_GET.

With this change, the pending state is tracked in hyp_pending and is only
cleared when the Hypervisor notifications are retrieved through
FFA_NOTIFICATION_GET.

I will reword the commit message to make that clearer.

> 
>> +    }
>> +
>>     if ( notif_pending )
>>     {
>>         /* A pending global notification for the guest */
>> @@ -174,12 +181,17 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>>             w6 = resp.a6;
>>     }
>> 
>> -    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) &&
>> -          flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
>> -          test_and_clear_bool(ctx->notif.buff_full_pending) )
>> +    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>>     {
>> -        ACCESS_ONCE(ctx->notif.vm_pending) = false;
>> -        w7 = FFA_NOTIF_RX_BUFFER_FULL;
>> +        spin_lock(&ctx->notif.notif_lock);
>> +
>> +        if ( (flags & FFA_NOTIF_FLAG_BITMAP_HYP) && ctx->notif.hyp_pending )
>> +        {
>> +            w7 = ctx->notif.hyp_pending;
>> +            ctx->notif.hyp_pending = 0;
>> +        }
>> +
>> +        spin_unlock(&ctx->notif.notif_lock);
>>     }
>> 
>>     ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
>> @@ -207,12 +219,17 @@ int32_t ffa_handle_notification_set(struct cpu_user_regs *regs)
>> void ffa_raise_rx_buffer_full(struct domain *d)
>> {
>>     struct ffa_ctx *ctx = d->arch.tee;
>> +    uint32_t prev_bitmap;
>> 
>>     if ( !ctx )
>>         return;
>> 
>> -    ACCESS_ONCE(ctx->notif.buff_full_pending) = true;
>> -    if ( !test_and_set_bool(ctx->notif.vm_pending) )
>> +    spin_lock(&ctx->notif.notif_lock);
>> +    prev_bitmap = ctx->notif.hyp_pending;
>> +    ctx->notif.hyp_pending |= FFA_NOTIF_RX_BUFFER_FULL;
>> +    spin_unlock(&ctx->notif.notif_lock);
>> +
>> +    if ( !(prev_bitmap & FFA_NOTIF_RX_BUFFER_FULL) )
> 
> Do we need to check for FFA_NOTIF_RX_BUFFER_FULL? Isn't !prev_bitmap
> more accurate, if any other bit would ever be used in the bitmap?

  I would keep the bit-specific check here, if that is OK with you.

 This function is about raising the RX buffer full notification,
 so I think it is clearer to check whether that bit was newly
 pended rather than whether the whole Hypervisor bitmap
 was previously empty.

Using !prev_bitmap would also make the condition depend on
unrelated Hypervisor notifications being pending.

Tell me if you are ok with that.

Cheers
Bertrand

Hi Bertrand,

On Thu, Apr 23, 2026 at 9:30 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> Thanks a lot for the review.
>
> > On 22 Apr 2026, at 11:34, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi Bertrand,
> >
> > On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
> > <bertrand.marquis@arm.com> wrote:
> >>
> >> Hypervisor notifications are currently tracked with a dedicated
> >> buff_full_pending boolean. That state only represents a single HYP
> >> notification bit and keeps HYP bitmap handling tied to single-purpose
> >> bookkeeping.
> >>
> >> Replace the boolean with a hypervisor notification bitmap protected by
> >> notif_lock. INFO_GET reports pending when the bitmap is non-zero, GET
> >> returns and clears the HYP bitmap under the lock, and RX-buffer-full
> >> sets FFA_NOTIF_RX_BUFFER_FULL in the bitmap instead of updating
> >> separate state.
> >>
> >> Initialize and clear the bitmap during domain lifecycle handling, and
> >> use ctx->ffa_id for bitmap create and destroy so the notification state
> >> stays tied to the cached FF-A endpoint ID.
> >>
> >> No functional changes.
> >>
> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >> ---
> >> xen/arch/arm/tee/ffa_notif.c   | 46 ++++++++++++++++++++++++++--------
> >> xen/arch/arm/tee/ffa_private.h |  9 +++++--
> >> 2 files changed, 43 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> >> index 07bc5cb3a430..d15119409a25 100644
> >> --- a/xen/arch/arm/tee/ffa_notif.c
> >> +++ b/xen/arch/arm/tee/ffa_notif.c
> >> @@ -94,8 +94,15 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
> >>
> >>     notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
> >>     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.hyp_pending )
> >> +            notif_pending = true;
> >> +        spin_unlock(&ctx->notif.notif_lock);
> >
> > Isn't this a functional change? Before this patch, we didn't consider
> > ctx->notif.buff_full_pending here. Am I missing something?
>
> We did consider it implicitly through vm_pending.
>
> This patch makes that cleaner by using hyp_pending for the Hypervisor
> framework notification itself. Previously, RX-buffer-full was made visible
> indirectly via vm_pending, and FFA_NOTIFICATION_INFO_GET
> cleared that summary state.
>
> As a result, the guest-visible pending indication could be lost before
> the Hypervisor notification was actually retrieved with
> FFA_NOTIFICATION_GET.
>
> With this change, the pending state is tracked in hyp_pending and is only
> cleared when the Hypervisor notifications are retrieved through
> FFA_NOTIFICATION_GET.
>
> I will reword the commit message to make that clearer.

Thanks

>
> >
> >> +    }
> >> +
> >>     if ( notif_pending )
> >>     {
> >>         /* A pending global notification for the guest */
> >> @@ -174,12 +181,17 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
> >>             w6 = resp.a6;
> >>     }
> >>
> >> -    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) &&
> >> -          flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
> >> -          test_and_clear_bool(ctx->notif.buff_full_pending) )
> >> +    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
> >>     {
> >> -        ACCESS_ONCE(ctx->notif.vm_pending) = false;
> >> -        w7 = FFA_NOTIF_RX_BUFFER_FULL;
> >> +        spin_lock(&ctx->notif.notif_lock);
> >> +
> >> +        if ( (flags & FFA_NOTIF_FLAG_BITMAP_HYP) && ctx->notif.hyp_pending )
> >> +        {
> >> +            w7 = ctx->notif.hyp_pending;
> >> +            ctx->notif.hyp_pending = 0;
> >> +        }
> >> +
> >> +        spin_unlock(&ctx->notif.notif_lock);
> >>     }
> >>
> >>     ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
> >> @@ -207,12 +219,17 @@ int32_t ffa_handle_notification_set(struct cpu_user_regs *regs)
> >> void ffa_raise_rx_buffer_full(struct domain *d)
> >> {
> >>     struct ffa_ctx *ctx = d->arch.tee;
> >> +    uint32_t prev_bitmap;
> >>
> >>     if ( !ctx )
> >>         return;
> >>
> >> -    ACCESS_ONCE(ctx->notif.buff_full_pending) = true;
> >> -    if ( !test_and_set_bool(ctx->notif.vm_pending) )
> >> +    spin_lock(&ctx->notif.notif_lock);
> >> +    prev_bitmap = ctx->notif.hyp_pending;
> >> +    ctx->notif.hyp_pending |= FFA_NOTIF_RX_BUFFER_FULL;
> >> +    spin_unlock(&ctx->notif.notif_lock);
> >> +
> >> +    if ( !(prev_bitmap & FFA_NOTIF_RX_BUFFER_FULL) )
> >
> > Do we need to check for FFA_NOTIF_RX_BUFFER_FULL? Isn't !prev_bitmap
> > more accurate, if any other bit would ever be used in the bitmap?
>
>   I would keep the bit-specific check here, if that is OK with you.
>
>  This function is about raising the RX buffer full notification,
>  so I think it is clearer to check whether that bit was newly
>  pended rather than whether the whole Hypervisor bitmap
>  was previously empty.
>
> Using !prev_bitmap would also make the condition depend on
> unrelated Hypervisor notifications being pending.
>
> Tell me if you are ok with that.

I see your point, it's fine.

Cheers,
Jens

>
> Cheers
> Bertrand
>

Hi Jens,

> On 23 Apr 2026, at 11:49, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> 
> Hi Bertrand,
> 
> On Thu, Apr 23, 2026 at 9:30 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>> 
>> Hi Jens,
>> 
>> Thanks a lot for the review.
>> 
>>> On 22 Apr 2026, at 11:34, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>> 
>>> Hi Bertrand,
>>> 
>>> On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
>>> <bertrand.marquis@arm.com> wrote:
>>>> 
>>>> Hypervisor notifications are currently tracked with a dedicated
>>>> buff_full_pending boolean. That state only represents a single HYP
>>>> notification bit and keeps HYP bitmap handling tied to single-purpose
>>>> bookkeeping.
>>>> 
>>>> Replace the boolean with a hypervisor notification bitmap protected by
>>>> notif_lock. INFO_GET reports pending when the bitmap is non-zero, GET
>>>> returns and clears the HYP bitmap under the lock, and RX-buffer-full
>>>> sets FFA_NOTIF_RX_BUFFER_FULL in the bitmap instead of updating
>>>> separate state.
>>>> 
>>>> Initialize and clear the bitmap during domain lifecycle handling, and
>>>> use ctx->ffa_id for bitmap create and destroy so the notification state
>>>> stays tied to the cached FF-A endpoint ID.
>>>> 
>>>> No functional changes.
>>>> 
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> ---
>>>> xen/arch/arm/tee/ffa_notif.c   | 46 ++++++++++++++++++++++++++--------
>>>> xen/arch/arm/tee/ffa_private.h |  9 +++++--
>>>> 2 files changed, 43 insertions(+), 12 deletions(-)
>>>> 
>>>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>>>> index 07bc5cb3a430..d15119409a25 100644
>>>> --- a/xen/arch/arm/tee/ffa_notif.c
>>>> +++ b/xen/arch/arm/tee/ffa_notif.c
>>>> @@ -94,8 +94,15 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
>>>> 
>>>>    notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
>>>>    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.hyp_pending )
>>>> +            notif_pending = true;
>>>> +        spin_unlock(&ctx->notif.notif_lock);
>>> 
>>> Isn't this a functional change? Before this patch, we didn't consider
>>> ctx->notif.buff_full_pending here. Am I missing something?
>> 
>> We did consider it implicitly through vm_pending.
>> 
>> This patch makes that cleaner by using hyp_pending for the Hypervisor
>> framework notification itself. Previously, RX-buffer-full was made visible
>> indirectly via vm_pending, and FFA_NOTIFICATION_INFO_GET
>> cleared that summary state.
>> 
>> As a result, the guest-visible pending indication could be lost before
>> the Hypervisor notification was actually retrieved with
>> FFA_NOTIFICATION_GET.
>> 
>> With this change, the pending state is tracked in hyp_pending and is only
>> cleared when the Hypervisor notifications are retrieved through
>> FFA_NOTIFICATION_GET.
>> 
>> I will reword the commit message to make that clearer.
> 
> Thanks
> 
>> 
>>> 
>>>> +    }
>>>> +
>>>>    if ( notif_pending )
>>>>    {
>>>>        /* A pending global notification for the guest */
>>>> @@ -174,12 +181,17 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs)
>>>>            w6 = resp.a6;
>>>>    }
>>>> 
>>>> -    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) &&
>>>> -          flags & FFA_NOTIF_FLAG_BITMAP_HYP &&
>>>> -          test_and_clear_bool(ctx->notif.buff_full_pending) )
>>>> +    if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>>>>    {
>>>> -        ACCESS_ONCE(ctx->notif.vm_pending) = false;
>>>> -        w7 = FFA_NOTIF_RX_BUFFER_FULL;
>>>> +        spin_lock(&ctx->notif.notif_lock);
>>>> +
>>>> +        if ( (flags & FFA_NOTIF_FLAG_BITMAP_HYP) && ctx->notif.hyp_pending )
>>>> +        {
>>>> +            w7 = ctx->notif.hyp_pending;
>>>> +            ctx->notif.hyp_pending = 0;
>>>> +        }
>>>> +
>>>> +        spin_unlock(&ctx->notif.notif_lock);
>>>>    }
>>>> 
>>>>    ffa_set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, w4, w5, w6, w7);
>>>> @@ -207,12 +219,17 @@ int32_t ffa_handle_notification_set(struct cpu_user_regs *regs)
>>>> void ffa_raise_rx_buffer_full(struct domain *d)
>>>> {
>>>>    struct ffa_ctx *ctx = d->arch.tee;
>>>> +    uint32_t prev_bitmap;
>>>> 
>>>>    if ( !ctx )
>>>>        return;
>>>> 
>>>> -    ACCESS_ONCE(ctx->notif.buff_full_pending) = true;
>>>> -    if ( !test_and_set_bool(ctx->notif.vm_pending) )
>>>> +    spin_lock(&ctx->notif.notif_lock);
>>>> +    prev_bitmap = ctx->notif.hyp_pending;
>>>> +    ctx->notif.hyp_pending |= FFA_NOTIF_RX_BUFFER_FULL;
>>>> +    spin_unlock(&ctx->notif.notif_lock);
>>>> +
>>>> +    if ( !(prev_bitmap & FFA_NOTIF_RX_BUFFER_FULL) )
>>> 
>>> Do we need to check for FFA_NOTIF_RX_BUFFER_FULL? Isn't !prev_bitmap
>>> more accurate, if any other bit would ever be used in the bitmap?
>> 
>>  I would keep the bit-specific check here, if that is OK with you.
>> 
>> This function is about raising the RX buffer full notification,
>> so I think it is clearer to check whether that bit was newly
>> pended rather than whether the whole Hypervisor bitmap
>> was previously empty.
>> 
>> Using !prev_bitmap would also make the condition depend on
>> unrelated Hypervisor notifications being pending.
>> 
>> Tell me if you are ok with that.
> 
> I see your point, it's fine.

In fact while working on this i ended up in something a bit more generic
due to your patch 6 findings and at the end i do not raise depending on
prev_bitmap anymore but just on if a notification was raised already or
not since something was changed which does not depend on hyp_pending
anymore.

I am working on v2, might be more clear then :-)

Cheers
Bertrand

> 
> Cheers,
> Jens
> 
>> 
>> Cheers
>> Bertrand