From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754253AbcGHIzY (ORCPT ); Fri, 8 Jul 2016 04:55:24 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33080 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754055AbcGHIzT (ORCPT ); Fri, 8 Jul 2016 04:55:19 -0400 Date: Fri, 8 Jul 2016 10:55:15 +0200 From: Ingo Molnar To: Jacob Pan Cc: LKML , Peter Zijlstra , Ingo Molnar , "H. Peter Anvin" , X86 Kernel , Arjan van de Ven , Len Brown Subject: Re: [PATCH] x86: add workaround monitor bug Message-ID: <20160708085515.GA4682@gmail.com> References: <1467824514-20607-1-git-send-email-jacob.jun.pan@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467824514-20607-1-git-send-email-jacob.jun.pan@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Jacob Pan wrote: > From: Peter Zijlstra > > Monitored cached line may not wake up from mwait on certain > Goldmont based CPUs. This patch will avoid calling > current_set_polling_and_test() and thereby not set the TIF_ flag. > The result is that we'll always send IPIs for wakeups. > > Signed-off-by: Peter Zijlstra > Signed-off-by: Jacob Pan > --- > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/mwait.h | 2 +- > arch/x86/kernel/cpu/intel.c | 5 +++++ > arch/x86/kernel/process.c | 2 +- > 4 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h > index 78dbd28..197a3f4 100644 > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -304,6 +304,7 @@ > #define X86_BUG_SYSRET_SS_ATTRS X86_BUG(8) /* SYSRET doesn't fix up SS attrs */ > #define X86_BUG_NULL_SEG X86_BUG(9) /* Nulling a selector preserves the base */ > #define X86_BUG_SWAPGS_FENCE X86_BUG(10) /* SWAPGS without input dep on GS */ > +#define X86_BUG_MONITOR X86_BUG(11) /* IPI required to wake up remote cpu */ > > > #ifdef CONFIG_X86_32 > diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h > index 0deeb2d..f37f2d8 100644 > --- a/arch/x86/include/asm/mwait.h > +++ b/arch/x86/include/asm/mwait.h > @@ -97,7 +97,7 @@ static inline void __sti_mwait(unsigned long eax, unsigned long ecx) > */ > static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx) > { > - if (!current_set_polling_and_test()) { > + if (static_cpu_has_bug(X86_BUG_MONITOR) || !current_set_polling_and_test()) { Hm, this might be suboptimal: if MONITOR/MWAIT is implemented by setting the exclusive flag for the monitored memory address and then snooping for cache invalidation requests for that cache line, then not modifying the ->flags value with TIF_POLLING_NRFLAG makes MWAIT not wake up - only the IPI would wake it up. I think a better approach would be to still optimistically modify the ->flags value _AND_ to also send an IPI, to make sure the wakeup is not lost. This means that the woken CPU will wake up much faster (no IPI latency). (The system will still bear the ovehread of sending and receiving the IPI, but that cost is unavoidable if there's no other workaround for this erratum.) Thanks, Ingo