From patchwork Mon Jan 27 20:21:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 11353231 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 C2A3A112B for ; Mon, 27 Jan 2020 20:22:51 +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 9556024679 for ; Mon, 27 Jan 2020 20:22:51 +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="QmYn+rdu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9556024679 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 1iwAsx-0003N1-Ho; Mon, 27 Jan 2020 20:21:27 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iwAsw-0003Mw-PC for xen-devel@lists.xenproject.org; Mon, 27 Jan 2020 20:21:26 +0000 X-Inumbo-ID: 9755ec20-4142-11ea-acc1-bc764e2007e4 Received: from esa4.hc3370-68.iphmx.com (unknown [216.71.155.144]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 9755ec20-4142-11ea-acc1-bc764e2007e4; Mon, 27 Jan 2020 20:21:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1580156485; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=t66ED8Brbp3tAEhsInxFtGAnkVTpTpZe78KO2ukSmKA=; b=QmYn+rduHa9qF7HPiUllj6BU9vDTo8GEpdDgDYdo6JGWXnun+rhP3cm1 Y0Xba7BfsXGfhH5B5vve5TKtw+nTGBUSYxs/1ff0xfEOfuOHu97kxy8Xg mang80yAELHB1dYeE+vLm3Trx9HOQquMjQOLmgu83O1hOOCm3SKaUsMNH c=; Authentication-Results: esa4.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 (esa4.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=esa4.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa4.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=esa4.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 (esa4.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=esa4.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: oyq5yt+KyEh9a71sXN+sglLo2EsCavXu91PtMxkkALYg6lYkEDGG6YerTv1589O4Zs1TTEkHLU 9WLkilbI9LSo9EYma2WuZ6Fd+pcyk8TMeMlkR8HI96jAP4VcOCSfHM+y+pfyyWT0XAt/RMzW3E +kiArF37u3VcYRPeyg/N6HvtaSDr6DVDj7/RRQ65xCRKlLEvlTZwGz12M9zx6IsQGQvFP+DXy4 gtHbO/B+UJcZ7/DXBDfHJfMhT6yQ7eabrYpgyncU3giL2MvHr8p2U+i9G9TfDkevwkSKkY1+/5 4ok= X-SBRS: 2.7 X-MesageID: 12118636 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.70,371,1574139600"; d="scan'208";a="12118636" From: Andrew Cooper To: Xen-devel Date: Mon, 27 Jan 2020 20:21:21 +0000 Message-ID: <20200127202121.2961-1-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH RFC] x86/amd: Avoid cpu_has_hypervisor evaluating true on native hardware 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: Andrew Cooper , Igor Druzhinin , Wei Liu , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Currently when booting native on AMD hardware, cpuidmask_defaults._1cd gets configured with the HYPERVISOR bit before native CPUID is scanned for feature bits. This results in cpu_has_hypervisor becoming set as part of identify_cpu(), and ends up appearing in the raw and host CPU policies. Nothing has really cared in the past. Alter amd_init_levelling() to exclude the HYPERVISOR bit from cpumask_defaults, and update domain_cpu_policy_changed() to allow it to be explicitly forwarded. This in turn highlighted that dom0 construction was asymetric with domU construction, by not having any calls to domain_cpu_policy_changed(). Extend arch_domain_create() to always call domain_cpu_policy_changed(). Reported-by: Igor Druzhinin Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Igor Druzhinin Without this fix, there is apparently a problem with Roger's "[PATCH v3 7/7] x86/tlb: use Xen L0 assisted TLB flush when available" on native AMD hardware. I haven't investgiated the issue with that patch specifically, because cpu_has_hypervisor being wrong is obviously a bug. This is one of two possible approaches, and both have their downsides. This one takes an extra hit on context switches between PV vcpus and idle/hvm, as they will usually differ in HYPERVISOR bit. The other approach is to order things more carefully so levelling is configured after scanning for cpuid bits, but that has the downside that it is very easy to regress. Thoughts on which is the least-bad approach to take? Having written this patch, I'm now erring on the side of doing it the other way. --- xen/arch/x86/cpu/amd.c | 3 --- xen/arch/x86/domain.c | 2 ++ xen/arch/x86/domctl.c | 9 ++++++++- xen/include/asm-x86/domain.h | 2 ++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c index 8b5f0f2e4c..0906b23582 100644 --- a/xen/arch/x86/cpu/amd.c +++ b/xen/arch/x86/cpu/amd.c @@ -297,9 +297,6 @@ static void __init noinline amd_init_levelling(void) ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE); edx |= cpufeat_mask(X86_FEATURE_APIC); - /* Allow the HYPERVISOR bit to be set via guest policy. */ - ecx |= cpufeat_mask(X86_FEATURE_HYPERVISOR); - cpuidmask_defaults._1cd = ((uint64_t)ecx << 32) | edx; } diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 28fefa1f81..316b801597 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -665,6 +665,8 @@ int arch_domain_create(struct domain *d, */ d->arch.x87_fip_width = cpu_has_fpu_sel ? 0 : 8; + domain_cpu_policy_changed(d); + return 0; fail: diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 5ed63ac10a..0627eb4e06 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -48,7 +48,7 @@ static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop) } #endif -static void domain_cpu_policy_changed(struct domain *d) +void domain_cpu_policy_changed(struct domain *d) { const struct cpuid_policy *p = d->arch.cpuid; struct vcpu *v; @@ -106,6 +106,13 @@ static void domain_cpu_policy_changed(struct domain *d) ecx = 0; edx = cpufeat_mask(X86_FEATURE_APIC); + /* + * If the Hypervisor bit is set in the policy, we can also + * forward it into real CPUID. + */ + if ( p->basic.hypervisor ) + ecx |= cpufeat_mask(X86_FEATURE_HYPERVISOR); + mask |= ((uint64_t)ecx << 32) | edx; break; } diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index a3ae5d9a20..817065bf81 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -624,6 +624,8 @@ struct guest_memory_policy void update_guest_memory_policy(struct vcpu *v, struct guest_memory_policy *policy); +void domain_cpu_policy_changed(struct domain *d); + bool update_runstate_area(struct vcpu *); bool update_secondary_system_time(struct vcpu *, struct vcpu_time_info *);