[donwalkarsoham]

Suspend destroys hw contexts non-gracefully, so firmware discards their state and resume rebuilds from scratch. Firmware can instead park a context and return a restore_id for a later create to replay.

Destroy gracefully on suspend, keep the restore_id and pass it to the next create. The parked state lives in a firmware context restore pool that does not survive a firmware suspend, so copy it into a driver buffer once all contexts are destroyed and hand it back after the partition is recreated. Reset and TDR stay non-graceful, and an FLR drops both the ids and the pool. If firmware returns restore_id 0, or the pool cannot be saved, the ids are cleared and parked jobs aborted so resume builds fresh contexts.

[xdavidz]

Suspend destroys hw contexts non-gracefully, so firmware discards their state and resume rebuilds from scratch. Firmware can instead park a context and return a restore_id for a later create to replay.

Destroy gracefully on suspend, keep the restore_id and pass it to the next create. The parked state lives in a firmware context restore pool that does not survive a firmware suspend, so copy it into a driver buffer once all contexts are destroyed and hand it back after the partition is recreated. Reset and TDR stay non-graceful, and an FLR drops both the ids and the pool. If firmware returns restore_id 0, or the pool cannot be saved, the ids are cleared and parked jobs aborted so resume builds fresh contexts.

Will you be able to develope a shim_test for this too?

[Copilot]

Pull request overview

This PR adds AIE4-specific support for preserving and restoring hardware context state across runtime auto-suspend/resume by using firmware “graceful destroy” restore IDs and persisting the firmware restore-pool across the suspend power boundary.

Changes:

• Implement graceful hwctx destroy on suspend, capturing per-context restore_id and saving the firmware context restore pool into a driver buffer for resume.
• Restore the saved restore pool on resume before recreating contexts, and fall back to fresh context creation if pool save/restore fails (clearing restore IDs and aborting parked jobs).
• Add a shim_test that holds an idle context across multiple runtime auto-suspend cycles and submits work after each resume to guard the restore path from regressions.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/shim_test/shim_test.cpp	Adds an AIE4 + native-driver device filter and registers the new suspend/restore regression test.
test/shim_test/io_test.cpp	Implements runtime PM polling via sysfs and adds TEST_ctx_restore_across_suspend.
drivers/accel/amdxdna/amdxdna_ctx.c	Resumes the device before hwctx erase/destroy so firmware restore data can be reclaimed when destroying a suspended context.
drivers/accel/amdxdna/aie4_pci.h	Adds restore_id to hwctx private state and adds a device-level saved restore-pool handle plus message prototypes.
drivers/accel/amdxdna/aie4_pci.c	Saves/clears/restores the firmware ctx restore pool across suspend/resume; switches suspend path to graceful destroy for non-clean shutdowns.
drivers/accel/amdxdna/aie4_msg_priv.h	Introduces new AIE4 mailbox opcodes and request/response structs for restore-pool get/set.
drivers/accel/amdxdna/aie4_message.c	Implements get-size/get/set message helpers for the context restore pool.
drivers/accel/amdxdna/aie4_ctx.c	Threads restore IDs through graceful destroy and into subsequent create; aborts parked jobs when graceful destroy returns restore_id=0.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[donwalkarsoham]

Suspend destroys hw contexts non-gracefully, so firmware discards their state and resume rebuilds from scratch. Firmware can instead park a context and return a restore_id for a later create to replay.
Destroy gracefully on suspend, keep the restore_id and pass it to the next create. The parked state lives in a firmware context restore pool that does not survive a firmware suspend, so copy it into a driver buffer once all contexts are destroyed and hand it back after the partition is recreated. Reset and TDR stay non-graceful, and an FLR drops both the ids and the pool. If firmware returns restore_id 0, or the pool cannot be saved, the ids are cleared and parked jobs aborted so resume builds fresh contexts.

Will you be able to develope a shim_test for this too?

Yes, a shim test is added in the PR.

[donwalkarsoham]

Retest this please

[xdavidz]

The comments look good to me, thanks for handling this.
Please double check with @wendyliang25 if this handling look ok by her from TDR point of view.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect parked-job recovery, DMA cache coherency, and suspended cleanup.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

drivers/accel/amdxdna/aie4_pci.c:472

• After GET_CTX_RESTORE_POOL firmware writes this DMA buffer, but this path never invalidates the CPU cache before retaining it for the later SET. On the non-coherent dma_alloc_noncoherent path, the resume can therefore hand firmware stale pool bytes; the existing telemetry read path flushes after a device write for the same reason. Flush the buffer after a successful get.

	ret = aie4_msg_get_ctx_restore_pool(ndev, to_dma_addr(buf_hdl, 0),
					    to_buf_size(buf_hdl));

• Files reviewed: 8/8 changed files
• Comments generated: 3
• Review effort level: Lite

[donwalkarsoham]

🟡 Changes recommended

Unresolved critical and moderate findings affect parked-job recovery, DMA cache coherency, and suspended cleanup.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

drivers/accel/amdxdna/aie4_pci.c:472

• After GET_CTX_RESTORE_POOL firmware writes this DMA buffer, but this path never invalidates the CPU cache before retaining it for the later SET. On the non-coherent dma_alloc_noncoherent path, the resume can therefore hand firmware stale pool bytes; the existing telemetry read path flushes after a device write for the same reason. Flush the buffer after a successful get.

	ret = aie4_msg_get_ctx_restore_pool(ndev, to_dma_addr(buf_hdl, 0),
					    to_buf_size(buf_hdl));

• Files reviewed: 8/8 changed files
• Comments generated: 3
• Review effort level: Lite

INVALID. drm_clflush_virt_range() at line 469 already writes back and invalidates, and the CPU never touches the buffer between the GET and the SET. Its own cited precedent flushes after only because the CPU is about to read.

[Copilot]

Copilot review overview

🟡 Changes recommended

User-mode abort handling and bidirectional DMA mapping must be corrected.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• Advance UMS read index before returning on NO_RESTORE · New
• Map context restore pool as DMA_BIDIRECTIONAL · New

Resolved since last review (3)

• When saving the pool fails, this helper only clears the restore IDs. Both callers then continue…
• This PM acquire only protects the explicit destroy ioctl. Closing a DRM file while the device is…
• These tests only observe that a no-op command runs after resume; as this comment notes, the old…

[Copilot]

Copilot review overview

🔵 Needs a closer look

Firmware state persistence, runtime power management, and context teardown interactions require final hardware-backed human validation.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Map context restore pool as DMA_BIDIRECTIONAL
• Advance UMS read index before returning on NO_RESTORE