Fix: honor ASCEND_RT_VISIBLE_DEVICES at comm_hccl MemSetAccess sites - #1826
Fix: honor ASCEND_RT_VISIBLE_DEVICES at comm_hccl MemSetAccess sites#1826lterrac wants to merge 1 commit into
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughVMM access descriptors now convert ACL-logical device IDs to driver-visible IDs before memory access setup. Documentation records the field-specific ID rules. Visibility-isolation tests validate communication-domain mappings across remapped logical ranks. ChangesVMM device ID translation
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This change translates device identifiers for isolated-device memory access while preserving existing behavior for identity mappings; no actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Under `ASCEND_RT_VISIBLE_DEVICES` isolation a launcher renumbers the visible cards
to logical `0..N-1`. `comm_alloc_domain_windows` then fails on every card pair
except an identity mapping, aborting the domain with
domain_alloc_via_fabric: create local Fabric window -> 507899
507899 is `ACL_ERROR_RT_DRV_INTERNAL_ERROR`: the driver was handed a device id in
a numbering it does not use.
`aclrtMemAccessDesc::location.id` is consumed in the driver-visible space and must
translate, even though `aclrtMemSetAccess` is an ACL entry point. Its sibling
`aclrtPhysicalMemProp::location.id` must NOT — `aclrtMallocPhysical` rejects a
driver-visible id outright. Walking the window sequence one call at a time under
`ASCEND_RT_VISIBLE_DEVICES=1,2`, bound to logical 0 (card 1):
aclrtMemGetAllocationGranularity prop.location.id=0 (logical) -> 0
aclrtMallocPhysical prop.location.id=0 (logical) -> 0
aclrtMallocPhysical prop.location.id=2 (driver-visible) -> 107001
aclrtReserveMemAddress -> 0
aclrtMapMem -> 0
aclrtMemSetAccess desc.location.id=0 (logical) -> 507899
aclrtMemSetAccess desc.location.id=1 (driver-visible) -> 0
With the variable unset and card 1 bound directly the whole sequence completes.
Identical for `ACL_HBM_MEM_HUGE` (fabric) and `ACL_HBM_MEM_NORMAL` (ipc). The
asymmetry is per-field, not per-API, so `acl_hal_device.h` records both halves —
translating the memory property too would break allocation outright.
Three sites carry the id into `aclrtMemSetAccess`: a2a3's
`reserve_and_map_vmm_window`, the funnel for every local and imported window, and
a5's `alloc_windows_via_ipc` / `domain_alloc_via_ipc`, whose descriptors are
reused for their peer mappings.
The ST gains a comm-domain case beside the chip-init one, so both device-id layers
are covered: chip init never maps device memory, so it cannot catch this. It runs
on all granted cards rather than a single logical id, and keeps the existing
skip-on-identity guard — an identity grant makes the two id spaces the same number
and would pass without proving anything.
a2a3 verified on 910B2: with the stock runtime both ranks abort with 507899 on
cards 1,2; with this change they complete byte-exact, and an identity grant (cards
0,1) still passes. a5 has no hardware here and rides on CI.
Related: the peer-access-status confusion recorded at a5 comm_hccl.cpp:553-561 and
:972-977 (hw-native-sys#1018) is the same id-space split at a different call, left untouched.
1c2b5ba to
a3a3b35
Compare
1 participant
Problem
Under
ASCEND_RT_VISIBLE_DEVICESisolation,comm_alloc_domain_windowsfails on every card pair except an identity mapping:507899isACL_ERROR_RT_DRV_INTERNAL_ERROR— the driver was handed a device id in a numbering it does not use. Cards0,1hide it because there logical == physical, which is also why serving has never hit it: DeepSeek V4 takes all 8 cards.This is the class #1600 fixed, at a site it did not cover — neither arch's
comm_hccl.cppincludedcommon/acl_hal_device.h.Root cause — the two
location.idfields want different id spacesaclrtMemAccessDesc::location.idis consumed in the driver-visible space and must translate, even thoughaclrtMemSetAccessis an ACL entry point. Its siblingaclrtPhysicalMemProp::location.idmust not —aclrtMallocPhysicalrejects a driver-visible id outright.Measured by walking the window sequence one call at a time, in one process, under
ASCEND_RT_VISIBLE_DEVICES=1,2bound to logical0(= card 1):With the variable unset and card 1 bound directly, the whole sequence completes. Identical for
ACL_HBM_MEM_HUGE(fabric) andACL_HBM_MEM_NORMAL(ipc).The asymmetry is per-field, not per-API, so
acl_hal_device.hnow records both halves — a symmetric "fix" of the memory property would break allocation.Change
Three sites carry the id into
aclrtMemSetAccess:reserve_and_map_vmm_window— the funnel for every local and imported window, fabric and IPCalloc_windows_via_ipcanddomain_alloc_via_ipc— their descriptors are reused for the peer mappings, so the translation rides alongNo CMake change: both onboard host targets already carry
src/common/platform/include.Test
tests/st/vis_isolationgains a comm-domain case next to the chip-init one, so both device-id layers live in one module. Chip init never maps device memory, so it cannot catch this. The new case runs on all granted cards rather than a single logical id, and reuses the existing ascending-list rule and skip-on-identity guard — under an identity grant the two id spaces hold the same number and the test would pass without proving anything.Verification
a2a3, on 910B2 — A/B on the same granted cards in the same job, only the runtime differing:
1,2, logical 0,1create local Fabric window -> 5078990,1, logical 0,1The identity case matters: the broker hands out the lowest free cards, so a two-card request usually lands on exactly the mapping that proves nothing.
a5 — no a5 hardware available to me; it rides on
_st-npu-a5.yml. The change there is the same pattern as a2a3, applied to the two inlined copies.Related, not fixed here
The peer-access-status confusion already recorded in comments at a5
comm_hccl.cpp:553-561and:972-977(#1018) is the same id-space split at a different call (aclrtDevicePeerAccessStatus/aclrtDeviceEnablePeerAccesstake a peer id). Left untouched rather than widening this diff.