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 [PATCH v2] KVM: x86: Unify L1TF flushing under per-CPU variable 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.