[reactos] 03/17: [USBSTOR] Do not create a new Irp for USB requests - use the original one from higher-level driver instead. Refactored CSWCompletionRoutine for correct handling different CSW statuses, more work to be done here.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=ed6724cd7e2d769d7453ff...
commit ed6724cd7e2d769d7453ff0bd3eb3bc835bf2f04 Author: Victor Perevertkin victor@perevertkin.ru AuthorDate: Thu Mar 28 02:45:32 2019 +0300 Commit: Victor Perevertkin victor@perevertkin.ru CommitDate: Tue Jun 11 04:39:43 2019 +0300
[USBSTOR] Do not create a new Irp for USB requests - use the original one from higher-level driver instead. Refactored CSWCompletionRoutine for correct handling different CSW statuses, more work to be done here. --- drivers/usb/usbstor/scsi.c | 179 +++++++++++++++++------------------------- drivers/usb/usbstor/usbstor.h | 4 + 2 files changed, 75 insertions(+), 108 deletions(-)
diff --git a/drivers/usb/usbstor/scsi.c b/drivers/usb/usbstor/scsi.c index b2989f01fce..120a3b09811 100644 --- a/drivers/usb/usbstor/scsi.c +++ b/drivers/usb/usbstor/scsi.c @@ -57,6 +57,7 @@ USBSTOR_AllocateIrpContext() return Context; }
+static BOOLEAN USBSTOR_IsCSWValid( PIRP_CONTEXT Context) @@ -73,12 +74,6 @@ USBSTOR_IsCSWValid( return FALSE; }
- if (Context->csw->Status != 0x00) - { - DPRINT1("[USBSTOR] Expected Status 0x00 but got %x\n", Context->csw->Status); - return FALSE; - } - return TRUE; }
@@ -128,99 +123,99 @@ USBSTOR_CSWCompletionRoutine( PIRP_CONTEXT Context; PIO_STACK_LOCATION IoStack; PSCSI_REQUEST_BLOCK Request; - PCDB pCDB; PUFI_CAPACITY_RESPONSE Response; NTSTATUS Status;
Context = (PIRP_CONTEXT)Ctx;
- if (Context->TransferBufferMDL) + if (Context->TransferBufferMDL && Context->TransferBufferMDL != Context->Irp->MdlAddress) { - // is there an irp associated - if (Context->Irp) - { - // did we allocate the mdl - if (Context->TransferBufferMDL != Context->Irp->MdlAddress) - { - IoFreeMdl(Context->TransferBufferMDL); - } - } - else - { - IoFreeMdl(Context->TransferBufferMDL); - } + IoFreeMdl(Context->TransferBufferMDL); }
- DPRINT("USBSTOR_CSWCompletionRoutine Status %x\n", Irp->IoStatus.Status); + DPRINT("USBSTOR_CSWCompletionRoutine Irp %p Ctx %p Status %x\n", Irp, Ctx, Irp->IoStatus.Status);
- if (!NT_SUCCESS(Irp->IoStatus.Information)) + // first check for Irp errors + if (!NT_SUCCESS(Irp->IoStatus.Status)) { - if (Context->ErrorIndex == 0) + if (USBD_STATUS(Context->Urb.UrbHeader.Status) == USBD_STATUS(USBD_STATUS_STALL_PID)) { - Context->ErrorIndex = 1; + if (Context->ErrorIndex == 0) + { + Context->ErrorIndex = 1; + + // clear stall and resend cbw + Status = USBSTOR_QueueWorkItem(Context, Irp); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED);
- // clear stall and resend cbw - Status = USBSTOR_QueueWorkItem(Context, Irp); - ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); - return STATUS_MORE_PROCESSING_REQUIRED; + return STATUS_MORE_PROCESSING_REQUIRED; + } + } + else + { + DPRINT1("USBSTOR_CSWCompletionRoutine: Urb.Hdr.Status - %x\n", Context->Urb.UrbHeader.Status); }
// 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)) + // now check the CSW packet validity + if (!USBSTOR_IsCSWValid(Context) || Context->csw->Status == CSW_STATUS_PHASE_ERROR) { // perform reset recovery Context->ErrorIndex = 2; - IoFreeIrp(Irp); Status = USBSTOR_QueueWorkItem(Context, NULL); ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); return STATUS_MORE_PROCESSING_REQUIRED; }
- - IoStack = IoGetCurrentIrpStackLocation(Context->Irp); - - Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1; + IoStack = IoGetCurrentIrpStackLocation(Irp); + Request = IoStack->Parameters.Scsi.Srb; ASSERT(Request);
Status = Irp->IoStatus.Status;
- pCDB = (PCDB)Request->Cdb; - Request->SrbStatus = SRB_STATUS_SUCCESS; - - // read capacity needs special work - if (pCDB->AsByte[0] == SCSIOP_READ_CAPACITY) + // finally check for CSW errors + if (Context->csw->Status == CSW_STATUS_COMMAND_PASSED) { - // get output buffer - Response = (PUFI_CAPACITY_RESPONSE)Context->TransferData; + // read capacity needs special work + if (Request->Cdb[0] == SCSIOP_READ_CAPACITY) + { + // get output buffer + Response = (PUFI_CAPACITY_RESPONSE)Context->TransferData;
- // store in pdo - Context->PDODeviceExtension->BlockLength = NTOHL(Response->BlockLength); - Context->PDODeviceExtension->LastLogicBlockAddress = NTOHL(Response->LastLogicalBlockAddress); + // store in pdo + Context->PDODeviceExtension->BlockLength = NTOHL(Response->BlockLength); + Context->PDODeviceExtension->LastLogicBlockAddress = NTOHL(Response->LastLogicalBlockAddress); + } + + Status = STATUS_SUCCESS; + Request->SrbStatus = SRB_STATUS_SUCCESS; + Irp->IoStatus.Status = STATUS_SUCCESS; + Irp->IoStatus.Information = Request->DataTransferLength; + } + else if (Context->csw->Status == CSW_STATUS_COMMAND_FAILED) + { + DPRINT("USBSTOR_CSWCompletionRoutine: CSW_STATUS_COMMAND_FAILED\n"); + // perform reset recovery + Context->ErrorIndex = 2; + Status = USBSTOR_QueueWorkItem(Context, NULL); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); + return STATUS_MORE_PROCESSING_REQUIRED; }
FreeItem(Context->cbw);
- // FIXME: check status - Context->Irp->IoStatus.Status = Irp->IoStatus.Status; - Context->Irp->IoStatus.Information = Context->TransferDataLength; - // terminate current request - USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Context->Irp); - - IoCompleteRequest(Context->Irp, IO_NO_INCREMENT); - + USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Irp); USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject);
- IoFreeIrp(Irp); FreeItem(Context); - return STATUS_MORE_PROCESSING_REQUIRED; + return Status; }
VOID @@ -406,7 +401,6 @@ USBSTOR_SendRequest( PIRP_CONTEXT Context; PPDO_DEVICE_EXTENSION PDODeviceExtension; PFDO_DEVICE_EXTENSION FDODeviceExtension; - PIRP Irp; PUCHAR MdlVirtualAddress;
Context = USBSTOR_AllocateIrpContext(); @@ -450,50 +444,34 @@ USBSTOR_SendRequest( if (Context->TransferDataLength) { // check if the original request already does have an mdl associated - if (OriginalRequest) + if ((OriginalRequest->MdlAddress != NULL) && + (Context->TransferData == NULL || Command[0] == SCSIOP_READ || Command[0] == SCSIOP_WRITE)) { - if ((OriginalRequest->MdlAddress != NULL) && - (Context->TransferData == NULL || Command[0] == SCSIOP_READ || Command[0] == SCSIOP_WRITE)) + // Sanity check that the Mdl does describe the TransferData for read/write + if (CommandLength == UFI_READ_WRITE_CMD_LEN) { - // Sanity check that the Mdl does describe the TransferData for read/write - if (CommandLength == UFI_READ_WRITE_CMD_LEN) - { - MdlVirtualAddress = MmGetMdlVirtualAddress(OriginalRequest->MdlAddress); + MdlVirtualAddress = MmGetMdlVirtualAddress(OriginalRequest->MdlAddress);
- // is there an offset - if (MdlVirtualAddress != Context->TransferData) + // is there an offset + if (MdlVirtualAddress != Context->TransferData) + { + // lets build an mdl + Context->TransferBufferMDL = IoAllocateMdl(Context->TransferData, MmGetMdlByteCount(OriginalRequest->MdlAddress), FALSE, FALSE, NULL); + if (!Context->TransferBufferMDL) { - // lets build an mdl - Context->TransferBufferMDL = IoAllocateMdl(Context->TransferData, MmGetMdlByteCount(OriginalRequest->MdlAddress), FALSE, FALSE, NULL); - if (!Context->TransferBufferMDL) - { - FreeItem(Context->cbw); - FreeItem(Context); - return STATUS_INSUFFICIENT_RESOURCES; - } - - IoBuildPartialMdl(OriginalRequest->MdlAddress, Context->TransferBufferMDL, Context->TransferData, Context->TransferDataLength); + FreeItem(Context->cbw); + FreeItem(Context); + return STATUS_INSUFFICIENT_RESOURCES; } - }
- if (!Context->TransferBufferMDL) - { - // I/O paging request - Context->TransferBufferMDL = OriginalRequest->MdlAddress; + IoBuildPartialMdl(OriginalRequest->MdlAddress, Context->TransferBufferMDL, Context->TransferData, Context->TransferDataLength); } } - else - { - // allocate mdl for buffer, buffer must be allocated from NonPagedPool - Context->TransferBufferMDL = IoAllocateMdl(Context->TransferData, Context->TransferDataLength, FALSE, FALSE, NULL); - if (!Context->TransferBufferMDL) - { - FreeItem(Context->cbw); - FreeItem(Context); - return STATUS_INSUFFICIENT_RESOURCES; - }
- MmBuildMdlForNonPagedPool(Context->TransferBufferMDL); + if (!Context->TransferBufferMDL) + { + // I/O paging request + Context->TransferBufferMDL = OriginalRequest->MdlAddress; } } else @@ -511,22 +489,7 @@ USBSTOR_SendRequest( } }
- Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) - { - FreeItem(Context->cbw); - FreeItem(Context); - return STATUS_INSUFFICIENT_RESOURCES; - } - - if (OriginalRequest) - { - IoMarkIrpPending(OriginalRequest); - } - - USBSTOR_SendCBW(Context, Irp); - - return STATUS_PENDING; + return USBSTOR_SendCBW(Context, OriginalRequest); }
NTSTATUS diff --git a/drivers/usb/usbstor/usbstor.h b/drivers/usb/usbstor/usbstor.h index f477e9a1a04..40b369bd15e 100644 --- a/drivers/usb/usbstor/usbstor.h +++ b/drivers/usb/usbstor/usbstor.h @@ -105,6 +105,10 @@ C_ASSERT(sizeof(CBW) == 31);
#define MAX_LUN 0xF
+#define CSW_STATUS_COMMAND_PASSED 0x00 +#define CSW_STATUS_COMMAND_FAILED 0x01 +#define CSW_STATUS_PHASE_ERROR 0x02 + typedef struct { ULONG Signature; // CSW signature
-
Victor Perevertkin