Message-ID: <xhsmh8qr9hikc.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Fri, 17 Jan 2025 10:54:11 +0100
From: Valentin Schneider <vschneid@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: 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, Peter Zijlstra
 <peterz@...radead.org>, Nicolas Saenz Julienne <nsaenzju@...hat.com>,
 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>, Kan Liang <kan.liang@...ux.intel.com>, Boris
 Ostrovsky <boris.ostrovsky@...cle.com>, Josh Poimboeuf
 <jpoimboe@...nel.org>, Pawan Gupta <pawan.kumar.gupta@...ux.intel.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>, 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 25/30] context_tracking,x86: Defer kernel text
 patching IPIs

On 14/01/25 13:48, Sean Christopherson wrote:
> On Tue, Jan 14, 2025, Sean Christopherson wrote:
>> On Tue, Jan 14, 2025, Valentin Schneider wrote:
>> > +/**
>> > + * is_kernel_noinstr_text - checks if the pointer address is located in the
>> > + *                    .noinstr section
>> > + *
>> > + * @addr: address to check
>> > + *
>> > + * Returns: true if the address is located in .noinstr, false otherwise.
>> > + */
>> > +static inline bool is_kernel_noinstr_text(unsigned long addr)
>> > +{
>> > +	return addr >= (unsigned long)__noinstr_text_start &&
>> > +	       addr < (unsigned long)__noinstr_text_end;
>> > +}
>>
>> This doesn't do the right thing for modules, which matters because KVM can be
>> built as a module on x86, and because context tracking understands transitions
>> to GUEST mode, i.e. CPUs that are running in a KVM guest will be treated as not
>> being in the kernel, and thus will have IPIs deferred.  If KVM uses a static key
>> or branch between guest_state_enter_irqoff() and guest_state_exit_irqoff(), the
>> patching code won't wait for CPUs to exit guest mode, i.e. KVM could theoretically
>> use the wrong static path.
>>
>> I don't expect this to ever cause problems in practice, because patching code in
>> KVM's VM-Enter/VM-Exit path that has *functional* implications, while CPUs are
>> actively running guest code, would be all kinds of crazy.  But I do think we
>> should plug the hole.
>>
>> If this issue is unique to KVM, i.e. is not a generic problem for all modules (I
>> assume module code generally isn't allowed in the entry path, even via NMI?), one
>> idea would be to let KVM register its noinstr section for text poking.
>
> Another idea would be to track which keys/branches are tagged noinstr, i.e. generate
> the information at compile time instead of doing lookups at runtime.  The biggest
> downside I can think of is that it would require plumbing in the information to
> text_poke_bp_batch().

IIUC that's what I went for in v3:

https://lore.kernel.org/lkml/20241119153502.41361-11-vschneid@redhat.com

but, modules notwithstanding, simply checking if the patched instruction is
in .noinstr was a lot neater.

Powered by blists - more mailing lists