new config option vtsc_khz_tolerance to avoid TSC emulation
PATCH v2 1/1 new config option vtsc_khz_tolerance to avoid TSC emulation
Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.
Add a new domctl XEN_DOMCTL_set_vtsc_khz_tolerance to adjust the tolerance value of a running domU that is supposed to be migrated.
One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.
With this option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable.
The new option allows jitter up to 65535 KHz, which is more than enough for the intended use. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.
A new utility is added which allows to adjust the vtsc_khz_tolerance value for running domUs. This is useful to avoid emulation for domUs that are already running and which can not be restarted.
Existing padding fields are reused to store vtsc_khz_tolerance as u16.
In case the migration stream should get a new record type, the ordering of records sent during migration becomes important. The value of vtsc_khz_tolerance must be known by the receiving host before configuring TSC, because this is the place where the decision of vTSC emulation is made.
---
Followup to earlier attempt with global knob:
https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg01589.html
.gitignore | 1 + docs/man/xl.cfg.pod.5.in | 10 ++++ docs/specs/libxc-migration-stream.pandoc | 6 ++- tools/libxc/include/xenctrl.h | 6 +++ tools/libxc/xc_domain.c | 15 ++++++ tools/libxc/xc_sr_common_x86.c | 6 ++- tools/libxc/xc_sr_stream_format.h | 3 +- tools/libxl/libxl_types.idl | 1 + tools/libxl/libxl_x86.c | 3 +- tools/misc/Makefile | 4 ++ tools/misc/xen-vtsc.c | 92 ++++++++++++++++++++++++++++++++ tools/python/xen/lowlevel/xc/xc.c | 2 +- tools/xl/xl_parse.c | 3 ++ xen/arch/x86/domain.c | 2 +- xen/arch/x86/domctl.c | 11 ++++ xen/arch/x86/time.c | 29 ++++++++-- xen/include/asm-x86/domain.h | 1 + xen/include/asm-x86/time.h | 4 +- xen/include/public/domctl.h | 5 +- xen/xsm/flask/hooks.c | 1 + 20 files changed, 191 insertions(+), 14 deletions(-) create mode 100644 tools/misc/xen-vtsc.c
.gitignore +1 -0
@@ -221,6 +221,7 @@ tools/misc/xen-detect tools/misc/xen-diag tools/misc/xen-tmem-list-parse tools/misc/xen-livepatch +tools/misc/xen-vtsc tools/misc/xenperf tools/misc/xenpm tools/misc/xen-hvmctx
docs/man/xl.cfg.pod.5.in +10 -0
@@ -1891,6 +1891,16 @@ determined in a similar way to that of B<default> TSC mode. Please see B<xen-tscmode(7)> for more information on this option. +=item B<vtsc_khz_tolerance="KHZ"> + +B<(x86 only, relevant only for tsc_mode=default)> +When a domU is started, the CPU frequency of the host is used by the domU for +TSC related time measurement. Once the domU is either migrated or +saved/restored on another host that CPU frequency has to be emulated to avoid +timedrift. To avoid the performance penalty of the TSC emulation, allow a +certain amount of jitter of the measured CPU frequency on the hosts the domU +is supposed to run on. + =item B<localtime=BOOLEAN> Set the real time clock to local time or to UTC. False (0) by default,
docs/specs/libxc-migration-stream.pandoc +4 -2
@@ -3,7 +3,7 @@ Andrew Cooper <<[email protected]>> Wen Congyang <<[email protected]>> Yang Hongyang <<[email protected]>> -% Revision 2 +% Revision 3 Introduction ============
@@ -472,7 +472,7 @@ XEN\_DOMCTL\_{get,set}tscinfo hypercall sub-ops. +------------------------+------------------------+ | nsec | +------------------------+------------------------+ - | incarnation | (reserved) | + | incarnation | tolerance | (reserved) | +------------------------+------------------------+ --------------------------------------------------------------------
@@ -485,6 +485,8 @@ khz TSC frequency, in kHz. nsec Elapsed time, in nanoseconds. incarnation Incarnation. + +vtsc_tolerance Amount of Jitter the domU can handle after migration. -------------------------------------------------------------------- \clearpage
tools/libxc/include/xenctrl.h +6 -0
@@ -1355,11 +1355,16 @@ int xc_domain_set_time_offset(xc_interface *xch,uint32_t domid, int32_t time_offset_seconds); +int xc_domain_set_vtsc_khz_tolerance (xc_interface *xch, +uint32_t domid, +uint16_t vtsc_khz_tolerance); +int xc_domain_set_tsc_info (xc_interface *xch,uint32_t domid,uint32_t tsc_mode,uint64_t elapsed_nsec,uint32_t gtsc_khz, +uint16_t vtsc_khz_tolerance,uint32_t incarnation); int xc_domain_get_tsc_info(xc_interface *xch,
@@ -1367,6 +1372,7 @@ int xc_domain_get_tsc_info(xc_interface *xch,uint32_t *tsc_mode, uint64_t *elapsed_nsec, uint32_t *gtsc_khz, + uint16_t *vtsc_khz_tolerance, uint32_t *incarnation);int xc_domain_disable_migrate (xc_interface *xch,uint32_t domid);
tools/libxc/xc_domain.c +15 -0
@@ -847,11 +847,23 @@ int xc_domain_disable_migrate(xc_interface *xch, uint32_t domid)return do_domctl (xch, &domctl); } +int xc_domain_set_vtsc_khz_tolerance (xc_interface *xch, +uint32_t domid, +uint16_t vtsc_khz_tolerance) +{ +DECLARE_DOMCTL ; + domctl.cmd = XEN_DOMCTL_set_vtsc_khz_tolerance; + domctl.domain = domid; + domctl.u.tsc_info.vtsc_khz_tolerance = vtsc_khz_tolerance; +return do_domctl (xch, &domctl); +} +int xc_domain_set_tsc_info (xc_interface *xch,uint32_t domid,uint32_t tsc_mode,uint64_t elapsed_nsec,uint32_t gtsc_khz, +uint16_t vtsc_khz_tolerance,uint32_t incarnation) {DECLARE_DOMCTL ;
@@ -860,6 +872,7 @@ int xc_domain_set_tsc_info(xc_interface *xch, domctl.u.tsc_info.tsc_mode = tsc_mode; domctl.u.tsc_info.elapsed_nsec = elapsed_nsec; domctl.u.tsc_info.gtsc_khz = gtsc_khz; + domctl.u.tsc_info.vtsc_khz_tolerance = vtsc_khz_tolerance; domctl.u.tsc_info.incarnation = incarnation;return do_domctl (xch, &domctl); }
@@ -869,6 +882,7 @@ int xc_domain_get_tsc_info(xc_interface *xch,uint32_t *tsc_mode, uint64_t *elapsed_nsec, uint32_t *gtsc_khz, + uint16_t *vtsc_khz_tolerance, uint32_t *incarnation) { int rc;
@@ -882,6 +896,7 @@ int xc_domain_get_tsc_info(xc_interface *xch, *tsc_mode = domctl.u.tsc_info.tsc_mode; *elapsed_nsec = domctl.u.tsc_info.elapsed_nsec; *gtsc_khz = domctl.u.tsc_info.gtsc_khz; + *vtsc_khz_tolerance = domctl.u.tsc_info.vtsc_khz_tolerance; *incarnation = domctl.u.tsc_info.incarnation; }return rc;
tools/libxc/xc_sr_common_x86.c +4 -2
@@ -12,7 +12,8 @@ int write_tsc_info(struct xc_sr_context *ctx) };if (xc_domain_get_tsc_info (xch, ctx->domid, &tsc.mode, - &tsc.nsec, &tsc.khz, &tsc.incarnation) <0 ) + &tsc.nsec, &tsc.khz, &tsc.vtsc_tolerance, + &tsc.incarnation) <0 ) {PERROR ("Unable to obtain TSC information" );return -1 ;
@@ -34,7 +35,8 @@ int handle_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec) }if (xc_domain_set_tsc_info (xch, ctx->domid, tsc->mode, - tsc->nsec, tsc->khz, tsc->incarnation) ) + tsc->nsec, tsc->khz, tsc->vtsc_tolerance, + tsc->incarnation) ) {PERROR ("Unable to set TSC information" );return -1 ;
tools/libxc/xc_sr_stream_format.h +2 -1
@@ -121,7 +121,8 @@ struct xc_sr_rec_tsc_infouint32_t khz;uint64_t nsec;uint32_t incarnation; -uint32_t _res1; +uint16_t vtsc_tolerance; +uint16_t _res1; };/* HVM_PARAMS */
tools/libxl/libxl_types.idl +1 -0
@@ -466,6 +466,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("vcpu_soft_affinity", Array(libxl_bitmap, "num_vcpu_soft_affinity")), ("numa_placement", libxl_defbool), ("tsc_mode", libxl_tsc_mode), + ("vtsc_khz_tolerance", uint32), ("max_memkb", MemKB), ("target_memkb", MemKB), ("video_memkb", MemKB),
tools/libxl/libxl_x86.c +2 -1
@@ -317,7 +317,8 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,default :abort (); } -xc_domain_set_tsc_info (ctx->xch, domid, tsc_mode,0 ,0 ,0 ); +xc_domain_set_tsc_info (ctx->xch, domid, tsc_mode,0 ,0 , + d_config->b_info.vtsc_khz_tolerance,0 );if (libxl_defbool_val (d_config->b_info.disable_migrate))xc_domain_disable_migrate (ctx->xch, domid); rtc_timeoffset = d_config->b_info.rtc_timeoffset;
tools/misc/Makefile +4 -0
@@ -36,6 +36,7 @@ INSTALL_SBIN += $(INSTALL_SBIN-y) # Everything to be installed in a private bin/ INSTALL_PRIVBIN += xenpvnetboot +INSTALL_PRIVBIN += xen-vtsc # Everything to be installed TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
@@ -112,6 +113,9 @@ xen-livepatch: xen-livepatch.o xen-diag: xen-diag.o $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS) +xen-vtsc: xen-vtsc.o + $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS) + xen-lowmemd: xen-lowmemd.o $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
tools/misc/xen-vtsc.c new file +92 -0
92 lines not on this page · raw message
tools/python/xen/lowlevel/xc/xc.c +1 -1
@@ -1522,7 +1522,7 @@ static PyObject *pyxc_domain_set_tsc_info(XcObject *self, PyObject *args)if (!PyArg_ParseTuple (args,"ii" , &dom, &tsc_mode))return NULL ; -if (xc_domain_set_tsc_info (self->xc_handle, dom, tsc_mode,0 ,0 ,0 ) !=0 ) +if (xc_domain_set_tsc_info (self->xc_handle, dom, tsc_mode,0 ,0 ,0 ,0 ) !=0 )return pyxc_error_to_exception (self->xc_handle);Py_INCREF (zero);
tools/xl/xl_parse.c +3 -0
@@ -1126,6 +1126,9 @@ void parse_config_data(const char *config_source, } } +if (!xlu_cfg_get_long (config,"vtsc_khz_tolerance" , &l,0 )) + b_info->vtsc_khz_tolerance = l <0 || l >UINT16_MAX ?UINT16_MAX : l; +if (!xlu_cfg_get_long (config,"rtc_timeoffset" , &l,0 )) b_info->rtc_timeoffset = l;
xen/arch/x86/domain.c +1 -1
@@ -558,7 +558,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, }/* initialize default tsc behavior in case tools don't */ -tsc_set_info (d,TSC_MODE_DEFAULT ,0UL ,0 ,0 ); +tsc_set_info (d,TSC_MODE_DEFAULT ,0UL ,0 ,0 ,0 );spin_lock_init (&d->arch.vtsc_lock);/* PV/PVH guests get an emulated PIT too for video BIOSes to use. */
xen/arch/x86/domctl.c +11 -0
@@ -939,6 +939,7 @@ long arch_do_domctl(tsc_get_info (d, &domctl->u.tsc_info.tsc_mode, &domctl->u.tsc_info.elapsed_nsec, &domctl->u.tsc_info.gtsc_khz, + &domctl->u.tsc_info.vtsc_khz_tolerance, &domctl->u.tsc_info.incarnation);domain_unpause (d); copyback = true;
@@ -954,11 +955,21 @@ long arch_do_domctl(tsc_set_info (d, domctl->u.tsc_info.tsc_mode, domctl->u.tsc_info.elapsed_nsec, domctl->u.tsc_info.gtsc_khz, + domctl->u.tsc_info.vtsc_khz_tolerance, domctl->u.tsc_info.incarnation);domain_unpause (d); }break ; +case XEN_DOMCTL_set_vtsc_khz_tolerance: +if ( d == currd ) + ret = -EINVAL ; +else + { + d->arch.vtsc_khz_tolerance = domctl->u.tsc_info.vtsc_khz_tolerance; + } +break ; +case XEN_DOMCTL_suppress_spurious_page_faults: d->arch.suppress_spurious_page_faults =1 ;break ;
xen/arch/x86/time.c +26 -3
@@ -2064,13 +2064,14 @@ int host_tsc_is_safe(void)*/ void tsc_get_info (struct domain *d,uint32_t *tsc_mode,uint64_t *elapsed_nsec,uint32_t *gtsc_khz, -uint32_t *incarnation) +uint16_t *vtsc_khz_tolerance,uint32_t *incarnation) {bool enable_tsc_scaling =is_hvm_domain (d) && hvm_tsc_scaling_supported && !d->arch.vtsc; *incarnation = d->arch.incarnation; *tsc_mode = d->arch.tsc_mode; + *vtsc_khz_tolerance = d->arch.vtsc_khz_tolerance;switch ( *tsc_mode ) {
@@ -2122,7 +2123,8 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,*/ void tsc_set_info (struct domain *d,uint32_t tsc_mode,uint64_t elapsed_nsec, -uint32_t gtsc_khz,uint32_t incarnation) +uint32_t gtsc_khz,uint16_t vtsc_khz_tolerance, +uint32_t incarnation) {if (is_idle_domain (d) ||is_hardware_domain (d) ) {
@@ -2132,9 +2134,12 @@ void tsc_set_info(struct domain *d,switch ( d->arch.tsc_mode = tsc_mode ) { +bool diff_tolerated;bool enable_tsc_scaling;case TSC_MODE_DEFAULT : + d->arch.vtsc_khz_tolerance = vtsc_khz_tolerance; +/* Fallthrough. */ case TSC_MODE_ALWAYS_EMULATE : d->arch.vtsc_offset =get_s_time () - elapsed_nsec; d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
@@ -2147,8 +2152,24 @@ void tsc_set_info(struct domain *d,* When a guest is created, gtsc_khz is passed in as zero, making * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation. */ + diff_tolerated = d->arch.tsc_khz == cpu_khz; + +if ( tsc_mode ==TSC_MODE_DEFAULT && gtsc_khz ) { +uint32_t khz_diff; + + khz_diff = cpu_khz > gtsc_khz ? + cpu_khz - gtsc_khz : gtsc_khz - cpu_khz; +if (vtsc_khz_tolerance) + diff_tolerated = khz_diff <= vtsc_khz_tolerance; + +printk (XENLOG_WARNING "%s: d%u: host has %lu kHz," +" domU expects %u kHz," +" difference of %u is %s tolerance of %u\n" , + __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff, + diff_tolerated ?"within" :"outside" , vtsc_khz_tolerance); + }if ( tsc_mode ==TSC_MODE_DEFAULT &&host_tsc_is_safe () && - (d->arch.tsc_khz == cpu_khz || + (diff_tolerated || (is_hvm_domain (d) &&hvm_get_tsc_scaling_ratio (d->arch.tsc_khz))) ) {
@@ -2237,6 +2258,8 @@ static void dump_softtsc(unsigned char key)printk (",ofs=%#" PRIx64, d->arch.vtsc_offset);if ( d->arch.tsc_khz )printk (",khz=%" PRIu32, d->arch.tsc_khz); +if ( d->arch.vtsc_khz_tolerance ) +printk (",tol=%" PRIu16, d->arch.vtsc_khz_tolerance);if ( d->arch.incarnation )printk (",inc=%" PRIu32, d->arch.incarnation);#if !defined(NDEBUG ) || defined(CONFIG_PERF_COUNTERS )
xen/include/asm-x86/domain.h +1 -0
@@ -369,6 +369,7 @@ struct arch_domain/* TSC management (emulation, pv, scaling, stats) */ int tsc_mode;/* see include/asm-x86/time.h */ bool_t vtsc;/* tsc is emulated (may change after migrate) */ +uint16_t vtsc_khz_tolerance;/* domU handles that much jitter in khz value */ s_time_t vtsc_last;/* previous TSC value (guarantee monotonicity) */ spinlock_t vtsc_lock;uint64_t vtsc_offset;/* adjustment for save/restore/migrate */
xen/include/asm-x86/time.h +2 -2
@@ -61,10 +61,10 @@ u64 gtime_to_gtsc(struct domain *d, u64 time);u64 gtsc_to_gtime (struct domain *d,u64 tsc);void tsc_set_info (struct domain *d,uint32_t tsc_mode,uint64_t elapsed_nsec, -uint32_t gtsc_khz,uint32_t incarnation); +uint32_t gtsc_khz,uint16_t vtsc_khz_tolerance,uint32_t incarnation);void tsc_get_info (struct domain *d,uint32_t *tsc_mode,uint64_t *elapsed_nsec, -uint32_t *gtsc_khz,uint32_t *incarnation); +uint32_t *gtsc_khz,uint16_t *vtsc_khz_tolerance,uint32_t *incarnation);void force_update_vcpu_system_time (struct vcpu *v);
xen/include/public/domctl.h +4 -1
@@ -697,12 +697,14 @@ struct xen_domctl_disable_migrate {/* XEN_DOMCTL_gettscinfo */ /* XEN_DOMCTL_settscinfo */ +/* XEN_DOMCTL_set_vtsc_khz_tolerance */ struct xen_domctl_tsc_info {/* IN/OUT */ uint32_t tsc_mode;uint32_t gtsc_khz;uint32_t incarnation; -uint32_t pad; +uint16_t vtsc_khz_tolerance; +uint16_t pad;uint64_aligned_t elapsed_nsec; };
@@ -1172,6 +1174,7 @@ struct xen_domctl {#define XEN_DOMCTL_soft_reset 79#define XEN_DOMCTL_set_gnttab_limits 80#define XEN_DOMCTL_vuart_op 81 +#define XEN_DOMCTL_set_vtsc_khz_tolerance 82#define XEN_DOMCTL_gdbsx_guestmemio 1000#define XEN_DOMCTL_gdbsx_pausevcpu 1001#define XEN_DOMCTL_gdbsx_unpausevcpu 1002
xen/xsm/flask/hooks.c +1 -0
@@ -721,6 +721,7 @@ static int flask_domctl(struct domain *d, int cmd)return current_has_perm (d,SECCLASS_DOMAIN2 ,DOMAIN2__GETTSC );case XEN_DOMCTL_settscinfo: +case XEN_DOMCTL_set_vtsc_khz_tolerance:return current_has_perm (d,SECCLASS_DOMAIN2 ,DOMAIN2__SETTSC );case XEN_DOMCTL_audit_p2m:
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 1 line
Quoted 1 line
Quoted 1 line · On 05.03.18 at 12:35, <[email protected]> wrote:
One thing I'm missing in the description (or the added documentation) is a discussion of the conditions under which it is safe to make use of the new setting.
xen/arch/x86/domctl.c long arch_do_domctl( · line 966
Quoted 13 lines
xen/arch/x86/domctl.c +4
-0@@ -954,11 +955,21 @@ long arch_do_domctl(tsc_set_info (d, domctl->u.tsc_info.tsc_mode, domctl->u.tsc_info.elapsed_nsec, domctl->u.tsc_info.gtsc_khz, + domctl->u.tsc_info.vtsc_khz_tolerance, domctl->u.tsc_info.incarnation);domain_unpause (d); }break ; +case XEN_DOMCTL_set_vtsc_khz_tolerance: +if ( d == currd ) + ret = -EINVAL ;
Why? There's e.g. no domain_pause() involved here. And without that there's no difference between changing the value behind the back of a foreign domain, of for oneself. Granted Dom0 isn't likely to want to do that, but such checks should have a reason.
xen/arch/x86/domctl.c long arch_do_domctl( · line 970
Quoted 4 lines
xen/arch/x86/domctl.c +4
-0+else + { + d->arch.vtsc_khz_tolerance = domctl->u.tsc_info.vtsc_khz_tolerance; + }
Stray braces (the more that the if() side doesn't have them).
Also throughout the patch I wonder if it wasn't more natural to put the unit last in the parameter / field names.
xen/arch/x86/time.c void tsc_get_info(struct domain *d, uint32_t *tsc_mode, · line 2127
Quoted 7 lines
xen/arch/x86/time.c +2
-1@@ -2122,7 +2123,8 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,*/ void tsc_set_info (struct domain *d,uint32_t tsc_mode,uint64_t elapsed_nsec, -uint32_t gtsc_khz,uint32_t incarnation) +uint32_t gtsc_khz,uint16_t vtsc_khz_tolerance, +uint32_t incarnation)
For the sake of consistency with the other types here I'm not going to demand to replace uint16_t here, but it's really not necessary to use that type here (other than on the read path).
xen/arch/x86/time.c void tsc_set_info(struct domain *d, · line 2157
Quoted 7 lines
xen/arch/x86/time.c +3
-0@@ -2147,8 +2152,24 @@ void tsc_set_info(struct domain *d,* When a guest is created, gtsc_khz is passed in as zero, making * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation. */ + diff_tolerated = d->arch.tsc_khz == cpu_khz; + +if ( tsc_mode ==TSC_MODE_DEFAULT && gtsc_khz ) {
Style.
xen/arch/x86/time.c void tsc_set_info(struct domain *d, · line 2162
Quoted 5 lines
xen/arch/x86/time.c +5
-0+uint32_t khz_diff; + + khz_diff = cpu_khz > gtsc_khz ? + cpu_khz - gtsc_khz : gtsc_khz - cpu_khz; +if (vtsc_khz_tolerance)
Again.
xen/arch/x86/time.c void tsc_set_info(struct domain *d, · line 2169
Quoted 7 lines
xen/arch/x86/time.c +7
-0+ diff_tolerated = khz_diff <= vtsc_khz_tolerance; + +printk (XENLOG_WARNING "%s: d%u: host has %lu kHz," +" domU expects %u kHz," +" difference of %u is %s tolerance of %u\n" , + __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff, + diff_tolerated ?"within" :"outside" , vtsc_khz_tolerance);
Leftover debugging message?
Quoted 5 lines
xen/include/public/domctl.h +0
-0@@ -697,12 +697,14 @@ struct xen_domctl_disable_migrate {/* XEN_DOMCTL_gettscinfo */
xen/include/public/domctl.h struct xen_domctl_disable_migrate { · line 707
Quoted 10 lines
xen/include/public/domctl.h +3
-1/* XEN_DOMCTL_settscinfo */ +/* XEN_DOMCTL_set_vtsc_khz_tolerance */ struct xen_domctl_tsc_info {/* IN/OUT */ uint32_t tsc_mode;uint32_t gtsc_khz;uint32_t incarnation; -uint32_t pad; +uint16_t vtsc_khz_tolerance; +uint16_t pad;
Generally with the prior pad field not being checked anywhere this isn't a valid extension. However, this being a domctl (and the interface version suitably bumped already) plus looking at how the libxc code works, I think you can get away doing it like this.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Am Mon, 05 Mar 2018 06:18:17 -0700
schrieb "Jan Beulich" <[email protected]>:
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line · On 05.03.18 at 12:35, <[email protected]> wrote:
One thing I'm missing in the description (or the added documentation) is a discussion of the conditions under which it is safe to make use of the new setting.
The same rules as tsc_mode=native apply, I think. xen-tscmode(7) has to be updated as well to cover the new knob.
Quoted 4 lines
xen/arch/x86/domctl.c long arch_do_domctl( · line 966
Quoted 3 lines
xen/arch/x86/domctl.c +3
-0+case XEN_DOMCTL_set_vtsc_khz_tolerance: +if ( d == currd ) + ret = -EINVAL ;Why? There's e.g. no
domain_pause()involved here.
I have thought about that. Now I think that part can be even simpler, just like the following XEN_DOMCTL_suppress_spurious_page_faults:
just change that read-only value unconditionally. There is no obvious harm in changing that value.
Quoted 2 lines
Also throughout the patch I wonder if it wasn't more natural to put the unit last in the parameter / field names.
That was just to keep the diff slightly smaller.
Quoted 6 lines
xen/arch/x86/time.c void tsc_set_info(struct domain *d, · line 2169
Quoted 5 lines
xen/arch/x86/time.c +5
-0+printk (XENLOG_WARNING "%s: d%u: host has %lu kHz," +" domU expects %u kHz," +" difference of %u is %s tolerance of %u\n" , + __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff, + diff_tolerated ?"within" :"outside" , vtsc_khz_tolerance);Leftover debugging message?
I think it is worth to log that event, perhaps not with WARNING level.
Olaf
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 15 lines
Quoted 1 line
Quoted 1 line · On 05.03.18 at 15:18, <[email protected]> wrote:
Am Mon, 05 Mar 2018 06:18:17 -0700
schrieb "Jan Beulich" <[email protected]>:Quoted 5 lines
xen/arch/x86/domctl.c long arch_do_domctl( · line 966
Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 05.03.18 at 12:35, <[email protected]> wrote:
xen/arch/x86/domctl.c +3
-0+case XEN_DOMCTL_set_vtsc_khz_tolerance: +if ( d == currd ) + ret = -EINVAL ;Why? There's e.g. no
domain_pause()involved here.I have thought about that. Now I think that part can be even simpler, just like the following
XEN_DOMCTL_suppress_spurious_page_faults:
just change that read-only value unconditionally. There is no obvious harm in changing that value.
Quoted 4 lines
Quoted 2 lines
Also throughout the patch I wonder if it wasn't more natural to put the unit last in the parameter / field names.
That was just to keep the diff slightly smaller.
I'm only talking about additions you make, and I don't see a size difference between "vtsc_khz_tolerance" and "vtsc_tolerance_khz".
Quoted 8 lines
Quoted 6 lines
xen/arch/x86/time.c void tsc_set_info(struct domain *d, · line 2169
Quoted 5 lines
xen/arch/x86/time.c +5
-0+printk (XENLOG_WARNING "%s: d%u: host has %lu kHz," +" domU expects %u kHz," +" difference of %u is %s tolerance of %u\n" , + __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff, + diff_tolerated ?"within" :"outside" , vtsc_khz_tolerance);Leftover debugging message?
I think it is worth to log that event, perhaps not with WARNING level.
XENLOG_G_INFO at most, I would say, and perhaps only when the setting is non-zero.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 15 lines · Jan Beulich
Quoted 15 lines
Quoted 1 line
Quoted 1 line · On 05.03.18 at 15:18, <[email protected]> wrote:
Am Mon, 05 Mar 2018 06:18:17 -0700
schrieb "Jan Beulich" <[email protected]>:Quoted 5 lines
xen/arch/x86/domctl.c long arch_do_domctl( · line 966
Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 05.03.18 at 12:35, <[email protected]> wrote:
xen/arch/x86/domctl.c +3
-0+case XEN_DOMCTL_set_vtsc_khz_tolerance: +if ( d == currd ) + ret = -EINVAL ;Why? There's e.g. no
domain_pause()involved here.I have thought about that. Now I think that part can be even simpler, just like the following
XEN_DOMCTL_suppress_spurious_page_faults:
just change that read-only value unconditionally. There is no obvious harm in changing that value.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Also throughout the patch I wonder if it wasn't more natural to put the unit last in the parameter / field names.
That was just to keep the diff slightly smaller.
I'm only talking about additions you make, and I don't see a size difference between "
vtsc_khz_tolerance" and "vtsc_tolerance_khz".
+1 to unit at the end.
-George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Am Mon, 05 Mar 2018 07:45:24 -0700
schrieb "Jan Beulich" <[email protected]>:
Quoted 2 lines
I'm only talking about additions you make, and I don't see a size difference between "
vtsc_khz_tolerance" and "vtsc_tolerance_khz".
Oh, I misunderstood that sentence. I will change the name as suggested.
Olaf
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel