[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aO0pf8h8k0NddyvX@google.com>
Date: Mon, 13 Oct 2025 09:31:59 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: 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, 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.
Powered by blists - more mailing lists