| Message ID | 1390855788-20422-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 27, 2014 at 09:49:47PM +0100, Daniel Vetter wrote: > Userspace can currently provoke this when e.g. trying to use a pinned > scanout as a cursor or overlay target. Later on that might lead to > some fun fence pin count mayhem. > > Spurred by Ville's report that something goes wrong here and > originally I've thought that this might slip through the pwrite gtt > fastpath. But that one checks of obj tiling, so should be ok. > > But one thing that _does_ blow up is the vma unbinding with more than > one address space. The next patch will fix this. It's buggy behaviour, so if (WARN_ON(fence->pin_count) return -EBUSY; I did worry that we would then have some unexpected failure, a new EBUSY returning to userspace, but I think along the paths that can trigger it, we have already returned EBUSY in the (dim and distant) past. -Chris
On Mon, Jan 27, 2014 at 10:26 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Mon, Jan 27, 2014 at 09:49:47PM +0100, Daniel Vetter wrote: >> Userspace can currently provoke this when e.g. trying to use a pinned >> scanout as a cursor or overlay target. Later on that might lead to >> some fun fence pin count mayhem. >> >> Spurred by Ville's report that something goes wrong here and >> originally I've thought that this might slip through the pwrite gtt >> fastpath. But that one checks of obj tiling, so should be ok. >> >> But one thing that _does_ blow up is the vma unbinding with more than >> one address space. The next patch will fix this. > > It's buggy behaviour, so if (WARN_ON(fence->pin_count) return -EBUSY; > > I did worry that we would then have some unexpected failure, a new EBUSY > returning to userspace, but I think along the paths that can trigger it, > we have already returned EBUSY in the (dim and distant) past. Indeed we seem to have the required tiling checks everywhere. At first I've had the WARN but then though userspace could sneak in through the cursor/overlay code, but it can't. I'll update the patch. -Daniel
On Mon, Jan 27, 2014 at 10:41:57PM +0100, Daniel Vetter wrote: > On Mon, Jan 27, 2014 at 10:26 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Mon, Jan 27, 2014 at 09:49:47PM +0100, Daniel Vetter wrote: > >> Userspace can currently provoke this when e.g. trying to use a pinned > >> scanout as a cursor or overlay target. Later on that might lead to > >> some fun fence pin count mayhem. > >> > >> Spurred by Ville's report that something goes wrong here and > >> originally I've thought that this might slip through the pwrite gtt > >> fastpath. But that one checks of obj tiling, so should be ok. > >> > >> But one thing that _does_ blow up is the vma unbinding with more than > >> one address space. The next patch will fix this. > > > > It's buggy behaviour, so if (WARN_ON(fence->pin_count) return -EBUSY; > > > > I did worry that we would then have some unexpected failure, a new EBUSY > > returning to userspace, but I think along the paths that can trigger it, > > we have already returned EBUSY in the (dim and distant) past. > > Indeed we seem to have the required tiling checks everywhere. At first > I've had the WARN but then though userspace could sneak in through the > cursor/overlay code, but it can't. I'll update the patch. I've tested these (+ Chris's WARN_ON) and the issue looks to be fixed. Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 05e79fa02db2..9559d33cd94b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3058,6 +3058,9 @@ i915_gem_object_put_fence(struct drm_i915_gem_object *obj) fence = &dev_priv->fence_regs[obj->fence_reg]; + if (fence->pin_count) + return -EBUSY; + i915_gem_object_fence_lost(obj); i915_gem_object_update_fence(obj, fence, false);
Userspace can currently provoke this when e.g. trying to use a pinned scanout as a cursor or overlay target. Later on that might lead to some fun fence pin count mayhem. Spurred by Ville's report that something goes wrong here and originally I've thought that this might slip through the pwrite gtt fastpath. But that one checks of obj tiling, so should be ok. But one thing that _does_ blow up is the vma unbinding with more than one address space. The next patch will fix this. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+)