Re: [PATCH v5 1/2] KVM: nSVM: Raise #UD if unhandled VMMCALL isn't intercepted by L1

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@xxxxxxxxxx> writes:

> From: Kevin Cheng <chengkev@xxxxxxxxxx>
>
> Explicitly synthesize a #UD for VMMCALL if L2 is active, L1 does NOT want
> to intercept VMMCALL, nested_svm_l2_tlb_flush_enabled() is true, and the
> hypercall is something other than one of the supported Hyper-V hypercalls.
> When all of the above conditions are met, KVM will intercept VMMCALL but
> never forward it to L1, i.e. will let L2 make hypercalls as if it were L1.
>
> The TLFS says a whole lot of nothing about this scenario, so go with the
> architectural behavior, which says that VMMCALL #UDs if it's not
> intercepted.
>
> Opportunistically do a 2-for-1 stub trade by stub-ifying the new API
> instead of the helpers it uses.  The last remaining "single" stub will
> soon be dropped as well.
>
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Fixes: 3f4a812edf5c ("KVM: nSVM: hyper-v: Enable L2 TLB flush")
> Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Kevin Cheng <chengkev@xxxxxxxxxx>
> Link: https://patch.msgid.link/20260228033328.2285047-5-chengkev@xxxxxxxxxx
> [sean: rewrite changelog and comment, tag for stable, remove defunct stubs]
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/kvm/hyperv.h     |  8 --------
>  arch/x86/kvm/svm/hyperv.h | 11 +++++++++++
>  arch/x86/kvm/svm/nested.c |  4 +---
>  arch/x86/kvm/svm/svm.c    | 19 ++++++++++++++++++-
>  4 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 6ce160ffa678..6301f79fcbae 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -305,14 +305,6 @@ static inline bool kvm_hv_has_stimer_pending(struct kvm_vcpu *vcpu)
>  {
>  	return false;
>  }
> -static inline bool kvm_hv_is_tlb_flush_hcall(struct kvm_vcpu *vcpu)
> -{
> -	return false;
> -}
> -static inline bool guest_hv_cpuid_has_l2_tlb_flush(struct kvm_vcpu *vcpu)
> -{
> -	return false;
> -}
>  static inline int kvm_hv_verify_vp_assist(struct kvm_vcpu *vcpu)
>  {
>  	return 0;
> diff --git a/arch/x86/kvm/svm/hyperv.h b/arch/x86/kvm/svm/hyperv.h
> index d3f8bfc05832..9af03970d40c 100644
> --- a/arch/x86/kvm/svm/hyperv.h
> +++ b/arch/x86/kvm/svm/hyperv.h
> @@ -41,6 +41,13 @@ static inline bool nested_svm_l2_tlb_flush_enabled(struct kvm_vcpu *vcpu)
>  	return hv_vcpu->vp_assist_page.nested_control.features.directhypercall;
>  }
>  
> +static inline bool nested_svm_is_l2_tlb_flush_hcall(struct kvm_vcpu *vcpu)
> +{
> +	return guest_hv_cpuid_has_l2_tlb_flush(vcpu) &&
> +	       nested_svm_l2_tlb_flush_enabled(vcpu) &&
> +	       kvm_hv_is_tlb_flush_hcall(vcpu);
> +}
> +
>  void svm_hv_inject_synthetic_vmexit_post_tlb_flush(struct kvm_vcpu *vcpu);
>  #else /* CONFIG_KVM_HYPERV */
>  static inline void nested_svm_hv_update_vm_vp_ids(struct kvm_vcpu *vcpu) {}
> @@ -48,6 +55,10 @@ static inline bool nested_svm_l2_tlb_flush_enabled(struct kvm_vcpu *vcpu)
>  {
>  	return false;
>  }
> +static inline bool nested_svm_is_l2_tlb_flush_hcall(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
>  static inline void svm_hv_inject_synthetic_vmexit_post_tlb_flush(struct kvm_vcpu *vcpu) {}
>  #endif /* CONFIG_KVM_HYPERV */
>  
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 53ab6ce3cc26..750bf93c5341 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1674,9 +1674,7 @@ int nested_svm_exit_special(struct vcpu_svm *svm)
>  	}
>  	case SVM_EXIT_VMMCALL:
>  		/* Hyper-V L2 TLB flush hypercall is handled by L0 */
> -		if (guest_hv_cpuid_has_l2_tlb_flush(vcpu) &&
> -		    nested_svm_l2_tlb_flush_enabled(vcpu) &&
> -		    kvm_hv_is_tlb_flush_hcall(vcpu))
> +		if (nested_svm_is_l2_tlb_flush_hcall(vcpu))
>  			return NESTED_EXIT_HOST;
>  		break;
>  	default:
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 8f8bc863e214..38a2fad81ad8 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -52,6 +52,7 @@
>  #include "svm.h"
>  #include "svm_ops.h"
>  
> +#include "hyperv.h"
>  #include "kvm_onhyperv.h"
>  #include "svm_onhyperv.h"
>  
> @@ -3228,6 +3229,22 @@ static int bus_lock_exit(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>  
> +static int vmmcall_interception(struct kvm_vcpu *vcpu)
> +{
> +	/*
> +	 * Inject a #UD if L2 is active and the VMMCALL isn't a Hyper-V TLB
> +	 * hypercall, as VMMCALL #UDs if it's not intercepted, and this path is
> +	 * reachable if and only if L1 doesn't want to intercept VMMCALL or has
> +	 * enabled L0 (KVM) handling of Hyper-V L2 TLB flush hypercalls.
> +	 */
> +	if (is_guest_mode(vcpu) && !nested_svm_is_l2_tlb_flush_hcall(vcpu)) {
> +		kvm_queue_exception(vcpu, UD_VECTOR);
> +		return 1;
> +	}
> +
> +	return kvm_emulate_hypercall(vcpu);
> +}
> +
>  static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[SVM_EXIT_READ_CR0]			= cr_interception,
>  	[SVM_EXIT_READ_CR3]			= cr_interception,
> @@ -3278,7 +3295,7 @@ static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[SVM_EXIT_TASK_SWITCH]			= task_switch_interception,
>  	[SVM_EXIT_SHUTDOWN]			= shutdown_interception,
>  	[SVM_EXIT_VMRUN]			= vmrun_interception,
> -	[SVM_EXIT_VMMCALL]			= kvm_emulate_hypercall,
> +	[SVM_EXIT_VMMCALL]			= vmmcall_interception,
>  	[SVM_EXIT_VMLOAD]			= vmload_interception,
>  	[SVM_EXIT_VMSAVE]			= vmsave_interception,
>  	[SVM_EXIT_STGI]				= stgi_interception,

Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

-- 
Vitaly