[PATCH v3 1/1] drm/xe/vf: Track writes to inaccessible registers from VF

[Piotr Piórkowski]

Michal Wajdeczko <michal.wajdeczko at intel.com> wrote on sob [2024-lip-13 16:26:43 +0200]:
> Only limited set of registers is accessible for the VF driver and
> the hardware will silently drop writes to inaccessible registers.
> To improve our VF driver lets intercept all such writes to warn
> about such unexpected writes on debug builds or optionally allow
> to provide some substitution (as a potential future extension).
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Gustavo Sousa <gustavo.sousa at intel.com>
> Cc: Piotr Piórkowski <piotr.piorkowski at intel.com>
> ---
> v2: update commit message (Gustavo)
> v3: drop case for SOFTWARE_FLAGS_SPR33 (xe)
>     improve function description, add comment (Piotr)
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.h |  1 +
>  drivers/gpu/drm/xe/xe_mmio.c        |  6 +++++-
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index 41e46a00c01e..6a87d31c44e6 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -892,6 +892,32 @@ u32 xe_gt_sriov_vf_read32(struct xe_gt *gt, struct xe_reg reg)
>  	return rr->value;
>  }
>  
> +/**
> + * xe_gt_sriov_vf_write32 - Handle a write to an inaccessible register.
> + * @gt: the &xe_gt
> + * @reg: the register to write
> + * @val: value to write
> + *
> + * This function is for VF use only.
> + * Currently it will trigger a WARN if running on debug build.
> + */
> +void xe_gt_sriov_vf_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
> +{
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
> +
> +	xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
> +	xe_gt_assert(gt, !reg.vf);
> +
> +	/*
> +	 * In the future, we may want to handle selected writes to inaccessible
> +	 * registers in some custom way, but for now let's just log a warning
> +	 * about such attempt, as likely we might be doing something wrong.
> +	 */
> +	xe_gt_WARN(gt, IS_ENABLED(CONFIG_DRM_XE_DEBUG),
> +		   "VF is trying to write %#x to an inaccessible register %#x+%#x\n",
> +		   val, reg.addr, addr - reg.addr);
> +}
> +
>  /**
>   * xe_gt_sriov_vf_print_config - Print VF self config.
>   * @gt: the &xe_gt
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> index 0de7f8cbcfa6..e541ce57bec2 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> @@ -22,6 +22,7 @@ u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt);
>  u16 xe_gt_sriov_vf_guc_ids(struct xe_gt *gt);
>  u64 xe_gt_sriov_vf_lmem(struct xe_gt *gt);
>  u32 xe_gt_sriov_vf_read32(struct xe_gt *gt, struct xe_reg reg);
> +void xe_gt_sriov_vf_write32(struct xe_gt *gt, struct xe_reg reg, u32 val);
>  
>  void xe_gt_sriov_vf_print_config(struct xe_gt *gt, struct drm_printer *p);
>  void xe_gt_sriov_vf_print_runtime(struct xe_gt *gt, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index cf622d07e190..ea3c37d3e13f 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -171,7 +171,11 @@ void xe_mmio_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>  	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
>  	trace_xe_reg_rw(gt, true, addr, val, sizeof(val));
> -	writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
> +
> +	if (!reg.vf && IS_SRIOV_VF(gt_to_xe(gt)))
> +		xe_gt_sriov_vf_write32(gt, reg, val);
> +	else
> +		writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  }
>  
Now it looks ok:
Reviewed-by: Piotr Piórkowski <piotr.piorkowski at intel.com>

>  u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> -- 
> 2.43.0
> 

--