Re: [PATCH v6 6/7] drm/xe: capture L3 node status registers in devcoredump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: MCR read of L3_NODE_IDLE warns on Xe_LP in devcoredump_snapshot()
--

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

drm/xe: capture L3 node status registers in devcoredump

This commit adds definitions for the L3_NODE_IDLE and L3STAT_LBCF_GFX
registers and captures them in a new node status section during devcoredump
snapshots.

> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index f884fc2e38aaf..28331505bb34e 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -454,6 +454,9 @@
>  #define   L3_ESC_MASK				REG_BIT(0)
>  #define   L3_ESC(value)				REG_FIELD_PREP(L3_ESC_MASK, value)
>  
> +#define L3_NODE_IDLE				XE_REG_MCR(0xb0b0)
> +#define L3STAT_LBCF_GFX				XE_REG_MCR(0xb128)
> +
>  #define XEHP_L3NODEARBCFG			XE_REG_MCR(0xb0b4)
>  #define   XEHP_LNESPARE				REG_BIT(19)
>  
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 90e16c89dd963..714b63e9653df 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -121,6 +121,11 @@ static const struct xe_reg_desc xe_dbg_gam_pf_reglist[] = {
>  	XE_REG_DESC_MCR(PAGE_FLTRPTQ_TAIL, "PAGE_FLTRPTQ_TAIL"),
>  };
>  
> +static const struct xe_reg_desc xe_dbg_l3_node_reglist[] = {
> +	XE_REG_DESC_MCR(L3_NODE_IDLE, "L3_NODE_IDLE"),
> +	XE_REG_DESC_MCR(L3STAT_LBCF_GFX, "L3STAT_LBCF_GFX"),
> +};

[ ... ]

> @@ -609,6 +625,10 @@ static void xe_devcoredump_capture_gt_regs(struct xe_devcoredump_snapshot *ss,
>  						       other_gt,
>  						       XE_DBG_REGSET_GAM_PF,
>  						       XE_GT_TYPE_MAIN);
> +	ss->ns = xe_dbg_gt_snapshot_capture(gt,
> +					    other_gt,
> +					    XE_DBG_REGSET_L3_NODE,
> +					    XE_GT_TYPE_MAIN);
>  }

[Severity: Medium]
Will capturing XE_DBG_REGSET_L3_NODE unconditionally on all platforms trigger
a kernel warning on Xe_LP platforms (such as TGL, RKL, ADL, and DG1) during
devcoredump_snapshot()?

In xe_devcoredump_capture_gt_regs(), xe_dbg_gt_snapshot_capture() is called
for XE_DBG_REGSET_L3_NODE without a platform or MCR range check, which leads
to xe_dbg_reg_snapshot_capture_target() reading L3_NODE_IDLE (0xb0b0) through
xe_gt_mcr_unicast_read_any() because reg.mcr is set:

drivers/gpu/drm/xe/xe_devcoredump.c:xe_dbg_reg_snapshot_capture_target() {
	...
	for (i = 0; i < reglist->num_regs; i++) {
		if (reglist->regs[i].reg.mcr)
			snap->values[i] = xe_gt_mcr_unicast_read_any(gt,
								     reglist->regs[i].mcr_reg);
		else
			snap->values[i] = xe_mmio_read32(&gt->mmio, reglist->regs[i].reg);
	}
	...
}

However, in xe_gt_mcr_init_early(), the 0x00b000-0x00b0ff range is not
registered in any MCR steering table on Xe_LP platforms (or on Xe3p platforms
where GRAPHICS_VERx100(xe) >= 3510):

drivers/gpu/drm/xe/xe_gt_mcr.c:xe_gt_mcr_init_early() {
		...
		} else {
			gt->steering[L3BANK].ranges = xelp_l3bank_steering_table;
			gt->steering[DSS].ranges = xelp_dss_steering_table;
		}
}

When a GPU hang or job timeout occurs on those platforms, calling
xe_gt_mcr_unicast_read_any() for L3_NODE_IDLE reaches
xe_gt_mcr_get_nonterminated_steering() and fires xe_gt_WARN():

drivers/gpu/drm/xe/xe_gt_mcr.c:xe_gt_mcr_get_nonterminated_steering() {
	...
	xe_gt_WARN(gt, true,
		   "Did not find MCR register %#x in any MCR steering table\n",
		   reg.addr);
	*group = 0;
	*instance = 0;

	return true;
}

Should xe_dbg_reg_snapshot_capture_target() check xe_gt_mcr_check_reg() before
using xe_gt_mcr_unicast_read_any(), or should the L3_NODE capture be guarded
by the appropriate platform version check?

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