Re: [PATCH] KVM: SVM: Flush Hyper-V TLB when required

Sean Christopherson
2023-03-24T14:10:16Z

On Fri, Mar 24, 2023, Jeremi Piotrowski wrote:
> I have the #ifdef version ready to send out, but what do you think about this:

Oh, nice! Yeah, that works, I didn't see the stub for hyperv_flush_guest_mapping().

> @@ -3753,6 +3753,39 @@ static void svm_flush_tlb_current(struct kvm_vcpu *vcpu)
> svm->current_vmcb->asid_generation--;
> }
>
> +static void svm_flush_tlb_current(struct kvm_vcpu *vcpu)
> +{
> + hpa_t root_tdp = vcpu->arch.mmu->root.hpa;
> +
> + /*
> + * When running on Hyper-V with EnlightenedNptTlb enabled, explicitly
> + * flush the NPT mappings via hypercall as flushing the ASID only
> + * affects virtual to physical mappings, it does not invalidate guest
> + * physical to host physical mappings.
> + */
> + if (IS_ENABLED(CONFIG_HYPERV) &&

No need for the IS_ENABLED(CONFIG_HYPERV) check here, the svm_hv_is_enlightened_tlb_enabled()
stub that's provided for CONFIG_HYPERV=n will guard this properly

if (svm_hv_is_enlightened_tlb_enabled(vcpu) && VALID_PAGE(root_tdp))
hyperv_flush_guest_mapping(root_tdp);

> + svm_hv_is_enlightened_tlb_enabled(vcpu) &&
> + VALID_PAGE(root_tdp))
> + hyperv_flush_guest_mapping(root_tdp);
> +
> + svm_flush_tlb_asid(vcpu);
> +}
> +
> +static void svm_flush_tlb_all(struct kvm_vcpu *vcpu)
> +{
> + /*
> + * When running on Hyper-V with EnlightenedNptTlb enabled, remote TLB
> + * flushes should be routed to hv_remote_flush_tlb() without requesting
> + * a "regular" remote flush. Reaching this point means either there's
> + * a KVM bug or a prior hv_remote_flush_tlb() call failed, both of
> + * which might be fatal to the guest. Yell, but try to recover.
> + */
> + if (IS_ENABLED(CONFIG_HYPERV) && WARN_ON_ONCE(svm_hv_is_enlightened_tlb_enabled(vcpu)))
> + hv_remote_flush_tlb(vcpu->kvm);

And then

if (WARN_ON_ONCE(svm_hv_is_enlightened_tlb_enabled(vcpu)))
hv_remote_flush_tlb(vcpu->kvm);

> +
> + svm_flush_tlb_asid(vcpu);
> +}
> +
> static void svm_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t gva)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> @@ -4745,10 +4778,10 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
> .set_rflags = svm_set_rflags,
> .get_if_flag = svm_get_if_flag,
>
> - .flush_tlb_all = svm_flush_tlb_current,
> + .flush_tlb_all = svm_flush_tlb_all,
> .flush_tlb_current = svm_flush_tlb_current,
> .flush_tlb_gva = svm_flush_tlb_gva,
> - .flush_tlb_guest = svm_flush_tlb_current,
> + .flush_tlb_guest = svm_flush_tlb_asid,
>
> .vcpu_pre_run = svm_vcpu_pre_run,
> .vcpu_run = svm_vcpu_run,
> diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
> index cff838f15db5..4c9e0d4ba3dd 100644
> --- a/arch/x86/kvm/svm/svm_onhyperv.h
> +++ b/arch/x86/kvm/svm/svm_onhyperv.h
> @@ -6,6 +6,8 @@
> #ifndef __ARCH_X86_KVM_SVM_ONHYPERV_H__
> #define __ARCH_X86_KVM_SVM_ONHYPERV_H__
>
> +#include <asm/mshyperv.h>
> +
> #if IS_ENABLED(CONFIG_HYPERV)
>
> #include "kvm_onhyperv.h"
> @@ -15,6 +17,14 @@ static struct kvm_x86_ops svm_x86_ops;
>
> int svm_hv_enable_l2_tlb_flush(struct kvm_vcpu *vcpu);
>
> +static inline bool svm_hv_is_enlightened_tlb_enabled(struct kvm_vcpu *vcpu)
> +{
> + struct hv_vmcb_enlightenments *hve = &to_svm(vcpu)->vmcb->control.hv_enlightenments;
> +
> + return ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB &&
> + !!hve->hv_enlightenments_control.enlightened_npt_tlb;

Uber nit, align the indentation (7 spaces instead of 1 tab):

return ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB &&
!!hve->hv_enlightenments_control.enlightened_npt_tlb;