Re: [PATCH v2 4/5] KVM: nVMX: Emulate MTF when performing instruction emulation

From: Jim Mattson <jmattson@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Oliver Upton <oupton@google.com>,
	kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Peter Shier <pshier@google.com>
Subject: Re: [PATCH v2 4/5] KVM: nVMX: Emulate MTF when performing instruction emulation
Date: Fri, 13 Aug 2021 10:03:28 -0700	[thread overview]
Message-ID: <CALMp9eR4_BFwoKGRC31hFo2ZX4iyF5k_APrW-hahqWbJ3cttLQ@mail.gmail.com> (raw)
In-Reply-To: <YRafVro7jZoswngG@google.com>

On Fri, Aug 13, 2021 at 9:35 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 12, 2021, Jim Mattson wrote:
> > On Tue, Jan 28, 2020 at 1:27 AM Oliver Upton <oupton@google.com> wrote:
> > >
> > > Since commit 5f3d45e7f282 ("kvm/x86: add support for
> > > MONITOR_TRAP_FLAG"), KVM has allowed an L1 guest to use the monitor trap
> > > flag processor-based execution control for its L2 guest. KVM simply
> > > forwards any MTF VM-exits to the L1 guest, which works for normal
> > > instruction execution.
> > >
> > > However, when KVM needs to emulate an instruction on the behalf of an L2
> > > guest, the monitor trap flag is not emulated. Add the necessary logic to
> > > kvm_skip_emulated_instruction() to synthesize an MTF VM-exit to L1 upon
> > > instruction emulation for L2.
> > >
> > > Fixes: 5f3d45e7f282 ("kvm/x86: add support for MONITOR_TRAP_FLAG")
> > > Signed-off-by: Oliver Upton <oupton@google.com>
> > > ---
>
> ...
>
> > > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > > index 503d3f42da16..3f3f780c8c65 100644
> > > --- a/arch/x86/include/uapi/asm/kvm.h
> > > +++ b/arch/x86/include/uapi/asm/kvm.h
> > > @@ -390,6 +390,7 @@ struct kvm_sync_regs {
> > >  #define KVM_STATE_NESTED_GUEST_MODE    0x00000001
> > >  #define KVM_STATE_NESTED_RUN_PENDING   0x00000002
> > >  #define KVM_STATE_NESTED_EVMCS         0x00000004
> > > +#define KVM_STATE_NESTED_MTF_PENDING   0x00000008
> >
> > Maybe I don't understand the distinction, but shouldn't this new flag
> > have a KVM_STATE_NESTED_VMX prefix and live with
> > KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE, below?
>
> That does seem to be the case, seems highly unlikely SVM will add MTF.  And SVM's
> KVM_STATE_NESTED_GIF_SET should have been SVM specific, but kvm_svm_nested_state_hdr
> doesn't even reserve a flags field :-/

APIs are hard.