From b3683d8aeed4214072835c4625f36a4b99c1ebf3 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Fri, 17 Sep 2021 13:33:02 -0400 Subject: [PATCH] XHCI: Get rid of the PortSlots array and use ControllerCookie in FreeDevice. This array was introduced by korli in hrev44089~1 (2012). It "mapped" ports to slots using a device's HubPort, and then used this value in FreeDevice() to locate the xhci_device struct in question. Well, when there are non-root hubs in use, the HubPort values can of course collide, leading us to tear down the wrong device in many circumstances. This appears to have been the true cause of #16794, and probably also #16878 and #17266, and maybe even some others. --- src/add-ons/kernel/busses/usb/xhci.cpp | 24 ++++++++++-------------- src/add-ons/kernel/busses/usb/xhci.h | 1 - 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp b/src/add-ons/kernel/busses/usb/xhci.cpp index 157de3d366..74bc18fcec 100644 --- a/src/add-ons/kernel/busses/usb/xhci.cpp +++ b/src/add-ons/kernel/busses/usb/xhci.cpp @@ -402,7 +402,6 @@ XHCI::XHCI(pci_info *info, Stack *stack) install_io_interrupt_handler(fIRQ, InterruptHandler, (void *)this, 0); memset(fPortSpeeds, 0, sizeof(fPortSpeeds)); - memset(fPortSlots, 0, sizeof(fPortSlots)); memset(fDevices, 0, sizeof(fDevices)); fInitOK = true; @@ -1656,31 +1655,28 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress, uint8 hubPort, // otherwise happen when this Pipe object is destroyed. pipe.SetControllerCookie(NULL); - fPortSlots[hubPort] = slot; TRACE("AllocateDevice() port %d slot %d\n", hubPort, slot); return deviceObject; } void -XHCI::FreeDevice(Device *device) +XHCI::FreeDevice(Device *usbDevice) { - uint8 hubPort = device->HubPort(); - uint8 slot = fPortSlots[hubPort]; - TRACE("FreeDevice() port %d slot %d\n", hubPort, slot); + xhci_device* device = (xhci_device*)usbDevice->ControllerCookie(); + TRACE("FreeDevice() slot %d\n", device->slot); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. - delete device; + delete usbDevice; - DisableSlot(slot); - fDcba->baseAddress[slot] = 0; - fPortSlots[hubPort] = 0; - delete_area(fDevices[slot].trb_area); - delete_area(fDevices[slot].input_ctx_area); - delete_area(fDevices[slot].device_ctx_area); + DisableSlot(device->slot); + fDcba->baseAddress[device->slot] = 0; + delete_area(device->trb_area); + delete_area(device->input_ctx_area); + delete_area(device->device_ctx_area); - memset(&fDevices[slot], 0, sizeof(xhci_device)); + memset(device, 0, sizeof(xhci_device)); } diff --git a/src/add-ons/kernel/busses/usb/xhci.h b/src/add-ons/kernel/busses/usb/xhci.h index d1974fbfe3..fbf6d43e7a 100644 --- a/src/add-ons/kernel/busses/usb/xhci.h +++ b/src/add-ons/kernel/busses/usb/xhci.h @@ -257,7 +257,6 @@ private: uint8 fPortCount; uint8 fSlotCount; usb_speed fPortSpeeds[XHCI_MAX_PORTS]; - uint8 fPortSlots[XHCI_MAX_PORTS]; // Scratchpad uint32 fScratchpadCount;