* [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-13 17:56 ` David Vrabel
` (4 subsequent siblings)
5 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Thomas Gleixner, John Stultz, David Vrabel,
Konrad Rzeszutek Wilk
From: David Vrabel <david.vrabel@citrix.com>
All the virtualized platforms (KVM, lguest and Xen) have persistent
wallclocks that have more than one second of precision.
read_persistent_wallclock() and update_persistent_wallclock() allow
for nanosecond precision but their implementation on x86 with
x86_platform.get/set_wallclock() only allows for one second precision.
This means guests may see a wallclock time that is off by up to 1
second.
Make set_wallclock() and get_wallclock() take a struct timespec
parameter (which allows for nanosecond precision) so KVM and Xen
guests may start with a more accurate wallclock time and a Xen dom0
can maintain a more accurate wallclock for guests.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
arch/x86/include/asm/mc146818rtc.h | 4 ++--
arch/x86/include/asm/x86_init.h | 6 ++++--
arch/x86/kernel/kvmclock.c | 9 +++------
arch/x86/kernel/rtc.c | 17 +++++++----------
arch/x86/lguest/boot.c | 4 ++--
arch/x86/platform/efi/efi.c | 10 ++++++----
arch/x86/xen/time.c | 19 ++++++-------------
include/linux/efi.h | 4 ++--
8 files changed, 32 insertions(+), 41 deletions(-)
diff --git a/arch/x86/include/asm/mc146818rtc.h b/arch/x86/include/asm/mc146818rtc.h
index d354fb7..a55c7ef 100644
--- a/arch/x86/include/asm/mc146818rtc.h
+++ b/arch/x86/include/asm/mc146818rtc.h
@@ -95,8 +95,8 @@ static inline unsigned char current_lock_cmos_reg(void)
unsigned char rtc_cmos_read(unsigned char addr);
void rtc_cmos_write(unsigned char val, unsigned char addr);
-extern int mach_set_rtc_mmss(unsigned long nowtime);
-extern unsigned long mach_get_cmos_time(void);
+extern int mach_set_rtc_mmss(const struct timespec *now);
+extern void mach_get_cmos_time(struct timespec *now);
#define RTC_IRQ 8
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index d8d9922..828a156 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -142,6 +142,8 @@ struct x86_cpuinit_ops {
void (*fixup_cpu_id)(struct cpuinfo_x86 *c, int node);
};
+struct timespec;
+
/**
* struct x86_platform_ops - platform specific runtime functions
* @calibrate_tsc: calibrate TSC
@@ -156,8 +158,8 @@ struct x86_cpuinit_ops {
*/
struct x86_platform_ops {
unsigned long (*calibrate_tsc)(void);
- unsigned long (*get_wallclock)(void);
- int (*set_wallclock)(unsigned long nowtime);
+ void (*get_wallclock)(struct timespec *ts);
+ int (*set_wallclock)(const struct timespec *ts);
void (*iommu_shutdown)(void);
bool (*is_untracked_pat_range)(u64 start, u64 end);
void (*nmi_init)(void);
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index d2c3812..0db81ab 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -48,10 +48,9 @@ static struct pvclock_wall_clock wall_clock;
* have elapsed since the hypervisor wrote the data. So we try to account for
* that with system time
*/
-static unsigned long kvm_get_wallclock(void)
+static void kvm_get_wallclock(struct timespec *now)
{
struct pvclock_vcpu_time_info *vcpu_time;
- struct timespec ts;
int low, high;
int cpu;
@@ -64,14 +63,12 @@ static unsigned long kvm_get_wallclock(void)
cpu = smp_processor_id();
vcpu_time = &hv_clock[cpu].pvti;
- pvclock_read_wallclock(&wall_clock, vcpu_time, &ts);
+ pvclock_read_wallclock(&wall_clock, vcpu_time, now);
preempt_enable();
-
- return ts.tv_sec;
}
-static int kvm_set_wallclock(unsigned long now)
+static int kvm_set_wallclock(const struct timespec *now)
{
return -1;
}
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 198eb20..0aa2939 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -38,8 +38,9 @@ EXPORT_SYMBOL(rtc_lock);
* jump to the next second precisely 500 ms later. Check the Motorola
* MC146818A or Dallas DS12887 data sheet for details.
*/
-int mach_set_rtc_mmss(unsigned long nowtime)
+int mach_set_rtc_mmss(const struct timespec *now)
{
+ unsigned long nowtime = now->tv_sec;
struct rtc_time tm;
int retval = 0;
@@ -58,7 +59,7 @@ int mach_set_rtc_mmss(unsigned long nowtime)
return retval;
}
-unsigned long mach_get_cmos_time(void)
+void mach_get_cmos_time(struct timespec *now)
{
unsigned int status, year, mon, day, hour, min, sec, century = 0;
unsigned long flags;
@@ -107,7 +108,8 @@ unsigned long mach_get_cmos_time(void)
} else
year += CMOS_YEARS_OFFS;
- return mktime(year, mon, day, hour, min, sec);
+ now->tv_sec = mktime(year, mon, day, hour, min, sec);
+ now->tv_nsec = 0;
}
/* Routines for accessing the CMOS RAM/RTC. */
@@ -135,18 +137,13 @@ EXPORT_SYMBOL(rtc_cmos_write);
int update_persistent_clock(struct timespec now)
{
- return x86_platform.set_wallclock(now.tv_sec);
+ return x86_platform.set_wallclock(&now);
}
/* not static: needed by APM */
void read_persistent_clock(struct timespec *ts)
{
- unsigned long retval;
-
- retval = x86_platform.get_wallclock();
-
- ts->tv_sec = retval;
- ts->tv_nsec = 0;
+ x86_platform.get_wallclock(ts);
}
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 7114c63..8424d5a 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -882,9 +882,9 @@ int lguest_setup_irq(unsigned int irq)
* It would be far better for everyone if the Guest had its own clock, but
* until then the Host gives us the time on every interrupt.
*/
-static unsigned long lguest_get_wallclock(void)
+static void lguest_get_wallclock(struct timespec *now)
{
- return lguest_data.time.tv_sec;
+ *now = lguest_data.time;
}
/*
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 55856b2..dd3b825 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -352,8 +352,9 @@ static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
return status;
}
-int efi_set_rtc_mmss(unsigned long nowtime)
+int efi_set_rtc_mmss(const struct timespec *now)
{
+ unsigned long nowtime = now->tv_sec;
efi_status_t status;
efi_time_t eft;
efi_time_cap_t cap;
@@ -388,7 +389,7 @@ int efi_set_rtc_mmss(unsigned long nowtime)
return 0;
}
-unsigned long efi_get_time(void)
+void efi_get_time(struct timespec *now)
{
efi_status_t status;
efi_time_t eft;
@@ -398,8 +399,9 @@ unsigned long efi_get_time(void)
if (status != EFI_SUCCESS)
pr_err("Oops: efitime: can't read time!\n");
- return mktime(eft.year, eft.month, eft.day, eft.hour,
- eft.minute, eft.second);
+ now->tv_sec = mktime(eft.year, eft.month, eft.day, eft.hour,
+ eft.minute, eft.second);
+ now->tv_nsec = 0;
}
/*
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 3d88bfd..a1947ac 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -191,32 +191,25 @@ static void xen_read_wallclock(struct timespec *ts)
put_cpu_var(xen_vcpu);
}
-static unsigned long xen_get_wallclock(void)
+static void xen_get_wallclock(struct timespec *now)
{
- struct timespec ts;
-
- xen_read_wallclock(&ts);
- return ts.tv_sec;
+ xen_read_wallclock(now);
}
-static int xen_set_wallclock(unsigned long now)
+static int xen_set_wallclock(const struct timespec *now)
{
struct xen_platform_op op;
- int rc;
/* do nothing for domU */
if (!xen_initial_domain())
return -1;
op.cmd = XENPF_settime;
- op.u.settime.secs = now;
- op.u.settime.nsecs = 0;
+ op.u.settime.secs = now->tv_sec;
+ op.u.settime.nsecs = now->tv_nsec;
op.u.settime.system_time = xen_clocksource_read();
- rc = HYPERVISOR_dom0_op(&op);
- WARN(rc != 0, "XENPF_settime failed: now=%ld\n", now);
-
- return rc;
+ return HYPERVISOR_dom0_op(&op);
}
static struct clocksource xen_clocksource __read_mostly = {
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 2bc0ad7..0068bba 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -594,8 +594,8 @@ extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size);
extern int __init efi_uart_console_only (void);
extern void efi_initialize_iomem_resources(struct resource *code_resource,
struct resource *data_resource, struct resource *bss_resource);
-extern unsigned long efi_get_time(void);
-extern int efi_set_rtc_mmss(unsigned long nowtime);
+extern void efi_get_time(struct timespec *now);
+extern int efi_set_rtc_mmss(const struct timespec *now);
extern void efi_reserve_boot_services(void);
extern struct efi_memory_map memmap;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
2013-05-13 17:56 ` [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock() David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-14 0:57 ` John Stultz
` (5 more replies)
2013-05-13 17:56 ` [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes David Vrabel
` (3 subsequent siblings)
5 siblings, 6 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, John Stultz,
Thomas Gleixner, linux-kernel
From: David Vrabel <david.vrabel@citrix.com>
All the virtualized platforms (KVM, lguest and Xen) have persistent
wallclocks that have more than one second of precision.
read_persistent_wallclock() and update_persistent_wallclock() allow
for nanosecond precision but their implementation on x86 with
x86_platform.get/set_wallclock() only allows for one second precision.
This means guests may see a wallclock time that is off by up to 1
second.
Make set_wallclock() and get_wallclock() take a struct timespec
parameter (which allows for nanosecond precision) so KVM and Xen
guests may start with a more accurate wallclock time and a Xen dom0
can maintain a more accurate wallclock for guests.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
arch/x86/include/asm/mc146818rtc.h | 4 ++--
arch/x86/include/asm/x86_init.h | 6 ++++--
arch/x86/kernel/kvmclock.c | 9 +++------
arch/x86/kernel/rtc.c | 17 +++++++----------
arch/x86/lguest/boot.c | 4 ++--
arch/x86/platform/efi/efi.c | 10 ++++++----
arch/x86/xen/time.c | 19 ++++++-------------
include/linux/efi.h | 4 ++--
8 files changed, 32 insertions(+), 41 deletions(-)
diff --git a/arch/x86/include/asm/mc146818rtc.h b/arch/x86/include/asm/mc146818rtc.h
index d354fb7..a55c7ef 100644
--- a/arch/x86/include/asm/mc146818rtc.h
+++ b/arch/x86/include/asm/mc146818rtc.h
@@ -95,8 +95,8 @@ static inline unsigned char current_lock_cmos_reg(void)
unsigned char rtc_cmos_read(unsigned char addr);
void rtc_cmos_write(unsigned char val, unsigned char addr);
-extern int mach_set_rtc_mmss(unsigned long nowtime);
-extern unsigned long mach_get_cmos_time(void);
+extern int mach_set_rtc_mmss(const struct timespec *now);
+extern void mach_get_cmos_time(struct timespec *now);
#define RTC_IRQ 8
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index d8d9922..828a156 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -142,6 +142,8 @@ struct x86_cpuinit_ops {
void (*fixup_cpu_id)(struct cpuinfo_x86 *c, int node);
};
+struct timespec;
+
/**
* struct x86_platform_ops - platform specific runtime functions
* @calibrate_tsc: calibrate TSC
@@ -156,8 +158,8 @@ struct x86_cpuinit_ops {
*/
struct x86_platform_ops {
unsigned long (*calibrate_tsc)(void);
- unsigned long (*get_wallclock)(void);
- int (*set_wallclock)(unsigned long nowtime);
+ void (*get_wallclock)(struct timespec *ts);
+ int (*set_wallclock)(const struct timespec *ts);
void (*iommu_shutdown)(void);
bool (*is_untracked_pat_range)(u64 start, u64 end);
void (*nmi_init)(void);
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index d2c3812..0db81ab 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -48,10 +48,9 @@ static struct pvclock_wall_clock wall_clock;
* have elapsed since the hypervisor wrote the data. So we try to account for
* that with system time
*/
-static unsigned long kvm_get_wallclock(void)
+static void kvm_get_wallclock(struct timespec *now)
{
struct pvclock_vcpu_time_info *vcpu_time;
- struct timespec ts;
int low, high;
int cpu;
@@ -64,14 +63,12 @@ static unsigned long kvm_get_wallclock(void)
cpu = smp_processor_id();
vcpu_time = &hv_clock[cpu].pvti;
- pvclock_read_wallclock(&wall_clock, vcpu_time, &ts);
+ pvclock_read_wallclock(&wall_clock, vcpu_time, now);
preempt_enable();
-
- return ts.tv_sec;
}
-static int kvm_set_wallclock(unsigned long now)
+static int kvm_set_wallclock(const struct timespec *now)
{
return -1;
}
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 198eb20..0aa2939 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -38,8 +38,9 @@ EXPORT_SYMBOL(rtc_lock);
* jump to the next second precisely 500 ms later. Check the Motorola
* MC146818A or Dallas DS12887 data sheet for details.
*/
-int mach_set_rtc_mmss(unsigned long nowtime)
+int mach_set_rtc_mmss(const struct timespec *now)
{
+ unsigned long nowtime = now->tv_sec;
struct rtc_time tm;
int retval = 0;
@@ -58,7 +59,7 @@ int mach_set_rtc_mmss(unsigned long nowtime)
return retval;
}
-unsigned long mach_get_cmos_time(void)
+void mach_get_cmos_time(struct timespec *now)
{
unsigned int status, year, mon, day, hour, min, sec, century = 0;
unsigned long flags;
@@ -107,7 +108,8 @@ unsigned long mach_get_cmos_time(void)
} else
year += CMOS_YEARS_OFFS;
- return mktime(year, mon, day, hour, min, sec);
+ now->tv_sec = mktime(year, mon, day, hour, min, sec);
+ now->tv_nsec = 0;
}
/* Routines for accessing the CMOS RAM/RTC. */
@@ -135,18 +137,13 @@ EXPORT_SYMBOL(rtc_cmos_write);
int update_persistent_clock(struct timespec now)
{
- return x86_platform.set_wallclock(now.tv_sec);
+ return x86_platform.set_wallclock(&now);
}
/* not static: needed by APM */
void read_persistent_clock(struct timespec *ts)
{
- unsigned long retval;
-
- retval = x86_platform.get_wallclock();
-
- ts->tv_sec = retval;
- ts->tv_nsec = 0;
+ x86_platform.get_wallclock(ts);
}
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 7114c63..8424d5a 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -882,9 +882,9 @@ int lguest_setup_irq(unsigned int irq)
* It would be far better for everyone if the Guest had its own clock, but
* until then the Host gives us the time on every interrupt.
*/
-static unsigned long lguest_get_wallclock(void)
+static void lguest_get_wallclock(struct timespec *now)
{
- return lguest_data.time.tv_sec;
+ *now = lguest_data.time;
}
/*
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 55856b2..dd3b825 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -352,8 +352,9 @@ static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
return status;
}
-int efi_set_rtc_mmss(unsigned long nowtime)
+int efi_set_rtc_mmss(const struct timespec *now)
{
+ unsigned long nowtime = now->tv_sec;
efi_status_t status;
efi_time_t eft;
efi_time_cap_t cap;
@@ -388,7 +389,7 @@ int efi_set_rtc_mmss(unsigned long nowtime)
return 0;
}
-unsigned long efi_get_time(void)
+void efi_get_time(struct timespec *now)
{
efi_status_t status;
efi_time_t eft;
@@ -398,8 +399,9 @@ unsigned long efi_get_time(void)
if (status != EFI_SUCCESS)
pr_err("Oops: efitime: can't read time!\n");
- return mktime(eft.year, eft.month, eft.day, eft.hour,
- eft.minute, eft.second);
+ now->tv_sec = mktime(eft.year, eft.month, eft.day, eft.hour,
+ eft.minute, eft.second);
+ now->tv_nsec = 0;
}
/*
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 3d88bfd..a1947ac 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -191,32 +191,25 @@ static void xen_read_wallclock(struct timespec *ts)
put_cpu_var(xen_vcpu);
}
-static unsigned long xen_get_wallclock(void)
+static void xen_get_wallclock(struct timespec *now)
{
- struct timespec ts;
-
- xen_read_wallclock(&ts);
- return ts.tv_sec;
+ xen_read_wallclock(now);
}
-static int xen_set_wallclock(unsigned long now)
+static int xen_set_wallclock(const struct timespec *now)
{
struct xen_platform_op op;
- int rc;
/* do nothing for domU */
if (!xen_initial_domain())
return -1;
op.cmd = XENPF_settime;
- op.u.settime.secs = now;
- op.u.settime.nsecs = 0;
+ op.u.settime.secs = now->tv_sec;
+ op.u.settime.nsecs = now->tv_nsec;
op.u.settime.system_time = xen_clocksource_read();
- rc = HYPERVISOR_dom0_op(&op);
- WARN(rc != 0, "XENPF_settime failed: now=%ld\n", now);
-
- return rc;
+ return HYPERVISOR_dom0_op(&op);
}
static struct clocksource xen_clocksource __read_mostly = {
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 2bc0ad7..0068bba 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -594,8 +594,8 @@ extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size);
extern int __init efi_uart_console_only (void);
extern void efi_initialize_iomem_resources(struct resource *code_resource,
struct resource *data_resource, struct resource *bss_resource);
-extern unsigned long efi_get_time(void);
-extern int efi_set_rtc_mmss(unsigned long nowtime);
+extern void efi_get_time(struct timespec *now);
+extern int efi_set_rtc_mmss(const struct timespec *now);
extern void efi_reserve_boot_services(void);
extern struct efi_memory_map memmap;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
@ 2013-05-14 0:57 ` John Stultz
2013-05-14 0:57 ` John Stultz
` (4 subsequent siblings)
5 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:57 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
This one looks pretty good.
Lets circle around on the other two and if after that they still don't
make sense, I'll just queue this one.
Sorry if I'm being thick headed.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
2013-05-14 0:57 ` John Stultz
@ 2013-05-14 0:57 ` John Stultz
2013-05-14 17:52 ` John Stultz
` (3 subsequent siblings)
5 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:57 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
This one looks pretty good.
Lets circle around on the other two and if after that they still don't
make sense, I'll just queue this one.
Sorry if I'm being thick headed.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
2013-05-14 0:57 ` John Stultz
2013-05-14 0:57 ` John Stultz
@ 2013-05-14 17:52 ` John Stultz
2013-05-14 17:52 ` John Stultz
` (2 subsequent siblings)
5 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:52 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
I've gone ahead and queued this one for 3.11
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
` (2 preceding siblings ...)
2013-05-14 17:52 ` John Stultz
@ 2013-05-14 17:52 ` John Stultz
2013-05-29 0:18 ` John Stultz
2013-05-29 0:18 ` John Stultz
5 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:52 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
I've gone ahead and queued this one for 3.11
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
` (3 preceding siblings ...)
2013-05-14 17:52 ` John Stultz
@ 2013-05-29 0:18 ` John Stultz
2013-05-29 0:18 ` John Stultz
5 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-29 0:18 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/include/asm/mc146818rtc.h | 4 ++--
> arch/x86/include/asm/x86_init.h | 6 ++++--
> arch/x86/kernel/kvmclock.c | 9 +++------
> arch/x86/kernel/rtc.c | 17 +++++++----------
> arch/x86/lguest/boot.c | 4 ++--
> arch/x86/platform/efi/efi.c | 10 ++++++----
> arch/x86/xen/time.c | 19 ++++++-------------
> include/linux/efi.h | 4 ++--
Just FYI, David: You missed the vrtc code with this conversion.
I finally got around to pushing my current queue to my public tree and
the kbuild robot noticed the build failure (required INTEL_MID config to
trigger).
The fix I've queued is here:
https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=52d8e9cc6718ae9e6c44b7723028e4717ba91c8b;hp=1dd5234774b33a17743ae62e8b46b5cd0059e1c7
Let me know if you have any objection or comments on this.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-13 17:56 ` David Vrabel
` (4 preceding siblings ...)
2013-05-29 0:18 ` John Stultz
@ 2013-05-29 0:18 ` John Stultz
2013-05-29 12:16 ` David Vrabel
2013-05-29 12:16 ` David Vrabel
5 siblings, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-29 0:18 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> All the virtualized platforms (KVM, lguest and Xen) have persistent
> wallclocks that have more than one second of precision.
>
> read_persistent_wallclock() and update_persistent_wallclock() allow
> for nanosecond precision but their implementation on x86 with
> x86_platform.get/set_wallclock() only allows for one second precision.
> This means guests may see a wallclock time that is off by up to 1
> second.
>
> Make set_wallclock() and get_wallclock() take a struct timespec
> parameter (which allows for nanosecond precision) so KVM and Xen
> guests may start with a more accurate wallclock time and a Xen dom0
> can maintain a more accurate wallclock for guests.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/include/asm/mc146818rtc.h | 4 ++--
> arch/x86/include/asm/x86_init.h | 6 ++++--
> arch/x86/kernel/kvmclock.c | 9 +++------
> arch/x86/kernel/rtc.c | 17 +++++++----------
> arch/x86/lguest/boot.c | 4 ++--
> arch/x86/platform/efi/efi.c | 10 ++++++----
> arch/x86/xen/time.c | 19 ++++++-------------
> include/linux/efi.h | 4 ++--
Just FYI, David: You missed the vrtc code with this conversion.
I finally got around to pushing my current queue to my public tree and
the kbuild robot noticed the build failure (required INTEL_MID config to
trigger).
The fix I've queued is here:
https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=52d8e9cc6718ae9e6c44b7723028e4717ba91c8b;hp=1dd5234774b33a17743ae62e8b46b5cd0059e1c7
Let me know if you have any objection or comments on this.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-29 0:18 ` John Stultz
@ 2013-05-29 12:16 ` David Vrabel
2013-05-29 12:16 ` David Vrabel
1 sibling, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-29 12:16 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 29/05/13 01:18, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> All the virtualized platforms (KVM, lguest and Xen) have persistent
>> wallclocks that have more than one second of precision.
>>
>> read_persistent_wallclock() and update_persistent_wallclock() allow
>> for nanosecond precision but their implementation on x86 with
>> x86_platform.get/set_wallclock() only allows for one second precision.
>> This means guests may see a wallclock time that is off by up to 1
>> second.
>>
>> Make set_wallclock() and get_wallclock() take a struct timespec
>> parameter (which allows for nanosecond precision) so KVM and Xen
>> guests may start with a more accurate wallclock time and a Xen dom0
>> can maintain a more accurate wallclock for guests.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/include/asm/mc146818rtc.h | 4 ++--
>> arch/x86/include/asm/x86_init.h | 6 ++++--
>> arch/x86/kernel/kvmclock.c | 9 +++------
>> arch/x86/kernel/rtc.c | 17 +++++++----------
>> arch/x86/lguest/boot.c | 4 ++--
>> arch/x86/platform/efi/efi.c | 10 ++++++----
>> arch/x86/xen/time.c | 19 ++++++-------------
>> include/linux/efi.h | 4 ++--
>
> Just FYI, David: You missed the vrtc code with this conversion.
>
> I finally got around to pushing my current queue to my public tree and
> the kbuild robot noticed the build failure (required INTEL_MID config to
> trigger).
>
> The fix I've queued is here:
> https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=52d8e9cc6718ae9e6c44b7723028e4717ba91c8b;hp=1dd5234774b33a17743ae62e8b46b5cd0059e1c7
Oops. Sorry about that. That fix looks fine.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock()
2013-05-29 0:18 ` John Stultz
2013-05-29 12:16 ` David Vrabel
@ 2013-05-29 12:16 ` David Vrabel
1 sibling, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-29 12:16 UTC (permalink / raw)
To: John Stultz
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 29/05/13 01:18, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> All the virtualized platforms (KVM, lguest and Xen) have persistent
>> wallclocks that have more than one second of precision.
>>
>> read_persistent_wallclock() and update_persistent_wallclock() allow
>> for nanosecond precision but their implementation on x86 with
>> x86_platform.get/set_wallclock() only allows for one second precision.
>> This means guests may see a wallclock time that is off by up to 1
>> second.
>>
>> Make set_wallclock() and get_wallclock() take a struct timespec
>> parameter (which allows for nanosecond precision) so KVM and Xen
>> guests may start with a more accurate wallclock time and a Xen dom0
>> can maintain a more accurate wallclock for guests.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/include/asm/mc146818rtc.h | 4 ++--
>> arch/x86/include/asm/x86_init.h | 6 ++++--
>> arch/x86/kernel/kvmclock.c | 9 +++------
>> arch/x86/kernel/rtc.c | 17 +++++++----------
>> arch/x86/lguest/boot.c | 4 ++--
>> arch/x86/platform/efi/efi.c | 10 ++++++----
>> arch/x86/xen/time.c | 19 ++++++-------------
>> include/linux/efi.h | 4 ++--
>
> Just FYI, David: You missed the vrtc code with this conversion.
>
> I finally got around to pushing my current queue to my public tree and
> the kbuild robot noticed the build failure (required INTEL_MID config to
> trigger).
>
> The fix I've queued is here:
> https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=52d8e9cc6718ae9e6c44b7723028e4717ba91c8b;hp=1dd5234774b33a17743ae62e8b46b5cd0059e1c7
Oops. Sorry about that. That fix looks fine.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
2013-05-13 17:56 ` [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock() David Vrabel
2013-05-13 17:56 ` David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-13 17:56 ` David Vrabel
` (2 subsequent siblings)
5 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Thomas Gleixner, John Stultz, David Vrabel,
Konrad Rzeszutek Wilk
From: David Vrabel <david.vrabel@citrix.com>
The persistent clock or the RTC is only synchronized with system time
every 11 minutes if NTP is running. This gives a window where the
persistent clock may be incorrect after a step change in the time
(such as on first boot).
This particularly affects Xen guests as until an update to the control
domain's persistent clock, new guests will start with the incorrect
system time.
When there is a step change in the system time, call
update_persistent_clock or rtc_set_ntp_time() to synchronize the
persistent clock or RTC to the new system time.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
kernel/time/timekeeping.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 98cd470..56f2349 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+
+static void __sync_persistent_clock(struct work_struct *work)
+{
+ struct timespec now;
+ int ret = 0;
+
+ getnstimeofday(&now);
+
+#ifdef CONFIG_GENERIC_CMOS_UPDATE
+ ret = update_persistent_clock(now);
+#endif
+#ifdef CONFIG_RTC_SYSTOHC
+ if (ret == -ENODEV)
+ rtc_set_ntp_time(now);
+#endif
+}
+
+static DECLARE_DELAYED_WORK(sync_persistent_clock_work, __sync_persistent_clock);
+
+static void sync_persistent_clock(struct timekeeper *tk)
+{
+ u64 nsecs;
+ u32 remainder;
+
+ /* Many RTCs require updates 500 ms before the next second. */
+ nsecs = timekeeping_get_ns(tk);
+ div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
+ if (remainder > NSEC_PER_SEC / 2)
+ nsecs = remainder - NSEC_PER_SEC / 2;
+ else
+ nsecs = remainder + NSEC_PER_SEC / 2;
+
+ if (system_wq)
+ schedule_delayed_work(&sync_persistent_clock_work, nsecs_to_jiffies(nsecs));
+}
+
+#endif
+
/* must hold timekeeper_lock */
-static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
+static void timekeeping_update(struct timekeeper *tk, bool step, bool mirror)
{
- if (clearntp) {
+ if (step) {
tk->ntp_error = 0;
ntp_clear();
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+ sync_persistent_clock(tk);
+#endif
}
update_vsyscall(tk);
update_pvclock_gtod(tk);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
` (2 preceding siblings ...)
2013-05-13 17:56 ` [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-14 0:40 ` John Stultz
2013-05-14 0:40 ` John Stultz
2013-05-13 17:56 ` [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock David Vrabel
2013-05-13 17:56 ` David Vrabel
5 siblings, 2 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, John Stultz,
Thomas Gleixner, linux-kernel
From: David Vrabel <david.vrabel@citrix.com>
The persistent clock or the RTC is only synchronized with system time
every 11 minutes if NTP is running. This gives a window where the
persistent clock may be incorrect after a step change in the time
(such as on first boot).
This particularly affects Xen guests as until an update to the control
domain's persistent clock, new guests will start with the incorrect
system time.
When there is a step change in the system time, call
update_persistent_clock or rtc_set_ntp_time() to synchronize the
persistent clock or RTC to the new system time.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
kernel/time/timekeeping.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 98cd470..56f2349 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+
+static void __sync_persistent_clock(struct work_struct *work)
+{
+ struct timespec now;
+ int ret = 0;
+
+ getnstimeofday(&now);
+
+#ifdef CONFIG_GENERIC_CMOS_UPDATE
+ ret = update_persistent_clock(now);
+#endif
+#ifdef CONFIG_RTC_SYSTOHC
+ if (ret == -ENODEV)
+ rtc_set_ntp_time(now);
+#endif
+}
+
+static DECLARE_DELAYED_WORK(sync_persistent_clock_work, __sync_persistent_clock);
+
+static void sync_persistent_clock(struct timekeeper *tk)
+{
+ u64 nsecs;
+ u32 remainder;
+
+ /* Many RTCs require updates 500 ms before the next second. */
+ nsecs = timekeeping_get_ns(tk);
+ div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
+ if (remainder > NSEC_PER_SEC / 2)
+ nsecs = remainder - NSEC_PER_SEC / 2;
+ else
+ nsecs = remainder + NSEC_PER_SEC / 2;
+
+ if (system_wq)
+ schedule_delayed_work(&sync_persistent_clock_work, nsecs_to_jiffies(nsecs));
+}
+
+#endif
+
/* must hold timekeeper_lock */
-static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
+static void timekeeping_update(struct timekeeper *tk, bool step, bool mirror)
{
- if (clearntp) {
+ if (step) {
tk->ntp_error = 0;
ntp_clear();
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+ sync_persistent_clock(tk);
+#endif
}
update_vsyscall(tk);
update_pvclock_gtod(tk);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-13 17:56 ` David Vrabel
@ 2013-05-14 0:40 ` John Stultz
2013-05-14 0:40 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:40 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> The persistent clock or the RTC is only synchronized with system time
> every 11 minutes if NTP is running. This gives a window where the
> persistent clock may be incorrect after a step change in the time
> (such as on first boot).
>
> This particularly affects Xen guests as until an update to the control
> domain's persistent clock, new guests will start with the incorrect
> system time.
>
> When there is a step change in the system time, call
> update_persistent_clock or rtc_set_ntp_time() to synchronize the
> persistent clock or RTC to the new system time.
I'm sorry, this isn't quite making sense to me. Could you further
describe the exact problematic behavior you're seeing here, and why its
a problem?
You seem to be saying we should always set the RTC any time settimeofday
is called (regardless of the NTP sync state), which doesn't seem right
to me. Also I worry that this would cause the RTC to be set when the RTC
hctosys() code (or hwclock) sets the time to the RTC clock, which is a
bit circular.
I suspect once the problem is better understood, there will be a better
solution then trying to always set the RTC on any settimeofday() call.
thanks
-john
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> kernel/time/timekeeping.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 98cd470..56f2349 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
>
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> +
> +static void __sync_persistent_clock(struct work_struct *work)
> +{
> + struct timespec now;
> + int ret = 0;
> +
> + getnstimeofday(&now);
> +
> +#ifdef CONFIG_GENERIC_CMOS_UPDATE
> + ret = update_persistent_clock(now);
> +#endif
> +#ifdef CONFIG_RTC_SYSTOHC
> + if (ret == -ENODEV)
> + rtc_set_ntp_time(now);
> +#endif
> +}
> +
> +static DECLARE_DELAYED_WORK(sync_persistent_clock_work, __sync_persistent_clock);
> +
> +static void sync_persistent_clock(struct timekeeper *tk)
> +{
> + u64 nsecs;
> + u32 remainder;
> +
> + /* Many RTCs require updates 500 ms before the next second. */
> + nsecs = timekeeping_get_ns(tk);
> + div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
> + if (remainder > NSEC_PER_SEC / 2)
> + nsecs = remainder - NSEC_PER_SEC / 2;
> + else
> + nsecs = remainder + NSEC_PER_SEC / 2;
> +
> + if (system_wq)
> + schedule_delayed_work(&sync_persistent_clock_work, nsecs_to_jiffies(nsecs));
> +}
> +
> +#endif
> +
> /* must hold timekeeper_lock */
> -static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
> +static void timekeeping_update(struct timekeeper *tk, bool step, bool mirror)
> {
> - if (clearntp) {
> + if (step) {
> tk->ntp_error = 0;
> ntp_clear();
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> + sync_persistent_clock(tk);
> +#endif
> }
> update_vsyscall(tk);
> update_pvclock_gtod(tk);
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-13 17:56 ` David Vrabel
2013-05-14 0:40 ` John Stultz
@ 2013-05-14 0:40 ` John Stultz
2013-05-14 9:47 ` David Vrabel
2013-05-14 9:47 ` David Vrabel
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:40 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> The persistent clock or the RTC is only synchronized with system time
> every 11 minutes if NTP is running. This gives a window where the
> persistent clock may be incorrect after a step change in the time
> (such as on first boot).
>
> This particularly affects Xen guests as until an update to the control
> domain's persistent clock, new guests will start with the incorrect
> system time.
>
> When there is a step change in the system time, call
> update_persistent_clock or rtc_set_ntp_time() to synchronize the
> persistent clock or RTC to the new system time.
I'm sorry, this isn't quite making sense to me. Could you further
describe the exact problematic behavior you're seeing here, and why its
a problem?
You seem to be saying we should always set the RTC any time settimeofday
is called (regardless of the NTP sync state), which doesn't seem right
to me. Also I worry that this would cause the RTC to be set when the RTC
hctosys() code (or hwclock) sets the time to the RTC clock, which is a
bit circular.
I suspect once the problem is better understood, there will be a better
solution then trying to always set the RTC on any settimeofday() call.
thanks
-john
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> kernel/time/timekeeping.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 98cd470..56f2349 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
>
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> +
> +static void __sync_persistent_clock(struct work_struct *work)
> +{
> + struct timespec now;
> + int ret = 0;
> +
> + getnstimeofday(&now);
> +
> +#ifdef CONFIG_GENERIC_CMOS_UPDATE
> + ret = update_persistent_clock(now);
> +#endif
> +#ifdef CONFIG_RTC_SYSTOHC
> + if (ret == -ENODEV)
> + rtc_set_ntp_time(now);
> +#endif
> +}
> +
> +static DECLARE_DELAYED_WORK(sync_persistent_clock_work, __sync_persistent_clock);
> +
> +static void sync_persistent_clock(struct timekeeper *tk)
> +{
> + u64 nsecs;
> + u32 remainder;
> +
> + /* Many RTCs require updates 500 ms before the next second. */
> + nsecs = timekeeping_get_ns(tk);
> + div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
> + if (remainder > NSEC_PER_SEC / 2)
> + nsecs = remainder - NSEC_PER_SEC / 2;
> + else
> + nsecs = remainder + NSEC_PER_SEC / 2;
> +
> + if (system_wq)
> + schedule_delayed_work(&sync_persistent_clock_work, nsecs_to_jiffies(nsecs));
> +}
> +
> +#endif
> +
> /* must hold timekeeper_lock */
> -static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
> +static void timekeeping_update(struct timekeeper *tk, bool step, bool mirror)
> {
> - if (clearntp) {
> + if (step) {
> tk->ntp_error = 0;
> ntp_clear();
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> + sync_persistent_clock(tk);
> +#endif
> }
> update_vsyscall(tk);
> update_pvclock_gtod(tk);
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 0:40 ` John Stultz
@ 2013-05-14 9:47 ` David Vrabel
2013-05-14 9:47 ` David Vrabel
1 sibling, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 9:47 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 14/05/13 01:40, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> The persistent clock or the RTC is only synchronized with system time
>> every 11 minutes if NTP is running. This gives a window where the
>> persistent clock may be incorrect after a step change in the time
>> (such as on first boot).
>>
>> This particularly affects Xen guests as until an update to the control
>> domain's persistent clock, new guests will start with the incorrect
>> system time.
>>
>> When there is a step change in the system time, call
>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>> persistent clock or RTC to the new system time.
>
> I'm sorry, this isn't quite making sense to me. Could you further
> describe the exact problematic behavior you're seeing here, and why its
> a problem?
The Xen wallclock is used as the persistent clock for Xen guests. This
is initialized (by Xen) with the CMOS RTC at the start of day. If the
RTC is incorrect then guests will see an incorrect wallclock time until
dom0 has corrected it.
Currently dom0 only updates the Xen wallclock with the 11 min periodic
work when NTP is synced. This leaves a window where newly started
guests will see an incorrect wallclock time. This can cause guests to
fail to start correctly if the wallclock is now behind what it was when
the guest last started. (e.g., fsck of its disk fails as its last mount
time appears to be far into the future).
Similarly (but less problematic), if a bare metal system is rebooted
before the RTC is updated it will still have the incorrect time.
> You seem to be saying we should always set the RTC any time settimeofday
> is called (regardless of the NTP sync state), which doesn't seem right
> to me. Also I worry that this would cause the RTC to be set when the RTC
> hctosys() code (or hwclock) sets the time to the RTC clock, which is a
> bit circular.
I'm not too concerned about the behaviour of manual syncs of the RTC
because: a) if the kernel does this automatically then the use of manual
syncs is no longer necessary; and b) the RTC will still end up with the
correct time.
> I suspect once the problem is better understood, there will be a better
> solution then trying to always set the RTC on any settimeofday() call.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 0:40 ` John Stultz
2013-05-14 9:47 ` David Vrabel
@ 2013-05-14 9:47 ` David Vrabel
2013-05-14 17:15 ` John Stultz
2013-05-14 17:15 ` John Stultz
1 sibling, 2 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 9:47 UTC (permalink / raw)
To: John Stultz
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 14/05/13 01:40, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> The persistent clock or the RTC is only synchronized with system time
>> every 11 minutes if NTP is running. This gives a window where the
>> persistent clock may be incorrect after a step change in the time
>> (such as on first boot).
>>
>> This particularly affects Xen guests as until an update to the control
>> domain's persistent clock, new guests will start with the incorrect
>> system time.
>>
>> When there is a step change in the system time, call
>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>> persistent clock or RTC to the new system time.
>
> I'm sorry, this isn't quite making sense to me. Could you further
> describe the exact problematic behavior you're seeing here, and why its
> a problem?
The Xen wallclock is used as the persistent clock for Xen guests. This
is initialized (by Xen) with the CMOS RTC at the start of day. If the
RTC is incorrect then guests will see an incorrect wallclock time until
dom0 has corrected it.
Currently dom0 only updates the Xen wallclock with the 11 min periodic
work when NTP is synced. This leaves a window where newly started
guests will see an incorrect wallclock time. This can cause guests to
fail to start correctly if the wallclock is now behind what it was when
the guest last started. (e.g., fsck of its disk fails as its last mount
time appears to be far into the future).
Similarly (but less problematic), if a bare metal system is rebooted
before the RTC is updated it will still have the incorrect time.
> You seem to be saying we should always set the RTC any time settimeofday
> is called (regardless of the NTP sync state), which doesn't seem right
> to me. Also I worry that this would cause the RTC to be set when the RTC
> hctosys() code (or hwclock) sets the time to the RTC clock, which is a
> bit circular.
I'm not too concerned about the behaviour of manual syncs of the RTC
because: a) if the kernel does this automatically then the use of manual
syncs is no longer necessary; and b) the RTC will still end up with the
correct time.
> I suspect once the problem is better understood, there will be a better
> solution then trying to always set the RTC on any settimeofday() call.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 9:47 ` David Vrabel
@ 2013-05-14 17:15 ` John Stultz
2013-05-14 17:15 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:15 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/14/2013 02:47 AM, David Vrabel wrote:
> On 14/05/13 01:40, John Stultz wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> The persistent clock or the RTC is only synchronized with system time
>>> every 11 minutes if NTP is running. This gives a window where the
>>> persistent clock may be incorrect after a step change in the time
>>> (such as on first boot).
>>>
>>> This particularly affects Xen guests as until an update to the control
>>> domain's persistent clock, new guests will start with the incorrect
>>> system time.
>>>
>>> When there is a step change in the system time, call
>>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>>> persistent clock or RTC to the new system time.
>> I'm sorry, this isn't quite making sense to me. Could you further
>> describe the exact problematic behavior you're seeing here, and why its
>> a problem?
> The Xen wallclock is used as the persistent clock for Xen guests. This
> is initialized (by Xen) with the CMOS RTC at the start of day.
Start of the day? I assume you mean on dom0 bootup? Or is it done
pre-dom0 bootup by Xen itself?
> If the
> RTC is incorrect then guests will see an incorrect wallclock time until
> dom0 has corrected it.
Sorry, just a bit more clarifying context here: So there is a 1:1
relationship between xen_wall_clock and the RTC for all domN guests? And
even if dom0 has set its system time properly, domN guests will
initialize (in effect) from the hardware RTC and not from dom0's system
time?
So, let me see if I'm getting this right:
* Hardware has misconfigured RTC, set to the wrong date/time
* Xen boots up dom0 (or Xen itself) initializes the xen_wall_clock to
the RTC
* dom0 finishes booting, and uses NTP to correct the system time
* dom1 starts up, uses xen_wall_clock to initialize its system time, but
the RTC is still wrong, so it boots with the wrong time.
* After 11 minutes of sync w/ NTP, dom0 sets the RTC fixing the time.
* dom2 starts up, uses xen_wall_clock to initialize its system time
correctly.
At this point, dom0 and dom2 have the correct time and dom1 is
incorrect, right?
And with your other patches, after the next boot up (assuming dom0 is
synced with NTP for > 11 minutes) everything will be fine (and if NTP
doesn't set dom0's RTC, or the hwclock isn't otherwise corrected we'll
see the same behavior).
Is this correct?
> Currently dom0 only updates the Xen wallclock with the 11 min periodic
> work when NTP is synced. This leaves a window where newly started
> guests will see an incorrect wallclock time. This can cause guests to
> fail to start correctly if the wallclock is now behind what it was when
> the guest last started. (e.g., fsck of its disk fails as its last mount
> time appears to be far into the future).
>
> Similarly (but less problematic), if a bare metal system is rebooted
> before the RTC is updated it will still have the incorrect time.
So this has been the existing behavior for quite some time. If the RTC
is misconfigured, it has to be corrected either explicitly by the admin
via hwclock or by the kernel but only if we're well synced with NTP.
>> You seem to be saying we should always set the RTC any time settimeofday
>> is called (regardless of the NTP sync state), which doesn't seem right
>> to me. Also I worry that this would cause the RTC to be set when the RTC
>> hctosys() code (or hwclock) sets the time to the RTC clock, which is a
>> bit circular.
> I'm not too concerned about the behaviour of manual syncs of the RTC
> because: a) if the kernel does this automatically then the use of manual
> syncs is no longer necessary;
Well, we can't break existing interface behavior. Even if its
unnecessary at that point.
> and b) the RTC will still end up with the
> correct time.
But this isn't in fact the case. Imagine an networkless embedded system
that's system clock drifts. Its setup to use a cron job to set the time
a few times a day to correct this (its a bit naive, I know, but this is
how these embedded systems often are configured).
The problem is, that every time it uses hwclock --hctosys, we read the
RTC, and write it to the system time, which will then write that value
back to the RTC. Since there will be some delay between the RTC read and
the RTC write, we will inject some slight error into the RTC, such that
it may be a second behind where it ought to be.
We do this regularly enough, and now the RTC clock is drifting behind.
And sure, this is somewhat of a contrived an example, but we can't break
folks using these approaches.
So I think the other patches in this series are fine, and should help
limit the effect of the problematic case of a mis-configured RTC. But
this one I don't think we can do reasonably.
If you really feel this tight-binding of the system time and the RTC is
necessary (if NTP synced or not), you might continue working the
approach with following modifications:
1) Limit the RTC syncing to settimeofday() and inject_time_offset().
Where it is now, we'd call it on every resume from suspend, which would
cause the same problematic circular RTC drift on systems that do
frequent suspend/resumes.
2) Modify the logic so we only set the RTC on settimeofday() if the
current RTC value is more then N seconds off. This would limit the
circular drift, allowing time being set by the RTC to not result in
modifying the RTC.
With those two changes you might have a better chance, but I'm still
hesitant. There may be use cases where the RTC and the system time are
intentionally kept out of sync.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 9:47 ` David Vrabel
2013-05-14 17:15 ` John Stultz
@ 2013-05-14 17:15 ` John Stultz
2013-05-15 8:16 ` [Xen-devel] " Jan Beulich
2013-05-15 8:16 ` Jan Beulich
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:15 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/14/2013 02:47 AM, David Vrabel wrote:
> On 14/05/13 01:40, John Stultz wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> The persistent clock or the RTC is only synchronized with system time
>>> every 11 minutes if NTP is running. This gives a window where the
>>> persistent clock may be incorrect after a step change in the time
>>> (such as on first boot).
>>>
>>> This particularly affects Xen guests as until an update to the control
>>> domain's persistent clock, new guests will start with the incorrect
>>> system time.
>>>
>>> When there is a step change in the system time, call
>>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>>> persistent clock or RTC to the new system time.
>> I'm sorry, this isn't quite making sense to me. Could you further
>> describe the exact problematic behavior you're seeing here, and why its
>> a problem?
> The Xen wallclock is used as the persistent clock for Xen guests. This
> is initialized (by Xen) with the CMOS RTC at the start of day.
Start of the day? I assume you mean on dom0 bootup? Or is it done
pre-dom0 bootup by Xen itself?
> If the
> RTC is incorrect then guests will see an incorrect wallclock time until
> dom0 has corrected it.
Sorry, just a bit more clarifying context here: So there is a 1:1
relationship between xen_wall_clock and the RTC for all domN guests? And
even if dom0 has set its system time properly, domN guests will
initialize (in effect) from the hardware RTC and not from dom0's system
time?
So, let me see if I'm getting this right:
* Hardware has misconfigured RTC, set to the wrong date/time
* Xen boots up dom0 (or Xen itself) initializes the xen_wall_clock to
the RTC
* dom0 finishes booting, and uses NTP to correct the system time
* dom1 starts up, uses xen_wall_clock to initialize its system time, but
the RTC is still wrong, so it boots with the wrong time.
* After 11 minutes of sync w/ NTP, dom0 sets the RTC fixing the time.
* dom2 starts up, uses xen_wall_clock to initialize its system time
correctly.
At this point, dom0 and dom2 have the correct time and dom1 is
incorrect, right?
And with your other patches, after the next boot up (assuming dom0 is
synced with NTP for > 11 minutes) everything will be fine (and if NTP
doesn't set dom0's RTC, or the hwclock isn't otherwise corrected we'll
see the same behavior).
Is this correct?
> Currently dom0 only updates the Xen wallclock with the 11 min periodic
> work when NTP is synced. This leaves a window where newly started
> guests will see an incorrect wallclock time. This can cause guests to
> fail to start correctly if the wallclock is now behind what it was when
> the guest last started. (e.g., fsck of its disk fails as its last mount
> time appears to be far into the future).
>
> Similarly (but less problematic), if a bare metal system is rebooted
> before the RTC is updated it will still have the incorrect time.
So this has been the existing behavior for quite some time. If the RTC
is misconfigured, it has to be corrected either explicitly by the admin
via hwclock or by the kernel but only if we're well synced with NTP.
>> You seem to be saying we should always set the RTC any time settimeofday
>> is called (regardless of the NTP sync state), which doesn't seem right
>> to me. Also I worry that this would cause the RTC to be set when the RTC
>> hctosys() code (or hwclock) sets the time to the RTC clock, which is a
>> bit circular.
> I'm not too concerned about the behaviour of manual syncs of the RTC
> because: a) if the kernel does this automatically then the use of manual
> syncs is no longer necessary;
Well, we can't break existing interface behavior. Even if its
unnecessary at that point.
> and b) the RTC will still end up with the
> correct time.
But this isn't in fact the case. Imagine an networkless embedded system
that's system clock drifts. Its setup to use a cron job to set the time
a few times a day to correct this (its a bit naive, I know, but this is
how these embedded systems often are configured).
The problem is, that every time it uses hwclock --hctosys, we read the
RTC, and write it to the system time, which will then write that value
back to the RTC. Since there will be some delay between the RTC read and
the RTC write, we will inject some slight error into the RTC, such that
it may be a second behind where it ought to be.
We do this regularly enough, and now the RTC clock is drifting behind.
And sure, this is somewhat of a contrived an example, but we can't break
folks using these approaches.
So I think the other patches in this series are fine, and should help
limit the effect of the problematic case of a mis-configured RTC. But
this one I don't think we can do reasonably.
If you really feel this tight-binding of the system time and the RTC is
necessary (if NTP synced or not), you might continue working the
approach with following modifications:
1) Limit the RTC syncing to settimeofday() and inject_time_offset().
Where it is now, we'd call it on every resume from suspend, which would
cause the same problematic circular RTC drift on systems that do
frequent suspend/resumes.
2) Modify the logic so we only set the RTC on settimeofday() if the
current RTC value is more then N seconds off. This would limit the
circular drift, allowing time being set by the RTC to not result in
modifying the RTC.
With those two changes you might have a better chance, but I'm still
hesitant. There may be use cases where the RTC and the system time are
intentionally kept out of sync.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 17:15 ` John Stultz
@ 2013-05-15 8:16 ` Jan Beulich
2013-05-15 18:10 ` John Stultz
2013-05-15 18:10 ` [Xen-devel] " John Stultz
2013-05-15 8:16 ` Jan Beulich
1 sibling, 2 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-15 8:16 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk, linux-kernel
>>> On 14.05.13 at 19:15, John Stultz <john.stultz@linaro.org> wrote:
> On 05/14/2013 02:47 AM, David Vrabel wrote:
>> On 14/05/13 01:40, John Stultz wrote:
>>> I'm sorry, this isn't quite making sense to me. Could you further
>>> describe the exact problematic behavior you're seeing here, and why its
>>> a problem?
>> The Xen wallclock is used as the persistent clock for Xen guests. This
>> is initialized (by Xen) with the CMOS RTC at the start of day.
>
> Start of the day? I assume you mean on dom0 bootup? Or is it done
> pre-dom0 bootup by Xen itself?
It is, indeed - Xen reads the CMOS clock (or consults EFI) once
when it starts up, but leaves those alone as soon as it launched
Dom0.
>> If the
>> RTC is incorrect then guests will see an incorrect wallclock time until
>> dom0 has corrected it.
>
>
> Sorry, just a bit more clarifying context here: So there is a 1:1
> relationship between xen_wall_clock and the RTC for all domN guests? And
> even if dom0 has set its system time properly, domN guests will
> initialize (in effect) from the hardware RTC and not from dom0's system
> time?
No, (PV) DomU-s will get their time from the software clock that
Xen maintains, which Dom0 is helping keep in good shape when
NTP-synced.
So the hypervisor really doesn't care about the actual RTC getting
updated in hardware, all it needs is for Dom0 to notify it of wall
clock corrections.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-15 8:16 ` [Xen-devel] " Jan Beulich
@ 2013-05-15 18:10 ` John Stultz
2013-05-15 18:10 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-15 18:10 UTC (permalink / raw)
To: Jan Beulich
Cc: linux-kernel, Thomas Gleixner, Konrad Rzeszutek Wilk,
David Vrabel, xen-devel
On 05/15/2013 01:16 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 19:15, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/14/2013 02:47 AM, David Vrabel wrote:
>>> On 14/05/13 01:40, John Stultz wrote:
>>>> I'm sorry, this isn't quite making sense to me. Could you further
>>>> describe the exact problematic behavior you're seeing here, and why its
>>>> a problem?
>>> The Xen wallclock is used as the persistent clock for Xen guests. This
>>> is initialized (by Xen) with the CMOS RTC at the start of day.
>> Start of the day? I assume you mean on dom0 bootup? Or is it done
>> pre-dom0 bootup by Xen itself?
> It is, indeed - Xen reads the CMOS clock (or consults EFI) once
> when it starts up, but leaves those alone as soon as it launched
> Dom0.
>
>>> If the
>>> RTC is incorrect then guests will see an incorrect wallclock time until
>>> dom0 has corrected it.
>>
>> Sorry, just a bit more clarifying context here: So there is a 1:1
>> relationship between xen_wall_clock and the RTC for all domN guests? And
>> even if dom0 has set its system time properly, domN guests will
>> initialize (in effect) from the hardware RTC and not from dom0's system
>> time?
> No, (PV) DomU-s will get their time from the software clock that
> Xen maintains, which Dom0 is helping keep in good shape when
> NTP-synced.
Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
see the right time? That makes more sense, and means the window for
these sorts of issues is reasonably quite small.
David: So I'm less inclined to merge this individual change, but if you
still feel strongly about it, let me know and we can circle around on it
after you've addressed the specific issues I pointed out earlier.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-15 8:16 ` [Xen-devel] " Jan Beulich
2013-05-15 18:10 ` John Stultz
@ 2013-05-15 18:10 ` John Stultz
2013-05-28 18:26 ` David Vrabel
2013-05-28 18:26 ` David Vrabel
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-15 18:10 UTC (permalink / raw)
To: Jan Beulich
Cc: David Vrabel, Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk,
linux-kernel
On 05/15/2013 01:16 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 19:15, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/14/2013 02:47 AM, David Vrabel wrote:
>>> On 14/05/13 01:40, John Stultz wrote:
>>>> I'm sorry, this isn't quite making sense to me. Could you further
>>>> describe the exact problematic behavior you're seeing here, and why its
>>>> a problem?
>>> The Xen wallclock is used as the persistent clock for Xen guests. This
>>> is initialized (by Xen) with the CMOS RTC at the start of day.
>> Start of the day? I assume you mean on dom0 bootup? Or is it done
>> pre-dom0 bootup by Xen itself?
> It is, indeed - Xen reads the CMOS clock (or consults EFI) once
> when it starts up, but leaves those alone as soon as it launched
> Dom0.
>
>>> If the
>>> RTC is incorrect then guests will see an incorrect wallclock time until
>>> dom0 has corrected it.
>>
>> Sorry, just a bit more clarifying context here: So there is a 1:1
>> relationship between xen_wall_clock and the RTC for all domN guests? And
>> even if dom0 has set its system time properly, domN guests will
>> initialize (in effect) from the hardware RTC and not from dom0's system
>> time?
> No, (PV) DomU-s will get their time from the software clock that
> Xen maintains, which Dom0 is helping keep in good shape when
> NTP-synced.
Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
see the right time? That makes more sense, and means the window for
these sorts of issues is reasonably quite small.
David: So I'm less inclined to merge this individual change, but if you
still feel strongly about it, let me know and we can circle around on it
after you've addressed the specific issues I pointed out earlier.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-15 18:10 ` [Xen-devel] " John Stultz
@ 2013-05-28 18:26 ` David Vrabel
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
` (3 more replies)
2013-05-28 18:26 ` David Vrabel
1 sibling, 4 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-28 18:26 UTC (permalink / raw)
To: John Stultz
Cc: Jan Beulich, Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk,
linux-kernel
On 15/05/13 19:10, John Stultz wrote:
>
> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> see the right time? That makes more sense, and means the window for
> these sorts of issues is reasonably quite small.
It's a small window but it's occurring in our automated test system.
> David: So I'm less inclined to merge this individual change, but if you
> still feel strongly about it, let me know and we can circle around on it
> after you've addressed the specific issues I pointed out earlier.
This patch was the actual bug fix but I've reworked it to use the
pvclock_gtod notifier chain as this seemed to be what KVM hosts were
using to maintain a clock for guests. Please review the new series, thanks.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:26 ` David Vrabel
@ 2013-05-28 18:31 ` Konrad Rzeszutek Wilk
2013-05-28 19:09 ` John Stultz
2013-05-28 19:09 ` John Stultz
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
` (2 subsequent siblings)
3 siblings, 2 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 18:31 UTC (permalink / raw)
To: David Vrabel
Cc: John Stultz, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> On 15/05/13 19:10, John Stultz wrote:
> >
> > Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> > see the right time? That makes more sense, and means the window for
> > these sorts of issues is reasonably quite small.
>
> It's a small window but it's occurring in our automated test system.
>
> > David: So I'm less inclined to merge this individual change, but if you
> > still feel strongly about it, let me know and we can circle around on it
> > after you've addressed the specific issues I pointed out earlier.
>
> This patch was the actual bug fix but I've reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests. Please review the new series, thanks.
Looks good.
John if you are OK I am thinking to push this to Linus shortly as it is
fixing a bug.
>
> David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
@ 2013-05-28 19:09 ` John Stultz
2013-05-28 19:48 ` Konrad Rzeszutek Wilk
2013-05-28 19:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-05-28 19:09 ` John Stultz
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 19:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>> On 15/05/13 19:10, John Stultz wrote:
>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>>> see the right time? That makes more sense, and means the window for
>>> these sorts of issues is reasonably quite small.
>> It's a small window but it's occurring in our automated test system.
>>
>>> David: So I'm less inclined to merge this individual change, but if you
>>> still feel strongly about it, let me know and we can circle around on it
>>> after you've addressed the specific issues I pointed out earlier.
>> This patch was the actual bug fix but I've reworked it to use the
>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>> using to maintain a clock for guests. Please review the new series, thanks.
> Looks good.
>
> John if you are OK I am thinking to push this to Linus shortly as it is
> fixing a bug.
I'm really not sure I'd call this a bug. That seems like an
over-reaction to a misconfigured system.
Or if there is a bug, I'm not sure its been clearly explained.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 19:09 ` John Stultz
@ 2013-05-28 19:48 ` Konrad Rzeszutek Wilk
2013-05-28 19:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
1 sibling, 0 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 19:48 UTC (permalink / raw)
To: John Stultz
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> >>On 15/05/13 19:10, John Stultz wrote:
> >>>Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> >>>see the right time? That makes more sense, and means the window for
> >>>these sorts of issues is reasonably quite small.
> >>It's a small window but it's occurring in our automated test system.
> >>
> >>>David: So I'm less inclined to merge this individual change, but if you
> >>>still feel strongly about it, let me know and we can circle around on it
> >>>after you've addressed the specific issues I pointed out earlier.
> >>This patch was the actual bug fix but I've reworked it to use the
> >>pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> >>using to maintain a clock for guests. Please review the new series, thanks.
> >Looks good.
> >
> >John if you are OK I am thinking to push this to Linus shortly as it is
> >fixing a bug.
>
> I'm really not sure I'd call this a bug. That seems like an
> over-reaction to a misconfigured system.
>
> Or if there is a bug, I'm not sure its been clearly explained.
The #1 patch - b/c you try to set the RTC time and it actually never takes. Meaning
on the next time the machine is booted the time is again off.
>
> thanks
> -john
>
>
>
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 19:09 ` John Stultz
2013-05-28 19:48 ` Konrad Rzeszutek Wilk
@ 2013-05-28 19:48 ` Konrad Rzeszutek Wilk
2013-05-28 20:03 ` John Stultz
2013-05-28 20:03 ` [Xen-devel] " John Stultz
1 sibling, 2 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 19:48 UTC (permalink / raw)
To: John Stultz
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> >>On 15/05/13 19:10, John Stultz wrote:
> >>>Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> >>>see the right time? That makes more sense, and means the window for
> >>>these sorts of issues is reasonably quite small.
> >>It's a small window but it's occurring in our automated test system.
> >>
> >>>David: So I'm less inclined to merge this individual change, but if you
> >>>still feel strongly about it, let me know and we can circle around on it
> >>>after you've addressed the specific issues I pointed out earlier.
> >>This patch was the actual bug fix but I've reworked it to use the
> >>pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> >>using to maintain a clock for guests. Please review the new series, thanks.
> >Looks good.
> >
> >John if you are OK I am thinking to push this to Linus shortly as it is
> >fixing a bug.
>
> I'm really not sure I'd call this a bug. That seems like an
> over-reaction to a misconfigured system.
>
> Or if there is a bug, I'm not sure its been clearly explained.
The #1 patch - b/c you try to set the RTC time and it actually never takes. Meaning
on the next time the machine is booted the time is again off.
>
> thanks
> -john
>
>
>
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 19:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2013-05-28 20:03 ` John Stultz
2013-05-28 20:03 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>>>>> see the right time? That makes more sense, and means the window for
>>>>> these sorts of issues is reasonably quite small.
>>>> It's a small window but it's occurring in our automated test system.
>>>>
>>>>> David: So I'm less inclined to merge this individual change, but if you
>>>>> still feel strongly about it, let me know and we can circle around on it
>>>>> after you've addressed the specific issues I pointed out earlier.
>>>> This patch was the actual bug fix but I've reworked it to use the
>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>> using to maintain a clock for guests. Please review the new series, thanks.
>>> Looks good.
>>>
>>> John if you are OK I am thinking to push this to Linus shortly as it is
>>> fixing a bug.
>> I'm really not sure I'd call this a bug. That seems like an
>> over-reaction to a misconfigured system.
>>
>> Or if there is a bug, I'm not sure its been clearly explained.
> The #1 patch - b/c you try to set the RTC time and it actually never takes. Meaning
> on the next time the machine is booted the time is again off.
Ok. Yea, that one I'm fine with and have queued for 3.11.
If you want to push it as a bug fix for 3.10, I'll leave it to you to
push to Linus.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 19:48 ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-05-28 20:03 ` John Stultz
@ 2013-05-28 20:03 ` John Stultz
2013-05-28 20:11 ` John Stultz
2013-05-28 20:11 ` [Xen-devel] " John Stultz
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>>>>> see the right time? That makes more sense, and means the window for
>>>>> these sorts of issues is reasonably quite small.
>>>> It's a small window but it's occurring in our automated test system.
>>>>
>>>>> David: So I'm less inclined to merge this individual change, but if you
>>>>> still feel strongly about it, let me know and we can circle around on it
>>>>> after you've addressed the specific issues I pointed out earlier.
>>>> This patch was the actual bug fix but I've reworked it to use the
>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>> using to maintain a clock for guests. Please review the new series, thanks.
>>> Looks good.
>>>
>>> John if you are OK I am thinking to push this to Linus shortly as it is
>>> fixing a bug.
>> I'm really not sure I'd call this a bug. That seems like an
>> over-reaction to a misconfigured system.
>>
>> Or if there is a bug, I'm not sure its been clearly explained.
> The #1 patch - b/c you try to set the RTC time and it actually never takes. Meaning
> on the next time the machine is booted the time is again off.
Ok. Yea, that one I'm fine with and have queued for 3.11.
If you want to push it as a bug fix for 3.10, I'll leave it to you to
push to Linus.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 20:03 ` [Xen-devel] " John Stultz
@ 2013-05-28 20:11 ` John Stultz
2013-05-28 20:11 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:11 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On 05/28/2013 01:03 PM, John Stultz wrote:
> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests
>>>>>> will
>>>>>> see the right time? That makes more sense, and means the window for
>>>>>> these sorts of issues is reasonably quite small.
>>>>> It's a small window but it's occurring in our automated test system.
>>>>>
>>>>>> David: So I'm less inclined to merge this individual change, but
>>>>>> if you
>>>>>> still feel strongly about it, let me know and we can circle
>>>>>> around on it
>>>>>> after you've addressed the specific issues I pointed out earlier.
>>>>> This patch was the actual bug fix but I've reworked it to use the
>>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>>> using to maintain a clock for guests. Please review the new
>>>>> series, thanks.
>>>> Looks good.
>>>>
>>>> John if you are OK I am thinking to push this to Linus shortly as
>>>> it is
>>>> fixing a bug.
>>> I'm really not sure I'd call this a bug. That seems like an
>>> over-reaction to a misconfigured system.
>>>
>>> Or if there is a bug, I'm not sure its been clearly explained.
>> The #1 patch - b/c you try to set the RTC time and it actually never
>> takes. Meaning
>> on the next time the machine is booted the time is again off.
>
> Ok. Yea, that one I'm fine with and have queued for 3.11.
>
> If you want to push it as a bug fix for 3.10, I'll leave it to you to
> push to Linus.
Probably obvious, but just in case its not clear:
Though if that one patch goes to linus for 3.10, it needs to be reworked
to not depend on the mach_set_rtc_mmss() interface change it currently
depends on. The interface change is not bugfix material.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 20:03 ` [Xen-devel] " John Stultz
2013-05-28 20:11 ` John Stultz
@ 2013-05-28 20:11 ` John Stultz
2013-05-28 20:25 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:11 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On 05/28/2013 01:03 PM, John Stultz wrote:
> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests
>>>>>> will
>>>>>> see the right time? That makes more sense, and means the window for
>>>>>> these sorts of issues is reasonably quite small.
>>>>> It's a small window but it's occurring in our automated test system.
>>>>>
>>>>>> David: So I'm less inclined to merge this individual change, but
>>>>>> if you
>>>>>> still feel strongly about it, let me know and we can circle
>>>>>> around on it
>>>>>> after you've addressed the specific issues I pointed out earlier.
>>>>> This patch was the actual bug fix but I've reworked it to use the
>>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>>> using to maintain a clock for guests. Please review the new
>>>>> series, thanks.
>>>> Looks good.
>>>>
>>>> John if you are OK I am thinking to push this to Linus shortly as
>>>> it is
>>>> fixing a bug.
>>> I'm really not sure I'd call this a bug. That seems like an
>>> over-reaction to a misconfigured system.
>>>
>>> Or if there is a bug, I'm not sure its been clearly explained.
>> The #1 patch - b/c you try to set the RTC time and it actually never
>> takes. Meaning
>> on the next time the machine is booted the time is again off.
>
> Ok. Yea, that one I'm fine with and have queued for 3.11.
>
> If you want to push it as a bug fix for 3.10, I'll leave it to you to
> push to Linus.
Probably obvious, but just in case its not clear:
Though if that one patch goes to linus for 3.10, it needs to be reworked
to not depend on the mach_set_rtc_mmss() interface change it currently
depends on. The interface change is not bugfix material.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 20:11 ` [Xen-devel] " John Stultz
@ 2013-05-28 20:25 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 20:25 UTC (permalink / raw)
To: John Stultz
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz wrote:
> On 05/28/2013 01:03 PM, John Stultz wrote:
> >On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
> >>>On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >>>>On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> >>>>>On 15/05/13 19:10, John Stultz wrote:
> >>>>>>Ok, so really, as soon as the Dom0 time is set by NTP,
> >>>>>>all guests will
> >>>>>>see the right time? That makes more sense, and means the window for
> >>>>>>these sorts of issues is reasonably quite small.
> >>>>>It's a small window but it's occurring in our automated test system.
> >>>>>
> >>>>>>David: So I'm less inclined to merge this individual
> >>>>>>change, but if you
> >>>>>>still feel strongly about it, let me know and we can
> >>>>>>circle around on it
> >>>>>>after you've addressed the specific issues I pointed out earlier.
> >>>>>This patch was the actual bug fix but I've reworked it to use the
> >>>>>pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> >>>>>using to maintain a clock for guests. Please review the
> >>>>>new series, thanks.
> >>>>Looks good.
> >>>>
> >>>>John if you are OK I am thinking to push this to Linus
> >>>>shortly as it is
> >>>>fixing a bug.
> >>>I'm really not sure I'd call this a bug. That seems like an
> >>>over-reaction to a misconfigured system.
> >>>
> >>>Or if there is a bug, I'm not sure its been clearly explained.
> >>The #1 patch - b/c you try to set the RTC time and it actually
> >>never takes. Meaning
> >>on the next time the machine is booted the time is again off.
> >
> >Ok. Yea, that one I'm fine with and have queued for 3.11.
> >
> >If you want to push it as a bug fix for 3.10, I'll leave it to you
> >to push to Linus.
>
> Probably obvious, but just in case its not clear:
>
> Though if that one patch goes to linus for 3.10, it needs to be
> reworked to not depend on the mach_set_rtc_mmss() interface change
> it currently depends on. The interface change is not bugfix
> material.
You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
Author: Prarit Bhargava <prarit@redhat.com>
Date: Thu Feb 14 12:02:54 2013 -0500
x86: Do full rtc synchronization with ntp
which is not CC-ed to stable@vger.kernel.org, so the #1 patch would
not be able to be back-ported to any kernel right (so v3.9, v3.8..)?
But it could go to v3.10 - as that change is there. But you are saying
it can't go to v3.10 b/c the interface change is not a bugfix
material. Is that b/c said git commit might cause regressions and
you wouldn't want to do two reverts?
That makes sense - which point I you are right that it makes
sense to stick said patch (#1) in your 3.11 queue and skip the v3.10
train for it.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
@ 2013-05-28 20:25 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 20:25 UTC (permalink / raw)
To: John Stultz
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz wrote:
> On 05/28/2013 01:03 PM, John Stultz wrote:
> >On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
> >>>On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >>>>On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> >>>>>On 15/05/13 19:10, John Stultz wrote:
> >>>>>>Ok, so really, as soon as the Dom0 time is set by NTP,
> >>>>>>all guests will
> >>>>>>see the right time? That makes more sense, and means the window for
> >>>>>>these sorts of issues is reasonably quite small.
> >>>>>It's a small window but it's occurring in our automated test system.
> >>>>>
> >>>>>>David: So I'm less inclined to merge this individual
> >>>>>>change, but if you
> >>>>>>still feel strongly about it, let me know and we can
> >>>>>>circle around on it
> >>>>>>after you've addressed the specific issues I pointed out earlier.
> >>>>>This patch was the actual bug fix but I've reworked it to use the
> >>>>>pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> >>>>>using to maintain a clock for guests. Please review the
> >>>>>new series, thanks.
> >>>>Looks good.
> >>>>
> >>>>John if you are OK I am thinking to push this to Linus
> >>>>shortly as it is
> >>>>fixing a bug.
> >>>I'm really not sure I'd call this a bug. That seems like an
> >>>over-reaction to a misconfigured system.
> >>>
> >>>Or if there is a bug, I'm not sure its been clearly explained.
> >>The #1 patch - b/c you try to set the RTC time and it actually
> >>never takes. Meaning
> >>on the next time the machine is booted the time is again off.
> >
> >Ok. Yea, that one I'm fine with and have queued for 3.11.
> >
> >If you want to push it as a bug fix for 3.10, I'll leave it to you
> >to push to Linus.
>
> Probably obvious, but just in case its not clear:
>
> Though if that one patch goes to linus for 3.10, it needs to be
> reworked to not depend on the mach_set_rtc_mmss() interface change
> it currently depends on. The interface change is not bugfix
> material.
You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
Author: Prarit Bhargava <prarit@redhat.com>
Date: Thu Feb 14 12:02:54 2013 -0500
x86: Do full rtc synchronization with ntp
which is not CC-ed to stable@vger.kernel.org, so the #1 patch would
not be able to be back-ported to any kernel right (so v3.9, v3.8..)?
But it could go to v3.10 - as that change is there. But you are saying
it can't go to v3.10 b/c the interface change is not a bugfix
material. Is that b/c said git commit might cause regressions and
you wouldn't want to do two reverts?
That makes sense - which point I you are right that it makes
sense to stick said patch (#1) in your 3.11 queue and skip the v3.10
train for it.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 20:25 ` Konrad Rzeszutek Wilk
(?)
@ 2013-05-28 20:30 ` John Stultz
-1 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:30 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On 05/28/2013 01:25 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz wrote:
>> On 05/28/2013 01:03 PM, John Stultz wrote:
>>> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>>>> Ok, so really, as soon as the Dom0 time is set by NTP,
>>>>>>>> all guests will
>>>>>>>> see the right time? That makes more sense, and means the window for
>>>>>>>> these sorts of issues is reasonably quite small.
>>>>>>> It's a small window but it's occurring in our automated test system.
>>>>>>>
>>>>>>>> David: So I'm less inclined to merge this individual
>>>>>>>> change, but if you
>>>>>>>> still feel strongly about it, let me know and we can
>>>>>>>> circle around on it
>>>>>>>> after you've addressed the specific issues I pointed out earlier.
>>>>>>> This patch was the actual bug fix but I've reworked it to use the
>>>>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>>>>> using to maintain a clock for guests. Please review the
>>>>>>> new series, thanks.
>>>>>> Looks good.
>>>>>>
>>>>>> John if you are OK I am thinking to push this to Linus
>>>>>> shortly as it is
>>>>>> fixing a bug.
>>>>> I'm really not sure I'd call this a bug. That seems like an
>>>>> over-reaction to a misconfigured system.
>>>>>
>>>>> Or if there is a bug, I'm not sure its been clearly explained.
>>>> The #1 patch - b/c you try to set the RTC time and it actually
>>>> never takes. Meaning
>>>> on the next time the machine is booted the time is again off.
>>> Ok. Yea, that one I'm fine with and have queued for 3.11.
>>>
>>> If you want to push it as a bug fix for 3.10, I'll leave it to you
>>> to push to Linus.
>> Probably obvious, but just in case its not clear:
>>
>> Though if that one patch goes to linus for 3.10, it needs to be
>> reworked to not depend on the mach_set_rtc_mmss() interface change
>> it currently depends on. The interface change is not bugfix
>> material.
> You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
> Author: Prarit Bhargava <prarit@redhat.com>
> Date: Thu Feb 14 12:02:54 2013 -0500
>
> x86: Do full rtc synchronization with ntp
No. I mean David's patch:
x86: Increase precision of x86_platform.get/set_wallclock()
That one should be held back for 3.11.
But the calling of mach_set_rtc_mmss() from the Xen set_wallclock hook
would still be reasonable to go in 3.10 (limited impact to only Xen,
fixing a bug), but it would have to limit the call to the second
granular mach_set_rtc_mmss() in 3.10.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 20:25 ` Konrad Rzeszutek Wilk
(?)
(?)
@ 2013-05-28 20:30 ` John Stultz
-1 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 20:30 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, Jan Beulich, Thomas Gleixner, xen-devel, linux-kernel
On 05/28/2013 01:25 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz wrote:
>> On 05/28/2013 01:03 PM, John Stultz wrote:
>>> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>>>> Ok, so really, as soon as the Dom0 time is set by NTP,
>>>>>>>> all guests will
>>>>>>>> see the right time? That makes more sense, and means the window for
>>>>>>>> these sorts of issues is reasonably quite small.
>>>>>>> It's a small window but it's occurring in our automated test system.
>>>>>>>
>>>>>>>> David: So I'm less inclined to merge this individual
>>>>>>>> change, but if you
>>>>>>>> still feel strongly about it, let me know and we can
>>>>>>>> circle around on it
>>>>>>>> after you've addressed the specific issues I pointed out earlier.
>>>>>>> This patch was the actual bug fix but I've reworked it to use the
>>>>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>>>>>>> using to maintain a clock for guests. Please review the
>>>>>>> new series, thanks.
>>>>>> Looks good.
>>>>>>
>>>>>> John if you are OK I am thinking to push this to Linus
>>>>>> shortly as it is
>>>>>> fixing a bug.
>>>>> I'm really not sure I'd call this a bug. That seems like an
>>>>> over-reaction to a misconfigured system.
>>>>>
>>>>> Or if there is a bug, I'm not sure its been clearly explained.
>>>> The #1 patch - b/c you try to set the RTC time and it actually
>>>> never takes. Meaning
>>>> on the next time the machine is booted the time is again off.
>>> Ok. Yea, that one I'm fine with and have queued for 3.11.
>>>
>>> If you want to push it as a bug fix for 3.10, I'll leave it to you
>>> to push to Linus.
>> Probably obvious, but just in case its not clear:
>>
>> Though if that one patch goes to linus for 3.10, it needs to be
>> reworked to not depend on the mach_set_rtc_mmss() interface change
>> it currently depends on. The interface change is not bugfix
>> material.
> You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
> Author: Prarit Bhargava <prarit@redhat.com>
> Date: Thu Feb 14 12:02:54 2013 -0500
>
> x86: Do full rtc synchronization with ntp
No. I mean David's patch:
x86: Increase precision of x86_platform.get/set_wallclock()
That one should be held back for 3.11.
But the calling of mach_set_rtc_mmss() from the Xen set_wallclock hook
would still be reasonable to go in 3.10 (limited impact to only Xen,
fixing a bug), but it would have to limit the call to the second
granular mach_set_rtc_mmss() in 3.10.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
2013-05-28 19:09 ` John Stultz
@ 2013-05-28 19:09 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 19:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, Thomas Gleixner, David Vrabel, Jan Beulich, xen-devel
On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>> On 15/05/13 19:10, John Stultz wrote:
>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>>> see the right time? That makes more sense, and means the window for
>>> these sorts of issues is reasonably quite small.
>> It's a small window but it's occurring in our automated test system.
>>
>>> David: So I'm less inclined to merge this individual change, but if you
>>> still feel strongly about it, let me know and we can circle around on it
>>> after you've addressed the specific issues I pointed out earlier.
>> This patch was the actual bug fix but I've reworked it to use the
>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>> using to maintain a clock for guests. Please review the new series, thanks.
> Looks good.
>
> John if you are OK I am thinking to push this to Linus shortly as it is
> fixing a bug.
I'm really not sure I'd call this a bug. That seems like an
over-reaction to a misconfigured system.
Or if there is a bug, I'm not sure its been clearly explained.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:26 ` David Vrabel
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
@ 2013-05-28 18:31 ` Konrad Rzeszutek Wilk
2013-05-28 19:06 ` John Stultz
2013-05-28 19:06 ` [Xen-devel] " John Stultz
3 siblings, 0 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 18:31 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, John Stultz, linux-kernel, Jan Beulich, xen-devel
On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> On 15/05/13 19:10, John Stultz wrote:
> >
> > Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> > see the right time? That makes more sense, and means the window for
> > these sorts of issues is reasonably quite small.
>
> It's a small window but it's occurring in our automated test system.
>
> > David: So I'm less inclined to merge this individual change, but if you
> > still feel strongly about it, let me know and we can circle around on it
> > after you've addressed the specific issues I pointed out earlier.
>
> This patch was the actual bug fix but I've reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests. Please review the new series, thanks.
Looks good.
John if you are OK I am thinking to push this to Linus shortly as it is
fixing a bug.
>
> David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:26 ` David Vrabel
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
2013-05-28 18:31 ` Konrad Rzeszutek Wilk
@ 2013-05-28 19:06 ` John Stultz
2013-05-28 19:06 ` [Xen-devel] " John Stultz
3 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 19:06 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel,
Jan Beulich, xen-devel
On 05/28/2013 11:26 AM, David Vrabel wrote:
> On 15/05/13 19:10, John Stultz wrote:
>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>> see the right time? That makes more sense, and means the window for
>> these sorts of issues is reasonably quite small.
> It's a small window but it's occurring in our automated test system.
What is it with about your automated test system that causes a
misconfigured RTC & system such that xen guests are started before Dom0
has finished being initialized? Or am I still misunderstanding the
situation?
1) Dom0 boots w/ misconfigured RTC
2) Dom0 starts ntp, correcting the date
3) After 11 minutes of sync the RTC is synced to NTP time.
From earlier discussions, the only problem would be with guests
starting between 1 and 2 (which seems a bit early for guests to be
starting). And after 3, with your patch 1/2 (that I've queued) the issue
should not recur again, as the RTC is corrected.
>
>> David: So I'm less inclined to merge this individual change, but if you
>> still feel strongly about it, let me know and we can circle around on it
>> after you've addressed the specific issues I pointed out earlier.
> This patch was the actual bug fix but I've reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests. Please review the new series, thanks.
I'm still not convinced that we should be changing the behavior we've
had for quite a long time.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-28 18:26 ` David Vrabel
` (2 preceding siblings ...)
2013-05-28 19:06 ` John Stultz
@ 2013-05-28 19:06 ` John Stultz
3 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-28 19:06 UTC (permalink / raw)
To: David Vrabel
Cc: Jan Beulich, Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk,
linux-kernel
On 05/28/2013 11:26 AM, David Vrabel wrote:
> On 15/05/13 19:10, John Stultz wrote:
>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>> see the right time? That makes more sense, and means the window for
>> these sorts of issues is reasonably quite small.
> It's a small window but it's occurring in our automated test system.
What is it with about your automated test system that causes a
misconfigured RTC & system such that xen guests are started before Dom0
has finished being initialized? Or am I still misunderstanding the
situation?
1) Dom0 boots w/ misconfigured RTC
2) Dom0 starts ntp, correcting the date
3) After 11 minutes of sync the RTC is synced to NTP time.
From earlier discussions, the only problem would be with guests
starting between 1 and 2 (which seems a bit early for guests to be
starting). And after 3, with your patch 1/2 (that I've queued) the issue
should not recur again, as the RTC is corrected.
>
>> David: So I'm less inclined to merge this individual change, but if you
>> still feel strongly about it, let me know and we can circle around on it
>> after you've addressed the specific issues I pointed out earlier.
> This patch was the actual bug fix but I've reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests. Please review the new series, thanks.
I'm still not convinced that we should be changing the behavior we've
had for quite a long time.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-15 18:10 ` [Xen-devel] " John Stultz
2013-05-28 18:26 ` David Vrabel
@ 2013-05-28 18:26 ` David Vrabel
1 sibling, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-28 18:26 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel,
Jan Beulich, xen-devel
On 15/05/13 19:10, John Stultz wrote:
>
> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> see the right time? That makes more sense, and means the window for
> these sorts of issues is reasonably quite small.
It's a small window but it's occurring in our automated test system.
> David: So I'm less inclined to merge this individual change, but if you
> still feel strongly about it, let me know and we can circle around on it
> after you've addressed the specific issues I pointed out earlier.
This patch was the actual bug fix but I've reworked it to use the
pvclock_gtod notifier chain as this seemed to be what KVM hosts were
using to maintain a clock for guests. Please review the new series, thanks.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
2013-05-14 17:15 ` John Stultz
2013-05-15 8:16 ` [Xen-devel] " Jan Beulich
@ 2013-05-15 8:16 ` Jan Beulich
1 sibling, 0 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-15 8:16 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
>>> On 14.05.13 at 19:15, John Stultz <john.stultz@linaro.org> wrote:
> On 05/14/2013 02:47 AM, David Vrabel wrote:
>> On 14/05/13 01:40, John Stultz wrote:
>>> I'm sorry, this isn't quite making sense to me. Could you further
>>> describe the exact problematic behavior you're seeing here, and why its
>>> a problem?
>> The Xen wallclock is used as the persistent clock for Xen guests. This
>> is initialized (by Xen) with the CMOS RTC at the start of day.
>
> Start of the day? I assume you mean on dom0 bootup? Or is it done
> pre-dom0 bootup by Xen itself?
It is, indeed - Xen reads the CMOS clock (or consults EFI) once
when it starts up, but leaves those alone as soon as it launched
Dom0.
>> If the
>> RTC is incorrect then guests will see an incorrect wallclock time until
>> dom0 has corrected it.
>
>
> Sorry, just a bit more clarifying context here: So there is a 1:1
> relationship between xen_wall_clock and the RTC for all domN guests? And
> even if dom0 has set its system time properly, domN guests will
> initialize (in effect) from the hardware RTC and not from dom0's system
> time?
No, (PV) DomU-s will get their time from the software clock that
Xen maintains, which Dom0 is helping keep in good shape when
NTP-synced.
So the hypervisor really doesn't care about the actual RTC getting
updated in hardware, all it needs is for Dom0 to notify it of wall
clock corrections.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
` (3 preceding siblings ...)
2013-05-13 17:56 ` David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-13 17:56 ` David Vrabel
5 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Thomas Gleixner, John Stultz, David Vrabel,
Konrad Rzeszutek Wilk
From: David Vrabel <david.vrabel@citrix.com>
If NTP is used in dom0 and it is synchronized to its clock source,
then the kernel will periodically synchronize the Xen wallclock with
the system time. Updates to the Xen wallclock do not persist across
reboots, so also synchronize the CMOS RTC (as on bare metal).
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
arch/x86/xen/time.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index a1947ac..4656165 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -14,6 +14,7 @@
#include <linux/kernel_stat.h>
#include <linux/math64.h>
#include <linux/gfp.h>
+#include <linux/mc146818rtc.h>
#include <asm/pvclock.h>
#include <asm/xen/hypervisor.h>
@@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
static int xen_set_wallclock(const struct timespec *now)
{
struct xen_platform_op op;
+ int ret;
/* do nothing for domU */
if (!xen_initial_domain())
return -1;
+ /* Set the Xen wallclock. */
op.cmd = XENPF_settime;
op.u.settime.secs = now->tv_sec;
op.u.settime.nsecs = now->tv_nsec;
op.u.settime.system_time = xen_clocksource_read();
- return HYPERVISOR_dom0_op(&op);
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret)
+ return ret;
+
+ /* Set the hardware RTC. */
+ return mach_set_rtc_mmss(now);
+
}
static struct clocksource xen_clocksource __read_mostly = {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
` (4 preceding siblings ...)
2013-05-13 17:56 ` [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock David Vrabel
@ 2013-05-13 17:56 ` David Vrabel
2013-05-14 0:52 ` John Stultz
` (3 more replies)
5 siblings, 4 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-13 17:56 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, John Stultz,
Thomas Gleixner, linux-kernel
From: David Vrabel <david.vrabel@citrix.com>
If NTP is used in dom0 and it is synchronized to its clock source,
then the kernel will periodically synchronize the Xen wallclock with
the system time. Updates to the Xen wallclock do not persist across
reboots, so also synchronize the CMOS RTC (as on bare metal).
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
arch/x86/xen/time.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index a1947ac..4656165 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -14,6 +14,7 @@
#include <linux/kernel_stat.h>
#include <linux/math64.h>
#include <linux/gfp.h>
+#include <linux/mc146818rtc.h>
#include <asm/pvclock.h>
#include <asm/xen/hypervisor.h>
@@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
static int xen_set_wallclock(const struct timespec *now)
{
struct xen_platform_op op;
+ int ret;
/* do nothing for domU */
if (!xen_initial_domain())
return -1;
+ /* Set the Xen wallclock. */
op.cmd = XENPF_settime;
op.u.settime.secs = now->tv_sec;
op.u.settime.nsecs = now->tv_nsec;
op.u.settime.system_time = xen_clocksource_read();
- return HYPERVISOR_dom0_op(&op);
+ ret = HYPERVISOR_dom0_op(&op);
+ if (ret)
+ return ret;
+
+ /* Set the hardware RTC. */
+ return mach_set_rtc_mmss(now);
+
}
static struct clocksource xen_clocksource __read_mostly = {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 ` David Vrabel
@ 2013-05-14 0:52 ` John Stultz
2013-05-14 0:52 ` John Stultz
` (2 subsequent siblings)
3 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:52 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel,
Jeremy Fitzhardinge, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If NTP is used in dom0 and it is synchronized to its clock source,
> then the kernel will periodically synchronize the Xen wallclock with
> the system time. Updates to the Xen wallclock do not persist across
> reboots, so also synchronize the CMOS RTC (as on bare metal).
Sorry again, not getting this one either.
So normally in this case we're using the Xen wallclock as the underlying
source for the persistent_clock here, my understanding is we use this
instead of the standard cmos, because we get benefits of using the
hypervisor's sense of time instead of the bare hardware, and allows for
virtualization of the persistent clock.
But the problem is that even if Dom0 tries to set the xen persistent
clock, it doesn't actually update anything in the underlying hardware?
So here you instead try to sync the underlying hardware cmos from the
same Xen dom0 environment?
Honestly, it seems a little strange to me. If you're running as dom0,
why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
its virtualizing? This seems to mess with the proper virtualization
layering.
I'd appreciate some more details and maybe some feedback from other xen
folks here to make sure this really makes sense.
thanks
-john
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/xen/time.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a1947ac..4656165 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -14,6 +14,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/math64.h>
> #include <linux/gfp.h>
> +#include <linux/mc146818rtc.h>
>
> #include <asm/pvclock.h>
> #include <asm/xen/hypervisor.h>
> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
> static int xen_set_wallclock(const struct timespec *now)
> {
> struct xen_platform_op op;
> + int ret;
>
> /* do nothing for domU */
> if (!xen_initial_domain())
> return -1;
>
> + /* Set the Xen wallclock. */
> op.cmd = XENPF_settime;
> op.u.settime.secs = now->tv_sec;
> op.u.settime.nsecs = now->tv_nsec;
> op.u.settime.system_time = xen_clocksource_read();
>
> - return HYPERVISOR_dom0_op(&op);
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /* Set the hardware RTC. */
> + return mach_set_rtc_mmss(now);
> +
> }
>
> static struct clocksource xen_clocksource __read_mostly = {
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 ` David Vrabel
2013-05-14 0:52 ` John Stultz
@ 2013-05-14 0:52 ` John Stultz
2013-05-14 7:57 ` Jan Beulich
` (3 more replies)
2013-05-14 17:24 ` John Stultz
2013-05-14 17:24 ` John Stultz
3 siblings, 4 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 0:52 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel,
Jeremy Fitzhardinge
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If NTP is used in dom0 and it is synchronized to its clock source,
> then the kernel will periodically synchronize the Xen wallclock with
> the system time. Updates to the Xen wallclock do not persist across
> reboots, so also synchronize the CMOS RTC (as on bare metal).
Sorry again, not getting this one either.
So normally in this case we're using the Xen wallclock as the underlying
source for the persistent_clock here, my understanding is we use this
instead of the standard cmos, because we get benefits of using the
hypervisor's sense of time instead of the bare hardware, and allows for
virtualization of the persistent clock.
But the problem is that even if Dom0 tries to set the xen persistent
clock, it doesn't actually update anything in the underlying hardware?
So here you instead try to sync the underlying hardware cmos from the
same Xen dom0 environment?
Honestly, it seems a little strange to me. If you're running as dom0,
why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
its virtualizing? This seems to mess with the proper virtualization
layering.
I'd appreciate some more details and maybe some feedback from other xen
folks here to make sure this really makes sense.
thanks
-john
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/xen/time.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a1947ac..4656165 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -14,6 +14,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/math64.h>
> #include <linux/gfp.h>
> +#include <linux/mc146818rtc.h>
>
> #include <asm/pvclock.h>
> #include <asm/xen/hypervisor.h>
> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
> static int xen_set_wallclock(const struct timespec *now)
> {
> struct xen_platform_op op;
> + int ret;
>
> /* do nothing for domU */
> if (!xen_initial_domain())
> return -1;
>
> + /* Set the Xen wallclock. */
> op.cmd = XENPF_settime;
> op.u.settime.secs = now->tv_sec;
> op.u.settime.nsecs = now->tv_nsec;
> op.u.settime.system_time = xen_clocksource_read();
>
> - return HYPERVISOR_dom0_op(&op);
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /* Set the hardware RTC. */
> + return mach_set_rtc_mmss(now);
> +
> }
>
> static struct clocksource xen_clocksource __read_mostly = {
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 0:52 ` John Stultz
@ 2013-05-14 7:57 ` Jan Beulich
2013-05-14 7:57 ` [Xen-devel] " Jan Beulich
` (2 subsequent siblings)
3 siblings, 0 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-14 7:57 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Thomas Gleixner, Jeremy Fitzhardinge, linux-kernel,
Konrad Rzeszutek Wilk, xen-devel
>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
> Honestly, it seems a little strange to me. If you're running as dom0,
> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
> its virtualizing? This seems to mess with the proper virtualization
> layering.
Thy hypervisor tries to control as little system and peripheral devices
as possible, and the CMOS (including the clock) is among those not
controlled by it, but by Dom0.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 0:52 ` John Stultz
2013-05-14 7:57 ` Jan Beulich
@ 2013-05-14 7:57 ` Jan Beulich
2013-05-14 15:59 ` John Stultz
2013-05-14 15:59 ` [Xen-devel] " John Stultz
2013-05-14 9:55 ` David Vrabel
2013-05-14 9:55 ` David Vrabel
3 siblings, 2 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-14 7:57 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Jeremy Fitzhardinge, Thomas Gleixner, xen-devel,
Konrad Rzeszutek Wilk, linux-kernel
>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
> Honestly, it seems a little strange to me. If you're running as dom0,
> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
> its virtualizing? This seems to mess with the proper virtualization
> layering.
Thy hypervisor tries to control as little system and peripheral devices
as possible, and the CMOS (including the clock) is among those not
controlled by it, but by Dom0.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 7:57 ` [Xen-devel] " Jan Beulich
@ 2013-05-14 15:59 ` John Stultz
2013-05-14 15:59 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 15:59 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk, linux-kernel,
xen-devel, David Vrabel, Thomas Gleixner
On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>> Honestly, it seems a little strange to me. If you're running as dom0,
>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>> its virtualizing? This seems to mess with the proper virtualization
>> layering.
> Thy hypervisor tries to control as little system and peripheral devices
> as possible, and the CMOS (including the clock) is among those not
> controlled by it, but by Dom0.
Huh. So what does calling HYPERVISOR_dom0_op do then?
Either way, I don't have the context to insightfully review this, so
I'll want this to get some Acked-bys from additional Xen folks before I
queue it.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 7:57 ` [Xen-devel] " Jan Beulich
2013-05-14 15:59 ` John Stultz
@ 2013-05-14 15:59 ` John Stultz
2013-05-14 16:14 ` Jan Beulich
2013-05-14 16:14 ` Jan Beulich
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 15:59 UTC (permalink / raw)
To: Jan Beulich
Cc: David Vrabel, Jeremy Fitzhardinge, Thomas Gleixner, xen-devel,
Konrad Rzeszutek Wilk, linux-kernel
On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>> Honestly, it seems a little strange to me. If you're running as dom0,
>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>> its virtualizing? This seems to mess with the proper virtualization
>> layering.
> Thy hypervisor tries to control as little system and peripheral devices
> as possible, and the CMOS (including the clock) is among those not
> controlled by it, but by Dom0.
Huh. So what does calling HYPERVISOR_dom0_op do then?
Either way, I don't have the context to insightfully review this, so
I'll want this to get some Acked-bys from additional Xen folks before I
queue it.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 15:59 ` [Xen-devel] " John Stultz
@ 2013-05-14 16:14 ` Jan Beulich
2013-05-14 16:17 ` John Stultz
2013-05-14 16:17 ` John Stultz
2013-05-14 16:14 ` Jan Beulich
1 sibling, 2 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-14 16:14 UTC (permalink / raw)
To: John Stultz
Cc: David Vrabel, Jeremy Fitzhardinge, Thomas Gleixner, xen-devel,
Konrad Rzeszutek Wilk, linux-kernel
>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>>> Honestly, it seems a little strange to me. If you're running as dom0,
>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>>> its virtualizing? This seems to mess with the proper virtualization
>>> layering.
>> Thy hypervisor tries to control as little system and peripheral devices
>> as possible, and the CMOS (including the clock) is among those not
>> controlled by it, but by Dom0.
>
> Huh. So what does calling HYPERVISOR_dom0_op do then?
Here it merely tells the hypervisor that the wall clock changed (so
it can propagate this on to DomU-s).
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:14 ` Jan Beulich
@ 2013-05-14 16:17 ` John Stultz
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
2013-05-14 16:17 ` John Stultz
1 sibling, 2 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 16:17 UTC (permalink / raw)
To: Jan Beulich
Cc: David Vrabel, Jeremy Fitzhardinge, Thomas Gleixner, xen-devel,
Konrad Rzeszutek Wilk, linux-kernel
On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>>>> Honestly, it seems a little strange to me. If you're running as dom0,
>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>>>> its virtualizing? This seems to mess with the proper virtualization
>>>> layering.
>>> Thy hypervisor tries to control as little system and peripheral devices
>>> as possible, and the CMOS (including the clock) is among those not
>>> controlled by it, but by Dom0.
>> Huh. So what does calling HYPERVISOR_dom0_op do then?
> Here it merely tells the hypervisor that the wall clock changed (so
> it can propagate this on to DomU-s).
Ok, I appreciate the explanation. Still waiting for acks on this one.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:17 ` John Stultz
@ 2013-05-14 16:24 ` Konrad Rzeszutek Wilk
2013-05-14 16:28 ` John Stultz
2013-05-14 16:28 ` [Xen-devel] " John Stultz
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
1 sibling, 2 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-14 16:24 UTC (permalink / raw)
To: John Stultz, Jan Beulich
Cc: David Vrabel, Jeremy Fitzhardinge, Thomas Gleixner, xen-devel,
linux-kernel
John Stultz <john.stultz@linaro.org> wrote:
>On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org>
>wrote:
>>>>> Honestly, it seems a little strange to me. If you're running as
>dom0,
>>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the
>cmos
>>>>> its virtualizing? This seems to mess with the proper
>virtualization
>>>>> layering.
>>>> Thy hypervisor tries to control as little system and peripheral
>devices
>>>> as possible, and the CMOS (including the clock) is among those not
>>>> controlled by it, but by Dom0.
>>> Huh. So what does calling HYPERVISOR_dom0_op do then?
>> Here it merely tells the hypervisor that the wall clock changed (so
>> it can propagate this on to DomU-s).
>
>Ok, I appreciate the explanation. Still waiting for acks on this one.
>
>thanks
>-john
Acked-by: Konrad Rzeszutek Wilk
Or Reviewed-by. Or John if you would like I can carry this in my branch for Linus.
Thought it might make sense to add the comment from Jan about the CMOS and the follow up explanation.
--
Sent from my Android phone. Please excuse my brevity.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
@ 2013-05-14 16:28 ` John Stultz
2013-05-14 16:28 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 16:28 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge, linux-kernel, xen-devel, David Vrabel,
Jan Beulich, Thomas Gleixner
On 05/14/2013 09:24 AM, Konrad Rzeszutek Wilk wrote:
> John Stultz <john.stultz@linaro.org> wrote:
>
>> On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>>>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org>
>> wrote:
>>>>>> Honestly, it seems a little strange to me. If you're running as
>> dom0,
>>>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the
>> cmos
>>>>>> its virtualizing? This seems to mess with the proper
>> virtualization
>>>>>> layering.
>>>>> Thy hypervisor tries to control as little system and peripheral
>> devices
>>>>> as possible, and the CMOS (including the clock) is among those not
>>>>> controlled by it, but by Dom0.
>>>> Huh. So what does calling HYPERVISOR_dom0_op do then?
>>> Here it merely tells the hypervisor that the wall clock changed (so
>>> it can propagate this on to DomU-s).
>> Ok, I appreciate the explanation. Still waiting for acks on this one.
>>
>> thanks
>> -john
> Acked-by: Konrad Rzeszutek Wilk
>
> Or Reviewed-by. Or John if you would like I can carry this in my branch for Linus.
>
> Thought it might make sense to add the comment from Jan about the CMOS and the follow up explanation.
I'm fine queuing it, but yea, the commit will need some more context.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
2013-05-14 16:28 ` John Stultz
@ 2013-05-14 16:28 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 16:28 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jan Beulich, David Vrabel, Jeremy Fitzhardinge, Thomas Gleixner,
xen-devel, linux-kernel
On 05/14/2013 09:24 AM, Konrad Rzeszutek Wilk wrote:
> John Stultz <john.stultz@linaro.org> wrote:
>
>> On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>>>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org>
>> wrote:
>>>>>> Honestly, it seems a little strange to me. If you're running as
>> dom0,
>>>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the
>> cmos
>>>>>> its virtualizing? This seems to mess with the proper
>> virtualization
>>>>>> layering.
>>>>> Thy hypervisor tries to control as little system and peripheral
>> devices
>>>>> as possible, and the CMOS (including the clock) is among those not
>>>>> controlled by it, but by Dom0.
>>>> Huh. So what does calling HYPERVISOR_dom0_op do then?
>>> Here it merely tells the hypervisor that the wall clock changed (so
>>> it can propagate this on to DomU-s).
>> Ok, I appreciate the explanation. Still waiting for acks on this one.
>>
>> thanks
>> -john
> Acked-by: Konrad Rzeszutek Wilk
>
> Or Reviewed-by. Or John if you would like I can carry this in my branch for Linus.
>
> Thought it might make sense to add the comment from Jan about the CMOS and the follow up explanation.
I'm fine queuing it, but yea, the commit will need some more context.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:17 ` John Stultz
2013-05-14 16:24 ` Konrad Rzeszutek Wilk
@ 2013-05-14 16:24 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 69+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-14 16:24 UTC (permalink / raw)
To: John Stultz, Jan Beulich
Cc: linux-kernel, Thomas Gleixner, Jeremy Fitzhardinge, David Vrabel,
xen-devel
John Stultz <john.stultz@linaro.org> wrote:
>On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org>
>wrote:
>>>>> Honestly, it seems a little strange to me. If you're running as
>dom0,
>>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the
>cmos
>>>>> its virtualizing? This seems to mess with the proper
>virtualization
>>>>> layering.
>>>> Thy hypervisor tries to control as little system and peripheral
>devices
>>>> as possible, and the CMOS (including the clock) is among those not
>>>> controlled by it, but by Dom0.
>>> Huh. So what does calling HYPERVISOR_dom0_op do then?
>> Here it merely tells the hypervisor that the wall clock changed (so
>> it can propagate this on to DomU-s).
>
>Ok, I appreciate the explanation. Still waiting for acks on this one.
>
>thanks
>-john
Acked-by: Konrad Rzeszutek Wilk
Or Reviewed-by. Or John if you would like I can carry this in my branch for Linus.
Thought it might make sense to add the comment from Jan about the CMOS and the follow up explanation.
--
Sent from my Android phone. Please excuse my brevity.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 16:14 ` Jan Beulich
2013-05-14 16:17 ` John Stultz
@ 2013-05-14 16:17 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 16:17 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk, linux-kernel,
xen-devel, David Vrabel, Thomas Gleixner
On 05/14/2013 09:14 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>>>> Honestly, it seems a little strange to me. If you're running as dom0,
>>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>>>> its virtualizing? This seems to mess with the proper virtualization
>>>> layering.
>>> Thy hypervisor tries to control as little system and peripheral devices
>>> as possible, and the CMOS (including the clock) is among those not
>>> controlled by it, but by Dom0.
>> Huh. So what does calling HYPERVISOR_dom0_op do then?
> Here it merely tells the hypervisor that the wall clock changed (so
> it can propagate this on to DomU-s).
Ok, I appreciate the explanation. Still waiting for acks on this one.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 15:59 ` [Xen-devel] " John Stultz
2013-05-14 16:14 ` Jan Beulich
@ 2013-05-14 16:14 ` Jan Beulich
1 sibling, 0 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-14 16:14 UTC (permalink / raw)
To: John Stultz
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk, linux-kernel,
xen-devel, David Vrabel, Thomas Gleixner
>>> On 14.05.13 at 17:59, John Stultz <john.stultz@linaro.org> wrote:
> On 05/14/2013 12:57 AM, Jan Beulich wrote:
>>>>> On 14.05.13 at 02:52, John Stultz <john.stultz@linaro.org> wrote:
>>> Honestly, it seems a little strange to me. If you're running as dom0,
>>> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
>>> its virtualizing? This seems to mess with the proper virtualization
>>> layering.
>> Thy hypervisor tries to control as little system and peripheral devices
>> as possible, and the CMOS (including the clock) is among those not
>> controlled by it, but by Dom0.
>
> Huh. So what does calling HYPERVISOR_dom0_op do then?
Here it merely tells the hypervisor that the wall clock changed (so
it can propagate this on to DomU-s).
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 0:52 ` John Stultz
2013-05-14 7:57 ` Jan Beulich
2013-05-14 7:57 ` [Xen-devel] " Jan Beulich
@ 2013-05-14 9:55 ` David Vrabel
2013-05-14 9:55 ` David Vrabel
3 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 9:55 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel,
Jeremy Fitzhardinge, xen-devel
On 14/05/13 01:52, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>
> Sorry again, not getting this one either.
>
> So normally in this case we're using the Xen wallclock as the underlying
> source for the persistent_clock here, my understanding is we use this
> instead of the standard cmos, because we get benefits of using the
> hypervisor's sense of time instead of the bare hardware, and allows for
> virtualization of the persistent clock.
>
> But the problem is that even if Dom0 tries to set the xen persistent
> clock, it doesn't actually update anything in the underlying hardware?
> So here you instead try to sync the underlying hardware cmos from the
> same Xen dom0 environment?
Yes.
> Honestly, it seems a little strange to me. If you're running as dom0,
> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
> its virtualizing? This seems to mess with the proper virtualization
> layering.
As Jan says the hypervisor only drives a minimal set of hardware,
everything else is made accessible to dom0 for it to control.
I think this makes sense as it allows us to reuse the existing RTC
drivers etc. in the Linux kernel, instead of having to reimplement them
in the hypervisor.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 0:52 ` John Stultz
` (2 preceding siblings ...)
2013-05-14 9:55 ` David Vrabel
@ 2013-05-14 9:55 ` David Vrabel
3 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 9:55 UTC (permalink / raw)
To: John Stultz
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel,
Jeremy Fitzhardinge
On 14/05/13 01:52, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>
> Sorry again, not getting this one either.
>
> So normally in this case we're using the Xen wallclock as the underlying
> source for the persistent_clock here, my understanding is we use this
> instead of the standard cmos, because we get benefits of using the
> hypervisor's sense of time instead of the bare hardware, and allows for
> virtualization of the persistent clock.
>
> But the problem is that even if Dom0 tries to set the xen persistent
> clock, it doesn't actually update anything in the underlying hardware?
> So here you instead try to sync the underlying hardware cmos from the
> same Xen dom0 environment?
Yes.
> Honestly, it seems a little strange to me. If you're running as dom0,
> why does HYPERVISOR_dom0_op() not cause the hypervisor to set the cmos
> its virtualizing? This seems to mess with the proper virtualization
> layering.
As Jan says the hypervisor only drives a minimal set of hardware,
everything else is made accessible to dom0 for it to control.
I think this makes sense as it allows us to reuse the existing RTC
drivers etc. in the Linux kernel, instead of having to reimplement them
in the hypervisor.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 ` David Vrabel
2013-05-14 0:52 ` John Stultz
2013-05-14 0:52 ` John Stultz
@ 2013-05-14 17:24 ` John Stultz
2013-05-14 17:24 ` John Stultz
3 siblings, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:24 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If NTP is used in dom0 and it is synchronized to its clock source,
> then the kernel will periodically synchronize the Xen wallclock with
> the system time. Updates to the Xen wallclock do not persist across
> reboots, so also synchronize the CMOS RTC (as on bare metal).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/xen/time.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a1947ac..4656165 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -14,6 +14,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/math64.h>
> #include <linux/gfp.h>
> +#include <linux/mc146818rtc.h>
>
> #include <asm/pvclock.h>
> #include <asm/xen/hypervisor.h>
> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
> static int xen_set_wallclock(const struct timespec *now)
> {
> struct xen_platform_op op;
> + int ret;
>
> /* do nothing for domU */
> if (!xen_initial_domain())
> return -1;
>
> + /* Set the Xen wallclock. */
> op.cmd = XENPF_settime;
> op.u.settime.secs = now->tv_sec;
> op.u.settime.nsecs = now->tv_nsec;
> op.u.settime.system_time = xen_clocksource_read();
>
> - return HYPERVISOR_dom0_op(&op);
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /* Set the hardware RTC. */
> + return mach_set_rtc_mmss(now);
Sorry, just noticed one more thing while applying this. Do all Xen
systems run on hardware that has the conventional CMOS clock?
What happens if the bare-metal needs to use efi_set_rtc() or
vrtc_set_mmss() ?
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-13 17:56 ` David Vrabel
` (2 preceding siblings ...)
2013-05-14 17:24 ` John Stultz
@ 2013-05-14 17:24 ` John Stultz
2013-05-14 18:00 ` David Vrabel
` (3 more replies)
3 siblings, 4 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 17:24 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If NTP is used in dom0 and it is synchronized to its clock source,
> then the kernel will periodically synchronize the Xen wallclock with
> the system time. Updates to the Xen wallclock do not persist across
> reboots, so also synchronize the CMOS RTC (as on bare metal).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/xen/time.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a1947ac..4656165 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -14,6 +14,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/math64.h>
> #include <linux/gfp.h>
> +#include <linux/mc146818rtc.h>
>
> #include <asm/pvclock.h>
> #include <asm/xen/hypervisor.h>
> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
> static int xen_set_wallclock(const struct timespec *now)
> {
> struct xen_platform_op op;
> + int ret;
>
> /* do nothing for domU */
> if (!xen_initial_domain())
> return -1;
>
> + /* Set the Xen wallclock. */
> op.cmd = XENPF_settime;
> op.u.settime.secs = now->tv_sec;
> op.u.settime.nsecs = now->tv_nsec;
> op.u.settime.system_time = xen_clocksource_read();
>
> - return HYPERVISOR_dom0_op(&op);
> + ret = HYPERVISOR_dom0_op(&op);
> + if (ret)
> + return ret;
> +
> + /* Set the hardware RTC. */
> + return mach_set_rtc_mmss(now);
Sorry, just noticed one more thing while applying this. Do all Xen
systems run on hardware that has the conventional CMOS clock?
What happens if the bare-metal needs to use efi_set_rtc() or
vrtc_set_mmss() ?
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 17:24 ` John Stultz
@ 2013-05-14 18:00 ` David Vrabel
2013-05-14 18:00 ` David Vrabel
` (2 subsequent siblings)
3 siblings, 0 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 18:00 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 14/05/13 18:24, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/xen/time.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>> index a1947ac..4656165 100644
>> --- a/arch/x86/xen/time.c
>> +++ b/arch/x86/xen/time.c
>> @@ -14,6 +14,7 @@
>> #include <linux/kernel_stat.h>
>> #include <linux/math64.h>
>> #include <linux/gfp.h>
>> +#include <linux/mc146818rtc.h>
>>
>> #include <asm/pvclock.h>
>> #include <asm/xen/hypervisor.h>
>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>> static int xen_set_wallclock(const struct timespec *now)
>> {
>> struct xen_platform_op op;
>> + int ret;
>>
>> /* do nothing for domU */
>> if (!xen_initial_domain())
>> return -1;
>>
>> + /* Set the Xen wallclock. */
>> op.cmd = XENPF_settime;
>> op.u.settime.secs = now->tv_sec;
>> op.u.settime.nsecs = now->tv_nsec;
>> op.u.settime.system_time = xen_clocksource_read();
>>
>> - return HYPERVISOR_dom0_op(&op);
>> + ret = HYPERVISOR_dom0_op(&op);
>> + if (ret)
>> + return ret;
>> +
>> + /* Set the hardware RTC. */
>> + return mach_set_rtc_mmss(now);
>
> Sorry, just noticed one more thing while applying this. Do all Xen
> systems run on hardware that has the conventional CMOS clock?
>
> What happens if the bare-metal needs to use efi_set_rtc() or
> vrtc_set_mmss() ?
Ug. I hadn't considered that. I'll have think some more on this.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 17:24 ` John Stultz
2013-05-14 18:00 ` David Vrabel
@ 2013-05-14 18:00 ` David Vrabel
2013-05-14 18:03 ` John Stultz
2013-05-14 18:03 ` John Stultz
2013-05-15 8:19 ` [Xen-devel] " Jan Beulich
2013-05-15 8:19 ` Jan Beulich
3 siblings, 2 replies; 69+ messages in thread
From: David Vrabel @ 2013-05-14 18:00 UTC (permalink / raw)
To: John Stultz
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 14/05/13 18:24, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/xen/time.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>> index a1947ac..4656165 100644
>> --- a/arch/x86/xen/time.c
>> +++ b/arch/x86/xen/time.c
>> @@ -14,6 +14,7 @@
>> #include <linux/kernel_stat.h>
>> #include <linux/math64.h>
>> #include <linux/gfp.h>
>> +#include <linux/mc146818rtc.h>
>>
>> #include <asm/pvclock.h>
>> #include <asm/xen/hypervisor.h>
>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>> static int xen_set_wallclock(const struct timespec *now)
>> {
>> struct xen_platform_op op;
>> + int ret;
>>
>> /* do nothing for domU */
>> if (!xen_initial_domain())
>> return -1;
>>
>> + /* Set the Xen wallclock. */
>> op.cmd = XENPF_settime;
>> op.u.settime.secs = now->tv_sec;
>> op.u.settime.nsecs = now->tv_nsec;
>> op.u.settime.system_time = xen_clocksource_read();
>>
>> - return HYPERVISOR_dom0_op(&op);
>> + ret = HYPERVISOR_dom0_op(&op);
>> + if (ret)
>> + return ret;
>> +
>> + /* Set the hardware RTC. */
>> + return mach_set_rtc_mmss(now);
>
> Sorry, just noticed one more thing while applying this. Do all Xen
> systems run on hardware that has the conventional CMOS clock?
>
> What happens if the bare-metal needs to use efi_set_rtc() or
> vrtc_set_mmss() ?
Ug. I hadn't considered that. I'll have think some more on this.
David
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 18:00 ` David Vrabel
@ 2013-05-14 18:03 ` John Stultz
2013-05-14 18:03 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 18:03 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, Konrad Rzeszutek Wilk, Thomas Gleixner, linux-kernel
On 05/14/2013 11:00 AM, David Vrabel wrote:
> On 14/05/13 18:24, John Stultz wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> If NTP is used in dom0 and it is synchronized to its clock source,
>>> then the kernel will periodically synchronize the Xen wallclock with
>>> the system time. Updates to the Xen wallclock do not persist across
>>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> ---
>>> arch/x86/xen/time.c | 11 ++++++++++-
>>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>>> index a1947ac..4656165 100644
>>> --- a/arch/x86/xen/time.c
>>> +++ b/arch/x86/xen/time.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/kernel_stat.h>
>>> #include <linux/math64.h>
>>> #include <linux/gfp.h>
>>> +#include <linux/mc146818rtc.h>
>>>
>>> #include <asm/pvclock.h>
>>> #include <asm/xen/hypervisor.h>
>>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>>> static int xen_set_wallclock(const struct timespec *now)
>>> {
>>> struct xen_platform_op op;
>>> + int ret;
>>>
>>> /* do nothing for domU */
>>> if (!xen_initial_domain())
>>> return -1;
>>>
>>> + /* Set the Xen wallclock. */
>>> op.cmd = XENPF_settime;
>>> op.u.settime.secs = now->tv_sec;
>>> op.u.settime.nsecs = now->tv_nsec;
>>> op.u.settime.system_time = xen_clocksource_read();
>>>
>>> - return HYPERVISOR_dom0_op(&op);
>>> + ret = HYPERVISOR_dom0_op(&op);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Set the hardware RTC. */
>>> + return mach_set_rtc_mmss(now);
>> Sorry, just noticed one more thing while applying this. Do all Xen
>> systems run on hardware that has the conventional CMOS clock?
>>
>> What happens if the bare-metal needs to use efi_set_rtc() or
>> vrtc_set_mmss() ?
> Ug. I hadn't considered that. I'll have think some more on this.
Ok. I'll look forward to a future revision then and not queue this one.
Just FYI, here's what I rewrote the commit message to:
x86/xen: Sync the CMOS RTC as well as the Xen wallclock
Adjustments to Xen's persistent_clock via update_persistent_clock()
don't actually persist, as the xen_set_walltime() just notifies
other domN guests that it has been updated, and does not modify
the underlying CMOS clock.
Thus, this patch modifies xen_set_wallclock() so it will set
the underlying CMOS clock when called from dom0, ensuring the
persistent_clock will be correct on the next hardware boot.
I feel its a bit more clear, but feel free to tweak it as you see fit.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 18:00 ` David Vrabel
2013-05-14 18:03 ` John Stultz
@ 2013-05-14 18:03 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-14 18:03 UTC (permalink / raw)
To: David Vrabel
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
On 05/14/2013 11:00 AM, David Vrabel wrote:
> On 14/05/13 18:24, John Stultz wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> If NTP is used in dom0 and it is synchronized to its clock source,
>>> then the kernel will periodically synchronize the Xen wallclock with
>>> the system time. Updates to the Xen wallclock do not persist across
>>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> ---
>>> arch/x86/xen/time.c | 11 ++++++++++-
>>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>>> index a1947ac..4656165 100644
>>> --- a/arch/x86/xen/time.c
>>> +++ b/arch/x86/xen/time.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/kernel_stat.h>
>>> #include <linux/math64.h>
>>> #include <linux/gfp.h>
>>> +#include <linux/mc146818rtc.h>
>>>
>>> #include <asm/pvclock.h>
>>> #include <asm/xen/hypervisor.h>
>>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>>> static int xen_set_wallclock(const struct timespec *now)
>>> {
>>> struct xen_platform_op op;
>>> + int ret;
>>>
>>> /* do nothing for domU */
>>> if (!xen_initial_domain())
>>> return -1;
>>>
>>> + /* Set the Xen wallclock. */
>>> op.cmd = XENPF_settime;
>>> op.u.settime.secs = now->tv_sec;
>>> op.u.settime.nsecs = now->tv_nsec;
>>> op.u.settime.system_time = xen_clocksource_read();
>>>
>>> - return HYPERVISOR_dom0_op(&op);
>>> + ret = HYPERVISOR_dom0_op(&op);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Set the hardware RTC. */
>>> + return mach_set_rtc_mmss(now);
>> Sorry, just noticed one more thing while applying this. Do all Xen
>> systems run on hardware that has the conventional CMOS clock?
>>
>> What happens if the bare-metal needs to use efi_set_rtc() or
>> vrtc_set_mmss() ?
> Ug. I hadn't considered that. I'll have think some more on this.
Ok. I'll look forward to a future revision then and not queue this one.
Just FYI, here's what I rewrote the commit message to:
x86/xen: Sync the CMOS RTC as well as the Xen wallclock
Adjustments to Xen's persistent_clock via update_persistent_clock()
don't actually persist, as the xen_set_walltime() just notifies
other domN guests that it has been updated, and does not modify
the underlying CMOS clock.
Thus, this patch modifies xen_set_wallclock() so it will set
the underlying CMOS clock when called from dom0, ensuring the
persistent_clock will be correct on the next hardware boot.
I feel its a bit more clear, but feel free to tweak it as you see fit.
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 17:24 ` John Stultz
2013-05-14 18:00 ` David Vrabel
2013-05-14 18:00 ` David Vrabel
@ 2013-05-15 8:19 ` Jan Beulich
2013-05-15 18:13 ` John Stultz
2013-05-15 18:13 ` [Xen-devel] " John Stultz
2013-05-15 8:19 ` Jan Beulich
3 siblings, 2 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-15 8:19 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk, linux-kernel
>>> On 14.05.13 at 19:24, John Stultz <john.stultz@linaro.org> wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/xen/time.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>> index a1947ac..4656165 100644
>> --- a/arch/x86/xen/time.c
>> +++ b/arch/x86/xen/time.c
>> @@ -14,6 +14,7 @@
>> #include <linux/kernel_stat.h>
>> #include <linux/math64.h>
>> #include <linux/gfp.h>
>> +#include <linux/mc146818rtc.h>
>>
>> #include <asm/pvclock.h>
>> #include <asm/xen/hypervisor.h>
>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>> static int xen_set_wallclock(const struct timespec *now)
>> {
>> struct xen_platform_op op;
>> + int ret;
>>
>> /* do nothing for domU */
>> if (!xen_initial_domain())
>> return -1;
>>
>> + /* Set the Xen wallclock. */
>> op.cmd = XENPF_settime;
>> op.u.settime.secs = now->tv_sec;
>> op.u.settime.nsecs = now->tv_nsec;
>> op.u.settime.system_time = xen_clocksource_read();
>>
>> - return HYPERVISOR_dom0_op(&op);
>> + ret = HYPERVISOR_dom0_op(&op);
>> + if (ret)
>> + return ret;
>> +
>> + /* Set the hardware RTC. */
>> + return mach_set_rtc_mmss(now);
>
> Sorry, just noticed one more thing while applying this. Do all Xen
> systems run on hardware that has the conventional CMOS clock?
>
> What happens if the bare-metal needs to use efi_set_rtc() or
> vrtc_set_mmss() ?
There's no EFI support in the upstream kernel yet when running
on Xen (the code that's in the kernel can't be used, as Dom0
can't directly invoke EFI runtime service functions).
And I don't think Xen is in any way prepared to run on
Moorestown, which afaict is where vrtc_set_mmss() comes into
the picture.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-15 8:19 ` [Xen-devel] " Jan Beulich
@ 2013-05-15 18:13 ` John Stultz
2013-05-15 18:13 ` [Xen-devel] " John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-15 18:13 UTC (permalink / raw)
To: Jan Beulich
Cc: linux-kernel, Thomas Gleixner, Konrad Rzeszutek Wilk,
David Vrabel, xen-devel
On 05/15/2013 01:19 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 19:24, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> If NTP is used in dom0 and it is synchronized to its clock source,
>>> then the kernel will periodically synchronize the Xen wallclock with
>>> the system time. Updates to the Xen wallclock do not persist across
>>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> ---
>>> arch/x86/xen/time.c | 11 ++++++++++-
>>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>>> index a1947ac..4656165 100644
>>> --- a/arch/x86/xen/time.c
>>> +++ b/arch/x86/xen/time.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/kernel_stat.h>
>>> #include <linux/math64.h>
>>> #include <linux/gfp.h>
>>> +#include <linux/mc146818rtc.h>
>>>
>>> #include <asm/pvclock.h>
>>> #include <asm/xen/hypervisor.h>
>>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>>> static int xen_set_wallclock(const struct timespec *now)
>>> {
>>> struct xen_platform_op op;
>>> + int ret;
>>>
>>> /* do nothing for domU */
>>> if (!xen_initial_domain())
>>> return -1;
>>>
>>> + /* Set the Xen wallclock. */
>>> op.cmd = XENPF_settime;
>>> op.u.settime.secs = now->tv_sec;
>>> op.u.settime.nsecs = now->tv_nsec;
>>> op.u.settime.system_time = xen_clocksource_read();
>>>
>>> - return HYPERVISOR_dom0_op(&op);
>>> + ret = HYPERVISOR_dom0_op(&op);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Set the hardware RTC. */
>>> + return mach_set_rtc_mmss(now);
>> Sorry, just noticed one more thing while applying this. Do all Xen
>> systems run on hardware that has the conventional CMOS clock?
>>
>> What happens if the bare-metal needs to use efi_set_rtc() or
>> vrtc_set_mmss() ?
> There's no EFI support in the upstream kernel yet when running
> on Xen (the code that's in the kernel can't be used, as Dom0
> can't directly invoke EFI runtime service functions).
>
> And I don't think Xen is in any way prepared to run on
> Moorestown, which afaict is where vrtc_set_mmss() comes into
> the picture.
Ok.
David: Would you resubmit this patch, adding this additional context in
a comment?
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-15 8:19 ` [Xen-devel] " Jan Beulich
2013-05-15 18:13 ` John Stultz
@ 2013-05-15 18:13 ` John Stultz
1 sibling, 0 replies; 69+ messages in thread
From: John Stultz @ 2013-05-15 18:13 UTC (permalink / raw)
To: Jan Beulich
Cc: David Vrabel, Thomas Gleixner, xen-devel, Konrad Rzeszutek Wilk,
linux-kernel
On 05/15/2013 01:19 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 19:24, John Stultz <john.stultz@linaro.org> wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> If NTP is used in dom0 and it is synchronized to its clock source,
>>> then the kernel will periodically synchronize the Xen wallclock with
>>> the system time. Updates to the Xen wallclock do not persist across
>>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> ---
>>> arch/x86/xen/time.c | 11 ++++++++++-
>>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>>> index a1947ac..4656165 100644
>>> --- a/arch/x86/xen/time.c
>>> +++ b/arch/x86/xen/time.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/kernel_stat.h>
>>> #include <linux/math64.h>
>>> #include <linux/gfp.h>
>>> +#include <linux/mc146818rtc.h>
>>>
>>> #include <asm/pvclock.h>
>>> #include <asm/xen/hypervisor.h>
>>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>>> static int xen_set_wallclock(const struct timespec *now)
>>> {
>>> struct xen_platform_op op;
>>> + int ret;
>>>
>>> /* do nothing for domU */
>>> if (!xen_initial_domain())
>>> return -1;
>>>
>>> + /* Set the Xen wallclock. */
>>> op.cmd = XENPF_settime;
>>> op.u.settime.secs = now->tv_sec;
>>> op.u.settime.nsecs = now->tv_nsec;
>>> op.u.settime.system_time = xen_clocksource_read();
>>>
>>> - return HYPERVISOR_dom0_op(&op);
>>> + ret = HYPERVISOR_dom0_op(&op);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Set the hardware RTC. */
>>> + return mach_set_rtc_mmss(now);
>> Sorry, just noticed one more thing while applying this. Do all Xen
>> systems run on hardware that has the conventional CMOS clock?
>>
>> What happens if the bare-metal needs to use efi_set_rtc() or
>> vrtc_set_mmss() ?
> There's no EFI support in the upstream kernel yet when running
> on Xen (the code that's in the kernel can't be used, as Dom0
> can't directly invoke EFI runtime service functions).
>
> And I don't think Xen is in any way prepared to run on
> Moorestown, which afaict is where vrtc_set_mmss() comes into
> the picture.
Ok.
David: Would you resubmit this patch, adding this additional context in
a comment?
thanks
-john
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock
2013-05-14 17:24 ` John Stultz
` (2 preceding siblings ...)
2013-05-15 8:19 ` [Xen-devel] " Jan Beulich
@ 2013-05-15 8:19 ` Jan Beulich
3 siblings, 0 replies; 69+ messages in thread
From: Jan Beulich @ 2013-05-15 8:19 UTC (permalink / raw)
To: David Vrabel, John Stultz
Cc: Thomas Gleixner, Konrad Rzeszutek Wilk, linux-kernel, xen-devel
>>> On 14.05.13 at 19:24, John Stultz <john.stultz@linaro.org> wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If NTP is used in dom0 and it is synchronized to its clock source,
>> then the kernel will periodically synchronize the Xen wallclock with
>> the system time. Updates to the Xen wallclock do not persist across
>> reboots, so also synchronize the CMOS RTC (as on bare metal).
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>> arch/x86/xen/time.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>> index a1947ac..4656165 100644
>> --- a/arch/x86/xen/time.c
>> +++ b/arch/x86/xen/time.c
>> @@ -14,6 +14,7 @@
>> #include <linux/kernel_stat.h>
>> #include <linux/math64.h>
>> #include <linux/gfp.h>
>> +#include <linux/mc146818rtc.h>
>>
>> #include <asm/pvclock.h>
>> #include <asm/xen/hypervisor.h>
>> @@ -199,17 +200,25 @@ static void xen_get_wallclock(struct timespec *now)
>> static int xen_set_wallclock(const struct timespec *now)
>> {
>> struct xen_platform_op op;
>> + int ret;
>>
>> /* do nothing for domU */
>> if (!xen_initial_domain())
>> return -1;
>>
>> + /* Set the Xen wallclock. */
>> op.cmd = XENPF_settime;
>> op.u.settime.secs = now->tv_sec;
>> op.u.settime.nsecs = now->tv_nsec;
>> op.u.settime.system_time = xen_clocksource_read();
>>
>> - return HYPERVISOR_dom0_op(&op);
>> + ret = HYPERVISOR_dom0_op(&op);
>> + if (ret)
>> + return ret;
>> +
>> + /* Set the hardware RTC. */
>> + return mach_set_rtc_mmss(now);
>
> Sorry, just noticed one more thing while applying this. Do all Xen
> systems run on hardware that has the conventional CMOS clock?
>
> What happens if the bare-metal needs to use efi_set_rtc() or
> vrtc_set_mmss() ?
There's no EFI support in the upstream kernel yet when running
on Xen (the code that's in the kernel can't be used, as Dom0
can't directly invoke EFI runtime service functions).
And I don't think Xen is in any way prepared to run on
Moorestown, which afaict is where vrtc_set_mmss() comes into
the picture.
Jan
^ permalink raw reply [flat|nested] 69+ messages in thread