hal/temperature: correct sysfs-hwmon temperature fallback for Arc Pro… by thevisad · Pull Request #149 · intel/xpumanager

thevisad · 2026-08-02T13:13:47Z

Description

xpu-smi reports GPU Core and Memory Temperature as N/A on the Intel Arc Pro B70 (Battlemage, xe
driver): Level Zero sysman enumerates no GPU/Memory temperature sensor on this device, even though the
readings are published on the PCI device's hwmon node (labels pkg / vram). This PR adds a correct,
validated sysfs-hwmon fallback so temperature is reported on such devices, while leaving behaviour
byte-identical where Level Zero already provides a sensor.

What it does

getCoreTemp / getMemoryTemp fall back to sysfs only when Level Zero reports the sensor type
unsupported (no matching sensor was enumerated); any other Level Zero result is propagated, so
device-loss / permission / state errors are never masked. The routing decision is a pure, unit-tested
helper (decideTempSource).
getTempPerTile uses the fallback only when no matching-type sensor was enumerated, not merely when the
tile map is empty (perTileShouldFallback).
resolveSysfsHwmon scans all hwmon* subdirs and caches the exact tempN_input path per label;
strict temp<N>_label filename matching; every stream open is verified.
One shared, bounds-checked read path (parse → milli→Celsius → physical-validity bounds). The lower
bound is absolute zero, not a device-specific 0 C floor, so a legitimate subzero reading on another
matching system is not wrongly rejected.
The pure logic lives in an internal xpum::hwmon utility (hwmon_temperature_utils.{h,cpp}) shared by
temperature.cpp and the tests, without expanding the exported temperature class / DLL surface.

Testing

doctest: 15 cases / 87 assertions (decision matrix SUCCESS→L0 / UNSUPPORTED→sysfs /
DEVICE_LOST,PERMISSIONS,UNKNOWN→propagate; strict filename parse; value bounds; multi-hwmon
discovery incl. labelled node not first; two roots don't cross-resolve). Full meson test: 9/9 suites.
Hardware (Arc Pro B70, xe, kernel 7.0.0-28-generic, Level Zero 1.27.0): xpu-smi stats reports
Core 51 / Mem 52, matching raw sysfs pkg=51000 / vram=52000; VR stays N/A.

Scope / limitations

Validated only on the tested single-tile Arc Pro B70 on xe. The fallback is condition-based, not
PCI-ID-gated: it activates when Level Zero exposes no matching sensor and the hwmon node provides the
expected label. Not generalized to multiple tiles/subdevices; VR temperature is not in sysfs and stays
N/A.
No unguarded POSIX-only header remains (directory scans and test fixtures use std::filesystem).
Windows was not build-tested. The doctest test targets require a doctest package in CI.

Fixes:
Relates-To:

Type of change

Affected components

Checklist

Commit messages follow the project guidelines
Each commit includes Signed-off-by (DCO)
New public APIs or CLI options are documented — n/a (no new public API/CLI; internal helpers only)
No unrelated changes included
Squash "fixup" commits after reviews — n/a (single commit)

savery42 · 2026-08-03T10:24:57Z

Related to #146

savery42

Apologies for the wait - got busy prior to a code-freeze and couldn't get a multi-tile until today. Overall looks good, though one general note:

The oal/ dir exists for OS-specific (or, potentially, platform specific) behavior

Some of the methods such as getHwmonLabelPaths that does a traversal within sysfs can be placed into that area to follow the project architecture

No issues with the code on Windows as you handled everything clearly, just a note on the general code structure. I also try and avoid the OAL as it's.. unique - though we do have a ticket to clean it up and make it testable (if we can ever get to a refactor ticket)

savery42 · 2026-08-10T14:40:39Z

+ *   - A matching sensor exists but its state read failed (device loss,
+ *     permission, invalid state, ...)   -> that underlying error is returned.


Due to a pre-existing issue in getTemp, in this case retval == ZE_RESULT_SUCCESS and param[out] temp == 0.0

This is because if the call to getState() (line 378) fails it just logs and continues without modifying result as well as the return statement not returning result if a "sensor is found" (property type matches the expected one)

Pedantic - apologies - I would've missed this if I hadn't been looking into another issue with garbage data after a DEVICE_LOST event from a GPU crash :)

Double note: the DEVICE_LOST issue is larger as smDeviceInit doesn't propagate errors but that's a story for a later fix..

savery42 · 2026-08-10T14:42:08Z

+	static const std::string kPrefix = "temp";
+	static const std::string kSuffix = "_label";


// c++20 style / nit

Suggested change

static const std::string kPrefix = "temp";

static const std::string kSuffix = "_label";

constexpr std::string_view kPrefix = "temp";

constexpr std::string_view kSuffix = "_label";

Avoids heap allocation, follows idiomatic c++20

Corrected in updated push

savery42 · 2026-08-10T14:50:30Z

+	// Need prefix + at least one digit + suffix.
+	if (filename.size() <= kPrefix.size() + kSuffix.size()) {
+		return false;
+	}
+	if (filename.compare(0, kPrefix.size(), kPrefix) != 0) {
+		return false;
+	}
+	if (filename.compare(filename.size() - kSuffix.size(), kSuffix.size(), kSuffix) != 0) {
+		return false;
+	}


// c++20 style

idiomatic c++20 has filename.starts_with(prefix), filename.ends_with(suffix), filename

savery42 · 2026-08-10T14:53:55Z

+	const size_t digitsBegin = kPrefix.size();
+	const size_t digitsEnd = filename.size() - kSuffix.size();
+	if (digitsEnd <= digitsBegin) {
+		return false; // no digits between prefix and suffix
+	}
+	unsigned long value = 0;
+	for (size_t i = digitsBegin; i < digitsEnd; ++i) {
+		const char c = filename[i];
+		if (c < '0' || c > '9') {
+			return false;
+		}
+		value = value * 10UL + (unsigned long)(c - '0');
+	}
+	if (idx != nullptr) {
+		*idx = (unsigned int)value;
+	}


// c++20 / no overflow risk on the unsigned long -> unsigned conversion

Can parse using std::from_chars

Suggested change

const size_t digitsBegin = kPrefix.size();

const size_t digitsEnd = filename.size() - kSuffix.size();

if (digitsEnd <= digitsBegin) {

return false; // no digits between prefix and suffix

}

unsigned long value = 0;

for (size_t i = digitsBegin; i < digitsEnd; ++i) {

const char c = filename[i];

if (c < '0' || c > '9') {

return false;

}

value = value * 10UL + (unsigned long)(c - '0');

}

if (idx != nullptr) {

*idx = (unsigned int)value;

}

unsigned int parsed = 0;

const auto [ptr, ec] = std::from_chars(digits.data(), digits.data() + digits.size(), parsed);

if (ec != std::errc{} || ptr != digits.data() + digits.size()) {

return false;

}

if (idx != nullptr) {

*idx = parsed;

}

savery42 · 2026-08-10T15:02:24Z

+		if (!parseTempLabelIndex(entryName, &idx)) {
+			continue;
+		}
+		const std::string labelPath = hwmonDir + "/" + entryName;


// nit

This is the same as entry.path(). Using that directly avoids allocation

savery42 · 2026-08-10T15:03:12Z

+			continue;
+		}
+		++visited;
+		scanHwmonForLabels(hwmonBase + "/" + name, out);


// nit

Suggested change

scanHwmonForLabels(hwmonBase + "/" + name, out);

scanHwmonForLabels(entry.path().string(), out);

savery42 · 2026-08-10T15:05:27Z

+	// lives in the internal xpum::hwmon utilities (hwmon_temperature_utils.h) so
+	// it is shared with the unit tests without expanding this exported class.
+	// Scope and non-goals are documented on resolveSysfsHwmon() in the .cpp.
+	std::string sysfsHwmonBase;                     // /sys/bus/pci/devices/<BDF>/hwmon


This doesn't seem to need to be a member and only passed as a function param after clearing -> setting

Reco removing

savery42 · 2026-08-10T15:06:48Z

+								 : (type == ZES_TEMP_SENSORS_MEMORY) ? "vram"
+																	 : nullptr;
+		double sysfsTemp = 0.0;
+		if (sysfsLabel != nullptr && readSysfsLabel(sysfsLabel, &sysfsTemp) == ZE_RESULT_SUCCESS) {


if readSysfsLabel() != ZE_RESULT_SUCCESS the error gets dropped and a ZE_RESULT_SUCCESS is returned

Report GPU package and VRAM temperatures on the tested Arc Pro B70 when Level Zero Sysman exposes no matching temperature sensors. Move hwmon traversal into the OS abstraction layer, keep temperature parsing/validation outside platform-specific discovery, preserve Level Zero error propagation, and add Linux OAL/unit coverage plus Windows compile validation. Signed-off-by: thevisad <373694+thevisad@users.noreply.github.com>

thevisad · 2026-08-11T12:49:49Z

@savery42 Thanks for the review. Reworked along the lines you asked.

OAL move. The hwmon sysfs traversal now lives in the OS abstraction layer: oal/os.h declares getHwmonLabelPaths(bdf, prefix), implemented in oal/lin/hwmon.cpp (a std::filesystem walk of the PCI device's hwmon* nodes with strict temp_label matching) and stubbed in oal/win/hwmon.cpp. The temperature interpretation (parse, milli-to-Celsius, physical-bounds validation, and the Level-Zero-vs-sysfs decision) stays portable in hal (hwmon_temperature_utils). OAL discovers the filesystem objects; HAL reads and validates them.

Error contract. decideTempSource falls back to the hwmon node when Level Zero reports the sensor unsupported or denies access to an unprivileged process (the "only works with sudo" symptom in #146), and propagates device-loss / other runtime errors so a real driver failure is never masked. While in here I also fixed getTempPerTile, which returned ZE_RESULT_SUCCESS after a failed sysfs read instead of propagating the error, and dropped the sysfsHwmonBase member (the resolved label-to-path map is cached instead).

Tests. New OAL unit suite oal/lin/test/hwmon_tests (7 cases / 26 assertions) covers discovery; temperature_tests (8 / 41) covers the decision matrix and bounds. meson test: 10/10 suites green. Hardware: on an Arc Pro B70 (xe, Level Zero 1.27.0) xpu-smi stats reports Core 51 / Mem 52 matching raw sysfs pkg / vram.

Windows. Linux is fully tested and hardware-validated; the touched Windows compilation surface (oal/win/hwmon.cpp, hwmon_temperature_utils.cpp) is verified clean with MSVC 19.44 at /std:c++20 /WX, which is a compile check of the modified translation units, not a full Windows product build.

Single DCO-signed commit; the PR description is updated to match. Relates to #146.

thevisad requested review from AshwinKumarKulkarni, aravindksg, saik-intel and savery42 as code owners August 2, 2026 13:13

thevisad force-pushed the xpum-temp-hwmon branch from cf3f931 to 4d8a702 Compare August 3, 2026 11:21

savery42 mentioned this pull request Aug 3, 2026

fan: multi-fan RPM with sysfs hwmon fallback for Battlemage/xe (Arc P… #148

Open

20 tasks

savery42 reviewed Aug 10, 2026

View reviewed changes

thevisad force-pushed the xpum-temp-hwmon branch from 4d8a702 to df3741f Compare August 11, 2026 12:14

Navigation Menu

Uh oh!

hal/temperature: correct sysfs-hwmon temperature fallback for Arc Pro… - #149

hal/temperature: correct sysfs-hwmon temperature fallback for Arc Pro…#149
thevisad wants to merge 1 commit into
intel:mainfrom
thevisad:xpum-temp-hwmon

thevisad commented Aug 2, 2026

Uh oh!

savery42 commented Aug 3, 2026

Uh oh!

savery42 left a comment

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

thevisad Aug 11, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

savery42 Aug 10, 2026

Uh oh!

thevisad commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		* - A matching sensor exists but its state read failed (device loss,
		* permission, invalid state, ...) -> that underlying error is returned.

		static const std::string kPrefix = "temp";
		static const std::string kSuffix = "_label";

	scanHwmonForLabels(hwmonBase + "/" + name, out);
	scanHwmonForLabels(entry.path().string(), out);

Uh oh!

Conversation

thevisad commented Aug 2, 2026

Description

Type of change

Affected components

Checklist

Uh oh!

savery42 commented Aug 3, 2026

Uh oh!

savery42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thevisad commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants