[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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;
+
+    spin_lock(&notif_info_lock);
+
+    do
+    {
         arm_smccc_1_2_smc(&arg, &resp);
         res = ffa_get_ret_code(&resp);
         if ( res )
         {
+            spin_unlock(&notif_info_lock);
             if ( res != FFA_RET_NO_DATA && printk_ratelimit() )
                 printk(XENLOG_WARNING
                        "ffa: notification info get failed: error %d\n", res);
@@ -391,7 +403,7 @@ static void notif_sri_action(void *unused)
         id_pos = 0;
         for ( n = 0; n < list_count; n++ )
         {
-            unsigned int count = ((ids_count >> 2 * n) & 0x3) + 1;
+            unsigned int count = ((ids_count >> (2 * n)) & 0x3) + 1;
             uint16_t vm_id = get_id_from_resp(&resp, id_pos);
 
             notif_vm_pend_intr(vm_id);
@@ -399,7 +411,9 @@ static void notif_sri_action(void *unused)
             id_pos += count;
         }
 
-    } while (resp.a2 & FFA_NOTIF_INFO_GET_MORE_FLAG);
+    } while ( resp.a2 & FFA_NOTIF_INFO_GET_MORE_FLAG );
+
+    spin_unlock(&notif_info_lock);
 }
 
 static DECLARE_TASKLET(notif_sri_tasklet, notif_sri_action, NULL);
@@ -486,6 +500,7 @@ int ffa_notif_domain_init(struct domain *d)
     int32_t res;
 
     spin_lock_init(&ctx->notif.notif_lock);
+    ctx->notif.secure_pending = false;
     ctx->notif.hyp_pending = 0;
 
     if ( fw_notif_enabled )
@@ -503,6 +518,7 @@ void ffa_notif_domain_destroy(struct domain *d)
     struct ffa_ctx *ctx = d->arch.tee;
 
     spin_lock(&ctx->notif.notif_lock);
+    ctx->notif.secure_pending = false;
     ctx->notif.hyp_pending = 0;
     spin_unlock(&ctx->notif.notif_lock);
 
-- 
2.53.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.