* [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test @ 2022-10-20 3:00 duntan 2022-10-27 2:08 ` Ni, Ray 0 siblings, 1 reply; 6+ messages in thread From: duntan @ 2022-10-20 3:00 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar Disable/Restore HpetTimer before and after running the Dxe CpuExceptionHandlerLib unit test module. During the UnitTest, a new Idt is initialized for the test. There is no handler for timer intrrupt in this new idt. After the test module, HpetTimer does not work any more since the comparator value register and main counter value register for timer does not match. To fix this issue, disable/restore HpetTimer before and after Unit Test if HpetTimer driver has been dispatched. Besides, send Apic EOI before restore HpetTimer. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf | 2 ++ UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf index e3dbe7b9ab..24f905936c 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf @@ -43,6 +43,7 @@ HobLib UefiBootServicesTableLib CpuPageTableLib + LocalApicLib [Guids] gEfiHobMemoryAllocStackGuid @@ -53,6 +54,7 @@ [Protocols] gEfiMpServiceProtocolGuid + gEfiTimerArchProtocolGuid [Depex] gEfiMpServiceProtocolGuid diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c index 917fc549bf..045f39fa00 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c @@ -8,6 +8,8 @@ #include "CpuExceptionHandlerTest.h" #include <Library/UefiBootServicesTableLib.h> +#include <Library/LocalApicLib.h> +#include <Protocol/Timer.h> /** Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR buffer. @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( { EFI_STATUS Status; UNIT_TEST_FRAMEWORK_HANDLE Framework; + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; + UINT64 TimerPeriod; - Framework = NULL; + Framework = NULL; + TimerArchProtocol = NULL; + TimerPeriod = 0; DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, UNIT_TEST_APP_VERSION)); @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( goto EXIT; } + // + // If HpetTimer driver has been dispatched, disable HpetTimer before Unit Test. + // + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID **)&TimerArchProtocol); + if (TimerArchProtocol != NULL) { + Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol, &TimerPeriod); + ASSERT_EFI_ERROR (Status); + if (TimerPeriod > 0) { + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable HpetTimer.\n")); + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, 0); + ASSERT_EFI_ERROR (Status); + } + } + // // Execute the tests. // Status = RunAllTestSuites (Framework); + // + // Restore HpetTimer after Unit Test. + // Send APIC EOI before SetTimerPeriod. + // + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { + DEBUG ((DEBUG_INFO, "Restore HpetTimer after DxeCpuExceptionHandlerLib UnitTest.\n")); + SendApicEoi (); + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, TimerPeriod); + ASSERT_EFI_ERROR (Status); + } + EXIT: if (Framework) { FreeUnitTestFramework (Framework); -- 2.31.1.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test 2022-10-20 3:00 [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test duntan @ 2022-10-27 2:08 ` Ni, Ray 2022-10-27 2:26 ` [edk2-devel] " Michael D Kinney 0 siblings, 1 reply; 6+ messages in thread From: Ni, Ray @ 2022-10-27 2:08 UTC (permalink / raw) To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Tan, Dun <dun.tan@intel.com> > Sent: Thursday, October 20, 2022 11:01 AM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, > Rahul R <rahul.r.kumar@intel.com> > Subject: [PATCH] UefiCpuPkg: Restore HpetTimer after > CpuExceptionHandlerLib test > > Disable/Restore HpetTimer before and after running the Dxe > CpuExceptionHandlerLib unit test module. During the UnitTest, a > new Idt is initialized for the test. There is no handler for timer > intrrupt in this new idt. After the test module, HpetTimer does > not work any more since the comparator value register and main > counter value register for timer does not match. To fix this issue, > disable/restore HpetTimer before and after Unit Test if HpetTimer > driver has been dispatched. Besides, send Apic EOI before restore > HpetTimer. > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > --- > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > dlerLibUnitTest.inf | 2 ++ > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > dlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerLibUnitTest.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerLibUnitTest.inf > index e3dbe7b9ab..24f905936c 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerLibUnitTest.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerLibUnitTest.inf > @@ -43,6 +43,7 @@ > HobLib > UefiBootServicesTableLib > CpuPageTableLib > + LocalApicLib > > [Guids] > gEfiHobMemoryAllocStackGuid > @@ -53,6 +54,7 @@ > > [Protocols] > gEfiMpServiceProtocolGuid > + gEfiTimerArchProtocolGuid > > [Depex] > gEfiMpServiceProtocolGuid > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerUnitTest.c > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerUnitTest.c > index 917fc549bf..045f39fa00 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerUnitTest.c > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > andlerUnitTest.c > @@ -8,6 +8,8 @@ > > #include "CpuExceptionHandlerTest.h" > #include <Library/UefiBootServicesTableLib.h> > +#include <Library/LocalApicLib.h> > +#include <Protocol/Timer.h> > > /** > Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR > buffer. > @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( > { > EFI_STATUS Status; > UNIT_TEST_FRAMEWORK_HANDLE Framework; > + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; > + UINT64 TimerPeriod; > > - Framework = NULL; > + Framework = NULL; > + TimerArchProtocol = NULL; > + TimerPeriod = 0; > > DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, > UNIT_TEST_APP_VERSION)); > > @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( > goto EXIT; > } > > + // > + // If HpetTimer driver has been dispatched, disable HpetTimer before Unit > Test. > + // > + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID > **)&TimerArchProtocol); > + if (TimerArchProtocol != NULL) { > + Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol, > &TimerPeriod); > + ASSERT_EFI_ERROR (Status); > + if (TimerPeriod > 0) { > + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable > HpetTimer.\n")); > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, 0); > + ASSERT_EFI_ERROR (Status); > + } > + } > + > // > // Execute the tests. > // > Status = RunAllTestSuites (Framework); > > + // > + // Restore HpetTimer after Unit Test. > + // Send APIC EOI before SetTimerPeriod. > + // > + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { > + DEBUG ((DEBUG_INFO, "Restore HpetTimer after > DxeCpuExceptionHandlerLib UnitTest.\n")); > + SendApicEoi (); > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, > TimerPeriod); > + ASSERT_EFI_ERROR (Status); > + } > + > EXIT: > if (Framework) { > FreeUnitTestFramework (Framework); > -- > 2.31.1.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test 2022-10-27 2:08 ` Ni, Ray @ 2022-10-27 2:26 ` Michael D Kinney 2022-10-27 2:32 ` Ni, Ray [not found] ` <1721CC636147BB21.550@groups.io> 0 siblings, 2 replies; 6+ messages in thread From: Michael D Kinney @ 2022-10-27 2:26 UTC (permalink / raw) To: devel@edk2.groups.io, Ni, Ray, Tan, Dun, Kinney, Michael D Cc: Dong, Eric, Kumar, Rahul R Hi Ray, I do not think it makes sense for the CpuExceptionHandlerLib unit test to directly access the IO APIC. It assumes that an IO APIC based timer interrupt is in use. Is it possible that this is a bug/race condition in the HpetDxe driver. Calling the Timer AP to set timer period from 0 -> Non Zero enables the timer. There could be a pending timer interrupt before this transition that would need to be cleared. Calling the Timer AP to set the timer period from Non Zero -> 0 disables the generation of timer interrupts. We need to make sure this disable action does not generate even one more pending timer interrupt after the timer is disabled. And we need to make sure a timer interrupt does not enter the pending state in the logic that is disabling the timer. We may need to always do an extra clear after the timer interrupt is disabled in case a timer interrupt is asserted during the disable logic. I suspect this unit test use case is not commonly exercised. Normal boots will go from 0 -> Non Zero period and may then optionally change the timer period. But going from Non Zero -> 0 timer period is not likely used. Seems like a good reason to add some Timer AP unit tests too. Thanks, Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Wednesday, October 26, 2022 7:08 PM > To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com> > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test > > Reviewed-by: Ray Ni <ray.ni@intel.com> > > > -----Original Message----- > > From: Tan, Dun <dun.tan@intel.com> > > Sent: Thursday, October 20, 2022 11:01 AM > > To: devel@edk2.groups.io > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, > > Rahul R <rahul.r.kumar@intel.com> > > Subject: [PATCH] UefiCpuPkg: Restore HpetTimer after > > CpuExceptionHandlerLib test > > > > Disable/Restore HpetTimer before and after running the Dxe > > CpuExceptionHandlerLib unit test module. During the UnitTest, a > > new Idt is initialized for the test. There is no handler for timer > > intrrupt in this new idt. After the test module, HpetTimer does > > not work any more since the comparator value register and main > > counter value register for timer does not match. To fix this issue, > > disable/restore HpetTimer before and after Unit Test if HpetTimer > > driver has been dispatched. Besides, send Apic EOI before restore > > HpetTimer. > > > > Signed-off-by: Dun Tan <dun.tan@intel.com> > > Cc: Eric Dong <eric.dong@intel.com> > > Cc: Ray Ni <ray.ni@intel.com> > > Cc: Rahul Kumar <rahul1.kumar@intel.com> > > --- > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > dlerLibUnitTest.inf | 2 ++ > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > dlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- > > 2 files changed, 34 insertions(+), 1 deletion(-) > > > > diff --git > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerLibUnitTest.inf > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerLibUnitTest.inf > > index e3dbe7b9ab..24f905936c 100644 > > --- > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerLibUnitTest.inf > > +++ > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerLibUnitTest.inf > > @@ -43,6 +43,7 @@ > > HobLib > > UefiBootServicesTableLib > > CpuPageTableLib > > + LocalApicLib > > > > [Guids] > > gEfiHobMemoryAllocStackGuid > > @@ -53,6 +54,7 @@ > > > > [Protocols] > > gEfiMpServiceProtocolGuid > > + gEfiTimerArchProtocolGuid > > > > [Depex] > > gEfiMpServiceProtocolGuid > > diff --git > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerUnitTest.c > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerUnitTest.c > > index 917fc549bf..045f39fa00 100644 > > --- > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerUnitTest.c > > +++ > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > andlerUnitTest.c > > @@ -8,6 +8,8 @@ > > > > #include "CpuExceptionHandlerTest.h" > > #include <Library/UefiBootServicesTableLib.h> > > +#include <Library/LocalApicLib.h> > > +#include <Protocol/Timer.h> > > > > /** > > Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR > > buffer. > > @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( > > { > > EFI_STATUS Status; > > UNIT_TEST_FRAMEWORK_HANDLE Framework; > > + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; > > + UINT64 TimerPeriod; > > > > - Framework = NULL; > > + Framework = NULL; > > + TimerArchProtocol = NULL; > > + TimerPeriod = 0; > > > > DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, > > UNIT_TEST_APP_VERSION)); > > > > @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( > > goto EXIT; > > } > > > > + // > > + // If HpetTimer driver has been dispatched, disable HpetTimer before Unit > > Test. > > + // > > + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID > > **)&TimerArchProtocol); > > + if (TimerArchProtocol != NULL) { > > + Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol, > > &TimerPeriod); > > + ASSERT_EFI_ERROR (Status); > > + if (TimerPeriod > 0) { > > + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable > > HpetTimer.\n")); > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, 0); > > + ASSERT_EFI_ERROR (Status); > > + } > > + } > > + > > // > > // Execute the tests. > > // > > Status = RunAllTestSuites (Framework); > > > > + // > > + // Restore HpetTimer after Unit Test. > > + // Send APIC EOI before SetTimerPeriod. > > + // > > + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { > > + DEBUG ((DEBUG_INFO, "Restore HpetTimer after > > DxeCpuExceptionHandlerLib UnitTest.\n")); > > + SendApicEoi (); > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, > > TimerPeriod); > > + ASSERT_EFI_ERROR (Status); > > + } > > + > > EXIT: > > if (Framework) { > > FreeUnitTestFramework (Framework); > > -- > > 2.31.1.windows.1 > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test 2022-10-27 2:26 ` [edk2-devel] " Michael D Kinney @ 2022-10-27 2:32 ` Ni, Ray [not found] ` <1721CC636147BB21.550@groups.io> 1 sibling, 0 replies; 6+ messages in thread From: Ni, Ray @ 2022-10-27 2:32 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io, Tan, Dun Cc: Dong, Eric, Kumar, Rahul R Mike, I agree there might be a bug in HPET timer driver where the EOI should be issued within that driver. Original idea was to avoid changing a driver used heavily in production firmware due to a new unit test failure and that test scenario is not common in production firmware. We would like to do more analysis on the impact if sending EOI when disabling timer and then move the EOI sending from the test code to HPET timer. Thanks, Ray > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, October 27, 2022 10:26 AM > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun > <dun.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com> > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > CpuExceptionHandlerLib test > > Hi Ray, > > I do not think it makes sense for the CpuExceptionHandlerLib unit > test to directly access the IO APIC. It assumes that an IO APIC > based timer interrupt is in use. > > Is it possible that this is a bug/race condition in the HpetDxe driver. > > Calling the Timer AP to set timer period from 0 -> Non Zero enables > the timer. There could be a pending timer interrupt before this > transition that would need to be cleared. > > Calling the Timer AP to set the timer period from Non Zero -> 0 > disables the generation of timer interrupts. We need to make > sure this disable action does not generate even one more > pending timer interrupt after the timer is disabled. And > we need to make sure a timer interrupt does not enter the > pending state in the logic that is disabling the timer. > We may need to always do an extra clear after the timer > interrupt is disabled in case a timer interrupt is asserted > during the disable logic. > > I suspect this unit test use case is not commonly exercised. > Normal boots will go from 0 -> Non Zero period and may then > optionally change the timer period. But going from > Non Zero -> 0 timer period is not likely used. > > Seems like a good reason to add some Timer AP unit tests too. > > Thanks, > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, > Ray > > Sent: Wednesday, October 26, 2022 7:08 PM > > To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io > > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com> > > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > CpuExceptionHandlerLib test > > > > Reviewed-by: Ray Ni <ray.ni@intel.com> > > > > > -----Original Message----- > > > From: Tan, Dun <dun.tan@intel.com> > > > Sent: Thursday, October 20, 2022 11:01 AM > > > To: devel@edk2.groups.io > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, > > > Rahul R <rahul.r.kumar@intel.com> > > > Subject: [PATCH] UefiCpuPkg: Restore HpetTimer after > > > CpuExceptionHandlerLib test > > > > > > Disable/Restore HpetTimer before and after running the Dxe > > > CpuExceptionHandlerLib unit test module. During the UnitTest, a > > > new Idt is initialized for the test. There is no handler for timer > > > intrrupt in this new idt. After the test module, HpetTimer does > > > not work any more since the comparator value register and main > > > counter value register for timer does not match. To fix this issue, > > > disable/restore HpetTimer before and after Unit Test if HpetTimer > > > driver has been dispatched. Besides, send Apic EOI before restore > > > HpetTimer. > > > > > > Signed-off-by: Dun Tan <dun.tan@intel.com> > > > Cc: Eric Dong <eric.dong@intel.com> > > > Cc: Ray Ni <ray.ni@intel.com> > > > Cc: Rahul Kumar <rahul1.kumar@intel.com> > > > --- > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > dlerLibUnitTest.inf | 2 ++ > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > dlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- > > > 2 files changed, 34 insertions(+), 1 deletion(-) > > > > > > diff --git > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerLibUnitTest.inf > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerLibUnitTest.inf > > > index e3dbe7b9ab..24f905936c 100644 > > > --- > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerLibUnitTest.inf > > > +++ > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerLibUnitTest.inf > > > @@ -43,6 +43,7 @@ > > > HobLib > > > UefiBootServicesTableLib > > > CpuPageTableLib > > > + LocalApicLib > > > > > > [Guids] > > > gEfiHobMemoryAllocStackGuid > > > @@ -53,6 +54,7 @@ > > > > > > [Protocols] > > > gEfiMpServiceProtocolGuid > > > + gEfiTimerArchProtocolGuid > > > > > > [Depex] > > > gEfiMpServiceProtocolGuid > > > diff --git > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerUnitTest.c > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerUnitTest.c > > > index 917fc549bf..045f39fa00 100644 > > > --- > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerUnitTest.c > > > +++ > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > andlerUnitTest.c > > > @@ -8,6 +8,8 @@ > > > > > > #include "CpuExceptionHandlerTest.h" > > > #include <Library/UefiBootServicesTableLib.h> > > > +#include <Library/LocalApicLib.h> > > > +#include <Protocol/Timer.h> > > > > > > /** > > > Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR > > > buffer. > > > @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( > > > { > > > EFI_STATUS Status; > > > UNIT_TEST_FRAMEWORK_HANDLE Framework; > > > + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; > > > + UINT64 TimerPeriod; > > > > > > - Framework = NULL; > > > + Framework = NULL; > > > + TimerArchProtocol = NULL; > > > + TimerPeriod = 0; > > > > > > DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, > > > UNIT_TEST_APP_VERSION)); > > > > > > @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( > > > goto EXIT; > > > } > > > > > > + // > > > + // If HpetTimer driver has been dispatched, disable HpetTimer before > Unit > > > Test. > > > + // > > > + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID > > > **)&TimerArchProtocol); > > > + if (TimerArchProtocol != NULL) { > > > + Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol, > > > &TimerPeriod); > > > + ASSERT_EFI_ERROR (Status); > > > + if (TimerPeriod > 0) { > > > + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable > > > HpetTimer.\n")); > > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, 0); > > > + ASSERT_EFI_ERROR (Status); > > > + } > > > + } > > > + > > > // > > > // Execute the tests. > > > // > > > Status = RunAllTestSuites (Framework); > > > > > > + // > > > + // Restore HpetTimer after Unit Test. > > > + // Send APIC EOI before SetTimerPeriod. > > > + // > > > + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { > > > + DEBUG ((DEBUG_INFO, "Restore HpetTimer after > > > DxeCpuExceptionHandlerLib UnitTest.\n")); > > > + SendApicEoi (); > > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, > > > TimerPeriod); > > > + ASSERT_EFI_ERROR (Status); > > > + } > > > + > > > EXIT: > > > if (Framework) { > > > FreeUnitTestFramework (Framework); > > > -- > > > 2.31.1.windows.1 > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1721CC636147BB21.550@groups.io>]
* Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test [not found] ` <1721CC636147BB21.550@groups.io> @ 2022-10-27 3:03 ` Ni, Ray 2022-10-27 3:08 ` duntan 0 siblings, 1 reply; 6+ messages in thread From: Ni, Ray @ 2022-10-27 3:03 UTC (permalink / raw) To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D, Tan, Dun Cc: Dong, Eric, Kumar, Rahul R TimerDriverSetTimerPeriod() { Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); HpetEnable (FALSE); Mike, In above code, it's possible that timer interrupt happens after TPL is HIGH but before HPET hardware is stopped. When that happens, EOI sending logic in the timer interrupt handler function doesn't run. So, there might be two options to fix the issue: 1. Send EOI again Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); // CLI HpetEnable (FALSE); // timer interrupt might happen before HPET is really disabled. SendApicEoi (); 2. Disable HPET with interrupt enabled. HpetEnable (FALSE); Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); // CLI Dun is testing the 2nd option. Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Thursday, October 27, 2022 10:33 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; > Tan, Dun <dun.tan@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com> > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > CpuExceptionHandlerLib test > > Mike, > I agree there might be a bug in HPET timer driver where the EOI should be > issued within that driver. > Original idea was to avoid changing a driver used heavily in production > firmware > due to a new unit test failure and that test scenario is not common in > production > firmware. > > We would like to do more analysis on the impact if sending EOI when > disabling timer > and then move the EOI sending from the test code to HPET timer. > > Thanks, > Ray > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Thursday, October 27, 2022 10:26 AM > > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun > > <dun.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> > > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > > <rahul.r.kumar@intel.com> > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > > CpuExceptionHandlerLib test > > > > Hi Ray, > > > > I do not think it makes sense for the CpuExceptionHandlerLib unit > > test to directly access the IO APIC. It assumes that an IO APIC > > based timer interrupt is in use. > > > > Is it possible that this is a bug/race condition in the HpetDxe driver. > > > > Calling the Timer AP to set timer period from 0 -> Non Zero enables > > the timer. There could be a pending timer interrupt before this > > transition that would need to be cleared. > > > > Calling the Timer AP to set the timer period from Non Zero -> 0 > > disables the generation of timer interrupts. We need to make > > sure this disable action does not generate even one more > > pending timer interrupt after the timer is disabled. And > > we need to make sure a timer interrupt does not enter the > > pending state in the logic that is disabling the timer. > > We may need to always do an extra clear after the timer > > interrupt is disabled in case a timer interrupt is asserted > > during the disable logic. > > > > I suspect this unit test use case is not commonly exercised. > > Normal boots will go from 0 -> Non Zero period and may then > > optionally change the timer period. But going from > > Non Zero -> 0 timer period is not likely used. > > > > Seems like a good reason to add some Timer AP unit tests too. > > > > Thanks, > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, > > Ray > > > Sent: Wednesday, October 26, 2022 7:08 PM > > > To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io > > > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > > <rahul.r.kumar@intel.com> > > > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > > CpuExceptionHandlerLib test > > > > > > Reviewed-by: Ray Ni <ray.ni@intel.com> > > > > > > > -----Original Message----- > > > > From: Tan, Dun <dun.tan@intel.com> > > > > Sent: Thursday, October 20, 2022 11:01 AM > > > > To: devel@edk2.groups.io > > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > Kumar, > > > > Rahul R <rahul.r.kumar@intel.com> > > > > Subject: [PATCH] UefiCpuPkg: Restore HpetTimer after > > > > CpuExceptionHandlerLib test > > > > > > > > Disable/Restore HpetTimer before and after running the Dxe > > > > CpuExceptionHandlerLib unit test module. During the UnitTest, a > > > > new Idt is initialized for the test. There is no handler for timer > > > > intrrupt in this new idt. After the test module, HpetTimer does > > > > not work any more since the comparator value register and main > > > > counter value register for timer does not match. To fix this issue, > > > > disable/restore HpetTimer before and after Unit Test if HpetTimer > > > > driver has been dispatched. Besides, send Apic EOI before restore > > > > HpetTimer. > > > > > > > > Signed-off-by: Dun Tan <dun.tan@intel.com> > > > > Cc: Eric Dong <eric.dong@intel.com> > > > > Cc: Ray Ni <ray.ni@intel.com> > > > > Cc: Rahul Kumar <rahul1.kumar@intel.com> > > > > --- > > > > > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > > dlerLibUnitTest.inf | 2 ++ > > > > > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > > dlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- > > > > 2 files changed, 34 insertions(+), 1 deletion(-) > > > > > > > > diff --git > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > index e3dbe7b9ab..24f905936c 100644 > > > > --- > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > +++ > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > @@ -43,6 +43,7 @@ > > > > HobLib > > > > UefiBootServicesTableLib > > > > CpuPageTableLib > > > > + LocalApicLib > > > > > > > > [Guids] > > > > gEfiHobMemoryAllocStackGuid > > > > @@ -53,6 +54,7 @@ > > > > > > > > [Protocols] > > > > gEfiMpServiceProtocolGuid > > > > + gEfiTimerArchProtocolGuid > > > > > > > > [Depex] > > > > gEfiMpServiceProtocolGuid > > > > diff --git > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > index 917fc549bf..045f39fa00 100644 > > > > --- > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > +++ > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > @@ -8,6 +8,8 @@ > > > > > > > > #include "CpuExceptionHandlerTest.h" > > > > #include <Library/UefiBootServicesTableLib.h> > > > > +#include <Library/LocalApicLib.h> > > > > +#include <Protocol/Timer.h> > > > > > > > > /** > > > > Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR > > > > buffer. > > > > @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( > > > > { > > > > EFI_STATUS Status; > > > > UNIT_TEST_FRAMEWORK_HANDLE Framework; > > > > + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; > > > > + UINT64 TimerPeriod; > > > > > > > > - Framework = NULL; > > > > + Framework = NULL; > > > > + TimerArchProtocol = NULL; > > > > + TimerPeriod = 0; > > > > > > > > DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, > > > > UNIT_TEST_APP_VERSION)); > > > > > > > > @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( > > > > goto EXIT; > > > > } > > > > > > > > + // > > > > + // If HpetTimer driver has been dispatched, disable HpetTimer before > > Unit > > > > Test. > > > > + // > > > > + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID > > > > **)&TimerArchProtocol); > > > > + if (TimerArchProtocol != NULL) { > > > > + Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol, > > > > &TimerPeriod); > > > > + ASSERT_EFI_ERROR (Status); > > > > + if (TimerPeriod > 0) { > > > > + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable > > > > HpetTimer.\n")); > > > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, > 0); > > > > + ASSERT_EFI_ERROR (Status); > > > > + } > > > > + } > > > > + > > > > // > > > > // Execute the tests. > > > > // > > > > Status = RunAllTestSuites (Framework); > > > > > > > > + // > > > > + // Restore HpetTimer after Unit Test. > > > > + // Send APIC EOI before SetTimerPeriod. > > > > + // > > > > + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { > > > > + DEBUG ((DEBUG_INFO, "Restore HpetTimer after > > > > DxeCpuExceptionHandlerLib UnitTest.\n")); > > > > + SendApicEoi (); > > > > + Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, > > > > TimerPeriod); > > > > + ASSERT_EFI_ERROR (Status); > > > > + } > > > > + > > > > EXIT: > > > > if (Framework) { > > > > FreeUnitTestFramework (Framework); > > > > -- > > > > 2.31.1.windows.1 > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test 2022-10-27 3:03 ` Ni, Ray @ 2022-10-27 3:08 ` duntan 0 siblings, 0 replies; 6+ messages in thread From: duntan @ 2022-10-27 3:08 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io, Kinney, Michael D Cc: Dong, Eric, Kumar, Rahul R Hi Ray, Mike, The 2nd option works in qsp boot. Thanks, Dun -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Thursday, October 27, 2022 11:03 AM To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Tan, Dun <dun.tan@intel.com> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test TimerDriverSetTimerPeriod() { Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); HpetEnable (FALSE); Mike, In above code, it's possible that timer interrupt happens after TPL is HIGH but before HPET hardware is stopped. When that happens, EOI sending logic in the timer interrupt handler function doesn't run. So, there might be two options to fix the issue: 1. Send EOI again Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); // CLI HpetEnable (FALSE); // timer interrupt might happen before HPET is really disabled. SendApicEoi (); 2. Disable HPET with interrupt enabled. HpetEnable (FALSE); Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); // CLI Dun is testing the 2nd option. Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Thursday, October 27, 2022 10:33 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; > devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com> > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after > CpuExceptionHandlerLib test > > Mike, > I agree there might be a bug in HPET timer driver where the EOI should > be issued within that driver. > Original idea was to avoid changing a driver used heavily in > production firmware due to a new unit test failure and that test > scenario is not common in production firmware. > > We would like to do more analysis on the impact if sending EOI when > disabling timer and then move the EOI sending from the test code to > HPET timer. > > Thanks, > Ray > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Thursday, October 27, 2022 10:26 AM > > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun > > <dun.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> > > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > > <rahul.r.kumar@intel.com> > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer > > after CpuExceptionHandlerLib test > > > > Hi Ray, > > > > I do not think it makes sense for the CpuExceptionHandlerLib unit > > test to directly access the IO APIC. It assumes that an IO APIC > > based timer interrupt is in use. > > > > Is it possible that this is a bug/race condition in the HpetDxe driver. > > > > Calling the Timer AP to set timer period from 0 -> Non Zero enables > > the timer. There could be a pending timer interrupt before this > > transition that would need to be cleared. > > > > Calling the Timer AP to set the timer period from Non Zero -> 0 > > disables the generation of timer interrupts. We need to make sure > > this disable action does not generate even one more pending timer > > interrupt after the timer is disabled. And we need to make sure a > > timer interrupt does not enter the pending state in the logic that > > is disabling the timer. > > We may need to always do an extra clear after the timer interrupt is > > disabled in case a timer interrupt is asserted during the disable > > logic. > > > > I suspect this unit test use case is not commonly exercised. > > Normal boots will go from 0 -> Non Zero period and may then > > optionally change the timer period. But going from Non Zero -> 0 > > timer period is not likely used. > > > > Seems like a good reason to add some Timer AP unit tests too. > > > > Thanks, > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, > > Ray > > > Sent: Wednesday, October 26, 2022 7:08 PM > > > To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io > > > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R > > <rahul.r.kumar@intel.com> > > > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer > > > after > > CpuExceptionHandlerLib test > > > > > > Reviewed-by: Ray Ni <ray.ni@intel.com> > > > > > > > -----Original Message----- > > > > From: Tan, Dun <dun.tan@intel.com> > > > > Sent: Thursday, October 20, 2022 11:01 AM > > > > To: devel@edk2.groups.io > > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray > > > > <ray.ni@intel.com>; > Kumar, > > > > Rahul R <rahul.r.kumar@intel.com> > > > > Subject: [PATCH] UefiCpuPkg: Restore HpetTimer after > > > > CpuExceptionHandlerLib test > > > > > > > > Disable/Restore HpetTimer before and after running the Dxe > > > > CpuExceptionHandlerLib unit test module. During the UnitTest, a > > > > new Idt is initialized for the test. There is no handler for > > > > timer intrrupt in this new idt. After the test module, HpetTimer > > > > does not work any more since the comparator value register and > > > > main counter value register for timer does not match. To fix > > > > this issue, disable/restore HpetTimer before and after Unit Test > > > > if HpetTimer driver has been dispatched. Besides, send Apic EOI > > > > before restore HpetTimer. > > > > > > > > Signed-off-by: Dun Tan <dun.tan@intel.com> > > > > Cc: Eric Dong <eric.dong@intel.com> > > > > Cc: Ray Ni <ray.ni@intel.com> > > > > Cc: Rahul Kumar <rahul1.kumar@intel.com> > > > > --- > > > > > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > > dlerLibUnitTest.inf | 2 ++ > > > > > > > > > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan > > > > dlerUnitTest.c | 33 ++++++++++++++++++++++++++++++++- > > > > 2 files changed, 34 insertions(+), 1 deletion(-) > > > > > > > > diff --git > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > index e3dbe7b9ab..24f905936c 100644 > > > > --- > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > +++ > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerLibUnitTest.inf > > > > @@ -43,6 +43,7 @@ > > > > HobLib > > > > UefiBootServicesTableLib > > > > CpuPageTableLib > > > > + LocalApicLib > > > > > > > > [Guids] > > > > gEfiHobMemoryAllocStackGuid > > > > @@ -53,6 +54,7 @@ > > > > > > > > [Protocols] > > > > gEfiMpServiceProtocolGuid > > > > + gEfiTimerArchProtocolGuid > > > > > > > > [Depex] > > > > gEfiMpServiceProtocolGuid > > > > diff --git > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > index 917fc549bf..045f39fa00 100644 > > > > --- > > > > > > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > +++ > > > > > > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH > > > > andlerUnitTest.c > > > > @@ -8,6 +8,8 @@ > > > > > > > > #include "CpuExceptionHandlerTest.h" > > > > #include <Library/UefiBootServicesTableLib.h> > > > > +#include <Library/LocalApicLib.h> #include <Protocol/Timer.h> > > > > > > > > /** > > > > Initialize Bsp Idt with a new Idt table and return the > > > > IA32_DESCRIPTOR buffer. > > > > @@ -162,8 +164,12 @@ CpuExceptionHandlerTestEntry ( { > > > > EFI_STATUS Status; > > > > UNIT_TEST_FRAMEWORK_HANDLE Framework; > > > > + EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol; > > > > + UINT64 TimerPeriod; > > > > > > > > - Framework = NULL; > > > > + Framework = NULL; > > > > + TimerArchProtocol = NULL; > > > > + TimerPeriod = 0; > > > > > > > > DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, > > > > UNIT_TEST_APP_VERSION)); > > > > > > > > @@ -182,11 +188,36 @@ CpuExceptionHandlerTestEntry ( > > > > goto EXIT; > > > > } > > > > > > > > + // > > > > + // If HpetTimer driver has been dispatched, disable HpetTimer > > > > + before > > Unit > > > > Test. > > > > + // > > > > + gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID > > > > **)&TimerArchProtocol); > > > > + if (TimerArchProtocol != NULL) { > > > > + Status = TimerArchProtocol->GetTimerPeriod > > > > + (TimerArchProtocol, > > > > &TimerPeriod); > > > > + ASSERT_EFI_ERROR (Status); > > > > + if (TimerPeriod > 0) { > > > > + DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. > > > > + Disable > > > > HpetTimer.\n")); > > > > + Status = TimerArchProtocol->SetTimerPeriod > > > > + (TimerArchProtocol, > 0); > > > > + ASSERT_EFI_ERROR (Status); > > > > + } > > > > + } > > > > + > > > > // > > > > // Execute the tests. > > > > // > > > > Status = RunAllTestSuites (Framework); > > > > > > > > + // > > > > + // Restore HpetTimer after Unit Test. > > > > + // Send APIC EOI before SetTimerPeriod. > > > > + // > > > > + if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) { > > > > + DEBUG ((DEBUG_INFO, "Restore HpetTimer after > > > > DxeCpuExceptionHandlerLib UnitTest.\n")); > > > > + SendApicEoi (); > > > > + Status = TimerArchProtocol->SetTimerPeriod > > > > + (TimerArchProtocol, > > > > TimerPeriod); > > > > + ASSERT_EFI_ERROR (Status); > > > > + } > > > > + > > > > EXIT: > > > > if (Framework) { > > > > FreeUnitTestFramework (Framework); > > > > -- > > > > 2.31.1.windows.1 > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-27 3:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-20 3:00 [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test duntan
2022-10-27 2:08 ` Ni, Ray
2022-10-27 2:26 ` [edk2-devel] " Michael D Kinney
2022-10-27 2:32 ` Ni, Ray
[not found] ` <1721CC636147BB21.550@groups.io>
2022-10-27 3:03 ` Ni, Ray
2022-10-27 3:08 ` duntan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox