docs: add AMD Instinct vGPU design draft (#1707) - #1985
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new design document for AMD Instinct vGPU support in Kubernetes, covering fractional GPU memory allocation, CU partitioning, scheduler-device-plugin annotations, resource mapping, limitations, and plugin layering discussion. ChangesAMD vGPU Design Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal document for AMD Instinct vGPU support in HAMi, detailing the motivation, goals, architecture approach, and resource allocation protocol. The review feedback focuses on minor grammatical, punctuation, and formatting improvements to enhance the readability and correctness of the documentation.
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.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/develop/amd-vgpu.md`:
- Around line 58-60: Add a language tag to the remaining fenced example blocks
in the AMD vGPU docs so they render cleanly and avoid MD040; update the fenced
snippets around the hami.io/amd-devices-allocated examples to use text fences
consistently, matching the other documentation examples in this file.
- Around line 16-20: The AMD GPU docs section on LD_AUDIT vs LD_PRELOAD needs a
source and tighter scope. Update the explanation in the documented ROCm/HIP area
to cite the upstream issue or release note that describes the LD_PRELOAD
symbol-resolution failure, and explicitly state that this workaround applies
only to the affected ROCm 7.x/HIP releases or the recursive interception failure
mode. Keep the existing NVIDIA LD_PRELOAD note unchanged while making the
ROCm-specific limitation clear in the same paragraph.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a7386b3a-3956-4869-a19b-03764911ff80
📒 Files selected for processing (1)
docs/develop/amd-vgpu.md
Design document for AMD Instinct GPU isolation (per-pod memory limiting + CU partitioning) on ROCm. Covers the scheduler<->device-plugin protocol (node/pod annotations), the CU-count -> ROC_GLOBAL_CU_MASK model, and open discussion points. Submitted as a design draft per discussion in Project-HAMi#1707. Signed-off-by: Kenji SHIMIZU <kenji.shimizu@midokura.com>
7b5fe21 to
68cbfef
Compare
…e tags) Signed-off-by: Kenji SHIMIZU <kenji.shimizu@midokura.com>
|
thanks @kenji-mido, i've assigned the development of device-plugin to @FouoF, he will finish his part in July, and find a digital-ocean environment to validate, after that, we will release that part on Aug, before the release of v2.10 |
Address review feedback on hami.io/amd-cu-mask delimiters (Project-HAMi#1985): switch the CU-mask annotation from a delimited <UUID>=<mask>:; string to a JSON array under the amd.com/ namespace, matching the per-device allocation-data convention used by other vendors (e.g. Ascend's huawei.com/<model>). Avoids the :/; collision with ROC_GLOBAL_CU_MASK's own grammar. Signed-off-by: Kenji SHIMIZU <kenji.shimizu@midokura.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Address review feedback (Project-HAMi#1985): AMDDevices.LockNode / ReleaseNodeLock currently return nil, so CU-bitmap exclusivity is not enforced yet. Change the wording from present tense to "will be enforced" and add a TODO note in both the protocol and resource-model sections. Signed-off-by: Kenji SHIMIZU <kenji.shimizu@midokura.com>
|
CC @mesutoezdil |
CC @mesutoezdil |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kenji-mido, Shouren The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The CU-mask annotation key is amd.com/cu-mask (per merged design doc Project-HAMi#1985), not hami.io/amd-cu-mask. Fix three stale references in comments/test message, and note reconstruction now happens in Fit (not AddResourceUsage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CU-mask annotation key is amd.com/cu-mask (per merged design doc Project-HAMi#1985), not hami.io/amd-cu-mask. Fix stale references in comments/test message, and note reconstruction happens in Fit (not AddResourceUsage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Could you share the current status of the AMD device-plugin? |
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Adds a design document (
docs/develop/amd-vgpu.md) for AMD Instinct GPU isolation(per-pod memory limiting + CU partitioning) on ROCm. It covers the scheduler<->device-plugin protocol
(node/pod annotations), the CU-count -> ROC_GLOBAL_CU_MASK model, and open discussion points.
Submitted as a design draft per the discussion in #1707
(#1707 (comment)).
Which issue(s) this PR fixes:
Relates to #1707
Special notes for your reviewer:
AI assistance disclosure: I used Claude Code to (double) fact-check claims against the HAMi source
(annotation keys, DeviceInfo fields, per-vendor conventions) and to draft wording.
The final text was authored, edited, and reviewed by me, and all technical claims were verified against the codebase.
Does this PR introduce a user-facing change?:
NONE
Summary by CodeRabbit