Index: /trunk/include/VBox/Graphics/VBoxVideoHost3D.h =================================================================== --- /trunk/include/VBox/Graphics/VBoxVideoHost3D.h (revision 70595) +++ /trunk/include/VBox/Graphics/VBoxVideoHost3D.h (revision 70596) @@ -150,5 +150,5 @@ union { - void (*pfnInternal)(void); + PFNRT pfnInternal; void *pvInternal; } u; Index: /trunk/include/VBox/vmm/pdmifs.h =================================================================== --- /trunk/include/VBox/vmm/pdmifs.h (revision 70595) +++ /trunk/include/VBox/vmm/pdmifs.h (revision 70596) @@ -736,5 +736,7 @@ typedef struct PDMIDISPLAYCONNECTOR *PPDMIDISPLAYCONNECTOR; struct VBOXCRCMDCTL; -typedef DECLCALLBACKPTR(void, PFNCRCTLCOMPLETION)(struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd, int rc, void *pvCompletion); +typedef DECLCALLBACK(void) FNCRCTLCOMPLETION(struct VBOXCRCMDCTL *pCmd, uint32_t cbCmd, int rc, void *pvCompletion); +typedef FNCRCTLCOMPLETION *PFNCRCTLCOMPLETION; + /** * Display connector interface (up). @@ -2267,20 +2269,17 @@ * completed. */ - DECLR3CALLBACKMEMBER(int, pfnVHWACommandCompleteAsync, (PPDMIDISPLAYVBVACALLBACKS pInterface, - PVBOXVHWACMD pCmd)); - - DECLR3CALLBACKMEMBER(int, pfnCrHgsmiCommandCompleteAsync, (PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXVDMACMD_CHROMIUM_CMD* pCmd, int rc)); - - DECLR3CALLBACKMEMBER(int, pfnCrHgsmiControlCompleteAsync, (PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXVDMACMD_CHROMIUM_CTL* pCmd, int rc)); - - DECLR3CALLBACKMEMBER(int, pfnCrCtlSubmit, (PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd, - PFNCRCTLCOMPLETION pfnCompletion, - void *pvCompletion)); - - DECLR3CALLBACKMEMBER(int, pfnCrCtlSubmitSync, (PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd)); + DECLR3CALLBACKMEMBER(int, pfnVHWACommandCompleteAsync,(PPDMIDISPLAYVBVACALLBACKS pInterface, PVBOXVHWACMD pCmd)); + + DECLR3CALLBACKMEMBER(int, pfnCrHgsmiCommandCompleteAsync,(PPDMIDISPLAYVBVACALLBACKS pInterface, + struct VBOXVDMACMD_CHROMIUM_CMD *pCmd, int rc)); + + DECLR3CALLBACKMEMBER(int, pfnCrHgsmiControlCompleteAsync,(PPDMIDISPLAYVBVACALLBACKS pInterface, + struct VBOXVDMACMD_CHROMIUM_CTL *pCmd, int rc)); + + DECLR3CALLBACKMEMBER(int, pfnCrCtlSubmit,(PPDMIDISPLAYVBVACALLBACKS pInterface, struct VBOXCRCMDCTL *pCmd, uint32_t cbCmd, + PFNCRCTLCOMPLETION pfnCompletion, void *pvCompletion)); + + DECLR3CALLBACKMEMBER(int, pfnCrCtlSubmitSync,(PPDMIDISPLAYVBVACALLBACKS pInterface, + struct VBOXCRCMDCTL *pCmd, uint32_t cbCmd)); } PDMIDISPLAYVBVACALLBACKS; /** PDMIDISPLAYVBVACALLBACKS */ Index: /trunk/src/VBox/Devices/Graphics/DevVGA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA.cpp (revision 70595) +++ /trunk/src/VBox/Devices/Graphics/DevVGA.cpp (revision 70596) @@ -5405,5 +5405,5 @@ #ifdef VBOX_WITH_CRHGSMI - vboxCmdVBVACmdTimer(pThis); + vboxCmdVBVATimerRefresh(pThis); #endif } Index: /trunk/src/VBox/Devices/Graphics/DevVGA.h =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA.h (revision 70595) +++ /trunk/src/VBox/Devices/Graphics/DevVGA.h (revision 70596) @@ -573,6 +573,6 @@ typedef struct VBOXVDMAHOST *PVBOXVDMAHOST; int vboxVDMAConstruct(PVGASTATE pVGAState, uint32_t cPipeElements); -int vboxVDMADestruct(PVBOXVDMAHOST pVdma); -int vboxVDMAReset(PVBOXVDMAHOST pVdma); +void vboxVDMADestruct(PVBOXVDMAHOST pVdma); +void vboxVDMAReset(PVBOXVDMAHOST pVdma); void vboxVDMAControl(PVBOXVDMAHOST pVdma, PVBOXVDMA_CTL pCmd, uint32_t cbCmd); void vboxVDMACommand(PVBOXVDMAHOST pVdma, PVBOXVDMACBUF_DR pCmd, uint32_t cbCmd); @@ -587,6 +587,6 @@ int vboxCmdVBVACmdSubmit(PVGASTATE pVGAState); int vboxCmdVBVACmdFlush(PVGASTATE pVGAState); -void vboxCmdVBVACmdTimer(PVGASTATE pVGAState); int vboxCmdVBVACmdCtl(PVGASTATE pVGAState, VBOXCMDVBVA_CTL *pCtl, uint32_t cbCtl); +void vboxCmdVBVATimerRefresh(PVGASTATE pVGAState); bool vboxCmdVBVAIsEnabled(PVGASTATE pVGAState); # endif /* VBOX_WITH_CRHGSMI */ Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 70595) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 70596) @@ -2449,12 +2449,10 @@ #ifdef VBOX_WITH_CRHGSMI case VBVA_CMDVBVA_SUBMIT: - { rc = vboxCmdVBVACmdSubmit(pVGAState); - } break; + break; case VBVA_CMDVBVA_FLUSH: - { rc = vboxCmdVBVACmdFlush(pVGAState); - } break; + break; case VBVA_CMDVBVA_CTL: @@ -2473,26 +2471,23 @@ #ifdef VBOX_WITH_VDMA case VBVA_VDMA_CMD: - { - if (cbBuffer < VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXVDMACBUF_DR)) - { + if (cbBuffer >= VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXVDMACBUF_DR)) + { + PVBOXVDMACBUF_DR pCmd = (PVBOXVDMACBUF_DR)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer); + vboxVDMACommand(pVGAState->pVdma, pCmd, cbBuffer - VBoxSHGSMIBufferHeaderSize()); + rc = VINF_SUCCESS; + } + else rc = VERR_INVALID_PARAMETER; - break; - } - - PVBOXVDMACBUF_DR pCmd = (PVBOXVDMACBUF_DR)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer); - vboxVDMACommand(pVGAState->pVdma, pCmd, cbBuffer - VBoxSHGSMIBufferHeaderSize()); - } break; + break; case VBVA_VDMA_CTL: - { - if (cbBuffer < VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXVDMA_CTL)) - { + if (cbBuffer >= VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXVDMA_CTL)) + { + PVBOXVDMA_CTL pCmd = (PVBOXVDMA_CTL)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer); + vboxVDMAControl(pVGAState->pVdma, pCmd, cbBuffer - VBoxSHGSMIBufferHeaderSize()); + } + else rc = VERR_INVALID_PARAMETER; - break; - } - - PVBOXVDMA_CTL pCmd = (PVBOXVDMA_CTL)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer); - vboxVDMAControl(pVGAState->pVdma, pCmd, cbBuffer - VBoxSHGSMIBufferHeaderSize()); - } break; + break; #endif /* VBOX_WITH_VDMA */ @@ -2919,6 +2914,5 @@ } -DECLCALLBACK(void) vbvaPortReportHostCursorPosition - (PPDMIDISPLAYPORT pInterface, uint32_t x, uint32_t y) +DECLCALLBACK(void) vbvaPortReportHostCursorPosition(PPDMIDISPLAYPORT pInterface, uint32_t x, uint32_t y) { PVGASTATE pThis = IDISPLAYPORT_2_VGASTATE(pInterface); @@ -2931,5 +2925,5 @@ } -int VBVAInit (PVGASTATE pVGAState) +int VBVAInit(PVGASTATE pVGAState) { PPDMDEVINS pDevIns = pVGAState->pDevInsR3; @@ -2937,23 +2931,22 @@ PVM pVM = PDMDevHlpGetVM(pDevIns); - int rc = HGSMICreate (&pVGAState->pHGSMI, - pVM, - "VBVA", - 0, - pVGAState->vram_ptrR3, - pVGAState->vram_size, - vbvaNotifyGuest, - pVGAState, - sizeof (VBVACONTEXT)); - - if (RT_SUCCESS (rc)) + int rc = HGSMICreate(&pVGAState->pHGSMI, + pVM, + "VBVA", + 0, + pVGAState->vram_ptrR3, + pVGAState->vram_size, + vbvaNotifyGuest, + pVGAState, + sizeof(VBVACONTEXT)); + if (RT_SUCCESS(rc)) { - rc = HGSMIHostChannelRegister (pVGAState->pHGSMI, - HGSMI_CH_VBVA, - vbvaChannelHandler, - pVGAState); - if (RT_SUCCESS (rc)) + rc = HGSMIHostChannelRegister(pVGAState->pHGSMI, + HGSMI_CH_VBVA, + vbvaChannelHandler, + pVGAState); + if (RT_SUCCESS(rc)) { - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext (pVGAState->pHGSMI); + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pVGAState->pHGSMI); pCtx->cViews = pVGAState->cMonitors; pCtx->fPaused = true; Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 70595) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 70596) @@ -136,5 +136,6 @@ struct VBVAEXHOSTCTL; -typedef DECLCALLBACKPTR(void, PFNVBVAEXHOSTCTL_COMPLETE)(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvComplete); +typedef DECLCALLBACK(void) FNVBVAEXHOSTCTL_COMPLETE(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvComplete); +typedef FNVBVAEXHOSTCTL_COMPLETE *PFNVBVAEXHOSTCTL_COMPLETE; typedef struct VBVAEXHOSTCTL @@ -205,6 +206,5 @@ *********************************************************************************************************************************/ #ifdef VBOX_WITH_CRHGSMI -static DECLCALLBACK(int) vdmaVBVANotifyDisable(PVGASTATE pVGAState); -static VBVAEXHOST_DATA_TYPE VBoxVBVAExHPDataGet(struct VBVAEXHOSTCONTEXT *pCmdVbva, uint8_t **ppCmd, uint32_t *pcbCmd); +static int vdmaVBVANotifyDisable(PVGASTATE pVGAState); static void VBoxVBVAExHPDataCompleteCmd(struct VBVAEXHOSTCONTEXT *pCmdVbva, uint32_t cbCmd); @@ -383,4 +383,9 @@ } +/** + * Worker for vboxVBVAExHPDataGet. + * @thread VDMA + * @todo r=bird: revalidate this code. + */ static int vboxVBVAExHPCmdGet(struct VBVAEXHOSTCONTEXT *pCmdVbva, uint8_t **ppCmd, uint32_t *pcbCmd) { @@ -393,6 +398,5 @@ uint32_t indexRecordFree = pVBVA->indexRecordFree; - Log(("first = %d, free = %d\n", - indexRecordFirst, indexRecordFree)); + Log(("first = %d, free = %d\n", indexRecordFirst, indexRecordFree)); if (indexRecordFirst == indexRecordFree) @@ -458,4 +462,8 @@ +/** + * Worker for VBoxVBVAExHPDataGet. + * @thread VDMA + */ static VBVAEXHOST_DATA_TYPE vboxVBVAExHPDataGet(struct VBVAEXHOSTCONTEXT *pCmdVbva, uint8_t **ppCmd, uint32_t *pcbCmd) { @@ -479,10 +487,7 @@ continue; } - else - { - *ppCmd = (uint8_t*)pCtl; - *pcbCmd = sizeof (*pCtl); - return VBVAEXHOST_DATA_TYPE_GUESTCTL; - } + *ppCmd = (uint8_t*)pCtl; + *pcbCmd = sizeof (*pCtl); + return VBVAEXHOST_DATA_TYPE_GUESTCTL; } @@ -509,4 +514,8 @@ } +/** + * Called by vboxVDMAWorkerThread to get the next command to process. + * @thread VDMA + */ static VBVAEXHOST_DATA_TYPE VBoxVBVAExHPDataGet(struct VBVAEXHOSTCONTEXT *pCmdVbva, uint8_t **ppCmd, uint32_t *pcbCmd) { @@ -516,5 +525,7 @@ vboxVBVAExHPHgEventClear(pCmdVbva); vboxVBVAExHPProcessorRelease(pCmdVbva); - /* we need to prevent racing between us clearing the flag and command check/submission thread, i.e. + + /* + * We need to prevent racing between us clearing the flag and command check/submission thread, i.e. * 1. we check the queue -> and it is empty * 2. submitter adds command to the queue @@ -523,5 +534,5 @@ * 5. ->here we need to re-check the queue state to ensure we do not leak the notification of the above command * 6. if the queue appears to be not-empty set the "processing" state back to "true" - **/ + */ int rc = vboxVBVAExHSProcessorAcquire(pCmdVbva); if (RT_SUCCESS(rc)) @@ -809,5 +820,6 @@ -static int VBoxVBVAExHCtlSubmit(VBVAEXHOSTCONTEXT *pCmdVbva, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) +static int VBoxVBVAExHCtlSubmit(VBVAEXHOSTCONTEXT *pCmdVbva, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource, + PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) { if (!VBoxVBVAExHSIsEnabled(pCmdVbva)) @@ -831,7 +843,5 @@ if (enmSource > VBVAEXHOSTCTL_SOURCE_GUEST) - { RTListAppend(&pCmdVbva->HostCtlList, &pCtl->Node); - } else RTListAppend(&pCmdVbva->GuestCtlList, &pCtl->Node); @@ -849,4 +859,7 @@ } +/** + * Call VDMA thread creation notification callback. + */ void VBoxVDMAThreadNotifyConstructSucceeded(PVBOXVDMATHREAD pThread, void *pvThreadContext) { @@ -864,4 +877,7 @@ } +/** + * Call VDMA thread termination notification callback. + */ void VBoxVDMAThreadNotifyTerminatingSucceeded(PVBOXVDMATHREAD pThread, void *pvThreadContext) { @@ -877,4 +893,7 @@ } +/** + * Check if VDMA thread is terminating. + */ DECLINLINE(bool) VBoxVDMAThreadIsTerminating(PVBOXVDMATHREAD pThread) { @@ -882,4 +901,7 @@ } +/** + * Init VDMA thread. + */ void VBoxVDMAThreadInit(PVBOXVDMATHREAD pThread) { @@ -888,4 +910,7 @@ } +/** + * Clean up VDMA thread. + */ int VBoxVDMAThreadCleanup(PVBOXVDMATHREAD pThread) { @@ -915,5 +940,9 @@ } -int VBoxVDMAThreadCreate(PVBOXVDMATHREAD pThread, PFNRTTHREAD pfnThread, void *pvThread, PFNVBOXVDMATHREAD_CHANGED pfnCreated, void*pvCreated) +/** + * Start VDMA thread. + */ +int VBoxVDMAThreadCreate(PVBOXVDMATHREAD pThread, PFNRTTHREAD pfnThread, void *pvThread, + PFNVBOXVDMATHREAD_CHANGED pfnCreated, void *pvCreated) { int rc = VBoxVDMAThreadCleanup(pThread); @@ -933,6 +962,5 @@ if (RT_SUCCESS(rc)) return VINF_SUCCESS; - else - WARN(("RTThreadCreate failed %d\n", rc)); + WARN(("RTThreadCreate failed %d\n", rc)); RTSemEventDestroy(pThread->hEvent); @@ -1001,5 +1029,5 @@ } -static int vdmaVBVACtlSubmitSync(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource); +static int vdmaVBVACtlSubmitSync(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL *pCtl, VBVAEXHOSTCTL_SOURCE enmSource); typedef DECLCALLBACK(void) FNVBOXVDMACRCTL_CALLBACK(PVGASTATE pVGAState, PVBOXVDMACMD_CHROMIUM_CTL pCmd, void* pvContext); @@ -1086,4 +1114,7 @@ } +/** + * Posts stuff and waits. + */ static int vboxVDMACrCtlPost(PVGASTATE pVGAState, PVBOXVDMACMD_CHROMIUM_CTL pCmd, uint32_t cbCmd) { @@ -1736,11 +1767,16 @@ } -/** + +/** + * Copies one page in a VBOXCMDVBVA_OPTYPE_PAGING_TRANSFER command. + * * @param fIn - whether this is a page in or out op. + * @thread VDMA + * * the direction is VRA#M - related, so fIn == true - transfer to VRAM); false - transfer from VRAM */ -static int vboxVDMACrCmdVbvaProcessPagingEl(PPDMDEVINS pDevIns, VBOXCMDVBVAPAGEIDX iPage, uint8_t *pu8Vram, bool fIn) -{ - RTGCPHYS phPage = (RTGCPHYS)iPage << PAGE_SHIFT; +static int vboxVDMACrCmdVbvaProcessPagingEl(PPDMDEVINS pDevIns, VBOXCMDVBVAPAGEIDX uPageNo, uint8_t *pbVram, bool fIn) +{ + RTGCPHYS GCPhysPage = (RTGCPHYS)uPageNo << X86_PAGE_SHIFT; PGMPAGEMAPLOCK Lock; int rc; @@ -1748,191 +1784,149 @@ if (fIn) { - const void * pvPage; - rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phPage, 0, &pvPage, &Lock); - if (!RT_SUCCESS(rc)) - { + const void *pvPage; + rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, GCPhysPage, 0, &pvPage, &Lock); + if (RT_SUCCESS(rc)) + { + memcpy(pbVram, pvPage, PAGE_SIZE); + PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + } + else WARN(("PDMDevHlpPhysGCPhys2CCPtrReadOnly failed %d", rc)); - return rc; - } - - memcpy(pu8Vram, pvPage, PAGE_SIZE); - - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); } else { - void * pvPage; - rc = PDMDevHlpPhysGCPhys2CCPtr(pDevIns, phPage, 0, &pvPage, &Lock); - if (!RT_SUCCESS(rc)) - { + void *pvPage; + rc = PDMDevHlpPhysGCPhys2CCPtr(pDevIns, GCPhysPage, 0, &pvPage, &Lock); + if (RT_SUCCESS(rc)) + { + memcpy(pvPage, pbVram, PAGE_SIZE); + PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + } + else WARN(("PDMDevHlpPhysGCPhys2CCPtr failed %d", rc)); - return rc; - } - - memcpy(pvPage, pu8Vram, PAGE_SIZE); - - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); - } - - return VINF_SUCCESS; -} - -static int vboxVDMACrCmdVbvaProcessPagingEls(PPDMDEVINS pDevIns, const VBOXCMDVBVAPAGEIDX *piPages, uint32_t cPages, uint8_t *pu8Vram, bool fIn) -{ - for (uint32_t i = 0; i < cPages; ++i, pu8Vram += PAGE_SIZE) - { - int rc = vboxVDMACrCmdVbvaProcessPagingEl(pDevIns, piPages[i], pu8Vram, fIn); - if (!RT_SUCCESS(rc)) - { - WARN(("vboxVDMACrCmdVbvaProcessPagingEl failed %d", rc)); - return rc; - } - } - - return VINF_SUCCESS; -} - -static int8_t vboxVDMACrCmdVbvaPagingDataInit(PVGASTATE pVGAState, const VBOXCMDVBVA_HDR *pHdr, const VBOXCMDVBVA_PAGING_TRANSFER_DATA *pData, uint32_t cbCmd, - const VBOXCMDVBVAPAGEIDX **ppPages, VBOXCMDVBVAPAGEIDX *pcPages, - uint8_t **ppu8Vram, bool *pfIn) -{ - if (cbCmd < sizeof (VBOXCMDVBVA_PAGING_TRANSFER)) - { - WARN(("cmd too small")); - return -1; - } - - VBOXCMDVBVAPAGEIDX cPages = cbCmd - RT_OFFSETOF(VBOXCMDVBVA_PAGING_TRANSFER, Data.aPageNumbers); - if (cPages % sizeof (VBOXCMDVBVAPAGEIDX)) - { - WARN(("invalid cmd size")); - return -1; - } - cPages /= sizeof (VBOXCMDVBVAPAGEIDX); - - VBOXCMDVBVAOFFSET offVRAM = pData->Alloc.u.offVRAM; - if (offVRAM & PAGE_OFFSET_MASK) - { + } + + return rc; +} + +/** + * Handles a VBOXCMDVBVA_OPTYPE_PAGING_TRANSFER command. + * + * @return 0 on success, -1 on failure. + * + * @thread VDMA + */ +static int8_t vboxVDMACrCmdVbvaPageTransfer(PVGASTATE pVGAState, VBOXCMDVBVA_HDR const volatile *pHdr, uint32_t cbCmd, + const VBOXCMDVBVA_PAGING_TRANSFER_DATA *pData) +{ + /* + * Extract and validate information. + */ + AssertMsgReturn(cbCmd >= sizeof(VBOXCMDVBVA_PAGING_TRANSFER), ("%#x\n", cbCmd), -1); + + bool const fIn = RT_BOOL(pHdr->u8Flags & VBOXCMDVBVA_OPF_PAGING_TRANSFER_IN); + + uint32_t cbPageNumbers = cbCmd - RT_OFFSETOF(VBOXCMDVBVA_PAGING_TRANSFER, Data.aPageNumbers); + AssertMsgReturn(!(cbPageNumbers % sizeof(VBOXCMDVBVAPAGEIDX)), ("%#x\n", cbPageNumbers), -1); + VBOXCMDVBVAPAGEIDX const cPages = cbPageNumbers / sizeof(VBOXCMDVBVAPAGEIDX); + + VBOXCMDVBVAOFFSET offVRam = pData->Alloc.u.offVRAM; + AssertMsgReturn(!(offVRam & X86_PAGE_OFFSET_MASK), ("%#x\n", offVRam), -1); + AssertMsgReturn(offVRam < pVGAState->vram_size, ("%#x vs %#x\n", offVRam, pVGAState->vram_size), -1); + uint32_t cVRamPages = (pVGAState->vram_size - offVRam) >> X86_PAGE_SHIFT; + AssertMsgReturn(cPages <= cVRamPages, ("cPages=%#x vs cVRamPages=%#x @ offVRam=%#x\n", cPages, cVRamPages, offVRam), -1); + + /* + * Execute the command. + */ + uint8_t *pbVRam = (uint8_t *)pVGAState->vram_ptrR3 + offVRam; + for (uint32_t iPage = 0; iPage < cPages; iPage++, pbVRam += X86_PAGE_SIZE) + { + uint32_t uPageNo = pData->aPageNumbers[iPage]; + int rc = vboxVDMACrCmdVbvaProcessPagingEl(pVGAState->pDevInsR3, uPageNo, pbVRam, fIn); + AssertMsgReturn(RT_SUCCESS(rc), ("#%#x: uPageNo=%#x rc=%Rrc\n", iPage, uPageNo, rc), -1); + } + return 0; +} + + +/** + * Handles VBOXCMDVBVA_OPTYPE_PAGING_FILL. + * + * @returns 0 on success, -1 on failure. + * @param pVGAState The VGA state. + * @param pFill The fill command (volatile). + * + * @thread VDMA + */ +static int8_t vboxVDMACrCmdVbvaPagingFill(PVGASTATE pVGAState, VBOXCMDVBVA_PAGING_FILL *pFill) +{ + VBOXCMDVBVA_PAGING_FILL FillSafe = *pFill; + VBOXCMDVBVAOFFSET offVRAM = FillSafe.offVRAM; + if (!(offVRAM & X86_PAGE_OFFSET_MASK)) + { + if (offVRAM <= pVGAState->vram_size) + { + uint32_t cbFill = FillSafe.u32CbFill; + AssertStmt(!(cbFill & 3), cbFill &= ~(uint32_t)3); + + if ( cbFill < pVGAState->vram_size + && offVRAM <= pVGAState->vram_size - cbFill) + { + uint32_t *pu32Vram = (uint32_t *)((uint8_t *)pVGAState->vram_ptrR3 + offVRAM); + uint32_t const u32Color = FillSafe.u32Pattern; + + uint32_t cLoops = cbFill / 4; + while (cLoops-- > 0) + pu32Vram[cLoops] = u32Color; + + return 0; + + } + else + WARN(("invalid cbFill")); + + } + WARN(("invalid vram offset")); + + } + else WARN(("offVRAM address is not on page boundary\n")); - return -1; - } - const VBOXCMDVBVAPAGEIDX *pPages = pData->aPageNumbers; - - uint8_t * pu8VramBase = pVGAState->vram_ptrR3; - if (offVRAM >= pVGAState->vram_size) - { - WARN(("invalid vram offset")); - return -1; - } - - if (~(~(VBOXCMDVBVAPAGEIDX)0 >> PAGE_SHIFT) & cPages) - { - WARN(("invalid cPages %d", cPages)); - return -1; - } - - if (offVRAM + ((VBOXCMDVBVAOFFSET)cPages << PAGE_SHIFT) >= pVGAState->vram_size) - { - WARN(("invalid cPages %d, exceeding vram size", cPages)); - return -1; - } - - uint8_t *pu8Vram = pu8VramBase + offVRAM; - bool fIn = !!(pHdr->u8Flags & VBOXCMDVBVA_OPF_PAGING_TRANSFER_IN); - - *ppPages = pPages; - *pcPages = cPages; - *ppu8Vram = pu8Vram; - *pfIn = fIn; - return 0; -} - -static int8_t vboxVDMACrCmdVbvaPagingFill(PVGASTATE pVGAState, VBOXCMDVBVA_PAGING_FILL *pFill) -{ - VBOXCMDVBVAOFFSET offVRAM = pFill->offVRAM; - if (offVRAM & PAGE_OFFSET_MASK) - { - WARN(("offVRAM address is not on page boundary\n")); - return -1; - } - - uint8_t * pu8VramBase = pVGAState->vram_ptrR3; - if (offVRAM >= pVGAState->vram_size) - { - WARN(("invalid vram offset")); - return -1; - } - - uint32_t cbFill = pFill->u32CbFill; - - if (offVRAM + cbFill >= pVGAState->vram_size) - { - WARN(("invalid cPages")); - return -1; - } - - uint32_t *pu32Vram = (uint32_t*)(pu8VramBase + offVRAM); - uint32_t u32Color = pFill->u32Pattern; - - Assert(!(cbFill % 4)); - for (uint32_t i = 0; i < cbFill / 4; ++i) - { - pu32Vram[i] = u32Color; - } - - return 0; -} - + return -1; +} + +/** + * Process command data. + * + * @returns zero or positive is success, negative failure. + * @param pVdma The VDMA channel. + * @param pCmd The command data to process. Assume volatile. + * @param cbCmd The amount of command data. + * + * @thread VDMA + */ static int8_t vboxVDMACrCmdVbvaProcessCmdData(struct VBOXVDMAHOST *pVdma, const VBOXCMDVBVA_HDR *pCmd, uint32_t cbCmd) { - switch (pCmd->u8OpCode) + uint8_t bOpCode = pCmd->u8OpCode; + switch (bOpCode) { case VBOXCMDVBVA_OPTYPE_NOPCMD: return 0; + case VBOXCMDVBVA_OPTYPE_PAGING_TRANSFER: - { - PVGASTATE pVGAState = pVdma->pVGAState; - const VBOXCMDVBVAPAGEIDX *pPages; - uint32_t cPages; - uint8_t *pu8Vram; - bool fIn; - int8_t i8Result = vboxVDMACrCmdVbvaPagingDataInit(pVGAState, pCmd, &((VBOXCMDVBVA_PAGING_TRANSFER*)pCmd)->Data, cbCmd, - &pPages, &cPages, - &pu8Vram, &fIn); - if (i8Result < 0) - { - WARN(("vboxVDMACrCmdVbvaPagingDataInit failed %d", i8Result)); - return i8Result; - } - - PPDMDEVINS pDevIns = pVGAState->pDevInsR3; - int rc = vboxVDMACrCmdVbvaProcessPagingEls(pDevIns, pPages, cPages, pu8Vram, fIn); - if (!RT_SUCCESS(rc)) - { - WARN(("vboxVDMACrCmdVbvaProcessPagingEls failed %d", rc)); - return -1; - } - - return 0; - } + return vboxVDMACrCmdVbvaPageTransfer(pVdma->pVGAState, pCmd, cbCmd, &((VBOXCMDVBVA_PAGING_TRANSFER *)pCmd)->Data); + case VBOXCMDVBVA_OPTYPE_PAGING_FILL: - { - PVGASTATE pVGAState = pVdma->pVGAState; - if (cbCmd != sizeof (VBOXCMDVBVA_PAGING_FILL)) - { - WARN(("cmd too small")); - return -1; - } - - return vboxVDMACrCmdVbvaPagingFill(pVGAState, (VBOXCMDVBVA_PAGING_FILL*)pCmd); - } + if (cbCmd == sizeof(VBOXCMDVBVA_PAGING_FILL)) + return vboxVDMACrCmdVbvaPagingFill(pVdma->pVGAState, (VBOXCMDVBVA_PAGING_FILL *)pCmd); + WARN(("cmd too small")); + return -1; + default: - if (!pVdma->CrSrvInfo.pfnCmd) - { - /* Unexpected. */ - WARN(("no HGCM")); - return -1; - } - return pVdma->CrSrvInfo.pfnCmd(pVdma->CrSrvInfo.hSvr, pCmd, cbCmd); + if (pVdma->CrSrvInfo.pfnCmd) + return pVdma->CrSrvInfo.pfnCmd(pVdma->CrSrvInfo.hSvr, pCmd, cbCmd); + /* Unexpected. */ + WARN(("no HGCM")); + return -1; } } @@ -1953,39 +1947,49 @@ AssertCompile(sizeof (VBOXCMDVBVA_ALLOCINFO) == 4); AssertCompile(sizeof (VBOXCMDVBVAPAGEIDX) == 4); -AssertCompile(!(PAGE_SIZE % sizeof (VBOXCMDVBVAPAGEIDX))); - -# define VBOXCMDVBVA_NUM_SYSMEMEL_PER_PAGE (PAGE_SIZE / sizeof (VBOXCMDVBVA_SYSMEMEL)) - -static int8_t vboxVDMACrCmdVbvaProcess(struct VBOXVDMAHOST *pVdma, const VBOXCMDVBVA_HDR *pCmd, uint32_t cbCmd) -{ - LogRelFlow(("VDMA: vboxVDMACrCmdVbvaProcess: ENTER, opCode(%i)\n", pCmd->u8OpCode)); - int8_t i8Result = 0; - - switch (pCmd->u8OpCode) +AssertCompile(!(X86_PAGE_SIZE % sizeof (VBOXCMDVBVAPAGEIDX))); + +# define VBOXCMDVBVA_NUM_SYSMEMEL_PER_PAGE (X86_PAGE_SIZE / sizeof (VBOXCMDVBVA_SYSMEMEL)) + +/** + * Worker for vboxVDMACrCmdProcess. + * + * @returns 8-bit result. + * @param pVdma The VDMA channel. + * @param pCmd The command. Consider volatile! + * @param cbCmd The size of what @a pCmd points to. At least + * sizeof(VBOXCMDVBVA_HDR). + * @param fRecursion Set if recursive call, false if not. + * + * @thread VDMA + */ +static int8_t vboxVDMACrCmdVbvaProcess(struct VBOXVDMAHOST *pVdma, const VBOXCMDVBVA_HDR *pCmd, uint32_t cbCmd, bool fRecursion) +{ + int8_t i8Result = 0; + uint8_t const bOpCode = pCmd->u8OpCode; + LogRelFlow(("VDMA: vboxVDMACrCmdVbvaProcess: ENTER, bOpCode=%u\n", bOpCode)); + switch (bOpCode) { case VBOXCMDVBVA_OPTYPE_SYSMEMCMD: { - if (cbCmd < sizeof (VBOXCMDVBVA_SYSMEMCMD)) - { - WARN(("invalid command size")); - return -1; - } - VBOXCMDVBVA_SYSMEMCMD *pSysmemCmd = (VBOXCMDVBVA_SYSMEMCMD*)pCmd; - const VBOXCMDVBVA_HDR *pRealCmdHdr; - uint32_t cbRealCmd = pCmd->u8Flags; + /* + * Extract the command physical address and size. + */ + AssertMsgReturn(cbCmd >= sizeof(VBOXCMDVBVA_SYSMEMCMD), ("%#x\n", cbCmd), -1); + RTGCPHYS GCPhysCmd = ((VBOXCMDVBVA_SYSMEMCMD *)pCmd)->phCmd; + uint32_t cbCmdPart = X86_PAGE_SIZE - (uint32_t)(GCPhysCmd & X86_PAGE_OFFSET_MASK); + + uint32_t cbRealCmd = pCmd->u8Flags; cbRealCmd |= (uint32_t)pCmd->u.u8PrimaryID << 8; - if (cbRealCmd < sizeof (VBOXCMDVBVA_HDR)) - { - WARN(("invalid sysmem cmd size")); - return -1; - } - - RTGCPHYS phCmd = (RTGCPHYS)pSysmemCmd->phCmd; - + AssertMsgReturn(cbRealCmd >= sizeof(VBOXCMDVBVA_HDR), ("%#x\n", cbRealCmd), -1); + AssertMsgReturn(cbRealCmd <= _1M, ("%#x\n", cbRealCmd), -1); + + /* + * Lock down the first page of the memory specified by the command. + */ PGMPAGEMAPLOCK Lock; PVGASTATE pVGAState = pVdma->pVGAState; PPDMDEVINS pDevIns = pVGAState->pDevInsR3; - const void * pvCmd; - int rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phCmd, 0, &pvCmd, &Lock); + VBOXCMDVBVA_HDR const *pRealCmdHdr = NULL; + int rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, GCPhysCmd, 0, (const void **)&pRealCmdHdr, &Lock); if (!RT_SUCCESS(rc)) { @@ -1993,147 +1997,62 @@ return -1; } - - Assert((phCmd & PAGE_OFFSET_MASK) == (((uintptr_t)pvCmd) & PAGE_OFFSET_MASK)); - - uint32_t cbCmdPart = PAGE_SIZE - (((uintptr_t)pvCmd) & PAGE_OFFSET_MASK); - + Assert((GCPhysCmd & PAGE_OFFSET_MASK) == (((uintptr_t)pRealCmdHdr) & PAGE_OFFSET_MASK)); + + /* + * All fits within one page? We can handle that pretty efficiently. + */ if (cbRealCmd <= cbCmdPart) { - pRealCmdHdr = (const VBOXCMDVBVA_HDR *)pvCmd; i8Result = vboxVDMACrCmdVbvaProcessCmdData(pVdma, pRealCmdHdr, cbRealCmd); PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); - return i8Result; - } - - VBOXCMDVBVA_HDR Hdr; - const void *pvCurCmdTail; - uint32_t cbCurCmdTail; - if (cbCmdPart >= sizeof (*pRealCmdHdr)) - { - pRealCmdHdr = (const VBOXCMDVBVA_HDR *)pvCmd; - pvCurCmdTail = (const void*)(pRealCmdHdr + 1); - cbCurCmdTail = cbCmdPart - sizeof (*pRealCmdHdr); } else { - memcpy(&Hdr, pvCmd, cbCmdPart); - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); - phCmd += cbCmdPart; - Assert(!(phCmd & PAGE_OFFSET_MASK)); - rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phCmd, 0, &pvCmd, &Lock); - if (!RT_SUCCESS(rc)) + /* + * To keep things damn simple, just double buffer cross page or + * multipage requests. + */ + uint8_t *pbCmdBuf = (uint8_t *)RTMemTmpAllocZ(RT_ALIGN_Z(cbRealCmd, 16)); + if (pbCmdBuf) { - WARN(("PDMDevHlpPhysGCPhys2CCPtrReadOnly failed %d\n", rc)); - return -1; + memcpy(pbCmdBuf, pRealCmdHdr, cbCmdPart); + PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + pRealCmdHdr = NULL; + + rc = PDMDevHlpPhysRead(pDevIns, GCPhysCmd + cbCmdPart, &pbCmdBuf[cbCmdPart], cbRealCmd - cbCmdPart); + if (RT_SUCCESS(rc)) + i8Result = vboxVDMACrCmdVbvaProcessCmdData(pVdma, (VBOXCMDVBVA_HDR const *)pbCmdBuf, cbRealCmd); + else + LogRelMax(200, ("VDMA: Error reading %#x bytes of guest memory %#RGp!\n", cbRealCmd, GCPhysCmd)); + RTMemTmpFree(pbCmdBuf); } - - cbCmdPart = sizeof (*pRealCmdHdr) - cbCmdPart; - memcpy(((uint8_t*)(&Hdr)) + cbCmdPart, pvCmd, cbCmdPart); - pRealCmdHdr = &Hdr; - pvCurCmdTail = (const void*)(((uint8_t*)pvCmd) + cbCmdPart); - cbCurCmdTail = PAGE_SIZE - cbCmdPart; - } - - if (cbCurCmdTail > cbRealCmd - sizeof (*pRealCmdHdr)) - cbCurCmdTail = cbRealCmd - sizeof (*pRealCmdHdr); - - switch (pRealCmdHdr->u8OpCode) - { - case VBOXCMDVBVA_OPTYPE_PAGING_TRANSFER: + else { - const uint32_t *pPages; - uint32_t cPages; - uint8_t *pu8Vram; - bool fIn; - i8Result = vboxVDMACrCmdVbvaPagingDataInit(pVGAState, pRealCmdHdr, (const VBOXCMDVBVA_PAGING_TRANSFER_DATA*)pvCurCmdTail, cbRealCmd, - &pPages, &cPages, - &pu8Vram, &fIn); - if (i8Result < 0) - { - WARN(("vboxVDMACrCmdVbvaPagingDataInit failed %d", i8Result)); - /* we need to break, not return, to ensure currently locked page is released */ - break; - } - - if (cbCurCmdTail & 3) - { - WARN(("command is not alligned properly %d", cbCurCmdTail)); - i8Result = -1; - /* we need to break, not return, to ensure currently locked page is released */ - break; - } - - uint32_t cCurPages = cbCurCmdTail / sizeof (VBOXCMDVBVAPAGEIDX); - Assert(cCurPages < cPages); - - do - { - rc = vboxVDMACrCmdVbvaProcessPagingEls(pDevIns, pPages, cCurPages, pu8Vram, fIn); - if (!RT_SUCCESS(rc)) - { - WARN(("vboxVDMACrCmdVbvaProcessPagingEls failed %d", rc)); - i8Result = -1; - /* we need to break, not return, to ensure currently locked page is released */ - break; - } - - Assert(cPages >= cCurPages); - cPages -= cCurPages; - - if (!cPages) - break; - - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); - - Assert(!(phCmd & PAGE_OFFSET_MASK)); - - phCmd += PAGE_SIZE; - pu8Vram += (VBOXCMDVBVAOFFSET)cCurPages << PAGE_SHIFT; - - rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phCmd, 0, &pvCmd, &Lock); - if (!RT_SUCCESS(rc)) - { - WARN(("PDMDevHlpPhysGCPhys2CCPtrReadOnly failed %d\n", rc)); - /* the page is not locked, return */ - return -1; - } - - cCurPages = PAGE_SIZE / sizeof (VBOXCMDVBVAPAGEIDX); - if (cCurPages > cPages) - cCurPages = cPages; - } while (1); - break; + PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + LogRelMax(200, ("VDMA: Out of temporary memory! %#x\n", cbRealCmd)); + i8Result = -1; } - default: - WARN(("command can not be splitted")); - i8Result = -1; - break; - } - - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + } return i8Result; } + case VBOXCMDVBVA_OPTYPE_COMPLEXCMD: { - Assert(cbCmd >= sizeof (VBOXCMDVBVA_HDR)); - ++pCmd; - cbCmd -= sizeof (*pCmd); - uint32_t cbCurCmd = 0; - for ( ; cbCmd; cbCmd -= cbCurCmd, pCmd = (VBOXCMDVBVA_HDR*)(((uint8_t*)pCmd) + cbCurCmd)) - { - if (cbCmd < sizeof (VBOXCMDVBVA_HDR)) - { - WARN(("invalid command size")); - return -1; - } - - cbCurCmd = pCmd->u2.complexCmdEl.u16CbCmdHost; - if (cbCmd < cbCurCmd) - { - WARN(("invalid command size")); - return -1; - } - - i8Result = vboxVDMACrCmdVbvaProcess(pVdma, pCmd, cbCurCmd); + Assert(cbCmd >= sizeof(VBOXCMDVBVA_HDR)); /* caller already checked this */ + AssertReturn(!fRecursion, -1); + + /* Skip current command. */ + cbCmd -= sizeof(*pCmd); + pCmd++; + + /* Process subcommands. */ + while (cbCmd > 0) + { + AssertMsgReturn(cbCmd >= sizeof(VBOXCMDVBVA_HDR), ("%#x\n", cbCmd), -1); + + uint16_t cbCurCmd = pCmd->u2.complexCmdEl.u16CbCmdHost; + AssertMsgReturn(cbCurCmd <= cbCmd, ("cbCurCmd=%#x, cbCmd=%#x\n", cbCurCmd, cbCmd), -1); + + i8Result = vboxVDMACrCmdVbvaProcess(pVdma, pCmd, cbCurCmd, true /*fRecursive*/); if (i8Result < 0) { @@ -2141,7 +2060,12 @@ return i8Result; } + + /* Advance to the next command. */ + pCmd = (VBOXCMDVBVA_HDR *)((uintptr_t)pCmd + cbCurCmd); + cbCmd -= cbCurCmd; } return 0; } + default: i8Result = vboxVDMACrCmdVbvaProcessCmdData(pVdma, pCmd, cbCmd); @@ -2151,25 +2075,30 @@ } -static void vboxVDMACrCmdProcess(struct VBOXVDMAHOST *pVdma, uint8_t* pu8Cmd, uint32_t cbCmd) -{ - if (*pu8Cmd == VBOXCMDVBVA_OPTYPE_NOP) - return; - - if (cbCmd < sizeof (VBOXCMDVBVA_HDR)) - { +/** + * Worker for vboxVDMAWorkerThread handling VBVAEXHOST_DATA_TYPE_CMD. + * + * @thread VDMA + */ +static void vboxVDMACrCmdProcess(struct VBOXVDMAHOST *pVdma, uint8_t* pbCmd, uint32_t cbCmd) +{ + if ( cbCmd > 0 + && *pbCmd == VBOXCMDVBVA_OPTYPE_NOP) + { /* nop */ } + else if (cbCmd >= sizeof(VBOXCMDVBVA_HDR)) + { + PVBOXCMDVBVA_HDR pCmd = (PVBOXCMDVBVA_HDR)pbCmd; + + /* check if the command is cancelled */ + if (ASMAtomicCmpXchgU8(&pCmd->u8State, VBOXCMDVBVA_STATE_IN_PROGRESS, VBOXCMDVBVA_STATE_SUBMITTED)) + { + /* Process it. */ + pCmd->u.i8Result = vboxVDMACrCmdVbvaProcess(pVdma, pCmd, cbCmd, false /*fRecursion*/); + } + else + Assert(pCmd->u8State == VBOXCMDVBVA_STATE_CANCELLED); + } + else WARN(("invalid command size")); - return; - } - - PVBOXCMDVBVA_HDR pCmd = (PVBOXCMDVBVA_HDR)pu8Cmd; - - /* check if the command is cancelled */ - if (!ASMAtomicCmpXchgU8(&pCmd->u8State, VBOXCMDVBVA_STATE_IN_PROGRESS, VBOXCMDVBVA_STATE_SUBMITTED)) - { - Assert(pCmd->u8State == VBOXCMDVBVA_STATE_CANCELLED); - return; - } - - pCmd->u.i8Result = vboxVDMACrCmdVbvaProcess(pVdma, pCmd, cbCmd); + } @@ -2208,29 +2137,26 @@ } -static int vboxVDMACmdExecBpbTransfer(PVBOXVDMAHOST pVdma, const PVBOXVDMACMD_DMA_BPB_TRANSFER pTransfer, uint32_t cbBuffer); - -/* check if this is external cmd to be passed to chromium backend */ +static int vboxVDMACmdExecBpbTransfer(PVBOXVDMAHOST pVdma, const VBOXVDMACMD_DMA_BPB_TRANSFER *pTransfer, uint32_t cbBuffer); + +/** + * Check if this is an external command to be passed to the chromium backend. + * + * @retval VINF_NOT_SUPPORTED if not chromium command. + * + * @note cbCmdDr is at least sizeof(VBOXVDMACBUF_DR). + */ static int vboxVDMACmdCheckCrCmd(struct VBOXVDMAHOST *pVdma, PVBOXVDMACBUF_DR pCmdDr, uint32_t cbCmdDr) { - PVBOXVDMACMD pDmaCmd = NULL; uint32_t cbDmaCmd = 0; - uint8_t * pvRam = pVdma->pVGAState->vram_ptrR3; + uint8_t *pbRam = pVdma->pVGAState->vram_ptrR3; int rc = VINF_NOT_SUPPORTED; cbDmaCmd = pCmdDr->cbBuf; + PVBOXVDMACMD pDmaCmd; if (pCmdDr->fFlags & VBOXVDMACBUF_FLAG_BUF_FOLLOWS_DR) { - if (cbCmdDr < sizeof (*pCmdDr) + VBOXVDMACMD_HEADER_SIZE()) - { - AssertMsgFailed(("invalid buffer data!")); - return VERR_INVALID_PARAMETER; - } - - if (cbDmaCmd < cbCmdDr - sizeof (*pCmdDr) - VBOXVDMACMD_HEADER_SIZE()) - { - AssertMsgFailed(("invalid command buffer data!")); - return VERR_INVALID_PARAMETER; - } + AssertReturn(cbCmdDr >= sizeof(*pCmdDr) + VBOXVDMACMD_HEADER_SIZE(), VERR_INVALID_PARAMETER); + AssertReturn(cbDmaCmd >= cbCmdDr - sizeof(*pCmdDr) - VBOXVDMACMD_HEADER_SIZE(), VERR_INVALID_PARAMETER); pDmaCmd = VBOXVDMACBUF_DR_TAIL(pCmdDr, VBOXVDMACMD); @@ -2239,12 +2165,10 @@ { VBOXVIDEOOFFSET offBuf = pCmdDr->Location.offVramBuf; - if (offBuf + cbDmaCmd > pVdma->pVGAState->vram_size) - { - AssertMsgFailed(("invalid command buffer data from offset!")); - return VERR_INVALID_PARAMETER; - } - pDmaCmd = (VBOXVDMACMD*)(pvRam + offBuf); - } - + AssertReturn( cbDmaCmd <= pVdma->pVGAState->vram_size + && offBuf <= pVdma->pVGAState->vram_size - cbDmaCmd, VERR_INVALID_PARAMETER); + pDmaCmd = (VBOXVDMACMD *)(pbRam + offBuf); + } + else + pDmaCmd = NULL; if (pDmaCmd) { @@ -2257,9 +2181,6 @@ { PVBOXVDMACMD_CHROMIUM_CMD pCrCmd = VBOXVDMACMD_BODY(pDmaCmd, VBOXVDMACMD_CHROMIUM_CMD); - if (cbBody < sizeof (*pCrCmd)) - { - AssertMsgFailed(("invalid chromium command buffer size!")); - return VERR_INVALID_PARAMETER; - } + AssertReturn(cbBody >= sizeof(*pCrCmd), VERR_INVALID_PARAMETER); + PVGASTATE pVGAState = pVdma->pVGAState; rc = VINF_SUCCESS; @@ -2270,21 +2191,15 @@ break; } - else - { - Assert(0); - } - - int tmpRc = VBoxSHGSMICommandComplete (pVdma->pHgsmi, pCmdDr); + + AssertFailed(); + int tmpRc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCmdDr); AssertRC(tmpRc); break; } + case VBOXVDMACMD_TYPE_DMA_BPB_TRANSFER: { PVBOXVDMACMD_DMA_BPB_TRANSFER pTransfer = VBOXVDMACMD_BODY(pDmaCmd, VBOXVDMACMD_DMA_BPB_TRANSFER); - if (cbBody < sizeof (*pTransfer)) - { - AssertMsgFailed(("invalid bpb transfer buffer size!")); - return VERR_INVALID_PARAMETER; - } + AssertReturn(cbBody >= sizeof(*pTransfer), VERR_INVALID_PARAMETER); rc = vboxVDMACmdExecBpbTransfer(pVdma, pTransfer, sizeof (*pTransfer)); @@ -2293,5 +2208,5 @@ { pCmdDr->rc = VINF_SUCCESS; - rc = VBoxSHGSMICommandComplete (pVdma->pHgsmi, pCmdDr); + rc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCmdDr); AssertRC(rc); rc = VINF_SUCCESS; @@ -2299,4 +2214,5 @@ break; } + default: break; @@ -2333,51 +2249,62 @@ } +/** + * Worker for vboxVDMACmdExecBlt(). + */ static int vboxVDMACmdExecBltPerform(PVBOXVDMAHOST pVdma, const VBOXVIDEOOFFSET offDst, const VBOXVIDEOOFFSET offSrc, const PVBOXVDMA_SURF_DESC pDstDesc, const PVBOXVDMA_SURF_DESC pSrcDesc, - const VBOXVDMA_RECTL * pDstRectl, const VBOXVDMA_RECTL * pSrcRectl) -{ - /* we do not support color conversion */ - Assert(pDstDesc->format == pSrcDesc->format); - /* we do not support stretching */ + const VBOXVDMA_RECTL *pDstRectl, const VBOXVDMA_RECTL *pSrcRectl) +{ + /* + * We do not support color conversion. + */ + AssertReturn(pDstDesc->format == pSrcDesc->format, VERR_INVALID_FUNCTION); + + /* we do not support stretching (checked by caller) */ Assert(pDstRectl->height == pSrcRectl->height); - Assert(pDstRectl->width == pSrcRectl->width); - - if (pDstDesc->format != pSrcDesc->format) - return VERR_INVALID_FUNCTION; - - uint8_t *pvRam = pVdma->pVGAState->vram_ptrR3; - uint8_t *pvDstSurf = pvRam + offDst; - uint8_t *pvSrcSurf = pvRam + offSrc; - - if (pDstDesc->width == pDstRectl->width - && pSrcDesc->width == pSrcRectl->width - && pSrcDesc->width == pDstDesc->width - && pSrcDesc->pitch == pDstDesc->pitch) + Assert(pDstRectl->width == pSrcRectl->width); + + uint8_t *pbRam = pVdma->pVGAState->vram_ptrR3; + AssertCompileSize(pVdma->pVGAState->vram_size, sizeof(uint32_t)); + uint32_t cbVRamSize = pVdma->pVGAState->vram_size; + uint8_t *pbDstSurf = pbRam + offDst; + uint8_t *pbSrcSurf = pbRam + offSrc; + + if ( pDstDesc->width == pDstRectl->width + && pSrcDesc->width == pSrcRectl->width + && pSrcDesc->width == pDstDesc->width + && pSrcDesc->pitch == pDstDesc->pitch) { Assert(!pDstRectl->left); Assert(!pSrcRectl->left); - uint32_t cbOff = pDstDesc->pitch * pDstRectl->top; - uint32_t cbSize = pDstDesc->pitch * pDstRectl->height; - memcpy(pvDstSurf + cbOff, pvSrcSurf + cbOff, cbSize); + uint32_t offBoth = pDstDesc->pitch * pDstRectl->top; + uint32_t cbToCopy = pDstDesc->pitch * pDstRectl->height; + + if ( cbToCopy <= cbVRamSize + && (uintptr_t)(pbDstSurf + offBoth) - (uintptr_t)pbRam <= cbVRamSize - cbToCopy + && (uintptr_t)(pbSrcSurf + offBoth) - (uintptr_t)pbRam <= cbVRamSize - cbToCopy) + memcpy(pbDstSurf + offBoth, pbSrcSurf + offBoth, cbToCopy); + else + return VERR_INVALID_PARAMETER; } else { - uint32_t offDstLineStart = pDstRectl->left * pDstDesc->bpp >> 3; - uint32_t offDstLineEnd = ((pDstRectl->left * pDstDesc->bpp + 7) >> 3) + ((pDstDesc->bpp * pDstRectl->width + 7) >> 3); - uint32_t cbDstLine = offDstLineEnd - offDstLineStart; - uint32_t offDstStart = pDstDesc->pitch * pDstRectl->top + offDstLineStart; + uint32_t offDstLineStart = pDstRectl->left * pDstDesc->bpp >> 3; + uint32_t offDstLineEnd = ((pDstRectl->left * pDstDesc->bpp + 7) >> 3) + ((pDstDesc->bpp * pDstRectl->width + 7) >> 3); + uint32_t cbDstLine = offDstLineEnd - offDstLineStart; + uint32_t offDstStart = pDstDesc->pitch * pDstRectl->top + offDstLineStart; Assert(cbDstLine <= pDstDesc->pitch); - uint32_t cbDstSkip = pDstDesc->pitch; - uint8_t * pvDstStart = pvDstSurf + offDstStart; - - uint32_t offSrcLineStart = pSrcRectl->left * pSrcDesc->bpp >> 3; + uint32_t cbDstSkip = pDstDesc->pitch; + uint8_t *pbDstStart = pbDstSurf + offDstStart; + + uint32_t offSrcLineStart = pSrcRectl->left * pSrcDesc->bpp >> 3; # ifdef VBOX_STRICT - uint32_t offSrcLineEnd = ((pSrcRectl->left * pSrcDesc->bpp + 7) >> 3) + ((pSrcDesc->bpp * pSrcRectl->width + 7) >> 3); - uint32_t cbSrcLine = offSrcLineEnd - offSrcLineStart; + uint32_t offSrcLineEnd = ((pSrcRectl->left * pSrcDesc->bpp + 7) >> 3) + ((pSrcDesc->bpp * pSrcRectl->width + 7) >> 3); + uint32_t cbSrcLine = offSrcLineEnd - offSrcLineStart; # endif - uint32_t offSrcStart = pSrcDesc->pitch * pSrcRectl->top + offSrcLineStart; + uint32_t offSrcStart = pSrcDesc->pitch * pSrcRectl->top + offSrcLineStart; Assert(cbSrcLine <= pSrcDesc->pitch); - uint32_t cbSrcSkip = pSrcDesc->pitch; - const uint8_t * pvSrcStart = pvSrcSurf + offSrcStart; + uint32_t cbSrcSkip = pSrcDesc->pitch; + const uint8_t *pbSrcStart = pbSrcSurf + offSrcStart; Assert(cbDstLine == cbSrcLine); @@ -2385,9 +2312,14 @@ for (uint32_t i = 0; ; ++i) { - memcpy (pvDstStart, pvSrcStart, cbDstLine); + if ( cbDstLine <= cbVRamSize + && (uintptr_t)pbSrcStart - (uintptr_t)pbRam <= cbVRamSize - cbDstLine + && (uintptr_t)pbSrcStart - (uintptr_t)pbRam <= cbVRamSize - cbDstLine) + memcpy(pbDstStart, pbSrcStart, cbDstLine); + else + return VERR_INVALID_PARAMETER; if (i == pDstRectl->height) break; - pvDstStart += cbDstSkip; - pvSrcStart += cbSrcSkip; + pbDstStart += cbDstSkip; + pbSrcStart += cbSrcSkip; } } @@ -2395,4 +2327,5 @@ } +#if 0 /* unused */ static void vboxVDMARectlUnite(VBOXVDMA_RECTL * pRectl1, const VBOXVDMA_RECTL * pRectl2) { @@ -2422,61 +2355,68 @@ } } - -/* - * @return on success the number of bytes the command contained, otherwise - VERR_xxx error code +#endif /* unused */ + +/** + * Handles VBOXVDMACMD_TYPE_DMA_PRESENT_BLT for vboxVDMACmdExec(). + * + * @returns number of bytes (positive) of the full command on success, + * otherwise a negative error status (VERR_XXX). + * + * @param pVdma The VDMA channel. + * @param pBlt Blit command buffer. This is to be considered + * volatile! + * @param cbBuffer Number of bytes accessible at @a pBtl. */ static int vboxVDMACmdExecBlt(PVBOXVDMAHOST pVdma, const PVBOXVDMACMD_DMA_PRESENT_BLT pBlt, uint32_t cbBuffer) { - const uint32_t cbBlt = VBOXVDMACMD_BODY_FIELD_OFFSET(uint32_t, VBOXVDMACMD_DMA_PRESENT_BLT, aDstSubRects[pBlt->cDstSubRects]); - Assert(cbBlt <= cbBuffer); - if (cbBuffer < cbBlt) - return VERR_INVALID_FUNCTION; - - /* we do not support stretching for now */ - Assert(pBlt->srcRectl.width == pBlt->dstRectl.width); - Assert(pBlt->srcRectl.height == pBlt->dstRectl.height); - if (pBlt->srcRectl.width != pBlt->dstRectl.width) - return VERR_INVALID_FUNCTION; - if (pBlt->srcRectl.height != pBlt->dstRectl.height) - return VERR_INVALID_FUNCTION; - Assert(pBlt->cDstSubRects); - - VBOXVDMA_RECTL updateRectl = {0, 0, 0, 0}; - - if (pBlt->cDstSubRects) - { - for (uint32_t i = 0; i < pBlt->cDstSubRects; ++i) + /* + * Validate and make a local copy of the blt command up to the rectangle array. + */ + AssertReturn(cbBuffer >= RT_UOFFSETOF(VBOXVDMACMD_DMA_PRESENT_BLT, aDstSubRects), VERR_INVALID_PARAMETER); + VBOXVDMACMD_DMA_PRESENT_BLT BltSafe; + memcpy(&BltSafe, pBlt, RT_UOFFSETOF(VBOXVDMACMD_DMA_PRESENT_BLT, aDstSubRects)); + + AssertReturn(BltSafe.cDstSubRects < _8M, VERR_INVALID_PARAMETER); + uint32_t const cbBlt = RT_UOFFSETOF(VBOXVDMACMD_DMA_PRESENT_BLT, aDstSubRects[BltSafe.cDstSubRects]); + AssertReturn(cbBuffer >= cbBlt, VERR_INVALID_PARAMETER); + + + /* + * We do not support stretching. + */ + AssertReturn(BltSafe.srcRectl.width == BltSafe.dstRectl.width, VERR_INVALID_FUNCTION); + AssertReturn(BltSafe.srcRectl.height == BltSafe.dstRectl.height, VERR_INVALID_FUNCTION); + + Assert(BltSafe.cDstSubRects); + + //VBOXVDMA_RECTL updateRectl = {0, 0, 0, 0}; - pointless + + if (BltSafe.cDstSubRects) + { + for (uint32_t i = 0; i < BltSafe.cDstSubRects; ++i) { VBOXVDMA_RECTL dstSubRectl = pBlt->aDstSubRects[i]; VBOXVDMA_RECTL srcSubRectl = dstSubRectl; - dstSubRectl.left += pBlt->dstRectl.left; - dstSubRectl.top += pBlt->dstRectl.top; - - srcSubRectl.left += pBlt->srcRectl.left; - srcSubRectl.top += pBlt->srcRectl.top; - - int rc = vboxVDMACmdExecBltPerform(pVdma, pBlt->offDst, pBlt->offSrc, - &pBlt->dstDesc, &pBlt->srcDesc, - &dstSubRectl, - &srcSubRectl); - AssertRC(rc); - if (!RT_SUCCESS(rc)) - return rc; - - vboxVDMARectlUnite(&updateRectl, &dstSubRectl); + dstSubRectl.left += BltSafe.dstRectl.left; + dstSubRectl.top += BltSafe.dstRectl.top; + + srcSubRectl.left += BltSafe.srcRectl.left; + srcSubRectl.top += BltSafe.srcRectl.top; + + int rc = vboxVDMACmdExecBltPerform(pVdma, BltSafe.offDst, BltSafe.offSrc, &BltSafe.dstDesc, &BltSafe.srcDesc, + &dstSubRectl, &srcSubRectl); + AssertRCReturn(rc, rc); + + //vboxVDMARectlUnite(&updateRectl, &dstSubRectl); - pointless } } else { - int rc = vboxVDMACmdExecBltPerform(pVdma, pBlt->offDst, pBlt->offSrc, - &pBlt->dstDesc, &pBlt->srcDesc, - &pBlt->dstRectl, - &pBlt->srcRectl); - AssertRC(rc); - if (!RT_SUCCESS(rc)) - return rc; - - vboxVDMARectlUnite(&updateRectl, &pBlt->dstRectl); + int rc = vboxVDMACmdExecBltPerform(pVdma, BltSafe.offDst, BltSafe.offSrc, &BltSafe.dstDesc, &BltSafe.srcDesc, + &BltSafe.dstRectl, &BltSafe.srcRectl); + AssertRCReturn(rc, rc); + + //vboxVDMARectlUnite(&updateRectl, &BltSafe.dstRectl); - pointless } @@ -2484,63 +2424,86 @@ } -static int vboxVDMACmdExecBpbTransfer(PVBOXVDMAHOST pVdma, const PVBOXVDMACMD_DMA_BPB_TRANSFER pTransfer, uint32_t cbBuffer) -{ - if (cbBuffer < sizeof (*pTransfer)) - return VERR_INVALID_PARAMETER; - - PVGASTATE pVGAState = pVdma->pVGAState; - uint8_t * pvRam = pVGAState->vram_ptrR3; - PGMPAGEMAPLOCK SrcLock; - PGMPAGEMAPLOCK DstLock; - PPDMDEVINS pDevIns = pVdma->pVGAState->pDevInsR3; - const void * pvSrc; - void * pvDst; - int rc = VINF_SUCCESS; - uint32_t cbTransfer = pTransfer->cbTransferSize; - uint32_t cbTransfered = 0; - bool bSrcLocked = false; - bool bDstLocked = false; + +/** + * Handles VBOXVDMACMD_TYPE_DMA_BPB_TRANSFER for vboxVDMACmdCheckCrCmd() and + * vboxVDMACmdExec(). + * + * @returns number of bytes (positive) of the full command on success, + * otherwise a negative error status (VERR_XXX). + * + * @param pVdma The VDMA channel. + * @param pTransfer Transfer command buffer. This is to be considered + * volatile! + * @param cbBuffer Number of bytes accessible at @a pTransfer. + */ +static int vboxVDMACmdExecBpbTransfer(PVBOXVDMAHOST pVdma, const VBOXVDMACMD_DMA_BPB_TRANSFER *pTransfer, uint32_t cbBuffer) +{ + /* + * Make a copy of the command (it's volatile). + */ + AssertReturn(cbBuffer >= sizeof(*pTransfer), VERR_INVALID_PARAMETER); + VBOXVDMACMD_DMA_BPB_TRANSFER const TransferSafeCopy = *pTransfer; + pTransfer = &TransferSafeCopy; + + PVGASTATE pVGAState = pVdma->pVGAState; + PPDMDEVINS pDevIns = pVGAState->pDevInsR3; + uint8_t *pbRam = pVGAState->vram_ptrR3; + uint32_t cbTransfer = TransferSafeCopy.cbTransferSize; + + /* + * Validate VRAM offset. + */ + if (TransferSafeCopy.fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_SRC_VRAMOFFSET) + AssertReturn( cbTransfer <= pVGAState->vram_size + && TransferSafeCopy.Src.offVramBuf <= pVGAState->vram_size - cbTransfer, + VERR_INVALID_PARAMETER); + + if (TransferSafeCopy.fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_DST_VRAMOFFSET) + AssertReturn( cbTransfer <= pVGAState->vram_size + && TransferSafeCopy.Dst.offVramBuf <= pVGAState->vram_size - cbTransfer, + VERR_INVALID_PARAMETER); + + /* + * Transfer loop. + */ + uint32_t cbTransfered = 0; + int rc = VINF_SUCCESS; do { uint32_t cbSubTransfer = cbTransfer; - if (pTransfer->fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_SRC_VRAMOFFSET) - { - pvSrc = pvRam + pTransfer->Src.offVramBuf + cbTransfered; - } + + const void *pvSrc; + bool fSrcLocked = false; + PGMPAGEMAPLOCK SrcLock; + if (TransferSafeCopy.fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_SRC_VRAMOFFSET) + pvSrc = pbRam + TransferSafeCopy.Src.offVramBuf + cbTransfered; else { - RTGCPHYS phPage = pTransfer->Src.phBuf; - phPage += cbTransfered; - rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phPage, 0, &pvSrc, &SrcLock); + RTGCPHYS GCPhysSrcPage = TransferSafeCopy.Src.phBuf + cbTransfered; + rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, GCPhysSrcPage, 0, &pvSrc, &SrcLock); AssertRC(rc); if (RT_SUCCESS(rc)) { - bSrcLocked = true; - cbSubTransfer = RT_MIN(cbSubTransfer, 0x1000); + fSrcLocked = true; + cbSubTransfer = RT_MIN(cbSubTransfer, X86_PAGE_SIZE - (uint32_t)(GCPhysSrcPage & X86_PAGE_OFFSET_MASK)); } else - { break; - } - } - - if (pTransfer->fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_DST_VRAMOFFSET) - { - pvDst = pvRam + pTransfer->Dst.offVramBuf + cbTransfered; - } + } + + void *pvDst; + PGMPAGEMAPLOCK DstLock; + bool fDstLocked = false; + if (TransferSafeCopy.fFlags & VBOXVDMACMD_DMA_BPB_TRANSFER_F_DST_VRAMOFFSET) + pvDst = pbRam + TransferSafeCopy.Dst.offVramBuf + cbTransfered; else { - RTGCPHYS phPage = pTransfer->Dst.phBuf; - phPage += cbTransfered; - rc = PDMDevHlpPhysGCPhys2CCPtr(pDevIns, phPage, 0, &pvDst, &DstLock); + RTGCPHYS GCPhysDstPage = TransferSafeCopy.Dst.phBuf + cbTransfered; + rc = PDMDevHlpPhysGCPhys2CCPtr(pDevIns, GCPhysDstPage, 0, &pvDst, &DstLock); AssertRC(rc); if (RT_SUCCESS(rc)) { - bDstLocked = true; - cbSubTransfer = RT_MIN(cbSubTransfer, 0x1000); - } - else - { - break; + fDstLocked = true; + cbSubTransfer = RT_MIN(cbSubTransfer, X86_PAGE_SIZE - (uint32_t)(GCPhysDstPage & X86_PAGE_OFFSET_MASK)); } } @@ -2549,37 +2512,40 @@ { memcpy(pvDst, pvSrc, cbSubTransfer); - cbTransfer -= cbSubTransfer; cbTransfered += cbSubTransfer; + cbTransfer -= cbSubTransfer; } else - { - cbTransfer = 0; /* to break */ - } - - if (bSrcLocked) + cbTransfer = 0; /* force break below */ + + if (fSrcLocked) PDMDevHlpPhysReleasePageMappingLock(pDevIns, &SrcLock); - if (bDstLocked) + if (fDstLocked) PDMDevHlpPhysReleasePageMappingLock(pDevIns, &DstLock); } while (cbTransfer); if (RT_SUCCESS(rc)) - return sizeof (*pTransfer); + return sizeof(TransferSafeCopy); return rc; } -static int vboxVDMACmdExec(PVBOXVDMAHOST pVdma, const uint8_t *pvBuffer, uint32_t cbBuffer) -{ - do - { - Assert(pvBuffer); - Assert(cbBuffer >= VBOXVDMACMD_HEADER_SIZE()); - - if (!pvBuffer) - return VERR_INVALID_PARAMETER; - if (cbBuffer < VBOXVDMACMD_HEADER_SIZE()) - return VERR_INVALID_PARAMETER; - - PVBOXVDMACMD pCmd = (PVBOXVDMACMD)pvBuffer; - switch (pCmd->enmType) +/** + * Worker for vboxVDMACommandProcess(). + * + * @param pVdma Tthe VDMA channel. + * @param pbBuffer Command buffer, considered volatile. + * @param cbBuffer The number of bytes at @a pbBuffer. + */ +static int vboxVDMACmdExec(PVBOXVDMAHOST pVdma, const uint8_t *pbBuffer, uint32_t cbBuffer) +{ + AssertReturn(pbBuffer, VERR_INVALID_POINTER); + + for (;;) + { + AssertReturn(cbBuffer >= VBOXVDMACMD_HEADER_SIZE(), VERR_INVALID_PARAMETER); + + VBOXVDMACMD const *pCmd = (VBOXVDMACMD const *)pbBuffer; + VBOXVDMACMD_TYPE enmCmdType = pCmd->enmType; + int cbProcessed; + switch (enmCmdType) { case VBOXVDMACMD_TYPE_CHROMIUM_CMD: @@ -2603,67 +2569,62 @@ return VINF_SUCCESS; } + case VBOXVDMACMD_TYPE_DMA_PRESENT_BLT: { const PVBOXVDMACMD_DMA_PRESENT_BLT pBlt = VBOXVDMACMD_BODY(pCmd, VBOXVDMACMD_DMA_PRESENT_BLT); - int cbBlt = vboxVDMACmdExecBlt(pVdma, pBlt, cbBuffer); - Assert(cbBlt >= 0); - Assert((uint32_t)cbBlt <= cbBuffer); - if (cbBlt >= 0) - { - if ((uint32_t)cbBlt == cbBuffer) - return VINF_SUCCESS; - else - { - cbBuffer -= (uint32_t)cbBlt; - pvBuffer -= cbBlt; - } - } - else - return cbBlt; /* error */ + cbProcessed = vboxVDMACmdExecBlt(pVdma, pBlt, cbBuffer - VBOXVDMACMD_HEADER_SIZE()); + Assert(cbProcessed >= 0); break; } + case VBOXVDMACMD_TYPE_DMA_BPB_TRANSFER: { const PVBOXVDMACMD_DMA_BPB_TRANSFER pTransfer = VBOXVDMACMD_BODY(pCmd, VBOXVDMACMD_DMA_BPB_TRANSFER); - int cbTransfer = vboxVDMACmdExecBpbTransfer(pVdma, pTransfer, cbBuffer); - Assert(cbTransfer >= 0); - Assert((uint32_t)cbTransfer <= cbBuffer); - if (cbTransfer >= 0) - { - if ((uint32_t)cbTransfer == cbBuffer) - return VINF_SUCCESS; - else - { - cbBuffer -= (uint32_t)cbTransfer; - pvBuffer -= cbTransfer; - } - } - else - return cbTransfer; /* error */ + cbProcessed = vboxVDMACmdExecBpbTransfer(pVdma, pTransfer, cbBuffer - VBOXVDMACMD_HEADER_SIZE()); + Assert(cbProcessed >= 0); break; } + case VBOXVDMACMD_TYPE_DMA_NOP: return VINF_SUCCESS; + case VBOXVDMACMD_TYPE_CHILD_STATUS_IRQ: return VINF_SUCCESS; + default: - AssertBreakpoint(); - return VERR_INVALID_FUNCTION; - } - } while (1); - - /* we should not be here */ - AssertBreakpoint(); - return VERR_INVALID_STATE; -} - + AssertFailedReturn(VERR_INVALID_FUNCTION); + } + + /* Advance buffer or return. */ + if (cbProcessed >= 0) + { + Assert(cbProcessed > 0); + cbProcessed += VBOXVDMACMD_HEADER_SIZE(); + if ((uint32_t)cbProcessed >= cbBuffer) + { + Assert((uint32_t)cbProcessed == cbBuffer); + return VINF_SUCCESS; + } + + cbBuffer -= cbProcessed; + pbBuffer += cbProcessed; + } + else + return cbProcessed; /* error status */ + + } +} + +/** + * VDMA worker thread procedure, see vdmaVBVACtlEnableSubmitInternal(). + * + * @thread VDMA + */ static DECLCALLBACK(int) vboxVDMAWorkerThread(RTTHREAD hThreadSelf, void *pvUser) { RT_NOREF(hThreadSelf); - PVBOXVDMAHOST pVdma = (PVBOXVDMAHOST)pvUser; - PVGASTATE pVGAState = pVdma->pVGAState; - VBVAEXHOSTCONTEXT *pCmdVbva = &pVdma->CmdVbva; - uint8_t *pCmd; - uint32_t cbCmd; + PVBOXVDMAHOST pVdma = (PVBOXVDMAHOST)pvUser; + PVGASTATE pVGAState = pVdma->pVGAState; + VBVAEXHOSTCONTEXT *pCmdVbva = &pVdma->CmdVbva; int rc; @@ -2672,21 +2633,25 @@ while (!VBoxVDMAThreadIsTerminating(&pVdma->Thread)) { - VBVAEXHOST_DATA_TYPE enmType = VBoxVBVAExHPDataGet(pCmdVbva, &pCmd, &cbCmd); + uint8_t *pbCmd = NULL; + uint32_t cbCmd = 0; + VBVAEXHOST_DATA_TYPE enmType = VBoxVBVAExHPDataGet(pCmdVbva, &pbCmd, &cbCmd); switch (enmType) { case VBVAEXHOST_DATA_TYPE_CMD: - vboxVDMACrCmdProcess(pVdma, pCmd, cbCmd); + vboxVDMACrCmdProcess(pVdma, pbCmd, cbCmd); VBoxVBVAExHPDataCompleteCmd(pCmdVbva, cbCmd); VBVARaiseIrq(pVGAState, 0); break; + case VBVAEXHOST_DATA_TYPE_GUESTCTL: - rc = vboxVDMACrGuestCtlProcess(pVdma, (VBVAEXHOSTCTL*)pCmd); - VBoxVBVAExHPDataCompleteCtl(pCmdVbva, (VBVAEXHOSTCTL*)pCmd, rc); + rc = vboxVDMACrGuestCtlProcess(pVdma, (VBVAEXHOSTCTL *)pbCmd); + VBoxVBVAExHPDataCompleteCtl(pCmdVbva, (VBVAEXHOSTCTL *)pbCmd, rc); break; + case VBVAEXHOST_DATA_TYPE_HOSTCTL: { bool fContinue = true; - rc = vboxVDMACrHostCtlProcess(pVdma, (VBVAEXHOSTCTL*)pCmd, &fContinue); - VBoxVBVAExHPDataCompleteCtl(pCmdVbva, (VBVAEXHOSTCTL*)pCmd, rc); + rc = vboxVDMACrHostCtlProcess(pVdma, (VBVAEXHOSTCTL *)pbCmd, &fContinue); + VBoxVBVAExHPDataCompleteCtl(pCmdVbva, (VBVAEXHOSTCTL *)pbCmd, rc); if (fContinue) break; @@ -2697,4 +2662,5 @@ AssertRC(rc); break; + default: WARN(("unexpected type %d\n", enmType)); @@ -2708,61 +2674,71 @@ } +/** + * Worker for vboxVDMACommand. + * + * @param pCmd The command to process. Consider content volatile. + * @param cbCmd Number of valid bytes at @a pCmd. This is at least + * sizeof(VBOXVDMACBUF_DR). + */ static void vboxVDMACommandProcess(PVBOXVDMAHOST pVdma, PVBOXVDMACBUF_DR pCmd, uint32_t cbCmd) { - RT_NOREF(cbCmd); PHGSMIINSTANCE pHgsmi = pVdma->pHgsmi; - const uint8_t * pvBuf; - PGMPAGEMAPLOCK Lock; int rc; - bool bReleaseLocked = false; - - do - { - PPDMDEVINS pDevIns = pVdma->pVGAState->pDevInsR3; - + + do /* break loop */ + { + /* + * Get the command buffer (volatile). + */ + uint16_t const cbCmdBuf = pCmd->cbBuf; + const uint8_t *pbCmdBuf; + PGMPAGEMAPLOCK Lock; + bool bReleaseLocked = false; if (pCmd->fFlags & VBOXVDMACBUF_FLAG_BUF_FOLLOWS_DR) - pvBuf = VBOXVDMACBUF_DR_TAIL(pCmd, const uint8_t); + { + pbCmdBuf = VBOXVDMACBUF_DR_TAIL(pCmd, const uint8_t); + rc = VINF_SUCCESS; + AssertBreakStmt((uintptr_t)&pbCmdBuf[cbCmdBuf] <= (uintptr_t)&((uint8_t *)pCmd)[cbCmd], + rc = VERR_INVALID_PARAMETER); + } else if (pCmd->fFlags & VBOXVDMACBUF_FLAG_BUF_VRAM_OFFSET) { - uint8_t * pvRam = pVdma->pVGAState->vram_ptrR3; - pvBuf = pvRam + pCmd->Location.offVramBuf; + uint64_t offVRam = pCmd->Location.offVramBuf; + pbCmdBuf = (uint8_t const *)pVdma->pVGAState->vram_ptrR3 + offVRam; + rc = VINF_SUCCESS; + AssertBreakStmt( offVRam <= pVdma->pVGAState->vram_size + && offVRam + cbCmdBuf <= pVdma->pVGAState->vram_size, + rc = VERR_INVALID_PARAMETER); } else { - RTGCPHYS phPage = pCmd->Location.phBuf & ~0xfffULL; - uint32_t offset = pCmd->Location.phBuf & 0xfff; - Assert(offset + pCmd->cbBuf <= 0x1000); - if (offset + pCmd->cbBuf > 0x1000) - { - /** @todo more advanced mechanism of command buffer proc is actually needed */ - rc = VERR_INVALID_PARAMETER; - break; - } - - const void * pvPageBuf; - rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pDevIns, phPage, 0, &pvPageBuf, &Lock); - AssertRC(rc); - if (!RT_SUCCESS(rc)) - { - /** @todo if (rc == VERR_PGM_PHYS_PAGE_RESERVED) -> fall back on using PGMPhysRead ?? */ - break; - } - - pvBuf = (const uint8_t *)pvPageBuf; - pvBuf += offset; - + /* Make sure it doesn't cross a page. */ + RTGCPHYS GCPhysBuf = pCmd->Location.phBuf; + AssertBreakStmt((uint32_t)(GCPhysBuf & X86_PAGE_OFFSET_MASK) + cbCmdBuf <= (uint32_t)X86_PAGE_SIZE, + rc = VERR_INVALID_PARAMETER); + + rc = PDMDevHlpPhysGCPhys2CCPtrReadOnly(pVdma->pVGAState->pDevInsR3, GCPhysBuf, 0 /*fFlags*/, + (const void **)&pbCmdBuf, &Lock); + AssertRCBreak(rc); /* if (rc == VERR_PGM_PHYS_PAGE_RESERVED) -> fall back on using PGMPhysRead ?? */ bReleaseLocked = true; } - rc = vboxVDMACmdExec(pVdma, pvBuf, pCmd->cbBuf); + /* + * Process the command. + */ + rc = vboxVDMACmdExec(pVdma, pbCmdBuf, cbCmdBuf); AssertRC(rc); + /* Clean up comand buffer. */ if (bReleaseLocked) - PDMDevHlpPhysReleasePageMappingLock(pDevIns, &Lock); + PDMDevHlpPhysReleasePageMappingLock(pVdma->pVGAState->pDevInsR3, &Lock); + } while (0); + /* + * Complete the command. + */ pCmd->rc = rc; - - rc = VBoxSHGSMICommandComplete (pHgsmi, pCmd); + rc = VBoxSHGSMICommandComplete(pHgsmi, pCmd); AssertRC(rc); } @@ -2781,4 +2757,7 @@ #ifdef VBOX_VDMA_WITH_WATCHDOG +/** + * @callback_method_impl{TMTIMER, VDMA watchdog timer.} + */ static DECLCALLBACK(void) vboxVDMAWatchDogTimer(PPDMDEVINS pDevIns, PTMTIMER pTimer, void *pvUser) { @@ -2788,4 +2767,7 @@ } +/** + * Handles VBOXVDMA_CTL_TYPE_WATCHDOG for vboxVDMAControl. + */ static int vboxVDMAWatchDogCtl(struct VBOXVDMAHOST *pVdma, uint32_t cMillis) { @@ -2800,4 +2782,9 @@ #endif /* VBOX_VDMA_WITH_WATCHDOG */ +/** + * Called by vgaR3Construct() to initialize the state. + * + * @returns VBox status code. + */ int vboxVDMAConstruct(PVGASTATE pVGAState, uint32_t cPipeElements) { @@ -2808,61 +2795,65 @@ if (pVdma) { - pVdma->pHgsmi = pVGAState->pHGSMI; + pVdma->pHgsmi = pVGAState->pHGSMI; pVdma->pVGAState = pVGAState; #ifdef VBOX_VDMA_WITH_WATCHDOG rc = PDMDevHlpTMTimerCreate(pVGAState->pDevInsR3, TMCLOCK_REAL, vboxVDMAWatchDogTimer, - pVdma, TMTIMER_FLAGS_NO_CRIT_SECT, - "VDMA WatchDog Timer", &pVdma->WatchDogTimer); + pVdma, TMTIMER_FLAGS_NO_CRIT_SECT, + "VDMA WatchDog Timer", &pVdma->WatchDogTimer); AssertRC(rc); +#else + rc = VINF_SUCCESS; #endif - + if (RT_SUCCESS(rc)) + { #ifdef VBOX_WITH_CRHGSMI - VBoxVDMAThreadInit(&pVdma->Thread); - - rc = RTSemEventMultiCreate(&pVdma->HostCrCtlCompleteEvent); - if (RT_SUCCESS(rc)) - { - rc = VBoxVBVAExHSInit(&pVdma->CmdVbva); + VBoxVDMAThreadInit(&pVdma->Thread); + + rc = RTSemEventMultiCreate(&pVdma->HostCrCtlCompleteEvent); if (RT_SUCCESS(rc)) { - rc = RTCritSectInit(&pVdma->CalloutCritSect); + rc = VBoxVBVAExHSInit(&pVdma->CmdVbva); if (RT_SUCCESS(rc)) { - pVGAState->pVdma = pVdma; - - /* No HGCM service if VMSVGA is enabled. */ - if (!pVGAState->fVMSVGAEnabled) + rc = RTCritSectInit(&pVdma->CalloutCritSect); + if (RT_SUCCESS(rc)) { - int rcIgnored = vboxVDMACrCtlHgsmiSetup(pVdma); NOREF(rcIgnored); /** @todo is this ignoring intentional? */ +#endif + pVGAState->pVdma = pVdma; + + /* No HGCM service if VMSVGA is enabled. */ + if (!pVGAState->fVMSVGAEnabled) + { + int rcIgnored = vboxVDMACrCtlHgsmiSetup(pVdma); NOREF(rcIgnored); /** @todo is this ignoring intentional? */ + } + return VINF_SUCCESS; + +#ifdef VBOX_WITH_CRHGSMI } - return VINF_SUCCESS; + + WARN(("RTCritSectInit failed %d\n", rc)); + VBoxVBVAExHSTerm(&pVdma->CmdVbva); } - WARN(("RTCritSectInit failed %d\n", rc)); - - VBoxVBVAExHSTerm(&pVdma->CmdVbva); + else + WARN(("VBoxVBVAExHSInit failed %d\n", rc)); + RTSemEventMultiDestroy(pVdma->HostCrCtlCompleteEvent); } else - WARN(("VBoxVBVAExHSInit failed %d\n", rc)); - - RTSemEventMultiDestroy(pVdma->HostCrCtlCompleteEvent); - } - else - WARN(("RTSemEventMultiCreate failed %d\n", rc)); - - + WARN(("RTSemEventMultiCreate failed %d\n", rc)); +#endif + /* the timer is cleaned up automatically */ + } RTMemFree(pVdma); -#else - pVGAState->pVdma = pVdma; - return VINF_SUCCESS; -#endif } else rc = VERR_OUT_OF_RESOURCES; - return rc; } -int vboxVDMAReset(struct VBOXVDMAHOST *pVdma) +/** + * Called by vgaR3Reset() to do reset. + */ +void vboxVDMAReset(struct VBOXVDMAHOST *pVdma) { #ifdef VBOX_WITH_CRHGSMI @@ -2871,11 +2862,13 @@ RT_NOREF(pVdma); #endif - return VINF_SUCCESS; -} - -int vboxVDMADestruct(struct VBOXVDMAHOST *pVdma) +} + +/** + * Called by vgaR3Destruct() to do cleanup.. + */ +void vboxVDMADestruct(struct VBOXVDMAHOST *pVdma) { if (!pVdma) - return VINF_SUCCESS; + return; #ifdef VBOX_WITH_CRHGSMI if (pVdma->pVGAState->fVMSVGAEnabled) @@ -2894,7 +2887,13 @@ #endif RTMemFree(pVdma); - return VINF_SUCCESS; -} - +} + +/** + * Handle VBVA_VDMA_CTL, see vbvaChannelHandler + * + * @param pVdma The VDMA channel. + * @param pCmd The control command to handle. Considered volatile. + * @param cbCmd The size of the command. At least sizeof(VBOXVDMA_CTL). + */ void vboxVDMAControl(struct VBOXVDMAHOST *pVdma, PVBOXVDMA_CTL pCmd, uint32_t cbCmd) { @@ -2902,5 +2901,6 @@ PHGSMIINSTANCE pIns = pVdma->pHgsmi; - switch (pCmd->enmCtl) + VBOXVDMA_CTL_TYPE enmCtl = pCmd->enmCtl; + switch (enmCtl) { case VBOXVDMA_CTL_TYPE_ENABLE: @@ -2921,18 +2921,24 @@ WARN(("cmd not supported")); pCmd->i32Result = VERR_NOT_SUPPORTED; - } - - int rc = VBoxSHGSMICommandComplete (pIns, pCmd); + break; + } + + int rc = VBoxSHGSMICommandComplete(pIns, pCmd); AssertRC(rc); } +/** + * Handle VBVA_VDMA_CMD, see vbvaChannelHandler(). + * + * @param pVdma The VDMA channel. + * @param pCmd The command to handle. Considered volatile. + * @param cbCmd The size of the command. At least sizeof(VBOXVDMACBUF_DR). + */ void vboxVDMACommand(struct VBOXVDMAHOST *pVdma, PVBOXVDMACBUF_DR pCmd, uint32_t cbCmd) { - int rc = VERR_NOT_IMPLEMENTED; - #ifdef VBOX_WITH_CRHGSMI /* chromium commands are processed by crhomium hgcm thread independently from our internal cmd processing pipeline * this is why we process them specially */ - rc = vboxVDMACmdCheckCrCmd(pVdma, pCmd, cbCmd); + int rc = vboxVDMACmdCheckCrCmd(pVdma, pCmd, cbCmd); if (rc == VINF_SUCCESS) return; @@ -2941,5 +2947,5 @@ { pCmd->rc = rc; - rc = VBoxSHGSMICommandComplete (pVdma->pHgsmi, pCmd); + rc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCmd); AssertRC(rc); return; @@ -2947,8 +2953,9 @@ vboxVDMACommandProcess(pVdma, pCmd, cbCmd); + #else RT_NOREF(cbCmd); - pCmd->rc = rc; - rc = VBoxSHGSMICommandComplete (pVdma->pHgsmi, pCmd); + pCmd->rc = VERR_NOT_IMPLEMENTED; + rc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCmd); AssertRC(rc); #endif @@ -2959,5 +2966,6 @@ static DECLCALLBACK(void) vdmaVBVACtlSubmitSyncCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvContext); -static int vdmaVBVACtlSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) +static int vdmaVBVACtlSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource, + PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) { int rc = VBoxVBVAExHCtlSubmit(&pVdma->CmdVbva, pCtl, enmSource, pfnComplete, pvComplete); @@ -2966,6 +2974,5 @@ if (rc == VINF_SUCCESS) return VBoxVDMAThreadEventNotify(&pVdma->Thread); - else - Assert(rc == VINF_ALREADY_INITIALIZED); + Assert(rc == VINF_ALREADY_INITIALIZED); } else @@ -2975,5 +2982,10 @@ } -static DECLCALLBACK(void) vboxCmdVBVACmdCtlGuestCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvContext) +/** + * @callback_method_impl{FNVBVAEXHOSTCTL_COMPLETE, + * Used by vdmaVBVACtlEnableDisableSubmit() and vdmaVBVACtlEnableDisableSubmit() } + */ +static DECLCALLBACK(void) vboxCmdVBVACmdCtlGuestCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, + int rc, void *pvContext) { PVBOXVDMAHOST pVdma = (PVBOXVDMAHOST)pvContext; @@ -2989,7 +3001,11 @@ } -static int vdmaVBVACtlGenericSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL_SOURCE enmSource, VBVAEXHOSTCTL_TYPE enmType, uint8_t* pu8Cmd, uint32_t cbCmd, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) -{ - VBVAEXHOSTCTL* pHCtl = VBoxVBVAExHCtlCreate(&pVdma->CmdVbva, enmType); +/** + * Worker for vdmaVBVACtlGenericGuestSubmit() and vdmaVBVACtlOpaqueHostSubmit(). + */ +static int vdmaVBVACtlGenericSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL_SOURCE enmSource, VBVAEXHOSTCTL_TYPE enmType, + uint8_t* pu8Cmd, uint32_t cbCmd, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) +{ + VBVAEXHOSTCTL *pHCtl = VBoxVBVAExHCtlCreate(&pVdma->CmdVbva, enmType); if (!pHCtl) { @@ -3010,9 +3026,13 @@ } +/** + * Handler for vboxCmdVBVACmdCtl()/VBOXCMDVBVACTL_TYPE_3DCTL. + */ static int vdmaVBVACtlGenericGuestSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL_TYPE enmType, VBOXCMDVBVA_CTL *pCtl, uint32_t cbCtl) { Assert(cbCtl >= sizeof (VBOXCMDVBVA_CTL)); VBoxSHGSMICommandMarkAsynchCompletion(pCtl); - int rc = vdmaVBVACtlGenericSubmit(pVdma, VBVAEXHOSTCTL_SOURCE_GUEST, enmType, (uint8_t*)(pCtl+1), cbCtl - sizeof (VBOXCMDVBVA_CTL), vboxCmdVBVACmdCtlGuestCompletion, pVdma); + int rc = vdmaVBVACtlGenericSubmit(pVdma, VBVAEXHOSTCTL_SOURCE_GUEST, enmType, (uint8_t*)(pCtl+1), + cbCtl - sizeof (VBOXCMDVBVA_CTL), vboxCmdVBVACmdCtlGuestCompletion, pVdma); if (RT_SUCCESS(rc)) return VINF_SUCCESS; @@ -3025,5 +3045,9 @@ } -static DECLCALLBACK(void) vboxCmdVBVACmdCtlHostCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvCompletion) +/** + * @callback_method_impl{FNVBVAEXHOSTCTL_COMPLETE, Used by vdmaVBVACtlOpaqueHostSubmit()} + */ +static DECLCALLBACK(void) vboxCmdVBVACmdCtlHostCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, + int rc, void *pvCompletion) { VBOXCRCMDCTL* pVboxCtl = (VBOXCRCMDCTL*)pCtl->u.cmd.pu8Cmd; @@ -3033,10 +3057,13 @@ } +/** + * Worker for vboxCmdVBVACmdHostCtl() and vboxCmdVBVACmdHostCtlSync(). + */ static int vdmaVBVACtlOpaqueHostSubmit(PVBOXVDMAHOST pVdma, struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd, - PFNCRCTLCOMPLETION pfnCompletion, - void *pvCompletion) -{ - pCmd->u.pfnInternal = (void(*)())pfnCompletion; - int rc = vdmaVBVACtlGenericSubmit(pVdma, VBVAEXHOSTCTL_SOURCE_HOST, VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE, (uint8_t*)pCmd, cbCmd, vboxCmdVBVACmdCtlHostCompletion, pvCompletion); + PFNCRCTLCOMPLETION pfnCompletion, void *pvCompletion) +{ + pCmd->u.pfnInternal = (PFNRT)pfnCompletion; + int rc = vdmaVBVACtlGenericSubmit(pVdma, VBVAEXHOSTCTL_SOURCE_HOST, VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE, + (uint8_t *)pCmd, cbCmd, vboxCmdVBVACmdCtlHostCompletion, pvCompletion); if (RT_FAILURE(rc)) { @@ -3058,5 +3085,8 @@ } -static DECLCALLBACK(int) vdmaVBVANotifyEnable(PVGASTATE pVGAState) +/** + * Called from vdmaVBVACtlThreadCreatedEnable(). + */ +static int vdmaVBVANotifyEnable(PVGASTATE pVGAState) { for (uint32_t i = 0; i < pVGAState->cMonitors; i++) @@ -3077,13 +3107,17 @@ } -static DECLCALLBACK(int) vdmaVBVANotifyDisable(PVGASTATE pVGAState) +/** + * Called from vdmaVBVACtlThreadCreatedEnable() and vdmaVBVADisableProcess(). + */ +static int vdmaVBVANotifyDisable(PVGASTATE pVGAState) { for (uint32_t i = 0; i < pVGAState->cMonitors; i++) - { - pVGAState->pDrv->pfnVBVADisable (pVGAState->pDrv, i); - } + pVGAState->pDrv->pfnVBVADisable(pVGAState->pDrv, i); return VINF_SUCCESS; } +/** + * Hook that is called by vboxVDMAWorkerThread when it starts. + */ static DECLCALLBACK(void) vdmaVBVACtlThreadCreatedEnable(struct VBOXVDMATHREAD *pThread, int rc, void *pvThreadContext, void *pvContext) @@ -3118,8 +3152,12 @@ } +/** + * Worker for vdmaVBVACtlEnableDisableSubmitInternal() and vdmaVBVACtlEnableSubmitSync(). + */ static int vdmaVBVACtlEnableSubmitInternal(PVBOXVDMAHOST pVdma, VBVAENABLE *pEnable, bool fPaused, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) { int rc; - VBVAEXHOSTCTL* pHCtl = VBoxVBVAExHCtlCreate(&pVdma->CmdVbva, fPaused ? VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED : VBVAEXHOSTCTL_TYPE_GHH_ENABLE); + VBVAEXHOSTCTL *pHCtl = VBoxVBVAExHCtlCreate(&pVdma->CmdVbva, + fPaused ? VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED : VBVAEXHOSTCTL_TYPE_GHH_ENABLE); if (pHCtl) { @@ -3132,6 +3170,5 @@ if (RT_SUCCESS(rc)) return VINF_SUCCESS; - else - WARN(("VBoxVDMAThreadCreate failed %d\n", rc)); + WARN(("VBoxVDMAThreadCreate failed %d\n", rc)); VBoxVBVAExHCtlFree(&pVdma->CmdVbva, pHCtl); @@ -3146,4 +3183,7 @@ } +/** + * Worker for vboxVDMASaveLoadExecPerform(). + */ static int vdmaVBVACtlEnableSubmitSync(PVBOXVDMAHOST pVdma, uint32_t offVram, bool fPaused) { @@ -3182,5 +3222,9 @@ } -static int vdmaVBVACtlDisableSubmitInternal(PVBOXVDMAHOST pVdma, VBVAENABLE *pEnable, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) +/** + * Worker for vdmaVBVACtlEnableDisableSubmitInternal(). + */ +static int vdmaVBVACtlDisableSubmitInternal(PVBOXVDMAHOST pVdma, VBVAENABLE *pEnable, + PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) { int rc; @@ -3210,7 +3254,11 @@ } -static int vdmaVBVACtlEnableDisableSubmitInternal(PVBOXVDMAHOST pVdma, VBVAENABLE *pEnable, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) -{ - bool fEnable = ((pEnable->u32Flags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_ENABLE); +/** + * Worker for vdmaVBVACtlEnableDisableSubmit(). + */ +static int vdmaVBVACtlEnableDisableSubmitInternal(PVBOXVDMAHOST pVdma, VBVAENABLE *pEnable, + PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete) +{ + bool fEnable = (pEnable->u32Flags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_ENABLE; if (fEnable) return vdmaVBVACtlEnableSubmitInternal(pVdma, pEnable, false, pfnComplete, pvComplete); @@ -3218,4 +3266,7 @@ } +/** + * Handler for vboxCmdVBVACmdCtl/VBOXCMDVBVACTL_TYPE_ENABLE. + */ static int vdmaVBVACtlEnableDisableSubmit(PVBOXVDMAHOST pVdma, VBOXCMDVBVA_CTL_ENABLE *pEnable) { @@ -3232,9 +3283,13 @@ } +/** + * @callback_method_impl{FNVBVAEXHOSTCTL_COMPLETE, + * Used by vdmaVBVACtlSubmitSync() and vdmaVBVACtlEnableSubmitSync().} + */ static DECLCALLBACK(void) vdmaVBVACtlSubmitSyncCompletion(VBVAEXHOSTCONTEXT *pVbva, struct VBVAEXHOSTCTL *pCtl, int rc, void *pvContext) { RT_NOREF(pVbva, pCtl); - VDMA_VBVA_CTL_CYNC_COMPLETION *pData = (VDMA_VBVA_CTL_CYNC_COMPLETION*)pvContext; + VDMA_VBVA_CTL_CYNC_COMPLETION *pData = (VDMA_VBVA_CTL_CYNC_COMPLETION *)pvContext; pData->rc = rc; rc = RTSemEventSignal(pData->hEvent); @@ -3243,4 +3298,5 @@ } + static int vdmaVBVACtlSubmitSync(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL* pCtl, VBVAEXHOSTCTL_SOURCE enmSource) { @@ -3275,4 +3331,7 @@ } +/** + * Worker for vboxVDMASaveStateExecPrep(). + */ static int vdmaVBVAPause(PVBOXVDMAHOST pVdma) { @@ -3282,4 +3341,7 @@ } +/** + * Worker for vboxVDMASaveLoadExecPerform() and vboxVDMASaveStateExecDone(). + */ static int vdmaVBVAResume(PVBOXVDMAHOST pVdma) { @@ -3289,4 +3351,7 @@ } +/** + * Worker for vboxCmdVBVACmdSubmit(), vboxCmdVBVACmdFlush() and vboxCmdVBVATimerRefresh(). + */ static int vboxVDMACmdSubmitPerform(struct VBOXVDMAHOST *pVdma) { @@ -3307,8 +3372,12 @@ +/** + * @interface_method_impl{PDMIDISPLAYVBVACALLBACKS,pfnCrCtlSubmit} + */ int vboxCmdVBVACmdHostCtl(PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd, - PFNCRCTLCOMPLETION pfnCompletion, - void *pvCompletion) + struct VBOXCRCMDCTL *pCmd, + uint32_t cbCmd, + PFNCRCTLCOMPLETION pfnCompletion, + void *pvCompletion) { PVGASTATE pVGAState = PPDMIDISPLAYVBVACALLBACKS_2_PVGASTATE(pInterface); @@ -3320,4 +3389,7 @@ } +/** + * Argument package from vboxCmdVBVACmdHostCtlSync to vboxCmdVBVACmdHostCtlSyncCb. + */ typedef struct VBOXCMDVBVA_CMDHOSTCTL_SYNC { @@ -3327,8 +3399,11 @@ } VBOXCMDVBVA_CMDHOSTCTL_SYNC; -static DECLCALLBACK(void) vboxCmdVBVACmdHostCtlSyncCb(struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd, int rc, void *pvCompletion) +/** + * @interface_method_impl{FNCRCTLCOMPLETION, Used by vboxCmdVBVACmdHostCtlSync.} + */ +static DECLCALLBACK(void) vboxCmdVBVACmdHostCtlSyncCb(struct VBOXCRCMDCTL *pCmd, uint32_t cbCmd, int rc, void *pvCompletion) { RT_NOREF(pCmd, cbCmd); - VBOXCMDVBVA_CMDHOSTCTL_SYNC *pData = (VBOXCMDVBVA_CMDHOSTCTL_SYNC*)pvCompletion; + VBOXCMDVBVA_CMDHOSTCTL_SYNC *pData = (VBOXCMDVBVA_CMDHOSTCTL_SYNC *)pvCompletion; pData->rc = rc; @@ -3343,5 +3418,11 @@ } -static DECLCALLBACK(int) vboxCmdVBVACmdCallout(struct VBOXVDMAHOST *pVdma, struct VBOXCRCMDCTL* pCmd, VBOXCRCMDCTL_CALLOUT_LISTENTRY *pEntry, PFNVBOXCRCMDCTL_CALLOUT_CB pfnCb) +/** + * @callback_method_impl{FNVBOXCRCLIENT_CALLOUT, Worker for vboxVDMACrCtlHgsmiSetup } + * + * @note r=bird: not to be confused with the callout function below. sigh. + */ +static DECLCALLBACK(int) vboxCmdVBVACmdCallout(struct VBOXVDMAHOST *pVdma, struct VBOXCRCMDCTL* pCmd, + VBOXCRCMDCTL_CALLOUT_LISTENTRY *pEntry, PFNVBOXCRCMDCTL_CALLOUT_CB pfnCb) { pEntry->pfnCb = pfnCb; @@ -3361,4 +3442,7 @@ +/** + * Worker for vboxCmdVBVACmdHostCtlSync. + */ static int vboxCmdVBVACmdCalloutProcess(struct VBOXVDMAHOST *pVdma, struct VBOXCRCMDCTL* pCmd) { @@ -3389,11 +3473,14 @@ } -DECLCALLBACK(int) vboxCmdVBVACmdHostCtlSync(PPDMIDISPLAYVBVACALLBACKS pInterface, - struct VBOXCRCMDCTL* pCmd, uint32_t cbCmd) -{ - PVGASTATE pVGAState = PPDMIDISPLAYVBVACALLBACKS_2_PVGASTATE(pInterface); - struct VBOXVDMAHOST *pVdma = pVGAState->pVdma; +/** + * @interface_method_impl{PDMIDISPLAYVBVACALLBACKS,pfnCrCtlSubmitSync} + */ +DECLCALLBACK(int) vboxCmdVBVACmdHostCtlSync(PPDMIDISPLAYVBVACALLBACKS pInterface, struct VBOXCRCMDCTL *pCmd, uint32_t cbCmd) +{ + PVGASTATE pVGAState = PPDMIDISPLAYVBVACALLBACKS_2_PVGASTATE(pInterface); + struct VBOXVDMAHOST *pVdma = pVGAState->pVdma; if (pVdma == NULL) return VERR_INVALID_STATE; + VBOXCMDVBVA_CMDHOSTCTL_SYNC Data; Data.pVdma = pVdma; @@ -3439,8 +3526,10 @@ } +/** + * Handler for VBVA_CMDVBVA_CTL, see vbvaChannelHandler(). + */ int vboxCmdVBVACmdCtl(PVGASTATE pVGAState, VBOXCMDVBVA_CTL *pCtl, uint32_t cbCtl) { struct VBOXVDMAHOST *pVdma = pVGAState->pVdma; - int rc = VINF_SUCCESS; switch (pCtl->u32Type) { @@ -3450,23 +3539,22 @@ return vdmaVBVACtlGenericGuestSubmit(pVdma, VBVAEXHOSTCTL_TYPE_GHH_RESIZE, pCtl, cbCtl); case VBOXCMDVBVACTL_TYPE_ENABLE: - if (cbCtl != sizeof (VBOXCMDVBVA_CTL_ENABLE)) - { - WARN(("incorrect enable size\n")); - rc = VERR_INVALID_PARAMETER; - break; - } - return vdmaVBVACtlEnableDisableSubmit(pVdma, (VBOXCMDVBVA_CTL_ENABLE*)pCtl); + if (cbCtl == sizeof(VBOXCMDVBVA_CTL_ENABLE)) + return vdmaVBVACtlEnableDisableSubmit(pVdma, (VBOXCMDVBVA_CTL_ENABLE *)pCtl); + WARN(("incorrect enable size\n")); + break; default: WARN(("unsupported type\n")); - rc = VERR_INVALID_PARAMETER; break; } - pCtl->i32Result = rc; - rc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCtl); + pCtl->i32Result = VERR_INVALID_PARAMETER; + int rc = VBoxSHGSMICommandComplete(pVdma->pHgsmi, pCtl); AssertRC(rc); return VINF_SUCCESS; } +/** + * Handler for VBVA_CMDVBVA_SUBMIT, see vbvaChannelHandler(). + */ int vboxCmdVBVACmdSubmit(PVGASTATE pVGAState) { @@ -3480,4 +3568,7 @@ } +/** + * Handler for VBVA_CMDVBVA_FLUSH, see vbvaChannelHandler(). + */ int vboxCmdVBVACmdFlush(PVGASTATE pVGAState) { @@ -3491,5 +3582,8 @@ } -void vboxCmdVBVACmdTimer(PVGASTATE pVGAState) +/** + * Called from vgaTimerRefresh(). + */ +void vboxCmdVBVATimerRefresh(PVGASTATE pVGAState) { if (!VBoxVBVAExHSIsEnabled(&pVGAState->pVdma->CmdVbva)) @@ -3504,4 +3598,15 @@ #endif /* VBOX_WITH_CRHGSMI */ + + +/* + * + * + * Saved state. + * Saved state. + * Saved state. + * + * + */ int vboxVDMASaveStateExecPrep(struct VBOXVDMAHOST *pVdma) Index: /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp (revision 70595) +++ /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp (revision 70596) @@ -1458,11 +1458,11 @@ AssertPtrReturn(pfnChannelHandler, VERR_INVALID_PARAMETER); - int rc = hgsmiLock (pIns); - - if (RT_SUCCESS (rc)) - { - rc = HGSMIChannelRegister (&pIns->channelInfo, u8Channel, NULL, pfnChannelHandler, pvChannelHandler); - - hgsmiUnlock (pIns); + int rc = hgsmiLock(pIns); + + if (RT_SUCCESS(rc)) + { + rc = HGSMIChannelRegister(&pIns->channelInfo, u8Channel, NULL, pfnChannelHandler, pvChannelHandler); + + hgsmiUnlock(pIns); } Index: /trunk/src/VBox/Main/src-client/DisplayImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 70595) +++ /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 70596) @@ -3697,5 +3697,5 @@ { RT_NOREF(u32Function); - VBOXCRCMDCTL *pCmd = (VBOXCRCMDCTL*)pParam->u.pointer.addr; + VBOXCRCMDCTL *pCmd = (VBOXCRCMDCTL *)pParam->u.pointer.addr; if (pCmd->u.pfnInternal) ((PFNCRCTLCOMPLETION)pCmd->u.pfnInternal)(pCmd, pParam->u.pointer.size, result, pvContext); @@ -3719,5 +3719,5 @@ parm.u.pointer.size = cbCmd; - pCmd->u.pfnInternal = (void(*)())pfnCompletion; + pCmd->u.pfnInternal = (PFNRT)pfnCompletion; int rc = pVMMDev->hgcmHostFastCallAsync(mhCrOglSvc, SHCRGL_HOST_FN_CTL, &parm, i_displayCrHgcmCtlSubmitCompletion, pvCompletion);