mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms
@ 2026-01-30  2:53 Qinyun Tan
  2026-02-13 18:11 ` Len Brown
  0 siblings, 1 reply; 3+ messages in thread
From: Qinyun Tan @ 2026-01-30  2:53 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-pm, linux-kernel, Xunlei Pang, oliver.yang, Qinyun Tan

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms
  2026-01-30  2:53 [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms Qinyun Tan
@ 2026-02-13 18:11 ` Len Brown
  2026-02-24  6:14   ` qinyuntan
  0 siblings, 1 reply; 3+ messages in thread
From: Len Brown @ 2026-02-13 18:11 UTC (permalink / raw)
  To: Qinyun Tan; +Cc: linux-pm, linux-kernel, Xunlei Pang, oliver.yang

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
>
>


-- 
Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms
  2026-02-13 18:11 ` Len Brown
@ 2026-02-24  6:14   ` qinyuntan
  0 siblings, 0 replies; 3+ messages in thread
From: qinyuntan @ 2026-02-24  6:14 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-pm, linux-kernel, Xunlei Pang, oliver.yang

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
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-02-24  6:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-30  2:53 [PATCH V1] tools/power turbostat: Fix RAPL MSR address selection on AMD platforms Qinyun Tan
2026-02-13 18:11 ` Len Brown
2026-02-24  6:14   ` qinyuntan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®