1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test

duntan posted 1 patch 1 year, 6 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf |  2 ++
UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerUnitTest.c      | 33 ++++++++++++++++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
[edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test
Posted by duntan 1 year, 6 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95414): https://edk2.groups.io/g/devel/message/95414
Mute This Topic: https://groups.io/mt/94446624/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test
Posted by Ni, Ray 1 year, 6 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95625): https://edk2.groups.io/g/devel/message/95625
Mute This Topic: https://groups.io/mt/94446624/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test
Posted by Michael D Kinney 1 year, 6 months ago 
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
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95626): https://edk2.groups.io/g/devel/message/95626
Mute This Topic: https://groups.io/mt/94446624/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Restore HpetTimer after CpuExceptionHandlerLib test
Posted by Ni, Ray 1 year, 6 months ago 
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
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95627): https://edk2.groups.io/g/devel/message/95627
Mute This Topic: https://groups.io/mt/94446624/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.