Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1425538 > unrolled thread

[PATCH 0/6] dell-smm-hwmon fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
    [PATCH 5/6] hwmon: (dell-smm) Detect fan with index=2 Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
      Re: [PATCH 5/6] hwmon: (dell-smm) Detect fan with index=2 Tolga Cakir <cevelnet@gmail.com> - 2016-06-19 02:10 +0200
    [PATCH 4/6] hwmon: (dell-smm) Cache fan_type() calls and change fan detection Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
      Re: [PATCH 4/6] hwmon: (dell-smm) Cache fan_type() calls and change  fan detection Tolga Cakir <cevelnet@gmail.com> - 2016-06-19 02:10 +0200
    [PATCH 1/6] hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios version is not a number Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
      Re: [PATCH 1/6] hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when  bios version is not a number Guenter Roeck <linux@roeck-us.net> - 2016-06-18 22:10 +0200
    [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken machines Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
      Re: [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken  machines Guenter Roeck <linux@roeck-us.net> - 2016-06-18 22:10 +0200
        Re: [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken machines Pali Rohár <pali.rohar@gmail.com> - 2016-06-19 00:50 +0200
    [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial number to CAP_SYS_ADMIN by default Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
      Re: [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial  number to CAP_SYS_ADMIN by default Guenter Roeck <linux@roeck-us.net> - 2016-06-18 22:10 +0200
    [PATCH 6/6] hwmon: (dell-smm) In debug mode log duration of SMM calls Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 01:00 +0200
    Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 14:30 +0200
    Re: [PATCH 0/6] dell-smm-hwmon fixes Guenter Roeck <linux@roeck-us.net> - 2016-06-18 17:20 +0200
      Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-18 17:30 +0200
        Re: [PATCH 0/6] dell-smm-hwmon fixes Guenter Roeck <linux@roeck-us.net> - 2016-06-18 19:00 +0200
          Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-19 00:40 +0200
            Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-20 11:20 +0200
              Re: [PATCH 0/6] dell-smm-hwmon fixes Guenter Roeck <linux@roeck-us.net> - 2016-06-20 15:30 +0200
                Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-23 14:20 +0200
                  Re: [PATCH 0/6] dell-smm-hwmon fixes Guenter Roeck <linux@roeck-us.net> - 2016-06-23 16:00 +0200
          Re: [PATCH 0/6] dell-smm-hwmon fixes Pali Rohár <pali.rohar@gmail.com> - 2016-06-19 00:50 +0200
    Re: [PATCH 0/6] dell-smm-hwmon fixes Michał Kępień <kernel@kempniu.pl> - 2016-06-22 10:10 +0200

Page 1 of 2  [1] 2  Next page →

#1425538 — [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

[toc] | [next] | [standalone]

#1425539 — [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 <cevelnet@gmail.com>
Signed-off-by: Pali Rohár <pali.rohar@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 (dz@debian.org)");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@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[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,	/* 0 */
@@ -735,6 +742,9 @@ static struct attribute *i8k_attrs[] = {
 	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 11 */
 	&sensor_dev_attr_fan2_label.dev_attr.attr,	/* 12 */
 	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 13 */
+	&sensor_dev_attr_fan3_input.dev_attr.attr,	/* 14 */
+	&sensor_dev_attr_fan3_label.dev_attr.attr,	/* 15 */
+	&sensor_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

[toc] | [prev] | [next] | [standalone]

#1425880 — Re: [PATCH 5/6] hwmon: (dell-smm) Detect fan with index=2

Tested on Dell Precision M3800. It now correctly detects 2 fans for my
machine, one cpu fan and one gpu fan. The reported fan type and fan
speeds look correct. I'm getting unique fan speeds for each fan. I've
also noticed the freeze, which happends on my machine upon
I8K_SMM_GET_FAN_TYPE call, is now doubled to around 4 - 5 seconds due
to detecting 2 fans.

Tested-by: Tolga Cakir <cevelnet@gmail.com>

2016-06-18 0:54 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> 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 <cevelnet@gmail.com>
> Signed-off-by: Pali Rohár <pali.rohar@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 (dz@debian.org)");
>  MODULE_AUTHOR("Pali Rohár <pali.rohar@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[] = {
>         &sensor_dev_attr_temp1_input.dev_attr.attr,     /* 0 */
> @@ -735,6 +742,9 @@ static struct attribute *i8k_attrs[] = {
>         &sensor_dev_attr_fan2_input.dev_attr.attr,      /* 11 */
>         &sensor_dev_attr_fan2_label.dev_attr.attr,      /* 12 */
>         &sensor_dev_attr_pwm2.dev_attr.attr,            /* 13 */
> +       &sensor_dev_attr_fan3_input.dev_attr.attr,      /* 14 */
> +       &sensor_dev_attr_fan3_label.dev_attr.attr,      /* 15 */
> +       &sensor_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
>

[toc] | [prev] | [next] | [standalone]

#1425540 — [PATCH 4/6] hwmon: (dell-smm) Cache fan_type() calls and change fan detection

On more Dell machines (e.g. Dell Precision M3800) fan_type() call is too
expensive (CPU is too long in SMM mode) and cause kernel to hang. This is
bug in Dell SMM or BIOS.

This patch caches type for each fan (as it should not change) and changes
the way how fan presense is detected. First it try function fan_status()
as was before commit f989e55452c7 ("i8k: Add support for fan labels"). And
if that fails fallback to fan_type(). *_status() functions can fail in case
fan is not currently accessible (e.g. present on GPU which is currently
turned off).

Reported-by: Tolga Cakir <cevelnet@gmail.com>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
Cc: stable@vger.kernel.org # v4.0+, will need backport
---
 drivers/hwmon/dell-smm-hwmon.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 4bbc587..2ac87d5 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -238,7 +238,7 @@ static int i8k_get_fan_speed(int fan)
 /*
  * Read the fan type.
  */
-static int i8k_get_fan_type(int fan)
+static int _i8k_get_fan_type(int fan)
 {
 	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
 
@@ -249,6 +249,17 @@ static int i8k_get_fan_type(int fan)
 	return i8k_smm(&regs) ? : regs.eax & 0xff;
 }
 
+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 };
+
+	if (types[fan] == INT_MIN)
+		types[fan] = _i8k_get_fan_type(fan);
+
+	return types[fan];
+}
+
 /*
  * Read the fan nominal rpm for specific fan speed.
  */
@@ -782,13 +793,17 @@ static int __init i8k_init_hwmon(void)
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
 
-	/* First fan attributes, if fan type is OK */
-	err = i8k_get_fan_type(0);
+	/* First fan attributes, if fan status or type is OK */
+	err = i8k_get_fan_status(0);
+	if (err < 0)
+		err = i8k_get_fan_type(0);
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
 
-	/* Second fan attributes, if fan type is OK */
-	err = i8k_get_fan_type(1);
+	/* Second fan attributes, if fan status or type is OK */
+	err = i8k_get_fan_status(1);
+	if (err < 0)
+		err = i8k_get_fan_type(1);
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1425879 — Re: [PATCH 4/6] hwmon: (dell-smm) Cache fan_type() calls and change fan detection

Tested on a Dell Precision M3800. After applying this patch, the
machine doesn't freeze for a couple of seconds on module load and on
each sensors (lm_sensors) call anymore. It now just freezes once, when
running sensors for the first time, and that's it. It was not possible
to read correct RPM values using sensors before, because the fans were
running at maximum speed, when the freeze occured. I can now read RPM
values correctly and the machine doesn't annoy me with freezing up on
boot anymore.

Tested-by: Tolga Cakir <cevelnet@gmail.com>


2016-06-18 0:54 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>:
> On more Dell machines (e.g. Dell Precision M3800) fan_type() call is too
> expensive (CPU is too long in SMM mode) and cause kernel to hang. This is
> bug in Dell SMM or BIOS.
>
> This patch caches type for each fan (as it should not change) and changes
> the way how fan presense is detected. First it try function fan_status()
> as was before commit f989e55452c7 ("i8k: Add support for fan labels"). And
> if that fails fallback to fan_type(). *_status() functions can fail in case
> fan is not currently accessible (e.g. present on GPU which is currently
> turned off).
>
> Reported-by: Tolga Cakir <cevelnet@gmail.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
> Cc: stable@vger.kernel.org # v4.0+, will need backport
> ---
>  drivers/hwmon/dell-smm-hwmon.c |   25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 4bbc587..2ac87d5 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -238,7 +238,7 @@ static int i8k_get_fan_speed(int fan)
>  /*
>   * Read the fan type.
>   */
> -static int i8k_get_fan_type(int fan)
> +static int _i8k_get_fan_type(int fan)
>  {
>         struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
>
> @@ -249,6 +249,17 @@ static int i8k_get_fan_type(int fan)
>         return i8k_smm(&regs) ? : regs.eax & 0xff;
>  }
>
> +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 };
> +
> +       if (types[fan] == INT_MIN)
> +               types[fan] = _i8k_get_fan_type(fan);
> +
> +       return types[fan];
> +}
> +
>  /*
>   * Read the fan nominal rpm for specific fan speed.
>   */
> @@ -782,13 +793,17 @@ static int __init i8k_init_hwmon(void)
>         if (err >= 0)
>                 i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
>
> -       /* First fan attributes, if fan type is OK */
> -       err = i8k_get_fan_type(0);
> +       /* First fan attributes, if fan status or type is OK */
> +       err = i8k_get_fan_status(0);
> +       if (err < 0)
> +               err = i8k_get_fan_type(0);
>         if (err >= 0)
>                 i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
>
> -       /* Second fan attributes, if fan type is OK */
> -       err = i8k_get_fan_type(1);
> +       /* Second fan attributes, if fan status or type is OK */
> +       err = i8k_get_fan_status(1);
> +       if (err < 0)
> +               err = i8k_get_fan_type(1);
>         if (err >= 0)
>                 i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
>
> --
> 1.7.9.5
>

[toc] | [prev] | [next] | [standalone]

#1425541 — [PATCH 1/6] hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios version is not a number

ABI of I8K_BIOS_VERSION ioctl can return only number. But new BIOS versions
contain also other characters, which does not fit into that ABI. So in case
of non digit values return -EINVAL.

Reported-by: Mario Limonciello <Mario_Limonciello@dell.com>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/hwmon/dell-smm-hwmon.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c43318d..480b2fa 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -35,6 +35,7 @@
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/sched.h>
+#include <linux/ctype.h>
 
 #include <linux/i8k.h>
 
@@ -387,6 +388,10 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case I8K_BIOS_VERSION:
+		if (!isdigit(bios_version[0]) || !isdigit(bios_version[1]) ||
+		    !isdigit(bios_version[2]))
+			return -EINVAL;
+
 		val = (bios_version[0] << 16) |
 				(bios_version[1] << 8) | bios_version[2];
 		break;
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1425826 — Re: [PATCH 1/6] hwmon: (dell-smm) Fail in ioctl I8K_BIOS_VERSION when bios version is not a number

On 06/17/2016 03:54 PM, Pali Rohár wrote:
> ABI of I8K_BIOS_VERSION ioctl can return only number. But new BIOS versions
> contain also other characters, which does not fit into that ABI. So in case
> of non digit values return -EINVAL.
>
> Reported-by: Mario Limonciello <Mario_Limonciello@dell.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Applied.

Guenter

> ---
>   drivers/hwmon/dell-smm-hwmon.c |    5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index c43318d..480b2fa 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -35,6 +35,7 @@
>   #include <linux/uaccess.h>
>   #include <linux/io.h>
>   #include <linux/sched.h>
> +#include <linux/ctype.h>
>
>   #include <linux/i8k.h>
>
> @@ -387,6 +388,10 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
>
>   	switch (cmd) {
>   	case I8K_BIOS_VERSION:
> +		if (!isdigit(bios_version[0]) || !isdigit(bios_version[1]) ||
> +		    !isdigit(bios_version[2]))
> +			return -EINVAL;
> +
>   		val = (bios_version[0] << 16) |
>   				(bios_version[1] << 8) | bios_version[2];
>   		break;
>

[toc] | [prev] | [next] | [standalone]

#1425542 — [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken machines

Some Dell machines have especially broken SMM or BIOS which cause that once
fan_type() is called then CPU fan speed going randomly up and down. And for
fixing this behaviour reboot is required.

So this patch creates fan_type blacklist of affected Dell machines and
disallow fan_type() call on them to prevent that erratic behaviour.

Old blacklist which disabled loading driver on some machines added in
commits a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")
and 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000") were
moved to FAN_TYPE blacklist.

Reported-by: Jan C Peters <jcpeters89@gmail.com>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
Cc: stable@vger.kernel.org # v4.0+, will need backport
---
 drivers/hwmon/dell-smm-hwmon.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c8bd3fdd..4bbc587 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -73,6 +73,7 @@ static u32 i8k_hwmon_flags;
 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;
 
 #define I8K_HWMON_HAVE_TEMP1	(1 << 0)
 #define I8K_HWMON_HAVE_TEMP2	(1 << 1)
@@ -241,6 +242,9 @@ static int i8k_get_fan_type(int fan)
 {
 	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
 
+	if (disallow_fan_type_call)
+		return -EINVAL;
+
 	regs.ebx = fan & 0xff;
 	return i8k_smm(&regs) ? : regs.eax & 0xff;
 }
@@ -726,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
 static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
 			      int index)
 {
+	if (disallow_fan_type_call &&
+	    (index == 9 || index == 12))
+		return 0;
 	if (index >= 0 && index <= 1 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1))
 		return 0;
@@ -937,12 +944,14 @@ static struct dmi_system_id i8k_dmi_table[] __initdata = {
 
 MODULE_DEVICE_TABLE(dmi, i8k_dmi_table);
 
-static struct dmi_system_id i8k_blacklist_dmi_table[] __initdata = {
+/*
+ * On some machines once I8K_SMM_GET_FAN_TYPE is issued then CPU fan speed
+ * randomly going up and down due to bug in Dell SMM or BIOS. Here is blacklist
+ * of affected Dell machines for which we disallow I8K_SMM_GET_FAN_TYPE call.
+ * See bug: https://bugzilla.kernel.org/show_bug.cgi?id=100121
+ */
+static struct dmi_system_id i8k_blacklist_fan_type_dmi_table[] __initdata = {
 	{
-		/*
-		 * CPU fan speed going up and down on Dell Studio XPS 8000
-		 * for unknown reasons.
-		 */
 		.ident = "Dell Studio XPS 8000",
 		.matches = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
@@ -950,16 +959,19 @@ static struct dmi_system_id i8k_blacklist_dmi_table[] __initdata = {
 		},
 	},
 	{
-		/*
-		 * CPU fan speed going up and down on Dell Studio XPS 8100
-		 * for unknown reasons.
-		 */
 		.ident = "Dell Studio XPS 8100",
 		.matches = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Studio XPS 8100"),
 		},
 	},
+	{
+		.ident = "Dell Inspiron 580",
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Inspiron 580 "),
+		},
+	},
 	{ }
 };
 
@@ -974,8 +986,7 @@ static int __init i8k_probe(void)
 	/*
 	 * Get DMI information
 	 */
-	if (!dmi_check_system(i8k_dmi_table) ||
-	    dmi_check_system(i8k_blacklist_dmi_table)) {
+	if (!dmi_check_system(i8k_dmi_table)) {
 		if (!ignore_dmi && !force)
 			return -ENODEV;
 
@@ -986,6 +997,9 @@ 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;
+
 	strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
 		sizeof(bios_version));
 	strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1425828 — Re: [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken machines

On 06/17/2016 03:54 PM, Pali Rohár wrote:
> Some Dell machines have especially broken SMM or BIOS which cause that once
> fan_type() is called then CPU fan speed going randomly up and down. And for
> fixing this behaviour reboot is required.
>
> So this patch creates fan_type blacklist of affected Dell machines and
> disallow fan_type() call on them to prevent that erratic behaviour.
>
> Old blacklist which disabled loading driver on some machines added in
> commits a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8100")
> and 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell Studio XPS 8000") were
> moved to FAN_TYPE blacklist.
>
> Reported-by: Jan C Peters <jcpeters89@gmail.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
> Cc: stable@vger.kernel.org # v4.0+, will need backport

Applied.

It seemed worthwhile to apply this patch now, without waiting for further test
feedback, since it at least reduces the impact of the observed problems.
If follow-up changes are necessary, those will have to be applied in separate
patches.

Guenter

> ---
>   drivers/hwmon/dell-smm-hwmon.c |   36 +++++++++++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index c8bd3fdd..4bbc587 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -73,6 +73,7 @@ static u32 i8k_hwmon_flags;
>   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;
>
>   #define I8K_HWMON_HAVE_TEMP1	(1 << 0)
>   #define I8K_HWMON_HAVE_TEMP2	(1 << 1)
> @@ -241,6 +242,9 @@ static int i8k_get_fan_type(int fan)
>   {
>   	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
>
> +	if (disallow_fan_type_call)
> +		return -EINVAL;
> +
>   	regs.ebx = fan & 0xff;
>   	return i8k_smm(&regs) ? : regs.eax & 0xff;
>   }
> @@ -726,6 +730,9 @@ static struct attribute *i8k_attrs[] = {
>   static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
>   			      int index)
>   {
> +	if (disallow_fan_type_call &&
> +	    (index == 9 || index == 12))
> +		return 0;
>   	if (index >= 0 && index <= 1 &&
>   	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1))
>   		return 0;
> @@ -937,12 +944,14 @@ static struct dmi_system_id i8k_dmi_table[] __initdata = {
>
>   MODULE_DEVICE_TABLE(dmi, i8k_dmi_table);
>
> -static struct dmi_system_id i8k_blacklist_dmi_table[] __initdata = {
> +/*
> + * On some machines once I8K_SMM_GET_FAN_TYPE is issued then CPU fan speed
> + * randomly going up and down due to bug in Dell SMM or BIOS. Here is blacklist
> + * of affected Dell machines for which we disallow I8K_SMM_GET_FAN_TYPE call.
> + * See bug: https://bugzilla.kernel.org/show_bug.cgi?id=100121
> + */
> +static struct dmi_system_id i8k_blacklist_fan_type_dmi_table[] __initdata = {
>   	{
> -		/*
> -		 * CPU fan speed going up and down on Dell Studio XPS 8000
> -		 * for unknown reasons.
> -		 */
>   		.ident = "Dell Studio XPS 8000",
>   		.matches = {
>   			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> @@ -950,16 +959,19 @@ static struct dmi_system_id i8k_blacklist_dmi_table[] __initdata = {
>   		},
>   	},
>   	{
> -		/*
> -		 * CPU fan speed going up and down on Dell Studio XPS 8100
> -		 * for unknown reasons.
> -		 */
>   		.ident = "Dell Studio XPS 8100",
>   		.matches = {
>   			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>   			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Studio XPS 8100"),
>   		},
>   	},
> +	{
> +		.ident = "Dell Inspiron 580",
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Inspiron 580 "),
> +		},
> +	},
>   	{ }
>   };
>
> @@ -974,8 +986,7 @@ static int __init i8k_probe(void)
>   	/*
>   	 * Get DMI information
>   	 */
> -	if (!dmi_check_system(i8k_dmi_table) ||
> -	    dmi_check_system(i8k_blacklist_dmi_table)) {
> +	if (!dmi_check_system(i8k_dmi_table)) {
>   		if (!ignore_dmi && !force)
>   			return -ENODEV;
>
> @@ -986,6 +997,9 @@ 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;
> +
>   	strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
>   		sizeof(bios_version));
>   	strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
>

[toc] | [prev] | [next] | [standalone]

#1425871 — Re: [PATCH 3/6] hwmon: (dell-smm) Disallow fan_type() calls on broken machines

[Multipart message — attachments visible in raw view] — view raw

On Saturday 18 June 2016 22:08:17 Guenter Roeck wrote:
> On 06/17/2016 03:54 PM, Pali Rohár wrote:
> > Some Dell machines have especially broken SMM or BIOS which cause
> > that once fan_type() is called then CPU fan speed going randomly
> > up and down. And for fixing this behaviour reboot is required.
> > 
> > So this patch creates fan_type blacklist of affected Dell machines
> > and disallow fan_type() call on them to prevent that erratic
> > behaviour.
> > 
> > Old blacklist which disabled loading driver on some machines added
> > in commits a4b45b25f18d ("hwmon: (dell-smm) Blacklist Dell Studio
> > XPS 8100") and 6220f4ebd7b4 ("hwmon: (dell-smm) Blacklist Dell
> > Studio XPS 8000") were moved to FAN_TYPE blacklist.
> > 
> > Reported-by: Jan C Peters <jcpeters89@gmail.com>
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
> > Cc: stable@vger.kernel.org # v4.0+, will need backport
> 
> Applied.
> 
> It seemed worthwhile to apply this patch now, without waiting for
> further test feedback, since it at least reduces the impact of the
> observed problems. If follow-up changes are necessary, those will
> have to be applied in separate patches.
> 
> Guenter

Ok. Meanwhile patch was tested on Inspiron 580, so maybe it could be 
appended into commit message.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1425543 — [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial number to CAP_SYS_ADMIN by default

For security reasons ordinary user must not be able to control fan speed
via /proc/i8k by default. Some malicious software running under "nobody"
user could be able to turn fan off and cause HW problems. So this patch
changes default value of "restricted" parameter to 1.

Also restrict reading of DMI_PRODUCT_SERIAL from /proc/i8k via "restricted"
parameter. It is because non root user cannot read DMI_PRODUCT_SERIAL from
sysfs file /sys/class/dmi/id/product_serial.

Old non secure behaviour of file /proc/i8k can be achieved by loading this
module with "restricted" parameter set to 0.

Note that this patch has effects only for kernels compiled with CONFIG_I8K
and only for file /proc/i8k. Hwmon interface provided by this driver was
not changed and root access for setting fan speed was needed also before.

Reported-by: Mario Limonciello <Mario_Limonciello@dell.com>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Cc: stable@vger.kernel.org # will need backport
---
 drivers/hwmon/dell-smm-hwmon.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 480b2fa..c8bd3fdd 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -67,6 +67,7 @@
 
 static DEFINE_MUTEX(i8k_mutex);
 static char bios_version[4];
+static char bios_machineid[16];
 static struct device *i8k_hwmon_dev;
 static u32 i8k_hwmon_flags;
 static uint i8k_fan_mult = I8K_FAN_MULT;
@@ -95,13 +96,13 @@ module_param(ignore_dmi, bool, 0);
 MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
 
 #if IS_ENABLED(CONFIG_I8K)
-static bool restricted;
+static bool restricted = true;
 module_param(restricted, bool, 0);
-MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
+MODULE_PARM_DESC(restricted, "Restrict fan control and serial number to CAP_SYS_ADMIN (default: 1)");
 
 static bool power_status;
 module_param(power_status, bool, 0600);
-MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
+MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k (default: 0)");
 #endif
 
 static uint fan_mult;
@@ -397,9 +398,11 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
 		break;
 
 	case I8K_MACHINE_ID:
-		memset(buff, 0, 16);
-		strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
-			sizeof(buff));
+		if (restricted && !capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+		memset(buff, 0, sizeof(buff));
+		strlcpy(buff, bios_machineid, sizeof(buff));
 		break;
 
 	case I8K_FN_STATUS:
@@ -516,7 +519,7 @@ static int i8k_proc_show(struct seq_file *seq, void *offset)
 	seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n",
 		   I8K_PROC_FMT,
 		   bios_version,
-		   i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
+		   (restricted && !capable(CAP_SYS_ADMIN)) ? "-1" : bios_machineid,
 		   cpu_temp,
 		   left_fan, right_fan, left_speed, right_speed,
 		   ac_power, fn_key);
@@ -985,6 +988,8 @@ static int __init i8k_probe(void)
 
 	strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
 		sizeof(bios_version));
+	strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
+		sizeof(bios_machineid));
 
 	/*
 	 * Get SMM Dell signature
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1425831 — Re: [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial number to CAP_SYS_ADMIN by default

On 06/17/2016 03:54 PM, Pali Rohár wrote:
> For security reasons ordinary user must not be able to control fan speed
> via /proc/i8k by default. Some malicious software running under "nobody"
> user could be able to turn fan off and cause HW problems. So this patch
> changes default value of "restricted" parameter to 1.
>
> Also restrict reading of DMI_PRODUCT_SERIAL from /proc/i8k via "restricted"
> parameter. It is because non root user cannot read DMI_PRODUCT_SERIAL from
> sysfs file /sys/class/dmi/id/product_serial.
>
> Old non secure behaviour of file /proc/i8k can be achieved by loading this
> module with "restricted" parameter set to 0.
>
> Note that this patch has effects only for kernels compiled with CONFIG_I8K
> and only for file /proc/i8k. Hwmon interface provided by this driver was
> not changed and root access for setting fan speed was needed also before.
>
> Reported-by: Mario Limonciello <Mario_Limonciello@dell.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Cc: stable@vger.kernel.org # will need backport

Applied.

Guenter

> ---
>   drivers/hwmon/dell-smm-hwmon.c |   19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 480b2fa..c8bd3fdd 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -67,6 +67,7 @@
>
>   static DEFINE_MUTEX(i8k_mutex);
>   static char bios_version[4];
> +static char bios_machineid[16];
>   static struct device *i8k_hwmon_dev;
>   static u32 i8k_hwmon_flags;
>   static uint i8k_fan_mult = I8K_FAN_MULT;
> @@ -95,13 +96,13 @@ module_param(ignore_dmi, bool, 0);
>   MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
>
>   #if IS_ENABLED(CONFIG_I8K)
> -static bool restricted;
> +static bool restricted = true;
>   module_param(restricted, bool, 0);
> -MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
> +MODULE_PARM_DESC(restricted, "Restrict fan control and serial number to CAP_SYS_ADMIN (default: 1)");
>
>   static bool power_status;
>   module_param(power_status, bool, 0600);
> -MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
> +MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k (default: 0)");
>   #endif
>
>   static uint fan_mult;
> @@ -397,9 +398,11 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
>   		break;
>
>   	case I8K_MACHINE_ID:
> -		memset(buff, 0, 16);
> -		strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> -			sizeof(buff));
> +		if (restricted && !capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +		memset(buff, 0, sizeof(buff));
> +		strlcpy(buff, bios_machineid, sizeof(buff));
>   		break;
>
>   	case I8K_FN_STATUS:
> @@ -516,7 +519,7 @@ static int i8k_proc_show(struct seq_file *seq, void *offset)
>   	seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n",
>   		   I8K_PROC_FMT,
>   		   bios_version,
> -		   i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> +		   (restricted && !capable(CAP_SYS_ADMIN)) ? "-1" : bios_machineid,
>   		   cpu_temp,
>   		   left_fan, right_fan, left_speed, right_speed,
>   		   ac_power, fn_key);
> @@ -985,6 +988,8 @@ static int __init i8k_probe(void)
>
>   	strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
>   		sizeof(bios_version));
> +	strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> +		sizeof(bios_machineid));
>
>   	/*
>   	 * Get SMM Dell signature
>

[toc] | [prev] | [next] | [standalone]

#1425544 — [PATCH 6/6] hwmon: (dell-smm) In debug mode log duration of SMM calls

This allow us to debug how long take each SMM call and how long is system
frozen in SMM handler.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/hwmon/dell-smm-hwmon.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 571d498..acf9c03 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -140,6 +140,14 @@ static int i8k_smm(struct smm_regs *regs)
 	int eax = regs->eax;
 	cpumask_var_t old_mask;
 
+#ifdef DEBUG
+	int ebx = regs->ebx;
+	unsigned long duration;
+	ktime_t calltime, delta, rettime;
+
+	calltime = ktime_get();
+#endif
+
 	/* SMM requires CPU 0 */
 	if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
 		return -ENOMEM;
@@ -211,6 +219,15 @@ static int i8k_smm(struct smm_regs *regs)
 out:
 	set_cpus_allowed_ptr(current, old_mask);
 	free_cpumask_var(old_mask);
+
+#ifdef DEBUG
+	rettime = ktime_get();
+	delta = ktime_sub(rettime, calltime);
+	duration = ktime_to_ns(delta) >> 10;
+	pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x  (took %7lu usecs)\n", eax, ebx,
+		(rc ? 0xffff : regs->eax & 0xffff), duration);
+#endif
+
 	return rc;
 }
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1425710

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.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1425745

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

[toc] | [prev] | [next] | [standalone]

#1425748

[Multipart message — attachments visible in raw view] — view raw

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.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1425798

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

[toc] | [prev] | [next] | [standalone]

#1425867

[Multipart message — attachments visible in raw view] — view raw

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.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1426371

On Sunday 19 June 2016 00:39:47 Pali Rohár wrote:
> 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.
> 

Guenter, now you can apply whole series + add all tested-by lines from
email threads. Looks like it is OK now.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1426600

On 06/20/2016 02:12 AM, Pali Rohár wrote:
> On Sunday 19 June 2016 00:39:47 Pali Rohár wrote:
>> 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.
>>
>
> Guenter, now you can apply whole series + add all tested-by lines from
> email threads. Looks like it is OK now.
>
First three patches applied and sent to Linus last night.

Remaining three patches applied to -next.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web