[PATCH] drm/radeon/kms: forbid big bo allocation (fdo 31708) v3

[Michel Dänzer]

On Fre, 2010-12-03 at 16:38 -0500, jglisse at redhat.com wrote: 
> From: Jerome Glisse <jglisse at redhat.com>
> 
> Forbid allocating buffer bigger than visible VRAM or GTT, also
> properly set lpfn field.
> 
> v2 - use max macro
>    - silence warning
> v3 - don't explicitly set range limit
>    - use min macro
> 
> Cc: stable <stable at kernel.org>
> 
> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
> ---
>  drivers/gpu/drm/radeon/radeon_object.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 1d06774..a598d00 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -91,7 +91,8 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>  {
>  	struct radeon_bo *bo;
>  	enum ttm_bo_type type;
> -	int page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> +	unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> +	unsigned long max_size = 0;

The initialization of max_size is superfluous.


> @@ -104,6 +105,14 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>  	}
>  	*bo_ptr = NULL;
>  
> +	/* maximun bo size is the minimun btw visible vram and gtt size */
> +	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> +	if ((page_align << PAGE_SHIFT) >= max_size) {
> +		printk(KERN_WARNING "%s:%d alloc size %ldM bigger than %ldMb limit\n",
> +			__func__, __LINE__, page_align  >> (20 - PAGE_SHIFT), max_size >> 20);
> +		return -ENOMEM;
> +	}

Still not sure this simple minimum is quite the right solution... e.g.
if GTT is larger than visible VRAM, what's the problem with having a BO
that fits in GTT but not in visible VRAM? On systems with 8 MB of VRAM,
this will effectively limit the maximum texture size to 2048x1024.

(That's ignoring the fact that userspace can't use the full size of GTT
or VRAM due to the BOs pinned to visible VRAM and the ring and indirect
buffers at the start of GTT)


P.S. I think it's better not to submit patches to stable at kernel.org
while they're being reviewed. Once a patch lands in mainline, the Cc:
stable at kernel.org tag in the commit message will cause it to be queued
for the stable branch(es).

-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer