vfio/cxl: Advertise DPA mmap and update its selftest#486
Conversation
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint ❌ Errors foundDetailsChecking 1 commits... Cherry-pick digest: ┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐ │ Local │ Referenced upstream / Patch subject │ Patch-ID │ Subject │ SoB chain │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 0c40c2c481af │ [SAUCE] vfio/cxl: advertise dpa mmap and update its selftest │ N/A │ N/A │ mhonap │ └──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘ Lint: all checks passed. PR metadata: W: PR title missing [<branch>] prefix: "vfio/cxl: Advertise DPA mmap and update its selftest" E: PR targets 26.04_linux-nvidia-bos but body has no https://bugs.launchpad.net/... link |
BaseOS Kernel ReviewSummaryThe vfio/cxl selftest has two unchecked-return NULL/error paths: iova_allocator_init() result used without a NULL check, and iommu_map() failure ignored before an unconditional iommu_unmap(), both of which can crash the test. Minor comment/wording issues also noted. Findings: Critical: 0, High: 0, Medium: 2, Low: 2 Latest watcher review: open review Generated test plan: open test plan Kernel deb build: successful (download debs, 4 files) Head: This comment is maintained by nv-pr-bot. It is updated when the GitHub watcher publishes a newer review. |
|
@mmhonap |
|
This is coming from #448 NVIDIA: VR: SAUCE: vfio/cxl: Register regions with VFIO layer · NVIDIA/NV-Kernels@e658757 I thought it came from upstream but that is not true. Cc @kobak2026 |
kobak2026
left a comment
There was a problem hiding this comment.
I validated this on 10.103.232.135 with the patch applied on
top of 26.04_linux-nvidia-bos.
Host kernel:
7.0.0-2015-pr486-nvidia-bos-64k
Result:
vfio_cxl_type2_test passed 11/11 on 0002:81:00.0.
The updated dpa_mmap_fault test passes with:
DPA mmap + IOAS map ok: iova=0x0 size=0x200000
I also rebuilt the old pre-PR selftest behavior, where
dpa_mmap_fault mmap()s the DPA region and directly
dereferences the host pointer. That still reproduces the
original SIGBUS on the same PR kernel:
pass:10 fail:1 skip:0 error:0
dpa_mmap_fault: Test terminated unexpectedly by signal 7
So the evidence matches the commit rationale: the old
failure is from host CPU dereference of DPA, while the PR
tests the VMM path of mmap + IOAS mapping.
One review comment:
Since this patch requires DPA MMAP for the VMM/ATS path, the
selftest should probably fail if VFIO_REGION_INFO_FLAG_MMAP
is missing instead of SKIPing. Otherwise a future
regression that drops MMAP again would be hidden as a
skipped test.
Minor nit:
The test name dpa_mmap_fault is now stale. It no longer
verifies host CPU fault/dereference behavior; it verifies
DPA mmap + IOAS mapping. A rename or comment update would
make the intent clearer.
nirmoy
left a comment
There was a problem hiding this comment.
Acked-by: Nirmoy Das <nirmoyd@nvidia.com>
|
@mmhonap I forgot please add "NVIDIA: VR: SAUCE:" tag for the patch as this is a internal patch |
The DPA region needs VFIO_REGION_INFO_FLAG_MMAP so a VMM can map it into stage-2 for the device to reach over ATS. Without the flag the region stays a slow-path I/O region that the stage-2 listener skips, and the GPU's ATS access to DPA-resident memory never resolves. This is the same mmap capability the upstream series and nvgrace-gpu advertise for coherent device memory. Whether a host CPU coherent access to DPA succeeds is a platform property and is not gated by this flag. Advertising the flag lets any fd holder mmap DPA and touch it from the host CPU, so a memory error on that struct-page-less range would otherwise escalate to a host SError. Register the range with memory_failure() via register_pfn_address_space(), as nvgrace-gpu does, so such an error is contained to unmapping the range and a SIGBUS to the fd holder. The mmap flag stays required for the ATS path regardless; containment does not replace it. The dpa_mmap_fault selftest is skipped when the flag is absent. With the flag set it runs, but its raw host CPU load/store of DPA does not model how a VMM uses the region: the VMM mmaps DPA and maps it into the IOAS, and the guest reaches it through stage-2. Rewrite the test to do the same (mmap plus iommu_map), drop the host dereference, and fail rather than skip when the mmap flag is missing so a future regression is caught. Since the test now performs a different operation renamed the test to dpa_mmap_ioas_map matching what it verifies. Fixes: e658757 ("NVIDIA: VR: SAUCE: vfio/cxl: Register regions with VFIO layer") Signed-off-by: Manish Honap <mhonap@nvidia.com> Acked-by: Nirmoy Das <nirmoyd@nvidia.com>
|
Thank you for the comments Nirmoy/Koba.
OK cxl_type2.dpa_mmap_ioas_map
|
clsotog
left a comment
There was a problem hiding this comment.
Acked-by: Carol L Soto <csoto@nvidia.com>
The DPA region needs VFIO_REGION_INFO_FLAG_MMAP so a VMM can mmap it and map it into stage-2 for the device to reach over ATS. Without the flag the VMM keeps DPA as a slow-path I/O region that the stage-2 listener skips, and the GPU's ATS access to DPA-resident memory never resolves.
The dpa_mmap_fault selftest is skipped when the flag is absent. With the flag set it runs, but its raw host CPU load/store of DPA does not model how a VMM uses the region: the VMM mmaps DPA and maps it into the IOAS, and the guest reaches it through stage-2. Rewrite the test to do the same (mmap plus iommu_map) and drop the host dereference.