Changeset 80931 in vbox for trunk

Timestamp:

Sep 22, 2019 9:56:37 AM (5 years ago)

Author:

vboxsync

Message:

Storage/DevVirtioSCSI.cpp: Fixed errors that prevented it from building on Windows and tested. See bugref:9440, Comment #93

Location:

trunk/src/VBox/Devices

Files:

• Storage/DevVirtioSCSI.cpp (modified) (10 diffs)
• VirtIO/Virtio_1_0.cpp (modified) (19 diffs)
• VirtIO/Virtio_1_0.h (modified) (5 diffs)
• VirtIO/Virtio_1_0_impl.h (modified) (4 diffs)

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

@@ -47 +47 @@
 #include "VBoxDD.h"
 
-
 /*
  * RT log-levels used:
@@ -327 +326 @@
     uint8_t  uResponse;                                        /** response                                          */
 } VIRTIOSCSI_CTRL_TMF_T, *PVIRTIOSCSI_CTRL_TMF_T;
-#pragma pack(0)
+#pragma pack()
 
 /**
@@ -514 +513 @@
 
     /** True if the guest/driver and VirtIO framework are in the ready state */
-    bool                            fVirtioReady;
+    unsigned                         fVirtioReady;
 
     /** True if VIRTIO_SCSI_F_T10_PI was negotiated */
-    bool                            fHasT10pi;
+    unsigned                         fHasT10pi;
 
     /** True if VIRTIO_SCSI_F_T10_PI was negotiated */
-    bool                            fHasHotplug;
+    unsigned                         fHasHotplug;
 
     /** True if VIRTIO_SCSI_F_T10_PI was negotiated */
-    bool                            fHasInOutBufs;
+    unsigned                         fHasInOutBufs;
 
     /** True if VIRTIO_SCSI_F_T10_PI was negotiated */
-    bool                            fHasLunChange;
+    unsigned                         fHasLunChange;
 
     /** True if in the process of resetting */
-    bool                            fResetting;
+    unsigned                         fResetting;
 
     /** True if in the process of quiescing I/O */
-    bool                            fQuiescing;
+    unsigned                         fQuiescing;
 
 } VIRTIOSCSI, *PVIRTIOSCSI;
@@ -861 +860 @@
     respHdr.uSenseLen = pReq->pbSense[2] == SCSI_SENSE_NONE ? 0 : (uint32_t)pReq->uSenseLen;
     AssertMsg(!(cbResidual & 0xffffffff00000000),
-            ("WARNING: Residual size larger than uint32_t holds, truncating"));
+            ("WARNING: Residual size larger than sizeof(uint32_t), truncating"));
     respHdr.uResidual = (uint32_t)(cbResidual & 0xffffffff);
     respHdr.uStatus   = pReq->uStatus;
@@ -938 +937 @@
     if (LogIs12Enabled())
     {
-        uint32_t cb = RT_MIN(cbXfer, 256);
+        Assert(!(cbXfer & 0xffffffff00000000));
+        uint32_t cbXfer32 = cbXfer & 0xffffffff;
+        uint32_t cb = RT_MIN(cbXfer32, 256);
         if (VIRTIO_IN_DIRECTION(pReq->enmTxDir))
         {
@@ -1003 +1004 @@
          * (phys. memory) with the Req response data in virtual memory.
          */
-        uint32_t cbReqSgBuf = RTSgBufCalcTotalLength(&reqSegBuf);
+        size_t cbReqSgBuf = RTSgBufCalcTotalLength(&reqSegBuf);
         AssertMsgReturn(cbReqSgBuf <= pReq->pDescChain->cbPhysDst,
-                       ("Guest expected less req data (space needed: %d, avail: %d)\n",
+                       ("Guest expected less req data (space needed: %ld, avail: %d)\n",
                          cbReqSgBuf, pReq->pDescChain->cbPhysDst),
                        VERR_BUFFER_OVERFLOW);
@@ -1349 +1350 @@
     while (pThread->enmState == PDMTHREADSTATE_RUNNING)
     {
-        if (pThis->fQuiescing || !pThis->fQueueAttached[qIdx])
-        {
-            Log6Func(("Skipping wakeup (quiescing or reset)\n"));
-            continue;
-        }
-
         if (virtioQueueIsEmpty(pThis->hVirtio, qIdx))
         {
@@ -1373 +1368 @@
             ASMAtomicWriteBool(&pWorker->fSleeping, false);
         }
-        Log6Func(("fetching next descriptor chain from %s\n", QUEUENAME(qIdx)));
-        PVIRTIO_DESC_CHAIN_T pDescChain;
-        rc = virtioQueueGet(pThis->hVirtio, qIdx, &pDescChain, true);
-        if (rc == VERR_NOT_AVAILABLE)
+
+        if (!pThis->fQueueAttached[qIdx])
         {
-            Log6Func(("Nothing found in %s\n", QUEUENAME(qIdx)));
-            continue;
+            LogFunc(("%s queue not attached, worker aborting...\n", QUEUENAME(qIdx)));
+            break;
+        } else {
+            LogFunc(("%s queue is still attached!\n", QUEUENAME(qIdx)));
         }
 
-        AssertRC(rc);
-        if (qIdx == CONTROLQ_IDX)
-            virtioScsiCtrl(pThis, qIdx, pDescChain);
-        else /* request queue index */
+        if (!pThis->fQuiescing)
         {
-            rc = virtioScsiReqSubmit(pThis, qIdx, pDescChain);
-            if (RT_FAILURE(rc))
-            {
-                LogRel(("Error submitting req packet, resetting %Rrc", rc));
-            }
+             Log6Func(("fetching next descriptor chain from %s\n", QUEUENAME(qIdx)));
+             PVIRTIO_DESC_CHAIN_T pDescChain;
+             rc = virtioQueueGet(pThis->hVirtio, qIdx, &pDescChain, true);
+             if (rc == VERR_NOT_AVAILABLE)
+             {
+                Log6Func(("Nothing found in %s\n", QUEUENAME(qIdx)));
+                continue;
+             }
+
+             AssertRC(rc);
+             if (qIdx == CONTROLQ_IDX)
+                 virtioScsiCtrl(pThis, qIdx, pDescChain);
+             else /* request queue index */
+             {
+                  rc = virtioScsiReqSubmit(pThis, qIdx, pDescChain);
+                  if (RT_FAILURE(rc))
+                  {
+                     LogRel(("Error submitting req packet, resetting %Rrc", rc));
+                  }
+             }
         }
     }
@@ -1513 +1520 @@
 }
 
-static DECLCALLBACK(void) virtioScsiStatusChanged(VIRTIOHANDLE hVirtio, void *pClient,  bool fVirtioReady)
+static DECLCALLBACK(void) virtioScsiStatusChanged(VIRTIOHANDLE hVirtio, void *pClient,  uint32_t fVirtioReady)
 {
     RT_NOREF(hVirtio);
@@ -1827 +1834 @@
 
 static int virtioScsiCfgAccessed(PVIRTIOSCSI pThis, uint32_t uOffset,
-                                    const void *pv, uint32_t cb, uint8_t fWrite)
+                                    const void *pv, uint32_t cb, bool fWrite)
 {
     int rc = VINF_SUCCESS;

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

@@ -35 +35 @@
 #define INSTANCE(pVirtio) pVirtio->szInstance
 #define QUEUENAME(qIdx) (pVirtio->virtqState[qIdx].szVirtqName)
+
 
 /**
@@ -246 +247 @@
     Log3Func(("Copying client data to %s, desc chain (head desc_idx %d)\n",
                QUEUENAME(qIdx), uUsedIdx));
+    (void)uUsedIdx;
 
     /*
@@ -252 +254 @@
      * (pulled from avail ring of queue), essentially giving result back to the guest driver.
      */
-    uint32_t cbRemain = RTSgBufCalcTotalLength(pSgVirtReturn);
-    uint32_t cbCopy = 0;
+    size_t cbCopy = 0;
+    size_t cbRemain = RTSgBufCalcTotalLength(pSgVirtReturn);
     while (cbRemain)
     {
@@ -260 +262 @@
         uint64_t dstSgLen   = (uint64_t)paSeg->cbSeg;
         uint64_t dstSgCur   = (uint64_t)pSgPhysReturn->pvSegCur;
-        cbCopy = RT_MIN(pSgVirtReturn->cbSegLeft, dstSgLen - (dstSgCur - dstSgStart));
+        cbCopy = RT_MIN((uint64_t)pSgVirtReturn->cbSegLeft, dstSgLen - (dstSgCur - dstSgStart));
         PDMDevHlpPhysWrite(pVirtio->CTX_SUFF(pDevIns),
                           (RTGCPHYS)pSgPhysReturn->pvSegCur, pSgVirtReturn->pvSegCur, cbCopy);
@@ -276 +278 @@
             pVirtq->fEventThresholdReached = true;
 
+    Assert(!(cbCopy & 0xffffffff00000000));
     /**
      * Place used buffer's descriptor in used ring but don't update used ring's slot index.
      * That will be done with a subsequent client call to virtioQueueSync() */
-    virtioWriteUsedElem(pVirtio, qIdx, pVirtq->uUsedIdx++, pDescChain->uHeadIdx, cbCopy);
-
-    Log2Func((".... Copied %u bytes to %u byte buffer, residual=%d\n",
+    virtioWriteUsedElem(pVirtio, qIdx, pVirtq->uUsedIdx++, pDescChain->uHeadIdx,
+            (uint32_t)(cbCopy & 0xffffffff));
+
+    Log2Func((".... Copied %ul bytes to %ul byte buffer, residual=%ul\n",
          cbCopy, pDescChain->cbPhysDst, pDescChain->cbPhysDst - cbCopy));
 
@@ -310 +314 @@
     Log6Func(("Updating %s used_idx from %u to %u\n",
               QUEUENAME(qIdx), uIdx, pVirtq->uUsedIdx));
+    (void)uIdx;
 
     virtioWriteUsedRingIdx(pVirtio, qIdx, pVirtq->uUsedIdx);
@@ -323 +328 @@
 {
     Assert(uNotifyIdx == qIdx);
+    (void)uNotifyIdx;
 
     PVIRTQSTATE pVirtq = &pVirtio->virtqState[qIdx];
     Log6Func(("%s\n", pVirtq->szVirtqName));
+    (void)pVirtq;
 
     /** Inform client */
@@ -488 +495 @@
 {
     PVIRTIOSTATE pVirtio = (PVIRTIOSTATE)hVirtio;
-    return pVirtio->uQueueEnable[qIdx];
+    return pVirtio->uQueueEnable[qIdx] != 0;
 }
 
@@ -528 +535 @@
 
     /** Let the client know */
-    pVirtio->virtioCallbacks.pfnVirtioStatusChanged((VIRTIOHANDLE)pVirtio, pVirtio->pClientContext, false);
+    pVirtio->virtioCallbacks.pfnVirtioStatusChanged((VIRTIOHANDLE)pVirtio, pVirtio->pClientContext, 0);
     virtioResetDevice(pVirtio);
 }
@@ -645 +652 @@
              * Notify client only if status actually changed from last time.
              */
-            bool fOkayNow = pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK;
-            bool fWasOkay = pVirtio->uPrevDeviceStatus & VIRTIO_STATUS_DRIVER_OK;
+            uint32_t fOkayNow = pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK;
+            uint32_t fWasOkay = pVirtio->uPrevDeviceStatus & VIRTIO_STATUS_DRIVER_OK;
+            uint32_t fDrvOk = pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK;
             if ((fOkayNow && !fWasOkay) || (!fOkayNow && fWasOkay))
-                pVirtio->virtioCallbacks.pfnVirtioStatusChanged((VIRTIOHANDLE)pVirtio, pVirtio->pClientContext,
-                       pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK);
+                pVirtio->virtioCallbacks.pfnVirtioStatusChanged((VIRTIOHANDLE)pVirtio, pVirtio->pClientContext, fDrvOk);
             pVirtio->uPrevDeviceStatus = pVirtio->uDeviceStatus;
         }
@@ -758 +765 @@
     {
         uint32_t uOffset = GCPhysAddr - pVirtio->pGcPhysCommonCfg;
-        virtioCommonCfgAccessed(pVirtio, 0 /* fWrite */, uOffset, cb, pv);
+        virtioCommonCfgAccessed(pVirtio, 0 /* fWrite */, uOffset, cb, (void const *)pv);
     }
     else
@@ -1072 +1079 @@
     pVirtio->virtioCallbacks.pfnSSMDevLoadExec      = ssmLoadExecCallback;
     pVirtio->virtioCallbacks.pfnSSMDevLoadDone      = ssmLoadDoneCallback;
-
 
     /* Set PCI config registers (assume 32-bit mode) */
@@ -1308 +1314 @@
 
     int rc = VINF_SUCCESS;
-    virtioDumpState(pVirtio, "virtioSaveExec");
-
+
+#ifdef DEBUG
+    virtioDumpState(pVirtio, (const char *)"virtioSaveExec");
+#endif
     rc = SSMR3PutBool(pSSM,   pVirtio->fGenUpdatePending);
     rc = SSMR3PutU8(pSSM,     pVirtio->uDeviceStatus);
@@ -1365 +1373 @@
 
     int rc = VINF_SUCCESS;
+
+#ifdef DEBUG
     virtioDumpState(pVirtio, "virtioLoadExec");
+#endif
 
     if (uPass == SSM_PASS_FINAL)
@@ -1424 +1435 @@
 
     int rc = VINF_SUCCESS;
+#ifdef DEBUG
     virtioDumpState(pVirtio, "virtioLoadDone");
-
+#endif
     rc = pVirtio->virtioCallbacks.pfnSSMDevLoadDone(pDevIns, pSSM);
 
@@ -1437 +1449 @@
 
     int rc = VINF_SUCCESS;
+
+#ifdef DEBUG
     virtioDumpState(pVirtio, "virtioLiveExec");
-
+#endif
     rc = pVirtio->virtioCallbacks.pfnSSMDevLiveExec(pDevIns, pSSM, uPass);
 
@@ -1473 +1487 @@
     }
     Log(("\n"));
+    RT_NOREF2(uBase, pv);
  }
 
@@ -1489 +1504 @@
  */
 void virtioLogMappedIoValue(const char *pszFunc, const char *pszMember, uint32_t uMemberSize,
-                        const void *pv, uint32_t cb, uint32_t uOffset, bool fWrite,
-                        bool fHasIndex, uint32_t idx)
+                        const void *pv, uint32_t cb, uint32_t uOffset, int fWrite,
+                        int fHasIndex, uint32_t idx)
 {
 
@@ -1529 +1544 @@
               pszIdx, uOffset, uOffset + cb, cb, pv));
     }
+    RT_NOREF2(fWrite, pszFunc);
 }
 

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

@@ -65 +65 @@
 {
     uint32_t  uHeadIdx;                                    /**< Head idx of associated desc chain        */
-    uint32_t    cbVirtSrc;                                   /**< Size of virt source buffer               */
+    uint32_t  cbVirtSrc;                                   /**< Size of virt source buffer               */
     void     *pVirtSrc;                                    /**< Virt mem buf holding out data from guest */
-    uint32_t    cbPhysDst;                                   /**< Total size of dst buffer                 */
+    uint32_t  cbPhysDst;                                   /**< Total size of dst buffer                 */
     PRTSGBUF  pSgPhysDst;                                  /**< Phys S/G buf to store result for guest   */
 } VIRTIO_DESC_CHAIN_T, *PVIRTIO_DESC_CHAIN_T, **PPVIRTIO_DESC_CHAIN_T;
@@ -96 +96 @@
  * @param pClient       Pointer to opaque client data (state)
  */
-typedef DECLCALLBACK(void)   FNVIRTIOSTATUSCHANGED(VIRTIOHANDLE hVirtio, void *pClient, bool fDriverOk);
+typedef DECLCALLBACK(void)   FNVIRTIOSTATUSCHANGED(VIRTIOHANDLE hVirtio, void *pClient, uint32_t fDriverOk);
 typedef FNVIRTIOSTATUSCHANGED *PFNVIRTIOSTATUSCHANGED;
 
@@ -140 +140 @@
 {
      DECLCALLBACKMEMBER(void, pfnVirtioStatusChanged)
-                                  (VIRTIOHANDLE hVirtio, void *pClient, bool fDriverOk);
+                                  (VIRTIOHANDLE hVirtio, void *pClient, uint32_t fDriverOk);
      DECLCALLBACKMEMBER(void, pfnVirtioQueueNotified)
                                   (VIRTIOHANDLE hVirtio, void *pClient, uint16_t qIdx);
@@ -320 +320 @@
  */
 uint64_t virtioGetNegotiatedFeatures(VIRTIOHANDLE hVirtio);
-
 
 /** CLIENT MUST CALL ON RELOCATE CALLBACK! */
@@ -383 +382 @@
 void virtioLogMappedIoValue(const char *pszFunc, const char *pszMember, uint32_t uMemberSize,
                             const void *pv, uint32_t cb, uint32_t uOffset,
-                            bool fWrite, bool fHasIndex, uint32_t idx);
-
+                            int fWrite, int fHasIndex, uint32_t idx);
 /**
  * Does a formatted hex dump using Log(()), recommend using VIRTIO_HEX_DUMP() macro to

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

@@ -240 +240 @@
 /**
 * This macro returns true if the implied parameter GCPhysAddr address and access length are
-* within mapped capability struct specified with the explicit parameters.
+* within the range of the mapped capability struct specified with the explicit parameters.
 *
 * Actual Parameters:
@@ -283 +283 @@
 #define LOG_COMMON_CFG_ACCESS_INDEXED(member, idx) \
         virtioLogMappedIoValue(__FUNCTION__, #member, RT_SIZEOFMEMB(VIRTIO_PCI_COMMON_CFG_T, member), \
-                               pv, cb, uIntraOff, fWrite, true, idx);
+                                pv, cb, uIntraOff, fWrite, true, idx);
 
 #define COMMON_CFG_ACCESSOR(member) \
@@ -521 +521 @@
 }
 
+static int  virtioCommonCfgAccessed (PVIRTIOSTATE pVirtio, int fWrite, off_t uOffset, unsigned cb, void const *pv);
 static void virtioResetQueue        (PVIRTIOSTATE pVirtio, uint16_t qIdx);
 static void virtioNotifyGuestDriver (PVIRTIOSTATE pVirtio, uint16_t qIdx, bool fForce);
@@ -526 +527 @@
 static void virtioLowerInterrupt    (PVIRTIOSTATE pVirtio);
 static void virtioQueueNotified     (PVIRTIOSTATE pVirtio, uint16_t qidx, uint16_t uDescIdx);
-static int  virtioCommonCfgAccessed (PVIRTIOSTATE pVirtio, int fWrite, off_t uOffset, unsigned cb, void const *pv);
 static void virtioGuestResetted     (PVIRTIOSTATE pVirtio);