VirtualBox

Changeset 37508 in vbox for trunk/src/VBox


Ignore:
Timestamp:
Jun 16, 2011 5:23:02 PM (14 years ago)
Author:
vboxsync
Message:

DevHPET: Fixed locking.

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/PC/DevHPET.cpp

    r37507 r37508  
    129129#define DEVHPET_UNLOCK(a_pThis) \
    130130    do { PDMCritSectLeave(&(a_pThis)->csLock); } while (0)
     131
     132
     133/**
     134 * Acquires the TM lock and HPET lock, returns on failure.
     135 */
     136#define DEVHPET_LOCK_BOTH_RETURN(a_pThis, a_rcBusy)  \
     137    do { \
     138        int rcLock = TMTimerLock((a_pThis)->aTimers[0].CTX_SUFF(pTimer), (a_rcBusy)); \
     139        if (rcLock != VINF_SUCCESS) \
     140            return rcLock; \
     141        rcLock = PDMCritSectEnter(&(a_pThis)->csLock, (a_rcBusy)); \
     142        if (rcLock != VINF_SUCCESS) \
     143        { \
     144            TMTimerUnlock((a_pThis)->aTimers[0].CTX_SUFF(pTimer)); \
     145            return rcLock; \
     146        } \
     147    } while (0)
     148
     149
     150/**
     151 * Releases the HPET lock and TM lock.
     152 */
     153#define DEVHPET_UNLOCK_BOTH(a_pThis) \
     154    do { \
     155        PDMCritSectLeave(&(a_pThis)->csLock); \
     156        TMTimerUnlock((a_pThis)->aTimers[0].CTX_SUFF(pTimer)); \
     157    } while (0)
    131158
    132159
    … …  
    338365static void hpetProgramTimer(HpetTimer *pHpetTimer)
    339366{
    340     uint64_t u64Ticks = hpetGetTicks(pHpetTimer->CTX_SUFF(pHpet));
    341     uint64_t u64Diff;
    342     uint32_t u32TillWrap;
    343 
    344367    /* no wrapping on new timers */
    345368    pHpetTimer->u8Wrap = 0;
    346369
     370    uint64_t u64Ticks = hpetGetTicks(pHpetTimer->CTX_SUFF(pHpet));
    347371    hpetAdjustComparator(pHpetTimer, u64Ticks);
    348372
    349     u64Diff = hpetComputeDiff(pHpetTimer, u64Ticks);
     373    uint64_t u64Diff = hpetComputeDiff(pHpetTimer, u64Ticks);
    350374
    351375    /*
    … …  
    356380        && !(pHpetTimer->u64Config & HPET_TN_PERIODIC))
    357381    {
    358         u32TillWrap = 0xffffffff - (uint32_t)u64Ticks + 1;
     382        uint32_t u32TillWrap = 0xffffffff - (uint32_t)u64Ticks + 1;
    359383        if (u32TillWrap < (uint32_t)u64Diff)
    360384        {
    … …  
    366390    }
    367391
    368     /* Avoid killing VM with interrupts */
    369 #if 1
    370     /* @todo: HACK, rethink, may have negative impact on the guest */
     392    /*
     393     * HACK ALERT! Avoid killing VM with interrupts.
     394     */
     395#if 1 /** @todo: HACK, rethink, may have negative impact on the guest */
    371396    if (u64Diff == 0)
    372397        u64Diff = 100000; /* 1 millisecond */
    … …  
    374399
    375400    Log4(("HPET: next IRQ in %lld ticks (%lld ns)\n", u64Diff, hpetTicksToNs(pHpetTimer->CTX_SUFF(pHpet), u64Diff)));
    376 
    377401    TMTimerSetNano(pHpetTimer->CTX_SUFF(pTimer), hpetTicksToNs(pHpetTimer->CTX_SUFF(pHpet), u64Diff));
    378402}
    … …  
    395419static int hpetTimerRegRead32(HpetState const *pThis, uint32_t iTimerNo, uint32_t iTimerReg, uint32_t *pu32Value)
    396420{
     421    Assert(PDMCritSectIsOwner(&pThis->csLock));
     422
    397423    if (iTimerNo >= HPET_NUM_TIMERS)
    398424    {
    … …  
    455481 * @param   idxReg          The register being written to.
    456482 * @param   u32NewValue     The value being written.
     483 *
     484 * @remarks The caller should not hold the device lock, unless it also holds
     485 *          the TM lock.
    457486 */
    458487static int hpetTimerRegWrite32(HpetState *pThis, uint32_t iTimerNo, uint32_t iTimerReg, uint32_t u32NewValue)
    459488{
     489    Assert(!PDMCritSectIsOwner(&pThis->csLock) || TMTimerIsLockOwner(pThis->aTimers[0].CTX_SUFF(pTimer)));
     490
    460491    if (iTimerNo >= HPET_NUM_TIMERS)
    461492    {
    … …  
    469500        case HPET_TN_CFG:
    470501        {
     502            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    471503            Log(("write HPET_TN_CFG: %d: %x\n", iTimerNo, u32NewValue));
    472504            uint64_t const iOldValue = (uint32_t)pHpetTimer->u64Config;
    … …  
    495527            /* We only care about lower 32-bits so far */
    496528            pHpetTimer->u64Config = hpetUpdateMasked(u32NewValue, iOldValue, u64Mask);
     529            DEVHPET_UNLOCK(pThis);
    497530            break;
    498531        }
    … …  
    506539        case HPET_TN_CMP: /* lower bits of comparator register */
    507540        {
    508             Log(("write HPET_TN_CMP on %d: %x\n", iTimerNo, u32NewValue));
     541            DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
     542            Log(("write HPET_TN_CMP on %d: %#x\n", iTimerNo, u32NewValue));
     543
    509544            if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    510545            {
    … …  
    515550
    516551            pHpetTimer->u64Cmp = (pHpetTimer->u64Cmp & UINT64_C(0xffffffff00000000))
    517                                | u32NewValue;
     552                               | u32NewValue; /** @todo RT_MAKE_U64 */
    518553
    519554            pHpetTimer->u64Config &= ~HPET_TN_SETVAL;
    520             Log2(("after HPET_TN_CMP cmp=%llx per=%llx\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period));
     555            Log2(("after HPET_TN_CMP cmp=%#llx per=%#llx\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period));
    521556
    522557            if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    523558                hpetProgramTimer(pHpetTimer);
     559            DEVHPET_UNLOCK_BOTH(pThis);
    524560            break;
    525561        }
    … …  
    527563        case HPET_TN_CMP + 4: /* upper bits of comparator register */
    528564        {
    529             Log(("write HPET_TN_CMP + 4 on %d: %x\n", iTimerNo, u32NewValue));
    530             if (hpet32bitTimer(pHpetTimer))
    531                 break;
    532 
    533             if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    534                 pHpetTimer->u64Period = RT_MAKE_U64(pHpetTimer->u64Period, u32NewValue);
    535             pHpetTimer->u64Cmp = RT_MAKE_U64(pHpetTimer->u64Cmp, u32NewValue);
    536 
    537             Log2(("after HPET_TN_CMP+4 cmp=%llx per=%llx tmr=%d\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period, iTimerNo));
    538 
    539             pHpetTimer->u64Config &= ~HPET_TN_SETVAL;
    540 
    541             if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    542                 hpetProgramTimer(pHpetTimer);
     565            DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
     566            Log(("write HPET_TN_CMP + 4 on %d: %#x\n", iTimerNo, u32NewValue));
     567            if (!hpet32bitTimer(pHpetTimer))
     568            {
     569                if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
     570                    pHpetTimer->u64Period = RT_MAKE_U64(pHpetTimer->u64Period, u32NewValue);
     571                pHpetTimer->u64Cmp = RT_MAKE_U64(pHpetTimer->u64Cmp, u32NewValue);
     572
     573                Log2(("after HPET_TN_CMP+4 cmp=%llx per=%llx tmr=%d\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period, iTimerNo));
     574
     575                pHpetTimer->u64Config &= ~HPET_TN_SETVAL;
     576
     577                if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
     578                    hpetProgramTimer(pHpetTimer);
     579            }
     580            DEVHPET_UNLOCK_BOTH(pThis);
    543581            break;
    544582        }
    … …  
    558596        default:
    559597        {
    560             LogRel(("invalid timer register write: %d\n", iTimerReg));
    561             AssertFailed();
     598            static unsigned s_cOccurences = 0;
     599            if (s_cOccurences++ < 10)
     600                LogRel(("invalid timer register write: %d\n", iTimerReg));
    562601            break;
    563602        }
    … …  
    578617 * @param   idxReg              The register to read.
    579618 * @param   pu32Value           Where to return the register value.
    580  */
    581 static int hpetConfigRegRead32(HpetState const *pThis, uint32_t idxReg, uint32_t *pu32Value)
    582 {
     619 *
     620 * @remarks The caller must not own the device lock if HPET_COUNTER is read.
     621 */
     622static int hpetConfigRegRead32(HpetState *pThis, uint32_t idxReg, uint32_t *pu32Value)
     623{
     624    Assert(!PDMCritSectIsOwner(&pThis->csLock) || (idxReg != HPET_COUNTER && idxReg != HPET_COUNTER + 4));
     625
    583626    uint32_t u32Value;
    584627    switch (idxReg)
    585628    {
    586629        case HPET_ID:
     630            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    587631            u32Value = (uint32_t)pThis->u64Capabilities;
     632            DEVHPET_UNLOCK(pThis);
    588633            Log(("read HPET_ID: %#x\n", u32Value));
    589634            break;
    590635
    591636        case HPET_PERIOD:
     637            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    592638            u32Value = (uint32_t)(pThis->u64Capabilities >> 32);
     639            DEVHPET_UNLOCK(pThis);
    593640            Log(("read HPET_PERIOD: %#x\n", u32Value));
    594641            break;
    595642
    596643        case HPET_CFG:
     644            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    597645            u32Value = (uint32_t)pThis->u64HpetConfig;
     646            DEVHPET_UNLOCK(pThis);
    598647            Log(("read HPET_CFG: %#x\n", u32Value));
    599648            break;
    600649
    601650        case HPET_CFG + 4:
     651            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    602652            u32Value = (uint32_t)(pThis->u64HpetConfig >> 32);
     653            DEVHPET_UNLOCK(pThis);
    603654            Log(("read of HPET_CFG + 4: %#x\n", u32Value));
    604655            break;
    … …  
    607658        case HPET_COUNTER + 4:
    608659        {
     660            DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
     661
    609662            uint64_t u64Ticks;
    610663            if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    … …  
    612665            else
    613666                u64Ticks = pThis->u64HpetCounter;
     667
     668            DEVHPET_UNLOCK_BOTH(pThis);
     669
    614670            /** @todo is it correct? */
    615671            u32Value = (idxReg == HPET_COUNTER) ? (uint32_t)u64Ticks : (uint32_t)(u64Ticks >> 32);
    … …  
    620676
    621677        case HPET_STATUS:
    622             Log(("read HPET_STATUS\n"));
     678            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    623679            u32Value = (uint32_t)pThis->u64Isr;
     680            DEVHPET_UNLOCK(pThis);
     681            Log(("read HPET_STATUS: %#x\n", u32Value));
    624682            break;
    625683
    … …  
    643701 * @param   idxReg          The register being written to.
    644702 * @param   u32NewValue     The value being written.
     703 *
     704 * @remarks The caller should not hold the device lock, unless it also holds
     705 *          the TM lock.
    645706 */
    646707static int hpetConfigRegWrite32(HpetState *pThis, uint32_t idxReg, uint32_t u32NewValue)
    647708{
     709    Assert(!PDMCritSectIsOwner(&pThis->csLock) || TMTimerIsLockOwner(pThis->aTimers[0].CTX_SUFF(pTimer)));
     710
    648711    int rc = VINF_SUCCESS;
    649712    switch (idxReg)
    … …  
    658721        case HPET_CFG:
    659722        {
     723            DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    660724            uint32_t const iOldValue = (uint32_t)(pThis->u64HpetConfig);
    661725            Log(("write HPET_CFG: %x (old %x)\n", u32NewValue, iOldValue));
    … …  
    671735                rc = pThis->pHpetHlpR3->pfnSetLegacyMode(pThis->pDevInsR3, RT_BOOL(u32NewValue & HPET_CFG_LEGACY));
    672736                if (rc != VINF_SUCCESS)
    673                     break;
    674737#else
    675738                rc = VINF_IOM_HC_MMIO_WRITE;
    676                 break;
    677739#endif
     740                {
     741                    DEVHPET_UNLOCK_BOTH(pThis);
     742                    break;
     743                }
    678744            }
    679745
    … …  
    697763                    TMTimerStop(pThis->aTimers[i].CTX_SUFF(pTimer));
    698764            }
     765
     766            DEVHPET_UNLOCK_BOTH(pThis);
    699767            break;
    700768        }
    … …  
    702770        case HPET_CFG + 4:
    703771        {
    704             /** @todo why do we let the guest write to the high bits but not
    705              *        read them? */
     772            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    706773            pThis->u64HpetConfig = hpetUpdateMasked((uint64_t)u32NewValue << 32,
    707774                                                    pThis->u64HpetConfig,
    708775                                                    UINT64_C(0xffffffff00000000));
    709776            Log(("write HPET_CFG + 4: %x -> %#llx\n", u32NewValue, pThis->u64HpetConfig));
     777            DEVHPET_UNLOCK(pThis);
    710778            break;
    711779        }
    … …  
    713781        case HPET_STATUS:
    714782        {
     783            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    715784            /* Clear ISR for all set bits in u32NewValue, see p. 14 of the HPET spec. */
    716785            pThis->u64Isr &= ~((uint64_t)u32NewValue);
    717786            Log(("write HPET_STATUS: %x -> ISR=%#llx\n", u32NewValue, pThis->u64Isr));
     787            DEVHPET_UNLOCK(pThis);
    718788            break;
    719789        }
    … …  
    733803        case HPET_COUNTER:
    734804        {
     805            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    735806            pThis->u64HpetCounter = RT_MAKE_U64(u32NewValue, pThis->u64HpetCounter);
    736807            Log(("write HPET_COUNTER: %#x -> %llx\n", u32NewValue, pThis->u64HpetCounter));
     808            DEVHPET_UNLOCK(pThis);
    737809            break;
    738810        }
    … …  
    740812        case HPET_COUNTER + 4:
    741813        {
     814            DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    742815            pThis->u64HpetCounter = RT_MAKE_U64(pThis->u64HpetCounter, u32NewValue);
    743816            Log(("write HPET_COUNTER + 4: %#x -> %llx\n", u32NewValue, pThis->u64HpetCounter));
     817            DEVHPET_UNLOCK(pThis);
    744818            break;
    745819        }
    746820
    747821        default:
    748             LogRel(("invalid HPET config write: %x\n", idxReg));
    749             break;
     822        {
     823            static unsigned s_cOccurences = 0;
     824            if (s_cOccurences++ < 10)
     825                LogRel(("invalid HPET config write: %x\n", idxReg));
     826            break;
     827        }
    750828    }
    751829
    … …  
    766844
    767845    LogFlow(("hpetMMIORead (%d): %llx (%x)\n", cb, (uint64_t)GCPhysAddr, idxReg));
    768     DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    769846
    770847    int rc = VINF_SUCCESS;
    … …  
    773850        case 4:
    774851            if (idxReg >= 0x100 && idxReg < 0x400)
     852            {
     853                DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
    775854                rc = hpetTimerRegRead32(pThis,
    776855                                        (idxReg - 0x100) / 0x20,
    777856                                        (idxReg - 0x100) % 0x20,
    778857                                        (uint32_t *)pv);
     858                DEVHPET_UNLOCK(pThis);
     859            }
    779860            else
    780861                rc = hpetConfigRegRead32(pThis, idxReg, (uint32_t *)pv);
    … …  
    797878                /* When reading HPET counter we must read it in a single read,
    798879                   to avoid unexpected time jumps on 32-bit overflow. */
    799                 pValue->u = (pThis->u64HpetConfig & HPET_CFG_ENABLE) != 0
    800                           ? hpetGetTicks(pThis)
    801                           : pThis->u64HpetCounter;
    802                 rc = VINF_SUCCESS;
    803             }
    804             else if (idxReg >= 0x100 && idxReg < 0x400)
    805             {
    806                 uint32_t iTimer    = (idxReg - 0x100) / 0x20;
    807                 uint32_t iTimerReg = (idxReg - 0x100) % 0x20;
    808                 rc = hpetTimerRegRead32(pThis, iTimer, iTimerReg, &pValue->s.Lo);
    809                 if (rc == VINF_SUCCESS)
    810                     rc = hpetTimerRegRead32(pThis, iTimer, iTimerReg + 4, &pValue->s.Hi);
     880                DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
     881                if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
     882                    pValue->u = hpetGetTicks(pThis);
     883                else
     884                    pValue->u = pThis->u64HpetCounter;
     885                DEVHPET_UNLOCK_BOTH(pThis);
    811886            }
    812887            else
    813888            {
    814                 /* for most 8-byte accesses we just split them, happens under lock anyway. */
    815                 rc = hpetConfigRegRead32(pThis, idxReg, &pValue->s.Lo);
    816                 if (rc == VINF_SUCCESS)
    817                     rc = hpetConfigRegRead32(pThis, idxReg + 4, &pValue->s.Hi);
     889                DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_READ);
     890                if (idxReg >= 0x100 && idxReg < 0x400)
     891                {
     892                    uint32_t iTimer    = (idxReg - 0x100) / 0x20;
     893                    uint32_t iTimerReg = (idxReg - 0x100) % 0x20;
     894                    rc = hpetTimerRegRead32(pThis, iTimer, iTimerReg, &pValue->s.Lo);
     895                    if (rc == VINF_SUCCESS)
     896                        rc = hpetTimerRegRead32(pThis, iTimer, iTimerReg + 4, &pValue->s.Hi);
     897                }
     898                else
     899                {
     900                    /* for most 8-byte accesses we just split them, happens under lock anyway. */
     901                    rc = hpetConfigRegRead32(pThis, idxReg, &pValue->s.Lo);
     902                    if (rc == VINF_SUCCESS)
     903                        rc = hpetConfigRegRead32(pThis, idxReg + 4, &pValue->s.Hi);
     904                }
     905                DEVHPET_UNLOCK(pThis);
    818906            }
    819907            break;
    … …  
    831919    }
    832920
    833     DEVHPET_UNLOCK(pThis);
    834921    return rc;
    835922}
    … …  
    845932    LogFlow(("hpetMMIOWrite: cb=%u reg=%03x (%RGp) val=%llx\n",
    846933             cb, idxReg, GCPhysAddr, cb == 4 ? *(uint32_t *)pv : cb == 8 ? *(uint64_t *)pv : 0xdeadbeef));
    847 
    848     DEVHPET_LOCK_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    849934
    850935    int rc;
    … …  
    871956
    872957            /* Split the access and rely on the locking to prevent trouble. */
     958            DEVHPET_LOCK_BOTH_RETURN(pThis, VINF_IOM_HC_MMIO_WRITE);
    873959            RTUINT64U uValue;
    874960            uValue.u = *(uint64_t const *)pv;
    … …  
    888974                    rc = hpetConfigRegWrite32(pThis, idxReg + 4, uValue.s.Hi);
    889975            }
     976            DEVHPET_UNLOCK_BOTH(pThis);
    890977            break;
    891978        }
    … …  
    903990    }
    904991
    905     DEVHPET_UNLOCK(pThis);
    906992    return rc;
    907993}
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