[janderwald] 55601: [USBSTOR] - Rewrite error handling - Check if CSW is valid - Check if the error handling was already started - Reset device if required - Error handling was completely broken an...
Author: janderwald Date: Wed Feb 15 03:52:37 2012 New Revision: 55601
URL: http://svn.reactos.org/svn/reactos?rev=55601&view=rev Log: [USBSTOR] - Rewrite error handling - Check if CSW is valid - Check if the error handling was already started - Reset device if required - Error handling was completely broken and did not follow the reset procedure as defined in USB Mass Storage Specification Bulk Only Section 5.3.4 - Mass storage device now longer hang when receiving the read capacity request and ReactOS assigns a symbolic link - Mass storage devices not yet fully working
Modified: trunk/reactos/drivers/usb/usbstor/error.c trunk/reactos/drivers/usb/usbstor/misc.c trunk/reactos/drivers/usb/usbstor/scsi.c trunk/reactos/drivers/usb/usbstor/usbstor.h
Modified: trunk/reactos/drivers/usb/usbstor/error.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbstor/error.c... ============================================================================== --- trunk/reactos/drivers/usb/usbstor/error.c [iso-8859-1] (original) +++ trunk/reactos/drivers/usb/usbstor/error.c [iso-8859-1] Wed Feb 15 03:52:37 2012 @@ -110,104 +110,157 @@ PDEVICE_OBJECT DeviceObject, PIRP_CONTEXT Context) { - NTSTATUS Status; + NTSTATUS Status = STATUS_SUCCESS; PIO_STACK_LOCATION Stack; - USBD_PIPE_HANDLE PipeHandle; + //USBD_PIPE_HANDLE PipeHandle; PSCSI_REQUEST_BLOCK Request; PCDB pCDB;
- DPRINT1("Entered Handle Transfer Error\n"); - // - // Determine pipehandle - // - if (Context->cbw->CommandBlock[0] == SCSIOP_WRITE) - { - // - // write request used bulk out pipe - // - PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkOutPipeIndex].PipeHandle; + + // + // first perform a mass storage reset step 1 in 5.3.4 USB Mass Storage Bulk Only Specification + // + Status = USBSTOR_ResetDevice(Context->FDODeviceExtension->LowerDeviceObject, Context->FDODeviceExtension); + if (NT_SUCCESS(Status)) + { + // + // step 2 reset bulk in pipe section 5.3.4 + // + Status = USBSTOR_ResetPipeWithHandle(Context->FDODeviceExtension->LowerDeviceObject, Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle); + if (NT_SUCCESS(Status)) + { + // + // finally reset bulk out pipe + // + Status = USBSTOR_ResetPipeWithHandle(Context->FDODeviceExtension->LowerDeviceObject, Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkOutPipeIndex].PipeHandle); + } + } + + if (Context->Irp) + { + // + // get next stack location + // + Stack = IoGetCurrentIrpStackLocation(Context->Irp); + + // + // get request block + // + Request = (PSCSI_REQUEST_BLOCK)Stack->Parameters.Others.Argument1; + ASSERT(Request); + + // + // obtain request type + // + pCDB = (PCDB)Request->Cdb; + ASSERT(pCDB); + + // + // Cleanup the IRP context + if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY) + { + FreeItem(Context->TransferData); + } + + if (Status != STATUS_SUCCESS) + { + // + // Complete the master IRP + // + Context->Irp->IoStatus.Status = Status; + Context->Irp->IoStatus.Information = 0; + USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Context->Irp); + IoCompleteRequest(Context->Irp, IO_NO_INCREMENT); + + // + // Start the next request + // + USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject); + } } else { - // - // default bulk in pipe - // - PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle; - } - - switch (Context->Urb.UrbHeader.Status) - { - case USBD_STATUS_STALL_PID: - { - // - // First attempt to reset the pipe - // - DPRINT1("Resetting Pipe\n"); - Status = USBSTOR_ResetPipeWithHandle(Context->FDODeviceExtension->LowerDeviceObject, PipeHandle); - if (NT_SUCCESS(Status)) - { - Status = STATUS_SUCCESS; - break; - } - - DPRINT1("Failed to reset pipe %x\n", Status); - - // - // FIXME: Reset of pipe failed, attempt to reset port - // - - Status = STATUS_UNSUCCESSFUL; - break; - } - // - // FIXME: Handle more errors - // - default: - { - DPRINT1("Error not handled\n"); - Status = STATUS_UNSUCCESSFUL; - } - } - - Stack = IoGetCurrentIrpStackLocation(Context->Irp); - Request = (PSCSI_REQUEST_BLOCK)Stack->Parameters.Others.Argument1; - pCDB = (PCDB)Request->Cdb; - if (Status != STATUS_SUCCESS) - { - /* Complete the master IRP */ - Context->Irp->IoStatus.Status = Status; - Context->Irp->IoStatus.Information = 0; - USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Context->Irp); - IoCompleteRequest(Context->Irp, IO_NO_INCREMENT); - - /* Start the next request */ - USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject); - - /* Signal the context event */ - if (Context->Event) + if (Status != STATUS_SUCCESS) + { + // + // Signal the context event + // + ASSERT(Context->Event); KeSetEvent(Context->Event, 0, FALSE); - - /* Cleanup the IRP context */ - if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY) - FreeItem(Context->TransferData); - FreeItem(Context->cbw); - FreeItem(Context); - } - else - { - + } + } + + if (NT_SUCCESS(Status)) + { DPRINT1("Retrying\n"); - Status = USBSTOR_HandleExecuteSCSI(*Context->PDODeviceExtension->PDODeviceObject, Context->Irp); - - /* Cleanup the old IRP context */ - if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY) - FreeItem(Context->TransferData); - FreeItem(Context->cbw); - FreeItem(Context); - } + USBSTOR_HandleExecuteSCSI(*Context->PDODeviceExtension->PDODeviceObject, Context->Irp); + } + + // + // cleanup irp context + // + FreeItem(Context->cbw); + FreeItem(Context); +
DPRINT1("USBSTOR_HandleTransferError returning with Status %x\n", Status); return Status; +} + +VOID +NTAPI +USBSTOR_ResetHandlerWorkItemRoutine( + PVOID Context) +{ + NTSTATUS Status; + USHORT Value; + PIO_STACK_LOCATION IoStack; + + PERRORHANDLER_WORKITEM_DATA WorkItemData = (PERRORHANDLER_WORKITEM_DATA)Context; + + // + // clear stall on BulkIn pipe + // + Status = USBSTOR_ResetPipeWithHandle(WorkItemData->Context->FDODeviceExtension->LowerDeviceObject, WorkItemData->Context->FDODeviceExtension->InterfaceInformation->Pipes[WorkItemData->Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle); + DPRINT1("USBSTOR_ResetPipeWithHandle Status %x\n", Status); + + // + // get next stack location + // + + IoStack = IoGetNextIrpStackLocation(WorkItemData->Irp); + + // + // now initialize the urb for sending the csw + // + UsbBuildInterruptOrBulkTransferRequest(&WorkItemData->Context->Urb, + sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER), + WorkItemData->Context->FDODeviceExtension->InterfaceInformation->Pipes[WorkItemData->Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle, + WorkItemData->Context->csw, + NULL, + 512, //FIXME + USBD_TRANSFER_DIRECTION_IN | USBD_SHORT_TRANSFER_OK, + NULL); + + // + // initialize stack location + // + IoStack->MajorFunction = IRP_MJ_INTERNAL_DEVICE_CONTROL; + IoStack->Parameters.DeviceIoControl.IoControlCode = IOCTL_INTERNAL_USB_SUBMIT_URB; + IoStack->Parameters.Others.Argument1 = (PVOID)&WorkItemData->Context->Urb; + IoStack->Parameters.DeviceIoControl.InputBufferLength = WorkItemData->Context->Urb.UrbHeader.Length; + WorkItemData->Irp->IoStatus.Status = STATUS_SUCCESS; + + + // + // setup completion routine + // + IoSetCompletionRoutine(WorkItemData->Irp, USBSTOR_CSWCompletionRoutine, Context, TRUE, TRUE, TRUE); + + // + // call driver + // + IoCallDriver(WorkItemData->Context->FDODeviceExtension->LowerDeviceObject, WorkItemData->Irp); }
VOID @@ -217,8 +270,21 @@ { NTSTATUS Status; PERRORHANDLER_WORKITEM_DATA WorkItemData = (PERRORHANDLER_WORKITEM_DATA)Context; - - Status = USBSTOR_HandleTransferError(WorkItemData->DeviceObject, WorkItemData->Context); + + if (WorkItemData->Context->ErrorIndex == 2) + { + // + // reset device + // + Status = USBSTOR_HandleTransferError(WorkItemData->DeviceObject, WorkItemData->Context); + } + else + { + // + // clear stall + // + USBSTOR_ResetHandlerWorkItemRoutine(WorkItemData); + }
// // Free Work Item Data
Modified: trunk/reactos/drivers/usb/usbstor/misc.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbstor/misc.c?... ============================================================================== --- trunk/reactos/drivers/usb/usbstor/misc.c [iso-8859-1] (original) +++ trunk/reactos/drivers/usb/usbstor/misc.c [iso-8859-1] Wed Feb 15 03:52:37 2012 @@ -411,7 +411,6 @@ // execute request // Status = USBSTOR_ClassRequest(DeviceObject, DeviceExtension, USB_BULK_RESET_DEVICE, DeviceExtension->InterfaceInformation->InterfaceNumber, USBD_TRANSFER_DIRECTION_OUT, 0, NULL); - DPRINT1("Status %x\n", Status);
// // done
Modified: trunk/reactos/drivers/usb/usbstor/scsi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbstor/scsi.c?... ============================================================================== --- trunk/reactos/drivers/usb/usbstor/scsi.c [iso-8859-1] (original) +++ trunk/reactos/drivers/usb/usbstor/scsi.c [iso-8859-1] Wed Feb 15 03:52:37 2012 @@ -83,6 +83,75 @@
}
+BOOLEAN +USBSTOR_IsCSWValid( + PIRP_CONTEXT Context) +{ + // + // sanity checks + // + if (Context->csw->Signature != CSW_SIGNATURE) + { + DPRINT1("[USBSTOR] Expected Signature %x but got %x\n", CSW_SIGNATURE, Context->csw->Signature); + return FALSE; + } + + if (Context->csw->Tag != (ULONG)Context->csw) + { + DPRINT1("[USBSTOR] Expected Tag %x but got %x\n", (ULONG)Context->csw, Context->csw->Tag); + return FALSE; + } + + if (Context->csw->Status != 0x00) + { + DPRINT1("[USBSTOR] Expected Status 0x00 but got %x\n", Context->csw->Status); + return FALSE; + } + + // + // CSW is valid + // + return TRUE; + +} + +NTSTATUS +USBSTOR_QueueWorkItem( + PIRP_CONTEXT Context, + PIRP Irp) +{ + PERRORHANDLER_WORKITEM_DATA ErrorHandlerWorkItemData; + + // + // Allocate Work Item Data + // + ErrorHandlerWorkItemData = ExAllocatePoolWithTag(NonPagedPool, sizeof(ERRORHANDLER_WORKITEM_DATA), USB_STOR_TAG); + if (!ErrorHandlerWorkItemData) + { + // + // no memory + // + return STATUS_INSUFFICIENT_RESOURCES; + } + + // + // Initialize and queue the work item to handle the error + // + ExInitializeWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem, + ErrorHandlerWorkItemRoutine, + ErrorHandlerWorkItemData); + + ErrorHandlerWorkItemData->DeviceObject = Context->FDODeviceExtension->FunctionalDeviceObject; + ErrorHandlerWorkItemData->Context = Context; + ErrorHandlerWorkItemData->Irp = Irp; + ErrorHandlerWorkItemData->DeviceObject = Context->FDODeviceExtension->FunctionalDeviceObject; + + DPRINT1("Queuing WorkItemROutine\n"); + ExQueueWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem, DelayedWorkQueue); + return STATUS_MORE_PROCESSING_REQUIRED; +} + + // // driver verifier // @@ -102,7 +171,7 @@ PREAD_CAPACITY_DATA_EX CapacityDataEx; PREAD_CAPACITY_DATA CapacityData; PUFI_CAPACITY_RESPONSE Response; - PERRORHANDLER_WORKITEM_DATA ErrorHandlerWorkItemData; + NTSTATUS Status; PURB Urb;
@@ -143,6 +212,48 @@ } }
+ DPRINT1("USBSTOR_CSWCompletionRoutine Status %x\n", Irp->IoStatus.Status); + + if (!NT_SUCCESS(Irp->IoStatus.Information)) + { + if (Context->ErrorIndex == 0) + { + // + // increment error index + // + Context->ErrorIndex = 1; + + // + // clear stall and resend cbw + // + Status = USBSTOR_QueueWorkItem(Context, Irp); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); + return STATUS_MORE_PROCESSING_REQUIRED; + } + + // + // perform reset recovery + // + Context->ErrorIndex = 2; + IoFreeIrp(Irp); + Status = USBSTOR_QueueWorkItem(Context, NULL); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); + return STATUS_MORE_PROCESSING_REQUIRED; + } + + if (!USBSTOR_IsCSWValid(Context)) + { + // + // perform reset recovery + // + Context->ErrorIndex = 2; + IoFreeIrp(Irp); + Status = USBSTOR_QueueWorkItem(Context, NULL); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); + return STATUS_MORE_PROCESSING_REQUIRED; + } + + if (Context->Irp) { // @@ -164,55 +275,6 @@ // get SCSI command data block // pCDB = (PCDB)Request->Cdb; - - // - // check status - // - if (!NT_SUCCESS(Status)) - { - DPRINT1("Status %x\n", Status); - DPRINT1("UrbStatus %x\n", Urb->UrbHeader.Status); - - // - // Check for errors that can be handled - // FIXME: Verify all usb errors that can be recovered via pipe reset/port reset/controller reset - // - if ((Urb->UrbHeader.Status & USB_RECOVERABLE_ERRORS) == Urb->UrbHeader.Status) - { - DPRINT1("Attempting Error Recovery\n"); - // - // free the allocated irp - // - IoFreeIrp(Irp); - - // - // Allocate Work Item Data - // - ErrorHandlerWorkItemData = ExAllocatePoolWithTag(NonPagedPool, sizeof(ERRORHANDLER_WORKITEM_DATA), USB_STOR_TAG); - if (!ErrorHandlerWorkItemData) - { - DPRINT1("Failed to allocate memory\n"); - Status = STATUS_INSUFFICIENT_RESOURCES; - } - else - { - // - // Initialize and queue the work item to handle the error - // - ExInitializeWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem, - ErrorHandlerWorkItemRoutine, - ErrorHandlerWorkItemData); - - ErrorHandlerWorkItemData->DeviceObject = Context->FDODeviceExtension->FunctionalDeviceObject; - ErrorHandlerWorkItemData->Context = Context; - DPRINT1("Queuing WorkItemROutine\n"); - ExQueueWorkItem(&ErrorHandlerWorkItemData->WorkQueueItem, DelayedWorkQueue); - - return STATUS_MORE_PROCESSING_REQUIRED; - } - } - } - Request->SrbStatus = SRB_STATUS_SUCCESS;
// @@ -268,28 +330,9 @@ }
// - // sanity checks - // - if (Context->csw->Signature != CSW_SIGNATURE) - { - DPRINT1("[USBSTOR] Expected Signature %x but got %x\n", CSW_SIGNATURE, Context->csw->Signature); - } - - if (Context->csw->Tag != (ULONG)Context->csw) - { - DPRINT1("[USBSTOR] Expected Tag %x but got %x\n", (ULONG)Context->csw, Context->csw->Tag); - } - - if (Context->csw->Status != 0x00) - { - DPRINT1("[USBSTOR] Expected Status 0x00 but got %x\n", Context->csw->Status); - } - - // // free cbw // FreeItem(Context->cbw); -
if (Context->Irp) { @@ -510,6 +553,19 @@
return STATUS_MORE_PROCESSING_REQUIRED; } + +VOID +DumpCBW( + PUCHAR Block) +{ + DPRINT1("%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", + Block[0] & 0xFF, Block[1] & 0xFF, Block[2] & 0xFF, Block[3] & 0xFF, Block[4] & 0xFF, Block[5] & 0xFF, Block[6] & 0xFF, Block[7] & 0xFF, Block[8] & 0xFF, Block[9] & 0xFF, + Block[10] & 0xFF, Block[11] & 0xFF, Block[12] & 0xFF, Block[13] & 0xFF, Block[14] & 0xFF, Block[15] & 0xFF, Block[16] & 0xFF, Block[17] & 0xFF, Block[18] & 0xFF, Block[19] & 0xFF, + Block[20] & 0xFF, Block[21] & 0xFF, Block[22] & 0xFF, Block[23] & 0xFF, Block[24] & 0xFF, Block[25] & 0xFF, Block[26] & 0xFF, Block[27] & 0xFF, Block[28] & 0xFF, Block[29] & 0xFF, + Block[30] & 0xFF); + +} +
NTSTATUS USBSTOR_SendRequest( @@ -561,6 +617,7 @@ Context->cbw);
DPRINT("CBW %p\n", Context->cbw); + DumpCBW((PUCHAR)Context->cbw);
// // now initialize the urb @@ -777,21 +834,6 @@ // KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL);
- KeResetEvent(&Event); - DPRINT("Resending request\n"); - - // - // now send the request - // - Status = USBSTOR_SendRequest(DeviceObject, NULL, &Event, UFI_INQUIRY_CMD_LEN, (PUCHAR)&Cmd, sizeof(UFI_INQUIRY_RESPONSE), (PUCHAR)Response); - - // - // wait for the action to complete - // - KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL); - - - DPRINT1("Response %p\n", Response); DPRINT1("DeviceType %x\n", Response->DeviceType); DPRINT1("RMB %x\n", Response->RMB);
Modified: trunk/reactos/drivers/usb/usbstor/usbstor.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbstor/usbstor... ============================================================================== --- trunk/reactos/drivers/usb/usbstor/usbstor.h [iso-8859-1] (original) +++ trunk/reactos/drivers/usb/usbstor/usbstor.h [iso-8859-1] Wed Feb 15 03:52:37 2012 @@ -279,6 +279,7 @@ PPDO_DEVICE_EXTENSION PDODeviceExtension; PMDL TransferBufferMDL; PKEVENT Event; + ULONG ErrorIndex; }IRP_CONTEXT, *PIRP_CONTEXT;
typedef struct _ERRORHANDLER_WORKITEM_DATA @@ -286,6 +287,7 @@ PDEVICE_OBJECT DeviceObject; PIRP_CONTEXT Context; WORK_QUEUE_ITEM WorkQueueItem; + PIRP Irp; } ERRORHANDLER_WORKITEM_DATA, *PERRORHANDLER_WORKITEM_DATA;
@@ -390,6 +392,13 @@ USBSTOR_SendInquiryCmd( IN PDEVICE_OBJECT DeviceObject);
+NTSTATUS +NTAPI +USBSTOR_CSWCompletionRoutine( + PDEVICE_OBJECT DeviceObject, + PIRP Irp, + PVOID Ctx); + //--------------------------------------------------------------------- // // disk.c routines @@ -443,6 +452,13 @@ PVOID Context);
VOID +NTAPI +ResetHandlerWorkItemRoutine( + PVOID Context); + + + +VOID USBSTOR_QueueNextRequest( IN PDEVICE_OBJECT DeviceObject);
@@ -458,3 +474,8 @@ IN UCHAR bEndpointAddress, OUT PUSHORT Value);
+NTSTATUS +USBSTOR_ResetPipeWithHandle( + IN PDEVICE_OBJECT DeviceObject, + IN USBD_PIPE_HANDLE PipeHandle); +
-
janderwald＠svn.reactos.org