From patchwork Mon May 20 15:26:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 10951697 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 929D06C5 for ; Mon, 20 May 2019 15:28:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8271428868 for ; Mon, 20 May 2019 15:28:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 76678288B8; Mon, 20 May 2019 15:28:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C1E8828803 for ; Mon, 20 May 2019 15:28:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hSkBO-0004sh-0h; Mon, 20 May 2019 15:26:34 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hSkBN-0004sc-Jq for xen-devel@lists.xenproject.org; Mon, 20 May 2019 15:26:33 +0000 X-Inumbo-ID: a3d2605a-7b13-11e9-bda7-9bde5fc7a186 Received: from esa3.hc3370-68.iphmx.com (unknown [216.71.145.155]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id a3d2605a-7b13-11e9-bda7-9bde5fc7a186; Mon, 20 May 2019 15:26:30 +0000 (UTC) Authentication-Results: esa3.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@MIAPEX02MSOL02.citrite.net Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of andrew.cooper3@citrix.com) identity=pra; client-ip=23.29.105.83; receiver=esa3.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa3.hc3370-68.iphmx.com: domain of Andrew.Cooper3@citrix.com designates 23.29.105.83 as permitted sender) identity=mailfrom; client-ip=23.29.105.83; receiver=esa3.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:23.29.105.83 ip4:162.221.156.83 ~all" Received-SPF: None (esa3.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@MIAPEX02MSOL02.citrite.net) identity=helo; client-ip=23.29.105.83; receiver=esa3.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@MIAPEX02MSOL02.citrite.net"; x-conformance=sidf_compatible IronPort-SDR: 9zP0fkp1dGSe18+JR1CZgjlDbka9kCoonsXhXMfBZrLvxFtHXkvwGmfkXtX7/La4bQbNJmbS7I 8ranl0JejigCtbMrpjVccynVhXSVwIoby/bSQtVtIUSQysP3zeVZCUSNtKK7gAYf95YbDr6C85 OFPVr0lBehpm4YvuF80mRtRxilirDYcV5Bjx+dERgrtSfrGRWxwRuXd1OZOWwAIC3ZeqjB2X3c w6Kpe/V3uQGQvcZFCA+3OpgwEg5aE2XlDDCNGFXV+aKL6ZnmsUVyw/HXIfQUvIitlmx1nADDda 820= X-SBRS: 2.7 X-MesageID: 671591 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 23.29.105.83 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.60,491,1549947600"; d="scan'208";a="671591" From: Andrew Cooper To: Xen-devel Date: Mon, 20 May 2019 16:26:21 +0100 Message-ID: <1558365981-3175-1-git-send-email-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH] x86/cpuid: Don't use volatile asm statements 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 , Wei Liu , Jan Beulich , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Common use of the CPUID instruction operates without side effects. Let the compiler better optimise code by dropping the volatile qualifier. The only place where order matters is for Intel microcode loading, where executing a CPUID instruction is used for its side effect of updating MSR_IA32_UCODE_REV. The existing logic is buggy because GCC has been seen to reorder independent asm volatile statements. Opencode the two cases, with a compiler barrier to enforce the correct ordering of operations. While here, fix the comment, which isn't correct. The SDM doesn't state that a read of leaf 1 is required - just that a CPUID instruction is required. Using leaf 0 results in better code generation, following the write of 0 to MSR_IA32_UCODE_REV. Suggested-by: Jan Beulich Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné --- xen/arch/x86/microcode_intel.c | 13 ++++++++----- xen/include/asm-x86/processor.h | 12 ++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 22fdeca..088cfa2 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -99,6 +99,7 @@ static DEFINE_SPINLOCK(microcode_update_lock); static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) { struct cpuinfo_x86 *c = &cpu_data[cpu_num]; + unsigned long tmp; uint64_t msr_content; BUG_ON(cpu_num != smp_processor_id()); @@ -122,8 +123,9 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) } wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL); - /* As documented in the SDM: Do a CPUID 1 here */ - cpuid_eax(1); + /* As documented in the SDM: Do a CPUID here. */ + asm volatile ( "cpuid" : "=a" (tmp) : "a" (0) + : "bx", "cx", "dx", "memory" ); /* get the current revision from MSR 0x8B */ rdmsrl(MSR_IA32_UCODE_REV, msr_content); @@ -286,7 +288,7 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) static int apply_microcode(unsigned int cpu) { - unsigned long flags; + unsigned long flags, tmp; uint64_t msr_content; unsigned int val[2]; unsigned int cpu_num = raw_smp_processor_id(); @@ -305,8 +307,9 @@ static int apply_microcode(unsigned int cpu) wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)uci->mc.mc_intel->bits); wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL); - /* As documented in the SDM: Do a CPUID 1 here */ - cpuid_eax(1); + /* As documented in the SDM: Do a CPUID here. */ + asm volatile ( "cpuid" : "=a" (tmp) : "a" (0) + : "bx", "cx", "dx", "memory" ); /* get the current revision from MSR 0x8B */ rdmsrl(MSR_IA32_UCODE_REV, msr_content); diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index cef3ffb..3c76e7e 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -185,7 +185,7 @@ unsigned int apicid_to_socket(unsigned int); * resulting in stale register contents being returned. */ #define cpuid(_op,_eax,_ebx,_ecx,_edx) \ - asm volatile ( "cpuid" \ + asm ( "cpuid" \ : "=a" (*(int *)(_eax)), \ "=b" (*(int *)(_ebx)), \ "=c" (*(int *)(_ecx)), \ @@ -201,7 +201,7 @@ static inline void cpuid_count( unsigned int *ecx, unsigned int *edx) { - asm volatile ( "cpuid" + asm ( "cpuid" : "=a" (*eax), "=b" (*ebx), "=c" (*ecx), "=d" (*edx) : "0" (op), "c" (count) ); } @@ -213,7 +213,7 @@ static always_inline unsigned int cpuid_eax(unsigned int op) { unsigned int eax; - asm volatile ( "cpuid" + asm ( "cpuid" : "=a" (eax) : "0" (op) : "bx", "cx", "dx" ); @@ -224,7 +224,7 @@ static always_inline unsigned int cpuid_ebx(unsigned int op) { unsigned int eax, ebx; - asm volatile ( "cpuid" + asm ( "cpuid" : "=a" (eax), "=b" (ebx) : "0" (op) : "cx", "dx" ); @@ -235,7 +235,7 @@ static always_inline unsigned int cpuid_ecx(unsigned int op) { unsigned int eax, ecx; - asm volatile ( "cpuid" + asm ( "cpuid" : "=a" (eax), "=c" (ecx) : "0" (op) : "bx", "dx" ); @@ -246,7 +246,7 @@ static always_inline unsigned int cpuid_edx(unsigned int op) { unsigned int eax, edx; - asm volatile ( "cpuid" + asm ( "cpuid" : "=a" (eax), "=d" (edx) : "0" (op) : "bx", "cx" );