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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPEULoJUUadbb3nn@google.com>
Date: Thu, 16 Oct 2025 08:50:06 -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 v2] KVM: x86: Unify L1TF flushing under per-CPU variable

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@...gle.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.