[Intel-gfx] [RFC] drm/i915/guc: capture GuC logs if FW fails to load

[Chris Wilson]

On Fri, May 05, 2017 at 08:43:36AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 04/05/17 14:31, Chris Wilson wrote:
> >On Thu, May 04, 2017 at 09:26:35PM +0000, Srivatsa, Anusha wrote:
> >>>+void i915_guc_load_error_log_capture(struct drm_i915_private *i915) {
> >>>+	void *log, *buf;
> >>>+	struct i915_vma *vma = i915->guc.log.vma;
> >>>+
> >>>+	if (i915->gpu_error.guc_load_fail_log || !vma)
> >>>+		return;
> >>>+
> >>>+	/*
> >>>+	 * the vma should be already pinned and mapped for log runtime
> >>>+	 * management but let's play safe
> >>>+	 */
> >>>+	log = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
> >>>+	if (IS_ERR(log)) {
> >>>+		DRM_ERROR("Failed to pin guc_log vma\n");
> >>>+		return;
> >>>+	}
> >>>+
> >>>+	buf = kzalloc(GUC_LOG_SIZE, GFP_KERNEL);
> >>>+	if (buf) {
> >>>+		memcpy(buf, log, GUC_LOG_SIZE);
> >>>+		i915->gpu_error.guc_load_fail_log = buf;
> >>>+	} else {
> >>>+		DRM_ERROR("Failed to copy guc log\n");
> >>>+	}
> >>>+
> >>>+	i915_gem_object_unpin_map(vma->obj);
> >
> >You are trading a swappable object for unswappable kernel memory. If you
> >want to have the guc log after guc is disabled, just keep the log object
> >around.
> >-Chris
> >
> 
> I had considered that, but in the end I wasn't sure if that was
> acceptable in case we end up modifying the code to recycle the
> object for future load attempts, although that is very unlikely. I
> was however unconvinced myself of using kzalloc and that's mainly
> why this was an RFC :)
> I'll flip it to take an extra reference on the object.

Just make sure you unpin it. I would suggest killing the guc->log.vma
and taking the object and placing it somewhere else as you have for the
buf, but not i915->gpu_error as that doesn't fit (this isn't a runtime
error that we are tracking). i915->guc.last_load_log?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre