From e44a4e8787b5388f63983c4460b495f53425162c Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Wed, 8 Jul 2015 22:36:00 -0700 Subject: [PATCH] libpayload: usb: xhci: Prevent address reuse We have been trying to avoid reassigning previously used USB addresses to different devices since CL:197420, because some devices seem to take issue with that. Unfortunately, that patch doesn't affect XHCI: those controllers insist on chosing addresses on their own. The only way to prevent them from reusing a previously assigned address is to not disable that slot at all. This patch implements address reuse avoidance on XHCI by not disabling slots when a device is detatched (which may occur both on physical detachment or if we simply couldn't find a driver for that device). Instead, we just release as many resources as we can for detached devices (by dropping all endpoint contexts) and defer the final cleanup until the point where the controller actually runs out of resources (a point that we probably don't often reach in most firmware scenarios). BRANCH=none BUG=chrome-os-partner:42181 TEST=Booted an Oak plugged into a Servo without having a driver for the SMSC network chip, observed that it could still enumerate the next device afterwards. Kept unplugging/replugging stuff until the cleanup triggered and made sure the controller still worked after that. Also played around a bit on a Falco without issues. Change-Id: Idfbab39abbc5bc5eff822bedf9c8d5bd4cad8cd2 Signed-off-by: Patrick Georgi Original-Commit-Id: 88c6bcbc41156729c3c38937c8a4adebc66f1ccb Original-Change-Id: I0653a4f6a02c02498210a70ffdda9d986592813b Original-Signed-off-by: Julius Werner Original-Reviewed-on: https://chromium-review.googlesource.com/284175 Original-Tested-by: Nicolas Boichat Original-Reviewed-by: Patrick Georgi Reviewed-on: http://review.coreboot.org/10957 Tested-by: build bot (Jenkins) Reviewed-by: Stefan Reinauer --- .../libpayload/drivers/usb/xhci_devconf.c | 77 ++++++++++++++++--- .../libpayload/drivers/usb/xhci_private.h | 5 +- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/payloads/libpayload/drivers/usb/xhci_devconf.c b/payloads/libpayload/drivers/usb/xhci_devconf.c index 5b5bb5e800..32cd2918c1 100644 --- a/payloads/libpayload/drivers/usb/xhci_devconf.c +++ b/payloads/libpayload/drivers/usb/xhci_devconf.c @@ -75,6 +75,31 @@ xhci_get_tt(xhci_t *const xhci, const usb_speed speed, return *tt != 0; } +static void +xhci_reap_slots(xhci_t *const xhci, int skip_slot) +{ + int i; + + xhci_debug("xHC resource shortage, trying to reap old slots...\n"); + for (i = 1; i <= xhci->max_slots_en; i++) { + if (i == skip_slot) + continue; /* don't reap slot we were working on */ + if (xhci->dev[i].transfer_rings[1]) + continue; /* slot still in use */ + if (!xhci->dev[i].ctx.raw) + continue; /* slot already disabled */ + + const int cc = xhci_cmd_disable_slot(xhci, i); + if (cc != CC_SUCCESS) + xhci_debug("Failed to disable slot %d: %d\n", i, cc); + else + xhci_spew("Successfully reaped slot %d\n", i); + xhci->dcbaa[i] = 0; + free(xhci->dev[i].ctx.raw); + xhci->dev[i].ctx.raw = NULL; + } +} + static inputctx_t * xhci_make_inputctx(const size_t ctxsize) { @@ -119,6 +144,10 @@ xhci_set_address (hci_t *controller, usb_speed speed, int hubport, int hubaddr) int slot_id; int cc = xhci_cmd_enable_slot(xhci, &slot_id); + if (cc == CC_NO_SLOTS_AVAILABLE) { + xhci_reap_slots(xhci, 0); + cc = xhci_cmd_enable_slot(xhci, &slot_id); + } if (cc != CC_SUCCESS) { xhci_debug("Enable slot failed: %d\n", cc); goto _free_return; @@ -163,6 +192,10 @@ xhci_set_address (hci_t *controller, usb_speed speed, int hubport, int hubaddr) xhci->dcbaa[slot_id] = virt_to_phys(di->ctx.raw); cc = xhci_cmd_address_device(xhci, slot_id, ic); + if (cc == CC_RESOURCE_ERROR) { + xhci_reap_slots(xhci, slot_id); + cc = xhci_cmd_address_device(xhci, slot_id, ic); + } if (cc != CC_SUCCESS) { xhci_debug("Address device failed: %d\n", cc); goto _disable_return; @@ -199,6 +232,10 @@ xhci_set_address (hci_t *controller, usb_speed speed, int hubport, int hubaddr) *ic->add = (1 << 1); /* EP0 Context */ EC_SET(MPS, ic->dev.ep0, dev->endpoints[0].maxpacketsize); cc = xhci_cmd_evaluate_context(xhci, slot_id, ic); + if (cc == CC_RESOURCE_ERROR) { + xhci_reap_slots(xhci, slot_id); + cc = xhci_cmd_evaluate_context(xhci, slot_id, ic); + } if (cc != CC_SUCCESS) { xhci_debug("Context evaluation failed: %d\n", cc); goto _disable_return; @@ -216,8 +253,10 @@ _free_return: if (tr) free((void *)tr->ring); free(tr); - if (di) + if (di) { free(di->ctx.raw); + di->ctx.raw = 0; + } _free_ic_return: if (ic) free(ic->raw); @@ -330,7 +369,8 @@ int xhci_finish_device_config(usbdev_t *const dev) { xhci_t *const xhci = XHCI_INST(dev->controller); - devinfo_t *const di = &xhci->dev[dev->address]; + int slot_id = dev->address; + devinfo_t *const di = &xhci->dev[slot_id]; int i, ret = 0; @@ -363,8 +403,11 @@ xhci_finish_device_config(usbdev_t *const dev) const int config_id = dev->configuration->bConfigurationValue; xhci_debug("config_id: %d\n", config_id); - const int cc = - xhci_cmd_configure_endpoint(xhci, dev->address, config_id, ic); + int cc = xhci_cmd_configure_endpoint(xhci, slot_id, config_id, ic); + if (cc == CC_RESOURCE_ERROR || cc == CC_BANDWIDTH_ERROR) { + xhci_reap_slots(xhci, slot_id); + cc = xhci_cmd_configure_endpoint(xhci, slot_id, config_id, ic); + } if (cc != CC_SUCCESS) { xhci_debug("Configure endpoint failed: %d\n", cc); ret = CONTROLLER_ERROR; @@ -396,19 +439,31 @@ xhci_destroy_dev(hci_t *const controller, const int slot_id) if (slot_id <= 0 || slot_id > xhci->max_slots_en) return; - int i; - - const int cc = xhci_cmd_disable_slot(xhci, slot_id); + inputctx_t *const ic = xhci_make_inputctx(CTXSIZE(xhci)); + if (!ic) { + xhci_debug("Out of memory, leaking resources!\n"); + return; + } + const int num_eps = controller->devices[slot_id]->num_endp; + *ic->add = 0; /* Leave Slot/EP0 state as it is for now. */ + *ic->drop = (1 << num_eps) - 1; /* Drop all endpoints we can. */ + *ic->drop &= ~(1 << 1 | 1 << 0); /* Not allowed to drop EP0 or Slot. */ + int cc = xhci_cmd_evaluate_context(xhci, slot_id, ic); if (cc != CC_SUCCESS) - xhci_debug("Failed to disable slot %d: %d\n", slot_id, cc); + xhci_debug("Failed to quiesce slot %d: %d\n", slot_id, cc); + cc = xhci_cmd_stop_endpoint(xhci, slot_id, 1); + if (cc != CC_SUCCESS) + xhci_debug("Failed to stop EP0 on slot %d: %d\n", slot_id, cc); + int i; devinfo_t *const di = &xhci->dev[slot_id]; - for (i = 1; i < 31; ++i) { + for (i = 1; i < num_eps; ++i) { if (di->transfer_rings[i]) free((void *)di->transfer_rings[i]->ring); free(di->transfer_rings[i]); - free(di->interrupt_queues[i]); } - xhci->dcbaa[slot_id] = 0; + + xhci_spew("Stopped slot %d, but not disabling it yet.\n", slot_id); + di->transfer_rings[1] = NULL; } diff --git a/payloads/libpayload/drivers/usb/xhci_private.h b/payloads/libpayload/drivers/usb/xhci_private.h index f01a37f842..26f7666539 100644 --- a/payloads/libpayload/drivers/usb/xhci_private.h +++ b/payloads/libpayload/drivers/usb/xhci_private.h @@ -55,6 +55,9 @@ #define CC_SUCCESS 1 #define CC_TRB_ERROR 5 #define CC_STALL_ERROR 6 +#define CC_RESOURCE_ERROR 7 +#define CC_BANDWIDTH_ERROR 8 +#define CC_NO_SLOTS_AVAILABLE 9 #define CC_SHORT_PACKET 13 #define CC_EVENT_RING_FULL_ERROR 21 #define CC_COMMAND_RING_STOPPED 24 @@ -307,7 +310,7 @@ typedef struct intrq { typedef struct devinfo { devctx_t ctx; transfer_ring_t *transfer_rings[NUM_EPS]; - intrq_t *interrupt_queues[32]; + intrq_t *interrupt_queues[NUM_EPS]; } devinfo_t; typedef struct erst_entry {