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);