On 20/01/2026 16:39, Jan Beulich wrote:
> On 20.01.2026 16:27, Tu Dinh wrote:
>> 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?
>
> Which new description? So far I only saw your responses to my questions, not
> an updated patch description.
>
Maybe my last email wasn't clear, it was in the part marked "Follow up",
reproduced below:
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.
>>> 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.
>
> What the guest can do to its (virtual) RTC is no different from what an OS
> running bare metal can do to the real RTC. Running on bare metal, you also
> don't have any other reference (without using e.g. NTP).
>
Since shared_info is a paravirtualized interface that's not meant to
replicate real hardware, I don't think it needs to be bound to the
functionalities of bare-metal RTC; for once, the host already provides
guests with high-precision wallclock more than what the emulated RTC
offer (via vcpu_time_info_t). Adding precision sync is also valuable for
VMs using this interface.
>>>>>> 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].
>
> Oh, right, I should have recalled this. Iirc there was an unwritten assumption
> that for Windows the public headers may need some massaging.
>
>> 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.
>
> Except that int64_aligned_t can be used in __XEN_TOOLS__ guarded areas only,
> for not being possible to make available with plain C89 / C99. So here we're
> working out a reason why the field may indeed better be split. Albeit as
> said, other areas of the public headers use {,u}int64_t as well, so maybe
> this still would only be a pretty weak reason (and you could make sure the
> field is properly padded for the x86-32 case).
>
I see, I didn't realize that int64_aligned_t is for __XEN_TOOLS__ only.
> and you could make sure the field is properly padded for the x86-32 case
This would not be possible either, as using int64_t would increase the
alignment of arch_shared_info to 8 on MSVC. Since wc_sec_hi is not
defined on x86-32, shared_info->arch is on a 4-byte offset (modulo 8),
and so its layout would be broken on this compiler if int64_t were to be
used.
> Jan
--
Ngoc Tu Dinh | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
