Hi Rebecca, Thank you for this patch.
I have some minor suggestions marked inline as [SAMI]. Regards, Sami Mujawar On 03/01/2024, 20:44, "Rebecca Cran" <rebe...@os.amperecomputing.com <mailto:rebe...@os.amperecomputing.com>> wrote: Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran <rebe...@os.amperecomputing.com <mailto:rebe...@os.amperecomputing.com>> --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 8f02f38c64e3..912106eb6ad2 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -36,10 +36,14 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xFFFFFFFFFFFFULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -91,7 +95,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mNumTimerTicks = 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -202,8 +207,9 @@ WatchdogSetTimerPeriod ( { UINTN SystemCount; - // if TimerPeriod is 0, this is a request to stop the watchdog. - if (TimerPeriod == 0) { + // if TimerPeriod is 0 or we've exited boot services, + // this is a request to stop the watchdog. + if (TimerPeriod == 0 || mExitedBootServices) { [SAMI] I think we need extra brackets here. Also, should EFI_DEVICE_ERROR be returned if ExitBootServices is signalled but TimerPeriod != 0? i.e. we stop the watchdog but return an error to the caller so that it knows to fix the broken code. [/SAMI] mNumTimerTicks = 0; WatchdogDisable (); return EFI_SUCCESS; @@ -303,8 +309,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113159): https://edk2.groups.io/g/devel/message/113159 Mute This Topic: https://groups.io/mt/103510105/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
