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 v2] KVM: x86: Unify L1TF flushing under per-CPU variable
Date: Thu, 16 Oct 2025 08:50:06 -0700 [thread overview]
Message-ID: <aPEULoJUUadbb3nn@google.com> (raw)
In-Reply-To: <20251015-b4-l1tf-percpu-v2-1-6d7a8d3d40e9@google.com>
On Wed, Oct 15, 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.
>
> The vCPU bit was being written from preemption-enabled regions. For
> those cases, use raw_cpu_write() (via a variant of the setter function)
> to avoid DEBUG_PREEMPT failures. If the vCPU is getting migrated, the
> CPU that gets its bit set in these paths is not important; vcpu_load()
> must always set it on the destination CPU before the guest is resumed.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
...
> @@ -78,6 +79,11 @@ static __always_inline void kvm_set_cpu_l1tf_flush_l1d(void)
> __this_cpu_write(irq_stat.kvm_cpu_l1tf_flush_l1d, 1);
> }
>
> +static __always_inline void kvm_set_cpu_l1tf_flush_l1d_raw(void)
> +{
> + raw_cpu_write(irq_stat.kvm_cpu_l1tf_flush_l1d, 1);
> +}
TL;DR: I'll post a v3 with a slightly tweaked version of this patch at the end.
Rather than add a "raw" variant, I would rather have a wrapper in arch/x86/kvm/x86.h
that disables preemption, with a comment explaining why it's ok to enable preemption
after setting the per-CPU flag. Without such a comment, choosing between the two
variants looks entirely random
Alternatively, all writes could be raw, but that
feels wrong/weird, and in practice disabling preemption in the relevant paths is a
complete non-issue.
<me rummages around>
Gah, I followed a tangential thought about the "cost" of disabling/enabling preemtion
and ended up with a 4-patch series. All of this code really should be conditioned
on CONFIG_CPU_MITIGATIONS=y. With that, the wrapper can be:
static __always_inline void kvm_request_l1tf_flush_l1d(void)
{
#if IS_ENABLED(CONFIG_CPU_MITIGATIONS) && IS_ENABLED(CONFIG_KVM_INTEL)
/*
* Temporarily disable preemption (if necessary) as the tracking is
* per-CPU. If the current vCPU task is migrated to a different CPU
* before the next VM-Entry, then kvm_arch_vcpu_load() will pend a
* flush on the new CPU.
*/
guard(preempt)();
kvm_set_cpu_l1tf_flush_l1d();
#endif
}
and kvm_set_cpu_l1tf_flush_l1d() and irq_cpustat_t.kvm_cpu_l1tf_flush_l1d can
likewise be gated on CONFIG_CPU_MITIGATIONS && CONFIG_KVM_INTEL.
> +
> static __always_inline void kvm_clear_cpu_l1tf_flush_l1d(void)
> {
> __this_cpu_write(irq_stat.kvm_cpu_l1tf_flush_l1d, 0);
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 48598d017d6f3f07263a2ffffe670be2658eb9cb..fcdc65ab13d8383018577aacf19e832e6c4ceb0b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1055,9 +1055,6 @@ struct kvm_vcpu_arch {
> /* be preempted when it's in kernel-mode(cpl=0) */
> bool preempted_in_kernel;
>
> - /* Flush the L1 Data cache for L1TF mitigation on VMENTER */
> - bool l1tf_flush_l1d;
> -
> /* Host CPU on which VM-entry was most recently attempted */
> int last_vmentry_cpu;
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 667d66cf76d5e52c22f9517914307244ae868eea..8c0dce401a42d977756ca82d249bb33c858b9c9f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4859,7 +4859,7 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
> */
> BUILD_BUG_ON(lower_32_bits(PFERR_SYNTHETIC_MASK));
>
> - vcpu->arch.l1tf_flush_l1d = true;
> + kvm_set_cpu_l1tf_flush_l1d();
This is wrong, kvm_handle_page_fault() runs with preemption enabled.
next prev parent reply other threads:[~2025-10-16 15:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-15 17:13 Brendan Jackman
2025-10-16 8:59 ` kernel test robot
2025-10-16 8:59 ` kernel test robot
2025-10-16 15:50 ` Sean Christopherson [this message]
2025-10-16 16:28 ` Brendan Jackman
2025-10-16 16:41 ` Sean Christopherson
2025-10-16 16:47 ` 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=aPEULoJUUadbb3nn@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
Powered by JetHome