Re: [Intel-gfx] [PATCH 10/61] drm/i915: Disable userptr pread/pwrite support.

Maarten Lankhorst
2020-10-12T14:13:48Z

Op 02-10-2020 om 22:14 schreef Ruhl, Michael J:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Maarten Lankhorst
>> Sent: Friday, October 2, 2020 8:59 AM
>> To: intel-gfx@lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH 10/61] drm/i915: Disable userptr pread/pwrite
>> support.
>>
>> Userptr should not need the kernel for a userspace memcpy, userspace
>> needs to call memcpy directly.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> .../gpu/drm/i915/gem/i915_gem_object_types.h | 2 ++
>> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 20
>> +++++++++++++++++++
>> drivers/gpu/drm/i915/i915_gem.c | 5 +++++
>> 3 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>> index 62dde3585b51..dbb6f6171165 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>> @@ -57,6 +57,8 @@ struct drm_i915_gem_object_ops {
>>
>> int (*pwrite)(struct drm_i915_gem_object *obj,
>> const struct drm_i915_gem_pwrite *arg);
>> + int (*pread)(struct drm_i915_gem_object *obj,
>> + const struct drm_i915_gem_pread *arg);
>>
>> int (*dmabuf_export)(struct drm_i915_gem_object *obj);
>> void (*release)(struct drm_i915_gem_object *obj);
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> index 22008948be58..136a589e5d94 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> @@ -700,6 +700,24 @@ i915_gem_userptr_dmabuf_export(struct
>> drm_i915_gem_object *obj)
>> return i915_gem_userptr_init__mmu_notifier(obj, 0);
>> }
>>
>> +static int
>> +i915_gem_userptr_pwrite(struct drm_i915_gem_object *obj,
>> + const struct drm_i915_gem_pwrite *args)
>> +{
>> + drm_dbg(obj->base.dev, "pwrite to userptr no longer allowed\n");
>> +
>> + return -EINVAL;
> I have seen ENOSYS used for unsupported pread/pwrite (see radeon_gem.c).
>
> I have also seen ENOTSUPP for similar return values.
>
> Is EINVAL the correct response?

It seems for some other things we use -ENXIO, I don't think it matters in the end.

As long as we fail in some recognisable way, I'm fine with it. :)

I chose -EINVAL as we return the same error with r/o objects.