Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Using DMA_RESV_USAGE_WRITE for dma_resv_wait_timeout() during a guest-bound transfer breaks implicit synchronization with concurrent readers. --
--- Patch [1]: [PATCH v3] drm/virtio: sync shmem backing on guest-bound transfers --- commit 7807a88173cac8b6af42b38366a2631991ee3af0 Author: Benjamin Leggett <[email protected]> drm/virtio: sync shmem backing on guest-bound transfers This commit fixes an issue where guest-bound transfers provide stale data by adding CPU synchronization for shmem backing. It primes the mapping before queueing the transfer and tags the buffer to be synced on completion. It also introduces waits for outstanding fences under the reservation to prevent overwriting during in-flight transfers. Reported-by: Sashiko AI review <[email protected]> Closes: https://lore.kernel.org/dri-devel/[email protected]/ > --- 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); [Severity: High] Does using DMA_RESV_USAGE_WRITE here break implicit synchronization by bypassing READ fences? If the wait uses the WRITE enum, the iterator might only wait for KERNEL and WRITE fences. If an exporter is actively reading the pages and has set a DMA_RESV_USAGE_READ fence, the host transfer could run concurrently with the reader. Could this cause dma_sync_sgtable_for_cpu() to overwrite the GEM object's pages while they are being read? Should this use DMA_RESV_USAGE_READ instead to ensure we wait for concurrent readers? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
