[USBEHCI][USBPORT]

- Fix returning data length of iso packet

Author: janderwald

drivers/usb/usbehci/usbehci.c

@@ -2703,6 +2703,7 @@ EHCI_SubmitIsoTransfer(IN PVOID ehciExtension,
             if (PacketsThisITD > PacketsPerITD)
                 PacketsThisITD = PacketsPerITD;
             ASSERT(PacketsThisITD <= 8);
+            ASSERT(PacketsThisITD > 0);
 
             /* Setup buffer page 0 with device/endpoint info */
             ITD->HwTD.Buffer[0].DeviceAddress = DeviceAddress;
@@ -2718,6 +2719,9 @@ EHCI_SubmitIsoTransfer(IN PVOID ehciExtension,
             ASSERT(ITD->HwTD.Buffer[2].Multi > 0);
             ASSERT(ITD->HwTD.Buffer[2].Multi <= 3);
 
+            // store number of packets
+            ITD->PacketsThisITD = PacketsThisITD;
+
             /* Program each packet into a transaction slot */
             for (p = 0; p < PacketsThisITD; p++)
             {
@@ -2727,6 +2731,9 @@ EHCI_SubmitIsoTransfer(IN PVOID ehciExtension,
                 ULONG Offset;
                 ULONG ThisPageSelect;
 
+                /* store USBPORT iso packet ref*/
+                ITD->IsoPacket[p] = Packet;
+
                 /* Determine microframe slot: for period=1, slots 0,1,2,...7 */
                 MicroFrame = p * Period;
 
@@ -2792,7 +2799,7 @@ EHCI_SubmitIsoTransfer(IN PVOID ehciExtension,
                 ITD->HwTD.Transaction[MicroFrame].xOffset = Offset;
                 ITD->HwTD.Transaction[MicroFrame].PageSelect = ThisPageSelect;
                 ITD->HwTD.Transaction[MicroFrame].xLength = Packet->PacketLength;
-                ITD->HwTD.Transaction[MicroFrame].Status = EHCI_TOKEN_STATUS_ACTIVE >> 4;
+                ITD->HwTD.Transaction[MicroFrame].Status = 1 << 3;
 
                 /* Set IOC on the last transaction of the last iTD */
                 if (p == PacketsThisITD - 1)
@@ -2823,22 +2830,11 @@ EHCI_SubmitIsoTransfer(IN PVOID ehciExtension,
             KeMemoryBarrier();
             /* Store the frame index in the iTD for unlinking later */
             ITD->ScheduledFrame = FrameIndex;
-            /* clear next itd software link */
-            ITD->NextHcdTD = NULL;
 
             /* Track which frame this iTD was inserted at */
             if (ITDCount == 0)
                 EhciEndpoint->StartingFrame = FrameIndex;
 
-            if (!FirstITD)
-            {
-                FirstITD = ITD;
-            }
-            if (LastITD)
-            {
-                LastITD->NextHcdTD = ITD;
-            }
-            LastITD = ITD;
             PacketIndex += PacketsThisITD;
             CurrentFrame++;
             ITDCount++;
@@ -3917,18 +3913,18 @@ EHCI_PollIsoEndpoint(IN PEHCI_EXTENSION EhciExtension,
                 ITD = (PEHCI_HCD_ITD)((ULONG_PTR)ITD + Size);
                 continue;
             }
-
+            KeMemoryBarrier();
             /* Check if any initialized transaction in this iTD is still active.
              * Also verify at least one transaction was programmed to avoid
              * false-completing an allocated but not-yet-programmed iTD. */
             StillActive = FALSE;
             BOOLEAN HasProgrammedTransactions = FALSE;
-            for (TransIdx = 0; TransIdx < EHCI_MAX_ITD_TRANSACTIONS; TransIdx++)
+            ASSERT(ITD->PacketsThisITD);
+            for (TransIdx = 0; TransIdx < ITD->PacketsThisITD; TransIdx++)
             {
                 if (ITD->PacketLength[TransIdx] > 0)
                 {
                     HasProgrammedTransactions = TRUE;
-
                     ULONG Status = ITD->HwTD.Transaction[TransIdx].Status;
                     if (Status & (1 << 3))
                     {
@@ -3937,7 +3933,7 @@ EHCI_PollIsoEndpoint(IN PEHCI_EXTENSION EhciExtension,
                     }
                 }
             }
-
+            KeMemoryBarrier();
             if (HasProgrammedTransactions && !StillActive)
             {
                 /* All programmed transactions completed, process this iTD */
@@ -4397,8 +4393,12 @@ EHCI_ProcessCompletedITD(IN PEHCI_EXTENSION EhciExtension,
     if (!EhciTransfer || !EhciEndpoint)
         return;
 
+    /* make sure we are in sync */
+    KeMemoryBarrier();
+
     /* Check all transactions in the iTD */
-    for (TransactionIndex = 0; TransactionIndex < EHCI_MAX_ITD_TRANSACTIONS; TransactionIndex++)
+    ASSERT(ITD->PacketsThisITD);
+    for (TransactionIndex = 0; TransactionIndex < ITD->PacketsThisITD; TransactionIndex++)
     {
         /* Use PacketLength[] (original programmed length) to detect used transactions.
          * The hardware xLength field contains REMAINING bytes after transfer, so a
@@ -4408,7 +4408,7 @@ EHCI_ProcessCompletedITD(IN PEHCI_EXTENSION EhciExtension,
         {
             ULONG Status = ITD->HwTD.Transaction[TransactionIndex].Status;
             ULONG Remaining = ITD->HwTD.Transaction[TransactionIndex].xLength;
-
+            ULONG Completed;
             if ((Status & (1 << 3))) //EHCI_TOKEN_STATUS_ACTIVE >> 4))
             {
                 /* Transaction still active, not complete yet */
@@ -4421,20 +4421,27 @@ EHCI_ProcessCompletedITD(IN PEHCI_EXTENSION EhciExtension,
                 ASSERT((Status & (1 << 1)) == 0);
 
                 /* Transaction completed - bytes transferred = programmed - remaining */
-                TotalBytesTransferred += ITD->PacketLength[TransactionIndex] - Remaining;
+                Completed = ITD->PacketLength[TransactionIndex] - Remaining;
+                TotalBytesTransferred += Completed;
 
                 /* Check for errors (bit 2 = Transaction Error in 4-bit status) */
                 if (Status & 0x04)
                 {
                     DPRINT1("EHCI_ProcessCompletedITD: Transaction %d error, status 0x%x\n",
                             TransactionIndex, Status);
                     EhciTransfer->USBDStatus = USBD_STATUS_XACT_ERROR;
+                    ITD->IsoPacket[TransactionIndex]->CompletionStatus = USBD_STATUS_XACT_ERROR;
+                }
+                else
+                {
+                    ITD->IsoPacket[TransactionIndex]->CompletionStatus = USBD_STATUS_SUCCESS;
+                    ITD->IsoPacket[TransactionIndex]->BytesTransferred = Completed;
                 }
-
                 ITD->PacketStatus[TransactionIndex] = Status;
             }
         }
     }
+    KeMemoryBarrier();
 
     if (TransferComplete)
     {
@@ -4443,14 +4450,15 @@ EHCI_ProcessCompletedITD(IN PEHCI_EXTENSION EhciExtension,
 
         /* Remove iTD from frame list */
         EHCI_UnlinkITDFromFrameList(EhciExtension, ITD, ITD->ScheduledFrame, EhciTransfer);
-
-        /* Mark iTD as free */
-        ITD->TdFlags &= ~EHCI_HCD_ITD_FLAG_ALLOCATED;
-        ITD->NextHcdTD = NULL;
         ITD->EhciTransfer = NULL;
         ITD->EhciEndpoint = NULL;
         for(ULONG Index = 0; Index < EHCI_MAX_ITD_TRANSACTIONS; Index++)
+        {
             ITD->PacketLength[Index] = 0;
+            ITD->IsoPacket[Index] = NULL;
+        }
+        /* Mark iTD as free */
+        ITD->TdFlags &= ~EHCI_HCD_ITD_FLAG_ALLOCATED;
 
         EhciEndpoint->RemainITDs++;
 
@@ -4508,9 +4516,13 @@ EHCI_UnlinkITDFromFrameList(IN PEHCI_EXTENSION EhciExtension,
         DPRINT_EHCI("EHCI_UnlinkITDFromFrameList: Unlinked iTD from frame %d head\n", FrameIndex);
         KeMemoryBarrier();
     }
+    else
+    {
+        // LINKING ERROR
+        ASSERT(FALSE);
+    }
     RemoveEntryList(&ITD->ActiveITDEntry);
     RtlClearBits(&EhciExtension->IsoBitmap, ITD->ScheduledFrame, 1);
-    ITD->NextHcdTD = NULL;
     ITD->EhciTransfer = NULL;
 }
 

drivers/usb/usbehci/usbehci.h

@@ -104,16 +104,17 @@ typedef struct _EHCI_HCD_ITD {
   ULONG TdFlags;
   struct _EHCI_ENDPOINT * EhciEndpoint;
   struct _EHCI_TRANSFER * EhciTransfer;
-  struct _EHCI_HCD_ITD * NextHcdTD;
   ULONG PacketLength[EHCI_MAX_ITD_TRANSACTIONS];
   ULONG PacketStatus[EHCI_MAX_ITD_TRANSACTIONS];
   LIST_ENTRY ActiveITDEntry;
   LIST_ENTRY DoneLink;
+  PUSBPORT_ISO_PACKET_DATA IsoPacket[EHCI_MAX_ITD_TRANSACTIONS];
   ULONG ScheduledFrame; /* Frame list index where this iTD was inserted */
+  ULONG PacketsThisITD;
 #ifdef _WIN64
   ULONG Pad[16];
 #else
-  ULONG Pad[23];
+  ULONG Pad[15];
 #endif
 } EHCI_HCD_ITD, *PEHCI_HCD_ITD;
 C_ASSERT(sizeof(EHCI_HCD_ITD) == ROUND_UP(sizeof(EHCI_HCD_ITD), 32));

drivers/usb/usbport/iso.c

@@ -218,10 +218,11 @@ USBPORT_CompleteIsoTransfer(IN PVOID MiniPortExtension,
 {
     PUSBPORT_ENDPOINT Endpoint;
     PUSBPORT_TRANSFER Transfer;
+    PUSBPORT_ISO_TRANSFER_DATA IsoBlock;
     struct _URB_ISOCH_TRANSFER *IsoUrb;
-    USBD_ISO_PACKET_DESCRIPTOR *PacketDescriptor, * NextPacketDescriptor;
-    ULONG i, PacketLength;
-    ULONG CompletedLength = 0;
+    PUSBPORT_ISO_PACKET_DATA IsoPacketData;
+    USBD_ISO_PACKET_DESCRIPTOR *PacketDescriptor;
+    ULONG i;
     ULONG RemainingLength = TransferLength;
 
     DPRINT("USBPORT_CompleteIsoTransfer: TransferLength - %lu\n", TransferLength);
@@ -245,6 +246,14 @@ USBPORT_CompleteIsoTransfer(IN PVOID MiniPortExtension,
         return 0;
     }
 
+
+    IsoBlock = (PUSBPORT_ISO_TRANSFER_DATA)Transfer->IsoBlockPtr;
+    if (!IsoBlock)
+    {
+        DPRINT1("USBPORT_CompleteIsoTransfer: Invalid Transfer or URB\n");
+        return 0;
+    }
+
     IsoUrb = (struct _URB_ISOCH_TRANSFER *)Transfer->Urb;
 
     // Validate this is actually an ISO transfer
@@ -255,59 +264,26 @@ USBPORT_CompleteIsoTransfer(IN PVOID MiniPortExtension,
     }
 
     // Update packet descriptors with completion status
+    ASSERT(IsoBlock->TotalPackets == IsoUrb->NumberOfPackets);
     i = 0;
     do
     {
         PacketDescriptor = &IsoUrb->IsoPacket[i];
+        IsoPacketData = &IsoBlock->Packets[i];
 
-        if (i + 1 < IsoUrb->NumberOfPackets)
-        {
-            NextPacketDescriptor = &IsoUrb->IsoPacket[i+1];
-            PacketLength = NextPacketDescriptor->Offset - PacketDescriptor->Offset;
-        }
-        else
-        {
-            /* last packet */
-            PacketLength = IsoUrb->TransferBufferLength - PacketDescriptor->Offset;
-        }
-
-        if (RemainingLength >= PacketLength)
-        {
-            PacketDescriptor->Status = USBD_STATUS_SUCCESS;
-            PacketDescriptor->Length = PacketLength;
-            CompletedLength += PacketDescriptor->Length;
-            RemainingLength -= PacketDescriptor->Length;
-        }
-        else
-        {
-            PacketDescriptor->Status = USBD_STATUS_SUCCESS;
-            PacketDescriptor->Length = RemainingLength;
-            CompletedLength += RemainingLength;
-            if (RemainingLength > 0)
-            {
-                RemainingLength = 0;
-                i++;
-            }
-            break;
-        }
+        PacketDescriptor->Status = IsoPacketData->CompletionStatus;
+        PacketDescriptor->Length = IsoPacketData->BytesTransferred;
+        DPRINT("Status %x Length %u Index %u\n", PacketDescriptor->Status, PacketDescriptor->Length, i);
     }while(i++ < IsoUrb->NumberOfPackets);
 
-    // Mark remaining packets as not processed if any
-    for (; i < IsoUrb->NumberOfPackets; i++)
-    {
-        PacketDescriptor = &IsoUrb->IsoPacket[i];
-        PacketDescriptor->Status = USBD_STATUS_NOT_ACCESSED;
-        PacketDescriptor->Length = 0;
-    }
-
     // Complete the transfer
     USBPORT_MiniportCompleteTransfer(MiniPortExtension,
                                      MiniPortEndpoint,
                                      TransferParameters,
                                      USBD_STATUS_SUCCESS,
-                                     CompletedLength);
+                                     TransferLength);
 
-    DPRINT("USBPORT_CompleteIsoTransfer: Completed %lu bytes\n", CompletedLength);
-    return CompletedLength;
+    DPRINT("USBPORT_CompleteIsoTransfer: Completed %lu bytes\n", TransferLength);
+    return TransferLength;
 }