On Wed, Sep 23, 2015 at 09:07:24PM +0100, Chris Wilson wrote:
> If the client revokes the virtual address it asked to be mapped into GPU
> space via userptr (by using anything along the lines of mmap, mprotect,
> madvise, munmap, ftruncate etc) the mmu notifier sends a range
> invalidate command to userptr. Upon receiving the invalidation signal
> for the revoked range, we try to release the struct pages we pinned into
> the GTT. However, this can fail if any of the GPU's VMA are pinned for
> use by the hardware (i.e. despite the user's intention we cannot
> relinquish the client's address range and keep uptodate with whatever is
> placed in there). Currently we emit a few WARN so that we would notice
> if this every occurred in the wild; it has. Sadly this means we need to
> replace those WARNs with the proper SIGBUS to the offending clients
> instead.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_userptr.c | 41 +++++++++++++++++++++++++++++----
> 1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index f75d90118888..efb404b9fe69 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -81,11 +81,44 @@ static void __cancel_userptr__worker(struct work_struct *work)
> was_interruptible = dev_priv->mm.interruptible;
> dev_priv->mm.interruptible = false;
>
> - list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link) {
> - int ret = i915_vma_unbind(vma);
> - WARN_ON(ret && ret != -EIO);
> + list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link)
> + i915_vma_unbind(vma);
> + if (i915_gem_object_put_pages(obj)) {
> + struct task_struct *p;
> +
> + DRM_ERROR("Unable to revoke ownership by userptr of"
> + " invalidated address range, sending SIGBUS"
> + " to attached clients.\n");
> +
> + rcu_read_lock();
> + for_each_process(p) {
> + siginfo_t info;
> +
> + /* This doesn't capture everyone who has
> + * the pages pinned behind a VMA as we
> + * do not have that tracking information
> + * available, it does however kill the
> + * original process (and siblings) who
> + * created the userptr and presumably tried
> + * to reuse the address space despite having
> + * pinned it (possibly indirectly) to the hw.
> + * Arguably, we don't even want to kill the
> + * other processes as they are not at fault,
> + * likely to be a display server, and hopefully
> + * will release the pages in due course once
> + * the client is dead.
> + */
> + if (p->mm != obj->userptr.mm->mm)
> + continue;
> +
> + info.si_signo = SIGBUS;
> + info.si_errno = 0;
> + info.si_code = BUS_ADRERR;
> + info.si_addr = (void __user *)obj->userptr.ptr;
> + force_sig_info(SIGBUS, &info, p);
> + }
> + rcu_read_unlock();
Why do we need to send a SIGBUS? It won't tear down the offending gem bo,
any new users will hopefully get it, and abusing SIGBUS without the thread
actually doing a memory access is a bit surprising. DRM_DEBUG seems to be
the most we can do here I think - I think userspace being able to do this
is just a fundamental property of userptr.
-Daniel
> }
> - WARN_ON(i915_gem_object_put_pages(obj));
>
> dev_priv->mm.interruptible = was_interruptible;
> }
> --
> 2.5.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
