Hi, Baranee Yes, XHCI spec has the same statement for short packet.
Quote from Xhci spec: A Short Packet will trigger the generation of a Transfer Event TRB on the Event Ring if the Interrupt-on-Short (ISP) or Interrupt On Completion(IOC) flags are set in the TRB that the Short Packet was detected on. The Completion Code field of the Transfer Event shall be set to Short Packet. The Length field of the Transfer Event shall be set to the residual number of bytes not written to the Transfer TRBs' data buffer. I attach the final git patch for your review. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Feng Tian <feng.t...@intel.com> Thanks Feng -----Original Message----- From: Anbazhagan, Baraneedharan [mailto:anbazha...@hp.com] Sent: Wednesday, April 22, 2015 06:46 To: Laszlo Ersek; Tian, Feng Cc: edk2-devel@lists.sourceforge.net Subject: RE: [edk2] XHCI data transfer size Hi Feng, Verified that incase of short packet, EvtTrb->Lenth reports remaining size and change should be okay. Assuming code change will be applied to below 3 modules. MdeModulePkg\Bus\Pci\XhciDxe\XhciSched.c MdeModulePkg\Bus\Pci\XhciPei\XhciSched.c SourceLevelDebugPkg\Library\DebugCommunicationLibUsb3\DebugCommunicationLibUsb3Transfer.c Also, could you please rename Lenth to Length in XhciDxe module. Thanks. -Baranee CONFIDENTIALITY NOTICE: The information contained in this e-mail and any accompanying documents may contain information which is HP confidential or otherwise protected from disclosure. This transmission may also be protected by the attorney-client privilege, the attorney work-product privilege, or both. If you are not the intended recipient of this message, or if this message has been addressed to you in error, please immediately alert the sender by reply e-mail and then delete this message, including any attachments. Any dissemination, distribution or other use of the contents of this message by anyone other than the intended recipient is strictly prohibited. -----Original Message----- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Tuesday, April 21, 2015 6:53 AM To: Tian, Feng Cc: edk2-devel@lists.sourceforge.net; Anbazhagan, Baraneedharan Subject: Re: [edk2] XHCI data transfer size On 04/21/15 09:29, Tian, Feng wrote: > Laszlo, > > Please note the code is checking the field of Event TRBs, so you need to > refer to Xhci 1.0 spec Table 85 for the meaning of TRB Transfer Length field. Right; what I have downloaded is 1.1, but table 91 ("Offset 08h - Transfer Event TRB Field Definitions") indeed says about "TRB Transfer Length" that it carries the residue. (Even if the receive transfer is terminated with a short packet.) There are cases when the field seems to be set to the EDTLA instead (dependent on the Event Data (ED) bit, and the condition code), but those don't seem to be handled here at all. So I guess this change should be okay (I'm definitely new to XHCI, beyond the one QEMU patch that I mentioned). Thanks Laszlo > For your concern on Isoch Endpoint Error Handling, the Xhci spec states it's > for Error case rather than success case. > > Thanks > Feng > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, April 21, 2015 15:09 > To: Tian, Feng > Cc: edk2-devel@lists.sourceforge.net; Anbazhagan, Baraneedharan > Subject: Re: [edk2] XHCI data transfer size > > On 04/21/15 05:04, Tian, Feng wrote: >> Baranee & Laszlo, >> >> I agree this is a bug and I made a unit test on this logic and found why >> this issue didn't get raised before is because Fat&DiskIo driver will split >> big data transfer request (>64K) to multiple of 64K. This way will cause a >> request, that is a URB, only needs a transfer TRB, so it will not return >> wrong completed data length as the Urb->DataLen is only accumulated once for >> every URB request. >> >> But if a URB request needs multiple TRBs, then the Urb->Completed is >> calculated wrongly. >> >> Here is my proposed fix, could you help review it? >> >> Index: XhciSched.c >> =================================================================== >> --- XhciSched.c (revision 17189) >> +++ XhciSched.c (working copy) >> @@ -1137,7 +1137,7 @@ >> if ((TRBType == TRB_TYPE_DATA_STAGE) || >> (TRBType == TRB_TYPE_NORMAL) || >> (TRBType == TRB_TYPE_ISOCH)) { >> - CheckedUrb->Completed += (CheckedUrb->DataLen - EvtTrb->Lenth); >> + CheckedUrb->Completed += >> + (((TRANSFER_TRB_NORMAL*)TRBPtr)->Length - EvtTrb->Lenth); >> } >> >> The TRBPtr->Length field means expected transfer length, and the >> EvtTrb->Lenth means the residual number of bytes not transferred. > > Can you please explain why "EvtTrb->Lenth" is the residue (ie. the number of > bytes *not* transferred) instead of the number of bytes that have been > transferred? > > My understanding would be helped most if you could provide: > - an explanation of the code that implies this (it's quite complicated > code...) > - and a reference from the XHCI spec that the code implements. > > I'm asking because (as I wrote earlier) the XHCI spec defines > EvtTrb->Lenth with different meanings in different contexts. In some > contexts it is indeed a residue, but in other contexts EvtTrb->Lenth is the > number of bytes actually transferred. I'm unsure which case we have at hand. > > Thanks! > Laszlo >
0001-SourceLevelDebugPkg-Correct-the-wrong-calculation-of.patch
Description: 0001-SourceLevelDebugPkg-Correct-the-wrong-calculation-of.patch
0001-MdeModulePkg-Correct-the-wrong-calculation-of-the-co.patch
Description: 0001-MdeModulePkg-Correct-the-wrong-calculation-of-the-co.patch
------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
