Index: /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp =================================================================== --- /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 80614) +++ /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 80615) @@ -108,4 +108,53 @@ #define IS_REQ_QUEUE(qIdx) (qIdx >= VIRTQ_REQ_BASE && qIdx < VIRTIOSCSI_QUEUE_CNT) + +/** + * 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). + * + * @param member - Member of VIRTIO_PCI_COMMON_CFG_T + * @result - true or false + */ +#define MATCH_SCSI_CONFIG(member) \ + (RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member) == 8 \ + && ( uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) \ + || uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) + sizeof(uint32_t)) \ + && cb == sizeof(uint32_t)) \ + || (uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) \ + && cb == RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member)) + +#define LOG_ACCESSOR(member) \ + virtioLogMappedIoValue(__FUNCTION__, #member, RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member), \ + pv, cb, uIntraOffset, fWrite, false, 0); + +#define SCSI_CONFIG_ACCESSOR(member) \ + { \ + uint32_t uIntraOffset = uOffset - RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member); \ + if (fWrite) \ + memcpy(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset, (const char *)pv, cb); \ + else \ + memcpy((char *)pv, (const char *)(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset), cb); \ + LOG_ACCESSOR(member); \ + } + +#define SCSI_CONFIG_ACCESSOR_READONLY(member) \ + { \ + uint32_t uIntraOffset = uOffset - RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member); \ + if (fWrite) \ + LogFunc(("Guest attempted to write readonly virtio_pci_common_cfg.%s\n", #member)); \ + else \ + { \ + memcpy((char *)pv, (const char *)(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset), cb); \ + LOG_ACCESSOR(member); \ + } \ + } + +#define VIRTIO_IN_DIRECTION(pMediaExTxDirEnumValue) \ + pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_FROM_DEVICE + +#define VIRTIO_OUT_DIRECTION(pMediaExTxDirEnumValue) \ + pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_TO_DEVICE /** * The following struct is the VirtIO SCSI Host Device device-specific configuration described in section 5.6.4 @@ -493,6 +542,9 @@ #define SET_LUN_BUF(target, lun, out) \ - out[0] = 0x01; out[1] = target; out[2] = (lun >> 8) & 0x40; out[3] = lun & 0xff; *((uint16_t *)out + 4) = 0; - + out[0] = 0x01; \ + out[1] = target; \ + out[2] = (lun >> 8) & 0x40; \ + out[3] = lun & 0xff; \ + *((uint16_t *)out + 4) = 0; DECLINLINE(bool) isBufZero(uint8_t *pv, size_t cb) @@ -595,53 +647,4 @@ } -/** - * 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). - * - * @param member - Member of VIRTIO_PCI_COMMON_CFG_T - * @result - true or false - */ -#define MATCH_SCSI_CONFIG(member) \ - (RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member) == 8 \ - && ( uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) \ - || uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) + sizeof(uint32_t)) \ - && cb == sizeof(uint32_t)) \ - || (uOffset == RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member) \ - && cb == RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member)) - -#define LOG_ACCESSOR(member) \ - virtioLogMappedIoValue(__FUNCTION__, #member, RT_SIZEOFMEMB(VIRTIOSCSI_CONFIG_T, member), \ - pv, cb, uIntraOffset, fWrite, false, 0); - -#define SCSI_CONFIG_ACCESSOR(member) \ - { \ - uint32_t uIntraOffset = uOffset - RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member); \ - if (fWrite) \ - memcpy(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset, (const char *)pv, cb); \ - else \ - memcpy((char *)pv, (const char *)(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset), cb); \ - LOG_ACCESSOR(member); \ - } - -#define SCSI_CONFIG_ACCESSOR_READONLY(member) \ - { \ - uint32_t uIntraOffset = uOffset - RT_UOFFSETOF(VIRTIOSCSI_CONFIG_T, member); \ - if (fWrite) \ - LogFunc(("Guest attempted to write readonly virtio_pci_common_cfg.%s\n", #member)); \ - else \ - { \ - memcpy((char *)pv, (const char *)(((char *)&pThis->virtioScsiConfig.member) + uIntraOffset), cb); \ - LOG_ACCESSOR(member); \ - } \ - } - -#define VIRTIO_IN_DIRECTION(pMediaExTxDirEnumValue) \ - pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_FROM_DEVICE - -#define VIRTIO_OUT_DIRECTION(pMediaExTxDirEnumValue) \ - pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_TO_DEVICE - typedef struct VIRTIOSCSIREQ *PVIRTIOSCSIREQ; @@ -667,4 +670,28 @@ } #endif + +DECLINLINE(void) virtioScsiVirtToSgPhys(PVIRTIOSCSI pThis, PRTSGBUF pSgDst, void *pvSrc, size_t cb) +{ + while (cb) + { + size_t cbSeg = cb; + RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(pSgDst, &cbSeg); + PDMDevHlpPhysWrite(pThis->CTX_SUFF(pDevIns), GCPhys, pvSrc, cbSeg); + pvSrc = ((uint8_t *)pvSrc) + cbSeg; + cb -= cbSeg; + } +} + +DECLINLINE(void) virtioScsiSgPhysTogVirt(PVIRTIOSCSI pThis, PRTSGBUF pSgSrc, void *pvDst, size_t cb) +{ + while (cb) + { + size_t cbSeg = cb; + RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(pSgSrc, &cbSeg); + PDMDevHlpPhysRead(pThis->CTX_SUFF(pDevIns), GCPhys, pvDst, cbSeg); + pvDst = ((uint8_t *)pvDst) + cbSeg; + cb -= cbSeg; + } +} /** @@ -701,32 +728,23 @@ */ static DECLCALLBACK(int) virtioScsiR3IoReqCopyFromBuf(PPDMIMEDIAEXPORT pInterface, PDMMEDIAEXIOREQ hIoReq, - void *pvIoReqAlloc, uint32_t offDst, PRTSGBUF pSgBuf, - size_t cbCopy) -{ - RT_NOREF(hIoReq); - RT_NOREF(pInterface); - + void *pvIoReqAlloc, uint32_t offDst, PRTSGBUF pSgBuf, size_t cbCopy) +{ + RT_NOREF2(hIoReq, pInterface); PVIRTIOSCSIREQ pReq = (PVIRTIOSCSIREQ)pvIoReqAlloc; - - /** DrvSCSI.cpp, that issues this callback, just sticks one segment in the buffer */ - memcpy(pReq->pbDataIn + offDst, pSgBuf->paSegs[0].pvSeg, cbCopy); + if (pReq->pbDataIn) + RTSgBufCopyToBuf(pSgBuf, pReq->pbDataIn + offDst, cbCopy); return VINF_SUCCESS; } - /** * @interface_method_impl{PDMIMEDIAEXPORT,pfnIoReqCopyToBuf} */ static DECLCALLBACK(int) virtioScsiR3IoReqCopyToBuf(PPDMIMEDIAEXPORT pInterface, PDMMEDIAEXIOREQ hIoReq, - void *pvIoReqAlloc, uint32_t offSrc, PRTSGBUF pSgBuf, - size_t cbCopy) -{ - RT_NOREF(hIoReq); - RT_NOREF(pInterface); - + void *pvIoReqAlloc, uint32_t offSrc, PRTSGBUF pSgBuf, size_t cbCopy) +{ + RT_NOREF2(hIoReq, pInterface); PVIRTIOSCSIREQ pReq = (PVIRTIOSCSIREQ)pvIoReqAlloc; - /** DrvSCSI.cpp, that issues this callback, just sticks one segment in the buffer */ - - memcpy(pSgBuf->paSegs[0].pvSeg, pReq->pbDataOut + offSrc, cbCopy); + if (pReq->pbDataOut) + RTSgBufCopyFromBuf(pSgBuf, pReq->pbDataOut + offSrc, cbCopy); return VINF_SUCCESS; @@ -799,4 +817,13 @@ } +static void virtioScsiFreeReq(PVIRTIOSCSITARGET pTarget, PVIRTIOSCSIREQ pReq) +{ + RTMemFree(pReq->pbSense); + RTMemFree(pReq->pbPiIn); + RTMemFree(pReq->pbDataIn); + RTMemFree(pReq->pVirtqReq); + pTarget->pDrvMediaEx->pfnIoReqFree(pTarget->pDrvMediaEx, pReq->hIoReq); +} + /** * This is called to complete a request immediately @@ -848,8 +875,4 @@ * and return the failure code for any-and-all until that's done before * allowing a reset to continue. - * - * In the absence of active I/O farmed out to VSCSI - * the device handles a guest driver unload/reload gracefully and has - * been tested. */ static int virtioScsiReqFinish(PVIRTIOSCSI pThis, PVIRTIOSCSIREQ pReq, int rcReq) @@ -1012,12 +1035,7 @@ Log(("-----------------------------------------------------------------------------------------\n")); - - } - RTMemFree(pReq->pbSense); - RTMemFree(pReq->pbDataIn); - RTMemFree(pReq->pbPiIn); - RTMemFree(pReq->pVirtqReq); - - pIMediaEx->pfnIoReqFree(pIMediaEx, pReq->hIoReq); + } + + virtioScsiFreeReq(pTarget, pReq); if (pTarget->cReqsInProgress == 0 && pThis->fSignalIdle) @@ -1026,4 +1044,5 @@ return VINF_SUCCESS; } + static int virtioScsiReqSubmit(PVIRTIOSCSI pThis, uint16_t qIdx, PRTSGBUF pInSgBuf, PRTSGBUF pOutSgBuf) @@ -1038,16 +1057,5 @@ AssertReturn(pVirtqReq, VERR_NO_MEMORY); - off_t cbOff = 0; - size_t cbCopy = cbOut; - while (cbCopy) - { - size_t cbSeg = cbCopy; - RTGCPHYS GCPhys = (RTGCPHYS)RTSgBufGetNextSegment(pOutSgBuf, &cbSeg); - PDMDevHlpPhysRead(pThis->CTX_SUFF(pDevIns), GCPhys, ((uint8_t *)pVirtqReq) + cbOff, cbSeg); - cbCopy -= cbSeg; - cbOff += cbSeg; - } - -// VIRTIO_HEX_DUMP(RTLOGGRPFLAGS_LEVEL_12, (uint8_t *)pVirtqReq, cbOut, 0, "\npVirtqReq"); + virtioScsiSgPhysTogVirt(pThis, pOutSgBuf, pVirtqReq, cbOut); uint8_t uTarget = pVirtqReq->cmdHdr.uLUN[1]; @@ -1093,4 +1101,5 @@ respHdr.uResidual = cbDataOut + cbDataIn; virtioScsiReqErr(pThis, qIdx, &respHdr, abSense); + RTMemFree(pVirtqReq); return VINF_SUCCESS; } @@ -1136,6 +1145,6 @@ pReq->cbIn = cbIn; pReq->cbOut = cbOut; - pReq->pbDataOut = pbDataOut; pReq->cbDataOut = cbDataOut; + pReq->pbDataOut = cbDataOut ? pbDataOut : 0; pReq->pVirtqReq = pVirtqReq; pReq->pInSgBuf = pInSgBuf; @@ -1155,5 +1164,6 @@ rc = pIMediaEx->pfnIoReqSendScsiCmd(pIMediaEx, pReq->hIoReq, uLUN, pVirtqReq->uCdb, pThis->virtioScsiConfig.uCdbSize, - PDMMEDIAEXIOREQSCSITXDIR_UNKNOWN, &pReq->enmTxDir, cbDataIn, + PDMMEDIAEXIOREQSCSITXDIR_UNKNOWN, &pReq->enmTxDir, + RT_MAX(cbDataIn, cbDataOut), pReq->pbSense, pReq->cbSense, &pReq->uSenseLen, &pReq->uStatus, 30 * RT_MS_1SEC); @@ -1188,9 +1198,5 @@ respHdr.uResidual = cbDataIn + cbDataOut; virtioScsiReqErr(pThis, qIdx, &respHdr, abSense); - RTMemFree(pReq->pbSense); - RTMemFree(pReq->pbDataIn); - RTMemFree(pReq->pbPiIn); - RTMemFree(pVirtqReq); - pIMediaEx->pfnIoReqFree(pIMediaEx, pReq->hIoReq); + virtioScsiFreeReq(pTarget, pReq); return VINF_SUCCESS; } @@ -1213,7 +1219,5 @@ static int virtioScsiCtrl(PVIRTIOSCSI pThis, uint16_t qIdx, PRTSGBUF pInSgBuf, PRTSGBUF pOutSgBuf) { - RT_NOREF(pThis); - RT_NOREF(qIdx); - RT_NOREF(pInSgBuf); + RT_NOREF3(pThis, qIdx, pInSgBuf); /** @@ -1227,16 +1231,5 @@ AssertMsgReturn(pScsiCtrl, ("Out of memory"), VERR_NO_MEMORY); - /** - * Get control command into virtual memory - */ - off_t cbOff = 0; - size_t cbSeg = 0; - while (cbOut) - { - RTGCPHYS pvSeg = (RTGCPHYS)RTSgBufGetNextSegment(pOutSgBuf, &cbSeg); - PDMDevHlpPhysRead(pThis->CTX_SUFF(pDevIns), pvSeg, pScsiCtrl + cbOff, cbSeg); - cbOut -= cbSeg; - cbOff += cbSeg; - } + virtioScsiSgPhysTogVirt(pThis, pOutSgBuf, pScsiCtrl, cbOut); uint8_t uResponse = VIRTIOSCSI_S_OK; @@ -1414,5 +1407,5 @@ if (rc == VERR_NOT_AVAILABLE) { - Log3Func(("Nothing found in %s\n", QUEUENAME(qIdx))); + Log6Func(("Nothing found in %s\n", QUEUENAME(qIdx))); continue; } Index: /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp =================================================================== --- /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp (revision 80614) +++ /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp (revision 80615) @@ -1463,5 +1463,5 @@ /** - * Do a hex dump of a buffer. + * Do a hex dump of a buffer * * @param pv Pointer to array to dump @@ -1475,26 +1475,17 @@ if (pszTitle) Log(("%s [%d bytes]:\n", pszTitle, cb)); - for (uint32_t row = 0; row < RT_MAX(1, (cb / 16) + 1); row++) + for (uint32_t row = 0; row < RT_MAX(1, (cb / 16) + 1) && row * 16 < cb; row++) { - uint32_t uAddr = row * 16 + uBase; - Log(("%x%x%x%x: ", (uAddr >> 12) & 0xf, (uAddr >> 8) & 0xf, (uAddr >> 4) & 0xf, uAddr & 0xf)); - for (int col = 0; col < 16; col++) + Log(("%04x: ", row * 16 + uBase)); /* line address */ + for (uint8_t col = 0; col < 16; col++) { uint32_t idx = row * 16 + col; - uint8_t u8 = pv[idx]; if (idx >= cb) Log(("-- %s", (col + 1) % 8 ? "" : " ")); else - Log(("%x%x %s", u8 >> 4 & 0xf, u8 & 0xf, (col + 1) % 8 ? "" : " ")); + Log(("%02x %s", pv[idx], (col + 1) % 8 ? "" : " ")); } - for (int col = 0; col < 16; col++) - { - uint32_t idx = row * 16 + col; - uint8_t u8 = pv[idx]; - if (idx >= cb) - Log((" ")); - else - Log(("%c", u8 >= 0x20 && u8 <= 0x7e ? u8 : '.')); - } + for (uint32_t idx = row * 16; idx < row * 16 + 16; idx++) + Log(("%c", (idx >= cb) ? ' ' : (pv[idx] >= 0x20 && pv[idx] <= 0x7e ? pv[idx] : '.'))); Log(("\n")); }