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

[Sean Christopherson]

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.