refactor(usb): the configuration-descriptor walk moved out of the event loop, unchanged
xhci.c's XHCI_XFER_CONFIG_DESC_FULL case walked the descriptor set inline, in the event loop, where nothing could test it. The walk is now usb_config_parse() (kernel/src/usb/usb_config.c), returning what it found; the case acts on it as before -- the same messages, the same decisions: the first Interface descriptor only, a BOT interface's bulk endpoints (the last of each direction), a boot keyboard's first interrupt IN endpoint, bLength 0 ending the walk, nothing read past len. One difference, in debug output only: the bulk endpoints are logged IN then OUT, not in descriptor order. kernel/tests/test_usb_config.c, 7 checks, pins the walk as moved: a BOT stick as QEMU presents it, a boot keyboard with its HID descriptor, a mouse before a drive (the first interface only), a drive with bulk IN only, bLength 0, an endpoint cut by len, an interface cut before its protocol byte. All pass. Built for amd64, aarch64, riscv64; the boots come with the protocol (a data stick enumerates through this path). The fixes for #170 and #171 follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017RzZgrf7V7Zk2iczBXDRbm
This commit is contained in:
@@ -0,0 +1,81 @@
|
||||
/* SPDX-License-Identifier: LicenseRef-Starship-1.0
|
||||
* Copyright (c) 2023-2026 Robert A. James
|
||||
*/
|
||||
/* usb_config.c -- see usb_config.h. Moved from xhci.c's XHCI_XFER_CONFIG_DESC_FULL case, the walk
|
||||
* unchanged: found by bDescriptorType, not by a fixed offset; the first Interface descriptor only;
|
||||
* a Mass Storage/SCSI/BOT interface's bulk endpoints up to the next Interface descriptor (the last
|
||||
* of each direction wins); a boot keyboard's first interrupt IN endpoint.
|
||||
*/
|
||||
#include "starkernel/usb_config.h"
|
||||
#include "starkernel/xhci.h"
|
||||
#include <string.h>
|
||||
|
||||
static uint16_t le16(const uint8_t *p) { return (uint16_t)(p[0] | ((uint16_t)p[1] << 8)); }
|
||||
|
||||
void usb_config_parse(const uint8_t *desc, uint16_t len, usb_config_t *out)
|
||||
{
|
||||
uint16_t off = 0;
|
||||
|
||||
memset(out, 0, sizeof *out);
|
||||
out->kind = USB_CONFIG_NO_INTERFACE;
|
||||
out->config_value = desc[USB_CONFIG_OFF_CONFIG_VALUE];
|
||||
|
||||
while (off + 2 <= len) {
|
||||
uint8_t desc_len = desc[off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t desc_type = desc[off + USB_DESC_OFF_TYPE];
|
||||
if (desc_len == 0) break; /* malformed -- avoid an infinite loop */
|
||||
if (desc_type == USB_DESC_TYPE_INTERFACE && off + USB_IFACE_OFF_PROTOCOL < len) {
|
||||
uint16_t ep_off = (uint16_t)(off + desc_len);
|
||||
out->iface_class = desc[off + USB_IFACE_OFF_CLASS];
|
||||
out->iface_subclass = desc[off + USB_IFACE_OFF_SUBCLASS];
|
||||
out->iface_protocol = desc[off + USB_IFACE_OFF_PROTOCOL];
|
||||
if (out->iface_class == USB_CLASS_MASS_STORAGE && out->iface_subclass == USB_SUBCLASS_SCSI &&
|
||||
out->iface_protocol == USB_PROTOCOL_BOT) {
|
||||
out->kind = USB_CONFIG_MSC;
|
||||
while (ep_off + 2 <= len) {
|
||||
uint8_t ep_len = desc[ep_off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t ep_type = desc[ep_off + USB_DESC_OFF_TYPE];
|
||||
if (ep_len == 0) break;
|
||||
if (ep_type == USB_DESC_TYPE_INTERFACE) break;
|
||||
if (ep_type == USB_DESC_TYPE_ENDPOINT && ep_off + USB_EP_OFF_MAX_PACKET_SIZE + 1 < len) {
|
||||
uint8_t ep_addr = desc[ep_off + USB_EP_OFF_ADDRESS];
|
||||
uint8_t ep_attr = desc[ep_off + USB_EP_OFF_ATTRIBUTES];
|
||||
if ((ep_attr & USB_EP_ATTR_TYPE_MASK) == USB_EP_TYPE_BULK) {
|
||||
if (ep_addr & USB_EP_ADDR_DIR_MASK) {
|
||||
out->bulk_in_ep = ep_addr; out->bulk_in_max = le16(desc + ep_off + USB_EP_OFF_MAX_PACKET_SIZE);
|
||||
} else {
|
||||
out->bulk_out_ep = ep_addr; out->bulk_out_max = le16(desc + ep_off + USB_EP_OFF_MAX_PACKET_SIZE);
|
||||
}
|
||||
}
|
||||
}
|
||||
ep_off = (uint16_t)(ep_off + ep_len);
|
||||
}
|
||||
} else if (out->iface_class == USB_CLASS_HID && out->iface_subclass == USB_SUBCLASS_HID_BOOT &&
|
||||
out->iface_protocol == USB_PROTOCOL_HID_KEYBOARD) {
|
||||
out->kind = USB_CONFIG_KBD;
|
||||
out->hid_iface = desc[off + USB_IFACE_OFF_NUMBER];
|
||||
while (ep_off + 2 <= len) {
|
||||
uint8_t ep_len = desc[ep_off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t ep_type = desc[ep_off + USB_DESC_OFF_TYPE];
|
||||
if (ep_len == 0) break;
|
||||
if (ep_type == USB_DESC_TYPE_INTERFACE) break;
|
||||
if (ep_type == USB_DESC_TYPE_ENDPOINT && ep_off + USB_EP_OFF_INTERVAL < len) {
|
||||
uint8_t ep_addr = desc[ep_off + USB_EP_OFF_ADDRESS];
|
||||
uint8_t ep_attr = desc[ep_off + USB_EP_OFF_ATTRIBUTES];
|
||||
if ((ep_attr & USB_EP_ATTR_TYPE_MASK) == USB_EP_TYPE_INTERRUPT && (ep_addr & USB_EP_ADDR_DIR_MASK)) {
|
||||
out->hid_ep = ep_addr;
|
||||
out->hid_max = (uint16_t)(le16(desc + ep_off + USB_EP_OFF_MAX_PACKET_SIZE) & 0x7FFu);
|
||||
out->hid_interval = desc[ep_off + USB_EP_OFF_INTERVAL];
|
||||
break;
|
||||
}
|
||||
}
|
||||
ep_off = (uint16_t)(ep_off + ep_len);
|
||||
}
|
||||
} else {
|
||||
out->kind = USB_CONFIG_OTHER;
|
||||
}
|
||||
return; /* the first Interface descriptor only */
|
||||
}
|
||||
off = (uint16_t)(off + desc_len);
|
||||
}
|
||||
}
|
||||
+56
-131
@@ -26,6 +26,7 @@
|
||||
#include "starkernel/xhci.h"
|
||||
#include "starkernel/xhci_driver.h"
|
||||
#include "starkernel/usb_kbd.h" /* A3: a boot keyboard's reports and lights */
|
||||
#include "starkernel/usb_config.h" /* the configuration descriptor set, read */
|
||||
#include "starkernel/console.h" /* A3: "USB keyboard ready" */
|
||||
#include "starkernel/kmalloc.h"
|
||||
#include "starkernel/timer.h"
|
||||
@@ -2221,140 +2222,64 @@ static void xhci_poll_one(xhci_dev_t *dev)
|
||||
}
|
||||
case XHCI_XFER_CONFIG_DESC_FULL: {
|
||||
log_message(LOG_DEBUG, "xhci: full config descriptor received");
|
||||
/* Walk the concatenated descriptor stream
|
||||
* (Config + Interface + Endpoint descriptors
|
||||
* back to back) looking for the Interface
|
||||
* descriptor -- its fixed offset within the
|
||||
* stream isn't guaranteed, has to be found by
|
||||
* bDescriptorType, not assumed. */
|
||||
/* The walk of the descriptor set is usb_config_parse() (usb_config.c,
|
||||
* moved out of here unchanged and tested on the host,
|
||||
* kernel/tests/test_usb_config.c); what it found is acted on here, as
|
||||
* before. */
|
||||
usb_config_t cfg;
|
||||
uint16_t len = xms->config_total_length;
|
||||
if (len > sizeof(xms->config_descriptor)) len = (uint16_t)sizeof(xms->config_descriptor);
|
||||
uint16_t off = 0;
|
||||
int found = 0;
|
||||
while (off + 2 <= len) {
|
||||
uint8_t desc_len = xms->config_descriptor[off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t desc_type = xms->config_descriptor[off + USB_DESC_OFF_TYPE];
|
||||
if (desc_len == 0) break; /* malformed -- avoid an infinite loop */
|
||||
if (desc_type == USB_DESC_TYPE_INTERFACE &&
|
||||
off + USB_IFACE_OFF_PROTOCOL < len) {
|
||||
uint8_t iface_class = xms->config_descriptor[off + USB_IFACE_OFF_CLASS];
|
||||
uint8_t iface_subclass = xms->config_descriptor[off + USB_IFACE_OFF_SUBCLASS];
|
||||
uint8_t iface_protocol = xms->config_descriptor[off + USB_IFACE_OFF_PROTOCOL];
|
||||
xhci_log_hex32("xhci: bInterfaceClass=", iface_class);
|
||||
xhci_log_hex32("xhci: bInterfaceSubClass=", iface_subclass);
|
||||
xhci_log_hex32("xhci: bInterfaceProtocol=", iface_protocol);
|
||||
if (iface_class == USB_CLASS_MASS_STORAGE &&
|
||||
iface_subclass == USB_SUBCLASS_SCSI &&
|
||||
iface_protocol == USB_PROTOCOL_BOT) {
|
||||
log_message(LOG_DEBUG, "xhci: confirmed Mass Storage / SCSI / BOT device");
|
||||
/* Walk the Endpoint descriptors that
|
||||
* follow this Interface descriptor,
|
||||
* stopping at the next Interface
|
||||
* descriptor (start of a different
|
||||
* interface's endpoints) or end of
|
||||
* the stream. Only bulk endpoints
|
||||
* are of interest for BOT. */
|
||||
uint16_t ep_off = (uint16_t)(off + desc_len);
|
||||
while (ep_off + 2 <= len) {
|
||||
uint8_t ep_desc_len = xms->config_descriptor[ep_off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t ep_desc_type = xms->config_descriptor[ep_off + USB_DESC_OFF_TYPE];
|
||||
if (ep_desc_len == 0) break; /* malformed -- avoid an infinite loop */
|
||||
if (ep_desc_type == USB_DESC_TYPE_INTERFACE) break;
|
||||
if (ep_desc_type == USB_DESC_TYPE_ENDPOINT &&
|
||||
ep_off + USB_EP_OFF_MAX_PACKET_SIZE + 1 < len) {
|
||||
uint8_t ep_addr = xms->config_descriptor[ep_off + USB_EP_OFF_ADDRESS];
|
||||
uint8_t ep_attr = xms->config_descriptor[ep_off + USB_EP_OFF_ATTRIBUTES];
|
||||
uint16_t ep_max_packet = (uint16_t)(xms->config_descriptor[ep_off + USB_EP_OFF_MAX_PACKET_SIZE] |
|
||||
((uint16_t)xms->config_descriptor[ep_off + USB_EP_OFF_MAX_PACKET_SIZE + 1] << 8));
|
||||
if ((ep_attr & USB_EP_ATTR_TYPE_MASK) == USB_EP_TYPE_BULK) {
|
||||
if (ep_addr & USB_EP_ADDR_DIR_MASK) {
|
||||
xms->bulk_in_ep_addr = ep_addr;
|
||||
xms->bulk_in_max_packet = ep_max_packet;
|
||||
xhci_log_hex32("xhci: bulk IN endpoint=", ep_addr);
|
||||
} else {
|
||||
xms->bulk_out_ep_addr = ep_addr;
|
||||
xms->bulk_out_max_packet = ep_max_packet;
|
||||
xhci_log_hex32("xhci: bulk OUT endpoint=", ep_addr);
|
||||
}
|
||||
}
|
||||
}
|
||||
ep_off = (uint16_t)(ep_off + ep_desc_len);
|
||||
}
|
||||
/* Deferred (see xhci_dev_t's
|
||||
* next_action doc comment) rather
|
||||
* than called directly here --
|
||||
* same doorbell-ordering hazard
|
||||
* as the device/config descriptor
|
||||
* chaining above. next_action_
|
||||
* config_value is staged now but
|
||||
* not consumed until SET_CONFIG
|
||||
* actually runs, after Configure
|
||||
* Endpoint completes below. */
|
||||
xms->next_action_config_value =
|
||||
xms->config_descriptor[USB_CONFIG_OFF_CONFIG_VALUE];
|
||||
if (xms->bulk_in_ep_addr == 0 || xms->bulk_out_ep_addr == 0) {
|
||||
/* Can't Configure Endpoint
|
||||
* without knowing both bulk
|
||||
* endpoints -- go straight to
|
||||
* SET_CONFIGURATION so the
|
||||
* device is still usable for
|
||||
* whatever doesn't need BOT
|
||||
* (nothing, today, but this
|
||||
* keeps the two concerns
|
||||
* separate rather than
|
||||
* failing enumeration
|
||||
* outright). */
|
||||
log_message(LOG_WARN, "xhci: warning -- BOT device missing a bulk IN or OUT endpoint, skipping configure endpoint");
|
||||
xms->next_action = XHCI_NEXT_ACTION_SET_CONFIG;
|
||||
} else {
|
||||
xms->next_action = XHCI_NEXT_ACTION_CONFIGURE_ENDPOINT;
|
||||
}
|
||||
} else if (iface_class == USB_CLASS_HID &&
|
||||
iface_subclass == USB_SUBCLASS_HID_BOOT &&
|
||||
iface_protocol == USB_PROTOCOL_HID_KEYBOARD) {
|
||||
/* A boot keyboard (2026-10-14, A3): its
|
||||
* interface number, and the first
|
||||
* interrupt IN endpoint after it. */
|
||||
uint16_t ep_off = (uint16_t)(off + desc_len);
|
||||
xms->hid_iface = xms->config_descriptor[off + USB_IFACE_OFF_NUMBER];
|
||||
while (ep_off + 2 <= len) {
|
||||
uint8_t ep_desc_len = xms->config_descriptor[ep_off + USB_DESC_OFF_LENGTH];
|
||||
uint8_t ep_desc_type = xms->config_descriptor[ep_off + USB_DESC_OFF_TYPE];
|
||||
if (ep_desc_len == 0) break;
|
||||
if (ep_desc_type == USB_DESC_TYPE_INTERFACE) break;
|
||||
if (ep_desc_type == USB_DESC_TYPE_ENDPOINT &&
|
||||
ep_off + USB_EP_OFF_INTERVAL < len) {
|
||||
uint8_t ep_addr = xms->config_descriptor[ep_off + USB_EP_OFF_ADDRESS];
|
||||
uint8_t ep_attr = xms->config_descriptor[ep_off + USB_EP_OFF_ATTRIBUTES];
|
||||
if ((ep_attr & USB_EP_ATTR_TYPE_MASK) == USB_EP_TYPE_INTERRUPT &&
|
||||
(ep_addr & USB_EP_ADDR_DIR_MASK)) {
|
||||
xms->hid_ep_addr = ep_addr;
|
||||
xms->hid_max_packet = (uint16_t)(xms->config_descriptor[ep_off + USB_EP_OFF_MAX_PACKET_SIZE] |
|
||||
((uint16_t)xms->config_descriptor[ep_off + USB_EP_OFF_MAX_PACKET_SIZE + 1] << 8)) & 0x7FFu;
|
||||
xms->hid_interval = xms->config_descriptor[ep_off + USB_EP_OFF_INTERVAL];
|
||||
break;
|
||||
}
|
||||
}
|
||||
ep_off = (uint16_t)(ep_off + ep_desc_len);
|
||||
}
|
||||
xms->next_action_config_value =
|
||||
xms->config_descriptor[USB_CONFIG_OFF_CONFIG_VALUE];
|
||||
if (xms->hid_ep_addr) {
|
||||
xms->hid_kbd = 1;
|
||||
xms->next_action = XHCI_NEXT_ACTION_CONFIGURE_ENDPOINT;
|
||||
} else {
|
||||
log_message(LOG_WARN, "xhci: keyboard without an interrupt IN endpoint -- not used");
|
||||
}
|
||||
} else {
|
||||
log_message(LOG_WARN, "xhci: neither a Mass Storage/SCSI/BOT drive nor a boot keyboard -- not used");
|
||||
}
|
||||
found = 1;
|
||||
break;
|
||||
}
|
||||
off = (uint16_t)(off + desc_len);
|
||||
}
|
||||
if (!found) {
|
||||
usb_config_parse(xms->config_descriptor, len, &cfg);
|
||||
if (cfg.kind == USB_CONFIG_NO_INTERFACE) {
|
||||
log_message(LOG_WARN, "xhci: no Interface descriptor found in config set");
|
||||
break;
|
||||
}
|
||||
xhci_log_hex32("xhci: bInterfaceClass=", cfg.iface_class);
|
||||
xhci_log_hex32("xhci: bInterfaceSubClass=", cfg.iface_subclass);
|
||||
xhci_log_hex32("xhci: bInterfaceProtocol=", cfg.iface_protocol);
|
||||
if (cfg.kind == USB_CONFIG_MSC) {
|
||||
log_message(LOG_DEBUG, "xhci: confirmed Mass Storage / SCSI / BOT device");
|
||||
if (cfg.bulk_in_ep) {
|
||||
xms->bulk_in_ep_addr = cfg.bulk_in_ep;
|
||||
xms->bulk_in_max_packet = cfg.bulk_in_max;
|
||||
xhci_log_hex32("xhci: bulk IN endpoint=", cfg.bulk_in_ep);
|
||||
}
|
||||
if (cfg.bulk_out_ep) {
|
||||
xms->bulk_out_ep_addr = cfg.bulk_out_ep;
|
||||
xms->bulk_out_max_packet = cfg.bulk_out_max;
|
||||
xhci_log_hex32("xhci: bulk OUT endpoint=", cfg.bulk_out_ep);
|
||||
}
|
||||
/* Deferred (see xhci_dev_t's next_action doc comment) rather than
|
||||
* called directly here -- the same doorbell-ordering hazard as the
|
||||
* device/config descriptor chaining above. */
|
||||
xms->next_action_config_value = cfg.config_value;
|
||||
if (xms->bulk_in_ep_addr == 0 || xms->bulk_out_ep_addr == 0) {
|
||||
/* Can't Configure Endpoint without both bulk endpoints -- go
|
||||
* straight to SET_CONFIGURATION, keeping the two concerns
|
||||
* separate rather than failing enumeration outright. */
|
||||
log_message(LOG_WARN, "xhci: warning -- BOT device missing a bulk IN or OUT endpoint, skipping configure endpoint");
|
||||
xms->next_action = XHCI_NEXT_ACTION_SET_CONFIG;
|
||||
} else {
|
||||
xms->next_action = XHCI_NEXT_ACTION_CONFIGURE_ENDPOINT;
|
||||
}
|
||||
} else if (cfg.kind == USB_CONFIG_KBD) {
|
||||
/* A boot keyboard (2026-10-14, A3): its interface number, and the
|
||||
* first interrupt IN endpoint after it. */
|
||||
xms->hid_iface = cfg.hid_iface;
|
||||
if (cfg.hid_ep) {
|
||||
xms->hid_ep_addr = cfg.hid_ep;
|
||||
xms->hid_max_packet = cfg.hid_max;
|
||||
xms->hid_interval = cfg.hid_interval;
|
||||
}
|
||||
xms->next_action_config_value = cfg.config_value;
|
||||
if (xms->hid_ep_addr) {
|
||||
xms->hid_kbd = 1;
|
||||
xms->next_action = XHCI_NEXT_ACTION_CONFIGURE_ENDPOINT;
|
||||
} else {
|
||||
log_message(LOG_WARN, "xhci: keyboard without an interrupt IN endpoint -- not used");
|
||||
}
|
||||
} else {
|
||||
log_message(LOG_WARN, "xhci: neither a Mass Storage/SCSI/BOT drive nor a boot keyboard -- not used");
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user