nv2a: Implement PTIMER alarm interrupts #2568

abaire · 2025-11-26T23:40:07Z

Looks like the PTIMER alarm handling was started but never finished. This implements my best guess at what the HW is doing (it's hard to test with a nanosecond timer). It fixes the blocked loading for Def Jam: Fight for NY, which I determined to be caused by a event handle never being set due to the lack of an alarm interrupt in the same DPC that handles video blanking and other PMC interrupts. The PTIMER pending flag is known from the nxdk:
https://github.com/XboxDev/nxdk/blob/d0ae96acc926a6c3f6254b289a1a9a3c1db182f1/lib/pbkit/outer.h#L83

PTIMER tests

Fixes #1124

Triticum0 · 2025-11-27T00:40:13Z

I tested only other game that freezes during intro and is unrelated issue. NHL Rivals 2004 Note: this game goes in-game on cxbx-reloaded. Also tested NFL Fever 2004 no change.

PatrickvL

This looks like a bug

PatrickvL · 2025-11-27T07:50:19Z

hw/xbox/nv2a/ptimer.c

+    uint32_t current_time_low = (guest_clock & CLOCK_LOW_MASK) << 5;
+    uint32_t current_time_high = (guest_clock >> 27) & CLOCK_HIGH_MASK;
+
+    if (d->ptimer.alarm_time <= current_time_low ||


This check is wrongly succeeding when current time low bits is lower or equal to alarm time (low bits) but current high bits are less than alarm high bits.

It seems wiser to reconstruct the full set of alarm bits and do a single comparison of that against the full bits of the current time.

I think it is correct due to the fact that there is no mechanism to specify the high bits of the alarm time.

If current_time_low < alarm the first condition is false so behavior is governed by whether current_time_high is > alarm (the timer has wrapped).

If current_time_low >= alarm the timer will fire (I'm assuming you meant that it wrongly succeeds if current time low bits is higher than alarm time?). If the high bits were controllable, this could indeed be erroneous.

I'm not sure what the HW behavior is if the alarm is set into the ambiguous past or if the caller is able to race the clock and set TIME_1 into the past before the alarm fires.

If the values observed being set to the ALARM_0 weren't so close to the current time I would've expected it to be a delta into the future, in which case it would be unambiguous and using 64-bit would certainly make things clearer.
The fact that it seems to really just be the low time bits makes me think that it's doing a 32-bit comparison with special affordance for the case where the value is very close to rollover (as implemented here).

I've also seen values with the high bit set, so it's not a case of using a 31-bit value to catch rollover.

I suspect this may be a case where an assumption was made that the driver writer would do the right thing and that regular users either wouldn't know the register existed or wouldn't attempt ~nonsensical things like setting a nanosecond timer into the past.

Turns out the alarm is not intended to be a one-shot (the game I was using to infer the behavior is just very spammy about updating the alarm time). I ended up writing a trivial test case by adding a counter to pbkit's DPC that is incremented on alarm interrupts and found that it fires every time the low 32 bits of the counter rolls over.

Joshua-1248 · 2025-11-30T04:49:25Z

Def Jam - Fight For NY is actually able to make it to the menus with this particular commit with some brute-force resetting whenever it freezes during the intro. And much like the Demo version of the game, it makes it in-game with no issues, save for one;

The game stutters with a 2-3 second freeze periodically throughout emulation, menus or in-game, so it's not exactly playable, but it does run full speed and can presumably be played from start to finish.

Tested with both OpenGL & Vulkan and both work, with Vulkan suffering less from the temporary freezes, though still not playable without interruptions.

mborgerson

Thanks! Some initial feedback

mborgerson · 2025-12-05T21:42:45Z

hw/xbox/nv2a/ptimer.c

+
 /* PTIMER - time measurement and time-based alarms */
-static uint64_t ptimer_get_clock(NV2AState *d)
+static uint64_t ptimer_get_host_clock(NV2AState *d)


This host/guest terminology here is a bit confusing. "Host" typically refers to the machine running xemu (and that is not the time domain source we are using--we are actually using the virtual clock here which stops when emulation is paused) and "guest" typically means the emulated machine state. Maybe host/guest → absolute/relative would be more clear in this case

Switched to absolute/relative. I think it's still unusual enough to justify a bit of documentation.

mborgerson · 2025-12-05T21:47:28Z

hw/xbox/nv2a/nv2a.c

        VMSTATE_UINT32(ptimer.numerator, NV2AState),
        VMSTATE_UINT32(ptimer.denominator, NV2AState),
        VMSTATE_UINT32(ptimer.alarm_time, NV2AState),
+        VMSTATE_UINT32(ptimer.alarm_time_high, NV2AState),


To be compatible with older vmstate versions, we should append new fields to the end

Done, also added a note to hopefully ensure future additions are also appended.

mborgerson · 2025-12-05T21:57:14Z

hw/xbox/nv2a/nv2a.c

    }

+    /* PTIMER */
+    ptimer_process_alarm(d);


Even though this function is called regularly, alarm management should be independent from this interrupt management function. We should use QEMUTimer instead for time based event signaling

For reference:

xemu/hw/xbox/mcpx/nvnet/nvnet.c

Line 669 in 9b7a43f

timer_mod(s->autoneg_timer,

Done, thanks for the pointer!

abaire · 2025-12-14T06:34:14Z

Looking at HW behavior, it seems that the alarm is not reset after firing, it's up to the application to clear it or disable the interrupt.

abaire · 2025-12-14T08:10:53Z

Updated to match observed HW behavior in a trivial test that sets the alarm to an arbitrary value and increments a counter every time the interrupt fires. Hopefully this also fixes the freezes observed in DJFfNY (I've restarted dozens of times without my tracing plugin attached now and have been unable to repro).

Triticum0 · 2025-12-15T15:54:08Z

Tested this build and freezing on intro fixed but freezing still occur when on start and first fight intro.
Only happen once and didn't occur after rebooting so not sure why it occurs.

retchless · 2025-12-22T11:30:19Z

Also tested this with Def Jam Fight for NY. Game loads now, and navigating menus seems reliable. But it freezes randomly during cut scenes (seemingly at different points, so truly random). Game is still unplayable.

mborgerson · 2026-01-15T03:53:04Z

/gemini review

gemini-code-assist

Code Review

This pull request implements PTIMER alarm interrupts, which appears to have been a partially implemented feature. The changes involve adding the necessary interrupt handling logic, state management for the timer alarm, and VM state integration. My review focuses on the correctness of the timer emulation logic. I've found a potential race condition when reading the 64-bit timer value and a critical issue that could lead to an interrupt storm. The rest of the implementation seems reasonable.

gemini-code-assist · 2026-01-15T03:58:15Z

hw/xbox/nv2a/ptimer.c

+    if (alarm_time <= now) {
+        d->ptimer.pending_interrupts |= NV_PTIMER_INTR_0_ALARM;
+        d->ptimer.alarm_time_high = now >> 32;
+        nv2a_update_irq(d);
+    }
+
+    schedule_qemu_timer(d);
 }


The current implementation of ptimer_alarm_fired will likely cause an interrupt storm. After an alarm fires and the interrupt is raised, schedule_qemu_timer(d) is called unconditionally. Since the alarm time has not been changed by the guest, schedule_qemu_timer will see that the alarm is still in the past and schedule the timer to fire again immediately. This will repeat, causing a high CPU load and flooding the guest with interrupts.

The line d->ptimer.alarm_time_high = now >> 32; does not prevent this and might be incorrect as it modifies the alarm time without guest intervention.

A safer approach is to treat the QEMU timer as a one-shot event. Once it fires, the interrupt is raised, and the timer is not rescheduled. The guest is then responsible for clearing the interrupt and programming a new alarm, which will in turn schedule a new QEMU timer via ptimer_write.

I suggest modifying the logic to only reschedule the timer in case of a spurious wakeup (where alarm_time > now).

Suggested change

if (alarm_time <= now) {

d->ptimer.pending_interrupts |= NV_PTIMER_INTR_0_ALARM;

d->ptimer.alarm_time_high = now >> 32;

nv2a_update_irq(d);

}

schedule_qemu_timer(d);

}

if (alarm_time <= now) {

d->ptimer.pending_interrupts |= NV_PTIMER_INTR_0_ALARM;

nv2a_update_irq(d);

} else {

/* Spurious wakeup, reschedule. */

schedule_qemu_timer(d);

}

gemini-code-assist · 2026-01-15T03:58:15Z

hw/xbox/nv2a/ptimer.c

+    case NV_PTIMER_TIME_0: {
+        uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));
+        r = now & 0xffffffff;
+    } break;
+    case NV_PTIMER_TIME_1: {
+        uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));
+        r = (now >> 32) & CLOCK_HIGH_MASK;
+    } break;


Reading a 64-bit timer value from two separate 32-bit register reads can be racy. If the timer ticks over and crosses a 32-bit boundary between the read of the low part (NV_PTIMER_TIME_0) and the high part (NV_PTIMER_TIME_1), the guest will get a corrupted time value. Both case statements call get_ptimer_clock independently, which can return different values.

A common hardware pattern to solve this is to latch the high part of the timer when the low part is read. The subsequent read of the high part returns the latched value.

To implement this, you could add a time_high_latched field to the ptimer struct in nv2a_int.h and initialize it in ptimer_reset. Then, modify the read logic as suggested.

Suggested change

case NV_PTIMER_TIME_0: {

uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));

r = now & 0xffffffff;

} break;

case NV_PTIMER_TIME_1: {

uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));

r = (now >> 32) & CLOCK_HIGH_MASK;

} break;

case NV_PTIMER_TIME_0: {

uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));

d->ptimer.time_high_latched = (now >> 32) & CLOCK_HIGH_MASK;

r = now & 0xffffffff;

} break;

case NV_PTIMER_TIME_1: {

r = d->ptimer.time_high_latched;

} break;

@gemini-code-assist: Latching could be the right approach, but we should review access patterns and confirm what hardware actually does. The proposed change might not be correct if NV_PTIMER_TIME_1 is read before NV_PTIMER_TIME_0, for instance. I suspect hardware leaves this to be checked by software anyway and that the current solution is correct.

You raise a very valid point, @mborgerson, regarding the need to confirm actual hardware behavior and access patterns. My suggestion was based on a common hardware pattern to mitigate race conditions when a 64-bit timer is exposed via two separate 32-bit registers.

The core issue is that in the current implementation, both NV_PTIMER_TIME_0 and NV_PTIMER_TIME_1 independently call get_ptimer_clock(d, ptimer_get_absolute_clock(d)). This means that if the guest reads NV_PTIMER_TIME_0, then the timer ticks, and then the guest reads NV_PTIMER_TIME_1, the resulting 64-bit value constructed by the guest will be inconsistent (the high part will be from a later time than the low part). This is a classic race condition that can lead to incorrect time values for the guest OS.

My proposed solution addresses this specific race condition by latching the high 32 bits of the timer when the low 32 bits (NV_PTIMER_TIME_0) are read. This ensures that when the guest subsequently reads the high 32 bits (NV_PTIMER_TIME_1), it receives the value that was current at the time of the NV_PTIMER_TIME_0 read, thus providing a consistent 64-bit snapshot.

Your concern about NV_PTIMER_TIME_1 being read before NV_PTIMER_TIME_0 is important. If the hardware indeed allows NV_PTIMER_TIME_1 to be read independently and still expects consistency, or if it has a different latching mechanism (e.g., a dedicated register write to latch both parts), then my proposed solution might not perfectly mirror that specific hardware behavior. However, in the absence of precise hardware documentation, the read low then read high pattern with implicit latching on the low read is a very common design to ensure atomic 64-bit reads from split registers. The current implementation, without any form of latching, will always be susceptible to the described race condition if the guest attempts to reconstruct a 64-bit timer value from two separate reads.

Even if software can work around this by reading the high part multiple times until it stabilizes (a common but inefficient software workaround), the goal of emulation is often to present a consistent and predictable hardware interface. Providing an atomic 64-bit read through a latching mechanism is generally a more robust emulation of a 64-bit timer.

Interesting response. My expectation is also that the HW isn't doing magic here and it really is left up to the SW to detect race-rollover if it actually cares to. When I was debugging the issue that precipitated this change I saw several places where the timer was being read in a tight loop, presumably to work around the potential race.

mborgerson · 2026-01-15T04:38:36Z

hw/xbox/nv2a/ptimer.c

+    case NV_PTIMER_TIME_0: {
+        uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));
+        r = now & 0xffffffff;
+    } break;
+    case NV_PTIMER_TIME_1: {
+        uint64_t now = get_ptimer_clock(d, ptimer_get_absolute_clock(d));
+        r = (now >> 32) & CLOCK_HIGH_MASK;
+    } break;


@gemini-code-assist: Latching could be the right approach, but we should review access patterns and confirm what hardware actually does. The proposed change might not be correct if NV_PTIMER_TIME_1 is read before NV_PTIMER_TIME_0, for instance. I suspect hardware leaves this to be checked by software anyway and that the current solution is correct.

mborgerson · 2026-01-15T04:43:05Z

hw/xbox/nv2a/nv2a.c

    .post_save = nv2a_post_save,
    .post_load = nv2a_post_load,
    .pre_load = nv2a_pre_load,
+    // NOTE: New fields should be appended, regardless of similarity to existing


Let's drop the comment. VMState is documented elsewhere

Done, though I think I've broken this at least a couple times over the years by inserting or resizing things inline.

I wonder if the review bots read AGENTS.md and could be instructed to look for this specifically?

I believe so! Not sure about the actual details. Maybe they can be instructed to read select pages in docs/ based on related subsystem under review.

mborgerson · 2026-01-15T04:44:25Z

hw/xbox/nv2a/nv2a_regs.h

 #define NV_PMC_INTR_0                                    0x00000100
 #   define NV_PMC_INTR_0_PFIFO                                 (1 << 8)
 #   define NV_PMC_INTR_0_PGRAPH                               (1 << 12)
+#   define NV_PMC_INTR_0_PTIMER_PENDING                       (1 << 20)


To match other fields, drop _PENDING suffix

mborgerson · 2026-01-15T04:56:03Z

hw/xbox/nv2a/ptimer.c

+
+    uint64_t diff_ns = 0;
+    if (alarm_time > now) {
+        diff_ns = ptimer_ticks_to_ns(d, (alarm_time - now) >> 5);


I'm a bit lost tracking the bit shifting. Is the left shifting in get_ptimer_clock just putting the clock in register format? We shift left in get_ptimer_clock, then shift right here to compute actual clock deltas. I wonder if this means we should really just do the shifting/masking at the register interface (e.g. in ptimer_read)

Yes, it's just moving in and out of the format used in the registers.

Aren't the read/write callbacks expected to act on data in HW format?

I wonder if some helper macros would be appropriate rather than messing with the guest-accessible callback behavior (but maybe I'm just not following your suggestion).

abaire · 2026-02-06T18:30:25Z

I've been continuing the investigation in #1124 - I think the remaining softlock is timing related rather than an issue with the PTIMER alarm code added in this PR.

To summarize my understanding at the moment, the game tries to keep a consistent timer interval and does some checking using the CPU timestamp counter (rdtsc) and a delta versus the last alarm time to account for small variations in the rescheduling of the alarm. The deltas are generally very small, but in xemu the interrupt interval is much less stable than on HW and we eventually run into a case where the code responsible for rescheduling the PTIMER alarm runs after the alarm should've fired. This leads to a state that the title never bothered to handle, since it shouldn't ever happen in HW, and it fails to schedule the alarm and disables PTIMER alarm events generally, causing the logic to sit around forever waiting for an alarm that cannot ever come.

This will require a separate fix to address. At this point I've created additional PTIMER tests and am relatively confident that the PTIMER behavior on its own matches hardware.

In the meantime you may be able to get the game to work by enabling the qemu icount feature. My ~30 minutes of testing showed that this prevents the freeze, but it causes a very noticeable performance degradation (at least in my debug build of xemu).

Looks like the PTIMER alarm handling was started but never finished. This implements my best guess at what the HW is doing (it's hard to test with a nanosecond timer). It fixes the blocked loading for Def Jam: Fight for NY, which I determined to be caused by a event handle never being set due to the lack of an alarm interrupt in the same DPC that handles video blanking and other PMC interrupts. The PTIMER pending flag is known from the nxdk: https://github.com/XboxDev/nxdk/blob/d0ae96acc926a6c3f6254b289a1a9a3c1db182f1/lib/pbkit/outer.h#L83 Fixes xemu-project#1124

- Timers on HW continue to fire every time the low 27 bits wrap around. - Standardized the logic such that the "now" clock is shifted 5 bits to match the setter and observed HW behavior.

abaire force-pushed the fix_1124_implement_ptimer_alarm branch from 79e6178 to 4a6a08d Compare November 27, 2025 00:19

PatrickvL suggested changes Nov 27, 2025

View reviewed changes

abaire marked this pull request as ready for review November 29, 2025 21:04

mborgerson reviewed Dec 5, 2025

View reviewed changes

abaire force-pushed the fix_1124_implement_ptimer_alarm branch from 4a6a08d to f0310fc Compare December 6, 2025 02:43

abaire mentioned this pull request Dec 6, 2025

Def Jam: Fight for NY "PTimer upsupported" leading to freezing #1124

Open

abaire marked this pull request as draft December 14, 2025 06:33

abaire marked this pull request as ready for review December 14, 2025 08:08

abaire mentioned this pull request Dec 14, 2025

True Crime: New York City Loading Softlock #2513

Open

gemini-code-assist bot reviewed Jan 15, 2026

View reviewed changes

mborgerson reviewed Jan 15, 2026

View reviewed changes

abaire force-pushed the fix_1124_implement_ptimer_alarm branch from 0dce0ec to a3daf09 Compare January 19, 2026 13:39

abaire marked this pull request as draft February 4, 2026 21:39

xemu-project deleted a comment from uchihagirlzz23-png Feb 5, 2026

abaire marked this pull request as ready for review February 6, 2026 18:23

abaire added 5 commits February 10, 2026 11:15

SQUASHME: Handle time register writes.

20449ec

SQUASHME: Address review feedback, switch to QEMUTimer

bce7f44

SQUASHME: Allows timers to fire continually while interrupt is enabled.

29ce639

- Timers on HW continue to fire every time the low 27 bits wrap around. - Standardized the logic such that the "now" clock is shifted 5 bits to match the setter and observed HW behavior.

SQUASHME: Dropped comment and updated interrupt name.

b03d8a4

abaire force-pushed the fix_1124_implement_ptimer_alarm branch from a3daf09 to b03d8a4 Compare February 10, 2026 19:15

Uh oh!

nv2a: Implement PTIMER alarm interrupts #2568

Are you sure you want to change the base?

nv2a: Implement PTIMER alarm interrupts #2568

Conversation

abaire commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Triticum0 commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PatrickvL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joshua-1248 commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mborgerson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abaire commented Dec 14, 2025

Uh oh!

abaire commented Dec 14, 2025

Uh oh!

Triticum0 commented Dec 15, 2025

Uh oh!

retchless commented Dec 22, 2025

Uh oh!

mborgerson commented Jan 15, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

abaire commented Nov 26, 2025 •

edited

Loading

Triticum0 commented Nov 27, 2025 •

edited

Loading

Joshua-1248 commented Nov 30, 2025 •

edited

Loading