| Message ID | 20200514154006.4761-3-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/params: don't expose inject_probe_failure in debugfs | expand |
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> On 14.5.2020 18.40, Jani Nikula wrote: > Only support runtime changes through the debugfs. > > i915.verbose_state_checks remains an exception, and is not exposed via > debugfs. > > This depends on IGT having been updated to use the debugfs for modifying > the parameters. > > Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com> > Cc: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_params.c | 38 +++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index a3dde770226d..ace44ad7e6df 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -40,6 +40,15 @@ struct i915_params i915_modparams __read_mostly = { > #undef MEMBER > }; > > +/* > + * Note: As a rule, keep module parameter sysfs permissions read-only > + * 0400. Runtime changes are only supported through i915 debugfs. > + * > + * For any exceptions requiring write access and runtime changes through module > + * parameter sysfs, prevent debugfs file creation by setting the parameter's > + * debugfs mode to 0. > + */ > + > i915_param_named(modeset, int, 0400, > "Use kernel modesetting [KMS] (0=disable, " > "1=on, -1=force vga console preference [default])"); > @@ -49,7 +58,7 @@ i915_param_named_unsafe(enable_dc, int, 0400, > "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6; " > "3=up to DC5 with DC3CO; 4=up to DC6 with DC3CO)"); > > -i915_param_named_unsafe(enable_fbc, int, 0600, > +i915_param_named_unsafe(enable_fbc, int, 0400, > "Enable frame buffer compression for power savings " > "(default: -1 (use per-chip default))"); > > @@ -57,7 +66,7 @@ i915_param_named_unsafe(lvds_channel_mode, int, 0400, > "Specify LVDS channel mode " > "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)"); > > -i915_param_named_unsafe(panel_use_ssc, int, 0600, > +i915_param_named_unsafe(panel_use_ssc, int, 0400, > "Use Spread Spectrum Clock with panels [LVDS/eDP] " > "(default: auto from VBT)"); > > @@ -65,25 +74,25 @@ i915_param_named_unsafe(vbt_sdvo_panel_type, int, 0400, > "Override/Ignore selection of SDVO panel mode in the VBT " > "(-2=ignore, -1=auto [default], index in VBT BIOS table)"); > > -i915_param_named_unsafe(reset, int, 0600, > +i915_param_named_unsafe(reset, int, 0400, > "Attempt GPU resets (0=disabled, 1=full gpu reset, 2=engine reset [default])"); > > i915_param_named_unsafe(vbt_firmware, charp, 0400, > "Load VBT from specified file under /lib/firmware"); > > #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) > -i915_param_named(error_capture, bool, 0600, > +i915_param_named(error_capture, bool, 0400, > "Record the GPU state following a hang. " > "This information in /sys/class/drm/card<N>/error is vital for " > "triaging and debugging hangs."); > #endif > > -i915_param_named_unsafe(enable_hangcheck, bool, 0600, > +i915_param_named_unsafe(enable_hangcheck, bool, 0400, > "Periodically check GPU activity for detecting hangs. " > "WARNING: Disabling this can cause system wide hangs. " > "(default: true)"); > > -i915_param_named_unsafe(enable_psr, int, 0600, > +i915_param_named_unsafe(enable_psr, int, 0400, > "Enable PSR " > "(0=disabled, 1=enabled) " > "Default: -1 (use per-chip default)"); > @@ -96,22 +105,22 @@ i915_param_named_unsafe(disable_power_well, int, 0400, > "Disable display power wells when possible " > "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)"); > > -i915_param_named_unsafe(enable_ips, int, 0600, "Enable IPS (default: true)"); > +i915_param_named_unsafe(enable_ips, int, 0400, "Enable IPS (default: true)"); > > -i915_param_named(fastboot, int, 0600, > +i915_param_named(fastboot, int, 0400, > "Try to skip unnecessary mode sets at boot time " > "(0=disabled, 1=enabled) " > "Default: -1 (use per-chip default)"); > > -i915_param_named_unsafe(load_detect_test, bool, 0600, > +i915_param_named_unsafe(load_detect_test, bool, 0400, > "Force-enable the VGA load detect code for testing (default:false). " > "For developers only."); > > -i915_param_named_unsafe(force_reset_modeset_test, bool, 0600, > +i915_param_named_unsafe(force_reset_modeset_test, bool, 0400, > "Force a modeset during gpu reset for testing (default:false). " > "For developers only."); > > -i915_param_named_unsafe(invert_brightness, int, 0600, > +i915_param_named_unsafe(invert_brightness, int, 0400, > "Invert backlight brightness " > "(-1 force normal, 0 machine defaults, 1 force inversion), please " > "report PCI device ID, subsystem vendor and subsystem device ID " > @@ -121,10 +130,11 @@ i915_param_named_unsafe(invert_brightness, int, 0600, > i915_param_named(disable_display, bool, 0400, > "Disable display (default: false)"); > > -i915_param_named(mmio_debug, int, 0600, > +i915_param_named(mmio_debug, int, 0400, > "Enable the MMIO debug code for the first N failures (default: off). " > "This may negatively affect performance."); > > +/* Special case writable file */ > i915_param_named(verbose_state_checks, bool, 0600, > "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions."); > > @@ -155,7 +165,7 @@ i915_param_named_unsafe(huc_firmware_path, charp, 0400, > i915_param_named_unsafe(dmc_firmware_path, charp, 0400, > "DMC firmware path to use instead of the default one"); > > -i915_param_named_unsafe(enable_dp_mst, bool, 0600, > +i915_param_named_unsafe(enable_dp_mst, bool, 0400, > "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); > > #if IS_ENABLED(CONFIG_DRM_I915_DEBUG) > @@ -163,7 +173,7 @@ i915_param_named_unsafe(inject_probe_failure, uint, 0400, > "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); > #endif > > -i915_param_named(enable_dpcd_backlight, int, 0600, > +i915_param_named(enable_dpcd_backlight, int, 0400, > "Enable support for DPCD backlight control" > "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enabled)"); > >
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index a3dde770226d..ace44ad7e6df 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -40,6 +40,15 @@ struct i915_params i915_modparams __read_mostly = { #undef MEMBER }; +/* + * Note: As a rule, keep module parameter sysfs permissions read-only + * 0400. Runtime changes are only supported through i915 debugfs. + * + * For any exceptions requiring write access and runtime changes through module + * parameter sysfs, prevent debugfs file creation by setting the parameter's + * debugfs mode to 0. + */ + i915_param_named(modeset, int, 0400, "Use kernel modesetting [KMS] (0=disable, " "1=on, -1=force vga console preference [default])"); @@ -49,7 +58,7 @@ i915_param_named_unsafe(enable_dc, int, 0400, "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6; " "3=up to DC5 with DC3CO; 4=up to DC6 with DC3CO)"); -i915_param_named_unsafe(enable_fbc, int, 0600, +i915_param_named_unsafe(enable_fbc, int, 0400, "Enable frame buffer compression for power savings " "(default: -1 (use per-chip default))"); @@ -57,7 +66,7 @@ i915_param_named_unsafe(lvds_channel_mode, int, 0400, "Specify LVDS channel mode " "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)"); -i915_param_named_unsafe(panel_use_ssc, int, 0600, +i915_param_named_unsafe(panel_use_ssc, int, 0400, "Use Spread Spectrum Clock with panels [LVDS/eDP] " "(default: auto from VBT)"); @@ -65,25 +74,25 @@ i915_param_named_unsafe(vbt_sdvo_panel_type, int, 0400, "Override/Ignore selection of SDVO panel mode in the VBT " "(-2=ignore, -1=auto [default], index in VBT BIOS table)"); -i915_param_named_unsafe(reset, int, 0600, +i915_param_named_unsafe(reset, int, 0400, "Attempt GPU resets (0=disabled, 1=full gpu reset, 2=engine reset [default])"); i915_param_named_unsafe(vbt_firmware, charp, 0400, "Load VBT from specified file under /lib/firmware"); #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) -i915_param_named(error_capture, bool, 0600, +i915_param_named(error_capture, bool, 0400, "Record the GPU state following a hang. " "This information in /sys/class/drm/card<N>/error is vital for " "triaging and debugging hangs."); #endif -i915_param_named_unsafe(enable_hangcheck, bool, 0600, +i915_param_named_unsafe(enable_hangcheck, bool, 0400, "Periodically check GPU activity for detecting hangs. " "WARNING: Disabling this can cause system wide hangs. " "(default: true)"); -i915_param_named_unsafe(enable_psr, int, 0600, +i915_param_named_unsafe(enable_psr, int, 0400, "Enable PSR " "(0=disabled, 1=enabled) " "Default: -1 (use per-chip default)"); @@ -96,22 +105,22 @@ i915_param_named_unsafe(disable_power_well, int, 0400, "Disable display power wells when possible " "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)"); -i915_param_named_unsafe(enable_ips, int, 0600, "Enable IPS (default: true)"); +i915_param_named_unsafe(enable_ips, int, 0400, "Enable IPS (default: true)"); -i915_param_named(fastboot, int, 0600, +i915_param_named(fastboot, int, 0400, "Try to skip unnecessary mode sets at boot time " "(0=disabled, 1=enabled) " "Default: -1 (use per-chip default)"); -i915_param_named_unsafe(load_detect_test, bool, 0600, +i915_param_named_unsafe(load_detect_test, bool, 0400, "Force-enable the VGA load detect code for testing (default:false). " "For developers only."); -i915_param_named_unsafe(force_reset_modeset_test, bool, 0600, +i915_param_named_unsafe(force_reset_modeset_test, bool, 0400, "Force a modeset during gpu reset for testing (default:false). " "For developers only."); -i915_param_named_unsafe(invert_brightness, int, 0600, +i915_param_named_unsafe(invert_brightness, int, 0400, "Invert backlight brightness " "(-1 force normal, 0 machine defaults, 1 force inversion), please " "report PCI device ID, subsystem vendor and subsystem device ID " @@ -121,10 +130,11 @@ i915_param_named_unsafe(invert_brightness, int, 0600, i915_param_named(disable_display, bool, 0400, "Disable display (default: false)"); -i915_param_named(mmio_debug, int, 0600, +i915_param_named(mmio_debug, int, 0400, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance."); +/* Special case writable file */ i915_param_named(verbose_state_checks, bool, 0600, "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions."); @@ -155,7 +165,7 @@ i915_param_named_unsafe(huc_firmware_path, charp, 0400, i915_param_named_unsafe(dmc_firmware_path, charp, 0400, "DMC firmware path to use instead of the default one"); -i915_param_named_unsafe(enable_dp_mst, bool, 0600, +i915_param_named_unsafe(enable_dp_mst, bool, 0400, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); #if IS_ENABLED(CONFIG_DRM_I915_DEBUG) @@ -163,7 +173,7 @@ i915_param_named_unsafe(inject_probe_failure, uint, 0400, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); #endif -i915_param_named(enable_dpcd_backlight, int, 0600, +i915_param_named(enable_dpcd_backlight, int, 0400, "Enable support for DPCD backlight control" "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enabled)");
Only support runtime changes through the debugfs. i915.verbose_state_checks remains an exception, and is not exposed via debugfs. This depends on IGT having been updated to use the debugfs for modifying the parameters. Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com> Cc: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_params.c | 38 +++++++++++++++++++----------- 1 file changed, 24 insertions(+), 14 deletions(-)