[PATCH net-next 1/5] net/neighbour: constify ctl_table arguments of utility function

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

Signed-off-by: Thomas Weißschuh 
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 45fd88405b6b..277751375b0a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -3578,7 +3578,7 @@ static void neigh_copy_dflt_parms(struct net *net, struct 
neigh_parms *p,
rcu_read_unlock();
 }
 
-static void neigh_proc_update(struct ctl_table *ctl, int write)
+static void neigh_proc_update(const struct ctl_table *ctl, int write)
 {
struct net_device *dev = ctl->extra1;
struct neigh_parms *p = ctl->extra2;

-- 
2.45.1

[PATCH net-next 5/5] ipvs: constify ctl_table arguments of utility functions

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

Signed-off-by: Thomas Weißschuh 
---
 net/netfilter/ipvs/ip_vs_ctl.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index b6d0dcf3a5c3..78a1cc72dc38 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1924,7 +1924,8 @@ proc_do_sync_ports(struct ctl_table *table, int write,
return rc;
 }
 
-static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer)
+static int ipvs_proc_est_cpumask_set(const struct ctl_table *table,
+void *buffer)
 {
struct netns_ipvs *ipvs = table->extra2;
cpumask_var_t *valp = table->data;
@@ -1962,8 +1963,8 @@ static int ipvs_proc_est_cpumask_set(struct ctl_table 
*table, void *buffer)
return ret;
 }
 
-static int ipvs_proc_est_cpumask_get(struct ctl_table *table, void *buffer,
-size_t size)
+static int ipvs_proc_est_cpumask_get(const struct ctl_table *table,
+void *buffer, size_t size)
 {
struct netns_ipvs *ipvs = table->extra2;
cpumask_var_t *valp = table->data;

-- 
2.45.1

[PATCH net-next 0/5] net: constify ctl_table arguments of utility functions

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

This patch(set) is meant to be applied through your subsystem tree.
Or at your preference through the sysctl tree.

Motivation
==

Moving structures containing function pointers into unmodifiable .rodata
prevents attackers or bugs from corrupting and diverting those pointers.

Also the "struct ctl_table" exposed by the sysctl core were never meant
to be mutated by users.

For this goal changes to both the sysctl core and "const" qualifiers for
various sysctl APIs are necessary.

Full Process


* Drop ctl_table modifications from the sysctl core ([0], in mainline)
* Constify arguments to ctl_table_root::{set_ownership,permissions}
  ([1], in mainline)
* Migrate users of "ctl_table_header::ctl_table_arg" to "const".
  (in mainline)
* Afterwards convert "ctl_table_header::ctl_table_arg" itself to const.
  (in mainline)
* Prepare helpers used to implement proc_handlers throughout the tree to
  use "const struct ctl_table *". ([2], in progress, this patch)
* Afterwards switch over all proc_handlers callbacks to use
  "const struct ctl_table *" in one commit. ([2], in progress)
  Only custom handlers will be affected, the big commit avoids a
  disruptive and messy transition phase.
* Switch over the internals of the sysctl core to "const struct ctl_table *" 
(to be done)
* Switch include/linux/sysctl.h to "const struct ctl_table *" (to be done)
* Transition instances of "struct ctl_table" through the tree to const (to be 
done)

A work-in-progress view containing all the outlined changes can be found at
https://git.sr.ht/~t-8ch/linux sysctl-constfy

[0] 
https://lore.kernel.org/lkml/20240322-sysctl-empty-dir-v2-0-e559cf8ec...@weissschuh.net/
[1] 
https://lore.kernel.org/lkml/20240315-sysctl-const-ownership-v3-0-b86680eae...@weissschuh.net/
[2] 
https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb83...@weissschuh.net/

---
Thomas Weißschuh (5):
  net/neighbour: constify ctl_table arguments of utility function
  net/ipv4/sysctl: constify ctl_table arguments of utility functions
  net/ipv6/addrconf: constify ctl_table arguments of utility functions
  net/ipv6/ndisc: constify ctl_table arguments of utility function
  ipvs: constify ctl_table arguments of utility functions

 net/core/neighbour.c   | 2 +-
 net/ipv4/sysctl_net_ipv4.c | 6 --
 net/ipv6/addrconf.c| 8 
 net/ipv6/ndisc.c   | 2 +-
 net/netfilter/ipvs/ip_vs_ctl.c | 7 ---
 5 files changed, 14 insertions(+), 11 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240523-sysctl-const-handler-net-824d4ad5a15a

Best regards,
-- 
Thomas Weißschuh

[PATCH net-next 4/5] net/ipv6/ndisc: constify ctl_table arguments of utility function

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

Signed-off-by: Thomas Weißschuh 
---
 net/ipv6/ndisc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index d914b23256ce..254b192c5705 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1936,7 +1936,7 @@ static struct notifier_block ndisc_netdev_notifier = {
 };
 
 #ifdef CONFIG_SYSCTL
-static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
+static void ndisc_warn_deprecated_sysctl(const struct ctl_table *ctl,
 const char *func, const char *dev_name)
 {
static char warncomm[TASK_COMM_LEN];

-- 
2.45.1

[PATCH net-next 3/5] net/ipv6/addrconf: constify ctl_table arguments of utility functions

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

Signed-off-by: Thomas Weißschuh 
---
 net/ipv6/addrconf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5c424a0e7232..1e69756d53d9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -863,7 +863,7 @@ static void addrconf_forward_change(struct net *net, __s32 
newf)
}
 }
 
-static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int newf)
+static int addrconf_fixup_forwarding(const struct ctl_table *table, int *p, 
int newf)
 {
struct net *net;
int old;
@@ -931,7 +931,7 @@ static void addrconf_linkdown_change(struct net *net, __s32 
newf)
}
 }
 
-static int addrconf_fixup_linkdown(struct ctl_table *table, int *p, int newf)
+static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int 
newf)
 {
struct net *net;
int old;
@@ -6378,7 +6378,7 @@ static void addrconf_disable_change(struct net *net, 
__s32 newf)
}
 }
 
-static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int newf)
+static int addrconf_disable_ipv6(const struct ctl_table *table, int *p, int 
newf)
 {
struct net *net = (struct net *)table->extra2;
int old;
@@ -6669,7 +6669,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev, 
int val)
 }
 
 static
-int addrconf_disable_policy(struct ctl_table *ctl, int *valp, int val)
+int addrconf_disable_policy(const struct ctl_table *ctl, int *valp, int val)
 {
struct net *net = (struct net *)ctl->extra2;
struct inet6_dev *idev;

-- 
2.45.1

[PATCH net-next 2/5] net/ipv4/sysctl: constify ctl_table arguments of utility functions

2024-05-27 Thread Thomas Weißschuh 
The sysctl core is preparing to only expose instances of
struct ctl_table as "const".
This will also affect the ctl_table argument of sysctl handlers.

As the function prototype of all sysctl handlers throughout the tree
needs to stay consistent that change will be done in one commit.

To reduce the size of that final commit, switch utility functions which
are not bound by "typedef proc_handler" to "const struct ctl_table".

No functional change.

Signed-off-by: Thomas Weißschuh 
---
 net/ipv4/sysctl_net_ipv4.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 162a0a3b6ba5..d7892f34a15b 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -130,7 +130,8 @@ static int ipv4_privileged_ports(struct ctl_table *table, 
int write,
return ret;
 }
 
-static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t 
*low, kgid_t *high)
+static void inet_get_ping_group_range_table(const struct ctl_table *table,
+   kgid_t *low, kgid_t *high)
 {
kgid_t *data = table->data;
struct net *net =
@@ -145,7 +146,8 @@ static void inet_get_ping_group_range_table(struct 
ctl_table *table, kgid_t *low
 }
 
 /* Update system visible IP port range */
-static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t 
high)
+static void set_ping_group_range(const struct ctl_table *table,
+kgid_t low, kgid_t high)
 {
kgid_t *data = table->data;
struct net *net =

-- 
2.45.1

[PATCH] selftests/nolibc: libc-test: avoid -Wstringop-overflow warnings

2023-09-10 Thread Thomas Weißschuh 
Newer versions of glibc annotate the poll() function with
__attribute__(access) which triggers a compiler warning inside the
testcase poll_fault.
Avoid this by using a plain NULL which is enough for the testcase.
To avoid potential future warnings also adapt the other EFAULT
testcases, except select_fault as NULL is a valid value for its
argument.

nolibc-test.c: In function ‘run_syscall’:
nolibc-test.c:338:62: warning: ‘poll’ writing 8 bytes into a region of size 0 
overflows the destination [-Wstringop-overflow=]
  338 | do { if (!(cond)) result(llen, SKIPPED); else ret += 
expect_syserr2(expr, expret, experr1, experr2, llen); } while (0)
  |  
^~~~
nolibc-test.c:341:9: note: in expansion of macro ‘EXPECT_SYSER2’
  341 | EXPECT_SYSER2(cond, expr, expret, experr, 0)
  | ^
nolibc-test.c:905:47: note: in expansion of macro ‘EXPECT_SYSER’
  905 | CASE_TEST(poll_fault);EXPECT_SYSER(1, 
poll((void *)1, 1, 0), -1, EFAULT); break;
  |   ^~~~
cc1: note: destination object is likely at address zero
In file included from /usr/include/poll.h:1,
 from nolibc-test.c:33:
/usr/include/sys/poll.h:54:12: note: in a call to function ‘poll’ declared with 
attribute ‘access (write_only, 1, 2)’
   54 | extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
  |^~~~

Signed-off-by: Thomas Weißschuh 
---
 tools/testing/selftests/nolibc/nolibc-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c 
b/tools/testing/selftests/nolibc/nolibc-test.c
index e2b70641a1e7..a0478f8eaee8 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -895,14 +895,14 @@ int run_syscall(int min, int max)
CASE_TEST(lseek_0);   EXPECT_SYSER(1, lseek(0, 0, 
SEEK_SET), -1, ESPIPE); break;
CASE_TEST(mkdir_root);EXPECT_SYSER(1, mkdir("/", 0755), 
-1, EEXIST); break;
CASE_TEST(mmap_bad);  EXPECT_PTRER(1, mmap(NULL, 0, 
PROT_READ, MAP_PRIVATE, 0, 0), MAP_FAILED, EINVAL); break;
-   CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap((void *)1, 
0), -1, EINVAL); break;
+   CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap(NULL, 0), 
-1, EINVAL); break;
CASE_TEST(mmap_munmap_good);  EXPECT_SYSZR(1, 
test_mmap_munmap()); break;
CASE_TEST(open_tty);  EXPECT_SYSNE(1, tmp = 
open("/dev/null", 0), -1); if (tmp != -1) close(tmp); break;
CASE_TEST(open_blah); EXPECT_SYSER(1, tmp = 
open("/proc/self/blah", 0), -1, ENOENT); if (tmp != -1) close(tmp); break;
CASE_TEST(pipe);  EXPECT_SYSZR(1, test_pipe()); 
break;
CASE_TEST(poll_null); EXPECT_SYSZR(1, poll(NULL, 0, 
0)); break;
CASE_TEST(poll_stdout);   EXPECT_SYSNE(1, ({ struct pollfd 
fds = { 1, POLLOUT, 0}; poll(, 1, 0); }), -1); break;
-   CASE_TEST(poll_fault);EXPECT_SYSER(1, poll((void *)1, 
1, 0), -1, EFAULT); break;
+   CASE_TEST(poll_fault);EXPECT_SYSER(1, poll(NULL, 1, 0), 
-1, EFAULT); break;
CASE_TEST(prctl); EXPECT_SYSER(1, 
prctl(PR_SET_NAME, (unsigned long)NULL, 0, 0, 0), -1, EFAULT); break;
CASE_TEST(read_badf); EXPECT_SYSER(1, read(-1, , 
1), -1, EBADF); break;
CASE_TEST(rmdir_blah);EXPECT_SYSER(1, rmdir("/blah"), 
-1, ENOENT); break;
@@ -911,7 +911,7 @@ int run_syscall(int min, int max)
CASE_TEST(select_stdout); EXPECT_SYSNE(1, ({ fd_set fds; 
FD_ZERO(); FD_SET(1, ); select(2, NULL, , NULL, NULL); }), -1); 
break;
CASE_TEST(select_fault);  EXPECT_SYSER(1, select(1, (void 
*)1, NULL, NULL, 0), -1, EFAULT); break;
CASE_TEST(stat_blah); EXPECT_SYSER(1, 
stat("/proc/self/blah", _buf), -1, ENOENT); break;
-   CASE_TEST(stat_fault);EXPECT_SYSER(1, stat((void *)1, 
_buf), -1, EFAULT); break;
+   CASE_TEST(stat_fault);EXPECT_SYSER(1, stat(NULL, 
_buf), -1, EFAULT); break;
CASE_TEST(stat_timestamps);   EXPECT_SYSZR(1, 
test_stat_timestamps()); break;
CASE_TEST(symlink_root);  EXPECT_SYSER(1, symlink("/", 
"/"), -1, EEXIST); break;
CASE_TEST(unlink_root);   EXPECT_SYSER(1, unlink("/"), -1, 
EISDIR); break;

---
base-commit: f7a6e4791e3d685eddca29b5d16d183ee0407caa
change-id: 20230910-nolibc-poll-fault-4152a6836ef8

Best regards,
-- 
Thomas Weißschuh

[PATCH v5] platform/x86: add Gigabyte WMI temperature driver

2021-04-12 Thread Thomas Weißschuh 
Tested with
* X570 I Aorus Pro Wifi (rev 1.0)
* B550M DS3H
* B550 Gaming X V2 (rev.1.x)
* Z390 I AORUS PRO WIFI (rev. 1.0)

Those mainboards contain an ITE chips for management and
monitoring.

They could also be handled by drivers/hwmon/i87.c.
But the SuperIO range used by i87 is already claimed and used by the
firmware.

The following warning is printed at boot:

kernel: ACPI Warning: SystemIO range 0x0A45-0x0A46 
conflicts with OpRegion 0x0A45-0x0A46 (\GSA1.SIO1) 
(20200528/utaddress-204)
kernel: ACPI: This conflict may cause random problems and system instability
kernel: ACPI: If an ACPI driver is available for this device, you should use it 
instead of the native driver

This driver implements such an ACPI driver.

Unfortunately not all sensor registers are handled by the firmware and even
less are exposed via WMI.

Signed-off-by: Thomas Weißschuh 
Reviewed-by: Guenter Roeck 

---

Changes since v4:
* Style
* Wording
* Alignment of email addresses
---
 MAINTAINERS |   6 +
 drivers/platform/x86/Kconfig|  11 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/gigabyte-wmi.c | 195 
 4 files changed, 213 insertions(+)
 create mode 100644 drivers/platform/x86/gigabyte-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..7fb5e2ba489b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7543,6 +7543,12 @@ F:   Documentation/filesystems/gfs2*
 F: fs/gfs2/
 F: include/uapi/linux/gfs2_ondisk.h
 
+GIGABYTE WMI DRIVER
+M: Thomas Weißschuh 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/gigabyte-wmi.c
+
 GNSS SUBSYSTEM
 M: Johan Hovold 
 S: Maintained
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad4e630e73e2..96622a2106f7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,17 @@ config XIAOMI_WMI
  To compile this driver as a module, choose M here: the module will
  be called xiaomi-wmi.
 
+config GIGABYTE_WMI
+   tristate "Gigabyte WMI temperature driver"
+   depends on ACPI_WMI
+   depends on HWMON
+   help
+ Say Y here if you want to support WMI-based temperature reporting on
+ Gigabyte mainboards.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gigabyte-wmi.
+
 config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 60d554073749..1621ebfd04fd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)   += 
intel-wmi-thunderbolt.o
 obj-$(CONFIG_MXM_WMI)  += mxm-wmi.o
 obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)   += xiaomi-wmi.o
+obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
diff --git a/drivers/platform/x86/gigabyte-wmi.c 
b/drivers/platform/x86/gigabyte-wmi.c
new file mode 100644
index ..bb1b0b205fa7
--- /dev/null
+++ b/drivers/platform/x86/gigabyte-wmi.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (C) 2021 Thomas Weißschuh 
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GIGABYTE_WMI_GUID  "DEADBEEF-2001--00A0-C9062910"
+#define NUM_TEMPERATURE_SENSORS6
+
+static bool force_load;
+module_param(force_load, bool, 0444);
+MODULE_PARM_DESC(force_load, "Force loading on unknown platform");
+
+static u8 usable_sensors_mask;
+
+enum gigabyte_wmi_commandtype {
+   GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
+   GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
+   GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
+   GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
+   GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
+};
+
+struct gigabyte_wmi_args {
+   u32 arg1;
+};
+
+static int gigabyte_wmi_perform_query(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, struct 
acpi_buffer *out)
+{
+   const struct acpi_buffer in = {
+   .length = sizeof(*args),
+   .pointer = args,
+   };
+
+   acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, , out);
+
+   if (ACPI_FAILURE(ret))
+   return -EIO;
+
+   return 0;
+}
+
+static int gigabyte_wmi_query_integer(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, u64 *res)
+{

[PATCH v4] platform/x86: add Gigabyte WMI temperature driver

2021-04-10 Thread Thomas Weißschuh 
Changes since v3:
* Completely hide unusable sensors
* Expose force_load parameter read-only via sysfs
* Naming
* Style cleanups

-- >8 --

Tested with
* X570 I Aorus Pro Wifi (rev 1.0)
* B550M DS3H
* B550 Gaming X V2 (rev.1.x)
* Z390 I AORUS PRO WIFI (rev. 1.0)

The mainboard contains an ITE IT8688E chip for management.
This chips is also handled by drivers/hwmon/i87.c but as it is also used
by the firmware itself it needs an ACPI driver.

Unfortunately not all sensor registers are handled by the firmware and even
less are exposed via WMI.

Signed-off-by: Thomas Weißschuh 
---
 MAINTAINERS |   6 +
 drivers/platform/x86/Kconfig|  11 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/gigabyte-wmi.c | 195 
 4 files changed, 213 insertions(+)
 create mode 100644 drivers/platform/x86/gigabyte-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..9c10cfc00fe8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7543,6 +7543,12 @@ F:   Documentation/filesystems/gfs2*
 F: fs/gfs2/
 F: include/uapi/linux/gfs2_ondisk.h
 
+GIGABYTE WMI DRIVER
+M:     Thomas Weißschuh 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/gigabyte-wmi.c
+
 GNSS SUBSYSTEM
 M: Johan Hovold 
 S: Maintained
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad4e630e73e2..96622a2106f7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,17 @@ config XIAOMI_WMI
  To compile this driver as a module, choose M here: the module will
  be called xiaomi-wmi.
 
+config GIGABYTE_WMI
+   tristate "Gigabyte WMI temperature driver"
+   depends on ACPI_WMI
+   depends on HWMON
+   help
+ Say Y here if you want to support WMI-based temperature reporting on
+ Gigabyte mainboards.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gigabyte-wmi.
+
 config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 60d554073749..1621ebfd04fd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)   += 
intel-wmi-thunderbolt.o
 obj-$(CONFIG_MXM_WMI)  += mxm-wmi.o
 obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)   += xiaomi-wmi.o
+obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
diff --git a/drivers/platform/x86/gigabyte-wmi.c 
b/drivers/platform/x86/gigabyte-wmi.c
new file mode 100644
index ..c17e51fcf000
--- /dev/null
+++ b/drivers/platform/x86/gigabyte-wmi.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (C) 2021 Thomas Weißschuh 
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GIGABYTE_WMI_GUID  "DEADBEEF-2001--00A0-C9062910"
+#define NUM_TEMPERATURE_SENSORS6
+
+static bool force_load;
+module_param(force_load, bool, 0444);
+MODULE_PARM_DESC(force_load, "Force loading on unknown platform");
+
+static u8 usable_sensors_mask;
+
+enum gigabyte_wmi_commandtype {
+   GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
+   GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
+   GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
+   GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
+   GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
+};
+
+struct gigabyte_wmi_args {
+   u32 arg1;
+};
+
+static int gigabyte_wmi_perform_query(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, struct 
acpi_buffer *out)
+{
+   const struct acpi_buffer in = {
+   .length = sizeof(*args),
+   .pointer = args,
+   };
+
+   acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, , out);
+
+   if ACPI_FAILURE(ret)
+   return -EIO;
+
+   return 0;
+}
+
+static int gigabyte_wmi_query_integer(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, u64 *res)
+{
+   union acpi_object *obj;
+   struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
+   int ret;
+
+   ret = gigabyte_wmi_perform_query(wdev, command, args, );
+   if (ret)
+   return ret;
+   obj = result.pointer;
+   if (obj && obj->type == ACPI_TYPE_INTEGER)
+   *res = obj->integer.value;
+   else
+   ret = -EIO;
+   kfree(result.poi

[PATCH v3] platform/x86: add Gigabyte WMI temperature driver

2021-04-10 Thread Thomas Weißschuh 
Changes since v1:
* Incorporate feedback from Barnabás Pőcze
  * Use a WMI driver instead of a platform driver
  * Let the kernel manage the driver lifecycle
  * Fix errno/ACPI error confusion
  * Fix resource cleanup
  * Document reason for integer casting

Changes since v2:
* Style cleanups
* Test for usability during probing
* DMI-based whitelist
* CC hwmon maintainers

-- >8 --

Tested with a X570 I Aorus Pro Wifi.
The mainboard contains an ITE IT8688E chip for management.
This chips is also handled by drivers/hwmon/i87.c but as it is also used
by the firmware itself it needs an ACPI driver.

Unfortunately not all sensor registers are handled by the firmware and even
less are exposed via WMI.

Signed-off-by: Thomas Weißschuh 
---
 MAINTAINERS |   6 +
 drivers/platform/x86/Kconfig|  11 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/gigabyte-wmi.c | 194 
 4 files changed, 212 insertions(+)
 create mode 100644 drivers/platform/x86/gigabyte-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..9c10cfc00fe8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7543,6 +7543,12 @@ F:   Documentation/filesystems/gfs2*
 F: fs/gfs2/
 F: include/uapi/linux/gfs2_ondisk.h
 
+GIGABYTE WMI DRIVER
+M:     Thomas Weißschuh 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/gigabyte-wmi.c
+
 GNSS SUBSYSTEM
 M: Johan Hovold 
 S: Maintained
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad4e630e73e2..96622a2106f7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,17 @@ config XIAOMI_WMI
  To compile this driver as a module, choose M here: the module will
  be called xiaomi-wmi.
 
+config GIGABYTE_WMI
+   tristate "Gigabyte WMI temperature driver"
+   depends on ACPI_WMI
+   depends on HWMON
+   help
+ Say Y here if you want to support WMI-based temperature reporting on
+ Gigabyte mainboards.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gigabyte-wmi.
+
 config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 60d554073749..1621ebfd04fd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)   += 
intel-wmi-thunderbolt.o
 obj-$(CONFIG_MXM_WMI)  += mxm-wmi.o
 obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)   += xiaomi-wmi.o
+obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
diff --git a/drivers/platform/x86/gigabyte-wmi.c 
b/drivers/platform/x86/gigabyte-wmi.c
new file mode 100644
index ..fb4e6d4c1823
--- /dev/null
+++ b/drivers/platform/x86/gigabyte-wmi.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (C) 2021 Thomas Weißschuh 
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910"
+#define NUM_TEMPERATURE_SENSORS 6
+
+static bool force_load;
+module_param(force_load, bool, 0);
+MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform");
+
+enum gigabyte_wmi_commandtype {
+   GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
+   GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
+   GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
+   GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
+   GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
+};
+
+struct gigabyte_wmi_args {
+   u32 arg1;
+};
+
+static int gigabyte_wmi_perform_query(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, struct 
acpi_buffer *out)
+{
+   const struct acpi_buffer in = {
+   .length = sizeof(*args),
+   .pointer = args,
+   };
+
+   acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, , out);
+
+   if ACPI_FAILURE(ret)
+   return -EIO;
+
+   return 0;
+}
+
+static int gigabyte_wmi_query_integer(struct wmi_device *wdev,
+ enum gigabyte_wmi_commandtype command,
+ struct gigabyte_wmi_args *args, u64 *res)
+{
+   union acpi_object *obj;
+   struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
+   int ret;
+
+   ret = gigabyte_wmi_perform_query(wdev, command, args, );
+   if (ret)
+   return ret;
+   obj = result.pointer;
+   if (obj && obj->type == AC

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

2021-04-09 Thread Thomas Weißschuh 
On Do, 2021-04-08T08:00-0700, Guenter Roeck wrote:
> On 4/8/21 2:36 AM, Hans de Goede wrote:
> > On 4/7/21 9:43 PM, Thomas Weißschuh wrote:
> >> On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote:
> > Jean, Guenter,
> > 
> > Thomas has been working on a WMI driver to expose various motherboard
> > temperatures on a gigabyte board where the IO-addresses for the it87 chip
> > are reserved by ACPI. We are discussing how best to deal with this, there
> > are some ACPI methods to directly access the super-IO registers (with 
> > locking
> > to protect against other ACPI accesses). This reminded me of an idea I had
> > a while ago to solve a similar issue with an other superIO chip, abstract
> > the superIO register access-es using some reg_ops struct and allow an 
> > ACPI/WMI
> > driver to provide alternative reg_ops:
> > https://bugzilla.kernel.org/show_bug.cgi?id=204807#c47
> > 
> > Do you think this is a good idea (or a bad one)? And would something like 
> > that
> > be acceptable to you ?
> > 
> 
> The upstream it87 driver is severely out of date. I had an out-of-tree driver
> with various improvements which I didn't upstream, first because no one was 
> willing
> to review changes and then because it had deviated too much. I pulled it from
> public view because I got pounded for not upstreaming it, because people 
> started
> demanding support (not asking, demanding) for it, and because Gigabyte stopped
> providing datasheets for the more recent ITE chips and it became effectively
> unmaintainable.
> 
> Some ITE chips have issues which can cause system hangs if accessed directly.
> I put some work to remedy that into the non-upstream driver, but that was all
> just guesswork. Gigabyte knows about the problem (or so I was told from 
> someone
> who has an NDA with them), but I didn't get them or ITE to even acknowledge it
> to me. I even had a support case open with Gigabyte for a while, but all I 
> could
> get out of them is that they don't support Linux and what I would have to 
> reproduce
> the problem with Windows for them to provide assistance (even though, again,
> they knew about it).
> 
> As for using ACPI locks or WMI to ensure that ACPI leaves the chip alone while
> the driver accesses chips directly: That is an option, but it has (at least)
> two problems.
> 
> First, ACPI access methods are not well documented or standardized. I had 
> tried
> to find useful means to do that some time ago, but I gave up because each 
> board
> (even from the same vendor) handles locking and accesses differently. We would
> end up with lots of board specific code. Coincidentally, that was for ASUS 
> boards
> and the nct6775 driver.

At least for all the Gigabyte ACPI tables I have looked at all access is done
via two-byte "OperationRegion" over the Index/Data addresses, a "Field" with
two entries for these and an "IndexField" that is actually used to perform the
accesses.
As the IndexField is synchronized via "Lock" it should take a lock on the
OperationRegion itself.

So I think we should be technically fine with validating these assumption and
then also taking locks on the OperationRegion.

If it is reasonable to do so is another question.

> Second, access through ACPI is only one of the issues. Turns out there are two
> ITE chips on many of the Gigabyte boards nowadays, and the two chips talk to 
> each
> other using I2C. My out-of-tree driver tried to remedy that by blocking those
> accesses while the driver used the chip, but, again, without Gigabyte / ITE
> support this was never a perfect solution, and there was always the risk that
> the board ended up hanging because that access was blocked for too long.
> Recent ITE chips solve that problem by providing memory mapped access to the
> chip registers, but that is only useful if one has a datasheet.

Are both of these chips available at the two well-known registers 0x2e and
0x4e?

Would this too-long blocking also occur when only accessing single registers
for read-only access?
Any write access would probably have to be blocked anyways.

> Overall, I don't think it makes much sense trying to make significant changes
> to the it87 driver without pulling in all the changes I had made, and without
> finding a better fix for the cross-chip access problems. I for sure won't have
> time for that (and getting hwmon patches reviewed is still very much an 
> issue).
> 
> Having said that, I am of course open to adding WMI/ACPI drivers for the 
> various
> boards. Good luck getting support from Gigabyte, though. Or from ASUS, for 
> that
> matter.

Thomas

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

2021-04-08 Thread Thomas Weißschuh 
Hi Hans,

On Do, 2021-04-08T11:36+0200, Hans de Goede wrote:
> On 4/7/21 9:43 PM, Thomas Weißschuh wrote:
> > Hi Hans,
> > 
> > On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote:
> >> Thank you for your new driver and thank you for the quick respin
> >> addressing Barnabás' request to make it a WMI driver.
> >>
> >> The code looks good, so merging this should be a no-brainer,
> >> yet I'm not sure if I should merge this driver as-is, let me
> >> explain.
> > 
> > thanks for the encouraging words.
> > 
> >> The problem is that I assume that this is based on reverse-engineering?
> > 
> > Yes, it is completely reverse-engineered.
> > Essentially I stumbled upon Matthews comment at
> > https://bugzilla.kernel.org/show_bug.cgi?id=204807#c37 and went from there.
> > 
> >> We have some mixes experiences with reverse-engineered WMI drivers,
> >> sometimes a feature is not supported yet the wmi_evaluate_method()
> >> call happily succeeds. One example of this causing trouble is:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c
> > 
> > There actually are reports of recent, similar mainboards with recent 
> > firmware and
> > similar sensor chips that do not support the temperature query.
> > (https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/3 and
> > https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/2)
> > 
> > Unfortunately for unknown WMI queries the firmware does not return any 
> > value.
> > This ends up as an ACPI integer with value 0 on the driver side.
> > (Which I could not yet find documentation for if that is expected)
> > In the current version of the driver EIO is returned for 0 values which
> > get translated to N/A by lm-sensors.
> > 
> >> At a minimum I think your driver should check in its
> >> probe function that
> >>
> >> gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, ...)
> >>
> >> actually succeeds on the machine the driver is running on chances
> >> are that Gigabyte has been using the DEADBEEF-2001--00A0-C9062910
> >> GUID for ages, where as the 0x125 value for GIGABYTE_WMI_TEMPERATURE_QUERY
> >> suggests that this is a pretty new API.
> > 
> > Would it be enough to probe all six sensors and check if all return 0?
> 
> I think that failing the probe with -ENODEV, with a dev_info() explaining why 
> when
> all six sensors return 0 would be good yes, that should also fix those 2
> issues on https://github.com/t-8ch/linux-gigabyte-wmi-driver/.

I added such a validation step.

> >> It would be good if you can see if you can find some DSDT-s for older
> >> gigabyte motherboards attached to various distro's bug-tracking systems
> >> or forum posts and see how those respond to an unknown 
> >> gigabyte_wmi_commandtype.
> > 
> > Will do.
> 
> Since you alreayd have bugreports of boards where this does not work,
> please don't spend too much time on this. I guess those older DSDT-s will
> also just return an integer with value 0.

Ok.

> >> Another open question to make sure this driver is suitable
> >> as a generic driver (and does not misbehave) is how to figure out
> >> how many temperature sensors there actually are.
> > 
> > So far I could not find out how to query this from the firmware.
> > The IT8688 chip can report the state of each sensor but that is not exposed 
> > by
> > the firmware.
> > But even the state information from the IT8688 is not accurate as is.
> > One of the sensors that is reported as being active (directly via it87) on 
> > my
> > machine always reports -55°C (yes, negative).
> 
> Ok.
> 
> >> Perhaps the WMI interface returns an error when you query an out-of-range
> >> temperature channel?
> > 
> > Also "0" as mentioned above.
> 
> Hmm, so maybe this can be used to limit the amount of reported temperature
> sensors, IOW if sensors 5 and 6 report 0, only register 4 sensors ?

So far the 0-returning sensors have not been at the end of the list but in the
middle. Is it worth building logic to properly probe a bitmask of useful
sensors?

> >> One option here might be to add a DMI matching table and only load on
> >> systems on that table for now. That table could then perhaps also provide
> >> labels for each of the temperature channels, which is something which
> >> would be nice to have regardless of my worries about how well this driver
> >>

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

2021-04-08 Thread Thomas Weißschuh 
Hi,

On Mi, 2021-04-07T18:27+, Barnabás Pőcze wrote:
> 2021. április 5., hétfő 22:48 keltezéssel, Thomas Weißschuh írta:
> > Tested with a X570 I Aorus Pro Wifi.
> > The mainboard contains an ITE IT8688E chip for management.
> > This chips is also handled by drivers/hwmon/i87.c but as it is also used
> > by the firmware itself it needs an ACPI driver.
> 
> I gather this means you're getting the
> 
>   ACPI Warning: SystemIO range ... conflicts with ...
>   ACPI: If an ACPI driver is available for this device, you should use it 
> instead of the native driver
> 
> warning?

Exactly.

> > +struct gigabyte_wmi_args {
> > +   u32 arg1;
> > +};
> > +
> > +static int gigabyte_wmi_perform_query(enum gigabyte_wmi_commandtype 
> > command,
> > +   struct gigabyte_wmi_args *args, struct acpi_buffer *out)
> > +{
> > +   const struct acpi_buffer in = {
> > +   .length = sizeof(*args),
> > +   .pointer = args,
> > +   };
> > +
> > +   acpi_status ret = wmi_evaluate_method(GIGABYTE_WMI_GUID, 0x0, command, 
> > , out);
> 
> Ideally, you'd use the WMI device that was passed to the probe method to do 
> the query
> using `wmidev_evaluate_method()`. You can pass the WMI device pointer
> to `devm_hwmon_device_register_with_info()` in the `drvdata` argument,
> then in the ->read() callback you can retrieve it:
> 
>   static int gigabyte_wmi_hwmon_read(struct device *dev, ...)
>   {
> struct wmi_device *wdev = dev_get_drvdata(dev);
> 
> and then you can pass that to the other functions.

Done.

> > +   if (ret == AE_OK) {
> > +   return 0;
> > +   } else {
> > +   return -EIO;
> > +   };
> 
> The `;` is not needed. And please use `ACPI_FAILURE()` or `ACPI_SUCCESS()`
> to check the returned value. For example:
> 
>   acpi_status ret = ...;
>   if (ACPI_FAILURE(ret))
> return -EIO;
> 
>   return 0;

Done.

> > +}
> > +
> > +static int gigabyte_wmi_query_integer(enum gigabyte_wmi_commandtype 
> > command,
> > +   struct gigabyte_wmi_args *args, u64 *res)
> > +{
> > +   union acpi_object *obj;
> > +   struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> > +   int ret;
> > +
> > +   ret = gigabyte_wmi_perform_query(command, args, );
> > +   if (ret) {
> > +   goto out;
> 
> I believe if this branch is taken, no buffer is allocated (due to the 
> failure),
> so you can just `return ret;` here and do away with the goto completely - if 
> I'm not mistaken.

Done.

> > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = {
> > +   HWMON_CHANNEL_INFO(temp,
> > +   HWMON_T_INPUT,
> > +   HWMON_T_INPUT,
> > +   HWMON_T_INPUT,
> > +   HWMON_T_INPUT,
> > +   HWMON_T_INPUT,
> > +   HWMON_T_INPUT),
> > +   NULL,
> ^
> Minor thing: usually commas after sentinel values are omitted.

Done.

> > +static const struct wmi_device_id gigabyte_wmi_id_table[] = {
> > +   { GIGABYTE_WMI_GUID, NULL },
> > +   { },
>^
> Same here.

Done.

> 
> > +};
> > +
> > +static struct wmi_driver gigabyte_wmi_driver = {
> > +   .driver = {
> > +   .name = "gigabyte-wmi",
> > +   },
> > +   .id_table = gigabyte_wmi_id_table,
> > +   .probe = gigabyte_wmi_probe,
> > +};
> > +module_wmi_driver(gigabyte_wmi_driver);
> > +
> > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table);
> > +MODULE_AUTHOR("Thomas Weißschuh ");
> > +MODULE_DESCRIPTION("Gigabyte Temperature WMI Driver");
> 
> It's a very minor thing, but could you please
> synchronize this description with the Kconfig?

Of course.

Thanks again for the review!

Thomas

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

2021-04-07 Thread Thomas Weißschuh 
Hi Hans,

On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote:
> Thank you for your new driver and thank you for the quick respin
> addressing Barnabás' request to make it a WMI driver.
> 
> The code looks good, so merging this should be a no-brainer,
> yet I'm not sure if I should merge this driver as-is, let me
> explain.

thanks for the encouraging words.

> The problem is that I assume that this is based on reverse-engineering?

Yes, it is completely reverse-engineered.
Essentially I stumbled upon Matthews comment at
https://bugzilla.kernel.org/show_bug.cgi?id=204807#c37 and went from there.

> We have some mixes experiences with reverse-engineered WMI drivers,
> sometimes a feature is not supported yet the wmi_evaluate_method()
> call happily succeeds. One example of this causing trouble is:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c

There actually are reports of recent, similar mainboards with recent firmware 
and
similar sensor chips that do not support the temperature query.
(https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/3 and
https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/2)

Unfortunately for unknown WMI queries the firmware does not return any value.
This ends up as an ACPI integer with value 0 on the driver side.
(Which I could not yet find documentation for if that is expected)
In the current version of the driver EIO is returned for 0 values which
get translated to N/A by lm-sensors.

> At a minimum I think your driver should check in its
> probe function that
> 
> gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, ...)
> 
> actually succeeds on the machine the driver is running on chances
> are that Gigabyte has been using the DEADBEEF-2001--00A0-C9062910
> GUID for ages, where as the 0x125 value for GIGABYTE_WMI_TEMPERATURE_QUERY
> suggests that this is a pretty new API.

Would it be enough to probe all six sensors and check if all return 0?

> It would be good if you can see if you can find some DSDT-s for older
> gigabyte motherboards attached to various distro's bug-tracking systems
> or forum posts and see how those respond to an unknown 
> gigabyte_wmi_commandtype.

Will do.

> Another open question to make sure this driver is suitable
> as a generic driver (and does not misbehave) is how to figure out
> how many temperature sensors there actually are.

So far I could not find out how to query this from the firmware.
The IT8688 chip can report the state of each sensor but that is not exposed by
the firmware.
But even the state information from the IT8688 is not accurate as is.
One of the sensors that is reported as being active (directly via it87) on my
machine always reports -55°C (yes, negative).

> Perhaps the WMI interface returns an error when you query an out-of-range
> temperature channel?

Also "0" as mentioned above.

> One option here might be to add a DMI matching table and only load on
> systems on that table for now. That table could then perhaps also provide
> labels for each of the temperature channels, which is something which
> would be nice to have regardless of my worries about how well this driver
> will work on motherboards on which it has not been tested.

I am collecting reports for working motherboards at
https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/1 .

> You could combine this DMI matching table with a "force" module option to
> continue with probing on boards which are not on the table to allow users
> to test and report their results to you.
>
> And hopefully after a while, when we're confident that the code works
> well on most gigabyte boards we can drop the DMI table, or at least
> only use it for the channel labels.

That sounds good.

> Please don't take this the wrong way; I think it is great that you are
> working on this. And the quick turnaround of the v2 of this drivers makes
> me pretty certain that we can figure something out and get this merged.

Thank you for the feedback!

> Have you tried contacting Gigabyte about this? I don't think the WMI
> interface is something which they need to keep secret for competitive
> reasons, so maybe they can help? Note if they want you to sign a NDA
> of sorts to view docs, then make sure that it contains some language
> about them allowing you to release an opensource driver for their
> hardware based on the "protected" information.

I have not contacted them yet, will do.

As mentioned in the initial patch submission there would be different ways to
access this information firmware:

* Directly call the underlying ACPI methods (these are present in all so far
  observed firmwares, even if not exposed via WMI).
* Directly access the ACPI IndexField representing the it87 chip.
* Directly access the it87 registers while holding the relevant locks via ACPI.

I assume all of those mechanisms have no place in a proper kernel driver but
would like to get your opinion on it.

Thomas

[PATCH v2] platform/x86: add Gigabyte WMI temperature driver

2021-04-05 Thread Thomas Weißschuh 
Changes since v1:
* Incorporate feedback from Barnabás Pőcze
  * Use a WMI driver instead of a platform driver
  * Let the kernel manage the driver lifecycle
  * Fix errno/ACPI error confusion
  * Fix resource cleanup
  * Document reason for integer casting

Thank you Barnabás for your review, it is much appreciated.

-- >8 --

Tested with a X570 I Aorus Pro Wifi.
The mainboard contains an ITE IT8688E chip for management.
This chips is also handled by drivers/hwmon/i87.c but as it is also used
by the firmware itself it needs an ACPI driver.

Unfortunately not all sensor registers are handled by the firmware and even
less are exposed via WMI.

Signed-off-by: Thomas Weißschuh 
---
 drivers/platform/x86/Kconfig|  11 +++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/gigabyte-wmi.c | 138 
 3 files changed, 150 insertions(+)
 create mode 100644 drivers/platform/x86/gigabyte-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad4e630e73e2..96622a2106f7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,17 @@ config XIAOMI_WMI
  To compile this driver as a module, choose M here: the module will
  be called xiaomi-wmi.
 
+config GIGABYTE_WMI
+   tristate "Gigabyte WMI temperature driver"
+   depends on ACPI_WMI
+   depends on HWMON
+   help
+ Say Y here if you want to support WMI-based temperature reporting on
+ Gigabyte mainboards.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gigabyte-wmi.
+
 config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 60d554073749..1621ebfd04fd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)   += 
intel-wmi-thunderbolt.o
 obj-$(CONFIG_MXM_WMI)  += mxm-wmi.o
 obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)   += xiaomi-wmi.o
+obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
diff --git a/drivers/platform/x86/gigabyte-wmi.c 
b/drivers/platform/x86/gigabyte-wmi.c
new file mode 100644
index ..8618363e3ccf
--- /dev/null
+++ b/drivers/platform/x86/gigabyte-wmi.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (C) 2021 Thomas Weißschuh 
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+
+#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910"
+
+enum gigabyte_wmi_commandtype {
+   GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
+   GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
+   GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
+   GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
+   GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
+};
+
+struct gigabyte_wmi_args {
+   u32 arg1;
+};
+
+static int gigabyte_wmi_perform_query(enum gigabyte_wmi_commandtype command,
+   struct gigabyte_wmi_args *args, struct acpi_buffer *out)
+{
+   const struct acpi_buffer in = {
+   .length = sizeof(*args),
+   .pointer = args,
+   };
+
+   acpi_status ret = wmi_evaluate_method(GIGABYTE_WMI_GUID, 0x0, command, 
, out);
+   if (ret == AE_OK) {
+   return 0;
+   } else {
+   return -EIO;
+   };
+}
+
+static int gigabyte_wmi_query_integer(enum gigabyte_wmi_commandtype command,
+   struct gigabyte_wmi_args *args, u64 *res)
+{
+   union acpi_object *obj;
+   struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
+   int ret;
+
+   ret = gigabyte_wmi_perform_query(command, args, );
+   if (ret) {
+   goto out;
+   }
+   obj = result.pointer;
+   if (obj && obj->type == ACPI_TYPE_INTEGER) {
+   *res = obj->integer.value;
+   ret = 0;
+   } else {
+   ret = -EIO;
+   }
+out:
+   kfree(result.pointer);
+   return ret;
+}
+
+static int gigabyte_wmi_temperature(u8 sensor, long *res)
+{
+   struct gigabyte_wmi_args args = {
+   .arg1 = sensor,
+   };
+   u64 temp;
+   acpi_status ret;
+
+   ret = gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, , 
);
+   if (ret == 0)
+   *res = (s8) temp * 1000; // value is a signed 8-bit integer
+   return ret;
+}
+
+static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types 
type,
+   u32 attr, int channel, long *val)
+{
+   return gigabyte_wmi_temperature(channel, val);
+}
+
+static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enu

[PATCH] platform/x86: add Gigabyte WMI temperature driver

2021-04-05 Thread Thomas Weißschuh 
Hi,

this patch adds support for temperature readings on Gigabyte Mainboards.
(At least on mine)

The current code should be usable as is.
I'd like for people to test it though and report their results with different
hardware.

Further development I have some general questions:

The ASL IndexField does not cover all relevant registers, can it be extended by
driver code?

* Not all registers are exposed via ACPI methods, can they be accessed via ACPI 
directly?
* Some registers are exposed via ACPI methods but are not reachable directly 
from the WMI dispatcher.
  * Does ASL have some sort of reflection that could enable those methods?
  * Is it possible to call those methods directly, bypassing WMI?

I suspect the answer to be "no" for all of these, but maybe I am wrong.

Furthermore there are WMI methods to return information about the installed
firmware.

* Version
* Build-date
* Motherboard name

Would it make sense to add this information as attributes to the
platform_device?

The ACPI tables can be found here:
https://github.com/t-8ch/linux-gigabyte-wmi-driver/blob/main/ssdt8.dsl

Thanks,
Thomas

-- >8 --

Tested with a X570 I Aorus Pro Wifi.
The mainboard contains a ITE IT8688E chip for management.
This chips is also handled by drivers/hwmon/i87.c but as it is also used
by the firmware itself it needs an ACPI driver.

Unfortunately not all sensor registers are handled by the firmware and even
less are exposed via WMI.

Signed-off-by: Thomas Weißschuh 
---
 drivers/platform/x86/Kconfig|  10 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/gigabyte-wmi.c | 178 
 3 files changed, 189 insertions(+)
 create mode 100644 drivers/platform/x86/gigabyte-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ad4e630e73e2..40d593ff1f01 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,16 @@ config XIAOMI_WMI
  To compile this driver as a module, choose M here: the module will
  be called xiaomi-wmi.
 
+config GIGABYTE_WMI
+   tristate "Gigabyte WMI temperature driver"
+   depends on ACPI_WMI
+   depends on HWMON
+   help
+ Say Y here if you want to support WMI-based temperature on Gigabyte 
mainboards.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gigabyte-wmi.
+
 config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 60d554073749..1621ebfd04fd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)   += 
intel-wmi-thunderbolt.o
 obj-$(CONFIG_MXM_WMI)  += mxm-wmi.o
 obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
 obj-$(CONFIG_XIAOMI_WMI)   += xiaomi-wmi.o
+obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
 
 # Acer
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
diff --git a/drivers/platform/x86/gigabyte-wmi.c 
b/drivers/platform/x86/gigabyte-wmi.c
new file mode 100644
index ..a3749cf248cb
--- /dev/null
+++ b/drivers/platform/x86/gigabyte-wmi.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Gigabyte WMI temperature driver
+ *
+ * Copyright (C) 2021 Thomas Weißschuh 
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910"
+#define DRVNAME "gigabyte-wmi"
+
+MODULE_AUTHOR("Thomas Weißschuh ");
+MODULE_DESCRIPTION("Gigabyte Generic WMI Driver");
+MODULE_LICENSE("GPL");
+
+MODULE_ALIAS("wmi:" GIGABYTE_WMI_GUID);
+
+enum gigabyte_wmi_commandtype {
+   GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
+   GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
+   GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
+   GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
+   GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
+};
+
+static int gigabyte_wmi_temperature(u8 sensor, s8 *res);
+
+static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+   struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+   int index = sattr->index;
+   s8 temp;
+   acpi_status res;
+
+   res = gigabyte_wmi_temperature(index, );
+   if (ACPI_FAILURE(res))
+   return -res;
+
+   return sprintf(buf, "%d\n", temp * 1000);
+}
+
+static SENSOR_DEVICE_ATTR_2_RO(temp1_input, temp, 0, 0);
+static SENSOR_DEVICE_ATTR_2_RO(temp2_input, temp, 0, 1);
+static SENSOR_DEVICE_ATTR_2_RO(temp3_input, temp, 0, 2);
+static SENSOR_DEVICE_ATTR_2_RO(temp4_input, temp, 0, 3);
+static SENSOR_DEVICE_AT

Re: [PATCH 3.16 310/366] vmxnet3: fix checks for dma mapping errors

2019-03-29 Thread Thomas Weißschuh 
> 3.16.60-rc1 review patch.  If anyone has any objections, please let me know.

Sorry for the late response, this just hit the kernel in Debian Jessie
(oldstable) a few days ago.

> --
> 
> From: Alexey Khoroshilov 
> 
> commit 5738a09d58d5ad2871f1f9a42bf6a3aa9ece5b3c upstream.
> 
> vmxnet3_drv does not check dma_addr with dma_mapping_error()
> after mapping dma memory. The patch adds the checks and
> tries to handle failures.

We are seeing kernel panics/machine freezes/BUGs with the new 3.16.64 from 
Debian.
I bisected it with the vanilla stable kernel and it boiled down to this commit.
VMs of multiple nodes of our vmware cluster are affected.
The bug can be triggered in multiple ways, I have seen it when an external
network request is served, when installing packages over the network and
performing a git clone.

I will try to get the specific versions of the involved hardware components
next week.
The 4.9.144 stable kernel (which also contains this commit works fine on the
affected machine)

Below you can see the dmesg log of one affected machine:

[1.772994] vmxnet3 :03:00.0 eth0: intr type 3, mode 0, 5 vectors 
allocated
[1.774079] vmxnet3 :03:00.0 eth0: NIC Link is Up 1 Mbps
[9.622787] gunicorn: worke: Corrupted page table at address 362d000
[9.622817] PGD 8000753b7067 PUD 6f84e067 PMD 76cbb067 PTE 
6461685368637845
[9.622848] Bad pagetable: 000d [#1] SMP 
[9.622866] Modules linked in: binfmt_misc ip6table_filter ip6_tables 
ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_comment xt_multiport 
xt_conntrack nf_conntrack iptable_filter ip_tables x_tables crc32_pclmul 
crc32c_intel aesni_intel aes_x86_64 glue_helper lrw vmw_vsock_vmci_transport 
vsock gf128mul vmw_balloon ppdev evdev ablk_helper cryptd pcspkr serio_raw 
vmwgfx drm_kms_helper ttm ac processor battery button parport_pc thermal_sys 
drm parport shpchp vmw_vmci autofs4 ext4 crc16 mbcache jbd2 dm_mod sg sr_mod 
cdrom sd_mod crc_t10dif crct10dif_generic ata_generic crct10dif_pclmul 
crct10dif_common psmouse vmxnet3 ata_piix mptspi scsi_transport_spi mptscsih 
libata i2c_piix4 mptbase scsi_mod i2c_core
[9.623168] CPU: 1 PID: 717 Comm: gunicorn: worke Not tainted 3.16.59+ #18
[9.623191] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
Desktop Reference Platform, BIOS 6.00 04/05/2016
[9.623225] task: 88007835e090 ti: 88006f834000 task.ti: 
88006f834000
[9.623249] RIP: 0033:[<7fb4bfb6d123>]  [<7fb4bfb6d123>] 
0x7fb4bfb6d123
[9.623278] RSP: 002b:7fff6e4718b8  EFLAGS: 00010206
[9.623296] RAX: fff7b8c0 RBX: 036aadc0 RCX: 036b1740
[9.623318] RDX: 0372f500 RSI: 03626690 RDI: 036aade0
[9.623341] RBP: 00084740 R08: fff7b8b0 R09: fff7b8a0
[9.623363] R10: fff7b890 R11: 0037 R12: 00085760
[9.623385] R13: 004cd810 R14: 1000 R15: 03589dd0
[9.623408] FS:  7fb4c0ffe700() GS:88007fc8() 
knlGS:
[9.623433] CS:  0010 DS:  ES:  CR0: 80050033
[9.623451] CR2: 0362d000 CR3: 753fa000 CR4: 00360770
[9.623524] DR0:  DR1:  DR2: 
[9.623547] DR3:  DR6: fffe0ff0 DR7: 0400

[9.623577] RIP  [<7fb4bfb6d123>] 0x7fb4bfb6d123
[9.623600]  RSP <7fff6e4718b8>
[9.623614] ---[ end trace f863ea854df6c9a5 ]---
[9.624169] swap_free: Bad swap file entry 1001a1e5a32423f7
[9.624189] BUG: Bad page map in process gunicorn: worke  
pte:417869736f702024 pmd:76cbb067
[9.624215] addr:0360 vm_flags:08100073 
anon_vma:88007538c470 mapping:  (null) index:3600
[9.625444] CPU: 1 PID: 717 Comm: gunicorn: worke Tainted: G  D   
3.16.59+ #18
[9.626070] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
Desktop Reference Platform, BIOS 6.00 04/05/2016
[9.627321]   8151fda4 0360 
8800753700d0
[9.627968]  8116f380 0008 880076cbb000 
417869736f702024
[9.628596]   0373f000 88006f837dd0 
0360
[9.629213] Call Trace:
[9.629811]  [] ? dump_stack+0x5d/0x78
[9.630413]  [] ? print_bad_pte+0x1b0/0x280
[9.630991]  [] ? unmap_single_vma+0x4c2/0x830
[9.631556]  [] ? unmap_vmas+0x4c/0xa0
[9.632106]  [] ? exit_mmap+0x92/0x160
[9.632640]  [] ? mmput+0x5c/0x120
[9.633162]  [] ? do_exit+0x333/0xae0
[9.633677]  [] ? printk+0x4f/0x57
[9.634171]  [] ? oops_end+0x97/0xe0
[9.634653]  [] ? __do_page_fault+0x376/0x470
[9.635129]  [] ? page_fault+0x28/0x30
[9.635629] BUG: Bad page map in process gunicorn: worke  
pte:2420746e756f6363 pmd:76cbb067
[9.636111] addr:03601000 vm_flags:08100073 
anon_vma:88007538c470 mapping:  (null)

[PATCH] scripts/spdxcheck.py: improve Python 3 compat

2018-09-16 Thread Thomas Weißschuh 
When reading lines from a text-mode fd strings are returned.
These can not be decoded again into strings, breaking the logic in
parser.
Just make sure all files are opened in binary mode on Python 3, so the
current logic keeps working.

This remains compatible with Python 2 and should have no functional
change.

Signed-off-by: Thomas Weißschuh 
---
 scripts/spdxcheck.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index 839e190bbd7a..8f472f995d70 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -250,12 +250,15 @@ if __name__ == '__main__':
 
 try:
 if len(args.path) and args.path[0] == '-':
-parser.parse_lines(sys.stdin, args.maxlines, '-')
+parser.parse_lines(
+# always get the binary fd
+getattr(sys.stdin, 'buffer', sys.stdin),
+args.maxlines, '-')
 else:
 if args.path:
 for p in args.path:
 if os.path.isfile(p):
-parser.parse_lines(open(p), args.maxlines, p)
+parser.parse_lines(open(p, 'rb'), args.maxlines, p)
 elif os.path.isdir(p):
 scan_git_subtree(repo.head.reference.commit.tree, p)
 else:
-- 
2.18.0

[PATCH] scripts/spdxcheck.py: improve Python 3 compat

2018-09-16 Thread Thomas Weißschuh 
When reading lines from a text-mode fd strings are returned.
These can not be decoded again into strings, breaking the logic in
parser.
Just make sure all files are opened in binary mode on Python 3, so the
current logic keeps working.

This remains compatible with Python 2 and should have no functional
change.

Signed-off-by: Thomas Weißschuh 
---
 scripts/spdxcheck.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index 839e190bbd7a..8f472f995d70 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -250,12 +250,15 @@ if __name__ == '__main__':
 
 try:
 if len(args.path) and args.path[0] == '-':
-parser.parse_lines(sys.stdin, args.maxlines, '-')
+parser.parse_lines(
+# always get the binary fd
+getattr(sys.stdin, 'buffer', sys.stdin),
+args.maxlines, '-')
 else:
 if args.path:
 for p in args.path:
 if os.path.isfile(p):
-parser.parse_lines(open(p), args.maxlines, p)
+parser.parse_lines(open(p, 'rb'), args.maxlines, p)
 elif os.path.isdir(p):
 scan_git_subtree(repo.head.reference.commit.tree, p)
 else:
-- 
2.18.0