Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 20 Jul 2022 21:54:14 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Santosh Shukla <santosh.shukla@amd.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Maxim Levitsky <mlevitsk@redhat.com>,
        Tom Lendacky <thomas.lendacky@amd.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2 4/7] KVM: SVM: Report NMI not allowed when Guest busy
 handling VNMI
Message-ID: <Yth5hl+RlTaa5ybj@google.com>
References: <20220709134230.2397-1-santosh.shukla@amd.com>
 <20220709134230.2397-5-santosh.shukla@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220709134230.2397-5-santosh.shukla@amd.com>
Precedence: bulk

On Sat, Jul 09, 2022, Santosh Shukla wrote:
> In the VNMI case, Report NMI is not allowed when the processor set the
> V_NMI_MASK to 1 which means the Guest is busy handling VNMI.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
> ---
> v2:
> - Moved vnmi check after is_guest_mode() in func _nmi_blocked().
> - Removed is_vnmi_mask_set check from _enable_nmi_window().
> as it was a redundent check.
> 
>  arch/x86/kvm/svm/svm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3574e804d757..44c1f2317b45 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3480,6 +3480,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
>  	if (is_guest_mode(vcpu) && nested_exit_on_nmi(svm))
>  		return false;
>  
> +	if (is_vnmi_enabled(svm) && is_vnmi_mask_set(svm))
> +		return true;
> +
>  	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
>  	      (vcpu->arch.hflags & HF_NMI_MASK);
>  
> @@ -3609,6 +3612,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
> +	if (is_vnmi_enabled(svm))
> +		return;

Ugh, is there really no way to trigger an exit when NMIs become unmasked?  Because
if there isn't, this is broken for KVM.

On bare metal, if two NMIs arrive "simultaneously", so long as NMIs aren't blocked,
the first NMI will be delivered and the second will be pended, i.e. software will
see both NMIs.  And if that doesn't hold true, the window for a true collision is
really, really tiny.

But in KVM, because a vCPU may not be run a long duration, that window becomes
very large.  To not drop NMIs and more faithfully emulate hardware, KVM allows two
NMIs to be _pending_.  And when that happens, KVM needs to trigger an exit when
NMIs become unmasked _after_ the first NMI is injected.

> +
>  	if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK)
>  		return; /* IRET will cause a vm exit */
>  
> -- 
> 2.25.1
>