public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Brendan Jackman <jackmanb@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org,  kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Unify L1TF flushing under per-CPU variable
Date: Mon, 13 Oct 2025 09:31:59 -0700	[thread overview]
Message-ID: <aO0pf8h8k0NddyvX@google.com> (raw)
In-Reply-To: <20251013-b4-l1tf-percpu-v1-1-d65c5366ea1a@google.com>

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.

  reply	other threads:[~2025-10-13 16:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 15:20 [PATCH] KVM: x86: Unify L1TF flushing under per-CPU variable Brendan Jackman
2025-10-13 16:31 ` Sean Christopherson [this message]
2025-10-13 16:52   ` Brendan Jackman
2025-10-14  6:13 ` [syzbot ci] " syzbot ci
2025-10-14  8:57   ` Brendan Jackman
2025-10-14 11:46     ` Brendan Jackman
2025-10-14  7:24 ` [PATCH] " kernel test robot
2025-10-14  9:02   ` Brendan Jackman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aO0pf8h8k0NddyvX@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jackmanb@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox