Re: [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms

[qinyuntan <qinyuntan@linux.alibaba.com>]

Hi Len,

Thank you for the quick response and for applying the fix to your tree.

I have tested the commit (16cc8f249c70) on our AMD platforms, and this 
issue is resolved. The turbostat tool now works correctly without the 
"msr offset 0x611 read failed" error.

Best regards,
Qinyun Tan

On 2/14/26 2:11 AM, Len Brown wrote:
> Thanks for noticing, reporting, and suggesting a fix!
> 
> Please let me know if this fix already in my tree does not address the
> issue for you.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git/commit/? 
> h=turbostat&id=16cc8f249c702b7cbb4c2c2be7cd8f4fdd5d1d0c
> 
> -Len
> 
> On Thu, Jan 29, 2026 at 8:54 PM Qinyun Tan<qinyuntan@linux.alibaba.com> wrote:
>> idx_to_offset() uses valid_rapl_msrs to determine which MSR address to
>> return for RAPL counters (Intel's MSR_PKG_ENERGY_STATUS at 0x611 vs AMD's
>> MSR_PKG_ENERGY_STAT at 0xc001029b).
>>
>> However, probe_rapl_msrs() calls idx_to_offset() before valid_rapl_msrs
>> has been set - it's only assigned at the end of probe_rapl_msrs() after
>> successful MSR validation. This causes idx_to_offset() to always return
>> the Intel MSR address (0x611) during the probe phase, even on AMD systems.
>>
>> On Intel platforms this works by coincidence since the default (else
>> branch) returns the correct Intel address. On AMD platforms, this causes
>> turbostat to fail with:
>>
>>    turbostat: cpu0: msr offset 0x611 read failed: Input/output error
>>
>> Fix this by:
>> 1. Falling back to platform->plat_rapl_msrs when valid_rapl_msrs is zero
>>     (not yet validated). This ensures the correct platform-specific MSR
>>     address is used during the initial probe.
>> 2. Changing valid_rapl_msrs type from 'unsigned int' to 'int' to match
>>     plat_rapl_msrs type and avoid sign comparison warnings.
>>
>> Fixes: 19476a592bf2 ("tools/power turbostat: Validate RAPL MSRs for AWS Nitro Hypervisor")
>> Signed-off-by: Qinyun Tan<qinyuntan@linux.alibaba.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 5ad45c2ac5bd..449492d5e043 100644
>> --- a/tools/power/x86/turbostat/turbostat.c
>> +++ b/tools/power/x86/turbostat/turbostat.c
>> @@ -492,7 +492,7 @@ unsigned int quiet;
>>   unsigned int shown;
>>   unsigned int sums_need_wide_columns;
>>   unsigned int rapl_joules;
>> -unsigned int valid_rapl_msrs;
>> +int valid_rapl_msrs;
>>   unsigned int summary_only;
>>   unsigned int list_header_only;
>>   unsigned int dump_only;
>> @@ -2132,10 +2132,18 @@ struct msr_sum_array *per_cpu_msr_sum;
>>   off_t idx_to_offset(int idx)
>>   {
>>          off_t offset;
>> +       int rapl_msrs;
>> +
>> +       /*
>> +        * Use valid_rapl_msrs if available (non-zero), otherwise fall back
>> +        * to platform->plat_rapl_msrs. This allows probe_rapl_msrs() to call
>> +        * this function before valid_rapl_msrs has been set.
>> +        */
>> +       rapl_msrs = valid_rapl_msrs ? valid_rapl_msrs : platform->plat_rapl_msrs;
>>
>>          switch (idx) {
>>          case IDX_PKG_ENERGY:
>> -               if (valid_rapl_msrs & RAPL_AMD_F17H)
>> +               if (rapl_msrs & RAPL_AMD_F17H)
>>                          offset = MSR_PKG_ENERGY_STAT;
>>                  else
>>                          offset = MSR_PKG_ENERGY_STATUS;
>> --
>> 2.43.5
>>