Re: [PATCH 1/3] drm/xe/xe3p_lpg: flush userptr/shrinker bo cachelines manually

[Matthew Auld <matthew.auld@intel.com>]

On 13/02/2026 17:16, Matt Roper wrote:
> On Fri, Feb 13, 2026 at 04:48:39PM +0000, Souza, Jose wrote:
>> On Fri, 2026-02-13 at 16:23 +0000, Upadhyay, Tejas wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Roper, Matthew D <matthew.d.roper@intel.com>
>>>> Sent: 12 February 2026 02:41
>>>> To: Upadhyay, Tejas <tejas.upadhyay@intel.com>
>>>> Cc: Brost, Matthew <matthew.brost@intel.com>; intel-
>>>> xe@lists.freedesktop.org; Auld, Matthew <matthew.auld@intel.com>;
>>>> thomas.hellstrom@linux.intel.com
>>>> Subject: Re: [PATCH 1/3] drm/xe/xe3p_lpg: flush userptr/shrinker bo
>>>> cachelines manually
>>>>
>>>> On Wed, Feb 11, 2026 at 07:06:05PM +0000, Upadhyay, Tejas wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Brost, Matthew <matthew.brost@intel.com>
>>>>>> Sent: 11 February 2026 05:32
>>>>>> To: Roper, Matthew D <matthew.d.roper@intel.com>
>>>>>> Cc: Upadhyay, Tejas <tejas.upadhyay@intel.com>; intel-
>>>>>> xe@lists.freedesktop.org; Auld, Matthew
>>>>>> <matthew.auld@intel.com>;
>>>>>> thomas.hellstrom@linux.intel.com
>>>>>> Subject: Re: [PATCH 1/3] drm/xe/xe3p_lpg: flush
>>>>>> userptr/shrinker bo
>>>>>> cachelines manually
>>>>>>
>>>>>> On Tue, Feb 10, 2026 at 01:05:25PM -0800, Matt Roper wrote:
>>>>>>> On Tue, Feb 10, 2026 at 06:21:22PM +0530, Tejas Upadhyay
>>>>>>> wrote:
>>>>>>>> "eXtended Architecture" (XA) tagged memory—memory shared
>>>> between
>>>>>> the
>>>>>>>> CPU and GPU
>>>>>>>
>>>>>>> I'm pretty sure this expansion of "XA" is wrong; where are
>>>>>>> you
>>>>>>> seeing this definition?  Everything in the bspec indicates
>>>>>>> that XA
>>>>>>> means "wb
>>>>>>> - transient app" (similar to how "XD" is 'wb - transient
>>>>>>> display").
>>>>>>> I'm not sure why exactly they picked "X" to refer to
>>>>>>> transient in
>>>>>>> both of these cases, but I've never seen any documentation
>>>>>>> that
>>>>>>> refers to it as "extended."
>>>>>>>
>>>>>>>> is treated differently from other GPU memory when the Media
>>>>>>>> engine is
>>>>>> power-gated.
>>>>>>>>
>>>>>>>> XA is *always* flushed, like at the end-of-submssion (and
>>>>>>>> maybe
>>>>>>>> other
>>>>>>>
>>>>>>> I assume you're referring to the fact that the driver
>>>>>>> performs
>>>>>>> flushes at the end of submission (via PIPE_CONTROL or
>>>>>>> MI_FLUSH_DW), and that depending on other state/optimizations
>>>>>>> in
>>>>>>> the system, those flushes may flush the entire device cache,
>>>>>>> or
>>>>>>> may only flush the subset of cache data that is not marked as
>>>>>>> transient.  The way you worded this was confusing since it
>>>>>>> makes
>>>>>>> it sound like cache flushes happen automatically somewhere in
>>>> hardware/firmware.
>>>>>>>
>>>>>>>> places), just that internally as an optimisation hw doesn't
>>>>>>>> need
>>>>>>>> to make that a full flush (which will also include XA) when
>>>>>>>> Media is off/powergated, since it doesn't need to worry
>>>>>>>> about GT
>>>>>>>> caches vs Media coherency, and only CPU vs GPU coherency,
>>>>>>>> so can
>>>>>>>> make that flush a targeted XA flush, since stuff tagged
>>>>>>>> with XA
>>>>>>>> now means it's shared with the CPU. The main implication is
>>>>>>>> that
>>>>>>>> we now need to somehow flush non-XA before freeing system
>>>>>>>> memory
>>>>>>>> pages, otherwise dirty cachelines could be flushed after
>>>>>>>> the
>>>>>>>> free (like if Media suddenly turns on and does a full
>>>>>>>> flush)
>>>>>>>
>>>>>>> This description seems really confusing.  My understanding is
>>>>>>> that
>>>>>>> marking something as wb-transient-app indicates that it might
>>>>>>> be
>>>>>>> accessed by something other than our graphics/media IP (i.e.,
>>>>>>> accessed from the CPU, exported to another device, etc.), so
>>>>>>> transient data truly does need to be flushed at the points in
>>>>>>> the
>>>>>>> driver where a flush typically happens.
>>>>>>>
>>>>>>> However when something is _not_ transient, then either:
>>>>>>>   - it's "private" to the GPU and only our graphics/media IP
>>>>>>> will be
>>>>>>>     accessing it
>>>>>>>   - it's bound with a coherent PAT index so that outside
>>>>>>> observers like
>>>>>>>     the CPU can snoop the device cache, even when the cache
>>>>>>> hasn't been
>>>>>>>     flushed
>>>>>>>
>>>>>>> If media is not active, then there's really no need to
>>>>>>> include
>>>>>>> non-transient data when an device cache flush happens since
>>>>>>> there's no real need for the data to get to RAM.  So that
>>>>>>> enables
>>>>>>> an optimization (which comes in your next patch), that allows
>>>>>>> flushes to only operate on the subset of the device cache
>>>>>>> tagged as
>>>> "transient" if media is idle.
>>>>>
>>>>> But what If we have stale non-XA marked pages for userptr, and
>>>>> that
>>>>> object moves out and at the same time media comes back, will end
>>>>> up in
>>>>> full flush and flush the stale entry to RAM.
>>>>
>>>> What makes userptr special here?  During general, active usage,
>>>> userptr would
>>>> be data that's accessible by the CPU, so it needs to either be
>>>> transient (so CPU
>>>> can see the data in RAM after explicit flushes) or it needs to be
>>>> using a
>>>> coherent PAT (so that the CPU can just snoop the GPU cache).  If
>>>> you marked
>>>> userptr as both non-XA and non-coherent, then that sounds likely to
>>>> be a
>>>> userspace bug (and probably something we can catch and reject as an
>>>> invalid
>>>> case on any Xe3p or later platforms that support this) since the
>>>> CPU wouldn't
>>>> have any reliable way of seeing GPU updates.
>>>
>>> Right. FYI @Mrozek, Michal @Souza, Jose
>>> For userptr, as explained above, it needs to be either coherent or XA
>>> pat index, or else KMD will reject as invalid case.
>>>
>>>
>> The coherency restriction is already in the uAPI:
>>
>> "Note: For userptr and externally imported dma-buf the kernel expects
>> either 1WAY or 2WAY for the @pat_index."
>>
>> Using 1 way is enough as Xe KMD does a PIPE_CONTROL flushing GPU caches
>> at the end of batch buffers.
> 
> But isn't that what we're discussing here?  1-way *won't* necessarily be
> enough anymore because PIPE_CONTROL instructions don't flush the entire
> cache anymore.  Whenever the GuC determines that media is inactive and
> activates the optimization, PIPE_CONTROL, MI_FLUSH_DW, etc. change
> behavior to only flush out the subset of data that was marked as
> app-transient; anything not marked that way doesn't get flushed now.  So
> there's a new requirement here that you ensure you're using an XA PAT
> index, or you switch to use 2-way coherency which will allow the CPU to
> snoop the GPU's caches.

That exactly matches my understanding also.

> 
> 
> Matt
> 
>>
>>>
>>>>
>>>> If something happens that changes the GTT mapping of an object,
>>>> then
>>>> doesn't that already trigger a TLB invalidation when necessary in
>>>> the driver
>>>> today?  It was my understanding that "heavy" TLB invalidations wait
>>>> for data
>>>> values to be globally observable before starting, so I think that
>>>> would ensure
>>>> that any non-XA data makes it to RAM before any binding changes,
>>>> object,
>>>> destruction, etc.?  Is there something special about userptr that
>>>> makes that
>>>> case more of a problem?
>>>>
>>>> I just found bspec page 74635 which gives an overview of the
>>>> various flush
>>>> and invalidate cases, and I don't see anything there that makes it
>>>> obvious to
>>>> me that userptr would be special.
>>>>
>>>>
>>>>>
>>>>>>>
>>>>>>> As you said, we eventually do want to force a flush of the
>>>>>>> non-transient data as well once we're freeing the underlying
>>>>>>> pages.
>>>>>>> So how do we do that?  It's not clear to me how the changes
>>>>>>> below
>>>>>>> are accomplishing that.  Is there a way to explicitly request
>>>>>>> a
>>>>>>> full device cache flush (ignoring the transient vs non-
>>>>>>> transient tagging)?
>>>>>>> Since the GuC handles the optimization in the next patch
>>>>>>> (toggling
>>>>>>> whether flushes are full flushes vs non-transient flushes
>>>>>>> depending on whether media is active), I thought there might
>>>>>>> be
>>>>>>> some kind of GuC interface to request "please do one full
>>>>>>> flush now, even
>>>> if media is idle."
>>>>>>>
>>>>>>
>>>>>> I’m not an expert here by any means, but everything above from
>>>>>> Matt
>>>>>> seems like valid concerns. Thomas also raised some concerns in
>>>>>> the
>>>>>> two previous revisions; again I’m not an expert, but reading
>>>>>> through
>>>>>> those, it doesn’t really seem like he received proper answers
>>>>>> to his
>>>> questions.
>>>>>
>>>>> Its forcing flush via tlb invalidation PPC flag under
>>>>> xe_invalidate_vma( ).
>>>>
>>>> By the way, what is "PPC?"  It seems like it's another new synonym
>>>> for the
>>>> device cache?  It's already really confusing that some of our
>>>> hardware docs use
>>>> a mix of both "L2" and "L3" to refer to the same device cache for
>>>> historical
>>>> reasons...
>>>>
>>>>
>>>> Matt
>>>>
>>>>>
>>>>>>
>>>>>> A couple of comments below.
>>>>>>
>>>>>>>
>>>>>>> Matt
>>>>>>>
>>>>>>>>
>>>>>>>> V2(MattA): Expand commit description
>>>>>>>>
>>>>>>>> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
>>>>>>>> ---
>>>>>>>>   drivers/gpu/drm/xe/xe_bo.c      |  3 ++-
>>>>>>>>   drivers/gpu/drm/xe/xe_device.c  | 23
>>>>>>>> +++++++++++++++++++++++
>>>>>>>> drivers/gpu/drm/xe/xe_device.h  |  1 +
>>>>>>>> drivers/gpu/drm/xe/xe_userptr.c |  3 ++-
>>>>>>>>   4 files changed, 28 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c
>>>>>>>> b/drivers/gpu/drm/xe/xe_bo.c index
>>>>>>>> e9180b01a4e4..4455886b211e
>>>>>>>> 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c
>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>>>>>>>> @@ -689,7 +689,8 @@ static int xe_bo_trigger_rebind(struct
>>>>>>>> xe_device *xe, struct xe_bo *bo,
>>>>>>>>
>>>>>>>>   		if (!xe_vm_in_fault_mode(vm)) {
>>>>>>>>   			drm_gpuvm_bo_evict(vm_bo, true);
>>>>>>>> -			continue;
>>>>>>>> +			if
>>>>>>>> (!xe_device_needs_cache_flush(xe))
>>>>>>>> +				continue;
>>>
>>> Matt R,
>>> This flush will be still needed as there can be non-xa buffers which
>>> can be evicted while media was off and stale entries can be flushed
>>> when media comes back on. Which was not case earlier as full flush
>>> was happening at regular sync points and that’s where this feature is
>>> bringing optimization now.
>>>
>>> Tejas
>>>
>>>>>>
>>>>>> This will trigger a TLB invalidation (and I assume a cache
>>>>>> flush)
>>>>>> every time we move or free memory in the 3D stack if it has a
>>>>>> binding. It also performs a synchronous wait on the BO being
>>>>>> idle.
>>>>>> Both of these are very expensive operations. I can’t imagine
>>>>>> the
>>>>>> granularity we want here is to do this on every move/free with
>>>>>> bindings.
>>>>>>
>>>>>> Also, for LR compute with preempt fences, we would trigger the
>>>>>> preempt fences during the wait, so a TLB invalidation after
>>>>>> this
>>>>>> seems unnecessary, though perhaps the cache flush is still
>>>>>> required?
>>>>>>
>>>>>> I think this needs a bit more explanation, because without
>>>>>> knowing a
>>>>>> lot about the exact requirements, the implementation does not
>>>>>> look
>>>> correct.
>>>>>
>>>>> The thing is that we are trying to solve problem with userptr
>>>>> with non-XA
>>>> pat, consider if that BO got moved while media is not active. As
>>>> soon as media
>>>> will come back active, stale cached entries of that object will be
>>>> flushed as part
>>>> of full flush , which may corrupt things.
>>>>> There was thinking that with this patch we would at least solve
>>>>> the problem
>>>> of corruption and later when page_reclamation feature comes in will
>>>> help in
>>>> performance as well. But now when page reclamation feature is
>>>> merged earlier
>>>> and it tightly coupled with bind/unbind some cases like discussed
>>>> above
>>>> (which are not doing unbind immediately on move/free) are missed in
>>>> reclamation.
>>>>>
>>>>> So thought was to let this solution go in with little perf hit
>>>>> and discuss with
>>>> page reclamation owner to come with cleaner solution together.
>>>>>
>>>>> Tejas
>>>>>>
>>>>>>>>   		}
>>>>>>>>
>>>>>>>>   		if (!idle) {
>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_device.c
>>>>>>>> b/drivers/gpu/drm/xe/xe_device.c index
>>>>>>>> 743c18e0c580..da2abed94bc0
>>>>>>>> 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/xe_device.c
>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_device.c
>>>>>>>> @@ -1097,6 +1097,29 @@ static void tdf_request_sync(struct
>>>>>>>> xe_device
>>>>>> *xe)
>>>>>>>>   	}
>>>>>>>>   }
>>>>>>>>
>>>>>>>> +/**
>>>>>>>> + * xe_device_needs_cache_flush - Whether the cache needs
>>>>>>>> to be
>>>>>>>> +flushed
>>>>>>>> + * @xe: The device to check.
>>>>>>>> + *
>>>>>>>> + * Return: true if the device needs cache flush, false
>>>>>>>> otherwise.
>>>>>>>> + */
>>>>>>>> +bool xe_device_needs_cache_flush(struct xe_device *xe) {
>>>>>>>> +	/* XA is *always* flushed, like at the end-of-
>>>>>>>> submssion (and
>>>>>>>> +maybe
>>>>>> other
>>>>>>>> +	 * places), just that internally as an
>>>>>>>> optimisation hw doesn't
>>>>>>>> +need to
>>>>>> make
>>>>>>>> +	 * that a full flush (which will also include XA)
>>>>>>>> when Media is
>>>>>>>> +	 * off/powergated, since it doesn't need to worry
>>>>>>>> about GT
>>>>>>>> +caches vs
>>>>>> Media
>>>>>>>> +	 * coherency, and only CPU vs GPU coherency, so
>>>>>>>> can make
>>>> that
>>>>>>>> +flush
>>>>>> a
>>>>>>>> +	 * targeted XA flush, since stuff tagged with XA
>>>>>>>> now means
>>>>>>>> +it's shared
>>>>>> with
>>>>>>>> +	 * the CPU. The main implication is that we now
>>>>>>>> need to
>>>>>>>> +somehow
>>>>>> flush non-XA before
>>>>>>>> +	 * freeing system memory pages, otherwise dirty
>>>>>>>> cachelines
>>>>>>>> +could be
>>>>>> flushed after the free
>>>>>>>> +	 * (like if Media suddenly turns on and does a
>>>>>>>> full flush)
>>>>>>>> +	 */
>>>>>>>> +	if (GRAPHICS_VER(xe) >= 35 && !IS_DGFX(xe))
>>>>>>>> +		return true;
>>>>>>>> +	return false;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>   void xe_device_l2_flush(struct xe_device *xe)  {
>>>>>>>>   	struct xe_gt *gt;
>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_device.h
>>>>>>>> b/drivers/gpu/drm/xe/xe_device.h index
>>>>>>>> 39464650533b..baf386e0e037
>>>>>>>> 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/xe_device.h
>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_device.h
>>>>>>>> @@ -184,6 +184,7 @@ void xe_device_snapshot_print(struct
>>>>>>>> xe_device *xe, struct drm_printer *p);
>>>>>>>>   u64 xe_device_canonicalize_addr(struct xe_device *xe, u64
>>>>>>>> address);
>>>>>>>>   u64 xe_device_uncanonicalize_addr(struct xe_device *xe,
>>>>>>>> u64
>>>>>>>> address);
>>>>>>>>
>>>>>>>> +bool xe_device_needs_cache_flush(struct xe_device *xe);
>>>>>>>>   void xe_device_td_flush(struct xe_device *xe);  void
>>>>>>>> xe_device_l2_flush(struct xe_device *xe);
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_userptr.c
>>>>>>>> b/drivers/gpu/drm/xe/xe_userptr.c index
>>>>>>>> e120323c43bc..b435ea7f9b66
>>>>>>>> 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/xe_userptr.c
>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_userptr.c
>>>>>>>> @@ -114,7 +114,8 @@ static void
>>>>>>>> __vma_userptr_invalidate(struct
>>>>>>>> xe_vm
>>>>>> *vm, struct xe_userptr_vma *uv
>>>>>>>>   				    false,
>>>>>>>> MAX_SCHEDULE_TIMEOUT);
>>>>>>>>   	XE_WARN_ON(err <= 0);
>>>>>>>>
>>>>>>>> -	if (xe_vm_in_fault_mode(vm) && userptr-
>>>>>>>>> initial_bind) {
>>>>>>>> +	if ((xe_vm_in_fault_mode(vm) ||
>>>>>>>> +xe_device_needs_cache_flush(vm-
>>>>>>> xe)) &&
>>>>>>>> +	    userptr->initial_bind) {
>>>>>>
>>>>>> Same concern with the LR preempt fence as above — the hardware
>>>>>> will
>>>>>> be interrupted via preempt fences, so it doesn’t seem necessary
>>>>>> to
>>>>>> invalidate the TLBs but perhaps we need a cflush and TLB
>>>>>> invalidation is the mechanism for that too?
>>>>>>
>>>>>> Matt
>>>>>>
>>>>>>>>   		err = xe_vm_invalidate_vma(vma);
>>>>>>>>   		XE_WARN_ON(err);
>>>>>>>>   	}
>>>>>>>> --
>>>>>>>> 2.52.0
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Roper
>>>>>>> Graphics Software Engineer
>>>>>>> Linux GPU Platform Enablement
>>>>>>> Intel Corporation
>>>>
>>>> --
>>>> Matt Roper
>>>> Graphics Software Engineer
>>>> Linux GPU Platform Enablement
>>>> Intel Corporation
>