Re: [PATCH v3 1/1] ArmPkg/TimerDxe: Add ISB for timer compare value reload

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 15 March 2018 at 07:17, Heyi Guo <heyi.guo@linaro.org> wrote:
> If timer interrupt is level sensitive, reloading timer compare
> register has a side effect of clearing GIC pending status, so a "ISB"
> is needed to make sure this instruction is executed before enabling
> CPU IRQ, or else we may get spurious timer interrupts.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>


Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as ac9b530e6b47

Thanks

> ---
>
> Notes:
>     v3:
>     - Move ISB after enabling timer [Marc]
>
>     v2:
>     - Use ISB instead of DSB [Marc]
>     - Update commit message accordingly.
>
>  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> index 33d7c922221f..a3202fa056f3 100644
> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> @@ -338,6 +338,7 @@ TimerInterruptHandler (
>      // Set next compare value
>      ArmGenericTimerSetCompareVal (CompareValue);
>      ArmGenericTimerEnableTimer ();
> +    ArmInstructionSynchronizationBarrier ();
>    }
>
>    gBS->RestoreTPL (OriginalTPL);
> --
> 2.7.4
>