Re: [PATCH v6 4/7] drm/xe: dump GAM page fault report registers in devcoredump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: WARN in xe_gt_mcr_get_nonterminated_steering() on Xe_LP coredump
--

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

drm/xe: dump GAM page fault report registers in devcoredump

This commit adds PAGE_FLTRPTQ_HEAD and PAGE_FLTRPTQ_TAIL as MCR register
definitions and captures them in the devcoredump snapshot for the main GT.

> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index 220f0968f9db8..ad8aa6e10e361 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -530,6 +530,9 @@
>  #define   LMEM_EN				REG_BIT(31)
>  #define   LMTT_DIR_PTR				REG_GENMASK(30, 0) /* in multiples of 64KB */
>  
> +#define PAGE_FLTRPTQ_HEAD			XE_REG_MCR(0xcf90)
> +#define PAGE_FLTRPTQ_TAIL			XE_REG_MCR(0xcf94)
> +
>  #define HALF_SLICE_CHICKEN5			XE_REG_MCR(0xe188, XE_REG_OPTION_MASKED)
>  #define   DISABLE_SAMPLE_G_PERFORMANCE		REG_BIT(0)
>  
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index cd2c641111d68..90e16c89dd963 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
[ ... ]
> @@ -114,6 +116,11 @@ static const struct xe_reg_desc xe_dbg_media_gt_rpm_reglist[] = {
>  	XE_REG_DESC(MIRROR_PM_DEBUG, "MIRROR_PM_DEBUG"),
>  };
>  
> +static const struct xe_reg_desc xe_dbg_gam_pf_reglist[] = {
> +	XE_REG_DESC_MCR(PAGE_FLTRPTQ_HEAD, "PAGE_FLTRPTQ_HEAD"),
> +	XE_REG_DESC_MCR(PAGE_FLTRPTQ_TAIL, "PAGE_FLTRPTQ_TAIL"),
> +};
> +
[ ... ]
> @@ -234,8 +264,13 @@ xe_dbg_reg_snapshot_capture_target(struct xe_gt *gt,
>  
>  	snap->desc	= reglist;
>  	snap->num_regs = reglist->num_regs;
> -	for (i = 0; i < reglist->num_regs; i++)
> -		snap->values[i] = xe_mmio_read32(&gt->mmio, reglist->regs[i].reg);
> +	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);
> +	}
>  
>  	return snap;
>  }
[ ... ]
> @@ -565,6 +605,10 @@ static void xe_devcoredump_capture_gt_regs(struct xe_devcoredump_snapshot *ss,
>  							  other_gt,
>  							  XE_DBG_REGSET_GUC,
>  							  XE_GT_TYPE_MEDIA);
> +	ss->gam_pf_report = xe_dbg_gt_snapshot_capture(gt,
> +						       other_gt,
> +						       XE_DBG_REGSET_GAM_PF,
> +						       XE_GT_TYPE_MAIN);
>  }

[Severity: High]
Will capturing XE_DBG_REGSET_GAM_PF unconditionally on all platforms trigger a
kernel warning in xe_gt_mcr_get_nonterminated_steering() on Xe_LP platforms
(such as TGL, DG1, and ADL)?

When a devcoredump is taken, the capture path runs:

  devcoredump_snapshot()
    -> xe_devcoredump_capture_gt_regs()
      -> xe_dbg_gt_snapshot_capture(..., XE_DBG_REGSET_GAM_PF, XE_GT_TYPE_MAIN)
        -> xe_dbg_reg_snapshot_capture_target()
          -> xe_gt_mcr_unicast_read_any()
            -> xe_gt_mcr_get_nonterminated_steering()

Because PAGE_FLTRPTQ_HEAD (0xcf90) and PAGE_FLTRPTQ_TAIL (0xcf94) are defined
with XE_REG_MCR(), reglist->regs[i].reg.mcr is always set, so
xe_dbg_reg_snapshot_capture_target() unconditionally calls
xe_gt_mcr_unicast_read_any().

However, in drivers/gpu/drm/xe/xe_gt_mcr.c:xe_gt_mcr_init_early(), the
0x00c800 to 0x00cfff range is only added to MCR steering tables on DG2, PVC,
and platforms with GRAPHICS_VERx100(xe) >= 1270. On Xe_LP platforms, only the
L3BANK and DSS tables are registered:

    } else {
        gt->steering[L3BANK].ranges = xelp_l3bank_steering_table;
        gt->steering[DSS].ranges = xelp_dss_steering_table;
    }

Neither table covers 0xcf90 or 0xcf94, so
xe_gt_mcr_get_nonterminated_steering() will not find a match and will hit:

    xe_gt_WARN(gt, true,
               "Did not find MCR register %#x in any MCR steering table\n",
               reg.addr);

before attempting a steered read on a non-MCR register range.

Should xe_dbg_reg_snapshot_capture_target() check xe_gt_mcr_check_reg(), or
should capturing XE_DBG_REGSET_GAM_PF be restricted to platforms that support
these GAM page fault registers as MCR?

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