Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/shmem: wrong vmap flush order in drm_gem_shmem_prime_end_cpu_access() --
commit 4543a541900e1a7c8880d3f04fbf76393b153371 Author: Boris Brezillon <[email protected]> drm/shmem: Provide a generic {begin, end}_cpu_access() implementation This commit adds default dma_buf_ops implementations for map_dma_buf, unmap_dma_buf, begin_cpu_access, and end_cpu_access to drm_gem_shmem_helper so that CPU caches and vmap ranges are synchronized across the exporter and all mapped attachments. [Severity: Medium] In drivers/gpu/drm/drm_gem_shmem_helper.c, inside drm_gem_shmem_prime_end_cpu_access() (around line 1093), should flush_kernel_vmap_range(shmem->vaddr, shmem->base.size) be called before iterating over dma_buf->attachments and calling dma_sync_sgtable_for_device(attach->dev, sgt, dir)? Currently, drm_gem_shmem_prime_end_cpu_access() syncs each attachment's sg_table for the device first, then calls flush_kernel_vmap_range() on shmem->vaddr, and finally calls dma_sync_sgtable_for_device() on shmem->sgt. When a cacheable shmem GEM object (with map_wc set to false) is exported, mapped by an importing device via drm_gem_shmem_prime_map_dma_buf(), and also mapped into kernel virtual address space via drm_gem_shmem_vmap_locked(), modifications made through shmem->vaddr may sit in a virtually indexed or aliasing cache line (such as on 32-bit arm with vivt or vipt-aliasing caches and an outer l2 cache, or mips with primary d-cache aliasing). Calling dma_sync_sgtable_for_device() on the importer attachments before flush_kernel_vmap_range() means the virtual alias in shmem->vaddr has not yet been written back when dma_sync_sgtable_for_device() cleans the physical mapping, cleans the outer cache, or copies to swiotlb bounce buffers, which can leave importing devices reading stale data. As documented in Documentation/core-api/cachetlb.rst and done in other dma-buf exporters such as system_heap_dma_buf_end_cpu_access() and cma_heap_dma_buf_end_cpu_access(), flush_kernel_vmap_range() should happen before all dma_sync_sgtable_for_device() calls. Similarly, in drm_gem_shmem_prime_begin_cpu_access() (around line 1058), invalidate_kernel_vmap_range(shmem->vaddr, shmem->base.size) is placed between syncing shmem->sgt and syncing dma_buf->attachments. Should invalidate_kernel_vmap_range() be called after all dma_sync_sgtable_for_cpu() calls have completed so that both exporter and importer sg_table syncs are ordered consistently with respect to the kernel vmap range invalidation? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
