Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/xe: dma-buf VRAM mappings are not invalidated on device wedge - [High] drm/xe: PAT aliasing violation by mapping GFP_KERNEL dummy page as WC --
commit c757615e833a0d49b9d8ef7258c5d9bef1106610 Author: Arvind Yadav <[email protected]> drm/xe: Invalidate existing VRAM mappings on wedge This commit introduces a mechanism to drain active CPU faults and invalidate all tracked VRAM mappings when a device wedges. Subsequent faults use a per-BO dummy page, preventing valid PTEs from keeping access to VRAM. > CPU mappings created before a device wedge can keep valid PTEs and > continue accessing VRAM. Mapping new faults to a dummy page does not > replace these existing mappings. > > Use the common device I/O SRCU gate to wait for active CPU faults to > finish. Then invalidate all tracked VRAM mappings. Faults starting > after the wedge use the per-BO dummy page. [Severity: High] Can mapping the dummy page cause a Machine Check Exception or SIGBUS? When CPU faults occur on a VRAM BO after the device wedges, they trigger xe_bo_vm_dummy_page(): drivers/gpu/drm/xe/xe_bo.c:xe_bo_vm_dummy_page() { ... page = alloc_page(GFP_KERNEL | __GFP_ZERO); ... return vmf_insert_pfn_prot(vma, vmf->address, pfn, vma->vm_page_prot); } This allocates a standard system RAM page using GFP_KERNEL (which has Write-Back caching) and maps it into userspace using the VMA's original vma->vm_page_prot. For VRAM mappings, vma->vm_page_prot contains Write-Combine (WC) bits. Does mapping a WB page as WC create a conflicting PAT alias on x86? track_pfn_insert() may reject this for RAM pages, leading to a SIGBUS, or if bypassed, a hardware MCE. > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index 7902ce3fe0127..73dcd397dc13c 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c > @@ -4155,6 +4155,26 @@ void xe_bo_runtime_pm_release_mmap_offset(struct xe_bo > *bo) > list_del_init(&bo->vram_userfault_link); > } > > +/** > + * xe_bo_wedged_invalidate_mmaps - Invalidate CPU mappings backed by VRAM > + * @xe: xe device instance > + * > + * The caller must drain the common device I/O gate before calling this > + * function. Remove all tracked VRAM mappings so later faults map the > + * per-BO dummy page. > + */ > +void xe_bo_wedged_invalidate_mmaps(struct xe_device *xe) > +{ > + struct xe_bo *bo, *next; > + > + mutex_lock(&xe->mem_access.vram_userfault.lock); > + list_for_each_entry_safe(bo, next, > + &xe->mem_access.vram_userfault.list, > + vram_userfault_link) > + xe_bo_runtime_pm_release_mmap_offset(bo); [Severity: High] Does this code miss active dma-buf CPU mappings? xe_bo_runtime_pm_release_mmap_offset() only unmaps the VMA node associated with the DRM device file (drm_vma_node_unmap()). It doesn't seem to clear dma-buf mappings, which are tracked under bo->ttm.base.dma_buf->file->f_mapping. If a userspace application exports a VRAM BO as a dma-buf and maps it, would it retain CPU access to VRAM after the device wedges? Accessing VRAM on a wedged device can cause PCIe hangs or system crashes. Could a call to dma_buf_invalidate_mappings() be required here, similar to how it is handled in xe_bo_move_notify()? > + mutex_unlock(&xe->mem_access.vram_userfault.lock); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
