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

[Hans de Goede]

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

My patch inserts a:

5. Restore egl context from before glamor_egl_init() got
    called in the error exit path of glamor_egl_init()
    making step 6. work as it should.

Note that we're in glamor_egl_init for the USB-GPU
Screen when doing this, so we do not have access
to the glamor-private data of the master Screen's
glamor private-data and thus cannot use eglMakeCurrent()

I hope this helps explains what I'm trying to do.

Note I'm not claiming my fix is pretty in anyway
and I'm open to suggestions for a better fix, but
not pretty as my fix may be it does fix the crash :)

Regards,

Hans