From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 10 Dec 2018 23:01:20 -0000 Received: from mail-lj1-x242.google.com ([2a00:1450:4864:20::242]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1gWUYB-0006b9-4a for speck@linutronix.de; Tue, 11 Dec 2018 00:01:19 +0100 Received: by mail-lj1-x242.google.com with SMTP id e5-v6so11232326lja.4 for ; Mon, 10 Dec 2018 15:01:19 -0800 (PST) Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com. [209.85.167.44]) by smtp.gmail.com with ESMTPSA id e19-v6sm2394436ljf.67.2018.12.10.15.01.11 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Dec 2018 15:01:11 -0800 (PST) Received: by mail-lf1-f44.google.com with SMTP id n18so9311405lfh.6 for ; Mon, 10 Dec 2018 15:01:11 -0800 (PST) MIME-Version: 1.0 References: <0d6a3fbe4c511152a0f5350e62e9e09ec545f709.1544464266.git.ak@linux.intel.com> In-Reply-To: <0d6a3fbe4c511152a0f5350e62e9e09ec545f709.1544464266.git.ak@linux.intel.com> From: Linus Torvalds Date: Mon, 10 Dec 2018 15:00:54 -0800 Message-ID: Subject: [MODERATED] Re: [PATCH v2 3/8] MDSv2 5 Content-Type: text/plain; charset="UTF-8" To: speck@linutronix.de List-ID: Honestly, this looks completely bogus. On Mon, Dec 10, 2018 at 2:37 PM speck for Andi Kleen wrote: > > +void clear_cpu_buffers_idle(void) > +{ > + if (cpu_smt_control != CPU_SMT_ENABLED) > + return; > + /* Has to be memory form, don't modify to use an register */ > + alternative_input("", > + "pushq %[kernelds]; verw (%%rsp) ; addq $8,%%rsp \n", > + X86_FEATURE_MB_CLEAR, > + [kernelds] "i" (__KERNEL_DS)); > +} So you have a non-inline function that - has a test to return early - does a *single* instruction - the instruction gets nop'ed out if it's not valid all of which just *screams* "that's wrong" to me. Why isn't it inline and a static branch? The actual asm sequence also seems bad to me. Why isn't that just using int val = __KERNEL_DS; with the asm just doing "verw %[kernelds]" .. [kernelds] "m" (val) instead, letting gcc do the stack setup part. Maybe none of this matters simply because it's in the idle case, but the exact same issues happen for the kernel exit case. And no, we're not doing that insane kernel exit software case by default. Not without a whole lot of examples of horrid badness. The data that system calls touch is basically already user data. Sure, there may be kernel pointers etc there, but we've already accepted those leaking locally for all the usual reasons that we have no control over. So the whole "let's add crazy long sequences to every kernel exit" is not going to happen. Not without a lot more explanations. Linus