Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing dma_sync_sgtable_for_cpu() in guest-bound transfer paths causes data corruption with SWIOTLB bounce buffers. -- --- Patch [1]: [PATCH] drm/virtio: use the DMA API for resource backing on Xen --- commit 5102ee1180b3819921a73a9ff7e4e18ce5a910f1 Author: Benjamin Leggett <[email protected]> drm/virtio: use the DMA API for resource backing on Xen This commit enables the use of the DMA API for virtio-gpu resource backing on Xen PV domains, regardless of the VIRTIO_F_ACCESS_PLATFORM feature bit. It introduces virtio_gpu_use_dma_api() and updates the resource initialization and transfer paths to use it. This ensures that the host receives correct DMA addresses instead of guest-physical addresses when swiotlb is active. > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c > b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 905b1f42cd983..568fefd713e0f 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c [ ... ] > @@ -1231,7 +1231,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct > virtio_gpu_device *vgdev, > struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]); > struct virtio_gpu_transfer_host_3d *cmd_p; > struct virtio_gpu_vbuffer *vbuf; > - bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev); > + bool use_dma_api = virtio_gpu_use_dma_api(vgdev->vdev); > > if (virtio_gpu_is_shmem(bo) && use_dma_api) > dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, > bo->base.sgt, DMA_TO_DEVICE); [Severity: High] This is a pre-existing issue, but while enabling the DMA API for Xen PV domains, does this leave guest-bound transfers vulnerable to data corruption with SWIOTLB bounce buffers? While this patch ensures dma_sync_sgtable_for_device() is called for host-bound transfers (DMA_TO_DEVICE), it appears that guest-bound transfers are missing the corresponding CPU synchronization: drivers/gpu/drm/virtio/virtgpu_vq.c:virtio_gpu_cmd_transfer_from_host_3d() { ... // Missing dma_sync_sgtable_for_cpu() or // dma_sync_sgtable_for_device(..., DMA_FROM_DEVICE) } If a user-space application maps a virtio-gpu SHMEM buffer and triggers a VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D via DRM_IOCTL_VIRTGPU_TRANSFER_FROM_HOST, will the CPU read stale data from the original backing pages? Because the driver never calls dma_sync_sgtable_for_cpu() when the transfer completes, it looks like the SWIOTLB will never copy the bounce buffer contents back to the original physical pages. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
