Index: /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp =================================================================== --- /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 80520) +++ /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 80521) @@ -21,5 +21,6 @@ * Header Files * *********************************************************************************************************************************/ -#define LOG_GROUP LOG_GROUP_DRV_SCSI +//#define LOG_GROUP LOG_GROUP_DRV_SCSI +#define LOG_GROUP LOG_GROUP_DEV_VIRTIO #include @@ -56,8 +57,12 @@ /** @} */ -#define VIRTIOSCSI_HOST_SCSI_ALL_FEATURES \ + +#define VIRTIOSCSI_HOST_SCSI_FEATURES_ALL \ (VIRTIO_SCSI_F_INOUT | VIRTIO_SCSI_F_HOTPLUG | VIRTIO_SCSI_F_CHANGE | VIRTIO_SCSI_F_T10_PI) -#define VIRTIOSCSI_HOST_SCSI_FEATURES_OFFERED 0 /**< TBD, support at least hotplug & in/out */ +#define VIRTIOSCSI_HOST_SCSI_FEATURES_NONE 0 + +#define VIRTIOSCSI_HOST_SCSI_FEATURES_OFFERED \ + VIRTIOSCSI_HOST_SCSI_FEATURES_NONE /** @@ -66,10 +71,10 @@ #define VIRTIOSCSI_REQ_QUEUE_CNT 1 /**< Number of req queues exposed by dev. */ #define VIRTIOSCSI_QUEUE_CNT VIRTIOSCSI_REQ_QUEUE_CNT + 2 -#define VIRTIOSCSI_MAX_TARGETS 1 /**< Can probably determined from higher layers */ -#define VIRTIOSCSI_MAX_LUN 16383 /* < VirtIO specification, section 5.6.4 */ +#define VIRTIOSCSI_MAX_TARGETS 1 /**< Can probably determined from higher layers */ +#define VIRTIOSCSI_MAX_LUN 1 /* < VirtIO specification, section 5.6.4 */ #define VIRTIOSCSI_MAX_COMMANDS_PER_LUN 1 /* < T.B.D. What is a good value for this? */ #define VIRTIOSCSI_MAX_SEG_COUNT 1024 /* < T.B.D. What is a good value for this? */ #define VIRTIOSCSI_MAX_SECTORS_HINT 0x10000 /* < VirtIO specification, section 5.6.4 */ -#define VIRTIOSCSI_MAX_CHANNEL_HINT 0 /* < VirtIO specification, section 5.6.4 */ +#define VIRTIOSCSI_MAX_CHANNEL_HINT 0 /* < VirtIO specification, section 5.6.4 should be 0 */ #define VIRTIOSCSI_SAVED_STATE_MINOR_VERSION 0x01 /**< SSM version # */ @@ -80,4 +85,11 @@ #define VIRTIOSCSI_PCI_CLASS 0x01 /**< Base class Mass Storage? */ + +#define VIRTIOSCSI_SENSE_SIZE_DEFAULT 96 /**< VirtIO 1.0: 96 on reset, guest can change */ +#define VIRTIOSCSI_CDB_SIZE_DEFAULT 32 /**< VirtIO 1.0: 32 on reset, guest can change */ +#define VIRTIOSCSI_PI_BYTES_IN 1 /**< Value TBD (see section 5.6.6.1) */ +#define VIRTIOSCSI_PI_BYTES_OUT 1 /**< Value TBD (see section 5.6.6.1) */ +#define VIRTIOSCSI_DATA_OUT 512 /**< Value TBD (see section 5.6.6.1) */ + /** * VirtIO SCSI Host Device device-specific queue indicies @@ -91,5 +103,5 @@ #define VIRTQ_REQ_BASE 2 /**< Spec-defined base index of request queues */ -#define QUEUENAME(qIdx) (pThis->szQueueNames[qIdx]) /**< Macro to get queue name from its index */ +#define QUEUENAME(qIdx) (pThis->szQueueNames[qIdx]) /**< Macro to get queue name from its index */ #define CBQUEUENAME(qIdx) RTStrNLen(QUEUENAME(qIdx), sizeof(QUEUENAME(qIdx))) @@ -144,17 +156,10 @@ /** @} */ + +#pragma pack(1) + /** * Device operation: reqestq - * - * The following struct is described in VirtIO 1.0 Specification, section 5.6.6.1 - */ -#define VIRTIOSCSI_SENSE_SIZE_DEFAULT 96 /**< VirtIO 1.0: 96 on reset, guest can change */ -#define VIRTIOSCSI_CDB_SIZE_DEFAULT 32 /**< VirtIO 1.0: 32 on reset, guest can change */ -#define VIRTIOSCSI_PI_BYTES_IN 1 /**< Value TBD (see section 5.6.6.1) */ -#define VIRTIOSCSI_PI_BYTES_OUT 1 /**< Value TBD (see section 5.6.6.1) */ -#define VIRTIOSCSI_DATA_OUT 512 /**< Value TBD (see section 5.6.6.1) */ - -#pragma pack(1) - + */ struct REQ_CMD_HDR { @@ -804,5 +809,14 @@ } -static int virtioScsiScrapReq(PVIRTIOSCSI pThis, uint16_t qIdx, uint32_t rcReq) +/** + * This is called to complete a request buffer immediately + * + * @param pThis - PDM driver instance state + * @param qIdx - Queue index + * @param rcReq - VirtualBox code to map to VirtIO response code. + * + * @returns virtual box status code + */ +static int virtioScsiReqComplete(PVIRTIOSCSI pThis, uint16_t qIdx, uint32_t rcReq) { struct REQ_RESP_HDR respHdr; @@ -815,5 +829,5 @@ RTSGSEG aReqSegs[] = { { &respHdr, sizeof(respHdr) } }; RTSgBufInit(&reqSegBuf, aReqSegs, 1); - virtioQueuePut(pThis->hVirtio, qIdx, &reqSegBuf, false /* fFence */); + virtioQueuePut(pThis->hVirtio, qIdx, &reqSegBuf, true /* fFence */); virtioQueueSync(pThis->hVirtio, qIdx); LogFunc(("Response code: %s\n", virtioGetReqRespText(respHdr.uResponse))); @@ -825,7 +839,5 @@ PVIRTIOSCSITARGET pTarget = pReq->pTarget; PPDMIMEDIAEX pIMediaEx = pTarget->pDrvMediaEx; - RTSGBUF reqSegBuf; - - Assert(pReq->pbDataIn && pReq->pbSense); + ASMAtomicDecU32(&pTarget->cReqsInProgress); @@ -833,4 +845,10 @@ int rc = pIMediaEx->pfnIoReqQueryResidual(pIMediaEx, pReq->hIoReq, &cbResidual); AssertRC(rc); Assert(cbResidual == (uint32_t)cbResidual); + + /** + * Linux does not want any sense code if there wasn't problem! + */ + if (pReq->pbSense[2] == SCSI_SENSE_NONE) + pReq->cbSense = 0; struct REQ_RESP_HDR respHdr; @@ -856,23 +874,25 @@ LogFunc(("Residual: %d\n", cbResidual)); - if (pReq->pbPiIn) - { - RTSGSEG aReqSegs[] = { - { &respHdr, sizeof(respHdr) }, - { pReq->pbSense, pReq->cbSense }, - { pReq->pbPiIn, pReq->cbPiIn }, - { pReq->pbDataIn, pReq->cbDataIn } - }; - RTSgBufInit(&reqSegBuf, aReqSegs, sizeof(aReqSegs) / sizeof(RTSGSEG)); - } - else /** Much easier not to include piIn sgbuf than to account for it during copy! */ - { - RTSGSEG aReqSegs[] = { - { &respHdr, sizeof(respHdr) }, - { pReq->pbSense, pReq->cbSense }, - { pReq->pbDataIn, pReq->cbDataIn } - }; - RTSgBufInit(&reqSegBuf, aReqSegs, sizeof(aReqSegs) / sizeof(RTSGSEG)); - } + int cSegs = 0; + RTSGSEG aReqSegs[4]; + aReqSegs[cSegs].pvSeg = &respHdr; + aReqSegs[cSegs++].cbSeg = sizeof(respHdr); + if (pReq->cbSense) + { + aReqSegs[cSegs].pvSeg = pReq->pbSense; + aReqSegs[cSegs++].cbSeg = pReq->cbSense; + } + if (pReq->cbPiIn) + { + aReqSegs[cSegs].pvSeg = pReq->pbPiIn; + aReqSegs[cSegs++].cbSeg = pReq->cbPiIn; + } + if (pReq->cbDataIn) + { + aReqSegs[cSegs].pvSeg = pReq->pbDataIn; + aReqSegs[cSegs++].cbSeg = pReq->cbDataIn; + } + RTSGBUF reqSegBuf; + RTSgBufInit(&reqSegBuf, aReqSegs, cSegs); /** @@ -890,5 +910,5 @@ * VirtIO 1.0 Spec requires mem. barrier for ctrl cmds * but doesn't mention fences in regard to requests. */ - virtioQueuePut(pThis->hVirtio, pReq->qIdx, &reqSegBuf, false /* fFence */); + virtioQueuePut(pThis->hVirtio, pReq->qIdx, &reqSegBuf, true /* fFence TBD */); virtioQueueSync(pThis->hVirtio, pReq->qIdx); @@ -963,5 +983,10 @@ if (uTarget >= pThis->cTargets) { - virtioScsiScrapReq(pThis, qIdx, VERR_IO_BAD_UNIT); + virtioScsiReqComplete(pThis, qIdx, VERR_IO_BAD_UNIT); + return VINF_SUCCESS; + } + if (uLUN != 0) + { + virtioScsiReqComplete(pThis, qIdx, VERR_IO_BAD_UNIT); return VINF_SUCCESS; } @@ -1023,12 +1048,19 @@ AssertMsgReturn(pReq->pbPiIn, ("Out of memory allocating pi_in buffer"), VERR_NO_MEMORY); } - pReq->cbDataIn = cbDataIn; - pReq->pbDataIn = (uint8_t *)RTMemAllocZ(cbDataIn); - AssertMsgReturn(pReq->pbDataIn, ("Out of memory allocating datain buffer"), VERR_NO_MEMORY); - - pReq->cbSense = cbSense; - pReq->pbSense = (uint8_t *)RTMemAllocZ(cbSense); - pReq->pInSgBuf = pInSgBuf; - AssertMsgReturn(pReq->pbSense, ("Out of memory allocating sense buffer"), VERR_NO_MEMORY); + + if (cbDataIn) + { + pReq->cbDataIn = cbDataIn; + pReq->pbDataIn = (uint8_t *)RTMemAllocZ(cbDataIn); + AssertMsgReturn(pReq->pbDataIn, ("Out of memory allocating datain buffer"), VERR_NO_MEMORY); + } + + if (cbSense) + { + pReq->cbSense = cbSense; + pReq->pbSense = (uint8_t *)RTMemAllocZ(cbSense); + pReq->pInSgBuf = pInSgBuf; + AssertMsgReturn(pReq->pbSense, ("Out of memory allocating sense buffer"), VERR_NO_MEMORY); + } LogFunc(("Submitting req (target=%d, LUN=%x) on %s\n", uTarget, uLUN, QUEUENAME(qIdx))); @@ -1040,9 +1072,10 @@ if (rc != VINF_PDM_MEDIAEX_IOREQ_IN_PROGRESS) { - virtioScsiScrapReq(pThis, qIdx, rc); + pIMediaEx->pfnIoReqFree(pIMediaEx, hIoReq); + virtioScsiReqComplete(pThis, qIdx, rc); return VINF_SUCCESS; } } else { - virtioScsiScrapReq(pThis, qIdx, VERR_IO_NOT_READY); + virtioScsiReqComplete(pThis, qIdx, VERR_IO_NOT_READY); return VINF_SUCCESS;