From 4165c42c69c4aacfef0d053ef0f877cf241c14a1 Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Mon, 2 Mar 2026 15:19:21 -0800 Subject: [PATCH 1/3] htool_console: Reconnect USB on failure We usually don't want to kill the console logger with an error when a USB failure occurs. Instead, we want to try reconnecting in a loop until the device comes back. This allows us to retain the console pointer around platform resets that cause USB disconnections and prevents our services streaming these logs from sending duplicate messages. --- examples/htool_console.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/examples/htool_console.c b/examples/htool_console.c index be687f5..2bcc0a0 100644 --- a/examples/htool_console.c +++ b/examples/htool_console.c @@ -105,26 +105,39 @@ int htool_console_run(struct libhoth_device* dev, bool quit = false; while (!quit) { + // Any previous failure should reset all of the USB state before retrying + while (status != LIBHOTH_OK) { + // TODO: Read STDIN during this time and buffer it so we can capture + // quit events even when the console is disconnected. We will also want + // to tune the reconnect time to match. + status = libhoth_device_reconnect(dev); + // If USB is down we might fail reconnect, just retry + if (status != LIBHOTH_OK) { + // Make sure we don't end up in a tight retry loop + usleep(100 * 1000); + } + } + // Give an opportunity for other clients to use the interface. + libhoth_release_device(dev); + usleep(1000 * opts->yield_ms); + status = libhoth_claim_device(dev, 1000 * 1000 * opts->claim_timeout_secs); + if (status != LIBHOTH_OK) { + // If USB is down we might fail claim, just go back and retry + continue; + } + status = libhoth_read_console(dev, STDOUT_FILENO, false, opts->channel_id, &offset); if (status != LIBHOTH_OK) { - break; + // Device resets cause failures, just loop and allow reconnection + continue; } status = libhoth_write_console(dev, opts->channel_id, opts->force_drive_tx, &quit); if (status != LIBHOTH_OK) { - break; - } - - libhoth_release_device(dev); - - // Give an opportunity for other clients to use the interface. - usleep(1000 * opts->yield_ms); - - status = libhoth_claim_device(dev, 1000 * 1000 * opts->claim_timeout_secs); - if (status != LIBHOTH_OK) { - break; + // Device resets cause failures, just loop and allow reconnection + continue; } } From 5a8270dd345c124337d056d8b3d10ff3e67ccb51 Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Mon, 2 Mar 2026 15:46:03 -0800 Subject: [PATCH 2/3] protocol/console: Log buffer jumps It's possible that if the console is disconnected long enough we can experience jumps in the console data because the buffer on the RoT side was not big enough for our time lapse. Log any time this happens so we can understand how much context we lost if any. --- protocol/console.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/protocol/console.c b/protocol/console.c index 141d9ce..9c6420f 100644 --- a/protocol/console.c +++ b/protocol/console.c @@ -106,6 +106,10 @@ int libhoth_read_console(struct libhoth_device* dev, int fd, if (status != 0) { return status; } + if (req.offset != resp.resp.offset) { + fprintf(stderr, "Console jumped forward %u bytes\n", + resp.resp.offset - req.offset); + } int len = response_size - sizeof(resp.resp); if (len > 0) { @@ -130,9 +134,9 @@ int libhoth_read_console(struct libhoth_device* dev, int fd, return -1; } } - *offset = resp.resp.offset + len; } + *offset = resp.resp.offset + len; return 0; } From 01a85e1992eb1b9204bf28cc109b3134c358f264 Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Thu, 19 Mar 2026 01:08:41 +0000 Subject: [PATCH 3/3] libhoth_usb: Refactor reconnection and user_ctx lifecycle The current code does not allow multiple reconnect attempts if a previous one fails. To alleviate this, we need to refactor the usb context a bit so that we keep the context around and only free it with the libusb_device struct. This allows us to store the usb_loc in the struct even if the underlying libusb struct is destroyed due to failed initializations. We also fix a possible null ptr dereference that existed in the reconnect function. Signed-off-by: William A. Kennington III --- transports/libhoth_usb.c | 138 ++++++++++++++++---------------- transports/libhoth_usb_device.h | 3 + 2 files changed, 72 insertions(+), 69 deletions(-) diff --git a/transports/libhoth_usb.c b/transports/libhoth_usb.c index bee2865..08a2476 100644 --- a/transports/libhoth_usb.c +++ b/transports/libhoth_usb.c @@ -92,41 +92,53 @@ static int libhoth_usb_release(struct libhoth_device* dev) { return libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id); } +static int libhoth_usb_close_internal(struct libhoth_usb_device* usb_dev) { + int status = LIBUSB_SUCCESS; + if (usb_dev->handle != NULL) { + switch (usb_dev->info.type) { + case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX: + status = libhoth_usb_mailbox_close(usb_dev); + break; + case LIBHOTH_USB_INTERFACE_TYPE_FIFO: + status = libhoth_usb_fifo_close(usb_dev); + break; + default: + status = LIBHOTH_ERR_INTERFACE_NOT_FOUND; + break; + } + libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id); + libusb_close(usb_dev->handle); + usb_dev->handle = NULL; + } + return status; +} + static int libhoth_usb_reconnect(struct libhoth_device* dev) { struct libhoth_usb_device* usb_dev = dev->user_ctx; + if (usb_dev == NULL) { + return LIBUSB_ERROR_INVALID_PARAM; + } libusb_context* usb_ctx = usb_dev->ctx; uint64_t timeout_us = usb_dev->claim_timeout_us; - - struct libusb_device* libusb_dev = libusb_get_device(usb_dev->handle); - - struct libhoth_usb_loc usb_loc; - usb_loc.bus = libusb_get_bus_number(libusb_dev); - usb_loc.num_ports = libusb_get_port_numbers( - libusb_dev, (uint8_t*)&usb_loc.ports, LIBHOTH_NUM_PORTS); - if (usb_loc.num_ports == LIBUSB_ERROR_OVERFLOW) { - fprintf(stderr, "Failed to list port numbers when reconnecting. (%s)\n", - libusb_strerror(LIBUSB_ERROR_OVERFLOW)); - return LIBUSB_ERROR_OVERFLOW; - } - - libhoth_usb_close(dev); + struct libhoth_usb_loc usb_loc = usb_dev->loc; uint64_t start_time_ms = libhoth_get_monotonic_ms(); - while (1) { - libusb_exit(usb_ctx); - int ret = libusb_init(&usb_ctx); - if (ret != 0) { - fprintf( - stderr, - "libusb_init_context failed while reconnecting (error: %d (%s))\n", - ret, libusb_strerror(ret)); - } - - ret = libhoth_usb_get_device(usb_ctx, &usb_loc, &libusb_dev); + libusb_device* new_libusb_dev = NULL; + int ret = libhoth_usb_get_device(usb_ctx, &usb_loc, &new_libusb_dev); if (ret == 0) { - // Found the device - break; + struct libhoth_usb_device_init_options opts; + opts.usb_ctx = usb_ctx; + opts.usb_device = new_libusb_dev; + opts.timeout_us = timeout_us; + opts.prng_seed = libhoth_prng_seed(); + + // Close the old handle and driver data, but keep the usb_dev structure. + libhoth_usb_close_internal(usb_dev); + + ret = libhoth_usb_device_open(&opts, dev); + libusb_unref_device(new_libusb_dev); + return ret; } uint64_t current_time_ms = libhoth_get_monotonic_ms(); @@ -136,27 +148,18 @@ static int libhoth_usb_reconnect(struct libhoth_device* dev) { stderr, "libhoth_usb_open timed out while reconnecting (error: %d (%s))\n", ret, libusb_strerror(ret)); - libusb_exit(usb_ctx); return ret; // Timeout } // 100ms delay usleep(100 * 1000); } - - struct libhoth_usb_device_init_options opts; - opts.usb_ctx = usb_ctx; - opts.usb_device = libusb_dev; - opts.timeout_us = timeout_us; - opts.prng_seed = libhoth_prng_seed(); - - return libhoth_usb_device_open(&opts, dev); } static int libhoth_usb_device_open( const struct libhoth_usb_device_init_options* options, struct libhoth_device* dev) { - struct libhoth_usb_device* usb_dev = NULL; + struct libhoth_usb_device* usb_dev = dev->user_ctx; struct libusb_device_descriptor device_descriptor; int status = libusb_get_device_descriptor(options->usb_device, &device_descriptor); @@ -185,14 +188,23 @@ static int libhoth_usb_device_open( goto err_out; } - usb_dev = calloc(1, sizeof(struct libhoth_usb_device)); if (usb_dev == NULL) { - status = LIBHOTH_ERR_MALLOC_FAILED; - goto err_out; + usb_dev = calloc(1, sizeof(struct libhoth_usb_device)); + if (usb_dev == NULL) { + status = LIBHOTH_ERR_MALLOC_FAILED; + goto err_out; + } + dev->user_ctx = usb_dev; } usb_dev->info = info; usb_dev->ctx = options->usb_ctx; usb_dev->claim_timeout_us = options->timeout_us; + + if (libhoth_get_usb_loc(options->usb_device, &usb_dev->loc) != 0) { + status = LIBUSB_ERROR_OTHER; + goto err_out; + } + status = libusb_open(options->usb_device, &usb_dev->handle); if (status != LIBUSB_SUCCESS) { goto err_out; @@ -204,7 +216,6 @@ static int libhoth_usb_device_open( dev->claim = libhoth_usb_claim; dev->release = libhoth_usb_release; dev->reconnect = libhoth_usb_reconnect; - dev->user_ctx = usb_dev; status = libhoth_claim_device(dev, options->timeout_us); if (status != LIBHOTH_OK) { @@ -231,14 +242,10 @@ static int libhoth_usb_device_open( return LIBHOTH_OK; err_out: - if (dev != NULL) { - if (usb_dev != NULL) { - if (usb_dev->handle != NULL) { - libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id); - libusb_close(usb_dev->handle); - } - free(usb_dev); - } + if (usb_dev != NULL && usb_dev->handle != NULL) { + libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id); + libusb_close(usb_dev->handle); + usb_dev->handle = NULL; } libusb_free_config_descriptor(config_descriptor); return status; @@ -257,6 +264,9 @@ int libhoth_usb_open(const struct libhoth_usb_device_init_options* options, int ret = libhoth_usb_device_open(options, dev); if (ret != LIBUSB_SUCCESS) { + if (dev->user_ctx != NULL) { + free(dev->user_ctx); + } free(dev); return ret; } @@ -273,6 +283,9 @@ int libhoth_usb_send_request(struct libhoth_device* dev, const void* request, struct libhoth_usb_device* usb_dev = (struct libhoth_usb_device*)dev->user_ctx; + if (usb_dev->handle == NULL) { + return LIBUSB_ERROR_NO_DEVICE; + } switch (usb_dev->info.type) { case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX: return libhoth_usb_mailbox_send_request(usb_dev, request, request_size); @@ -293,6 +306,9 @@ int libhoth_usb_receive_response(struct libhoth_device* dev, void* response, struct libhoth_usb_device* usb_dev = (struct libhoth_usb_device*)dev->user_ctx; + if (usb_dev->handle == NULL) { + return LIBUSB_ERROR_NO_DEVICE; + } switch (usb_dev->info.type) { case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX: return libhoth_usb_mailbox_receive_response( @@ -307,33 +323,17 @@ int libhoth_usb_receive_response(struct libhoth_device* dev, void* response, } int libhoth_usb_close(struct libhoth_device* dev) { - int status; if (dev->user_ctx == NULL) { - return LIBUSB_ERROR_INVALID_PARAM; + return LIBUSB_SUCCESS; } struct libhoth_usb_device* usb_dev = (struct libhoth_usb_device*)dev->user_ctx; dev->user_ctx = NULL; - switch (usb_dev->info.type) { - case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX: - status = libhoth_usb_mailbox_close(usb_dev); - break; - case LIBHOTH_USB_INTERFACE_TYPE_FIFO: - status = libhoth_usb_fifo_close(usb_dev); - break; - default: - return LIBHOTH_ERR_INTERFACE_NOT_FOUND; - } - if (status != LIBHOTH_OK) { - return status; - } - if (usb_dev->handle != NULL) { - libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id); - libusb_close(usb_dev->handle); - } + + int status = libhoth_usb_close_internal(usb_dev); free(usb_dev); - return LIBHOTH_OK; + return status; } enum libusb_error transfer_status_to_error( diff --git a/transports/libhoth_usb_device.h b/transports/libhoth_usb_device.h index b9c9a46..147d984 100644 --- a/transports/libhoth_usb_device.h +++ b/transports/libhoth_usb_device.h @@ -19,6 +19,8 @@ #include #include +#include "transports/libhoth_usb.h" + #ifdef __cplusplus extern "C" { #endif @@ -72,6 +74,7 @@ struct libhoth_usb_device { libusb_context* ctx; libusb_device_handle* handle; uint32_t claim_timeout_us; + struct libhoth_usb_loc loc; struct libhoth_usb_interface_info info; union driver_data { struct libhoth_usb_mailbox mailbox;