From nobody Mon Feb 9 16:53:57 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=1566177780; cv=none; d=zoho.com; s=zohoarc; b=Q/rQ4ebev7HsvHbX759S7MAe0nBkGBu+UWysYTKs9LPg9o8YfhCo6hDtRBz4C25H8PmbeaFe7eYaT3IMe51vQb54z89B5AXST3Nn5muXoRKK+ZZJrNG1rEmKba0+ZRnG9+9nli8Eop6Ksvqdv6ekhccrGDSuOY0sBa+3RauYJfk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1566177780; 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=AB1dn6UFTkQZp2znGZ6c8yZkFJ1uAOHL57zCP4HNdPU=; b=A8QFsTQdkmOk6NxerX0tLUMijwKHi9QWrSNRUDLpJTSNn7aN17xNbKpiIqcpWGvZn5a6a4whpMVXbGu4rXTLD41F7eyezBnnCjWhZ9PdNygJ05xKX5aZpE0RpJJVDgohJYHBCl5HEXazeT7TvsHlV40OAIMjjhbF57JoNdZkL14= 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 1566177780257425.89684191538527; Sun, 18 Aug 2019 18:23:00 -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 1hzWMu-0001Kb-UU; Mon, 19 Aug 2019 01:21:56 +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 1hzWMs-0001Ik-Qp for xen-devel@lists.xenproject.org; Mon, 19 Aug 2019 01:21:54 +0000 Received: from mga03.intel.com (unknown [134.134.136.65]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id ba902162-c21f-11e9-8be6-12813bfff9fa; Mon, 19 Aug 2019 01:21:54 +0000 (UTC) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Aug 2019 18:21:54 -0700 Received: from gao-cwp.sh.intel.com ([10.239.159.26]) by orsmga001.jf.intel.com with ESMTP; 18 Aug 2019 18:21:52 -0700 X-Inumbo-ID: ba902162-c21f-11e9-8be6-12813bfff9fa X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,403,1559545200"; d="scan'208";a="261683929" From: Chao Gao To: xen-devel@lists.xenproject.org Date: Mon, 19 Aug 2019 09:25:23 +0800 Message-Id: <1566177928-19114-11-git-send-email-chao.gao@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1566177928-19114-1-git-send-email-chao.gao@intel.com> References: <1566177928-19114-1-git-send-email-chao.gao@intel.com> Subject: [Xen-devel] [PATCH v9 10/15] 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: 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. Signed-off-by: Chao Gao Reviewed-by: Roger Pau Monn=C3=A9 --- 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 | 177 ++++++++++++++++++++++++++----------= ---- 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(+), 114 deletions(-) diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 0e9322a..a2febc7 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, uint32_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,13 +227,6 @@ 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) { @@ -250,49 +250,71 @@ 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_unlock(µcode_mutex); + if ( patch ) + err =3D microcode_ops->apply_microcode(patch); + else if ( microcode_cache ) + { + spin_lock(µcode_mutex); + err =3D microcode_ops->apply_microcode(microcode_cache); + if ( err =3D=3D -EIO ) + { + microcode_free_patch(microcode_cache); + microcode_cache =3D NULL; + } + spin_unlock(µcode_mutex); + } + else + /* No patch to update */ + err =3D -ENOENT; =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; =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 ( !microcode_update_cpu(patch) && patch ) + { + spin_lock(µcode_mutex); + microcode_update_cache(patch); + spin_unlock(µcode_mutex); + patch =3D NULL; + } =20 if ( microcode_ops->end_update ) microcode_ops->end_update(); =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); + 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); =20 - error =3D info->error; - xfree(info); - return error; + /* Free the patch if no CPU has loaded it successfully. */ + if ( patch ) + microcode_free_patch(patch); + + return 0; } =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; @@ -300,32 +322,44 @@ 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 ) + if ( copy_from_guest(buffer, buf, len) ) { - xfree(info); - return ret; + ret =3D -EFAULT; + goto free; } =20 - info->buffer_size =3D len; - info->error =3D 0; - info->cpu =3D cpumask_first(&cpu_online_map); - if ( microcode_ops->start_update ) { ret =3D microcode_ops->start_update(); if ( ret !=3D 0 ) - { - xfree(info); - return ret; - } + goto free; } =20 - return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); + patch =3D parse_blob(buffer, len); + if ( IS_ERR(patch) ) + { + ret =3D PTR_ERR(patch); + printk(XENLOG_INFO "Parsing microcode blob error %d\n", ret); + goto free; + } + + if ( !patch ) + { + printk(XENLOG_INFO "No ucode found. Update aborted!\n"); + ret =3D -EINVAL; + goto free; + } + + ret =3D continue_hypercall_on_cpu(cpumask_first(&cpu_online_map), + do_microcode_update, patch); + + free: + xfree(buffer); + return ret; } =20 static int __init microcode_init(void) @@ -372,23 +406,46 @@ 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; + + if ( microcode_ops->start_update ) rc =3D microcode_ops->start_update(); =20 if ( rc ) return rc; =20 - rc =3D microcode_update_cpu(data, len); + patch =3D parse_blob(data, len); + if ( IS_ERR(patch) ) + { + printk(XENLOG_INFO "Parsing microcode blob error %ld\n", + PTR_ERR(patch)); + return PTR_ERR(patch); + } + + if ( !patch ) + { + printk(XENLOG_INFO "No ucode found. Update aborted!\n"); + return -EINVAL; + } =20 - if ( microcode_ops->end_update ) - microcode_ops->end_update(); + spin_lock(µcode_mutex); + rc =3D microcode_update_cache(patch); + spin_unlock(µcode_mutex); =20 - return rc; + ASSERT(rc); } - else - return -ENOMEM; + + rc =3D microcode_update_cpu(NULL); + + if ( microcode_ops->end_update ) + microcode_ops->end_update(); + + 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 21cdfe0..6353323 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -447,9 +447,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; @@ -548,19 +550,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 @@ -593,13 +598,10 @@ static int cpu_request_microcode(const void *buf, siz= e_t bufsize) xfree(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 static int start_update(void) diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 8c0008c..96b38f8 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -282,14 +282,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); @@ -298,25 +293,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) @@ -392,26 +374,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); } @@ -420,10 +420,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 8c7de9d..8e71615 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); @@ -41,8 +42,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