Two parallel review agents cross-checked the virtio-inputd driver against
the Linux 7.1 reference (drivers/virtio/virtio_input.c, virtio_input.h)
and the proven redox-drm virtio transport. 12 issues were found across
BLOCKER, MAJOR, MINOR, and NIT severity, all fixed in this commit before
runtime testing.
BLOCKERs (would have prevented the driver from working in QEMU):
1. fill_avail() never wrote avail_idx after pushing the 64 ring
entries. The device reads avail_ring[avail_idx % size] to discover
new buffers, so without publishing avail_idx = size, the device
saw avail_idx = 0 and ignored all initial buffers. Fix: explicit
'fence(Release); write_avail_idx(self.size)' with a spec citation.
2. drain() recycled IDs derived from 'last_used_idx - drained_count',
which is wrong when the used ring wraps and a single drain cycle
spans more than one full ring revolution. Fix: collect the actual
drained 'id' values in a stack '[u16; 64]' array during the drain
loop, then push those exact ids back to the avail ring. The
doc-comment explains why the derivation is unsafe.
3. config_read_string() and config_read_bitmap() used
'self.device_cfg.size()' (the MMIO region size = 40) instead of
the device-reported config size from offset 2. Fix: use
'config_read_size()' to read the actual size field.
4. config/redbear-full.toml: device_id_range was a TOML string
('0x1042..=0x107F') but serde's Range<u16> deserializes from a
sequence, not a string. pcid-spawner would have silently failed
to load the fragment. Fix: use serde array form
'device_id_range = [0x1042, 0x107F]'.
MAJORs (silent failure modes or runtime bugs):
5. activate_queue() had no fence between address writes and
'queue_enable = 1'. A CPU write buffer may reorder writes to
distinct MMIO addresses. Fix: explicit 'fence(SeqCst)' with
a comment citing virtio spec 2.8 and Linux's virtio_wmb.
6. No 'reset_device()' on error path after partial init. The device
would be left in ACKNOWLEDGE|DRIVER with no driver active,
requiring a guest reboot to recover. Fix: wrap init in a closure;
any error calls 'transport.reset_device()' before propagating.
7. Drain loop never checked DEVICE_NEEDS_RESET or DEVICE_STATUS_FAILED.
If the device entered an unrecoverable state, the driver would
poll forever with stale state. Fix: 'device_in_error_state()' on
the transport; loop checks it each iteration and exits cleanly.
8. The abs_count probe used 'config_read_size() == 24', which was
always false (virtio_input_absinfo is 20 bytes, not 24). The
count was always logged as 0. Fix: '>= 20' per spec.
MINORs / NITs (hardening, no functional impact):
9. config_read_absinfo() returned AbsInfo without validating
device-reported size. Now returns Option<AbsInfo> and validates
size >= 20.
10. map_cap_region() missing bounds check: capability range may
extend past BAR end (QEMU is permissive; bare-metal is not).
Added 'cap_end > bar_size' check with spec reference.
11. Legacy device 0x1052 entry in pcid fragment caused spurious
spawn + log noise. Removed.
12. notify_queue() error silently dropped with .ok(). Now logs warn
and continues.
Plan update:
The CONSOLE-TO-KDE-DESKTOP-PLAN.md v5.1 changelog now has a new
'9.1.1 Phase 5.1 review-driven fixes' section documenting all 12
findings with file:line, severity, and fix. Future maintainers can
trace the BLOCKERs back to specific commits to understand the
critical-path safety net this review provided.
Verification: cargo check zero errors, 64 warnings (all unused
keycode constants reserved for Phase 5.2 expansion). The driver
is now ready for runtime testing in QEMU.
Add a real, QEMU-targeted virtio-input driver as a new Red Bear recipe at
local/recipes/drivers/virtio-inputd/. The driver handles virtio-input-host-pci,
virtio-input-keyboard, virtio-input-mouse, and virtio-input-tablet devices and
closes Gap #19 of the v5.0 desktop plan.
The driver:
* Walks the PCI capability list to find the modern virtio 1.0 capability
block (common_cfg, notify_cfg, isr_cfg, device_cfg) using MmioRegion
mappings via redox-driver-sys. Rejects legacy virtio-input (device 0x1052)
which lacks the modern transport.
* Negotiates VIRTIO_F_VERSION_1 only (the only required feature).
* Allocates one event virtqueue (size up to 64) backed by four DMA buffers
(desc, avail, used, event_buffers) and pre-fills the avail ring.
* Polls the used ring at 60 Hz, drains completed events, decodes each
virtio_input_event (8-byte type/code/value), and recycles drained buffers
back to the avail ring.
* Translates events to orbclient format and pushes them to inputd via
ProducerHandle (Orbital path):
- EV_KEY -> KeyEvent (with US-QWERTY character mapping)
- EV_REL -> MouseRelativeEvent (REL_X/REL_Y) or ScrollEvent (REL_WHEEL)
- EV_SYN -> dropped (inputd multiplexes)
- Other -> dropped (Phase 5.2 will add evdevd path)
Probe-time checks:
* Vendor 0x1AF4, device_id >= 0x1042, revision >= 1
* Caps include a device_cfg block with virtio type == 18 (virtio_input)
Configuration: a pcid-spawner fragment is added to config/redbear-full.toml
under /etc/pcid.d/virtio-inputd.toml matching class=0x09 vendor=0x1AF4 with
device_id_range 0x1042..=0x107F (and a separate 0x1052 entry that the driver
intentionally rejects).
Verification: cargo check produces 0 errors and 65 warnings, all of which are
unused input-event-codes.h constants reserved for the Phase 5.2 expansion.
Linking the binary requires the Redox cross-toolchain (relibc provides
redox_sys_call_v0); this is provided by the build system, not the host
toolchain.
Plan: this is Phase 5.1 of CONSOLE-TO-KDE-DESKTOP-PLAN.md v5.0. The plan is
updated to v5.1 with: (a) a 'What Changed Since v5.0' section, (b) Gap #19
marked DONE, (c) Phase 5 row marked DONE with sub-task status, (d) Gate E
updated, (e) Input pipeline section updated to reflect the (c) path is now
implemented. Phase 5.2 (evdevd producer path + virtio-snd) is documented as
the next planned work but not yet implemented.
vasilito