Skip to content

Commit b392886

Browse files
committed
fix(smu): derive the physical CO slot from Linux topology, not an SMU probe
The per-core Curve Optimizer slot was discovered with an SMU "echo probe" (GET each slot, treat an argument-echo as a harvested/disabled slot). That heuristic rests on undocumented GET-on-disabled-slot firmware behavior, and a closed-loop simulation showed it can build a self-consistent WRONG map under plausible response models -- a silent wrong-core CO write that read-back cannot catch, because read-back re-derives the same wrong slot. It is also unnecessary on Linux. The kernel decodes each core's APIC ID (CPUID Fn8000_001E) with the hardware's own field widths and exposes the result as the physical, gap-preserving /proc/cpuinfo "core id" -- so core_id % 8 already IS the physical SMU slot on harvested / multi-CCD parts (5900X, 7900X, 9900X, 5600X, ...), and core_id // 8 the CCD. (Linux-vs-Windows: Windows enumerates cores contiguously, which is why ZenStates/ryzen_monitor read the SMN core-disable fuse to rebuild the slot; Linux hands it to us.) Remove probe_slot_map and the slot-map state; set_topology now only supplies the L3-detected CCD. encode_co_arg already derived slot = core_id % 8; its docstring's "kernel renumbers cores contiguously" rationale (a Windows-ism) is corrected. New tests drive the real encode+write+read-back on harvested 1-CCD and 2-CCD layouts and assert each CO lands on the true physical slot (ablation-verified); the obsolete echo-probe simulation is removed. Residual (inherent to any approach, unchanged): the APIC-core-field == SMU-CO-slot identity is a strong two-source inference (gap positions match on measured 3600X/5900X parts), not a single measured write-test on harvested silicon. Eval: gate-green
1 parent 1b3a9b8 commit b392886

5 files changed

Lines changed: 137 additions & 230 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@ Optimizer tuner for Linux, packaged as a NixOS module with an overlay.
4040
a hard crash there was previously invisible to both crash detectors, so a profile
4141
that crashes only under combined load could be re-applied into the same crash on
4242
every resume.
43-
- Zen 3 Curve Optimizer writes now honor the topology-probed physical core slot, so
44-
a harvested or 2-CCD Zen 3 part (5900X, 5600X) no longer writes the wrong core.
43+
- Per-core Curve Optimizer writes address the correct physical core on harvested and
44+
multi-CCD parts (5900X, 7900X, 9900X, 5600X, ...) by using the physical,
45+
gap-preserving core id Linux exposes (the kernel's own APIC-ID decode); the earlier
46+
SMU slot-probe heuristic was removed as unnecessary on Linux and unreliable
47+
(it depended on undocumented GET-on-disabled-slot firmware behavior).
4548
- Backend pass/fail parsing treats a crash signal (including SIGILL and SIGFPE) as
4649
instability even after an earlier "passed" line, so an unstable offset is never
4750
reported as stable.

docs/hardware.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,13 @@ AMD-specific.
2929
| Zen 5 (Strix Halo) | Ryzen AI Max | -60 to +10 | RSMU | PPT/TDC/EDC | -- | APU, uses Strix Point commands |
3030
| Zen 5 (Shimada Peak) | Zen 5 Threadripper | -60 to +10 | RSMU | PPT/TDC/EDC | Read/Write | Different SMU addresses (`get_co=0xA3`) |
3131

32-
On harvested or multi-CCD parts (5900X 6+6, 5600X 6-of-8, 9900X, ...) the kernel
33-
renumbers cores contiguously while the SMU addresses physical slots; the driver probes
34-
the SMU to build the correct kernel-core to physical-slot mapping so a CO write always
35-
lands on the intended core.
32+
On harvested or multi-CCD parts (5900X 6+6, 5600X 6-of-8, 9900X, ...) the SMU
33+
addresses physical core slots, with gaps where cores are fused off. On Linux the
34+
kernel exposes each core's physical, gap-preserving `core id` (decoded from the APIC
35+
ID via CPUID `Fn8000_001E`), so the driver derives the physical slot directly as
36+
`core id % 8` and the CCD from L3 topology -- a CO write always lands on the intended
37+
core, with no SMU slot-probing. (Windows tools read the SMN core-disable fuse for the
38+
same result because Windows enumerates cores contiguously; Linux gives it to us.)
3639

3740
All generations support PBO scalar read/write (1.0x to 10.0x) and OC mode
3841
enable/disable. SMU features require the

src/smu/commands.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,12 @@ def encode_co_arg(
496496
ccd: Topology-detected CCD index. If provided, used instead of
497497
deriving CCD from ``core_id // 8``. Always prefer passing the
498498
L3-detected CCD from topology when available.
499-
slot: Physical slot index (0-7) within the CCD. On harvested chips
500-
the kernel renumbers cores contiguously, so ``core_id % 8``
501-
targets the wrong SMU slot. When provided, overrides the
502-
``core_id % 8`` fallback.
499+
slot: Physical slot index (0-7) within the CCD, overriding the default
500+
``core_id % 8``. On Linux ``core_id`` is already the physical,
501+
gap-preserving core index (the kernel's own APIC-ID decode), so the
502+
default is exact even on harvested parts; this override exists only
503+
for a caller with a different authoritative source (e.g. an SMN
504+
core-disable fuse map, as the Windows tools use).
503505
"""
504506
commands = get_commands(generation)
505507
if commands is None:
@@ -512,15 +514,14 @@ def encode_co_arg(
512514
margin = value & 0xFFFF
513515

514516
if scheme in ("zen3", "zen4_5"):
515-
# Zen 3/4/5 share the per-core layout: CCD in bits [31:28], core within
516-
# CCD in bits [23:20]. Prefer the topology-detected CCD and the probed
517-
# physical slot; harvested / multi-CCD parts (5900X, 5600X, 9900X, ...)
518-
# renumber kernel core-ids contiguously, so deriving the slot from
519-
# core_id alone targets the WRONG SMU slot — and that write is silent,
520-
# because read-back hits the same wrong slot. Fall back to core_id only
521-
# when topology did not supply the CCD/slot.
522-
# Zen 3 once used (((core_id&8)<<5 | core_id&7)<<20), which is bit-identical
523-
# to this for the core_id-derived case but discarded the ccd/slot args.
517+
# Zen 3/4/5 per-core layout: CCD in bits [31:28], physical core-in-CCD in
518+
# bits [23:20]. On Linux core_id is the kernel's APIC-ID-derived PHYSICAL
519+
# core index and is gap-preserving on harvested parts (a fused-off core
520+
# leaves a hole, not a renumber), so core_id % 8 IS the physical slot and
521+
# core_id // 8 the CCD. The ccd/slot keyword overrides let a caller supply a
522+
# more authoritative source: the L3-detected CCD (RyzenSMU.set_topology), or
523+
# a slot from an SMN core-disable fuse map on a platform where core_id is not
524+
# already physical.
524525
detected_ccd = ccd if ccd is not None else core_id // 8
525526
core_in_ccd = slot if slot is not None else core_id % 8
526527
return (detected_ccd << 28) | (core_in_ccd << 20) | margin

src/smu/driver.py

Lines changed: 21 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -118,110 +118,34 @@ def __init__(
118118
self.dry_run = dry_run
119119
self._smu_lock = threading.Lock()
120120
self._backup: dict[int, int] | None = None
121-
# Topology-detected CCD map: {physical_core_id: ccd_index}
122-
# Set via set_topology() to use L3-detected CCD instead of core_id // 8
121+
# Topology-detected CCD map: {physical_core_id: ccd_index}. Set via
122+
# set_topology() to pin the SMU CCD field from L3 topology instead of
123+
# core_id // 8. The physical SLOT is not stored: it is core_id % 8 (see
124+
# set_topology / encode_co_arg for why that is exact on Linux).
123125
self._topology_ccd: dict[int, int] | None = None
124-
# Harvested core slot map: {physical_core_id: physical_slot_in_ccd}
125-
# Populated by probe_slot_map() for zen4_5 chips with harvested cores.
126-
# When None, encode_co_arg falls back to core_id % 8.
127-
self._slot_map: dict[int, int] | None = None
128126

129127
def set_topology(self, topology) -> None:
130-
"""Load CCD mapping from CPU topology for accurate SMU encoding.
131-
132-
Without this, CCD is derived from ``core_id // 8`` which is correct
133-
for standard AMD desktop layouts but may break on non-standard configs.
134-
135-
For Zen 3/4/5 chips, also probes the SMU to detect harvested core slots.
136-
On harvested / multi-CCD chips (9900X, 9700X, 7900X, 5900X, 5600X, etc.)
137-
the kernel renumbers cores contiguously but the SMU uses physical slot
138-
indices — probing builds the correct mapping.
128+
"""Load the L3-detected CCD map so the SMU CCD field is exact.
129+
130+
The physical slot within a CCD is NOT probed. On Linux the kernel decodes
131+
each core's APIC ID (CPUID Fn8000_001E) with the hardware's own field
132+
widths and exposes the result as the physical, gap-preserving /proc/cpuinfo
133+
"core id" -- so ``core_id % 8`` IS the physical SMU slot, including on
134+
harvested / multi-CCD parts (5900X, 7900X, 9900X, 5600X, ...), where a
135+
fused-off core leaves a hole in the numbering rather than renumbering the
136+
rest. (This is the Linux-vs-Windows difference: Windows enumerates cores
137+
contiguously, which is why the Windows tools read the SMN core-disable fuse
138+
to rebuild the physical slot; Linux hands us the physical slot directly.)
139+
140+
encode_co_arg derives the slot from core_id; this method only supplies the
141+
CCD index, which L3 topology pins more robustly than core_id // 8 on
142+
non-standard layouts.
139143
"""
140144
self._topology_ccd = {}
141145
for core_id, core_info in topology.cores.items():
142146
if core_info.ccd is not None:
143147
self._topology_ccd[core_id] = core_info.ccd
144148

145-
if self.commands.encoding_scheme in ("zen3", "zen4_5") and not self.dry_run:
146-
try:
147-
self._slot_map = self.probe_slot_map()
148-
except Exception as e:
149-
log.warning("Slot probing failed, using core_id %% 8 fallback: %s", e)
150-
self._slot_map = None
151-
152-
def probe_slot_map(self) -> dict[int, int] | None:
153-
"""Probe SMU to discover the physical slot for each core within its CCD.
154-
155-
On harvested Zen 3/4/5 chips the kernel renumbers cores contiguously
156-
but the SMU addresses physical slots (0-7) including gaps from
157-
disabled cores. Probes all 8 slots per CCD using GET_CO: harvested
158-
slots echo the argument, active slots return the actual CO value.
159-
160-
HARDWARE-UNVERIFIED: the echo-vs-value discrimination has not been
161-
confirmed on real harvested silicon (9900X/9700X/5600X). If a chip
162-
returns the CO value in a different field/format, the probe could map
163-
to the wrong slot; the count-based fallback (active_slots !=
164-
expected_count -> core_id %% 8) bounds the damage but does not prove it.
165-
166-
Returns the slot map {core_id: physical_slot}, or None if probing
167-
is not needed or fails.
168-
"""
169-
if self.commands.encoding_scheme not in ("zen3", "zen4_5"):
170-
return None
171-
if not self.commands.has_co or self.commands.get_co_cmd is None:
172-
return None
173-
if self._topology_ccd is None:
174-
return None
175-
176-
ccd_cores: dict[int, list[int]] = {}
177-
for core_id, ccd_idx in self._topology_ccd.items():
178-
ccd_cores.setdefault(ccd_idx, []).append(core_id)
179-
180-
slot_map: dict[int, int] = {}
181-
182-
for ccd_idx, core_ids in sorted(ccd_cores.items()):
183-
expected_count = len(core_ids)
184-
active_slots: list[int] = []
185-
ambiguous_slot: int | None = None
186-
187-
for phys_slot in range(8):
188-
arg = (ccd_idx << 28) | (phys_slot << 20)
189-
resp = self._send_command(self.commands.get_co_cmd, (arg,))
190-
if not resp.success:
191-
log.warning(
192-
"Slot probe failed for CCD %d slot %d — SMU error",
193-
ccd_idx, phys_slot,
194-
)
195-
continue
196-
197-
if arg == 0:
198-
ambiguous_slot = phys_slot
199-
continue
200-
201-
if resp.args[0] != arg:
202-
active_slots.append(phys_slot)
203-
204-
if ambiguous_slot is not None and len(active_slots) < expected_count:
205-
active_slots.insert(0, ambiguous_slot)
206-
207-
active_slots.sort()
208-
209-
if len(active_slots) != expected_count:
210-
log.warning(
211-
"Slot probe mismatch on CCD %d: found %d active slots %s "
212-
"but topology has %d cores — falling back to core_id %% 8",
213-
ccd_idx, len(active_slots), active_slots, expected_count,
214-
)
215-
for core_id in sorted(core_ids):
216-
slot_map[core_id] = core_id % 8
217-
continue
218-
219-
for core_id, phys_slot in zip(sorted(core_ids), active_slots, strict=True):
220-
slot_map[core_id] = phys_slot
221-
222-
log.info("Slot map probed: %s", slot_map)
223-
return slot_map
224-
225149
@staticmethod
226150
def is_available(sysfs_path: Path = SYSFS_BASE) -> bool:
227151
"""Check if ryzen_smu driver is loaded and accessible."""
@@ -367,8 +291,7 @@ def get_co_offset(self, core_id: int) -> int | None:
367291
if not self.commands.has_co:
368292
return None
369293
ccd = self._topology_ccd.get(core_id) if self._topology_ccd else None
370-
slot = self._slot_map.get(core_id) if self._slot_map else None
371-
arg = encode_co_arg(core_id, 0, self.commands.generation, ccd=ccd, slot=slot)
294+
arg = encode_co_arg(core_id, 0, self.commands.generation, ccd=ccd)
372295
resp = self._send_command(self.commands.get_co_cmd, (arg,))
373296
if not resp.success:
374297
return None
@@ -412,8 +335,7 @@ def set_co_offset(self, core_id: int, value: int) -> bool:
412335
return False
413336

414337
ccd = self._topology_ccd.get(core_id) if self._topology_ccd else None
415-
slot = self._slot_map.get(core_id) if self._slot_map else None
416-
arg = encode_co_arg(core_id, value, self.commands.generation, ccd=ccd, slot=slot)
338+
arg = encode_co_arg(core_id, value, self.commands.generation, ccd=ccd)
417339
resp = self._send_command(self.commands.set_co_cmd, (arg,))
418340
if not resp.success:
419341
log.error("SMU rejected CO write for core %d value %d", core_id, value)

0 commit comments

Comments
 (0)