Using pfn_mkwrite breaks KVM with "error: kvm run failed Bad address". Seen on a Mali-G610 GPU. Fix this by marking writable mmapped pages as written when installing the mapping in the page table.
v2: - remove obsolete comment new vmf_insert_pfn_pmd() (Boris) - fix style (Boris) Suggested-by: Boris Brezillon <[email protected]> Signed-off-by: Thomas Zimmermann <[email protected]> Reported-by: Igor Torrente <[email protected]> Closes: https://lore.kernel.org/dri-devel/[email protected]/raw Tested-by: Igor Torrente <[email protected]> Reviewed-by: Boris Brezillon <[email protected]> Fixes: 28e3918179aa ("drm/gem-shmem: Track folio accessed/dirty status in mmap") --- drivers/gpu/drm/drm_gem_shmem_helper.c | 51 +++++++------------------- 1 file changed, 13 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 545933c7f712..75e9970f443d 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -554,21 +554,6 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, } EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create); -static void drm_gem_shmem_record_mkwrite(struct vm_fault *vmf) -{ - struct vm_area_struct *vma = vmf->vma; - struct drm_gem_object *obj = vma->vm_private_data; - struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); - loff_t num_pages = obj->size >> PAGE_SHIFT; - pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */ - - if (drm_WARN_ON(obj->dev, !shmem->pages || page_offset >= num_pages)) - return; - - file_update_time(vma->vm_file); - folio_mark_dirty(page_folio(shmem->pages[page_offset])); -} - static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order, unsigned long pfn) { @@ -581,23 +566,9 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order, if (aligned && folio_test_pmd_mappable(page_folio(pfn_to_page(pfn)))) { - vm_fault_t ret; - pfn &= PMD_MASK >> PAGE_SHIFT; - /* Unlike PTEs which are automatically upgraded to - * writeable entries, the PMD upgrades go through - * .huge_fault(). Make sure we pass the "write" info - * along in that case. - * This also means we have to record the write fault - * here, instead of in .pfn_mkwrite(). - */ - ret = vmf_insert_pfn_pmd(vmf, pfn, - vmf->flags & FAULT_FLAG_WRITE); - if (ret == VM_FAULT_NOPAGE && (vmf->flags & FAULT_FLAG_WRITE)) - drm_gem_shmem_record_mkwrite(vmf); - - return ret; + return vmf_insert_pfn_pmd(vmf, pfn, false); } #endif } @@ -635,9 +606,20 @@ static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf, unsigned int ord pfn = page_to_pfn(page); ret = try_insert_pfn(vmf, order, pfn); - if (ret == VM_FAULT_NOPAGE) + if (ret == VM_FAULT_NOPAGE) { folio_mark_accessed(folio); + /* + * Immediately record write access to the buffer. The + * natural place would be pfn_mkwrite at the time when + * the access happens, but this breaks KVM. + */ + if (vma->vm_flags & VM_WRITE) { + file_update_time(vma->vm_file); + folio_mark_dirty(folio); + } + } + out: dma_resv_unlock(obj->resv); @@ -683,12 +665,6 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma) drm_gem_vm_close(vma); } -static vm_fault_t drm_gem_shmem_pfn_mkwrite(struct vm_fault *vmf) -{ - drm_gem_shmem_record_mkwrite(vmf); - return 0; -} - const struct vm_operations_struct drm_gem_shmem_vm_ops = { .fault = drm_gem_shmem_fault, #ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP @@ -696,7 +672,6 @@ const struct vm_operations_struct drm_gem_shmem_vm_ops = { #endif .open = drm_gem_shmem_vm_open, .close = drm_gem_shmem_vm_close, - .pfn_mkwrite = drm_gem_shmem_pfn_mkwrite, }; EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops); -- 2.54.0
