From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0074C3E4C61; Fri, 14 Aug 2026 21:22:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786742524; cv=none; b=pAltMph2DO9A4y/KWrxMmOmp2LXD8Mueb5eFXFx39XgrmJbS7Edq29tJpReAfGQXD86LoZHk1t6jBg+eOiMPQlrtNOt2O37vGetVI5wXxIew4qY46IC1VGP91EdFRmuN9DzCT9ACV2c023z0qFEPeggoTUi35UqunwBvxY1zleI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786742524; c=relaxed/simple; bh=Sdwy4fsW1rrgJc9P9RdMJgmx4Sf8jQJkOxzxA8zS+I8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:To:Cc; b=A7JbGRPBhTNcSGPWhhZU0BnZ4GT4ZLhrYSNfTzwRuLj4ZRwjdyqoKQnVwhHUI7A4warswOpcRpOqfJEPFThZz/r+UXLqAkXQpbkPzyys1xRqIj1fpv8iKqF7cs8W+3oyfyUZvDR9J/4qt7stSzWRZyBw4sZQv6EeZ4OMoX0+Lvw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=YOwMpOrw; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="YOwMpOrw" Received: by smtp.kernel.org (Postfix) with ESMTPS id 7DB15C2BCC7; Fri, 14 Aug 2026 21:22:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1786742523; bh=Sdwy4fsW1rrgJc9P9RdMJgmx4Sf8jQJkOxzxA8zS+I8=; h=From:Date:Subject:To:Cc:Reply-To:From; b=YOwMpOrw8L9qsKsvk3fwfx1mBnYSVlbMlWjQsj0tHgFrm93rNJOtaHIACe7+qZD34 n+5rjuaUE7orfl62D2E2kPp3pTxWLhuryTVqp0M+vgw2HE9fbu9lIGss8GrOIF0127 ZD8oxNYfa+6bCb5nCPTHjxZFLwX1U/XVMUg7tmIa0qrDmyVMn16m6Dt3tBxWOJfcUj D+8Xb0wWRKT4h+muOaa4vfFIYVTQyb8p3/qlZgggo6I+csvUhV7OSWX0nUtJDK16uu wdaFV4ch9H7pUNwOTkp5A2EzFVTGnQsMwQyfBt3PBlUEQ619Coy3aKejcvFITueYen KeXxOaO0RBG3A== Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6823EC5CFCF; Fri, 14 Aug 2026 21:22:03 +0000 (UTC) From: Benjamin Leggett via B4 Relay Date: Fri, 14 Aug 2026 17:21:55 -0400 Subject: [PATCH v2] drm/virtio: sync shmem backing on guest-bound transfers Precedence: bulk X-Mailing-List: virtualization@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20260814-virtgpu-from-host-sync-v2-1-fa3910caf3e5@edera.io> X-B4-Tracking: v=1; b=H4sIAAAAAAAC/4WOSw6CMBCGr0K6dpQiaOPKexgWfQwwC1oyxUZCu LsFD+DiX3zJ/1pFRCaM4lGsgjFRpOAzVKdC2EH7HoFcZlGV1a1UsoZEPPfTGzoOIwwhzhAXb0F JZ1xl741VtcjhibGjz1H8ajMbHREMa2+Hvc7xeMmCkaKFnzGbBopz4OU4k+Qe/bubJMg8XuNVd 6YxpXqiQ9ZnCqLdtu0LAnTngeAAAAA= X-Change-ID: 20260814-virtgpu-from-host-sync-81dbd2c75c84 To: David Airlie , Gerd Hoffmann , Dmitry Osipenko , Gurchetan Singh , Chia-I Wu , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Simona Vetter Cc: dri-devel@lists.freedesktop.org, virtualization@lists.linux.dev, linux-kernel@vger.kernel.org, Sashiko AI review , Benjamin Leggett X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1786742522; l=10431; i=benjamin@edera.io; s=20260806; h=from:subject:message-id; bh=Q48RitstjVkcmNz3grfIAtP0/bAL8w0Cxpw+3k9Fn6c=; b=uleENpmff90A1ssc7GyWl1gtU0zp1W7/ug4AxedIFDGOzapqXg/9y0+ytyy0k8MwolUuD00RF Dc3J5lCLD+YB0hv7GAgrDepGwcRjJbn+d1yqzpB/qnHCpQvlqP7yghF X-Developer-Key: i=benjamin@edera.io; a=ed25519; pk=prj3m3DtYeYw1akLHnM/sXm8eau0Ar/IO93YoYnOr+I= X-Endpoint-Received: by B4 Relay for benjamin@edera.io/20260806 with auth_id=923 X-Original-From: Benjamin Leggett Reply-To: benjamin@edera.io From: Benjamin Leggett virtio_gpu_cmd_transfer_to_host_{2d,3d}() sync the shmem backing for the device before the transfer, but nothing syncs for the CPU when a transfer runs the other way. That breaks two ways. Where the DMA layer bounces, the device writes into the bounce buffer while the guest keeps reading the original pages. Where DMA is not coherent, the device writes memory while the CPU keeps stale cache lines, because nothing reaches arch_sync_dma_for_cpu(). Either way DRM_IOCTL_VIRTGPU_TRANSFER_FROM_HOST hands back stale data. Sashiko originally found this in https://lore.kernel.org/dri-devel/20260806231002.27B4D1F000E9@smtp.kernel.org but the suggestion there to fix this with dma_sync_sgtable_for_cpu() isn't a sufficient fix, for two reasons. - The transfer is asynchronous. virtio_gpu_cmd_transfer_from_host_3d() only queues the command, so a sync there would run before the device had written anything. It belongs on completion, and ahead of any fence signalling. A waiter woken by the fence would otherwise race the sync and read the backing pages regardless. It needs its own pass over the reclaim list rather than a step inside the existing one, because virtio_gpu_fence_event_process() also signals every earlier fence in the same context, so any entry in that loop may signal an earlier entry's fence. - The transfer is also partial, carrying an offset, a level and a box. Where the mapping bounces, a sync for the CPU copies the whole mapping back, so unless the mapping is primed first the regions the device did not write come back holding whatever the bounce buffer contained, discarding data the guest owned. So the fix: Prime the mapping before queueing, tag the vbuffer, and sync for the CPU on completion before the fence is signalled. A second transfer must not snapshot the mapping while an earlier one is still in flight, or the snapshot would predate whatever the CPU wrote once the earlier fence signalled and the later sync would discard it. To mitigate this, wait for outstanding fences under the reservation before priming. Neither sync copies anything unless the mapping genuinely bounces: swiotlb_sync_single_for_cpu() and its Xen counterpart look the address up in the bounce pool and return early when it is absent. On a platform with non-coherent DMA they still perform the necessary cache maintenance. The range cannot be narrowed to the box, since for a non-blob resource virtio_gpu_transfer_from_host_ioctl() rejects a caller-supplied stride and layer_stride, leaving the layout to the host and the guest with no way to work out which bytes the device writes. A host3d guest blob does carry both, so its extent could be bounded, but the sync is left whole there too rather than special-cased: priming makes the untouched regions round-trip unchanged either way. Behaviour changes worth noting: - TRANSFER_FROM_HOST can now block, where before it returned as soon as the command was queued. Repeated readbacks of one resource serialise, and a readback can wait behind an earlier queued command that touched it, since virtio_gpu_array_add_fence() tags uploads, execbufs and plane flushes alike with DMA_RESV_USAGE_WRITE. -ERESTARTSYS was already possible here via dma_resv_lock_interruptible(). - A CPU write racing an in-flight transfer to the same resource is now lost, where before it survived and the transfer was lost instead. Priming captures the pages as of queueing, so a write landing before completion is overwritten by the sync. - TRANSFER_TO_HOST can also block now, but only while a guest-bound transfer on the same resource is outstanding, which the flag confines to callers that issue both without waiting. - Where a batch of completions contains a guest-bound transfer, the sync pass delays fence signalling for the whole batch. Only bounced pages are copied and the swiotlb pool bounds it. A batch with no such transfer is unaffected. Tested under QEMU on x86 with swiotlb=force and virtio-vga-gl iommu_platform=on, which forces both preconditions required to hit the original bug. Reported-by: Sashiko AI review Closes: https://lore.kernel.org/dri-devel/20260806231002.27B4D1F000E9@smtp.kernel.org/ Signed-off-by: Benjamin Leggett --- This depends on 6a736d2f9d0c ("drm/virtio: use the DMA API for resource backing on Xen"), currently in drm-misc-fixes only, so it needs to go through the same branch. --- Changes in v2: - drm/virtio: add guard on virtio_gpu_transfer_to_host_ioctl. - Link to v1: https://lore.kernel.org/r/20260814-virtgpu-from-host-sync-v1-1-814e3afb5b08@edera.io --- drivers/gpu/drm/virtio/virtgpu_drv.h | 5 +++++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 40 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/virtio/virtgpu_vq.c | 37 +++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 626aadf680bd..5faff0d1861d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -114,6 +114,8 @@ struct virtio_gpu_object { bool dumb; bool created; bool attached; + /* a guest-bound transfer is queued and its mapping not yet synced */ + bool from_host_pending; bool host3d_blob, guest_blob; uint32_t blob_mem, blob_flags; @@ -192,6 +194,9 @@ struct virtio_gpu_vbuffer { struct list_head list; uint32_t seqno; + + /* guest-bound transfer whose shmem backing needs a CPU sync */ + bool sync_for_cpu; }; struct virtio_gpu_output { diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 3d8e4ccdb7c1..a041b8e336e9 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; + } + } + fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); if (!fence) { ret = -ENOMEM; @@ -320,6 +339,27 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, if (ret != 0) goto err_put_free; + if (READ_ONCE(bo->from_host_pending)) { + /* + * A transfer the other way has queued but not yet + * synced its mapping. Pushing the guest pages into it + * now would discard what the device wrote there, so + * wait for that sync: it runs before the fence it + * belongs to is signalled. The flag is only set under + * this reservation, so it cannot appear behind our + * back; a stale read merely waits for nothing. + */ + 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; + } + } + ret = -ENOMEM; fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 5e9b7b192db0..0f5317f3da8a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -256,6 +256,24 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work) } while (!virtqueue_enable_cb(vgdev->ctrlq.vq)); spin_unlock(&vgdev->ctrlq.qlock); + /* + * Sync guest-bound transfers before signalling anything, so that a + * waiter cannot read the backing pages while what the device wrote is + * still in a bounce buffer. This cannot be folded into the loop below: + * virtio_gpu_fence_event_process() also signals every earlier fence in + * the same context, so any entry there may signal this entry's fence. + */ + list_for_each_entry(entry, &reclaim_list, list) { + if (entry->sync_for_cpu) { + struct virtio_gpu_object *bo = + gem_to_virtio_gpu_obj(entry->objs->objs[0]); + + dma_sync_sgtable_for_cpu(vgdev->vdev->dev.parent, + bo->base.sgt, DMA_FROM_DEVICE); + WRITE_ONCE(bo->from_host_pending, false); + } + } + list_for_each_entry(entry, &reclaim_list, list) { resp = (struct virtio_gpu_ctrl_hdr *)entry->resp_buf; @@ -1238,12 +1256,31 @@ void virtio_gpu_cmd_transfer_from_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_gpu_use_dma_api(vgdev->vdev); cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p)); vbuf->objs = objs; + if (virtio_gpu_is_shmem(bo) && use_dma_api) { + /* + * The device writes only the requested box, so prime the + * mapping with the current contents: otherwise the sync on + * completion would hand back whatever a bounce buffer held for + * the regions the device does not touch. + */ + dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, + bo->base.sgt, DMA_TO_DEVICE); + vbuf->sync_for_cpu = true; + /* + * Set under the reservation the caller holds, so a transfer + * the other way cannot miss it and push the guest pages into + * the mapping while the device still owns it. + */ + WRITE_ONCE(bo->from_host_pending, true); + } + cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D); cmd_p->hdr.ctx_id = cpu_to_le32(ctx_id); cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle); --- base-commit: 6a736d2f9d0c6e6217fe7532bc4c50ceca71db78 change-id: 20260814-virtgpu-from-host-sync-81dbd2c75c84 Best regards, -- Benjamin Leggett From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Leggett Date: Fri, 14 Aug 2026 17:21:55 -0400 Subject: [PATCH v2] drm/virtio: sync shmem backing on guest-bound transfers MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20260814-virtgpu-from-host-sync-v2-1-fa3910caf3e5@edera.io> X-B4-Tracking: v=1; b=H4sIAAAAAAAC/4WOSw6CMBCGr0K6dpQiaOPKexgWfQwwC1oyxUZCu LsFD+DiX3zJ/1pFRCaM4lGsgjFRpOAzVKdC2EH7HoFcZlGV1a1UsoZEPPfTGzoOIwwhzhAXb0F JZ1xl741VtcjhibGjz1H8ajMbHREMa2+Hvc7xeMmCkaKFnzGbBopz4OU4k+Qe/bubJMg8XuNVd 6YxpXqiQ9ZnCqLdtu0LAnTngeAAAAA= X-Change-ID: 20260814-virtgpu-from-host-sync-81dbd2c75c84 To: David Airlie , Gerd Hoffmann , Dmitry Osipenko , Gurchetan Singh , Chia-I Wu , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Simona Vetter Cc: dri-devel@lists.freedesktop.org, virtualization@lists.linux.dev, linux-kernel@vger.kernel.org, Sashiko AI review , Benjamin Leggett X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1786742522; l=10431; i=benjamin@edera.io; s=20260806; h=from:subject:message-id; bh=Q48RitstjVkcmNz3grfIAtP0/bAL8w0Cxpw+3k9Fn6c=; b=uleENpmff90A1ssc7GyWl1gtU0zp1W7/ug4AxedIFDGOzapqXg/9y0+ytyy0k8MwolUuD00RF Dc3J5lCLD+YB0hv7GAgrDepGwcRjJbn+d1yqzpB/qnHCpQvlqP7yghF X-Developer-Key: i=benjamin@edera.io; a=ed25519; pk=prj3m3DtYeYw1akLHnM/sXm8eau0Ar/IO93YoYnOr+I= X-Endpoint-Received: by B4 Relay for benjamin@edera.io/20260806 with auth_id=923 List-Id: B4 Relay Submissions virtio_gpu_cmd_transfer_to_host_{2d,3d}() sync the shmem backing for the device before the transfer, but nothing syncs for the CPU when a transfer runs the other way. That breaks two ways. Where the DMA layer bounces, the device writes into the bounce buffer while the guest keeps reading the original pages. Where DMA is not coherent, the device writes memory while the CPU keeps stale cache lines, because nothing reaches arch_sync_dma_for_cpu(). Either way DRM_IOCTL_VIRTGPU_TRANSFER_FROM_HOST hands back stale data. Sashiko originally found this in https://lore.kernel.org/dri-devel/20260806231002.27B4D1F000E9@smtp.kernel.org but the suggestion there to fix this with dma_sync_sgtable_for_cpu() isn't a sufficient fix, for two reasons. - The transfer is asynchronous. virtio_gpu_cmd_transfer_from_host_3d() only queues the command, so a sync there would run before the device had written anything. It belongs on completion, and ahead of any fence signalling. A waiter woken by the fence would otherwise race the sync and read the backing pages regardless. It needs its own pass over the reclaim list rather than a step inside the existing one, because virtio_gpu_fence_event_process() also signals every earlier fence in the same context, so any entry in that loop may signal an earlier entry's fence. - The transfer is also partial, carrying an offset, a level and a box. Where the mapping bounces, a sync for the CPU copies the whole mapping back, so unless the mapping is primed first the regions the device did not write come back holding whatever the bounce buffer contained, discarding data the guest owned. So the fix: Prime the mapping before queueing, tag the vbuffer, and sync for the CPU on completion before the fence is signalled. A second transfer must not snapshot the mapping while an earlier one is still in flight, or the snapshot would predate whatever the CPU wrote once the earlier fence signalled and the later sync would discard it. To mitigate this, wait for outstanding fences under the reservation before priming. Neither sync copies anything unless the mapping genuinely bounces: swiotlb_sync_single_for_cpu() and its Xen counterpart look the address up in the bounce pool and return early when it is absent. On a platform with non-coherent DMA they still perform the necessary cache maintenance. The range cannot be narrowed to the box, since for a non-blob resource virtio_gpu_transfer_from_host_ioctl() rejects a caller-supplied stride and layer_stride, leaving the layout to the host and the guest with no way to work out which bytes the device writes. A host3d guest blob does carry both, so its extent could be bounded, but the sync is left whole there too rather than special-cased: priming makes the untouched regions round-trip unchanged either way. Behaviour changes worth noting: - TRANSFER_FROM_HOST can now block, where before it returned as soon as the command was queued. Repeated readbacks of one resource serialise, and a readback can wait behind an earlier queued command that touched it, since virtio_gpu_array_add_fence() tags uploads, execbufs and plane flushes alike with DMA_RESV_USAGE_WRITE. -ERESTARTSYS was already possible here via dma_resv_lock_interruptible(). - A CPU write racing an in-flight transfer to the same resource is now lost, where before it survived and the transfer was lost instead. Priming captures the pages as of queueing, so a write landing before completion is overwritten by the sync. - TRANSFER_TO_HOST can also block now, but only while a guest-bound transfer on the same resource is outstanding, which the flag confines to callers that issue both without waiting. - Where a batch of completions contains a guest-bound transfer, the sync pass delays fence signalling for the whole batch. Only bounced pages are copied and the swiotlb pool bounds it. A batch with no such transfer is unaffected. Tested under QEMU on x86 with swiotlb=force and virtio-vga-gl iommu_platform=on, which forces both preconditions required to hit the original bug. Reported-by: Sashiko AI review Closes: https://lore.kernel.org/dri-devel/20260806231002.27B4D1F000E9@smtp.kernel.org/ Signed-off-by: Benjamin Leggett --- This depends on 6a736d2f9d0c ("drm/virtio: use the DMA API for resource backing on Xen"), currently in drm-misc-fixes only, so it needs to go through the same branch. --- Changes in v2: - drm/virtio: add guard on virtio_gpu_transfer_to_host_ioctl. - Link to v1: https://lore.kernel.org/r/20260814-virtgpu-from-host-sync-v1-1-814e3afb5b08@edera.io --- drivers/gpu/drm/virtio/virtgpu_drv.h | 5 +++++ drivers/gpu/drm/virtio/virtgpu_ioctl.c | 40 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/virtio/virtgpu_vq.c | 37 +++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 626aadf680bd..5faff0d1861d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -114,6 +114,8 @@ struct virtio_gpu_object { bool dumb; bool created; bool attached; + /* a guest-bound transfer is queued and its mapping not yet synced */ + bool from_host_pending; bool host3d_blob, guest_blob; uint32_t blob_mem, blob_flags; @@ -192,6 +194,9 @@ struct virtio_gpu_vbuffer { struct list_head list; uint32_t seqno; + + /* guest-bound transfer whose shmem backing needs a CPU sync */ + bool sync_for_cpu; }; struct virtio_gpu_output { diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 3d8e4ccdb7c1..a041b8e336e9 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; + } + } + fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); if (!fence) { ret = -ENOMEM; @@ -320,6 +339,27 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, if (ret != 0) goto err_put_free; + if (READ_ONCE(bo->from_host_pending)) { + /* + * A transfer the other way has queued but not yet + * synced its mapping. Pushing the guest pages into it + * now would discard what the device wrote there, so + * wait for that sync: it runs before the fence it + * belongs to is signalled. The flag is only set under + * this reservation, so it cannot appear behind our + * back; a stale read merely waits for nothing. + */ + 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; + } + } + ret = -ENOMEM; fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 5e9b7b192db0..0f5317f3da8a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -256,6 +256,24 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work) } while (!virtqueue_enable_cb(vgdev->ctrlq.vq)); spin_unlock(&vgdev->ctrlq.qlock); + /* + * Sync guest-bound transfers before signalling anything, so that a + * waiter cannot read the backing pages while what the device wrote is + * still in a bounce buffer. This cannot be folded into the loop below: + * virtio_gpu_fence_event_process() also signals every earlier fence in + * the same context, so any entry there may signal this entry's fence. + */ + list_for_each_entry(entry, &reclaim_list, list) { + if (entry->sync_for_cpu) { + struct virtio_gpu_object *bo = + gem_to_virtio_gpu_obj(entry->objs->objs[0]); + + dma_sync_sgtable_for_cpu(vgdev->vdev->dev.parent, + bo->base.sgt, DMA_FROM_DEVICE); + WRITE_ONCE(bo->from_host_pending, false); + } + } + list_for_each_entry(entry, &reclaim_list, list) { resp = (struct virtio_gpu_ctrl_hdr *)entry->resp_buf; @@ -1238,12 +1256,31 @@ void virtio_gpu_cmd_transfer_from_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_gpu_use_dma_api(vgdev->vdev); cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p)); vbuf->objs = objs; + if (virtio_gpu_is_shmem(bo) && use_dma_api) { + /* + * The device writes only the requested box, so prime the + * mapping with the current contents: otherwise the sync on + * completion would hand back whatever a bounce buffer held for + * the regions the device does not touch. + */ + dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, + bo->base.sgt, DMA_TO_DEVICE); + vbuf->sync_for_cpu = true; + /* + * Set under the reservation the caller holds, so a transfer + * the other way cannot miss it and push the guest pages into + * the mapping while the device still owns it. + */ + WRITE_ONCE(bo->from_host_pending, true); + } + cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D); cmd_p->hdr.ctx_id = cpu_to_le32(ctx_id); cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle); --- base-commit: 6a736d2f9d0c6e6217fe7532bc4c50ceca71db78 change-id: 20260814-virtgpu-from-host-sync-81dbd2c75c84 Best regards, -- Benjamin Leggett