Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755374AbcLBTU7 (ORCPT ); Fri, 2 Dec 2016 14:20:59 -0500 Received: from mail.skyhub.de ([78.46.96.112]:39412 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753436AbcLBTUy (ORCPT ); Fri, 2 Dec 2016 14:20:54 -0500 Date: Fri, 2 Dec 2016 20:20:50 +0100 From: Borislav Petkov To: Linus Torvalds Cc: Borislav Petkov , Andy Lutomirski , Peter Anvin , the arch/x86 maintainers , One Thousand Gnomes , "linux-kernel@vger.kernel.org" , Brian Gerst , Matthew Whitehead , Henrique de Moraes Holschuh , Peter Zijlstra , Andrew Cooper Subject: Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation Message-ID: <20161202192050.l5l3rcwems6hptub@pd.tnic> References: <0a21157c2233ba7d0781bbf07866b3f2d7e7c25d.1480638597.git.luto@kernel.org> <20161202180343.gehqor7lgtmzwqq3@pd.tnic> <20161202185008.tdziqrzi4a3axord@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3026 Lines: 79 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.