tools/include/xen-foreign/reference.size | 5 ++--- xen/common/kernel.c | 3 ++- xen/common/time.c | 5 +++++ xen/include/public/arch-arm.h | 2 ++ xen/include/public/arch-ppc.h | 3 +++ xen/include/public/arch-riscv.h | 3 ++- xen/include/public/arch-x86/xen.h | 3 +++ xen/include/public/features.h | 5 +++++ 8 files changed, 24 insertions(+), 5 deletions(-)
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.
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.
Provide a new feature bit XENFEAT_shared_info_time_offset for this
functionality.
Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
---
v2: Remove unnecessary casts.
---
tools/include/xen-foreign/reference.size | 5 ++---
xen/common/kernel.c | 3 ++-
xen/common/time.c | 5 +++++
xen/include/public/arch-arm.h | 2 ++
xen/include/public/arch-ppc.h | 3 +++
xen/include/public/arch-riscv.h | 3 ++-
xen/include/public/arch-x86/xen.h | 3 +++
xen/include/public/features.h | 5 +++++
8 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/tools/include/xen-foreign/reference.size b/tools/include/xen-foreign/reference.size
index 11a06a7a43..38e799617a 100644
--- a/tools/include/xen-foreign/reference.size
+++ b/tools/include/xen-foreign/reference.size
@@ -9,6 +9,5 @@ vcpu_guest_context | 352 352 2800 5168
arch_vcpu_info | 0 0 24 16
vcpu_time_info | 32 32 32 32
vcpu_info | 48 48 64 64
-arch_shared_info | 0 0 28 48
-shared_info | 1088 1088 2344 3136
-
+arch_shared_info | 8 8 36 56
+shared_info | 1096 1096 2352 3144
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index fb45f81399..44af6f8f04 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -676,7 +676,8 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
#ifdef CONFIG_X86
(1U << XENFEAT_vcpu_time_phys_area) |
#endif
- (1U << XENFEAT_runstate_phys_area);
+ (1U << XENFEAT_runstate_phys_area) |
+ (1U << XENFEAT_shared_info_time_offset);
if ( VM_ASSIST(d, pae_extended_cr3) )
fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
if ( paging_mode_translate(d) )
diff --git a/xen/common/time.c b/xen/common/time.c
index c873b5731b..6f1a8a11c2 100644
--- 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);
+
smp_wmb();
*wc_version = version_update_end(*wc_version);
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index cd563cf706..4360f9835b 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -363,6 +363,8 @@ struct arch_vcpu_info {
typedef struct arch_vcpu_info arch_vcpu_info_t;
struct arch_shared_info {
+ uint32_t time_offset;
+ uint32_t time_offset_hi;
};
typedef struct arch_shared_info arch_shared_info_t;
typedef uint64_t xen_callback_t;
diff --git a/xen/include/public/arch-ppc.h b/xen/include/public/arch-ppc.h
index b5e1a940a5..42ade11171 100644
--- a/xen/include/public/arch-ppc.h
+++ b/xen/include/public/arch-ppc.h
@@ -92,6 +92,9 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
struct arch_shared_info {
uint64_t boot_timebase;
+
+ uint32_t time_offset;
+ uint32_t time_offset_hi;
};
struct arch_vcpu_info {
diff --git a/xen/include/public/arch-riscv.h b/xen/include/public/arch-riscv.h
index 360d8e6871..286bf92473 100644
--- a/xen/include/public/arch-riscv.h
+++ b/xen/include/public/arch-riscv.h
@@ -65,8 +65,9 @@ struct arch_vcpu_info {
};
typedef struct arch_vcpu_info arch_vcpu_info_t;
-/* TODO: add a placeholder entry if no real ones surface */
struct arch_shared_info {
+ uint32_t time_offset;
+ uint32_t time_offset_hi;
};
typedef struct arch_shared_info arch_shared_info_t;
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index a7bf046ee0..401583383c 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -263,6 +263,9 @@ struct arch_shared_info {
/* There's no room for this field in the generic structure. */
uint32_t wc_sec_hi;
#endif
+
+ uint32_t time_offset;
+ uint32_t time_offset_hi;
};
typedef struct arch_shared_info arch_shared_info_t;
diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 8801930947..b48591c17a 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -128,6 +128,11 @@
*/
#define XENFEAT_dm_msix_all_writes 20
+/*
+ * arch_shared_info provides time_offset and time_offset_hi
+ */
+#define XENFEAT_shared_info_time_offset 21
+
#define XENFEAT_NR_SUBMAPS 1
#endif /* __XEN_PUBLIC_FEATURES_H__ */
--
2.43.0
--
Ngoc Tu Dinh | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
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. > 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. > Provide a new feature bit XENFEAT_shared_info_time_offset for this > functionality. > > Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech> > --- > 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? Jan
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 <ngoc-tu.dinh@vates.tech> >> --- >> 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
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. Plus I fear I don't view any of this a basis to suggest
to expose some host property to guests.
>>> 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.
Jan
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
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.
>> 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 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).
Jan
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
On 20.01.2026 17:06, Tu Dinh wrote: > 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. What I still can't extract from this is why Windows running bare-metal is fine but Windows running on Xen's vRTC isn't. If there's a problem with our vRTC, shouldn't that be addressed there? Also, just ftaod: If other maintainers find this convincing, my failure to understand shouldn't get in the way. They may still approve this change, i.e. I'm not vetoing it. It's just that as of now I also wouldn't ack it. Jan
Hello, On 21/01/2026 10:17, Jan Beulich wrote: > On 20.01.2026 17:06, Tu Dinh wrote: >> 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. > > What I still can't extract from this is why Windows running bare-metal is > fine but Windows running on Xen's vRTC isn't. If there's a problem with > our vRTC, shouldn't that be addressed there? > In this case, it's not because the vRTC emulation was wrong, but rather because Windows's internal wallclock is not Xen-aware and needs to be synchronized after some Xen-specific events. So it's more of an accommodation for Windows guests. Also, Windows timekeeping integrates closely with its internal time service, which assumes a NTP-like interface (and thus an external time reference). The current way of time synchronization in the Windows PV drivers doesn't work well in this model, which is why I'm looking for a way to get the external time reference from Xen. > Also, just ftaod: If other maintainers find this convincing, my failure to > understand shouldn't get in the way. They may still approve this change, > i.e. I'm not vetoing it. It's just that as of now I also wouldn't ack it. > > Jan > -- Ngoc Tu Dinh | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
On 21.01.2026 14:02, Tu Dinh wrote: > Hello, > > On 21/01/2026 10:17, Jan Beulich wrote: >> On 20.01.2026 17:06, Tu Dinh wrote: >>> 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. >> >> What I still can't extract from this is why Windows running bare-metal is >> fine but Windows running on Xen's vRTC isn't. If there's a problem with >> our vRTC, shouldn't that be addressed there? >> > > In this case, it's not because the vRTC emulation was wrong, but rather > because Windows's internal wallclock is not Xen-aware And it shouldn't need to be. > and needs to be > synchronized after some Xen-specific events. So it's more of an > accommodation for Windows guests. > > Also, Windows timekeeping integrates closely with its internal time > service, which assumes a NTP-like interface (and thus an external time > reference). The current way of time synchronization in the Windows PV > drivers doesn't work well in this model, which is why I'm looking for a > way to get the external time reference from Xen. Are you suggesting then that plain Windows is fine, but Windows with the PV drivers isn't? That would look to be an issue with the PV drivers then, wouldn't it? Jan
On 21/01/2026 14:06, Jan Beulich wrote: > On 21.01.2026 14:02, Tu Dinh wrote: >> Hello, >> >> On 21/01/2026 10:17, Jan Beulich wrote: >>> On 20.01.2026 17:06, Tu Dinh wrote: >>>> 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. >>> >>> What I still can't extract from this is why Windows running bare-metal is >>> fine but Windows running on Xen's vRTC isn't. If there's a problem with >>> our vRTC, shouldn't that be addressed there? >>> >> >> In this case, it's not because the vRTC emulation was wrong, but rather >> because Windows's internal wallclock is not Xen-aware > > And it shouldn't need to be. > >> and needs to be >> synchronized after some Xen-specific events. So it's more of an >> accommodation for Windows guests. >> >> Also, Windows timekeeping integrates closely with its internal time >> service, which assumes a NTP-like interface (and thus an external time >> reference). The current way of time synchronization in the Windows PV >> drivers doesn't work well in this model, which is why I'm looking for a >> way to get the external time reference from Xen. > > Are you suggesting then that plain Windows is fine, but Windows with the > PV drivers isn't? That would look to be an issue with the PV drivers then, > wouldn't it? > No, it just means that Windows running on Xen without PV drivers is not fine, and the PV drivers currently need this feature in order to correctly sync the guest time. This new functionality will be used in the Windows PV drivers if it were to be merged. > Jan -- Ngoc Tu Dinh | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
© 2016 - 2026 Red Hat, Inc.