| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220215191941.azk5gpcn42ahcnna@pali>
Date: Tue, 15 Feb 2022 20:19:41 +0100
From: Pali Rohár <pali@...nel.org>
To: Armin Wolf <W_Armin@....de>
Cc: jdelvare@...e.com, linux@...ck-us.net, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] hwmon: (dell-smm) Allow for specifying fan control
method as module parameter
On Tuesday 15 February 2022 20:11:07 Armin Wolf wrote:
> Right now, the only way to test if setting manual/auto fan control works
> is to edit and recompile the module, which may be too cumbersome for
> the average user.
There is also another way suitable for testing purposes which do not
requires any kernel patch. Call iopl(3) syscall which changes I/O
privilege level to 3 and which allows to poke I/O registers.
> Allow for specifying the desired fan mode control method when loading
> the module, but taint the kernel if so since there is the possibility
> for strange side effects on non-whitelisted models.
> Also update docs and kernel-parameters.txt accordingly.
>
> Tested on a Dell Inspiron 3505.
>
> Signed-off-by: Armin Wolf <W_Armin@....de>
> ---
> .../admin-guide/kernel-parameters.txt | 3 +++
> Documentation/hwmon/dell-smm-hwmon.rst | 21 ++++++++++------
> drivers/hwmon/dell-smm-hwmon.c | 25 +++++++++++++------
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index d68053db21cc..4f1b6c2b7ed1 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -968,6 +968,9 @@
> dell_smm_hwmon.fan_max=
> [HW] Maximum configurable fan speed.
>
> + dell_smm_hwmon.fan_mode_method=
> + [HW] Method to use for changing fan mode.
> +
> dfltcc= [HW,S390]
> Format: { on | off | def_only | inf_only | always }
> on: s390 zlib hardware support for compression on
> diff --git a/Documentation/hwmon/dell-smm-hwmon.rst b/Documentation/hwmon/dell-smm-hwmon.rst
> index beec88491171..564d99cda869 100644
> --- a/Documentation/hwmon/dell-smm-hwmon.rst
> +++ b/Documentation/hwmon/dell-smm-hwmon.rst
> @@ -67,13 +67,16 @@ for your hardware. It is possible that codes that work for other
> laptops actually work for yours as well, or that you have to discover
> new codes.
>
> -Check the list ``i8k_whitelist_fan_control`` in file
> -``drivers/hwmon/dell-smm-hwmon.c`` in the kernel tree: as a first
> -attempt you can try to add your machine and use an already-known code
> -pair. If, after recompiling the kernel, you see that ``pwm1_enable``
> -is present and works (i.e., you can manually control the fan speed),
> -then please submit your finding as a kernel patch, so that other users
> -can benefit from it. Please see
> +As a first step, you can load the module with the module parameter
> +``fan_mode_method`` set to 1 to test if your hardware works with
> +an already know method for disabling automatic BIOS fan control.
> +If ``pwm1_enable`` is now present and works (i.e., you can
> +manually control the fan speed), then please submit your finding
> +as a kernel patch, so that other users can benefit from it.
> +Just add your model to the list ``i8k_whitelist_fan_control`` in
> +file ``drivers/hwmon/dell-smm-hwmon.c`` in the kernel tree and use
> +the already known code pair.
> +Please read
> :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
> for information on submitting patches.
>
> @@ -120,6 +123,10 @@ Module parameters
> Maximum configurable fan speed. (default:
> autodetect)
>
> +* fan_mode_method:uint
> + Method to use for changing fan mode (default:
> + from whitelist)
> +
> Legacy ``/proc`` interface
> --------------------------
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 9949eeb79378..1c4cc516c8b2 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -111,6 +111,10 @@ static uint fan_max;
> module_param(fan_max, uint, 0);
> MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");
>
> +static uint fan_mode_method;
> +module_param_unsafe(fan_mode_method, uint, 0);
> +MODULE_PARM_DESC(fan_mode_method, "Method to use for changing fan mode (default: from whitelist)");
No, please really do not introduce another kernel parameter for this
driver. There are already many and we do not need to extend this list.
> +
> struct smm_regs {
> unsigned int eax;
> unsigned int ebx;
> @@ -677,7 +681,7 @@ static umode_t dell_smm_is_visible(const void *drvdata, enum hwmon_sensor_types
>
> break;
> case hwmon_pwm_enable:
> - if (data->auto_fan)
> + if (data->auto_fan && data->manual_fan)
> /*
> * There is no command for retrieve the current status
> * from BIOS, and userspace/firmware itself can change
> @@ -1282,14 +1286,21 @@ static int __init dell_smm_probe(struct platform_device *pdev)
> data->i8k_fan_max = fan_max ? : I8K_FAN_HIGH; /* Must not be 0 */
> data->i8k_pwm_mult = DIV_ROUND_UP(255, data->i8k_fan_max);
>
> - fan_control = dmi_first_match(i8k_whitelist_fan_control);
> - if (fan_control && fan_control->driver_data) {
> - const struct i8k_fan_control_data *control = fan_control->driver_data;
> + /* value specified via module param overrides whitelist */
> + if (fan_mode_method > 0 && fan_mode_method <= ARRAY_SIZE(i8k_fan_control_data)) {
> + data->manual_fan = i8k_fan_control_data[fan_mode_method - 1].manual_fan;
> + data->auto_fan = i8k_fan_control_data[fan_mode_method - 1].auto_fan;
> + } else {
> + fan_control = dmi_first_match(i8k_whitelist_fan_control);
> + if (fan_control && fan_control->driver_data) {
> + const struct i8k_fan_control_data *control = fan_control->driver_data;
>
> - data->manual_fan = control->manual_fan;
> - data->auto_fan = control->auto_fan;
> - dev_info(&pdev->dev, "enabling support for setting automatic/manual fan control\n");
> + data->manual_fan = control->manual_fan;
> + data->auto_fan = control->auto_fan;
> + }
> }
> + if (data->manual_fan && data->auto_fan)
> + dev_info(&pdev->dev, "enabling support for setting automatic/manual fan control\n");
>
> if (!fan_mult) {
> /*
> --
> 2.30.2
>
Powered by blists - more mailing lists