On 20/01/2026 11:35, Jan Beulich wrote: > On 20.01.2026 10:57, Tu Dinh wrote: >> time_offset is currently always added to wc_sec. This means that without >> the actual value of time_offset, guests have no way of knowing what's >> the actual host clock. Once the guest clock drifts beyond 1 second, >> updates to the guest RTC would themselves change time_offset and make it >> impossible to resync guest time to host time. > > Despite my earlier comments this part of the description looks unchanged. > I still don't see why host time (or in fact about any host property) should > be exposed to guests. >
I've answered this question in a followup reply from November, which I'll reproduce here: vRTC drift can happen for other reasons. For example, Windows can write to the RTC at any time; if a guest clock drift has already happened (e.g. after a migration), an unfortunately-timed RTC write will make it permanent. Windows time providers don't have the ability to control when Windows writes to RTC either. Thus the "real" host clock time is needed to help the VM adjust to the correct time. IOW, it's the distinction between "keeping track of already correct time" versus "correcting wrong time by adjusting the offset"; the latter is what I'm looking for. (Follow up: Here's my attempt to rewrite the description to account for the above) Xen currently does not expose the host's wall clock time in shared_info. This means while shared_info can be used as an alternative to the emulated RTC, it can't be used to keep the virtual wall clock in sync. Expose the time_offset value in struct shared_info in order to allow guests to synchronize their own wall clock to that of the host. This is needed because on Windows guests, the PV drivers don't control the timing of RTC updates, as this is done by the kernel itself periodically. If the guest's internal clock deviates from the RTC (e.g. after resuming from suspend), a RTC write would cause time_offset to deviate from the supposed value (timezone offset) and thus cause the RTC to become incorrect. Provide a new feature bit XENFEAT_shared_info_time_offset for this functionality. >> Since there's no way to add more fields to struct shared_info, the >> addition has to be done through struct arch_shared_info instead. Add two >> fields in arch_shared_info representing time_offset's low and high >> 32-bit halves. > > Again, despite my earlier question, reasoning of why two halves rather than > a (signed) 64-bit value isn't supplied here. > This was also in my last email: Both are just for easy consumption of the time offset on 32-bit guests. Unsigned is particularly because these are only parts of an int64_t (and therefore have no signedness themselves) and I prefer to let the conversion happen after reading the two fields. (Follow up: Also, the alignment of int64_t differs between GCC and MSVC compilers. Using int64_t here would change the alignment of struct arch_shared_info) >> Provide a new feature bit XENFEAT_shared_info_time_offset for this >> functionality. >> >> Signed-off-by: Tu Dinh <[email protected]> >> --- >> v2: Remove unnecessary casts. > > Did you really? What about ... > >> --- a/xen/common/time.c >> +++ b/xen/common/time.c >> @@ -118,6 +118,11 @@ void update_domain_wallclock_time(struct domain *d) >> shared_info(d, wc_sec_hi) = sec >> 32; >> #endif >> >> + shared_info(d, arch.time_offset) = >> + (uint32_t)d->time_offset.seconds; >> + shared_info(d, arch.time_offset_hi) = >> + (uint32_t)(d->time_offset.seconds >> 32); > > ... both of these? > I thought these downcasts should be explicit. I can remove these as well. NB: A blank line in reference.size was lost when preparing the patch, I'll fix it in the resend > Jan -- Ngoc Tu Dinh | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
