Skip to content

Fix host USB HID boot, mode, and recovery bugs#814

Open
mjc wants to merge 7 commits into
sipeed:mainfrom
mjc:mjc/fix-usb-hid-boot-subclass
Open

Fix host USB HID boot, mode, and recovery bugs#814
mjc wants to merge 7 commits into
sipeed:mainfrom
mjc:mjc/fix-usb-hid-boot-subclass

Conversation

@mjc

@mjc mjc commented Jun 5, 2026

Copy link
Copy Markdown

Summary

This PR fixes NanoKVM host HID failures by making the USB gadget definition correct and consistent, then hardening the server-side HID writer against gadget resets.

The failures were host-visible USB bugs, not just a web UI toggle problem. On affected machines, BIOS/UEFI and boot menus could ignore keyboard or mouse input, Linux could partially bind or reject HID interfaces, usbhid/hid-generic could fail probing, and host-side consumers such as udev could churn around unstable or malformed gadget identifiers. NanoKVM could also keep stale /dev/hidg* file handles after the gadget was rebuilt.

The branch keeps the scope to HID and host-visible USB gadget correctness. It does not change virtual media behavior and does not change the UI.

中文说明

这个 PR 修复 NanoKVM 在 BIOS/UEFI 和启动菜单阶段 HID 键盘、鼠标不能被主机稳定识别的问题。核心修复是统一 normal 和 HID-only 两种模式的 USB gadget 配置,并按 USB HID 规范暴露 boot keyboard、boot mouse 和 non-boot absolute/touch mouse。

同时,这个 PR 修复了 gadget 重建后服务端继续使用旧 /dev/hidg* 句柄的问题,并让 USB 字符串 descriptor 写入精确内容,避免 shell 换行影响主机侧识别。范围仅限 HID 和主机可见的 USB gadget 正确性,不包含虚拟媒体和 UI 修改。

What changed

Area Before Now
Keyboard HID Could be exposed through drifted normal/HID-only setup paths Always boot HID keyboard: subclass 1, protocol 1
Relative mouse HID Could be non-boot or differ by mode Always boot HID mouse: subclass 1, protocol 2
Absolute/touch HID Could be treated like a boot device in some paths Always non-boot HID mouse: subclass 0, protocol 2
Normal vs HID-only Separate scripts duplicated large parts of gadget setup and could drift Both modes use the same HID setup helper and descriptors
USB strings Shell writes could include newline-terminated descriptor contents String descriptors are written as exact strings
Report descriptors Descriptor byte strings were duplicated across scripts Descriptor byte strings are centralized and tested
Gadget rebuilds Stale configfs links could survive rebuilds, including OS descriptor links Rebuild removes stale config and os_desc/c.1 links before recreating the gadget
Wake-on-write Related PRs proposed changing the default policy Existing policy is preserved: enabled when available unless /boot/usb.notwakeup exists
HID writes Server could keep writing to deleted/recreated /dev/hidg* handles Stale handles are detected, closed, reopened, and retried once
HID mode/status Status checks used brittle wildcard paths or shell command strings Checks use concrete configfs paths and direct command execution
OLED USB state HID presence used a wildcard path with access() HID presence checks the actual configfs function links

Scope

Included here:

  • Correct boot HID descriptors for keyboard and relative mouse.
  • Keep absolute/touch HID as non-boot.
  • Make normal mode and HID-only mode produce the same HID function definitions.
  • Preserve Rev2.2 composite USB behavior, including RNDIS/NCM and Microsoft OS descriptor setup.
  • Preserve the existing wake-on-write default and only honor /boot/usb.notwakeup as the opt-out.
  • Recover HID writes after gadget rebuilds or USB resets.
  • Add regression coverage for the USB init scripts and Go HID behavior.

Explicitly not included here:

  • Virtual media behavior, no-media semantics, or /dev/mmcblk0p3 cleanup.
  • UI wording or layout changes.
  • Descriptor customization UI/API work.
  • A no-reboot HID mode switch redesign.

Related issues and PRs

Most directly related:

May overlap but may have additional causes:

Related PRs:

Validation

Automated validation run on this branch:

  • USB init script regression suite: tests/usb-init-scripts-test.sh
  • Shell script linting for the USB scripts and test suite
  • Go HID/service tests, including all server packages
  • kvm_system support binary build
  • git diff --check

Hardware validation:

  • Deployed the scripts and server changes to a NanoKVM device.
  • Verified normal mode enumerates the expected composite gadget: RNDIS/NCM, keyboard HID, two mouse HID interfaces, and mass storage.
  • Verified HID-only mode exposes HID functions without non-HID functions.
  • Verified the host sees the keyboard as boot HID keyboard and the relative mouse as boot HID mouse.
  • Verified BIOS/UEFI keyboard and mouse input on an ASRock X570 Steel Legend host.
  • Verified Linux host logs no longer show the reproduced usbhid probe failure for the fixed gadget definition.

Copilot AI review requested due to automatic review settings June 5, 2026 22:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors USB gadget init scripts to share common logic, adds regression tests for gadget configuration, and improves HID/USB status handling in both the Go server and support firmware.

Changes:

  • Introduce S03usb-common and refactor S03usbdev / S03usbhid to use shared gadget setup + action routing.
  • Add a shell-based regression test suite for init scripts and a make test-usb-scripts target.
  • Improve HID mode/state detection and robustness (Go HID stale-handle recovery; C++ HID symlink checks).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/usb-init-scripts-test.sh Adds regression coverage for gadget configuration and actions in test mode.
kvmapp/system/init.d/S03usb-common New shared USB gadget script library used by init scripts.
kvmapp/system/init.d/S03usbdev Refactors normal mode init to use shared common functions.
kvmapp/system/init.d/S03usbhid Refactors HID-only init to use shared common functions.
server/service/hid/status.go Makes mode flag testable and adds timeout-bounded PHY reset execution.
server/service/hid/status_test.go Adds tests for HID mode detection via bcdDevice.
server/service/hid/hid.go Adds stale HID handle detection/reopen & error factoring.
server/service/hid/hid_test.go Adds unit tests for stale HID error classification and deleted-fd detection.
support/sg2002/kvm_system/main/lib/system_state/system_state.cpp Fixes HID presence detection (no wildcard access) via explicit paths.
Makefile Adds test-usb-scripts target to run the new regression suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/service/hid/status.go
Comment thread kvmapp/system/init.d/S03usb-common
Comment thread kvmapp/system/init.d/S03usb-common
Comment thread kvmapp/system/init.d/S03usb-common Outdated
Comment thread kvmapp/system/init.d/S03usb-common
Comment thread support/sg2002/kvm_system/main/lib/system_state/system_state.cpp
Comment thread server/service/hid/hid.go
@mjc mjc force-pushed the mjc/fix-usb-hid-boot-subclass branch 2 times, most recently from a65a15b to 3d3c5e0 Compare June 7, 2026 19:04
@mjc mjc force-pushed the mjc/fix-usb-hid-boot-subclass branch from 3d3c5e0 to 04e0aa5 Compare June 11, 2026 18:48
@mjc

mjc commented Jun 11, 2026

Copy link
Copy Markdown
Author

Rebased this branch onto current upstream main (2.4.3).

The only conflict was in S03usbdev, where upstream added the Rev2.2 composite/NCM/RNDIS descriptor setup. I resolved that by keeping this PR's shared HID gadget setup and carrying forward the upstream USB networking/OS descriptor writes, so the branch still fixes the HID boot/protocol handling without dropping the newer composite gadget behavior.

After the rebase I re-ran:

sh tests/usb-init-scripts-test.sh
git diff --check origin/main..HEAD

Both passed.

@mjc mjc marked this pull request as draft June 11, 2026 19:07
@mjc mjc marked this pull request as ready for review June 11, 2026 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants