Re: [PATCH v3 2/2] drm/xe/vf: Fix up GGTT mappings on S4 resume across VFs.

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Nov 18, 2025 at 09:08:11AM -0800, Matthew Brost wrote:
> On Tue, Nov 18, 2025 at 12:47:41PM +0000, Satyanarayana K V P wrote:
> > When SR-IOV is enabled, a VM may suspend on one VF and resume on a
> > different VF after S4 sleep cycle. Because GGTT space is not virtualized,
> > this transition can lead to incorrect GGTT references. To address this,
> > validate the GGTT space during resume and apply necessary fixups if the
> > VF has changed.
> > 
> > Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> 
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> 

I was little to fast with the RB, one question below.

> > Cc: Tomasz Lis <tomasz.lis@intel.com>
> > 
> > ---
> > V2 -> V3:
> > - Fixed review comments (Michal).
> > - Added a new function xe_gt_sriov_vf_resume_fixups() which handshakes
> > with GUC and does GGTT fixups.
> > - Updated commit message.
> > 
> > V1 -> V2:
> > - Rebased to latest drm-tip.
> > ---
> >  drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 38 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_gt_sriov_vf.h |  1 +
> >  drivers/gpu/drm/xe/xe_pm.c          |  7 ++++++
> >  drivers/gpu/drm/xe/xe_sriov_vf.c    | 30 +++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_sriov_vf.h    |  1 +
> >  5 files changed, 77 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > index be7986537aad..87aa1d6576d6 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > @@ -1335,6 +1335,44 @@ int xe_gt_sriov_vf_init(struct xe_gt *gt)
> >  					vf_migration_fini, gt);
> >  }
> >  
> > +/**
> > + * xe_gt_sriov_vf_resume_early() - Resume from S4.
> > + * @gt: the &xe_gt.
> > + *
> > + * This function shall be called only by VF.
> > + *
> > + * Returns: 0 if the operation completed successfully, or a negative
> > + * error code otherwise.
> > + */
> > +int xe_gt_sriov_vf_resume_early(struct xe_gt *gt)
> > +{
> > +	u64 start, size;
> > +	s64 shift;
> > +	int err;
> > +
> > +	xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
> > +
> > +	err = xe_gt_sriov_vf_bootstrap(gt);
> > +	if (unlikely(err))
> > +		goto out;
> > +
> > +	err = vf_query_ggtt_info(gt, &start, &size, &shift);
> > +	if (unlikely(err))
> > +		goto out;

Now that this function has split out query of shift into 2 functions,
don't we also need to call vf_fixup_ggtt_nodes too - the function will
set the set the base, size, and shift any existing GGTT nodes. With
that, why even split this function as it seems to do all the steps we
need here.

Matt

> > +
> > +	if (shift) {
> > +		err = vf_post_migration_fixups(gt);
> > +		if (unlikely(err))
> > +			goto out;
> > +	}
> > +
> > +out:
> > +	if (unlikely(err))
> > +		xe_gt_sriov_err(gt, "Fixups during resume failed (%d)\n", err);
> > +
> > +	return err;
> > +}
> > +
> >  /**
> >   * xe_gt_sriov_vf_recovery_pending() - VF post migration recovery pending
> >   * @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 af40276790fa..68026b561d7d 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> > @@ -25,6 +25,7 @@ void xe_gt_sriov_vf_migrated_event_handler(struct xe_gt *gt);
> >  
> >  int xe_gt_sriov_vf_init_early(struct xe_gt *gt);
> >  int xe_gt_sriov_vf_init(struct xe_gt *gt);
> > +int xe_gt_sriov_vf_resume_early(struct xe_gt *gt);
> >  bool xe_gt_sriov_vf_recovery_pending(struct xe_gt *gt);
> >  
> >  u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt);
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 4f8688fd3f00..a6fcddc47d62 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -24,6 +24,7 @@
> >  #include "xe_late_bind_fw.h"
> >  #include "xe_pcode.h"
> >  #include "xe_pxp.h"
> > +#include "xe_sriov_vf.h"
> >  #include "xe_sriov_vf_ccs.h"
> >  #include "xe_trace.h"
> >  #include "xe_vm.h"
> > @@ -248,6 +249,12 @@ int xe_pm_resume(struct xe_device *xe)
> >  
> >  	xe_display_pm_resume_early(xe);
> >  
> > +	if (IS_SRIOV_VF(xe)) {
> > +		err = xe_sriov_vf_resume_early(xe);
> > +		if (unlikely(err))
> > +			goto err;
> > +	}
> > +
> >  	/*
> >  	 * This only restores pinned memory which is the memory required for the
> >  	 * GT(s) to resume.
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.c b/drivers/gpu/drm/xe/xe_sriov_vf.c
> > index 39c829daa97c..a33f86b7986d 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_vf.c
> > +++ b/drivers/gpu/drm/xe/xe_sriov_vf.c
> > @@ -191,6 +191,36 @@ int xe_sriov_vf_init_late(struct xe_device *xe)
> >  	return xe_sriov_vf_ccs_init(xe);
> >  }
> >  
> > +/**
> > + * xe_sriov_vf_resume_early() - Early resume operations for SR-IOV Virtual
> > + * Function.
> > + * @xe: the &xe_device to resume
> > + *
> > + * This function shall be called only by VF.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int xe_sriov_vf_resume_early(struct xe_device *xe)
> > +{
> > +	struct xe_gt *gt;
> > +	int err = 0;
> > +	u8 id;
> > +
> > +	xe_assert(xe, IS_SRIOV_VF(xe));
> > +
> > +	for_each_gt(gt, xe, id) {
> > +		err = xe_gt_sriov_vf_resume_early(gt);
> > +		if (unlikely(err))
> > +			goto out;
> > +	}
> > +
> > +out:
> > +	if (unlikely(err))
> > +		xe_sriov_err(xe, "VF resume early failed with (%d)\n", err);
> > +
> > +	return err;
> > +}
> > +
> >  static int sa_info_vf_ccs(struct seq_file *m, void *data)
> >  {
> >  	struct drm_info_node *node = m->private;
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.h b/drivers/gpu/drm/xe/xe_sriov_vf.h
> > index e967d4166a43..54fe6eedeff6 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_vf.h
> > +++ b/drivers/gpu/drm/xe/xe_sriov_vf.h
> > @@ -13,6 +13,7 @@ struct xe_device;
> >  
> >  void xe_sriov_vf_init_early(struct xe_device *xe);
> >  int xe_sriov_vf_init_late(struct xe_device *xe);
> > +int xe_sriov_vf_resume_early(struct xe_device *xe);
> >  bool xe_sriov_vf_migration_supported(struct xe_device *xe);
> >  void xe_sriov_vf_migration_disable(struct xe_device *xe, const char *fmt, ...);
> >  void xe_sriov_vf_debugfs_register(struct xe_device *xe, struct dentry *root);
> > -- 
> > 2.51.0
> >