Add: HostApi register/unregister_device_memory_to_host (SVM host-map)#1324
Merged
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDevice-memory host mapping callbacks are added to the shared ChangesDevice Memory Host Mapping
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant HostApi
participant DeviceRunner
participant ascend_hal
Runtime->>HostApi: register_device_memory_to_host(dev_ptr, bytes)
HostApi->>DeviceRunner: forward registration request
DeviceRunner->>ascend_hal: halHostRegister with DEV_SVM_MAP_HOST
ascend_hal-->>DeviceRunner: host virtual address or failure
DeviceRunner-->>Runtime: host address or nullptr
Runtime->>HostApi: unregister_device_memory_from_host(dev_ptr)
HostApi->>DeviceRunner: forward unregistration request
DeviceRunner->>ascend_hal: halHostUnregister
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces register_device_memory_to_host and unregister_device_memory_from_host APIs to map and unmap device buffers into the host address space. This is implemented across the onboard and simulator runners, with the onboard implementation wrapping the lazy-loaded HAL's halHostRegister and halHostUnregister functions. The review feedback suggests improving robustness by validating that device_id_ is non-negative before invoking HAL functions, and making the unregistration function symmetric to the registration function by ensuring the HAL is loaded and logging errors if the unregister symbol is missing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/a2a3/platform/onboard/host/device_runner.cpp (1)
724-740: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog when
halHostUnregistersymbol is missing but HAL is loaded.
unregister_device_memory_from_hostsilently does nothing whenget_halHostUnregister()returnsnullptr. If the HAL is loaded (g_hal_handle != nullptr) but the symbol is missing, this is a real resource leak — the paired register succeeded but unregister can't run.register_device_memory_to_hostlogs the analogous symbol-not-found case; unregister should do the same for symmetry and debuggability.♻️ Proposed fix: add a warning log for symbol-missing case
HalHostUnregisterFn fn = get_halHostUnregister(); if (fn != nullptr) { int rc = fn(dev_ptr, device_id_); if (rc != 0) { LOG_ERROR( "unregister_device_memory_from_host: halHostUnregister failed for dev_ptr %p (rc=%d)", dev_ptr, rc ); } + } else if (g_hal_handle != nullptr) { + LOG_ERROR("unregister_device_memory_from_host: halHostUnregister symbol not found"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/a2a3/platform/onboard/host/device_runner.cpp` around lines 724 - 740, Add an else branch in DeviceRunner::unregister_device_memory_from_host after checking get_halHostUnregister(), logging a warning when the function pointer is null while g_hal_handle is non-null. Match the symbol-not-found logging behavior and context used by register_device_memory_to_host, while preserving the existing null-pointer and HAL-call error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/a2a3/platform/onboard/host/device_runner.cpp`:
- Around line 724-740: Add an else branch in
DeviceRunner::unregister_device_memory_from_host after checking
get_halHostUnregister(), logging a warning when the function pointer is null
while g_hal_handle is non-null. Match the symbol-not-found logging behavior and
context used by register_device_memory_to_host, while preserving the existing
null-pointer and HAL-call error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 663ea4fe-25c1-4a22-9ebc-3333c3560347
📒 Files selected for processing (7)
src/a2a3/platform/onboard/host/device_runner.cppsrc/a2a3/platform/onboard/host/device_runner.hsrc/common/platform/include/common/host_api.hsrc/common/platform/onboard/host/c_api_shared.cppsrc/common/platform/onboard/host/device_runner_base.hsrc/common/platform/sim/host/c_api_shared.cppsrc/common/platform/sim/host/device_runner_base.h
ChaoWao
force-pushed
the
hostapi-map-device-to-host
branch
from
f08c307 to
e0f9841
Compare
Adds a platform capability for mapping a device buffer into the host address space and returning a host-readable VA, exposed through the shared HostApi table so a host-side orchestrator can dereference control tensors whose buffer.addr is a device address. - DeviceRunnerBase (onboard) gains virtual register_device_memory_to_host / unregister_device_memory_from_host (base default: nullptr / no-op). - a2a3 onboard overrides them via halHostRegister(DEV_SVM_MAP_HOST) / halHostUnregister; a5 onboard uses the base default (no host-map path). - sim SimDeviceRunnerBase implements them as identity / no-op (its "device" pointer is already host-readable). - HostApi (common/host_api.h) gains the two function pointers; both c_api_shared.cpp tables (onboard + sim) wire wrappers that forward to current_runner(). The returned host VA may differ from dev_ptr, so callers must use it for host access, and pair every register with an unregister before free. No consumer in this change; host_build_graph will use it. Builds clean on all 4 platforms x 2 runtimes with -Werror.
ChaoWao
added a commit
to ChaoWao/simpler-fork
that referenced
this pull request
Jul 11, 2026
Introduces the host_build_graph (hbg) runtime: the host-orchestration variant of tensormap_and_ringbuffer. The orchestrator runs on the host to completion — building the whole task graph, relocating it to device addresses, and H2D'ing the image — then the device boots scheduler-only. Only the orchestration timing/location differs from tensormap; the data structures and dispatch are shared. Pipeline (host): stage tensors (device alloc + H2D) and host-map them so the host can read control tensors directly; dlopen the orchestration .so and run it; wire fanout inline during submit (build dep_pool/fanout_head + seed the ready queue on the host); relocate every cross-task pointer to its final device address (range-based, two-delta: SM region + arena region); H2D the populated SM + arena. Device: attach the already-device-addressed image and dispatch from the seeded ready queue; no on-device orchestrator, no pointer fixup, no execution-time reclaim. Completion is tracked by completed_tasks_, independent of last_task_alive. Simplifications enabled by host-orchestration: - No device-orch path (the device boots scheduler-only). - No execution-time reclaim: the whole graph is host-resident and runs once, so advance_ring_pointers / reset_for_reuse / the COMPLETED->CONSUMED flip are removed. Consumer waits key on fanout_refcount. - Single ring: with no reclaim and a whole-graph-resident ring, the per-scope-depth multi-ring split is gone (PTO2_MAX_RING_DEPTH == 1); rings[]/ring_id are physically removed. All scope depths map to ring 0. - Host reads device control tensors (e.g. paged_attention's context_lens / block_table) via the host-map (a2a3 halHostRegister identity map; sim is already a host pointer), so get_tensor_data dereferences buffer.addr directly. The host-map is consumed through HostApi::register_device_memory_to_host / unregister_device_memory_from_host (added in hw-native-sys#1324): a2a3 onboard wraps halHostRegister(DEV_SVM_MAP_HOST), sim is identity, and a5 onboard uses the base default (no host-map path). tensormap_and_ringbuffer and a5 runtimes are not modified. Verified on a2a3 onboard: paged_attention passes (host_build_graph suite, reading control tensors via the HostApi host-map); a2a3sim hbg scene suite 10 passed; cpput 54/54; all 8 runtime targets build with -Werror.
ChaoWao
added a commit
that referenced
this pull request
Jul 11, 2026
…p) (#1185) Introduces the host_build_graph (hbg) runtime: the host-orchestration variant of tensormap_and_ringbuffer. The orchestrator runs on the host to completion — building the whole task graph, relocating it to device addresses, and H2D'ing the image — then the device boots scheduler-only. Only the orchestration timing/location differs from tensormap; the data structures and dispatch are shared. Pipeline (host): stage tensors (device alloc + H2D) and host-map them so the host can read control tensors directly; dlopen the orchestration .so and run it; wire fanout inline during submit (build dep_pool/fanout_head + seed the ready queue on the host); relocate every cross-task pointer to its final device address (range-based, two-delta: SM region + arena region); H2D the populated SM + arena. Device: attach the already-device-addressed image and dispatch from the seeded ready queue; no on-device orchestrator, no pointer fixup, no execution-time reclaim. Completion is tracked by completed_tasks_, independent of last_task_alive. Simplifications enabled by host-orchestration: - No device-orch path (the device boots scheduler-only). - No execution-time reclaim: the whole graph is host-resident and runs once, so advance_ring_pointers / reset_for_reuse / the COMPLETED->CONSUMED flip are removed. Consumer waits key on fanout_refcount. - Single ring: with no reclaim and a whole-graph-resident ring, the per-scope-depth multi-ring split is gone (PTO2_MAX_RING_DEPTH == 1); rings[]/ring_id are physically removed. All scope depths map to ring 0. - Host reads device control tensors (e.g. paged_attention's context_lens / block_table) via the host-map (a2a3 halHostRegister identity map; sim is already a host pointer), so get_tensor_data dereferences buffer.addr directly. The host-map is consumed through HostApi::register_device_memory_to_host / unregister_device_memory_from_host (added in #1324): a2a3 onboard wraps halHostRegister(DEV_SVM_MAP_HOST), sim is identity, and a5 onboard uses the base default (no host-map path). tensormap_and_ringbuffer and a5 runtimes are not modified. Verified on a2a3 onboard: paged_attention passes (host_build_graph suite, reading control tensors via the HostApi host-map); a2a3sim hbg scene suite 10 passed; cpput 54/54; all 8 runtime targets build with -Werror.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a platform capability for mapping a device buffer into the host address space, returning a host-readable VA, exposed through the shared
HostApitable. This lets a host-side orchestrator dereference control tensors whosebuffer.addris a device address.DeviceRunnerBase(onboard) gains virtualregister_device_memory_to_host/unregister_device_memory_from_host(base default:nullptr/ no-op).halHostRegister(DEV_SVM_MAP_HOST)/halHostUnregister.HostApi(common/host_api.h) gains the two function pointers; bothc_api_shared.cpptables (onboard + sim) wire wrappers forwarding tocurrent_runner().Contract: the returned host VA may differ from
dev_ptr— callers must use it (notdev_ptr) for host access, and pair every register with an unregister beforefree_tensor.Context
This is the platform half of the
host_build_graph(host-orchestration) runtime: the host orchestrator needs to read control tensors (e.g. paged_attention'scontext_lens) that live in device memory. Landing the capability separately, with the two-function contract named for intent (register_device_memory_to_host) rather than the a2a3 HAL mechanism, keeps the interface clean and correct across all backends (a2a3 SVM, a5 shadow, sim identity).No consumer in this change — the hbg runtime PR (#1185) consumes it and drops its previous ad-hoc bridge.
Testing
-Werror(a2a3sim, a5sim, a2a3, a5).