Re: [PATCH v3 2/2] drm/amdgpu: Add TLB flush after MES user queue unmap
Alex Deucher <[email protected]>
| Newsgroups | org.freedesktop.lists.amd-gfx |
|---|---|
| Message-ID | <CADnq5_Pj0gOHMupA+X9x6F-71TQJDeDiJQ6kdFgrLdzLh-z70Q@mail.gmail.com> |
On Sun, Aug 16, 2026 at 1:25 PM Hosur, Priya <[email protected]> wrote: > > AMD General > > > Hi Felix, Lijo, > > > > Thanks for the clarification. I understand now - TLB flush is only needed after batch queue unmapping (eviction/suspension), not after each individual queue unmap. > > > > Alex - based on Felix's feedback, the userq unmap path doesn't need the same fix since it's per-queue unmap rather than batch unmap. > I think it's still needed when we evict all KGD userqs for a process (technically per GPU VM instance). E.g., the eviction fence case. Alex > > > I'll drop patch 2/2 (userq fix) and send v4 with only the KFD fix. > > > > Thanks and Regards, > Priya Hosur > > > > From: Lazar, Lijo <[email protected]> > Sent: Friday, August 14, 2026 10:56 PM > To: Kuehling, Felix <[email protected]>; Hosur, Priya <[email protected]>; [email protected]; Deucher, Alexander <[email protected]>; Khatri, Sunil <[email protected]>; Liu, Shaoyun <[email protected]>; Limonciello, Mario <[email protected]>; Koenig, Christian <[email protected]> > Cc: Vishwakarma, Pratik <[email protected]>; Gopalakrishnan, Veerabadhran (Veera) <[email protected]> > Subject: Re: [PATCH v3 2/2] drm/amdgpu: Add TLB flush after MES user queue unmap > > > > AMD General > > > > Thanks for the details. Didn't realise that it is one by one removal. I thought it was something like unmap-all for a PAS id. > > > > Thanks, > > Lijo > > ________________________________ > > From: Kuehling, Felix <[email protected]> > Sent: Friday, 14 August 2026 22:17:50 > To: Lazar, Lijo <[email protected]>; Hosur, Priya <[email protected]>; [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Khatri, Sunil <[email protected]>; Liu, Shaoyun <[email protected]>; Limonciello, Mario <[email protected]>; Koenig, Christian <[email protected]> > Cc: Vishwakarma, Pratik <[email protected]>; Gopalakrishnan, Veerabadhran (Veera) <[email protected]> > Subject: Re: [PATCH v3 2/2] drm/amdgpu: Add TLB flush after MES user queue unmap > > > > AMD General > > > > We don't need to flush TLBs after every queue unmap. We only need to do it if there is an expectation that memory access is quiesced after unmapping. In the case of KFD, we are doing it when we're unmapping all queues, e.g. in MMU notifiers, evictions or when the debugger needs to stop the queues and look at memory contents. We only need one TLB flush after unmapping many queues. > > > > Therefore, MES should flush TLBs after every queue unmap. OTOH, MES must flush TLBs by itself when it switches VMIDs. This is not under driver control. > > > > I don't know what the requirements are for quiescing memory access after unmapping amdgpu user mode queues. > > > > Regards, > > Felix > > ________________________________ > > From: Lazar, Lijo <[email protected]> > Sent: Friday, August 14, 2026 12:29 > To: Hosur, Priya <[email protected]>; [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Khatri, Sunil <[email protected]>; Kuehling, Felix <[email protected]>; Liu, Shaoyun <[email protected]>; Limonciello, Mario <[email protected]>; Koenig, Christian <[email protected]> > Cc: Vishwakarma, Pratik <[email protected]>; Gopalakrishnan, Veerabadhran (Veera) <[email protected]>; Hosur, Priya <[email protected]> > Subject: Re: [PATCH v3 2/2] drm/amdgpu: Add TLB flush after MES user queue unmap > > > > AMD General > > > > Hi, > > > > Shouldn't this be a FW fix? > > > > Thanks, > > Lijo > > ________________________________ > > From: Priya Hosur <[email protected]> > Sent: Friday, 14 August 2026 21:48:40 > To: [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Khatri, Sunil <[email protected]>; Kuehling, Felix <[email protected]>; Liu, Shaoyun <[email protected]>; Lazar, Lijo <[email protected]>; Limonciello, Mario <[email protected]>; Koenig, Christian <[email protected]> > Cc: Vishwakarma, Pratik <[email protected]>; Gopalakrishnan, Veerabadhran (Veera) <[email protected]>; Hosur, Priya <[email protected]> > Subject: [PATCH v3 2/2] drm/amdgpu: Add TLB flush after MES user queue unmap > > > > Similar to the KFD fix, MES does not perform heavy-weight TLB > invalidation after unmapping user queues. Add > amdgpu_gmc_flush_gpu_tlb_pasid() after MES unmap succeeds in > amdgpu_userq_unmap_helper() to ensure in-flight memory accesses > complete before memory is freed or migrated. > > Change-Id: I94fe2c84547723b6b73816ce8d727a54bd773a6a > Signed-off-by: Priya Hosur <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > index 6d3ed55e9ab4..9fcf15d69b7b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > @@ -375,6 +375,12 @@ static int amdgpu_userq_unmap_helper(struct amdgpu_usermode_queue *queue) > } else { > trace_amdgpu_userq_state_changed(queue, AMDGPU_USERQ_STATE_UNMAPPED); > queue->state = AMDGPU_USERQ_STATE_UNMAPPED; > + /* > + * Heavy-weight TLB flush after MES unmaps queue to ensure > + * in-flight memory accesses complete before memory is freed/migrated. > + * MES does not do this automatically unlike HWS. > + */ > + amdgpu_gmc_flush_gpu_tlb_pasid(adev, queue->vm->pasid, 2, true, 0); > } > } > > -- > 2.43.0