Changeset 70597 in vbox

Timestamp:

Jan 15, 2018 11:46:26 PM (7 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

120305

Message:

DevVGA: cleanup in progress

Location:

trunk/src/VBox/Devices/Graphics

Files:

• DevVGA_VBVA.cpp (modified) (1 diff)
• DevVGA_VDMA.cpp (modified) (12 diffs)

trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp

@@ -2457 +2457 @@
 
         case VBVA_CMDVBVA_CTL:
-        {
-            if (cbBuffer < VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXCMDVBVA_CTL))
-            {
+            if (cbBuffer >= VBoxSHGSMIBufferHeaderSize() + sizeof(VBOXCMDVBVA_CTL))
+            {
+                VBOXCMDVBVA_CTL *pCtl = (VBOXCMDVBVA_CTL *)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer);
+                rc = vboxCmdVBVACmdCtl(pVGAState, pCtl, cbBuffer - VBoxSHGSMIBufferHeaderSize());
+            }
+            else
                 rc = VERR_INVALID_PARAMETER;
-                break;
-            }
-
-            VBOXCMDVBVA_CTL *pCtl = (VBOXCMDVBVA_CTL*)VBoxSHGSMIBufferData((PVBOXSHGSMIHEADER)pvBuffer);
-            rc = vboxCmdVBVACmdCtl(pVGAState, pCtl, cbBuffer - VBoxSHGSMIBufferHeaderSize());
-        } break;
+            break;
 #endif /* VBOX_WITH_CRHGSMI */
 

trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp

@@ -317 +317 @@
 }
 
+/**
+ * Worker for vboxVBVAExHPCheckProcessCtlInternal() and
+ * vboxVDMACrGuestCtlProcess() / VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED.
+ */
 static int VBoxVBVAExHPPause(struct VBVAEXHOSTCONTEXT *pCmdVbva)
 {
@@ -1586 +1590 @@
 }
 
+/**
+ * Handles on entry in a VBVAEXHOSTCTL_TYPE_GHH_RESIZE command.
+ *
+ * @returns IPRT status code.
+ * @param   pVdma               The VDMA channel
+ * @param   pEntry              The entry to handle.  Considered volatile.
+ *
+ * @thread  VDMA
+ */
 static int vboxVDMACrGuestCtlResizeEntryProcess(struct VBOXVDMAHOST *pVdma, VBOXCMDVBVA_RESIZE_ENTRY *pEntry)
 {
@@ -1662 +1675 @@
 }
 
+
+/**
+ * Processes VBVAEXHOST_DATA_TYPE_GUESTCTL for vboxVDMAWorkerThread and
+ * vdmaVBVACtlThreadCreatedEnable.
+ *
+ * @returns VBox status code.
+ * @param   pVdma               The VDMA channel.
+ * @param   pCmd                The command to process.
+ *
+ * @thread  VDMA
+ */
 static int vboxVDMACrGuestCtlProcess(struct VBOXVDMAHOST *pVdma, VBVAEXHOSTCTL *pCmd)
 {
@@ -1667 +1691 @@
     switch (enmType)
     {
+        /*
+         * See handling of VBOXCMDVBVACTL_TYPE_3DCTL in vboxCmdVBVACmdCtl().
+         */
         case VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE:
-        {
-            if (!VBoxVBVAExHSIsEnabled(&pVdma->CmdVbva))
-            {
-                WARN(("VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE for disabled vdma VBVA\n"));
-                return VERR_INVALID_STATE;
-            }
-            if (!pVdma->CrSrvInfo.pfnGuestCtl)
-            {
+            if (VBoxVBVAExHSIsEnabled(&pVdma->CmdVbva))
+            {
+                if (pVdma->CrSrvInfo.pfnGuestCtl)
+                    return pVdma->CrSrvInfo.pfnGuestCtl(pVdma->CrSrvInfo.hSvr, pCmd->u.cmd.pu8Cmd, pCmd->u.cmd.cbCmd);
+
                 /* Unexpected. */
                 WARN(("VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE in HGCM-less mode\n"));
-                return VERR_INVALID_STATE;
-            }
-            return pVdma->CrSrvInfo.pfnGuestCtl(pVdma->CrSrvInfo.hSvr, pCmd->u.cmd.pu8Cmd, pCmd->u.cmd.cbCmd);
-        }
+            }
+            else
+                WARN(("VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE for disabled vdma VBVA\n"));
+            return VERR_INVALID_STATE;
+
+        /*
+         * See handling of VBOXCMDVBVACTL_TYPE_RESIZE in vboxCmdVBVACmdCtl().
+         */
         case VBVAEXHOSTCTL_TYPE_GHH_RESIZE:
-        {
-            if (!VBoxVBVAExHSIsEnabled(&pVdma->CmdVbva))
-            {
-                WARN(("VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE for disabled vdma VBVA\n"));
-                return VERR_INVALID_STATE;
-            }
-
-            uint32_t cbCmd = pCmd->u.cmd.cbCmd;
-
-            if (cbCmd % sizeof (VBOXCMDVBVA_RESIZE_ENTRY))
-            {
-                WARN(("invalid buffer size\n"));
+            if (VBoxVBVAExHSIsEnabled(&pVdma->CmdVbva))
+            {
+                uint32_t cbCmd = pCmd->u.cmd.cbCmd;
+                if (   !(cbCmd % sizeof(VBOXCMDVBVA_RESIZE_ENTRY))
+                    && cbCmd > 0)
+                {
+                    uint32_t cElements = cbCmd / sizeof(VBOXCMDVBVA_RESIZE_ENTRY);
+                    VBOXCMDVBVA_RESIZE *pResize = (VBOXCMDVBVA_RESIZE *)pCmd->u.cmd.pu8Cmd;
+                    for (uint32_t i = 0; i < cElements; ++i)
+                    {
+                        VBOXCMDVBVA_RESIZE_ENTRY *pEntry = &pResize->aEntries[i];
+                        int rc = vboxVDMACrGuestCtlResizeEntryProcess(pVdma, pEntry);
+                        if (RT_FAILURE(rc))
+                        {
+                            WARN(("vboxVDMACrGuestCtlResizeEntryProcess failed %d\n", rc));
+                            return rc;
+                        }
+                    }
+                    return VINF_SUCCESS;
+                }
+                else
+                    WARN(("invalid buffer size: cbCmd=%#x\n", cbCmd));
                 return VERR_INVALID_PARAMETER;
             }
-
-            uint32_t cElements = cbCmd / sizeof (VBOXCMDVBVA_RESIZE_ENTRY);
-            if (!cElements)
-            {
-                WARN(("invalid buffer size\n"));
-                return VERR_INVALID_PARAMETER;
-            }
-
-            VBOXCMDVBVA_RESIZE *pResize = (VBOXCMDVBVA_RESIZE*)pCmd->u.cmd.pu8Cmd;
-
-            int rc = VINF_SUCCESS;
-
-            for (uint32_t i = 0; i < cElements; ++i)
-            {
-                VBOXCMDVBVA_RESIZE_ENTRY *pEntry = &pResize->aEntries[i];
-                rc = vboxVDMACrGuestCtlResizeEntryProcess(pVdma, pEntry);
-                if (RT_FAILURE(rc))
-                {
-                    WARN(("vboxVDMACrGuestCtlResizeEntryProcess failed %d\n", rc));
-                    break;
-                }
-            }
-            return rc;
-        }
+            WARN(("VBVAEXHOSTCTL_TYPE_GHH_RESIZE for disabled vdma VBVA\n"));
+            return VERR_INVALID_STATE;
+
+        /*
+         * See vdmaVBVACtlEnableSubmitInternal().
+         */
         case VBVAEXHOSTCTL_TYPE_GHH_ENABLE:
         case VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED:
         {
             VBVAENABLE *pEnable = (VBVAENABLE *)pCmd->u.cmd.pu8Cmd;
-            Assert(pCmd->u.cmd.cbCmd == sizeof (VBVAENABLE));
+            Assert(pCmd->u.cmd.cbCmd == sizeof(VBVAENABLE));
+
             uint32_t u32Offset = pEnable->u32Offset;
             int rc = vdmaVBVAEnableProcess(pVdma, u32Offset);
-            if (!RT_SUCCESS(rc))
-            {
+            if (RT_SUCCESS(rc))
+            {
+                if (enmType != VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED)
+                    return VINF_SUCCESS;
+
+                rc = VBoxVBVAExHPPause(&pVdma->CmdVbva);
+                if (RT_SUCCESS(rc))
+                    return VINF_SUCCESS;
+                WARN(("VBoxVBVAExHPPause failed %d\n", rc));
+            }
+            else
                 WARN(("vdmaVBVAEnableProcess failed %d\n", rc));
-                return rc;
-            }
-
-            if (enmType == VBVAEXHOSTCTL_TYPE_GHH_ENABLE_PAUSED)
-            {
-                rc = VBoxVBVAExHPPause(&pVdma->CmdVbva);
-                if (!RT_SUCCESS(rc))
-                {
-                    WARN(("VBoxVBVAExHPPause failed %d\n", rc));
-                    return rc;
-                }
-            }
-
-            return VINF_SUCCESS;
-        }
+            return rc;
+        }
+
+        /*
+         * See vdmaVBVACtlDisableSubmitInternal().
+         */
         case VBVAEXHOSTCTL_TYPE_GHH_DISABLE:
         {
@@ -1761 +1782 @@
             return VBoxVDMAThreadTerm(&pVdma->Thread, NULL, NULL, false);
         }
+
         default:
             WARN(("unexpected ctl type %d\n", pCmd->enmType));
@@ -2865 +2887 @@
 
 /**
- * Called by vgaR3Destruct() to do cleanup..
+ * Called by vgaR3Destruct() to do cleanup.
  */
 void vboxVDMADestruct(struct VBOXVDMAHOST *pVdma)
@@ -3007 +3029 @@
                                     uint8_t* pu8Cmd, uint32_t cbCmd, PFNVBVAEXHOSTCTL_COMPLETE pfnComplete, void *pvComplete)
 {
+    int            rc;
     VBVAEXHOSTCTL *pHCtl = VBoxVBVAExHCtlCreate(&pVdma->CmdVbva, enmType);
-    if (!pHCtl)
-    {
-        WARN(("VBoxVBVAExHCtlCreate failed\n"));
-        return VERR_NO_MEMORY;
-    }
-
-    pHCtl->u.cmd.pu8Cmd = pu8Cmd;
-    pHCtl->u.cmd.cbCmd = cbCmd;
-    int rc = vdmaVBVACtlSubmit(pVdma, pHCtl, enmSource, pfnComplete, pvComplete);
-    if (RT_FAILURE(rc))
-    {
+    if (pHCtl)
+    {
+        pHCtl->u.cmd.pu8Cmd = pu8Cmd;
+        pHCtl->u.cmd.cbCmd  = cbCmd;
+        rc = vdmaVBVACtlSubmit(pVdma, pHCtl, enmSource, pfnComplete, pvComplete);
+        if (RT_SUCCESS(rc))
+            return VINF_SUCCESS;
+
         VBoxVBVAExHCtlFree(&pVdma->CmdVbva, pHCtl);
         Log(("vdmaVBVACtlSubmit failed rc %d\n", rc));
-        return rc;;
-    }
-    return VINF_SUCCESS;
+    }
+    else
+    {
+        WARN(("VBoxVBVAExHCtlCreate failed\n"));
+        rc = VERR_NO_MEMORY;
+    }
+    return rc;
 }
 
@@ -3031 +3055 @@
 static int vdmaVBVACtlGenericGuestSubmit(PVBOXVDMAHOST pVdma, VBVAEXHOSTCTL_TYPE enmType, VBOXCMDVBVA_CTL *pCtl, uint32_t cbCtl)
 {
-    Assert(cbCtl >= sizeof (VBOXCMDVBVA_CTL));
+    Assert(cbCtl >= sizeof(VBOXCMDVBVA_CTL)); /* Checked by callers caller, vbvaChannelHandler(). */
+
     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;
@@ -3119 +3144 @@
 /**
  * Hook that is called by vboxVDMAWorkerThread when it starts.
+ *
+ * @thread VDMA
  */
 static DECLCALLBACK(void) vdmaVBVACtlThreadCreatedEnable(struct VBOXVDMATHREAD *pThread, int rc,
@@ -3162 +3189 @@
     if (pHCtl)
     {
-        pHCtl->u.cmd.pu8Cmd = (uint8_t*)pEnable;
-        pHCtl->u.cmd.cbCmd = sizeof (*pEnable);
-        pHCtl->pfnComplete = pfnComplete;
-        pHCtl->pvComplete = pvComplete;
+        pHCtl->u.cmd.pu8Cmd = (uint8_t *)pEnable;
+        pHCtl->u.cmd.cbCmd  = sizeof(*pEnable);
+        pHCtl->pfnComplete  = pfnComplete;
+        pHCtl->pvComplete   = pvComplete;
 
         rc = VBoxVDMAThreadCreate(&pVdma->Thread, vboxVDMAWorkerThread, pVdma, vdmaVBVACtlThreadCreatedEnable, pHCtl);
@@ -3528 +3555 @@
 /**
  * Handler for VBVA_CMDVBVA_CTL, see vbvaChannelHandler().
+ *
+ * @returns VBox status code
+ * @param   pVGAState           The VGA state.
+ * @param   pCtl                The control command.
+ * @param   cbCtl               The size of it.  This is at least
+ *                              sizeof(VBOXCMDVBVA_CTL).
  */
 int vboxCmdVBVACmdCtl(PVGASTATE pVGAState, VBOXCMDVBVA_CTL *pCtl, uint32_t cbCtl)
@@ -3536 +3569 @@
         case VBOXCMDVBVACTL_TYPE_3DCTL:
             return vdmaVBVACtlGenericGuestSubmit(pVdma, VBVAEXHOSTCTL_TYPE_GHH_BE_OPAQUE, pCtl, cbCtl);
+
         case VBOXCMDVBVACTL_TYPE_RESIZE:
             return vdmaVBVACtlGenericGuestSubmit(pVdma, VBVAEXHOSTCTL_TYPE_GHH_RESIZE, pCtl, cbCtl);
+
         case VBOXCMDVBVACTL_TYPE_ENABLE:
             if (cbCtl == sizeof(VBOXCMDVBVA_CTL_ENABLE))