Re: [Intel-gfx] [PATCH 3/5] drm/i915: Disable pread/pwrite ioctl's for future platforms (v3)

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 17, 2021 at 06:40:12PM -0500, Jason Ekstrand wrote:
> From: Ashutosh Dixit <ashutosh.dixit@intel.com>
> 
> The rationale for this change is roughly as follows:
> 
>  1. The functionality can be done entirely in userspace with a
>     combination of mmap + memcpy
> 
>  2. The only reason anyone in userspace is still using it is because
>     someone implemented bo_subdata that way in libdrm ages ago and
>     they're all too lazy to write the 5 lines of code to do a map.
> 
>  3. This falls cleanly into the category of things which will only get
>     more painful with local memory support.
> 
> These ioctls aren't used much anymore by "real" userspace drivers.
> Vulkan has never used them and neither has the iris GL driver.  The old
> i965 GL driver does use PWRITE for glBufferSubData but it only supports
> up through Gen11; Gen12 was never enabled in i965.  The compute driver
> has never used PREAD/PWRITE.  The only remaining user is the media
> driver which uses it exactly twice and they're easily removed [1] so
> expecting them to drop it going forward is reasonable.
> 
> IGT changes which handle this kernel change have also been submitted [2].
> 
> [1] https://github.com/intel/media-driver/pull/1160
> [2] https://patchwork.freedesktop.org/series/81384/
> 
> v2 (Jason Ekstrand):
>  - Improved commit message with the status of all usermode drivers
>  - A more future-proof platform check
> 
> v3 (Jason Ekstrand):
>  - Drop the HAS_LMEM checks as they're already covered by the version
>    checks
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

Merged the first three here. For the scheduler/context stuff I think we
should go back to normal due process with kernel patch + igt patches
tested together, then land igt first, then kernel, just to avoid hiccups
in CI.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b2e3b5cfccb4a..80915467e0d84 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -374,10 +374,17 @@ int
>  i915_gem_pread_ioctl(struct drm_device *dev, void *data,
>  		     struct drm_file *file)
>  {
> +	struct drm_i915_private *i915 = to_i915(dev);
>  	struct drm_i915_gem_pread *args = data;
>  	struct drm_i915_gem_object *obj;
>  	int ret;
>  
> +	/* PREAD is disallowed for all platforms after TGL-LP.  This also
> +	 * covers all platforms with local memory.
> +	 */
> +	if (INTEL_GEN(i915) >= 12 && !IS_TIGERLAKE(i915))
> +		return -EOPNOTSUPP;
> +
>  	if (args->size == 0)
>  		return 0;
>  
> @@ -675,10 +682,17 @@ int
>  i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>  		      struct drm_file *file)
>  {
> +	struct drm_i915_private *i915 = to_i915(dev);
>  	struct drm_i915_gem_pwrite *args = data;
>  	struct drm_i915_gem_object *obj;
>  	int ret;
>  
> +	/* PWRITE is disallowed for all platforms after TGL-LP.  This also
> +	 * covers all platforms with local memory.
> +	 */
> +	if (INTEL_GEN(i915) >= 12 && !IS_TIGERLAKE(i915))
> +		return -EOPNOTSUPP;
> +
>  	if (args->size == 0)
>  		return 0;
>  
> -- 
> 2.29.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx