<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">Yeah, that sounds like a plan to me. Going to commit the patches I've already done in a minute, they still seem to implement quite a bit of what we want to do here. Regards, Christian. Am 12.10.2017 um 10:03 schrieb Liu, Monk: </div> <blockquote type="cite" cite="mid:BLUPR12MB0449E6EBA74EC3EB8918F71E844B0@BLUPR12MB0449.namprd12.prod.outlook.com"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> <div class="WordSection1"> V2 summary<o:p></o:p> <o:p> </o:p> Hi team<o:p></o:p> <o:p> </o:p> please give your comments<o:p></o:p> <o:p> </o:p> l When a job timed out (set from lockup_timeout kernel parameter), What KMD should do in TDR routine :<o:p></o:p> <o:p> </o:p> 1. Update adev->gpu_reset_counter, and stop scheduler first<o:p></o:p> 2. Set its fence error status to “ECANCELED”,<o:p></o:p> 3. Find the context behind this job, and set this context as “guilty” (will have a new member field in context structure – bool guilty)<o:p></o:p> a) There will be “bool * guilty” in entity structure, which points to its father context’s member – “bool guilty” when context initialized , so no matter we get context or entity, we always know if it is “guilty”<o:p></o:p> b) For kernel entity that used for VM updates, there is no context back it, so kernel entity’s “bool *guilty” always “NULL”.<o:p></o:p> c) The idea to skip the whole context is for consistence consideration, because we’ll fake signal the hang job in job_run(), so all jobs in its context shall be dropped otherwise either bad drawing/computing results or more GPU hang.<o:p></o:p> <o:p> </o:p> 4. Do GPU reset, which is can be some callbacks to let bare-metal and SR-IOV implement with their favor style <o:p></o:p> 5. After reset, KMD need to aware if the VRAM lost happens or not, bare-metal can implement some function to judge, while for SR-IOV I prefer to read it from GIM side (for initial version we consider it’s always VRAM lost, till GIM side change aligned)<o:p></o:p> 6. If VRAM lost hit, update adev->vram_lost_counter.<o:p></o:p> 7. Do GTT recovery and shadow buffer recovery.<o:p></o:p> 8. Re-schedule all JOBs in mirror list and restart scheduler<o:p></o:p> <o:p> </o:p> l For GPU scheduler function --- job_run()<o:p></o:p> 1. Before schedule a job to ring, checks if job->vram_lost_counter == adev->vram_lost_counter, and drop this job if mismatch<o:p></o:p> 2. Before schedule a job to ring, checks if job->entity->guilty is NULL or not, and drop this job if (guilty!=NULL && *guilty == TRUE)<o:p></o:p> 3. if a job is dropped:<o:p></o:p> a) set job’s sched_fence status to “ECANCELED”<o:p></o:p> b) fake/force signal job’s hw fence (no need to set hw fence’s status)<o:p></o:p> <o:p> </o:p> l For cs_wait() IOCTL:<o:p></o:p> After it found fence signaled, it should check if there is error on this fence and return the error status of this fence<o:p></o:p> <o:p> </o:p> l For cs_wait_fences() IOCTL:<o:p></o:p> Similar with above approach<o:p></o:p> <o:p> </o:p> l For cs_submit() IOCTL:<o:p></o:p> 1. check if current ctx been marked “guilty” and return “ECANCELED” if so.<o:p></o:p> 2. set job->vram_lost_counter with adev->vram_lost_counter, and return “ECANCELED” if ctx->vram_lost_counter != job->vram_lost_counter (Christian already submitted this patch)<o:p></o:p> a) discussion: can we return “ENODEV” if vram_lost_counter mismatch ? that way UMD know this context is under “device lost”<o:p></o:p> <o:p> </o:p> l Introduce a new IOCTL to let UMD query latest adev->vram_lost_counter:<o:p></o:p> <o:p> </o:p> l For amdgpu_ctx_query(): <o:p></o:p> n Don’t update ctx->reset_counter when querying this function, otherwise the query result is not consistent <o:p></o:p> n Set out->state.reset_status to “AMDGPU_CTX_GUILTY_RESET” if the ctx is “guilty”, no need to check “ctx->reset_counter”<o:p></o:p> n Set out->state.reset_status to “AMDGPU_CTX_INNOCENT_RESET” if the ctx isn’t “guilty” && ctx->reset_counter != adev->reset_counter <o:p></o:p> n Set out->state.reset_status to “AMDGPU_CTX_NO_RESET” if ctx->reset_counter == adev->reset_counter<o:p></o:p> n Set out->state.flags to “AMDGPU_CTX_FLAG_VRAM_LOST” if ctx->vram_lost_counter != adev->vram_lost_counter<o:p></o:p> u discussion: can we return “ENODEV” for amdgpu_ctx_query() if ctx->vram_lost_counter != adev->vram_lost_counter ? that way UMD know this context is under “device lost”<o:p></o:p> n UMD shall release this context if it is AMDGPU_CTX_GUILTY_RESET or its flags is “AMDGPU_CTX_FLAG_VRAM_LOST”<o:p></o:p> <o:p> </o:p> For UMD behavior we still have something need to consider:<o:p></o:p> If MESA creates a new context from an old context (share list?? I’m not familiar with UMD , David Mao shall have some discuss on it with Nicolai), the new created context’s vram_lost_counter<o:p></o:p> And reset_counter shall all be ported from that old context , otherwise CS_SUBMIT will not block it which isn’t correct <o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> Need your feedback, thx<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: amd-gfx [<a class="moz-txt-link-freetext" href="mailto:amd-gfx-bounces@lists.freedesktop.org">mailto:amd-gfx-bounces@lists.freedesktop.org</a>] On Behalf Of Liu, Monk Sent: 2017年10月11日 13:34 To: Koenig, Christian <a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a>; Haehnle, Nicolai <a class="moz-txt-link-rfc2396E" href="mailto:Nicolai.Haehnle@amd.com"><Nicolai.Haehnle@amd.com></a>; Olsak, Marek <a class="moz-txt-link-rfc2396E" href="mailto:Marek.Olsak@amd.com"><Marek.Olsak@amd.com></a>; Deucher, Alexander <a class="moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"><Alexander.Deucher@amd.com></a> Cc: Ramirez, Alejandro <a class="moz-txt-link-rfc2396E" href="mailto:Alejandro.Ramirez@amd.com"><Alejandro.Ramirez@amd.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Filipas, Mario <a class="moz-txt-link-rfc2396E" href="mailto:Mario.Filipas@amd.com"><Mario.Filipas@amd.com></a>; Ding, Pixel <a class="moz-txt-link-rfc2396E" href="mailto:Pixel.Ding@amd.com"><Pixel.Ding@amd.com></a>; Li, Bingley <a class="moz-txt-link-rfc2396E" href="mailto:Bingley.Li@amd.com"><Bingley.Li@amd.com></a>; Jiang, Jerry (SW) <a class="moz-txt-link-rfc2396E" href="mailto:Jerry.Jiang@amd.com"><Jerry.Jiang@amd.com></a> Subject: TDR and VRAM lost handling in KMD:<o:p></o:p> </div> </div> <o:p> </o:p> Hi Christian & Nicolai,<o:p></o:p> <o:p> </o:p> We need to achieve some agreements on what should MESA/UMD do and what should KMD do, please give your comments with “okay” or “No” and your idea on below items,<o:p></o:p> <o:p> </o:p> l When a job timed out (set from lockup_timeout kernel parameter), What KMD should do in TDR routine :<o:p></o:p> <o:p> </o:p> 1. Update adev->gpu_reset_counter, and stop scheduler first, (gpu_reset_counter is used to force vm flush after GPU reset, out of this thread’s scope so no more discussion on it)<o:p></o:p> 2. Set its fence error status to “ETIME”,<o:p></o:p> 3. Find the entity/ctx behind this job, and set this ctx as “guilty”<o:p></o:p> 4. Kick out this job from scheduler’s mirror list, so this job won’t get re-scheduled to ring anymore.<o:p></o:p> 5. Kick out all jobs in this “guilty” ctx’s KFIFO queue, and set all their fence status to “ECANCELED”<o:p></o:p> 6. Force signal all fences that get kicked out by above two steps, otherwise UMD will block forever if waiting on those fences<o:p></o:p> 7. Do gpu reset, which is can be some callbacks to let bare-metal and SR-IOV implement with their favor style <o:p></o:p> 8. After reset, KMD need to aware if the VRAM lost happens or not, bare-metal can implement some function to judge, while for SR-IOV I prefer to read it from GIM side (for initial version we consider it’s always VRAM lost, till GIM side change aligned)<o:p></o:p> 9. If VRAM lost not hit, continue, otherwise:<o:p></o:p> a) Update adev->vram_lost_counter,<o:p></o:p> b) Iterate over all living ctx, and set all ctx as “guilty” since VRAM lost actually ruins all VRAM contents<o:p></o:p> c) Kick out all jobs in all ctx’s KFIFO queue, and set all their fence status to “ECANCELDED”<o:p></o:p> 10. Do GTT recovery and VRAM page tables/entries recovery (optional, do we need it ???)<o:p></o:p> 11. Re-schedule all JOBs remains in mirror list to ring again and restart scheduler (for VRAM lost case, no JOB will re-scheduled)<o:p></o:p> <o:p> </o:p> l For cs_wait() IOCTL:<o:p></o:p> After it found fence signaled, it should check with “dma_fence_get_status” to see if there is error there,<o:p></o:p> And return the error status of fence<o:p></o:p> <o:p> </o:p> l For cs_wait_fences() IOCTL:<o:p></o:p> Similar with above approach<o:p></o:p> <o:p> </o:p> l For cs_submit() IOCTL:<o:p></o:p> It need to check if current ctx been marked as “guilty” and return “ECANCELED” if so<o:p></o:p> <o:p> </o:p> l Introduce a new IOCTL to let UMD query vram_lost_counter:<o:p></o:p> This way, UMD can also block app from submitting, like @Nicolai mentioned, we can cache one copy of vram_lost_counter when enumerate physical device, and deny all <o:p></o:p> gl-context from submitting if the counter queried bigger than that one cached in physical device. (looks a little overkill to me, but easy to implement ) <o:p></o:p> UMD can also return error to APP when creating gl-context if found current queried vram_lost_counter bigger than that one cached in physical device.<o:p></o:p> <o:p> </o:p> BTW: I realized that gl-context is a little different with kernel’s context. Because for kernel. BO is not related with context but only with FD, while in UMD, BO have a backend<o:p></o:p> gl-context, so block submitting in UMD layer is also needed although KMD will do its job as bottom line <o:p></o:p> <o:p> </o:p> l Basically “vram_lost_counter” is exposure by kernel to let UMD take the control of robust extension feature, it will be UMD’s call to move, KMD only deny “guilty” context from submitting<o:p></o:p> <o:p> </o:p> <o:p> </o:p> Need your feedback, thx<o:p></o:p> <o:p> </o:p> We’d better make TDR feature landed ASAP<o:p></o:p> <o:p> </o:p> BR Monk<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> </div> </blockquote> </body> </html>