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

[Brendan Jackman <jackmanb@google.com>]

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.