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;
Message-ID: <1521442faa5abeb4449209550015040f2b2db849.camel@redhat.com>
Subject: Re: [PATCH v2 05/21] KVM: nVMX: Prioritize TSS T-flag #DBs over
 Monitor Trap Flag
From:   Maxim Levitsky <mlevitsk@redhat.com>
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Cc:     Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Oliver Upton <oupton@google.com>,
        Peter Shier <pshier@google.com>
Date:   Wed, 06 Jul 2022 14:57:31 +0300
In-Reply-To: <20220614204730.3359543-6-seanjc@google.com>
References: <20220614204730.3359543-1-seanjc@google.com>
         <20220614204730.3359543-6-seanjc@google.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2022-06-14 at 20:47 +0000, Sean Christopherson wrote:
> Service TSS T-flag #DBs prior to pending MTFs, as such #DBs are higher
> priority than MTF. 
>  KVM itself doesn't emulate TSS #DBs, and any such
> exceptions injected from L1 will be handled by hardware (or morphed to
> a fault-like exception if injection fails), but theoretically userspace
> could pend a TSS T-flag #DB in conjunction with a pending MTF.


After reading the Jim's table 6-2, this makes sense, however note that
*check_nested_events is a bit different in the regard that CPU checks
the events when the previous instruction fully done committing it state,
and all it is left of it is maybe pending trap like events,

but in the KVM, the *check_nested_events, happens when we still didn't deliver
the fault like exception from the previous instruction, and thus,
a fault like exception appears to have higher priority than a pending MTF.

Assuming that my analysis is right:

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky


> 
> Note, there's no known use case this fixes, it's purely to be technically
> correct with respect to Intel's SDM.
> 
> Cc: Oliver Upton <oupton@google.com>
> Cc: Peter Shier <pshier@google.com>
> Fixes: 5ef8acbdd687 ("KVM: nVMX: Emulate MTF when performing instruction emulation")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 61bc80fc4cfa..e794791a6bdd 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -3943,15 +3943,17 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
>  	}
>  
>  	/*
> -	 * Process any exceptions that are not debug traps before MTF.
> +	 * Process exceptions that are higher priority than Monitor Trap Flag:
> +	 * fault-like exceptions, TSS T flag #DB (not emulated by KVM, but
> +	 * could theoretically come in from userspace), and ICEBP (INT1).
>  	 *
>  	 * Note that only a pending nested run can block a pending exception.
>  	 * Otherwise an injected NMI/interrupt should either be
>  	 * lost or delivered to the nested hypervisor in the IDT_VECTORING_INFO,
>  	 * while delivering the pending exception.
>  	 */
> -
> -	if (vcpu->arch.exception.pending && !vmx_get_pending_dbg_trap(vcpu)) {
> +	if (vcpu->arch.exception.pending &&
> +	    !(vmx_get_pending_dbg_trap(vcpu) & ~DR6_BT)) {
>  		if (vmx->nested.nested_run_pending)
>  			return -EBUSY;
>  		if (!nested_vmx_check_exception(vcpu, &exit_qual))