Re: [PATCH v3 1/7] drm/xe/devcoredump: Capture GT fuse registers in devcoredump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Hardcoded array index used to check the POWERGATE_ENABLE register is fragile.
--

commit b7d03cc3b56be0544af3370f72a4384e4131de66
Author: Nareshkumar Gollakoti <naresh.kumar.g@intel.com>

    drm/xe/devcoredump: Capture GT fuse registers in devcoredump
    
    Capture fuse-related GT register state in the Xe devcoredump to preserve
    additional hardware configuration information for postmortem debugging.

> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c

[ ... ]

> +static const struct xe_reg_desc xe3p_gt_rpm_reglist[] = {
> +	XE_REG_DESC(RPM_GCD, "RPM_GCD"),
> +	XE_REG_DESC(POWERGATE_ENABLE, "POWERGATE_ENABLE"),
> +};

[ ... ]

> +static void xe_gt_rpm_snapshot_print(struct drm_printer *p,
> +				     const struct xe_dbg_reg_snapshot *snap)
> +{
> +	xe_dbg_reg_snapshot_print(p, snap);
> +
> +	if (snap && snap->num_regs >= 2 &&
> +	    !(snap->regs[1].value & RENDER_POWERGATE_ENABLE))
[Severity: Low]
Is it safe to use a hardcoded array index in xe_gt_rpm_snapshot_print() here?

This check tightly couples the print logic to the exact ordering of elements in
the xe3p_gt_rpm_reglist array, relying on POWERGATE_ENABLE remaining at index
1. If a future commit inserts or reorders elements in xe3p_gt_rpm_reglist,
it could silently break this print logic by evaluating the wrong register's
value.

Could this be made more robust against future modifications to the underlying
static register list?

> +		drm_puts(p, "render powergating appears not enabled\n");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903141834.1315675-1-naresh.kumar.g@intel.com?part=1