[reactos] 07/17: [USBSTOR] Refactor request issue code. Pipe handle selection now made more correctly. Simplified an MDL allocation for a request.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=c452f7dab49f815ccd5048...
commit c452f7dab49f815ccd5048d8d261dfbf1843c341 Author: Victor Perevertkin victor@perevertkin.ru AuthorDate: Mon Apr 1 01:07:55 2019 +0300 Commit: Victor Perevertkin victor@perevertkin.ru CommitDate: Tue Jun 11 04:39:43 2019 +0300
[USBSTOR] Refactor request issue code. Pipe handle selection now made more correctly. Simplified an MDL allocation for a request. --- drivers/usb/usbstor/error.c | 2 +- drivers/usb/usbstor/scsi.c | 372 ++++++++++++++++++------------------------ drivers/usb/usbstor/usbstor.h | 14 +- 3 files changed, 162 insertions(+), 226 deletions(-)
diff --git a/drivers/usb/usbstor/error.c b/drivers/usb/usbstor/error.c index 0256c741a64..3ae8a3bfca4 100644 --- a/drivers/usb/usbstor/error.c +++ b/drivers/usb/usbstor/error.c @@ -160,7 +160,7 @@ USBSTOR_ResetHandlerWorkItemRoutine( DPRINT1("USBSTOR_ResetPipeWithHandle Status %x\n", Status);
// now resend the csw as the stall got cleared - USBSTOR_SendCSW(WorkItemData->Context, WorkItemData->Irp); + USBSTOR_SendCSWRequest(WorkItemData->Context, WorkItemData->Irp); }
VOID diff --git a/drivers/usb/usbstor/scsi.c b/drivers/usb/usbstor/scsi.c index f563f77fa8e..5411468a0a9 100644 --- a/drivers/usb/usbstor/scsi.c +++ b/drivers/usb/usbstor/scsi.c @@ -55,27 +55,45 @@ USBSTOR_SrbStatusToNtStatus( } }
+static NTSTATUS -USBSTOR_BuildCBW( - IN ULONG Tag, - IN ULONG DataTransferLength, - IN UCHAR LUN, - IN UCHAR CommandBlockLength, - IN PUCHAR CommandBlock, - IN OUT PCBW Control) +USBSTOR_IssueBulkOrInterruptRequest( + IN PFDO_DEVICE_EXTENSION FDODeviceExtension, + IN PIRP Irp, + IN USBD_PIPE_HANDLE PipeHandle, + IN ULONG TransferFlags, + IN ULONG TransferBufferLength, + IN PVOID TransferBuffer, + IN PMDL TransferBufferMDL, + IN PIO_COMPLETION_ROUTINE CompletionRoutine, + IN PIRP_CONTEXT Context) { - ASSERT(CommandBlockLength <= 16); + PIO_STACK_LOCATION NextStack; + + RtlZeroMemory(&Context->Urb, sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER)); + + Context->Urb.UrbHeader.Length = sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER); + Context->Urb.UrbHeader.Function = URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER;
- Control->Signature = CBW_SIGNATURE; - Control->Tag = Tag; - Control->DataTransferLength = DataTransferLength; - Control->Flags = (CommandBlock[0] != SCSIOP_WRITE) ? 0x80 : 0x00; - Control->LUN = (LUN & MAX_LUN); - Control->CommandBlockLength = CommandBlockLength; + Context->Urb.UrbBulkOrInterruptTransfer.PipeHandle = PipeHandle; + Context->Urb.UrbBulkOrInterruptTransfer.TransferFlags = TransferFlags; + Context->Urb.UrbBulkOrInterruptTransfer.TransferBufferLength = TransferBufferLength; + Context->Urb.UrbBulkOrInterruptTransfer.TransferBuffer = TransferBuffer; + Context->Urb.UrbBulkOrInterruptTransfer.TransferBufferMDL = TransferBufferMDL;
- RtlCopyMemory(Control->CommandBlock, CommandBlock, CommandBlockLength); + NextStack = IoGetNextIrpStackLocation(Irp); + NextStack->MajorFunction = IRP_MJ_INTERNAL_DEVICE_CONTROL; + NextStack->Parameters.DeviceIoControl.IoControlCode = IOCTL_INTERNAL_USB_SUBMIT_URB; + NextStack->Parameters.Others.Argument1 = &Context->Urb;
- return STATUS_SUCCESS; + IoSetCompletionRoutine(Irp, + CompletionRoutine, + Context, + TRUE, + TRUE, + TRUE); + + return IoCallDriver(FDODeviceExtension->LowerDeviceObject, Irp); }
PIRP_CONTEXT @@ -170,11 +188,6 @@ USBSTOR_CSWCompletionRoutine(
Context = (PIRP_CONTEXT)Ctx;
- if (Context->TransferBufferMDL && Context->TransferBufferMDL != Context->Irp->MdlAddress) - { - IoFreeMdl(Context->TransferBufferMDL); - } - DPRINT("USBSTOR_CSWCompletionRoutine Irp %p Ctx %p Status %x\n", Irp, Ctx, Irp->IoStatus.Status);
// first check for Irp errors @@ -274,35 +287,20 @@ USBSTOR_CSWCompletionRoutine( return Status; }
-VOID -USBSTOR_SendCSW( +NTSTATUS +USBSTOR_SendCSWRequest( PIRP_CONTEXT Context, PIRP Irp) { - PIO_STACK_LOCATION IoStack; - - IoStack = IoGetNextIrpStackLocation(Irp); - - // now initialize the urb for sending the csw - UsbBuildInterruptOrBulkTransferRequest(&Context->Urb, - sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER), - Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle, - 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)&Context->Urb; - IoStack->Parameters.DeviceIoControl.InputBufferLength = Context->Urb.UrbHeader.Length; - Irp->IoStatus.Status = STATUS_SUCCESS; - - IoSetCompletionRoutine(Irp, USBSTOR_CSWCompletionRoutine, Context, TRUE, TRUE, TRUE); - - IoCallDriver(Context->FDODeviceExtension->LowerDeviceObject, Irp); + return USBSTOR_IssueBulkOrInterruptRequest(Context->FDODeviceExtension, + Irp, + Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle, + USBD_TRANSFER_DIRECTION_IN, + sizeof(CSW), + Context->csw, + NULL, + USBSTOR_CSWCompletionRoutine, + Context); }
IO_COMPLETION_ROUTINE USBSTOR_DataCompletionRoutine; @@ -316,13 +314,19 @@ USBSTOR_DataCompletionRoutine( { PIRP_CONTEXT Context; NTSTATUS Status; - + PIO_STACK_LOCATION IoStack; + PSCSI_REQUEST_BLOCK Request;
DPRINT("USBSTOR_DataCompletionRoutine Irp %p Ctx %p Status %x\n", Irp, Ctx, Irp->IoStatus.Status);
Context = (PIRP_CONTEXT)Ctx; - PIO_STACK_LOCATION IoStack = IoGetCurrentIrpStackLocation(Irp); - PSCSI_REQUEST_BLOCK Request = IoStack->Parameters.Scsi.Srb; + IoStack = IoGetCurrentIrpStackLocation(Irp); + Request = IoStack->Parameters.Scsi.Srb; + + if (Context->Urb.UrbBulkOrInterruptTransfer.TransferBufferMDL != Irp->MdlAddress) + { + IoFreeMdl(Context->Urb.UrbBulkOrInterruptTransfer.TransferBufferMDL); + }
if (NT_SUCCESS(Irp->IoStatus.Status)) { @@ -336,7 +340,7 @@ USBSTOR_DataCompletionRoutine( }
Request->DataTransferLength = Context->Urb.UrbBulkOrInterruptTransfer.TransferBufferLength; - USBSTOR_SendCSW(Context, Irp); + USBSTOR_SendCSWRequest(Context, Irp); } else if (USBD_STATUS(Context->Urb.UrbHeader.Status) == USBD_STATUS(USBD_STATUS_STALL_PID)) { @@ -373,8 +377,11 @@ USBSTOR_CBWCompletionRoutine( PIRP_CONTEXT Context; PIO_STACK_LOCATION IoStack; PSCSI_REQUEST_BLOCK Request; - UCHAR Code; USBD_PIPE_HANDLE PipeHandle; + ULONG TransferFlags; + PMDL Mdl = NULL; + PVOID TransferBuffer = NULL; + NTSTATUS Status;
DPRINT("USBSTOR_CBWCompletionRoutine Irp %p Ctx %p Status %x\n", Irp, Ctx, Irp->IoStatus.Status);
@@ -384,73 +391,79 @@ USBSTOR_CBWCompletionRoutine(
if (!NT_SUCCESS(Irp->IoStatus.Status)) { - // perform reset recovery - Context->ErrorIndex = 2; - Status = USBSTOR_QueueWorkItem(Context, NULL); - ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); + goto ResetRecovery; + } + + // a request without the buffer + if (!Irp->MdlAddress) + { + Request->SrbStatus = SRB_STATUS_SUCCESS; + USBSTOR_SendCSWRequest(Context, Irp); return STATUS_MORE_PROCESSING_REQUIRED; }
- // is there data to be submitted - if (Context->TransferDataLength) + // a request with the data buffer + + if ((Request->SrbFlags & SRB_FLAGS_UNSPECIFIED_DIRECTION) == SRB_FLAGS_DATA_IN) + { + PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle; + TransferFlags = USBD_TRANSFER_DIRECTION_IN | USBD_SHORT_TRANSFER_OK; + } + else if ((Request->SrbFlags & SRB_FLAGS_UNSPECIFIED_DIRECTION) == SRB_FLAGS_DATA_OUT) + { + PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkOutPipeIndex].PipeHandle; + TransferFlags = USBD_TRANSFER_DIRECTION_OUT; + } + else { - // get command code - Code = Context->cbw->CommandBlock[0]; + // we check the validity of a request in disk.c so we should never be here + DPRINT1("Warning: shouldn't be here\n"); + goto ResetRecovery; + }
- if (Code == SCSIOP_WRITE) + if (MmGetMdlVirtualAddress(Irp->MdlAddress) == Request->DataBuffer) { - // write request - use bulk out pipe - PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkOutPipeIndex].PipeHandle; + Mdl = Irp->MdlAddress; } else { - // default bulk in pipe - PipeHandle = Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle; + Mdl = IoAllocateMdl(Request->DataBuffer, + Request->DataTransferLength, + FALSE, + FALSE, + NULL); + + if (Mdl) + { + IoBuildPartialMdl(Irp->MdlAddress, + Mdl, + Request->DataBuffer, + Request->DataTransferLength); + } }
- // now initialize the urb for sending data - UsbBuildInterruptOrBulkTransferRequest(&Context->Urb, - sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER), - PipeHandle, - NULL, - Context->TransferBufferMDL, - Context->TransferDataLength, - ((Code == SCSIOP_WRITE) ? USBD_TRANSFER_DIRECTION_OUT : (USBD_TRANSFER_DIRECTION_IN | USBD_SHORT_TRANSFER_OK)), - NULL); - - IoSetCompletionRoutine(Irp, USBSTOR_DataCompletionRoutine, Context, TRUE, TRUE, TRUE); - } - else - { - if (NT_SUCCESS(Irp->IoStatus.Status)) + if (!Mdl) { - Request->SrbStatus = SRB_STATUS_SUCCESS; + DPRINT1("USBSTOR_DataTransfer: Mdl - %p\n", Mdl); + goto ResetRecovery; }
- // now initialize the urb for sending the csw - UsbBuildInterruptOrBulkTransferRequest(&Context->Urb, - sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER), - Context->FDODeviceExtension->InterfaceInformation->Pipes[Context->FDODeviceExtension->BulkInPipeIndex].PipeHandle, - Context->csw, - NULL, - 512, //FIXME - USBD_TRANSFER_DIRECTION_IN | USBD_SHORT_TRANSFER_OK, - NULL); - - IoSetCompletionRoutine(Irp, USBSTOR_CSWCompletionRoutine, Context, TRUE, TRUE, TRUE); - } - - IoStack = IoGetNextIrpStackLocation(Irp); + USBSTOR_IssueBulkOrInterruptRequest(Context->FDODeviceExtension, + Irp, + PipeHandle, + TransferFlags, + Request->DataTransferLength, + TransferBuffer, + Mdl, + USBSTOR_DataCompletionRoutine, + Context);
- // initialize stack location - IoStack->MajorFunction = IRP_MJ_INTERNAL_DEVICE_CONTROL; - IoStack->Parameters.DeviceIoControl.IoControlCode = IOCTL_INTERNAL_USB_SUBMIT_URB; - IoStack->Parameters.Others.Argument1 = (PVOID)&Context->Urb; - IoStack->Parameters.DeviceIoControl.InputBufferLength = Context->Urb.UrbHeader.Length; - Irp->IoStatus.Status = STATUS_SUCCESS; - - IoCallDriver(Context->FDODeviceExtension->LowerDeviceObject, Irp); + return STATUS_MORE_PROCESSING_REQUIRED;
+ResetRecovery: + Context->ErrorIndex = 2; + Status = USBSTOR_QueueWorkItem(Context, NULL); + ASSERT(Status == STATUS_MORE_PROCESSING_REQUIRED); return STATUS_MORE_PROCESSING_REQUIRED; }
@@ -463,132 +476,67 @@ DumpCBW( 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_SendCBW( - PIRP_CONTEXT Context, - PIRP Irp) -{ - PIO_STACK_LOCATION IoStack; - - IoStack = IoGetNextIrpStackLocation(Irp); - - // initialize stack location - IoStack->MajorFunction = IRP_MJ_INTERNAL_DEVICE_CONTROL; - IoStack->Parameters.DeviceIoControl.IoControlCode = IOCTL_INTERNAL_USB_SUBMIT_URB; - IoStack->Parameters.Others.Argument1 = (PVOID)&Context->Urb; - IoStack->Parameters.DeviceIoControl.InputBufferLength = Context->Urb.UrbHeader.Length; - Irp->IoStatus.Status = STATUS_SUCCESS; - - IoSetCompletionRoutine(Irp, USBSTOR_CBWCompletionRoutine, Context, TRUE, TRUE, TRUE); - - return IoCallDriver(Context->FDODeviceExtension->LowerDeviceObject, Irp); }
+static NTSTATUS -USBSTOR_SendRequest( - IN PDEVICE_OBJECT DeviceObject, - IN PIRP OriginalRequest, - IN UCHAR CommandLength, - IN PUCHAR Command, - IN ULONG TransferDataLength, - IN PUCHAR TransferData, - IN ULONG RetryCount) +USBSTOR_SendCBWRequest( + IN PFDO_DEVICE_EXTENSION FDODeviceExtension, + IN PIRP Irp, + IN PIRP_CONTEXT Context) { - PIRP_CONTEXT Context; PPDO_DEVICE_EXTENSION PDODeviceExtension; - PFDO_DEVICE_EXTENSION FDODeviceExtension; - PUCHAR MdlVirtualAddress; + PIO_STACK_LOCATION IoStack; + PSCSI_REQUEST_BLOCK Request;
- Context = USBSTOR_AllocateIrpContext(); if (!Context) { - return STATUS_INSUFFICIENT_RESOURCES; + Context = USBSTOR_AllocateIrpContext(); + if (!Context) + { + return STATUS_INSUFFICIENT_RESOURCES; + } } + else + { + RtlZeroMemory(Context->cbw, sizeof(CBW)); + RtlZeroMemory(&Context->Urb, sizeof(URB)); + }
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension; - FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension; + IoStack = IoGetCurrentIrpStackLocation(Irp); + PDODeviceExtension = IoStack->DeviceObject->DeviceExtension; + Request = IoStack->Parameters.Scsi.Srb;
- USBSTOR_BuildCBW(PtrToUlong(Context->cbw), - TransferDataLength, - PDODeviceExtension->LUN, - CommandLength, - Command, - Context->cbw); + Context->cbw->Signature = CBW_SIGNATURE; + Context->cbw->Tag = PtrToUlong(Context->cbw); + Context->cbw->DataTransferLength = Request->DataTransferLength; + Context->cbw->Flags = ((UCHAR)Request->SrbFlags & SRB_FLAGS_UNSPECIFIED_DIRECTION) << 1; + Context->cbw->LUN = PDODeviceExtension->LUN; + Context->cbw->CommandBlockLength = Request->CdbLength; + + RtlCopyMemory(Context->cbw->CommandBlock, Request->Cdb, Request->CdbLength);
DPRINT("CBW %p\n", Context->cbw); DumpCBW((PUCHAR)Context->cbw);
- // now initialize the urb - UsbBuildInterruptOrBulkTransferRequest(&Context->Urb, - sizeof(struct _URB_BULK_OR_INTERRUPT_TRANSFER), - FDODeviceExtension->InterfaceInformation->Pipes[FDODeviceExtension->BulkOutPipeIndex].PipeHandle, - Context->cbw, - NULL, - sizeof(CBW), - USBD_TRANSFER_DIRECTION_OUT, - NULL); - // initialize rest of context - Context->Irp = OriginalRequest; - Context->TransferData = TransferData; - Context->TransferDataLength = TransferDataLength; + Context->Irp = Irp; + Context->TransferData = Request->DataBuffer; + Context->TransferDataLength = Request->DataTransferLength; Context->FDODeviceExtension = FDODeviceExtension; Context->PDODeviceExtension = PDODeviceExtension; - Context->RetryCount = RetryCount; - - // is there transfer data - if (Context->TransferDataLength) - { - // check if the original request already does have an mdl associated - 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) - { - MdlVirtualAddress = MmGetMdlVirtualAddress(OriginalRequest->MdlAddress); - - // 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) - { - FreeItem(Context->cbw); - FreeItem(Context); - return STATUS_INSUFFICIENT_RESOURCES; - } - - IoBuildPartialMdl(OriginalRequest->MdlAddress, Context->TransferBufferMDL, Context->TransferData, Context->TransferDataLength); - } - } - - if (!Context->TransferBufferMDL) - { - // I/O paging request - Context->TransferBufferMDL = OriginalRequest->MdlAddress; - } - } - 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); - } - } - - return USBSTOR_SendCBW(Context, OriginalRequest); + Context->RetryCount = 0; + + return USBSTOR_IssueBulkOrInterruptRequest( + FDODeviceExtension, + Irp, + FDODeviceExtension->InterfaceInformation->Pipes[FDODeviceExtension->BulkOutPipeIndex].PipeHandle, + USBD_TRANSFER_DIRECTION_OUT, + sizeof(CBW), + Context->cbw, + NULL, + USBSTOR_CBWCompletionRoutine, + Context); }
NTSTATUS @@ -614,7 +562,7 @@ USBSTOR_HandleExecuteSCSI(
// check that we're sending to the right LUN ASSERT(pCDB->CDB10.LogicalUnitNumber == (PDODeviceExtension->LUN & MAX_LUN)); - Status = USBSTOR_SendRequest(DeviceObject, Irp, Request->CdbLength, (PUCHAR)pCDB, Request->DataTransferLength, Request->DataBuffer, RetryCount); + Status = USBSTOR_SendCBWRequest(PDODeviceExtension->LowerDeviceObject->DeviceExtension, Irp, NULL);
return Status; } diff --git a/drivers/usb/usbstor/usbstor.h b/drivers/usb/usbstor/usbstor.h index 6d23a567853..fc3870ed298 100644 --- a/drivers/usb/usbstor/usbstor.h +++ b/drivers/usb/usbstor/usbstor.h @@ -413,19 +413,7 @@ USBSTOR_HandleExecuteSCSI( IN ULONG RetryCount);
NTSTATUS -NTAPI -USBSTOR_CSWCompletionRoutine( - PDEVICE_OBJECT DeviceObject, - PIRP Irp, - PVOID Ctx); - -NTSTATUS -USBSTOR_SendCBW( - PIRP_CONTEXT Context, - PIRP Irp); - -VOID -USBSTOR_SendCSW( +USBSTOR_SendCSWRequest( PIRP_CONTEXT Context, PIRP Irp);
-
Victor Perevertkin