[PATCH 1/2] drm/i915: Silence the WARN if the user tries to GTT mmap an incoherent object

[PATCH 1/2] drm/i915: Silence the WARN if the user tries to GTT mmap an incoherent object

[Chris Wilson]

If the user tries to mmap through the GTT an object that is marked as
snooped, we report an error rather than allow the GPU to hang the
machine. The choice of EINVAL, however, was unfortunate as we turn that
into a WARN rather than a quiet SIGBUS.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index df3b964..2fb9c47 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1778,7 +1778,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	/* Access to snoopable pages through the GTT is incoherent. */
 	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
-		ret = -EINVAL;
+		ret = -EFAULT;
 		goto unlock;
 	}
 
-- 
1.7.9.5

[PATCH 2/2] drm/i915: Check for a NULL shared dpll before dereferencing

[Chris Wilson]

This doesn't look possible but a little extra defense against the
improbable is worth it - an oops here could lockup the machine.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 62fac7e..61fd887 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1751,6 +1751,9 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
+	if (WARN_ON(pll == NULL))
+		return;
+
 	WARN_ON(!pll->refcount);
 	if (pll->active == 0) {
 		DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
-- 
1.7.9.5

Re: [PATCH 2/2] drm/i915: Check for a NULL shared dpll before dereferencing

[Daniel Vetter]

On Wed, May 28, 2014 at 04:16:42PM +0100, Chris Wilson wrote:
> This doesn't look possible but a little extra defense against the
> improbable is worth it - an oops here could lockup the machine.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 62fac7e..61fd887 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1751,6 +1751,9 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
>  
> +	if (WARN_ON(pll == NULL))
> +		return;

Yeah, nicely completes our set of WARN checks. Also merged the other patch
for sigbus instead of loud WARN to dinq.

Thanks, Daniel
> +
>  	WARN_ON(!pll->refcount);
>  	if (pll->active == 0) {
>  		DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch