On Wed, 05 Jun 2024 16:35, Alex Bennée <alex.ben...@linaro.org> wrote:
As the latest features for virtio-gpu need a pretty recent version of
libvirglrenderer. When it is not available on the system we can use a
meson wrapper and provide it when --download is specified in
configure.
We have to take some additional care as currently QEMU will hang
libvirglrenderer fails to exec the render server. As the error isn't
back propagated we make sure we at least test we have a path to an
executable before tweaking the environment.
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
Cc: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Cc: Akihiko Odaki <akihiko.od...@daynix.com>
---
meson.build | 7 ++++++-
hw/display/virtio-gpu-virgl.c | 24 ++++++++++++++++++++++++
subprojects/virglrenderer.wrap | 6 ++++++
3 files changed, 36 insertions(+), 1 deletion(-)
create mode 100644 subprojects/virglrenderer.wrap
diff --git a/meson.build b/meson.build
index 1d7346b703..e4e270df78 100644
--- a/meson.build
+++ b/meson.build
@@ -1203,7 +1203,8 @@ have_vhost_user_gpu = have_tools and host_os == 'linux'
and pixman.found()
if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu
virgl = dependency('virglrenderer',
method: 'pkg-config',
- required: get_option('virglrenderer'))
+ required: get_option('virglrenderer'),
+ default_options: ['default_library=static',
'render-server=true', 'venus=true'])
endif
rutabaga = not_found
if not get_option('rutabaga_gfx').auto() or have_system or have_vhost_user_gpu
@@ -2314,6 +2315,10 @@ if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB', 1)
config_host_data.set('HAVE_VIRGL_VENUS', 1)
endif
+if virgl.type_name().contains('internal')
+ config_host_data.set('HAVE_BUNDLED_VIRGL_SERVER', 1)
+endif
+
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
config_host_data.set('CONFIG_XKBCOMMON', xkbcommon.found())
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index c9d20a8a60..53d6742e79 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu/error-report.h"
#include "qemu/iov.h"
+#include "qemu/cutils.h"
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
@@ -1122,6 +1123,26 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g)
virgl_renderer_reset();
}
+/*
+ * If we fail to spawn the render server things tend to hang so it is
+ * important to do our due diligence before then. If QEMU has bundled
+ * the virgl server we want to ensure we can run it from the build
+ * directory and if installed.
+ *
+ * The principle way we can override the libvirglrenders behaviour is
+ * by setting environment variables.
+ */
+static void virgl_set_render_env(void)
+{
Since it's a few lines we could also inline this in
virtio_gpu_virgl_init()
+#ifdef HAVE_BUNDLED_VIRGL_SERVER
+ g_autofree char *file = get_relocated_path(CONFIG_QEMU_HELPERDIR
"/virgl_render_server");
+ if (g_file_test(file, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_EXECUTABLE)) {
+ g_setenv("RENDER_SERVER_EXEC_PATH", file, false);
Return value
Type: gboolean
FALSE if the environment variable couldn’t be set.
Worth adding a check here.
Offtopic, but it feels weird to set our environment without creating the
process ourselves.
There's also an option to launch the server in threads, so that if it
crashes it pulls qemu down with it. Would that work with our thread
setup?
+ }
+#endif
+}
+
+
int virtio_gpu_virgl_init(VirtIOGPU *g)
{
int ret;
@@ -1145,6 +1166,9 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
}
#endif
+ /* Ensure we can find the render server */
+ virgl_set_render_env();
+
ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
if (ret != 0) {
error_report("virgl could not be initialized: %d", ret);
diff --git a/subprojects/virglrenderer.wrap b/subprojects/virglrenderer.wrap
new file mode 100644
index 0000000000..3656a478c4
--- /dev/null
+++ b/subprojects/virglrenderer.wrap
@@ -0,0 +1,6 @@
+[wrap-git]
+url = https://gitlab.freedesktop.org/virgl/virglrenderer.git
+revision = virglrenderer-1.0.1
Can we say "at least 1.0.1" here? Should we? Dunno.
+
+[provide]
+virglrenderer = libvirglrenderer_dep
--
2.39.2
