| Message ID | 20170404095304.17599-4-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/04/2017 11:52 AM, Daniel Vetter wrote: > Also unify/merge with the existing stuff. > > I was a bit torn where to put this, but in the end I decided to put > all the ioctl/sysfs/debugfs stuff into drm-uapi.rst. That means we > have a bit a split with the other uapi related stuff used internally, > like drm_file.[hc], but I think overall this makes more sense. > > If it's too confusing we can always add more cross-links to make it > more discoverable. But the auto-sprinkling of links kernel-doc already > does seems sufficient. > > Also for prettier docs and more cross-links, switch the internal > defines over to an enum, as usual. > > v2: Update kerneldoc fro drm_compat_ioctl too (caught by 0day), plus a > bit more drive-by polish. > > v3: Fix typo, spotted by xerpi on irc (Sergi). > > Cc: Sergi Granell <xerpi.g.12@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > Documentation/gpu/drm-internals.rst | 50 ---------------- > Documentation/gpu/drm-uapi.rst | 14 +++++ > drivers/gpu/drm/drm_ioc32.c | 76 ++++++++++++----------- > drivers/gpu/drm/drm_ioctl.c | 50 +++++++++++++++- > include/drm/drm_ioctl.h | 116 +++++++++++++++++++++++++++++++----- > 5 files changed, 203 insertions(+), 103 deletions(-) > > diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst > index a09c721f9e89..babfb6143bd9 100644 > --- a/Documentation/gpu/drm-internals.rst > +++ b/Documentation/gpu/drm-internals.rst > @@ -255,56 +255,6 @@ File Operations > .. kernel-doc:: drivers/gpu/drm/drm_file.c > :export: > > -IOCTLs > ------- > - > -struct drm_ioctl_desc \*ioctls; int num_ioctls; > - Driver-specific ioctls descriptors table. > - > -Driver-specific ioctls numbers start at DRM_COMMAND_BASE. The ioctls > -descriptors table is indexed by the ioctl number offset from the base > -value. Drivers can use the DRM_IOCTL_DEF_DRV() macro to initialize > -the table entries. > - > -:: > - > - DRM_IOCTL_DEF_DRV(ioctl, func, flags) > - > -``ioctl`` is the ioctl name. Drivers must define the DRM_##ioctl and > -DRM_IOCTL_##ioctl macros to the ioctl number offset from > -DRM_COMMAND_BASE and the ioctl number respectively. The first macro is > -private to the device while the second must be exposed to userspace in a > -public header. > - > -``func`` is a pointer to the ioctl handler function compatible with the > -``drm_ioctl_t`` type. > - > -:: > - > - typedef int drm_ioctl_t(struct drm_device *dev, void *data, > - struct drm_file *file_priv); > - > -``flags`` is a bitmask combination of the following values. It restricts > -how the ioctl is allowed to be called. > - > -- DRM_AUTH - Only authenticated callers allowed > - > -- DRM_MASTER - The ioctl can only be called on the master file handle > - > -- DRM_ROOT_ONLY - Only callers with the SYSADMIN capability allowed > - > -- DRM_CONTROL_ALLOW - The ioctl can only be called on a control > - device > - > -- DRM_UNLOCKED - The ioctl handler will be called without locking the > - DRM global mutex. This is the enforced default for kms drivers (i.e. > - using the DRIVER_MODESET flag) and hence shouldn't be used any more > - for new drivers. > - > -.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c > - :export: > - > - > Misc Utilities > ============== > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 8b0f0e403f0c..858457567d3d 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -160,6 +160,20 @@ other hand, a driver requires shared state between clients which is > visible to user-space and accessible beyond open-file boundaries, they > cannot support render nodes. > > +IOCTL Support on Device Nodes > +============================= > + > +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c > + :doc: driver specific ioctls > + > +.. kernel-doc:: include/drm/drm_ioctl.h > + :internal: > + > +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c > + :export: > + > +.. kernel-doc:: drivers/gpu/drm/drm_ioc32.c > + :export: > > Testing and validation > ====================== > diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c > index b134482f4022..0fa7827cb0fd 100644 > --- a/drivers/gpu/drm/drm_ioc32.c > +++ b/drivers/gpu/drm/drm_ioc32.c > @@ -1,4 +1,4 @@ > -/** > +/* > * \file drm_ioc32.c > * > * 32-bit ioctl compatibility routines for the DRM. > @@ -72,15 +72,15 @@ > #define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8, drm_mode_fb_cmd232_t) > > typedef struct drm_version_32 { > - int version_major; /**< Major version */ > - int version_minor; /**< Minor version */ > - int version_patchlevel; /**< Patch level */ > - u32 name_len; /**< Length of name buffer */ > - u32 name; /**< Name of driver */ > - u32 date_len; /**< Length of date buffer */ > - u32 date; /**< User-space buffer to hold date */ > - u32 desc_len; /**< Length of desc buffer */ > - u32 desc; /**< User-space buffer to hold desc */ > + int version_major; /* Major version */ > + int version_minor; /* Minor version */ > + int version_patchlevel; /* Patch level */ > + u32 name_len; /* Length of name buffer */ > + u32 name; /* Name of driver */ > + u32 date_len; /* Length of date buffer */ > + u32 date; /* User-space buffer to hold date */ > + u32 desc_len; /* Length of desc buffer */ > + u32 desc; /* User-space buffer to hold desc */ > } drm_version32_t; > > static int compat_drm_version(struct file *file, unsigned int cmd, > @@ -126,8 +126,8 @@ static int compat_drm_version(struct file *file, unsigned int cmd, > } > > typedef struct drm_unique32 { > - u32 unique_len; /**< Length of unique */ > - u32 unique; /**< Unique name for driver instantiation */ > + u32 unique_len; /* Length of unique */ > + u32 unique; /* Unique name for driver instantiation */ > } drm_unique32_t; > > static int compat_drm_getunique(struct file *file, unsigned int cmd, > @@ -180,12 +180,12 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, > } > > typedef struct drm_map32 { > - u32 offset; /**< Requested physical address (0 for SAREA)*/ > - u32 size; /**< Requested physical size (bytes) */ > - enum drm_map_type type; /**< Type of memory to map */ > - enum drm_map_flags flags; /**< Flags */ > - u32 handle; /**< User-space: "Handle" to pass to mmap() */ > - int mtrr; /**< MTRR slot used */ > + u32 offset; /* Requested physical address (0 for SAREA)*/ /\ Missing space here > + u32 size; /* Requested physical size (bytes) */ > + enum drm_map_type type; /* Type of memory to map */ > + enum drm_map_flags flags; /* Flags */ > + u32 handle; /* User-space: "Handle" to pass to mmap() */ > + int mtrr; /* MTRR slot used */ > } drm_map32_t; > > static int compat_drm_getmap(struct file *file, unsigned int cmd, > @@ -286,12 +286,12 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, > } > > typedef struct drm_client32 { > - int idx; /**< Which client desired? */ > - int auth; /**< Is client authenticated? */ > - u32 pid; /**< Process ID */ > - u32 uid; /**< User ID */ > - u32 magic; /**< Magic */ > - u32 iocs; /**< Ioctl count */ > + int idx; /* Which client desired? */ > + int auth; /* Is client authenticated? */ > + u32 pid; /* Process ID */ > + u32 uid; /* User ID */ > + u32 magic; /* Magic */ > + u32 iocs; /* Ioctl count */ > } drm_client32_t; > > static int compat_drm_getclient(struct file *file, unsigned int cmd, > @@ -366,12 +366,12 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, > } > > typedef struct drm_buf_desc32 { > - int count; /**< Number of buffers of this size */ > - int size; /**< Size in bytes */ > - int low_mark; /**< Low water mark */ > - int high_mark; /**< High water mark */ > + int count; /* Number of buffers of this size */ > + int size; /* Size in bytes */ > + int low_mark; /* Low water mark */ > + int high_mark; /* High water mark */ > int flags; > - u32 agp_start; /**< Start address in the AGP aperture */ > + u32 agp_start; /* Start address in the AGP aperture */ > } drm_buf_desc32_t; > > static int compat_drm_addbufs(struct file *file, unsigned int cmd, > @@ -1111,13 +1111,18 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = { > }; > > /** > - * Called whenever a 32-bit process running under a 64-bit kernel > - * performs an ioctl on /dev/drm. > + * drm_compat_ioctl - 32bit IOCTL compatibility handler for DRM drivers > + * @filp: file this ioctl is called on > + * @cmd: ioctl cmd number > + * @arg: user argument > + * > + * Compatibility handler for 32 bit userspace running on 64 kernels. All actual > + * IOCTL handling is forwarded to drm_ioctl(), while marshalling structures as > + * appropriate. Note that this only handles DRM core IOCTLs, if the driver has > + * botched IOCTL itself, it must handle those by wrapping this function. > * > - * \param file_priv DRM file private. > - * \param cmd command. > - * \param arg user argument. > - * \return zero on success or negative number on failure. > + * Returns: > + * Zero on success, negative error code on failure. > */ > long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > { > @@ -1141,5 +1146,4 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > return ret; > } > - > EXPORT_SYMBOL(drm_compat_ioctl); > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 7f4f4f48e390..f908ea94b784 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -647,13 +647,59 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) > > /** > + * DOC: driver specific ioctls > + * > + * First things first, driver private IOCTLs should only be needed for drivers > + * supporting rendering. Kernel modesetting is all standardized, and extended > + * through properties. There are a few exceptions in some existing drivers, > + * which define IOCTL for use by the display DRM master, but they all predate > + * properties. > + * > + * Now if you do have a render driver you always have to support it through > + * driver private properties. There's a few steps needed to wire all the things > + * up. > + * > + * First you need to define the structure for your IOCTL in your driver private > + * UAPI header in ``include/uapi/drm/my_driver_drm.h``:: > + * > + * struct my_driver_operation { > + * u32 some_thing; > + * u32 another_thing; > + * }; > + * > + * Please make sure that you follow all the best practices from > + * ``Documentation/ioctl/botching-up-ioctls.txt``. Note that drm_ioctl() > + * automatically zero-extends structures, hence make sure you can add more stuff > + * at the end, i.e. don't put a variable sized array there. > + * > + * Then you need to define your IOCTL number, using one of DRM_IO(), DRM_IOR(), > + * DRM_IOW() or DRM_IOWR(). It must start with the DRM_IOCTL\_ prefix:: > + * > + * ##define DRM_IOCTL_MY_DRIVER_OPERATION \ > + * DRM_IOW(DRM_COMMAND_BASE, struct my_driver_operation) > + * > + * DRM driver private IOCTL must be in the range from DRM_COMMAND_BASE to > + * DRM_COMMAND_END. Finally you need an array of &struct drm_ioctl_desc to wire > + * up the handlers and set the access rights: > + * > + * static const struct drm_ioctl_desc my_driver_ioctls[] = { > + * DRM_IOCTL_DEF_DRV(MY_DRIVER_OPERATION, my_driver_operation, > + * DRM_AUTH|DRM_RENDER_ALLOW), > + * }; > + * > + * And then assign this to the &drm_driver.ioctls field in your driver > + * structure. > + */ > + > +/** > * drm_ioctl - ioctl callback implementation for DRM drivers > * @filp: file this ioctl is called on > * @cmd: ioctl cmd number > * @arg: user argument > * > - * Looks up the ioctl function in the ::ioctls table, checking for root > - * previleges if so required, and dispatches to the respective function. > + * Looks up the ioctl function in the DRM core and the driver dispatch table, > + * stored in &drm_driver.ioctls. It checks for necessary permission by calling > + * drm_ioctl_permit(), and dispatches to the respective function. > * > * Returns: > * Zero on success, negative error code on failure. > diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h > index f17ee077f649..ee03b3c44b3b 100644 > --- a/include/drm/drm_ioctl.h > +++ b/include/drm/drm_ioctl.h > @@ -33,6 +33,7 @@ > #define _DRM_IOCTL_H_ > > #include <linux/types.h> > +#include <linux/bitops.h> > > #include <asm/ioctl.h> > > @@ -41,41 +42,126 @@ struct drm_file; > struct file; > > /** > - * Ioctl function type. > + * drm_ioctl_t - DRM ioctl function type. > + * @dev: DRM device inode > + * @data: private pointer of the ioctl call > + * @file_priv: DRM file this ioctl was made on > * > - * \param inode device inode. > - * \param file_priv DRM file private pointer. > - * \param cmd command. > - * \param arg argument. > + * This is the DRM ioctl typedef. Note that drm_ioctl() has alrady copied @data > + * into kernel-space, and will also copy it back, depending upon the read/write > + * settings in the ioctl command code. > */ > typedef int drm_ioctl_t(struct drm_device *dev, void *data, > struct drm_file *file_priv); > > +/** > + * drm_ioctl_compat_t - compatibility DRM ioctl function type. > + * @filp: file pointer > + * @cmd: ioctl command code > + * @arg: DRM file this ioctl was made on > + * > + * Just a typedef to make declaring an array of compatibility handlers easier. > + * New drivers shouldn't screw up the structure layout for their ioctl > + * structures and hence never need this. > + */ > typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, > unsigned long arg); > > #define DRM_IOCTL_NR(n) _IOC_NR(n) > #define DRM_MAJOR 226 > > -#define DRM_AUTH 0x1 > -#define DRM_MASTER 0x2 > -#define DRM_ROOT_ONLY 0x4 > -#define DRM_CONTROL_ALLOW 0x8 > -#define DRM_UNLOCKED 0x10 > -#define DRM_RENDER_ALLOW 0x20 > +/** > + * enum drm_ioctl_flags - DRM ioctl flags > + * > + * Various flags that can be set in &drm_ioctl_desc.flags to control how > + * userspace can use a given ioctl. > + */ > +enum drm_ioctl_flags { > + /** > + * @DRM_AUTH: > + * > + * This is for ioctl which are used for rendering, and require that the > + * file descriptor is either for a render node, or if it's a > + * legacy/primary node, then it must be authenticated. > + */ > + DRM_AUTH = BIT(0), > + /** > + * @DRM_MASTER: > + * > + * This must be set for any ioctl which can change the modeset or > + * display state. Userspace must call the ioctl through a primary node, > + * while it is the active master. > + * > + * Note that read-only modeset ioctl can also be called by > + * unauthenticated clients, or when a master is not the currently active > + * one. > + */ > + DRM_MASTER = BIT(1), > + /** > + * @DRM_ROOT_ONLY: > + * > + * Anything that could potentially wreak a master file descriptor needs > + * to have this flag set. Current that's only for the SETMASTER and > + * DROPMASTER ioctl, which e.g. logind can call to force a non-behaving > + * master (display compositor) into compliance. > + * > + * This is equivalent to callers with the SYSADMIN capability. > + */ > + DRM_ROOT_ONLY = BIT(2), > + /** > + * @DRM_CONTROL_ALLOW: > + * > + * Deprecated, do not use. Control nodes are in the process of getting > + * removed. > + */ > + DRM_CONTROL_ALLOW = BIT(3), > + /** > + * @DRM_UNLOCKED: > + * > + * Whether &drm_ioctl_desc.func should be called with the DRM BKL held > + * or not. Enforced as the default for all modern drivers, hence there > + * should never be a need to set this flag. > + */ > + DRM_UNLOCKED = BIT(4), > + /** > + * @DRM_RENDER_ALLOW: > + * > + * This is used for all ioctl needed for rendering only, for drivers > + * which support render nodes. This should be all new render drivers, > + * and hence it should be always set for any ioctl with DRM_AUTH set. > + * Note though that read-only query ioctl might have this set, but have > + * not set DRM_AUTH because they do not require authentication. > + */ > + DRM_RENDER_ALLOW = BIT(5), > +}; > > +/** > + * struct drm_ioctl_desc - DRM driver ioctl entry > + * @cmd: ioctl command number, without flags > + * @flags: a bitmask of &enum drm_ioctl_flags > + * @func: handler for this ioctl > + * @name: user-readable name for debug output > + * > + * For convenience it's easier to create these using the DRM_IOCTL_DEF_DRV() > + * macro. > + */ > struct drm_ioctl_desc { > unsigned int cmd; > - int flags; > + enum drm_ioctl_flags flags; > drm_ioctl_t *func; > const char *name; > }; > > /** > - * Creates a driver or general drm_ioctl_desc array entry for the given > - * ioctl, for use by drm_ioctl(). > + * DRM_IOCTL_DEF_DRV() - helper macro to fill out a &struct drm_ioctl_desc > + * @ioctl: ioctl command suffix > + * @_func: handler for the ioctl > + * @_flags: a bitmask of &enum drm_ioctl_flags > + * > + * Small helper macro to create a &struct drm_ioctl_desc entry. The ioctl > + * command number is constructed by prepending ``DRM_IOCTL\_`` and passing that > + * to DRM_IOCTL_NR(). > */ > - > #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags) \ > [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { \ > .cmd = DRM_IOCTL_##ioctl, \ > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index a09c721f9e89..babfb6143bd9 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -255,56 +255,6 @@ File Operations .. kernel-doc:: drivers/gpu/drm/drm_file.c :export: -IOCTLs ------- - -struct drm_ioctl_desc \*ioctls; int num_ioctls; - Driver-specific ioctls descriptors table. - -Driver-specific ioctls numbers start at DRM_COMMAND_BASE. The ioctls -descriptors table is indexed by the ioctl number offset from the base -value. Drivers can use the DRM_IOCTL_DEF_DRV() macro to initialize -the table entries. - -:: - - DRM_IOCTL_DEF_DRV(ioctl, func, flags) - -``ioctl`` is the ioctl name. Drivers must define the DRM_##ioctl and -DRM_IOCTL_##ioctl macros to the ioctl number offset from -DRM_COMMAND_BASE and the ioctl number respectively. The first macro is -private to the device while the second must be exposed to userspace in a -public header. - -``func`` is a pointer to the ioctl handler function compatible with the -``drm_ioctl_t`` type. - -:: - - typedef int drm_ioctl_t(struct drm_device *dev, void *data, - struct drm_file *file_priv); - -``flags`` is a bitmask combination of the following values. It restricts -how the ioctl is allowed to be called. - -- DRM_AUTH - Only authenticated callers allowed - -- DRM_MASTER - The ioctl can only be called on the master file handle - -- DRM_ROOT_ONLY - Only callers with the SYSADMIN capability allowed - -- DRM_CONTROL_ALLOW - The ioctl can only be called on a control - device - -- DRM_UNLOCKED - The ioctl handler will be called without locking the - DRM global mutex. This is the enforced default for kms drivers (i.e. - using the DRIVER_MODESET flag) and hence shouldn't be used any more - for new drivers. - -.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c - :export: - - Misc Utilities ============== diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 8b0f0e403f0c..858457567d3d 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -160,6 +160,20 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +IOCTL Support on Device Nodes +============================= + +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c + :doc: driver specific ioctls + +.. kernel-doc:: include/drm/drm_ioctl.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c + :export: + +.. kernel-doc:: drivers/gpu/drm/drm_ioc32.c + :export: Testing and validation ====================== diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index b134482f4022..0fa7827cb0fd 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -1,4 +1,4 @@ -/** +/* * \file drm_ioc32.c * * 32-bit ioctl compatibility routines for the DRM. @@ -72,15 +72,15 @@ #define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8, drm_mode_fb_cmd232_t) typedef struct drm_version_32 { - int version_major; /**< Major version */ - int version_minor; /**< Minor version */ - int version_patchlevel; /**< Patch level */ - u32 name_len; /**< Length of name buffer */ - u32 name; /**< Name of driver */ - u32 date_len; /**< Length of date buffer */ - u32 date; /**< User-space buffer to hold date */ - u32 desc_len; /**< Length of desc buffer */ - u32 desc; /**< User-space buffer to hold desc */ + int version_major; /* Major version */ + int version_minor; /* Minor version */ + int version_patchlevel; /* Patch level */ + u32 name_len; /* Length of name buffer */ + u32 name; /* Name of driver */ + u32 date_len; /* Length of date buffer */ + u32 date; /* User-space buffer to hold date */ + u32 desc_len; /* Length of desc buffer */ + u32 desc; /* User-space buffer to hold desc */ } drm_version32_t; static int compat_drm_version(struct file *file, unsigned int cmd, @@ -126,8 +126,8 @@ static int compat_drm_version(struct file *file, unsigned int cmd, } typedef struct drm_unique32 { - u32 unique_len; /**< Length of unique */ - u32 unique; /**< Unique name for driver instantiation */ + u32 unique_len; /* Length of unique */ + u32 unique; /* Unique name for driver instantiation */ } drm_unique32_t; static int compat_drm_getunique(struct file *file, unsigned int cmd, @@ -180,12 +180,12 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, } typedef struct drm_map32 { - u32 offset; /**< Requested physical address (0 for SAREA)*/ - u32 size; /**< Requested physical size (bytes) */ - enum drm_map_type type; /**< Type of memory to map */ - enum drm_map_flags flags; /**< Flags */ - u32 handle; /**< User-space: "Handle" to pass to mmap() */ - int mtrr; /**< MTRR slot used */ + u32 offset; /* Requested physical address (0 for SAREA)*/ + u32 size; /* Requested physical size (bytes) */ + enum drm_map_type type; /* Type of memory to map */ + enum drm_map_flags flags; /* Flags */ + u32 handle; /* User-space: "Handle" to pass to mmap() */ + int mtrr; /* MTRR slot used */ } drm_map32_t; static int compat_drm_getmap(struct file *file, unsigned int cmd, @@ -286,12 +286,12 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, } typedef struct drm_client32 { - int idx; /**< Which client desired? */ - int auth; /**< Is client authenticated? */ - u32 pid; /**< Process ID */ - u32 uid; /**< User ID */ - u32 magic; /**< Magic */ - u32 iocs; /**< Ioctl count */ + int idx; /* Which client desired? */ + int auth; /* Is client authenticated? */ + u32 pid; /* Process ID */ + u32 uid; /* User ID */ + u32 magic; /* Magic */ + u32 iocs; /* Ioctl count */ } drm_client32_t; static int compat_drm_getclient(struct file *file, unsigned int cmd, @@ -366,12 +366,12 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, } typedef struct drm_buf_desc32 { - int count; /**< Number of buffers of this size */ - int size; /**< Size in bytes */ - int low_mark; /**< Low water mark */ - int high_mark; /**< High water mark */ + int count; /* Number of buffers of this size */ + int size; /* Size in bytes */ + int low_mark; /* Low water mark */ + int high_mark; /* High water mark */ int flags; - u32 agp_start; /**< Start address in the AGP aperture */ + u32 agp_start; /* Start address in the AGP aperture */ } drm_buf_desc32_t; static int compat_drm_addbufs(struct file *file, unsigned int cmd, @@ -1111,13 +1111,18 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = { }; /** - * Called whenever a 32-bit process running under a 64-bit kernel - * performs an ioctl on /dev/drm. + * drm_compat_ioctl - 32bit IOCTL compatibility handler for DRM drivers + * @filp: file this ioctl is called on + * @cmd: ioctl cmd number + * @arg: user argument + * + * Compatibility handler for 32 bit userspace running on 64 kernels. All actual + * IOCTL handling is forwarded to drm_ioctl(), while marshalling structures as + * appropriate. Note that this only handles DRM core IOCTLs, if the driver has + * botched IOCTL itself, it must handle those by wrapping this function. * - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument. - * \return zero on success or negative number on failure. + * Returns: + * Zero on success, negative error code on failure. */ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { @@ -1141,5 +1146,4 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return ret; } - EXPORT_SYMBOL(drm_compat_ioctl); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 7f4f4f48e390..f908ea94b784 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -647,13 +647,59 @@ static const struct drm_ioctl_desc drm_ioctls[] = { #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) /** + * DOC: driver specific ioctls + * + * First things first, driver private IOCTLs should only be needed for drivers + * supporting rendering. Kernel modesetting is all standardized, and extended + * through properties. There are a few exceptions in some existing drivers, + * which define IOCTL for use by the display DRM master, but they all predate + * properties. + * + * Now if you do have a render driver you always have to support it through + * driver private properties. There's a few steps needed to wire all the things + * up. + * + * First you need to define the structure for your IOCTL in your driver private + * UAPI header in ``include/uapi/drm/my_driver_drm.h``:: + * + * struct my_driver_operation { + * u32 some_thing; + * u32 another_thing; + * }; + * + * Please make sure that you follow all the best practices from + * ``Documentation/ioctl/botching-up-ioctls.txt``. Note that drm_ioctl() + * automatically zero-extends structures, hence make sure you can add more stuff + * at the end, i.e. don't put a variable sized array there. + * + * Then you need to define your IOCTL number, using one of DRM_IO(), DRM_IOR(), + * DRM_IOW() or DRM_IOWR(). It must start with the DRM_IOCTL\_ prefix:: + * + * ##define DRM_IOCTL_MY_DRIVER_OPERATION \ + * DRM_IOW(DRM_COMMAND_BASE, struct my_driver_operation) + * + * DRM driver private IOCTL must be in the range from DRM_COMMAND_BASE to + * DRM_COMMAND_END. Finally you need an array of &struct drm_ioctl_desc to wire + * up the handlers and set the access rights: + * + * static const struct drm_ioctl_desc my_driver_ioctls[] = { + * DRM_IOCTL_DEF_DRV(MY_DRIVER_OPERATION, my_driver_operation, + * DRM_AUTH|DRM_RENDER_ALLOW), + * }; + * + * And then assign this to the &drm_driver.ioctls field in your driver + * structure. + */ + +/** * drm_ioctl - ioctl callback implementation for DRM drivers * @filp: file this ioctl is called on * @cmd: ioctl cmd number * @arg: user argument * - * Looks up the ioctl function in the ::ioctls table, checking for root - * previleges if so required, and dispatches to the respective function. + * Looks up the ioctl function in the DRM core and the driver dispatch table, + * stored in &drm_driver.ioctls. It checks for necessary permission by calling + * drm_ioctl_permit(), and dispatches to the respective function. * * Returns: * Zero on success, negative error code on failure. diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h index f17ee077f649..ee03b3c44b3b 100644 --- a/include/drm/drm_ioctl.h +++ b/include/drm/drm_ioctl.h @@ -33,6 +33,7 @@ #define _DRM_IOCTL_H_ #include <linux/types.h> +#include <linux/bitops.h> #include <asm/ioctl.h> @@ -41,41 +42,126 @@ struct drm_file; struct file; /** - * Ioctl function type. + * drm_ioctl_t - DRM ioctl function type. + * @dev: DRM device inode + * @data: private pointer of the ioctl call + * @file_priv: DRM file this ioctl was made on * - * \param inode device inode. - * \param file_priv DRM file private pointer. - * \param cmd command. - * \param arg argument. + * This is the DRM ioctl typedef. Note that drm_ioctl() has alrady copied @data + * into kernel-space, and will also copy it back, depending upon the read/write + * settings in the ioctl command code. */ typedef int drm_ioctl_t(struct drm_device *dev, void *data, struct drm_file *file_priv); +/** + * drm_ioctl_compat_t - compatibility DRM ioctl function type. + * @filp: file pointer + * @cmd: ioctl command code + * @arg: DRM file this ioctl was made on + * + * Just a typedef to make declaring an array of compatibility handlers easier. + * New drivers shouldn't screw up the structure layout for their ioctl + * structures and hence never need this. + */ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, unsigned long arg); #define DRM_IOCTL_NR(n) _IOC_NR(n) #define DRM_MAJOR 226 -#define DRM_AUTH 0x1 -#define DRM_MASTER 0x2 -#define DRM_ROOT_ONLY 0x4 -#define DRM_CONTROL_ALLOW 0x8 -#define DRM_UNLOCKED 0x10 -#define DRM_RENDER_ALLOW 0x20 +/** + * enum drm_ioctl_flags - DRM ioctl flags + * + * Various flags that can be set in &drm_ioctl_desc.flags to control how + * userspace can use a given ioctl. + */ +enum drm_ioctl_flags { + /** + * @DRM_AUTH: + * + * This is for ioctl which are used for rendering, and require that the + * file descriptor is either for a render node, or if it's a + * legacy/primary node, then it must be authenticated. + */ + DRM_AUTH = BIT(0), + /** + * @DRM_MASTER: + * + * This must be set for any ioctl which can change the modeset or + * display state. Userspace must call the ioctl through a primary node, + * while it is the active master. + * + * Note that read-only modeset ioctl can also be called by + * unauthenticated clients, or when a master is not the currently active + * one. + */ + DRM_MASTER = BIT(1), + /** + * @DRM_ROOT_ONLY: + * + * Anything that could potentially wreak a master file descriptor needs + * to have this flag set. Current that's only for the SETMASTER and + * DROPMASTER ioctl, which e.g. logind can call to force a non-behaving + * master (display compositor) into compliance. + * + * This is equivalent to callers with the SYSADMIN capability. + */ + DRM_ROOT_ONLY = BIT(2), + /** + * @DRM_CONTROL_ALLOW: + * + * Deprecated, do not use. Control nodes are in the process of getting + * removed. + */ + DRM_CONTROL_ALLOW = BIT(3), + /** + * @DRM_UNLOCKED: + * + * Whether &drm_ioctl_desc.func should be called with the DRM BKL held + * or not. Enforced as the default for all modern drivers, hence there + * should never be a need to set this flag. + */ + DRM_UNLOCKED = BIT(4), + /** + * @DRM_RENDER_ALLOW: + * + * This is used for all ioctl needed for rendering only, for drivers + * which support render nodes. This should be all new render drivers, + * and hence it should be always set for any ioctl with DRM_AUTH set. + * Note though that read-only query ioctl might have this set, but have + * not set DRM_AUTH because they do not require authentication. + */ + DRM_RENDER_ALLOW = BIT(5), +}; +/** + * struct drm_ioctl_desc - DRM driver ioctl entry + * @cmd: ioctl command number, without flags + * @flags: a bitmask of &enum drm_ioctl_flags + * @func: handler for this ioctl + * @name: user-readable name for debug output + * + * For convenience it's easier to create these using the DRM_IOCTL_DEF_DRV() + * macro. + */ struct drm_ioctl_desc { unsigned int cmd; - int flags; + enum drm_ioctl_flags flags; drm_ioctl_t *func; const char *name; }; /** - * Creates a driver or general drm_ioctl_desc array entry for the given - * ioctl, for use by drm_ioctl(). + * DRM_IOCTL_DEF_DRV() - helper macro to fill out a &struct drm_ioctl_desc + * @ioctl: ioctl command suffix + * @_func: handler for the ioctl + * @_flags: a bitmask of &enum drm_ioctl_flags + * + * Small helper macro to create a &struct drm_ioctl_desc entry. The ioctl + * command number is constructed by prepending ``DRM_IOCTL\_`` and passing that + * to DRM_IOCTL_NR(). */ - #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags) \ [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { \ .cmd = DRM_IOCTL_##ioctl, \
Also unify/merge with the existing stuff. I was a bit torn where to put this, but in the end I decided to put all the ioctl/sysfs/debugfs stuff into drm-uapi.rst. That means we have a bit a split with the other uapi related stuff used internally, like drm_file.[hc], but I think overall this makes more sense. If it's too confusing we can always add more cross-links to make it more discoverable. But the auto-sprinkling of links kernel-doc already does seems sufficient. Also for prettier docs and more cross-links, switch the internal defines over to an enum, as usual. v2: Update kerneldoc fro drm_compat_ioctl too (caught by 0day), plus a bit more drive-by polish. v3: Fix typo, spotted by xerpi on irc (Sergi). Cc: Sergi Granell <xerpi.g.12@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/drm-internals.rst | 50 ---------------- Documentation/gpu/drm-uapi.rst | 14 +++++ drivers/gpu/drm/drm_ioc32.c | 76 ++++++++++++----------- drivers/gpu/drm/drm_ioctl.c | 50 +++++++++++++++- include/drm/drm_ioctl.h | 116 +++++++++++++++++++++++++++++++----- 5 files changed, 203 insertions(+), 103 deletions(-)