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>
---
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 e6979352e1..dbd2cf9c76 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..0f38b1342d 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)(uint64_t)d->time_offset.seconds;
+ shared_info(d, arch.time_offset_hi) =
+ (uint32_t)((uint64_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 b31324f8d4..2f34fb409b 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 4ca453a284..5bb47e09d3 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 168263b920..e502aade99 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 b99a691706..598af31e4e 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.techOn 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? > 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. Jan
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
© 2016 - 2025 Red Hat, Inc.