Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] DMA mappings are leaked in migration error paths because the destination page array is zeroed before unmapping. - [High] DMA unmapping occurs after pages are exposed to the CPU, violating cache coherency rules. -- --- Patch [1]: [PATCH] drm/pagemap: Reset migration page count on eviction retry --- commit f4a68acf50f418cad59d91b95424edfe21778ec5 Author: Arvind Yadav <[email protected]> drm/pagemap: Reset migration page count on eviction retry This commit updates drm_pagemap_evict_to_ram() to reset the mpages counter to 0 at the retry label. This ensures that retried eviction attempts do not incorrectly proceed using the page count from the previous attempt. > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 15c78eca180b7..695401cb2da24 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c [ ... ] > @@ -1121,6 +1121,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem > *devmem_allocation) > npages = devmem_allocation->size >> PAGE_SHIFT; > > retry: > + mpages = 0; > if (!mmget_not_zero(devmem_allocation->mm)) > return -EFAULT; > Further down in drm_pagemap_evict_to_ram(): > err_finalize: > if (err) > drm_pagemap_migration_unlock_put_pages(npages, dst); [Severity: High] This isn't a bug introduced by this patch, but does this error path leak DMA mappings? If an error occurs (such as a failure to copy to RAM), drm_pagemap_migration_unlock_put_pages() is called, which zeroes the destination page array entries in dst. Subsequently, when drm_pagemap_migrate_unmap_pages() iterates over these entries, it skips unmapping because the page evaluates to NULL: drm_pagemap_migrate_unmap_pages() { ... struct page *page = migrate_pfn_to_page(migrate_pfn[i]); if (!page || !pagemap_addr[i].addr || dma_mapping_error(dev, pagemap_addr[i].addr)) goto next; ... } Could this cause the DMA addresses to remain permanently mapped in the IOMMU or SWIOTLB? > migrate_device_pages(src, dst, npages); > migrate_device_finalize(src, dst, npages); > drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, > dst, npages, > DMA_FROM_DEVICE, &state); [Severity: High] This is also a pre-existing issue, but is the ordering of CPU exposure and DMA unmapping safe here? migrate_device_finalize() is called before drm_pagemap_migrate_unmap_pages(). The finalize function removes migration PTEs and restores normal CPU page table entries, allowing userspace to immediately access the migrated pages. However, the DMA API requires that dma_unmap_page() completes before the CPU accesses the memory. Because dma_unmap_page() for DMA_FROM_DEVICE mappings invalidates the CPU cache on non-coherent architectures, executing it after the CPU has been granted access could destroy recent CPU writes or cause the CPU to read stale speculative cache lines. The identical unsafe ordering appears to be present in __drm_pagemap_migrate_to_ram() where migrate_vma_finalize() is called. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
