From 27bf91cfe0f1b46c660cb23c18663ba56e3af31c Mon Sep 17 00:00:00 2001 From: Nexoniarz Date: Thu, 22 Oct 2026 23:47:17 +0200 Subject: [PATCH] usb: recover endpoint 0 after a failed request; report-protocol keyboards From the first boot on real hardware: a Razer mouse's control transfer failed with a babble, and an ITE controller's late endpoint-0 completion was reported as an "unexpected event" during a command. A failed control transfer left endpoint 0 halted (or, on a timeout, still running with TRBs queued), so every request after it to that device failed too. It is now reset or stopped and its dequeue pointer moved past what was left on the ring. A STALL -- a device declining a request it does not implement -- is logged only on a verbose boot. HID interfaces without the boot subclass were turned away unless they had a pointer. The report descriptor's keyboard page is now read too -- modifier bits, a key array, an NKRO bitmap -- and each report rewritten into the boot layout for the existing path. Checked with QEMU's usb-kbd forced into report protocol: "echo Hi", Shift included, typed and ran. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01S1giNvrHTrPMPw7V6x1kQ4 --- kernel/drivers/bus/xhci/xhci.c | 43 +++++++++- kernel/drivers/bus/xhci/xhci.h | 4 + kernel/drivers/usb/hid/usb_hid.c | 141 +++++++++++++++++++++++++++++-- 3 files changed, 177 insertions(+), 11 deletions(-) diff --git a/kernel/drivers/bus/xhci/xhci.c b/kernel/drivers/bus/xhci/xhci.c index f3b905d..b9096be 100644 --- a/kernel/drivers/bus/xhci/xhci.c +++ b/kernel/drivers/bus/xhci/xhci.c @@ -313,11 +313,18 @@ event_wait(struct xhci *x, int type, struct trb *out, int ms) // A transfer on some other endpoint is not either: with no interrupt // the hot-plug thread is the only reader of this ring, and a keyboard // report it threw away here would be a lost keypress. + // + // One on endpoint 0 is a straggler from a control transfer that has + // already been given up on -- an error ends a transfer at the TRB that + // failed, and its completion can trail the wait that returned for it. if(t == TRB_XFER_EVENT && TRB_GET_EPID(out->control) != DCI_EP0){ acquire(&x->lock); transfer_done(x, out); release(&x->lock); - } else if(t != TRB_PORT_STATUS) + } else if(t == TRB_XFER_EVENT) + kinfo("xhci: late completion on slot %d endpoint 0, code %d\n", + TRB_GET_SLOT(out->control), TRB_CC(out->status)); + else if(t != TRB_PORT_STATUS) cprintf("xhci: unexpected event type %d while waiting for %d\n", t, type); } @@ -431,6 +438,26 @@ speed_name(int speed) // Control transfers // --------------------------------------------------------------------------- +// Put endpoint 0 back in service after a transfer that did not finish. +// +// A STALL or a babble halts the endpoint, and a halted endpoint refuses every +// transfer after it -- so one request a device did not like (plenty stall a +// string descriptor or SET_IDLE they do not implement) used to cost the whole +// device: everything that followed timed out. A timeout leaves it running +// with TRBs still queued, which is no better. Either way it is stopped or +// reset, and its dequeue pointer moved past whatever was left on the ring. +static void +ep0_recover(struct xhci *x, int slot, int halted) +{ + struct ring *r = &x->slot[slot].ep0; + + command(x, 0, 0, TRB_TYPE(halted ? TRB_RESET_EP : TRB_STOP_EP) | + ((uint32)DCI_EP0 << 16) | TRB_SLOT(slot), 0); + command(x, (r->phys + (uint64)r->enq * 16) | (r->cycle ? 1 : 0), 0, + TRB_TYPE(TRB_SET_TR_DEQ) | ((uint32)DCI_EP0 << 16) | TRB_SLOT(slot), + 0); +} + // A control transfer is three stages on the endpoint ring: Setup, an optional // Data stage, and Status. Only the last carries IOC, so one transfer produces // one event. @@ -481,6 +508,7 @@ control_xfer(struct xhci *x, int slot, struct usb_setup *s, void *buf, int len) if(rc < 0){ cprintf("xhci: control transfer timed out (slot %d, request 0x%x)\n", slot, s->bRequest); + ep0_recover(x, slot, 0); return -1; } } else { @@ -490,6 +518,7 @@ control_xfer(struct xhci *x, int slot, struct usb_setup *s, void *buf, int len) if(event_wait(x, TRB_XFER_EVENT, &ev, 1000) < 0){ cprintf("xhci: control transfer timed out (slot %d, request 0x%x)\n", slot, s->bRequest); + ep0_recover(x, slot, 0); return -1; } } @@ -498,7 +527,17 @@ control_xfer(struct xhci *x, int slot, struct usb_setup *s, void *buf, int len) // have", which is the normal reply to asking for more than it holds. int cc = TRB_CC(ev.status); if(cc != CC_SUCCESS && cc != CC_SHORT_PACKET){ - cprintf("xhci: control transfer failed, completion code %d\n", cc); + // A stall is a device declining a request, which is its right and + // which callers already treat as "not supported". Only say so on a + // verbose boot; anything else is worth a line. + if(cc == CC_STALL) + kinfo("xhci: slot %d stalled request 0x%x (type 0x%x, value 0x%x)\n", + slot, s->bRequest, s->bmRequestType, s->wValue); + else + cprintf("xhci: control transfer failed, completion code %d " + "(slot %d, request 0x%x, value 0x%x)\n", + cc, slot, s->bRequest, s->wValue); + ep0_recover(x, slot, 1); return -1; } return 0; diff --git a/kernel/drivers/bus/xhci/xhci.h b/kernel/drivers/bus/xhci/xhci.h index e433386..3ba069f 100644 --- a/kernel/drivers/bus/xhci/xhci.h +++ b/kernel/drivers/bus/xhci/xhci.h @@ -143,6 +143,8 @@ struct trb { #define TRB_CONFIG_EP 12 #define TRB_EVAL_CTX 13 #define TRB_RESET_EP 14 +#define TRB_STOP_EP 15 +#define TRB_SET_TR_DEQ 16 #define TRB_NOOP_CMD 23 // Event TRBs #define TRB_XFER_EVENT 32 @@ -151,6 +153,8 @@ struct trb { #define TRB_CC(status) (((status) >> 24) & 0xFF) #define CC_SUCCESS 1 +#define CC_BABBLE 3 +#define CC_STALL 6 #define CC_SHORT_PACKET 13 // Setup Stage transfer type, and Data Stage direction. diff --git a/kernel/drivers/usb/hid/usb_hid.c b/kernel/drivers/usb/hid/usb_hid.c index ba769de..f35c56c 100644 --- a/kernel/drivers/usb/hid/usb_hid.c +++ b/kernel/drivers/usb/hid/usb_hid.c @@ -48,6 +48,17 @@ struct hid { int boot; int report_id; // 0 when reports carry no ID byte struct hfield x, y, wheel, btn[HID_NBTN]; + + // A keyboard in report protocol: where its keys are, so that each report + // can be rewritten into the boot layout and go through the same path. Not + // every keyboard interface offers the boot subclass -- the second + // interface of a gaming keyboard, the macro keys of a mouse, an NKRO mode + // that reports a bitmap instead of six slots. + int rkbd; // set when any of the below was found + int kbd_id; // their report ID, 0 for none + int mod_bit[8]; // E0-E7, -1 where the report has none + int arr_bit, arr_n, arr_size; // an array of usages, arr_n slots + int map_bit, map_first, map_n; // a bitmap: usage map_first + i }; static struct hid hids[NHID]; @@ -284,6 +295,7 @@ usb_hid_tick(void) // --------------------------------------------------------------------------- #define PAGE_DESKTOP 0x01 +#define PAGE_KEYBOARD 0x07 #define PAGE_BUTTON 0x09 #define U_X 0x30 #define U_Y 0x31 @@ -320,7 +332,49 @@ take_field(struct hid *h, uint32 usage, int bit, int size, int lmin, int lmax, f->rel = rel; } -// Returns 0 if a pointer was found: an X and a Y in the same report. +// One Input item on the keyboard page. Everything is taken from the first +// report ID that has any of it, for the same reason as take_field's. +static void +take_keys(struct hid *h, int variable, int bit, int size, int count, + uint32 first, int id) +{ + int k; + + if(h->rkbd && id != h->kbd_id) + return; + + if(!variable){ + // An array: `count` slots, each holding the usage of a key that is + // down. The usage range only says which keys it can name. + if(h->arr_n || size < 1 || size > 16) + return; + h->arr_bit = bit; + h->arr_n = count; + h->arr_size = size; + } else { + // One bit per key, the usage stepping up from `first`. Modifiers are + // the common case; a whole keyboard's worth is NKRO. + if(size != 1) + return; + for(k = 0; k < count; k++){ + uint32 u = (first & 0xFFFF) + k; + if(u >= 0xE0 && u <= 0xE7) + h->mod_bit[u - 0xE0] = bit + k; + else if(h->map_n == 0){ + h->map_bit = bit + k; + h->map_first = u; + h->map_n = 1; + } else if(bit + k == h->map_bit + h->map_n && + (int)u == h->map_first + h->map_n) + h->map_n++; + } + } + h->rkbd = 1; + h->kbd_id = id; +} + +// Returns 0 if a pointer was found -- an X and a Y in the same report -- or a +// keyboard. static int parse_report(struct hid *h, const uchar *d, int n) { @@ -331,6 +385,8 @@ parse_report(struct hid *h, const uchar *d, int n) int i = 0, k; memset(off, 0, sizeof(off)); + for(k = 0; k < 8; k++) + h->mod_bit[k] = -1; while(i < n){ uchar b = d[i++]; int sz = b & 3, type = (b >> 2) & 3, tag = b >> 4; @@ -371,7 +427,13 @@ parse_report(struct hid *h, const uchar *d, int n) case 2: umax = full; break; } } else if(type == 0){ // main - if(tag == 8 && id < 16 && rsize <= 32 && rcount <= 256){ + if(tag == 8 && id < 16 && rsize <= 32 && rcount <= 256 && + page == PAGE_KEYBOARD){ + uint32 first = range ? umin : nusage ? usage[0] : 0; + if(!(v & 1)) + take_keys(h, (v & 2) != 0, off[id], rsize, rcount, first, id); + off[id] += rsize * rcount; + } else if(tag == 8 && id < 16 && rsize <= 32 && rcount <= 256){ for(k = 0; k < (int)rcount; k++){ uint32 u = 0; if(k < nusage) @@ -392,12 +454,65 @@ parse_report(struct hid *h, const uchar *d, int n) } } - if(!h->x.ok || !h->y.ok) - return -1; + if(!h->x.ok || !h->y.ok){ + memset(&h->x, 0, sizeof(h->x)); // half a pointer is no pointer + memset(&h->y, 0, sizeof(h->y)); + return h->rkbd && (h->arr_n || h->map_n) ? 0 : -1; + } h->report_id = target; return 0; } +static uint32 +bits(const uchar *p, int len, int bit, int size) +{ + uint32 v = 0; + int i; + + if((bit + size + 7) / 8 > len) + return 0; + for(i = 0; i < size; i++, bit++) + if((p[bit >> 3] >> (bit & 7)) & 1) + v |= 1u << i; + return v; +} + +// A report-protocol keyboard report, rewritten as a boot one: modifiers, a +// reserved byte, six usages. Six is a limit only of the rewrite; the seventh +// key held at once is dropped, which is what a boot keyboard does too. +static void +report_keyboard(struct hid *h, const uchar *rep, int len) +{ + uchar boot[8]; + int i, n = 0; + + if(h->kbd_id){ + if(len < 1 || rep[0] != h->kbd_id) + return; + rep++; + len--; + } + memset(boot, 0, sizeof(boot)); + for(i = 0; i < 8; i++) + if(h->mod_bit[i] >= 0 && bits(rep, len, h->mod_bit[i], 1)) + boot[0] |= 1 << i; + for(i = 0; i < h->arr_n; i++){ + uint32 u = bits(rep, len, h->arr_bit + i * h->arr_size, h->arr_size); + if(u >= 0xE0 && u <= 0xE7) + boot[0] |= 1 << (u - 0xE0); + else if(u >= 4 && u < 0xE0 && n < 6) // 1-3 are error codes + boot[2 + n++] = u; + } + for(i = 0; i < h->map_n; i++){ + uint32 u = h->map_first + i; + if(u < 4 || u >= 0xE0 || !bits(rep, len, h->map_bit + i, 1)) + continue; + if(n < 6) + boot[2 + n++] = u; + } + keyboard_report(h, boot, sizeof(boot)); +} + static int field(const uchar *p, int len, const struct hfield *f) { @@ -486,9 +601,13 @@ hid_complete(struct usb_dev *dev, void *buf, int len) if(h == 0) return; - if(h->proto == USB_HID_PROTO_KEYBOARD) + // One interface can be both: a wireless receiver describes its keyboard + // and its mouse as two report IDs of one descriptor. + if(h->rkbd) + report_keyboard(h, buf, len); + if(h->proto == USB_HID_PROTO_KEYBOARD && !h->rkbd) keyboard_report(h, buf, len); - else + else if(h->x.ok || h->boot) pointer_report(h, buf, len); } @@ -595,7 +714,8 @@ hid_probe(struct usb_dev *dev) // always a device reset. Only a boot device knows the request. if(boot) hid_request(dev, USB_HID_SET_PROTOCOL, USB_HID_PROTOCOL_REPORT); - h->proto = h->x.rel ? USB_HID_PROTO_MOUSE : USB_HID_PROTO_TABLET; + h->proto = !h->x.ok ? USB_HID_PROTO_KEYBOARD : + h->x.rel ? USB_HID_PROTO_MOUSE : USB_HID_PROTO_TABLET; } else if(boot && proto == USB_HID_PROTO_MOUSE){ memset(h, 0, sizeof(*h)); if(hid_request(dev, USB_HID_SET_PROTOCOL, USB_HID_PROTOCOL_BOOT) < 0){ @@ -635,7 +755,10 @@ hid_probe(struct usb_dev *dev) cprintf("usb-hid: %s%s on slot %d, endpoint 0x%x every %d ms\n", h->proto == USB_HID_PROTO_KEYBOARD ? "keyboard" : h->proto == USB_HID_PROTO_MOUSE ? "mouse" : "absolute pointer", - h->proto == USB_HID_PROTO_KEYBOARD || h->boot ? "" : + h->proto == USB_HID_PROTO_KEYBOARD ? + (h->rkbd ? " (report protocol)" : "") : + h->boot ? "" : + h->rkbd ? " and keyboard" : h->wheel.ok ? " with wheel" : "", dev->slot, dev->intr_in.bEndpointAddress, dev->intr_in.bInterval); return 0; @@ -651,7 +774,7 @@ hid_disconnect(struct usb_dev *dev) if(h == 0) return; - if(h->proto == USB_HID_PROTO_KEYBOARD){ + if(h->proto == USB_HID_PROTO_KEYBOARD || h->rkbd){ keyboard_events(h->prev, none); acquire(&kqlock); krep.c = 0;