Re: [PATCH v2 08/11] drm/xe: Prep page reclaim in tlb inval job

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

On Wed, Dec 03, 2025 at 10:42:00PM -0700, Nguyen, Brian3 wrote:
> On Wednesday, December 3, 2025 5:21 PM, Matthew Brost wrote:
> > On Wed, Dec 03, 2025 at 05:22:10PM -0700, Nguyen, Brian3 wrote:
> > > On Wednesday, December 3, 2025 3:14 PM, Matthew Brost wrote:
> > > > On Thu, Nov 27, 2025 at 07:02:09AM +0800, Brian Nguyen wrote:
> > > > > Use page reclaim list as indicator if page reclaim action is
> > > > > desired and pass it to tlb inval fence to handle.
> > > > >
> > > > > Job will need to maintain its own embedded copy to ensure lifetime
> > > > > of PRL exist until job has run.
> > > > >
> > > > > v2:
> > > > >  - Use xe variant of WARN_ON (Michal)
> > > > >
> > > > > Signed-off-by: Brian Nguyen <brian3.nguyen@intel.com>
> > > > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/xe/xe_pt.c            |  6 ++++++
> > > > >  drivers/gpu/drm/xe/xe_tlb_inval_job.c | 26
> > > > > ++++++++++++++++++++++++++ drivers/gpu/drm/xe/xe_tlb_inval_job.h |
> > > > > 4 ++++
> > > > >  3 files changed, 36 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/xe/xe_pt.c
> > > > > b/drivers/gpu/drm/xe/xe_pt.c index 347b111dc097..833d6762dd8d
> > > > > 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_pt.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > > > > @@ -2498,6 +2498,12 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct xe_vma_ops *vops)
> > > > >  			goto kill_vm_tile1;
> > > > >  		}
> > > > >  		update.ijob = ijob;
> > > > > +		if (pt_update_ops->prl.num_entries != XE_PAGE_RECLAIM_INVALID_LIST) {
> > > > > +			xe_tlb_inval_job_add_page_reclaim(ijob, &pt_update_ops->prl);
> > > > > +			/* Release ref from alloc, job will now handle it */
> > > > > +			xe_page_reclaim_entries_put(pt_update_ops->prl.entries);
> > > > > +			pt_update_ops->prl.entries = NULL;
> > > > > +		}
> > > >
> > > > Do we not need to do this on the media-GT? Or is issuing PRL a single GuC sufficient?
> > > >
> > > > Everything else looks good.
> > > >
> > >
> > > It should be sufficient per tile. KMD will request GuC FW per tile and
> > > afterwards GuC FW will issue Page Reclamation to all the primary XeTLBs, for GT and media within the tile.
> > >
> > > However, I realize that this means we should be disabling PPC flush
> > > for media TLB invalidation otherwise, it will conflict and do a PPC
> > > flush anyway. Will have to rework some of the logic to handle this if so... Taking another look at this.
> > 
> > Yes, that's a good catch and would've missed this part. Now I'm wondering if on upstream devices if flushing the PPC cache on both
> > primary GT and media GT is really neeeded. If not, this might be small perf win.
> > 
> 
> I asked Nekkalapu, Satyanarayana for HW perspective confirmation. Media doesn't have any PPC to speak of,
> so, it looks like PPC flush bit for media translate to a NOP so we wouldn't be getting any perf here on upstream,
> maybe skipping a handshake at most.
> 
> So, regardless of PPC bit enabled or disabled for media, it shouldn't interfere with the primary GT caches.
> 
> Therefore, I will plan to keep this logic the same for simplicity unless you suggest otherwise.
> 

Can you just add a comment around why we only add the PRL to the primary
GT? To naive reader of the code it immediately looks like a bug that PRL
is only attached to the primary GT.

Matt

> Brian
> 
> > Matt
> > 
> > >
> > > Brian
> > >
> > > > Matt
> > > >
> > > > >
> > > > >  		if (tile->media_gt) {
> > > > >  			dep_scheduler = to_dep_scheduler(q, tile->media_gt); diff
> > > > > --git a/drivers/gpu/drm/xe/xe_tlb_inval_job.c
> > > > > b/drivers/gpu/drm/xe/xe_tlb_inval_job.c
> > > > > index dbd3171fff12..2185f42b9644 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_tlb_inval_job.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_job.c
> > > > > @@ -7,7 +7,9 @@
> > > > >  #include "xe_dep_job_types.h"
> > > > >  #include "xe_dep_scheduler.h"
> > > > >  #include "xe_exec_queue.h"
> > > > > +#include "xe_gt_printk.h"
> > > > >  #include "xe_gt_types.h"
> > > > > +#include "xe_page_reclaim.h"
> > > > >  #include "xe_tlb_inval.h"
> > > > >  #include "xe_tlb_inval_job.h"
> > > > >  #include "xe_migrate.h"
> > > > > @@ -116,6 +118,7 @@ xe_tlb_inval_job_create(struct xe_exec_queue *q, struct xe_tlb_inval *tlb_inval,
> > > > >  	job->start = start;
> > > > >  	job->end = end;
> > > > >  	job->fence_armed = false;
> > > > > +	xe_page_reclaim_list_init(&job->prl);
> > > > >  	job->dep.ops = &dep_job_ops;
> > > > >  	job->type = type;
> > > > >  	kref_init(&job->refcount);
> > > > > @@ -149,6 +152,25 @@ xe_tlb_inval_job_create(struct xe_exec_queue *q, struct xe_tlb_inval *tlb_inval,
> > > > >  	return ERR_PTR(err);
> > > > >  }
> > > > >
> > > > > +/**
> > > > > + * xe_tlb_inval_job_add_page_reclaim() - Embed PRL into a TLB job
> > > > > + * @job: TLB invalidation job that may trigger reclamation
> > > > > + * @prl: Page reclaim list populated during unbind
> > > > > + *
> > > > > + * Copies @prl into the job and takes an extra reference to the
> > > > > +entry page so
> > > > > + * ownership can transfer to the TLB fence when the job is pushed.
> > > > > + */
> > > > > +void xe_tlb_inval_job_add_page_reclaim(struct xe_tlb_inval_job *job,
> > > > > +				       struct xe_page_reclaim_list *prl) {
> > > > > +	struct xe_device *xe = gt_to_xe(job->q->gt);
> > > > > +
> > > > > +	xe_gt_WARN_ON(job->q->gt, !xe->info.has_page_reclaim_hw_assist);
> > > > > +	job->prl = *prl;
> > > > > +	/* Pair with put after bo creation */
> > > > > +	xe_page_reclaim_entries_get(job->prl.entries);
> > > > > +}
> > > > > +
> > > > >  static void xe_tlb_inval_job_destroy(struct kref *ref)  {
> > > > >  	struct xe_tlb_inval_job *job = container_of(ref, typeof(*job),
> > > > > @@
> > > > > -159,6 +181,10 @@ static void xe_tlb_inval_job_destroy(struct kref *ref)
> > > > >  	struct xe_device *xe = gt_to_xe(q->gt);
> > > > >  	struct xe_vm *vm = job->vm;
> > > > >
> > > > > +	/* BO creation retains a copy (if used), so no longer needed */
> > > > > +	if (job->prl.entries)
> > > > > +		xe_page_reclaim_entries_put(job->prl.entries);
> > > > > +
> > > > >  	if (!job->fence_armed)
> > > > >  		kfree(ifence);
> > > > >  	else
> > > > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_job.h
> > > > > b/drivers/gpu/drm/xe/xe_tlb_inval_job.h
> > > > > index 4d6df1a6c6ca..03d6e21cd611 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_tlb_inval_job.h
> > > > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_job.h
> > > > > @@ -12,6 +12,7 @@ struct dma_fence;  struct xe_dep_scheduler;
> > > > > struct xe_exec_queue;  struct xe_migrate;
> > > > > +struct xe_page_reclaim_list;
> > > > >  struct xe_tlb_inval;
> > > > >  struct xe_tlb_inval_job;
> > > > >  struct xe_vm;
> > > > > @@ -21,6 +22,9 @@ xe_tlb_inval_job_create(struct xe_exec_queue *q, struct xe_tlb_inval *tlb_inval,
> > > > >  			struct xe_dep_scheduler *dep_scheduler,
> > > > >  			struct xe_vm *vm, u64 start, u64 end, int type);
> > > > >
> > > > > +void xe_tlb_inval_job_add_page_reclaim(struct xe_tlb_inval_job *job,
> > > > > +				       struct xe_page_reclaim_list *prl);
> > > > > +
> > > > >  int xe_tlb_inval_job_alloc_dep(struct xe_tlb_inval_job *job);
> > > > >
> > > > >  struct dma_fence *xe_tlb_inval_job_push(struct xe_tlb_inval_job
> > > > > *job,
> > > > > --
> > > > > 2.52.0
> > > > >