linux-kernel - Re: [PATCH] KVM: x86: Unify L1TF flushing under per-CPU variable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <DDHCMHEN3389.3KHOEVYLJY26S@google.com>
Date: Mon, 13 Oct 2025 16:52:24 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>, Brendan Jackman <jackmanb@...gle.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, 
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>, 
	"H. Peter Anvin" <hpa@...or.com>, Paolo Bonzini <pbonzini@...hat.com>, <linux-kernel@...r.kernel.org>, 
	<kvm@...r.kernel.org>
Subject: Re: [PATCH] KVM: x86: Unify L1TF flushing under per-CPU variable

On Mon Oct 13, 2025 at 4:31 PM UTC, Sean Christopherson wrote:
> On Mon, Oct 13, 2025, Brendan Jackman wrote:
>> Currently the tracking of the need to flush L1D for L1TF is tracked by
>> two bits: one per-CPU and one per-vCPU.
>> 
>> The per-vCPU bit is always set when the vCPU shows up on a core, so
>> there is no interesting state that's truly per-vCPU. Indeed, this is a
>> requirement, since L1D is a part of the physical CPU.
>> 
>> So simplify this by combining the two bits.
>> 
>> Since this requires a DECLARE_PER_CPU() which belongs in kvm_host.h,
>
> No, it doesn't belong in kvm_host.h.
>
> One of my biggest gripes with Google's prodkernel is that we only build with one
> .config, and that breeds bad habits and some truly awful misconceptions about
> kernel programming because engineers tend to treat that one .config as gospel.
>
> Information *never* flows from a module to code that can _only_ be built-in, i.e.
> to the so called "core kernel".  KVM x86 can be, and _usually_ is, built as a module,
> kvm.ko.  Thus, KVM should *never* declare/provide symbols that are used by the
> core kernel, because it simply can't work (without some abusrdly stupid logic)
> when kvm.ko is built as a module:
>
>   ld: vmlinux.o: in function `common_interrupt':
>   arch/x86/include/asm/kvm_host.h:2486:(.noinstr.text+0x2b56): undefined reference to `l1tf_flush_l1d'
>   ld: vmlinux.o: in function `sysvec_x86_platform_ipi':
>   arch/x86/include/asm/kvm_host.h:2486:(.noinstr.text+0x2bf1): undefined reference to `l1tf_flush_l1d'
>   ld: vmlinux.o: in function `sysvec_kvm_posted_intr_ipi':
>   arch/x86/include/asm/kvm_host.h:2486:(.noinstr.text+0x2c81): undefined reference to `l1tf_flush_l1d'
>   ld: vmlinux.o: in function `sysvec_kvm_posted_intr_wakeup_ipi':
>   arch/x86/include/asm/kvm_host.h:2486:(.noinstr.text+0x2cd1): undefined reference to `l1tf_flush_l1d'
>   ld: vmlinux.o: in function `sysvec_kvm_posted_intr_nested_ipi':
>   arch/x86/include/asm/kvm_host.h:2486:(.noinstr.text+0x2d61): undefined reference to `l1tf_flush_l1d'
>   ld: vmlinux.o:arch/x86/include/asm/kvm_host.h:2486: more undefined references to `l1tf_flush_l1d' follow
>
> Because prodkernel's .config forces CONFIG_KVM=y (for equally awful reasons),
> Google engineers completely forget/miss that having information flow from kvm.ko
> to vmlinux is broken (though I am convinced that a large percentage of engineers
> that work (almost) exclusively on prodkernel simply have no clue about how kernel
> modules work in the first place).
>
> I am 100% in favor of dropping kvm_vcpu_arch.l1tf_flush_l1d, but the per-CPU flag
> needs to stay in IRQ stats.  The alternative would be to have KVM (un)register a
> pointer at module (un)load, but I don't see any point in doing so.  And _if_ we
> wanted to go that route, it should be done in a separate patch.

Ack, thanks for the correction. I indeed thought wrongly of kvm_host.h
as the "builtin KVM stuff" place.

I also see from the commit message of
45b575c00d8e72d69d75dd8c112f044b7b01b069 that PeterZ suggested irq_stat
should be cache-hot.

Will wait a day or two in case anyone spots other issues.