Re: [Patch] PcAtChipsetPkg/HpetTimerDxe: Fix race condition in SetTimerPeriod()

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>-----Original Message-----
>From: Kinney, Michael D
>Sent: Thursday, October 27, 2016 6:31 AM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
>Subject: [Patch] PcAtChipsetPkg/HpetTimerDxe: Fix race condition in SetTimerPeriod()
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=182
>
>The function TimerDriverSetTimerPeriod() disables the HPET timer
>while the HPET timer HW is reprogrammed with a new timer period.
>However, the MMIO write to disable the HPET timer HW can be
>delayed and an HPET timer interrupt may be processed in the middle
>of reprogramming the HPET timer HW and this may produced unexpected
>results.
>
>The fix is to raise TPL to TPL_HIGH_LEVEL in
>TimerDriverSetTimerPeriod() during the time the HPET timer HW is
>reprogrammed.  This guarantees that no timer interrupts are
>processed during reprogramming.
>
>The TimerDriverGenerateSoftInterrupt() function in this same
>driver also raises TPL to TPL_HIGH_LEVEL, so this fix matches
>the logic that is already used in another function for the same
>reason.
>
>Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
>---
> PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
>index 0ed8743..c62c3a9 100644
>--- a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
>+++ b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
>@@ -492,11 +492,17 @@ TimerDriverSetTimerPeriod (
>   IN UINT64                   TimerPeriod
>   )
> {
>+  EFI_TPL                        Tpl;
>   UINT64                         MainCounter;
>   UINT64                         Delta;
>   UINT64                         CurrentComparator;
>   HPET_TIMER_MSI_ROUTE_REGISTER  HpetTimerMsiRoute;
>-
>+
>+  //
>+  // Disable interrupts
>+  //
>+  Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>+
>   //
>   // Disable HPET timer when adjusting the timer period
>   //
>@@ -616,7 +622,12 @@ TimerDriverSetTimerPeriod (
>   // is disabled.
>   //
>   HpetEnable (TRUE);
>-
>+
>+  //
>+  // Restore interrupts
>+  //
>+  gBS->RestoreTPL (Tpl);
>+
>   return EFI_SUCCESS;
> }
>
>--
>2.6.3.windows.1