From patchwork Tue Mar 3 18:24:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 11418677 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 10342138D for ; Tue, 3 Mar 2020 18:26:29 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D596720842 for ; Tue, 3 Mar 2020 18:26:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="ItGQoI3K" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D596720842 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1j9CDu-0004zF-AC; Tue, 03 Mar 2020 18:24:54 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1j9CDt-0004z9-3p for xen-devel@lists.xenproject.org; Tue, 03 Mar 2020 18:24:53 +0000 X-Inumbo-ID: 4582bcba-5d7c-11ea-8efe-bc764e2007e4 Received: from esa2.hc3370-68.iphmx.com (unknown [216.71.145.153]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 4582bcba-5d7c-11ea-8efe-bc764e2007e4; Tue, 03 Mar 2020 18:24:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1583259892; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=KAQatxYRLdKb3n3X1uMDD2WK3/IZ8WxcTj+T0knAkTw=; b=ItGQoI3K6CK0lF1ezNq2kuA+dSLsCaWYxotyFbtHNj66jpa8x5FT0zXz zCAKEHu/UJCYSF95ZkawdLsSM1noGCbkBkqn8aZfrbDc1popBpv0d/EY2 kBj08EExz6HRhbzqvGdG/ZWdBQtf0xHbZQAkXxtr+dLDI7l58k74Eqeb/ E=; Authentication-Results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@citrix.com; spf=Pass smtp.mailfrom=Andrew.Cooper3@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa2.hc3370-68.iphmx.com: no sender authenticity information available from domain of andrew.cooper3@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa2.hc3370-68.iphmx.com: domain of Andrew.Cooper3@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="Andrew.Cooper3@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa2.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: HP3tepLCLn1x8U0BAFrNFXuIHdxjTnOf3OuWc3em7rl1GDkwM1bYcpcLP4tTrSw87g7SYH2lHW M8o5o5S/7TqIqrbQAj6Cnx99i/E92aH7i5pskmkCOTZZ1GGpiKn2Detv3wAydTX8abDNjrc/tk g2y/v2uGJsPe35fkDXJOzHtsK/D0Tungh/tbKLLmtFhYt8kwhV/0yojahbWUp5i+bmdseeSVKD 7AAF034DjdB6ciDhsMZzXyrlz1GCrI2yVvU3P/vv+cqKa47SRVhWj4tqEmVAjvZgoW9Pdw/73u v+0= X-SBRS: 2.7 X-MesageID: 13358219 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.70,511,1574139600"; d="scan'208";a="13358219" From: Andrew Cooper To: Xen-devel Date: Tue, 3 Mar 2020 18:24:47 +0000 Message-ID: <20200303182447.15469-1-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH] x86/cpuid: Untangle Invariant TSC handling X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Wei Liu , Andrew Cooper , Jan Beulich , Anthony PERARD , Ian Jackson , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" ITSC being visible to the guest is currently implicit with the toolstack unconditionally asking for it, and Xen clipping it based on the vTSC and/or XEN_DOMCTL_disable_migrate settings. This is problematic for several reasons. First, the implicit vTSC behaviour manifests as a real bug on migration to a host with a different frequency, with ITSC but without TSC scaling capabilities, whereby the ITSC feature becomes advertised to the guest. ITSC will disappear again if the guest migrates to server with the same frequency as the original, or to one with TSC scaling support. Secondly, disallowing ITSC unless the guest doesn't migrate is conceptually wrong. It is common to have migration pools of identical hardware, at which point the TSC frequency is the same, and more modern hardware has TSC scaling support anyway. In both cases, it is safe to advertise ITSC and migrate the guest. Remove all implicit logic logic in Xen, and make ITSC part of the max CPUID policies for guests. Plumb an itsc parameter into xc_cpuid_apply_policy() and have libxl__cpuid_legacy() fill in the two cases where it can reasonably expect ITSC to be safe for the guest to see. This is a behaviour change for TSC_MODE_NATIVE, where the ITSC will now reliably not appear, and for the case where the user explicitly requests ITSC, in which case it will appear even if the guest isn't marked as nomigrate. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Ian Jackson CC: Anthony PERARD --- tools/libxc/include/xenctrl.h | 4 ++-- tools/libxc/xc_cpuid_x86.c | 12 ++++++------ tools/libxl/libxl_cpuid.c | 18 +++++++++++++++++- xen/arch/x86/cpuid.c | 8 -------- xen/arch/x86/time.c | 2 -- xen/include/public/arch-x86/cpufeatureset.h | 2 +- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 8d13a7e20b..80a42776e2 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1802,12 +1802,12 @@ int xc_cpuid_set(xc_interface *xch, * Make adjustments to the CPUID settings for a domain. * * Either pass a full new @featureset (and @nr_features), or adjust individual - * features (@pae). + * features (@pae, @itsc). */ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, const uint32_t *featureset, - unsigned int nr_features, bool pae); + unsigned int nr_features, bool pae, bool itsc); int xc_mca_op(xc_interface *xch, struct xen_mc *mc); int xc_mca_op_inject_v2(xc_interface *xch, unsigned int flags, xc_cpumap_t cpumap, unsigned int nr_cpus); diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c index f045b03223..35fd36741b 100644 --- a/tools/libxc/xc_cpuid_x86.c +++ b/tools/libxc/xc_cpuid_x86.c @@ -438,7 +438,7 @@ int xc_cpuid_set( int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, const uint32_t *featureset, unsigned int nr_features, - bool pae) + bool pae, bool itsc) { int rc; xc_dominfo_t di; @@ -534,6 +534,8 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, } else { + p->extd.itsc = itsc; + if ( di.hvm ) p->basic.pae = pae; } @@ -621,12 +623,10 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, } /* - * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM / - * XEN_DOMCTL_disable_migrate settings to be reflected correctly in - * CPUID. Xen will discard these bits if configuration hasn't been - * set for the domain. + * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM + * to be reflected correctly in CPUID. Xen will discard these bits if + * configuration hasn't been set for the domain. */ - p->extd.itsc = true; p->basic.vmx = true; p->extd.svm = true; } diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c index b4f6fd590d..715d195a4c 100644 --- a/tools/libxl/libxl_cpuid.c +++ b/tools/libxl/libxl_cpuid.c @@ -418,6 +418,7 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, int i; char *cpuid_res[4]; bool pae = true; + bool itsc; /* * For PV guests, PAE is Xen-controlled (it is the 'p' that differentiates @@ -432,7 +433,22 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, if (info->type == LIBXL_DOMAIN_TYPE_HVM) pae = libxl_defbool_val(info->u.hvm.pae); - xc_cpuid_apply_policy(ctx->xch, domid, NULL, 0, pae); + /* + * Advertising Invariant TSC to a guest means that the TSC frequency won't + * change at any point in the future. + * + * We do not have enough information about potential migration + * destinations to know whether advertising ITSC is safe, but if the guest + * isn't going to migrate, then the current hardware is all that matters. + * + * Alternatively, an internal property of vTSC is that the values read are + * invariant. Advertise ITSC when we know the domain will have emualted + * TSC everywhere it goes. + */ + itsc = (libxl_defbool_val(info->disable_migrate) || + info->tsc_mode == LIBXL_TSC_MODE_ALWAYS_EMULATE); + + xc_cpuid_apply_policy(ctx->xch, domid, NULL, 0, pae, itsc); if (!cpuid) return; diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index 6e01394fd2..1f9bab7bc1 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -594,14 +594,6 @@ void recalculate_cpuid_policy(struct domain *d) } /* - * ITSC is masked by default (so domains are safe to migrate), but a - * toolstack which has configured disable_migrate or vTSC for a domain may - * safely select it, and needs a way of doing so. - */ - if ( cpu_has_itsc && (d->disable_migrate || d->arch.vtsc) ) - __set_bit(X86_FEATURE_ITSC, max_fs); - - /* * On hardware with MSR_TSX_CTRL, the admin may have elected to disable * TSX and hide the feature bits. Migrating-in VMs may have been booted * pre-mitigation when the TSX features were visbile. diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index bb1b97787f..bdb7979d2a 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -2380,8 +2380,6 @@ int tsc_set_info(struct domain *d, } } - recalculate_cpuid_policy(d); - return 0; } diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index e63a93119e..0e29ca763f 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -241,7 +241,7 @@ XEN_CPUFEATURE(RDPID, 6*32+22) /*A RDPID instruction */ XEN_CPUFEATURE(CLDEMOTE, 6*32+25) /*A CLDEMOTE instruction */ /* AMD-defined CPU features, CPUID level 0x80000007.edx, word 7 */ -XEN_CPUFEATURE(ITSC, 7*32+ 8) /* Invariant TSC */ +XEN_CPUFEATURE(ITSC, 7*32+ 8) /*a Invariant TSC */ XEN_CPUFEATURE(EFRO, 7*32+10) /* APERF/MPERF Read Only interface */ /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */