Implement virtio-gpu 2D and virtio-input devices by Mes0903 · Pull Request #121 · sysprog21/semu

Mes0903 · 2026-01-09T09:34:32Z

This pull request is derived from #34 and implements VirtIO-GPU 2D and VirtIO-Input devices for semu, enabling graphics and input support without requiring 3D/VirGL dependencies, in conformance with the VirtIO specification v1.3.

VirtIO-GPU

Based on VirtIO MMIO transport layer (i.e., Linux platform device)
Implements basic 2D rendering support.
- Resource creation and management
- 2D transfer (transformation between host and guest memory)
- Scanout configuration and display flushing
- Cursor operations (set, move, clear)
Provides software-based backend (supports 2D rendering only).
Compatible with:
- DirectFB2
- X11 Window System
Successfully tested with:
- kmscube
- glmark2
- glxgears

VirtIO-Input

Implements keyboard device with event forwarding
Implements mouse device with absolute positioning support
Integrates with window system for event handling

Prerequisites

Note: this list is not yet exhaustive

$ sudo apt install libsdl2-dev libsdl2-2.0-0 libsdl2-image-dev libsdl2-image-2.0-0

Build kernel image and test rootfs image

$ make rootfs.cpio # Download prebuilt minimal rootfs for initrd
$ ./scripts/build-image.sh --all --x11 --external-root --extra-packages

Use --x11 for full setup; omit it for a minimal setup.

Run X11

Launch semu:
```
$ make check
```
Run startx in semu
```
$ startx
```
After X11 shows up, launch the benchmark programs:
```
$ glmark2
$ glxgears
```

Run DirectFB2

Launch semu:
```
$ make check
```

Kill X11 (Skip if --x11 is not used)

$ ps | grep 'Xorg'
  <PID> root     /usr/bin/Xorg :0.0 vt01 -s 0 -noreset -allowMouseOpenFail
$ kill <PID>

Set up environment variables via run.sh:
```
$ source ./run.sh
```

Run DirectFB2 test programs:

df_fire        df_matrix      df_pss         df_window
df_andi        df_glgears     df_neo         df_spacedream
df_cpuload     df_input       df_palette     df_texture
df_dok         df_knuckles    df_particle    df_video
df_drivertest  df_layers      df_porter      df_vkcolor

Run kmscube

Launch semu:
```
$ make check
```

Kill X11 (Skip if --x11 is not used)

$ ps | grep 'Xorg'
  <PID> root     /usr/bin/Xorg :0.0 vt01 -s 0 -noreset -allowMouseOpenFail
$ kill <PID>

Run kmscube
```
$ kmscube
```

Run modetest

The simplest method to test VirtIO-GPU.

Unlike #34, this pull request enables the BR2_PACKAGE_LIBDRM_INSTALL_TESTS option by default in the Buildroot

Launch semu:
```
$ make check
```
Run modetest program
```
$ modetest -M virtio_gpu -s 37:1024x768
```

A test image should then show up in the display window.

The 37 is connector ID, you can check it by modetest -M virtio_gpu command. If you encounter a "Invalid argument" error, check your connector ID.

If you encounter a “Permission denied” error, make sure no other process is using the GPU. You can check this with ps | grep -E "Xorg|weston|kmscube|DirectFB" command. Then kill the relevant process(es) and try again.

Summary by cubic

Implements VirtIO-GPU 2D and VirtIO-Input with an SDL2 software display/input stack, plus offscreen/headless support and CI on Linux/macOS. Adds DirectFB2 cross-compilation, optional X11 builds, macOS main‑thread window handling, and independent primary/cursor tracking to avoid dropped flushes.

New Features
- VirtIO-GPU 2D over MMIO with software backend: create/attach/transfer, set scanout/flush, cursor set/move/clear, EDID; primary and cursor tracked independently.
- SDL2 display/input: macOS main-thread loop; offscreen on Linux and headless fallback; validated with libdrm modetest, X11 (glxgears, glmark2), kmscube, DirectFB2.
- VirtIO-Input keyboard and mouse with evdev mapping and absolute pointer; device config region supports sub-32-bit accesses.
- Build/test: Buildroot/Linux enable DRM/KMS, virtio-gpu/input, evdev, libdrm tests; DirectFB2 cross-compilation (Meson RISC-V cross file) and target/run.sh; optional X11 via --x11; CI runs .ci/test-vgpu.sh and .ci/test-vinput.sh offscreen/headless on Linux/macOS with ext4.img checks; Makefile auto-detects SDL2 and disables vGPU/vInput if missing.

^{Written for commit f5478f4. Summary will update on new commits.}

cubic-dev-ai

17 issues found across 24 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="scripts/rootfs_ext4.sh">

<violation number="1" location="scripts/rootfs_ext4.sh:14">
P2: This `cp` command will fail if the `extra_packages` directory doesn't exist. The directory is not present in the repository and there's no guard checking for its existence. Consider adding a conditional check to only copy when the directory exists.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:598">
P2: Halfword reads (RV_MEM_LH, RV_MEM_LHU) are passing size=1 instead of size=2. While the FIXME comment suggests per-byte access, this could cause incorrect behavior if the guest issues actual halfword reads. Consider separating the cases or documenting why size=1 is correct for all access widths.</violation>
</file>

<file name="configs/riscv-cross-file">

<violation number="1" location="configs/riscv-cross-file:16">
P2: Invalid `cpu_family` value: `'riscv'` is not a valid Meson cpu_family. According to Meson's reference tables, valid values for RISC-V are `'riscv32'` or `'riscv64'`. Since this targets 32-bit RISC-V, use `'riscv32'`.</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:96">
P2: Missing NULL check: `SDL_CreateRGBSurfaceWithFormatFrom` can return NULL on allocation failure. The returned `surface` is used directly in `SDL_CreateTextureFromSurface` without validation, which could cause undefined behavior.</violation>

<violation number="2" location="window-sw.c:212">
P1: Race condition: `cursor_clear_sw` modifies shared state and signals condition variable without holding the mutex. This can cause data races with the window thread which reads `cursor_rect` and `cursor` while holding the lock. Add `window_lock_mutex(scanout_id)` before modifying state and `window_unlock_mutex(scanout_id)` after signaling.</violation>

<violation number="3" location="window-sw.c:246">
P2: Race condition: cursor position is updated before acquiring the mutex. The `cursor_rect.x` and `cursor_rect.y` assignments should be inside the critical section to prevent data races with the window thread.</violation>
</file>

<file name="virtio-gpu.h">

<violation number="1" location="virtio-gpu.h:143">
P1: Pointer `uint8_t *capset_data` inside a PACKED structure is problematic. PACKED structures are used for wire format serialization, and pointers (which contain host addresses and vary in size between architectures) are not suitable. Consider using a flexible array member `uint8_t capset_data[];` instead.</violation>

<violation number="2" location="virtio-gpu.h:299">
P2: Typo in field name: `trasfer_to_host_2d` should be `transfer_to_host_2d` (missing 'n'). This inconsistency with the corresponding enum `VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D` could lead to confusion and bugs.</violation>

<violation number="3" location="virtio-gpu.h:332">
P2: Typo in function name: `vgpu_destory_resource_2d` should be `vgpu_destroy_resource_2d` (misspelled 'destroy' as 'destory').</violation>
</file>

<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:26">
P1: Memory leak: `res_2d->iovec` is allocated but not freed when returning early due to corrupted page address. Add `free(res_2d->iovec); res_2d->iovec = NULL;` before the return statement.</violation>

<violation number="2" location="virtio-gpu-sw.c:82">
P2: Incorrect buffer allocation formula mixes pixels and bytes. `request->width` is in pixels while `res_2d->stride` is in bytes (4096). The formula should likely be `res_2d->stride * request->height` since stride already represents bytes per row.</violation>
</file>

<file name="common.h">

<violation number="1" location="common.h:25">
P1: Using `1 << bit` where `bit` can be >= 32 causes undefined behavior. The literal `1` is of type `int` (32-bit), but `bit` is `unsigned long` and can exceed 31 on 64-bit systems (since `BITS_PER_LONG` would be 64). Use `1UL << bit` to ensure the shift operates on an `unsigned long`.</violation>
</file>

<file name="virtio-gpu.c">

<violation number="1" location="virtio-gpu.c:78">
P0: Buffer offset calculated twice: `dest` already includes `done` offset (line 76), but `memcpy` adds `done` again. This writes data to the wrong location (`buf + 2*done` instead of `buf + done`).</violation>

<violation number="2" location="virtio-gpu.c:273">
P1: Logical AND `&&` used instead of bitwise AND `&`. This results in `edid[9]` being set to 1 (if vendor_id is non-zero) instead of the lower 8 bits of vendor_id, producing an incorrect EDID vendor identifier.</violation>
</file>

<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:180">
P1: Missing directory existence check before `git clone`. If the script is run twice, this will fail because the `DirectFB2` directory already exists. This is inconsistent with the patterns used in `do_buildroot` and `do_linux` which check `if [ ! -d directory ]` before cloning.</violation>

<violation number="2" location="scripts/build-image.sh:190">
P1: Missing directory existence check before `git clone`. Same issue as above - running the script twice will fail because the `DirectFB-examples` directory already exists.</violation>
</file>

<file name="window-events.c">

<violation number="1" location="window-events.c:86">
P1: Copy-paste error: `SDLK_F7` is mapped twice while `SDLK_F8` is missing. The F8 key won't work.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-gpu.c

window-sw.c

virtio-gpu.h

virtio-gpu-sw.c

common.h

window-sw.c

virtio-gpu.h

virtio-gpu-sw.c

shengwen-tw · 2026-01-09T17:52:54Z

input-event-codes.h

This file is copied directly from the Linux kernel source.
Define the key codes required by semu and retain the reference to the original source to avoid license violations.

shengwen-tw · 2026-01-09T17:58:58Z

configs/riscv-cross-file

 [host_machine]
    system = 'linux'
-    cpu_family = 'riscv'
+    cpu_family = 'riscv32'


Any reference to justify this?

There are only riscv32 and riscv64 in the Meson reference table for CPU families

https://mesonbuild.com/Reference-tables.html#cpu-families

I see, thanks for the clarification.

shengwen-tw · 2026-01-09T18:06:05Z

window-events.c

+
+struct key_map_entry key_map[] = {
+    /* Mouse */
+    DEF_KEY_MAP(SDL_BUTTON_LEFT, BTN_LEFT),


This does not cover all key code, we can create a new GitHub issue once the pull request is merged.

shengwen-tw · 2026-01-09T18:13:03Z

virtio-gpu-sw.c

    res_2d->stride = STRIDE_SIZE;
-    res_2d->image = malloc(bytes_per_pixel * (request->width + res_2d->stride) *
-                           request->height);
+    res_2d->image = malloc(res_2d->stride * request->height);


Not sure why this change is made.
Could you compare with this reference? (line 511)
psun3x/acrn-hypervisor@edc137c

The call graph in the reference you gave will call

pixman_image_create_bits(r2d->format, r2d->width, r2d->height, NULL, 0);

This will get into create_bits_image_internal with rowstride_bytes set to 0. Then it call create_bits further, which is the place that will calculate the stride.

In create_bits, the main process for the calculation of stride is:

stride = width * bpp;: Calculate the total bit.

stride += 0x1f;: Alignment.

stride >>= 5;: Equivlaent to devided by 32, to calculate how many 32-bit it need.

stride *= sizeof(uint32_t);: Transform to byte.

For example, take width as 100, if the format is VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM, which will be transformed to PIXMAN_x8r8g8b8, then the bit per pixel (bpp) is 32, thus the stride is ((100 * 32 + 31) / 32) * 4, which is 400 (byte).

After it get the stride, it will just calculate the size of buffer via height * stride directly and malloc(buf_size).

Thus, I think the point is that the size we need to malloc is height * stride, but the original implementation mixes pixels and bytes (request->width + res_2d->stride). The correct formula is (bytes_per_pixel * request->width) * request->height, where the first part is res_2d->stride.

BTW, as @visitorckw mentioned below, the res_2d->stride should be calculated as width * bpp, but I'm not sure if I need to do the alignment here.

I briefly looked through the kernel code. For dumb buffers, it's simply width * bpp without alignment.

https://elixir.bootlin.com/linux/v6.12.64/source/drivers/gpu/drm/virtio/virtgpu_gem.c#L74

The original implementation of my virtio-gpu used pixman. When I attempted to remove the pixman dependency, I introduced a hard-coded STRIDE_SIZE of 4096 as @visitorckw pointed out. After reviewing the VirtIO GPU spec. again, I think this may be incorrect. shengwen-tw@1120d55

In virtio-gpu 2D mode (VIRTIO_GPU_CMD_RESOURCE_CREATE_2D, VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D, and VIRTIO_GPU_CMD_SET_SCANOUT), the protocol does not define an explicit stride field. This suggests that the expected memory layout might be just tightly packed, i.e., each row is exactly width * bytes_per_pixel with no additional per-row padding. If so, explicit stride handling may be unnecessary or even incorrect.

@Mes0903 :
Could you help run some experiments to confirm whether we can remove the stride handling entirely? For example, by checking the size of the received buffer and testing what happens when STRIDE_SIZE is set to 0.

shengwen-tw · 2026-01-09T18:17:18Z

scripts/build-image.sh

+function copy_buildroot_config
+{
+    local buildroot_config="configs/buildroot.config"
+    local x11_config="configs/x11.config"


@jserv:
I prefer to keep the X11 build configuration optional, as we will need it later to test and verify VirtIO GPU 3D mode.

I prefer to keep the X11 build configuration optional, as we will need it later to test and verify VirtIO GPU 3D mode.

X11 specific configurations should be excluded for this pull request, keeping it simple.

@Mes0903
Let’s separate the X11 build option from this pull request and move it into a new commit, in case we need to use it later.

jserv

Add headless tests in CI/CD.

shengwen-tw · 2026-01-10T12:12:27Z

virtio-input.c

+    vinput->ram[queue->QueueUsed] &= MASK(16);
+    /* Update the used ring pointer (virtq_used.idx) */
+    /* TODO: Check if the or-ing is valid or not */
+    // vinput->ram[queue->QueueUsed] |= ((uint32_t) new_used) << 16;


@Mes0903
Could you check the correct implementation according to the VirtIO spec.?

I think the current implementation already following the spec, but the MASK(16) is for lower 16 bits, which means keep lower 16 bits (flags) and clean higher 16 bits (idx). I think the original implementation is contrary to the comment.

I changed to the following implementation for readability for now:

/* Update used ring header */ uint16_t *used_hdr = (uint16_t *) &vinput->ram[queue->QueueUsed]; used_hdr[0] = 0; /* virtq_used.flags */ used_hdr[1] = new_used; /* virtq_used.idx */

Also, the spec said

A device MUST NOT write to any descriptor table entry.

So I delete the desc[3] = 0 part and add some simple check.

shengwen-tw · 2026-01-10T12:16:44Z

virtio-input.c

+        break;
+    case RV_MEM_SB:
+    case RV_MEM_SH:
+        /* FIXME: virtio-input driver need to access device config register per


Please refactor the code here.

visitorckw

Patches 4 and 5 appear to be fixups for earlier commits. This violates commit atomicity and breaks bisectability. Please use git rebase -i to squash them into the relevant patches.

visitorckw · 2026-01-10T13:38:02Z

virtio-gpu-sw.c

+#include "virtio-gpu.h"
+#include "window.h"
+
+#define STRIDE_SIZE 4096


How was this value derived?

Shouldn't this be calculated dynamically based on width * bytes_per_pixel (or the stride provided by the guest) to support arbitrary resolutions and avoid potential display corruption?

I think you're right here, thank you.

shengwen-tw · 2026-01-10T15:58:31Z

Patches 4 and 5 appear to be fixups for earlier commits. This violates commit atomicity and breaks bisectability. Please use git rebase -i to squash them into the relevant patches.

@Mes0903
Please rebase the commits once the following issue is addressed:
https://github.com/sysprog21/semu/pull/121/files/d56eae31d498ba1c7b84e3bda2ed4b6c9671f439..a641357e4101ab10111311fdb3ebb2a86d7fdeb5#diff-ff10d79192b7c7bd8c0fd33ee7d45867c5f30eb1866a0a22f8b7c2b4e159312eR82

ChinYikMing · 2026-01-11T09:40:00Z

@Mes0903 , could you please upload the patched rootfs.cpio to this PR thread for quick testing? Thanks.

ChinYikMing · 2026-01-11T09:41:15Z

device.h

+    /* supplied by environment */
+    uint32_t *ram;
+    /* implementation-specific */
+    int id;  // FIXME


More comment for this?

ChinYikMing · 2026-01-11T09:43:14Z

main.c

+#if SEMU_HAS(VIRTIOGPU)
+    emu->vgpu.ram = emu->ram;
+    virtio_gpu_init(&(emu->vgpu));
+    virtio_gpu_add_scanout(&(emu->vgpu), 1024, 768);


Can you reuse SCREEN_WIDTH and SCREEN_HEIGHT?

ChinYikMing · 2026-01-11T09:46:37Z

virtio-gpu-sw.c

+        bits_per_pixel = 32;
+        break;
+    case VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM:
+        bits_per_pixel = 32;


The cases here and above are all do the same thing: bits_per_pixel = 32; break;, can you merge them?

cubic-dev-ai

11 issues found across 24 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="target/run.sh">

<violation number="1" location="target/run.sh:3">
P2: Avoid introducing an empty LD_LIBRARY_PATH entry when the variable was previously unset—an empty entry makes the dynamic linker search the current directory first, which is a security risk.</violation>
</file>

<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:175">
P2: Inconsistent PATH handling: appends buildroot toolchain to PATH instead of prepending. Other functions (`do_linux`, `do_extra_packages`) correctly prepend to ensure the cross-compiler is found first. This could cause the wrong toolchain to be used if the system has conflicting tools.</violation>
</file>

<file name="virtio-gpu.h">

<violation number="1" location="virtio-gpu.h:7">
P2: Duplicate macro definition. `VIRTIO_GPU_MAX_SCANOUTS` is already defined in `virtio.h` which is included by this file. Remove this duplicate definition to avoid potential maintenance issues if the values diverge.</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:245">
P1: Missing NULL check after `vgpu_get_resource_2d`. If `res_id` is invalid, the function returns NULL and `resource->format` on line 249 will cause a null pointer dereference. Other call sites in virtio-gpu-sw.c always check for NULL.</violation>

<violation number="2" location="window-sw.c:271">
P1: Missing NULL check after `malloc`. If memory allocation fails, `memcpy` on line 267 will dereference NULL pointer.</violation>
</file>

<file name="scripts/rootfs_ext4.sh">

<violation number="1" location="scripts/rootfs_ext4.sh:16">
P2: Copying extra packages with `cp -r` drops ownership/permission data and dereferences symlinks; use `cp -a` to preserve metadata so the rootfs image contains valid files.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:666">
P1: Missing bounds check on `vinput_dev_cnt` before using as array index. If `virtio_input_init` is called more than `VINPUT_DEV_CNT` (2) times, this causes an out-of-bounds write to `vinput_dev[]`.</violation>
</file>

<file name="window-events.c">

<violation number="1" location="window-events.c:138">
P1: Busy-waiting loop will consume 100% CPU when no events are pending. Consider using `SDL_WaitEvent` instead of `SDL_PollEvent`, or add `SDL_Delay(1)` inside the loop when no event is received.</violation>

<violation number="2" location="window-events.c:145">
P2: Unchecked return value of `sdl_key_to_linux_key`. When the key is not found, -1 is passed to `virtio_input_update_key`, which will be interpreted as 0xFFFFFFFF (invalid key code). Add a check before calling the virtio function.</violation>
</file>

<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:32">
P1: Memory leak: when `malloc(res_2d->image)` fails, the previously allocated `res_2d` structure is not freed and not removed from the resource list. Call `vgpu_destroy_resource_2d(request->resource_id)` before returning to properly clean up.</violation>

<violation number="2" location="virtio-gpu-sw.c:209">
P1: Potential NULL pointer dereference: `res_2d->iovec` may be NULL if `attach_backing` hasn't been called yet for this resource. Add a NULL check before accessing `res_2d->iovec[0].iov_base`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

window-sw.c

virtio-input.c

window-events.c

virtio-gpu-sw.c

cubic-dev-ai · 2026-01-11T09:55:26Z

target/run.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/bash
+
+export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib


P2: Avoid introducing an empty LD_LIBRARY_PATH entry when the variable was previously unset—an empty entry makes the dynamic linker search the current directory first, which is a security risk.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At target/run.sh, line 3: <comment>Avoid introducing an empty LD_LIBRARY_PATH entry when the variable was previously unset—an empty entry makes the dynamic linker search the current directory first, which is a security risk.</comment> <file context> @@ -0,0 +1,4 @@ +#!/usr/bin/bash + +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib +export PATH=$PATH:/usr/local/bin/ </file context>

Suggested change

export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib

export LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}/usr/local/lib

cubic-dev-ai · 2026-01-11T09:55:26Z

scripts/build-image.sh


+function do_directfb
+{
+    export PATH=$PATH:$PWD/buildroot/output/host/bin


P2: Inconsistent PATH handling: appends buildroot toolchain to PATH instead of prepending. Other functions (do_linux, do_extra_packages) correctly prepend to ensure the cross-compiler is found first. This could cause the wrong toolchain to be used if the system has conflicting tools.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/build-image.sh, line 175: <comment>Inconsistent PATH handling: appends buildroot toolchain to PATH instead of prepending. Other functions (`do_linux`, `do_extra_packages`) correctly prepend to ensure the cross-compiler is found first. This could cause the wrong toolchain to be used if the system has conflicting tools.</comment> <file context> @@ -133,11 +170,72 @@ while [[ $# -gt 0 ]]; do +function do_directfb +{ + export PATH=$PATH:$PWD/buildroot/output/host/bin + export BUILDROOT_OUT=$PWD/buildroot/output/ + mkdir -p directfb </file context>

Suggested change

export PATH=$PATH:$PWD/buildroot/output/host/bin

export PATH="$PWD/buildroot/output/host/bin:$PATH"

virtio-gpu.h

scripts/rootfs_ext4.sh

window-events.c

cubic-dev-ai

17 issues found across 24 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="target/run.sh">

<violation number="1" location="target/run.sh:3">
P2: Prevent LD_LIBRARY_PATH from acquiring a leading empty entry (which adds the current directory to the search path) when the variable is initially unset.</violation>

<violation number="2" location="target/run.sh:4">
P2: Guard against introducing an empty PATH entry so the current working directory is not implicitly added ahead of trusted binaries.</violation>
</file>

<file name="configs/riscv-cross-file">

<violation number="1" location="configs/riscv-cross-file:8">
P1: `@GLOBAL_SOURCE_ROOT@` is not a valid Meson cross-file variable and won't be substituted. The build script copies this file directly without preprocessing. Consider using absolute paths or add a preprocessing step to substitute this placeholder before running meson.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:168">
P2: Wraparound arithmetic is off by one. When handling uint16_t index wraparound, adding `UINT16_MAX` (65535) is incorrect; it should be `UINT16_MAX + 1` (65536) or `(1U << 16)` to properly account for the full 16-bit range.</violation>

<violation number="2" location="virtio-input.c:207">
P1: Missing bounds validation on `vq_desc.addr` before memory write. A malicious guest could provide an out-of-bounds address, causing writes outside the allocated RAM. Add a check: `if (vq_desc.addr + sizeof(struct virtio_input_event) > RAM_SIZE) return false;`</violation>

<violation number="3" location="virtio-input.c:666">
P1: Missing bounds check on `vinput_dev_cnt` before array access. If `virtio_input_init` is called more than `VINPUT_DEV_CNT` (2) times, this will cause an out-of-bounds write to `vinput_dev[]`. Add a bounds check before assignment.</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:245">
P1: Missing NULL check after `vgpu_get_resource_2d()`. If the resource is not found, the function returns NULL and accessing `resource->format` will cause a crash.</violation>

<violation number="2" location="window-sw.c:271">
P1: Missing NULL check after `malloc()`. If memory allocation fails, `memcpy` on line 263 will dereference NULL and crash.</violation>
</file>

<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:24">
P1: Memory leak: `res_2d` is already created and added to the resource list when image allocation fails. Call `vgpu_destroy_resource_2d(request->resource_id)` before returning to properly clean up the resource.</violation>

<violation number="2" location="virtio-gpu-sw.c:209">
P1: Potential NULL pointer dereference: `res_2d->iovec[0].iov_base` is accessed without checking if `res_2d->iovec` is non-NULL. If transfer is called before backing memory is attached, this will crash. Add a null check before accessing iovec.</violation>
</file>

<file name="window-events.c">

<violation number="1" location="window-events.c:145">
P1: The return value of `sdl_key_to_linux_key` (-1 for unmapped keys) is not validated before being passed to `virtio_input_update_key`. This causes -1 to be implicitly converted to `UINT32_MAX` and sent as an invalid key code to the guest.</violation>

<violation number="2" location="window-events.c:150">
P1: Same issue as SDL_KEYDOWN: the return value of `sdl_key_to_linux_key` (-1 for unmapped keys) is not validated before being passed to `virtio_input_update_key`.</violation>

<violation number="3" location="window-events.c:155">
P2: The return value of `sdl_key_to_linux_key` is not validated before being passed to `virtio_input_update_mouse_button_state`. Unmapped mouse buttons (scroll wheel, side buttons) will result in invalid button codes.</violation>

<violation number="4" location="window-events.c:160">
P2: Same issue as SDL_MOUSEBUTTONDOWN: the return value of `sdl_key_to_linux_key` is not validated before being passed to `virtio_input_update_mouse_button_state`.</violation>
</file>

<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:47">
P1: Missing `ASSERT` wrapper on merge_tool command. If the config merge fails, the script will continue with an incorrect or incomplete Buildroot configuration, leading to confusing build failures later.</violation>

<violation number="2" location="scripts/build-image.sh:190">
P2: Missing `ASSERT` wrapper on `meson install` command. If installation fails, the script continues silently, potentially resulting in incomplete package installation.</violation>
</file>

<file name="virtio-gpu.c">

<violation number="1" location="virtio-gpu.c:139">
P1: Missing bounds validation for guest address. The function converts guest addresses to host pointers without checking if `addr` is within valid RAM bounds. This could allow a malicious guest to cause out-of-bounds memory access by providing an invalid address in virtqueue descriptors.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-01-11T09:57:03Z

configs/riscv-cross-file

@@ -0,0 +1,18 @@
+[binaries]


P1: @GLOBAL_SOURCE_ROOT@ is not a valid Meson cross-file variable and won't be substituted. The build script copies this file directly without preprocessing. Consider using absolute paths or add a preprocessing step to substitute this placeholder before running meson.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At configs/riscv-cross-file, line 8: <comment>`@GLOBAL_SOURCE_ROOT@` is not a valid Meson cross-file variable and won't be substituted. The build script copies this file directly without preprocessing. Consider using absolute paths or add a preprocessing step to substitute this placeholder before running meson.</comment> <file context> @@ -0,0 +1,18 @@ + python = '/usr/bin/python3' + +[properties] + pkg_config_libdir = ['@GLOBAL_SOURCE_ROOT@' / '../buildroot/output/host/riscv32-buildroot-linux-gnu/sysroot/usr/local/lib/pkgconfig', + '@GLOBAL_SOURCE_ROOT@' / '../buildroot/output/host/riscv32-buildroot-linux-gnu/sysroot/usr/share/pkgconfig/', + '@GLOBAL_SOURCE_ROOT@' / '../buildroot/output/host/riscv32-buildroot-linux-gnu/sysroot/usr/lib/pkgconfig/' </file context>

virtio-input.c

window-sw.c

cubic-dev-ai · 2026-01-11T09:57:04Z

target/run.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/bash
+
+export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib


P2: Prevent LD_LIBRARY_PATH from acquiring a leading empty entry (which adds the current directory to the search path) when the variable is initially unset.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At target/run.sh, line 3: <comment>Prevent LD_LIBRARY_PATH from acquiring a leading empty entry (which adds the current directory to the search path) when the variable is initially unset.</comment> <file context> @@ -0,0 +1,4 @@ +#!/usr/bin/bash + +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib +export PATH=$PATH:/usr/local/bin/ </file context>

Suggested change

export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib

export LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+${LD_LIBRARY_PATH}:}/usr/local/lib

virtio-input.c

window-events.c

cubic-dev-ai · 2026-01-11T09:57:04Z

scripts/build-image.sh

+    cp ../configs/riscv-cross-file .
+    ASSERT meson -Ddrmkms=true --cross-file riscv-cross-file build/riscv
+    ASSERT meson compile -C build/riscv
+    DESTDIR=$BUILDROOT_OUT/host/riscv32-buildroot-linux-gnu/sysroot meson install -C build/riscv


P2: Missing ASSERT wrapper on meson install command. If installation fails, the script continues silently, potentially resulting in incomplete package installation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/build-image.sh, line 190: <comment>Missing `ASSERT` wrapper on `meson install` command. If installation fails, the script continues silently, potentially resulting in incomplete package installation.</comment> <file context> @@ -133,11 +170,72 @@ while [[ $# -gt 0 ]]; do + cp ../configs/riscv-cross-file . + ASSERT meson -Ddrmkms=true --cross-file riscv-cross-file build/riscv + ASSERT meson compile -C build/riscv + DESTDIR=$BUILDROOT_OUT/host/riscv32-buildroot-linux-gnu/sysroot meson install -C build/riscv + DESTDIR=../../../directfb meson install -C build/riscv + popd </file context>

shengwen-tw · 2026-01-11T12:20:46Z

virtio-gpu-sw.c

    res_2d->stride = STRIDE_SIZE;
-    res_2d->image = malloc(bytes_per_pixel * (request->width + res_2d->stride) *
-                           request->height);
+    res_2d->image = malloc(res_2d->stride * request->height);


The original implementation of my virtio-gpu used pixman. When I attempted to remove the pixman dependency, I introduced a hard-coded STRIDE_SIZE of 4096 as @visitorckw pointed out. After reviewing the VirtIO GPU spec. again, I think this may be incorrect. shengwen-tw@1120d55

In virtio-gpu 2D mode (VIRTIO_GPU_CMD_RESOURCE_CREATE_2D, VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D, and VIRTIO_GPU_CMD_SET_SCANOUT), the protocol does not define an explicit stride field. This suggests that the expected memory layout might be just tightly packed, i.e., each row is exactly width * bytes_per_pixel with no additional per-row padding. If so, explicit stride handling may be unnecessary or even incorrect.

@Mes0903 :
Could you help run some experiments to confirm whether we can remove the stride handling entirely? For example, by checking the size of the received buffer and testing what happens when STRIDE_SIZE is set to 0.

Mes0903 · 2026-01-28T12:41:35Z

@shengwen-tw :

Regarding how stride should be set, after a quick review of the Mesa and Xorg codebases, I think that in the 2D case the code path used is the dumb-buffer path in the guest Linux. In that path, stride is computed and provided by the guest Linux, and the key part is:

	if (args->bpp != 32)
		return -EINVAL;

	pitch = args->width * 4;

As shown, it pins bits-per-pixel to 32, and the pitch is directly width * 4. See virtgpu_gem.c:61:
https://github.com/torvalds/linux/blob/0f61b1860cc3f52aef9036d7235ed1f017632193/drivers/gpu/drm/virtio/virtgpu_gem.c#L61

Therefore, I currently follow the approach used by QEMU and ACRN (with pixman), namely applying an additional alignment step on top of this, as a precaution:

stride = ((request->width * bits_per_pixel + 0x1f) >> 5) * sizeof(uint32_t);

See QEMU implementation and pixman implementation for more details.

Also, I recorded my trace process in my blog (in Chinese)

Mes0903 · 2026-01-28T12:44:33Z

@ChinYikMing :

I’ve uploaded ext4.img.bz2 and Image.bz2 for testing usage. I think the code can now be tested quickly by running:

bunzip2 Image.bz2 ext4.img.bz2
make check

Mes0903 · 2026-01-28T12:46:36Z

There are two remaining problem for now:

Add headless tests in CI/CD.
Make the X11 build configuration optional.

Did I miss anything?

Mes0903 · 2026-01-29T10:39:51Z

I have now moved the X11-related changes into the second commit. The first commit can be tested by DirectFB2 test program.

cubic-dev-ai · 2026-02-27T13:33:58Z

@cubic-dev-ai

@Mes0903 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

4 issues found across 28 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="virtio-gpu.c">

<violation number="1" location="virtio-gpu.c:437">
P1: Out-of-bounds read: the chained descriptor index from `desc[3] >> 16` (guest-controlled) is not validated against `queue->QueueNum` before being used to compute the next descriptor address. A malicious or buggy guest can cause `queue->QueueDesc + desc_idx * 4` to exceed the RAM array bounds. Add a bounds check on the new `desc_idx` before the next iteration.</violation>
</file>

<file name=".ci/test-vgpu.sh">

<violation number="1" location=".ci/test-vgpu.sh:27">
P2: Validate the cached `ext4.img.bz2` checksum (and re-download on mismatch). Skipping integrity checks for existing files can lead to using a corrupted cache and flaky CI failures.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:252">
P1: Buffer leak on partial descriptor handler failure: if a descriptor check fails mid-loop, `queue->last_avail` has already been incremented for prior iterations, but the used ring header idx is never updated (and the caller doesn't trigger a device reset). This permanently leaks avail ring buffers. Consider either (a) not incrementing `queue->last_avail` until all events are validated, (b) updating the used ring header before returning `false` for partially processed events, or (c) treating the partial failure as a device error via `goto fail` in the caller.</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:302">
P1: Validate scanout_id before indexing displays[] to avoid out-of-bounds access from guest-controlled scanout IDs. Add a range check (e.g., 0 <= scanout_id < display_cnt) in each window_* entry point or validate in the virtio-gpu command handlers before calling the window backend.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-gpu.c

virtio-input.c

window-sw.c

.ci/test-vgpu.sh

Mes0903 · 2026-02-27T15:34:46Z

@cubic-dev-ai

cubic-dev-ai · 2026-02-27T15:34:54Z

@cubic-dev-ai

@Mes0903 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

6 issues found across 28 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:48">
P2: `--x11` is silently ignored when buildroot/.config already exists. Consider re-merging the config or at least updating it when X11 is requested so the flag consistently takes effect.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:257">
P1: Potential NULL pointer dereference: `vinput_dev[dev_id].vinput` is used without checking for NULL. If an SDL input event arrives before `virtio_input_init()` has been called for this device, `vinput` will be NULL and the subsequent `vinput->ram` dereference will crash. Add a NULL guard early in the function.</violation>

<violation number="2" location="virtio-input.c:344">
P2: Keyboard device incorrectly reports `INPUT_PROP_POINTER` and `INPUT_PROP_DIRECT` properties. These properties are for pointing devices (mice, touchscreens), not keyboards. This could cause the Linux guest's input subsystem to misclassify the keyboard. For keyboards, the property bitmap should be empty (size = 0).</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:347">
P1: Cursor buffer allocation ignores resource stride, which can cause out-of-bounds reads when stride includes padding. Allocate/copy using `resource->stride * resource->height` like the primary plane.</violation>
</file>

<file name="virtio-gpu.c">

<violation number="1" location="virtio-gpu.c:137">
P1: Potential buffer overflow: `vgpu_mem_guest_to_host` only checks that the base `addr < RAM_SIZE`, but callers write entire structs (e.g., `struct vgpu_resp_disp_info`, which is hundreds of bytes) through the returned pointer without verifying `addr + sizeof(struct) <= RAM_SIZE`. A malformed guest descriptor could place addr near the RAM boundary, causing out-of-bounds writes in subsequent `memset`/`memcpy` calls. Consider adding a size parameter to `vgpu_mem_guest_to_host` so it can validate the full access range.</violation>

<violation number="2" location="virtio-gpu.c:436">
P2: Missing bounds check on descriptor table access. The computed index `queue->QueueDesc + desc_idx * 4 + 3` is not validated against `RAM_SIZE / 4` before dereferencing `vgpu->ram[...]`. While `QueueDesc` is validated when set, and `desc_idx < QueueNum`, the combined offset could still exceed the RAM buffer when `QueueDesc` is near the end of memory.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-input.c

window-sw.c

virtio-gpu.c

scripts/build-image.sh

virtio-input.c

virtio-gpu.c

Mes0903 · 2026-03-02T14:56:13Z

@cubic-dev-ai

cubic-dev-ai · 2026-03-02T14:56:23Z

@cubic-dev-ai

@Mes0903 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

8 issues found across 28 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:25">
P1: Missing `*plen = 0` before return. Every other `virtio_gpu_set_fail` error path in this file sets `*plen = 0`, but this one does not. The caller will use a stale/undefined `plen` value, potentially leading to incorrect virtqueue completion.</violation>

<violation number="2" location="virtio-gpu-sw.c:73">
P1: Missing `*plen = 0` before return. Same pattern as the other allocation failure path — `virtio_gpu_set_fail` is called but `*plen` is not zeroed, inconsistent with every other error path in this file.</violation>

<violation number="3" location="virtio-gpu-sw.c:444">
P1: Missing `*plen = 0` before return. The iovec allocation failure path calls `virtio_gpu_set_fail` but doesn't zero `*plen`, unlike every other error path in this function and file.</violation>
</file>

<file name=".ci/test-vgpu.sh">

<violation number="1" location=".ci/test-vgpu.sh:24">
P2: The GitHub raw URL uses `/raw/blob/...`, but raw links must be `/raw/<branch>/...`. Unless a branch named `blob` exists, this will 404 and the DirectFB test download will fail. Replace `blob` with the actual branch name (e.g., `main`).</violation>
</file>

<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:41">
P2: Always regenerate buildroot/.config instead of skipping when it already exists so config fragments (like X11) are reliably applied.

(Based on your team's feedback about always regenerating Buildroot’s .config during builds.) [FEEDBACK_USED]</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:476">
P1: `VIRTIO_INPUT_CFG_UNSET` (0x00) is not handled in `virtio_input_cfg_read`, so it falls into the default case which prints an error and returns `false`, ultimately raising a LOAD_FAULT exception in the guest. Per the VirtIO spec, unsupported select/subsel combinations (including UNSET) should simply set `size = 0` and succeed. This will trigger on every config read after device reset.</violation>
</file>

<file name="window-sw.c">

<violation number="1" location="window-sw.c:305">
P2: `SDL_LockMutex` return value is not checked — if the lock fails, the function proceeds to modify shared display state unsafely. The main render loop correctly guards with `if (SDL_LockMutex(...) == 0)`, but these producer functions do not. Either check the return value and bail on failure, or at minimum log the error.</violation>

<violation number="2" location="window-sw.c:413">
P1: Missing bounds validation on `scanout_id` before array access. If an invalid `scanout_id` (>= `display_cnt`) is passed, the code accesses uninitialized `displays[]` entries with NULL mutex/cond pointers, leading to undefined behavior. Add a bounds check early in each function, consistent with how `window_add_sw` validates `display_cnt`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-gpu-sw.c

virtio-input.c

window-sw.c

.ci/test-vgpu.sh

scripts/build-image.sh

window-sw.c

Mes0903 · 2026-03-02T16:42:16Z

@cubic-dev-ai

cubic-dev-ai · 2026-03-02T16:42:35Z

@cubic-dev-ai

@Mes0903 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

3 issues found across 28 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:41">
P2: Always regenerate Buildroot’s .config (and merge the X11 fragment when requested) instead of skipping when buildroot/.config exists; otherwise config updates and toggling --x11 won’t take effect.

(Based on your team's feedback about always regenerating Buildroot’s .config during builds.) [FEEDBACK_USED]</violation>
</file>

<file name="virtio-gpu.c">

<violation number="1" location="virtio-gpu.c:588">
P2: Misleading comments: (1) `/* Reset low 16 bits to zero */` is inverted — `&= MASK(16)` keeps the low 16 bits and clears the *high* 16 bits. (2) `/* len */` should be `/* idx */` since this writes `virtq_used.idx`, not a length. These could mislead maintainers modifying the used-ring update logic.</violation>
</file>

<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:406">
P1: Buffer over-read: only a single `vgpu_mem_entry` is validated/mapped, but `nr_entries` are iterated. When `nr_entries > 1`, entries beyond the first access memory that was never bounds-checked against `RAM_SIZE`. Both the descriptor length check and the `vgpu_mem_guest_to_host` call should account for the full array size (`sizeof(struct vgpu_mem_entry) * backing_info->nr_entries`).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-gpu-sw.c

scripts/build-image.sh

virtio-gpu.c

Mes0903 · 2026-03-02T17:05:07Z

@cubic-dev-ai

cubic-dev-ai · 2026-03-02T17:05:14Z

@cubic-dev-ai

@Mes0903 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

4 issues found across 28 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/build-image.sh">

<violation number="1" location="scripts/build-image.sh:41">
P2: Always regenerate Buildroot’s .config (merging the base config and optional X11 fragment) instead of skipping when it already exists; otherwise builds can silently use stale settings.

(Based on your team's feedback about always regenerating Buildroot’s .config.) [FEEDBACK_USED]</violation>
</file>

<file name=".ci/test-vgpu.sh">

<violation number="1" location=".ci/test-vgpu.sh:24">
P2: `EXT4_URL` uses an invalid GitHub raw URL (`/raw/blob` with no branch), which will fail to download the ext4 image and break the vGPU CI test.</violation>
</file>

<file name="virtio-input.c">

<violation number="1" location="virtio-input.c:273">
P1: Bug: Input events arriving before the guest driver is fully initialized (`DRIVER_OK` not set or queue not ready) will trigger `virtio_input_set_fail()`, setting `DEVICE_NEEDS_RESET` and permanently bricking the device. Any key press or mouse movement during guest boot would cause this. These events should be silently dropped (use `goto success`) instead of failing the device, matching the pattern used for the `DEVICE_NEEDS_RESET` check just above.</violation>
</file>

<file name="virtio-gpu-sw.c">

<violation number="1" location="virtio-gpu-sw.c:88">
P1: Integer overflow in image buffer allocation: `res_2d->stride * request->height` is computed as `uint32_t * uint32_t`, which can silently wrap around to a small value (or zero) for large guest-controlled dimensions. This leads to an undersized `malloc` while subsequent transfer operations write based on the original (non-truncated) dimensions, causing a heap buffer overflow on the host. Cast to `size_t` before multiplying, and add an upper-bound sanity check on the requested dimensions.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

virtio-input.c

virtio-gpu-sw.c

scripts/build-image.sh

.ci/test-vgpu.sh

shengwen-tw · 2026-03-02T17:14:11Z

@cubic-dev-ai Is there a limit number of suggestions you can make per one code review request?

cubic-dev-ai · 2026-03-02T17:14:57Z

@cubic-dev-ai Is there a limit number of suggestion you can make per one code review request?

@shengwen-tw There isn’t a fixed "hard cap" on the number of suggestions I can make in a single review, but I am designed to prioritize high-signal issues to avoid cluttering your PR with noise.

However, there are a few specific limits and settings that govern how much feedback you’ll see:

Sensitivity Levels: You can adjust how "picky" I am via the sensitivity setting in your cubic.yaml or project settings. High sensitivity surfaces more minor improvements and stylistic suggestions, while Low sensitivity focuses only on critical bugs and security risks.
File Limit: For very large PRs, I review the first 150 eligible files. Files exceeding this limit or those matching your ignore patterns are skipped to ensure the review remains timely.
Incremental Reviews: By default, I only post comments on new issues introduced in recent commits. If you've already addressed a suggestion or it hasn't changed, I won't repeat it in subsequent review runs.
Monthly Quota: If you are on the free plan, there is a limit of 20 AI reviews per month across your organization. You can check your remaining quota in the review comments I post.

If you feel I'm being too quiet or too verbose, you can tweak these behaviors in your AI review settings or by providing specific instructions in your cubic.yaml.

<helpful_urls>

cubic-dev-ai · 2026-03-02T17:15:19Z

@cubic-dev-ai Is there a limit number of suggestions you can make per one code review request?

@shengwen-tw There isn't a hard numeric limit on the number of suggestions I can make in a single review, but I typically prioritize the most impactful issues—like bugs, security risks, and logic errors—to keep the feedback actionable and avoid overwhelming you with "comment fatigue."

A few specific constraints and behaviors to keep in mind:

File limits: For very large pull requests, I review up to the first 150 eligible files. This ensures the review remains timely and efficient.
Incremental reviews: If you have the "Review incremental commits" setting enabled, I’ll only post comments on new issues introduced in subsequent pushes. This prevents me from repeating the same suggestions you've already seen.
Interactive follow-ups: If you feel I missed something or want a deeper dive into a specific file, you can always tag me (@cubic-dev-ai) with a specific question, like "Can you do a more thorough security pass on this file?" or "Are there any performance concerns here?"

You can adjust how I behave—including which files I ignore or which coding standards I prioritize—in your AI review settings or via a cubic.yaml file in your repository.

Helpful Links:

Add a minimal virtio-gpu device model in 2D mode, backed by a software renderer and SDL2 host window, to reach a baseline sufficient for DRM dumb buffer scanout. 3D/blob/virgl acceleration is explicitly out of scope. There are multiple ways to provide display output: 1. Implement the full virtio-gpu 3D/blob path (virgl integration), which requires a substantially larger surface area and additional host dependencies. 2. Implement virtio-gpu 2D only, backed by a software renderer and a host window system, which is enough for DRM dumb buffer style usage and basic compositing. 3. Provide a non-virtio framebuffer-like device, which would not match the intended virtio workflow and would not exercise guest virtio drivers. This commit adopts (2) to provide the smallest virtio-conformant baseline that still enables real guest drivers and guest userspace stacks. virtio-gpu (2D): - Add a core virtio-gpu device model with MMIO register handling ('virtio-gpu.c'). - Add a software backend for 2D resources and transfers ('virtio-gpu-sw.c'). - Add a host window path via SDL2 ('window-sw.c') to present the scanout surface. - Generate EDID data so the guest can probe display identification and modes. Supported GPU commands (2D path): - 'GET_DISPLAY_INFO' - 'RESOURCE_CREATE_2D', 'RESOURCE_UNREF' - 'SET_SCANOUT' - 'TRANSFER_TO_HOST_2D' - 'RESOURCE_FLUSH' - 'RESOURCE_ATTACH_BACKING', 'RESOURCE_DETACH_BACKING' - 'GET_EDID' - 'UPDATE_CURSOR', 'MOVE_CURSOR' Explicitly not implemented (3D/blob related): - 'GET_CAPSET_INFO', 'GET_CAPSET', 'RESOURCE_ASSIGN_UUID' - 'RESOURCE_CREATE_BLOB', 'SET_SCANOUT_BLOB' The device model is intentionally limited to the 2D command set needed for a basic DRM/KMS-based flow: create 2D resources, attach guest backing memory, transfer to host, flush to scanout, and optionally update/move the cursor. MMIO access rule: virtio-mmio common registers are restricted to aligned 32-bit accesses. The device-specific configuration region starts at Config (offset 0x100) and permits byte/halfword accesses as required by certain virtio devices. QEMU uses a shared virtio-mmio layer to handle sub-32-bit accesses in the device-specific configuration region and to avoid duplicating MMIO read/write boilerplate across each virtio device. In contrast, this project keeps the device model intentionally minimal, so each virtio device currently implements its own MMIO read/write handlers instead of introducing a unified MMIO framework. CI test: Currently, on Linux we use 'SDL_VIDEODRIVER=offscreen' to help validate the correctness of the vGPU functionality in CI. However, since this flag does not work properly on macOS, we apply headless mode in macOS CI test for now. With headless mode enabled, we can still verify the following: - The Virtio-GPU driver loads in the guest kernel. - The '/dev/dri/card0' DRM device node appears. - The 'virtio_gpu' driver binds correctly. - GPU commands work (create resource, attach backing, transfer data). - DirectFB/graphics libraries can initialize via DRM/KMS. - The protocol implementation is correct. References: - VirtIO Spec v1.3 (virtio-mmio, virtio-gpu) - QEMU: - hw/display/virtio-gpu.c - Linux: - drivers/virtio/virtio_ring.c - drivers/virtio/virtio_mmio.c - drivers/gpu/drm/virtio/virtgpu_drv.c - drivers/gpu/drm/virtio/virtgpu_display.c - drivers/gpu/drm/virtio/virtgpu_ioctl.c - drivers/gpu/drm/virtio/virtgpu_vq.c - drivers/gpu/drm/virtio/virtgpu_object.c - drivers/gpu/drm/virtio/virtgpu_gem.c - drivers/gpu/drm/virtio/virtgpu_prime.c - drivers/gpu/drm/virtio/virtgpu_plane.c Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

Add virtio-input device emulation to deliver keyboard and mouse input from the host SDL2 window to the guest. This enables interactive control of the guest userspace through the host window. virtio-input: - Add virtio-input emulation for keyboard and mouse ('virtio-input.c'). - Provide SDL-to-evdev event mapping ('window-events.c') and queue update logic with synchronization to avoid concurrent ring corruption. - Implement device configuration queries via 'struct virtio_input_config' and 'VIRTIO_INPUT_CFG_*' selectors. Supported configuration selectors: - 'VIRTIO_INPUT_CFG_ID_NAME' - 'VIRTIO_INPUT_CFG_ID_SERIAL' - 'VIRTIO_INPUT_CFG_ID_DEVIDS' - 'VIRTIO_INPUT_CFG_PROP_BITS' - 'VIRTIO_INPUT_CFG_EV_BITS' - 'VIRTIO_INPUT_CFG_ABS_INFO' MMIO access rule: virtio-mmio common registers are restricted to aligned 32-bit accesses. The device-specific configuration region starts at Config (offset 0x100) and permits byte/halfword accesses as required by virtio-input ('select', 'subsel', and byte-sized fields). This prevents guest drivers from faulting when performing per-byte configuration accesses, while keeping the common register model strict. Build and configuration: - Guard virtio-input code with SEMU_HAS(VIRTIOINPUT) so that the virtio-gpu device can be built independently without virtio-input. - Move 'window-events.o' to the VIRTIOINPUT build section, since it depends on virtio-input symbols. Testing notes: - Boot a guest kernel with virtio-gpu and virtio-input enabled. - Verify that the guest detects an input device (virtio-input). - Start a userspace UI stack (e.g., DirectFB2 on top of DRM/KMS) and confirm: - keyboard and mouse events are delivered to the guest userspace - More specifically, execute 'source ./run.sh', then run DirectFB2 test programs to verify. References: - VirtIO Spec v1.3 (virtio-mmio, virtio-input) - Linux: - drivers/virtio/virtio_ring.c - drivers/virtio/virtio_mmio.c Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

Update Buildroot configuration to include dependencies required for basic DRM userspace (e.g., 'libdrm') and for the host window path (SDL2). Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

Update guest kernel configuration to enable virtio-gpu and virtio-input. Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

On macOS, GUI-related code must run on the main thread. Previously, we called 'SDL_CreateThread()' inside 'window_init_sw()' in 'window-sw.c' to spawn a new thread running 'window_thread()', and 'window_thread()' then called 'SDL_CreateWindow()'. This fails on macOS because 'SDL_CreateWindow()' must be called from the main thread. In addition, the official SDL documentation [1] notes that most graphics backends are not thread-safe, and that SDL video functions should only be called from the application's main thread. This is especially important on macOS. In QEMU, the vCPU execution thread(s) and the emulator main loop thread are separated. Typically, SDL handling is placed in the emulator main loop (managed via timers and executed sequentially). However, to support different UI backends such as Cocoa, QEMU also provides an option to run the emulator main loop in a background thread, while keeping the UI event loop on the main thread. This commit follows QEMU's approach: it spawns a 'pthread' thread from 'main()' to run the emulator main loop 'semu_run', and starts the SDL-related event loop in the original main thread with a few minor changes to ensure the code runs correctly. Finally, since some environments (such as CI) may not be able to create an SDL window, this commit added a headless mode so that vGPU functionality can still be tested without a GUI using test programs such as DirectFB2. [1]: https://wiki.libsdl.org/SDL3/FAQDevelopment

The single 'render_type'/'render_pending' pair in struct 'display_info' can lose updates: if the emulator thread signals a 'PRIMARY_FLUSH' and then a CURSOR_MOVE before the main loop wakes up, the second assignment silently overwrites 'render_type', causing the flush to be dropped and the screen to stall. Replace the shared fields with two independent pending state fields: - enum primary_op: primary_pending (NONE / CLEAR / FLUSH) - enum cursor_op: cursor_pending (NONE / CLEAR / UPDATE / MOVE) The main loop now handles both planes in separate if-blocks within the same iteration, calling 'SDL_RenderPresent' once whenever either field is non-NONE. This ensures that a concurrent flush and cursor update are both applied without either being lost.

Add optional X11 support for Buildroot image building: - Add configs/x11.config with X11/Xorg related packages (mesa3d, xorg7, xserver, xterm, etc.) - Add --x11 flag to build-image.sh to enable X11 package merging - Add copy_buildroot_config function to handle config merging with X11 This allows building a Buildroot image with X11 support using: './scripts/build-image.sh --buildroot --x11' Also, the Xserver can be started by the 'startx' command. The X11 packages are optional and not required for basic virtio-gpu functionality with DirectFB2 or other non-X11 display backends.

Since the pull request has not been finalized yet, the corresponding 'Image' and 'ext4.img' have not been uploaded to the blob. Therefore, this commit is added to allow the CI tests to run. In addition, due to GitHub Actions caching behavior, we found that the 'Image' must be deleted and re-downloaded during testing for the new 'Image' to take effect. To address this, this commit adds an extra step in the script to remove the 'Image' and download it again. This commit will be removed once the code review is completed.

cubic-dev-ai bot reviewed Jan 9, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

shengwen-tw reviewed Jan 9, 2026

View reviewed changes

jserv requested changes Jan 10, 2026

View reviewed changes

shengwen-tw reviewed Jan 10, 2026

View reviewed changes

visitorckw suggested changes Jan 10, 2026

View reviewed changes

visitorckw reviewed Jan 10, 2026

View reviewed changes

Mes0903 force-pushed the vgpu-2D-rework branch from 50961c6 to 6c8ed02 Compare January 11, 2026 02:15

sysprog21 deleted a comment from cubic-dev-ai bot Jan 11, 2026

ChinYikMing reviewed Jan 11, 2026

View reviewed changes

cubic-dev-ai bot reviewed Jan 11, 2026

View reviewed changes

shengwen-tw suggested changes Jan 11, 2026

View reviewed changes

Mes0903 force-pushed the vgpu-2D-rework branch from 6c8ed02 to b30842d Compare January 28, 2026 12:37

Mes0903 force-pushed the vgpu-2D-rework branch from b30842d to 5b63aff Compare January 29, 2026 10:36

Mes0903 force-pushed the vgpu-2D-rework branch 2 times, most recently from 3a921c9 to 421f166 Compare February 4, 2026 04:57

jserv requested review from ChinYikMing, shengwen-tw and visitorckw February 4, 2026 04:57

cubic-dev-ai bot reviewed Feb 27, 2026

View reviewed changes

virtio-gpu.c Show resolved Hide resolved

virtio-input.c Show resolved Hide resolved

window-sw.c Show resolved Hide resolved

.ci/test-vgpu.sh Show resolved Hide resolved

Mes0903 force-pushed the vgpu-2D-rework branch from 08a7a10 to 7d9758f Compare February 27, 2026 15:31

cubic-dev-ai bot reviewed Feb 27, 2026

View reviewed changes

virtio-input.c Show resolved Hide resolved

window-sw.c Outdated Show resolved Hide resolved

virtio-gpu.c Outdated Show resolved Hide resolved

scripts/build-image.sh Show resolved Hide resolved

virtio-input.c Outdated Show resolved Hide resolved

virtio-gpu.c Outdated Show resolved Hide resolved

Mes0903 force-pushed the vgpu-2D-rework branch from 7d9758f to 11c8d92 Compare March 2, 2026 14:55

cubic-dev-ai bot reviewed Mar 2, 2026

View reviewed changes

Mes0903 force-pushed the vgpu-2D-rework branch from 11c8d92 to 734ae1e Compare March 2, 2026 16:41

cubic-dev-ai bot reviewed Mar 2, 2026

View reviewed changes

virtio-gpu-sw.c Outdated Show resolved Hide resolved

scripts/build-image.sh Show resolved Hide resolved

virtio-gpu.c Outdated Show resolved Hide resolved

Mes0903 force-pushed the vgpu-2D-rework branch from 734ae1e to 20432ab Compare March 2, 2026 17:04

cubic-dev-ai bot reviewed Mar 2, 2026

View reviewed changes

virtio-input.c Show resolved Hide resolved

virtio-gpu-sw.c Outdated Show resolved Hide resolved

scripts/build-image.sh Show resolved Hide resolved

.ci/test-vgpu.sh Show resolved Hide resolved

Mes0903 and others added 8 commits March 3, 2026 20:07

Add buildroot config

d4b318a

Update Buildroot configuration to include dependencies required for basic DRM userspace (e.g., 'libdrm') and for the host window path (SDL2). Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

Add linux configs

7de0866

Update guest kernel configuration to enable virtio-gpu and virtio-input. Co-authored-by: Shengwen Cheng <shengwen1997.tw@gmail.com>

Mes0903 force-pushed the vgpu-2D-rework branch from 20432ab to f5478f4 Compare March 3, 2026 12:15

		@@ -0,0 +1,4 @@
		#!/usr/bin/bash

		export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib

	export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib
	export LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}/usr/local/lib

	export PATH=$PATH:$PWD/buildroot/output/host/bin
	export PATH="$PWD/buildroot/output/host/bin:$PATH"

Conversation

Mes0903 commented Jan 9, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

VirtIO-GPU

VirtIO-Input

Prerequisites

Build kernel image and test rootfs image

Run X11

Run DirectFB2

Run kmscube

Run modetest

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

shengwen-tw Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shengwen-tw Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

visitorckw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shengwen-tw commented Jan 10, 2026

Uh oh!

ChinYikMing commented Jan 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mes0903 commented Jan 9, 2026 •

edited by cubic-dev-ai bot

Loading

shengwen-tw Jan 9, 2026 •

edited

Loading

shengwen-tw Jan 11, 2026 •

edited

Loading

cubic-dev-ai bot Jan 11, 2026 •

edited

Loading

cubic-dev-ai bot Jan 11, 2026 •

edited

Loading

cubic-dev-ai bot Jan 11, 2026 •

edited

Loading

cubic-dev-ai bot Jan 11, 2026 •

edited

Loading

cubic-dev-ai bot Jan 11, 2026 •

edited

Loading

shengwen-tw Jan 11, 2026 •

edited

Loading