| Message ID | 20221019083325.214960-1-andrzej.hajda@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/guc: add CAT error handler | expand |
On 10/19/2022 01:33, Andrzej Hajda wrote: > In case of catastrophic errors GuC sends notification, which results in > cryptic message. Let's add handler which, for starters, dumps state > of affected engine. See below - the notification is sent by the GPU memory subsystem not the GuC. Also, not sure what you mean by 'cryptic message'. It would be better to say something like: Bad GPU memory accesses can result in catastrophic error notifications being send from the GPU to the KMD via the GuC. Add a handler to process the notification by printing a kernel message and dumping the related engine state (if appropriate). > > Signed-off-by: Andrzej Hajda<andrzej.hajda@intel.com> > --- > .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 3 ++ > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 +++++++++++++++++++ > 4 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > index f359bef046e0b2..f9a1c5642855e3 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > @@ -138,6 +138,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, > INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507, > INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A, > + INTEL_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR = 0x6000, > INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002, > INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003, > INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004, > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > index 804133df1ac9b4..61b412732d095a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > @@ -445,6 +445,8 @@ int intel_guc_engine_failure_process_msg(struct intel_guc *guc, > const u32 *msg, u32 len); > int intel_guc_error_capture_process_msg(struct intel_guc *guc, > const u32 *msg, u32 len); > +int intel_guc_cat_error_process_msg(struct intel_guc *guc, > + const u32 *msg, u32 len); > > struct intel_engine_cs * > intel_guc_lookup_engine(struct intel_guc *guc, u8 guc_class, u8 instance); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > index 2b22065e87bf9a..f55f724e264407 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > @@ -1035,6 +1035,9 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r > CT_ERROR(ct, "Received GuC exception notification!\n"); > ret = 0; > break; > + case INTEL_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR: > + ret = intel_guc_cat_error_process_msg(guc, payload, len); > + break; > default: > ret = -EOPNOTSUPP; > break; > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 693b07a977893d..94f91dfa3ec456 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -4659,6 +4659,37 @@ int intel_guc_engine_failure_process_msg(struct intel_guc *guc, > return 0; > } > > +int intel_guc_cat_error_process_msg(struct intel_guc *guc, > + const u32 *msg, u32 len) > +{ > + struct drm_i915_private *i915 = guc_to_gt(guc)->i915; > + struct intel_engine_cs *engine; > + struct intel_context *ce; > + struct drm_printer p; > + unsigned long flags; > + int ctx_id; > + > + if (unlikely(len != 1)) { > + drm_dbg(&i915->drm, "Invalid length %u", len); > + return -EPROTO; > + } > + ctx_id = msg[0]; > + > + xa_lock_irqsave(&guc->context_lookup, flags); > + ce = g2h_context_lookup(guc, ctx_id); > + if (ce) > + engine = ce->engine; > + xa_unlock_irqrestore(&guc->context_lookup, flags); > + if (unlikely(!ce || !engine)) > + return -EPROTO; This is not correct. There is no guarantee that a context will be associated with the catastrophic error. Certain categories of error are caused by context activity but other categories are caused by 'global' activity (e.g. CPU hitting a bad address over the PCIe bus, other non-engine hardware in the GPU, etc.). In those cases, the ctx_id field will be set to an invalid context (~0, I believe) and the ce will come back as NULL. > + > + drm_err(&i915->drm, "%s: CAT error reported by GuC\n", engine->name); This is not accurate. The error is reported *via* GuC but it is reported by the hardware. The GuC does not do anything to detect memory errors. GuC receives an interrupt from the hardware to say a catastrophic error has occurred, it then reads some data from a FIFO and basically returns that data back to the KMD. The sole extent of the GuC processing is to convert the hardware id into a context id. Saying that GuC is reporting the error implies that GuC knows what went wrong or could maybe even do something about it. Certainly it makes it likely that bugs will be logged against the GuC which is not correct. Also, you are not reporting the context in the error message. Assuming that a context has been provided then that is important information to debug where the error came from. You should at least include ce->guc_id.id (which should match ctx_id) in the message. John. > + p = drm_info_printer(i915->drm.dev); > + intel_engine_dump(engine, &p, "%s\n", engine->name); > + > + return 0; > +} > + > void intel_guc_find_hung_context(struct intel_engine_cs *engine) > { > struct intel_guc *guc = &engine->gt->uc.guc;
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index f359bef046e0b2..f9a1c5642855e3 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -138,6 +138,7 @@ enum intel_guc_action { INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507, INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A, + INTEL_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR = 0x6000, INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002, INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003, INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004, diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 804133df1ac9b4..61b412732d095a 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -445,6 +445,8 @@ int intel_guc_engine_failure_process_msg(struct intel_guc *guc, const u32 *msg, u32 len); int intel_guc_error_capture_process_msg(struct intel_guc *guc, const u32 *msg, u32 len); +int intel_guc_cat_error_process_msg(struct intel_guc *guc, + const u32 *msg, u32 len); struct intel_engine_cs * intel_guc_lookup_engine(struct intel_guc *guc, u8 guc_class, u8 instance); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index 2b22065e87bf9a..f55f724e264407 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -1035,6 +1035,9 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r CT_ERROR(ct, "Received GuC exception notification!\n"); ret = 0; break; + case INTEL_GUC_ACTION_NOTIFY_MEMORY_CAT_ERROR: + ret = intel_guc_cat_error_process_msg(guc, payload, len); + break; default: ret = -EOPNOTSUPP; break; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 693b07a977893d..94f91dfa3ec456 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4659,6 +4659,37 @@ int intel_guc_engine_failure_process_msg(struct intel_guc *guc, return 0; } +int intel_guc_cat_error_process_msg(struct intel_guc *guc, + const u32 *msg, u32 len) +{ + struct drm_i915_private *i915 = guc_to_gt(guc)->i915; + struct intel_engine_cs *engine; + struct intel_context *ce; + struct drm_printer p; + unsigned long flags; + int ctx_id; + + if (unlikely(len != 1)) { + drm_dbg(&i915->drm, "Invalid length %u", len); + return -EPROTO; + } + ctx_id = msg[0]; + + xa_lock_irqsave(&guc->context_lookup, flags); + ce = g2h_context_lookup(guc, ctx_id); + if (ce) + engine = ce->engine; + xa_unlock_irqrestore(&guc->context_lookup, flags); + if (unlikely(!ce || !engine)) + return -EPROTO; + + drm_err(&i915->drm, "%s: CAT error reported by GuC\n", engine->name); + p = drm_info_printer(i915->drm.dev); + intel_engine_dump(engine, &p, "%s\n", engine->name); + + return 0; +} + void intel_guc_find_hung_context(struct intel_engine_cs *engine) { struct intel_guc *guc = &engine->gt->uc.guc;
In case of catastrophic errors GuC sends notification, which results in cryptic message. Let's add handler which, for starters, dumps state of affected engine. Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> --- .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 ++ drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 3 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 +++++++++++++++++++ 4 files changed, 37 insertions(+)