Skip to content

Commit eb15453

Browse files
DarkFire01DarkFire01
authored
[USBPORT][USBHUB] Improve Unplug and Plugin stability (reactos#8624)
Dramatically improves stability when unplugging a bunch of devices at once (Except storage obviously) and implements primitive ESD handling.
1 parent c56950d commit eb15453

5 files changed

Lines changed: 199 additions & 40 deletions

File tree

‎drivers/usb/usbhub/power.c‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,33 @@ USBH_HubESDRecoverySetD3Completion(IN PDEVICE_OBJECT DeviceObject,
7272
FALSE);
7373
}
7474

75+
VOID
76+
NTAPI
77+
USBH_HubESDRecoverySetD0Completion(IN PDEVICE_OBJECT DeviceObject,
78+
IN UCHAR MinorFunction,
79+
IN POWER_STATE PowerState,
80+
IN PVOID Context,
81+
IN PIO_STATUS_BLOCK IoStatus)
82+
{
83+
PUSBHUB_FDO_EXTENSION HubExtension = (PUSBHUB_FDO_EXTENSION)Context;
84+
85+
DPRINT("USBH_HubESDRecoverySetD0Completion: HubExtension - %p\n", HubExtension);
86+
87+
if (HubExtension && NT_SUCCESS(IoStatus->Status))
88+
{
89+
/* Clear the ESD recovery flag after successful power cycle */
90+
HubExtension->HubFlags &= ~USBHUB_FDO_FLAG_ESD_RECOVERING;
91+
DPRINT("USBH_HubESDRecoverySetD0Completion: ESD recovery completed\n");
92+
}
93+
else if (HubExtension)
94+
{
95+
/* If D0 transition failed, clear the flag anyway to allow retry */
96+
HubExtension->HubFlags &= ~USBHUB_FDO_FLAG_ESD_RECOVERING;
97+
DPRINT1("USBH_HubESDRecoverySetD0Completion: ESD recovery failed with status 0x%08lx\n",
98+
IoStatus->Status);
99+
}
100+
}
101+
75102
NTSTATUS
76103
NTAPI
77104
USBH_HubSetD0(IN PUSBHUB_FDO_EXTENSION HubExtension)
@@ -130,6 +157,70 @@ USBH_HubSetD0(IN PUSBHUB_FDO_EXTENSION HubExtension)
130157
return Status;
131158
}
132159

160+
NTSTATUS
161+
NTAPI
162+
USBH_HubStartESDRecovery(IN PUSBHUB_FDO_EXTENSION HubExtension)
163+
{
164+
PUSBHUB_FDO_EXTENSION RootHubDevExt;
165+
NTSTATUS Status;
166+
KEVENT Event;
167+
POWER_STATE PowerState;
168+
169+
DPRINT("USBH_HubStartESDRecovery: HubExtension - %p\n", HubExtension);
170+
171+
RootHubDevExt = USBH_GetRootHubExtension(HubExtension);
172+
173+
if (RootHubDevExt->SystemPowerState.SystemState != PowerSystemWorking)
174+
{
175+
Status = STATUS_INVALID_DEVICE_STATE;
176+
HubExtension->HubFlags &= ~USBHUB_FDO_FLAG_ESD_RECOVERING;
177+
return Status;
178+
}
179+
180+
KeInitializeEvent(&Event, NotificationEvent, FALSE);
181+
182+
/* First, set the hub to D3 (power off) */
183+
PowerState.DeviceState = PowerDeviceD3;
184+
185+
Status = PoRequestPowerIrp(HubExtension->LowerPDO,
186+
IRP_MN_SET_POWER,
187+
PowerState,
188+
USBH_HubESDRecoverySetD3Completion,
189+
&Event,
190+
NULL);
191+
if (Status == STATUS_PENDING)
192+
{
193+
Status = KeWaitForSingleObject(&Event,
194+
Suspended,
195+
KernelMode,
196+
FALSE,
197+
NULL);
198+
}
199+
if (!NT_SUCCESS(Status))
200+
{
201+
DPRINT1("USBH_HubStartESDRecovery: Failed to set D3, Status - %lX\n", Status);
202+
HubExtension->HubFlags &= ~USBHUB_FDO_FLAG_ESD_RECOVERING;
203+
return Status;
204+
}
205+
206+
/* Now set it back to D0 (power on) to complete the recovery */
207+
PowerState.DeviceState = PowerDeviceD0;
208+
209+
Status = PoRequestPowerIrp(HubExtension->LowerPDO,
210+
IRP_MN_SET_POWER,
211+
PowerState,
212+
USBH_HubESDRecoverySetD0Completion,
213+
HubExtension,
214+
NULL);
215+
if (!NT_SUCCESS(Status) && Status != STATUS_PENDING)
216+
{
217+
DPRINT1("USBH_HubStartESDRecovery: Failed to set D0, Status - %lX\n", Status);
218+
HubExtension->HubFlags &= ~USBHUB_FDO_FLAG_ESD_RECOVERING;
219+
}
220+
221+
return Status;
222+
}
223+
133224
VOID
134225
NTAPI
135226
USBH_IdleCancelPowerHubWorker(IN PUSBHUB_FDO_EXTENSION HubExtension,

‎drivers/usb/usbhub/usbhub.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2234,8 +2234,10 @@ USBH_ChangeIndicationWorker(IN PUSBHUB_FDO_EXTENSION HubExtension,
22342234
{
22352235
HubExtension->HubFlags |= USBHUB_FDO_FLAG_ESD_RECOVERING;
22362236

2237-
DPRINT1("USBH_ChangeIndicationWorker: USBHUB_FDO_FLAG_ESD_RECOVERING FIXME\n");
2238-
DbgBreakPoint();
2237+
DPRINT("USBH_ChangeIndicationWorker: Starting ESD recovery\n");
2238+
2239+
/* Initiate ESD recovery: power cycle the hub (D3 -> D0) */
2240+
USBH_HubStartESDRecovery(HubExtension);
22392241

22402242
goto Exit;
22412243
}

‎drivers/usb/usbhub/usbhub.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,11 @@ NTAPI
335335
USBH_HubSetD0(
336336
IN PUSBHUB_FDO_EXTENSION HubExtension);
337337

338+
NTSTATUS
339+
NTAPI
340+
USBH_HubStartESDRecovery(
341+
IN PUSBHUB_FDO_EXTENSION HubExtension);
342+
338343
NTSTATUS
339344
NTAPI
340345
USBH_FdoPower(

‎drivers/usb/usbport/endpoint.c‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,17 @@ USBPORT_SetEndpointState(IN PUSBPORT_ENDPOINT Endpoint,
410410
Endpoint->FrameNumber = Packet->Get32BitFrameNumber(FdoExtension->MiniPortExt);
411411
KeReleaseSpinLock(&FdoExtension->MiniportSpinLock, OldIrql);
412412

413-
ExInterlockedInsertTailList(&FdoExtension->EpStateChangeList,
414-
&Endpoint->StateChangeLink,
415-
&FdoExtension->EpStateChangeSpinLock);
413+
KIRQL StateChangeOldIrql;
414+
KeAcquireSpinLock(&FdoExtension->EpStateChangeSpinLock, &StateChangeOldIrql);
415+
416+
if (Endpoint->StateChangeLink.Flink == NULL &&
417+
Endpoint->StateChangeLink.Blink == NULL)
418+
{
419+
InsertTailList(&FdoExtension->EpStateChangeList,
420+
&Endpoint->StateChangeLink);
421+
}
422+
423+
KeReleaseSpinLock(&FdoExtension->EpStateChangeSpinLock, StateChangeOldIrql);
416424

417425
KeAcquireSpinLock(&FdoExtension->MiniportSpinLock, &OldIrql);
418426
Packet->InterruptNextSOF(FdoExtension->MiniPortExt);
@@ -506,6 +514,24 @@ USBPORT_DeleteEndpoint(IN PDEVICE_OBJECT FdoDevice,
506514

507515
FdoExtension = FdoDevice->DeviceExtension;
508516

517+
/*
518+
* Remove endpoint from EpStateChangeList if it's still there.
519+
* This prevents the DPC handler from accessing a deleted endpoint.
520+
*/
521+
KIRQL StateChangeOldIrql;
522+
KeAcquireSpinLock(&FdoExtension->EpStateChangeSpinLock, &StateChangeOldIrql);
523+
524+
if (Endpoint->StateChangeLink.Flink != NULL &&
525+
Endpoint->StateChangeLink.Blink != NULL)
526+
{
527+
RemoveEntryList(&Endpoint->StateChangeLink);
528+
}
529+
530+
Endpoint->StateChangeLink.Flink = NULL;
531+
Endpoint->StateChangeLink.Blink = NULL;
532+
533+
KeReleaseSpinLock(&FdoExtension->EpStateChangeSpinLock, StateChangeOldIrql);
534+
509535
if ((Endpoint->WorkerLink.Flink && Endpoint->WorkerLink.Blink) ||
510536
Endpoint->LockCounter != -1)
511537
{

‎drivers/usb/usbport/usbport.c‎

Lines changed: 70 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -961,63 +961,98 @@ USBPORT_IsrDpcHandler(IN PDEVICE_OBJECT FdoDevice,
961961
return;
962962
}
963963

964-
for (List = ExInterlockedRemoveHeadList(&FdoExtension->EpStateChangeList,
965-
&FdoExtension->EpStateChangeSpinLock);
966-
List != NULL;
967-
List = ExInterlockedRemoveHeadList(&FdoExtension->EpStateChangeList,
968-
&FdoExtension->EpStateChangeSpinLock))
969-
{
964+
/* Process the state change list.
965+
* - Always process the list (don't flush during suspend)
966+
* - If controller is suspended/off, mark endpoints as ready immediately
967+
* - Don't request interrupts if suspended */
968+
KeAcquireSpinLockAtDpcLevel(&FdoExtension->EpStateChangeSpinLock);
969+
List = FdoExtension->EpStateChangeList.Flink;
970+
while (List != &FdoExtension->EpStateChangeList)
971+
{
972+
BOOLEAN EndpointReady = FALSE;
973+
PLIST_ENTRY NextList;
974+
BOOLEAN ControllerSuspended;
975+
970976
Endpoint = CONTAINING_RECORD(List,
971977
USBPORT_ENDPOINT,
972978
StateChangeLink);
973979

974980
DPRINT_CORE("USBPORT_IsrDpcHandler: Endpoint - %p\n", Endpoint);
975981

976-
KeAcquireSpinLockAtDpcLevel(&Endpoint->EndpointSpinLock);
982+
/* Save the next entry before we potentially remove this one */
983+
NextList = List->Flink;
977984

978-
KeAcquireSpinLockAtDpcLevel(&FdoExtension->MiniportSpinLock);
979-
FrameNumber = Packet->Get32BitFrameNumber(FdoExtension->MiniPortExt);
980-
KeReleaseSpinLockFromDpcLevel(&FdoExtension->MiniportSpinLock);
985+
/* Check if controller is suspended/off */
986+
ControllerSuspended = (FdoExtension->Flags & USBPORT_FLAG_HC_SUSPEND) != 0 ||
987+
(FdoExtension->CommonExtension.DevicePowerState == PowerDeviceD3);
981988

982-
if (FrameNumber <= Endpoint->FrameNumber &&
983-
!(Endpoint->Flags & ENDPOINT_FLAG_NUKE))
989+
if (ControllerSuspended)
990+
{
991+
/* Controller is suspended/off - mark endpoint as ready immediately */
992+
EndpointReady = TRUE;
993+
}
994+
else
984995
{
985-
KeReleaseSpinLockFromDpcLevel(&Endpoint->EndpointSpinLock);
986-
987-
ExInterlockedInsertHeadList(&FdoExtension->EpStateChangeList,
988-
&Endpoint->StateChangeLink,
989-
&FdoExtension->EpStateChangeSpinLock);
990-
991996
KeAcquireSpinLockAtDpcLevel(&FdoExtension->MiniportSpinLock);
992-
Packet->InterruptNextSOF(FdoExtension->MiniPortExt);
997+
FrameNumber = Packet->Get32BitFrameNumber(FdoExtension->MiniPortExt);
993998
KeReleaseSpinLockFromDpcLevel(&FdoExtension->MiniportSpinLock);
994999

995-
break;
1000+
/* Check if the endpoint is ready to be processed */
1001+
if (FrameNumber > Endpoint->FrameNumber ||
1002+
(Endpoint->Flags & ENDPOINT_FLAG_NUKE))
1003+
{
1004+
EndpointReady = TRUE;
1005+
}
9961006
}
9971007

998-
KeReleaseSpinLockFromDpcLevel(&Endpoint->EndpointSpinLock);
9991008

1000-
KeAcquireSpinLockAtDpcLevel(&Endpoint->StateChangeSpinLock);
1001-
Endpoint->StateLast = Endpoint->StateNext;
1002-
KeReleaseSpinLockFromDpcLevel(&Endpoint->StateChangeSpinLock);
1009+
if (EndpointReady)
1010+
{
1011+
/* Endpoint is ready - remove it from the list and process it */
1012+
RemoveEntryList(&Endpoint->StateChangeLink);
1013+
Endpoint->StateChangeLink.Flink = NULL;
1014+
Endpoint->StateChangeLink.Blink = NULL;
10031015

1004-
DPRINT_CORE("USBPORT_IsrDpcHandler: Endpoint->StateLast - %x\n",
1005-
Endpoint->StateLast);
1016+
KeAcquireSpinLockAtDpcLevel(&Endpoint->StateChangeSpinLock);
1017+
Endpoint->StateLast = Endpoint->StateNext;
1018+
KeReleaseSpinLockFromDpcLevel(&Endpoint->StateChangeSpinLock);
10061019

1007-
if (IsDpcHandler)
1008-
{
1009-
USBPORT_InvalidateEndpointHandler(FdoDevice,
1010-
Endpoint,
1011-
INVALIDATE_ENDPOINT_ONLY);
1020+
DPRINT_CORE("USBPORT_IsrDpcHandler: Endpoint->StateLast - %x\n",
1021+
Endpoint->StateLast);
1022+
1023+
KeReleaseSpinLockFromDpcLevel(&FdoExtension->EpStateChangeSpinLock);
1024+
1025+
if (IsDpcHandler)
1026+
{
1027+
USBPORT_InvalidateEndpointHandler(FdoDevice,
1028+
Endpoint,
1029+
INVALIDATE_ENDPOINT_ONLY);
1030+
}
1031+
else
1032+
{
1033+
USBPORT_InvalidateEndpointHandler(FdoDevice,
1034+
Endpoint,
1035+
INVALIDATE_ENDPOINT_WORKER_THREAD);
1036+
}
1037+
1038+
KeAcquireSpinLockAtDpcLevel(&FdoExtension->EpStateChangeSpinLock);
10121039
}
1013-
else
1040+
else if (!ControllerSuspended)
10141041
{
1015-
USBPORT_InvalidateEndpointHandler(FdoDevice,
1016-
Endpoint,
1017-
INVALIDATE_ENDPOINT_WORKER_THREAD);
1042+
/* Endpoint is not ready yet - leave it in the list and request interrupt.
1043+
* Don't request interrupts if controller is suspended. */
1044+
KeAcquireSpinLockAtDpcLevel(&FdoExtension->MiniportSpinLock);
1045+
Packet->InterruptNextSOF(FdoExtension->MiniPortExt);
1046+
KeReleaseSpinLockFromDpcLevel(&FdoExtension->MiniportSpinLock);
10181047
}
1048+
/* If suspended and not ready, leave it in list but don't request interrupt */
1049+
1050+
/* Move to the next entry */
1051+
List = NextList;
10191052
}
10201053

1054+
KeReleaseSpinLockFromDpcLevel(&FdoExtension->EpStateChangeSpinLock);
1055+
10211056
if (IsDpcHandler)
10221057
{
10231058
USBPORT_DpcHandler(FdoDevice);

0 commit comments

Comments
 (0)