From b6de568e24491f24f87fd45193e0b0aafcfe122d Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Fri, 26 Jun 2026 15:14:04 -0300 Subject: [PATCH 1/2] virgl: capture host-rendered virtio-gpu scanouts via GPU EGL readback A virgl (virtio-gpu 3D) scanout is rendered on the host GPU. The KMS framebuffer handle the guest gets from GetFB2 points at a host-side resource, so any guest CPU read of it (dumb-map V2 or a CPU mmap of the exported DMA-BUF) comes back black, and TRANSFER_FROM_HOST does not populate a guest-readable buffer for it. The pixels are reachable on the guest GPU, though: importing the DMA-BUF as an EGLImage and glReadPixels() resolves the real host-rendered content. Helper: when virtio 3D features are present, export the scanout as a DMA-BUF (V3, as for plain virtio) and additionally tag the frame FLAG_VIRGL. The export condition is simplified to 'any virtio or a tiled non-virtio framebuffer'; plain virtio keeps the existing zero-copy direct-map path (no FLAG_VIRGL). Library: thread a force_egl flag into gpu_auto_process(). On FLAG_VIRGL the linear-modifier fast path is bypassed and the frame is run through the EGL import + glReadPixels path (the same backend used to detile tiled buffers), which reads the host-rendered pixels. Plain virtio and all other paths pass force_egl=0 and are unchanged. Verified end to end in a virgl VM (qemu virtio-gpu-gl + egl-headless, weston --renderer=gl -> 'GL renderer: virgl'): a non-root capture through the helper now returns the live weston desktop (helper logs 'DMA-BUF + GPU readback (virgl)', library logs EGL convert ret=0) instead of a black frame. Re-checked plain virtio-gpu (weston --renderer=pixman): still the zero-copy direct-map path (helper logs 'zero-copy DMA-BUF', no EGL convert), no regression. Closes #15 --- bindings/rust/libdrmtap-sys/csrc/drm_grab.c | 30 +++++++----- .../rust/libdrmtap-sys/csrc/drmtap-helper.c | 47 ++++++++++++------- .../rust/libdrmtap-sys/csrc/drmtap_internal.h | 3 ++ helper/drmtap-helper.c | 47 ++++++++++++------- src/drm_grab.c | 30 +++++++----- src/drmtap_internal.h | 3 ++ 6 files changed, 104 insertions(+), 56 deletions(-) diff --git a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c index af4f2f4..da13ffb 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c +++ b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c @@ -59,7 +59,7 @@ /* Forward declaration */ static int gpu_auto_process(drmtap_ctx *ctx, void *data, - drmtap_frame_info *frame); + drmtap_frame_info *frame, int force_egl); /* ========================================================================= */ /* Internal state for a captured frame (stored in frame->_priv) */ @@ -449,6 +449,9 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { * calloc'd (mapped=MAP_FAILED, prime_fd=-1), so there is no prior * mapping to release here. */ int dmabuf_fd = hresult.dmabuf_fd; + /* virgl scanout: the helper exported the fd but a CPU mmap of it is + * black (host-side resource), so force the GPU EGL readback path. */ + int is_virgl = (hresult.wire.flags & HELPER_FLAG_VIRGL) != 0; size_t mmap_size = (size_t)frame->stride * frame->height; /* mmap the DMA-BUF in the parent */ @@ -478,7 +481,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { dmabuf_fd, mmap_size); if (do_mmap) { - gpu_auto_process(ctx, mapped, frame); + gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -493,7 +496,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { "for EGL zero-copy", dmabuf_fd); if (do_mmap) { - gpu_auto_process(ctx, NULL, frame); + gpu_auto_process(ctx, NULL, frame, is_virgl); } } @@ -522,7 +525,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { frame->_priv = priv; if (do_mmap) { - gpu_auto_process(ctx, pixel_buf, frame); + gpu_auto_process(ctx, pixel_buf, frame, 0); } drmModeFreeFB2(fb2); @@ -599,7 +602,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { drmtap_debug_log(ctx, "mapped %zu bytes at %p", size, mapped); /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, mapped, frame); + gpu_auto_process(ctx, mapped, frame, 0); } } @@ -622,13 +625,18 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { /* ========================================================================= */ static int gpu_auto_process(drmtap_ctx *ctx, void *data, - drmtap_frame_info *frame) { - if (!data) return 0; + drmtap_frame_info *frame, int force_egl) { + /* force_egl is set for a virgl scanout: the DMA-BUF holds host-rendered + * pixels that a CPU mmap reads back black, so we must go down the EGL path + * (which imports the fd on the GPU) even though there is no usable `data` + * and the modifier is linear. */ + if (!data && !force_egl) return 0; uint64_t modifier = frame->modifier; - /* Linear framebuffer: no deswizzle needed */ - if (modifier == DRM_FORMAT_MOD_LINEAR || modifier == 0) { + /* Linear framebuffer: no deswizzle needed (unless a virgl readback is + * forced — see force_egl above). */ + if ((modifier == DRM_FORMAT_MOD_LINEAR || modifier == 0) && !force_egl) { return 0; } @@ -963,7 +971,7 @@ int drmtap_grab_mapped_fast(drmtap_ctx *ctx, drmtap_frame_info *frame) { frame->_priv = NULL; /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, frame->data, frame); + gpu_auto_process(ctx, frame->data, frame, 0); return 0; /* new frame */ } @@ -1058,7 +1066,7 @@ int drmtap_grab_mapped_fast(drmtap_ctx *ctx, drmtap_frame_info *frame) { frame->_priv = NULL; /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, frame->data, frame); + gpu_auto_process(ctx, frame->data, frame, 0); return 0; /* new frame */ } diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c index 7665bff..9376be1 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c @@ -126,6 +126,10 @@ struct helper_cmd_grab { /* Metadata sent before pixel data — must match helper_grab_result_t in library */ #define FLAG_HAS_DMABUF 0x01 +/* The exported DMA-BUF is a host-rendered virgl scanout: the parent must read it + * back on the GPU (EGL import + glReadPixels), not via a CPU mmap, which comes + * back black for a host-side resource. */ +#define FLAG_VIRGL 0x02 struct grab_metadata { uint32_t width; @@ -522,27 +526,33 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt #endif } - /* Decide once whether this virtio-gpu is "plain" (no virgl 3D). Without 3D - * the scanout is a 2D dumb buffer in guest RAM that we can export as a - * DMA-BUF and let the parent map directly (zero-copy), instead of copying - * the whole framebuffer over the socket every frame. With virgl the scanout - * can be a host-side 3D resource, so we keep the proven copy-after-transfer - * path there until the zero-copy path is validated on virgl. */ - static int virtio_plain = -1; - if (is_virtio && virtio_plain < 0) { - virtio_plain = 0; + /* Decide once how to serve this virtio-gpu scanout. Two sub-cases: + * - plain (no virgl 3D): the scanout is a 2D dumb buffer in guest RAM, so + * we export it as a DMA-BUF and let the parent map it directly + * (zero-copy), instead of copying the whole framebuffer every frame. + * - virgl (3D features present): the scanout can be a host-side 3D + * resource that a guest CPU mmap reads back black. We still export the + * DMA-BUF, but tag it FLAG_VIRGL so the parent reads it back on the GPU + * (EGL import + glReadPixels), which sees the real host-rendered pixels. + * virtio_plain stays 0 if detection is unavailable; that just means "treat + * like plain" (export + direct map), which is correct for 2D virtio. */ + static int virtio_detected = 0; /* 1 once we've probed for 3D features */ + static int virtio_virgl = 0; /* 1 = confirmed virgl (3D features present) */ + if (is_virtio && !virtio_detected) { + virtio_detected = 1; #if defined(__linux__) && defined(DRM_IOCTL_VIRTGPU_GETPARAM) uint64_t features = 0; struct drm_virtgpu_getparam gp = { .param = VIRTGPU_PARAM_3D_FEATURES, .value = (uint64_t)(uintptr_t)&features, }; - if (drmIoctl(drm_fd, DRM_IOCTL_VIRTGPU_GETPARAM, &gp) == 0 && features == 0) { - virtio_plain = 1; + if (drmIoctl(drm_fd, DRM_IOCTL_VIRTGPU_GETPARAM, &gp) == 0 && features != 0) { + virtio_virgl = 1; } fprintf(stderr, "drmtap-helper: virtio 3D features=%llu -> %s path\n", (unsigned long long)features, - virtio_plain ? "zero-copy DMA-BUF" : "pixel-copy"); + virtio_virgl ? "DMA-BUF + GPU readback (virgl)" + : "zero-copy DMA-BUF"); #endif } @@ -576,17 +586,20 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt * buffers on some drivers are not dumb-mappable (nvidia-drm on Tegra * fails MODE_MAP_DUMB with "Failed to lookup gem object"), so requiring a * dumb map first would break capture there even though the export works. - * - plain virtio-gpu (guest-rendered, no host transfer): a linear scanout - * in guest RAM the parent maps directly (no detile). Replaces the V2 - * pixel copy, dropping the helper's per-frame CPU cost. + * - virtio-gpu (plain or virgl): a linear scanout the parent either maps + * directly (plain, guest RAM) or reads back on the GPU (virgl, tagged + * FLAG_VIRGL — a CPU mmap of a host-side resource is black). Replaces the + * V2 pixel copy, dropping the helper's per-frame CPU cost. * Either way, if the export fails we fall through to the dumb-map pixel path. */ - if ((meta.modifier != 0 /* DRM_FORMAT_MOD_LINEAR */ && !is_virtio) - || (is_virtio && virtio_plain)) { + if (meta.modifier != 0 /* DRM_FORMAT_MOD_LINEAR */ || is_virtio) { int prime_fd = -1; int prime_ret = drmPrimeHandleToFD(drm_fd, gem_handle, DRM_CLOEXEC | DRM_RDWR, &prime_fd); if (prime_ret == 0 && prime_fd >= 0) { meta.flags = FLAG_HAS_DMABUF; + if (virtio_virgl) { + meta.flags |= FLAG_VIRGL; /* parent must GPU-read it back */ + } meta.data_size = 0; /* no pixel data follows */ ret = send_fd(sock, prime_fd, &meta, sizeof(meta)); close(prime_fd); diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap_internal.h b/bindings/rust/libdrmtap-sys/csrc/drmtap_internal.h index 31b44e1..77d835b 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap_internal.h +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap_internal.h @@ -119,6 +119,9 @@ typedef struct { * Must match struct grab_metadata in drmtap-helper.c */ /* Flags for helper_grab_result_t.flags */ #define HELPER_FLAG_HAS_DMABUF 0x01 /* DMA-BUF fd follows via SCM_RIGHTS */ +#define HELPER_FLAG_VIRGL 0x02 /* DMA-BUF is a host-rendered virgl scanout: + * read it back on the GPU (EGL), not via a + * CPU mmap (which is black for it) */ typedef struct { uint32_t width; diff --git a/helper/drmtap-helper.c b/helper/drmtap-helper.c index 7665bff..9376be1 100644 --- a/helper/drmtap-helper.c +++ b/helper/drmtap-helper.c @@ -126,6 +126,10 @@ struct helper_cmd_grab { /* Metadata sent before pixel data — must match helper_grab_result_t in library */ #define FLAG_HAS_DMABUF 0x01 +/* The exported DMA-BUF is a host-rendered virgl scanout: the parent must read it + * back on the GPU (EGL import + glReadPixels), not via a CPU mmap, which comes + * back black for a host-side resource. */ +#define FLAG_VIRGL 0x02 struct grab_metadata { uint32_t width; @@ -522,27 +526,33 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt #endif } - /* Decide once whether this virtio-gpu is "plain" (no virgl 3D). Without 3D - * the scanout is a 2D dumb buffer in guest RAM that we can export as a - * DMA-BUF and let the parent map directly (zero-copy), instead of copying - * the whole framebuffer over the socket every frame. With virgl the scanout - * can be a host-side 3D resource, so we keep the proven copy-after-transfer - * path there until the zero-copy path is validated on virgl. */ - static int virtio_plain = -1; - if (is_virtio && virtio_plain < 0) { - virtio_plain = 0; + /* Decide once how to serve this virtio-gpu scanout. Two sub-cases: + * - plain (no virgl 3D): the scanout is a 2D dumb buffer in guest RAM, so + * we export it as a DMA-BUF and let the parent map it directly + * (zero-copy), instead of copying the whole framebuffer every frame. + * - virgl (3D features present): the scanout can be a host-side 3D + * resource that a guest CPU mmap reads back black. We still export the + * DMA-BUF, but tag it FLAG_VIRGL so the parent reads it back on the GPU + * (EGL import + glReadPixels), which sees the real host-rendered pixels. + * virtio_plain stays 0 if detection is unavailable; that just means "treat + * like plain" (export + direct map), which is correct for 2D virtio. */ + static int virtio_detected = 0; /* 1 once we've probed for 3D features */ + static int virtio_virgl = 0; /* 1 = confirmed virgl (3D features present) */ + if (is_virtio && !virtio_detected) { + virtio_detected = 1; #if defined(__linux__) && defined(DRM_IOCTL_VIRTGPU_GETPARAM) uint64_t features = 0; struct drm_virtgpu_getparam gp = { .param = VIRTGPU_PARAM_3D_FEATURES, .value = (uint64_t)(uintptr_t)&features, }; - if (drmIoctl(drm_fd, DRM_IOCTL_VIRTGPU_GETPARAM, &gp) == 0 && features == 0) { - virtio_plain = 1; + if (drmIoctl(drm_fd, DRM_IOCTL_VIRTGPU_GETPARAM, &gp) == 0 && features != 0) { + virtio_virgl = 1; } fprintf(stderr, "drmtap-helper: virtio 3D features=%llu -> %s path\n", (unsigned long long)features, - virtio_plain ? "zero-copy DMA-BUF" : "pixel-copy"); + virtio_virgl ? "DMA-BUF + GPU readback (virgl)" + : "zero-copy DMA-BUF"); #endif } @@ -576,17 +586,20 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt * buffers on some drivers are not dumb-mappable (nvidia-drm on Tegra * fails MODE_MAP_DUMB with "Failed to lookup gem object"), so requiring a * dumb map first would break capture there even though the export works. - * - plain virtio-gpu (guest-rendered, no host transfer): a linear scanout - * in guest RAM the parent maps directly (no detile). Replaces the V2 - * pixel copy, dropping the helper's per-frame CPU cost. + * - virtio-gpu (plain or virgl): a linear scanout the parent either maps + * directly (plain, guest RAM) or reads back on the GPU (virgl, tagged + * FLAG_VIRGL — a CPU mmap of a host-side resource is black). Replaces the + * V2 pixel copy, dropping the helper's per-frame CPU cost. * Either way, if the export fails we fall through to the dumb-map pixel path. */ - if ((meta.modifier != 0 /* DRM_FORMAT_MOD_LINEAR */ && !is_virtio) - || (is_virtio && virtio_plain)) { + if (meta.modifier != 0 /* DRM_FORMAT_MOD_LINEAR */ || is_virtio) { int prime_fd = -1; int prime_ret = drmPrimeHandleToFD(drm_fd, gem_handle, DRM_CLOEXEC | DRM_RDWR, &prime_fd); if (prime_ret == 0 && prime_fd >= 0) { meta.flags = FLAG_HAS_DMABUF; + if (virtio_virgl) { + meta.flags |= FLAG_VIRGL; /* parent must GPU-read it back */ + } meta.data_size = 0; /* no pixel data follows */ ret = send_fd(sock, prime_fd, &meta, sizeof(meta)); close(prime_fd); diff --git a/src/drm_grab.c b/src/drm_grab.c index af4f2f4..da13ffb 100644 --- a/src/drm_grab.c +++ b/src/drm_grab.c @@ -59,7 +59,7 @@ /* Forward declaration */ static int gpu_auto_process(drmtap_ctx *ctx, void *data, - drmtap_frame_info *frame); + drmtap_frame_info *frame, int force_egl); /* ========================================================================= */ /* Internal state for a captured frame (stored in frame->_priv) */ @@ -449,6 +449,9 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { * calloc'd (mapped=MAP_FAILED, prime_fd=-1), so there is no prior * mapping to release here. */ int dmabuf_fd = hresult.dmabuf_fd; + /* virgl scanout: the helper exported the fd but a CPU mmap of it is + * black (host-side resource), so force the GPU EGL readback path. */ + int is_virgl = (hresult.wire.flags & HELPER_FLAG_VIRGL) != 0; size_t mmap_size = (size_t)frame->stride * frame->height; /* mmap the DMA-BUF in the parent */ @@ -478,7 +481,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { dmabuf_fd, mmap_size); if (do_mmap) { - gpu_auto_process(ctx, mapped, frame); + gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -493,7 +496,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { "for EGL zero-copy", dmabuf_fd); if (do_mmap) { - gpu_auto_process(ctx, NULL, frame); + gpu_auto_process(ctx, NULL, frame, is_virgl); } } @@ -522,7 +525,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { frame->_priv = priv; if (do_mmap) { - gpu_auto_process(ctx, pixel_buf, frame); + gpu_auto_process(ctx, pixel_buf, frame, 0); } drmModeFreeFB2(fb2); @@ -599,7 +602,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { drmtap_debug_log(ctx, "mapped %zu bytes at %p", size, mapped); /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, mapped, frame); + gpu_auto_process(ctx, mapped, frame, 0); } } @@ -622,13 +625,18 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { /* ========================================================================= */ static int gpu_auto_process(drmtap_ctx *ctx, void *data, - drmtap_frame_info *frame) { - if (!data) return 0; + drmtap_frame_info *frame, int force_egl) { + /* force_egl is set for a virgl scanout: the DMA-BUF holds host-rendered + * pixels that a CPU mmap reads back black, so we must go down the EGL path + * (which imports the fd on the GPU) even though there is no usable `data` + * and the modifier is linear. */ + if (!data && !force_egl) return 0; uint64_t modifier = frame->modifier; - /* Linear framebuffer: no deswizzle needed */ - if (modifier == DRM_FORMAT_MOD_LINEAR || modifier == 0) { + /* Linear framebuffer: no deswizzle needed (unless a virgl readback is + * forced — see force_egl above). */ + if ((modifier == DRM_FORMAT_MOD_LINEAR || modifier == 0) && !force_egl) { return 0; } @@ -963,7 +971,7 @@ int drmtap_grab_mapped_fast(drmtap_ctx *ctx, drmtap_frame_info *frame) { frame->_priv = NULL; /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, frame->data, frame); + gpu_auto_process(ctx, frame->data, frame, 0); return 0; /* new frame */ } @@ -1058,7 +1066,7 @@ int drmtap_grab_mapped_fast(drmtap_ctx *ctx, drmtap_frame_info *frame) { frame->_priv = NULL; /* Auto-deswizzle tiled framebuffers + format convert */ - gpu_auto_process(ctx, frame->data, frame); + gpu_auto_process(ctx, frame->data, frame, 0); return 0; /* new frame */ } diff --git a/src/drmtap_internal.h b/src/drmtap_internal.h index 31b44e1..77d835b 100644 --- a/src/drmtap_internal.h +++ b/src/drmtap_internal.h @@ -119,6 +119,9 @@ typedef struct { * Must match struct grab_metadata in drmtap-helper.c */ /* Flags for helper_grab_result_t.flags */ #define HELPER_FLAG_HAS_DMABUF 0x01 /* DMA-BUF fd follows via SCM_RIGHTS */ +#define HELPER_FLAG_VIRGL 0x02 /* DMA-BUF is a host-rendered virgl scanout: + * read it back on the GPU (EGL), not via a + * CPU mmap (which is black for it) */ typedef struct { uint32_t width; From 408a723504dcf72533f7c98ccb93c2c1e092cc90 Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Fri, 26 Jun 2026 15:23:57 -0300 Subject: [PATCH 2/2] virgl: fail closed when GPU readback is unavailable Addresses review feedback. A virgl scanout has no CPU-readable pixels, so on every path where the GPU readback cannot happen we now return an error instead of emitting an all-black frame as success: - Helper: if drmPrimeHandleToFD fails for a confirmed-virgl frame, send_error instead of falling through to the dumb-map pixel path (which is black). - Library: gpu_auto_process() returns -ENOTSUP when force_egl is set but the EGL path did not produce pixels (EGL unavailable or convert failed), and the helper-V3 call sites propagate that error for virgl frames (non-virgl frames keep the prior best-effort behaviour). Also fixes a stale comment that still referenced the removed virtio_plain variable. Happy path is unchanged: a successful EGL readback returns before these checks, as verified in the virgl VM. --- bindings/rust/libdrmtap-sys/csrc/drm_grab.c | 22 +++++++++++++++++-- .../rust/libdrmtap-sys/csrc/drmtap-helper.c | 11 ++++++++-- helper/drmtap-helper.c | 11 ++++++++-- src/drm_grab.c | 22 +++++++++++++++++-- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c index da13ffb..c8420ac 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c +++ b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c @@ -452,6 +452,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { /* virgl scanout: the helper exported the fd but a CPU mmap of it is * black (host-side resource), so force the GPU EGL readback path. */ int is_virgl = (hresult.wire.flags & HELPER_FLAG_VIRGL) != 0; + int pret = 0; /* gpu_auto_process result (propagated for virgl) */ size_t mmap_size = (size_t)frame->stride * frame->height; /* mmap the DMA-BUF in the parent */ @@ -481,7 +482,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { dmabuf_fd, mmap_size); if (do_mmap) { - gpu_auto_process(ctx, mapped, frame, is_virgl); + pret = gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -496,11 +497,18 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { "for EGL zero-copy", dmabuf_fd); if (do_mmap) { - gpu_auto_process(ctx, NULL, frame, is_virgl); + pret = gpu_auto_process(ctx, NULL, frame, is_virgl); } } drmModeFreeFB2(fb2); + /* For a virgl frame the GPU readback is the only way to get real + * pixels; if it failed, propagate the error rather than handing the + * caller a black frame. Non-virgl frames keep the prior best-effort + * behaviour (pret is ignored). */ + if (is_virgl && pret != 0) { + return pret; + } return 0; } @@ -716,6 +724,16 @@ static int gpu_auto_process(drmtap_ctx *ctx, void *data, } #endif + /* A virgl readback was forced but the EGL path did not produce pixels (EGL + * unavailable, or the import/readback failed). The only CPU-visible copy of + * a host-rendered scanout is black, so fail closed instead of returning a + * bogus frame as success. */ + if (force_egl) { + drmtap_set_error(ctx, + "virgl scanout needs GPU EGL readback, which is unavailable or failed"); + return -ENOTSUP; + } + /* --- CPU deswizzle for classic tiling (buffer already allocated above) --- */ const char *driver = ctx->driver_name; if (drmtap_gpu_intel_match(driver) || diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c index 9376be1..2852fb6 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c @@ -534,7 +534,7 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt * resource that a guest CPU mmap reads back black. We still export the * DMA-BUF, but tag it FLAG_VIRGL so the parent reads it back on the GPU * (EGL import + glReadPixels), which sees the real host-rendered pixels. - * virtio_plain stays 0 if detection is unavailable; that just means "treat + * If detection is unavailable, virtio_virgl stays 0; that just means "treat * like plain" (export + direct map), which is correct for 2D virtio. */ static int virtio_detected = 0; /* 1 once we've probed for 3D features */ static int virtio_virgl = 0; /* 1 = confirmed virgl (3D features present) */ @@ -605,7 +605,14 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt close(prime_fd); return ret; } - /* Export failed — fall through to the dumb-map pixel path. */ + /* Export failed. For a confirmed virgl scanout the dumb-map fallback + * below would read back black (host-side resource), so fail closed + * rather than send a bogus all-black frame. */ + if (virtio_virgl) { + return send_error(sock, "virgl DMA-BUF export failed " + "(no usable CPU readback for a host scanout)"); + } + /* Otherwise fall through to the dumb-map pixel path. */ fprintf(stderr, "drmtap-helper: drmPrimeHandleToFD failed (%d), falling back to pixels\n", prime_ret); diff --git a/helper/drmtap-helper.c b/helper/drmtap-helper.c index 9376be1..2852fb6 100644 --- a/helper/drmtap-helper.c +++ b/helper/drmtap-helper.c @@ -534,7 +534,7 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt * resource that a guest CPU mmap reads back black. We still export the * DMA-BUF, but tag it FLAG_VIRGL so the parent reads it back on the GPU * (EGL import + glReadPixels), which sees the real host-rendered pixels. - * virtio_plain stays 0 if detection is unavailable; that just means "treat + * If detection is unavailable, virtio_virgl stays 0; that just means "treat * like plain" (export + direct map), which is correct for 2D virtio. */ static int virtio_detected = 0; /* 1 once we've probed for 3D features */ static int virtio_virgl = 0; /* 1 = confirmed virgl (3D features present) */ @@ -605,7 +605,14 @@ static int grab_and_send(int sock, int drm_fd, uint32_t target_crtc, int is_virt close(prime_fd); return ret; } - /* Export failed — fall through to the dumb-map pixel path. */ + /* Export failed. For a confirmed virgl scanout the dumb-map fallback + * below would read back black (host-side resource), so fail closed + * rather than send a bogus all-black frame. */ + if (virtio_virgl) { + return send_error(sock, "virgl DMA-BUF export failed " + "(no usable CPU readback for a host scanout)"); + } + /* Otherwise fall through to the dumb-map pixel path. */ fprintf(stderr, "drmtap-helper: drmPrimeHandleToFD failed (%d), falling back to pixels\n", prime_ret); diff --git a/src/drm_grab.c b/src/drm_grab.c index da13ffb..c8420ac 100644 --- a/src/drm_grab.c +++ b/src/drm_grab.c @@ -452,6 +452,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { /* virgl scanout: the helper exported the fd but a CPU mmap of it is * black (host-side resource), so force the GPU EGL readback path. */ int is_virgl = (hresult.wire.flags & HELPER_FLAG_VIRGL) != 0; + int pret = 0; /* gpu_auto_process result (propagated for virgl) */ size_t mmap_size = (size_t)frame->stride * frame->height; /* mmap the DMA-BUF in the parent */ @@ -481,7 +482,7 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { dmabuf_fd, mmap_size); if (do_mmap) { - gpu_auto_process(ctx, mapped, frame, is_virgl); + pret = gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -496,11 +497,18 @@ static int do_grab(drmtap_ctx *ctx, drmtap_frame_info *frame, int do_mmap) { "for EGL zero-copy", dmabuf_fd); if (do_mmap) { - gpu_auto_process(ctx, NULL, frame, is_virgl); + pret = gpu_auto_process(ctx, NULL, frame, is_virgl); } } drmModeFreeFB2(fb2); + /* For a virgl frame the GPU readback is the only way to get real + * pixels; if it failed, propagate the error rather than handing the + * caller a black frame. Non-virgl frames keep the prior best-effort + * behaviour (pret is ignored). */ + if (is_virgl && pret != 0) { + return pret; + } return 0; } @@ -716,6 +724,16 @@ static int gpu_auto_process(drmtap_ctx *ctx, void *data, } #endif + /* A virgl readback was forced but the EGL path did not produce pixels (EGL + * unavailable, or the import/readback failed). The only CPU-visible copy of + * a host-rendered scanout is black, so fail closed instead of returning a + * bogus frame as success. */ + if (force_egl) { + drmtap_set_error(ctx, + "virgl scanout needs GPU EGL readback, which is unavailable or failed"); + return -ENOTSUP; + } + /* --- CPU deswizzle for classic tiling (buffer already allocated above) --- */ const char *driver = ctx->driver_name; if (drmtap_gpu_intel_match(driver) ||