[PATCH xserver] glamor: Fix crash when master gpu is using glamor and another gpu is hotplugged

[Eric Anholt]

Hans de Goede <hdegoede at redhat.com> writes:

> Hi,
>
> On 06-09-16 22:22, Eric Anholt wrote:
>> Hans de Goede <hdegoede at redhat.com> writes:
>>
>>> When a GPU gets hotplugged while X is already running, glamor_egl_init()
>>> gets called and changes the current egl context at a point where glamor
>>> does not expect this.
>>>
>>> This causes glamor to e.g. crash in the next glamor_create_pixmap() call
>>> (caled through the master's screen->CreatePixmap), note this is not the
>>> only troublesome entry point I've seen other backtraces when using a
>>> composting window manager.
>>>
>>> Adding glamor_make_current calls to all entry points is quite expensive,
>>> so this commit consists of a miminal fix for this problem by restoring the
>>> original egl context when leaving glamor_egl_init() with an error, based
>>> on only usb gpu's getting hotplugged and they do not support glamor.
>>
>> I think the problem is just mismatching our lastGLContext with the
>> actual makecurrent state.  Couldn't we just use glamor_make_current()
>> instead of our own eglMakeCurrent() call in this function?
>
> We cannot use glamor_make_current() in the problem scenario I'm
> trying to fix, because we do not need to set the egl context
> from the current screen, we need to not mess up the egl context
> which was current *before* glamor_egl_init() gets called for
> a different screen then the one which owns the current egl context.
>
> What happens is:
>
> 1.current egl context is the master GPUs / Screens egl context
> 2. USB GPU gets hotplugged
> 3. modesetting driver's PreInit() gets called on the
>     USB-GPU's Screen
> 4. PreInit() calls glamor_egl_init() which creates a new
>     egl context for the USB-GPU Screen, makes that current,
>     then fails and sets the current egl context to NONE
> 6. master Screen->CreatePixmap gets called, this is
>     glamor_make_pixmap, this fails (in an assert -> boom)
>     because the current egl context is NONE

There's this callchain:

glamor_make_pixmap() ->
glamor_create_fbo() ->
glamor_create_tex() ->
glamor_make_current()

And every entrypoint should be doing that before making any other GL
calls (if there are any missing, they must be fixed).  The problem is
that glamor_egl_init() is making a GL context current without updating
lastGLcontext, so glamor_make_current() short-circuits.  Calling
glamor_make_current() from glamor_egl_init() was my proposal for keeping
lastGLcontext updated.

If we can't use that for some reason, we should just NULL out
lastGLcontext at our custom MakeCurrent time so that it's never out of
sync with reality.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160907/37acb8ec/attachment.sig>