linux-kernel - Re: [PATCH v2] tools/power turbostat: Fix RAPL summary collection on AMD processors

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 for Android: free password hash cracker in your pocket

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

Message-ID: <4cbb1eff77de1e843912267ade4686cfa1acd610.camel@kepstin.ca>
Date:   Tue, 20 Apr 2021 09:28:06 -0400
From:   Calvin Walton <calvin.walton@...stin.ca>
To:     Chen Yu <yu.c.chen@...el.com>, Borislav Petkov <bp@...e.de>
Cc:     Terry Bowman <terry.bowman@....com>, lenb@...nel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        wei.huang2@....com, aros@....com
Subject: Re: [PATCH v2] tools/power turbostat: Fix RAPL summary collection
 on AMD processors

On Tue, 2021-04-20 at 21:15 +0800, Chen Yu wrote:
> 
> Okay. I would vote for the the patch from Bas as it was a combined
> work from two
> authors and tested by several AMD users. But let me paste it here too
> for Artem to
> see if this also works for him:
> 
> 
> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00
> 2001
> From: Bas Nieuwenhuizen <bas@...nieuwenhuizen.nl>
> Date: Fri, 12 Mar 2021 21:27:40 +0800
> Subject: [PATCH] tools/power/turbostat: Fix turbostat for AMD Zen
> CPUs
> 
> 
> @@ -297,7 +297,10 @@ int idx_to_offset(int idx)
>  
>         switch (idx) {
>         case IDX_PKG_ENERGY:
> -               offset = MSR_PKG_ENERGY_STATUS;
> +               if (do_rapl & RAPL_AMD_F17H)
> +                       offset = MSR_PKG_ENERGY_STAT;
> +               else
> +                       offset = MSR_PKG_ENERGY_STATUS;
>                 break;
>         case IDX_DRAM_ENERGY:
>                 offset = MSR_DRAM_ENERGY_STATUS;

This patch has the same issue I noticed with the initial revision of
Terry's patch - the idx_to_offset function returns type int (32-bit
signed), but MSR_PKG_ENERGY_STAT is greater than INT_MAX (or rather,
would be interpreted as a negative number)

The end result is, as far as I can tell, that it hits the if (offset <
0) check in update_msr_sum() resulting in the timer callback for
updating the stat in the background when long durations are used to not
happen.

For short durations it still works fine since the background update
isn't used.


-- 
Calvin Walton <calvin.walton@...stin.ca>