Skip to content

Commit bd738d8

Browse files
committed
drm/i915: Prevent modesets during driver init/shutdown
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. During driver loading/unloading/system suspend/shutdown and during system resume after calling intel_display_driver_disable_user_access() or intel_display_driver_resume_access() correspondigly, the current thread is allowed to modeset (as this thread requires to do an initial/restoring modeset or a disabling modeset), other threads (the user threads) are not allowed to modeset. During driver loading/system resume after calling intel_display_driver_enable_user_access() all threads are allowed to modeset. During driver unloading/system suspend/shutdown after calling intel_display_driver_suspend_access() no threads are allowed to modeset (as the HW got disabled and should stay in this state). v2: Call intel_display_driver_suspend_access()/resume_access() only for HAS_DISPLAY(). (CI) v3: (Jouni) - Add commit log comments explaining how the permission of modeset changes during HW init/deinit wrt. to the current and other user processes. Link: https://patchwork.freedesktop.org/patch/msgid/20240104132335.2766434-1-imre.deak@intel.com Reviewed-by: Jouni Högander <jouni.hogander@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com>
1 parent 1ef28d8 commit bd738d8

File tree

5 files changed

+171
-2
lines changed

5 files changed

+171
-2
lines changed

drivers/gpu/drm/i915/display/intel_display.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6310,6 +6310,9 @@ int intel_atomic_check(struct drm_device *dev,
63106310
int ret, i;
63116311
bool any_ms = false;
63126312

6313+
if (!intel_display_driver_check_access(dev_priv))
6314+
return -ENODEV;
6315+
63136316
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
63146317
new_crtc_state, i) {
63156318
/*

drivers/gpu/drm/i915/display/intel_display_core.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include "intel_opregion.h"
2929
#include "intel_wm_types.h"
3030

31+
struct task_struct;
32+
3133
struct drm_i915_private;
3234
struct drm_property;
3335
struct drm_property_blob;
@@ -295,6 +297,11 @@ struct intel_display {
295297
const struct intel_audio_funcs *audio;
296298
} funcs;
297299

300+
struct {
301+
bool any_task_allowed;
302+
struct task_struct *allowed_task;
303+
} access;
304+
298305
struct {
299306
/* backlight registers and fields in struct intel_panel */
300307
struct mutex lock;

drivers/gpu/drm/i915/display/intel_display_driver.c

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include "intel_hdcp.h"
4646
#include "intel_hotplug.h"
4747
#include "intel_hti.h"
48+
#include "intel_modeset_lock.h"
4849
#include "intel_modeset_setup.h"
4950
#include "intel_opregion.h"
5051
#include "intel_overlay.h"
@@ -276,6 +277,135 @@ int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
276277
return ret;
277278
}
278279

280+
static void set_display_access(struct drm_i915_private *i915,
281+
bool any_task_allowed,
282+
struct task_struct *allowed_task)
283+
{
284+
struct drm_modeset_acquire_ctx ctx;
285+
int err;
286+
287+
intel_modeset_lock_ctx_retry(&ctx, NULL, 0, err) {
288+
err = drm_modeset_lock_all_ctx(&i915->drm, &ctx);
289+
if (err)
290+
continue;
291+
292+
i915->display.access.any_task_allowed = any_task_allowed;
293+
i915->display.access.allowed_task = allowed_task;
294+
}
295+
296+
drm_WARN_ON(&i915->drm, err);
297+
}
298+
299+
/**
300+
* intel_display_driver_enable_user_access - Enable display HW access for all threads
301+
* @i915: i915 device instance
302+
*
303+
* Enable the display HW access for all threads. Examples for such accesses
304+
* are modeset commits and connector probing.
305+
*
306+
* This function should be called during driver loading and system resume once
307+
* all the HW initialization steps are done.
308+
*/
309+
void intel_display_driver_enable_user_access(struct drm_i915_private *i915)
310+
{
311+
set_display_access(i915, true, NULL);
312+
}
313+
314+
/**
315+
* intel_display_driver_disable_user_access - Disable display HW access for user threads
316+
* @i915: i915 device instance
317+
*
318+
* Disable the display HW access for user threads. Examples for such accesses
319+
* are modeset commits and connector probing. For the current thread the
320+
* access is still enabled, which should only perform HW init/deinit
321+
* programming (as the initial modeset during driver loading or the disabling
322+
* modeset during driver unloading and system suspend/shutdown). This function
323+
* should be followed by calling either intel_display_driver_enable_user_access()
324+
* after completing the HW init programming or
325+
* intel_display_driver_suspend_access() after completing the HW deinit
326+
* programming.
327+
*
328+
* This function should be called during driver loading/unloading and system
329+
* suspend/shutdown before starting the HW init/deinit programming.
330+
*/
331+
void intel_display_driver_disable_user_access(struct drm_i915_private *i915)
332+
{
333+
set_display_access(i915, false, current);
334+
}
335+
336+
/**
337+
* intel_display_driver_suspend_access - Suspend display HW access for all threads
338+
* @i915: i915 device instance
339+
*
340+
* Disable the display HW access for all threads. Examples for such accesses
341+
* are modeset commits and connector probing. This call should be either
342+
* followed by calling intel_display_driver_resume_access(), or the driver
343+
* should be unloaded/shutdown.
344+
*
345+
* This function should be called during driver unloading and system
346+
* suspend/shutdown after completing the HW deinit programming.
347+
*/
348+
void intel_display_driver_suspend_access(struct drm_i915_private *i915)
349+
{
350+
set_display_access(i915, false, NULL);
351+
}
352+
353+
/**
354+
* intel_display_driver_resume_access - Resume display HW access for the resume thread
355+
* @i915: i915 device instance
356+
*
357+
* Enable the display HW access for the current resume thread, keeping the
358+
* access disabled for all other (user) threads. Examples for such accesses
359+
* are modeset commits and connector probing. The resume thread should only
360+
* perform HW init programming (as the restoring modeset). This function
361+
* should be followed by calling intel_display_driver_enable_user_access(),
362+
* after completing the HW init programming steps.
363+
*
364+
* This function should be called during system resume before starting the HW
365+
* init steps.
366+
*/
367+
void intel_display_driver_resume_access(struct drm_i915_private *i915)
368+
{
369+
set_display_access(i915, false, current);
370+
}
371+
372+
/**
373+
* intel_display_driver_check_access - Check if the current thread has disaplay HW access
374+
* @i915: i915 device instance
375+
*
376+
* Check whether the current thread has display HW access, print a debug
377+
* message if it doesn't. Such accesses are modeset commits and connector
378+
* probing. If the function returns %false any HW access should be prevented.
379+
*
380+
* Returns %true if the current thread has display HW access, %false
381+
* otherwise.
382+
*/
383+
bool intel_display_driver_check_access(struct drm_i915_private *i915)
384+
{
385+
char comm[TASK_COMM_LEN];
386+
char current_task[TASK_COMM_LEN + 16];
387+
char allowed_task[TASK_COMM_LEN + 16] = "none";
388+
389+
if (i915->display.access.any_task_allowed ||
390+
i915->display.access.allowed_task == current)
391+
return true;
392+
393+
snprintf(current_task, sizeof(current_task), "%s[%d]",
394+
get_task_comm(comm, current),
395+
task_pid_vnr(current));
396+
397+
if (i915->display.access.allowed_task)
398+
snprintf(allowed_task, sizeof(allowed_task), "%s[%d]",
399+
get_task_comm(comm, i915->display.access.allowed_task),
400+
task_pid_vnr(i915->display.access.allowed_task));
401+
402+
drm_dbg_kms(&i915->drm,
403+
"Reject display access from task %s (allowed to %s)\n",
404+
current_task, allowed_task);
405+
406+
return false;
407+
}
408+
279409
/* part #2: call after irq install, but before gem init */
280410
int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
281411
{
@@ -326,6 +456,8 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
326456
intel_vga_disable(i915);
327457
intel_setup_outputs(i915);
328458

459+
intel_display_driver_disable_user_access(i915);
460+
329461
drm_modeset_lock_all(dev);
330462
intel_modeset_setup_hw_state(i915, dev->mode_config.acquire_ctx);
331463
intel_acpi_assign_connector_fwnodes(i915);
@@ -393,6 +525,8 @@ void intel_display_driver_register(struct drm_i915_private *i915)
393525

394526
intel_audio_init(i915);
395527

528+
intel_display_driver_enable_user_access(i915);
529+
396530
intel_display_debugfs_register(i915);
397531

398532
/*
@@ -440,6 +574,8 @@ void intel_display_driver_remove_noirq(struct drm_i915_private *i915)
440574
if (!HAS_DISPLAY(i915))
441575
return;
442576

577+
intel_display_driver_suspend_access(i915);
578+
443579
/*
444580
* Due to the hpd irq storm handling the hotplug work can re-arm the
445581
* poll handlers. Hence disable polling after hpd handling is shut down.
@@ -493,6 +629,8 @@ void intel_display_driver_unregister(struct drm_i915_private *i915)
493629
*/
494630
drm_kms_helper_poll_fini(&i915->drm);
495631

632+
intel_display_driver_disable_user_access(i915);
633+
496634
intel_audio_deinit(i915);
497635

498636
drm_atomic_helper_shutdown(&i915->drm);

drivers/gpu/drm/i915/display/intel_display_driver.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,11 @@ int __intel_display_driver_resume(struct drm_i915_private *i915,
3232
struct drm_atomic_state *state,
3333
struct drm_modeset_acquire_ctx *ctx);
3434

35+
void intel_display_driver_enable_user_access(struct drm_i915_private *i915);
36+
void intel_display_driver_disable_user_access(struct drm_i915_private *i915);
37+
void intel_display_driver_suspend_access(struct drm_i915_private *i915);
38+
void intel_display_driver_resume_access(struct drm_i915_private *i915);
39+
bool intel_display_driver_check_access(struct drm_i915_private *i915);
40+
3541
#endif /* __INTEL_DISPLAY_DRIVER_H__ */
3642

drivers/gpu/drm/i915/i915_driver.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,6 +1005,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
10051005
intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true);
10061006
if (HAS_DISPLAY(i915)) {
10071007
drm_kms_helper_poll_disable(&i915->drm);
1008+
intel_display_driver_disable_user_access(i915);
10081009

10091010
drm_atomic_helper_shutdown(&i915->drm);
10101011
}
@@ -1014,6 +1015,9 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
10141015
intel_runtime_pm_disable_interrupts(i915);
10151016
intel_hpd_cancel_work(i915);
10161017

1018+
if (HAS_DISPLAY(i915))
1019+
intel_display_driver_suspend_access(i915);
1020+
10171021
intel_suspend_encoders(i915);
10181022
intel_shutdown_encoders(i915);
10191023

@@ -1081,8 +1085,10 @@ static int i915_drm_suspend(struct drm_device *dev)
10811085
* properly. */
10821086
intel_power_domains_disable(dev_priv);
10831087
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
1084-
if (HAS_DISPLAY(dev_priv))
1088+
if (HAS_DISPLAY(dev_priv)) {
10851089
drm_kms_helper_poll_disable(dev);
1090+
intel_display_driver_disable_user_access(dev_priv);
1091+
}
10861092

10871093
pci_save_state(pdev);
10881094

@@ -1093,6 +1099,9 @@ static int i915_drm_suspend(struct drm_device *dev)
10931099
intel_runtime_pm_disable_interrupts(dev_priv);
10941100
intel_hpd_cancel_work(dev_priv);
10951101

1102+
if (HAS_DISPLAY(dev_priv))
1103+
intel_display_driver_suspend_access(dev_priv);
1104+
10961105
intel_suspend_encoders(dev_priv);
10971106

10981107
/* Must be called before GGTT is suspended. */
@@ -1242,14 +1251,20 @@ static int i915_drm_resume(struct drm_device *dev)
12421251
intel_display_driver_init_hw(dev_priv);
12431252

12441253
intel_clock_gating_init(dev_priv);
1254+
1255+
if (HAS_DISPLAY(dev_priv))
1256+
intel_display_driver_resume_access(dev_priv);
1257+
12451258
intel_hpd_init(dev_priv);
12461259

12471260
/* MST sideband requires HPD interrupts enabled */
12481261
intel_dp_mst_resume(dev_priv);
12491262
intel_display_driver_resume(dev_priv);
12501263

1251-
if (HAS_DISPLAY(dev_priv))
1264+
if (HAS_DISPLAY(dev_priv)) {
1265+
intel_display_driver_enable_user_access(dev_priv);
12521266
drm_kms_helper_poll_enable(dev);
1267+
}
12531268
intel_hpd_poll_disable(dev_priv);
12541269

12551270
intel_opregion_resume(dev_priv);

0 commit comments

Comments
 (0)