intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler
@ 2021-08-13  8:54 Desmond Cheong Zhi Xi
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-13  8:54 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig
  Cc: Desmond Cheong Zhi Xi, dri-devel, linux-kernel, linux-media,
	linaro-mm-sig, intel-gfx, skhan, gregkh, linux-kernel-mentees

Hi,

Finally got around to it. This patchset implements some updates to the
drm ioctl handler that were first raised by Daniel Vetter in [1].
Namely:

- Flush concurrent processes that can change the modeset when
DRM masters are set/dropped or a lease is revoked
- Unexport drm_ioctl_permit()

Thoughts and comments would be very appreciated.

Link: https://lore.kernel.org/lkml/YN9kAFcfGoB13x7f@phenom.ffwll.local/ [1]

Best wishes,
Desmond

Desmond Cheong Zhi Xi (2):
  drm: avoid races with modesetting rights
  drm: unexport drm_ioctl_permit

 drivers/gpu/drm/drm_auth.c           | 17 +++++++++---
 drivers/gpu/drm/drm_client_modeset.c | 10 ++++---
 drivers/gpu/drm/drm_drv.c            |  2 ++
 drivers/gpu/drm/drm_fb_helper.c      | 20 ++++++++------
 drivers/gpu/drm/drm_internal.h       |  5 ++--
 drivers/gpu/drm/drm_ioctl.c          | 40 +++++++++++++++-------------
 include/drm/drm_device.h             | 11 ++++++++
 include/drm/drm_ioctl.h              |  8 +++++-
 8 files changed, 77 insertions(+), 36 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights
  2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
@ 2021-08-13  8:54 ` Desmond Cheong Zhi Xi
  2021-08-13 15:49   ` Daniel Vetter
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit Desmond Cheong Zhi Xi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-13  8:54 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig
  Cc: Desmond Cheong Zhi Xi, dri-devel, linux-kernel, linux-media,
	linaro-mm-sig, intel-gfx, skhan, gregkh, linux-kernel-mentees,
	Daniel Vetter

In drm_client_modeset.c and drm_fb_helper.c,
drm_master_internal_{acquire,release} are used to avoid races with DRM
userspace. These functions hold onto drm_device.master_mutex while
committing, and bail if there's already a master.

However, ioctls can still race between themselves. A
time-of-check-to-time-of-use error can occur if an ioctl that changes
the modeset has its rights revoked after it validates its permissions,
but before it completes.

There are three ioctls that can affect modesetting permissions:

- DROP_MASTER ioctl removes rights for a master and its leases

- REVOKE_LEASE ioctl revokes rights for a specific lease

- SET_MASTER ioctl sets the device master if the master role hasn't
been acquired yet

All these races can be avoided by introducing an SRCU that acts as a
barrier for ioctls that can change modesetting permissions. Processes
that perform modesetting should hold a read lock on the new
drm_device.master_barrier_srcu, and ioctls that change these
permissions should call synchronize_srcu before returning.

This ensures that any process that might have seen old permissions are
flushed out before DROP_MASTER/REVOKE_LEASE/SET_MASTER ioctls return
to userspace.

Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
 drivers/gpu/drm/drm_auth.c           | 17 ++++++++++++++---
 drivers/gpu/drm/drm_client_modeset.c | 10 ++++++----
 drivers/gpu/drm/drm_drv.c            |  2 ++
 drivers/gpu/drm/drm_fb_helper.c      | 20 ++++++++++++--------
 drivers/gpu/drm/drm_internal.h       |  5 +++--
 drivers/gpu/drm/drm_ioctl.c          | 25 +++++++++++++++++++++----
 include/drm/drm_device.h             | 11 +++++++++++
 include/drm/drm_ioctl.h              |  7 +++++++
 8 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 60a6b21474b1..004506608e76 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -29,6 +29,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/srcu.h>
 
 #include <drm/drm_auth.h>
 #include <drm/drm_drv.h>
@@ -448,21 +449,31 @@ void drm_master_put(struct drm_master **master)
 EXPORT_SYMBOL(drm_master_put);
 
 /* Used by drm_client and drm_fb_helper */
-bool drm_master_internal_acquire(struct drm_device *dev)
+bool drm_master_internal_acquire(struct drm_device *dev, int *idx)
 {
+	*idx = srcu_read_lock(&dev->master_barrier_srcu);
+
 	mutex_lock(&dev->master_mutex);
 	if (dev->master) {
 		mutex_unlock(&dev->master_mutex);
+		srcu_read_unlock(&dev->master_barrier_srcu, *idx);
 		return false;
 	}
+	mutex_unlock(&dev->master_mutex);
 
 	return true;
 }
 EXPORT_SYMBOL(drm_master_internal_acquire);
 
 /* Used by drm_client and drm_fb_helper */
-void drm_master_internal_release(struct drm_device *dev)
+void drm_master_internal_release(struct drm_device *dev, int idx)
 {
-	mutex_unlock(&dev->master_mutex);
+	srcu_read_unlock(&dev->master_barrier_srcu, idx);
 }
 EXPORT_SYMBOL(drm_master_internal_release);
+
+/* Used by drm_ioctl */
+void drm_master_flush(struct drm_device *dev)
+{
+	synchronize_srcu(&dev->master_barrier_srcu);
+}
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..9885f36f71b7 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1165,13 +1165,14 @@ int drm_client_modeset_commit(struct drm_client_dev *client)
 {
 	struct drm_device *dev = client->dev;
 	int ret;
+	int idx;
 
-	if (!drm_master_internal_acquire(dev))
+	if (!drm_master_internal_acquire(dev, &idx))
 		return -EBUSY;
 
 	ret = drm_client_modeset_commit_locked(client);
 
-	drm_master_internal_release(dev);
+	drm_master_internal_release(dev, idx);
 
 	return ret;
 }
@@ -1215,8 +1216,9 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
 {
 	struct drm_device *dev = client->dev;
 	int ret = 0;
+	int idx;
 
-	if (!drm_master_internal_acquire(dev))
+	if (!drm_master_internal_acquire(dev, &idx))
 		return -EBUSY;
 
 	mutex_lock(&client->modeset_mutex);
@@ -1226,7 +1228,7 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
 		drm_client_modeset_dpms_legacy(client, mode);
 	mutex_unlock(&client->modeset_mutex);
 
-	drm_master_internal_release(dev);
+	drm_master_internal_release(dev, idx);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7a5097467ba5..c313f0674db3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -574,6 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
 	mutex_destroy(&dev->clientlist_mutex);
 	mutex_destroy(&dev->filelist_mutex);
 	mutex_destroy(&dev->struct_mutex);
+	cleanup_srcu_struct(&dev->master_barrier_srcu);
 	drm_legacy_destroy_members(dev);
 }
 
@@ -612,6 +613,7 @@ static int drm_dev_init(struct drm_device *dev,
 	mutex_init(&dev->filelist_mutex);
 	mutex_init(&dev->clientlist_mutex);
 	mutex_init(&dev->master_mutex);
+	init_srcu_struct(&dev->master_barrier_srcu);
 
 	ret = drmm_add_action(dev, drm_dev_init_release, NULL);
 	if (ret)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3ab078321045..0d594bc15f18 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1116,13 +1116,14 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
 	int ret;
+	int idx;
 
 	if (oops_in_progress)
 		return -EBUSY;
 
 	mutex_lock(&fb_helper->lock);
 
-	if (!drm_master_internal_acquire(dev)) {
+	if (!drm_master_internal_acquire(dev, &idx)) {
 		ret = -EBUSY;
 		goto unlock;
 	}
@@ -1136,7 +1137,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		ret = setcmap_legacy(cmap, info);
 	mutex_unlock(&fb_helper->client.modeset_mutex);
 
-	drm_master_internal_release(dev);
+	drm_master_internal_release(dev, idx);
 unlock:
 	mutex_unlock(&fb_helper->lock);
 
@@ -1160,9 +1161,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_crtc *crtc;
 	int ret = 0;
+	int idx;
 
 	mutex_lock(&fb_helper->lock);
-	if (!drm_master_internal_acquire(dev)) {
+	if (!drm_master_internal_acquire(dev, &idx)) {
 		ret = -EBUSY;
 		goto unlock;
 	}
@@ -1204,7 +1206,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 		ret = -ENOTTY;
 	}
 
-	drm_master_internal_release(dev);
+	drm_master_internal_release(dev, idx);
 unlock:
 	mutex_unlock(&fb_helper->lock);
 	return ret;
@@ -1474,12 +1476,13 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
 	int ret;
+	int idx;
 
 	if (oops_in_progress)
 		return -EBUSY;
 
 	mutex_lock(&fb_helper->lock);
-	if (!drm_master_internal_acquire(dev)) {
+	if (!drm_master_internal_acquire(dev, &idx)) {
 		ret = -EBUSY;
 		goto unlock;
 	}
@@ -1489,7 +1492,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	else
 		ret = pan_display_legacy(var, info);
 
-	drm_master_internal_release(dev);
+	drm_master_internal_release(dev, idx);
 unlock:
 	mutex_unlock(&fb_helper->lock);
 
@@ -1948,6 +1951,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
 	int err = 0;
+	int idx;
 
 	if (!drm_fbdev_emulation || !fb_helper)
 		return 0;
@@ -1959,13 +1963,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 		return err;
 	}
 
-	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
+	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev, &idx)) {
 		fb_helper->delayed_hotplug = true;
 		mutex_unlock(&fb_helper->lock);
 		return err;
 	}
 
-	drm_master_internal_release(fb_helper->dev);
+	drm_master_internal_release(fb_helper->dev, idx);
 
 	drm_dbg_kms(fb_helper->dev, "\n");
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 17f3548c8ed2..578fd2769913 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,8 +142,9 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv);
 int drm_master_open(struct drm_file *file_priv);
 void drm_master_release(struct drm_file *file_priv);
-bool drm_master_internal_acquire(struct drm_device *dev);
-void drm_master_internal_release(struct drm_device *dev);
+bool drm_master_internal_acquire(struct drm_device *dev, int *idx);
+void drm_master_internal_release(struct drm_device *dev, int idx);
+void drm_master_flush(struct drm_device *dev);
 
 /* drm_sysfs.c */
 extern struct class *drm_class;
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index be4a52dc4d6f..eb4ec3fab7d1 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -600,8 +600,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, 0),
-	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, 0),
+	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl,
+		      DRM_MASTER_FLUSH),
+	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl,
+		      DRM_MASTER_FLUSH),
 
 	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY),
 	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
@@ -722,7 +724,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER),
-	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl,
+		      DRM_MASTER | DRM_MASTER_FLUSH),
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
@@ -781,13 +784,17 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
 	struct drm_file *file_priv = file->private_data;
 	struct drm_device *dev = file_priv->minor->dev;
 	int retcode;
+	int idx;
 
 	if (drm_dev_is_unplugged(dev))
 		return -ENODEV;
 
+	if (unlikely(flags & DRM_MASTER))
+		idx = srcu_read_lock(&dev->master_barrier_srcu);
+
 	retcode = drm_ioctl_permit(flags, file_priv);
 	if (unlikely(retcode))
-		return retcode;
+		goto release_master;
 
 	/* Enforce sane locking for modern driver ioctls. */
 	if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
@@ -798,6 +805,16 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
 		retcode = func(dev, kdata, file_priv);
 		mutex_unlock(&drm_global_mutex);
 	}
+
+release_master:
+	if (unlikely(flags & DRM_MASTER))
+		srcu_read_unlock(&dev->master_barrier_srcu, idx);
+	/* After flushing, processes are guaranteed to see the new master/lease
+	 * permissions, and any process which might have seen the old
+	 * permissions is guaranteed to have finished.
+	 */
+	if (unlikely(flags & DRM_MASTER_FLUSH))
+		drm_master_flush(dev);
 	return retcode;
 }
 EXPORT_SYMBOL(drm_ioctl_kernel);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 604b1d1b2d72..0ac5fdb375f8 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -111,6 +111,17 @@ struct drm_device {
 	 */
 	struct drm_master *master;
 
+	/**
+	 * @master_barrier_srcu:
+	 *
+	 * Used to synchronize modesetting rights between multiple users. Users
+	 * that can change the modeset or display state must hold an
+	 * srcu_read_lock() on @master_barrier_srcu, and ioctls that can change
+	 * modesetting rights should call synchronize_srcu() before returning
+	 * to userspace.
+	 */
+	struct srcu_struct master_barrier_srcu;
+
 	/**
 	 * @driver_features: per-device driver features
 	 *
diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
index afb27cb6a7bd..13a68cdcea36 100644
--- a/include/drm/drm_ioctl.h
+++ b/include/drm/drm_ioctl.h
@@ -130,6 +130,13 @@ enum drm_ioctl_flags {
 	 * not set DRM_AUTH because they do not require authentication.
 	 */
 	DRM_RENDER_ALLOW	= BIT(5),
+	/**
+	 * @DRM_MASTER_FLUSH:
+	 *
+	 * This must be set for any ioctl which can change the modesetting
+	 * permissions for DRM users.
+	 */
+	DRM_MASTER_FLUSH	= BIT(6),
 };
 
 /**
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit
  2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
@ 2021-08-13  8:54 ` Desmond Cheong Zhi Xi
  2021-08-13 15:51   ` Daniel Vetter
  2021-08-13 15:23 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: update the ioctl handler Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-13  8:54 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig
  Cc: Desmond Cheong Zhi Xi, dri-devel, linux-kernel, linux-media,
	linaro-mm-sig, intel-gfx, skhan, gregkh, linux-kernel-mentees,
	Daniel Vetter

Since the last user of drm_ioctl_permit was removed, and it's now only
used in drm_ioctl.c, unexport the symbol.

Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
 drivers/gpu/drm/drm_ioctl.c | 15 +--------------
 include/drm/drm_ioctl.h     |  1 -
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index eb4ec3fab7d1..fe271f6f96ab 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -522,19 +522,7 @@ int drm_version(struct drm_device *dev, void *data,
 	return err;
 }
 
-/**
- * drm_ioctl_permit - Check ioctl permissions against caller
- *
- * @flags: ioctl permission flags.
- * @file_priv: Pointer to struct drm_file identifying the caller.
- *
- * Checks whether the caller is allowed to run an ioctl with the
- * indicated permissions.
- *
- * Returns:
- * Zero if allowed, -EACCES otherwise.
- */
-int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
+static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 {
 	/* ROOT_ONLY is only for CAP_SYS_ADMIN */
 	if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
@@ -557,7 +545,6 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 
 	return 0;
 }
-EXPORT_SYMBOL(drm_ioctl_permit);
 
 #define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
 	[DRM_IOCTL_NR(ioctl)] = {		\
diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
index 13a68cdcea36..fd29842127e5 100644
--- a/include/drm/drm_ioctl.h
+++ b/include/drm/drm_ioctl.h
@@ -174,7 +174,6 @@ struct drm_ioctl_desc {
 		.name = #ioctl						\
 	}
 
-int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
 long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
 long drm_ioctl_kernel(struct file *, drm_ioctl_t, void *, u32);
 #ifdef CONFIG_COMPAT
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: update the ioctl handler
  2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit Desmond Cheong Zhi Xi
@ 2021-08-13 15:23 ` Patchwork
  2021-08-13 15:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2021-08-13 19:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2021-08-13 15:23 UTC (permalink / raw)
  To: Desmond Cheong Zhi Xi; +Cc: intel-gfx

== Series Details ==

Series: drm: update the ioctl handler
URL   : https://patchwork.freedesktop.org/series/93677/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:135:6: warning: symbol 'dcn10_log_hubbub_state' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:1902:10: warning: symbol 'reduceSizeAndFraction' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:1953:6: warning: symbol 'is_low_refresh_rate' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:1962:9: warning: symbol 'get_clock_divider' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:1982:5: warning: symbol 'dcn10_align_pixel_clocks' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2274:6: warning: symbol 'dcn10_program_pte_vm' was not declared. Should it be static?
+drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:80:6: warning: symbol 'print_microsec' was not declared. Should it be static?
+drivers/gpu/drm/drm_auth.c:452:6: warning: context imbalance in 'drm_master_internal_acquire' - different lock contexts for basic block
+drivers/gpu/drm/drm_ioctl.c:803:9: warning: context imbalance in 'drm_ioctl_kernel' - different lock contexts for basic block
+drivers/gpu/drm/i915/display/intel_display.c:1901:21:    expected struct i915_vma *[assigned] vma
+drivers/gpu/drm/i915/display/intel_display.c:1901:21:    got void [noderef] __iomem *[assigned] iomem
+drivers/gpu/drm/i915/display/intel_display.c:1901:21: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    expected struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    got struct i915_address_space *
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1392:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/intel_ring_submission.c:1268:24: warning: Using plain integer as NULL pointer
+drivers/gpu/drm/i915/i915_perf.c:1442:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1496:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/selftests/i915_syncmap.c:80:54: warning: dubious: x | !y
+drivers/gpu/drm/selftests/test-drm_damage_helper.c:101:30: warning: symbol 'fb' was not declared. Should it be static?
+drivers/gpu/drm/selftests/test-drm_damage_helper.c:14:19: warning: symbol 'mock_driver' was not declared. Should it be static?
+drivers/gpu/drm/selftests/test-drm_damage_helper.c:259:23: warning: Using plain integer as NULL pointer
+./include/asm-generic/bitops/find.h:112:45: warning: shift count is negative (-262080)
+./include/asm-generic/bitops/find.h:32:31: warning: shift count is negative (-262080)
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block
+./include/linux/srcu.h:188:9: warning: context imbalance in 'drm_master_internal_release' - unexpected unlock
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
@ 2021-08-13 15:49   ` Daniel Vetter
  2021-08-15  7:10     ` Desmond Cheong Zhi Xi
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2021-08-13 15:49 UTC (permalink / raw)
  To: Desmond Cheong Zhi Xi
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig, dri-devel, linux-kernel,
	linux-media, linaro-mm-sig, intel-gfx, skhan, gregkh,
	linux-kernel-mentees, Daniel Vetter

On Fri, Aug 13, 2021 at 04:54:49PM +0800, Desmond Cheong Zhi Xi wrote:
> In drm_client_modeset.c and drm_fb_helper.c,
> drm_master_internal_{acquire,release} are used to avoid races with DRM
> userspace. These functions hold onto drm_device.master_mutex while
> committing, and bail if there's already a master.
> 
> However, ioctls can still race between themselves. A
> time-of-check-to-time-of-use error can occur if an ioctl that changes
> the modeset has its rights revoked after it validates its permissions,
> but before it completes.
> 
> There are three ioctls that can affect modesetting permissions:
> 
> - DROP_MASTER ioctl removes rights for a master and its leases
> 
> - REVOKE_LEASE ioctl revokes rights for a specific lease
> 
> - SET_MASTER ioctl sets the device master if the master role hasn't
> been acquired yet
> 
> All these races can be avoided by introducing an SRCU that acts as a
> barrier for ioctls that can change modesetting permissions. Processes
> that perform modesetting should hold a read lock on the new
> drm_device.master_barrier_srcu, and ioctls that change these
> permissions should call synchronize_srcu before returning.
> 
> This ensures that any process that might have seen old permissions are
> flushed out before DROP_MASTER/REVOKE_LEASE/SET_MASTER ioctls return
> to userspace.
> 
> Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

This looks pretty solid, but I think there's one gap where we can still
race. Scenario.

Process A has a drm fd with master rights and two threads:
- thread 1 does a long-running display operation (like a modeset or
  whatever)
- thread 2 does a drop-master

Then we start a new process B, which acquires master in drm_open (there is
no other one left). This is like setmaster ioctl, but your
DRM_MASTER_FLUSH bit doesn't work there.

The other thing is that for modeset stuff (which this all is) srcu is
probably massive overkill, and a simple rwsem should be good enough too.
Maybe even better, since the rwsem guarantees that no new reader can start
once you try to acquire the write side.

Finally, and this is a bit a bikeshed: I don't like much how
DRM_MASTER_FLUSH leaks the need of these very few places into the very
core drm_ioctl function. One idea I had was to use task_work in a special
function, roughly

void master_flush()
{
	down_write(master_rwsem);
	up_write(master_rwms);
}
void drm_master_flush()
{
	init_task_work(fpriv->master_flush_work, master_flush)
	task_work_add(fpriv->master_flush_work);
	/* if task_work_add fails we're exiting, at which point the lack
	 * of master flush doesn't matter);
}

And maybe put a comment above the function explaining why and how this
works.

We could even do a drm_master_unlock_and_flush helper, since that's really
what everyone wants, and it would make it very clear which master state
changes need this flush. Instead of setting a flag bit in an ioctl table
very far away ...

Thoughts?
-Daniel

> ---
>  drivers/gpu/drm/drm_auth.c           | 17 ++++++++++++++---
>  drivers/gpu/drm/drm_client_modeset.c | 10 ++++++----
>  drivers/gpu/drm/drm_drv.c            |  2 ++
>  drivers/gpu/drm/drm_fb_helper.c      | 20 ++++++++++++--------
>  drivers/gpu/drm/drm_internal.h       |  5 +++--
>  drivers/gpu/drm/drm_ioctl.c          | 25 +++++++++++++++++++++----
>  include/drm/drm_device.h             | 11 +++++++++++
>  include/drm/drm_ioctl.h              |  7 +++++++
>  8 files changed, 76 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 60a6b21474b1..004506608e76 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -29,6 +29,7 @@
>   */
>  
>  #include <linux/slab.h>
> +#include <linux/srcu.h>
>  
>  #include <drm/drm_auth.h>
>  #include <drm/drm_drv.h>
> @@ -448,21 +449,31 @@ void drm_master_put(struct drm_master **master)
>  EXPORT_SYMBOL(drm_master_put);
>  
>  /* Used by drm_client and drm_fb_helper */
> -bool drm_master_internal_acquire(struct drm_device *dev)
> +bool drm_master_internal_acquire(struct drm_device *dev, int *idx)
>  {
> +	*idx = srcu_read_lock(&dev->master_barrier_srcu);
> +
>  	mutex_lock(&dev->master_mutex);
>  	if (dev->master) {
>  		mutex_unlock(&dev->master_mutex);
> +		srcu_read_unlock(&dev->master_barrier_srcu, *idx);
>  		return false;
>  	}
> +	mutex_unlock(&dev->master_mutex);
>  
>  	return true;
>  }
>  EXPORT_SYMBOL(drm_master_internal_acquire);
>  
>  /* Used by drm_client and drm_fb_helper */
> -void drm_master_internal_release(struct drm_device *dev)
> +void drm_master_internal_release(struct drm_device *dev, int idx)
>  {
> -	mutex_unlock(&dev->master_mutex);
> +	srcu_read_unlock(&dev->master_barrier_srcu, idx);
>  }
>  EXPORT_SYMBOL(drm_master_internal_release);
> +
> +/* Used by drm_ioctl */
> +void drm_master_flush(struct drm_device *dev)
> +{
> +	synchronize_srcu(&dev->master_barrier_srcu);
> +}
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index ced09c7c06f9..9885f36f71b7 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -1165,13 +1165,14 @@ int drm_client_modeset_commit(struct drm_client_dev *client)
>  {
>  	struct drm_device *dev = client->dev;
>  	int ret;
> +	int idx;
>  
> -	if (!drm_master_internal_acquire(dev))
> +	if (!drm_master_internal_acquire(dev, &idx))
>  		return -EBUSY;
>  
>  	ret = drm_client_modeset_commit_locked(client);
>  
> -	drm_master_internal_release(dev);
> +	drm_master_internal_release(dev, idx);
>  
>  	return ret;
>  }
> @@ -1215,8 +1216,9 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
>  {
>  	struct drm_device *dev = client->dev;
>  	int ret = 0;
> +	int idx;
>  
> -	if (!drm_master_internal_acquire(dev))
> +	if (!drm_master_internal_acquire(dev, &idx))
>  		return -EBUSY;
>  
>  	mutex_lock(&client->modeset_mutex);
> @@ -1226,7 +1228,7 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
>  		drm_client_modeset_dpms_legacy(client, mode);
>  	mutex_unlock(&client->modeset_mutex);
>  
> -	drm_master_internal_release(dev);
> +	drm_master_internal_release(dev, idx);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7a5097467ba5..c313f0674db3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -574,6 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
>  	mutex_destroy(&dev->clientlist_mutex);
>  	mutex_destroy(&dev->filelist_mutex);
>  	mutex_destroy(&dev->struct_mutex);
> +	cleanup_srcu_struct(&dev->master_barrier_srcu);
>  	drm_legacy_destroy_members(dev);
>  }
>  
> @@ -612,6 +613,7 @@ static int drm_dev_init(struct drm_device *dev,
>  	mutex_init(&dev->filelist_mutex);
>  	mutex_init(&dev->clientlist_mutex);
>  	mutex_init(&dev->master_mutex);
> +	init_srcu_struct(&dev->master_barrier_srcu);
>  
>  	ret = drmm_add_action(dev, drm_dev_init_release, NULL);
>  	if (ret)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 3ab078321045..0d594bc15f18 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1116,13 +1116,14 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
>  	int ret;
> +	int idx;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
>  
>  	mutex_lock(&fb_helper->lock);
>  
> -	if (!drm_master_internal_acquire(dev)) {
> +	if (!drm_master_internal_acquire(dev, &idx)) {
>  		ret = -EBUSY;
>  		goto unlock;
>  	}
> @@ -1136,7 +1137,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		ret = setcmap_legacy(cmap, info);
>  	mutex_unlock(&fb_helper->client.modeset_mutex);
>  
> -	drm_master_internal_release(dev);
> +	drm_master_internal_release(dev, idx);
>  unlock:
>  	mutex_unlock(&fb_helper->lock);
>  
> @@ -1160,9 +1161,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  	struct drm_device *dev = fb_helper->dev;
>  	struct drm_crtc *crtc;
>  	int ret = 0;
> +	int idx;
>  
>  	mutex_lock(&fb_helper->lock);
> -	if (!drm_master_internal_acquire(dev)) {
> +	if (!drm_master_internal_acquire(dev, &idx)) {
>  		ret = -EBUSY;
>  		goto unlock;
>  	}
> @@ -1204,7 +1206,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  		ret = -ENOTTY;
>  	}
>  
> -	drm_master_internal_release(dev);
> +	drm_master_internal_release(dev, idx);
>  unlock:
>  	mutex_unlock(&fb_helper->lock);
>  	return ret;
> @@ -1474,12 +1476,13 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
>  	int ret;
> +	int idx;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
>  
>  	mutex_lock(&fb_helper->lock);
> -	if (!drm_master_internal_acquire(dev)) {
> +	if (!drm_master_internal_acquire(dev, &idx)) {
>  		ret = -EBUSY;
>  		goto unlock;
>  	}
> @@ -1489,7 +1492,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  	else
>  		ret = pan_display_legacy(var, info);
>  
> -	drm_master_internal_release(dev);
> +	drm_master_internal_release(dev, idx);
>  unlock:
>  	mutex_unlock(&fb_helper->lock);
>  
> @@ -1948,6 +1951,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  {
>  	int err = 0;
> +	int idx;
>  
>  	if (!drm_fbdev_emulation || !fb_helper)
>  		return 0;
> @@ -1959,13 +1963,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  		return err;
>  	}
>  
> -	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
> +	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev, &idx)) {
>  		fb_helper->delayed_hotplug = true;
>  		mutex_unlock(&fb_helper->lock);
>  		return err;
>  	}
>  
> -	drm_master_internal_release(fb_helper->dev);
> +	drm_master_internal_release(fb_helper->dev, idx);
>  
>  	drm_dbg_kms(fb_helper->dev, "\n");
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 17f3548c8ed2..578fd2769913 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -142,8 +142,9 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
>  			 struct drm_file *file_priv);
>  int drm_master_open(struct drm_file *file_priv);
>  void drm_master_release(struct drm_file *file_priv);
> -bool drm_master_internal_acquire(struct drm_device *dev);
> -void drm_master_internal_release(struct drm_device *dev);
> +bool drm_master_internal_acquire(struct drm_device *dev, int *idx);
> +void drm_master_internal_release(struct drm_device *dev, int idx);
> +void drm_master_flush(struct drm_device *dev);
>  
>  /* drm_sysfs.c */
>  extern struct class *drm_class;
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index be4a52dc4d6f..eb4ec3fab7d1 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -600,8 +600,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>  	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH),
>  
> -	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, 0),
> -	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, 0),
> +	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl,
> +		      DRM_MASTER_FLUSH),
> +	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl,
> +		      DRM_MASTER_FLUSH),
>  
>  	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY),
>  	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> @@ -722,7 +724,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER),
> -	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER),
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl,
> +		      DRM_MASTER | DRM_MASTER_FLUSH),
>  };
>  
>  #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
> @@ -781,13 +784,17 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
>  	struct drm_file *file_priv = file->private_data;
>  	struct drm_device *dev = file_priv->minor->dev;
>  	int retcode;
> +	int idx;
>  
>  	if (drm_dev_is_unplugged(dev))
>  		return -ENODEV;
>  
> +	if (unlikely(flags & DRM_MASTER))
> +		idx = srcu_read_lock(&dev->master_barrier_srcu);
> +
>  	retcode = drm_ioctl_permit(flags, file_priv);
>  	if (unlikely(retcode))
> -		return retcode;
> +		goto release_master;
>  
>  	/* Enforce sane locking for modern driver ioctls. */
>  	if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
> @@ -798,6 +805,16 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
>  		retcode = func(dev, kdata, file_priv);
>  		mutex_unlock(&drm_global_mutex);
>  	}
> +
> +release_master:
> +	if (unlikely(flags & DRM_MASTER))
> +		srcu_read_unlock(&dev->master_barrier_srcu, idx);
> +	/* After flushing, processes are guaranteed to see the new master/lease
> +	 * permissions, and any process which might have seen the old
> +	 * permissions is guaranteed to have finished.
> +	 */
> +	if (unlikely(flags & DRM_MASTER_FLUSH))
> +		drm_master_flush(dev);
>  	return retcode;
>  }
>  EXPORT_SYMBOL(drm_ioctl_kernel);
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 604b1d1b2d72..0ac5fdb375f8 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -111,6 +111,17 @@ struct drm_device {
>  	 */
>  	struct drm_master *master;
>  
> +	/**
> +	 * @master_barrier_srcu:
> +	 *
> +	 * Used to synchronize modesetting rights between multiple users. Users
> +	 * that can change the modeset or display state must hold an
> +	 * srcu_read_lock() on @master_barrier_srcu, and ioctls that can change
> +	 * modesetting rights should call synchronize_srcu() before returning
> +	 * to userspace.
> +	 */
> +	struct srcu_struct master_barrier_srcu;
> +
>  	/**
>  	 * @driver_features: per-device driver features
>  	 *
> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> index afb27cb6a7bd..13a68cdcea36 100644
> --- a/include/drm/drm_ioctl.h
> +++ b/include/drm/drm_ioctl.h
> @@ -130,6 +130,13 @@ enum drm_ioctl_flags {
>  	 * not set DRM_AUTH because they do not require authentication.
>  	 */
>  	DRM_RENDER_ALLOW	= BIT(5),
> +	/**
> +	 * @DRM_MASTER_FLUSH:
> +	 *
> +	 * This must be set for any ioctl which can change the modesetting
> +	 * permissions for DRM users.
> +	 */
> +	DRM_MASTER_FLUSH	= BIT(6),
>  };
>  
>  /**
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm: update the ioctl handler
  2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
                   ` (2 preceding siblings ...)
  2021-08-13 15:23 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: update the ioctl handler Patchwork
@ 2021-08-13 15:50 ` Patchwork
  2021-08-13 19:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2021-08-13 15:50 UTC (permalink / raw)
  To: Desmond Cheong Zhi Xi; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 6303 bytes --]

== Series Details ==

Series: drm: update the ioctl handler
URL   : https://patchwork.freedesktop.org/series/93677/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10482 -> Patchwork_20818
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/index.html

Known issues
------------

  Here are the changes found in Patchwork_20818 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][1] ([fdo#109315])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][2] ([fdo#109271]) +29 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][3] ([fdo#109315] / [i915#2575]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-bdw-5557u:       NOTRUN -> [WARN][4] ([i915#3718])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][6] ([i915#1155])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][7] ([i915#579])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@i915_pm_rpm@basic-rte.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][8] ([i915#579])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-bdw-5557u/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_pm_rpm@module-reload:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][9] ([i915#579]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][10] ([fdo#111827]) +8 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][11] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][12] ([fdo#109285])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][13] ([i915#1072]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][14] ([i915#3301])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - {fi-hsw-gt1}:       [DMESG-WARN][15] ([i915#3303]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/fi-hsw-gt1/igt@i915_selftest@live@hangcheck.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-hsw-gt1/igt@i915_selftest@live@hangcheck.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-u2:          [SKIP][17] ([i915#579]) -> [INCOMPLETE][18] ([i915#2405])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/fi-icl-u2/igt@i915_pm_rpm@module-reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/fi-icl-u2/igt@i915_pm_rpm@module-reload.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3718]: https://gitlab.freedesktop.org/drm/intel/issues/3718
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579


Participating hosts (35 -> 34)
------------------------------

  Additional (2): fi-tgl-1115g4 fi-bdw-5557u 
  Missing    (3): fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10482 -> Patchwork_20818

  CI-20190529: 20190529
  CI_DRM_10482: 772cc2d9bb24a9f750e3a88c6b2172873830e095 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6175: c91f99c74b966f635d7e2eb898bf0f78383d281b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20818: 02942448f1376d07fd5eb8bf8804beab5c1a5c0a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

02942448f137 drm: unexport drm_ioctl_permit
5b82a01bb586 drm: avoid races with modesetting rights

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/index.html

[-- Attachment #2: Type: text/html, Size: 7641 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit
  2021-08-13  8:54 ` [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit Desmond Cheong Zhi Xi
@ 2021-08-13 15:51   ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2021-08-13 15:51 UTC (permalink / raw)
  To: Desmond Cheong Zhi Xi
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig, dri-devel, linux-kernel,
	linux-media, linaro-mm-sig, intel-gfx, skhan, gregkh,
	linux-kernel-mentees, Daniel Vetter

On Fri, Aug 13, 2021 at 04:54:50PM +0800, Desmond Cheong Zhi Xi wrote:
> Since the last user of drm_ioctl_permit was removed, and it's now only
> used in drm_ioctl.c, unexport the symbol.
> 
> Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

Applied to drm-misc-next for 5.16, thanks for your patch.
-Daniel

> ---
>  drivers/gpu/drm/drm_ioctl.c | 15 +--------------
>  include/drm/drm_ioctl.h     |  1 -
>  2 files changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index eb4ec3fab7d1..fe271f6f96ab 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -522,19 +522,7 @@ int drm_version(struct drm_device *dev, void *data,
>  	return err;
>  }
>  
> -/**
> - * drm_ioctl_permit - Check ioctl permissions against caller
> - *
> - * @flags: ioctl permission flags.
> - * @file_priv: Pointer to struct drm_file identifying the caller.
> - *
> - * Checks whether the caller is allowed to run an ioctl with the
> - * indicated permissions.
> - *
> - * Returns:
> - * Zero if allowed, -EACCES otherwise.
> - */
> -int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
> +static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  {
>  	/* ROOT_ONLY is only for CAP_SYS_ADMIN */
>  	if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
> @@ -557,7 +545,6 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(drm_ioctl_permit);
>  
>  #define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
>  	[DRM_IOCTL_NR(ioctl)] = {		\
> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> index 13a68cdcea36..fd29842127e5 100644
> --- a/include/drm/drm_ioctl.h
> +++ b/include/drm/drm_ioctl.h
> @@ -174,7 +174,6 @@ struct drm_ioctl_desc {
>  		.name = #ioctl						\
>  	}
>  
> -int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
>  long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
>  long drm_ioctl_kernel(struct file *, drm_ioctl_t, void *, u32);
>  #ifdef CONFIG_COMPAT
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm: update the ioctl handler
  2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
                   ` (3 preceding siblings ...)
  2021-08-13 15:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2021-08-13 19:40 ` Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2021-08-13 19:40 UTC (permalink / raw)
  To: Desmond Cheong Zhi Xi; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30252 bytes --]

== Series Details ==

Series: drm: update the ioctl handler
URL   : https://patchwork.freedesktop.org/series/93677/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10482_full -> Patchwork_20818_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_20818_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([i915#658])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@feature_discovery@psr2.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@feature_discovery@psr2.html

  * igt@gem_create@create-massive:
    - shard-snb:          NOTRUN -> [DMESG-WARN][3] ([i915#3002])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-snb5/igt@gem_create@create-massive.html

  * igt@gem_ctx_persistence@legacy-engines-queued:
    - shard-snb:          NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#1099]) +4 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-snb7/igt@gem_ctx_persistence@legacy-engines-queued.html

  * igt@gem_ctx_persistence@many-contexts:
    - shard-tglb:         [PASS][5] -> [FAIL][6] ([i915#2410])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb7/igt@gem_ctx_persistence@many-contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@gem_ctx_persistence@many-contexts.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([i915#2842]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [PASS][9] -> [FAIL][10] ([i915#2842])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-kbl:          [PASS][11] -> [SKIP][12] ([fdo#109271])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-glk:          [PASS][13] -> [FAIL][14] ([i915#2842]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-glk5/igt@gem_exec_fair@basic-throttle@rcs0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-glk3/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_whisper@basic-contexts-forked-all:
    - shard-glk:          [PASS][15] -> [DMESG-WARN][16] ([i915#118] / [i915#95])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-glk3/igt@gem_exec_whisper@basic-contexts-forked-all.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-glk6/igt@gem_exec_whisper@basic-contexts-forked-all.html

  * igt@gem_mmap_gtt@cpuset-big-copy-xy:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([i915#307])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@gem_mmap_gtt@cpuset-big-copy-xy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@gem_mmap_gtt@cpuset-big-copy-xy.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [WARN][19] ([i915#2658])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl2/igt@gem_pread@exhaustion.html

  * igt@gen7_exec_parse@basic-offset:
    - shard-tglb:         NOTRUN -> [SKIP][20] ([fdo#109289])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@gen7_exec_parse@basic-offset.html

  * igt@i915_pm_rpm@pc8-residency:
    - shard-tglb:         NOTRUN -> [SKIP][21] ([i915#579])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@i915_pm_rpm@pc8-residency.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-apl:          NOTRUN -> [DMESG-WARN][22] ([i915#180])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl7/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_atomic_transition@plane-all-modeset-transition:
    - shard-tglb:         NOTRUN -> [SKIP][23] ([i915#1769])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_atomic_transition@plane-all-modeset-transition.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-iclb:         [PASS][24] -> [DMESG-WARN][25] ([i915#3621])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_big_fb@yf-tiled-addfb-size-overflow:
    - shard-tglb:         NOTRUN -> [SKIP][26] ([fdo#111615]) +2 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb8/igt@kms_big_fb@yf-tiled-addfb-size-overflow.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-apl:          NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#3777])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl7/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_ccs@pipe-a-bad-pixel-format-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][28] ([fdo#109271] / [i915#3886]) +4 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl2/igt@kms_ccs@pipe-a-bad-pixel-format-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][29] ([i915#3689] / [i915#3886])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_rc_ccs_cc:
    - shard-kbl:          NOTRUN -> [SKIP][30] ([fdo#109271] / [i915#3886])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-pixel-format-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][31] ([i915#3689])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_ccs@pipe-c-bad-pixel-format-yf_tiled_ccs.html

  * igt@kms_chamelium@hdmi-audio-edid:
    - shard-kbl:          NOTRUN -> [SKIP][32] ([fdo#109271] / [fdo#111827]) +5 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_chamelium@hdmi-audio-edid.html

  * igt@kms_chamelium@vga-edid-read:
    - shard-apl:          NOTRUN -> [SKIP][33] ([fdo#109271] / [fdo#111827]) +12 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl1/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_chamelium@vga-hpd-fast:
    - shard-skl:          NOTRUN -> [SKIP][34] ([fdo#109271] / [fdo#111827])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl2/igt@kms_chamelium@vga-hpd-fast.html

  * igt@kms_color_chamelium@pipe-c-ctm-red-to-blue:
    - shard-snb:          NOTRUN -> [SKIP][35] ([fdo#109271] / [fdo#111827]) +12 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-snb5/igt@kms_color_chamelium@pipe-c-ctm-red-to-blue.html

  * igt@kms_color_chamelium@pipe-d-ctm-blue-to-red:
    - shard-tglb:         NOTRUN -> [SKIP][36] ([fdo#109284] / [fdo#111827]) +3 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_color_chamelium@pipe-d-ctm-blue-to-red.html

  * igt@kms_content_protection@uevent:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([fdo#111828])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb8/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_crc@pipe-a-cursor-512x170-onscreen:
    - shard-skl:          NOTRUN -> [SKIP][38] ([fdo#109271]) +4 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl2/igt@kms_cursor_crc@pipe-a-cursor-512x170-onscreen.html

  * igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][39] ([i915#3319])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-max-size-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][40] ([i915#3359]) +1 similar issue
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_cursor_crc@pipe-b-cursor-max-size-rapid-movement.html

  * igt@kms_cursor_crc@pipe-c-cursor-512x512-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][41] ([fdo#109279] / [i915#3359])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_cursor_crc@pipe-c-cursor-512x512-rapid-movement.html

  * igt@kms_cursor_crc@pipe-d-cursor-suspend:
    - shard-kbl:          NOTRUN -> [SKIP][42] ([fdo#109271]) +67 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_cursor_crc@pipe-d-cursor-suspend.html

  * igt@kms_cursor_edge_walk@pipe-a-128x128-bottom-edge:
    - shard-skl:          [PASS][43] -> [DMESG-WARN][44] ([i915#1982])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl1/igt@kms_cursor_edge_walk@pipe-a-128x128-bottom-edge.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl4/igt@kms_cursor_edge_walk@pipe-a-128x128-bottom-edge.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][45] -> [FAIL][46] ([i915#2346])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@pipe-d-torture-bo:
    - shard-skl:          NOTRUN -> [SKIP][47] ([fdo#109271] / [i915#533])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl2/igt@kms_cursor_legacy@pipe-d-torture-bo.html

  * igt@kms_flip@flip-vs-expired-vblank@c-edp1:
    - shard-skl:          [PASS][48] -> [FAIL][49] ([i915#79])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl2/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl9/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-edp1:
    - shard-iclb:         [PASS][50] -> [INCOMPLETE][51] ([i915#2910])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb3/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb3/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
    - shard-skl:          [PASS][52] -> [FAIL][53] ([i915#2122])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-msflip-blt:
    - shard-snb:          NOTRUN -> [SKIP][54] ([fdo#109271]) +293 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-snb7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][55] -> [DMESG-WARN][56] ([i915#180]) +8 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-shrfb-plflip-blt:
    - shard-tglb:         NOTRUN -> [SKIP][57] ([fdo#111825]) +10 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-shrfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-2p-pri-indfb-multidraw:
    - shard-apl:          NOTRUN -> [SKIP][58] ([fdo#109271]) +179 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl1/igt@kms_frontbuffer_tracking@psr-2p-pri-indfb-multidraw.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:
    - shard-kbl:          NOTRUN -> [SKIP][59] ([fdo#109271] / [i915#533])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][60] ([fdo#108145] / [i915#265]) +2 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl1/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
    - shard-skl:          NOTRUN -> [FAIL][61] ([fdo#108145] / [i915#265])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
    - shard-kbl:          NOTRUN -> [FAIL][62] ([fdo#108145] / [i915#265])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl3/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-kbl:          NOTRUN -> [FAIL][63] ([i915#265])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-0:
    - shard-tglb:         NOTRUN -> [SKIP][64] ([i915#2920])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][65] ([fdo#109271] / [i915#658]) +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl3/igt@kms_psr2_sf@plane-move-sf-dmg-area-2.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-3:
    - shard-kbl:          NOTRUN -> [SKIP][66] ([fdo#109271] / [i915#658]) +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_psr2_sf@plane-move-sf-dmg-area-3.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][67] -> [SKIP][68] ([fdo#109642] / [fdo#111068] / [i915#658])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-tglb:         NOTRUN -> [FAIL][69] ([i915#132] / [i915#3467])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_psr@psr2_primary_mmap_gtt.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][70] -> [SKIP][71] ([fdo#109441])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vrr@flip-suspend:
    - shard-tglb:         NOTRUN -> [SKIP][72] ([fdo#109502])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_vrr@flip-suspend.html

  * igt@kms_writeback@writeback-check-output:
    - shard-skl:          NOTRUN -> [SKIP][73] ([fdo#109271] / [i915#2437])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl3/igt@kms_writeback@writeback-check-output.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-apl:          NOTRUN -> [SKIP][74] ([fdo#109271] / [i915#2437])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl2/igt@kms_writeback@writeback-fb-id.html

  * igt@perf_pmu@rc6-runtime-pm:
    - shard-tglb:         NOTRUN -> [SKIP][75] ([fdo#111719])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@perf_pmu@rc6-runtime-pm.html

  * igt@prime_nv_test@nv_write_i915_gtt_mmap_read:
    - shard-tglb:         NOTRUN -> [SKIP][76] ([fdo#109291])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb8/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html

  * igt@prime_vgem@fence-read-hang:
    - shard-tglb:         NOTRUN -> [SKIP][77] ([fdo#109295])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@prime_vgem@fence-read-hang.html

  * igt@runner@aborted:
    - shard-snb:          NOTRUN -> [FAIL][78] ([i915#3002])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-snb5/igt@runner@aborted.html

  * igt@sysfs_clients@fair-1:
    - shard-apl:          NOTRUN -> [SKIP][79] ([fdo#109271] / [i915#2994]) +2 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl7/igt@sysfs_clients@fair-1.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@smoketest:
    - shard-tglb:         [FAIL][80] ([i915#2896]) -> [PASS][81]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb2/igt@gem_ctx_persistence@smoketest.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb5/igt@gem_ctx_persistence@smoketest.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-tglb:         [FAIL][82] ([i915#2842]) -> [PASS][83] +1 similar issue
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb5/igt@gem_exec_fair@basic-flow@rcs0.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb6/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [FAIL][84] ([i915#2842]) -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb4/igt@gem_exec_fair@basic-none-share@rcs0.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb6/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][86] ([i915#2849]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb4/igt@gem_exec_fair@basic-throttle@rcs0.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb6/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_softpin@allocator-evict@rcs0:
    - shard-glk:          [DMESG-WARN][88] ([i915#118] / [i915#95]) -> [PASS][89] +1 similar issue
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-glk9/igt@gem_softpin@allocator-evict@rcs0.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-glk6/igt@gem_softpin@allocator-evict@rcs0.html

  * igt@gem_spin_batch@spin-each:
    - shard-skl:          [FAIL][90] ([i915#2898]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl3/igt@gem_spin_batch@spin-each.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl10/igt@gem_spin_batch@spin-each.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [DMESG-WARN][92] ([i915#1436] / [i915#716]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl8/igt@gen9_exec_parse@allowed-single.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl2/igt@gen9_exec_parse@allowed-single.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding:
    - shard-skl:          [FAIL][94] ([i915#3444]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl10/igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-128x42-sliding.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-skl:          [FAIL][96] ([i915#2346]) -> [PASS][97]
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
    - shard-skl:          [FAIL][98] ([i915#79]) -> [PASS][99] +1 similar issue
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-apl:          [DMESG-WARN][100] ([i915#180]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [DMESG-WARN][102] ([i915#180]) -> [PASS][103] +3 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1:
    - shard-skl:          [FAIL][104] ([i915#2122]) -> [PASS][105]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [FAIL][106] ([i915#1188]) -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl5/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][108] ([fdo#108145] / [i915#265]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][110] ([fdo#109441]) -> [PASS][111] +2 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb7/igt@kms_psr@psr2_cursor_render.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_vblank@pipe-c-wait-forked-hang:
    - shard-tglb:         [INCOMPLETE][112] -> [PASS][113]
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb7/igt@kms_vblank@pipe-c-wait-forked-hang.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb1/igt@kms_vblank@pipe-c-wait-forked-hang.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][114] ([i915#1542]) -> [PASS][115] +1 similar issue
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl6/igt@perf@blocking.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl3/igt@perf@blocking.html

  * igt@perf@polling-parameterized:
    - shard-glk:          [FAIL][116] ([i915#1542]) -> [PASS][117]
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-glk8/igt@perf@polling-parameterized.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-glk5/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - shard-tglb:         [SKIP][118] ([i915#2848]) -> [FAIL][119] ([i915#2842])
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-tglb6/igt@gem_exec_fair@basic-none-vip@rcs0.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-tglb8/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3:
    - shard-iclb:         [SKIP][120] ([i915#2920]) -> [SKIP][121] ([i915#658]) +1 similar issue
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-1:
    - shard-iclb:         [SKIP][122] ([i915#658]) -> [SKIP][123] ([i915#2920]) +2 similar issues
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb6/igt@kms_psr2_sf@plane-move-sf-dmg-area-1.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb2/igt@kms_psr2_sf@plane-move-sf-dmg-area-1.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][124], [FAIL][125], [FAIL][126], [FAIL][127], [FAIL][128]) ([i915#180] / [i915#2505] / [i915#3002] / [i915#3363]) -> ([FAIL][129], [FAIL][130], [FAIL][131], [FAIL][132], [FAIL][133], [FAIL][134], [FAIL][135], [FAIL][136], [FAIL][137]) ([fdo#109271] / [i915#1436] / [i915#180] / [i915#1814] / [i915#3002] / [i915#3363] / [i915#602])
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl1/igt@runner@aborted.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl6/igt@runner@aborted.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl1/igt@runner@aborted.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl1/igt@runner@aborted.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-kbl1/igt@runner@aborted.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl4/igt@runner@aborted.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-kbl1/igt@runner@aborted.html
    - shard-iclb:         ([FAIL][138], [FAIL][139]) ([i915#3002]) -> ([FAIL][140], [FAIL][141], [FAIL][142]) ([i915#1814] / [i915#3002])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb8/igt@runner@aborted.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-iclb6/igt@runner@aborted.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb1/igt@runner@aborted.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb4/igt@runner@aborted.html
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-iclb7/igt@runner@aborted.html
    - shard-apl:          ([FAIL][143], [FAIL][144], [FAIL][145], [FAIL][146], [FAIL][147]) ([i915#180] / [i915#1814] / [i915#3002] / [i915#3363]) -> ([FAIL][148], [FAIL][149], [FAIL][150]) ([i915#180] / [i915#3002] / [i915#3363])
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl7/igt@runner@aborted.html
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl1/igt@runner@aborted.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl2/igt@runner@aborted.html
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl3/igt@runner@aborted.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-apl8/igt@runner@aborted.html
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl7/igt@runner@aborted.html
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl8/igt@runner@aborted.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-apl1/igt@runner@aborted.html
    - shard-skl:          ([FAIL][151], [FAIL][152], [FAIL][153]) ([i915#1436] / [i915#3002] / [i915#3363]) -> ([FAIL][154], [FAIL][155]) ([i915#3002] / [i915#3363])
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl8/igt@runner@aborted.html
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl8/igt@runner@aborted.html
   [153]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10482/shard-skl5/igt@runner@aborted.html
   [154]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl7/igt@runner@aborted.html
   [155]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/shard-skl2/igt@runner@aborted.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#10

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20818/index.html

[-- Attachment #2: Type: text/html, Size: 35165 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights
  2021-08-13 15:49   ` Daniel Vetter
@ 2021-08-15  7:10     ` Desmond Cheong Zhi Xi
  0 siblings, 0 replies; 9+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-15  7:10 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, sumit.semwal,
	christian.koenig, dri-devel, linux-kernel, linux-media,
	linaro-mm-sig, intel-gfx, skhan, gregkh, linux-kernel-mentees

On 13/8/21 11:49 pm, Daniel Vetter wrote:
> On Fri, Aug 13, 2021 at 04:54:49PM +0800, Desmond Cheong Zhi Xi wrote:
>> In drm_client_modeset.c and drm_fb_helper.c,
>> drm_master_internal_{acquire,release} are used to avoid races with DRM
>> userspace. These functions hold onto drm_device.master_mutex while
>> committing, and bail if there's already a master.
>>
>> However, ioctls can still race between themselves. A
>> time-of-check-to-time-of-use error can occur if an ioctl that changes
>> the modeset has its rights revoked after it validates its permissions,
>> but before it completes.
>>
>> There are three ioctls that can affect modesetting permissions:
>>
>> - DROP_MASTER ioctl removes rights for a master and its leases
>>
>> - REVOKE_LEASE ioctl revokes rights for a specific lease
>>
>> - SET_MASTER ioctl sets the device master if the master role hasn't
>> been acquired yet
>>
>> All these races can be avoided by introducing an SRCU that acts as a
>> barrier for ioctls that can change modesetting permissions. Processes
>> that perform modesetting should hold a read lock on the new
>> drm_device.master_barrier_srcu, and ioctls that change these
>> permissions should call synchronize_srcu before returning.
>>
>> This ensures that any process that might have seen old permissions are
>> flushed out before DROP_MASTER/REVOKE_LEASE/SET_MASTER ioctls return
>> to userspace.
>>
>> Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
> 
> This looks pretty solid, but I think there's one gap where we can still
> race. Scenario.
> 
> Process A has a drm fd with master rights and two threads:
> - thread 1 does a long-running display operation (like a modeset or
>    whatever)
> - thread 2 does a drop-master
> 
> Then we start a new process B, which acquires master in drm_open (there is
> no other one left). This is like setmaster ioctl, but your
> DRM_MASTER_FLUSH bit doesn't work there.
> 

Ah, I see the race. I think a good place to plug this would be in 
drm_master_open using the drm_master_flush (or 
drm_master_unlock_and_flush) that you suggested below.

> The other thing is that for modeset stuff (which this all is) srcu is
> probably massive overkill, and a simple rwsem should be good enough too.
> Maybe even better, since the rwsem guarantees that no new reader can start
> once you try to acquire the write side.
> 

Makes sense, I'll switch to a rwsem then.

> Finally, and this is a bit a bikeshed: I don't like much how
> DRM_MASTER_FLUSH leaks the need of these very few places into the very
> core drm_ioctl function. One idea I had was to use task_work in a special
> function, roughly
> 
> void master_flush()
> {
> 	down_write(master_rwsem);
> 	up_write(master_rwms);
> }
> void drm_master_flush()
> {
> 	init_task_work(fpriv->master_flush_work, master_flush)
> 	task_work_add(fpriv->master_flush_work);
> 	/* if task_work_add fails we're exiting, at which point the lack
> 	 * of master flush doesn't matter);
> }
> 
> And maybe put a comment above the function explaining why and how this
> works.
> 
> We could even do a drm_master_unlock_and_flush helper, since that's really
> what everyone wants, and it would make it very clear which master state
> changes need this flush. Instead of setting a flag bit in an ioctl table
> very far away ...
> 
> Thoughts?
> -Daniel
> 

Sounds good. I wasn't aware of the task_work_add mechanism to queue work 
before the task returns to usermode, but this seems like a more explicit 
way to flush.

Thanks for the feedback, Daniel. I'll fix this up in a v2.

>> ---
>>   drivers/gpu/drm/drm_auth.c           | 17 ++++++++++++++---
>>   drivers/gpu/drm/drm_client_modeset.c | 10 ++++++----
>>   drivers/gpu/drm/drm_drv.c            |  2 ++
>>   drivers/gpu/drm/drm_fb_helper.c      | 20 ++++++++++++--------
>>   drivers/gpu/drm/drm_internal.h       |  5 +++--
>>   drivers/gpu/drm/drm_ioctl.c          | 25 +++++++++++++++++++++----
>>   include/drm/drm_device.h             | 11 +++++++++++
>>   include/drm/drm_ioctl.h              |  7 +++++++
>>   8 files changed, 76 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
>> index 60a6b21474b1..004506608e76 100644
>> --- a/drivers/gpu/drm/drm_auth.c
>> +++ b/drivers/gpu/drm/drm_auth.c
>> @@ -29,6 +29,7 @@
>>    */
>>   
>>   #include <linux/slab.h>
>> +#include <linux/srcu.h>
>>   
>>   #include <drm/drm_auth.h>
>>   #include <drm/drm_drv.h>
>> @@ -448,21 +449,31 @@ void drm_master_put(struct drm_master **master)
>>   EXPORT_SYMBOL(drm_master_put);
>>   
>>   /* Used by drm_client and drm_fb_helper */
>> -bool drm_master_internal_acquire(struct drm_device *dev)
>> +bool drm_master_internal_acquire(struct drm_device *dev, int *idx)
>>   {
>> +	*idx = srcu_read_lock(&dev->master_barrier_srcu);
>> +
>>   	mutex_lock(&dev->master_mutex);
>>   	if (dev->master) {
>>   		mutex_unlock(&dev->master_mutex);
>> +		srcu_read_unlock(&dev->master_barrier_srcu, *idx);
>>   		return false;
>>   	}
>> +	mutex_unlock(&dev->master_mutex);
>>   
>>   	return true;
>>   }
>>   EXPORT_SYMBOL(drm_master_internal_acquire);
>>   
>>   /* Used by drm_client and drm_fb_helper */
>> -void drm_master_internal_release(struct drm_device *dev)
>> +void drm_master_internal_release(struct drm_device *dev, int idx)
>>   {
>> -	mutex_unlock(&dev->master_mutex);
>> +	srcu_read_unlock(&dev->master_barrier_srcu, idx);
>>   }
>>   EXPORT_SYMBOL(drm_master_internal_release);
>> +
>> +/* Used by drm_ioctl */
>> +void drm_master_flush(struct drm_device *dev)
>> +{
>> +	synchronize_srcu(&dev->master_barrier_srcu);
>> +}
>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
>> index ced09c7c06f9..9885f36f71b7 100644
>> --- a/drivers/gpu/drm/drm_client_modeset.c
>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> @@ -1165,13 +1165,14 @@ int drm_client_modeset_commit(struct drm_client_dev *client)
>>   {
>>   	struct drm_device *dev = client->dev;
>>   	int ret;
>> +	int idx;
>>   
>> -	if (!drm_master_internal_acquire(dev))
>> +	if (!drm_master_internal_acquire(dev, &idx))
>>   		return -EBUSY;
>>   
>>   	ret = drm_client_modeset_commit_locked(client);
>>   
>> -	drm_master_internal_release(dev);
>> +	drm_master_internal_release(dev, idx);
>>   
>>   	return ret;
>>   }
>> @@ -1215,8 +1216,9 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
>>   {
>>   	struct drm_device *dev = client->dev;
>>   	int ret = 0;
>> +	int idx;
>>   
>> -	if (!drm_master_internal_acquire(dev))
>> +	if (!drm_master_internal_acquire(dev, &idx))
>>   		return -EBUSY;
>>   
>>   	mutex_lock(&client->modeset_mutex);
>> @@ -1226,7 +1228,7 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
>>   		drm_client_modeset_dpms_legacy(client, mode);
>>   	mutex_unlock(&client->modeset_mutex);
>>   
>> -	drm_master_internal_release(dev);
>> +	drm_master_internal_release(dev, idx);
>>   
>>   	return ret;
>>   }
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 7a5097467ba5..c313f0674db3 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -574,6 +574,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
>>   	mutex_destroy(&dev->clientlist_mutex);
>>   	mutex_destroy(&dev->filelist_mutex);
>>   	mutex_destroy(&dev->struct_mutex);
>> +	cleanup_srcu_struct(&dev->master_barrier_srcu);
>>   	drm_legacy_destroy_members(dev);
>>   }
>>   
>> @@ -612,6 +613,7 @@ static int drm_dev_init(struct drm_device *dev,
>>   	mutex_init(&dev->filelist_mutex);
>>   	mutex_init(&dev->clientlist_mutex);
>>   	mutex_init(&dev->master_mutex);
>> +	init_srcu_struct(&dev->master_barrier_srcu);
>>   
>>   	ret = drmm_add_action(dev, drm_dev_init_release, NULL);
>>   	if (ret)
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 3ab078321045..0d594bc15f18 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1116,13 +1116,14 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>>   	struct drm_fb_helper *fb_helper = info->par;
>>   	struct drm_device *dev = fb_helper->dev;
>>   	int ret;
>> +	int idx;
>>   
>>   	if (oops_in_progress)
>>   		return -EBUSY;
>>   
>>   	mutex_lock(&fb_helper->lock);
>>   
>> -	if (!drm_master_internal_acquire(dev)) {
>> +	if (!drm_master_internal_acquire(dev, &idx)) {
>>   		ret = -EBUSY;
>>   		goto unlock;
>>   	}
>> @@ -1136,7 +1137,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>>   		ret = setcmap_legacy(cmap, info);
>>   	mutex_unlock(&fb_helper->client.modeset_mutex);
>>   
>> -	drm_master_internal_release(dev);
>> +	drm_master_internal_release(dev, idx);
>>   unlock:
>>   	mutex_unlock(&fb_helper->lock);
>>   
>> @@ -1160,9 +1161,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>>   	struct drm_device *dev = fb_helper->dev;
>>   	struct drm_crtc *crtc;
>>   	int ret = 0;
>> +	int idx;
>>   
>>   	mutex_lock(&fb_helper->lock);
>> -	if (!drm_master_internal_acquire(dev)) {
>> +	if (!drm_master_internal_acquire(dev, &idx)) {
>>   		ret = -EBUSY;
>>   		goto unlock;
>>   	}
>> @@ -1204,7 +1206,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>>   		ret = -ENOTTY;
>>   	}
>>   
>> -	drm_master_internal_release(dev);
>> +	drm_master_internal_release(dev, idx);
>>   unlock:
>>   	mutex_unlock(&fb_helper->lock);
>>   	return ret;
>> @@ -1474,12 +1476,13 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>>   	struct drm_fb_helper *fb_helper = info->par;
>>   	struct drm_device *dev = fb_helper->dev;
>>   	int ret;
>> +	int idx;
>>   
>>   	if (oops_in_progress)
>>   		return -EBUSY;
>>   
>>   	mutex_lock(&fb_helper->lock);
>> -	if (!drm_master_internal_acquire(dev)) {
>> +	if (!drm_master_internal_acquire(dev, &idx)) {
>>   		ret = -EBUSY;
>>   		goto unlock;
>>   	}
>> @@ -1489,7 +1492,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>>   	else
>>   		ret = pan_display_legacy(var, info);
>>   
>> -	drm_master_internal_release(dev);
>> +	drm_master_internal_release(dev, idx);
>>   unlock:
>>   	mutex_unlock(&fb_helper->lock);
>>   
>> @@ -1948,6 +1951,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>>   int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>>   {
>>   	int err = 0;
>> +	int idx;
>>   
>>   	if (!drm_fbdev_emulation || !fb_helper)
>>   		return 0;
>> @@ -1959,13 +1963,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>>   		return err;
>>   	}
>>   
>> -	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
>> +	if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev, &idx)) {
>>   		fb_helper->delayed_hotplug = true;
>>   		mutex_unlock(&fb_helper->lock);
>>   		return err;
>>   	}
>>   
>> -	drm_master_internal_release(fb_helper->dev);
>> +	drm_master_internal_release(fb_helper->dev, idx);
>>   
>>   	drm_dbg_kms(fb_helper->dev, "\n");
>>   
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index 17f3548c8ed2..578fd2769913 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -142,8 +142,9 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
>>   			 struct drm_file *file_priv);
>>   int drm_master_open(struct drm_file *file_priv);
>>   void drm_master_release(struct drm_file *file_priv);
>> -bool drm_master_internal_acquire(struct drm_device *dev);
>> -void drm_master_internal_release(struct drm_device *dev);
>> +bool drm_master_internal_acquire(struct drm_device *dev, int *idx);
>> +void drm_master_internal_release(struct drm_device *dev, int idx);
>> +void drm_master_flush(struct drm_device *dev);
>>   
>>   /* drm_sysfs.c */
>>   extern struct class *drm_class;
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index be4a52dc4d6f..eb4ec3fab7d1 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -600,8 +600,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>>   	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>   	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH),
>>   
>> -	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, 0),
>> -	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, 0),
>> +	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl,
>> +		      DRM_MASTER_FLUSH),
>> +	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl,
>> +		      DRM_MASTER_FLUSH),
>>   
>>   	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY),
>>   	DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>> @@ -722,7 +724,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>>   	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER),
>>   	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER),
>>   	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER),
>> -	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER),
>> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl,
>> +		      DRM_MASTER | DRM_MASTER_FLUSH),
>>   };
>>   
>>   #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
>> @@ -781,13 +784,17 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
>>   	struct drm_file *file_priv = file->private_data;
>>   	struct drm_device *dev = file_priv->minor->dev;
>>   	int retcode;
>> +	int idx;
>>   
>>   	if (drm_dev_is_unplugged(dev))
>>   		return -ENODEV;
>>   
>> +	if (unlikely(flags & DRM_MASTER))
>> +		idx = srcu_read_lock(&dev->master_barrier_srcu);
>> +
>>   	retcode = drm_ioctl_permit(flags, file_priv);
>>   	if (unlikely(retcode))
>> -		return retcode;
>> +		goto release_master;
>>   
>>   	/* Enforce sane locking for modern driver ioctls. */
>>   	if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
>> @@ -798,6 +805,16 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
>>   		retcode = func(dev, kdata, file_priv);
>>   		mutex_unlock(&drm_global_mutex);
>>   	}
>> +
>> +release_master:
>> +	if (unlikely(flags & DRM_MASTER))
>> +		srcu_read_unlock(&dev->master_barrier_srcu, idx);
>> +	/* After flushing, processes are guaranteed to see the new master/lease
>> +	 * permissions, and any process which might have seen the old
>> +	 * permissions is guaranteed to have finished.
>> +	 */
>> +	if (unlikely(flags & DRM_MASTER_FLUSH))
>> +		drm_master_flush(dev);
>>   	return retcode;
>>   }
>>   EXPORT_SYMBOL(drm_ioctl_kernel);
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 604b1d1b2d72..0ac5fdb375f8 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -111,6 +111,17 @@ struct drm_device {
>>   	 */
>>   	struct drm_master *master;
>>   
>> +	/**
>> +	 * @master_barrier_srcu:
>> +	 *
>> +	 * Used to synchronize modesetting rights between multiple users. Users
>> +	 * that can change the modeset or display state must hold an
>> +	 * srcu_read_lock() on @master_barrier_srcu, and ioctls that can change
>> +	 * modesetting rights should call synchronize_srcu() before returning
>> +	 * to userspace.
>> +	 */
>> +	struct srcu_struct master_barrier_srcu;
>> +
>>   	/**
>>   	 * @driver_features: per-device driver features
>>   	 *
>> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
>> index afb27cb6a7bd..13a68cdcea36 100644
>> --- a/include/drm/drm_ioctl.h
>> +++ b/include/drm/drm_ioctl.h
>> @@ -130,6 +130,13 @@ enum drm_ioctl_flags {
>>   	 * not set DRM_AUTH because they do not require authentication.
>>   	 */
>>   	DRM_RENDER_ALLOW	= BIT(5),
>> +	/**
>> +	 * @DRM_MASTER_FLUSH:
>> +	 *
>> +	 * This must be set for any ioctl which can change the modesetting
>> +	 * permissions for DRM users.
>> +	 */
>> +	DRM_MASTER_FLUSH	= BIT(6),
>>   };
>>   
>>   /**
>> -- 
>> 2.25.1
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-16  9:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13  8:54 [Intel-gfx] [PATCH 0/2] drm: update the ioctl handler Desmond Cheong Zhi Xi
2021-08-13  8:54 ` [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
2021-08-13 15:49   ` Daniel Vetter
2021-08-15  7:10     ` Desmond Cheong Zhi Xi
2021-08-13  8:54 ` [Intel-gfx] [PATCH 2/2] drm: unexport drm_ioctl_permit Desmond Cheong Zhi Xi
2021-08-13 15:51   ` Daniel Vetter
2021-08-13 15:23 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: update the ioctl handler Patchwork
2021-08-13 15:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-08-13 19:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).