Skip to content

Performance Research C04 PFIFO PGRAPH Dispatch Handoffs

github-actions[bot] edited this page Sep 18, 2026 · 2 revisions

PFIFO/PGRAPH dispatch handoffs and command-front-end batching

C04 — Implementation and validation notes

Field Value
Evidence state SOURCE_CONFIRMED; steady-state lock cost and end-to-end gain remain unmeasured
Inspected source 5cf180b290ea0fd71c45035df103e44b68c2d9ca
First experiment Count PFIFO→PGRAPH ownership transfers, words per transfer, contention, and zero-progress returns
Candidate scope Renderer-independent NV2A command-front-end efficiency; not Vulkan/OpenGL command-buffer batching
Related work Issue #65 — renderer-switch PFIFO/PGRAPH lock-order inversion, G10 — renderer command recording

Unproven performance research · Performance research — common test protocol · Performance research — reproducible checks

Why this vector merits measurement

The PFIFO worker parses guest DMA push-buffer commands while owning pfifo.lock. For an object bind or ordinary graphics method, pfifo_run_puller() currently releases pfifo.lock, acquires pgraph.lock, calls pgraph_context_switch() and/or pgraph_method(), releases pgraph.lock, and reacquires pfifo.lock before committing DMA/FIFO progress. Both call sites carry explicit locking TODO comments. Current PFIFO source.

PFIFO owns pfifo.lock
    -> parse packet / inspect CACHE1 state
    -> release pfifo.lock
    -> wait for and acquire pgraph.lock
    -> execute one object bind or pgraph_method() call
    -> release pgraph.lock
    -> wait for and reacquire pfifo.lock
    -> advance DMA GET, method count and dcount
    -> repeat

Potential local costs are mutex operations, cache-line ownership transfer, scheduler blocking, and lost locality between adjacent methods. The source proves the transfer pattern, not that it is expensive on a current workload. A retained PGR2 profile reported 36.198 CPU-seconds on the PFIFO thread during an approximately 60-second capture, but its named costs were shader/uniform/hash/pipeline work; it did not isolate mutex handoff time. Historical PGR2 investigation.

This investigation must also account for work already batched. pgraph_method() accepts a parameter pointer plus available/lookahead word counts, and the existing inline-array path can consume an entire eligible packet through pgraph_method_array_bulk(). The proposed vector is therefore cross-packet ownership amortization, not reimplementation of the existing INLINE_ARRAY / ARRAY_ELEMENT16 / ARRAY_ELEMENT32 bulk path. PGRAPH source, inline packet helpers.

Current ownership boundaries

Boundary Current behavior Why it matters
Guest PFIFO MMIO pfifo_read() / pfifo_write() take pfifo.lock; writes kick the worker A longer PFIFO critical section can delay vCPU-visible access
PGRAPH MMIO Most pgraph_read() calls take pgraph.lock; pgraph_write() takes PFIFO then PGRAPH Any new dual-lock epoch must use one explicit global order
PFIFO worker Holds pfifo.lock around pending renderer work, pusher execution, report processing and condition wait Counters should be owner-local; no extra hot-path mutex is needed
PGRAPH method execution Serialized by pgraph.lock Multiple safe packets could theoretically share one acquisition
Renderer switch Contains a separately tracked reverse-order transition C04 cannot broaden dual-lock holding until issue #65 is resolved or proven unreachable
Push-buffer memory Read from mapped guest DMA memory Reading future packets earlier can change observation timing if the guest rewrites them

Relevant state definitions are in nv2a_int.h and pgraph.h.

Phase 0 — attribution before behavior changes

Add exact owner-local counts and sampled timing. Do not call a host clock on every method. One sample per 1,024 ownership transfers is a reasonable initial diagnostic rate; record the sampling denominator in the evidence.

Required counters

Counter Meaning
pfifo.dispatch.puller_calls Calls into pfifo_run_puller()
pfifo.dispatch.handoffs PFIFO→PGRAPH ownership transfers
pfifo.dispatch.method_packets Method packets presented to PGRAPH
pfifo.dispatch.parameter_words Parameter words actually consumed
pfifo.dispatch.object_binds Method-zero and object-parameter RAMHT binds
pfifo.dispatch.context_switches Actual channel-context switch attempts
pfifo.dispatch.zero_progress Transfers returning zero processed words
pfifo.dispatch.stall_returns Returns caused by flip, NOP, context-switch or FIFO-access stalls
pfifo.dispatch.partial_packets Packets completed over more than one transfer
pfifo.dispatch.max_words_per_handoff Largest useful transfer observed
pfifo.dispatch.pgraph_wait_ns_sampled Sampled time waiting for pgraph.lock
pfifo.dispatch.pgraph_hold_ns_sampled Sampled time executing while holding pgraph.lock
pfifo.dispatch.pfifo_reacquire_ns_sampled Sampled time reacquiring pfifo.lock
pfifo.thread.kicks Kick requests observed by the PFIFO worker
pfifo.thread.cond_sleeps Actual condition-variable sleeps
pfifo.thread.empty_wakes Wakes that make no FIFO/PGRAPH/report progress

Report histograms or buckets for words per handoff rather than only a mean. A mean can hide thousands of one-word transfers behind a small number of large inline packets.

Recommended instrumentation structure

typedef struct PFIFODispatchStats {
    uint64_t puller_calls;
    uint64_t handoffs;
    uint64_t method_packets;
    uint64_t parameter_words;
    uint64_t object_binds;
    uint64_t context_switches;
    uint64_t zero_progress;
    uint64_t stall_returns;
    uint64_t partial_packets;
    uint64_t kicks;
    uint64_t cond_sleeps;
    uint64_t empty_wakes;

    uint64_t sampled_handoffs;
    uint64_t pgraph_wait_ns;
    uint64_t pgraph_hold_ns;
    uint64_t pfifo_reacquire_ns;

    uint64_t words_per_handoff[9]; /* 0, 1, 2-3, 4-7 ... 128+ */
} PFIFODispatchStats;

Keep this storage under NV2AState.pfifo or another PFIFO-owned structure. Snapshot it only at a safe low-frequency boundary. Do not use contended atomics when the PFIFO thread is the sole writer.

Recommended first code change: behavior-preserving handoff helper

Before batching, extract the duplicated unlock/lock sequence into one helper. This reduces review surface and gives one place for counters and sampled timestamps while preserving the current call order.

typedef struct PFIFOPGraphCall {
    unsigned int subchannel;
    unsigned int method;
    uint32_t parameter;
    uint32_t *parameters;
    size_t num_words_available;
    size_t max_lookahead_words;
    bool incrementing;
    bool context_switch;
    unsigned int channel_id;
} PFIFOPGraphCall;

static ssize_t pfifo_call_pgraph(NV2AState *d,
                                 const PFIFOPGraphCall *call)
{
    /* Called with pfifo.lock held; returns with pfifo.lock held. */
    qemu_mutex_unlock(&d->pfifo.lock);
    qemu_mutex_lock(&d->pgraph.lock);

    ssize_t processed = -1;
    if (can_fifo_access(d)) {
        if (call->context_switch) {
            pgraph_context_switch(d, call->channel_id);
        }
        if (!d->pgraph.waiting_for_context_switch) {
            processed = pgraph_method(
                d, call->subchannel, call->method, call->parameter,
                call->parameters, call->num_words_available,
                call->max_lookahead_words, call->incrementing);
        }
    }

    qemu_mutex_unlock(&d->pgraph.lock);
    qemu_mutex_lock(&d->pfifo.lock);
    return processed;
}

The production helper must preserve the two existing cases exactly, including RAMHT lookup, engine selection, context-switch behavior, can_fifo_access() recheck, status updates and return values. The sketch is not a drop-in patch.

Phase 1 candidate — bounded safe dispatch epoch

Only proceed if Phase 0 records a high transfer rate or meaningful wait/reacquire cost. The smallest useful candidate is a bounded list of separate, explicitly batch-safe method packets executed under one PGRAPH acquisition.

Recommended limits for the first experiment

#define PFIFO_DISPATCH_MAX_PACKETS 32
#define PFIFO_DISPATCH_MAX_WORDS   256

These are diagnostic starting limits, not tuned constants. Stop earlier at every semantic barrier. Never allocate an unbounded queue from guest packet counts.

Prepared representation

typedef struct PFIFOPreparedPacket {
    uint16_t method;
    uint8_t subchannel;
    bool incrementing;
    uint16_t first_word;
    uint16_t word_count;
} PFIFOPreparedPacket;

typedef enum PFIFODispatchStopReason {
    PFIFO_DISPATCH_STOP_LIMIT,
    PFIFO_DISPATCH_STOP_PACKET_END,
    PFIFO_DISPATCH_STOP_OBJECT_BIND,
    PFIFO_DISPATCH_STOP_CONTROL_FLOW,
    PFIFO_DISPATCH_STOP_CONTEXT_SWITCH,
    PFIFO_DISPATCH_STOP_GUEST_VISIBLE_SYNC,
    PFIFO_DISPATCH_STOP_DRAW_OR_SURFACE,
    PFIFO_DISPATCH_STOP_FIFO_STALL,
    PFIFO_DISPATCH_STOP_UNKNOWN_METHOD,
} PFIFODispatchStopReason;

typedef struct PFIFOPreparedBatch {
    PFIFOPreparedPacket packets[PFIFO_DISPATCH_MAX_PACKETS];
    uint32_t words[PFIFO_DISPATCH_MAX_WORDS];
    uint16_t packet_count;
    uint16_t word_count;
    PFIFODispatchStopReason stop_reason;
} PFIFOPreparedBatch;

Batch eligibility should be whitelist-based

Default every method to a barrier. Mark a method batch-safe only when it is proven to be an ordered PGRAPH state update with no required intermediate PFIFO observation, renderer wait, report/semaphore publication, draw/surface action, context change, or externally visible interrupt transition.

typedef enum PGRAPHDispatchClass {
    PGRAPH_DISPATCH_STATE_WRITE_SAFE,
    PGRAPH_DISPATCH_INLINE_PAYLOAD_EXISTING_BULK,
    PGRAPH_DISPATCH_DRAW_OR_SURFACE_BARRIER,
    PGRAPH_DISPATCH_REPORT_OR_SEMAPHORE_BARRIER,
    PGRAPH_DISPATCH_CONTEXT_OR_OBJECT_BARRIER,
    PGRAPH_DISPATCH_UNKNOWN_BARRIER,
} PGRAPHDispatchClass;

PGRAPHDispatchClass pgraph_method_dispatch_class(
    unsigned int subchannel, unsigned int method, bool incrementing);

Do not infer safety only from a numeric method range. The handler body and class-specific side effects decide eligibility. Unknown or newly added methods remain barriers until classified and tested.

Recommended execution flow

PFIFO lock held
    -> snapshot current DMA GET/PUT and packet state
    -> copy only the bounded, currently available safe prefix
    -> stop before object bind, jump/call/return, context switch,
       draw/clear/surface work, report/semaphore, stall, unknown method or limit
    -> acquire PGRAPH using the globally approved lock order
    -> execute the prepared prefix in exact order
    -> commit exactly the consumed prefix to DMA GET/count/dcount
    -> release locks and allow other threads to progress

There are two possible ownership implementations:

  1. Bounded dual-lock epoch: retain pfifo.lock, then acquire pgraph.lock in the approved global order while executing the small safe prefix. This prevents concurrent PFIFO-state mutation, but is forbidden until all reachable reverse-order paths—especially issue #65—are removed or proven unreachable. Hold-time and fairness limits are mandatory.
  2. Single-owner command mailbox: make the PFIFO worker the sole owner of command-front-end state and queue guest MMIO operations. This avoids dual-lock epochs but changes MMIO visibility and lifecycle architecture. Treat it as a later redesign, not the first candidate.

An optimistic “execute PGRAPH, then verify PFIFO state before commit” design is unsafe because PGRAPH side effects cannot be rolled back after a failed verification.

Push-buffer observation and read-ahead rule

Cross-packet preparation reads guest DMA memory earlier than the current one-packet sequence. A title may rewrite a future push-buffer word after publishing DMA_PUT. Do not assume that all published packet bytes are immutable without evidence.

The first candidate must therefore satisfy one of these contracts:

  • prove that the supported producer contract makes the snapshotted [GET, PUT) prefix immutable until consumed;
  • or limit batching to words already copied by an existing safe packet path;
  • or add a deterministic test for self-modifying push buffers and preserve the selected observation semantics.

Do not store raw pointers into guest DMA memory for later execution. Copy the bounded parameters into owned storage or prove their lifetime through execution.

Files to change

Phase 0: instrumentation-only PR

File Change
hw/xbox/nv2a/pfifo.c Centralize the handoff, count calls/words/stalls, sample lock wait/hold/reacquire time, count kicks/sleeps/wakes
hw/xbox/nv2a/nv2a_int.h Add PFIFO-owned statistics or a pointer to a focused stats structure
hw/xbox/nv2a/debug.h Optional: expose only low-frequency display/export counters; do not add per-dispatch formatting
hw/xbox/nv2a/pgraph/profile.c Optional aggregate snapshot/export using the existing profiling seam
tests/unit/test-xbox-pfifo-dispatch.c Test counter buckets and the behavior-preserving handoff decision helpers
tests/unit/meson.build Register the focused unit target

A cleaner alternative is to create hw/xbox/nv2a/pfifo-dispatch.h for pure structs/bucketing helpers and keep pfifo.c responsible for synchronization.

Phase 1: bounded batching candidate

File Change
hw/xbox/nv2a/pfifo.c Prepare a bounded safe prefix, execute it under one approved ownership epoch, and commit exact FIFO progress
hw/xbox/nv2a/pfifo-dispatch.h Define prepared packet/batch types, limits, stop reasons and pure planning helpers
hw/xbox/nv2a/pfifo-dispatch.c Optional if planning/commit logic becomes too large for pfifo.c
hw/xbox/nv2a/meson.build Compile a new dispatch source exactly once if created
hw/xbox/nv2a/pgraph/pgraph.h Declare dispatch classification and, only if needed, a batch-execution interface
hw/xbox/nv2a/pgraph/pgraph.c Classify handlers conservatively and execute prepared packets without duplicating existing packet-level bulk logic
tests/unit/test-xbox-pfifo-dispatch.c Parser cursor, stop-reason, commit-prefix, limit and failure tests
tests/unit/test-xbox-pgraph-dispatch-class.c Prove safe whitelist/default-barrier behavior for representative methods
tests/unit/meson.build Register both focused targets
Mainkill1/xemu-perf-tests XISO source/catalog Add short-packet, large-packet, object-bind, control-flow and barrier-mix production oracles

Do not add a normal user-facing setting in the first implementation. Use an opt-in diagnostic build flag or same-binary developer override only after the reference route and evidence counters are stable.

Focused tests

These are implementation gates, not passing results.

Test ID Setup / deliberate control Required oracle
C04.01 Same final PGRAPH state encoded as thousands of one-word packets versus fewer large packets. Identical register/state checksum; handoff and words-per-handoff sensitivity is recorded.
C04.02 Increasing and non-increasing methods, partial packets, and exact batch-limit boundaries. Exact method order, consumed count, DMA GET, method count and dcount match the reference.
C04.03 INLINE_ARRAY, ARRAY_ELEMENT16, and ARRAY_ELEMENT32 with the existing bulk toggle/reference route. Existing packet-level bulk behavior is unchanged; no double batching or capacity bypass.
C04.04 Object binds and a required context switch inserted between otherwise safe state writes. Batch stops before the boundary; interrupt/context timing and later progress match.
C04.05 Jump, old jump, call, return and reserved-command controls. Parser/subroutine/error state and GET progression match exactly; no read-ahead across control flow.
C04.06 GET_REPORT, CLEAR_REPORT_VALUE, semaphore release, clear, draw begin/end, flip stall and NOP boundaries. Guest-visible publication and wait ordering are unchanged; each is a batch barrier until separately proven safe.
C04.07 Guest reads/writes PFIFO/PGRAPH MMIO while a maximum-sized epoch executes. No deadlock, bounded access latency, correct values, and no starvation.
C04.08 Repeated Vulkan↔OpenGL switch, resize, pause/save/load, reset and shutdown during FIFO pressure. Lock order, ownership, framebuffer lifetime and worker exit remain correct.
C04.09 Future push-buffer words are rewritten after PUT publication but before reference consumption. Candidate matches the selected supported observation semantics or batching is disabled for this case.
C04.10 Inject zero progress or failure after a prefix of prepared packets. Only the executed prefix is committed; no duplicated or skipped PGRAPH side effect.

Production qualification workloads

  1. Synthetic XISO short-packet and large-packet pair.
  2. PGR2 snapshot and full start.
  3. Morrowind snapshot.
  4. Complete current XISO on OpenGL and Vulkan.
  5. Renderer-switch and snapshot lifecycle loop under active push-buffer load.

Run instrumentation-only and candidate builds separately from the final uninstrumented performance comparison.

Performance model

Let:

  • H = ownership transfers per second,
  • C = measured average effective transfer cost in nanoseconds, including wait and cache ownership,
  • B = useful dispatches completed per optimized ownership epoch.

The local CPU saving ceiling is:

saved CPU seconds per second = H × C × (1 - 1/B) / 1,000,000,000

This is a sizing model, not a prediction. Populate it from counters. Report process CPU, PFIFO-thread CPU and guest progress separately.

Observation Interpretation
High handoff rate, almost no waiting Possible CPU-efficiency gain; frame-tail gain may be negligible
Material PGRAPH wait or PFIFO reacquire time Coordination may affect throughput or tails
Large words-per-handoff already Cross-packet batching is probably low priority
Lower PFIFO CPU but longer vCPU/MMIO waits Candidate is not acceptable without a better fairness bound
Wait label merely moves to renderer/report work No demonstrated total-work reduction

Correctness and concurrency rules

  • Preserve exact push-buffer command order and packet consumption.
  • Preserve DMA GET/PUT, method count, dcount, shadow registers and error state.
  • Preserve object binding, engine selection and context-switch timing.
  • Preserve report, clear, semaphore, fence, flip and interrupt observation boundaries.
  • Preserve the existing lock-free PGRAPH fence-read ordering.
  • Preserve OpenGL/Vulkan renderer behavior; this layer is renderer-independent.
  • Use one documented global PFIFO/PGRAPH lock order; never add a new reverse edge.
  • Bound packets, words and lock hold time even when the queue is continuously full.
  • Yield at lifecycle requests, renderer switch, pause, save/load, reset and shutdown.
  • Unknown methods and uncertain side effects are barriers, not optimization opportunities.
  • Do not call timing, JSON, logging or allocation APIs for every command in release builds.

Acceptance and rejection criteria

A viable implementation must demonstrate:

same guest command stream
same ordered PGRAPH method sequence
same consumed-word and FIFO-register results
same report/semaphore/fence/interrupt behavior
same functional hashes or approved pixel oracles
zero new validation or sanitizer failures
no lock-order, lifecycle or shutdown hang
bounded vCPU/MMIO access latency
fewer ownership transfers or lower combined coordination CPU
no material whole-workload p95/p99 regression

Compare at least:

  • A: previous accepted main.
  • B: instrumentation-only source with diagnostics disabled and enabled.
  • C: one isolated batching candidate.

Reject: any command reordering, missed/duplicated side effect, read-ahead semantic change, reverse lock edge, deadlock, unbounded MMIO latency, or tail regression beyond the repository threshold. Hold: if counters show low opportunity, or local lock counts improve while equivalent-work CPU and frame tails do not.

Suggested first implementation order

  1. Add the behavior-preserving handoff helper and exact counters.
  2. Run the synthetic packet pair plus PGR2/Morrowind to measure opportunity.
  3. Complete the issue #65 lock-order audit before any dual-lock candidate.
  4. Add pure batch planner/classifier tests with every unknown method treated as a barrier.
  5. Implement one bounded 32-packet/256-word safe-prefix candidate.
  6. Qualify MMIO fairness and lifecycle before broad gameplay performance testing.
  7. Keep, revise or abandon the vector based on total-work evidence—not lock-count reduction alone.

No performance gain is established by this documentation entry. The purpose of C04 is to turn a plausible coordination cost into a quickly falsifiable, source-pinned experiment.

Recent evidence — lock ordering must include progress dependencies

Merged PR #132 demonstrates that a nominal lock order is not enough when progress requires another thread to acquire those locks.

Renderer switching could reacquire PFIFO while holding PGRAPH, while framebuffer/backend synchronization needed the render thread to progress. The repair uses an explicit handoff admission, drops incompatible locks, services pending backend work during nonblocking renderer-lock retries, and sleeps on a progress event instead of spinning or servicing only once.

sequenceDiagram
    participant S as Switch
    participant P as PFIFO/PGRAPH
    participant R as Renderer lock
    participant B as Backend/framebuffer
    S->>P: claim handoff admission
    S->>P: publish flush then release incompatible locks
    loop bounded retry
        S->>R: try lock
        alt busy
            S->>B: service pending progress
            S->>S: wait on progress event if needed
        end
    end
    S->>R: drain outstanding lease
    S->>P: finalize/init under defined ownership
    S->>P: publish installed renderer
Loading

Reusable rules: never sleep while holding a lock needed by the progress path; use explicit admission; preserve pre-handoff ownership; use event-driven bounded retries; document the lock contract for progress callbacks.

For concurrency fixtures, a controlled regression that removes a required progress step and reliably times out is stronger than a happy-path-only test.

Clone this wiki locally