| Message ID | a4ce05dc1da29722dc6292159a2f83811af84f6f.1567448405.git.edvin.torok@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Avoiding RDTSC emulation due to host clock drift | expand |
On 02.09.2019 20:27, Edwin Török wrote: > On a Intel(R) Xeon(R) CPU E5-2697 v3 @ 2.60GHz the host frequency drifts: > ``` > (XEN) [ 6.607693] Detected 2600.004 MHz processor. > (XEN) [ 2674.213081] dom1(hvm): mode=0,ofs=0xfffee6f70b7faa48,khz=2600018,inc=3 > (XEN) [ 2674.213087] dom2(hvm): mode=0,ofs=0xfffee6fd499835c0,khz=2600018,inc=2 > ``` > > The 2 domains were suspended prior to rebooting the host and applying a > xen/microcode patch. After the reboot the frequency of the host was deemed to > be slightly different, and therefore switching on RDTSC emulation for the Linux > HVM guest, even though the difference was only 5 ppm. This CPU doesn't support > TSC scaling. > > Therefore we should either measure the standard deviation of our calibration > and have a range of acceptable frequencies as "same", or have a static > tolerance value. > > The platform timer's clock frequency accuracy is: > * IA-PC HPET Specification 1.0a sections 2.2 and 2.4.1: 500 ppm or better > * ACPI PM timer, and PIT timer do not have defined accuracies > * Intel 300 Series datasheet section 25.6: 24 MHz crystal 100 ppm or better > * NTP FAQ section 3.3 Clock Quality: 11 ppm drift due to temperature > * section 2.2.2 claims that PIT/ACPI PM timer share the same crystal as HPET and > thus 500 ppm as an upper bound, "the real drift is usually smaller than 30ppm" > > For simplicity and determinism opted for a static tolerance value of 100 ppm > here, such that the any error would be well within the error you would get with > HPET/Linux's calibration. NTP can cope with a drift < 500 ppm. > Most importantly this should stop Xen from claiming that the clock frequency on > the same host is different across reboots. Specifications do not currently > mandate an accuracy higher than 100 ppm, therefore OSes should already be able > to cope with such drift on real hardware. Any improvements in accuracy from > future specifications/motherboards wouldn't be applicable, because they would > also come with newer CPUs that support TSC scaling. > > If the CPU does support TSC scaling Xen will of course still attempt to match > the exact frequency value it thinks the guest had when it was suspended. > See below for `if ( hvm_tsc_scaling_supported && !d->arch.vtsc )` (not visible > in patch context). > > llabs() doesn't appear to be available when building xen, hence the 2 comparisons. > > After this patch when suspending a VM, and rebooting the host I get this output: > ``` > (XEN) [ 6.614703] Detected 2600.010 MHz processor. > (XEN) [ 138.924342] TSC marked as reliable, warp = 0 (count=2) > (XEN) [ 138.924346] dom1(hvm): mode=0,ofs=0xfffed01901016d18,khz=2600012,inc=2 > ``` > > Signed-off-by: Edwin Török <edvin.torok@citrix.com> First of all - are you aware that there had been multiple iterations of a patch (by Olaf Hering) making this a command line and/or guest config controllable setting? If so, it would have been nice if at least the cover letter identified the correlation. If not, please take a look. After all it hasn't gone in so far because of objections by Andrew. Using a hardcoded tolerance value in any event raises the question of how you know whether a particular guest OS can actually cope. > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -2171,6 +2171,12 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode, > *elapsed_nsec = 0; > } > > +static inline int frequency_same_with_tolerance(int64_t khz1, int64_t khz2) The return type wants to be bool. And there wants to be an explaining comment ahead of the function, (re-)stating some of what you have in the description. > +{ > + int64_t ppm = (khz2 - khz1) * 1000000 / khz2; > + return -100 < ppm && ppm < 100; While we have no llabs(), you should imo use either ABS() or __builtin_labs() / __builtin_llabs(). Furthermore, could we limit this behavior to the case of there not being TSC scaling available (due to running on old hardware, or due to it being a PV guest)? Jan
On 03/09/2019 08:54, Jan Beulich wrote: > On 02.09.2019 20:27, Edwin Török wrote: >> On a Intel(R) Xeon(R) CPU E5-2697 v3 @ 2.60GHz the host frequency drifts: >> ``` >> (XEN) [ 6.607693] Detected 2600.004 MHz processor. >> (XEN) [ 2674.213081] dom1(hvm): mode=0,ofs=0xfffee6f70b7faa48,khz=2600018,inc=3 >> (XEN) [ 2674.213087] dom2(hvm): mode=0,ofs=0xfffee6fd499835c0,khz=2600018,inc=2 >> ``` >> >> The 2 domains were suspended prior to rebooting the host and applying a >> xen/microcode patch. After the reboot the frequency of the host was deemed to >> be slightly different, and therefore switching on RDTSC emulation for the Linux >> HVM guest, even though the difference was only 5 ppm. This CPU doesn't support >> TSC scaling. >> >> Therefore we should either measure the standard deviation of our calibration >> and have a range of acceptable frequencies as "same", or have a static >> tolerance value. >> >> The platform timer's clock frequency accuracy is: >> * IA-PC HPET Specification 1.0a sections 2.2 and 2.4.1: 500 ppm or better >> * ACPI PM timer, and PIT timer do not have defined accuracies >> * Intel 300 Series datasheet section 25.6: 24 MHz crystal 100 ppm or better >> * NTP FAQ section 3.3 Clock Quality: 11 ppm drift due to temperature >> * section 2.2.2 claims that PIT/ACPI PM timer share the same crystal as HPET and >> thus 500 ppm as an upper bound, "the real drift is usually smaller than 30ppm" >> >> For simplicity and determinism opted for a static tolerance value of 100 ppm >> here, such that the any error would be well within the error you would get with >> HPET/Linux's calibration. NTP can cope with a drift < 500 ppm. >> Most importantly this should stop Xen from claiming that the clock frequency on >> the same host is different across reboots. Specifications do not currently >> mandate an accuracy higher than 100 ppm, therefore OSes should already be able >> to cope with such drift on real hardware. Any improvements in accuracy from >> future specifications/motherboards wouldn't be applicable, because they would >> also come with newer CPUs that support TSC scaling. >> >> If the CPU does support TSC scaling Xen will of course still attempt to match >> the exact frequency value it thinks the guest had when it was suspended. >> See below for `if ( hvm_tsc_scaling_supported && !d->arch.vtsc )` (not visible >> in patch context). >> >> llabs() doesn't appear to be available when building xen, hence the 2 comparisons. >> >> After this patch when suspending a VM, and rebooting the host I get this output: >> ``` >> (XEN) [ 6.614703] Detected 2600.010 MHz processor. >> (XEN) [ 138.924342] TSC marked as reliable, warp = 0 (count=2) >> (XEN) [ 138.924346] dom1(hvm): mode=0,ofs=0xfffed01901016d18,khz=2600012,inc=2 >> ``` >> >> Signed-off-by: Edwin Török <edvin.torok@citrix.com> > > First of all - are you aware that there had been multiple iterations > of a patch (by Olaf Hering) making this a command line and/or guest > config controllable setting? No, I'll take a look at them and the associated discussion. Found a '[PATCH v10] new config option vtsc_tolerance_khz to avoid TSC emulation' in the archives from 9 months ago. > If so, it would have been nice if at > least the cover letter identified the correlation. If not, please > take a look. After all it hasn't gone in so far because of objections > by Andrew. > > Using a hardcoded tolerance value in any event raises the question > of how you know whether a particular guest OS can actually cope. > >> --- a/xen/arch/x86/time.c >> +++ b/xen/arch/x86/time.c >> @@ -2171,6 +2171,12 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode, >> *elapsed_nsec = 0; >> } >> >> +static inline int frequency_same_with_tolerance(int64_t khz1, int64_t khz2) > > The return type wants to be bool. And there wants to be an explaining > comment ahead of the function, (re-)stating some of what you have in > the description. > >> +{ >> + int64_t ppm = (khz2 - khz1) * 1000000 / khz2; >> + return -100 < ppm && ppm < 100; > > While we have no llabs(), you should imo use either ABS() or > __builtin_labs() / __builtin_llabs(). > > Furthermore, could we limit this behavior to the case of there not > being TSC scaling available (due to running on old hardware, or due > to it being a PV guest)? Yes, that'd make sense. Best regards, --Edwin
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 9a6ea8ffcb..a0b99f5fff 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -2171,6 +2171,12 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode, *elapsed_nsec = 0; } +static inline int frequency_same_with_tolerance(int64_t khz1, int64_t khz2) +{ + int64_t ppm = (khz2 - khz1) * 1000000 / khz2; + return -100 < ppm && ppm < 100; +} + /* * This may be called as many as three times for a domain, once when the * hypervisor creates the domain, once when the toolstack creates the @@ -2207,7 +2213,7 @@ int tsc_set_info(struct domain *d, * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation. */ if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() && - (d->arch.tsc_khz == cpu_khz || + (frequency_same_with_tolerance(d->arch.tsc_khz, cpu_khz) || (is_hvm_domain(d) && hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) ) {
On a Intel(R) Xeon(R) CPU E5-2697 v3 @ 2.60GHz the host frequency drifts: ``` (XEN) [ 6.607693] Detected 2600.004 MHz processor. (XEN) [ 2674.213081] dom1(hvm): mode=0,ofs=0xfffee6f70b7faa48,khz=2600018,inc=3 (XEN) [ 2674.213087] dom2(hvm): mode=0,ofs=0xfffee6fd499835c0,khz=2600018,inc=2 ``` The 2 domains were suspended prior to rebooting the host and applying a xen/microcode patch. After the reboot the frequency of the host was deemed to be slightly different, and therefore switching on RDTSC emulation for the Linux HVM guest, even though the difference was only 5 ppm. This CPU doesn't support TSC scaling. Therefore we should either measure the standard deviation of our calibration and have a range of acceptable frequencies as "same", or have a static tolerance value. The platform timer's clock frequency accuracy is: * IA-PC HPET Specification 1.0a sections 2.2 and 2.4.1: 500 ppm or better * ACPI PM timer, and PIT timer do not have defined accuracies * Intel 300 Series datasheet section 25.6: 24 MHz crystal 100 ppm or better * NTP FAQ section 3.3 Clock Quality: 11 ppm drift due to temperature * section 2.2.2 claims that PIT/ACPI PM timer share the same crystal as HPET and thus 500 ppm as an upper bound, "the real drift is usually smaller than 30ppm" For simplicity and determinism opted for a static tolerance value of 100 ppm here, such that the any error would be well within the error you would get with HPET/Linux's calibration. NTP can cope with a drift < 500 ppm. Most importantly this should stop Xen from claiming that the clock frequency on the same host is different across reboots. Specifications do not currently mandate an accuracy higher than 100 ppm, therefore OSes should already be able to cope with such drift on real hardware. Any improvements in accuracy from future specifications/motherboards wouldn't be applicable, because they would also come with newer CPUs that support TSC scaling. If the CPU does support TSC scaling Xen will of course still attempt to match the exact frequency value it thinks the guest had when it was suspended. See below for `if ( hvm_tsc_scaling_supported && !d->arch.vtsc )` (not visible in patch context). llabs() doesn't appear to be available when building xen, hence the 2 comparisons. After this patch when suspending a VM, and rebooting the host I get this output: ``` (XEN) [ 6.614703] Detected 2600.010 MHz processor. (XEN) [ 138.924342] TSC marked as reliable, warp = 0 (count=2) (XEN) [ 138.924346] dom1(hvm): mode=0,ofs=0xfffed01901016d18,khz=2600012,inc=2 ``` Signed-off-by: Edwin Török <edvin.torok@citrix.com> --- xen/arch/x86/time.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)