On Mon, Feb 03, 2020 at 11:13:30AM -0800, Sean Christopherson wrote: > On Tue, Jan 28, 2020 at 01:27:12AM -0800, Oliver Upton wrote: > > SDM 27.3.4 states that the 'pending debug exceptions' VMCS field will > > be populated if a VM-exit caused by an INIT signal takes priority over a > > debug-trap. Emulate this behavior when synthesizing an INIT signal > > VM-exit into L1. > > > > Fixes: 558b8d50dbff ("KVM: x86: Fix INIT signal handling in various CPU states") > > Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx> > > --- > > arch/x86/kvm/vmx/nested.c | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 95b3f4306ac2..aba16599ca69 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -3572,6 +3572,27 @@ static void nested_vmx_inject_exception_vmexit(struct kvm_vcpu *vcpu, > > nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI, intr_info, exit_qual); > > } > > > > +static inline bool nested_vmx_check_pending_dbg(struct kvm_vcpu *vcpu) > > Really dislike the name, partially because the code checks @has_payload and > partially because the part, nested_vmx_set_pending_dbg() "sets" completely > different state than this checks. > > Checking has_payload may also be wrong, e.g. wouldn't it make sense to > update GUEST_PENDING_DBG_EXCEPTIONS, even if we crush it with '0'? > > > +{ > > + return vcpu->arch.exception.nr == DB_VECTOR && > > + vcpu->arch.exception.pending && > > + vcpu->arch.exception.has_payload; > > +} > > + > > +/* > > + * If a higher priority VM-exit is delivered before a debug-trap, hardware will > > + * set the 'pending debug exceptions' field appropriately for reinjection on the > > + * next VM-entry. > > + */ > > +static void nested_vmx_set_pending_dbg(struct kvm_vcpu *vcpu) > > +{ > > + vmcs_writel(GUEST_PENDING_DBG_EXCEPTIONS, vcpu->arch.exception.payload); > > + vcpu->arch.exception.has_payload = false; > > + vcpu->arch.exception.payload = 0; > > + vcpu->arch.exception.pending = false; > > + vcpu->arch.exception.injected = true; > > This looks wrong. The #DB hasn't been injected, KVM is simply emulating > the side effect of the VMCS field being updated. E.g. KVM will have > different architecturally visible behavior depending on @has_payload. My head is spinning trying to work through the #DB/MTF interactions. I think this ends up being a moot point because prepare_vmcs12() will purge the pending exceptions. If it is a moot point, then I'd prefer to not do the explicit arch.exception updates so as to keep this similar to other exceptions.