intel / intel/xpumanager

[BUG] power.cpp: one value written to every power level; a failed write reports a contradictory second error; the advertised lower bound is a literal; one failing domain hides telemetry from all the others

Open
#168 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
194
Forks
34
PR merge metrics
No merged PRs in 30d

Description

**Repo:** intel/xpumanager · **Affected:** v2.1.0 **and current `main`** (`hal/core/power.cpp`
is unchanged between the tag and `origin/main` as of 06.09.2026)
Package as installed: `xpu-smi 2.1.0+26.33.6468cec-1~26.04`.
**Hardware:** 8 × Intel Arc Pro B60 (`8086:e211`), Linux 7.0.0-31, `xe`, Level Zero 1.32.0.
On these cards `sustained` is not exposed; `burst = 200 W`, `peak = 400 W`.

## 1. `--powerlimit` writes the same value into sustained, burst and peak

```cpp
// hal/core/power.cpp:347-349
for (uint32_t j = 0; j < powerLimitsCount; ++j) {
powerLimits[j].limit = static_cast(powerLimit * 1000); // Convert to mW
}
```

The loop walks **every** descriptor without looking at its `level`
(sustained / burst / peak) or its `source`. Setting "the power limit" therefore silently
flattens burst down to the sustained value.

Normally burst sits above the sustained limit — that is what lets short spikes through. After
this write the burst headroom is gone, and the card throttles earlier than the number the
user entered would suggest.

⚠️ There is also no range check: `powerLimit * 1000` is cast to `uint32_t` without comparing
against `minLimit` / `maxLimit`, although both are read a few lines above (`:194-196`).

**Suggested fix:** write only the descriptor whose `level` matches the user's intent, and
validate against the reported range.

## 2. A rejected write produces two messages, and the second one contradicts the first

```cpp
// hal/core/power.cpp:549-564
if (isMatch) {
...
res = zesPowerSetLimits(powerHandles[i], &sustained, nullptr, nullptr);
if (res == ZE_RESULT_SUCCESS) {
return res;
}
ERR("Failed to set sustained limit. 0x{:X} ({})\n", res, l0_error_to_string(res));
}
}

ERR("No matching power domain found for tileId {}.\n", tileId);
return ZE_RESULT_ERROR_UNKNOWN;
```

When the domain *is* matched but the write is rejected (insufficient privileges, value out of
range, driver refusal), the loop prints the first error and continues; falling out of the
loop it unconditionally prints **"No matching power domain found"**.

⇒ The user sees two errors in a row, the second of which is false and points at the wrong
cause — a tile that was in fact found. From the output alone, "domain not found" and "domain
found but not writable" are indistinguishable. The returned code is
`ZE_RESULT_ERROR_UNKNOWN` in both cases.

**Suggested fix:** remember whether any domain matched, and return the write's own error code
when one did.

## 3. The lower bound of the advertised range is a hard-coded `"1"`

```cpp
// hal/core/power.cpp:804-825
if (props.maxLimit > 0) {
uint32_t maxLimitW = props.maxLimit / 1000;
range = "1 to " + std::to_string(maxLimitW);
if (extProps.domain == ZES_POWER_DOMAIN_PACKAGE) {
break;
}
}
```

The minimum is the literal `1`, while the real one (`props.minLimit`) is available in the very
same structure and is printed elsewhere in this file (`:196`). The maximum is truncated by
integer division — `120500 mW` is shown as `120`.

⇒ The user is told a range whose lower end is invented. Attempting a value near "1" is
rejected — and by defect 2 that rejection arrives as two messages, the second one misleading.

⚠️ Also, `range` is overwritten for every domain with `maxLimit > 0` and the loop only breaks
on `PACKAGE`; if no `PACKAGE` domain exists, whichever domain came last wins.

## 4. One failing power domain suppresses telemetry from all the healthy ones

```cpp
// hal/core/power.cpp:948-953
for (uint32_t i = 0; i < powerCount; ++i) {
result = getPowerLimits(powerHandles[i]);
if (result != ZE_RESULT_SUCCESS) {
return result;
}
}
```

`return` where `continue` is meant. A single domain that refuses to report aborts the walk
over the remaining domains. The same shape appears at `:978-983` and `:1004-1020`.

⇒ This is not cosmetic: telemetry from working domains is lost because a neighbouring one
failed. On a card exposing several power domains, one unsupported query blanks the rest.

**Suggested fix:** `continue` and accumulate; return an error only if every domain failed.

## Related existing report

Issue [#122](https://github.com/intel/xpumanager/issues/122) ("Can't set powerlimit on Arc
Pro B60") reports `--powerlimit` failing with `Error: Error` on the same device ID
(`0xe211`). That report is about the failure being opaque; the defects above describe what
the code does when it does not fail — and defect **2** explains why the message there is
uninformative.

## Minor, in the same file

- **`:65-84`** — a missing `device_thresholds.json` is not logged at all (there is no `else`
branch). All thresholds silently fall back to defaults, which from the outside is
indistinguishable from normal operation.
- **`:692-738` vs `:750-792`** — two nearly identical domain-walking helpers log the *same*
failure at different levels: `ERR` in one, `DBG` in the other. Whether a problem is visible
depends on which path reached it.

Contributor guide

Open the contributing guide

Research direction

Start in hal/core/power.cpp at the cited power-limit write, error-handling, range-formatting, and telemetry loops. Trace the power descriptor levels and min/max properties first, then inspect how each domain-walking path handles failures. Done means the reported limit, range, error, and multi-domain telemetry behaviors are corrected without regressing the existing paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, linux
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.