VirtualBox

Changeset 88351 in vbox


Ignore:
Timestamp:
Apr 2, 2021 9:50:33 PM (4 years ago)
Author:
vboxsync
svn:sync-xref-src-repo-rev:
143595
Message:

Forward ported r143586 from 6.1: DevHPET: Reduce locking in timer callback and other places. Fixed timer frequency hint calculation. Don't get virtual sync time more than once if we can help it. Separate 64-bit timer register access implemention. General cleanups. A bunch of TODOs. ​oem2ticketref:40

File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r87767 r88351  
    204204/**
    205205 * A HPET timer.
     206 *
     207 * @note    To avoid excessive locking, we many of the updates atomically.
    206208 */
    207209typedef struct HPETTIMER
    … …  
    237239} HPETTIMER;
    238240AssertCompileMemberAlignment(HPETTIMER, u64Config, sizeof(uint64_t));
     241AssertCompileSizeAlignment(HPETTIMER, 64);
    239242/** Pointer to the shared state of an HPET timer. */
    240243typedef HPETTIMER *PHPETTIMER;
    … …  
    271274    bool                        fIch9;
    272275    /** Size alignment padding. */
    273     uint8_t                     abPadding0[7];
     276    uint8_t                     abPadding0[7+8];
     277
     278    /** The handle of the MMIO region. */
     279    IOMMMIOHANDLE               hMmio;
    274280
    275281    /** Global device lock. */
    276282    PDMCRITSECT                 CritSect;
    277 
    278     /** The handle of the MMIO region. */
    279     IOMMMIOHANDLE               hMmio;
    280283
    281284    STAMCOUNTER                 StatCounterRead4Byte;
    … …  
    283286    STAMCOUNTER                 StatCounterWriteLow;
    284287    STAMCOUNTER                 StatCounterWriteHigh;
     288    STAMCOUNTER                 StatZeroDeltaHack;
    285289} HPET;
     290AssertCompileMemberAlignment(HPET, aTimers, 64);
     291AssertCompileMemberAlignment(HPET, CritSect, 64);
    286292/** Pointer to the shared HPET device state. */
    287293typedef HPET *PHPET;
    … …  
    334340#ifndef VBOX_DEVICE_STRUCT_TESTCASE
    335341
     342DECLINLINE(bool) hpet32bitTimerEx(uint64_t fConfig)
     343{
     344    return !(fConfig & HPET_TN_SIZE_CAP)
     345        ||  (fConfig & HPET_TN_32BIT);
     346}
     347
    336348
    337349DECLINLINE(bool) hpet32bitTimer(PHPETTIMER pHpetTimer)
    338350{
    339     uint64_t u64Cfg = pHpetTimer->u64Config;
    340     return ((u64Cfg & HPET_TN_SIZE_CAP) == 0) || ((u64Cfg & HPET_TN_32BIT) != 0);
    341 }
     351    return hpet32bitTimerEx(ASMAtomicUoReadU64(&pHpetTimer->u64Config));
     352}
     353
    342354
    343355DECLINLINE(uint64_t) hpetInvalidValue(PHPETTIMER pHpetTimer)
    … …  
    356368}
    357369
    358 DECLINLINE(uint64_t) hpetGetTicks(PPDMDEVINS pDevIns, PCHPET pThis)
    359 {
    360     /*
    361      * We can use any timer to get current time, they all go with the same speed.
    362      */
    363     return nsToHpetTicks(pThis, PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer) + pThis->u64HpetOffset);
     370DECLINLINE(uint64_t) hpetGetTicksEx(PCHPET pThis, uint64_t tsNow)
     371{
     372    return nsToHpetTicks(pThis, tsNow + pThis->u64HpetOffset);
    364373}
    365374
    … …  
    383392}
    384393
    385 DECLINLINE(uint64_t) hpetComputeDiff(PHPETTIMER pHpetTimer, uint64_t u64Now)
    386 {
    387 
    388     if (hpet32bitTimer(pHpetTimer))
    389     {
    390         uint32_t u32Diff;
    391 
    392         u32Diff = (uint32_t)pHpetTimer->u64Cmp - (uint32_t)u64Now;
    393         u32Diff = (int32_t)u32Diff > 0 ? u32Diff : (uint32_t)0;
    394         return (uint64_t)u32Diff;
    395     }
    396     uint64_t u64Diff;
    397 
    398     u64Diff = pHpetTimer->u64Cmp - u64Now;
    399     u64Diff = (int64_t)u64Diff > 0 ?  u64Diff : (uint64_t)0;
    400     return u64Diff;
    401 }
    402 
    403 
    404 static void hpetAdjustComparator(PHPETTIMER pHpetTimer, uint64_t u64Now)
    405 {
    406     if ((pHpetTimer->u64Config & HPET_TN_PERIODIC))
    407     {
    408         uint64_t u64Period = pHpetTimer->u64Period;
    409         if (u64Period)
    410         {
    411             uint64_t  cPeriods = (u64Now - pHpetTimer->u64Cmp) / u64Period;
    412             pHpetTimer->u64Cmp += (cPeriods + 1) * u64Period;
    413         }
    414     }
     394DECLINLINE(uint64_t) hpetComputeDiff(uint64_t fConfig, uint64_t uCmp, uint64_t uHpetNow)
     395{
     396    if (hpet32bitTimerEx(fConfig))
     397    {
     398        uint32_t u32Diff = (uint32_t)uCmp - (uint32_t)uHpetNow;
     399        if ((int32_t)u32Diff > 0)
     400            return u32Diff;
     401    }
     402    else
     403    {
     404        uint64_t u64Diff = uCmp - uHpetNow;
     405        if ((int64_t)u64Diff > 0)
     406            return u64Diff;
     407    }
     408    return 0;
     409}
     410
     411
     412DECLINLINE(uint64_t) hpetAdjustComparator(PHPETTIMER pHpetTimer, uint64_t fConfig, uint64_t uCmp,
     413                                          uint64_t uPeriod, uint64_t uHpetNow)
     414{
     415    if (fConfig & HPET_TN_PERIODIC)
     416    {
     417        if (uPeriod)
     418        {
     419            uint64_t cPeriods = (uHpetNow - uCmp) / uPeriod;
     420            uCmp += (cPeriods + 1) * uPeriod;
     421            ASMAtomicWriteU64(&pHpetTimer->u64Cmp, uCmp);
     422        }
     423    }
     424    return uCmp;
    415425}
    416426
    … …  
    422432 * @param   pThis       The shared HPET state.
    423433 * @param   pHpetTimer  The timer.
    424  */
    425 DECLINLINE(void) hpetTimerSetFrequencyHint(PPDMDEVINS pDevIns, PHPET pThis, PHPETTIMER pHpetTimer)
    426 {
    427     if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    428     {
    429         uint64_t const u64Period = pHpetTimer->u64Period;
    430         uint32_t const u32Freq   = pThis->u32Period;
    431         if (u64Period > 0 && u64Period < u32Freq)
    432             PDMDevHlpTimerSetFrequencyHint(pDevIns, pHpetTimer->hTimer, u32Freq / (uint32_t)u64Period);
    433     }
    434 }
    435 
    436 
    437 static void hpetProgramTimer(PPDMDEVINS pDevIns, PHPET pThis, PHPETTIMER pHpetTimer)
    438 {
    439     /* no wrapping on new timers */
    440     pHpetTimer->u8Wrap = 0;
    441 
    442     uint64_t u64Ticks = hpetGetTicks(pDevIns, pThis);
    443     hpetAdjustComparator(pHpetTimer, u64Ticks);
    444 
    445     uint64_t u64Diff = hpetComputeDiff(pHpetTimer, u64Ticks);
     434 * @param   fConfig     Already read config value.
     435 * @param   uPeriod     Already read period value.
     436 */
     437DECLINLINE(void) hpetTimerSetFrequencyHint(PPDMDEVINS pDevIns, PHPET pThis, PHPETTIMER pHpetTimer,
     438                                           uint64_t fConfig, uint64_t uPeriod)
     439{
     440    if (fConfig & HPET_TN_PERIODIC)
     441    {
     442        uint64_t const nsPeriod = hpetTicksToNs(pThis, uPeriod);
     443        if (nsPeriod < RT_NS_100MS)
     444            PDMDevHlpTimerSetFrequencyHint(pDevIns, pHpetTimer->hTimer, RT_NS_1SEC / (uint32_t)nsPeriod);
     445    }
     446}
     447
     448
     449/**
     450 * Programs an HPET timer, arming hTimer for the next IRQ.
     451 *
     452 * @param   pDevIns     The device instance.
     453 * @param   pThis       The HPET instance data.
     454 * @param   pHpetTimer  The HPET timer to program.  The wrap-around indicator is
     455 *                      updates, and for periodic timer the comparator.
     456 * @param   tsNow       The current virtual sync clock time.
     457 * @note    Caller must both the virtual sync (timer) and HPET locks.
     458 */
     459static void hpetProgramTimer(PPDMDEVINS pDevIns, PHPET pThis, PHPETTIMER pHpetTimer, uint64_t const tsNow)
     460{
     461    /*
     462     * Calculate the number of HPET ticks to the next timer IRQ, but
     463     * first updating comparator if periodic timer.
     464     */
     465    uint64_t const fConfig   = pHpetTimer->u64Config;
     466    uint64_t const uPeriod   = pHpetTimer->u64Period;
     467    uint64_t       uCmp      = pHpetTimer->u64Cmp;
     468    uint64_t const uHpetNow  = hpetGetTicksEx(pThis, tsNow);
     469    uCmp = hpetAdjustComparator(pHpetTimer, fConfig, uCmp, uPeriod, uHpetNow);
     470    uint64_t       uHpetDelta = hpetComputeDiff(fConfig, uCmp, uHpetNow);
    446471
    447472    /*
    … …  
    449474     * counter wraps in addition to an interrupt with comparator match.
    450475     */
    451     if (    hpet32bitTimer(pHpetTimer)
    452         && !(pHpetTimer->u64Config & HPET_TN_PERIODIC))
    453     {
    454         uint32_t u32TillWrap = 0xffffffff - (uint32_t)u64Ticks + 1;
    455         if (u32TillWrap < (uint32_t)u64Diff)
    456         {
    457             Log(("wrap on timer %d: till=%u ticks=%lld diff64=%lld\n",
    458                  pHpetTimer->idxTimer, u32TillWrap, u64Ticks, u64Diff));
    459             u64Diff = u32TillWrap;
    460             pHpetTimer->u8Wrap = 1;
    461         }
    462     }
     476    bool fWrap = false;
     477    if (    hpet32bitTimerEx(fConfig)
     478        && !(fConfig & HPET_TN_PERIODIC))
     479    {
     480        uint32_t cHpetTicksTillWrap = UINT32_MAX - (uint32_t)uHpetNow + 1;
     481        if (cHpetTicksTillWrap < (uint32_t)uHpetDelta)
     482        {
     483            Log(("HPET[%u]: wrap: till=%u ticks=%lld diff64=%lld\n",
     484                 pHpetTimer->idxTimer, cHpetTicksTillWrap, uHpetNow, uHpetDelta));
     485            uHpetDelta = cHpetTicksTillWrap;
     486            fWrap      = true;
     487        }
     488    }
     489    pHpetTimer->u8Wrap = fWrap;
    463490
    464491    /*
    … …  
    466493     */
    467494#if 1 /** @todo HACK, rethink, may have negative impact on the guest */
    468     if (u64Diff == 0)
    469         u64Diff = 100000; /* 1 millisecond */
     495    if (uHpetDelta != 0)
     496    { /* likely? */ }
     497    else
     498    {
     499        Log(("HPET[%u]: Applying zero delta hack!\n", pHpetTimer->idxTimer));
     500        STAM_REL_COUNTER_INC(&pThis->StatZeroDeltaHack);
     501/** @todo lower this.   */
     502        uHpetDelta = pThis->fIch9 ? 14318 : 100000; /* 1 millisecond */
     503    }
    470504#endif
    471505
     506    /*
     507     * Arm the timer.
     508     */
    472509    uint64_t u64TickLimit = pThis->fIch9 ? HPET_TICKS_IN_100YR_ICH9 : HPET_TICKS_IN_100YR_PIIX;
    473     if (u64Diff <= u64TickLimit)
    474     {
    475         Log4(("HPET: next IRQ in %lld ticks (%lld ns)\n", u64Diff, hpetTicksToNs(pThis, u64Diff)));
     510    if (uHpetDelta <= u64TickLimit)
     511    {
     512        uint64_t const tsDeadline = tsNow + hpetTicksToNs(pThis, uHpetDelta);
     513        Log4(("HPET[%u]: next IRQ in %lld ticks (at %llu)\n", pHpetTimer->idxTimer, uHpetDelta, tsDeadline));
     514        PDMDevHlpTimerSet(pDevIns, pHpetTimer->hTimer, tsDeadline);
     515        hpetTimerSetFrequencyHint(pDevIns, pThis, pHpetTimer, fConfig, uPeriod);
    476516        STAM_REL_COUNTER_INC(&pHpetTimer->StatSetTimer);
    477         PDMDevHlpTimerSetNano(pDevIns, pHpetTimer->hTimer, hpetTicksToNs(pThis, u64Diff));
    478517    }
    479518    else
    480     {
    481         LogRelMax(10, ("HPET: Not scheduling an interrupt more than 100 years in the future.\n"));
    482     }
    483     hpetTimerSetFrequencyHint(pDevIns, pThis, pHpetTimer);
     519        LogRelMax(10, ("HPET[%u]: Not scheduling an interrupt more than 100 years in the future.\n", pHpetTimer->idxTimer));
    484520}
    485521
    … …  
    491527 * Reads a HPET timer register.
    492528 *
    493  * @param   pDevIns             The device instance.
     529 * @returns The register value.
    494530 * @param   pThis               The HPET instance.
    495531 * @param   iTimerNo            The timer index.
    496532 * @param   iTimerReg           The index of the timer register to read.
    497  * @param   pu32Value           Where to return the register value.
    498  *
    499  * @remarks ASSUMES the caller holds the HPET lock.
    500  */
    501 static void hpetTimerRegRead32(PPDMDEVINS pDevIns, PCHPET pThis, uint32_t iTimerNo, uint32_t iTimerReg, uint32_t *pu32Value)
    502 {
    503     Assert(PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect));
    504     RT_NOREF(pDevIns);
    505 
     533 *
     534 * @note    No locking required.
     535 */
     536static uint32_t hpetTimerRegRead32(PHPET pThis, uint32_t iTimerNo, uint32_t iTimerReg)
     537{
    506538    uint32_t u32Value;
    507539    if (   iTimerNo < HPET_CAP_GET_TIMERS(pThis->u32Capabilities)
    508540        && iTimerNo < RT_ELEMENTS(pThis->aTimers) )
    509541    {
    510         PCHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
     542        PHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
    511543        switch (iTimerReg)
    512544        {
    513545            case HPET_TN_CFG:
    514                 u32Value = (uint32_t)pHpetTimer->u64Config;
    515                 Log(("read HPET_TN_CFG on %d: %#x\n", iTimerNo, u32Value));
     546                u32Value = (uint32_t)ASMAtomicReadU64(&pHpetTimer->u64Config);
     547                Log(("HPET[%u]: read32 HPET_TN_CFG: %#x\n", iTimerNo, u32Value));
    516548                break;
    517549
    518550            case HPET_TN_CFG + 4:
    519                 u32Value = (uint32_t)(pHpetTimer->u64Config >> 32);
    520                 Log(("read HPET_TN_CFG+4 on %d: %#x\n", iTimerNo, u32Value));
     551                u32Value = (uint32_t)(ASMAtomicReadU64(&pHpetTimer->u64Config) >> 32);
     552                Log(("HPET[%u]: read32 HPET_TN_CFG+4: %#x\n", iTimerNo, u32Value));
    521553                break;
    522554
    523555            case HPET_TN_CMP:
    524                 u32Value = (uint32_t)pHpetTimer->u64Cmp;
    525                 Log(("read HPET_TN_CMP on %d: %#x (%#llx)\n", pHpetTimer->idxTimer, u32Value, pHpetTimer->u64Cmp));
    526                 break;
     556            {
     557                uint64_t uCmp = ASMAtomicReadU64(&pHpetTimer->u64Cmp);
     558                u32Value = (uint32_t)uCmp;
     559                Log(("HPET[%u]: read32 HPET_TN_CMP: %#x (%#RX64)\n", pHpetTimer->idxTimer, u32Value, uCmp));
     560                break;
     561            }
    527562
    528563            case HPET_TN_CMP + 4:
    529                 u32Value = (uint32_t)(pHpetTimer->u64Cmp >> 32);
    530                 Log(("read HPET_TN_CMP+4 on %d: %#x (%#llx)\n", pHpetTimer->idxTimer, u32Value, pHpetTimer->u64Cmp));
    531                 break;
     564            {
     565                uint64_t uCmp = ASMAtomicReadU64(&pHpetTimer->u64Cmp);
     566                u32Value = (uint32_t)(uCmp >> 32);
     567                Log(("HPET[%u]: read32 HPET_TN_CMP+4: %#x (%#RX64)\n", pHpetTimer->idxTimer, u32Value, uCmp));
     568                break;
     569            }
    532570
    533571            case HPET_TN_ROUTE:
    534572                u32Value = (uint32_t)(pHpetTimer->u64Fsb >> 32); /** @todo Looks wrong, but since it's not supported, who cares. */
    535                 Log(("read HPET_TN_ROUTE on %d: %#x\n", iTimerNo, u32Value));
     573                Log(("HPET[%u]: read32 HPET_TN_ROUTE: %#x\n", iTimerNo, u32Value));
    536574                break;
    537575
    538576            default:
    539             {
    540                 LogRelMax(10, ("HPET: Invalid HPET register read %d on %d\n", iTimerReg, pHpetTimer->idxTimer));
     577                LogRelMax(10, ("HPET[%u]: Invalid HPET register read: %d\n", iTimerNo, iTimerReg));
    541578                u32Value = 0;
    542579                break;
    543             }
    544580        }
    545581    }
    … …  
    549585        u32Value = 0;
    550586    }
    551     *pu32Value = u32Value;
     587    return u32Value;
     588}
     589
     590
     591/**
     592 * Reads a HPET timer register, 64-bit access.
     593 *
     594 * @returns The register value.
     595 * @param   pThis               The HPET instance.
     596 * @param   iTimerNo            The timer index.
     597 * @param   iTimerReg           The index of the timer register to read.
     598 */
     599static uint64_t hpetTimerRegRead64(PHPET pThis, uint32_t iTimerNo, uint32_t iTimerReg)
     600{
     601    uint64_t u64Value;
     602    if (   iTimerNo < HPET_CAP_GET_TIMERS(pThis->u32Capabilities)
     603        && iTimerNo < RT_ELEMENTS(pThis->aTimers) )
     604    {
     605        PHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
     606        switch (iTimerReg)
     607        {
     608            case HPET_TN_CFG:
     609                u64Value = ASMAtomicReadU64(&pHpetTimer->u64Config);
     610                Log(("HPET[%u]: read64 HPET_TN_CFG: %#RX64\n", iTimerNo, u64Value));
     611                break;
     612
     613            case HPET_TN_CMP:
     614                u64Value = ASMAtomicReadU64(&pHpetTimer->u64Config);
     615                Log(("HPET[%u]: read64 HPET_TN_CMP: %#RX64\n", iTimerNo, u64Value));
     616                break;
     617
     618            case HPET_TN_ROUTE:
     619                u64Value = (uint32_t)(pHpetTimer->u64Fsb >> 32); /** @todo Looks wrong, but since it's not supported, who cares. */
     620                Log(("HPET[%u]: read64 HPET_TN_ROUTE: %#RX64\n", iTimerNo, u64Value));
     621                break;
     622
     623            default:
     624                LogRelMax(10, ("HPET[%u]: Invalid 64-bit HPET register read64: %d\n", iTimerNo, iTimerReg));
     625                u64Value = 0;
     626                break;
     627        }
     628    }
     629    else
     630    {
     631        LogRelMax(10, ("HPET: Using timer above configured range: %d\n", iTimerNo));
     632        u64Value = 0;
     633    }
     634    return u64Value;
    552635}
    553636
    … …  
    564647 * @param   u32NewValue     The value being written.
    565648 *
    566  * @remarks The caller should not hold the device lock, unless it also holds
    567  *          the TM lock.
     649 * @remarks The caller should not hold any locks.
    568650 */
    569651static VBOXSTRICTRC hpetTimerRegWrite32(PPDMDEVINS pDevIns, PHPET pThis, uint32_t iTimerNo,
    570652                                        uint32_t iTimerReg, uint32_t u32NewValue)
    571653{
    572     Assert(!PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect) || PDMDevHlpTimerIsLockOwner(pDevIns, pThis->aTimers[0].hTimer));
    573 
    574     if (   iTimerNo >= HPET_CAP_GET_TIMERS(pThis->u32Capabilities)
    575         || iTimerNo >= RT_ELEMENTS(pThis->aTimers) )    /* Parfait - see above. */
    576     {
    577         LogRelMax(10, ("HPET: Using timer above configured range: %d\n", iTimerNo));
    578         return VINF_SUCCESS;
    579     }
    580     PHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
    581 
    582     switch (iTimerReg)
    583     {
    584         case HPET_TN_CFG:
    585         {
    586             DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    587             uint64_t    u64Mask = HPET_TN_CFG_WRITE_MASK;
    588 
    589             Log(("write HPET_TN_CFG: %d: %x\n", iTimerNo, u32NewValue));
    590             if (pHpetTimer->u64Config & HPET_TN_PERIODIC_CAP)
    591                 u64Mask |= HPET_TN_PERIODIC;
    592 
    593             if (pHpetTimer->u64Config & HPET_TN_SIZE_CAP)
    594                 u64Mask |= HPET_TN_32BIT;
    595             else
    596                 u32NewValue &= ~HPET_TN_32BIT;
    597 
    598             if (u32NewValue & HPET_TN_32BIT)
     654    Assert(!PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect));
     655    Assert(!PDMDevHlpTimerIsLockOwner(pDevIns, pThis->aTimers[0].hTimer));
     656
     657    if (   iTimerNo < HPET_CAP_GET_TIMERS(pThis->u32Capabilities)
     658        && iTimerNo < RT_ELEMENTS(pThis->aTimers) )    /* Parfait - see above. */
     659    {
     660        PHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
     661
     662        switch (iTimerReg)
     663        {
     664            case HPET_TN_CFG:
    599665            {
    600                 Log(("setting timer %d to 32-bit mode\n", iTimerNo));
    601                 pHpetTimer->u64Cmp    = (uint32_t)pHpetTimer->u64Cmp;
    602                 pHpetTimer->u64Period = (uint32_t)pHpetTimer->u64Period;
     666                /*
     667                 * Calculate the writable mask and see if anything actually changed
     668                 * before doing any locking.  Windows 10 (1809) does two CFG writes
     669                 * with the same value (0x134) when reprogramming the HPET#0 timer.
     670                 */
     671                uint64_t       fConfig = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
     672                uint64_t const fMask   = HPET_TN_CFG_WRITE_MASK
     673                                       | (fConfig & HPET_TN_PERIODIC_CAP ? HPET_TN_PERIODIC : 0)
     674                                       | (fConfig & HPET_TN_SIZE_CAP     ? HPET_TN_32BIT    : 0);
     675                if ((u32NewValue & fMask) == (fConfig & fMask))
     676                    Log(("HPET[%u]: write32 HPET_TN_CFG: %#x - no change (%#RX64)\n", iTimerNo, u32NewValue, fConfig));
     677                else
     678                {
     679#ifndef IN_RING3
     680                    /* Return to ring-3 (where LogRel works) to complain about level-triggered interrupts. */
     681                    if ((u32NewValue & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_LEVEL)
     682                        return VINF_IOM_R3_MMIO_WRITE;
     683#endif
     684                    DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
     685
     686                    fConfig = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
     687                    uint64_t const fConfigNew = hpetUpdateMasked(u32NewValue, fConfig, fMask);
     688                    Log(("HPET[%u]: write HPET_TN_CFG: %#RX64 -> %#RX64\n", iTimerNo, fConfig, fConfigNew));
     689
     690                    if ((fConfigNew & HPET_TN_32BIT) == (fConfig & HPET_TN_32BIT))
     691                    { /* likely it stays the same */ }
     692                    else if (fConfigNew & HPET_TN_32BIT)
     693                    {
     694                        Log(("HPET[%u]: Changing timer to 32-bit mode.\n", iTimerNo));
     695                        ASMAtomicUoWriteU64(&pHpetTimer->u64Cmp,    (uint32_t)pHpetTimer->u64Cmp);
     696                        ASMAtomicUoWriteU64(&pHpetTimer->u64Period, (uint32_t)pHpetTimer->u64Period);
     697                    }
     698                    else
     699                        Log(("HPET[%u]: Changing timer to 64-bit mode.\n", iTimerNo));
     700                    ASMAtomicWriteU64(&pHpetTimer->u64Config, fConfigNew);
     701
     702                    DEVHPET_UNLOCK(pDevIns, pThis);
     703
     704                    if (RT_LIKELY((fConfigNew & HPET_TN_INT_TYPE) != HPET_TIMER_TYPE_LEVEL))
     705                    { /* likely */ }
     706                    else
     707                    {
     708                        LogRelMax(10, ("HPET[%u]: Level-triggered config not yet supported\n", iTimerNo));
     709                        AssertFailed();
     710                    }
     711                }
     712                break;
    603713            }
    604             if ((u32NewValue & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_LEVEL)
     714
     715            case HPET_TN_CFG + 4: /* Interrupt capabilities - read only. */
     716                Log(("HPET[%u]: write32 HPET_TN_CFG + 4 (ignored)\n", iTimerNo));
     717                break;
     718
     719            case HPET_TN_CMP: /* lower bits of comparator register */
    605720            {
    606                 LogRelMax(10, ("HPET: Level-triggered config not yet supported\n"));
    607                 AssertFailed();
     721                DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
     722                Log(("HPET[%u]: write32 HPET_TN_CMP: %#x\n", iTimerNo, u32NewValue));
     723                uint64_t fConfig = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
     724
     725/** @todo r=bird: This is wrong.  Specification says: "By writing this bit to a 1, the software is then allowed to directly set a periodic timer’s accumulator."
     726 * See https://wiki.osdev.org/HPET and hpet_clkevt_set_state_periodic() in linux. */
     727                if (fConfig & HPET_TN_PERIODIC)
     728                    ASMAtomicUoWriteU64(&pHpetTimer->u64Period, RT_MAKE_U64(u32NewValue, RT_HI_U32(pHpetTimer->u64Period)));
     729                ASMAtomicUoWriteU64(&pHpetTimer->u64Cmp, RT_MAKE_U64(u32NewValue, RT_HI_U32(pHpetTimer->u64Cmp)));
     730                ASMAtomicAndU64(&pHpetTimer->u64Config, ~HPET_TN_SETVAL);
     731                Log2(("HPET[%u]: after32 HPET_TN_CMP cmp=%#llx per=%#llx\n", iTimerNo, pHpetTimer->u64Cmp, pHpetTimer->u64Period));
     732
     733                if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
     734                    hpetProgramTimer(pDevIns, pThis, pHpetTimer, PDMDevHlpTimerGet(pDevIns, pHpetTimer->hTimer));
     735                DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     736                break;
    608737            }
    609738
    610             /* We only care about lower 32-bits so far */
    611             pHpetTimer->u64Config = hpetUpdateMasked(u32NewValue, pHpetTimer->u64Config, u64Mask);
    612             DEVHPET_UNLOCK(pDevIns, pThis);
    613             break;
    614         }
    615 
    616         case HPET_TN_CFG + 4: /* Interrupt capabilities - read only. */
    617             Log(("write HPET_TN_CFG + 4, useless\n"));
    618             break;
    619 
    620         case HPET_TN_CMP: /* lower bits of comparator register */
    621         {
    622             DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    623             Log(("write HPET_TN_CMP on %d: %#x\n", iTimerNo, u32NewValue));
    624 
    625             if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    626                 pHpetTimer->u64Period = RT_MAKE_U64(u32NewValue, RT_HI_U32(pHpetTimer->u64Period));
    627             pHpetTimer->u64Cmp     = RT_MAKE_U64(u32NewValue, RT_HI_U32(pHpetTimer->u64Cmp));
    628             pHpetTimer->u64Config &= ~HPET_TN_SETVAL;
    629             Log2(("after HPET_TN_CMP cmp=%#llx per=%#llx\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period));
    630 
    631             if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    632                 hpetProgramTimer(pDevIns, pThis, pHpetTimer);
    633             DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
    634             break;
    635         }
    636 
    637         case HPET_TN_CMP + 4: /* upper bits of comparator register */
    638         {
    639             DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    640             Log(("write HPET_TN_CMP + 4 on %d: %#x\n", iTimerNo, u32NewValue));
    641             if (!hpet32bitTimer(pHpetTimer))
     739            case HPET_TN_CMP + 4: /* upper bits of comparator register */
    642740            {
    643                 if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    644                     pHpetTimer->u64Period = RT_MAKE_U64(RT_LO_U32(pHpetTimer->u64Period), u32NewValue);
    645                 pHpetTimer->u64Cmp = RT_MAKE_U64(RT_LO_U32(pHpetTimer->u64Cmp), u32NewValue);
    646 
    647                 Log2(("after HPET_TN_CMP+4 cmp=%llx per=%llx tmr=%d\n", pHpetTimer->u64Cmp, pHpetTimer->u64Period, iTimerNo));
    648 
    649                 pHpetTimer->u64Config &= ~HPET_TN_SETVAL;
     741                DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
     742                uint64_t fConfig = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
     743
     744                if (!hpet32bitTimerEx(fConfig))
     745                {
     746                    Log(("HPET[%u]: write32 HPET_TN_CMP + 4: %#x\n", iTimerNo, u32NewValue));
     747/** @todo r=bird: This is wrong.  Specification says: "By writing this bit to a 1, the software is then allowed to directly set a periodic timer’s accumulator."
     748 * See https://wiki.osdev.org/HPET and hpet_clkevt_set_state_periodic() in linux. */
     749                    if (fConfig & HPET_TN_PERIODIC)
     750                        ASMAtomicUoWriteU64(&pHpetTimer->u64Period, RT_MAKE_U64(RT_LO_U32(pHpetTimer->u64Period), u32NewValue));
     751                    ASMAtomicUoWriteU64(&pHpetTimer->u64Cmp, RT_MAKE_U64(RT_LO_U32(pHpetTimer->u64Cmp), u32NewValue));
     752                    ASMAtomicAndU64(&pHpetTimer->u64Config, ~HPET_TN_SETVAL);
     753                    Log2(("HPET[%u]: after32 HPET_TN_CMP+4: cmp=%#llx per=%#llx\n", iTimerNo, pHpetTimer->u64Cmp, pHpetTimer->u64Period));
     754
     755                    if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
     756                        hpetProgramTimer(pDevIns, pThis, pHpetTimer, PDMDevHlpTimerGet(pDevIns, pHpetTimer->hTimer));
     757                }
     758                else
     759                    Log(("HPET[%u]: write32 HPET_TN_CMP + 4: %#x - but timer is 32-bit!!\n", iTimerNo, u32NewValue));
     760                DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     761                break;
     762            }
     763
     764            case HPET_TN_ROUTE:
     765                Log(("HPET[%u]: write32 HPET_TN_ROUTE (ignored)\n", iTimerNo));
     766                break;
     767
     768            case HPET_TN_ROUTE + 4:
     769                Log(("HPET[%u]: write32 HPET_TN_ROUTE + 4 (ignored)\n", iTimerNo));
     770                break;
     771
     772            default:
     773                LogRelMax(10, ("HPET[%u]: Invalid timer register write: %d\n", iTimerNo, iTimerReg));
     774                break;
     775        }
     776    }
     777    else
     778        LogRelMax(10, ("HPET: Using timer above configured range: %d (reg %#x)\n", iTimerNo, iTimerReg));
     779    return VINF_SUCCESS;
     780}
     781
     782
     783/**
     784 * 32-bit write to a HPET timer register.
     785 *
     786 * @returns Strict VBox status code.
     787 *
     788 * @param   pDevIns         The device instance.
     789 * @param   pThis           The shared HPET state.
     790 * @param   iTimerNo        The timer being written to.
     791 * @param   iTimerReg       The register being written to.
     792 * @param   u32NewValue     The value being written.
     793 *
     794 * @remarks The caller should not hold any locks.
     795 */
     796static VBOXSTRICTRC hpetTimerRegWrite64(PPDMDEVINS pDevIns, PHPET pThis, uint32_t iTimerNo,
     797                                        uint32_t iTimerReg, uint64_t u64NewValue)
     798{
     799    Assert(!PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect));
     800    Assert(!PDMDevHlpTimerIsLockOwner(pDevIns, pThis->aTimers[0].hTimer));
     801    Assert(!(iTimerReg & 7));
     802
     803    if (   iTimerNo < HPET_CAP_GET_TIMERS(pThis->u32Capabilities)
     804        && iTimerNo < RT_ELEMENTS(pThis->aTimers) )    /* Parfait - see above. */
     805    {
     806        PHPETTIMER pHpetTimer = &pThis->aTimers[iTimerNo];
     807
     808        switch (iTimerReg)
     809        {
     810            case HPET_TN_CFG:
     811                /* The upper 32 bits are not writable, so join paths with the 32-bit version. */
     812                return hpetTimerRegWrite32(pDevIns, pThis, iTimerNo, iTimerReg, (uint32_t)u64NewValue);
     813
     814            case HPET_TN_CMP:
     815            {
     816                DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
     817                Log(("HPET[%u]: write64 HPET_TN_CMP: %#RX64\n", iTimerNo, u64NewValue));
     818                uint64_t fConfig = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
     819
     820/** @todo r=bird: This is wrong.  Specification says: "By writing this bit to a 1, the software is then allowed to directly set a periodic timer’s accumulator."
     821 * See https://wiki.osdev.org/HPET and hpet_clkevt_set_state_periodic() in linux. */
     822                if (fConfig & HPET_TN_PERIODIC)
     823                    ASMAtomicUoWriteU64(&pHpetTimer->u64Period, u64NewValue);
     824                ASMAtomicUoWriteU64(&pHpetTimer->u64Cmp, u64NewValue);
     825                ASMAtomicAndU64(&pHpetTimer->u64Config, ~HPET_TN_SETVAL);
     826                Log2(("HPET[%u]: after64 HPET_TN_CMP cmp=%#llx per=%#llx\n", iTimerNo, pHpetTimer->u64Cmp, pHpetTimer->u64Period));
    650827
    651828                if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    652                     hpetProgramTimer(pDevIns, pThis, pHpetTimer);
     829                    hpetProgramTimer(pDevIns, pThis, pHpetTimer, PDMDevHlpTimerGet(pDevIns, pHpetTimer->hTimer));
     830                DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     831                break;
    653832            }
    654             DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
    655             break;
    656         }
    657 
    658         case HPET_TN_ROUTE:
    659             Log(("write HPET_TN_ROUTE\n"));
    660             break;
    661 
    662         case HPET_TN_ROUTE + 4:
    663             Log(("write HPET_TN_ROUTE + 4\n"));
    664             break;
    665 
    666         default:
    667             LogRelMax(10, ("HPET: Invalid timer register write: %d\n", iTimerReg));
    668             break;
    669     }
    670 
     833
     834            case HPET_TN_ROUTE:
     835                Log(("HPET[%u]: write64 HPET_TN_ROUTE (ignored)\n", iTimerNo));
     836                break;
     837
     838            default:
     839                LogRelMax(10, ("HPET[%u]: Invalid timer register write: %d\n", iTimerNo, iTimerReg));
     840                break;
     841        }
     842    }
     843    else
     844        LogRelMax(10, ("HPET: Using timer above configured range: %d (reg %#x)\n", iTimerNo, iTimerReg));
    671845    return VINF_SUCCESS;
    672846}
    … …  
    725899        case HPET_COUNTER + 4:
    726900        {
    727             STAM_REL_COUNTER_INC(&pThis->StatCounterRead4Byte);
     901            /** @todo We don't technically need to sit on the virtualsync lock here to
     902             *        read it, but it helps wrt quality... */
    728903            DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
    729904
    730905            uint64_t u64Ticks;
    731906            if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    732                 u64Ticks = hpetGetTicks(pDevIns, pThis);
     907            {
     908                uint64_t const tsNow = PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer);
     909                PDMDevHlpTimerUnlockClock(pDevIns, pThis->aTimers[0].hTimer);
     910                u64Ticks = hpetGetTicksEx(pThis, tsNow);
     911            }
    733912            else
     913            {
     914                PDMDevHlpTimerUnlockClock(pDevIns, pThis->aTimers[0].hTimer);
    734915                u64Ticks = pThis->u64HpetCounter;
    735 
    736             DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     916            }
     917
     918            STAM_REL_COUNTER_INC(&pThis->StatCounterRead4Byte);
     919            DEVHPET_UNLOCK(pDevIns, pThis);
    737920
    738921            /** @todo is it correct? */
    739             u32Value = (idxReg == HPET_COUNTER) ? (uint32_t)u64Ticks : (uint32_t)(u64Ticks >> 32);
     922            u32Value = idxReg == HPET_COUNTER ? (uint32_t)u64Ticks : (uint32_t)(u64Ticks >> 32);
    740923            Log(("read HPET_COUNTER: %s part value %x (%#llx)\n", (idxReg == HPET_COUNTER) ? "low" : "high", u32Value, u64Ticks));
    741924            break;
    … …  
    8171000            }
    8181001
    819             pThis->u64HpetConfig = hpetUpdateMasked(u32NewValue, iOldValue, HPET_CFG_WRITE_MASK);
    820 
    821             uint32_t const cTimers = HPET_CAP_GET_TIMERS(pThis->u32Capabilities);
     1002            /* Updating it using an atomic write just to be on the safe side. */
     1003            ASMAtomicWriteU64(&pThis->u64HpetConfig, hpetUpdateMasked(u32NewValue, iOldValue, HPET_CFG_WRITE_MASK));
     1004
     1005            uint32_t const cTimers = RT_MIN(HPET_CAP_GET_TIMERS(pThis->u32Capabilities), RT_ELEMENTS(pThis->aTimers));
    8221006            if (hpetBitJustSet(iOldValue, u32NewValue, HPET_CFG_ENABLE))
    8231007            {
    824 /** @todo Only get the time stamp once when reprogramming? */
    825                 /* Enable main counter and interrupt generation. */
     1008                /*
     1009                 * Enable main counter and interrupt generation.
     1010                 */
    8261011                uint64_t u64TickLimit = pThis->fIch9 ? HPET_TICKS_IN_100YR_ICH9 : HPET_TICKS_IN_100YR_PIIX;
    8271012                if (pThis->u64HpetCounter <= u64TickLimit)
    828                 {
    829                     pThis->u64HpetOffset = hpetTicksToNs(pThis, pThis->u64HpetCounter)
    830                                          - PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer);
    831                 }
     1013                    pThis->u64HpetOffset = hpetTicksToNs(pThis, pThis->u64HpetCounter);
    8321014                else
    8331015                {
    8341016                    LogRelMax(10, ("HPET: Counter set more than 100 years in the future, reducing.\n"));
    835                     pThis->u64HpetOffset = 1000000LL * 60 * 60 * 24 * 365 * 100
    836                                          - PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer);
     1017                    pThis->u64HpetOffset = 1000000LL * 60 * 60 * 24 * 365 * 100;
    8371018                }
     1019
     1020                uint64_t const tsNow = PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer);
     1021                pThis->u64HpetOffset -= tsNow;
     1022
    8381023                for (uint32_t i = 0; i < cTimers; i++)
    8391024                    if (pThis->aTimers[i].u64Cmp != hpetInvalidValue(&pThis->aTimers[i]))
    840                         hpetProgramTimer(pDevIns, pThis, &pThis->aTimers[i]);
     1025                        hpetProgramTimer(pDevIns, pThis, &pThis->aTimers[i], tsNow);
    8411026            }
    8421027            else if (hpetBitJustCleared(iOldValue, u32NewValue, HPET_CFG_ENABLE))
    8431028            {
    844                 /* Halt main counter and disable interrupt generation. */
    845                 pThis->u64HpetCounter = hpetGetTicks(pDevIns, pThis);
     1029                /*
     1030                 * Halt main counter and disable interrupt generation.
     1031                 */
     1032                pThis->u64HpetCounter = hpetGetTicksEx(pThis, PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer));
    8461033                for (uint32_t i = 0; i < cTimers; i++)
    8471034                    PDMDevHlpTimerStop(pDevIns, pThis->aTimers[i].hTimer);
    … …  
    8541041        case HPET_CFG + 4:
    8551042        {
     1043/** @todo r=bird: Is the whole upper part of the config register really
     1044 * writable?  Only 2 bits are writable in the lower part... */
    8561045            DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    8571046            pThis->u64HpetConfig = hpetUpdateMasked((uint64_t)u32NewValue << 32,
    … …  
    8861075            DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    8871076            pThis->u64HpetCounter = RT_MAKE_U64(u32NewValue, RT_HI_U32(pThis->u64HpetCounter));
     1077/** @todo how is this supposed to work if the HPET is enabled? */
    8881078            Log(("write HPET_COUNTER: %#x -> %llx\n", u32NewValue, pThis->u64HpetCounter));
    8891079            DEVHPET_UNLOCK(pDevIns, pThis);
    … …  
    9211111    NOREF(pvUser);
    9221112    Assert(cb == 4 || cb == 8);
     1113    Assert(!(off & (cb - 1)));
    9231114
    9241115    LogFlow(("hpetMMIORead (%d): %RGp\n", cb, off));
    … …  
    9321123        if (off >= 0x100 && off < 0x400)
    9331124        {
    934             DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
    935             hpetTimerRegRead32(pDevIns, pThis,
    936                                (uint32_t)(off - 0x100) / 0x20,
    937                                (uint32_t)(off - 0x100) % 0x20,
    938                                (uint32_t *)pv);
    939             DEVHPET_UNLOCK(pDevIns, pThis);
     1125            *(uint32_t *)pv = hpetTimerRegRead32(pThis,
     1126                                                 (uint32_t)(off - 0x100) / 0x20,
     1127                                                 (uint32_t)(off - 0x100) % 0x20);
    9401128            rc = VINF_SUCCESS;
    9411129        }
    … …  
    9521140        if (off == HPET_COUNTER)
    9531141        {
     1142            /** @todo We don't technically need to sit on the virtualsync lock here to
     1143             *        read it, but it helps wrt quality... */
    9541144            /* When reading HPET counter we must read it in a single read,
    9551145               to avoid unexpected time jumps on 32-bit overflow. */
     1146            DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
     1147
     1148            if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
     1149            {
     1150                uint64_t const tsNow = PDMDevHlpTimerGet(pDevIns, pThis->aTimers[0].hTimer);
     1151                PDMDevHlpTimerUnlockClock(pDevIns, pThis->aTimers[0].hTimer);
     1152                pValue->u = hpetGetTicksEx(pThis, tsNow);
     1153            }
     1154            else
     1155            {
     1156                PDMDevHlpTimerUnlockClock(pDevIns, pThis->aTimers[0].hTimer);
     1157                pValue->u = pThis->u64HpetCounter;
     1158            }
     1159
    9561160            STAM_REL_COUNTER_INC(&pThis->StatCounterRead8Byte);
    957             DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
    958             if (pThis->u64HpetConfig & HPET_CFG_ENABLE)
    959                 pValue->u = hpetGetTicks(pDevIns, pThis);
    960             else
    961                 pValue->u = pThis->u64HpetCounter;
    962             DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     1161            DEVHPET_UNLOCK(pDevIns, pThis);
    9631162            rc = VINF_SUCCESS;
    9641163        }
    9651164        else
    9661165        {
    967             DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
    9681166            if (off >= 0x100 && off < 0x400)
    9691167            {
    9701168                uint32_t iTimer    = (uint32_t)(off - 0x100) / 0x20;
    9711169                uint32_t iTimerReg = (uint32_t)(off - 0x100) % 0x20;
    972                 hpetTimerRegRead32(pDevIns, pThis, iTimer, iTimerReg, &pValue->s.Lo);
    973                 hpetTimerRegRead32(pDevIns, pThis, iTimer, iTimerReg + 4, &pValue->s.Hi);
     1170                Assert(!(iTimerReg & 7));
     1171                pValue->u = hpetTimerRegRead64(pThis, iTimer, iTimerReg);
    9741172                rc = VINF_SUCCESS;
    9751173            }
    … …  
    9771175            {
    9781176                /* for most 8-byte accesses we just split them, happens under lock anyway. */
     1177                DEVHPET_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ);
    9791178                rc = hpetConfigRegRead32(pDevIns, pThis, off, &pValue->s.Lo);
    9801179                if (rc == VINF_SUCCESS)
    9811180                    rc = hpetConfigRegRead32(pDevIns, pThis, off + 4, &pValue->s.Hi);
     1181                DEVHPET_UNLOCK(pDevIns, pThis);
    9821182            }
    983             DEVHPET_UNLOCK(pDevIns, pThis);
    9841183        }
    9851184    }
    … …  
    9981197    NOREF(pvUser);
    9991198    Assert(cb == 4 || cb == 8);
     1199    Assert(!(off & (cb - 1)));
    10001200
    10011201    VBOXSTRICTRC rc;
    … …  
    10151215         * 8-byte access.
    10161216         */
    1017         /* Split the access and rely on the locking to prevent trouble. */
    1018         DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    1019         RTUINT64U uValue;
    1020         uValue.u = *(uint64_t const *)pv;
    10211217        if (off >= 0x100 && off < 0x400)
    1022         {
    1023             uint32_t iTimer    = (uint32_t)(off - 0x100) / 0x20;
    1024             uint32_t iTimerReg = (uint32_t)(off - 0x100) % 0x20;
    1025     /** @todo Consider handling iTimerReg == HPET_TN_CMP specially here */
    1026             rc = hpetTimerRegWrite32(pDevIns, pThis, iTimer, iTimerReg, uValue.s.Lo);
    1027             if (RT_LIKELY(rc == VINF_SUCCESS))
    1028                 rc = hpetTimerRegWrite32(pDevIns, pThis, iTimer, iTimerReg + 4, uValue.s.Hi);
    1029         }
     1218            rc = hpetTimerRegWrite64(pDevIns, pThis,
     1219                                     (uint32_t)(off - 0x100) / 0x20,
     1220                                     (uint32_t)(off - 0x100) % 0x20,
     1221                                     *(uint64_t const *)pv);
    10301222        else
    10311223        {
     1224            /* Split the access and rely on the locking to prevent trouble. */
     1225            RTUINT64U uValue;
     1226            uValue.u = *(uint64_t const *)pv;
     1227            DEVHPET_LOCK_BOTH_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
    10321228            rc = hpetConfigRegWrite32(pDevIns, pThis, off, uValue.s.Lo);
    10331229            if (RT_LIKELY(rc == VINF_SUCCESS))
    10341230                rc = hpetConfigRegWrite32(pDevIns, pThis, off + 4, uValue.s.Hi);
    1035         }
    1036         DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     1231            DEVHPET_UNLOCK_BOTH(pDevIns, pThis);
     1232        }
    10371233    }
    10381234
    … …  
    10501246 * @param   pThis               The shared HPET state.
    10511247 * @param   pHpetTimer          The HPET timer.
    1052  */
    1053 static uint32_t hpetR3TimerGetIrq(PHPET pThis, PCHPETTIMER pHpetTimer)
     1248 * @param   fConfig             The HPET timer config value.
     1249 */
     1250DECLINLINE(uint32_t) hpetR3TimerGetIrq(PHPET pThis, PCHPETTIMER pHpetTimer, uint64_t fConfig)
    10541251{
    10551252    /*
    … …  
    10611258     * to the different ICs.
    10621259     */
    1063     if (   (pHpetTimer->idxTimer <= 1)
     1260    if (   pHpetTimer->idxTimer <= 1
    10641261        && (pThis->u64HpetConfig & HPET_CFG_LEGACY))
    1065         return (pHpetTimer->idxTimer == 0) ? 0 : 8;
    1066 
    1067     return (pHpetTimer->u64Config & HPET_TN_INT_ROUTE_MASK) >> HPET_TN_INT_ROUTE_SHIFT;
    1068 }
    1069 
    1070 
    1071 /**
    1072  * Used by hpetR3Timer to update the IRQ status.
    1073  *
    1074  * @param   pDevIns             The device instance.
    1075  * @param   pThis               The shared HPET state.
    1076  * @param   pHpetTimer          The HPET timer.
    1077  */
    1078 static void hpetR3TimerUpdateIrq(PPDMDEVINS pDevIns, PHPET pThis, PHPETTIMER pHpetTimer)
    1079 {
    1080     /** @todo is it correct? */
    1081     if (   !!(pHpetTimer->u64Config & HPET_TN_ENABLE)
    1082         && !!(pThis->u64HpetConfig & HPET_CFG_ENABLE))
    1083     {
    1084         uint32_t irq = hpetR3TimerGetIrq(pThis, pHpetTimer);
    1085         Log4(("HPET: raising IRQ %d\n", irq));
    1086 
    1087         /* ISR bits are only set in level-triggered mode. */
    1088         if ((pHpetTimer->u64Config & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_LEVEL)
    1089             pThis->u64Isr |= UINT64_C(1) << pHpetTimer->idxTimer;
    1090 
    1091         /* We trigger flip/flop in edge-triggered mode and do nothing in
    1092            level-triggered mode yet. */
    1093         if ((pHpetTimer->u64Config & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_EDGE)
    1094         {
    1095             PHPETCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PHPETCC);
    1096             AssertReturnVoid(pThisCC);
    1097             pThisCC->pHpetHlp->pfnSetIrq(pDevIns, irq, PDM_IRQ_LEVEL_FLIP_FLOP);
    1098             STAM_REL_COUNTER_INC(&pHpetTimer->StatSetIrq);
    1099         }
    1100         else
    1101             AssertFailed();
    1102         /** @todo implement IRQs in level-triggered mode */
    1103     }
     1262        return pHpetTimer->idxTimer == 0 ? 0 : 8;
     1263
     1264    return (fConfig & HPET_TN_INT_ROUTE_MASK) >> HPET_TN_INT_ROUTE_SHIFT;
    11041265}
    11051266
    … …  
    11071268/**
    11081269 * @callback_method_impl{FNTMTIMERDEV, Device timer callback function.}
     1270 *
     1271 * @note    Only the virtual sync lock is held when called.
    11091272 */
    11101273static DECLCALLBACK(void) hpetR3Timer(PPDMDEVINS pDevIns, TMTIMERHANDLE hTimer, void *pvUser)
    11111274{
    1112     PHPET       pThis      = PDMDEVINS_2_DATA(pDevIns, PHPET);
    1113     PHPETTIMER  pHpetTimer = (HPETTIMER *)pvUser;
    1114     uint64_t    u64Period  = pHpetTimer->u64Period;
    1115     uint64_t    u64CurTick = hpetGetTicks(pDevIns, pThis);
    1116     uint64_t    u64Diff;
     1275    PHPET           pThis      = PDMDEVINS_2_DATA(pDevIns, PHPET);
     1276    PHPETTIMER      pHpetTimer = (HPETTIMER *)pvUser;
     1277
     1278    /*
     1279     * Read the timer configuration values we need first.
     1280     *
     1281     * The comparator and period are only written while owning the virtual sync
     1282     * lock, so we don't run any risk there.  The configuration register is
     1283     * written with only the device lock, so must be a bit more careful with it.
     1284     */
     1285    uint64_t        uCmp       = ASMAtomicUoReadU64(&pHpetTimer->u64Cmp);
     1286    uint64_t const  uPeriod    = ASMAtomicUoReadU64(&pHpetTimer->u64Period);
     1287    uint64_t const  fConfig    = ASMAtomicUoReadU64(&pHpetTimer->u64Config);
    11171288    Assert(hTimer == pHpetTimer->hTimer);
    11181289
    1119     if (pHpetTimer->u64Config & HPET_TN_PERIODIC)
    1120     {
    1121         if (u64Period)
    1122         {
    1123             hpetAdjustComparator(pHpetTimer, u64CurTick);
    1124 
    1125             u64Diff = hpetComputeDiff(pHpetTimer, u64CurTick);
    1126 
    1127             uint64_t u64TickLimit = pThis->fIch9 ? HPET_TICKS_IN_100YR_ICH9 : HPET_TICKS_IN_100YR_PIIX;
    1128             if (u64Diff <= u64TickLimit)
     1290    if (fConfig & HPET_TN_PERIODIC)
     1291    {
     1292        if (uPeriod)
     1293        {
     1294            uint64_t const tsNow        = PDMDevHlpTimerGet(pDevIns, pHpetTimer->hTimer);
     1295            uint64_t const uHpetNow     = hpetGetTicksEx(pThis, tsNow);
     1296            uCmp = hpetAdjustComparator(pHpetTimer, fConfig, uCmp, uPeriod, uHpetNow);
     1297            uint64_t const cTicksDiff   = hpetComputeDiff(fConfig, uCmp, uHpetNow);
     1298            uint64_t const u64TickLimit = pThis->fIch9 ? HPET_TICKS_IN_100YR_ICH9 : HPET_TICKS_IN_100YR_PIIX;
     1299            if (cTicksDiff <= u64TickLimit)
    11291300            {
    1130                 Log4(("HPET: periodic: next in %llu\n", hpetTicksToNs(pThis, u64Diff)));
     1301                uint64_t const tsDeadline = tsNow + hpetTicksToNs(pThis, cTicksDiff);
     1302                Log4(("HPET: periodic: next in %llu\n", tsDeadline));
     1303                PDMDevHlpTimerSet(pDevIns, hTimer, tsDeadline);
    11311304                STAM_REL_COUNTER_INC(&pHpetTimer->StatSetTimer);
    1132                 PDMDevHlpTimerSetNano(pDevIns, hTimer, hpetTicksToNs(pThis, u64Diff));
    11331305            }
    11341306            else
    … …  
    11381310        }
    11391311    }
    1140     else if (hpet32bitTimer(pHpetTimer))
    1141     {
    1142         /* For 32-bit non-periodic timers, generate wrap-around interrupts. */
    1143         if (pHpetTimer->u8Wrap)
    1144         {
    1145             u64Diff = hpetComputeDiff(pHpetTimer, u64CurTick);
    1146             PDMDevHlpTimerSetNano(pDevIns, hTimer, hpetTicksToNs(pThis, u64Diff));
    1147             pHpetTimer->u8Wrap = 0;
    1148         }
    1149     }
    1150 
    1151     /* Should it really be under lock, does it really matter? */
    1152     hpetR3TimerUpdateIrq(pDevIns, pThis, pHpetTimer);
     1312    /* For 32-bit non-periodic timers, generate wrap-around interrupts. */
     1313    else if (pHpetTimer->u8Wrap && hpet32bitTimerEx(fConfig))
     1314    {
     1315        pHpetTimer->u8Wrap = 0;         /* (only modified while owning the virtual sync lock) */
     1316        uint64_t const tsNow      = PDMDevHlpTimerGet(pDevIns, hTimer);
     1317        uint64_t const uHpetNow   = nsToHpetTicks(pThis, tsNow + pThis->u64HpetOffset);
     1318        uint64_t const cTicksDiff = hpetComputeDiff(fConfig, uCmp, uHpetNow);
     1319        uint64_t const tsDeadline = tsNow + hpetTicksToNs(pThis, cTicksDiff);
     1320        Log4(("HPET: post-wrap deadline: %llu\n", tsDeadline));
     1321        PDMDevHlpTimerSet(pDevIns, pHpetTimer->hTimer, tsDeadline);
     1322    }
     1323
     1324    /*
     1325     * IRQ update.
     1326     */
     1327    if (   (fConfig & HPET_TN_ENABLE)
     1328        && (pThis->u64HpetConfig & HPET_CFG_ENABLE))
     1329    {
     1330        AssertCompile(HPET_TN_INT_TYPE == 2);
     1331
     1332        /* We trigger flip/flop in edge-triggered mode and do nothing in
     1333           level-triggered mode yet. */
     1334        if ((fConfig & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_EDGE)
     1335        {
     1336            PHPETCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PHPETCC);
     1337            AssertReturnVoid(pThisCC);
     1338
     1339            uint32_t const uIrq = hpetR3TimerGetIrq(pThis, pHpetTimer, fConfig);
     1340            Log4(("HPET: raising IRQ %u\n", uIrq));
     1341
     1342            pThisCC->pHpetHlp->pfnSetIrq(pDevIns, uIrq, PDM_IRQ_LEVEL_FLIP_FLOP);
     1343            STAM_REL_COUNTER_INC(&pHpetTimer->StatSetIrq);
     1344        }
     1345        /* ISR bits are only set in level-triggered mode. */
     1346        else
     1347        {
     1348            Assert((fConfig & HPET_TN_INT_TYPE) == HPET_TIMER_TYPE_LEVEL);
     1349            ASMAtomicOrU64(&pThis->u64Isr, RT_BIT_64(pHpetTimer->idxTimer));
     1350            /** @todo implement IRQs in level-triggered mode */
     1351        }
     1352    }
     1353
    11531354}
    11541355
    … …  
    13211522        PHPETTIMER pHpetTimer = &pThis->aTimers[iTimer];
    13221523        if (PDMDevHlpTimerIsActive(pDevIns, pHpetTimer->hTimer))
    1323             hpetTimerSetFrequencyHint(pDevIns, pThis, pHpetTimer);
     1524            hpetTimerSetFrequencyHint(pDevIns, pThis, pHpetTimer, pHpetTimer->u64Config, pHpetTimer->u64Period);
    13241525    }
    13251526    PDMDevHlpCritSectLeave(pDevIns, &pThis->CritSect);
    … …  
    13631564
    13641565        /* capable of periodic operations and 64-bits */
     1566        uint64_t fConfig;
    13651567        if (pThis->fIch9)
    1366             pHpetTimer->u64Config = (i == 0)
    1367                                   ? (HPET_TN_PERIODIC_CAP | HPET_TN_SIZE_CAP)
    1368                                   : 0;
     1568            fConfig = i == 0 ? HPET_TN_PERIODIC_CAP | HPET_TN_SIZE_CAP : 0;
    13691569        else
    1370             pHpetTimer->u64Config = HPET_TN_PERIODIC_CAP | HPET_TN_SIZE_CAP;
     1570            fConfig = HPET_TN_PERIODIC_CAP | HPET_TN_SIZE_CAP;
    13711571
    13721572        /* We can do all IRQs */
    13731573        uint32_t u32RoutingCap = 0xffffffff;
    1374         pHpetTimer->u64Config |= ((uint64_t)u32RoutingCap) << HPET_TN_INT_ROUTE_CAP_SHIFT;
     1574        fConfig |= ((uint64_t)u32RoutingCap) << HPET_TN_INT_ROUTE_CAP_SHIFT;
     1575        ASMAtomicWriteU64(&pHpetTimer->u64Config, fConfig);
    13751576        pHpetTimer->u64Period  = 0;
    13761577        pHpetTimer->u8Wrap     = 0;
    … …  
    14601661    {
    14611662        PHPETTIMER pHpetTimer = &pThis->aTimers[i];
    1462 
    14631663        rc = PDMDevHlpTimerCreate(pDevIns, TMCLOCK_VIRTUAL_SYNC, hpetR3Timer, pHpetTimer,
    14641664                                  TMTIMER_FLAGS_NO_CRIT_SECT | TMTIMER_FLAGS_RING0,
    14651665                                  s_apszTimerNames[i], &pThis->aTimers[i].hTimer);
    14661666        AssertRCReturn(rc, rc);
    1467         /** @todo r=bird: This is TOTALLY MESSED UP!  Why do we need
    1468          *        DEVHPET_LOCK_BOTH_RETURN() when the timers use the same critsect as
    1469          *        we do?!? */
    1470         rc = PDMDevHlpTimerSetCritSect(pDevIns, pThis->aTimers[i].hTimer, &pThis->CritSect);
    1471         AssertRCReturn(rc, rc);
     1667        uint64_t const cTicksPerSec = PDMDevHlpTimerGetFreq(pDevIns, pThis->aTimers[i].hTimer);
     1668        if (cTicksPerSec != RT_NS_1SEC)
     1669            return PDMDevHlpVMSetError(pDevIns, VERR_INTERNAL_ERROR_2, RT_SRC_POS,
     1670                                       "Unexpected timer resolution %RU64, code assumes nanonsecond resolution!", cTicksPerSec);
    14721671    }
    14731672
    … …  
    15091708    /* Statistics: */
    15101709    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatCounterRead4Byte, STAMTYPE_COUNTER,
    1511                           "CounterRead4Byte", STAMUNIT_OCCURENCES, "HPET_COUNTER 32-bit reads");
     1710                          "ReadCounter32bit", STAMUNIT_OCCURENCES, "HPET_COUNTER 32-bit reads");
    15121711    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatCounterRead8Byte, STAMTYPE_COUNTER,
    1513                           "CounterRead8Byte", STAMUNIT_OCCURENCES, "HPET_COUNTER 64-bit reads");
     1712                          "ReadCounter64bit", STAMUNIT_OCCURENCES, "HPET_COUNTER 64-bit reads");
    15141713    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatCounterWriteLow,  STAMTYPE_COUNTER,
    1515                           "CounterWriteLow",  STAMUNIT_OCCURENCES, "Low HPET_COUNTER writes");
     1714                          "WriteCounterLow",  STAMUNIT_OCCURENCES, "Low HPET_COUNTER writes");
    15161715    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatCounterWriteHigh, STAMTYPE_COUNTER,
    1517                           "CounterWriteHigh", STAMUNIT_OCCURENCES, "High HPET_COUNTER writes");
     1716                          "WriteCounterHigh", STAMUNIT_OCCURENCES, "High HPET_COUNTER writes");
     1717    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatZeroDeltaHack, STAMTYPE_COUNTER,
     1718                          "ZeroDeltaHacks", STAMUNIT_OCCURENCES, "High HPET_COUNTER writes");
     1719
    15181720    for (unsigned i = 0; i < RT_ELEMENTS(pThis->aTimers); i++)
    15191721    {
Note: See TracChangeset for help on using the changeset viewer.

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