Re: [PATCH 2/4] drm/i915: Support for creating Stolen memory backed objects

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Hi,

On 07/22/2015 02:51 PM, ankitprasad.r.sharma@intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>
> Extend the drm_i915_gem_create structure to add support for
> creating Stolen memory backed objects. Added a new flag through
> which user can specify the preference to allocate the object from
> stolen memory, which if set, an attempt will be made to allocate
> the object from stolen memory subject to the availability of
> free space in the stolen region.
>
> v2: Rebased to the latest drm-intel-nightly (Ankit)
>
> v3: Changed versioning of GEM_CREATE param, added new comments (Tvrtko)
>
> Testcase: igt/gem_stolen
>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_dma.c |  3 +++
>   drivers/gpu/drm/i915/i915_gem.c | 33 +++++++++++++++++++++++++++++----
>   include/uapi/drm/i915_drm.h     | 15 +++++++++++++++
>   3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index c5349fa..bfb07ab 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -167,6 +167,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>   		value = i915.enable_hangcheck &&
>   			intel_has_gpu_reset(dev);
>   		break;
> +	case I915_PARAM_CREATE_VERSION:
> +		value = 2;
> +		break;
>   	default:
>   		DRM_DEBUG("Unknown parameter %d\n", param->param);
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fc434ae..9e7e182 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -391,7 +391,8 @@ static int
>   i915_gem_create(struct drm_file *file,
>   		struct drm_device *dev,
>   		uint64_t size,
> -		uint32_t *handle_p)
> +		uint32_t *handle_p,
> +		uint32_t flags)
>   {
>   	struct drm_i915_gem_object *obj;
>   	int ret;
> @@ -401,8 +402,31 @@ i915_gem_create(struct drm_file *file,
>   	if (size == 0)
>   		return -EINVAL;
>
> +	if (flags & ~(I915_CREATE_PLACEMENT_STOLEN))
> +		return -EINVAL;
> +
>   	/* Allocate the new object */
> -	obj = i915_gem_alloc_object(dev, size);
> +	if (flags & I915_CREATE_PLACEMENT_STOLEN) {
> +		mutex_lock(&dev->struct_mutex);
> +		obj = i915_gem_object_create_stolen(dev, size);

Is the compiler not complaining that "uint64_t size" is being passed 
into "u32 size" here?

Either since there are no checks, can't userspace overflow u32 with a 
right value and get success and much smaller object than intended?

Perhaps should test with an IGT.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx