Re: [PATCH] drm/i915: Remove incorrect restriction on 32bit offsets in ppGTT backend

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org,
	Mika Kuoppala <mika.kuoppala@intel.com>
Subject: Re: [PATCH] drm/i915: Remove incorrect restriction on 32bit offsets in ppGTT backend
Date: Mon, 4 May 2015 17:04:01 +0200	[thread overview]
Message-ID: <20150504150401.GS30184@phenom.ffwll.local> (raw)
In-Reply-To: <1430207283-16129-1-git-send-email-chris@chris-wilson.co.uk>

On Tue, Apr 28, 2015 at 08:48:03AM +0100, Chris Wilson wrote:
> This is the wrong layer to apply an arbitrary restriction and the wrong
> error code (object too large!). If we do want to prevent large offsets
> being return to the user on 32bit systems (to hide bugs in userspace),
> you want to restrict the drm_mm range manager instead. This first tells
> userspace about the correct size of the GTT they can use (so they don't
> try and overallocate object or batches), and fixes the eviction logic to
> avoid the eventual and *guaranteed* error.
> 
> Fixes regression in
> commit d7b2633dba04ef0fd7385f02a7b552abc5f1062f
> Author: Michel Thierry <michel.thierry@intel.com>
> Date:   Wed Apr 8 12:13:34 2015 +0100
> 
>     drm/i915/gen8: Dynamic page table allocations
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 968e8f9dc6dd..e96a694413a6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -844,15 +844,6 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>  	uint32_t pdpe;
>  	int ret;
>  
> -#ifndef CONFIG_64BIT
> -	/* Disallow 64b address on 32b platforms. Nothing is wrong with doing
> -	 * this in hardware, but a lot of the drm code is not prepared to handle
> -	 * 64b offset on 32b platforms.
> -	 * This will be addressed when 48b PPGTT is added */
> -	if (start + length > 0x100000000ULL)
> -		return -E2BIG;
> -#endif

Aggreed, we already limit the ggtt size appropriately. If this would slip
through it'd be a bug in drm_mm. Queued for -next, thanks for the patch.
-Daniel

> -
>  	/* Wrap is never okay since we can only represent 48b, and we don't
>  	 * actually use the other side of the canonical address space.
>  	 */
> -- 
> 2.1.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx