Re: [Intel-gfx] [PATCH v2 1/3] drm: Add support for panic message output

["Noralf Trønnes" <noralf@tronnes.org>]

Den 12.03.2019 17.17, skrev Ville Syrjälä:
> On Tue, Mar 12, 2019 at 11:47:04AM +0100, Michel Dänzer wrote:
>> On 2019-03-11 6:42 p.m., Noralf Trønnes wrote:
>>> This adds support for outputting kernel messages on panic().
>>> A kernel message dumper is used to dump the log. The dumper iterates
>>> over each DRM device and it's crtc's to find suitable framebuffers.
>>>
>>> All the other dumpers are run before this one except mtdoops.
>>> Only atomic drivers are supported.
>>>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>>>  [...]
>>>
>>> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
>>> index f0b34c977ec5..f3274798ecfe 100644
>>> --- a/include/drm/drm_framebuffer.h
>>> +++ b/include/drm/drm_framebuffer.h
>>> @@ -94,6 +94,44 @@ struct drm_framebuffer_funcs {
>>>  		     struct drm_file *file_priv, unsigned flags,
>>>  		     unsigned color, struct drm_clip_rect *clips,
>>>  		     unsigned num_clips);
>>> +
>>> +	/**
>>> +	 * @panic_vmap:
>>> +	 *
>>> +	 * Optional callback for panic handling.
>>> +	 *
>>> +	 * For vmapping the selected framebuffer in a panic context. Must
>>> +	 * be super careful about locking (only trylocking allowed).
>>> +	 *
>>> +	 * RETURNS:
>>> +	 *
>>> +	 * NULL if it didn't work out, otherwise an opaque cookie which is
>>> +	 * passed to @panic_draw_xy. It can be anything: vmap area, structure
>>> +	 * with more details, just a few flags, ...
>>> +	 */
>>> +	void *(*panic_vmap)(struct drm_framebuffer *fb);
>>
>> FWIW, the panic_vmap hook cannot work in general with the amdgpu/radeon
>> drivers:
>>
>> Framebuffers are normally tiled, writing to them with the CPU results in
>> garbled output.
>>

In which case the driver needs to support the ->panic_draw_xy callback,
or maybe it's possible to make a generic helper for tiled buffers.

>> With a discrete GPU having a large amount of VRAM, the framebuffer may
>> not be directly CPU accessible at all.
>>

I would have been nice to know how Windows works around this.

>>
>> There would need to be a mechanism for switching scanout to a linear,
>> CPU accessible framebuffer.
> 
> I suppose panic_vmap() could just provide a linear temp buffer
> to the panic handler, and panic_unmap() could copy the contents
> over to the real fb.
> 
> That said, this approach of scribbling over the primary plane's
> framebuffer has some clear limitations:
> * something may overwrite the oops message before the user
>   can even read it

When the dumper drm_panic_kmsg_dump() runs, the other CPU's should have
been stopped. See panic().

> * there may be other planes obscuring part or all of the
>   primary plane
> 

Yeah, this is a problem, again I wonder how Windows deals with this.

> Also scribbling over the user's framebuffer seems rather rude
> to me, so I'm thinking this approach should be limited to kernel
> panics only.
> 

Yes this will only happen on kernel panics:

panic() -> kmsg_dump() -> drm_panic_kmsg_dump()

(Unless invoking through debugfs ofc)

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel