diff --git a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c index af4f2f4..c8420ac 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,10 @@ 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; + 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 */ @@ -478,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); + pret = gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -493,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); + 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; } @@ -522,7 +533,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 +610,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 +633,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; } @@ -708,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) || @@ -963,7 +989,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 +1084,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..2852fb6 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. + * 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) */ + 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,23 +586,33 @@ 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); 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/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..2852fb6 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. + * 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) */ + 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,23 +586,33 @@ 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); 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 af4f2f4..c8420ac 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,10 @@ 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; + 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 */ @@ -478,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); + pret = gpu_auto_process(ctx, mapped, frame, is_virgl); } } else { /* mmap failed — still have the fd for EGL zero-copy */ @@ -493,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); + 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; } @@ -522,7 +533,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 +610,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 +633,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; } @@ -708,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) || @@ -963,7 +989,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 +1084,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;