Changeset 58390 in vbox

Timestamp:

Oct 23, 2015 12:35:35 PM (9 years ago)

Author:

vboxsync

Message:

VMM/GIM: Implement Hyper-V debug receive thread optimization. Added a few statistics to GIM.
Fixed a bug in Windows guests' debug DHCP handling.

Location:

trunk

Files:

• include/VBox/vmm/gim.h (modified) (2 diffs)
• src/VBox/Devices/GIMDev/DrvUDP.cpp (modified) (3 diffs)
• src/VBox/Devices/GIMDev/GIMDev.cpp (modified) (5 diffs)
• src/VBox/VMM/VMMAll/GIMAll.cpp (modified) (1 diff)
• src/VBox/VMM/VMMAll/GIMAllHv.cpp (modified) (9 diffs)
• src/VBox/VMM/VMMR3/GIM.cpp (modified) (5 diffs)
• src/VBox/VMM/VMMR3/GIMHv.cpp (modified) (22 diffs)
• src/VBox/VMM/include/GIMHvInternal.h (modified) (3 diffs)
• src/VBox/VMM/include/GIMInternal.h (modified) (4 diffs)

trunk/include/VBox/vmm/gim.h

@@ -100 +100 @@
 AssertCompileMemberAlignment(GIMMMIO2REGION, pvPageR0, 8);
 
-#if 0
 /**
- * A GIM Hypercall handler.
+ * Debug data buffer available callback over the GIM debug connection.
  *
- * @param   pVCpu   The cross context virtual CPU structure of the calling EMT.
- * @param   pCtx    Pointer to the guest-CPU context.
+ * @param   pVM             The cross context VM structure.
  */
-typedef DECLCALLBACK(int) FNGIMHYPERCALL(PVMCPU pVCpu, PCPUMCTX pCtx);
-/** Pointer to a GIM hypercall handler. */
-typedef FNGIMHYPERCALL *PFNGIMHYPERCALL;
+typedef DECLCALLBACK(void) FNGIMDEBUGBUFAVAIL(PVM pVM);
+/** Pointer to GIM debug buffer available callback. */
+typedef FNGIMDEBUGBUFAVAIL *PFNGIMDEBUGBUFAVAIL;
 
 /**
- * A GIM MSR-read handler.
+ * GIM debug setup.
  *
- * @returns VBox status code.
- * @param   pVCpu   The cross context virtual CPU structure of the calling EMT.
- * @param   idMsr   The MSR being read.
- * @param   pRange  The range that the MSR belongs to.
- * @param   puValue Where to store the value of the MSR.
+ * These are parameters/options filled in by the GIM provider and passed along
+ * to the GIM device.
  */
-typedef DECLCALLBACK(int) FNGIMRDMSR(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t *puValue);
-/** Pointer to a GIM MSR-read handler. */
-typedef FNGIMRDMSR *PFNGIMRDMSR;
+typedef struct GIMDEBUGSETUP
+{
+    /** The callback to invoke when the receive buffer has data. */
+    PFNGIMDEBUGBUFAVAIL     pfnDbgRecvBufAvail;
+    /** The size of the receive buffer as specified by the GIM provider. */
+    uint32_t                cbDbgRecvBuf;
+} GIMDEBUGSETUP;
+/** Pointer to a GIM debug setup struct. */
+typedef struct GIMDEBUGSETUP *PGIMDEBUGSETUP;
+/** Pointer to a const GIM debug setup struct. */
+typedef struct GIMDEBUGSETUP const *PCGGIMDEBUGSETUP;
 
 /**
- * A GIM MSR-write handler.
+ * GIM debug structure (common to the GIM device and GIM).
  *
- * @returns VBox status code.
- * @param   pVCpu       The cross context virtual CPU structure of the calling EMT.
- * @param   idMsr       The MSR being written.
- * @param   pRange      The range that the MSR belongs to.
- * @param   uValue      The value to set, ignored bits masked.
- * @param   uRawValue   The raw value with the ignored bits not masked.
+ * This is used to exchanging data between the GIM provider and the GIM device.
  */
-typedef DECLCALLBACK(int) FNGIMWRMSR(PVMCPU pVCpu, uint32_t idMsr, PCCPUMMSRRANGE pRange, uint64_t uValue, uint64_t uRawValue);
-/** Pointer to a GIM MSR-write handler. */
-typedef FNGIMWRMSR *PFNGIMWRMSR;
-#endif
+typedef struct GIMDEBUG
+{
+    /** The receive buffer. */
+    void                   *pvDbgRecvBuf;
+    /** The debug I/O stream driver. */
+    PPDMISTREAM             pDbgDrvStream;
+    /** Number of bytes pending to be read from the receive buffer. */
+    uint32_t                cbDbgRecvBufRead;
+    /** The flag synchronizing reads of the receive buffer from EMT. */
+    volatile bool           fDbgRecvBufRead;
+    /** The receive thread wakeup semaphore. */
+    RTSEMEVENTMULTI         hDbgRecvThreadSem;
+} GIMDEBUG;
+/** Pointer to a GIM debug struct. */
+typedef struct GIMDEBUG *PGIMDEBUG;
+/** Pointer to a const GIM debug struct. */
+typedef struct GIMDEBUG const *PCGIMDEBUG;
 
 
@@ -166 +177 @@
 VMMR3_INT_DECL(int)         GIMR3Term(PVM pVM);
 VMMR3_INT_DECL(void)        GIMR3Reset(PVM pVM);
-VMMR3DECL(void)             GIMR3GimDeviceRegister(PVM pVM, PPDMDEVINS pDevInsR3, PPDMISTREAM pDebugStreamR3);
+VMMR3DECL(void)             GIMR3GimDeviceRegister(PVM pVM, PPDMDEVINS pDevInsR3, PGIMDEBUG pDbg);
+VMMR3DECL(int)              GIMR3GetDebugSetup(PVM pVM, PGIMDEBUGSETUP pDbgSetup);
 VMMR3DECL(PGIMMMIO2REGION)  GIMR3GetMmio2Regions(PVM pVM, uint32_t *pcRegions);
 /** @} */

trunk/src/VBox/Devices/GIMDev/DrvUDP.cpp

@@ -80 +80 @@
     {
         size_t cbReallyRead = 0;
-        rc = RTSocketReadNB(pThis->hSocket, pvBuf, *pcbRead, &cbReallyRead);
+        rc = RTSocketRead(pThis->hSocket, pvBuf, *pcbRead, &cbReallyRead);
         if (RT_SUCCESS(rc))
             *pcbRead = cbReallyRead;
@@ -103 +103 @@
     if (pThis->hSocket != NIL_RTSOCKET)
     {
-        size_t cbReallyWritten = 0;
-        rc = RTSocketWriteNB(pThis->hSocket, pvBuf, *pcbWrite, &cbReallyWritten);
+        size_t cbBuf = *pcbWrite;
+        rc = RTSocketWriteToNB(pThis->hSocket, pvBuf, cbBuf, NULL /*pDstAddr*/);
         if (RT_SUCCESS(rc))
-            *pcbWrite = cbReallyWritten;
+            *pcbWrite = cbBuf;
     }
     else
@@ -147 +147 @@
     if (pThis->hSocket != NIL_RTSOCKET)
     {
+        RTSocketRetain(pThis->hSocket);
+        RTSocketShutdown(pThis->hSocket, true, true);
         RTSocketClose(pThis->hSocket);
         pThis->hSocket = NIL_RTSOCKET;

trunk/src/VBox/Devices/GIMDev/GIMDev.cpp

@@ -26 +26 @@
 
 #include "VBoxDD.h"
+#include <iprt/alloc.h>
+#include <iprt/semaphore.h>
 #include <iprt/uuid.h>
 
@@ -50 +52 @@
     /** Pointer to the attached base debug driver. */
     R3PTRTYPE(PPDMIBASE)            pDbgDrvBase;
-    /** Pointer to the attached debug stream driver. */
-    R3PTRTYPE(PPDMISTREAM)          pDbgDrvStream;
+    /** The debug receive thread. */
+    RTTHREAD                        hDbgRecvThread;
+    /** Flag to indicate shutdown of the debug receive thread. */
+    bool volatile                   fDbgRecvThreadShutdown;
+    /** The debug setup parameters. */
+    GIMDEBUGSETUP                   DbgSetup;
+    /** The debug transfer struct. */
+    GIMDEBUG                        Dbg;
 } GIMDEV;
 /** Pointer to the GIM device state. */
@@ -76 +84 @@
 
 
+static DECLCALLBACK(int) gimDevR3DbgRecvThread(RTTHREAD ThreadSelf, void *pvUser)
+{
+    /*
+     * Validate.
+     */
+    PPDMDEVINS pDevIns = (PPDMDEVINS)pvUser;
+    AssertReturn(pDevIns, VERR_INVALID_PARAMETER);
+    PDMDEV_CHECK_VERSIONS_RETURN(pDevIns);
+
+    PGIMDEV pThis = PDMINS_2_DATA(pDevIns, PGIMDEV);
+    AssertReturn(pThis, VERR_INVALID_POINTER);
+    AssertReturn(pThis->DbgSetup.cbDbgRecvBuf, VERR_INTERNAL_ERROR);
+    AssertReturn(pThis->Dbg.hDbgRecvThreadSem != NIL_RTSEMEVENTMULTI, VERR_INTERNAL_ERROR_2);
+    AssertReturn(pThis->Dbg.pvDbgRecvBuf, VERR_INTERNAL_ERROR_3);
+
+    PVM pVM = PDMDevHlpGetVM(pDevIns);
+    AssertReturn(pVM, VERR_INVALID_POINTER);
+
+    PPDMISTREAM pDbgDrvStream = pThis->Dbg.pDbgDrvStream;
+    AssertReturn(pDbgDrvStream, VERR_INVALID_POINTER);
+
+    for (;;)
+    {
+        /*
+         * Read incoming debug data.
+         */
+        size_t cbRead = pThis->DbgSetup.cbDbgRecvBuf;
+        int rc = pDbgDrvStream->pfnRead(pDbgDrvStream, pThis->Dbg.pvDbgRecvBuf, &cbRead);
+        if (   RT_SUCCESS(rc)
+            && cbRead > 0)
+        {
+            /*
+             * Notify the consumer thread.
+             */
+            if (ASMAtomicReadBool(&pThis->Dbg.fDbgRecvBufRead) == false)
+            {
+                if (pThis->DbgSetup.pfnDbgRecvBufAvail)
+                    pThis->DbgSetup.pfnDbgRecvBufAvail(pVM);
+                pThis->Dbg.cbDbgRecvBufRead = cbRead;
+                RTSemEventMultiReset(pThis->Dbg.hDbgRecvThreadSem);
+                ASMAtomicWriteBool(&pThis->Dbg.fDbgRecvBufRead, true);
+            }
+
+            /*
+             * Wait until the consumer thread has acknowledged reading of the
+             * current buffer or we're asked to shut down.
+             *
+             * It is important that we do NOT re-invoke 'pfnRead' before the
+             * current buffer is consumed, otherwise we risk data corruption.
+             */
+            while (   ASMAtomicReadBool(&pThis->Dbg.fDbgRecvBufRead) == true
+                   && !pThis->fDbgRecvThreadShutdown)
+            {
+                RTSemEventMultiWait(pThis->Dbg.hDbgRecvThreadSem, RT_INDEFINITE_WAIT);
+            }
+        }
+        else if (   rc != VINF_TRY_AGAIN
+                 && rc != VERR_TRY_AGAIN)
+        {
+            LogRel(("GIMDev: Debug thread terminating with rc=%Rrc\n", rc));
+            break;
+        }
+
+        if (pThis->fDbgRecvThreadShutdown)
+        {
+            LogRel(("GIMDev: Debug thread shutting down\n"));
+            break;
+        }
+    }
+
+    return VINF_SUCCESS;
+}
+
+
 /**
  * @interface_method_impl{PDMDEVREG,pfnConstruct}
@@ -93 +175 @@
 
     /*
-     * Attach the stream driver for the debug connection.
-     */
-    pThis->IDbgBase.pfnQueryInterface = gimdevR3QueryInterface;
-    int rc = PDMDevHlpDriverAttach(pDevIns, GIMDEV_DEBUG_LUN, &pThis->IDbgBase, &pThis->pDbgDrvBase, "GIM Debug Port");
-    if (RT_SUCCESS(rc))
-    {
-        pThis->pDbgDrvStream = PDMIBASE_QUERY_INTERFACE(pThis->pDbgDrvBase, PDMISTREAM);
-        if (pThis->pDbgDrvStream)
-            LogRel(("GIMDev: LUN#%u: Debug port configured\n", GIMDEV_DEBUG_LUN));
+     * Get debug setup requirements from GIM.
+     */
+    PVM pVM = PDMDevHlpGetVM(pDevIns);
+    int rc = GIMR3GetDebugSetup(pVM, &pThis->DbgSetup);
+    if (   RT_SUCCESS(rc)
+        && pThis->DbgSetup.cbDbgRecvBuf > 0)
+    {
+        /*
+         * Attach the stream driver for the debug connection.
+         */
+        PPDMISTREAM pDbgDrvStream = NULL;
+        pThis->IDbgBase.pfnQueryInterface = gimdevR3QueryInterface;
+        rc = PDMDevHlpDriverAttach(pDevIns, GIMDEV_DEBUG_LUN, &pThis->IDbgBase, &pThis->pDbgDrvBase, "GIM Debug Port");
+        if (RT_SUCCESS(rc))
+        {
+            pDbgDrvStream = PDMIBASE_QUERY_INTERFACE(pThis->pDbgDrvBase, PDMISTREAM);
+            if (pDbgDrvStream)
+                LogRel(("GIMDev: LUN#%u: Debug port configured\n", GIMDEV_DEBUG_LUN));
+            else
+                LogRel(("GIMDev: LUN#%u: No unit\n", GIMDEV_DEBUG_LUN));
+        }
+        else if (rc == VERR_PDM_NO_ATTACHED_DRIVER)
+        {
+            pThis->pDbgDrvBase = NULL;
+            LogRel(("GIMDev: LUN#%u: No debug port configured\n", GIMDEV_DEBUG_LUN));
+        }
         else
-            LogRel(("GIMDev: LUN#%u: No unit\n", GIMDEV_DEBUG_LUN));
-    }
-    else if (rc == VERR_PDM_NO_ATTACHED_DRIVER)
-    {
-        pThis->pDbgDrvBase   = NULL;
-        pThis->pDbgDrvStream = NULL;
-        LogRel(("GIMDev: LUN#%u: No debug port configured\n", GIMDEV_DEBUG_LUN));
-    }
-    else
-    {
-        AssertLogRelMsgFailed(("GIMDev: LUN#%u: Failed to attach to driver on debug port. rc=%Rrc\n", GIMDEV_DEBUG_LUN, rc));
-        /* Don't call VMSetError here as we assume that the driver already set an appropriate error */
-        return rc;
-    }
-
-    /*
-     * Register ourselves with the GIM VMM component.
-     */
-    PVM pVM = PDMDevHlpGetVM(pDevIns);
-    GIMR3GimDeviceRegister(pVM, pDevIns, pThis->pDbgDrvStream);
+        {
+            AssertLogRelMsgFailed(("GIMDev: LUN#%u: Failed to attach to driver on debug port. rc=%Rrc\n", GIMDEV_DEBUG_LUN, rc));
+            /* Don't call VMSetError here as we assume that the driver already set an appropriate error */
+            return rc;
+        }
+
+        void *pvDbgRecvBuf = RTMemAllocZ(pThis->DbgSetup.cbDbgRecvBuf);
+        if (RT_UNLIKELY(!pvDbgRecvBuf))
+        {
+            LogRel(("GIMDev: Failed to alloc %u bytes for debug receive buffer\n", pThis->DbgSetup.cbDbgRecvBuf));
+            return VERR_NO_MEMORY;
+        }
+
+        /*
+         * Update the shared debug struct.
+         */
+        pThis->Dbg.pDbgDrvStream    = pDbgDrvStream;
+        pThis->Dbg.pvDbgRecvBuf     = pvDbgRecvBuf;
+        pThis->Dbg.cbDbgRecvBufRead = 0;
+        pThis->Dbg.fDbgRecvBufRead  = false;
+
+        /*
+         * Create the sempahore and the debug receive thread itself.
+         */
+        rc = RTSemEventMultiCreate(&pThis->Dbg.hDbgRecvThreadSem);
+        if (RT_SUCCESS(rc))
+        {
+            rc = RTThreadCreate(&pThis->hDbgRecvThread, gimDevR3DbgRecvThread, pDevIns, 0 /*cbStack*/, RTTHREADTYPE_IO,
+                                RTTHREADFLAGS_WAITABLE, "GIMDebugRecv");
+            if (RT_FAILURE(rc))
+            {
+                RTSemEventMultiDestroy(pThis->Dbg.hDbgRecvThreadSem);
+                pThis->Dbg.hDbgRecvThreadSem = NIL_RTSEMEVENTMULTI;
+
+                RTMemFree(pThis->Dbg.pvDbgRecvBuf);
+                pThis->Dbg.pvDbgRecvBuf = NULL;
+                return rc;
+            }
+        }
+        else
+            return rc;
+    }
+
+    /*
+     * Register this device with the GIM component.
+     */
+    GIMR3GimDeviceRegister(pVM, pDevIns, pThis->DbgSetup.cbDbgRecvBuf ? &pThis->Dbg : NULL);
 
     /*
@@ -197 +323 @@
         if (RT_FAILURE(rc))
             return rc;
+    }
+
+    /*
+     * Signal and wait for the debug thread to terminate.
+     */
+    if (pThis->hDbgRecvThread != NIL_RTTHREAD)
+    {
+        pThis->fDbgRecvThreadShutdown = true;
+        if (pThis->Dbg.hDbgRecvThreadSem != NIL_RTSEMEVENT)
+            RTSemEventMultiSignal(pThis->Dbg.hDbgRecvThreadSem);
+
+        int rc = RTThreadWait(pThis->hDbgRecvThread, 20000, NULL /*prc*/);
+        if (RT_SUCCESS(rc))
+            pThis->hDbgRecvThread = NIL_RTTHREAD;
+        else
+        {
+            LogRel(("GIMDev: Debug thread did not terminate, rc=%Rrc!\n", rc));
+            return VERR_RESOURCE_BUSY;
+        }
+    }
+
+    /*
+     * Now clean up the semaphore & buffer now that the thread is gone.
+     */
+    if (pThis->Dbg.hDbgRecvThreadSem != NIL_RTSEMEVENT)
+    {
+        RTSemEventMultiDestroy(pThis->Dbg.hDbgRecvThreadSem);
+        pThis->Dbg.hDbgRecvThreadSem = NIL_RTSEMEVENTMULTI;
+    }
+    if (pThis->Dbg.pvDbgRecvBuf)
+    {
+        RTMemFree(pThis->Dbg.pvDbgRecvBuf);
+        pThis->Dbg.pvDbgRecvBuf = NULL;
     }
 

trunk/src/VBox/VMM/VMMAll/GIMAll.cpp

@@ -99 +99 @@
         return VERR_GIM_NOT_ENABLED;
 
+    STAM_COUNTER_INC(&pVM->gim.s.StatHypercalls);
     switch (pVM->gim.s.enmProviderId)
     {

trunk/src/VBox/VMM/VMMAll/GIMAllHv.cpp

@@ -242 +242 @@
 
                     /*
-                     * Since we don't really maintain our own buffers for the debug
-                     * communication channel, we don't have anything to flush.
+                     * Nothing to flush on the sending side as we don't maintain our own buffers.
                      */
+                    /** @todo We should probably ask the debug receive thread to flush it's buffer. */
                     if (rcHv == GIM_HV_STATUS_SUCCESS)
                     {
                         if (fFlags)
-                            LogRelMax(1, ("GIM: HyperV: Resetting debug session via hypercall\n"));
+                            LogRel(("GIM: HyperV: Resetting debug session via hypercall\n"));
                         else
                             rcHv = GIM_HV_STATUS_INVALID_PARAMETER;
@@ -470 +470 @@
 
         case MSR_GIM_HV_SYNTH_DEBUG_STATUS:
-            *puValue = pHv->uDebugStatusMsr;
+            *puValue = pHv->uDbgStatusMsr;
             return VINF_SUCCESS;
 
@@ -508 +508 @@
                 return VINF_CPUM_R3_MSR_READ;
 #else
-                LogRelMax(1, ("GIM: HyperV: Guest queried debug options MSR\n"));
-                *puValue = GIM_HV_DEBUG_OPTIONS_MSR_ENABLE;
+                LogRelMax(1, ("GIM: HyperV: Guest querying debug options, suggesting %s interface\n",
+                              pHv->fDbgHypercallInterface ? "hypercall" : "MSR"));
+                *puValue = pHv->fDbgHypercallInterface ? GIM_HV_DEBUG_OPTIONS_USE_HYPERCALLS : 0;
                 return VINF_SUCCESS;
 #endif
@@ -724 +725 @@
         case MSR_GIM_HV_SYNTH_DEBUG_SEND_BUFFER:
         {
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
 #ifndef IN_RING3
             return VINF_CPUM_R3_MSR_WRITE;
 #else
             RTGCPHYS GCPhysBuffer    = (RTGCPHYS)uRawValue;
-            pHv->uDebugSendBufferMsr = GCPhysBuffer;
+            pHv->uDbgSendBufferMsr = GCPhysBuffer;
             if (PGMPhysIsGCPhysNormal(pVM, GCPhysBuffer))
                 LogRel(("GIM: HyperV: Set up debug send buffer at %#RGp\n", GCPhysBuffer));
             else
                 LogRel(("GIM: HyperV: Destroyed debug send buffer\n"));
-            pHv->uDebugSendBufferMsr = uRawValue;
+            pHv->uDbgSendBufferMsr = uRawValue;
             return VINF_SUCCESS;
 #endif
@@ -740 +743 @@
         case MSR_GIM_HV_SYNTH_DEBUG_RECEIVE_BUFFER:
         {
-#ifndef IN_RING3
-            return VINF_CPUM_R3_MSR_WRITE;
-#else
-            RTGCPHYS GCPhysBuffer    = (RTGCPHYS)uRawValue;
-            pHv->uDebugRecvBufferMsr = GCPhysBuffer;
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
+#ifndef IN_RING3
+            return VINF_CPUM_R3_MSR_WRITE;
+#else
+            RTGCPHYS GCPhysBuffer  = (RTGCPHYS)uRawValue;
+            pHv->uDbgRecvBufferMsr = GCPhysBuffer;
             if (PGMPhysIsGCPhysNormal(pVM, GCPhysBuffer))
                 LogRel(("GIM: HyperV: Set up debug receive buffer at %#RGp\n", GCPhysBuffer));
@@ -755 +760 @@
         case MSR_GIM_HV_SYNTH_DEBUG_PENDING_BUFFER:
         {
-#ifndef IN_RING3
-            return VINF_CPUM_R3_MSR_WRITE;
-#else
-            RTGCPHYS GCPhysBuffer        = (RTGCPHYS)uRawValue;
-            pHv->uDebugPendingBufferMsr  = GCPhysBuffer;
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
+#ifndef IN_RING3
+            return VINF_CPUM_R3_MSR_WRITE;
+#else
+            RTGCPHYS GCPhysBuffer      = (RTGCPHYS)uRawValue;
+            pHv->uDbgPendingBufferMsr  = GCPhysBuffer;
             if (PGMPhysIsGCPhysNormal(pVM, GCPhysBuffer))
-            {
                 LogRel(("GIM: HyperV: Set up debug pending buffer at %#RGp\n", uRawValue));
-
-                /* Indicate that there is always debug data to be read (guest will poll). */
-                /** @todo Later implement this by doing the transport read in a separate
-                 *        thread and updating this page, saves lots of VM-exits as the guest
-                 *        won't poll and fail all the time. */
-                uint8_t uPendingData = 1;
-                int rc = PGMPhysSimpleWriteGCPhys(pVM, GCPhysBuffer, (void *)&uPendingData, sizeof(uPendingData));
-                if (RT_FAILURE(rc))
-                    LogRelMax(5, ("GIM: HyperV: Failed to update pending buffer at %#RGp, rc=%Rrc\n", GCPhysBuffer, rc));
-            }
             else
                 LogRel(("GIM: HyperV: Destroyed debug pending buffer\n"));
@@ -781 +777 @@
         case MSR_GIM_HV_SYNTH_DEBUG_CONTROL:
         {
-#ifndef IN_RING3
-            return VINF_CPUM_R3_MSR_WRITE;
-#else
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
+#ifndef IN_RING3
+            return VINF_CPUM_R3_MSR_WRITE;
+#else
+            if (   MSR_GIM_HV_SYNTH_DEBUG_CONTROL_IS_WRITE(uRawValue)
+                && MSR_GIM_HV_SYNTH_DEBUG_CONTROL_IS_READ(uRawValue))
+            {
+                LogRel(("GIM: HyperV: Requesting both read and write through debug control MSR -> #GP(0)\n"));
+                return VERR_CPUM_RAISE_GP_0;
+            }
+
             if (MSR_GIM_HV_SYNTH_DEBUG_CONTROL_IS_WRITE(uRawValue))
             {
-                LogRelMax(1, ("GIM: HyperV: Initiated debug data transmission via MSR\n",
-                              MSR_GIM_HV_SYNTH_DEBUG_CONTROL_W_LEN(uRawValue)));
                 uint32_t cbWrite = MSR_GIM_HV_SYNTH_DEBUG_CONTROL_W_LEN(uRawValue);
                 if (   cbWrite > 0
                     && cbWrite < GIM_HV_PAGE_SIZE)
                 {
-                    void *pvBuf = RTMemAlloc(cbWrite);  /** @todo perhaps we can do this alloc once during VM init. */
-                    if (RT_LIKELY(pvBuf))
+                    if (PGMPhysIsGCPhysNormal(pVM, (RTGCPHYS)pHv->uDbgSendBufferMsr))
                     {
-                        if (PGMPhysIsGCPhysNormal(pVM, (RTGCPHYS)pHv->uDebugSendBufferMsr))
+                        Assert(pHv->pvDbgBuffer);
+                        int rc = PGMPhysSimpleReadGCPhys(pVM, pHv->pvDbgBuffer, (RTGCPHYS)pHv->uDbgSendBufferMsr, cbWrite);
+                        if (RT_SUCCESS(rc))
                         {
-                            int rc = PGMPhysSimpleReadGCPhys(pVM, pvBuf, (RTGCPHYS)pHv->uDebugSendBufferMsr, cbWrite);
-                            if (RT_SUCCESS(rc))
-                            {
-                                uint32_t cbWritten = 0;
-                                rc = gimR3HvDebugWrite(pVM, pvBuf, cbWrite, &cbWritten, false /*fUdpPkt*/);
-                                if (   RT_SUCCESS(rc)
-                                    && cbWrite == cbWritten)
-                                    pHv->uDebugStatusMsr = MSR_GIM_HV_SYNTH_DEBUG_STATUS_W_SUCCESS_BIT;
-                                else
-                                    pHv->uDebugStatusMsr = 0;
-                            }
+                            LogRelMax(1, ("GIM: HyperV: Initiated debug data transmission via MSR\n"));
+                            uint32_t cbWritten = 0;
+                            rc = gimR3HvDebugWrite(pVM, pHv->pvDbgBuffer, cbWrite, &cbWritten, false /*fUdpPkt*/);
+                            if (   RT_SUCCESS(rc)
+                                && cbWrite == cbWritten)
+                                pHv->uDbgStatusMsr = MSR_GIM_HV_SYNTH_DEBUG_STATUS_W_SUCCESS_BIT;
                             else
-                            {
-                                LogRelMax(5, ("GIM: HyperV: Failed to read debug send buffer at %#RGp, rc=%Rrc\n",
-                                              (RTGCPHYS)pHv->uDebugSendBufferMsr, rc));
-                            }
+                                pHv->uDbgStatusMsr = 0;
                         }
                         else
-                            LogRelMax(5, ("GIM: HyperV: Debug send buffer address %#RGp invalid! Ignoring debug write\n",
-                                          (RTGCPHYS)pHv->uDebugSendBufferMsr));
-                        RTMemFree(pvBuf);
+                            LogRelMax(5, ("GIM: HyperV: Failed to read debug send buffer at %#RGp, rc=%Rrc\n",
+                                          (RTGCPHYS)pHv->uDbgSendBufferMsr, rc));
                     }
                     else
+                        LogRelMax(5, ("GIM: HyperV: Debug send buffer address %#RGp invalid! Ignoring debug write!\n",
+                                      (RTGCPHYS)pHv->uDbgSendBufferMsr));
+                }
+                else
+                    LogRelMax(5, ("GIM: HyperV: Invalid write size %u specified in MSR, ignoring debug write!\n",
+                                  MSR_GIM_HV_SYNTH_DEBUG_CONTROL_W_LEN(uRawValue)));
+            }
+            else if (MSR_GIM_HV_SYNTH_DEBUG_CONTROL_IS_READ(uRawValue))
+            {
+                if (PGMPhysIsGCPhysNormal(pVM, (RTGCPHYS)pHv->uDbgRecvBufferMsr))
+                {
+                    LogRelMax(1, ("GIM: HyperV: Initiated debug data reception via MSR\n"));
+                    uint32_t cbReallyRead;
+                    Assert(pHv->pvDbgBuffer);
+                    int rc = gimR3HvDebugRead(pVM, pHv->pvDbgBuffer, PAGE_SIZE, PAGE_SIZE, &cbReallyRead, 0, false /*fUdpPkt*/);
+                    if (   RT_SUCCESS(rc)
+                        && cbReallyRead > 0)
                     {
-                        LogRel(("GIM: HyperV: Failed to alloc %u bytes for copying debug send buffer\n", cbWrite));
-                        return VERR_NO_MEMORY;
-                    }
-                }
-            }
-            else if (MSR_GIM_HV_SYNTH_DEBUG_CONTROL_IS_READ(uRawValue))
-            {
-                LogRelMax(1, ("GIM: HyperV: Initiated debug data reception via MSR\n"));
-                if (PGMPhysIsGCPhysNormal(pVM, (RTGCPHYS)pHv->uDebugRecvBufferMsr))
-                {
-                    void *pvBuf = RTMemAlloc(PAGE_SIZE);        /** @todo perhaps we can do this alloc once during VM init. */
-                    if (RT_LIKELY(pvBuf))
-                    {
-                        uint32_t cbReallyRead;
-                        int rc = gimR3HvDebugRead(pVM, pvBuf, PAGE_SIZE, PAGE_SIZE, &cbReallyRead, 0, false /*fUdpPkt*/);
-                        if (   RT_SUCCESS(rc)
-                            && cbReallyRead > 0)
+                        rc = PGMPhysSimpleWriteGCPhys(pVM, (RTGCPHYS)pHv->uDbgRecvBufferMsr, pHv->pvDbgBuffer, cbReallyRead);
+                        if (RT_SUCCESS(rc))
                         {
-                            rc = PGMPhysSimpleWriteGCPhys(pVM, (RTGCPHYS)pHv->uDebugRecvBufferMsr, pvBuf, cbReallyRead);
-                            if (RT_SUCCESS(rc))
-                            {
-                                pHv->uDebugStatusMsr  = ((uint16_t)cbReallyRead) << 16;
-                                pHv->uDebugStatusMsr |= MSR_GIM_HV_SYNTH_DEBUG_STATUS_R_SUCCESS_BIT;
-                            }
-                            else
-                            {
-                                pHv->uDebugStatusMsr = 0;
-                                LogRelMax(5, ("GIM: HyperV: PGMPhysSimpleWriteGCPhys failed. rc=%Rrc\n", rc));
-                            }
+                            pHv->uDbgStatusMsr  = ((uint16_t)cbReallyRead) << 16;
+                            pHv->uDbgStatusMsr |= MSR_GIM_HV_SYNTH_DEBUG_STATUS_R_SUCCESS_BIT;
                         }
                         else
-                            pHv->uDebugStatusMsr = 0;
-                        RTMemFree(pvBuf);
+                        {
+                            pHv->uDbgStatusMsr = 0;
+                            LogRelMax(5, ("GIM: HyperV: PGMPhysSimpleWriteGCPhys failed. rc=%Rrc\n", rc));
+                        }
                     }
                     else
-                    {
-                        LogRel(("GIM: HyperV: Failed to alloc %u bytes for copying debug receive buffer\n", PAGE_SIZE));
-                        return VERR_NO_MEMORY;
-                    }
+                        pHv->uDbgStatusMsr = 0;
                 }
                 else
-                    LogRelMax(5, ("GIM: HyperV: Debug receive buffer address %#RGp invalid! Ignoring debug data reception\n"));
+                    LogRelMax(5, ("GIM: HyperV: Debug receive buffer address %#RGp invalid! Ignoring debug read!\n"));
             }
             return VINF_SUCCESS;
@@ -870 +857 @@
         case MSR_GIM_HV_SINT2:
         {
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
 #ifndef IN_RING3
             return VINF_CPUM_R3_MSR_WRITE;
@@ -893 +882 @@
         case MSR_GIM_HV_SIMP:
         {
+            if (!pHv->fDbgEnabled)
+                return VERR_CPUM_RAISE_GP_0;
 #ifndef IN_RING3
             return VINF_CPUM_R3_MSR_WRITE;

trunk/src/VBox/VMM/VMMR3/GIM.cpp

@@ -59 +59 @@
 
 #include <iprt/err.h>
+#include <iprt/semaphore.h>
 #include <iprt/string.h>
 
@@ -167 +168 @@
             rc = VMR3SetError(pVM->pUVM, VERR_GIM_INVALID_PROVIDER, RT_SRC_POS, "Provider '%s' unknown.", szProvider);
     }
+
+    /*
+     * Statistics.
+     */
+    STAM_REL_REG_USED(pVM, &pVM->gim.s.StatDbgXmit,      STAMTYPE_COUNTER, "/GIM/Debug/Transmit",      STAMUNIT_OCCURENCES, "Debug packets sent.");
+    STAM_REL_REG_USED(pVM, &pVM->gim.s.StatDbgXmitBytes, STAMTYPE_COUNTER, "/GIM/Debug/TransmitBytes", STAMUNIT_OCCURENCES, "Debug bytes sent.");
+    STAM_REL_REG_USED(pVM, &pVM->gim.s.StatDbgRecv,      STAMTYPE_COUNTER, "/GIM/Debug/Receive",       STAMUNIT_OCCURENCES, "Debug packets received.");
+    STAM_REL_REG_USED(pVM, &pVM->gim.s.StatDbgRecvBytes, STAMTYPE_COUNTER, "/GIM/Debug/ReceiveBytes",  STAMUNIT_OCCURENCES, "Debug bytes received.");
+
+    STAM_REL_REG_USED(pVM, &pVM->gim.s.StatHypercalls,   STAMTYPE_COUNTER, "/GIM/Hypercalls",          STAMUNIT_OCCURENCES, "Number of hypercalls performed.");
     return rc;
 }
@@ -415 +426 @@
  * @param   pVM             The cross context VM structure.
  * @param   pDevIns         Pointer to the GIM device instance.
- * @param   pDebugStream    Pointer to the GIM device debug connection, can be
+ * @param   pDbg            Pointer to the GIM device debug structure, can be
  *                          NULL.
  */
-VMMR3DECL(void) GIMR3GimDeviceRegister(PVM pVM, PPDMDEVINS pDevIns, PPDMISTREAM pDebugStream)
+VMMR3DECL(void) GIMR3GimDeviceRegister(PVM pVM, PPDMDEVINS pDevIns, PGIMDEBUG pDbg)
 {
     pVM->gim.s.pDevInsR3 = pDevIns;
-    pVM->gim.s.pDebugStreamR3 = pDebugStream;
+    pVM->gim.s.pDbgR3    = pDbg;
+}
+
+
+/**
+ * Gets debug setup specified by the provider.
+ *
+ * @returns VBox status code.
+ * @param   pVM             The cross context VM structure.
+ * @param   pDevIns         Where to store the debug setup details.
+ */
+VMMR3DECL(int) GIMR3GetDebugSetup(PVM pVM, PGIMDEBUGSETUP pDbgSetup)
+{
+    AssertReturn(pVM, VERR_INVALID_PARAMETER);
+    AssertReturn(pDbgSetup, VERR_INVALID_PARAMETER);
+
+    switch (pVM->gim.s.enmProviderId)
+    {
+        case GIMPROVIDERID_HYPERV:
+            return gimR3HvGetDebugSetup(pVM, pDbgSetup);
+        default:
+            break;
+    }
+    return VERR_GIM_NO_DEBUG_CONNECTION;
 }
 
@@ -430 +464 @@
  * @returns VBox status code.
  *
- * @param   pVM         The cross context VM structure.
- * @param   pvRead      The read buffer.
- * @param   pcbRead     The size of the read buffer as well as where to store
- *                      the number of bytes read.
+ * @param   pVM                 The cross context VM structure.
+ * @param   pvRead              The read buffer.
+ * @param   pcbRead             The size of the read buffer as well as where to store
+ *                              the number of bytes read.
+ * @param   pfnReadComplete     Callback when the buffer has been read and
+ *                              before signaling reading of the next buffer.
+ *                              Optional, can be NULL.
  * @thread  EMT.
  */
-VMMR3_INT_DECL(int) GIMR3DebugRead(PVM pVM, void *pvRead, size_t *pcbRead)
-{
-    PPDMISTREAM pDebugStream = pVM->gim.s.pDebugStreamR3;
-    if (pDebugStream)
-        return pDebugStream->pfnRead(pDebugStream, pvRead, pcbRead);
+VMMR3_INT_DECL(int) GIMR3DebugRead(PVM pVM, void *pvRead, size_t *pcbRead, PFNGIMDEBUGBUFREADCOMPLETED pfnReadComplete)
+{
+    PGIMDEBUG pDbg = pVM->gim.s.pDbgR3;
+    if (pDbg)
+    {
+        if (ASMAtomicReadBool(&pDbg->fDbgRecvBufRead) == true)
+        {
+            STAM_COUNTER_INC(&pVM->gim.s.StatDbgRecv);
+            STAM_COUNTER_ADD(&pVM->gim.s.StatDbgRecvBytes, pDbg->cbDbgRecvBufRead);
+
+            memcpy(pvRead, pDbg->pvDbgRecvBuf, pDbg->cbDbgRecvBufRead);
+            *pcbRead = pDbg->cbDbgRecvBufRead;
+            if (pfnReadComplete)
+                pfnReadComplete(pVM);
+            RTSemEventMultiSignal(pDbg->hDbgRecvThreadSem);
+            ASMAtomicWriteBool(&pDbg->fDbgRecvBufRead, false);
+            return VINF_SUCCESS;
+        }
+        else
+            *pcbRead = 0;
+        return VERR_NO_DATA;
+    }
     return VERR_GIM_NO_DEBUG_CONNECTION;
 }
@@ -458 +512 @@
 VMMR3_INT_DECL(int) GIMR3DebugWrite(PVM pVM, void *pvWrite, size_t *pcbWrite)
 {
-    PPDMISTREAM pDebugStream = pVM->gim.s.pDebugStreamR3;
-    if (pDebugStream)
-        return pDebugStream->pfnWrite(pDebugStream, pvWrite, pcbWrite);
+    PGIMDEBUG pDbg = pVM->gim.s.pDbgR3;
+    if (pDbg)
+    {
+        PPDMISTREAM pDbgStream = pDbg->pDbgDrvStream;
+        if (pDbgStream)
+        {
+            size_t cbWrite = *pcbWrite;
+            int rc = pDbgStream->pfnWrite(pDbgStream, pvWrite, pcbWrite);
+            if (   RT_SUCCESS(rc)
+                && *pcbWrite == cbWrite)
+            {
+                STAM_COUNTER_INC(&pVM->gim.s.StatDbgXmit);
+                STAM_COUNTER_ADD(&pVM->gim.s.StatDbgXmitBytes, *pcbWrite);
+            }
+            return rc;
+        }
+    }
     return VERR_GIM_NO_DEBUG_CONNECTION;
 }

trunk/src/VBox/VMM/VMMR3/GIMHv.cpp

@@ -37 +37 @@
 #include <VBox/vmm/pdmapi.h>
 #include <VBox/version.h>
+#ifdef DEBUG_ramshankar
+# include <iprt/udp.h>
+#endif
 
 
@@ -175 +178 @@
 static int    gimR3HvInitHypercallSupport(PVM pVM);
 static void   gimR3HvTermHypercallSupport(PVM pVM);
+static int    gimR3HvInitDebugSupport(PVM pVM);
+static void   gimR3HvTermDebugSupport(PVM pVM);
 
 
@@ -203 +208 @@
         rc = CFGMR3ValidateConfig(pCfgHv, "/HyperV/",
                                   "VendorID"
-                                  "|VSInterface",
+                                  "|VSInterface"
+                                  "|HypercallDebugInterface",
                                   "" /* pszValidNodes */, "GIM/HyperV" /* pszWho */, 0 /* uInstance */);
         if (RT_FAILURE(rc))
@@ -218 +224 @@
     if (!RTStrNCmp(szVendor, GIM_HV_VENDOR_MICROSOFT, sizeof(GIM_HV_VENDOR_MICROSOFT) - 1))
     {
-        LogRel(("GIM: HyperV: Warning! Posing as the Microsoft vendor, guest behavior may be altered!\n"));
+        LogRel(("GIM: HyperV: Warning! Posing as the Microsoft vendor may alter guest behaviour!\n"));
         pHv->fIsVendorMsHv = true;
     }
@@ -227 +233 @@
          * The Microsoft virtualization service interface (debugging). */
         rc = CFGMR3QueryBoolDef(pCfgHv, "VSInterface", &pHv->fIsInterfaceVs, true);
+        AssertLogRelRCReturn(rc, rc);
+
+        /** @cfgm{/GIM/HyperV/HypercallDebugInterface, bool, true}
+         * Whether we specify the guest to use hypercalls for debugging rather than MSRs. */
+        rc = CFGMR3QueryBoolDef(pCfgHv, "HypercallDebugInterface", &pHv->fDbgHypercallInterface, true);
         AssertLogRelRCReturn(rc, rc);
     }
@@ -438 +449 @@
     AssertLogRelRCReturn(rc, rc);
 
+    /*
+     * Setup debug support.
+     */
+    rc = gimR3HvInitDebugSupport(pVM);
+    AssertLogRelRCReturn(rc, rc);
+
     return VINF_SUCCESS;
 }
@@ -559 +576 @@
      * Reset MSRs.
      */
-    pHv->u64GuestOsIdMsr        = 0;
-    pHv->u64HypercallMsr        = 0;
-    pHv->u64TscPageMsr          = 0;
-    pHv->uCrashP0Msr            = 0;
-    pHv->uCrashP1Msr            = 0;
-    pHv->uCrashP2Msr            = 0;
-    pHv->uCrashP3Msr            = 0;
-    pHv->uCrashP4Msr            = 0;
-    pHv->uDebugStatusMsr        = 0;
-    pHv->uDebugPendingBufferMsr = 0;
-    pHv->uDebugSendBufferMsr    = 0;
-    pHv->uDebugRecvBufferMsr    = 0;
+    pHv->u64GuestOsIdMsr      = 0;
+    pHv->u64HypercallMsr      = 0;
+    pHv->u64TscPageMsr        = 0;
+    pHv->uCrashP0Msr          = 0;
+    pHv->uCrashP1Msr          = 0;
+    pHv->uCrashP2Msr          = 0;
+    pHv->uCrashP3Msr          = 0;
+    pHv->uCrashP4Msr          = 0;
+    pHv->uDbgStatusMsr        = 0;
+    pHv->uDbgPendingBufferMsr = 0;
+    pHv->uDbgSendBufferMsr    = 0;
+    pHv->uDbgRecvBufferMsr    = 0;
     for (VMCPUID i = 0; i < pVM->cCpus; i++)
     {
@@ -595 +612 @@
     Assert(*pcRegions <= UINT8_MAX);    /* See PGMR3PhysMMIO2Register(). */
     return pHv->aMmio2Regions;
+}
+
+
+/**
+ * Callback for when debug data is available over the debugger connection.
+ *
+ * @param pVM       The cross context VM structure.
+ */
+static DECLCALLBACK(void) gimR3HvDebugBufAvail(PVM pVM)
+{
+    PGIMHV   pHv = &pVM->gim.s.u.Hv;
+    RTGCPHYS GCPhysPendingBuffer = pHv->uDbgPendingBufferMsr;
+    if (   GCPhysPendingBuffer
+        && PGMPhysIsGCPhysNormal(pVM, GCPhysPendingBuffer))
+    {
+        uint8_t bPendingData = 1;
+        int rc = PGMPhysSimpleWriteGCPhys(pVM, GCPhysPendingBuffer, &bPendingData, sizeof(bPendingData));
+        if (RT_FAILURE(rc))
+        {
+            LogRelMax(5, ("GIM: HyperV: Failed to set pending debug receive buffer at %#RGp, rc=%Rrc\n", GCPhysPendingBuffer,
+                          rc));
+        }
+    }
+}
+
+
+/**
+ * Callback for when debug data has been read from the debugger connection.
+ *
+ * This will be invoked before signalling read of the next debug buffer.
+ *
+ * @param pVM       The cross context VM structure.
+ */
+static DECLCALLBACK(void) gimR3HvDebugBufReadCompleted(PVM pVM)
+{
+    PGIMHV   pHv = &pVM->gim.s.u.Hv;
+    RTGCPHYS GCPhysPendingBuffer = pHv->uDbgPendingBufferMsr;
+    if (   GCPhysPendingBuffer
+        && PGMPhysIsGCPhysNormal(pVM, GCPhysPendingBuffer))
+    {
+        uint8_t bPendingData = 0;
+        int rc = PGMPhysSimpleWriteGCPhys(pVM, GCPhysPendingBuffer, &bPendingData, sizeof(bPendingData));
+        if (RT_FAILURE(rc))
+        {
+            LogRelMax(5, ("GIM: HyperV: Failed to clear pending debug receive buffer at %#RGp, rc=%Rrc\n", GCPhysPendingBuffer,
+                          rc));
+        }
+    }
+}
+
+
+/**
+ * Get Hyper-V debug setup parameters.
+ *
+ * @returns VBox status code.
+ * @param   pVM         The cross context VM structure.
+ * @param   pDbgSetup   Where to store the debug setup details.
+ */
+VMMR3_INT_DECL(int) gimR3HvGetDebugSetup(PVM pVM, PGIMDEBUGSETUP pDbgSetup)
+{
+    Assert(pDbgSetup);
+    PGIMHV pHv = &pVM->gim.s.u.Hv;
+    if (pHv->fDbgEnabled)
+    {
+        pDbgSetup->pfnDbgRecvBufAvail = gimR3HvDebugBufAvail;
+        pDbgSetup->cbDbgRecvBuf       = GIM_HV_PAGE_SIZE;
+        return VINF_SUCCESS;
+    }
+    return VERR_GIM_NO_DEBUG_CONNECTION;
 }
 
@@ -663 +749 @@
      * Save debug support data.
      */
-    SSMR3PutU64(pSSM, pcHv->uDebugPendingBufferMsr);
-    SSMR3PutU64(pSSM, pcHv->uDebugSendBufferMsr);
-    SSMR3PutU64(pSSM, pcHv->uDebugRecvBufferMsr);
-    SSMR3PutU64(pSSM, pcHv->uDebugStatusMsr);
-    SSMR3PutU32(pSSM, pcHv->enmDebugReply);
-    SSMR3PutU32(pSSM, pcHv->uBootpXId);
+    SSMR3PutU64(pSSM, pcHv->uDbgPendingBufferMsr);
+    SSMR3PutU64(pSSM, pcHv->uDbgSendBufferMsr);
+    SSMR3PutU64(pSSM, pcHv->uDbgRecvBufferMsr);
+    SSMR3PutU64(pSSM, pcHv->uDbgStatusMsr);
+    SSMR3PutU32(pSSM, pcHv->enmDbgReply);
+    SSMR3PutU32(pSSM, pcHv->uDbgBootpXId);
     SSMR3PutU32(pSSM, pcHv->DbgGuestIp4Addr.u);
 
@@ -792 +878 @@
     if (uHvSavedStatVersion > GIM_HV_SAVED_STATE_VERSION_PRE_DEBUG)
     {
-        SSMR3GetU64(pSSM, &pHv->uDebugPendingBufferMsr);
-        SSMR3GetU64(pSSM, &pHv->uDebugSendBufferMsr);
-        SSMR3GetU64(pSSM, &pHv->uDebugRecvBufferMsr);
-        SSMR3GetU64(pSSM, &pHv->uDebugStatusMsr);
-        SSMR3GetU32(pSSM, (uint32_t *)&pHv->enmDebugReply);
-        SSMR3GetU32(pSSM, &pHv->uBootpXId);
+        SSMR3GetU64(pSSM, &pHv->uDbgPendingBufferMsr);
+        SSMR3GetU64(pSSM, &pHv->uDbgSendBufferMsr);
+        SSMR3GetU64(pSSM, &pHv->uDbgRecvBufferMsr);
+        SSMR3GetU64(pSSM, &pHv->uDbgStatusMsr);
+        SSMR3GetU32(pSSM, (uint32_t *)&pHv->enmDbgReply);
+        SSMR3GetU32(pSSM, &pHv->uDbgBootpXId);
         rc = SSMR3GetU32(pSSM, &pHv->DbgGuestIp4Addr.u);
         AssertRCReturn(rc, rc);
@@ -1137 +1223 @@
 
 /**
+ * Initializes Hyper-V guest debug support.
+ *
+ * @returns VBox status code.
+ * @param   pVM     The cross context VM structure.
+ */
+static int gimR3HvInitDebugSupport(PVM pVM)
+{
+    PGIMHV pHv = &pVM->gim.s.u.Hv;
+    if (   (pHv->uPartFlags & GIM_HV_PART_FLAGS_DEBUGGING)
+        || pHv->fIsInterfaceVs)
+    {
+        pHv->fDbgEnabled = true;
+        pHv->pvDbgBuffer = RTMemAllocZ(PAGE_SIZE);
+        if (!pHv->pvDbgBuffer)
+            return VERR_NO_MEMORY;
+    }
+    return VINF_SUCCESS;
+}
+
+
+/**
+ * Terminates Hyper-V guest debug support.
+ *
+ * @param   pVM     The cross context VM structure.
+ */
+static void gimR3HvTermDebugSupport(PVM pVM)
+{
+    PGIMHV pHv = &pVM->gim.s.u.Hv;
+    if (pHv->pvDbgBuffer)
+    {
+        RTMemFree(pHv->pvDbgBuffer);
+        pHv->pvDbgBuffer = NULL;
+    }
+}
+
+
+/**
  * Reads data from a debugger connection, asynchronous.
  *
@@ -1165 +1288 @@
          */
         size_t cbReallyRead = cbRead;
-        rc = GIMR3DebugRead(pVM, pvBuf, &cbReallyRead);
+        rc = GIMR3DebugRead(pVM, pvBuf, &cbReallyRead, gimR3HvDebugBufReadCompleted);
         *pcbRead = (uint32_t)cbReallyRead;
     }
@@ -1175 +1298 @@
         PGIMHV pHv = &pVM->gim.s.u.Hv;
         rc = VERR_GIM_IPE_1;
-        switch (pHv->enmDebugReply)
+        switch (pHv->enmDbgReply)
         {
             case GIMHVDEBUGREPLY_UDP:
             {
                 size_t cbReallyRead = cbRead;
-                rc = GIMR3DebugRead(pVM, pvBuf, &cbReallyRead);
+                rc = GIMR3DebugRead(pVM, pvBuf, &cbReallyRead, gimR3HvDebugBufReadCompleted);
                 if (   RT_SUCCESS(rc)
                     && cbReallyRead > 0)
@@ -1217 +1340 @@
                         pIpHdr->ip_sum     = RTNetIPv4HdrChecksum(pIpHdr);
                         /* UDP */
+                        pUdpHdr->uh_dport  = pHv->uUdpGuestSrcPort;
+                        pUdpHdr->uh_sport  = pHv->uUdpGuestDstPort;
                         pUdpHdr->uh_ulen   = RT_H2N_U16_C((uint16_t)cbReallyRead + sizeof(*pUdpHdr));
 
@@ -1242 +1367 @@
                     rc = VINF_SUCCESS;
                     *pcbRead = cbArpReplyPkt;
-                    pHv->enmDebugReply = GIMHVDEBUGREPLY_ARP_REPLY_SENT;
+                    pHv->enmDbgReply = GIMHVDEBUGREPLY_ARP_REPLY_SENT;
                 }
                 else
@@ -1262 +1387 @@
                     PRTNETUDP      pUdpHdr   = (PRTNETUDP)     ((uint8_t *)pIpHdr + RTNETIPV4_MIN_LEN);
                     PRTNETBOOTP    pBootpHdr = (PRTNETBOOTP)   (pUdpHdr + 1);
-                    pBootpHdr->bp_xid = pHv->uBootpXId;
+                    pBootpHdr->bp_xid = pHv->uDbgBootpXId;
 
                     rc = VINF_SUCCESS;
                     *pcbRead = cbDhcpOfferPkt;
-                    pHv->enmDebugReply = GIMHVDEBUGREPLY_DHCP_OFFER_SENT;
+                    pHv->enmDbgReply = GIMHVDEBUGREPLY_DHCP_OFFER_SENT;
                     LogRel(("GIM: HyperV: Debug DHCP offered IP address %RTnaipv4, transaction Id %#x\n", pBootpHdr->bp_yiaddr,
-                            RT_N2H_U32(pHv->uBootpXId)));
+                            RT_N2H_U32(pHv->uDbgBootpXId)));
                 }
                 else
@@ -1288 +1413 @@
                     PRTNETUDP      pUdpHdr   = (PRTNETUDP)     ((uint8_t *)pIpHdr + RTNETIPV4_MIN_LEN);
                     PRTNETBOOTP    pBootpHdr = (PRTNETBOOTP)   (pUdpHdr + 1);
-                    pBootpHdr->bp_xid = pHv->uBootpXId;
+                    pBootpHdr->bp_xid = pHv->uDbgBootpXId;
 
                     rc = VINF_SUCCESS;
                     *pcbRead = cbDhcpAckPkt;
-                    pHv->enmDebugReply = GIMHVDEBUGREPLY_DHCP_ACK_SENT;
+                    pHv->enmDbgReply = GIMHVDEBUGREPLY_DHCP_ACK_SENT;
                     LogRel(("GIM: HyperV: Debug DHCP acknowledged IP address %RTnaipv4, transaction Id %#x\n",
-                            pBootpHdr->bp_yiaddr, RT_N2H_U32(pHv->uBootpXId)));
+                            pBootpHdr->bp_yiaddr, RT_N2H_U32(pHv->uDbgBootpXId)));
                 }
                 else
@@ -1315 +1440 @@
             default:
             {
-                AssertMsgFailed(("GIM: HyperV: Invalid/unimplemented debug reply type %u\n", pHv->enmDebugReply));
+                AssertMsgFailed(("GIM: HyperV: Invalid/unimplemented debug reply type %u\n", pHv->enmDbgReply));
                 rc = VERR_INTERNAL_ERROR_2;
             }
         }
         Assert(rc != VERR_GIM_IPE_1);
-    }
+
+#ifdef DEBUG_ramshankar
+        if (   rc == VINF_SUCCESS
+            && *pcbRead > 0)
+        {
+            RTSOCKET hSocket;
+            int rc2 = RTUdpCreateClientSocket("localhost", 52000, NULL, &hSocket);
+            if (RT_SUCCESS(rc2))
+            {
+                size_t cbTmpWrite = *pcbRead;
+                RTSocketWriteNB(hSocket, pvBuf, *pcbRead, &cbTmpWrite); NOREF(cbTmpWrite);
+                RTSocketClose(hSocket);
+            }
+        }
+#endif
+    }
+
     return rc;
 }
@@ -1347 +1488 @@
     if (fUdpPkt)
     {
+#ifdef DEBUG_ramshankar
+        RTSOCKET hSocket;
+        int rc2 = RTUdpCreateClientSocket("localhost", 52000, NULL, &hSocket);
+        if (RT_SUCCESS(rc2))
+        {
+            size_t cbTmpWrite = cbWrite;
+            RTSocketWriteNB(hSocket, pbData, cbWrite, &cbTmpWrite);  NOREF(cbTmpWrite);
+            RTSocketClose(hSocket);
+        }
+#endif
         /*
          * Windows guests sends us ethernet frames over the Hyper-V debug connection.
@@ -1388 +1539 @@
                              * Check for DHCP.
                              */
+                            bool fBuggyPkt = false;
                             size_t const cbUdpPkt = cbMaxIpPkt - cbIpHdr;
                             if (   pUdpHdr->uh_dport == RT_N2H_U16_C(RTNETIPV4_PORT_BOOTPS)
-                                && pUdpHdr->uh_sport == RT_N2H_U16_C(RTNETIPV4_PORT_BOOTPC)
-                                && cbMaxIpPkt >= cbIpHdr + RTNETUDP_MIN_LEN + RTNETBOOTP_DHCP_MIN_LEN)
+                                && pUdpHdr->uh_sport == RT_N2H_U16_C(RTNETIPV4_PORT_BOOTPC))
                             {
                                 PCRTNETBOOTP pDhcpPkt = (PCRTNETBOOTP)(pUdpHdr + 1);
                                 uint8_t bMsgType;
-                                if (RTNetIPv4IsDHCPValid(pUdpHdr, pDhcpPkt, cbUdpPkt - sizeof(*pUdpHdr), &bMsgType))
+                                if (   cbMaxIpPkt >= cbIpHdr + RTNETUDP_MIN_LEN + RTNETBOOTP_DHCP_MIN_LEN
+                                    && RTNetIPv4IsDHCPValid(pUdpHdr, pDhcpPkt, cbUdpPkt - sizeof(*pUdpHdr), &bMsgType))
                                 {
                                     switch (bMsgType)
                                     {
                                         case RTNET_DHCP_MT_DISCOVER:
-                                            pHv->enmDebugReply = GIMHVDEBUGREPLY_DHCP_OFFER;
-                                            pHv->uBootpXId = pDhcpPkt->bp_xid;
+                                            pHv->enmDbgReply = GIMHVDEBUGREPLY_DHCP_OFFER;
+                                            pHv->uDbgBootpXId = pDhcpPkt->bp_xid;
                                             break;
                                         case RTNET_DHCP_MT_REQUEST:
-                                            pHv->enmDebugReply = GIMHVDEBUGREPLY_DHCP_ACK;
-                                            pHv->uBootpXId = pDhcpPkt->bp_xid;
+                                            pHv->enmDbgReply = GIMHVDEBUGREPLY_DHCP_ACK;
+                                            pHv->uDbgBootpXId = pDhcpPkt->bp_xid;
                                             break;
                                         default:
@@ -1412 +1564 @@
                                             break;
                                     }
+                                    fIgnorePkt = true;
                                 }
-                                fIgnorePkt = true;
+                                else if (   pIp4Hdr->ip_src.u == GIMHV_DEBUGCLIENT_IPV4
+                                         && pIp4Hdr->ip_dst.u == 0)
+                                {
+                                    /*
+                                     * Windows 8.1 seems to be sending malformed BOOTP packets at the final stage of the
+                                     * debugger sequence. It appears that a previously sent DHCP request buffer wasn't cleared
+                                     * in the guest and they re-use it instead of sending a zero destination+source port packet
+                                     * as expected below.
+                                     *
+                                     * We workaround Microsoft's bug here, or at least, I'm classifying it as a bug to
+                                     * preserve my own sanity, see @bugref{8006#c54}.
+                                     */
+                                    fBuggyPkt = true;
+                                }
                             }
-                            else if (   !pUdpHdr->uh_dport
-                                     && !pUdpHdr->uh_sport)
+
+                            if (  (   !pUdpHdr->uh_dport
+                                   && !pUdpHdr->uh_sport)
+                                || fBuggyPkt)
                             {
                                 /*
                                  * Extract the UDP payload and pass it to the debugger and record the guest IP address.
-                                 * Hyper-V sends UDP debugger packets with source and destination port as 0. If we don't
-                                 * filter out the ports here, we would receive BOOTP, NETBIOS and other UDP sub-protocol
-                                 * packets which the debugger yells as "Bad packet received from...".
+                                 *
+                                 * Hyper-V sends UDP debugger packets with source and destination port as 0 except in the
+                                 * aformentioned buggy case. The buggy packet case requires us to remember the ports and
+                                 * reply to them, otherwise the guest won't receive the replies we sent with port 0.
                                  */
                                 uint32_t const cbFrameHdr = sizeof(RTNETETHERHDR) + cbIpHdr + sizeof(RTNETUDP);
                                 pbData  += cbFrameHdr;
                                 cbWrite -= cbFrameHdr;
-                                pHv->DbgGuestIp4Addr = pIp4Hdr->ip_src;
-                                pHv->enmDebugReply   = GIMHVDEBUGREPLY_UDP;
+                                pHv->DbgGuestIp4Addr.u = pIp4Hdr->ip_src.u;
+                                pHv->uUdpGuestDstPort  = pUdpHdr->uh_dport;
+                                pHv->uUdpGuestSrcPort  = pUdpHdr->uh_sport;
+                                pHv->enmDbgReply       = GIMHVDEBUGREPLY_UDP;
                             }
                             else
                             {
-                                LogFlow(("GIM: HyperV: Ignoring UDP packet not src and dst port 0\n"));
+                                LogFlow(("GIM: HyperV: Ignoring UDP packet SourcePort=%u DstPort=%u\n", pUdpHdr->uh_sport,
+                                         pUdpHdr->uh_dport));
                                 fIgnorePkt = true;
                             }
@@ -1476 +1648 @@
                             &&  pArpPkt->ar_tpa.u == GIMHV_DEBUGSERVER_IPV4)
                         {
-                            pHv->enmDebugReply = GIMHVDEBUGREPLY_ARP_REPLY;
+                            pHv->enmDbgReply = GIMHVDEBUGREPLY_ARP_REPLY;
                         }
                     }

trunk/src/VBox/VMM/include/GIMHvInternal.h

@@ -713 +713 @@
 /** The undocumented bit for MSR_GIM_HV_DEBUG_OPTIONS_MSR that makes it all
  *  work. */
-#define GIM_HV_DEBUG_OPTIONS_MSR_ENABLE           RT_BIT(2)
+#define GIM_HV_DEBUG_OPTIONS_USE_HYPERCALLS       RT_BIT(2)
 
 /** Guest will perform the HvPostDebugData hypercall until completion. */
@@ -1047 +1047 @@
     /** Whether we're posing as the Microsoft virtualization service. */
     bool                        fIsInterfaceVs;
-    bool                        afAlignment0[6];
-    /** The auto IP address last chosen by the guest after failed ARP queries. */
+    /** Whether debugging support is enabled. */
+    bool                        fDbgEnabled;
+    /** Whether we should suggest a hypercall-based debug interface to the guest. */
+    bool                        fDbgHypercallInterface;
+    bool                        afAlignment0[4];
+    /** The action to take while sending replies. */
+    GIMHVDEBUGREPLY             enmDbgReply;
+    /** The IP address chosen by/assigned to the guest. */
     RTNETADDRIPV4               DbgGuestIp4Addr;
-    /** The action to take while sending replies. */
-    GIMHVDEBUGREPLY             enmDebugReply;
     /** Transaction ID for the BOOTP+DHCP sequence. */
-    uint32_t                    uBootpXId;
-    /** Padding. */
-    uint32_t                    uAlignment0;
+    uint32_t                    uDbgBootpXId;
+    /** The source UDP port used by the guest while sending debug packets. */
+    uint16_t                    uUdpGuestSrcPort;
+    /** The destination UDP port used by the guest while sending debug packets. */
+    uint16_t                    uUdpGuestDstPort;
     /** Debug send buffer MSR. */
-    uint64_t                    uDebugSendBufferMsr;
+    uint64_t                    uDbgSendBufferMsr;
     /** Debug receive buffer MSR. */
-    uint64_t                    uDebugRecvBufferMsr;
+    uint64_t                    uDbgRecvBufferMsr;
     /** Debug pending buffer MSR. */
-    uint64_t                    uDebugPendingBufferMsr;
+    uint64_t                    uDbgPendingBufferMsr;
     /** Debug status MSR. */
-    uint64_t                    uDebugStatusMsr;
+    uint64_t                    uDbgStatusMsr;
+    /** Intermediate debug I/O buffer. */
+    R3PTRTYPE(void *)           pvDbgBuffer;
     /** @} */
 
@@ -1113 +1121 @@
 VMMR3_INT_DECL(int)             gimR3HvSave(PVM pVM, PSSMHANDLE pSSM);
 VMMR3_INT_DECL(int)             gimR3HvLoad(PVM pVM, PSSMHANDLE pSSM, uint32_t uSSMVersion);
+VMMR3_INT_DECL(int)             gimR3HvGetDebugSetup(PVM pVM, PGIMDEBUGSETUP pDbgSetup);
 
 VMMR3_INT_DECL(int)             gimR3HvDisableTscPage(PVM pVM);

trunk/src/VBox/VMM/include/GIMInternal.h

@@ -54 +54 @@
     /** Pointer to the GIM device - R3 ptr. */
     R3PTRTYPE(PPDMDEVINS)            pDevInsR3;
-    /** Pointer to the GIM device debug stream - R3 ptr. */
-    R3PTRTYPE(PPDMISTREAM)           pDebugStreamR3;
-#if 0
-    /** Pointer to the provider's ring-3 hypercall handler. */
-    R3PTRTYPE(PFNGIMHYPERCALL)       pfnHypercallR3;
-    /** Pointer to the provider's ring-0 hypercall handler. */
-    R0PTRTYPE(PFNGIMHYPERCALL)       pfnHypercallR0;
-    /** Pointer to the provider's raw-mode context hypercall handler. */
-    RCPTRTYPE(PFNGIMHYPERCALL)       pfnHypercallRC;
+    /** The debug struct - R3 ptr. */
+    R3PTRTYPE(PGIMDEBUG)             pDbgR3;
 
-    /** Pointer to the provider's ring-3 MSR-read handler. */
-    R3PTRTYPE(PFNGIMRDMSR)           pfnReadMsrR3;
-    /** Pointer to the provider's ring-0 MSR-read handler. */
-    R0PTRTYPE(PFNGIMRDMSR)           pfnReadMsrR0;
-    /** Pointer to the provider's raw-mode context MSR-read handler. */
-    RCPTRTYPE(PFNGIMRDMSR)           pfnReadMsrRC;
-
-    /** Pointer to the provider's ring-3 MSR-read handler. */
-    R3PTRTYPE(PFNGIMWDMSR)           pfnWriteMsrR3;
-    /** Pointer to the provider's ring-0 MSR-read handler. */
-    R0PTRTYPE(PFNGIMWDMSR)           pfnWriteMsrRR0;
-    /** Pointer to the provider's raw-mode context MSR-read handler. */
-    RCPTRTYPE(PFNGIMWDMSR)           pfnWriteMsrRRC;
-#endif
-
+    /** The provider specific data. */
     union
     {
@@ -84 +63 @@
         GIMKVM Kvm;
     } u;
+
+    /** Number of hypercalls initiated. */
+    STAMCOUNTER                      StatHypercalls;
+    /** Debug packets sent. */
+    STAMCOUNTER                      StatDbgXmit;
+    /** Debug bytes sent. */
+    STAMCOUNTER                      StatDbgXmitBytes;
+    /** Debug packets received. */
+    STAMCOUNTER                      StatDbgRecv;
+    /** Debug bytes received. */
+    STAMCOUNTER                      StatDbgRecvBytes;
 } GIM;
 /** Pointer to GIM VM instance data. */
 typedef GIM *PGIM;
-
 
 /**
@@ -103 +92 @@
 typedef GIMCPU *PGIMCPU;
 
+/**
+ * Callback when a debug buffer read has completed and before signaling
+ * the next read.
+ *
+ * @param   pVM             The cross context VM structure.
+ */
+typedef DECLCALLBACK(void) FNGIMDEBUGBUFREADCOMPLETED(PVM pVM);
+/** Pointer to GIM debug buffer read completion callback. */
+typedef FNGIMDEBUGBUFREADCOMPLETED *PFNGIMDEBUGBUFREADCOMPLETED;
+
 #ifdef IN_RING3
 VMMR3_INT_DECL(int)           GIMR3Mmio2Unmap(PVM pVM, PGIMMMIO2REGION pRegion);
@@ -109 +108 @@
 VMMR3_INT_DECL(int)           GIMR3Mmio2HandlerPhysicalDeregister(PVM pVM, PGIMMMIO2REGION pRegion);
 
-VMMR3_INT_DECL(int)           GIMR3DebugRead(PVM pVM, void *pvRead, size_t *pcbRead);
+VMMR3_INT_DECL(int)           GIMR3DebugRead(PVM pVM, void *pvRead, size_t *pcbRead, PFNGIMDEBUGBUFREADCOMPLETED pfnReadComplete);
 VMMR3_INT_DECL(int)           GIMR3DebugWrite(PVM pVM, void *pvWrite, size_t *pcbWrite);
 #endif /* IN_RING3 */