On Tue, Mar 13, 2018 at 09:33:33AM +0000, Marc Zyngier wrote: > On 13/03/18 00:31, Heyi Guo 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....@linaro.org> > > Signed-off-by: Yi Li <phoenix.l...@huawei.com> > > Cc: Leif Lindholm <leif.lindh...@linaro.org> > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > > Cc: Marc Zyngier <marc.zyng...@arm.com> > > --- > > > > Notes: > > 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..32abee8726a0 100644 > > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c > > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c > > @@ -337,6 +337,7 @@ TimerInterruptHandler ( > > > > // Set next compare value > > ArmGenericTimerSetCompareVal (CompareValue); > > + ArmInstructionSynchronizationBarrier (); > > ArmGenericTimerEnableTimer (); > > } > > Sorry for being pedantic here, but it would make more sense if ISB was > placed after the enabling of the timer. Otherwise, you only force the > update of the comparator. But on the other hand, the timer was never > disabled the first place, in which case you'd wonder why you're trying > to enable it again. Yes, I also had such question and hesitated at this place :) > > So either you leave the ISB here and remove the enable call, or move the > ISB after the enable.
If we are going to remove the enable call, is it better to be changed in a separate patch? It seems not related with adding ISB, though it is only a one-line change. Thanks and regards, Heyi > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... _______________________________________________ edk2-devel mailing list firstname.lastname@example.org https://lists.01.org/mailman/listinfo/edk2-devel