Re: [PATCH 0/1] Fix XhciDxe Timeouts

[patrick.henz@hpe.com]

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 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