Quoting Abdiel Janulgue (2019-11-15 11:45:49)
> +static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + vm_fault_t vmf_ret;
> + unsigned long i, size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + int ret;
> +
> + if (!i915_gem_object_has_struct_page(obj))
> + return VM_FAULT_SIGBUS;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
It is probably a good idea to mention here how we revoke the PTE.
/* PTEs are revoked in obj->ops->put_pages() */
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + struct page *page = i915_gem_object_get_page(obj, i);
> +
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + page_to_pfn(page));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
> +
> + i915_gem_object_unpin_pages(obj);
> +
> + return vmf_ret;
With the revoke + selftests in place, this looks correct.
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
There might be room for fine tuning (not pin/mapping the whole object),
but that has been a concern for a long time and remains a concern :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
