| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <70781ff8-31d6-deef-ac05-f882699901f8@roeck-us.net>
Date: Fri, 17 Sep 2021 14:48:53 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Denis Pauk <pauk.denis@...il.com>
Cc: Bernhard Seibold <mail@...nhard-seibold.de>,
Pär Ekholm <pehlm@...holm.org>,
to.eivind@...il.com, "Artem S . Tashkinov" <aros@....com>,
Vittorio Roberto Alfieri <me@...toor.com>,
Sahan Fernando <sahan.h.fernando@...il.com>,
Andy Shevchenko <andriy.shevchenko@...el.com>,
Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 3/3] hwmon: (nct6775) Support access via Asus WMI
On 9/17/21 2:18 PM, Denis Pauk wrote:
> Support accessing the NCT677x via Asus WMI functions.
>
> On mainboards that support this way of accessing the chip, the driver will
> usually not work without this option since in these mainboards, ACPI will
> mark the I/O port as used.
>
> Code uses ACPI firmware interface to communicate with sensors with ASUS
> motherboards:
> * PRIME B460-PLUS,
> * ROG CROSSHAIR VIII IMPACT,
> * ROG STRIX B550-E GAMING,
> * ROG STRIX B550-F GAMING,
> * ROG STRIX B550-F GAMING (WI-FI),
> * ROG STRIX Z490-I GAMING,
> * TUF GAMING B550M-PLUS,
> * TUF GAMING B550M-PLUS (WI-FI),
> * TUF GAMING B550-PLUS,
> * TUF GAMING X570-PLUS,
> * TUF GAMING X570-PRO (WI-FI).
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
> Signed-off-by: Denis Pauk <pauk.denis@...il.com>
> Co-developed-by: Bernhard Seibold <mail@...nhard-seibold.de>
> Signed-off-by: Bernhard Seibold <mail@...nhard-seibold.de>
> Tested-by: Pär Ekholm <pehlm@...holm.org>
> Tested-by: <to.eivind@...il.com>
> Tested-by: Artem S. Tashkinov <aros@....com>
> Tested-by: Vittorio Roberto Alfieri <me@...toor.com>
> Tested-by: Sahan Fernando <sahan.h.fernando@...il.com>
> Cc: Andy Shevchenko <andriy.shevchenko@...el.com>
> Cc: Guenter Roeck <linux@...ck-us.net>
>
> ---
> Changes in v8:
> - Rename asuswmi_evaluate_method() to nct6775_asuswmi_evaluate_method().
> - Add out error message if WMI method is failed.
> - Stop futher read/write if first operation is failed.
>
> Changes in v7:
> - Remove unrequred & 0xff with int8 variables.
> - Make ASUSWMI_UNSUPPORTED_METHOD as default value for WMI responce,
> before run wmi_evaluate_method().
> - Rename ASUSWMI_MGMT2_GUID to ASUSWMI_MONITORING_GUID.
> - Replace checks of 'err != -EINVAL' with 'err >= 0' for match_string result.
>
> Changes in v6:
> - Minimaze codes inside code inside defined(CONFIG_ACPI_WMI).
>
> Changes in v5:
> - Use IS_ENABLED(CONFIG_ACPI_WMI) instead defined(CONFIG_ACPI_WMI)
>
> Changes in v4:
> - Fix build without ACPI WMI.
>
> Changes in v3:
> - Remove unrequired type conversions.
> - Save result of match_string before check.
>
> Changes in v2:
> - Split changes to separate patches.
> - Limit WMI usage by DMI_BOARD_NAME in checked ASUS motherboards.
> ---
> drivers/hwmon/Kconfig | 1 +
> drivers/hwmon/nct6775.c | 256 ++++++++++++++++++++++++++++++++++++----
> 2 files changed, 236 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e3675377bc5d..9eefb1014b53 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1423,6 +1423,7 @@ config SENSORS_NCT6683
> config SENSORS_NCT6775
> tristate "Nuvoton NCT6775F and compatibles"
> depends on !PPC
> + depends on ACPI_WMI || ACPI_WMI=n
> select HWMON_VID
> help
> If you say yes here you get support for the hardware monitoring
> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> index 4253eed7f5b0..1f79546730e4 100644
> --- a/drivers/hwmon/nct6775.c
> +++ b/drivers/hwmon/nct6775.c
> @@ -55,6 +55,7 @@
> #include <linux/dmi.h>
> #include <linux/io.h>
> #include <linux/nospec.h>
> +#include <linux/wmi.h>
> #include "lm75.h"
>
> #define USE_ALTERNATE
> @@ -132,10 +133,13 @@ MODULE_PARM_DESC(fan_debounce, "Enable debouncing for fan RPM signal");
> #define SIO_ID_MASK 0xFFF8
>
> enum pwm_enable { off, manual, thermal_cruise, speed_cruise, sf3, sf4 };
> +enum sensor_access { access_direct, access_asuswmi };
>
> struct nct6775_sio_data {
> int sioreg;
> + int ld;
> enum kinds kind;
> + enum sensor_access access;
>
> /* superio_() callbacks */
> void (*sio_outb)(struct nct6775_sio_data *sio_data, int reg, int val);
> @@ -145,6 +149,107 @@ struct nct6775_sio_data {
> void (*sio_exit)(struct nct6775_sio_data *sio_data);
> };
>
> +#define ASUSWMI_MONITORING_GUID "466747A0-70EC-11DE-8A39-0800200C9A66"
> +#define ASUSWMI_METHODID_RSIO 0x5253494F
> +#define ASUSWMI_METHODID_WSIO 0x5753494F
> +#define ASUSWMI_METHODID_RHWM 0x5248574D
> +#define ASUSWMI_METHODID_WHWM 0x5748574D
> +#define ASUSWMI_UNSUPPORTED_METHOD 0xFFFFFFFE
> +
> +static int nct6775_asuswmi_evaluate_method(u32 method_id, u8 bank, u8 reg, u8 val, u32 *retval)
> +{
> +#if IS_ENABLED(CONFIG_ACPI_WMI)
> + u32 args = bank | (reg << 8) | (val << 16);
> + struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> + acpi_status status;
> + union acpi_object *obj;
> + u32 tmp = ASUSWMI_UNSUPPORTED_METHOD;
> +
> + status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0,
> + method_id, &input, &output);
> +
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + obj = output.pointer;
> + if (obj && obj->type == ACPI_TYPE_INTEGER)
> + tmp = obj->integer.value;
> +
> + if (retval)
> + *retval = tmp;
> +
> + kfree(obj);
> +
> + if (tmp == ASUSWMI_UNSUPPORTED_METHOD)
> + return -ENODEV;
> + return 0;
> +#else
> + return -EOPNOTSUPP;
> +#endif
> +}
> +
> +static inline int nct6775_asuswmi_write(u8 bank, u8 reg, u8 val)
> +{
> + int err;
> +
> + err = nct6775_asuswmi_evaluate_method(ASUSWMI_METHODID_WHWM, bank,
> + reg, val, NULL);
> + if (err)
> + pr_err("Failed to write %#x:%#x\n", bank, reg);
> +
Please avoid error message noise. Just return the error.
Thanks,
Guenter
> + return err;
> +}
> +
> +static inline int nct6775_asuswmi_read(u8 bank, u8 reg, u8 *val)
> +{
> + u32 ret, tmp = 0;
> +
> + ret = nct6775_asuswmi_evaluate_method(ASUSWMI_METHODID_RHWM, bank,
> + reg, 0, &tmp);
> + if (ret)
> + pr_err("Failed to read %#x:%#x\n", bank, reg);
> +
> + *val = tmp;
> + return ret;
> +}
> +
> +static int superio_wmi_inb(struct nct6775_sio_data *sio_data, int reg)
> +{
> + int ret, tmp = 0;
> +
> + ret = nct6775_asuswmi_evaluate_method(ASUSWMI_METHODID_RSIO, sio_data->ld,
> + reg, 0, &tmp);
> + if (ret)
> + pr_err("Failed to inb %#x:%#x\n", sio_data->ld, reg);
> +
> + return tmp;
> +}
> +
> +static void superio_wmi_outb(struct nct6775_sio_data *sio_data, int reg, int val)
> +{
> + int ret;
> +
> + ret = nct6775_asuswmi_evaluate_method(ASUSWMI_METHODID_WSIO, sio_data->ld,
> + reg, val, NULL);
> + if (ret)
> + pr_err("Failed to outb %#x:%#x\n", sio_data->ld, reg);
> +}
> +
> +static void superio_wmi_select(struct nct6775_sio_data *sio_data, int ld)
> +{
> + sio_data->ld = ld;
> +}
> +
> +static int superio_wmi_enter(struct nct6775_sio_data *sio_data)
> +{
> + return 0;
> +}
> +
> +static void superio_wmi_exit(struct nct6775_sio_data *sio_data)
> +{
> +}
> +
> static void superio_outb(struct nct6775_sio_data *sio_data, int reg, int val)
> {
> int ioreg = sio_data->sioreg;
> @@ -207,6 +312,7 @@ static void superio_exit(struct nct6775_sio_data *sio_data)
>
> #define NCT6775_REG_BANK 0x4E
> #define NCT6775_REG_CONFIG 0x40
> +#define NCT6775_PORT_CHIPID 0x58
>
> /*
> * Not currently used:
> @@ -1423,6 +1529,54 @@ static bool is_word_sized(struct nct6775_data *data, u16 reg)
> return false;
> }
>
> +static inline void nct6775_wmi_set_bank(struct nct6775_data *data, u16 reg)
> +{
> + u8 bank = reg >> 8;
> +
> + data->bank = bank;
> +}
> +
> +static u16 nct6775_wmi_read_value(struct nct6775_data *data, u16 reg)
> +{
> + int res, err, word_sized = is_word_sized(data, reg);
> + u8 tmp = 0;
> +
> + nct6775_wmi_set_bank(data, reg);
> +
> + err = nct6775_asuswmi_read(data->bank, reg, &tmp);
> + if (err)
> + return 0;
> +
> + res = tmp;
> + if (word_sized) {
> + err = nct6775_asuswmi_read(data->bank, (reg & 0xff) + 1, &tmp);
> + if (err)
> + return 0;
> +
> + res = (res << 8) + tmp;
> + }
> + return res;
> +}
> +
> +static int nct6775_wmi_write_value(struct nct6775_data *data, u16 reg, u16 value)
> +{
> + int res, word_sized = is_word_sized(data, reg);
> +
> + nct6775_wmi_set_bank(data, reg);
> +
> + if (word_sized) {
> + res = nct6775_asuswmi_write(data->bank, reg & 0xff, value >> 8);
> + if (res)
> + return res;
> +
> + res = nct6775_asuswmi_write(data->bank, (reg & 0xff) + 1, value);
> + } else {
> + res = nct6775_asuswmi_write(data->bank, reg & 0xff, value);
> + }
> +
> + return res;
> +}
> +
> /*
> * On older chips, only registers 0x50-0x5f are banked.
> * On more recent chips, all registers are banked.
> @@ -3818,10 +3972,12 @@ static int nct6775_probe(struct platform_device *pdev)
> struct device *hwmon_dev;
> int num_attr_groups = 0;
>
> - res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> - if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> - DRVNAME))
> - return -EBUSY;
> + if (sio_data->access == access_direct) {
> + res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> + DRVNAME))
> + return -EBUSY;
> + }
>
> data = devm_kzalloc(&pdev->dev, sizeof(struct nct6775_data),
> GFP_KERNEL);
> @@ -3830,9 +3986,16 @@ static int nct6775_probe(struct platform_device *pdev)
>
> data->kind = sio_data->kind;
> data->sioreg = sio_data->sioreg;
> - data->addr = res->start;
> - data->read_value = nct6775_read_value;
> - data->write_value = nct6775_write_value;
> +
> + if (sio_data->access == access_direct) {
> + data->addr = res->start;
> + data->read_value = nct6775_read_value;
> + data->write_value = nct6775_write_value;
> + } else {
> + data->read_value = nct6775_wmi_read_value;
> + data->write_value = nct6775_wmi_write_value;
> + }
> +
> mutex_init(&data->update_lock);
> data->name = nct6775_device_names[data->kind];
> data->bank = 0xff; /* Force initial bank selection */
> @@ -4743,6 +4906,7 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
> int err;
> int addr;
>
> + sio_data->access = access_direct;
> sio_data->sioreg = sioaddr;
>
> err = sio_data->sio_enter(sio_data);
> @@ -4837,6 +5001,23 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
> */
> static struct platform_device *pdev[2];
>
> +static const char * const asus_wmi_boards[] = {
> + "PRIME B460-PLUS",
> + "ROG CROSSHAIR VIII DARK HERO",
> + "ROG CROSSHAIR VIII HERO",
> + "ROG CROSSHAIR VIII IMPACT",
> + "ROG STRIX B550-E GAMING",
> + "ROG STRIX B550-F GAMING",
> + "ROG STRIX B550-F GAMING (WI-FI)",
> + "ROG STRIX Z490-I GAMING",
> + "TUF GAMING B550M-PLUS",
> + "TUF GAMING B550M-PLUS (WI-FI)",
> + "TUF GAMING B550-PLUS",
> + "TUF GAMING X570-PLUS",
> + "TUF GAMING X570-PLUS (WI-FI)",
> + "TUF GAMING X570-PRO (WI-FI)",
> +};
> +
> static int __init sensors_nct6775_init(void)
> {
> int i, err;
> @@ -4845,11 +5026,32 @@ static int __init sensors_nct6775_init(void)
> struct resource res;
> struct nct6775_sio_data sio_data;
> int sioaddr[2] = { 0x2e, 0x4e };
> + enum sensor_access access = access_direct;
> + const char *board_vendor, *board_name;
> + u8 tmp;
>
> err = platform_driver_register(&nct6775_driver);
> if (err)
> return err;
>
> + board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> + board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> + if (board_name && board_vendor &&
> + !strcmp(board_vendor, "ASUSTeK COMPUTER INC.")) {
> + err = match_string(asus_wmi_boards, ARRAY_SIZE(asus_wmi_boards),
> + board_name);
> + if (err >= 0) {
> + /* if reading chip id via WMI succeeds, use WMI */
> + if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp)) {
> + pr_info("Using Asus WMI to access %#x chip.\n", tmp);
> + access = access_asuswmi;
> + } else {
> + pr_err("Can't read ChipID by Asus WMI.\n");
> + }
> + }
> + }
> +
> /*
> * initialize sio_data->kind and sio_data->sioreg.
> *
> @@ -4870,6 +5072,16 @@ static int __init sensors_nct6775_init(void)
>
> found = true;
>
> + sio_data.access = access;
> +
> + if (access == access_asuswmi) {
> + sio_data.sio_outb = superio_wmi_outb;
> + sio_data.sio_inb = superio_wmi_inb;
> + sio_data.sio_select = superio_wmi_select;
> + sio_data.sio_enter = superio_wmi_enter;
> + sio_data.sio_exit = superio_wmi_exit;
> + }
> +
> pdev[i] = platform_device_alloc(DRVNAME, address);
> if (!pdev[i]) {
> err = -ENOMEM;
> @@ -4881,23 +5093,25 @@ static int __init sensors_nct6775_init(void)
> if (err)
> goto exit_device_put;
>
> - memset(&res, 0, sizeof(res));
> - res.name = DRVNAME;
> - res.start = address + IOREGION_OFFSET;
> - res.end = address + IOREGION_OFFSET + IOREGION_LENGTH - 1;
> - res.flags = IORESOURCE_IO;
> + if (sio_data.access == access_direct) {
> + memset(&res, 0, sizeof(res));
> + res.name = DRVNAME;
> + res.start = address + IOREGION_OFFSET;
> + res.end = address + IOREGION_OFFSET + IOREGION_LENGTH - 1;
> + res.flags = IORESOURCE_IO;
> +
> + err = acpi_check_resource_conflict(&res);
> + if (err) {
> + platform_device_put(pdev[i]);
> + pdev[i] = NULL;
> + continue;
> + }
>
> - err = acpi_check_resource_conflict(&res);
> - if (err) {
> - platform_device_put(pdev[i]);
> - pdev[i] = NULL;
> - continue;
> + err = platform_device_add_resources(pdev[i], &res, 1);
> + if (err)
> + goto exit_device_put;
> }
>
> - err = platform_device_add_resources(pdev[i], &res, 1);
> - if (err)
> - goto exit_device_put;
> -
> /* platform_device_add calls probe() */
> err = platform_device_add(pdev[i]);
> if (err)
>
Powered by blists - more mailing lists