diff mbox series

[v2,07/12] drm/i915: Prevent modesets during driver init/shutdown

Message ID 20240104132335.2766434-1-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Imre Deak Jan. 4, 2024, 1:23 p.m. UTC
An unexpected modeset or connector detection by a user (user space or FB
console) during the initialization/shutdown sequence is possible either
via a hotplug IRQ handling work or via the connector sysfs
(status/detect) interface. These modesets/detections should be prevented
by disabling/flushing all related hotplug handling work and
unregistering the interfaces that can start them at the beginning of the
shutdown sequence. Some of this - disabling all related intel_hotplug
work - will be done by the next patch, but others - for instance
disabling the MST hotplug works - require a bigger rework.

It makes sense - for diagnostic purpose, even with all the above work and
interface disabled - to detect and reject any such user access. This
patch does that for modeset accesses and a follow-up patch for connector
detection.

After the display is disabled during the shutdown sequence, no modeset
should happen so it's disabled for both users and the shutdown thread.

v2: Call intel_display_driver_suspend_access()/resume_access() only
    for HAS_DISPLAY(). (CI)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  3 +
 .../gpu/drm/i915/display/intel_display_core.h |  7 ++
 .../drm/i915/display/intel_display_driver.c   | 74 +++++++++++++++++++
 .../drm/i915/display/intel_display_driver.h   |  6 ++
 drivers/gpu/drm/i915/i915_driver.c            | 19 ++++-
 5 files changed, 107 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 927d124457b61..31a6a82c12616 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6310,6 +6310,9 @@  int intel_atomic_check(struct drm_device *dev,
 	int ret, i;
 	bool any_ms = false;
 
+	if (!intel_display_driver_check_access(dev_priv))
+		return -ENODEV;
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		/*
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 47297ed858223..0b130ca9e6698 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -28,6 +28,8 @@ 
 #include "intel_opregion.h"
 #include "intel_wm_types.h"
 
+struct task_struct;
+
 struct drm_i915_private;
 struct drm_property;
 struct drm_property_blob;
@@ -298,6 +300,11 @@  struct intel_display {
 		const struct intel_audio_funcs *audio;
 	} funcs;
 
+	struct {
+		bool any_task_allowed;
+		struct task_struct *allowed_task;
+	} access;
+
 	struct {
 		/* backlight registers and fields in struct intel_panel */
 		struct mutex lock;
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 1974f2394a518..b2441ab9822c2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -45,6 +45,7 @@ 
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
 #include "intel_hti.h"
+#include "intel_modeset_lock.h"
 #include "intel_modeset_setup.h"
 #include "intel_opregion.h"
 #include "intel_overlay.h"
@@ -276,6 +277,71 @@  int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
 	return ret;
 }
 
+static void set_display_access(struct drm_i915_private *i915,
+			       bool any_task_allowed,
+			       struct task_struct *allowed_task)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	int err;
+
+	intel_modeset_lock_ctx_retry(&ctx, NULL, 0, err) {
+		err = drm_modeset_lock_all_ctx(&i915->drm, &ctx);
+		if (err)
+			continue;
+
+		i915->display.access.any_task_allowed = any_task_allowed;
+		i915->display.access.allowed_task = allowed_task;
+	}
+
+	drm_WARN_ON(&i915->drm, err);
+}
+
+void intel_display_driver_enable_user_access(struct drm_i915_private *i915)
+{
+	set_display_access(i915, true, NULL);
+}
+
+void intel_display_driver_disable_user_access(struct drm_i915_private *i915)
+{
+	set_display_access(i915, false, current);
+}
+
+void intel_display_driver_suspend_access(struct drm_i915_private *i915)
+{
+	set_display_access(i915, false, NULL);
+}
+
+void intel_display_driver_resume_access(struct drm_i915_private *i915)
+{
+	set_display_access(i915, false, current);
+}
+
+bool intel_display_driver_check_access(struct drm_i915_private *i915)
+{
+	char comm[TASK_COMM_LEN];
+	char current_task[TASK_COMM_LEN + 16];
+	char allowed_task[TASK_COMM_LEN + 16] = "none";
+
+	if (i915->display.access.any_task_allowed ||
+	    i915->display.access.allowed_task == current)
+		return true;
+
+	snprintf(current_task, sizeof(current_task), "%s[%d]",
+		 get_task_comm(comm, current),
+		 task_pid_vnr(current));
+
+	if (i915->display.access.allowed_task)
+		snprintf(allowed_task, sizeof(allowed_task), "%s[%d]",
+			 get_task_comm(comm, i915->display.access.allowed_task),
+			 task_pid_vnr(i915->display.access.allowed_task));
+
+	drm_dbg_kms(&i915->drm,
+		    "Reject display access from task %s (allowed to %s)\n",
+		    current_task, allowed_task);
+
+	return false;
+}
+
 /* part #2: call after irq install, but before gem init */
 int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
 {
@@ -326,6 +392,8 @@  int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
 	intel_vga_disable(i915);
 	intel_setup_outputs(i915);
 
+	intel_display_driver_disable_user_access(i915);
+
 	drm_modeset_lock_all(dev);
 	intel_modeset_setup_hw_state(i915, dev->mode_config.acquire_ctx);
 	intel_acpi_assign_connector_fwnodes(i915);
@@ -393,6 +461,8 @@  void intel_display_driver_register(struct drm_i915_private *i915)
 
 	intel_audio_init(i915);
 
+	intel_display_driver_enable_user_access(i915);
+
 	intel_display_debugfs_register(i915);
 
 	/*
@@ -440,6 +510,8 @@  void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return;
 
+	intel_display_driver_suspend_access(i915);
+
 	/*
 	 * Due to the hpd irq storm handling the hotplug work can re-arm the
 	 * poll handlers. Hence disable polling after hpd handling is shut down.
@@ -493,6 +565,8 @@  void intel_display_driver_unregister(struct drm_i915_private *i915)
 	 */
 	drm_kms_helper_poll_fini(&i915->drm);
 
+	intel_display_driver_disable_user_access(i915);
+
 	intel_audio_deinit(i915);
 
 	drm_atomic_helper_shutdown(&i915->drm);
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h b/drivers/gpu/drm/i915/display/intel_display_driver.h
index c276a58ee3293..42cc4af6d3fd5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.h
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.h
@@ -32,5 +32,11 @@  int __intel_display_driver_resume(struct drm_i915_private *i915,
 				  struct drm_atomic_state *state,
 				  struct drm_modeset_acquire_ctx *ctx);
 
+void intel_display_driver_enable_user_access(struct drm_i915_private *i915);
+void intel_display_driver_disable_user_access(struct drm_i915_private *i915);
+void intel_display_driver_suspend_access(struct drm_i915_private *i915);
+void intel_display_driver_resume_access(struct drm_i915_private *i915);
+bool intel_display_driver_check_access(struct drm_i915_private *i915);
+
 #endif /* __INTEL_DISPLAY_DRIVER_H__ */
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 166476948c896..a951050f6a75a 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1006,6 +1006,7 @@  void i915_driver_shutdown(struct drm_i915_private *i915)
 	intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true);
 	if (HAS_DISPLAY(i915)) {
 		drm_kms_helper_poll_disable(&i915->drm);
+		intel_display_driver_disable_user_access(i915);
 
 		drm_atomic_helper_shutdown(&i915->drm);
 	}
@@ -1015,6 +1016,9 @@  void i915_driver_shutdown(struct drm_i915_private *i915)
 	intel_runtime_pm_disable_interrupts(i915);
 	intel_hpd_cancel_work(i915);
 
+	if (HAS_DISPLAY(i915))
+		intel_display_driver_suspend_access(i915);
+
 	intel_suspend_encoders(i915);
 	intel_shutdown_encoders(i915);
 
@@ -1082,8 +1086,10 @@  static int i915_drm_suspend(struct drm_device *dev)
 	 * properly. */
 	intel_power_domains_disable(dev_priv);
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
-	if (HAS_DISPLAY(dev_priv))
+	if (HAS_DISPLAY(dev_priv)) {
 		drm_kms_helper_poll_disable(dev);
+		intel_display_driver_disable_user_access(dev_priv);
+	}
 
 	pci_save_state(pdev);
 
@@ -1094,6 +1100,9 @@  static int i915_drm_suspend(struct drm_device *dev)
 	intel_runtime_pm_disable_interrupts(dev_priv);
 	intel_hpd_cancel_work(dev_priv);
 
+	if (HAS_DISPLAY(dev_priv))
+		intel_display_driver_suspend_access(dev_priv);
+
 	intel_suspend_encoders(dev_priv);
 
 	/* Must be called before GGTT is suspended. */
@@ -1243,14 +1252,20 @@  static int i915_drm_resume(struct drm_device *dev)
 	intel_display_driver_init_hw(dev_priv);
 
 	intel_clock_gating_init(dev_priv);
+
+	if (HAS_DISPLAY(dev_priv))
+		intel_display_driver_resume_access(dev_priv);
+
 	intel_hpd_init(dev_priv);
 
 	/* MST sideband requires HPD interrupts enabled */
 	intel_dp_mst_resume(dev_priv);
 	intel_display_driver_resume(dev_priv);
 
-	if (HAS_DISPLAY(dev_priv))
+	if (HAS_DISPLAY(dev_priv)) {
+		intel_display_driver_enable_user_access(dev_priv);
 		drm_kms_helper_poll_enable(dev);
+	}
 	intel_hpd_poll_disable(dev_priv);
 
 	intel_opregion_resume(dev_priv);