linux-kernel - Re: [PATCH v2 2/3] hwmon: (asus_wmi_ec

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 10 Oct 2021 20:45:13 +0200
From:   Eugene Shalygin <eugene.shalygin@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Denis Pauk <pauk.denis@...il.com>, andy.shevchenko@...il.com,
        Jean Delvare <jdelvare@...e.com>, linux-kernel@...r.kernel.org,
        linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2 2/3] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI.

Günter,

On Sun, 10 Oct 2021 at 16:33, Guenter Roeck <linux@...ck-us.net> wrote:
> > Why do you believe they are more reliable? How does it cover more motherboards?
> >
>
> You said yourself below: "I know the naive reading from the ACPI EC registers
> leads to problems (fans get stuck, etc.)".

I also know what the WMI functions (BREC, RSIO, WSIO, RHWM, ) do, as I
see their source. They lock the mutex (\AMW0.ASMX) and through
multi-level mapping access EC or Super I/O chip registers. I know all
the hardware access WMI functions begin with acquiring the same mutex,
and those accessing the SIO acquire one more mutex
(\_SB.PCI0.SBRG.SIO1.MUT0). Thus locking the same mutex and accessing
the hardware directly is safe.

By naive I meant reading EC registers without acquiring the ACPI mutex.

> Something in the WMI code is obviously broken and, ultimately, will need
> to get fixed. I don't know if that something is on the ASUS side or on the
> kernel side, or on the interface between the two. A single WMI call taking
> 1 second is way too long and strongly suggests that some timeout is involved.
> Not using WMI because of that just seems wrong.

On that machine a single reading of the EC register (i.e. a call to
ec_read()) takes approx. 14 ms. The timeout is probably right here.

>From that the 990 ms delay of BREC is understandable, because for each
register it performs 4 EC transactions: read current bank,
unconditionally switch current bank, read register, switch to the old
bank. When I ask it to read 14 registers, it needs 42 ec_transaction()
calls which would sum up to 600 ms alone. In the new approach I switch
EC banks only when needed (currently only 2 times including rollback
to the previous bank) and save a lot of ec_transaction() calls. It
would help, however, to extend the EC interface in the acpi/ec.c to
allow for block reads in a single transaction.

I don't know why ec_transaction() takes so long, but in any case the
API of the WMI BREC function dictates it to perform slowly because it
needs to do a bank switch for each item in the input array, while the
direct reading allows us to avoid that bloating.

Regards,
Eugene