Ros-diffs August 2016

ros-diffs@reactos.org

20 participants
462 discussions

[vmikayelyan] 72388: usb: HUB: Add PnP state tracking Added PDO/FDO PnP state tracking, which is done according MSDN's "State Transitions for PnP Devices" topic.

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:46:18 2016 New Revision: 72388 URL: http://svn.reactos.org/svn/reactos?rev=72388&view=rev Log: usb: HUB: Add PnP state tracking Added PDO/FDO PnP state tracking, which is done according MSDN's "State Transitions for PnP Devices" topic. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:46:18 2016 @@ -1322,7 +1322,7 @@ RtlCopyMemory(&UsbChildExtension->DeviceInterface, &HubDeviceExtension->UsbDInterface, sizeof(USB_BUS_INTERFACE_USBDI_V2)); UsbChildExtension->DeviceInterface.InterfaceReference(UsbChildExtension->DeviceInterface.BusContext); - UsbChildExtension->IsRemovePending = FALSE; + INITIALIZE_PNP_STATE(UsbChildExtension->Common); KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); @@ -2053,6 +2053,8 @@ Status = USBHUB_ParentFDOStartDevice(DeviceObject, Irp); } + SET_NEW_PNP_STATE(HubDeviceExtension->Common, Started); + Irp->IoStatus.Status = Status; IoCompleteRequest(Irp, IO_NO_INCREMENT); return Status; @@ -2116,6 +2118,18 @@ // No action is required from FDO because it have nothing to free. DPRINT("IRP_MN_QUERY_REMOVE_DEVICE\n"); + SET_NEW_PNP_STATE(HubDeviceExtension->Common, RemovePending); + + Irp->IoStatus.Status = STATUS_SUCCESS; + break; + } + case IRP_MN_CANCEL_REMOVE_DEVICE: + { + DPRINT("IRP_MN_CANCEL_REMOVE_DEVICE\n"); + + if (HubDeviceExtension->Common.PnPState == RemovePending) + RESTORE_PREVIOUS_PNP_STATE(HubDeviceExtension->Common); + Irp->IoStatus.Status = STATUS_SUCCESS; break; } @@ -2127,6 +2141,7 @@ // our children that their parent is removed and on next removal // they also can be removed. // + SET_NEW_PNP_STATE(HubDeviceExtension->Common, SurpriseRemovePending); KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] Fri Aug 19 16:46:18 2016 @@ -200,7 +200,9 @@ ChildDeviceExtension = (PHUB_CHILDDEVICE_EXTENSION)DeviceObject->DeviceExtension; ASSERT(ChildDeviceExtension->Common.IsFDO == FALSE); - if (ChildDeviceExtension->IsRemovePending || !IsValidPDO(DeviceObject)) + if (ChildDeviceExtension->Common.PnPState == SurpriseRemovePending || + ChildDeviceExtension->Common.PnPState == RemovePending || + !IsValidPDO(DeviceObject)) { // Parent or child device was surprise removed. DPRINT1("[USBHUB] Request for removed device object %p\n", DeviceObject); @@ -431,6 +433,8 @@ // IoRegisterDeviceInterface(DeviceObject, &GUID_DEVINTERFACE_USB_DEVICE, NULL, &ChildDeviceExtension->SymbolicLinkName); IoSetDeviceInterfaceState(&ChildDeviceExtension->SymbolicLinkName, TRUE); + + SET_NEW_PNP_STATE(ChildDeviceExtension->Common, Started); UNIMPLEMENTED return STATUS_SUCCESS; @@ -668,9 +672,15 @@ DPRINT("IRP_MJ_PNP / IRP_MN_REMOVE_DEVICE\n"); + ASSERT((UsbChildExtension->Common.PnPState == RemovePending) || + (UsbChildExtension->Common.PnPState == SurpriseRemovePending)); + + SET_NEW_PNP_STATE(UsbChildExtension->Common, NotStarted); + if (!IsValidPDO(DeviceObject)) { // Parent or child device was surprise removed, freeing resources allocated for child device. + SET_NEW_PNP_STATE(UsbChildExtension->Common, Deleted); // Remove the usb device if (UsbChildExtension->UsbDeviceHandle) @@ -699,8 +709,7 @@ IoDeleteDevice(DeviceObject); } - // Device is physically presented, so we leave its PDO undeleted. - ASSERT(UsbChildExtension->IsRemovePending == TRUE); + // If device is physically presented, we leave its PDO undeleted. /* Complete the IRP */ Irp->IoStatus.Status = STATUS_SUCCESS; @@ -760,7 +769,7 @@ // UsbChildExtension->DeviceInterface.InterfaceDereference(UsbChildExtension->DeviceInterface.BusContext); - UsbChildExtension->IsRemovePending = TRUE; + SET_NEW_PNP_STATE(UsbChildExtension->Common, RemovePending); /* Sure, no problem */ Status = STATUS_SUCCESS; @@ -770,9 +779,9 @@ case IRP_MN_CANCEL_REMOVE_DEVICE: { // Check to see have we received query-remove before - if (UsbChildExtension->IsRemovePending == TRUE) - { - UsbChildExtension->IsRemovePending = FALSE; + if (UsbChildExtension->Common.PnPState == RemovePending) + { + RESTORE_PREVIOUS_PNP_STATE(UsbChildExtension->Common); UsbChildExtension->DeviceInterface.InterfaceReference(UsbChildExtension->DeviceInterface.BusContext); } @@ -804,7 +813,7 @@ // the flag and do further clean-up in subsequent IRP_MN_REMOVE_DEVICE // We can receive this IRP when device is physically connected (on stop/start fail). // - UsbChildExtension->IsRemovePending = TRUE; + SET_NEW_PNP_STATE(UsbChildExtension->Common, SurpriseRemovePending); Status = STATUS_SUCCESS; break; Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c [iso-8859-1] Fri Aug 19 16:46:18 2016 @@ -86,6 +86,8 @@ HubDeviceExtension = (PHUB_DEVICE_EXTENSION)DeviceObject->DeviceExtension; RtlZeroMemory(HubDeviceExtension, sizeof(HUB_DEVICE_EXTENSION)); + INITIALIZE_PNP_STATE(HubDeviceExtension->Common); + // // Set this to Fdo // Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] Fri Aug 19 16:46:18 2016 @@ -39,9 +39,43 @@ PVOID Context; } WORK_ITEM_DATA, *PWORK_ITEM_DATA; + +// +// Definitions for device's PnP state tracking, all this states are described +// in PnP Device States diagram of DDK documentation. +// +typedef enum _DEVICE_PNP_STATE { + + NotStarted = 0, // Not started + Started, // After handling of START_DEVICE IRP + StopPending, // After handling of QUERY_STOP IRP + Stopped, // After handling of STOP_DEVICE IRP + RemovePending, // After handling of QUERY_REMOVE IRP + SurpriseRemovePending, // After handling of SURPRISE_REMOVE IRP + Deleted, // After handling of REMOVE_DEVICE IRP + UnKnown // Unknown state + +} DEVICE_PNP_STATE; + +#define INITIALIZE_PNP_STATE(Data) \ +(Data).PnPState = NotStarted;\ +(Data).PreviousPnPState = NotStarted; + +#define SET_NEW_PNP_STATE(Data, state) \ +(Data).PnPState = (Data).PnPState;\ +(Data).PnPState = (state); + +#define RESTORE_PREVIOUS_PNP_STATE(Data) \ +(Data).PnPState = (Data).PreviousPnPState; + typedef struct { BOOLEAN IsFDO; + // We'll track device PnP state via this variables + DEVICE_PNP_STATE PnPState; + DEVICE_PNP_STATE PreviousPnPState; + // Remove lock + IO_REMOVE_LOCK RemoveLock; } COMMON_DEVICE_EXTENSION, *PCOMMON_DEVICE_EXTENSION; typedef struct _HUB_CHILDDEVICE_EXTENSION @@ -61,7 +95,6 @@ UNICODE_STRING SymbolicLinkName; USB_BUS_INTERFACE_USBDI_V2 DeviceInterface; USB_DEVICE_INFORMATION_0 DeviceInformation; - BOOLEAN IsRemovePending; } HUB_CHILDDEVICE_EXTENSION, *PHUB_CHILDDEVICE_EXTENSION; typedef struct _HUB_DEVICE_EXTENSION

8 years, 4 months

[vmikayelyan] 72387: usb: hub: FDO: Handle surprise removal, fail stop request.

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:38:11 2016 New Revision: 72387 URL: http://svn.reactos.org/svn/reactos?rev=72387&view=rev Log: usb: hub: FDO: Handle surprise removal, fail stop request. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:38:11 2016 @@ -2030,7 +2030,9 @@ { PIO_STACK_LOCATION Stack; NTSTATUS Status = STATUS_SUCCESS; + PDEVICE_OBJECT ChildDeviceObject; PHUB_DEVICE_EXTENSION HubDeviceExtension; + PHUB_CHILDDEVICE_EXTENSION ChildDeviceExtension; HubDeviceExtension = (PHUB_DEVICE_EXTENSION) DeviceObject->DeviceExtension; @@ -2098,10 +2100,49 @@ } break; } + case IRP_MN_QUERY_STOP_DEVICE: + { + // + // We should fail this request, because we're not handling + // IRP_MN_STOP_DEVICE for now.We'll receive this IRP ONLY when + // PnP manager rebalances resources. + // + Irp->IoStatus.Status = STATUS_NOT_SUPPORTED; + IoCompleteRequest(Irp, IO_NO_INCREMENT); + return STATUS_NOT_SUPPORTED; + } case IRP_MN_QUERY_REMOVE_DEVICE: - case IRP_MN_QUERY_STOP_DEVICE: - { - DPRINT("IRP_MN_QUERY_STOP_DEVICE\n"); + { + // No action is required from FDO because it have nothing to free. + DPRINT("IRP_MN_QUERY_REMOVE_DEVICE\n"); + + Irp->IoStatus.Status = STATUS_SUCCESS; + break; + } + case IRP_MN_SURPRISE_REMOVAL: + { + // + // We'll receive this IRP on HUB unexpected removal, or on USB + // controller removal from PCI port. Here we should "let know" all + // our children that their parent is removed and on next removal + // they also can be removed. + // + + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); + + for (int i = 0; i < USB_MAXCHILDREN; i++) + { + ChildDeviceObject = HubDeviceExtension->ChildDeviceObject[i]; + if (ChildDeviceObject) + { + ChildDeviceExtension = (PHUB_CHILDDEVICE_EXTENSION)ChildDeviceObject->DeviceObjectExtension; + ChildDeviceExtension->ParentDeviceObject = NULL; + } + } + + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); + + // This IRP can't be failed Irp->IoStatus.Status = STATUS_SUCCESS; break; }

8 years, 4 months

[vmikayelyan] 72386: usb: hub: Add sychronization for FDO's child list access. As FDO's child list is getting changed not from PnP dispatcher but from DeviceStatusChangeThread(), we need to sychron...

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:36:43 2016 New Revision: 72386 URL: http://svn.reactos.org/svn/reactos?rev=72386&view=rev Log: usb: hub: Add sychronization for FDO's child list access. As FDO's child list is getting changed not from PnP dispatcher but from DeviceStatusChangeThread(), we need to sychronize access for it. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:36:43 2016 @@ -1094,6 +1094,7 @@ PDEVICE_OBJECT ChildDeviceObject = NULL; ULONG Index = 0; + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); for (Index = 0; Index < USB_MAXCHILDREN; Index++) { if (HubDeviceExtension->ChildDeviceObject[Index]) @@ -1114,14 +1115,16 @@ if (!ChildDeviceObject) { DPRINT1("Removal request for non-existant device!\n"); + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); return STATUS_UNSUCCESSFUL; } DPRINT("Removing device on port %d (Child index: %d)\n", PortId, Index); /* Remove the device from the table */ - // is lock needed? HubDeviceExtension->ChildDeviceObject[Index] = NULL; + + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); /* Invalidate device relations for the root hub */ IoInvalidateDeviceRelations(HubDeviceExtension->RootHubPhysicalDeviceObject, BusRelations); @@ -1153,26 +1156,6 @@ HubInterface = &HubDeviceExtension->HubInterface; RootHubDeviceObject = HubDeviceExtension->RootHubPhysicalDeviceObject; HubInterfaceBusContext = HubDeviceExtension->UsbDInterface.BusContext; - // - // Find an empty slot in the child device array - // - for (ChildDeviceCount = 0; ChildDeviceCount < USB_MAXCHILDREN; ChildDeviceCount++) - { - if (HubDeviceExtension->ChildDeviceObject[ChildDeviceCount] == NULL) - { - DPRINT("Found unused entry at %d\n", ChildDeviceCount); - break; - } - } - - // - // Check if the limit has been reached for maximum usb devices - // - if (ChildDeviceCount == USB_MAXCHILDREN) - { - DPRINT1("USBHUB: Too many child devices!\n"); - return STATUS_UNSUCCESSFUL; - } while (TRUE) { @@ -1341,8 +1324,35 @@ UsbChildExtension->IsRemovePending = FALSE; + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); + + // + // Find an empty slot in the child device array + // + for (ChildDeviceCount = 0; ChildDeviceCount < USB_MAXCHILDREN; ChildDeviceCount++) + { + if (HubDeviceExtension->ChildDeviceObject[ChildDeviceCount] == NULL) + { + DPRINT("Found unused entry at %d\n", ChildDeviceCount); + break; + } + } + + // + // Check if the limit has been reached for maximum usb devices + // + if (ChildDeviceCount == USB_MAXCHILDREN) + { + DPRINT1("USBHUB: Too many child devices!\n"); + Status = STATUS_UNSUCCESSFUL; + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); + UsbChildExtension->DeviceInterface.InterfaceDereference(UsbChildExtension->DeviceInterface.BusContext); + goto Cleanup; + } + HubDeviceExtension->ChildDeviceObject[ChildDeviceCount] = NewChildDeviceObject; HubDeviceExtension->InstanceCount++; + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); IoInvalidateDeviceRelations(RootHubDeviceObject, BusRelations); return STATUS_SUCCESS; @@ -1398,6 +1408,8 @@ HubDeviceExtension = (PHUB_DEVICE_EXTENSION)DeviceObject->DeviceExtension; + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); + // // Count the number of children // @@ -1418,6 +1430,7 @@ { // We have nothing to add *pDeviceRelations = RelationsFromTop; + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); return STATUS_SUCCESS; } } @@ -1432,6 +1445,7 @@ if (!DeviceRelations) { + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); if (!RelationsFromTop) return STATUS_INSUFFICIENT_RESOURCES; else @@ -1460,6 +1474,8 @@ DeviceRelations->Objects[Children++] = HubDeviceExtension->ChildDeviceObject[i]; } } + + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); // We should do this, because replaced this with our's one if (RelationsFromTop) @@ -2198,6 +2214,7 @@ // sanity checks ASSERT(NodeConnectionInfo); + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); for(Index = 0; Index < USB_MAXCHILDREN; Index++) { if (HubDeviceExtension->ChildDeviceObject[Index] == NULL) @@ -2224,6 +2241,7 @@ } break; } + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); // done Irp->IoStatus.Information = sizeof(USB_NODE_INFORMATION); Status = STATUS_SUCCESS; @@ -2244,6 +2262,7 @@ // sanity checks ASSERT(NodeKey); + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); for(Index = 0; Index < USB_MAXCHILDREN; Index++) { if (HubDeviceExtension->ChildDeviceObject[Index] == NULL) @@ -2283,6 +2302,7 @@ NodeKey->ActualLength = Length + sizeof(USB_NODE_CONNECTION_DRIVERKEY_NAME); break; } + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); } } else if (IoStack->Parameters.DeviceIoControl.IoControlCode == IOCTL_USB_GET_NODE_CONNECTION_NAME) Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] Fri Aug 19 16:36:43 2016 @@ -153,14 +153,18 @@ HubDeviceExtension = (PHUB_DEVICE_EXTENSION)ChildDeviceExtension->ParentDeviceObject->DeviceExtension; + KeAcquireGuardedMutex(&HubDeviceExtension->HubMutexLock); for(Index = 0; Index < USB_MAXCHILDREN; Index++) { if (HubDeviceExtension->ChildDeviceObject[Index] == DeviceObject) { + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); + /* PDO exists */ return TRUE; } } + KeReleaseGuardedMutex(&HubDeviceExtension->HubMutexLock); /* invalid pdo */ return FALSE; Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.c [iso-8859-1] Fri Aug 19 16:36:43 2016 @@ -92,6 +92,9 @@ HubDeviceExtension->Common.IsFDO = TRUE; DeviceObject->Flags |= DO_POWER_PAGABLE; + // initialize mutex + KeInitializeGuardedMutex(&HubDeviceExtension->HubMutexLock); + // // initialize reset complete event // Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] Fri Aug 19 16:36:43 2016 @@ -72,6 +72,8 @@ PDEVICE_OBJECT ChildDeviceObject[USB_MAXCHILDREN]; PDEVICE_OBJECT RootHubPhysicalDeviceObject; PDEVICE_OBJECT RootHubFunctionalDeviceObject; + + KGUARDED_MUTEX HubMutexLock; ULONG NumberOfHubs; KEVENT ResetComplete;

8 years, 4 months

[vmikayelyan] 72385: Change CMakLists to use GuardedMutexes

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:35:06 2016 New Revision: 72385 URL: http://svn.reactos.org/svn/reactos?rev=72385&view=rev Log: Change CMakLists to use GuardedMutexes Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/CMakeLists.txt Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/CMakeLists.txt URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/CMakeLists.txt [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/CMakeLists.txt [iso-8859-1] Fri Aug 19 16:35:06 2016 @@ -1,5 +1,6 @@ add_definitions(-DDEBUG_MODE) +add_definitions(-DNTDDI_VERSION=0x05020400) include_directories(${REACTOS_SOURCE_DIR}/ntoskrnl/include) list(APPEND SOURCE

8 years, 4 months

[vmikayelyan] 72384: usb: hub: PDO: Refernce interface before passing up

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:33:34 2016 New Revision: 72384 URL: http://svn.reactos.org/svn/reactos?rev=72384&view=rev Log: usb: hub: PDO: Refernce interface before passing up Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] Fri Aug 19 16:33:34 2016 @@ -782,6 +782,7 @@ { DPRINT1("USB_BUS_INTERFACE_USBDI_GUID\n"); RtlCopyMemory(Stack->Parameters.QueryInterface.Interface, &UsbChildExtension->DeviceInterface, Stack->Parameters.QueryInterface.Size); + UsbChildExtension->DeviceInterface.InterfaceReference(UsbChildExtension->DeviceInterface.BusContext); Status = STATUS_SUCCESS; break; }

8 years, 4 months

[vmikayelyan] 72383: usb: hub: PDO: Added query-remove and query-cancel-remove handlers On IRP_MN_QUERY_REMOVE_DEVICE we are freeing interface obtained from bottom according MSDN we should check in...

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:30:53 2016 New Revision: 72383 URL: http://svn.reactos.org/svn/reactos?rev=72383&view=rev Log: usb: hub: PDO: Added query-remove and query-cancel-remove handlers On IRP_MN_QUERY_REMOVE_DEVICE we are freeing interface obtained from bottom according MSDN we should check interfaces provided to top, but here we are not checking. All checking will be performed in roothub driver's IRP_MN_QUERY_REMOVE_DEVICE handler. This will make problems when buggy driver is loaded on top of us. But we decided to keep source simpler, because in any case buggy driver will prevent removing of whole stack. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/pdo.c [iso-8859-1] Fri Aug 19 16:30:53 2016 @@ -745,14 +745,34 @@ } case IRP_MN_QUERY_REMOVE_DEVICE: { - // HERE SHOULD BE CHECKED INTERFACE COUNT PROVIED TO UPPER LAYER TO BE ZERO, AS WE ARE HANDLING - // IRP_MN_QUERY_INTERFACE IN WRONG WAY, THAT WILL BE DONE LATER. SEE MSDN "IRP_MN_QUERY_INTERFACE" + // + // Free interface obtained from bottom, according MSDN we should + // check interfaces provided to top, but here we are not checking. + // All checking will be performed in roothub driver's + // IRP_MN_QUERY_REMOVE_DEVICE handler. This will make problems when + // buggy driver is loaded on top of us. But we decided to keep source + // simpler, because in any case buggy driver will prevent removing of + // whole stack. + // + UsbChildExtension->DeviceInterface.InterfaceDereference(UsbChildExtension->DeviceInterface.BusContext); UsbChildExtension->IsRemovePending = TRUE; /* Sure, no problem */ Status = STATUS_SUCCESS; Information = 0; + break; + } + case IRP_MN_CANCEL_REMOVE_DEVICE: + { + // Check to see have we received query-remove before + if (UsbChildExtension->IsRemovePending == TRUE) + { + UsbChildExtension->IsRemovePending = FALSE; + UsbChildExtension->DeviceInterface.InterfaceReference(UsbChildExtension->DeviceInterface.BusContext); + } + + Status = STATUS_SUCCESS; break; } case IRP_MN_QUERY_INTERFACE:

8 years, 4 months

[vmikayelyan] 72382: usb: hub: FDO: Removed dublicated functions and fields FDO_QueryInterface() is unuseful beacuse we have more generic QueryInterface() function which is able to replace all func...

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:29:16 2016 New Revision: 72382 URL: http://svn.reactos.org/svn/reactos?rev=72382&view=rev Log: usb: hub: FDO: Removed dublicated functions and fields FDO_QueryInterface() is unuseful beacuse we have more generic QueryInterface() function which is able to replace all functionality which provides FDO_QueryInterface(). Also we do not need to have two "USB_BUS_INTERFACE_USBDI_V2" fields in "HUB_DEVICE_EXTENSION". FDO_QueryInterface() was added in svn-rev 55983, and on review of that commit I have found some copypasta issues and also tricks which was not documented and marked by me as wrong. For example changing if buscontext which was fixed in previous commits of this commit set. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c branches/GSoC_2016/USB/drivers/usb/usbhub/misc.c branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:29:16 2016 @@ -1889,8 +1889,6 @@ HubDeviceExtension->ConfigurationHandle = ConfigUrb->UrbSelectConfiguration.ConfigurationHandle; HubDeviceExtension->PipeHandle = ConfigUrb->UrbSelectConfiguration.Interface.Pipes[0].PipeHandle; DPRINT("Configuration Handle %x\n", HubDeviceExtension->ConfigurationHandle); - - FDO_QueryInterface(DeviceObject, &HubDeviceExtension->DeviceInterface); // check if function is available if (HubDeviceExtension->UsbDInterface.IsDeviceHighSpeed) Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/misc.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/misc.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/misc.c [iso-8859-1] Fri Aug 19 16:29:16 2016 @@ -211,84 +211,3 @@ return Status; } - -NTSTATUS -NTAPI -FDO_QueryInterfaceCompletionRoutine( - IN PDEVICE_OBJECT DeviceObject, - IN PIRP Irp, - IN PVOID Context) -{ - /* Set event */ - KeSetEvent((PRKEVENT)Context, 0, FALSE); - - /* Completion is done in the HidClassFDO_QueryCapabilities routine */ - return STATUS_MORE_PROCESSING_REQUIRED; -} - -NTSTATUS -FDO_QueryInterface( - IN PDEVICE_OBJECT DeviceObject, - IN OUT PUSB_BUS_INTERFACE_USBDI_V2 Interface) -{ - PIRP Irp; - KEVENT Event; - NTSTATUS Status; - PIO_STACK_LOCATION IoStack; - PHUB_DEVICE_EXTENSION HubDeviceExtension; - - /* Get device extension */ - HubDeviceExtension = (PHUB_DEVICE_EXTENSION)DeviceObject->DeviceExtension; - ASSERT(HubDeviceExtension->Common.IsFDO); - - /* Init event */ - KeInitializeEvent(&Event, NotificationEvent, FALSE); - - /* Now allocte the irp */ - Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) - { - /* No memory */ - return STATUS_INSUFFICIENT_RESOURCES; - } - - /* Get next stack location */ - IoStack = IoGetNextIrpStackLocation(Irp); - - /* Init stack location */ - IoStack->MajorFunction = IRP_MJ_PNP; - IoStack->MinorFunction = IRP_MN_QUERY_INTERFACE; - IoStack->Parameters.QueryInterface.Interface = (PINTERFACE)Interface; - IoStack->Parameters.QueryInterface.InterfaceType = &USB_BUS_INTERFACE_USBDI_GUID; - IoStack->Parameters.QueryInterface.Version = USB_BUSIF_USBDI_VERSION_2; - IoStack->Parameters.QueryInterface.Size = sizeof(USB_BUS_INTERFACE_USBDI_V2); - - - /* Set completion routine */ - IoSetCompletionRoutine(Irp, - FDO_QueryInterfaceCompletionRoutine, - (PVOID)&Event, - TRUE, - TRUE, - TRUE); - - /* Pnp irps have default completion code */ - Irp->IoStatus.Status = STATUS_NOT_SUPPORTED; - - /* Call lower device */ - Status = IoCallDriver(HubDeviceExtension->LowerDeviceObject, Irp); - if (Status == STATUS_PENDING) - { - /* Wait for completion */ - KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL); - } - - /* Get status */ - Status = Irp->IoStatus.Status; - - /* Complete request */ - IoFreeIrp(Irp); - - /* Done */ - return Status; -} Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/usbhub.h [iso-8859-1] Fri Aug 19 16:29:16 2016 @@ -95,7 +95,6 @@ USBD_CONFIGURATION_HANDLE ConfigurationHandle; USBD_PIPE_HANDLE PipeHandle; PVOID RootHubHandle; - USB_BUS_INTERFACE_USBDI_V2 DeviceInterface; UNICODE_STRING SymbolicLinkName; ULONG InstanceCount;

8 years, 4 months

[vmikayelyan] 72381: usb: hub: FDO: Fix USBHUB_FdoStartDevice() Done fixes in cleanup also refactored code to be more readable and error safe.

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:27:37 2016 New Revision: 72381 URL: http://svn.reactos.org/svn/reactos?rev=72381&view=rev Log: usb: hub: FDO: Fix USBHUB_FdoStartDevice() Done fixes in cleanup also refactored code to be more readable and error safe. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:27:37 2016 @@ -1581,7 +1581,8 @@ if (!Urb) { // no memory - return STATUS_INSUFFICIENT_RESOURCES; + Status = STATUS_INSUFFICIENT_RESOURCES; + goto cleanup; } // zero urb @@ -1596,8 +1597,7 @@ { // failed to obtain hub pdo DPRINT1("IOCTL_INTERNAL_USB_GET_ROOTHUB_PDO failed with %x\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } // sanity checks @@ -1614,8 +1614,7 @@ { // failed to start pdo DPRINT1("Failed to start the RootHub PDO\n"); - ExFreePool(Urb); - return Status; + goto cleanup; } // Get the current number of hubs @@ -1626,8 +1625,7 @@ { // failed to get number of hubs DPRINT1("IOCTL_INTERNAL_USB_GET_HUB_COUNT failed with %x\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } // Get the Hub Interface @@ -1641,8 +1639,7 @@ { // failed to get root hub interface DPRINT1("Failed to get HUB_GUID interface with status 0x%08lx\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } HubInterfaceBusContext = HubDeviceExtension->HubInterface.BusContext; @@ -1658,8 +1655,7 @@ { // failed to get usbdi interface DPRINT1("Failed to get USBDI_GUID interface with status 0x%08lx\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } // Get Root Hub Device Handle @@ -1672,8 +1668,7 @@ { // failed DPRINT1("IOCTL_INTERNAL_USB_GET_DEVICE_HANDLE failed with status 0x%08lx\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } // @@ -1717,8 +1712,7 @@ { // failed to get device descriptor of hub DPRINT1("Failed to get HubDeviceDescriptor!\n"); - ExFreePool(Urb); - return Status; + goto cleanup; } // build configuration request @@ -1745,8 +1739,7 @@ { // failed to get configuration descriptor DPRINT1("Failed to get RootHub Configuration with status %x\n", Status); - ExFreePool(Urb); - return Status; + goto cleanup; } // sanity checks @@ -1772,16 +1765,15 @@ { // failed to get hub information DPRINT1("Failed to extended hub information. Unable to determine the number of ports!\n"); - ExFreePool(Urb); - return Status; + goto cleanup; } if (!HubDeviceExtension->UsbExtHubInfo.NumberOfPorts) { // bogus port driver DPRINT1("Failed to retrieve the number of ports\n"); - ExFreePool(Urb); - return STATUS_UNSUCCESSFUL; + Status = STATUS_UNSUCCESSFUL; + goto cleanup; } DPRINT("HubDeviceExtension->UsbExtHubInfo.NumberOfPorts %x\n", HubDeviceExtension->UsbExtHubInfo.NumberOfPorts); @@ -1812,8 +1804,8 @@ if (!NT_SUCCESS(Status)) { DPRINT1("Failed to get Hub Descriptor!\n"); - ExFreePool(Urb); - return STATUS_UNSUCCESSFUL; + Status = STATUS_UNSUCCESSFUL; + goto cleanup; } // sanity checks @@ -1841,14 +1833,21 @@ { // failed to get hub status DPRINT1("Failed to get Hub Status!\n"); - ExFreePool(Urb); - return STATUS_UNSUCCESSFUL; + Status = STATUS_UNSUCCESSFUL; + goto cleanup; } // Allocate memory for PortStatusChange to hold 2 USHORTs for each port on hub HubDeviceExtension->PortStatusChange = ExAllocatePoolWithTag(NonPagedPool, sizeof(ULONG) * HubDeviceExtension->UsbExtHubInfo.NumberOfPorts, USB_HUB_TAG); + + if (!HubDeviceExtension->PortStatusChange) + { + DPRINT1("Failed to allocate pool for PortStatusChange!\n"); + Status = STATUS_INSUFFICIENT_RESOURCES; + goto cleanup; + } // Get the first Configuration Descriptor Pid = USBD_ParseConfigurationDescriptorEx(&HubDeviceExtension->HubConfigDescriptor, @@ -1858,8 +1857,8 @@ { // failed parse hub descriptor DPRINT1("Failed to parse configuration descriptor\n"); - ExFreePool(Urb); - return STATUS_UNSUCCESSFUL; + Status = STATUS_UNSUCCESSFUL; + goto cleanup; } // create configuration request @@ -1870,8 +1869,8 @@ { // failed to build urb DPRINT1("Failed to allocate urb\n"); - ExFreePool(Urb); - return STATUS_INSUFFICIENT_RESOURCES; + Status = STATUS_INSUFFICIENT_RESOURCES; + goto cleanup; } // send request @@ -1883,9 +1882,7 @@ { // failed to select configuration DPRINT1("Failed to select configuration with %x\n", Status); - ExFreePool(Urb); - ExFreePool(ConfigUrb); - return Status; + goto cleanup; } // store configuration & pipe handle @@ -1895,10 +1892,6 @@ FDO_QueryInterface(DeviceObject, &HubDeviceExtension->DeviceInterface); - - // free urb - ExFreePool(ConfigUrb); - // check if function is available if (HubDeviceExtension->UsbDInterface.IsDeviceHighSpeed) { @@ -1938,8 +1931,7 @@ if (!NT_SUCCESS(Status)) { DPRINT1("Failed to set callback\n"); - ExFreePool(Urb); - return Status; + goto cleanup; } } else @@ -1991,7 +1983,29 @@ // free urb ExFreePool(Urb); + // free ConfigUrb + ExFreePool(ConfigUrb); + // done + return Status; + +cleanup: + if (Urb) + ExFreePool(Urb); + + // Dereference interfaces + if (HubDeviceExtension->HubInterface.Size) + HubDeviceExtension->HubInterface.InterfaceDereference(HubDeviceExtension->HubInterface.BusContext); + + if (HubDeviceExtension->UsbDInterface.Size) + HubDeviceExtension->UsbDInterface.InterfaceDereference(HubDeviceExtension->UsbDInterface.BusContext); + + if (HubDeviceExtension->PortStatusChange) + ExFreePool(HubDeviceExtension->PortStatusChange); + + if (ConfigUrb) + ExFreePool(ConfigUrb); + return Status; }

8 years, 4 months

[vmikayelyan] 72380: usb: libusb: hub: Refernce interface before passing up Added InterfaceReference() calls before passing interface to upper layer.

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:24:25 2016 New Revision: 72380 URL: http://svn.reactos.org/svn/reactos?rev=72380&view=rev Log: usb: libusb: hub: Refernce interface before passing up Added InterfaceReference() calls before passing interface to upper layer. Modified: branches/GSoC_2016/USB/sdk/lib/drivers/libusb/hub_controller.cpp Modified: branches/GSoC_2016/USB/sdk/lib/drivers/libusb/hub_controller.cpp URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/sdk/lib/drivers/l… ============================================================================== --- branches/GSoC_2016/USB/sdk/lib/drivers/libusb/hub_controller.cpp [iso-8859-1] (original) +++ branches/GSoC_2016/USB/sdk/lib/drivers/libusb/hub_controller.cpp [iso-8859-1] Fri Aug 19 16:24:25 2016 @@ -3745,6 +3745,7 @@ InterfaceHub->SetDeviceHandleData = USBHI_SetDeviceHandleData; } + InterfaceHub->InterfaceReference(InterfaceHub->BusContext); // // request completed // @@ -3799,6 +3800,7 @@ InterfaceDI->EnumLogEntry = USBDI_EnumLogEntry; } + InterfaceDI->InterfaceReference(InterfaceDI->BusContext); // // request completed //

8 years, 4 months

[vmikayelyan] 72379: usb: hub: FDO: Fix CreateUsbChildDeviceObject() There were code dublication on copy device interface, also after copying interface, we should call InterfaceReference() routine ...

by vmikayelyan＠svn.reactos.org

Author: vmikayelyan Date: Fri Aug 19 16:22:18 2016 New Revision: 72379 URL: http://svn.reactos.org/svn/reactos?rev=72379&view=rev Log: usb: hub: FDO: Fix CreateUsbChildDeviceObject() There were code dublication on copy device interface, also after copying interface, we should call InterfaceReference() routine of interface, to prevent interface provider deleiton. I have moved RtlCopyMemory() and InterfaceReference() to the end of this function to siplify cleanup part, because otherwise we should call InterfaceDereference() in cleanup part. The changing of interface context seems wrong for me, let me give just one example. When we are changing only buscontext, then when upper layer calls interfaceDereference(), which is "thinking" that works with it's buscontext, and tries to work with it using it's buscontext pointer. So changing of buscontext is completely wrong. Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c Modified: branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/USB/drivers/usb/usbhu… ============================================================================== --- branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] (original) +++ branches/GSoC_2016/USB/drivers/usb/usbhub/fdo.c [iso-8859-1] Fri Aug 19 16:22:18 2016 @@ -1232,10 +1232,6 @@ UsbChildExtension->ParentDeviceObject = UsbHubDeviceObject; UsbChildExtension->PortNumber = PortId; - // copy device interface - RtlCopyMemory(&UsbChildExtension->DeviceInterface, &HubDeviceExtension->DeviceInterface, sizeof(USB_BUS_INTERFACE_USBDI_V2)); - - // // Create the UsbDeviceObject // @@ -1249,12 +1245,6 @@ DPRINT1("USBHUB: CreateUsbDevice failed with status %x\n", Status); goto Cleanup; } - - // copy device interface - RtlCopyMemory(&UsbChildExtension->DeviceInterface, &HubDeviceExtension->DeviceInterface, sizeof(USB_BUS_INTERFACE_USBDI_V2)); - - // FIXME replace buscontext - UsbChildExtension->DeviceInterface.BusContext = UsbChildExtension->UsbDeviceHandle; // // Initialize UsbDevice @@ -1344,6 +1334,10 @@ DPRINT1("Failed to create strings needed to describe device to PNP.\n"); goto Cleanup; } + + // copy device interface + RtlCopyMemory(&UsbChildExtension->DeviceInterface, &HubDeviceExtension->UsbDInterface, sizeof(USB_BUS_INTERFACE_USBDI_V2)); + UsbChildExtension->DeviceInterface.InterfaceReference(UsbChildExtension->DeviceInterface.BusContext); UsbChildExtension->IsRemovePending = FALSE;

8 years, 4 months

← Newer
1
...
12
13
14
15
16
17
18
...
47
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs August 2016