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

Zheyun Shen
2024-03-05T14:57:13Z

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.

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?

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() ?

Best regards,
Zheyun Shen