From patchwork Thu Sep 12 07:22:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142485 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 E982A1599 for ; Thu, 12 Sep 2019 07:20: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 CF03821479 for ; Thu, 12 Sep 2019 07:20:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CF03821479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNI-0001PZ-Kq; Thu, 12 Sep 2019 07:18:40 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1i8JNH-0001PN-3f for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:39 +0000 X-Inumbo-ID: 8730777e-d52d-11e9-978d-bc764e2007e4 Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 8730777e-d52d-11e9-978d-bc764e2007e4; Thu, 12 Sep 2019 07:18:33 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906197" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:30 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:14 +0800 Message-Id: <1568272949-1086-2-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 01/16] microcode/intel: extend microcode_update_match() 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" to a more generic function. So that it can be used alone to check an update against the CPU signature and current update revision. Note that enum microcode_match_result will be used in common code (aka microcode.c), it has been placed in the common header. And constifying the parameter of microcode_sanity_check() such that it can be called by microcode_update_match(). Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v10: - Drop RBs - assert that microcode passed to microcode_update_match() would pass sanity check. Constify the parameter of microcode_sanity_check() Changes in v9: - microcode_update_match() doesn't accept (sig, pf, rev) any longer. Hence, it won't be used to compare two arbitrary updates. - rewrite patch description Changes in v8: - make sure enough room for an extended header and signature array Changes in v6: - eliminate unnecessary type casting in microcode_update_match - check if a patch has an extend header Changes in v5: - constify the extended_signature - use named enum type for the return value of microcode_update_match --- xen/arch/x86/microcode_intel.c | 75 ++++++++++++++++++++++------------------- xen/include/asm-x86/microcode.h | 6 ++++ 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 22fdeca..1a3ffa5 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -134,21 +134,11 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) return 0; } -static inline int microcode_update_match( - unsigned int cpu_num, const struct microcode_header_intel *mc_header, - int sig, int pf) +static int microcode_sanity_check(const void *mc) { - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num); - - return (sigmatch(sig, uci->cpu_sig.sig, pf, uci->cpu_sig.pf) && - (mc_header->rev > uci->cpu_sig.rev)); -} - -static int microcode_sanity_check(void *mc) -{ - struct microcode_header_intel *mc_header = mc; - struct extended_sigtable *ext_header = NULL; - struct extended_signature *ext_sig; + const struct microcode_header_intel *mc_header = mc; + const struct extended_sigtable *ext_header = NULL; + const struct extended_signature *ext_sig; unsigned long total_size, data_size, ext_table_size; unsigned int ext_sigcount = 0, i; uint32_t sum, orig_sum; @@ -234,6 +224,42 @@ static int microcode_sanity_check(void *mc) return 0; } +/* Check an update against the CPU signature and current update revision */ +static enum microcode_match_result microcode_update_match( + const struct microcode_header_intel *mc_header, unsigned int cpu) +{ + const struct extended_sigtable *ext_header; + const struct extended_signature *ext_sig; + unsigned int i; + struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + unsigned int sig = uci->cpu_sig.sig; + unsigned int pf = uci->cpu_sig.pf; + unsigned int rev = uci->cpu_sig.rev; + unsigned long data_size = get_datasize(mc_header); + const void *end = (const void *)mc_header + get_totalsize(mc_header); + + ASSERT(!microcode_sanity_check(mc_header)); + if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) ) + return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE; + + ext_header = (const void *)(mc_header + 1) + data_size; + ext_sig = (const void *)(ext_header + 1); + + /* + * Make sure there is enough space to hold an extended header and enough + * array elements. + */ + if ( (end < (const void *)ext_sig) || + (end < (const void *)(ext_sig + ext_header->count)) ) + return MIS_UCODE; + + for ( i = 0; i < ext_header->count; i++ ) + if ( sigmatch(sig, ext_sig[i].sig, pf, ext_sig[i].pf) ) + return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE; + + return MIS_UCODE; +} + /* * return 0 - no update found * return 1 - found update @@ -243,31 +269,12 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) { struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); const struct microcode_header_intel *mc_header = mc; - const struct extended_sigtable *ext_header; unsigned long total_size = get_totalsize(mc_header); - int ext_sigcount, i; - struct extended_signature *ext_sig; void *new_mc; - if ( microcode_update_match(cpu, mc_header, - mc_header->sig, mc_header->pf) ) - goto find; - - if ( total_size <= (get_datasize(mc_header) + MC_HEADER_SIZE) ) + if ( microcode_update_match(mc, cpu) != NEW_UCODE ) return 0; - ext_header = mc + get_datasize(mc_header) + MC_HEADER_SIZE; - ext_sigcount = ext_header->count; - ext_sig = (void *)ext_header + EXT_HEADER_SIZE; - for ( i = 0; i < ext_sigcount; i++ ) - { - if ( microcode_update_match(cpu, mc_header, - ext_sig->sig, ext_sig->pf) ) - goto find; - ext_sig++; - } - return 0; - find: pr_debug("microcode: CPU%d found a matching microcode update with" " version %#x (current=%#x)\n", cpu, mc_header->rev, uci->cpu_sig.rev); diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 23ea954..882f560 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -3,6 +3,12 @@ #include +enum microcode_match_result { + OLD_UCODE, /* signature matched, but revision id is older or equal */ + NEW_UCODE, /* signature matched, but revision id is newer */ + MIS_UCODE, /* signature mismatched */ +}; + struct cpu_signature; struct ucode_cpu_info; From patchwork Thu Sep 12 07:22:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142469 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 72FBC13BD for ; Thu, 12 Sep 2019 07:20:22 +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 58D1721479 for ; Thu, 12 Sep 2019 07:20:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 58D1721479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNG-0001PH-AZ; Thu, 12 Sep 2019 07:18:38 +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 1i8JNF-0001P9-98 for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:37 +0000 X-Inumbo-ID: 88e1f85e-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 88e1f85e-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:36 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906208" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:33 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:15 +0800 Message-Id: <1568272949-1086-3-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 02/16] microcode/amd: distinguish old and mismatched ucode in microcode_fits() 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Sometimes, an ucode with a level lower than or equal to current CPU's patch level is useful. For example, to work around a broken bios which only loads ucode for BSP, when BSP parses an ucode blob during bootup, it is better to save an ucode with lower or equal level for APs No functional change is made in this patch. But following patch would handle "old ucode" and "mismatched ucode" separately. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v8: - new --- xen/arch/x86/microcode_amd.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 9b74330..7fa700b 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -152,8 +152,8 @@ static bool_t find_equiv_cpu_id(const struct equiv_cpu_entry *equiv_cpu_table, return 0; } -static bool_t microcode_fits(const struct microcode_amd *mc_amd, - unsigned int cpu) +static enum microcode_match_result microcode_fits( + const struct microcode_amd *mc_amd, unsigned int cpu) { struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); const struct microcode_header_amd *mc_header = mc_amd->mpb; @@ -167,27 +167,27 @@ static bool_t microcode_fits(const struct microcode_amd *mc_amd, current_cpu_id = cpuid_eax(0x00000001); if ( !find_equiv_cpu_id(equiv_cpu_table, current_cpu_id, &equiv_cpu_id) ) - return 0; + return MIS_UCODE; if ( (mc_header->processor_rev_id) != equiv_cpu_id ) - return 0; + return MIS_UCODE; if ( !verify_patch_size(mc_amd->mpb_size) ) { pr_debug("microcode: patch size mismatch\n"); - return 0; + return MIS_UCODE; } if ( mc_header->patch_id <= uci->cpu_sig.rev ) { pr_debug("microcode: patch is already at required level or greater.\n"); - return 0; + return OLD_UCODE; } pr_debug("microcode: CPU%d found a matching microcode update with version %#x (current=%#x)\n", cpu, mc_header->patch_id, uci->cpu_sig.rev); - return 1; + return NEW_UCODE; } static int apply_microcode(unsigned int cpu) @@ -496,7 +496,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize, &offset)) == 0 ) { - if ( microcode_fits(mc_amd, cpu) ) + if ( microcode_fits(mc_amd, cpu) == NEW_UCODE ) { error = apply_microcode(cpu); if ( error ) @@ -579,7 +579,7 @@ static int microcode_resume_match(unsigned int cpu, const void *mc) struct microcode_amd *mc_amd = uci->mc.mc_amd; const struct microcode_amd *src = mc; - if ( !microcode_fits(src, cpu) ) + if ( microcode_fits(src, cpu) != NEW_UCODE ) return 0; if ( src != mc_amd ) From patchwork Thu Sep 12 07:22:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142475 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 159781599 for ; Thu, 12 Sep 2019 07:20:31 +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 E45D421479 for ; Thu, 12 Sep 2019 07:20:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E45D421479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNL-0001Pu-Vc; Thu, 12 Sep 2019 07:18:43 +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 1i8JNK-0001Pk-6U for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:42 +0000 X-Inumbo-ID: 8b0c5fac-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8b0c5fac-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:39 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906215" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:36 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:16 +0800 Message-Id: <1568272949-1086-4-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v10 03/16] microcode: introduce a global cache of ucode patch 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" to replace the current per-cpu cache 'uci->mc'. With the assumption that all CPUs in the system have the same signature (family, model, stepping and 'pf'), one microcode update matches with one cpu should match with others. Having differing microcode revisions on cpus would cause system unstable and should be avoided. Hence, caching one microcode update is good enough for all cases. Introduce a global variable, microcode_cache, to store the newest matching microcode update. Whenever we get a new valid microcode update, its revision id is compared against that of the microcode update to determine whether the "microcode_cache" needs to be replaced. And this global cache is loaded to cpu in apply_microcode(). All operations on the cache is protected by 'microcode_mutex'. Note that I deliberately avoid touching the old per-cpu cache ('uci->mc') as I am going to remove it completely in the following patches. We copy everything to create the new cache blob to avoid reusing some buffers previously allocated for the old per-cpu cache. It is not so efficient, but it is already corrected by a patch later in this series. Signed-off-by: Chao Gao Reviewed-by: Roger Pau MonnĂ© Acked-by: Jan Beulich --- Changes in v10: - assert mismatched ucode won't be passed to ->compare_patch. - return -ENOENT if patch is NULL in .apply_microcode(). - check against NULL pointer dereference in free_patch() on AMD side - cosmetic changes suggested by Roger and Jan. Changes in v9: - on Intel side, ->compare_patch just checks the patch revision number. - explain why all buffers are copied in alloc_microcode_patch() in patch description. Changes in v8: - Free generic wrapper struct in general code - Try to update cache as long as a patch covers current cpu. Previsouly, cache is updated only if the patch is newer than current update revision in the CPU. The small difference can work around a broken bios which only applies microcode update to BSP and software has to apply the same update to other CPUs. Changes in v7: - reworked to cache only one microcode patch rather than a list of microcode patches. --- xen/arch/x86/microcode.c | 38 ++++++++++++++++ xen/arch/x86/microcode_amd.c | 98 ++++++++++++++++++++++++++++++++++++++--- xen/arch/x86/microcode_intel.c | 81 +++++++++++++++++++++++++++------- xen/include/asm-x86/microcode.h | 16 +++++++ 4 files changed, 211 insertions(+), 22 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 421d57e..e218a9d 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -61,6 +61,9 @@ static struct ucode_mod_blob __initdata ucode_blob; */ static bool_t __initdata ucode_scan; +/* Protected by microcode_mutex */ +static struct microcode_patch *microcode_cache; + void __init microcode_set_module(unsigned int idx) { ucode_mod_idx = idx; @@ -262,6 +265,41 @@ int microcode_resume_cpu(unsigned int cpu) return err; } +void microcode_free_patch(struct microcode_patch *microcode_patch) +{ + microcode_ops->free_patch(microcode_patch->mc); + xfree(microcode_patch); +} + +const struct microcode_patch *microcode_get_cache(void) +{ + ASSERT(spin_is_locked(µcode_mutex)); + + return microcode_cache; +} + +/* Return true if cache gets updated. Otherwise, return false */ +bool microcode_update_cache(struct microcode_patch *patch) +{ + ASSERT(spin_is_locked(µcode_mutex)); + + if ( !microcode_cache ) + microcode_cache = patch; + else if ( microcode_ops->compare_patch(patch, + microcode_cache) == NEW_UCODE ) + { + microcode_free_patch(microcode_cache); + microcode_cache = patch; + } + else + { + microcode_free_patch(patch); + return false; + } + + return true; +} + static int microcode_update_cpu(const void *buf, size_t size) { int err; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 7fa700b..2dca1df 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -190,25 +190,92 @@ static enum microcode_match_result microcode_fits( return NEW_UCODE; } +static bool match_cpu(const struct microcode_patch *patch) +{ + if ( !patch ) + return false; + return microcode_fits(patch->mc_amd, smp_processor_id()) == NEW_UCODE; +} + +static struct microcode_patch *alloc_microcode_patch( + const struct microcode_amd *mc_amd) +{ + struct microcode_patch *microcode_patch = xmalloc(struct microcode_patch); + struct microcode_amd *cache = xmalloc(struct microcode_amd); + void *mpb = xmalloc_bytes(mc_amd->mpb_size); + struct equiv_cpu_entry *equiv_cpu_table = + xmalloc_bytes(mc_amd->equiv_cpu_table_size); + + if ( !microcode_patch || !cache || !mpb || !equiv_cpu_table ) + { + xfree(microcode_patch); + xfree(cache); + xfree(mpb); + xfree(equiv_cpu_table); + return ERR_PTR(-ENOMEM); + } + + memcpy(mpb, mc_amd->mpb, mc_amd->mpb_size); + cache->mpb = mpb; + cache->mpb_size = mc_amd->mpb_size; + memcpy(equiv_cpu_table, mc_amd->equiv_cpu_table, + mc_amd->equiv_cpu_table_size); + cache->equiv_cpu_table = equiv_cpu_table; + cache->equiv_cpu_table_size = mc_amd->equiv_cpu_table_size; + microcode_patch->mc_amd = cache; + + return microcode_patch; +} + +static void free_patch(void *mc) +{ + struct microcode_amd *mc_amd = mc; + + if ( mc_amd ) + { + xfree(mc_amd->equiv_cpu_table); + xfree(mc_amd->mpb); + xfree(mc_amd); + } +} + +static enum microcode_match_result compare_patch( + const struct microcode_patch *new, const struct microcode_patch *old) +{ + const struct microcode_header_amd *new_header = new->mc_amd->mpb; + const struct microcode_header_amd *old_header = old->mc_amd->mpb; + + /* Both patches to compare are supposed to be applicable to local CPU. */ + ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE); + ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE); + + if ( new_header->processor_rev_id == old_header->processor_rev_id ) + return (new_header->patch_id > old_header->patch_id) ? + NEW_UCODE : OLD_UCODE; + + return MIS_UCODE; +} + static int apply_microcode(unsigned int cpu) { unsigned long flags; struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); uint32_t rev; - struct microcode_amd *mc_amd = uci->mc.mc_amd; - struct microcode_header_amd *hdr; int hw_err; + const struct microcode_header_amd *hdr; + const struct microcode_patch *patch = microcode_get_cache(); /* We should bind the task to the CPU */ BUG_ON(raw_smp_processor_id() != cpu); - if ( mc_amd == NULL ) - return -EINVAL; + if ( !patch ) + return -ENOENT; - hdr = mc_amd->mpb; - if ( hdr == NULL ) + if ( !match_cpu(patch) ) return -EINVAL; + hdr = patch->mc_amd->mpb; + spin_lock_irqsave(µcode_update_lock, flags); hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr); @@ -496,7 +563,21 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize, &offset)) == 0 ) { - if ( microcode_fits(mc_amd, cpu) == NEW_UCODE ) + struct microcode_patch *new_patch = alloc_microcode_patch(mc_amd); + + if ( IS_ERR(new_patch) ) + { + error = PTR_ERR(new_patch); + break; + } + + /* Update cache if this patch covers current CPU */ + if ( microcode_fits(new_patch->mc_amd, cpu) != MIS_UCODE ) + microcode_update_cache(new_patch); + else + microcode_free_patch(new_patch); + + if ( match_cpu(microcode_get_cache()) ) { error = apply_microcode(cpu); if ( error ) @@ -643,6 +724,9 @@ static const struct microcode_ops microcode_amd_ops = { .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, .start_update = start_update, + .free_patch = free_patch, + .compare_patch = compare_patch, + .match_cpu = match_cpu, }; int __init microcode_init_amd(void) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 1a3ffa5..eefc2d2 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -260,6 +260,36 @@ static enum microcode_match_result microcode_update_match( return MIS_UCODE; } +static bool match_cpu(const struct microcode_patch *patch) +{ + if ( !patch ) + return false; + + return microcode_update_match(&patch->mc_intel->hdr, + smp_processor_id()) == NEW_UCODE; +} + +static void free_patch(void *mc) +{ + xfree(mc); +} + +static enum microcode_match_result compare_patch( + const struct microcode_patch *new, const struct microcode_patch *old) +{ + /* + * Both patches to compare are supposed to be applicable to local CPU. + * Just compare the revision number. + */ + ASSERT(microcode_update_match(&old->mc_intel->hdr, smp_processor_id()) != + MIS_UCODE); + ASSERT(microcode_update_match(&new->mc_intel->hdr, smp_processor_id()) != + MIS_UCODE); + + return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE + : OLD_UCODE; +} + /* * return 0 - no update found * return 1 - found update @@ -270,10 +300,26 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); const struct microcode_header_intel *mc_header = mc; unsigned long total_size = get_totalsize(mc_header); - void *new_mc; + void *new_mc = xmalloc_bytes(total_size); + struct microcode_patch *new_patch = xmalloc(struct microcode_patch); - if ( microcode_update_match(mc, cpu) != NEW_UCODE ) + if ( !new_patch || !new_mc ) + { + xfree(new_patch); + xfree(new_mc); + return -ENOMEM; + } + memcpy(new_mc, mc, total_size); + new_patch->mc_intel = new_mc; + + /* Make sure that this patch covers current CPU */ + if ( microcode_update_match(mc, cpu) == MIS_UCODE ) + { + microcode_free_patch(new_patch); return 0; + } + + microcode_update_cache(new_patch); pr_debug("microcode: CPU%d found a matching microcode update with" " version %#x (current=%#x)\n", @@ -298,18 +344,25 @@ static int apply_microcode(unsigned int cpu) unsigned int val[2]; unsigned int cpu_num = raw_smp_processor_id(); struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num); + const struct microcode_intel *mc_intel; + const struct microcode_patch *patch = microcode_get_cache(); /* We should bind the task to the CPU */ BUG_ON(cpu_num != cpu); - if ( uci->mc.mc_intel == NULL ) + if ( !patch ) + return -ENOENT; + + if ( !match_cpu(patch) ) return -EINVAL; + mc_intel = patch->mc_intel; + /* serialize access to the physical write to MSR 0x79 */ spin_lock_irqsave(µcode_update_lock, flags); /* write microcode via MSR 0x79 */ - wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)uci->mc.mc_intel->bits); + wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits); wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL); /* As documented in the SDM: Do a CPUID 1 here */ @@ -320,19 +373,17 @@ static int apply_microcode(unsigned int cpu) val[1] = (uint32_t)(msr_content >> 32); spin_unlock_irqrestore(µcode_update_lock, flags); - if ( val[1] != uci->mc.mc_intel->hdr.rev ) + if ( val[1] != mc_intel->hdr.rev ) { printk(KERN_ERR "microcode: CPU%d update from revision " "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num, - uci->cpu_sig.rev, uci->mc.mc_intel->hdr.rev, val[1]); + uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]); return -EIO; } printk(KERN_INFO "microcode: CPU%d updated from revision " "%#x to %#x, date = %04x-%02x-%02x \n", - cpu_num, uci->cpu_sig.rev, val[1], - uci->mc.mc_intel->hdr.year, - uci->mc.mc_intel->hdr.month, - uci->mc.mc_intel->hdr.day); + cpu_num, uci->cpu_sig.rev, val[1], mc_intel->hdr.year, + mc_intel->hdr.month, mc_intel->hdr.day); uci->cpu_sig.rev = val[1]; return 0; @@ -372,7 +423,6 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, long offset = 0; int error = 0; void *mc; - unsigned int matching_count = 0; /* We should bind the task to the CPU */ BUG_ON(cpu != raw_smp_processor_id()); @@ -390,10 +440,8 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, * lets keep searching till the latest version */ if ( error == 1 ) - { - matching_count++; error = 0; - } + xfree(mc); } if ( offset > 0 ) @@ -401,7 +449,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, if ( offset < 0 ) error = offset; - if ( !error && matching_count ) + if ( !error && match_cpu(microcode_get_cache()) ) error = apply_microcode(cpu); return error; @@ -417,6 +465,9 @@ static const struct microcode_ops microcode_intel_ops = { .cpu_request_microcode = cpu_request_microcode, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, + .free_patch = free_patch, + .compare_patch = compare_patch, + .match_cpu = match_cpu, }; int __init microcode_init_intel(void) diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 882f560..4d45401 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -12,6 +12,14 @@ enum microcode_match_result { struct cpu_signature; struct ucode_cpu_info; +struct microcode_patch { + union { + struct microcode_intel *mc_intel; + struct microcode_amd *mc_amd; + void *mc; + }; +}; + struct microcode_ops { int (*microcode_resume_match)(unsigned int cpu, const void *mc); int (*cpu_request_microcode)(unsigned int cpu, const void *buf, @@ -19,6 +27,10 @@ struct microcode_ops { int (*collect_cpu_info)(unsigned int cpu, struct cpu_signature *csig); int (*apply_microcode)(unsigned int cpu); int (*start_update)(void); + void (*free_patch)(void *mc); + bool (*match_cpu)(const struct microcode_patch *patch); + enum microcode_match_result (*compare_patch)( + const struct microcode_patch *new, const struct microcode_patch *old); }; struct cpu_signature { @@ -39,4 +51,8 @@ struct ucode_cpu_info { DECLARE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info); extern const struct microcode_ops *microcode_ops; +const struct microcode_patch *microcode_get_cache(void); +bool microcode_update_cache(struct microcode_patch *patch); +void microcode_free_patch(struct microcode_patch *patch); + #endif /* ASM_X86__MICROCODE_H */ From patchwork Thu Sep 12 07:22:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142481 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 6317313BD for ; Thu, 12 Sep 2019 07:20:48 +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 48E4421479 for ; Thu, 12 Sep 2019 07:20:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 48E4421479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNQ-0001Qp-ER; Thu, 12 Sep 2019 07:18:48 +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 1i8JNP-0001QW-5H for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:47 +0000 X-Inumbo-ID: 8c4cf6d8-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8c4cf6d8-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:41 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906226" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:39 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:17 +0800 Message-Id: <1568272949-1086-5-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 04/16] microcode: clean up microcode_resume_cpu 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Previously, a per-cpu ucode cache is maintained. Then each CPU had one per-cpu update cache and there might be multiple versions of microcode. Thus microcode_resume_cpu tried best to update microcode by loading every update cache until a successful load. But now the cache struct is simplified a lot and only a single ucode is cached. a single invocation of ->apply_microcode() would load the cache and make microcode updated. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- changes in v8: - new - separated from the following patch --- xen/arch/x86/microcode.c | 40 ++--------------------------------- xen/arch/x86/microcode_amd.c | 47 ----------------------------------------- xen/arch/x86/microcode_intel.c | 6 ------ xen/include/asm-x86/microcode.h | 1 - 4 files changed, 2 insertions(+), 92 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index e218a9d..922b94f 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -215,8 +215,6 @@ int microcode_resume_cpu(unsigned int cpu) { int err; struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); - struct cpu_signature nsig; - unsigned int cpu2; if ( !microcode_ops ) return 0; @@ -224,42 +222,8 @@ int microcode_resume_cpu(unsigned int cpu) spin_lock(µcode_mutex); err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); - if ( err ) - { - __microcode_fini_cpu(cpu); - spin_unlock(µcode_mutex); - return err; - } - - if ( uci->mc.mc_valid ) - { - err = microcode_ops->microcode_resume_match(cpu, uci->mc.mc_valid); - if ( err >= 0 ) - { - if ( err ) - err = microcode_ops->apply_microcode(cpu); - spin_unlock(µcode_mutex); - return err; - } - } - - nsig = uci->cpu_sig; - __microcode_fini_cpu(cpu); - uci->cpu_sig = nsig; - - err = -EIO; - for_each_online_cpu ( cpu2 ) - { - uci = &per_cpu(ucode_cpu_info, cpu2); - if ( uci->mc.mc_valid && - microcode_ops->microcode_resume_match(cpu, uci->mc.mc_valid) > 0 ) - { - err = microcode_ops->apply_microcode(cpu); - break; - } - } - - __microcode_fini_cpu(cpu); + if ( likely(!err) ) + err = microcode_ops->apply_microcode(cpu); spin_unlock(µcode_mutex); return err; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 2dca1df..04b00aa 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -654,52 +654,6 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, return error; } -static int microcode_resume_match(unsigned int cpu, const void *mc) -{ - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); - struct microcode_amd *mc_amd = uci->mc.mc_amd; - const struct microcode_amd *src = mc; - - if ( microcode_fits(src, cpu) != NEW_UCODE ) - return 0; - - if ( src != mc_amd ) - { - if ( mc_amd ) - { - xfree(mc_amd->equiv_cpu_table); - xfree(mc_amd->mpb); - xfree(mc_amd); - } - - mc_amd = xmalloc(struct microcode_amd); - uci->mc.mc_amd = mc_amd; - if ( !mc_amd ) - return -ENOMEM; - mc_amd->equiv_cpu_table = xmalloc_bytes(src->equiv_cpu_table_size); - if ( !mc_amd->equiv_cpu_table ) - goto err1; - mc_amd->mpb = xmalloc_bytes(src->mpb_size); - if ( !mc_amd->mpb ) - goto err2; - - mc_amd->equiv_cpu_table_size = src->equiv_cpu_table_size; - mc_amd->mpb_size = src->mpb_size; - memcpy(mc_amd->mpb, src->mpb, src->mpb_size); - memcpy(mc_amd->equiv_cpu_table, src->equiv_cpu_table, - src->equiv_cpu_table_size); - } - - return 1; - -err2: - xfree(mc_amd->equiv_cpu_table); -err1: - xfree(mc_amd); - uci->mc.mc_amd = NULL; - return -ENOMEM; -} - static int start_update(void) { #if CONFIG_HVM @@ -719,7 +673,6 @@ static int start_update(void) } static const struct microcode_ops microcode_amd_ops = { - .microcode_resume_match = microcode_resume_match, .cpu_request_microcode = cpu_request_microcode, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index eefc2d2..97f759e 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -455,13 +455,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, return error; } -static int microcode_resume_match(unsigned int cpu, const void *mc) -{ - return get_matching_microcode(mc, cpu); -} - static const struct microcode_ops microcode_intel_ops = { - .microcode_resume_match = microcode_resume_match, .cpu_request_microcode = cpu_request_microcode, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 4d45401..da0b156 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -21,7 +21,6 @@ struct microcode_patch { }; struct microcode_ops { - int (*microcode_resume_match)(unsigned int cpu, const void *mc); int (*cpu_request_microcode)(unsigned int cpu, const void *buf, size_t size); int (*collect_cpu_info)(unsigned int cpu, struct cpu_signature *csig); From patchwork Thu Sep 12 07:22:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142467 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 C58501599 for ; Thu, 12 Sep 2019 07:20:20 +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 9F1D121479 for ; Thu, 12 Sep 2019 07:20:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9F1D121479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNV-0001TH-PO; Thu, 12 Sep 2019 07:18:53 +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 1i8JNU-0001Rk-77 for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:52 +0000 X-Inumbo-ID: 8e10dee4-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8e10dee4-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:44 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906239" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:42 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:18 +0800 Message-Id: <1568272949-1086-6-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 05/16] microcode: remove struct ucode_cpu_info 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Remove the per-cpu cache field in struct ucode_cpu_info since it has been replaced by a global cache. It would leads to only one field remaining in ucode_cpu_info. Then, this struct is removed and the remaining field (cpu signature) is stored in per-cpu area. The cpu status notifier is also removed. It was used to free the "mc" field to avoid memory leak. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v9: - rebase and fix conflict Changes in v8: - split microcode_resume_cpu() cleanup to a separate patch. Changes in v6: - remove the whole struct ucode_cpu_info instead of the per-cpu cache in it. --- xen/arch/x86/apic.c | 2 +- xen/arch/x86/microcode.c | 57 +++++++-------------------------------- xen/arch/x86/microcode_amd.c | 59 +++++++++-------------------------------- xen/arch/x86/microcode_intel.c | 28 +++++++------------ xen/arch/x86/spec_ctrl.c | 2 +- xen/include/asm-x86/microcode.h | 12 +-------- 6 files changed, 34 insertions(+), 126 deletions(-) diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c index ea0d561..6cdb50c 100644 --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -1190,7 +1190,7 @@ static void __init check_deadline_errata(void) else rev = (unsigned long)m->driver_data; - if ( this_cpu(ucode_cpu_info).cpu_sig.rev >= rev ) + if ( this_cpu(cpu_sig).rev >= rev ) return; setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE); diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 922b94f..d17dbec 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -187,7 +187,7 @@ const struct microcode_ops *microcode_ops; static DEFINE_SPINLOCK(microcode_mutex); -DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info); +DEFINE_PER_CPU(struct cpu_signature, cpu_sig); struct microcode_info { unsigned int cpu; @@ -196,32 +196,17 @@ struct microcode_info { char buffer[1]; }; -static void __microcode_fini_cpu(unsigned int cpu) -{ - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); - - xfree(uci->mc.mc_valid); - memset(uci, 0, sizeof(*uci)); -} - -static void microcode_fini_cpu(unsigned int cpu) -{ - spin_lock(µcode_mutex); - __microcode_fini_cpu(cpu); - spin_unlock(µcode_mutex); -} - int microcode_resume_cpu(unsigned int cpu) { int err; - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); if ( !microcode_ops ) return 0; spin_lock(µcode_mutex); - err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + err = microcode_ops->collect_cpu_info(cpu, sig); if ( likely(!err) ) err = microcode_ops->apply_microcode(cpu); spin_unlock(µcode_mutex); @@ -268,16 +253,13 @@ static int microcode_update_cpu(const void *buf, size_t size) { int err; unsigned int cpu = smp_processor_id(); - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); spin_lock(µcode_mutex); - err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + err = microcode_ops->collect_cpu_info(cpu, sig); if ( likely(!err) ) err = microcode_ops->cpu_request_microcode(cpu, buf, size); - else - __microcode_fini_cpu(cpu); - spin_unlock(µcode_mutex); return err; @@ -364,29 +346,10 @@ static int __init microcode_init(void) } __initcall(microcode_init); -static int microcode_percpu_callback( - struct notifier_block *nfb, unsigned long action, void *hcpu) -{ - unsigned int cpu = (unsigned long)hcpu; - - switch ( action ) - { - case CPU_DEAD: - microcode_fini_cpu(cpu); - break; - } - - return NOTIFY_DONE; -} - -static struct notifier_block microcode_percpu_nfb = { - .notifier_call = microcode_percpu_callback, -}; - int __init early_microcode_update_cpu(bool start_update) { unsigned int cpu = smp_processor_id(); - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); int rc = 0; void *data = NULL; size_t len; @@ -405,7 +368,7 @@ int __init early_microcode_update_cpu(bool start_update) data = bootstrap_map(&ucode_mod); } - microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + microcode_ops->collect_cpu_info(cpu, sig); if ( data ) { @@ -424,7 +387,7 @@ int __init early_microcode_update_cpu(bool start_update) int __init early_microcode_init(void) { unsigned int cpu = smp_processor_id(); - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); int rc; rc = microcode_init_intel(); @@ -437,12 +400,10 @@ int __init early_microcode_init(void) if ( microcode_ops ) { - microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + microcode_ops->collect_cpu_info(cpu, sig); if ( ucode_mod.mod_end || ucode_blob.size ) rc = early_microcode_update_cpu(true); - - register_cpu_notifier(µcode_percpu_nfb); } return rc; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 04b00aa..69c9cfe 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -155,7 +155,7 @@ static bool_t find_equiv_cpu_id(const struct equiv_cpu_entry *equiv_cpu_table, static enum microcode_match_result microcode_fits( const struct microcode_amd *mc_amd, unsigned int cpu) { - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_header_amd *mc_header = mc_amd->mpb; const struct equiv_cpu_entry *equiv_cpu_table = mc_amd->equiv_cpu_table; unsigned int current_cpu_id; @@ -178,14 +178,14 @@ static enum microcode_match_result microcode_fits( return MIS_UCODE; } - if ( mc_header->patch_id <= uci->cpu_sig.rev ) + if ( mc_header->patch_id <= sig->rev ) { pr_debug("microcode: patch is already at required level or greater.\n"); return OLD_UCODE; } pr_debug("microcode: CPU%d found a matching microcode update with version %#x (current=%#x)\n", - cpu, mc_header->patch_id, uci->cpu_sig.rev); + cpu, mc_header->patch_id, sig->rev); return NEW_UCODE; } @@ -259,9 +259,9 @@ static enum microcode_match_result compare_patch( static int apply_microcode(unsigned int cpu) { unsigned long flags; - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); uint32_t rev; int hw_err; + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_header_amd *hdr; const struct microcode_patch *patch = microcode_get_cache(); @@ -300,9 +300,9 @@ static int apply_microcode(unsigned int cpu) } printk(KERN_WARNING "microcode: CPU%d updated from revision %#x to %#x\n", - cpu, uci->cpu_sig.rev, hdr->patch_id); + cpu, sig->rev, hdr->patch_id); - uci->cpu_sig.rev = rev; + sig->rev = rev; return 0; } @@ -448,14 +448,14 @@ static bool_t check_final_patch_levels(unsigned int cpu) * any of the 'final_levels', then we should not update the microcode * patch on the cpu as system will hang otherwise. */ - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); unsigned int i; if ( boot_cpu_data.x86 != 0x10 ) return 0; for ( i = 0; i < ARRAY_SIZE(final_levels); i++ ) - if ( uci->cpu_sig.rev == final_levels[i] ) + if ( sig->rev == final_levels[i] ) return 1; return 0; @@ -464,13 +464,12 @@ static bool_t check_final_patch_levels(unsigned int cpu) static int cpu_request_microcode(unsigned int cpu, const void *buf, size_t bufsize) { - struct microcode_amd *mc_amd, *mc_old; + struct microcode_amd *mc_amd; size_t offset = 0; - size_t last_offset, applied_offset = 0; - int error = 0, save_error = 1; - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); + int error = 0; unsigned int current_cpu_id; unsigned int equiv_cpu_id; + const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); /* We should bind the task to the CPU */ BUG_ON(cpu != raw_smp_processor_id()); @@ -539,7 +538,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, { printk(KERN_ERR "microcode: CPU%d incorrect or corrupt container file\n" "microcode: Failed to update patch level. " - "Current lvl:%#x\n", cpu, uci->cpu_sig.rev); + "Current lvl:%#x\n", cpu, sig->rev); break; } } @@ -551,15 +550,10 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, goto out; } - mc_old = uci->mc.mc_amd; - /* implicitely validates uci->mc.mc_valid */ - uci->mc.mc_amd = mc_amd; - /* * It's possible the data file has multiple matching ucode, * lets keep searching till the latest version */ - last_offset = offset; while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize, &offset)) == 0 ) { @@ -582,11 +576,8 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, error = apply_microcode(cpu); if ( error ) break; - applied_offset = last_offset; } - last_offset = offset; - if ( offset >= bufsize ) break; @@ -614,31 +605,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, *(const uint32_t *)(buf + offset) == UCODE_MAGIC ) break; } - - /* On success keep the microcode patch for - * re-apply on resume. - */ - if ( applied_offset ) - { - save_error = get_ucode_from_buffer_amd( - mc_amd, buf, bufsize, &applied_offset); - - if ( save_error ) - error = save_error; - } - - if ( save_error ) - { - uci->mc.mc_amd = mc_old; - mc_old = mc_amd; - } - - if ( mc_old ) - { - xfree(mc_old->mpb); - xfree(mc_old->equiv_cpu_table); - xfree(mc_old); - } + free_patch(mc_amd); out: #if CONFIG_HVM diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 97f759e..f63e4bd 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -231,10 +231,10 @@ static enum microcode_match_result microcode_update_match( const struct extended_sigtable *ext_header; const struct extended_signature *ext_sig; unsigned int i; - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); - unsigned int sig = uci->cpu_sig.sig; - unsigned int pf = uci->cpu_sig.pf; - unsigned int rev = uci->cpu_sig.rev; + struct cpu_signature *cpu_sig = &per_cpu(cpu_sig, cpu); + unsigned int sig = cpu_sig->sig; + unsigned int pf = cpu_sig->pf; + unsigned int rev = cpu_sig->rev; unsigned long data_size = get_datasize(mc_header); const void *end = (const void *)mc_header + get_totalsize(mc_header); @@ -297,7 +297,6 @@ static enum microcode_match_result compare_patch( */ static int get_matching_microcode(const void *mc, unsigned int cpu) { - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); const struct microcode_header_intel *mc_header = mc; unsigned long total_size = get_totalsize(mc_header); void *new_mc = xmalloc_bytes(total_size); @@ -323,17 +322,8 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) pr_debug("microcode: CPU%d found a matching microcode update with" " version %#x (current=%#x)\n", - cpu, mc_header->rev, uci->cpu_sig.rev); - new_mc = xmalloc_bytes(total_size); - if ( new_mc == NULL ) - { - printk(KERN_ERR "microcode: error! Can not allocate memory\n"); - return -ENOMEM; - } + cpu, mc_header->rev, per_cpu(cpu_sig, cpu).rev); - memcpy(new_mc, mc, total_size); - xfree(uci->mc.mc_intel); - uci->mc.mc_intel = new_mc; return 1; } @@ -343,7 +333,7 @@ static int apply_microcode(unsigned int cpu) uint64_t msr_content; unsigned int val[2]; unsigned int cpu_num = raw_smp_processor_id(); - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num); + struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_intel *mc_intel; const struct microcode_patch *patch = microcode_get_cache(); @@ -377,14 +367,14 @@ static int apply_microcode(unsigned int cpu) { printk(KERN_ERR "microcode: CPU%d update from revision " "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num, - uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]); + sig->rev, mc_intel->hdr.rev, val[1]); return -EIO; } printk(KERN_INFO "microcode: CPU%d updated from revision " "%#x to %#x, date = %04x-%02x-%02x \n", - cpu_num, uci->cpu_sig.rev, val[1], mc_intel->hdr.year, + cpu_num, sig->rev, val[1], mc_intel->hdr.year, mc_intel->hdr.month, mc_intel->hdr.day); - uci->cpu_sig.rev = val[1]; + sig->rev = val[1]; return 0; } diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index 468a847..4761be8 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -438,7 +438,7 @@ static bool __init check_smt_enabled(void) /* Calculate whether Retpoline is known-safe on this CPU. */ static bool __init retpoline_safe(uint64_t caps) { - unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev; + unsigned int ucode_rev = this_cpu(cpu_sig).rev; if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) ) return true; diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index da0b156..3f4c4be 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -10,7 +10,6 @@ enum microcode_match_result { }; struct cpu_signature; -struct ucode_cpu_info; struct microcode_patch { union { @@ -38,16 +37,7 @@ struct cpu_signature { unsigned int rev; }; -struct ucode_cpu_info { - struct cpu_signature cpu_sig; - union { - struct microcode_intel *mc_intel; - struct microcode_amd *mc_amd; - void *mc_valid; - } mc; -}; - -DECLARE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info); +DECLARE_PER_CPU(struct cpu_signature, cpu_sig); extern const struct microcode_ops *microcode_ops; const struct microcode_patch *microcode_get_cache(void); From patchwork Thu Sep 12 07:22:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142487 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 E15A813BD for ; Thu, 12 Sep 2019 07:20:52 +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 BBDFC21479 for ; Thu, 12 Sep 2019 07:20:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BBDFC21479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNb-0001W7-5o; Thu, 12 Sep 2019 07:18:59 +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 1i8JNZ-0001VH-5E for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:18:57 +0000 X-Inumbo-ID: 8fea8882-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8fea8882-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:47 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906249" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:45 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:19 +0800 Message-Id: <1568272949-1086-7-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 06/16] microcode: remove pointless 'cpu' parameter 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Some callbacks in microcode_ops or related functions take a cpu id parameter. But at current call sites, the cpu id parameter is always equal to current cpu id. Some of them even use an assertion to guarantee this. Remove this redundent 'cpu' parameter. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v9: - use a convenience variable 'cpu' in collect_cpu_info() on AMD side - rebase and fix conflicts Changes in v8: - Use current_cpu_data in collect_cpu_info() - keep the cpu parameter of check_final_patch_levels() - use smp_processor_id() in get_matching_microcode() rather than define a local variable and label it "__maybe_unused" --- xen/arch/x86/acpi/power.c | 2 +- xen/arch/x86/microcode.c | 20 ++++++++------------ xen/arch/x86/microcode_amd.c | 34 +++++++++++++--------------------- xen/arch/x86/microcode_intel.c | 41 +++++++++++++++-------------------------- xen/arch/x86/smpboot.c | 2 +- xen/include/asm-x86/microcode.h | 7 +++---- xen/include/asm-x86/processor.h | 2 +- 7 files changed, 42 insertions(+), 66 deletions(-) diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c index e3954ee..269b140 100644 --- a/xen/arch/x86/acpi/power.c +++ b/xen/arch/x86/acpi/power.c @@ -278,7 +278,7 @@ static int enter_state(u32 state) console_end_sync(); - microcode_resume_cpu(0); + microcode_resume_cpu(); if ( !recheck_cpu_features(0) ) panic("Missing previously available feature(s)\n"); diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index d17dbec..89a8d2b 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -196,19 +196,19 @@ struct microcode_info { char buffer[1]; }; -int microcode_resume_cpu(unsigned int cpu) +int microcode_resume_cpu(void) { int err; - struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); + struct cpu_signature *sig = &this_cpu(cpu_sig); if ( !microcode_ops ) return 0; spin_lock(µcode_mutex); - err = microcode_ops->collect_cpu_info(cpu, sig); + err = microcode_ops->collect_cpu_info(sig); if ( likely(!err) ) - err = microcode_ops->apply_microcode(cpu); + err = microcode_ops->apply_microcode(); spin_unlock(µcode_mutex); return err; @@ -257,9 +257,9 @@ static int microcode_update_cpu(const void *buf, size_t size) spin_lock(µcode_mutex); - err = microcode_ops->collect_cpu_info(cpu, sig); + err = microcode_ops->collect_cpu_info(sig); if ( likely(!err) ) - err = microcode_ops->cpu_request_microcode(cpu, buf, size); + err = microcode_ops->cpu_request_microcode(buf, size); spin_unlock(µcode_mutex); return err; @@ -348,8 +348,6 @@ __initcall(microcode_init); int __init early_microcode_update_cpu(bool start_update) { - unsigned int cpu = smp_processor_id(); - struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); int rc = 0; void *data = NULL; size_t len; @@ -368,7 +366,7 @@ int __init early_microcode_update_cpu(bool start_update) data = bootstrap_map(&ucode_mod); } - microcode_ops->collect_cpu_info(cpu, sig); + microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); if ( data ) { @@ -386,8 +384,6 @@ int __init early_microcode_update_cpu(bool start_update) int __init early_microcode_init(void) { - unsigned int cpu = smp_processor_id(); - struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); int rc; rc = microcode_init_intel(); @@ -400,7 +396,7 @@ int __init early_microcode_init(void) if ( microcode_ops ) { - microcode_ops->collect_cpu_info(cpu, sig); + microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); if ( ucode_mod.mod_end || ucode_blob.size ) rc = early_microcode_update_cpu(true); diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 69c9cfe..1d27c71 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -78,8 +78,9 @@ struct mpbhdr { static DEFINE_SPINLOCK(microcode_update_lock); /* See comment in start_update() for cases when this routine fails */ -static int collect_cpu_info(unsigned int cpu, struct cpu_signature *csig) +static int collect_cpu_info(struct cpu_signature *csig) { + unsigned int cpu = smp_processor_id(); struct cpuinfo_x86 *c = &cpu_data[cpu]; memset(csig, 0, sizeof(*csig)); @@ -153,17 +154,15 @@ static bool_t find_equiv_cpu_id(const struct equiv_cpu_entry *equiv_cpu_table, } static enum microcode_match_result microcode_fits( - const struct microcode_amd *mc_amd, unsigned int cpu) + const struct microcode_amd *mc_amd) { + unsigned int cpu = smp_processor_id(); const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_header_amd *mc_header = mc_amd->mpb; const struct equiv_cpu_entry *equiv_cpu_table = mc_amd->equiv_cpu_table; unsigned int current_cpu_id; unsigned int equiv_cpu_id; - /* We should bind the task to the CPU */ - BUG_ON(cpu != raw_smp_processor_id()); - current_cpu_id = cpuid_eax(0x00000001); if ( !find_equiv_cpu_id(equiv_cpu_table, current_cpu_id, &equiv_cpu_id) ) @@ -192,9 +191,7 @@ static enum microcode_match_result microcode_fits( static bool match_cpu(const struct microcode_patch *patch) { - if ( !patch ) - return false; - return microcode_fits(patch->mc_amd, smp_processor_id()) == NEW_UCODE; + return patch && (microcode_fits(patch->mc_amd) == NEW_UCODE); } static struct microcode_patch *alloc_microcode_patch( @@ -246,8 +243,8 @@ static enum microcode_match_result compare_patch( const struct microcode_header_amd *old_header = old->mc_amd->mpb; /* Both patches to compare are supposed to be applicable to local CPU. */ - ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE); - ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE); + ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE); + ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE); if ( new_header->processor_rev_id == old_header->processor_rev_id ) return (new_header->patch_id > old_header->patch_id) ? @@ -256,18 +253,16 @@ static enum microcode_match_result compare_patch( return MIS_UCODE; } -static int apply_microcode(unsigned int cpu) +static int apply_microcode(void) { unsigned long flags; uint32_t rev; int hw_err; + unsigned int cpu = smp_processor_id(); struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_header_amd *hdr; const struct microcode_patch *patch = microcode_get_cache(); - /* We should bind the task to the CPU */ - BUG_ON(raw_smp_processor_id() != cpu); - if ( !patch ) return -ENOENT; @@ -461,19 +456,16 @@ static bool_t check_final_patch_levels(unsigned int cpu) return 0; } -static int cpu_request_microcode(unsigned int cpu, const void *buf, - size_t bufsize) +static int cpu_request_microcode(const void *buf, size_t bufsize) { struct microcode_amd *mc_amd; size_t offset = 0; int error = 0; unsigned int current_cpu_id; unsigned int equiv_cpu_id; + unsigned int cpu = smp_processor_id(); const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); - /* We should bind the task to the CPU */ - BUG_ON(cpu != raw_smp_processor_id()); - current_cpu_id = cpuid_eax(0x00000001); if ( *(const uint32_t *)buf != UCODE_MAGIC ) @@ -566,14 +558,14 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, } /* Update cache if this patch covers current CPU */ - if ( microcode_fits(new_patch->mc_amd, cpu) != MIS_UCODE ) + if ( microcode_fits(new_patch->mc_amd) != MIS_UCODE ) microcode_update_cache(new_patch); else microcode_free_patch(new_patch); if ( match_cpu(microcode_get_cache()) ) { - error = apply_microcode(cpu); + error = apply_microcode(); if ( error ) break; } diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index f63e4bd..5f1ae2f 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -96,13 +96,12 @@ struct extended_sigtable { /* serialize access to the physical write to MSR 0x79 */ static DEFINE_SPINLOCK(microcode_update_lock); -static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) +static int collect_cpu_info(struct cpu_signature *csig) { + unsigned int cpu_num = smp_processor_id(); struct cpuinfo_x86 *c = &cpu_data[cpu_num]; uint64_t msr_content; - BUG_ON(cpu_num != smp_processor_id()); - memset(csig, 0, sizeof(*csig)); if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) ) @@ -226,12 +225,12 @@ static int microcode_sanity_check(const void *mc) /* Check an update against the CPU signature and current update revision */ static enum microcode_match_result microcode_update_match( - const struct microcode_header_intel *mc_header, unsigned int cpu) + const struct microcode_header_intel *mc_header) { const struct extended_sigtable *ext_header; const struct extended_signature *ext_sig; unsigned int i; - struct cpu_signature *cpu_sig = &per_cpu(cpu_sig, cpu); + struct cpu_signature *cpu_sig = &this_cpu(cpu_sig); unsigned int sig = cpu_sig->sig; unsigned int pf = cpu_sig->pf; unsigned int rev = cpu_sig->rev; @@ -265,8 +264,7 @@ static bool match_cpu(const struct microcode_patch *patch) if ( !patch ) return false; - return microcode_update_match(&patch->mc_intel->hdr, - smp_processor_id()) == NEW_UCODE; + return microcode_update_match(&patch->mc_intel->hdr) == NEW_UCODE; } static void free_patch(void *mc) @@ -281,10 +279,8 @@ static enum microcode_match_result compare_patch( * Both patches to compare are supposed to be applicable to local CPU. * Just compare the revision number. */ - ASSERT(microcode_update_match(&old->mc_intel->hdr, smp_processor_id()) != - MIS_UCODE); - ASSERT(microcode_update_match(&new->mc_intel->hdr, smp_processor_id()) != - MIS_UCODE); + ASSERT(microcode_update_match(&old->mc_intel->hdr) != MIS_UCODE); + ASSERT(microcode_update_match(&new->mc_intel->hdr) != MIS_UCODE); return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE : OLD_UCODE; @@ -295,7 +291,7 @@ static enum microcode_match_result compare_patch( * return 1 - found update * return < 0 - error */ -static int get_matching_microcode(const void *mc, unsigned int cpu) +static int get_matching_microcode(const void *mc) { const struct microcode_header_intel *mc_header = mc; unsigned long total_size = get_totalsize(mc_header); @@ -312,7 +308,7 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) new_patch->mc_intel = new_mc; /* Make sure that this patch covers current CPU */ - if ( microcode_update_match(mc, cpu) == MIS_UCODE ) + if ( microcode_update_match(mc) == MIS_UCODE ) { microcode_free_patch(new_patch); return 0; @@ -322,24 +318,21 @@ static int get_matching_microcode(const void *mc, unsigned int cpu) pr_debug("microcode: CPU%d found a matching microcode update with" " version %#x (current=%#x)\n", - cpu, mc_header->rev, per_cpu(cpu_sig, cpu).rev); + smp_processor_id(), mc_header->rev, this_cpu(cpu_sig).rev); return 1; } -static int apply_microcode(unsigned int cpu) +static int apply_microcode(void) { unsigned long flags; uint64_t msr_content; unsigned int val[2]; unsigned int cpu_num = raw_smp_processor_id(); - struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); + struct cpu_signature *sig = &this_cpu(cpu_sig); const struct microcode_intel *mc_intel; const struct microcode_patch *patch = microcode_get_cache(); - /* We should bind the task to the CPU */ - BUG_ON(cpu_num != cpu); - if ( !patch ) return -ENOENT; @@ -407,22 +400,18 @@ static long get_next_ucode_from_buffer(void **mc, const u8 *buf, return offset + total_size; } -static int cpu_request_microcode(unsigned int cpu, const void *buf, - size_t size) +static int cpu_request_microcode(const void *buf, size_t size) { long offset = 0; int error = 0; void *mc; - /* We should bind the task to the CPU */ - BUG_ON(cpu != raw_smp_processor_id()); - while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 ) { error = microcode_sanity_check(mc); if ( error ) break; - error = get_matching_microcode(mc, cpu); + error = get_matching_microcode(mc); if ( error < 0 ) break; /* @@ -440,7 +429,7 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf, error = offset; if ( !error && match_cpu(microcode_get_cache()) ) - error = apply_microcode(cpu); + error = apply_microcode(); return error; } diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 911416c..73a1afc 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -363,7 +363,7 @@ void start_secondary(void *unused) if ( system_state <= SYS_STATE_smp_boot ) early_microcode_update_cpu(false); else - microcode_resume_cpu(cpu); + microcode_resume_cpu(); /* * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 3f4c4be..f2a5ea4 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -20,10 +20,9 @@ struct microcode_patch { }; struct microcode_ops { - int (*cpu_request_microcode)(unsigned int cpu, const void *buf, - size_t size); - int (*collect_cpu_info)(unsigned int cpu, struct cpu_signature *csig); - int (*apply_microcode)(unsigned int cpu); + int (*cpu_request_microcode)(const void *buf, size_t size); + int (*collect_cpu_info)(struct cpu_signature *csig); + int (*apply_microcode)(void); int (*start_update)(void); void (*free_patch)(void *mc); bool (*match_cpu)(const struct microcode_patch *patch); diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index 3660238..a673372 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -569,7 +569,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val); void microcode_set_module(unsigned int); int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len); -int microcode_resume_cpu(unsigned int cpu); +int microcode_resume_cpu(void); int early_microcode_update_cpu(bool start_update); int early_microcode_init(void); int microcode_init_intel(void); From patchwork Thu Sep 12 07:22:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142471 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 09BC313BD for ; Thu, 12 Sep 2019 07:20:26 +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 E3760214AF for ; Thu, 12 Sep 2019 07:20:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E3760214AF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNf-0001Z8-ND; Thu, 12 Sep 2019 07:19:03 +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 1i8JNe-0001YD-5p for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:02 +0000 X-Inumbo-ID: 917aef70-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 917aef70-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:50 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906264" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:48 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:20 +0800 Message-Id: <1568272949-1086-8-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v10 07/16] microcode/amd: call svm_host_osvw_init() in common code 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Introduce a vendor hook, .end_update_percpu, for svm_host_osvw_init(). The hook function is called on each cpu after loading an update. It is a preparation for spliting out apply_microcode() from cpu_request_microcode(). Note that svm_host_osvm_init() should be called regardless of the result of loading an update. Signed-off-by: Chao Gao Reviewed-by: Roger Pau MonnĂ© Reviewed-by: Jan Beulich --- Changes in v10: - rename end_update to end_update_percpu. - use #ifdef rather than #if and frame the implementation with Changes in v9: - call .end_update in early loading path - on AMD side, initialize .{start,end}_update only if "CONFIG_HVM" is true. --- xen/arch/x86/microcode.c | 10 +++++++++- xen/arch/x86/microcode_amd.c | 25 ++++++++++++------------- xen/include/asm-x86/microcode.h | 1 + 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 89a8d2b..5c82a2d 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -276,6 +276,9 @@ static long do_microcode_update(void *_info) if ( error ) info->error = error; + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); + info->cpu = cpumask_next(info->cpu, &cpu_online_map); if ( info->cpu < nr_cpu_ids ) return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); @@ -376,7 +379,12 @@ int __init early_microcode_update_cpu(bool start_update) if ( rc ) return rc; - return microcode_update_cpu(data, len); + rc = microcode_update_cpu(data, len); + + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); + + return rc; } else return -ENOMEM; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 1d27c71..c96a3b3 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -600,10 +600,6 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) free_patch(mc_amd); out: -#if CONFIG_HVM - svm_host_osvw_init(); -#endif - /* * In some cases we may return an error even if processor's microcode has * been updated. For example, the first patch in a container file is loaded @@ -613,29 +609,32 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) return error; } +#ifdef CONFIG_HVM static int start_update(void) { -#if CONFIG_HVM /* - * We assume here that svm_host_osvw_init() will be called on each cpu (from - * cpu_request_microcode()). - * - * Note that if collect_cpu_info() returns an error then - * cpu_request_microcode() will not invoked thus leaving OSVW bits not - * updated. Currently though collect_cpu_info() will not fail on processors - * supporting OSVW so we will not deal with this possibility. + * svm_host_osvw_init() will be called on each cpu by calling '.end_update' + * in common code. */ svm_host_osvw_reset(); -#endif return 0; } +static void end_update_percpu(void) +{ + svm_host_osvw_init(); +} +#endif + static const struct microcode_ops microcode_amd_ops = { .cpu_request_microcode = cpu_request_microcode, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, +#ifdef CONFIG_HVM .start_update = start_update, + .end_update_percpu = end_update_percpu, +#endif .free_patch = free_patch, .compare_patch = compare_patch, .match_cpu = match_cpu, diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index f2a5ea4..b0eee0e 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -24,6 +24,7 @@ struct microcode_ops { int (*collect_cpu_info)(struct cpu_signature *csig); int (*apply_microcode)(void); int (*start_update)(void); + void (*end_update_percpu)(void); void (*free_patch)(void *mc); bool (*match_cpu)(const struct microcode_patch *patch); enum microcode_match_result (*compare_patch)( From patchwork Thu Sep 12 07:22:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142491 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 C2CDF13BD for ; Thu, 12 Sep 2019 07:20:53 +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 A899620678 for ; Thu, 12 Sep 2019 07:20:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A899620678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNl-0001cq-1f; Thu, 12 Sep 2019 07:19:09 +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 1i8JNj-0001bl-59 for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:07 +0000 X-Inumbo-ID: 92b5caf5-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 92b5caf5-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:53 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906277" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:50 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:21 +0800 Message-Id: <1568272949-1086-9-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 08/16] microcode: pass a patch pointer to apply_microcode() 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" apply_microcode()'s always loading the cached ucode patch forces a patch to be stored before being loaded. Make apply_microcode() accept a patch pointer to remove the limitation so that a patch can be stored after a successful loading. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- xen/arch/x86/microcode.c | 2 +- xen/arch/x86/microcode_amd.c | 5 ++--- xen/arch/x86/microcode_intel.c | 5 ++--- xen/include/asm-x86/microcode.h | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 5c82a2d..b44e4d7 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -208,7 +208,7 @@ int microcode_resume_cpu(void) err = microcode_ops->collect_cpu_info(sig); if ( likely(!err) ) - err = microcode_ops->apply_microcode(); + err = microcode_ops->apply_microcode(microcode_cache); spin_unlock(µcode_mutex); return err; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index c96a3b3..c6d2ea3 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -253,7 +253,7 @@ static enum microcode_match_result compare_patch( return MIS_UCODE; } -static int apply_microcode(void) +static int apply_microcode(const struct microcode_patch *patch) { unsigned long flags; uint32_t rev; @@ -261,7 +261,6 @@ static int apply_microcode(void) unsigned int cpu = smp_processor_id(); struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); const struct microcode_header_amd *hdr; - const struct microcode_patch *patch = microcode_get_cache(); if ( !patch ) return -ENOENT; @@ -565,7 +564,7 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) if ( match_cpu(microcode_get_cache()) ) { - error = apply_microcode(); + error = apply_microcode(microcode_get_cache()); if ( error ) break; } diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 5f1ae2f..b1ec81d 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -323,7 +323,7 @@ static int get_matching_microcode(const void *mc) return 1; } -static int apply_microcode(void) +static int apply_microcode(const struct microcode_patch *patch) { unsigned long flags; uint64_t msr_content; @@ -331,7 +331,6 @@ static int apply_microcode(void) unsigned int cpu_num = raw_smp_processor_id(); struct cpu_signature *sig = &this_cpu(cpu_sig); const struct microcode_intel *mc_intel; - const struct microcode_patch *patch = microcode_get_cache(); if ( !patch ) return -ENOENT; @@ -429,7 +428,7 @@ static int cpu_request_microcode(const void *buf, size_t size) error = offset; if ( !error && match_cpu(microcode_get_cache()) ) - error = apply_microcode(); + error = apply_microcode(microcode_get_cache()); return error; } diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index b0eee0e..02feb09 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -22,7 +22,7 @@ struct microcode_patch { struct microcode_ops { int (*cpu_request_microcode)(const void *buf, size_t size); int (*collect_cpu_info)(struct cpu_signature *csig); - int (*apply_microcode)(void); + int (*apply_microcode)(const struct microcode_patch *patch); int (*start_update)(void); void (*end_update_percpu)(void); void (*free_patch)(void *mc); From patchwork Thu Sep 12 07:22:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142473 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 A399713BD for ; Thu, 12 Sep 2019 07:20: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 7DCF321479 for ; Thu, 12 Sep 2019 07:20:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7DCF321479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNq-0001g5-CA; Thu, 12 Sep 2019 07:19:14 +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 1i8JNo-0001ey-5Q for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:12 +0000 X-Inumbo-ID: 9450dadf-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 9450dadf-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:56 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906292" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:53 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:22 +0800 Message-Id: <1568272949-1086-10-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v10 09/16] microcode: split out apply_microcode() from cpu_request_microcode() 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" During late microcode loading, apply_microcode() is invoked in cpu_request_microcode(). To make late microcode update more reliable, we want to put the apply_microcode() into stop_machine context. So we split out it from cpu_request_microcode(). In general, for both early loading on BSP and late loading, cpu_request_microcode() is called first to get the matching microcode update contained by the blob and then apply_microcode() is invoked explicitly on each cpu in common code. Given that all CPUs are supposed to have the same signature, parsing microcode only needs to be done once. So cpu_request_microcode() is also moved out of microcode_update_cpu(). In some cases (e.g. a broken bios), the system may have multiple revisions of microcode update. So we would try to load a microcode update as long as it covers current cpu. And if a cpu loads this patch successfully, the patch would be stored into the patch cache. Signed-off-by: Chao Gao Reviewed-by: Roger Pau MonnĂ© --- Changes in v10: - make microcode_update_cache static - raise an error if loading ucode failed with -EIO - ensure end_update_percpu() is called following a successful call of start_update() Changes in v9: - remove the calling of ->compare_patch in microcode_update_cpu(). - drop "microcode_" prefix for static function - microcode_parse_blob(). - rebase and fix conflict Changes in v8: - divide the original patch into three patches to improve readability - load an update on each cpu as long as the update covers current cpu - store an update after the first successful loading on a CPU - Make sure the current CPU (especially pf value) is covered by updates. changes in v7: - to handle load failure, unvalidated patches won't be cached. They are passed as function arguments. So if update failed, we needn't any cleanup to microcode cache. --- xen/arch/x86/microcode.c | 182 +++++++++++++++++++++++++++------------- xen/arch/x86/microcode_amd.c | 38 +++++---- xen/arch/x86/microcode_intel.c | 66 +++++++-------- xen/include/asm-x86/microcode.h | 5 +- 4 files changed, 178 insertions(+), 113 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index b44e4d7..d4738f6 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -189,12 +189,19 @@ static DEFINE_SPINLOCK(microcode_mutex); DEFINE_PER_CPU(struct cpu_signature, cpu_sig); -struct microcode_info { - unsigned int cpu; - uint32_t buffer_size; - int error; - char buffer[1]; -}; +/* + * Return a patch that covers current CPU. If there are multiple patches, + * return the one with the highest revision number. Return error If no + * patch is found and an error occurs during the parsing process. Otherwise + * return NULL. + */ +static struct microcode_patch *parse_blob(const char *buf, size_t len) +{ + if ( likely(!microcode_ops->collect_cpu_info(&this_cpu(cpu_sig))) ) + return microcode_ops->cpu_request_microcode(buf, len); + + return NULL; +} int microcode_resume_cpu(void) { @@ -220,15 +227,8 @@ void microcode_free_patch(struct microcode_patch *microcode_patch) xfree(microcode_patch); } -const struct microcode_patch *microcode_get_cache(void) -{ - ASSERT(spin_is_locked(µcode_mutex)); - - return microcode_cache; -} - /* Return true if cache gets updated. Otherwise, return false */ -bool microcode_update_cache(struct microcode_patch *patch) +static bool microcode_update_cache(struct microcode_patch *patch) { ASSERT(spin_is_locked(µcode_mutex)); @@ -249,49 +249,80 @@ bool microcode_update_cache(struct microcode_patch *patch) return true; } -static int microcode_update_cpu(const void *buf, size_t size) +/* + * Load a microcode update to current CPU. + * + * If no patch is provided, the cached patch will be loaded. Microcode update + * during APs bringup and CPU resuming falls into this case. + */ +static int microcode_update_cpu(const struct microcode_patch *patch) { - int err; - unsigned int cpu = smp_processor_id(); - struct cpu_signature *sig = &per_cpu(cpu_sig, cpu); + int err = microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); - spin_lock(µcode_mutex); + if ( unlikely(err) ) + return err; - err = microcode_ops->collect_cpu_info(sig); - if ( likely(!err) ) - err = microcode_ops->cpu_request_microcode(buf, size); - spin_unlock(µcode_mutex); + if ( patch ) + err = microcode_ops->apply_microcode(patch); + else if ( microcode_cache ) + { + spin_lock(µcode_mutex); + err = microcode_ops->apply_microcode(microcode_cache); + if ( err == -EIO ) + { + microcode_free_patch(microcode_cache); + microcode_cache = NULL; + } + spin_unlock(µcode_mutex); + } + else + /* No patch to update */ + err = -ENOENT; return err; } -static long do_microcode_update(void *_info) +static long do_microcode_update(void *patch) { - struct microcode_info *info = _info; - int error; - - BUG_ON(info->cpu != smp_processor_id()); + unsigned int cpu; + int ret = microcode_update_cpu(patch); - error = microcode_update_cpu(info->buffer, info->buffer_size); - if ( error ) - info->error = error; + /* Store the patch after a successful loading */ + if ( !ret && patch ) + { + spin_lock(µcode_mutex); + microcode_update_cache(patch); + spin_unlock(µcode_mutex); + patch = NULL; + } if ( microcode_ops->end_update_percpu ) microcode_ops->end_update_percpu(); - info->cpu = cpumask_next(info->cpu, &cpu_online_map); - if ( info->cpu < nr_cpu_ids ) - return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); + /* + * Each thread tries to load ucode and only the first thread of a core + * would succeed. Ignore error other than -EIO. + */ + if ( ret != -EIO ) + ret = 0; + + cpu = cpumask_next(smp_processor_id(), &cpu_online_map); + if ( cpu < nr_cpu_ids ) + return continue_hypercall_on_cpu(cpu, do_microcode_update, patch) ? + : ret; - error = info->error; - xfree(info); - return error; + /* Free the patch if no CPU has loaded it successfully. */ + if ( patch ) + microcode_free_patch(patch); + + return ret; } int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) { int ret; - struct microcode_info *info; + void *buffer; + struct microcode_patch *patch; if ( len != (uint32_t)len ) return -E2BIG; @@ -299,32 +330,46 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) if ( microcode_ops == NULL ) return -EINVAL; - info = xmalloc_bytes(sizeof(*info) + len); - if ( info == NULL ) + buffer = xmalloc_bytes(len); + if ( !buffer ) return -ENOMEM; - ret = copy_from_guest(info->buffer, buf, len); - if ( ret != 0 ) + if ( copy_from_guest(buffer, buf, len) ) + { + ret = -EFAULT; + goto free; + } + + patch = parse_blob(buffer, len); + if ( IS_ERR(patch) ) { - xfree(info); - return ret; + ret = PTR_ERR(patch); + printk(XENLOG_WARNING "Parsing microcode blob error %d\n", ret); + goto free; } - info->buffer_size = len; - info->error = 0; - info->cpu = cpumask_first(&cpu_online_map); + if ( !patch ) + { + ret = -ENOENT; + goto free; + } if ( microcode_ops->start_update ) { ret = microcode_ops->start_update(); if ( ret != 0 ) { - xfree(info); - return ret; + microcode_free_patch(patch); + goto free; } } - return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); + ret = continue_hypercall_on_cpu(cpumask_first(&cpu_online_map), + do_microcode_update, patch); + + free: + xfree(buffer); + return ret; } static int __init microcode_init(void) @@ -371,23 +416,42 @@ int __init early_microcode_update_cpu(bool start_update) microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); - if ( data ) + if ( !data ) + return -ENOMEM; + + if ( start_update ) { - if ( start_update && microcode_ops->start_update ) + struct microcode_patch *patch; + + patch = parse_blob(data, len); + if ( IS_ERR(patch) ) + { + printk(XENLOG_WARNING "Parsing microcode blob error %ld\n", + PTR_ERR(patch)); + return PTR_ERR(patch); + } + + if ( !patch ) + return -ENOENT; + + spin_lock(µcode_mutex); + rc = microcode_update_cache(patch); + spin_unlock(µcode_mutex); + ASSERT(rc); + + if ( microcode_ops->start_update ) rc = microcode_ops->start_update(); if ( rc ) return rc; + } - rc = microcode_update_cpu(data, len); + rc = microcode_update_cpu(NULL); - if ( microcode_ops->end_update_percpu ) - microcode_ops->end_update_percpu(); + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); - return rc; - } - else - return -ENOMEM; + return rc; } int __init early_microcode_init(void) diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index c6d2ea3..1d1bea4 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -455,9 +455,11 @@ static bool_t check_final_patch_levels(unsigned int cpu) return 0; } -static int cpu_request_microcode(const void *buf, size_t bufsize) +static struct microcode_patch *cpu_request_microcode(const void *buf, + size_t bufsize) { struct microcode_amd *mc_amd; + struct microcode_patch *patch = NULL; size_t offset = 0; int error = 0; unsigned int current_cpu_id; @@ -556,19 +558,22 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) break; } - /* Update cache if this patch covers current CPU */ - if ( microcode_fits(new_patch->mc_amd) != MIS_UCODE ) - microcode_update_cache(new_patch); - else - microcode_free_patch(new_patch); - - if ( match_cpu(microcode_get_cache()) ) + /* + * If the new patch covers current CPU, compare patches and store the + * one with higher revision. + */ + if ( (microcode_fits(new_patch->mc_amd) != MIS_UCODE) && + (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) ) { - error = apply_microcode(microcode_get_cache()); - if ( error ) - break; + struct microcode_patch *tmp = patch; + + patch = new_patch; + new_patch = tmp; } + if ( new_patch ) + microcode_free_patch(new_patch); + if ( offset >= bufsize ) break; @@ -599,13 +604,10 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) free_patch(mc_amd); out: - /* - * In some cases we may return an error even if processor's microcode has - * been updated. For example, the first patch in a container file is loaded - * successfully but subsequent container file processing encounters a - * failure. - */ - return error; + if ( error && !patch ) + patch = ERR_PTR(error); + + return patch; } #ifdef CONFIG_HVM diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index b1ec81d..c3083d7 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -286,14 +286,9 @@ static enum microcode_match_result compare_patch( : OLD_UCODE; } -/* - * return 0 - no update found - * return 1 - found update - * return < 0 - error - */ -static int get_matching_microcode(const void *mc) +static struct microcode_patch *alloc_microcode_patch( + const struct microcode_header_intel *mc_header) { - const struct microcode_header_intel *mc_header = mc; unsigned long total_size = get_totalsize(mc_header); void *new_mc = xmalloc_bytes(total_size); struct microcode_patch *new_patch = xmalloc(struct microcode_patch); @@ -302,25 +297,12 @@ static int get_matching_microcode(const void *mc) { xfree(new_patch); xfree(new_mc); - return -ENOMEM; + return ERR_PTR(-ENOMEM); } - memcpy(new_mc, mc, total_size); + memcpy(new_mc, mc_header, total_size); new_patch->mc_intel = new_mc; - /* Make sure that this patch covers current CPU */ - if ( microcode_update_match(mc) == MIS_UCODE ) - { - microcode_free_patch(new_patch); - return 0; - } - - microcode_update_cache(new_patch); - - pr_debug("microcode: CPU%d found a matching microcode update with" - " version %#x (current=%#x)\n", - smp_processor_id(), mc_header->rev, this_cpu(cpu_sig).rev); - - return 1; + return new_patch; } static int apply_microcode(const struct microcode_patch *patch) @@ -399,26 +381,44 @@ static long get_next_ucode_from_buffer(void **mc, const u8 *buf, return offset + total_size; } -static int cpu_request_microcode(const void *buf, size_t size) +static struct microcode_patch *cpu_request_microcode(const void *buf, + size_t size) { long offset = 0; int error = 0; void *mc; + struct microcode_patch *patch = NULL; while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 ) { + struct microcode_patch *new_patch; + error = microcode_sanity_check(mc); if ( error ) break; - error = get_matching_microcode(mc); - if ( error < 0 ) + + new_patch = alloc_microcode_patch(mc); + if ( IS_ERR(new_patch) ) + { + error = PTR_ERR(new_patch); break; + } + /* - * It's possible the data file has multiple matching ucode, - * lets keep searching till the latest version + * If the new patch covers current CPU, compare patches and store the + * one with higher revision. */ - if ( error == 1 ) - error = 0; + if ( (microcode_update_match(&new_patch->mc_intel->hdr) != MIS_UCODE) && + (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) ) + { + struct microcode_patch *tmp = patch; + + patch = new_patch; + new_patch = tmp; + } + + if ( new_patch ) + microcode_free_patch(new_patch); xfree(mc); } @@ -427,10 +427,10 @@ static int cpu_request_microcode(const void *buf, size_t size) if ( offset < 0 ) error = offset; - if ( !error && match_cpu(microcode_get_cache()) ) - error = apply_microcode(microcode_get_cache()); + if ( error && !patch ) + patch = ERR_PTR(error); - return error; + return patch; } static const struct microcode_ops microcode_intel_ops = { diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 02feb09..7d5a1f8 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -20,7 +20,8 @@ struct microcode_patch { }; struct microcode_ops { - int (*cpu_request_microcode)(const void *buf, size_t size); + struct microcode_patch *(*cpu_request_microcode)(const void *buf, + size_t size); int (*collect_cpu_info)(struct cpu_signature *csig); int (*apply_microcode)(const struct microcode_patch *patch); int (*start_update)(void); @@ -40,8 +41,6 @@ struct cpu_signature { DECLARE_PER_CPU(struct cpu_signature, cpu_sig); extern const struct microcode_ops *microcode_ops; -const struct microcode_patch *microcode_get_cache(void); -bool microcode_update_cache(struct microcode_patch *patch); void microcode_free_patch(struct microcode_patch *patch); #endif /* ASM_X86__MICROCODE_H */ From patchwork Thu Sep 12 07:22:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142495 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 1E4B213BD for ; Thu, 12 Sep 2019 07:21:04 +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 01D6120678 for ; Thu, 12 Sep 2019 07:21:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 01D6120678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNt-0001iZ-S2; Thu, 12 Sep 2019 07:19:17 +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 1i8JNt-0001i4-5j for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:17 +0000 X-Inumbo-ID: 96bf3842-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 96bf3842-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:18:59 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:18:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906302" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:56 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:23 +0800 Message-Id: <1568272949-1086-11-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 10/16] microcode: unify ucode loading during system bootup and resuming 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" During system bootup and resuming, CPUs just load the cached ucode. So one unified function microcode_update_one() is introduced. It takes a boolean to indicate whether ->start_update should be called. Since early_microcode_update_cpu() is only called on BSP (APs call the unified function), start_update is always true and so remove this parameter. There is a functional change: ->start_update is called on BSP and ->end_update_percpu is called during system resuming. They are not invoked by previous microcode_resume_cpu(). Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v10: - call ->start_update for system resume from suspension Changes in v9: - return -EOPNOTSUPP rather than 0 if microcode_ops is NULL in microcode_update_one() - rebase and fix conflicts. Changes in v8: - split out from the previous patch --- xen/arch/x86/acpi/power.c | 2 +- xen/arch/x86/microcode.c | 91 +++++++++++++++++++---------------------- xen/arch/x86/smpboot.c | 5 +-- xen/include/asm-x86/processor.h | 4 +- 4 files changed, 45 insertions(+), 57 deletions(-) diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c index 269b140..01e6aec 100644 --- a/xen/arch/x86/acpi/power.c +++ b/xen/arch/x86/acpi/power.c @@ -278,7 +278,7 @@ static int enter_state(u32 state) console_end_sync(); - microcode_resume_cpu(); + microcode_update_one(true); if ( !recheck_cpu_features(0) ) panic("Missing previously available feature(s)\n"); diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index d4738f6..c2ea20f 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -203,24 +203,6 @@ static struct microcode_patch *parse_blob(const char *buf, size_t len) return NULL; } -int microcode_resume_cpu(void) -{ - int err; - struct cpu_signature *sig = &this_cpu(cpu_sig); - - if ( !microcode_ops ) - return 0; - - spin_lock(µcode_mutex); - - err = microcode_ops->collect_cpu_info(sig); - if ( likely(!err) ) - err = microcode_ops->apply_microcode(microcode_cache); - spin_unlock(µcode_mutex); - - return err; -} - void microcode_free_patch(struct microcode_patch *microcode_patch) { microcode_ops->free_patch(microcode_patch->mc); @@ -394,11 +376,38 @@ static int __init microcode_init(void) } __initcall(microcode_init); -int __init early_microcode_update_cpu(bool start_update) +/* Load a cached update to current cpu */ +int microcode_update_one(bool start_update) +{ + int err; + + if ( !microcode_ops ) + return -EOPNOTSUPP; + + microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); + + if ( start_update && microcode_ops->start_update ) + { + err = microcode_ops->start_update(); + if ( err ) + return err; + } + + err = microcode_update_cpu(NULL); + + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); + + return err; +} + +/* BSP calls this function to parse ucode blob and then apply an update. */ +int __init early_microcode_update_cpu(void) { int rc = 0; void *data = NULL; size_t len; + struct microcode_patch *patch; if ( !microcode_ops ) return -ENOSYS; @@ -414,44 +423,26 @@ int __init early_microcode_update_cpu(bool start_update) data = bootstrap_map(&ucode_mod); } - microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); - if ( !data ) return -ENOMEM; - if ( start_update ) + patch = parse_blob(data, len); + if ( IS_ERR(patch) ) { - struct microcode_patch *patch; - - patch = parse_blob(data, len); - if ( IS_ERR(patch) ) - { - printk(XENLOG_WARNING "Parsing microcode blob error %ld\n", - PTR_ERR(patch)); - return PTR_ERR(patch); - } - - if ( !patch ) - return -ENOENT; - - spin_lock(µcode_mutex); - rc = microcode_update_cache(patch); - spin_unlock(µcode_mutex); - ASSERT(rc); - - if ( microcode_ops->start_update ) - rc = microcode_ops->start_update(); - - if ( rc ) - return rc; + printk(XENLOG_WARNING "Parsing microcode blob error %ld\n", + PTR_ERR(patch)); + return PTR_ERR(patch); } - rc = microcode_update_cpu(NULL); + if ( !patch ) + return -ENOENT; - if ( microcode_ops->end_update_percpu ) - microcode_ops->end_update_percpu(); + spin_lock(µcode_mutex); + rc = microcode_update_cache(patch); + spin_unlock(µcode_mutex); + ASSERT(rc); - return rc; + return microcode_update_one(true); } int __init early_microcode_init(void) @@ -471,7 +462,7 @@ int __init early_microcode_init(void) microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); if ( ucode_mod.mod_end || ucode_blob.size ) - rc = early_microcode_update_cpu(true); + rc = early_microcode_update_cpu(); } return rc; diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 73a1afc..179f6b6 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -360,10 +360,7 @@ void start_secondary(void *unused) initialize_cpu_data(cpu); - if ( system_state <= SYS_STATE_smp_boot ) - early_microcode_update_cpu(false); - else - microcode_resume_cpu(); + microcode_update_one(false); /* * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index a673372..c92956f 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -569,9 +569,9 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val); void microcode_set_module(unsigned int); int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len); -int microcode_resume_cpu(void); -int early_microcode_update_cpu(bool start_update); +int early_microcode_update_cpu(void); int early_microcode_init(void); +int microcode_update_one(bool start_update); int microcode_init_intel(void); int microcode_init_amd(void); From patchwork Thu Sep 12 07:22:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142497 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 883401599 for ; Thu, 12 Sep 2019 07:21:05 +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 628FB20678 for ; Thu, 12 Sep 2019 07:21:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 628FB20678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JNz-0001mJ-7o; Thu, 12 Sep 2019 07:19:23 +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 1i8JNy-0001lX-6c for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:22 +0000 X-Inumbo-ID: 989afc8c-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 989afc8c-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:02 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906315" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:18:59 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:24 +0800 Message-Id: <1568272949-1086-12-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v10 11/16] microcode: reduce memory allocation and copy when creating a patch 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To create a microcode patch from a vendor-specific update, allocate_microcode_patch() copied everything from the update. It is not efficient. Essentially, we just need to go through ucodes in the blob, find the one with the newest revision and install it into the microcode_patch. In the process, buffers like mc_amd, equiv_cpu_table (on AMD side), and mc (on Intel side) can be reused. microcode_patch now is allocated after it is sure that there is a matching ucode. Signed-off-by: Chao Gao Reviewed-by: Roger Pau MonnĂ© Reviewed-by: Jan Beulich --- Changes in v10: - avoid unnecessary type casting * introduce compare_header on AMD side * specify the type of the first parameter of get_next_ucode_from_buffer() on Intel side Changes in v9: - new --- xen/arch/x86/microcode_amd.c | 112 +++++++++++++++++------------------------ xen/arch/x86/microcode_intel.c | 67 +++++++++--------------- 2 files changed, 69 insertions(+), 110 deletions(-) diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 1d1bea4..f05db72 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -194,36 +194,6 @@ static bool match_cpu(const struct microcode_patch *patch) return patch && (microcode_fits(patch->mc_amd) == NEW_UCODE); } -static struct microcode_patch *alloc_microcode_patch( - const struct microcode_amd *mc_amd) -{ - struct microcode_patch *microcode_patch = xmalloc(struct microcode_patch); - struct microcode_amd *cache = xmalloc(struct microcode_amd); - void *mpb = xmalloc_bytes(mc_amd->mpb_size); - struct equiv_cpu_entry *equiv_cpu_table = - xmalloc_bytes(mc_amd->equiv_cpu_table_size); - - if ( !microcode_patch || !cache || !mpb || !equiv_cpu_table ) - { - xfree(microcode_patch); - xfree(cache); - xfree(mpb); - xfree(equiv_cpu_table); - return ERR_PTR(-ENOMEM); - } - - memcpy(mpb, mc_amd->mpb, mc_amd->mpb_size); - cache->mpb = mpb; - cache->mpb_size = mc_amd->mpb_size; - memcpy(equiv_cpu_table, mc_amd->equiv_cpu_table, - mc_amd->equiv_cpu_table_size); - cache->equiv_cpu_table = equiv_cpu_table; - cache->equiv_cpu_table_size = mc_amd->equiv_cpu_table_size; - microcode_patch->mc_amd = cache; - - return microcode_patch; -} - static void free_patch(void *mc) { struct microcode_amd *mc_amd = mc; @@ -236,6 +206,17 @@ static void free_patch(void *mc) } } +static enum microcode_match_result compare_header( + const struct microcode_header_amd *new_header, + const struct microcode_header_amd *old_header) +{ + if ( new_header->processor_rev_id == old_header->processor_rev_id ) + return (new_header->patch_id > old_header->patch_id) ? NEW_UCODE + : OLD_UCODE; + + return MIS_UCODE; +} + static enum microcode_match_result compare_patch( const struct microcode_patch *new, const struct microcode_patch *old) { @@ -246,11 +227,7 @@ static enum microcode_match_result compare_patch( ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE); ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE); - if ( new_header->processor_rev_id == old_header->processor_rev_id ) - return (new_header->patch_id > old_header->patch_id) ? - NEW_UCODE : OLD_UCODE; - - return MIS_UCODE; + return compare_header(new_header, old_header); } static int apply_microcode(const struct microcode_patch *patch) @@ -328,18 +305,10 @@ static int get_ucode_from_buffer_amd( return -EINVAL; } - if ( mc_amd->mpb_size < mpbuf->len ) - { - if ( mc_amd->mpb ) - { - xfree(mc_amd->mpb); - mc_amd->mpb_size = 0; - } - mc_amd->mpb = xmalloc_bytes(mpbuf->len); - if ( mc_amd->mpb == NULL ) - return -ENOMEM; - mc_amd->mpb_size = mpbuf->len; - } + mc_amd->mpb = xmalloc_bytes(mpbuf->len); + if ( !mc_amd->mpb ) + return -ENOMEM; + mc_amd->mpb_size = mpbuf->len; memcpy(mc_amd->mpb, mpbuf->data, mpbuf->len); pr_debug("microcode: CPU%d size %zu, block size %u offset %zu equivID %#x rev %#x\n", @@ -459,8 +428,9 @@ static struct microcode_patch *cpu_request_microcode(const void *buf, size_t bufsize) { struct microcode_amd *mc_amd; + struct microcode_header_amd *saved = NULL; struct microcode_patch *patch = NULL; - size_t offset = 0; + size_t offset = 0, saved_size = 0; int error = 0; unsigned int current_cpu_id; unsigned int equiv_cpu_id; @@ -550,29 +520,22 @@ static struct microcode_patch *cpu_request_microcode(const void *buf, while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize, &offset)) == 0 ) { - struct microcode_patch *new_patch = alloc_microcode_patch(mc_amd); - - if ( IS_ERR(new_patch) ) - { - error = PTR_ERR(new_patch); - break; - } - /* - * If the new patch covers current CPU, compare patches and store the + * If the new ucode covers current CPU, compare ucodes and store the * one with higher revision. */ - if ( (microcode_fits(new_patch->mc_amd) != MIS_UCODE) && - (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) ) + if ( (microcode_fits(mc_amd) != MIS_UCODE) && + (!saved || (compare_header(mc_amd->mpb, saved) == NEW_UCODE)) ) { - struct microcode_patch *tmp = patch; - - patch = new_patch; - new_patch = tmp; + xfree(saved); + saved = mc_amd->mpb; + saved_size = mc_amd->mpb_size; + } + else + { + xfree(mc_amd->mpb); + mc_amd->mpb = NULL; } - - if ( new_patch ) - microcode_free_patch(new_patch); if ( offset >= bufsize ) break; @@ -601,7 +564,22 @@ static struct microcode_patch *cpu_request_microcode(const void *buf, *(const uint32_t *)(buf + offset) == UCODE_MAGIC ) break; } - free_patch(mc_amd); + + if ( saved ) + { + mc_amd->mpb = saved; + mc_amd->mpb_size = saved_size; + patch = xmalloc(struct microcode_patch); + if ( patch ) + patch->mc_amd = mc_amd; + else + { + free_patch(mc_amd); + error = -ENOMEM; + } + } + else + free_patch(mc_amd); out: if ( error && !patch ) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index c3083d7..4e811b7 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -286,25 +286,6 @@ static enum microcode_match_result compare_patch( : OLD_UCODE; } -static struct microcode_patch *alloc_microcode_patch( - const struct microcode_header_intel *mc_header) -{ - unsigned long total_size = get_totalsize(mc_header); - void *new_mc = xmalloc_bytes(total_size); - struct microcode_patch *new_patch = xmalloc(struct microcode_patch); - - if ( !new_patch || !new_mc ) - { - xfree(new_patch); - xfree(new_mc); - return ERR_PTR(-ENOMEM); - } - memcpy(new_mc, mc_header, total_size); - new_patch->mc_intel = new_mc; - - return new_patch; -} - static int apply_microcode(const struct microcode_patch *patch) { unsigned long flags; @@ -353,8 +334,9 @@ static int apply_microcode(const struct microcode_patch *patch) return 0; } -static long get_next_ucode_from_buffer(void **mc, const u8 *buf, - unsigned long size, long offset) +static long get_next_ucode_from_buffer(struct microcode_intel **mc, + const u8 *buf, unsigned long size, + long offset) { struct microcode_header_intel *mc_header; unsigned long total_size; @@ -386,47 +368,46 @@ static struct microcode_patch *cpu_request_microcode(const void *buf, { long offset = 0; int error = 0; - void *mc; + struct microcode_intel *mc, *saved = NULL; struct microcode_patch *patch = NULL; while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 ) { - struct microcode_patch *new_patch; - error = microcode_sanity_check(mc); if ( error ) - break; - - new_patch = alloc_microcode_patch(mc); - if ( IS_ERR(new_patch) ) { - error = PTR_ERR(new_patch); + xfree(mc); break; } /* - * If the new patch covers current CPU, compare patches and store the + * If the new update covers current CPU, compare updates and store the * one with higher revision. */ - if ( (microcode_update_match(&new_patch->mc_intel->hdr) != MIS_UCODE) && - (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) ) + if ( (microcode_update_match(&mc->hdr) != MIS_UCODE) && + (!saved || (mc->hdr.rev > saved->hdr.rev)) ) { - struct microcode_patch *tmp = patch; - - patch = new_patch; - new_patch = tmp; + xfree(saved); + saved = mc; } - - if ( new_patch ) - microcode_free_patch(new_patch); - - xfree(mc); + else + xfree(mc); } - if ( offset > 0 ) - xfree(mc); if ( offset < 0 ) error = offset; + if ( saved ) + { + patch = xmalloc(struct microcode_patch); + if ( patch ) + patch->mc_intel = saved; + else + { + xfree(saved); + error = -ENOMEM; + } + } + if ( error && !patch ) patch = ERR_PTR(error); From patchwork Thu Sep 12 07:22:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142483 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 36F5D1599 for ; Thu, 12 Sep 2019 07:20: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 1163E21479 for ; Thu, 12 Sep 2019 07:20:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1163E21479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JO3-0001pi-Py; Thu, 12 Sep 2019 07:19:27 +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 1i8JO3-0001oj-5x for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:27 +0000 X-Inumbo-ID: 9ac2198c-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 9ac2198c-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:05 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906348" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:19:02 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:25 +0800 Message-Id: <1568272949-1086-13-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 12/16] x86/microcode: Synchronize late microcode loading 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: Sergey Dyasli , Kevin Tian , Borislav Petkov , Ashok Raj , Wei Liu , Jun Nakajima , Andrew Cooper , Jan Beulich , Thomas Gleixner , Chao Gao , =?utf-8?q?Roger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" This patch ports microcode improvement patches from linux kernel. Before you read any further: the early loading method is still the preferred one and you should always do that. The following patch is improving the late loading mechanism for long running jobs and cloud use cases. Gather all cores and serialize the microcode update on them by doing it one-by-one to make the late update process as reliable as possible and avoid potential issues caused by the microcode update. Signed-off-by: Chao Gao Tested-by: Chao Gao [linux commit: a5321aec6412b20b5ad15db2d6b916c05349dbff] [linux commit: bb8c13d61a629276a162c1d2b1a20a815cbcfbb7] Cc: Kevin Tian Cc: Jun Nakajima Cc: Ashok Raj Cc: Borislav Petkov Cc: Thomas Gleixner Cc: Andrew Cooper Cc: Jan Beulich --- Changes in v10: - introduce wait_for_state() and set_state() helper functions - make wait_for_condition() return bool and take const void * - disable/enable watchdog in control thread - rename "master" and "slave" thread to "primary" and "secondary" Changes in v9: - log __buildin_return_address(0) when timeout - divide CPUs into three logical sets and they will call different functions during ucode loading. The 'control thread' is chosen to coordinate ucode loading on all CPUs. Since only control thread would set 'loading_state', we can get rid of 'cmpxchg' stuff in v8. - s/rep_nop/cpu_relax - each thread updates its revision number itself - add XENLOG_ERR prefix for each line of multi-line log messages Changes in v8: - to support blocking #NMI handling during loading ucode * introduce a flag, 'loading_state', to mark the start or end of ucode loading. * use a bitmap for cpu callin since if cpu may stay in #NMI handling, there are two places for a cpu to call in. bitmap won't be counted twice. * don't wait for all CPUs callout, just wait for CPUs that perform the update. We have to do this because some threads may be stuck in NMI handling (where cannot reach the rendezvous). - emit a warning if the system stays in stop_machine context for more than 1s - comment that rdtsc is fine while loading an update - use cmpxchg() to avoid panic being called on multiple CPUs - Propagate revision number to other threads - refine comments and prompt messages Changes in v7: - Check whether 'timeout' is 0 rather than "<=0" since it is unsigned int. - reword the comment above microcode_update_cpu() to clearly state that one thread per core should do the update. --- xen/arch/x86/microcode.c | 296 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 269 insertions(+), 27 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index c2ea20f..049eda6 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -30,18 +30,52 @@ #include #include #include +#include #include #include #include +#include +#include #include #include #include #include +/* + * Before performing a late microcode update on any thread, we + * rendezvous all cpus in stop_machine context. The timeout for + * waiting for cpu rendezvous is 30ms. It is the timeout used by + * live patching + */ +#define MICROCODE_CALLIN_TIMEOUT_US 30000 + +/* + * Timeout for each thread to complete update is set to 1s. It is a + * conservative choice considering all possible interference. + */ +#define MICROCODE_UPDATE_TIMEOUT_US 1000000 + static module_t __initdata ucode_mod; static signed int __initdata ucode_mod_idx; static bool_t __initdata ucode_mod_forced; +static unsigned int nr_cores; + +/* + * These states help to coordinate CPUs during loading an update. + * + * The semantics of each state is as follow: + * - LOADING_PREPARE: initial state of 'loading_state'. + * - LOADING_CALLIN: CPUs are allowed to callin. + * - LOADING_ENTER: all CPUs have called in. Initiate ucode loading. + * - LOADING_EXIT: ucode loading is done or aborted. + */ +static enum { + LOADING_PREPARE, + LOADING_CALLIN, + LOADING_ENTER, + LOADING_EXIT, +} loading_state; /* * If we scan the initramfs.cpio for the early microcode code @@ -190,6 +224,16 @@ static DEFINE_SPINLOCK(microcode_mutex); DEFINE_PER_CPU(struct cpu_signature, cpu_sig); /* + * Count the CPUs that have entered, exited the rendezvous and succeeded in + * microcode update during late microcode update respectively. + * + * Note that a bitmap is used for callin to allow cpu to set a bit multiple + * times. It is required to do busy-loop in #NMI handling. + */ +static cpumask_t cpu_callin_map; +static atomic_t cpu_out, cpu_updated; + +/* * Return a patch that covers current CPU. If there are multiple patches, * return the one with the highest revision number. Return error If no * patch is found and an error occurs during the parsing process. Otherwise @@ -231,6 +275,34 @@ static bool microcode_update_cache(struct microcode_patch *patch) return true; } +/* Wait for a condition to be met with a timeout (us). */ +static int wait_for_condition(bool (*func)(const void *data), void *data, + unsigned int timeout) +{ + while ( !func(data) ) + { + if ( !timeout-- ) + { + printk("CPU%u: Timeout in %pS\n", + smp_processor_id(), __builtin_return_address(0)); + return -EBUSY; + } + udelay(1); + } + + return 0; +} + +static bool wait_cpu_callin(const void *nr) +{ + return cpumask_weight(&cpu_callin_map) >= (unsigned long)nr; +} + +static bool wait_cpu_callout(const void *nr) +{ + return atomic_read(&cpu_out) >= (unsigned long)nr; +} + /* * Load a microcode update to current CPU. * @@ -264,38 +336,158 @@ static int microcode_update_cpu(const struct microcode_patch *patch) return err; } -static long do_microcode_update(void *patch) +static bool wait_for_state(unsigned int state) +{ + while ( loading_state != state ) + { + if ( state != LOADING_EXIT && loading_state == LOADING_EXIT ) + return false; + cpu_relax(); + } + + return true; +} + +static void set_state(unsigned int state) +{ + loading_state = state; + smp_wmb(); +} + +static int secondary_thread_fn(void) +{ + unsigned int primary = cpumask_first(this_cpu(cpu_sibling_mask)); + + if ( !wait_for_state(LOADING_CALLIN) ) + return -EBUSY; + + cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); + + if ( !wait_for_state(LOADING_EXIT) ) + return -EBUSY; + + /* Copy update revision from the primary thread. */ + this_cpu(cpu_sig).rev = per_cpu(cpu_sig, primary).rev; + + return 0; +} + +static int primary_thread_fn(const struct microcode_patch *patch) +{ + int ret = 0; + + if ( !wait_for_state(LOADING_CALLIN) ) + return -EBUSY; + + cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); + + if ( !wait_for_state(LOADING_ENTER) ) + return -EBUSY; + + ret = microcode_ops->apply_microcode(patch); + if ( !ret ) + atomic_inc(&cpu_updated); + atomic_inc(&cpu_out); + + return ret; +} + +static int control_thread_fn(const struct microcode_patch *patch) { - unsigned int cpu; - int ret = microcode_update_cpu(patch); + unsigned int cpu = smp_processor_id(), done; + unsigned long tick; + int ret; + + /* + * We intend to disable interrupt for long time, which may lead to + * watchdog timeout. + */ + watchdog_disable(); - /* Store the patch after a successful loading */ - if ( !ret && patch ) + /* Allow threads to call in */ + set_state(LOADING_CALLIN); + + cpumask_set_cpu(cpu, &cpu_callin_map); + + /* Waiting for all threads calling in */ + ret = wait_for_condition(wait_cpu_callin, + (void *)(unsigned long)num_online_cpus(), + MICROCODE_CALLIN_TIMEOUT_US); + if ( ret ) { - spin_lock(µcode_mutex); - microcode_update_cache(patch); - spin_unlock(µcode_mutex); - patch = NULL; + set_state(LOADING_EXIT); + return ret; } - if ( microcode_ops->end_update_percpu ) - microcode_ops->end_update_percpu(); + /* Let primary threads load the given ucode update */ + set_state(LOADING_ENTER); + ret = microcode_ops->apply_microcode(patch); + if ( !ret ) + atomic_inc(&cpu_updated); + atomic_inc(&cpu_out); + + tick = rdtsc_ordered(); + /* Wait for primary threads finishing update */ + done = atomic_read(&cpu_out); + while ( done != nr_cores ) + { + /* + * During each timeout interval, at least a CPU is expected to + * finish its update. Otherwise, something goes wrong. + * + * Note that RDTSC (in wait_for_condition()) is safe for threads to + * execute while waiting for completion of loading an update. + */ + if ( wait_for_condition(wait_cpu_callout, + (void *)(unsigned long)(done + 1), + MICROCODE_UPDATE_TIMEOUT_US) ) + panic("Timeout when finished updating microcode (finished %u/%u)", + done, nr_cores); + + /* Print warning message once if long time is spent here */ + if ( tick && rdtsc_ordered() - tick >= cpu_khz * 1000 ) + { + printk(XENLOG_WARNING + "WARNING: UPDATING MICROCODE HAS CONSUMED MORE THAN 1 SECOND!\n"); + tick = 0; + } + done = atomic_read(&cpu_out); + } + + /* Mark loading is done to unblock other threads */ + set_state(LOADING_EXIT); + + watchdog_enable(); + + return ret; +} + +static int do_microcode_update(void *patch) +{ + unsigned int cpu = smp_processor_id(); /* - * Each thread tries to load ucode and only the first thread of a core - * would succeed. Ignore error other than -EIO. + * primary thread is the one with the lowest thread id among all siblings + * thread in a core or a compute unit. It is chosen to load a microcode + * update. */ - if ( ret != -EIO ) - ret = 0; + unsigned int primary = cpumask_first(this_cpu(cpu_sibling_mask)); + int ret; - cpu = cpumask_next(smp_processor_id(), &cpu_online_map); - if ( cpu < nr_cpu_ids ) - return continue_hypercall_on_cpu(cpu, do_microcode_update, patch) ? - : ret; + /* + * The control thread set state to coordinate ucode loading. Primary + * threads load the given ucode patch. Secondary threads just wait for + * the completion of the ucode loading process. + */ + if ( cpu == cpumask_first(&cpu_online_map) ) + ret = control_thread_fn(patch); + else if ( cpu == primary ) + ret = primary_thread_fn(patch); + else + ret = secondary_thread_fn(); - /* Free the patch if no CPU has loaded it successfully. */ - if ( patch ) - microcode_free_patch(patch); + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); return ret; } @@ -304,6 +496,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) { int ret; void *buffer; + unsigned int cpu, updated; struct microcode_patch *patch; if ( len != (uint32_t)len ) @@ -322,18 +515,25 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) goto free; } + /* cpu_online_map must not change during update */ + if ( !get_cpu_maps() ) + { + ret = -EBUSY; + goto free; + } + patch = parse_blob(buffer, len); if ( IS_ERR(patch) ) { ret = PTR_ERR(patch); printk(XENLOG_WARNING "Parsing microcode blob error %d\n", ret); - goto free; + goto put; } if ( !patch ) { ret = -ENOENT; - goto free; + goto put; } if ( microcode_ops->start_update ) @@ -342,13 +542,55 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) if ( ret != 0 ) { microcode_free_patch(patch); - goto free; + goto put; } } - ret = continue_hypercall_on_cpu(cpumask_first(&cpu_online_map), - do_microcode_update, patch); + cpumask_clear(&cpu_callin_map); + atomic_set(&cpu_out, 0); + atomic_set(&cpu_updated, 0); + loading_state = LOADING_PREPARE; + + /* Calculate the number of online CPU core */ + nr_cores = 0; + for_each_online_cpu(cpu) + if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) ) + nr_cores++; + + printk(XENLOG_INFO "%u cores are to update their microcode\n", nr_cores); + + /* + * Late loading dance. Why the heavy-handed stop_machine effort? + * + * - HT siblings must be idle and not execute other code while the other + * sibling is loading microcode in order to avoid any negative + * interactions cause by the loading. + * + * - In addition, microcode update on the cores must be serialized until + * this requirement can be relaxed in the future. Right now, this is + * conservative and good. + */ + ret = stop_machine_run(do_microcode_update, patch, NR_CPUS); + + updated = atomic_read(&cpu_updated); + if ( updated > 0 ) + { + spin_lock(µcode_mutex); + microcode_update_cache(patch); + spin_unlock(µcode_mutex); + } + else + microcode_free_patch(patch); + + if ( updated && updated != nr_cores ) + printk(XENLOG_ERR "ERROR: Updating microcode succeeded on %u cores and failed\n" + XENLOG_ERR "on other %u cores. A system with differing microcode\n" + XENLOG_ERR "revisions is considered unstable. Please reboot and do not\n" + XENLOG_ERR "load the microcode that triggers this warning!\n", + updated, nr_cores - updated); + put: + put_cpu_maps(); free: xfree(buffer); return ret; From patchwork Thu Sep 12 07:22:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142499 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 3B9121599 for ; Thu, 12 Sep 2019 07:21:11 +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 2131920678 for ; Thu, 12 Sep 2019 07:21:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2131920678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JO9-0001vu-Ea; Thu, 12 Sep 2019 07:19:33 +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 1i8JO8-0001u7-6M for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:32 +0000 X-Inumbo-ID: 9c6cebcc-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 9c6cebcc-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:08 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906357" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:19:06 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:26 +0800 Message-Id: <1568272949-1086-14-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 13/16] microcode: remove microcode_update_lock 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" microcode_update_lock is to prevent logic threads of a same core from updating microcode at the same time. But due to using a global lock, it also prevented parallel microcode updating on different cores. Remove this lock in order to update microcode in parallel. It is safe because we have already ensured serialization of sibling threads at the caller side. 1.For late microcode update, do_microcode_update() ensures that only one sibiling thread of a core can update microcode. 2.For microcode update during system startup or CPU-hotplug, microcode_mutex() guarantees update serialization of logical threads. 3.get/put_cpu_bitmaps() prevents the concurrency of CPU-hotplug and late microcode update. Note that printk in apply_microcode() and svm_host_osvm_init() (for AMD only) are still processed sequentially. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v7: - reworked. Remove complex lock logics introduced in v5 and v6. The microcode patch to be applied is passed as an argument without any global variable. Thus no lock is added to serialize potential readers/writers. Callers of apply_microcode() will guarantee the correctness: the patch poninted by the arguments won't be changed by others. Changes in v6: - introduce early_ucode_update_lock to serialize early ucode update. Changes in v5: - newly add --- xen/arch/x86/microcode_amd.c | 8 +------- xen/arch/x86/microcode_intel.c | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index f05db72..856caea 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -74,9 +74,6 @@ struct mpbhdr { uint8_t data[]; }; -/* serialize access to the physical write */ -static DEFINE_SPINLOCK(microcode_update_lock); - /* See comment in start_update() for cases when this routine fails */ static int collect_cpu_info(struct cpu_signature *csig) { @@ -232,7 +229,6 @@ static enum microcode_match_result compare_patch( static int apply_microcode(const struct microcode_patch *patch) { - unsigned long flags; uint32_t rev; int hw_err; unsigned int cpu = smp_processor_id(); @@ -247,15 +243,13 @@ static int apply_microcode(const struct microcode_patch *patch) hdr = patch->mc_amd->mpb; - spin_lock_irqsave(µcode_update_lock, flags); + BUG_ON(local_irq_is_enabled()); hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr); /* get patch id after patching */ rdmsrl(MSR_AMD_PATCHLEVEL, rev); - spin_unlock_irqrestore(µcode_update_lock, flags); - /* * Some processors leave the ucode blob mapping as UC after the update. * Flush the mapping to regain normal cacheability. diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 4e811b7..19f1ba0 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -93,9 +93,6 @@ struct extended_sigtable { #define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE) -/* serialize access to the physical write to MSR 0x79 */ -static DEFINE_SPINLOCK(microcode_update_lock); - static int collect_cpu_info(struct cpu_signature *csig) { unsigned int cpu_num = smp_processor_id(); @@ -288,7 +285,6 @@ static enum microcode_match_result compare_patch( static int apply_microcode(const struct microcode_patch *patch) { - unsigned long flags; uint64_t msr_content; unsigned int val[2]; unsigned int cpu_num = raw_smp_processor_id(); @@ -303,8 +299,7 @@ static int apply_microcode(const struct microcode_patch *patch) mc_intel = patch->mc_intel; - /* serialize access to the physical write to MSR 0x79 */ - spin_lock_irqsave(µcode_update_lock, flags); + BUG_ON(local_irq_is_enabled()); /* write microcode via MSR 0x79 */ wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits); @@ -317,7 +312,6 @@ static int apply_microcode(const struct microcode_patch *patch) rdmsrl(MSR_IA32_UCODE_REV, msr_content); val[1] = (uint32_t)(msr_content >> 32); - spin_unlock_irqrestore(µcode_update_lock, flags); if ( val[1] != mc_intel->hdr.rev ) { printk(KERN_ERR "microcode: CPU%d update from revision " From patchwork Thu Sep 12 07:22:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142493 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 808ED13BD for ; Thu, 12 Sep 2019 07:20:56 +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 5ABD920678 for ; Thu, 12 Sep 2019 07:20:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5ABD920678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JOD-00020B-Qf; Thu, 12 Sep 2019 07:19:37 +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 1i8JOD-0001ze-6W for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:37 +0000 X-Inumbo-ID: 9f2fabce-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 9f2fabce-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:13 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906379" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:19:09 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:27 +0800 Message-Id: <1568272949-1086-15-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 14/16] microcode: rendezvous CPUs in NMI handler and load ucode 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: Sergey Dyasli , Stefano Stabellini , Ashok Raj , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Ian Jackson , Tim Deegan , Julien Grall , Jan Beulich , Andrew Cooper , Chao Gao , =?utf-8?q?Roger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" When one core is loading ucode, handling NMI on sibling threads or on other cores in the system might be problematic. By rendezvousing all CPUs in NMI handler, it prevents NMI acceptance during ucode loading. Basically, some work previously done in stop_machine context is moved to NMI handler. Primary threads call in and load ucode in NMI handler. Secondary threads wait for the completion of ucode loading on all CPU cores. An option is introduced to disable this behavior. Signed-off-by: Chao Gao Signed-off-by: Sergey Dyasli --- Changes in v10: - rewrite based on Sergey's idea and patch - add Sergey's SOB. - add an option to disable ucode loading in NMI handler - don't send IPI NMI to the control thread to avoid unknown_nmi_error() in do_nmi(). - add an assertion to make sure the cpu chosen to handle platform NMI won't send self NMI. Otherwise, there is a risk that we encounter unknown_nmi_error() and system crashes. Changes in v9: - control threads send NMI to all other threads. Slave threads will stay in the NMI handling to prevent NMI acceptance during ucode loading. Note that self-nmi is invalid according to SDM. - s/rep_nop/cpu_relax - remove debug message in microcode_nmi_callback(). Printing debug message would take long times and control thread may timeout. - rebase and fix conflicts Changes in v8: - new --- docs/misc/xen-command-line.pandoc | 10 +++++ xen/arch/x86/microcode.c | 95 ++++++++++++++++++++++++++++++++------- xen/arch/x86/traps.c | 6 ++- xen/include/asm-x86/nmi.h | 3 ++ 4 files changed, 96 insertions(+), 18 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 7c72e31..3017073 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -2056,6 +2056,16 @@ microcode in the cpio name space must be: - on Intel: kernel/x86/microcode/GenuineIntel.bin - on AMD : kernel/x86/microcode/AuthenticAMD.bin +### ucode_loading_in_nmi (x86) +> `= ` + +> Default: `true` + +When one CPU is loading ucode, handling NMIs on sibling threads or threads on +other cores might cause problems. By default, all CPUs rendezvous in NMI handler +and load ucode. This option provides a way to disable it in case of some CPUs +don't allow ucode loading in NMI handler. + ### unrestricted_guest (Intel) > `= ` diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 049eda6..64a4321 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -36,8 +36,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -125,6 +127,9 @@ static int __init parse_ucode(const char *s) } custom_param("ucode", parse_ucode); +static bool __read_mostly opt_ucode_loading_in_nmi = true; +boolean_runtime_param("ucode_loading_in_nmi", opt_ucode_loading_in_nmi); + /* * 8MB ought to be enough. */ @@ -232,6 +237,7 @@ DEFINE_PER_CPU(struct cpu_signature, cpu_sig); */ static cpumask_t cpu_callin_map; static atomic_t cpu_out, cpu_updated; +const struct microcode_patch *nmi_patch; /* * Return a patch that covers current CPU. If there are multiple patches, @@ -354,6 +360,50 @@ static void set_state(unsigned int state) smp_wmb(); } +static int secondary_thread_work(void) +{ + cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); + + return wait_for_state(LOADING_EXIT) ? 0 : -EBUSY; +} + +static int primary_thread_work(const struct microcode_patch *patch) +{ + int ret; + + cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); + + if ( !wait_for_state(LOADING_ENTER) ) + return -EBUSY; + + ret = microcode_ops->apply_microcode(patch); + if ( !ret ) + atomic_inc(&cpu_updated); + atomic_inc(&cpu_out); + + return ret; +} + +static int microcode_nmi_callback(const struct cpu_user_regs *regs, int cpu) +{ + unsigned int primary = cpumask_first(this_cpu(cpu_sibling_mask)); + unsigned int controller = cpumask_first(&cpu_online_map); + + /* System-generated NMI, will be ignored */ + if ( loading_state != LOADING_CALLIN ) + return 0; + + if ( cpu == controller || (!opt_ucode_loading_in_nmi && cpu == primary) ) + return 0; + + if ( cpu == primary ) + primary_thread_work(nmi_patch); + else + secondary_thread_work(); + + return 0; +} + static int secondary_thread_fn(void) { unsigned int primary = cpumask_first(this_cpu(cpu_sibling_mask)); @@ -361,10 +411,7 @@ static int secondary_thread_fn(void) if ( !wait_for_state(LOADING_CALLIN) ) return -EBUSY; - cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); - - if ( !wait_for_state(LOADING_EXIT) ) - return -EBUSY; + self_nmi(); /* Copy update revision from the primary thread. */ this_cpu(cpu_sig).rev = per_cpu(cpu_sig, primary).rev; @@ -379,15 +426,10 @@ static int primary_thread_fn(const struct microcode_patch *patch) if ( !wait_for_state(LOADING_CALLIN) ) return -EBUSY; - cpumask_set_cpu(smp_processor_id(), &cpu_callin_map); - - if ( !wait_for_state(LOADING_ENTER) ) - return -EBUSY; - - ret = microcode_ops->apply_microcode(patch); - if ( !ret ) - atomic_inc(&cpu_updated); - atomic_inc(&cpu_out); + if ( opt_ucode_loading_in_nmi ) + self_nmi(); + else + ret = primary_thread_work(patch); return ret; } @@ -397,6 +439,7 @@ static int control_thread_fn(const struct microcode_patch *patch) unsigned int cpu = smp_processor_id(), done; unsigned long tick; int ret; + nmi_callback_t *saved_nmi_callback; /* * We intend to disable interrupt for long time, which may lead to @@ -404,6 +447,9 @@ static int control_thread_fn(const struct microcode_patch *patch) */ watchdog_disable(); + nmi_patch = patch; + saved_nmi_callback = set_nmi_callback(microcode_nmi_callback); + /* Allow threads to call in */ set_state(LOADING_CALLIN); @@ -419,14 +465,23 @@ static int control_thread_fn(const struct microcode_patch *patch) return ret; } - /* Let primary threads load the given ucode update */ - set_state(LOADING_ENTER); - + /* Control thread loads ucode first while others are in NMI handler. */ ret = microcode_ops->apply_microcode(patch); if ( !ret ) atomic_inc(&cpu_updated); atomic_inc(&cpu_out); + if ( ret == -EIO ) + { + printk(XENLOG_ERR + "Late loading aborted: CPU%u failed to update ucode\n", cpu); + set_state(LOADING_EXIT); + return ret; + } + + /* Let primary threads load the given ucode update */ + set_state(LOADING_ENTER); + tick = rdtsc_ordered(); /* Wait for primary threads finishing update */ done = atomic_read(&cpu_out); @@ -458,6 +513,7 @@ static int control_thread_fn(const struct microcode_patch *patch) /* Mark loading is done to unblock other threads */ set_state(LOADING_EXIT); + set_nmi_callback(saved_nmi_callback); watchdog_enable(); return ret; @@ -522,6 +578,13 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) goto free; } + /* + * CPUs except the first online CPU would send a fake (self) NMI to + * rendezvous in NMI handler. But a fake NMI to nmi_cpu may trigger + * unknown_nmi_error(). It ensures nmi_cpu won't receive a fake NMI. + */ + ASSERT( !cpu_online(nmi_cpu) || nmi_cpu == cpumask_first(&cpu_online_map) ); + patch = parse_blob(buffer, len); if ( IS_ERR(patch) ) { diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 16c590d..503f5c8 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -126,6 +126,8 @@ boolean_param("ler", opt_ler); /* LastExceptionFromIP on this hardware. Zero if LER is not in use. */ unsigned int __read_mostly ler_msr; +unsigned int __read_mostly nmi_cpu; + #define stack_words_per_line 4 #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp) @@ -1679,7 +1681,7 @@ void do_nmi(const struct cpu_user_regs *regs) * this port before we re-arm the NMI watchdog, we reduce the chance * of having an NMI watchdog expire while in the SMI handler. */ - if ( cpu == 0 ) + if ( cpu == nmi_cpu ) reason = inb(0x61); if ( (nmi_watchdog == NMI_NONE) || @@ -1687,7 +1689,7 @@ void do_nmi(const struct cpu_user_regs *regs) handle_unknown = true; /* Only the BSP gets external NMIs from the system. */ - if ( cpu == 0 ) + if ( cpu == nmi_cpu ) { if ( reason & 0x80 ) pci_serr_error(regs); diff --git a/xen/include/asm-x86/nmi.h b/xen/include/asm-x86/nmi.h index 99f6284..dbebffe 100644 --- a/xen/include/asm-x86/nmi.h +++ b/xen/include/asm-x86/nmi.h @@ -11,6 +11,9 @@ extern bool opt_watchdog; /* Watchdog force parameter from the command line */ extern bool watchdog_force; + +/* CPU to handle platform NMI */ +extern unsigned int nmi_cpu; typedef int nmi_callback_t(const struct cpu_user_regs *regs, int cpu); From patchwork Thu Sep 12 07:22:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142489 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 74CE516B1 for ; Thu, 12 Sep 2019 07:20:53 +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 5A60020678 for ; Thu, 12 Sep 2019 07:20:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5A60020678 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JOJ-000251-7e; Thu, 12 Sep 2019 07:19:43 +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 1i8JOI-00024F-72 for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:42 +0000 X-Inumbo-ID: a0d2847e-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id a0d2847e-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:16 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906389" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:19:13 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:28 +0800 Message-Id: <1568272949-1086-16-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 15/16] microcode: disable late loading if CPUs are affected by BDF90 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" It ports the implementation of is_blacklisted() in linux kernel to Xen. Late loading may cause system hang if CPUs are affected by BDF90. Check against BDF90 before performing a late loading. Signed-off-by: Chao Gao --- xen/arch/x86/microcode.c | 6 ++++++ xen/arch/x86/microcode_intel.c | 23 +++++++++++++++++++++++ xen/include/asm-x86/microcode.h | 1 + 3 files changed, 30 insertions(+) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 64a4321..dbd2730 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -561,6 +561,12 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len) if ( microcode_ops == NULL ) return -EINVAL; + if ( microcode_ops->is_blacklisted && microcode_ops->is_blacklisted() ) + { + printk(XENLOG_WARNING "Late ucode loading is disabled!\n"); + return -EPERM; + } + buffer = xmalloc_bytes(len); if ( !buffer ) return -ENOMEM; diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 19f1ba0..bcef668 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -283,6 +284,27 @@ static enum microcode_match_result compare_patch( : OLD_UCODE; } +static bool is_blacklisted(void) +{ + struct cpuinfo_x86 *c = ¤t_cpu_data; + uint64_t llc_size = c->x86_cache_size * 1024ULL; + struct cpu_signature *sig = &this_cpu(cpu_sig); + + do_div(llc_size, c->x86_max_cores); + + /* + * Late loading on model 79 with microcode revision less than 0x0b000021 + * and LLC size per core bigger than 2.5MB may result in a system hang. + * This behavior is documented in item BDF90, #334165 (Intel Xeon + * Processor E7-8800/4800 v4 Product Family). + */ + if ( c->x86 == 6 && c->x86_model == 0x4F && c->x86_mask == 0x1 && + llc_size > 2621440 && sig->rev < 0x0b000021 ) + return true; + + return false; +} + static int apply_microcode(const struct microcode_patch *patch) { uint64_t msr_content; @@ -415,6 +437,7 @@ static const struct microcode_ops microcode_intel_ops = { .free_patch = free_patch, .compare_patch = compare_patch, .match_cpu = match_cpu, + .is_blacklisted = is_blacklisted, }; int __init microcode_init_intel(void) diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 7d5a1f8..9ffd9d2 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -30,6 +30,7 @@ struct microcode_ops { bool (*match_cpu)(const struct microcode_patch *patch); enum microcode_match_result (*compare_patch)( const struct microcode_patch *new, const struct microcode_patch *old); + bool (*is_blacklisted)(void); }; struct cpu_signature { From patchwork Thu Sep 12 07:22:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chao Gao X-Patchwork-Id: 11142477 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 830F11599 for ; Thu, 12 Sep 2019 07:20:34 +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 6948421479 for ; Thu, 12 Sep 2019 07:20:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6948421479 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.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 1i8JOO-0002AM-K5; Thu, 12 Sep 2019 07:19:48 +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 1i8JON-000290-7S for xen-devel@lists.xenproject.org; Thu, 12 Sep 2019 07:19:47 +0000 X-Inumbo-ID: a27761e6-d52d-11e9-83e3-12813bfff9fa Received: from mga01.intel.com (unknown [192.55.52.88]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id a27761e6-d52d-11e9-83e3-12813bfff9fa; Thu, 12 Sep 2019 07:19:18 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 00:19:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="189906404" Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga006.jf.intel.com with ESMTP; 12 Sep 2019 00:19:16 -0700 From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 12 Sep 2019 15:22:29 +0800 Message-Id: <1568272949-1086-17-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1568272949-1086-1-git-send-email-chao.gao@intel.com> References: <1568272949-1086-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v10 16/16] microcode/intel: writeback and invalidate cache conditionally 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: Sergey Dyasli , Ashok Raj , Wei Liu , Andrew Cooper , Jan Beulich , Chao Gao , =?utf-8?q?R?= =?utf-8?q?oger_Pau_Monn=C3=A9?= MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" It is needed to mitigate some issues on this specific Broadwell CPU. Signed-off-by: Chao Gao --- xen/arch/x86/microcode_intel.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index bcef668..4e5e7f9 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -305,6 +305,31 @@ static bool is_blacklisted(void) return false; } +static void microcode_quirk(void) +{ + struct cpuinfo_x86 *c; + uint64_t llc_size; + + /* + * Don't refer to current_cpu_data, which isn't fully initialized + * before this stage. + */ + if ( system_state < SYS_STATE_smp_boot ) + return; + + c = ¤t_cpu_data; + llc_size = c->x86_cache_size * 1024ULL; + do_div(llc_size, c->x86_max_cores); + + /* + * To mitigate some issues on this specific Broadwell CPU, writeback and + * invalidate cache regardless of ucode revision. + */ + if ( c->x86 == 6 && c->x86_model == 0x4F && c->x86_mask == 0x1 && + llc_size > 2621440 ) + wbinvd(); +} + static int apply_microcode(const struct microcode_patch *patch) { uint64_t msr_content; @@ -323,6 +348,8 @@ static int apply_microcode(const struct microcode_patch *patch) BUG_ON(local_irq_is_enabled()); + microcode_quirk(); + /* write microcode via MSR 0x79 */ wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits); wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);