VirtualBox

Changeset 49958 in vbox for trunk/src


Ignore:
Timestamp:
Dec 17, 2013 3:47:27 PM (11 years ago)
Author:
vboxsync
Message:

EHCI,OHCI: Fix possible deadlock introduced with the first version of the rework. Use a separate critical section instead of the per device one to synchronise interrupt register access between EMT and the frame thread

Location:
trunk/src/VBox/Devices
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/USB/DevOHCI.cpp

    r49814 r49958  
    361361
    362362    uint32_t            Alignment3;     /**< Align size on a 8 byte boundary. */
     363
     364    /** Critical section synchronising interrupt handling. */
     365    PDMCRITSECT         CsIrq;
    363366
    364367    /** The framer thread. */
    … …  
    797800 * Update PCI IRQ levels
    798801 */
    799 static void ohciUpdateInterrupt(POHCI ohci, const char *msg)
     802static void ohciUpdateInterruptLocked(POHCI ohci, const char *msg)
    800803{
    801804    int level = 0;
    802 
    803 #ifdef IN_RING3
    804     PDMCritSectEnter(ohci->pDevInsR3->pCritSectRoR3, VERR_IGNORED);
    805 #endif
    806805
    807806    if (    (ohci->intr & OHCI_INTR_MASTER_INTERRUPT_ENABLED)
    … …  
    818817              (val >> 6) & 1, (val >> 30) & 1, msg)); NOREF(val); NOREF(msg);
    819818    }
    820 
    821 #ifdef IN_RING3
    822     PDMCritSectLeave(ohci->pDevInsR3->pCritSectRoR3);
    823 #endif
    824819}
    825820
    … …  
    827822 * Set an interrupt, use the wrapper ohciSetInterrupt.
    828823 */
    829 DECLINLINE(void) ohciSetInterruptInt(POHCI ohci, uint32_t intr, const char *msg)
    830 {
    831     if ( (ohci->intr_status & intr) == intr )
    832         return;
    833     ohci->intr_status |= intr;
    834     ohciUpdateInterrupt(ohci, msg);
     824DECLINLINE(int) ohciSetInterruptInt(POHCI ohci, int rcBusy, uint32_t intr, const char *msg)
     825{
     826    int rc = VINF_SUCCESS;
     827
     828    rc = PDMCritSectEnter(&ohci->CsIrq, rcBusy);
     829    if (rc != VINF_SUCCESS)
     830        return rc;
     831
     832    if ( (ohci->intr_status & intr) != intr )
     833    {
     834        ohci->intr_status |= intr;
     835        ohciUpdateInterruptLocked(ohci, msg);
     836    }
     837
     838    PDMCritSectLeave(&ohci->CsIrq);
     839    return rc;
    835840}
    836841
    … …  
    838843 * Set an interrupt wrapper macro for logging purposes.
    839844 */
    840 #define ohciSetInterrupt(ohci, intr) ohciSetInterruptInt(ohci, intr, #intr)
    841 
     845#define ohciSetInterrupt(ohci, a_rcBusy, intr) ohciSetInterruptInt(ohci, a_rcBusy, intr, #intr)
     846#define ohciR3SetInterrupt(ohci, intr) ohciSetInterruptInt(ohci, VERR_IGNORED, intr, #intr)
    842847
    843848#ifdef IN_RING3
    … …  
    960965
    961966    ohci_remote_wakeup(pThis);
    962     ohciSetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
     967    ohciR3SetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
    963968
    964969    PDMCritSectLeave(pThis->pDevInsR3->pCritSectRoR3);
    … …  
    9971002
    9981003    ohci_remote_wakeup(pThis);
    999     ohciSetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
     1004    ohciR3SetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
    10001005
    10011006    PDMCritSectLeave(pThis->pDevInsR3->pCritSectRoR3);
    … …  
    35553560    ohciPhysWrite(pThis, pThis->hcca + OHCI_HCCA_OFS, (uint8_t *)&hcca, sizeof(hcca));
    35563561    if (fWriteDoneHeadInterrupt)
    3557         ohciSetInterrupt(pThis, OHCI_INTR_WRITE_DONE_HEAD);
     3562        ohciR3SetInterrupt(pThis, OHCI_INTR_WRITE_DONE_HEAD);
    35583563}
    35593564
    … …  
    37013706
    37023707    /* "After writing to HCCA, HC will set SF in HcInterruptStatus" - guest isn't executing, so ignore the order! */
    3703     ohciSetInterrupt(pThis, OHCI_INTR_START_OF_FRAME);
     3708    ohciR3SetInterrupt(pThis, OHCI_INTR_START_OF_FRAME);
    37043709
    37053710    if (pThis->fno)
    37063711    {
    3707         ohciSetInterrupt(pThis, OHCI_INTR_FRAMENUMBER_OVERFLOW);
     3712        ohciR3SetInterrupt(pThis, OHCI_INTR_FRAMENUMBER_OVERFLOW);
    37083713        pThis->fno = 0;
    37093714    }
    … …  
    39213926
    39223927    if (fHardware && (pThis->ctl & OHCI_CTL_RWE))
    3923         ohciSetInterrupt(pThis, OHCI_INTR_RESUME_DETECT);
     3928        ohciR3SetInterrupt(pThis, OHCI_INTR_RESUME_DETECT);
    39243929
    39253930    ohciBusStart(pThis);
    … …  
    41274132    uint32_t res = pThis->intr_status & ~val;
    41284133    uint32_t chg = pThis->intr_status ^ res; NOREF(chg);
     4134    int rc = VINF_SUCCESS;
     4135
     4136    rc = PDMCritSectEnter(&pThis->CsIrq, VINF_IOM_R3_MMIO_WRITE);
     4137    if (rc != VINF_SUCCESS)
     4138        return rc;
     4139
    41294140    Log2(("HcInterruptStatus_w(%#010x) => %sSO=%d %sWDH=%d %sSF=%d %sRD=%d %sUE=%d %sFNO=%d %sRHSC=%d %sOC=%d\n",
    41304141          val,
    … …  
    41454156     */
    41464157    pThis->intr_status &= ~val;
    4147     ohciUpdateInterrupt(pThis, "HcInterruptStatus_w");
     4158    ohciUpdateInterruptLocked(pThis, "HcInterruptStatus_w");
     4159    PDMCritSectLeave(&pThis->CsIrq);
    41484160    return VINF_SUCCESS;
    41494161}
    … …  
    41694181    uint32_t res = pThis->intr | val;
    41704182    uint32_t chg = pThis->intr ^ res; NOREF(chg);
     4183    int rc = VINF_SUCCESS;
     4184
     4185    rc = PDMCritSectEnter(&pThis->CsIrq, VINF_IOM_R3_MMIO_WRITE);
     4186    if (rc != VINF_SUCCESS)
     4187        return rc;
     4188
    41714189    Log2(("HcInterruptEnable_w(%#010x) => %sSO=%d %sWDH=%d %sSF=%d %sRD=%d %sUE=%d %sFNO=%d %sRHSC=%d %sOC=%d %sMIE=%d\n",
    41724190          val,
    … …  
    41844202
    41854203    pThis->intr |= val;
    4186     ohciUpdateInterrupt(pThis, "HcInterruptEnable_w");
     4204    ohciUpdateInterruptLocked(pThis, "HcInterruptEnable_w");
     4205    PDMCritSectLeave(&pThis->CsIrq);
    41874206    return VINF_SUCCESS;
    41884207}
    … …  
    42134232    uint32_t res = pThis->intr & ~val;
    42144233    uint32_t chg = pThis->intr ^ res; NOREF(chg);
     4234    int rc = VINF_SUCCESS;
     4235
     4236    rc = PDMCritSectEnter(&pThis->CsIrq, VINF_IOM_R3_MMIO_WRITE);
     4237    if (rc != VINF_SUCCESS)
     4238        return rc;
     4239
    42154240    Log2(("HcInterruptDisable_w(%#010x) => %sSO=%d %sWDH=%d %sSF=%d %sRD=%d %sUE=%d %sFNO=%d %sRHSC=%d %sOC=%d %sMIE=%d\n",
    42164241          val,
    … …  
    42284253
    42294254    pThis->intr &= ~val;
    4230     ohciUpdateInterrupt(pThis, "HcInterruptDisable_w");
     4255    ohciUpdateInterruptLocked(pThis, "HcInterruptDisable_w");
     4256    PDMCritSectLeave(&pThis->CsIrq);
    42314257    return VINF_SUCCESS;
    42324258}
    … …  
    47684794
    47694795    /* Raise roothub status change interrupt. */
    4770     ohciSetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
     4796    ohciR3SetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
    47714797}
    47724798
    … …  
    47924818    {
    47934819        pRh->aPorts[iPort].fReg |= OHCI_PORT_R_CONNECT_STATUS_CHANGE;
    4794         ohciSetInterrupt(pRh->pOhci, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
     4820        ohciR3SetInterrupt(pRh->pOhci, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
    47954821        return false;
    47964822    }
    … …  
    48874913        pThis->RootHub.aPorts[i].fReg &= ~OHCI_PORT_R_SUSPEND_STATUS;
    48884914        pThis->RootHub.aPorts[i].fReg |= OHCI_PORT_R_SUSPEND_STATUS_CHANGE;
    4889         ohciSetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
     4915        ohciR3SetInterrupt(pThis, OHCI_INTR_ROOT_HUB_STATUS_CHANGE);
    48904916    }
    48914917
    … …  
    55185544    RTSemEventDestroy(pThis->hSemEventFrame);
    55195545    RTCritSectDelete(&pThis->CritSect);
     5546    PDMR3CritSectDelete(&pThis->CsIrq);
    55205547
    55215548    /*
    … …  
    56645691    pThis->fBusStarted = false;
    56655692
     5693    rc = PDMDevHlpCritSectInit(pDevIns, &pThis->CsIrq, RT_SRC_POS, "OHCI#%uIrq", iInstance);
     5694    if (RT_FAILURE(rc))
     5695        return PDMDevHlpVMSetError(pDevIns, rc, RT_SRC_POS,
     5696                                   N_("EHCI: Failed to create critical section"));
     5697
    56665698    rc = RTSemEventCreate(&pThis->hSemEventFrame);
    56675699    AssertRCReturn(rc, rc);
    56685700
    5669     /*
    5670      * Initialize the critical section without the lock validator.
    5671      * This is necessary because USB devices attached to this controller
    5672      * will be detached in the save state callback with the
    5673      * per device PDM critical section held. If there are still URBs pending
    5674      * for this device they will get reaped and cause a lock validator error
    5675      * because they will take this critical section.
    5676      *
    5677      * The framer thread on the other hand will first take this critical section
    5678      * during a run and might take the PDM critical section when issuing an interrupt.
    5679      * Normally this is a real deadlock issue but we make sure
    5680      * that the framer thread is not running when the save state handler is called.
    5681      */
    5682     rc = RTCritSectInitEx(&pThis->CritSect, RTCRITSECT_FLAGS_NO_LOCK_VAL,
    5683                           NIL_RTLOCKVALCLASS, RTLOCKVAL_SUB_CLASS_USER, "OhciCritSect");
     5701    rc = RTCritSectInit(&pThis->CritSect);
    56845702    if (RT_FAILURE(rc))
    56855703        return PDMDevHlpVMSetError(pDevIns, rc, RT_SRC_POS,

  • trunk/src/VBox/Devices/testcase/tstDeviceStructSizeRC.cpp

    r49814 r49958  
    10031003    GEN_CHECK_OFF(OHCI, hSemEventFrame);
    10041004    GEN_CHECK_OFF(OHCI, fBusStarted);
     1005    GEN_CHECK_OFF(OHCI, CsIrq);
    10051006    GEN_CHECK_OFF(OHCI, nsWait);
    10061007    GEN_CHECK_OFF(OHCI, CritSect);
    … …  
    10851086    GEN_CHECK_OFF(EHCI, hSemEventFrame);
    10861087    GEN_CHECK_OFF(EHCI, fBusStarted);
     1088    GEN_CHECK_OFF(EHCI, CsIrq);
    10871089    GEN_CHECK_OFF(EHCI, nsWait);
    10881090    GEN_CHECK_OFF(EHCI, CritSect);
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette