DF-2205 — amdgpu_xgmi: error path leaves device half-initialized on hive list (Low)

Claim

amdgpu_xgmi_add_device() in sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c list_add_tail()s the device into hive->device_list (line 85) before attempting psp_xgmi_get_topology_info() (line 89). If that call fails, the goto exit at line 95 (and a second one at 114) does not remove the device, so the hive permanently contains an entry whose topology was never fetched or set. The next amdgpu_xgmi_add_device() for any other node then copies that stale device_id into tmp_topology (line 87) and drives psp_xgmi_set_topology_info across the half-baked node (line 99). ret is only re-checked once (if (!ret) at line 109), so a set_topology_info failure on a later node still leaves earlier nodes' state partially applied.

Verification approach

HW-gated / source-only. Reachable only on Vega20 hardware with a PSP topology fetch that fails (faulty PSP firmware / bad hardware link). No GPU on this audit guest. Per the task brief, source-only confirmation is acceptable.

1. Confirm the list_add_tail precedes the psp_xgmi_get_topology_info call.
2. Confirm the error-path goto exit does not call list_del.
3. Confirm the subsequent list_for_each_entry would re-enter the stale node.
4. Author fix.diff (undo the linkage on error).
5. Phase 8 — apply all 5 batched fixes and rebuild amdgpu.ko with -Werror.

Source trace (confirmed)

• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:85 — list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:89 — ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:90-96 — if (ret) { dev_err(...); goto exit; } — no list_del before goto.
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:86-87 — the next add for any node in the same hive will iterate hive->device_list (including the half-baked node) and copy its stale device_id into tmp_topology.
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:98-108 — list_for_each_entry(tmp_adev, &hive->device_list, ...) will drive psp_xgmi_set_topology_info across every node, half-baked or not.
• sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c:109 — if (!ret) re-checks ret only once; a set_topology_info failure on a later node leaves earlier nodes' partial state in place.

Files

• VERDICT.md — full narrative.
• fix.diff — on the psp_xgmi_get_topology_info failure path, list_del(&adev->gmc.xgmi.head) + INIT_LIST_HEAD(...) before the existing dev_err / goto exit.
• build_baseline.log — unpatched amdgpu.ko build, rc=0, -Werror.
• build_patched.log — patched amdgpu.ko rebuild (only amdgpu_xgmi.o recompiled), rc=0, -Werror.
• env.txt — guest environment.

Reproduce

./build.sh    # applies fix.diff + rebuilds amdgpu.ko
./run.sh      # HW-gated no-op (see VERDICT.md)

DF-2205 — VERDICT

Verdict: REPRODUCED (source-only, HW-gated).

Class: silent state inconsistency / partial-init → topology corruption.

Mechanism

amdgpu_xgmi_add_device() (sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c) links the device into its hive before the topology-fetch step has been validated:

/* amdgpu_xgmi.c:85 */
list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
list_for_each_entry(entry, &hive->device_list, head)
    tmp_topology[count++].device_id = entry->device_id;

/* amdgpu_xgmi.c:89 */
ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
if (ret) {
    dev_err(...);
    goto exit;          /* <-- list_del missing here */
}

If psp_xgmi_get_topology_info() fails, the function logs an error and jumps to exit (mutex_unlock + return ret) without removing the device from hive->device_list. The hive now permanently contains an entry whose topology was never fetched or set.

The next amdgpu_xgmi_add_device() for any other device in the same hive:

• Iterates hive->device_list at line 86 and copies the stale node's device_id into tmp_topology at line 87.
• Drives psp_xgmi_set_topology_info across every node — including the half-baked one — at line 99.

Worse, ret from psp_xgmi_set_topology_info is only re-checked once (if (!ret) at line 109). If set_topology_info fails on a later node, the loop breaks but earlier nodes' partial state remains applied — the function returns the failure but the hive is left in a silently inconsistent state.

This defect amplifies DF-2204: combined with the missing removal path, half-baked entries never get cleaned up, so the inconsistency is permanent for the lifetime of the hive (and across driver reloads).

Trigger surface / reachability

psp_xgmi_get_topology_info() fails when the PSP firmware is faulty, the XGMI link is unhealthy, or the GPU is in an error state during probe. Vega20-only (the function early-returns for older ASICs and APUs). On this audit guest there is no GPU, so the partial-init cannot be exercised live; the bug is confirmed at the source level and the fix compiles cleanly (Phase 8).

Fix

fix.diff adds the missing cleanup on the error path:

ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
if (ret) {
    list_del(&adev->gmc.xgmi.head);
    INIT_LIST_HEAD(&adev->gmc.xgmi.head);
    dev_err(...);
    goto exit;
}

list_del + INIT_LIST_HEAD is the standard pattern: unlink from the hive, then mark the node empty so a later amdgpu_xgmi_remove_device (DF-2204's fix) and the idempotency guard both see it as unlinked. This prevents the half-baked node from being visible to subsequent adds and breaks the inconsistency amplification.

Phase 8 build validation

Applied fix.diff (plus the four other batched DRM fixes) to the in-guest /usr/src, rebuilt amdgpu.ko with -Werror: * baseline (unpatched) amdgpu.ko: rc=0 (build_baseline.log). * patched amdgpu.ko: rc=0, only amdgpu_xgmi.o recompiled (build_patched.log); no warnings, no errors.

Verdict

REPRODUCED at source level (HW-gated; no live repro possible on guest). The missing cleanup on the topology-fetch error path is unambiguous from the source; the fix compiles cleanly under -Werror.