From nobody Mon Feb 9 19:52:49 2026 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1569505848; cv=none; d=zoho.com; s=zohoarc; b=Ln9X0tsbAKIcfeOsi2626qquewx/xLRGeOqpIUGklh8ykW4JAYANCEEXIgsyWhLtHavWz81UXsElWtGFdC2iIt1K7Hhgsx+xMoykWM9e8vuvUPptig0TLJdXyN1Hri4lYFNfddJpgrnvmEsiS2XldwUyWJhH3UKeAPogjhyz8kc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1569505848; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=mdVBYvquGdyv0Ktwed47EUK2lsQzR900HL+t2wiy9o4=; b=EDjJxTphr7uohgZPIF0QyAVLWyhFvB5LEUpy967GygjvEOW4UtxgpOo5gJxz4LwJjMiN/wT0xKzTG3lnQcv+3Sl9+P8hr8LB/6Nkrr4kWIYKDErJa3ltQTjl/JboLSKSXFQG5lTCYFAsD2ZRuhAuPMcjyOKtv0acJeJqazdad3k= ARC-Authentication-Results: i=1; mx.zoho.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 156950584879111.845499244017333; Thu, 26 Sep 2019 06:50:48 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDU9S-00022Q-MJ; Thu, 26 Sep 2019 13:49:46 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDU9R-00022D-8P for xen-devel@lists.xenproject.org; Thu, 26 Sep 2019 13:49:45 +0000 Received: from mga04.intel.com (unknown [192.55.52.120]) by localhost (Halon) with ESMTPS id 7c129f88-e064-11e9-97fb-bc764e2007e4; Thu, 26 Sep 2019 13:49:39 +0000 (UTC) Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Sep 2019 06:49:39 -0700 Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by fmsmga008.fm.intel.com with ESMTP; 26 Sep 2019 06:49:37 -0700 X-Inumbo-ID: 7c129f88-e064-11e9-97fb-bc764e2007e4 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,552,1559545200"; d="scan'208";a="189126020" From: Chao Gao To: xen-devel@lists.xenproject.org Date: Thu, 26 Sep 2019 21:53:29 +0800 Message-Id: <1569506015-26938-2-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1569506015-26938-1-git-send-email-chao.gao@intel.com> References: <1569506015-26938-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v11 1/7] 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?Roger=20Pau=20Monn=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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. Note that calling ->apply_microcode() itself doesn't require any lock being held. But the parameter passed to it may be protected by some locks. E.g. microcode_update_cpu() acquires microcode_mutex to avoid microcode_cache being updated by others. Signed-off-by: Chao Gao Reviewed-by: Jan Beulich --- Changes in v11: - drop Roger's RB. - acquire microcode_mutex before checking whether microcode_cache is NULL - ignore -EINVAL which indicates a equal/newer ucode is already loaded. - free 'buffer' earlier to avoid goto clauses in microcode_update() 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 | 173 +++++++++++++++++++++++++++---------= ---- xen/arch/x86/microcode_amd.c | 38 ++++----- xen/arch/x86/microcode_intel.c | 66 +++++++-------- xen/include/asm-x86/microcode.h | 5 +- 4 files changed, 172 insertions(+), 110 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index b44e4d7..3ea2a6e 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -189,12 +189,19 @@ static DEFINE_SPINLOCK(microcode_mutex); =20 DEFINE_PER_CPU(struct cpu_signature, cpu_sig); =20 -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; +} =20 int microcode_resume_cpu(void) { @@ -220,15 +227,8 @@ void microcode_free_patch(struct microcode_patch *micr= ocode_patch) xfree(microcode_patch); } =20 -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)); =20 @@ -249,49 +249,82 @@ bool microcode_update_cache(struct microcode_patch *p= atch) return true; } =20 -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 upd= ate + * 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 =3D smp_processor_id(); - struct cpu_signature *sig =3D &per_cpu(cpu_sig, cpu); + int err =3D microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); =20 - spin_lock(µcode_mutex); + if ( unlikely(err) ) + return err; =20 - err =3D microcode_ops->collect_cpu_info(sig); - if ( likely(!err) ) - err =3D microcode_ops->cpu_request_microcode(buf, size); + spin_lock(µcode_mutex); + if ( patch ) + err =3D microcode_ops->apply_microcode(patch); + else if ( microcode_cache ) + { + err =3D microcode_ops->apply_microcode(microcode_cache); + if ( err =3D=3D -EIO ) + { + microcode_free_patch(microcode_cache); + microcode_cache =3D NULL; + } + } + else + /* No patch to update */ + err =3D -ENOENT; spin_unlock(µcode_mutex); =20 return err; } =20 -static long do_microcode_update(void *_info) +static long do_microcode_update(void *patch) { - struct microcode_info *info =3D _info; - int error; - - BUG_ON(info->cpu !=3D smp_processor_id()); + unsigned int cpu; + int ret =3D microcode_update_cpu(patch); =20 - error =3D microcode_update_cpu(info->buffer, info->buffer_size); - if ( error ) - info->error =3D error; + /* Store the patch after a successful loading */ + if ( !ret && patch ) + { + spin_lock(µcode_mutex); + microcode_update_cache(patch); + spin_unlock(µcode_mutex); + patch =3D NULL; + } =20 if ( microcode_ops->end_update_percpu ) microcode_ops->end_update_percpu(); =20 - info->cpu =3D cpumask_next(info->cpu, &cpu_online_map); - if ( info->cpu < nr_cpu_ids ) - return continue_hypercall_on_cpu(info->cpu, do_microcode_update, i= nfo); + /* + * Each thread tries to load ucode. Only the first thread of a core + * would succeed while other threads would encounter -EINVAL which + * indicates current ucode revision is equal to or newer than the + * given patch. It is actually expected; so ignore this error. + */ + if ( ret =3D=3D -EINVAL ) + ret =3D 0; + + cpu =3D cpumask_next(smp_processor_id(), &cpu_online_map); + if ( cpu < nr_cpu_ids ) + return continue_hypercall_on_cpu(cpu, do_microcode_update, patch) = ?: + ret; + + /* Free the patch if no CPU has loaded it successfully. */ + if ( patch ) + microcode_free_patch(patch); =20 - error =3D info->error; - xfree(info); - return error; + return ret; } =20 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; =20 if ( len !=3D (uint32_t)len ) return -E2BIG; @@ -299,32 +332,41 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_voi= d) buf, unsigned long len) if ( microcode_ops =3D=3D NULL ) return -EINVAL; =20 - info =3D xmalloc_bytes(sizeof(*info) + len); - if ( info =3D=3D NULL ) + buffer =3D xmalloc_bytes(len); + if ( !buffer ) return -ENOMEM; =20 - ret =3D copy_from_guest(info->buffer, buf, len); - if ( ret !=3D 0 ) + ret =3D copy_from_guest(buffer, buf, len); + if ( ret ) + { + xfree(buffer); + return -EFAULT; + } + + patch =3D parse_blob(buffer, len); + xfree(buffer); + if ( IS_ERR(patch) ) { - xfree(info); + ret =3D PTR_ERR(patch); + printk(XENLOG_WARNING "Parsing microcode blob error %d\n", ret); return ret; } =20 - info->buffer_size =3D len; - info->error =3D 0; - info->cpu =3D cpumask_first(&cpu_online_map); + if ( !patch ) + return -ENOENT; =20 if ( microcode_ops->start_update ) { ret =3D microcode_ops->start_update(); if ( ret !=3D 0 ) { - xfree(info); + microcode_free_patch(patch); return ret; } } =20 - return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); + return continue_hypercall_on_cpu(cpumask_first(&cpu_online_map), + do_microcode_update, patch); } =20 static int __init microcode_init(void) @@ -371,23 +413,42 @@ int __init early_microcode_update_cpu(bool start_upda= te) =20 microcode_ops->collect_cpu_info(&this_cpu(cpu_sig)); =20 - if ( data ) + if ( !data ) + return -ENOMEM; + + if ( start_update ) { - if ( start_update && microcode_ops->start_update ) + struct microcode_patch *patch; + + patch =3D 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 =3D microcode_update_cache(patch); + spin_unlock(µcode_mutex); + ASSERT(rc); + + if ( microcode_ops->start_update ) rc =3D microcode_ops->start_update(); =20 if ( rc ) return rc; + } =20 - rc =3D microcode_update_cpu(data, len); + rc =3D microcode_update_cpu(NULL); =20 - if ( microcode_ops->end_update_percpu ) - microcode_ops->end_update_percpu(); + if ( microcode_ops->end_update_percpu ) + microcode_ops->end_update_percpu(); =20 - return rc; - } - else - return -ENOMEM; + return rc; } =20 int __init early_microcode_init(void) diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index 8e4cdab..0199308 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 cp= u) return 0; } =20 -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 =3D NULL; size_t offset =3D 0; int error =3D 0; unsigned int current_cpu_id; @@ -556,19 +558,22 @@ static int cpu_request_microcode(const void *buf, siz= e_t bufsize) break; } =20 - /* Update cache if this patch covers current CPU */ - if ( microcode_fits(new_patch->mc_amd) !=3D 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) !=3D MIS_UCODE) && + (!patch || (compare_patch(new_patch, patch) =3D=3D NEW_UCODE)= ) ) { - error =3D apply_microcode(microcode_get_cache()); - if ( error ) - break; + struct microcode_patch *tmp =3D patch; + + patch =3D new_patch; + new_patch =3D tmp; } =20 + if ( new_patch ) + microcode_free_patch(new_patch); + if ( offset >=3D bufsize ) break; =20 @@ -599,13 +604,10 @@ static int cpu_request_microcode(const void *buf, siz= e_t bufsize) free_patch(mc_amd); =20 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 l= oaded - * successfully but subsequent container file processing encounters a - * failure. - */ - return error; + if ( error && !patch ) + patch =3D ERR_PTR(error); + + return patch; } =20 #ifdef CONFIG_HVM diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 23197ca..1b3ac93 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -285,14 +285,9 @@ static enum microcode_match_result compare_patch( : OLD_UCODE; } =20 -/* - * 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 =3D mc; unsigned long total_size =3D get_totalsize(mc_header); void *new_mc =3D xmalloc_bytes(total_size); struct microcode_patch *new_patch =3D xmalloc(struct microcode_patch); @@ -301,25 +296,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 =3D new_mc; =20 - /* Make sure that this patch covers current CPU */ - if ( microcode_update_match(mc) =3D=3D 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=3D%#x)\n", - smp_processor_id(), mc_header->rev, this_cpu(cpu_sig).rev); - - return 1; + return new_patch; } =20 static int apply_microcode(const struct microcode_patch *patch) @@ -398,26 +380,44 @@ static long get_next_ucode_from_buffer(void **mc, con= st u8 *buf, return offset + total_size; } =20 -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 =3D 0; int error =3D 0; void *mc; + struct microcode_patch *patch =3D NULL; =20 while ( (offset =3D get_next_ucode_from_buffer(&mc, buf, size, offset)= ) > 0 ) { + struct microcode_patch *new_patch; + error =3D microcode_sanity_check(mc); if ( error ) break; - error =3D get_matching_microcode(mc); - if ( error < 0 ) + + new_patch =3D alloc_microcode_patch(mc); + if ( IS_ERR(new_patch) ) + { + error =3D 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 =3D=3D 1 ) - error =3D 0; + if ( (microcode_update_match(&new_patch->mc_intel->hdr) !=3D MIS_U= CODE) && + (!patch || (compare_patch(new_patch, patch) =3D=3D NEW_UCODE)= ) ) + { + struct microcode_patch *tmp =3D patch; + + patch =3D new_patch; + new_patch =3D tmp; + } + + if ( new_patch ) + microcode_free_patch(new_patch); =20 xfree(mc); } @@ -426,10 +426,10 @@ static int cpu_request_microcode(const void *buf, siz= e_t size) if ( offset < 0 ) error =3D offset; =20 - if ( !error && match_cpu(microcode_get_cache()) ) - error =3D apply_microcode(microcode_get_cache()); + if ( error && !patch ) + patch =3D ERR_PTR(error); =20 - return error; + return patch; } =20 static const struct microcode_ops microcode_intel_ops =3D { diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcod= e.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 { }; =20 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; =20 -const struct microcode_patch *microcode_get_cache(void); -bool microcode_update_cache(struct microcode_patch *patch); void microcode_free_patch(struct microcode_patch *patch); =20 #endif /* ASM_X86__MICROCODE_H */ --=20 1.8.3.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel