Repository navigation
fix(rocr/thunk): Revoke SVM-API GPU access attributes on deregister - #6699
Merged
Merged
Conversation
saleelk
requested review from
cfreeamd,
dayatsin-amd and
kentrussell
as code owners
June 2, 2026 16:34
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves ROCr thunk SVM-API memory lifecycle handling by tracking host VA ranges registered via the KFD SVM API and explicitly revoking GPU access / coherency attributes when the final registration is deregistered, addressing previously silent no-op deregistration behavior.
Changes:
- Add refcounted bookkeeping for SVM-API-registered host ranges (aligned base + size) protected by a new mutex in the FMM context.
- On final deregistration of a tracked range, issue the inverse SVM SET_ATTR sequence (NO_ACCESS for all GPUs + clear COHERENT/EXT_COHERENT flags).
- Free any remaining SVM-API range bookkeeping during process aperture teardown.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dayatsin-amd
approved these changes
Jun 12, 2026
amd-yangp
reviewed
Jun 15, 2026
amd-yangp
reviewed
Jun 15, 2026
amd-yangp
reviewed
Jun 15, 2026
amd-yangp
reviewed
Jun 15, 2026
amd-yangp
left a comment
amd-yangp
left a comment
Contributor
There was a problem hiding this comment.
Feel like the new svm_api_range track in Thunk is not needed since this is used to find the original range start, size when unregister svm range, KFD already handles the range split, overlap cases.
dayatsin-amd
force-pushed
the
users/saleelk/FmmUnreg
branch
from
June 16, 2026 02:29
08d8d89 to
6fac797
Compare
kentrussell
approved these changes
Jun 16, 2026
Contributor
|
Hello @dayatsin-amd, Can you please review 1168b96?
Thanks! |
ashetaia-amd
force-pushed
the
users/saleelk/FmmUnreg
branch
from
June 18, 2026 20:41
619205a to
1168b96
Compare
dayatsin-amd
force-pushed
the
users/saleelk/FmmUnreg
branch
from
June 19, 2026 03:06
1168b96 to
cd6acba
Compare
dayatsin-amd
requested changes
Jun 19, 2026
ashetaia-amd
force-pushed
the
users/saleelk/FmmUnreg
branch
3 times, most recently
from
June 19, 2026 17:07
d103f34 to
06d52c4
Compare
kentrussell
force-pushed
the
users/saleelk/FmmUnreg
branch
from
June 22, 2026 21:44
70bcb7d to
a1a1330
Compare
cfreeamd
approved these changes
Jul 7, 2026
saleelk
force-pushed
the
users/saleelk/FmmUnreg
branch
from
July 20, 2026 15:44
a1a1330 to
868c967
Compare
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Host ranges registered via the KFD SVM API (fmm_register_mem_svm_api) only set VA-range attributes and never create a vm_object, so deregister was a silent no-op leaving stale ACCESS_IN_PLACE and COHERENT attributes on the kernel-side SVM range after unpin. Track SVM-API ranges in an rb-tree (aligned base/size, refcounted). On the last deregistration of a base, compute the sub-ranges not covered by any other surviving registration and issue the inverse SET_ATTR: NO_ACCESS for every GPU plus clearing COHERENT/EXT_COHERENT flags. Same-base re-registration with a different size is rejected (INVALID_PARAMETER) so the extent to revoke is unambiguous. Includes kfdtest coverage for refcount, mismatched re-register rejection, and overlapping page-rounded registrations. Co-authored-by: Amir Shetaia <Amir.Shetaia@amd.com> Co-authored-by: David Yat Sin <David.YatSin@amd.com>
saleelk
force-pushed
the
users/saleelk/FmmUnreg
branch
from
July 20, 2026 15:54
868c967 to
32bf9fe
Compare
saleelk
changed the title
Distinct sub-page host buffers can share a page-aligned base but round to different page-aligned spans (small calloc'd buffers packed into one page, one straddling into the next). The same-base/different-size reject wrongly failed the second registration, regressing Unit_hipHostRegister_Nbuf_MultiFlag_RegisterUnregister. Deregister is keyed by the user pointer, so these are distinguishable: refcount them together on the base and grow the tracked extent to the largest span, so deregister revokes the full extent once the last owner is dropped. Overlap with other bases is still resolved by the interval subtraction in svm_api_range_put. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
Add KFDSVMRangeTest.SVMApiSharedPageTest guarding the calloc-packed pattern that regressed: two sub-page buffers sharing a page base with different page-aligned spans must both register and be revoked correctly. Update SVMApiDeregisterTest Phase 1, which previously asserted the now-removed reject, to assert the accepted + refcounted + grow behavior. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
dayatsin-amd
self-requested a review
July 21, 2026 21:05
dayatsin-amd
approved these changes
Jul 21, 2026
amd-chiranjeevi
added a commit
that referenced
this pull request
Jul 24, 2026
…gister" (#9187) Reverts PR: #6699 Fixes Issue: #9202 Test Run: https://github.com/ROCm/TheRock/actions/runs/30089750380/job/89484623849?pr=6839 --> all the math lib tests passed
Closed
1 task done
shwetagkhatri
pushed a commit
that referenced
this pull request
Aug 14, 2026
Registration only set the coherency flags and relied on the SVM range's default access. Deregister sets NO_ACCESS per GPU, and allocators such as jemalloc recycle the VA without munmap, so that NO_ACCESS persists on the kernel SVM range past the original registration. A later register at the reused VA never cleared it, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer once enough register/deregister churn built up - this is what caused the hip-tests hangs that reverted #6699 under ASan/XNACK. Re-grant ACCESS_IN_PLACE on every GPU at register time so a reused VA regains access. ACCESS_IN_PLACE (not ACCESS) is required: this is registered host (userptr) memory that cannot migrate, and ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
ashetaia-amd
added a commit
that referenced
this pull request
Aug 14, 2026
Registration only set the coherency flags and relied on the SVM range's default access. Deregister sets NO_ACCESS per GPU, and allocators such as jemalloc recycle the VA without munmap, so that NO_ACCESS persists on the kernel SVM range past the original registration. A later register at the reused VA never cleared it, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer once enough register/deregister churn built up - this is what caused the hip-tests hangs that reverted #6699 under ASan/XNACK. Re-grant ACCESS_IN_PLACE on every GPU at register time so a reused VA regains access. ACCESS_IN_PLACE (not ACCESS) is required: this is registered host (userptr) memory that cannot migrate, and ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
ashetaia-amd
added a commit
that referenced
this pull request
Aug 19, 2026
Registration only set the coherency flags and relied on the SVM range's default access. Deregister sets NO_ACCESS per GPU, and allocators such as jemalloc recycle the VA without munmap, so that NO_ACCESS persists on the kernel SVM range past the original registration. A later register at the reused VA never cleared it, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer once enough register/deregister churn built up - this is what caused the hip-tests hangs that reverted #6699 under ASan/XNACK. Re-grant ACCESS_IN_PLACE on every GPU at register time so a reused VA regains access. ACCESS_IN_PLACE (not ACCESS) is required: this is registered host (userptr) memory that cannot migrate, and ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
ashetaia-amd
added a commit
that referenced
this pull request
Aug 19, 2026
Registration only set the coherency flags and relied on the SVM range's default access. Deregister sets NO_ACCESS per GPU, and allocators such as jemalloc recycle the VA without munmap, so that NO_ACCESS persists on the kernel SVM range past the original registration. A later register at the reused VA never cleared it, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer once enough register/deregister churn built up - this is what caused the hip-tests hangs that reverted #6699 under ASan/XNACK. Re-grant ACCESS_IN_PLACE on every GPU at register time so a reused VA regains access. ACCESS_IN_PLACE (not ACCESS) is required: this is registered host (userptr) memory that cannot migrate, and ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
ashetaia-amd
added a commit
that referenced
this pull request
Aug 20, 2026
Registration only set the coherency flags and relied on the SVM range's default access. Deregister sets NO_ACCESS per GPU, and allocators such as jemalloc recycle the VA without munmap, so that NO_ACCESS persists on the kernel SVM range past the original registration. A later register at the reused VA never cleared it, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer once enough register/deregister churn built up - this is what caused the hip-tests hangs that reverted #6699 under ASan/XNACK. Re-grant ACCESS_IN_PLACE on every GPU at register time so a reused VA regains access. ACCESS_IN_PLACE (not ACCESS) is required: this is registered host (userptr) memory that cannot migrate, and ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
14 tasks done
17 tasks done
ammallya
pushed a commit
that referenced
this pull request
Sep 24, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- Co-authored-by: Cursor <cursoragent@cursor.com>
anujshuk-amd
pushed a commit
that referenced
this pull request
Sep 25, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- Co-authored-by: Cursor <cursoragent@cursor.com>
dgaliffiAMD
pushed a commit
that referenced
this pull request
Sep 25, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Host ranges registered via the KFD SVM API (fmm_register_mem_svm_api)
only set VA-range attributes and never create a vm_object, so deregister
was a silent no-op leaving stale ACCESS_IN_PLACE and COHERENT attributes
on the kernel-side SVM range after unpin.
Track SVM-API ranges in an rb-tree (aligned base/size, refcounted). On
the last deregistration of a base, compute the sub-ranges not covered by
any other surviving registration and issue the inverse SET_ATTR:
NO_ACCESS for every GPU plus clearing COHERENT/EXT_COHERENT flags.
Same-base re-registration with a different size is rejected
(INVALID_PARAMETER) so the extent to revoke is unambiguous.
Includes kfdtest coverage for refcount, mismatched re-register rejection,
and overlapping page-rounded registrations.
Co-authored-by: Amir Shetaia Amir.Shetaia@amd.com
Co-authored-by: David Yat Sin David.YatSin@amd.com