Changeset 84689 in vbox

Timestamp:

Jun 5, 2020 9:07:54 AM (5 years ago)

Author:

vboxsync

Message:

Devices/Serial/UartCore: Revert part of r137918 and switch the receive FIFO timeout timer back to just virtual because virtual sync timers, hoping that we don't require the precise timing in that path (OpenIndiana 2020 boot menu still works correctly). Also leave the device critical section before calling into the lower drivers pfnDataAvailWrNotify callback to avoid a possible deadlock

File:

• trunk/src/VBox/Devices/Serial/UartCore.cpp (modified) (9 diffs)

trunk/src/VBox/Devices/Serial/UartCore.cpp

@@ -813 +813 @@
 {
     int rc = VINF_SUCCESS;
+#ifdef IN_RING3
+    bool fNotifyDrv = false;
+#endif
 
     if (pThis->uRegFcr & UART_REG_FCR_FIFO_EN)
@@ -831 +834 @@
         uartIrqUpdate(pDevIns, pThis, pThisCC);
         if (uartFifoUsedGet(&pThis->FifoXmit) == 1)
-        {
-            if (   pThisCC->pDrvSerial
-                && !(pThis->uRegMcr & UART_REG_MCR_LOOP))
-            {
-                int rc2 = pThisCC->pDrvSerial->pfnDataAvailWrNotify(pThisCC->pDrvSerial);
-                if (RT_FAILURE(rc2))
-                    LogRelMax(10, ("Serial#%d: Failed to send data with %Rrc\n", pDevIns->iInstance, rc2));
-            }
-            else
-                PDMDevHlpTimerSetRelative(pDevIns, pThis->hTimerTxUnconnected, pThis->cSymbolXferTicks, NULL);
-        }
+            fNotifyDrv = true;
 #endif
     }
@@ -856 +849 @@
             pThis->fThreEmptyPending = false;
             uartIrqUpdate(pDevIns, pThis, pThisCC);
-            if (   pThisCC->pDrvSerial
-                && !(pThis->uRegMcr & UART_REG_MCR_LOOP))
-            {
-                int rc2 = pThisCC->pDrvSerial->pfnDataAvailWrNotify(pThisCC->pDrvSerial);
-                if (RT_FAILURE(rc2))
-                    LogRelMax(10, ("Serial#%d: Failed to send data with %Rrc\n", pDevIns->iInstance, rc2));
-            }
-            else
-                PDMDevHlpTimerSetRelative(pDevIns, pThis->hTimerTxUnconnected, pThis->cSymbolXferTicks, NULL);
+            fNotifyDrv = true;
 #endif
         }
@@ -870 +855 @@
             pThis->uRegThr = bVal;
     }
+
+#ifdef IN_RING3
+    if (fNotifyDrv)
+    {
+        if (   pThisCC->pDrvSerial
+            && !(pThis->uRegMcr & UART_REG_MCR_LOOP))
+        {
+            /* Leave the device critical section before calling into the lower driver. */
+            PDMDevHlpCritSectLeave(pDevIns, &pThis->CritSect);
+            int rc2 = pThisCC->pDrvSerial->pfnDataAvailWrNotify(pThisCC->pDrvSerial);
+            if (RT_FAILURE(rc2))
+                LogRelMax(10, ("Serial#%d: Failed to send data with %Rrc\n", pDevIns->iInstance, rc2));
+            PDMDevHlpCritSectEnter(pDevIns, &pThis->CritSect, VINF_SUCCESS);
+        }
+        else
+            PDMDevHlpTimerSetRelative(pDevIns, pThis->hTimerTxUnconnected, pThis->cSymbolXferTicks, NULL);
+    }
+#endif
 
     return rc;
@@ -1501 +1504 @@
     RT_NOREF(pTimer);
 
-    VBOXSTRICTRC rc1 = PDMDevHlpTimerLockClock2(pDevIns, pThis->hTimerRcvFifoTimeout, &pThis->CritSect,
-                                                VINF_SUCCESS /* must get it */);
-    AssertRCReturnVoid(VBOXSTRICTRC_VAL(rc1));
-
     if (pThis->FifoRecv.cbUsed < pThis->FifoRecv.cbItl)
     {
@@ -1510 +1509 @@
         uartIrqUpdate(pDevIns, pThis, pThisCC);
     }
-
-    PDMDevHlpTimerUnlockClock2(pDevIns, pThis->hTimerRcvFifoTimeout, &pThis->CritSect);
 }
 
@@ -1940 +1937 @@
             return VERR_PDM_MISSING_INTERFACE;
         }
+        PDMDevHlpCritSectEnter(pDevIns, &pThis->CritSect, VERR_IGNORED);
         uartR3XferReset(pDevIns, pThis, pThisCC);
+        PDMDevHlpCritSectLeave(pDevIns, &pThis->CritSect);
     }
     else if (rc == VERR_PDM_NO_ATTACHED_DRIVER)
@@ -1947 +1946 @@
         pThisCC->pDrvSerial = NULL;
         rc = VINF_SUCCESS;
+        PDMDevHlpCritSectEnter(pDevIns, &pThis->CritSect, VERR_IGNORED);
         uartR3XferReset(pDevIns, pThis, pThisCC);
+        PDMDevHlpCritSectLeave(pDevIns, &pThis->CritSect);
         LogRel(("Serial#%d: no unit\n", pDevIns->iInstance));
     }
@@ -2060 +2061 @@
      * Create the receive FIFO character timeout indicator timer.
      */
-    rc = PDMDevHlpTimerCreate(pDevIns, TMCLOCK_VIRTUAL_SYNC, uartR3RcvFifoTimeoutTimer, pThisCC,
+    rc = PDMDevHlpTimerCreate(pDevIns, TMCLOCK_VIRTUAL, uartR3RcvFifoTimeoutTimer, pThisCC,
                               TMTIMER_FLAGS_NO_CRIT_SECT, "UART Rcv FIFO Timer",
                               &pThis->hTimerRcvFifoTimeout);
+    AssertRCReturn(rc, rc);
+
+    rc = PDMDevHlpTimerSetCritSect(pDevIns, pThis->hTimerRcvFifoTimeout, &pThis->CritSect);
     AssertRCReturn(rc, rc);