fix(native-usb): prevent transmit-bank starvation and retain battery telemetry

This commit is contained in:
Joey Yakimowich-Payne 2026-09-13 21:00:17 -06:00
commit a699a6920b
16 changed files with 342 additions and 82 deletions

View file

@ -17,6 +17,9 @@ bool native_test_setup(uint8_t, const tusb_control_request_t*, bool);
bool native_test_out(uint8_t, const uint8_t*, uint16_t, bool);
bool native_test_in(uint8_t, uint8_t*, uint16_t*, bool);
void native_test_bus_reset(bool);
void native_test_hold_abort(bool);
bool native_test_select(uint8_t);
bool native_test_private_in(uint8_t, uint8_t, uint8_t*, uint16_t*);
}
namespace {
@ -26,6 +29,7 @@ uint32_t programs = 0;
uint32_t erases = 0;
uint32_t bootsel_calls = 0;
std::array<uint8_t, 64> child_identity[2];
bool interleave_identity_ack = false;
void require(bool condition, const char* message) {
if (!condition) { std::cerr << message << '\n'; std::exit(1); }
@ -155,9 +159,9 @@ void test_profile_transport() {
const auto original = encoded_profile(0);
const auto edited = encoded_profile(4);
auto info = read_operation(Operation::kInfo);
require(info[kResponseHeaderSize] == 0 && info[kResponseHeaderSize + 1] == 72 &&
info[kResponseHeaderSize + 2] == 0 && info[kResponseHeaderSize + 4] == 5 &&
info[kResponseHeaderSize + 5] == 7, "native INFO does not describe the fixed image");
require(info.size() == kResponseHeaderSize + 8 &&
info[kResponseHeaderSize + 4] == 5 && info[kResponseHeaderSize + 5] == 7,
"native INFO does not describe the fixed output and its capabilities");
auto list = read_operation(Operation::kProfileList);
require(list[kResponseHeaderSize] == 1 && list.size() > 64, "root catalog omitted the global profile owner");
auto playtest = read_operation(Operation::kProfilePlaytest);
@ -261,6 +265,74 @@ void test_interrupted_transactions() {
require(!native_test_in(0, packet, &length, true), "completed request retained a second status ACK");
}
void test_pending_control_buffer_ownership() {
const auto info = request(Operation::kInfo, true, kMaximumResponseSize);
require(native_test_setup(0, &info, true), "pending INFO setup failed");
const unsigned programs_before = programs, erases_before = erases;
native_test_hold_abort(true);
const auto replacement = request(Operation::kProfileSelect, false, kRequestHeaderSize + 15);
require(!native_test_setup(0, &replacement, false),
"new SETUP replaced an EP0 buffer before the controller released ownership");
uint8_t packet[64]; uint16_t length;
require(!native_test_in(0, packet, &length, false),
"an unquiesced control endpoint acknowledged replacement work");
profile_service_task_on_storage_core(4000);
require(programs == programs_before && erases == erases_before,
"unquiesced replacement changed saved profiles");
native_test_initialize();
}
void test_read_ack_allows_usb_progress() {
interleave_identity_ack = true;
read_child(1);
const auto next = receive(2);
require(next == std::vector<uint8_t>(child_identity[1].begin(), child_identity[1].end()),
"SETUP received during read ACK did not retain the next child's response");
}
void test_private_transmit_survives_round_robin_tokens() {
native_test_initialize();
tusb_control_request_t configuration{};
configuration.bRequest = TUSB_REQ_SET_CONFIGURATION;
configuration.wValue = 1;
for (uint8_t slot : {1, 2}) {
require(native_test_setup(slot, &configuration, true), "child configuration failed");
acknowledge(slot);
}
const uint8_t payloads[2][3] = {{0x11, 0x22, 0x33}, {0x44, 0x55, 0x66}};
for (uint8_t instance : {0, 1}) {
require(native_hub_hid_report(instance, instance ? 7 : 8, payloads[instance], 3),
"could not queue HID packet");
require(native_hub_vendor_write(instance, payloads[instance], 3) == 3 &&
native_hub_vendor_write_flush(instance) == 3, "could not queue bulk packet");
}
uint8_t packet[64];
uint16_t length = 0;
for (uint8_t endpoint : {0x81, 0x82}) {
for (uint8_t slot : {1, 2}) {
require(native_test_private_in(slot, endpoint, packet, &length),
"queued private IN packet required foreground work after bank selection");
const unsigned prefix = endpoint == 0x81 ? 1 : 0;
require(length == 3 + prefix &&
(!prefix || packet[0] == (slot == 1 ? 8 : 7)) &&
std::memcmp(packet + prefix, payloads[slot - 1], 3) == 0,
"round-robin IN token received another endpoint's payload");
}
}
native_test_drain();
require(native_hub_hid_ready(0) && native_hub_hid_ready(1),
"acknowledged HID packets did not release their queues");
require(!native_test_private_in(1, 0x81, packet, &length),
"acknowledged HID packet was retransmitted");
// The idle poll selected R without restoring its shared EP0 image.
require(native_hub_hid_report(0, 8, payloads[0], 3), "could not queue the next HID packet");
require(native_test_private_in(1, 0x81, packet, &length) && length == 4 &&
std::memcmp(packet + 1, payloads[0], 3) == 0,
"pending shared EP0 restoration blocked a newly queued private IN packet");
native_test_drain();
native_test_initialize();
}
void test_private_bootsel() {
const auto bytes = envelope(Operation::kBootselReboot, {});
const auto setup = request(Operation::kBootselReboot, false, bytes.size());
@ -336,6 +408,14 @@ extern "C" bool tud_vendor_control_xfer_cb(uint8_t slot, uint8_t stage, const tu
if (probe_management_vendor_control(slot, stage, setup)) return true;
if (slot < 1 || slot > 2 || setup->bmRequestType != 0xc0 ||
setup->bRequest != 3 || setup->wValue || setup->wIndex) return false;
if (stage == CONTROL_STAGE_ACK && slot == 1 && interleave_identity_ack) {
interleave_identity_ack = false;
const auto next = *setup;
require(native_test_setup(2, &next, false), "next child's SETUP was rejected during read ACK");
// SETUP must be serviced before another token can change the bank.
// This observes IRQ progress, rather than inspecting the CPU mask.
require(native_test_select(0), "read ACK callback blocked servicing the next USB SETUP");
}
return stage != CONTROL_STAGE_SETUP || native_hub_control_xfer(slot, setup,
child_identity[slot - 1].data(), child_identity[slot - 1].size());
}
@ -347,6 +427,9 @@ int main() {
native_test_initialize();
test_profile_transport();
test_interrupted_transactions();
test_pending_control_buffer_ownership();
test_read_ack_allows_usb_progress();
test_private_transmit_survives_round_robin_tokens();
test_private_bootsel();
std::cout << "native root management packet and persistence regressions passed\n";
}

View file

@ -5,14 +5,28 @@
#include <string.h>
#define __not_in_flash_func(name) name
#define __no_inline_not_in_flash_func(name) __attribute__((noinline)) name
#define __force_inline inline __attribute__((always_inline))
#define __dmb() ((void)0)
typedef struct { unsigned unused; } spin_lock_t;
static inline uint32_t save_and_disable_interrupts(void) { return 0; }
static inline void restore_interrupts(uint32_t flags) { (void)flags; }
static inline uint32_t spin_lock_blocking(spin_lock_t* lock) { (void)lock; return 0; }
static inline void spin_unlock(spin_lock_t* lock, uint32_t flags) { (void)lock; (void)flags; }
extern uint32_t native_test_interrupt_mask;
void native_test_service_interrupt(void);
static inline uint32_t save_and_disable_interrupts(void) {
uint32_t flags = native_test_interrupt_mask;
native_test_interrupt_mask = 1;
return flags;
}
static inline void restore_interrupts(uint32_t flags) {
native_test_interrupt_mask = flags;
native_test_service_interrupt();
}
static inline uint32_t spin_lock_blocking(spin_lock_t* lock) {
(void)lock; return save_and_disable_interrupts();
}
static inline void spin_unlock(spin_lock_t* lock, uint32_t flags) {
(void)lock; restore_interrupts(flags);
}
static inline bool spin_try_lock_unsafe(spin_lock_t* lock) { (void)lock; return true; }
static inline void spin_unlock_unsafe(spin_lock_t* lock) { (void)lock; }
static inline int spin_lock_claim_unused(bool required) { (void)required; return 0; }
@ -20,13 +34,13 @@ static inline spin_lock_t* spin_lock_instance(unsigned index) {
static spin_lock_t lock; (void)index; return &lock;
}
static inline void hw_clear_bits(volatile uint32_t* address, uint32_t bits) { *address &= ~bits; }
static inline void hw_set_bits(volatile uint32_t* address, uint32_t bits) { *address |= bits; }
typedef struct {
volatile uint32_t ints, sie_status, buf_status, dev_addr_ctrl, inte;
volatile uint32_t ep_stall_arm, muxing, phy_direct, phy_direct_override;
volatile uint32_t pwr, main_ctrl, sie_ctrl, ep_nak_stall_status;
volatile uint32_t ep_tx_error, ep_rx_error;
volatile uint32_t abort, abort_done;
} usb_hw_t;
typedef struct { volatile uint32_t in, out; } usb_pair_t;
typedef struct {
@ -46,6 +60,13 @@ extern sio_hw_t native_test_sio;
#define USBCTRL_DPRAM_BASE ((uintptr_t)usb_dpram)
#define USB_DPRAM_SIZE sizeof(*usb_dpram)
extern bool native_test_abort_stuck;
static inline void hw_set_bits(volatile uint32_t* address, uint32_t bits) {
*address |= bits;
if (address == &usb_hw->abort && !native_test_abort_stuck)
usb_hw->abort_done |= bits;
}
#define USB_BUF_CTRL_LEN_MASK 0x3ffu
#define USB_BUF_CTRL_AVAIL (1u << 10)
#define USB_BUF_CTRL_STALL (1u << 11)

View file

@ -4,6 +4,17 @@
usb_hw_t native_test_usb;
usb_device_dpram_t native_test_dpram;
sio_hw_t native_test_sio;
bool native_test_abort_stuck;
uint32_t native_test_interrupt_mask;
static bool servicing_interrupt;
void native_test_service_interrupt(void) {
if (native_test_interrupt_mask || servicing_interrupt || !usb_hw->ints) return;
servicing_interrupt = true;
usb_interrupt();
usb_hw->ints = 0;
servicing_interrupt = false;
}
void probe_router_init(uint32_t hz) { (void)hz; }
void probe_router_core1(void) {}
@ -36,6 +47,9 @@ void native_test_initialize(void) {
memset(usb_hw,0,sizeof(*usb_hw));
memset(usb_dpram,0,sizeof(*usb_dpram));
event_head = event_tail = 0;
native_test_abort_stuck = false;
native_test_interrupt_mask = 0;
servicing_interrupt = false;
failed = bus_suspended = bank_restore_pending = false;
bank_lock = spin_lock_instance(0);
active_device = default_device = 0;
@ -49,6 +63,8 @@ static bool select_slot(uint8_t slot) {
return true;
}
bool native_test_select(uint8_t slot) { return select_slot(slot); }
void native_test_drain(void) { native_hub_task(); }
bool native_test_setup(uint8_t slot, const tusb_control_request_t* request, bool drain) {
@ -56,14 +72,16 @@ bool native_test_setup(uint8_t slot, const tusb_control_request_t* request, bool
memcpy(usb_dpram->setup_packet,request,sizeof(*request));
usb_hw->sie_status = USB_SIE_STATUS_SETUP_REC_BITS;
usb_hw->ints = USB_INTS_SETUP_REQ_BITS;
usb_interrupt();
usb_hw->ints = 0;
native_test_service_interrupt();
if (drain) native_hub_task();
return !failed && devices[slot].control.stage != STALLED;
}
void native_test_hold_abort(bool hold) { native_test_abort_stuck = hold; }
bool native_test_out(uint8_t slot, const uint8_t* data, uint16_t length, bool drain) {
if (!select_slot(slot)) return false;
if (usb_hw->abort & 2u) return false;
uint32_t value = buffer_regs()[1];
if (!(value & USB_BUF_CTRL_AVAIL) || (value & USB_BUF_CTRL_STALL) ||
length > (value & USB_BUF_CTRL_LEN_MASK)) return false;
@ -71,14 +89,14 @@ bool native_test_out(uint8_t slot, const uint8_t* data, uint16_t length, bool dr
buffer_regs()[1] = (value & ~(USB_BUF_CTRL_AVAIL | USB_BUF_CTRL_LEN_MASK)) | length;
usb_hw->buf_status = 2;
usb_hw->ints = USB_INTS_BUFF_STATUS_BITS;
usb_interrupt();
usb_hw->ints = 0;
native_test_service_interrupt();
if (drain) native_hub_task();
return !failed && devices[slot].control.stage != STALLED;
}
bool native_test_in(uint8_t slot, uint8_t* data, uint16_t* length, bool drain) {
if (!select_slot(slot)) return false;
if (usb_hw->abort & 1u) return false;
uint32_t value = buffer_regs()[0];
if (!(value & USB_BUF_CTRL_AVAIL) || !(value & USB_BUF_CTRL_FULL) ||
(value & USB_BUF_CTRL_STALL)) return false;
@ -87,17 +105,35 @@ bool native_test_in(uint8_t slot, uint8_t* data, uint16_t* length, bool drain) {
buffer_regs()[0] = value & ~USB_BUF_CTRL_AVAIL;
usb_hw->buf_status = 1;
usb_hw->ints = USB_INTS_BUFF_STATUS_BITS;
usb_interrupt();
usb_hw->ints = 0;
native_test_service_interrupt();
if (drain) native_hub_task();
return !failed && devices[slot].control.stage != STALLED;
}
bool native_test_private_in(uint8_t slot, uint8_t endpoint, uint8_t* data, uint16_t* length) {
if (slot < 1 || slot > 2 || (endpoint != 0x81 && endpoint != 0x82)) return false;
// A host token selects the bank, but cannot wait for a foreground task.
if (!native_hub_select_device(addresses[slot],slot,UINT32_MAX / 2)) return false;
unsigned channel = (endpoint & 15u) * 2u;
uint32_t control = endpoint_regs()[channel - 2u];
uint32_t value = buffer_regs()[channel];
if (!(control & EP_CTRL_ENABLE_BITS) || !(value & USB_BUF_CTRL_AVAIL) ||
!(value & USB_BUF_CTRL_FULL) || (value & USB_BUF_CTRL_STALL)) return false;
*length = value & USB_BUF_CTRL_LEN_MASK;
if (*length > PACKET) return false;
if (*length) copy_from_usb(data,
(const volatile uint8_t*)USBCTRL_DPRAM_BASE + (control & 0xffffu), *length);
buffer_regs()[channel] = value & ~USB_BUF_CTRL_AVAIL;
usb_hw->buf_status |= 1u << channel;
usb_hw->ints |= USB_INTS_BUFF_STATUS_BITS;
native_test_service_interrupt();
return !failed;
}
void native_test_bus_reset(bool drain) {
usb_hw->sie_status = USB_SIE_STATUS_BUS_RESET_BITS;
usb_hw->ints = USB_INTS_BUS_RESET_BITS;
usb_interrupt();
usb_hw->ints = 0;
native_test_service_interrupt();
if (drain) native_hub_task();
// Assign fixture addresses after reset, independently of EP0 state.
addresses[0] = 0; addresses[1] = 1; addresses[2] = 2;

View file

@ -121,7 +121,7 @@ void mapped_halves_and_calibration() {
source.controller.active = true;
source.controller.connection_generation = 7;
source.controller.identity = controller_identity_global();
source.battery = 255;
source.battery = 128;
publish();
// Neither an absent source nor an uncalibrated child masquerades as active.
assert(!peek(0) && !controls[0].active);
@ -132,7 +132,7 @@ void mapped_halves_and_calibration() {
pair();
assert(stick_x(0) == 2000 && stick_y(0) == 2100);
assert(stick_x(1) == 1800 && stick_y(1) == 1900);
assert(reports[0][1] == 0x25 && reports[1][1] == 0x25); // Known full level, USB powered, not charging.
assert(reports[0][1] == 0x15 && reports[1][1] == 0x15); // Measured half battery, USB powered, not charging.
// Actual profile transforms can move controls across native children.
profile.button_map[static_cast<unsigned>(ControllerProfileLogicalButton::kSouth)] =
static_cast<uint8_t>(ControllerProfileLogicalButton::kDpadRight);
@ -364,7 +364,7 @@ void nunchuk_buttons_map_to_native_left_shoulders() {
++source.controller.connection_generation;
source.controller.state = {};
source.accel_valid = source.gyro_valid = false;
source.battery = 0; // Unknown source battery must not invent a full charge.
source.battery = 0;
profile = controller_profile_default(controller_identity_global(), 0);
// The real Wii parser maps Nunchuk C to west and Z to north. These are
// ordinary profile inputs, not the unrelated Switch2 extra "C" control.
@ -375,7 +375,7 @@ void nunchuk_buttons_map_to_native_left_shoulders() {
source.controller.state.button_west = true; // C -> ZL.
publish(false); pair();
assert(reports[0][2] == 0 && reports[1][2] == 0x20);
assert(reports[0][1] == 0x01 && reports[1][1] == 0x01); // USB power remains real even without battery telemetry.
assert(reports[0][1] == 0x01 && reports[1][1] == 0x01);
source.controller.state.button_west = false;
source.controller.state.button_north = true; // Z -> L.
publish(false); pair();

View file

@ -143,9 +143,10 @@ int main() {
source.accel_valid = source.gyro_valid = true;
source.accel_q13[1] = 8192; // SDL up -> virtual native rail-down +X.
memcpy(source.gyro_q10, bias_q10, sizeof(bias_q10));
source.battery = 128;
assert(poll());
assert(controls.active && packet[15] == 30); // Wii IMU starts before background bias learning.
assert(packet[1] == 0x01); // USB power, no fabricated charge level or charging state.
assert(packet[1] == 0x15); // Measured half battery, USB powered, not charging.
for (unsigned i = 1; i < 450; ++i) assert(poll());
assert(controls.active && packet[15] == 30 && packet[19] == 0x0c);
assert(signed32(packet+32) == (1 << 28));

View file

@ -2801,6 +2801,8 @@ def test_profile_playtest_decodes_raw_controller_state() -> None:
"dpad_up",
"dpad_right",
]
assert replace(playtest, battery=255).to_json_object()["battery"] == 100
assert replace(playtest, battery=1).to_json_object()["battery"] == 0
device.playtest_connected = False
disconnected = config_manager.read_profile_playtest(device)
@ -2818,6 +2820,7 @@ def test_profile_playtest_decodes_raw_controller_state() -> None:
capabilities=0,
motion=None,
)
assert disconnected.to_json_object()["battery"] is None
device.playtest_connected = True
payload, flags = device._profile_playtest_payload()
malformed = bytearray(payload)

View file

@ -1232,6 +1232,33 @@ static void nunchuk_accel_detach_and_replacement_require_fresh_calibration(void)
}
}
static void battery_status_survives_input_reports(void) {
reset_fixture(0x0306, false, EXT_NONE);
assert(f.device.controller.battery == UNI_CONTROLLER_BATTERY_NOT_AVAILABLE);
uni_hid_parser_wii_setup(&f.device);
finish_setup();
assert(f.device.controller.battery == UNI_CONTROLLER_BATTERY_FULL);
send_core_and_accel();
assert(f.device.controller.battery == UNI_CONTROLLER_BATTERY_FULL);
uint8_t status[] = {0x20, 0, 0, 0, 0, 0, 52};
feed(status, sizeof(status));
finish_setup();
assert(f.device.controller.battery == 179); // Measured 70% band, not a full-charge placeholder.
send_core_and_accel();
assert(f.device.controller.battery == 179);
status[6] = 0;
feed(status, sizeof(status) - 1);
assert(f.device.controller.battery == 179); // Truncation cannot erase the last measurement.
feed(status, sizeof(status));
finish_setup();
assert(f.device.controller.battery == 13); // Measured low band remains distinct from unknown.
send_core_and_accel();
assert(f.device.controller.battery == 13);
uni_hid_parser_wii_setup(&f.device);
assert(f.device.controller.battery == UNI_CONTROLLER_BATTERY_NOT_AVAILABLE);
}
static void run_case(const char* name, void (*test)(void)) {
printf("Wii parser: %s\n", name);
fflush(stdout);
@ -1239,6 +1266,7 @@ static void run_case(const char* name, void (*test)(void)) {
}
int main(void) {
run_case("real battery retained between status and input reports", battery_status_survives_input_reports);
run_case("fresh calibrated accelerometer snapshots without MotionPlus", accelerometer_snapshot_requires_fresh_calibrated_reports);
run_case("independent fresh calibrated MotionPlus gyro snapshots", gyro_snapshot_advances_only_on_calibrated_motionplus_packets);
run_case("gyro validity through hotplug, replacement and teardown", gyro_snapshot_invalidates_on_topology_and_teardown);