Re: [PATCH 4/6] xen/arm: ffa: Preserve secure notification state when polling SPMC

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

HI Jens,

> On 22 Apr 2026, at 14:45, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
> <bertrand.marquis@xxxxxxx> wrote:
>> 
>> Secure pending state is latched when the SPMC raises the schedule
>> receiver interrupt, but Xen currently clears that latch too aggressively.
>> Guest FFA_NOTIFICATION_INFO_GET consumes secure_pending even though it
>> only reports pending state, and secure FFA_NOTIFICATION_GET only clears
>> the latch when both SP and SPM bitmaps are requested together. This can
>> drop a pending indication before the receiver retrieves secure
>> notifications, or keep INFO_GET reporting stale secure pending state
>> after a successful GET.
>> 
>> Keep secure_pending as a latched indication until secure notifications
>> are actually retrieved. Guest FFA_NOTIFICATION_INFO_GET now reports the
>> latched state without clearing it, while a successful secure
>> FFA_NOTIFICATION_GET clears the latch regardless of which secure bitmap
>> flags were requested. Also protect secure_pending with notif_lock,
>> serialize SPMC INFO_GET polling behind notif_info_lock, and preserve the
>> caller-visible INFO_GET success width.
>> 
>> Functional impact: guest INFO_GET preserves the secure pending
>> indication until secure notifications are retrieved, and successful
>> secure GET clears the guest-visible pending latch.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> ---
>> xen/arch/arm/tee/ffa_notif.c | 54 +++++++++++++++++++++++-------------
>> 1 file changed, 35 insertions(+), 19 deletions(-)
>> 
>> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
>> index 491db3b04df5..fff00ca2baec 100644
>> --- a/xen/arch/arm/tee/ffa_notif.c
>> +++ b/xen/arch/arm/tee/ffa_notif.c
>> @@ -18,6 +18,7 @@
>> 
>> static bool __ro_after_init fw_notif_enabled;
>> static unsigned int __ro_after_init notif_sri_irq;
>> +static DEFINE_SPINLOCK(notif_info_lock);
>> 
>> static void inject_notif_pending(struct domain *d)
>> {
>> @@ -109,6 +110,7 @@ void ffa_handle_notification_info_get(struct 
>> cpu_user_regs *regs)
>> {
>>     struct domain *d = current->domain;
>>     struct ffa_ctx *ctx = d->arch.tee;
>> +    uint32_t fid = get_user_reg(regs, 0);
>>     bool notif_pending;
>> 
>>     if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled )
>> @@ -117,7 +119,10 @@ void ffa_handle_notification_info_get(struct 
>> cpu_user_regs *regs)
>>         return;
>>     }
>> 
>> -    notif_pending = test_and_clear_bool(ctx->notif.secure_pending);
>> +    spin_lock(&ctx->notif.notif_lock);
>> +    notif_pending = ctx->notif.secure_pending;
>> +    spin_unlock(&ctx->notif.notif_lock);
>> +
>>     if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>>     {
>>         notif_pending |= test_and_clear_bool(ctx->notif.vm_pending);
>> @@ -131,7 +136,9 @@ void ffa_handle_notification_info_get(struct 
>> cpu_user_regs *regs)
>>     if ( notif_pending )
>>     {
>>         /* A pending global notification for the guest */
>> -        ffa_set_regs(regs, FFA_SUCCESS_64, 0,
>> +        ffa_set_regs(regs,
>> +                     smccc_is_conv_64(fid) ? FFA_SUCCESS_64 : 
>> FFA_SUCCESS_32,
>> +                     0,
>>                      1U << FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT, 
>> ffa_get_vm_id(d),
>>                      0, 0, 0, 0);
>>     }
>> @@ -154,6 +161,8 @@ void ffa_handle_notification_get(struct cpu_user_regs 
>> *regs)
>>     uint32_t w5 = 0;
>>     uint32_t w6 = 0;
>>     uint32_t w7 = 0;
>> +    uint32_t secure_flags = flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
>> +                                      FFA_NOTIF_FLAG_BITMAP_SPM );
>> 
>>     if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled )
>>     {
>> @@ -173,27 +182,16 @@ void ffa_handle_notification_get(struct cpu_user_regs 
>> *regs)
>>         return;
>>     }
>> 
>> -    if ( fw_notif_enabled && (flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
>> -                                        FFA_NOTIF_FLAG_BITMAP_SPM )) )
>> +    if ( fw_notif_enabled && secure_flags )
>>     {
>>         struct arm_smccc_1_2_regs arg = {
>>             .a0 = FFA_NOTIFICATION_GET,
>>             .a1 = recv,
>> -            .a2 = flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
>> -                            FFA_NOTIF_FLAG_BITMAP_SPM ),
>> +            .a2 = secure_flags,
>>         };
>>         struct arm_smccc_1_2_regs resp;
>>         int32_t e;
>> 
>> -        /*
>> -         * Clear secure pending if both FFA_NOTIF_FLAG_BITMAP_SP and
>> -         * FFA_NOTIF_FLAG_BITMAP_SPM are set since secure world can't have
>> -         * any more pending notifications.
>> -         */
>> -        if ( ( flags  & FFA_NOTIF_FLAG_BITMAP_SP ) &&
>> -             ( flags & FFA_NOTIF_FLAG_BITMAP_SPM ) )
>> -            ACCESS_ONCE(ctx->notif.secure_pending) = false;
>> -
>>         arm_smccc_1_2_smc(&arg, &resp);
>>         e = ffa_get_ret_code(&resp);
>>         if ( e )
>> @@ -210,6 +208,10 @@ void ffa_handle_notification_get(struct cpu_user_regs 
>> *regs)
>> 
>>         if ( flags & FFA_NOTIF_FLAG_BITMAP_SPM )
>>             w6 = resp.a6;
>> +
>> +        spin_lock(&ctx->notif.notif_lock);
>> +        ctx->notif.secure_pending = false;
>> +        spin_unlock(&ctx->notif.notif_lock);
>>     }
>> 
>>     if ( IS_ENABLED(CONFIG_FFA_VM_TO_VM) )
>> @@ -354,7 +356,10 @@ static void notif_vm_pend_intr(uint16_t vm_id)
>>      * guarantees that the data structure isn't freed while we're accessing
>>      * it.
>>      */
>> -    ACCESS_ONCE(ctx->notif.secure_pending) = true;
>> +    spin_lock(&ctx->notif.notif_lock);
>> +    ctx->notif.secure_pending = true;
>> +    spin_unlock(&ctx->notif.notif_lock);
>> +
>>     inject_notif_pending(d);
>> 
>> out_unlock:
>> @@ -373,11 +378,18 @@ static void notif_sri_action(void *unused)
>>     unsigned int n;
>>     int32_t res;
>> 
>> -    do {
>> +    if ( !fw_notif_enabled )
>> +        return;
> 
> Can this ever happen? Am I missing something?

No in fact it cannot you are right, we will never get
an SRI if firmware notifications are not enabled so
I will remove that.

Cheers
Bertrand