Skip to content

Commit 4cb12a8

Browse files
committed
s2idle: Look at the socket level for telling how many CPUs are present
Looking at the complex level will return the wrong number of cores in a multi-CCD system. Reported-by: Pratap Nirujogi <[email protected]> Signed-off-by: Mario Limonciello <[email protected]>
1 parent 7f6fa73 commit 4cb12a8

File tree

3 files changed

+202
-5
lines changed

3 files changed

+202
-5
lines changed

src/amd_debug/failures.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,15 @@ def __init__(self, actual_cores, max_cores):
576576
)
577577

578578

579+
class CpuTopologyUnknown(S0i3Failure):
580+
"""CPU topology unknown"""
581+
582+
def __init__(self):
583+
super().__init__()
584+
self.description = "CPU topology unknown"
585+
self.explanation = "The CPU topology could not be determined. This may prevent the system from entering hardware sleep states correctly."
586+
587+
579588
class RogAllyOldMcu(S0i3Failure):
580589
"""MCU firwmare is too old"""
581590

src/amd_debug/prerequisites.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
AmdHsmpBug,
4545
AmdgpuPpFeatureMask,
4646
ASpmWrong,
47+
CpuTopologyUnknown,
4748
DeepSleep,
4849
DevSlpDiskIssue,
4950
DevSlpHostIssue,
@@ -1113,11 +1114,23 @@ def read_cpuid(cpu, leaf, subleaf):
11131114
# check for artificially limited CPUs
11141115
p = os.path.join("/", "sys", "devices", "system", "cpu", "kernel_max")
11151116
max_cpus = int(read_file(p)) + 1 # 0 indexed
1116-
# https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24594.pdf
1117-
# Extended Topology Enumeration (NumLogCores)
1118-
# CPUID 0x80000026 subleaf 1
1117+
# https://docs.amd.com/v/u/en-US/40332-PUB_4.08
1118+
# E.4.24 Function 8000_0026—Extended CPU Topology
11191119
try:
1120-
_, cpu_count, _, _ = read_cpuid(0, 0x80000026, 1)
1120+
# Need to discover the socket level to look at all CCDs in the socket
1121+
for level in range(0, 5):
1122+
_, _, ecx, _ = read_cpuid(0, 0x80000026, level)
1123+
level_type = (ecx >> 8) & 0xFF
1124+
if level_type == 0:
1125+
self.db.record_prereq(
1126+
"Unable to discover CPU topology, didn't find socket level",
1127+
"❌",
1128+
)
1129+
self.failures += [CpuTopologyUnknown()]
1130+
return False
1131+
if level_type == 4:
1132+
break
1133+
_, cpu_count, _, _ = read_cpuid(0, 0x80000026, level)
11211134
if cpu_count > max_cpus:
11221135
self.db.record_prereq(
11231136
f"The kernel has been limited to {max_cpus} CPU cores, "

src/test_prerequisites.py

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import logging
99
import unittest
1010
import subprocess
11+
import struct
1112
from unittest.mock import patch, MagicMock, mock_open
1213

1314
from amd_debug.prerequisites import PrerequisiteValidator
@@ -436,6 +437,176 @@ def test_check_cpu_unsupported_model(self, mock_path_exists, mock_read_file):
436437
"This CPU model does not support hardware sleep over s2idle", "❌"
437438
)
438439

440+
@patch("builtins.open", new_callable=mock_open)
441+
@patch("amd_debug.prerequisites.read_file")
442+
@patch("amd_debug.prerequisites.os.path.exists")
443+
def test_check_cpu_limited_cores_single_ccd(
444+
self, mock_path_exists, mock_read_file, mock_file_open
445+
):
446+
"""Test check_cpu with artificially limited CPUs on single-CCD system"""
447+
self.validator.cpu_family = 0x19
448+
self.validator.cpu_model = 0x74
449+
mock_read_file.return_value = "7" # kernel_max = 7 (8 cores)
450+
mock_path_exists.return_value = True
451+
# Simulate finding socket level at subleaf 1
452+
# First call: subleaf 0, level_type = 1 (not socket)
453+
# Second call: subleaf 1, level_type = 4 (socket level)
454+
# Third call: read cpu_count from subleaf 1
455+
mock_file_open.return_value.read.side_effect = [
456+
struct.pack("4I", 0, 0, 0x100, 0), # subleaf 0: level_type = 1 (thread)
457+
struct.pack(
458+
"4I", 0, 0, 0x400, 0
459+
), # subleaf 1: level_type = 4 (socket) - FOUND
460+
struct.pack("4I", 0, 16, 0, 0), # subleaf 1: cpu_count = 16
461+
]
462+
result = self.validator.check_cpu()
463+
self.assertFalse(result)
464+
self.assertTrue(
465+
any(isinstance(f, LimitedCores) for f in self.validator.failures)
466+
)
467+
self.mock_db.record_prereq.assert_called_with(
468+
"The kernel has been limited to 8 CPU cores, but the system has 16 cores",
469+
"❌",
470+
)
471+
472+
@patch("builtins.open", new_callable=mock_open)
473+
@patch("amd_debug.prerequisites.read_file")
474+
@patch("amd_debug.prerequisites.os.path.exists")
475+
def test_check_cpu_limited_cores_multi_ccd(
476+
self, mock_path_exists, mock_read_file, mock_file_open
477+
):
478+
"""Test check_cpu with multi-CCD system (tests socket-level counting)"""
479+
self.validator.cpu_family = 0x19
480+
self.validator.cpu_model = 0x74
481+
mock_read_file.return_value = "15" # kernel_max = 15 (16 cores)
482+
mock_path_exists.return_value = True
483+
# Simulate multi-CCD: iterate through levels to find socket
484+
# subleaf 0: level_type = 1 (thread)
485+
# subleaf 1: level_type = 2 (core)
486+
# subleaf 2: level_type = 3 (complex/CCD)
487+
# subleaf 3: level_type = 4 (socket) - FOUND
488+
mock_file_open.return_value.read.side_effect = [
489+
struct.pack("4I", 0, 0, 0x100, 0), # subleaf 0: level_type = 1 (thread)
490+
struct.pack("4I", 0, 0, 0x200, 0), # subleaf 1: level_type = 2 (core)
491+
struct.pack(
492+
"4I", 0, 0, 0x300, 0
493+
), # subleaf 2: level_type = 3 (complex/CCD)
494+
struct.pack(
495+
"4I", 0, 0, 0x400, 0
496+
), # subleaf 3: level_type = 4 (socket) - FOUND
497+
struct.pack("4I", 0, 32, 0, 0), # subleaf 3: cpu_count = 32
498+
]
499+
result = self.validator.check_cpu()
500+
self.assertFalse(result)
501+
self.assertTrue(
502+
any(isinstance(f, LimitedCores) for f in self.validator.failures)
503+
)
504+
self.mock_db.record_prereq.assert_called_with(
505+
"The kernel has been limited to 16 CPU cores, but the system has 32 cores",
506+
"❌",
507+
)
508+
509+
@patch("builtins.open", new_callable=mock_open)
510+
@patch("amd_debug.prerequisites.read_file")
511+
@patch("amd_debug.prerequisites.os.path.exists")
512+
def test_check_cpu_not_limited(
513+
self, mock_path_exists, mock_read_file, mock_file_open
514+
):
515+
"""Test check_cpu when CPUs are not artificially limited"""
516+
self.validator.cpu_family = 0x19
517+
self.validator.cpu_model = 0x74
518+
mock_read_file.return_value = "31" # kernel_max = 31 (32 cores)
519+
mock_path_exists.return_value = True
520+
# Simulate finding socket level
521+
mock_file_open.return_value.read.side_effect = [
522+
struct.pack("4I", 0, 0, 0x100, 0), # subleaf 0: level_type = 1
523+
struct.pack("4I", 0, 0, 0x400, 0), # subleaf 1: level_type = 4 (socket)
524+
struct.pack("4I", 0, 16, 0, 0), # subleaf 1: cpu_count = 16
525+
]
526+
result = self.validator.check_cpu()
527+
self.assertTrue(result)
528+
self.mock_db.record_debug.assert_called_with("CPU core count: 16 max: 32")
529+
530+
@patch("builtins.open", new_callable=mock_open)
531+
@patch("amd_debug.prerequisites.read_file")
532+
@patch("amd_debug.prerequisites.os.path.exists")
533+
def test_check_cpu_socket_level_at_boundary(
534+
self, mock_path_exists, mock_read_file, mock_file_open
535+
):
536+
"""Test check_cpu when socket level is found at the last checked subleaf"""
537+
self.validator.cpu_family = 0x19
538+
self.validator.cpu_model = 0x74
539+
mock_read_file.return_value = "15" # kernel_max = 15 (16 cores)
540+
mock_path_exists.return_value = True
541+
# Socket level found at subleaf 4 (last iteration)
542+
mock_file_open.return_value.read.side_effect = [
543+
struct.pack("4I", 0, 0, 0x100, 0), # subleaf 0: level_type = 1
544+
struct.pack("4I", 0, 0, 0x200, 0), # subleaf 1: level_type = 2
545+
struct.pack("4I", 0, 0, 0x300, 0), # subleaf 2: level_type = 3
546+
struct.pack("4I", 0, 0, 0x000, 0), # subleaf 3: level_type = 0
547+
struct.pack("4I", 0, 0, 0x400, 0), # subleaf 4: level_type = 4 (socket)
548+
struct.pack("4I", 0, 16, 0, 0), # subleaf 4: cpu_count = 16
549+
]
550+
result = self.validator.check_cpu()
551+
self.assertFalse(result)
552+
self.mock_db.record_prereq.assert_called_with(
553+
"Unable to discover CPU topology, didn't find socket level", "❌"
554+
)
555+
556+
@patch("builtins.open", new_callable=mock_open)
557+
@patch("amd_debug.prerequisites.read_file")
558+
@patch("amd_debug.prerequisites.os.path.exists")
559+
def test_check_cpu_socket_level_first_subleaf(
560+
self, mock_path_exists, mock_read_file, mock_file_open
561+
):
562+
"""Test check_cpu when socket level is found at first subleaf"""
563+
self.validator.cpu_family = 0x19
564+
self.validator.cpu_model = 0x74
565+
mock_read_file.return_value = "7" # kernel_max = 7 (8 cores)
566+
mock_path_exists.return_value = True
567+
# Socket level found immediately at subleaf 0
568+
mock_file_open.return_value.read.side_effect = [
569+
struct.pack(
570+
"4I", 0, 0, 0x400, 0
571+
), # subleaf 0: level_type = 4 (socket) - FOUND
572+
struct.pack("4I", 0, 8, 0, 0), # subleaf 0: cpu_count = 8
573+
]
574+
result = self.validator.check_cpu()
575+
self.assertTrue(result)
576+
self.mock_db.record_debug.assert_called_with("CPU core count: 8 max: 8")
577+
578+
@patch("builtins.open", side_effect=FileNotFoundError)
579+
@patch("amd_debug.prerequisites.read_file")
580+
@patch("amd_debug.prerequisites.os.path.exists")
581+
def test_check_cpu_cpuid_file_not_found(
582+
self, mock_path_exists, mock_read_file, mock_file_open
583+
):
584+
"""Test check_cpu when cpuid kernel module is not loaded"""
585+
self.validator.cpu_family = 0x19
586+
self.validator.cpu_model = 0x74
587+
mock_read_file.return_value = "7"
588+
mock_path_exists.return_value = False
589+
result = self.validator.check_cpu()
590+
self.assertFalse(result)
591+
self.mock_db.record_prereq.assert_called_with(
592+
"Unable to check CPU topology: cpuid kernel module not loaded", "❌"
593+
)
594+
595+
@patch("builtins.open", side_effect=PermissionError)
596+
@patch("amd_debug.prerequisites.read_file")
597+
@patch("amd_debug.prerequisites.os.path.exists")
598+
def test_check_cpu_cpuid_permission_error(
599+
self, mock_path_exists, mock_read_file, mock_file_open
600+
):
601+
"""Test check_cpu when there is a permission error accessing cpuid"""
602+
self.validator.cpu_family = 0x19
603+
self.validator.cpu_model = 0x74
604+
mock_read_file.return_value = "7"
605+
mock_path_exists.return_value = True
606+
result = self.validator.check_cpu()
607+
self.assertTrue(result)
608+
self.mock_db.record_prereq.assert_called_with("CPUID checks unavailable", "🚦")
609+
439610
@patch("amd_debug.prerequisites.os.walk")
440611
@patch(
441612
"amd_debug.prerequisites.open",
@@ -1127,7 +1298,11 @@ def test_capture_disabled_pins_with_parameters(self, _mock_open, mock_path_exist
11271298
)
11281299

11291300
@patch("amd_debug.prerequisites.os.path.exists")
1130-
@patch("amd_debug.prerequisites.open", new_callable=unittest.mock.mock_open, read_data="(null)")
1301+
@patch(
1302+
"amd_debug.prerequisites.open",
1303+
new_callable=unittest.mock.mock_open,
1304+
read_data="(null)",
1305+
)
11311306
def test_capture_disabled_pins_with_null_values(self, _mock_open, mock_path_exists):
11321307
mock_path_exists.side_effect = (
11331308
lambda path: "ignore_wake" in path or "ignore_interrupt" in path

0 commit comments

Comments
 (0)