[mjmartin] 51684: [USBEHCI_NEW] - Modify BuildBulkTransferQueueHead to support TransferBufferLengths larger than PAGE_SIZE * 5. - Acquire a SpinLock before adding QueueHeads to AsyncList and Pendin...
12 May
2011
12 May
'11
1:35 p.m.
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;
}
//
4936
days inactive
4936
days old
0 comments
1 participants
participants (1)
-
mjmartin＠svn.reactos.org