Repository navigation
Amd: phytium: update phytium amd controller driver support to 6.6.0.4 - #2242
wangchenlu2236 wants to merge 2 commits into
Conversation
With phytium virtual gpu acceleration method in qemu windows guest OS, guest OS driver needs to share GPU device memory with host and requires the shared buffer mapping remains valid, but the ttm map implemenataion did not account for this usecase. So we add a ref count for every pages of the shared buffer. Mainline: NA Signed-off-by: Liu Tao <liutao@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
In Phyfusion DirectX 11 API remoting, bad address error occurs in qemu when running some d3d11 demos. The patch fixed this issue. Mainline: NA Signed-off-by: Liu Tao <liutao@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the AMD GPU support for Phytium 6.6.0.4 by adding a SoC-specific uncached mmap workaround and balancing page references around TTM page population, helping guest memory visibility while avoiding address errors in D3D11 remoting. Sequence diagram for Phytium AMD GPU mmap workaroundsequenceDiagram
participant App as D3D11 remoting
participant GEM as amdgpu_gem_object_mmap
participant SoC as Phytium SoC
participant TTM as drm_gem_ttm_mmap
App->>GEM: amdgpu_gem_object_mmap
GEM->>SoC: is_phytium_soc()
alt Phytium SoC
GEM->>GEM: vm_flags_set(VM_ALLOW_ANY_UNCACHED)
end
GEM->>TTM: drm_gem_ttm_mmap
TTM-->>App: GPU buffer mapping
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @wangchenlu2236. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c" line_range="269" />
<code_context>
+#ifdef CONFIG_ARCH_PHYTIUM
+ /*lt: workaround for bad address error with d3d11 api remoting*/
+ if (is_phytium_soc())
+ vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED);
+#endif
return drm_gem_ttm_mmap(obj, vma);
</code_context>
<issue_to_address>
**Unsafe device memory attributes crash host**
When a guest maps a device PFN from an AMDGPU GEM buffer whose backing is unsafe for Normal-NC access on a Phytium host, `amdgpu_gem_object_mmap()` sets `VM_ALLOW_ANY_UNCACHED`, so KVM maps the device PFN as Normal-NC; guest access triggers an uncontained hardware fault and crashes the host.
Set `VM_ALLOW_ANY_UNCACHED` only for mappings whose backing is safe for Normal-NC access.
</issue_to_address>
### Comment 2
<location path="drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c" line_range="1208" />
<code_context>
- for (i = 0; i < ttm->num_pages; ++i)
+ for (i = 0; i < ttm->num_pages; ++i) {
ttm->pages[i]->mapping = NULL;
+ page_ref_dec(ttm->pages[i]);
+ }
</code_context>
<issue_to_address>
**Shared mappings see reused pages**
When a host or guest mapping remains active when TTM unpopulates its pages, `amdgpu_ttm_tt_unpopulate()` drops the added reference before `ttm_pool_free()` returns, clears, reuses, or frees the pages without waiting for external references, so the mapping sees changed contents or reclaimed memory.
Keep pages unavailable to the pool and defer freeing them until external mappings are gone.
Also at `drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1174`.
</issue_to_address>| #ifdef CONFIG_ARCH_PHYTIUM | ||
| /*lt: workaround for bad address error with d3d11 api remoting*/ | ||
| if (is_phytium_soc()) | ||
| vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED); |
There was a problem hiding this comment.
🔴 Critical · Unsafe device memory attributes crash host
When a guest maps a device PFN from an AMDGPU GEM buffer whose backing is unsafe for Normal-NC access on a Phytium host, amdgpu_gem_object_mmap() sets VM_ALLOW_ANY_UNCACHED, so KVM maps the device PFN as Normal-NC; guest access triggers an uncontained hardware fault and crashes the host.
Set VM_ALLOW_ANY_UNCACHED only for mappings whose backing is safe for Normal-NC access.
Prompt for AI agents
In `drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c` at line 269:
**Unsafe device memory attributes crash host**
When a guest maps a device PFN from an AMDGPU GEM buffer whose backing is unsafe for Normal-NC access on a Phytium host, `amdgpu_gem_object_mmap()` sets `VM_ALLOW_ANY_UNCACHED`, so KVM maps the device PFN as Normal-NC; guest access triggers an uncontained hardware fault and crashes the host.
Set `VM_ALLOW_ANY_UNCACHED` only for mappings whose backing is safe for Normal-NC access.| for (i = 0; i < ttm->num_pages; ++i) | ||
| for (i = 0; i < ttm->num_pages; ++i) { | ||
| ttm->pages[i]->mapping = NULL; | ||
| page_ref_dec(ttm->pages[i]); |
There was a problem hiding this comment.
🟠 High · Shared mappings see reused pages
When a host or guest mapping remains active when TTM unpopulates its pages, amdgpu_ttm_tt_unpopulate() drops the added reference before ttm_pool_free() returns, clears, reuses, or frees the pages without waiting for external references, so the mapping sees changed contents or reclaimed memory.
Keep pages unavailable to the pool and defer freeing them until external mappings are gone.
Also at drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1174.
Prompt for AI agents
In `drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c` at line 1208:
**Shared mappings see reused pages**
When a host or guest mapping remains active when TTM unpopulates its pages, `amdgpu_ttm_tt_unpopulate()` drops the added reference before `ttm_pool_free()` returns, clears, reuses, or frees the pages without waiting for external references, so the mapping sees changed contents or reclaimed memory.
Keep pages unavailable to the pool and defer freeing them until external mappings are gone.
Also at `drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1174`.
There was a problem hiding this comment.
🟡 Changes recommended
TTM can recycle higher-order blocks while KVM still holds non-head-page references, allowing a later put_page() to free pool-owned memory and cause corruption.
1 open issue
What changed in this PR
Updates AMDGPU’s memory-management paths to support GPU memory access from virtual-machine guests and improve D3D11 API remoting compatibility on Phytium platforms.
Changes:
- Add page-reference increments and decrements during TTM population and unpopulation.
- Set
VM_ALLOW_ANY_UNCACHEDfor GEM mappings on detected Phytium SoCs.
| File | Description |
|---|---|
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |
Adjust page references around allocation-pool use to support guest memory access. |
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |
Allow additional uncached KVM mapping types on Phytium systems. |
Review effort: Max
| for (i = 0; i < ttm->num_pages; ++i) | ||
| for (i = 0; i < ttm->num_pages; ++i) { | ||
| ttm->pages[i]->mapping = NULL; | ||
| page_ref_dec(ttm->pages[i]); |
This patches updates the support for phytium amd controller driver.
1.amd-ttm: amd gpu memory visibility for vm guest
2.amd-gem: Add VM_ALLOW_ANY_UNCACHED flag to mmap
Summary by Sourcery
Improve AMDGPU support on Phytium platforms by addressing guest memory visibility and GPU memory mapping compatibility.
New Features:
Bug Fixes: