[PATCH AUTOSEL 6.18] tools/power turbostat: Fix --show/--hide for individual cpuidle counters

[PATCH AUTOSEL 6.18] tools/power turbostat: Fix --show/--hide for individual cpuidle counters

[Sasha Levin]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

[ Upstream commit b6398bc2ef3a78f1be37ba01ae0a5eedaee47803 ]

Problem: individual swidle counter names (C1, C1+, C1-, etc.) cannot be
selected via --show/--hide due to two bugs in probe_cpuidle_counts():
1. The function returns immediately when BIC_cpuidle is not enabled,
   without checking deferred_add_index.
2. The deferred name check runs against name_buf before the trailing
   newline is stripped, so is_deferred_add("C1\n") never matches "C1".

Fix:
1. Relax the early return to pass through when deferred names are
   queued.
2. Strip the trailing newline from name_buf before performing deferred
   name checks.
3. Check each suffixed variant (C1+, C1, C1-) individually so that
   e.g. "--show C1+" enables only the requested metric.

In addition, introduce a helper function to avoid repeating the
condition (readability cleanup).

Fixes: ec4acd3166d8 ("tools/power turbostat: disable "cpuidle" invocation counters, by default")
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Error: Failed to generate final synthesis

 tools/power/x86/turbostat/turbostat.c | 35 ++++++++++++++++-----------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 67dfd3eaad014..48677f1846347 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -10890,6 +10890,14 @@ void probe_cpuidle_residency(void)
 	}
 }
 
+static bool cpuidle_counter_wanted(char *name)
+{
+	if (is_deferred_skip(name))
+		return false;
+
+	return DO_BIC(BIC_cpuidle) || is_deferred_add(name);
+}
+
 void probe_cpuidle_counts(void)
 {
 	char path[64];
@@ -10899,7 +10907,7 @@ void probe_cpuidle_counts(void)
 	int min_state = 1024, max_state = 0;
 	char *sp;
 
-	if (!DO_BIC(BIC_cpuidle))
+	if (!DO_BIC(BIC_cpuidle) && !deferred_add_index)
 		return;
 
 	for (state = 10; state >= 0; --state) {
@@ -10914,12 +10922,6 @@ void probe_cpuidle_counts(void)
 
 		remove_underbar(name_buf);
 
-		if (!DO_BIC(BIC_cpuidle) && !is_deferred_add(name_buf))
-			continue;
-
-		if (is_deferred_skip(name_buf))
-			continue;
-
 		/* truncate "C1-HSW\n" to "C1", or truncate "C1\n" to "C1" */
 		sp = strchr(name_buf, '-');
 		if (!sp)
@@ -10934,16 +10936,19 @@ void probe_cpuidle_counts(void)
 			 * Add 'C1+' for C1, and so on. The 'below' sysfs file always contains 0 for
 			 * the last state, so do not add it.
 			 */
-
 			*sp = '+';
 			*(sp + 1) = '\0';
-			sprintf(path, "cpuidle/state%d/below", state);
-			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			if (cpuidle_counter_wanted(name_buf)) {
+				sprintf(path, "cpuidle/state%d/below", state);
+				add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			}
 		}
 
 		*sp = '\0';
-		sprintf(path, "cpuidle/state%d/usage", state);
-		add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+		if (cpuidle_counter_wanted(name_buf)) {
+			sprintf(path, "cpuidle/state%d/usage", state);
+			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+		}
 
 		/*
 		 * The 'above' sysfs file always contains 0 for the shallowest state (smallest
@@ -10952,8 +10957,10 @@ void probe_cpuidle_counts(void)
 		if (state != min_state) {
 			*sp = '-';
 			*(sp + 1) = '\0';
-			sprintf(path, "cpuidle/state%d/above", state);
-			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			if (cpuidle_counter_wanted(name_buf)) {
+				sprintf(path, "cpuidle/state%d/above", state);
+				add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			}
 		}
 	}
 }
-- 
2.53.0

[PATCH AUTOSEL 7.0-5.10] power: supply: sbs-manager: normalize return value of gpio_get

[Sasha Levin]

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

[ Upstream commit 5c2ffc0b215a884dbc961d4737f636067348b8bd ]

The GPIO get callback is expected to return 0 or 1 (or a negative error
code). Ensure that the value returned by sbsm_gpio_get_value() is
normalized to the [0, 1] range.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Link: https://patch.msgid.link/aZYoL2MnTYU5FuQh@google.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a complete understanding. Let me compile the analysis.

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1 - Subject Line Parsing:**
Record: Subsystem: `power: supply: sbs-manager` | Action verb:
"normalize" | Summary: Normalize the return value of the GPIO `get`
callback to the [0, 1] range.

**Step 1.2 - Commit Tags:**
Record:
- Signed-off-by: Dmitry Torokhov (author)
- Reviewed-by: Linus Walleij (GPIO maintainer)
- Reviewed-by: Bartosz Golaszewski (GPIO maintainer)
- Link: https://patch.msgid.link/aZYoL2MnTYU5FuQh@google.com
- Signed-off-by: Sebastian Reichel (power supply maintainer)
- NO Fixes: tag, NO Cc: stable tag (unlike sibling patches)

**Step 1.3 - Commit Body Analysis:**
Record: Bug description: The GPIO `.get()` callback is contractually
required to return 0, 1, or a negative error code.
`sbsm_gpio_get_value()` was returning `ret & BIT(off)`, which yields
`BIT(off)` = 1, 2, 4, 8 for `off` = 0, 1, 2, 3 respectively. Values of
2, 4, 8 violate the API contract. The fix applies `!!()` to normalize.

**Step 1.4 - Hidden Bug Fix Detection:**
Record: "Normalize" is a bug-fix verb. This is a real bug fix disguised
as a "normalization" — the driver's GPIO callback was violating the
gpio_chip API contract.

## PHASE 2: DIFF ANALYSIS

**Step 2.1 - Inventory:**
Record: Single file `drivers/power/supply/sbs-manager.c`, 1 line changed
(1+/1-). Function affected: `sbsm_gpio_get_value()`. Scope: surgical.

**Step 2.2 - Code Flow:**
Record:
- Before: `return ret & BIT(off)` returns `BIT(off)` value (1, 2, 4, 8)
  when the bit is set
- After: `return !!(ret & BIT(off))` returns 0 or 1 as required by the
  API

**Step 2.3 - Bug Mechanism:**
Record: Category (g) Logic / correctness fix — API contract violation.
The `ngpio = SBSM_MAX_BATS = 4`, so `off` takes values 0-3 corresponding
to 4 smart batteries. For `off=0`, `BIT(0)=1` happens to be valid. For
`off=1,2,3`, the raw return is 2, 4, 8 — invalid.

**Step 2.4 - Fix Quality:**
Record: The fix is obviously correct. `!!()` is the idiomatic C
conversion to 0/1 boolean. No regression risk.

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1 - Blame:**
Record: The buggy code was present since the driver was introduced in
commit `dbc4deda03fe6` ("power: Adds support for Smart Battery System
Manager") in v4.15 (2017). The bug was latent for years.

**Step 3.2 - Follow Fixes: (None in this commit, but related commit):**
Record: The gpiolib wrapper that makes the invalid return value actually
matter is commit `86ef402d805d` ("gpiolib: sanitize the return value of
gpio_chip::get()") in v6.15-rc1. This wrapper rejects any value > 1 by
returning -EBADE.

**Step 3.3 - File History:**
Record: Recent changes to sbs-manager.c are minor (probe conversions,
fwnode updates). No prerequisites for this fix.

**Step 3.4 - Author Context:**
Record: Dmitry Torokhov submitted 3 sibling commits across multiple
subsystems for the SAME class of bug:
- `e2fa075d5ce19 iio: adc: ti-ads7950: normalize return value of
  gpio_get` (with Fixes: + Cc: stable)
- `2bb995e6155cb net: phy: qcom: qca807x: normalize return value of
  gpio_get` (with Fixes:)
- `5c2ffc0b215a8 power: supply: sbs-manager: normalize return value of
  gpio_get` (this commit, no Fixes/stable)

**Step 3.5 - Dependencies:**
Record: No dependencies. Standalone fix.

## PHASE 4: MAILING LIST RESEARCH

**Step 4.1 - Original Discussion:**
Record: `b4 dig -c 5c2ffc0b215a8` found the thread at
https://lore.kernel.org/all/aZYoL2MnTYU5FuQh@google.com/. Single patch
(v1), two positive Reviewed-by responses, applied by the power supply
maintainer.

**Step 4.2 - Reviewers:**
Record: Linus Walleij (GPIO maintainer), Bartosz Golaszewski (GPIO
maintainer) both Reviewed-by. Sebastian Reichel (power supply
maintainer) applied. Appropriate maintainer review coverage.

**Step 4.3 - Bug Report:**
Record: No external bug report. The fix was found as part of Dmitry
Torokhov auditing drivers for return value compliance after the sanitize
wrapper exposed the issue.

**Step 4.4 - Related Patches:**
Record: Part of a broader effort by Dmitry to fix drivers that violated
the new API contract. See sibling patches above.

**Step 4.5 - Stable Discussion:**
Record: No explicit stable discussion for this specific commit. However,
the CLOSELY related commit `ec2cceadfae72` ("gpiolib: normalize the
return value of gc->get() on behalf of buggy drivers") explicitly has
`Cc: stable@vger.kernel.org` and `Fixes: 86ef402d805d`, acknowledging
that the sanitize change broke multiple drivers. That commit references
`aZSkqGTqMp_57qC7@google.com` as a closes link and was co-reported by
Dmitry.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1 - Key Functions:**
Record: `sbsm_gpio_get_value()` — assigned to `gc->get` at
`drivers/power/supply/sbs-manager.c:287`.

**Step 5.2 - Callers:**
Record: Called via the gpiolib layer `gpiochip_get()` which is called
from `gpio_chip_get_value()` and `gpio_chip_get_multiple()`. Any
consumer that calls `gpiod_get_value()` on these GPIO lines routes
through this function.

**Step 5.3 - Callees:**
Record: Calls `sbsm_read_word()` which performs an SMBus read on the
hardware.

**Step 5.4 - Reachability:**
Record: Reachable from userspace via GPIO character device ioctls
(GPIO_V2_LINE_VALUES_IOCTL), sysfs GPIO interface, or any in-kernel
consumer. With the SBS manager hardware present and a userspace tool
like `gpioget`, the buggy path is trivially reached.

**Step 5.5 - Similar Patterns:**
Record: Sibling fixes `e2fa075d5ce19` (ti-ads7950), `2bb995e6155cb`
(qca807x). The qca807x fix is ALREADY in stable 6.17.y
(`cb8f0a3857386`), 6.18.y, 6.19.y — establishing precedent that this
class of fix is appropriate for stable.

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

**Step 6.1 - Buggy Code in Stable:**
Record: `sbsm_gpio_get_value()` with the buggy `ret & BIT(off)` exists
identically in 6.17.y, 6.18.y, 6.19.y stable trees. For older trees
(5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y), the gpiolib wrapper
`86ef402d805d` is NOT present, so the bug is latent (API contract
violation without functional consequence).

**Step 6.2 - Backport Complications:**
Record: The patch applies cleanly to 6.17.y, 6.18.y, 6.19.y (verified
code is identical). For older stable trees, the patch would also apply
but the functional benefit is minimal until the wrapper lands there.

**Step 6.3 - Related Fixes in Stable:**
Record: Qca807x normalize fix IS already in 6.17.y (`cb8f0a3857386`),
6.18.y, 6.19.y (`554e8f2fbce86`). The alternative "normalize-on-behalf"
wrapper fix `ec2cceadfae72` has `Cc: stable` but has not yet landed in
these stable trees (verified: all three still have the EBADE-returning
version).

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1 - Subsystem:**
Record: `drivers/power/supply/` — power supply (battery) subsystem.
PERIPHERAL criticality (specific hardware), but affects laptops/embedded
systems with SBS-compliant multi-battery setups.

**Step 7.2 - Activity:**
Record: sbs-manager is a mature, low-activity driver (since 2017). Used
in real products with LTC1760 and similar chips.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1 - Affected Users:**
Record: Users of SBS-compliant smart battery manager chips (LTC1760 and
others). Target: laptops, industrial embedded devices with multiple hot-
swappable batteries. Specifically affected on kernels 6.15+ that have
the gpiolib sanitize wrapper.

**Step 8.2 - Trigger:**
Record: Triggered automatically whenever userspace (or any in-kernel
consumer) reads the state of battery 2, 3, or 4 via the exposed GPIO
lines on an SBS manager. For battery 1 (`off=0`), `BIT(0)=1` escapes the
check. Trigger is deterministic — not a race.

**Step 8.3 - Failure Mode Severity:**
Record: On stable trees 6.17.y/6.18.y/6.19.y that have the EBADE
wrapper, reading batteries 2-4 returns `-EBADE` instead of the actual
state. Userspace tools that read these GPIOs see errors. This is a
functional regression: MEDIUM severity (broken hardware functionality,
but not a crash/corruption).

**Step 8.4 - Risk-Benefit:**
Record:
- BENEFIT: Restores correct GPIO reporting for SBS multi-battery systems
  on 6.17+.
- RISK: Extremely low — single-line `!!` normalization, impossible to
  regress correctness. Returns 0 or 1 as API requires.
- Ratio: Very favorable — trivial risk, real user benefit.

## PHASE 9: FINAL SYNTHESIS

**Step 9.1 - Evidence Compilation:**

FOR backporting:
- Fixes a real functional regression on stable 6.17.y/6.18.y/6.19.y
  (batteries 2-4 return -EBADE)
- One-line, obviously correct `!!` normalization
- Reviewed by both GPIO maintainers (Linus Walleij, Bartosz Golaszewski)
- Applied by power supply maintainer (Sebastian Reichel)
- Precedent: sibling commit for qca807x (same fix pattern) is already in
  6.17.y, 6.18.y, 6.19.y stable
- Applies cleanly (identical code in stable trees)

AGAINST backporting:
- Original submission had no `Fixes:` tag or `Cc: stable` (unlike
  sibling ti-ads7950 patch)
- On older stable trees (<6.15) without the gpiolib wrapper, the bug is
  purely theoretical

UNRESOLVED: Whether `ec2cceadfae72` will be backported first and make
this technically unnecessary — but even so, this patch properly fixes
the driver rather than relying on a workaround.

**Step 9.2 - Stable Rules Checklist:**
1. Obviously correct and tested: YES — trivial `!!`, reviewed by
   maintainers
2. Real bug: YES — -EBADE returned for real GPIO reads on 6.17+
3. Important issue: YES (MEDIUM) — broken functionality on SBS multi-
   battery systems
4. Small and contained: YES — 1 line
5. No new features: YES
6. Applies to stable: YES

**Step 9.3 - Exception Categories:**
Not a device-ID/quirk addition, but it's a small, surgical correctness
fix. Meets core stable criteria on its own merits.

**Step 9.4 - Decision:**
The patch fixes a real regression on stable kernels 6.17.y, 6.18.y, and
6.19.y (which contain the gpiolib sanitize wrapper from v6.15). It's a
one-line, obviously correct normalization endorsed by two GPIO subsystem
maintainers and the power supply maintainer. The sibling qca807x fix has
already been accepted into these same stable trees, establishing clear
precedent. For older stable trees lacking the wrapper, the fix is still
beneficial as a correctness improvement with zero risk.

## Verification

- [Phase 1] Parsed tags: confirmed author, two Reviewed-by entries from
  GPIO maintainers, applied by power supply maintainer; no Fixes/Cc-
  stable tags (unlike sibling ti-ads7950 patch)
- [Phase 2] Diff analysis: verified in original commit that exactly one
  line changes `ret & BIT(off)` to `!!(ret & BIT(off))`
- [Phase 2] Verified `ngpio = SBSM_MAX_BATS = 4` via Read of
  `drivers/power/supply/sbs-manager.c`, confirming off = 0..3 and
  BIT(off) = 1, 2, 4, 8
- [Phase 3] `git log` showed driver introduced in `dbc4deda03fe6`
  (v4.15, 2017), buggy code present since inception
- [Phase 3] `git log --grep="normalize return value"` found 2 sibling
  Dmitry Torokhov commits (ti-ads7950 with `Cc: Stable`, qca807x with
  `Fixes: 86ef402d805d`)
- [Phase 3] `git show 86ef402d805d` confirmed gpiolib wrapper returns
  `-EBADE` for ret > 1
- [Phase 3] `git describe --contains 86ef402d805d` → `v6.15-rc1` —
  confirms wrapper is in v6.15+
- [Phase 4] `b4 dig -c 5c2ffc0b215a8` located thread at
  lore.kernel.org/all/aZYoL2MnTYU5FuQh@google.com/
- [Phase 4] Read `/tmp/sbsm_thread.mbox`: confirmed single-version
  submission, positive reviews, quick application
- [Phase 4] Identified related commit `ec2cceadfae72` with `Cc:
  stable@vger.kernel.org` acknowledging the wrapper broke drivers
- [Phase 5] Confirmed `sbsm_gpio_get_value` is assigned to `gc->get` and
  reachable from userspace GPIO APIs
- [Phase 6] Verified with `git show stable-
  push/linux-6.17.y:drivers/gpio/gpiolib.c` that 6.17.y has the EBADE-
  returning wrapper
- [Phase 6] Verified same for 6.18.y and 6.19.y
- [Phase 6] Verified 5.10/5.15/6.1/6.6/6.12 do NOT have the wrapper (bug
  is latent there)
- [Phase 6] Verified via `git show stable-
  push/linux-6.17.y:drivers/power/supply/sbs-manager.c` that buggy `ret
  & BIT(off)` is present
- [Phase 6] Verified qca807x sibling fix is already in
  6.17.y/6.18.y/6.19.y (`cb8f0a3857386`, `554e8f2fbce86`)
- [Phase 6] Verified `ec2cceadfae72` alternative fix is NOT yet in those
  stable trees
- [Phase 8] Failure mode: on 6.17/6.18/6.19 the wrapper converts
  BIT(1/2/3) to -EBADE — verified by reading the wrapper implementation

**YES**

 drivers/power/supply/sbs-manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index 6fe526222f7f4..343ad4ab4082c 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -199,7 +199,7 @@ static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned int off)
 	if (ret < 0)
 		return ret;
 
-	return ret & BIT(off);
+	return !!(ret & BIT(off));
 }
 
 /*
-- 
2.53.0

[PATCH AUTOSEL 6.18] tools/power/turbostat: Fix microcode patch level output for AMD/Hygon

[Sasha Levin]

From: Serhii Pievniev <spevnev16@gmail.com>

[ Upstream commit a444083286434ec1fd127c5da11a3091e6013008 ]

turbostat always used the same logic to read the microcode patch level,
which is correct for Intel but not for AMD/Hygon.
While Intel stores the patch level in the upper 32 bits of MSR, AMD
stores it in the lower 32 bits, which causes turbostat to report the
microcode version as 0x0 on AMD/Hygon.

Fix by shifting right by 32 for non-AMD/Hygon, preserving the existing
behavior for Intel and unknown vendors.

Fixes: 3e4048466c39 ("tools/power turbostat: Add --no-msr option")
Signed-off-by: Serhii Pievniev <spevnev16@gmail.com>
Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Error: Failed to generate final synthesis

 tools/power/x86/turbostat/turbostat.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 1b5ca2f4e92ff..67dfd3eaad014 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -8842,10 +8842,13 @@ void process_cpuid()
 	edx_flags = edx;
 
 	if (!no_msr) {
-		if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch))
+		if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch)) {
 			warnx("get_msr(UCODE)");
-		else
+		} else {
 			ucode_patch_valid = true;
+			if (!authentic_amd && !hygon_genuine)
+				ucode_patch >>= 32;
+		}
 	}
 
 	/*
@@ -8860,7 +8863,7 @@ void process_cpuid()
 		fprintf(outf, "CPUID(1): family:model:stepping 0x%x:%x:%x (%d:%d:%d)",
 			family, model, stepping, family, model, stepping);
 		if (ucode_patch_valid)
-			fprintf(outf, " microcode 0x%x", (unsigned int)((ucode_patch >> 32) & 0xFFFFFFFF));
+			fprintf(outf, " microcode 0x%x", (unsigned int)ucode_patch);
 		fputc('\n', outf);
 
 		fprintf(outf, "CPUID(0x80000000): max_extended_levels: 0x%x\n", max_extended_level);
-- 
2.53.0

Re: [PATCH AUTOSEL 6.18] tools/power/turbostat: Fix microcode patch level output for AMD/Hygon

[Christian Ludloff]

> + if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch)) {

Intel prescribes a sequence of WRMSR(0x8B,0) + CPUID(1) + RDMSR.

This goes back to the original P6, where the upper half of the
BBL_CR_D3 L2 cache test register was abused to report the microcode
revision, and CPUID was abused to actually load the revision into said
MSR. Afaik the WRMSR(0x8B,0) is still required today.

turbostat has the prescribed CPUID(1) – by chance, a few lines earlier
– but it lacks the prescribed WRMSR(0x8B,0).

Also, note that the prescribed Intel sequence is neither required for
AMD, nor actually works for AMD – there the MSR is read-only.

--
C.

Re: [PATCH AUTOSEL 6.18] tools/power/turbostat: Fix microcode patch level output for AMD/Hygon

[Serhii Pievniev]

> Intel prescribes a sequence of WRMSR(0x8B,0) + CPUID(1) + RDMSR.
>
> This goes back to the original P6, where the upper half of the
> BBL_CR_D3 L2 cache test register was abused to report the microcode
> revision, and CPUID was abused to actually load the revision into said
> MSR. Afaik the WRMSR(0x8B,0) is still required today.

The latest Intel SDM [1] confirms that CPUID(1) is still required and
WRMSR(0x8B, 0) is recommended.

> turbostat has the prescribed CPUID(1) – by chance, a few lines earlier
> – but it lacks the prescribed WRMSR(0x8B,0).
>
> Also, note that the prescribed Intel sequence is neither required for
> AMD, nor actually works for AMD – there the MSR is read-only.

I agree that WRMSR should be added, however it is a separate issue and
should be addressed in a separate patch.

[1] https://cdrdv2-public.intel.com/916746/335592-091-sdm-vol-4.pdf