Fix SNC state detection on non-SNC-capable platforms - #304
shenxiaochen wants to merge 1 commit into
Conversation
|
Note: The code base of this PR is on top of #300 and #299 [This PR - #304 ] [PR #300 ] [PR #299 ] Best regards, |
|
I would appreciate it if you could review this PR at your convenience. As a heads-up, it depends on PR #300. |
|
Hi @shenxiaochen |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where SNC state detection fails on non-SNC-capable platforms (AMD and Hygon) by adding support for Hygon vendor detection and ensuring early returns in SNC-related code paths for both AMD and Hygon platforms.
Changes:
- Added PQOS_VENDOR_HYGON vendor enum value and detection logic
- Updated SNC state detection to return early for AMD and Hygon platforms
- Extended all AMD-specific vendor checks to also include Hygon
- Fixed Hygon-specific CPUID counter length calculation
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/pqos.h | Added PQOS_VENDOR_HYGON enum value and updated documentation |
| lib/cpuinfo.c | Added Hygon vendor detection and initialization logic |
| lib/hw_cap.c | Added early return for AMD/Hygon in SNC state detection and fixed Hygon counter length |
| lib/cap.c | Extended MBA discovery vendor check to include Hygon |
| lib/api.c | Extended API initialization vendor checks to include Hygon |
| rdtset/rdt.c | Extended all AMD vendor checks to include Hygon |
| pqos/alloc.c | Extended all AMD vendor checks to include Hygon |
| lib/python/pqos/native_struct.py | Added PQOS_VENDOR_HYGON constant |
| lib/python/pqos/cpuinfo.py | Added Hygon vendor string mapping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@rkanagar Thank you very much for code review! I will address the Copilot review comments inline. |
12685be to
73f4472
Compare
|
Hi @shenxiaochen , Please rebase this PR. We are ready to merge. Thanks, |
On platforms that do not support Sub-NUMA Clustering (SNC) monitoring (e.g., AMD and Hygon), if the CPU topology indicates that the number of NUMA nodes and the number of sockets are not equal, the SNC state detection logic in hw_cap_mon_snc_state() will attempt to read the SNC configuration MSR register PQOS_MSR_SNC_CFG (0xCA0) that is not available. This causes the command "pqos --iface=msr" to fail with the error: "ERROR: RDMSR failed for reg[0xca0] on lcore 0" "ERROR: Error reading SNC information!" "ERROR: Error encounter in monitoring discovery!" "ERROR: discover_capabilities() error 1" "Error initializing PQoS library!" Fix the issue by ensuring hw_cap_mon_snc_state() returns early on non-SNC-capable platforms. Fixes: bfc7c70 ("SNC is added") Signed-off-by: Xiaochen Shen <shenxiaochen@open-hieco.net>
73f4472 to
5afb324
Compare
Rebased on top of master tree (after PR #300 merged). Thank you! |
|
Merged by |
Fix SNC state detection on non-SNC-capable platforms
Description
On platforms that do not support Sub-NUMA Clustering (SNC) monitoring (e.g., AMD and Hygon), if the CPU topology indicates that the number of NUMA nodes and the number of sockets are not equal, the SNC state detection logic in hw_cap_mon_snc_state() will attempt to read the SNC configuration MSR register PQOS_MSR_SNC_CFG (0xCA0) that is not
available. This causes the command "pqos --iface=msr" to fail with the error:
Fix the issue by ensuring hw_cap_mon_snc_state() returns early on non-SNC-capable platforms.
Affected parts
Motivation and Context
The command "pqos --iface=msr" fails on non-SNC-capable platforms during SNC state detection with the error:
The code changes fix the issue on non-SNC-capable platforms.
How Has This Been Tested?
(1) Run "pqos --iface=msr" without the error described above on non-SNC-capable platforms (e.g., AMD or Hygon).
(2) Passed all tests in intel-cmt-cat/unit-test.
Types of changes
Checklist: