[PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[Vivek Kasireddy]

Since not all listeners (i.e VirtualConsoles) of GL events have
a valid EGL context, make sure that there is a valid context
before making EGL calls.

This fixes the following crash seen while launching the VM with
"-device virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on"

No provider of eglCreateImageKHR found.  Requires one of:
EGL_KHR_image
EGL_KHR_image_base

Fixes: 7cc712e9862ff ("ui: dispatch GL events to all listeners")

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 ui/gtk-egl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index e3bd4bc274..31175827d0 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -244,6 +244,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    if (!vc->gfx.ectx || !vc->gfx.esurface) {
+        return;
+    }
+
     eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
                    vc->gfx.esurface, vc->gfx.ectx);
 
@@ -269,6 +273,10 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
 #ifdef CONFIG_GBM
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    if (!vc->gfx.ectx || !vc->gfx.esurface) {
+        return;
+    }
+
     if (dmabuf) {
         egl_dmabuf_import_texture(dmabuf);
         if (!dmabuf->texture) {
-- 
2.35.1

Re: [PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[Marc-André Lureau]

Hi Vivek

On Mon, Mar 7, 2022 at 8:39 AM Vivek Kasireddy
<vivek.kasireddy@intel.com> wrote:
>
> Since not all listeners (i.e VirtualConsoles) of GL events have
> a valid EGL context, make sure that there is a valid context
> before making EGL calls.
>
> This fixes the following crash seen while launching the VM with
> "-device virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on"
>
> No provider of eglCreateImageKHR found.  Requires one of:
> EGL_KHR_image
> EGL_KHR_image_base
>
> Fixes: 7cc712e9862ff ("ui: dispatch GL events to all listeners")

I am not able to reproduce on current master.

Isn't it fixed with commit a9fbce5e9 ("ui/console: fix crash when
using gl context with non-gl listeners") ?

Could you also check after "[PATCH v3 00/12] GL & D-Bus display related fixes" ?

thanks

>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  ui/gtk-egl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> index e3bd4bc274..31175827d0 100644
> --- a/ui/gtk-egl.c
> +++ b/ui/gtk-egl.c
> @@ -244,6 +244,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
>  #ifdef CONFIG_GBM
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>
> +    if (!vc->gfx.ectx || !vc->gfx.esurface) {
> +        return;
> +    }
> +
>      eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
>                     vc->gfx.esurface, vc->gfx.ectx);
>
> @@ -269,6 +273,10 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
>  #ifdef CONFIG_GBM
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>
> +    if (!vc->gfx.ectx || !vc->gfx.esurface) {
> +        return;
> +    }
> +
>      if (dmabuf) {
>          egl_dmabuf_import_texture(dmabuf);
>          if (!dmabuf->texture) {
> --
> 2.35.1
>

RE: [PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[Kasireddy, Vivek]

Hi Marc-Andre,

> 
> Hi Vivek
> 
> On Mon, Mar 7, 2022 at 8:39 AM Vivek Kasireddy
> <vivek.kasireddy@intel.com> wrote:
> >
> > Since not all listeners (i.e VirtualConsoles) of GL events have
> > a valid EGL context, make sure that there is a valid context
> > before making EGL calls.
> >
> > This fixes the following crash seen while launching the VM with
> > "-device virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on"
> >
> > No provider of eglCreateImageKHR found.  Requires one of:
> > EGL_KHR_image
> > EGL_KHR_image_base
> >
> > Fixes: 7cc712e9862ff ("ui: dispatch GL events to all listeners")
> 
> I am not able to reproduce on current master.
[Kasireddy, Vivek] I can still see it with current master. I think this issue
is only seen when running Qemu in an Xorg based Host environment and
cannot be reproduced in a Wayland based environment -- as Qemu UI 
uses the GLArea widget in the Wayland case where the EGL context
is managed by GTK.

> 
> Isn't it fixed with commit a9fbce5e9 ("ui/console: fix crash when
> using gl context with non-gl listeners") ?
[Kasireddy, Vivek] No, it unfortunately does not fix the issue I am seeing. In 
my case, there are three VirtualConsoles created ("parallel0", "compatmonitor0",
"virtio-gpu-pci") and all three of them seem to have a valid dpy_gl_scanout_dmabuf()
but only virtio-gpu-pci has a valid EGL context. 

> 
> Could you also check after "[PATCH v3 00/12] GL & D-Bus display related fixes" ?
[Kasireddy, Vivek] I can check but I don't think this issue can be fixed in ui/console.c
as all three VirtualConsoles pass the console_has_gl() check and one of the only things
that distinguishes them is whether they have a valid EGL context. 

Thanks,
Vivek

> 
> thanks
> 
> >
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Dongwon Kim <dongwon.kim@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >  ui/gtk-egl.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > index e3bd4bc274..31175827d0 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -244,6 +244,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
> >  #ifdef CONFIG_GBM
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >
> > +    if (!vc->gfx.ectx || !vc->gfx.esurface) {
> > +        return;
> > +    }
> > +
> >      eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
> >                     vc->gfx.esurface, vc->gfx.ectx);
> >
> > @@ -269,6 +273,10 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
> >  #ifdef CONFIG_GBM
> >      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >
> > +    if (!vc->gfx.ectx || !vc->gfx.esurface) {
> > +        return;
> > +    }
> > +
> >      if (dmabuf) {
> >          egl_dmabuf_import_texture(dmabuf);
> >          if (!dmabuf->texture) {
> > --
> > 2.35.1
> >

Re: [PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[Marc-André Lureau]

Hi

On Mon, Mar 7, 2022 at 10:00 PM Kasireddy, Vivek
<vivek.kasireddy@intel.com> wrote:
>
> Hi Marc-Andre,
>
> >
> > Hi Vivek
> >
> > On Mon, Mar 7, 2022 at 8:39 AM Vivek Kasireddy
> > <vivek.kasireddy@intel.com> wrote:
> > >
> > > Since not all listeners (i.e VirtualConsoles) of GL events have
> > > a valid EGL context, make sure that there is a valid context
> > > before making EGL calls.
> > >
> > > This fixes the following crash seen while launching the VM with
> > > "-device virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on"
> > >
> > > No provider of eglCreateImageKHR found.  Requires one of:
> > > EGL_KHR_image
> > > EGL_KHR_image_base
> > >
> > > Fixes: 7cc712e9862ff ("ui: dispatch GL events to all listeners")
> >
> > I am not able to reproduce on current master.
> [Kasireddy, Vivek] I can still see it with current master. I think this issue
> is only seen when running Qemu in an Xorg based Host environment and
> cannot be reproduced in a Wayland based environment -- as Qemu UI
> uses the GLArea widget in the Wayland case where the EGL context
> is managed by GTK.
>
> >
> > Isn't it fixed with commit a9fbce5e9 ("ui/console: fix crash when
> > using gl context with non-gl listeners") ?
> [Kasireddy, Vivek] No, it unfortunately does not fix the issue I am seeing. In
> my case, there are three VirtualConsoles created ("parallel0", "compatmonitor0",
> "virtio-gpu-pci") and all three of them seem to have a valid dpy_gl_scanout_dmabuf()
> but only virtio-gpu-pci has a valid EGL context.
>
> >
> > Could you also check after "[PATCH v3 00/12] GL & D-Bus display related fixes" ?
> [Kasireddy, Vivek] I can check but I don't think this issue can be fixed in ui/console.c
> as all three VirtualConsoles pass the console_has_gl() check and one of the only things
> that distinguishes them is whether they have a valid EGL context.
>

Under X11, I get the same error on v6.2.0 and master:
qemu-system-x86_64  -m 4G -object
memory-backend-memfd,id=mem,size=4G,share=on -machine
q35,accel=kvm,memory-backend=mem -device
virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on -cdrom
rawhide.iso
No provider of eglCreateImageKHR found.  Requires one of:
    EGL_KHR_image
    EGL_KHR_image_base

Note that with virtio-gpu-gl-pci I get:
qemu-system-x86_64: ../src/dispatch_common.c:868:
epoxy_get_proc_address: Assertion `0 && "Couldn't find current GLX or
EGL context.\n"' failed.

RE: [PATCH v1] ui/gtk-egl: Check for a valid context before making EGL calls

[Kasireddy, Vivek]

Hi Marc-Andre,

> 
> Hi
> 
> On Mon, Mar 7, 2022 at 10:00 PM Kasireddy, Vivek
> <vivek.kasireddy@intel.com> wrote:
> >
> > Hi Marc-Andre,
> >
> > >
> > > Hi Vivek
> > >
> > > On Mon, Mar 7, 2022 at 8:39 AM Vivek Kasireddy
> > > <vivek.kasireddy@intel.com> wrote:
> > > >
> > > > Since not all listeners (i.e VirtualConsoles) of GL events have
> > > > a valid EGL context, make sure that there is a valid context
> > > > before making EGL calls.
> > > >
> > > > This fixes the following crash seen while launching the VM with
> > > > "-device virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on"
> > > >
> > > > No provider of eglCreateImageKHR found.  Requires one of:
> > > > EGL_KHR_image
> > > > EGL_KHR_image_base
> > > >
> > > > Fixes: 7cc712e9862ff ("ui: dispatch GL events to all listeners")
> > >
> > > I am not able to reproduce on current master.
> > [Kasireddy, Vivek] I can still see it with current master. I think this issue
> > is only seen when running Qemu in an Xorg based Host environment and
> > cannot be reproduced in a Wayland based environment -- as Qemu UI
> > uses the GLArea widget in the Wayland case where the EGL context
> > is managed by GTK.
> >
> > >
> > > Isn't it fixed with commit a9fbce5e9 ("ui/console: fix crash when
> > > using gl context with non-gl listeners") ?
> > [Kasireddy, Vivek] No, it unfortunately does not fix the issue I am seeing. In
> > my case, there are three VirtualConsoles created ("parallel0", "compatmonitor0",
> > "virtio-gpu-pci") and all three of them seem to have a valid dpy_gl_scanout_dmabuf()
> > but only virtio-gpu-pci has a valid EGL context.
> >
> > >
> > > Could you also check after "[PATCH v3 00/12] GL & D-Bus display related fixes" ?
> > [Kasireddy, Vivek] I can check but I don't think this issue can be fixed in ui/console.c
> > as all three VirtualConsoles pass the console_has_gl() check and one of the only things
> > that distinguishes them is whether they have a valid EGL context.
> >
> 
> Under X11, I get the same error on v6.2.0 and master:
> qemu-system-x86_64  -m 4G -object
> memory-backend-memfd,id=mem,size=4G,share=on -machine
> q35,accel=kvm,memory-backend=mem -device
> virtio-gpu-pci,max_outputs=1,blob=true -display gtk,gl=on -cdrom
> rawhide.iso
> No provider of eglCreateImageKHR found.  Requires one of:
>     EGL_KHR_image
>     EGL_KHR_image_base
> 
> Note that with virtio-gpu-gl-pci I get:
> qemu-system-x86_64: ../src/dispatch_common.c:868:
> epoxy_get_proc_address: Assertion `0 && "Couldn't find current GLX or
> EGL context.\n"' failed.
[Kasireddy, Vivek] It looks like this particular error and the one I saw are
both resolved by this commit:
Author: Akihiko Odaki <akihiko.odaki@gmail.com>
Date:   Sat Mar 26 01:12:16 2022 +0900

    ui/console: Check console before emitting GL event

On a completely different note, I am wondering if you have any plan to
eventually integrate the Rust based Gtk4 client into Qemu source repo?
Or, is it going to stay out-of-tree even after it is no longer WIP?

Thanks,
Vivek