Re: [PATCH] drm/xe/pf: Sanitize VF scratch registers on FLR

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on pon [2024-wrz-02 21:29:53 +0200]:
> Some VF accessible registers (like GuC scratch registers) must be
> explicitly reset during the FLR. While this is today done by the GuC
> firmware, according to the design, this should be responsibility of
> the PF driver, as future platforms may require more registers to be
> reset. Likewise GuC, the PF can access VFs registers by adding some
> platform specific offset to the original register address.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_pf.c         | 52 +++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_sriov_pf.h         |  1 +
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c |  3 +-
>  3 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
> index 905f409db74b..919d960165d5 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
> @@ -5,8 +5,10 @@
>  
>  #include <drm/drm_managed.h>
>  
> +#include "regs/xe_guc_regs.h"
>  #include "regs/xe_regs.h"
>  
> +#include "xe_gt.h"
>  #include "xe_gt_sriov_pf.h"
>  #include "xe_gt_sriov_pf_config.h"
>  #include "xe_gt_sriov_pf_control.h"
> @@ -89,6 +91,56 @@ void xe_gt_sriov_pf_init_hw(struct xe_gt *gt)
>  	xe_gt_sriov_pf_service_update(gt);
>  }
>  
> +static u32 pf_get_vf_regs_stride(struct xe_device *xe)
> +{
> +	return GRAPHICS_VERx100(xe) > 1200 ? 0x400 : 0x1000;
> +}

NIT: Maybe it's worth moving it to the header.
At first it seemed to me that we have some header xe_sriov_regs.h,
but I see that for SR-IOV we use xe_regs.h - What do you think to move it there?

> +
> +static struct xe_reg xe_reg_vf_to_pf(struct xe_reg vf_reg, unsigned int vfid, u32 stride)
> +{
> +	struct xe_reg pf_reg = vf_reg;
> +
> +	pf_reg.vf = 0;
> +	pf_reg.addr += stride * vfid;
> +
> +	return pf_reg;
> +}
> +
> +static void pf_clear_vf_scratch_regs(struct xe_gt *gt, unsigned int vfid)
> +{
> +	u32 stride = pf_get_vf_regs_stride(gt_to_xe(gt));
> +	struct xe_reg scratch;
> +	int n, count;
> +
> +	if (xe_gt_is_media_type(gt)) {
> +		count = MED_VF_SW_FLAG_COUNT;
> +		for (n = 0; n < count; n++) {
> +			scratch = xe_reg_vf_to_pf(MED_VF_SW_FLAG(n), vfid, stride);
> +			xe_mmio_write32(gt, scratch, 0);
> +		}
> +	} else {
> +		count = VF_SW_FLAG_COUNT;
> +		for (n = 0; n < count; n++) {
> +			scratch = xe_reg_vf_to_pf(VF_SW_FLAG(n), vfid, stride);
> +			xe_mmio_write32(gt, scratch, 0);
> +		}
> +	}
> +}
> +
> +/**
> + * xe_gt_sriov_pf_sanitize_hw() - Reset hardware state related to a VF.
> + * @gt: the &xe_gt
> + * @vfid: the VF identifier
> + *
> + * This function can only be called on PF.
> + */
> +void xe_gt_sriov_pf_sanitize_hw(struct xe_gt *gt, unsigned int vfid)
> +{
> +	xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt)));
> +
> +	pf_clear_vf_scratch_regs(gt, vfid);
> +}
> +
>  /**
>   * xe_gt_sriov_pf_restart - Restart SR-IOV support after a GT reset.
>   * @gt: the &xe_gt
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf.h
> index f0cb726a6919..96fab779a906 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.h
> @@ -11,6 +11,7 @@ struct xe_gt;
>  #ifdef CONFIG_PCI_IOV
>  int xe_gt_sriov_pf_init_early(struct xe_gt *gt);
>  void xe_gt_sriov_pf_init_hw(struct xe_gt *gt);
> +void xe_gt_sriov_pf_sanitize_hw(struct xe_gt *gt, unsigned int vfid);
>  void xe_gt_sriov_pf_restart(struct xe_gt *gt);
>  #else
>  static inline int xe_gt_sriov_pf_init_early(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> index 02f7328bd6ce..b4fd5a81aff1 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> @@ -9,6 +9,7 @@
>  
>  #include "xe_device.h"
>  #include "xe_gt.h"
> +#include "xe_gt_sriov_pf.h"
>  #include "xe_gt_sriov_pf_config.h"
>  #include "xe_gt_sriov_pf_control.h"
>  #include "xe_gt_sriov_pf_helpers.h"
> @@ -1008,7 +1009,7 @@ static bool pf_exit_vf_flr_reset_mmio(struct xe_gt *gt, unsigned int vfid)
>  	if (!pf_exit_vf_state(gt, vfid, XE_GT_SRIOV_STATE_FLR_RESET_MMIO))
>  		return false;
>  
> -	/* XXX: placeholder */
> +	xe_gt_sriov_pf_sanitize_hw(gt, vfid);
>  
>  	pf_enter_vf_flr_send_finish(gt, vfid);
>  	return true;

LGTM:
Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

> -- 
> 2.43.0
> 

--