6.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhang Rui rui.zhang@intel.com
[ Upstream commit b312d880fb462d4759396950865ec914de9d253c ]
turbostat aborted with below messages on a dual-package system,
turbostat: turbostat.c:3744: rapl_counter_accumulate: Assertion `dst->unit == src->unit' failed. Aborted
This is because 1. the MSR_DRAM_PERF_STATUS returns Zero for one package, and non-Zero for another package 2. probe_msr() treats Zero return value as a failure so this feature is enabled on one package, and disabled for another package. 3. turbostat aborts because the feature is invalid on some package
Unlike the RAPL energy counter registers, MSR_DRAM_PERF_STATUS can return Zero value, and this should not be treated as a failure.
Fix the problem by allowing Zero return value for RAPL registers other than the energy counters.
Fixes: 7c6fee25bdf5 ("tools/power turbostat: Check for non-zero value when MSR probing") Reported-by: Artem Bityutskiy artem.bityutskiy@intel.com Signed-off-by: Zhang Rui rui.zhang@intel.com Signed-off-by: Len Brown len.brown@intel.com Signed-off-by: Sasha Levin sashal@kernel.org --- tools/power/x86/turbostat/turbostat.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 8d5011a0bf60d..ff238798b7747 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -2211,7 +2211,7 @@ int get_msr(int cpu, off_t offset, unsigned long long *msr) return 0; }
-int probe_msr(int cpu, off_t offset) +int probe_rapl_msr(int cpu, off_t offset, int index) { ssize_t retval; unsigned long long value; @@ -2220,13 +2220,22 @@ int probe_msr(int cpu, off_t offset)
retval = pread(get_msr_fd(cpu), &value, sizeof(value), offset);
- /* - * Expect MSRs to accumulate some non-zero value since the system was powered on. - * Treat zero as a read failure. - */ - if (retval != sizeof(value) || value == 0) + /* if the read failed, the probe fails */ + if (retval != sizeof(value)) return 1;
+ /* If an Energy Status Counter MSR returns 0, the probe fails */ + switch (index) { + case RAPL_RCI_INDEX_ENERGY_PKG: + case RAPL_RCI_INDEX_ENERGY_CORES: + case RAPL_RCI_INDEX_DRAM: + case RAPL_RCI_INDEX_GFX: + case RAPL_RCI_INDEX_ENERGY_PLATFORM: + if (value == 0) + return 1; + } + + /* PKG,DRAM_PERF_STATUS MSRs, can return any value */ return 0; }
@@ -7896,7 +7905,7 @@ void rapl_perf_init(void) rci->flags[cai->rci_index] = cai->flags;
/* Use MSR for this counter */ - } else if (!no_msr && cai->msr && probe_msr(cpu, cai->msr) == 0) { + } else if (!no_msr && cai->msr && probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) { rci->source[cai->rci_index] = COUNTER_SOURCE_MSR; rci->msr[cai->rci_index] = cai->msr; rci->msr_mask[cai->rci_index] = cai->msr_mask; @@ -8034,7 +8043,7 @@ void msr_perf_init_(void) cai->present = true;
/* User MSR for this counter */ - } else if (!no_msr && cai->msr && probe_msr(cpu, cai->msr) == 0) { + } else if (!no_msr && cai->msr && probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) { cci->source[cai->rci_index] = COUNTER_SOURCE_MSR; cci->msr[cai->rci_index] = cai->msr; cci->msr_mask[cai->rci_index] = cai->msr_mask; @@ -8148,7 +8157,7 @@ void cstate_perf_init_(bool soft_c1)
/* User MSR for this counter */ } else if (!no_msr && cai->msr && pkg_cstate_limit >= cai->pkg_cstate_limit - && probe_msr(cpu, cai->msr) == 0) { + && probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) { cci->source[cai->rci_index] = COUNTER_SOURCE_MSR; cci->msr[cai->rci_index] = cai->msr; }