Re: [edk2-devel] [PATCH 0/1] Fix XhciDxe Timeouts

["Ni, Ray" <ray.ni@intel.com>]

Patrick,
Please make sure unit test is done for the code you changed.
If you cannot find environment to test the changes, I prefer to not make the change.

Thanks,
Ray

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Thursday, September 3, 2020 11:55 AM
> To: devel@edk2.groups.io; patrick.henz@hpe.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/1] Fix XhciDxe Timeouts
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Henz,
> > Patrick
> > Sent: Thursday, September 3, 2020 7:03 AM
> > To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: Re: [edk2-devel] [PATCH 0/1] Fix XhciDxe Timeouts
> >
> > Hello Hao,
> >
> > Yes, I can provide patches for the UHCI and EHCI drivers. I haven't done any
> > of the work on either of these yet but will hopefully get to them in the next
> > day or so.
> 
> 
> Thanks Patrick,
> 
> Could you help to add the UHCI and EHCI changes together with the existing
> XHCI patch into one series for the next version?
> That will make a series which include 3 patches.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Thanks,
> > Patrick Henz
> >
> > -----Original Message-----
> > From: Wu, Hao A [mailto:hao.a.wu@intel.com]
> > Sent: Wednesday, September 2, 2020 12:09 AM
> > To: Henz, Patrick <patrick.henz@hpe.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [PATCH 0/1] Fix XhciDxe Timeouts
> >
> > > -----Original Message-----
> > > From: patrick.henz@hpe.com <patrick.henz@hpe.com>
> > > Sent: Wednesday, September 2, 2020 2:55 AM
> > > To: devel@edk2.groups.io
> > > Cc: henz <patrick.henz@hpe.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > > Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: [PATCH 0/1] Fix XhciDxe Timeouts
> > >
> > > From: henz <patrick.henz@hpe.com>
> > >
> > > Timeouts in the XhciDxe driver are taking longer than expected due to
> > > the timeout loops not accounting for code execution time. As en
> > > example, 5 second timeouts have been observed to take around 36
> > seconds to complete.
> > > Use SetTimer and Create/CheckEvent from Boot Services to determine
> > > when timeout occurred. This patch was tested using forced timeouts and
> > > print statements with QEmu as well as phycial hardware. The forced
> > > timeouts were implemented in code via static variables that guaranteed
> > > a timeout the first time the function with the broken timeout was called.
> > >
> > > Example:
> > >
> > > XhcExecTransfer (
> > >   .
> > >   .
> > >  )
> > > {
> > >   .
> > >   .
> > >   static int do_once = 1;  // test line
> > >   .
> > >   .
> > >   do {
> > >     Finished = XhcCheckUrbResult (Xhc, Urb);
> > >     if (do_once) Finished = 0; // test line
> > >     if (Finished) {
> > >       break;
> > >     }
> > >     gBS->Stall (XHC_1_MICROSECOND);
> > >   } while (!EFI_ERROR(TimerStatus) && EFI_ERROR(gBS->CheckEvent
> > > (TimeoutEvent)));
> > >
> > >   do_once = 0; // test line
> > >
> > > Using this forced timeout approach the correct timeouts were observed
> > > on both hardware and in QEmu.
> > >
> > > Similar broken timeout loops have been found in the Uhci and Ehci
> > > drivers. This patch does not fix those issues.
> >
> >
> > Hello Patrick,
> >
> > Besides the comments made by Ray in patch 1, could you help to provide 2
> > more patches for UHCI and EHCI drivers as well for complete enhancement?
> > Thanks in advance.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Signed-off-by: Patrick Henz <patrick.henz@hpe.com>
> > >
> > > Patrick Henz (1):
> > >   MdeModulePkg/XhciDxe: Fix Broken Timeouts
> > >
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 28 ++++++++++++++++---
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 34
> > > +++++++++++++++++-------
> > >  2 files changed, 49 insertions(+), 13 deletions(-)
> > >
> > > --
> > > 2.27.0
> >
> >
> >