On 2026/09/29 11:22, Huang, Honglei wrote:
On 9/29/2026 1:25 AM, Akihiko Odaki wrote:
On 2026/09/29 1:22, Huang, Honglei wrote:
On 9/26/2026 7:33 PM, Akihiko Odaki wrote:
On 2026/09/24 18:55, Honglei Huang wrote:
Add userptr blob objects so the guest kernel can pin an existing
userspace mapping and advertise it as CREATE_BLOB backing entries.
- New virtio_gpu_object_userptr type for userptr resources
- Pin pages with pin_user_pages_fast() and FOLL_LONGTERM
- Omit FOLL_WRITE when VIRTGPU_BLOB_FLAG_USE_READONLY is set
- Charge FOLL_LONGTERM pins against RLIMIT_MEMLOCK
- DMA-map the scatterlist only when virtio_gpu_use_dma_api() is
required; use DMA_TO_DEVICE for USE_READONLY blobs
- Mark writable pages dirty when unpinning
- Keep pages pinned until RESOURCE_UNREF is queued; drop them from
cleanup_object() on the unref response or on create failure
- Clear userptr->pages on pin failure to avoid double-free on cleanup
- Reject unaligned or overflowing userptr ranges at create time
- Disallow PRIME export of userptr objects
- Save CREATE_BLOB params and restore userptr resources after
hibernation without using the shmem restore path
The hibernation restore path maps with the same attrs and direction
used at create time, as documented next to the call. Like shmem
blobs, the DMA API can still bounce through a buffer there. This
design only avoids a second guest side shmem allocation and memcpy,
nothing more.
I don't see the restore path is documented though this says
"documented next to the call".
Will fix in next version
...
+ goto err_cleanup;
+ }
+
+ userptr->sgt = sgt;
+
+ if (virtio_gpu_use_dma_api(vgdev->vdev)) {
+ enum dma_data_direction dir =
+ (userptr->flags & VIRTGPU_BLOB_FLAG_USE_READONLY) ?
+ DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
+
+ ret = dma_map_sgtable(drm_dev_dma_dev(vgdev->ddev), sgt,
+ dir, 0);
I checked the ROCm code and documentation, and I do not see how this
mapping satisfies the HIP coherence contract when bounce buffers or
explicit DMA cache maintenance are required.
HIP distinguishes two coherence models:
> Coarse-grained coherence: The memory is considered up-to-date only
> after synchronization performed using hipDeviceSynchronize(),
> hipStreamSynchronize(), or any blocking operation that acts on the
> null stream such as hipMemcpy(). To avoid the cache from being
> accessed by a part of the system while simultaneously being
written by
> another, the memory is made visible only after the caches have been
> flushed.
>
> Fine-grained coherence: The memory is coherent even while being
> modified by a part of the system. Fine-grained coherence ensures
that
> up-to-date data is visible to others regardless of kernel
boundaries.
> This can be useful if both host and device operate on the same data.
https://rocm.docs.amd.com/projects/HIP/en/docs-10.0.0/how-to/
hip_runtime_api/memory_management/coherence_control.html
Fine-grained access requires coherence throughout execution; copying
between the original pages and a bounce buffer only at kernel
boundaries
would not suffice. Coarse-grained access could support such copies, but
they must occur at the required synchronization points. I found no code
that synchronizes this DMA mapping at those points.
Whether dma_map_sgtable() bounces is decided by the platform's IOMMU/
SWIOTLB behind the device, not by we request. attrs=0 here just
matches the existing shmem blob path, and is exactly what native
(non- virtualized) ROCm's own userptr support does too:
// drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c,
kfd_mem_dmamap_userptr()
ret = dma_map_sgtable(adev->dev, ttm->sg, direction, 0);
And HIP's coarse-grained/fine-grained coherence isn't a DMA layer
property, it is a GPU VM/cache thing. In upstream KFD it's
implemented as GPU MMU page table attributes (MTYPE_CC/MTYPE_RW, the
SNOOPED bit), set by whichever driver programs the real GPU's page
tables for this memory:
// drivers/gpu/drm/amd/amdkfd/kfd_svm.c, svm_range_get_pte_flags()
mapping_flags |= coherent ? AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
I really want to disable the bounce buffers in DMA, but it is a
platform behaviour, can not disable it in this layer. And in XEN, the
dmapping for it is another form of iovector/simple sgtable. And if
use the virtio-iommu or somthing else, the efficiency and the
complexity of driver integration are both lower than using iovector/
sgtable directly.
And for how to handle the DMA thing for userptr, since we have so
many concern about it, maybe we can drop this part in next version.
But we may have the AI review warn/error.
Documentation/core-api/dma-attributes.rst says:
> DMA_ATTR_REQUIRE_COHERENT
> -------------------------
>
> DMA mapping requests with the DMA_ATTR_REQUIRE_COHERENT fail on any
> system where SWIOTLB or cache management is required. This should only
> be used to support uAPI designs that require continuous HW DMA
> coherence with userspace processes, for example RDMA and DRM. At a
> minimum the memory being mapped must be userspace memory from
> pin_user_pages() or similar.
>
> Drivers should consider using dma_mmap_pages() instead of this
> interface when building their uAPIs, when possible.
>
> It must never be used in an in-kernel driver that only works with
> kernel memory.
This is exactly our use case. Although the platform decides whether a
mapping requires bouncing or cache maintenance,
DMA_ATTR_REQUIRE_COHERENT lets the driver reject mappings that cannot
meet this requirement.
The shmem path serves a different use case, but amdkfd is a relevant
comparison. HIP's coherence guarantees depend on both the GPU VM/cache
configuration and the DMA mapping established by the kernel driver.
GPU page table attributes alone cannot make a bounce buffer coherent
with the original userspace pages.
The amdkfd mapping you cited therefore warrants discussion with its
developers as well. I have CCed the maintainer.
Will add DMA_ATTR_REQUIRE_COHERENT in virtio GPU userptr in next version.
And are there some places else need to be modified?
I don't have anything else to add right now. As a QEMU developer, I've
only reviewed the more obvious aspects of this patch, so others may have
deeper kernel-specific feedback.
At this stage, my main priority is seeing how the current feedback is
resolved. Before we dig deeper into the low-level details, I’d also like
to review the virtio specification changes alongside the
QEMU/virglrenderer implementation so I can better evaluate the overall
design.
Regards,
Akihiko Odaki
