Re: [PATCH v3] drm/virtio: sync shmem backing on guest-bound transfers

[sashiko-bot@kernel.org]

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 <benjamin@edera.io>

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 <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/dri-devel/20260806231002.27B4D1F000E9@smtp.kernel.org/

> --- 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/20260814-virtgpu-from-host-sync-v3-1-f2538afd7d6e@edera.io?part=1