Re: [PATCH] xen: Expose time_offset in struct arch_shared_info

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 13/11/2025 13:09, Jan Beulich wrote:
> On 12.11.2025 08:08, 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.
> 
> I guess I don't understand what the problem is, or why it would need a
> change in the hypervisor (interface). If the guest updates the vRTC, it is
> (implicitly) asking for a change of time offset, isn't it? And whatever
> adjustments it makes, it could track and accumulate them?

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.

> 
>> 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.
> 
> Any why in two halves? Without that, ...
> 
>> --- 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)(uint64_t)d->time_offset.seconds;
>> +    shared_info(d, arch.time_offset_hi) =
>> +        (uint32_t)((uint64_t)d->time_offset.seconds >> 32);
> 
> ... no casting at all would be needed here. (Even when there are two halves,
> some of the casting can be dropped.)
> 
> Another question is - why unsigned? struct domain's time_offset.seconds is
> signed for a reason, aiui.

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.

> 
> Jan



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech