Changeset 81628 in vbox for trunk/src/VBox/Devices/VirtIO

Timestamp:

Nov 2, 2019 7:40:09 AM (5 years ago)

Author:

vboxsync

Message:

Storage/DevVirtioSCSI.cpp Fixed leaks found with Sanitizer. Worked on save/live exec code but that is untested, due host crash on crash on Linux that needs to be fixed first. See BugRef:9440, Comment #119

Location:

trunk/src/VBox/Devices/VirtIO

Files:

• Virtio_1_0.cpp (modified) (6 diffs)
• Virtio_1_0_impl.h (modified) (2 diffs)

trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp

@@ -278 +278 @@
          pVirtq->uUsedIdx,  QUEUENAME(qIdx), uUsedIdx));
 
+    RTMemFree((void *)pDescChain->pSgPhysSend->paSegs);
+    RTMemFree(pDescChain->pSgPhysSend);
     RTMemFree((void *)pSgPhysReturn->paSegs);
     RTMemFree(pSgPhysReturn);
@@ -314 +316 @@
 static void virtioQueueNotified(PVIRTIOSTATE pVirtio, uint16_t qIdx, uint16_t uNotifyIdx)
 {
-    Assert(uNotifyIdx == qIdx);
-    (void)uNotifyIdx;
+    /* See VirtIO 1.0, section 4.1.5.2 It implies that qIdx and uNotifyIdx should match.
+     * Disregarding this notification may cause throughput to stop, however there's no way to know
+     * which was queue was intended for wake-up if the two parameters disagree. */
+
+    AssertMsg(uNotifyIdx == qIdx,
+        ("Notification param disagreement. Guest kicked virtq %d's notify addr w/non-corresponding virtq idx %d\n",
+            qIdx, uNotifyIdx));
+
+//    AssertMsgReturn(uNotifyIdx == qIdx,
+//        ("Notification param disagreement. Guest kicked virtq %d's notify addr w/non-corresponding virtq idx %d\n",
+//            qIdx, uNotifyIdx));
+    RT_NOREF(uNotifyIdx);
 
     PVIRTQSTATE pVirtq = &pVirtio->virtqState[qIdx];
     Log6Func(("%s\n", pVirtq->szVirtqName));
-    (void)pVirtq;
+    RT_NOREF(pVirtq);
 
     /** Inform client */
@@ -470 +482 @@
     pVirtio->uISR                   = 0;
 
+    virtioLowerInterrupt(pVirtio);
 
     if (!pVirtio->fMsiSupport)  /* VirtIO 1.0, 4.1.4.3 and 4.1.5.1.2 */
@@ -827 +840 @@
         uint16_t qIdx = uNotifyBaseOffset / VIRTIO_NOTIFY_OFFSET_MULTIPLIER;
         uint16_t uAvailDescIdx = *(uint16_t *)pv;
+
         virtioQueueNotified(pVirtio, qIdx, uAvailDescIdx);
     }
@@ -1260 +1274 @@
     rc = SSMR3PutU32(pSSM,    pVirtio->uDriverFeaturesSelect);
     rc = SSMR3PutU32(pSSM,    pVirtio->uNumQueues);
-    rc = SSMR3PutU32(pSSM,    pVirtio->cbDevSpecificCfg);
-    rc = SSMR3PutU64(pSSM,    pVirtio->uDeviceFeatures);
     rc = SSMR3PutU64(pSSM,    pVirtio->uDriverFeatures);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->pDevSpecificCfg);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnVirtioStatusChanged);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnVirtioQueueNotified);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnVirtioDevCapRead);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnVirtioDevCapWrite);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnSSMDevLiveExec);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnSSMDevSaveExec);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnSSMDevLoadExec);
-    rc = SSMR3PutU64(pSSM,    (uint64_t)pVirtio->virtioCallbacks.pfnSSMDevLoadDone);
-    rc = SSMR3PutGCPhys(pSSM, pVirtio->pGcPhysCommonCfg);
-    rc = SSMR3PutGCPhys(pSSM, pVirtio->pGcPhysNotifyCap);
-    rc = SSMR3PutGCPhys(pSSM, pVirtio->pGcPhysIsrCap);
-    rc = SSMR3PutGCPhys(pSSM, pVirtio->pGcPhysDeviceCap);
-    rc = SSMR3PutGCPhys(pSSM, pVirtio->pGcPhysPciCapBase);
 
     for (uint16_t i = 0; i < pVirtio->uNumQueues; i++)
@@ -1316 +1314 @@
         rc = SSMR3GetU32(pSSM,   &pVirtio->uDriverFeaturesSelect);
         rc = SSMR3GetU32(pSSM,   &pVirtio->uNumQueues);
-        rc = SSMR3GetU32(pSSM,   &pVirtio->cbDevSpecificCfg);
-        rc = SSMR3GetU64(pSSM,   &pVirtio->uDeviceFeatures);
         rc = SSMR3GetU64(pSSM,   &pVirtio->uDriverFeatures);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->pDevSpecificCfg);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnVirtioStatusChanged);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnVirtioQueueNotified);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnVirtioDevCapRead);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnVirtioDevCapWrite);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnSSMDevLiveExec);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnSSMDevSaveExec);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnSSMDevLoadExec);
-        rc = SSMR3GetU64(pSSM,   (uint64_t *)&pVirtio->virtioCallbacks.pfnSSMDevLoadDone);
-        rc = SSMR3GetGCPhys(pSSM, &pVirtio->pGcPhysCommonCfg);
-        rc = SSMR3GetGCPhys(pSSM, &pVirtio->pGcPhysNotifyCap);
-        rc = SSMR3GetGCPhys(pSSM, &pVirtio->pGcPhysIsrCap);
-        rc = SSMR3GetGCPhys(pSSM, &pVirtio->pGcPhysDeviceCap);
-        rc = SSMR3GetGCPhys(pSSM, &pVirtio->pGcPhysPciCapBase);
+
 
         for (uint16_t i = 0; i < pVirtio->uNumQueues; i++)

trunk/src/VBox/Devices/VirtIO/Virtio_1_0_impl.h

@@ -465 +465 @@
 {
     AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));
+    RT_UNTRUSTED_VALIDATED_FENCE(); /* VirtIO 1.0, Section 3.2.1.4.1 */
     PDMDevHlpPCIPhysWrite(pVirtio->CTX_SUFF(pDevIns),
                           pVirtio->pGcPhysQueueUsed[qIdx] + RT_UOFFSETOF(VIRTQ_USED_T, fFlags),
@@ -473 +474 @@
 {
     uint16_t uAvailEventIdx;
+    RT_UNTRUSTED_VALIDATED_FENCE(); /* VirtIO 1.0, Section 3.2.1.4.1 */
     /** VirtIO 1.0 uAvailEventIdx (avail_event) immediately follows ring */
     AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));