Re: [PATCH] dell-smm-hwmon.c: Additional temperature sensors
Do you have definite response from Dell support that they are not going to fix it? Then it is pity :-( On Thursday 07 February 2019 12:16:06 Michele Sorcinelli wrote: > As far as I know Dell won't help with fixing the SMM layer, they probably > changed something starting with firmware version 1.3.0 and they don't > wanna release information about it. > > https://www.hwinfo.com/forum/Thread-Dell-XPS-15-9570-temperatures-not-named-anymore > > I wonder if something can be done to force the discovery of the sensors in > the driver, > maybe adding a module option to use i8k_get_temp() as probe method as a > workaround, > or maybe just forcing that method for this specific model? > > Let me know your thoughts. > > Thanks, > Michele. > > On 12/10/18 10:58 AM, Pali Rohár wrote: > > On Friday 07 December 2018 20:24:49 Guenter Roeck wrote: > > > Anyway, I would like to get some feedback if this can cause regressions > > > on systems which don't support that many sensors and maybe report > > > something > > > completely different if one tries to read the high-numbered sensors. > > > I seem to recall that there was a reason for checking the type and not > > > just > > > trying to read sensor values. > > > > There can be also different problem for sensors which are turned off. > > E.g. on notebooks with switchable graphic cards which have included > > temperature sensors. When graphic card is turned off, then SMM returns > > error when asking for temperature value (for obvious reason). But > > temperature type still returns correct value "this is GPU sensors". > > > > So we cannot replace temp_type check by temp_value check. It introduce > > race condition between "starting GPU" and initializing dell-smm hwmon. > > Now linux kernel has support for dynamic turning ON/OFF switchable GPU, > > so we need to care about these race conditions too. > > -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 23/30] hwmon: (dell-smm-hwmon) Use permission specific SENSOR[_DEVICE]_ATTR variants
On Monday 10 December 2018 14:08:41 Guenter Roeck wrote:
> Use SENSOR[_DEVICE]_ATTR[_2]_{RO,RW,WO} to simplify the source code,
> to improve readbility, and to reduce the chance of inconsistencies.
>
> Also replace any remaining S_ in the driver with octal values.
>
> The conversion was done automatically with coccinelle. The semantic patches
> and the scripts used to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches/hwmon/.
>
> This patch does not introduce functional changes. It was verified by
> compiling the old and new files and comparing text and data sizes.
>
> Cc: "Pali Rohár"
> Signed-off-by: Guenter Roeck
Looks good, add my
Acked-By: Pali Rohár
> ---
> drivers/hwmon/dell-smm-hwmon.c | 68
> +-
> 1 file changed, 27 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 367a8a61712c..68c9a6664557 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -618,7 +618,7 @@ static inline void __exit i8k_exit_procfs(void)
> * Hwmon interface
> */
>
> -static ssize_t i8k_hwmon_show_temp_label(struct device *dev,
> +static ssize_t i8k_hwmon_temp_label_show(struct device *dev,
>struct device_attribute *devattr,
>char *buf)
> {
> @@ -641,7 +641,7 @@ static ssize_t i8k_hwmon_show_temp_label(struct device
> *dev,
> return sprintf(buf, "%s\n", labels[type]);
> }
>
> -static ssize_t i8k_hwmon_show_temp(struct device *dev,
> +static ssize_t i8k_hwmon_temp_show(struct device *dev,
> struct device_attribute *devattr,
> char *buf)
> {
> @@ -654,7 +654,7 @@ static ssize_t i8k_hwmon_show_temp(struct device *dev,
> return sprintf(buf, "%d\n", temp * 1000);
> }
>
> -static ssize_t i8k_hwmon_show_fan_label(struct device *dev,
> +static ssize_t i8k_hwmon_fan_label_show(struct device *dev,
> struct device_attribute *devattr,
> char *buf)
> {
> @@ -685,9 +685,8 @@ static ssize_t i8k_hwmon_show_fan_label(struct device
> *dev,
> return sprintf(buf, "%s%s\n", (dock ? "Docking " : ""), labels[type]);
> }
>
> -static ssize_t i8k_hwmon_show_fan(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static ssize_t i8k_hwmon_fan_show(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> {
> int index = to_sensor_dev_attr(devattr)->index;
> int fan_speed;
> @@ -698,9 +697,8 @@ static ssize_t i8k_hwmon_show_fan(struct device *dev,
> return sprintf(buf, "%d\n", fan_speed);
> }
>
> -static ssize_t i8k_hwmon_show_pwm(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static ssize_t i8k_hwmon_pwm_show(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> {
> int index = to_sensor_dev_attr(devattr)->index;
> int status;
> @@ -711,9 +709,9 @@ static ssize_t i8k_hwmon_show_pwm(struct device *dev,
> return sprintf(buf, "%d\n", clamp_val(status * i8k_pwm_mult, 0, 255));
> }
>
> -static ssize_t i8k_hwmon_set_pwm(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> +static ssize_t i8k_hwmon_pwm_store(struct device *dev,
> +struct device_attribute *attr,
> +const char *buf, size_t count)
> {
> int index = to_sensor_dev_attr(attr)->index;
> unsigned long val;
> @@ -731,35 +729,23 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
> return err < 0 ? -EIO : count;
> }
>
> -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> 0);
> -static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label,
> NULL,
> - 0);
> -static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> 1);
> -static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, i8k_hwmon_show_temp_label,
> NULL,
> - 1);
> -static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> 2);
> -static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, i8k_hwmon_show_temp_l
Re: [PATCH v3] dell-smm-hwmon.c: Additional temperature sensors
On Friday 21 December 2018 15:40:32 Guenter Roeck wrote:
> On Fri, Dec 14, 2018 at 11:57:59PM +, Michele Sorcinelli wrote:
> > The code has been extended to support up to 10 temperature sensors.
> >
> > Changes from previous patch versions:
> >
> > - fix wrong index ranges in i8k_is_visible() if conditions
> > - restore i8k_get_temp_type() as probe method for temp sensors because
> > i8k_get_temp() is not reliable (for example it won't work when
> > the sensors is turned off)
> >
> > Signed-off-by: Michele Sorcinelli
>
> Pali, this is waiting for your feedbacck. Any comments ?
>
> Thanks,
> Guenter
Hi! Sorry for delay.
> > ---
> > drivers/hwmon/dell-smm-hwmon.c | 105 -
> > 1 file changed, 90 insertions(+), 15 deletions(-)
> > mode change 100644 => 100755 drivers/hwmon/dell-smm-hwmon.c
This change is wrong, source code files should not be executable.
Otherwise looks good. After removing executable bit, you can add my
Reviewed-by: Pali Rohár
> >
> > --
> > 2.20.0
> >
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> > old mode 100644
> > new mode 100755
> > index 367a8a617..73a30e3e4
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -82,9 +82,16 @@ static bool disallow_fan_support;
> > #define I8K_HWMON_HAVE_TEMP2 (1 << 1)
> > #define I8K_HWMON_HAVE_TEMP3 (1 << 2)
> > #define I8K_HWMON_HAVE_TEMP4 (1 << 3)
> > -#define I8K_HWMON_HAVE_FAN1(1 << 4)
> > -#define I8K_HWMON_HAVE_FAN2(1 << 5)
> > -#define I8K_HWMON_HAVE_FAN3(1 << 6)
> > +#define I8K_HWMON_HAVE_TEMP5 (1 << 4)
> > +#define I8K_HWMON_HAVE_TEMP6 (1 << 5)
> > +#define I8K_HWMON_HAVE_TEMP7 (1 << 6)
> > +#define I8K_HWMON_HAVE_TEMP8 (1 << 7)
> > +#define I8K_HWMON_HAVE_TEMP9 (1 << 8)
> > +#define I8K_HWMON_HAVE_TEMP10 (1 << 9)
> > +
> > +#define I8K_HWMON_HAVE_FAN1(1 << 10)
> > +#define I8K_HWMON_HAVE_FAN2(1 << 11)
> > +#define I8K_HWMON_HAVE_FAN3(1 << 12)
> >
> > MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
> > MODULE_AUTHOR("Pali Rohár ");
> > @@ -743,6 +750,25 @@ static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO,
> > i8k_hwmon_show_temp_label, NULL,
> > static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 3);
> > static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > 3);
> > +static SENSOR_DEVICE_ATTR(temp5_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 4);
> > +static SENSOR_DEVICE_ATTR(temp5_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > + 4);
> > +static SENSOR_DEVICE_ATTR(temp6_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 5);
> > +static SENSOR_DEVICE_ATTR(temp6_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > + 5);
> > +static SENSOR_DEVICE_ATTR(temp7_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 6);
> > +static SENSOR_DEVICE_ATTR(temp7_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > + 6);
> > +static SENSOR_DEVICE_ATTR(temp8_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 7);
> > +static SENSOR_DEVICE_ATTR(temp8_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > + 7);
> > +static SENSOR_DEVICE_ATTR(temp9_input, S_IRUGO, i8k_hwmon_show_temp, NULL,
> > 8);
> > +static SENSOR_DEVICE_ATTR(temp9_label, S_IRUGO, i8k_hwmon_show_temp_label,
> > NULL,
> > + 8);
> > +static SENSOR_DEVICE_ATTR(temp10_input, S_IRUGO, i8k_hwmon_show_temp,
> > NULL, 9);
> > +static SENSOR_DEVICE_ATTR(temp10_label, S_IRUGO,
> > i8k_hwmon_show_temp_label, NULL,
> > + 9);
> > +
> > static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> > 0);
> > static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, i8k_hwmon_show_fan_label,
> > NULL,
> > 0);
> > @@ -770,15 +796,27 @@ static struct attribute *i8k_attrs[] = {
> > _dev_attr_temp3_label.dev_attr.attr, /* 5 */
> > _dev_attr_temp4_input.dev_attr.attr, /* 6 */
> > _dev_attr_temp4_label.dev_attr.attr, /* 7 */
> > - _dev_attr_fan1_input.dev_attr.attr, /* 8 */
> > - _dev_attr_fan1_label.dev_attr.attr,
Re: [PATCH] dell-smm-hwmon.c: Additional temperature sensors
On Monday 10 December 2018 11:53:58 Michele Sorcinelli wrote: > I wonder what's the best way to inform the firmware developers. Mario from @Dell wrote how to report firware bugs in discussion: https://github.com/dell/libsmbios/issues/48#issuecomment-391328501 You should have bought your machine in USA and then open ticket at: http://www.dell.com/support/incidents-online/us/en/04 For my question how non-USA people should report bugs I have not received answer yet. But probably similar steps, contacting Dell support. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH] dell-smm-hwmon.c: Additional temperature sensors
On Friday 07 December 2018 20:24:49 Guenter Roeck wrote: > Anyway, I would like to get some feedback if this can cause regressions > on systems which don't support that many sensors and maybe report something > completely different if one tries to read the high-numbered sensors. > I seem to recall that there was a reason for checking the type and not just > trying to read sensor values. There can be also different problem for sensors which are turned off. E.g. on notebooks with switchable graphic cards which have included temperature sensors. When graphic card is turned off, then SMM returns error when asking for temperature value (for obvious reason). But temperature type still returns correct value "this is GPU sensors". So we cannot replace temp_type check by temp_value check. It introduce race condition between "starting GPU" and initializing dell-smm hwmon. Now linux kernel has support for dynamic turning ON/OFF switchable GPU, so we need to care about these race conditions too. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH] dell-smm-hwmon.c: Add XPS 9570 to supported devices list
Hi! On Thursday 29 November 2018 21:42:54 Michele Sorcinelli wrote: > I've been playing with smm.c from a old version of i8kutils, that > is just an userspace utility to trigger the SMM RPC: > > http://sprunge.us/llxG7R > > I used that in combination with the following test script: > > http://sprunge.us/izJmQK > > I managed to run that on a XPS 9560 and 9570 and compare the results. > > XPS 9560: http://sprunge.us/xCLNSy > XPS 9570: http://sprunge.us/wSFTKs > > The main difference between 9560 and 9570 is that the calls to get > the type of either fan or temperature sensor are not working (they all > return 0xfff in the eax register), where in the 9560 they're returning > proper values. So it looks like that SMM on you machine does not support call to get type of temperature sensor. And dell-smm-hwmon.c uses type of sensor to detect if sensor is present or not. Also from that output it looks like that you have 8 temperature sensors present, but dell-smm-hwmon.c currently supports only first 4. If you want to have support for temperature sensors on your machine, you can look at code in i8k_init_hwmon() how fans are initialized. Code there first check if fan status does not produce error OR fan type does not produce error. And you can use same logic for temperature sensors. And if you want to support all 8 temp sensors, you need to extend dell-smm-hwmon.c code to support more then 4 temp sensors. This means that you would not see temperature sensors names as your SMM does not provide them. But it is a problem at all? > However, it looks like the call for reading values (fan speed or > temperatures) are all returning proper values in 9570 as well. > > I'm assuming the following: > > - API should be the same between 9560 and 9670 (why should Dell change > it just now, and just for a single feature?); I think API was not changed. Just one RPC over SMM function is not supported. > - in the 9670 the type calls are not implemented correctly; > - the sensor indexes should be correct (because they work when used to > read the temperature directly) > > I also tried using higher indexes but I always get 0x. > > We could either wait for Dell to eventually release a firmware update > that implements those calls correctly or we could force the reading of > the temperatures even if the type is not returned properly (although > I didn't know how to map the sensor with the proper label). Have you reported bug to Dell? If not, then Dell probably do not know about it and so cannot fix anything. > I'm not keen to try other unknown procedures as I don't know what could > happen, but I've found that other documented procedures are working properly > (eg. set fan speed, read firmware version, disable fan control, > enable fan control, etc.), so again I think the API shouldn't have changed. > > Let me know your thoughts. > > Thanks, > Michele. > > On 11/29/18 9:48 AM, Pali Rohár wrote: > > On Wednesday 28 November 2018 23:23:07 Michele Sorcinelli wrote: > > > I tried to debug the temperature sensor probing with some printk() > > > around the code... it looks like the ssm calls are all returning -22 > > > as result: > > > > > > [ 90.565793] Calling i8k_get_temp_type(0) > > > [ 90.565795] Inside i8k_get_temp_type() with sensors = 0 > > > [ 90.565795] Performing i8k_smm call > > > [ 90.567708] Result of the i8k_smm call: -22 > > > [ 90.567708] Return value of i8k_get_temp_type: -22 > > > [ 90.567709] Result of i8k_get_temp_type(0): -22 > > > [ 90.567709] Calling i8k_get_temp_type(1) > > > [ 90.567710] Inside i8k_get_temp_type() with sensors = 1 > > > [ 90.567710] Performing i8k_smm call > > > [ 90.568567] Result of the i8k_smm call: -22 > > > [ 90.568567] Return value of i8k_get_temp_type: -22 > > > [ 90.568568] Result of i8k_get_temp_type(0): -22 > > > [ 90.568568] Calling i8k_get_temp_type(2) > > > [ 90.568568] Inside i8k_get_temp_type() with sensors = 2 > > > [ 90.568569] Performing i8k_smm call > > > [ 90.569441] Result of the i8k_smm call: -22 > > > [ 90.569441] Return value of i8k_get_temp_type: -22 > > > [ 90.569442] Result of i8k_get_temp_type(0): -22 > > > [ 90.569442] Calling i8k_get_temp_type(3) > > > [ 90.569442] Inside i8k_get_temp_type() with sensors = 3 > > > [ 90.569443] Performing i8k_smm call > > > [ 90.570321] Result of the i8k_smm call: -22 > > > [ 90.570322] Return value of i8k_get_temp_type: -22 > > > [ 90.570322] Result of i8k_get_temp_type(0): -22 > > > > > > So this must be the reason for which sensors aren't detec
Re: [PATCH] dell-smm-hwmon.c: Add XPS 9570 to supported devices list
On Wednesday 28 November 2018 23:23:07 Michele Sorcinelli wrote: > I tried to debug the temperature sensor probing with some printk() > around the code... it looks like the ssm calls are all returning -22 > as result: > > [ 90.565793] Calling i8k_get_temp_type(0) > [ 90.565795] Inside i8k_get_temp_type() with sensors = 0 > [ 90.565795] Performing i8k_smm call > [ 90.567708] Result of the i8k_smm call: -22 > [ 90.567708] Return value of i8k_get_temp_type: -22 > [ 90.567709] Result of i8k_get_temp_type(0): -22 > [ 90.567709] Calling i8k_get_temp_type(1) > [ 90.567710] Inside i8k_get_temp_type() with sensors = 1 > [ 90.567710] Performing i8k_smm call > [ 90.568567] Result of the i8k_smm call: -22 > [ 90.568567] Return value of i8k_get_temp_type: -22 > [ 90.568568] Result of i8k_get_temp_type(0): -22 > [ 90.568568] Calling i8k_get_temp_type(2) > [ 90.568568] Inside i8k_get_temp_type() with sensors = 2 > [ 90.568569] Performing i8k_smm call > [ 90.569441] Result of the i8k_smm call: -22 > [ 90.569441] Return value of i8k_get_temp_type: -22 > [ 90.569442] Result of i8k_get_temp_type(0): -22 > [ 90.569442] Calling i8k_get_temp_type(3) > [ 90.569442] Inside i8k_get_temp_type() with sensors = 3 > [ 90.569443] Performing i8k_smm call > [ 90.570321] Result of the i8k_smm call: -22 > [ 90.570322] Return value of i8k_get_temp_type: -22 > [ 90.570322] Result of i8k_get_temp_type(0): -22 > > So this must be the reason for which sensors aren't detected. Yes. SMM just say "I do not support this temp sensor". Also it is possible that Dell's SMM does not implement it at all. You can try to play with it more and check if temp sensor does not have higher index. But I cannot help more. As you said it is RPC black-box. > AFAIK, SSM code is much of a black-box and finding how to fix > this will probably require some reverse engineering. > > However, apart from this and the fact that kernel hangs during > the SSM call (that is reasonable and again not much can't be done), > everything looks fine. > > Let me know if you want me to include more information in the commit message > for the patch or want me to do more testing. That is enough. You can add my Reviewed-by: Pali Rohár and hwmon maintainers could pick up this patch. > On 11/28/18 1:01 PM, Michele Sorcinelli wrote: > > Yes, values are reported correctly when the fan are on. I tested it > > by disabling the firmware automatic control using > > https://github.com/TomFreudenberg/dell-bios-fan-control and then running > > i8kfan 1 1 and i8kfan 2 2, which brought the fans to 2500 and 5100 > > respectively. > > > > When automatic control is on I can see intermediate speeds as well. > > > > Is there anything that can be done to help the driver to discover > > the temperature sensors? > > > > On 11/28/18 12:56 PM, Pali Rohár wrote: > > > On Wednesday 28 November 2018 12:30:31 Michele Sorcinelli wrote: > > > > The output of sensors related to the is: > > > > > > > > dell_smm-virtual-0 > > > > Adapter: Virtual device > > > > fan1: 0 RPM > > > > fan2: 0 RPM > > > > > > Ok, this means that both fans are turned off. > > > > > > When you start fans, have both number meaningful/correct value? > > > > > > > This suggests that temperature sensors are not discovered. > > > > > > Yes, probably they are unsupported or not discovered. But this is not > > > a problem. > > > > > > > Speeds reported in the hwmon interface are consistent with the others. > > > > > > > > > > > > On 11/27/18 11:06 PM, Michele Sorcinelli wrote: > > > > > Allow the module to be loaded on Dell XPS 9570, without having to use > > > > > the "force=1" option. The module loads without problems, and reports > > > > > correct fan values: > > > > > > > > > > $ time cat /proc/i8k > > > > > 1.0 1.5 -1 35 0 0 0 0 -1 -22 > > > > > cat /proc/i8k 0.00s user 0.00s system 7% cpu 0.033 total > > > > > > > > > > However, the call may freeze the kernel for a very small time due to > > > > > code running in the SSM layer. This is a known issue with > > > > > the driver, and > > > > > can be reproduced with other supported models. Average execution > > > > > time is 33 ms. > > > > > > > > > > The command line tools from i8kutils can properly set the fan speed, > > > &g
Re: [PATCH] dell-smm-hwmon.c: Add XPS 9570 to supported devices list
On Wednesday 28 November 2018 12:30:31 Michele Sorcinelli wrote:
> The output of sensors related to the is:
>
> dell_smm-virtual-0
> Adapter: Virtual device
> fan1: 0 RPM
> fan2: 0 RPM
Ok, this means that both fans are turned off.
When you start fans, have both number meaningful/correct value?
> This suggests that temperature sensors are not discovered.
Yes, probably they are unsupported or not discovered. But this is not
a problem.
> Speeds reported in the hwmon interface are consistent with the others.
>
>
> On 11/27/18 11:06 PM, Michele Sorcinelli wrote:
> > Allow the module to be loaded on Dell XPS 9570, without having to use
> > the "force=1" option. The module loads without problems, and reports
> > correct fan values:
> >
> > $ time cat /proc/i8k
> > 1.0 1.5 -1 35 0 0 0 0 -1 -22
> > cat /proc/i8k 0.00s user 0.00s system 7% cpu 0.033 total
> >
> > However, the call may freeze the kernel for a very small time due to
> > code running in the SSM layer. This is a known issue with the driver, and
> > can be reproduced with other supported models. Average execution
> > time is 33 ms.
> >
> > The command line tools from i8kutils can properly set the fan speed,
> > although the firmware will override it, unless automatic fan
> > control is disabled with the proper SSM call.
> >
> > Average fans speed (when firwmare automatic control is off):
> >
> > STATE -> RPM
> > 0 0 -> 0 0
> > 1 1 -> 2500 2500
> > 2 2 -> 5100 5100
> > 3 3 -> same as 2 2
> > ---
> > drivers/hwmon/dell-smm-hwmon.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> > index 9d3ef879d..367a8a617 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -1017,6 +1017,13 @@ static const struct dmi_system_id i8k_dmi_table[]
> > __initconst = {
> > DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9560"),
> > },
> > },
> > + {
> > + .ident = "Dell XPS 15 9570",
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9570"),
> > + },
> > + },
> > { }
> > };
> >
--
Pali Rohár
pali.ro...@gmail.com
Re: [PATCH] dell-smm-hwmon.c: Add XPS 9570 to supported devices list
On Tuesday 27 November 2018 23:06:37 Michele Sorcinelli wrote:
> Allow the module to be loaded on Dell XPS 9570, without having to use
> the "force=1" option. The module loads without problems, and reports
> correct fan values:
>
> $ time cat /proc/i8k
> 1.0 1.5 -1 35 0 0 0 0 -1 -22
> cat /proc/i8k 0.00s user 0.00s system 7% cpu 0.033 total
Hi! This interface is deprecated and should not be used in new project
or software...
> However, the call may freeze the kernel for a very small time due to
> code running in the SSM layer. This is a known issue with the driver, and
> can be reproduced with other supported models. Average execution
> time is 33 ms.
>
> The command line tools from i8kutils can properly set the fan speed,
> although the firmware will override it, unless automatic fan
> control is disabled with the proper SSM call.
i8kutils uses /proc/i8k too.
Can you check standard hwmon interface via lm-sensors / sensors command
is also working correctly? If you do not have sensors command you can
also look into /sys/class/hwmon/
> Average fans speed (when firwmare automatic control is off):
>
> STATE -> RPM
> 0 0 -> 0 0
> 1 1 -> 2500 2500
> 2 2 -> 5100 5100
> 3 3 -> same as 2 2
> ---
> drivers/hwmon/dell-smm-hwmon.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 9d3ef879d..367a8a617 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1017,6 +1017,13 @@ static const struct dmi_system_id i8k_dmi_table[]
> __initconst = {
> DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9560"),
> },
> },
> + {
> + .ident = "Dell XPS 15 9570",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9570"),
> + },
> + },
> { }
> };
>
--
Pali Rohár
pali.ro...@gmail.com
Re: [PATCH] hwmon: (dell-smm) Disable fan support for Dell XPS13 9333
On Friday 08 June 2018 06:51:55 Guenter Roeck wrote:
> On Thu, Jun 07, 2018 at 10:25:42AM +0200, Pali Rohár wrote:
> > On Thursday 07 June 2018 08:04:55 Helge Eichelberg wrote:
> > > On Wed, 6 Jun 2018 17:21:34 +0200
> > > Pali Rohár wrote:
> > >
> > > > On Tuesday 05 June 2018 19:38:32 Helge Eichelberg wrote:
> > > > > Calling fan related SMM functions implemented by Dell BIOS firmware
> > > > > on Dell
> > > > > XPS13 9333 freeze kernel for about 500ms. Until Dell fixes it we need
> > > > > to
> > > > > disable fan support for Dell XPS13 9333.
> > > >
> > > > Hi! Have you reported this firmware bug to Dell?
> > >
> > > No, I haven't.
> >
> > Mario (from @Dell) wrote on github, that we should start reporting
> > firmware bugs to Dell as "voice of the customer is the most important".
> > https://github.com/dell/libsmbios/issues/48#issuecomment-393527813
> >
> > > > Also we should probably match also BIOS version and do not apply this
> > > > blacklist quirk for BIOS versions in which Dell fixed it.
> > >
> > > The BIOS hasn't been updated since 08/31/2015 and I'm running the latest
> > > version (A08). I wonder if Dell cares anymore about the 9333 which has
> > > been replaced by the 9343 in early 2015. Nevertheless, should I add a
> > > line matching the BIOS version and should it look like this?
> >
> > Ok, I was in impression that it was fixed or was going to be fixed. This
> > looks like that laptop does not have any support. In this case would
> > need to blacklist it for all bios version and forever. So no match for
> > bios version. Maybe you should re-phrase commit message to indicate that
> > Dell probably does not fix it. Or drop that sentence "Until Dell...".
> >
> Those would just be assumptions and not add any value to the commit log.
> I don't recall similar comments for previous patches either, and it is not
> as if Dell stopped supporting Linux only recently. I'll apply the patch
> as-is; I don't see value in a respin.
Ok. I have no objections. That was only suggestion. Also you can add my
Reviewed-by: Pali Rohár
Anyway, if last version of Bios was released 3 years ago, I really doubt
that something is going to be fixed...
> Guenter
>
> > > DMI_EXACT_MATCH(DMI_BIOS_VERSION, "A08")
> > >
> > > Helge
> > >
> > > > > Via "force" module param fan support can be enabled.
> > > > >
> > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195751
> > > > > Signed-off-by: Helge Eichelberg
> > > > > ---
> > > > > drivers/hwmon/dell-smm-hwmon.c | 7 +++
> > > > > 1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > > > > b/drivers/hwmon/dell-smm-hwmon.c
> > > > > index bf3bb7e1adab..9d3ef879dc51 100644
> > > > > --- a/drivers/hwmon/dell-smm-hwmon.c
> > > > > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > > > > @@ -1074,6 +1074,13 @@ static struct dmi_system_id
> > > > > i8k_blacklist_fan_support_dmi_table[] __initdata = {
> > > > > DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Vostro
> > > > > 3360"),
> > > > > },
> > > > > },
> > > > > + {
> > > > > + .ident = "Dell XPS13 9333",
> > > > > + .matches = {
> > > > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > > > > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS13 9333"),
> > > > > + },
> > > > > + },
> > > > > { }
> > > > > };
> > > > >
> > > >
> > > > --
> > > > Pali Rohár
> > > > pali.ro...@gmail.com
> >
> > --
> > Pali Rohár
> > pali.ro...@gmail.com
--
Pali Rohár
pali.ro...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (dell-smm) Disable fan support for Dell XPS13 9333
On Thursday 07 June 2018 08:04:55 Helge Eichelberg wrote:
> On Wed, 6 Jun 2018 17:21:34 +0200
> Pali Rohár wrote:
>
> > On Tuesday 05 June 2018 19:38:32 Helge Eichelberg wrote:
> > > Calling fan related SMM functions implemented by Dell BIOS firmware on
> > > Dell
> > > XPS13 9333 freeze kernel for about 500ms. Until Dell fixes it we need to
> > > disable fan support for Dell XPS13 9333.
> >
> > Hi! Have you reported this firmware bug to Dell?
>
> No, I haven't.
Mario (from @Dell) wrote on github, that we should start reporting
firmware bugs to Dell as "voice of the customer is the most important".
https://github.com/dell/libsmbios/issues/48#issuecomment-393527813
> > Also we should probably match also BIOS version and do not apply this
> > blacklist quirk for BIOS versions in which Dell fixed it.
>
> The BIOS hasn't been updated since 08/31/2015 and I'm running the latest
> version (A08). I wonder if Dell cares anymore about the 9333 which has been
> replaced by the 9343 in early 2015. Nevertheless, should I add a line
> matching the BIOS version and should it look like this?
Ok, I was in impression that it was fixed or was going to be fixed. This
looks like that laptop does not have any support. In this case would
need to blacklist it for all bios version and forever. So no match for
bios version. Maybe you should re-phrase commit message to indicate that
Dell probably does not fix it. Or drop that sentence "Until Dell...".
> DMI_EXACT_MATCH(DMI_BIOS_VERSION, "A08")
>
> Helge
>
> > > Via "force" module param fan support can be enabled.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195751
> > > Signed-off-by: Helge Eichelberg
> > > ---
> > > drivers/hwmon/dell-smm-hwmon.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > > b/drivers/hwmon/dell-smm-hwmon.c
> > > index bf3bb7e1adab..9d3ef879dc51 100644
> > > --- a/drivers/hwmon/dell-smm-hwmon.c
> > > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > > @@ -1074,6 +1074,13 @@ static struct dmi_system_id
> > > i8k_blacklist_fan_support_dmi_table[] __initdata = {
> > > DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Vostro 3360"),
> > > },
> > > },
> > > + {
> > > + .ident = "Dell XPS13 9333",
> > > + .matches = {
> > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > > + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS13 9333"),
> > > + },
> > > + },
> > > { }
> > > };
> > >
> >
> > --
> > Pali Rohár
> > pali.ro...@gmail.com
--
Pali Rohár
pali.ro...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (dell-smm) Disable fan support for Dell XPS13 9333
On Tuesday 05 June 2018 19:38:32 Helge Eichelberg wrote:
> Calling fan related SMM functions implemented by Dell BIOS firmware on Dell
> XPS13 9333 freeze kernel for about 500ms. Until Dell fixes it we need to
> disable fan support for Dell XPS13 9333.
Hi! Have you reported this firmware bug to Dell?
Also we should probably match also BIOS version and do not apply this
blacklist quirk for BIOS versions in which Dell fixed it.
> Via "force" module param fan support can be enabled.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=195751
> Signed-off-by: Helge Eichelberg
> ---
> drivers/hwmon/dell-smm-hwmon.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index bf3bb7e1adab..9d3ef879dc51 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1074,6 +1074,13 @@ static struct dmi_system_id
> i8k_blacklist_fan_support_dmi_table[] __initdata = {
> DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Vostro 3360"),
> },
> },
> + {
> + .ident = "Dell XPS13 9333",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS13 9333"),
> + },
> + },
> { }
> };
>
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: PGP signature
Re: [4/4] hwmon: (dell-smm) Measure time duration of SMM call around inlined asm
On Saturday 27 January 2018 09:51:45 Guenter Roeck wrote:
> On Sat, Jan 27, 2018 at 05:23:51PM +0100, Pali Rohár wrote:
> > Measure only inlined asm code, not other functions to have as precise as
> > possible measured time.
> >
> > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > ---
> > drivers/hwmon/dell-smm-hwmon.c | 14 ++
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> > index bf3bb7e1adab..e001afd53f46 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -147,14 +147,16 @@ static int i8k_smm_func(void *par)
> > int ebx = regs->ebx;
> > unsigned long duration;
> > ktime_t calltime, delta, rettime;
> > -
> > - calltime = ktime_get();
> > #endif
> >
> > /* SMM requires CPU 0 */
> > if (smp_processor_id() != 0)
> > return -EBUSY;
> >
> > +#ifdef DEBUG
> > + calltime = ktime_get();
> > +#endif
> > +
> > #if defined(CONFIG_X86_64)
> > asm volatile("pushq %%rax\n\t"
> > "movl 0(%%rax),%%edx\n\t"
> > @@ -208,13 +210,17 @@ static int i8k_smm_func(void *par)
> > :"a"(regs)
> > :"%ebx", "%ecx", "%edx", "%esi", "%edi", "memory");
> > #endif
> > - if (rc != 0 || (regs->eax & 0x) == 0x || regs->eax == eax)
> > - rc = -EINVAL;
> >
> > #ifdef DEBUG
> > rettime = ktime_get();
> > delta = ktime_sub(rettime, calltime);
> > duration = ktime_to_ns(delta) >> 10;
> > +#endif
> > +
> > + if (rc != 0 || (regs->eax & 0x) == 0x || regs->eax == eax)
> > + rc = -EINVAL;
> > +
> > +#ifdef DEBUG
>
> FWIW, the error introduced by dividing nS by 1,024 instead of 1,000 is
> much worse than the improvements from moving the calls around. Using
> specific numbers, the current code reports 500 mS as 488,281 uS.
> So why bother ?
Ah, I have not noticed this yet :-(
> I would have suggested to use ktime_us_delta(ktime_get(), calltime)
> instead to make the results more accurate. Sure, you get the offset from
> the additional divide operation, but at least that would be a constant
> and not a systematic error.
>
> I'll hold this patch off for a bit. Please confirm that you want it
> applied as-is. I applied the other patches from the series.
Ok, I will fix this patch and resend just new version of this one.
Anyway, if 2/4 is targeting stable, then 3/4 should too. But now I see
that I added Cc only to patch 2/4.
--
Pali Rohár
pali.ro...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] hwmon: (dell-smm) Disable fan support for Dell Inspiron 7720
Calling fan related SMM functions implemented by Dell BIOS firmware on Dell
Inspiron 7720 freeze kernel for about 500ms. Until Dell fixes it we need to
disable fan support for Dell Inspiron 7720 as it makes system unusable.
Via "force" module param fan support can be enabled.
Reported-by: vova7...@mail.ru
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=195751
Cc: sta...@vger.kernel.org # v4.0+, will need backport
---
drivers/hwmon/dell-smm-hwmon.c | 40 +++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index aef4f8317ae2..3f8b4e482b64 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -76,6 +76,7 @@ static uint i8k_fan_mult = I8K_FAN_MULT;
static uint i8k_pwm_mult;
static uint i8k_fan_max = I8K_FAN_HIGH;
static bool disallow_fan_type_call;
+static bool disallow_fan_support;
#define I8K_HWMON_HAVE_TEMP1 (1 << 0)
#define I8K_HWMON_HAVE_TEMP2 (1 << 1)
@@ -242,6 +243,9 @@ static int i8k_get_fan_status(int fan)
{
struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, };
+ if (disallow_fan_support)
+ return -EINVAL;
+
regs.ebx = fan & 0xff;
return i8k_smm() ? : regs.eax & 0xff;
}
@@ -253,6 +257,9 @@ static int i8k_get_fan_speed(int fan)
{
struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, };
+ if (disallow_fan_support)
+ return -EINVAL;
+
regs.ebx = fan & 0xff;
return i8k_smm() ? : (regs.eax & 0x) * i8k_fan_mult;
}
@@ -264,7 +271,7 @@ static int _i8k_get_fan_type(int fan)
{
struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
- if (disallow_fan_type_call)
+ if (disallow_fan_support || disallow_fan_type_call)
return -EINVAL;
regs.ebx = fan & 0xff;
@@ -289,6 +296,9 @@ static int i8k_get_fan_nominal_speed(int fan, int speed)
{
struct smm_regs regs = { .eax = I8K_SMM_GET_NOM_SPEED, };
+ if (disallow_fan_support)
+ return -EINVAL;
+
regs.ebx = (fan & 0xff) | (speed << 8);
return i8k_smm() ? : (regs.eax & 0x) * i8k_fan_mult;
}
@@ -300,6 +310,9 @@ static int i8k_set_fan(int fan, int speed)
{
struct smm_regs regs = { .eax = I8K_SMM_SET_FAN, };
+ if (disallow_fan_support)
+ return -EINVAL;
+
speed = (speed < 0) ? 0 : ((speed > i8k_fan_max) ? i8k_fan_max : speed);
regs.ebx = (fan & 0xff) | (speed << 8);
@@ -772,6 +785,8 @@ static struct attribute *i8k_attrs[] = {
static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
int index)
{
+ if (disallow_fan_support && index >= 8)
+ return 0;
if (disallow_fan_type_call &&
(index == 9 || index == 12 || index == 15))
return 0;
@@ -1039,6 +1054,23 @@ static const struct dmi_system_id
i8k_blacklist_fan_type_dmi_table[] __initconst
};
/*
+ * On some machines all fan related SMM functions implemented by Dell BIOS
+ * firmware freeze kernel for about 500ms. Until Dell fixes these problems fan
+ * support for affected blacklisted Dell machines stay disabled.
+ * See bug: https://bugzilla.kernel.org/show_bug.cgi?id=195751
+ */
+static struct dmi_system_id i8k_blacklist_fan_support_dmi_table[] __initdata =
{
+ {
+ .ident = "Dell Inspiron 7720",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Inspiron 7720"),
+ },
+ },
+ { }
+};
+
+/*
* Probe for the presence of a supported laptop.
*/
static int __init i8k_probe(void)
@@ -1060,6 +1092,12 @@ static int __init i8k_probe(void)
i8k_get_dmi_data(DMI_BIOS_VERSION));
}
+ if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
+ pr_warn("broken Dell BIOS detected, disallow fan support\n");
+ if (!force)
+ disallow_fan_support = true;
+ }
+
if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
pr_warn("broken Dell BIOS detected, disallow fan type call\n");
if (!force)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] hwmon: (dell-smm) Measure time duration of SMM call around inlined asm
Measure only inlined asm code, not other functions to have as precise as
possible measured time.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/hwmon/dell-smm-hwmon.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index bf3bb7e1adab..e001afd53f46 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -147,14 +147,16 @@ static int i8k_smm_func(void *par)
int ebx = regs->ebx;
unsigned long duration;
ktime_t calltime, delta, rettime;
-
- calltime = ktime_get();
#endif
/* SMM requires CPU 0 */
if (smp_processor_id() != 0)
return -EBUSY;
+#ifdef DEBUG
+ calltime = ktime_get();
+#endif
+
#if defined(CONFIG_X86_64)
asm volatile("pushq %%rax\n\t"
"movl 0(%%rax),%%edx\n\t"
@@ -208,13 +210,17 @@ static int i8k_smm_func(void *par)
:"a"(regs)
:"%ebx", "%ecx", "%edx", "%esi", "%edi", "memory");
#endif
- if (rc != 0 || (regs->eax & 0x) == 0x || regs->eax == eax)
- rc = -EINVAL;
#ifdef DEBUG
rettime = ktime_get();
delta = ktime_sub(rettime, calltime);
duration = ktime_to_ns(delta) >> 10;
+#endif
+
+ if (rc != 0 || (regs->eax & 0x) == 0x || regs->eax == eax)
+ rc = -EINVAL;
+
+#ifdef DEBUG
pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x (took %7lu usecs)\n", eax, ebx,
(rc ? 0x : regs->eax & 0x), duration);
#endif
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] hwmon: (dell-smm) Disable fan support on Inspiron 7720 and Vostro 3360
This patch series disable fan support on two machines on which is BIOS broken. And it changes measurement of SMM calls duration. Oleksandr Natalenko (1): hwmon: (dell-smm) Disable fan support for Dell Vostro 3360 Pali Rohár (3): hwmon: (dell-smm) Enable broken functionality via "force" module param hwmon: (dell-smm) Disable fan support for Dell Inspiron 7720 hwmon: (dell-smm) Measure time duration of SMM call around inlined asm drivers/hwmon/dell-smm-hwmon.c | 68 +- 1 file changed, 61 insertions(+), 7 deletions(-) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] hwmon: (dell-smm) Enable broken functionality via "force" module param
Some Dell machines are broken and some functionality is disabled. Show
warning into dmesg about this fact and allow user via "force" module param
to override brokenness and enable broken functionality.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/hwmon/dell-smm-hwmon.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c7c9e95e58a8..aef4f8317ae2 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1060,8 +1060,11 @@ static int __init i8k_probe(void)
i8k_get_dmi_data(DMI_BIOS_VERSION));
}
- if (dmi_check_system(i8k_blacklist_fan_type_dmi_table))
- disallow_fan_type_call = true;
+ if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
+ pr_warn("broken Dell BIOS detected, disallow fan type call\n");
+ if (!force)
+ disallow_fan_type_call = true;
+ }
strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
sizeof(bios_version));
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i8k_smm_func() takes enormous of time to execute
On Monday 04 December 2017 13:18:29 Pali Rohár wrote: > > > Your last patch in bugzilla looks ok, you add my Reviewed-by: Pali > > > Rohár <pali.ro...@gmail.com> > > > > Could you please advice on how to proceed further? I can submit all 3 > > patches (incl. yours two), to a ML. > > Now it is up to hwmon maintainers (Jean Delvare & Guenter Roeck) to pick > up these patches. I think nothing more is needed from your side. (And if > yes, Jean would write.) > Jean Delvare & Guenter Roeck: PING Can you process patches from that bugzilla ticket? https://bugzilla.kernel.org/show_bug.cgi?id=195751 -- Pali Rohár pali.ro...@gmail.com signature.asc Description: PGP signature
Re: i8k_smm_func() takes enormous of time to execute
On Monday 04 December 2017 13:08:18 Oleksandr Natalenko wrote: > Hi. > > 04.12.2017 11:35, Pali Rohár wrote: > > On Friday 24 November 2017 13:28:59 Oleksandr Natalenko wrote: > > > On pátek 24. listopadu 2017 12:25:43 CET Pali Rohár wrote: > > > > On Friday 24 November 2017 12:17:30 Oleksandr Natalenko wrote: > > > > > > There are two patches waiting to be tested in > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=195751 > > > > > > > > > > Tested and attached a couple of patches on top of those to the BZ. If > > > > > disabling fan control is the only approach here, I can confirm that > > > > > this > > > > > works. > > > > > > > > Hi! Please figure out which SMM call (they are identified by eax and ebx > > > > registers) cause this freeze. So we would now what needs to be > > > > blacklisted. > > > > > > Here it goes: > > > > > > [ 7.191081] dell_smm_hwmon: smm(0x10a3 0x) = 0x002e (took > > > 1837 usecs) > > > [ 7.194007] dell_smm_hwmon: smm(0x00a3 0x0001) = 0x (took > > > 151 usecs) > > > [ 7.198239] dell_smm_hwmon: smm(0x00a3 0x) = 0x0002 (took > > > 1411 usecs) > > > [ 7.199095] dell_smm_hwmon: smm(0x02a3 0x0001) = 0x (took > > > 71 usecs) > > > [ 7.700493] dell_smm_hwmon: smm(0x02a3 0x) = 0x157c (took > > > 488912 usecs) > > > [ 7.701277] dell_smm_hwmon: smm(0x0025 0X) = 0x (took > > > 71 usecs) > > > > > > 0x02a3 is I8K_SMM_GET_SPEED. > > > > Ok, so it is really a good idea to disable fan control completely on > > your machine. > > This still doesn't explain why things work in non-Linux environment… No, they does not work in non-Linux environment too. You can call same smm request also on Windows and it freeze computer in same way. Just windows do not have such smm driver out of box. To reproduce this problem you need to write either own kernel driver in assembler (like driver for linux) or write that smm code in assembler for classic (userspace) application and find out how to call that smm code via WinAPI (probably you would need Administrator rights and study lot of WinAPI documentation). > But as a workaround, okay, let it be. Basically once you issue this 0x02a3 call your computer freeze (more precisely stay in SMM mode -- it is x86 processor mode) and such operation is fully independent of what is running on your x86 cpu (Linux, Windows, DOS, some RTOS system, ...) > > Your last patch in bugzilla looks ok, you add my Reviewed-by: Pali > > Rohár <pali.ro...@gmail.com> > > Could you please advice on how to proceed further? I can submit all 3 > patches (incl. yours two), to a ML. Now it is up to hwmon maintainers (Jean Delvare & Guenter Roeck) to pick up these patches. I think nothing more is needed from your side. (And if yes, Jean would write.) -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i8k_smm_func() takes enormous of time to execute
On Friday 24 November 2017 13:28:59 Oleksandr Natalenko wrote: > On pátek 24. listopadu 2017 12:25:43 CET Pali Rohár wrote: > > On Friday 24 November 2017 12:17:30 Oleksandr Natalenko wrote: > > > > There are two patches waiting to be tested in > > > > https://bugzilla.kernel.org/show_bug.cgi?id=195751 > > > > > > Tested and attached a couple of patches on top of those to the BZ. If > > > disabling fan control is the only approach here, I can confirm that this > > > works. > > > > Hi! Please figure out which SMM call (they are identified by eax and ebx > > registers) cause this freeze. So we would now what needs to be > > blacklisted. > > Here it goes: > > [ 7.191081] dell_smm_hwmon: smm(0x10a3 0x) = 0x002e (took 1837 usecs) > [ 7.194007] dell_smm_hwmon: smm(0x00a3 0x0001) = 0x (took 151 usecs) > [ 7.198239] dell_smm_hwmon: smm(0x00a3 0x) = 0x0002 (took 1411 usecs) > [ 7.199095] dell_smm_hwmon: smm(0x02a3 0x0001) = 0x (took 71 usecs) > [ 7.700493] dell_smm_hwmon: smm(0x02a3 0x) = 0x157c (took 488912 usecs) > [ 7.701277] dell_smm_hwmon: smm(0x0025 0X) = 0x (took 71 usecs) > > 0x02a3 is I8K_SMM_GET_SPEED. Ok, so it is really a good idea to disable fan control completely on your machine. Your last patch in bugzilla looks ok, you add my Reviewed-by: Pali Rohár <pali.ro...@gmail.com> -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i8k_smm_func() takes enormous of time to execute
On Friday 24 November 2017 12:17:30 Oleksandr Natalenko wrote: > > There are two patches waiting to be tested in > > https://bugzilla.kernel.org/show_bug.cgi?id=195751 > > Tested and attached a couple of patches on top of those to the BZ. If > disabling fan control is the only approach here, I can confirm that this > works. Hi! Please figure out which SMM call (they are identified by eax and ebx registers) cause this freeze. So we would now what needs to be blacklisted. -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i8k_smm_func() takes enormous of time to execute
On Thursday 23 November 2017 22:41:03 Oleksandr Natalenko wrote: > Hi, Jonathan, Mario et al. > > I've noticed that querying Dell Vostro 3360 hwmon sensor freezes the system > completely (like, really completely, even mouse cursor does not move, and > sound playback stops) for approx. half of a second. Then, system recovers and > runs as usual. Also, sensor readings are okay, for instance: > > === > $ sensors dell_smm-virtual-0 > dell_smm-virtual-0 > Adapter: Virtual device > Processor Fan: 5615 RPM > CPU:+51.0°C > Ambient:+36.0°C > Other: +63.0°C > SODIMM: +41.0°C > === > > So, I've used trace-cmd to check what takes that much amount of time to > execute, and got this: > > === > # trace-cmd record -p function_graph -l i8k_smm -F sensors dell_smm-virtual-0 > $ trace-cmd report > ... > sensors-23694 [002] 89099.214369: funcgraph_entry: # 503440.746 us | > i8k_smm(); > ... > === > > Clearly, 0.5 s delay. > > Looking at i8k_smm(), it calls i8k_smm_func() on 0th CPU: > > === > 232 ret = smp_call_on_cpu(0, i8k_smm_func, regs, true); > === > > which, in turn, does some asm magic. Hi! If you compile module with DEBUG, then i8k_smm_func function print into dmes how much time it spend in SMM mode. And also it print which SMM action it was. Therefore you would be able to figure out which SMM call (which is just RPC) cause this freeze. Basically when CPU is in SMM mode, then execution of kernel is stopped and kernel does not know that such thing happened. So it is up to the SMM implementation to return control ASAP back to non-SMM mode. Kernel has no control over SMM, it can be fixed only by vendor by new BIOS or firmware update. What we can do is just identify problematic calls and avoid their usage. This problem is independent of kernel or whole operating system. > I know that SMM is kinda "black box", and kernel has little to do with it, > but > I think that under Windows, for instance, it would work without freezes. So, > likely, querying SMM might be done differently. > > I do not know how to approach this issue, thus asking for help/advice. Also, > CCing Jonathan since the comment before asm magic says this: > > === > 137 * Call the System Management Mode BIOS. Code provided by Jonathan > Buzzard. > === > > This was also reported in various places before, for instance, [1], but > unfortunately, without any solution. > > Thanks. > > Regards, > Oleksandr > > [1] https://bugs.launchpad.net/ubuntu/+source/acpi/+bug/10490 -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hwmon: (dell-smm) Add Dell XPS 15 9560 into DMI list
It was reported that dell-smm-hwmon is working fine on Dell XPS 15 9560.
Link: http://www.spinics.net/lists/platform-driver-x86/msg10751.html
Reported-by: Vasile Dumitrescu <vasile.dumitre...@undeva.eu>
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/hwmon/dell-smm-hwmon.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 34704b0..3189246 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -995,6 +995,13 @@ enum i8k_configs {
},
.driver_data = (void *)_config_data[DELL_XPS],
},
+ {
+ .ident = "Dell XPS 15 9560",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9560"),
+ },
+ },
{ }
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] dell-smm-hwmon fixes
Thanks for testing! It took too long, but bugs in vendor SMM code are hard to detect and probably impossible to fix. So I would call this patch just as "workaround" and not proper bug fix... On Saturday 18 June 2016 23:58:19 Leon Yu wrote: > Just installed on "Inspiron 580", appears to have fixed the problem. > > On Sat, Jun 18, 2016 at 12:54 PM, Guenter Roeck <li...@roeck-us.net> > wrote: > > On 06/18/2016 08:26 AM, Pali Rohár wrote: > >> On Saturday 18 June 2016 17:13:59 Guenter Roeck wrote: > >>> On 06/17/2016 03:54 PM, Pali Rohár wrote: > >>>> I'm sending all my dell-smm-hwmon patches in one series, because > >>>> due to changes in code other patches depends on previous. > >>>> > >>>> First two patches fixes problem with old /proc/i8k file. Second > >>>> is security fix and should be backported to all stable kernels > >>>> (that problem was there always). I tested i8kctl tool (from > >>>> i8kutils package) that it still works with these patches. > >>>> Without root access for those security operations just showes > >>>> '?' or '-1'. > >>>> > >>>> Third and fourth patches try to fix problem on machines with > >>>> broken SMM/BIOS when calling function fan_type(). > >>>> > >>>> Fifth is new feature and last sixth useful for debugging. > >>>> > >>>> Pali Rohár (6): > >>>> hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios > >>>> version is > >>>> > >>>> not a number > >>>> > >>>> hwmon: (dell-smm) Restrict fan control and serial number to > >>>> > >>>> CAP_SYS_ADMIN by default > >>>> > >>>> hwmon: (dell-smm) Disallow fan_type() calls on broken > >>>> machines hwmon: (dell-smm) Cache fan_type() calls and > >>>> change fan detection hwmon: (dell-smm) Detect fan with > >>>> index=2 hwmon: (dell-smm) In debug mode log duration of SMM > >>>> calls > >>>> > >>>>drivers/hwmon/dell-smm-hwmon.c | 122 > >>>> 1 file changed, 99 > >>>>insertions(+), 23 deletions(-) > >>> > >>> Pali, > >>> > >>> You asked for additional testing, so I am not sure what you > >>> expect me to do. > >>> > >>> Which of the patches can/should I apply now ? > >>> > >>> Guenter > >> > >> Test 3/6 and 4/6 patches on affected Dell machines. I CCed all > >> people who tried to debug those bugs, so need confirmation from > >> them that after applying 3/6 and 4/6 patches, erratic fan > >> behaviour is not there... > >> > >> But because those two patches depends on previous, it is needed to > >> test whole series... > >> > >> This doesn't tell me which patches to apply now. The first two ? > > > > Guenter -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH 0/6] dell-smm-hwmon fixes
On Saturday 18 June 2016 18:54:58 Guenter Roeck wrote: > On 06/18/2016 08:26 AM, Pali Rohár wrote: > > On Saturday 18 June 2016 17:13:59 Guenter Roeck wrote: > >> On 06/17/2016 03:54 PM, Pali Rohár wrote: > >>> I'm sending all my dell-smm-hwmon patches in one series, because > >>> due to changes in code other patches depends on previous. > >>> > >>> First two patches fixes problem with old /proc/i8k file. Second > >>> is security fix and should be backported to all stable kernels > >>> (that problem was there always). I tested i8kctl tool (from > >>> i8kutils package) that it still works with these patches. > >>> Without root access for those security operations just showes > >>> '?' or '-1'. > >>> > >>> Third and fourth patches try to fix problem on machines with > >>> broken SMM/BIOS when calling function fan_type(). > >>> > >>> Fifth is new feature and last sixth useful for debugging. > >>> > >>> Pali Rohár (6): > >>> hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios > >>> version is > >>> > >>> not a number > >>> > >>> hwmon: (dell-smm) Restrict fan control and serial number to > >>> > >>> CAP_SYS_ADMIN by default > >>> > >>> hwmon: (dell-smm) Disallow fan_type() calls on broken > >>> machines hwmon: (dell-smm) Cache fan_type() calls and change > >>> fan detection hwmon: (dell-smm) Detect fan with index=2 > >>> hwmon: (dell-smm) In debug mode log duration of SMM calls > >>> > >>>drivers/hwmon/dell-smm-hwmon.c | 122 > >>> 1 file changed, 99 > >>>insertions(+), 23 deletions(-) > >> > >> Pali, > >> > >> You asked for additional testing, so I am not sure what you expect > >> me to do. > >> > >> Which of the patches can/should I apply now ? > >> > >> Guenter > > > > Test 3/6 and 4/6 patches on affected Dell machines. I CCed all > > people who tried to debug those bugs, so need confirmation from > > them that after applying 3/6 and 4/6 patches, erratic fan > > behaviour is not there... > > > > But because those two patches depends on previous, it is needed to > > test whole series... > > This doesn't tell me which patches to apply now. The first two ? Yes, 1/6 and 2/6 are OK. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH 0/6] dell-smm-hwmon fixes
On Saturday 18 June 2016 17:13:59 Guenter Roeck wrote: > On 06/17/2016 03:54 PM, Pali Rohár wrote: > > I'm sending all my dell-smm-hwmon patches in one series, because > > due to changes in code other patches depends on previous. > > > > First two patches fixes problem with old /proc/i8k file. Second is > > security fix and should be backported to all stable kernels (that > > problem was there always). I tested i8kctl tool (from i8kutils > > package) that it still works with these patches. Without root > > access for those security operations just showes '?' or '-1'. > > > > Third and fourth patches try to fix problem on machines with broken > > SMM/BIOS when calling function fan_type(). > > > > Fifth is new feature and last sixth useful for debugging. > > > > Pali Rohár (6): > >hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios > >version is > > > > not a number > > > >hwmon: (dell-smm) Restrict fan control and serial number to > > > > CAP_SYS_ADMIN by default > > > >hwmon: (dell-smm) Disallow fan_type() calls on broken machines > >hwmon: (dell-smm) Cache fan_type() calls and change fan > >detection hwmon: (dell-smm) Detect fan with index=2 > >hwmon: (dell-smm) In debug mode log duration of SMM calls > > > > drivers/hwmon/dell-smm-hwmon.c | 122 > > 1 file changed, 99 > > insertions(+), 23 deletions(-) > > Pali, > > You asked for additional testing, so I am not sure what you expect me > to do. > > Which of the patches can/should I apply now ? > > Guenter Test 3/6 and 4/6 patches on affected Dell machines. I CCed all people who tried to debug those bugs, so need confirmation from them that after applying 3/6 and 4/6 patches, erratic fan behaviour is not there... But because those two patches depends on previous, it is needed to test whole series... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH 0/6] dell-smm-hwmon fixes
On Saturday 18 June 2016 00:54:43 Pali Rohár wrote: > I'm sending all my dell-smm-hwmon patches in one series, because due to > changes in code other patches depends on previous. > > First two patches fixes problem with old /proc/i8k file. Second is security > fix and should be backported to all stable kernels (that problem was there > always). I tested i8kctl tool (from i8kutils package) that it still works > with these patches. Without root access for those security operations just > showes '?' or '-1'. > > Third and fourth patches try to fix problem on machines with broken > SMM/BIOS when calling function fan_type(). > > Fifth is new feature and last sixth useful for debugging. > > Pali Rohár (6): > hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios version is > not a number > hwmon: (dell-smm) Restrict fan control and serial number to > CAP_SYS_ADMIN by default > hwmon: (dell-smm) Disallow fan_type() calls on broken machines > hwmon: (dell-smm) Cache fan_type() calls and change fan detection > hwmon: (dell-smm) Detect fan with index=2 > hwmon: (dell-smm) In debug mode log duration of SMM calls > > drivers/hwmon/dell-smm-hwmon.c | 122 > > 1 file changed, 99 insertions(+), 23 deletions(-) > Hi, can you test this patch series on affected Dell Studio XPS and Inspiron 580 machines? Also on affected Precision machines? I believe it finally fix those freeze and cpu fan speed problems, but needs testing... -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] dell-smm-hwmon fixes
I'm sending all my dell-smm-hwmon patches in one series, because due to changes in code other patches depends on previous. First two patches fixes problem with old /proc/i8k file. Second is security fix and should be backported to all stable kernels (that problem was there always). I tested i8kctl tool (from i8kutils package) that it still works with these patches. Without root access for those security operations just showes '?' or '-1'. Third and fourth patches try to fix problem on machines with broken SMM/BIOS when calling function fan_type(). Fifth is new feature and last sixth useful for debugging. Pali Rohár (6): hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios version is not a number hwmon: (dell-smm) Restrict fan control and serial number to CAP_SYS_ADMIN by default hwmon: (dell-smm) Disallow fan_type() calls on broken machines hwmon: (dell-smm) Cache fan_type() calls and change fan detection hwmon: (dell-smm) Detect fan with index=2 hwmon: (dell-smm) In debug mode log duration of SMM calls drivers/hwmon/dell-smm-hwmon.c | 122 1 file changed, 99 insertions(+), 23 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] hwmon: (dell-smm) Detect fan with index=2
Some Dell machines (e.g. Dell Precision M3800) have two fans, first with
index=0 and second with index=2. So export also attributes for third fan
device with index=2.
Reported-by: Tolga Cakir <cevel...@gmail.com>
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/hwmon/dell-smm-hwmon.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 2ac87d5..571d498 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -81,6 +81,7 @@ static bool disallow_fan_type_call;
#define I8K_HWMON_HAVE_TEMP4 (1 << 3)
#define I8K_HWMON_HAVE_FAN1(1 << 4)
#define I8K_HWMON_HAVE_FAN2(1 << 5)
+#define I8K_HWMON_HAVE_FAN3(1 << 6)
MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
MODULE_AUTHOR("Pali Rohár <pali.ro...@gmail.com>");
@@ -252,7 +253,7 @@ static int _i8k_get_fan_type(int fan)
static int i8k_get_fan_type(int fan)
{
/* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
- static int types[2] = { INT_MIN, INT_MIN };
+ static int types[3] = { INT_MIN, INT_MIN, INT_MIN };
if (types[fan] == INT_MIN)
types[fan] = _i8k_get_fan_type(fan);
@@ -719,6 +720,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO,
i8k_hwmon_show_fan_label, NULL,
1);
static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
i8k_hwmon_set_pwm, 1);
+static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
+ 2);
+static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
+ i8k_hwmon_set_pwm, 2);
static struct attribute *i8k_attrs[] = {
_dev_attr_temp1_input.dev_attr.attr, /* 0 */
@@ -735,6 +742,9 @@ static struct attribute *i8k_attrs[] = {
_dev_attr_fan2_input.dev_attr.attr, /* 11 */
_dev_attr_fan2_label.dev_attr.attr, /* 12 */
_dev_attr_pwm2.dev_attr.attr,/* 13 */
+ _dev_attr_fan3_input.dev_attr.attr, /* 14 */
+ _dev_attr_fan3_label.dev_attr.attr, /* 15 */
+ _dev_attr_pwm3.dev_attr.attr,/* 16 */
NULL
};
@@ -742,7 +752,7 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct
attribute *attr,
int index)
{
if (disallow_fan_type_call &&
- (index == 9 || index == 12))
+ (index == 9 || index == 12 || index == 15))
return 0;
if (index >= 0 && index <= 1 &&
!(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1))
@@ -762,6 +772,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct
attribute *attr,
if (index >= 11 && index <= 13 &&
!(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
return 0;
+ if (index >= 14 && index <= 16 &&
+ !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
+ return 0;
return attr->mode;
}
@@ -807,6 +820,13 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
+ /* Third fan attributes, if fan status or type is OK */
+ err = i8k_get_fan_status(2);
+ if (err < 0)
+ err = i8k_get_fan_type(2);
+ if (err >= 0)
+ i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
+
i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm",
NULL, i8k_groups);
if (IS_ERR(i8k_hwmon_dev)) {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection
On Monday 30 May 2016 13:45:51 Thorsten Leemhuis wrote: > Pali Rohár wrote on 27.05.2016 15:21: > > On Friday 27 May 2016 15:05:54 Thorsten Leemhuis wrote: > >> Pali Rohár wrote on 27.05.2016 12:45: > >> So I tried a few things > >> and came to the conclusion: the problem shows up as soon as > >> i8k_get_fan_type() (introduced in f989e55452) is called somewhere. > > > > So, once kernel call i8k_get_fan_type() function, then fan speed > > going up/down? > > Yes. > > > To make sure that this is root of your problem, can you take some > > older kernel version (where is i8k working fine) and try to > > patch+call that i8k_get_fan_type() function? To check that > > something else cannot interference with it... > > I just tried with 3.19.8 (had to install a older distro first :-/ ), > where the problem does not show up (I verified just to be sure). Then > I applied below patch and voila: the fan speed starts going up/down. > > IOW: From what I can see that SMM call that i8k_get_fan_type() > triggers the problem on my Studio 8000 > > CU, knurd > Thank you for your testing! I believe now we definitely know root of this problem. It is buggy Dell BIOS/SMM code and we need to avoid calling I8K_SMM_GET_FAN_TYPE on affected buggy Dell machines. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: dell-smm-hwmon: security problems
On Wednesday 08 June 2016 19:37:43 Guenter Roeck wrote: > On Wed, Jun 08, 2016 at 03:55:48PM +0200, Pali Rohár wrote: > > And do you have idea what to do with problem 1)? > > If you really want to do something about it, you could whiteout the > serial number if CAP_SYS_ADMIN is not set. Ok, that sounds reasonable. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: dell-smm-hwmon: security problems
On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote: > On 06/08/2016 02:57 AM, Pali Rohár wrote: > > Hello! > > > > Mario wrote me about two I think security problems in > > dell-smm-hwmon driver and I would like to ask you, how to fix > > them. > > > > 1) File /proc/i8k (exists only when kernel is compiled with > > CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by > > ordinary user, without root permission. Normally > > DMI_PRODUCT_SERIAL can be read from sysfs file > > /sys/class/dmi/id/product_serial but only by root user. > > > > 2) Via /proc/i8k ordinary user can set fan speed. This is because > > how "restricted" parameter and variable works. Setting fan speed > > by normal non-root user can be dangerous, e.g. malicious > > application under user "nobody" could take control of fans. > > > > Do you have idea how to fix these problems? Just to note that > > /proc/i8k has stable kernel ABI and changing it will break all > > existing i8k* applications. But /proc/i8k is there only for old > > legacy laptops (year 2000). > > > > There is module parameter "restricted" with default value false and > > description: "Allow fan control if SYS_ADMIN capability set". > > Current code do: > > > > case I8K_SET_FAN: > > if (restricted && !capable(CAP_SYS_ADMIN)) > > return -EPERM; > > > > For me description is a bit ambiguous. What about setting > > "restricted" by default to true and updating description to > > something like this? > > > > "Disallow fan control when SYS_ADMIN capability is not set > > (default: 1)" > > Sure. I am sure that someone will complain (we learned just recently > that people still use the old commands, after all), but then the old > behavior can be restored by setting the flag to 0. Either setting that flag to 0 or running that tool under root or with capability CAP_SYS_ADMIN. > I would not use a double negative to describe it. Why not just > something like "Allow fan control only if SYS_ADMIN capability set > (default 1)" ? I was thinking about that description too, but there is problem with meaning too... 0 means fan control is allowed for any user 1 means fan control is allowed only for CAP_SYS_ADMIN Description should be unambiguous for situation when flag is set to 0. === And do you have idea what to do with problem 1)? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
dell-smm-hwmon: security problems
Hello! Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them. 1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user. 2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans. Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000). There is module parameter "restricted" with default value false and description: "Allow fan control if SYS_ADMIN capability set". Current code do: case I8K_SET_FAN: if (restricted && !capable(CAP_SYS_ADMIN)) return -EPERM; For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this? "Disallow fan control when SYS_ADMIN capability is not set (default: 1)" -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control
On Monday 30 May 2016 17:26:36 Gabriele Mazzotta wrote: > On 30/05/2016 11:32, Pali Rohár wrote: > > On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote: > >> On 22/05/2016 13:50, Pali Rohár wrote: > >>> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling > >>> or disabling automatic fan control by BIOS. Standard value "1" is for > >>> disabling automatic BIOS fan control and value "2" for enabling. > >>> > >>> Currently there is no way to check if BIOS auto mode is enabled (at least > >>> it is not know how to do it), so hwmon sysfs attribute is write-only. > >>> > >>> By default BIOS auto mode is enabled by laptop firmware. > >>> > >>> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX > >>> sysfs attribute) is overwritten by SMM in few seconds and therefore any > >>> custom settings are without effect. So this is reason why implementing > >>> option for disabling BIOS auto mode is needed. > >>> > >>> So finally this patch allows kernel to set and control fan speed on > >>> laptops, but it can be dangerous (like setting speed of other fans). > >>> > >>> This new feature is highly experimental, uses Dell SMM calls, so does not > >>> have to be supported by all laptops BIOSes. It was tested on Dell Latitude > >>> E6440 with BIOS A5. > >> > >> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears > >> to have no effect here. > > > > Can you check what i8k_enable_fan_auto_mode() returns? Error code or not? > > No error. However, independently on the value I set, the BIOS always > overrides the speed of the fan. Ok, looks like this is model or platform specific call... If you want, you can try to play with dellfan source code [1]. There is e.g. in comment specified two methods for disabling that BIOS thermal automode [2]. Looks like second method is that which I implemented in patch... [1] - https://github.com/clopez/dellfan [2] - https://github.com/clopez/dellfan/blob/master/dellfan.c#L64 -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection
On Monday 30 May 2016 17:25:26 Peter Saunderson wrote: > On 30/05/16 10:36, Pali Rohár wrote: > >Hi Peter! Thank you for information! Are you able to try to call that > >function on some old kernel (e.g. 3.12 or 3.14) to verify that it is > >caused only and only by that function? > > > I have tried to use my old 3.19.0 kernel that did not have the problem but > now it will not boot. The boot screen is left with > > Loading Linux linux... > Loading initial ramdisk... > > No logs etc.. I am guessing that some grub or ram disk change is giving me > the problem. I have just upgraded to Ubuntu 16.04 and do not want to > downgrade so I am a bit stuck at the moment and doubt that I can do this > test. > > Peter. You can try to regenerate initramfs with ubuntu command: $ sudo update-initramfs -u -k all In case you modified or updated some initram files or kernel modules you need to regenerate it to take effect at boot... -k all tells to regenerate *all* initramfs images, not only for currently booted kernel. -- Pali Rohár pali.ro...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dell-smm-hwmon: Detect fan with index=2
On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote:
> On 05/21/2016 07:52 AM, Pali Rohár wrote:
> > Some Dell machines (e.g. Dell Precision M3800) have two fans, first
> > with index=0 and second with index=2. So export also attributes
> > for third fan device with index=2.
> >
> > Reported-by: Tolga Cakir <cevel...@gmail.com>
> > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > ---
> >
> > drivers/hwmon/dell-smm-hwmon.c | 20 +++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > ---
> >
> > Hi Tolga! Can you test this patch if sensors see fan device
> > correctly?
>
> I assume this means I should wait for test results before applying
> the patch ?
Yes, testing should be done.
Do you have some pending/testing/next tree/branch for such patches?
> Thanks,
> Guenter
>
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
> >
> > #define I8K_HWMON_HAVE_TEMP4 (1 << 3)
> > #define I8K_HWMON_HAVE_FAN1 (1 << 4)
> > #define I8K_HWMON_HAVE_FAN2 (1 << 5)
> >
> > +#define I8K_HWMON_HAVE_FAN3(1 << 6)
> >
> > MODULE_AUTHOR("Massimo Dal Zotto (d...@debian.org)");
> > MODULE_AUTHOR("Pali Rohár <pali.ro...@gmail.com>");
> >
> > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan)
> >
> > static int i8k_get_fan_type(int fan)
> > {
> >
> > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values
> > */
> >
> > - static int types[2] = { INT_MIN, INT_MIN };
> > + static int types[3] = { INT_MIN, INT_MIN, INT_MIN };
> >
> > if (types[fan] == INT_MIN)
> >
> > types[fan] = _i8k_get_fan_type(fan);
> >
> > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL,
> >
> > 1);
> >
> > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR,
> > i8k_hwmon_show_pwm,
> >
> > i8k_hwmon_set_pwm, 1);
> >
> > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan,
> > NULL, + 2);
> > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO,
> > i8k_hwmon_show_fan_label, NULL, + 2);
> > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR,
> > i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2);
> >
> > static struct attribute *i8k_attrs[] = {
> >
> > _dev_attr_temp1_input.dev_attr.attr, /* 0 */
> >
> > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
> >
> > _dev_attr_fan2_input.dev_attr.attr, /* 11 */
> > _dev_attr_fan2_label.dev_attr.attr, /* 12 */
> > _dev_attr_pwm2.dev_attr.attr,/* 13 */
> >
> > + _dev_attr_fan3_input.dev_attr.attr, /* 14 */
> > + _dev_attr_fan3_label.dev_attr.attr, /* 15 */
> > + _dev_attr_pwm3.dev_attr.attr,/* 16 */
> >
> > NULL
> >
> > };
> >
> > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject
> > *kobj, struct attribute *attr,
> >
> > if (index >= 11 && index <= 13 &&
> >
> > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
> >
> > return 0;
> >
> > + if (index >= 14 && index <= 16 &&
> > + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3))
> > + return 0;
> >
> > return attr->mode;
> >
> > }
> >
> > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void)
> >
> > if (err >= 0)
> >
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> >
> > + /* Third fan attributes, if fan status is OK */
> > + err = i8k_get_fan_status(2);
> > + if (err >= 0)
> > + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
> > +
> >
> > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > "dell_smm",
> >
> > NULL, i8k_groups);
> >
> > if (IS_ERR(i8k_hwmon_dev)) {
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
