[Intel-gfx] [PATCH 24/29] drm/i915: Allow multiple user handles to the same VM

[Tvrtko Ursulin]

On 08/04/2019 10:17, Chris Wilson wrote:
> It was noted that we made the same mistake for VM_ID as for object
> handles, whereby we ensured that we only allocated a single handle for
> one ppgtt. This has the unfortunate consequence for userspace that they
> need to reference count the handles to avoid destroying an active ID. If
> we allow multiple handles to the same ppgtt, userspace can freely
> unreference any handle they own without fear of destroying the same
> handle in use elsewhere.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem_context.c | 26 ++++++++-----------------
>   drivers/gpu/drm/i915/i915_gem_gtt.h     |  2 --
>   2 files changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index c05f949ed893..2a30cab7a65d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -837,8 +837,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
>   	if (err < 0)
>   		goto err_unlock;
>   
> -	GEM_BUG_ON(err == 0); /* reserved for default/unassigned ppgtt */
> -	ppgtt->user_handle = err;
> +	GEM_BUG_ON(err == 0); /* reserved for invalid/unassigned ppgtt */
>   
>   	mutex_unlock(&file_priv->vm_idr_lock);
>   
> @@ -876,10 +875,6 @@ int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data,
>   		return err;
>   
>   	ppgtt = idr_remove(&file_priv->vm_idr, id);
> -	if (ppgtt) {
> -		GEM_BUG_ON(ppgtt->user_handle != id);
> -		ppgtt->user_handle = 0;
> -	}
>   
>   	mutex_unlock(&file_priv->vm_idr_lock);
>   	if (!ppgtt)
> @@ -1001,18 +996,15 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
>   	if (ret)
>   		goto err_put;
>   
> -	if (!ppgtt->user_handle) {
> -		ret = idr_alloc(&file_priv->vm_idr, ppgtt, 0, 0, GFP_KERNEL);
> -		GEM_BUG_ON(!ret);
> -		if (ret < 0)
> -			goto err_unlock;
> +	ret = idr_alloc(&file_priv->vm_idr, ppgtt, 0, 0, GFP_KERNEL);
> +	GEM_BUG_ON(!ret);
> +	if (ret < 0)
> +		goto err_unlock;
>   
> -		ppgtt->user_handle = ret;
> -		i915_ppgtt_get(ppgtt);
> -	}
> +	i915_ppgtt_get(ppgtt);
>   
>   	args->size = 0;
> -	args->value = ppgtt->user_handle;
> +	args->value = ret;
>   
>   	ret = 0;
>   err_unlock:
> @@ -1103,10 +1095,8 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv,
>   		return err;
>   
>   	ppgtt = idr_find(&file_priv->vm_idr, args->value);
> -	if (ppgtt) {
> -		GEM_BUG_ON(ppgtt->user_handle != args->value);
> +	if (ppgtt)
>   		i915_ppgtt_get(ppgtt);
> -	}
>   	mutex_unlock(&file_priv->vm_idr_lock);
>   	if (!ppgtt)
>   		return -ENOENT;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index c8d96e91f3dc..4832bb5c5fc0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -396,8 +396,6 @@ struct i915_hw_ppgtt {
>   		struct i915_page_directory_pointer pdp;	/* GEN8+ */
>   		struct i915_page_directory pd;		/* GEN6-7 */
>   	};
> -
> -	u32 user_handle;
>   };
>   
>   struct gen6_hw_ppgtt {
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko