On 20/01/2026 13:42, Jan Beulich wrote:
> On 20.01.2026 13:12, Tu Dinh wrote:
>> 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:
>
> I did read your reply, yet nothing of it appeared here as additional
> justification.
Is the new description OK for you?
> Plus I fear I don't view any of this a basis to suggest
> to expose some host property to guests.
>
The only host property being exposed would be the UTC wallclock as kept
track by the host (as is specified by XENPF_settime). This information
(wallclock from an external reference) is necessary for guest timesync,
whereas an RTC which guests can update by themselves simply cannot be
used for this purpose.
Also, IMO Xen is the right place for adding proper timekeeping to the
guest, as it's aware of both the "true" time as tracked by the host's
hardware and the guest's TSC values (used for virtual clock
calculations. Similar functionalities are provided by other hypervisors
(KVM ptpclock, Hyper-V).
>>>> 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.
>
> I don't buy this. I should probably have replied to this effect when
> you first wrote it. {,u}int64_t is hardly a hurdle anymore there. Nor
> would I expect any halfway up-to-date 32-bit guest to manage time as
> a 32-bit quantity anymore.
>
>> 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.
>
> There may be benefits to this, yes, but imo they want to be spelled out,
> rather than left vague.
>
>> (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)
>
> Does it? For which target and in which way? This would, after all, render
> other uses of {,u}int64_t in the public headers problematic as well.
>
For the x86 32-bit target, the Windows ABI uses 8-byte alignment for
(u)int64_t as opposed to 4-byte for the System V ABI [1]. Most of the
other uses of 64-bit integers look to be manually aligned and/or using
(u)int64_aligned_t (I haven't looked at them all). I can switch
time_offset to int64_aligned_t and avoid the issues above.
[1] https://godbolt.org/z/x8o8K51Kv
> Jan
--
Ngoc Tu Dinh | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
