On Tue, Mar 15, 2022 at 11:39:29AM +0100, Jan Beulich wrote: > On 15.03.2022 10:12, Roger Pau Monné wrote: > > On Mon, Mar 14, 2022 at 05:19:37PM +0100, Jan Beulich wrote: > >> One thing seems quite clear though: Doing any of this with interrupts > >> enabled increases the chances for the read pairs to not properly > >> correlate, due to an interrupt happening in the middle. This alone is > >> a reason for me to want to keep IRQs off here. > > > > Right, TSC calibration is also done with interrupts disabled, so it > > does seem correct to do the same here for APIC. > > > > Maybe it would be cleaner to hide the specific PIT logic in > > calibrate_apic_timer() so that we could remove get_8254_timer_count() > > and wait_8254_wraparound() from apic.c and apic.c doesn't have any PIT > > specific code anymore? > > Yes, that's certainly a further cleanup step to take (saying this > without actually having tried, so there may be obstacles).
OK, I think you are planning to post a new version of this to avoid open-coding apic_read() in read_tmcct()? TBH the PIT calibration done in calibrate_APIC_clock seems fairly bogus, as it's possible the counter wraps around more than once between calls when running virtualized. Maybe reprogramming channel 2 would be better, as then at least wrap around would be detected (albeit it's unclear how much delta we would have between the counter reaching 0 and Xen realizing). Thanks, Roger.
