On 15/12/17 10:10, Christoffer Dall wrote: > On Fri, Dec 15, 2017 at 09:09:05AM +0000, Marc Zyngier wrote: >> On 15/12/17 02:27, Jia He wrote: >>> >>> >> >> [...] >> >>>> @@ -367,6 +368,7 @@ static void vtimer_save_state(struct kvm_vcpu *vcpu) >>>> >>>> /* Disable the virtual timer */ >>>> write_sysreg_el0(0, cntv_ctl); >>>> + isb(); >>> My only concern is whether this isb() is required here? >>> Sorryif this is a stupid question.I understand little about arm arch >>> memory barrier. But seems isb will flush all the instruction prefetch.Do >>> you think if an timer interrupt irq arrives, arm will use the previous >>> instruction prefetch? >> >> >> This barrier has little to do with prefetch. It just guarantees that the >> code after the isb() is now running with a disabled virtual timer. >> Otherwise, a CPU can freely reorder the write_sysreg() until the next >> context synchronization event. >> >> An interrupt coming between the write and the barrier will also act as a >> context synchronization event. For more details, see the ARMv8 ARM (the >> glossary has a section on the concept). >> > > So since an ISB doesn't guarantee that the timer will actually be > disabled, is it a waste of energy to have it, or should we keep it as a > best effort kind of thing? nit: the ISB does offer that guarantee. It is just that the guarantee doesn't extend to an interrupt that has already been signalled. The main issue I have with not having an ISB is that it makes it harder to think of when the disabling actually happens. The disabling could be delayed for a very long time, and would make things harder to debug if they were going wrong. Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
