Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1535149

Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation

From Borislav Petkov <bp@alien8.de>
Newsgroups linux.kernel
Subject Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation
Date 2016-12-02 20:30 +0100
Message-ID <sK1GV-7VR-27@gated-at.bofh.it> (permalink)
References (4 earlier) <sJZYt-6OX-19@gated-at.bofh.it> <sK0Bc-7kF-27@gated-at.bofh.it> <sK0KR-7nG-11@gated-at.bofh.it> <sK1dU-7x5-5@gated-at.bofh.it> <sK1nz-7Px-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Dec 02, 2016 at 11:03:50AM -0800, Linus Torvalds wrote:
> I'd really rather rjust mark it noinline with a comment. That way the
> return from the function acts as the control flow change.

Something like below?

It boots in a guest but that doesn't mean anything.

> 'sync_core()' doesn't help for other CPU's anyway, you need to do the
> cross-call IPI. So worrying about other CPU's is *not* a valid reason
> to keep a "sync_core()" call.

Yeah, no, I'm not crazy about it either - I was just sanity-checking all
call sites of apply_alternatives(). But as you say, we would've gotten
much bigger problems if other CPUs would walk in there on us.

> Seriously, the only reason I can see for "sync_core()" really is:
> 
>  - some deep non-serialized MSR  access or similar (ie things like
> firmware loading etc really might want it, and a mchine check might
> want it)

Yah, we do it in the #MC handler - apparently we need it there - and
in the microcode loader to tickle out the version of the microcode
currently applied into the MSR.

> The issues with modifying code while another CPU may be just about to
> access it is a separate issue. And as noted, "sync_core()" is not
> sufficient for that, you have to do a whole careful dance with
> single-byte debug instruction writes and then a final cross-call.
> 
> See the whole "text_poke_bp()" and "text_poke()" for *that* whole
> dance. That's a much more complex thing from the normal
> apply_alternatives().

Yeah.

---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5cb272a7a5a3..b1d0c35e6dcb 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -346,7 +346,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 +358,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 +669,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. */

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/4] CPUID-less CPU fixes and improvements Andy Lutomirski <luto@kernel.org> - 2016-11-30 21:40 +0100
  [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to serialize Andy Lutomirski <luto@kernel.org> - 2016-11-30 21:40 +0100
    Re: [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to  serialize Borislav Petkov <bp@alien8.de> - 2016-12-01 10:30 +0100
      Re: [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to serialize Andy Lutomirski <luto@amacapital.net> - 2016-12-01 18:10 +0100
        Re: [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to  serialize Andrew Cooper <andrew.cooper3@citrix.com> - 2016-12-01 18:50 +0100
      Re: [PATCH 4/4] x86/asm: Change sync_core() to use MOV to CR2 to  serialize Ingo Molnar <mingo@kernel.org> - 2016-12-02 08:40 +0100
  [PATCH 3/4] x86/microcode/intel: Replace sync_core() with cpuid_eax(1) Andy Lutomirski <luto@kernel.org> - 2016-11-30 21:40 +0100
    Re: [PATCH 3/4] x86/microcode/intel: Replace sync_core() with  cpuid_eax(1) Borislav Petkov <bp@alien8.de> - 2016-12-01 10:20 +0100
  [PATCH v2 6/6] x86/asm: Change sync_core() to use MOV to CR2 to serialize Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
  [PATCH v2 4/6] x86/paravirt: Make sync_core() be a paravirt op Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
  [PATCH v2 3/6] x86/microcode/intel: Replace sync_core() with cpuid_eax(1) Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
  [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
    Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Andrew Cooper <andrew.cooper3@citrix.com> - 2016-12-02 12:50 +0100
      Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 18:10 +0100
        Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Andrew Cooper <andrew.cooper3@citrix.com> - 2016-12-02 18:20 +0100
          Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Andrew Cooper <andrew.cooper3@citrix.com> - 2016-12-02 18:30 +0100
          Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 18:30 +0100
      Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-02 19:50 +0100
        Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 20:40 +0100
      Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-02 21:10 +0100
    Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 18:40 +0100
      Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@kernel.org> - 2016-12-02 18:40 +0100
        Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@kernel.org> - 2016-12-02 19:20 +0100
          Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 19:30 +0100
            Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@kernel.org> - 2016-12-02 20:00 +0100
              Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 20:10 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 20:30 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@alien8.de> - 2016-12-02 20:30 +0100
                [PATCH] x86/alternatives: Do not use sync_core() to serialize I$ Borislav Petkov <bp@alien8.de> - 2016-12-03 16:10 +0100
                Re: [PATCH] x86/alternatives: Do not use sync_core() to serialize I$ Andy Lutomirski <luto@amacapital.net> - 2016-12-03 18:20 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 20:40 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 21:50 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 22:20 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-03 00:00 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-03 00:20 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 20:40 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Andy Lutomirski <luto@amacapital.net> - 2016-12-02 20:30 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@alien8.de> - 2016-12-02 20:40 +0100
                Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@alien8.de> - 2016-12-02 20:30 +0100
        Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 19:20 +0100
        Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core()  implementation Borislav Petkov <bp@alien8.de> - 2016-12-03 13:50 +0100
  [PATCH v2 2/6] Revert "x86/boot: Fail the boot if !M486 and CPUID is missing" Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
  Re: [PATCH 0/4] CPUID-less CPU fixes and improvements Andy Lutomirski <luto@amacapital.net> - 2016-12-02 01:40 +0100
    Re: [PATCH 0/4] CPUID-less CPU fixes and improvements Ingo Molnar <mingo@kernel.org> - 2016-12-02 11:20 +0100
  [PATCH v2 0/6] CPUID-less CPU/sync_core fixes and improvements Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100
  [PATCH v2 1/6] x86/asm/32: Make sync_core() handle missing CPUID on all 32-bit kernels Andy Lutomirski <luto@kernel.org> - 2016-12-02 01:40 +0100

csiph-web