Skip to content

[test] Add npu-xrt local_reset on-board tests (core/dma) - #3397

Merged
jskimko merged 8 commits into
Xilinx:mainfrom
jskimko:local-reset-tests
Jul 24, 2026
Merged

[test] Add npu-xrt local_reset on-board tests (core/dma)#3397
jskimko merged 8 commits into
Xilinx:mainfrom
jskimko:local-reset-tests

Conversation

@jskimko

@jskimko jskimko commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds a test/npu-xrt/local_reset/ suite of on-board tests for local reset -- returning a single tile-local block to a clean state from the runtime sequence, without rebuilding the design or reloading the array. Each family resets one block and checks that a resident design survives it.

Direct register-write families issue the reset from aie.runtime_sequence, using the same register-write type the aie-rt driver uses:

  • core -- Core_Control (0x32000): masked reset -> unreset -> enable (XAie_CoreReset / XAie_CoreUnreset / XAie_CoreEnable) re-runs a halted core.
  • dma -- DMA_MM2S_0_Ctrl (0x1DE10): masked reset pulse (XAie_DmaChannelReset), re-push BD, re-arm lock. Updated: the MM2S channel now holds a "bad" BD queued ahead of a "good" BD (both gated on a lock); the reset flushes the queue so only the good BD runs once the lock is armed.
  • switch -- Stream_Switch_Slave_DMA_0_Config (0x3F104): tear down the slave port at each dispatch end and re-enable it at the start of the next, re-arm lock (XAie_StrmConnCctEnable / XAie_StrmConnCctDisable; slave port only -- the master route is resident, noted in the README). Removed: the stream switch is not a hardware reset domain (aie-rt has no stream-switch reset routine), so this was runtime connection reconfiguration rather than a reset.

Op-based families drive the merged runtime-sequence ops instead of writing the registers directly -- the on-board counterparts of core and dma, adding on-silicon coverage for ops that otherwise have only aie-opt FileCheck tests:

Each family drives its protocol across repeated dispatches and self-checks the host-visible result; the op-based families reuse their raw sibling's test.cpp directly. All run on npu1 and npu2 (REQUIRES: ryzen_ai); every register offset is identical on both. Every offset, register-write type, and reset protocol is cross-referenced in the READMEs to the vendored aie-rt driver (third_party/aie-rt/).

Checklist

  • Tests pass locally, or the change is covered by CI
  • Docs / docstrings updated if the change is user-facing
  • Code is formatted (clang-format for C++, black for Python -- see CONTRIBUTING)

@jskimko

jskimko commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

I'll wait on #3375 to be merged, then add tests using the new API for both core and dma.

@hunhoffe

Copy link
Copy Markdown
Collaborator

#3375 in merge queue :)

jskimko added 6 commits July 23, 2026 14:07
On-board tests for local reset -- returning a single tile-local hardware
block to a clean state with raw runtime-sequence register writes, without
rebuilding the design or reloading the array. Three families, each resetting
one block and checking that a resident design survives it:
  - core:   CORE_CONTROL reset -> unreset -> enable re-runs a halted core
  - dma:    DMA channel reset flushes a stalled queue; re-push BD + re-arm lock
  - switch: stream-switch port config-disable then re-enable rebuilds a route

Each family drives its correct protocol from aie.runtime_sequence across
repeated dispatches and self-checks the host-visible result. All three run on
npu1 (Phoenix / AIE-ML) and npu2 (Strix / AIE2P) -- every tile-local register
they write has the same offset on both (REQUIRES: ryzen_ai). A top-level README
plus per-family docs cross-reference every offset and reset protocol to the
public aie-rt driver.
Apply clang-format (v22.1.8, repo-pinned) to the three test.cpp files.
Whitespace only -- 80-column reflow of long lines, no behavioral change.
The switch test previously disabled then immediately re-enabled the DMA:0
slave port inside one quiescent window, so the two writes were a no-op net of
each other: the route stayed resident and the pass was gated solely by the lock
re-arm. Removing both switch writes passed identically, so the test did not
actually exercise the port reset.

Split the disable and re-enable across the dispatch boundary: re-enable the
port at the start of each dispatch and tear it down again at the end. The config
register is sticky, so the port is genuinely off at the start of every dispatch
after the first, putting the re-enable on the critical path. Verified on npu2:
the correct protocol passes; omitting the re-enable fails at dispatch 1;
omitting both switch writes still passes (which is why the end-disable is kept).

Docs updated to describe the straddled sequence and the three behaviours.
…channel_reset

Add on-board npu-xrt tests that drive the merged reset ops (Xilinx#3375, Xilinx#3370)
instead of raw register writes, as op-based counterparts of the core/ and dma/
families:

- core_reset_op: aiex.core_reset supplies the reset->unreset pulse on
  CORE_CONTROL (0x32000, bit 1) = XAie_CoreReset + XAie_CoreUnreset. Because the
  core has run to aie.end it is no longer enabled, so the test adds a masked
  re-enable of bit 0 mirroring XAie_CoreEnable -- making the whole test the
  driver's XAie_CoreReset -> XAie_CoreUnreset -> XAie_CoreEnable sequence, every
  write masked to one field. On-board, the op alone leaves the core halted --
  pinning the op's documented still-enabled assumption.
- dma_channel_reset_op: aiex.dma_channel_reset is a drop-in for dma/'s four
  reset writes on DMA_MM2S_0_Ctrl (0x1DE10, bit 1) = XAie_DmaChannelReset; the
  re-push BD + lock re-arm remain around it.

Both ops lower to a masked maskwrite32 pulse on the same register and reset bit
the raw siblings write, matching the aie-rt driver (all masked field writes).
The ops previously had only aie-opt FileCheck coverage; these are their first
on-silicon tests. Each op-based test compiles its raw sibling's test.cpp
directly (%S/../core, %S/../dma) rather than duplicating the host harness.
All five local_reset families pass on npu2 (Strix).
…ite protocol

The raw core and dma reset tests issued full-word aiex.npu.write32s, which clobber
the whole control register. aie-rt drives these resets with XAie_MaskWrite32 of a
single bit-field, so switch each to aiex.npu.maskwrite32 to match the driver
exactly and leave sibling fields intact:

- core: reset->unreset->enable as three masked writes -- XAie_CoreReset (bit 1,
  mask 0x2), XAie_CoreUnreset (mask 0x2), XAie_CoreEnable (bit 0, mask 0x1).
- dma: the four disable/reset/disable/enable writes collapse to a masked reset
  pulse (bit 1, mask 0x2), matching XAie_DmaChannelReset; masking preserves ENABLE
  and the other CTRL fields, so no disable/enable is needed. push_queue + lock
  re-arm are unchanged.

switch is left on full-word write32: aie-rt's XAie_StrmConnCctEnable/Disable and
XAie_LockSetValue are Write32, not MaskWrite32, so the switch slave-port toggle and
the lock re-arm already match the driver's write type. READMEs updated to state the
per-family write type. All five local_reset families pass on npu2 (Strix).
…witch deviation

Reference the driver routine each register write mirrors, inline at the write, so
the aie-rt driver is the explicit source of truth for every protocol:
- core: XAie_CoreReset / XAie_CoreUnreset / XAie_CoreEnable.
- dma: XAie_DmaChannelReset (reset pulse), XAie_DmaChannelPushBdToQueue (re-push),
  XAie_LockSetValue (lock re-arm).
- switch: XAie_StrmConnCctEnable / XAie_StrmConnCctDisable (slave-port toggle),
  XAie_LockSetValue (lock re-arm).

For the switch, add an explicit deviation note: XAie_StrmConnCctEnable/Disable
write both the master- and slave-port config registers, but this test writes only
the slave register because the master (South) route is resident from the CDO load,
so rewriting it would be an idempotent no-op (and toggling only the slave port
isolates a failure to it). Noted in switch/aie.mlir, switch/README.md, and the
top-level README.

Comments and READMEs describe the current state of the tests only (no iterative
history); also correct a stale reference that still described the raw core/dma
siblings as using full-word write32 (they use masked maskwrite32).

Comment/README only; IR round-trips unchanged, so the compiled artifacts and the
on-board results are unaffected.
@jskimko
jskimko force-pushed the local-reset-tests branch from 6298936 to 1286fd5 Compare July 23, 2026 22:08

@andrej andrej left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, my comments are mostly nitpicks, but should be easy to address if you could.

Follow-ups:

  • Getting native MLIR operations for those raw-register writes would be useful. This helps with portability for future architectures, as well as understandability and debuggability. Lowerings for these additional ops could probably be tacked onto @atassis' existing core reset lowering pass.
  • For the switchbox and DMA channel reset, I think it would be especially interesting if it is possible to change the switchbox or DMA channel after the reset. It looks to me that these tests currently re-run the same configuration after resetting and unresetting as before. I'd love to see this working for a reconfiguration, e.g. reset the switchbox and route it somewhere else between iterations of the test.

Comment thread test/npu-xrt/local_reset/core/test.cpp Outdated
Comment thread test/npu-xrt/local_reset/core_reset_op/aie.mlir
Comment thread test/npu-xrt/local_reset/dma/test.cpp Outdated
Comment thread test/npu-xrt/local_reset/switch/test.cpp Outdated
Comment thread test/npu-xrt/local_reset/switch/aie.mlir Outdated
Rework the dma and dma_channel_reset_op tests so the channel reset is
load-bearing and the outcome is deterministic. The MM2S channel now holds two
buffer descriptors gated on the same lock -- a "bad" BD 0 ([900..907]) queued
ahead of a "good" BD 1 ([100..107]). Each dispatch enqueues the bad BD, resets
the channel to flush the queue, enqueues the good BD, then arms the lock, so
only the good BD runs. Removing the reset leaves the bad BD ahead in the queue
and the host collects [900..907] -- a clean wrong-data failure rather than a
hang, which makes the reset falsifiable. Nothing moves until the lock is armed,
so the result no longer depends on unobservable channel re-quiescence between
dispatches (this removes the earlier intermittent failure under concurrency).

dma issues the reset directly as a masked XAie_DmaChannelReset pulse;
dma_channel_reset_op issues it through the aiex.dma_channel_reset op, which
lowers to the same pulse. Both remain byte-mirrored and reuse a single host
oracle (test.cpp), updated for the two-BD flow.

Also drop the switch family: the stream switch is not a hardware reset domain
(no reset routine in aie-rt), so a "switch reset" test misrepresents the
hardware. Runtime reconfiguration of switch connections is a separate concern.
@jskimko
jskimko force-pushed the local-reset-tests branch from 47167ee to 6b849fe Compare July 24, 2026 20:46
@jskimko

jskimko commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author
  • Replaced the dma test to avoid flaky sync issues by observing a queue reset instead.
  • Dropped the switch test since this turned out to be more of a reconfiguration test rather than a true reset.

@jskimko jskimko changed the title [test] Add npu-xrt local_reset on-board tests (core/dma/switch) [test] Add npu-xrt local_reset on-board tests (core/dma) Jul 24, 2026

@andrej andrej left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with Jason: the deleted reconfiguration tests might be added in a future PR.

@jskimko
jskimko added this pull request to the merge queue Jul 24, 2026
Merged via the queue into Xilinx:main with commit 05ce230 Jul 24, 2026
67 checks passed
@jskimko
jskimko deleted the local-reset-tests branch July 24, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants