Hi,
If a module is has a dependency on the Timer Arch Protocol, that module
should list the Timer Arch Protocol as only of its dependencies in the
dependency expression.
I agree that we should avoid a hang condition, but it is not clear if
new return statuses need to be added for this case where a module may not
have a correct dependency expression.
An ASSERT() or DEBUG() message may be a better way to inform the
developer that they may have an incorrect dependency expression.
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ying-Tsun
> Huang via groups.io
> Sent: Tuesday, September 20, 2022 7:10 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.w...@intel.com>; Bi, Dandan <dandan...@intel.com>;
> Gao, Liming <gaolim...@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Core: Fix the potential
> hang of calling SetTimer.
>
> When calling SetTimer with Type is not TimerCancel and TriggerTime is 0,
> gTimer is used to get the timer period. However, gTimer is NULL before
> EFI_TIMER_ARCH_PROTOCOL is installed. Adding the check of gTimer and
> return EFI_NOT_READY to avoid the hang.
>
> Cc: Jian J Wang <jian.j.w...@intel.com>
> Cc: Dandan Bi <dandan...@intel.com>
> Cc: Liming Gao <gaolim...@byosoft.com.cn>
> Signed-off-by: Ying-Tsun Huang <ying-tsun.hu...@amd.com>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.h | 2 ++
> MdeModulePkg/Core/Dxe/Event/Timer.c | 14 ++++++++++----
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 815a6b4bd844..acf0b244aeda 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -1538,6 +1538,8 @@ CoreCreateEventInternal (
> @retval EFI_SUCCESS The event has been set to be signaled at the
> requested time
> @retval EFI_INVALID_PARAMETER Event or Type is not valid
> + @retval EFI_NOT_READY Type is not TimerCancel, TriggerTime is 0,
> but
> + EFI_TIMER_ARCH_PROTOCOL is not installed yet
>
> **/
> EFI_STATUS
> diff --git a/MdeModulePkg/Core/Dxe/Event/Timer.c
> b/MdeModulePkg/Core/Dxe/Event/Timer.c
> index 29e507c67c50..1621814ef3c5 100644
> --- a/MdeModulePkg/Core/Dxe/Event/Timer.c
> +++ b/MdeModulePkg/Core/Dxe/Event/Timer.c
> @@ -229,6 +229,8 @@ CoreTimerTick (
> @retval EFI_SUCCESS The event has been set to be signaled at the
> requested time
> @retval EFI_INVALID_PARAMETER Event or Type is not valid
> + @retval EFI_NOT_READY Type is not TimerCancel, TriggerTime is 0,
> but
> + EFI_TIMER_ARCH_PROTOCOL is not installed yet
>
> **/
> EFI_STATUS
> @@ -255,6 +257,14 @@ CoreSetTimer (
> return EFI_INVALID_PARAMETER;
> }
>
> + if ((Type != TimerCancel) && (TriggerTime == 0)) {
> + if (gTimer) {
> + gTimer->GetTimerPeriod (gTimer, &TriggerTime);
> + } else {
> + return EFI_NOT_READY;
> + }
> + }
> +
> CoreAcquireLock (&mEfiTimerLock);
>
> //
> @@ -270,10 +280,6 @@ CoreSetTimer (
>
> if (Type != TimerCancel) {
> if (Type == TimerPeriodic) {
> - if (TriggerTime == 0) {
> - gTimer->GetTimerPeriod (gTimer, &TriggerTime);
> - }
> -
> Event->Timer.Period = TriggerTime;
> }
>
> --
> 2.25.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94085): https://edk2.groups.io/g/devel/message/94085
Mute This Topic: https://groups.io/mt/93824436/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
