Hi Everyone, Can we please make some progress here?
I've still got 3 patch series on the mailing list to improve retry fault handling, without any feedback for many weeks now. Can someone please review these please? Thanks & best regards, Timur On 2026. május 14., csütörtök 23:24:49 közép-európai nyári idő Timur Kristóf wrote: > Hi Amir, > > Thanks, glad to hear that! > > I recommend to also apply "Add fence argument to amdgpu_vm_handle_fault()" > and "ACK the retry CAM after VM update finishes" as these fix the race > between the CAM and the SDMA, and should improve the reliability of the > whole mechanism in my opinion. > > In the meantime I think I figured out what I was missing. > Considering that the FORCE_MISS bits didn't change the behavior, I was > thinking that the curlpit must be something else, so I started looking into > the PTE that is being added. > > It all started working for me when I added some new PTE flags to > amdgpu_vm_handle_fault(): AMDGPU_PTE_IS_PTE and AMDGPU_PTE_NOALLOC. > With that, I cleaned up my patches and force-pushed them to the same branch: > > https://gitlab.freedesktop.org/Venemo/linux/-/commits/ven_retry_faults_navi4 > > What do you think? > > Best regards, > Timur > > > On Thursday, May 14, 2026 9:32:16 PM Central European Summer Time Shetaia, > > Amir wrote: > > AMD General > > > > Hi Timur, > > > > I cherry-picked your soft-IH-ring + timestamp-filter patches onto our > > build,> > > ran malloc 1 GiB sweep (N=10) on Navi4 (gfx1201): > > PASS 1st hang > > sR_r > > > > entered > > Baseline (no patches) 1/10 trial 6 97k > > > With your patches 2/10 trial 8 43k > > > > What I noticed on NV4: > > - 2× trials before failure (soft-IH unbound workqueue helping) > > - ~55% reduction in spurious svm_range_restore_pages (your > > timestamp-filter > > fix catches a real bug here too, not just Strix Halo) > > - Soft IH worker > > > visibly on events_unbound queue > > > > Same dma_fence_wait_timeout signature underneath, now on TWO workers > > (amdgpu_irq_handle_ih_soft + ttm_bo_delayed_delete, > > TTM cleanup gated on > > > the same fence chain). Central BO-clear/GCR deadlock unchanged, your > > patches fix adjacent bugs but that one needs the HW fix. > > Also tested UTCL0 retry-rate knob. On NV4 the gfx12 equivalents are > > regGL1C_UTCL0_RETRY / regGL1XC_UTCL0_RETRY > > (the TCP-level register moved > > > to GL1C-level on gfx12). Wrote INCR=0xff COUNT=0xf via umr to all SE/SA > > banks → 0/10 PASS, hung sooner. So, slowing UTCL0 retries doesn't help. > > deadlock isn't pressure-driven on UTCL0. > > > > AMIR SHETAIA > > Senior Software Development Engineer | AMD > > Software Platform Architecture Team > > -------------------------------------------------------------------------- > > -- ------------------------------------------------------ > > 1 Commerce Valley > > > Drive, Markham, ON L3T 7X6 > > LinkedIn | Instagram | X | amd.com > > > > > > > > > > -----Original Message----- > > From: Timur Kristóf <[email protected]> > > Sent: Thursday, May 14, 2026 1:03 PM > > To: Alex Deucher <[email protected]>; Shetaia, Amir > > <[email protected]> > > Cc: [email protected]; Deucher, > > > Alexander <[email protected]>; Koenig, Christian > > <[email protected]>; Marek Olšák <[email protected]>; Natalie Vock > > <[email protected]>; Melissa Wen <[email protected]> Subject: Re: [PATCH > > 0/6] drm/amdgpu: Improve retry fault handling > > Hi Amir, > > > > I pushed a work in progress branch here: > > https://gitlab.freedesktop.org/Venemo/linux/-/commits/ven_retry_faults_nav > > i4 > > > > > > This contains the two series that I sent to the mailing list yesterday: > > "Improve retry fault handling" > > "Improve soft IH ring" > > > > If we have to rely on the soft IH ring on Navi4, then we absolutely need > > the patches from "Improve soft IH ring", as it fixes a few bugs and > > switches the soft IH ring to use the system unbound workqueue. Otherwise > > it can happen that the soft IH ring fills up even before your CPU can > > process anything in it. > > (This is especially bad on HW that doesn't have the filter > > > CAM enabled, eg. Strix Halo.) > > On top of that, > > > > There is a patch to solve the race condition and make sure that the retry > > CAM is ACKed after the VM update is complete. On Navi 31, this works and > > successfully makes sure that the race is eliminated and each VM fault is > > only processed once. I'd like to submit these if you guys agree on the > > approach. > > > > And finally, there is a very messy patch that contains what I tried to do > > to get it all working on Navi 48. This has a lot of comments with my > > notes during development, and some attempts I tried to fix the issue > > (without success). > > > > Basically, what I did was: > > > > - Enable the retry CAM the same way as I did on IH 6.0 exactly like you > > said > > - Added the cam_index to gmc_v12_0_process_interrupt > > > - Added a few bits to gfxhub_v12_0 and gfx_v12_0 to actually enable retry > > faults (without that, the faults don't have the retry bit in src_data[1]) > > > > Now, what I observe on Navi 48 is: > > > > 1. Retry fault interrupt is triggered > > 2. Dispatched on the soft IH ring > > 3. The interrupt handler is called again on the soft IRQ ring 4. The fault > > is mitigated by amdgpu_vm_handle_fault() 5. Finally, the retry CAM is > > ACKed > > 6. As soon as the CAM is ACKed, I get another interrupt for the same > > address, and it goes on infinitely or until GPU reset is triggered. > > > > Attempts at fixing what I saw on Navi 48: > > > > - Tried to add a TLB flush to various places. Now that the race is > > mitigated, the TLB flush doesn't make things worse anymore, but it's also > > not helping. > > - Tried to set the FORCE_MISS bits on various cache related > > > registers, hoping that it would work around the possibility of the PTE/PDE > > not being updated in the cache. This didn't help either. - Tried to use > > amdgpu.vm_update_mode=3 to use the CPU to update the page tables, but that > > didn't help on Navi 48 - Tried to halt the CP using CP_ME_CNTL to see if > > it > > would stop sending interrupts, but it didn't. - Using umr I tried to > > disable the L2 cache and reset it with the GRBM, but that just broke the > > whole system. > > Other notes on the retry fault topic in general: > > > > - On GPUs that don't have the retry CAM (eg. Navi 1-2 and APUs) we'll need > > to consider disabling the VM fault interrupt while processing the retry, > > because the soft IH ring can fill up really quickly and the CPU may > > struggle to keep up with it. > > - In general (for all GPUs), I think it would > > > be better to update a larger VA range of subsequent pages rather than just > > a single page, in order to more efficiently mitigate VM faults to > > neighbouring pages. - Currently all of the invalid accesses are redirected > > to the same dummy page, which is a security hole because it means that a > > process that had an out of bounds write can leak data to a process that > > does an out of bounds read. I already talked to Christian about this, he > > has a few ideas how to fix it. > > Thanks & best regards, > > Timur > > > > > > On Thursday, May 14, 2026 5:04:00 PM Central European Summer Time Shetaia, > > > > Amir wrote: > > > AMD General > > > > > > > > > > > > Hi Timur, > > > > > > > > > > > > Thanks for clarifying. yes, please push the Navi 48 WIP branch when > > > you have a chance, useful to compare side-by-side. > > > > > > > > > > > > Patch 5 / WDOORBELL on ih_v7_0: I have been able to confirm that > > > doorbell ACK does not free the CAM slot on the NV4 family > > > > (gfx1200/1201). I > > > > > verified empirically, slots walk monotonically until the CAM fills, > > > then HW silently drops retry events. MMIO write to regIH_RETRY_CAM_ACK > > > does free it. Almost certainly required for your Navi 48 attempt. > > > Why PTE update doesn't reach UTC L0: Most likely a race between the > > > SDMA PTE-update job completing and the CAM ACK firing the > > > > IRQ for re-handling. > > > > > Until SDMA finishes the write + invalidation propagates, UTC L0 keeps > > > getting the stale "invalid" reply which is exactly your symptom. > > > To inspect with umr: clone > > > https://gitlab.freedesktop.org/tomstdenis/umr and use > > > scripts/diag/dump_all_cpc_info.sh (needs > > > > halt_if_hws_hang=1 > > > > > gpu_recovery=0). For UTC specifically: umr -i <gpu> -O bits -r > > > '*.*.regGCVM_L2_STATUS' (look at L2_BUSY and CONTEXT_DOMAIN_BUSY > > > per-VMID > > > bits) and regGCVM_L2_PROTECTION_FAULT_STATUS (empty + busy = > > > translation timeout, not denial). Page-table walk to confirm PTE > > > landed: sudo umr -i <gpu> -vm <vmid>@<va> 1. > > > Your ACK-after-SDMA-fence idea: good catch. We hadn't dug into that > > > race because the BO-clear deadlock dominates on NV4, but > > > > the ordering bug is > > > > > almost certainly there too. Fence callback is the right shape, cleaner > > > than blocking the worker. Would be a generic fix benefiting both Navi 3 > > > and 4. > > Happy to review if you take a swing. > > > > IH1 routing: fair point. I just inherited the existing IH0 path and > > > haven't tried IH1. On the list once CAM ACK and fence ordering are > > > stable. > > > > > > > > > > > > AMIR SHETAIA > > > Senior Software Development Engineer | AMD Software Platform > > > Architecture Team > > > ---------------------------------------------------------------------- > > > ------ > > > ------------------------------------------------------ > > > > 1 Commerce Valley > > > > > Drive, Markham, ON L3T 7X6 > > > LinkedIn | Instagram | X | amd.com > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > From: Timur Kristóf <[email protected]> > > > Sent: Wednesday, May 13, 2026 6:12 PM > > > To: Alex Deucher <[email protected]>; Shetaia, Amir > > > <[email protected]> > > > > Cc: [email protected]; Deucher, > > > > > Alexander <[email protected]>; Koenig, Christian > > > <[email protected]>; Marek Olšák <[email protected]>; Natalie > > > Vock <[email protected]>; Melissa Wen <[email protected]> Subject: Re: > > > [PATCH 0/6] drm/amdgpu: Improve retry fault handling Hi Amir, > > > > > > > Timur, you are right, I see your patch 6 already does the MMIO ACK > > > > for gmc_v11_0/ih_v6_0. I missed that. The gap is only in patch 5's > > > > ih_v7_0 implementation, which still does WDOORBELL. that's where I'd > > > > suggest swapping in MMIO for NV4. > > > > > > First, let me clear up a slight misunderstanding here. The patch > > > series that I sent here only contains what I managed to get working on > > > Navi 31, it does not contain any code from my attempt at Navi 48. > > > > > > > > > > > > The patch 5 is just a slight refactor of the pre-existing code and > > > (intentionally) doesn't contain any functional changes. It seems that > > > IH 7.1 relies on the doorbell, so I didn't want to remove it, albeit I > > > have no means to verify if that actually works or not. > > > > > > > > > > > > If you like, I can push a few WIP patches to a different branch > > > tomorrow to show you exactly what I did on Navi 48. However, I > > > wouldn't want to submit that to the mailing list without first making > > > sure > > > that it works well. > > > > > > > 1. "Fault never resolves on NV48" different shape from our > > > > broken-CAM-ACK symptom. > > > > > > > > > > > > > > > > > > > > > > > > You're right, those are different. Our cam-walk-monotonically > > > > symptom only shows up when CAM is enabled but the ACK is broken. > > > > > > > > On your NV48 setup CAM > > > > > > > > probably isn't enabled at all (your patch 6 only enables it for > > > > ih_v6_0_irq_init, no equivalent in ih_v7_0_irq_init) > > > > > > For the attempt on Navi 48, I enabled the CAM in ih_v7_0 the same way > > > I do for > > > > ih_v6_0 in the series. But, because I couldn't get it fully working, I > > > > > didn't include any of that code in the series. > > > > > > > so retries fire > > > > repeatedly on the IH ring instead of being deduped by CAM. That > > > > matches what you're seeing .. amdgpu_vm_handle_fault keeps being > > > > called but each call is on a fresh IRQ for the same address. > > > > Two things that could be happening underneath: > > > > - The fault handler runs but the updated PTE never reaches UTC L0 > > > > (TLB invalidation gap). On NV4 we see this as "valid PTEs failing to > > > > translate" > > > > in our UMR captures. > > > > > > I think this explanation may fit what I saw. > > > Why is it not reaching UTC L0? > > > Also, how do you inspect this stuff in umr? > > > > > > > 2. What bits we check on src_data[2]: > > > > > > > > > > > > > > > > > > > > > > > > Honestly, we don't use src_data[2] for retry detection. We use it > > > > only for > > > > the cam_index: cam_index = entry->src_data[2] & 0x3ff; /* low 10 > > > > bits > > > > = > > > > CAM slot */ > > > > > > > > > > > > > > > > > > > > > > > > For retry detection we initially used the gfx9 constant on > > > > src_data[1] like you, but observed the bit cleared on a lot of NV4 > > > > events that should have been retries (waves were hung in xnack-stall > > > > but no IH event matched). > > > > So > > > > > > we just go through the retry path unconditionally on NV4 > > > > > > I see. I think I needed to change some gfxhub registers to get those > > > src_data bits that you are missing. > > > > > > > and let > > > > amdgpu_vm_handle_fault sort it out via SVM range migration. May be > > > > specific to gfx1201 / our test path > > > > > > My test case is a simple Vulkan shader which I am executing with > > > vkrunner. > > > Each shader invocation does an out of bounds read from a different > > > page. For Navi 31 (and Strix Halo), I started out with just 1 page > > > fault, and once I could mitigate that reliably, I turned it up to > > > several > > > hundred faults. > > > > > > > 3. TLB flush making it worse .. clue about what to do: > > > > > > > > > > > > > > > > > > > > > > > > Honest answer: not really, not a SW-only fix. Our 1 GiB hang is an > > > > architectural deadlock ... ih_soft_work blocks on a dma_fence for an > > > > SDMA BO-clear, the BO-clear is stalled on a GCR (cache flush) > > > > request, and the GC cache block isn't ACK'ing the GCR while UTC L2 > > > > is saturated by the user shader's XNACK retry storm. Adding a TLB > > > > flush adds another translation request to the same saturated UTC, > > > > which is why it makes things worse. > > > > > > This may be related to a flaw in amdgpu_gmc_handle_retry_fault(): > > > what the function does is first call amdgpu_vm_handle_fault() which > > > kicks off an SDMA job to update page tables, and then it ACKs the filter > > > CAM. > > However, at the moment when the CAM is ACKed, the SDMA job is not > > > > finished yet, so the CP sees that the page is still invalid and the page > > > fault interrupt is fired again. I actually noticed that on Navi 31 too, > > > but it's just not fatal there. > > > > It just basically handles the same page fault twice. > > > > > Once we solve this flaw, I would like to propose to enable retry > > > faults by default on Navi 3. > > > Here is an idea for a solution: > > > > > > > > > > > > Instead of ACKing the CAM right away, we should do it after the SDMA > > > fence is signalled, ie. when we are sure the page tables are updated. > > > Maybe we can set a callback on the fence and do it there, though it > > > would require a slight code churn to get that to work. > > > > > > > > > > > > What do you think? > > > > > > > 4. IH1 ring on NV4: > > > > > > > > > > > > > > > > > > > > > > > > Same as you ... retry faults on NV4 always come in on IH0. We > > > > delegate from > > > > IH0 to ih.ring_soft (amdgpu_irq_delegate(adev, entry, 8)) so the > > > > SVM/migration path can sleep, but the original entry is on IH0. We > > > > haven't tried IH1 routing. > > > > > > Why, though? > > > > > > > > > > > > The ih_v7_0 code does set up the IH1 ring and configure it exactly the > > > same as the ih_v6_0, so I don't see why it wouldn't work? > > > > > > > Re your branch: thanks for the gitlab link, easier than digging > > > > through patchwork. > > > > I'll cherry-pick patches 1, 3, 4 into our test build to see if patch > > > > 4 cleans up the timestamp filter delta we're seeing (97k entered / > > > > 2.8k completed at 1 GiB might be partly explained by your Strix Halo > > > > bug). > > > > > > The timestamp issue actually gave me an endless headache when I first > > > got into this topic in December. I hope the patch helps! > > > > > > > > > > > > Best regards, > > > Timur
