[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.…
==============================================================================
--- 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/usbsto…
==============================================================================
--- 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);
+