Re: [PATCH v4 2/4] virtio-gpu: decouple Venus from CONFIG_OPENGL

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Lucas,

On Tue, Mar 17, 2026 at 02:49:13PM -0300, Lucas Amaral wrote:
> Venus (virtio-gpu Vulkan context) currently requires an OpenGL
> display backend due to build-time and runtime coupling.  On macOS,
> no OpenGL display backend exists.
> 
> Remove the opengl.found() build requirement for the virtio-gpu-gl
> module; virglrenderer provides Venus independently of OpenGL.
> 
> Gate GL-specific code paths behind CONFIG_OPENGL and display_opengl:
> - Only advertise VIRGL/VIRGL2 capsets when display_opengl is set
> - Only pass VIRGL_RENDERER_NO_VIRGL when !display_opengl with Venus
> - Null out GL context callbacks when no GL display is available
> - Route 2D display commands to the software renderer (pixman) when
>   Venus runs without GL (venus no-GL mode)
> - Allow Venus to bypass the OpenGL display check at device realize
> 
> Signed-off-by: Lucas Amaral <lucaaamaral@gmail.com>
> ---
>  hw/display/meson.build        |  8 ++--
>  hw/display/virtio-gpu-base.c  | 15 ++++++-
>  hw/display/virtio-gpu-gl.c    |  6 ++-
>  hw/display/virtio-gpu-virgl.c | 85 ++++++++++++++++++++++++++++++-----
>  4 files changed, 98 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 90e6c041..509479e7 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -76,9 +76,9 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>    virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: files('vhost-user-gpu.c'))
>    hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
>  
> -  if virgl.found() and opengl.found()
> +  if virgl.found()
>      virtio_gpu_gl_ss = ss.source_set()
> -    virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl, opengl],
> +    virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl],
>                           if_true: [files('virtio-gpu-gl.c', 'virtio-gpu-virgl.c'), pixman, virgl])
>      hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss}
>    endif
> @@ -99,9 +99,9 @@ if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
>                          if_true: files('vhost-user-gpu-pci.c'))
>    hw_display_modules += {'virtio-gpu-pci': virtio_gpu_pci_ss}
>  
> -  if virgl.found() and opengl.found()
> +  if virgl.found()
>      virtio_gpu_pci_gl_ss = ss.source_set()
> -    virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', virgl, opengl],
> +    virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', virgl],
>                               if_true: [files('virtio-gpu-pci-gl.c'), pixman])
>      hw_display_modules += {'virtio-gpu-pci-gl': virtio_gpu_pci_gl_ss}
>    endif
> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> index cb76302e..cc9704cd 100644
> --- a/hw/display/virtio-gpu-base.c
> +++ b/hw/display/virtio-gpu-base.c
> @@ -18,6 +18,7 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "hw/display/edid.h"
> +#include "system/system.h"
>  #include "trace.h"
>  #include "qapi/qapi-types-virtio.h"
>  
> @@ -157,7 +158,18 @@ virtio_gpu_get_flags(void *opaque)
>      VirtIOGPUBase *g = opaque;
>      int flags = GRAPHIC_FLAGS_NONE;
>  
> -    if (virtio_gpu_virgl_enabled(g->conf)) {
> +    if (virtio_gpu_venus_enabled(g->conf)) {
> +        /* TODO: set GRAPHIC_FLAGS_VK for direct Vulkan scanout */
> +    }
> +
> +    /*
> +     * TODO: virtio_gpu_virgl_enabled() checks VIRTIO_GPU_FLAG_VIRGL_ENABLED
> +     * which is set for both OpenGL (VIRGL) and Vulkan (Venus) backends.
> +     * This condition should ideally use a dedicated OpenGL-only flag. For
> +     * now, display_opengl correctly gates GL scanout since Venus leaves it
> +     * at 0.
> +     */
> +    if (virtio_gpu_virgl_enabled(g->conf) && display_opengl) {
>          flags |= GRAPHIC_FLAGS_GL;
>      }
>  
> @@ -273,6 +285,7 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
>      }
>      if (virtio_gpu_blob_enabled(g->conf)) {
>          features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
> +        features |= (1 << VIRTIO_GPU_F_BLOB_ALIGNMENT);

Thanks for the patch.

You are using VIRTIO_GPU_F_BLOB_ALIGNMENT here in patch2, but the macro
isn't actually defined until Patch 3. This breaks git bisect because
the project will fail to compile if someone checks out this specific
commit.

Here is the build error it causes:

../hw/display/virtio-gpu-base.c: In function ‘virtio_gpu_base_get_features’:
../hw/display/virtio-gpu-base.c:288:27: error: ‘VIRTIO_GPU_F_BLOB_ALIGNMENT’ undeclared (first use in this function); did you mean ‘VIRTIO_GPU_BLOB_MEM_GUEST’?
  288 |         features |= (1 << VIRTIO_GPU_F_BLOB_ALIGNMENT);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                           VIRTIO_GPU_BLOB_MEM_GUEST

Regards,
Kuan-Wei

>      }
>      if (virtio_gpu_context_init_enabled(g->conf)) {
>          features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index 2b7a41c4..1a95e6a7 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -124,7 +124,9 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
>          return;
>      }
>  
> -    if (!display_opengl) {
> +    if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
> +        /* Venus uses Vulkan in the render server, no GL needed */
> +    } else if (!display_opengl) {
>          error_setg(errp,
>                     "The display backend does not have OpenGL support enabled");
>          error_append_hint(errp,
> @@ -245,4 +247,6 @@ static void virtio_register_types(void)
>  type_init(virtio_register_types)
>  
>  module_dep("hw-display-virtio-gpu");
> +#ifdef CONFIG_OPENGL
>  module_dep("ui-opengl");
> +#endif
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index b7a2d160..2f9700e0 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -19,8 +19,10 @@
>  #include "hw/virtio/virtio-gpu.h"
>  #include "hw/virtio/virtio-gpu-bswap.h"
>  #include "hw/virtio/virtio-gpu-pixman.h"
> -
> +#include "system/system.h"
> +#ifdef CONFIG_OPENGL
>  #include "ui/egl-helpers.h"
> +#endif
>  
>  #include <virglrenderer.h>
>  
> @@ -50,6 +52,16 @@ struct virtio_gpu_virgl_resource {
>      void *map_fixed;
>  };
>  
> +/*
> + * Venus no-GL mode: Venus is enabled but no OpenGL display is available.
> + * Display commands use software (pixman) rendering; Venus/3D commands
> + * go through the virglrenderer render server.
> + */
> +static bool virtio_gpu_venus_nogl(VirtIOGPU *g)
> +{
> +    return virtio_gpu_venus_enabled(g->parent_obj.conf) && !display_opengl;
> +}
> +
>  static struct virtio_gpu_virgl_resource *
>  virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
>  {
> @@ -63,7 +75,7 @@ virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
>      return container_of(res, struct virtio_gpu_virgl_resource, base);
>  }
>  
> -#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
> +#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4 && defined(CONFIG_OPENGL)
>  static void *
>  virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
>  {
> @@ -1032,6 +1044,45 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>  
>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>  
> +    /*
> +     * Venus no-GL mode: route 2D display commands to the base software
> +     * renderer (pixman). The guest kernel always uses 2D commands for
> +     * display framebuffers even with VIRGL enabled; Venus/3D commands
> +     * go through the virglrenderer render server as usual.
> +     */
> +    if (virtio_gpu_venus_nogl(g)) {
> +        switch (cmd->cmd_hdr.type) {
> +        case VIRTIO_GPU_CMD_RESOURCE_CREATE_2D:
> +        case VIRTIO_GPU_CMD_SET_SCANOUT:
> +        case VIRTIO_GPU_CMD_RESOURCE_FLUSH:
> +        case VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D:
> +        case VIRTIO_GPU_CMD_GET_DISPLAY_INFO:
> +        case VIRTIO_GPU_CMD_GET_EDID:
> +            virtio_gpu_simple_process_cmd(g, cmd);
> +            return;
> +        case VIRTIO_GPU_CMD_RESOURCE_UNREF:
> +        case VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING:
> +        case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING: {
> +            /*
> +             * Both 2D (simple) and blob (virgl) resources share g->reslist.
> +             * Check if virglrenderer owns the resource to pick
> +             * the right handler.
> +             */
> +            struct virtio_gpu_resource_unref hdr;
> +            struct virgl_renderer_resource_info info;
> +            VIRTIO_GPU_FILL_CMD(hdr);
> +            if (virgl_renderer_resource_get_info(hdr.resource_id, &info)) {
> +                /* Not in virglrenderer — it's a 2D software resource */
> +                virtio_gpu_simple_process_cmd(g, cmd);
> +                return;
> +            }
> +            break; /* virglrenderer owns it — fall through */
> +        }
> +        default:
> +            break;
> +        }
> +    }
> +
>      virgl_renderer_force_ctx_0();
>      switch (cmd->cmd_hdr.type) {
>      case VIRTIO_GPU_CMD_CTX_CREATE:
> @@ -1433,6 +1484,7 @@ static int virtio_gpu_virgl_init(VirtIOGPU *g)
>      uint32_t flags = 0;
>      VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
>  
> +#ifdef CONFIG_OPENGL
>  #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
>      if (qemu_egl_display) {
>          virtio_gpu_3d_cbs.version = 4;
> @@ -1450,9 +1502,14 @@ static int virtio_gpu_virgl_init(VirtIOGPU *g)
>          flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
>      }
>  #endif
> +#endif /* CONFIG_OPENGL */
>  #if VIRGL_VERSION_MAJOR >= 1
>      if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
> -        flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
> +        flags |= VIRGL_RENDERER_VENUS
> +               | VIRGL_RENDERER_RENDER_SERVER;
> +        if (!display_opengl) {
> +            flags |= VIRGL_RENDERER_NO_VIRGL;
> +        }
>      }
>      if (virtio_gpu_drm_enabled(g->parent_obj.conf)) {
>          flags |= VIRGL_RENDERER_DRM;
> @@ -1475,6 +1532,12 @@ static int virtio_gpu_virgl_init(VirtIOGPU *g)
>      }
>  #endif
>  
> +    if (!display_opengl) {
> +        virtio_gpu_3d_cbs.create_gl_context = NULL;
> +        virtio_gpu_3d_cbs.destroy_gl_context = NULL;
> +        virtio_gpu_3d_cbs.make_current = NULL;
> +    }
> +
>      ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
>      if (ret != 0) {
>          error_report("virgl could not be initialized: %d", ret);
> @@ -1546,14 +1609,16 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
>  
>      capset_ids = g_array_new(false, false, sizeof(uint32_t));
>  
> -    /* VIRGL is always supported. */
> -    virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
> +    /* OpenGL: VIRGL/VIRGL2 require a GL display backend */
> +    if (display_opengl) {
> +        virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
>  
> -    virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
> -                               &capset_max_ver,
> -                               &capset_max_size);
> -    if (capset_max_ver) {
> -        virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
> +        virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
> +                                   &capset_max_ver,
> +                                   &capset_max_size);
> +        if (capset_max_ver) {
> +            virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
> +        }
>      }
>  
>      if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
> -- 
> 2.52.0
> 
> 
>