broadband-modem-mbim: ignore SIM swap events during a slot switch action

When we explicitly request to perform a SIM slot switch operation, we
should ignore the 'slot info status' and 'subscriber ready status'
indications that report an active SIM slot change, or otherwise they'll
end up interfering with the correct reporting of the slot switch
operation.

We could have attempted to complete the slot switch operation early
using the indications, but that solution would have forced us to
assume that theinfo in the indications is the one we had requested,
which is not easy because the 'slot info status' indication is
reported once for each slot, so a single SIM slot switch operation
triggers multiple 'slot info status' indications, as well as
additional 'subscriber ready status' indications reporting the same.

Fixes suggested by Pavan Holla <pholla@chromium.org> and tested by
Jinjian Song <Jinjian.Song@fibocom.com>.

Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/550

1 files changed, 40 insertions, 11 deletions

diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
index 29b845b0..b3137089 100644
--- a/src/mm-broadband-modem-mbim.c
+++ b/src/mm-broadband-modem-mbim.c
@@ -170,8 +170,9 @@ struct _MMBroadbandModemMbimPrivate {
     gboolean intel_firmware_update_unsupported;
 
     /* Multi-SIM support */
-    guint32 executor_index;
-    guint active_slot_index;
+    guint32  executor_index;
+    guint    active_slot_index;
+    gboolean pending_sim_slot_switch_action;
 
     MMUnlockRetries *unlock_retries;
 };
@@ -4671,6 +4672,7 @@ basic_connect_notification_subscriber_ready_status (MMBroadbandModemMbim *self,
     MbimSubscriberReadyState ready_state;
     g_auto(GStrv)            telephone_numbers = NULL;
     g_autoptr(GError)        error = NULL;
+    gboolean                 active_sim_event = FALSE;
 
     if (mbim_device_check_ms_mbimex_version (device, 3, 0)) {
         if (!mbim_message_ms_basic_connect_v3_subscriber_ready_status_notification_parse (
@@ -4712,7 +4714,7 @@ basic_connect_notification_subscriber_ready_status (MMBroadbandModemMbim *self,
          ready_state != MBIM_SUBSCRIBER_READY_STATE_NO_ESIM_PROFILE)) {
         /* eSIM profiles have been added or removed, re-probe to ensure correct interfaces are exposed */
         mm_obj_dbg (self, "eSIM profile updates detected");
-        mm_iface_modem_process_sim_event (MM_IFACE_MODEM (self));
+        active_sim_event = TRUE;
     }
 
     if ((self->priv->last_ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
@@ -4721,10 +4723,17 @@ basic_connect_notification_subscriber_ready_status (MMBroadbandModemMbim *self,
          ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)) {
         /* SIM has been removed or reinserted, re-probe to ensure correct interfaces are exposed */
         mm_obj_dbg (self, "SIM hot swap detected");
-        mm_iface_modem_process_sim_event (MM_IFACE_MODEM (self));
+        active_sim_event = TRUE;
     }
 
     self->priv->last_ready_state = ready_state;
+
+    if (active_sim_event) {
+        if (self->priv->pending_sim_slot_switch_action)
+            mm_obj_dbg (self, "ignoring slot status change");
+        else
+            mm_iface_modem_process_sim_event (MM_IFACE_MODEM (self));
+    }
 }
 
 typedef struct {
@@ -5227,19 +5236,26 @@ ms_basic_connect_extensions_notification_slot_info_status (MMBroadbandModemMbim
         return;
     }
 
-    if (self->priv->active_slot_index == slot_index + 1) {
-        /* Major SIM event on the active slot, will request reprobing the
-         * modem from scratch. */
-        mm_iface_modem_process_sim_event (MM_IFACE_MODEM (self));
-    } else {
+    if (self->priv->pending_sim_slot_switch_action) {
+        mm_obj_dbg (self, "ignoring slot status change in SIM slot %d: %s", slot_index + 1, mbim_uicc_slot_state_get_string (slot_state));
+        return;
+    }
+
+    if (self->priv->active_slot_index != (slot_index + 1)) {
         /* Modifies SIM object at the given slot based on the reported state,
          * when the slot is not the active one. */
         g_autoptr(MMBaseSim) sim = NULL;
 
-        mm_obj_dbg (self, "Updating inactive sim at slot %d", slot_index + 1);
+        mm_obj_dbg (self, "processing slot status change in non-active SIM slot %d: %s", slot_index + 1, mbim_uicc_slot_state_get_string (slot_state));
         sim = create_sim_from_slot_state (self, FALSE, slot_index, slot_state);
         mm_iface_modem_modify_sim (MM_IFACE_MODEM (self), slot_index, sim);
+        return;
     }
+
+    /* Major SIM event on the active slot, will request reprobing the
+     * modem from scratch. */
+    mm_obj_dbg (self, "processing slot status change in active SIM slot %d: %s", slot_index + 1, mbim_uicc_slot_state_get_string (slot_state));
+    mm_iface_modem_process_sim_event (MM_IFACE_MODEM (self));
 }
 
 static void
@@ -8721,6 +8737,9 @@ set_device_slot_mappings_ready (MbimDevice   *device,
 
     self = g_task_get_source_object (task);
 
+    g_assert (self->priv->pending_sim_slot_switch_action);
+    self->priv->pending_sim_slot_switch_action = FALSE;
+
     /* the slot index in MM starts at 1 */
     slot_number = GPOINTER_TO_UINT (g_task_get_task_data (task)) - 1;
 
@@ -8752,7 +8771,7 @@ set_device_slot_mappings_ready (MbimDevice   *device,
     }
 
     g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_NOT_FOUND,
-                                 "Can't find executor index '%u'", self->priv->executor_index);
+                             "Can't find executor index '%u'", self->priv->executor_index);
     g_object_unref (task);
 }
 
@@ -8802,6 +8821,16 @@ before_set_query_device_slot_mappings_ready (MbimDevice   *device,
         }
     }
 
+    /* Flag a pending SIM slot switch operation, so that we can ignore slot state updates
+     * during the process. */
+    if (self->priv->pending_sim_slot_switch_action) {
+        g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_IN_PROGRESS,
+                                 "there is already an ongoing SIM slot switch operation");
+        g_object_unref (task);
+        return;
+    }
+    self->priv->pending_sim_slot_switch_action = TRUE;
+
     for (i = 0; i < map_count; i++) {
         if (i == self->priv->executor_index)
             slot_mappings[i]->slot = slot_number;