fix(thunderbolt): add nvInvalidateDeviceReferences to fix module unload crash

Author: bdandy

src/nvidia-modeset/include/nvkms-private.h

@@ -35,6 +35,8 @@ struct NvKmsPerOpenDev *nvAllocPerOpenDev(struct NvKmsPerOpen *pOpen,
 
 void nvRevokeDevice(NVDevEvoPtr pDevEvo);
 
+void nvInvalidateDeviceReferences(NVDevEvoPtr pDevEvo);
+
 void nvFreePerOpenDev(struct NvKmsPerOpen *pOpen,
                       struct NvKmsPerOpenDev *pOpenDev);
 
@@ -73,8 +75,6 @@ const NVEvoApiHandlesRec *nvGetSurfaceHandlesFromOpenDevConst(
 
 void nvKmsServiceNonStallInterrupt(void *dataPtr, NvU32 dataU32);
 
-void nvKmsReinitializeGlobalClient(void);
-
 #ifdef __cplusplus
 };
 #endif

src/nvidia-modeset/kapi/src/nvkms-kapi.c

@@ -633,8 +633,13 @@ static void FreeDevice(struct NvKmsKapiDevice *device)
  * faults or hangs when trying to access unmapped GPU memory.
  *
  * We only:
- * 1. Release the GPU reference count (nvkms_close_gpu)
- * 2. Free kernel memory resources (semaphore, device struct)
+ * 1. Mark GPU as lost to prevent hardware access
+ * 2. Release the GPU reference count (nvkms_close_gpu)
+ * 3. Clean up kernel memory resources (handle allocator, semaphore, device struct)
+ *
+ * We skip:
+ * - KmsFreeDevice() - would call nvkms_ioctl_from_kapi() which accesses hardware
+ * - RmFreeDevice() - would call nvRmApiFree() which accesses hardware
  *
  * The hardware resources will be cleaned up when the GPU is physically
  * removed from the system.
@@ -653,17 +658,39 @@ static void FreeDeviceForSurpriseRemoval(struct NvKmsKapiDevice *device)
     nvkms_gpu_lost(device->gpuId);
 
     /*
-     * Skip KmsFreeDevice() and RmFreeDevice() - these try to access
-     * GPU hardware via ioctls and RM API calls, which will crash
-     * since the GPU memory is unmapped after surprise removal.
+     * Clear device handles to prevent any stale references.
+     * Don't call nvRmApiFree() as that would access hardware.
+     */
+    device->hKmsDevice = 0;
+    device->hKmsDisp = 0;
+    device->hRmSubDevice = 0;
+    device->hRmDevice = 0;
+    device->hRmClient = 0;
+    device->smgGpuInstSubscriptionHdl = 0;
+    device->smgComputeInstSubscriptionHdl = 0;
+
+    /*
+     * Tear down the handle allocator - this only frees kernel memory
+     * (bitmaps), no hardware access.
+     */
+    nvTearDownUnixRmHandleAllocator(&device->handleAllocator);
+    device->deviceInstance = 0;
+
+    /*
+     * Clear pKmsOpen - we can't call nvkms_close_from_kapi() as that
+     * would try to access hardware through nvKmsClose(). The popen
+     * structure will be leaked, but this only happens during surprise
+     * removal which is an abnormal condition.
      */
+    device->pKmsOpen = NULL;
 
     /* Lower the reference count of gpu - this is safe, no hardware access */
     nvkms_close_gpu(device->gpuId);
 
     /* Free kernel memory resources */
     if (device->pSema != NULL) {
         nvkms_sema_free(device->pSema);
+        device->pSema = NULL;
     }
 
     nvKmsKapiFree(device);

src/nvidia-modeset/src/nvkms-evo.c

@@ -9050,14 +9050,21 @@ NvBool nvFreeDevEvo(NVDevEvoPtr pDevEvo)
     /*
      * If the GPU was lost (surprise removal), skip all hardware-related
      * cleanup. Just free software resources and remove from device list.
+     *
+     * NOTE: We do NOT call nvFreePerOpenDev() here because the pNvKmsOpenDev
+     * is still in the global open list. It will be properly cleaned up during
+     * module unload when nvKmsClose iterates through all open handles.
+     * Calling nvFreePerOpenDev here would cause a double-free crash.
      */
     if (pDevEvo->gpuLost) {
         nvEvoLogDev(pDevEvo, EVO_LOG_INFO,
                     "Freeing device after GPU lost, skipping hardware cleanup");
 
-        /* Still need to free the per-open data (software resources only) */
-        nvFreePerOpenDev(nvEvoGlobal.nvKmsPerOpen, pDevEvo->pNvKmsOpenDev);
-        pDevEvo->pNvKmsOpenDev = NULL;
+        /*
+         * Invalidate all pOpenDev references to this device before freeing it.
+         * This ensures nvKmsClose won't try to access the freed pDevEvo.
+         */
+        nvInvalidateDeviceReferences(pDevEvo);
 
         goto free_software_resources;
     }
@@ -9125,12 +9132,16 @@ NvBool nvFreeDevEvo(NVDevEvoPtr pDevEvo)
         nvFree(pDevEvo);
 
         /*
-         * If the GPU was lost and the device list is now empty, reinitialize
-         * the global RM client so that newly attached GPUs can be used.
+         * NOTE: We intentionally do NOT call nvKmsReinitializeGlobalClient()
+         * here even if the device list is empty. The global client handle
+         * is still referenced by open handles (pNvKmsOpenDev) that will be
+         * cleaned up during module unload by nvKmsClose(). Reinitializing
+         * the client here would corrupt those handles and cause a crash.
+         *
+         * If the user reconnects the GPU before unloading the module, it will
+         * work because AllocDevice checks for stale gpuLost devices and cleans
+         * them up. The global client doesn't need to be reinitialized for that.
          */
-        if (wasGpuLost && nvListIsEmpty(&nvEvoGlobal.devList)) {
-            nvKmsReinitializeGlobalClient();
-        }
 
         return TRUE;
     }

src/nvidia-modeset/src/nvkms.c

@@ -1635,6 +1635,16 @@ static void DisableRemainingVblankSemControls(
 static void FreeDeviceReference(struct NvKmsPerOpen *pOpen,
                                 struct NvKmsPerOpenDev *pOpenDev)
 {
+    /*
+     * If pDevEvo is NULL, the device was already freed due to GPU loss
+     * (surprise removal). In this case, skip all hardware-related cleanup
+     * and just free the software structures.
+     */
+    if (pOpenDev->pDevEvo == NULL) {
+        nvFreePerOpenDev(pOpen, pOpenDev);
+        return;
+    }
+
     /* Disable all client-owned vblank sync objects that still exist. */
     DisableRemainingVblankSyncObjects(pOpen, pOpenDev);
 
@@ -5327,6 +5337,31 @@ void nvRevokeDevice(NVDevEvoPtr pDevEvo)
     }
 }
 
+/*
+ * Invalidate all pOpenDev references to a device.
+ * Called when GPU is lost to ensure nvKmsClose doesn't access freed pDevEvo.
+ * This sets pOpenDev->pDevEvo to NULL for all open handles.
+ */
+void nvInvalidateDeviceReferences(NVDevEvoPtr pDevEvo)
+{
+    struct NvKmsPerOpen *pOpen;
+    struct NvKmsPerOpenDev *pOpenDev;
+    NvKmsGenericHandle dev;
+
+    if (pDevEvo == NULL) {
+        return;
+    }
+
+    nvListForEachEntry(pOpen, &perOpenIoctlList, perOpenIoctlListEntry) {
+        FOR_ALL_POINTERS_IN_EVO_API_HANDLES(&pOpen->ioctl.devHandles,
+                                            pOpenDev, dev) {
+            if (pOpenDev->pDevEvo == pDevEvo) {
+                pOpenDev->pDevEvo = NULL;
+            }
+        }
+    }
+}
+
 /*!
  * Open callback.
  *
@@ -6318,44 +6353,6 @@ static void FreeGlobalState(void)
     nvClearDpyOverrides();
 }
 
-/*
- * Reinitialize the global RM client after a GPU surprise removal.
- * When a GPU is removed, the RM client handle may become invalid.
- * This function re-creates the client handle so that newly attached
- * GPUs can be used.
- */
-void nvKmsReinitializeGlobalClient(void)
-{
-    NvU32 ret;
-
-    /*
-     * First, try to free the old client handle. This may fail if RM
-     * already invalidated it, but that's OK.
-     */
-    if (nvEvoGlobal.clientHandle != 0) {
-        nvRmApiFree(nvEvoGlobal.clientHandle, nvEvoGlobal.clientHandle,
-                    nvEvoGlobal.clientHandle);
-        nvEvoGlobal.clientHandle = 0;
-    }
-
-    /* Allocate a new root client */
-    ret = nvRmApiAlloc(NV01_NULL_OBJECT,
-                       NV01_NULL_OBJECT,
-                       NV01_NULL_OBJECT,
-                       NV01_ROOT,
-                       &nvEvoGlobal.clientHandle);
-
-    if (ret != NVOS_STATUS_SUCCESS) {
-        nvEvoLog(EVO_LOG_ERROR, "Failed to reinitialize client after GPU removal");
-        return;
-    }
-
-    /* Update the RM context */
-    nvEvoGlobal.rmSmgContext.clientHandle = nvEvoGlobal.clientHandle;
-
-    nvEvoLog(EVO_LOG_INFO, "Reinitialized global client after GPU surprise removal");
-}
-
 /*
  * Wrappers to help SMG access NvKmsKAPI's RM context.
  */