linux-kernel - Re: [PATCH] KVM:SVM: Flush cache only on CPUs running SEV guest

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] [day] [month] [year] [list]

Message-ID: <ZedNH2DFpsg5RvHC@google.com>
Date: Tue, 5 Mar 2024 08:49:35 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Zheyun Shen <szy0127@...u.edu.cn>
Cc: pbonzini <pbonzini@...hat.com>, tglx <tglx@...utronix.de>, 
	thomas lendacky <thomas.lendacky@....com>, kvm <kvm@...r.kernel.org>, 
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM:SVM: Flush cache only on CPUs running SEV guest

On Tue, Mar 05, 2024, Zheyun Shen wrote:
> On Mon, Mar 04, 2024, Sean Christopherson wrote:
> > Instead of copy+paste WBINVD+cpumask_clear() everywhere, add a prep patch to
> > replace relevant open coded calls to wbinvd_on_all_cpus() with calls to
> > sev_guest_memory_reclaimed().  Then only sev_guest_memory_reclaimed() needs to
> > updated, and IMO it helps document why KVM is blasting WBINVD.
> 
> > I'm also pretty sure this should be a cpumask_var_t, and dynamically allocated
> > as appropriate.  And at that point, it should be allocated and filled if and only
> > if the CPU doesn't have X86_FEATURE_SME_COHERENT
> 
> I notice that several callers of wbinvd_on_all_cpus() must use wbinvd to flush cache
> instead of using clflush or just doing nothing if the CPU has X86_FEATURE_SME_COHERENT,
> according to https://github.com/AMDESE/linux/commit/2e2409afe5f0c284c7dfe5504058e8d115806a7d
> Therefore, I think the flush operation should be divided into two functions. One is the 
> optimized wbinvd, which does not consider X86_FEATURE_SME_COHERENT, and the other is 
> sev_guest_memory_reclaimed(), which should use clflush instead of wbinvd in case of 
> X86_FEATURE_SME_COHERENT. Thus the cpumask struct should be exist whether the CPU has
> X86_FEATURE_SME_COHERENT or not.

FWIW, the usage of sev_flush_asids() isn't tied to a single VM, i.e. KVM can't use
per-VM tracking in that case.  But...

> Besides, if we consider X86_FEATURE_SME_COHERENT to get rid of wbinvd in sev_guest_memory_reclaimed(),
> we should ensure the clflush is called on corresponding addresses, as mentioned in  
> https://github.com/AMDESE/linux/commit/d45829b351ee6ec5f54dd55e6aca1f44fe239fe6 
> However, caller of sev_guest_memory_reclaimed() (e.g., kvm_mmu_notifier_invalidate_range_start()) 
> only get HVA belongs to userspace(e.g., qemu), so calling clflush with this HVA may 
> lead to a page fault in kernel. I was wondering if notifying userspace applications to 
> do clflush themselves is the only solution here. But for the sake of safety, maybe KVM 
> cannot left the work for untrusted userspace applications?

Ugh, right, I forgot the whole mess with userspace virtual addresses.  Bummer.

> Or should I just temporarily ignore the X86_FEATURE_SME_COHERENT scenario
> which is hard to implement, and just refine the patch only for
> wbinvd_on_all_cpus() ?

Ignore X86_FEATURE_SME_COHERENT and just refine the patch to optimize WBINVDs that
are tied to a specific VM.  I simply forgot that KVM only uses CLFLUSHOPT when
purging VMSA pages.