Index: /trunk/include/VBox/pdmifs.h =================================================================== --- /trunk/include/VBox/pdmifs.h (revision 24097) +++ /trunk/include/VBox/pdmifs.h (revision 24098) @@ -2556,8 +2556,19 @@ * * @returns VBox status code. - * @param pInterface Pointer to this interface. - * @param pSCSIRequest Pointer to the SCSI request to execute. + * @param pInterface Pointer to this interface. + * @param pSCSIRequest Pointer to the SCSI request to execute. */ DECLR3CALLBACKMEMBER(int, pfnSCSIRequestSend, (PPDMISCSICONNECTOR pInterface, PPDMSCSIREQUEST pSCSIRequest)); + + /** + * Wait for all current requests to complete. + * + * This can only be called while the VM is running. + * + * @returns VINF_SUCCESS, VERR_NO_MEMORY, VERR_INVALID_STATE or VERR_TIMEOUT. + * @param pInterface Pointer to this interface. + * @param cMillies The timeout in milliseconds or RT_INDEFINITE_WAIT. + */ + DECLR3CALLBACKMEMBER(int, pfnSyncronizeRequests, (PPDMISCSICONNECTOR pInterface, uint32_t cMillies)); } PDMISCSICONNECTOR; Index: /trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp =================================================================== --- /trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp (revision 24097) +++ /trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp (revision 24098) @@ -2381,5 +2381,5 @@ pThis->cMaxBuses = 1; pThis->cbReplyFrame = 128; /* @todo Figure out where it is needed. */ - /* @todo: Put stuff to reset here. */ + /** @todo: Put stuff to reset here. */ lsilogicInitializeConfigurationPages(pThis); @@ -4786,10 +4786,10 @@ * @retruns Flag which indicates if all I/O completed in the given timeout. * @param pLsiLogic Pointer to the dveice instance to check. - * @param cMillis Timeout in milliseconds to wait. + * @param cMillis Timeout in milliseconds to wait per device. */ static bool lsilogicWaitForAsyncIOFinished(PLSILOGICSCSI pLsiLogic, unsigned cMillies) { uint64_t u64Start; - bool fIdle; + unsigned i; /* @@ -4799,25 +4799,20 @@ for (;;) { - fIdle = true; - /* Check every port. */ - for (unsigned i = 0; i < RT_ELEMENTS(pLsiLogic->aDeviceStates); i++) - { - PLSILOGICDEVICE pLsiLogicDevice = &pLsiLogic->aDeviceStates[i]; - if (ASMAtomicReadU32(&pLsiLogicDevice->cOutstandingRequests)) - { - fIdle = false; + for (i = 0; i < RT_ELEMENTS(pLsiLogic->aDeviceStates); i++) + if (ASMAtomicReadU32(&pLsiLogic->aDeviceStates[i].cOutstandingRequests)) break; - } - } - if ( fIdle - || RTTimeMilliTS() - u64Start >= cMillies) - break; - - /* Sleep for a bit. */ - RTThreadSleep(100); /** @todo wait on something which can be woken up. 100ms is too long for teleporting VMs! */ - } - - return fIdle; + if (i >= RT_ELEMENTS(pLsiLogic->aDeviceStates)) + return true; + + uint64_t cMsElapsed = RTTimeMilliTS() - u64Start; + if (cMsElapsed >= cMillies) + return false; + + /* Try synchronize the request queue for this device. */ + PPDMISCSICONNECTOR pDrvSCSIConnector = pLsiLogic->aDeviceStates[i].pDrvSCSIConnector; + uint32_t cMsLeft = cMillies - (uint32_t)cMsElapsed; + pDrvSCSIConnector->pfnSyncronizeRequests(pDrvSCSIConnector, RT_MAX(cMsLeft, 100)); + } } @@ -4833,24 +4828,10 @@ } -static DECLCALLBACK(int) lsilogicSaveLoadPrep(PPDMDEVINS pDevIns, PSSMHANDLE pSSM) +static DECLCALLBACK(int) lsilogicSaveExec(PPDMDEVINS pDevIns, PSSMHANDLE pSSM) { PLSILOGICSCSI pLsiLogic = PDMINS_2_DATA(pDevIns, PLSILOGICSCSI); - /* Wait that no task is pending on any device. */ - if (!lsilogicWaitForAsyncIOFinished(pLsiLogic, 20000)) - { - AssertLogRelMsgFailed(("LsiLogic: There are still tasks outstanding\n")); - return VERR_TIMEOUT; - } - - return VINF_SUCCESS; -} - -static DECLCALLBACK(int) lsilogicSaveExec(PPDMDEVINS pDevIns, PSSMHANDLE pSSM) -{ - PLSILOGICSCSI pLsiLogic = PDMINS_2_DATA(pDevIns, PLSILOGICSCSI); - /* Every device first. */ - lsilogicSaveLoadPrep(pDevIns, pSSM); + lsilogicLiveExec(pDevIns, pSSM, SSM_PASS_FINAL); for (unsigned i = 0; i < RT_ELEMENTS(pLsiLogic->aDeviceStates); i++) { @@ -5172,26 +5153,4 @@ /** - * @copydoc FNPDMDEVPOWEROFF - */ -static DECLCALLBACK(void) lsilogicPowerOff(PPDMDEVINS pDevIns) -{ - PLSILOGICSCSI pLsiLogic = PDMINS_2_DATA(pDevIns, PLSILOGICSCSI); - - bool fIdle = lsilogicWaitForAsyncIOFinished(pLsiLogic, 20000); - Assert(fIdle); -} - -/** - * @copydoc FNPDMDEVSUSPEND - */ -static DECLCALLBACK(void) lsilogicSuspend(PPDMDEVINS pDevIns) -{ - PLSILOGICSCSI pLsiLogic = PDMINS_2_DATA(pDevIns, PLSILOGICSCSI); - - bool fIdle = lsilogicWaitForAsyncIOFinished(pLsiLogic, 20000); - Assert(fIdle); -} - -/** * @copydoc FNPDMDEVRESET */ @@ -5201,4 +5160,17 @@ int rc; +/** @todo r=bird: this is a deadlock trap. We're EMT(0), if there are + * outstanding requests they may require EMT interaction because of + * physical write backs around lsilogicDeviceSCSIRequestCompleted... + * + * I have some more generic solution for delayed suspend, reset and + * poweroff handling that I'm considering. The idea is that the + * notification callback returns a status indicating that it didn't + * complete and needs to be called again or something. EMT continues on + * the next device and when it's done, it processes incoming requests and + * does another notification round... This way we could combine the waits + * in the I/O controllers and reduce the time it takes to suspend a VM a + * wee bit... + */ bool fIdle = lsilogicWaitForAsyncIOFinished(pLsiLogic, 20000); Assert(fIdle); @@ -5449,8 +5421,6 @@ /* Register save state handlers. */ - rc = PDMDevHlpSSMRegisterEx(pDevIns, LSILOGIC_SAVED_STATE_VERSION, sizeof(*pThis), NULL, - NULL, lsilogicLiveExec, NULL, - lsilogicSaveLoadPrep, lsilogicSaveExec, NULL, - lsilogicSaveLoadPrep, lsilogicLoadExec, NULL); + rc = PDMDevHlpSSMRegister3(pDevIns, LSILOGIC_SAVED_STATE_VERSION, sizeof(*pThis), + lsilogicLiveExec, lsilogicSaveExec, lsilogicLoadExec); if (RT_FAILURE(rc)) return PDMDEV_SET_ERROR(pDevIns, rc, N_("LsiLogic cannot register save state handlers")); @@ -5503,5 +5473,5 @@ lsilogicReset, /* pfnSuspend */ - lsilogicSuspend, + NULL, /* pfnResume */ NULL, @@ -5515,5 +5485,5 @@ NULL, /* pfnPowerOff */ - lsilogicPowerOff, + NULL, /* pfnSoftReset */ NULL, Index: /trunk/src/VBox/Devices/Storage/DrvSCSI.cpp =================================================================== --- /trunk/src/VBox/Devices/Storage/DrvSCSI.cpp (revision 24097) +++ /trunk/src/VBox/Devices/Storage/DrvSCSI.cpp (revision 24098) @@ -671,4 +671,14 @@ /** + * Request function to syncronize the request execution. + * + * @returns VINF_SUCCESS. + */ +static int drvscsiAsyncIOLoopSyncCallback(void) +{ + return VINF_SUCCESS; +} + +/** * Request function to wakeup the thread. * @@ -716,4 +726,7 @@ rc = RTReqCall(pThis->pQueueRequests, &pReq, 10000 /* 10 sec. */, (PFNRT)drvscsiAsyncIOLoopWakeupFunc, 0); AssertMsgRC(rc, ("Inserting request into queue failed rc=%Rrc\n", rc)); + if (RT_SUCCESS(rc)) + RTReqFree(pReq); + /*else: leak it */ return rc; @@ -735,4 +748,23 @@ return VINF_SUCCESS; +} + +/** @copydoc PDMISCSICONNECTOR::pfnSyncronizeRequests. */ +static DECLCALLBACK(int) drvscsiSyncronizeRequests(PPDMISCSICONNECTOR pInterface, uint32_t cMillies) +{ + int rc; + PDRVSCSI pThis = PDMISCSICONNECTOR_2_DRVSCSI(pInterface); + PRTREQ pReq; + + Assert(cMillies > 100); + AssertReturn(pThis->pQueueRequests, VERR_INVALID_STATE); + + rc = RTReqCall(pThis->pQueueRequests, &pReq, cMillies, (PFNRT)drvscsiAsyncIOLoopSyncCallback, 0); + AssertMsgRC(rc, ("Inserting request into queue failed rc=%Rrc\n", rc)); + if (RT_SUCCESS(rc)) + RTReqFree(pReq); + /*else: leak it */ + + return rc; } @@ -794,4 +826,5 @@ pDrvIns->IBase.pfnQueryInterface = drvscsiQueryInterface; pThis->ISCSIConnector.pfnSCSIRequestSend = drvscsiRequestSend; + pThis->ISCSIConnector.pfnSyncronizeRequests = drvscsiSyncronizeRequests; /* @@ -904,7 +937,7 @@ NULL, /* pfnDetach */ - NULL, + NULL, /* pfnPowerOff */ - NULL, + NULL, /* pfnSoftReset */ NULL,