Index: /trunk/src/VBox/Main/include/DisplayImpl.h =================================================================== --- /trunk/src/VBox/Main/include/DisplayImpl.h (revision 52651) +++ /trunk/src/VBox/Main/include/DisplayImpl.h (revision 52652) @@ -164,4 +164,11 @@ bool i_VideoAccelAllowed(void); void i_VideoAccelVRDP(bool fEnable); + + /* Legacy video acceleration requests coming from the VGA refresh timer. */ + int VideoAccelEnableVGA(bool fEnable, VBVAMEMORY *pVbvaMemory); + + /* Legacy video acceleration requests coming from VMMDev. */ + int VideoAccelEnableVMMDev(bool fEnable, VBVAMEMORY *pVbvaMemory); + void VideoAccelFlushVMMDev(void); int i_VideoCaptureStart(); @@ -375,17 +382,19 @@ void i_handleResizeCompletedEMT(unsigned uScreenId, BOOL fResizeContext); - /* Old guest additions (3.x?) use VMMDev for VBVA and the host VBVA code (VideoAccel*) - * can be executed concurrently by VGA refresh timer and the guest VMMDev request - * in SMP VMs. The lock serialized this. + /* Old guest additions (3.x and older) use both VMMDev and DevVGA refresh timer + * to process the VBVABUFFER memory. Therefore the legacy VBVA (VideoAccel) host + * code can be executed concurrently by VGA refresh timer and the guest VMMDev + * request in SMP VMs. The semaphore serialized this. */ - RTCRITSECT mVBVALock; - volatile uint32_t mfu32PendingVideoAccelDisable; - - int i_vbvaLock(void); - void i_vbvaUnlock(void); + RTSEMXROADS mhXRoadsVideoAccel; + int videoAccelEnterVGA(void); + void videoAccelLeaveVGA(void); + int videoAccelEnterVMMDev(void); + void videoAccelLeaveVMMDev(void); + + /* Serializes access to mpVbvaMemory, etc between VRDP and Display. */ + RTCRITSECT mVideoAccelLock; public: - - bool i_vbvaLockIsOwner(void); static int i_displayTakeScreenshotEMT(Display *pDisplay, ULONG aScreenId, uint8_t **ppu8Data, size_t *pcbData, Index: /trunk/src/VBox/Main/src-client/DisplayImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 52651) +++ /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 52652) @@ -123,8 +123,10 @@ mfVMMDevInited = false; - int rc = RTCritSectInit(&mVBVALock); + int rc = RTCritSectInit(&mVideoAccelLock); AssertRC(rc); - mfu32PendingVideoAccelDisable = false; + mhXRoadsVideoAccel = NIL_RTSEMXROADS; + rc = RTSemXRoadsCreate(&mhXRoadsVideoAccel); + AssertRC(rc); #ifdef VBOX_WITH_HGSMI @@ -160,8 +162,10 @@ uninit(); - if (RTCritSectIsInitialized (&mVBVALock)) - { - RTCritSectDelete (&mVBVALock); - RT_ZERO(mVBVALock); + RTSemXRoadsDestroy(mhXRoadsVideoAccel); + + if (RTCritSectIsInitialized(&mVideoAccelLock)) + { + RTCritSectDelete(&mVideoAccelLock); + RT_ZERO(mVideoAccelLock); } @@ -398,5 +402,4 @@ /* This can be called from any thread. */ - Assert(!that->i_vbvaLockIsOwner()); that->mpDrv->pUpPort->pfnFreeScreenshot(that->mpDrv->pUpPort, pu8Data); } @@ -876,5 +879,4 @@ if (uScreenId == VBOX_VIDEO_PRIMARY_SCREEN) { - Assert(!i_vbvaLockIsOwner()); mpDrv->pUpPort->pfnSetRenderVRAM(mpDrv->pUpPort, false); @@ -1521,17 +1523,60 @@ } -int Display::i_vbvaLock(void) -{ - return RTCritSectEnter(&mVBVALock); -} - -void Display::i_vbvaUnlock(void) -{ - RTCritSectLeave(&mVBVALock); -} - -bool Display::i_vbvaLockIsOwner(void) -{ - return RTCritSectIsOwner(&mVBVALock); +int Display::videoAccelEnterVGA(void) +{ + return RTSemXRoadsNSEnter(mhXRoadsVideoAccel); +} + +void Display::videoAccelLeaveVGA(void) +{ + RTSemXRoadsNSLeave(mhXRoadsVideoAccel); +} + +int Display::videoAccelEnterVMMDev(void) +{ + return RTSemXRoadsEWEnter(mhXRoadsVideoAccel); +} + +void Display::videoAccelLeaveVMMDev(void) +{ + RTSemXRoadsEWLeave(mhXRoadsVideoAccel); +} + +int Display::VideoAccelEnableVMMDev(bool fEnable, VBVAMEMORY *pVbvaMemory) +{ + LogFlowFunc(("%d %p\n", fEnable, pVbvaMemory)); + int rc = videoAccelEnterVMMDev(); + if (RT_SUCCESS(rc)) + { + rc = i_VideoAccelEnable(fEnable, pVbvaMemory); + videoAccelLeaveVMMDev(); + } + LogFlowFunc(("leave %Rrc\n", rc)); + return rc; +} + +int Display::VideoAccelEnableVGA(bool fEnable, VBVAMEMORY *pVbvaMemory) +{ + LogFlowFunc(("%d %p\n", fEnable, pVbvaMemory)); + int rc = videoAccelEnterVGA(); + if (RT_SUCCESS(rc)) + { + rc = i_VideoAccelEnable(fEnable, pVbvaMemory); + videoAccelLeaveVGA(); + } + LogFlowFunc(("leave %Rrc\n", rc)); + return rc; +} + +void Display::VideoAccelFlushVMMDev(void) +{ + LogFlowFunc(("enter\n")); + int rc = videoAccelEnterVMMDev(); + if (RT_SUCCESS(rc)) + { + i_VideoAccelFlush(); + videoAccelLeaveVMMDev(); + } + LogFlowFunc(("leave\n")); } @@ -1542,21 +1587,9 @@ { int rc; - if (fEnable) - { - /* Process any pending VGA device changes, resize. */ - Assert(!i_vbvaLockIsOwner()); - mpDrv->pUpPort->pfnUpdateDisplayAll(mpDrv->pUpPort, /* fFailOnResize = */ false); - } - - i_vbvaLock(); + LogRelFlowFunc(("fEnable = %d\n", fEnable)); + rc = i_videoAccelEnable(fEnable, pVbvaMemory); - i_vbvaUnlock(); - - if (!fEnable) - { - Assert(!i_vbvaLockIsOwner()); - mpDrv->pUpPort->pfnUpdateDisplayAll(mpDrv->pUpPort, /* fFailOnResize = */ false); - } - + + LogRelFlowFunc(("%Rrc.\n", rc)); return rc; } @@ -1564,6 +1597,4 @@ int Display::i_videoAccelEnable(bool fEnable, VBVAMEMORY *pVbvaMemory) { - Assert(i_vbvaLockIsOwner()); - int rc = VINF_SUCCESS; /* Called each time the guest wants to use acceleration, @@ -1618,9 +1649,44 @@ mpVbvaMemory->fu32ModeFlags &= ~VBVA_F_MODE_ENABLED; - /* Safety precaution. There is no more VBVA until everything is setup! */ - mpVbvaMemory = NULL; - mfVideoAccelEnabled = false; - - /* Everything OK. VBVA status can be changed. */ + if (fEnable) + { + /* Process any pending VGA device changes, resize. */ + mpDrv->pUpPort->pfnUpdateDisplayAll(mpDrv->pUpPort, /* fFailOnResize = */ false); + } + + /* Protect the videoaccel state transition. */ + RTCritSectEnter(&mVideoAccelLock); + + if (fEnable) + { + mpVbvaMemory = pVbvaMemory; + mfVideoAccelEnabled = true; + + /* Initialize the hardware memory. */ + i_vbvaSetMemoryFlags(mpVbvaMemory, mfVideoAccelEnabled, mfVideoAccelVRDP, + mfu32SupportedOrders, maFramebuffers, mcMonitors); + mpVbvaMemory->off32Data = 0; + mpVbvaMemory->off32Free = 0; + + memset(mpVbvaMemory->aRecords, 0, sizeof(mpVbvaMemory->aRecords)); + mpVbvaMemory->indexRecordFirst = 0; + mpVbvaMemory->indexRecordFree = 0; + + LogRel(("VBVA: Enabled.\n")); + } + else + { + mpVbvaMemory = NULL; + mfVideoAccelEnabled = false; + + LogRel(("VBVA: Disabled.\n")); + } + + RTCritSectLeave(&mVideoAccelLock); + + if (!fEnable) + { + mpDrv->pUpPort->pfnUpdateDisplayAll(mpDrv->pUpPort, /* fFailOnResize = */ false); + } /* Notify the VMMDev, which saves VBVA status in the saved state, @@ -1635,30 +1701,5 @@ } - if (fEnable) - { - mpVbvaMemory = pVbvaMemory; - mfVideoAccelEnabled = true; - - /* Initialize the hardware memory. */ - i_vbvaSetMemoryFlags(mpVbvaMemory, mfVideoAccelEnabled, mfVideoAccelVRDP, - mfu32SupportedOrders, maFramebuffers, mcMonitors); - mpVbvaMemory->off32Data = 0; - mpVbvaMemory->off32Free = 0; - - memset(mpVbvaMemory->aRecords, 0, sizeof(mpVbvaMemory->aRecords)); - mpVbvaMemory->indexRecordFirst = 0; - mpVbvaMemory->indexRecordFree = 0; - - mfu32PendingVideoAccelDisable = false; - - LogRel(("VBVA: Enabled.\n")); - } - else - { - LogRel(("VBVA: Disabled.\n")); - } - - LogRelFlowFunc(("VideoAccelEnable: rc = %Rrc.\n", rc)); - + LogRelFlowFunc(("%Rrc.\n", rc)); return rc; } @@ -1670,6 +1711,4 @@ LogRelFlowFunc(("fEnable = %d\n", fEnable)); - i_vbvaLock(); - int c = fEnable? ASMAtomicIncS32(&mcVideoAccelVRDPRefs): @@ -1677,4 +1716,7 @@ Assert (c >= 0); + + /* This can run concurrently with Display videoaccel state change. */ + RTCritSectEnter(&mVideoAccelLock); if (c == 0) @@ -1724,5 +1766,6 @@ Assert(mfVideoAccelVRDP == true); } - i_vbvaUnlock(); + + RTCritSectLeave(&mVideoAccelLock); } @@ -2000,5 +2043,4 @@ void Display::i_VideoAccelFlush(void) { - i_vbvaLock(); int rc = i_videoAccelFlush(); if (RT_FAILURE(rc)) @@ -2007,10 +2049,8 @@ i_videoAccelEnable(false, NULL); } - i_vbvaUnlock(); if (RT_FAILURE(rc)) { /* VideoAccel was disabled because of a failure, switching back to VGA updates. Redraw the screen. */ - Assert(!i_vbvaLockIsOwner()); mpDrv->pUpPort->pfnUpdateDisplayAll(mpDrv->pUpPort, /* fFailOnResize = */ false); } @@ -2019,6 +2059,4 @@ int Display::i_videoAccelFlush(void) { - Assert(i_vbvaLockIsOwner()); - #ifdef DEBUG_sunlover_2 LogFlowFunc(("mfVideoAccelEnabled = %d\n", mfVideoAccelEnabled)); @@ -2131,11 +2169,7 @@ int rc = VWRN_INVALID_STATE; /* Default is to do a display update in VGA device. */ - i_vbvaLock(); - - if (ASMAtomicCmpXchgU32(&mfu32PendingVideoAccelDisable, false, true)) - { - i_videoAccelEnable(false, NULL); - } - else if (mfPendingVideoAccelEnable) + videoAccelEnterVGA(); + + if (mfPendingVideoAccelEnable) { /* Acceleration was enabled while machine was not yet running @@ -2181,5 +2215,5 @@ } - i_vbvaUnlock(); + videoAccelLeaveVGA(); return rc; @@ -2238,5 +2272,4 @@ alock.release(); - Assert(!i_vbvaLockIsOwner()); int rc = mpDrv->pUpPort->pfnQueryColorDepth(mpDrv->pUpPort, &u32BitsPerPixel); AssertRC(rc); @@ -2424,5 +2457,4 @@ uint32_t cBits = 0; - Assert(!i_vbvaLockIsOwner()); int rc = mpDrv->pUpPort->pfnQueryColorDepth(mpDrv->pUpPort, &cBits); AssertRC(rc); @@ -2569,5 +2601,4 @@ && pDisplay->maFramebuffers[aScreenId].fVBVAEnabled == false) /* A non-VBVA mode. */ { - Assert(!pDisplay->i_vbvaLockIsOwner()); rc = pDisplay->mpDrv->pUpPort->pfnTakeScreenshot(pDisplay->mpDrv->pUpPort, ppu8Data, pcbData, pu32Width, pu32Height); } @@ -2609,5 +2640,4 @@ uint32_t u32DstBitsPerPixel = 32; - Assert(!pDisplay->i_vbvaLockIsOwner()); rc = pDisplay->mpDrv->pUpPort->pfnCopyRect(pDisplay->mpDrv->pUpPort, width, height, @@ -2635,5 +2665,4 @@ && aScreenId == VBOX_VIDEO_PRIMARY_SCREEN) { - Assert(!pDisplay->i_vbvaLockIsOwner()); rc = pDisplay->mpDrv->pUpPort->pfnTakeScreenshot(pDisplay->mpDrv->pUpPort, ppu8Data, pcbData, pu32Width, pu32Height); @@ -2721,5 +2750,4 @@ { /* This can be called from any thread. */ - Assert(!pDisplay->i_vbvaLockIsOwner()); pDrv->pUpPort->pfnFreeScreenshot(pDrv->pUpPort, pu8Data); } @@ -3034,5 +3062,4 @@ if (aScreenId == VBOX_VIDEO_PRIMARY_SCREEN) { - Assert(!pDisplay->i_vbvaLockIsOwner()); rc = pDisplay->mpDrv->pUpPort->pfnDisplayBlt(pDisplay->mpDrv->pUpPort, address, x, y, width, height); } @@ -3056,5 +3083,4 @@ uint32_t u32DstBitsPerPixel = pFBInfo->u16BitsPerPixel; - Assert(!pDisplay->i_vbvaLockIsOwner()); rc = pDisplay->mpDrv->pUpPort->pfnCopyRect(pDisplay->mpDrv->pUpPort, width, height, @@ -3109,5 +3135,4 @@ u32DstBitsPerPixel = 32; - Assert(!pDisplay->i_vbvaLockIsOwner()); pDisplay->mpDrv->pUpPort->pfnCopyRect(pDisplay->mpDrv->pUpPort, width, height, @@ -3206,5 +3231,4 @@ && uScreenId == VBOX_VIDEO_PRIMARY_SCREEN) { - Assert(!pDisplay->i_vbvaLockIsOwner()); pDisplay->mpDrv->pUpPort->pfnUpdateDisplayAll(pDisplay->mpDrv->pUpPort, /* fFailOnResize = */ true); } @@ -3259,5 +3283,4 @@ if (ulWidth == pFBInfo->w && ulHeight == pFBInfo->h) { - Assert(!pDisplay->i_vbvaLockIsOwner()); pDisplay->mpDrv->pUpPort->pfnCopyRect(pDisplay->mpDrv->pUpPort, width, height, @@ -3467,5 +3490,4 @@ if (fSetRenderVRAM) { - Assert(!i_vbvaLockIsOwner()); mpDrv->pUpPort->pfnSetRenderVRAM(mpDrv->pUpPort, true); } @@ -3696,5 +3718,4 @@ /* No VBVA do a display update. */ DISPLAYFBINFO *pFBInfo = &pDisplay->maFramebuffers[VBOX_VIDEO_PRIMARY_SCREEN]; - Assert(!pDisplay->i_vbvaLockIsOwner()); pDrv->pUpPort->pfnUpdateDisplay(pDrv->pUpPort); } @@ -3817,5 +3838,5 @@ /* Disable VBVA mode. */ - pDrv->pDisplay->i_VideoAccelEnable(false, NULL); + pDrv->pDisplay->VideoAccelEnableVGA(false, NULL); } @@ -3834,6 +3855,5 @@ /* Disable VBVA mode in any case. The guest driver reenables VBVA mode if necessary. */ - /* The LFBModeChange function is called under DevVGA lock. Postpone disabling VBVA, do it in the refresh timer. */ - ASMAtomicWriteU32(&pDrv->pDisplay->mfu32PendingVideoAccelDisable, true); + pDrv->pDisplay->VideoAccelEnableVGA(false, NULL); } @@ -4493,5 +4513,4 @@ { /* Force full screen update, because VGA device must take control, do resize, etc. */ - Assert(!pThis->i_vbvaLockIsOwner()); pThis->mpDrv->pUpPort->pfnUpdateDisplayAll(pThis->mpDrv->pUpPort, /* fFailOnResize = */ false); } @@ -4529,5 +4548,4 @@ && !pFBInfo->fDisabled) { - Assert(!pThis->i_vbvaLockIsOwner()); pDrv->pUpPort->pfnUpdateDisplayRect(pDrv->pUpPort, pCmd->x, pCmd->y, pCmd->w, pCmd->h); } @@ -4570,5 +4588,4 @@ uint32_t u32DstBitsPerPixel = 32; - Assert(!pThis->i_vbvaLockIsOwner()); pDrv->pUpPort->pfnCopyRect(pDrv->pUpPort, width, height, @@ -4855,7 +4872,4 @@ LogRelFlowFunc(("iInstance=%d\n", pDrvIns->iInstance)); - if (pThis->pDisplay) - Assert(!pThis->pDisplay->i_vbvaLockIsOwner()); - pThis->pUpPort->pfnSetRenderVRAM(pThis->pUpPort, false); @@ -4965,5 +4979,4 @@ /* Disable VRAM to a buffer copy initially. */ - Assert(!pDisplay->i_vbvaLockIsOwner()); pThis->pUpPort->pfnSetRenderVRAM(pThis->pUpPort, false); pThis->IConnector.cBits = 32; /* DevVGA does nothing otherwise. */ @@ -4972,5 +4985,4 @@ * Start periodic screen refreshes */ - Assert(!pDisplay->i_vbvaLockIsOwner()); pThis->pUpPort->pfnSetRefreshRate(pThis->pUpPort, 20); Index: /trunk/src/VBox/Main/src-client/VMMDevInterface.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/VMMDevInterface.cpp (revision 52651) +++ /trunk/src/VBox/Main/src-client/VMMDevInterface.cpp (revision 52652) @@ -369,5 +369,5 @@ { LogSunlover(("MAIN::VMMDevInterface::iface_VideoAccelEnable: %d, %p\n", fEnable, pVbvaMemory)); - return display->i_VideoAccelEnable(fEnable, pVbvaMemory); + return display->VideoAccelEnableVMMDev(fEnable, pVbvaMemory); } @@ -384,5 +384,5 @@ { LogSunlover(("MAIN::VMMDevInterface::iface_VideoAccelFlush\n")); - display->i_VideoAccelFlush(); + display->VideoAccelFlushVMMDev(); } }