Changeset 81660 in vbox for trunk/src/VBox/Devices/Storage

Timestamp:

Nov 4, 2019 9:46:54 PM (5 years ago)

Author:

vboxsync

Message:

Virtio_1_0,DevVirtioScsi: More common virtio code cleanups. bugref:9218 bugref:9440

File:

• trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (modified) (21 diffs)

trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp

@@ -81 +81 @@
 
 #define VIRTIOSCSI_REQ_QUEUE_CNT                    1           /**< T.B.D. Consider increasing                      */
-#define VIRTIOSCSI_QUEUE_CNT                        VIRTIOSCSI_REQ_QUEUE_CNT + 2
+#define VIRTIOSCSI_QUEUE_CNT                        (VIRTIOSCSI_REQ_QUEUE_CNT + 2)
 #define VIRTIOSCSI_MAX_LUN                          256         /**< VirtIO specification, section 5.6.4             */
 #define VIRTIOSCSI_MAX_COMMANDS_PER_LUN             128         /**< T.B.D. What is a good value for this?           */
@@ -394 +394 @@
 } VIRTIOSCSITARGET, *PVIRTIOSCSITARGET;
 
+
+/** Why we're quiescing. */
+typedef enum VIRTIOSCSIQUIESCINGFOR
+{
+    kvirtIoScsiQuiescingForInvalid = 0,
+    kvirtIoScsiQuiescingForReset,
+    kvirtIoScsiQuiescingForSuspend,
+    kvirtIoScsiQuiescingForPowerOff,
+    kvirtIoScsiQuiescingFor32BitHack = 0x7fffffff
+} VIRTIOSCSIQUIESCINGFOR;
+
+
 /**
  * PDM instance data (state) for VirtIO Host SCSI device
@@ -492 +504 @@
     /** True if in the process of quiescing I/O */
     uint32_t                        fQuiescing;
+    /** For which purpose we're quiescing. */
+    VIRTIOSCSIQUIESCINGFOR          enmQuiescingFor;
 
 } VIRTIOSCSI, *PVIRTIOSCSI;
@@ -663 +677 @@
 
     PVIRTIO_DESC_CHAIN_T pDescChain;
-    virtioQueueGet(&pThis->Virtio, EVENTQ_IDX, &pDescChain, true);
+    virtioR3QueueGet(&pThis->Virtio, EVENTQ_IDX, &pDescChain, true);
 
     RTSGBUF reqSegBuf;
@@ -669 +683 @@
     RTSgBufInit(&reqSegBuf, aReqSegs, RT_ELEMENTS(aReqSegs));
 
-    virtioQueuePut( &pThis->Virtio, EVENTQ_IDX, &reqSegBuf, pDescChain, true);
+    virtioR3QueuePut( &pThis->Virtio, EVENTQ_IDX, &reqSegBuf, pDescChain, true);
     virtioQueueSync(&pThis->Virtio, EVENTQ_IDX);
 
@@ -719 +733 @@
         pRespHdr->uResponse = VIRTIOSCSI_S_RESET;
 
-    virtioQueuePut(&pThis->Virtio, qIdx, &reqSegBuf, pDescChain, true /* fFence */);
+    virtioR3QueuePut(&pThis->Virtio, qIdx, &reqSegBuf, pDescChain, true /* fFence */);
     virtioQueueSync(&pThis->Virtio, qIdx);
 
@@ -875 +889 @@
 
 
-        virtioQueuePut(&pThis->Virtio, pReq->qIdx, &reqSegBuf, pReq->pDescChain, true /* fFence TBD */);
+        virtioR3QueuePut(&pThis->Virtio, pReq->qIdx, &reqSegBuf, pReq->pDescChain, true /* fFence TBD */);
         virtioQueueSync(&pThis->Virtio, pReq->qIdx);
 
@@ -1417 +1431 @@
 
     LogFunc(("Response code: %s\n", virtioGetReqRespText(bResponse)));
-    virtioQueuePut( &pThis->Virtio, qIdx, &reqSegBuf, pDescChain, true);
+    virtioR3QueuePut( &pThis->Virtio, qIdx, &reqSegBuf, pDescChain, true);
     virtioQueueSync(&pThis->Virtio, qIdx);
 
@@ -1476 +1490 @@
              Log6Func(("fetching next descriptor chain from %s\n", QUEUENAME(qIdx)));
              PVIRTIO_DESC_CHAIN_T pDescChain;
-             int rc = virtioQueueGet(&pThis->Virtio, qIdx, &pDescChain, true);
+             int rc = virtioR3QueueGet(&pThis->Virtio, qIdx, &pDescChain, true);
              if (rc == VERR_NOT_AVAILABLE)
              {
@@ -2056 +2070 @@
 static DECLCALLBACK(bool) virtioScsiR3DeviceQuiesced(PPDMDEVINS pDevIns)
 {
-    LogFunc(("Device I/O activity quiesced.\n"));
     PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
-
+    LogFunc(("Device I/O activity quiesced: enmQuiescingFor=%d\n", pThis->enmQuiescingFor));
+
+    if (pThis->enmQuiescingFor == kvirtIoScsiQuiescingForReset)
+        virtioR3PropagateResetNotification(&pThis->Virtio);
+    /** @todo r=bird: Do we need other notifications here for suspend and/or poweroff? */
+
+    pThis->enmQuiescingFor = kvirtIoScsiQuiescingForInvalid;
     pThis->fQuiescing = false;
-
     return true;
 }
@@ -2067 +2085 @@
  * Worker for virtioScsiR3Reset() and virtioScsiR3SuspendOrPowerOff().
  */
-static void virtioScsiR3QuiesceDevice(PPDMDEVINS pDevIns)
+static void virtioScsiR3QuiesceDevice(PPDMDEVINS pDevIns, VIRTIOSCSIQUIESCINGFOR enmQuiscingFor)
 {
     PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
@@ -2073 +2091 @@
     /* Prevent worker threads from removing/processing elements from virtq's */
     pThis->fQuiescing = true;
+    pThis->enmQuiescingFor = enmQuiscingFor;
 
     PDMDevHlpSetAsyncNotification(pDevIns, virtioScsiR3DeviceQuiesced);
@@ -2082 +2101 @@
 
 /**
- * @interface_method_impl{PDMDEVREGR3,pfnResume}
- */
-static DECLCALLBACK(void) virtioScsiR3Resume(PPDMDEVINS pDevIns)
-{
-    LogFunc(("\n"));
-
+ * Common worker for suspend and power off.
+ */
+static void virtioScsiR3SuspendOrPowerOff(PPDMDEVINS pDevIns, VIRTIOSCSIQUIESCINGFOR enmQuiscingFor)
+{
     PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
 
-    pThis->fQuiescing = false;
-
-    /* Wake worker threads flagged to skip pulling queue entries during quiesce
-     * to ensure they re-check their queues. Active request queues may already
-     * be awake due to new reqs coming in.
-     */
-    for (uint16_t qIdx = 0; qIdx < VIRTIOSCSI_REQ_QUEUE_CNT; qIdx++)
-    {
-        PVIRTIOSCSIWORKER pWorker = &pThis->aWorkers[qIdx];
-
-        if (ASMAtomicReadBool(&pWorker->fSleeping))
-        {
-            Log6Func(("waking %s worker.\n", QUEUENAME(qIdx)));
-            int rc = SUPSemEventSignal(pThis->pSupDrvSession, pWorker->hEvtProcess);
-            AssertRC(rc);
-        }
-    }
-
-    /* Ensure guest is working the queues too. */
-    virtioPropagateResumeNotification(&pThis->Virtio);
-}
-
-/**
- * @interface_method_impl{PDMDEVREGR3,pfnSuspend}
- */
-static DECLCALLBACK(void) virtioScsiR3SuspendOrPowerOff(PPDMDEVINS pDevIns)
-{
-    LogFunc(("\n"));
-
-    virtioScsiR3QuiesceDevice(pDevIns);
-
-    PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
+    virtioScsiR3QuiesceDevice(pDevIns, enmQuiscingFor);
+
 
     /* VM is halted, thus no new I/O being dumped into queues by the guest.
@@ -2139 +2126 @@
 
 /**
+ * @interface_method_impl{PDMDEVREGR3,pfnPowerOff}
+ */
+static DECLCALLBACK(void) virtioScsiR3PowerOff(PPDMDEVINS pDevIns)
+{
+    LogFunc(("\n"));
+    virtioScsiR3SuspendOrPowerOff(pDevIns, kvirtIoScsiQuiescingForPowerOff);
+}
+
+/**
+ * @interface_method_impl{PDMDEVREGR3,pfnSuspend}
+ */
+static DECLCALLBACK(void) virtioScsiR3Suspend(PPDMDEVINS pDevIns)
+{
+    LogFunc(("\n"));
+    virtioScsiR3SuspendOrPowerOff(pDevIns, kvirtIoScsiQuiescingForSuspend);
+}
+
+/**
+ * @interface_method_impl{PDMDEVREGR3,pfnResume}
+ */
+static DECLCALLBACK(void) virtioScsiR3Resume(PPDMDEVINS pDevIns)
+{
+    PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
+    LogFunc(("\n"));
+
+    pThis->fQuiescing = false;
+
+    /* Wake worker threads flagged to skip pulling queue entries during quiesce
+     * to ensure they re-check their queues. Active request queues may already
+     * be awake due to new reqs coming in.
+     */
+    for (uint16_t qIdx = 0; qIdx < VIRTIOSCSI_REQ_QUEUE_CNT; qIdx++)
+    {
+        PVIRTIOSCSIWORKER pWorker = &pThis->aWorkers[qIdx];
+
+        if (ASMAtomicReadBool(&pWorker->fSleeping))
+        {
+            Log6Func(("waking %s worker.\n", QUEUENAME(qIdx)));
+            int rc = SUPSemEventSignal(pThis->pSupDrvSession, pWorker->hEvtProcess);
+            AssertRC(rc);
+        }
+    }
+
+    /* Ensure guest is working the queues too. */
+    virtioR3PropagateResumeNotification(&pThis->Virtio);
+}
+
+/**
  * @interface_method_impl{PDMDEVREGR3,pfnReset}
  */
@@ -2146 +2181 @@
     PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
     pThis->fResetting = true;
-    virtioScsiR3QuiesceDevice(pDevIns);
+    virtioScsiR3QuiesceDevice(pDevIns, kvirtIoScsiQuiescingForReset);
+
+    /** @todo r=bird: Shouldn't you reset the device here?!? */
 }
 
@@ -2154 +2191 @@
 static DECLCALLBACK(int) virtioScsiR3Destruct(PPDMDEVINS pDevIns)
 {
-    /*
-     * Check the versions here as well since the destructor is *always* called.
-     */
-
     PDMDEV_CHECK_VERSIONS_RETURN_QUIET(pDevIns);
-
-    PVIRTIOSCSI  pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
+    PVIRTIOSCSI pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
 
     RTMemFree(pThis->paTargetInstances);
@@ -2173 +2205 @@
         }
     }
-     return VINF_SUCCESS;
+
+    virtioR3Term(&pThis->Virtio, pDevIns);
+    return VINF_SUCCESS;
 }
 
@@ -2265 +2299 @@
         return PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio-scsi: failed to initialize VirtIO"));
 
+    /* Name the queues: */
     RTStrCopy(pThis->aszQueueNames[CONTROLQ_IDX], VIRTIO_MAX_QUEUE_NAME_SIZE, "controlq");
     RTStrCopy(pThis->aszQueueNames[EVENTQ_IDX],   VIRTIO_MAX_QUEUE_NAME_SIZE, "eventq");
@@ -2271 +2306 @@
                     "requestq<%d>", qIdx - VIRTQ_REQ_BASE);
 
+    /* Attach the queues and create worker threads for them: */
     for (uint16_t qIdx = 0; qIdx < VIRTIOSCSI_QUEUE_CNT; qIdx++)
     {
-        rc = virtioQueueAttach(&pThis->Virtio, qIdx, QUEUENAME(qIdx));
-        pThis->afQueueAttached[qIdx] = (rc == VINF_SUCCESS);
+        rc = virtioR3QueueAttach(&pThis->Virtio, qIdx, QUEUENAME(qIdx));
+        pThis->afQueueAttached[qIdx] = (rc == VINF_SUCCESS); /** @todo r=bird: This looks a bit fishy, esp. giving the following. */
 
         if (qIdx == CONTROLQ_IDX || IS_REQ_QUEUE(qIdx))
@@ -2437 +2473 @@
     /* .pfnPowerOn = */             NULL,
     /* .pfnReset = */               virtioScsiR3Reset,
-    /* .pfnSuspend = */             virtioScsiR3SuspendOrPowerOff,
+    /* .pfnSuspend = */             virtioScsiR3Suspend,
     /* .pfnResume = */              virtioScsiR3Resume,
     /* .pfnAttach = */              virtioScsiR3Attach,
@@ -2443 +2479 @@
     /* .pfnQueryInterface = */      NULL,
     /* .pfnInitComplete = */        NULL,
-    /* .pfnPowerOff = */            virtioScsiR3SuspendOrPowerOff,
+    /* .pfnPowerOff = */            virtioScsiR3PowerOff,
     /* .pfnSoftReset = */           NULL,
     /* .pfnReserved0 = */           NULL,