Changeset 80928 in vbox for trunk

Timestamp:

Sep 21, 2019 5:29:54 AM (5 years ago)

Author:

vboxsync

Message:

Storage/DevVirtioSCSI.cpp: Fixed/improved attach target count/allocation and processing, converted size_t to uint32_t where possible and added mapping on uint32_t <-> size_t conversions between VirtIO and the VSCSI layer. Other clean-up

Location:

trunk/src/VBox/Devices

Files:

• Storage/DevVirtioSCSI.cpp (modified) (47 diffs)
• VirtIO/Virtio_1_0.cpp (modified) (13 diffs)
• VirtIO/Virtio_1_0.h (modified) (6 diffs)
• VirtIO/Virtio_1_0_impl.h (modified) (1 diff)

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

@@ -47 +47 @@
 #include "VBoxDD.h"
 
+
+/*
+ * RT log-levels used:
+ *
+ *    Level 1:   The most important (but usually rare) things to note
+ *    Level 2:   SCSI command logging
+ *    Level 3:   Vector and I/O transfer summary (shows what client sent an expects and fulfillment)
+ *    Level 6:   Device ⟷ Guest Driver negotation, traffic, notifications and state handling
+ *    Level 12:  Brief formatted hex dumps of I/O data
+ */
+
 #define LUN0    0
 /**
@@ -69 +80 @@
  * TEMPORARY NOTE: following parameter is set to 1 for early development. Will be increased later
  */
-#define VIRTIOSCSI_REQ_QUEUE_CNT                    1            /**< Number of req queues exposed by dev.            */
+#define VIRTIOSCSI_REQ_QUEUE_CNT                    2            /**< Number of req queues exposed by dev.            */
 #define VIRTIOSCSI_QUEUE_CNT                        VIRTIOSCSI_REQ_QUEUE_CNT + 2
-#define VIRTIOSCSI_MAX_TARGETS                      256          /**< Default value                                   */
 #define VIRTIOSCSI_MAX_LUN                          256          /* < VirtIO specification, section 5.6.4             */
 #define VIRTIOSCSI_MAX_COMMANDS_PER_LUN             1            /* < T.B.D. What is a good value for this?           */
@@ -110 +120 @@
 
 /**
- * This macro resolves to boolean true if uOffset matches a field offset and size exactly,
- * (or if it is a 64-bit field, if it accesses either 32-bit part as a 32-bit access)
- * ASSUMED this critereon is mandated by section 4.1.3.1 of the VirtIO 1.0 specification)
- * This MACRO can be re-written to allow unaligned access to a field (within bounds).
+ * Resolves to boolean true if uOffset matches a field offset and size exactly,
+ * (or if 64-bit field, if it accesses either 32-bit part as a 32-bit access)
+ * It is *assumed* this critereon is mandated by section 4.1.3.1 of the VirtIO 1.0 spec)
+ * This macro can be re-written to allow unaligned access to a field (within bounds).
  *
  * @param   member   - Member of VIRTIO_PCI_COMMON_CFG_T
@@ -160 +170 @@
             pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_TO_DEVICE
 /**
- * The following struct is the VirtIO SCSI Host Device   device-specific configuration described in section 5.6.4
- * of the VirtIO 1.0 specification. This layout maps an MMIO area shared VirtIO guest driver. The VBox VirtIO
- * this virtual controller device implementation is a client of. The frame work calls back whenever the guest driver
- * accesses any part of field in this struct
+ * Following struct is the VirtIO SCSI Host Device device-specific configuration described in section 5.6.4
+ * of the VirtIO 1.0 spec. Layout maps an MMIO area shared VirtIO guest driver. The VBox VirtIO
+ * this virtual controller device implementation is a client of. Framework does a callback whenever
+ * guest driver accesses any part of field in this struct
  */
 typedef struct virtio_scsi_config
@@ -312 +322 @@
     uint32_t uType;                                            /** type                                              */
     uint32_t uSubtype;                                         /** subtype                                           */
-    uint8_t  uScsiLun[8];                                    /** lun                                               */
+    uint8_t  uScsiLun[8];                                      /** lun                                               */
     uint64_t uId;                                              /** id                                                */
     // Device-writable part
@@ -335 +345 @@
     // Device-readable part
     uint32_t  uType;                                          /** type                                               */
-    uint8_t   uScsiLun[8];                                  /** lun                                                */
+    uint8_t   uScsiLun[8];                                    /** lun                                                */
     uint32_t  uEventsRequested;                               /** event_requested                                    */
     // Device-writable part
@@ -429 +439 @@
     VIRTIOHANDLE                    hVirtio;
 
-    /** SCSI target instances data */
-    VIRTIOSCSITARGET                aTargetInstances[VIRTIOSCSI_MAX_TARGETS];
+    /** Number of targets detected */
+    uint64_t                        cTargets;
+
+    R3PTRTYPE(PVIRTIOSCSITARGET)    paTargetInstances;
+#if HC_ARCH_BITS == 32
+      RTR3PTR                 R3PtrPadding0;
+#endif
 
     /** Per device-bound virtq worker-thread contexts (eventq slot unused) */
@@ -482 +497 @@
     /** The event semaphore the processing thread waits on. */
 
-    /** Number of targets detected */
-    uint64_t                        cTargets;
 
     /** Total number of requests active across all targets */
@@ -532 +545 @@
     PVIRTIOSCSITARGET              pTarget;                  /**< Target                                            */
     uint16_t                       qIdx;                     /**< Index of queue this request arrived on            */
-    size_t                         cbPiOut;                  /**< Size of T10 pi in buffer                          */
+    uint32_t                       cbPiOut;                  /**< Size of T10 pi in buffer                          */
     uint8_t                       *pbPiOut;                  /**< Address of pi out buffer                          */
     uint8_t                       *pbDataOut;                /**< dataout                                           */
-    size_t                         cbDataOut;                /**< size of dataout buffer                            */
-    size_t                         cbPiIn;                   /**< Size of T10 pi buffer                             */
+    uint32_t                       cbDataOut;                /**< size of dataout buffer                            */
+    uint32_t                       cbPiIn;                   /**< Size of T10 pi buffer                             */
     uint8_t                       *pbPiIn;                   /**< Address of pi in buffer                           */
-    size_t                         cbDataIn;                 /**< Size of datain buffer                             */
+    uint32_t                       cbDataIn;                 /**< Size of datain buffer                             */
     uint8_t                       *pbDataIn;                 /**< datain                                            */
-    size_t                         cbSense;                  /**< Size of sense buffer                              */
+    uint32_t                       cbSense;                  /**< Size of sense buffer                              */
     size_t                         uSenseLen;                /**< Receives # bytes written into sense buffer        */
     uint8_t                       *pbSense;                  /**< Pointer to R3 sense buffer                        */
@@ -548 +561 @@
 } VIRTIOSCSIREQ;
 
-DECLINLINE(bool) isBufZero(uint8_t *pv, size_t cb)
+DECLINLINE(bool) isBufZero(uint8_t *pv, uint32_t cb)
 {
     for (uint32_t i = 0; i < cb; i++)
@@ -619 +632 @@
 }
 
-DECLINLINE(void) virtioGetControlAsyncMaskText(char *pszOutput, size_t cbOutput, uint32_t uAsyncTypesMask)
+DECLINLINE(void) virtioGetControlAsyncMaskText(char *pszOutput, uint32_t cbOutput, uint32_t uAsyncTypesMask)
 {
     RTStrPrintf(pszOutput, cbOutput, "%s%s%s%s%s%s",
@@ -713 +726 @@
         case VIRTIOSCSI_T_NO_EVENT:
             if (uEventType & VIRTIOSCSI_T_EVENTS_MISSED)
-                LogFunc(("(target=%d, LUN=%d) Warning driver that events were missed\n", uTarget, LUN0));
+                Log6Func(("(target=%d, LUN=%d) Warning driver that events were missed\n", uTarget, LUN0));
             else
-                LogFunc(("(target=%d, LUN=%d): Warning event info guest queued is shorter than configured\n",
+                Log6Func(("(target=%d, LUN=%d): Warning event info guest queued is shorter than configured\n",
                           uTarget, LUN0));
             break;
@@ -722 +735 @@
             {
                 case VIRTIOSCSI_EVT_RESET_REMOVED:
-                    LogFunc(("(target=%d, LUN=%d): Target or LUN removed\n", uTarget, LUN0));
+                    Log6Func(("(target=%d, LUN=%d): Target or LUN removed\n", uTarget, LUN0));
                     break;
                 case VIRTIOSCSI_EVT_RESET_RESCAN:
-                    LogFunc(("(target=%d, LUN=%d): Target or LUN added\n", uTarget, LUN0));
+                    Log6Func(("(target=%d, LUN=%d): Target or LUN added\n", uTarget, LUN0));
                     break;
                 case VIRTIOSCSI_EVT_RESET_HARD:
-                    LogFunc(("(target=%d, LUN=%d): Target was reset\n", uTarget, LUN0));
+                    Log6Func(("(target=%d, LUN=%d): Target was reset\n", uTarget, LUN0));
                     break;
             }
@@ -735 +748 @@
             char szTypeText[128];
             virtioGetControlAsyncMaskText(szTypeText, sizeof(szTypeText), uReason);
-            LogFunc(("(target=%d, LUN=%d): Delivering subscribed async notification %s\n", uTarget, LUN0));
+            Log6Func(("(target=%d, LUN=%d): Delivering subscribed async notification %s\n", uTarget, LUN0));
             break;
         case VIRTIOSCSI_T_PARAM_CHANGE:
@@ -742 +755 @@
             break;
         default:
-            LogFunc(("(target=%d, LUN=%d): Unknown event type: %d, ignoring\n", uTarget, LUN0));
+            Log6Func(("(target=%d, LUN=%d): Unknown event type: %d, ignoring\n", uTarget, LUN0));
             return VINF_SUCCESS;
     }
@@ -837 +850 @@
     PPDMIMEDIAEX      pIMediaEx = pTarget->pDrvMediaEx;
 
-    size_t cbResidual = 0, cbXfer = 0;
+    size_t cbResidual = 0;
+    size_t cbXfer = 0;
     int rc = pIMediaEx->pfnIoReqQueryResidual(pIMediaEx, hIoReq, &cbResidual);
     AssertRC(rc);
@@ -845 +859 @@
 
     struct REQ_RESP_HDR respHdr = { 0 };
-    respHdr.uSenseLen = pReq->pbSense[2] == SCSI_SENSE_NONE ? 0 : pReq->uSenseLen;
-    respHdr.uResidual = cbResidual;
+    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"));
+    respHdr.uResidual = (uint32_t)(cbResidual & 0xffffffff);
     respHdr.uStatus   = pReq->uStatus;
 
@@ -909 +925 @@
         LogFunc(("cbDataIn = %u, cbDataOut = %u (cbIn = %u, cbOut = %u)\n",
                   pReq->cbDataIn, pReq->cbDataOut, pReq->pDescChain->cbPhysDst, pReq->pDescChain->cbVirtSrc));
-        LogFunc(("xfer = %d, residual = %u\n", cbXfer, cbResidual));
+        LogFunc(("xfer = %lu, residual = %u\n", cbXfer, cbResidual));
         LogFunc(("xfer direction: %s, sense written = %d, sense size = %d\n",
              virtioGetTxDirText(pReq->enmTxDir), respHdr.uSenseLen, pThis->virtioScsiConfig.uSenseSize));
@@ -920 +936 @@
     }
 
-    if (LogIsItEnabled(RTLOGGRPFLAGS_LEVEL_12, LOG_GROUP))
-    {
-        size_t cb = RT_MIN(cbXfer, 256);
+    if (LogIs12Enabled())
+    {
+        uint32_t cb = RT_MIN(cbXfer, 256);
         if (VIRTIO_IN_DIRECTION(pReq->enmTxDir))
         {
-            Log(("datain[256 of %d total bytes xferred]:\n", cbXfer));
+            Log(("datain[%d of %d total bytes xferred]:\n", cb, cbXfer));
             if (!isBufZero(pReq->pbDataIn, cb))
                 VIRTIO_HEX_DUMP(RTLOGGRPFLAGS_LEVEL_12, pReq->pbDataIn,  cb,  0, 0);
@@ -934 +950 @@
         if (VIRTIO_OUT_DIRECTION(pReq->enmTxDir))
         {
-            Log(("dataout[256 of %d total bytes xferred]:\n", cbXfer));
+            Log(("dataout[%d of %d total bytes xferred]:\n", cb, cbXfer));
             if (!isBufZero(pReq->pbDataOut, cb))
                 VIRTIO_HEX_DUMP(RTLOGGRPFLAGS_LEVEL_12, pReq->pbDataOut, cb, 0, 0);
@@ -987 +1003 @@
          * (phys. memory) with the Req response data in virtual memory.
          */
-        size_t cbReqSgBuf = RTSgBufCalcTotalLength(&reqSegBuf);
+        uint32_t cbReqSgBuf = RTSgBufCalcTotalLength(&reqSegBuf);
         AssertMsgReturn(cbReqSgBuf <= pReq->pDescChain->cbPhysDst,
                        ("Guest expected less req data (space needed: %d, avail: %d)\n",
@@ -1016 +1032 @@
     uint8_t  uTarget  = pVirtqReq->cmdHdr.uVirtioLun[1];
     uint32_t uScsiLun = (pVirtqReq->cmdHdr.uVirtioLun[2] << 8 | pVirtqReq->cmdHdr.uVirtioLun[3]) & 0x3fff;
-    PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[uTarget];
+    PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[uTarget];
 
     LogFunc(("[%s] (Target: %d LUN: %d)  CDB: %.*Rhxs\n",
@@ -1029 +1045 @@
      * Calculate request offsets
      */
-    size_t  cbPiIn = 0, cbPiOut = 0, cbPiHdr = 0;
+    uint32_t  cbPiIn = 0, cbPiOut = 0, cbPiHdr = 0;
     off_t   piOutOff = 0;
 
@@ -1043 +1059 @@
     off_t   uDataInOff  = sizeof(REQ_RESP_HDR) + pThis->virtioScsiConfig.uSenseSize + cbPiIn;
     uint8_t *pbDataOut = (uint8_t *)((uint64_t)pVirtqReq + uDataOutOff);
-    size_t  cbDataOut = pDescChain->cbVirtSrc - uDataOutOff;
-    size_t  cbDataIn  = pDescChain->cbPhysDst - uDataInOff;
+    uint32_t  cbDataOut = pDescChain->cbVirtSrc - uDataOutOff;
+    uint32_t  cbDataIn  = pDescChain->cbPhysDst - uDataInOff;
 
     /**
@@ -1132 +1148 @@
 
     rc = pIMediaEx->pfnIoReqSendScsiCmd(pIMediaEx, pReq->hIoReq, uScsiLun,
-                                        pVirtqReq->uCdb, pThis->virtioScsiConfig.uCdbSize,
+                                        pVirtqReq->uCdb, (size_t)pThis->virtioScsiConfig.uCdbSize,
                                         PDMMEDIAEXIOREQSCSITXDIR_UNKNOWN, &pReq->enmTxDir,
-                                        RT_MAX(cbDataIn, cbDataOut),
-                                        pReq->pbSense, pReq->cbSense, &pReq->uSenseLen,
+                                        (size_t)RT_MAX(cbDataIn, cbDataOut),
+                                        pReq->pbSense, (size_t)pReq->cbSense, &pReq->uSenseLen,
                                         &pReq->uStatus, 30 * RT_MS_1SEC);
 
@@ -1145 +1161 @@
          */
         Log2Func(("Request submission error from lower-level driver\n"));
-        size_t cbResidual;
-        pIMediaEx->pfnIoReqQueryResidual(pIMediaEx, pReq->hIoReq, &cbResidual);
         uint8_t uASC, uASCQ = 0;
         switch (rc)
@@ -1343 +1357 @@
         if (virtioQueueIsEmpty(pThis->hVirtio, qIdx))
         {
-            /* Interlocks avoid missing alarm while going to sleep & notifier doesn't wake the awoken */
+            /* Atomic interlocks avoid missing alarm while going to sleep & notifier waking the awoken */
             ASMAtomicWriteBool(&pWorker->fSleeping, true);
             bool fNotificationSent = ASMAtomicXchgBool(&pWorker->fNotified, false);
@@ -1371 +1385 @@
         if (qIdx == CONTROLQ_IDX)
             virtioScsiCtrl(pThis, qIdx, pDescChain);
-        else
+        else /* request queue index */
         {
             rc = virtioScsiReqSubmit(pThis, qIdx, pDescChain);
@@ -1472 +1486 @@
     PVIRTIOSCSI pThis = (PVIRTIOSCSI)pClient;
     PWORKER pWorker = &pThis->aWorker[qIdx];
-
 
     RTLogFlush(RTLogDefaultInstanceEx(RT_MAKE_U32(0, UINT16_MAX)));
@@ -1736 +1749 @@
     for (uint32_t i = 0; i < pThis->cTargets; i++)
     {
-        PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[i];
+        PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[i];
         if (pTarget->pDrvBase)
             if (pTarget->pDrvMediaEx)
@@ -1778 +1791 @@
  * @returns VBox status code.
  * @param   pInterface      Pointer to the interface structure containing the called function pointer.
- * @param   iTarget            The unit which status LED we desire.
+ * @param   iTarget         The unit which status LED we desire.
  * @param   ppLed           Where to store the LED pointer.
  */
@@ -1798 +1811 @@
  * @returns VBox status code.
  * @param   pInterface      Pointer to the interface structure containing the called function pointer.
- * @param   iTarget            The unit which status LED we desire.
+ * @param   iTarget         The unit which status LED we desire.
  * @param   ppLed           Where to store the LED pointer.
  */
@@ -1806 +1819 @@
     if (iTarget < pThis->cTargets)
     {
-        *ppLed = &pThis->aTargetInstances[iTarget].led;
+        *ppLed = &pThis->paTargetInstances[iTarget].led;
         Assert((*ppLed)->u32Magic == PDMLED_MAGIC);
         return VINF_SUCCESS;
@@ -1814 +1827 @@
 
 static int virtioScsiCfgAccessed(PVIRTIOSCSI pThis, uint32_t uOffset,
-                                    const void *pv, size_t cb, uint8_t fWrite)
+                                    const void *pv, uint32_t cb, uint8_t fWrite)
 {
     int rc = VINF_SUCCESS;
@@ -1863 +1876 @@
  * @param   cb          Number of bytes to read
  */
-static DECLCALLBACK(int) virtioScsiDevCapRead(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pv, size_t cb)
+static DECLCALLBACK(int) virtioScsiDevCapRead(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pv, uint32_t cb)
 {
     int rc = VINF_SUCCESS;
@@ -1882 +1895 @@
  * @param   cb          Number of bytes to write
  */
-static DECLCALLBACK(int) virtioScsiDevCapWrite(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pv, size_t cb)
+static DECLCALLBACK(int) virtioScsiDevCapWrite(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pv, uint32_t cb)
 {
     int rc = VINF_SUCCESS;
@@ -1916 +1929 @@
     pThis->pDevInsR3 = pDevIns;
 
-    for (uint32_t i = 0; i < VIRTIOSCSI_MAX_TARGETS; i++)
-    {
-        PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[i];
+    for (uint32_t i = 0; i < pThis->cTargets; i++)
+    {
+        PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[i];
         pTarget->pVirtioScsi = pThis;
     }
@@ -1986 +1999 @@
     RT_NOREF(fFlags);
     PVIRTIOSCSI       pThis = PDMINS_2_DATA(pDevIns, PVIRTIOSCSI);
-    PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[iTarget];
+    PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[iTarget];
 
     LogFunc((""));
@@ -2013 +2026 @@
 {
     PVIRTIOSCSI       pThis   = PDMINS_2_DATA(pDevIns, PVIRTIOSCSI);
-    PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[iTarget];
+    PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[iTarget];
     int rc;
 
@@ -2057 +2070 @@
     PVIRTIOSCSI  pThis = PDMINS_2_DATA(pDevIns, PVIRTIOSCSI);
 
+    RTMemFree(pThis->paTargetInstances);
+    pThis->paTargetInstances = NULL;
     for (int qIdx = 0; qIdx < VIRTIOSCSI_QUEUE_CNT; qIdx++)
     {
@@ -2085 +2100 @@
 
     /* Usable defaults */
-    pThis->cTargets = VIRTIOSCSI_MAX_TARGETS;
+    pThis->cTargets = 1;
 
     /*
@@ -2206 +2221 @@
     Log2Func(("Found %d targets attached to controller\n", pThis->cTargets));
 
+     pThis->paTargetInstances = (PVIRTIOSCSITARGET)RTMemAllocZ(sizeof(VIRTIOSCSITARGET) * pThis->cTargets);
+     if (!pThis->paTargetInstances)
+          return PDMDEV_SET_ERROR(pDevIns, rc, N_("Failed to allocate memory for target states"));
+
     for (RTUINT iTarget = 0; iTarget < pThis->cTargets; iTarget++)
     {
-        PVIRTIOSCSITARGET pTarget = &pThis->aTargetInstances[iTarget];
+        PVIRTIOSCSITARGET pTarget = &pThis->paTargetInstances[iTarget];
 
         if (RTStrAPrintf(&pTarget->pszTargetName, "VSCSI%u", iTarget) < 0)
@@ -2238 +2257 @@
         LogFunc(("Attaching LUN: %s\n", pTarget->pszTargetName));
 
-        AssertReturn(iTarget < RT_ELEMENTS(pThis->aTargetInstances), VERR_PDM_NO_SUCH_LUN);
-        rc = PDMDevHlpDriverAttach(pDevIns, iTarget, &pTarget->IBase, &pTarget->pDrvBase, (const char *)&pTarget->pszTargetName);
+        AssertReturn(iTarget < pThis->cTargets, VERR_PDM_NO_SUCH_LUN);
+        rc = PDMDevHlpDriverAttach(pDevIns, iTarget, &pTarget->IBase, &pTarget->pDrvBase, pTarget->pszTargetName);
         if (RT_SUCCESS(rc))
         {
@@ -2246 +2265 @@
             pTarget->pDrvMedia = PDMIBASE_QUERY_INTERFACE(pTarget->pDrvBase, PDMIMEDIA);
             AssertMsgReturn(VALID_PTR(pTarget->pDrvMedia),
-                         ("virtio-scsi configuration error: LUN#%d missing basic media interface!\n", pTarget->iTarget),
+                         ("virtio-scsi configuration error: LUN#%d missing basic media interface!\n", iTarget),
                          VERR_PDM_MISSING_INTERFACE);
 
@@ -2252 +2271 @@
             pTarget->pDrvMediaEx = PDMIBASE_QUERY_INTERFACE(pTarget->pDrvBase, PDMIMEDIAEX);
             AssertMsgReturn(VALID_PTR(pTarget->pDrvMediaEx),
-                         ("virtio-scsi configuration error: LUN#%d missing extended media interface!\n", pTarget->iTarget),
+                         ("virtio-scsi configuration error: LUN#%d missing extended media interface!\n", iTarget),
                          VERR_PDM_MISSING_INTERFACE);
 
             rc = pTarget->pDrvMediaEx->pfnIoReqAllocSizeSet(pTarget->pDrvMediaEx, sizeof(VIRTIOSCSIREQ));
             AssertMsgReturn(VALID_PTR(pTarget->pDrvMediaEx),
-                         ("virtio-scsi configuration error: LUN#%u: Failed to set I/O request size!\n", pTarget->iTarget),
+                         ("virtio-scsi configuration error: LUN#%u: Failed to set I/O request size!\n", iTarget),
                          rc);
 
@@ -2263 +2282 @@
             AssertMsgReturn(VALID_PTR(pTarget->pDrvMediaEx),
                          ("virtio-scsi configuration error: LUN#%u: Failed to get set Media notify obj!\n",
-                          pTarget->iTarget), rc);
+                          iTarget), rc);
 
         }
@@ -2275 +2294 @@
         else
         {
-            AssertLogRelMsgFailed(("virtio-scsi: Failed to attach %s: %Rrc\n", pTarget->pszTargetName));
+            AssertLogRelMsgFailed(("virtio-scsi: Failed to attach %s: %Rrc\n", pTarget->pszTargetName, rc));
             return rc;
         }

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

@@ -39 +39 @@
  * See API comments in header file for description
  */
-void virtioVirtToSgPhys(VIRTIOHANDLE hVirtio, PRTSGBUF pSgDst, void *pvSrc, size_t cb)
-{
-    while (cb)
-    {
-        size_t cbSeg = cb;
-        RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(pSgDst, &cbSeg);
-        PDMDevHlpPhysWrite(((PVIRTIOSTATE)hVirtio)->CTX_SUFF(pDevIns), GCPhys, pvSrc, cbSeg);
-        pvSrc = ((uint8_t *)pvSrc) + cbSeg;
-        cb -= cbSeg;
-    }
-}
-
-/**
- * See API comments in header file for description
- */
-void virtioSgPhysToVirt(VIRTIOHANDLE hVirtio, PRTSGBUF pSgSrc, void *pvDst, size_t cb)
-{
-    while (cb)
-    {
-        size_t cbSeg = cb;
-        RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(pSgSrc, &cbSeg);
-        PDMDevHlpPhysRead(((PVIRTIOSTATE)hVirtio)->CTX_SUFF(pDevIns), GCPhys, pvDst, cbSeg);
-        pvDst = ((uint8_t *)pvDst) + cbSeg;
-        cb -= cbSeg;
-    }
-}
-
-/**
- * See API comments in header file for description
- */
 int virtioQueueAttach(VIRTIOHANDLE hVirtio, uint16_t qIdx, const char *pcszName)
 {
@@ -79 +49 @@
     RTStrCopy((char *)pVirtq->szVirtqName, sizeof(pVirtq->szVirtqName), pcszName);
     return VINF_SUCCESS;
-
 }
 
@@ -157 +126 @@
     uint16_t uDescIdx = uHeadIdx;
 
-    Log6Func(("%s DESC CHAIN: (head) desc_idx=%u [avail_idx=%u]\n",
+    Log3Func(("%s DESC CHAIN: (head) desc_idx=%u [avail_idx=%u]\n",
             pVirtq->szVirtqName, uHeadIdx, pVirtq->uAvailIdx));
 
@@ -198 +167 @@
         if (desc.fFlags & VIRTQ_DESC_F_WRITE)
         {
-            Log6Func(("%s IN  desc_idx=%u seg=%u addr=%RGp cb=%u\n",
+            Log3Func(("%s IN  desc_idx=%u seg=%u addr=%RGp cb=%u\n",
                 QUEUENAME(qIdx), uDescIdx, cSegsIn, desc.pGcPhysBuf, desc.cb));
             cbIn += desc.cb;
@@ -205 +174 @@
         else
         {
-            Log6Func(("%s OUT desc_idx=%u seg=%u addr=%RGp cb=%u\n",
+            Log3Func(("%s OUT desc_idx=%u seg=%u addr=%RGp cb=%u\n",
                 QUEUENAME(qIdx), uDescIdx, cSegsOut, desc.pGcPhysBuf, desc.cb));
             cbOut += desc.cb;
@@ -223 +192 @@
     RTSgBufInit(pSgPhysIn, (PCRTSGSEG)paSegsIn, cSegsIn);
 
-    void *pSgVirtOut = RTMemAlloc(cbOut);
-    AssertReturn(pSgVirtOut, VERR_NO_MEMORY);
+    void *pVirtOut = RTMemAlloc(cbOut);
+    AssertReturn(pVirtOut, VERR_NO_MEMORY);
+
+    /* If there's any guest → device data in phys. memory pulled
+     * from queue, copy it into virtual memory to return to caller */
 
     if (cSegsOut)
     {
+        uint8_t *outSgVirt = (uint8_t *)pVirtOut;
         RTSGBUF outSgPhys;
         RTSgBufInit(&outSgPhys, (PCRTSGSEG)paSegsOut, cSegsOut);
-        virtioSgPhysToVirt((PVIRTIOSTATE)hVirtio, &outSgPhys, pSgVirtOut, cbOut);
+        for (size_t cb = cbOut; cb;)
+        {
+            size_t cbSeg = cb;
+            RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(&outSgPhys, &cbSeg);
+            PDMDevHlpPhysRead(((PVIRTIOSTATE)hVirtio)->CTX_SUFF(pDevIns), GCPhys, outSgVirt, cbSeg);
+            outSgVirt = ((uint8_t *)outSgVirt) + cbSeg;
+            cb -= cbSeg;
+        }
         RTMemFree(paSegsOut);
     }
@@ -239 +219 @@
     pDescChain->uHeadIdx   = uHeadIdx;
     pDescChain->cbVirtSrc  = cbOut;
-    pDescChain->pVirtSrc   = pSgVirtOut;
+    pDescChain->pVirtSrc   = pVirtOut;
     pDescChain->cbPhysDst  = cbIn;
     pDescChain->pSgPhysDst = pSgPhysIn;
     *ppDescChain = pDescChain;
 
-    Log6Func(("%s -- segs OUT: %u (%u bytes)   IN: %u (%u bytes) --\n",
+    Log3Func(("%s -- segs OUT: %u (%u bytes)   IN: %u (%u bytes) --\n",
               pVirtq->szVirtqName, cSegsOut, cbOut, cSegsIn, cbIn));
 
@@ -264 +244 @@
 
     uint16_t uUsedIdx = virtioReadUsedRingIdx(pVirtio, qIdx);
-    Log6Func(("Copying client data to %s, desc chain (head desc_idx %d)\n",
+    Log3Func(("Copying client data to %s, desc chain (head desc_idx %d)\n",
                QUEUENAME(qIdx), uUsedIdx));
 
@@ -270 +250 @@
      * Copy virtual memory s/g buffer containing data to return to the guest
      * to phys. memory described by (IN direction ) s/g buffer of the descriptor chain
-     * original pulled from the queue, to 'send back' to the guest driver.
+     * (pulled from avail ring of queue), essentially giving result back to the guest driver.
      */
-    size_t cbRemain = RTSgBufCalcTotalLength(pSgVirtReturn);
-    size_t cbCopy = 0;
+    uint32_t cbRemain = RTSgBufCalcTotalLength(pSgVirtReturn);
+    uint32_t cbCopy = 0;
     while (cbRemain)
     {
@@ -288 +268 @@
     }
 
-
     if (fFence)
         RT_UNTRUSTED_NONVOLATILE_COPY_FENCE();
@@ -302 +281 @@
     virtioWriteUsedElem(pVirtio, qIdx, pVirtq->uUsedIdx++, pDescChain->uHeadIdx, cbCopy);
 
-
-    if (LogIs2Enabled())
-    {
-        Log2Func((".... Copied %u bytes to %u byte buffer, residual=%d\n",
-             cbCopy, pDescChain->cbPhysDst, pDescChain->cbPhysDst - cbCopy));
-    }
+    Log2Func((".... Copied %u bytes to %u byte buffer, residual=%d\n",
+         cbCopy, pDescChain->cbPhysDst, pDescChain->cbPhysDst - cbCopy));
+
     Log6Func(("Write ahead used_idx=%d, %s used_idx=%d\n",
          pVirtq->uUsedIdx,  QUEUENAME(qIdx), uUsedIdx));
@@ -1477 +1453 @@
   *
   */
- void virtioHexDump(uint8_t *pv, size_t cb, uint32_t uBase, const char *pszTitle)
+ void virtioHexDump(uint8_t *pv, uint32_t cb, uint32_t uBase, const char *pszTitle)
  {
      if (pszTitle)
@@ -1512 +1488 @@
  * @param   idx         - The index, if fHasIndex is true
  */
-void virtioLogMappedIoValue(const char *pszFunc, const char *pszMember, size_t uMemberSize,
+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)

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

@@ -65 +65 @@
 {
     uint32_t  uHeadIdx;                                    /**< Head idx of associated desc chain        */
-    size_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 */
-    size_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;
@@ -119 +119 @@
  * @param   cbWrite     Number of bytes to write
  */
-typedef DECLCALLBACK(int)   FNVIRTIODEVCAPWRITE(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, size_t cbWrite);
+typedef DECLCALLBACK(int)   FNVIRTIODEVCAPWRITE(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, uint32_t cbWrite);
 typedef FNVIRTIODEVCAPWRITE *PFNVIRTIODEVCAPWRITE;
 
@@ -131 +131 @@
  * @param   cbRead      Number of bytes to read
  */
-typedef DECLCALLBACK(int)   FNVIRTIODEVCAPREAD(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, size_t cbRead);
+typedef DECLCALLBACK(int)   FNVIRTIODEVCAPREAD(PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, uint32_t cbRead);
 typedef FNVIRTIODEVCAPREAD *PFNVIRTIODEVCAPREAD;
 
@@ -144 +144 @@
                                   (VIRTIOHANDLE hVirtio, void *pClient, uint16_t qIdx);
      DECLCALLBACKMEMBER(int,  pfnVirtioDevCapRead)
-                                  (PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, size_t cbRead);
+                                  (PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, uint32_t cbRead);
      DECLCALLBACKMEMBER(int,  pfnVirtioDevCapWrite)
-                                  (PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, size_t cbWrite);
+                                  (PPDMDEVINS pDevIns, uint32_t uOffset, const void *pvBuf, uint32_t cbWrite);
      DECLCALLBACKMEMBER(int,  pfnSSMDevLiveExec)
                                   (PPDMDEVINS pDevIns, PSSMHANDLE pSSM, uint32_t uPass);
@@ -381 +381 @@
  * @param   idx         - The index if fHasIndex
  */
-void virtioLogMappedIoValue(const char *pszFunc, const char *pszMember, size_t uMemberSize,
+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);
@@ -395 +395 @@
  *                        provided text with value of cb to indicate size next to it.
  */
-void virtioHexDump(uint8_t *pv, size_t cb, uint32_t uBase, const char *pszTitle);
+void virtioHexDump(uint8_t *pv, uint32_t cb, uint32_t uBase, const char *pszTitle);
 
 #endif /* !VBOX_INCLUDED_SRC_VirtIO_Virtio_1_0_h */

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

@@ -517 +517 @@
         if (status & VIRTIO_STATUS_DEVICE_NEEDS_RESET)
             Log6(("%sNEEDS_RESET", primed++ ? " | " : ""));
+        (void)primed;
     }
 }