linux-kernel - Re: [RFC PATCH] tools/power turbostat: Do not print negative LPI residency

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] [day] [month] [year] [list]

Message-ID: <CAJvTdKkFLTzWdU7bssUSJkTW4ocfKcFEu5c+8m=EVNZ44+iXug@mail.gmail.com>
Date:   Mon, 27 Nov 2023 20:10:50 -0500
From:   Len Brown <lenb@...nel.org>
To:     Chen Yu <yu.c.chen@...el.com>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Todd Brandt <todd.e.brandt@...el.com>
Subject: Re: [RFC PATCH] tools/power turbostat: Do not print negative LPI residency

BIOS bugs:-(

I agree that printing 0 is an improvement over printing an insane
negative number.

But printing 0 suggests that there was no residency, and that could be
misleading...

Maybe we should output some kind of warning about the broken BIOS?

On Sun, Oct 22, 2023 at 1:53 AM Chen Yu <yu.c.chen@...el.com> wrote:
>
> turbostat prints the abnormal SYS%LPI across suspend-to-idle:
> SYS%LPI = 114479815993277.50
>
> This is reproduced by:
> Run a freeze cycle, e.g. "sleepgraph -m freeze -rtcwake 15".
> Then do a reboot. After boot up, launch the suspend-idle-idle
> and check the SYS%LPI field.
>
> The slp_so residence counter is in LPIT table, and BIOS does not
> clears this register across reset. The PMC expects the OS to calculate
> the LPI residency based on the delta. However, there is an firmware
> issue that the LPIT gets cleared to 0 during the second suspend
> to idle after the reboot, which brings negative delta value.
>
> Prints a simple 0 to indicate this error to not confuse the user.
>
> Reported-by: Todd Brandt <todd.e.brandt@...el.com>
> Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 9a10512e3407..3fa5f9a0218a 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -1472,8 +1472,16 @@ int delta_package(struct pkg_data *new, struct pkg_data *old)
>         old->pc8 = new->pc8 - old->pc8;
>         old->pc9 = new->pc9 - old->pc9;
>         old->pc10 = new->pc10 - old->pc10;
> -       old->cpu_lpi = new->cpu_lpi - old->cpu_lpi;
> -       old->sys_lpi = new->sys_lpi - old->sys_lpi;
> +       if (new->cpu_lpi > old->cpu_lpi) {
> +               old->cpu_lpi = new->cpu_lpi - old->cpu_lpi;
> +       } else {
> +               old->cpu_lpi = 0;
> +       }
> +       if (new->sys_lpi > old->sys_lpi) {
> +               old->sys_lpi = new->sys_lpi - old->sys_lpi;
> +       } else {
> +               old->sys_lpi = 0;
> +       }
>         old->pkg_temp_c = new->pkg_temp_c;
>
>         /* flag an error when rc6 counter resets/wraps */
> --
> 2.25.1
>


-- 
Len Brown, Intel