Changeset 61973 in vbox for trunk

Timestamp:

Jul 1, 2016 11:02:10 AM (9 years ago)

Author:

vboxsync

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

108420

Message:

DevVGA: bugref:8447: update and clear HGSMI IRQ flag under lock to avoid a race

Location:

trunk/src/VBox/Devices/Graphics

Files:

• DevVGA.cpp (modified) (3 diffs)
• DevVGA.h (modified) (2 diffs)
• DevVGA_VBVA.cpp (modified) (4 diffs)
• DevVGA_VDMA.cpp (modified) (1 diff)

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

@@ -3053 +3053 @@
                 if (u32 == HGSMIOFFSET_VOID)
                 {
-                    PDMDevHlpPCISetIrq(pDevIns, 0, PDM_IRQ_LEVEL_LOW);
-                    HGSMIClearHostGuestFlags(pThis->pHGSMI,
-                                             HGSMIHOSTFLAGS_IRQ
+                    PDMCritSectEnter(&pThis->critSectIRQ, VERR_SEM_BUSY);
+
+                    if (pThis->fu32PendingGuestFlags == 0)
+                    {
+                        PDMDevHlpPCISetIrqNoWait(pDevIns, 0, PDM_IRQ_LEVEL_LOW);
+                        HGSMIClearHostGuestFlags(pThis->pHGSMI,
+                                                 HGSMIHOSTFLAGS_IRQ
 #  ifdef VBOX_VDMA_WITH_WATCHDOG
-                                             | HGSMIHOSTFLAGS_WATCHDOG
+                                                 | HGSMIHOSTFLAGS_WATCHDOG
 #  endif
-                                             | HGSMIHOSTFLAGS_VSYNC
-                                             | HGSMIHOSTFLAGS_HOTPLUG
-                                             | HGSMIHOSTFLAGS_CURSOR_CAPABILITIES
-                                             );
+                                                 | HGSMIHOSTFLAGS_VSYNC
+                                                 | HGSMIHOSTFLAGS_HOTPLUG
+                                                 | HGSMIHOSTFLAGS_CURSOR_CAPABILITIES
+                                                );
+                    }
+                    else
+                    {
+                        HGSMISetHostGuestFlags(pThis->pHGSMI, HGSMIHOSTFLAGS_IRQ | pThis->fu32PendingGuestFlags);
+                        pThis->fu32PendingGuestFlags = 0;
+                        /* Keep the IRQ unchanged. */
+                    }
+
+                    PDMCritSectLeave(&pThis->critSectIRQ);
                 }
                 else
@@ -5941 +5954 @@
     }
 
+    PDMR3CritSectDelete(&pThis->critSectIRQ);
     PDMR3CritSectDelete(&pThis->CritSect);
     return VINF_SUCCESS;
@@ -6170 +6184 @@
     AssertRCReturn(rc, rc);
     rc = PDMDevHlpSetDeviceCritSect(pDevIns, &pThis->CritSect);
+    AssertRCReturn(rc, rc);
+
+    rc = PDMDevHlpCritSectInit(pDevIns, &pThis->critSectIRQ, RT_SRC_POS, "VGA#%u_IRQ", iInstance);
     AssertRCReturn(rc, rc);
 

trunk/src/VBox/Devices/Graphics/DevVGA.h

@@ -602 +602 @@
     uint8_t                     Padding10[14];
 #  endif
+
+    /** The critical section serializes the HGSMI IRQ setting/clearing. */
+    PDMCRITSECT                 critSectIRQ;
+    /** VBVARaiseIRQ flags which were set when the guest was still processing previous IRQ. */
+    uint32_t                    fu32PendingGuestFlags;
+    uint32_t                    Padding11;
 # endif /* VBOX_WITH_HGSMI */
 
@@ -689 +695 @@
 
 void VBVARaiseIrq (PVGASTATE pVGAState, uint32_t fFlags);
-void VBVARaiseIrqNoWait(PVGASTATE pVGAState, uint32_t fFlags);
 
 int VBVAInfoView(PVGASTATE pVGAState, const VBVAINFOVIEW *pView);

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

@@ -2082 +2082 @@
     PPDMDEVINS pDevIns = pVGAState->pDevInsR3;
 
-    PDMCritSectEnter(&pVGAState->CritSect, VERR_SEM_BUSY);
-    HGSMISetHostGuestFlags(pVGAState->pHGSMI, HGSMIHOSTFLAGS_IRQ | fFlags);
-    PDMCritSectLeave(&pVGAState->CritSect);
-
-    /* Set IRQ only for a running VM.
-     * If HGSMIHOSTFLAGS_IRQ is set, then vgaR3Resume/vgaR3PowerOn will
-     * set the postponed IRQ.
-     */
-    VMSTATE enmVMState = PDMDevHlpVMState(pDevIns);
-    if (   enmVMState == VMSTATE_RUNNING
-        || enmVMState == VMSTATE_RUNNING_LS)
-    {
-        PDMDevHlpPCISetIrq(pDevIns, 0, PDM_IRQ_LEVEL_HIGH);
-    }
-}
-
-static DECLCALLBACK(int) vbvaRaiseIrqEMT(PVGASTATE pVGAState, uint32_t fFlags)
-{
-    VBVARaiseIrq(pVGAState, fFlags);
-    return VINF_SUCCESS;
-}
-
-void VBVARaiseIrqNoWait(PVGASTATE pVGAState, uint32_t fFlags)
-{
-    /* we can not use PDMDevHlpPCISetIrqNoWait here, because we need to set IRG host flag and raise IRQ atomically,
-     * otherwise there might be a situation, when:
-     * 1. Flag is set
-     * 2. guest issues an IRQ clean request, that cleans up the flag and the interrupt
-     * 3. IRQ is set */
-    VMR3ReqCallNoWait(PDMDevHlpGetVM(pVGAState->pDevInsR3), VMCPUID_ANY, (PFNRT)vbvaRaiseIrqEMT, 2, pVGAState, fFlags);
+    PDMCritSectEnter(&pVGAState->critSectIRQ, VERR_SEM_BUSY);
+
+    const uint32_t fu32CurrentGuestFlags = HGSMIGetHostGuestFlags(pVGAState->pHGSMI);
+    if ((fu32CurrentGuestFlags & HGSMIHOSTFLAGS_IRQ) == 0)
+    {
+        /* No IRQ set yet. */
+        Assert(pVGAState->fu32PendingGuestFlags == 0);
+
+        HGSMISetHostGuestFlags(pVGAState->pHGSMI, HGSMIHOSTFLAGS_IRQ | fFlags);
+
+        /* If VM is not running, the IRQ will be set in VBVAOnResume. */
+        const VMSTATE enmVMState = PDMDevHlpVMState(pDevIns);
+        if (   enmVMState == VMSTATE_RUNNING
+            || enmVMState == VMSTATE_RUNNING_LS)
+            PDMDevHlpPCISetIrqNoWait(pDevIns, 0, PDM_IRQ_LEVEL_HIGH);
+    }
+    else
+    {
+        /* IRQ already set, remember the new flags. */
+        pVGAState->fu32PendingGuestFlags |= HGSMIHOSTFLAGS_IRQ | fFlags;
+    }
+
+    PDMCritSectLeave(&pVGAState->critSectIRQ);
 }
 
@@ -2118 +2111 @@
     PPDMDEVINS pDevIns = pThis->pDevInsR3;
 
-    PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
-    bool fIrq = RT_BOOL(HGSMIGetHostGuestFlags(pThis->pHGSMI) & HGSMIHOSTFLAGS_IRQ);
-    PDMCritSectLeave(&pThis->CritSect);
-
-    if (fIrq)
-        PDMDevHlpPCISetIrq(pDevIns, 0, PDM_IRQ_LEVEL_HIGH);
+    PDMCritSectEnter(&pThis->critSectIRQ, VERR_SEM_BUSY);
+
+    if (HGSMIGetHostGuestFlags(pThis->pHGSMI) & HGSMIHOSTFLAGS_IRQ)
+        PDMDevHlpPCISetIrqNoWait(pDevIns, 0, PDM_IRQ_LEVEL_HIGH);
+
+    PDMCritSectLeave(&pThis->critSectIRQ);
 }
 
@@ -2437 +2430 @@
 #if defined(VBOX_WITH_HGSMI) && (defined(VBOX_WITH_VIDEOHWACCEL) || defined(VBOX_WITH_VDMA) || defined(VBOX_WITH_WDDM))
     PVGASTATE pVGAState = (PVGASTATE)pvCallback;
-    VBVARaiseIrqNoWait (pVGAState, 0);
+    VBVARaiseIrq (pVGAState, 0);
 #else
     NOREF(pvCallback);
@@ -2933 +2926 @@
     pThis->fHostCursorCapabilities &= ~fCapabilitiesRemoved;
     if (pThis->fGuestCaps & VBVACAPS_IRQ && pThis->fGuestCaps & VBVACAPS_DISABLE_CURSOR_INTEGRATION)
-        VBVARaiseIrqNoWait(pThis, HGSMIHOSTFLAGS_CURSOR_CAPABILITIES);
+        VBVARaiseIrq(pThis, HGSMIHOSTFLAGS_CURSOR_CAPABILITIES);
     PDMCritSectLeave(&pThis->CritSect);
 }

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

@@ -2592 +2592 @@
                 vboxVDMACrCmdProcess(pVdma, pCmd, cbCmd);
                 VBoxVBVAExHPDataCompleteCmd(pCmdVbva, cbCmd);
-                VBVARaiseIrqNoWait(pVGAState, 0);
+                VBVARaiseIrq(pVGAState, 0);
                 break;
             case VBVAEXHOST_DATA_TYPE_GUESTCTL: