Changeset 80539 in vbox

Timestamp:

Sep 2, 2019 7:22:42 AM (5 years ago)

Author:

vboxsync

Message:

Storage/DevVirtioSCSI.cpp: Added code to properly free some allocated entities when done. Also gave the request completion command access to the OUT components of the request (CDB) in case it needs to determine the command type. Also moved SCSI status text lookup into Storage/Debug.cpp

Location:

trunk

Files:

• include/VBox/scsi.h (modified) (1 diff)
• src/VBox/Devices/Storage/Debug.cpp (modified) (2 diffs)
• src/VBox/Devices/Storage/DevVirtioSCSI.cpp (modified) (11 diffs)

trunk/include/VBox/scsi.h

@@ -317 +317 @@
 #if defined(IN_RING3) && (defined(LOG_ENABLED) || defined(RT_STRICT))
 const char * SCSICmdText(uint8_t uCmd);
+const char * SCSIStatusText(uint8_t uStatus);
 const char * SCSISenseText(uint8_t uSense);
 const char * SCSISenseExtText(uint8_t uASC, uint8_t uASCQ);

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

@@ -575 +575 @@
     "MISCOMPARE",
     "(reserved)"
+};
+
+static struct
+{
+    uint8_t uStatus;
+    const char * const pszStatusText;
+} g_aSCSIStatusText[]
+=
+{
+    { 0x00, "GOOD" },
+    { 0x02, "CHECK CONDITION" },
+    { 0x04, "CONDITION MET" },
+    { 0x08, "BUSY" },
+    { 0x10, "INTERMEDIATE"},
+    { 0x14, "CONDITION MET" },
+    { 0x18, "RESERVATION CONFLICT" },
+    { 0x22, "COMMAND TERMINATED" },
+    { 0x28, "TASK SET FULL" },
+    { 0x30, "ACA ACTIVE" },
+    { 0x40, "TASK ABORTED" },
 };
 
@@ -937 +957 @@
 }
 
+const char * SCSIStatusText(uint8_t uStatus)
+{
+    unsigned iIdx;
+
+    /* Linear search. Doesn't hurt as we don't call this function very frequently */
+    for (iIdx = 0; iIdx < RT_ELEMENTS(g_aSCSISenseText); iIdx++)
+    {
+        if (g_aSCSIStatusText[iIdx].uStatus  == uStatus)
+            return g_aSCSIStatusText[iIdx].pszStatusText;
+    }
+    return "(Unknown extended status code)";
+}
+
 /**
  * Return the plain text of an extended SCSI sense key.

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

@@ -471 +471 @@
     size_t                         cbPiOut;                  /**< Size of T10 pi in buffer                          */
     uint8_t                       *pbPiOut;                  /**< Address of pi out buffer                          */
-    uint8_t                       *pbDataOut;                /**< dataout */
+    uint8_t                       *pbDataOut;                /**< dataout                                           */
     size_t                         cbPiIn;                   /**< Size of T10 pi buffer                             */
     uint8_t                       *pbPiIn;                   /**< Address of pi in buffer                           */
@@ -480 +480 @@
     uint8_t                        uStatus;                  /**< SCSI status code                                  */
     PRTSGBUF                       pInSgBuf;                 /**< Buf vector to return PDM result to VirtIO Guest   */
+    PVIRTIOSCSI_REQ_CMD_T          pVirtqReq;                /**< Allocated to receive translated queued req        */
 } VIRTIOSCSIREQ;
 
@@ -524 +525 @@
         default:                                        return "<unknown>";
     }
-}
-
-DECLINLINE(void) virtioGetControlAsyncMaskText(char *pszOutput, size_t cbOutput, uint32_t uAsyncTypesMask)
-{
-    RTStrPrintf(pszOutput, cbOutput, "%s%s%s%s%s%s",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_OPERATIONAL_CHANGE) ? "CHANGE_OPERATION  "   : "",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_POWER_MGMT)         ? "POWER_MGMT  "         : "",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_EXTERNAL_REQUEST)   ? "EXTERNAL_REQ  "       : "",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_MEDIA_CHANGE)       ? "MEDIA_CHANGE  "       : "",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_MULTI_HOST)         ? "MULTI_HOST  "         : "",
-        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_DEVICE_BUSY)        ? "DEVICE_BUSY  "        : "");
 }
 
@@ -571 +561 @@
 }
 
-DECLINLINE(const char *) virtioGetScsiStatusText(uint8_t uScsiStatusCode)
-{
-    switch (uScsiStatusCode)
-    {
-        case 0x00:  return "GOOD";
-        case 0x02:  return "CHECK CONDITION";
-        case 0x04:  return "CONDITION MET";
-        case 0x08:  return "BUSY";
-        case 0x10:  return "INTERMEDIATE";
-        case 0x14:  return "CONDITION MET";
-        case 0x18:  return "RESERVATION CONFLICT";
-        case 0x22:  return "COMMAND TERMINATED";
-        case 0x28:  return "TASK SET FULL";
-        case 0x30:  return "ACA ACTIVE";
-        case 0x40:  return "TASK ABORTED";
-        default:    return "<UNKNOWN CODE>";
-    }
+DECLINLINE(void) virtioGetControlAsyncMaskText(char *pszOutput, size_t cbOutput, uint32_t uAsyncTypesMask)
+{
+    RTStrPrintf(pszOutput, cbOutput, "%s%s%s%s%s%s",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_OPERATIONAL_CHANGE) ? "CHANGE_OPERATION  "   : "",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_POWER_MGMT)         ? "POWER_MGMT  "         : "",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_EXTERNAL_REQUEST)   ? "EXTERNAL_REQ  "       : "",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_MEDIA_CHANGE)       ? "MEDIA_CHANGE  "       : "",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_MULTI_HOST)         ? "MULTI_HOST  "         : "",
+        (uAsyncTypesMask & VIRTIOSCSI_EVT_ASYNC_DEVICE_BUSY)        ? "DEVICE_BUSY  "        : "");
 }
 
@@ -837 +819 @@
 
     LogFunc(("status: %s  response: %s  0x%x%x/0x%x%x  cbXfer=%d, cbResidual: %u\n",
-                virtioGetScsiStatusText(pReq->uStatus),  virtioGetReqRespText(respHdr.uResponse),
+                SCSIStatusText(pReq->uStatus),  virtioGetReqRespText(respHdr.uResponse),
                 pReq->uStatus >> 4 & 0xf, pReq->uStatus & 0xf,
                 respHdr.uResponse >> 4 & 0xf, respHdr.uResponse & 0xf,
@@ -898 +880 @@
     virtioQueueSync(pThis->hVirtio, pReq->qIdx);
 
+    RTMemFree(pReq->pVirtqReq);
     RTMemFree(pReq->pbSense);
     RTMemFree(pReq->pbDataIn);
@@ -1051 +1034 @@
         respHdr.uStatusQualifier = 0;
         virtioScsiReqFinish(pThis, qIdx, &respHdr, pbSense);
+        RTMemFree(pVirtqReq);
         return VINF_SUCCESS;
     }
@@ -1070 +1054 @@
         pReq->pbDataOut = pbDataOut;
         pReq->cbPiIn    = cbPiIn;
+        pReq->pVirtqReq = pVirtqReq;
 
         if (cbPiIn)
@@ -1120 +1105 @@
             uint8_t pbSense[] = { RT_BIT(7) | SCSI_SENSE_RESPONSE_CODE_CURR_FIXED, 0, SCSI_SENSE_VENDOR_SPECIFIC,
                                   0, 0, 0, 0, 10, uASC, uASCQ, 0 };
-            respHdr.cbSense = sizeof(pbSense);
+            respHdr.cbSense   = sizeof(pbSense);
             respHdr.uResidual = cbDataIn + cbDataOut;
             respHdr.uStatus   = SCSI_STATUS_CHECK_CONDITION;
@@ -1126 +1111 @@
             respHdr.uStatusQualifier = 0;
             virtioScsiReqFinish(pThis, qIdx, &respHdr,  NULL /* pbSense */);
+            RTMemFree(pVirtqReq);
+            RTMemFree(pReq->pbSense);
+            RTMemFree(pReq->pbDataIn);
+            RTMemFree(pReq->pbPiIn);
+            pIMediaEx->pfnIoReqFree(pIMediaEx, pReq->hIoReq);
             return VINF_SUCCESS;
         }
@@ -1132 +1122 @@
         uint8_t pbSense[] = { RT_BIT(7) | SCSI_SENSE_RESPONSE_CODE_CURR_FIXED, 0, SCSI_SENSE_NOT_READY,
                               0, 0, 0, 0, 10, 0, 0, 0 };
-        respHdr.cbSense = sizeof(pbSense);
+        respHdr.cbSense   = sizeof(pbSense);
         respHdr.uResidual = cbDataIn + cbDataOut;
         respHdr.uStatus   = SCSI_STATUS_CHECK_CONDITION;