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

[Michel Dänzer]

On Fre, 2010-11-19 at 16:34 -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
> 
> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
> ---
>  drivers/gpu/drm/radeon/radeon_object.c |   34 ++++++++++++++++++++++++++-----
>  1 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 1d06774..c2fa64c 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -69,18 +69,28 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>  	u32 c = 0;
>  
>  	rbo->placement.fpfn = 0;
> -	rbo->placement.lpfn = rbo->rdev->mc.active_vram_size >> PAGE_SHIFT;
> +	rbo->placement.lpfn = 0;
>  	rbo->placement.placement = rbo->placements;
>  	rbo->placement.busy_placement = rbo->placements;
> -	if (domain & RADEON_GEM_DOMAIN_VRAM)
> +	if (domain & RADEON_GEM_DOMAIN_VRAM) {
> +		rbo->placement.lpfn = max((unsigned)rbo->placement.lpfn, (unsigned)rbo->rdev->mc.active_vram_size >> PAGE_SHIFT);
>  		rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>  					TTM_PL_FLAG_VRAM;
> -	if (domain & RADEON_GEM_DOMAIN_GTT)
> +	}
> +	if (domain & RADEON_GEM_DOMAIN_GTT) {
> +		rbo->placement.lpfn = max((unsigned)rbo->placement.lpfn, (unsigned)rbo->rdev->mc.gtt_size >> PAGE_SHIFT);
>  		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
> -	if (domain & RADEON_GEM_DOMAIN_CPU)
> +	}
> +	if (domain & RADEON_GEM_DOMAIN_CPU) {
> +		/* 4G limit for CPU domain */
> +		rbo->placement.lpfn = max(rbo->placement.lpfn, 0xFFFFFFFF >> PAGE_SHIFT);
>  		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
> -	if (!c)
> +	}
> +	if (!c) {
> +		/* 4G limit for CPU domain */
> +		rbo->placement.lpfn = max(rbo->placement.lpfn, 0xFFFFFFFF >> PAGE_SHIFT);
>  		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
> +	}

I don't think taxing the maximum is the right thing to do: If domain is
(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT) and VRAM doesn't happen
to be the same size as GTT, lpfn will end up larger than one of them.

AFAICT radeon_ttm_placement_from_domain() should just set lpfn to 0
(i.e. unrestricted), the callers that need it to be non-0 already set it
afterwards.

Out of curiosity, where does the 4G limit come from?


> @@ -104,6 +115,17 @@ 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 = rdev->mc.visible_vram_size;
> +	if (max_size > rdev->mc.gtt_size) {
> +		max_size = rdev->mc.gtt_size;
> +	}

max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);

Except I think this should actually be max(...).


> +	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;
> +	}

Maybe this should be rate-limited, or userspace can spam dmesg.


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