All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Borislav Petkov <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, hpa@zytor.com,
	torvalds@linux-foundation.org, peterz@infradead.org,
	tglx@linutronix.de, bp@suse.de, luto@amacapital.net,
	gnomes@lxorguk.ukuu.org.uk, luto@kernel.org, hmh@hmh.eng.br,
	brgerst@gmail.com, mingo@kernel.org, tedheadster@gmail.com,
	andrew.cooper3@citrix.com
Subject: [tip:x86/urgent] x86/alternatives: Do not use sync_core() to serialize I$
Date: Tue, 20 Dec 2016 01:35:08 -0800	[thread overview]
Message-ID: <tip-34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466@git.kernel.org> (raw)
In-Reply-To: <20161203150258.vwr5zzco7ctgc4pe@pd.tnic>

Commit-ID:  34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466
Gitweb:     http://git.kernel.org/tip/34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Sat, 3 Dec 2016 16:02:58 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 20 Dec 2016 09:36:42 +0100

x86/alternatives: Do not use sync_core() to serialize I$

We use sync_core() in the alternatives code to stop speculative
execution of prefetched instructions because we are potentially changing
them and don't want to execute stale bytes.

What it does on most machines is call CPUID which is a serializing
instruction. And that's expensive.

However, the instruction cache is serialized when we're on the local CPU
and are changing the data through the same virtual address. So then, we
don't need the serializing CPUID but a simple control flow change. Last
being accomplished with a CALL/RET which the noinline causes.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Matthew Whitehead <tedheadster@gmail.com>
Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20161203150258.vwr5zzco7ctgc4pe@pd.tnic
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/alternative.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5cb272a..c5b8f76 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -337,7 +337,11 @@ done:
 		n_dspl, (unsigned long)orig_insn + n_dspl + repl_len);
 }
 
-static void __init_or_module optimize_nops(struct alt_instr *a, u8 *instr)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+static void __init_or_module noinline optimize_nops(struct alt_instr *a, u8 *instr)
 {
 	unsigned long flags;
 
@@ -346,7 +350,6 @@ static void __init_or_module optimize_nops(struct alt_instr *a, u8 *instr)
 
 	local_irq_save(flags);
 	add_nops(instr + (a->instrlen - a->padlen), a->padlen);
-	sync_core();
 	local_irq_restore(flags);
 
 	DUMP_BYTES(instr, a->instrlen, "%p: [%d:%d) optimized NOPs: ",
@@ -359,9 +362,12 @@ static void __init_or_module optimize_nops(struct alt_instr *a, u8 *instr)
  * This implies that asymmetric systems where APs have less capabilities than
  * the boot processor are not handled. Tough. Make sure you disable such
  * features by hand.
+ *
+ * Marked "noinline" to cause control flow change and thus insn cache
+ * to refetch changed I$ lines.
  */
-void __init_or_module apply_alternatives(struct alt_instr *start,
-					 struct alt_instr *end)
+void __init_or_module noinline apply_alternatives(struct alt_instr *start,
+						  struct alt_instr *end)
 {
 	struct alt_instr *a;
 	u8 *instr, *replacement;
@@ -667,7 +673,6 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
 	unsigned long flags;
 	local_irq_save(flags);
 	memcpy(addr, opcode, len);
-	sync_core();
 	local_irq_restore(flags);
 	/* Could also do a CLFLUSH here to speed up CPU recovery; but
 	   that causes hangs on some VIA CPUs. */

  parent reply	other threads:[~2016-12-20  9:36 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-30 20:34 [PATCH 0/4] CPUID-less CPU fixes and improvements Andy Lutomirski
2016-11-30 20:34 ` [PATCH 1/4] x86/asm/32: Make sync_core() handle missing CPUID on all 32-bit kernels Andy Lutomirski
2016-11-30 20:34 ` [PATCH 2/4] Revert "x86/boot: Fail the boot if !M486 and CPUID is missing" Andy Lutomirski
2016-12-01  5:53   ` Peter Zijlstra
2016-12-01  9:02     ` Borislav Petkov
2016-12-01 10:07       ` Thomas Gleixner
2016-12-01 11:15         ` [PATCH] x86/CPU: Add X86_FEATURE_CPUID Borislav Petkov
2016-12-01 17:00           ` Andy Lutomirski
2016-12-02  0:33       ` [PATCH 2/4] Revert "x86/boot: Fail the boot if !M486 and CPUID is missing" Andy Lutomirski
2016-12-01 17:00     ` Andy Lutomirski
2016-11-30 20:34 ` [PATCH 3/4] x86/microcode/intel: Replace sync_core() with cpuid_eax(1) Andy Lutomirski
2016-12-01  9:11   ` Borislav Petkov
2016-11-30 20:34 ` [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to serialize Andy Lutomirski
2016-12-01  9:22   ` Borislav Petkov
2016-12-01 17:08     ` Andy Lutomirski
2016-12-01 17:46       ` Andrew Cooper
2016-12-02  7:34     ` Ingo Molnar
2016-12-02  0:34 ` [PATCH v2 0/6] CPUID-less CPU/sync_core fixes and improvements Andy Lutomirski
2016-12-02  0:34   ` [PATCH v2 1/6] x86/asm/32: Make sync_core() handle missing CPUID on all 32-bit kernels Andy Lutomirski
2016-12-02  0:34   ` [PATCH v2 2/6] Revert "x86/boot: Fail the boot if !M486 and CPUID is missing" Andy Lutomirski
2016-12-02  0:34   ` [PATCH v2 3/6] x86/microcode/intel: Replace sync_core() with cpuid_eax(1) Andy Lutomirski
2016-12-02  0:35   ` [PATCH v2 4/6] x86/paravirt: Make sync_core() be a paravirt op Andy Lutomirski
2016-12-02  0:35   ` [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski
2016-12-02 11:44     ` Andrew Cooper
2016-12-02 11:44     ` Andrew Cooper
2016-12-02 17:07       ` Andy Lutomirski
2016-12-02 17:07       ` Andy Lutomirski
2016-12-02 17:16         ` Andrew Cooper
2016-12-02 17:16         ` Andrew Cooper
2016-12-02 17:23           ` Andy Lutomirski
2016-12-02 17:26             ` Andrew Cooper
2016-12-02 17:26             ` Andrew Cooper
2016-12-02 17:23           ` Andy Lutomirski
2016-12-02 18:50       ` Boris Ostrovsky
2016-12-02 18:50       ` Boris Ostrovsky
2016-12-02 19:34         ` Andy Lutomirski
2016-12-02 19:34         ` Andy Lutomirski
2016-12-02 20:09       ` Boris Ostrovsky
2016-12-02 20:09         ` Boris Ostrovsky
2016-12-02 17:32     ` Linus Torvalds
2016-12-02 17:38       ` Andy Lutomirski
2016-12-02 17:53         ` Linus Torvalds
2016-12-02 18:03         ` Borislav Petkov
2016-12-02 18:27           ` Linus Torvalds
2016-12-02 18:50             ` Borislav Petkov
2016-12-02 19:03               ` Linus Torvalds
2016-12-02 19:20                 ` Borislav Petkov
2016-12-02 19:24                   ` Linus Torvalds
2016-12-02 19:28                     ` Borislav Petkov
2016-12-03 15:02                       ` [PATCH] x86/alternatives: Do not use sync_core() to serialize I$ Borislav Petkov
2016-12-03 17:05                         ` Andy Lutomirski
2016-12-20  7:58                         ` [tip:x86/urgent] " tip-bot for Borislav Petkov
2016-12-20  9:35                         ` tip-bot for Borislav Petkov [this message]
2016-12-02 19:30                     ` [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski
2016-12-02 19:35                       ` Linus Torvalds
2016-12-02 20:41                         ` Andy Lutomirski
2016-12-02 21:10                           ` Linus Torvalds
2016-12-02 22:55                             ` Andy Lutomirski
2016-12-02 23:09                               ` Linus Torvalds
2016-12-02 19:23                 ` Andy Lutomirski
2016-12-02 19:30                   ` Borislav Petkov
2016-12-03 12:44         ` Borislav Petkov
2016-12-02  0:35   ` [PATCH v2 6/6] x86/asm: Change sync_core() to use MOV to CR2 to serialize Andy Lutomirski
2016-12-02  0:36 ` [PATCH 0/4] CPUID-less CPU fixes and improvements Andy Lutomirski
2016-12-02 10:17   ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bp@suse.de \
    --cc=brgerst@gmail.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=hmh@hmh.eng.br \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tedheadster@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.