DF-4999 — dc_helper.c: generic_reg_wait prototype in dm_services.h declares (mask, shift) but the definition is (shift, mask) — latent API-contract trap for every DC consumer

Where

Declaration: sys/dev/drm/amd/display/dc/dm_services.h:152-156 Definition: sys/dev/drm/amd/display/dc/dc_helper.c:254-257

/* dm_services.h:152 */
unsigned int generic_reg_wait(const struct dc_context *ctx,
    uint32_t addr, uint32_t mask, uint32_t shift, uint32_t condition_value, ...)

/* dc_helper.c:254 */
uint32_t generic_reg_wait(const struct dc_context *ctx,
    uint32_t addr, uint32_t shift, uint32_t mask, uint32_t condition_value, ...)

What

The shared-header prototype swaps mask/shift relative to the actual definition. Today this is harmless at the ABI level (both parameters are uint32_t, so the compiler binds arguments positionally with no diagnostic) and harmless at the behavior level because the only call family, REG_WAIT() (inc/reg_helper.h), expands FN(reg,field) to reg__field__SHIFT, reg__field__MASK — i.e. it passes shift first, matching the definition, not the header. (There are no direct generic_reg_wait() call sites outside reg_helper.h/dm_services.h.)

The trap: any future caller that reads the header (as one does) and writes generic_reg_wait(ctx, addr, mask, shift, cond, delay, tries, __func__, __LINE__) silently transposes the pair — get_reg_field_value_ex() (dm_services.h:112) then computes (shift & reg_val) >> mask (garbage), the poll condition never matches, and every such wait burns its full budget (in-tree budgets reach 100 ms) and lands in the error path (dm_error + BREAK_TO_DEBUGGER). The same mis-ordering applied to the varargs generic_reg_update_ex family would be memory-unsafe (stack garbage merged into MMIO writes); for generic_reg_wait it is functional-only.

The function's own comment block at dm_services.h:146-148 ("return number of poll before condition is met / return 0 if condition is not meet") further documents that the API is meant to be consumed directly.

Impact

No in-tree defect today (audited every call path); API-contract hazard / documentation defect in the single most-used helper hub of the DC subsystem. Info severity, hardening.

Pass-2 companion audit of the hub (negative results recorded in the finding entry)

• generic_reg_update_ex/generic_indirect_reg_update_ex varargs: every direct *_REG_SET_N/UPDATE_N call site in the tree was pair-counted — all match (n never exceeds the supplied triplets), so there is no stack-garbage-into-MMIO primitive; the only mismatch in-tree is DF-4435 (n smaller, writes silently dropped — filed pass 1, not re-reported).
• generic_reg_wait loop is bounded (time_out_num_tries+1 iterations); all in-tree budgets are compile-time constants with products ≤ 100 ms (largest seen: dcn10_optc.c:496/762, 1 µs × 100000); no user-controlled argument ever reaches this file.

DF-4999 — VERDICT

Finding: prototype/definition parameter-order mismatch for generic_reg_wait between dm_services.h:152-156 and dc_helper.c:254-257.

Status: untested (static; the audit guest has no DRM/DC device — /dev/dri absent, same as the DF-4435 verification)

Proof

1. Declaration (header every DC translation unit includes):

c /* sys/dev/drm/amd/display/dc/dm_services.h:152 */ unsigned int generic_reg_wait(const struct dc_context *ctx, uint32_t addr, uint32_t mask, uint32_t shift, uint32_t condition_value, unsigned int delay_between_poll_us, unsigned int time_out_num_tries, const char *func_name, int line);

2. Definition (this file):

c /* sys/dev/drm/amd/display/dc/dc_helper.c:254 */ uint32_t generic_reg_wait(const struct dc_context *ctx, uint32_t addr, uint32_t shift, uint32_t mask, uint32_t condition_value, unsigned int delay_between_poll_us, unsigned int time_out_num_tries, const char *func_name, int line)

3. Harmless today only because C binds arguments positionally over identical uint32_t types and the sole call family is the REG_WAIT macro (inc/reg_helper.h:245-248), whose FN() expansion orders shift, mask — matching the definition, contradicting the header. Direct-call census: rg generic_reg_wait\( over sys/dev/drm/amd/display/dc returns only the definition, the header declaration, and the REG_WAIT macro — no other call sites.

4. The trap for future callers is concrete: a header-believing caller transposes the pair; get_reg_field_value_ex(reg_val, mask, shift) (dm_services.h:112-116) computes (mask & reg_val) >> shift with the roles swapped → condition never satisfied → full poll budget burned (in-tree budgets up to 100 ms, e.g. dcn10_optc.c:496) → dm_error timeout + BREAK_TO_DEBUGGER() (dc_helper.c:288-293) on every call — and for the sibling varargs family (generic_reg_update_ex) an equivalent declaration/consumer disagreement would feed stack garbage into MMIO writes (va_arg walks past the supplied triplets), which is why the header contract matters even where the runtime effect today is nil. AMD's own code comment (dc_helper.c:223-225) acknowledges the varargs family is "prone to stack corruption type of bugs".

Fix

One-line header correction (fix.diff) making the declaration identical to the definition (also unifying the return type to uint32_t). No behavior change for existing callers.

Fix validation

patch --dry-run OK. No kernel build possible on the guest (DC requires AMD display hardware; guest has no DRM device), but the edit is declaration-only — it cannot change codegen of the existing macro callers.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.