Message-ID: <52311c3d-83cf-4dc4-bbcb-5fbca8eb249c@intel.com>
Date: Tue, 14 Jan 2025 13:45:39 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Valentin Schneider <vschneid@...hat.com>, linux-kernel@...r.kernel.org,
 x86@...nel.org, virtualization@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, loongarch@...ts.linux.dev,
 linux-riscv@...ts.infradead.org, linux-perf-users@...r.kernel.org,
 xen-devel@...ts.xenproject.org, kvm@...r.kernel.org,
 linux-arch@...r.kernel.org, rcu@...r.kernel.org,
 linux-hardening@...r.kernel.org, linux-mm@...ck.org,
 linux-kselftest@...r.kernel.org, bpf@...r.kernel.org,
 bcm-kernel-feedback-list@...adcom.com
Cc: Peter Zijlstra <peterz@...radead.org>, Juergen Gross <jgross@...e.com>,
 Ajay Kaher <ajay.kaher@...adcom.com>,
 Alexey Makhalov <alexey.amakhalov@...adcom.com>,
 Russell King <linux@...linux.org.uk>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, Arnaldo Carvalho de Melo
 <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
 Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>,
 Boris Ostrovsky <boris.ostrovsky@...cle.com>,
 Josh Poimboeuf <jpoimboe@...nel.org>,
 Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
 Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
 <pbonzini@...hat.com>, Andy Lutomirski <luto@...nel.org>,
 Arnd Bergmann <arnd@...db.de>, Frederic Weisbecker <frederic@...nel.org>,
 "Paul E. McKenney" <paulmck@...nel.org>, Jason Baron <jbaron@...mai.com>,
 Steven Rostedt <rostedt@...dmis.org>, Ard Biesheuvel <ardb@...nel.org>,
 Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
 Joel Fernandes <joel@...lfernandes.org>,
 Josh Triplett <josh@...htriplett.org>, Boqun Feng <boqun.feng@...il.com>,
 Uladzislau Rezki <urezki@...il.com>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Lai Jiangshan <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
 Juri Lelli <juri.lelli@...hat.com>, Clark Williams <williams@...hat.com>,
 Yair Podemsky <ypodemsk@...hat.com>, Tomas Glozar <tglozar@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>,
 Dietmar Eggemann <dietmar.eggemann@....com>, Ben Segall
 <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
 Kees Cook <kees@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Christoph Hellwig <hch@...radead.org>, Shuah Khan <shuah@...nel.org>,
 Sami Tolvanen <samitolvanen@...gle.com>, Miguel Ojeda <ojeda@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 "Mike Rapoport (Microsoft)" <rppt@...nel.org>,
 Samuel Holland <samuel.holland@...ive.com>, Rong Xu <xur@...gle.com>,
 Nicolas Saenz Julienne <nsaenzju@...hat.com>,
 Geert Uytterhoeven <geert@...ux-m68k.org>,
 Yosry Ahmed <yosryahmed@...gle.com>,
 "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 "Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
 Jinghao Jia <jinghao7@...inois.edu>, Luis Chamberlain <mcgrof@...nel.org>,
 Randy Dunlap <rdunlap@...radead.org>, Tiezhu Yang <yangtiezhu@...ngson.cn>
Subject: Re: [PATCH v4 26/30] x86,tlb: Make __flush_tlb_global()
 noinstr-compliant

On 1/14/25 09:51, Valentin Schneider wrote:
> +	cr4 = this_cpu_read(cpu_tlbstate.cr4);
> +	asm volatile("mov %0,%%cr4": : "r" (cr4 ^ X86_CR4_PGE) : "memory");
> +	asm volatile("mov %0,%%cr4": : "r" (cr4) : "memory");
> +	/*
> +	 * In lieu of not having the pinning crap, hard fail if CR4 doesn't
> +	 * match the expected value. This ensures that anybody doing dodgy gets
> +	 * the fallthrough check.
> +	 */
> +	BUG_ON(cr4 != this_cpu_read(cpu_tlbstate.cr4));

Let's say someone managed to write to cpu_tlbstate.cr4 where they
cleared one of the pinned bits.

Before this patch, CR4 pinning would WARN_ONCE() about it pretty quickly
and also reset the cleared bits.

After this patch, the first native_flush_tlb_global() can clear pinned
bits, at least until native_write_cr4() gets called the next time. That
seems like it'll undermine CR4 pinning at least somewhat.

What keeps native_write_cr4() from being noinstr-compliant now? Is it
just the WARN_ONCE()?

If so, I'd kinda rather have a native_write_cr4_nowarn() that's
noinstr-compliant but retains all the other CR4 pinning behavior. Would
something like the attached patch be _worse_?
View attachment "cr4.patch" of type "text/x-patch" (1484 bytes)

Powered by blists - more mailing lists