Index: /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp =================================================================== --- /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 81121) +++ /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (revision 81122) @@ -27,4 +27,5 @@ #include #include +#include #include #include @@ -79,12 +80,23 @@ * TEMPORARY NOTE: following parameter is set to 1 for early development. Will be increased later */ -#define VIRTIOSCSI_REQ_QUEUE_CNT 2 /**< 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_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? */ +//#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 should be 0 */ +//#define VIRTIOSCSI_SAVED_STATE_MINOR_VERSION 0x01 /**< SSM version # */ + + +#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_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? */ -#define VIRTIOSCSI_MAX_SEG_COUNT 1024 /* < T.B.D. What is a good value for this? */ +#define VIRTIOSCSI_MAX_COMMANDS_PER_LUN 128 /* < T.B.D. What is a good value for this? */ +#define VIRTIOSCSI_MAX_SEG_COUNT 126 /* < 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 should be 0 */ #define VIRTIOSCSI_SAVED_STATE_MINOR_VERSION 0x01 /**< SSM version # */ + #define PCI_DEVICE_ID_VIRTIOSCSI_HOST 0x1048 /**< Informs guest driver of type of VirtIO device */ @@ -169,8 +181,9 @@ pMediaExTxDirEnumValue == PDMMEDIAEXIOREQSCSITXDIR_TO_DEVICE /** - * 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 + * The following struct is the VirtIO SCSI Host Device device-specific configuration described + * in section 5.6.4 of the VirtIO 1.0 spec. The VBox VirtIO framework calls back to this driver + * to handle MMIO accesses to the device-specific configuration parameters whenever any bytes in the + * device-specific region areaccessed, since which the generic portion shouldn't know anything about + * the device-specific VirtIO cfg data. */ typedef struct virtio_scsi_config @@ -187,5 +200,4 @@ uint32_t uMaxLun; /**< max_lun Hint to guest driver */ } VIRTIOSCSI_CONFIG_T, PVIRTIOSCSI_CONFIG_T; - /** @@ -479,7 +491,4 @@ R3PTRTYPE(PPDMILEDCONNECTORS) pLedsConnector; - /** Base address of the memory mapping. */ - RTGCPHYS GCPhysMMIOBase; - /** IMediaExPort: Media ejection notification */ R3PTRTYPE(PPDMIMEDIANOTIFY) pMediaNotify; @@ -493,7 +502,4 @@ /** Mask of VirtIO Async Event types this device will deliver */ uint32_t uAsyncEvtsEnabled; - - /** The event semaphore the processing thread waits on. */ - /** Total number of requests active across all targets */ @@ -1002,5 +1008,4 @@ || (VIRTIO_OUT_DIRECTION(pReq->enmTxDir) && cbXfer32 > pReq->cbDataOut)) { - /* TBD try to figure out optimal sense info to send back besides response of VIRTIOSCSI_S_OVERRUN */ Log2Func((" * * * * Data overrun, returning sense\n")); uint8_t abSense[] = { RT_BIT(7) | SCSI_SENSE_RESPONSE_CODE_CURR_FIXED, @@ -1038,8 +1043,4 @@ RTSgBufInit(&reqSegBuf, aReqSegs, cSegs); - /* - * Fill in the request queue current descriptor chain's IN queue entry/entries - * (phys. memory) with the Req response data in virtual memory. - */ size_t cbReqSgBuf = RTSgBufCalcTotalLength(&reqSegBuf); AssertMsgReturn(cbReqSgBuf <= pReq->pDescChain->cbPhysDst, @@ -1104,5 +1105,5 @@ * Handle submission errors */ - if (pThis->fResetting) + if (RT_UNLIKELY(pThis->fResetting)) { Log2Func(("Aborting req submission because reset is in progress\n")); @@ -1116,5 +1117,5 @@ } else - if (uTarget >= pThis->cTargets || uScsiLun != 0) + if (RT_UNLIKELY(uTarget >= pThis->cTargets || uScsiLun != 0)) { Log2Func(("Error submitting request to bad target (%d) or bad LUN (%d)\n", uTarget, uScsiLun)); @@ -1140,4 +1141,18 @@ respHdr.uStatus = SCSI_STATUS_CHECK_CONDITION; respHdr.uResponse = VIRTIOSCSI_S_TARGET_FAILURE; + respHdr.uResidual = cbDataIn + cbDataOut; + virtioScsiReqErr(pThis, qIdx, pDescChain, &respHdr , abSense); + return VINF_SUCCESS; + } + else + if (RT_UNLIKELY(cbDataIn && cbDataOut && !pThis->fHasInOutBufs)) /* VirtIO 1.0, 5.6.6.1.1 */ + { + Log2Func(("Error submitting request, got datain & dataout bufs w/o INOUT feature negotated\n")); + uint8_t abSense[] = { RT_BIT(7) | SCSI_SENSE_RESPONSE_CODE_CURR_FIXED, + 0, SCSI_SENSE_ILLEGAL_REQUEST, 0, 0, 0, 0, 10, 0, 0, 0 }; + struct REQ_RESP_HDR respHdr = { 0 }; + respHdr.uSenseLen = sizeof(abSense); + respHdr.uStatus = SCSI_STATUS_CHECK_CONDITION; + respHdr.uResponse = VIRTIOSCSI_S_FAILURE; respHdr.uResidual = cbDataIn + cbDataOut; virtioScsiReqErr(pThis, qIdx, pDescChain, &respHdr , abSense); @@ -1758,6 +1773,6 @@ /* Wake worker threads flagged to skip pulling queue entries during quiesce - * to ensure they re-check their queues. Active request queues may already - * be awake due to new reqs coming in. + * to ensure they re-check their queues. Active request queues may already + * be awake due to new reqs coming in. */ for (uint16_t qIdx = 0; qIdx < VIRTIOSCSI_REQ_QUEUE_CNT; qIdx++) @@ -2386,5 +2401,5 @@ /* .cbInstanceRC = */ 0, /* .cMaxPciDevices = */ 1, - /* .cMaxMsixVectors = */ 0, + /* .cMaxMsixVectors = */ VBOX_MSIX_MAX_ENTRIES, /* .pszDescription = */ "Virtio Host SCSI.\n", #if defined(IN_RING3) Index: /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp =================================================================== --- /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp (revision 81121) +++ /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp (revision 81122) @@ -366,31 +366,23 @@ AssertMsgReturnVoid(DRIVER_OK(pVirtio), ("Guest driver not in ready state.\n")); - - if (pVirtio->uMsixConfig == VIRTIO_MSI_NO_VECTOR) - { - if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX) - { - if (pVirtq->fEventThresholdReached) - { - virtioRaiseInterrupt(pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, fForce); - pVirtq->fEventThresholdReached = false; - return; - } - Log6Func(("...skipping interrupt: VIRTIO_F_EVENT_IDX set but threshold not reached\n")); - } - else - { - /** If guest driver hasn't suppressed interrupts, interrupt */ - if (fForce || !(virtioReadUsedFlags(pVirtio, qIdx) & VIRTQ_AVAIL_F_NO_INTERRUPT)) - { - virtioRaiseInterrupt(pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, fForce); - return; - } - Log6Func(("...skipping interrupt. Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n")); - } - } - else - { - /* TBD, do MSI notification if criteria met */ + if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX) + { + if (pVirtq->fEventThresholdReached) + { + virtioKick(pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[qIdx], fForce); + pVirtq->fEventThresholdReached = false; + return; + } + Log6Func(("...skipping interrupt: VIRTIO_F_EVENT_IDX set but threshold not reached\n")); + } + else + { + /** If guest driver hasn't suppressed interrupts, interrupt */ + if (fForce || !(virtioReadUsedFlags(pVirtio, qIdx) & VIRTQ_AVAIL_F_NO_INTERRUPT)) + { + virtioKick(pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[qIdx], fForce); + return; + } + Log6Func(("...skipping interrupt. Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n")); } } @@ -425,11 +417,14 @@ } -/** - * Raise interrupt. + +/** + * Raise interrupt or MSI-X * * @param pVirtio The device state structure. * @param uCause Interrupt cause bit mask to set in PCI ISR port. - */ -static int virtioRaiseInterrupt(PVIRTIOSTATE pVirtio, uint8_t uCause, bool fForce) + * @param uVec MSI-X vector, if enabled + * @param uForce True of out-of-band + */ +static int virtioKick(PVIRTIOSTATE pVirtio, uint8_t uCause, uint16_t uMsixVector, bool fForce) { @@ -443,6 +438,14 @@ Log6Func(("reason: device config change\n")); - pVirtio->uISR |= uCause; - PDMDevHlpPCISetIrq(pVirtio->CTX_SUFF(pDevIns), 0, 1); + if (!pVirtio->fMsiSupport) + { + pVirtio->uISR |= uCause; + PDMDevHlpPCISetIrq(pVirtio->CTX_SUFF(pDevIns), 0, PDM_IRQ_LEVEL_HIGH); + } + else if (uMsixVector != VIRTIO_MSI_NO_VECTOR) + { + Log6Func(("MSI-X enabled, calling PDMDevHlpPCISetIrq with vector: 0x%x\n", uMsixVector)); + PDMDevHlpPCISetIrq(pVirtio->CTX_SUFF(pDevIns), uMsixVector, 1); + } return VINF_SUCCESS; } @@ -455,5 +458,5 @@ static void virtioLowerInterrupt(PVIRTIOSTATE pVirtio) { - PDMDevHlpPCISetIrq(pVirtio->CTX_SUFF(pDevIns), 0, 0); + PDMDevHlpPCISetIrq(pVirtio->CTX_SUFF(pDevIns), 0, PDM_IRQ_LEVEL_LOW); } @@ -466,6 +469,9 @@ pVirtio->uQueueSize[qIdx] = VIRTQ_MAX_SIZE; pVirtio->uQueueNotifyOff[qIdx] = qIdx; -} - + + pVirtio->uQueueMsixVector[qIdx] = qIdx + 2; + if (!pVirtio->fMsiSupport) /* VirtIO 1.0, 4.1.4.3 and 4.1.5.1.2 */ + pVirtio->uQueueMsixVector[qIdx] = VIRTIO_MSI_NO_VECTOR; +} static void virtioResetDevice(PVIRTIOSTATE pVirtio) @@ -478,10 +484,7 @@ pVirtio->uISR = 0; -#ifndef MSIX_SUPPORT - /** This is required by VirtIO 1.0 specification, section 4.1.5.1.2 */ - pVirtio->uMsixConfig = VIRTIO_MSI_NO_VECTOR; - for (int i = 0; i < VIRTQ_MAX_CNT; i++) - pVirtio->uQueueMsixVector[i] = VIRTIO_MSI_NO_VECTOR; -#endif + + if (!pVirtio->fMsiSupport) /* VirtIO 1.0, 4.1.4.3 and 4.1.5.1.2 */ + pVirtio->uMsixConfig = VIRTIO_MSI_NO_VECTOR; pVirtio->uNumQueues = VIRTQ_MAX_CNT; @@ -523,5 +526,5 @@ { pVirtio->fGenUpdatePending = true; - virtioRaiseInterrupt(pVirtio, VIRTIO_ISR_DEVICE_CONFIG, false /* fForce */); + virtioKick(pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig, false /* fForce */); } } @@ -529,5 +532,5 @@ /** * Invoked by this implementation when guest driver resets the device. - * The driver itself will not reset until the device has read the status change. + * The driver itself will not until the device has read the status change. */ static void virtioGuestResetted(PVIRTIOSTATE pVirtio) @@ -1015,5 +1018,5 @@ VIRTIOHANDLE *phVirtio, PVIRTIOPCIPARAMS pPciParams, - const char *pcszInstance, + const char *pcszInstance, uint64_t uDevSpecificFeatures, PFNVIRTIODEVCAPREAD devCapReadCallback, @@ -1028,4 +1031,7 @@ void *pDevSpecificCfg) { + + extern PDMDEVREG g_DeviceVirtioSCSI; + PVIRTIOSTATE pVirtio = (PVIRTIOSTATE)RTMemAllocZ(sizeof(VIRTIOSTATE)); if (!pVirtio) @@ -1034,4 +1040,8 @@ return VERR_NO_MEMORY; } + +#ifdef VBOX_WITH_MSI_DEVICES + pVirtio->fMsiSupport = true; +#endif pVirtio->pClientContext = pClientContext; @@ -1109,10 +1119,4 @@ /* Construct & map PCI vendor-specific capabilities for virtio host negotiation with guest driver */ - -#if 0 && defined(VBOX_WITH_MSI_DEVICES) /* T.B.D. */ - uint8_t fMsiSupport = true; -#else - uint8_t fMsiSupport = false; -#endif /* The following capability mapped via VirtIO 1.0: struct virtio_pci_cfg_cap (VIRTIO_PCI_CFG_CAP_T) @@ -1184,5 +1188,5 @@ pCfg->uCapVndr = VIRTIO_PCI_CAP_ID_VENDOR; pCfg->uCapLen = sizeof(VIRTIO_PCI_CFG_CAP_T); - pCfg->uCapNext = (fMsiSupport || pVirtio->pDevSpecificCfg) ? CFGADDR2IDX(pCfg) + pCfg->uCapLen : 0; + pCfg->uCapNext = (pVirtio->fMsiSupport || pVirtio->pDevSpecificCfg) ? CFGADDR2IDX(pCfg) + pCfg->uCapLen : 0; pCfg->uBar = 0; pCfg->uOffset = 0; @@ -1199,5 +1203,5 @@ pCfg->uCapVndr = VIRTIO_PCI_CAP_ID_VENDOR; pCfg->uCapLen = sizeof(VIRTIO_PCI_CAP_T); - pCfg->uCapNext = fMsiSupport ? CFGADDR2IDX(pCfg) + pCfg->uCapLen : 0; + pCfg->uCapNext = pVirtio->fMsiSupport ? CFGADDR2IDX(pCfg) + pCfg->uCapLen : 0; pCfg->uBar = VIRTIO_REGION_PCI_CAP; pCfg->uOffset = pVirtio->pIsrCap->uOffset + pVirtio->pIsrCap->uLength; @@ -1208,9 +1212,5 @@ } - /* Set offset to first capability and enable PCI dev capabilities */ - PDMPciDevSetCapabilityList(pPciDev, 0x40); - PDMPciDevSetStatus(pPciDev, VBOX_PCI_STATUS_CAP_LIST); - - if (fMsiSupport) + if (pVirtio->fMsiSupport) { PDMMSIREG aMsiReg; @@ -1218,11 +1218,23 @@ aMsiReg.iMsixCapOffset = pCfg->uCapNext; aMsiReg.iMsixNextOffset = 0; - aMsiReg.iMsixBar = 0; - aMsiReg.cMsixVectors = 1; + aMsiReg.iMsixBar = VIRTIO_REGION_MSIX_CAP; + aMsiReg.cMsixVectors = g_DeviceVirtioSCSI.cMaxMsixVectors; rc = PDMDevHlpPCIRegisterMsi(pDevIns, &aMsiReg); /* see MsixR3init() */ - if (RT_FAILURE (rc)) - /* The following is moot, we need to flag no MSI-X support */ - PDMPciDevSetCapabilityList(pPciDev, 0x40); - } + if (RT_FAILURE(rc)) + { + /* See PDMDevHlp.cpp:pdmR3DevHlp_PCIRegisterMsi */ + Log(("Failed to configure MSI-X (%Rrc). Reverting to INTx\n")); + pVirtio->fMsiSupport = false; + } + else + Log(("Using MSI-X for guest driver notification\n")); + } + else + Log(("MSI-X not available for VBox, using INTx notification\n")); + + + /* Set offset to first capability and enable PCI dev capabilities */ + PDMPciDevSetCapabilityList(pPciDev, 0x40); + PDMPciDevSetStatus(pPciDev, VBOX_PCI_STATUS_CAP_LIST); /* Linux drivers/virtio/virtio_pci_modern.c tries to map at least a page for the Index: /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.h =================================================================== --- /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.h (revision 81121) +++ /trunk/src/VBox/Devices/VirtIO/Virtio_1_0.h (revision 81122) @@ -39,4 +39,5 @@ #define VIRTIO_NOTIFY_OFFSET_MULTIPLIER 2 /**< VirtIO Notify Cap. MMIO config param */ #define VIRTIO_REGION_PCI_CAP 2 /**< BAR for VirtIO Cap. MMIO (impl specific) */ +#define VIRTIO_REGION_MSIX_CAP 0 /**< Bar for MSI-X handling */ #define VIRTIO_HEX_DUMP(logLevel, pv, cb, base, title) \ @@ -163,6 +164,4 @@ /** * Allocate client context for client to work with VirtIO-provided with queue - * As a side effect creates a buffer vector a client can get a pointer to - * with a call to virtioQueueDescChain() * * @param hVirtio - Handle to VirtIO framework @@ -211,5 +210,5 @@ * The caller passes in a pointer to a scatter-gather buffer of virtual memory segments * and a pointer to the descriptor chain context originally derived from the pulled - * queue entry, and this function will put write the virtual memory s/g buffer into the + * queue entry, and this function will write the virtual memory s/g buffer into the * guest's physical memory free the descriptor chain. The caller handles the freeing * (as needed) of the virtual memory buffer. Index: /trunk/src/VBox/Devices/VirtIO/Virtio_1_0_impl.h =================================================================== --- /trunk/src/VBox/Devices/VirtIO/Virtio_1_0_impl.h (revision 81121) +++ /trunk/src/VBox/Devices/VirtIO/Virtio_1_0_impl.h (revision 81122) @@ -189,4 +189,5 @@ uint8_t uPciCfgDataOff; uint8_t uISR; /**< Interrupt Status Register. */ + uint8_t fMsiSupport; } VIRTIOSTATE, *PVIRTIOSTATE; @@ -520,6 +521,5 @@ static void virtioResetQueue (PVIRTIOSTATE pVirtio, uint16_t qIdx); static void virtioNotifyGuestDriver (PVIRTIOSTATE pVirtio, uint16_t qIdx, bool fForce); -static int virtioRaiseInterrupt (PVIRTIOSTATE pVirtio, uint8_t uCause, bool fForce); -static void virtioLowerInterrupt (PVIRTIOSTATE pVirtio); +static int virtioKick(PVIRTIOSTATE pVirtio, uint8_t uCause, uint16_t uVec, bool fForce); static void virtioQueueNotified (PVIRTIOSTATE pVirtio, uint16_t qidx, uint16_t uDescIdx); static void virtioGuestResetted (PVIRTIOSTATE pVirtio);