[mjmartin] 51684: [USBEHCI_NEW] - Modify BuildBulkTransferQueueHead to support TransferBufferLengths larger than PAGE_SIZE * 5. - Acquire a SpinLock before adding QueueHeads to AsyncList and Pendin...
Author: mjmartin Date: Thu May 12 13:35:06 2011 New Revision: 51684
URL: http://svn.reactos.org/svn/reactos?rev=51684&view=rev Log: [USBEHCI_NEW] - Modify BuildBulkTransferQueueHead to support TransferBufferLengths larger than PAGE_SIZE * 5. - Acquire a SpinLock before adding QueueHeads to AsyncList and PendingList. - Dont request a new QueueHead for incomplete transfers in QueueHeadCompletion, as the memory for the just completed QueueHead has not been released yet. Doing so overwrites the m_TransferDescriptor[x] members with new address resulting in memory leaks. Instead request a new QueueHead after the QueueHead has been freed in QueueHeadCleanup. - Fix a bug where a QueueHead was removed from the m_CompletedRequestAsyncList instead of the m_PendingRequestAsyncList. - Temporary hackfix InternalCalculateTransferLength to return the TransferBufferLength. This hack will be removed as soon as possible. - With these changes the hub and ehci driver allow viewing content of and transfers to/from usb disks.
Modified: branches/usb-bringup/drivers/usb/usbehci_new/usb_queue.cpp branches/usb-bringup/drivers/usb/usbehci_new/usb_request.cpp
Modified: branches/usb-bringup/drivers/usb/usbehci_new/usb_queue.cpp URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbehci_... ============================================================================== --- branches/usb-bringup/drivers/usb/usbehci_new/usb_queue.cpp [iso-8859-1] (original) +++ branches/usb-bringup/drivers/usb/usbehci_new/usb_queue.cpp [iso-8859-1] Thu May 12 13:35:06 2011 @@ -137,8 +137,7 @@ // Loop through the pending list and iterrate one for each QueueHead that // has a IRP to complete. // - - + return 0; }
@@ -149,6 +148,7 @@ PQUEUE_HEAD QueueHead; NTSTATUS Status; ULONG Type; + KIRQL OldLevel;
// // sanity check @@ -214,7 +214,9 @@ // // Add it to the pending list // + KeAcquireSpinLock(&m_Lock, &OldLevel); LinkQueueHead(AsyncListQueueHead, QueueHead); + KeReleaseSpinLock(&m_Lock, OldLevel); }
@@ -251,12 +253,12 @@
*OutRequest = NULL; Status = InternalCreateUSBRequest(&UsbRequest); - + if (NT_SUCCESS(Status)) { *OutRequest = UsbRequest; } - + return Status; }
@@ -432,41 +434,20 @@ PQUEUE_HEAD CurrentQH, NTSTATUS Status) { - IUSBRequest *Request; - PQUEUE_HEAD NewQueueHead; + KIRQL OldLevel;
// // now unlink the queue head // FIXME: implement chained queue heads // + + KeAcquireSpinLock(&m_Lock, &OldLevel); + UnlinkQueueHead(CurrentQH);
- // - // get contained usb request - // - Request = (IUSBRequest*)CurrentQH->Request; - - // - // check if the request is complete - // - if (Request->IsRequestComplete() == FALSE) - { - // - // request is still in complete - // get new queue head - // - Status = Request->GetQueueHead(&NewQueueHead); - - // - // add to pending list - // - InsertTailList(&m_PendingRequestAsyncList, &NewQueueHead->LinkedQueueHeads); - } - - // - // put queue head into completed queue head list - // InsertTailList(&m_CompletedRequestAsyncList, &CurrentQH->LinkedQueueHeads); + + KeReleaseSpinLock(&m_Lock, OldLevel);
}
@@ -508,7 +489,6 @@ // Request = (IUSBRequest*)QueueHead->Request;
- // // move to next entry // @@ -568,6 +548,7 @@ CUSBQueue::QueueHeadCleanup( PQUEUE_HEAD CurrentQH) { + PQUEUE_HEAD NewQueueHead; IUSBRequest * Request; BOOLEAN ShouldReleaseWhenDone; USBD_STATUS UrbStatus; @@ -625,9 +606,47 @@ }
// - // notify request that a queue head has been completed - // - Request->CompletionCallback(STATUS_SUCCESS /*FIXME*/, UrbStatus, CurrentQH); + // Check if the transfer was completed and if UrbStatus is ok + // + if ((Request->IsRequestComplete() == FALSE) && (UrbStatus == USBD_STATUS_SUCCESS)) + { + // + // let IUSBRequest free the queue head + // + Request->FreeQueueHead(CurrentQH); + + // + // request is incomplete, get new queue head + // + if (Request->GetQueueHead(&NewQueueHead) == STATUS_SUCCESS) + { + // + // add to pending list + // + InsertTailList(&m_PendingRequestAsyncList, &NewQueueHead->LinkedQueueHeads); + + // + // Done for now + // + return; + } + DPRINT1("Unable to create a new QueueHead\n"); + PC_ASSERT(FALSE); + + // + // Else there was a problem + // FIXME: Find better return + UrbStatus = USBD_STATUS_INSUFFICIENT_RESOURCES; + } + + if (UrbStatus != USBD_STATUS_SUCCESS) PC_ASSERT(FALSE); + + // + // notify request that a transfer has completed + // + Request->CompletionCallback(UrbStatus != USBD_STATUS_SUCCESS ? STATUS_UNSUCCESSFUL : STATUS_SUCCESS, + UrbStatus, + CurrentQH);
// // let IUSBRequest free the queue head @@ -666,6 +685,7 @@ KIRQL OldLevel; PLIST_ENTRY Entry; PQUEUE_HEAD CurrentQH; + IUSBRequest *Request;
DPRINT("CUSBQueue::CompleteAsyncRequests\n");
@@ -692,6 +712,11 @@ CurrentQH = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
// + // Get the Request for this QueueHead + // + Request = (IUSBRequest*) CurrentQH->Request; + + // // complete request now // QueueHeadCleanup(CurrentQH); @@ -705,7 +730,7 @@ // // remove first entry // - Entry = RemoveHeadList(&m_CompletedRequestAsyncList); + Entry = RemoveHeadList(&m_PendingRequestAsyncList);
// // get queue head structure @@ -713,7 +738,7 @@ CurrentQH = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
// - // Add it to the pending list + // Add it to the AsyncList list // LinkQueueHead(AsyncListQueueHead, CurrentQH); }
Modified: branches/usb-bringup/drivers/usb/usbehci_new/usb_request.cpp URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbehci_... ============================================================================== --- branches/usb-bringup/drivers/usb/usbehci_new/usb_request.cpp [iso-8859-1] (original) +++ branches/usb-bringup/drivers/usb/usbehci_new/usb_request.cpp [iso-8859-1] Thu May 12 13:35:06 2011 @@ -86,6 +86,16 @@ ULONG m_TransferBufferLength;
// + // current transfer length + // + ULONG m_TransferBufferLengthCompleted; + + // + // Total Transfer Length + // + ULONG m_TotalBytesTransferred; + + // // transfer buffer MDL // PMDL m_TransferBufferMDL; @@ -169,6 +179,12 @@ m_TransferBufferMDL = TransferBuffer; m_DeviceAddress = DeviceAddress; m_EndpointDescriptor = EndpointDescriptor; + m_TotalBytesTransferred = 0; + + // + // Set Length Completed to 0 + // + m_TransferBufferLengthCompleted = 0;
// // allocate completion event @@ -208,6 +224,7 @@ PC_ASSERT(Irp);
m_DmaManager = DmaManager; + m_TotalBytesTransferred = 0;
// // get current irp stack location @@ -244,7 +261,7 @@ case URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER: { // - // bulk / interrupt transfer + // bulk interrupt transfer // if (Urb->UrbBulkOrInterruptTransfer.TransferBufferLength) { @@ -281,6 +298,10 @@ // FIXME: Does hub driver already do this when passing MDL? // MmBuildMdlForNonPagedPool(m_TransferBufferMDL); + + // + // Keep that ehci created the MDL and needs to free it. + // } else { @@ -291,6 +312,11 @@ // save buffer length // m_TransferBufferLength = Urb->UrbBulkOrInterruptTransfer.TransferBufferLength; + + // + // Set Length Completed to 0 + // + m_TransferBufferLengthCompleted = 0;
// // get endpoint descriptor @@ -357,7 +383,6 @@ // // Check if the MDL was created // - if (!Urb->UrbBulkOrInterruptTransfer.TransferBufferMDL) { // @@ -439,6 +464,7 @@ // // store urb status // + DPRINT1("Request Cancelled\n"); Urb->UrbHeader.Status = USBD_STATUS_CANCELED; Urb->UrbHeader.Length = 0;
@@ -521,6 +547,18 @@ // // FIXME: check if request was split // + + // + // Check if the transfer was completed, only valid for Bulk Transfers + // + if ((m_TransferBufferLengthCompleted < m_TransferBufferLength) + && (GetTransferType() == USB_ENDPOINT_TYPE_BULK)) + { + // + // Transfer not completed + // + return FALSE; + } return TRUE; } //---------------------------------------------------------------------------------------- @@ -642,7 +680,7 @@ // now initialize the queue head // QueueHead->EndPointCharacteristics.DeviceAddress = GetDeviceAddress(); - + if (m_EndpointDescriptor) { // @@ -751,7 +789,7 @@ ULONG TransferDescriptorCount, Index; ULONG BytesAvailable, BufferIndex; PVOID Base; - ULONG PageOffset; + ULONG PageOffset, CurrentTransferBufferLength;
// // Allocate the queue head @@ -770,24 +808,59 @@ // sanity checks // PC_ASSERT(QueueHead); - - // - // FIXME: support more than one descriptor - // - PC_ASSERT(m_TransferBufferLength < PAGE_SIZE * 5); PC_ASSERT(m_TransferBufferLength);
- TransferDescriptorCount = 1; + // + // Max default of 3 descriptors + // + TransferDescriptorCount = 3;
// // get virtual base of mdl // Base = MmGetSystemAddressForMdlSafe(m_TransferBufferMDL, NormalPagePriority); - BytesAvailable = m_TransferBufferLength; + + // + // Increase the size of last transfer, 0 in case this is the first + // + Base = (PVOID)((ULONG_PTR)Base + m_TransferBufferLengthCompleted);
PC_ASSERT(m_EndpointDescriptor); PC_ASSERT(Base);
+ // + // Get the offset from page size + // + PageOffset = BYTE_OFFSET(Base); + + // + // PageOffset should only be > 0 if this is the first transfer for this requests + // + if ((PageOffset != 0) && (m_TransferBufferLengthCompleted != 0)) + { + ASSERT(FALSE); + } + + // + // Calculate the size of this transfer + // + if ((PageOffset != 0) && ((m_TransferBufferLength - m_TransferBufferLengthCompleted) >= (PAGE_SIZE * 4) + PageOffset)) + { + CurrentTransferBufferLength = (PAGE_SIZE * 4) + PageOffset; + } + else if ((m_TransferBufferLength - m_TransferBufferLengthCompleted) >= PAGE_SIZE * 5) + { + CurrentTransferBufferLength = PAGE_SIZE * 5; + } + else + CurrentTransferBufferLength = (m_TransferBufferLength - m_TransferBufferLengthCompleted); + + // + // Add current transfer length to transfer length completed + // + m_TransferBufferLengthCompleted += CurrentTransferBufferLength; + BytesAvailable = CurrentTransferBufferLength; + DPRINT("CurrentTransferBufferLength %x, m_TransferBufferLengthCompleted %x\n", CurrentTransferBufferLength, m_TransferBufferLengthCompleted);
DPRINT("EndPointAddress %x\n", m_EndpointDescriptor->bEndpointAddress); DPRINT("EndPointDirection %x\n", USB_ENDPOINT_DIRECTION_IN(m_EndpointDescriptor->bEndpointAddress)); @@ -833,9 +906,9 @@ for(BufferIndex = 0; BufferIndex < 5; BufferIndex++) { // - // setup buffer - // - if (BufferIndex == 0) + // If this is the first buffer of the first descriptor and there is a PageOffset + // + if ((BufferIndex == 0) && (PageOffset != 0) && (Index == 0)) { // // use physical address @@ -843,45 +916,25 @@ m_TransferDescriptors[Index]->BufferPointer[0] = MmGetPhysicalAddress(Base).LowPart;
// - // get offset within page - // - PageOffset = BYTE_OFFSET(m_TransferDescriptors[Index]->BufferPointer[0]); - - // - // check if request fills another page - // - if (PageOffset + BytesAvailable > PAGE_SIZE) - { - // - // move to next page - // - Base = (PVOID)ROUND_TO_PAGES(Base); - - // - // increment transfer bytes - // + // move to next page + // + Base = (PVOID)ROUND_TO_PAGES(Base); + + // + // increment transfer bytes + // + if (CurrentTransferBufferLength > PAGE_SIZE - PageOffset) m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = PAGE_SIZE - PageOffset; - - // - // decrement available byte count - // - BytesAvailable -= m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer; - - DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex], - BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable); - } - else - { - // - // request ends on the first buffer page - // - m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = BytesAvailable; - BytesAvailable = 0; - - DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex], - BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable); - break; - } + else + m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = CurrentTransferBufferLength; + + // + // decrement available byte count + // + BytesAvailable -= m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer; + + DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex], + BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable); } else { @@ -935,7 +988,7 @@ BytesAvailable -= BytesAvailable;
// - // done + // done as this is the last partial or full page // DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex], BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable); @@ -943,6 +996,12 @@ break; } } + + // + // Check if all bytes have been consumed + // + if (BytesAvailable == 0) + break; }
// @@ -971,6 +1030,12 @@ // // FIXME need dead queue transfer descriptor? // + + // + // Check if all bytes have been consumed + // + if (BytesAvailable == 0) + break; }
// @@ -982,7 +1047,7 @@ // Initialize the QueueHead // QueueHead->EndPointCharacteristics.DeviceAddress = GetDeviceAddress(); - + if (m_EndpointDescriptor) { // @@ -1440,10 +1505,12 @@ CUSBRequest::FreeQueueHead( IN struct _QUEUE_HEAD * QueueHead) { + LONG DescriptorCount; + // // FIXME: support chained queue heads // - PC_ASSERT(QueueHead == m_QueueHead); + //PC_ASSERT(QueueHead == m_QueueHead);
// // release queue head @@ -1458,32 +1525,40 @@ // // release transfer descriptors // - - if (m_TransferDescriptors[0]) - { - // - // release transfer descriptors - // - m_DmaManager->Release(m_TransferDescriptors[0], sizeof(QUEUE_TRANSFER_DESCRIPTOR)); - m_TransferDescriptors[0] = 0; - } - - if (m_TransferDescriptors[1]) - { - // - // release transfer descriptors - // - m_DmaManager->Release(m_TransferDescriptors[1], sizeof(QUEUE_TRANSFER_DESCRIPTOR)); - m_TransferDescriptors[1] = 0; - } - - if (m_TransferDescriptors[2]) - { - // - // release transfer descriptors - // - m_DmaManager->Release(m_TransferDescriptors[2], sizeof(QUEUE_TRANSFER_DESCRIPTOR)); - m_TransferDescriptors[2] = 0; + for (DescriptorCount = 0; DescriptorCount < 3; DescriptorCount++) + { + if (m_TransferDescriptors[DescriptorCount]) + { + // + // Calculate Total Bytes Transferred + // FIXME: Is this the correct method of determine bytes transferred? + // + if (USB_ENDPOINT_TYPE_BULK == GetTransferType()) + { + // + // sanity check + // + ASSERT(m_EndpointDescriptor); + + if (USB_ENDPOINT_DIRECTION_IN(m_EndpointDescriptor->bEndpointAddress)) + { + DPRINT1("m_TotalBytesTransferred %x, %x - %x\n", + m_TotalBytesTransferred, + m_TransferDescriptors[DescriptorCount]->TotalBytesToTransfer, + m_TransferDescriptors[DescriptorCount]->Token.Bits.TotalBytesToTransfer); + + m_TotalBytesTransferred += + m_TransferDescriptors[DescriptorCount]->TotalBytesToTransfer - + m_TransferDescriptors[DescriptorCount]->Token.Bits.TotalBytesToTransfer; + } + } + + // + // release transfer descriptors + // + m_DmaManager->Release(m_TransferDescriptors[DescriptorCount], sizeof(QUEUE_TRANSFER_DESCRIPTOR)); + m_TransferDescriptors[DescriptorCount] = 0; + } }
if (m_DescriptorPacket) @@ -1573,8 +1648,9 @@ { // // bulk in request - // - return m_TransferDescriptors[0]->TotalBytesToTransfer - m_TransferDescriptors[0]->Token.Bits.TotalBytesToTransfer; + // HACK: Properly determine transfer length + // + return m_TransferBufferLength;//m_TotalBytesTransferred; }
//
-
mjmartin＠svn.reactos.org