Changeset 9662 in vbox for trunk

Timestamp:

Jun 12, 2008 2:48:02 PM (17 years ago)

Author:

vboxsync

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

31993

Message:

Implemented cancelling of HGCM requests from the guest.

Location:

trunk

Files:

• include/VBox/VBoxGuest.h (modified) (3 diffs)
• src/VBox/Additions/WINNT/VBoxGuest/VBoxGuest.cpp (modified) (1 diff)
• src/VBox/Additions/common/VBoxGuestLib/HGCMInternal.cpp (modified) (1 diff)
• src/VBox/Devices/VMMDev/VBoxDev.cpp (modified) (1 diff)
• src/VBox/Devices/VMMDev/VMMDevHGCM.cpp (modified) (9 diffs)
• src/VBox/Devices/VMMDev/VMMDevHGCM.h (modified) (1 diff)

trunk/include/VBox/VBoxGuest.h

@@ -169 +169 @@
     VMMDevReq_HGCMCall                   = 62,
 #endif /* VBOX_WITH_64_BITS_GUESTS */
+    VMMDevReq_HGCMCancel                 = 64,
 #endif
     VMMDevReq_VideoAccelEnable           = 70,
@@ -791 +792 @@
 
 #define VBOX_HGCM_MAX_PARMS 32
+
+/* The Cancel request is issued using the same physical memory address
+ * as was used for the corresponding initial HGCMCall.
+ */
+typedef struct
+{
+    /* request header */
+    VMMDevHGCMRequestHeader header;
+} VMMDevHGCMCancel;
 
 #endif /* VBOX_HGCM */
@@ -1372 +1382 @@
             return sizeof(VMMDevHGCMCall);
 #endif /* VBOX_WITH_64_BITS_GUESTS */
+        case VMMDevReq_HGCMCancel:
+            return sizeof(VMMDevHGCMCancel);
 #endif /* VBOX_HGCM */
         case VMMDevReq_VideoAccelEnable:

trunk/src/VBox/Additions/WINNT/VBoxGuest/VBoxGuest.cpp

@@ -370 +370 @@
         if (rc != STATUS_WAIT_0)
         {
-            dprintf(("VBoxHGCMCallback: The external event was signalled or the wait timed out or terminated.\n"));
+            dprintf(("VBoxHGCMCallback: The external event was signalled or the wait timed out or terminated rc = 0x%08X.\n", rc));
             break;
         }

trunk/src/VBox/Additions/common/VBoxGuestLib/HGCMInternal.cpp

@@ -249 +249 @@
                      * that means the wait was interrrupted. That can happen
                      * if system reboots or the VBoxService ended abnormally.
-                     * In both cases it is OK to just leave the allocated memory
-                     * in the physical heap. The memory leak does not affect normal
-                     * operations.
-                     * @todo VbglGRCancel (&pHGCMCall->header.header) need to be implemented.
-                     *       The host will not write to the cancelled memory.
+                     *
+                     * Cancel the request, the host will not write to the
+                     * memory related to the cancelled request.
                      */
                     pHGCMCall->header.fu32Flags |= VBOX_HGCM_REQ_CANCELLED;
+
+                    pHGCMCall->header.header.requestType = VMMDevReq_HGCMCancel;
+                    VbglGRPerform (&pHGCMCall->header.header);
                 }
             }

trunk/src/VBox/Devices/VMMDev/VBoxDev.cpp

@@ -1100 +1100 @@
         }
 #endif /* VBOX_HGCM */
+
+        case VMMDevReq_HGCMCancel:
+        {
+            if (pRequestHeader->size < sizeof(VMMDevHGCMCancel))
+            {
+                AssertMsgFailed(("VMMDevReq_HGCMCancel structure has invalid size!\n"));
+                pRequestHeader->rc = VERR_INVALID_PARAMETER;
+            }
+            else if (!pData->pHGCMDrv)
+            {
+                Log(("VMMDevReq_HGCMCancel HGCM Connector is NULL!\n"));
+                pRequestHeader->rc = VERR_NOT_SUPPORTED;
+            }
+            else
+            {
+                VMMDevHGCMCancel *pHGCMCancel = (VMMDevHGCMCancel *)pRequestHeader;
+
+                Log(("VMMDevReq_VMMDevHGCMCancel\n"));
+                pRequestHeader->rc = vmmdevHGCMCancel (pData, pHGCMCancel, (RTGCPHYS)u32);
+            }
+            break;
+        }
 
         case VMMDevReq_VideoAccelEnable:

trunk/src/VBox/Devices/VMMDev/VMMDevHGCM.cpp

@@ -74 +74 @@
     VBOXHGCMCMDTYPE enmCmdType;
 
+    /* Whether the command was cancelled by the guest. */
+    bool fCancelled;
+
     /* GC physical address of the guest request. */
     RTGCPHYS        GCPhys;
@@ -179 +182 @@
     return rc;
 }
+
+
+static PVBOXHGCMCMD vmmdevHGCMFindCommand (VMMDevState *pVMMDevState, RTGCPHYS GCPhys)
+{
+    PVBOXHGCMCMD pCmd = NULL;
+
+    int rc = vmmdevHGCMCmdListLock (pVMMDevState);
+    
+    if (VBOX_SUCCESS (rc))
+    {
+        pCmd = pVMMDevState->pHGCMCmdList;
+
+        while (pCmd)
+        {
+            if (pCmd->GCPhys == GCPhys)
+            {
+                break;
+            }
+            pCmd = pCmd->pNext;
+        }
+
+        vmmdevHGCMCmdListUnlock (pVMMDevState);
+    }
+    
+    LogFlowFunc(("%p\n", pCmd));
+    return pCmd;
+}
+
 
 static int vmmdevHGCMSaveLinPtr (PPDMDEVINS pDevIns,
@@ -746 +777 @@
 }
 
+/* @thread EMT */
+int vmmdevHGCMCancel (VMMDevState *pVMMDevState, VMMDevHGCMCancel *pHGCMCancel, RTGCPHYS GCPhys)
+{
+    int rc = VINF_SUCCESS;
+
+    NOREF(pHGCMCancel);
+
+    Log(("vmmdevHGCMCancel\n"));
+
+    /* Find the command in the list. */
+    PVBOXHGCMCMD pCmd = vmmdevHGCMFindCommand (pVMMDevState, GCPhys);
+
+    if (pCmd)
+    {
+        pCmd->fCancelled = true;
+    }
+    else
+    {
+        rc = VERR_INVALID_PARAMETER;
+    }
+
+    return rc;
+}
+
 static int vmmdevHGCMCmdVerify (PVBOXHGCMCMD pCmd, VMMDevHGCMRequestHeader *pHeader)
 {
@@ -751 +806 @@
     {
         case VBOXHGCMCMDTYPE_CONNECT:
-            if (pHeader->header.requestType == VMMDevReq_HGCMConnect) return VINF_SUCCESS;
+            if (   pHeader->header.requestType == VMMDevReq_HGCMConnect
+                || pHeader->header.requestType == VMMDevReq_HGCMCancel) return VINF_SUCCESS;
             break;
 
         case VBOXHGCMCMDTYPE_DISCONNECT:
-            if (pHeader->header.requestType == VMMDevReq_HGCMDisconnect) return VINF_SUCCESS;
+            if (   pHeader->header.requestType == VMMDevReq_HGCMDisconnect
+                || pHeader->header.requestType == VMMDevReq_HGCMCancel) return VINF_SUCCESS;
             break;
 
@@ -761 +818 @@
 #ifdef VBOX_WITH_64_BITS_GUESTS
             if (   pHeader->header.requestType == VMMDevReq_HGCMCall32
-                || pHeader->header.requestType == VMMDevReq_HGCMCall64) return VINF_SUCCESS;
+                || pHeader->header.requestType == VMMDevReq_HGCMCall64
+                || pHeader->header.requestType == VMMDevReq_HGCMCancel) return VINF_SUCCESS;
 #else
-            if (   pHeader->header.requestType == VMMDevReq_HGCMCall) return VINF_SUCCESS;
+            if (   pHeader->header.requestType == VMMDevReq_HGCMCall
+                || pHeader->header.requestType == VMMDevReq_HGCMCancel) return VINF_SUCCESS;
 #endif /* VBOX_WITH_64_BITS_GUESTS */
 
@@ -781 +840 @@
 DECLCALLBACK(void) hgcmCompletedWorker (PPDMIHGCMPORT pInterface, int32_t result, PVBOXHGCMCMD pCmd)
 {
-    VMMDevState             *pVMMDevState = PDMIHGCMPORT_2_VMMDEVSTATE(pInterface);
-    VMMDevHGCMRequestHeader *pHeader;
-    int                      rc = VINF_SUCCESS;
-
-    pHeader = (VMMDevHGCMRequestHeader *)RTMemAllocZ (pCmd->cbSize);
-    Assert(pHeader);
-    if (pHeader == NULL)
+    VMMDevState *pVMMDevState = PDMIHGCMPORT_2_VMMDEVSTATE(pInterface);
+
+    int rc = VINF_SUCCESS;
+
+    if (result == VINF_HGCM_SAVE_STATE)
+    {
+        /* If the completion routine was called because HGCM saves its state,
+         * then currently nothing to be done here. The pCmd stays in the list
+         * and will be saved later when the VMMDev state will be saved.
+         *
+         * It it assumed that VMMDev saves state after the HGCM services,
+         * and, therefore, VBOXHGCMCMD structures are not removed by
+         * vmmdevHGCMSaveState from the list, while HGCM uses them.
+         */
+        LogFlowFunc(("VINF_HGCM_SAVE_STATE for command %p\n", pCmd));
         return;
-
-    PDMDevHlpPhysRead(pVMMDevState->pDevIns, (RTGCPHYS)pCmd->GCPhys, pHeader, pCmd->cbSize);
-
-    if (result != VINF_HGCM_SAVE_STATE)
-    {
+    }
+
+    /* Check whether the command has been already cancelled by the guest.
+     * If it was cancelled, then the data must not be written back to the
+     * guest RAM.
+     */
+    if (pCmd->fCancelled)
+    {
+        /* Just remove the command from the internal list, so the memory can be freed. */
+        LogFlowFunc(("A cancelled command %p\n", pCmd));
+        vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
+    }
+    else
+    {
+        /* Preallocated block for requests which have up to 8 parameters (most of requests). */
+#ifdef VBOX_WITH_64_BITS_GUESTS
+        uint8_t au8Prealloc[sizeof (VMMDevHGCMCall) + 8 * sizeof (HGCMFunctionParameter64)];
+#else
+        uint8_t au8Prealloc[sizeof (VMMDevHGCMCall) + 8 * sizeof (HGCMFunctionParameter)];
+#endif /* VBOX_WITH_64_BITS_GUESTS */
+
+        VMMDevHGCMRequestHeader *pHeader;
+
+        if (pCmd->cbSize <= sizeof (au8Prealloc))
+        {
+            pHeader = (VMMDevHGCMRequestHeader *)&au8Prealloc[0];
+        }
+        else
+        {
+            pHeader = (VMMDevHGCMRequestHeader *)RTMemAlloc (pCmd->cbSize);
+            Assert(pHeader);
+            if (pHeader == NULL)
+            {
+                LogRel(("VMMDev: Failed to allocate %d bytes for HGCM request completion!!!\n", pCmd->cbSize));
+
+                /* Do some cleanup. The command have to be excluded from list of active commands anyway. */
+                vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
+                return;
+            }
+        }
+
+        PDMDevHlpPhysRead(pVMMDevState->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
+
         /* Setup return codes. */
         pHeader->result = result;
@@ -995 +1100 @@
         else
         {
-            /* Return error to the guest. */
+            /* Command type is wrong. Return error to the guest. */
             pHeader->header.rc = rc;
         }
 
-        /* Mark request as processed*/
+        /* Mark request as processed. */
         pHeader->fu32Flags |= VBOX_HGCM_REQ_DONE;
 
@@ -1005 +1110 @@
         PDMDevHlpPhysWrite(pVMMDevState->pDevIns, pCmd->GCPhys, pHeader, pCmd->cbSize);
 
-        /* It it assumed that VMMDev saves state after the HGCM services. */
+        /* The command has been completely processed and can be removed from the list. */
         vmmdevHGCMRemoveCommand (pVMMDevState, pCmd);
 
@@ -1011 +1116 @@
         VMMDevNotifyGuest (pVMMDevState, VMMDEV_EVENT_HGCM);
 
-        if (pCmd->paLinPtrs)
-        {
-            RTMemFree (pCmd->paLinPtrs);
-        }
-        
-        RTMemFree (pCmd);
-    }
-    RTMemFree(pHeader);
+        if ((uint8_t *)pHeader != &au8Prealloc[0])
+        {
+            /* Only if it was allocated from heap. */
+            RTMemFree (pHeader);
+        }
+    }
+
+    /* Deallocate the command memory. */
+    if (pCmd->paLinPtrs)
+    {
+        RTMemFree (pCmd->paLinPtrs);
+    }
+        
+    RTMemFree (pCmd);
 
     return;

trunk/src/VBox/Devices/VMMDev/VMMDevHGCM.h

@@ -35 +35 @@
 DECLCALLBACK(int) vmmdevHGCMDisconnect (VMMDevState *pVMMDevState, VMMDevHGCMDisconnect *pHGCMDisconnect, RTGCPHYS GCPtr);
 DECLCALLBACK(int) vmmdevHGCMCall (VMMDevState *pVMMDevState, VMMDevHGCMCall *pHGCMCall, RTGCPHYS GCPtr, bool f64Bits);
+DECLCALLBACK(int) vmmdevHGCMCancel (VMMDevState *pVMMDevState, VMMDevHGCMCancel *pHGCMCancel, RTGCPHYS GCPtr);
 
 DECLCALLBACK(void) hgcmCompleted (PPDMIHGCMPORT pInterface, int32_t result, PVBOXHGCMCMD pCmdPtr);