Re: [PATCH 4/5] drm/xe: Don't use __user error pointers

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jan 17, 2024 at 02:40:47PM +0100, Thomas Hellström wrote:
> The error pointer macros are not aware of __user pointers and as a
> consequence sparse warns.
> 
> Have the copy_mask() function return an integer instead of a __user
> pointer.
> 
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_query.c | 50 +++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index 9b35673b286c..7e924faeeea0 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -459,21 +459,21 @@ static size_t calc_topo_query_size(struct xe_device *xe)
>  		 sizeof_field(struct xe_gt, fuse_topo.eu_mask_per_dss));
>  }
>  
> -static void __user *copy_mask(void __user *ptr,
> -			      struct drm_xe_query_topology_mask *topo,
> -			      void *mask, size_t mask_size)
> +static int copy_mask(void __user **ptr,
> +		     struct drm_xe_query_topology_mask *topo,
> +		     void *mask, size_t mask_size)
>  {
>  	topo->num_bytes = mask_size;
>  
> -	if (copy_to_user(ptr, topo, sizeof(*topo)))
> -		return ERR_PTR(-EFAULT);
> -	ptr += sizeof(topo);
> +	if (copy_to_user(*ptr, topo, sizeof(*topo)))
> +		return -EFAULT;
> +	*ptr += sizeof(topo);
>  
> -	if (copy_to_user(ptr, mask, mask_size))
> -		return ERR_PTR(-EFAULT);
> -	ptr += mask_size;
> +	if (copy_to_user(*ptr, mask, mask_size))
> +		return -EFAULT;
> +	*ptr += mask_size;
>  
> -	return ptr;
> +	return 0;
>  }
>  
>  static int query_gt_topology(struct xe_device *xe,
> @@ -493,28 +493,28 @@ static int query_gt_topology(struct xe_device *xe,
>  	}
>  
>  	for_each_gt(gt, xe, id) {
> +		int err;
> +
>  		topo.gt_id = id;
>  
>  		topo.type = DRM_XE_TOPO_DSS_GEOMETRY;
> -		query_ptr = copy_mask(query_ptr, &topo,
> -				      gt->fuse_topo.g_dss_mask,
> -				      sizeof(gt->fuse_topo.g_dss_mask));
> -		if (IS_ERR(query_ptr))
> -			return PTR_ERR(query_ptr);
> +		err = copy_mask(&query_ptr, &topo, gt->fuse_topo.g_dss_mask,
> +				sizeof(gt->fuse_topo.g_dss_mask));
> +		if (err)
> +			return err;
>  
>  		topo.type = DRM_XE_TOPO_DSS_COMPUTE;
> -		query_ptr = copy_mask(query_ptr, &topo,
> -				      gt->fuse_topo.c_dss_mask,
> -				      sizeof(gt->fuse_topo.c_dss_mask));
> -		if (IS_ERR(query_ptr))
> -			return PTR_ERR(query_ptr);
> +		err = copy_mask(&query_ptr, &topo, gt->fuse_topo.c_dss_mask,
> +				sizeof(gt->fuse_topo.c_dss_mask));
> +		if (err)
> +			return err;
>  
>  		topo.type = DRM_XE_TOPO_EU_PER_DSS;
> -		query_ptr = copy_mask(query_ptr, &topo,
> -				      gt->fuse_topo.eu_mask_per_dss,
> -				      sizeof(gt->fuse_topo.eu_mask_per_dss));
> -		if (IS_ERR(query_ptr))
> -			return PTR_ERR(query_ptr);
> +		err = copy_mask(&query_ptr, &topo,
> +				gt->fuse_topo.eu_mask_per_dss,
> +				sizeof(gt->fuse_topo.eu_mask_per_dss));
> +		if (err)
> +			return err;
>  	}
>  
>  	return 0;
> -- 
> 2.43.0
>