diff options
author | Chris Wilson <chris@chris-wilson.co.uk> | 2019-03-22 12:23:22 +0300 |
---|---|---|
committer | Chris Wilson <chris@chris-wilson.co.uk> | 2019-03-22 16:12:30 +0300 |
commit | 9d1305ef80b95dde0337106ed8b826604e2155ad (patch) | |
tree | 8826f83b7b05432593b7296c73b80f974d438249 /drivers/gpu/drm/i915 | |
parent | b9d52d381e142fb3275430ac40ab119565d046f4 (diff) | |
download | linux-9d1305ef80b95dde0337106ed8b826604e2155ad.tar.xz |
drm/i915: Introduce the i915_user_extension_method
An idea for extending uABI inspired by Vulkan's extension chains.
Instead of expanding the data struct for each ioctl every time we need
to add a new feature, define an extension chain instead. As we add
optional interfaces to control the ioctl, we define a new extension
struct that can be linked into the ioctl data only when required by the
user. The key advantage being able to ignore large control structs for
optional interfaces/extensions, while being able to process them in a
consistent manner.
In comparison to other extensible ioctls, the key difference is the
use of a linked chain of extension structs vs an array of tagged
pointers. For example,
struct drm_amdgpu_cs_chunk {
__u32 chunk_id;
__u32 length_dw;
__u64 chunk_data;
};
struct drm_amdgpu_cs_in {
__u32 ctx_id;
__u32 bo_list_handle;
__u32 num_chunks;
__u32 _pad;
__u64 chunks;
};
allows userspace to pass in array of pointers to extension structs, but
must therefore keep constructing that array along side the command stream.
In dynamic situations like that, a linked list is preferred and does not
similar from extra cache line misses as the extension structs themselves
must still be loaded separate to the chunks array.
v2: Apply the tail call optimisation directly to nip the worry of stack
overflow in the bud.
v3: Defend against recursion.
v4: Fixup local types to match new uabi
Opens:
- do we include the result as an out-field in each chain?
struct i915_user_extension {
__u64 next_extension;
__u64 name;
__s32 result;
__u32 mbz; /* reserved for future use */
};
* Undecided, so provision some room for future expansion.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190322092325.5883-1-chris@chris-wilson.co.uk
Diffstat (limited to 'drivers/gpu/drm/i915')
-rw-r--r-- | drivers/gpu/drm/i915/Makefile | 1 | ||||
-rw-r--r-- | drivers/gpu/drm/i915/i915_user_extensions.c | 61 | ||||
-rw-r--r-- | drivers/gpu/drm/i915/i915_user_extensions.h | 20 | ||||
-rw-r--r-- | drivers/gpu/drm/i915/i915_utils.h | 31 |
4 files changed, 113 insertions, 0 deletions
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 68fecf355471..60de05f3fa60 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -46,6 +46,7 @@ i915-y := i915_drv.o \ i915_sw_fence.o \ i915_syncmap.o \ i915_sysfs.o \ + i915_user_extensions.o \ intel_csr.o \ intel_device_info.o \ intel_pm.o \ diff --git a/drivers/gpu/drm/i915/i915_user_extensions.c b/drivers/gpu/drm/i915/i915_user_extensions.c new file mode 100644 index 000000000000..c822d0aafd2d --- /dev/null +++ b/drivers/gpu/drm/i915/i915_user_extensions.c @@ -0,0 +1,61 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2018 Intel Corporation + */ + +#include <linux/nospec.h> +#include <linux/sched/signal.h> +#include <linux/uaccess.h> + +#include <uapi/drm/i915_drm.h> + +#include "i915_user_extensions.h" +#include "i915_utils.h" + +int i915_user_extensions(struct i915_user_extension __user *ext, + const i915_user_extension_fn *tbl, + unsigned int count, + void *data) +{ + unsigned int stackdepth = 512; + + while (ext) { + int i, err; + u32 name; + u64 next; + + if (!stackdepth--) /* recursion vs useful flexibility */ + return -E2BIG; + + err = check_user_mbz(&ext->flags); + if (err) + return err; + + for (i = 0; i < ARRAY_SIZE(ext->rsvd); i++) { + err = check_user_mbz(&ext->rsvd[i]); + if (err) + return err; + } + + if (get_user(name, &ext->name)) + return -EFAULT; + + err = -EINVAL; + if (name < count) { + name = array_index_nospec(name, count); + if (tbl[name]) + err = tbl[name](ext, data); + } + if (err) + return err; + + if (get_user(next, &ext->next_extension) || + overflows_type(next, ext)) + return -EFAULT; + + ext = u64_to_user_ptr(next); + } + + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_user_extensions.h b/drivers/gpu/drm/i915/i915_user_extensions.h new file mode 100644 index 000000000000..a14bf6bba9a1 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_user_extensions.h @@ -0,0 +1,20 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2018 Intel Corporation + */ + +#ifndef I915_USER_EXTENSIONS_H +#define I915_USER_EXTENSIONS_H + +struct i915_user_extension; + +typedef int (*i915_user_extension_fn)(struct i915_user_extension __user *ext, + void *data); + +int i915_user_extensions(struct i915_user_extension __user *ext, + const i915_user_extension_fn *tbl, + unsigned int count, + void *data); + +#endif /* I915_USER_EXTENSIONS_H */ diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 9726df37c4c4..8baedcaddbba 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -105,6 +105,37 @@ __T; \ }) +/* + * container_of_user: Extract the superclass from a pointer to a member. + * + * Exactly like container_of() with the exception that it plays nicely + * with sparse for __user @ptr. + */ +#define container_of_user(ptr, type, member) ({ \ + void __user *__mptr = (void __user *)(ptr); \ + BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ + !__same_type(*(ptr), void), \ + "pointer type mismatch in container_of()"); \ + ((type __user *)(__mptr - offsetof(type, member))); }) + +/* + * check_user_mbz: Check that a user value exists and is zero + * + * Frequently in our uABI we reserve space for future extensions, and + * two ensure that userspace is prepared we enforce that space must + * be zero. (Then any future extension can safely assume a default value + * of 0.) + * + * check_user_mbz() combines checking that the user pointer is accessible + * and that the contained value is zero. + * + * Returns: -EFAULT if not accessible, -EINVAL if !zero, or 0 on success. + */ +#define check_user_mbz(U) ({ \ + typeof(*(U)) mbz__; \ + get_user(mbz__, (U)) ? -EFAULT : mbz__ ? -EINVAL : 0; \ +}) + static inline u64 ptr_to_u64(const void *ptr) { return (uintptr_t)ptr; |