Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp4177525pxb; Mon, 8 Feb 2021 09:42:00 -0800 (PST) X-Google-Smtp-Source: ABdhPJzvJpot5RtQyN22rc4nXBj1UVmn3f4QS0raDscvQlOeU41kJD+FZDoU23dJl7sT/EmbR10Z X-Received: by 2002:a05:6402:3510:: with SMTP id b16mr17958482edd.242.1612806119957; Mon, 08 Feb 2021 09:41:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1612806119; cv=none; d=google.com; s=arc-20160816; b=dKl8ZtJDP5BGGaUHpf9Qr4Zwbfaur7+bGiDddPkSigK0/rs4wKwG7fToIEax8V6ULv an7Mx4xhMpDmyLtpPlxzK4CkSZ1ExqCjG46Dm6odcFw2fagKGnhF4JCsX8MHXSlLf49i eSWGUGXaiW1JXW+QIjF87wwquc12rtGYwc4rk1WJQi+ZJP4Xa509Imwvj4jVDOhDU7Ec F7UiSCgcwOQozgwOG2wonK7B3v3S0Q9LjPqSHt2dsgrDT7o7Rt77s8L/f9ij/TH1id1j 4zRY52uPDB3veZ8Hl6sVRDVQIuIthl4/TjHqoaJefDHAEh7HTzkxDsqcbY4RPgtlnp8z YUsA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=lXXhqc3E7mhRsbZIqamcMzEDFrwcnWV0uAAJCjZpkY0=; b=L7JqjW6QzmhkED/MYoXGLM+4jdtfawZhpWvcCzItWFsrksDAFvMvBXpl8rDYGevXuc LOFmjj1ldFbTQl27hTsfMjI9aEjJqzEYA+/4HWHFV//LaWhyQ+7o05fFQQuB3KUmEMCh ogb6r9aot29VyiWkkd3tFXqfBm04jSrIiEIZWUHpO8EnykAQ6y1oRikUtDdZR321wc2g UHwsLtIEIbhxdNfsXdkOdaKqTs6T2l4dP4ds/eWCbCxLkiJVf1CCVTNxURxK1CMcXssF g+flCO8dJorcAoF2Kyxc/SaVDeterOlXIyFAblgifZXpTmy5hI+Gz/9M7ZjP7nuPvHzu fUUg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=OX2lrgQK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id m9si13772045ejj.472.2021.02.08.09.41.36; Mon, 08 Feb 2021 09:41:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=OX2lrgQK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235015AbhBHRiI (ORCPT + 99 others); Mon, 8 Feb 2021 12:38:08 -0500 Received: from mail.kernel.org ([198.145.29.99]:34492 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233318AbhBHPSn (ORCPT ); Mon, 8 Feb 2021 10:18:43 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id AD71E64EC6; Mon, 8 Feb 2021 15:12:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1612797135; bh=ynb4w0o1+rhflb8tl3T9Vu5cz7uL0Xt2S0fGJqEV7Tw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OX2lrgQKZRV+GwrQoW72E7QCycDm19BDfhxhkpJaJyIk4Q9L/Jeba1kLL9yYP6hcX xi9r7v/JPxFToRszTn/aOA7EzyTMGdTP4n97MIkPWlQG1pu3yVB/VRu0UwfP4Jt7zj Y+fIe9YUHpFFRuUxuRx0dFQ5QQ4Z1dik6wZCYAKo= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Jan Kiszka , Dave Hansen , Borislav Petkov , "Peter Zijlstra (Intel)" , Thomas Gleixner Subject: [PATCH 5.4 55/65] x86/apic: Add extra serialization for non-serializing MSRs Date: Mon, 8 Feb 2021 16:01:27 +0100 Message-Id: <20210208145812.351639855@linuxfoundation.org> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210208145810.230485165@linuxfoundation.org> References: <20210208145810.230485165@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Dave Hansen commit 25a068b8e9a4eb193d755d58efcb3c98928636e0 upstream. Jan Kiszka reported that the x2apic_wrmsr_fence() function uses a plain MFENCE while the Intel SDM (10.12.3 MSR Access in x2APIC Mode) calls for MFENCE; LFENCE. Short summary: we have special MSRs that have weaker ordering than all the rest. Add fencing consistent with current SDM recommendations. This is not known to cause any issues in practice, only in theory. Longer story below: The reason the kernel uses a different semantic is that the SDM changed (roughly in late 2017). The SDM changed because folks at Intel were auditing all of the recommended fences in the SDM and realized that the x2apic fences were insufficient. Why was the pain MFENCE judged insufficient? WRMSR itself is normally a serializing instruction. No fences are needed because the instruction itself serializes everything. But, there are explicit exceptions for this serializing behavior written into the WRMSR instruction documentation for two classes of MSRs: IA32_TSC_DEADLINE and the X2APIC MSRs. Back to x2apic: WRMSR is *not* serializing in this specific case. But why is MFENCE insufficient? MFENCE makes writes visible, but only affects load/store instructions. WRMSR is unfortunately not a load/store instruction and is unaffected by MFENCE. This means that a non-serializing WRMSR could be reordered by the CPU to execute before the writes made visible by the MFENCE have even occurred in the first place. This means that an x2apic IPI could theoretically be triggered before there is any (visible) data to process. Does this affect anything in practice? I honestly don't know. It seems quite possible that by the time an interrupt gets to consume the (not yet) MFENCE'd data, it has become visible, mostly by accident. To be safe, add the SDM-recommended fences for all x2apic WRMSRs. This also leaves open the question of the _other_ weakly-ordered WRMSR: MSR_IA32_TSC_DEADLINE. While it has the same ordering architecture as the x2APIC MSRs, it seems substantially less likely to be a problem in practice. While writes to the in-memory Local Vector Table (LVT) might theoretically be reordered with respect to a weakly-ordered WRMSR like TSC_DEADLINE, the SDM has this to say: In x2APIC mode, the WRMSR instruction is used to write to the LVT entry. The processor ensures the ordering of this write and any subsequent WRMSR to the deadline; no fencing is required. But, that might still leave xAPIC exposed. The safest thing to do for now is to add the extra, recommended LFENCE. [ bp: Massage commit message, fix typos, drop accidentally added newline to tools/arch/x86/include/asm/barrier.h. ] Reported-by: Jan Kiszka Signed-off-by: Dave Hansen Signed-off-by: Borislav Petkov Acked-by: Peter Zijlstra (Intel) Acked-by: Thomas Gleixner Cc: Link: https://lkml.kernel.org/r/20200305174708.F77040DD@viggo.jf.intel.com Signed-off-by: Greg Kroah-Hartman --- arch/x86/include/asm/apic.h | 10 ---------- arch/x86/include/asm/barrier.h | 18 ++++++++++++++++++ arch/x86/kernel/apic/apic.c | 4 ++++ arch/x86/kernel/apic/x2apic_cluster.c | 6 ++++-- arch/x86/kernel/apic/x2apic_phys.c | 9 ++++++--- 5 files changed, 32 insertions(+), 15 deletions(-) --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -197,16 +197,6 @@ static inline bool apic_needs_pit(void) #endif /* !CONFIG_X86_LOCAL_APIC */ #ifdef CONFIG_X86_X2APIC -/* - * Make previous memory operations globally visible before - * sending the IPI through x2apic wrmsr. We need a serializing instruction or - * mfence for this. - */ -static inline void x2apic_wrmsr_fence(void) -{ - asm volatile("mfence" : : : "memory"); -} - static inline void native_apic_msr_write(u32 reg, u32 v) { if (reg == APIC_DFR || reg == APIC_ID || reg == APIC_LDR || --- a/arch/x86/include/asm/barrier.h +++ b/arch/x86/include/asm/barrier.h @@ -84,4 +84,22 @@ do { \ #include +/* + * Make previous memory operations globally visible before + * a WRMSR. + * + * MFENCE makes writes visible, but only affects load/store + * instructions. WRMSR is unfortunately not a load/store + * instruction and is unaffected by MFENCE. The LFENCE ensures + * that the WRMSR is not reordered. + * + * Most WRMSRs are full serializing instructions themselves and + * do not require this barrier. This is only required for the + * IA32_TSC_DEADLINE and X2APIC MSRs. + */ +static inline void weak_wrmsr_fence(void) +{ + asm volatile("mfence; lfence" : : : "memory"); +} + #endif /* _ASM_X86_BARRIER_H */ --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -472,6 +473,9 @@ static int lapic_next_deadline(unsigned { u64 tsc; + /* This MSR is special and need a special fence: */ + weak_wrmsr_fence(); + tsc = rdtsc(); wrmsrl(MSR_IA32_TSC_DEADLINE, tsc + (((u64) delta) * TSC_DIVISOR)); return 0; --- a/arch/x86/kernel/apic/x2apic_cluster.c +++ b/arch/x86/kernel/apic/x2apic_cluster.c @@ -29,7 +29,8 @@ static void x2apic_send_IPI(int cpu, int { u32 dest = per_cpu(x86_cpu_to_logical_apicid, cpu); - x2apic_wrmsr_fence(); + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); __x2apic_send_IPI_dest(dest, vector, APIC_DEST_LOGICAL); } @@ -41,7 +42,8 @@ __x2apic_send_IPI_mask(const struct cpum unsigned long flags; u32 dest; - x2apic_wrmsr_fence(); + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); local_irq_save(flags); tmpmsk = this_cpu_cpumask_var_ptr(ipi_mask); --- a/arch/x86/kernel/apic/x2apic_phys.c +++ b/arch/x86/kernel/apic/x2apic_phys.c @@ -43,7 +43,8 @@ static void x2apic_send_IPI(int cpu, int { u32 dest = per_cpu(x86_cpu_to_apicid, cpu); - x2apic_wrmsr_fence(); + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); __x2apic_send_IPI_dest(dest, vector, APIC_DEST_PHYSICAL); } @@ -54,7 +55,8 @@ __x2apic_send_IPI_mask(const struct cpum unsigned long this_cpu; unsigned long flags; - x2apic_wrmsr_fence(); + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); local_irq_save(flags); @@ -125,7 +127,8 @@ void __x2apic_send_IPI_shorthand(int vec { unsigned long cfg = __prepare_ICR(which, vector, 0); - x2apic_wrmsr_fence(); + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); native_x2apic_icr_write(cfg, 0); }