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

Ruhl, Michael J
2020-10-02T20:14:53Z

>-----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?

Thanks,

m

>+}
>+
>+static int
>+i915_gem_userptr_pread(struct drm_i915_gem_object *obj,
>+ const struct drm_i915_gem_pread *args)
>+{
>+ drm_dbg(obj->base.dev, "pread from userptr no longer allowed\n");
>+
>+ return -EINVAL;
>+}
>+
> static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
> .name = "i915_gem_object_userptr",
> .flags = I915_GEM_OBJECT_IS_SHRINKABLE |
>@@ -708,6 +726,8 @@ static const struct drm_i915_gem_object_ops
>i915_gem_userptr_ops = {
> .get_pages = i915_gem_userptr_get_pages,
> .put_pages = i915_gem_userptr_put_pages,
> .dmabuf_export = i915_gem_userptr_dmabuf_export,
>+ .pwrite = i915_gem_userptr_pwrite,
>+ .pread = i915_gem_userptr_pread,
> .release = i915_gem_userptr_release,
> };
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c
>b/drivers/gpu/drm/i915/i915_gem.c
>index 30af7e4b71ab..d349c0b796ec 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -526,6 +526,11 @@ i915_gem_pread_ioctl(struct drm_device *dev, void
>*data,
> }
>
> trace_i915_gem_object_pread(obj, args->offset, args->size);
>+ ret = -ENODEV;
>+ if (obj->ops->pread)
>+ ret = obj->ops->pread(obj, args);
>+ if (ret != -ENODEV)
>+ goto out;
>
> ret = i915_gem_object_wait(obj,
> I915_WAIT_INTERRUPTIBLE,
>--
>2.28.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx