Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Data corruption occurs in `virtio_gpu_transfer_to_host_ioctl` due to immediate bounce buffer snapshotting without waiting for in-flight transfers. --
--- Patch [1]: [PATCH] drm/virtio: sync shmem backing on guest-bound transfers --- commit 785780bb521416de7cc6cc47fe1be44a03a3d772 Author: Benjamin Leggett <[email protected]> Subject: drm/virtio: sync shmem backing on guest-bound transfers This commit fixes syncing issues for guest-bound transfers by priming the mapping before queueing and syncing for the CPU on completion. It also adds a wait before priming to prevent a second transfer from snapshotting while an earlier one is in flight. Link: https://lore.kernel.org/dri-devel/[email protected]/ > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > index 3d8e4ccdb7c1f..88320275eadbf 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > @@ -261,6 +261,25 @@ static int virtio_gpu_transfer_from_host_ioctl(struct > drm_device *dev, > if (ret != 0) > goto err_put_free; > > + if (virtio_gpu_is_shmem(bo) && virtio_gpu_use_dma_api(vgdev->vdev)) { > + /* > + * The sync on completion restores the whole mapping, so an > + * earlier transfer has to be done before this one snapshots it. > + * Otherwise the snapshot predates anything the CPU wrote once > + * that transfer's fence signalled, and the later sync would > + * discard it. Nothing can add a fence behind our back here, > + * since doing so takes the reservation we already hold. > + */ > + long wait = dma_resv_wait_timeout(objs->objs[0]->resv, > + DMA_RESV_USAGE_WRITE, true, > + MAX_SCHEDULE_TIMEOUT); > + > + if (wait < 0) { > + ret = wait; > + goto err_unlock; > + } > + } > + [Severity: High] Does virtio_gpu_transfer_to_host_ioctl() need a similar wait added? If a user concurrently issues a transfer to the host while a transfer from the host is in flight for the same buffer, could it overwrite the bounce buffer with stale memory? virtio_gpu_transfer_to_host_ioctl() calls virtio_gpu_cmd_transfer_to_host_3d() without a dma_resv_wait_timeout() check. This triggers a call to dma_sync_sgtable_for_device(), which copies the entire sgtable and might erase the device's ongoing writes before the in-flight transfer completes: virtio_gpu_transfer_to_host_ioctl() -> virtio_gpu_cmd_transfer_to_host_3d() { ... if (virtio_gpu_is_shmem(bo) && use_dma_api) dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, bo->base.sgt, DMA_TO_DEVICE); ... } > fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); > if (!fence) { > ret = -ENOMEM; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
