Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

["Henz, Patrick" <patrick.henz@hpe.com>]

Hi Michael,

I agree that XhcGetElapsedTime() would be better off in TimerLib, but I wasn't sure how the community would feel about adding to the interface.

As for your question, I was wondering the same thing, I'm not sure if there are any platforms that do not have a free-running timer that would be accessed through TimerLib. Because of this unknown I included a check on the return value from XhcGetElapsedTime(), if that value is 0 ElapsedTime is incremented regardless to avoid a potential hang. The behavior of the function essentially reverts to the pre "MdeModulePkg/XhciDxe: Fix Broken Timeouts" commit implementation.

Thanks,
Patrick Henz

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Brown
Sent: Thursday, July 6, 2023 8:02 AM
To: devel@edk2.groups.io; Henz, Patrick <patrick.henz@hpe.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

On 05/07/2023 21:15, Henz, Patrick wrote:
> REF:INVALID URI REMOVED
> g.cgi?id=2948__;!!NpxR!mnYVFUERkrUcRiNbM7pejt6qy6h5tilkuGydeM0skcGMAzS
> 1oEy1DSDT720vj4Le2cGtf_kmPYFIng$
> 
> XhciDxe uses the timer functionality provided by the boot services 
> table to detect timeout conditions. This breaks the driver's 
> ExitBootServices call back, as CoreExitBootServices halts the timer 
> before signaling the ExitBootServices event. If the host controller 
> fails to halt in the call back, the timeout condition will never occur 
> and the boot gets stuck in an indefinite spin loop. Use the free 
> running timer provided by TimerLib to calculate timeouts, avoiding the 
> potential hang.

Two points:

1. The XhcGetElapsedTime() function feels like a generally reusable abstraction that should exist within TimerLib, rather than being open-coded within XhciDxe.

2. Is the performance counter guaranteed to exist and work as expected on all platforms and CPU architectures?  (For example: what if the CPU does not support a constant TSC?)

Thanks,

Michael