Re: [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset

[Chris Wilson <chris@chris-wilson.co.uk>]

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@chris-wilson.co.uk>

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx